Rev 4131: check for duplicate file-ids in Inventory.apply_delta (Ian Clatworthy) in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

Canonical.com Patch Queue Manager pqm at pqm.ubuntu.com
Thu Mar 12 10:40:11 GMT 2009


At file:///home/pqm/archives/thelove/bzr/%2Btrunk/

------------------------------------------------------------
revno: 4131
revision-id: pqm at pqm.ubuntu.com-20090312104001-k2722d61gjdpb91v
parent: pqm at pqm.ubuntu.com-20090312095736-fizxxpwp4gj9zqx9
parent: ian.clatworthy at canonical.com-20090312081218-axutugn3qc89nc3d
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Thu 2009-03-12 10:40:01 +0000
message:
  check for duplicate file-ids in Inventory.apply_delta (Ian Clatworthy)
modified:
  NEWS                           NEWS-20050323055033-4e00b5db738777ff
  bzrlib/inventory.py            inventory.py-20050309040759-6648b84ca2005b37
  bzrlib/tests/inventory_implementations/basics.py basics.py-20070903044446-kdjwbiu1p1zi9phs-1
    ------------------------------------------------------------
    revno: 4127.1.1
    revision-id: ian.clatworthy at canonical.com-20090312081218-axutugn3qc89nc3d
    parent: pqm at pqm.ubuntu.com-20090312075222-g575i2pcrliafetw
    parent: ian.clatworthy at canonical.com-20090312080141-63gsbiotrvgnxdpg
    committer: Ian Clatworthy <ian.clatworthy at canonical.com>
    branch nick: ianc-integration
    timestamp: Thu 2009-03-12 18:12:18 +1000
    message:
      check for duplicate file-ids in Inventory.apply_delta (Ian Clatworthy)
    modified:
      NEWS                           NEWS-20050323055033-4e00b5db738777ff
      bzrlib/inventory.py            inventory.py-20050309040759-6648b84ca2005b37
      bzrlib/tests/inventory_implementations/basics.py basics.py-20070903044446-kdjwbiu1p1zi9phs-1
    ------------------------------------------------------------
    revno: 4090.3.2
    revision-id: ian.clatworthy at canonical.com-20090312080141-63gsbiotrvgnxdpg
    parent: ian.clatworthy at canonical.com-20090309055106-gan5ohyjamvxu4x4
    committer: Ian Clatworthy <ian.clatworthy at canonical.com>
    branch nick: bzr.apply-delta-checking
    timestamp: Thu 2009-03-12 18:01:41 +1000
    message:
      review tweak from jam
    modified:
      bzrlib/inventory.py            inventory.py-20050309040759-6648b84ca2005b37
    ------------------------------------------------------------
    revno: 4090.3.1
    revision-id: ian.clatworthy at canonical.com-20090309055106-gan5ohyjamvxu4x4
    parent: pqm at pqm.ubuntu.com-20090308222317-d1nw017gzfen1aqs
    committer: Ian Clatworthy <ian.clatworthy at canonical.com>
    branch nick: bzr.apply-delta-checking
    timestamp: Mon 2009-03-09 15:51:06 +1000
    message:
      check delta is legal in Inventory.apply_delta()
    modified:
      NEWS                           NEWS-20050323055033-4e00b5db738777ff
      bzrlib/inventory.py            inventory.py-20050309040759-6648b84ca2005b37
      bzrlib/tests/inventory_implementations/basics.py basics.py-20070903044446-kdjwbiu1p1zi9phs-1
=== modified file 'NEWS'
--- a/NEWS	2009-03-12 09:57:36 +0000
+++ b/NEWS	2009-03-12 10:40:01 +0000
@@ -223,6 +223,9 @@
       UI at all - indeed it only reflects the repository status not
       changes to the branch itself. (Robert Collins)
 
+    * ``Inventory.apply_delta`` now raises an AssertionError if a file-id
+      appears multiple times within the delta. (Ian Clatworthy)
+
     * MutableTree.commit now favours the "authors" argument, with the old
       "author" argument being deprecated.
 

=== modified file 'bzrlib/inventory.py'
--- a/bzrlib/inventory.py	2009-01-17 01:30:58 +0000
+++ b/bzrlib/inventory.py	2009-03-12 08:01:41 +0000
@@ -806,7 +806,19 @@
             change regardless. E.g. in the recursive deletion of a directory -
             the directory's children must be included in the delta, or the
             final inventory will be invalid.
