Rev 4531: Require that added ids in inventory deltas be new. in http://bazaar.launchpad.net/~lifeless/bzr/apply-inventory-delta
Robert Collins
robertc at robertcollins.net
Tue Jul 14 01:20:11 BST 2009
At http://bazaar.launchpad.net/~lifeless/bzr/apply-inventory-delta
------------------------------------------------------------
revno: 4531
revision-id: robertc at robertcollins.net-20090714002003-ingqr8vlkz5577v1
parent: robertc at robertcollins.net-20090713054406-py4vt6uns5yyoya0
committer: Robert Collins <robertc at robertcollins.net>
branch nick: apply-inventory-delta
timestamp: Tue 2009-07-14 10:20:03 +1000
message:
Require that added ids in inventory deltas be new.
=== modified file 'bzrlib/chk_map.py'
--- a/bzrlib/chk_map.py 2009-07-01 10:52:23 +0000
+++ b/bzrlib/chk_map.py 2009-07-14 00:20:03 +0000
@@ -26,9 +26,9 @@
Updates to a CHKMap are done preferentially via the apply_delta method, to
allow optimisation of the update operation; but individual map/unmap calls are
-possible and supported. All changes via map/unmap are buffered in memory until
-the _save method is called to force serialisation of the tree. apply_delta
-performs a _save implicitly.
+possible and supported. Individual changes via map/unmap are buffered in memory
+until the _save method is called to force serialisation of the tree.
+apply_delta records its changes immediately by performing an implicit _save.
TODO:
-----
@@ -41,7 +41,10 @@
from bzrlib import lazy_import
lazy_import.lazy_import(globals(), """
-from bzrlib import versionedfile
+from bzrlib import (
+ errors,
+ versionedfile,
+ )
""")
from bzrlib import (
lru_cache,
@@ -105,6 +108,14 @@
of old_key is removed.
"""
delete_count = 0
+ # Check preconditions first.
+ new_items = set([key for (old, key, value) in delta if key is not None
+ and old is None])
+ existing_new = list(self.iteritems(key_filter=new_items))
+ if existing_new:
+ raise errors.InconsistentDeltaDelta(delta,
+ "New items are already in the map %r." % existing_new)
+ # Now apply changes.
for old, new, value in delta:
if old is not None and old != new:
self.unmap(old, check_remap=False)
@@ -482,7 +493,11 @@
return len(self._root_node)
def map(self, key, value):
- """Map a key tuple to value."""
+ """Map a key tuple to value.
+
+ :param key: A key to map.
+ :param value: The value to assign to key.
+ """
# Need a root object.
self._ensure_root()
prefix, node_details = self._root_node.map(self._store, key, value)
=== modified file 'bzrlib/dirstate.py'
--- a/bzrlib/dirstate.py 2009-07-13 05:44:06 +0000
+++ b/bzrlib/dirstate.py 2009-07-14 00:20:03 +0000
@@ -1293,6 +1293,9 @@
# references from every item in the delta that is not a deletion and
# is not itself the root.
parents = set()
+ # Added ids must not be in the dirstate already. This set holds those
+ # ids.
+ new_ids = set()
for old_path, new_path, file_id, inv_entry in sorted(
inventory._check_delta_unique_old_paths(
inventory._check_delta_unique_new_paths(
@@ -1306,6 +1309,8 @@
if old_path is not None:
old_path = old_path.encode('utf-8')
removals[file_id] = old_path
+ else:
+ new_ids.add(file_id)
if new_path is not None:
if inv_entry is None:
raise errors.InconsistentDelta(new_path, file_id,
@@ -1343,6 +1348,7 @@
child_basename)
insertions[child[0][2]] = (key, minikind, executable,
fingerprint, new_child_path)
+ self._check_delta_ids_absent(new_ids, delta, 0)
try:
self._apply_removals(removals.iteritems())
self._apply_insertions(insertions.values())
@@ -1454,6 +1460,9 @@
# references from every item in the delta that is not a deletion and
# is not itself the root.
parents = set()
+ # Added ids must not be in the dirstate already. This set holds those
+ # ids.
+ new_ids = set()
for old_path, new_path, file_id, inv_entry in delta:
if inv_entry is not None and file_id != inv_entry.file_id:
raise errors.InconsistentDelta(new_path, file_id,
@@ -1470,6 +1479,7 @@
if old_path is None:
adds.append((None, encode(new_path), file_id,
inv_to_entry(inv_entry), True))
+ new_ids.add(file_id)
elif new_path is None:
deletes.append((encode(old_path), None, file_id, None, True))
elif (old_path, new_path) != root_only:
@@ -1518,7 +1528,7 @@
# of everything.
changes.append((encode(old_path), encode(new_path), file_id,
inv_to_entry(inv_entry)))
-
+ self._check_delta_ids_absent(new_ids, delta, 1)
try:
# Finish expunging deletes/first half of renames.
self._update_basis_apply_deletes(deletes)
@@ -1544,6 +1554,29 @@
self._id_index = None
return
+ def _check_delta_ids_absent(self, new_ids, delta, tree_index):
+ """Check that none of the file_ids in new_ids are present in a tree."""
+ if not new_ids:
+ return
+ id_index = self._get_id_index()
+ for file_id in new_ids:
+ for key in id_index.get(file_id, []):
+ block_i, entry_i, d_present, f_present = \
+ self._get_block_entry_index(key[0], key[1], tree_index)
+ if not f_present:
+ # In a different tree
+ continue
+ entry = self._dirblocks[block_i][1][entry_i]
+ if entry[0][2] != file_id:
+ # Different file_id, so not what we want.
+ continue
+ # NB: No changes made before this helper is called, so no need
+ # to set the _changes_aborted flag.
+ raise errors.InconsistentDelta(
+ ("%s/%s" % key[0:2]).decode('utf8'), file_id,
+ "This file_id is new in the delta but already present in "
+ "the target")
+
def _update_basis_apply_adds(self, adds):
"""Apply a sequence of adds to tree 1 during update_basis_by_delta.
=== modified file 'bzrlib/inventory.py'
--- a/bzrlib/inventory.py 2009-07-13 05:44:06 +0000
+++ b/bzrlib/inventory.py 2009-07-14 00:20:03 +0000
@@ -1170,6 +1170,9 @@
new_entry = replacement
try:
self.add(new_entry)
+ except errors.DuplicateFileId:
+ raise errors.InconsistentDelta(new_path, new_entry.file_id,
+ "New id is already present in target.")
except AttributeError:
raise errors.InconsistentDelta(new_path, new_entry.file_id,
"Parent is not a directory.")
=== modified file 'bzrlib/tests/test_chk_map.py'
--- a/bzrlib/tests/test_chk_map.py 2009-06-26 09:24:34 +0000
+++ b/bzrlib/tests/test_chk_map.py 2009-07-14 00:20:03 +0000
@@ -20,6 +20,7 @@
from bzrlib import (
chk_map,
+ errors,
osutils,
tests,
)
@@ -228,6 +229,18 @@
# updated key.
self.assertEqual(new_root, chkmap._root_node._key)
+ def test_apply_new_keys_must_be_new(self):
+ # applying a delta (None, "a", "b") to a map with 'a' in it generates
+ # an error.
+ chk_bytes = self.get_chk_bytes()
+ root_key = CHKMap.from_dict(chk_bytes, {("a",):"b"})
+ chkmap = CHKMap(chk_bytes, root_key)
+ self.assertRaises(errors.InconsistentDelta, chkmap.apply_delta,
+ [(None, ("a",), "b")])
+ # As an error occured, the update should have left us without changing
+ # anything (the root should be unchanged).
+ self.assertEqual(root_key, chkmap._root_node._key)
+
def test_apply_delta_is_deterministic(self):
chk_bytes = self.get_chk_bytes()
chkmap1 = CHKMap(chk_bytes, None)
=== modified file 'bzrlib/tests/test_inv.py'
--- a/bzrlib/tests/test_inv.py 2009-07-13 05:44:06 +0000
+++ b/bzrlib/tests/test_inv.py 2009-07-14 00:20:03 +0000
@@ -440,6 +440,17 @@
self.assertRaises(errors.InconsistentDelta, self.apply_delta, self,
inv, delta)
+ def test_add_existing_id_new_path(self):
+ inv = self.get_empty_inventory()
+ parent1 = inventory.InventoryDirectory('p-1', 'dir1', inv.root.file_id)
+ parent1.revision = 'result'
+ parent2 = inventory.InventoryDirectory('p-1', 'dir2', inv.root.file_id)
+ parent2.revision = 'result'
+ inv.add(parent1)
+ delta = [(None, u'dir2', 'p-1', parent2)]
+ self.assertRaises(errors.InconsistentDelta, self.apply_delta, self,
+ inv, delta)
+
class TestInventoryEntry(TestCase):
More information about the bazaar-commits
mailing list