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.

src/caosdb/apiutils.py

@@ -269,7+269,7 @@
                     new_entity.get_importance(prop.name)):
                 olddiff["properties"][prop.name]["importance"] = \
                     old_entity.get_importance(prop.name)
                 newdiff["properties"][prop.name]["importance"] = \
                     new_entity.get_importance(prop.name)
 
             if (prop.datatype != matching[0].datatype):
@@ -283,7+283,7 @@
                     matching[0].unit
 
             if (prop.value != matching[0].value):
-                olddiff["properties"][prop.name]["value"] = prop.value
-                newdiff["properties"][prop.name]["value"] = \
-                    matching[0].value
+                # basic comparison of value objects says they are different
+                same_value = False
+                if compare_referenced_records:
+                    # scalar reference
+                    if isinstance(prop.value, Entity) and isinstance(matching[0].value, Entity):
+                        # exlicitely not recursive to prevent infinite recursion
+                        same_value = empty_diff(
+                            prop.value, matching[0].value, compare_referenced_records=False)
+                    # list of references
+                    elif isinstance(prop.value, list) and isinstance(matching[0].value, list):
+                        # all elements in both lists actually are entity objects
+                        if all([isinstance(x, Entity) for x in prop.value]) and all([isinstance(x, Entity) for x in matching[0].value]):
+                            # can't be the same if the lengths are different
+                            if len(prop.value) == len(matching[0].value):
+                                # do a one-by-one comparison; the values are the same, if all diffs are empty
+                                same_value = all(
+                                    [empty_diff(x, y, False) for x, y in zip(prop.value, matching[0].value)])
+
+                if not same_value:
+                    olddiff["properties"][prop.name]["value"] = prop.value
+                    newdiff["properties"][prop.name]["value"] = \
+                        matching[0].value
 
             if (len(newdiff["properties"][prop.name]) == 0
                     and len(olddiff["properties"][prop.name]) == 0):
@@ -338,7 +357,7 @@ def empty_diff(old_entity: Entity, new_entity: Entity, compare_referenced_record
     return True
 
 
-def merge_entities(entity_a: Entity, entity_b: Entity):
+def merge_entities(entity_a: Entity, entity_b: Entity, merge_references_with_empty_diffs=True):
     """
     Merge entity_b into entity_a such that they have the same parents and properties.
 
@@ -352,13 +371,30 @@ def merge_entities(entity_a: Entity, entity_b: Entity):
     Returns entity_a.
 
     WARNING: This function is currently experimental and insufficiently tested. Use with care.
+
+    Parameters
+    ----------
+    entity_a, entity_b : Entity
+       The entities to be merged. entity_b will be merged into entity_a in place
+    merge_references_with_empty_diffs : bool, optional
+       Whether the merge is performed if entity_a and entity_b both reference
+       record(s) that may be different Python objects but have empty diffs. If
+       set to `False` a merge conflict will be raised in this case
+       instead. Default is True.
+
+    Returns
+    -------
+    entity_a : Entity
+       The initial entity_a after the in-place merge
+
     """
 
     logging.warning(
         "This function is currently experimental and insufficiently tested. Use with care.")
 
     # Compare both entities:
-    diff_r1, diff_r2 = compare_entities(entity_a, entity_b)
+    diff_r1, diff_r2 = compare_entities(
+        entity_a, entity_b, compare_referenced_records=merge_references_with_empty_diffs)
 
     # Go through the comparison and try to apply changes to entity_a:
     for key in diff_r2["parents"]: