From: Jeremy Kerr Date: Wed, 29 Oct 2008 00:21:05 +0000 (+1100) Subject: [models] Make patches unique on (msgid, project), not just (msgid) X-Git-Url: http://git.ozlabs.org/?p=patchwork;a=commitdiff_plain;h=74425beba0dc641509c5268571ea5328ac8185ec [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 --- diff --git a/apps/patchwork/bin/parsemail.py b/apps/patchwork/bin/parsemail.py index 772728e..7f6727f 100755 --- 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 bb0b52c..91c0f97 100644 --- 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 3518432..0fad67b 100644 --- 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 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;