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

@@ -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
@@ -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)
 
 
@@ -2400,9 +2401,8 @@ class _ParentList(list):
 
 
 class Messages(list):
-
-    """This 'kind of dictionary' stores error, warning, info, and other
-    messages. The mentioned three messages types are messages of special use.
+    """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,13 +2413,6 @@ class Messages(list):
 
     <$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:
@@ -2442,30 +2435,12 @@ class Messages(list):
     <<< 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 messages of type error, warning and info."""
-        # TODO Why do we not remove ALL 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:
@@ -2548,7 +2523,7 @@ class Messages(list):
             code = None
         m = self.get(type, code)
         if m is None:
-            return
+            raise KeyError()
         if m.description:
             return (m.description, m.body)
         else:
@@ -2580,6 +2555,7 @@ class Messages(list):
 
         super().append(msg)
 
+    @staticmethod
     def _hash(t, c):
         return hash(str(t).lower() + (str(",") + str(c) if c is not None else ''))
     # end remove
@@ -2600,10 +2576,10 @@ class Messages(list):
 
         for msg in self:
             if exact:
-                if self._hash(msg.t, msg.c) == self._hash(type, code):
+                if msg.type == type and msg.code == code:
                     return msg
             else:
-                if msg.type == type and msg.code == code:
+                if self._hash(msg.type, msg.code) == self._hash(type, code):
                     return msg
 
         return default
@@ -2636,7 +2612,8 @@ class Messages(list):
             return self.__hash__() == obj.__hash__()
 
         def __hash__(self):
-            return
+            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)
@@ -3161,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)
@@ -3186,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)
@@ -3216,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)
@@ -3246,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)
@@ -3265,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)
@@ -4669,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":