Merge lp:~leonardr/lazr.restful/promote-write-operation-to-mutator into lp:lazr.restful

Proposed by Leonard Richardson
Status: Merged
Approved by: Данило Шеган
Approved revision: 174
Merged at revision: 172
Proposed branch: lp:~leonardr/lazr.restful/promote-write-operation-to-mutator
Merge into: lp:lazr.restful
Diff against target: 231 lines (+102/-25)
4 files modified
src/lazr/restful/NEWS.txt (+11/-5)
src/lazr/restful/docs/webservice-declarations.txt (+65/-5)
src/lazr/restful/metazcml.py (+25/-14)
src/lazr/restful/version.txt (+1/-1)
To merge this branch: bzr merge lp:~leonardr/lazr.restful/promote-write-operation-to-mutator
Reviewer Review Type Date Requested Status
Данило Шеган (community) Approve
Review via email: mp+49976@code.launchpad.net

Description of the change

Let me explain this in terms of the class I used to test it.

    >>> class IOperationPromotedToMutator(Interface):
    ... export_as_webservice_entry()
    ...
    ... field = exported(TextLine(readonly=True))
    ...
    ... @mutator_for(field)
    ... @operation_for_version('3.0')
    ... @operation_parameters(text=TextLine())
    ... @export_write_operation()
    ... @operation_for_version('1.0')
    ... def set_value(text):
    ... pass

The example web service has four versions: beta, 1.0, 2.0, and 3.0. In 'beta', mutator methods were also published as named operations (just like in Launchpad 'beta'), but that behavior stopped in 1.0. Call 1.0 the 'cutoff version'.

The 'set_value' method is introduced in 1.0, and it's a normal write operation in 2.0, but in 3.0 it's promoted to a mutator method.

Here's the problem: this method was not available in 1.0 or 2.0. lazr.restful saw that set_value was defined as a mutator, and added a mask registration to stop it from showing up as of the 'cutoff version', 1.0. This would have been the correct behavior if set_value was defined as a mutator *before* the cutoff version, but actually it was defined as a mutator *after* the cutoff version.

lazr.restful was masking the write operation registration in the cutoff version if the method was defined as a mutator in *any* version. This branch changes it so that the operation is only masked in the cutoff version if it was defined as a mutator *before* the cutoff version. If a method is promoted to a mutator after the cutoff version, a lookup for that method will fail starting in the promotion version.

This branch also changes the versioned request marker interfaces in webservice-declarations.txt so that version n+1 is a subclass of version n. This more closely reproduces the behavior of a real lazr.restful web service, and lets me do a more realistic test.

To post a comment you must log in.
Данило Шеган (danilo) wrote :

FTR:

<danilos> leonardr, I assume there are tests for mutators working correctly otherwise? (since your test only "proves" that it stops providing a method)
<leonardr> danilos: yes, there are. a mutator is just a write operation that doesn't show up in lookups
<leonardr> i could add a test that the mutator works in 3.0
<danilos> leonardr, right, thanks
 leonardr, if there are already tests for that elsewhere, no need to imho
 leonardr, just one more question: is it still possible to keep a method and make it a mutator? (i.e. have set_value keep working in 3.0)
<leonardr> danilos: no, because there's no point in having a named operation that duplicates the effects of a mutator
<leonardr> the situation we have here is where we made a mistake--this should have been a mutator method all along
 but we can't change an old version for backwards compatibility reasons
<danilos> leonardr, well, I can imagine someone wanting to keep both so you don't have to port client code, no?
 leonardr, maybe that'd be hidden by things like launchpadlib instead, so I am just wondering
