Merge branch 'f-bug-fit-94-compare-properties-with-id' into 'dev'

Compare_entities diff uses id instead of name where needed

See merge request !165

CHANGELOG.md

@@ -24,6 +24,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * [#103](https://gitlab.com/linkahead/linkahead-pylib/-/issues/103)
   `authentication/interface/on_response()` does not overwrite
   `auth_token` if new value is `None`
+* [#119](https://gitlab.com/linkahead/linkahead-pylib/-/issues/119)
+  The diff returned by compare_entities now uses id instead of name as key if either property does not have a name
 
 ### Security ###
 

src/linkahead/apiutils.py

@@ -197,13 +197,19 @@ def compare_entities(entity0: Optional[Entity] = None,
     properties and SPECIAL_ATTRIBUTES if they are missing or different from
     their counterparts in the other entity.
 
+    The key used to represent a parent in the parent list or a
+    property in the property dictionary is the entity's name if the
+    name is present for both compared entities, the id otherwise.
+
     The value of the properties dict for each listed property is again a dict
     detailing the differences between this property and its counterpart.
     The characteristics that are checked to determine whether two properties
     match are the following:
+
     - datatype
     - importance
     - value
+
     If any of these characteristics differ for a property, the respective
     string (datatype, importance, value) is added as a key to the dict of the
     property with its value being the characteristics value,
@@ -242,6 +248,7 @@ def compare_entities(entity0: Optional[Entity] = None,
                                 entity and an int or str also checks whether
                                 the int/str matches the name or id of the
                                 entity, so Entity(id=100) == 100 == "100".
+
     """
     # ToDo: Discuss intended behaviour
     # Questions that need clarification:
@@ -377,15 +384,20 @@ def compare_entities(entity0: Optional[Entity] = None,
 
     # compare properties
     for prop in entity0.properties:
-        matching = entity1.properties.filter(name=prop.name, pid=prop.id)
+        # ToDo: Would making id default break anything?
+        key = prop.name if prop.name is not None else prop.id
+        matching = entity1.properties.filter(prop)
         if len(matching) == 0:
             # entity1 has prop, entity0 does not
-            diff[0]["properties"][prop.name] = {}
+            diff[0]["properties"][key] = {}
         elif len(matching) == 1:
-            diff[0]["properties"][prop.name] = {}
-            diff[1]["properties"][prop.name] = {}
-            propdiff = (diff[0]["properties"][prop.name],
-                        diff[1]["properties"][prop.name])
+            # It's possible that prop has name and id, but match only has id
+            key = prop.name if (prop.name is not None and
+                                matching[0].name == prop.name) else prop.id
+            diff[0]["properties"][key] = {}
+            diff[1]["properties"][key] = {}
+            propdiff = (diff[0]["properties"][key],
+                        diff[1]["properties"][key])
 
             # We should compare the wrapped properties instead of the
             # wrapping entities if possible:
@@ -417,8 +429,8 @@ def compare_entities(entity0: Optional[Entity] = None,
 
             # in case there is no difference, we remove the dict keys again
             if len(propdiff[0]) == 0 and len(propdiff[1]) == 0:
-                diff[0]["properties"].pop(prop.name)
-                diff[1]["properties"].pop(prop.name)
+                diff[0]["properties"].pop(key)
+                diff[1]["properties"].pop(key)
 
         else:
             raise NotImplementedError(
@@ -426,11 +438,12 @@ def compare_entities(entity0: Optional[Entity] = None,
 
     # we have not yet compared properties that do not exist in entity0
     for prop in entity1.properties:
+        key = prop.name if prop.name is not None else prop.id
         # check how often the property appears in entity0
         num_prop_in_ent0 = len(entity0.properties.filter(prop))
         if num_prop_in_ent0 == 0:
             # property is only present in entity0 - add to diff
-            diff[1]["properties"][prop.name] = {}
+            diff[1]["properties"][key] = {}
         if num_prop_in_ent0 > 1:
             # Check whether the property is present multiple times in entity0
             # and raise error - result would be incorrect
@@ -441,9 +454,10 @@ def compare_entities(entity0: Optional[Entity] = None,
     for index, parents, other_entity in [(0, entity0.parents, entity1),
                                          (1, entity1.parents, entity0)]:
         for parent in parents:
+            key = parent.name if parent.name is not None else parent.id
             matching = other_entity.parents.filter(parent)
             if len(matching) == 0:
-                diff[index]["parents"].append(parent.name)
+                diff[index]["parents"].append(key)
                 continue
 
     return diff

unittests/test_apiutils.py

@@ -991,3 +991,31 @@ def test_describe_diff():
 
     assert "first" not in diffout
     assert "second" not in diffout
+
+
+def test_diff_without_names():
+    """Test compare_entities in case of properties and parents with
+    ids and without names
+    (cf. https://gitlab.com/linkahead/linkahead-pylib/-/issues/119).
+
+    """
+
+    r1 = db.Record(name="Test").add_parent(name="TestType")
+    r2 = db.Record(name="Test").add_parent(name="TestType")
+    r2.add_property(id=123, value="Test")
+
+    diff1, diff2 = compare_entities(r1, r2)
+    assert len(diff1["properties"]) == 0
+    assert len(diff2["properties"]) == 1
+    assert 123 in diff2["properties"]
+    assert None not in diff2["properties"]
+
+    r3 = db.Record().add_parent(id=101)
+    r4 = db.Record().add_parent(id=102)
+    diff3, diff4 = compare_entities(r3, r4)
+    assert len(diff3["parents"]) == 1
+    assert 101 in diff3["parents"]
+    assert None not in diff3["parents"]
+    assert len(diff4["parents"]) == 1
+    assert 102 in diff4["parents"]
+    assert None not in diff3["parents"]