diff --git a/CHANGELOG.md b/CHANGELOG.md
index 81b19e4c41504ef350fb638f10b9195564c17109..e109ba5a37dcccabcaf2b341212def848e676a99 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,6 +43,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed ###
+* #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 a6d085c170a17722f53f51845e57e9411cb6a12b..9a168a7f83c13d2e2ae1bd7bed857dedc364fa7d 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:
@@ -848,7 +857,9 @@ class Entity(object):
"""
if xml is None:
- xml = etree.Element("Entity")
+ # use role as xml tag name, fall-back to "Entity"
+ elem_tag = "Entity" if self.role is None else self.role
+ xml = etree.Element(elem_tag)
assert isinstance(xml, etree._Element)
# unwrap wrapped entity
@@ -967,6 +978,8 @@ class Entity(object):
@param elem: the xml element
"""
+ if isinstance(entity, Entity):
+ entity.role = elem.tag
entity._cuid = elem.get("cuid")
entity.id = elem.get("id") # @ReservedAssignment
entity.name = elem.get("name")
@@ -1281,6 +1294,7 @@ class QueryTemplate():
def __init__(self, id=None, name=None, query=None, description=None): # @ReservedAssignment
self.id = (int(id) if id is not None else None)
+ self.role = "QueryTemplate"
self.name = name
self.description = description
self.query = query
@@ -2313,15 +2327,45 @@ class _Messages(dict):
def _basic_sync(e_local, e_remote):
+ '''Copy all state from a one entity to another.
+
+ This method is used to syncronize an entity with a remote (i.e. a newly
+ retrieved) one.
+
+ Any entity state of the local one will be overriden.
+
+ Parameters
+ ----------
+ e_local : Entity
+ Destination of the copy.
+ e_local : Entity
+ Source of the copy.
+
+
+ Returns
+ -------
+ e_local : Entity
+ The syncronized entity.
+ '''
if e_local is None or e_remote is None:
return None
+ 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
e_local.path = e_remote.path
e_local._checksum = e_remote._checksum
e_local._size = e_remote._size
- e_local.datatype = e_remote.datatype # @ReservedAssignment
+ e_local.datatype = e_remote.datatype
e_local.unit = e_remote.unit
e_local.value = e_remote.value
e_local.properties = e_remote.properties
@@ -4215,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 5bae6c219732f0170f5c351eae58148c9d3d065a..0d3183b4c0ca5517ecea68d0e49bbf335bb2a13e 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.")
diff --git a/unittests/test_entity.py b/unittests/test_entity.py
index e98dfbef5b6b5e5f691e8aecc2fa7d4a86991452..1e88702ac016d7dcfdf00919dd0f93b5d3345e00 100644
--- a/unittests/test_entity.py
+++ b/unittests/test_entity.py
@@ -24,6 +24,7 @@
"""Tests for the Entity class."""
# pylint: disable=missing-docstring
import unittest
+from lxml import etree
from caosdb import (INTEGER, Entity, Property, Record, RecordType,
configure_connection)
@@ -42,17 +43,54 @@ class TestEntity(unittest.TestCase):
def test_instance_variables(self):
entity = Entity()
self.assertTrue(hasattr(entity, "role"))
+ self.assertIsNone(entity.role)
self.assertTrue(hasattr(entity, "id"))
self.assertTrue(hasattr(entity, "name"))
self.assertTrue(hasattr(entity, "description"))
self.assertTrue(hasattr(entity, "parents"))
self.assertTrue(hasattr(entity, "properties"))
- def test_role(self):
+ def test_entity_role_1(self):
entity = Entity(role="TestRole")
self.assertEqual(entity.role, "TestRole")
entity.role = "TestRole2"
self.assertEqual(entity.role, "TestRole2")
- def test_instanciation(self):
+ def test_entity_role_2(self):
+ entity = Entity()
+
+ self.assertIsNone(entity.role)
+ self.assertEqual(entity.to_xml().tag, "Entity")
+
+ entity.role = "Record"
+ self.assertEqual(entity.role, "Record")
+ self.assertEqual(entity.to_xml().tag, "Record")
+
+ def test_recordtype_role(self):
+ entity = RecordType()
+
+ self.assertEqual(entity.role, "RecordType")
+ self.assertEqual(entity.to_xml().tag, "RecordType")
+
+ def test_property_role(self):
+ entity = Property()
+
+ self.assertEqual(entity.role, "Property")
+ self.assertEqual(entity.to_xml().tag, "Property")
+
+ def test_instantiation(self):
self.assertRaises(Exception, Entity())
+
+ def test_parse_role(self):
+ """During parsing, the role of an entity is set explicitely. All other
+ classes use the class name as a "natural" value for the role property.
+ """
+ parser = etree.XMLParser(remove_comments=True)
+ entity = Entity._from_xml(Entity(),
+ etree.parse("unittests/test_record.xml",
+ parser).getroot())
+
+ self.assertEqual(entity.role, "Record")
+ # test whether the __role property of this object has explicitely been
+ # set.
+ self.assertEqual(getattr(entity, "_Entity__role"), "Record")