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)
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
