launchpad-reviewers team mailing list archive

[Colin Watson <mp+436457@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:stricter-branch-getByPath into launchpad:master.

Commit message:
Make BranchSet.getByPath forbid paths with suffixes

Requested reviews:
  Jelmer Vernooij (jelmer)
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #2003950 in Launchpad itself: "brz branch <location> partially ignores the location path when using the lp: shorthand"
  https://bugs.launchpad.net/launchpad/+bug/2003950

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/436457

It previously accepted suffixes and discarded them.  This stricter approach is consistent with how `GitRepositorySet.getByPath` works, and it makes things easier for Breezy (which already walks up the path structure until it finds a match, but we were deceiving it by returning inexact matches).  It's also a better match to the existing API documentation, which didn't mention anything about accepting suffixes.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:stricter-branch-getByPath into launchpad:master.

diff --git a/lib/lp/code/model/branchlookup.py b/lib/lp/code/model/branchlookup.py
index 8888a0f..5c40120 100644
--- a/lib/lp/code/model/branchlookup.py
+++ b/lib/lp/code/model/branchlookup.py
@@ -410,7 +410,7 @@ class BranchLookup:
     def getByPath(self, path):
         """See `IBranchLookup`."""
         try:
-            return self.getByLPPath(path)[0]
+            branch, suffix = self.getByLPPath(path)
         except (
             CannotHaveLinkedBranch,
             InvalidNamespace,
@@ -424,6 +424,9 @@ class BranchLookup:
             NoLinkedBranch,
         ):
             return None
+        if suffix:
+            return None
+        return branch
 
     def _getLinkedBranchAndPath(self, provided):
         """Get the linked branch for 'provided', and the bzr_path.
diff --git a/lib/lp/code/model/tests/test_branchlookup.py b/lib/lp/code/model/tests/test_branchlookup.py
index 206e540..6cf90c1 100644
--- a/lib/lp/code/model/tests/test_branchlookup.py
+++ b/lib/lp/code/model/tests/test_branchlookup.py
@@ -268,7 +268,10 @@ class TestGetByPath(TestGetByLPPath):
         self.assertIsNone(self.getByPath(path))
 
     def assertPath(self, expected_branch, expected_suffix, path):
-        self.assertEqual(expected_branch, self.getByPath(path))
+        if expected_suffix:
+            self.assertIsNone(self.getByPath(path))
+        else:
+            self.assertEqual(expected_branch, self.getByPath(path))
 
 
 class TestGetByUrl(TestCaseWithFactory):