Merge lp:~bac/charmworld/bundles-display into lp:~juju-jitsu/charmworld/trunk

Proposed by Brad Crittenden
Status: Merged
Approved by: Brad Crittenden
Approved revision: 348
Merged at revision: 344
Proposed branch: lp:~bac/charmworld/bundles-display
Merge into: lp:~juju-jitsu/charmworld/trunk
Diff against target: 553 lines (+173/-74)
9 files modified
charmworld/jobs/ingest.py (+10/-8)
charmworld/jobs/tests/test_ingest.py (+15/-11)
charmworld/models.py (+50/-14)
charmworld/search.py (+4/-3)
charmworld/testing/factory.py (+19/-7)
charmworld/tests/test_models.py (+19/-8)
charmworld/tests/test_search.py (+35/-0)
charmworld/views/api.py (+6/-5)
charmworld/views/tests/test_api.py (+15/-18)
To merge this branch: bzr merge lp:~bac/charmworld/bundles-display
Reviewer Review Type Date Requested Status
Benji York (community) Approve
Review via email: mp+179541@code.launchpad.net

Commit message

Change the basket field in the Bundle to be a basket name and revision number in the form 'basket/revno'.

Description of the change

Change the basket field in the Bundle to be a basket name and revision number in the form 'basket/revno'. Previously there was inconsistency in the way the basket id was created and used.

Remove from the top-level any bundle items that are in the 'data' dictionary.

Change models.py:store_bundles to be easier to test by making the collection and indexer optional.

To post a comment you must log in.
Revision history for this message
Benji York (benji) wrote :

The branch looks good. Here are some semi-bikesheddy comments.

It's kind of strange that construct_basket_id returns two values while
all the other "construct_*_id" functions just return the ID. Perhaps
two functions would be better. (?)

Does "make lint" complain about the new unused variables?

Re. get_bundle_data: so now if you don't specify an owner argument you
get an empty string, but if you pass owner=None you get a random string?
That seems a little odd. Oh, maybe you did that because the change in
behavior broke a bunch of tests.

Another thing, about get_bundle_data: I always thought it was weird to
use the local namespace as a sandbox in which to construct the return
value. Now that you're doing more in the function and having to do the
"del rep[k]" at the end it's getting more strained. You might consider
revamping the function to use **charm_data as the parameter list instead
and then manipulating charm_data instead of the locals.

review: Approve
Revision history for this message
Brad Crittenden (bac) wrote :

> The branch looks good. Here are some semi-bikesheddy comments.
>
> It's kind of strange that construct_basket_id returns two values while
> all the other "construct_*_id" functions just return the ID. Perhaps
> two functions would be better. (?)

I renamed to set_basket_info and modify the basket_data dictionary in place.

>
> Does "make lint" complain about the new unused variables?

Nope. Which ones?

>
>
> Re. get_bundle_data: so now if you don't specify an owner argument you
> get an empty string, but if you pass owner=None you get a random string?

No, both cases give you a random string.

> That seems a little odd. Oh, maybe you did that because the change in
> behavior broke a bunch of tests.

No, any broken tests were fixed.

>
> Another thing, about get_bundle_data: I always thought it was weird to
> use the local namespace as a sandbox in which to construct the return
> value. Now that you're doing more in the function and having to do the
> "del rep[k]" at the end it's getting more strained. You might consider
> revamping the function to use **charm_data as the parameter list instead
> and then manipulating charm_data instead of the locals.

I've fixed it to be more explicit.

Thanks for the review and good suggestions.

