From 19c2841f20653fd3c59f73fdb16f7f7b1ea15434 Mon Sep 17 00:00:00 2001 From: Dan McGee Date: Thu, 3 Nov 2011 22:46:36 -0500 Subject: 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 --- main/models.py | 14 +++++++++++--- packages/utils.py | 34 +++++++++++++++++++++++++++++++--- 2 files changed, 42 insertions(+), 6 deletions(-) diff --git a/main/models.py b/main/models.py index 972b098c..db456c20 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 65769baf..0d756a85 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') -- cgit v1.2.3-55-g3dc8