undo duplication of add_msg method in mail api

* Set the internal date from within the incoming mail service.

3 files changed, 66 insertions, 64 deletions

diff --git a/mail/src/leap/mail/imap/mailbox.py b/mail/src/leap/mail/imap/mailbox.py
index 3769a3ed..c5016146 100644
--- a/mail/src/leap/mail/imap/mailbox.py
+++ b/mail/src/leap/mail/imap/mailbox.py
@@ -20,8 +20,12 @@ IMAP Mailbox.
 import re
 import logging
 import os
+import cStringIO
+import StringIO
+import time
 
 from collections import defaultdict
+from email.utils import formatdate
 
 from twisted.internet import defer
 from twisted.internet import reactor
@@ -31,6 +35,7 @@ from twisted.mail import imap4
 from zope.interface import implements
 
 from leap.common.check import leap_assert
+from leap.common.check import leap_assert_type
 from leap.mail.constants import INBOX_NAME, MessageFlags
 from leap.mail.imap.messages import IMAPMessage
 
@@ -50,7 +55,6 @@ NOTIFY_NEW = not os.environ.get('LEAP_SKIPNOTIFY', False)
 PROFILE_CMD = os.environ.get('LEAP_PROFILE_IMAPCMD', False)
 
 if PROFILE_CMD:
-    import time
 
     def _debugProfiling(result, cmdname, start):
         took = (time.time() - start) * 1000
@@ -315,12 +319,41 @@ class IMAPMailbox(object):
         :type flags: list of str
 
         :param date: timestamp
-        :type date: str
+        :type date: str, or None
 
         :return: a deferred that will be triggered with the UID of the added
                  message.
         """
-        return self.collection.add_raw_message(message, flags, date)
+        # TODO should raise ReadOnlyMailbox if not rw.
+        # TODO have a look at the cases for internal date in the rfc
+        # XXX we could treat the message as an IMessage from here
+
+        if isinstance(message, (cStringIO.OutputType, StringIO.StringIO)):
+            message = message.getvalue()
+
+        leap_assert_type(message, basestring)
+
+        if flags is None:
+            flags = tuple()
+        else:
+            flags = tuple(str(flag) for flag in flags)
+
+        if date is None:
+            date = formatdate(time.time())
+
+        # notify_just_mdoc=True: feels HACKY, but improves a *lot* the
+        # responsiveness of the APPENDS: we just need to be notified when the
+        # mdoc is saved, and let's hope that the other parts are doing just
+        # fine.  This will not catch any errors when the inserts of the other
+        # parts fail, but on the other hand allows us to return very quickly,
+        # which seems a good compromise given that we have to serialize the
+        # appends.
+        # A better solution will probably involve implementing MULTIAPPEND
+        # extension or patching imap server to support pipelining.
+
+        # TODO add notify_new as a callback here...
+        return self.collection.add_msg(message, flags, date,
+                                       notify_just_mdoc=True)
 
     def notify_new(self, *args):
         """
diff --git a/mail/src/leap/mail/incoming/service.py b/mail/src/leap/mail/incoming/service.py
index 8b5c3713..ea790fe8 100644
--- a/mail/src/leap/mail/incoming/service.py
+++ b/mail/src/leap/mail/incoming/service.py
@@ -27,6 +27,7 @@ import warnings
 from email.parser import Parser
 from email.generator import Generator
 from email.utils import parseaddr
+from email.utils import formatdate
 from StringIO import StringIO
 from urlparse import urlparse
 
@@ -117,7 +118,8 @@ class IncomingMail(Service):
         :param soledad: a soledad instance
         :type soledad: Soledad
 
-        :param inbox: the inbox where the new emails will be stored
+        :param inbox: the collection for the inbox where the new emails will be
+                      stored
         :type inbox: MessageCollection
 
         :param check_period: the period to fetch new mail, in seconds.
@@ -132,7 +134,7 @@ class IncomingMail(Service):
 
         self._keymanager = keymanager
         self._soledad = soledad
-        self._inbox = inbox
+        self._inbox_collection = inbox
         self._userid = userid
 
         self._listeners = []
@@ -266,7 +268,7 @@ class IncomingMail(Service):
         Sends unread event to ui.
         """
         leap_events.signal(
-            IMAP_UNREAD_MAIL, str(self._inbox.count_unseen()))
+            IMAP_UNREAD_MAIL, str(self._inbox_collection.count_unseen()))
 
     # process incoming mail.
 
@@ -710,7 +712,8 @@ class IncomingMail(Service):
         :return: A Deferred that will be fired when the messages is stored
         :rtype: Defferred
         """
-        doc, data = msgtuple
+        doc, raw_data = msgtuple
+        insertion_date = formatdate(time.time())
         log.msg('adding message %s to local db' % (doc.doc_id,))
 
         def msgSavedCallback(result):
@@ -729,7 +732,8 @@ class IncomingMail(Service):
             d.addCallback(signal_deleted)
             return d
 
