launchpad-reviewers team mailing list archive

["Thiago F. Pappacena" <pappacena@xxxxxxxxx>]

Thiago F. Pappacena has proposed merging ~pappacena/launchpad:getrefpermission-constant-query-count into launchpad:master.

Commit message:
Ensuring constant query count on GitRepository.checkRefPermissions

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/384843
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:getrefpermission-constant-query-count into launchpad:master.

diff --git a/lib/lp/code/model/gitrepository.py b/lib/lp/code/model/gitrepository.py
index bc9785a..2e9080f 100644
--- a/lib/lp/code/model/gitrepository.py
+++ b/lib/lp/code/model/gitrepository.py
@@ -1393,6 +1393,8 @@ class GitRepository(StormBase, WebhookTargetMixin, GitIdentityMixin):
         if is_owner:
             grants = grants.union(
                 self.findRuleGrantsByGrantee(GitGranteeType.REPOSITORY_OWNER))
+
+        bulk.load_related(Person, grants, ["grantee_id"])
         for grant in grants:
             grants_for_user[grant.rule].append(grant)
 
diff --git a/lib/lp/code/model/tests/test_gitrepository.py b/lib/lp/code/model/tests/test_gitrepository.py
index 2917c4e..1ef34b9 100644
--- a/lib/lp/code/model/tests/test_gitrepository.py
+++ b/lib/lp/code/model/tests/test_gitrepository.py
@@ -514,6 +514,40 @@ class TestGitRepository(TestCaseWithFactory):
             include_transitive=False)
         self.assertEqual([exact_grant], list(results))
 
+    def test_getRefsPermission_query_count(self):
+        repository = self.factory.makeGitRepository()
+        owner = repository.owner
+        grantees = [self.factory.makePerson() for _ in range(2)]
+
+        ref_paths = ['refs/heads/master']
+        creator_info = {"n": 1}
+
+        def add_fake_refs_to_request():
+            creator_info["n"] += 1
+            n = creator_info["n"]
+            ref_paths.append("refs/heads/branch-%s" % n)
+
+            with admin_logged_in():
+                teams = [self.factory.makeTeam() for _ in range(2)]
+                teams[0].addMember(grantees[0], teams[0].teamowner)
+                teams[1].addMember(grantees[1], teams[1].teamowner)
+            master_rule = self.factory.makeGitRule(
+                repository=repository, ref_pattern="refs/heads/branch-%s" % n)
+            self.factory.makeGitRuleGrant(
+                rule=master_rule, grantee=teams[0], can_create=True)
+            self.factory.makeGitRuleGrant(
+                rule=master_rule, grantee=teams[1], can_push=True)
+
+        def check_permissions():
+            repository.checkRefPermissions(grantees[0], ref_paths)
+
+        recorder1, recorder2 = record_two_runs(
+            check_permissions, add_fake_refs_to_request, 1, 10,
+            login_method=lambda: login_person(owner))
+
+        self.assertThat(recorder2, HasQueryCount.byEquality(recorder1))
+        self.assertEqual(7, recorder1.count)
+
 
 class TestGitIdentityMixin(TestCaseWithFactory):
     """Test the defaults and identities provided by GitIdentityMixin."""