From 8e916eeadfcd76d50b54a2621d789e6a296dcce6 Mon Sep 17 00:00:00 2001 From: Kali Kaneko Date: Thu, 26 Mar 2015 15:59:33 -0400 Subject: [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 --- src/leap/mail/adaptors/soledad.py | 34 ++++++++++++++++++++++++++-------- src/leap/mail/imap/mailbox.py | 12 ++---------- src/leap/mail/imap/messages.py | 14 +------------- src/leap/mail/imap/tests/test_imap.py | 13 +++++++------ src/leap/mail/mail.py | 32 ++++++++++++++++++++++++++------ src/leap/mail/utils.py | 21 +++++++++++++++++++++ 6 files changed, 83 insertions(+), 43 deletions(-) diff --git a/src/leap/mail/adaptors/soledad.py b/src/leap/mail/adaptors/soledad.py index 7a1a92d..b8e5fd4 100644 --- a/src/leap/mail/adaptors/soledad.py +++ b/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/src/leap/mail/imap/mailbox.py b/src/leap/mail/imap/mailbox.py index 0eff317..1412344 100644 --- a/src/leap/mail/imap/mailbox.py +++ b/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/src/leap/mail/imap/messages.py b/src/leap/mail/imap/messages.py index 13943b1..4c6f10d 100644 --- a/src/leap/mail/imap/messages.py +++ b/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/src/leap/mail/imap/tests/test_imap.py b/src/leap/mail/imap/tests/test_imap.py index c4f752b..af1bd69 100644 --- a/src/leap/mail/imap/tests/test_imap.py +++ b/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/src/leap/mail/mail.py b/src/leap/mail/mail.py index 89f89b0..4fe08a6 100644 --- a/src/leap/mail/mail.py +++ b/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/src/leap/mail/utils.py b/src/leap/mail/utils.py index 8e51024..029e9f5 100644 --- a/src/leap/mail/utils.py +++ b/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()) -- cgit v1.2.3