launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #00663
[Merge] lp:~thumper/launchpad/fix-canonical-url into lp:launchpad/devel
Tim Penhey has proposed merging lp:~thumper/launchpad/fix-canonical-url into lp:launchpad/devel.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#620247 canonical_url breaks given views on a different layer to the current request
https://bugs.launchpad.net/bugs/620247
This branch changes the way that canonical_url does the view_name check.
Currently it checks using the current browser request to define the layer rather than the layer that is defined by the canonical_url_data of the context object.
Instead we lookup the layer for the given rootsite and use that to query for the existence of the view.
Pre-impl call: mwhudson, and the mailing list thread: https://lists.launchpad.net/launchpad-dev/msg04345.html
Tests:
canonical_url
--
https://code.launchpad.net/~thumper/launchpad/fix-canonical-url/+merge/33079
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~thumper/launchpad/fix-canonical-url into lp:launchpad/devel.
=== modified file 'lib/canonical/launchpad/doc/canonical_url.txt'
--- lib/canonical/launchpad/doc/canonical_url.txt 2009-11-14 22:44:07 +0000
+++ lib/canonical/launchpad/doc/canonical_url.txt 2010-08-19 03:17:51 +0000
@@ -28,11 +28,11 @@
Add a view for Country/+map.
>>> from zope.component import provideAdapter
- >>> from zope.publisher.interfaces.http import IHTTPApplicationRequest
+ >>> from zope.publisher.interfaces.browser import IDefaultBrowserLayer
>>> class CountryMapView(object):
... def __init__(self, country, request):
... pass
- >>> provideAdapter(CountryMapView, (ICountry, IHTTPApplicationRequest),
+ >>> provideAdapter(CountryMapView, (ICountry, IDefaultBrowserLayer),
... name='+map', provides=Interface)
Define a navigation for the Country URL.
@@ -56,7 +56,7 @@
... town.name = 'Greenwich'
... return town
>>> provideAdapter(
- ... CountryNavigation, [ICountry, IHTTPApplicationRequest],
+ ... CountryNavigation, [ICountry, IDefaultBrowserLayer],
... IBrowserPublisher)
Put the interfaces into canonical.launchpad.ftests, ensuring that there
@@ -352,6 +352,7 @@
getApplicationURL() from
zope.publisher.interfaces.http.IHTTPApplicationRequest.
+ >>> from zope.publisher.interfaces.http import IHTTPApplicationRequest
>>> class FakeRequest:
...
... implements(IHTTPApplicationRequest)
=== modified file 'lib/canonical/launchpad/webapp/publisher.py'
--- lib/canonical/launchpad/webapp/publisher.py 2010-08-02 02:13:52 +0000
+++ lib/canonical/launchpad/webapp/publisher.py 2010-08-19 03:17:51 +0000
@@ -31,14 +31,22 @@
from zope.app.publisher.interfaces.xmlrpc import IXMLRPCView
from zope.app.publisher.xmlrpc import IMethodPublisher
from zope.component import getUtility, queryMultiAdapter
-from zope.interface import implements
+from zope.component.interfaces import ComponentLookupError
+from zope.interface import directlyProvides, implements
from zope.interface.advice import addClassAdvisor
from zope.publisher.interfaces import NotFound
-from zope.publisher.interfaces.browser import IBrowserPublisher
+from zope.publisher.interfaces.browser import (
+ IBrowserPublisher,
+ IDefaultBrowserLayer,
+ )
from zope.security.checker import ProxyFactory, NamesChecker
from zope.traversing.browser.interfaces import IAbsoluteURL
-from canonical.launchpad.layers import setFirstLayer, WebServiceLayer
+from canonical.launchpad.layers import (
+ LaunchpadLayer,
+ WebServiceLayer,
+ setFirstLayer,
+ )
from canonical.launchpad.webapp.vhosts import allvhosts
from canonical.launchpad.webapp.interfaces import (
ICanonicalUrlData, ILaunchBag, ILaunchpadApplication, ILaunchpadContainer,
@@ -408,6 +416,27 @@
__call__ = __str__
+def layer_for_rootsite(rootsite):
+ """Return the registered layer for the specified rootsite.
+
+ 'code' -> lp.code.publisher.CodeLayer
+ 'translations' -> lp.translations.publisher.TranslationsLayer
+ et al.
+
+ The layer is defined in ZCML using a named utility with the name of the
+ rootsite, and providing IDefaultBrowserLayer. If there is no utility
+ defined with the specified name, then LaunchpadLayer is returned.
+ """
+ try:
+ return getUtility(IDefaultBrowserLayer, rootsite)
+ except ComponentLookupError:
+ return LaunchpadLayer
+
+
+class FakeRequest:
+ """Used solely to provide a layer for the view check in canonical_url."""
+
+
def canonical_url(
obj, request=None, rootsite=None, path_only_if_possible=False,
view_name=None, force_local_path=False):
@@ -472,14 +501,14 @@
request = current_request
if view_name is not None:
- assert request is not None, (
- "Cannot check view_name parameter when the request is not "
- "available.")
-
+ # Make sure that the view is registered for the site requested.
+ fake_request = FakeRequest()
+ directlyProvides(fake_request, layer_for_rootsite(rootsite))
# Look first for a view.
- if queryMultiAdapter((obj, request), name=view_name) is None:
+ if queryMultiAdapter((obj, fake_request), name=view_name) is None:
# Look if this is a special name defined by Navigation.
- navigation = queryMultiAdapter((obj, request), IBrowserPublisher)
+ navigation = queryMultiAdapter(
+ (obj, fake_request), IBrowserPublisher)
if isinstance(navigation, Navigation):
all_names = navigation.all_traversal_and_redirection_names
else:
@@ -491,8 +520,6 @@
urlparts.insert(0, view_name)
if request is None:
- if rootsite is None:
- rootsite = 'mainsite'
root_url = allvhosts.configs[rootsite].rooturl
else:
root_url = request.getRootURL(rootsite)
=== modified file 'lib/lp/answers/configure.zcml'
--- lib/lp/answers/configure.zcml 2010-07-27 17:18:24 +0000
+++ lib/lp/answers/configure.zcml 2010-08-19 03:17:51 +0000
@@ -11,8 +11,12 @@
<include package=".browser" />
<publisher
- name="answers"
- factory="lp.answers.publisher.answers_request_publication_factory"/>
+ name="answers"
+ factory="lp.answers.publisher.answers_request_publication_factory"/>
+ <utility
+ component="lp.answers.publisher.AnswersLayer"
+ provides="zope.publisher.interfaces.browser.IDefaultBrowserLayer"
+ name="answers" />
<subscriber
for=".interfaces.question.IQuestion
=== modified file 'lib/lp/blueprints/configure.zcml'
--- lib/lp/blueprints/configure.zcml 2010-07-16 16:58:55 +0000
+++ lib/lp/blueprints/configure.zcml 2010-08-19 03:17:51 +0000
@@ -9,12 +9,17 @@
xmlns:xmlrpc="http://namespaces.zope.org/xmlrpc"
xmlns:lp="http://namespaces.canonical.com/lp"
i18n_domain="launchpad">
- <include
- package=".browser"/>
-
- <publisher
- name="blueprints"
- factory="lp.blueprints.publisher.blueprints_request_publication_factory"/>
+
+ <include package=".browser"/>
+
+ <publisher
+ name="blueprints"
+ factory="lp.blueprints.publisher.blueprints_request_publication_factory"/>
+ <utility
+ component="lp.blueprints.publisher.BlueprintsLayer"
+ provides="zope.publisher.interfaces.browser.IDefaultBrowserLayer"
+ name="blueprints" />
+
<lp:help-folder
folder="help" type="lp.blueprints.publisher.BlueprintsLayer" />
=== modified file 'lib/lp/bugs/configure.zcml'
--- lib/lp/bugs/configure.zcml 2010-08-02 17:48:13 +0000
+++ lib/lp/bugs/configure.zcml 2010-08-19 03:17:51 +0000
@@ -9,12 +9,16 @@
xmlns:xmlrpc="http://namespaces.zope.org/xmlrpc"
xmlns:lp="http://namespaces.canonical.com/lp"
i18n_domain="launchpad">
- <include
- package=".browser"/>
-
- <publisher
- name="bugs"
- factory="lp.bugs.publisher.bugs_request_publication_factory"/>
+
+ <include package=".browser"/>
+
+ <publisher
+ name="bugs"
+ factory="lp.bugs.publisher.bugs_request_publication_factory"/>
+ <utility
+ component="lp.bugs.publisher.BugsLayer"
+ provides="zope.publisher.interfaces.browser.IDefaultBrowserLayer"
+ name="bugs" />
<lp:help-folder
folder="help" type="lp.bugs.publisher.BugsLayer" />
=== modified file 'lib/lp/code/configure.zcml'
--- lib/lp/code/configure.zcml 2010-08-17 18:33:26 +0000
+++ lib/lp/code/configure.zcml 2010-08-19 03:17:51 +0000
@@ -14,8 +14,12 @@
<authorizations module="lp.code.security" />
<publisher
- name="code"
- factory="lp.code.publisher.code_request_publication_factory"/>
+ name="code"
+ factory="lp.code.publisher.code_request_publication_factory"/>
+ <utility
+ component="lp.code.publisher.CodeLayer"
+ provides="zope.publisher.interfaces.browser.IDefaultBrowserLayer"
+ name="code" />
<!-- Branch Merge Queue -->
=== modified file 'lib/lp/code/publisher.py'
--- lib/lp/code/publisher.py 2010-07-20 23:52:46 +0000
+++ lib/lp/code/publisher.py 2010-08-19 03:17:51 +0000
@@ -13,11 +13,15 @@
from zope.interface import implements
from zope.publisher.interfaces.browser import (
- IBrowserRequest, IDefaultBrowserLayer)
+ IBrowserRequest,
+ IDefaultBrowserLayer,
+ )
from canonical.launchpad.webapp.publication import LaunchpadBrowserPublication
from canonical.launchpad.webapp.servers import (
- LaunchpadBrowserRequest, VHostWebServiceRequestPublicationFactory)
+ LaunchpadBrowserRequest,
+ VHostWebServiceRequestPublicationFactory,
+ )
class CodeLayer(IBrowserRequest, IDefaultBrowserLayer):
=== modified file 'lib/lp/translations/configure.zcml'
--- lib/lp/translations/configure.zcml 2010-07-23 09:41:07 +0000
+++ lib/lp/translations/configure.zcml 2010-08-19 03:17:51 +0000
@@ -10,12 +10,16 @@
xmlns:webservice="http://namespaces.canonical.com/webservice"
xmlns:lp="http://namespaces.canonical.com/lp"
i18n_domain="launchpad">
- <include
- package=".browser"/>
-
- <publisher
- name="translations"
- factory="lp.translations.publisher.translations_request_publication_factory"/>
+
+ <include package=".browser"/>
+
+ <publisher
+ name="translations"
+ factory="lp.translations.publisher.translations_request_publication_factory"/>
+ <utility
+ component="lp.translations.publisher.TranslationsLayer"
+ provides="zope.publisher.interfaces.browser.IDefaultBrowserLayer"
+ name="translations" />
<lp:help-folder
folder="help" type="lp.translations.publisher.TranslationsLayer" />
=== modified file 'lib/lp/translations/tests/test_pofile.py'
--- lib/lp/translations/tests/test_pofile.py 2010-08-06 06:51:37 +0000
+++ lib/lp/translations/tests/test_pofile.py 2010-08-19 03:17:51 +0000
@@ -34,7 +34,7 @@
# Create a product with two series and a shared POTemplate
# in different series ('devel' and 'stable').
super(TestTranslationSharedPOFile, self).setUp()
- self.foo = self.factory.makeProduct()
+ self.foo = self.factory.makeProduct(name='foo')
self.foo_devel = self.factory.makeProductSeries(
name='devel', product=self.foo)
self.foo_stable = self.factory.makeProductSeries(
@@ -58,6 +58,15 @@
self.potmsgset = self.factory.makePOTMsgSet(self.devel_potemplate)
self.potmsgset.setSequence(self.devel_potemplate, 1)
+ def test_POFile_canonical_url(self):
+ # Test the canonical_url of the POFile.
+ self.assertEqual(
+ 'http://translations.launchpad.dev/foo/devel/+pots/messages/sr',
+ canonical_url(self.devel_sr_pofile))
+ self.assertEqual(
+ 'http://translations.launchpad.dev/foo/devel/+pots/messages/sr/+details',
+ canonical_url(self.devel_sr_pofile, view_name="+details"))
+
def test_findPOTMsgSetsContaining(self):
# Test that search works correctly.
=== modified file 'lib/lp/vostok/configure.zcml'
--- lib/lp/vostok/configure.zcml 2010-08-06 18:13:00 +0000
+++ lib/lp/vostok/configure.zcml 2010-08-19 03:17:51 +0000
@@ -8,10 +8,14 @@
<include package=".browser" />
<publisher
- name="vostok"
- factory="lp.vostok.publisher.vostok_request_publication_factory"
- methods="*"
- mimetypes="*" />
+ name="vostok"
+ factory="lp.vostok.publisher.vostok_request_publication_factory"
+ methods="*"
+ mimetypes="*" />
+ <utility
+ component="lp.vostok.publisher.VostokLayer"
+ provides="zope.publisher.interfaces.browser.IDefaultBrowserLayer"
+ name="vostok" />
<securedutility
class="lp.vostok.publisher.VostokRoot"