From: Stephen Finucane <stephen@that.guru>
Date: Fri, 28 Oct 2016 18:02:41 +0000 (+0100)
Subject: REST: Resolve performance issues with tags
X-Git-Tag: v2.0.0-rc1~123
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=dc787e93a771addddf0f6ceff5d853240d217f2b;p=thirdparty%2Fpatchwork.git

REST: Resolve performance issues with tags

The list comprehension used to generate a list of tags resulted in a
significant number of duplicated queries. Resolve this by copying the
approach taken to minimize patch queries in the UI.

This changes the output of the response in two ways. First, counts for
all tag patches are now shown, even if the count is 0. Secondly, a
dictionary is returned, with the tag name as the key, rather than a
list. As such, the output for a typical patch is transformed from:

    [
      {
        'name': 'Reviewed-by',
        'count': 1
      }
    ]

to

    {
      'Reviewed-by': 1
      'Acked-by': 0,
      'Tested-by': 0
    }

How this is achieved is a little hacky, but reworked tags are on the
post-v2.0 which will allow this to be resolved.

Signed-off-by: Stephen Finucane <stephen@that.guru>
---

diff --git a/patchwork/api/patch.py b/patchwork/api/patch.py
index 3af5994f..811ba1e9 100644
--- a/patchwork/api/patch.py
+++ b/patchwork/api/patch.py
@@ -52,9 +52,11 @@ class PatchSerializer(HyperlinkedModelSerializer):
         return request.build_absolute_uri(instance.get_mbox_url())
 
     def get_tags(self, instance):
-        # TODO(stephenfin): I don't think this is correct - too many queries
-        return [{'name': x.tag.name, 'count': x.count}
-                for x in instance.patchtag_set.all()]
+        if instance.project.tags:
+            return {x.name: getattr(instance, x.attr_name)
+                    for x in instance.project.tags}
+        else:
+            return None
 
     def get_headers(self, instance):
         if instance.headers:
@@ -85,10 +87,9 @@ class PatchViewSet(PatchworkViewSet):
     serializer_class = PatchSerializer
 
     def get_queryset(self):
-        qs = super(PatchViewSet, self).get_queryset(
-        ).prefetch_related(
-            'check_set', 'patchtag_set'
-        ).select_related('state', 'submitter', 'delegate')
+        qs = super(PatchViewSet, self).get_queryset().with_tag_counts()\
+            .prefetch_related('check_set')\
+            .select_related('state', 'submitter', 'delegate')
         if 'pk' not in self.kwargs:
             # we are doing a listing, we don't need these fields
             qs = qs.defer('content', 'diff', 'headers')
diff --git a/patchwork/models.py b/patchwork/models.py
index cff9587f..4ae41bed 100644
--- a/patchwork/models.py
+++ b/patchwork/models.py
@@ -226,8 +226,8 @@ def get_default_initial_patch_state():
 
 class PatchQuerySet(models.query.QuerySet):
 
-    def with_tag_counts(self, project):
-        if not project.use_tags:
+    def with_tag_counts(self, project=None):
+        if project and not project.use_tags:
             return self
 
         # We need the project's use_tags field loaded for Project.tags().
@@ -237,7 +237,14 @@ class PatchQuerySet(models.query.QuerySet):
         qs = self.prefetch_related('project')
         select = OrderedDict()
         select_params = []
-        for tag in project.tags:
+
+        # All projects have the same tags, so we're good to go here
+        if project:
+            tags = project.tags
+        else:
+            tags = Tag.objects.all()
+
+        for tag in tags:
             select[tag.attr_name] = (
                 "coalesce("
                 "(SELECT count FROM patchwork_patchtag"
diff --git a/patchwork/tests/test_rest_api.py b/patchwork/tests/test_rest_api.py
index 3be8ecf4..ddce4d75 100644
--- a/patchwork/tests/test_rest_api.py
+++ b/patchwork/tests/test_rest_api.py
@@ -304,9 +304,8 @@ class TestPatchAPI(APITestCase):
             content='Reviewed-by: Test User <test@example.com>\n')
         resp = self.client.get(self.api_url(patch.id))
         tags = resp.data['tags']
-        self.assertEqual(1, len(tags))
-        self.assertEqual(1, tags[0]['count'])
-        self.assertEqual('Reviewed-by', tags[0]['name'])
+        self.assertEqual(3, len(tags))
+        self.assertEqual(1, tags['Reviewed-by'])
 
     def test_anonymous_create(self):
         """Ensure anonymous "POST" operations are rejected."""