[bug] fix early append notification

There's a workaround for "slow" APPENDS to an inbox, and it is that we
have a flag to allow returning early when JUST the mdoc (the
meta-document) has been written. However, this was givin a problem when
doing a FETCH right after an APPEND (with notify_just_mdoc=True) has
been done.

This commit fixes it by making the FETCH command first check if there's
an ongoing pending write, and queueing itself right after the write
queue has been completed.

This fixes the testFullAppend regression.

Releases: 0.4.0

6 files changed, 83 insertions, 43 deletions

diff --git a/mail/src/leap/mail/adaptors/soledad.py b/mail/src/leap/mail/adaptors/soledad.py
index 7a1a92db..b8e5fd49 100644
--- a/mail/src/leap/mail/adaptors/soledad.py
+++ b/mail/src/leap/mail/adaptors/soledad.py
@@ -491,7 +491,7 @@ class MessageWrapper(object):
         for doc_id, cdoc in zip(self.mdoc.cdocs, self.cdocs.values()):
             cdoc.set_future_doc_id(doc_id)
 
-    def create(self, store, notify_just_mdoc=False):
+    def create(self, store, notify_just_mdoc=False, pending_inserts_dict=None):
         """
         Create all the parts for this message in the store.
 
@@ -503,7 +503,7 @@ class MessageWrapper(object):
             Be warned that in that case there will be no record of failures
             when creating the other part-documents.
 
-            Other-wise, this method will return a deferred that will wait for
+            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
@@ -513,6 +513,9 @@ class MessageWrapper(object):
             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,
@@ -527,26 +530,41 @@ class MessageWrapper(object):
         leap_assert(self.fdoc.doc_id is None,
                     "Cannot create: fdoc has a doc_id")
 
+        def unblock_pending_insert(result):
+            msgid = self.hdoc.headers.get('Message-Id', None)
+            try:
+                d = pending_inserts_dict[msgid]
+                d.callback(msgid)
+            except KeyError:
+                pass
+            return result
+
         # TODO check that the doc_ids in the mdoc are coherent
-        d = []
+        self.d = []
+
         mdoc_created = self.mdoc.create(store)
-        d.append(mdoc_created)
-        d.append(self.fdoc.create(store))
+        fdoc_created = self.fdoc.create(store)
+
+        self.d.append(mdoc_created)
+        self.d.append(fdoc_created)
 
         if not self._is_copy:
             if self.hdoc.doc_id is None:
-                d.append(self.hdoc.create(store))
+                self.d.append(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
-                d.append(cdoc.create(store))
+                self.d.append(cdoc.create(store))
+
+        self.all_inserted_d = defer.gatherResults(self.d)
 
         if notify_just_mdoc:
+            self.all_inserted_d.addCallback(unblock_pending_insert)
             return mdoc_created
         else:
-            return defer.gatherResults(d)
+            return self.all_inserted_d
 
     def update(self, store):
         """
diff --git a/mail/src/leap/mail/imap/mailbox.py b/mail/src/leap/mail/imap/mailbox.py
index 0eff317d..14123441 100644
--- a/mail/src/leap/mail/imap/mailbox.py
+++ b/mail/src/leap/mail/imap/mailbox.py
@@ -492,13 +492,8 @@ class IMAPMailbox(object):
 
         :rtype: deferred with a generator that yields...
         """
-        # For the moment our UID is sequential, so we
-        # can treat them all the same.
-        # Change this to the flag that twisted expects when we
-        # switch to content-hash based index + local UID table.
-
-        is_sequence = True if uid == 0 else False
-
+        # TODO implement sequence
+        # is_sequence = True if uid == 0 else False
         # XXX DEBUG ---  if you attempt to use the `getmail` utility under
         # imap/tests, or muas like mutt, it will choke until we implement
         # sequence numbers. This is an easy hack meanwhile.
@@ -583,7 +578,6 @@ class IMAPMailbox(object):
         :rtype: tuple
         """
         # is_sequence = True if uid == 0 else False
-
         # XXX FIXME -----------------------------------------------------
         # imap/tests, or muas like mutt, it will choke until we implement
         # sequence numbers. This is an easy hack meanwhile.
