From 1fc80f6cccd1353de6f872595b8e7a5217f649d1 Mon Sep 17 00:00:00 2001
From: Timm Fitschen <t.fitschen@indiscale.com>
Date: Tue, 28 Sep 2021 10:54:39 +0200
Subject: [PATCH] EHN: throw error when retrieving with wrong entity class
---
CHANGELOG.md | 11 ++++++++---
src/caosdb/common/models.py | 25 +++++++++++++++++++++----
unittests/test_add_property.py | 30 ++++++++++++++++++++++++++++++
3 files changed, 59 insertions(+), 7 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index db5501f1..867ca883 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -9,13 +9,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Added ###
+- It is possible now to supply a password for caosdb_admin on the command line and also activate the user directly using "-c".
* Added examples for complex data models to documentation
* extended apiutils with `resolve_reference(Property)`
* is_reference function for Properties
### Changed ###
-- It is possible now to supply a password for caosdb_admin on the command line and also activate the user directly using "-c".
+* Retrievals of entities where the class does not match the entity role raise
+ a ValueError now. See
+ [here](https://gitlab.indiscale.com/caosdb/src/caosdb-pylib/-/issues/66) for more
+ information. Updating a role is now being done by setting the `Entity.role`
+ to the new role (as string).
* Entity.add_property and Entity.add_parent do not accept `**kwargs`-style
keywords anymore. Formerly known keywords have been refactored into named
parameters.
@@ -38,8 +43,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed ###
-* Unintuitive behavior of `Entity.role` after a `Entity(id).retrieve()` where
- the correct role is present now.
+* #60 Unintuitive behavior of `Entity.role` after a `Entity(id).retrieve()`
+ Originally the role was `None`. The correct role is present now.
* #53 Documentation of inheritance
* #38 Dependencies in chunk-deletion of containers
diff --git a/src/caosdb/common/models.py b/src/caosdb/common/models.py
index 3aab7fc7..9a168a7f 100644
--- a/src/caosdb/common/models.py
+++ b/src/caosdb/common/models.py
@@ -130,7 +130,10 @@ class Entity(object):
@role.setter
def role(self, role):
- self.__role = role
+ if role is not None and role.lower() == "entity":
+ self.__role = None
+ else:
+ self.__role = role
@property
def size(self):
@@ -390,6 +393,12 @@ class Entity(object):
abstract_property = None
if isinstance(property, Entity):
+ if property.role is not None and property.role.lower() in ["record", "file"]:
+ raise ValueError("The property parameter is a {0}. This "
+ "is very unusual and probably not what you "
+ "want. Otherwise, construct a property from "
+ "a {0} using the Property class and add "
+ "that to this entity.".format(property.role))
abstract_property = property
elif isinstance(property, int):
if pid is not None:
@@ -969,7 +978,7 @@ class Entity(object):
@param elem: the xml element
"""
- if type(entity) is Entity:
+ if isinstance(entity, Entity):
entity.role = elem.tag
entity._cuid = elem.get("cuid")
entity.id = elem.get("id") # @ReservedAssignment
@@ -2340,8 +2349,16 @@ def _basic_sync(e_local, e_remote):
'''
if e_local is None or e_remote is None:
return None
- if type(e_local) is Entity:
+ if e_local.role is None:
e_local.role = e_remote.role
+ elif e_remote.role is not None and not e_local.role.lower() == e_remote.role.lower():
+ raise ValueError("The resulting entity had a different role ({0}) "
+ "than the local one ({1}). This probably means, that "
+ "the entity was intialized with a wrong class "
+ "by this client or it has changed in the past and "
+ "this client did't know about it yet.".format(
+ e_remote.role, e_local.role))
+
e_local.id = e_remote.id
e_local.name = e_remote.name
e_local.description = e_remote.description
@@ -4242,7 +4259,7 @@ def _evaluate_and_add_error(parent_error, ent):
Parameters:
-----------
- parent_error : TrancactionError
+ parent_error : TransactionError
Parent error to which the new exception will be attached. This
exception will be a direct child.
ent : Entity
diff --git a/unittests/test_add_property.py b/unittests/test_add_property.py
index 5bae6c21..0d3183b4 100644
--- a/unittests/test_add_property.py
+++ b/unittests/test_add_property.py
@@ -264,3 +264,33 @@ def test_add_list_of_entitities():
for e in rec.get_property("listOfEntities").value:
assert i == e.id
i += 1
+
+
+def test_add_property_with_wrong_role():
+ entity = db.Entity()
+
+ r = db.Record()
+ rt = db.RecordType()
+ p = db.Property()
+ f = db.File()
+ e = db.Entity()
+
+ entity.add_property(rt)
+ entity.add_property(p)
+ entity.add_property(e)
+
+ with raises(ValueError) as cm:
+ entity.add_property(r)
+ assert cm.value.args[0] == ("The property parameter is a Record. This is "
+ "very unusual and probably not what you want. "
+ "Otherwise, construct a property from a "
+ "Record using the Property class and add that "
+ "to this entity.")
+
+ with raises(ValueError) as cm:
+ entity.add_property(f)
+ assert cm.value.args[0] == ("The property parameter is a File. This is "
+ "very unusual and probably not what you want. "
+ "Otherwise, construct a property from a File "
+ "using the Property class and add that to "
+ "this entity.")
--
GitLab