MAINT: refactor _Messages class

Summary

For caosdb-server#286 (closed)

• rename _Messages to Messages since it is public API
• change parent class to list since Multiple Messages with same type and code may occur
• deprecate lots of stuff that makes usage fuzzy (e.g. three different ways to supply arguments) and concentrate on behavior that is close to that of lists (which is what users know)

Focus

It is quite hard to figure out what the current behavior is. However, I preserved it for now and added lots of deprecation warnings.

Test Environment

unit and int test

Check List for the Author

Please, prepare your MR for a review. Be sure to write a summary and a focus and create gitlab comments for the reviewer. They should guide the reviewer through the changes, explain your changes and also point out open questions. For further good practices have a look at our review guidelines

• All automated tests pass
• Reference related issues
• Up-to-date CHANGELOG.md (or not necessary)
• Up-to-date JSON schema (or not necessary)
• Appropriate user and developer documentation (or not necessary)
  • How do I use the software? Assume "stupid" users.
  • How do I develop or debug the software? Assume novice developers.
• Annotations in code (Gitlab comments)
  • Intent of new code
  • Problems with old code
  • Why this implementation?

Check List for the Reviewer

• I understand the intent of this MR
• All automated tests pass
• Up-to-date CHANGELOG.md (or not necessary)
• Appropriate user and developer documentation (or not necessary)
• The test environment setup works and the intended behavior is reproducible in the test environment
• In-code documentation and comments are up-to-date.
• Check: Are there specifications? Are they satisfied?

For further good practices have a look at our review guidelines.

src/caosdb/common/models.py

 # -*- coding: utf-8 -*-
 #
-# ** header v3.0
 # This file is a part of the CaosDB Project.
 #
 # Copyright (C) 2018 Research Group Biomedical Physics,
 # Max-Planck-Institute for Dynamics and Self-Organization Göttingen
-# Copyright (C) 2020-2022 Indiscale GmbH <info@indiscale.com>
-# Copyright (C) 2020 Florian Spreckelsen <f.spreckelsen@indiscale.com>
+# Copyright (C) 2020-2023 Indiscale GmbH <info@indiscale.com>
+# Copyright (C) 2020-2023 Florian Spreckelsen <f.spreckelsen@indiscale.com>
 # Copyright (C) 2020-2022 Timm Fitschen <t.fitschen@indiscale.com>
 #
 # This program is free software: you can redistribute it and/or modify
@@ -22,7 +21,6 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with this program. If not, see <https://www.gnu.org/licenses/>.
 #
