Merge lp:~leonardr/lazr.restful/multiversion-pagetest into lp:lazr.restful

Proposed by Leonard Richardson
Status: Merged
Merged at revision: not available
Proposed branch: lp:~leonardr/lazr.restful/multiversion-pagetest
Merge into: lp:lazr.restful
Diff against target: 467 lines (+238/-69)
5 files modified
src/lazr/restful/declarations.py (+32/-46)
src/lazr/restful/docs/webservice-declarations.txt (+3/-13)
src/lazr/restful/example/multiversion/resources.py (+57/-4)
src/lazr/restful/example/multiversion/root.py (+3/-1)
src/lazr/restful/example/multiversion/tests/introduction.txt (+143/-5)
To merge this branch: bzr merge lp:~leonardr/lazr.restful/multiversion-pagetest
Reviewer Review Type Date Requested Status
Paul Hummer (community) code Approve
Review via email: mp+18868@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Leonard Richardson (leonardr) wrote :

This branch adds more multi-version capabilities to the example multiversion web service (fields that are not published in all versions, a field that's manipulated with a mutator, and two destructor methods for different versions), and adds pagetests to make sure all the features I've been writing work in a real multi-version web service.

I found one part of the system which did _not_ work in a real web service. If you define a field's mutator method for version 1.0, that works fine for version 1.0, but the mutator is not inherited in version 2.0--the field goes back to being read-only. There was no inheritance mechanism to make sure that version 2.0's mutator defaults to version 1.0's mutator.

Fortunately, I've already dealt with this situation. I had the exact same problem on the field level, where version 2.0's field was not published under version 1.0's name, and I resolved it with _normalize_field_annotations. In this branch, I removed the _get_by_version helper method which I was using to retrieve mutator information from the special mutator dictionary. Instead, I changed _normalize_field_annotations to merge the special mutator dictionary into the standard annotation dictionary as it normalizes that dictionary. This is where inheritance is implemented for fields, and now that's also where it's implemented for field mutators.

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

<rockstar> leonardr, hi. Something is fishy about this diff.
<rockstar> leonardr, is it not based on one of the previous branches I reviewed?
<rockstar> leonardr, nevermind. I plead temporary insanity.
<rockstar> leonardr, so, if there's no mutator specified for 2.0, it uses the mutator specified from 1.0?
<leonardr> rockstar: right
<leonardr> same as other named operations
<rockstar> leonardr, so is "beta" the most recent version, and then 3.0, 2.0, and 1.0?
<leonardr> rockstar: right, according to the list of versions defined in the iwebserviceconfiguration

review: Approve (code)
Revision history for this message
Leonard Richardson (leonardr) wrote :

Wait, actually, the list of versions goes beta, 1.0, 2.0, 3.0. Not beta, 3.0, 2.0, 1.0. I hope you haven't found a problem.

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

<rockstar> leonardr, actually, that would be a problem, wouldn't it?
<rockstar> beta should be newer than 3.0, which is newer than 2.0, etc.
<leonardr> rockstar: actually beta is the _earliest_ version, but 3.0 is newer than 2.0, etc
<rockstar> leonardr, what is meant by "earliest" though.
<rockstar> It shouldn't mean that 1.0 overrides it.
<leonardr> rockstar: well, right now launchpad has a 'beta' web service and nothing else
<leonardr> now that i have multi-version working i'm going to create a '1.0' version that's different
<rockstar> leonardr, yes, but then we'll go 1.0, and beta will be what 2.0 is created from.
<leonardr> that's what i mean by saying beta is the earliest
<leonardr> rockstar: no, we do have a 'floating development version', but in launchpad's case that is called 'trunk'
<leonardr> 'beta' is a specific version
<leonardr> that will become obsolete and stop changing
<rockstar> leonardr, huh, that's odd.
<rockstar> leonardr, okay, I guess it makes sense to have 1.0 supersede beta then.
<leonardr> rockstar: if you are confused it's likely others will be confused and think that 'beta' is the permanent cutting edge
<leonardr> but i don't know what we can do about that
<leonardr> it's more like gmail, which was in 'beta' for 5 years and now it's not
<leonardr> the only differnece is that we have to keep 'beta' around for a while
<rockstar> leonardr, yeah. Having worked on the other side of the REST API (on Tarmac), when the API changes now, baby Jesus cries.
<leonardr> thus the multi-version project
<rockstar> leonardr, I think "trunk" for bleeding edge is probably misnamed, since it's likely it could be in production as "trunk"
<leonardr> rockstar: it's not too late to change it
<leonardr> if you have a better idea
<rockstar> leonardr, well, just what I was assuming where beta is the floating development version.
<leonardr> my only alternative is 'dev' or 'development'
<rockstar> leonardr, are we going to release an API version on every release of Launchpad?
<leonardr> rockstar: no, API versions will correspond roughly with ubuntu releases
<leonardr> and will be retired at the same rate as ubuntu releases
<rockstar> Ah, okay.
<rockstar> So maybe "dev" would be appropriate, since (at this point in time), it would correspond to the scripts in Lucid.
<leonardr> flacoste: ^- what do you think?
<leonardr> incidentally, flacoste, the multiversion lazr.restful work is complete as of yesterday
<flacoste> leonardr: i saw good progress on that, great! but complete? i guess we still need some changes in the client support, no?
<flacoste> leonardr: btw, i don't have an opinion on dev vs trunk
<flacoste> we could call it 'unstable'
<leonardr> flacoste: yes, we need to add multiversion to lazr.restfulclient

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/lazr/restful/declarations.py'
2--- src/lazr/restful/declarations.py 2010-02-05 14:30:30 +0000
3+++ src/lazr/restful/declarations.py 2010-02-08 19:15:22 +0000
4@@ -894,14 +894,8 @@
5 tags = tags[0]
6 if tags.get('exported') is False:
7 continue
8- mutator_annotations = field.queryTaggedValue(
9- LAZR_WEBSERVICE_MUTATORS)
10- error_prefix = (
11- 'Duplicate mutator for interface "%s", field "%s":' % (
12- interface.__name__, field.__name__))
13- mutated_by, mutated_by_annotations = _get_by_version(
14- mutator_annotations, version, versions[0],
15- error_prefix, (None, {}))
16+ mutated_by, mutated_by_annotations = tags.get(
17+ 'mutator_annotations', (None, {}))
18 readonly = (field.readonly and mutated_by is None)
19 attrs[tags['as']] = copy_field(
20 field, __name__=tags['as'], readonly=readonly)
21@@ -941,7 +935,6 @@
22 adapters_by_version = {}
23 for name, field in getFields(content_interface).items():
24 tag_stack = field.queryTaggedValue(LAZR_WEBSERVICE_EXPORTED)
25- mutator_annotations = field.queryTaggedValue(LAZR_WEBSERVICE_MUTATORS)
26 if tag_stack is None:
27 continue
28 for tags_version, tags in tag_stack.stack:
29@@ -954,12 +947,8 @@
30 # this version's adapter class dictionary.
31 if tags.get('exported') is False:
32 continue
33- error_prefix = (
34- 'Duplicate mutator for interface "%s", field "%s":' % (
35- content_interface.__name__, field.__name__))
36- mutator, annotations = _get_by_version(
37- mutator_annotations, tags_version, webservice_interfaces[0][0],
38- error_prefix, (None, {}))
39+ mutator, annotations = tags.get(
40+ 'mutator_annotations', (None, {}))
41
42 if mutator is None:
43 property = Passthrough(name, 'context')
44@@ -1222,13 +1211,18 @@
45 Since we have the list of versions available, this is also a good
46 time to do some error checking: make sure that version annotations
47 are not duplicated or in the wrong order.
48+
49+ Finally, this is a good time to integrate the mutator annotations
50+ into the field annotations.
51 """
52 versioned_dict = field.queryTaggedValue(LAZR_WEBSERVICE_EXPORTED)
53+ mutator_annotations = field.queryTaggedValue(LAZR_WEBSERVICE_MUTATORS)
54
55 # If the first version is None and the second version is the
56 # earliest version, consolidate the two versions.
57 earliest_version = versions[0]
58 stack = versioned_dict.stack
59+
60 if (len(stack) >= 2
61 and stack[0][0] is None and stack[1][0] == earliest_version):
62
63@@ -1253,6 +1247,20 @@
64 if stack[0][0] is None:
65 stack[0] = (earliest_version, stack[0][1])
66
67+ # Make sure there is at most one mutator for the earliest version.
68+ # If there is one, move it from the mutator-specific dictionary to
69+ # the normal tag stack.
70+ implicit_earliest_mutator = mutator_annotations.get(None, None)
71+ explicit_earliest_mutator = mutator_annotations.get(earliest_version, None)
72+ if (implicit_earliest_mutator is not None
73+ and explicit_earliest_mutator is not None):
74+ raise ValueError(
75+ error_prefix + " Both implicit and explicit mutator definitions "
76+ "found for earliest version %s." % earliest_version)
77+ earliest_mutator = implicit_earliest_mutator or explicit_earliest_mutator
78+ if earliest_mutator is not None:
79+ stack[0][1]['mutator_annotations'] = earliest_mutator
80+
81 # Do some error checking.
82 max_index = -1
83 for version, tags in versioned_dict.stack:
84@@ -1279,7 +1287,10 @@
85 # The field is not initially published.
86 new_stack = (earliest_version, dict(published=False))
87 most_recent_tags = new_stack[0][1]
88+ most_recent_mutator_tags = earliest_mutator
89 for version in versions[1:]:
90+ most_recent_mutator_tags = mutator_annotations.get(
91+ version, most_recent_mutator_tags)
92 match = [(stack_version, stack_tags)
93 for stack_version, stack_tags in stack
94 if stack_version == version]
95@@ -1293,6 +1304,12 @@
96 new_stack.append(match[0])
97 most_recent_tags = match[0][1]
98
99+ # Install a (possibly inherited) mutator for this field in
100+ # this version.
101+ if most_recent_mutator_tags is not None:
102+ new_stack[-1][1]['mutator_annotations'] = copy.deepcopy(
103+ most_recent_mutator_tags)
104+
105 versioned_dict.stack = new_stack
106 return field
107
108@@ -1319,34 +1336,3 @@
109 version = "__Earliest"
110 name = "%s_%s" % (base_name, version.encode('utf8'))
111 return make_identifier_safe(name)
112-
113-
114-def _get_by_version(dictionary, version, earliest_version,
115- error_prefix, default=None):
116- """Pull from a dictionary using `version` as the key.
117-
118- Under normal circumstances, this will be an ordinary lookup. But
119- some values for the earliest version are stored under None
120- (because the name of the earliest version wasn't known at the
121- time). This helper function knows to look again under None if
122- a lookup for the earliest version fails.
123-
124- It also knows to raise an exception signaling a conflict if a
125- value is present under the earliest version *and* under None.
126-
127- :param dictionary: The dictionary to use for the lookup.
128- :param version: The dictionary key.
129- :param earliest_version: The version that might also be filed under None.
130- :param default: A default value to use if the lookup fails.
131- """
132- value = dictionary.get(version, None)
133- if version == earliest_version:
134- earliest_value = dictionary.get(None, None)
135- if value is not None and earliest_value is not None:
136- raise ValueError(
137- error_prefix + " Both implicit and explicit definitions found "
138- "for earliest version %s." % version)
139- value = value or earliest_value
140- if value is None:
141- value = default
142- return value
143
144=== modified file 'src/lazr/restful/docs/webservice-declarations.txt'
145--- src/lazr/restful/docs/webservice-declarations.txt 2010-02-05 14:30:30 +0000
146+++ src/lazr/restful/docs/webservice-declarations.txt 2010-02-08 19:15:22 +0000
147@@ -1808,16 +1808,6 @@
148 AttributeError: 'MultiVersionEntryEntry_3_0Adapter' object has no
149 attribute 'new_in_10'
150
151-[XXX leonardr These tests need to be done in pagetests, since they
152- test the behavior of the webservice rather than whether certain
153- combinations of annotations are valid.
154-
155- If a field has a mutator in version n, later versions will inherit the
156- same mutator.
157-
158- An entry can have different operations marked as the destructor for
159- different versions.]
160-
161 Why the list of version strings?
162 ********************************
163
164@@ -2327,9 +2317,9 @@
165 ... IImplicitAndExplicitMutator, 'beta', '1.0')
166 Traceback (most recent call last):
167 ...
168- ValueError: Duplicate mutator for interface
169- "IImplicitAndExplicitMutator", field "field": Both implicit and
170- explicit definitions found for earliest version beta.
171+ ValueError: Field "field" in interface
172+ "IImplicitAndExplicitMutator": Both implicit and explicit mutator
173+ definitions found for earliest version beta.
174
175 This error isn't detected until you try to generate the entry
176 interfaces, because until that point lazr.restful doesn't know that
177
178=== modified file 'src/lazr/restful/example/multiversion/resources.py'
179--- src/lazr/restful/example/multiversion/resources.py 2010-01-27 15:05:56 +0000
180+++ src/lazr/restful/example/multiversion/resources.py 2010-02-08 19:15:22 +0000
181@@ -6,26 +6,57 @@
182 'PairSet']
183
184 from zope.interface import implements
185-from zope.schema import Text
186+from zope.schema import Bool, Text
187 from zope.location.interfaces import ILocation
188
189 from lazr.restful.declarations import (
190 collection_default_content, export_as_webservice_collection,
191- export_as_webservice_entry, export_operation_as,
192- export_read_operation, exported, operation_for_version,
193- operation_parameters, operation_removed_in_version)
194+ export_as_webservice_entry, export_destructor_operation,
195+ export_operation_as, export_read_operation, export_write_operation,
196+ exported, mutator_for, operation_for_version, operation_parameters,
197+ operation_removed_in_version)
198
199 # Our implementations of these classes can be based on the
200 # implementations from the WSGI example.
201 from lazr.restful.example.wsgi.resources import (
202 PairSet as BasicPairSet, KeyValuePair as BasicKeyValuePair)
203
204+
205 # Our interfaces _will_ diverge from the WSGI example interfaces, so
206 # define them separately.
207 class IKeyValuePair(ILocation):
208 export_as_webservice_entry()
209 key = exported(Text(title=u"The key"))
210 value = exported(Text(title=u"The value"))
211+ a_comment = exported(Text(title=u"A comment on this key-value pair.",
212+ readonly=True),
213+ ('1.0', dict(exported=True, exported_as='comment')))
214+ deleted = exported(Bool(title=u"Whether this key-value pair has been "
215+ "deleted"),
216+ ('3.0', dict(exported=True)), exported=False)
217+ @mutator_for(a_comment)
218+ @export_write_operation()
219+ @operation_parameters(comment=Text())
220+ @operation_for_version('1.0')
221+ def comment_mutator_1(comment):
222+ """A comment mutator that adds some junk on the end."""
223+
224+ @mutator_for(a_comment)
225+ @export_write_operation()
226+ @operation_parameters(comment=Text())
227+ @operation_for_version('3.0')
228+ def comment_mutator_2(comment):
229+ """A comment mutator that adds different junk on the end."""
230+
231+ @export_destructor_operation()
232+ @operation_for_version('1.0')
233+ def total_destruction():
234+ """A destructor that removes the key-value pair altogether."""
235+
236+ @export_destructor_operation()
237+ @operation_for_version('3.0')
238+ def mark_as_deleted():
239+ """A destructor that simply sets .deleted to True."""
240
241
242 class IPairSet(ILocation):
243@@ -69,5 +100,27 @@
244 def getNonEmptyPairs(self):
245 return [pair for pair in self.pairs if pair.value is not None]
246
247+
248 class KeyValuePair(BasicKeyValuePair):
249 implements(IKeyValuePair)
250+
251+ def __init__(self, pairset, key, value):
252+ super(KeyValuePair, self).__init__(pairset, key, value)
253+ self.a_comment = ''
254+ self.deleted = False
255+
256+ def comment_mutator_1(self, comment):
257+ """A comment mutator."""
258+ self.a_comment = comment + " (modified by mutator #1)"
259+
260+ def comment_mutator_2(self, comment):
261+ """A comment mutator."""
262+ self.a_comment = comment + " (modified by mutator #2)"
263+
264+ def total_destruction(self):
265+ """Remove the pair from the pairset."""
266+ self.set.pairs.remove(self)
267+
268+ def mark_as_deleted(self):
269+ """Set .deleted to True."""
270+ self.deleted = True
271
272=== modified file 'src/lazr/restful/example/multiversion/root.py'
273--- src/lazr/restful/example/multiversion/root.py 2010-01-27 15:05:56 +0000
274+++ src/lazr/restful/example/multiversion/root.py 2010-02-08 19:15:22 +0000
275@@ -47,7 +47,9 @@
276 pairset.pairs = [
277 KeyValuePair(pairset, "foo", "bar"),
278 KeyValuePair(pairset, "1", "2"),
279- KeyValuePair(pairset, "Some", None)
280+ KeyValuePair(pairset, "Some", None),
281+ KeyValuePair(pairset, "Delete", "me"),
282+ KeyValuePair(pairset, "Also delete", "me")
283 ]
284 collections = dict(pairs=(IKeyValuePair, pairset))
285 return collections, {}
286
287=== modified file 'src/lazr/restful/example/multiversion/tests/introduction.txt'
288--- src/lazr/restful/example/multiversion/tests/introduction.txt 2010-01-27 15:05:56 +0000
289+++ src/lazr/restful/example/multiversion/tests/introduction.txt 2010-02-08 19:15:22 +0000
290@@ -68,8 +68,8 @@
291 HTTP/1.1 404 Not Found
292 ...
293
294-Collections and entries
295-=======================
296+Collections
297+===========
298
299 The web service presents a single collection of key-value pairs. In
300 versions previous to 2.0, the collection omits key-value pairs where
301@@ -83,25 +83,163 @@
302
303 >>> show_pairs('beta')
304 1: 2
305+ Also delete: me
306+ Delete: me
307 foo: bar
308
309 >>> show_pairs('1.0')
310 1: 2
311+ Also delete: me
312+ Delete: me
313 foo: bar
314
315 >>> show_pairs('2.0')
316 1: 2
317+ Also delete: me
318+ Delete: me
319 Some: None
320 foo: bar
321
322 >>> show_pairs('3.0')
323 1: 2
324+ Also delete: me
325+ Delete: me
326 Some: None
327 foo: bar
328
329- >>> body = webservice.get('/pairs/foo').jsonBody()
330- >>> print body['key'], body['value']
331- foo bar
332+Entries
333+=======
334+
335+Let's take a look at 'comment' and 'deleted', two fields with
336+interesting properties.
337+
338+The 'comment' field is not modified directly, but by internal mutator
339+methods which append some useless text to your comment.
340+
341+ >>> import simplejson
342+ >>> def get_comment(version):
343+ ... response = webservice.get("/pairs/foo", api_version=version)
344+ ... return response.jsonBody()['comment']
345+
346+ >>> def change_comment(comment, version, get_comment_afterwards=True):
347+ ... ignored = webservice.patch(
348+ ... "/pairs/foo/", 'application/json',
349+ ... simplejson.dumps({"comment": comment}),
350+ ... api_version=version)
351+ ... if get_comment_afterwards:
352+ ... return get_comment(version)
353+ ... return None
354+
355+ >>> get_comment('1.0')
356+ u''
357+ >>> print change_comment('I changed 1.0', '1.0')
358+ I changed 1.0 (modified by mutator #1)
359+
360+ >>> print change_comment('I changed 2.0', '2.0')
361+ I changed 2.0 (modified by mutator #1)
362+
363+ >>> print change_comment('I changed 3.0', '3.0')
364+ I changed 3.0 (modified by mutator #2)
365+
366+You can try to modify the 'comment' field from a version that doesn't
367+publish that field, but lazr.restful will ignore your request.
368+
369+ >>> change_comment('I changed beta', 'beta', False)
370+
371+ >>> print get_comment('1.0')
372+ I changed 3.0 (modified by mutator #2)
373+
374+A field called 'deleted' is published starting in version '3.0'. A
375+comment field is called 'a_comment' in version 'beta' and 'comment' in
376+all later versions.
377+
378+ >>> def show_fields(version):
379+ ... entry_body = webservice.get(
380+ ... '/pairs/foo', api_version=version).jsonBody()
381+ ... for key in sorted(entry_body.keys()):
382+ ... print key
383+
384+ >>> show_fields('beta')
385+ a_comment
386+ http_etag
387+ key
388+ resource_type_link
389+ self_link
390+ value
391+
392+ >>> show_fields('1.0')
393+ comment
394+ http_etag
395+ key
396+ resource_type_link
397+ self_link
398+ value
399+
400+ >>> show_fields('3.0')
401+ comment
402+ deleted
403+ http_etag
404+ key
405+ resource_type_link
406+ self_link
407+ value
408+
409+In the 'beta' version, attempting to delete a key-value pair will
410+result in a status code of 405 ("Method Not Available").
411+
412+ >>> response = webservice.delete('/pairs/Delete', api_version='beta')
413+ >>> response.status
414+ 405
415+
416+As of '1.0', attempting to delete a key-value pair results in the
417+key-value pair being totally removed from the web service.
418+
419+ >>> ignore = webservice.delete('/pairs/Delete', api_version='1.0')
420+ >>> show_pairs('beta')
421+ 1: 2
422+ Also delete: me
423+ foo: bar
424+
425+In '3.0', deleting a key-value pair simply sets its 'deleted' field
426+to True. (This is an abuse of the HTTP DELETE method, but it makes a
427+good demo.)
428+
429+ >>> body = webservice.get(
430+ ... '/pairs/Also%20delete', api_version='3.0').jsonBody()
431+ >>> body['deleted']
432+ False
433+
434+ >>> ignore = webservice.delete('/pairs/Also%20delete', api_version='3.0')
435+
436+The "deleted" key-value pair is still visible in all versions:
437+
438+ >>> show_pairs('beta')
439+ 1: 2
440+ Also delete: me
441+ foo: bar
442+
443+And in a version which publishes the 'delete' field, we can check the
444+key-value pair's value for that field.
445+
446+ >>> body = webservice.get(
447+ ... '/pairs/Also%20delete', api_version='3.0').jsonBody()
448+ >>> body['deleted']
449+ True
450+
451+Fields
452+======
453+
454+If an entry field is not published in a certain version, the
455+corresponding field resource does not exist for that version.
456+
457+ >>> webservice.get('/pairs/foo/deleted', api_version='beta')
458+ Traceback (most recent call last):
459+ ...
460+ NotFound: ... name: u'deleted'
461+
462+ >>> print webservice.get(
463+ ... '/pairs/foo/deleted', api_version='3.0').jsonBody()
464+ False
465
466 Named operations
467 ================

Subscribers

People subscribed via source and target branches