Merge lp:~leonardr/lazr.restful/launchpad-integration into lp:lazr.restful

Proposed by Leonard Richardson
Status: Merged
Merged at revision: not available
Proposed branch: lp:~leonardr/lazr.restful/launchpad-integration
Merge into: lp:lazr.restful
Diff against target: 516 lines (+141/-47)
9 files modified
src/lazr/restful/_resource.py (+5/-5)
src/lazr/restful/declarations.py (+2/-2)
src/lazr/restful/docs/multiversion.txt (+3/-3)
src/lazr/restful/example/base/filemanager.py (+2/-2)
src/lazr/restful/example/base/root.py (+3/-2)
src/lazr/restful/tales.py (+11/-11)
src/lazr/restful/tests/test_utils.py (+3/-0)
src/lazr/restful/tests/test_webservice.py (+93/-22)
src/lazr/restful/utils.py (+19/-0)
To merge this branch: bzr merge lp:~leonardr/lazr.restful/launchpad-integration
Reviewer Review Type Date Requested Status
Paul Hummer (community) code Approve
Review via email: mp+19639@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Leonard Richardson (leonardr) wrote :

This branch creates a new utility function get_current_web_service_request() and changes all lazr.restful code to use it instead of get_current_browser_request(). I have not removed get_current_browser_request(), because I know it's used correctly elsewhere (eg. in Launchpad).

The multi-version code creates a big distinction between requests initiated by a web service client, and requests initiated by a web browser. Specifically, web browser requests don't have a version number or version-specific marker interface associated with them.

When I integrated the multi-version lazr.restful into Launchpadlib, this caused many Windmill tests to fail: the browser would make a request, and Launchpad would try to generate a representation of the context as though the incoming request were a web service request. But since the incoming request wasn't versioned, the versioned IEntry lookup would fail, and it would appear as though (eg.) bugs and users weren't published on the web service. Without a value in LP.cache.context, subsequent Ajax requests would fail, causing the Windmill test to fail.

The most confusing part of this branch is only somewhat related. While trying to get test_applyChanges_binds_to_resource_context to use a versioned request object, I noticed that applyChanges wasn't really doing anything (even in the case where the test supposedly passed). I added tests that prove that applyChanges changes the value for the field it's trying to change, and to get them to work I had to define a dummy IAbsoluteURL implementation for the entry on which applyChanges was running.

Revision history for this message
Paul Hummer (rockstar) wrote :

Thanks for giving me an opportunity to look at this code. It all looks good to me.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/lazr/restful/_resource.py'
2--- src/lazr/restful/_resource.py 2010-02-15 12:18:53 +0000
3+++ src/lazr/restful/_resource.py 2010-02-18 17:57:15 +0000
4@@ -87,7 +87,7 @@
5 IUnmarshallingDoesntNeedValue, IWebServiceClientRequest,
6 IWebServiceConfiguration, IWebServiceLayer, IWebServiceVersion,
7 LAZR_WEBSERVICE_NAME)
8-from lazr.restful.utils import get_current_browser_request
9+from lazr.restful.utils import get_current_web_service_request
10
11
12 # The path to the WADL XML Schema definition.
13@@ -158,7 +158,7 @@
14 return tuple(obj)
15 if isinstance(underlying_object, dict):
16 return dict(obj)
17- request = get_current_browser_request()
18+ request = get_current_web_service_request()
19 if queryMultiAdapter((obj, request), IEntry):
20 obj = EntryResource(obj, request)
21
22@@ -1549,8 +1549,8 @@
23
24 @property
25 def request(self):
26- """Fetch the current browser request."""
27- return get_current_browser_request()
28+ """Fetch the current web service request."""
29+ return get_current_web_service_request()
30
31 def _getETagCore(self, cache=None):
32 """Calculate an ETag for a representation of this resource.
33@@ -1793,7 +1793,7 @@
34
35 def _service_root_url(self):
36 """Return the URL to the service root."""
37- request = get_current_browser_request()
38+ request = get_current_web_service_request()
39 return absoluteURL(request.publication.getApplication(request),
40 request)
41
42
43=== modified file 'src/lazr/restful/declarations.py'
44--- src/lazr/restful/declarations.py 2010-02-16 16:14:01 +0000
45+++ src/lazr/restful/declarations.py 2010-02-18 17:57:15 +0000
46@@ -60,7 +60,7 @@
47 Collection, Entry, EntryAdapterUtility, ResourceOperation, ObjectLink)
48 from lazr.restful.security import protect_schema
49 from lazr.restful.utils import (
50- camelcase_to_underscore_separated, get_current_browser_request,
51+ camelcase_to_underscore_separated, get_current_web_service_request,
52 make_identifier_safe, VersionedDict)
53
54 LAZR_WEBSERVICE_EXPORTED = '%s.exported' % LAZR_WEBSERVICE_NS
55@@ -1030,7 +1030,7 @@
56 def __get__(self, instance, owner):
57 """Look up the entry schema that adapts the model schema."""
58 if instance is None or instance.request is None:
59- request = get_current_browser_request()
60+ request = get_current_web_service_request()
61 else:
62 request = instance.request
63 request_interface = getUtility(
64
65=== modified file 'src/lazr/restful/docs/multiversion.txt'
66--- src/lazr/restful/docs/multiversion.txt 2010-02-11 17:57:16 +0000
67+++ src/lazr/restful/docs/multiversion.txt 2010-02-18 17:57:15 +0000
68@@ -174,7 +174,7 @@
69 >>> from zope.publisher.interfaces.browser import IBrowserRequest
70 >>> from lazr.restful.interfaces import IServiceRootResource
71 >>> from lazr.restful.simple import TraverseWithGet
72- >>> from lazr.restful.utils import get_current_browser_request
73+ >>> from lazr.restful.utils import get_current_web_service_request
74 >>> class ContactSet(TraverseWithGet):
75 ... implements(IContactSet, ILocation)
76 ...
77@@ -204,13 +204,13 @@
78 ...
79 ... @property
80 ... def __parent__(self):
81- ... request = get_current_browser_request()
82+ ... request = get_current_web_service_request()
83 ... return getUtility(
84 ... IServiceRootResource, name=request.version)
85 ...
86 ... @property
87 ... def __name__(self):
88- ... request = get_current_browser_request()
89+ ... request = get_current_web_service_request()
90 ... if request.version == 'beta':
91 ... return 'contact_list'
92 ... return 'contacts'
93
94=== modified file 'src/lazr/restful/example/base/filemanager.py'
95--- src/lazr/restful/example/base/filemanager.py 2009-10-20 17:00:23 +0000
96+++ src/lazr/restful/example/base/filemanager.py 2010-02-18 17:57:15 +0000
97@@ -21,7 +21,7 @@
98
99 from lazr.restful import ReadOnlyResource
100 from lazr.restful.example.base.interfaces import IFileManager
101-from lazr.restful.utils import get_current_browser_request
102+from lazr.restful.utils import get_current_web_service_request
103
104
105 class FileManager:
106@@ -65,7 +65,7 @@
107
108 def __call__(self):
109 """Write the file to the current request."""
110- request = get_current_browser_request()
111+ request = get_current_web_service_request()
112 response = request.response
113
114 # Handle a conditional request
115
116=== modified file 'src/lazr/restful/example/base/root.py'
117--- src/lazr/restful/example/base/root.py 2010-02-11 17:57:16 +0000
118+++ src/lazr/restful/example/base/root.py 2010-02-18 17:57:15 +0000
119@@ -29,7 +29,7 @@
120 IFileManager, IRecipe, IRecipeSet, IHasGet, NameAlreadyTaken)
121 from lazr.restful.simple import BaseWebServiceConfiguration
122 from lazr.restful.testing.webservice import WebServiceTestPublication
123-from lazr.restful.utils import get_current_browser_request
124+from lazr.restful.utils import get_current_web_service_request
125
126
127 #Entry classes.
128@@ -148,7 +148,8 @@
129 self.recipes.remove(recipe)
130
131 def replace_cover(self, cover):
132- entry = getMultiAdapter((self, get_current_browser_request()), IEntry)
133+ entry = getMultiAdapter(
134+ (self, get_current_web_service_request()), IEntry)
135 storage = SimpleByteStorage(entry, ICookbook['cover'])
136 storage.createStored('application/octet-stream', cover, 'cover')
137
138
139=== modified file 'src/lazr/restful/tales.py'
140--- src/lazr/restful/tales.py 2010-02-11 16:32:17 +0000
141+++ src/lazr/restful/tales.py 2010-02-18 17:57:15 +0000
142@@ -34,7 +34,7 @@
143 IResourceOperation, IResourcePOSTOperation, IScopedCollection,
144 ITopLevelEntryLink, IWebServiceClientRequest, IWebServiceVersion,
145 LAZR_WEBSERVICE_NAME)
146-from lazr.restful.utils import get_current_browser_request
147+from lazr.restful.utils import get_current_web_service_request
148
149
150 class WadlDocstringLinker(DocstringLinker):
151@@ -110,13 +110,13 @@
152 @property
153 def is_entry(self):
154 """Whether the object is published as an entry."""
155- return queryMultiAdapter(
156- (self.context, get_current_browser_request()), IEntry) != None
157+ request = get_current_web_service_request()
158+ return queryMultiAdapter((self.context, request), IEntry) != None
159
160 @property
161 def json(self):
162 """Return a JSON description of the object."""
163- request = IWebServiceClientRequest(get_current_browser_request())
164+ request = get_current_web_service_request
165 if queryMultiAdapter((self.context, request), IEntry):
166 resource = EntryResource(self.context, request)
167 else:
168@@ -137,7 +137,7 @@
169 @property
170 def url(self):
171 """Return the full URL to the resource."""
172- return absoluteURL(self.context, get_current_browser_request())
173+ return absoluteURL(self.context, get_current_web_service_request())
174
175
176 class WadlEntryResourceAPI(WadlResourceAPI):
177@@ -182,7 +182,7 @@
178 else:
179 relationship_name = self.context.relationship.__name__
180 return (absoluteURL(self.context.context,
181- get_current_browser_request()) + '/' +
182+ get_current_web_service_request()) + '/' +
183 urllib.quote(relationship_name))
184 else:
185 return super(WadlCollectionResourceAPI, self).url
186@@ -287,7 +287,7 @@
187 model_class = self._model_class
188 operations = []
189 request_interface = getUtility(
190- IWebServiceVersion, get_current_browser_request().version)
191+ IWebServiceVersion, get_current_web_service_request().version)
192 for interface in (IResourceGETOperation, IResourcePOSTOperation):
193 operations.extend(getGlobalSiteManager().adapters.lookupAll(
194 (model_class, request_interface), interface))
195@@ -309,7 +309,7 @@
196 super(WadlEntryInterfaceAdapterAPI, self).__init__(
197 entry_interface, IEntry)
198 self.utility = EntryAdapterUtility.forEntryInterface(
199- entry_interface, get_current_browser_request())
200+ entry_interface, get_current_web_service_request())
201
202 @property
203 def entry_page_representation_link(self):
204@@ -383,7 +383,7 @@
205 def supports_delete(self):
206 """Return true if this entry responds to DELETE."""
207 request_interface = getUtility(
208- IWebServiceVersion, get_current_browser_request().version)
209+ IWebServiceVersion, get_current_web_service_request().version)
210 operations = getGlobalSiteManager().adapters.lookupAll(
211 (self._model_class, request_interface),
212 IResourceDELETEOperation)
213@@ -521,7 +521,7 @@
214 assert schema is not IObject, (
215 "Null schema provided for %s" % self.field.__name__)
216 return EntryAdapterUtility.forSchemaInterface(
217- schema, get_current_browser_request())
218+ schema, get_current_web_service_request())
219
220
221 @property
222@@ -546,7 +546,7 @@
223 def type_link(self):
224 return EntryAdapterUtility.forSchemaInterface(
225 self.entry_link.entry_type,
226- get_current_browser_request()).type_link
227+ get_current_web_service_request()).type_link
228
229
230 class WadlOperationAPI(RESTUtilityBase):
231
232=== modified file 'src/lazr/restful/tests/test_utils.py'
233--- src/lazr/restful/tests/test_utils.py 2010-01-15 14:12:26 +0000
234+++ src/lazr/restful/tests/test_utils.py 2010-02-18 17:57:15 +0000
235@@ -27,3 +27,6 @@
236 newInteraction(request)
237 self.assertEquals(request, get_current_browser_request())
238 endInteraction()
239+
240+ # For the sake of convenience, test_get_current_web_service_request()
241+ # is tested in test_webservice.py.
242
243=== modified file 'src/lazr/restful/tests/test_webservice.py'
244--- src/lazr/restful/tests/test_webservice.py 2010-02-11 17:57:16 +0000
245+++ src/lazr/restful/tests/test_webservice.py 2010-02-18 17:57:15 +0000
246@@ -5,6 +5,7 @@
247 __metaclass__ = type
248
249 from cStringIO import StringIO
250+from operator import attrgetter
251 import sys
252 from types import ModuleType
253 import unittest
254@@ -12,25 +13,30 @@
255 from zope.component import getGlobalSiteManager, getUtility
256 from zope.configuration import xmlconfig
257 from zope.interface import alsoProvides, implements, Interface
258+from zope.publisher.browser import TestRequest
259 from zope.schema import Date, Datetime, TextLine
260+from zope.security.management import (
261+ endInteraction, newInteraction, queryInteraction)
262 from zope.testing.cleanup import CleanUp
263 from zope.traversing.browser.interfaces import IAbsoluteURL
264
265 from lazr.restful.fields import Reference
266 from lazr.restful.interfaces import (
267 ICollection, IEntry, IEntryResource, IResourceGETOperation,
268- IServiceRootResource, IWebServiceClientRequest, IWebServiceVersion)
269+ IServiceRootResource, IWebServiceConfiguration,
270+ IWebServiceClientRequest, IWebServiceVersion)
271 from lazr.restful import (
272 EntryResource, ServiceRootResource, ResourceGETOperation)
273 from lazr.restful.simple import BaseWebServiceConfiguration, Request
274 from lazr.restful.declarations import (
275 collection_default_content, exported, export_as_webservice_collection,
276 export_as_webservice_entry, export_read_operation, operation_parameters)
277-from lazr.restful.interfaces import IWebServiceConfiguration
278 from lazr.restful.simple import RootResourceAbsoluteURL
279 from lazr.restful.testing.webservice import (
280 create_web_service_request, WebServiceTestPublication)
281 from lazr.restful.testing.tales import test_tales
282+from lazr.restful.utils import (
283+ get_current_browser_request, get_current_web_service_request)
284
285
286 def get_resource_factory(model_interface, resource_interface):
287@@ -42,7 +48,7 @@
288 `IEntry` or `ICollection`.
289 :return: the resource factory (the autogenerated adapter class.
290 """
291- request_interface = getUtility(IWebServiceVersion, name='trunk')
292+ request_interface = getUtility(IWebServiceVersion, name='2.0')
293 return getGlobalSiteManager().adapters.lookup(
294 (model_interface, request_interface), resource_interface)
295
296@@ -56,7 +62,7 @@
297 :return: the factory (autogenerated class) that implements the operation
298 on the webservice.
299 """
300- request_interface = getUtility(IWebServiceVersion, name='trunk')
301+ request_interface = getUtility(IWebServiceVersion, name='2.0')
302 return getGlobalSiteManager().adapters.lookup(
303 (model_interface, request_interface),
304 IResourceGETOperation, name=name)
305@@ -95,23 +101,30 @@
306 """Returns all the entries."""
307
308
309+class IWebServiceRequest10(IWebServiceClientRequest):
310+ """A marker interface for requests to the '1.0' web service."""
311+
312+
313+class IWebServiceRequest20(IWebServiceClientRequest):
314+ """A marker interface for requests to the '2.0' web service."""
315+
316+
317 class SimpleWebServiceConfiguration(BaseWebServiceConfiguration):
318 implements(IWebServiceConfiguration)
319 show_tracebacks = False
320- active_versions = ['trunk']
321+ active_versions = ['1.0', '2.0']
322 hostname = "webservice_test"
323
324 def createRequest(self, body_instream, environ):
325 request = Request(body_instream, environ)
326- request.setPublication(WebServiceTestPublication(None))
327- request.version = 'trunk'
328+ request.version = '2.0'
329+ alsoProvides(request, IWebServiceRequest20)
330+ request.setPublication(WebServiceTestPublication(
331+ getUtility(IServiceRootResource)))
332+ request.annotations[request.VERSION_ANNOTATION] = '2.0'
333 return request
334
335
336-class IWebServiceRequestTrunk(IWebServiceClientRequest):
337- """A marker interface for requests to the 'trunk' web service."""
338-
339-
340 class WebServiceTestCase(CleanUp, unittest.TestCase):
341 """A test case for web service operations."""
342
343@@ -126,11 +139,14 @@
344 sm = getGlobalSiteManager()
345 sm.registerUtility(webservice_configuration)
346
347- # Register an IWebServiceVersion for the
348- # 'trunk' web service version.
349- alsoProvides(IWebServiceRequestTrunk, IWebServiceVersion)
350- sm.registerUtility(
351- IWebServiceRequestTrunk, IWebServiceVersion, name='trunk')
352+ # Register IWebServiceVersions for the
353+ # '1.0' and '2.0' web service versions.
354+ alsoProvides(IWebServiceRequest10, IWebServiceVersion)
355+ sm.registerUtility(
356+ IWebServiceRequest10, IWebServiceVersion, name='1.0')
357+ alsoProvides(IWebServiceRequest20, IWebServiceVersion)
358+ sm.registerUtility(
359+ IWebServiceRequest20, IWebServiceVersion, name='2.0')
360
361 # Register a service root resource
362 service_root = ServiceRootResource()
363@@ -224,11 +240,28 @@
364 self.a_field = value
365
366
367+class DummyAbsoluteURL:
368+ """Implements IAbsoluteURL for when you don't care what the URL is."""
369+ def __init__(self, *args):
370+ pass
371+
372+ def __str__(self):
373+ return 'http://dummyurl/'
374+
375+ __call__ = __str__
376+
377+
378 class EntryTestCase(WebServiceTestCase):
379 """A test suite for entries."""
380
381 testmodule_objects = [HasRestrictedField, IHasRestrictedField]
382
383+ def setUp(self):
384+ super(EntryTestCase, self).setUp()
385+ getGlobalSiteManager().registerAdapter(
386+ DummyAbsoluteURL, [IHasRestrictedField, IWebServiceClientRequest],
387+ IAbsoluteURL)
388+
389 def test_applyChanges_binds_to_resource_context(self):
390 """Make sure applyChanges binds fields to the resource context.
391
392@@ -238,13 +271,15 @@
393 expose the right interface, it will raise an exception.
394 """
395 entry_class = get_resource_factory(IHasRestrictedField, IEntry)
396- request = Request(StringIO(""), {})
397+ request = getUtility(IWebServiceConfiguration).createRequest(
398+ StringIO(""), {})
399
400 entry = entry_class(HasRestrictedField(""), request)
401 resource = EntryResource(HasRestrictedField(""), request)
402-
403 entry.schema['a_field'].restrict_to_interface = IHasRestrictedField
404- resource.applyChanges({'a_field': 'a_value'})
405+ self.assertEquals(resource.entry.a_field, '')
406+ resource.applyChanges({'a_field': u'a_value'})
407+ self.assertEquals(resource.entry.a_field, 'a_value')
408
409 # Make sure that IHasRestrictedField itself works correctly.
410 class IOtherInterface(Interface):
411@@ -252,8 +287,8 @@
412 pass
413 entry.schema['a_field'].restrict_to_interface = IOtherInterface
414 self.assertRaises(AssertionError, resource.applyChanges,
415- {'a_field': 'a_value'})
416-
417+ {'a_field': u'a_new_value'})
418+ self.assertEquals(resource.entry.a_field, 'a_value')
419
420 class WadlAPITestCase(WebServiceTestCase):
421 """Test the docstring generation."""
422@@ -353,7 +388,7 @@
423 This will fail due to a name conflict.
424 """
425 resource = getUtility(IServiceRootResource)
426- request = create_web_service_request('/trunk')
427+ request = create_web_service_request('/2.0')
428 request.traverse(resource)
429 try:
430 resource.toWADL()
431@@ -390,5 +425,41 @@
432 "plural name 'generic_entrys'.")
433
434
435+class GetCurrentWebServiceRequestTestCase(WebServiceTestCase):
436+ """Test the get_current_web_service_request utility function."""
437+
438+ testmodule_objects = [IGenericEntry]
439+
440+ def test_web_service_request_is_versioned(self):
441+ """Ensure the get_current_web_service_request() result is versioned.
442+
443+ When a normal browser request is turned into a web service
444+ request, it needs to have a version associated with it.
445+ lazr.restful associates the new request with the latest
446+ version of the web service: in this case, version 2.0.
447+ """
448+
449+ # When there's no interaction setup, get_current_web_service_request()
450+ # returns None.
451+ self.assertEquals(None, queryInteraction())
452+ self.assertEquals(None, get_current_web_service_request())
453+
454+ # Set up an interaction.
455+ request = TestRequest()
456+ newInteraction(request)
457+
458+ # A normal web browser request isn't associated with any version.
459+ website_request = get_current_browser_request()
460+ self.assertRaises(AttributeError,
461+ attrgetter('version', website_request), None)
462+
463+ # But the result of get_current_web_service_request() is
464+ # associated with version 2.0.
465+ webservice_request = get_current_web_service_request()
466+ self.assertEquals("2.0", webservice_request.version)
467+ marker_20 = getUtility(IWebServiceVersion, "2.0")
468+ marker_20.providedBy(webservice_request)
469+
470+
471 def additional_tests():
472 return unittest.TestLoader().loadTestsFromName(__name__)
473
474=== modified file 'src/lazr/restful/utils.py'
475--- src/lazr/restful/utils.py 2010-01-20 15:21:38 +0000
476+++ src/lazr/restful/utils.py 2010-02-18 17:57:15 +0000
477@@ -6,6 +6,7 @@
478 __all__ = [
479 'camelcase_to_underscore_separated',
480 'get_current_browser_request',
481+ 'get_current_web_service_request',
482 'implement_from_dict',
483 'make_identifier_safe',
484 'safe_js_escape',
485@@ -27,6 +28,8 @@
486 from zope.schema import getFieldsInOrder
487 from zope.interface import classImplements
488
489+from lazr.restful.interfaces import IWebServiceClientRequest
490+
491
492 missing = object()
493
494@@ -217,6 +220,22 @@
495 return requests[0]
496
497
498+def get_current_web_service_request():
499+ """Return the current web service request.
500+
501+ This may be a new request object based on the browser request (if
502+ the client is a web browser that might make AJAX calls to a
503+ separate web service) or it may be the same as the browser request
504+ (if the client is a web service client accessing the service
505+ directly).
506+
507+ :return: An object providing IWebserviceClientRequest.
508+ """
509+ request = get_current_browser_request()
510+ if request is None:
511+ return None
512+ return IWebServiceClientRequest(request)
513+
514 def simple_popen2(command, input, env=None, in_bufsize=1024, out_bufsize=128):
515 """Run a command, give it input on its standard input, and capture its
516 standard output.

Subscribers

People subscribed via source and target branches