From: Stephen Finucane Date: Fri, 6 Jan 2017 15:51:05 +0000 (+0000) Subject: models: Add 'project' field to Series X-Git-Tag: v2.0.0-rc1~112 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e3cbe4939b6589631d962a51469ab5b550d58e92;p=thirdparty%2Fpatchwork.git models: Add 'project' field to Series This is helpful for filtering. We use RunPython because folks are likely to have few if any Series objects existing yet. In addition, we update the unique constraints for SeriesReference as it's now possible to handle messages with duplicate message IDs. The update is included in parser to ensure this applies immediately. Signed-off-by: Stephen Finucane Reviewed-by: Andy Doan --- diff --git a/patchwork/migrations/0016_series_project.py b/patchwork/migrations/0016_series_project.py new file mode 100644 index 00000000..ecd8984b --- /dev/null +++ b/patchwork/migrations/0016_series_project.py @@ -0,0 +1,48 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models +import django.db.models.deletion + + +def copy_series_field(apps, schema_editor): + """Populate the project field from child cover letter/patches.""" + # TODO(stephenfin): Perhaps we'd like to include an SQL variant of the + # below though I'd imagine it would be rather tricky + Series = apps.get_model('patchwork', 'Series') + + for series in Series.objects.all(): + if series.cover_letter: + series.project = series.cover_letter.project + series.save() + elif series.patches: + series.project = series.patches.first().project + series.save() + else: + # a series without patches or cover letters should not exist. + # Delete it. + series.delete() + +class Migration(migrations.Migration): + + dependencies = [ + ('patchwork', '0015_add_series_models'), + ] + + operations = [ + migrations.AddField( + model_name='series', + name='project', + field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='series', to='patchwork.Project'), + ), + migrations.RunPython(copy_series_field, migrations.RunPython.noop), + migrations.AlterField( + model_name='seriesreference', + name='msgid', + field=models.CharField(max_length=255), + ), + migrations.AlterUniqueTogether( + name='seriesreference', + unique_together=set([('series', 'msgid')]), + ), + ] diff --git a/patchwork/models.py b/patchwork/models.py index 4ae41bed..ea40fbf9 100644 --- a/patchwork/models.py +++ b/patchwork/models.py @@ -549,6 +549,9 @@ class Comment(EmailMixin, models.Model): class Series(models.Model): """An collection of patches.""" + # parent + project = models.ForeignKey(Project, related_name='series') + # content cover_letter = models.ForeignKey(CoverLetter, related_name='series', @@ -676,11 +679,14 @@ class SeriesReference(models.Model): """ series = models.ForeignKey(Series, related_name='references', related_query_name='reference') - msgid = models.CharField(max_length=255, unique=True) + msgid = models.CharField(max_length=255) def __str__(self): return self.msgid + class Meta: + unique_together = [('series', 'msgid')] + class Bundle(models.Model): owner = models.ForeignKey(User) diff --git a/patchwork/parser.py b/patchwork/parser.py index 16cc53c0..7e2067ef 100644 --- a/patchwork/parser.py +++ b/patchwork/parser.py @@ -171,7 +171,7 @@ def find_project_by_header(mail): return project -def find_series(mail): +def find_series(project, mail): """Find a patch's series. Traverse RFC822 headers, starting with most recent first, to find @@ -194,15 +194,17 @@ def find_series(mail): recent (the patch's closest ancestor) to least recent Args: + project (patchwork.Project): The project that the series + belongs to mail (email.message.Message): The mail to extract series from Returns: The matching ``Series`` instance, if any """ for ref in [mail.get('Message-Id')] + find_references(mail): - # try parsing by RFC5322 fields first try: - return SeriesReference.objects.get(msgid=ref).series + return SeriesReference.objects.get( + msgid=ref, series__project=project).series except SeriesReference.DoesNotExist: pass @@ -787,7 +789,7 @@ def parse_mail(mail, list_id=None): filenames = find_filenames(diff) delegate = find_delegate_by_filename(project, filenames) - series = find_series(mail) + series = find_series(project, mail) # We will create a new series if: # - we have a patch number (x of n), and # - either: @@ -798,7 +800,8 @@ def parse_mail(mail, list_id=None): (series.version != version) or (SeriesPatch.objects.filter(series=series, number=x).count() )): - series = Series(date=date, + series = Series(project=project, + date=date, submitter=author, version=version, total=n) @@ -817,7 +820,8 @@ def parse_mail(mail, list_id=None): # series.) That should not create a series ref # for this series, so check for the msg-id only, # not the msg-id/series pair. - SeriesReference.objects.get(msgid=ref) + SeriesReference.objects.get(msgid=ref, + series__project=project) except SeriesReference.DoesNotExist: SeriesReference.objects.create(series=series, msgid=ref) @@ -869,12 +873,14 @@ def parse_mail(mail, list_id=None): # could only point to a different series or unrelated # message try: - series = SeriesReference.objects.get(msgid=msgid).series + series = SeriesReference.objects.get( + msgid=msgid, series__project=project).series except SeriesReference.DoesNotExist: series = None if not series: - series = Series(date=date, + series = Series(project=project, + date=date, submitter=author, version=version, total=n) diff --git a/patchwork/tests/test_parser.py b/patchwork/tests/test_parser.py index e4a379d8..8d0eba01 100644 --- a/patchwork/tests/test_parser.py +++ b/patchwork/tests/test_parser.py @@ -44,6 +44,7 @@ from patchwork.parser import split_prefixes from patchwork.parser import subject_check from patchwork.tests import TEST_MAIL_DIR from patchwork.tests.utils import create_project +from patchwork.tests.utils import create_series from patchwork.tests.utils import create_series_reference from patchwork.tests.utils import create_state from patchwork.tests.utils import create_user @@ -347,8 +348,9 @@ class SeriesCorrelationTest(TestCase): def test_new_series(self): msgid = make_msgid() email = self._create_email(msgid) + project = create_project() - self.assertIsNone(find_series(email)) + self.assertIsNone(find_series(project, email)) def test_first_reply(self): msgid_a = make_msgid() @@ -358,29 +360,31 @@ class SeriesCorrelationTest(TestCase): # assume msgid_a was already handled ref = create_series_reference(msgid=msgid_a) - series = find_series(email) + series = find_series(ref.series.project, email) self.assertEqual(series, ref.series) def test_nested_series(self): """Handle a series sent in-reply-to an existing series.""" # create an old series with a "cover letter" msgids = [make_msgid()] - ref_v1 = create_series_reference(msgid=msgids[0]) + project = create_project() + series_v1 = create_series(project=project) + create_series_reference(msgid=msgids[0], series=series_v1) # ...and three patches for i in range(3): msgids.append(make_msgid()) - create_series_reference(msgid=msgids[-1], - series=ref_v1.series) + create_series_reference(msgid=msgids[-1], series=series_v1) # now create a new series with "cover letter" msgids.append(make_msgid()) - ref_v2 = create_series_reference(msgid=msgids[-1]) + series_v2 = create_series(project=project) + ref_v2 = create_series_reference(msgid=msgids[-1], series=series_v2) # ...and the "first patch" of this new series msgid = make_msgid() email = self._create_email(msgid, msgids) - series = find_series(email) + series = find_series(project, email) # this should link to the second series - not the first self.assertEqual(len(msgids), 4 + 1) # old series + new cover diff --git a/patchwork/tests/test_rest_api.py b/patchwork/tests/test_rest_api.py index 88b7163b..0d1c7024 100644 --- a/patchwork/tests/test_rest_api.py +++ b/patchwork/tests/test_rest_api.py @@ -529,12 +529,12 @@ class TestSeriesAPI(APITestCase): self.assertEqual(status.HTTP_200_OK, resp.status_code) self.assertSerialized(series, resp.data) - patch = create_patch() + patch = create_patch(project=series.project) series.add_patch(patch, 1) resp = self.client.get(self.api_url(series.id)) self.assertSerialized(series, resp.data) - cover_letter = create_cover() + cover_letter = create_cover(project=series.project) series.add_cover_letter(cover_letter) resp = self.client.get(self.api_url(series.id)) self.assertSerialized(series, resp.data) diff --git a/patchwork/tests/test_series.py b/patchwork/tests/test_series.py index b9a764cf..7377d911 100644 --- a/patchwork/tests/test_series.py +++ b/patchwork/tests/test_series.py @@ -33,10 +33,9 @@ TEST_SERIES_DIR = os.path.join(os.path.dirname(__file__), 'series') class _BaseTestCase(TestCase): def setUp(self): - self.project = utils.create_project() utils.create_state() - def _parse_mbox(self, name, counts): + def _parse_mbox(self, name, counts, project=None): """Parse an mbox file and return the results. :param name: Name of mbox file @@ -44,10 +43,11 @@ class _BaseTestCase(TestCase): letters, patches and replies parsed """ results = [[], [], []] + project = project or utils.create_project() mbox = mailbox.mbox(os.path.join(TEST_SERIES_DIR, name)) for msg in mbox: - obj = parser.parse_mail(msg, self.project.listid) + obj = parser.parse_mail(msg, project.listid) if type(obj) == models.CoverLetter: results[0].append(obj) elif type(obj) == models.Patch: @@ -144,6 +144,29 @@ class BaseSeriesTest(_BaseTestCase): self.assertSerialized(patches, [2]) self.assertSerialized(covers, [1]) + def test_duplicated(self): + """Series received on multiple mailing lists. + + Parse a series with a two patches sent to two mailing lists + at the same time. + + Input: + + - [PATCH 1/2] test: Add some lorem ipsum + - [PATCH 2/2] test: Convert to Markdown + - [PATCH 1/2] test: Add some lorem ipsum + - [PATCH 2/2] test: Convert to Markdown + """ + project_a = utils.create_project() + project_b = utils.create_project() + + _, patches_a, _ = self._parse_mbox( + 'base-no-cover-letter.mbox', [0, 2, 0], project=project_a) + _, patches_b, _ = self._parse_mbox( + 'base-no-cover-letter.mbox', [0, 2, 0], project=project_b) + + self.assertSerialized(patches_a + patches_b, [2, 2]) + class RevisedSeriesTest(_BaseTestCase): """Tests for a series plus a single revision. diff --git a/patchwork/tests/utils.py b/patchwork/tests/utils.py index d94e8891..0c6f7634 100644 --- a/patchwork/tests/utils.py +++ b/patchwork/tests/utils.py @@ -223,6 +223,7 @@ def create_check(**kwargs): def create_series(**kwargs): """Create 'Series' object.""" values = { + 'project': create_project() if 'project' not in kwargs else None, 'date': dt.now(), 'submitter': create_person() if 'submitter' not in kwargs else None, 'total': 1,