Ratings and Reviews server

Merge lp:~elachuni/rnr-server/rnrclient-schemes into lp:~rnr-developers/rnr-server/rnrclient

  1. rnrclient-schemes
  2. Merge into rnrclient
Proposed by Anthony Lenton
Status: Merged
Merged at revision: 29
Proposed branch: lp:~elachuni/rnr-server/rnrclient-schemes
Merge into: lp:~rnr-developers/rnr-server/rnrclient
Diff against target: 183 lines (+57/-28)
2 files modified
rnrclient.py (+12/-6)
tests/test_rnrclient.py (+45/-22)
To merge this branch: bzr merge lp:~elachuni/rnr-server/rnrclient-schemes
Reviewer Review Type Date Requested Status
Danny Tamez (community) Approve
Review via email: mp+47078@code.launchpad.net

Description of the change

Overview
========
This branch uses piston_mini_client's new 'scheme' optional arguments to _get and _post to ensure that reviews are submitted via https, as requested in the related bug

Details
=======
At the same time, all public methods were forced to http requests, and all authenticated methods were forced to https, for the same reasons.

To post a comment you must log in.
Revision history for this message
Danny Tamez (zematynnad) wrote :

Approved. Everything looks good - tests run fine.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'rnrclient.py'
2--- rnrclient.py 2011-01-19 19:50:59 +0000
3+++ rnrclient.py 2011-01-21 19:13:15 +0000
4@@ -7,6 +7,10 @@
5 PistonSerializable, returns_json, returns_list_of)
6 from piston_mini_client.validators import validate_pattern, validate
7
8+# These are factored out as constants for if you need to work against a
9+# server that doesn't support both schemes (like http-only dev servers)
10+PUBLIC_API_SCHEME = 'http'
11+AUTHENTICATED_API_SCHEME = 'https'
12
13 class ReviewRequest(PistonSerializable):
14 """A review request.
15@@ -64,7 +68,7 @@
16 @returns_json
17 def server_status(self):
18 """Check the state of the server, to see if everything's ok."""
19- return self._get('server-status/')
20+ return self._get('server-status/', scheme=PUBLIC_API_SCHEME)
21
22 @validate('days', int, required=False)
23 @returns_list_of(ReviewsStats)
24@@ -79,7 +83,7 @@
25 if days <= valid_day:
26 url += 'updates-last-{0}-days/'.format(valid_day)
27 break
28- return self._get(url)
29+ return self._get(url, scheme=PUBLIC_API_SCHEME)
30
31 @validate_pattern('language', r'\w+', required=False)
32 @validate_pattern('origin', r'[0-9a-z+-.:/]+', required=False)
33@@ -99,14 +103,15 @@
34 if appname:
35 appname = quote_plus(';' + appname)
36 return self._get('%s/%s/%s/%s/%s%s/page/%s/' % (language, origin,
37- distroseries, version, packagename, appname, page))
38+ distroseries, version, packagename, appname, page),
39+ scheme=PUBLIC_API_SCHEME)
40
41 @validate('review', ReviewRequest)
42 @returns_json
43 def submit_review(self, review):
44 """Submit a rating/review."""
45 return self._post('reviews/', data=review,
46- content_type='application/json')
47+ scheme=AUTHENTICATED_API_SCHEME, content_type='application/json')
48
49 @validate('review_id', int)
50 @validate_pattern('reason', r'[^\n]+')
51@@ -117,7 +122,8 @@
52 data = {'reason': reason,
53 'text': text,
54 }
55- return self._post('reviews/%s/flags/' % review_id, data=data)
56+ return self._post('reviews/%s/flags/' % review_id, data=data,
57+ scheme=AUTHENTICATED_API_SCHEME)
58
59 @validate('review_id', int)
60 @validate_pattern('useful', 'True|False')
61@@ -125,5 +131,5 @@
62 def submit_usefulness(self, review_id, useful):
63 """Submit a usefulness vote."""
64 return self._post('/reviews/%s/recommendations/' % review_id,
65- data={'useful': useful})
66+ data={'useful': useful}, scheme=AUTHENTICATED_API_SCHEME)
67
68
69=== modified file 'tests/test_rnrclient.py'
70--- tests/test_rnrclient.py 2010-12-21 09:13:16 +0000
71+++ tests/test_rnrclient.py 2011-01-21 19:13:15 +0000
72@@ -2,59 +2,58 @@
73
74 import sys
75 import unittest
76+from urlparse import urlparse
77
78-from rnrclient import RatingsAndReviewsAPI
79 import piston_mini_client
80+from mock import patch
81+from piston_mini_client.failhandlers import NoneFailHandler
82+from rnrclient import RatingsAndReviewsAPI, ReviewRequest
83
84 class TestRnRClient(unittest.TestCase):
85
86 def setUp(self):
87- self.rnrclient = RatingsAndReviewsAPI()
88- self.rnrclient.default_service_root = "http://testme/review/api/1.0"
89+ self.rnrclient = RatingsAndReviewsAPI("http://testme/review/api/1.0")
90 self.rnrclient._get = self._monkey_patched_get
91
92- def _monkey_patched_get(self, url):
93+ def _monkey_patched_get(self, url, scheme=None):
94 self.hit_url = url
95 return ("200 ok", "[]")
96
97 def test_get_review(self):
98 # simple case, just pkgname
99+ self.rnrclient.get_reviews(packagename="foo")
100+ self.assertEqual(self.hit_url, "any/any/any/any/foo/page/1/")
101+ # pkgname with language, origin and distroseries
102 self.rnrclient.get_reviews(language="zh_CN", origin="ubuntu",
103 distroseries="lucid", packagename="foo")
104- self.assertEqual(self.hit_url, "zh_CN/ubuntu/lucid/foo/page/1/")
105+ self.assertEqual(self.hit_url, "zh_CN/ubuntu/lucid/any/foo/page/1/")
106+ # pkgname with version
107+ self.rnrclient.get_reviews(version="1.0", packagename="foo")
108+ self.assertEqual(self.hit_url, "any/any/any/1.0/foo/page/1/")
109 # pkgname with appname and space
110- self.rnrclient.get_reviews(language="en", origin="ubuntu",
111- distroseries="lucid", packagename="foo",
112+ self.rnrclient.get_reviews(language="en", packagename="foo",
113 appname="Foo Bar")
114- self.assertEqual(self.hit_url, "en/ubuntu/lucid/foo%3BFoo+Bar/page/1/")
115+ self.assertEqual(self.hit_url, "en/any/any/any/foo%3BFoo+Bar/page/1/")
116 # complicated appname
117- self.rnrclient.get_reviews(language="en", origin="ubuntu",
118- distroseries="lucid", packagename="foo",
119- appname="Foo!;-+*;)")
120+ self.rnrclient.get_reviews(packagename="foo", appname="Foo!;-+*;)")
121 self.assertEqual(self.hit_url,
122- "en/ubuntu/lucid/foo%3BFoo%21%3B-%2B%2A%3B%29/page/1/")
123+ "any/any/any/any/foo%3BFoo%21%3B-%2B%2A%3B%29/page/1/")
124 # invalid pkgname
125- kwargs = {"language" : "en",
126- "origin" : "ubuntu",
127- "distroseries" : "lucid",
128- "packagename" : "foo;invalid" }
129 self.assertRaises(piston_mini_client.validators.ValidationException,
130- self.rnrclient.get_reviews, kwargs)
131+ self.rnrclient.get_reviews,
132+ packagename="foo;invalid")
133 # invalid distroseries
134- kwargs = {"language" : "en",
135- "origin" : "ubuntu",
136- "distroseries" : "lucid;invalid-chars",
137+ kwargs = {"distroseries" : "lucid;invalid-chars",
138 "packagename" : "foo" }
139 self.assertRaises(piston_mini_client.validators.ValidationException,
140 self.rnrclient.get_reviews, kwargs)
141 # valid (but complicated) ppa origin
142 kwargs = {"language" : "en",
143 "origin" : "ppa:a.bono-wicked/ppa+fun-0.96,1",
144- "distroseries" : "lucid",
145 "packagename" : "foo" }
146 self.rnrclient.get_reviews(**kwargs)
147 self.assertEqual(self.hit_url,
148- "en/ppa:a.bono-wicked/ppa+fun-0.96,1/lucid/foo/page/1/")
149+ "en/ppa:a.bono-wicked/ppa+fun-0.96,1/any/any/foo/page/1/")
150
151 def test_get_review_stats(self):
152 self.rnrclient.review_stats()
153@@ -70,6 +69,30 @@
154 self.rnrclient.review_stats(days=2)
155 self.assertEqual(self.hit_url, "review-stats/updates-last-3-days/")
156
157+ @patch('httplib2.Http.request')
158+ def test_schemes(self, mock_request):
159+ mock_request.return_value = {}, '"uh?"'
160+ def called_scheme():
161+ return urlparse(mock_request.call_args[0][0]).scheme
162+ for service_root_scheme in ['http', 'https']:
163+ service_root = service_root_scheme + "://testme/review/api/1.0"
164+ rnrclient = RatingsAndReviewsAPI(service_root)
165+ rnrclient.fail_handler = NoneFailHandler
166+ rnrclient.server_status()
167+ self.assertEquals('http', called_scheme())
168+ rnrclient.review_stats()
169+ self.assertEquals('http', called_scheme())
170+ rnrclient.get_reviews(packagename="foo")
171+ self.assertEquals('http', called_scheme())
172+ review = ReviewRequest(package_name='foo', summary='bar',
173+ version='1.0', review_text='baz', rating=4, language='en',
174+ origin='troz', distroseries='eegad', arch_tag='zot')
175+ rnrclient.submit_review(review=review)
176+ self.assertEquals('https', called_scheme())
177+ rnrclient.flag_review(review_id=1, reason='foo', text='bar')
178+ self.assertEquals('https', called_scheme())
179+
180+
181
182 if __name__ == "__main__":
183 unittest.main()

Subscribers

People subscribed via source and target branches

to all changes: