PARTIALLY REMOVING MAXAGE (was: [PATCH v4] make maxage use UIDs to avoid timezone issues)
Janna Martl
janna.martl109 at gmail.com
Mon Apr 6 08:29:11 BST 2015
On Thu, Apr 02, 2015 at 10:50:21AM +0200, Nicolas Sebrecht wrote:
>
> This is PATCH v8.
now v9.
I fixed some bugs and implemented your suggestions. This applies on top
of version 8.
>> + if localfolder.getmessagecount() or statusfolder.getmessagecount():
>
>Shouldn't it be:
> if localfolder.getmessagecount() > 0 and statusfolder.getmessagecount() > 0:
I'm not sure about this; it was pre-existing code that I just moved. Do
you disagree with the comment? I tried to keep this the same as before,
including the fact that uidvalidity is always called after local is
synced but before remote is synced. It was probably this way because you
don't need to get an IMAP folder's messagelist to check uidvalidity.
But uidvalidity doesn't do anything for Maildirs, so it seems to me the
obvious thing to do is:
if local is empty and (or?) statusfolder is empty:
local.check_uidvalidity()
if remote is empty and (or?) statusfolder is empty:
remote.check_uidvalidity()
# do something depending on startdate/maxage to cache local and remote
# folder's messagelists
I didn't change anything because I'm not sure if I'm understanding this
correctly, and I also don't think the previous thing was wrong. I'm just
explaining why I did
if maxage != None:
# uidvalidity is called in cachemessagelists_upto_date
cachemessagelists_upto_date(localfolder, remotefolder, maxage)
elif localstart != None:
cachemessagelists_startdate(remotefolder, localfolder,
localstart)
check_uid_validity(localfolder, remotefolder, statusfolder)
elif remotestart != None:
cachemessagelists_startdate(localfolder, remotefolder,
remotestart)
check_uid_validity(localfolder, remotefolder, statusfolder)
1. When using maxage, local and remote messagelists are supposed to only
contain messages from at most maxage days ago. But local and remote used
different timezones to calculate what "maxage days ago" means, resulting
in removals on one side. 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.
2. maxage was fundamentally wrong in the IMAP-IMAP case: it assumed that
remote messages have UIDs in the same order as their local counterparts,
which could be false, e.g. when messages are copied in quick succession.
So, remove support for maxage in the IMAP-IMAP case.
3. Add startdate option for IMAP-IMAP syncs: use messages from the given
repository starting at startdate, and all messages from the other
repository. In the first sync, the other repository must be empty.
4. Allow maxage to be specified either as number of days to sync (as
previously) or as a fixed date.
---
offlineimap/accounts.py | 37 +++++++++++++++++++++++--------------
offlineimap/folder/Base.py | 14 +++++++++-----
offlineimap/folder/GmailMaildir.py | 4 ++--
3 files changed, 34 insertions(+), 21 deletions(-)
diff --git a/offlineimap/accounts.py b/offlineimap/accounts.py
index b192eca..872b835 100644
--- a/offlineimap/accounts.py
+++ b/offlineimap/accounts.py
@@ -409,10 +409,10 @@ def syncfolder(account, remotefolder, quick):
# no messages, UW IMAPd loses UIDVALIDITY. But we don't really
# need it if both local folders are empty. So, in that case,
# just save it off.
- if localfolder.getmessagecount() or statusfolder.getmessagecount():
+ if localfolder.getmessagecount() > 0 or statusfolder.getmessagecount() > 0:
if not localfolder.check_uidvalidity():
ui.validityproblem(localfolder)
- localrepos.restore_atime()
+ localfolder.repository.restore_atime()
return
if not remotefolder.check_uidvalidity():
ui.validityproblem(remotefolder)
@@ -429,9 +429,8 @@ def syncfolder(account, remotefolder, quick):
fd.write(str(min_uid) + "\n")
fd.close()
- def cachemessagelists_by_date(localfolder, remotefolder, date):
- """ Returns messages with uid > min(uids of within-date
- messages)."""
+ def cachemessagelists_upto_date(localfolder, remotefolder, date):
+ """ Returns messages with uid > min(uids of messages newer than date)."""
localfolder.cachemessagelist(min_date=date)
check_uid_validity(localfolder, remotefolder, statusfolder)
@@ -471,16 +470,26 @@ def syncfolder(account, remotefolder, quick):
"""
new.cachemessagelist()
- if not new.getmessageuidlist():
- partial.cachemessagelist(min_date=date)
- uids = partial.getmessageuidlist()
- if len(uids) > 0:
- min_uid = min(uids)
+ min_uid = partial.retrieve_min_uid()
+ if min_uid == None: # min_uid file didn't exist
+ if len(new.getmessageuidlist()) > 0:
+ raise OfflineImapError("To use startdate on Repository %s, "
+ "Repository %s must be empty"%
+ (partial.repository.name, new.repository.name),
+ OfflineImapError.ERROR.MESSAGE)
else:
- min_uid = 1
- save_min_uid(partial, min_uid)
+ partial.cachemessagelist(min_date=date)
+ # messagelist.keys() instead of getuidmessagelist() because in
+ # the UID mapped case we want the actual local UIDs, not their
+ # remote counterparts
+ positive_uids = filter(
+ lambda uid: uid > 0, partial.messagelist.keys())
+ if len(positive_uids) > 0:
+ min_uid = min(positive_uids)
+ else:
+ min_uid = 1
+ save_min_uid(partial, min_uid)
else:
- min_uid = partial.retrieve_min_uid()
partial.cachemessagelist(min_uid=min_uid)
@@ -532,7 +541,7 @@ def syncfolder(account, remotefolder, quick):
ui.warn("Quick syncs (-q) not supported in conjunction "
"with maxage or startdate; ignoring -q.")
if maxage != None:
- cachemessagelists_by_date(localfolder, remotefolder, maxage)
+ cachemessagelists_upto_date(localfolder, remotefolder, maxage)
elif localstart != None:
cachemessagelists_startdate(remotefolder, localfolder,
localstart)
diff --git a/offlineimap/folder/Base.py b/offlineimap/folder/Base.py
index e52bec8..fdb26f1 100644
--- a/offlineimap/folder/Base.py
+++ b/offlineimap/folder/Base.py
@@ -305,14 +305,17 @@ class BaseFolder(object):
maxagestr = self.config.getdefault("Account %s"%
self.accountname, "maxage", None)
- if not maxagestr:
+ if maxagestr == None:
return None
# is it a number?
try:
maxage = int(maxagestr)
+ if maxage < 1:
+ raise OfflineImapError("invalid maxage value %d"% maxage,
+ OfflineImapError.ERROR.MESSAGE)
return time.gmtime(time.time() - 60*60*24*maxage)
except ValueError:
- pass
+ pass # maybe it was a date
# is it a date string?
try:
date = time.strptime(maxagestr, "%Y-%m-%d")
@@ -322,7 +325,7 @@ class BaseFolder(object):
OfflineImapError.ERROR.MESSAGE)
return date
except ValueError:
- raise OfflineImapError("invalid maxage value %s",
+ raise OfflineImapError("invalid maxage value %s"% maxagestr,
OfflineImapError.ERROR.MESSAGE)
def getmaxsize(self):
@@ -355,14 +358,15 @@ class BaseFolder(object):
def retrieve_min_uid(self):
uidfile = self.get_min_uid_file()
+ if not os.path.exists(uidfile):
+ return None
try:
fd = open(uidfile, 'rt')
min_uid = long(fd.readline().strip())
fd.close()
return min_uid
except:
- raise IOError("Can't read %s. To start using startdate, "\
- "folder must be empty"% uidfile)
+ raise IOError("Can't read %s"% uidfile)
def savemessage(self, uid, content, flags, rtime):
diff --git a/offlineimap/folder/GmailMaildir.py b/offlineimap/folder/GmailMaildir.py
index 0ae00bf..f3b9467 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, maxage=None, min_uid=None):
+ def cachemessagelist(self, min_date=None, min_uid=None):
if self.ismessagelistempty():
- self.messagelist = self._scanfolder(maxage=maxage, min_uid=min_uid)
+ self.messagelist = self._scanfolder(min_date=min_date, min_uid=min_uid)
# Get mtimes
if self.synclabels:
--
2.3.5
More information about the OfflineIMAP-project
mailing list