At http://bazaar.launchpad.net/~lifeless/bzr/apply-inventory-delta
------------------------------------------------------------ revno: 4531 revision-id: [email protected] parent: [email protected] committer: Robert Collins <[email protected]> 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): -- bazaar-commits mailing list [email protected] https://lists.ubuntu.com/mailman/listinfo/bazaar-commits
