Merge ~cjwatson/launchpad:more-distribution-traversal-policies into launchpad:master

[Colin Watson]

Add distro traversal policies for source packages and OCI projects

These allow distributions to be configured so that their default traversal resolves to a DistributionSourcePackage or to an OCIProject, depending on the kinds of structures that the distribution most commonly contains.

[Thiago F. Pappacena (pappacena)]

LGTM

review: Approve

diff --git a/lib/lp/registry/browser/configure.zcml b/lib/lp/registry/browser/configure.zcml
index 777652f..7c97dc5 100644
--- a/lib/lp/registry/browser/configure.zcml
+++ b/lib/lp/registry/browser/configure.zcml
@@ -543,8 +543,7 @@
         />
     <browser:url
         for="lp.registry.interfaces.distributionsourcepackage.IDistributionSourcePackage"
-        path_expression="string:+source/${name}"
-        attribute_to_parent="distribution"
+        urldata="lp.registry.browser.distributionsourcepackage.DistributionSourcePackageURL"
         />
     <browser:navigation
         module="lp.registry.browser.distributionsourcepackage"
@@ -607,8 +606,7 @@
         />
     <browser:url
         for="lp.registry.interfaces.ociproject.IOCIProject"
-        path_expression="string:+oci/${name}"
-        attribute_to_parent="pillar"
+        urldata="lp.registry.browser.ociproject.OCIProjectURL"
         />
     <browser:url
         for="lp.registry.interfaces.ociprojectseries.IOCIProjectSeries"
