save only regenerates manifests when told to; this guards against recreating manifests for invalid bags

Author: edsu

bagit.py

@@ -256,42 +256,53 @@ def payload_entries(self):
         return dict((key, value) for (key, value) in self.entries.iteritems() \
                      if key.startswith("data" + os.sep))
 
-    def save(self, processes=1):
+    def save(self, processes=1, manifests=False):
         """
-        If the contents of the payload directory have changed, this will update
-        the bag for the new or removed files. Any changes made to bag metadata 
-        will also be saved and tag manifests will be updated.
+        save will persist any changes that have been made to the bag 
+        metadata (self.info). 
+        
+        If you have modified the payload of the bag (added, modified, 
+        removed files in the data directory) and want to regenerate manifests
+        set the manifests parameter to True. The default is False since you 
+        wouldn't want a save to accidentally create a new manifest for 
+        a corrupted bag. 
+        
+        If you want to control the number of processes that are used when
+        recalculating checksums use the processes parameter.
         """
         # Error checking
         if not self.path:
             raise BagError("Bag does not have a path.")
-        unbaggable = _can_bag(self.path)
-        if unbaggable:
-            logging.error("no write permissions for the following directories and files: \n%s", unbaggable)
-            raise BagError("Not all files/folders can be moved.")
-        unreadable_dirs, unreadable_files = _can_read(self.path)
-        if unreadable_dirs or unreadable_files:
-            if unreadable_dirs:
-                logging.error("The following directories do not have read permissions: \n%s", unreadable_dirs)
-            if unreadable_files:
-                logging.error("The following files do not have read permissions: \n%s", unreadable_files)
-            raise BagError("Read permissions are required to calculate file fixities.")
 
         # Change working directory to bag directory so helper functions work
         old_dir = os.path.abspath(os.path.curdir)
         os.chdir(self.path)
 
         # Generate new manifest files
-        oxum = None
-        self.algs = list(set(self.algs))  # Dedupe
-        for alg in self.algs:
-            logging.info('updating manifest-%s.txt', alg)
-            oxum = _make_manifest('manifest-%s.txt' % alg, 'data', processes, alg)
+        if manifests:
+            unbaggable = _can_bag(self.path)
+            if unbaggable:
+                logging.error("no write permissions for the following directories and files: \n%s", unbaggable)
+                raise BagError("Not all files/folders can be moved.")
+            unreadable_dirs, unreadable_files = _can_read(self.path)
+            if unreadable_dirs or unreadable_files:
+                if unreadable_dirs:
+                    logging.error("The following directories do not have read permissions: \n%s", unreadable_dirs)
+                if unreadable_files:
+                    logging.error("The following files do not have read permissions: \n%s", unreadable_files)
+                raise BagError("Read permissions are required to calculate file fixities.")
+
+            oxum = None
+            self.algs = list(set(self.algs))  # Dedupe
+            for alg in self.algs:
+                logging.info('updating manifest-%s.txt', alg)
+                oxum = _make_manifest('manifest-%s.txt' % alg, 'data', processes, alg)
+
+            # Update Payload-Oxum
+            logging.info('updating %s', self.tag_file_name)
+            if oxum:
+                self.info['Payload-Oxum'] = oxum
 
-        # Update Payload-Oxum
-        logging.info('updating %s', self.tag_file_name)
-        if oxum:
-            self.info['Payload-Oxum'] = oxum
         _make_tag_file(self.tag_file_name, self.info)
 
         # Update tag-manifest for changes to manifest & bag-info files

test.py

@@ -432,19 +432,17 @@ def test_payload_permissions(self):
         self.assertTrue(perms != new_perms)
         os.chmod(self.tmpdir, new_perms)
         bag = bagit.make_bag(self.tmpdir)
-        payload_dir = os.path.join(self.tmpdir, 'data')
+        payload_dir = j(self.tmpdir, 'data')
         self.assertEqual(os.stat(payload_dir).st_mode, new_perms)
 
-    def test_save(self):
+    def test_save_manifests(self):
         bag = bagit.make_bag(self.tmpdir)
         self.assertTrue(bag.is_valid())
-        bag.save()
+        bag.save(manifests=True)
         self.assertTrue(bag.is_valid())
-        f = j(self.tmpdir, "data", "NEWFILE")
-        with open(f, 'w') as fp:
-            fp.write("NEWFILE")
+        open(j(self.tmpdir, "data", "newfile"), 'w').write('newfile')
         self.assertRaises(bagit.BagValidationError, bag.validate, bag, fast=False)
-        bag.save()
+        bag.save(manifests=True)
         self.assertTrue(bag.is_valid())
 
     def test_save_baginfo(self):
@@ -454,14 +452,25 @@ def test_save_baginfo(self):
         bag.save()
         bag = bagit.Bag(self.tmpdir)
         self.assertEqual(bag.info["foo"], "bar")
+        self.assertTrue(bag.is_valid())
 
         bag.info['x'] = ["a", "b", "c"]
         bag.save()
         b = bagit.Bag(self.tmpdir)
         self.assertEqual(b.info["x"], ["a", "b", "c"])
-
         self.assertTrue(bag.is_valid())
 
+    def test_save_only_baginfo(self):
+        bag = bagit.make_bag(self.tmpdir)
+        open(j(self.tmpdir, 'data', 'newfile'), 'w').write('newfile')
+        bag.info["foo"] = "bar"
+        bag.save()
+        
+        bag = bagit.Bag(self.tmpdir)
+        self.assertEqual(bag.info["foo"], "bar")
+        self.assertFalse(bag.is_valid())
+
+
     def test_make_bag_with_newline(self):
         bag = bagit.make_bag(self.tmpdir, {"test": "foo\nbar"})
         self.assertEqual(bag.info["test"], "foobar")