Merge lp:~thumper/launchpad/branch-target-adapters into lp:launchpad

[Tim Penhey]

Add a few more adapters to IBranchTarget.

As part of the on-going work to make the smart server understand the short lp: style urls for private branches, I need some extra branch target adapters. This branch adds the adapters for IProductSeries -> IBranchTarget, and IDistributionSourcePackage -> IBranchTarget.

tests:
  test_.*_adapter

[Jonathan Lange (jml)]

On Tue, Aug 3, 2010 at 10:33 PM, Tim Penhey <email address hidden> wrote:
> Tim Penhey has proposed merging lp:~thumper/launchpad/branch-target-adapters into lp:launchpad/devel.
...
> As part of the on-going work to make the smart server understand the short lp: style urls for private branches, I need some extra branch target adapters.  This branch adds the adapters for IProductSeries -> IBranchTarget, and IDistributionSourcePackage -> IBranchTarget.

I'm curious. This wasn't a part of the work we did at the Epic... why
is this needed?

jml

[Tim Penhey (thumper)]

On Wed, 04 Aug 2010 09:41:13 you wrote:
> On Tue, Aug 3, 2010 at 10:33 PM, Tim Penhey <email address hidden>
wrote:
> > Tim Penhey has proposed merging
> > lp:~thumper/launchpad/branch-target-adapters into lp:launchpad/devel.
>
> ...
>
> > As part of the on-going work to make the smart server understand the
> > short lp: style urls for private branches, I need some extra branch
> > target adapters. This branch adds the adapters for IProductSeries ->
> > IBranchTarget, and IDistributionSourcePackage -> IBranchTarget.
>
> I'm curious. This wasn't a part of the work we did at the Epic... why
> is this needed?
>
> jml

It is part of that "minor" bit that was left :-)

It makes the code that allows the branch creation to work when given a short
name, and the ability to set the linked branch.

We use the ILinkedBranchTraverser to get one of:
  Product, ProductSeries, SourcePackage, DistributionSourcePackage

We then adapt that to an IBranchTarget. We then use the target to get the
IBranchNamespace for the requester. Then we can check branch names to make
sure there isn't a conflict.

The the guts of the reason is to go from something that can have a linked
branch to a branch namespace we go through a branch target.

Tim

[Michael Hudson-Doyle (mwhudson)]

Looks fine. The tests could use comments. I don't like the use of self.original (a product?) in test_productseries_adapter.

review: Approve

=== modified file 'lib/lp/code/configure.zcml'
--- lib/lp/code/configure.zcml	2010-07-25 12:55:56 +0000
+++ lib/lp/code/configure.zcml	2010-08-10 22:53:44 +0000
@@ -606,6 +606,11 @@
       for="lp.registry.interfaces.sourcepackage.ISourcePackage"
       provides="lp.code.interfaces.branchtarget.IBranchTarget"
       factory="lp.code.model.branchtarget.PackageBranchTarget"/>
+  <adapter
+      for="lp.registry.interfaces.distributionsourcepackage.IDistributionSourcePackage"
+      provides="lp.code.interfaces.branchtarget.IBranchTarget"
+      factory="lp.code.model.branchtarget.distribution_sourcepackage_to_branch_target"/>
+
   <class class="lp.code.model.branchtarget.PersonBranchTarget">
     <allow interface="lp.code.interfaces.branchtarget.IBranchTarget"/>
   </class>
@@ -613,6 +618,7 @@
       for="lp.registry.interfaces.person.IPerson"
       provides="lp.code.interfaces.branchtarget.IBranchTarget"
       factory="lp.code.model.branchtarget.PersonBranchTarget"/>
+
   <class class="lp.code.model.branchtarget.ProductBranchTarget">
     <allow interface="lp.code.interfaces.branchtarget.IBranchTarget"/>
   </class>
@@ -621,6 +627,10 @@
       provides="lp.code.interfaces.branchtarget.IBranchTarget"
       factory="lp.code.model.branchtarget.ProductBranchTarget"/>
   <adapter
