From 4d5c02c19603f134da15e68d0f2f599768ff666d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Tue, 1 Nov 2016 12:33:03 +0100 Subject: [PATCH] Usable SVN activities --- attract/__init__.py | 2 +- attract/node_types/task.py | 5 +++ attract/routes.py | 1 + attract/setup.py | 5 ++- attract/shortcodes.py | 43 ++++++++++++++++++++ attract/shots/__init__.py | 2 + attract/subversion/__init__.py | 25 ++++++------ attract/subversion/routes.py | 13 ++++-- attract/tasks/__init__.py | 72 ++++++++++++++++++++++++++++------ attract/tasks/eve_hooks.py | 19 +++++++-- notify_attract.py | 63 +++++++++++++++++++++++++++++ tests/abstract_attract_test.py | 4 ++ tests/logging_config.py | 4 +- tests/test_shortcode.py | 41 +++++++++++++++++++ tests/test_subversion.py | 21 ++++++---- 15 files changed, 278 insertions(+), 42 deletions(-) create mode 100644 attract/shortcodes.py create mode 100755 notify_attract.py create mode 100644 tests/test_shortcode.py diff --git a/attract/__init__.py b/attract/__init__.py index 48adce6..a9a671a 100644 --- a/attract/__init__.py +++ b/attract/__init__.py @@ -90,7 +90,7 @@ class AttractExtension(PillarExtension): from . import subversion, tasks, eve_hooks, shots - subversion.task_logged.connect(self.task_manager.task_logged_in_svn) + subversion.task_logged.connect(self.task_manager.api_task_logged_in_svn) tasks.setup_app(app) shots.setup_app(app) eve_hooks.setup_app(app) diff --git a/attract/node_types/task.py b/attract/node_types/task.py index 4ab4392..caa97d6 100644 --- a/attract/node_types/task.py +++ b/attract/node_types/task.py @@ -63,6 +63,11 @@ node_type_task = { }, } }, + 'shortcode': { + 'type': 'string', + 'required': False, + 'maxlength': 16, + }, }, 'form_schema': { diff --git a/attract/routes.py b/attract/routes.py index b7d6c4c..fa61d44 100644 --- a/attract/routes.py +++ b/attract/routes.py @@ -8,6 +8,7 @@ from pillar.web.utils import attach_project_pictures import pillar.web.subquery from pillar.web.system_util import pillar_api import pillarsdk +import pillarsdk.exceptions as sdk_exceptions from attract import current_attract from attract.node_types.task import node_type_task diff --git a/attract/setup.py b/attract/setup.py index d7f69e5..4297ecc 100644 --- a/attract/setup.py +++ b/attract/setup.py @@ -91,7 +91,10 @@ def setup_for_attract(project_url, replace=False, svn_url=None): # Set default extension properties. Be careful not to overwrite any properties that # are already there. eprops = project.setdefault('extension_props', {}) - attract_props = eprops.setdefault(EXTENSION_NAME, {}) + attract_props = eprops.setdefault(EXTENSION_NAME, { + 'last_used_shortcodes': {}, + 'svn_usermap': {}, # mapping from SVN username to Pillar user ObjectID. + }) if svn_url: log.info('Setting SVN URL to %s', svn_url) diff --git a/attract/shortcodes.py b/attract/shortcodes.py new file mode 100644 index 0000000..54359b3 --- /dev/null +++ b/attract/shortcodes.py @@ -0,0 +1,43 @@ +"""Shortcode management. + +A shortcode is an incremental number that's unique per project and per type. +Example types are 'task' and 'shot. +""" + +import logging + +from bson import ObjectId +from flask import current_app +import pymongo + +log = logging.getLogger(__name__) + + +def generate_shortcode(project_id, node_type, prefix): + """Generates and returns a new shortcode. + + :param project_id: project ID + :type project_id: bson.ObjectId + :param node_type: the type, for now 'attract_task' or 'attract_shot', but can be extended. + :type node_type: unicode + :param prefix: one-letter prefix for these shortcodes + :type prefix: unicode + """ + + assert isinstance(project_id, ObjectId) + + db = current_app.db() + db_fieldname = 'extension_props.attract.last_used_shortcodes.%s' % node_type + + log.debug('Incrementing project %s shortcode for type %r', + project_id, node_type) + + new_proj = db['projects'].find_one_and_update( + {'_id': project_id}, + {'$inc': {db_fieldname: 1}}, + {db_fieldname: 1}, + return_document=pymongo.ReturnDocument.AFTER, + ) + + shortcode = new_proj['extension_props']['attract']['last_used_shortcodes'][node_type] + return '%s%i' % (prefix, shortcode) diff --git a/attract/shots/__init__.py b/attract/shots/__init__.py index 273a43b..3414d9b 100644 --- a/attract/shots/__init__.py +++ b/attract/shots/__init__.py @@ -6,6 +6,7 @@ import logging import attr import flask import flask_login +from bson import ObjectId from eve.methods.put import put_internal from werkzeug import exceptions as wz_exceptions @@ -76,6 +77,7 @@ class ShotManager(object): :rtype: pillarsdk.Node """ + from attract import shortcodes project_id = project['_id'] self._log.info('Creating shot for project %s', project_id) diff --git a/attract/subversion/__init__.py b/attract/subversion/__init__.py index 2f430fc..dfd607e 100644 --- a/attract/subversion/__init__.py +++ b/attract/subversion/__init__.py @@ -13,12 +13,10 @@ import svn.common from pillar import attrs_extra task_logged = blinker.NamedSignal('task_logged') -shot_logged = blinker.NamedSignal('shot_logged') -marker_re = re.compile(r'\[(?P[TS])(?P[0-9a-zA-Z]+)\]') +marker_re = re.compile(r'\[(?P[TS])(?P[0-9a-zA-Z]+)\]') signals = { 'T': task_logged, - 'S': shot_logged, } # Copy of namedtuple defined in svn.common.log_default(). @@ -75,7 +73,7 @@ class CommitLogObserver(object): self.process_log(log_entry) except svn.common.SvnException: - # The SVN library just raises a SvnCommandError when something goes wrong, + # The SVN library just raises a SvnException when something goes wrong, # without any structured indication of the error. There isn't much else # we can do, except to log the error and return. self._log.exception('Error calling self.svn_client.log_default()') @@ -89,15 +87,16 @@ class CommitLogObserver(object): self._log.debug('%s: process_log() rev=%s, author=%s', self, log_entry.revision, log_entry.author) - for node_type, shortcode in self._find_ids(log_entry.msg): - signal = signals[node_type] - signal.send(self, shortcode=shortcode, log_entry=log_entry) + tasks_found = 0 + for codetype, shortcode in self._find_ids(log_entry.msg): + signal = signals[codetype] + self._log.debug('Sending signal %s with shortcode=%s%s', signal, codetype, shortcode) + signal.send(self, shortcode='%s%s' % (codetype, shortcode), log_entry=log_entry) + tasks_found += 1 def _find_ids(self, message): # Parse the commit log to see if there are any task/shot markers. - lines = message.split('\n', 100)[:100] - for line in lines: - for match in marker_re.finditer(line): - type = match.group('type') - shortcode = match.group('shortcode') - yield type, shortcode + for match in marker_re.finditer(message[:1024]): + codetype = match.group('codetype') + shortcode = match.group('shortcode') + yield codetype, shortcode diff --git a/attract/subversion/routes.py b/attract/subversion/routes.py index f766815..02a5b5d 100644 --- a/attract/subversion/routes.py +++ b/attract/subversion/routes.py @@ -44,10 +44,14 @@ def subversion_log(project_url): # Parse the request args = request.json - revision = args['revision'] - commit_message = args['msg'] - commit_author = args['author'] - commit_date = args['date'] + try: + revision = args['revision'] + commit_message = args['msg'] + commit_author = args['author'] + commit_date = args['date'] + except KeyError as ex: + log.info('subversion_log(%s): request is missing key %s', project_url, ex) + raise wz_exceptions.BadRequest() current_user_id = authentication.current_user_id() log.info('Service account %s registers SVN commit %s of user %s', @@ -90,6 +94,7 @@ def subversion_log(project_url): author=commit_author, date_text=commit_date) observer = subversion.CommitLogObserver() + log.debug('Processing %s via %s', log_entry, observer) observer.process_log(log_entry) return 'Registered in Attract' diff --git a/attract/tasks/__init__.py b/attract/tasks/__init__.py index a4887a0..3fe394a 100644 --- a/attract/tasks/__init__.py +++ b/attract/tasks/__init__.py @@ -1,11 +1,14 @@ """Task management.""" import attr +import flask import flask_login import pillarsdk from pillar import attrs_extra +from pillar.api.activities import register_activity from pillar.web.system_util import pillar_api +from pillar.api.utils import authentication from attract.node_types.task import node_type_task @@ -14,17 +17,6 @@ from attract.node_types.task import node_type_task class TaskManager(object): _log = attrs_extra.log('%s.TaskManager' % __name__) - def task_logged_in_svn(self, sender, shortcode, log_entry): - """Blinker signal receiver; connects the logged commit with the task. - - :param sender: sender of the signal - :type sender: attract_server.subversion.CommitLogObserver - :type log_entry: attract.subversion.LogEntry - """ - - self._log.info("Task '%s' logged in SVN by %s: %s", - shortcode, log_entry.author, log_entry.msg) - def create_task(self, project, task_type=None, parent=None): """Creates a new task, owned by the current user. @@ -126,6 +118,64 @@ class TaskManager(object): }}, api=api) return tasks + def api_task_for_shortcode(self, shortcode): + """Returns the task for the given shortcode. + + :returns: the task Node, or None if not found. + """ + + db = flask.current_app.db() + task = db['nodes'].find_one({ + 'properties.shortcode': shortcode, + 'node_type': node_type_task['name'], + }) + + return task + + def api_task_logged_in_svn(self, sender, shortcode, log_entry): + """Blinker signal receiver; connects the logged commit with the task. + + :param sender: sender of the signal + :type sender: attract_server.subversion.CommitLogObserver + :type log_entry: attract.subversion.LogEntry + """ + + self._log.info(u"Task '%s' logged in SVN by %s: %s...", + shortcode, log_entry.author, log_entry.msg[:30].replace('\n', ' // ')) + + # Find the task + task = self.api_task_for_shortcode(shortcode) + if not task: + self._log.warning(u'Task %s not found, ignoring SVN commit.', shortcode) + return + + # Find the author + db = flask.current_app.db() + proj = db['projects'].find_one({'_id': task['project']}, + projection={'extension_props.attract': 1}) + if not proj: + self._log.warning(u'Project %s for task %s not found, ignoring SVN commit.', + task['project'], task['_id']) + return + + # We have to have a user ID to register an activity, which is why we fall back + # to the current user (the SVNer service account) if there is no mapping. + usermap = proj['extension_props'].get('attract', {}).get('svn_usermap', {}) + user_id = usermap.get(log_entry.author, None) + msg = 'committed SVN revision %s' % log_entry.revision + if not user_id: + self._log.warning(u'No Pillar user mapped for SVN user %s, using SVNer account.', + log_entry.author) + user_id = authentication.current_user_id() + msg = 'committed SVN revision %s authored by SVN user %s' % ( + log_entry.revision, log_entry.author) + + register_activity( + user_id, msg, + 'node', task['_id'], + 'node', task['parent'] or task['_id'], + project_id=task['project']) + def setup_app(app): from . import eve_hooks diff --git a/attract/tasks/eve_hooks.py b/attract/tasks/eve_hooks.py index a5b1c9b..5955926 100644 --- a/attract/tasks/eve_hooks.py +++ b/attract/tasks/eve_hooks.py @@ -113,9 +113,8 @@ def get_user_list(user_list): return u'-nobody-' user_coll = current_app.db()['users'] - users = user_coll.find({ - '_id': {'$in': user_list} - }, + users = user_coll.find( + {'_id': {'$in': user_list}}, projection={ 'full_name': 1, } @@ -172,11 +171,25 @@ def activity_after_deleting_task(task): register_task_activity(task, 'deleted task "%s"' % task['name']) +@only_for_task +def create_shortcode(task): + from attract import shortcodes + + shortcode = shortcodes.generate_shortcode(task['project'], task['node_type'], u'T') + task.setdefault('properties', {})['shortcode'] = shortcode + + +def create_shortcodes(nodes): + for node in nodes: + create_shortcode(node) + + def setup_app(app): app.on_fetched_item_nodes += fetch_task_extra_info app.on_fetched_resource_nodes += fetch_tasks_parent_info app.on_replaced_nodes += activity_after_replacing_task app.on_inserted_nodes += activity_after_creating_tasks + app.on_insert_nodes += create_shortcodes app.on_deleted_item_nodes += activity_after_deleting_task app.on_deleted_resource_nodes += activity_after_deleting_task diff --git a/notify_attract.py b/notify_attract.py new file mode 100755 index 0000000..db05b31 --- /dev/null +++ b/notify_attract.py @@ -0,0 +1,63 @@ +#!/usr/bin/env python3 + +"""To be called as SVN post-commit hook. + +Stupidly simple, only pushes this commit, doesn't register which commits were +pushed and which weren't, doesn't retry anything later. + +Example call: + notify_attract.py "$REPOS" "$REV" +""" + +import json +import os.path +import subprocess +import sys + +try: + # Try Python 3 import first + from urllib import parse +except ImportError: + # If it fails, fall back to Python 2 + import urlparse as parse + +import requests + +# ################# CONFIGURE THIS FOR YOUR OWN PROJECT/ATTRACT ############################## +AUTH_TOKEN = 'SRVNZNxzvaDnnewyoq7IGiHufcrT4nsXiay2W8Jz3AxA8A' +PILLAR_URL = 'http://pillar-web:5001/' +PROJECT_URLS = { # Mapping from SVN repository name to Attract project URL. + 'repo': 'sybren', +} +# ################# END OF CONFIGURATION ############################## + +svn_repo = sys.argv[1] +svn_revision = int(sys.argv[2]) + +repo_basename = os.path.basename(svn_repo) +try: + project_url = PROJECT_URLS[repo_basename] +except KeyError: + raise SystemExit('Not configured for repository %r' % repo_basename) + +url = parse.urljoin(PILLAR_URL, '/attract/api/%s/subversion/log' % project_url) + + +def svnlook(subcmd): + info = subprocess.check_output(['/usr/bin/svnlook', subcmd, svn_repo, '-r', str(svn_revision)]) + return info.decode('utf8').strip() + + +data = { + 'repo': svn_repo, + 'revision': svn_revision, + 'msg': svnlook('log'), + 'author': svnlook('author'), + 'date': svnlook('date').split(' (', 1)[0], +} + +print('POSTing to %s' % url) +print('DATA:') +print(json.dumps(data, indent=4)) +resp = requests.post(url, json=data, auth=(AUTH_TOKEN, '')) +sys.stderr.write(resp.text) diff --git a/tests/abstract_attract_test.py b/tests/abstract_attract_test.py index d45c4e4..baf1f4a 100644 --- a/tests/abstract_attract_test.py +++ b/tests/abstract_attract_test.py @@ -15,7 +15,11 @@ class AbstractAttractTest(AbstractPillarTest): pillar_server_class = AttractTestServer def tearDown(self): + from attract import subversion + + subversion.task_logged._clear_state() self.unload_modules('attract') + AbstractPillarTest.tearDown(self) def ensure_project_exists(self, project_overrides=None): diff --git a/tests/logging_config.py b/tests/logging_config.py index 04098e6..b651882 100644 --- a/tests/logging_config.py +++ b/tests/logging_config.py @@ -1,7 +1,7 @@ LOGGING = { 'version': 1, 'formatters': { - 'default': {'format': '%(asctime)-15s %(levelname)8s %(name)s %(message)s'} + 'default': {'format': '%(asctime)-15s %(levelname)8s %(name)36s %(message)s'} }, 'handlers': { 'console': { @@ -12,7 +12,7 @@ LOGGING = { }, 'loggers': { 'pillar': {'level': 'DEBUG'}, - 'attract_server': {'level': 'DEBUG'}, + 'attract': {'level': 'DEBUG'}, 'werkzeug': {'level': 'INFO'}, 'eve': {'level': 'WARNING'}, # 'requests': {'level': 'DEBUG'}, diff --git a/tests/test_shortcode.py b/tests/test_shortcode.py new file mode 100644 index 0000000..b1fc848 --- /dev/null +++ b/tests/test_shortcode.py @@ -0,0 +1,41 @@ +# -*- coding=utf-8 -*- + + +from __future__ import absolute_import + +import collections +import datetime +import logging.config +import unittest + +from dateutil.tz import tzutc +import mock +import svn.common + +from pillar.tests import common_test_data as ctd + +import logging_config +from attract import subversion +from abstract_attract_test import AbstractAttractTest + +SVN_SERVER_URL = 'svn://biserver/agent327' + + +class ShortcodeTest(AbstractAttractTest): + def setUp(self, **kwargs): + AbstractAttractTest.setUp(self, **kwargs) + + self.mngr = self.app.pillar_extensions['attract'].task_manager + self.proj_id, self.project = self.ensure_project_exists() + + def test_increment_simple(self): + + from attract import shortcodes + + with self.app.test_request_context(): + code = shortcodes.generate_shortcode(self.proj_id, u'jemoeder', u'ø') + self.assertEqual(u'ø1', code) + + with self.app.test_request_context(): + code = shortcodes.generate_shortcode(self.proj_id, u'jemoeder', u'č') + self.assertEqual(u'č2', code) diff --git a/tests/test_subversion.py b/tests/test_subversion.py index 7bc25a8..1d2de0b 100644 --- a/tests/test_subversion.py +++ b/tests/test_subversion.py @@ -13,10 +13,7 @@ from dateutil.tz import tzutc import mock import svn.common -from pillar.tests import common_test_data as ctd - import logging_config -from attract import subversion from abstract_attract_test import AbstractAttractTest SVN_SERVER_URL = 'svn://biserver/agent327' @@ -66,6 +63,8 @@ SVN_LOG_BATCH_WITH_TASK_MARKERS = [ class TestCommitLogObserver(unittest.TestCase): def setUp(self): + from attract import subversion + self.client = subversion.obtain(SVN_SERVER_URL) # Passing in a real client to Mock() will ensure that isinstance() checks return True. self.mock_client = mock.Mock(self.client, name='svn_client') @@ -77,6 +76,8 @@ class TestCommitLogObserver(unittest.TestCase): Keep the underscore in the name when committing, and don't call it from anywhere. Unit tests shouldn't be dependent on network connections. """ + from attract import subversion + observer = subversion.CommitLogObserver(self.client) observer.fetch_and_observe() @@ -110,6 +111,8 @@ class TestCommitLogObserver(unittest.TestCase): self.assertEqual(self.observer.last_seen_revision, 51) def test_task_markers(self): + from attract import subversion + self.mock_client.log_default = mock.Mock(name='log_default', return_value=SVN_LOG_BATCH_WITH_TASK_MARKERS) blinks = [] @@ -123,15 +126,16 @@ class TestCommitLogObserver(unittest.TestCase): self.observer.fetch_and_observe() self.assertEqual(3, len(blinks)) - self.assertEqual({'log_entry': SVN_LOG_BATCH_WITH_TASK_MARKERS[1], 'shortcode': '1234'}, + self.assertEqual({'log_entry': SVN_LOG_BATCH_WITH_TASK_MARKERS[1], 'shortcode': 'T1234'}, blinks[0]) - self.assertEqual({'log_entry': SVN_LOG_BATCH_WITH_TASK_MARKERS[2], 'shortcode': '4415'}, + self.assertEqual({'log_entry': SVN_LOG_BATCH_WITH_TASK_MARKERS[2], 'shortcode': 'T4415'}, blinks[1]) - self.assertEqual({'log_entry': SVN_LOG_BATCH_WITH_TASK_MARKERS[2], 'shortcode': '4433'}, + self.assertEqual({'log_entry': SVN_LOG_BATCH_WITH_TASK_MARKERS[2], 'shortcode': 'T4433'}, blinks[2]) def test_svn_error(self): """SVN errors should not crash the observer.""" + from attract import subversion self.mock_client.log_default = mock.Mock(name='log_default', side_effect=svn.common.SvnException('unittest')) @@ -146,6 +150,8 @@ class TestCommitLogObserver(unittest.TestCase): self.mock_client.log_default.assert_called_once() def test_create_log_entry(self): + from attract import subversion + entry = subversion.create_log_entry(date_text=u'2016-10-21 17:40:17 +0200', msg=u'Ünicøde is good', revision='123', @@ -200,7 +206,7 @@ class PushCommitTest(AbstractAttractTest): self.proj_id, self.project = self.ensure_project_exists() def test_push_happy(self): - from attract import cli + from attract import cli, subversion with self.app.test_request_context(): _, token = cli.create_svner_account('svner@example.com', self.project['url']) @@ -225,6 +231,7 @@ class PushCommitTest(AbstractAttractTest): auth_token=token['token']) self.assertEqual(1, len(blinks)) + self.assertEqual(u'T431134', blinks[0]['shortcode']) self.assertEqual(u'မြန်မာဘာသာ is beautiful.\n\nThis solves task [T431134]', blinks[0]['log_entry'].msg) self.assertEqual(datetime.datetime(2016, 10, 21, 15, 40, 17, 0, tzinfo=tzutc()),