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
1=== modified file 'lib/canonical/launchpad/configure.zcml'
2--- lib/canonical/launchpad/configure.zcml 2010-04-27 21:36:59 +0000
3+++ lib/canonical/launchpad/configure.zcml 2010-06-04 21:48:33 +0000
4@@ -94,4 +94,9 @@
5 module="canonical.launchpad.browser"
6 classes="LaunchpadRootNavigation"
7 />
8+
9+ <!-- Register a handler to fix things up just before the application
10+ starts (and after zcml has been processed). -->
11+ <subscriber handler=".webapp.initialization.handle_process_start" />
12+
13 </configure>
14
15=== added file 'lib/canonical/launchpad/webapp/initialization.py'
16--- lib/canonical/launchpad/webapp/initialization.py 1970-01-01 00:00:00 +0000
17+++ lib/canonical/launchpad/webapp/initialization.py 2010-06-04 21:48:33 +0000
18@@ -0,0 +1,57 @@
19+# Copyright 2010 Canonical Ltd. This software is licensed under the
20+# GNU Affero General Public License version 3 (see the file LICENSE).
21+
22+"""Initializes the application after ZCML has been processed."""
23+
24+from zope.interface import implementer, Interface
25+from zope.component import adapter, getSiteManager
26+from zope.processlifetime import IDatabaseOpened
27+from zope.publisher.interfaces import IRequest
28+from zope.publisher.interfaces.browser import IBrowserRequest
29+from zope.publisher.interfaces.http import IHTTPRequest
30+from zope.traversing.interfaces import ITraversable
31+
32+
33+@implementer(Interface)
34+def adapter_mask(*args):
35+ return None
36+
37+
38+@adapter(IDatabaseOpened)
39+def handle_process_start(ev):
40+ """Post-process ZCML configuration.
41+
42+ Normal configuration should happen in ZCML (or whatever our Zope
43+ configuration standard might become in the future). The only kind
44+ of configuration that should happen here is automated fix-up
45+ configuration. Code below should call functions, each of which explains
46+ why it cannot be performed in ZCML.
47+
48+ Also see the lp_sitecustomize module for initialization that is done when
49+ Python first starts.
50+ """
51+ fix_up_namespace_traversers()
52+
53+
54+def fix_up_namespace_traversers():
55+ """Block namespace traversers from being found as normal views.
56+
57+ See bug 589010.
58+
59+ This is done in a function rather than in ZCML because automation is
60+ appropriate: there has already been an explicit registration of the
61+ namespace, and having to also say "please don't assume it is a view"
62+ is a DRY violation that we can avoid.
63+ """
64+ sm = getSiteManager()
65+ info = 'see %s.fix_up_namespace_traversers' % (__name__,)
66+ namespace_factories = sm.adapters.lookupAll(
67+ (Interface, IBrowserRequest), ITraversable)
68+ for request_iface in (Interface, IRequest, IHTTPRequest, IBrowserRequest):
69+ for name, factory in namespace_factories:
70+ current = sm.adapters.lookup(
71+ (Interface, request_iface), Interface, name)
72+ if current is factory:
73+ sm.registerAdapter(
74+ adapter_mask,
75+ required=(Interface, request_iface), name=name, info=info)
76
77=== added file 'lib/canonical/launchpad/webapp/tests/test_initialization.py'
78--- lib/canonical/launchpad/webapp/tests/test_initialization.py 1970-01-01 00:00:00 +0000
79+++ lib/canonical/launchpad/webapp/tests/test_initialization.py 2010-06-04 21:48:33 +0000
80@@ -0,0 +1,64 @@
81+# Copyright 2010 Canonical Ltd. This software is licensed under the
82+# GNU Affero General Public License version 3 (see the file LICENSE).
83+
84+"""Tests post-zcml application initialization.
85+
86+As found in canonical.launchpad.webapp.initialization.py."""
87+
88+import unittest
89+
90+from lp.testing import TestCase
91+from zope.interface import Interface
92+from zope.component import getSiteManager
93+from zope.publisher.interfaces.browser import IBrowserRequest
94+from zope.traversing.interfaces import ITraversable
95+
96+from canonical.launchpad.webapp.errorlog import OopsNamespace
97+from canonical.launchpad.webapp.servers import LaunchpadTestRequest
98+from canonical.testing import FunctionalLayer
99+
100+
101+class AnyObject:
102+ pass
103+
104+
105+class TestURLNamespace(TestCase):
106+
107+ layer = FunctionalLayer
108+
109+ def setUp(self):
110+ TestCase.setUp(self)
111+ self.sm = getSiteManager()
112+ self.context = AnyObject()
113+ self.request = LaunchpadTestRequest()
114+
115+ def test_oops_namespace_not_view(self):
116+ # The ++oops++ namespace should not be available as a "oops" view.
117+ # First, we will verify that it is available as a namespace.
118+ namespace = self.sm.getMultiAdapter(
119+ (self.context, self.request), ITraversable, 'oops')
120+ self.failUnless(isinstance(namespace, OopsNamespace))
121+ # However, it is not available as a view.
122+ not_a_namespace = self.sm.queryMultiAdapter(
123+ (self.context, self.request), Interface, 'oops')
124+ self.failIf(isinstance(not_a_namespace, OopsNamespace))
125+
126+ def test_no_namespaces_are_views(self):
127+ # This tests an abstract superset of test_oops_namespace_not_view.
128+ # At the time of writing, namespaces were 'resource', 'oops', 'form',
129+ # and 'view'.
130+ namespace_info = self.sm.adapters.lookupAll(
131+ (Interface, IBrowserRequest), ITraversable)
132+ for name, factory in namespace_info:
133+ try:
134+ not_the_namespace_factory = self.sm.adapters.lookup(
135+ (Interface, IBrowserRequest), Interface, name)
136+ except LookupError:
137+ pass
138+ else:
139+ self.assertNotEqual(factory, not_the_namespace_factory)
140+
141+
142+def test_suite():
143+ suite = unittest.TestLoader().loadTestsFromName(__name__)
144+ return suite