[refactor] simplify wrapper create and add_msg

- remove premature optimization for fast-notifies. blobs will cover
that, no point in maintaning the optimization at the price of creeping
complexity.

7 files changed, 31 insertions, 151 deletions

diff --git a/src/leap/bitmask/mail/adaptors/soledad.py b/src/leap/bitmask/mail/adaptors/soledad.py
index 25e4ad31..56bca363 100644
--- a/src/leap/bitmask/mail/adaptors/soledad.py
+++ b/src/leap/bitmask/mail/adaptors/soledad.py
@@ -126,7 +126,6 @@ class SoledadDocumentWrapper(models.DocumentWrapper):
         """
         assert self.doc_id is None, "This document already has a doc_id!"
 
-        print "FUTURE", self.future_doc_id
         try:
             if self.future_doc_id is None:
                 newdoc = yield store.create_doc(
@@ -504,92 +503,39 @@ class MessageWrapper(object):
             cdoc.set_future_doc_id(doc_id)
 
     @defer.inlineCallbacks
-    def create(self, store, notify_just_mdoc=False, pending_inserts_dict=None):
+    def create(self, store):
         """
         Create all the parts for this message in the store.
 
         :param store: an instance of Soledad
 
-        :param notify_just_mdoc:
-            if set to True, this method will return *only* the deferred
-            corresponding to the creation of the meta-message document.
-            Be warned that in that case there will be no record of failures
-            when creating the other part-documents.
-
-            Otherwise, this method will return a deferred that will wait for
-            the creation of all the part documents.
-
-            Setting this flag to True is mostly a convenient workaround for the
-            fact that massive serial appends will take too much time, and in
-            most of the cases the MUA will only switch to the mailbox where the
-            appends have happened after a certain time, which in most of the
-            times will be enough to have all the queued insert operations
-            finished.
-        :type notify_just_mdoc: bool
-        :param pending_inserts_dict:
-            a dictionary with the pending inserts ids.
-        :type pending_inserts_dict: dict
-
-        :return: a deferred whose callback will be called when either all the
-                 part documents have been written, or just the metamsg-doc,
-                 depending on the value of the notify_just_mdoc flag
+        :return: a deferred whose callback will be called when all the
+                 part documents have been written.
         :rtype: defer.Deferred
         """
-        if pending_inserts_dict is None:
-            pending_inserts_dict = {}
-
         assert self.cdocs, "Need cdocs to create the MessageWrapper docs"
         assert self.mdoc.doc_id is None, "Cannot create: mdoc has a doc_id"
         assert self.fdoc.doc_id is None, "Cannot create: fdoc has a doc_id"
 
-        def maybe_unblock_pending():
-            if pending_inserts_dict:
-                ci_headers = lowerdict(self.hdoc.headers)
-                msgid = ci_headers.get('message-id', None)
-                try:
-                    d = pending_inserts_dict[msgid]
-                    d.callback(msgid)
-                except KeyError:
-                    pass
-
         copy = self._is_copy
 
-        try:
-            mdoc = yield self.mdoc.create(store, is_copy=copy)
-            print "GOT MDOC >>>>>>>>>>>>>>>>>>", mdoc, "copy?", copy
-            assert mdoc
-            self.mdoc = mdoc
-        except Exception:
-            self.log.failure('Error creating mdoc')
-
-        if notify_just_mdoc:
-            # fire and forget, fast notifies
-            self.fdoc.create(store, is_copy=copy)
-            if not copy:
-                if self.hdoc.doc_id is None:
-                    self.hdoc.create(store)
-                for cdoc in self.cdocs.values():
-                    if cdoc.doc_id is not None:
-                        continue
-                    cdoc.create(store)
-
-        else:
-            yield self.fdoc.create(store, is_copy=copy)
-            if not copy:
-                if self.hdoc.doc_id is None:
-                    yield self.hdoc.create(store)
-                for cdoc in self.cdocs.values():
-                    if cdoc.doc_id is not None:
-                        # we could be just linking to an existing
-                        # content-doc.
-                        continue
-                    yield cdoc.create(store)
-
-        maybe_unblock_pending()
+        mdoc = yield self.mdoc.create(store, is_copy=copy)
+        assert mdoc
+        self.mdoc = mdoc
+        fdoc = yield self.fdoc.create(store, is_copy=copy)
+        self.fdoc = fdoc
+        if not copy:
+            if self.hdoc.doc_id is None:
+                hdoc = yield self.hdoc.create(store)
+                self.hdoc = hdoc
+            for cdoc in self.cdocs.values():
+                if cdoc.doc_id is not None:
+                    # we could be just linking to an existing
+                    # content-doc.
+                    continue
+                yield cdoc.create(store)
         defer.returnValue(self)
 
-
-
     def update(self, store):
         """
         Update the only mutable parts, which are within the flags document.
