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

@@ -2339,7 +2339,7 @@ class _ParentList(list):
         raise KeyError(str(parent) + " not found.")
 
 
-class _Messages(dict):
+class _Messages(list):
 
     """This 'kind of dictionary' stores error, warning, info, and other
     messages. The mentioned three messages types are messages of special use.
@@ -2404,6 +2404,7 @@ class _Messages(dict):
     """
 
     def clear_server_messages(self):
+        # TODO can we deprecate this and refer people to clear
         """Removes all error, warning and info messages."""
         rem = []
 
@@ -2414,9 +2415,17 @@ class _Messages(dict):
         for m in rem:
             self.remove(m)
 
-        return self
+        return _message_wrap_deprication(self)
 
     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
@@ -2448,13 +2457,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
@@ -2466,82 +2481,73 @@ class _Messages(dict):
                 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
+            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:
+            return
         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))
-
-        return self.__delitem__((obj, obj2))
+        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)
 
     def get(self, type, code=None, default=None):  # @ReservedAssignment
-        try:
-            return dict.__getitem__(self, _Messages._msg_key(type, code))
-        except KeyError:
-            return default
+        for msg in self:
+            if msg.type == type and msg.code == code:
+                return msg
+        return default
 
     def extend(self, messages):
-        self.append(messages)
-
-        return self
+        super().extend(messages)
+        return _message_wrap_deprication(self)
 
     def append(self, msg):
-        if hasattr(msg, "__iter__"):
+        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 _message_wrap_deprication(self)
 
-            return self
+        super().append(msg)
 
-        if isinstance(msg, Message):
-            dict.__setitem__(self, _Messages._msg_key.get(msg), msg)
+        return _message_wrap_deprication(self)
 
-            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
+        return _message_wrap_deprication(self)
+
+    def __repr__(self):
+        xml = etree.Element("Messages")
+        self.to_xml(xml)
 
-    def __iter__(self):
-        return dict.values(self).__iter__()
+        return xml2str(xml)
 
     class _msg_key:
 
         def __init__(self, type, code):  # @ReservedAssignment
+            warn("This class is deprecated.", DeprecationWarning)
             self._type = type
             self._code = code
 
@@ -2560,18 +2566,31 @@ class _Messages(dict):
             return str(self._type) + (str(",") + str(self._code)
                                       if self._code is not None else '')
 
-    def to_xml(self, add_to_element):
-        for m in self:
-            melem = m.to_xml()
-            add_to_element.append(melem)
 
-        return self
+def donotuse(func):
+    """ used by _message_wrap_deprication to deprecate return values"""
+    def inner(*args, **kwargs):
+        warn(DeprecationWarning(
+                "This object was returned by a function that will in future return None."))
+        return func(*args, **kwargs)
 
-    def __repr__(self):
-        xml = etree.Element("Messages")
-        self.to_xml(xml)
+    return inner
 
-        return xml2str(xml)
+
+def _message_wrap_deprication(msg):
+    """ function to allow deprecation of _Messages return values """
+    msg.clear_server_messages = donotuse(msg.clear_server_messages)
+    msg.__setitem__ = donotuse(msg.__setitem__)
+    msg.__delitem__ = donotuse(msg.__delitem__)
+    msg.__getitem__ = donotuse(msg.__getitem__)
+    msg.remove = donotuse(msg.remove)
+    msg.get = donotuse(msg.get)
+    msg.extend = donotuse(msg.extend)
+    msg.append = donotuse(msg.append)
+    msg.__iter__ = donotuse(msg.__iter__)
+    msg.to_xml = donotuse(msg.to_xml)
+    msg.__repr__ = donotuse(msg.__repr__)
+    return msg
 
 
 def _basic_sync(e_local, e_remote):
@@ -4396,6 +4415,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