Moved Subversion stuff to its own module, and unified push & pull approaches

attract/__init__.py

@@ -64,12 +64,15 @@ class AttractExtension(PillarExtension):
         from . import routes
         import attract.tasks.routes
         import attract.shots.routes
+        import attract.subversion.routes
 
         return [
             routes.blueprint,
             attract.tasks.routes.blueprint,
             attract.tasks.routes.perproject_blueprint,
             attract.shots.routes.perproject_blueprint,
+            attract.subversion.routes.blueprint,
+            attract.subversion.routes.api_blueprint,
         ]
 
     @property

attract/routes.py

@@ -1,18 +1,15 @@
 import functools
 import logging
 
-from flask import Blueprint, render_template, url_for, request, current_app
+from flask import Blueprint, render_template, url_for
 import flask_login
 
 from pillar.web.utils import attach_project_pictures
-from pillar.api.utils import jsonify
-from pillar.api.utils import authorization, authentication
 import pillar.web.subquery
 from pillar.web.system_util import pillar_api
 import pillarsdk
-import werkzeug.exceptions as wz_exceptions
 
-from attract import current_attract, EXTENSION_NAME
+from attract import current_attract
 from attract.node_types.task import node_type_task
 from attract.node_types.shot import node_type_shot
 
@@ -150,82 +147,6 @@ def attract_project_view(extra_project_projections=None, extension_props=False):
     return decorator
 
 
-@blueprint.route('/<project_url>/subversion/kick')
-@attract_project_view(extension_props=True)
-def subversion_kick(project, attract_props):
-    from . import subversion
-
-    svn_server_url = attract_props.svn_url  # 'svn://localhost/agent327'
-    log.info('Re-examining SVN server %s', svn_server_url)
-    client = subversion.obtain(svn_server_url)
-
-    # TODO: last_seen_revision should be stored, probably at the project level.
-    last_seen_revision = 0
-    observer = subversion.CommitLogObserver(client, last_seen_revision=last_seen_revision)
-    observer.fetch_and_observe()
-
-    return jsonify({
-        'previous_last_seen_revision': last_seen_revision,
-        'last_seen_revision': observer.last_seen_revision,
-    })
-
-
-@blueprint.route('/api/<project_url>/subversion/log', methods=['POST'])
-@authorization.require_login(require_roles={u'service', u'svner'}, require_all=True)
-def subversion_log(project_url):
-    if request.mimetype != 'application/json':
-        log.debug('Received %s instead of application/json', request.mimetype)
-        raise wz_exceptions.BadRequest()
-
-    # Parse the request
-    args = request.json
-    revision = args['revision']
-    commit_message = args['log']
-    commit_author = args['author']
-
-    current_user_id = authentication.current_user_id()
-    log.info('Service account %s registers SVN commit %s of user %s',
-             current_user_id, revision, commit_author)
-    assert current_user_id
-
-    users_coll = current_app.db()['users']
-    projects_coll = current_app.db()['projects']
-    project = projects_coll.find_one({'url': project_url},
-                                     projection={'_id': 1, 'url': 1,
-                                                 'extension_props': 1})
-    if not project:
-        return 'Project not found', 403
-
-    # Check that the service user is allowed to log on this project.
-    srv_user = users_coll.find_one(current_user_id,
-                                   projection={'service.svner': 1})
-    if srv_user is None:
-        log.error('subversion_log(%s): current user %s not found -- how did they log in?',
-                  project['url'], current_user_id)
-        return 'User not found', 403
-
-    allowed_project = srv_user.get('service', {}).get('svner', {}).get('project')
-    if allowed_project != project['_id']:
-        log.warning('subversion_log(%s): current user %s not authorized to project %s',
-                    project['url'], current_user_id, project['_id'])
-        return 'Project not allowed', 403
-
-    from . import subversion
-
-    try:
-        attract_props = project['extension_props'][EXTENSION_NAME]
-    except KeyError:
-        return 'Not set up for Attract', 400
-
-    svn_server_url = attract_props['svn_url']
-    log.info('Re-examining SVN server %s', svn_server_url)
-    client = subversion.obtain(svn_server_url)
-    observer = subversion.CommitLogObserver(client)
-    observer.process_log(revision, commit_author, commit_message)
-
-    return 'Registered in Attract'
-
-
 @blueprint.route('/<project_url>')
 @attract_project_view(extension_props=True)
 def project_index(project, attract_props):

