← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~thumper/launchpad/get-linked-to-branch into lp:launchpad/devel

 

Tim Penhey has proposed merging lp:~thumper/launchpad/get-linked-to-branch into lp:launchpad/devel with lp:~thumper/launchpad/branch-target-adapters as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)


This branch is a pre-branch for the real work in lp:~thumper/launchpad/private-branch-lookup-bug-261609.

This branch takes the changes necessary to change the implementation of get_linked_branch to be get_linked_to_branch.  The function is changed to return the ILinkedBranch object rather than the IBranch object. 

lib/lp/code/model/branchlookup.py
 - with the short names being used for actual branch traversal rather than just identification
   the lookup method now has to return the rest of the path from the input (where it was previously
   discarded)

lib/lp/code/model/tests/test_branchlookup.py
 - tests to make sure that the lookup utility does return the rest of the path

There were a couple of other drive-by changes in this branch:

  lib/lp/code/interfaces/branchlookup.py
    - updated the docstring to actually represent the exceptions that could be raised

  lib/lp/codehosting/vfs/branchfsclient.py
    - a comment was added to a piece of code that look like it should be a string.startswith
      call, but the content is actually a list

  lib/lp/testing/tests/test_factory.py
    - a simple test to make sure that factory created branches have the correct next_mirror_time.
-- 
https://code.launchpad.net/~thumper/launchpad/get-linked-to-branch/+merge/32166
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~thumper/launchpad/get-linked-to-branch into lp:launchpad/devel.
=== modified file 'lib/lp/code/interfaces/branchlookup.py'
--- lib/lp/code/interfaces/branchlookup.py	2009-11-19 15:46:10 +0000
+++ lib/lp/code/interfaces/branchlookup.py	2010-08-10 03:38:54 +0000
@@ -34,10 +34,8 @@
     def traverse(path):
         """Traverse to the linked object referred to by 'path'.
 
-        :raises NoSuchBranch: If we can't find a branch that matches the
-            branch component of the path.
-        :raises NoSuchPerson: If we can't find a person who matches the person
-            component of the path.
+        :raises InvalidProductName: If the first segment of the path is not a
+            valid name.
         :raises NoSuchProduct: If we can't find a product that matches the
             product component of the path.
         :raises NoSuchProductSeries: If the series component doesn't match an

=== modified file 'lib/lp/code/interfaces/linkedbranch.py'
--- lib/lp/code/interfaces/linkedbranch.py	2010-08-03 03:37:46 +0000
+++ lib/lp/code/interfaces/linkedbranch.py	2010-08-10 03:38:54 +0000
@@ -12,7 +12,7 @@
 
 __metaclass__ = type
 __all__ = [
-    'get_linked_branch',
+    'get_linked_to_branch',
     'ICanHasLinkedBranch',
     ]
 
@@ -41,14 +41,14 @@
         """
 
 
-def get_linked_branch(provided):
-    """Get the linked branch for 'provided', whatever that is.
+def get_linked_to_branch(provided):
+    """Get the `ICanHasLinkedBranch` for 'provided', whatever that is.
 
     :raise CannotHaveLinkedBranch: If 'provided' can never have a linked
         branch.
     :raise NoLinkedBranch: If 'provided' could have a linked branch, but
         doesn't.
-    :return: The linked branch, an `IBranch`.
+    :return: The `ICanHasLinkedBranch` object.
     """
     has_linked_branch = ICanHasLinkedBranch(provided, None)
     if has_linked_branch is None:
@@ -57,7 +57,6 @@
             # pocket.
             provided = provided[0]
         raise CannotHaveLinkedBranch(provided)
-    branch = has_linked_branch.branch
-    if branch is None:
+    if has_linked_branch.branch is None:
         raise NoLinkedBranch(provided)
-    return branch
+    return has_linked_branch

=== modified file 'lib/lp/code/model/branchlookup.py'
--- lib/lp/code/model/branchlookup.py	2010-08-03 03:43:33 +0000
+++ lib/lp/code/model/branchlookup.py	2010-08-10 03:38:54 +0000
@@ -26,7 +26,7 @@
 from lp.code.interfaces.branchlookup import (
     IBranchLookup, ILinkedBranchTraversable, ILinkedBranchTraverser)
 from lp.code.interfaces.branchnamespace import IBranchNamespaceSet
-from lp.code.interfaces.linkedbranch import get_linked_branch
+from lp.code.interfaces.linkedbranch import get_linked_to_branch
 from lp.registry.interfaces.distribution import IDistribution
 from lp.registry.interfaces.distroseries import (
     IDistroSeries, IDistroSeriesSet, NoSuchDistroSeries)
@@ -357,7 +357,9 @@
             # If the first element doesn't start with a tilde, then maybe
             # 'path' is a shorthand notation for a branch.
             result = getUtility(ILinkedBranchTraverser).traverse(path)
-            branch = self._getLinkedBranch(result)
+            linked = self._getLinkedBranch(result)
+            branch = linked.branch
+            suffix = path[len(linked.bzr_path) + 1:]
         else:
             namespace_set = getUtility(IBranchNamespaceSet)
             segments = iter(path.lstrip('~').split('/'))
@@ -378,7 +380,7 @@
             doesn't.
         :return: The linked branch, an `IBranch`.
         """
-        branch = get_linked_branch(provided)
-        if not check_permission('launchpad.View', branch):
+        linked = get_linked_to_branch(provided)
+        if not check_permission('launchpad.View', linked.branch):
             raise NoLinkedBranch(provided)
-        return branch
+        return linked

