[PATCH v4] Re: make maxage use UIDs to avoid timezone issues

[Janna Martl]

On Thu, Mar 26, 2015 at 03:18:50AM +0100, Nicolas Sebrecht wrote:
>I never looked at this deeply because I don't use IMAP/IMAP myself but
>you will be highly interested by the content of the folder/UIDMaps.py
>file. ,-)

Thanks for the hint :)

>I just did a quick read but I really looks easy to fix. This might
>require to introduce one or two more methods to make it proper (not even
>sure about that, though).

Thanks to UIDMaps magic, it turns out that this works without
modification for the case when the local folder has type IMAP. The mess
I noticed before is because I was using type Gmail, and Gmail-IMAP
sync isn't supported (yet). In the IMAP case, UIDMaps.py defines a
MappedIMAPFolder class, so I made a MappedGmailFolder case that inherits
from GmailFolder and MappedIMAPFolder. I tried to check this in some
basic cases (copying to/from empty folder, copying/ deleting messages,
with/without maxsize) but this all seems too good to be true and I can't
shake the feeling that I missed something.


In order to do IMAP-IMAP sync, the local folder needs to inherit from
MappedIMAPFolder. Make this work for Gmail as the local folder.
---
 offlineimap/folder/Gmail.py        | 6 ++++++
 offlineimap/folder/UIDMaps.py      | 4 ++--
 offlineimap/repository/Gmail.py    | 5 +++++
 offlineimap/repository/__init__.py | 3 ++-
 4 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/offlineimap/folder/Gmail.py b/offlineimap/folder/Gmail.py
index 93e8eee..10ee45e 100644
--- a/offlineimap/folder/Gmail.py
+++ b/offlineimap/folder/Gmail.py
@@ -23,6 +23,7 @@ from offlineimap import imaputil, OfflineImapError
 from offlineimap import imaplibutil
 import offlineimap.accounts
 from .IMAP import IMAPFolder
+from offlineimap.folder.UIDMaps import MappedIMAPFolder
 
 """Folder implementation to support features of the Gmail IMAP server."""
 
@@ -369,3 +370,8 @@ class GmailFolder(IMAPFolder):
 
         except NotImplementedError:
             self.ui.warn("Can't sync labels. You need to configure a local repository of type GmailMaildir")
+
+class MappedGmailFolder(MappedIMAPFolder, GmailFolder):
+    def __init__(self, *args, **kwargs):
+        MappedIMAPFolder.__init__(self, *args, **kwargs)
+        GmailFolder.__init__(self, *args, **kwargs)
diff --git a/offlineimap/folder/UIDMaps.py b/offlineimap/folder/UIDMaps.py
index 04a986b..136b686 100644
--- a/offlineimap/folder/UIDMaps.py
+++ b/offlineimap/folder/UIDMaps.py
@@ -94,8 +94,8 @@ class MappedIMAPFolder(IMAPFolder):
                 OfflineImapError.ERROR.MESSAGE), None, exc_info()[2]
 
     # Interface from BaseFolder
-    def cachemessagelist(self):
-        self._mb.cachemessagelist()
+    def cachemessagelist(self, maxage=None, min_uid=None):
+        self._mb.cachemessagelist(maxage=maxage)
         reallist = self._mb.getmessagelist()
 
         self.maplock.acquire()
diff --git a/offlineimap/repository/Gmail.py b/offlineimap/repository/Gmail.py
index 2e23e62..f85f306 100644
--- a/offlineimap/repository/Gmail.py
+++ b/offlineimap/repository/Gmail.py
@@ -16,6 +16,7 @@
 #    Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
 
 from offlineimap.repository.IMAP import IMAPRepository
+from offlineimap.folder.Gmail import MappedGmailFolder
 from offlineimap import folder, OfflineImapError
 
 class GmailRepository(IMAPRepository):
@@ -72,3 +73,7 @@ class GmailRepository(IMAPRepository):
     def getspamfolder(self):
         #: Gmail also deletes messages upon EXPUNGE in the Spam folder
         return  self.getconf('spamfolder','[Gmail]/Spam')
