Merge lp:~cjwatson/lazr.restful/double-closing-brace into lp:lazr.restful

Proposed by Colin Watson on 2018-09-28
Status: Merged
Merged at revision: 221
Proposed branch: lp:~cjwatson/lazr.restful/double-closing-brace
Merge into: lp:lazr.restful
Diff against target: 58 lines (+14/-5)
3 files modified
src/lazr/restful/NEWS.txt (+6/-0)
src/lazr/restful/_resource.py (+2/-3)
src/lazr/restful/docs/webservice.txt (+6/-2)
To merge this branch: bzr merge lp:~cjwatson/lazr.restful/double-closing-brace
Reviewer Review Type Date Requested Status
William Grant code 2018-09-28 Approve on 2018-10-01
Review via email: mp+355847@code.launchpad.net

Commit message

Fix double closing brace when encoding the result of a custom operation where the result has an adapter to ICollection.

Description of the change

Batch encoding is very weird; BatchingResourceMixin.batch intentionally omits the closing brace, and the caller is supposed to add it after possibly adding other parameters. This means that it's important for overridden implementations of the batch method in subclasses not to add the closing brace, as that violates the implied contract; only methods other than batch are allowed to add it. We were violating that rule in one code path, which resulted in a double closing brace.

This previously went unnoticed because of an overly-liberal doctest (which I had to fix anyway in order to cope with varying dict iteration order) and I think also because Launchpad tends to declare operations as having a return_type providing ICollectionField rather than directly returning something that can be adapted to ICollection.

To post a comment you must log in.
William Grant (wgrant) :
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 2018-02-21 15:18:34 +0000
3+++ src/lazr/restful/NEWS.txt 2018-09-28 15:11:57 +0000
4@@ -2,6 +2,12 @@
5 NEWS for lazr.restful
6 =====================
7
8+0.20.2
9+======
10+
11+Fix double closing brace when encoding the result of a custom operation
12+where the result has an adapter to ICollection.
13+
14 0.20.1 (2018-02-21)
15 ===================
16
17
18=== modified file 'src/lazr/restful/_resource.py'
19--- src/lazr/restful/_resource.py 2016-02-17 01:07:21 +0000
20+++ src/lazr/restful/_resource.py 2018-09-28 15:11:57 +0000
21@@ -1801,7 +1801,7 @@
22 self.request.response.setHeader('Content-Type', media_type)
23 return result
24
25- result = self.batch(entries)
26+ result = self.batch(entries) + '}'
27
28 self.request.response.setHeader('Content-type', self.JSON_TYPE)
29 return result
30@@ -1818,8 +1818,7 @@
31 request = self.request
32 result = super(CollectionResource, self).batch(entries, request)
33 result += (
34- ', "resource_type_link" : ' + simplejson.dumps(self.type_url)
35- + '}')
36+ ', "resource_type_link" : ' + simplejson.dumps(self.type_url))
37 return result
38
39 @property
40
41=== modified file 'src/lazr/restful/docs/webservice.txt'
42--- src/lazr/restful/docs/webservice.txt 2016-02-17 01:07:21 +0000
43+++ src/lazr/restful/docs/webservice.txt 2018-09-28 15:11:57 +0000
44@@ -1585,8 +1585,12 @@
45 batch from the collection.
46
47 >>> request, operation = make_dummy_operation_request(DishSet())
48- >>> operation()
49- '{"total_size": ..., "start": ...}'
50+ >>> response = operation()
51+ >>> for key, value in sorted(simplejson.loads(response).items()):
52+ ... print '%s: %s' % (key, value)
53+ entries: ...
54+ start: ...
55+ total_size: ...
56
57 If the return value can't be converted into JSON, you'll get an
58 exception.

Subscribers

People subscribed via source and target branches