[PATCH 1/1] make maxage use UIDs to avoid timezone issues (edit: correction)

Janna Martl janna.martl109 at gmail.com
Fri Mar 20 20:23:01 GMT 2015


When using maxage, local and remote messagelists are supposed to only
contain messages from "maxage days ago". But local and remote used
different timezones to calculate what "maxage days ago" means, resulting
in loss of mail. Now, we ask the local folder for maxage days' worth of
mail, find the lowest UID, and then ask the remote folder for all UID's
starting with that lowest one.

Signed-off-by: Janna Martl <janna.martl109 at gmail.com>
---
 offlineimap/accounts.py            | 30 ++++++++++++++++++++++++------
 offlineimap/folder/Gmail.py        |  5 +++--
 offlineimap/folder/GmailMaildir.py |  4 ++--
 offlineimap/folder/IMAP.py         | 25 ++++++++++++++-----------
 offlineimap/folder/Maildir.py      | 21 +++++++++++++--------
 5 files changed, 56 insertions(+), 29 deletions(-)

diff --git a/offlineimap/accounts.py b/offlineimap/accounts.py
index 62ed5c3..e7d6c3c 100644
--- a/offlineimap/accounts.py
+++ b/offlineimap/accounts.py
@@ -429,6 +429,15 @@ def syncfolder(account, remotefolder, quick):
 
         statusfolder.cachemessagelist()
 
+        maxage = localfolder.config.getdefaultint("Account %s"%
+            localfolder.accountname, "maxage", None)
+
+        # Load local folder.
+        ui.syncingfolder(remoterepos, remotefolder, localrepos, localfolder)
+        ui.loadmessagelist(localrepos, localfolder)
+        localfolder.cachemessagelist(maxage=maxage)
+        ui.messagelistloaded(localrepos, localfolder, localfolder.getmessagecount())
+
         if quick:
             if (not localfolder.quickchanged(statusfolder) and
                 not remotefolder.quickchanged(statusfolder)):
@@ -436,11 +445,6 @@ def syncfolder(account, remotefolder, quick):
                 localrepos.restore_atime()
                 return
 
-        # Load local folder.
-        ui.syncingfolder(remoterepos, remotefolder, localrepos, localfolder)
-        ui.loadmessagelist(localrepos, localfolder)
-        localfolder.cachemessagelist()
-        ui.messagelistloaded(localrepos, localfolder, localfolder.getmessagecount())
 
         # If either the local or the status folder has messages and
         # there is a UID validity problem, warn and abort.  If there are
@@ -463,7 +467,21 @@ def syncfolder(account, remotefolder, quick):
 
         # Load remote folder.
         ui.loadmessagelist(remoterepos, remotefolder)
-        remotefolder.cachemessagelist()
+        # Enforce maxage on remote folder by restricting to the lowest UID in
+        # the local messagelist (which has already been restricted to > maxage)
+        if maxage:
+            if localfolder.getmessageuidlist():
+                # local messagelist might contain new messages (with uid's < 0)
+                min_uid = min(filter(lambda uid: uid > 0,
+                    localfolder.getmessageuidlist()))
+                remotefolder.cachemessagelist(min_uid=min_uid)
+            else: # no messages in range in localfolder
+                # maxage is calculated with respect to folder timezone, and
+                # remote folder timezone might be different from local, so be
+                # safe and make sure the range isn't bigger than in local
+                remotefolder.cachemessagelist(maxage=maxage - 1)
+        else:
+            remotefolder.cachemessagelist()
         ui.messagelistloaded(remoterepos, remotefolder,
                              remotefolder.getmessagecount())
 
diff --git a/offlineimap/folder/Gmail.py b/offlineimap/folder/Gmail.py
index 1afbe47..93e8eee 100644
--- a/offlineimap/folder/Gmail.py
+++ b/offlineimap/folder/Gmail.py
@@ -121,9 +121,10 @@ class GmailFolder(IMAPFolder):
 
     # TODO: merge this code with the parent's cachemessagelist:
     # TODO: they have too much common logics.
