Merge lp:~mwhudson/launchpad/vostok-add-root into lp:launchpad

Proposed by Michael Hudson-Doyle
Status: Merged
Approved by: Tim Penhey
Approved revision: no longer in the source branch.
Merged at revision: 11299
Proposed branch: lp:~mwhudson/launchpad/vostok-add-root
Merge into: lp:launchpad
Prerequisite: lp:~mwhudson/launchpad/vostok-add-layer
Diff against target: 238 lines (+140/-5)
10 files modified
lib/lp/vostok/browser/__init__.py (+4/-0)
lib/lp/vostok/browser/configure.zcml (+22/-0)
lib/lp/vostok/browser/root.py (+15/-0)
lib/lp/vostok/browser/tests/__init__.py (+4/-0)
lib/lp/vostok/browser/tests/request.py (+19/-0)
lib/lp/vostok/browser/tests/test_root.py (+38/-0)
lib/lp/vostok/configure.zcml (+7/-0)
lib/lp/vostok/publisher.py (+12/-4)
lib/lp/vostok/templates/root.pt (+3/-0)
lib/lp/vostok/tests/test_publisher.py (+16/-1)
To merge this branch: bzr merge lp:~mwhudson/launchpad/vostok-add-root
Reviewer Review Type Date Requested Status
Tim Penhey (community) Approve
Review via email: mp+31239@code.launchpad.net

Description of the change

Hi,

This branch adds a custom root object for the vostok vhost. This makes us more
separate from the other Launchpad publication stuff, which given the goal of
vostok is a good thing (tm).

Cheers,
mwh

To post a comment you must log in.
Revision history for this message
Tim Penhey (thumper) wrote :

> class VostokRootView(LaunchpadView):
> pass

Shouldn't you at least have

    __used_for__ = IVostokRoot

Instead of:
    view = getMultiAdapter(
       (VostokRoot(), VostokTestRequest()), name='+index')
    view.initialize()

Can you alter the get_initialized_view function?

> class IVostokRoot(Interface): # might need to inherit from some IRoot thing

You can probably lose the comment.

Also... the root.pt needs some work :-)

Why is the
  getUtility(IOpenLaunchBag).clear()
needed in test_root_object?

review: Needs Information
Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

On Thu, 29 Jul 2010 23:27:43 -0000, Tim Penhey <email address hidden> wrote:
> Review: Needs Information
> > class VostokRootView(LaunchpadView):
> > pass
>
> Shouldn't you at least have
>
> __used_for__ = IVostokRoot

After a bit of googling, apparently not. Maybe I should sed out all the
existing __used_for__s in our code base :-)

> Instead of:
> view = getMultiAdapter(
> (VostokRoot(), VostokTestRequest()), name='+index')
> view.initialize()
>
> Can you alter the get_initialized_view function?

Apparently yes, once I was told that it's actually called
create_initialized_view :-)

> > class IVostokRoot(Interface): # might need to inherit from some IRoot thing
>
> You can probably lose the comment.

Yeah, thanks.

> Also... the root.pt needs some work :-)

That's in the next pipe :-)

> Why is the
> getUtility(IOpenLaunchBag).clear()
> needed in test_root_object?

Errr... because the test blows up if its not there. Specifically, the
root object seems to be cached on the launchbag, and if the launchpad
isn't set up, it blows up.

I don't know if this is still used at all, quite possibly not, but that
seems a job for another branch.

I've expanded the comment.

Cheers,
mwh