@@ -672,7 +666,6 @@ class IMAPMailbox(object):
         :rtype: tuple
         """
         # TODO implement sequences
-        # TODO how often is thunderbird doing this?
         is_sequence = True if uid == 0 else False
         if is_sequence:
             raise NotImplementedError
@@ -730,7 +723,6 @@ class IMAPMailbox(object):
                                 read-write.
         """
         # TODO implement sequences
-        # TODO how often is thunderbird doing this?
         is_sequence = True if uid == 0 else False
         if is_sequence:
             raise NotImplementedError
diff --git a/mail/src/leap/mail/imap/messages.py b/mail/src/leap/mail/imap/messages.py
index 13943b10..4c6f10d1 100644
--- a/mail/src/leap/mail/imap/messages.py
+++ b/mail/src/leap/mail/imap/messages.py
@@ -22,7 +22,7 @@ from twisted.mail import imap4
 from twisted.internet import defer
 from zope.interface import implements
 
-from leap.mail.utils import find_charset
+from leap.mail.utils import find_charset, CaseInsensitiveDict
 
 
 logger = logging.getLogger(__name__)
@@ -208,18 +208,6 @@ class IMAPMessagePart(object):
         return IMAPMessagePart(subpart)
 
 
-class CaseInsensitiveDict(dict):
-    """
-    A dictionary subclass that will allow case-insenstive key lookups.
-    """
-
-    def __setitem__(self, key, value):
-        super(CaseInsensitiveDict, self).__setitem__(key.lower(), value)
-
-    def __getitem__(self, key):
-        return super(CaseInsensitiveDict, self).__getitem__(key.lower())
-
-
 def _format_headers(headers, negate, *names):
     # current server impl. expects content-type to be present, so if for
     # some reason we do not have headers, we have to return at least that
diff --git a/mail/src/leap/mail/imap/tests/test_imap.py b/mail/src/leap/mail/imap/tests/test_imap.py
index c4f752bc..af1bd691 100644
--- a/mail/src/leap/mail/imap/tests/test_imap.py
+++ b/mail/src/leap/mail/imap/tests/test_imap.py
@@ -25,8 +25,8 @@ XXX add authors from the original twisted tests.
 @license: GPLv3, see included LICENSE file
 """
 # XXX review license of the original tests!!!
-
 import os
+import string
 import types
 
 
@@ -38,6 +38,7 @@ from twisted.python import failure
 from twisted import cred
 
 from leap.mail.imap.mailbox import IMAPMailbox
+from leap.mail.imap.messages import CaseInsensitiveDict
 from leap.mail.imap.tests.utils import IMAP4HelperMixin
 
 
@@ -74,8 +75,8 @@ class TestRealm:
 #
 
 # DEBUG ---
-#from twisted.internet.base import DelayedCall
-#DelayedCall.debug = True
+# from twisted.internet.base import DelayedCall
+# DelayedCall.debug = True
 
 
 class LEAPIMAP4ServerTestCase(IMAP4HelperMixin):
@@ -810,7 +811,7 @@ class LEAPIMAP4ServerTestCase(IMAP4HelperMixin):
         infile = util.sibpath(__file__, 'rfc822.message')
         message = open(infile)
         acc = self.server.theAccount
-        mailbox_name = "root/subthing"
+        mailbox_name = "appendmbox/subthing"
 
         def add_mailbox():
             return acc.addMailbox(mailbox_name)
@@ -843,7 +844,7 @@ class LEAPIMAP4ServerTestCase(IMAP4HelperMixin):
         uid, msg = fetched[0]
         parsed = self.parser.parse(open(infile))
         expected_body = parsed.get_payload()
-        expected_headers = dict(parsed.items())
+        expected_headers = CaseInsensitiveDict(parsed.items())
 
         def assert_flags(flags):
             self.assertEqual(
@@ -860,7 +861,7 @@ class LEAPIMAP4ServerTestCase(IMAP4HelperMixin):
             self.assertEqual(expected_body, gotbody)
 
         def assert_headers(headers):
-            self.assertItemsEqual(expected_headers, headers)
+            self.assertItemsEqual(map(string.lower, expected_headers), headers)
 
         d = defer.maybeDeferred(msg.getFlags)
         d.addCallback(assert_flags)
diff --git a/mail/src/leap/mail/mail.py b/mail/src/leap/mail/mail.py
index 89f89b04..4fe08a69 100644
--- a/mail/src/leap/mail/mail.py
+++ b/mail/src/leap/mail/mail.py
@@ -35,7 +35,7 @@ from leap.mail.adaptors.soledad import SoledadMailAdaptor
 from leap.mail.constants import INBOX_NAME
 from leap.mail.constants import MessageFlags
 from leap.mail.mailbox_indexer import MailboxIndexer
-from leap.mail.utils import find_charset
+from leap.mail.utils import find_charset, CaseInsensitiveDict
 
 logger = logging.getLogger(name=__name__)
 
@@ -179,7 +179,7 @@ class MessagePart(object):
         return _write_and_rewind(payload)
 
     def get_headers(self):
-        return self._pmap.get("headers", [])
+        return CaseInsensitiveDict(self._pmap.get("headers", []))
 
     def is_multipart(self):
         return self._pmap.get("multi", False)
@@ -261,7 +261,7 @@ class Message(object):
         """
         Get the raw headers document.
         """
-        return self._wrapper.hdoc.headers
+        return CaseInsensitiveDict(self._wrapper.hdoc.headers)
 
     def get_body_file(self, store):
         """