+
+class MappedGmailRepository(GmailRepository):
+    def getfoldertype(self):
+        return MappedGmailFolder
diff --git a/offlineimap/repository/__init__.py b/offlineimap/repository/__init__.py
index 0fbbc13..4b9ded4 100644
--- a/offlineimap/repository/__init__.py
+++ b/offlineimap/repository/__init__.py
@@ -23,7 +23,7 @@ except ImportError: #python2
     from ConfigParser import NoSectionError
 
 from offlineimap.repository.IMAP import IMAPRepository, MappedIMAPRepository
-from offlineimap.repository.Gmail import GmailRepository
+from offlineimap.repository.Gmail import GmailRepository, MappedGmailRepository
 from offlineimap.repository.Maildir import MaildirRepository
 from offlineimap.repository.GmailMaildir import GmailMaildirRepository
 from offlineimap.repository.LocalStatus import LocalStatusRepository
@@ -49,6 +49,7 @@ class Repository(object):
         elif reqtype == 'local':
             name = account.getconf('localrepository')
             typemap = {'IMAP': MappedIMAPRepository,
+                'Gmail': MappedGmailRepository,
                 'Maildir': MaildirRepository,
                 'GmailMaildir': GmailMaildirRepository}
 
-- 
2.3.4



There is another caveat though (*sigh*): suppose remote is empty, and
local has messages with UID's L_1 < ... < L_n. Then they get copied to
remote in a random order, so, if R_1 < ... < R_n are the remote UID's in
order, then maybe R5 is the message that was copied from L22. This is
bad: if min_uid = L_k, and this corresponds to R_l, the local
messagelist [L_k, ...] has nothing to do with the remote messagelist
[R_l, ...], even after correcting for UID mapping.

I figured that this problem would go away if you make sure the copylist
is in order, but (1) this probably isn't the nicest solution; (2) I
think I implemented it kind of awkwardly; (3) it doesn't even always
work -- even after sorting, I still found a pair of local messages whose
remote counterparts weren't in the same order (?). Anyway, for what it's
worth:

---
 offlineimap/folder/Base.py    | 9 +++++++++
 offlineimap/folder/UIDMaps.py | 6 ++++++
 2 files changed, 15 insertions(+)

diff --git a/offlineimap/folder/Base.py b/offlineimap/folder/Base.py
index 123508c..912b4e8 100644
--- a/offlineimap/folder/Base.py
+++ b/offlineimap/folder/Base.py
@@ -306,6 +306,12 @@ class BaseFolder(object):
         return self.config.getdefaultint("Account %s"%
             self.accountname, "maxsize", None)
 
+    def sortlocal(self, uidlist):
+        """ Sorts uidlist. This is just to have something overridden by
+        sortlocal() in UIDMaps.py """
+        uidlist.sort()
+        return uidlist
+
     def savemessage(self, uid, content, flags, rtime):
         """Writes a new message, with the specified uid.
 
@@ -761,6 +767,9 @@ class BaseFolder(object):
 
         copylist = filter(lambda uid: not statusfolder.uidexists(uid),
             self.getmessageuidlist())
+        # For IMAP-IMAP sync case, make sure that UID's of copied messages are
+        # in the same order as UID's of original messages
+        copylist = self.sortlocal(copylist)
         num_to_copy = len(copylist)
         if num_to_copy and self.repository.account.dryrun:
             self.ui.info("[DRYRUN] Copy {0} messages from {1}[{2}] to {3}".format(
diff --git a/offlineimap/folder/UIDMaps.py b/offlineimap/folder/UIDMaps.py
index 136b686..d558e0d 100644
--- a/offlineimap/folder/UIDMaps.py
+++ b/offlineimap/folder/UIDMaps.py
@@ -151,6 +151,12 @@ class MappedIMAPFolder(IMAPFolder):
         # much more efficient for the mapped case.
         return len(self.r2l)
 
+    def sortlocal(self, uidlist):
+        """ uidlist is a list of remote UID's. Sort this according to local order"""
+        local_uidlist = map(lambda r: self.r2l[r], uidlist)
+        local_uidlist.sort()
+        return map(lambda l: self.l2r[l],local_uidlist)
+
     # Interface from BaseFolder
     def getmessagelist(self):
         """Gets the current message list. This function's implementation
-- 
2.3.4