diff --git a/CHANGELOG.md b/CHANGELOG.md index 3acce8439d9e3ce3d4a35a4806c72d2f9b16b2a2..cd37b71e60f243c5504fd61401e393c6e9eb1f97 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,11 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Removed ### ### Fixed ### - - Fixed `src/caosdb/utils/checkFileSystemConsistency.py` -- `compare_entities` now corretly raises a `NotImplementedError` in case of - comparing two `Property` entities or in case of entities with multi - properties. ### Security ### diff --git a/src/caosdb/apiutils.py b/src/caosdb/apiutils.py index 7a353b28b65eadca97578731b825bcf366098934..a46e30375b924d358448e73aece61562c36c700b 100644 --- a/src/caosdb/apiutils.py +++ b/src/caosdb/apiutils.py @@ -178,8 +178,7 @@ def getCommitIn(folder): return get_commit_in(folder) -def compare_entities(old_entity: Entity, new_entity: Entity, - compare_referenced_records: bool = False): +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 @@ -190,9 +189,8 @@ def compare_entities(old_entity: Entity, new_entity: Entity, - Information about properties: - Each property lists either an additional property or a property with a changed: - datatype - - unit - importance or - - value + - 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. @@ -202,9 +200,6 @@ def compare_entities(old_entity: Entity, new_entity: Entity, `compare_referenced_records = False` to prevent infinite recursion in case of circular references). - NOTE: This function does not work for abstract properties! I.e. it is not possible - to directly compare two entities that are of class caosdb.Property. - Parameters ---------- old_entity, new_entity : Entity @@ -217,12 +212,6 @@ def compare_entities(old_entity: Entity, new_entity: Entity, identical records are stored in different objects. Default is False. """ - - for entity in (old_entity, new_entity): - if isinstance(entity, Property): - raise NotImplementedError("The function compare_entities does not work for " - "comparing abstract properties.") - olddiff: Dict[str, Any] = {"properties": {}, "parents": []} newdiff: Dict[str, Any] = {"properties": {}, "parents": []} @@ -250,7 +239,7 @@ def compare_entities(old_entity: Entity, new_entity: Entity, if not old_entity_attr_exists and not new_entity_attr_exists: continue - if ((old_entity_attr_exists != new_entity_attr_exists) + if ((old_entity_attr_exists ^ new_entity_attr_exists) or (oldattr != newattr)): if old_entity_attr_exists: @@ -262,21 +251,9 @@ def compare_entities(old_entity: Entity, new_entity: Entity, # properties for prop in old_entity.properties: - # Find the corresponding property in new_entity: matching = [p for p in new_entity.properties if p.name == prop.name] - # This is needed for checking for multi properties in old_entity: - # TODO: is there a better way? - matching_old = [p for p in old_entity.properties if p.name == prop.name] - if len(matching_old) != 1: - raise NotImplementedError( - "Comparison not implemented for multi-properties.") - if len(matching) > 1: - raise NotImplementedError( - "Comparison not implemented for multi-properties.") - if len(matching) == 0: - # There is no matching property in new_entity: olddiff["properties"][prop.name] = {} elif len(matching) == 1: newdiff["properties"][prop.name] = {} @@ -306,7 +283,6 @@ def compare_entities(old_entity: Entity, new_entity: Entity, # scalar reference if isinstance(prop.value, Entity) and isinstance(matching[0].value, Entity): # explicitely not recursive to prevent infinite recursion - # TODO: why not use a recursion detection with a cache? same_value = empty_diff( prop.value, matching[0].value, compare_referenced_records=False) # list of references @@ -330,14 +306,11 @@ def compare_entities(old_entity: Entity, new_entity: Entity, newdiff["properties"].pop(prop.name) olddiff["properties"].pop(prop.name) - # Check whether there are missing properties in old_entity, additionally - # check for multi-properties that are currently not supported: - for prop in new_entity.properties: - matching = [p for p in new_entity.properties if p.name == prop.name] - if len(matching) > 1: + else: raise NotImplementedError( "Comparison not implemented for multi-properties.") + for prop in new_entity.properties: if len([0 for p in old_entity.properties if p.name == prop.name]) == 0: newdiff["properties"][prop.name] = {} diff --git a/src/caosdb/utils/git_utils.py b/src/caosdb/utils/git_utils.py index 7a58272a3bef1930f75a1e08364349388e2bb89f..98408fe22023e182fe9c9906a17f7d1a414dc5b3 100644 --- a/src/caosdb/utils/git_utils.py +++ b/src/caosdb/utils/git_utils.py @@ -34,6 +34,8 @@ from subprocess import call logger = logging.getLogger(__name__) + + def get_origin_url_in(folder: str): """return the Fetch URL of the git repository in the given folder.""" with tempfile.NamedTemporaryFile(delete=False, mode="w") as t: diff --git a/unittests/test_apiutils.py b/unittests/test_apiutils.py index c6cee61b63b9b5b7ae1dbe2d00959490b16eff75..bda381cf6427377194e272dfa14b83399b6f012f 100644 --- a/unittests/test_apiutils.py +++ b/unittests/test_apiutils.py @@ -101,15 +101,14 @@ def test_compare_entities(): r2 = db.Record() r1.add_parent("bla") r2.add_parent("bla") - r1.add_parent("lopp") # r1 has one additional parent + r1.add_parent("lopp") r1.add_property("test", value=2) - r2.add_property("test", value=2) # a property with the same value + r2.add_property("test", value=2) r1.add_property("tests", value=3) - r2.add_property("tests", value=45) # a property with different value + r2.add_property("tests", value=45) r1.add_property("tester", value=3) - r2.add_property("tester", ) # a property where r2 has no value + r2.add_property("tester", ) r1.add_property("tests_234234", value=45) - # one additional property that the other one does not have for both r2.add_property("tests_TT", value=45) diff_r1, diff_r2 = compare_entities(r1, r2) @@ -131,33 +130,6 @@ def test_compare_entities(): assert "tests_234234" in diff_r1["properties"] assert "tests_TT" in diff_r2["properties"] - # Check the value: - assert diff_r1["properties"]["tests"]["value"] == 3 - assert diff_r2["properties"]["tests"]["value"] == 45 - - -def test_compare_equality(): - r1 = db.Record() - r2 = db.Record() - diff_r1, diff_r2 = compare_entities(r1, r2) - for i in ("parents", "properties"): - assert len(diff_r1[i]) == 0 - assert len(diff_r2[i]) == 0 - assert empty_diff(r1, r2) - - r1.add_parent("bla") - r2.add_parent("bla") - assert empty_diff(r1, r2) - - r1.add_property("test", value=2) - r2.add_property("test", value=2) # a property with the same value - diff_r1, diff_r2 = compare_entities(r1, r2) - assert empty_diff(r1, r2) - - r1.add_parent("blobb") - r2.add_parent("blobb") - assert empty_diff(r1, r2) - def test_compare_entities_units(): r1 = db.Record() @@ -193,10 +165,6 @@ def test_compare_entities_units(): assert diff_r1["properties"]["test"]["unit"] == "cm" assert diff_r2["properties"]["test"]["unit"] == "m" - # Check the value: - assert diff_r1["properties"]["tests"]["value"] == 3 - assert diff_r2["properties"]["tests"]["value"] == 45 - def test_compare_special_properties(): # Test for all known special properties: @@ -244,114 +212,46 @@ def test_compare_special_properties(): assert len(diff_r2["properties"]) == 0 -def test_compare_importances(): - r1 = db.Record() - r2 = db.Record() - r1.add_property("test", value=2, unit="cm", importance="SUGGESTED") - r2.add_property("test", value=2, unit="cm") - assert not empty_diff(r1, r2) - diff_r1, diff_r2 = compare_entities(r1, r2) - assert diff_r1["properties"]["test"]["importance"] == "SUGGESTED" - assert diff_r2["properties"]["test"]["importance"] == "FIX" - for diff in (diff_r1, diff_r2): - assert len(diff["properties"]["test"]) == 1 - assert len(diff["parents"]) == 0 - - +@pytest.mark.xfail def test_compare_properties(): p1 = db.Property() p2 = db.Property() - with pytest.raises(NotImplementedError, match=".*abstract properties.*"): - compare_entities(p1, p2) - - -def test_multi_properties(): - # This test is rather lengthy, because: - # - previously the check for multi-properties was only implemented for the - # new_entity paramter of the function. - # - Because of the API of pylib the behavior depended on the order of adding the - # properties to the records. - - r1 = db.Record() - r2 = db.Record() - r1.add_property("test", value=2) - r1.add_property("test", value=4) - r2.add_property("test", value=2) - # That would be expected: - # assert not empty_diff(r1, r2) - with pytest.raises(NotImplementedError, match=".*multi-properties.*"): - compare_entities(r1, r2) - - r1 = db.Record() - r2 = db.Record() - r1.add_property("test", value=4) - r1.add_property("test", value=2) - r2.add_property("test", value=2) - # That would be expected: - # assert not empty_diff(r1, r2) - with pytest.raises(NotImplementedError, match=".*multi-properties.*"): - compare_entities(r1, r2) - - r1 = db.Record() - r2 = db.Record() - r1.add_property("test", value=4) - r1.add_property("test", value=2) - r2.add_property("test", value=2) - r2.add_property("test", value=4) - # That would be expected: - # assert empty_diff(r1, r2) - with pytest.raises(NotImplementedError, match=".*multi-properties.*"): - compare_entities(r1, r2) - - r1 = db.Record() - r2 = db.Record() - r1.add_property("test", value=4) - r2.add_property("test", value=4) - r2.add_property("test", value=2) - # That would be expected: - # assert not empty_diff(r1, r2) - with pytest.raises(NotImplementedError, match=".*multi-properties.*"): - compare_entities(r1, r2) - - r1 = db.Record() - r2 = db.Record() - r1.add_property("test", value=4) - r2.add_property("test", value=2) - r2.add_property("test", value=4) - # That would be expected: - # assert not empty_diff(r1, r2) - with pytest.raises(NotImplementedError, match=".*multi-properties.*"): - compare_entities(r1, r2) - - r1 = db.Record() - r2 = db.Record() - r1.add_property("test", value=4) - r1.add_property("test", value=2) - r2.add_property("test", value=2) - r2.add_property("test", value=5) - # That would be expected: - # assert empty_diff(r1, r2) - with pytest.raises(NotImplementedError, match=".*multi-properties.*"): - compare_entities(r1, r2) - - r1 = db.Record() - r2 = db.Record() - r1.add_property("test", value=4) - r1.add_property("test", value=2) - # That would be expected: - # assert empty_diff(r1, r2) - with pytest.raises(NotImplementedError, match=".*multi-properties.*"): - compare_entities(r1, r2) + diff_r1, diff_r2 = compare_entities(p1, p2) + assert len(diff_r1["parents"]) == 0 + assert len(diff_r2["parents"]) == 0 + assert len(diff_r1["properties"]) == 0 + assert len(diff_r2["properties"]) == 0 - r1 = db.Record() - r2 = db.Record() - r2.add_property("test", value=2) - r2.add_property("test", value=5) - # That would be expected: - # assert empty_diff(r1, r2) - with pytest.raises(NotImplementedError, match=".*multi-properties.*"): - compare_entities(r1, r2) + p1.importance = "SUGGESTED" + diff_r1, diff_r2 = compare_entities(p1, p2) + assert len(diff_r1["parents"]) == 0 + assert len(diff_r2["parents"]) == 0 + assert len(diff_r1["properties"]) == 0 + assert len(diff_r2["properties"]) == 0 + assert "importance" in diff_r1 + assert diff_r1["importance"] == "SUGGESTED" + + # TODO: I'm not sure why it is not like this: + # assert diff_r2["importance"] is None + # ... but: + assert "importance" not in diff_r2 + + p2.importance = "SUGGESTED" + p1.value = 42 + p2.value = 4 + + diff_r1, diff_r2 = compare_entities(p1, p2) + assert len(diff_r1["parents"]) == 0 + assert len(diff_r2["parents"]) == 0 + assert len(diff_r1["properties"]) == 0 + assert len(diff_r2["properties"]) == 0 + + # Comparing values currently does not seem to be implemented: + assert "value" in diff_r1 + assert diff_r1["value"] == 42 + assert "value" in diff_r2 + assert diff_r2["value"] == 4 def test_copy_entities():