diff --git a/attract/__init__.py b/attract/__init__.py index 01f5f1e..48adce6 100644 --- a/attract/__init__.py +++ b/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 diff --git a/attract/routes.py b/attract/routes.py index 59c71f5..b7d6c4c 100644 --- a/attract/routes.py +++ b/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('//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//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('/') @attract_project_view(extension_props=True) def project_index(project, attract_props): diff --git a/attract/subversion.py b/attract/subversion/__init__.py similarity index 57% rename from attract/subversion.py rename to attract/subversion/__init__.py index 4f4dc3c..5fe323f 100644 --- a/attract/subversion.py +++ b/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[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(). +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): - """Obtains task IDs without accessing the SVN server directly.""" + def process_log(self, log_entry): + """Obtains task IDs without accessing the SVN server directly. - self._log.debug('%s: process_log(%s, %s, ...)', self, revision, commit_author) - for node_id, node_type, in self._find_ids(commit_message): + :type log_entry: LogEntry + """ + + 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') - yield node_id, type - - 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) + shortcode = match.group('shortcode') + yield type, shortcode diff --git a/attract/subversion/routes.py b/attract/subversion/routes.py new file mode 100644 index 0000000..22a1837 --- /dev/null +++ b/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('//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('//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' diff --git a/attract/tasks/__init__.py b/attract/tasks/__init__.py index cc7b219..a4887a0 100644 --- a/attract/tasks/__init__.py +++ b/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 task_info: dict + :type log_entry: attract.subversion.LogEntry """ - 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. diff --git a/requirements.txt b/requirements.txt index b580215..645f0bf 100644 --- a/requirements.txt +++ b/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 diff --git a/tests/test_subversion.py b/tests/test_subversion.py index b36d1e2..ea35c19 100644 --- a/tests/test_subversion.py +++ b/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 + ))