core (XMPP): sendMessage refactoring:

- better separation of message sending actions
    - use of more generic exceptions to hook the behaviour (SkipHistory and CancelError)
    - use of raise instead of return
    - use of failure.trap

src/core/exceptions.py

@@ -92,3 +92,6 @@ class DatabaseError(Exception):
 
 class PasswordError(Exception):
     pass
+
+class SkipHistory(Exception): # used in MessageReceivedTrigger to avoid history writting
+    pass

src/core/sat_main.py

@@ -54,22 +54,6 @@ def sat_next_id():
     return "sat_id_" + str(sat_id)
 
 
-class MessageSentAndStored(Exception):
-    """ Exception to raise if the message has been already sent and stored in the
-    history by the trigger, so the rest of the process should be stopped. This
-    should normally be raised by the trigger with the minimal priority """
-    def __init__(self, reason, mess_data):
-        Exception.__init__(self, reason)
-        self.mess_data = mess_data  # added for testing purpose
-
-
-class AbortSendMessage(Exception):
-    """ Exception to raise if sending the message should be aborted. This can be
-    raised by any trigger but a side action should be planned by the trigger
-    to inform the user about what happened """
-    pass
-
-
 class SAT(service.Service):
 
     @property
@@ -433,7 +417,7 @@ class SAT(service.Service):
         return self.profiles[profile].isConnected()
 
 
-    ## jabber methods ##
+    ## XMPP methods ##
 
     def getWaitingConf(self, profile_key=None):
         assert(profile_key)
@@ -444,6 +428,18 @@ class SAT(service.Service):
             ret.append((conf_id, conf_type, data))
         return ret
 
+    def generateMessageXML(self, mess_data):
+        mess_data['xml'] = domish.Element((None, 'message'))
+        mess_data['xml']["to"] = mess_data["to"].full()
+        mess_data['xml']["from"] = mess_data['from'].full()
+        mess_data['xml']["type"] = mess_data["type"]
+        mess_data['xml']['id'] = str(uuid4())
+        if mess_data["subject"]:
+            mess_data['xml'].addElement("subject", None, mess_data['subject'])
+        if mess_data["message"]: # message without body are used to send chat states
+            mess_data['xml'].addElement("body", None, mess_data["message"])
+        return mess_data
+
     def _sendMessage(self, to_s, msg, subject=None, mess_type='auto', extra={}, profile_key=C.PROF_KEY_NONE):
         to_jid = jid.JID(to_s)
         #XXX: we need to use the dictionary comprehension because D-Bus return its own types, and pickle can't manage them. TODO: Need to find a better way
@@ -454,11 +450,11 @@ class SAT(service.Service):
         profile = self.memory.getProfileName(profile_key)
         assert(profile)
         client = self.profiles[profile]
