Rev 2918: Fix bug #152360. The xml5 serializer should be using in http://bzr.arbash-meinel.com/branches/bzr/0.92-dev/missing_root_revision_152360

[John Arbash Meinel]

At http://bzr.arbash-meinel.com/branches/bzr/0.92-dev/missing_root_revision_152360

------------------------------------------------------------
revno: 2918
revision-id: john at arbash-meinel.com-20071019171433-ko3319eemyhpb7kz
parent: pqm at pqm.ubuntu.com-20071019042839-xwvsz0loa77yokxm
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: missing_root_revision_152360
timestamp: Fri 2007-10-19 12:14:33 -0500
message:
  Fix bug #152360. The xml5 serializer should be using
  the hint it was given about the revision id.
  It was accidentally overwriting the revision_id to None when the
  data did not hold a valid value.
  Add tests that this is done properly, at both the Repository level
  and at the xml5 layer.
modified:
  bzrlib/tests/test_repository.py test_repository.py-20060131075918-65c555b881612f4d
  bzrlib/tests/test_xml.py       test_xml.py-20050905091053-80b45588931a9b35
  bzrlib/xml5.py                 xml5.py-20050907032657-aac8f960815b66b1
-------------- next part --------------
=== modified file 'bzrlib/tests/test_repository.py'
--- a/bzrlib/tests/test_repository.py	2007-10-05 05:52:45 +0000
+++ b/bzrlib/tests/test_repository.py	2007-10-19 17:14:33 +0000
@@ -373,6 +373,31 @@
         self.assertRaises(errors.OutSideTransaction,
             inv.add_lines, 'foo', [], [])
 
+    def test_deserialise_sets_root_revision(self):
+        """We must have a inventory.root.revision
+
+        Old versions of the XML5 serializer did not set the revision_id for
+        the whole inventory. So we grab the one from the expected text. Which
+        is valid when the api is not being abused.
+        """
+        repo = self.make_repository('.',
+                format=bzrdir.format_registry.get('knit')())
+        inv_xml = '<inventory format="5">\n</inventory>\n'
+        inv = repo.deserialise_inventory('test-rev-id', inv_xml)
+        self.assertEqual('test-rev-id', inv.root.revision)
+
+    def test_deserialise_uses_global_revision_id(self):
+        """If it is set, then we re-use the global revision id"""
+        repo = self.make_repository('.',
+                format=bzrdir.format_registry.get('knit')())
+        inv_xml = ('<inventory format="5" revision_id="other-rev-id">\n'
+                   '</inventory>\n')
+        # Arguably, the deserialise_inventory should detect a mismatch, and
+        # raise an error, rather than silently using one revision_id over the
+        # other.
+        inv = repo.deserialise_inventory('test-rev-id', inv_xml)
+        self.assertEqual('other-rev-id', inv.root.revision)
+
 
 class KnitRepositoryStreamTests(test_knit.KnitTests):
     """Tests for knitrepo._get_stream_as_bytes."""

=== modified file 'bzrlib/tests/test_xml.py'
--- a/bzrlib/tests/test_xml.py	2007-10-05 02:41:37 +0000
+++ b/bzrlib/tests/test_xml.py	2007-10-19 17:14:33 +0000
@@ -175,6 +175,16 @@
 </inventory>
 """
 
+# Before revision_id was always stored as an attribute
+_inventory_v5a = """<inventory format="5">
+</inventory>
+"""
+
+# Before revision_id was always stored as an attribute
+_inventory_v5b = """<inventory format="5" revision_id="a-rev-id">
+</inventory>
+"""
+
 
 class TestSerializer(TestCase):
     """Test XML serialization"""
@@ -247,6 +257,16 @@
         eq(ie.name, 'bar')
         eq(inv[ie.parent_id].kind, 'directory')
 
+    def test_unpack_inventory_5a(self):
+        inv = bzrlib.xml5.serializer_v5.read_inventory_from_string(
+                _inventory_v5a, revision_id='test-rev-id')
+        self.assertEqual('test-rev-id', inv.root.revision)
+
+    def test_unpack_inventory_5b(self):
+        inv = bzrlib.xml5.serializer_v5.read_inventory_from_string(
+                _inventory_v5b, revision_id='test-rev-id')
+        self.assertEqual('a-rev-id', inv.root.revision)
+
     def test_repack_inventory_5(self):
         inp = StringIO(_committed_inv_v5)
         inv = bzrlib.xml5.serializer_v5.read_inventory(inp)

=== modified file 'bzrlib/xml5.py'
--- a/bzrlib/xml5.py	2007-10-05 02:41:37 +0000
+++ b/bzrlib/xml5.py	2007-10-19 17:14:33 +0000
@@ -348,9 +348,9 @@
             if format != '5':
                 raise BzrError("invalid format version %r on inventory"
                                 % format)
-        revision_id = elt.get('revision_id')
-        if revision_id is not None:
-            revision_id = cache_utf8.encode(revision_id)
+        data_revision_id = elt.get('revision_id')
+        if data_revision_id is not None:
+            revision_id = cache_utf8.encode(data_revision_id)
         inv = Inventory(root_id, revision_id=revision_id)
         for e in elt:
             ie = self._unpack_entry(e)