+      for="lp.registry.interfaces.productseries.IProductSeries"
+      provides="lp.code.interfaces.branchtarget.IBranchTarget"
+      factory="lp.code.model.branchtarget.product_series_to_branch_target"/>
+  <adapter
       for="lp.code.interfaces.branchtarget.IBranchTarget"
       provides="canonical.launchpad.webapp.interfaces.ICanonicalUrlData"
       factory="lp.code.model.branchtarget.get_canonical_url_data_for_target"/>

=== modified file 'lib/lp/code/interfaces/branchtarget.py'
--- lib/lp/code/interfaces/branchtarget.py	2010-04-23 04:11:12 +0000
+++ lib/lp/code/interfaces/branchtarget.py	2010-08-10 22:53:44 +0000
@@ -60,13 +60,15 @@
     target = Attribute("The branch target, as an `IBranchTarget`.")
 
 
-class IBranchTarget(IPrimaryContext):
+class IBranchTarget(Interface):
     """A target of branches.
 
     A product contains branches, a source package on a distroseries contains
     branches, and a person contains 'junk' branches.
     """
 
+    context = Attribute('The primary context.')
+
     name = Attribute("The name of the target.")
 
     components = Attribute(

=== modified file 'lib/lp/code/model/branchtarget.py'
--- lib/lp/code/model/branchtarget.py	2010-04-14 17:44:00 +0000
+++ lib/lp/code/model/branchtarget.py	2010-08-10 22:53:44 +0000
@@ -340,3 +340,17 @@
 def get_canonical_url_data_for_target(branch_target):
     """Return the `ICanonicalUrlData` for an `IBranchTarget`."""
     return ICanonicalUrlData(branch_target.context)
+
+
+def product_series_to_branch_target(product_series):
+    """The Product itself is the branch target given a ProductSeries."""
+    return ProductBranchTarget(product_series.product)
+
+def distribution_sourcepackage_to_branch_target(distro_sourcepackage):
+    """The development version of the distro sourcepackage is the target."""
+    dev_version = distro_sourcepackage.development_version
+    # It is possible for distributions to not have any series, and if that is
+    # the case, the dev_version is None.
+    if dev_version is None:
+        return None
+    return PackageBranchTarget(dev_version)

=== modified file 'lib/lp/code/model/tests/test_branchtarget.py'
--- lib/lp/code/model/tests/test_branchtarget.py	2010-07-28 04:18:53 +0000
+++ lib/lp/code/model/tests/test_branchtarget.py	2010-08-10 22:53:44 +0000
@@ -26,8 +26,8 @@
 
 class BaseBranchTargetTests:
 
-    def test_provides_IPrimaryContext(self):
-        self.assertProvides(self.target, IPrimaryContext)
+    def test_provides_IBranchTarget(self):
+        self.assertProvides(self.target, IBranchTarget)
 
     def test_context(self):
         # IBranchTarget.context is the original object.
@@ -93,6 +93,21 @@
         target = IBranchTarget(self.original)
         self.assertIsInstance(target, PackageBranchTarget)
 
+    def test_distrosourcepackage_adapter(self):
+        # Adapting a distrosourcepackage will make a branch target with the
+        # current series of the distro as the distroseries.
+        distro = self.original.distribution
+        distro_sourcepackage = distro.getSourcePackage(
+            self.original.sourcepackagename)
+        target = IBranchTarget(distro_sourcepackage)
+        self.assertIsInstance(target, PackageBranchTarget)
+        self.assertEqual(
+            [distro, distro.currentseries],
+            target.components[:2])
+        self.assertEqual(
+            self.original.sourcepackagename,
+            target.components[2].sourcepackagename)
+
     def test_components(self):
         target = IBranchTarget(self.original)
         self.assertEqual(
@@ -320,6 +335,14 @@
         target = IBranchTarget(self.original)
         self.assertIsInstance(target, ProductBranchTarget)
 
+    def test_productseries_adapter(self):
+        # Adapting a product series will make a product branch target.
+        product = self.factory.makeProduct()
+        series = self.factory.makeProductSeries(product)
+        target = IBranchTarget(series)
+        self.assertIsInstance(target, ProductBranchTarget)
+        self.assertEqual([product], target.components)
+
     def test_components(self):
         target = IBranchTarget(self.original)
         self.assertEqual([self.original], list(target.components))