From bceb49a97a8221f709860b83cd44623fa072c70c Mon Sep 17 00:00:00 2001
From: Alexander Schlemmer <alexander@mail-schlemmer.de>
Date: Fri, 5 May 2023 16:41:13 +0200
Subject: [PATCH] ENH: added missing check for multi-properties and added
 corresponding test cases

---
 src/caosdb/apiutils.py     | 10 +++++-
 unittests/test_apiutils.py | 68 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 77 insertions(+), 1 deletion(-)

diff --git a/src/caosdb/apiutils.py b/src/caosdb/apiutils.py
index 60a75a12..0a9b9590 100644
--- a/src/caosdb/apiutils.py
+++ b/src/caosdb/apiutils.py
@@ -267,7 +267,7 @@ def compare_entities(old_entity: Entity, new_entity: Entity,
             continue
 
         if ((old_entity_attr_exists != new_entity_attr_exists)
-            or (oldattr != newattr)):
+                or (oldattr != newattr)):
 
             if old_entity_attr_exists:
                 olddiff[attr] = oldattr
@@ -278,8 +278,16 @@ 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) == 0:
             olddiff["properties"][prop.name] = {}
         elif len(matching) == 1:
diff --git a/unittests/test_apiutils.py b/unittests/test_apiutils.py
index 1d6406a2..91d0eeb2 100644
--- a/unittests/test_apiutils.py
+++ b/unittests/test_apiutils.py
@@ -261,7 +261,75 @@ def test_compare_properties():
 
     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)
 
 
 def test_copy_entities():
-- 
GitLab