[PATCH v3 07/16] REST: Resolve performance issues with tags

[Stephen Finucane]

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 at that.guru>
---
 patchwork/api/patch.py           | 15 ++++++++-------
 patchwork/models.py              | 13 ++++++++++---
 patchwork/tests/test_rest_api.py |  5 ++---
 3 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/patchwork/api/patch.py b/patchwork/api/patch.py
index 3af5994..811ba1e 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 15a2936..6501958 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 3be8ecf..ddce4d7 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 at 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."""
-- 
2.7.4