Clone id handling changes

CHANGELOG.md

@@ -24,6 +24,19 @@ XML files via lxml. See #301.
 The `odmlconversion` convenience console script has been added to convert multiple 
 previous odML version files to the latest odML version.
 
+## Changes in cloning behaviour
+
+When cloning a `Section` or a `Property` by default the id of any object is changed
+to a new UUID. The cloning methods now feature a new `keep_id` attribute. If set to
+`True`, the cloned object and any cloned children retain their original id. This
+is meant to create exact copies of Section-Property trees in different documents.
+
+## Additional validation
+
+When a document is saved, a new validation check makes sure, that a document
+contains only unique UUIDs this is required due to the introduction of creating
+clones with identical ids. 
+
 
 # Version 1.4.1
 

odml/base.py

@@ -581,7 +581,7 @@ def clean(self):
         for i in self:
             i.clean()
 
-    def clone(self, children=True):
+    def clone(self, children=True, keep_id=False):
         """
         Clone this object recursively allowing to copy it independently
         to another document
@@ -592,7 +592,7 @@ def clone(self, children=True):
         obj._sections = SmartList(BaseSection)
         if children:
             for s in self._sections:
-                obj.append(s.clone())
+                obj.append(s.clone(keep_id=keep_id))
 
         return obj
 

odml/property.py

@@ -407,15 +407,20 @@ def get_path(self):
 
         return self.parent.get_path() + ":" + self.name
 
-    def clone(self):
+    def clone(self, keep_id=False):
         """
-        Clone this object to copy it independently to another document.
-        The id of the cloned object will be set to a different uuid.
+        Clone this property to copy it independently to another document.
+        By default the id of the cloned object will be set to a different uuid.
+
+        :param keep_id: If this attribute is set to True, the uuid of the
+                        object will remain unchanged.
+        :return: The cloned property
         """
         obj = super(BaseProperty, self).clone()
         obj._parent = None
         obj.value = self._value
-        obj._id = str(uuid.uuid4())
+        if not keep_id:
+            obj.new_id()
 
         return obj
 

odml/section.py

@@ -390,18 +390,26 @@ def remove(self, obj):
         else:
             raise ValueError("Can only remove sections and properties")
 
-    def clone(self, children=True):
+    def clone(self, children=True, keep_id=False):
         """
-        Clone this object recursively allowing to copy it independently
-        to another document
+        Clone this Section allowing to copy it independently
+        to another document. By default the id of any cloned
+        object will be set to a new uuid.
+
+        :param children: If True, also clone child sections and properties
+                         recursively.
+        :param keep_id: If this attribute is set to True, the uuids of the
+                        Section and all child objects will remain unchanged.
+        :return: The cloned Section.
         """
-        obj = super(BaseSection, self).clone(children)
-        obj._id = str(uuid.uuid4())
+        obj = super(BaseSection, self).clone(children, keep_id)
+        if not keep_id:
+            obj.new_id()
 
         obj._props = base.SmartList(BaseProperty)
         if children:
             for p in self._props:
-                obj.append(p.clone())
+                obj.append(p.clone(keep_id))
 
         return obj
 

odml/tools/odmlparser.py

@@ -50,7 +50,7 @@ def write_file(self, odml_document, filename):
         msg = ""
         for err in validation.errors:
             if err.is_error:
-                msg += "\n\t- %s %s: %s" % (err.obj, err.type, err.msg)
+                msg += "\n\t- %s %s: %s" % (err.obj, err.rank, err.msg)
         if msg != "":
             msg = "Resolve document validation errors before saving %s" % msg
             raise ParserException(msg)

odml/validation.py

@@ -174,7 +174,7 @@ def section_unique_ids(parent, id_map=None):
             yield i
 
         if sec.id in id_map:
-            yield ValidationError(sec, "Duplicate id in Section '%s' and '%s'" %
+            yield ValidationError(sec, "Duplicate id in Section '%s' and %s" %
                                   (sec.get_path(), id_map[sec.id]))
         else:
             id_map[sec.id] = "Section '%s'" % sec.get_path()
@@ -203,7 +203,7 @@ def property_unique_ids(section, id_map=None):
 
     for prop in section.properties:
         if prop.id in id_map:
-            yield ValidationError(prop, "Duplicate id in Property '%s' and '%s'" %
+            yield ValidationError(prop, "Duplicate id in Property '%s' and %s" %
                                   (prop.get_path(), id_map[prop.id]))
         else:
             id_map[prop.id] = "Property '%s'" % prop.get_path()

test/test_property.py

@@ -623,6 +623,11 @@ def test_clone(self):
         self.assertIsNotNone(prop.parent)
         self.assertIsNone(clone_prop.parent)
 
+        # Check keep_id
+        prop = Property(name="keepid")
+        clone_prop = prop.clone(True)
+        self.assertEqual(prop.id, clone_prop.id)
+
     def test_get_merged_equivalent(self):
         sec = Section(name="parent")
         mersec = Section(name="merged_section")

test/test_section.py

@@ -274,6 +274,30 @@ def test_clone(self):
         self.assertListEqual(clone_sec.sections, [])
         self.assertListEqual(clone_sec.properties, [])
 
+    def test_clone_keep_id(self):
+        # Check id kept in clone.
+        sec = Section(name="original")
+        clone_sec = sec.clone(keep_id=True)
+        self.assertEqual(sec, clone_sec)
+        self.assertEqual(sec.id, clone_sec.id)
+
+        # Check cloned child Sections keep their ids.
+        Section(name="sec_one", parent=sec)
+        Section(name="sec_two", parent=sec)
+        clone_sec = sec.clone(keep_id=True)
+        self.assertListEqual(sec.sections, clone_sec.sections)
+        self.assertEqual(sec.sections["sec_one"], clone_sec.sections["sec_one"])
+        self.assertEqual(sec.sections["sec_one"].id, clone_sec.sections["sec_one"].id)
+
+        # Check cloned child Properties keep their ids.
+        Property(name="prop_one", parent=sec)
+        Property(name="prop_two", parent=sec)
+        clone_sec = sec.clone(keep_id=True)
+        self.assertListEqual(sec.properties, clone_sec.properties)
+        self.assertEqual(sec.properties["prop_one"], clone_sec.properties["prop_one"])
+        self.assertEqual(sec.properties["prop_one"].id,
+                         clone_sec.properties["prop_one"].id)
+
     def test_reorder(self):
         # Test reorder of document sections
         doc = Document()

test/test_validation.py

@@ -98,3 +98,31 @@ def test_property_values(self):
         p.dependency = "p2"
         res = validate(doc)
         self.assertError(res, "non-existent dependency object")
+
+    def test_property_unique_ids(self):
+        """
+        Test if identical ids in properties raise a validation error
+        """
+        doc = odml.Document()
+        sec_one = odml.Section("sec1", parent=doc)
+        sec_two = odml.Section("sec2", parent=doc)
+        prop = odml.Property("prop", parent=sec_one)
+
+        cprop = prop.clone(keep_id=True)
+        sec_two.append(cprop)
+
+        res = validate(doc)
+        self.assertError(res, "Duplicate id in Property")
+
+    def test_section_unique_ids(self):
+        """
+        Test if identical ids in sections raise a validation error.
+        """
+        doc = odml.Document()
+        sec = odml.Section("sec", parent=doc)
+
+        csec = sec.clone(keep_id=True)
+        sec.append(csec)
+
+        res = validate(doc)
+        self.assertError(res, "Duplicate id in Section")