-    def cachemessagelist(self):
+    def cachemessagelist(self, maxage=None, min_uid=None):
         if not self.synclabels:
-            return super(GmailFolder, self).cachemessagelist()
+            return super(GmailFolder, self).cachemessagelist(maxage=maxage,
+                min_uid=min_uid)
 
         self.messagelist = {}
 
diff --git a/offlineimap/folder/GmailMaildir.py b/offlineimap/folder/GmailMaildir.py
index 894792d..9a092cb 100644
--- a/offlineimap/folder/GmailMaildir.py
+++ b/offlineimap/folder/GmailMaildir.py
@@ -64,9 +64,9 @@ class GmailMaildirFolder(MaildirFolder):
                 'filename': '/no-dir/no-such-file/', 'mtime': 0}
 
 
-    def cachemessagelist(self):
+    def cachemessagelist(self, maxage=None, min_uid=None):
         if self.ismessagelistempty():
-            self.messagelist = self._scanfolder()
+            self.messagelist = self._scanfolder(maxage=maxage, min_uid=min_uid)
 
         # Get mtimes
         if self.synclabels:
diff --git a/offlineimap/folder/IMAP.py b/offlineimap/folder/IMAP.py
index ccc83b0..2b59840 100644
--- a/offlineimap/folder/IMAP.py
+++ b/offlineimap/folder/IMAP.py
@@ -144,7 +144,7 @@ class IMAPFolder(BaseFolder):
         return False
 
 
-    def _msgs_to_fetch(self, imapobj):
+    def _msgs_to_fetch(self, imapobj, maxage=None, min_uid=None):
         """Determines sequence numbers of messages to be fetched.
 
         Message sequence numbers (MSNs) are more easily compacted
@@ -164,16 +164,14 @@ class IMAPFolder(BaseFolder):
         # By default examine all messages in this folder
         msgsToFetch = '1:*'
 
-        maxage = self.config.getdefaultint(
-            "Account %s"% self.accountname, "maxage", -1)
         maxsize = self.config.getdefaultint(
-            "Account %s"% self.accountname, "maxsize", -1)
+            "Account %s"% self.accountname, "maxsize", None)
 
         # Build search condition
-        if (maxage != -1) | (maxsize != -1):
+        if maxage or min_uid or maxsize:
             search_cond = "(";
 
-            if(maxage != -1):
+            if maxage:
                 #find out what the oldest message is that we should look at
                 oldest_struct = time.gmtime(time.time() - (60*60*24*maxage))
                 if oldest_struct[0] < 1900:
@@ -184,9 +182,13 @@ class IMAPFolder(BaseFolder):
                     oldest_struct[2],
                     MonthNames[oldest_struct[1]],
                     oldest_struct[0])
+            if min_uid:
+                if maxage: # There are two conditions, add space
+                    search_cond += " "
+                search_cond += "UID %d:*"% min_uid
 
-            if(maxsize != -1):
-                if(maxage != -1): # There are two conditions, add space
+            if maxsize:
+                if maxage or min_uid: # There are at least two conditions, add space
                     search_cond += " "
                 search_cond += "SMALLER %d"% maxsize
 
@@ -210,19 +212,19 @@ class IMAPFolder(BaseFolder):
 
 
     # Interface from BaseFolder
-    def cachemessagelist(self):
+    def cachemessagelist(self, maxage=None, min_uid=None):
         self.messagelist = {}
 
         imapobj = self.imapserver.acquireconnection()
         try:
-            msgsToFetch = self._msgs_to_fetch(imapobj)
+            msgsToFetch = self._msgs_to_fetch(imapobj, maxage=maxage, min_uid=min_uid)
             if not msgsToFetch:
                 return # No messages to sync
 
             # Get the flags and UIDs for these. single-quotes prevent
             # imaplib2 from quoting the sequence.
             res_type, response = imapobj.fetch("'%s'"%
-                msgsToFetch, '(FLAGS UID)')
+                msgsToFetch, '(FLAGS UID INTERNALDATE)')
             if res_type != 'OK':
                 raise OfflineImapError("FETCHING UIDs in folder [%s]%s failed. "
                     "Server responded '[%s] %s'"% (self.getrepository(), self,
@@ -840,6 +842,7 @@ class IMAPFolder(BaseFolder):
         uidlist = [uid for uid in uidlist if self.uidexists(uid)]
         if not len(uidlist):
             return
+        uid = uidlist[0]
 
         self.__addmessagesflags_noconvert(uidlist, set('T'))
         imapobj = self.imapserver.acquireconnection()
diff --git a/offlineimap/folder/Maildir.py b/offlineimap/folder/Maildir.py
index 77f7ebb..d06d8f6 100644
--- a/offlineimap/folder/Maildir.py
+++ b/offlineimap/folder/Maildir.py
@@ -150,7 +150,7 @@ class MaildirFolder(BaseFolder):
             flags = set((c for c in flagmatch.group(1) if not c.islower()))
         return prefix, uid, fmd5, flags
 
-    def _scanfolder(self):
+    def _scanfolder(self, maxage=None, min_uid=None):
         """Cache the message list from a Maildir.
 
         Maildir flags are: R (replied) S (seen) T (trashed) D (draft) F
