Rework tagging infrastructure

Solve getpatchwork#113 and getpatchwork#57 GitHub issues, fix up returning tags in the API,
keep track of tag origin to later be able to add tags to comments in
the API.

Use relations Tag-Patch and Tag-CoverLetter to avoid duplication of
tags for each patch in series.

Signed-off-by: Veronika Kabatova <vkabatov@redhat.com>

docs/usage/overview.rst

@@ -119,10 +119,11 @@ one delegate can be assigned to a patch.
 Tags
 ~~~~
 
-Tags are specially formatted metadata appended to the foot the body of a patch
-or a comment on a patch. Patchwork extracts these tags at parse time and
-associates them with the patch. You add extra tags to an email by replying to
-the email. The following tags are available on a standard Patchwork install:
+Tags are specially formatted metadata appended to the foot the body of a patch,
+cover letter or a comment related to them. Patchwork extracts these tags at
+parse time and associates them with patches. You add extra tags to an email by
+replying to the email. The following tags are available on a standard Patchwork
+install:
 
 ``Acked-by:``
   For example::

patchwork/api/comment.py

@@ -26,18 +26,34 @@
 from patchwork.api.base import PatchworkPermission
 from patchwork.api.embedded import PersonSerializer
 from patchwork.models import Comment
+from patchwork.models import SubmissionTag
 
 
 class CommentListSerializer(BaseHyperlinkedModelSerializer):
 
     subject = SerializerMethodField()
     headers = SerializerMethodField()
     submitter = PersonSerializer(read_only=True)
+    tags = SerializerMethodField()
 
     def get_subject(self, comment):
         return email.parser.Parser().parsestr(comment.headers,
                                               True).get('Subject', '')
 
+    def get_tags(self, instance):
+        if not instance.submission.project.use_tags:
+            return {}
+
+        tags = SubmissionTag.objects.filter(
+            comment=instance
+        ).values_list('tag__name', 'value')
+
+        result = {}
+        for name, value in tags:
+            result.setdefault(name, []).append(value)
+
+        return result
+
     def get_headers(self, comment):
         headers = {}
 
@@ -55,7 +71,7 @@ def get_headers(self, comment):
     class Meta:
         model = Comment
         fields = ('id', 'msgid', 'date', 'subject', 'submitter', 'content',
-                  'headers')
+                  'headers', 'tags')
         read_only_fields = fields
 
 

patchwork/api/cover.py

@@ -30,6 +30,7 @@
 from patchwork.api.embedded import ProjectSerializer
 from patchwork.api.embedded import SeriesSerializer
 from patchwork.models import CoverLetter
+from patchwork.models import SubmissionTag
 
 
 class CoverLetterListSerializer(BaseHyperlinkedModelSerializer):
@@ -39,6 +40,7 @@ class CoverLetterListSerializer(BaseHyperlinkedModelSerializer):
     mbox = SerializerMethodField()
     series = SeriesSerializer(many=True, read_only=True)
     comments = SerializerMethodField()
+    tags = SerializerMethodField()
 
     def get_mbox(self, instance):
         request = self.context.get('request')
@@ -48,13 +50,26 @@ def get_comments(self, cover):
         return self.context.get('request').build_absolute_uri(
             reverse('api-cover-comment-list', kwargs={'pk': cover.id}))
 
