[PATCH 1/2] models: Add 'project' field to Series

Stephen Finucane stephen at that.guru
Sat Jan 7 09:52:58 AEDT 2017


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 constaints 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 <stephen at that.guru>
Cc: Daniel Axtens <dja at axtens.net>
Cc: Andrew Donnellan <andrew.donnellan at au1.ibm.com>
---
 patchwork/migrations/0016_series_project.py | 48 +++++++++++++++++++++++++++++
 patchwork/models.py                         |  8 ++++-
 patchwork/parser.py                         | 22 ++++++++-----
 patchwork/tests/test_parser.py              | 18 ++++++-----
 patchwork/tests/test_rest_api.py            |  4 +--
 patchwork/tests/test_series.py              | 29 +++++++++++++++--
 patchwork/tests/utils.py                    |  1 +
 7 files changed, 109 insertions(+), 21 deletions(-)
 create mode 100644 patchwork/migrations/0016_series_project.py

diff --git a/patchwork/migrations/0016_series_project.py b/patchwork/migrations/0016_series_project.py
new file mode 100644
index 0000000..ecd8984
--- /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 4ae41be..ea40fbf 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 16cc53c..7e2067e 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 e4a379d..80f2f01 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)
+        ref_v1 = 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 88b7163..0d1c702 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 b9a764c..7377d91 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 d94e889..0c6f763 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,
-- 
2.9.3



More information about the Patchwork mailing list