diff --git a/CHANGELOG.md b/CHANGELOG.md index cb648b1d7404a1af4f00ff0aa5b26b4ebfc1e144..928c5230be4c72a55b8e7689eb8b6ad9d35416be 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,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 ### diff --git a/src/linkahead/apiutils.py b/src/linkahead/apiutils.py index 31e49aa9a25ea8a58a0cdf2b5e81f6c489a36b5f..1aa127d31d6456076ffdd6dc5b591c3bb2f7b0b7 100644 --- a/src/linkahead/apiutils.py +++ b/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 + + - 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, @@ -224,9 +230,9 @@ def compare_entities(entity0: Optional[Entity] = None, Params ------ - entity0 : Entity + entity0: Entity First entity to be compared. - entity1 : Entity + entity1: Entity Second entity to be compared. compare_referenced_records: bool, default: False If set to True, values with referenced records @@ -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 diff --git a/unittests/test_apiutils.py b/unittests/test_apiutils.py index fdd5adda065a563b15008f1b840539c110921b65..6667089abc2d16e59bd97d16f7d0fe75d07afe1b 100644 --- a/unittests/test_apiutils.py +++ b/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"]