-        current_jid = client.jid
         if extra is None:
             extra = {}
         mess_data = {  # we put data in a dict, so trigger methods can change them
             "to": to_jid,
+            "from": client.jid,
             "message": msg,
             "subject": subject,
             "type": mess_type,
@@ -493,67 +489,67 @@ class SAT(service.Service):
 
         log.debug(_("Sending jabber message of type [%(type)s] to %(to)s...") % {"type": mess_data["type"], "to": to_jid.full()})
 
-        def generateXML(mess_data):
+        def cancelErrorTrap(failure):
-            mess_data['xml'] = domish.Element((None, 'message'))
+            """A message sending can be cancelled by a plugin treatment"""
-            mess_data['xml']["to"] = mess_data["to"].full()
+            failure.trap(exceptions.CancelError)
-            mess_data['xml']["from"] = current_jid.full()
-            mess_data['xml']["type"] = mess_data["type"]
-            if mess_data["subject"]:
-                mess_data['xml'].addElement("subject", None, subject)
-            # message without body are used to send chat states
-            if mess_data["message"]:
-                mess_data['xml'].addElement("body", None, mess_data["message"])
-            return mess_data
 
-        def sendErrback(e):
+        pre_xml_treatments.addCallback(lambda dummy: self.generateMessageXML(mess_data))
-            text = '%s: %s' % (e.value.__class__.__name__, e.getErrorMessage())
-            if e.check(MessageSentAndStored):
-                log.debug(text)
-            elif e.check(AbortSendMessage):
-                log.warning(text)
-                return e
-            else:
-                log.error("Unmanaged exception: %s" % text)
-                return e
-        pre_xml_treatments.addCallback(generateXML)
         pre_xml_treatments.chainDeferred(post_xml_treatments)
-        post_xml_treatments.addCallback(self.sendAndStoreMessage, False, profile)
+        post_xml_treatments.addCallback(self._sendMessageToStream, client)
-        post_xml_treatments.addErrback(sendErrback)
+        post_xml_treatments.addCallback(self._storeMessage, client)
+        post_xml_treatments.addCallback(self.sendMessageToBridge, client)
+        post_xml_treatments.addErrback(cancelErrorTrap)
         pre_xml_treatments.callback(mess_data)
         return pre_xml_treatments
 
-    def sendAndStoreMessage(self, mess_data, skip_send=False, profile=None):
+    def _sendMessageToStream(self, mess_data, client):
-        """Actually send and store the message to history, after all the treatments have been done.
+        """Actualy send the message to the server
-        This has been moved outside the main sendMessage method because it is used by XEP-0033 to complete a server-side feature not yet
+
-        implemented by the prosody plugin.
+        @param mess_data: message data dictionnary
-        @param mess_data: message data dictionary
+        @param client: profile's client
-        @param skip_send: set to True to skip sending the message to only store it
+        """
-        @param profile: profile
+        client.xmlstream.send(mess_data['xml'])
+        return mess_data
+
+    def _storeMessage(self, mess_data, client):
+        """Store message into database (for local history)
+
+        @param mess_data: message data dictionnary
+        @param client: profile's client
         """
-        try:
-            client = self.profiles[profile]
-        except KeyError:
-            log.error(_("Trying to send a message with no profile"))
-            return
-        current_jid = client.jid
-        if not skip_send:
-            client.xmlstream.send(mess_data['xml'])
         if mess_data["type"] != "groupchat":
             # we don't add groupchat message to history, as we get them back
             # and they will be added then
             if mess_data['message']: # we need a message to save something
-                self.memory.addToHistory(current_jid, mess_data['to'],
+                self.memory.addToHistory(client.jid, mess_data['to'],
                                      unicode(mess_data["message"]),
                                      unicode(mess_data["type"]),
                                      mess_data['extra'],
-                                     profile=profile)
+                                     profile=client.profile)
+            else:
+               log.warning(_("No message found")) # empty body should be managed by plugins before this point
+        return mess_data
+
+    def sendMessageToBridge(self, mess_data, client):
+        """Send message to bridge, so frontends can display it
+
+        @param mess_data: message data dictionnary
+        @param client: profile's client
+        """
+        if mess_data["type"] != "groupchat":
+            # we don't send groupchat message back to bridge, as we get them back
+            # and they will be added the
+            if mess_data['message']: # we need a message to save something
                 # We send back the message, so all clients are aware of it
-                self.bridge.newMessage(mess_data['xml']['from'],
+                self.bridge.newMessage(mess_data['from'].full(),
                                        unicode(mess_data["message"]),
                                        mess_type=mess_data["type"],
-                                       to_jid=mess_data['xml']['to'],
+                                       to_jid=mess_data['to'].full(),
                                        extra=mess_data['extra'],
-                                       profile=profile)
+                                       profile=client.profile)
+            else:
+               log.warning(_("No message found"))
+        return mess_data
 
     def _setPresence(self, to="", show="", statuses=None, profile_key=C.PROF_KEY_NONE):
         return self.setPresence(jid.JID(to) if to else None, show, statuses, profile_key)

src/plugins/plugin_misc_text_commands.py

@@ -19,12 +19,12 @@
 
 from sat.core.i18n import _
 from sat.core.constants import Const as C
-from sat.core.sat_main import MessageSentAndStored
+from sat.core import exceptions
 from twisted.words.protocols.jabber import jid
 from twisted.internet import defer
-from twisted.python.failure import Failure
 from sat.core.log import getLogger
 log = getLogger(__name__)
+from twisted.python import failure
 
 PLUGIN_INFO = {
     "name": "Text commands",
@@ -128,7 +128,8 @@ class TextCommands(object):
                 if ret:
                     return mess_data
                 else:
-                    return Failure(MessageSentAndStored("text commands took over", mess_data))
+                    log.debug("text commands took over")
+                    raise failure.Failure(exceptions.CancelError())
 
             try:
                 mess_data["unparsed"] = msg[1 + len(command):]  # part not yet parsed of the message
@@ -136,7 +137,8 @@ class TextCommands(object):
                 d.addCallback(retHandling)
             except KeyError:
                 self.feedBack(_("Unknown command /%s. ") % command + self.HELP_SUGGESTION, mess_data, profile)
-                return Failure(MessageSentAndStored("text commands took over", mess_data))
+                self.debug("text commands took over")
+                raise failure.Failure(exceptions.CancelError())
 
         return d or mess_data  # if a command is detected, we should have a deferred, else we send the message normally
 

src/plugins/plugin_xep_0033.py

@@ -20,10 +20,11 @@
 from sat.core.i18n import _
 from sat.core.log import getLogger
 log = getLogger(__name__)
+from sat.core import exceptions
 from wokkel import disco, iwokkel
 from zope.interface import implements
 from twisted.words.protocols.jabber.jid import JID
-from twisted.python.failure import Failure
+from twisted.python import failure
 import copy
 try:
     from twisted.words.protocols.xmlstream import XMPPHandler
@@ -32,7 +33,6 @@ except ImportError:
 from twisted.words.xish import domish
 from twisted.internet import defer
 
-from sat.core.sat_main import MessageSentAndStored, AbortSendMessage
 from sat.tools.misc import TriggerManager
 from time import time
 
@@ -84,7 +84,8 @@ class XEP_0033(object):
 
             def discoCallback(entities):
                 if not entities:
-                    return Failure(AbortSendMessage(_("XEP-0033 is being used but the server doesn't support it!")))
+                    log.warning(_("XEP-0033 is being used but the server doesn't support it!"))
+                    raise failure.Failure(exceptions.CancelError())
                 if mess_data["to"] not in entities:
                     expected = _(' or ').join([entity.userhost() for entity in entities])
                     log.warning(_("Stanzas using XEP-0033 should be addressed to %(expected)s, not %(current)s!") % {'expected': expected, 'current': mess_data["to"]})
@@ -96,7 +97,8 @@ class XEP_0033(object):
                     element.addChild(domish.Element((None, 'address'), None, {'type': type_, 'jid': jid_}))
                 # when the prosody plugin is completed, we can immediately return mess_data from here
                 self.sendAndStoreMessage(mess_data, entries, profile)
-                return Failure(MessageSentAndStored("XEP-0033 took over", mess_data))
+                log.debug("XEP-0033 took over")
+                raise failure.Failure(exceptions.CancelError())
             d = self.host.findFeaturesSet([NS_ADDRESS], profile_key=profile)
             d.addCallbacks(discoCallback, lambda dummy: discoCallback(None))
             return d