diff --git a/lib/lp/registry/browser/distribution.py b/lib/lp/registry/browser/distribution.py
index 04d6dc1..e401ab8 100644
--- a/lib/lp/registry/browser/distribution.py
+++ b/lib/lp/registry/browser/distribution.py
@@ -159,11 +159,25 @@ class DistributionNavigation(
 
     @stepthrough('+source')
     def traverse_sources(self, name):
-        return self.context.getSourcePackage(name)
+        dsp = self.context.getSourcePackage(name)
+        policy = self.context.default_traversal_policy
+        if (policy == DistributionDefaultTraversalPolicy.SOURCE_PACKAGE and
+                not self.context.redirect_default_traversal):
+            return self.redirectSubTree(
+                canonical_url(dsp, request=self.request), status=303)
+        else:
+            return dsp
 
     @stepthrough('+oci')
     def traverse_oci(self, name):
-        return self.context.getOCIProject(name)
+        oci_project = self.context.getOCIProject(name)
+        policy = self.context.default_traversal_policy
+        if (policy == DistributionDefaultTraversalPolicy.OCI_PROJECT and
+                not self.context.redirect_default_traversal):
+            return self.redirectSubTree(
+                canonical_url(oci_project, request=self.request), status=303)
+        else:
+            return oci_project
 
     @stepthrough('+milestone')
     def traverse_milestone(self, name):
@@ -203,19 +217,25 @@ class DistributionNavigation(
             return series
 
     def traverse(self, name):
-        series, redirect = self._resolveSeries(name)
-        if series is None:
+        policy = self.context.default_traversal_policy
+        if policy == DistributionDefaultTraversalPolicy.SERIES:
+            obj, redirect = self._resolveSeries(name)
+        elif policy == DistributionDefaultTraversalPolicy.SOURCE_PACKAGE:
+            obj = self.context.getSourcePackage(name)
+            redirect = False
+        elif policy == DistributionDefaultTraversalPolicy.OCI_PROJECT:
+            obj = self.context.getOCIProject(name)
+            redirect = False
+        else:
+            raise AssertionError(
+                "Unknown default traversal policy %r" % policy)
+        if obj is None:
             return None
-        if not redirect:
-            policy = self.context.default_traversal_policy
-            if (policy == DistributionDefaultTraversalPolicy.SERIES and
-                    self.context.redirect_default_traversal):
-                redirect = True
-        if redirect:
+        if redirect or self.context.redirect_default_traversal:
             return self.redirectSubTree(
-                canonical_url(series, request=self.request), status=303)
+                canonical_url(obj, request=self.request), status=303)
         else:
-            return series
+            return obj
 
 
 class DistributionSetNavigation(Navigation):
diff --git a/lib/lp/registry/browser/distributionsourcepackage.py b/lib/lp/registry/browser/distributionsourcepackage.py
index d70467b..df3cd55 100644
--- a/lib/lp/registry/browser/distributionsourcepackage.py
+++ b/lib/lp/registry/browser/distributionsourcepackage.py
@@ -13,6 +13,7 @@ __all__ = [
     'DistributionSourcePackageNavigation',
     'DistributionSourcePackageOverviewMenu',
     'DistributionSourcePackagePublishingHistoryView',
+    'DistributionSourcePackageURL',
     'DistributionSourcePackageView',
     'PublishingHistoryViewMixin',
     ]
@@ -53,6 +54,7 @@ from lp.bugs.interfaces.bugtasksearch import BugTaskSearchParams
 from lp.code.browser.vcslisting import TargetDefaultVCSNavigationMixin
 from lp.registry.browser import add_subscribe_link
 from lp.registry.browser.pillar import PillarBugsMenu
+from lp.registry.enums import DistributionDefaultTraversalPolicy
 from lp.registry.interfaces.distributionsourcepackage import (
     IDistributionSourcePackage,
     )
@@ -69,7 +71,10 @@ from lp.services.webapp import (
     )
 from lp.services.webapp.batching import BatchNavigator
 from lp.services.webapp.breadcrumb import Breadcrumb
-from lp.services.webapp.interfaces import IMultiFacetedBreadcrumb
+from lp.services.webapp.interfaces import (
+    ICanonicalUrlData,
+    IMultiFacetedBreadcrumb,
+    )
 from lp.services.webapp.menu import (
     ApplicationMenu,
     enabled_with_permission,
@@ -89,6 +94,34 @@ from lp.translations.browser.customlanguagecode import (
     )
 
 
+@implementer(ICanonicalUrlData)
+class DistributionSourcePackageURL:
+    """Distribution source package URL creation rules.
+
+    The canonical URL for a distribution source package depends on the
+    values of `default_traversal_policy` and `redirect_default_traversal` on
+    the context distribution.
+    """
+
+    rootsite = None
+
+    def __init__(self, context):
+        self.context = context
+
+    @property
+    def inside(self):
+        return self.context.distribution
+
+    @property
+    def path(self):
+        policy = self.context.distribution.default_traversal_policy
+        if (policy == DistributionDefaultTraversalPolicy.SOURCE_PACKAGE and
+                not self.context.distribution.redirect_default_traversal):
+            return self.context.name
+        else:
+            return u"+source/%s" % self.context.name
+
+
 class DistributionSourcePackageFormatterAPI(CustomizableFormatter):
     """Adapt IDistributionSourcePackage objects to a formatted string."""
 
diff --git a/lib/lp/registry/browser/ociproject.py b/lib/lp/registry/browser/ociproject.py
index 0d23a15..fe5f8b4 100644
--- a/lib/lp/registry/browser/ociproject.py
+++ b/lib/lp/registry/browser/ociproject.py
@@ -12,6 +12,7 @@ __all__ = [
     'OCIProjectFacets',
     'OCIProjectNavigation',
     'OCIProjectNavigationMenu',
+    'OCIProjectURL',
     ]
 
 from zope.component import getUtility
@@ -28,6 +29,7 @@ from lp.app.browser.tales import CustomizableFormatter
 from lp.app.errors import NotFoundError
 from lp.code.browser.vcslisting import TargetDefaultVCSNavigationMixin
 from lp.oci.interfaces.ocirecipe import IOCIRecipeSet
+from lp.registry.enums import DistributionDefaultTraversalPolicy
 from lp.registry.interfaces.distribution import IDistribution
 from lp.registry.interfaces.ociproject import (
     IOCIProject,
@@ -55,7 +57,38 @@ from lp.services.webapp import (
     )
 from lp.services.webapp.batching import BatchNavigator
 from lp.services.webapp.breadcrumb import Breadcrumb
-from lp.services.webapp.interfaces import IMultiFacetedBreadcrumb
+from lp.services.webapp.interfaces import (
+    ICanonicalUrlData,
+    IMultiFacetedBreadcrumb,
+    )
+
+
+@implementer(ICanonicalUrlData)
+class OCIProjectURL:
+    """OCI project URL creation rules.
+
+    The canonical URL for an OCI project in a distribution depends on the
+    values of `default_traversal_policy` and `redirect_default_traversal` on
+    the context distribution.
+    """
+
+    rootsite = None
+
+    def __init__(self, context):
+        self.context = context
+
+    @property
+    def inside(self):
+        return self.context.pillar
+
+    @property
+    def path(self):
+        if self.context.distribution is not None:
+            policy = self.context.distribution.default_traversal_policy
+            if (policy == DistributionDefaultTraversalPolicy.OCI_PROJECT and
+                    not self.context.distribution.redirect_default_traversal):
+                return self.context.name
+        return u"+oci/%s" % self.context.name
 
 
 def getPillarFieldName(pillar):
diff --git a/lib/lp/registry/browser/tests/test_distribution.py b/lib/lp/registry/browser/tests/test_distribution.py
index b92d7b2..d465e16 100644
--- a/lib/lp/registry/browser/tests/test_distribution.py
+++ b/lib/lp/registry/browser/tests/test_distribution.py
@@ -25,7 +25,10 @@ from zope.schema.vocabulary import SimpleVocabulary
 from zope.security.proxy import removeSecurityProxy
 
 from lp.app.browser.lazrjs import vocabulary_to_choice_edit_items
-from lp.registry.enums import EXCLUSIVE_TEAM_POLICY
+from lp.registry.enums import (
+    DistributionDefaultTraversalPolicy,
+    EXCLUSIVE_TEAM_POLICY,
+    )
 from lp.registry.interfaces.distroseries import IDistroSeries
 from lp.registry.interfaces.ociproject import OCI_PROJECT_ALLOW_CREATE
 from lp.registry.interfaces.series import SeriesStatus
@@ -224,6 +227,105 @@ class TestDistributionNavigation(TestCaseWithFactory):
             distroarchseries_url, distroarchseries,
             environ={"HTTPS": "on", "SERVER_URL": None})
 
+    def test_short_source_url(self):
+        dsp = self.factory.makeDistributionSourcePackage()
+        dsp.distribution.default_traversal_policy = (
+            DistributionDefaultTraversalPolicy.SOURCE_PACKAGE)
+        obj, _, _ = test_traverse(
+            "http://launchpad.test/%s/%s" % (
+                dsp.distribution.name, dsp.name))
+        self.assertEqual(dsp, obj)
+
+    def test_short_source_url_redirects(self):
+        dsp = self.factory.makeDistributionSourcePackage()
+        dsp.distribution.default_traversal_policy = (
+            DistributionDefaultTraversalPolicy.SOURCE_PACKAGE)
+        dsp.distribution.redirect_default_traversal = True
+        self.assertRedirects(
+            "http://launchpad.test/%s/%s" % (
+                dsp.distribution.name, dsp.name),
+            "http://launchpad.test/%s/+source/%s" % (
+                dsp.distribution.name, dsp.name))
+
+    def test_long_non_default_source_url(self):
+        dsp = self.factory.makeDistributionSourcePackage()
+        obj, _, _ = test_traverse(
+            "http://launchpad.test/%s/+source/%s" % (
+                dsp.distribution.name, dsp.name))
+        self.assertEqual(dsp, obj)
+
+    def test_long_default_source_url(self):
+        dsp = self.factory.makeDistributionSourcePackage()
+        dsp.distribution.default_traversal_policy = (
+            DistributionDefaultTraversalPolicy.SOURCE_PACKAGE)
+        dsp.distribution.redirect_default_traversal = True
+        obj, _, _ = test_traverse(
+            "http://launchpad.test/%s/+source/%s" % (
+                dsp.distribution.name, dsp.name))
+        self.assertEqual(dsp, obj)
+
+    def test_long_default_source_url_redirects(self):
+        dsp = self.factory.makeDistributionSourcePackage()
+        dsp.distribution.default_traversal_policy = (
+            DistributionDefaultTraversalPolicy.SOURCE_PACKAGE)
+        self.assertRedirects(
+            "http://launchpad.test/%s/+source/%s" % (
+                dsp.distribution.name, dsp.name),
+            "http://launchpad.test/%s/%s" % (
+                dsp.distribution.name, dsp.name))
+
+    def test_short_oci_url(self):
+        oci_project = self.factory.makeOCIProject(
+            pillar=self.factory.makeDistribution())
+        oci_project.distribution.default_traversal_policy = (
+            DistributionDefaultTraversalPolicy.OCI_PROJECT)
+        obj, _, _ = test_traverse(
+            "http://launchpad.test/%s/%s" % (
+                oci_project.distribution.name, oci_project.name))
+        self.assertEqual(oci_project, obj)
+
+    def test_short_oci_url_redirects(self):
+        oci_project = self.factory.makeOCIProject(
+            pillar=self.factory.makeDistribution())
+        oci_project.distribution.default_traversal_policy = (
+            DistributionDefaultTraversalPolicy.OCI_PROJECT)
+        oci_project.distribution.redirect_default_traversal = True
+        self.assertRedirects(
+            "http://launchpad.test/%s/%s" % (
+                oci_project.distribution.name, oci_project.name),
+            "http://launchpad.test/%s/+oci/%s" % (
+                oci_project.distribution.name, oci_project.name))
+
+    def test_long_non_default_oci_url(self):
+        oci_project = self.factory.makeOCIProject(
+            pillar=self.factory.makeDistribution())
+        obj, _, _ = test_traverse(
+            "http://launchpad.test/%s/+oci/%s" % (
+                oci_project.distribution.name, oci_project.name))
+        self.assertEqual(oci_project, obj)
+
+    def test_long_default_oci_url(self):
+        oci_project = self.factory.makeOCIProject(
+            pillar=self.factory.makeDistribution())
+        oci_project.distribution.default_traversal_policy = (
+            DistributionDefaultTraversalPolicy.OCI_PROJECT)
+        oci_project.distribution.redirect_default_traversal = True
+        obj, _, _ = test_traverse(
+            "http://launchpad.test/%s/+oci/%s" % (
+                oci_project.distribution.name, oci_project.name))
+        self.assertEqual(oci_project, obj)
+
+    def test_long_default_oci_url_redirects(self):
+        oci_project = self.factory.makeOCIProject(
+            pillar=self.factory.makeDistribution())
+        oci_project.distribution.default_traversal_policy = (
+            DistributionDefaultTraversalPolicy.OCI_PROJECT)
+        self.assertRedirects(
+            "http://launchpad.test/%s/+oci/%s" % (
+                oci_project.distribution.name, oci_project.name),
+            "http://launchpad.test/%s/%s" % (
+                oci_project.distribution.name, oci_project.name))
+
 
 class TestDistributionPage(TestCaseWithFactory):
     """A TestCase for the distribution index page."""
diff --git a/lib/lp/registry/enums.py b/lib/lp/registry/enums.py
index 30f4a79..b71dec6 100644
--- a/lib/lp/registry/enums.py
+++ b/lib/lp/registry/enums.py
@@ -442,3 +442,17 @@ class DistributionDefaultTraversalPolicy(DBEnumeratedType):
         The default traversal from a distribution is used for series of that
         distribution.
         """)
+
+    SOURCE_PACKAGE = DBItem(1, """
+        Source package
+
+        The default traversal from a distribution is used for source
+        packages in that distribution.
+        """)
+
+    OCI_PROJECT = DBItem(2, """
+        OCI project
+
+        The default traversal from a distribution is used for OCI projects
+        in that distribution.
+        """)