launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #25523
[Merge] ~cjwatson/launchpad:branch-repository-api-redirects into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:branch-repository-api-redirects into launchpad:master.
Commit message:
Send proper webservice redirects for branches and repositories
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/392544
launchpadlib handles this correctly nowadays, but only if we send redirects to the correct target host (e.g. api.launchpad.net rather than code.launchpad.net).
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:branch-repository-api-redirects into launchpad:master.
diff --git a/lib/lp/app/browser/launchpad.py b/lib/lp/app/browser/launchpad.py
index d31df20..eb4b9bb 100644
--- a/lib/lp/app/browser/launchpad.py
+++ b/lib/lp/app/browser/launchpad.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2019 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).
"""Browser code for the launchpad application."""
@@ -739,7 +739,7 @@ class LaunchpadRootNavigation(Navigation):
path = '/'.join(self.request.stepstogo)
try:
branch, trailing = getUtility(IBranchLookup).getByLPPath(path)
- target_url = canonical_url(branch)
+ target_url = canonical_url(branch, request=self.request)
if trailing != '':
target_url = urlappend(target_url, trailing)
except (NoLinkedBranch) as e:
@@ -789,7 +789,7 @@ class LaunchpadRootNavigation(Navigation):
try:
repository, trailing = getUtility(IGitLookup).getByPath(path)
if repository is not None:
- target_url = canonical_url(repository)
+ target_url = canonical_url(repository, request=self.request)
if trailing:
target_url = urlappend(target_url, trailing)
except (InvalidNamespace, InvalidProductName, NameLookupFailed,
@@ -802,7 +802,7 @@ class LaunchpadRootNavigation(Navigation):
# Attempt a bzr lookup as well:
try:
branch, trailing = getUtility(IBranchLookup).getByLPPath(path)
- bzr_url = canonical_url(branch)
+ bzr_url = canonical_url(branch, request=self.request)
if trailing != '':
bzr_url = urlappend(bzr_url, trailing)
diff --git a/lib/lp/app/browser/tests/test_launchpad.py b/lib/lp/app/browser/tests/test_launchpad.py
index a9c6adf..8339fc6 100644
--- a/lib/lp/app/browser/tests/test_launchpad.py
+++ b/lib/lp/app/browser/tests/test_launchpad.py
@@ -36,7 +36,10 @@ from lp.services.webapp.interfaces import (
BrowserNotificationLevel,
ILaunchpadRoot,
)
-from lp.services.webapp.servers import LaunchpadTestRequest
+from lp.services.webapp.servers import (
+ LaunchpadTestRequest,
+ WebServiceTestRequest,
+ )
from lp.services.webapp.url import urlappend
from lp.testing import (
admin_logged_in,
@@ -108,11 +111,12 @@ class TraversalMixin:
NotFound, self.traverse, path,
use_default_referer=use_default_referer)
- def assertRedirects(self, segments, url):
- redirection = self.traverse(segments)
+ def assertRedirects(self, segments, url, webservice=False):
+ redirection = self.traverse(segments, webservice=webservice)
self.assertEqual(url, redirection.target)
- def traverse(self, path, first_segment, use_default_referer=True):
+ def traverse(self, path, first_segment, use_default_referer=True,
+ webservice=False):
"""Traverse to 'path' using a 'LaunchpadRootNavigation' object.
Using the Zope traversal machinery, traverse to the path given by
@@ -132,7 +136,9 @@ class TraversalMixin:
extra = {'PATH_INFO': urlappend('/%s' % first_segment, path)}
if use_default_referer:
extra['HTTP_REFERER'] = DEFAULT_REFERER
- request = LaunchpadTestRequest(**extra)
+ request_factory = (
+ WebServiceTestRequest if webservice else LaunchpadTestRequest)
+ request = request_factory(**extra)
segments = reversed(path.split('/'))
request.setTraversalStack(segments)
traverser = LaunchpadRootNavigation(
@@ -168,6 +174,9 @@ class TestBranchTraversal(TestCaseWithFactory, TraversalMixin):
# branch.
branch = self.factory.makeAnyBranch()
self.assertRedirects(branch.unique_name, canonical_url(branch))
+ self.assertRedirects(
+ branch.unique_name, canonical_url(branch, rootsite='api'),
+ webservice=True)
def test_no_such_unique_name(self):
# Traversing to /+branch/<unique_name> where 'unique_name' is for a
@@ -346,10 +355,16 @@ class TestCodeTraversal(TestCaseWithFactory, TraversalMixin):
def test_project_bzr_branch(self):
branch = self.factory.makeAnyBranch()
self.assertRedirects(branch.unique_name, canonical_url(branch))
+ self.assertRedirects(
+ branch.unique_name, canonical_url(branch, rootsite='api'),
+ webservice=True)
def test_project_git_branch(self):
git_repo = self.factory.makeGitRepository()
self.assertRedirects(git_repo.unique_name, canonical_url(git_repo))
+ self.assertRedirects(
+ git_repo.unique_name, canonical_url(git_repo, rootsite='api'),
+ webservice=True)
def test_no_such_bzr_unique_name(self):
branch = self.factory.makeAnyBranch()
diff --git a/lib/lp/code/browser/tests/test_branchtraversal.py b/lib/lp/code/browser/tests/test_branchtraversal.py
index 98c9f75..c26a699 100644
--- a/lib/lp/code/browser/tests/test_branchtraversal.py
+++ b/lib/lp/code/browser/tests/test_branchtraversal.py
@@ -16,10 +16,11 @@ from lp.registry.interfaces.personproduct import (
IPersonProductFactory,
)
from lp.services.webapp.publisher import canonical_url
-from lp.testing import (
- FakeLaunchpadRequest,
- TestCaseWithFactory,
+from lp.services.webapp.servers import (
+ LaunchpadTestRequest,
+ WebServiceTestRequest,
)
+from lp.testing import TestCaseWithFactory
from lp.testing.layers import DatabaseFunctionalLayer
@@ -36,11 +37,11 @@ class TestPersonBranchTraversal(TestCaseWithFactory):
TestCaseWithFactory.setUp(self)
self.person = self.factory.makePerson()
- def assertRedirects(self, segments, url):
- redirection = self.traverse(segments)
+ def assertRedirects(self, segments, url, webservice=False):
+ redirection = self.traverse(segments, webservice=webservice)
self.assertEqual(url, redirection.target)
- def traverse(self, segments):
+ def traverse(self, segments, webservice=False):
"""Traverse to 'segments' using a 'PersonNavigation' object.
Using the Zope traversal machinery, traverse to the path given by
@@ -48,11 +49,16 @@ class TestPersonBranchTraversal(TestCaseWithFactory):
'person' attribute.
:param segments: A list of path segments.
+ :param webservice: If True, use a webservice-like request rather
+ than a normal test request.
:return: The object found.
"""
stack = list(reversed(segments))
name = stack.pop()
- request = FakeLaunchpadRequest(['~' + self.person.name], stack)
+ request_factory = (
+ WebServiceTestRequest if webservice else LaunchpadTestRequest)
+ request = request_factory()
+ request.setTraversalStack(stack)
traverser = PersonNavigation(self.person, request)
return traverser.publishTraverse(request, name)
@@ -60,11 +66,15 @@ class TestPersonBranchTraversal(TestCaseWithFactory):
branch = self.factory.makeProductBranch(owner=self.person)
segments = ['+branch', branch.product.name, branch.name]
self.assertRedirects(segments, canonical_url(branch))
+ self.assertRedirects(
+ segments, canonical_url(branch, rootsite='api'), webservice=True)
def test_redirect_junk_branch(self):
branch = self.factory.makePersonalBranch(owner=self.person)
segments = ['+branch', '+junk', branch.name]
self.assertRedirects(segments, canonical_url(branch))
+ self.assertRedirects(
+ segments, canonical_url(branch, rootsite='api'), webservice=True)
def test_redirect_branch_not_found(self):
self.assertRaises(
@@ -78,6 +88,10 @@ class TestPersonBranchTraversal(TestCaseWithFactory):
['foo', branch.distroseries.name, branch.sourcepackagename.name,
branch.name],
canonical_url(branch))
+ self.assertRedirects(
+ ['foo', branch.distroseries.name, branch.sourcepackagename.name,
+ branch.name],
+ canonical_url(branch, rootsite='api'), webservice=True)
def test_junk_branch(self):
branch = self.factory.makePersonalBranch(owner=self.person)
@@ -134,8 +148,8 @@ class TestPersonProductBranchTraversal(TestCaseWithFactory):
"""
stack = list(reversed(segments))
name = stack.pop()
- request = FakeLaunchpadRequest(
- ['~' + self.person.name, self.product.name], stack)
+ request = LaunchpadTestRequest()
+ request.setTraversalStack(stack)
traverser = PersonProductNavigation(self.person_product, request)
return traverser.publishTraverse(request, name)
diff --git a/lib/lp/registry/browser/person.py b/lib/lp/registry/browser/person.py
index facd4d2..193df25 100644
--- a/lib/lp/registry/browser/person.py
+++ b/lib/lp/registry/browser/person.py
@@ -364,7 +364,8 @@ class BranchTraversalMixin:
"""Redirect to canonical_url."""
branch = getUtility(IBranchNamespaceSet).traverse(self._getSegments())
if branch:
- return self.redirectSubTree(canonical_url(branch))
+ return self.redirectSubTree(
+ canonical_url(branch, request=self.request))
raise NotFoundError
@stepthrough('+git')
@@ -414,7 +415,8 @@ class BranchTraversalMixin:
if using_pillar_alias:
# This repository was accessed through one of its project's
# aliases, so we must redirect to its canonical URL.
- return self.redirectSubTree(canonical_url(repository))
+ return self.redirectSubTree(
+ canonical_url(repository, request=self.request))
return repository
except (NotFoundError, InvalidNamespace, InvalidProductName):
@@ -478,13 +480,15 @@ class BranchTraversalMixin:
if branch.product.name != pillar_name:
# This branch was accessed through one of its project's
# aliases, so we must redirect to its canonical URL.
- return self.redirectSubTree(canonical_url(branch))
+ return self.redirectSubTree(
+ canonical_url(branch, request=self.request))
if branch.distribution is not None:
if branch.distribution.name != pillar_name:
# This branch was accessed through one of its distribution's
# aliases, so we must redirect to its canonical URL.
- return self.redirectSubTree(canonical_url(branch))
+ return self.redirectSubTree(
+ canonical_url(branch, request=self.request))
return branch
diff --git a/lib/lp/registry/browser/tests/test_person.py b/lib/lp/registry/browser/tests/test_person.py
index 2008a61..07ef8d4 100644
--- a/lib/lp/registry/browser/tests/test_person.py
+++ b/lib/lp/registry/browser/tests/test_person.py
@@ -184,6 +184,32 @@ class TestPersonNavigation(TestCaseWithFactory):
repository.owner.name, project.name, repository.name)
self.assertEqual(repository, test_traverse(url)[0])
+ def test_traverse_git_repository_project_alias(self):
+ project = self.factory.makeProduct()
+ alias = project.name + "-2"
+ removeSecurityProxy(project).setAliases([alias])
+ repository = self.factory.makeGitRepository(target=project)
+ url = "/~%s/%s/+git/%s" % (
+ repository.owner.name, alias, repository.name)
+ expected_url = "http://code.launchpad.test/~%s/%s/+git/%s"; % (
+ repository.owner.name, project.name, repository.name)
+ _, view, _ = test_traverse(url)
+ self.assertIsInstance(view, RedirectionView)
+ self.assertEqual(expected_url, removeSecurityProxy(view).target)
+
+ def test_traverse_git_repository_project_alias_api(self):
+ project = self.factory.makeProduct()
+ alias = project.name + "-2"
+ removeSecurityProxy(project).setAliases([alias])
+ repository = self.factory.makeGitRepository(target=project)
+ url = "http://api.launchpad.test/devel/~%s/%s/+git/%s"; % (
+ repository.owner.name, alias, repository.name)
+ expected_url = "http://api.launchpad.test/devel/~%s/%s/+git/%s"; % (
+ repository.owner.name, project.name, repository.name)
+ _, view, _ = test_traverse(url)
+ self.assertIsInstance(view, RedirectionView)
+ self.assertEqual(expected_url, removeSecurityProxy(view).target)
+
def test_traverse_git_repository_package(self):
dsp = self.factory.makeDistributionSourcePackage()
repository = self.factory.makeGitRepository(target=dsp)
@@ -192,6 +218,38 @@ class TestPersonNavigation(TestCaseWithFactory):
dsp.sourcepackagename.name, repository.name)
self.assertEqual(repository, test_traverse(url)[0])
+ def test_traverse_git_repository_package_alias(self):
+ dsp = self.factory.makeDistributionSourcePackage()
+ alias = dsp.distribution.name + "-2"
+ removeSecurityProxy(dsp.distribution).setAliases([alias])
+ repository = self.factory.makeGitRepository(target=dsp)
+ url = "/~%s/%s/+source/%s/+git/%s" % (
+ repository.owner.name, alias,
+ dsp.sourcepackagename.name, repository.name)
+ expected_url = (
+ "http://code.launchpad.test/~%s/%s/+source/%s/+git/%s"; % (
+ repository.owner.name, dsp.distribution.name,
+ dsp.sourcepackagename.name, repository.name))
+ _, view, _ = test_traverse(url)
+ self.assertIsInstance(view, RedirectionView)
+ self.assertEqual(expected_url, removeSecurityProxy(view).target)
+
+ def test_traverse_git_repository_package_alias_api(self):
+ dsp = self.factory.makeDistributionSourcePackage()
+ alias = dsp.distribution.name + "-2"
+ removeSecurityProxy(dsp.distribution).setAliases([alias])
+ repository = self.factory.makeGitRepository(target=dsp)
+ url = "http://api.launchpad.test/devel/~%s/%s/+source/%s/+git/%s"; % (
+ repository.owner.name, alias,
+ dsp.sourcepackagename.name, repository.name)
+ expected_url = (
+ "http://api.launchpad.test/devel/~%s/%s/+source/%s/+git/%s"; % (
+ repository.owner.name, dsp.distribution.name,
+ dsp.sourcepackagename.name, repository.name))
+ _, view, _ = test_traverse(url)
+ self.assertIsInstance(view, RedirectionView)
+ self.assertEqual(expected_url, removeSecurityProxy(view).target)
+
def test_traverse_git_repository_oci_project(self):
oci_project = self.factory.makeOCIProject()
repository = self.factory.makeGitRepository(target=oci_project)