<leonardr> danilos: this falls into the category of 'if you don't want to port code, don't use the new version'
<leonardr> it's an easy change to make
<danilos> leonardr, ok, as long as it's recognized as being a use-case (or not), I think it's fine; some explicit documentation might be useful, if you agree
 leonardr, anyway, with that considered, r=me

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/lazr/restful/NEWS.txt'
2--- src/lazr/restful/NEWS.txt 2011-02-02 17:55:01 +0000
3+++ src/lazr/restful/NEWS.txt 2011-02-16 14:39:11 +0000
4@@ -2,9 +2,15 @@
5 NEWS for lazr.restful
6 =====================
7
8-0.16.0 (Unreleased)
9+0.16.1 (2011-02-16)
10 ===================
11
12+Fixed a bug that prevented a write operation from being promoted to a
13+mutator operation.
14+
15+0.16.0 (No official release)
16+============================
17+
18 If each entry in the web service corresponds to some object on a
19 website, and there's a way of converting a web service request into a
20 website request, the web service will now provide website links for
21@@ -16,13 +22,13 @@
22 Validation errors for named operations will be properly sent to the
23 client even if they contain Unicode characters. (Launchpad bug 619180.)
24
25-0.15.4 (2010-01-26)
26+0.15.4 (2011-01-26)
27 ===================
28
29 Fixed inconsistent handling of custom HTML field renderings. An
30 IFieldHTMLRenderer can now return either Unicode or UTF-8.
31
32-0.15.3 (2010-01-21)
33+0.15.3 (2011-01-21)
34 ===================
35
36 lazr.restful will now complain if you try to export an IObject, as
37@@ -32,7 +38,7 @@
38 use IObject.
39
40
41-0.15.2 (2010-01-20)
42+0.15.2 (2011-01-20)
43 ===================
44
45 lazr.restful gives a more helpful error message when a published
46@@ -41,7 +47,7 @@
47
48 lazr.restful's tests now pass in Python 2.7. (Launchpad bug 691841)
49
50-0.15.1 (2010-01-19)
51+0.15.1 (2011-01-19)
52 ===================
53
54 Fixed a redirect bug when a web browser requests a representation
55
56=== modified file 'src/lazr/restful/docs/webservice-declarations.txt'
57--- src/lazr/restful/docs/webservice-declarations.txt 2011-01-24 22:00:56 +0000
58+++ src/lazr/restful/docs/webservice-declarations.txt 2011-02-16 14:39:11 +0000
59@@ -989,15 +989,19 @@
60 need a marker interface for every version, registered as a utility
61 under the name of the version.
62
63+Each version interface subclasses the previous version's
64+interface. This lets a request use a resource definition for the
65+previous version if it hasn't changed since then.
66+
67 >>> from zope.component import getSiteManager
68 >>> from lazr.restful.interfaces import IWebServiceVersion
69 >>> class ITestServiceRequestBeta(IWebServiceVersion):
70 ... pass
71- >>> class ITestServiceRequest10(IWebServiceVersion):
72- ... pass
73- >>> class ITestServiceRequest20(IWebServiceVersion):
74- ... pass
75- >>> class ITestServiceRequest30(IWebServiceVersion):
76+ >>> class ITestServiceRequest10(ITestServiceRequestBeta):
77+ ... pass
78+ >>> class ITestServiceRequest20(ITestServiceRequest10):
79+ ... pass
80+ >>> class ITestServiceRequest30(ITestServiceRequest20):
81 ... pass
82 >>> sm = getSiteManager()
83 >>> for marker, name in [(ITestServiceRequestBeta, 'beta'),
84@@ -2804,6 +2808,62 @@
85 Edge cases
86 **********
87
88+You can promote a named operation to a mutator operation
89+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
90+
91+Here's a named operation that was defined in '1.0' and promoted to a
92+mutator in '3.0'.
93+
94+ >>> class IOperationPromotedToMutator(Interface):
95+ ... export_as_webservice_entry()
96+ ...
97+ ... field = exported(TextLine(readonly=True))
98+ ...
99+ ... @mutator_for(field)
100+ ... @operation_for_version('3.0')
101+ ... @operation_parameters(text=TextLine())
102+ ... @export_write_operation()
103+ ... @operation_for_version('1.0')
104+ ... def set_value(text):
105+ ... pass
106+
107+ >>> class OperationPromotedToMutator:
108+ ... implements(IOperationPromotedToMutator)
109+
110+ >>> module = register_test_module(
111+ ... 'mutatorpromotion', IOperationPromotedToMutator,
112+ ... OperationPromotedToMutator)
113+
114+ >>> context = OperationPromotedToMutator()
115+
116+The operation is not available in 'beta', because it hasn't been
117+defined yet.
118+
119+ >>> print operation_for(context, 'beta', 'set_value').__class__.__name__
120+ Traceback (most recent call last):
121+ ...
122+ ComponentLookupError: ...
123+
124+The operation is available in both '1.0', and '2.0', even though
125+mutator operations aren't published as named operations after
126+1.0. This is because the operation doesn't become a mutator operation
127+until 3.0.
128+
129+ >>> print operation_for(context, '1.0', 'set_value').__class__.__name__
130+ POST_IOperationPromotedToMutator_set_value_1_0
131+
132+ >>> print operation_for(context, '2.0', 'set_value').__class__.__name__
133+ POST_IOperationPromotedToMutator_set_value_1_0
134+
135+The operation is not available in 3.0, the version in which it becomes
136+a mutator.
137+
138+ >>> operation_for(context, '3.0', 'set_value')
139+ Traceback (most recent call last):
140+ ...
141+ ComponentLookupError: ...
142+
143+
144 You can't immediately reinstate a mutator operation as a named operation
145 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
146
147
148=== modified file 'src/lazr/restful/metazcml.py'
149--- src/lazr/restful/metazcml.py 2011-01-12 05:30:51 +0000
150+++ src/lazr/restful/metazcml.py 2011-02-16 14:39:11 +0000
151@@ -336,8 +336,15 @@
152 # version.
153 previous_operation_name = None
154 previous_operation_provides = None
155- operation_registered_as_mutator = False
156+ mutator_operation_needs_to_be_blocked = False
157+ master_stack = tag
158 for version, tag in tag.stack:
159+ if version is None:
160+ this_version_index = 0
161+ else:
162+ this_version_index = config.active_versions.index(
163+ version)
164+
165 if tag['type'] == REMOVED_OPERATION_TYPE:
166 # This operation is not present in this version.
167 # We'll represent this by setting the operation_name
168@@ -389,15 +396,9 @@
169 if tag['type'] in ['destructor']:
170 operation_name = ''
171
172- if version is None:
173- this_version_index = 0
174- else:
175- this_version_index = config.active_versions.index(
176- version)
177 if (tag.get('is_mutator', False)
178 and (no_mutator_operations_after is None
179 or no_mutator_operations_after < this_version_index)):
180-
181 # This is a mutator method, and in this version,
182 # mutator methods are not published as named
183 # operations at all. Block any lookup of the named
184@@ -406,7 +407,7 @@
185 # This will save us from having to do another
186 # de-registration later.
187 factory = _mask_adapter_registration
188- operation_registered_as_mutator = False
189+ mutator_operation_needs_to_be_blocked = False
190 else:
191 factory = generate_operation_adapter(method, version)
192
193@@ -428,15 +429,25 @@
194 register_adapter_for_version(
195 factory, interface, version, operation_provides,
196 operation_name, context.info)
197- if tag.get('is_mutator'):
198- operation_registered_as_mutator = True
199+ if (tag.get('is_mutator')
200+ and block_mutator_operations_as_of_version != None):
201+ defined_in = this_version_index
202+ blocked_as_of = config.active_versions.index(
203+ block_mutator_operations_as_of_version)
204+ if defined_in < blocked_as_of:
205+ # In this version, this mutator is also a
206+ # named operation. But in a later version
207+ # mutators stop being named operations, and
208+ # the operation registration for this mutator
209+ # will need to be blocked.
210+ mutator_operation_needs_to_be_blocked = True
211 previous_operation_name = operation_name
212 previous_operation_provides = operation_provides
213
214- if (operation_registered_as_mutator and
215- block_mutator_operations_as_of_version is not None):
216- # The operation was registered as a mutator, and it never
217- # got de-registered. De-register it now.
218+ if mutator_operation_needs_to_be_blocked:
219+ # The operation was registered as a mutator, back in the
220+ # days when mutator operations were also named operations,
221+ # and it never got de-registered. De-register it now.
222 register_adapter_for_version(
223 _mask_adapter_registration, interface,
224 block_mutator_operations_as_of_version,
225
226=== modified file 'src/lazr/restful/version.txt'
227--- src/lazr/restful/version.txt 2011-01-26 16:50:13 +0000
228+++ src/lazr/restful/version.txt 2011-02-16 14:39:11 +0000
229@@ -1,1 +1,1 @@
230-0.16.0
231+0.16.1

Subscribers

People subscribed via source and target branches