From cc487c6bc77769079fc4cdc23b6ad2bdd7c22394 Mon Sep 17 00:00:00 2001 From: Kali Kaneko Date: Thu, 25 Jun 2015 10:45:46 -0400 Subject: [bug] avoid duplication of copies in draft folder Although this draft-saving feature seems to be somehow brittle, since it breaks from time to time, the causes are different and subtle on each case. This time the bug is related to having the notify_just_mdoc (or fast_notifies as I'm tempted to call it in the future for more clarity) set to True by default in the APPENDS. Fast notifies break the "save draft" functionality because what Thunderbird does is an Append of the newest message, followed by deletion of the old message and a SEARCH by Message-ID. For this we need the headers-doc to be already inserted in the store. Since the fast-notify makes a *big* difference in terms of insertion times for serialized appends, I've opted for using a heuristic for detecting if it's the case of a Draft message, using a mozilla-specific header. If this is found, we set the notify_just_mdoc unconditionally to False. We'll deal with other MUAs on its due time. Releases: 0.4.0 --- src/leap/mail/adaptors/soledad.py | 4 ++-- src/leap/mail/imap/mailbox.py | 41 +++++++++++++++++++++++++-------------- src/leap/mail/mail.py | 28 ++++++++++++++------------ 3 files changed, 44 insertions(+), 29 deletions(-) diff --git a/src/leap/mail/adaptors/soledad.py b/src/leap/mail/adaptors/soledad.py index 7e41f94..0565877 100644 --- a/src/leap/mail/adaptors/soledad.py +++ b/src/leap/mail/adaptors/soledad.py @@ -547,8 +547,7 @@ class MessageWrapper(object): "Cannot create: fdoc has a doc_id") def unblock_pending_insert(result): - h = self.hdoc.headers - ci_headers = dict([(k.lower(), v) for (k, v) in h.items()]) + ci_headers = lowerdict(self.hdoc.headers) msgid = ci_headers.get('message-id', None) try: d = pending_inserts_dict[msgid] @@ -1101,6 +1100,7 @@ class SoledadMailAdaptor(SoledadIndexMixin): def get_mdoc_id(hdoc): if not hdoc: + log.msg("Could not find a HDOC with MSGID %s" % msgid) return None hdoc = hdoc[0] mdoc_id = hdoc.doc_id.replace("H-", "M-%s-" % uuid) diff --git a/src/leap/mail/imap/mailbox.py b/src/leap/mail/imap/mailbox.py index 72f5a43..139ae66 100644 --- a/src/leap/mail/imap/mailbox.py +++ b/src/leap/mail/imap/mailbox.py @@ -320,6 +320,24 @@ 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. """ @@ -327,18 +345,14 @@ 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. - # 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. @@ -355,9 +369,11 @@ class IMAPMailbox(object): if date is None: date = formatdate(time.time()) - # TODO add notify_new as a callback here... - return self.collection.add_msg(message, flags, date=date, - notify_just_mdoc=notify_just_mdoc) + d = self.collection.add_msg(message, flags, date=date, + notify_just_mdoc=notify_just_mdoc) + d.addCallback(self.notify_new) + d.addErrback(lambda failure: log.err(failure)) + return d def notify_new(self, *args): """ @@ -504,13 +520,8 @@ class IMAPMailbox(object): getimapmsg = self.get_imap_message def get_imap_messages_for_range(msg_range): - print - print - print - print "GETTING FOR RANGE", msg_range def _get_imap_msg(messages): - print "GETTING IMAP MSG FOR", messages d_imapmsg = [] for msg in messages: d_imapmsg.append(getimapmsg(msg)) diff --git a/src/leap/mail/mail.py b/src/leap/mail/mail.py index b4602b3..faaabf6 100644 --- a/src/leap/mail/mail.py +++ b/src/leap/mail/mail.py @@ -37,6 +37,7 @@ from leap.mail.constants import MessageFlags from leap.mail.mailbox_indexer import MailboxIndexer from leap.mail.plugins import soledad_sync_hooks from leap.mail.utils import find_charset, CaseInsensitiveDict +from leap.mail.utils import lowerdict logger = logging.getLogger(name=__name__) @@ -570,11 +571,14 @@ class MessageCollection(object): 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 - the mdoc has been written (faster, but potentially unsafe), or we - want to wait untill all the parts have been written. + 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 the UID of the inserted @@ -590,8 +594,14 @@ class MessageCollection(object): 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: + log.msg("Setting fast notify to False, Draft detected") + notify_just_mdoc = False + if notify_just_mdoc: - msgid = msg.get_headers()['message-id'] + msgid = headers['message-id'] self._pending_inserts[msgid] = defer.Deferred() if not self.is_mailbox_collection(): @@ -622,12 +632,6 @@ class MessageCollection(object): d = self.mbox_indexer.create_table(self.mbox_uuid) d.addBoth(lambda _: self.mbox_indexer.insert_doc( self.mbox_uuid, doc_id)) - # XXX--------------------------------- - def print_inserted(r): - print "INSERTED", r - return r - d.addCallback(print_inserted) - # XXX--------------------------------- return d d = wrapper.create( @@ -636,7 +640,7 @@ class MessageCollection(object): pending_inserts_dict=self._pending_inserts) d.addCallback(insert_mdoc_id, wrapper) d.addErrback(lambda failure: log.err(failure)) - #d.addCallback(self.cb_signal_unread_to_ui) + d.addCallback(self.cb_signal_unread_to_ui) return d -- cgit v1.2.3