attract/subversion/__init__.py

@@ -2,6 +2,8 @@
 
 from __future__ import absolute_import
 
+import collections
+import dateutil.parser
 import re
 
 import attr
@@ -12,13 +14,32 @@ from pillar import attrs_extra
 
 task_logged = blinker.NamedSignal('task_logged')
 shot_logged = blinker.NamedSignal('shot_logged')
-marker_re = re.compile(r'\[(?P<type>[TS])(?P<node_id>[0-9a-zA-Z]+)\]')
+marker_re = re.compile(r'\[(?P<type>[TS])(?P<shortcode>[0-9a-zA-Z]+)\]')
 
 signals = {
     'T': task_logged,
     'S': shot_logged,
 }
 
+# Copy of namedtuple defined in svn.common.log_default().
+LogEntry = collections.namedtuple(
+    'LogEntry',
+    ['date', 'msg', 'revision', 'author', 'changelist']
+)
+
+
+def create_log_entry(**namedfields):
+    date = namedfields.pop('date', None)
+    date_text = namedfields.pop('date_text', None)
+    if bool(date) == bool(date_text):
+        raise ValueError('Either date or date_text must be given.')
+
+    if date_text is not None:
+        date = dateutil.parser.parse(date_text)
+    changelist = namedfields.pop('changelist', None)
+
+    return LogEntry(date=date, changelist=changelist, **namedfields)
+
 
 def obtain(server_location):
     """Returns a Connection object for the given server location."""
@@ -28,7 +49,8 @@ def obtain(server_location):
 
 @attr.s
 class CommitLogObserver(object):
-    svn_client = attr.ib(validator=attr.validators.instance_of(svn.remote.RemoteClient))
+    svn_client = attr.ib(default=None,
+                         validator=attr.validators.instance_of(svn.remote.RemoteClient))
     last_seen_revision = attr.ib(default=0, validator=attr.validators.instance_of(int))
     _log = attrs_extra.log('%s.CommitLogObserver' % __name__)
 
@@ -45,7 +67,12 @@ class CommitLogObserver(object):
                 # assumption: revisions are always logged in strictly increasing order.
                 self.last_seen_revision = log_entry.revision
 
-                self._parse_log_entry(log_entry)
+                # Log entries can be None.
+                if not log_entry.msg:
+                    continue
+
+                self.process_log(log_entry)
+
         except svn.common.SvnException:
             # The SVN library just raises a SvnCommandError when something goes wrong,
             # without any structured indication of the error. There isn't much else
@@ -53,13 +80,17 @@ class CommitLogObserver(object):
             self._log.exception('Error calling self.svn_client.log_default()')
             return
 
-    def process_log(self, revision, commit_author, commit_message):
+    def process_log(self, log_entry):
-        """Obtains task IDs without accessing the SVN server directly."""
+        """Obtains task IDs without accessing the SVN server directly.
 
-        self._log.debug('%s: process_log(%s, %s, ...)', self, revision, commit_author)
+        :type log_entry: LogEntry
-        for node_id, node_type, in self._find_ids(commit_message):
+        """
+
+        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, node_id=node_id, log_entry=commit_message, author=commit_author)
+            signal.send(self, shortcode=shortcode, log_entry=log_entry)
 
     def _find_ids(self, message):
         # Parse the commit log to see if there are any task/shot markers.
@@ -67,21 +98,5 @@ class CommitLogObserver(object):
         for line in lines:
             for match in marker_re.finditer(line):
                 type = match.group('type')
