launchpad-reviewers team mailing list archive

[Tom Wardill <tom.wardill@xxxxxxxxxxxxx>]

Tom Wardill has proposed merging ~twom/launchpad:stats-fix-valueerror-on-redirect into launchpad:master.

Commit message:
Fix setting pageids in a redirect

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

Redirects don't have a view context. With the move to generating pageid at traversal rather than call time, we are now catching some redirects in this flow.
Catch the ValueError on attempting to access them, and allow the existing methods that deal with not having a context to DTRT.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~twom/launchpad:stats-fix-valueerror-on-redirect into launchpad:master.

diff --git a/lib/lp/services/webapp/publication.py b/lib/lp/services/webapp/publication.py
index 3b86566..d66d575 100644
--- a/lib/lp/services/webapp/publication.py
+++ b/lib/lp/services/webapp/publication.py
@@ -564,7 +564,11 @@ class LaunchpadBrowserPublication(
         view = removeSecurityProxy(view)
         # It's possible that the view is a bound method.
         view = getattr(view, '__self__', view)
-        context = removeSecurityProxy(getattr(view, 'context', None))
+        try:
+            context = removeSecurityProxy(getattr(view, 'context', None))
+        except ValueError:
+            # Redirects don't have a view context
+            context = None
         pageid = self.constructPageID(view, context)
         request.setInWSGIEnvironment('launchpad.pageid', pageid)
         return pageid
diff --git a/lib/lp/services/webapp/tests/test_publication.py b/lib/lp/services/webapp/tests/test_publication.py
index 9bc4089..245fc28 100644
--- a/lib/lp/services/webapp/tests/test_publication.py
+++ b/lib/lp/services/webapp/tests/test_publication.py
@@ -388,3 +388,17 @@ class TestPublisherStats(StatsMixin, TestCaseWithFactory):
         self.assertEqual(
             'RootObject-index-html',
             publication._prepPageIDForMetrics("RootObject:index.html"))
+
+    def test_no_context_pageid(self):
+        # request context may not exist in redirect scenarios
+        owner = self.factory.makePerson()
+        ppa = self.factory.makeArchive(owner=owner)
+        redirect_url = ("http://launchpad.test/api/devel/~{}/";
+                        "+archive/{}/testpackage".format(owner.name, ppa.name))
+        self.useFixture(FakeLogger())
+        browser = self.getUserBrowser()
+        # This shouldn't raise ValueError
+        self.assertRaises(
+            NotFound,
+            browser.open,
+            redirect_url)