Launchpad itself

Merge lp:~gary/launchpad/bug589010 into lp:launchpad

  1. bug589010
  2. Merge into devel
Proposed by Gary Poster
Status: Merged
Approved by: Curtis Hovey
Approved revision: no longer in the source branch.
Merged at revision: 10958
Proposed branch: lp:~gary/launchpad/bug589010
Merge into: lp:launchpad
Diff against target: 144 lines (+126/-0)
3 files modified
lib/canonical/launchpad/configure.zcml (+5/-0)
lib/canonical/launchpad/webapp/initialization.py (+57/-0)
lib/canonical/launchpad/webapp/tests/test_initialization.py (+64/-0)
To merge this branch: bzr merge lp:~gary/launchpad/bug589010
Reviewer Review Type Date Requested Status
Curtis Hovey (community) Approve
Review via email: mp+26842@code.launchpad.net

Commit message

hide namespace adapters from view lookup to fix bug 589010.

Description of the change

The purpose of this branch, and its approach, is best described in the description to bug 589010.

To try to summarize, registering a Zope URI namespace adapter effectively registers a view. This is not desired behavior. It is a failure in the framework, but fixing the framework properly is significantly backwards incompatible. Here, I make registrations that essentially hide namespace adapters from view lookup.

To run tests, use ./bin/test -vvt TestURLNamespace

To QA, create a project named "oops" or "resource" or "form" or "view" and try to view it in the browser.

Lint seems happy.

Thank you.

To post a comment you must log in.
Revision history for this message
Curtis Hovey (sinzui) wrote :

This is clever. I will speak to you about launchpad.conf shortly after this review.

This is good to land after you make some trivial changes.

There should be two blank lines before each module member in initialization.py

test_initialization.py doc string is not PEP 256 friendly

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/canonical/launchpad/configure.zcml'
--- lib/canonical/launchpad/configure.zcml 2010-04-27 21:36:59 +0000
+++ lib/canonical/launchpad/configure.zcml 2010-06-04 21:48:33 +0000
@@ -94,4 +94,9 @@
94 module="canonical.launchpad.browser"94 module="canonical.launchpad.browser"
95 classes="LaunchpadRootNavigation"95 classes="LaunchpadRootNavigation"
96 />96 />
97
98 <!-- Register a handler to fix things up just before the application
99 starts (and after zcml has been processed). -->
100 <subscriber handler=".webapp.initialization.handle_process_start" />
101
97</configure>102</configure>
98103
=== added file 'lib/canonical/launchpad/webapp/initialization.py'
--- lib/canonical/launchpad/webapp/initialization.py 1970-01-01 00:00:00 +0000
+++ lib/canonical/launchpad/webapp/initialization.py 2010-06-04 21:48:33 +0000
@@ -0,0 +1,57 @@
1# Copyright 2010 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).
3
4"""Initializes the application after ZCML has been processed."""
5
6from zope.interface import implementer, Interface
7from zope.component import adapter, getSiteManager
8from zope.processlifetime import IDatabaseOpened
9from zope.publisher.interfaces import IRequest
10from zope.publisher.interfaces.browser import IBrowserRequest
11from zope.publisher.interfaces.http import IHTTPRequest
12from zope.traversing.interfaces import ITraversable
13
14
15@implementer(Interface)
16def adapter_mask(*args):
17 return None
18
19
20@adapter(IDatabaseOpened)
21def handle_process_start(ev):
22 """Post-process ZCML configuration.
23
24 Normal configuration should happen in ZCML (or whatever our Zope
25 configuration standard might become in the future). The only kind
26 of configuration that should happen here is automated fix-up
27 configuration. Code below should call functions, each of which explains
28 why it cannot be performed in ZCML.
29
30 Also see the lp_sitecustomize module for initialization that is done when
31 Python first starts.
32 """
33 fix_up_namespace_traversers()
34
35
36def fix_up_namespace_traversers():
37 """Block namespace traversers from being found as normal views.
38
39 See bug 589010.
40
41 This is done in a function rather than in ZCML because automation is
42 appropriate: there has already been an explicit registration of the
43 namespace, and having to also say "please don't assume it is a view"
44 is a DRY violation that we can avoid.
45 """
46 sm = getSiteManager()
47 info = 'see %s.fix_up_namespace_traversers' % (__name__,)
48 namespace_factories = sm.adapters.lookupAll(
49 (Interface, IBrowserRequest), ITraversable)
50 for request_iface in (Interface, IRequest, IHTTPRequest, IBrowserRequest):
51 for name, factory in namespace_factories:
52 current = sm.adapters.lookup(
53 (Interface, request_iface), Interface, name)
54 if current is factory:
55 sm.registerAdapter(
56 adapter_mask,
57 required=(Interface, request_iface), name=name, info=info)
058
=== added file 'lib/canonical/launchpad/webapp/tests/test_initialization.py'
--- lib/canonical/launchpad/webapp/tests/test_initialization.py 1970-01-01 00:00:00 +0000
+++ lib/canonical/launchpad/webapp/tests/test_initialization.py 2010-06-04 21:48:33 +0000
@@ -0,0 +1,64 @@
1# Copyright 2010 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).
3
4"""Tests post-zcml application initialization.
5
6As found in canonical.launchpad.webapp.initialization.py."""
7
8import unittest
9
10from lp.testing import TestCase
11from zope.interface import Interface
12from zope.component import getSiteManager
13from zope.publisher.interfaces.browser import IBrowserRequest
14from zope.traversing.interfaces import ITraversable
15
16from canonical.launchpad.webapp.errorlog import OopsNamespace
17from canonical.launchpad.webapp.servers import LaunchpadTestRequest
18from canonical.testing import FunctionalLayer
19
20
21class AnyObject:
22 pass
23
24
25class TestURLNamespace(TestCase):
26
27 layer = FunctionalLayer
28
29 def setUp(self):
30 TestCase.setUp(self)
31 self.sm = getSiteManager()
32 self.context = AnyObject()
33 self.request = LaunchpadTestRequest()
34
35 def test_oops_namespace_not_view(self):
36 # The ++oops++ namespace should not be available as a "oops" view.
37 # First, we will verify that it is available as a namespace.
38 namespace = self.sm.getMultiAdapter(
39 (self.context, self.request), ITraversable, 'oops')
40 self.failUnless(isinstance(namespace, OopsNamespace))
41 # However, it is not available as a view.
42 not_a_namespace = self.sm.queryMultiAdapter(
43 (self.context, self.request), Interface, 'oops')
44 self.failIf(isinstance(not_a_namespace, OopsNamespace))
45
46 def test_no_namespaces_are_views(self):
47 # This tests an abstract superset of test_oops_namespace_not_view.
48 # At the time of writing, namespaces were 'resource', 'oops', 'form',
49 # and 'view'.
50 namespace_info = self.sm.adapters.lookupAll(
51 (Interface, IBrowserRequest), ITraversable)
52 for name, factory in namespace_info:
53 try:
54 not_the_namespace_factory = self.sm.adapters.lookup(
55 (Interface, IBrowserRequest), Interface, name)
56 except LookupError:
57 pass
58 else:
59 self.assertNotEqual(factory, not_the_namespace_factory)
60
61
62def test_suite():
63 suite = unittest.TestLoader().loadTestsFromName(__name__)
64 return suite