@@ -158,8 +158,6 @@ class MaildirFolder(BaseFolder):
         :returns: dict that can be used as self.messagelist.
         """
 
-        maxage = self.config.getdefaultint("Account " + self.accountname,
-                                           "maxage", None)
         maxsize = self.config.getdefaultint("Account " + self.accountname,
                                             "maxsize", None)
         retval = {}
@@ -185,6 +183,9 @@ class MaildirFolder(BaseFolder):
                 uid = nouidcounter
                 nouidcounter -= 1
             else:                       # It comes from our folder.
+                # Check min_uid if this message should be considered.
+                if min_uid and uid < min_uid:
+                    continue
                 uidmatch = re_uidmatch.search(filename)
                 uid = None
                 if not uidmatch:
@@ -192,6 +193,7 @@ class MaildirFolder(BaseFolder):
                     nouidcounter -= 1
                 else:
                     uid = long(uidmatch.group(1))
+
             # 'filename' is 'dirannex/filename', e.g. cur/123,U=1,FMD5=1:2,S
             retval[uid] = self.msglist_item_initializer(uid)
             retval[uid]['flags'] = flags
@@ -200,8 +202,8 @@ class MaildirFolder(BaseFolder):
 
     # Interface from BaseFolder
     def quickchanged(self, statusfolder):
-        """Returns True if the Maildir has changed"""
-        self.cachemessagelist()
+        """Returns True if the Maildir has changed
+        Assumes cachemessagelist() has already been called """
         # Folder has different uids than statusfolder => TRUE.
         if sorted(self.getmessageuidlist()) != \
                 sorted(statusfolder.getmessageuidlist()):
@@ -218,9 +220,9 @@ class MaildirFolder(BaseFolder):
         return {'flags': set(), 'filename': '/no-dir/no-such-file/'}
 
     # Interface from BaseFolder
-    def cachemessagelist(self):
+    def cachemessagelist(self, maxage=None, min_uid=None):
         if self.ismessagelistempty():
-            self.messagelist = self._scanfolder()
+            self.messagelist = self._scanfolder(maxage=maxage, min_uid=min_uid)
 
     # Interface from BaseFolder
     def getmessagelist(self):
@@ -448,7 +450,10 @@ class MaildirFolder(BaseFolder):
             os.unlink(filepath)
         except OSError:
             # Can't find the file -- maybe already deleted?
-            newmsglist = self._scanfolder()
+            maxage = localfolder.config.getdefaultint("Account %s"%
+                localfolder.accountname, "maxage", None)
+            # get more than enough messages to be sure we captured this uid
+            newmsglist = self._scanfolder(maxage=maxage + 1)
             if uid in newmsglist:       # Nope, try new filename.
                 filename = newmsglist[uid]['filename']
                 filepath = os.path.join(self.getfullname(), filename)
-- 
2.3.3





More information about the OfflineIMAP-project mailing list