diff --git a/src/leap/bitmask/mail/imap/account.py b/src/leap/bitmask/mail/imap/account.py
index 9b26dbaa..dab8812d 100644
--- a/src/leap/bitmask/mail/imap/account.py
+++ b/src/leap/bitmask/mail/imap/account.py
@@ -156,9 +156,7 @@ class IMAPAccount(object):
         """
         name = normalize_mailbox(name)
 
-        # FIXME --- return failure instead of AssertionError
-        # See AccountTestCase...
-        leap_assert(name, "Need a mailbox name to create a mailbox")
+        assert name, "Need a mailbox name to create a mailbox"
 
         def check_it_does_not_exist(mailboxes):
             if name in mailboxes:
diff --git a/src/leap/bitmask/mail/imap/mailbox.py b/src/leap/bitmask/mail/imap/mailbox.py
index 0820a040..20b18761 100644
--- a/src/leap/bitmask/mail/imap/mailbox.py
+++ b/src/leap/bitmask/mail/imap/mailbox.py
@@ -317,7 +317,7 @@ class IMAPMailbox(object):
         d.addCallback(as_a_dict)
         return d
 
-    def addMessage(self, message, flags, date=None, notify_just_mdoc=True):
+    def addMessage(self, message, flags, date=None):
         """
         Adds a message to this mailbox.
 
@@ -330,24 +330,6 @@ class IMAPMailbox(object):
         :param date: timestamp
         :type date: str, or None
 
-        :param notify_just_mdoc:
-            boolean passed to the wrapper.create method, to indicate whether
-            we're insterested in being notified right after the mdoc has been
-            written (as it's the first doc to be written, and quite small, this
-            is faster, though potentially unsafe).
-            Setting it to True improves a *lot* the responsiveness of the
-            APPENDS: we just need to be notified when the mdoc is saved, and
-            let's just expect 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.
-            However, some operations like the saving of drafts need to wait for
-            all the parts to be saved, so if some heuristics are met down in
-            the call chain a Draft message will unconditionally set this flag
-            to False, and therefore ignoring the setting of this flag here.
-        :type notify_just_mdoc: bool
-
         :return: a deferred that will be triggered with the UID of the added
                  message.
         """
@@ -355,14 +337,7 @@ class IMAPMailbox(object):
         # TODO have a look at the cases for internal date in the rfc
         # XXX we could treat the message as an IMessage from here
 
-        # TODO change notify_just_mdoc to something more meaningful, like
-        # fast_insert_notify?
-
-        # TODO  notify_just_mdoc *sometimes* make the append tests fail.
-        # have to find a better solution for this. A workaround could probably
-        # be to have a list of the ongoing deferreds related to append, so that
-        # we queue for later all the requests having to do with these.
-
+        # TODO -- fast appends should be definitely solved by Blobs.
         # A better solution will probably involve implementing MULTIAPPEND
         # extension or patching imap server to support pipelining.
 
@@ -380,8 +355,7 @@ class IMAPMailbox(object):
         if date is None:
             date = formatdate(time.time())
 
-        d = self.collection.add_msg(message, flags, date=date,
-                                    notify_just_mdoc=notify_just_mdoc)
+        d = self.collection.add_msg(message, flags, date=date)
         d.addCallback(lambda message: message.get_uid())
         d.addErrback(
             lambda failure: self.log.failure('Error while adding msg'))
diff --git a/src/leap/bitmask/mail/incoming/service.py b/src/leap/bitmask/mail/incoming/service.py
index f1f4ab22..6a5d7fe3 100644
--- a/src/leap/bitmask/mail/incoming/service.py
+++ b/src/leap/bitmask/mail/incoming/service.py
@@ -847,8 +847,7 @@ class IncomingMail(Service):
 
         # Cannot do fast notifies, otherwise fucks with pixelated.
         d = self._inbox_collection.add_msg(
-            raw_data, (self.RECENT_FLAG,), date=insertion_date,
-            notify_just_mdoc=False)
+            raw_data, (self.RECENT_FLAG,), date=insertion_date)
         d.addCallbacks(msgSavedCallback, self._errback)
         return d
 
diff --git a/src/leap/bitmask/mail/interfaces.py b/src/leap/bitmask/mail/interfaces.py
index 10f51237..9ae61690 100644
--- a/src/leap/bitmask/mail/interfaces.py
+++ b/src/leap/bitmask/mail/interfaces.py
@@ -36,7 +36,7 @@ class IMessageWrapper(Interface):
                      '(immutable)')
     cdocs = Attribute('A dictionary with the content-docs, one-indexed')
 
-    def create(self, store, notify_just_mdoc=False, pending_inserts_dict={}):
+    def create(self, store):
         """
         Create the underlying wrapper.
         """
diff --git a/src/leap/bitmask/mail/mail.py b/src/leap/bitmask/mail/mail.py
index 3dbb0a1f..9a7b1aed 100644
--- a/src/leap/bitmask/mail/mail.py
+++ b/src/leap/bitmask/mail/mail.py
@@ -41,7 +41,6 @@ from leap.bitmask.mail.constants import MessageFlags
 from leap.bitmask.mail.mailbox_indexer import MailboxIndexer
 from leap.bitmask.mail.plugins import soledad_sync_hooks
 from leap.bitmask.mail.utils import find_charset, CaseInsensitiveDict
-from leap.bitmask.mail.utils import lowerdict
 
 log = Logger()
 
@@ -89,6 +88,7 @@ def _encode_payload(payload, ctype=""):
     # soledad when it's creating the documents.
     # if not charset:
     # charset = get_email_charset(payload)
+    # TODO there's also some charset detection in the pixelated adapters.
     # -----------------------------------------------------
 
     if not charset:
@@ -368,8 +368,6 @@ class MessageCollection(object):
     store = None
     messageklass = Message
 
-    _pending_inserts = dict()
-
     def __init__(self, adaptor, store, mbox_indexer=None, mbox_wrapper=None):
         """
         Constructor for a MessageCollection.