1=== modified file 'lib/lp/vostok/browser/root.py'
2--- lib/lp/vostok/browser/root.py 2010-07-15 07:49:25 +0000
3+++ lib/lp/vostok/browser/root.py 2010-07-29 23:45:26 +0000
4@@ -12,4 +12,4 @@
5
6
7 class VostokRootView(LaunchpadView):
8- pass
9+ """The view for the Vostok root object."""
10
11=== modified file 'lib/lp/vostok/browser/tests/test_root.py'
12--- lib/lp/vostok/browser/tests/test_root.py 2010-07-29 04:30:04 +0000
13+++ lib/lp/vostok/browser/tests/test_root.py 2010-07-29 23:51:17 +0000
14@@ -8,14 +8,15 @@
15 import unittest
16
17 from zope.app.publisher.browser import getDefaultViewName
18-from zope.component import getMultiAdapter
19
20 from canonical.testing.layers import FunctionalLayer
21
22 from lp.testing import TestCase
23+from lp.testing.views import create_initialized_view
24 from lp.vostok.browser.root import VostokRootView
25 from lp.vostok.browser.tests.request import VostokTestRequest
26-from lp.vostok.publisher import VostokRoot
27+from lp.vostok.publisher import VostokLayer, VostokRoot
28+
29
30 class TestBrowseRoot(TestCase):
31
32@@ -28,9 +29,8 @@
33
34 def test_root_index_view(self):
35 # VostokRootView is registered as the view for the VostokRoot object.
36- view = getMultiAdapter(
37- (VostokRoot(), VostokTestRequest()), name='+index')
38- view.initialize()
39+ view = create_initialized_view(
40+ VostokRoot(), name='+index', layer=VostokLayer)
41 self.assertIsInstance(view, VostokRootView)
42
43
44
45=== modified file 'lib/lp/vostok/publisher.py'
46--- lib/lp/vostok/publisher.py 2010-07-14 14:27:27 +0000
47+++ lib/lp/vostok/publisher.py 2010-07-29 23:39:07 +0000
48@@ -28,7 +28,7 @@
49 implements(VostokLayer)
50
51
52-class IVostokRoot(Interface): # might need to inherit from some IRoot thing
53+class IVostokRoot(Interface):
54 """Marker interface for the root vostok object."""
55
56
57
58=== modified file 'lib/lp/vostok/tests/test_publisher.py'
59--- lib/lp/vostok/tests/test_publisher.py 2010-07-29 04:28:08 +0000
60+++ lib/lp/vostok/tests/test_publisher.py 2010-07-30 00:01:34 +0000
61@@ -37,7 +37,8 @@
62 request, publication = get_request_and_publication(
63 host=config.vhost.vostok.hostname)
64 self.assertProvides(request, VostokLayer)
65- # XXX This shouldn't be needed:
66+ # XXX getApplication caches the root object in the LaunchBag, so we
67+ # need to set it up, or it crashes.
68 getUtility(IOpenLaunchBag).clear()
69 root = publication.getApplication(request)
70 self.assertIsInstance(root, VostokRoot)
Revision history for this message
Tim Penhey (thumper) wrote :

The XXX comment should have name, date and bug as well.