+
+            Note that a file_id must only appear once within a given delta.
+            An AssertionError is raised otherwise.
         """
+        # Check that the delta is legal. It would be nice if this could be
+        # done within the loops below but it's safer to validate the delta
+        # before starting to mutate the inventory.
+        unique_file_ids = set([f for _, _, f, _ in delta])
+        if len(unique_file_ids) != len(delta):
+            raise AssertionError("a file-id appears multiple times in %r"
+                    % (delta,))
+        del unique_file_ids
+
         children = {}
         # Remove all affected items which were in the original inventory,
         # starting with the longest paths, thus ensuring parents are examined

=== modified file 'bzrlib/tests/inventory_implementations/basics.py'
--- a/bzrlib/tests/inventory_implementations/basics.py	2009-03-07 06:58:17 +0000
+++ b/bzrlib/tests/inventory_implementations/basics.py	2009-03-12 08:12:18 +0000
@@ -37,12 +37,14 @@
         )
 
 
-class TestInventoryBasics(TestCase):
-    # Most of these were moved the rather old bzrlib.tests.test_inv module
+class TestInventory(TestCase):
 
     def make_inventory(self, root_id):
         return self.inventory_class(root_id=root_id)
 
+
+class TestInventoryUpdates(TestInventory):
+
     def test_creation_from_root_id(self):
         # iff a root id is passed to the constructor, a root directory is made
         inv = self.make_inventory(root_id='tree-root')
@@ -88,19 +90,6 @@
         self.assertEquals('someroot', inv2.root.file_id)
         self.assertEquals('therev', inv2.root.revision)
 
-    def test_is_root(self):
-        """Ensure our root-checking code is accurate."""
-        inv = self.make_inventory('TREE_ROOT')
-        self.assertTrue(inv.is_root('TREE_ROOT'))
-        self.assertFalse(inv.is_root('booga'))
-        inv.root.file_id = 'booga'
-        self.assertFalse(inv.is_root('TREE_ROOT'))
-        self.assertTrue(inv.is_root('booga'))
-        # works properly even if no root is set
-        inv.root = None
-        self.assertFalse(inv.is_root('TREE_ROOT'))
-        self.assertFalse(inv.is_root('booga'))
-
     def test_create_tree_reference(self):
         inv = self.make_inventory('tree-root-123')
         inv.add(TreeReference('nested-id', 'nested', parent_id='tree-root-123',
@@ -116,6 +105,69 @@
         else:
             self.fail('BzrError not raised')
 
+    def test_add_recursive(self):
+        parent = InventoryDirectory('src-id', 'src', 'tree-root')
+        child = InventoryFile('hello-id', 'hello.c', 'src-id')
+        parent.children[child.file_id] = child
+        inv = self.make_inventory('tree-root')
+        inv.add(parent)
+        self.assertEqual('src/hello.c', inv.id2path('hello-id'))
+
+
+class TestInventoryApplyDelta(TestInventory):
+
+    def test_apply_delta_add(self):
+        inv = self.make_inventory('tree-root')
+        inv.apply_delta([
+            (None, "a", "a-id", InventoryFile('a-id', 'a', 'tree-root')),
+            ])
+        self.assertEqual('a', inv.id2path('a-id'))
+
+    def test_apply_delta_delete(self):
+        inv = self.make_inventory('tree-root')
+        inv.apply_delta([
+            (None, "a", "a-id", InventoryFile('a-id', 'a', 'tree-root')),
+            ])
+        self.assertEqual('a', inv.id2path('a-id'))
+        a_ie = inv['a-id']
+        inv.apply_delta([("a", None, "a-id", a_ie)])
+        self.assertRaises(errors.NoSuchId, inv.id2path, 'a-id')
+
+    def test_apply_delta_rename(self):
+        inv = self.make_inventory('tree-root')
+        inv.apply_delta([
+            (None, "a", "a-id", InventoryFile('a-id', 'a', 'tree-root')),
+            ])
+        self.assertEqual('a', inv.id2path('a-id'))
+        a_ie = inv['a-id']
+        b_ie = InventoryFile(a_ie.file_id, "b", a_ie.parent_id)
+        inv.apply_delta([("a", "b", "a-id", b_ie)])
+        self.assertEqual("b", inv.id2path('a-id'))
+
+    def test_apply_delta_illegal(self):
+        # A file-id cannot appear in a delta more than once
+        inv = self.make_inventory('tree-root')
+        self.assertRaises(AssertionError, inv.apply_delta, [
+            ("a", "a", "id-1", InventoryFile('id-1', 'a', 'tree-root')),
+            ("a", "b", "id-1", InventoryFile('id-1', 'b', 'tree-root')),
+            ])
+
+
+class TestInventoryReads(TestInventory):
+
+    def test_is_root(self):
+        """Ensure our root-checking code is accurate."""
+        inv = self.make_inventory('TREE_ROOT')
+        self.assertTrue(inv.is_root('TREE_ROOT'))
+        self.assertFalse(inv.is_root('booga'))
+        inv.root.file_id = 'booga'
+        self.assertFalse(inv.is_root('TREE_ROOT'))
+        self.assertTrue(inv.is_root('booga'))
+        # works properly even if no root is set
+        inv.root = None
+        self.assertFalse(inv.is_root('TREE_ROOT'))
+        self.assertFalse(inv.is_root('booga'))
+
     def test_ids(self):
         """Test detection of files within selected directories."""
         inv = self.make_inventory(ROOT_ID)
@@ -231,11 +283,3 @@
             ('src/bye.c', 'bye-id'),
             ], [(path, ie.file_id) for path, ie in inv.iter_entries_by_dir(
                 specific_file_ids=('bye-id',), yield_parents=True)])
-
-    def test_add_recursive(self):
-        parent = InventoryDirectory('src-id', 'src', 'tree-root')
-        child = InventoryFile('hello-id', 'hello.c', 'src-id')
-        parent.children[child.file_id] = child
-        inv = self.make_inventory('tree-root')
-        inv.add(parent)
-        self.assertEqual('src/hello.c', inv.id2path('hello-id'))




More information about the bazaar-commits mailing list