Add new attach_maintainers() utility method

This allows us to alleviate the N+1 query problem when we want
maintainer data for a queryset of packages. We use it on signoffs here;
we should also be able to apply this to the todolist section where this
problem has existed for some time.

Signed-off-by: Dan McGee <dan@archlinux.org>

2 files changed, 42 insertions, 6 deletions

diff --git a/main/models.py b/main/models.py
index 972b098..db456c2 100644
--- a/main/models.py
+++ b/main/models.py
@@ -206,11 +206,19 @@ class Package(models.Model):
     def is_signed(self):
         return bool(self.pgp_signature)
 
+    _maintainers = None
+
     @property
     def maintainers(self):
-        return User.objects.filter(
-                package_relations__pkgbase=self.pkgbase,
-                package_relations__type=PackageRelation.MAINTAINER)
+        if self._maintainers is None:
+            self._maintainers = User.objects.filter(
+                    package_relations__pkgbase=self.pkgbase,
+                    package_relations__type=PackageRelation.MAINTAINER)
+        return self._maintainers
+
+    @maintainers.setter
+    def maintainers(self, maintainers):
+        self._maintainers = maintainers
 
     @cache_function(300)
     def applicable_arches(self):
diff --git a/packages/utils.py b/packages/utils.py
index 65769ba..0d756a8 100644
--- a/packages/utils.py
+++ b/packages/utils.py
@@ -1,3 +1,4 @@
+from collections import defaultdict
 from operator import itemgetter
 
 from django.db import connection
@@ -127,6 +128,7 @@ SELECT p.id, q.id
     differences.sort(key=lambda a: (a.repo.name, a.pkgname))
     return differences
 
+
 def get_wrong_permissions():
     sql = """
 SELECT DISTINCT id
@@ -150,6 +152,32 @@ SELECT DISTINCT id
     return relations
 
 
+def attach_maintainers(packages):
+    '''Given a queryset or something resembling it of package objects, find all
+    the maintainers and attach them to the packages to prevent N+1 query
+    cascading.'''
+    pkgbases = set(p.pkgbase for p in packages)
+    rels = PackageRelation.objects.filter(type=PackageRelation.MAINTAINER,
+            pkgbase__in=pkgbases).values_list('pkgbase', 'user_id').distinct()
+
+    # get all the user objects we will need
+    user_ids = set(rel[1] for rel in rels)
+    users = User.objects.in_bulk(user_ids)
+
+    # now build a pkgbase -> [maintainers...] map
+    maintainers = defaultdict(list)
+    for rel in rels:
+        maintainers[rel[0]].append(users[rel[1]])
+
+    annotated = []
+    # and finally, attach the maintainer lists on the original packages
+    for package in packages:
+        package.maintainers = maintainers[package.pkgbase]
+        annotated.append(package)
+
+    return annotated
+
+
 def approved_by_signoffs(signoffs, spec):
     if signoffs:
         good_signoffs = sum(1 for s in signoffs if not s.revoked)
@@ -173,9 +201,7 @@ class PackageSignoffGroup(object):
         self.version = ''
         self.last_update = first.last_update
         self.packager = first.packager
-        self.maintainers = User.objects.filter(
-                package_relations__type=PackageRelation.MAINTAINER,
-                package_relations__pkgbase=self.pkgbase)
+        self.maintainers = first.maintainers
 
         self.specification = \
                 SignoffSpecification.objects.get_or_default_from_package(first)
@@ -236,6 +262,7 @@ class PackageSignoffGroup(object):
 
 def get_current_signoffs(repos):
     '''Returns a mapping of pkgbase -> signoff objects for the given repos.'''
+    # TODO this isn't current at all- this is every single signoff...
     cursor = connection.cursor()
     sql = """
 SELECT DISTINCT s.id
@@ -274,6 +301,7 @@ def get_signoff_groups(repos=None):
     test_pkgs = Package.objects.select_related(
             'arch', 'repo', 'packager').filter(repo__in=repo_ids)
     packages = test_pkgs.order_by('pkgname')
+    packages = attach_maintainers(packages)
 
     # Collect all pkgbase values in testing repos
     q_pkgbase = test_pkgs.values('pkgbase')