launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/git-basic-browser into lp:launchpad.

Commit message:
Fix the query count for lp.code.browser.tests.test_branch.TestBranchView.test_query_count_index_with_subscribers.

Requested reviews:
  Colin Watson (cjwatson)
Related bugs:
  Bug #1032731 in Launchpad itself: "Support for Launchpad-hosted Git repositories"
  https://bugs.launchpad.net/launchpad/+bug/1032731

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/git-basic-browser/+merge/251911

Fix the query count for lp.code.browser.tests.test_branch.TestBranchView.test_query_count_index_with_subscribers.  One of the queries is easily avoidable by passing the context IPerson down to IGitTraverse.traverse so that it doesn't have to look it up again; the other is an extra PillarName lookup and doesn't seem worth the effort to avoid.
-- 
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.

=== modified file 'lib/lp/code/browser/tests/test_branch.py'
--- lib/lp/code/browser/tests/test_branch.py	2014-12-06 10:45:17 +0000
+++ lib/lp/code/browser/tests/test_branch.py	2015-03-05 11:45:47 +0000
@@ -558,7 +558,7 @@
         logout()
         with StormStatementRecorder() as recorder:
             browser.open(branch_url)
-        self.assertThat(recorder, HasQueryCount(Equals(26)))
+        self.assertThat(recorder, HasQueryCount(Equals(27)))
 
 
 class TestBranchViewPrivateArtifacts(BrowserTestCase):

=== modified file 'lib/lp/code/interfaces/gitlookup.py'
--- lib/lp/code/interfaces/gitlookup.py	2015-02-27 10:01:45 +0000
+++ lib/lp/code/interfaces/gitlookup.py	2015-03-05 11:45:47 +0000
@@ -33,9 +33,9 @@
 class IGitTraverser(Interface):
     """Utility for traversing to an object that can have a Git repository."""
 
-    def traverse(segments):
+    def traverse(segments, owner=None):
         """Traverse to the object referred to by a prefix of the 'segments'
-        iterable.
+        iterable, starting from 'owner' if given.
 
         :raises InvalidNamespace: If the path cannot be parsed as a
             repository namespace.

=== modified file 'lib/lp/code/model/gitlookup.py'
--- lib/lp/code/model/gitlookup.py	2015-03-03 01:36:13 +0000
+++ lib/lp/code/model/gitlookup.py	2015-03-05 11:45:47 +0000
@@ -252,12 +252,15 @@
 
     implements(IGitTraverser)
 
-    def traverse(self, segments):
+    def traverse(self, segments, owner=None):
         """See `IGitTraverser`."""
-        owner = None
-        target = None
         repository = None
-        traversable = RootGitTraversable()
+        if owner is None:
+            target = None
+            traversable = RootGitTraversable()
+        else:
+            target = owner
+            traversable = adapt(owner, IGitTraversable)
         segments_iter = SegmentIterator(segments)
         while traversable is not None:
             try:

=== modified file 'lib/lp/code/model/tests/test_gitlookup.py'
--- lib/lp/code/model/tests/test_gitlookup.py	2015-03-03 01:36:13 +0000
+++ lib/lp/code/model/tests/test_gitlookup.py	2015-03-05 11:45:47 +0000
@@ -453,3 +453,17 @@
                 person.name, dsp.distribution.name, dsp.sourcepackagename.name,
                 repository.name),
             person, dsp, repository)
+
+    def test_person_repository_from_person(self):
+        # To save on queries, `traverse` can be given a person as a starting
+        # point for the traversal.
+        person = self.factory.makePerson(name="person")
+        repository = self.factory.makeGitRepository(
+            owner=person, target=person, name=u"repository")
+        segments = ["~person", "+git", "repository"]
+        self.assertEqual(
+            (person, person, repository),
+            self.traverser.traverse(iter(segments)))
+        self.assertEqual(
+            (person, person, repository),
+            self.traverser.traverse(iter(segments[1:]), owner=person))

=== modified file 'lib/lp/registry/browser/person.py'
--- lib/lp/registry/browser/person.py	2015-03-05 10:18:41 +0000
+++ lib/lp/registry/browser/person.py	2015-03-05 11:45:47 +0000
@@ -373,12 +373,13 @@
             # the traversal stack immediately, as if we fail to find a Git
             # repository we will need to look for a Bazaar branch instead.
             segments = (
-                ["~%s" % self.context.name, pillar_name] +
+                [pillar_name] +
                 list(reversed(self.request.getTraversalStack())))
             num_segments = len(segments)
             iter_segments = iter(segments)
             traverser = getUtility(IGitTraverser)
-            _, target, repository = traverser.traverse(iter_segments)
+            _, target, repository = traverser.traverse(
+                iter_segments, owner=self.context)
             if repository is None:
                 raise NotFoundError
             for i in range(num_segments - len(list(iter_segments))):