diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c9ed3081d083c1a78c14b590cb4b0e716478e71..4d42e0b26a2b493d210ff0a55105d70cfd6c90a3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,8 +14,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed ### +* `_Messages` is now `Messages` and inherits from list instead of dict +* `Message.__init__` signature changed and `type` defaults to "Info" now. +* `Message.__eq__` changed. Equality is equality of `type`, `code`, and + `description` now. + ### Deprecated ### +* The API of Messages has been simplified and some ways to interact with + messages have been deprecated. Warnings are raised correspondingly. +* `Message.get_code`. Use the `code` property instead. + ### Removed ### ### Fixed ### diff --git a/src/caosdb/common/models.py b/src/caosdb/common/models.py index 758252d1ee6b7136b3396bbb15693b4ef1816207..df77bb7311a86abc8a78715e082f115c6a3efc2b 100644 --- a/src/caosdb/common/models.py +++ b/src/caosdb/common/models.py @@ -32,8 +32,8 @@ All additional classes are either important for the entities or the transactions. """ -from __future__ import print_function, unicode_literals from __future__ import annotations # Can be removed with 3.10. +from __future__ import print_function, unicode_literals import re import sys @@ -51,8 +51,8 @@ from warnings import warn from caosdb.common.datatype import (BOOLEAN, DATETIME, DOUBLE, INTEGER, TEXT, is_list_datatype, is_reference) from caosdb.common.state import State -from caosdb.common.utils import uuid, xml2str from caosdb.common.timezone import TimeZone +from caosdb.common.utils import uuid, xml2str from caosdb.common.versioning import Version from caosdb.configuration import get_config from caosdb.connection.connection import get_connection @@ -112,7 +112,7 @@ class Entity: self.__datatype = None self.datatype = datatype self.value = value - self.messages = _Messages() + self.messages = Messages() self.properties = _Properties() self.parents = _ParentList() self.path = None @@ -419,7 +419,7 @@ class Entity: self.acl.is_permitted(permission=permission) def get_all_messages(self): - ret = _Messages() + ret = Messages() ret.append(self.messages) for p in self.properties: @@ -682,7 +682,8 @@ class Entity: if msg is not None: pass else: - msg = Message(type, code, description, body) + msg = Message(description=description, type=type, code=code, + body=body) self.messages.append(msg) return self @@ -1056,7 +1057,7 @@ out: List[Entity] def get_messages(self): """Get all messages of this entity. - @return: _Messages(list) + @return: Messages(list) """ return self.messages @@ -1064,9 +1065,9 @@ out: List[Entity] def get_warnings(self): """Get all warning messages of this entity. - @return _Messages(list): Warning messages. + @return Messages(list): Warning messages. """ - ret = _Messages() + ret = Messages() for m in self.messages: if m.type.lower() == "warning": @@ -1077,9 +1078,9 @@ out: List[Entity] def get_errors(self): """Get all error messages of this entity. - @return _Messages(list): Error messages. + @return Messages(list): Error messages. """ - ret = _Messages() + ret = Messages() for m in self.messages: if m.type.lower() == "error": @@ -1599,7 +1600,7 @@ class QueryTemplate(): self._cuid = None self.value = None self.datatype = None - self.messages = _Messages() + self.messages = Messages() self.properties = None self.parents = None self.path = None @@ -1719,7 +1720,7 @@ class QueryTemplate(): return self.id is not None def get_errors(self): - ret = _Messages() + ret = Messages() for m in self.messages: if m.type.lower() == "error": @@ -1870,12 +1871,10 @@ class Property(Entity): class Message(object): - # @ReservedAssignment - - def __init__(self, type, code=None, description=None, body=None): # @ReservedAssignment - self.type = type - self.code = code + def __init__(self, type=None, code=None, description=None, body=None): # @ReservedAssignment self.description = description + self.type = type if type is not None else "Info" + self.code = int(code) if code is not None else None self.body = body def to_xml(self, xml=None): @@ -1898,11 +1897,13 @@ class Message(object): def __eq__(self, obj): if isinstance(obj, Message): - return self.type == obj.type and self.code == obj.code + return self.type == obj.type and self.code == obj.code and self.description == obj.description return False def get_code(self): + warn(("get_code is deprecated and will be removed in future. " + "Use self.code instead."), DeprecationWarning) return int(self.code) @@ -2399,10 +2400,9 @@ class _ParentList(list): raise KeyError(str(parent) + " not found.") -class _Messages(dict): - - """This 'kind of dictionary' stores error, warning, info, and other - messages. The mentioned three messages types are messages of special use. +class Messages(list): + """This specialization of list stores error, warning, info, and other + messages. The mentioned three messages types play a special role. They are generated by the client and the server while processing the entity to which the message in question belongs. It is RECOMMENDED NOT to specify such messages manually. The other messages are ignored by the server unless @@ -2413,25 +2413,18 @@ class _Messages(dict): <$Type code=$code description=$description>$body</$Type> - Messages are treated as 'equal' if and only if both they have the same type (case-insensitive), - and the same code (or no code). Every message - MUST NOT occur more than once per entity (to which the message in question belongs). - - If a message m2 is added while a messages m1 is already in this _Message object m2 will - OVERRIDE m1. - Error, warning, and info messages will be deleted before any transaction. Examples: - <<< msgs = _Messages() + <<< msgs = Messages() <<< # create Message <<< msg = Message(type="HelloWorld", code=1, description="Greeting the world", body="Hello, world!") - <<< # append it to the _Messages + <<< # append it to the Messages <<< msgs.append(msg) - <<< # use _Messages as list of Message objects + <<< # use Messages as list of Message objects <<< for m in msgs: ... assert isinstance(m,Message) @@ -2442,29 +2435,12 @@ class _Messages(dict): <<< msgs.append(msg) <<< # get it back via get(...) and the key tuple (type, code) <<< assert id(msgs.get("HelloWorld",1))==id(msg) - - <<< # delete Message via remove and the (type,code) tuple - <<< msgs.remove("HelloWorld",1) - <<< assert msgs.get("HelloWorld",1) == None - - <<< # short version of adding/setting/resetting a new Message - <<< msgs["HelloWorld",2] = "Greeting the world in German", "Hallo, Welt!" - <<< assert msgs["HelloWorld",2] == ("Greeting the world in German","Hallo, Welt!") - <<< msgs["HelloWorld",2] = "Greeting the world in German", "Huhu, Welt!" - <<< assert msgs["HelloWorld",2] == ("Greeting the world in German","Huhu, Welt!") - <<< del msgs["HelloWorld",2] - <<< assert msgs.get("HelloWorld",2) == None - - # this Message has no code and no description (make easy things easy...) - <<< - <<< msgs["HelloWorld"] = "Hello!" - <<< assert msgs["HelloWorld"] == "Hello!" - - (to be continued...) """ def clear_server_messages(self): - """Removes all error, warning and info messages.""" + """Removes all messages of type error, warning and info. All other + messages types are custom types which should be handled by custom + code.""" rem = [] for m in self: @@ -2474,9 +2450,18 @@ class _Messages(dict): for m in rem: self.remove(m) - return self - + ####################################################################### + # can be removed after 01.07.24 + # default implementation of list is sufficient def __setitem__(self, key, value): # @ReservedAssignment + if not isinstance(value, Message): + warn("__setitem__ will in future only accept Message objects as second argument. " + "You will no longe be" + " able to pass bodys such that Message object is created on the fly", + DeprecationWarning) + if not isinstance(key, int): + warn("__setitem__ will in future only accept int as first argument", + DeprecationWarning) if isinstance(key, tuple): if len(key) == 2: type = key[0] # @ReservedAssignment @@ -2487,7 +2472,7 @@ class _Messages(dict): else: raise TypeError( "('type', 'code'), ('type'), or 'type' expected.") - elif isinstance(key, _Messages._msg_key): + elif isinstance(key, Messages._msg_key): type = key._type # @ReservedAssignment code = key._code else: @@ -2508,13 +2493,19 @@ class _Messages(dict): if isinstance(value, Message): body = value.body description = value.description + m = Message else: body = value description = None - m = Message(type=type, code=code, description=description, body=body) - dict.__setitem__(self, _Messages._msg_key(type, code), m) + m = Message(type=type, code=code, description=description, body=body) + if isinstance(key, int): + super().__setitem__(key, m) + else: + self.append(m) def __getitem__(self, key): + if not isinstance(key, int): + warn("__getitem__ only supports integer keys in future.", DeprecationWarning) if isinstance(key, tuple): if len(key) == 2: type = key[0] # @ReservedAssignment @@ -2525,113 +2516,118 @@ class _Messages(dict): else: raise TypeError( "('type', 'code'), ('type'), or 'type' expected.") - elif isinstance(key, int) and int(key) >= 0: - for m in self.values(): - if key == 0: - return m - else: - key -= 1 - type = key # @ReservedAssignment - code = None + elif isinstance(key, int) and key >= 0: + return super().__getitem__(key) else: type = key # @ReservedAssignment code = None - m = dict.__getitem__(self, _Messages._msg_key(type, code)) - + m = self.get(type, code) + if m is None: + raise KeyError() if m.description: return (m.description, m.body) else: return m.body - def __init__(self): - dict.__init__(self) - def __delitem__(self, key): if isinstance(key, tuple): - if len(key) == 2: - type = key[0] # @ReservedAssignment - code = key[1] - elif len(key) == 1: - type = key[0] # @ReservedAssignment - code = None - else: - raise TypeError( - "('type', 'code'), ('type'), or 'type' expected.") + warn("__delitem__ only supports integer keys in future.", DeprecationWarning) + if self.get(key[0], key[1]) is not None: + self.remove(self.get(key[0], key[1])) else: - type = key # @ReservedAssignment - code = None - - return dict.__delitem__(self, _Messages._msg_key(type, code)) + super().__delitem__(key) def remove(self, obj, obj2=None): - if isinstance(obj, Message): - return dict.__delitem__(self, _Messages._msg_key.get(obj)) + if obj2 is not None: + warn("Supplying a second argument to remove is deprecated.", + DeprecationWarning) + super().remove(self.get(obj, obj2)) + else: + super().remove(obj) - return self.__delitem__((obj, obj2)) + def append(self, msg): + if isinstance(msg, Messages) or isinstance(msg, list): + warn("Supplying a list-like object to append is deprecated. Please use extend" + " instead.", DeprecationWarning) + for m in msg: + self.append(m) + return - def get(self, type, code=None, default=None): # @ReservedAssignment - try: - return dict.__getitem__(self, _Messages._msg_key(type, code)) - except KeyError: - return default + super().append(msg) - def extend(self, messages): - self.append(messages) + @staticmethod + def _hash(t, c): + return hash(str(t).lower() + (str(",") + str(c) if c is not None else '')) + # end remove + ####################################################################### - return self + def get(self, type, code=None, default=None, exact=False): # @ReservedAssignment + """ + returns a message from the list that kind of matches type and code - def append(self, msg): - if hasattr(msg, "__iter__"): - for m in msg: - self.append(m) + case and types (str/int) are ignored - return self + If no suitable message is found, the default argument is returned + If exact=True, the message has to match code and type exactly + """ + if not exact: + warn("The fuzzy mode (exact=False) is deprecated. Please use exact in future.", + DeprecationWarning) + + for msg in self: + if exact: + if msg.type == type and msg.code == code: + return msg + else: + if self._hash(msg.type, msg.code) == self._hash(type, code): + return msg - if isinstance(msg, Message): - dict.__setitem__(self, _Messages._msg_key.get(msg), msg) + return default - return self - else: - raise TypeError("Argument was not a Message") + def to_xml(self, add_to_element): + for m in self: + melem = m.to_xml() + add_to_element.append(melem) - return self + def __repr__(self): + xml = etree.Element("Messages") + self.to_xml(xml) - def __iter__(self): - return dict.values(self).__iter__() + return xml2str(xml) + ####################################################################### + # can be removed after 01.07.24 class _msg_key: def __init__(self, type, code): # @ReservedAssignment + warn("This class is deprecated.", DeprecationWarning) self._type = type self._code = code @staticmethod def get(msg): - return _Messages._msg_key(msg.type, msg.code) + return Messages._msg_key(msg.type, msg.code) def __eq__(self, obj): return self.__hash__() == obj.__hash__() def __hash__(self): - return hash(str(self._type).lower() + (str(",") + - str(self._code) if self._code is not None else '')) + return hash(str(self._type).lower() + (str(",") + str(self._code) + if self._code is not None else '')) def __repr__(self): return str(self._type) + (str(",") + str(self._code) if self._code is not None else '') + # end remove + ####################################################################### - def to_xml(self, add_to_element): - for m in self: - melem = m.to_xml() - add_to_element.append(melem) - - return self - def __repr__(self): - xml = etree.Element("Messages") - self.to_xml(xml) - - return xml2str(xml) +class _Messages(Messages): + def __init__(self, *args, **kwargs): + warn("_Messages is deprecated. " + "Use class Messages instead and beware of the slightly different API of the new" + " Messages class", DeprecationWarning) + super().__init__(*args, **kwargs) def _basic_sync(e_local, e_remote): @@ -2834,7 +2830,7 @@ class Container(list): list.__init__(self) self._timestamp = None self._srid = None - self.messages = _Messages() + self.messages = Messages() def extend(self, entities): """Extend this Container by appending all single entities in the given @@ -2927,11 +2923,11 @@ class Container(list): def get_errors(self): """Get all error messages of this container. - @return _Messages: Error messages. + @return Messages: Error messages. """ if self.has_errors(): - ret = _Messages() + ret = Messages() for m in self.messages: if m.type.lower() == "error": @@ -2944,11 +2940,11 @@ class Container(list): def get_warnings(self): """Get all warning messages of this container. - @return _Messages: Warning messages. + @return Messages: Warning messages. """ if self.has_warnings(): - ret = _Messages() + ret = Messages() for m in self.messages: if m.type.lower() == "warning": @@ -2959,7 +2955,7 @@ class Container(list): return None def get_all_messages(self): - ret = _Messages() + ret = Messages() for e in self: ret.extend(e.get_all_messages()) @@ -3142,7 +3138,7 @@ class Container(list): msg = "Request was not unique. CUID " + \ str(local_entity._cuid) + " was found " + \ str(len(sync_remote_entities)) + " times." - local_entity.add_message(Message("Error", None, msg)) + local_entity.add_message(Message(description=msg, type="Error")) if raise_exception_on_error: raise MismatchingEntitiesError(msg) @@ -3167,7 +3163,7 @@ class Container(list): msg = "Request was not unique. ID " + \ str(local_entity.id) + " was found " + \ str(len(sync_remote_entities)) + " times." - local_entity.add_message(Message("Error", None, msg)) + local_entity.add_message(Message(description=msg, type="Error")) if raise_exception_on_error: raise MismatchingEntitiesError(msg) @@ -3197,7 +3193,7 @@ class Container(list): msg = "Request was not unique. Path " + \ str(local_entity.path) + " was found " + \ str(len(sync_remote_entities)) + " times." - local_entity.add_message(Message("Error", None, msg)) + local_entity.add_message(Message(description=msg, type="Error")) if raise_exception_on_error: raise MismatchingEntitiesError(msg) @@ -3227,7 +3223,7 @@ class Container(list): msg = "Request was not unique. Name " + \ str(local_entity.name) + " was found " + \ str(len(sync_remote_entities)) + " times." - local_entity.add_message(Message("Error", None, msg)) + local_entity.add_message(Message(description=msg, type="Error")) if raise_exception_on_error: raise MismatchingEntitiesError(msg) @@ -3246,7 +3242,7 @@ class Container(list): msg = "Request was not unique. There are " + \ str(len(sync_remote_entities)) + \ " entities which could not be matched to one of the requested ones." - remote_container.add_message(Message("Error", None, msg)) + remote_container.add_message(Message(description=msg, type="Error")) if raise_exception_on_error: raise MismatchingEntitiesError(msg) @@ -4309,7 +4305,7 @@ class Query(): The query string. flags : dict of str A dictionary of flags to be send with the query request. - messages : _Messages() + messages : Messages() A container of messages included in the last query response. cached : bool indicates whether the server used the query cache for the execution of @@ -4332,7 +4328,7 @@ class Query(): def __init__(self, q): self.flags = dict() - self.messages = _Messages() + self.messages = Messages() self.cached = None self.etag = None @@ -4456,6 +4452,10 @@ def execute_query(q, unique=False, raise_exception_on_error=True, cache=True, fl class DropOffBox(list): + def __init__(self, *args, **kwargs): + warn(DeprecationWarning( + "The DropOffBox is deprecated and will be removed in future.")) + super().__init__(*args, **kwargs) path = None @@ -4499,7 +4499,7 @@ class UserInfo(): class Info(): def __init__(self): - self.messages = _Messages() + self.messages = Messages() self.sync() def sync(self): @@ -4646,7 +4646,7 @@ def _parse_single_xml_element(elem): elif elem.tag.lower() == 'stats': counts = elem.find("counts") - return Message(type="Counts", body=counts.attrib) + return Message(type="Counts", description=None, body=counts.attrib) elif elem.tag == "EntityACL": return ACL(xml=elem) elif elem.tag == "Permissions": diff --git a/src/caosdb/utils/checkFileSystemConsistency.py b/src/caosdb/utils/checkFileSystemConsistency.py index a142c1dd7ffd1a4e6ee6cfc85891e1bf70f98d89..6c053fdca6acb3a6585589c0e6298ba0704ea590 100755 --- a/src/caosdb/utils/checkFileSystemConsistency.py +++ b/src/caosdb/utils/checkFileSystemConsistency.py @@ -30,7 +30,6 @@ import caosdb as db from argparse import ArgumentParser from argparse import RawDescriptionHelpFormatter -from _testcapi import raise_exception __all__ = [] __version__ = 0.1 diff --git a/unittests/test_high_level_api.py b/unittests/test_high_level_api.py index a9e55c9c2a79f7ead8bbb3fb652c1b81427e69e9..51993b78b700236618daef2f07dbe754121384c4 100644 --- a/unittests/test_high_level_api.py +++ b/unittests/test_high_level_api.py @@ -154,7 +154,7 @@ def test_convert_with_references(): obj = convert_to_python_object(r) assert obj.ref.a == 42 # Parent does not automatically lead to a datatype: - assert obj.get_property_metadata("ref").datatype is "bla" + assert obj.get_property_metadata("ref").datatype == "bla" assert obj.ref.has_parent("bla") is True # Unresolved Reference: @@ -163,7 +163,7 @@ def test_convert_with_references(): obj = convert_to_python_object(r) # Parent does not automatically lead to a datatype: - assert obj.get_property_metadata("ref").datatype is "bla" + assert obj.get_property_metadata("ref").datatype == "bla" assert isinstance(obj.ref, CaosDBPythonUnresolvedReference) assert obj.ref.id == 27 diff --git a/unittests/test_message.py b/unittests/test_message.py index 5e1003056c1b606a004b63bb7618e5e0474952bc..440e7169501afb0a35acb78df95cefae01bd9426 100644 --- a/unittests/test_message.py +++ b/unittests/test_message.py @@ -27,11 +27,14 @@ import caosdb as db from copy import deepcopy +import pytest + + def test_messages_dict_behavior(): from caosdb.common.models import Message - from caosdb.common.models import _Messages + from caosdb.common.models import Messages - msgs = _Messages() + msgs = Messages() # create Message msg = Message( @@ -40,12 +43,12 @@ def test_messages_dict_behavior(): description="Greeting the world", body="Hello, world!") - # append it to the _Messages + # append it to the Messages assert repr(msg) == '<HelloWorld code="1" description="Greeting the world">Hello, world!</HelloWorld>\n' msgs.append(msg) assert len(msgs) == 1 - # use _Messages as list of Message objects + # use Messages as list of Message objects for m in msgs: assert isinstance(m, Message) @@ -70,10 +73,6 @@ def test_messages_dict_behavior(): assert msgs["HelloWorld", 2] == ( "Greeting the world in German", "Hallo, Welt!") - msgs["HelloWorld", 2] = "Greeting the world in German", "Huhu, Welt!" - assert len(msgs) == 1 - assert msgs["HelloWorld", 2] == ( - "Greeting the world in German", "Huhu, Welt!") del msgs["HelloWorld", 2] assert msgs.get("HelloWorld", 2) is None @@ -83,11 +82,11 @@ def test_messages_dict_behavior(): def test_deepcopy(): - """Test whether deepcopy of _Messages objects doesn't mess up + """Test whether deepcopy of Messages objects doesn't mess up contained Messages objects. """ - msgs = db.common.models._Messages() + msgs = db.common.models.Messages() msg = db.Message(type="bla", code=1234, description="desc", body="blabla") msgs.append(msg) msg_copy = deepcopy(msgs)[0] @@ -102,7 +101,7 @@ def test_deepcopy(): def test_deepcopy_clear_server(): - msgs = db.common.models._Messages() + msgs = db.common.models.Messages() msg = db.Message(type="bla", code=1234, description="desc", body="blabla") err_msg = db.Message(type="Error", code=1357, description="error") msgs.extend([msg, err_msg]) @@ -116,3 +115,18 @@ def test_deepcopy_clear_server(): copied_msgs.clear_server_messages() assert len(copied_msgs) == 1 assert copied_msgs[0].code == msg.code + + +def test_list_behavior(): + msgs = db.common.models.Messages() + msgs.append(db.Message("test")) + assert len(msgs) == 1 + assert msgs[0] == db.Message("test") + assert msgs[0] != db.Message("test2") + + msgs.append(db.Message("test")) + assert len(msgs) == 2 + assert msgs[0] == msgs[1] + + with pytest.raises(IndexError): + msgs[3]