F fix merge entity conflicts

Summary

Part of https://gitlab.indiscale.com/caosdb/customers/leibniz-zmt/management/-/issues/125. Adds a new apiutils.empty_diff function to check for the identity of two Entity objects in the sense of no differences between the two entities. It also adds optional arguments to compare_entities and merge_entities to determine whether or not referenced Entity objects will be compared/merged.

Focus

Mainly, new cases to the value comparison of reference properties have been added to the compare_entities function. Note that the compare_referenced_records argument of the compare_entities function defaults False in order to restore the previous behavior. In contrast, the default of merge_references_with_empty_diffs of merge_entities is True because a) the function is Experimental anyway, so breaking previous behavior is ok, and b) in most of the (e.g., crawler) cases this is probably what a merge is expected to do.

Test Environment

Unit tests should be sufficient; but you can also test it with ZMT's Pangaea crawler.

Check List for the Author

Please, prepare your MR for a review. Be sure to write a summary and a focus and create gitlab comments for the reviewer. They should guide the reviewer through the changes, explain your changes and also point out open questions. For further good practices have a look at our review guidelines

• All automated tests pass
• Reference related issues
• Up-to-date CHANGELOG.md (or not necessary)
• Annotations in code (Gitlab comments)
  • Intent of new code
  • Problems with old code
  • Why this implementation?

Check List for the Reviewer

• I understand the intent of this MR
• All automated tests pass
• Up-to-date CHANGELOG.md (or not necessary)
• The test environment setup works and the intended behavior is reproducible in the test environment
• In-code documentation and comments are up-to-date.
• Check: Are there specifications? Are they satisfied?

For further good practices have a look at our review guidelines.

unittests/test_apiutils.py

@@ -30,7 +30,7 @@ import pytest
 import caosdb as db
 import caosdb.apiutils
 from caosdb.apiutils import (apply_to_ids, compare_entities, create_id_query,
-                             resolve_reference, merge_entities)
+                             empty_diff, resolve_reference, merge_entities)
 
 from caosdb.common.models import SPECIAL_ATTRIBUTES
 
@@ -272,8 +272,10 @@ def test_copy_entities():
     for i in [0, 1]:
         assert c.properties[i] is not r.properties[i]
         for special in SPECIAL_ATTRIBUTES:
-            assert getattr(c.properties[i], special) == getattr(r.properties[i], special)
-        assert c.get_importance(c.properties[i]) == r.get_importance(r.properties[i])
+            assert getattr(c.properties[i], special) == getattr(
+                r.properties[i], special)
+        assert c.get_importance(
+            c.properties[i]) == r.get_importance(r.properties[i])
 
 
 def test_merge_entities():
@@ -326,10 +328,12 @@ def test_merge_bug_109():
     assert r_a.get_property("test_bug_property").value == [18, 19]
 
     assert "<Value>18</Value>\n    <Value>19</Value>" in str(r_b)
-    assert "<Value>18</Value>\n    <Value>19</Value>\n    <Value>18</Value>\n    <Value>19</Value>" not in str(r_b)
+    assert "<Value>18</Value>\n    <Value>19</Value>\n    <Value>18</Value>\n    <Value>19</Value>" not in str(
+        r_b)
 
     assert "<Value>18</Value>\n    <Value>19</Value>" in str(r_a)
-    assert "<Value>18</Value>\n    <Value>19</Value>\n    <Value>18</Value>\n    <Value>19</Value>" not in str(r_a)
+    assert "<Value>18</Value>\n    <Value>19</Value>\n    <Value>18</Value>\n    <Value>19</Value>" not in str(
+        r_a)
 
 
 @pytest.mark.xfail
@@ -349,7 +353,98 @@ def test_bug_109():
     assert r_a.get_property("test_bug_property").value == [18, 19]
 
     assert "<Value>18</Value>\n    <Value>19</Value>" in str(r_b)
-    assert "<Value>18</Value>\n    <Value>19</Value>\n    <Value>18</Value>\n    <Value>19</Value>" not in str(r_b)
+    assert "<Value>18</Value>\n    <Value>19</Value>\n    <Value>18</Value>\n    <Value>19</Value>" not in str(
+        r_b)
 
     assert "<Value>18</Value>\n    <Value>19</Value>" in str(r_a)
