Merge lp:~leonardr/lazr.restful/stop-using-object into lp:lazr.restful

Proposed by Leonard Richardson on 2011-01-21
Status: Merged
Approved by: Tim Penhey on 2011-01-21
Approved revision: 167
Merged at revision: 164
Proposed branch: lp:~leonardr/lazr.restful/stop-using-object
Merge into: lp:lazr.restful
Diff against target: 467 lines (+112/-87) 11 files modified
To merge this branch: bzr merge lp:~leonardr/lazr.restful/stop-using-object
Reviewer Review Type Date Requested Status
Tim Penhey (community) 2011-01-21 Approve on 2011-01-21
Review via email: mp+47106@code.launchpad.net

Description of the Change

This branch prohibits the use of IObject in exported interfaces. The validation code associated with IObject causes infinite recursion. We previously had special code to avoid validation if validation would cause infinite recursion, but this meant that validation code wasn't running. We have a perfectly acceptable alternative to IObject that doesn't cause the problem (IReference). This branch replaces all our Objects with References, and adds code to raise an exception if you try to export an IObject that's not an IReference.

To post a comment you must log in.
166. By Leonard Richardson on 2011-01-21

Removed unused method.

167. By Leonard Richardson on 2011-01-21

Fixed test failures.

Tim Penhey (thumper) wrote :

Looks good.

review: Approve

Preview Diff

