lazr.restful

Merge lp:~leonardr/lazr.restful/fix-request-user into lp:lazr.restful

  1. fix-request-user
  2. Merge into trunk
Proposed by Leonard Richardson
Status: Merged
Merged at revision: not available
Proposed branch: lp:~leonardr/lazr.restful/fix-request-user
Merge into: lp:lazr.restful
Diff against target: 212 lines (+67/-25)
4 files modified
src/lazr/restful/NEWS.txt (+6/-0)
src/lazr/restful/declarations.py (+10/-2)
src/lazr/restful/docs/webservice-declarations.txt (+50/-22)
src/lazr/restful/version.txt (+1/-1)
To merge this branch: bzr merge lp:~leonardr/lazr.restful/fix-request-user
Reviewer Review Type Date Requested Status
Paul Hummer (community) code Approve
Review via email: mp+19418@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Leonard Richardson (leonardr) wrote :

This branch changes REQUEST_USER from a marker object to a marker class. The underlying reason is that copy.deepcopy turns a marker object into a different object, but leaves a marker class alone.

Let's say that REQUEST_USER is <object object at 0x123>, and consider a named operation that has REQUEST_USER as a fixed value. The idea is that at runtime, lazr.restful will see that the fixed value is this marker object, determine the current user, and plug that in instead of the marker object.

Now consider what happens in a multiversioned environment, when the named operation's annotations dict for version '1.0' is run through copy.deepcopy() to create the basis for the annotations dict for version '2.0'.

In 2.0, the fixed value is a copy of REQUEST_USER, <object object at 0xabc>. At runtime, lazr.restful checks whether <object object at 0xabc> 'is' <object object at 0x123> and decides that the answer is no. Apparently the web service designer wants this random object <object object at 0xabc> passed in as the value for 'user'. The underlying function takes one look at this random object and barfs--it was expecting an instance of User!

By changing REQUEST_USER to a marker class, we ensure that copy.deepcopy doesn't change it. The '2.0' object is the same as the '1.0' object, and they both return True for 'is REQUEST_USER'.

I tested this by adding a fixed argument to the named operations in webservice-declaration, and making sure values were inherited between versions 2.0 and 3.0.

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

