launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #25646
[Merge] ~cjwatson/launchpad:distribution-traversal into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:distribution-traversal into launchpad:master.
Commit message:
Add configurable distribution default traversal rules
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/393731
We can use redirect_default_traversal to configure whether the canonical URL for a distroseries is (e.g.) /ubuntu/focal or /ubuntu/+series/focal. In the future we'll also be able to use default_traversal_policy to configure the default traversal for a distribution to mean something else: for example it might well make sense to use it for distribution source packages rather than for series.
DB MP: https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/393730
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:distribution-traversal into launchpad:master.
diff --git a/lib/lp/registry/browser/configure.zcml b/lib/lp/registry/browser/configure.zcml
index a718640..777652f 100644
--- a/lib/lp/registry/browser/configure.zcml
+++ b/lib/lp/registry/browser/configure.zcml
@@ -99,8 +99,7 @@
/>
<browser:url
for="lp.registry.interfaces.distroseries.IDistroSeries"
- path_expression="name"
- attribute_to_parent="distribution"
+ urldata="lp.registry.browser.distroseries.DistroSeriesURL"
/>
<browser:defaultView
name="+index"
diff --git a/lib/lp/registry/browser/distribution.py b/lib/lp/registry/browser/distribution.py
index 8d566e0..04d6dc1 100644
--- a/lib/lp/registry/browser/distribution.py
+++ b/lib/lp/registry/browser/distribution.py
@@ -95,6 +95,7 @@ from lp.registry.browser.pillar import (
PillarNavigationMixin,
PillarViewMixin,
)
+from lp.registry.enums import DistributionDefaultTraversalPolicy
from lp.registry.interfaces.distribution import (
IDistribution,
IDistributionMirrorMenuMarker,
@@ -189,13 +190,28 @@ class DistributionNavigation(
@stepthrough('+series')
def traverse_series(self, name):
- series, _ = self._resolveSeries(name)
- return self.redirectSubTree(
- canonical_url(series, request=self.request), status=303)
+ series, redirect = self._resolveSeries(name)
+ if not redirect:
+ policy = self.context.default_traversal_policy
+ if (policy == DistributionDefaultTraversalPolicy.SERIES and
+ not self.context.redirect_default_traversal):
+ redirect = True
+ if redirect:
+ return self.redirectSubTree(
+ canonical_url(series, request=self.request), status=303)
+ else:
+ return series
def traverse(self, name):
- series, is_alias = self._resolveSeries(name)
- if is_alias:
+ series, redirect = self._resolveSeries(name)
+ if series 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:
return self.redirectSubTree(
canonical_url(series, request=self.request), status=303)
else:
@@ -965,6 +981,8 @@ class DistributionEditView(RegistryEditFormView,
'translations_usage',
'answers_usage',
'translation_focus',
+ 'default_traversal_policy',
+ 'redirect_default_traversal',
]
custom_widget_icon = CustomWidgetFactory(
diff --git a/lib/lp/registry/browser/distroseries.py b/lib/lp/registry/browser/distroseries.py
index 89114b9..75c0468 100644
--- a/lib/lp/registry/browser/distroseries.py
+++ b/lib/lp/registry/browser/distroseries.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2020 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""View classes related to `IDistroSeries`."""
@@ -17,6 +17,7 @@ __all__ = [
'DistroSeriesPackageSearchView',
'DistroSeriesPackagesView',
'DistroSeriesUniquePackagesView',
+ 'DistroSeriesURL',
'DistroSeriesView',
]
@@ -27,7 +28,10 @@ from zope.component import getUtility
from zope.event import notify
from zope.formlib import form
from zope.formlib.widget import CustomWidgetFactory
-from zope.interface import Interface
+from zope.interface import (
+ implementer,
+ Interface,
+ )
from zope.lifecycleevent import ObjectCreatedEvent
from zope.schema import (
Choice,
@@ -67,6 +71,7 @@ from lp.registry.browser import (
MilestoneOverlayMixin,
)
from lp.registry.enums import (
+ DistributionDefaultTraversalPolicy,
DistroSeriesDifferenceStatus,
DistroSeriesDifferenceType,
)
@@ -86,6 +91,7 @@ from lp.services.webapp.authorization import check_permission
from lp.services.webapp.batching import BatchNavigator
from lp.services.webapp.breadcrumb import Breadcrumb
from lp.services.webapp.escaping import structured
+from lp.services.webapp.interfaces import ICanonicalUrlData
from lp.services.webapp.menu import (
ApplicationMenu,
enabled_with_permission,
@@ -129,6 +135,34 @@ def get_dsd_source():
return getUtility(IDistroSeriesDifferenceSource)
+@implementer(ICanonicalUrlData)
+class DistroSeriesURL:
+ """Distro series URL creation rules.
+
+ The canonical URL for a distro series 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.SERIES and
+ not self.context.distribution.redirect_default_traversal):
+ return self.context.name
+ else:
+ return u"+series/%s" % self.context.name
+
+
class DistroSeriesNavigation(GetitemNavigation, BugTargetTraversalMixin,
StructuralSubscriptionTargetTraversalMixin):
diff --git a/lib/lp/registry/browser/tests/distribution-views.txt b/lib/lp/registry/browser/tests/distribution-views.txt
index 810f118..533d559 100644
--- a/lib/lp/registry/browser/tests/distribution-views.txt
+++ b/lib/lp/registry/browser/tests/distribution-views.txt
@@ -132,7 +132,8 @@ The view accepts most of the distribution fields.
'package_derivatives_email', 'icon',
'logo', 'mugshot', 'official_malone', 'enable_bug_expiration',
'blueprints_usage', 'translations_usage', 'answers_usage',
- 'translation_focus']
+ 'translation_focus',
+ 'default_traversal_policy', 'redirect_default_traversal']
>>> del form['field.name']
>>> del form['field.actions.save']
diff --git a/lib/lp/registry/browser/tests/test_distribution.py b/lib/lp/registry/browser/tests/test_distribution.py
index d47eb99..b92d7b2 100644
--- a/lib/lp/registry/browser/tests/test_distribution.py
+++ b/lib/lp/registry/browser/tests/test_distribution.py
@@ -72,6 +72,42 @@ class TestDistributionNavigation(TestCaseWithFactory):
"http://launchpad.test/%s/%s"; % (
distroseries.distribution.name, distroseries.name))
+ def test_classic_series_url_redirects(self):
+ distroseries = self.factory.makeDistroSeries()
+ distroseries.distribution.redirect_default_traversal = True
+ self.assertRedirects(
+ "http://launchpad.test/%s/%s"; % (
+ distroseries.distribution.name, distroseries.name),
+ "http://launchpad.test/%s/+series/%s"; % (
+ distroseries.distribution.name, distroseries.name))
+
+ def test_classic_series_url_with_alias_redirects(self):
+ distroseries = self.factory.makeDistroSeries()
+ distroseries.distribution.redirect_default_traversal = True
+ distroseries.distribution.development_series_alias = "devel"
+ self.assertRedirects(
+ "http://launchpad.test/%s/devel"; % distroseries.distribution.name,
+ "http://launchpad.test/%s/+series/%s"; % (
+ distroseries.distribution.name, distroseries.name))
+
+ def test_new_series_url(self):
+ distroseries = self.factory.makeDistroSeries()
+ distroseries.distribution.redirect_default_traversal = True
+ obj, _, _ = test_traverse(
+ "http://launchpad.test/%s/+series/%s"; % (
+ distroseries.distribution.name, distroseries.name))
+ self.assertEqual(distroseries, obj)
+
+ def test_new_series_url_with_alias(self):
+ distroseries = self.factory.makeDistroSeries()
+ distroseries.distribution.redirect_default_traversal = True
+ distroseries.distribution.development_series_alias = "devel"
+ self.assertRedirects(
+ "http://launchpad.test/%s/+series/devel"; % (
+ distroseries.distribution.name),
+ "http://launchpad.test/%s/+series/%s"; % (
+ distroseries.distribution.name, distroseries.name))
+
def test_new_series_url_redirects(self):
distroseries = self.factory.makeDistroSeries()
self.assertRedirects(
@@ -97,6 +133,54 @@ class TestDistributionNavigation(TestCaseWithFactory):
self.assertIsInstance(marshaller.dereference_url(url), RedirectionView)
self.assertEqual(expected_obj, marshaller.marshall_from_json_data(url))
+ def test_classic_series_url_supports_object_lookup(self):
+ # Classic series URLs (without +series) are compatible with
+ # webservice object lookup, even if the distribution is configured
+ # to redirect the default traversal.
+ distroseries = self.factory.makeDistroSeries()
+ distroseries.distribution.redirect_default_traversal = True
+ distroseries_url = "/%s/%s" % (
+ distroseries.distribution.name, distroseries.name)
+ self.assertDereferences(distroseries_url, distroseries)
+
+ # Objects subordinate to the redirected series work too.
+ distroarchseries = self.factory.makeDistroArchSeries(
+ distroseries=distroseries)
+ distroarchseries_url = "/%s/%s/%s" % (
+ distroarchseries.distroseries.distribution.name,
+ distroarchseries.distroseries.name,
+ distroarchseries.architecturetag)
+ self.assertDereferences(distroarchseries_url, distroarchseries)
+
+ def test_classic_series_url_supports_object_lookup_https(self):
+ # Classic series URLs (without +series) are compatible with
+ # webservice object lookup, even if the distribution is configured
+ # to redirect the default traversal and the vhost is configured to
+ # use HTTPS. "SERVER_URL": None exposes a bug in lazr.restful <
+ # 0.22.2.
+ self.addCleanup(allvhosts.reload)
+ self.pushConfig("vhosts", use_https=True)
+ allvhosts.reload()
+
+ distroseries = self.factory.makeDistroSeries()
+ distroseries.distribution.redirect_default_traversal = True
+ distroseries_url = "/%s/%s" % (
+ distroseries.distribution.name, distroseries.name)
+ self.assertDereferences(
+ distroseries_url, distroseries,
+ environ={"HTTPS": "on", "SERVER_URL": None})
+
+ # Objects subordinate to the redirected series work too.
+ distroarchseries = self.factory.makeDistroArchSeries(
+ distroseries=distroseries)
+ distroarchseries_url = "/%s/%s/%s" % (
+ distroarchseries.distroseries.distribution.name,
+ distroarchseries.distroseries.name,
+ distroarchseries.architecturetag)
+ self.assertDereferences(
+ distroarchseries_url, distroarchseries,
+ environ={"HTTPS": "on", "SERVER_URL": None})
+
def test_new_series_url_supports_object_lookup(self):
# New-style +series URLs are compatible with webservice object
# lookup.
diff --git a/lib/lp/registry/configure.zcml b/lib/lp/registry/configure.zcml
index 1c73f4b..659b8d9 100644
--- a/lib/lp/registry/configure.zcml
+++ b/lib/lp/registry/configure.zcml
@@ -1820,6 +1820,7 @@
set_attributes="
bug_reported_acknowledgement
bug_reporting_guidelines
+ default_traversal_policy
description
development_series_alias
display_name
@@ -1837,6 +1838,7 @@
official_malone
owner
package_derivatives_email
+ redirect_default_traversal
redirect_release_uploads
summary
vcs
diff --git a/lib/lp/registry/enums.py b/lib/lp/registry/enums.py
index 631ba6f..30f4a79 100644
--- a/lib/lp/registry/enums.py
+++ b/lib/lp/registry/enums.py
@@ -1,4 +1,4 @@
-# Copyright 2010-2015 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2020 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Enums for the Registry app."""
@@ -7,6 +7,7 @@ __metaclass__ = type
__all__ = [
'BranchSharingPolicy',
'BugSharingPolicy',
+ 'DistributionDefaultTraversalPolicy',
'DistroSeriesDifferenceStatus',
'DistroSeriesDifferenceType',
'EXCLUSIVE_TEAM_POLICY',
@@ -425,3 +426,19 @@ class VCSType(DBEnumeratedType):
The Git DVCS is used as the default project or distribution VCS.
""")
+
+
+class DistributionDefaultTraversalPolicy(DBEnumeratedType):
+ """Policy for the default traversal from a distribution.
+
+ This determines what the "name" segment in a URL such as
+ "/{distro}/{name}" (with no intervening segment such as "+source")
+ means.
+ """
+
+ SERIES = DBItem(0, """
+ Series
+
+ The default traversal from a distribution is used for series of that
+ distribution.
+ """)
diff --git a/lib/lp/registry/interfaces/distribution.py b/lib/lp/registry/interfaces/distribution.py
index ecd6c49..8c23b90 100644
--- a/lib/lp/registry/interfaces/distribution.py
+++ b/lib/lp/registry/interfaces/distribution.py
@@ -73,7 +73,10 @@ from lp.bugs.interfaces.bugtarget import (
from lp.bugs.interfaces.structuralsubscription import (
IStructuralSubscriptionTarget,
)
-from lp.registry.enums import VCSType
+from lp.registry.enums import (
+ DistributionDefaultTraversalPolicy,
+ VCSType,
+ )
from lp.registry.interfaces.announcement import IMakesAnnouncements
from lp.registry.interfaces.distributionmirror import IDistributionMirror
from lp.registry.interfaces.distroseries import DistroSeriesNameField
@@ -390,6 +393,20 @@ class IDistributionPublic(
description=_(
"Version control system for this distribution's code.")))
+ default_traversal_policy = exported(Choice(
+ title=_("Default traversal policy"),
+ description=_(
+ "The type of object that /{distro}/{name} URLs for this "
+ "distribution resolve to."),
+ vocabulary=DistributionDefaultTraversalPolicy,
+ readonly=False, required=False))
+ redirect_default_traversal = exported(Bool(
+ title=_("Redirect the default traversal"),
+ description=_(
+ "If true, the default traversal is for migration and redirects "
+ "to a different canonical URL."),
+ readonly=False, required=False))
+
def getArchiveIDList(archive=None):
"""Return a list of archive IDs suitable for sqlvalues() or quote().
diff --git a/lib/lp/registry/model/distribution.py b/lib/lp/registry/model/distribution.py
index b9baba7..ba9b432 100644
--- a/lib/lp/registry/model/distribution.py
+++ b/lib/lp/registry/model/distribution.py
@@ -88,6 +88,7 @@ from lp.code.interfaces.seriessourcepackagebranch import (
from lp.registry.enums import (
BranchSharingPolicy,
BugSharingPolicy,
+ DistributionDefaultTraversalPolicy,
SpecificationSharingPolicy,
VCSType,
)
@@ -262,6 +263,10 @@ class Distribution(SQLBase, BugTargetBase, MakesAnnouncements,
redirect_release_uploads = BoolCol(notNull=True, default=False)
development_series_alias = StringCol(notNull=False, default=None)
vcs = EnumCol(enum=VCSType, notNull=False)
+ default_traversal_policy = EnumCol(
+ enum=DistributionDefaultTraversalPolicy, notNull=False,
+ default=DistributionDefaultTraversalPolicy.SERIES)
+ redirect_default_traversal = BoolCol(notNull=False, default=False)
def __repr__(self):
display_name = self.display_name.encode('ASCII', 'backslashreplace')
diff --git a/lib/lp/registry/tests/test_distribution.py b/lib/lp/registry/tests/test_distribution.py
index 9eb671d..a053738 100644
--- a/lib/lp/registry/tests/test_distribution.py
+++ b/lib/lp/registry/tests/test_distribution.py
@@ -31,6 +31,7 @@ from lp.app.interfaces.launchpad import ILaunchpadCelebrities
from lp.registry.enums import (
BranchSharingPolicy,
BugSharingPolicy,
+ DistributionDefaultTraversalPolicy,
EXCLUSIVE_TEAM_POLICY,
INCLUSIVE_TEAM_POLICY,
)
@@ -345,6 +346,31 @@ class TestDistribution(TestCaseWithFactory):
self.assertEqual(1, result.count())
self.assertEqual(first_project, result[0])
+ def test_default_traversal(self):
+ # By default, a distribution's default traversal refers to its
+ # series.
+ distro = self.factory.makeDistribution()
+ self.assertEqual(
+ DistributionDefaultTraversalPolicy.SERIES,
+ distro.default_traversal_policy)
+ self.assertFalse(distro.redirect_default_traversal)
+
+ def test_default_traversal_permissions(self):
+ # Only distribution owners can change the default traversal
+ # behaviour.
+ distro = self.factory.makeDistribution()
+ with person_logged_in(self.factory.makePerson()):
+ self.assertRaises(
+ Unauthorized, setattr, distro, 'default_traversal_policy',
+ DistributionDefaultTraversalPolicy.SERIES)
+ self.assertRaises(
+ Unauthorized, setattr, distro, 'redirect_default_traversal',
+ True)
+ with person_logged_in(distro.owner):
+ distro.default_traversal_policy = (
+ DistributionDefaultTraversalPolicy.SERIES)
+ distro.redirect_default_traversal = True
+
class TestDistributionCurrentSourceReleases(
CurrentSourceReleasesMixin, TestCase):