-        d = self._inbox.add_raw_message(data, (self.RECENT_FLAG,))
+        d = self._inbox_collection.add_msg(
+            raw_data, (self.RECENT_FLAG,), date=insertion_date)
         d.addCallbacks(msgSavedCallback, self._errback)
         return d
 
diff --git a/mail/src/leap/mail/mail.py b/mail/src/leap/mail/mail.py
index 3127ef5e..f9e99f00 100644
--- a/mail/src/leap/mail/mail.py
+++ b/mail/src/leap/mail/mail.py
@@ -20,10 +20,6 @@ Generic Access to Mail objects: Public LEAP Mail API.
 import uuid
 import logging
 import StringIO
-import cStringIO
-import time
-
-from email.utils import formatdate
 from twisted.internet import defer
 
 from leap.common.check import leap_assert_type
@@ -521,6 +517,15 @@ class MessageCollection(object):
         """
         Add a message to this collection.
 
+        :param raw_message: the raw message
+        :param flags: tuple of flags for this message
+        :param tags: tuple of tags for this message
+        :param date:
+            formatted date, it will be used to retrieve the internal
+            date for this message.  According to the spec, this is NOT the date
+            and time in the RFC-822 header, but rather a date and time that
+            reflects when the message was received.
+        :type date: str
         :param notify_just_mdoc:
             boolean passed to the wrapper.create method,
             to indicate whether we're interested in being notified when only
@@ -533,7 +538,11 @@ class MessageCollection(object):
                   message.
         :rtype: deferred
         """
+        # TODO watch out if the use of this method in IMAP COPY/APPEND is
+        # passing the right date.
+
         # XXX mdoc ref is a leaky abstraction here. generalize.
+
         leap_assert_type(flags, tuple)
         leap_assert_type(date, str)
 
@@ -558,66 +567,22 @@ class MessageCollection(object):
         d = wrapper.create(self.store, notify_just_mdoc=notify_just_mdoc)
         d.addCallback(insert_mdoc_id, wrapper)
         d.addErrback(lambda f: f.printTraceback())
-        return d
-
-    def add_raw_message(self, message, flags, date=None):
-        """
-        Adds a message to this collection.
-
-        :param message: the raw message
-        :type message: str
-
-        :param flags: flag list
-        :type flags: list of str
-
-        :param date: timestamp
-        :type date: str
-
-        :return: a deferred that will be triggered with the UID of the added
-                 message.
-        """
-        # TODO should raise ReadOnlyMailbox if not rw.
-        # TODO have a look at the cases for internal date in the rfc
-        if isinstance(message, (cStringIO.OutputType, StringIO.StringIO)):
-            message = message.getvalue()
-
-        # XXX we could treat the message as an IMessage from here
-        leap_assert_type(message, basestring)
-
-        if flags is None:
-            flags = tuple()
-        else:
-            flags = tuple(str(flag) for flag in flags)
-
-        if date is None:
-            date = formatdate(time.time())
-
-        # A better place for this would be  the COPY/APPEND dispatcher
-        # if PROFILE_CMD:
-        # do_profile_cmd(d, "APPEND")
-
-        # just_mdoc=True: feels HACKY, but improves a *lot* the responsiveness
-        # of the APPENDS: we just need to be notified when the mdoc
-        # is saved, and let's hope that the other parts are doing just fine.
-        # This will not catch any errors when the inserts of the other parts
-        # fail, but on the other hand allows us to return very quickly, which
-        # seems a good compromise given that we have to serialize the appends.
-        # A better solution will probably involve implementing MULTIAPPEND
-        # or patching imap server to support pipelining.
-
-        d = self.add_msg(message, flags=flags, date=date,
-                         notify_just_mdoc=True)
-        d.addErrback(lambda f: logger.warning(f.getTraceback()))
         d.addCallback(self.cb_signal_unread_to_ui)
         return d
 
     def cb_signal_unread_to_ui(self, result):
         """
-        Sends unread event to ui.
+        Sends an unread event to ui, passing *only* the number of unread
+        messages if *this* is the inbox. This event is catched, for instance,
+        in the Bitmask client that displays a message with the number of unread
+        mails in the INBOX.
+
         Used as a callback in several commands.
 
         :param result: ignored
         """
+        # TODO it might make sense to modify the event so that
+        # it receives both the mailbox name AND the number of unread messages.
         if self.mbox_name.lower() == "inbox":
             d = defer.maybeDeferred(self.count_unseen)
             d.addCallback(self.__cb_signal_unread_to_ui)
@@ -629,7 +594,7 @@ class MessageCollection(object):
         :param unseen: number of unseen messages.
         :type unseen: int
         """
-        # TODO change name of the signal, non-imap now.
+        # TODO change name of the signal, independent from imap now.
         leap_events.signal(IMAP_UNREAD_MAIL, str(unseen))
 
     def copy_msg(self, msg, new_mbox_uuid):