[models] Make patches unique on (msgid, project), not just (msgid)

On patchwork.ozlabs.org, we may see multiple patches for different
projects, but with the same message-id.

We want these patches to show up on both projects, so we need to change
the current UNIQUE contstraint on msgid.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>

diff --git a/apps/patchwork/bin/parsemail.py b/apps/patchwork/bin/parsemail.py
index 772728e41e82ffbba9e2d42a54ca17004652e3bf..7f6727fc79c9d7e7efaf36f303e8f3c2ee188a4d 100755 (executable)
--- a/apps/patchwork/bin/parsemail.py
+++ b/apps/patchwork/bin/parsemail.py
@@ -173,7 +173,7 @@ def find_content(project, mail):
         if patch:
             cpatch = patch
         else:
-            cpatch = find_patch_for_comment(mail)
+            cpatch = find_patch_for_comment(project, mail)
             if not cpatch:
                 return (None, None)
         comment = Comment(patch = cpatch, date = mail_date(mail),
@@ -182,7 +182,7 @@ def find_content(project, mail):
 
     return (patch, comment)
 
-def find_patch_for_comment(mail):
+def find_patch_for_comment(project, mail):
     # construct a list of possible reply message ids
     refs = []
     if 'In-Reply-To' in mail:
@@ -200,14 +200,14 @@ def find_patch_for_comment(mail):
 
         # first, check for a direct reply
         try:
-            patch = Patch.objects.get(msgid = ref)
+            patch = Patch.objects.get(project = project, msgid = ref)
             return patch
         except Patch.DoesNotExist:
             pass
 
         # see if we have comments that refer to a patch
         try:
-            comment = Comment.objects.get(msgid = ref)
+            comment = Comment.objects.get(patch__project = project, msgid = ref)
             return comment.patch
         except Comment.DoesNotExist:
             pass
@@ -319,8 +319,7 @@ def clean_content(str):
     str = sig_re.sub('', str)
     return str.strip()
 
-def main(args):
-    mail = message_from_file(sys.stdin)
+def parse_mail(mail):
 
     # some basic sanity checks
     if 'From' not in mail:
@@ -376,5 +375,9 @@ def main(args):
 
     return 0
 
+def main(args):
+    mail = message_from_file(sys.stdin)
+    return parse_mail(mail)
+
 if __name__ == '__main__':
     sys.exit(main(sys.argv))

diff --git a/apps/patchwork/models.py b/apps/patchwork/models.py
index bb0b52c165e979ed9f482290ecb1e0680b3ad25c..91c0f97f3f02e1208047757a1c022ccdd94df205 100644 (file)
--- a/apps/patchwork/models.py
+++ b/apps/patchwork/models.py
@@ -178,7 +178,7 @@ class PatchMbox(MIMENonMultipart):
 
 class Patch(models.Model):
     project = models.ForeignKey(Project)
-    msgid = models.CharField(max_length=255, unique = True)
+    msgid = models.CharField(max_length=255)
     name = models.CharField(max_length=255)
     date = models.DateTimeField(default=datetime.datetime.now)
     submitter = models.ForeignKey(Person)
@@ -265,10 +265,11 @@ class Patch(models.Model):
     class Meta:
         verbose_name_plural = 'Patches'
         ordering = ['date']
+        unique_together = [('msgid', 'project')]
 
 class Comment(models.Model):
     patch = models.ForeignKey(Patch)
-    msgid = models.CharField(max_length=255, unique = True)
+    msgid = models.CharField(max_length=255)
     submitter = models.ForeignKey(Person)
     date = models.DateTimeField(default = datetime.datetime.now)
     headers = models.TextField(blank = True)
@@ -282,6 +283,7 @@ class Comment(models.Model):
 
     class Meta:
         ordering = ['date']
+        unique_together = [('msgid', 'patch')]
 
 class Bundle(models.Model):
     owner = models.ForeignKey(User)

diff --git a/apps/patchwork/tests/patchparser.py b/apps/patchwork/tests/patchparser.py
index 351843259b69ab8c84012c817f349b282e55f381..0fad67b2eecf7c6875a30cd7e6d3948fe4135bb3 100644 (file)
--- a/apps/patchwork/tests/patchparser.py
+++ b/apps/patchwork/tests/patchparser.py
@@ -20,7 +20,7 @@
 import unittest
 import os
 from email import message_from_string
-from patchwork.models import Project, Person
+from patchwork.models import Project, Person, Patch, Comment
 from patchwork.tests.utils import read_patch, create_email, defaults
 
 try:
@@ -34,7 +34,7 @@ class PatchTest(unittest.TestCase):
     default_subject = defaults.subject
     project = defaults.project
 
-from patchwork.bin.parsemail import find_content, find_author
+from patchwork.bin.parsemail import find_content, find_author, parse_mail
 
 class InlinePatchTest(PatchTest):
     patch_filename = '0001-add-line.patch'
@@ -210,3 +210,68 @@ class SenderCorrelationTest(unittest.TestCase):
 
     def tearDown(self):
         self.person.delete()
+
+class MultipleProjectPatchTest(unittest.TestCase):
+    """ Test that patches sent to multiple patchwork projects are
+        handled correctly """
+
+    test_comment = 'Test Comment'
+    patch_filename = '0001-add-line.patch'
+    msgid = '<1@example.com>'
+
+    def setUp(self):
+        self.p1 = Project(linkname = 'test-project-1', name = 'Project 1',
+                listid = '1.example.com', listemail='1@example.com')
+        self.p2 = Project(linkname = 'test-project-2', name = 'Project 2',
+                listid = '2.example.com', listemail='2@example.com')
+
+        self.p1.save()
+        self.p2.save()
+
+        patch = read_patch(self.patch_filename)
+        email = create_email(self.test_comment + '\n' + patch)
+        email['Message-Id'] = self.msgid
+
+        del email['List-ID']
+        email['List-ID'] = '<' + self.p1.listid + '>'
+        parse_mail(email)
+
+        del email['List-ID']
+        email['List-ID'] = '<' + self.p2.listid + '>'
+        parse_mail(email)
+
+    def testParsedProjects(self):
+        self.assertEquals(Patch.objects.filter(project = self.p1).count(), 1)
+        self.assertEquals(Patch.objects.filter(project = self.p2).count(), 1)
+
+    def tearDown(self):
+        self.p1.delete()
+        self.p2.delete()
+
+
+class MultipleProjectPatchCommentTest(MultipleProjectPatchTest):
+    """ Test that followups to multiple-project patches end up on the
+        correct patch """
+
+    comment_msgid = '<2@example.com>'
+    comment_content = 'test comment'
+
+    def setUp(self):
+        super(MultipleProjectPatchCommentTest, self).setUp()
+
+        for project in [self.p1, self.p2]:
+            email = MIMEText(self.comment_content)
+            email['From'] = defaults.sender
+            email['Subject'] = defaults.subject
+            email['Message-Id'] = self.comment_msgid
+            email['List-ID'] = '<' + project.listid + '>'
+            email['In-Reply-To'] = self.msgid
+            parse_mail(email)
+
+    def testParsedComment(self):
+        for project in [self.p1, self.p2]:
+            patch = Patch.objects.filter(project = project)[0]
+            # we should see two comments now - the original mail with the patch,
+            # and the one we parsed in setUp()
+            self.assertEquals(Comment.objects.filter(patch = patch).count(), 2)
+

diff --git a/lib/sql/migration/004-msgid-uniqueness.sql b/lib/sql/migration/004-msgid-uniqueness.sql
new file mode 100644 (file)
index 0000000..1e8ac0b
--- /dev/null
+++ b/lib/sql/migration/004-msgid-uniqueness.sql
@@ -0,0 +1,7 @@
+BEGIN;
+ALTER TABLE patchwork_patch DROP CONSTRAINT "patchwork_patch_msgid_key";
+ALTER TABLE patchwork_comment DROP CONSTRAINT "patchwork_comment_msgid_key";
+
+ALTER TABLE patchwork_patch ADD UNIQUE ("msgid", "project_id");
+ALTER TABLE patchwork_comment ADD UNIQUE ("msgid", "patch_id");
+COMMIT;