Merge lp:~ricardokirkner/rnr-server/review-crud-api-with-macaroons into lp:rnr-server
- review-crud-api-with-macaroons
- Merge into trunk
Proposed by
Ricardo Kirkner
Status: | Merged |
---|---|
Approved by: | Ricardo Kirkner |
Approved revision: | 334 |
Merged at revision: | 318 |
Proposed branch: | lp:~ricardokirkner/rnr-server/review-crud-api-with-macaroons |
Merge into: | lp:rnr-server |
Diff against target: |
611 lines (+172/-107) 4 files modified
src/clickreviews/api/urls.py (+3/-1) src/clickreviews/tests/test_handlers.py (+160/-65) src/reviewsapp/api/urls.py (+2/-3) src/reviewsapp/tests/test_handlers.py (+7/-38) |
To merge this branch: | bzr merge lp:~ricardokirkner/rnr-server/review-crud-api-with-macaroons |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Fabián Ezequiel Gallina (community) | Approve | ||
Review via email: mp+294951@code.launchpad.net |
Commit message
add macaroons support for review CRUD apis
Description of the change
To post a comment you must log in.
Revision history for this message
Fabián Ezequiel Gallina (fgallina) wrote : | # |
Revision history for this message
Ricardo Kirkner (ricardokirkner) : | # |
- 332. By Ricardo Kirkner
-
use public names for helper methods
- 333. By Ricardo Kirkner
-
refactor to avoid duplication
- 334. By Ricardo Kirkner
-
assert changes on db when deleting a review
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'src/clickreviews/api/urls.py' | |||
2 | --- src/clickreviews/api/urls.py 2014-11-21 20:27:01 +0000 | |||
3 | +++ src/clickreviews/api/urls.py 2016-05-20 13:59:57 +0000 | |||
4 | @@ -9,6 +9,7 @@ | |||
5 | 9 | from piston.resource import Resource | 9 | from piston.resource import Resource |
6 | 10 | from core.api.auth import ( | 10 | from core.api.auth import ( |
7 | 11 | DelegatedSSOAuthentication, | 11 | DelegatedSSOAuthentication, |
8 | 12 | MacaroonsAuthentication, | ||
9 | 12 | SSOOAuthAuthentication, | 13 | SSOOAuthAuthentication, |
10 | 13 | ) | 14 | ) |
11 | 14 | from core.api.decorators import gzip_content, vary_only_on_headers | 15 | from core.api.decorators import gzip_content, vary_only_on_headers |
12 | @@ -24,9 +25,10 @@ | |||
13 | 24 | 25 | ||
14 | 25 | 26 | ||
15 | 26 | auth = SSOOAuthAuthentication(realm="Ratings and Reviews") | 27 | auth = SSOOAuthAuthentication(realm="Ratings and Reviews") |
16 | 28 | macaroons_auth = MacaroonsAuthentication(realm="Ratings and Reviews") | ||
17 | 27 | 29 | ||
18 | 28 | review_resource = CSRFExemptResource(handler=ClickReviewHandler, | 30 | review_resource = CSRFExemptResource(handler=ClickReviewHandler, |
20 | 29 | authentication=auth) | 31 | authentication=(macaroons_auth, auth)) |
21 | 30 | review_resource.callmap['PATCH'] = 'patch' | 32 | review_resource.callmap['PATCH'] = 'patch' |
22 | 31 | review_resource = never_cache(review_resource) | 33 | review_resource = never_cache(review_resource) |
23 | 32 | 34 | ||
24 | 33 | 35 | ||
25 | === modified file 'src/clickreviews/tests/test_handlers.py' | |||
26 | --- src/clickreviews/tests/test_handlers.py 2015-04-22 14:04:11 +0000 | |||
27 | +++ src/clickreviews/tests/test_handlers.py 2016-05-20 13:59:57 +0000 | |||
28 | @@ -12,6 +12,7 @@ | |||
29 | 12 | from django.core.cache import cache | 12 | from django.core.cache import cache |
30 | 13 | from django.core.serializers.json import DateTimeAwareJSONEncoder | 13 | from django.core.serializers.json import DateTimeAwareJSONEncoder |
31 | 14 | from django.core.urlresolvers import reverse | 14 | from django.core.urlresolvers import reverse |
32 | 15 | from mock import patch | ||
33 | 15 | 16 | ||
34 | 16 | from clickreviews.api.urls import sca_delegated_sso_auth | 17 | from clickreviews.api.urls import sca_delegated_sso_auth |
35 | 17 | from clickreviews.models import ( | 18 | from clickreviews.models import ( |
36 | @@ -23,10 +24,28 @@ | |||
37 | 23 | ) | 24 | ) |
38 | 24 | from clickreviews.tests.factory import TestCaseWithFactory | 25 | from clickreviews.tests.factory import TestCaseWithFactory |
39 | 25 | from core.models import BaseModeration | 26 | from core.models import BaseModeration |
44 | 26 | from core.tests.helpers import SSOTestCaseMixin, assert_no_extra_queries_after | 27 | from core.tests.doubles import SCADouble |
45 | 27 | 28 | from core.tests.helpers import ( | |
46 | 28 | 29 | RequestsDoubleTestCaseMixin, | |
47 | 29 | class ClickReviewsHandlerTestCase(TestCaseWithFactory): | 30 | SSOTestCaseMixin, |
48 | 31 | assert_no_extra_queries_after, | ||
49 | 32 | ) | ||
50 | 33 | |||
51 | 34 | |||
52 | 35 | class MacaroonsAuthenticatedTestCase(TestCaseWithFactory, | ||
53 | 36 | RequestsDoubleTestCaseMixin): | ||
54 | 37 | |||
55 | 38 | def setUp(self): | ||
56 | 39 | super(MacaroonsAuthenticatedTestCase, self).setUp() | ||
57 | 40 | |||
58 | 41 | self.patch_requests() | ||
59 | 42 | self.sca_double = SCADouble(self.requests_double) | ||
60 | 43 | p = patch('core.utilities.web_services', new=self.sca_double) | ||
61 | 44 | p.start() | ||
62 | 45 | self.addCleanup(p.stop) | ||
63 | 46 | |||
64 | 47 | |||
65 | 48 | class ClickReviewsHandlerTestCase(MacaroonsAuthenticatedTestCase): | ||
66 | 30 | 49 | ||
67 | 31 | def setUp(self): | 50 | def setUp(self): |
68 | 32 | super(ClickReviewsHandlerTestCase, self).setUp() | 51 | super(ClickReviewsHandlerTestCase, self).setUp() |
69 | @@ -58,36 +77,38 @@ | |||
70 | 58 | 'arch_tag': 'i386', | 77 | 'arch_tag': 'i386', |
71 | 59 | } | 78 | } |
72 | 60 | 79 | ||
74 | 61 | def _post_new_review(self, data, user=None, verify_result=True): | 80 | def post_new_review(self, data, user=None, verify_result=True, |
75 | 81 | auth='oauth'): | ||
76 | 62 | # Post a review as an authenticated user. | 82 | # Post a review as an authenticated user. |
77 | 63 | url = reverse('click-api-reviews') | 83 | url = reverse('click-api-reviews') |
81 | 64 | if user is None: | 84 | headers = {} |
82 | 65 | user = self.factory.makeUser() | 85 | if auth == 'macaroon': |
83 | 66 | self.client.login(username=user.username, password='test') | 86 | authorization = 'Macaroon root="%s", discharge="%s"' % ( |
84 | 87 | 'root-macaroon-data', 'discharge-macaroon-data') | ||
85 | 88 | headers['HTTP_AUTHORIZATION'] = authorization | ||
86 | 89 | else: | ||
87 | 90 | if user is None: | ||
88 | 91 | user = self.factory.makeUser() | ||
89 | 92 | self.client.login(username=user.username, password='test') | ||
90 | 67 | 93 | ||
91 | 68 | response = self.client.post( | 94 | response = self.client.post( |
93 | 69 | url, data=data, content_type='application/json') | 95 | url, data=data, content_type='application/json', **headers) |
94 | 70 | return response | 96 | return response |
95 | 71 | 97 | ||
102 | 72 | def test_create(self): | 98 | def assert_review_created(self, response, expected): |
97 | 73 | data = self.make_data(package_name='com.example.me.myapp', | ||
98 | 74 | rating=5) | ||
99 | 75 | |||
100 | 76 | response = self._post_new_review(json.dumps(data)) | ||
101 | 77 | |||
103 | 78 | # Piston uses 200, rather than 201. | 99 | # Piston uses 200, rather than 201. |
104 | 79 | self.assertEqual(httplib.OK, response.status_code) | 100 | self.assertEqual(httplib.OK, response.status_code) |
105 | 80 | self.assertEqual('application/json; charset=utf-8', | 101 | self.assertEqual('application/json; charset=utf-8', |
106 | 81 | response['content-type']) | 102 | response['content-type']) |
107 | 82 | reviews = ClickPackageReview.objects.filter( | 103 | reviews = ClickPackageReview.objects.filter( |
109 | 83 | click_package__package_name='com.example.me.myapp') | 104 | click_package__package_name=expected['package_name']) |
110 | 84 | self.assertEqual(1, reviews.count()) | 105 | self.assertEqual(1, reviews.count()) |
111 | 85 | review = reviews[0] | 106 | review = reviews[0] |
112 | 86 | response_dict = json.loads(response.content) | 107 | response_dict = json.loads(response.content) |
113 | 87 | self.assertEqual({ | 108 | self.assertEqual({ |
114 | 88 | 'id': review.id, | 109 | 'id': review.id, |
117 | 89 | 'package_name': 'com.example.me.myapp', | 110 | 'package_name': expected['package_name'], |
118 | 90 | 'language': data['language'], | 111 | 'language': expected['language'], |
119 | 91 | 'date_created': DateTimeAwareJSONEncoder().encode( | 112 | 'date_created': DateTimeAwareJSONEncoder().encode( |
120 | 92 | review.date_created).strip('"'), | 113 | review.date_created).strip('"'), |
121 | 93 | 'rating': 5, | 114 | 'rating': 5, |
122 | @@ -102,10 +123,27 @@ | |||
123 | 102 | 'usefulness_favorable': 0, | 123 | 'usefulness_favorable': 0, |
124 | 103 | }, response_dict) | 124 | }, response_dict) |
125 | 104 | 125 | ||
126 | 126 | def test_create(self): | ||
127 | 127 | data = self.make_data(package_name='com.example.me.myapp', | ||
128 | 128 | rating=5) | ||
129 | 129 | |||
130 | 130 | response = self.post_new_review(json.dumps(data)) | ||
131 | 131 | self.assert_review_created(response, data) | ||
132 | 132 | |||
133 | 133 | def test_create_using_macaroon_auth(self): | ||
134 | 134 | user = self.factory.makeUser() | ||
135 | 135 | account = self.make_account_data(user) | ||
136 | 136 | self.sca_double.set_verify_acl_response(account=account) | ||
137 | 137 | data = self.make_data(package_name='com.example.me.myapp', | ||
138 | 138 | rating=5) | ||
139 | 139 | response = self.post_new_review( | ||
140 | 140 | json.dumps(data), user=user, auth='macaroon') | ||
141 | 141 | self.assert_review_created(response, data) | ||
142 | 142 | |||
143 | 105 | def test_invalid_rating(self): | 143 | def test_invalid_rating(self): |
144 | 106 | data = self.make_data(rating=6) | 144 | data = self.make_data(rating=6) |
145 | 107 | 145 | ||
147 | 108 | response = self._post_new_review(json.dumps(data)) | 146 | response = self.post_new_review(json.dumps(data)) |
148 | 109 | 147 | ||
149 | 110 | self.assertEqual(httplib.BAD_REQUEST, response.status_code) | 148 | self.assertEqual(httplib.BAD_REQUEST, response.status_code) |
150 | 111 | 149 | ||
151 | @@ -116,7 +154,7 @@ | |||
152 | 116 | data = valid_data.copy() | 154 | data = valid_data.copy() |
153 | 117 | del data[key] | 155 | del data[key] |
154 | 118 | 156 | ||
156 | 119 | response = self._post_new_review(json.dumps(data)) | 157 | response = self.post_new_review(json.dumps(data)) |
157 | 120 | 158 | ||
158 | 121 | self.assertEqual(httplib.BAD_REQUEST, response.status_code) | 159 | self.assertEqual(httplib.BAD_REQUEST, response.status_code) |
159 | 122 | 160 | ||
160 | @@ -127,13 +165,13 @@ | |||
161 | 127 | click_package = ClickPackage.objects.create( | 165 | click_package = ClickPackage.objects.create( |
162 | 128 | package_name='com.example.me.myapp') | 166 | package_name='com.example.me.myapp') |
163 | 129 | 167 | ||
165 | 130 | self._post_new_review(json.dumps(data)) | 168 | self.post_new_review(json.dumps(data)) |
166 | 131 | 169 | ||
167 | 132 | click_package = ClickPackage.objects.get(id=click_package.id) | 170 | click_package = ClickPackage.objects.get(id=click_package.id) |
168 | 133 | self.assertEqual(1, click_package.ratings_total) | 171 | self.assertEqual(1, click_package.ratings_total) |
169 | 134 | self.assertEqual(5, click_package.ratings_average) | 172 | self.assertEqual(5, click_package.ratings_average) |
170 | 135 | 173 | ||
172 | 136 | def _get_reviews(self, data=None): | 174 | def get_reviews(self, data=None): |
173 | 137 | url = reverse('click-api-reviews') | 175 | url = reverse('click-api-reviews') |
174 | 138 | kwargs = {} | 176 | kwargs = {} |
175 | 139 | if data is not None: | 177 | if data is not None: |
176 | @@ -146,7 +184,7 @@ | |||
177 | 146 | for _ in range(5): | 184 | for _ in range(5): |
178 | 147 | self.factory.makeClickPackageReview(click_package=package) | 185 | self.factory.makeClickPackageReview(click_package=package) |
179 | 148 | 186 | ||
181 | 149 | json_result = self._get_reviews(data={ | 187 | json_result = self.get_reviews(data={ |
182 | 150 | 'package_name': package.package_name, | 188 | 'package_name': package.package_name, |
183 | 151 | }) | 189 | }) |
184 | 152 | 190 | ||
185 | @@ -163,7 +201,7 @@ | |||
186 | 163 | version='0.1.1', date_created=datetime( | 201 | version='0.1.1', date_created=datetime( |
187 | 164 | 2014, 1, 15, 11, 13, 55, 123456, tzinfo=pytz.UTC)) | 202 | 2014, 1, 15, 11, 13, 55, 123456, tzinfo=pytz.UTC)) |
188 | 165 | 203 | ||
190 | 166 | json_result = self._get_reviews(data={ | 204 | json_result = self.get_reviews(data={ |
191 | 167 | 'package_name': package.package_name, | 205 | 'package_name': package.package_name, |
192 | 168 | }) | 206 | }) |
193 | 169 | self.assertEqual(200, json_result.status_code) | 207 | self.assertEqual(200, json_result.status_code) |
194 | @@ -190,7 +228,7 @@ | |||
195 | 190 | }, result_review) | 228 | }, result_review) |
196 | 191 | 229 | ||
197 | 192 | def test_must_filter_by_packagename(self): | 230 | def test_must_filter_by_packagename(self): |
199 | 193 | result = self._get_reviews() | 231 | result = self.get_reviews() |
200 | 194 | 232 | ||
201 | 195 | self.assertEqual(400, result.status_code) | 233 | self.assertEqual(400, result.status_code) |
202 | 196 | self.assertEqual(['package_name'], | 234 | self.assertEqual(['package_name'], |
203 | @@ -202,7 +240,7 @@ | |||
204 | 202 | click_package=package, | 240 | click_package=package, |
205 | 203 | deleted=True) | 241 | deleted=True) |
206 | 204 | 242 | ||
208 | 205 | json_result = self._get_reviews(data={ | 243 | json_result = self.get_reviews(data={ |
209 | 206 | 'package_name': package.package_name, | 244 | 'package_name': package.package_name, |
210 | 207 | }) | 245 | }) |
211 | 208 | self.assertEqual(200, json_result.status_code) | 246 | self.assertEqual(200, json_result.status_code) |
212 | @@ -213,7 +251,7 @@ | |||
213 | 213 | package = self.factory.makeClickPackage() | 251 | package = self.factory.makeClickPackage() |
214 | 214 | self.factory.makeClickPackageReview(click_package=package, hide=True) | 252 | self.factory.makeClickPackageReview(click_package=package, hide=True) |
215 | 215 | 253 | ||
217 | 216 | json_result = self._get_reviews(data={ | 254 | json_result = self.get_reviews(data={ |
218 | 217 | 'package_name': package.package_name, | 255 | 'package_name': package.package_name, |
219 | 218 | }) | 256 | }) |
220 | 219 | self.assertEqual(200, json_result.status_code) | 257 | self.assertEqual(200, json_result.status_code) |
221 | @@ -240,7 +278,7 @@ | |||
222 | 240 | reviews[2].id, | 278 | reviews[2].id, |
223 | 241 | ] | 279 | ] |
224 | 242 | 280 | ||
226 | 243 | json_result = self._get_reviews(data={ | 281 | json_result = self.get_reviews(data={ |
227 | 244 | 'package_name': package.package_name, | 282 | 'package_name': package.package_name, |
228 | 245 | }) | 283 | }) |
229 | 246 | self.assertEqual(200, json_result.status_code) | 284 | self.assertEqual(200, json_result.status_code) |
230 | @@ -255,33 +293,63 @@ | |||
231 | 255 | self.factory.makeClickPackageReview(click_package=package) | 293 | self.factory.makeClickPackageReview(click_package=package) |
232 | 256 | assert_no_extra_queries_after( | 294 | assert_no_extra_queries_after( |
233 | 257 | lambda: self.factory.makeClickPackageReview(click_package=package), | 295 | lambda: self.factory.makeClickPackageReview(click_package=package), |
235 | 258 | self._get_reviews, data={'package_name': package.package_name} | 296 | self.get_reviews, data={'package_name': package.package_name} |
236 | 259 | ) | 297 | ) |
237 | 260 | 298 | ||
239 | 261 | def _update_review(self, review, data, user=None): | 299 | def update_review(self, review, data, user=None, auth='oauth'): |
240 | 262 | # Post a review as an authenticated user. | 300 | # Post a review as an authenticated user. |
241 | 263 | url = reverse('click-api-reviews', args=(review.pk,)) | 301 | url = reverse('click-api-reviews', args=(review.pk,)) |
245 | 264 | if user is None: | 302 | headers = {} |
246 | 265 | user = review.reviewer | 303 | if auth == 'macaroon': |
247 | 266 | self.client.login(username=user.username, password='test') | 304 | authorization = 'Macaroon root="%s", discharge="%s"' % ( |
248 | 305 | 'root-macaroon-data', 'discharge-macaroon-data') | ||
249 | 306 | headers['HTTP_AUTHORIZATION'] = authorization | ||
250 | 307 | else: | ||
251 | 308 | if user is None: | ||
252 | 309 | user = review.reviewer | ||
253 | 310 | self.client.login(username=user.username, password='test') | ||
254 | 267 | response = self.client.put( | 311 | response = self.client.put( |
256 | 268 | url, data=json.dumps(data), content_type='application/json') | 312 | url, data=json.dumps(data), content_type='application/json', |
257 | 313 | **headers) | ||
258 | 269 | return response | 314 | return response |
259 | 270 | 315 | ||
260 | 316 | def assert_review_updated(self, response, expected): | ||
261 | 317 | self.assertEqual(response.status_code, httplib.OK) | ||
262 | 318 | review = json.loads(response.content) | ||
263 | 319 | for key, value in expected.items(): | ||
264 | 320 | self.assertEqual(review[key], value) | ||
265 | 321 | |||
266 | 271 | def test_update_review(self): | 322 | def test_update_review(self): |
267 | 272 | review = self.factory.makeClickPackageReview() | 323 | review = self.factory.makeClickPackageReview() |
268 | 273 | data = {'summary': 'New summary', | 324 | data = {'summary': 'New summary', |
269 | 274 | 'review_text': 'Review text', | 325 | 'review_text': 'Review text', |
270 | 275 | 'rating': 5} | 326 | 'rating': 5} |
276 | 276 | response = self._update_review(review, data) | 327 | response = self.update_review(review, data) |
277 | 277 | self.assertEqual(response.status_code, httplib.OK) | 328 | self.assert_review_updated(response, data) |
278 | 278 | review = json.loads(response.content) | 329 | |
279 | 279 | for key, value in data.items(): | 330 | def make_account_data(self, user): |
280 | 280 | self.assertEqual(review[key], data[key]) | 331 | return { |
281 | 332 | 'verified': True, | ||
282 | 333 | 'openid': user.useropenid_set.first().claimed_id.split('/')[-1], | ||
283 | 334 | 'email': user.email, | ||
284 | 335 | 'displayname': user.username, | ||
285 | 336 | } | ||
286 | 337 | |||
287 | 338 | def test_update_review_using_macaroon_auth(self): | ||
288 | 339 | user = self.factory.makeUser() | ||
289 | 340 | account = self.make_account_data(user) | ||
290 | 341 | self.sca_double.set_verify_acl_response(account=account) | ||
291 | 342 | review = self.factory.makeClickPackageReview(reviewer=user) | ||
292 | 343 | data = {'summary': 'New summary', | ||
293 | 344 | 'review_text': 'Review text', | ||
294 | 345 | 'rating': 5} | ||
295 | 346 | response = self.update_review( | ||
296 | 347 | review, data, user=user, auth='macaroon') | ||
297 | 348 | self.assert_review_updated(response, data) | ||
298 | 281 | 349 | ||
299 | 282 | def test_update_review_missing_data(self): | 350 | def test_update_review_missing_data(self): |
300 | 283 | review = self.factory.makeClickPackageReview() | 351 | review = self.factory.makeClickPackageReview() |
302 | 284 | response = self._update_review(review, {}) | 352 | response = self.update_review(review, {}) |
303 | 285 | self.assertEqual(response.status_code, httplib.BAD_REQUEST) | 353 | self.assertEqual(response.status_code, httplib.BAD_REQUEST) |
304 | 286 | errors = json.loads(response.content) | 354 | errors = json.loads(response.content) |
305 | 287 | expected = {'errors': | 355 | expected = {'errors': |
306 | @@ -293,45 +361,72 @@ | |||
307 | 293 | def test_update_review_wrong_user(self): | 361 | def test_update_review_wrong_user(self): |
308 | 294 | review = self.factory.makeClickPackageReview() | 362 | review = self.factory.makeClickPackageReview() |
309 | 295 | user = self.factory.makeUser() | 363 | user = self.factory.makeUser() |
311 | 296 | response = self._update_review(review, {}, user) | 364 | response = self.update_review(review, {}, user) |
312 | 297 | self.assertEqual(response.status_code, httplib.NOT_FOUND) | 365 | self.assertEqual(response.status_code, httplib.NOT_FOUND) |
313 | 298 | 366 | ||
315 | 299 | def _delete_review(self, review, user=None): | 367 | def delete_review(self, review, user=None, auth='oauth'): |
316 | 300 | # Delete a review as an authenticated user. | 368 | # Delete a review as an authenticated user. |
317 | 301 | url = reverse('click-api-reviews', args=(review.pk,)) | 369 | url = reverse('click-api-reviews', args=(review.pk,)) |
322 | 302 | if user is None: | 370 | headers = {} |
323 | 303 | user = review.reviewer | 371 | if auth == 'macaroon': |
324 | 304 | self.client.login(username=user.username, password='test') | 372 | authorization = 'Macaroon root="%s", discharge="%s"' % ( |
325 | 305 | response = self.client.delete(url) | 373 | 'root-macaroon-data', 'discharge-macaroon-data') |
326 | 374 | headers['HTTP_AUTHORIZATION'] = authorization | ||
327 | 375 | else: | ||
328 | 376 | if user is None: | ||
329 | 377 | user = review.reviewer | ||
330 | 378 | self.client.login(username=user.username, password='test') | ||
331 | 379 | response = self.client.delete(url, **headers) | ||
332 | 306 | return response | 380 | return response |
333 | 307 | 381 | ||
334 | 382 | def assert_review_deleted(self, response): | ||
335 | 383 | self.assertEqual(response.status_code, httplib.OK) | ||
336 | 384 | review = json.loads(response.content) | ||
337 | 385 | db_review = ClickPackageReview.objects.get(id=review['id']) | ||
338 | 386 | self.assertTrue(db_review.hide) | ||
339 | 387 | self.assertIsNotNone(db_review.date_deleted) | ||
340 | 388 | # XXX: ignore time differences at the microsecond level | ||
341 | 389 | date_deleted = db_review.date_deleted | ||
342 | 390 | parsed = datetime.strptime( | ||
343 | 391 | review['date_deleted'], '%Y-%m-%dT%H:%M:%S.%fZ') | ||
344 | 392 | self.assertEqual( | ||
345 | 393 | date_deleted.replace(microsecond=0), | ||
346 | 394 | parsed.replace(microsecond=0, tzinfo=date_deleted.tzinfo)) | ||
347 | 395 | |||
348 | 308 | def test_delete_review(self): | 396 | def test_delete_review(self): |
349 | 309 | review = self.factory.makeClickPackageReview() | 397 | review = self.factory.makeClickPackageReview() |
353 | 310 | response = self._delete_review(review) | 398 | response = self.delete_review(review) |
354 | 311 | self.assertEqual(response.status_code, httplib.OK) | 399 | self.assert_review_deleted(response) |
355 | 312 | review = json.loads(response.content) | 400 | |
356 | 401 | def test_delete_review_using_macaroon_auth(self): | ||
357 | 402 | user = self.factory.makeUser() | ||
358 | 403 | account = self.make_account_data(user) | ||
359 | 404 | self.sca_double.set_verify_acl_response(account=account) | ||
360 | 405 | review = self.factory.makeClickPackageReview(reviewer=user) | ||
361 | 406 | response = self.delete_review(review, auth='macaroon') | ||
362 | 407 | self.assert_review_deleted(response) | ||
363 | 313 | 408 | ||
364 | 314 | def test_delete_review_wrong_id(self): | 409 | def test_delete_review_wrong_id(self): |
365 | 315 | review = self.factory.makeClickPackageReview() | 410 | review = self.factory.makeClickPackageReview() |
366 | 316 | ClickPackageReview.objects.all().delete() | 411 | ClickPackageReview.objects.all().delete() |
368 | 317 | response = self._delete_review(review) | 412 | response = self.delete_review(review) |
369 | 318 | self.assertEqual(response.status_code, httplib.NOT_FOUND) | 413 | self.assertEqual(response.status_code, httplib.NOT_FOUND) |
370 | 319 | 414 | ||
371 | 320 | def test_delete_already_deleted(self): | 415 | def test_delete_already_deleted(self): |
372 | 321 | review = self.factory.makeClickPackageReview() | 416 | review = self.factory.makeClickPackageReview() |
373 | 322 | review.delete() | 417 | review.delete() |
375 | 323 | response = self._delete_review(review) | 418 | response = self.delete_review(review) |
376 | 324 | self.assertEqual(response.status_code, httplib.NOT_FOUND) | 419 | self.assertEqual(response.status_code, httplib.NOT_FOUND) |
377 | 325 | 420 | ||
378 | 326 | def test_delete_hidden_review(self): | 421 | def test_delete_hidden_review(self): |
379 | 327 | review = self.factory.makeClickPackageReview(hide=True) | 422 | review = self.factory.makeClickPackageReview(hide=True) |
381 | 328 | response = self._delete_review(review) | 423 | response = self.delete_review(review) |
382 | 329 | self.assertEqual(response.status_code, httplib.NOT_FOUND) | 424 | self.assertEqual(response.status_code, httplib.NOT_FOUND) |
383 | 330 | 425 | ||
384 | 331 | def test_delete_review_wrong_user(self): | 426 | def test_delete_review_wrong_user(self): |
385 | 332 | review = self.factory.makeClickPackageReview() | 427 | review = self.factory.makeClickPackageReview() |
386 | 333 | user = self.factory.makeUser() | 428 | user = self.factory.makeUser() |
388 | 334 | response = self._delete_review(review, user=user) | 429 | response = self.delete_review(review, user=user) |
389 | 335 | self.assertEqual(response.status_code, httplib.NOT_FOUND) | 430 | self.assertEqual(response.status_code, httplib.NOT_FOUND) |
390 | 336 | 431 | ||
391 | 337 | 432 | ||
392 | @@ -368,7 +463,7 @@ | |||
393 | 368 | cache._cache.clear() | 463 | cache._cache.clear() |
394 | 369 | cache._expire_info.clear() | 464 | cache._expire_info.clear() |
395 | 370 | 465 | ||
397 | 371 | def _request_stats(self, data=None, headers=None, **kwargs): | 466 | def request_stats(self, data=None, headers=None, **kwargs): |
398 | 372 | url = reverse( | 467 | url = reverse( |
399 | 373 | 'click-api-reviews-stats', kwargs=kwargs) | 468 | 'click-api-reviews-stats', kwargs=kwargs) |
400 | 374 | if data is None: | 469 | if data is None: |
401 | @@ -385,7 +480,7 @@ | |||
402 | 385 | click_package=self.factory.makeClickPackage('package_bar'), | 480 | click_package=self.factory.makeClickPackage('package_bar'), |
403 | 386 | language='de') | 481 | language='de') |
404 | 387 | 482 | ||
406 | 388 | response = self._request_stats() | 483 | response = self.request_stats() |
407 | 389 | reviews = json.loads(response.content) | 484 | reviews = json.loads(response.content) |
408 | 390 | 485 | ||
409 | 391 | foo = foo_review.click_package | 486 | foo = foo_review.click_package |
410 | @@ -405,7 +500,7 @@ | |||
411 | 405 | click_package=self.factory.makeClickPackage('package_bar'), | 500 | click_package=self.factory.makeClickPackage('package_bar'), |
412 | 406 | language='de') | 501 | language='de') |
413 | 407 | 502 | ||
415 | 408 | response = self._request_stats(package_name='package_foo') | 503 | response = self.request_stats(package_name='package_foo') |
416 | 409 | reviews = json.loads(response.content) | 504 | reviews = json.loads(response.content) |
417 | 410 | 505 | ||
418 | 411 | foo = foo_review.click_package | 506 | foo = foo_review.click_package |
419 | @@ -413,12 +508,12 @@ | |||
420 | 413 | pk=foo.pk, package_name=foo.package_name)) | 508 | pk=foo.pk, package_name=foo.package_name)) |
421 | 414 | 509 | ||
422 | 415 | def test_read_for_missing_package(self): | 510 | def test_read_for_missing_package(self): |
424 | 416 | response = self._request_stats(package_name='not_found') | 511 | response = self.request_stats(package_name='not_found') |
425 | 417 | self.assertEqual(response.status_code, 404) | 512 | self.assertEqual(response.status_code, 404) |
426 | 418 | 513 | ||
427 | 419 | def test_request_cached_with_extra_headers(self): | 514 | def test_request_cached_with_extra_headers(self): |
428 | 420 | # Requests for server status are never cached. | 515 | # Requests for server status are never cached. |
430 | 421 | response = self._request_stats() | 516 | response = self.request_stats() |
431 | 422 | 517 | ||
432 | 423 | self.assertEqual( | 518 | self.assertEqual( |
433 | 424 | 'max-age={0}'.format(settings.CACHE_REVIEW_STATS_SECONDS), | 519 | 'max-age={0}'.format(settings.CACHE_REVIEW_STATS_SECONDS), |
434 | @@ -427,13 +522,13 @@ | |||
435 | 427 | # A subsequent call will not hit the view. | 522 | # A subsequent call will not hit the view. |
436 | 428 | handler = 'clickreviews.api.handlers.ClickReviewsStatsHandler.read' | 523 | handler = 'clickreviews.api.handlers.ClickReviewsStatsHandler.read' |
437 | 429 | with mock.patch(handler) as mock_read: | 524 | with mock.patch(handler) as mock_read: |
439 | 430 | response = self._request_stats() | 525 | response = self.request_stats() |
440 | 431 | self.assertFalse(mock_read.called) | 526 | self.assertFalse(mock_read.called) |
441 | 432 | 527 | ||
442 | 433 | def test_cannot_request_with_get_params(self): | 528 | def test_cannot_request_with_get_params(self): |
443 | 434 | # Requesting the un-cached response (by adding get params) is | 529 | # Requesting the un-cached response (by adding get params) is |
444 | 435 | # not allowed. | 530 | # not allowed. |
446 | 436 | response = self._request_stats(data=dict(foo='bar')) | 531 | response = self.request_stats(data=dict(foo='bar')) |
447 | 437 | 532 | ||
448 | 438 | self.assertEqual(400, response.status_code) | 533 | self.assertEqual(400, response.status_code) |
449 | 439 | 534 | ||
450 | @@ -447,7 +542,7 @@ | |||
451 | 447 | click_package=self.factory.makeClickPackage('package_bar'), | 542 | click_package=self.factory.makeClickPackage('package_bar'), |
452 | 448 | language='de') | 543 | language='de') |
453 | 449 | 544 | ||
455 | 450 | response = self._request_stats( | 545 | response = self.request_stats( |
456 | 451 | headers={'HTTP_ACCEPT_ENCODING': 'gzip, deflate'}) | 546 | headers={'HTTP_ACCEPT_ENCODING': 'gzip, deflate'}) |
457 | 452 | 547 | ||
458 | 453 | fileobj = BytesIO() | 548 | fileobj = BytesIO() |
459 | @@ -468,7 +563,7 @@ | |||
460 | 468 | pk=bar.pk, package_name=bar.package_name)]) | 563 | pk=bar.pk, package_name=bar.package_name)]) |
461 | 469 | 564 | ||
462 | 470 | def test_vary_headers(self): | 565 | def test_vary_headers(self): |
464 | 471 | response = self._request_stats() | 566 | response = self.request_stats() |
465 | 472 | self.assertEqual(200, response.status_code) | 567 | self.assertEqual(200, response.status_code) |
466 | 473 | self.assertTrue(response.has_header('vary')) | 568 | self.assertTrue(response.has_header('vary')) |
467 | 474 | vary = response['vary'] | 569 | vary = response['vary'] |
468 | @@ -692,7 +787,7 @@ | |||
469 | 692 | self.addCleanup(p.stop) | 787 | self.addCleanup(p.stop) |
470 | 693 | self.patch_sso() | 788 | self.patch_sso() |
471 | 694 | 789 | ||
473 | 695 | def _make_delegated_sso_signed_headers(self, url, data=None): | 790 | def make_delegated_sso_signed_headers(self, url, data=None): |
474 | 696 | if data is None: | 791 | if data is None: |
475 | 697 | data = '' | 792 | data = '' |
476 | 698 | url = 'http://testserver' + url | 793 | url = 'http://testserver' + url |
477 | @@ -705,7 +800,7 @@ | |||
478 | 705 | 800 | ||
479 | 706 | def request_get(self, url, authenticated=True, **headers): | 801 | def request_get(self, url, authenticated=True, **headers): |
480 | 707 | if authenticated: | 802 | if authenticated: |
482 | 708 | headers.update(self._make_delegated_sso_signed_headers(url)) | 803 | headers.update(self.make_delegated_sso_signed_headers(url)) |
483 | 709 | return self.client.get(url, **headers) | 804 | return self.client.get(url, **headers) |
484 | 710 | 805 | ||
485 | 711 | def request_patch( | 806 | def request_patch( |
486 | @@ -714,7 +809,7 @@ | |||
487 | 714 | if data is None: | 809 | if data is None: |
488 | 715 | data = '' | 810 | data = '' |
489 | 716 | if authenticated: | 811 | if authenticated: |
491 | 717 | headers.update(self._make_delegated_sso_signed_headers(url, data)) | 812 | headers.update(self.make_delegated_sso_signed_headers(url, data)) |
492 | 718 | return self.client.patch( | 813 | return self.client.patch( |
493 | 719 | url, data=data, content_type=content_type, **headers) | 814 | url, data=data, content_type=content_type, **headers) |
494 | 720 | 815 | ||
495 | 721 | 816 | ||
496 | === modified file 'src/reviewsapp/api/urls.py' | |||
497 | --- src/reviewsapp/api/urls.py 2016-05-16 14:32:26 +0000 | |||
498 | +++ src/reviewsapp/api/urls.py 2016-05-20 13:59:57 +0000 | |||
499 | @@ -22,7 +22,7 @@ | |||
500 | 22 | ) | 22 | ) |
501 | 23 | 23 | ||
502 | 24 | from piston.resource import Resource | 24 | from piston.resource import Resource |
504 | 25 | from core.api.auth import MacaroonsAuthentication, SSOOAuthAuthentication | 25 | from core.api.auth import SSOOAuthAuthentication |
505 | 26 | from core.api.decorators import ( | 26 | from core.api.decorators import ( |
506 | 27 | gzip_content, | 27 | gzip_content, |
507 | 28 | vary_only_on_accept, | 28 | vary_only_on_accept, |
508 | @@ -44,7 +44,6 @@ | |||
509 | 44 | ) | 44 | ) |
510 | 45 | 45 | ||
511 | 46 | auth = SSOOAuthAuthentication(realm="Ratings and Reviews") | 46 | auth = SSOOAuthAuthentication(realm="Ratings and Reviews") |
512 | 47 | macaroons_auth = MacaroonsAuthentication(realm="Ratings and Reviews") | ||
513 | 48 | 47 | ||
514 | 49 | 48 | ||
515 | 50 | # The server status resource should never be cached. | 49 | # The server status resource should never be cached. |
516 | @@ -101,7 +100,7 @@ | |||
517 | 101 | # The following POST handlers have POST data and will not be cached. | 100 | # The following POST handlers have POST data and will not be cached. |
518 | 102 | # The .__name__ hack is needed to support Django 1.1 | 101 | # The .__name__ hack is needed to support Django 1.1 |
519 | 103 | submit_review_resource = CSRFExemptResource( | 102 | submit_review_resource = CSRFExemptResource( |
521 | 104 | handler=SubmitReviewHandler, authentication=(macaroons_auth, auth)) | 103 | handler=SubmitReviewHandler, authentication=auth) |
522 | 105 | modify_review_resource = CSRFExemptResource(handler=ModifyReviewHandler, | 104 | modify_review_resource = CSRFExemptResource(handler=ModifyReviewHandler, |
523 | 106 | authentication=auth) | 105 | authentication=auth) |
524 | 107 | flag_review_resource = CSRFExemptResource(handler=FlagReviewHandler, | 106 | flag_review_resource = CSRFExemptResource(handler=FlagReviewHandler, |
525 | 108 | 107 | ||
526 | === modified file 'src/reviewsapp/tests/test_handlers.py' | |||
527 | --- src/reviewsapp/tests/test_handlers.py 2016-05-16 14:32:26 +0000 | |||
528 | +++ src/reviewsapp/tests/test_handlers.py 2016-05-20 13:59:57 +0000 | |||
529 | @@ -56,11 +56,7 @@ | |||
530 | 56 | from piston.emitters import Emitter | 56 | from piston.emitters import Emitter |
531 | 57 | from piston.handler import typemapper | 57 | from piston.handler import typemapper |
532 | 58 | 58 | ||
538 | 59 | from core.tests.doubles import SCADouble | 59 | from core.tests.helpers import assert_no_extra_queries_after |
534 | 60 | from core.tests.helpers import ( | ||
535 | 61 | RequestsDoubleTestCaseMixin, | ||
536 | 62 | assert_no_extra_queries_after, | ||
537 | 63 | ) | ||
539 | 64 | from reviewsapp.api.handlers import ( | 60 | from reviewsapp.api.handlers import ( |
540 | 65 | Django13JSONEncoder, | 61 | Django13JSONEncoder, |
541 | 66 | Django13JSONEmitter, | 62 | Django13JSONEmitter, |
542 | @@ -987,8 +983,7 @@ | |||
543 | 987 | self.assertEqual(response['vary'], 'Accept') | 983 | self.assertEqual(response['vary'], 'Accept') |
544 | 988 | 984 | ||
545 | 989 | 985 | ||
548 | 990 | class SubmitReviewHandlerTestCase(TestCaseWithFactory, | 986 | class SubmitReviewHandlerTestCase(TestCaseWithFactory): |
547 | 991 | RequestsDoubleTestCaseMixin): | ||
549 | 992 | 987 | ||
550 | 993 | def setUp(self): | 988 | def setUp(self): |
551 | 994 | super(SubmitReviewHandlerTestCase, self).setUp() | 989 | super(SubmitReviewHandlerTestCase, self).setUp() |
552 | @@ -1005,26 +1000,13 @@ | |||
553 | 1005 | } | 1000 | } |
554 | 1006 | self.factory.makeArch('i386') | 1001 | self.factory.makeArch('i386') |
555 | 1007 | 1002 | ||
564 | 1008 | self.patch_requests() | 1003 | def _post_new_review(self, data, user=None, verify_result=True): |
557 | 1009 | self.sca_double = SCADouble(self.requests_double) | ||
558 | 1010 | p = patch('core.utilities.web_services', new=self.sca_double) | ||
559 | 1011 | p.start() | ||
560 | 1012 | self.addCleanup(p.stop) | ||
561 | 1013 | |||
562 | 1014 | def _post_new_review(self, data, user=None, verify_result=True, | ||
563 | 1015 | auth='oauth'): | ||
565 | 1016 | # Post a review as an authenticated user. | 1004 | # Post a review as an authenticated user. |
566 | 1017 | url = reverse('rnr-api-reviews') | 1005 | url = reverse('rnr-api-reviews') |
567 | 1018 | 1006 | ||
577 | 1019 | headers = {} | 1007 | if user is None: |
578 | 1020 | if auth == 'macaroon': | 1008 | user = self.factory.makeUser() |
579 | 1021 | authorization = 'Macaroon root="%s", discharge="%s"' % ( | 1009 | self.client.login(username=user.username, password='test') |
571 | 1022 | 'root-macaroon-data', 'discharge-macaroon-data') | ||
572 | 1023 | headers['HTTP_AUTHORIZATION'] = authorization | ||
573 | 1024 | else: | ||
574 | 1025 | if user is None: | ||
575 | 1026 | user = self.factory.makeUser() | ||
576 | 1027 | self.client.login(username=user.username, password='test') | ||
580 | 1028 | 1010 | ||
581 | 1029 | # Ensure we don't hit the network for our tests. | 1011 | # Ensure we don't hit the network for our tests. |
582 | 1030 | is_authed_fn = ('core.api.auth.SSOOAuthAuthentication.' | 1012 | is_authed_fn = ('core.api.auth.SSOOAuthAuthentication.' |
583 | @@ -1040,8 +1022,7 @@ | |||
584 | 1040 | with patch(sca_verify_fn) as mock_sca_verify_method: | 1022 | with patch(sca_verify_fn) as mock_sca_verify_method: |
585 | 1041 | mock_sca_verify_method.return_value = verify_result | 1023 | mock_sca_verify_method.return_value = verify_result |
586 | 1042 | response = self.client.post( | 1024 | response = self.client.post( |
589 | 1043 | url, data=data, content_type='application/json', | 1025 | url, data=data, content_type='application/json') |
588 | 1044 | **headers) | ||
590 | 1045 | return response | 1026 | return response |
591 | 1046 | 1027 | ||
592 | 1047 | def test_bogus_data(self): | 1028 | def test_bogus_data(self): |
593 | @@ -1077,18 +1058,6 @@ | |||
594 | 1077 | response_dict = simplejson.loads(response.content) | 1058 | response_dict = simplejson.loads(response.content) |
595 | 1078 | self.assertEqual('inkscape', response_dict['package_name']) | 1059 | self.assertEqual('inkscape', response_dict['package_name']) |
596 | 1079 | 1060 | ||
597 | 1080 | def test_create_using_macaroon_auth(self): | ||
598 | 1081 | self.sca_double.set_verify_acl_response( | ||
599 | 1082 | is_verified=True, openid='oid1234', | ||
600 | 1083 | email='user@example.com', displayname='user1234') | ||
601 | 1084 | response = self._post_new_review(simplejson.dumps(self.required_data), | ||
602 | 1085 | auth='macaroon') | ||
603 | 1086 | self.assertEqual(httplib.OK, response.status_code) | ||
604 | 1087 | self.assertEqual('application/json; charset=utf-8', | ||
605 | 1088 | response['content-type']) | ||
606 | 1089 | response_dict = simplejson.loads(response.content) | ||
607 | 1090 | self.assertEqual('inkscape', response_dict['package_name']) | ||
608 | 1091 | |||
609 | 1092 | def test_creates_repository_and_software_item(self): | 1061 | def test_creates_repository_and_software_item(self): |
610 | 1093 | # Submitting a review for a software item or repository of which | 1062 | # Submitting a review for a software item or repository of which |
611 | 1094 | # we don't yet have a record will create those records (after | 1063 | # we don't yet have a record will create those records (after |
Looking good, added few comments.