[PATCH 1/1] Re: make maxage use UIDs to avoid timezone issues (edit: correction)
Nicolas Sebrecht
nicolas.s-dev at laposte.net
Sat Mar 21 11:16:27 GMT 2015
Nice.
I have some little comments but I'm not asking for another round; I'll
fix them myself if they are fine for you.
On Sat, Mar 21, 2015 at 02:27:53AM -0400, Janna Martl wrote:
> diff --git a/docs/offlineimap.txt b/docs/offlineimap.txt
> index f4b844c..9fb29a1 100644
> --- a/docs/offlineimap.txt
> +++ b/docs/offlineimap.txt
> @@ -135,7 +135,8 @@ Ignore any autorefresh setting in the configuration file.
> Run only quick synchronizations.
> +
> Ignore any flag updates on IMAP servers. If a flag on the remote IMAP changes,
> -and we have the message locally, it will be left untouched in a quick run.
> +and we have the message locally, it will be left untouched in a quick run. This
> +option is ignored if maxage is set in offlineimap.conf.
>
>
> -u <UI>::
> diff --git a/offlineimap/accounts.py b/offlineimap/accounts.py
> index 62ed5c3..8c940ff 100644
> --- a/offlineimap/accounts.py
> +++ b/offlineimap/accounts.py
> @@ -429,19 +429,27 @@ def syncfolder(account, remotefolder, quick):
>
> statusfolder.cachemessagelist()
>
> - if quick:
> - if (not localfolder.quickchanged(statusfolder) and
> - not remotefolder.quickchanged(statusfolder)):
> - ui.skippingfolder(remotefolder)
> - localrepos.restore_atime()
> - return
> + maxage = localfolder.getmaxage()
>
> # Load local folder.
> ui.syncingfolder(remoterepos, remotefolder, localrepos, localfolder)
> ui.loadmessagelist(localrepos, localfolder)
> - localfolder.cachemessagelist()
> + localfolder.cachemessagelist(maxage=maxage)
> ui.messagelistloaded(localrepos, localfolder, localfolder.getmessagecount())
>
> + # IMAP quickcheck can't take maxage into account, so if that's set, don't try.
s/quickcheck/quickchanged/
But the wording makes it souns wrong.
# IMAP quickchanged logic is not compatible with a restricted
# messagelist since it expects all the UIDs list of the folder.
> + if quick:
> + if maxage:
> + ui.warn("Warning: quick syncs (-q) not supported in conjunction\
> + with maxage; ignoring -q.")
s/Warning: //
> + else:
> + if (not localfolder.quickchanged(statusfolder) and
> + not remotefolder.quickchanged(statusfolder)):
> + ui.skippingfolder(remotefolder)
> + localrepos.restore_atime()
> + return
> +
> +
> # If either the local or the status folder has messages and
> # there is a UID validity problem, warn and abort. If there are
> # no messages, UW IMAPd loses UIDVALIDITY. But we don't really
> @@ -463,7 +471,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)
s/which.*/which had maxage option already applied)/
This is because '> maxage' sounds wrong. maxage is a number of days.
Stricly speaking it should be '< maxage' but it's still ambigous because
we are working with messages '> min_uid.
> + 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/Base.py b/offlineimap/folder/Base.py
> index df2e654..af4bfc9 100644
> --- a/offlineimap/folder/Base.py
> +++ b/offlineimap/folder/Base.py
> @@ -298,6 +298,14 @@ class BaseFolder(object):
>
> raise NotImplementedError
>
> + def getmaxage(self):
> + return self.config.getdefaultint("Account %s"%
> + self.accountname, "maxage", None)
> +
> + def getmaxsize(self):
> + return self.config.getdefaultint("Account %s"%
> + self.accountname, "maxsize", None)
> +
> def savemessage(self, uid, content, flags, rtime):
> """Writes a new message, with the specified uid.
>
> 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..0ae00bf 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..20f5cda 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,13 @@ 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)
> + maxsize = self.getmaxsize()
>
> # 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 +181,13 @@ class IMAPFolder(BaseFolder):
> oldest_struct[2],
> MonthNames[oldest_struct[1]],
> oldest_struct[0])
> + if min_uid:
Didn't catched this before but this will fail if min_uid is 0.
We want
if min_uid != None:
I'll make the change everywhere for maxage and maxsize too because this
is more explicit and the latter comparison don't have edge-cases.
> + 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 +211,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,
> diff --git a/offlineimap/folder/Maildir.py b/offlineimap/folder/Maildir.py
> index 77f7ebb..8bbfa07 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,10 +158,8 @@ 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)
> + maxsize = self.getmaxsize()
> +
> retval = {}
> files = []
> nouidcounter = -1 # Messages without UIDs get negative UIDs.
> @@ -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:
> @@ -200,8 +201,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
/new blank line/
> + Assumes cachemessagelist() has already been called """
> # Folder has different uids than statusfolder => TRUE.
> if sorted(self.getmessageuidlist()) != \
> sorted(statusfolder.getmessageuidlist()):
> @@ -218,9 +219,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 +449,9 @@ class MaildirFolder(BaseFolder):
> os.unlink(filepath)
> except OSError:
> # Can't find the file -- maybe already deleted?
> - newmsglist = self._scanfolder()
> + maxage = localfolder.getmaxage()
> + # get more than enough messages to be sure we captured this uid
> + newmsglist = self._scanfolder(maxage=maxage + 1)
s/this uid/the min UID expected on the other side./
> if uid in newmsglist: # Nope, try new filename.
> filename = newmsglist[uid]['filename']
> filepath = os.path.join(self.getfullname(), filename)
--
Nicolas Sebrecht
More information about the OfflineIMAP-project
mailing list