← Back to team overview

launchpad-reviewers team mailing list archive

[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"