Merge lp:~abentley/charmworld/remove-api-0 into lp:~juju-jitsu/charmworld/trunk
- remove-api-0
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Aaron Bentley |
Approved revision: | 250 |
Merged at revision: | 243 |
Proposed branch: | lp:~abentley/charmworld/remove-api-0 |
Merge into: | lp:~juju-jitsu/charmworld/trunk |
Diff against target: |
727 lines (+140/-317) 13 files modified
charmworld/routes.py (+0/-3) charmworld/search.py (+24/-7) charmworld/tests/test_search.py (+30/-6) charmworld/views/api.py (+0/-30) charmworld/views/tests/test_api.py (+33/-252) migrations/migrate.py (+11/-6) migrations/test_migrate.py (+8/-4) migrations/versions/001_load_qa_questions.py (+1/-1) migrations/versions/002_Escape_dots_dollar_signs_percent_signs_in_mongo_keys.py (+1/-1) migrations/versions/003_remove_provider_qa_questions.py (+1/-1) migrations/versions/004_remove_charm_errors.py (+3/-1) migrations/versions/005_delete_icon_field.py (+8/-0) migrations/versions/tests/test_migrations.py (+20/-5) |
To merge this branch: | bzr merge lp:~abentley/charmworld/remove-api-0 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Curtis Hovey (community) | code | Approve | |
Review via email:
|
Commit message
Remove API 0
Description of the change
Remove API 0 and introduce a migration to remove icons from charm data.
Since the charm formatting changes, the migration must also update elasticsearch. This is done using reindex, which gains a charms parameter so that mongodb can be used as a data source, and now handles the case where the index does not already exist.
Much test refactoring was done, since all current APIs (i.e. API1) run all
tests now.
copy_index was broken, because it invoked index_charms on itself, not the pending copy. This is now fixed. It is also renamed to "create_
unlimited_search is updated to raise IndexMissing if the search errors out with an IndexMissingExc
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Charmworld Lander (charmworld-lander) wrote : | # |
There are additional revisions which have not been approved in review. Please seek review and approval of these new revisions.
Preview Diff
1 | === modified file 'charmworld/routes.py' | |||
2 | --- charmworld/routes.py 2013-04-29 18:21:35 +0000 | |||
3 | +++ charmworld/routes.py 2013-06-04 16:12:32 +0000 | |||
4 | @@ -80,9 +80,6 @@ | |||
5 | 80 | config.add_route('auth_callback', '/auth_callback') | 80 | config.add_route('auth_callback', '/auth_callback') |
6 | 81 | config.add_route('logout', '/logout') | 81 | config.add_route('logout', '/logout') |
7 | 82 | 82 | ||
8 | 83 | # API | ||
9 | 84 | config.add_route('api_0', '/api/0/{endpoint}{remainder:.*}') | ||
10 | 85 | config.add_view('charmworld.views.api.API0', route_name='api_0') | ||
11 | 86 | config.add_route('api_1', '/api/1/{endpoint}{remainder:.*}') | 83 | config.add_route('api_1', '/api/1/{endpoint}{remainder:.*}') |
12 | 87 | config.add_view('charmworld.views.api.API1', route_name='api_1') | 84 | config.add_view('charmworld.views.api.API1', route_name='api_1') |
13 | 88 | 85 | ||
14 | 89 | 86 | ||
15 | === modified file 'charmworld/search.py' | |||
16 | --- charmworld/search.py 2013-05-29 12:36:35 +0000 | |||
17 | +++ charmworld/search.py 2013-06-04 16:12:32 +0000 | |||
18 | @@ -233,7 +233,8 @@ | |||
19 | 233 | kwargs = {'index': self.index_name} | 233 | kwargs = {'index': self.index_name} |
20 | 234 | if limit is not None: | 234 | if limit is not None: |
21 | 235 | kwargs['size'] = limit | 235 | kwargs['size'] = limit |
23 | 236 | hits = self._client.search(dsl, **kwargs) | 236 | with translate_error(): |
24 | 237 | hits = self._client.search(dsl, **kwargs) | ||
25 | 237 | if limit is None and len(hits['hits']['hits']) < hits['hits']['total']: | 238 | if limit is None and len(hits['hits']['hits']) < hits['hits']['total']: |
26 | 238 | kwargs['size'] = hits['hits']['total'] | 239 | kwargs['size'] = hits['hits']['total'] |
27 | 239 | hits = self._client.search(dsl, **kwargs) | 240 | hits = self._client.search(dsl, **kwargs) |
28 | @@ -348,40 +349,56 @@ | |||
29 | 348 | name = '%s-%d' % (self.index_name, random.randint(1, 99999)) | 349 | name = '%s-%d' % (self.index_name, random.randint(1, 99999)) |
30 | 349 | return self.__class__(self._client, name) | 350 | return self.__class__(self._client, name) |
31 | 350 | 351 | ||
34 | 351 | def copy_index(self, name=None): | 352 | def create_replacement(self, name=None, charms=None): |
35 | 352 | """Create a copy of this index with the specified name. | 353 | """Create a replacement for this index. |
36 | 353 | 354 | ||
37 | 354 | The mapping will not be this index's mapping, it will be the current | 355 | The mapping will not be this index's mapping, it will be the current |
38 | 355 | mapping (e.g. put_mapping). | 356 | mapping (e.g. put_mapping). |
39 | 356 | 357 | ||
40 | 358 | :param name: If not supplied, a name based on this index's name will | ||
41 | 359 | be used. | ||
42 | 360 | :param charms: A list of charms. If None, the charms in the current | ||
43 | 361 | index will be used. | ||
44 | 357 | :return: The ElasticSearchClient for the supplied mapping. | 362 | :return: The ElasticSearchClient for the supplied mapping. |
45 | 358 | """ | 363 | """ |
46 | 359 | copy = self.get_aliasable_client(name) | 364 | copy = self.get_aliasable_client(name) |
47 | 360 | copy.create_index() | 365 | copy.create_index() |
48 | 361 | try: | 366 | try: |
49 | 362 | copy.put_mapping() | 367 | copy.put_mapping() |
51 | 363 | self.index_charms(self.api_search(valid_charms_only=False)) | 368 | if charms is None: |
52 | 369 | try: | ||
53 | 370 | charms = self.api_search(valid_charms_only=False) | ||
54 | 371 | except IndexMissing: | ||
55 | 372 | charms = [] | ||
56 | 373 | copy.index_charms(charms) | ||
57 | 364 | return copy | 374 | return copy |
58 | 365 | except: | 375 | except: |
59 | 366 | copy.delete_index() | 376 | copy.delete_index() |
60 | 367 | raise | 377 | raise |
61 | 368 | 378 | ||
62 | 369 | 379 | ||
64 | 370 | def reindex(index_client): | 380 | def reindex(index_client, charms=None): |
65 | 371 | """Reindex documents with the current mapping. | 381 | """Reindex documents with the current mapping. |
66 | 372 | 382 | ||
67 | 373 | This works by creating a new index and then aliasing it to the old index. | 383 | This works by creating a new index and then aliasing it to the old index. |
68 | 374 | If the old index was already an alias, this is a simple swap. If not, the | 384 | If the old index was already an alias, this is a simple swap. If not, the |
69 | 375 | old index is deleted. | 385 | old index is deleted. |
70 | 386 | |||
71 | 387 | :param charms: A list of charms to use. If None, the charms in the index | ||
72 | 388 | will be used. | ||
73 | 376 | """ | 389 | """ |
75 | 377 | new_index = index_client.copy_index() | 390 | new_index = index_client.create_replacement(charms=charms) |
76 | 378 | try: | 391 | try: |
77 | 379 | aliased = index_client.get_aliased() | 392 | aliased = index_client.get_aliased() |
78 | 380 | if aliased == []: | 393 | if aliased == []: |
80 | 381 | index_client.delete_index() | 394 | try: |
81 | 395 | index_client.delete_index() | ||
82 | 396 | except IndexMissing: | ||
83 | 397 | pass | ||
84 | 382 | index_client.update_aliased(new_index.index_name, aliased) | 398 | index_client.update_aliased(new_index.index_name, aliased) |
85 | 383 | except: | 399 | except: |
86 | 384 | new_index.delete_index() | 400 | new_index.delete_index() |
87 | 401 | raise | ||
88 | 385 | if aliased != []: | 402 | if aliased != []: |
89 | 386 | for name in aliased: | 403 | for name in aliased: |
90 | 387 | ElasticSearchClient(index_client._client, name).delete_index() | 404 | ElasticSearchClient(index_client._client, name).delete_index() |
91 | 388 | 405 | ||
92 | === modified file 'charmworld/tests/test_search.py' | |||
93 | --- charmworld/tests/test_search.py 2013-05-29 12:36:35 +0000 | |||
94 | +++ charmworld/tests/test_search.py 2013-06-04 16:12:32 +0000 | |||
95 | @@ -444,12 +444,29 @@ | |||
96 | 444 | alias.update_aliased('bar', alias.get_aliased()) | 444 | alias.update_aliased('bar', alias.get_aliased()) |
97 | 445 | self.assertEqual(['bar'], alias.get_aliased()) | 445 | self.assertEqual(['bar'], alias.get_aliased()) |
98 | 446 | 446 | ||
105 | 447 | def test_copy_index(self): | 447 | def test_create_replacement(self): |
106 | 448 | self.index_client.index_charm({'_id': 'a', 'name': 'foo'}) | 448 | self.index_client.index_charm({'_id': 'a', 'name': 'foo'}) |
107 | 449 | copy = self.index_client.copy_index('index-copy') | 449 | copy = self.index_client.create_replacement('index-copy') |
108 | 450 | self.addCleanup(copy.delete_index) | 450 | self.addCleanup(copy.delete_index) |
109 | 451 | copy.wait_for_startup() | 451 | copy.wait_for_startup() |
110 | 452 | self.assertIn('series', copy.get_mapping()['charm']['properties']) | 452 | self.assertIn('series', copy.get_mapping()['charm']['properties']) |
111 | 453 | self.assertEqual({'_id': 'a', 'name': 'foo'}, copy.get('a')) | ||
112 | 454 | |||
113 | 455 | def test_create_replacement_charms(self): | ||
114 | 456 | self.index_client.index_charm({'_id': 'a', 'name': 'foo'}) | ||
115 | 457 | copy = self.index_client.create_replacement( | ||
116 | 458 | charms=[{'_id': 'a', 'name': 'bar'}]) | ||
117 | 459 | self.addCleanup(copy.delete_index) | ||
118 | 460 | copy.wait_for_startup() | ||
119 | 461 | self.assertIn('series', copy.get_mapping()['charm']['properties']) | ||
120 | 462 | self.assertEqual({'_id': 'a', 'name': 'bar'}, copy.get('a')) | ||
121 | 463 | |||
122 | 464 | def test_create_replacement_misssing(self): | ||
123 | 465 | client = ElasticSearchClient.from_settings(get_ini(), 'temp-index') | ||
124 | 466 | # This should not raise an exception, even though the index has not | ||
125 | 467 | # been created. | ||
126 | 468 | copy = client.create_replacement() | ||
127 | 469 | self.addCleanup(copy.delete_index) | ||
128 | 453 | 470 | ||
129 | 454 | def test_put_mapping_incompatible_mapping_error(self): | 471 | def test_put_mapping_incompatible_mapping_error(self): |
130 | 455 | # The error we get from an incompatible mapping is IncompatibleMapping | 472 | # The error we get from an incompatible mapping is IncompatibleMapping |
131 | @@ -600,3 +617,10 @@ | |||
132 | 600 | mapping = alias.get_mapping() | 617 | mapping = alias.get_mapping() |
133 | 601 | self.assertEqual('not_analyzed', | 618 | self.assertEqual('not_analyzed', |
134 | 602 | mapping['charm']['properties']['name']['index']) | 619 | mapping['charm']['properties']['name']['index']) |
135 | 620 | |||
136 | 621 | def test_reindexed_no_client_charms(self): | ||
137 | 622 | client = ElasticSearchClient.from_settings(get_ini()) | ||
138 | 623 | # This should not raise an exception, even though the index does not | ||
139 | 624 | # exist. | ||
140 | 625 | new_client = reindex(client, charms=[]) | ||
141 | 626 | new_client.delete_index() | ||
142 | 603 | 627 | ||
143 | === modified file 'charmworld/views/api.py' | |||
144 | --- charmworld/views/api.py 2013-06-03 16:18:58 +0000 | |||
145 | +++ charmworld/views/api.py 2013-06-04 16:12:32 +0000 | |||
146 | @@ -412,33 +412,3 @@ | |||
147 | 412 | 'popular': self._charm_results(popular), | 412 | 'popular': self._charm_results(popular), |
148 | 413 | } | 413 | } |
149 | 414 | } | 414 | } |
150 | 415 | |||
151 | 416 | |||
152 | 417 | class API0(API1): | ||
153 | 418 | """Implementation of API 0. | ||
154 | 419 | |||
155 | 420 | All methods whose names do not begin with an underscore are exposed. | ||
156 | 421 | Methods may return a webob Response, which is returned directly, or a | ||
157 | 422 | json-serializable value, which will be returned as a json HTTP response. | ||
158 | 423 | """ | ||
159 | 424 | |||
160 | 425 | @classmethod | ||
161 | 426 | def _charm_result(cls, charm, requires, provides): | ||
162 | 427 | return cls._format_charm(charm) | ||
163 | 428 | |||
164 | 429 | def _charm_results(self, charms): | ||
165 | 430 | return [self._charm_result(charm, {}, {}) for charm in charms] | ||
166 | 431 | |||
167 | 432 | def _charms(self, limit=None, name=None, series=None, owner=None, | ||
168 | 433 | provides=None, requires=None, type_=None, provider=None, | ||
169 | 434 | scope=None, category=None, categories=None, text=None): | ||
170 | 435 | return super(API0, self)._charms( | ||
171 | 436 | limit, name, series, owner, provides, requires, type_, provider, | ||
172 | 437 | scope, categories, text) | ||
173 | 438 | |||
174 | 439 | @staticmethod | ||
175 | 440 | def _format_charm(charm): | ||
176 | 441 | output = API1._format_charm(charm) | ||
177 | 442 | if 'icon' in charm: | ||
178 | 443 | output['icon'] = charm['icon'] | ||
179 | 444 | return output | ||
180 | 445 | 415 | ||
181 | === modified file 'charmworld/views/tests/test_api.py' | |||
182 | --- charmworld/views/tests/test_api.py 2013-06-03 16:18:58 +0000 | |||
183 | +++ charmworld/views/tests/test_api.py 2013-06-04 16:12:32 +0000 | |||
184 | @@ -15,7 +15,6 @@ | |||
185 | 15 | ) | 15 | ) |
186 | 16 | from charmworld.models import CharmSource | 16 | from charmworld.models import CharmSource |
187 | 17 | from charmworld.views.api import ( | 17 | from charmworld.views.api import ( |
188 | 18 | API0, | ||
189 | 19 | API1, | 18 | API1, |
190 | 20 | json_response, | 19 | json_response, |
191 | 21 | ) | 20 | ) |
192 | @@ -73,6 +72,30 @@ | |||
193 | 73 | return self.api_class(request)() | 72 | return self.api_class(request)() |
194 | 74 | 73 | ||
195 | 75 | 74 | ||
196 | 75 | class API1Mixin(APITestBase): | ||
197 | 76 | |||
198 | 77 | api_class = API1 | ||
199 | 78 | |||
200 | 79 | @staticmethod | ||
201 | 80 | def weightless(charm): | ||
202 | 81 | """Remove weight from charm because we cannot predict its value.""" | ||
203 | 82 | charm = dict(charm) | ||
204 | 83 | del charm['weight'] | ||
205 | 84 | return charm | ||
206 | 85 | |||
207 | 86 | @classmethod | ||
208 | 87 | def weightless_relation(cls, relation): | ||
209 | 88 | new_relation = {} | ||
210 | 89 | for interface, charms in relation.items(): | ||
211 | 90 | charms = [cls.weightless(charm) for charm in charms] | ||
212 | 91 | new_relation[interface] = charms | ||
213 | 92 | return new_relation | ||
214 | 93 | |||
215 | 94 | @classmethod | ||
216 | 95 | def related_weightless(cls, charm): | ||
217 | 96 | return cls.weightless(cls.api_class._format_related(charm, 0)) | ||
218 | 97 | |||
219 | 98 | |||
220 | 76 | class TestAPICharms: | 99 | class TestAPICharms: |
221 | 77 | 100 | ||
222 | 78 | def setUp(self): | 101 | def setUp(self): |
223 | @@ -259,19 +282,20 @@ | |||
224 | 259 | self.assertIn('new', data['result']) | 282 | self.assertIn('new', data['result']) |
225 | 260 | self.assertIn('featured', data['result']) | 283 | self.assertIn('featured', data['result']) |
226 | 261 | self.assertEqual(['pop%d' % num for num in range(14, 4, -1)], | 284 | self.assertEqual(['pop%d' % num for num in range(14, 4, -1)], |
228 | 262 | [self.get_name(r) for r in data['result']['popular']]) | 285 | [r['charm']['name'] |
229 | 286 | for r in data['result']['popular']]) | ||
230 | 263 | self.assertEqual(self.api_class._charm_result(last_popular_charm, {}, | 287 | self.assertEqual(self.api_class._charm_result(last_popular_charm, {}, |
231 | 264 | {}), | 288 | {}), |
232 | 265 | data['result']['popular'][0]) | 289 | data['result']['popular'][0]) |
233 | 266 | self.assertEqual(['new%d' % num for num in range(14, 4, -1)], | 290 | self.assertEqual(['new%d' % num for num in range(14, 4, -1)], |
235 | 267 | [self.get_name(r) for r in data['result']['new']]) | 291 | [r['charm']['name'] for r in data['result']['new']]) |
236 | 268 | self.assertEqual(self.api_class._charm_result(last_new_charm, {}, {}), | 292 | self.assertEqual(self.api_class._charm_result(last_new_charm, {}, {}), |
237 | 269 | data['result']['new'][0]) | 293 | data['result']['new'][0]) |
238 | 270 | self.assertItemsEqual( | 294 | self.assertItemsEqual( |
239 | 271 | ['featured%d' % num for num in range(14, -1, -1)], | 295 | ['featured%d' % num for num in range(14, -1, -1)], |
241 | 272 | [self.get_name(r) for r in data['result']['featured']]) | 296 | [r['charm']['name'] for r in data['result']['featured']]) |
242 | 273 | featured14, = [charm for charm in data['result']['featured'] if | 297 | featured14, = [charm for charm in data['result']['featured'] if |
244 | 274 | self.get_name(charm) == 'featured14'] | 298 | charm['charm']['name'] == 'featured14'] |
245 | 275 | self.assertEqual( | 299 | self.assertEqual( |
246 | 276 | self.api_class._charm_result(last_featured_charm, {}, {}), | 300 | self.api_class._charm_result(last_featured_charm, {}, {}), |
247 | 277 | featured14) | 301 | featured14) |
248 | @@ -281,40 +305,6 @@ | |||
249 | 281 | response = self.get_response('charms', categories='asdf') | 305 | response = self.get_response('charms', categories='asdf') |
250 | 282 | self.assertEqual(200, response.status_code) | 306 | self.assertEqual(200, response.status_code) |
251 | 283 | 307 | ||
252 | 284 | |||
253 | 285 | class API1Mixin: | ||
254 | 286 | |||
255 | 287 | api_class = API1 | ||
256 | 288 | |||
257 | 289 | @staticmethod | ||
258 | 290 | def weightless(charm): | ||
259 | 291 | """Remove weight from charm because we cannot predict its value.""" | ||
260 | 292 | charm = dict(charm) | ||
261 | 293 | del charm['weight'] | ||
262 | 294 | return charm | ||
263 | 295 | |||
264 | 296 | @classmethod | ||
265 | 297 | def weightless_relation(cls, relation): | ||
266 | 298 | new_relation = {} | ||
267 | 299 | for interface, charms in relation.items(): | ||
268 | 300 | charms = [cls.weightless(charm) for charm in charms] | ||
269 | 301 | new_relation[interface] = charms | ||
270 | 302 | return new_relation | ||
271 | 303 | |||
272 | 304 | @classmethod | ||
273 | 305 | def related_weightless(cls, charm): | ||
274 | 306 | return cls.weightless(cls.api_class._format_related(charm, 0)) | ||
275 | 307 | |||
276 | 308 | |||
277 | 309 | class TestAPI1Charms(TestAPICharms, APITestBase, API1Mixin): | ||
278 | 310 | |||
279 | 311 | def setUp(self): | ||
280 | 312 | super(TestAPI1Charms, self).setUp() | ||
281 | 313 | |||
282 | 314 | @staticmethod | ||
283 | 315 | def get_name(charm): | ||
284 | 316 | return charm['charm']['name'] | ||
285 | 317 | |||
286 | 318 | def test_charms(self): | 308 | def test_charms(self): |
287 | 319 | """Charms endpoint produces expected result.""" | 309 | """Charms endpoint produces expected result.""" |
288 | 320 | one_changes = [factory.makeChange(created=datetime(1999, 12, 31), | 310 | one_changes = [factory.makeChange(created=datetime(1999, 12, 31), |
289 | @@ -585,190 +575,8 @@ | |||
290 | 585 | ) | 575 | ) |
291 | 586 | 576 | ||
292 | 587 | 577 | ||
477 | 588 | class TestAPI0Charms(TestAPICharms, APITestBase): | 578 | class TestAPI1Charms(TestAPICharms, API1Mixin): |
478 | 589 | 579 | """Test the AP1 implementation of charms endpoint.""" | |
295 | 590 | api_class = API0 | ||
296 | 591 | |||
297 | 592 | @staticmethod | ||
298 | 593 | def get_name(charm): | ||
299 | 594 | return charm['name'] | ||
300 | 595 | |||
301 | 596 | def test_charms(self): | ||
302 | 597 | """Charms endpoint produces expected result.""" | ||
303 | 598 | one_changes = [factory.makeChange(created=datetime(1999, 12, 31), | ||
304 | 599 | revno=37, message='Hello')] | ||
305 | 600 | one_id, one = self.makeCharm(name='name1', changes=one_changes, | ||
306 | 601 | date_created=datetime(1997, 10, 29, | ||
307 | 602 | 12, 10, 9), | ||
308 | 603 | promulgated=True) | ||
309 | 604 | self.index_client.index_charm(one) | ||
310 | 605 | maintainer2 = 'jrandom@example.com (J. Random Hacker)' | ||
311 | 606 | provides2 = {'cookie-factory': { | ||
312 | 607 | 'interface': 'imap', | ||
313 | 608 | }} | ||
314 | 609 | requires2 = {'postal-service': { | ||
315 | 610 | 'interface': 'envelope', | ||
316 | 611 | }} | ||
317 | 612 | options2 = { | ||
318 | 613 | 'foobar': { | ||
319 | 614 | 'type': 'str', | ||
320 | 615 | 'default': 'baz', | ||
321 | 616 | 'description': 'quxx', | ||
322 | 617 | }, | ||
323 | 618 | } | ||
324 | 619 | files2 = { | ||
325 | 620 | 'readme': { | ||
326 | 621 | 'filename': 'README.md', | ||
327 | 622 | 'subdir': '', | ||
328 | 623 | }, | ||
329 | 624 | 'config-changed': { | ||
330 | 625 | 'filename': 'config-changed', | ||
331 | 626 | 'subdir': 'hooks', | ||
332 | 627 | }, | ||
333 | 628 | } | ||
334 | 629 | two_changes = [factory.makeChange(created=datetime(1998, 11, 30), | ||
335 | 630 | revno=32, message='Hi', | ||
336 | 631 | authors=['Baz <baz@example.org>'])] | ||
337 | 632 | two_id, two = self.makeCharm(owner='jrandom', revision=3, | ||
338 | 633 | series='warty', summary='Charm two', | ||
339 | 634 | name='name2', revno=13, | ||
340 | 635 | maintainer=maintainer2, | ||
341 | 636 | commit_message='message2', | ||
342 | 637 | provides=provides2, | ||
343 | 638 | requires=requires2, options=options2, | ||
344 | 639 | files=files2, subordinate=True, | ||
345 | 640 | changes=two_changes, | ||
346 | 641 | date_created=datetime(1996, 9, 28, | ||
347 | 642 | 11, 9, 8)) | ||
348 | 643 | response = self.get_response('charms') | ||
349 | 644 | self.assertEqual(200, response.status_code) | ||
350 | 645 | self.assertEqual('application/json', response.content_type) | ||
351 | 646 | result = response.json_body['result'] | ||
352 | 647 | self.assertEqual(2, len(result)) | ||
353 | 648 | one_url = make_store_url(1, get_address(one, short=True)) | ||
354 | 649 | self.assertEqual({'owner': 'charmers', | ||
355 | 650 | 'maintainer': { | ||
356 | 651 | 'name': 'John Doe', | ||
357 | 652 | 'email': 'jdoe@example.com', | ||
358 | 653 | }, | ||
359 | 654 | 'id': one_url[3:], | ||
360 | 655 | 'categories': [], | ||
361 | 656 | 'date_created': '1997-10-29T12:10:09', | ||
362 | 657 | 'name': 'name1', | ||
363 | 658 | 'url': one_url, | ||
364 | 659 | 'description': one['description'], | ||
365 | 660 | 'revision': 1, | ||
366 | 661 | 'summary': "Remote terminal classroom over https", | ||
367 | 662 | 'distro_series': 'precise', | ||
368 | 663 | 'is_approved': True, | ||
369 | 664 | 'rating_numerator': 0, | ||
370 | 665 | 'rating_denominator': 0, | ||
371 | 666 | 'code_source': { | ||
372 | 667 | 'type': 'bzr', | ||
373 | 668 | 'location': | ||
374 | 669 | 'lp:~charmers/charms/precise/name1/trunk', | ||
375 | 670 | 'revision': '12', | ||
376 | 671 | 'last_log': 'maintainer', | ||
377 | 672 | 'bugs_link': | ||
378 | 673 | 'https://bugs.launchpad.net/charms/' | ||
379 | 674 | '+source/name1', | ||
380 | 675 | 'revisions': [{ | ||
381 | 676 | 'revno': 37, | ||
382 | 677 | 'message': 'Hello', | ||
383 | 678 | 'date': '1999-12-31T00:00:00Z', | ||
384 | 679 | 'authors': [ | ||
385 | 680 | { | ||
386 | 681 | 'name': 'Foo', | ||
387 | 682 | 'email': 'foo@example.com' | ||
388 | 683 | }, | ||
389 | 684 | { | ||
390 | 685 | 'name': 'Bar', | ||
391 | 686 | 'email': 'bar@example.com' | ||
392 | 687 | }, | ||
393 | 688 | ], | ||
394 | 689 | }], | ||
395 | 690 | }, | ||
396 | 691 | 'relations': { | ||
397 | 692 | 'provides': { | ||
398 | 693 | 'website': { | ||
399 | 694 | 'interface': 'https', | ||
400 | 695 | }, | ||
401 | 696 | }, | ||
402 | 697 | 'requires': { | ||
403 | 698 | 'database': { | ||
404 | 699 | 'interface': 'mongodb', | ||
405 | 700 | }, | ||
406 | 701 | }, | ||
407 | 702 | }, | ||
408 | 703 | 'options': { | ||
409 | 704 | 'script-interval': { | ||
410 | 705 | 'type': 'int', | ||
411 | 706 | 'default': 5, | ||
412 | 707 | 'description': | ||
413 | 708 | 'The interval between script runs.' | ||
414 | 709 | }, | ||
415 | 710 | }, | ||
416 | 711 | 'files': ['README', 'hooks/install'], | ||
417 | 712 | 'tested_providers': {}, | ||
418 | 713 | 'downloads_in_past_30_days': 0, | ||
419 | 714 | 'is_subordinate': False, | ||
420 | 715 | }, result[0]) | ||
421 | 716 | two_url = make_store_url(1, get_address(two, short=False)) | ||
422 | 717 | self.assertEqual({'owner': 'jrandom', | ||
423 | 718 | 'maintainer': { | ||
424 | 719 | 'name': 'J. Random Hacker', | ||
425 | 720 | 'email': 'jrandom@example.com', | ||
426 | 721 | }, | ||
427 | 722 | 'id': two_url[3:], | ||
428 | 723 | 'categories': [], | ||
429 | 724 | 'date_created': '1996-09-28T11:09:08', | ||
430 | 725 | 'name': 'name2', | ||
431 | 726 | 'url': two_url, | ||
432 | 727 | 'description': two['description'], | ||
433 | 728 | 'revision': 3, | ||
434 | 729 | 'summary': 'Charm two', | ||
435 | 730 | 'distro_series': 'warty', | ||
436 | 731 | 'is_approved': False, | ||
437 | 732 | 'rating_numerator': 0, | ||
438 | 733 | 'rating_denominator': 0, | ||
439 | 734 | 'code_source': { | ||
440 | 735 | 'type': 'bzr', | ||
441 | 736 | 'location': | ||
442 | 737 | 'lp:~jrandom/charms/warty/name2/trunk', | ||
443 | 738 | 'revision': '13', | ||
444 | 739 | 'revisions': [{ | ||
445 | 740 | 'revno': 32, | ||
446 | 741 | 'message': 'Hi', | ||
447 | 742 | 'date': '1998-11-30T00:00:00Z', | ||
448 | 743 | 'authors': [ | ||
449 | 744 | { | ||
450 | 745 | 'name': 'Baz', | ||
451 | 746 | 'email': 'baz@example.org' | ||
452 | 747 | }, | ||
453 | 748 | ], | ||
454 | 749 | }], | ||
455 | 750 | 'last_log': 'message2', | ||
456 | 751 | 'bugs_link': | ||
457 | 752 | 'https://bugs.launchpad.net/charms/+source/name2' | ||
458 | 753 | }, | ||
459 | 754 | 'relations': { | ||
460 | 755 | 'provides': provides2, | ||
461 | 756 | 'requires': requires2, | ||
462 | 757 | |||
463 | 758 | }, | ||
464 | 759 | 'options': options2, | ||
465 | 760 | 'files': ['README.md', 'hooks/config-changed'], | ||
466 | 761 | 'tested_providers': {}, | ||
467 | 762 | 'downloads_in_past_30_days': 0, | ||
468 | 763 | 'is_subordinate': True, | ||
469 | 764 | }, result[1]) | ||
470 | 765 | self.assertNotEqual(one_id, two_id) | ||
471 | 766 | self.assertNotEqual(one_url, two_url) | ||
472 | 767 | |||
473 | 768 | def test_charms_accepts_category(self): | ||
474 | 769 | """Category is accepted (but ignored).""" | ||
475 | 770 | response = self.get_response('charms', category='asdf') | ||
476 | 771 | self.assertEqual(200, response.status_code) | ||
479 | 772 | 580 | ||
480 | 773 | 581 | ||
481 | 774 | class TestAPI: | 582 | class TestAPI: |
482 | @@ -1151,9 +959,6 @@ | |||
483 | 1151 | # Only the headers matter for a CORS preflight response. | 959 | # Only the headers matter for a CORS preflight response. |
484 | 1152 | self.assertEqual(0, response.content_length) | 960 | self.assertEqual(0, response.content_length) |
485 | 1153 | 961 | ||
486 | 1154 | |||
487 | 1155 | class TestAPI1(APITestBase, TestAPI, API1Mixin): | ||
488 | 1156 | |||
489 | 1157 | def test_charm_result_includes_charm_and_metadata(self): | 962 | def test_charm_result_includes_charm_and_metadata(self): |
490 | 1158 | charm = factory.get_charm_json(promulgated=False) | 963 | charm = factory.get_charm_json(promulgated=False) |
491 | 1159 | result = self.api_class._charm_result(charm, {}, {}) | 964 | result = self.api_class._charm_result(charm, {}, {}) |
492 | @@ -1283,29 +1088,5 @@ | |||
493 | 1283 | self.assertNotIn('icon', formatted) | 1088 | self.assertNotIn('icon', formatted) |
494 | 1284 | 1089 | ||
495 | 1285 | 1090 | ||
522 | 1286 | class TestAPI0(APITestBase, TestAPI): | 1091 | class TestAPI1(TestAPI, API1Mixin): |
523 | 1287 | 1092 | """Test API 1.""" | |
498 | 1288 | api_class = API0 | ||
499 | 1289 | |||
500 | 1290 | @staticmethod | ||
501 | 1291 | def get_id(charm): | ||
502 | 1292 | return charm['id'] | ||
503 | 1293 | |||
504 | 1294 | def test_charm_result_excludes_charm_and_metadata(self): | ||
505 | 1295 | charm = factory.get_charm_json(promulgated=False) | ||
506 | 1296 | result = self.api_class._charm_result(charm, {}, {}) | ||
507 | 1297 | self.assertNotIn('metadata', result) | ||
508 | 1298 | self.assertNotIn('charm', result) | ||
509 | 1299 | |||
510 | 1300 | def test_format_charm_adds_icon(self): | ||
511 | 1301 | """If a charm provides an icon, it is added to the data returned | ||
512 | 1302 | by the API. | ||
513 | 1303 | """ | ||
514 | 1304 | charmid, charm = factory.makeCharm(self.db) | ||
515 | 1305 | # The test data does not contain an icon. | ||
516 | 1306 | self.assertFalse('icon' in charm) | ||
517 | 1307 | formatted = self.api_class._format_charm(charm) | ||
518 | 1308 | self.assertFalse('icon' in formatted) | ||
519 | 1309 | charm['icon'] = 'fake' | ||
520 | 1310 | formatted = self.api_class._format_charm(charm) | ||
521 | 1311 | self.assertTrue('icon' in formatted) | ||
524 | 1312 | 1093 | ||
525 | === modified file 'migrations/migrate.py' | |||
526 | --- migrations/migrate.py 2013-02-12 11:36:14 +0000 | |||
527 | +++ migrations/migrate.py 2013-06-04 16:12:32 +0000 | |||
528 | @@ -17,6 +17,7 @@ | |||
529 | 17 | 17 | ||
530 | 18 | from charmworld.models import getconnection | 18 | from charmworld.models import getconnection |
531 | 19 | from charmworld.models import getdb | 19 | from charmworld.models import getdb |
532 | 20 | from charmworld.search import ElasticSearchClient | ||
533 | 20 | from charmworld.utils import get_ini | 21 | from charmworld.utils import get_ini |
534 | 21 | 22 | ||
535 | 22 | SCRIPT_TEMPLATE = """ | 23 | SCRIPT_TEMPLATE = """ |
536 | @@ -152,7 +153,13 @@ | |||
537 | 152 | """:returns: Latest version in Collection""" | 153 | """:returns: Latest version in Collection""" |
538 | 153 | return self.version_indexes[-1] if len(self.version_indexes) > 0 else 0 | 154 | return self.version_indexes[-1] if len(self.version_indexes) > 0 else 0 |
539 | 154 | 155 | ||
541 | 155 | def upgrade(self, datastore, init): | 156 | def run_migration(self, db, index_client, module_name): |
542 | 157 | module = imp.load_source( | ||
543 | 158 | module_name.strip('.py'), | ||
544 | 159 | join(self.versions_dir, module_name)) | ||
545 | 160 | getattr(module, 'upgrade')(db, index_client) | ||
546 | 161 | |||
547 | 162 | def upgrade(self, datastore, index_client, init): | ||
548 | 156 | """Run `upgrade` methods for required version files. | 163 | """Run `upgrade` methods for required version files. |
549 | 157 | 164 | ||
550 | 158 | :param datastore: An instance of DataStore | 165 | :param datastore: An instance of DataStore |
551 | @@ -176,10 +183,7 @@ | |||
552 | 176 | # Let's get processing. | 183 | # Let's get processing. |
553 | 177 | next_version = current_version + 1 | 184 | next_version = current_version + 1 |
554 | 178 | module_name = self.versions[next_version] | 185 | module_name = self.versions[next_version] |
559 | 179 | module = imp.load_source( | 186 | self.run_migration(datastore.db, index_client, module_name) |
556 | 180 | module_name.strip('.py'), | ||
557 | 181 | join(self.versions_dir, module_name)) | ||
558 | 182 | getattr(module, 'upgrade')(datastore.db) | ||
560 | 183 | current_version = next_version | 187 | current_version = next_version |
561 | 184 | datastore.update_version(current_version) | 188 | datastore.update_version(current_version) |
562 | 185 | return current_version | 189 | return current_version |
563 | @@ -252,8 +256,9 @@ | |||
564 | 252 | def upgrade(cls, ini, args): | 256 | def upgrade(cls, ini, args): |
565 | 253 | """Upgrade the data store to the latest available migration.""" | 257 | """Upgrade the data store to the latest available migration.""" |
566 | 254 | ds = cls.get_datastore(ini) | 258 | ds = cls.get_datastore(ini) |
567 | 259 | index_client = ElasticSearchClient.from_settings(ini) | ||
568 | 255 | migrations = Versions(ini['migrations']) | 260 | migrations = Versions(ini['migrations']) |
570 | 256 | new_version = migrations.upgrade(ds, args.init) | 261 | new_version = migrations.upgrade(ds, index_client, args.init) |
571 | 257 | 262 | ||
572 | 258 | if new_version is None: | 263 | if new_version is None: |
573 | 259 | print 'There are no new migrations to apply' | 264 | print 'There are no new migrations to apply' |
574 | 260 | 265 | ||
575 | === modified file 'migrations/test_migrate.py' | |||
576 | --- migrations/test_migrate.py 2013-02-12 18:57:02 +0000 | |||
577 | +++ migrations/test_migrate.py 2013-06-04 16:12:32 +0000 | |||
578 | @@ -6,10 +6,10 @@ | |||
579 | 6 | from os.path import exists | 6 | from os.path import exists |
580 | 7 | import pymongo | 7 | import pymongo |
581 | 8 | from tempfile import mkdtemp | 8 | from tempfile import mkdtemp |
582 | 9 | from unittest import TestCase | ||
583 | 10 | 9 | ||
584 | 11 | from charmworld.models import getconnection | 10 | from charmworld.models import getconnection |
585 | 12 | from charmworld.models import getdb | 11 | from charmworld.models import getdb |
586 | 12 | from charmworld.testing import TestCase | ||
587 | 13 | from charmworld.testing.factory import random_string | 13 | from charmworld.testing.factory import random_string |
588 | 14 | from charmworld.utils import get_ini | 14 | from charmworld.utils import get_ini |
589 | 15 | 15 | ||
590 | @@ -155,16 +155,19 @@ | |||
591 | 155 | 155 | ||
592 | 156 | def test_upgrade_wo_migrations(self): | 156 | def test_upgrade_wo_migrations(self): |
593 | 157 | """Upgrade runs silently if no migrations are created yet.""" | 157 | """Upgrade runs silently if no migrations are created yet.""" |
594 | 158 | self.use_index_client() | ||
595 | 158 | self.ds.version_datastore() | 159 | self.ds.version_datastore() |
596 | 159 | tmpdir = mkdtemp() | 160 | tmpdir = mkdtemp() |
597 | 160 | versions = Versions(tmpdir) | 161 | versions = Versions(tmpdir) |
599 | 161 | self.assertEqual(versions.upgrade(self.ds, False), None) | 162 | self.assertEqual(versions.upgrade(self.ds, self.index_client, False), |
600 | 163 | None) | ||
601 | 162 | 164 | ||
602 | 163 | def test_upgrade_succeeds(self): | 165 | def test_upgrade_succeeds(self): |
603 | 164 | """upgrade will run the methods and we get a version of 4.""" | 166 | """upgrade will run the methods and we get a version of 4.""" |
604 | 165 | self.ds.version_datastore() | 167 | self.ds.version_datastore() |
605 | 168 | self.use_index_client() | ||
606 | 166 | # We should get 4 versions done. The upgrade returns 4 | 169 | # We should get 4 versions done. The upgrade returns 4 |
608 | 167 | last_version = self.version.upgrade(self.ds, False) | 170 | last_version = self.version.upgrade(self.ds, self.index_client, False) |
609 | 168 | self.assertEqual(4, last_version) | 171 | self.assertEqual(4, last_version) |
610 | 169 | 172 | ||
611 | 170 | # and we can query the db for the revision of 4 | 173 | # and we can query the db for the revision of 4 |
612 | @@ -172,7 +175,8 @@ | |||
613 | 172 | 175 | ||
614 | 173 | def test_upgrade_succeeds_from_unversioned(self): | 176 | def test_upgrade_succeeds_from_unversioned(self): |
615 | 174 | """Forcing init no an unversioned db will upgrade successfully.""" | 177 | """Forcing init no an unversioned db will upgrade successfully.""" |
617 | 175 | last_version = self.version.upgrade(self.ds, True) | 178 | self.use_index_client() |
618 | 179 | last_version = self.version.upgrade(self.ds, self.index_client, True) | ||
619 | 176 | self.assertEqual(4, last_version) | 180 | self.assertEqual(4, last_version) |
620 | 177 | 181 | ||
621 | 178 | # and we can query the db for the revision of 4 | 182 | # and we can query the db for the revision of 4 |
622 | 179 | 183 | ||
623 | === modified file 'migrations/versions/001_load_qa_questions.py' | |||
624 | --- migrations/versions/001_load_qa_questions.py 2013-02-12 11:36:14 +0000 | |||
625 | +++ migrations/versions/001_load_qa_questions.py 2013-06-04 16:12:32 +0000 | |||
626 | @@ -9,7 +9,7 @@ | |||
627 | 9 | """ | 9 | """ |
628 | 10 | 10 | ||
629 | 11 | 11 | ||
631 | 12 | def upgrade(db): | 12 | def upgrade(db, index_client): |
632 | 13 | """Complete this function with work to be done for the migration/update. | 13 | """Complete this function with work to be done for the migration/update. |
633 | 14 | 14 | ||
634 | 15 | db is the pymongo db instance for our datastore. Charms are in db.charms | 15 | db is the pymongo db instance for our datastore. Charms are in db.charms |
635 | 16 | 16 | ||
636 | === modified file 'migrations/versions/002_Escape_dots_dollar_signs_percent_signs_in_mongo_keys.py' | |||
637 | --- migrations/versions/002_Escape_dots_dollar_signs_percent_signs_in_mongo_keys.py 2013-02-12 11:36:14 +0000 | |||
638 | +++ migrations/versions/002_Escape_dots_dollar_signs_percent_signs_in_mongo_keys.py 2013-06-04 16:12:32 +0000 | |||
639 | @@ -5,7 +5,7 @@ | |||
640 | 5 | 5 | ||
641 | 6 | from charmworld.utils import quote_yaml | 6 | from charmworld.utils import quote_yaml |
642 | 7 | 7 | ||
644 | 8 | def upgrade(db): | 8 | def upgrade(db, index_client): |
645 | 9 | """Iterate all charm data and replace all dictionary keys containing | 9 | """Iterate all charm data and replace all dictionary keys containing |
646 | 10 | a '.', '$' or '%' with an escaped variant. | 10 | a '.', '$' or '%' with an escaped variant. |
647 | 11 | """ | 11 | """ |
648 | 12 | 12 | ||
649 | === modified file 'migrations/versions/003_remove_provider_qa_questions.py' | |||
650 | --- migrations/versions/003_remove_provider_qa_questions.py 2013-04-23 19:23:17 +0000 | |||
651 | +++ migrations/versions/003_remove_provider_qa_questions.py 2013-06-04 16:12:32 +0000 | |||
652 | @@ -21,7 +21,7 @@ | |||
653 | 21 | ] | 21 | ] |
654 | 22 | 22 | ||
655 | 23 | 23 | ||
657 | 24 | def upgrade(db): | 24 | def upgrade(db, index_client): |
658 | 25 | """Update each category of questions from new_category_data. | 25 | """Update each category of questions from new_category_data. |
659 | 26 | 26 | ||
660 | 27 | This update elaborates on the 001_* migration. Subsequent qa form | 27 | This update elaborates on the 001_* migration. Subsequent qa form |
661 | 28 | 28 | ||
662 | === modified file 'migrations/versions/004_remove_charm_errors.py' | |||
663 | --- migrations/versions/004_remove_charm_errors.py 2013-05-27 19:02:19 +0000 | |||
664 | +++ migrations/versions/004_remove_charm_errors.py 2013-06-04 16:12:32 +0000 | |||
665 | @@ -1,2 +1,4 @@ | |||
667 | 1 | def upgrade(db): | 1 | # Copyright 2013 Canonical Ltd. This software is licensed under the |
668 | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). | ||
669 | 3 | def upgrade(db, index_client): | ||
670 | 2 | db.drop_collection('charm-errors') | 4 | db.drop_collection('charm-errors') |
671 | 3 | 5 | ||
672 | === added file 'migrations/versions/005_delete_icon_field.py' | |||
673 | --- migrations/versions/005_delete_icon_field.py 1970-01-01 00:00:00 +0000 | |||
674 | +++ migrations/versions/005_delete_icon_field.py 2013-06-04 16:12:32 +0000 | |||
675 | @@ -0,0 +1,8 @@ | |||
676 | 1 | # Copyright 2013 Canonical Ltd. This software is licensed under the | ||
677 | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). | ||
678 | 3 | from charmworld.search import reindex | ||
679 | 4 | |||
680 | 5 | |||
681 | 6 | def upgrade(db, index_client): | ||
682 | 7 | db.charms.update({}, {'$unset': {'icon':''}}) | ||
683 | 8 | reindex(index_client, list(db.charms.find({}))) | ||
684 | 0 | 9 | ||
685 | === modified file 'migrations/versions/tests/test_migrations.py' | |||
686 | --- migrations/versions/tests/test_migrations.py 2013-05-27 19:02:19 +0000 | |||
687 | +++ migrations/versions/tests/test_migrations.py 2013-06-04 16:12:32 +0000 | |||
688 | @@ -2,8 +2,15 @@ | |||
689 | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). |
690 | 3 | 3 | ||
691 | 4 | import imp | 4 | import imp |
692 | 5 | from charmworld.models import CharmSource | ||
693 | 5 | from charmworld.testing import MongoTestBase | 6 | from charmworld.testing import MongoTestBase |
694 | 6 | from charmworld.testing.factory import random_string | 7 | from charmworld.testing.factory import random_string |
695 | 8 | from migrations.migrate import Versions | ||
696 | 9 | |||
697 | 10 | |||
698 | 11 | def run_migration(db, index_client, module_name): | ||
699 | 12 | versions = Versions('migrations/versions/') | ||
700 | 13 | versions.run_migration(db, index_client, module_name) | ||
701 | 7 | 14 | ||
702 | 8 | 15 | ||
703 | 9 | class TestMigration002(MongoTestBase): | 16 | class TestMigration002(MongoTestBase): |
704 | @@ -53,10 +60,18 @@ | |||
705 | 53 | class TestMigration004(MongoTestBase): | 60 | class TestMigration004(MongoTestBase): |
706 | 54 | 61 | ||
707 | 55 | def test_migration(self): | 62 | def test_migration(self): |
708 | 63 | self.use_index_client() | ||
709 | 56 | self.db.create_collection('charm-errors') | 64 | self.db.create_collection('charm-errors') |
715 | 57 | module = imp.load_source( | 65 | run_migration(self.db, self.index_client, '004_remove_charm_errors.py') |
711 | 58 | '004_remove_charm_errors', | ||
712 | 59 | 'migrations/versions/' | ||
713 | 60 | '004_remove_charm_errors.py') | ||
714 | 61 | module.upgrade(self.db) | ||
716 | 62 | self.assertNotIn('charm-errors', self.db.collection_names()) | 66 | self.assertNotIn('charm-errors', self.db.collection_names()) |
717 | 67 | |||
718 | 68 | |||
719 | 69 | class TestMigration005(MongoTestBase): | ||
720 | 70 | |||
721 | 71 | def test_migration(self): | ||
722 | 72 | self.use_index_client() | ||
723 | 73 | source = CharmSource.from_request(self) | ||
724 | 74 | source.save({'_id': 'a', 'icon': 'asdf', 'asdf': 'asdf'}) | ||
725 | 75 | run_migration(self.db, self.index_client, '005_delete_icon_field.py') | ||
726 | 76 | for charm in source._get_all('a'): | ||
727 | 77 | self.assertNotIn('icon', charm) |
Thank you.