-                node_id = match.group('task_id')
+                shortcode = match.group('shortcode')
-                yield node_id, type
+                yield type, shortcode
-
-    def _parse_log_entry(self, log_entry):
-        """Parses the commit log to see if there are any task markers."""
-
-        # Log entries can be None.
-        if not log_entry.msg:
-            return
-
-        # Parse the commit log to see if there are any task markers.
-        lines = log_entry.msg.split('\n', 1)
-        first_line = lines[0]
-        for match in marker_re.finditer(first_line):
-            task_id = match.group('task_id')
-
-            # Send a Blinker signal for each observed task identifier.
-            task_logged.send(self, task_id=task_id, log_entry=log_entry)

attract/subversion/routes.py

@@ -0,0 +1,95 @@
+import logging
+
+from flask import Blueprint, render_template, url_for, request, current_app
+import werkzeug.exceptions as wz_exceptions
+
+from pillar.api.utils import jsonify
+from pillar.api.utils import authorization, authentication
+
+from attract import EXTENSION_NAME
+from attract.routes import attract_project_view
+
+blueprint = Blueprint('attract.subversion', __name__, url_prefix='/')
+api_blueprint = Blueprint('attract.api.subversion', __name__, url_prefix='/api')
+
+log = logging.getLogger(__name__)
+
+
+@blueprint.route('/<project_url>/subversion/kick')
+@attract_project_view(extension_props=True)
+def subversion_kick(project, attract_props):
+    from attract import subversion
+
+    svn_server_url = attract_props.svn_url  # 'svn://localhost/agent327'
+    log.info('Re-examining SVN server %s', svn_server_url)
+    client = subversion.obtain(svn_server_url)
+
+    # TODO: last_seen_revision should be stored, probably at the project level.
+    last_seen_revision = 0
+    observer = subversion.CommitLogObserver(client, last_seen_revision=last_seen_revision)
+    observer.fetch_and_observe()
+
+    return jsonify({
+        'previous_last_seen_revision': last_seen_revision,
+        'last_seen_revision': observer.last_seen_revision,
+    })
+
+
+@api_blueprint.route('/<project_url>/subversion/log', methods=['POST'])
+@authorization.require_login(require_roles={u'service', u'svner'}, require_all=True)
+def subversion_log(project_url):
+    if request.mimetype != 'application/json':
+        log.debug('Received %s instead of application/json', request.mimetype)
+        raise wz_exceptions.BadRequest()
+
+    # Parse the request
+    args = request.json
+    revision = args['revision']
+    commit_message = args['msg']
+    commit_author = args['author']
+    commit_date = args['date']
+
+    current_user_id = authentication.current_user_id()
+    log.info('Service account %s registers SVN commit %s of user %s',
+             current_user_id, revision, commit_author)
+    assert current_user_id
+
+    users_coll = current_app.db()['users']
+    projects_coll = current_app.db()['projects']
+    project = projects_coll.find_one({'url': project_url},
+                                     projection={'_id': 1, 'url': 1,
+                                                 'extension_props': 1})
+    if not project:
+        return 'Project not found', 403
+
+    # Check that the service user is allowed to log on this project.
+    srv_user = users_coll.find_one(current_user_id,
+                                   projection={'service.svner': 1})
+    if srv_user is None:
+        log.error('subversion_log(%s): current user %s not found -- how did they log in?',
+                  project['url'], current_user_id)
+        return 'User not found', 403
+
+    allowed_project = srv_user.get('service', {}).get('svner', {}).get('project')
+    if allowed_project != project['_id']:
+        log.warning('subversion_log(%s): current user %s not authorized to project %s',
+                    project['url'], current_user_id, project['_id'])
+        return 'Project not allowed', 403
+
+    from attract import subversion
+
+    try:
+        attract_props = project['extension_props'][EXTENSION_NAME]
+    except KeyError:
+        return 'Not set up for Attract', 400
+
+    svn_server_url = attract_props['svn_url']
+    log.info('Re-examining SVN server %s', svn_server_url)
+    log_entry = subversion.create_log_entry(revision=revision,
+                                            msg=commit_message,
+                                            author=commit_author,
+                                            date_text=commit_date)
+    observer = subversion.CommitLogObserver()
+    observer.process_log(log_entry)
+
+    return 'Registered in Attract'

attract/tasks/__init__.py

@@ -14,16 +14,16 @@ 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, task_id, log_entry, author):
+    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
-        :param task_info: {'task_id': '123', 'log_entry': LogEntry} dict.
+        :type log_entry: attract.subversion.LogEntry
-        :type task_info: dict
         """
 
-        self._log.info("Task '%s' logged in SVN by %s: %s", task_id, author, log_entry)
+        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.

requirements.txt

@@ -4,6 +4,7 @@
 
 attrs==16.2.0
 svn==0.3.43
+python-dateutil==2.5.3
 
 # Testing requirements:
 pytest==3.0.1

tests/test_subversion.py

@@ -1,3 +1,5 @@
+# -*- coding=utf-8 -*-
+
 """Unit test for SVN interface."""
 
 from __future__ import absolute_import
@@ -97,6 +99,7 @@ class TestCommitLogObserver(unittest.TestCase):
 
         self.observer.fetch_and_observe()
         self.mock_client.log_default.assert_called_with(revision_from=43)
+        self.assertEqual(self.observer.last_seen_revision, 46)
 
         self.observer.fetch_and_observe()
         self.mock_client.log_default.assert_called_with(revision_from=47)
@@ -117,11 +120,11 @@ 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], 'task_id': 'T1234'},
+        self.assertEqual({'log_entry': SVN_LOG_BATCH_WITH_TASK_MARKERS[1], 'shortcode': '1234'},
                          blinks[0])
-        self.assertEqual({'log_entry': SVN_LOG_BATCH_WITH_TASK_MARKERS[2], 'task_id': 'T4415'},
+        self.assertEqual({'log_entry': SVN_LOG_BATCH_WITH_TASK_MARKERS[2], 'shortcode': '4415'},
                          blinks[1])
-        self.assertEqual({'log_entry': SVN_LOG_BATCH_WITH_TASK_MARKERS[2], 'task_id': 'T4433'},
+        self.assertEqual({'log_entry': SVN_LOG_BATCH_WITH_TASK_MARKERS[2], 'shortcode': '4433'},
                          blinks[2])
 
     def test_svn_error(self):
@@ -138,3 +141,49 @@ class TestCommitLogObserver(unittest.TestCase):
 
         record_blink.assert_not_called()
         self.mock_client.log_default.assert_called_once()
+
+    def test_create_log_entry(self):
+        entry = subversion.create_log_entry(date_text=u'2016-10-21 17:40:17 +0200',
+                                            msg=u'Ünicøde is good',
+                                            revision='123',
+                                            author=u'børk',
+                                            changelist='nothing')
+        self.assertEqual(tuple(entry), (
+            datetime.datetime(2016, 10, 21, 15, 40, 17, 0, tzinfo=tzutc()),
+            u'Ünicøde is good',
+            '123',
+            u'børk',
+            'nothing'
+        ))
+
+        self.assertRaises(ValueError, subversion.create_log_entry,
+                          date_text='Unparseable date',
+                          msg=u'Ünicøde is good',
+                          revision='123',
+                          author=u'børk',
+                          changelist='nothing')
+
+        entry = subversion.create_log_entry(date_text=u'2016-10-21 17:40:17 +0200',
+                                            msg=u'Ünicøde is good',
+                                            revision='123',
+                                            author=u'børk')
+        self.assertEqual(tuple(entry), (
+            datetime.datetime(2016, 10, 21, 15, 40, 17, 0, tzinfo=tzutc()),
+            u'Ünicøde is good',
+            '123',
+            u'børk',
+            None
+        ))
+
+        entry = subversion.create_log_entry(
+            date=datetime.datetime(2016, 10, 21, 15, 40, 17, 0, tzinfo=tzutc()),
+            msg=u'Ünicøde is good',
+            revision='123',
+            author=u'børk')
+        self.assertEqual(tuple(entry), (
+            datetime.datetime(2016, 10, 21, 15, 40, 17, 0, tzinfo=tzutc()),
+            u'Ünicøde is good',
+            '123',
+            u'børk',
+            None
+        ))