diff --git a/attract/subversion/__init__.py b/attract/subversion/__init__.py index cd77faa..f60fd46 100644 --- a/attract/subversion/__init__.py +++ b/attract/subversion/__init__.py @@ -17,10 +17,11 @@ signals = { 'T': task_logged, } -# Copy of namedtuple defined in svn.common.log_default(). +# Copy of namedtuple defined in svn.common.log_default(), +# extended with our project_id. LogEntry = collections.namedtuple( 'LogEntry', - ['date', 'msg', 'revision', 'author', 'changelist'] + ['date', 'msg', 'revision', 'author', 'changelist', 'project_id'] ) @@ -54,6 +55,7 @@ class CommitLogObserver(object): def fetch_and_observe(self): """Obtains task IDs from SVN logs.""" + # FIXME: this code is unaware of the fact that task markers are only unique per project. self._log.debug('%s: fetch_and_observe()', self) try: diff --git a/attract/subversion/routes.py b/attract/subversion/routes.py index 2529f92..36d3dde 100644 --- a/attract/subversion/routes.py +++ b/attract/subversion/routes.py @@ -87,12 +87,13 @@ def subversion_log(project_url): except KeyError: return 'Not set up for Attract', 400 - svn_server_url = attract_props['svn_url'] + svn_server_url = attract_props.get('svn_url', '-unknown-') log.debug('Receiving commit from SVN server %s', svn_server_url) log_entry = subversion.create_log_entry(revision=revision, msg=commit_message, author=commit_author, - date_text=commit_date) + date_text=commit_date, + project_id=project['_id']) observer = subversion.CommitLogObserver() log.debug('Processing %s via %s', log_entry, observer) observer.process_log(log_entry) diff --git a/attract/tasks/__init__.py b/attract/tasks/__init__.py index d453484..6610a55 100644 --- a/attract/tasks/__init__.py +++ b/attract/tasks/__init__.py @@ -4,6 +4,7 @@ import attr import flask import flask_login from dateutil import parser +import bson import pillarsdk from pillar import attrs_extra @@ -128,7 +129,7 @@ class TaskManager(object): }}, api=api) return tasks - def api_task_for_shortcode(self, shortcode): + def api_task_for_shortcode(self, project_id: bson.ObjectId, shortcode: str) -> dict: """Returns the task for the given shortcode. :returns: the task Node, or None if not found. @@ -137,6 +138,7 @@ class TaskManager(object): db = flask.current_app.db() task = db['nodes'].find_one({ 'properties.shortcode': shortcode, + 'project': project_id, 'node_type': node_type_task['name'], }) @@ -150,11 +152,12 @@ class TaskManager(object): :type log_entry: attract.subversion.LogEntry """ - self._log.info("Task '%s' logged in SVN by %s: %s...", - shortcode, log_entry.author, log_entry.msg[:30].replace('\n', ' // ')) + self._log.info("Project %s, task '%s' logged in SVN by %s: %s...", + log_entry.project_id, shortcode, + log_entry.author, log_entry.msg[:30].replace('\n', ' // ')) # Find the task - task = self.api_task_for_shortcode(shortcode) + task = self.api_task_for_shortcode(log_entry.project_id, shortcode) if not task: self._log.warning('Task %s not found, ignoring SVN commit.', shortcode) return @@ -162,7 +165,7 @@ class TaskManager(object): # Find the author db = flask.current_app.db() proj = db['projects'].find_one({'_id': task['project']}, - projection={'extension_props.attract': 1}) + projection={'extension_props.attract': 1}) if not proj: self._log.warning('Project %s for task %s not found, ignoring SVN commit.', task['project'], task['_id']) diff --git a/docs/docs/user_manual/subversion.md b/docs/docs/user_manual/subversion.md index bbb3b26..222ccaa 100644 --- a/docs/docs/user_manual/subversion.md +++ b/docs/docs/user_manual/subversion.md @@ -23,3 +23,6 @@ file is bundled with Attract's source code, and needs to be copied to the Subver } The authentication token must be created on the server using `manage.py attract create_svner_account {emailaddress} {project-url}` + +The project should have an `svn_url` extension property in the MongoDB database, which points to +the Subversion server URL. This is for logging use only. diff --git a/tests/test_shortcode.py b/tests/test_shortcode.py index ca3d754..257af54 100644 --- a/tests/test_shortcode.py +++ b/tests/test_shortcode.py @@ -1,18 +1,5 @@ -# -*- coding=utf-8 -*- +import bson -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' @@ -26,7 +13,6 @@ class ShortcodeTest(AbstractAttractTest): self.proj_id, self.project = self.ensure_project_exists() def test_increment_simple(self): - from attract import shortcodes with self.app.test_request_context(): @@ -36,3 +22,19 @@ class ShortcodeTest(AbstractAttractTest): with self.app.test_request_context(): code = shortcodes.generate_shortcode(self.proj_id, 'jemoeder', 'č') self.assertEqual('č2', code) + + def test_multiple_projects(self): + from attract import shortcodes + + proj_id2, project2 = self.ensure_project_exists(project_overrides={ + '_id': bson.ObjectId(24 * 'f'), + 'url': 'proj2', + }) + + with self.app.app_context(): + code1 = shortcodes.generate_shortcode(self.proj_id, 'jemoeder', 'č') + code2 = shortcodes.generate_shortcode(proj_id2, 'jemoeder', 'č') + code3 = shortcodes.generate_shortcode(proj_id2, 'jemoeder', 'č') + self.assertEqual('č1', code1) + self.assertEqual('č1', code2) + self.assertEqual('č2', code3) diff --git a/tests/test_subversion.py b/tests/test_subversion.py index c51fead..48661b7 100644 --- a/tests/test_subversion.py +++ b/tests/test_subversion.py @@ -22,51 +22,63 @@ SVN_SERVER_URL = 'svn://biserver/agent327' logging.config.dictConfig(logging_config.LOGGING) -# Unfortunately, the svn module doesn't use classes, but uses in-function-defined -# namedtuples instead. As a result, we can't import them, but have to recreate. -LogEntry = collections.namedtuple('LogEntry', ['date', 'msg', 'revision', 'author', 'changelist']) - -SVN_LOG_BATCH_1 = [ - LogEntry(date=datetime.datetime(2016, 4, 5, 10, 8, 5, 19211, tzinfo=tzutc()), - msg='Initial commit', revision=43, author='fsiddi', changelist=None), - LogEntry(date=datetime.datetime(2016, 4, 8, 13, 5, 39, 42537, tzinfo=tzutc()), - msg='Initial commit of layout files', revision=44, author='hjalti', changelist=None), - LogEntry(date=datetime.datetime(2016, 4, 8, 13, 6, 18, 947830, tzinfo=tzutc()), - msg=None, revision=45, author='andy', changelist=None), - LogEntry(date=datetime.datetime(2016, 4, 8, 14, 22, 24, 411916, tzinfo=tzutc()), - msg="Add the eye lattices to the main group\n\nOtherwise when you link the agent group, those two lattices would be\nlinked as regular objects, and you'd need to move both proxy+lattices\nindividually.\n\n\n", - revision=46, author='pablo', changelist=None), -] - -SVN_LOG_BATCH_2 = [ - LogEntry(date=datetime.datetime(2016, 4, 13, 17, 54, 50, 244305, tzinfo=tzutc()), - msg='first initial agent model rework.', revision=47, author='andy', changelist=None), - LogEntry(date=datetime.datetime(2016, 4, 14, 15, 57, 30, 951714, tzinfo=tzutc()), - msg='third day of puching verts around', revision=48, author='andy', changelist=None), - LogEntry(date=datetime.datetime(2016, 4, 21, 8, 21, 19, 390478, tzinfo=tzutc()), - msg='last weeks edit. a couple of facial expression tests.\nstarting to modify the agent head heavily... W A R N I N G', - revision=49, author='andy', changelist=None), - LogEntry(date=datetime.datetime(2016, 4, 25, 9, 18, 17, 23841, tzinfo=tzutc()), - msg='some expression tests.', revision=50, author='andy', changelist=None), - LogEntry(date=datetime.datetime(2016, 4, 25, 10, 12, 23, 233796, tzinfo=tzutc()), - msg='older version of the layout', revision=51, author='hjalti', changelist=None), -] - -SVN_LOG_BATCH_WITH_TASK_MARKERS = [ - LogEntry(date=datetime.datetime(2016, 4, 5, 10, 8, 5, 19211, tzinfo=tzutc()), - msg='Initial commit', revision=1, author='fsiddi', changelist=None), - LogEntry(date=datetime.datetime(2016, 4, 8, 13, 5, 39, 42537, tzinfo=tzutc()), - msg='[T1234] modeled Hendrik IJzerbroot', revision=2, author='andy', changelist=None), - LogEntry(date=datetime.datetime(2016, 4, 8, 13, 6, 18, 947830, tzinfo=tzutc()), - msg='[T4415] scene layout, which also closes [T4433]', revision=3, author='hjalti', - changelist=None), -] - class TestCommitLogObserver(unittest.TestCase): def setUp(self): from attract import subversion + self.pid1 = ObjectId(24 * 'a') + self.pid2 = ObjectId(24 * 'b') + + LogEntry = subversion.LogEntry + self.SVN_LOG_BATCH_1 = [ + LogEntry(date=datetime.datetime(2016, 4, 5, 10, 8, 5, 19211, tzinfo=tzutc()), + msg='Initial commit', revision=43, author='fsiddi', changelist=None, + project_id=self.pid1), + LogEntry(date=datetime.datetime(2016, 4, 8, 13, 5, 39, 42537, tzinfo=tzutc()), + msg='Initial commit of layout files', revision=44, author='hjalti', + changelist=None, project_id=self.pid1), + LogEntry(date=datetime.datetime(2016, 4, 8, 13, 6, 18, 947830, tzinfo=tzutc()), + msg=None, revision=45, author='andy', changelist=None, project_id=self.pid1), + LogEntry(date=datetime.datetime(2016, 4, 8, 14, 22, 24, 411916, tzinfo=tzutc()), + msg="Add the eye lattices to the main group\n\nOtherwise when you link the " + "agent group, those two lattices would be\nlinked as regular objects, and " + "you'd need to move both proxy+lattices\nindividually.\n\n\n", + revision=46, author='pablo', changelist=None, project_id=self.pid1), + ] + + self.SVN_LOG_BATCH_2 = [ + LogEntry(date=datetime.datetime(2016, 4, 13, 17, 54, 50, 244305, tzinfo=tzutc()), + msg='first initial agent model rework.', revision=47, author='andy', + changelist=None, project_id=self.pid1), + LogEntry(date=datetime.datetime(2016, 4, 14, 15, 57, 30, 951714, tzinfo=tzutc()), + msg='third day of puching verts around', revision=48, author='andy', + changelist=None, project_id=self.pid1), + LogEntry(date=datetime.datetime(2016, 4, 21, 8, 21, 19, 390478, tzinfo=tzutc()), + msg='last weeks edit. a couple of facial expression tests.\nstarting to ' + 'modify the agent head heavily... W A R N I N G', + revision=49, author='andy', changelist=None, project_id=self.pid1), + LogEntry(date=datetime.datetime(2016, 4, 25, 9, 18, 17, 23841, tzinfo=tzutc()), + msg='some expression tests.', revision=50, author='andy', changelist=None, + project_id=self.pid1), + LogEntry(date=datetime.datetime(2016, 4, 25, 10, 12, 23, 233796, tzinfo=tzutc()), + msg='older version of the layout', revision=51, author='hjalti', + changelist=None, project_id=self.pid1), + ] + + self.SVN_LOG_BATCH_WITH_TASK_MARKERS = [ + LogEntry(date=datetime.datetime(2016, 4, 5, 10, 8, 5, 19211, tzinfo=tzutc()), + msg='Initial commit', revision=1, author='fsiddi', changelist=None, + project_id=self.pid1), + LogEntry(date=datetime.datetime(2016, 4, 8, 13, 5, 39, 42537, tzinfo=tzutc()), + msg='[T1234] modeled Hendrik IJzerbroot', revision=2, author='andy', + changelist=None, project_id=self.pid1), + LogEntry(date=datetime.datetime(2016, 4, 8, 13, 6, 18, 947830, tzinfo=tzutc()), + msg='[T4415] scene layout, which also closes [T4433]', revision=3, + author='hjalti', + changelist=None, project_id=ObjectId(24 * 'b')), + ] + 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') @@ -96,9 +108,9 @@ class TestCommitLogObserver(unittest.TestCase): self.mock_client.log_default = mock.Mock(name='log_default') self.mock_client.log_default.side_effect = [ # First call, only four commits. - SVN_LOG_BATCH_1, + self.SVN_LOG_BATCH_1, # Second call, five commits. - SVN_LOG_BATCH_2 + self.SVN_LOG_BATCH_2 ] self.observer.last_seen_revision = 42 @@ -116,7 +128,7 @@ class TestCommitLogObserver(unittest.TestCase): from attract import subversion self.mock_client.log_default = mock.Mock(name='log_default', - return_value=SVN_LOG_BATCH_WITH_TASK_MARKERS) + return_value=self.SVN_LOG_BATCH_WITH_TASK_MARKERS) blinks = [] def record_blink(sender, **kwargs): @@ -128,12 +140,15 @@ 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': 'T1234'}, - blinks[0]) - 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': 'T4433'}, - blinks[2]) + self.assertEqual( + {'log_entry': self.SVN_LOG_BATCH_WITH_TASK_MARKERS[1], 'shortcode': 'T1234'}, + blinks[0]) + self.assertEqual( + {'log_entry': self.SVN_LOG_BATCH_WITH_TASK_MARKERS[2], 'shortcode': 'T4415'}, + blinks[1]) + self.assertEqual( + {'log_entry': self.SVN_LOG_BATCH_WITH_TASK_MARKERS[2], 'shortcode': 'T4433'}, + blinks[2]) def test_svn_error(self): """SVN errors should not crash the observer.""" @@ -158,13 +173,15 @@ class TestCommitLogObserver(unittest.TestCase): msg='Ünicøde is good', revision='123', author='børk', - changelist='nothing') + changelist='nothing', + project_id=self.pid1) self.assertEqual(tuple(entry), ( datetime.datetime(2016, 10, 21, 15, 40, 17, 0, tzinfo=tzutc()), 'Ünicøde is good', '123', 'børk', - 'nothing' + 'nothing', + self.pid1, )) self.assertRaises(ValueError, subversion.create_log_entry, @@ -172,31 +189,36 @@ class TestCommitLogObserver(unittest.TestCase): msg='Ünicøde is good', revision='123', author='børk', - changelist='nothing') + changelist='nothing', + project_id=self.pid1) entry = subversion.create_log_entry(date_text='2016-10-21 17:40:17 +0200', msg='Ünicøde is good', revision='123', - author='børk') + author='børk', + project_id=self.pid1) self.assertEqual(tuple(entry), ( datetime.datetime(2016, 10, 21, 15, 40, 17, 0, tzinfo=tzutc()), 'Ünicøde is good', '123', 'børk', - None + None, + self.pid1, )) entry = subversion.create_log_entry( date=datetime.datetime(2016, 10, 21, 15, 40, 17, 0, tzinfo=tzutc()), msg='Ünicøde is good', revision='123', - author='børk') + author='børk', + project_id=self.pid1) self.assertEqual(tuple(entry), ( datetime.datetime(2016, 10, 21, 15, 40, 17, 0, tzinfo=tzutc()), 'Ünicøde is good', '123', 'børk', - None + None, + self.pid1, )) @@ -238,6 +260,93 @@ class PushCommitTest(AbstractAttractTest): blinks[0]['log_entry'].msg) self.assertEqual(datetime.datetime(2016, 10, 21, 15, 40, 17, 0, tzinfo=tzutc()), blinks[0]['log_entry'].date) + self.assertEqual(self.proj_id, blinks[0]['log_entry'].project_id) + + def test_two_projects(self): + from attract import cli, subversion + + proj_id2, project2 = self.ensure_project_exists(project_overrides={ + '_id': ObjectId(24 * 'f'), + 'url': 'proj2', + }) + + with self.app.test_request_context(): + _, token1 = cli.create_svner_account('svner1@example.com', self.project['url']) + _, token2 = cli.create_svner_account('svner2@example.com', project2['url']) + + blinks = [] + + def record_blink(sender, **kwargs): + blinks.append(kwargs) + + subversion.task_logged.connect(record_blink) + + push_data = { + 'repo': 'strange-repo™', + 'revision': '4', + 'msg': 'မြန်မာဘာသာ is beautiful.\n\nThis solves task [T431134]', + 'author': 'Haha', + 'date': '2016-10-21 17:40:17 +0200', + } + self.post('/attract/api/%s/subversion/log' % self.project['url'], + json=push_data, + auth_token=token1['token']) + self.post('/attract/api/%s/subversion/log' % project2['url'], + json=push_data, + auth_token=token2['token']) + + self.assertEqual(2, len(blinks)) + + self.assertEqual('T431134', blinks[0]['shortcode']) + self.assertEqual('မြန်မာဘာသာ 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()), + blinks[0]['log_entry'].date) + self.assertEqual(self.proj_id, blinks[0]['log_entry'].project_id) + + self.assertEqual('T431134', blinks[1]['shortcode']) + self.assertEqual('မြန်မာဘာသာ is beautiful.\n\nThis solves task [T431134]', + blinks[1]['log_entry'].msg) + self.assertEqual(datetime.datetime(2016, 10, 21, 15, 40, 17, 0, tzinfo=tzutc()), + blinks[1]['log_entry'].date) + self.assertEqual(proj_id2, blinks[1]['log_entry'].project_id) + + def test_wrong_project(self): + from attract import cli, subversion + + proj_id2, project2 = self.ensure_project_exists(project_overrides={ + '_id': ObjectId(24 * 'f'), + 'url': 'proj2', + }) + + with self.app.test_request_context(): + _, token1 = cli.create_svner_account('svner1@example.com', self.project['url']) + _, token2 = cli.create_svner_account('svner2@example.com', project2['url']) + + blinks = [] + + def record_blink(sender, **kwargs): + blinks.append(kwargs) + + subversion.task_logged.connect(record_blink) + + push_data = { + 'repo': 'strange-repo™', + 'revision': '4', + 'msg': 'မြန်မာဘာသာ is beautiful.\n\nThis solves task [T431134]', + 'author': 'Haha', + 'date': '2016-10-21 17:40:17 +0200', + } + self.post('/attract/api/%s/subversion/log' % self.project['url'], + json=push_data, + auth_token=token2['token'], + expected_status=403) + self.post('/attract/api/%s/subversion/log' % project2['url'], + json=push_data, + auth_token=token1['token'], + expected_status=403) + + self.assertEqual(0, len(blinks)) class SvnTaskLoggedTest(AbstractAttractTest):