-# ** end header
 #
 
 """
@@ -34,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
@@ -47,13 +45,14 @@ from os import listdir
 from os.path import isdir
 from random import randint
 from tempfile import NamedTemporaryFile
+from typing import Any, Optional
 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
@@ -113,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
@@ -420,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:
@@ -453,6 +452,66 @@ class Entity:
 
         return self
 
+    def remove_value_from_property(self, property_name: str, value: Any,
+                                   remove_if_empty_afterwards: Optional[bool] = True):
+        """Remove a value from a property given by name.
+
+        Do nothing if this entity does not have a property of this
+        ``property_name`` or if the property value is different of the given
+        ``value``. By default, the property is removed from this entity if it
+        becomes empty (i.e., value=None) through removal of the value. This
+        behavior can be changed by setting ``remove_if_empty_afterwards`` to
+        ``False`` in which case the property remains.
+
+        Notes
+        -----
+        If the property value is a list and the value to be removed occurs more
+        than once in this list, only its first occurrance is deleted (similar
+        to the behavior of Python's ``list.remove()``.)
+
+        If the property was empty (prop.value == None) before, the property is
+        not removed afterwards even if ``remove_if_empty_afterwards`` is set to
+        ``True``.  Rationale: the property being empty is not an effect of
+        calling this function.
+
+        Parameters
+        ----------
+        property_name : str
+            Name of the property from which the ``value`` will be removed.
+
+        value
+            Value that is to be removed.
+
+        remove_if_empty_afterwards : bool, optional
+            Whether the property shall be removed from this entity if it is
+            emptied by removing the ``value``. Default is ``True``.
+
+        Returns
+        -------
+        self
+            This entity.
+
+        """
+
+        if self.get_property(property_name) is None:
+            return self
+        if self.get_property(property_name).value is None:
+            remove_if_empty_afterwards = False
+        empty_afterwards = False
+        if isinstance(self.get_property(property_name).value, list):
+            if value in self.get_property(property_name).value:
+                self.get_property(property_name).value.remove(value)
+                if self.get_property(property_name).value == []:
+                    self.get_property(property_name).value = None
+                    empty_afterwards = True
+        elif self.get_property(property_name).value == value:
+            self.get_property(property_name).value = None
+            empty_afterwards = True
+        if remove_if_empty_afterwards and empty_afterwards:
+            self.remove_property(property_name)
+
+        return self
+
     def remove_parent(self, parent):
         self.parents.remove(parent)
 
@@ -997,7 +1056,7 @@ out: List[Entity]
     def get_messages(self):
         """Get all messages of this entity.
 
-        @return: _Messages(list)
+        @return: Messages(list)
         """
 
         return self.messages
@@ -1005,9 +1064,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":
@@ -1018,9 +1077,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":
@@ -1540,7 +1599,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
@@ -1660,7 +1719,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":
@@ -2358,21 +2417,21 @@ class Messages(list):
     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
+    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)
 
@@ -2406,6 +2465,7 @@ class Messages(list):
 
     def clear_server_messages(self):
         """Removes all messages of type error, warning and info."""
+        # TODO Why do we not remove ALL messages?
         rem = []
 
         for m in self:
@@ -2415,6 +2475,9 @@ class Messages(list):
         for m in rem:
             self.remove(m)
 
+    #######################################################################
+    # 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. "
@@ -2434,7 +2497,7 @@ class Messages(list):
             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:
@@ -2478,7 +2541,7 @@ class Messages(list):
             else:
                 raise TypeError(
                     "('type', 'code'), ('type'), or 'type' expected.")
-        elif isinstance(key, int) and int(key) >= 0:
+        elif isinstance(key, int) and key >= 0:
             return super().__getitem__(key)
         else:
             type = key  # @ReservedAssignment
@@ -2507,17 +2570,8 @@ class Messages(list):
         else:
             super().remove(obj)
 
-    def get(self, type, code=None, default=None):  # @ReservedAssignment
-        for msg in self:
-            if msg.type == type and msg.code == code:
-                return msg
-        return default
-
-    def extend(self, messages):
-        super().extend(messages)
-
     def append(self, msg):
-        if isinstance(msg, _Messages) or isinstance(msg, list):
+        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:
@@ -2526,6 +2580,34 @@ class Messages(list):
 
         super().append(msg)
 
+    def _hash(t, c):
+        return hash(str(t).lower() + (str(",") + str(c) if c is not None else ''))
+    # end remove
+    #######################################################################
+
+    def get(self, type, code=None, default=None, exact=False):  # @ReservedAssignment
+        """
+        returns a message from the list that kind of matches type and code
+
+        case and types (str/int) are ignored
+
+        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 self._hash(msg.t, msg.c) == self._hash(type, code):
+                    return msg
+            else:
+                if msg.type == type and msg.code == code:
+                    return msg
+
+        return default
+
     def to_xml(self, add_to_element):
         for m in self:
             melem = m.to_xml()
@@ -2537,6 +2619,8 @@ class Messages(list):
 
         return xml2str(xml)
 
+    #######################################################################
+    # can be removed after 01.07.24
     class _msg_key:
 
         def __init__(self, type, code):  # @ReservedAssignment
@@ -2546,18 +2630,19 @@ class Messages(list):
 
         @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
 
         def __repr__(self):
             return str(self._type) + (str(",") + str(self._code)
                                       if self._code is not None else '')
+    # end remove
+    #######################################################################
 
 
 class _Messages(Messages):
@@ -2768,7 +2853,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
@@ -2861,11 +2946,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":
@@ -2878,11 +2963,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":
@@ -2893,7 +2978,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())
@@ -4243,7 +4328,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
@@ -4266,7 +4351,7 @@ class Query():
 
     def __init__(self, q):
         self.flags = dict()
-        self.messages = _Messages()
+        self.messages = Messages()
         self.cached = None
         self.etag = None
 
@@ -4437,7 +4522,7 @@ class UserInfo():
 class Info():
 
     def __init__(self):
-        self.messages = _Messages()
+        self.messages = Messages()
         self.sync()
 
     def sync(self):