@@ -472,16 +470,7 @@ class MessageCollection(object):
                 self.messageklass, self.store,
                 doc_id, uid=uid, get_cdocs=get_cdocs)
 
-        def cleanup_and_get_doc_after_pending_insert(result):
-            for key in result:
-                self._pending_inserts.pop(key, None)
-            return get_doc_fun(self.mbox_uuid, uid)
-
-        if not self._pending_inserts:
-            d = get_doc_fun(self.mbox_uuid, uid)
-        else:
-            d = defer.gatherResults(self._pending_inserts.values())
-            d.addCallback(cleanup_and_get_doc_after_pending_insert)
+        d = get_doc_fun(self.mbox_uuid, uid)
         d.addCallback(get_msg_from_mdoc_id)
         return d
 
@@ -579,8 +568,7 @@ class MessageCollection(object):
     # Manipulate messages
 
     @defer.inlineCallbacks
-    def add_msg(self, raw_msg, flags=tuple(), tags=tuple(), date="",
-                notify_just_mdoc=False):
+    def add_msg(self, raw_msg, flags=tuple(), tags=tuple(), date=""):
         """
         Add a message to this collection.
 
@@ -593,16 +581,6 @@ class MessageCollection(object):
             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 insterested in being notified right after the mdoc has been
-            written (as it's the first doc to be written, and quite small, this
-            is faster, though potentially unsafe), or on the contrary we want
-            to wait untill all the parts have been written.
-            Used by the imap mailbox implementation to get faster responses.
-            This will be ignored (and set to False) if a heuristic for a Draft
-            message is met, which currently is a specific mozilla header.
-        :type notify_just_mdoc: bool
 
         :returns: a deferred that will fire with a Message when this is
                   inserted.
@@ -610,24 +588,14 @@ class MessageCollection(object):
         """
         # 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.
+        # XXX mdoc ref is a leaky abstraction here. generalize, that SHOULD be
+        # moved inside soledad adaptor.
         leap_assert_type(flags, tuple)
         leap_assert_type(date, str)
 
         msg = self.adaptor.get_msg_from_string(Message, raw_msg)
         wrapper = msg.get_wrapper()
 
-        headers = lowerdict(msg.get_headers())
-        moz_draft_hdr = "X-Mozilla-Draft-Info"
-        if moz_draft_hdr.lower() in headers:
-            self.log.debug('Setting fast notify to False, Draft detected')
-            notify_just_mdoc = False
-
-        if notify_just_mdoc:
-            msgid = headers.get('message-id')
-            if msgid:
-                self._pending_inserts[msgid] = defer.Deferred()
-
         if not self.is_mailbox_collection():
             raise NotImplementedError()
 
@@ -639,11 +607,7 @@ class MessageCollection(object):
         wrapper.set_date(date)
 
         try:
-            updated_wrapper = yield wrapper.create(
-                self.store,
-                notify_just_mdoc=notify_just_mdoc,
-                pending_inserts_dict=self._pending_inserts)
-
+            updated_wrapper = yield wrapper.create(self.store)
             doc_id = updated_wrapper.mdoc.doc_id
             if not doc_id:
                 doc_id = updated_wrapper.mdoc.future_doc_id
diff --git a/src/leap/bitmask/mua/pixelizer.py b/src/leap/bitmask/mua/pixelizer.py
index 138774b0..98333bd2 100644
--- a/src/leap/bitmask/mua/pixelizer.py
+++ b/src/leap/bitmask/mua/pixelizer.py
@@ -78,8 +78,7 @@ try:
             flags = ['\\Recent']
             if mailbox_name.lower() == 'sent':
                 flags += '\\Seen'
-            message = yield mailbox.add_msg(
-                raw_msg, tuple(flags), notify_just_mdoc=False)
+            message = yield mailbox.add_msg(raw_msg, tuple(flags))
 
             # this still needs the pixelated interface because it does stuff
             # like indexing the mail in whoosh, etc.