Merge lp:~abentley/charmworld/remove-redudant-id into lp:~juju-jitsu/charmworld/trunk

[Aaron Bentley]

_id is not added to bundle's data attribute while ingesting.

Fix bug #1208481: bundles contain redundant _id in their data

Currently, _id is added to a bundle's data attribute, but this is wrong-- _id should be stored at the root level of the bundle, and already is. So this code can be deleted.

[Curtis Hovey (sinzui)]

Thank you.

review: Approve (code)

=== modified file 'charmworld/jobs/ingest.py'
--- charmworld/jobs/ingest.py	2013-08-13 14:46:22 +0000
+++ charmworld/jobs/ingest.py	2013-08-16 19:30:25 +0000
@@ -298,12 +298,6 @@
         data['name_revno'] = "%s/%d" % (data['name'], revno)
         data['_id'] = '%s/%s' % (owner_segment, data['name_revno'])
 
-    @staticmethod
-    def decorate_bundles(basket_contents, basket_id):
-        # Give each bundle a unique ID.
-        for bundle_name, bundle in basket_contents.items():
-            bundle['_id'] = '%s/%s' % (basket_id, bundle_name)
-
     def decorate_basket(self, basket_data, fs):
         branch_dir = fetch_branch(self.working_dir, basket_data, self.log)
         branch = Branch.open(branch_dir)
@@ -325,7 +319,6 @@
         self.decorate_basket(basket_data, fs)
         self.db.baskets.save(basket_data)
         deployer_config = self.get_deployer_config(fs, basket_data)
-        self.decorate_bundles(deployer_config, basket_data['_id'])
         self.store_bundles(
             deployer_config, basket_data['owner'], basket_data['name_revno'])
 

=== modified file 'charmworld/jobs/tests/test_ingest.py'
--- charmworld/jobs/tests/test_ingest.py	2013-08-14 15:12:04 +0000
+++ charmworld/jobs/tests/test_ingest.py	2013-08-16 19:30:25 +0000
@@ -264,10 +264,10 @@
     def test_run_updates_mongo(self):
         # Running a bundle job creates a bundle document in mongo.
         NAME = 'our-source-package'
-        bundle_data = factory.get_payload_json(name=NAME)
+        basket_data = factory.get_payload_json(name=NAME)
         with bundle_update_environment():
-            run_job(UpdateBundleJob(), bundle_data, db=self.db)
-            self.assertIsNotNone(self.db.baskets.find_one(bundle_data['_id']))
+            run_job(UpdateBundleJob(), basket_data, db=self.db)
+            self.assertIsNotNone(self.db.baskets.find_one(basket_data['_id']))
 
     def test_run_creates_branch(self):
         # The ingest job fetches the target branch.
@@ -399,11 +399,6 @@
         self.assertIsNot(None, bundles_hash)
         self.assertEqual('{foo: {}}', fs.get(bundles_hash).read())
 
-    def test_decorate_bundles(self):
-        basket_contents = {'foo': {}}
-        UpdateBundleJob.decorate_bundles(basket_contents, '~bar/baz')
-        self.assertEqual({'foo': {'_id': '~bar/baz/foo'}}, basket_contents)
-
     def test_missing_revisions_generate_errors(self):
         job = UpdateBundleJob()
         job.setup(self.db)
@@ -472,6 +467,19 @@
                     job.run(basket_data)
                     self.assertTrue(store_bundles.called)
 
+    def test_job_run_does_not_alter_bundle_data(self):
+        basket_data = factory.get_payload_json(name='wordpress')
+        basket_data['name_revno'] = 'dummy'
+        job = UpdateBundleJob()
+        job.setup(self.db)
+        with patch.object(job, 'get_deployer_config',
+                          side_effect=lambda x, y: {'foo': {}}):
+            with patch.object(job, 'decorate_basket'):
+                with patch.object(job, 'store_bundles') as store_bundles:
+                    job.run(basket_data)
+                    store_bundles.assert_called_with(
+                        {'foo': {}}, 'charmers', 'dummy')
+
 
 class TestUpdateCharm(MongoTestBase):