From patchwork Wed Oct 16 12:06:38 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stephen Finucane X-Patchwork-Id: 1177845 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.ozlabs.org (lists.ozlabs.org [203.11.71.2]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 46tWHt17vtz9sP6 for ; Wed, 16 Oct 2019 23:07:06 +1100 (AEDT) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=that.guru Authentication-Results: ozlabs.org; dkim=fail reason="key not found in DNS" (0-bit key; unprotected) header.d=that.guru header.i=@that.guru header.b="pIR9xwwI"; dkim-atps=neutral Received: from bilbo.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 46tWHs6d59zDqhn for ; Wed, 16 Oct 2019 23:07:05 +1100 (AEDT) X-Original-To: patchwork@lists.ozlabs.org Delivered-To: patchwork@lists.ozlabs.org Authentication-Results: lists.ozlabs.org; spf=none (no SPF record) smtp.mailfrom=that.guru (client-ip=199.181.239.144; helo=relay0144.mxlogin.com; envelope-from=stephen@that.guru; receiver=) Authentication-Results: lists.ozlabs.org; dmarc=none (p=none dis=none) header.from=that.guru Authentication-Results: lists.ozlabs.org; dkim=fail reason="key not found in DNS" (0-bit key; unprotected) header.d=that.guru header.i=@that.guru header.b="pIR9xwwI"; dkim-atps=neutral Received: from relay0144.mxlogin.com (relay0144.mxlogin.com [199.181.239.144]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 46tWHk0VjRzDqbs for ; Wed, 16 Oct 2019 23:06:56 +1100 (AEDT) Received: from filter004.mxroute.com (unknown [94.130.183.33]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay0144.mxlogin.com (Postfix) with ESMTPS id 01F5CCC30328; Wed, 16 Oct 2019 07:06:52 -0500 (CDT) Received: from one.mxroute.com (one.mxroute.com [195.201.59.211]) by filter004.mxroute.com (Postfix) with ESMTPS id E28F43F4EA; Wed, 16 Oct 2019 12:06:48 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=that.guru; s=default; h=Content-Transfer-Encoding:MIME-Version:Message-Id:Date:Subject: Cc:To:From:Sender:Reply-To:Content-Type:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: In-Reply-To:References:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=U6wWb8t21G1dc4/9ctNqtU6ANbe9EMtrBi/uV9qYTl8=; b=pIR9xwwIoX4EiJzQT++NAZFZeD CwTvcIo5LVTb5BK4xuP919/qnEocCRLfu75xt15uCvGuFElGWanOH1qlwSqSXjbrwKuiONRXiDmEA JjxopK6xql4S+6SeB+yrKqJ0T7gXFduv1hZxY1JrLNHMbHxG875ZRXf8hCC+v4aO5HBJuUknJQln/ YiTS+XTQf7HwGT77/zZGWKqDsLos/C0RxlVjSQe+eL52pXn1vwWYVWjNOtcFTErgNKo8eAdSNiNEh nZjoEuQGjfnaqjbTmdSuRU2E2CNNn4Dd4yjvfsm3diDQEuLEDWJOLhmCOL9zA6KDvGeUFErjatA/u GctDhOUw==; From: Stephen Finucane To: patchwork@lists.ozlabs.org Subject: [PATCH] models: Use database constraints to prevent split Series Date: Wed, 16 Oct 2019 13:06:38 +0100 Message-Id: <20191016120638.28682-1-stephen@that.guru> X-Mailer: git-send-email 2.21.0 MIME-Version: 1.0 X-AuthUser: stephen@that.guru X-BeenThere: patchwork@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Patchwork development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: patchwork-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org Sender: "Patchwork" Currently, the 'SeriesReference' object has a unique constraint on the two fields it has, 'series', which is a foreign key to 'Series', and 'msgid'. This is the wrong constraint. What we actually want to enforce is that a patch, cover letter or comment is referenced by a single series, or rather a single series per project the submission appears on. As such, we should be enforcing uniqueness on the msgid and the project that the patch, cover letter or comment belongs to. This requires adding a new field to the object, 'project', since it's not possible to do something like the following: unique_together = [('msgid', 'series__project')] This is detailed here [1]. In addition, the migration needs a precursor migration step to merge any broken series. [1] https://stackoverflow.com/a/4440189/613428 Signed-off-by: Stephen Finucane Closes: #241 Cc: Daniel Axtens Cc: Petr Vorel --- patchwork/migrations/0037_python_3.py | 298 ++++++++++++++++++ .../0038_unique_series_references.py | 121 +++++++ patchwork/models.py | 6 +- 3 files changed, 423 insertions(+), 2 deletions(-) create mode 100644 patchwork/migrations/0037_python_3.py create mode 100644 patchwork/migrations/0038_unique_series_references.py diff --git patchwork/migrations/0037_python_3.py patchwork/migrations/0037_python_3.py new file mode 100644 index 00000000..416a7045 --- /dev/null +++ patchwork/migrations/0037_python_3.py @@ -0,0 +1,298 @@ +import datetime + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion + +import patchwork.models + + +class Migration(migrations.Migration): + + dependencies = [('patchwork', '0036_project_commit_url_format')] + + operations = [ + migrations.AlterField( + model_name='check', + name='context', + field=models.SlugField( + default='default', + help_text='A label to discern check from checks of other testing systems.', # noqa + max_length=255, + ), + ), + migrations.AlterField( + model_name='check', + name='description', + field=models.TextField( + blank=True, + help_text='A brief description of the check.', + null=True, + ), + ), + migrations.AlterField( + model_name='check', + name='state', + field=models.SmallIntegerField( + choices=[ + (0, 'pending'), + (1, 'success'), + (2, 'warning'), + (3, 'fail'), + ], + default=0, + help_text='The state of the check.', + ), + ), + migrations.AlterField( + model_name='check', + name='target_url', + field=models.URLField( + blank=True, + help_text='The target URL to associate with this check. This should be specific to the patch.', # noqa + null=True, + ), + ), + migrations.AlterField( + model_name='comment', + name='submission', + field=models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name='comments', + related_query_name='comment', + to='patchwork.Submission', + ), + ), + migrations.AlterField( + model_name='delegationrule', + name='path', + field=models.CharField( + help_text='An fnmatch-style pattern to match filenames against.', # noqa + max_length=255, + ), + ), + migrations.AlterField( + model_name='delegationrule', + name='priority', + field=models.IntegerField( + default=0, + help_text='The priority of the rule. Rules with a higher priority will override rules with lower priorities', # noqa + ), + ), + migrations.AlterField( + model_name='delegationrule', + name='user', + field=models.ForeignKey( + help_text='A user to delegate the patch to.', + on_delete=django.db.models.deletion.CASCADE, + to=settings.AUTH_USER_MODEL, + ), + ), + migrations.AlterField( + model_name='emailconfirmation', + name='type', + field=models.CharField( + choices=[ + ('userperson', 'User-Person association'), + ('registration', 'Registration'), + ('optout', 'Email opt-out'), + ], + max_length=20, + ), + ), + migrations.AlterField( + model_name='event', + name='category', + field=models.CharField( + choices=[ + ('cover-created', 'Cover Letter Created'), + ('patch-created', 'Patch Created'), + ('patch-completed', 'Patch Completed'), + ('patch-state-changed', 'Patch State Changed'), + ('patch-delegated', 'Patch Delegate Changed'), + ('check-created', 'Check Created'), + ('series-created', 'Series Created'), + ('series-completed', 'Series Completed'), + ], + db_index=True, + help_text='The category of the event.', + max_length=20, + ), + ), + migrations.AlterField( + model_name='event', + name='cover', + field=models.ForeignKey( + blank=True, + help_text='The cover letter that this event was created for.', + null=True, + on_delete=django.db.models.deletion.CASCADE, + related_name='+', + to='patchwork.CoverLetter', + ), + ), + migrations.AlterField( + model_name='event', + name='date', + field=models.DateTimeField( + default=datetime.datetime.utcnow, + help_text='The time this event was created.', + ), + ), + migrations.AlterField( + model_name='event', + name='patch', + field=models.ForeignKey( + blank=True, + help_text='The patch that this event was created for.', + null=True, + on_delete=django.db.models.deletion.CASCADE, + related_name='+', + to='patchwork.Patch', + ), + ), + migrations.AlterField( + model_name='event', + name='project', + field=models.ForeignKey( + help_text='The project that the events belongs to.', + on_delete=django.db.models.deletion.CASCADE, + related_name='+', + to='patchwork.Project', + ), + ), + migrations.AlterField( + model_name='event', + name='series', + field=models.ForeignKey( + blank=True, + help_text='The series that this event was created for.', + null=True, + on_delete=django.db.models.deletion.CASCADE, + related_name='+', + to='patchwork.Series', + ), + ), + migrations.AlterField( + model_name='patch', + name='number', + field=models.PositiveSmallIntegerField( + default=None, + help_text='The number assigned to this patch in the series', + null=True, + ), + ), + migrations.AlterField( + model_name='project', + name='commit_url_format', + field=models.CharField( + blank=True, + help_text='URL format for a particular commit. {} will be replaced by the commit SHA.', # noqa + max_length=2000, + ), + ), + migrations.AlterField( + model_name='project', + name='list_archive_url_format', + field=models.CharField( + blank=True, + help_text="URL format for the list archive's Message-ID redirector. {} will be replaced by the Message-ID.", # noqa + max_length=2000, + ), + ), + migrations.AlterField( + model_name='project', + name='subject_match', + field=models.CharField( + blank=True, + default='', + help_text='Regex to match the subject against if only part of emails sent to the list belongs to this project. Will be used with IGNORECASE and MULTILINE flags. If rules for more projects match the first one returned from DB is chosen; empty field serves as a default for every email which has no other match.', # noqa + max_length=64, + validators=[patchwork.models.validate_regex_compiles], + ), + ), + migrations.AlterField( + model_name='series', + name='name', + field=models.CharField( + blank=True, + help_text='An optional name to associate with the series, e.g. "John\'s PCI series".', # noqa + max_length=255, + null=True, + ), + ), + migrations.AlterField( + model_name='series', + name='total', + field=models.IntegerField( + help_text='Number of patches in series as indicated by the subject prefix(es)' # noqa + ), + ), + migrations.AlterField( + model_name='series', + name='version', + field=models.IntegerField( + default=1, + help_text='Version of series as indicated by the subject prefix(es)', # noqa + ), + ), + migrations.AlterField( + model_name='seriesreference', + name='series', + field=models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name='references', + related_query_name='reference', + to='patchwork.Series', + ), + ), + migrations.AlterField( + model_name='tag', + name='abbrev', + field=models.CharField( + help_text='Short (one-or-two letter) abbreviation for the tag, used in table column headers', # noqa + max_length=2, + unique=True, + ), + ), + migrations.AlterField( + model_name='tag', + name='pattern', + field=models.CharField( + help_text='A simple regex to match the tag in the content of a message. Will be used with MULTILINE and IGNORECASE flags. eg. ^Acked-by:', # noqa + max_length=50, + validators=[patchwork.models.validate_regex_compiles], + ), + ), + migrations.AlterField( + model_name='tag', + name='show_column', + field=models.BooleanField( + default=True, + help_text="Show a column displaying this tag's count in the patch list view", # noqa + ), + ), + migrations.AlterField( + model_name='userprofile', + name='items_per_page', + field=models.PositiveIntegerField( + default=100, help_text='Number of items to display per page' + ), + ), + migrations.AlterField( + model_name='userprofile', + name='send_email', + field=models.BooleanField( + default=False, + help_text='Selecting this option allows patchwork to send email on your behalf', # noqa + ), + ), + migrations.AlterField( + model_name='userprofile', + name='show_ids', + field=models.BooleanField( + default=False, + help_text='Show click-to-copy patch IDs in the list view', + ), + ), + ] diff --git patchwork/migrations/0038_unique_series_references.py patchwork/migrations/0038_unique_series_references.py new file mode 100644 index 00000000..c5517ada --- /dev/null +++ patchwork/migrations/0038_unique_series_references.py @@ -0,0 +1,121 @@ +from django.db import connection, migrations, models +from django.db.models import Count +import django.db.models.deletion + + +def merge_duplicate_series(apps, schema_editor): + SeriesReference = apps.get_model('patchwork', 'SeriesReference') + Patch = apps.get_model('patchwork', 'Patch') + + msgid_seriesrefs = {} + + # find all SeriesReference that share a msgid but point to different series + # and decide which of the series is going to be the authoritative one + msgid_counts = ( + SeriesReference.objects.values('msgid') + .annotate(count=Count('msgid')) + .filter(count__gt=1) + ) + for msgid_count in msgid_counts: + msgid = msgid_count['msgid'] + chosen_ref = None + for series_ref in SeriesReference.objects.filter(msgid=msgid): + if series_ref.series.cover_letter: + if chosen_ref: + # I don't think this can happen, but explode if it does + raise Exception( + "Looks like you've got two or more series that share " + "some patches but do not share a cover letter. Unable " + "to auto-resolve." + ) + + # if a series has a cover letter, that's the one we'll group + # everything under + chosen_ref = series_ref + + if not chosen_ref: + # if none of the series have cover letters, simply use the last + # one (hint: this relies on Python's weird scoping for for loops + # where 'series_ref' is still accessible outside the loop) + chosen_ref = series_ref + + msgid_seriesrefs[msgid] = chosen_ref + + # reassign any patches referring to non-authoritative series to point to + # the authoritative one, and delete the other series; we do this separately + # to allow us a chance to raise the exception above if necessary + for msgid, chosen_ref in msgid_seriesrefs.items(): + for series_ref in SeriesReference.objects.filter(msgid=msgid): + if series_ref == chosen_ref: + continue + + # update the patches to point to our chosen series instead, on the + # assumption that all other metadata is correct + for patch in Patch.objects.filter(series=series_ref.series): + patch.series = chosen_ref.series + patch.save() + + # delete the other series (which will delete the series ref) + series_ref.series.delete() + + +def copy_project_field(apps, schema_editor): + if connection.vendor == 'postgresql': + schema_editor.execute( + """ + UPDATE patchwork_seriesreference + SET project_id = patchwork_series.project_id + FROM patchwork_series + WHERE patchwork_seriesreference.series_id = patchwork_series.id + """ + ) + elif connection.vendor == 'mysql': + schema_editor.execute( + """ + UPDATE patchwork_seriesreference, patchwork_series + SET patchwork_seriesreference.project_id = patchwork_series.project_id + WHERE patchwork_seriesreference.series_id = patchwork_series.id + """ + ) # noqa + else: + SeriesReference = apps.get_model('patchwork', 'SeriesReference') + + for series_ref in SeriesReference.objects.all().select_related( + 'series' + ): + series_ref.project = series_ref.series.project + series_ref.save() + + +class Migration(migrations.Migration): + + dependencies = [('patchwork', '0037_python_3')] + + operations = [ + migrations.RunPython( + merge_duplicate_series, migrations.RunPython.noop, atomic=False + ), + migrations.AddField( + model_name='seriesreference', + name='project', + field=models.ForeignKey( + null=True, + on_delete=django.db.models.deletion.CASCADE, + to='patchwork.Project', + ), + ), + migrations.AlterUniqueTogether( + name='seriesreference', unique_together={('project', 'msgid')} + ), + migrations.RunPython( + copy_project_field, migrations.RunPython.noop, atomic=False + ), + migrations.AlterField( + model_name='seriesreference', + name='project', + field=models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + to='patchwork.Project', + ), + ), + ] diff --git patchwork/models.py patchwork/models.py index c198bc2c..e5bfecf8 100644 --- patchwork/models.py +++ patchwork/models.py @@ -79,7 +79,8 @@ class Project(models.Model): webscm_url = models.CharField(max_length=2000, blank=True) list_archive_url = models.CharField(max_length=2000, blank=True) list_archive_url_format = models.CharField( - max_length=2000, blank=True, + max_length=2000, + blank=True, help_text="URL format for the list archive's Message-ID redirector. " "{} will be replaced by the Message-ID.") commit_url_format = models.CharField( @@ -783,6 +784,7 @@ class SeriesReference(models.Model): required to handle the case whereby one or more patches are received before the cover letter. """ + project = models.ForeignKey(Project, on_delete=models.CASCADE) series = models.ForeignKey(Series, related_name='references', related_query_name='reference', on_delete=models.CASCADE) @@ -792,7 +794,7 @@ class SeriesReference(models.Model): return self.msgid class Meta: - unique_together = [('series', 'msgid')] + unique_together = [('project', 'msgid')] class Bundle(models.Model):