+    def get_tags(self, instance):
+        if not instance.project.use_tags:
+            return {}
+
+        tags = SubmissionTag.objects.filter(
+            submission=instance).values_list('tag__name', 'value').distinct()
+
+        result = {}
+        for name, value in tags:
+            result.setdefault(name, []).append(value)
+
+        return result
+
     class Meta:
         model = CoverLetter
         fields = ('id', 'url', 'project', 'msgid', 'date', 'name', 'submitter',
-                  'mbox', 'series', 'comments')
+                  'mbox', 'series', 'comments', 'tags')
         read_only_fields = fields
         versioned_fields = {
-            '1.1': ('mbox', 'comments'),
+            '1.1': ('mbox', 'comments', 'tags'),
         }
         extra_kwargs = {
             'url': {'view_name': 'api-cover-detail'},

patchwork/api/filters.py

@@ -21,9 +21,11 @@
 from django.core.exceptions import ValidationError
 from django.db.models import Q
 from django_filters.rest_framework import FilterSet
+from django_filters import Filter
 from django_filters import IsoDateTimeFilter
 from django_filters import ModelMultipleChoiceFilter
 from django.forms import ModelMultipleChoiceField as BaseMultipleChoiceField
+from django.forms.widgets import HiddenInput
 from django.forms.widgets import MultipleHiddenInput
 
 from patchwork.models import Bundle
@@ -35,6 +37,7 @@
 from patchwork.models import Project
 from patchwork.models import Series
 from patchwork.models import State
+from patchwork.models import Tag
 
 
 # custom fields, filters
@@ -136,6 +139,22 @@ class StateFilter(ModelMultipleChoiceFilter):
     field_class = StateChoiceField
 
 
+class TagNameChoiceField(ModelMultipleChoiceField):
+
+    alternate_lookup = 'name__iexact'
+
+
+class TagNameFilter(ModelMultipleChoiceFilter):
+
+    field_class = TagNameChoiceField
+
+
+class TagValueFilter(Filter):
+
+    def filter(self, qs, value):
+        return qs.filter(submissiontag__value__icontains=value)
+
+
 class UserChoiceField(ModelMultipleChoiceField):
 
     alternate_lookup = 'username__iexact'
@@ -173,10 +192,14 @@ class CoverLetterFilterSet(TimestampMixin, FilterSet):
     series = BaseFilter(queryset=Project.objects.all(),
                         widget=MultipleHiddenInput)
     submitter = PersonFilter(queryset=Person.objects.all())
+    tagname = TagNameFilter(name='related_tags',
+                            label='Tag name',
+                            queryset=Tag.objects.all())
+    tagvalue = TagValueFilter(widget=HiddenInput)
 
     class Meta:
         model = CoverLetter
-        fields = ('project', 'series', 'submitter')
+        fields = ('project', 'series', 'submitter', 'tagname', 'tagvalue')
 
 
 class PatchFilterSet(TimestampMixin, FilterSet):
@@ -189,11 +212,15 @@ class PatchFilterSet(TimestampMixin, FilterSet):
     submitter = PersonFilter(queryset=Person.objects.all())
     delegate = UserFilter(queryset=User.objects.all())
     state = StateFilter(queryset=State.objects.all())
+    tagname = TagNameFilter(name='related_tags',
+                            label='Tag name',
+                            queryset=Tag.objects.all())
+    tagvalue = TagValueFilter(widget=HiddenInput)
 
     class Meta:
         model = Patch
         fields = ('project', 'series', 'submitter', 'delegate',
-                  'state', 'archived')
+                  'state', 'archived', 'tagname', 'tagvalue')
 
 
 class CheckFilterSet(TimestampMixin, FilterSet):

patchwork/api/patch.py

@@ -34,7 +34,9 @@
 from patchwork.api.embedded import SeriesSerializer
 from patchwork.api.embedded import UserSerializer
 from patchwork.models import Patch
+from patchwork.models import SeriesPatch
 from patchwork.models import State
+from patchwork.models import SubmissionTag
 from patchwork.parser import clean_subject
 
 
@@ -104,9 +106,24 @@ def get_checks(self, instance):
             reverse('api-check-list', kwargs={'patch_id': instance.id}))
 
     def get_tags(self, instance):
-        # TODO(stephenfin): Make tags performant, possibly by reworking the
-        # model
-        return {}
+        if not instance.project.use_tags:
+            return {}
+
+        sub_ids = [instance.id]
+        cover = SeriesPatch.objects.get(
+            patch_id=instance.id).series.cover_letter
+        if cover:
+            sub_ids.append(cover.id)
+
+        tags = SubmissionTag.objects.filter(
+            submission__id__in=sub_ids).values_list('tag__name',
+                                                    'value').distinct()
+
+        result = {}
+        for name, value in tags:
+            result.setdefault(name, []).append(value)
+
+        return result
 
     class Meta:
         model = Patch
@@ -123,6 +140,9 @@ class Meta:
         extra_kwargs = {
             'url': {'view_name': 'api-patch-detail'},
         }
+        versioned_fields = {
+            '1.1': ('tags', ),
+        }
 
 
 class PatchDetailSerializer(PatchListSerializer):
@@ -155,6 +175,7 @@ class Meta:
             'headers', 'content', 'diff', 'prefixes')
         versioned_fields = PatchListSerializer.Meta.versioned_fields
         extra_kwargs = PatchListSerializer.Meta.extra_kwargs
+        versioned_fields = PatchListSerializer.Meta.versioned_fields
 
 
 class PatchList(ListAPIView):

patchwork/management/commands/retag.py

@@ -19,11 +19,12 @@
 
 from django.core.management.base import BaseCommand
 