-    assert "<Value>18</Value>\n    <Value>19</Value>\n    <Value>18</Value>\n    <Value>19</Value>" not in str(r_a)
+    assert "<Value>18</Value>\n    <Value>19</Value>\n    <Value>18</Value>\n    <Value>19</Value>" not in str(
+        r_a)
+
+
+def test_wrong_merge_conflict_reference():
+    """Test a wrongly detected merge conflict in case of two records referencing
+    two different, but identical objects.
+
+    """
+    # Two identical license records will be referenced from both records to be
+    # merged
+    license_rt = db.RecordType(name="license")
+    license_rec_a = db.Record(name="CC-BY-3.0").add_parent(license_rt)
+    license_rec_b = db.Record(name="CC-BY-3.0").add_parent(license_rt)
+
+    # two referencing records
+    dataset_rt = db.RecordType(name="Dataset")
+    title_prop = db.Property(name="title", datatype=db.TEXT)
+    doi_prop = db.Property(name="DOI", datatype=db.TEXT)
+    rec_a = db.Record().add_parent(dataset_rt)
+    rec_a.add_property(name=license_rt.name,
+                       datatype=license_rt.name, value=license_rec_a)
+    rec_a.add_property(name=title_prop.name, value="Some dataset title")
+
+    rec_b = db.Record().add_parent(dataset_rt)
+    rec_b.add_property(name=license_rt.name,
+                       datatype=license_rt.name, value=license_rec_b)
+    rec_b.add_property(name=doi_prop.name, value="https://doi.org/12345.678")
+
+    print(compare_entities(rec_a, rec_b))
+    print(compare_entities(license_rec_a, license_rec_b))
+    print(license_rec_b == license_rec_a)
+    merge_entities(rec_a, rec_b)
+    assert rec_a.get_property(license_rt.name) is not None
+    assert rec_a.get_property(license_rt.name).value is not None
+    assert isinstance(rec_a.get_property(license_rt.name).value, db.Record)
+
+
+def test_empty_diff():
+
+    rec_a = db.Record(name="A")
+    rec_b = db.Record(name="B")
+
+    assert empty_diff(rec_a, rec_a)
+    assert not empty_diff(rec_a, rec_b)
+
+    rec_a.add_parent(name="RT")
+    rec_b.add_parent(name="RT")
+    assert empty_diff(rec_a, rec_a)
+    assert not empty_diff(rec_a, rec_b)
+
+    rec_b.name = "A"
+    assert empty_diff(rec_a, rec_b)
+
+    rec_a.add_property(name="some_prop", value=1)
+    assert not empty_diff(rec_a, rec_b)
+
+    rec_b.add_property(name="some_prop", value=1)
+    assert empty_diff(rec_a, rec_b)
+
+    rec_b.get_property("some_prop").value = 2
+    assert not empty_diff(rec_a, rec_b)
+
+    rec_b.get_property("some_prop").value = 1
+    rec_b.add_property(name="some_other_prop", value="Test")
+    assert not empty_diff(rec_a, rec_b)
+
+    rec_a.add_property(name="some_other_prop", value="Test")
+    assert empty_diff(rec_a, rec_b)
+
+    # reference identical records, but different Python Record objects
+    ref_rec_a = db.Record(name="Ref").add_parent(name="RefType")
+    ref_rec_b = db.Record(name="Ref").add_parent(name="RefType")
+    rec_a.add_property(name="RefType", datatype="RefType", value=ref_rec_a)
+    rec_b.add_property(name="RefType", datatype="RefType", value=ref_rec_b)
+    # the default is `compare_referenced_records=False`, so the diff shouldn't
+    # be empty (different Python objects are referenced.)
+    assert not empty_diff(rec_a, rec_b)
+    # when looking into the referenced record, the diffs should be empty again
+    assert empty_diff(rec_a, rec_b, compare_referenced_records=True)
+
+    # The same for lists of references
+    rec_a.remove_property("RefType")
+    rec_b.remove_property("RefType")
+    assert empty_diff(rec_a, rec_b)
+    rec_a.add_property(name="RefType", datatype=db.LIST(
+        "RefType"), value=[ref_rec_a, ref_rec_a])
+    rec_b.add_property(name="RefType", datatype=db.LIST(
+        "RefType"), value=[ref_rec_b, ref_rec_b])
+    assert not empty_diff(rec_a, rec_b)
+    assert empty_diff(rec_a, rec_b, compare_referenced_records=True)