From a7e6bfb1f7befb16926353519787155178194140 Mon Sep 17 00:00:00 2001 From: Kali Kaneko Date: Thu, 16 Jan 2014 17:13:05 -0400 Subject: Dispatch the flags query if it's the only one. ie, we got something like FETCH 1:* (FLAGS) but not for FETCH 1:* (FLAGS INTERNALDATE) --- mail/src/leap/mail/imap/service/imap.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/mail/src/leap/mail/imap/service/imap.py b/mail/src/leap/mail/imap/service/imap.py index e877869..8c5b488 100644 --- a/mail/src/leap/mail/imap/service/imap.py +++ b/mail/src/leap/mail/imap/service/imap.py @@ -124,7 +124,9 @@ class LeapIMAPServer(imap4.IMAP4Server): cbFetch = self._IMAP4Server__cbFetch ebFetch = self._IMAP4Server__ebFetch - if str(query[0]) == "flags": + print "QUERY: ", query + + if len(query) == 1 and str(query[0]) == "flags": self._oldTimeout = self.setTimeout(None) # no need to call iter, we get a generator maybeDeferred( -- cgit v1.2.3 From b8f726fd579a826c0ba6cfc454ff9dc9dc6d95ef Mon Sep 17 00:00:00 2001 From: Kali Kaneko Date: Thu, 16 Jan 2014 17:15:27 -0400 Subject: patch UIDVALIDITY response for conformance to the spec testimap was choking on this. --- mail/src/leap/mail/imap/service/imap.py | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/mail/src/leap/mail/imap/service/imap.py b/mail/src/leap/mail/imap/service/imap.py index 8c5b488..6e03456 100644 --- a/mail/src/leap/mail/imap/service/imap.py +++ b/mail/src/leap/mail/imap/service/imap.py @@ -146,6 +146,36 @@ class LeapIMAPServer(imap4.IMAP4Server): select_FETCH = (do_FETCH, imap4.IMAP4Server.arg_seqset, imap4.IMAP4Server.arg_fetchatt) + def _cbSelectWork(self, mbox, cmdName, tag): + """ + Callback for selectWork, patched to avoid conformance errors due to + incomplete UIDVALIDITY line. + """ + if mbox is None: + self.sendNegativeResponse(tag, 'No such mailbox') + return + if '\\noselect' in [s.lower() for s in mbox.getFlags()]: + self.sendNegativeResponse(tag, 'Mailbox cannot be selected') + return + + flags = mbox.getFlags() + self.sendUntaggedResponse(str(mbox.getMessageCount()) + ' EXISTS') + self.sendUntaggedResponse(str(mbox.getRecentCount()) + ' RECENT') + self.sendUntaggedResponse('FLAGS (%s)' % ' '.join(flags)) + + # Patched ------------------------------------------------------- + # imaptest was complaining about the incomplete line, we're adding + # "UIDs valid" here. + self.sendPositiveResponse( + None, '[UIDVALIDITY %d] UIDs valid' % mbox.getUIDValidity()) + # ---------------------------------------------------------------- + + s = mbox.isWriteable() and 'READ-WRITE' or 'READ-ONLY' + mbox.addListener(self) + self.sendPositiveResponse(tag, '[%s] %s successful' % (s, cmdName)) + self.state = 'select' + self.mbox = mbox + class IMAPAuthRealm(object): """ -- cgit v1.2.3 From cfe173321dfcf55da571d0e42f93680e4452b5c1 Mon Sep 17 00:00:00 2001 From: Kali Kaneko Date: Thu, 16 Jan 2014 17:18:11 -0400 Subject: reset last uid on expunge --- mail/src/leap/mail/imap/mailbox.py | 1 + mail/src/leap/mail/imap/messages.py | 12 ++++++++++++ 2 files changed, 13 insertions(+) diff --git a/mail/src/leap/mail/imap/mailbox.py b/mail/src/leap/mail/imap/mailbox.py index 94070ac..86dac77 100644 --- a/mail/src/leap/mail/imap/mailbox.py +++ b/mail/src/leap/mail/imap/mailbox.py @@ -463,6 +463,7 @@ class SoledadMailbox(WithMsgFields, MBoxParser): if not self.isWriteable(): raise imap4.ReadOnlyMailbox d = self.messages.remove_all_deleted() + d.addCallback(self.messages.reset_last_uid) d.addCallback(self._expunge_cb) return d diff --git a/mail/src/leap/mail/imap/messages.py b/mail/src/leap/mail/imap/messages.py index 22de356..02df38e 100644 --- a/mail/src/leap/mail/imap/messages.py +++ b/mail/src/leap/mail/imap/messages.py @@ -1337,6 +1337,18 @@ class MessageCollection(WithMsgFields, IndexedDB, MailParser, MBoxParser, fields.TYPE_FLAGS_VAL, self.mbox)) return (u for u in sorted(all_uids)) + def reset_last_uid(self, param): + """ + Set the last uid to the highest uid found. + Used while expunging, passed as a callback. + """ + try: + self.last_uid = max(self.all_uid_iter()) + 1 + except ValueError: + # empty sequence + pass + return param + def all_flags(self): """ Return a dict with all flags documents for this mailbox. -- cgit v1.2.3 From b5f2c2596af6a7143718ca97e9ce15bcbaacfb9b Mon Sep 17 00:00:00 2001 From: Kali Kaneko Date: Thu, 16 Jan 2014 17:19:31 -0400 Subject: fix internaldate storage --- mail/src/leap/mail/imap/messages.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/mail/src/leap/mail/imap/messages.py b/mail/src/leap/mail/imap/messages.py index 02df38e..1b996b6 100644 --- a/mail/src/leap/mail/imap/messages.py +++ b/mail/src/leap/mail/imap/messages.py @@ -467,7 +467,8 @@ class LeapMessage(fields, MailParser, MBoxParser): :rtype: C{str} :return: An RFC822-formatted date string. """ - return str(self._hdoc.content.get(self.DATE_KEY, '')) + date = self._hdoc.content.get(self.DATE_KEY, '') + return str(date) # # IMessagePart @@ -1077,12 +1078,12 @@ class MessageCollection(WithMsgFields, IndexedDB, MailParser, MBoxParser, hd[self.MSGID_KEY] = msgid if not subject and self.SUBJECT_FIELD in headers: - hd[self.SUBJECT_KEY] = first(headers[self.SUBJECT_FIELD]) + hd[self.SUBJECT_KEY] = headers[self.SUBJECT_FIELD] else: hd[self.SUBJECT_KEY] = subject if not date and self.DATE_FIELD in headers: - hd[self.DATE_KEY] = first(headers[self.DATE_FIELD]) + hd[self.DATE_KEY] = headers[self.DATE_FIELD] else: hd[self.DATE_KEY] = date return hd -- cgit v1.2.3 From 6b7337bf23303c8cb04f7c3b13a8a753ea153567 Mon Sep 17 00:00:00 2001 From: Kali Kaneko Date: Thu, 16 Jan 2014 17:24:27 -0400 Subject: factor out bound and filter for msg seqs --- mail/src/leap/mail/imap/mailbox.py | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/mail/src/leap/mail/imap/mailbox.py b/mail/src/leap/mail/imap/mailbox.py index 86dac77..84eb528 100644 --- a/mail/src/leap/mail/imap/mailbox.py +++ b/mail/src/leap/mail/imap/mailbox.py @@ -467,6 +467,36 @@ class SoledadMailbox(WithMsgFields, MBoxParser): d.addCallback(self._expunge_cb) return d + def _bound_seq(self, messages_asked): + """ + Put an upper bound to a messages sequence if this is open. + + :param messages_asked: IDs of the messages. + :type messages_asked: MessageSet + :rtype: MessageSet + """ + if not messages_asked.last: + try: + iter(messages_asked) + except TypeError: + # looks like we cannot iterate + messages_asked.last = self.last_uid + return messages_asked + + def _filter_msg_seq(self, messages_asked): + """ + Filter a message sequence returning only the ones that do exist in the + collection. + + :param messages_asked: IDs of the messages. + :type messages_asked: MessageSet + :rtype: set + """ + set_asked = set(messages_asked) + set_exist = set(self.messages.all_uid_iter()) + seq_messg = set_asked.intersection(set_exist) + return seq_messg + @deferred def fetch(self, messages_asked, uid): """ -- cgit v1.2.3 From aa396a17b7d5aec0665cff7f547b49395341116c Mon Sep 17 00:00:00 2001 From: Kali Kaneko Date: Thu, 16 Jan 2014 17:26:01 -0400 Subject: Fix grave bug with iteration in STORE This was in the root for problems with Trash behavior. Closes: #4958 Make use of the refactored utilities for bounding and filtering sequences. --- mail/src/leap/mail/imap/mailbox.py | 37 ++++++++++++------------------------- 1 file changed, 12 insertions(+), 25 deletions(-) diff --git a/mail/src/leap/mail/imap/mailbox.py b/mail/src/leap/mail/imap/mailbox.py index 84eb528..137f9f5 100644 --- a/mail/src/leap/mail/imap/mailbox.py +++ b/mail/src/leap/mail/imap/mailbox.py @@ -526,17 +526,9 @@ class SoledadMailbox(WithMsgFields, MBoxParser): sequence = False #sequence = True if uid == 0 else False - if not messages_asked.last: - try: - iter(messages_asked) - except TypeError: - # looks like we cannot iterate - messages_asked.last = self.last_uid + messages_asked = self._bound_seq(messages_asked) + seq_messg = self._filter_msg_seq(messages_asked) - set_asked = set(messages_asked) - set_exist = set(self.messages.all_uid_iter()) - seq_messg = set_asked.intersection(set_exist) - getmsg = lambda msgid: self.messages.get_msg_by_uid(msgid) # for sequence numbers (uid = 0) if sequence: @@ -563,6 +555,7 @@ class SoledadMailbox(WithMsgFields, MBoxParser): A fast method to fetch all flags, tricking just the needed subset of the MIME interface that's needed to satisfy a generic FLAGS query. + Given how LEAP Mail is supposed to work without local cache, this query is going to be quite common, and also we expect it to be in the form 1:* at the beginning of a session, so @@ -592,16 +585,9 @@ class SoledadMailbox(WithMsgFields, MBoxParser): def getFlags(self): return map(str, self.flags) - if not messages_asked.last: - try: - iter(messages_asked) - except TypeError: - # looks like we cannot iterate - messages_asked.last = self.last_uid + messages_asked = self._bound_seq(messages_asked) + seq_messg = self._filter_msg_seq(messages_asked) - set_asked = set(messages_asked) - set_exist = set(self.messages.all_uid_iter()) - seq_messg = set_asked.intersection(set_exist) all_flags = self.messages.all_flags() result = ((msgid, flagsPart( msgid, all_flags[msgid])) for msgid in seq_messg) @@ -648,7 +634,7 @@ class SoledadMailbox(WithMsgFields, MBoxParser): leap_events.signal(IMAP_UNREAD_MAIL, str(unseen)) @deferred - def store(self, messages, flags, mode, uid): + def store(self, messages_asked, flags, mode, uid): """ Sets the flags of one or more messages. @@ -677,25 +663,26 @@ class SoledadMailbox(WithMsgFields, MBoxParser): :raise ReadOnlyMailbox: Raised if this mailbox is not open for read-write. """ + from twisted.internet import reactor # XXX implement also sequence (uid = 0) # XXX we should prevent cclient from setting Recent flag. leap_assert(not isinstance(flags, basestring), "flags cannot be a string") flags = tuple(flags) + messages_asked = self._bound_seq(messages_asked) + seq_messg = self._filter_msg_seq(messages_asked) + if not self.isWriteable(): log.msg('read only mailbox!') raise imap4.ReadOnlyMailbox - if not messages.last: - messages.last = self.messages.count() - result = {} - for msg_id in messages: + for msg_id in seq_messg: log.msg("MSG ID = %s" % msg_id) msg = self.messages.get_msg_by_uid(msg_id) if not msg: - return result + continue if mode == 1: msg.addFlags(flags) elif mode == -1: -- cgit v1.2.3 From d9de65c60b2c9515541dff8565e045db2538ec65 Mon Sep 17 00:00:00 2001 From: Kali Kaneko Date: Thu, 16 Jan 2014 17:33:20 -0400 Subject: Temporal refactor setting of recent flag. This flag is set way too often, and is damaging performance. Will move it to a single doc per mailbox in subsequente commits. --- mail/src/leap/mail/imap/mailbox.py | 37 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/mail/src/leap/mail/imap/mailbox.py b/mail/src/leap/mail/imap/mailbox.py index 137f9f5..cf09bc4 100644 --- a/mail/src/leap/mail/imap/mailbox.py +++ b/mail/src/leap/mail/imap/mailbox.py @@ -529,6 +529,10 @@ class SoledadMailbox(WithMsgFields, MBoxParser): messages_asked = self._bound_seq(messages_asked) seq_messg = self._filter_msg_seq(messages_asked) + def getmsg(msgid): + if self.isWriteable(): + deferLater(reactor, 2, self._unset_recent_flag, messages_asked) + return self.messages.get_msg_by_uid(msgid) # for sequence numbers (uid = 0) if sequence: @@ -538,12 +542,6 @@ class SoledadMailbox(WithMsgFields, MBoxParser): else: result = ((msgid, getmsg(msgid)) for msgid in seq_messg) - if self.isWriteable(): - deferLater(reactor, 30, self._unset_recent_flag) - # XXX I should rewrite the scheduler so it handles a - # set of queues with different priority. - self._unset_recent_flag() - # this should really be called as a final callback of # the do_FETCH method... deferLater(reactor, 1, self._signal_unread_to_ui) @@ -594,7 +592,7 @@ class SoledadMailbox(WithMsgFields, MBoxParser): return result @deferred - def _unset_recent_flag(self): + def _unset_recent_flag(self, message_uid): """ Unsets `Recent` flag from a tuple of messages. Called from fetch. @@ -610,19 +608,16 @@ class SoledadMailbox(WithMsgFields, MBoxParser): If it is not possible to determine whether or not this session is the first session to be notified about a message, then that message SHOULD be considered recent. - """ - # TODO this fucker, for the sake of correctness, is messing with - # the whole collection of flag docs. - # Possible ways of action: - # 1. Ignore it, we want fun. - # 2. Trigger it with a delay - # 3. Route it through a queue with lesser priority than the - # regularar writer. + :param message_uids: the sequence of msg ids to update. + :type message_uids: sequence + """ + # XXX deprecate this! + # move to a mailbox-level call, and do it in batches! - log.msg('unsetting recent flags...') - for msg in self.messages.get_recent(): - msg.removeFlags((fields.RECENT_FLAG,)) + log.msg('unsetting recent flag: %s' % message_uid) + msg = self.messages.get_msg_by_uid(message_uid) + msg.removeFlags((fields.RECENT_FLAG,)) self._signal_unread_to_ui() @deferred @@ -691,7 +686,9 @@ class SoledadMailbox(WithMsgFields, MBoxParser): msg.setFlags(flags) result[msg_id] = msg.getFlags() - self._signal_unread_to_ui() + # this should really be called as a final callback of + # the do_FETCH method... + deferLater(reactor, 1, self._signal_unread_to_ui) return result # ISearchableMailbox @@ -758,6 +755,8 @@ class SoledadMailbox(WithMsgFields, MBoxParser): new_fdoc[self.MBOX_KEY] = self.mbox d = self._do_add_doc(new_fdoc) + # XXX notify should be done when all the + # copies in the batch are finished. d.addCallback(self._notify_new) @deferred -- cgit v1.2.3