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

@@ -188,9 +188,8 @@ def getCommitIn(folder):
         return t.readline().strip()
 
 
-def compare_entities(old_entity: Entity, new_entity: Entity):
-    """
-    Compare two entites.
+def compare_entities(old_entity: Entity, new_entity: Entity, compare_referenced_records: bool = False):
+    """Compare two entites.
 
     Return a tuple of dictionaries, the first index belongs to additional information for old
     entity, the second index belongs to additional information for new entity.
@@ -204,6 +203,23 @@ def compare_entities(old_entity: Entity, new_entity: Entity):
         - ... value (not implemented yet)
         In case of changed information the value listed under the respective key shows the
         value that is stored in the respective entity.
+
+    If `compare_referenced_records` is `True`, also referenced entities will be
+    compared using this function (which is then called with
+    `compare_referenced_records = False` to prevent infinite recursion in case
+    of circular references).
+
+    Parameters
+    ----------
+    old_entity, new_entity : Entity
+        Entities to be compared
+    compare_referenced_records : bool, optional
+        Whether to compare referenced records in case of both, `old_entity` and
+        `new_entity`, have the same reference properties and both have a Record
+        object as value. If set to `False`, only the corresponding Python
+        objects are compared which may lead to unexpected behavior when
+        identical records are stored in different objects. Default is False.
+
     """
     olddiff: Dict[str, Any] = {"properties": {}, "parents": []}
     newdiff: Dict[str, Any] = {"properties": {}, "parents": []}
@@ -270,6 +286,26 @@ def compare_entities(old_entity: Entity, new_entity: Entity):
                     matching[0].unit
 
             if (prop.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):
+                        # explicitely 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
+                        # TODO: check, whether mixed cases can be allowed or should lead to an error
+                        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
@@ -300,7 +336,32 @@ def compare_entities(old_entity: Entity, new_entity: Entity):
     return (olddiff, newdiff)
 
 
-def merge_entities(entity_a: Entity, entity_b: Entity):
+def empty_diff(old_entity: Entity, new_entity: Entity, compare_referenced_records: bool = False):
+    """Check whether the `compare_entities` found any differences between
+    old_entity and new_entity.
+
+    Parameters
+    ----------
+    old_entity, new_entity : Entity
+        Entities to be compared
+    compare_referenced_records : bool, optional
+        Whether to compare referenced records in case of both, `old_entity` and
+        `new_entity`, have the same reference properties and both have a Record
+        object as value.
+
+    """
+    olddiff, newdiff = compare_entities(
+        old_entity, new_entity, compare_referenced_records)
+    for diff in [olddiff, newdiff]:
+        for key in diff:
+            if len(diff[key]) > 0:
+                # There is a difference somewhere in the diff
+                return False
+    # all elements of the two diffs were empty
+    return True
+
+
+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.
 
@@ -314,13 +375,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"]: