diff --git a/CHANGELOG.md b/CHANGELOG.md
index cd37b71e60f243c5504fd61401e393c6e9eb1f97..3acce8439d9e3ce3d4a35a4806c72d2f9b16b2a2 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -22,7 +22,11 @@ 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 a46e30375b924d358448e73aece61562c36c700b..7a353b28b65eadca97578731b825bcf366098934 100644
--- a/src/caosdb/apiutils.py
+++ b/src/caosdb/apiutils.py
@@ -178,7 +178,8 @@ 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
@@ -189,8 +190,9 @@ def compare_entities(old_entity: Entity, new_entity: Entity, compare_referenced_
- Information about properties:
- Each property lists either an additional property or a property with a changed:
- datatype
+ - unit
- importance or
- - value (not implemented yet)
+ - value
In case of changed information the value listed under the respective key shows the
value that is stored in the respective entity.
@@ -200,6 +202,9 @@ def compare_entities(old_entity: Entity, new_entity: Entity, compare_referenced_
`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
@@ -212,6 +217,12 @@ def compare_entities(old_entity: Entity, new_entity: Entity, compare_referenced_
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": []}
@@ -239,7 +250,7 @@ def compare_entities(old_entity: Entity, new_entity: Entity, compare_referenced_
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:
@@ -251,9 +262,21 @@ def compare_entities(old_entity: Entity, new_entity: Entity, compare_referenced_
# 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] = {}
@@ -283,6 +306,7 @@ def compare_entities(old_entity: Entity, new_entity: Entity, compare_referenced_
# 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
@@ -306,11 +330,14 @@ def compare_entities(old_entity: Entity, new_entity: Entity, compare_referenced_
newdiff["properties"].pop(prop.name)
olddiff["properties"].pop(prop.name)
- else:
+ # 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:
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 98408fe22023e182fe9c9906a17f7d1a414dc5b3..7a58272a3bef1930f75a1e08364349388e2bb89f 100644
--- a/src/caosdb/utils/git_utils.py
+++ b/src/caosdb/utils/git_utils.py
@@ -34,8 +34,6 @@ 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 bda381cf6427377194e272dfa14b83399b6f012f..c6cee61b63b9b5b7ae1dbe2d00959490b16eff75 100644
--- a/unittests/test_apiutils.py
+++ b/unittests/test_apiutils.py
@@ -101,14 +101,15 @@ def test_compare_entities():
r2 = db.Record()
r1.add_parent("bla")
r2.add_parent("bla")
- r1.add_parent("lopp")
+ r1.add_parent("lopp") # r1 has one additional parent
r1.add_property("test", value=2)
- r2.add_property("test", value=2)
+ r2.add_property("test", value=2) # a property with the same value
r1.add_property("tests", value=3)
- r2.add_property("tests", value=45)
+ r2.add_property("tests", value=45) # a property with different value
r1.add_property("tester", value=3)
- r2.add_property("tester", )
+ r2.add_property("tester", ) # a property where r2 has no value
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)
@@ -130,6 +131,33 @@ 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()
@@ -165,6 +193,10 @@ 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:
@@ -212,46 +244,114 @@ def test_compare_special_properties():
assert len(diff_r2["properties"]) == 0
-@pytest.mark.xfail
+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
+
+
def test_compare_properties():
p1 = db.Property()
p2 = db.Property()
- 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
+ with pytest.raises(NotImplementedError, match=".*abstract properties.*"):
+ compare_entities(p1, p2)
- 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_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)
+
+ 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)
def test_copy_entities():