Rev 4535: Add interface tests for dangling children in inventory deltas. in http://bazaar.launchpad.net/~lifeless/bzr/apply-inventory-delta

[Robert Collins]

At http://bazaar.launchpad.net/~lifeless/bzr/apply-inventory-delta

------------------------------------------------------------
revno: 4535
revision-id: robertc at robertcollins.net-20090714051723-4cmkyblxwe90cb6a
parent: robertc at robertcollins.net-20090714045319-r30rduhsss03ngfy
committer: Robert Collins <robertc at robertcollins.net>
branch nick: apply-inventory-delta
timestamp: Tue 2009-07-14 15:17:23 +1000
message:
  Add interface tests for dangling children in inventory deltas.
=== modified file 'NEWS'
--- a/NEWS	2009-07-14 01:43:03 +0000
+++ b/NEWS	2009-07-14 05:17:23 +0000
@@ -39,6 +39,10 @@
 * ``CHKMap.apply_delta`` now raises ``InconsistentDelta`` if a delta adds
   as new a key which was already mapped. (Robert Collins)
 
+* Inventory delta application catches more cases of corruption and can
+  prevent corrupt deltas from affecting consistency of data structures on
+  disk. (Robert Collins)
+
 
 bzr 1.17rc1 "So late it's brunch" 2009-07-13
 ############################################

=== modified file 'bzrlib/inventory.py'
--- a/bzrlib/inventory.py	2009-07-14 00:20:03 +0000
+++ b/bzrlib/inventory.py	2009-07-14 05:17:23 +0000
@@ -1646,6 +1646,12 @@
         # All changed entries need to have their parents be directories and be
         # at the right path. This set contains (path, id) tuples.
         parents = set()
+        # When we delete an item, all the children of it must be either deleted
+        # or altered in their own right. As we batch process the change via
+        # CHKMap.apply_delta, we build a set of things to use to validate the
+        # delta.
+        deletes = set()
+        altered = set()
         for old_path, new_path, file_id, entry in inventory_delta:
             # file id changes
             if new_path == '':
@@ -1660,6 +1666,7 @@
                         del result._path_to_fileid_cache[old_path]
                     except KeyError:
                         pass
+                deletes.add(file_id)
             else:
                 new_key = (file_id,)
                 new_value = result._entry_to_bytes(entry)
@@ -1675,6 +1682,7 @@
                     raise errors.InconsistentDelta(old_path, file_id,
                         "Entry was at wrong other path %r." %
                         self.id2path(file_id))
+                altered.add(file_id)
             id_to_entry_delta.append((old_key, new_key, new_value))
             if result.parent_id_basename_to_file_id is not None:
                 # parent_id, basename changes
@@ -1693,6 +1701,18 @@
                     # If the two keys are the same, the value will be unchanged
                     # as its always the file id.
                     parent_id_basename_delta.append((old_key, new_key, new_value))
+        # validate that deletes are complete.
+        for file_id in deletes:
+            entry = self[file_id]
+            if entry.kind != 'directory':
+                continue
+            # This loop could potentially be better by using the id_basename
+            # map to just get the child file ids.
+            for child in entry.children.values():
+                if child.file_id not in altered:
+                    raise errors.InconsistentDelta(self.id2path(child.file_id),
+                        child.file_id, "Child not deleted or reparented when "
+                        "parent deleted.")
         result.id_to_entry.apply_delta(id_to_entry_delta)
         if parent_id_basename_delta:
             result.parent_id_basename_to_file_id.apply_delta(parent_id_basename_delta)

=== modified file 'bzrlib/tests/test_inv.py'
--- a/bzrlib/tests/test_inv.py	2009-07-14 04:53:19 +0000
+++ b/bzrlib/tests/test_inv.py	2009-07-14 05:17:23 +0000
@@ -462,6 +462,22 @@
         self.assertRaises(errors.InconsistentDelta, self.apply_delta, self,
             inv, delta)
 
+    def test_remove_dir_leaving_dangling_child(self):
+        inv = self.get_empty_inventory()
+        dir1 = inventory.InventoryDirectory('p-1', 'dir1', inv.root.file_id)
+        dir1.revision = 'result'
+        dir2 = inventory.InventoryDirectory('p-2', 'child1', 'p-1')
+        dir2.revision = 'result'
+        dir3 = inventory.InventoryDirectory('p-3', 'child2', 'p-1')
+        dir3.revision = 'result'
+        inv.add(dir1)
+        inv.add(dir2)
+        inv.add(dir3)
+        delta = [(u'dir1', None, 'p-1', None),
+            (u'dir1/child2', None, 'p-3', None)]
+        self.assertRaises(errors.InconsistentDelta, self.apply_delta, self,
+            inv, delta)
+
 
 class TestInventoryEntry(TestCase):