Merge lp:~ricardokirkner/rnr-server/review-crud-api-with-macaroons into lp:rnr-server

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
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

To post a comment you must log in.
Revision history for this message
Fabián Ezequiel Gallina (fgallina) wrote :

Looking good, added few comments.

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

Revision history for this message
Fabián Ezequiel Gallina (fgallina) wrote :

LGTM

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/clickreviews/api/urls.py'
--- src/clickreviews/api/urls.py 2014-11-21 20:27:01 +0000
+++ src/clickreviews/api/urls.py 2016-05-20 13:59:57 +0000
@@ -9,6 +9,7 @@
9from piston.resource import Resource9from piston.resource import Resource
10from core.api.auth import (10from core.api.auth import (
11 DelegatedSSOAuthentication,11 DelegatedSSOAuthentication,
12 MacaroonsAuthentication,
12 SSOOAuthAuthentication,13 SSOOAuthAuthentication,
13)14)
14from core.api.decorators import gzip_content, vary_only_on_headers15from core.api.decorators import gzip_content, vary_only_on_headers
@@ -24,9 +25,10 @@
2425
2526
26auth = SSOOAuthAuthentication(realm="Ratings and Reviews")27auth = SSOOAuthAuthentication(realm="Ratings and Reviews")
28macaroons_auth = MacaroonsAuthentication(realm="Ratings and Reviews")
2729
28review_resource = CSRFExemptResource(handler=ClickReviewHandler,30review_resource = CSRFExemptResource(handler=ClickReviewHandler,
29 authentication=auth)31 authentication=(macaroons_auth, auth))
30review_resource.callmap['PATCH'] = 'patch'32review_resource.callmap['PATCH'] = 'patch'
31review_resource = never_cache(review_resource)33review_resource = never_cache(review_resource)
3234
3335
=== modified file 'src/clickreviews/tests/test_handlers.py'
--- src/clickreviews/tests/test_handlers.py 2015-04-22 14:04:11 +0000
+++ src/clickreviews/tests/test_handlers.py 2016-05-20 13:59:57 +0000
@@ -12,6 +12,7 @@
12from django.core.cache import cache12from django.core.cache import cache
13from django.core.serializers.json import DateTimeAwareJSONEncoder13from django.core.serializers.json import DateTimeAwareJSONEncoder
14from django.core.urlresolvers import reverse14from django.core.urlresolvers import reverse
15from mock import patch
1516
16from clickreviews.api.urls import sca_delegated_sso_auth17from clickreviews.api.urls import sca_delegated_sso_auth
17from clickreviews.models import (18from clickreviews.models import (
@@ -23,10 +24,28 @@
23)24)
24from clickreviews.tests.factory import TestCaseWithFactory25from clickreviews.tests.factory import TestCaseWithFactory
25from core.models import BaseModeration26from core.models import BaseModeration
26from core.tests.helpers import SSOTestCaseMixin, assert_no_extra_queries_after27from core.tests.doubles import SCADouble
2728from core.tests.helpers import (
2829 RequestsDoubleTestCaseMixin,
29class ClickReviewsHandlerTestCase(TestCaseWithFactory):30 SSOTestCaseMixin,
31 assert_no_extra_queries_after,
32)
33
34
35class MacaroonsAuthenticatedTestCase(TestCaseWithFactory,
36 RequestsDoubleTestCaseMixin):
37
38 def setUp(self):
39 super(MacaroonsAuthenticatedTestCase, self).setUp()
40
41 self.patch_requests()
42 self.sca_double = SCADouble(self.requests_double)
43 p = patch('core.utilities.web_services', new=self.sca_double)
44 p.start()
45 self.addCleanup(p.stop)
46
47
48class ClickReviewsHandlerTestCase(MacaroonsAuthenticatedTestCase):
3049
31 def setUp(self):50 def setUp(self):
32 super(ClickReviewsHandlerTestCase, self).setUp()51 super(ClickReviewsHandlerTestCase, self).setUp()
@@ -58,36 +77,38 @@
58 'arch_tag': 'i386',77 'arch_tag': 'i386',
59 }78 }
6079
61 def _post_new_review(self, data, user=None, verify_result=True):80 def post_new_review(self, data, user=None, verify_result=True,
81 auth='oauth'):
62 # Post a review as an authenticated user.82 # Post a review as an authenticated user.
63 url = reverse('click-api-reviews')83 url = reverse('click-api-reviews')
64 if user is None:84 headers = {}
65 user = self.factory.makeUser()85 if auth == 'macaroon':
66 self.client.login(username=user.username, password='test')86 authorization = 'Macaroon root="%s", discharge="%s"' % (
87 'root-macaroon-data', 'discharge-macaroon-data')
88 headers['HTTP_AUTHORIZATION'] = authorization
89 else:
90 if user is None:
91 user = self.factory.makeUser()
92 self.client.login(username=user.username, password='test')
6793
68 response = self.client.post(94 response = self.client.post(
69 url, data=data, content_type='application/json')95 url, data=data, content_type='application/json', **headers)
70 return response96 return response
7197
72 def test_create(self):98 def assert_review_created(self, response, expected):
73 data = self.make_data(package_name='com.example.me.myapp',
74 rating=5)
75
76 response = self._post_new_review(json.dumps(data))
77
78 # Piston uses 200, rather than 201.99 # Piston uses 200, rather than 201.
79 self.assertEqual(httplib.OK, response.status_code)100 self.assertEqual(httplib.OK, response.status_code)
80 self.assertEqual('application/json; charset=utf-8',101 self.assertEqual('application/json; charset=utf-8',
81 response['content-type'])102 response['content-type'])
82 reviews = ClickPackageReview.objects.filter(103 reviews = ClickPackageReview.objects.filter(
83 click_package__package_name='com.example.me.myapp')104 click_package__package_name=expected['package_name'])
84 self.assertEqual(1, reviews.count())105 self.assertEqual(1, reviews.count())
85 review = reviews[0]106 review = reviews[0]
86 response_dict = json.loads(response.content)107 response_dict = json.loads(response.content)
87 self.assertEqual({108 self.assertEqual({
88 'id': review.id,109 'id': review.id,
89 'package_name': 'com.example.me.myapp',110 'package_name': expected['package_name'],
90 'language': data['language'],111 'language': expected['language'],
91 'date_created': DateTimeAwareJSONEncoder().encode(112 'date_created': DateTimeAwareJSONEncoder().encode(
92 review.date_created).strip('"'),113 review.date_created).strip('"'),
93 'rating': 5,114 'rating': 5,
@@ -102,10 +123,27 @@
102 'usefulness_favorable': 0,123 'usefulness_favorable': 0,
103 }, response_dict)124 }, response_dict)
104125
126 def test_create(self):
127 data = self.make_data(package_name='com.example.me.myapp',
128 rating=5)
129
130 response = self.post_new_review(json.dumps(data))
131 self.assert_review_created(response, data)
132
133 def test_create_using_macaroon_auth(self):
134 user = self.factory.makeUser()
135 account = self.make_account_data(user)
136 self.sca_double.set_verify_acl_response(account=account)
137 data = self.make_data(package_name='com.example.me.myapp',
138 rating=5)
139 response = self.post_new_review(
140 json.dumps(data), user=user, auth='macaroon')
141 self.assert_review_created(response, data)
142
105 def test_invalid_rating(self):143 def test_invalid_rating(self):
106 data = self.make_data(rating=6)144 data = self.make_data(rating=6)
107145
108 response = self._post_new_review(json.dumps(data))146 response = self.post_new_review(json.dumps(data))
109147
110 self.assertEqual(httplib.BAD_REQUEST, response.status_code)148 self.assertEqual(httplib.BAD_REQUEST, response.status_code)
111149
@@ -116,7 +154,7 @@
116 data = valid_data.copy()154 data = valid_data.copy()
117 del data[key]155 del data[key]
118156
119 response = self._post_new_review(json.dumps(data))157 response = self.post_new_review(json.dumps(data))
120158
121 self.assertEqual(httplib.BAD_REQUEST, response.status_code)159 self.assertEqual(httplib.BAD_REQUEST, response.status_code)
122160
@@ -127,13 +165,13 @@
127 click_package = ClickPackage.objects.create(165 click_package = ClickPackage.objects.create(
128 package_name='com.example.me.myapp')166 package_name='com.example.me.myapp')
129167
130 self._post_new_review(json.dumps(data))168 self.post_new_review(json.dumps(data))
131169
132 click_package = ClickPackage.objects.get(id=click_package.id)170 click_package = ClickPackage.objects.get(id=click_package.id)
133 self.assertEqual(1, click_package.ratings_total)171 self.assertEqual(1, click_package.ratings_total)
134 self.assertEqual(5, click_package.ratings_average)172 self.assertEqual(5, click_package.ratings_average)
135173
136 def _get_reviews(self, data=None):174 def get_reviews(self, data=None):
137 url = reverse('click-api-reviews')175 url = reverse('click-api-reviews')
138 kwargs = {}176 kwargs = {}
139 if data is not None:177 if data is not None:
@@ -146,7 +184,7 @@
146 for _ in range(5):184 for _ in range(5):
147 self.factory.makeClickPackageReview(click_package=package)185 self.factory.makeClickPackageReview(click_package=package)
148186
149 json_result = self._get_reviews(data={187 json_result = self.get_reviews(data={
150 'package_name': package.package_name,188 'package_name': package.package_name,
151 })189 })
152190
@@ -163,7 +201,7 @@
163 version='0.1.1', date_created=datetime(201 version='0.1.1', date_created=datetime(
164 2014, 1, 15, 11, 13, 55, 123456, tzinfo=pytz.UTC))202 2014, 1, 15, 11, 13, 55, 123456, tzinfo=pytz.UTC))
165203
166 json_result = self._get_reviews(data={204 json_result = self.get_reviews(data={
167 'package_name': package.package_name,205 'package_name': package.package_name,
168 })206 })
169 self.assertEqual(200, json_result.status_code)207 self.assertEqual(200, json_result.status_code)
@@ -190,7 +228,7 @@
190 }, result_review)228 }, result_review)
191229
192 def test_must_filter_by_packagename(self):230 def test_must_filter_by_packagename(self):
193 result = self._get_reviews()231 result = self.get_reviews()
194232
195 self.assertEqual(400, result.status_code)233 self.assertEqual(400, result.status_code)
196 self.assertEqual(['package_name'],234 self.assertEqual(['package_name'],
@@ -202,7 +240,7 @@
202 click_package=package,240 click_package=package,
203 deleted=True)241 deleted=True)
204242
205 json_result = self._get_reviews(data={243 json_result = self.get_reviews(data={
206 'package_name': package.package_name,244 'package_name': package.package_name,
207 })245 })
208 self.assertEqual(200, json_result.status_code)246 self.assertEqual(200, json_result.status_code)
@@ -213,7 +251,7 @@
213 package = self.factory.makeClickPackage()251 package = self.factory.makeClickPackage()
214 self.factory.makeClickPackageReview(click_package=package, hide=True)252 self.factory.makeClickPackageReview(click_package=package, hide=True)
215253
216 json_result = self._get_reviews(data={254 json_result = self.get_reviews(data={
217 'package_name': package.package_name,255 'package_name': package.package_name,
218 })256 })
219 self.assertEqual(200, json_result.status_code)257 self.assertEqual(200, json_result.status_code)
@@ -240,7 +278,7 @@
240 reviews[2].id,278 reviews[2].id,
241 ]279 ]
242280
243 json_result = self._get_reviews(data={281 json_result = self.get_reviews(data={
244 'package_name': package.package_name,282 'package_name': package.package_name,
245 })283 })
246 self.assertEqual(200, json_result.status_code)284 self.assertEqual(200, json_result.status_code)
@@ -255,33 +293,63 @@
255 self.factory.makeClickPackageReview(click_package=package)293 self.factory.makeClickPackageReview(click_package=package)
256 assert_no_extra_queries_after(294 assert_no_extra_queries_after(
257 lambda: self.factory.makeClickPackageReview(click_package=package),295 lambda: self.factory.makeClickPackageReview(click_package=package),
258 self._get_reviews, data={'package_name': package.package_name}296 self.get_reviews, data={'package_name': package.package_name}
259 )297 )
260298
261 def _update_review(self, review, data, user=None):299 def update_review(self, review, data, user=None, auth='oauth'):
262 # Post a review as an authenticated user.300 # Post a review as an authenticated user.
263 url = reverse('click-api-reviews', args=(review.pk,))301 url = reverse('click-api-reviews', args=(review.pk,))
264 if user is None:302 headers = {}
265 user = review.reviewer303 if auth == 'macaroon':
266 self.client.login(username=user.username, password='test')304 authorization = 'Macaroon root="%s", discharge="%s"' % (
305 'root-macaroon-data', 'discharge-macaroon-data')
306 headers['HTTP_AUTHORIZATION'] = authorization
307 else:
308 if user is None:
309 user = review.reviewer
310 self.client.login(username=user.username, password='test')
267 response = self.client.put(311 response = self.client.put(
268 url, data=json.dumps(data), content_type='application/json')312 url, data=json.dumps(data), content_type='application/json',
313 **headers)
269 return response314 return response
270315
316 def assert_review_updated(self, response, expected):
317 self.assertEqual(response.status_code, httplib.OK)
318 review = json.loads(response.content)
319 for key, value in expected.items():
320 self.assertEqual(review[key], value)
321
271 def test_update_review(self):322 def test_update_review(self):
272 review = self.factory.makeClickPackageReview()323 review = self.factory.makeClickPackageReview()
273 data = {'summary': 'New summary',324 data = {'summary': 'New summary',
274 'review_text': 'Review text',325 'review_text': 'Review text',
275 'rating': 5}326 'rating': 5}
276 response = self._update_review(review, data)327 response = self.update_review(review, data)
277 self.assertEqual(response.status_code, httplib.OK)328 self.assert_review_updated(response, data)
278 review = json.loads(response.content)329
279 for key, value in data.items():330 def make_account_data(self, user):
280 self.assertEqual(review[key], data[key])331 return {
332 'verified': True,
333 'openid': user.useropenid_set.first().claimed_id.split('/')[-1],
334 'email': user.email,
335 'displayname': user.username,
336 }
337
338 def test_update_review_using_macaroon_auth(self):
339 user = self.factory.makeUser()
340 account = self.make_account_data(user)
341 self.sca_double.set_verify_acl_response(account=account)
342 review = self.factory.makeClickPackageReview(reviewer=user)
343 data = {'summary': 'New summary',
344 'review_text': 'Review text',
345 'rating': 5}
346 response = self.update_review(
347 review, data, user=user, auth='macaroon')
348 self.assert_review_updated(response, data)
281349
282 def test_update_review_missing_data(self):350 def test_update_review_missing_data(self):
283 review = self.factory.makeClickPackageReview()351 review = self.factory.makeClickPackageReview()
284 response = self._update_review(review, {})352 response = self.update_review(review, {})
285 self.assertEqual(response.status_code, httplib.BAD_REQUEST)353 self.assertEqual(response.status_code, httplib.BAD_REQUEST)
286 errors = json.loads(response.content)354 errors = json.loads(response.content)
287 expected = {'errors':355 expected = {'errors':
@@ -293,45 +361,72 @@
293 def test_update_review_wrong_user(self):361 def test_update_review_wrong_user(self):
294 review = self.factory.makeClickPackageReview()362 review = self.factory.makeClickPackageReview()
295 user = self.factory.makeUser()363 user = self.factory.makeUser()
296 response = self._update_review(review, {}, user)364 response = self.update_review(review, {}, user)
297 self.assertEqual(response.status_code, httplib.NOT_FOUND)365 self.assertEqual(response.status_code, httplib.NOT_FOUND)
298366
299 def _delete_review(self, review, user=None):367 def delete_review(self, review, user=None, auth='oauth'):
300 # Delete a review as an authenticated user.368 # Delete a review as an authenticated user.
301 url = reverse('click-api-reviews', args=(review.pk,))369 url = reverse('click-api-reviews', args=(review.pk,))
302 if user is None:370 headers = {}
303 user = review.reviewer371 if auth == 'macaroon':
304 self.client.login(username=user.username, password='test')372 authorization = 'Macaroon root="%s", discharge="%s"' % (
305 response = self.client.delete(url)373 'root-macaroon-data', 'discharge-macaroon-data')
374 headers['HTTP_AUTHORIZATION'] = authorization
375 else:
376 if user is None:
377 user = review.reviewer
378 self.client.login(username=user.username, password='test')
379 response = self.client.delete(url, **headers)
306 return response380 return response
307381
382 def assert_review_deleted(self, response):
383 self.assertEqual(response.status_code, httplib.OK)
384 review = json.loads(response.content)
385 db_review = ClickPackageReview.objects.get(id=review['id'])
386 self.assertTrue(db_review.hide)
387 self.assertIsNotNone(db_review.date_deleted)
388 # XXX: ignore time differences at the microsecond level
389 date_deleted = db_review.date_deleted
390 parsed = datetime.strptime(
391 review['date_deleted'], '%Y-%m-%dT%H:%M:%S.%fZ')
392 self.assertEqual(
393 date_deleted.replace(microsecond=0),
394 parsed.replace(microsecond=0, tzinfo=date_deleted.tzinfo))
395
308 def test_delete_review(self):396 def test_delete_review(self):
309 review = self.factory.makeClickPackageReview()397 review = self.factory.makeClickPackageReview()
310 response = self._delete_review(review)398 response = self.delete_review(review)
311 self.assertEqual(response.status_code, httplib.OK)399 self.assert_review_deleted(response)
312 review = json.loads(response.content)400
401 def test_delete_review_using_macaroon_auth(self):
402 user = self.factory.makeUser()
403 account = self.make_account_data(user)
404 self.sca_double.set_verify_acl_response(account=account)
405 review = self.factory.makeClickPackageReview(reviewer=user)
406 response = self.delete_review(review, auth='macaroon')
407 self.assert_review_deleted(response)
313408
314 def test_delete_review_wrong_id(self):409 def test_delete_review_wrong_id(self):
315 review = self.factory.makeClickPackageReview()410 review = self.factory.makeClickPackageReview()
316 ClickPackageReview.objects.all().delete()411 ClickPackageReview.objects.all().delete()
317 response = self._delete_review(review)412 response = self.delete_review(review)
318 self.assertEqual(response.status_code, httplib.NOT_FOUND)413 self.assertEqual(response.status_code, httplib.NOT_FOUND)
319414
320 def test_delete_already_deleted(self):415 def test_delete_already_deleted(self):
321 review = self.factory.makeClickPackageReview()416 review = self.factory.makeClickPackageReview()
322 review.delete()417 review.delete()
323 response = self._delete_review(review)418 response = self.delete_review(review)
324 self.assertEqual(response.status_code, httplib.NOT_FOUND)419 self.assertEqual(response.status_code, httplib.NOT_FOUND)
325420
326 def test_delete_hidden_review(self):421 def test_delete_hidden_review(self):
327 review = self.factory.makeClickPackageReview(hide=True)422 review = self.factory.makeClickPackageReview(hide=True)
328 response = self._delete_review(review)423 response = self.delete_review(review)
329 self.assertEqual(response.status_code, httplib.NOT_FOUND)424 self.assertEqual(response.status_code, httplib.NOT_FOUND)
330425
331 def test_delete_review_wrong_user(self):426 def test_delete_review_wrong_user(self):
332 review = self.factory.makeClickPackageReview()427 review = self.factory.makeClickPackageReview()
333 user = self.factory.makeUser()428 user = self.factory.makeUser()
334 response = self._delete_review(review, user=user)429 response = self.delete_review(review, user=user)
335 self.assertEqual(response.status_code, httplib.NOT_FOUND)430 self.assertEqual(response.status_code, httplib.NOT_FOUND)
336431
337432
@@ -368,7 +463,7 @@
368 cache._cache.clear()463 cache._cache.clear()
369 cache._expire_info.clear()464 cache._expire_info.clear()
370465
371 def _request_stats(self, data=None, headers=None, **kwargs):466 def request_stats(self, data=None, headers=None, **kwargs):
372 url = reverse(467 url = reverse(
373 'click-api-reviews-stats', kwargs=kwargs)468 'click-api-reviews-stats', kwargs=kwargs)
374 if data is None:469 if data is None:
@@ -385,7 +480,7 @@
385 click_package=self.factory.makeClickPackage('package_bar'),480 click_package=self.factory.makeClickPackage('package_bar'),
386 language='de')481 language='de')
387482
388 response = self._request_stats()483 response = self.request_stats()
389 reviews = json.loads(response.content)484 reviews = json.loads(response.content)
390485
391 foo = foo_review.click_package486 foo = foo_review.click_package
@@ -405,7 +500,7 @@
405 click_package=self.factory.makeClickPackage('package_bar'),500 click_package=self.factory.makeClickPackage('package_bar'),
406 language='de')501 language='de')
407502
408 response = self._request_stats(package_name='package_foo')503 response = self.request_stats(package_name='package_foo')
409 reviews = json.loads(response.content)504 reviews = json.loads(response.content)
410505
411 foo = foo_review.click_package506 foo = foo_review.click_package
@@ -413,12 +508,12 @@
413 pk=foo.pk, package_name=foo.package_name))508 pk=foo.pk, package_name=foo.package_name))
414509
415 def test_read_for_missing_package(self):510 def test_read_for_missing_package(self):
416 response = self._request_stats(package_name='not_found')511 response = self.request_stats(package_name='not_found')
417 self.assertEqual(response.status_code, 404)512 self.assertEqual(response.status_code, 404)
418513
419 def test_request_cached_with_extra_headers(self):514 def test_request_cached_with_extra_headers(self):
420 # Requests for server status are never cached.515 # Requests for server status are never cached.
421 response = self._request_stats()516 response = self.request_stats()
422517
423 self.assertEqual(518 self.assertEqual(
424 'max-age={0}'.format(settings.CACHE_REVIEW_STATS_SECONDS),519 'max-age={0}'.format(settings.CACHE_REVIEW_STATS_SECONDS),
@@ -427,13 +522,13 @@
427 # A subsequent call will not hit the view.522 # A subsequent call will not hit the view.
428 handler = 'clickreviews.api.handlers.ClickReviewsStatsHandler.read'523 handler = 'clickreviews.api.handlers.ClickReviewsStatsHandler.read'
429 with mock.patch(handler) as mock_read:524 with mock.patch(handler) as mock_read:
430 response = self._request_stats()525 response = self.request_stats()
431 self.assertFalse(mock_read.called)526 self.assertFalse(mock_read.called)
432527
433 def test_cannot_request_with_get_params(self):528 def test_cannot_request_with_get_params(self):
434 # Requesting the un-cached response (by adding get params) is529 # Requesting the un-cached response (by adding get params) is
435 # not allowed.530 # not allowed.
436 response = self._request_stats(data=dict(foo='bar'))531 response = self.request_stats(data=dict(foo='bar'))
437532
438 self.assertEqual(400, response.status_code)533 self.assertEqual(400, response.status_code)
439534
@@ -447,7 +542,7 @@
447 click_package=self.factory.makeClickPackage('package_bar'),542 click_package=self.factory.makeClickPackage('package_bar'),
448 language='de')543 language='de')
449544
450 response = self._request_stats(545 response = self.request_stats(
451 headers={'HTTP_ACCEPT_ENCODING': 'gzip, deflate'})546 headers={'HTTP_ACCEPT_ENCODING': 'gzip, deflate'})
452547
453 fileobj = BytesIO()548 fileobj = BytesIO()
@@ -468,7 +563,7 @@
468 pk=bar.pk, package_name=bar.package_name)])563 pk=bar.pk, package_name=bar.package_name)])
469564
470 def test_vary_headers(self):565 def test_vary_headers(self):
471 response = self._request_stats()566 response = self.request_stats()
472 self.assertEqual(200, response.status_code)567 self.assertEqual(200, response.status_code)
473 self.assertTrue(response.has_header('vary'))568 self.assertTrue(response.has_header('vary'))
474 vary = response['vary']569 vary = response['vary']
@@ -692,7 +787,7 @@
692 self.addCleanup(p.stop)787 self.addCleanup(p.stop)
693 self.patch_sso()788 self.patch_sso()
694789
695 def _make_delegated_sso_signed_headers(self, url, data=None):790 def make_delegated_sso_signed_headers(self, url, data=None):
696 if data is None:791 if data is None:
697 data = ''792 data = ''
698 url = 'http://testserver' + url793 url = 'http://testserver' + url
@@ -705,7 +800,7 @@
705800
706 def request_get(self, url, authenticated=True, **headers):801 def request_get(self, url, authenticated=True, **headers):
707 if authenticated:802 if authenticated:
708 headers.update(self._make_delegated_sso_signed_headers(url))803 headers.update(self.make_delegated_sso_signed_headers(url))
709 return self.client.get(url, **headers)804 return self.client.get(url, **headers)
710805
711 def request_patch(806 def request_patch(
@@ -714,7 +809,7 @@
714 if data is None:809 if data is None:
715 data = ''810 data = ''
716 if authenticated:811 if authenticated:
717 headers.update(self._make_delegated_sso_signed_headers(url, data))812 headers.update(self.make_delegated_sso_signed_headers(url, data))
718 return self.client.patch(813 return self.client.patch(
719 url, data=data, content_type=content_type, **headers)814 url, data=data, content_type=content_type, **headers)
720815
721816
=== modified file 'src/reviewsapp/api/urls.py'
--- src/reviewsapp/api/urls.py 2016-05-16 14:32:26 +0000
+++ src/reviewsapp/api/urls.py 2016-05-20 13:59:57 +0000
@@ -22,7 +22,7 @@
22)22)
2323
24from piston.resource import Resource24from piston.resource import Resource
25from core.api.auth import MacaroonsAuthentication, SSOOAuthAuthentication25from core.api.auth import SSOOAuthAuthentication
26from core.api.decorators import (26from core.api.decorators import (
27 gzip_content,27 gzip_content,
28 vary_only_on_accept,28 vary_only_on_accept,
@@ -44,7 +44,6 @@
44)44)
4545
46auth = SSOOAuthAuthentication(realm="Ratings and Reviews")46auth = SSOOAuthAuthentication(realm="Ratings and Reviews")
47macaroons_auth = MacaroonsAuthentication(realm="Ratings and Reviews")
4847
4948
50# The server status resource should never be cached.49# The server status resource should never be cached.
@@ -101,7 +100,7 @@
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.
102# The .__name__ hack is needed to support Django 1.1101# The .__name__ hack is needed to support Django 1.1
103submit_review_resource = CSRFExemptResource(102submit_review_resource = CSRFExemptResource(
104 handler=SubmitReviewHandler, authentication=(macaroons_auth, auth))103 handler=SubmitReviewHandler, authentication=auth)
105modify_review_resource = CSRFExemptResource(handler=ModifyReviewHandler,104modify_review_resource = CSRFExemptResource(handler=ModifyReviewHandler,
106 authentication=auth)105 authentication=auth)
107flag_review_resource = CSRFExemptResource(handler=FlagReviewHandler,106flag_review_resource = CSRFExemptResource(handler=FlagReviewHandler,
108107
=== modified file 'src/reviewsapp/tests/test_handlers.py'
--- src/reviewsapp/tests/test_handlers.py 2016-05-16 14:32:26 +0000
+++ src/reviewsapp/tests/test_handlers.py 2016-05-20 13:59:57 +0000
@@ -56,11 +56,7 @@
56from piston.emitters import Emitter56from piston.emitters import Emitter
57from piston.handler import typemapper57from piston.handler import typemapper
5858
59from core.tests.doubles import SCADouble59from core.tests.helpers import assert_no_extra_queries_after
60from core.tests.helpers import (
61 RequestsDoubleTestCaseMixin,
62 assert_no_extra_queries_after,
63)
64from reviewsapp.api.handlers import (60from reviewsapp.api.handlers import (
65 Django13JSONEncoder,61 Django13JSONEncoder,
66 Django13JSONEmitter,62 Django13JSONEmitter,
@@ -987,8 +983,7 @@
987 self.assertEqual(response['vary'], 'Accept')983 self.assertEqual(response['vary'], 'Accept')
988984
989985
990class SubmitReviewHandlerTestCase(TestCaseWithFactory,986class SubmitReviewHandlerTestCase(TestCaseWithFactory):
991 RequestsDoubleTestCaseMixin):
992987
993 def setUp(self):988 def setUp(self):
994 super(SubmitReviewHandlerTestCase, self).setUp()989 super(SubmitReviewHandlerTestCase, self).setUp()
@@ -1005,26 +1000,13 @@
1005 }1000 }
1006 self.factory.makeArch('i386')1001 self.factory.makeArch('i386')
10071002
1008 self.patch_requests()1003 def _post_new_review(self, data, user=None, verify_result=True):
1009 self.sca_double = SCADouble(self.requests_double)
1010 p = patch('core.utilities.web_services', new=self.sca_double)
1011 p.start()
1012 self.addCleanup(p.stop)
1013
1014 def _post_new_review(self, data, user=None, verify_result=True,
1015 auth='oauth'):
1016 # Post a review as an authenticated user.1004 # Post a review as an authenticated user.
1017 url = reverse('rnr-api-reviews')1005 url = reverse('rnr-api-reviews')
10181006
1019 headers = {}1007 if user is None:
1020 if auth == 'macaroon':1008 user = self.factory.makeUser()
1021 authorization = 'Macaroon root="%s", discharge="%s"' % (1009 self.client.login(username=user.username, password='test')
1022 'root-macaroon-data', 'discharge-macaroon-data')
1023 headers['HTTP_AUTHORIZATION'] = authorization
1024 else:
1025 if user is None:
1026 user = self.factory.makeUser()
1027 self.client.login(username=user.username, password='test')
10281010
1029 # Ensure we don't hit the network for our tests.1011 # Ensure we don't hit the network for our tests.
1030 is_authed_fn = ('core.api.auth.SSOOAuthAuthentication.'1012 is_authed_fn = ('core.api.auth.SSOOAuthAuthentication.'
@@ -1040,8 +1022,7 @@
1040 with patch(sca_verify_fn) as mock_sca_verify_method:1022 with patch(sca_verify_fn) as mock_sca_verify_method:
1041 mock_sca_verify_method.return_value = verify_result1023 mock_sca_verify_method.return_value = verify_result
1042 response = self.client.post(1024 response = self.client.post(
1043 url, data=data, content_type='application/json',1025 url, data=data, content_type='application/json')
1044 **headers)
1045 return response1026 return response
10461027
1047 def test_bogus_data(self):1028 def test_bogus_data(self):
@@ -1077,18 +1058,6 @@
1077 response_dict = simplejson.loads(response.content)1058 response_dict = simplejson.loads(response.content)
1078 self.assertEqual('inkscape', response_dict['package_name'])1059 self.assertEqual('inkscape', response_dict['package_name'])
10791060
1080 def test_create_using_macaroon_auth(self):
1081 self.sca_double.set_verify_acl_response(
1082 is_verified=True, openid='oid1234',
1083 email='user@example.com', displayname='user1234')
1084 response = self._post_new_review(simplejson.dumps(self.required_data),
1085 auth='macaroon')
1086 self.assertEqual(httplib.OK, response.status_code)
1087 self.assertEqual('application/json; charset=utf-8',
1088 response['content-type'])
1089 response_dict = simplejson.loads(response.content)
1090 self.assertEqual('inkscape', response_dict['package_name'])
1091
1092 def test_creates_repository_and_software_item(self):1061 def test_creates_repository_and_software_item(self):
1093 # Submitting a review for a software item or repository of which1062 # Submitting a review for a software item or repository of which
1094 # we don't yet have a record will create those records (after1063 # we don't yet have a record will create those records (after

Subscribers

People subscribed via source and target branches