Other than that, this is good to go now.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== added directory 'lib/lp/vostok/browser'
2=== added file 'lib/lp/vostok/browser/__init__.py'
3--- lib/lp/vostok/browser/__init__.py 1970-01-01 00:00:00 +0000
4+++ lib/lp/vostok/browser/__init__.py 2010-07-30 00:06:10 +0000
5@@ -0,0 +1,4 @@
6+# Copyright 2010 Canonical Ltd. This software is licensed under the
7+# GNU Affero General Public License version 3 (see the file LICENSE).
8+
9+"""Browser code for the Linaro archive management skin."""
10
11=== added file 'lib/lp/vostok/browser/configure.zcml'
12--- lib/lp/vostok/browser/configure.zcml 1970-01-01 00:00:00 +0000
13+++ lib/lp/vostok/browser/configure.zcml 2010-07-30 00:06:10 +0000
14@@ -0,0 +1,22 @@
15+<configure
16+ xmlns="http://namespaces.zope.org/zope"
17+ xmlns:browser="http://namespaces.zope.org/browser"
18+ xmlns:i18n="http://namespaces.zope.org/i18n"
19+ xmlns:xmlrpc="http://namespaces.zope.org/xmlrpc"
20+ i18n_domain="launchpad">
21+
22+ <browser:defaultView
23+ for="lp.vostok.publisher.IVostokRoot"
24+ name="+index"
25+ />
26+
27+ <browser:page
28+ for="lp.vostok.publisher.IVostokRoot"
29+ name="+index"
30+ class="lp.vostok.browser.root.VostokRootView"
31+ template="../templates/root.pt"
32+ permission="zope.Public"
33+ layer="lp.vostok.publisher.VostokLayer"
34+ />
35+
36+</configure>
37
38=== added file 'lib/lp/vostok/browser/root.py'
39--- lib/lp/vostok/browser/root.py 1970-01-01 00:00:00 +0000
40+++ lib/lp/vostok/browser/root.py 2010-07-30 00:06:10 +0000
41@@ -0,0 +1,15 @@
42+# Copyright 2010 Canonical Ltd. This software is licensed under the
43+# GNU Affero General Public License version 3 (see the file LICENSE).
44+
45+"""Browser code for the Vostok root."""
46+
47+__metaclass__ = type
48+__all__ = [
49+ 'VostokRootView',
50+ ]
51+
52+from canonical.launchpad.webapp import LaunchpadView
53+
54+
55+class VostokRootView(LaunchpadView):
56+ """The view for the Vostok root object."""
57
58=== added directory 'lib/lp/vostok/browser/tests'
59=== added file 'lib/lp/vostok/browser/tests/__init__.py'
60--- lib/lp/vostok/browser/tests/__init__.py 1970-01-01 00:00:00 +0000
61+++ lib/lp/vostok/browser/tests/__init__.py 2010-07-30 00:06:10 +0000
62@@ -0,0 +1,4 @@
63+# Copyright 2010 Canonical Ltd. This software is licensed under the
64+# GNU Affero General Public License version 3 (see the file LICENSE).
65+
66+"""Tests for the browser code of the Linaro archive management skin."""
67
68=== added file 'lib/lp/vostok/browser/tests/request.py'
69--- lib/lp/vostok/browser/tests/request.py 1970-01-01 00:00:00 +0000
70+++ lib/lp/vostok/browser/tests/request.py 2010-07-30 00:06:10 +0000
71@@ -0,0 +1,19 @@
72+# Copyright 2010 Canonical Ltd. This software is licensed under the
73+# GNU Affero General Public License version 3 (see the file LICENSE).
74+
75+"""A VostokLayer request class for use in tests."""
76+
77+__metaclass__ = type
78+__all__ = [
79+ 'VostokTestRequest',
80+ ]
81+
82+from zope.interface import implements
83+
84+from canonical.launchpad.webapp.servers import LaunchpadTestRequest
85+
86+from lp.vostok.publisher import VostokLayer
87+
88+
89+class VostokTestRequest(LaunchpadTestRequest):
90+ implements(VostokLayer)
91
92=== added file 'lib/lp/vostok/browser/tests/test_root.py'
93--- lib/lp/vostok/browser/tests/test_root.py 1970-01-01 00:00:00 +0000
94+++ lib/lp/vostok/browser/tests/test_root.py 2010-07-30 00:06:10 +0000
95@@ -0,0 +1,38 @@
96+# Copyright 2010 Canonical Ltd. This software is licensed under the
97+# GNU Affero General Public License version 3 (see the file LICENSE).
98+
99+"""Tests for browsing the root of the vostok skin."""
100+
101+__metaclass__ = type
102+
103+import unittest
104+
105+from zope.app.publisher.browser import getDefaultViewName
106+
107+from canonical.testing.layers import FunctionalLayer
108+
109+from lp.testing import TestCase
110+from lp.testing.views import create_initialized_view
111+from lp.vostok.browser.root import VostokRootView
112+from lp.vostok.browser.tests.request import VostokTestRequest
113+from lp.vostok.publisher import VostokLayer, VostokRoot
114+
115+
116+class TestBrowseRoot(TestCase):
117+
118+ layer = FunctionalLayer
119+
120+ def test_root_default_view_name(self):
121+ # The default view for the vostok root object is called "+index".
122+ view_name = getDefaultViewName(VostokRoot(), VostokTestRequest())
123+ self.assertEquals('+index', view_name)
124+
125+ def test_root_index_view(self):
126+ # VostokRootView is registered as the view for the VostokRoot object.
127+ view = create_initialized_view(
128+ VostokRoot(), name='+index', layer=VostokLayer)
129+ self.assertIsInstance(view, VostokRootView)
130+
131+
132+def test_suite():
133+ return unittest.TestLoader().loadTestsFromName(__name__)
134
135=== modified file 'lib/lp/vostok/configure.zcml'
136--- lib/lp/vostok/configure.zcml 2010-07-30 00:06:05 +0000
137+++ lib/lp/vostok/configure.zcml 2010-07-30 00:06:10 +0000
138@@ -5,10 +5,17 @@
139 xmlns:xmlrpc="http://namespaces.zope.org/xmlrpc"
140 i18n_domain="launchpad">
141
142+ <include package=".browser" />
143+
144 <publisher
145 name="vostok"
146 factory="lp.vostok.publisher.vostok_request_publication_factory"
147 methods="*"
148 mimetypes="*" />
149
150+ <securedutility
151+ class="lp.vostok.publisher.VostokRoot"
152+ provides="lp.vostok.publisher.IVostokRoot">
153+ <allow interface="lp.vostok.publisher.IVostokRoot" />
154+ </securedutility>
155 </configure>
156
157=== modified file 'lib/lp/vostok/publisher.py'
158--- lib/lp/vostok/publisher.py 2010-07-30 00:06:05 +0000
159+++ lib/lp/vostok/publisher.py 2010-07-30 00:06:10 +0000
160@@ -11,7 +11,7 @@
161 ]
162
163
164-from zope.interface import implements
165+from zope.interface import implements, Interface
166 from zope.publisher.interfaces.browser import (
167 IBrowserRequest, IDefaultBrowserLayer)
168
169@@ -28,10 +28,18 @@
170 implements(VostokLayer)
171
172
173-# We *might* end up customizing the root object and so need our own
174-# LaunchpadBrowserPublication subclass. Not yet though.
175+class IVostokRoot(Interface):
176+ """Marker interface for the root vostok object."""
177+
178+
179+class VostokRoot:
180+ implements(IVostokRoot)
181+
182+
183+class VostokBrowserPublication(LaunchpadBrowserPublication):
184+ root_object_interface = IVostokRoot
185
186
187 def vostok_request_publication_factory():
188 return VirtualHostRequestPublicationFactory(
189- 'vostok', VostokBrowserRequest, LaunchpadBrowserPublication)
190+ 'vostok', VostokBrowserRequest, VostokBrowserPublication)
191
192=== added directory 'lib/lp/vostok/templates'
193=== added file 'lib/lp/vostok/templates/root.pt'
194--- lib/lp/vostok/templates/root.pt 1970-01-01 00:00:00 +0000
195+++ lib/lp/vostok/templates/root.pt 2010-07-30 00:06:10 +0000
196@@ -0,0 +1,3 @@
197+<big>
198+THIS IS VOSTOK
199+</big>
200
201=== modified file 'lib/lp/vostok/tests/test_publisher.py'
202--- lib/lp/vostok/tests/test_publisher.py 2010-07-30 00:06:05 +0000
203+++ lib/lp/vostok/tests/test_publisher.py 2010-07-30 00:06:10 +0000
204@@ -9,11 +9,14 @@
205
206 from canonical.config import config
207 from canonical.testing.layers import FunctionalLayer
208+from canonical.launchpad.webapp.interfaces import IOpenLaunchBag
209
210 from lp.testing import TestCase
211 from lp.testing.publication import get_request_and_publication
212
213-from lp.vostok.publisher import VostokLayer
214+from lp.vostok.publisher import VostokLayer, VostokRoot
215+
216+from zope.component import getUtility
217
218
219 class TestRegistration(TestCase):
220@@ -28,6 +31,18 @@
221 host=config.vhost.vostok.hostname)
222 self.assertProvides(request, VostokLayer)
223
224+ def test_root_object(self):
225+ # The root object for requests to the vostok host is an instance of
226+ # VostokRoot.
227+ request, publication = get_request_and_publication(
228+ host=config.vhost.vostok.hostname)
229+ self.assertProvides(request, VostokLayer)
230+ # XXX getApplication caches the root object in the LaunchBag, so we
231+ # need to set it up, or it crashes.
232+ getUtility(IOpenLaunchBag).clear()
233+ root = publication.getApplication(request)
234+ self.assertIsInstance(root, VostokRoot)
235+
236
237 def test_suite():
238 return unittest.TestLoader().loadTestsFromName(__name__)