launchpad-reviewers team mailing list archive

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

Thiago F. Pappacena has proposed merging ~pappacena/launchpad:refactoring-gitrepo-getRefByPath into launchpad:master.

Commit message:
Refactoring GitRepository.getRefByPath method to do less database queries.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/383250
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:refactoring-gitrepo-getRefByPath into launchpad:master.

diff --git a/lib/lp/code/model/gitrepository.py b/lib/lp/code/model/gitrepository.py
index d88a384..416a4f3 100644
--- a/lib/lp/code/model/gitrepository.py
+++ b/lib/lp/code/model/gitrepository.py
@@ -596,11 +596,13 @@ class GitRepository(StormBase, WebhookTargetMixin, GitIdentityMixin):
         paths = [path]
         if not path.startswith(u"refs/heads/"):
             paths.append(u"refs/heads/%s" % path)
-        for try_path in paths:
-            ref = Store.of(self).find(
-                GitRef,
-                GitRef.repository_id == self.id,
-                GitRef.path == try_path).one()
+        refs = Store.of(self).find(
+            GitRef,
+            GitRef.repository_id == self.id,
+            GitRef.path.is_in(paths))
+        refs_by_path = {r.path: r for r in refs}
+        for path in paths:
+            ref = refs_by_path.get(path)
             if ref is not None:
                 return ref
         return None
diff --git a/lib/lp/code/model/tests/test_gitrepository.py b/lib/lp/code/model/tests/test_gitrepository.py
index 8a9bcc8..165980b 100644
--- a/lib/lp/code/model/tests/test_gitrepository.py
+++ b/lib/lp/code/model/tests/test_gitrepository.py
@@ -170,6 +170,7 @@ from lp.testing import (
     logout,
     person_logged_in,
     record_two_runs,
+    StormStatementRecorder,
     TestCaseWithFactory,
     verifyObject,
     )
@@ -3918,6 +3919,17 @@ class TestGitRepositoryWebservice(TestCaseWithFactory):
         self.assertEqual(200, response.status)
         self.assertIsNone(response.jsonBody())
 
+    def test_getRefByPath_query_count(self):
+        repository_db = self.factory.makeGitRepository()
+        ref_dbs = self.factory.makeGitRefs(
+            repository=repository_db, paths=[
+                "refs/heads/devel", "refs/heads/master"])
+
+        with StormStatementRecorder() as recorder:
+            ref = repository_db.getRefByPath("master")
+            self.assertEqual(ref_dbs[1], ref)
+            self.assertEqual(1, recorder.count)
+
     def test_subscribe(self):
         # A user can subscribe to a repository.
         repository_db = self.factory.makeGitRepository()