+from patchwork.models import Cover
 from patchwork.models import Patch
 
 
 class Command(BaseCommand):
-    help = 'Update the tag (Ack/Review/Test) counts on existing patches'
+    help = 'Update tags on existing patches'
     args = '[<patch_id>...]'
 
     def handle(self, *args, **options):
@@ -37,7 +38,17 @@ def handle(self, *args, **options):
         count = query.count()
 
         for i, patch in enumerate(query.iterator()):
-            patch.refresh_tag_counts()
+            patch.refresh_tags()
+            for comment in patch.comments.all():
+                comment.refresh_tags()
+
+            cover = SeriesPatch.objects.get(
+                patch_id=patch.id).series.cover_letter
+            if cover:
+                cover.refresh_tags()
+                for comment in cover.comments.all():
+                    comment.refresh_tags()
+
             if (i % 10) == 0:
                 self.stdout.write('%06d/%06d\r' % (i, count), ending='')
                 self.stdout.flush()

patchwork/migrations/0027_rework_tagging.py

@@ -0,0 +1,126 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+import django.db.models.deletion
+
+import re
+
+
+# Django migrations don't allow us to call models' methods because the
+# migration will break if the methods change. Therefore we need to use an
+# altered copy of all the code needed.
+def extract_tags(extract_from, tags):
+    found_tags = {}
+
+    if not extract_from.content:
+        return found_tags
+
+    for tag in tags:
+        regex = re.compile(tag.pattern + r'\s(.*)', re.M | re.I)
+        found_tags[tag] = regex.findall(extract_from.content)
+
+    return found_tags
+
+
+def add_tags(apps, submission, tag, values, comment=None):
+    if not values:
+        # We don't need to delete tags since none exist yet and we can't
+        # delete comments etc. during the migration
+        return
+
+    SubmissionTag = apps.get_model('patchwork', 'SubmissionTag')
+    SubmissionTag.objects.bulk_create([SubmissionTag(
+        submission=submission,
+        tag=tag,
+        value=value,
+        comment=comment
+    ) for value in values])
+
+
+def create_all(apps, schema_editor):
+    Tag = apps.get_model('patchwork', 'Tag')
+    tags = Tag.objects.all()
+
+    Submission = apps.get_model('patchwork', 'Submission')
+    for submission in Submission.objects.all():
+        extracted = extract_tags(submission, tags)
+        for tag in extracted:
+            add_tags(apps, submission, tag, extracted[tag])
+
+    Comment = apps.get_model('patchwork', 'Comment')
+    for comment in Comment.objects.all():
+        extracted = extract_tags(comment, tags)
+        for tag in extracted:
+            add_tags(apps,
+                     comment.submission,
+                     tag,
+                     extracted[tag],
+                     comment=comment)
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('patchwork', '0026_add_user_bundles_backref'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='SubmissionTag',
+            fields=[
+                ('id', models.AutoField(auto_created=True,
+                                        primary_key=True,
+                                        serialize=False,
+                                        verbose_name='ID')),
+                ('value', models.CharField(max_length=255)),
+                ('comment', models.ForeignKey(
+                    to='patchwork.Comment',
+                    null=True
+                )),
+                ('submission', models.ForeignKey(
+                    on_delete=django.db.models.deletion.CASCADE,
+                    to='patchwork.Submission'
+                )),
+                ('tag', models.ForeignKey(
+                    on_delete=django.db.models.deletion.CASCADE,
+                    to='patchwork.Tag'
+                )),
+            ],
+        ),
+        migrations.AlterUniqueTogether(
+            name='patchtag',
+            unique_together=set([]),
+        ),
+        migrations.RemoveField(
+            model_name='patchtag',
+            name='patch',
+        ),
+        migrations.RemoveField(
+            model_name='patchtag',
+            name='tag',
+        ),
+        migrations.RemoveField(
+            model_name='patch',
+            name='tags',
+        ),
+        migrations.DeleteModel(
+            name='PatchTag',
+        ),
+        migrations.AddField(
+            model_name='submission',
+            name='related_tags',
+            field=models.ManyToManyField(
+                through='patchwork.SubmissionTag',
+                to='patchwork.Tag'
+            ),
+        ),
+        migrations.AlterUniqueTogether(
+            name='submissiontag',
+            unique_together=set([('submission',
+                                  'tag',
+                                  'value',
+                                  'comment')]),
+        ),
+        migrations.RunPython(create_all, atomic=False),
+    ]