=== modified file 'lib/lp/code/model/tests/test_branchlookup.py'
--- lib/lp/code/model/tests/test_branchlookup.py	2010-08-03 03:43:33 +0000
+++ lib/lp/code/model/tests/test_branchlookup.py	2010-08-10 03:38:54 +0000
@@ -538,7 +538,7 @@
             registrant,
             ICanHasLinkedBranch(distro_package).setBranch, branch, registrant)
         self.assertEqual(
-            (branch, None),
+            (branch, u''),
             self.branch_lookup.getByLPPath(
                 '%s/%s' % (
                     distro_package.distribution.name,
@@ -636,7 +636,7 @@
         series.branch = branch
         result = self.branch_lookup.getByLPPath(
             '%s/%s/other/bits' % (series.product.name, series.name))
-        self.assertEqual((branch, None), result)
+        self.assertEqual((branch, u'other/bits'), result)
 
     def test_too_long_sourcepackage(self):
         # If the provided path points to an existing source package with a
@@ -654,7 +654,7 @@
             ubuntu_branches.teamowner)
         result = self.branch_lookup.getByLPPath(
             '%s/other/bits' % package.path)
-        self.assertEqual((branch, None), result)
+        self.assertEqual((branch, u'other/bits'), result)
 
 
 def test_suite():

=== modified file 'lib/lp/code/model/tests/test_linkedbranch.py'
--- lib/lp/code/model/tests/test_linkedbranch.py	2010-07-20 17:50:45 +0000
+++ lib/lp/code/model/tests/test_linkedbranch.py	2010-08-10 03:38:54 +0000
@@ -14,7 +14,7 @@
 from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
 from canonical.testing.layers import DatabaseFunctionalLayer
 from lp.code.interfaces.linkedbranch import (
-    CannotHaveLinkedBranch, get_linked_branch, ICanHasLinkedBranch)
+    CannotHaveLinkedBranch, get_linked_to_branch, ICanHasLinkedBranch)
 from lp.registry.interfaces.distroseries import NoSuchDistroSeries
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.testing import run_with_login, TestCaseWithFactory
@@ -75,6 +75,13 @@
         ICanHasLinkedBranch(product).setBranch(branch)
         self.assertEqual(branch, product.development_focus.branch)
 
+    def test_get_linked_to_branch(self):
+        branch = self.factory.makeProductBranch()
+        product = removeSecurityProxy(branch.product)
+        ICanHasLinkedBranch(product).setBranch(branch)
+        got_linkable = get_linked_to_branch(product)
+        self.assertEqual(got_linkable, ICanHasLinkedBranch(product))
+
     def test_bzr_path(self):
         # The bzr_path of a product linked branch is the product name.
         product = self.factory.makeProduct()
@@ -203,7 +210,7 @@
         # ProjectGroups cannot have linked branches.
         project = self.factory.makeProject()
         self.assertRaises(
-            CannotHaveLinkedBranch, get_linked_branch, project)
+            CannotHaveLinkedBranch, get_linked_to_branch, project)
 
 
 class TestLinkedBranchSorting(TestCaseWithFactory):

=== modified file 'lib/lp/codehosting/vfs/branchfsclient.py'
--- lib/lp/codehosting/vfs/branchfsclient.py	2010-04-20 04:39:31 +0000
+++ lib/lp/codehosting/vfs/branchfsclient.py	2010-08-10 03:38:54 +0000
@@ -95,6 +95,8 @@
         for object_path, value in self._cache.iteritems():
             transport_type, data, inserted_time = value
             split_object_path = object_path.strip('/').split('/')
+            # Do a segment-by-segment comparison. Python sucks, lists should
+            # also have startswith.
             if split_path[:len(split_object_path)] == split_object_path:
                 if (self.expiry_time is not None
                     and self._now() > inserted_time + self.expiry_time):

=== modified file 'lib/lp/testing/tests/test_factory.py'
--- lib/lp/testing/tests/test_factory.py	2010-08-03 01:28:16 +0000
+++ lib/lp/testing/tests/test_factory.py	2010-08-10 03:38:54 +0000
@@ -13,6 +13,7 @@
 from zope.security.proxy import removeSecurityProxy
 
 from canonical.launchpad.webapp.interfaces import ILaunchBag
+<<<<<<< TREE
 from canonical.testing.layers import (
     DatabaseFunctionalLayer, LaunchpadZopelessLayer)
 from lp.buildmaster.interfaces.buildbase import BuildStatus
@@ -28,6 +29,10 @@
 from lp.soyuz.interfaces.publishing import (
     IBinaryPackagePublishingHistory, ISourcePackagePublishingHistory,
     PackagePublishingPriority, PackagePublishingStatus)
+=======
+from canonical.testing.layers import DatabaseFunctionalLayer
+from lp.code.enums import BranchType, CodeImportReviewStatus
+>>>>>>> MERGE-SOURCE
 from lp.testing import TestCaseWithFactory
 from lp.testing.factory import is_security_proxied_or_harmless
 from lp.testing.matchers import IsProxied, Provides, ProvidesAndIsProxied
@@ -37,6 +42,7 @@
 
     layer = DatabaseFunctionalLayer
 
+<<<<<<< TREE
     # loginAsAnyone
     def test_loginAsAnyone(self):
         # Login as anyone logs you in as any user.
@@ -116,6 +122,14 @@
         self.assertThat(bpr, ProvidesAndIsProxied(IBinaryPackageRelease))
 
     # makeCodeImport
+=======
+    def test_makeBranch_initialMirrorRequest(self):
+        # The default 'next_mirror_time' for a newly created hosted branch
+        # should be None.
+        branch = self.factory.makeAnyBranch(branch_type=BranchType.HOSTED)
+        self.assertIs(None, branch.next_mirror_time)
+
+>>>>>>> MERGE-SOURCE
     def test_makeCodeImportNoStatus(self):
         # If makeCodeImport is not given a review status, it defaults to NEW.
         code_import = self.factory.makeCodeImport()