Revision history for this message
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

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'charmworld/jobs/ingest.py'
--- charmworld/jobs/ingest.py 2013-08-09 18:53:40 +0000
+++ charmworld/jobs/ingest.py 2013-08-11 23:10:56 +0000
@@ -288,13 +288,14 @@
288 self.working_dir = working_dir288 self.working_dir = working_dir
289 super(UpdateBundleJob, self).__init__()289 super(UpdateBundleJob, self).__init__()
290290
291 def store_bundles(self, deployer_config, basket_id):291 def store_bundles(self, deployer_config, owner, basket_id):
292 store_bundles(self.db.bundles, deployer_config, basket_id)292 store_bundles(self.db.bundles, deployer_config, owner, basket_id)
293293
294 @staticmethod294 @staticmethod
295 def construct_basket_id(basket_data, revno):295 def set_basket_info(data, revno):
296 owner_segment = '~%s/' % basket_data['owner']296 owner_segment = '~%s' % data['owner']
297 return '%s%s-%d' % (owner_segment, basket_data['name'], revno)297 data['name_revno'] = "%s/%d" % (data['name'], revno)
298 data['_id'] = '%s/%s' % (owner_segment, data['name_revno'])
298299
299 @staticmethod300 @staticmethod
300 def decorate_bundles(basket_contents, basket_id):301 def decorate_bundles(basket_contents, basket_id):
@@ -306,7 +307,7 @@
306 branch_dir = fetch_branch(self.working_dir, basket_data, self.log)307 branch_dir = fetch_branch(self.working_dir, basket_data, self.log)
307 branch = Branch.open(branch_dir)308 branch = Branch.open(branch_dir)
308 revno = branch.revision_id_to_revno(basket_data['commit'])309 revno = branch.revision_id_to_revno(basket_data['commit'])
309 basket_data['_id'] = self.construct_basket_id(basket_data, revno)310 self.set_basket_info(basket_data, revno)
310 with read_locked(branch):311 with read_locked(branch):
311 tree = branch.repository.revision_tree(basket_data['commit'])312 tree = branch.repository.revision_tree(basket_data['commit'])
312 basket_data['file_hashes'] = quote_yaml(slurp_files(fs, tree))313 basket_data['file_hashes'] = quote_yaml(slurp_files(fs, tree))
@@ -324,7 +325,8 @@
324 self.db.baskets.save(basket_data)325 self.db.baskets.save(basket_data)
325 deployer_config = self.get_deployer_config(fs, basket_data)326 deployer_config = self.get_deployer_config(fs, basket_data)
326 self.decorate_bundles(deployer_config, basket_data['_id'])327 self.decorate_bundles(deployer_config, basket_data['_id'])
327 self.store_bundles(deployer_config, basket_data['_id'])328 self.store_bundles(
329 deployer_config, basket_data['owner'], basket_data['name_revno'])
328330
329331
330def _rev_info(r, branch):332def _rev_info(r, branch):
@@ -333,7 +335,7 @@
333 "revno": branch.revision_id_to_revno(r.revision_id),335 "revno": branch.revision_id_to_revno(r.revision_id),
334 "committer": r.committer,336 "committer": r.committer,
335 "created": r.timestamp,337 "created": r.timestamp,
336 "message": r.message338 "message": r.message,
337 }339 }
338 return d340 return d
339341
340342
=== modified file 'charmworld/jobs/tests/test_ingest.py'
--- charmworld/jobs/tests/test_ingest.py 2013-08-09 18:53:40 +0000
+++ charmworld/jobs/tests/test_ingest.py 2013-08-11 23:10:56 +0000
@@ -383,7 +383,7 @@
383 with bundle_update_environment(source_branch_dir):383 with bundle_update_environment(source_branch_dir):
384 job.decorate_basket(bundle_data, None)384 job.decorate_basket(bundle_data, None)
385385
386 self.assertTrue(bundle_data['_id'].endswith('-8'))386 self.assertTrue(bundle_data['_id'].endswith('/8'))
387387
388 def test_decorate_basket_stores_files(self):388 def test_decorate_basket_stores_files(self):
389 job = UpdateBundleJob()389 job = UpdateBundleJob()
@@ -423,21 +423,22 @@
423 with self.assertRaises(bzrlib.errors.NoSuchRevision):423 with self.assertRaises(bzrlib.errors.NoSuchRevision):
424 job.decorate_basket(bundle_data, None)424 job.decorate_basket(bundle_data, None)
425425
426 def test_construct_basket_id_includes_owner_if_promulgated(self):426 def test_set_basket_info_includes_owner_if_promulgated(self):
427 payload = factory.get_payload_json(name='mysql', promulgated=True)427 payload = factory.get_payload_json(name='mysql', promulgated=True)
428 basket_id = UpdateBundleJob.construct_basket_id(payload, 5)428 UpdateBundleJob.set_basket_info(payload, 5)
429 self.assertEqual('~charmers/mysql-5', basket_id)429 self.assertEqual('mysql/5', payload['name_revno'])
430 self.assertEqual('~charmers/mysql/5', payload['_id'])
430431
431 def test_construct_basket_id_includes_owner_if_not_promulgated(self):432 def test_set_basket_info_includes_owner_if_not_promulgated(self):
432 payload = factory.get_payload_json(name='mysql', promulgated=False)433 payload = factory.get_payload_json(name='mysql', promulgated=False)
433 basket_id = UpdateBundleJob.construct_basket_id(payload, 5)434 UpdateBundleJob.set_basket_info(payload, 5)
434 self.assertEqual('~charmers/mysql-5', basket_id)435 self.assertEqual('mysql/5', payload['name_revno'])
436 self.assertEqual('~charmers/mysql/5', payload['_id'])
435437
436 def test_job_can_store_bundles_in_the_db(self):438 def test_job_can_store_bundles_in_the_db(self):
437 basket_data = factory.get_payload_json(name='wordpress')439 basket_data = factory.get_payload_json(name='wordpress')
438 basket_id = UpdateBundleJob.construct_basket_id(basket_data, 5)440 UpdateBundleJob.set_basket_info(basket_data, 5)
439 bundle_id = '%s/%s' % (basket_id, 'stage')441 bundle_id = '%s/%s' % (basket_data['_id'], 'stage')
440 basket_data['_id'] = basket_id
441 job = UpdateBundleJob()442 job = UpdateBundleJob()
442 job.setup(self.db)443 job.setup(self.db)
443 DEPLOYER_CONFIG_HASH = '31337'444 DEPLOYER_CONFIG_HASH = '31337'
@@ -457,11 +458,14 @@
457 engine: apache458 engine: apache
458 """)459 """)
459 fs.put(deployer_config, _id=DEPLOYER_CONFIG_HASH)460 fs.put(deployer_config, _id=DEPLOYER_CONFIG_HASH)
460 job.store_bundles(yaml.safe_load(deployer_config), basket_id)461 job.store_bundles(
462 yaml.safe_load(deployer_config),
463 basket_data['owner'], basket_data['name_revno'])
461 self.assertIsNotNone(self.db.bundles.find_one(bundle_id))464 self.assertIsNotNone(self.db.bundles.find_one(bundle_id))
462465
463 def test_job_run_stores_bundles_in_the_db(self):466 def test_job_run_stores_bundles_in_the_db(self):
464 basket_data = factory.get_payload_json(name='wordpress')467 basket_data = factory.get_payload_json(name='wordpress')
468 basket_data['name_revno'] = 'dummy'
465 job = UpdateBundleJob()469 job = UpdateBundleJob()
466 job.setup(self.db)470 job.setup(self.db)
467 with patch.object(job, 'get_deployer_config'):471 with patch.object(job, 'get_deployer_config'):
468472
=== modified file 'charmworld/models.py'
--- charmworld/models.py 2013-08-09 18:53:40 +0000
+++ charmworld/models.py 2013-08-11 23:10:56 +0000
@@ -7,14 +7,17 @@
7from bzrlib.branch import Branch7from bzrlib.branch import Branch
8from bzrlib.errors import NoSuchRevision8from bzrlib.errors import NoSuchRevision
9from calendar import timegm9from calendar import timegm
10import copy
10import hashlib11import hashlib
11import logging12import logging
12from mimetypes import guess_type13from mimetypes import guess_type
13import os14import os
14from os.path import dirname15from os.path import (
15from os.path import join16 dirname,
16from os.path import normpath17 join,
17from os.path import split18 normpath,
19 split,
20)
18import random21import random
19import re22import re
2023
@@ -1176,11 +1179,9 @@
1176 'basket': None,1179 'basket': None,
1177 'name': '',1180 'name': '',
1178 'inherits': None,1181 'inherits': None,
1179 'series': '',
1180 'title': '',1182 'title': '',
1181 'description': '',1183 'description': '',
1182 'services': dict(),1184 'data': dict(),
1183 'relations': dict(),
1184 'promulgated': False,1185 'promulgated': False,
1185 'branch_deleted': False,1186 'branch_deleted': False,
1186 'doctype': 'bundle',1187 'doctype': 'bundle',
@@ -1204,9 +1205,13 @@
1204 else:1205 else:
1205 raise AttributeError1206 raise AttributeError
12061207
1208 @staticmethod
1209 def construct_id(owner, basket, name):
1210 return "~%s/%s/%s" % (owner, basket, name)
1211
1207 @property1212 @property
1208 def id(self):1213 def id(self):
1209 return "~{}/{}/{}".format(self.owner, self.basket, self.name)1214 return self.construct_id(self.owner, self.basket, self.name)
12101215
1211 @property1216 @property
1212 def permanent_url(self):1217 def permanent_url(self):
@@ -1303,14 +1308,45 @@
1303 return options1308 return options
13041309
13051310
1306def store_bundles(collection, deployer_config, basket_id):1311def store_bundles(collection, deployer_config, owner, basket_id,
1307 """Store a basket of bundles into both MongoDB and ElasticSearch."""1312 index_client=None):
1308 index_client = ElasticSearchClient.from_settings(settings)1313 """Store a basket of bundles into MongoDB and/or ElasticSearch.
1309 index_client.index_bundles(deployer_config.values())1314
1315 :param: collection: A db bundles collection. If None the bundles are not
1316 stored to Mongo.
1317 :param: deployer_config: A dictionary representing the basket of bundles.
1318 :param: owner: A string representing the owner. No leading ~.
1319 :param: basket_id: A slash-joined string of name and revision for the
1320 basket. Does not include the owner name.
1321 :param: index_client: The elastic search client. If None it will be
1322 retrieved. Useful for Mongo-only tests where the indexing is not
1323 needed.
1324 """
1325
1326 if index_client is None:
1327 index_client = ElasticSearchClient.from_settings(settings)
1328
1329 # Augment the deployer_config with additional information for indexing.
1330 ids = {}
1331 config_copy = copy.deepcopy(deployer_config)
1332 for key in config_copy:
1333 ids[key] = Bundle.construct_id(owner, basket_id, key)
1334 config_copy[key]['id'] = ids[key]
1335 config_copy[key]['owner'] = owner
1336 config_copy[key]['name'] = key
1337 config_copy[key]['basket'] = basket_id
1338
1339 index_client.index_bundles(config_copy.values())
1340 if collection is None:
1341 return
1310 for key in deployer_config:1342 for key in deployer_config:
1343 data = get_flattened_deployment(deployer_config, key)
1311 bundle_doc = {1344 bundle_doc = {
1312 '_id': '%s/%s' % (basket_id, key),1345 '_id': ids[key],
1313 'data': get_flattened_deployment(deployer_config, key),1346 'name': key,
1347 'owner': owner,
1348 'basket': basket_id,
1349 'data': data,
1314 }1350 }
1315 collection.save(bundle_doc)1351 collection.save(bundle_doc)
13161352
13171353
=== modified file 'charmworld/search.py'
--- charmworld/search.py 2013-08-09 18:53:40 +0000
+++ charmworld/search.py 2013-08-11 23:10:56 +0000
@@ -32,6 +32,7 @@
32 'description': 3,32 'description': 3,
33 'config.options.description': None,33 'config.options.description': None,
34 'relations': None,34 'relations': None,
35 'services': None,
35}36}
3637
3738
@@ -430,9 +431,9 @@
430431
431 with Timer() as timer:432 with Timer() as timer:
432 status = self.get_status()433 status = self.get_status()
433 for real_index in self.get_aliased():434 try:
434 break435 real_index = self.get_aliased()[0]
435 else:436 except IndexError:
436 real_index = self.index_name437 real_index = self.index_name
437 docs = status[real_index].get('docs')438 docs = status[real_index].get('docs')
438 if docs is None:439 if docs is None:
439440
=== modified file 'charmworld/testing/factory.py'
--- charmworld/testing/factory.py 2013-08-09 18:53:40 +0000
+++ charmworld/testing/factory.py 2013-08-11 23:10:56 +0000
@@ -11,7 +11,6 @@
11)11)
12import hashlib12import hashlib
13import os13import os
14from os.path import basename
15from random import randint14from random import randint
16import random15import random
17import stat16import stat
@@ -311,17 +310,29 @@
311 return record_id, charm310 return record_id, charm
312311
313312
314def get_bundle_data(name=None, owner='', basket=None, inherits=None,313def get_bundle_data(name=None, owner=None, basket=None, inherits=None,
315 series='precise', title='', description='',314 series='precise', title='', description='',
316 services=dict(), relations=dict(), promulgated=False,315 services=dict(), relations=dict(), promulgated=False,
317 branch_deleted=False):316 branch_deleted=False):
318 if name is None:317 if name is None:
319 name = random_string(10)318 name = random_string(10)
319 if owner is None:
320 owner = random_string(10)
320 if basket is None:321 if basket is None:
321 basket = random_string(10)322 basket = "%s/%d" % (random_string(10), randint(0, 100))
322 doctype = 'bundle'323 data = dict(series=series,
323 doctype # Hush lint's dislike of defining variables for the vars() call.324 relations=relations,
324 return dict(**vars())325 services=services)
326 return dict(basket=basket,
327 branch_deleted=branch_deleted,
328 data=data,
329 description=description,
330 doctype='bundle',
331 inherits=inherits,
332 name=name,
333 owner=owner,
334 promulgated=promulgated,
335 title=title)
325336
326337
327def makeBundle(db, *args, **kwargs):338def makeBundle(db, *args, **kwargs):
@@ -334,7 +345,8 @@
334345
335def make_charm_file(db, charm, path, content=None):346def make_charm_file(db, charm, path, content=None):
336 fs = getfs(db)347 fs = getfs(db)
337 charm_file = CharmFileSet.make_charm_file(fs, charm, path, basename(path))348 charm_file = CharmFileSet.make_charm_file(
349 fs, charm, path, os.path.basename(path))
338 if content is None:350 if content is None:
339 content = random_string(10).encode('utf-8')351 content = random_string(10).encode('utf-8')
340 charm_file.save(content)352 charm_file.save(content)
341353
=== modified file 'charmworld/tests/test_models.py'
--- charmworld/tests/test_models.py 2013-08-09 18:53:40 +0000
+++ charmworld/tests/test_models.py 2013-08-11 23:10:56 +0000
@@ -1458,6 +1458,7 @@
14581458
1459 def test_store_bundles(self):1459 def test_store_bundles(self):
1460 # The bundles can be stored in the database.1460 # The bundles can be stored in the database.
1461
1461 deployer_config = dedent("""\1462 deployer_config = dedent("""\
1462 wordpress-stage:1463 wordpress-stage:
1463 series: precise1464 series: precise
@@ -1470,13 +1471,21 @@
1470 engine: apache1471 engine: apache
1471 """)1472 """)
1472 parsed = yaml.safe_load(deployer_config)1473 parsed = yaml.safe_load(deployer_config)
1473 store_bundles(self.db.bundles, parsed, 'wordpress-5')1474 owner = 'bac'
1475 bundle_name = 'wordpress-stage'
1476 basket_id = 'wordpress-basket/5'
1477 _id = "~%s/%s/%s" % (owner, basket_id, bundle_name)
1478 store_bundles(
1479 self.db.bundles, parsed, 'bac', basket_id)
1474 self.assertEqual(1480 self.assertEqual(
1475 {1481 {
1476 '_id': 'wordpress-5/wordpress-stage',1482 '_id': _id,
1483 'basket': basket_id,
1484 'name': bundle_name,
1485 'owner': owner,
1477 'data': parsed['wordpress-stage'],1486 'data': parsed['wordpress-stage'],
1478 },1487 },
1479 self.db.bundles.find_one('wordpress-5/wordpress-stage'))1488 self.db.bundles.find_one(_id))
14801489
1481 def test_store_bundles_calls_get_flattened_deployment(self):1490 def test_store_bundles_calls_get_flattened_deployment(self):
1482 # For every key in the top-level deployment data,1491 # For every key in the top-level deployment data,
@@ -1509,7 +1518,7 @@
15091518
1510 with patch('charmworld.models.get_flattened_deployment',1519 with patch('charmworld.models.get_flattened_deployment',
1511 get_flattened_deployment):1520 get_flattened_deployment):
1512 store_bundles(self.db.bundles, parsed, 'wordpress-5')1521 store_bundles(self.db.bundles, parsed, 'bac', 'wordpress-basket/5')
1513 self.assertItemsEqual(['wordpress-stage', 'wordpress-prod'], keys)1522 self.assertItemsEqual(['wordpress-stage', 'wordpress-prod'], keys)
15141523
1515 def test_storing_a_bundle_includes_indexing_it(self):1524 def test_storing_a_bundle_includes_indexing_it(self):
@@ -1527,7 +1536,7 @@
1527 with patch(1536 with patch(
1528 'charmworld.models.ElasticSearchClient',1537 'charmworld.models.ElasticSearchClient',
1529 FauxElasticSearchClient):1538 FauxElasticSearchClient):
1530 store_bundles(self.db.bundles, {}, 'wordpress-5')1539 store_bundles(self.db.bundles, {}, 'owner', 'wordpress-basket/5')
15311540
1532 self.assertTrue(FauxElasticSearchClient.index_bundles_called)1541 self.assertTrue(FauxElasticSearchClient.index_bundles_called)
15331542
@@ -1547,12 +1556,14 @@
1547 'basket': 'mysql',1556 'basket': 'mysql',
1548 'name': 'tiny',1557 'name': 'tiny',
1549 'inherits': 'main-mysql',1558 'inherits': 'main-mysql',
1550 'series': 'series',
1551 'title': 'Tiny bundle',1559 'title': 'Tiny bundle',
1552 'description': 'My Tiny Bundle',1560 'description': 'My Tiny Bundle',
1553 'doctype': 'bundle',1561 'doctype': 'bundle',
1554 'services': dict(apache=1),1562 'data': {
1555 'relations': dict(a=1),1563 'services': dict(apache=1),
1564 'relations': dict(a=1),
1565 'series': 'series',
1566 },
1556 'promulgated': True,1567 'promulgated': True,
1557 'branch_deleted': True,1568 'branch_deleted': True,
1558 }1569 }
15591570
=== modified file 'charmworld/tests/test_search.py'
--- charmworld/tests/test_search.py 2013-08-09 18:53:40 +0000
+++ charmworld/tests/test_search.py 2013-08-11 23:10:56 +0000
@@ -5,12 +5,14 @@
55
66
7from datetime import datetime7from datetime import datetime
8from textwrap import dedent
89
9from pyelasticsearch import ElasticSearch10from pyelasticsearch import ElasticSearch
10from pyelasticsearch.exceptions import (11from pyelasticsearch.exceptions import (
11 ElasticHttpError,12 ElasticHttpError,
12 ElasticHttpNotFoundError,13 ElasticHttpNotFoundError,
13)14)
15import yaml
1416
15from charmworld.charmstore import (17from charmworld.charmstore import (
16 get_address,18 get_address,
@@ -19,6 +21,7 @@
19from charmworld.models import (21from charmworld.models import (
20 Bundle,22 Bundle,
21 Charm,23 Charm,
24 store_bundles,
22)25)
23from charmworld.search import (26from charmworld.search import (
24 BUNDLE,27 BUNDLE,
@@ -146,6 +149,38 @@
146 self.assertEqual(1, len(results))149 self.assertEqual(1, len(results))
147 self.assertEqual(bundle, results[0]['data'])150 self.assertEqual(bundle, results[0]['data'])
148151
152 def test_store_bundles(self):
153 # Ensure that bundles stored into the index via the `store_bundles`
154 # function are searchable via the index.
155 _id = '~abentley/wordpress-basket/5/wordpress-stage'
156 deployer_config = dedent("""\
157 wordpress-stage:
158 series: precise
159 services:
160 blog:
161 charm: cs:precise/wordpress
162 constraints: mem=2
163 options:
164 tuning: optimized
165 engine: apache
166 """)
167 parsed = yaml.safe_load(deployer_config)
168 client = self.index_client
169 store_bundles(None, parsed, 'abentley', 'wordpress-basket/5',
170 index_client=client)
171 # Bundle name is indexed.
172 results = client.search('wordpress-stage')['results']['bundle']
173 self.assertEqual(1, len(results))
174 self.assertEqual(_id, results[0]['data'].id)
175 # Series is indexed.
176 results = client.search('precise')['results']['bundle']
177 self.assertEqual(1, len(results))
178 self.assertEqual(_id, results[0]['data'].id)
179 # Owner is indexed.
180 results = client.search('abentley')['results']['bundle']
181 self.assertEqual(1, len(results))
182 self.assertEqual(_id, results[0]['data'].id)
183
149 def test_search_charms_and_bundles_same_name(self):184 def test_search_charms_and_bundles_same_name(self):
150 charm = Charm(self.makeCharm(name='mozilla'))185 charm = Charm(self.makeCharm(name='mozilla'))
151 bundle = Bundle(self.makeBundle(name='mozilla'))186 bundle = Bundle(self.makeBundle(name='mozilla'))
152187
=== modified file 'charmworld/views/api.py'
--- charmworld/views/api.py 2013-08-07 13:31:20 +0000
+++ charmworld/views/api.py 2013-08-11 23:10:56 +0000
@@ -351,11 +351,12 @@
351 if path is None:351 if path is None:
352 raise HTTPNotFound(self.request.path)352 raise HTTPNotFound(self.request.path)
353 fullpath = '/'.join(path)353 fullpath = '/'.join(path)
354 if len(path) == 3:354 if len(path) == 4:
355 # We have an owner.355 # We have an owner.
356 query = {'_id': fullpath}356 query = {'_id': fullpath}
357 elif len(path) == 2:357 elif len(path) == 3:
358 query = {'basket': path[0], 'name': path[1], 'promulgated': True}358 basket_id = join(path[0], path[1])
359 query = {'basket': basket_id, 'name': path[2], 'promulgated': True}
359 else:360 else:
360 raise HTTPNotFound(self.request.path)361 raise HTTPNotFound(self.request.path)
361 bundle_data = self.request.db.bundles.find_one(query)362 bundle_data = self.request.db.bundles.find_one(query)
@@ -363,7 +364,7 @@
363 return json_response(364 return json_response(
364 404, {'type': 'no_such_bundle', 'bundle_id': fullpath})365 404, {'type': 'no_such_bundle', 'bundle_id': fullpath})
365 bundle = Bundle(bundle_data)366 bundle = Bundle(bundle_data)
366 return bundle._representation # wrong367 return {bundle.name: bundle.data}
367368
368 @staticmethod369 @staticmethod
369 def _get_file_headers(md5sum, content_type=None):370 def _get_file_headers(md5sum, content_type=None):
@@ -371,7 +372,7 @@
371 ("Access-Control-Allow-Origin", "*"),372 ("Access-Control-Allow-Origin", "*"),
372 ("Access-Control-Allow-Headers", "X-Requested-With"),373 ("Access-Control-Allow-Headers", "X-Requested-With"),
373 ("Cache-Control", "max-age=86400, public"),374 ("Cache-Control", "max-age=86400, public"),
374 ("Etag", '"%s"' % md5sum)375 ("Etag", '"%s"' % md5sum),
375 ]376 ]
376 if content_type is not None:377 if content_type is not None:
377 headerlist.append(('Content-Type', content_type))378 headerlist.append(('Content-Type', content_type))
378379
=== modified file 'charmworld/views/tests/test_api.py'
--- charmworld/views/tests/test_api.py 2013-08-09 15:46:51 +0000
+++ charmworld/views/tests/test_api.py 2013-08-11 23:10:56 +0000
@@ -635,15 +635,16 @@
635 bundle = self.makeBundle()635 bundle = self.makeBundle()
636 expected = dict(636 expected = dict(
637 name=bundle.name,637 name=bundle.name,
638 owner='',638 owner=bundle.owner,
639 basket=bundle.basket,639 basket=bundle.basket,
640 inherits=None,640 inherits=None,
641 series='precise',
642 title='',641 title='',
642 data=dict(services=dict(),
643 relations=dict(),
644 series='precise',
645 ),
643 description='',646 description='',
644 doctype='bundle',647 doctype='bundle',
645 services=dict(),
646 relations=dict(),
647 promulgated=False,648 promulgated=False,
648 branch_deleted=False,649 branch_deleted=False,
649 )650 )
@@ -667,7 +668,7 @@
667668
668 def test_not_found(self):669 def test_not_found(self):
669 self.makeBundle(name='bat')670 self.makeBundle(name='bat')
670 response = self.get_response('bundle', 'foo/bar')671 response = self.get_response('bundle', 'foo/4/bar')
671 self.assertEqual(404, response.status_code)672 self.assertEqual(404, response.status_code)
672673
673 def test_no_path(self):674 def test_no_path(self):
@@ -675,32 +676,28 @@
675 self.get_response('bundle', remainder=None)676 self.get_response('bundle', remainder=None)
676677
677 def test_valid_lookup_unpromulgated(self):678 def test_valid_lookup_unpromulgated(self):
678 self.makeBundle(name='bat', owner='bac', basket='byobu-4')679 self.makeBundle(name='bat', owner='bac', basket='byobu/4')
679 response = self.get_response('bundle', '~bac/byobu-4/bat')680 response = self.get_response('bundle', '~bac/byobu/4/bat')
680 self.assertEqual(200, response.status_code)681 self.assertEqual(200, response.status_code)
681 response = self.get_response('bundle', 'byobu-4/bat')682 response = self.get_response('bundle', 'byobu/4/bat')
682 self.assertEqual(404, response.status_code)683 self.assertEqual(404, response.status_code)
683684
684 def test_valid_lookup_promulgated(self):685 def test_valid_lookup_promulgated(self):
685 self.makeBundle(686 self.makeBundle(
686 name='bat', owner='bac', basket='byobu-4',687 name='bat', owner='bac', basket='byobu/4',
687 promulgated=True)688 promulgated=True)
688 # Can look up via owner-based id and short id.689 # Can look up via owner-based id and short id.
689 response = self.get_response('bundle', '~bac/byobu-4/bat')690 response = self.get_response('bundle', '~bac/byobu/4/bat')
690 self.assertEqual(200, response.status_code)691 self.assertEqual(200, response.status_code)
691 response = self.get_response('bundle', 'byobu-4/bat')692 response = self.get_response('bundle', 'byobu/4/bat')
692 self.assertEqual(200, response.status_code)693 self.assertEqual(200, response.status_code)
693694
694 def test_results_match(self):695 def test_results_match(self):
695 self.maxDiff = None696 bundle = self.makeBundle(name='bat', owner='bac', basket='byobu/4')
696 bundle = self.makeBundle(name='bat', owner='bac', basket='byobu-4')697 response = self.get_response('bundle', '~bac/byobu/4/bat')
697 response = self.get_response('bundle', '~bac/byobu-4/bat')
698 self.assertEqual(200, response.status_code)698 self.assertEqual(200, response.status_code)
699 self.assertEqual(bundle.id, response.json['_id'])
700 # Copy the json dict so we can modify it.
701 json = dict(response.json)699 json = dict(response.json)
702 del json['_id']700 self.assertEqual(json['bat'], bundle.data)
703 self.assertEqual(json, bundle._representation)
704701
705702
706class TestAPI2Bundles(TestAPIBundles, API2Mixin):703class TestAPI2Bundles(TestAPIBundles, API2Mixin):

Subscribers

People subscribed via source and target branches