@@ -364,6 +364,8 @@ 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.
@@ -440,6 +442,8 @@ class MessageCollection(object):
         if not absolute:
             raise NotImplementedError("Does not support relative ids yet")
 
+        get_doc_fun = self.mbox_indexer.get_doc_id_from_uid
+
         def get_msg_from_mdoc_id(doc_id):
             if doc_id is None:
                 return None
@@ -447,7 +451,16 @@ class MessageCollection(object):
                 self.messageklass, self.store,
                 doc_id, uid=uid, get_cdocs=get_cdocs)
 
-        d = self.mbox_indexer.get_doc_id_from_uid(self.mbox_uuid, uid)
+        def cleanup_and_get_doc_after_pending_insert(result):
+            for key in result:
+                self._pending_inserts.pop(key)
+            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.addCallback(get_msg_from_mdoc_id)
         return d
 
@@ -572,13 +585,16 @@ 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.
-
         leap_assert_type(flags, tuple)
         leap_assert_type(date, str)
 
         msg = self.adaptor.get_msg_from_string(Message, raw_msg)
         wrapper = msg.get_wrapper()
 
+        if notify_just_mdoc:
+            msgid = msg.get_headers()['message-id']
+            self._pending_inserts[msgid] = defer.Deferred()
+
         if not self.is_mailbox_collection():
             raise NotImplementedError()
 
@@ -600,10 +616,14 @@ class MessageCollection(object):
                              (wrapper.mdoc.serialize(),))
                 return defer.succeed("mdoc_id not inserted")
                 # XXX BUG -----------------------------------------
+
             return self.mbox_indexer.insert_doc(
                 self.mbox_uuid, doc_id)
 
-        d = wrapper.create(self.store, notify_just_mdoc=notify_just_mdoc)
+        d = wrapper.create(
+            self.store,
+            notify_just_mdoc=notify_just_mdoc,
+            pending_inserts_dict=self._pending_inserts)
         d.addCallback(insert_mdoc_id, wrapper)
         d.addErrback(lambda f: f.printTraceback())
         d.addCallback(self.cb_signal_unread_to_ui)
diff --git a/mail/src/leap/mail/utils.py b/mail/src/leap/mail/utils.py
index 8e510247..029e9f57 100644
--- a/mail/src/leap/mail/utils.py
+++ b/mail/src/leap/mail/utils.py
@@ -351,3 +351,24 @@ def json_loads(data):
         obj = json.loads(data, cls=json.JSONDecoder)
 
     return obj
+
+
+class CaseInsensitiveDict(dict):
+    """
+    A dictionary subclass that will allow case-insenstive key lookups.
+    """
+    def __init__(self, d=None):
+        if d is None:
+            d = []
+        if isinstance(d, dict):
+            for key, value in d.items():
+                self[key] = value
+        else:
+            for key, value in d:
+                self[key] = value
+
+    def __setitem__(self, key, value):
+        super(CaseInsensitiveDict, self).__setitem__(key.lower(), value)
+
+    def __getitem__(self, key):
+        return super(CaseInsensitiveDict, self).__getitem__(key.lower())