archive/attract
0
0
Fork 0
Code Issues 7 Pull Requests Projects Releases Wiki Activity

Fix issue with task shortcodes

Browse Source 
Part of the code assumed shortcodes were globally unique, and another part
assumed the shortcodes are unique per project (the latter is correct).

Now the project ID is taken from the URL the Subversion hook pushes to.
This commit is contained in:
Sybren A. Stüvel
2018-04-19 18:00:03 +02:00
parent b3e21d4b02
commit 1be31bdb22
6 changed files with 200 additions and 80 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
attract/subversion/__init__.py
View File

@@ -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:

5
attract/subversion/routes.py
View File

@@ -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)

13
attract/tasks/__init__.py
View File

@@ -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'])

3
docs/docs/user_manual/subversion.md
View File

@@ -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.

32
tests/test_shortcode.py
View File

@@ -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)

221
tests/test_subversion.py
View File

@@ -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):