Merge lp:~benji/lazr.restful/size-link into lp:lazr.restful
- size-link
- Merge into trunk
Status: | Merged |
---|---|
Merge reported by: | Leonard Richardson |
Merged at revision: | not available |
Proposed branch: | lp:~benji/lazr.restful/size-link |
Merge into: | lp:lazr.restful |
Prerequisite: | lp:~benji/lazr.restful/nail-versions |
Diff against target: |
1106 lines (+420/-89) 21 files modified
setup.py (+1/-1) src/lazr/restful/NEWS.txt (+9/-0) src/lazr/restful/_operation.py (+28/-2) src/lazr/restful/_resource.py (+48/-18) src/lazr/restful/declarations.py (+19/-12) src/lazr/restful/docs/webservice-declarations.txt (+89/-21) src/lazr/restful/docs/webservice.txt (+1/-0) src/lazr/restful/example/base/root.py (+1/-0) src/lazr/restful/example/base/tests/collection.txt (+23/-8) src/lazr/restful/example/base/tests/wadl.txt (+6/-5) src/lazr/restful/example/multiversion/tests/wadl.txt (+29/-0) src/lazr/restful/interfaces/_rest.py (+7/-0) src/lazr/restful/simple.py (+2/-1) src/lazr/restful/tales.py (+7/-1) src/lazr/restful/templates/wadl-root.pt (+31/-8) src/lazr/restful/testing/webservice.py (+3/-0) src/lazr/restful/tests/test_utils.py (+23/-1) src/lazr/restful/tests/test_webservice.py (+78/-2) src/lazr/restful/utils.py (+12/-0) src/lazr/restful/version.txt (+1/-5) versions.cfg (+2/-4) |
To merge this branch: | bzr merge lp:~benji/lazr.restful/size-link |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Leonard Richardson (community) | Needs Fixing | ||
Review via email: mp+31971@code.launchpad.net |
Commit message
Description of the change
Bug 590708 revealed that the web service often calls len() on
collections when it is both expensive and unnecessary (leading to bug
613555).
This branch modifies lazr.restful to defer calling len() when possible
and instead expose a URL at which the total size of the collection can
be retrieved.
This necessitated changing lazr.batchnavigator to call len() less often
(branch lp:~benji/lazr.batchnavigator/optimize-last-batch-len) and
lazr.restfulclient (branch lp:~benji/lazr.restfulclient/total_size_link)
to be able to handle the new behavior.
Launchpad has been modified to take advantage of this behavior as well
(no public branch yet).
The modified Launchpad has been tested against the trunk version of
Apport to ensure continued interoperation.
The approach and particulars have been discussed at length with Leonard
and Gary.
Gary Poster (gary) wrote : | # |
Benji York (benji) wrote : | # |
On Fri, Aug 6, 2010 at 1:49 PM, Gary Poster <email address hidden> wrote:
> The NEWS file has both the British and American spellings of "behavior."
> Officially, Canonical uses British spellings. I generally stick with
> American spellings for lazr packages, since that's easier for all or most of
> the main contributors they have. At the very least, for one NEWS entry, I'd
> like to stick to one approach. :-)
You can colour me honoured by your skilful counselling.
> The name "total_size_only" appears several times. Its behavior is not
> clear to me. The ``if not self.total_
> in _operations.py particularly confuse me. I'd like to see some
> comments, and I'd like it explained in appropriate docstrings. It
> also makes me question the name in batchnavigator, TBH, but maybe
> there's not a better option. What does it mean?
I'll try to come up with a better name. The "if not" bit is especially
hard to grok.
--
Benji York
- 152. By Benji York
-
'mericanificate spelling
Leonard Richardson (leonardr) wrote : | # |
Needs some minor changes (beyond the refactoring we discussed in IRC, which I approve of.)
1. You should make is_total_
2. Since we are no longer publishing total_size_link on a per-operation basis, the example named operation searchBookText serves no purpose. Remove it. Also remove IBook.text.
3. Line 588 is no longer accurate, since all named operations behave the same way in a given version.
4. Similarly, 'total_size' should not be included in the WADL at all for a version that doesn't use it. (However, I would like to see a follow-up branch that included total_size instead of total_size_link in cases where the entire list fits in one batch. So don't change this, as total_size will never really go away totally.)
5. You have a minor conflict in version.txt.
Stuart Bishop (stub) wrote : | # |
On Sat, Aug 7, 2010 at 12:49 AM, Gary Poster <email address hidden> wrote:
> The NEWS file has both the British and American spellings of "behavior." Officially, Canonical uses British spellings. I generally stick with American spellings for lazr packages, since that's easier for all or most of the main contributors they have. At the very least, for one NEWS entry, I'd like to stick to one approach. :-)
We do? We have always used American for code...
--
Stuart Bishop <email address hidden>
http://
- 153. By Benji York
-
add tests for total_size_only in preparation for refactoring it into its own
method; also fixed a bug that the tests revealed - 154. By Benji York
-
checkpoint
- 155. By Benji York
-
checkpoint
- 156. By Benji York
-
checkpoint
- 157. By Benji York
-
be willing to use the -dev version of lazr.batchnavigator 1.2.0
- 158. By Benji York
-
merge in Leonard's work on addressing review comments
(lp:~leonardr/lazr.restful/benji-size-link)
Benji York (benji) wrote : | # |
On Fri, Aug 6, 2010 at 2:57 PM, Leonard Richardson
<email address hidden> wrote:
> Review: Needs Fixing
> Needs some minor changes (beyond the refactoring we discussed in IRC, which I approve of.)
The refactoring (extracting the get_total_size method) has been done.
> 1. You should make is_total_
> WebserviceReque
> TALES namespace on its own.
>
> 2. Since we are no longer publishing total_size_link on a
> per-operation basis, the example named operation searchBookText serves
> no purpose. Remove it. Also remove IBook.text.
>
> 3. Line 588 is no longer accurate, since all named operations behave
> the same way in a given version.
Leonard fixed the three above.
> 4. Similarly, 'total_size' should not be included in the WADL at all
> for a version that doesn't use it. (However, I would like to see a
> follow-up branch that included total_size instead of total_size_link
> in cases where the entire list fits in one batch. So don't change
> this, as total_size will never really go away totally.)
We discussed this today and agree that there's nothing to do since we
want to integrate lp:~benji/lazr.batchnavigator/optimize-last-batch-len
which will require the current behavior.
> 5. You have a minor conflict in version.txt.
I tried another merge/push but I don't think it fixed it. Ideas?
--
Benji York
Preview Diff
1 | === modified file 'setup.py' |
2 | --- setup.py 2010-03-04 14:27:58 +0000 |
3 | +++ setup.py 2010-08-09 20:39:47 +0000 |
4 | @@ -57,7 +57,7 @@ |
5 | 'docutils', |
6 | 'epydoc', # used by wadl generation |
7 | 'grokcore.component==1.6', |
8 | - 'lazr.batchnavigator', |
9 | + 'lazr.batchnavigator>=1.2.0-dev', |
10 | 'lazr.delegates', |
11 | 'lazr.enum', |
12 | 'lazr.lifecycle', |
13 | |
14 | === modified file 'src/lazr/restful/NEWS.txt' |
15 | --- src/lazr/restful/NEWS.txt 2010-08-05 14:01:44 +0000 |
16 | +++ src/lazr/restful/NEWS.txt 2010-08-09 20:39:47 +0000 |
17 | @@ -2,6 +2,15 @@ |
18 | NEWS for lazr.restful |
19 | ===================== |
20 | |
21 | +0.11.0 (unreleased) |
22 | +=================== |
23 | + |
24 | +Added an optimization to total_size so that it is fetched via a link when |
25 | +possible. The new configuration option first_version_with_total_size_link |
26 | +specifies what version should be the first to expose the behavior. The default |
27 | +is for it to be enabled for all versions so set this option to preserve the |
28 | +earlier behavior for previously released web services. |
29 | + |
30 | 0.10.0 (2010-08-05) |
31 | =================== |
32 | |
33 | |
34 | === modified file 'src/lazr/restful/_operation.py' |
35 | --- src/lazr/restful/_operation.py 2010-04-20 18:26:49 +0000 |
36 | +++ src/lazr/restful/_operation.py 2010-08-09 20:39:47 +0000 |
37 | @@ -44,6 +44,23 @@ |
38 | def __init__(self, context, request): |
39 | self.context = context |
40 | self.request = request |
41 | + self.total_size_only = False |
42 | + |
43 | + def total_size_link(self, navigator): |
44 | + """Return a link to the total size of a collection.""" |
45 | + if getattr(self, 'include_total_size', True): |
46 | + # This is a named operation that includes the total size |
47 | + # inline rather than with a link. |
48 | + return None |
49 | + if not IResourceGETOperation.providedBy(self): |
50 | + # Only GET operations can have their total size split out into |
51 | + # a link, because only GET operations are safe. |
52 | + return None |
53 | + base = str(self.request.URL) |
54 | + query = navigator.getCleanQueryString() |
55 | + if query != '': |
56 | + query += '&' |
57 | + return base + '?' + query + "ws.show=total_size" |
58 | |
59 | def __call__(self): |
60 | values, errors = self.validate() |
61 | @@ -80,13 +97,22 @@ |
62 | # this object served to the client. |
63 | return result |
64 | |
65 | + # The similar patterns in the two branches below suggest some deeper |
66 | + # symmetry that should be extracted. |
67 | if queryMultiAdapter((result, self.request), ICollection): |
68 | # If the result is a web service collection, serve only one |
69 | # batch of the collection. |
70 | collection = getMultiAdapter((result, self.request), ICollection) |
71 | - result = CollectionResource(collection, self.request).batch() + '}' |
72 | + resource = CollectionResource(collection, self.request) |
73 | + if self.total_size_only: |
74 | + result = resource.get_total_size(collection) |
75 | + else: |
76 | + result = resource.batch() + '}' |
77 | elif self.should_batch(result): |
78 | - result = self.batch(result, self.request) + '}' |
79 | + if self.total_size_only: |
80 | + result = self.get_total_size(result) |
81 | + else: |
82 | + result = self.batch(result, self.request) + '}' |
83 | else: |
84 | # Serialize the result to JSON. Any embedded entries will be |
85 | # automatically serialized. |
86 | |
87 | === modified file 'src/lazr/restful/_resource.py' |
88 | --- src/lazr/restful/_resource.py 2010-08-04 18:25:21 +0000 |
89 | +++ src/lazr/restful/_resource.py 2010-08-09 20:39:47 +0000 |
90 | @@ -110,6 +110,7 @@ |
91 | status_reasons[code] = reason |
92 | init_status_codes() |
93 | |
94 | + |
95 | def decode_value(value): |
96 | """Return a unicode value curresponding to `value`.""" |
97 | if isinstance(value, unicode): |
98 | @@ -589,27 +590,45 @@ |
99 | |
100 | """A mixin for resources that need to batch lists of entries.""" |
101 | |
102 | - def __init__(self, context, request): |
103 | - """A basic constructor.""" |
104 | - # Like all mixin classes, this class is designed to be used |
105 | - # with multiple inheritance. That requires defining __init__ |
106 | - # to call the next constructor in the chain, which means using |
107 | - # super() even though this class itself has no superclass. |
108 | - super(BatchingResourceMixin, self).__init__(context, request) |
109 | + # TODO: determine real need for __init__ and super() call |
110 | + |
111 | + def total_size_link(self, navigator): |
112 | + """Return the URL to fetch to find out the collection's total size. |
113 | + |
114 | + If this is None, the total size will be included inline. |
115 | + |
116 | + :param navigator: A BatchNavigator object for the current batch. |
117 | + """ |
118 | + return None |
119 | + |
120 | + def get_total_size(self, entries): |
121 | + """Get the number of items in entries. |
122 | + |
123 | + :return: a JSON string representing the number of objects in the list |
124 | + """ |
125 | + if not hasattr(entries, '__len__'): |
126 | + entries = IFiniteSequence(entries) |
127 | + |
128 | + return simplejson.dumps(len(entries)) |
129 | + |
130 | |
131 | def batch(self, entries, request): |
132 | """Prepare a batch from a (possibly huge) list of entries. |
133 | |
134 | - :return: A JSON string representing a hash: |
135 | + :return: a JSON string representing a hash: |
136 | + |
137 | 'entries' contains a list of EntryResource objects for the |
138 | entries that actually made it into this batch |
139 | 'total_size' contains the total size of the list. |
140 | + 'total_size_link' contains a link to the total size of the list. |
141 | 'next_url', if present, contains a URL to get the next batch |
142 | in the list. |
143 | 'prev_url', if present, contains a URL to get the previous batch |
144 | in the list. |
145 | 'start' contains the starting index of this batch |
146 | |
147 | + Only one of 'total_size' or 'total_size_link' will be present. |
148 | + |
149 | Note that the JSON string will be missing its final curly |
150 | brace. This is in case the caller wants to add some additional |
151 | keys to the JSON hash. It's the caller's responsibility to add |
152 | @@ -620,11 +639,12 @@ |
153 | navigator = WebServiceBatchNavigator(entries, request) |
154 | |
155 | view_permission = getUtility(IWebServiceConfiguration).view_permission |
156 | - resources = [EntryResource(entry, request) |
157 | - for entry in navigator.batch |
158 | - if checkPermission(view_permission, entry)] |
159 | - batch = { 'total_size' : navigator.batch.listlength, |
160 | - 'start' : navigator.batch.start } |
161 | + batch = { 'start' : navigator.batch.start } |
162 | + total_size_link = self.total_size_link(navigator) |
163 | + if total_size_link is None: |
164 | + batch['total_size'] = navigator.batch.listlength |
165 | + else: |
166 | + batch['total_size_link'] = total_size_link |
167 | if navigator.batch.start < 0: |
168 | batch['start'] = None |
169 | next_url = navigator.nextBatchURL() |
170 | @@ -637,6 +657,9 @@ |
171 | |
172 | # String together a bunch of entry representations, possibly |
173 | # obtained from a representation cache. |
174 | + resources = [EntryResource(entry, request) |
175 | + for entry in navigator.batch |
176 | + if checkPermission(view_permission, entry)] |
177 | entry_strings = [ |
178 | resource._representation(HTTPResource.JSON_TYPE) |
179 | for resource in resources] |
180 | @@ -674,6 +697,10 @@ |
181 | except ComponentLookupError: |
182 | self.request.response.setStatus(400) |
183 | return "No such operation: " + operation_name |
184 | + |
185 | + show = self.request.form.get('ws.show') |
186 | + if show == 'total_size': |
187 | + operation.total_size_only = True |
188 | return operation() |
189 | |
190 | def handleCustomPOST(self, operation_name): |
191 | @@ -1664,14 +1691,17 @@ |
192 | self.request.response.setHeader('Content-type', self.JSON_TYPE) |
193 | return result |
194 | |
195 | - def batch(self, entries=None): |
196 | + def batch(self, entries=None, request=None): |
197 | """Return a JSON representation of a batch of entries. |
198 | |
199 | :param entries: (Optional) A precomputed list of entries to batch. |
200 | + :param request: (Optional) The current request. |
201 | """ |
202 | if entries is None: |
203 | entries = self.collection.find() |
204 | - result = super(CollectionResource, self).batch(entries, self.request) |
205 | + if request is None: |
206 | + request = self.request |
207 | + result = super(CollectionResource, self).batch(entries, request) |
208 | result += ( |
209 | ', "resource_type_link" : ' + simplejson.dumps(self.type_url) |
210 | + '}') |
211 | @@ -1862,7 +1892,7 @@ |
212 | # by the entry classes. |
213 | collection_classes.append(registration.factory) |
214 | namespace = self.WADL_TEMPLATE.pt_getContext() |
215 | - namespace['context'] = self |
216 | + namespace['service'] = self |
217 | namespace['request'] = self.request |
218 | namespace['entries'] = entry_classes |
219 | namespace['collections'] = collection_classes |
220 | @@ -2061,13 +2091,13 @@ |
221 | def singular_type(self): |
222 | """Return the singular name for this object type.""" |
223 | interface = self.entry_interface |
224 | - return interface.queryTaggedValue(LAZR_WEBSERVICE_NAME)['singular'] |
225 | + return interface.getTaggedValue(LAZR_WEBSERVICE_NAME)['singular'] |
226 | |
227 | @property |
228 | def plural_type(self): |
229 | """Return the plural name for this object type.""" |
230 | interface = self.entry_interface |
231 | - return interface.queryTaggedValue(LAZR_WEBSERVICE_NAME)['plural'] |
232 | + return interface.getTaggedValue(LAZR_WEBSERVICE_NAME)['plural'] |
233 | |
234 | @property |
235 | def type_link(self): |
236 | |
237 | === modified file 'src/lazr/restful/declarations.py' |
238 | --- src/lazr/restful/declarations.py 2010-08-04 18:25:21 +0000 |
239 | +++ src/lazr/restful/declarations.py 2010-08-09 20:39:47 +0000 |
240 | @@ -63,7 +63,7 @@ |
241 | from lazr.restful.security import protect_schema |
242 | from lazr.restful.utils import ( |
243 | camelcase_to_underscore_separated, get_current_web_service_request, |
244 | - make_identifier_safe, VersionedDict) |
245 | + make_identifier_safe, VersionedDict, is_total_size_link_active) |
246 | |
247 | LAZR_WEBSERVICE_EXPORTED = '%s.exported' % LAZR_WEBSERVICE_NS |
248 | LAZR_WEBSERVICE_MUTATORS = '%s.exported.mutators' % LAZR_WEBSERVICE_NS |
249 | @@ -712,10 +712,9 @@ |
250 | class operation_returns_collection_of(_method_annotator): |
251 | """Specify that the exported operation returns a collection. |
252 | |
253 | - The decorator takes a single argument: an interface that's been |
254 | - exported as an entry. |
255 | + The decorator takes one required argument, "schema", an interface that's |
256 | + been exported as an entry. |
257 | """ |
258 | - |
259 | def __init__(self, schema): |
260 | _check_called_from_interface_def('%s()' % self.__class__.__name__) |
261 | if not IInterface.providedBy(schema): |
262 | @@ -1209,23 +1208,22 @@ |
263 | version = getUtility(IWebServiceConfiguration).active_versions[0] |
264 | |
265 | bases = (BaseResourceOperationAdapter, ) |
266 | - if tag['type'] == 'read_operation': |
267 | + operation_type = tag['type'] |
268 | + if operation_type == 'read_operation': |
269 | prefix = 'GET' |
270 | provides = IResourceGETOperation |
271 | - elif tag['type'] in ('factory', 'write_operation'): |
272 | + elif operation_type in ('factory', 'write_operation'): |
273 | provides = IResourcePOSTOperation |
274 | prefix = 'POST' |
275 | - if tag['type'] == 'factory': |
276 | + if operation_type == 'factory': |
277 | bases = (BaseFactoryResourceOperationAdapter,) |
278 | - elif tag['type'] == 'destructor': |
279 | + elif operation_type == 'destructor': |
280 | provides = IResourceDELETEOperation |
281 | prefix = 'DELETE' |
282 | else: |
283 | - raise AssertionError('Unknown method export type: %s' % tag['type']) |
284 | + raise AssertionError('Unknown method export type: %s' % operation_type) |
285 | |
286 | return_type = tag['return_type'] |
287 | - if return_type is None: |
288 | - return_type = None |
289 | |
290 | name = _versioned_class_name( |
291 | '%s_%s_%s' % (prefix, method.interface.__name__, tag['as']), |
292 | @@ -1238,7 +1236,16 @@ |
293 | '_method_name': method.__name__, |
294 | '__doc__': method.__doc__} |
295 | |
296 | - if tag['type'] == 'write_operation': |
297 | + if isinstance(return_type, CollectionField): |
298 | + # If the version we're being asked for is equal to or later than the |
299 | + # version in which we started exposing total_size_link and this is a |
300 | + # read operation, then include it, otherwise include total_size. |
301 | + config = getUtility(IWebServiceConfiguration) |
302 | + class_dict['include_total_size'] = not ( |
303 | + is_total_size_link_active(version, config) and |
304 | + operation_type == 'read_operation') |
305 | + |
306 | + if operation_type == 'write_operation': |
307 | class_dict['send_modification_event'] = True |
308 | factory = type(name, bases, class_dict) |
309 | classImplements(factory, provides) |
310 | |
311 | === modified file 'src/lazr/restful/docs/webservice-declarations.txt' |
312 | --- src/lazr/restful/docs/webservice-declarations.txt 2010-08-02 20:10:33 +0000 |
313 | +++ src/lazr/restful/docs/webservice-declarations.txt 2010-08-09 20:39:47 +0000 |
314 | @@ -32,7 +32,7 @@ |
315 | field, but not the inventory_number field. |
316 | |
317 | >>> from zope.interface import Interface |
318 | - >>> from zope.schema import TextLine, Float, List |
319 | + >>> from zope.schema import Text, TextLine, Float, List |
320 | >>> from lazr.restful.declarations import ( |
321 | ... export_as_webservice_entry, exported) |
322 | >>> class IBook(Interface): |
323 | @@ -48,6 +48,8 @@ |
324 | ... exported_as='price') |
325 | ... |
326 | ... inventory_number = TextLine(title=u'The inventory part number.') |
327 | + ... |
328 | + ... text = exported(Text(title=u'The text of the book')) |
329 | |
330 | These declarations add tagged values to the original interface elements. |
331 | The tags are in the lazr.restful namespace and are dictionaries of |
332 | @@ -213,7 +215,7 @@ |
333 | TypeError: export_as_webservice_collection() is missing a method |
334 | tagged with @collection_default_content. |
335 | |
336 | -As it is an error, to mark more than one methods: |
337 | +As it is an error, to mark more than one method: |
338 | |
339 | >>> class TwoDefaultContent(Interface): |
340 | ... export_as_webservice_collection(IDummyInterface) |
341 | @@ -330,8 +332,15 @@ |
342 | ... text=copy_field(IBook['title'], title=u'Text to search for.')) |
343 | ... @operation_returns_collection_of(IBook) |
344 | ... @export_read_operation() |
345 | - ... def searchBooks(text): |
346 | - ... """Return list of books containing 'text'.""" |
347 | + ... def searchBookTitles(text): |
348 | + ... """Return list of books whose titles contain 'text'.""" |
349 | + ... |
350 | + ... @operation_parameters( |
351 | + ... text=copy_field(IBook['text'], title=u'Text to search for.')) |
352 | + ... @operation_returns_collection_of(IBook) |
353 | + ... @export_read_operation() |
354 | + ... def searchBookText(text): |
355 | + ... """Return list of books whose text contains 'text'.""" |
356 | ... |
357 | ... @operation_parameters( |
358 | ... text=copy_field(IBook['title'], title=u'Text to search for.')) |
359 | @@ -393,11 +402,11 @@ |
360 | return_type: <lazr.restful._operation.ObjectLink object...> |
361 | type: 'factory' |
362 | |
363 | -We did specify the return type for the 'searchBooks' method: it |
364 | +We did specify the return type for the 'searchBookTitles' method: it |
365 | returns a collection. |
366 | |
367 | - >>> print_export_tag(IBookSetOnSteroids['searchBooks']) |
368 | - as: 'searchBooks' |
369 | + >>> print_export_tag(IBookSetOnSteroids['searchBookTitles']) |
370 | + as: 'searchBookTitles' |
371 | call_with: {} |
372 | params: {'text': <...TextLine...>} |
373 | return_type: <lazr.restful.fields.CollectionField object...> |
374 | @@ -849,6 +858,7 @@ |
375 | >>> dump_entry_interface(entry_interface) |
376 | author: TextLine |
377 | price: Float |
378 | + text: Text |
379 | title: TextLine |
380 | |
381 | The field __name__ attribute contains the exported name: |
382 | @@ -920,9 +930,11 @@ |
383 | >>> class Book(object): |
384 | ... """Simple IBook implementation.""" |
385 | ... implements(IBook) |
386 | - ... def __init__(self, author, title, base_price, inventory_number): |
387 | + ... def __init__(self, author, title, text, base_price, |
388 | + ... inventory_number): |
389 | ... self.author = author |
390 | ... self.title = title |
391 | + ... self.text = text |
392 | ... self.base_price = base_price |
393 | ... self.inventory_number = inventory_number |
394 | |
395 | @@ -937,6 +949,7 @@ |
396 | >>> class MyWebServiceConfiguration(TestWebServiceConfiguration): |
397 | ... active_versions = ["beta", "1.0", "2.0", "3.0"] |
398 | ... last_version_with_mutator_named_operations = "1.0" |
399 | + ... first_version_with_total_size_link = "2.0" |
400 | ... code_revision = "1.0b" |
401 | ... default_batch_size = 50 |
402 | >>> provideUtility(MyWebServiceConfiguration(), IWebServiceConfiguration) |
403 | @@ -970,7 +983,7 @@ |
404 | IBookEntry. |
405 | |
406 | >>> entry_adapter = entry_adapter_factory( |
407 | - ... Book(u'Aldous Huxley', u'Island', 10.0, '12345'), |
408 | + ... Book(u'Aldous Huxley', u'Island', 'Text here', 10.0, '12345'), |
409 | ... request) |
410 | |
411 | >>> entry_adapter.schema is entry_interface |
412 | @@ -1087,7 +1100,7 @@ |
413 | ... generate_operation_adapter) |
414 | |
415 | >>> read_method_adapter_factory = generate_operation_adapter( |
416 | - ... IBookSetOnSteroids['searchBooks']) |
417 | + ... IBookSetOnSteroids['searchBookTitles']) |
418 | >>> IResourceGETOperation.implementedBy(read_method_adapter_factory) |
419 | True |
420 | |
421 | @@ -1099,14 +1112,14 @@ |
422 | |
423 | >>> from lazr.restful import ResourceOperation |
424 | >>> read_method_adapter_factory.__name__ |
425 | - 'GET_IBookSetOnSteroids_searchBooks_beta' |
426 | + 'GET_IBookSetOnSteroids_searchBookTitles_beta' |
427 | >>> issubclass(read_method_adapter_factory, ResourceOperation) |
428 | True |
429 | |
430 | The adapter's docstring is taken from the decorated method docstring. |
431 | |
432 | >>> read_method_adapter_factory.__doc__ |
433 | - "Return list of books containing 'text'." |
434 | + "Return list of books whose titles contain 'text'." |
435 | |
436 | The adapter's params attribute contains the specification of the |
437 | parameters accepted by the operation. |
438 | @@ -1126,11 +1139,15 @@ |
439 | ... |
440 | ... result = None |
441 | ... |
442 | - ... def searchBooks(self, text): |
443 | + ... def searchBookTitles(self, text): |
444 | + ... return self.result |
445 | + ... |
446 | + ... def searchBookText(self, text): |
447 | ... return self.result |
448 | ... |
449 | ... def new(self, author, base_price, title): |
450 | - ... return Book(author, title, base_price, "unknown") |
451 | + ... return Book(author, title, "default text", base_price, |
452 | + ... "unknown") |
453 | |
454 | Now we can create a fake request that invokes the named operation. |
455 | |
456 | @@ -1148,7 +1165,7 @@ |
457 | return value is a dictionary containing a batched list. |
458 | |
459 | >>> print read_method_adapter.call(text='') |
460 | - {"total_size": 0, "start": null, "entries": []} |
461 | + {"total_size": 0, "start": 0, "entries": []} |
462 | |
463 | Methods exported as a write operations generates an adapter providing |
464 | IResourcePOSTOperation. |
465 | @@ -1178,7 +1195,8 @@ |
466 | |
467 | >>> write_method_adapter = write_method_adapter_factory( |
468 | ... BookOnSteroids( |
469 | - ... 'Aldous Huxley', 'The Doors of Perception', 8, 'unknown'), |
470 | + ... 'Aldous Huxley', 'The Doors of Perception', |
471 | + ... 'The text', 8, 'unknown'), |
472 | ... FakeRequest()) |
473 | |
474 | >>> verifyObject(IResourcePOSTOperation, write_method_adapter) |
475 | @@ -1649,6 +1667,51 @@ |
476 | AssertionError: 'IMultiVersionCollection' isn't tagged for export |
477 | to web service version 'NoSuchVersion'. |
478 | |
479 | +total_size_link |
480 | +~~~~~~~~~~~~~~~ |
481 | + |
482 | +Collections previously exposed their total size via a `total_size` attribute. |
483 | +However, newer versions of lazr.restful expose a `total_size_link` intead. To |
484 | +facilitate transitioning from one approach to the other the configuration |
485 | +option `first_version_with_total_size_link` has been added to |
486 | +IWebServiceConfiguration. |
487 | + |
488 | +By default the first_version_with_total_size_link is set to the earliest |
489 | +available web service version, but if you have stable versions of your web |
490 | +service you wish to maintain compatability with you can specify the version in |
491 | +which you want the new behavior to take effect in the web service |
492 | +configuration. |
493 | + |
494 | + >>> from zope.component import getUtility |
495 | + >>> config = getUtility(IWebServiceConfiguration) |
496 | + >>> config.last_version_with_mutator_named_operations = '1.0' |
497 | + |
498 | + >>> from lazr.restful.declarations import ( |
499 | + ... export_read_operation, operation_returns_collection_of, |
500 | + ... operation_for_version) |
501 | + >>> class IWithMultiVersionCollection(Interface): |
502 | + ... export_as_webservice_entry() |
503 | + ... |
504 | + ... @operation_for_version('2.0') |
505 | + ... @operation_for_version('1.0') |
506 | + ... @operation_returns_collection_of(Interface) |
507 | + ... @export_read_operation() |
508 | + ... def method(): |
509 | + ... """A method that returns a collection.""" |
510 | + |
511 | + >>> method = IWithMultiVersionCollection['method'] |
512 | + >>> dummy_data = None # this would be an intance that has the method |
513 | + >>> v10 = generate_operation_adapter(method, '1.0')(dummy_data, request) |
514 | + >>> v20 = generate_operation_adapter(method, '2.0')(dummy_data, request) |
515 | + |
516 | +We can see that version 1.0 includes the total size for backward compatability |
517 | +while version 2.0 includes a link to fetch the total size. |
518 | + |
519 | + >>> v10.include_total_size |
520 | + True |
521 | + >>> v20.include_total_size |
522 | + False |
523 | + |
524 | Entries |
525 | ------- |
526 | |
527 | @@ -2477,8 +2540,8 @@ |
528 | # ProxyFactory wraps the content using the defined checker. |
529 | >>> print debug_proxy(ProxyFactory(entry_adapter)) |
530 | zope.security._proxy._Proxy (using zope.security.checker.Checker) |
531 | - public: author, price, schema, title |
532 | - public (set): author, price, schema, title |
533 | + public: author, price, schema, text, title |
534 | + public (set): author, price, schema, text, title |
535 | |
536 | >>> print debug_proxy(ProxyFactory(collection_adapter)) |
537 | zope.security._proxy._Proxy (using zope.security.checker.Checker) |
538 | @@ -2515,7 +2578,8 @@ |
539 | ICollection are available: |
540 | |
541 | >>> from zope.component import getMultiAdapter |
542 | - >>> book = Book(u'George Orwell', u'1984', 10.0, u'12345-1984') |
543 | + >>> book = Book(u'George Orwell', u'1984', 'Text here', 10.0, |
544 | + ... u'12345-1984') |
545 | >>> bookset = BookSet([book]) |
546 | |
547 | >>> entry_adapter = getMultiAdapter((book, request), IEntry) |
548 | @@ -2541,8 +2605,12 @@ |
549 | >>> request_interface = IWebServiceClientRequest |
550 | >>> adapter_registry.lookup( |
551 | ... (IBookSetOnSteroids, request_interface), |
552 | - ... IResourceGETOperation, 'searchBooks') |
553 | - <class '...GET_IBookSetOnSteroids_searchBooks_beta'> |
554 | + ... IResourceGETOperation, 'searchBookTitles') |
555 | + <class '...GET_IBookSetOnSteroids_searchBookTitles_beta'> |
556 | + >>> adapter_registry.lookup( |
557 | + ... (IBookSetOnSteroids, request_interface), |
558 | + ... IResourceGETOperation, 'searchBookText') |
559 | + <class '...GET_IBookSetOnSteroids_searchBookText_beta'> |
560 | >>> adapter_registry.lookup( |
561 | ... (IBookSetOnSteroids, request_interface), |
562 | ... IResourcePOSTOperation, 'create_book') |
563 | |
564 | === modified file 'src/lazr/restful/docs/webservice.txt' |
565 | --- src/lazr/restful/docs/webservice.txt 2010-08-04 18:25:21 +0000 |
566 | +++ src/lazr/restful/docs/webservice.txt 2010-08-09 20:39:47 +0000 |
567 | @@ -503,6 +503,7 @@ |
568 | ... code_revision = 'test' |
569 | ... max_batch_size = 100 |
570 | ... directives.publication_class(WebServiceTestPublication) |
571 | + ... first_version_with_total_size_link = 'devel' |
572 | |
573 | >>> from grokcore.component.testing import grok_component |
574 | >>> ignore = grok_component( |
575 | |
576 | === modified file 'src/lazr/restful/example/base/root.py' |
577 | --- src/lazr/restful/example/base/root.py 2010-06-14 14:09:31 +0000 |
578 | +++ src/lazr/restful/example/base/root.py 2010-08-09 20:39:47 +0000 |
579 | @@ -398,6 +398,7 @@ |
580 | <p>Don't use this unless you like changing things.</p>""" |
581 | } |
582 | last_version_with_mutator_named_operations = None |
583 | + first_version_with_total_size_link = None |
584 | use_https = False |
585 | view_permission = 'lazr.restful.example.base.View' |
586 | |
587 | |
588 | === modified file 'src/lazr/restful/example/base/tests/collection.txt' |
589 | --- src/lazr/restful/example/base/tests/collection.txt 2009-04-24 15:14:07 +0000 |
590 | +++ src/lazr/restful/example/base/tests/collection.txt 2010-08-09 20:39:47 +0000 |
591 | @@ -173,14 +173,12 @@ |
592 | ... "/cookbooks?ws.op=find_recipes&%s" % args).jsonBody() |
593 | |
594 | >>> s_recipes = search_recipes("chicken") |
595 | - >>> s_recipes['total_size'] |
596 | - 3 |
597 | >>> sorted(r['instructions'] for r in s_recipes['entries']) |
598 | [u'Draw, singe, stuff, and truss...', u'You can always judge...'] |
599 | |
600 | >>> veg_recipes = search_recipes("chicken", True) |
601 | - >>> veg_recipes['total_size'] |
602 | - 0 |
603 | + >>> veg_recipes['entries'] |
604 | + [] |
605 | |
606 | A custom operation that returns a list of objects is paginated, just |
607 | like a collection. |
608 | @@ -196,11 +194,27 @@ |
609 | empty list of results: |
610 | |
611 | >>> empty_collection = search_recipes("nosuchrecipe") |
612 | - >>> empty_collection['total_size'] |
613 | - 0 |
614 | >>> [r['instructions'] for r in empty_collection['entries']] |
615 | [] |
616 | |
617 | +Some named operations that return a collection include the total size |
618 | +of the collection inline. The 'search_recipes' operation includes a |
619 | +_link_ to the total size of the collection. |
620 | + |
621 | + >>> print s_recipes['total_size_link'] |
622 | + http://.../cookbooks?search=chicken&vegetarian=false&ws.op=find_recipes&ws.show=total_size |
623 | + >>> print webservice.get(s_recipes['total_size_link']) |
624 | + HTTP/1.1 200 Ok |
625 | + ... |
626 | + 3 |
627 | + |
628 | + |
629 | +Sending a GET request to that link yields a JSON representation of the |
630 | +total size. |
631 | + |
632 | + >>> print webservice.get(s_recipes['total_size_link']).jsonBody() |
633 | + 3 |
634 | + |
635 | Custom operations may have error handling. In this case, the error |
636 | handling is in the validate() method of the 'search' field. |
637 | |
638 | @@ -216,8 +230,9 @@ |
639 | |
640 | >>> general_cookbooks = webservice.get( |
641 | ... "/cookbooks?ws.op=find_for_cuisine&cuisine=General") |
642 | - >>> general_cookbooks.jsonBody()['total_size'] |
643 | - 3 |
644 | + >>> print general_cookbooks.jsonBody()['total_size_link'] |
645 | + http://cookbooks.dev/devel/cookbooks?cuisine=General&ws.op=find_for_cuisine&ws.show=total_size |
646 | + |
647 | |
648 | POST operations |
649 | =============== |
650 | |
651 | === modified file 'src/lazr/restful/example/base/tests/wadl.txt' |
652 | --- src/lazr/restful/example/base/tests/wadl.txt 2010-03-10 18:45:04 +0000 |
653 | +++ src/lazr/restful/example/base/tests/wadl.txt 2010-08-09 20:39:47 +0000 |
654 | @@ -537,8 +537,9 @@ |
655 | 'entry_resource_descriptions'. |
656 | |
657 | >>> entry_resource_descriptions = [] |
658 | - >>> entry_resource_types = other_children[first_entry_type_index:-2] |
659 | - >>> hosted_binary_resource_type, simple_binary_type = other_children[-2:] |
660 | + >>> entry_resource_types = other_children[first_entry_type_index:-3] |
661 | + >>> (hosted_binary_resource_type, scalar_type, simple_binary_type |
662 | + ... ) = other_children[-3:] |
663 | >>> for index in range(0, len(entry_resource_types), 5): |
664 | ... entry_resource_descriptions.append( |
665 | ... (tuple(entry_resource_types[index:index + 5]))) |
666 | @@ -1155,9 +1156,9 @@ |
667 | All collection representations have the same five <param> tags. |
668 | |
669 | >>> [param.attrib['name'] for param in collection_rep] |
670 | - ['resource_type_link', 'total_size', 'start', 'next_collection_link', |
671 | - 'prev_collection_link', 'entries', 'entry_links'] |
672 | - >>> (type_link, size, start, next, prev, entries, |
673 | + ['resource_type_link', 'total_size', 'total_size_link', 'start', |
674 | + 'next_collection_link', 'prev_collection_link', 'entries', 'entry_links'] |
675 | + >>> (type_link, size, size_link, start, next, prev, entries, |
676 | ... entry_links) = collection_rep |
677 | |
678 | So what's the difference between a collection of people and a |
679 | |
680 | === modified file 'src/lazr/restful/example/multiversion/tests/wadl.txt' |
681 | --- src/lazr/restful/example/multiversion/tests/wadl.txt 2010-03-10 20:44:03 +0000 |
682 | +++ src/lazr/restful/example/multiversion/tests/wadl.txt 2010-08-09 20:39:47 +0000 |
683 | @@ -92,3 +92,32 @@ |
684 | >>> pair_collection = contents['pair_collection'] |
685 | >>> sorted([method.attrib['id'] for method in pair_collection]) |
686 | ['key_value_pairs-get'] |
687 | + |
688 | +total_size_link |
689 | +=============== |
690 | + |
691 | +The version in which total_size_link is introduced is controlled by the |
692 | +first_version_with_total_size_link attribute of the web service configuration |
693 | +(IWebServiceConfiguration) utility. |
694 | + |
695 | +We'll configure the web service to begin including `total_size_link` values |
696 | +in version 3.0: |
697 | + |
698 | + >>> from zope.component import getUtility |
699 | + >>> from lazr.restful.interfaces import IWebServiceConfiguration |
700 | + >>> config = getUtility(IWebServiceConfiguration) |
701 | + >>> config.first_version_with_total_size_link = '3.0' |
702 | + |
703 | +Now if we request the WADL for 3.0 it will include a description of |
704 | +total_size_link. |
705 | + |
706 | + >>> webservice.get('/', media_type='application/vnd.sun.wadl+xml', |
707 | + ... api_version='3.0').body |
708 | + '...<wadl:param style="plain" name="total_size_link"...' |
709 | + |
710 | +If we request an earlier version, total_size_link is not described. |
711 | + |
712 | + >>> wadl = webservice.get('/', media_type='application/vnd.sun.wadl+xml', |
713 | + ... api_version='2.0').body |
714 | + >>> 'total_size_link' in wadl |
715 | + False |
716 | |
717 | === modified file 'src/lazr/restful/interfaces/_rest.py' |
718 | --- src/lazr/restful/interfaces/_rest.py 2010-06-03 14:42:44 +0000 |
719 | +++ src/lazr/restful/interfaces/_rest.py 2010-08-09 20:39:47 +0000 |
720 | @@ -507,6 +507,13 @@ |
721 | all subsequent versions, they will not be published as named |
722 | operations.""") |
723 | |
724 | + first_version_with_total_size_link = TextLine( |
725 | + default=None, |
726 | + description=u"""In earlier versions of lazr.restful collections |
727 | + included a total_size field, now they include a total_size_link |
728 | + instead. Setting this value determines in which version the new |
729 | + behavior takes effect.""") |
730 | + |
731 | code_revision = TextLine( |
732 | default=u"", |
733 | description=u"""A string designating the current revision |
734 | |
735 | === modified file 'src/lazr/restful/simple.py' |
736 | --- src/lazr/restful/simple.py 2010-06-14 18:47:39 +0000 |
737 | +++ src/lazr/restful/simple.py 2010-08-09 20:39:47 +0000 |
738 | @@ -477,5 +477,6 @@ |
739 | |
740 | |
741 | BaseWebServiceConfiguration = implement_from_dict( |
742 | - "BaseWebServiceConfiguration", IWebServiceConfiguration, {}, object) |
743 | + "BaseWebServiceConfiguration", IWebServiceConfiguration, |
744 | + {'first_version_with_total_size_link': None}, object) |
745 | |
746 | |
747 | === modified file 'src/lazr/restful/tales.py' |
748 | --- src/lazr/restful/tales.py 2010-03-10 20:44:03 +0000 |
749 | +++ src/lazr/restful/tales.py 2010-08-09 20:39:47 +0000 |
750 | @@ -34,7 +34,8 @@ |
751 | IResourceOperation, IResourcePOSTOperation, IScopedCollection, |
752 | ITopLevelEntryLink, IWebServiceClientRequest, IWebServiceConfiguration, |
753 | IWebServiceVersion, LAZR_WEBSERVICE_NAME) |
754 | -from lazr.restful.utils import get_current_web_service_request |
755 | +from lazr.restful.utils import (get_current_web_service_request, |
756 | + is_total_size_link_active) |
757 | |
758 | |
759 | class WadlDocstringLinker(DocstringLinker): |
760 | @@ -234,6 +235,11 @@ |
761 | return config.version_descriptions.get(self.service_version, None) |
762 | |
763 | @property |
764 | + def is_total_size_link_active(self): |
765 | + config = getUtility(IWebServiceConfiguration) |
766 | + return is_total_size_link_active(self.resource.request.version, config) |
767 | + |
768 | + @property |
769 | def top_level_resources(self): |
770 | """Return a list of dicts describing the top-level resources.""" |
771 | resource_dicts = [] |
772 | |
773 | === modified file 'src/lazr/restful/templates/wadl-root.pt' |
774 | --- src/lazr/restful/templates/wadl-root.pt 2010-03-10 18:50:27 +0000 |
775 | +++ src/lazr/restful/templates/wadl-root.pt 2010-08-09 20:39:47 +0000 |
776 | @@ -13,21 +13,21 @@ |
777 | |
778 | <wadl:doc xmlns="http://www.w3.org/1999/xhtml" |
779 | title="About this service" |
780 | - tal:content="structure context/wadl:description"> |
781 | + tal:content="structure service/wadl:description"> |
782 | Version-independent description of the web service. |
783 | </wadl:doc> |
784 | |
785 | <wadl:doc xmlns="http://www.w3.org/1999/xhtml" |
786 | - tal:define="version context/wadl:service_version" |
787 | + tal:define="version service/wadl:service_version" |
788 | tal:attributes="title string:About version ${version}" |
789 | - tal:content="structure context/wadl:version_description"> |
790 | + tal:content="structure service/wadl:version_description"> |
791 | Description of this version of the web service. |
792 | </wadl:doc> |
793 | |
794 | <!--There is one "service root" resource, located (as you'd expect) |
795 | at the service root. This very document is the WADL |
796 | representation of the "service root" resource.--> |
797 | - <resources tal:attributes="base context/wadl:url"> |
798 | + <resources tal:attributes="base service/wadl:url"> |
799 | <resource path="" type="#service-root" /> |
800 | </resources> |
801 | |
802 | @@ -46,7 +46,7 @@ |
803 | <!--The JSON representation of a "service root" resource contains a |
804 | number of links to collection-type resources.--> |
805 | <representation mediaType="application/json" id="service-root-json"> |
806 | - <tal:root_params tal:repeat="param context/wadl:top_level_resources"> |
807 | + <tal:root_params tal:repeat="param service/wadl:top_level_resources"> |
808 | <param style="plain" tal:attributes="name param/name; |
809 | path param/path"> |
810 | <link tal:attributes="resource_type param/resource/wadl:type_link" /> |
811 | @@ -284,15 +284,29 @@ |
812 | |
813 | <representation mediaType="application/json" |
814 | tal:attributes="id |
815 | - string:${context/wadl_entry:entry_page_representation_id}"> |
816 | + string:${context/wadl_entry:entry_page_representation_id}" |
817 | + tal:define="is_total_size_link_active |
818 | + service/wadl:is_total_size_link_active"> |
819 | |
820 | <param style="plain" name="resource_type_link" |
821 | path="$['resource_type_link']"> |
822 | <link /> |
823 | </param> |
824 | |
825 | + <tal:comment condition="nothing"> |
826 | + If we are not using total_size_link, continue to signal that |
827 | + total_size is required as it has been in the past. |
828 | + </tal:comment> |
829 | + |
830 | <param style="plain" name="total_size" path="$['total_size']" |
831 | - required="true" /> |
832 | + tal:attributes=" |
833 | + required python:is_total_size_link_active and 'false' or 'true'"/> |
834 | + |
835 | + <param tal:condition="is_total_size_link_active" |
836 | + style="plain" name="total_size_link" path="$['total_size_link']" |
837 | + required="false"> |
838 | + <link resource_type="#ScalarValue" /> |
839 | + </param> |
840 | |
841 | <param style="plain" name="start" path="$['start']" required="true" /> |
842 | |
843 | @@ -321,7 +335,7 @@ |
844 | <!--End representation and resource_type definitions for entry |
845 | resources. --> |
846 | |
847 | - <!--Finally, describe the 'hosted binary file' type.--> |
848 | + <!--Finally, describe the 'hosted binary file' type...--> |
849 | <resource_type id="HostedFile"> |
850 | <method name="GET" id="HostedFile-get"> |
851 | <response> |
852 | @@ -334,6 +348,15 @@ |
853 | <method name="DELETE" id="HostedFile-delete" /> |
854 | </resource_type> |
855 | |
856 | + <!--...and the simple 'scalar value' type.--> |
857 | + <resource_type id="ScalarValue"> |
858 | + <method name="GET" id="ScalarValue-get"> |
859 | + <response> |
860 | + <representation mediaType="application/json" /> |
861 | + </response> |
862 | + </method> |
863 | + </resource_type> |
864 | + |
865 | <!--Define a data type for binary data.--> |
866 | <xsd:simpleType name="binary"> |
867 | <xsd:list itemType="byte" /> |
868 | |
869 | === modified file 'src/lazr/restful/testing/webservice.py' |
870 | --- src/lazr/restful/testing/webservice.py 2010-03-03 16:40:14 +0000 |
871 | +++ src/lazr/restful/testing/webservice.py 2010-08-09 20:39:47 +0000 |
872 | @@ -135,6 +135,7 @@ |
873 | self.traversed_objects = [] |
874 | self.query_string_params = {} |
875 | self.method = 'GET' |
876 | + self.URL = 'http://api.example.org/' |
877 | |
878 | |
879 | def getTraversalStack(self): |
880 | @@ -173,6 +174,8 @@ |
881 | def pprint_collection(json_body): |
882 | """Pretty-print a webservice collection JSON representation.""" |
883 | for key, value in sorted(json_body.items()): |
884 | + if key == 'total_size_link': |
885 | + continue |
886 | if key != 'entries': |
887 | print '%s: %r' % (key, value) |
888 | print '---' |
889 | |
890 | === modified file 'src/lazr/restful/tests/test_utils.py' |
891 | --- src/lazr/restful/tests/test_utils.py 2010-03-03 13:08:12 +0000 |
892 | +++ src/lazr/restful/tests/test_utils.py 2010-08-09 20:39:47 +0000 |
893 | @@ -10,7 +10,8 @@ |
894 | from zope.security.management import ( |
895 | endInteraction, newInteraction, queryInteraction) |
896 | |
897 | -from lazr.restful.utils import get_current_browser_request |
898 | +from lazr.restful.utils import (get_current_browser_request, |
899 | + is_total_size_link_active) |
900 | |
901 | |
902 | class TestUtils(unittest.TestCase): |
903 | @@ -28,6 +29,27 @@ |
904 | self.assertEquals(request, get_current_browser_request()) |
905 | endInteraction() |
906 | |
907 | + def test_is_total_size_link_active(self): |
908 | + # Parts of the code want to know if the sizes of collections should be |
909 | + # reported in an attribute or via a link back to the service. The |
910 | + # is_total_size_link_active function takes the version of the API in |
911 | + # question and a web service configuration object and returns a |
912 | + # boolean that is true if a link should be used, false otherwise. |
913 | + |
914 | + # Here's the fake web service config we'll be using. |
915 | + class FakeConfig: |
916 | + active_versions = ['1.0', '2.0', '3.0'] |
917 | + first_version_with_total_size_link = '2.0' |
918 | + |
919 | + # First, if the version is lower than the threshold for using links, |
920 | + # the result is false (i.e., links should not be used). |
921 | + self.assertEqual(is_total_size_link_active('1.0', FakeConfig), False) |
922 | + |
923 | + # However, if the requested version is equal to, or higher than the |
924 | + # threshold, the result is true (i.e., links should be used). |
925 | + self.assertEqual(is_total_size_link_active('2.0', FakeConfig), True) |
926 | + self.assertEqual(is_total_size_link_active('3.0', FakeConfig), True) |
927 | + |
928 | # For the sake of convenience, test_get_current_web_service_request() |
929 | # and tag_request_with_version_name() are tested in test_webservice.py. |
930 | |
931 | |
932 | === modified file 'src/lazr/restful/tests/test_webservice.py' |
933 | --- src/lazr/restful/tests/test_webservice.py 2010-03-03 16:33:21 +0000 |
934 | +++ src/lazr/restful/tests/test_webservice.py 2010-08-09 20:39:47 +0000 |
935 | @@ -9,20 +9,22 @@ |
936 | import unittest |
937 | |
938 | from zope.component import getGlobalSiteManager, getUtility |
939 | -from zope.interface import implements, Interface |
940 | +from zope.interface import implements, Interface, directlyProvides |
941 | from zope.publisher.browser import TestRequest |
942 | from zope.schema import Date, Datetime, TextLine |
943 | from zope.security.management import ( |
944 | endInteraction, newInteraction, queryInteraction) |
945 | from zope.traversing.browser.interfaces import IAbsoluteURL |
946 | |
947 | +from lazr.restful import ResourceOperation |
948 | from lazr.restful.fields import Reference |
949 | from lazr.restful.interfaces import ( |
950 | ICollection, IEntry, IEntryResource, IResourceGETOperation, |
951 | IServiceRootResource, IWebServiceConfiguration, |
952 | IWebServiceClientRequest, IWebServiceVersion) |
953 | from lazr.restful import EntryResource, ResourceGETOperation |
954 | -from lazr.restful.declarations import exported, export_as_webservice_entry |
955 | +from lazr.restful.declarations import (exported, export_as_webservice_entry, |
956 | + LAZR_WEBSERVICE_NAME) |
957 | from lazr.restful.testing.webservice import ( |
958 | create_web_service_request, IGenericCollection, IGenericEntry, |
959 | WebServiceTestCase, WebServiceTestPublication) |
960 | @@ -30,6 +32,7 @@ |
961 | from lazr.restful.utils import ( |
962 | get_current_browser_request, get_current_web_service_request, |
963 | tag_request_with_version_name) |
964 | +from lazr.restful._resource import CollectionResource, BatchingResourceMixin |
965 | |
966 | |
967 | def get_resource_factory(model_interface, resource_interface): |
968 | @@ -350,5 +353,78 @@ |
969 | self.assertEquals("2.0", webservice_request.version) |
970 | self.assertTrue(marker_20.providedBy(webservice_request)) |
971 | |
972 | + |
973 | def additional_tests(): |
974 | return unittest.TestLoader().loadTestsFromName(__name__) |
975 | + |
976 | + |
977 | +class ITestEntry(IEntry): |
978 | + """Interface for a test entry.""" |
979 | + export_as_webservice_entry() |
980 | + |
981 | + |
982 | +class TestEntry: |
983 | + implements(ITestEntry) |
984 | + def __init__(self, context, request): |
985 | + pass |
986 | + |
987 | + |
988 | +class BaseBatchingTest: |
989 | + """A base class which tests BatchingResourceMixin and subclasses.""" |
990 | + |
991 | + testmodule_objects = [HasRestrictedField, IHasRestrictedField] |
992 | + |
993 | + def setUp(self): |
994 | + super(BaseBatchingTest, self).setUp() |
995 | + # Register TestEntry as the IEntry implementation for ITestEntry. |
996 | + getGlobalSiteManager().registerAdapter( |
997 | + TestEntry, [ITestEntry, IWebServiceClientRequest], provided=IEntry) |
998 | + # Is doing this by hand the right way? |
999 | + ITestEntry.setTaggedValue( |
1000 | + LAZR_WEBSERVICE_NAME, |
1001 | + dict(singular='test_entity', plural='test_entities')) |
1002 | + |
1003 | + |
1004 | + def make_instance(self, entries, request): |
1005 | + raise NotImplementedError('You have to make your own instances.') |
1006 | + |
1007 | + def test_getting_a_batch(self): |
1008 | + entries = [1, 2, 3] |
1009 | + request = create_web_service_request('/devel') |
1010 | + instance = self.make_instance(entries, request) |
1011 | + total_size = instance.get_total_size(entries) |
1012 | + self.assertEquals(total_size, '3') |
1013 | + |
1014 | + |
1015 | +class TestBatchingResourceMixin(BaseBatchingTest, WebServiceTestCase): |
1016 | + """Test that BatchingResourceMixin does batching correctly.""" |
1017 | + |
1018 | + def make_instance(self, entries, request): |
1019 | + return BatchingResourceMixin() |
1020 | + |
1021 | + |
1022 | +class TestCollectionResourceBatching(BaseBatchingTest, WebServiceTestCase): |
1023 | + """Test that CollectionResource does batching correctly.""" |
1024 | + |
1025 | + def make_instance(self, entries, request): |
1026 | + class Collection: |
1027 | + implements(ICollection) |
1028 | + entry_schema = ITestEntry |
1029 | + |
1030 | + def __init__(self, entries): |
1031 | + self.entries = entries |
1032 | + |
1033 | + def find(self): |
1034 | + return self.entries |
1035 | + |
1036 | + return CollectionResource(Collection(entries), request) |
1037 | + |
1038 | + |
1039 | +class TestResourceOperationBatching(BaseBatchingTest, WebServiceTestCase): |
1040 | + """Test that ResourceOperation does batching correctly.""" |
1041 | + |
1042 | + def make_instance(self, entries, request): |
1043 | + # constructor parameters are ignored |
1044 | + return ResourceOperation(None, request) |
1045 | + |
1046 | + |
1047 | |
1048 | === modified file 'src/lazr/restful/utils.py' |
1049 | --- src/lazr/restful/utils.py 2010-03-10 20:44:03 +0000 |
1050 | +++ src/lazr/restful/utils.py 2010-08-09 20:39:47 +0000 |
1051 | @@ -37,6 +37,18 @@ |
1052 | missing = object() |
1053 | |
1054 | |
1055 | +def is_total_size_link_active(version, config): |
1056 | + versions = config.active_versions |
1057 | + total_size_link_version = config.first_version_with_total_size_link |
1058 | + # The version "None" is a special marker for the earliest version. |
1059 | + if total_size_link_version is None: |
1060 | + total_size_link_version = versions[0] |
1061 | + # If the version we're being asked about is equal to or later than the |
1062 | + # version in which we started exposing total_size_link, then we should |
1063 | + # return True, False otherwise. |
1064 | + return versions.index(total_size_link_version) <= versions.index(version) |
1065 | + |
1066 | + |
1067 | class VersionedDict(object): |
1068 | """A stack of named dictionaries. |
1069 | |
1070 | |
1071 | === modified file 'src/lazr/restful/version.txt' |
1072 | --- src/lazr/restful/version.txt 2010-08-09 20:39:47 +0000 |
1073 | +++ src/lazr/restful/version.txt 2010-08-09 20:39:47 +0000 |
1074 | @@ -1,5 +1,1 @@ |
1075 | -<<<<<<< TREE |
1076 | -0.10.0 |
1077 | -======= |
1078 | -0.10.0-dev |
1079 | ->>>>>>> MERGE-SOURCE |
1080 | +0.11.0-dev |
1081 | |
1082 | === modified file 'versions.cfg' |
1083 | --- versions.cfg 2010-08-09 20:39:47 +0000 |
1084 | +++ versions.cfg 2010-08-09 20:39:47 +0000 |
1085 | @@ -3,7 +3,6 @@ |
1086 | allow-picked-versions = false |
1087 | use-dependency-links = false |
1088 | |
1089 | - |
1090 | [versions] |
1091 | # Alphabetical, case-SENSITIVE, blank line after this comment |
1092 | |
1093 | @@ -16,11 +15,10 @@ |
1094 | docutils = 0.5 |
1095 | epydoc = 3.0.1 |
1096 | grokcore.component = 1.6 |
1097 | -lazr.batchnavigator = 1.1.1 |
1098 | -lazr.delegates = 1.1.0 |
1099 | +lazr.batchnavigator = 1.2.0 |
1100 | +lazr.delegates = 1.2.0 |
1101 | lazr.enum = 1.1.2 |
1102 | lazr.lifecycle = 1.0 |
1103 | -lazr.restful = 0.9.29 |
1104 | lazr.uri = 1.0.2 |
1105 | lxml = 2.2.7 |
1106 | martian = 0.11 |
The NEWS file has both the British and American spellings of "behavior." Officially, Canonical uses British spellings. I generally stick with American spellings for lazr packages, since that's easier for all or most of the main contributors they have. At the very least, for one NEWS entry, I'd like to stick to one approach. :-)
The name "total_size_only" appears several times. Its behavior is not clear to me. The ``if not self.total_ size_only: `` lines that appear in _operations.py particularly confuse me. I'd like to see some comments, and I'd like it explained in appropriate docstrings. It also makes me question the name in batchnavigator, TBH, but maybe there's not a better option. What does it mean?
Other than that, it looks good to me, though I'll defer to Leonard's review when he is available.