Merge lp:~abentley/charmworld/related-charms into lp:~juju-jitsu/charmworld/trunk
- related-charms
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Diogo Matsubara |
Approved revision: | 241 |
Merged at revision: | 226 |
Proposed branch: | lp:~abentley/charmworld/related-charms |
Merge into: | lp:~juju-jitsu/charmworld/trunk |
Diff against target: |
1038 lines (+578/-220) 5 files modified
charmworld/search.py (+34/-6) charmworld/tests/test_models.py (+1/-1) charmworld/tests/test_search.py (+54/-0) charmworld/views/api.py (+64/-10) charmworld/views/tests/test_api.py (+425/-203) |
To merge this branch: | bzr merge lp:~abentley/charmworld/related-charms |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Curtis Hovey (community) | code | Approve | |
Review via email: mp+164223@code.launchpad.net |
Commit message
Implement related charms in API1.
Description of the change
This branch adds related charms to the data returned by the charms, charms/interesting and charm endpoints in API 1.
The data includes the charms and their weight or 'score' in ElasticSearch, so that they can be sorted externally. Score includes consideration of whether they are official charms.
The data also include icon information. That can be removed as part of "Remove charm icon SVGs from API response"
ElasticSearch.
All the endpoints are updated to use _charm_results. For the charms/interesting endpoint, this means ElasticSearch.
Most of the line count comes from reorganizing the tests. I'm not especially happy with the triple-inheritance this provoked, but I'll leave it for now.
Curtis Hovey (sinzui) wrote : | # |
We spoke on IRC and we know there are many similar apache charms that could match. This is not ideal. Excluding exact match on name seems sensible. We will explore this issue in a follow up branch. This branch is fine to land.
Charmworld Lander (charmworld-lander) wrote : | # |
The attempt to merge lp:~abentley/charmworld/related-charms into lp:charmworld failed. Below is the output from the failed tests.
/home/charmworl
Preview Diff
1 | === modified file 'charmworld/search.py' |
2 | --- charmworld/search.py 2013-05-03 19:10:32 +0000 |
3 | +++ charmworld/search.py 2013-05-16 15:59:26 +0000 |
4 | @@ -57,6 +57,13 @@ |
5 | ] |
6 | } |
7 | |
8 | + valid_charms_clauses = [ |
9 | + {'missing': {'field': 'store_data.errors'}}, |
10 | + # 'missing' ignores objects/dicts, but 'not': {'exists'} |
11 | + # considers them. |
12 | + {'not': {'exists': {'field': 'error'}}} |
13 | + ] |
14 | + |
15 | def __init__(self, client, index_name): |
16 | self._client = client |
17 | self.index_name = index_name |
18 | @@ -173,12 +180,7 @@ |
19 | ] |
20 | } |
21 | if valid_charms_only: |
22 | - and_clause['and'].extend( |
23 | - [{'missing': {'field': 'store_data.errors'}}, |
24 | - # 'missing' ignores objects/dicts, but 'not': {'exists'} |
25 | - # considers them. |
26 | - {'not': {'exists': {'field': 'error'}}}] |
27 | - ) |
28 | + and_clause['and'].extend(cls.valid_charms_clauses) |
29 | if type_ is not None: |
30 | typeclauses = [] |
31 | for value in type_: |
32 | @@ -301,6 +303,32 @@ |
33 | 'matches_human_readable_estimate': '%d' % len(charms), |
34 | } |
35 | |
36 | + def related_charms(self, requires, provides): |
37 | + and_clauses = [{'or': [ |
38 | + {'terms': {'i_provides': provides}}, |
39 | + {'terms': {'i_requires': requires}}, |
40 | + ]}] |
41 | + and_clauses.extend(self.valid_charms_clauses) |
42 | + dsl = { |
43 | + 'filtered': { |
44 | + 'filter': { |
45 | + 'and': and_clauses |
46 | + } |
47 | + } |
48 | + } |
49 | + dsl = {'query': self._get_official_boost(dsl)} |
50 | + hits = self._unlimited_search(dsl, None) |
51 | + result_p = {} |
52 | + result_r = {} |
53 | + for hit in hits['hits']['hits']: |
54 | + charm = hit['_source'] |
55 | + payload = {'data': charm, 'weight': hit['_score']} |
56 | + for interface in provides.intersection(charm['i_provides']): |
57 | + result_p.setdefault(interface, []).append(payload) |
58 | + for interface in requires.intersection(charm['i_requires']): |
59 | + result_r.setdefault(interface, []).append(payload) |
60 | + return result_r, result_p, |
61 | + |
62 | def get_aliasable_client(self, name=None): |
63 | """Return a client that can be aliased to this one. |
64 | |
65 | |
66 | === renamed file 'charmworld/test_models.py' => 'charmworld/tests/test_models.py' |
67 | --- charmworld/test_models.py 2013-05-03 15:47:38 +0000 |
68 | +++ charmworld/tests/test_models.py 2013-05-16 15:59:26 +0000 |
69 | @@ -193,7 +193,7 @@ |
70 | 'bname': 'trunk', |
71 | } |
72 | charm_path = join( |
73 | - dirname(__file__), |
74 | + dirname(dirname(__file__)), |
75 | 'testing/data/sample_charm' |
76 | ) |
77 | |
78 | |
79 | === modified file 'charmworld/tests/test_search.py' |
80 | --- charmworld/tests/test_search.py 2013-05-03 19:04:04 +0000 |
81 | +++ charmworld/tests/test_search.py 2013-05-16 15:59:26 +0000 |
82 | @@ -456,6 +456,60 @@ |
83 | with self.assertRaises(IndexMissing): |
84 | client.put_mapping() |
85 | |
86 | + def test_related_charms(self): |
87 | + client = self.index_client |
88 | + charm = factory.get_charm_json(provides={'abc': {'interface': 'def'}}) |
89 | + client.index_charm(charm) |
90 | + requires, provides = client.related_charms( |
91 | + set(charm['i_provides']), set(charm['i_requires'])) |
92 | + self.assertEqual({}, provides) |
93 | + self.assertEqual({}, requires) |
94 | + requires, provides = client.related_charms(set(['def']), set(['def'])) |
95 | + self.assertEqual(['def'], provides.keys()) |
96 | + self.assertEqual(charm, provides['def'][0]['data']) |
97 | + self.assertIs(float, type(provides['def'][0]['weight'])) |
98 | + self.assertEqual(2, len(provides['def'][0])) |
99 | + self.assertEqual({}, requires) |
100 | + charm2 = factory.get_charm_json( |
101 | + provides={ |
102 | + 'abc': {'interface': 'def'}, |
103 | + 'mno': {'interface': 'pqr'}}, |
104 | + requires={'ghi': {'interface': 'jkl'}}) |
105 | + client.index_charm(charm2) |
106 | + requires, provides = client.related_charms(set(['jkl']), set()) |
107 | + self.assertEqual({}, provides) |
108 | + self.assertEqual(['jkl'], requires.keys()) |
109 | + self.assertEqual(charm2, requires['jkl'][0]['data']) |
110 | + requires, provides = client.related_charms(set(['jkl']), set(['def'])) |
111 | + self.assertEqual(['def'], provides.keys()) |
112 | + self.assertItemsEqual([charm, charm2], |
113 | + [payload['data'] for payload in provides['def']]) |
114 | + self.assertEqual([charm2], |
115 | + [payload['data'] for payload in requires['jkl']]) |
116 | + requires, provides = client.related_charms(set(), |
117 | + set(['def', 'pqr', 'xyz'])) |
118 | + self.assertItemsEqual(['def', 'pqr'], provides.keys()) |
119 | + self.assertEqual([charm2], [payload['data'] |
120 | + for payload in provides['pqr']]) |
121 | + self.assertItemsEqual([charm, charm2], |
122 | + [payload['data'] for payload in provides['def']]) |
123 | + |
124 | + def test_related_charms_official_boost(self): |
125 | + """An official version of a charm rates 10x higher.""" |
126 | + charm = factory.get_charm_json( |
127 | + provides={'foo': {'interface': 'telnet'}}) |
128 | + official_id = charm['_id'] |
129 | + unofficial_id = official_id + '-1' |
130 | + client = self.index_client |
131 | + client.index_charm(charm) |
132 | + charm['owner'] = 'jrandom' |
133 | + charm['_id'] = unofficial_id |
134 | + client.index_charm(charm) |
135 | + requires, provides = client.related_charms(set(), set(['telnet'])) |
136 | + by_id = dict((result['data']['_id'], result['weight']) |
137 | + for result in provides['telnet']) |
138 | + self.assertAlmostEqual(by_id[official_id], by_id[unofficial_id] * 10) |
139 | + |
140 | |
141 | def put_mapping(client, properties): |
142 | client._client.put_mapping( |
143 | |
144 | === modified file 'charmworld/views/api.py' |
145 | --- charmworld/views/api.py 2013-05-03 19:02:38 +0000 |
146 | +++ charmworld/views/api.py 2013-05-16 15:59:26 +0000 |
147 | @@ -128,8 +128,40 @@ |
148 | } |
149 | |
150 | @classmethod |
151 | - def _charm_result(cls, charm): |
152 | - return {'charm': cls._format_charm(charm)} |
153 | + def _charm_result(cls, charm, requires, provides): |
154 | + charm_provides = set(requires).intersection(charm['i_provides']) |
155 | + charm_requires = set(provides).intersection(charm['i_requires']) |
156 | + related = { |
157 | + 'requires': dict((key, requires[key]) for key in charm_provides), |
158 | + 'provides': dict((key, provides[key]) for key in charm_requires), |
159 | + } |
160 | + return { |
161 | + 'charm': cls._format_charm(charm), |
162 | + 'metadata': {'related': related} |
163 | + } |
164 | + |
165 | + def _related_charms(self, charms): |
166 | + charms_provide = set() |
167 | + charms_require = set() |
168 | + for charm in charms: |
169 | + charms_provide.update(charm['i_provides']) |
170 | + charms_require.update(charm['i_requires']) |
171 | + # Swap provides and requires, so we get the charms which *require* |
172 | + # what these charms provide, and the charms that *provide* what these |
173 | + # charms require. |
174 | + r_requires, r_provides = self.request.index_client.related_charms( |
175 | + charms_provide, charms_require) |
176 | + f_requires = {} |
177 | + for key, value in r_requires.items(): |
178 | + f_requires[key] = [ |
179 | + self._format_related(payload['data'], payload['weight']) |
180 | + for payload in value] |
181 | + f_provides = {} |
182 | + for key, value in r_provides.items(): |
183 | + f_provides[key] = [ |
184 | + self._format_related(payload['data'], payload['weight']) |
185 | + for payload in value] |
186 | + return f_requires, f_provides |
187 | |
188 | @classmethod |
189 | def _format_charm(cls, charm): |
190 | @@ -189,6 +221,21 @@ |
191 | output['icon'] = charm['icon'] |
192 | return output |
193 | |
194 | + @classmethod |
195 | + def _format_related(cls, charm, weight): |
196 | + result = { |
197 | + 'id': cls._get_api_id(charm), |
198 | + 'name': charm['name'], |
199 | + 'categories': charm.get('categories', []), |
200 | + 'weight': weight, |
201 | + } |
202 | + if 'icon' in charm: |
203 | + result['icon'] = { |
204 | + 'body': charm['icon'], |
205 | + 'link': '/api/1/' + result['id'] + '/icon' |
206 | + } |
207 | + return result |
208 | + |
209 | @staticmethod |
210 | def _extract_charm_id(path): |
211 | """Extract the charm_id from a path. |
212 | @@ -254,7 +301,7 @@ |
213 | return json_response( |
214 | 404, {'type': 'no_such_charm', 'charm_id': charm_id}) |
215 | if trailing is None: |
216 | - return self._charm_result(charm) |
217 | + return self._charm_results([charm])[0] |
218 | elif trailing.startswith('file/'): |
219 | return self._charm_file(charm, trailing) |
220 | elif trailing == ('qa'): |
221 | @@ -329,7 +376,12 @@ |
222 | return json_response(406, { |
223 | 'type': 'negative_value', |
224 | 'parameter': 'limit'}) |
225 | - return {'result': [self._charm_result(result) for result in results]} |
226 | + return {'result': self._charm_results(results)} |
227 | + |
228 | + def _charm_results(self, charms): |
229 | + requires, provides = self._related_charms(charms) |
230 | + return [self._charm_result(charm, requires, provides) |
231 | + for charm in charms] |
232 | |
233 | def _interesting_charms(self): |
234 | """Generate a JSON structure of interesting charms. |
235 | @@ -338,15 +390,14 @@ |
236 | """ |
237 | popular = self.request.index_client.api_search( |
238 | sort='downloaded', limit=10) |
239 | - new = [self._charm_result(charm) for charm in |
240 | - self.request.index_client.api_search(sort='new', limit=10)] |
241 | + new = self.request.index_client.api_search(sort='new', limit=10) |
242 | featured = self.request.index_client.api_search( |
243 | filters={'is_featured': [True]}) |
244 | return { |
245 | 'result': { |
246 | - 'new': new, |
247 | - 'featured': [self._charm_result(charm) for charm in featured], |
248 | - 'popular': [self._charm_result(charm) for charm in popular], |
249 | + 'new': self._charm_results(new), |
250 | + 'featured': self._charm_results(featured), |
251 | + 'popular': self._charm_results(popular), |
252 | } |
253 | } |
254 | |
255 | @@ -360,9 +411,12 @@ |
256 | """ |
257 | |
258 | @classmethod |
259 | - def _charm_result(cls, charm): |
260 | + def _charm_result(cls, charm, requires, provides): |
261 | return cls._format_charm(charm) |
262 | |
263 | + def _charm_results(self, charms): |
264 | + return [self._charm_result(charm, {}, {}) for charm in charms] |
265 | + |
266 | def _charms(self, limit=None, name=None, series=None, owner=None, |
267 | provides=None, requires=None, type_=None, provider=None, |
268 | scope=None, category=None, categories=None, text=None): |
269 | |
270 | === modified file 'charmworld/views/tests/test_api.py' |
271 | --- charmworld/views/tests/test_api.py 2013-05-01 19:20:02 +0000 |
272 | +++ charmworld/views/tests/test_api.py 2013-05-16 15:59:26 +0000 |
273 | @@ -1,6 +1,8 @@ |
274 | # Copyright 2012, 2013 Canonical Ltd. This software is licensed under the |
275 | # GNU Affero General Public License version 3 (see the file LICENSE). |
276 | |
277 | +__metaclass__ = type |
278 | + |
279 | from datetime import datetime |
280 | import hashlib |
281 | |
282 | @@ -11,6 +13,7 @@ |
283 | get_address, |
284 | make_store_url, |
285 | ) |
286 | +from charmworld.models import CharmSource |
287 | from charmworld.views.api import ( |
288 | API0, |
289 | API1, |
290 | @@ -70,12 +73,10 @@ |
291 | return self.api_class(request)() |
292 | |
293 | |
294 | -class TestAPI1Charms(APITestBase): |
295 | - |
296 | - api_class = API1 |
297 | +class TestAPICharms: |
298 | |
299 | def setUp(self): |
300 | - super(ViewTestBase, self).setUp() |
301 | + super(TestAPICharms, self).setUp() |
302 | self.use_index_client() |
303 | |
304 | def makeCharm(self, *args, **kwargs): |
305 | @@ -92,179 +93,6 @@ |
306 | ("Access-Control-Allow-Headers", "X-Requested-With"), |
307 | resp.headerlist) |
308 | |
309 | - def test_charms(self): |
310 | - """Charms endpoint produces expected result.""" |
311 | - one_changes = [factory.makeChange(created=datetime(1999, 12, 31), |
312 | - revno=37, message='Hello')] |
313 | - one_id, one = self.makeCharm(name='name1', changes=one_changes, |
314 | - date_created=datetime(1997, 10, 29, |
315 | - 12, 10, 9)) |
316 | - self.index_client.index_charm(one) |
317 | - maintainer2 = 'jrandom@example.com (J. Random Hacker)' |
318 | - provides2 = {'cookie-factory': { |
319 | - 'interface': 'imap', |
320 | - }} |
321 | - requires2 = {'postal-service': { |
322 | - 'interface': 'envelope', |
323 | - }} |
324 | - options2 = { |
325 | - 'foobar': { |
326 | - 'type': 'str', |
327 | - 'default': 'baz', |
328 | - 'description': 'quxx', |
329 | - }, |
330 | - } |
331 | - files2 = { |
332 | - 'readme': { |
333 | - 'filename': 'README.md', |
334 | - 'subdir': '', |
335 | - }, |
336 | - 'config-changed': { |
337 | - 'filename': 'config-changed', |
338 | - 'subdir': 'hooks', |
339 | - }, |
340 | - } |
341 | - two_changes = [factory.makeChange(created=datetime(1998, 11, 30), |
342 | - revno=32, message='Hi', |
343 | - authors=['Baz <baz@example.org>'])] |
344 | - two_id, two = self.makeCharm(owner='jrandom', revision=3, |
345 | - series='warty', summary='Charm two', |
346 | - name='name2', revno=13, |
347 | - maintainer=maintainer2, |
348 | - commit_message='message2', |
349 | - provides=provides2, |
350 | - requires=requires2, options=options2, |
351 | - files=files2, subordinate=True, |
352 | - changes=two_changes, |
353 | - date_created=datetime(1996, 9, 28, |
354 | - 11, 9, 8)) |
355 | - response = self.get_response('charms') |
356 | - self.assertEqual(200, response.status_code) |
357 | - self.assertEqual('application/json', response.content_type) |
358 | - result = response.json_body['result'] |
359 | - self.assertEqual(2, len(result)) |
360 | - one_url = make_store_url(1, get_address(one, short=True)) |
361 | - self.assertEqual({ |
362 | - 'charm': { |
363 | - 'owner': 'charmers', |
364 | - 'maintainer': { |
365 | - 'name': 'John Doe', |
366 | - 'email': 'jdoe@example.com', |
367 | - }, |
368 | - 'id': one_url[3:], |
369 | - 'date_created': '1997-10-29T12:10:09', |
370 | - 'name': 'name1', |
371 | - 'url': one_url, |
372 | - 'description': one['description'], |
373 | - 'revision': 1, |
374 | - 'summary': "Remote terminal classroom over https", |
375 | - 'distro_series': 'precise', |
376 | - 'is_approved': True, |
377 | - 'rating_numerator': 0, |
378 | - 'rating_denominator': 0, |
379 | - 'code_source': { |
380 | - 'type': 'bzr', |
381 | - 'location': |
382 | - 'lp:~charmers/charms/precise/name1/trunk', |
383 | - 'revision': '12', |
384 | - 'last_log': 'maintainer', |
385 | - 'bugs_link': |
386 | - 'https://bugs.launchpad.net/charms/' |
387 | - '+source/name1', |
388 | - 'revisions': [{ |
389 | - 'revno': 37, |
390 | - 'message': 'Hello', |
391 | - 'date': '1999-12-31T00:00:00Z', |
392 | - 'authors': [ |
393 | - { |
394 | - 'name': 'Foo', |
395 | - 'email': 'foo@example.com' |
396 | - }, |
397 | - { |
398 | - 'name': 'Bar', |
399 | - 'email': 'bar@example.com' |
400 | - }, |
401 | - ], |
402 | - }], |
403 | - }, |
404 | - 'relations': { |
405 | - 'provides': { |
406 | - 'website': { |
407 | - 'interface': 'https', |
408 | - }, |
409 | - }, |
410 | - 'requires': { |
411 | - 'database': { |
412 | - 'interface': 'mongodb', |
413 | - }, |
414 | - }, |
415 | - }, |
416 | - 'options': { |
417 | - 'script-interval': { |
418 | - 'type': 'int', |
419 | - 'default': 5, |
420 | - 'description': |
421 | - 'The interval between script runs.' |
422 | - }, |
423 | - }, |
424 | - 'files': ['README', 'hooks/install'], |
425 | - 'tested_providers': {}, |
426 | - 'downloads_in_past_30_days': 0, |
427 | - 'is_subordinate': False, |
428 | - }}, result[0]) |
429 | - two_url = make_store_url(1, get_address(two, short=False)) |
430 | - self.assertEqual({ |
431 | - 'charm': { |
432 | - 'owner': 'jrandom', |
433 | - 'maintainer': { |
434 | - 'name': 'J. Random Hacker', |
435 | - 'email': 'jrandom@example.com', |
436 | - }, |
437 | - 'id': two_url[3:], |
438 | - 'date_created': '1996-09-28T11:09:08', |
439 | - 'name': 'name2', |
440 | - 'url': two_url, |
441 | - 'description': two['description'], |
442 | - 'revision': 3, |
443 | - 'summary': 'Charm two', |
444 | - 'distro_series': 'warty', |
445 | - 'is_approved': False, |
446 | - 'rating_numerator': 0, |
447 | - 'rating_denominator': 0, |
448 | - 'code_source': { |
449 | - 'type': 'bzr', |
450 | - 'location': |
451 | - 'lp:~jrandom/charms/warty/name2/trunk', |
452 | - 'revision': '13', |
453 | - 'revisions': [{ |
454 | - 'revno': 32, |
455 | - 'message': 'Hi', |
456 | - 'date': '1998-11-30T00:00:00Z', |
457 | - 'authors': [ |
458 | - { |
459 | - 'name': 'Baz', |
460 | - 'email': 'baz@example.org' |
461 | - }, |
462 | - ], |
463 | - }], |
464 | - 'last_log': 'message2', |
465 | - 'bugs_link': |
466 | - 'https://bugs.launchpad.net/charms/+source/name2' |
467 | - }, |
468 | - 'relations': { |
469 | - 'provides': provides2, |
470 | - 'requires': requires2, |
471 | - |
472 | - }, |
473 | - 'options': options2, |
474 | - 'files': ['README.md', 'hooks/config-changed'], |
475 | - 'tested_providers': {}, |
476 | - 'downloads_in_past_30_days': 0, |
477 | - 'is_subordinate': True, |
478 | - }}, result[1]) |
479 | - self.assertNotEqual(one_id, two_id) |
480 | - self.assertNotEqual(one_url, two_url) |
481 | - |
482 | def get_charms(self, *args, **kwargs): |
483 | return self.get_response('charms', *args, **kwargs).json_body['result'] |
484 | |
485 | @@ -285,7 +113,7 @@ |
486 | self.index_client.index_charm(four) |
487 | result = self.get_charms(**{filter_str: 'common'}) |
488 | self.assertEqual(2, len(result)) |
489 | - res = self.api_class._charm_result |
490 | + res = lambda x: self.api_class._charm_result(x, {}, {}) |
491 | self.assertItemsEqual([res(one), res(two)], result) |
492 | result = self.get_charms(**{filter_str: 'different'}) |
493 | self.assertItemsEqual([res(three)], result) |
494 | @@ -332,8 +160,9 @@ |
495 | promulgated), 'community' otherwise. 'environment' causes no charms |
496 | to be returned. |
497 | """ |
498 | - charmers = self.api_class._charm_result(self.makeCharm()[1]) |
499 | - steve = self.api_class._charm_result(self.makeCharm(owner='steve')[1]) |
500 | + charmers = self.api_class._charm_result(self.makeCharm()[1], {}, {}) |
501 | + steve = self.api_class._charm_result(self.makeCharm(owner='steve')[1], |
502 | + {}, {}) |
503 | result = self.get_charms(type_='community') |
504 | self.assertEqual([steve], [charm for charm in result]) |
505 | result = self.get_charms(type_='approved') |
506 | @@ -352,9 +181,9 @@ |
507 | def test_charms_type_filter_considers_promulgation(self): |
508 | """Promulgation controls type.""" |
509 | reviewed = self.api_class._charm_result( |
510 | - self.makeCharm(promulgated=True)[1]) |
511 | + self.makeCharm(promulgated=True)[1], {}, {}) |
512 | unreviewed = self.api_class._charm_result( |
513 | - self.makeCharm(promulgated=False)[1]) |
514 | + self.makeCharm(promulgated=False)[1], {}, {}) |
515 | result = self.get_charms(type_='community') |
516 | self.assertEqual([unreviewed], [charm for charm in result]) |
517 | result = self.get_charms(type_='approved') |
518 | @@ -379,7 +208,7 @@ |
519 | self.assertEqual(200, response.status_code) |
520 | charms = response.json_body['result'] |
521 | self.assertEqual(1, len(charms)) |
522 | - self.assertEqual(self.api_class._charm_result(abc_charm), |
523 | + self.assertEqual(self.api_class._charm_result(abc_charm, {}, {}), |
524 | charms[0]) |
525 | |
526 | def test_charms_text_search_respects_filters(self): |
527 | @@ -388,7 +217,7 @@ |
528 | self.makeCharm(summary='abc def', owner='steve') |
529 | self.makeCharm(owner='steve') |
530 | result = self.get_charms(type_=['approved'], text='def') |
531 | - self.assertEqual([self.api_class._charm_result(official)], |
532 | + self.assertEqual([self.api_class._charm_result(official, {}, {})], |
533 | [charm for charm in result]) |
534 | |
535 | def test_charms_limit(self): |
536 | @@ -411,10 +240,6 @@ |
537 | 'parameter': 'limit', |
538 | }, response.json_body) |
539 | |
540 | - @staticmethod |
541 | - def get_name(charm): |
542 | - return charm['charm']['name'] |
543 | - |
544 | def test_charms_interesting(self): |
545 | """A json dump of interesting charms is available.""" |
546 | for num in range(15): |
547 | @@ -437,19 +262,21 @@ |
548 | self.assertIn('featured', data['result']) |
549 | self.assertEqual(['pop%d' % num for num in range(14, 4, -1)], |
550 | [self.get_name(r) for r in data['result']['popular']]) |
551 | - self.assertEqual(self.api_class._charm_result(last_popular_charm), |
552 | + self.assertEqual(self.api_class._charm_result(last_popular_charm, {}, |
553 | + {}), |
554 | data['result']['popular'][0]) |
555 | self.assertEqual(['new%d' % num for num in range(14, 4, -1)], |
556 | [self.get_name(r) for r in data['result']['new']]) |
557 | - self.assertEqual(self.api_class._charm_result(last_new_charm), |
558 | + self.assertEqual(self.api_class._charm_result(last_new_charm, {}, {}), |
559 | data['result']['new'][0]) |
560 | self.assertItemsEqual( |
561 | ['featured%d' % num for num in range(14, -1, -1)], |
562 | [self.get_name(r) for r in data['result']['featured']]) |
563 | featured14, = [charm for charm in data['result']['featured'] if |
564 | self.get_name(charm) == 'featured14'] |
565 | - self.assertEqual(self.api_class._charm_result(last_featured_charm), |
566 | - featured14) |
567 | + self.assertEqual( |
568 | + self.api_class._charm_result(last_featured_charm, {}, {}), |
569 | + featured14) |
570 | |
571 | def test_charms_accepts_categories(self): |
572 | """Categories are accepted (but ignored).""" |
573 | @@ -457,14 +284,308 @@ |
574 | response = self.get_response('charms', categories='asdf') |
575 | self.assertEqual(200, response.status_code) |
576 | |
577 | + |
578 | +class API1Mixin: |
579 | + |
580 | + api_class = API1 |
581 | + |
582 | + @staticmethod |
583 | + def weightless(charm): |
584 | + """Remove weight from charm because we cannot predict its value.""" |
585 | + charm = dict(charm) |
586 | + del charm['weight'] |
587 | + return charm |
588 | + |
589 | + @classmethod |
590 | + def weightless_relation(cls, relation): |
591 | + new_relation = {} |
592 | + for interface, charms in relation.items(): |
593 | + charms = [cls.weightless(charm) for charm in charms] |
594 | + new_relation[interface] = charms |
595 | + return new_relation |
596 | + |
597 | + @classmethod |
598 | + def related_weightless(cls, charm): |
599 | + return cls.weightless(cls.api_class._format_related(charm, 0)) |
600 | + |
601 | + |
602 | +class TestAPI1Charms(TestAPICharms, APITestBase, API1Mixin): |
603 | + |
604 | + def setUp(self): |
605 | + super(TestAPI1Charms, self).setUp() |
606 | + |
607 | + @staticmethod |
608 | + def get_name(charm): |
609 | + return charm['charm']['name'] |
610 | + |
611 | + def test_charms(self): |
612 | + """Charms endpoint produces expected result.""" |
613 | + one_changes = [factory.makeChange(created=datetime(1999, 12, 31), |
614 | + revno=37, message='Hello')] |
615 | + one_id, one = self.makeCharm(name='name1', changes=one_changes, |
616 | + date_created=datetime(1997, 10, 29, |
617 | + 12, 10, 9)) |
618 | + self.index_client.index_charm(one) |
619 | + maintainer2 = 'jrandom@example.com (J. Random Hacker)' |
620 | + provides2 = {'cookie-factory': { |
621 | + 'interface': 'imap', |
622 | + }} |
623 | + requires2 = {'postal-service': { |
624 | + 'interface': 'envelope', |
625 | + }} |
626 | + options2 = { |
627 | + 'foobar': { |
628 | + 'type': 'str', |
629 | + 'default': 'baz', |
630 | + 'description': 'quxx', |
631 | + }, |
632 | + } |
633 | + files2 = { |
634 | + 'readme': { |
635 | + 'filename': 'README.md', |
636 | + 'subdir': '', |
637 | + }, |
638 | + 'config-changed': { |
639 | + 'filename': 'config-changed', |
640 | + 'subdir': 'hooks', |
641 | + }, |
642 | + } |
643 | + two_changes = [factory.makeChange(created=datetime(1998, 11, 30), |
644 | + revno=32, message='Hi', |
645 | + authors=['Baz <baz@example.org>'])] |
646 | + two_id, two = self.makeCharm(owner='jrandom', revision=3, |
647 | + series='warty', summary='Charm two', |
648 | + name='name2', revno=13, |
649 | + maintainer=maintainer2, |
650 | + commit_message='message2', |
651 | + provides=provides2, |
652 | + requires=requires2, options=options2, |
653 | + files=files2, subordinate=True, |
654 | + changes=two_changes, |
655 | + date_created=datetime(1996, 9, 28, |
656 | + 11, 9, 8)) |
657 | + response = self.get_response('charms') |
658 | + self.assertEqual(200, response.status_code) |
659 | + self.assertEqual('application/json', response.content_type) |
660 | + result = response.json_body['result'] |
661 | + self.assertEqual(2, len(result)) |
662 | + one_url = make_store_url(1, get_address(one, short=True)) |
663 | + self.assertEqual({ |
664 | + 'charm': { |
665 | + 'owner': 'charmers', |
666 | + 'maintainer': { |
667 | + 'name': 'John Doe', |
668 | + 'email': 'jdoe@example.com', |
669 | + }, |
670 | + 'id': one_url[3:], |
671 | + 'date_created': '1997-10-29T12:10:09', |
672 | + 'name': 'name1', |
673 | + 'url': one_url, |
674 | + 'description': one['description'], |
675 | + 'revision': 1, |
676 | + 'summary': "Remote terminal classroom over https", |
677 | + 'distro_series': 'precise', |
678 | + 'is_approved': True, |
679 | + 'rating_numerator': 0, |
680 | + 'rating_denominator': 0, |
681 | + 'code_source': { |
682 | + 'type': 'bzr', |
683 | + 'location': |
684 | + 'lp:~charmers/charms/precise/name1/trunk', |
685 | + 'revision': '12', |
686 | + 'last_log': 'maintainer', |
687 | + 'bugs_link': |
688 | + 'https://bugs.launchpad.net/charms/' |
689 | + '+source/name1', |
690 | + 'revisions': [{ |
691 | + 'revno': 37, |
692 | + 'message': 'Hello', |
693 | + 'date': '1999-12-31T00:00:00Z', |
694 | + 'authors': [ |
695 | + { |
696 | + 'name': 'Foo', |
697 | + 'email': 'foo@example.com' |
698 | + }, |
699 | + { |
700 | + 'name': 'Bar', |
701 | + 'email': 'bar@example.com' |
702 | + }, |
703 | + ], |
704 | + }], |
705 | + }, |
706 | + 'relations': { |
707 | + 'provides': { |
708 | + 'website': { |
709 | + 'interface': 'https', |
710 | + }, |
711 | + }, |
712 | + 'requires': { |
713 | + 'database': { |
714 | + 'interface': 'mongodb', |
715 | + }, |
716 | + }, |
717 | + }, |
718 | + 'options': { |
719 | + 'script-interval': { |
720 | + 'type': 'int', |
721 | + 'default': 5, |
722 | + 'description': |
723 | + 'The interval between script runs.' |
724 | + }, |
725 | + }, |
726 | + 'files': ['README', 'hooks/install'], |
727 | + 'tested_providers': {}, |
728 | + 'downloads_in_past_30_days': 0, |
729 | + 'is_subordinate': False, |
730 | + }, |
731 | + 'metadata': { |
732 | + 'related': { |
733 | + 'provides': {}, |
734 | + 'requires': {}, |
735 | + } |
736 | + } |
737 | + }, result[0]) |
738 | + two_url = make_store_url(1, get_address(two, short=False)) |
739 | + self.assertEqual({ |
740 | + 'charm': { |
741 | + 'owner': 'jrandom', |
742 | + 'maintainer': { |
743 | + 'name': 'J. Random Hacker', |
744 | + 'email': 'jrandom@example.com', |
745 | + }, |
746 | + 'id': two_url[3:], |
747 | + 'date_created': '1996-09-28T11:09:08', |
748 | + 'name': 'name2', |
749 | + 'url': two_url, |
750 | + 'description': two['description'], |
751 | + 'revision': 3, |
752 | + 'summary': 'Charm two', |
753 | + 'distro_series': 'warty', |
754 | + 'is_approved': False, |
755 | + 'rating_numerator': 0, |
756 | + 'rating_denominator': 0, |
757 | + 'code_source': { |
758 | + 'type': 'bzr', |
759 | + 'location': |
760 | + 'lp:~jrandom/charms/warty/name2/trunk', |
761 | + 'revision': '13', |
762 | + 'revisions': [{ |
763 | + 'revno': 32, |
764 | + 'message': 'Hi', |
765 | + 'date': '1998-11-30T00:00:00Z', |
766 | + 'authors': [ |
767 | + { |
768 | + 'name': 'Baz', |
769 | + 'email': 'baz@example.org' |
770 | + }, |
771 | + ], |
772 | + }], |
773 | + 'last_log': 'message2', |
774 | + 'bugs_link': |
775 | + 'https://bugs.launchpad.net/charms/+source/name2' |
776 | + }, |
777 | + 'relations': { |
778 | + 'provides': provides2, |
779 | + 'requires': requires2, |
780 | + |
781 | + }, |
782 | + 'options': options2, |
783 | + 'files': ['README.md', 'hooks/config-changed'], |
784 | + 'tested_providers': {}, |
785 | + 'downloads_in_past_30_days': 0, |
786 | + 'is_subordinate': True, |
787 | + }, |
788 | + 'metadata': { |
789 | + 'related': { |
790 | + 'provides': {}, |
791 | + 'requires': {}, |
792 | + } |
793 | + } |
794 | + }, result[1]) |
795 | + self.assertNotEqual(one_id, two_id) |
796 | + self.assertNotEqual(one_url, two_url) |
797 | + |
798 | def test_charms_denies_category(self): |
799 | """Category is not accepted.""" |
800 | self.index_client.wait_for_startup() |
801 | response = self.get_response('charms', category='asdf') |
802 | self.assertEqual(406, response.status_code) |
803 | |
804 | - |
805 | -class TestAPI0Charms(TestAPI1Charms): |
806 | + def test_charms_interesting_includes_related(self): |
807 | + older = datetime.utcfromtimestamp(0) |
808 | + newer = datetime.utcfromtimestamp(7200) |
809 | + self.makeCharm( |
810 | + date_created=newer, name='new', |
811 | + provides={'asdf': {'interface': 'new-p'}}, |
812 | + requires={'asdf': {'interface': 'new-r'}}, |
813 | + )[1] |
814 | + requires_newest = self.makeCharm( |
815 | + date_created=older, name='req-new', requires={ |
816 | + 'asdf': {'interface': 'new-p'} |
817 | + })[1] |
818 | + provides_newest = self.makeCharm( |
819 | + date_created=older, name='provides-new', provides={ |
820 | + 'asdf': {'interface': 'new-r'} |
821 | + })[1] |
822 | + self.makeCharm( |
823 | + date_created=older, name='pop', |
824 | + provides={'asdf': {'interface': 'pop-p'}}, |
825 | + requires={'asdf': {'interface': 'pop-r'}}, |
826 | + downloads_in_past_30_days=10, |
827 | + )[1] |
828 | + requires_popular = self.makeCharm( |
829 | + date_created=older, name='req-pop', requires={ |
830 | + 'asdf': {'interface': 'pop-p'} |
831 | + })[1] |
832 | + provides_popular = self.makeCharm( |
833 | + date_created=older, name='provides-pop', provides={ |
834 | + 'asdf': {'interface': 'pop-r'} |
835 | + })[1] |
836 | + self.makeCharm( |
837 | + date_created=older, name='feat', |
838 | + provides={'asdf': {'interface': 'feat-p'}}, |
839 | + requires={'asdf': {'interface': 'feat-r'}}, |
840 | + is_featured=True, |
841 | + )[1] |
842 | + requires_featured = self.makeCharm( |
843 | + date_created=older, name='req-feat', requires={ |
844 | + 'asdf': {'interface': 'feat-p'} |
845 | + })[1] |
846 | + provides_featured = self.makeCharm( |
847 | + date_created=older, name='provides-feat', provides={ |
848 | + 'asdf': {'interface': 'feat-r'} |
849 | + })[1] |
850 | + data = self.get_response('charms', remainder='/interesting').json_body |
851 | + new_related = data['result']['new'][0]['metadata']['related'] |
852 | + self.assertEqual( |
853 | + self.weightless_relation(new_related['requires']), |
854 | + {'new-p': [self.related_weightless(requires_newest)]} |
855 | + ) |
856 | + self.assertEqual( |
857 | + self.weightless_relation(new_related['provides']), |
858 | + {'new-r': [self.related_weightless(provides_newest)]} |
859 | + ) |
860 | + pop_related = data['result']['popular'][0]['metadata']['related'] |
861 | + self.assertEqual( |
862 | + self.weightless_relation(pop_related['requires']), |
863 | + {'pop-p': [self.related_weightless(requires_popular)]} |
864 | + ) |
865 | + self.assertEqual( |
866 | + self.weightless_relation(pop_related['provides']), |
867 | + {'pop-r': [self.related_weightless(provides_popular)]} |
868 | + ) |
869 | + feat_related = data['result']['featured'][0]['metadata']['related'] |
870 | + self.assertEqual( |
871 | + self.weightless_relation(feat_related['requires']), |
872 | + {'feat-p': [self.related_weightless(requires_featured)]} |
873 | + ) |
874 | + self.assertEqual( |
875 | + self.weightless_relation(feat_related['provides']), |
876 | + {'feat-r': [self.related_weightless(provides_featured)]} |
877 | + ) |
878 | + |
879 | + |
880 | +class TestAPI0Charms(TestAPICharms, APITestBase): |
881 | |
882 | api_class = API0 |
883 | |
884 | @@ -647,19 +768,14 @@ |
885 | response = self.get_response('charms', category='asdf') |
886 | self.assertEqual(200, response.status_code) |
887 | |
888 | - def test_charms_denies_category(self): |
889 | - pass |
890 | - |
891 | - |
892 | -class TestAPI1(APITestBase): |
893 | - |
894 | - api_class = API1 |
895 | + |
896 | +class TestAPI: |
897 | |
898 | def make_charm(self, *args, **kwargs): |
899 | """Make a charm formatted for api.""" |
900 | charm = factory.makeCharm(self.db, *args, **kwargs)[1] |
901 | return (self.api_class._get_api_id(charm), |
902 | - self.api_class._charm_result(charm)) |
903 | + self.api_class._charm_result(charm, {}, {})) |
904 | |
905 | def test_unknown_endpoint(self): |
906 | """Unknown endpoints produce a 404.""" |
907 | @@ -807,6 +923,7 @@ |
908 | |
909 | def test_charm(self): |
910 | """Retrieving a charm works.""" |
911 | + self.use_index_client() |
912 | charmid, charm = self.make_charm() |
913 | response = self.get_response( |
914 | 'charm', remainder=charmid) |
915 | @@ -823,6 +940,7 @@ |
916 | |
917 | def test_charm_wrong_revision(self): |
918 | """Retrieving a charm works even if revision is wrong.""" |
919 | + self.use_index_client() |
920 | charm = self.make_charm()[1] |
921 | stale_charm_id = self.get_id(charm) + '37' |
922 | response = self.get_response('charm', remainder=stale_charm_id) |
923 | @@ -1003,10 +1121,114 @@ |
924 | self.assertEqual(0, response.content_length) |
925 | |
926 | |
927 | -class TestAPI0(TestAPI1): |
928 | +class TestAPI1(APITestBase, TestAPI, API1Mixin): |
929 | + |
930 | + def test_charm_result_includes_charm_and_metadata(self): |
931 | + charm = factory.get_charm_json(promulgated=False) |
932 | + result = self.api_class._charm_result(charm, {}, {}) |
933 | + self.assertIn('metadata', result) |
934 | + self.assertIn('charm', result) |
935 | + |
936 | + def test_format_related(self): |
937 | + charm = factory.get_charm_json() |
938 | + formatted = self.api_class._format_related(charm, 0) |
939 | + self.assertItemsEqual(['id', 'name', 'categories', 'weight'], |
940 | + formatted.keys()) |
941 | + self.assertEqual(self.api_class._get_api_id(charm), formatted['id']) |
942 | + self.assertEqual(charm['name'], formatted['name']) |
943 | + self.assertEqual(charm['categories'], formatted['categories']) |
944 | + |
945 | + def test_format_related_no_categories(self): |
946 | + charm = factory.get_charm_json() |
947 | + del charm['categories'] |
948 | + formatted = self.api_class._format_related(charm, 0) |
949 | + self.assertEqual([], formatted['categories']) |
950 | + |
951 | + def test_format_related_icon(self): |
952 | + charm = factory.get_charm_json() |
953 | + charm['icon'] = 'hello_world' |
954 | + formatted = self.api_class._format_related(charm, 0) |
955 | + self.assertEqual(charm['icon'], formatted['icon']['body']) |
956 | + self.assertEqual('/api/1/' + formatted['id'] + '/icon', |
957 | + formatted['icon']['link']) |
958 | + |
959 | + def test_charm_result_includes_related(self): |
960 | + charm = factory.get_charm_json(promulgated=False, provides={ |
961 | + 'cookie-factory': { |
962 | + 'interface': 'imap', |
963 | + } |
964 | + }, requires={ |
965 | + 'cookie-factory': { |
966 | + 'interface': 'ssl', |
967 | + } |
968 | + }) |
969 | + imap_user = factory.get_charm_json(requires={ |
970 | + 'foo': {'interface': 'imap'} |
971 | + }) |
972 | + ssl_user = factory.get_charm_json(requires={ |
973 | + 'bar': {'interface': 'ssl'} |
974 | + }) |
975 | + ssl_provider = factory.get_charm_json(provides={ |
976 | + 'bar': {'interface': 'ssl'} |
977 | + }) |
978 | + imap_provider = factory.get_charm_json(provides={ |
979 | + 'bar': {'interface': 'imap'} |
980 | + }) |
981 | + requires = { |
982 | + 'imap': [self.api_class._format_related(imap_user, 0)], |
983 | + 'ssl': [self.api_class._format_related(ssl_user, 0)], |
984 | + } |
985 | + provides = { |
986 | + 'ssl': [self.api_class._format_related(ssl_provider, 0)], |
987 | + 'imap': [self.api_class._format_related(imap_provider, 0)], |
988 | + } |
989 | + result = self.api_class._charm_result(charm, requires, provides) |
990 | + self.assertEqual([ |
991 | + self.api_class._format_related(imap_user, 0)], |
992 | + result['metadata']['related']['requires']['imap']) |
993 | + self.assertEqual([ |
994 | + self.api_class._format_related(ssl_provider, 0)], |
995 | + result['metadata']['related']['provides']['ssl']) |
996 | + |
997 | + def test_charm_includes_related(self): |
998 | + self.use_index_client() |
999 | + source = CharmSource.from_request(self) |
1000 | + main_charm = factory.get_charm_json( |
1001 | + requires={'foo': {'interface': 'imap'}}, |
1002 | + provides={'foo': {'interface': 'smtp'}}, |
1003 | + ) |
1004 | + source.save(main_charm) |
1005 | + provides_charm = factory.get_charm_json( |
1006 | + provides={'foo': {'interface': 'imap'}}, |
1007 | + ) |
1008 | + source.save(provides_charm) |
1009 | + requires_charm = factory.get_charm_json( |
1010 | + requires={'foo': {'interface': 'smtp'}}, |
1011 | + ) |
1012 | + source.save(requires_charm) |
1013 | + charm_id = self.api_class._get_api_id(main_charm) |
1014 | + response = self.get_response('charm', remainder=charm_id).json_body |
1015 | + related = response['metadata']['related'] |
1016 | + self.assertEqual( |
1017 | + self.weightless_relation(related['requires']), |
1018 | + {'smtp': [self.related_weightless(requires_charm)]} |
1019 | + ) |
1020 | + self.assertEqual( |
1021 | + self.weightless_relation(related['provides']), |
1022 | + {'imap': [self.related_weightless(provides_charm)]} |
1023 | + ) |
1024 | + |
1025 | + |
1026 | +class TestAPI0(APITestBase, TestAPI): |
1027 | |
1028 | api_class = API0 |
1029 | |
1030 | @staticmethod |
1031 | def get_id(charm): |
1032 | return charm['id'] |
1033 | + |
1034 | + def test_charm_result_excludes_charm_and_metadata(self): |
1035 | + charm = factory.get_charm_json(promulgated=False) |
1036 | + result = self.api_class._charm_result(charm, {}, {}) |
1037 | + self.assertNotIn('metadata', result) |
1038 | + self.assertNotIn('charm', result) |
Hi Aaron.
It seems possible (because a test does not show it is not) that a charm could be listed among it's related charms. Eg apache requires and provides http, and I think its popularity would guarantee it to be listed among the other related charms. Except that is is not an "other" charm. Are we concerned about this?