This all looks good.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/lazr/restful/NEWS.txt'
2--- src/lazr/restful/NEWS.txt 2010-02-15 14:34:37 +0000
3+++ src/lazr/restful/NEWS.txt 2010-02-16 16:27:16 +0000
4@@ -2,6 +2,12 @@
5 NEWS for lazr.restful
6 =====================
7
8+0.9.20 (2010-02-16)
9+===================
10+
11+Fixed a bug that broke multi-versioned named operations that take
12+the request user as a fixed argument.
13+
14 0.9.19 (2010-02-15)
15 ===================
16
17
18=== modified file 'src/lazr/restful/declarations.py'
19--- src/lazr/restful/declarations.py 2010-02-15 02:24:19 +0000
20+++ src/lazr/restful/declarations.py 2010-02-16 16:27:16 +0000
21@@ -73,8 +73,16 @@
22 'destructor', 'factory', 'read_operation', 'write_operation',
23 REMOVED_OPERATION_TYPE)
24
25-# Marker to specify that a parameter should contain the request user.
26-REQUEST_USER = object()
27+
28+class REQUEST_USER:
29+ """Marker class standing in for the user of the current request.
30+
31+ This is passed in to annotations like @call_with. This is a class
32+ rather than an object because it's going to be run through
33+ copy.deepcopy, and we want 'is REQUEST_USER' to succeed on the
34+ copy.
35+ """
36+ pass
37
38
39 def _check_called_from_interface_def(name):
40
41=== modified file 'src/lazr/restful/docs/webservice-declarations.txt'
42--- src/lazr/restful/docs/webservice-declarations.txt 2010-02-11 17:57:16 +0000
43+++ src/lazr/restful/docs/webservice-declarations.txt 2010-02-16 16:27:16 +0000
44@@ -177,7 +177,7 @@
45 >>> print_export_tag(ICheckedOutBookSet)
46 collection_default_content: {None: ('getByTitle',
47 {'title': '',
48- 'user': <object object ...>})}
49+ 'user': <class '...REQUEST_USER'>})}
50 collection_entry_schema: <InterfaceClass __builtin__.IBook>
51 type: 'collection'
52
53@@ -413,7 +413,7 @@
54
55 >>> print_export_tag(IBookOnSteroids['checkout'])
56 as: 'checkout'
57- call_with: {'kind': 'normal', 'who': <object...>}
58+ call_with: {'kind': 'normal', 'who': <class '...REQUEST_USER'>}
59 params: {}
60 return_type: None
61 type: 'write_operation'
62@@ -1991,23 +1991,26 @@
63 >>> class IMultiVersionMethod(Interface):
64 ... export_as_webservice_entry()
65 ...
66- ... @call_with(fixed='2.0 value')
67+ ...
68 ... @cache_for(300)
69+ ... @operation_for_version('3.0')
70+ ...
71+ ... @call_with(fixed='2.0 value', user=REQUEST_USER)
72 ... @operation_for_version('2.0')
73 ...
74- ... @call_with(fixed='1.0 value')
75+ ... @call_with(fixed='1.0 value', user=REQUEST_USER)
76 ... @export_operation_as('new_name')
77 ... @rename_parameters_as(required="required_argument")
78 ... @operation_for_version('1.0')
79 ...
80- ... @call_with(fixed='pre-1.0 value')
81+ ... @call_with(fixed='pre-1.0 value', user=REQUEST_USER)
82 ... @cache_for(100)
83 ... @operation_parameters(
84 ... required=TextLine(),
85 ... fixed=TextLine()
86 ... )
87 ... @export_read_operation()
88- ... def a_method(required, fixed):
89+ ... def a_method(required, fixed, user):
90 ... """Method demonstrating multiversion publication."""
91
92 Here's a simple implementation of IMultiVersionMethod. It'll
93@@ -2018,9 +2021,9 @@
94 ... """Simple IMultiVersionMethod implementation."""
95 ... implements(IMultiVersionMethod)
96 ...
97- ... def a_method(self, required, fixed='Fixed value'):
98- ... return "Required value: %s. Fixed value: %s" % (
99- ... required, fixed)
100+ ... def a_method(self, required, fixed, user):
101+ ... return "Required value: %s. Fixed value: %s. User: %s." % (
102+ ... required, fixed, user)
103
104 By passing a version string into generate_operation_adapter(), we can
105 get different adapter classes for different versions of the web
106@@ -2039,7 +2042,7 @@
107
108 >>> method_earliest = adapter_earliest_factory(data_object, request)
109 >>> print method_earliest.call(required="foo")
110- Required value: foo. Fixed value: pre-1.0 value
111+ Required value: foo. Fixed value: pre-1.0 value. User: A user.
112
113 Passing in '1.0' or '2.0' gets us the method as it appears in the
114 appropriate version of the web service. Note that the name of the
115@@ -2052,7 +2055,7 @@
116
117 >>> method_10 = adapter_10_factory(data_object, request)
118 >>> print method_10.call(required="bar")
119- Required value: bar. Fixed value: 1.0 value
120+ Required value: bar. Fixed value: 1.0 value. User: A user.
121
122 >>> adapter_20_factory = generate_operation_adapter(method, '2.0')
123 >>> print adapter_20_factory.__name__
124@@ -2060,7 +2063,14 @@
125
126 >>> method_20 = adapter_20_factory(data_object, request)
127 >>> print method_20.call(required="baz")
128- Required value: baz. Fixed value: 2.0 value
129+ Required value: baz. Fixed value: 2.0 value. User: A user.
130+
131+ >>> adapter_30_factory = generate_operation_adapter(method, '3.0')
132+ >>> print adapter_30_factory.__name__
133+ GET_IMultiVersionMethod_new_name_3_0
134+ >>> method_30 = adapter_30_factory(data_object, request)
135+ >>> print method_30.call(required="baz")
136+ Required value: baz. Fixed value: 2.0 value. User: A user.
137
138 An error occurs when we try to generate an adapter for a version
139 that's not mentioned in the annotations.
140@@ -2081,19 +2091,35 @@
141 but it's actually a stack of dictionaries named after the versions.
142
143 >>> dictionary.dict_names
144- [None, '1.0', '2.0']
145+ [None, '1.0', '2.0', '3.0']
146
147-The dictionary on top of the stack is for the 2.0 version of the web
148-service. In 2.0, the method is published as 'new_name' and its 'fixed'
149-argument is fixed to the string '2.0 value'.
150+The dictionary on top of the stack is for the 3.0 version of the web
151+service. This version inherits its name ('new_name') and its fixed
152+arguments ('2.0 value' and REQUEST_USER) from the 2.0 version, but it
153+also sets a new value for 'cache_for'.
154
155 >>> print dictionary['as']
156 new_name
157- >>> dictionary['call_with']
158- {'fixed': '2.0 value'}
159+ >>> print pformat(dictionary['call_with'])
160+ {'fixed': '2.0 value',
161+ 'user': <class '...REQUEST_USER'>}
162 >>> dictionary['cache_for']
163 300
164
165+Let's pop the 3.0 version off the stack. Now we can see how the method
166+looks in 2.0. In 2.0, the method is published as 'new_name' and its
167+'fixed' argument is fixed to the string '2.0 value'. It inherits its
168+value for 'cache_for' from version 1.0.
169+
170+ >>> ignored = dictionary.pop()
171+ >>> print dictionary['as']
172+ new_name
173+ >>> print pformat(dictionary['call_with'])
174+ {'fixed': '2.0 value',
175+ 'user': <class '...REQUEST_USER'>}
176+ >>> dictionary['cache_for']
177+ 100
178+
179 The published name of the 'required' argument is 'required_argument',
180 not 'required'.
181
182@@ -2108,8 +2134,9 @@
183 >>> ignored = dictionary.pop()
184 >>> print dictionary['as']
185 new_name
186- >>> dictionary['call_with']
187- {'fixed': '1.0 value'}
188+ >>> print pformat(dictionary['call_with'])
189+ {'fixed': '1.0 value',
190+ 'user': <class '...REQUEST_USER'>}
191 >>> print dictionary['params']['required'].__name__
192 required_argument
193 >>> dictionary['cache_for']
194@@ -2125,8 +2152,9 @@
195 a_method
196 >>> print dictionary['params']['required'].__name__
197 required
198- >>> dictionary['call_with']
199- {'fixed': 'pre-1.0 value'}
200+ >>> print pformat(dictionary['call_with'])
201+ {'fixed': 'pre-1.0 value',
202+ 'user': <class '...REQUEST_USER'>}
203 >>> dictionary['cache_for']
204 100
205
206
207=== modified file 'src/lazr/restful/version.txt'
208--- src/lazr/restful/version.txt 2010-02-15 14:34:37 +0000
209+++ src/lazr/restful/version.txt 2010-02-16 16:27:16 +0000
210@@ -1,1 +1,1 @@
211-0.9.19
212+0.9.20

Subscribers

People subscribed via source and target branches