Merge ~twom/launchpad:stats-fix-valueerror-on-redirect into launchpad:master

[Tom Wardill]

Fix setting pageids in a redirect

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.
Check for RedirectionView on generation and allow the existing methods that deal with not having a context to DTRT.

f804ccb... by Tom Wardill on 2020-10-01

Check for RedirectionView

[Colin Watson (cjwatson)]

LGTM - could you update the MP description as well please?

review: Approve

c222314... by Tom Wardill on 2020-10-01

Fix imports

diff --git a/lib/lp/services/webapp/publication.py b/lib/lp/services/webapp/publication.py
index 3b86566..6324a64 100644
--- a/lib/lp/services/webapp/publication.py
+++ b/lib/lp/services/webapp/publication.py
@@ -53,7 +53,10 @@ from zope.publisher.interfaces.browser import (
     )
 from zope.publisher.publish import mapply
 from zope.security.management import newInteraction
-from zope.security.proxy import removeSecurityProxy
+from zope.security.proxy import (
+    isinstance as zope_isinstance,
+    removeSecurityProxy,
+    )
 from zope.traversing.interfaces import BeforeTraverseEvent
 
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
@@ -85,6 +88,7 @@ from lp.services.webapp.interfaces import (
     OffsiteFormPostError,
     )
 from lp.services.webapp.opstats import OpStats
+from lp.services.webapp.publisher import RedirectionView
 from lp.services.webapp.vhosts import allvhosts
 
 
@@ -564,7 +568,10 @@ 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))
+        if zope_isinstance(view, RedirectionView):
+            context = None
+        else:
+            context = removeSecurityProxy(getattr(view, '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)