launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:buildmaster-api-redirects into launchpad:master.

Commit message:
Send proper webservice redirects for builds

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/392364

launchpadlib handles this correctly nowadays, but only if we send redirects to the correct target host (e.g. api.launchpad.net rather than launchpad.net).
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:buildmaster-api-redirects into launchpad:master.

diff --git a/lib/lp/buildmaster/browser/builder.py b/lib/lp/buildmaster/browser/builder.py
index 7f8b9b9..1c67f11 100644
--- a/lib/lp/buildmaster/browser/builder.py
+++ b/lib/lp/buildmaster/browser/builder.py
@@ -80,28 +80,28 @@ class BuilderSetNavigation(GetitemNavigation):
         build = get_build_by_id_str(IBinaryPackageBuildSet, name)
         if build is None:
             return None
-        return self.redirectSubTree(canonical_url(build))
+        return self.redirectSubTree(canonical_url(build, request=self.request))
 
     @stepthrough('+recipebuild')
     def traverse_recipebuild(self, name):
         build = get_build_by_id_str(ISourcePackageRecipeBuildSource, name)
         if build is None:
             return None
-        return self.redirectSubTree(canonical_url(build))
+        return self.redirectSubTree(canonical_url(build, request=self.request))
 
     @stepthrough('+livefsbuild')
     def traverse_livefsbuild(self, name):
         build = get_build_by_id_str(ILiveFSBuildSet, name)
         if build is None:
             return None
-        return self.redirectSubTree(canonical_url(build))
+        return self.redirectSubTree(canonical_url(build, request=self.request))
 
     @stepthrough('+snapbuild')
     def traverse_snapbuild(self, name):
         build = get_build_by_id_str(ISnapBuildSet, name)
         if build is None:
             return None
-        return self.redirectSubTree(canonical_url(build))
+        return self.redirectSubTree(canonical_url(build, request=self.request))
 
 
 class BuilderSetBreadcrumb(Breadcrumb):
diff --git a/lib/lp/buildmaster/browser/tests/test_builder.py b/lib/lp/buildmaster/browser/tests/test_builder.py
index 018d7f1..86c798b 100644
--- a/lib/lp/buildmaster/browser/tests/test_builder.py
+++ b/lib/lp/buildmaster/browser/tests/test_builder.py
@@ -22,17 +22,25 @@ from lp.buildmaster.interfaces.builder import IBuilderSet
 from lp.buildmaster.model.builder import Builder
 from lp.services.database.interfaces import IStore
 from lp.services.database.sqlbase import get_transaction_timestamp
+from lp.services.features.testing import FeatureFixture
 from lp.services.job.model.job import Job
+from lp.services.webapp.publisher import canonical_url
+from lp.soyuz.interfaces.livefs import LIVEFS_FEATURE_FLAG
 from lp.testing import (
     admin_logged_in,
+    logout,
     record_two_runs,
     TestCaseWithFactory,
     )
-from lp.testing.layers import LaunchpadFunctionalLayer
+from lp.testing.layers import (
+    DatabaseFunctionalLayer,
+    LaunchpadFunctionalLayer,
+    )
 from lp.testing.matchers import HasQueryCount
 from lp.testing.pages import (
     extract_text,
     find_tags_by_class,
+    setupBrowser,
     )
 from lp.testing.views import create_initialized_view
 
@@ -42,6 +50,62 @@ def builders_homepage_render():
     return create_initialized_view(builders, "+index").render()
 
 
+class TestBuilderSetNavigation(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_binary_package_build_api_redirects(self):
+        build = self.factory.makeBinaryPackageBuild()
+        url = "http://api.launchpad.test/devel/builders/+build/%s"; % build.id
+        expected_url = (
+            "http://api.launchpad.test/devel"; +
+            canonical_url(build, path_only_if_possible=True))
+        logout()
+        browser = setupBrowser()
+        browser.open(url)
+        self.assertEqual(expected_url, browser.url)
+
+    def test_source_package_recipe_build_api_redirects(self):
+        build = self.factory.makeSourcePackageRecipeBuild()
+        url = (
+            "http://api.launchpad.test/devel/builders/+recipebuild/%s"; %
+            build.id)
+        expected_url = (
+            "http://api.launchpad.test/devel"; +
+            canonical_url(build, path_only_if_possible=True))
+        logout()
+        browser = setupBrowser()
+        browser.open(url)
+        self.assertEqual(expected_url, browser.url)
+
+    def test_livefs_build_api_redirects(self):
+        self.useFixture(FeatureFixture({LIVEFS_FEATURE_FLAG: "on"}))
+        build = self.factory.makeLiveFSBuild()
+        url = (
+            "http://api.launchpad.test/devel/builders/+livefsbuild/%s"; %
+            build.id)
+        expected_url = (
+            "http://api.launchpad.test/devel"; +
+            canonical_url(build, path_only_if_possible=True))
+        logout()
+        browser = setupBrowser()
+        browser.open(url)
+        self.assertEqual(expected_url, browser.url)
+
+    def test_snap_build_api_redirects(self):
+        build = self.factory.makeSnapBuild()
+        url = (
+            "http://api.launchpad.test/devel/builders/+snapbuild/%s"; %
+            build.id)
+        expected_url = (
+            "http://api.launchpad.test/devel"; +
+            canonical_url(build, path_only_if_possible=True))
+        logout()
+        browser = setupBrowser()
+        browser.open(url)
+        self.assertEqual(expected_url, browser.url)
+
+
 class TestBuildersHomepage(TestCaseWithFactory, BuildCreationMixin):
 
     layer = LaunchpadFunctionalLayer