F merge id with resolved entity

Summary

One half of the fix for caosdb-crawler#113 (closed). Introduces a merge_id_with_resolved_entity parameter to the apiutils.merge_entities function, that allows to identify an id value of a reference property with an entity that has this id.

Focus

Check whether you can find cases that the new unit test doesn't cover.

Test Environment

Unit test should be sufficient, but you can also run the new int test from caosdb-crawler!141 (merged) and verify that this enhancement resolves the bug.

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)
• Up-to-date JSON schema (or not necessary)
• Appropriate user and developer documentation (or not necessary)
  • How do I use the software? Assume "stupid" users.
  • How do I develop or debug the software? Assume novice developers.
• 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)
• Appropriate user and developer documentation (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/linkahead/apiutils.py

@@ -354,7 +354,7 @@ def empty_diff(old_entity: Entity, new_entity: Entity, compare_referenced_record
 
 
 def merge_entities(entity_a: Entity, entity_b: Entity, merge_references_with_empty_diffs=True,
-                   force=False):
+                   force=False, merge_id_with_resolved_entity: bool = False):
     """Merge entity_b into entity_a such that they have the same parents and properties.
 
     datatype, unit, value, name and description will only be changed in entity_a
@@ -372,16 +372,22 @@ def merge_entities(entity_a: Entity, entity_b: Entity, merge_references_with_emp
     Parameters
     ----------
     entity_a, entity_b : Entity
-       The entities to be merged. entity_b will be merged into entity_a in place
+        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.
+        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.
     force : bool, optional
-       If True, in case `entity_a` and `entity_b` have the same properties, the
-       values of `entity_a` are replaced by those of `entity_b` in the merge.
-       If `False`, an EntityMergeConflictError is raised instead. Default is False.
+        If True, in case `entity_a` and `entity_b` have the same properties, the
+        values of `entity_a` are replaced by those of `entity_b` in the
+        merge. If `False`, an EntityMergeConflictError is raised
+        instead. Default is False.
+    merge_id_with_resolved_entity : bool, optional
+        If true, the values of two reference properties will be considered the
+        same if one is an integer id and the other is a db.Entity with this
+        id. I.e., a value 123 is identified with a value ``<Record
+        id=123/>``. Default is False.
 
     Returns
     -------
@@ -427,13 +433,31 @@ def merge_entities(entity_a: Entity, entity_b: Entity, merge_references_with_emp
                         setattr(entity_a.get_property(key), attribute,
                                 diff_r2["properties"][key][attribute])
                     else:
-                        raise EntityMergeConflictError(
-                            f"Entity a ({entity_a.id}, {entity_a.name}) "
-                            f"has a Property '{key}' with {attribute}="
-                            f"{diff_r2['properties'][key][attribute]}\n"
-                            f"Entity b ({entity_b.id}, {entity_b.name}) "
-                            f"has a Property '{key}' with {attribute}="
-                            f"{diff_r1['properties'][key][attribute]}")
+                        raise_error = True
+                        if merge_id_with_resolved_entity is True and attribute == "value":
+                            # Do a special check for the case of an id value on the
+                            # one hand, and a resolved entity on the other side.
+                            this = entity_a.get_property(key).value
+                            that = entity_b.get_property(key).value
+                            same = False
+                            if isinstance(this, list) and isinstance(that, list):
+                                if len(this) == len(that):
+                                    same = all([_same_id_as_resolved_entity(a, b)
+                                                for a, b in zip(this, that)])
+                            else:
+                                same = _same_id_as_resolved_entity(this, that)
+                            if same is True:
+                                setattr(entity_a.get_property(key), attribute,
+                                        diff_r2["properties"][key][attribute])
+                                raise_error = False
+                        if raise_error is True:
+                            raise EntityMergeConflictError(
+                                f"Entity a ({entity_a.id}, {entity_a.name}) "
+                                f"has a Property '{key}' with {attribute}="
+                                f"{diff_r2['properties'][key][attribute]}\n"
+                                f"Entity b ({entity_b.id}, {entity_b.name}) "
+                                f"has a Property '{key}' with {attribute}="
+                                f"{diff_r1['properties'][key][attribute]}")
         else:
             # TODO: This is a temporary FIX for
             #       https://gitlab.indiscale.com/caosdb/src/caosdb-pylib/-/issues/105
@@ -586,3 +610,16 @@ def create_flat_list(ent_list: List[Entity], flat: List[Entity]):
                     flat.append(p.value)
                 # TODO: move inside if block?
                 create_flat_list([p.value], flat)
+
+
+def _same_id_as_resolved_entity(this, that):
+    """Checks whether ``this`` and ``that`` either are the same or whether one
+    is an id and the other is a db.Entity with this id.
+
+    """
+    if isinstance(this, Entity) and not isinstance(that, Entity):
+        # this is an Entity with an id, that is not
+        return this.id is not None and this.id == that
+    if not isinstance(this, Entity) and isinstance(that, Entity):
+        return that.id is not None and that.id == this
+    return this == that

[Florian Spreckelsen]

I do not check for int explicitly here, so that we don't have to change this for string ids.