1=== modified file 'src/lazr/restful/NEWS.txt'
2--- src/lazr/restful/NEWS.txt 2011-01-20 20:25:31 +0000
3+++ src/lazr/restful/NEWS.txt 2011-01-21 21:12:04 +0000
4@@ -2,6 +2,15 @@
5 NEWS for lazr.restful
6 =====================
7
8+0.15.3 (2010-01-21)
9+===================
10+
11+lazr.restful will now complain if you try to export an IObject, as
12+this causes infinite recursion during field validation. We had code
13+that worked around the infinite recursion, but it wasn't reliable and
14+we've now removed it to simplify. Use IReference whenever you would
15+use IObject.
16+
17 0.15.2 (2010-01-20)
18 ===================
19
20
21=== modified file 'src/lazr/restful/_resource.py'
22--- src/lazr/restful/_resource.py 2011-01-19 19:00:10 +0000
23+++ src/lazr/restful/_resource.py 2011-01-21 21:12:04 +0000
24@@ -65,7 +65,7 @@
25 from zope.publisher.interfaces.http import IHTTPRequest
26 from zope.schema import ValidationError, getFieldsInOrder
27 from zope.schema.interfaces import (
28- ConstraintNotSatisfied, IBytes, IField, IObject, RequiredMissing)
29+ ConstraintNotSatisfied, IBytes, IField, RequiredMissing)
30 from zope.security.interfaces import Unauthorized
31 from zope.security.proxy import getChecker, removeSecurityProxy
32 from zope.security.management import checkPermission
33@@ -82,12 +82,12 @@
34 from lazr.restful.interfaces import (
35 ICollection, ICollectionField, ICollectionResource, IEntry, IEntryField,
36 IEntryFieldResource, IEntryResource, IFieldHTMLRenderer, IFieldMarshaller,
37- IHTTPResource, IJSONPublishable, IReferenceChoice, IRepresentationCache,
38- IResourceDELETEOperation, IResourceGETOperation, IResourcePOSTOperation,
39- IScopedCollection, IServiceRootResource, ITopLevelEntryLink,
40- IUnmarshallingDoesntNeedValue, IWebServiceClientRequest,
41- IWebServiceConfiguration, IWebServiceLayer, IWebServiceVersion,
42- LAZR_WEBSERVICE_NAME)
43+ IHTTPResource, IJSONPublishable, IReference, IReferenceChoice,
44+ IRepresentationCache, IResourceDELETEOperation, IResourceGETOperation,
45+ IResourcePOSTOperation, IScopedCollection, IServiceRootResource,
46+ ITopLevelEntryLink, IUnmarshallingDoesntNeedValue,
47+ IWebServiceClientRequest, IWebServiceConfiguration, IWebServiceLayer,
48+ IWebServiceVersion, LAZR_WEBSERVICE_NAME)
49 from lazr.restful.utils import (
50 extract_write_portion,
51 get_current_web_service_request,
52@@ -463,16 +463,6 @@
53 etags.append(etag)
54 return etags
55
56- def _fieldValueIsObject(self, field):
57- """Does the given field expect a data model object as its value?
58-
59- Obviously an IObject field is expected to have a data model
60- object as its value. But an IChoice field might also have a
61- vocabulary drawn from the set of data model objects.
62- """
63- return (IObject.providedBy(field)
64- or IReferenceChoice.providedBy(field))
65-
66 def _parseContentDispositionHeader(self, value):
67 """Parse a Content-Disposition header.
68
69@@ -1068,12 +1058,12 @@
70
71 # If the new value is an object, make sure it provides the correct
72 # interface.
73- if value is not None and IObject.providedBy(field):
74+ if value is not None and IReference.providedBy(field):
75 # XXX leonardr 2008-04-12 spec=api-wadl-description:
76 # This should be moved into the
77 # ObjectLookupFieldMarshaller, once we make it
78 # possible for Vocabulary fields to specify a schema
79- # class the way IObject fields can.
80+ # class the way IReference fields can.
81 if value != None and not field.schema.providedBy(value):
82 errors.append(u"%s: Your value points to the "
83 "wrong kind of object" % repr_name)
84@@ -1096,39 +1086,30 @@
85 continue
86
87 if change_this_field is True and value != current_value:
88- if not IObject.providedBy(field):
89- # We don't validate IObject values because that
90- # can lead to infinite recursion. We don't _need_
91- # to validate IObject values because a client
92- # isn't changing anything about the IObject; it's
93- # just associating one IObject or another with an
94- # entry. We're already checking the type of the
95- # new IObject, and that's the only error the
96- # client can cause.
97- try:
98- # Do any field-specific validation.
99- field.validate(value)
100- except ConstraintNotSatisfied, e:
101- # Try to get a string error message out of
102- # the exception; otherwise use a generic message
103- # instead of whatever object the raise site
104- # thought would be a good idea.
105- if (len(e.args) > 0 and
106- isinstance(e.args[0], basestring)):
107- error = e.args[0]
108- else:
109- error = "Constraint not satisfied."
110- errors.append(u"%s: %s" % (repr_name, error))
111- continue
112- except RequiredMissing, e:
113- error = "Missing required value."
114- errors.append(u"%s: %s" % (repr_name, error))
115- except (ValueError, ValidationError), e:
116- error = str(e)
117- if error == "":
118- error = "Validation error"
119- errors.append(u"%s: %s" % (repr_name, error))
120- continue
121+ try:
122+ # Do any field-specific validation.
123+ field.validate(value)
124+ except ConstraintNotSatisfied, e:
125+ # Try to get a string error message out of
126+ # the exception; otherwise use a generic message
127+ # instead of whatever object the raise site
128+ # thought would be a good idea.
129+ if (len(e.args) > 0 and
130+ isinstance(e.args[0], basestring)):
131+ error = e.args[0]
132+ else:
133+ error = "Constraint not satisfied."
134+ errors.append(u"%s: %s" % (repr_name, error))
135+ continue
136+ except RequiredMissing, e:
137+ error = "Missing required value."
138+ errors.append(u"%s: %s" % (repr_name, error))
139+ except (ValueError, ValidationError), e:
140+ error = str(e)
141+ if error == "":
142+ error = "Validation error"
143+ errors.append(u"%s: %s" % (repr_name, error))
144+ continue
145 validated_changeset.append((field, value))
146 # If there are any fields left in the changeset, they're
147 # fields that don't correspond to some field in the
148
149=== modified file 'src/lazr/restful/declarations.py'
150--- src/lazr/restful/declarations.py 2010-08-26 19:49:20 +0000
151+++ src/lazr/restful/declarations.py 2011-01-21 21:12:04 +0000
152@@ -47,7 +47,11 @@
153 from zope.interface.interface import fromFunction, InterfaceClass, TAGGED_DATA
154 from zope.interface.interfaces import IInterface, IMethod
155 from zope.schema import getFields
156-from zope.schema.interfaces import IField, IText
157+from zope.schema.interfaces import (
158+ IField,
159+ IObject,
160+ IText,
161+ )
162 from zope.security.checker import CheckerPublic
163 from zope.traversing.browser import absoluteURL
164
165@@ -56,9 +60,9 @@
166 from lazr.restful.fields import CollectionField, Reference
167 from lazr.restful.interface import copy_field
168 from lazr.restful.interfaces import (
169- ICollection, IEntry, IResourceDELETEOperation, IResourceGETOperation,
170- IResourcePOSTOperation, IWebServiceConfiguration, IWebServiceVersion,
171- LAZR_WEBSERVICE_NAME, LAZR_WEBSERVICE_NS)
172+ ICollection, IEntry, IReference, IResourceDELETEOperation,
173+ IResourceGETOperation, IResourcePOSTOperation, IWebServiceConfiguration,
174+ IWebServiceVersion, LAZR_WEBSERVICE_NAME, LAZR_WEBSERVICE_NS)
175 from lazr.restful import (
176 Collection, Entry, EntryAdapterUtility, ResourceOperation, ObjectLink)
177 from lazr.restful.security import protect_schema
178@@ -204,6 +208,9 @@
179 if not IField.providedBy(field):
180 raise TypeError("exported() can only be used on IFields.")
181
182+ if IObject.providedBy(field) and not IReference.providedBy(field):
183+ raise TypeError("Object exported; use Reference instead.")
184+
185 # The first step is to turn the arguments into a
186 # VersionedDict. We'll start by collecting annotations for the
187 # earliest version, which we refer to as None because we don't
188
189=== modified file 'src/lazr/restful/docs/webservice-declarations.txt'
190--- src/lazr/restful/docs/webservice-declarations.txt 2010-08-26 19:49:20 +0000
191+++ src/lazr/restful/docs/webservice-declarations.txt 2011-01-21 21:12:04 +0000
192@@ -98,6 +98,23 @@
193 ...
194 TypeError: exported() can only be used on IFields.
195
196+Object fields cannot be exported because they cause validation problems.
197+
198+ >>> from zope.schema import Object
199+ >>> class UsesIObject(Interface):
200+ ... export_as_webservice_entry()
201+ ... object = exported(Object(schema=Interface))
202+ Traceback (most recent call last):
203+ TypeError: Object exported; use Reference instead.
204+
205+Instead you should use Reference, a subclass of Object designed to
206+avoid the validation problems.
207+
208+ >>> from lazr.restful.fields import Reference
209+ >>> class UsesIReference(Interface):
210+ ... export_as_webservice_entry()
211+ ... object = exported(Reference(schema=Interface))
212+
213 In the same vein, export_as_webservice_entry() can only be used on
214 Interface.
215
216
217=== modified file 'src/lazr/restful/docs/webservice.txt'
218--- src/lazr/restful/docs/webservice.txt 2010-10-25 13:48:07 +0000
219+++ src/lazr/restful/docs/webservice.txt 2011-01-21 21:12:04 +0000
220@@ -22,7 +22,8 @@
221 >>> __metaclass__ = type
222
223 >>> from zope.interface import Interface, Attribute
224- >>> from zope.schema import Bool, Bytes, Int, Text, TextLine, Object
225+ >>> from zope.schema import Bool, Bytes, Int, Text, TextLine
226+ >>> from lazr.restful.fields import Reference
227
228 >>> class ITestDataObject(Interface):
229 ... """A marker interface for data objects."""
230@@ -33,7 +34,7 @@
231 ... name = TextLine(title=u"Name", required=True)
232 ... # favorite_recipe.schema will be set to IRecipe once
233 ... # IRecipe is defined.
234- ... favorite_recipe = Object(schema=Interface)
235+ ... favorite_recipe = Reference(schema=Interface)
236 ... popularity = Int(readonly=True)
237
238 >>> class ICommentTarget(ITestDataObject):
239@@ -45,7 +46,7 @@
240
241 >>> class ICookbook(ICommentTarget):
242 ... name = TextLine(title=u"Name", required=True)
243- ... author = Object(schema=IAuthor)
244+ ... author = Reference(schema=IAuthor)
245 ... cuisine = TextLine(title=u"Cuisine", required=False, default=None)
246 ... recipes = Attribute("List of recipes published in this cookbook.")
247 ... cover = Bytes(0, 5000, title=u"An image of the cookbook's cover.")
248@@ -60,8 +61,8 @@
249
250 >>> class IRecipe(ICommentTarget):
251 ... id = Int(title=u"Unique ID", required=True)
252- ... dish = Object(schema=IDish)
253- ... cookbook = Object(schema=ICookbook)
254+ ... dish = Reference(schema=IDish)
255+ ... cookbook = Reference(schema=ICookbook)
256 ... instructions = Text(title=u"How to prepare the recipe.",
257 ... required=True)
258 ... private = Bool(title=u"Whether the public can see this recipe.",
259@@ -579,7 +580,7 @@
260 >>> from lazr.restful.fields import CollectionField
261 >>> class IDishEntry(IEntry):
262 ... "The part of a dish that we expose through the web service."
263- ... recipes = CollectionField(value_type=Object(schema=IRecipe))
264+ ... recipes = CollectionField(value_type=Reference(schema=IRecipe))
265 ... taggedValue(LAZR_WEBSERVICE_NAME,
266 ... dict(singular="dish", plural="dishes"))
267
268@@ -590,10 +591,10 @@
269
270 >>> class IRecipeEntry(IEntry):
271 ... "The part of a recipe that we expose through the web service."
272- ... cookbook = Object(schema=ICookbook)
273- ... dish = Object(schema=IDish)
274+ ... cookbook = Reference(schema=ICookbook)
275+ ... dish = Reference(schema=IDish)
276 ... instructions = Text(title=u"Name", required=True)
277- ... comments = CollectionField(value_type=Object(schema=IComment))
278+ ... comments = CollectionField(value_type=Reference(schema=IComment))
279 ... taggedValue(LAZR_WEBSERVICE_NAME,
280 ... dict(singular="recipe", plural="recipes"))
281
282@@ -603,8 +604,8 @@
283 ... cuisine = TextLine(title=u"Cuisine", required=False, default=None)
284 ... author = ReferenceChoice(
285 ... schema=IAuthor, vocabulary=AuthorVocabulary())
286- ... recipes = CollectionField(value_type=Object(schema=IRecipe))
287- ... comments = CollectionField(value_type=Object(schema=IComment))
288+ ... recipes = CollectionField(value_type=Reference(schema=IRecipe))
289+ ... comments = CollectionField(value_type=Reference(schema=IComment))
290 ... cover = Bytes(0, 5000, title=u"An image of the cookbook's cover.")
291 ... taggedValue(LAZR_WEBSERVICE_NAME,
292 ... dict(singular="cookbook", plural="cookbooks"))
293@@ -962,7 +963,7 @@
294 ... TextLine(
295 ... __name__='cuisine', default=u'Brazilian', required=False),
296 ... )
297- ... return_type = Object(schema=IRecipe)
298+ ... return_type = Reference(schema=IRecipe)
299 ...
300 ... def call(self, author_name, title, cuisine):
301 ... cookbook = CookbookSet().newCookbook(
302
303=== modified file 'src/lazr/restful/marshallers.py'
304--- src/lazr/restful/marshallers.py 2010-10-25 16:31:09 +0000
305+++ src/lazr/restful/marshallers.py 2011-01-21 21:12:04 +0000
306@@ -525,7 +525,7 @@
307 """A marshaller that turns URLs into data model objects.
308
309 This marshaller can be used with a IChoice field (initialized
310- with a vocabulary) or with an IObject field (no vocabulary).
311+ with a vocabulary) or with an IReference field (no vocabulary).
312 """
313
314 def __init__(self, field, request, vocabulary=None):
315
316=== modified file 'src/lazr/restful/publisher.py'
317--- src/lazr/restful/publisher.py 2011-01-17 23:48:11 +0000
318+++ src/lazr/restful/publisher.py 2011-01-21 21:12:04 +0000
319@@ -25,7 +25,7 @@
320 from zope.interface import alsoProvides, implementer, implements
321 from zope.publisher.interfaces import NotFound
322 from zope.publisher.interfaces.browser import IBrowserRequest
323-from zope.schema.interfaces import IBytes, IObject
324+from zope.schema.interfaces import IBytes
325 from zope.security.checker import ProxyFactory
326
327 from lazr.uri import URI
328@@ -35,8 +35,9 @@
329 EntryResource, ScopedCollection, ServiceRootResource)
330 from lazr.restful.interfaces import (
331 IByteStorage, ICollection, ICollectionField, IEntry, IEntryField,
332- IHTTPResource, IServiceRootResource, IWebBrowserInitiatedRequest,
333- IWebServiceClientRequest, IWebServiceConfiguration, IWebServiceVersion)
334+ IHTTPResource, IReference, IServiceRootResource,
335+ IWebBrowserInitiatedRequest, IWebServiceClientRequest,
336+ IWebServiceConfiguration, IWebServiceVersion)
337 from lazr.restful.utils import tag_request_with_version_name
338
339
340@@ -83,7 +84,7 @@
341 elif IBytes.providedBy(field):
342 return self._traverseToByteStorage(
343 request, entry, field, name)
344- elif IObject.providedBy(field):
345+ elif IReference.providedBy(field):
346 sub_entry = getattr(entry, name, None)
347 if sub_entry is None:
348 raise NotFound(ob, name, request)
349
350=== modified file 'src/lazr/restful/tales.py'
351--- src/lazr/restful/tales.py 2011-01-19 19:00:10 +0000
352+++ src/lazr/restful/tales.py 2011-01-21 21:12:04 +0000
353@@ -25,7 +25,13 @@
354 from zope.interface import implements
355 from zope.interface.interfaces import IInterface
356 from zope.schema import getFields
357-from zope.schema.interfaces import IBytes, IChoice, IDate, IDatetime, IObject
358+from zope.schema.interfaces import (
359+ IBytes,
360+ IChoice,
361+ IDate,
362+ IDatetime,
363+ IObject,
364+ )
365 from zope.security.proxy import removeSecurityProxy
366 from zope.publisher.interfaces.browser import IBrowserRequest
367 from zope.traversing.browser import absoluteURL
368@@ -38,7 +44,7 @@
369 from lazr.restful._resource import UnknownEntryAdapter
370 from lazr.restful.interfaces import (
371 ICollection, ICollectionField, IEntry, IJSONRequestCache,
372- IReferenceChoice, IResourceDELETEOperation, IResourceGETOperation,
373+ IReference, IResourceDELETEOperation, IResourceGETOperation,
374 IResourceOperation, IResourcePOSTOperation, IScopedCollection,
375 ITopLevelEntryLink, IWebServiceClientRequest, IWebServiceConfiguration,
376 IWebServiceVersion, LAZR_WEBSERVICE_NAME)
377@@ -512,8 +518,8 @@
378 name = self.field.__name__
379 if ICollectionField.providedBy(self.field):
380 return name + '_collection_link'
381- elif (IObject.providedBy(self.field) or IBytes.providedBy(self.field)
382- or IReferenceChoice.providedBy(self.field)):
383+ elif (IReference.providedBy(self.field)
384+ or IBytes.providedBy(self.field)):
385 return name + '_link'
386 else:
387 return name
388@@ -548,8 +554,7 @@
389 @property
390 def is_represented_as_link(self):
391 """Is this field represented as a link to another resource?"""
392- return (IObject.providedBy(self.field) or
393- IReferenceChoice.providedBy(self.field) or
394+ return (IReference.providedBy(self.field) or
395 ICollectionField.providedBy(self.field) or
396 IBytes.providedBy(self.field) or
397 self.is_link)
398@@ -582,9 +587,8 @@
399 """Find an entry adapter for this field."""
400 if ICollectionField.providedBy(self.field):
401 schema = self.field.value_type.schema
402- elif (IObject.providedBy(self.field)
403- or IObjectLink.providedBy(self.field)
404- or IReferenceChoice.providedBy(self.field)):
405+ elif (IReference.providedBy(self.field)
406+ or IObjectLink.providedBy(self.field)):
407 schema = self.field.schema
408 else:
409 raise TypeError("Field is not of a supported type.")
410
411=== modified file 'src/lazr/restful/tests/test_declarations.py'
412--- src/lazr/restful/tests/test_declarations.py 2010-11-04 18:21:58 +0000
413+++ src/lazr/restful/tests/test_declarations.py 2011-01-21 21:12:04 +0000
414@@ -13,6 +13,7 @@
415 export_as_webservice_entry, exported, export_read_operation,
416 export_write_operation, mutator_for, operation_for_version,
417 operation_parameters)
418+from lazr.restful.fields import Reference
419 from lazr.restful.interfaces import (
420 IEntry, IResourceGETOperation, IWebServiceConfiguration,
421 IWebServiceVersion)
422@@ -306,7 +307,7 @@
423 export_as_webservice_entry(contributes_to=[IProduct])
424 not_exported = TextLine(title=u'Not exported')
425 development_branch = exported(
426- Object(schema=IBranch, readonly=True),
427+ Reference(schema=IBranch, readonly=True),
428 ('1.0', dict(exported_as='development_branch_10')),
429 ('beta', dict(exported_as='development_branch')))
430
431
432=== modified file 'src/lazr/restful/tests/test_navigation.py'
433--- src/lazr/restful/tests/test_navigation.py 2010-01-11 17:45:07 +0000
434+++ src/lazr/restful/tests/test_navigation.py 2011-01-21 21:12:04 +0000
435@@ -9,10 +9,14 @@
436 from zope.component import getSiteManager
437 from zope.interface import Interface, implements
438 from zope.publisher.interfaces import NotFound
439-from zope.schema import Text, Object
440+from zope.schema import Text
441 from zope.testing.cleanup import cleanUp
442
443-from lazr.restful.interfaces import IEntry, IWebServiceClientRequest
444+from lazr.restful.fields import Reference
445+from lazr.restful.interfaces import (
446+ IEntry,
447+ IWebServiceClientRequest,
448+ )
449 from lazr.restful.simple import Publication
450 from lazr.restful.testing.webservice import FakeRequest
451
452@@ -26,7 +30,7 @@
453 class IParent(Interface):
454 """Interface for a simple entry that contains another entry."""
455 three = Text(title=u'Three')
456- child = Object(schema=IChild)
457+ child = Reference(schema=IChild)
458
459
460 class Child:
461
462=== modified file 'src/lazr/restful/version.txt'
463--- src/lazr/restful/version.txt 2011-01-20 20:25:31 +0000
464+++ src/lazr/restful/version.txt 2011-01-21 21:12:04 +0000
465@@ -1,1 +1,1 @@
466-0.15.2
467+0.15.3

Subscribers

People subscribed via source and target branches