Merge lp:~fabricematrat/charmworld/redirect-interfaces into lp:charmworld
- redirect-interfaces
- Merge into trunk
Proposed by
Fabrice Matrat
Status: | Merged |
---|---|
Approved by: | Fabrice Matrat |
Approved revision: | 521 |
Merged at revision: | 522 |
Proposed branch: | lp:~fabricematrat/charmworld/redirect-interfaces |
Merge into: | lp:charmworld |
Diff against target: |
317 lines (+62/-104) 2 files modified
charmworld/views/charms.py (+45/-21) charmworld/views/tests/test_charms.py (+17/-83) |
To merge this branch: | bzr merge lp:~fabricematrat/charmworld/redirect-interfaces |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Juju Gui Bot | continuous-integration | Approve | |
Francesco Banconi | Approve | ||
Brad Crittenden (community) | code + qa | Approve | |
Review via email:
|
Commit message
Added redirection for interface, config, hook
R=bac, frankban
Description of the change
Added redirection for interface, config, hook
To post a comment you must log in.
Revision history for this message

Francesco Banconi (frankban) wrote : | # |
Looks good.
review:
Approve
Revision history for this message

Juju Gui Bot (juju-gui-bot) : | # |
review:
Approve
(continuous-integration)
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'charmworld/views/charms.py' | |||
2 | --- charmworld/views/charms.py 2015-02-09 18:46:13 +0000 | |||
3 | +++ charmworld/views/charms.py 2015-02-13 16:29:40 +0000 | |||
4 | @@ -35,7 +35,6 @@ | |||
5 | 35 | format_change, | 35 | format_change, |
6 | 36 | format_proof, | 36 | format_proof, |
7 | 37 | found, | 37 | found, |
8 | 38 | interfaces, | ||
9 | 39 | name_filter, | 38 | name_filter, |
10 | 40 | ) | 39 | ) |
11 | 41 | 40 | ||
12 | @@ -356,30 +355,62 @@ | |||
13 | 356 | route_name="hook", | 355 | route_name="hook", |
14 | 357 | renderer="charmworld:templates/hook.pt") | 356 | renderer="charmworld:templates/hook.pt") |
15 | 358 | def hook(request): | 357 | def hook(request): |
18 | 359 | charm = Charm(find_charm(request, promulgated=True)) | 358 | _reconcile_revision(request) |
19 | 360 | return _hook_view(request, charm, request.matchdict["hook"]) | 359 | redirect_url = request.registry.settings.get('redirect_jujucharms') |
20 | 360 | match = request.matchdict | ||
21 | 361 | location = '{url}/{charm}/{series}'.format( | ||
22 | 362 | url=redirect_url, | ||
23 | 363 | charm=match['charm'], | ||
24 | 364 | series=match['series'] | ||
25 | 365 | ) | ||
26 | 366 | raise HTTPMovedPermanently(location=location) | ||
27 | 361 | 367 | ||
28 | 362 | 368 | ||
29 | 363 | @cached_view_config( | 369 | @cached_view_config( |
30 | 364 | route_name="personal-hook", | 370 | route_name="personal-hook", |
31 | 365 | renderer="charmworld:templates/hook.pt") | 371 | renderer="charmworld:templates/hook.pt") |
32 | 366 | def personal_hook(request): | 372 | def personal_hook(request): |
35 | 367 | charm = Charm(find_charm(request)) | 373 | _reconcile_revision(request) |
36 | 368 | return _hook_view(request, charm, request.matchdict["hook"]) | 374 | redirect_url = request.registry.settings.get('redirect_jujucharms') |
37 | 375 | match = request.matchdict | ||
38 | 376 | location = '{url}/u/{owner}/{charm}/{series}'.format( | ||
39 | 377 | url=redirect_url, | ||
40 | 378 | owner=match['owner'], | ||
41 | 379 | charm=match['charm'], | ||
42 | 380 | series=match['series'] | ||
43 | 381 | ) | ||
44 | 382 | raise HTTPMovedPermanently(location=location) | ||
45 | 369 | 383 | ||
46 | 370 | 384 | ||
47 | 371 | @cached_view_config( | 385 | @cached_view_config( |
48 | 372 | route_name="config", | 386 | route_name="config", |
49 | 373 | renderer="charmworld:templates/config.pt") | 387 | renderer="charmworld:templates/config.pt") |
50 | 374 | def config(request): | 388 | def config(request): |
52 | 375 | return _config_view(find_charm(request, promulgated=True)) | 389 | _reconcile_revision(request) |
53 | 390 | redirect_url = request.registry.settings.get('redirect_jujucharms') | ||
54 | 391 | match = request.matchdict | ||
55 | 392 | location = '{url}/{charm}/{series}#configuration'.format( | ||
56 | 393 | url=redirect_url, | ||
57 | 394 | charm=match['charm'], | ||
58 | 395 | series=match['series'] | ||
59 | 396 | ) | ||
60 | 397 | raise HTTPMovedPermanently(location=location) | ||
61 | 376 | 398 | ||
62 | 377 | 399 | ||
63 | 378 | @cached_view_config( | 400 | @cached_view_config( |
64 | 379 | route_name="personal-config", | 401 | route_name="personal-config", |
65 | 380 | renderer="charmworld:templates/config.pt") | 402 | renderer="charmworld:templates/config.pt") |
66 | 381 | def personal_config(request): | 403 | def personal_config(request): |
68 | 382 | return _config_view(find_charm(request)) | 404 | _reconcile_revision(request) |
69 | 405 | redirect_url = request.registry.settings.get('redirect_jujucharms') | ||
70 | 406 | match = request.matchdict | ||
71 | 407 | location = '{url}/u/{owner}/{charm}/{series}#configuration'.format( | ||
72 | 408 | url=redirect_url, | ||
73 | 409 | owner=match['owner'], | ||
74 | 410 | charm=match['charm'], | ||
75 | 411 | series=match['series'] | ||
76 | 412 | ) | ||
77 | 413 | raise HTTPMovedPermanently(location=location) | ||
78 | 383 | 414 | ||
79 | 384 | 415 | ||
80 | 385 | @cached_view_config( | 416 | @cached_view_config( |
81 | @@ -389,10 +420,8 @@ | |||
82 | 389 | route_name="interface-collection", | 420 | route_name="interface-collection", |
83 | 390 | renderer="charmworld:templates/interface-collection-redux.pt") | 421 | renderer="charmworld:templates/interface-collection-redux.pt") |
84 | 391 | def interface_collection(request): | 422 | def interface_collection(request): |
89 | 392 | return { | 423 | redirect_url = request.registry.settings.get('redirect_jujucharms') |
90 | 393 | "interfaces": interfaces(request.db), | 424 | raise HTTPMovedPermanently(location='{}/solutions'.format(redirect_url)) |
87 | 394 | "project": request.registry.settings.get('project_name', 'Not Set.'), | ||
88 | 395 | } | ||
91 | 396 | 425 | ||
92 | 397 | 426 | ||
93 | 398 | @cached_view_config( | 427 | @cached_view_config( |
94 | @@ -400,16 +429,11 @@ | |||
95 | 400 | renderer="charmworld:templates/interface.pt") | 429 | renderer="charmworld:templates/interface.pt") |
96 | 401 | def interface(request): | 430 | def interface(request): |
97 | 402 | iface = request.matchdict["interface"] | 431 | iface = request.matchdict["interface"] |
108 | 403 | return { | 432 | redirect_url = request.registry.settings.get('redirect_jujucharms') |
109 | 404 | "interface": iface, | 433 | raise HTTPMovedPermanently( |
110 | 405 | "providers": [ | 434 | location='{url}/q/{interface}'.format(url=redirect_url, |
111 | 406 | Charm(charm) | 435 | interface=iface) |
112 | 407 | for charm in find_charms(request.db, {"i_provides": iface}, | 436 | ) |
103 | 408 | sort=[("name", pymongo.ASCENDING)])], | ||
104 | 409 | "requirers": [ | ||
105 | 410 | Charm(charm) | ||
106 | 411 | for charm in find_charms(request.db, {"i_requires": iface}, | ||
107 | 412 | sort=[("name", pymongo.ASCENDING)])]} | ||
113 | 413 | 437 | ||
114 | 414 | 438 | ||
115 | 415 | @view_config( | 439 | @view_config( |
116 | 416 | 440 | ||
117 | === modified file 'charmworld/views/tests/test_charms.py' | |||
118 | --- charmworld/views/tests/test_charms.py 2015-02-09 14:16:33 +0000 | |||
119 | +++ charmworld/views/tests/test_charms.py 2015-02-13 16:29:40 +0000 | |||
120 | @@ -159,64 +159,29 @@ | |||
121 | 159 | self.assertIn('/q/me/one', e.exception.location) | 159 | self.assertIn('/q/me/one', e.exception.location) |
122 | 160 | 160 | ||
123 | 161 | def test_interface(self): | 161 | def test_interface(self): |
158 | 162 | # interface() returns all charms requiring or providing a given | 162 | request = self.getRequest() |
159 | 163 | # interface. | 163 | request.matchdict = {'interface': 'httpX'} |
160 | 164 | providing = Charm(factory.makeCharm( | 164 | with self.assertRaises(HTTPMovedPermanently) as e: |
161 | 165 | self.db, provides={'foo': {'interface': 'httpX'}})[1]) | 165 | interface(request) |
162 | 166 | requiring = Charm(factory.makeCharm( | 166 | self.assertIn('/q/httpX', e.exception.location) |
129 | 167 | self.db, requires={'bar': {'interface': 'httpX'}})[1]) | ||
130 | 168 | ignore, other = factory.makeCharm(self.db) | ||
131 | 169 | request = self.getRequest() | ||
132 | 170 | request.matchdict = {'interface': 'httpX'} | ||
133 | 171 | response = interface(request) | ||
134 | 172 | self.assertEqual('httpX', response['interface']) | ||
135 | 173 | found_providing = response['providers'] | ||
136 | 174 | self.assertEqual(1, len(found_providing)) | ||
137 | 175 | self.assertEqual(providing, found_providing[0]) | ||
138 | 176 | found_requiring = response['requirers'] | ||
139 | 177 | self.assertEqual(1, len(found_requiring)) | ||
140 | 178 | self.assertEqual(requiring, found_requiring[0]) | ||
141 | 179 | |||
142 | 180 | def test_interface_charm_with_errors(self): | ||
143 | 181 | # Charms with errors are not returned by interface(). | ||
144 | 182 | providing = Charm(factory.makeCharm( | ||
145 | 183 | self.db, provides={'foo': {'interface': 'httpX'}})[1]) | ||
146 | 184 | Charm(factory.makeCharm( | ||
147 | 185 | self.db, requires={'bar': {'interface': 'httpX'}}, | ||
148 | 186 | charm_error=True)[1]) | ||
149 | 187 | request = self.getRequest() | ||
150 | 188 | request.matchdict = {'interface': 'httpX'} | ||
151 | 189 | response = interface(request) | ||
152 | 190 | self.assertEqual('httpX', response['interface']) | ||
153 | 191 | found_providing = response['providers'] | ||
154 | 192 | self.assertEqual(1, len(found_providing)) | ||
155 | 193 | self.assertEqual(providing, found_providing[0]) | ||
156 | 194 | found_requiring = response['requirers'] | ||
157 | 195 | self.assertEqual(0, len(found_requiring)) | ||
163 | 196 | 167 | ||
164 | 197 | def test_config(self): | 168 | def test_config(self): |
165 | 198 | ignore, charm = factory.makeCharm( | ||
166 | 199 | self.db, name='foo', owner='bar', series='precise', | ||
167 | 200 | promulgated=True) | ||
168 | 201 | request = self.getRequest() | 169 | request = self.getRequest() |
169 | 202 | request.matchdict = {'charm': 'foo', 'series': 'precise'} | 170 | request.matchdict = {'charm': 'foo', 'series': 'precise'} |
173 | 203 | response = config(request) | 171 | with self.assertRaises(HTTPMovedPermanently) as e: |
174 | 204 | expected = {"charm": Charm(charm), "onload": "prettyPrint()"} | 172 | config(request) |
175 | 205 | self.assertEqual(expected, response) | 173 | self.assertIn('/foo/precise#configuration', e.exception.location) |
176 | 206 | 174 | ||
177 | 207 | def test_personal_config(self): | 175 | def test_personal_config(self): |
178 | 208 | ignore, charm = factory.makeCharm( | ||
179 | 209 | self.db, name='foo', owner='bar', series='precise', | ||
180 | 210 | promulgated=False) | ||
181 | 211 | request = self.getRequest() | 176 | request = self.getRequest() |
182 | 212 | request.matchdict = { | 177 | request.matchdict = { |
183 | 213 | 'charm': 'foo', | 178 | 'charm': 'foo', |
184 | 214 | 'series': 'precise', | 179 | 'series': 'precise', |
185 | 215 | 'owner': 'bar' | 180 | 'owner': 'bar' |
186 | 216 | } | 181 | } |
190 | 217 | response = personal_config(request) | 182 | with self.assertRaises(HTTPMovedPermanently) as e: |
191 | 218 | expected = {"charm": Charm(charm), "onload": "prettyPrint()"} | 183 | personal_config(request) |
192 | 219 | self.assertEqual(expected, response) | 184 | self.assertIn('/bar/foo/precise#configuration', e.exception.location) |
193 | 220 | 185 | ||
194 | 221 | def make_charm_with_files(self, promulgated): | 186 | def make_charm_with_files(self, promulgated): |
195 | 222 | ignore, charm = factory.makeCharm( | 187 | ignore, charm = factory.makeCharm( |
196 | @@ -232,23 +197,17 @@ | |||
197 | 232 | return charm | 197 | return charm |
198 | 233 | 198 | ||
199 | 234 | def test_hook(self): | 199 | def test_hook(self): |
200 | 235 | charm = Charm(self.make_charm_with_files(promulgated=True)) | ||
201 | 236 | request = self.getRequest() | 200 | request = self.getRequest() |
202 | 237 | request.matchdict = { | 201 | request.matchdict = { |
203 | 238 | 'charm': 'sample_charm', | 202 | 'charm': 'sample_charm', |
204 | 239 | 'series': 'precise', | 203 | 'series': 'precise', |
205 | 240 | 'hook': 'install', | 204 | 'hook': 'install', |
206 | 241 | } | 205 | } |
214 | 242 | response = hook(request) | 206 | with self.assertRaises(HTTPMovedPermanently) as e: |
215 | 243 | self.assertEqual('Not Set.', response['project']) | 207 | hook(request) |
216 | 244 | self.assertEqual('install', response['hook']) | 208 | self.assertIn('/sample_charm/precise', e.exception.location) |
210 | 245 | self.assertTrue(response['hook_raw'].startswith( | ||
211 | 246 | '#!/bin/bash\nset -eux\n\n# This PPA is no longer required.')) | ||
212 | 247 | self.assertEqual(charm, response['charm']) | ||
213 | 248 | self.assertEqual('prettyPrint()', response['onload']) | ||
217 | 249 | 209 | ||
218 | 250 | def test_personal_hook(self): | 210 | def test_personal_hook(self): |
219 | 251 | charm = Charm(self.make_charm_with_files(promulgated=False)) | ||
220 | 252 | request = self.getRequest() | 211 | request = self.getRequest() |
221 | 253 | request.matchdict = { | 212 | request.matchdict = { |
222 | 254 | 'charm': 'sample_charm', | 213 | 'charm': 'sample_charm', |
223 | @@ -256,17 +215,11 @@ | |||
224 | 256 | 'hook': 'install', | 215 | 'hook': 'install', |
225 | 257 | 'owner': 'bar', | 216 | 'owner': 'bar', |
226 | 258 | } | 217 | } |
234 | 259 | response = personal_hook(request) | 218 | with self.assertRaises(HTTPMovedPermanently) as e: |
235 | 260 | self.assertEqual('Not Set.', response['project']) | 219 | personal_hook(request) |
236 | 261 | self.assertEqual('install', response['hook']) | 220 | self.assertIn('/bar/sample_charm/precise', e.exception.location) |
230 | 262 | self.assertTrue(response['hook_raw'].startswith( | ||
231 | 263 | '#!/bin/bash\nset -eux\n\n# This PPA is no longer required.')) | ||
232 | 264 | self.assertEqual(charm, response['charm']) | ||
233 | 265 | self.assertEqual('prettyPrint()', response['onload']) | ||
237 | 266 | 221 | ||
238 | 267 | def test_distro_charm(self): | 222 | def test_distro_charm(self): |
239 | 268 | self.enable_routes() | ||
240 | 269 | self.make_charm_with_files(promulgated=True) | ||
241 | 270 | request = self.getRequest() | 223 | request = self.getRequest() |
242 | 271 | request.matchdict = { | 224 | request.matchdict = { |
243 | 272 | 'charm': 'sample_charm', | 225 | 'charm': 'sample_charm', |
244 | @@ -377,10 +330,6 @@ | |||
245 | 377 | 330 | ||
246 | 378 | def test_charm_collection_with_missing_data(self): | 331 | def test_charm_collection_with_missing_data(self): |
247 | 379 | """Charms with missing data are rendered.""" | 332 | """Charms with missing data are rendered.""" |
248 | 380 | one_id, one = factory.makeCharm(self.db, promulgated=True) | ||
249 | 381 | two_id, two = factory.makeCharm(self.db, promulgated=True) | ||
250 | 382 | del two['summary'] | ||
251 | 383 | self.db.charms.save(two) | ||
252 | 384 | response = self.app.get('/charms/precise', status=301) | 333 | response = self.app.get('/charms/precise', status=301) |
253 | 385 | self.assertIn('/q/precise', response.body) | 334 | self.assertIn('/q/precise', response.body) |
254 | 386 | 335 | ||
255 | @@ -388,8 +337,6 @@ | |||
256 | 388 | # The route /~owner/series/charm-1 finds charms using owner, | 337 | # The route /~owner/series/charm-1 finds charms using owner, |
257 | 389 | # series, charm name, and revision. The deploy instruction | 338 | # series, charm name, and revision. The deploy instruction |
258 | 390 | # does not include the revision. | 339 | # does not include the revision. |
259 | 391 | ignore, charm = factory.makeCharm( | ||
260 | 392 | self.db, owner='owner', series='series', name='charm', files={}) | ||
261 | 393 | response = self.app.get('/~owner/series/charm-1', status=301) | 340 | response = self.app.get('/~owner/series/charm-1', status=301) |
262 | 394 | self.assertIn('/u/owner/charm/series/1', response.body) | 341 | self.assertIn('/u/owner/charm/series/1', response.body) |
263 | 395 | 342 | ||
264 | @@ -397,17 +344,12 @@ | |||
265 | 397 | # /~owner/series/charm-name-1 route find the charm. | 344 | # /~owner/series/charm-name-1 route find the charm. |
266 | 398 | # This is a corner-case where the revision and charm name are | 345 | # This is a corner-case where the revision and charm name are |
267 | 399 | # ambigous because they are munged. | 346 | # ambigous because they are munged. |
268 | 400 | ignore, charm = factory.makeCharm( | ||
269 | 401 | self.db, owner='owner', series='series', name='charm-name', | ||
270 | 402 | files={}) | ||
271 | 403 | response = self.app.get('/~owner/series/charm-name-1', status=301) | 347 | response = self.app.get('/~owner/series/charm-name-1', status=301) |
272 | 404 | self.assertIn('/u/owner/charm-name/series/1', response.body) | 348 | self.assertIn('/u/owner/charm-name/series/1', response.body) |
273 | 405 | 349 | ||
274 | 406 | def test_route_personal_charm_with_head_revision(self): | 350 | def test_route_personal_charm_with_head_revision(self): |
275 | 407 | # the /~owner/series/charm-name-HEAD route finds the charm. | 351 | # the /~owner/series/charm-name-HEAD route finds the charm. |
276 | 408 | # The juju-gui apache rewrite versionless charm urls to help the GUI. | 352 | # The juju-gui apache rewrite versionless charm urls to help the GUI. |
277 | 409 | ignore, charm = factory.makeCharm( | ||
278 | 410 | self.db, owner='owner', series='series', name='charm', files={}) | ||
279 | 411 | response = self.app.get('/~owner/series/charm-HEAD', status=301) | 353 | response = self.app.get('/~owner/series/charm-HEAD', status=301) |
280 | 412 | self.assertIn('/u/owner/charm/series', response.body) | 354 | self.assertIn('/u/owner/charm/series', response.body) |
281 | 413 | self.assertNotIn('HEAD', response.body) | 355 | self.assertNotIn('HEAD', response.body) |
282 | @@ -416,8 +358,6 @@ | |||
283 | 416 | # The route /~owner/series/charm route finds the charm using owner | 358 | # The route /~owner/series/charm route finds the charm using owner |
284 | 417 | # series, and charm name. Revision is ignored. The deploy instruction | 359 | # series, and charm name. Revision is ignored. The deploy instruction |
285 | 418 | # matches the route. | 360 | # matches the route. |
286 | 419 | ignore, charm = factory.makeCharm( | ||
287 | 420 | self.db, owner='owner', series='series', name='charm', files={}) | ||
288 | 421 | response = self.app.get('/~owner/series/charm', status=301) | 361 | response = self.app.get('/~owner/series/charm', status=301) |
289 | 422 | self.assertIn('/u/owner/charm/series', response.body) | 362 | self.assertIn('/u/owner/charm/series', response.body) |
290 | 423 | 363 | ||
291 | @@ -443,8 +383,6 @@ | |||
292 | 443 | # The route /series/charm-1 route finds promulgated charms using | 383 | # The route /series/charm-1 route finds promulgated charms using |
293 | 444 | # The series, charm name and version. The deploy instruction does not | 384 | # The series, charm name and version. The deploy instruction does not |
294 | 445 | # include the version. | 385 | # include the version. |
295 | 446 | factory.makeCharm( | ||
296 | 447 | self.db, promulgated=True, series='series', name='charm', files={}) | ||
297 | 448 | response = self.app.get('/series/charm-1', status=301) | 386 | response = self.app.get('/series/charm-1', status=301) |
298 | 449 | self.assertIn('/charm/series/1', response.body) | 387 | self.assertIn('/charm/series/1', response.body) |
299 | 450 | 388 | ||
300 | @@ -452,8 +390,6 @@ | |||
301 | 452 | # The route /series/charm-HEAD finds promulgated charms using | 390 | # The route /series/charm-HEAD finds promulgated charms using |
302 | 453 | # series, charm name, and head revision. The revision is not included | 391 | # series, charm name, and head revision. The revision is not included |
303 | 454 | # in the deploy instruction. | 392 | # in the deploy instruction. |
304 | 455 | ignore, charm = factory.makeCharm( | ||
305 | 456 | self.db, promulgated=True, series='series', name='charm', files={}) | ||
306 | 457 | response = self.app.get('/series/charm-HEAD', status=301) | 393 | response = self.app.get('/series/charm-HEAD', status=301) |
307 | 458 | self.assertNotIn('HEAD', response.body) | 394 | self.assertNotIn('HEAD', response.body) |
308 | 459 | self.assertIn('/charm/series', response.body) | 395 | self.assertIn('/charm/series', response.body) |
309 | @@ -461,8 +397,6 @@ | |||
310 | 461 | def test_route_promulgated_charm_without_revision(self): | 397 | def test_route_promulgated_charm_without_revision(self): |
311 | 462 | # The route /series/charm finds the promulgated charms using just | 398 | # The route /series/charm finds the promulgated charms using just |
312 | 463 | # the series and charm name. The deploy instruction matches the route. | 399 | # the series and charm name. The deploy instruction matches the route. |
313 | 464 | factory.makeCharm( | ||
314 | 465 | self.db, promulgated=True, series='series', name='charm', files={}) | ||
315 | 466 | self.app.get('/series/charm', status=301) | 400 | self.app.get('/series/charm', status=301) |
316 | 467 | 401 | ||
317 | 468 | def test_charm_short_url_route(self): | 402 | def test_charm_short_url_route(self): |
Code looks good.
QA OK