[PATCH] Re: properly check whether dstfolder supports labels
Nicolas Sebrecht
nicolas.s-dev at laposte.net
Mon Apr 6 18:43:46 BST 2015
On Mon, Apr 06, 2015 at 06:08:11PM +0200, Abdo Roig-Maranges wrote:
> This adds a storeslabels() method to check whether a folder supports
> labels, and checks it in copymessageto and copymessageto_labels,
> replacing an exception-based check.
Good!
It's straightforward enough so that I'm inclined to merge even without
IMAP/Gmail support if the "warnings" and style issues get fixed.
> ---
> offlineimap/folder/Base.py | 4 +
> offlineimap/folder/Gmail.py | 110 +++++++++--------
> offlineimap/folder/GmailMaildir.py | 202 ++++++++++++++++----------------
> offlineimap/folder/LocalStatus.py | 4 +
> offlineimap/folder/LocalStatusSQLite.py | 6 +-
> 5 files changed, 175 insertions(+), 151 deletions(-)
>
> diff --git a/offlineimap/folder/Base.py b/offlineimap/folder/Base.py
> index 16b5819..88b233b 100644
> --- a/offlineimap/folder/Base.py
> +++ b/offlineimap/folder/Base.py
> @@ -134,6 +134,10 @@ class BaseFolder(object):
>
> return 1
>
> + def storeslabels(self):
> + """Should be true for any backend that is able to store message labels."""
> + return False
> +
> def getvisiblename(self):
> """The nametrans-transposed name of the folder's name."""
>
> diff --git a/offlineimap/folder/Gmail.py b/offlineimap/folder/Gmail.py
> index 1afbe47..c17d819 100644
> --- a/offlineimap/folder/Gmail.py
> +++ b/offlineimap/folder/Gmail.py
> @@ -64,6 +64,11 @@ class GmailFolder(IMAPFolder):
> self.ignorelabels = set([l for l in re.split(r'\s*,\s*', ignorelabels) if len(l)])
>
>
> + def storeslabels(self):
> + """Should be true for any backend that is able to store message labels."""
> + return True
> +
> +
> def getmessage(self, uid):
> """Retrieve message with UID from the IMAP server (incl body). Also
> gets Gmail labels and embeds them into the message.
> @@ -290,16 +295,16 @@ class GmailFolder(IMAPFolder):
>
> # sync labels and mtime now when the message is new (the embedded labels are up to date)
> # otherwise we may be spending time for nothing, as they will get updated on a later pass.
> + # check that target folder supports labels.
> if realcopy and self.synclabels:
> - try:
> - mtime = dstfolder.getmessagemtime(uid)
> - labels = dstfolder.getmessagelabels(uid)
> - statusfolder.savemessagelabels(uid, labels, mtime=mtime)
> -
> - # dstfolder is not GmailMaildir.
> - except NotImplementedError:
> + if not dstfolder.storeslabels():
> + self.ui.warn("Repository '%s' does not support labels." % dstfolder.repository.name)
> return
About the warnings:
- either we fully support Gmail/IMAP and warn about that is not
expected
- or we don't support Gmaild/IMAP (yet) and we should stop syncing the
current account.
>
> + mtime = dstfolder.getmessagemtime(uid)
Side note: so, mtime would be better called labels_mtime, right?
> + labels = dstfolder.getmessagelabels(uid)
> + statusfolder.savemessagelabels(uid, labels, mtime=mtime)
> +
> def syncmessagesto_labels(self, dstfolder, statusfolder):
> """Pass 4: Label Synchronization (Gmail only)
>
> @@ -315,56 +320,57 @@ class GmailFolder(IMAPFolder):
> # the fastest thing in the world though...
> uidlist = []
>
> + # check that target folder supports labels.
> + if not dstfolder.storeslabels():
> + self.ui.warn("Repository '%s' does not support labels." % dstfolder.repository.name)
Ditto.
As a side note, please avoid the whitespace between " and % for string
substitutions.
> + return
> +
> # filter the uids (fast)
> - try:
> - for uid in self.getmessageuidlist():
> - # bail out on CTRL-C or SIGTERM
> - if offlineimap.accounts.Account.abort_NOW_signal.is_set():
> - break
> + for uid in self.getmessageuidlist():
> + # bail out on CTRL-C or SIGTERM
> + if offlineimap.accounts.Account.abort_NOW_signal.is_set():
> + break
> +
> + # Ignore messages with negative UIDs missed by pass 1 and
> + # don't do anything if the message has been deleted remotely
> + if uid < 0 or not dstfolder.uidexists(uid):
> + continue
>
> - # Ignore messages with negative UIDs missed by pass 1 and
> - # don't do anything if the message has been deleted remotely
> - if uid < 0 or not dstfolder.uidexists(uid):
> - continue
> + selflabels = self.getmessagelabels(uid) - self.ignorelabels
Side note: I don't like "selflabels". It looks like a dot was forgot.ten.
>
> - selflabels = self.getmessagelabels(uid) - self.ignorelabels
> + if statusfolder.uidexists(uid):
> + statuslabels = statusfolder.getmessagelabels(uid) - self.ignorelabels
> + else:
> + statuslabels = set()
>
> - if statusfolder.uidexists(uid):
> - statuslabels = statusfolder.getmessagelabels(uid) - self.ignorelabels
> - else:
> - statuslabels = set()
> + if selflabels != statuslabels:
> + uidlist.append(uid)
>
> - if selflabels != statuslabels:
> - uidlist.append(uid)
> + # now sync labels (slow)
While at it, explain why it's slow!
> + mtimes = {}
> + labels = {}
> + for i, uid in enumerate(uidlist):
> + # bail out on CTRL-C or SIGTERM
> + if offlineimap.accounts.Account.abort_NOW_signal.is_set():
> + break
>
> - # now sync labels (slow)
> - mtimes = {}
> - labels = {}
> - for i, uid in enumerate(uidlist):
> - # bail out on CTRL-C or SIGTERM
> - if offlineimap.accounts.Account.abort_NOW_signal.is_set():
> - break
> + selflabels = self.getmessagelabels(uid) - self.ignorelabels
>
> - selflabels = self.getmessagelabels(uid) - self.ignorelabels
> + if statusfolder.uidexists(uid):
> + statuslabels = statusfolder.getmessagelabels(uid) - self.ignorelabels
> + else:
> + statuslabels = set()
>
> - if statusfolder.uidexists(uid):
> - statuslabels = statusfolder.getmessagelabels(uid) - self.ignorelabels
> - else:
> - statuslabels = set()
> -
> - if selflabels != statuslabels:
> - self.ui.settinglabels(uid, i+1, len(uidlist), sorted(selflabels), dstfolder)
> - if self.repository.account.dryrun:
> - continue #don't actually add in a dryrun
> - dstfolder.savemessagelabels(uid, selflabels, ignorelabels = self.ignorelabels)
> - mtime = dstfolder.getmessagemtime(uid)
> - mtimes[uid] = mtime
> - labels[uid] = selflabels
> -
> - # Update statusfolder in a single DB transaction. It is safe, as if something fails,
> - # statusfolder will be updated on the next run.
> - statusfolder.savemessageslabelsbulk(labels)
> - statusfolder.savemessagesmtimebulk(mtimes)
> -
> - except NotImplementedError:
> - self.ui.warn("Can't sync labels. You need to configure a local repository of type GmailMaildir")
> + if selflabels != statuslabels:
> + self.ui.settinglabels(uid, i+1, len(uidlist), sorted(selflabels), dstfolder)
> + if self.repository.account.dryrun:
> + continue #don't actually add in a dryrun
^^
Missing whitespace.
> + dstfolder.savemessagelabels(uid, selflabels, ignorelabels = self.ignorelabels)
^^^
Too much whitespaces
> + mtime = dstfolder.getmessagemtime(uid)
> + mtimes[uid] = mtime
> + labels[uid] = selflabels
> +
> + # Update statusfolder in a single DB transaction. It is safe, as if something fails,
> + # statusfolder will be updated on the next run.
> + statusfolder.savemessageslabelsbulk(labels)
> + statusfolder.savemessagesmtimebulk(mtimes)
> diff --git a/offlineimap/folder/GmailMaildir.py b/offlineimap/folder/GmailMaildir.py
> index 894792d..acd4dff 100644
> --- a/offlineimap/folder/GmailMaildir.py
> +++ b/offlineimap/folder/GmailMaildir.py
> @@ -39,6 +39,12 @@ class GmailMaildirFolder(MaildirFolder):
> if self.synclabels:
> self.syncmessagesto_passes.append(('syncing labels', self.syncmessagesto_labels))
>
> +
> + def storeslabels(self):
> + """Should be true for any backend that is able to store message labels."""
> + return True
> +
> +
> def quickchanged(self, statusfolder):
> """Returns True if the Maildir has changed. Checks uids, flags and mtimes"""
>
> @@ -205,14 +211,14 @@ class GmailMaildirFolder(MaildirFolder):
> # for message which already existed on the remote, this is useless, as later the labels may
> # get updated.
> if realcopy and self.synclabels:
> - try:
> - labels = dstfolder.getmessagelabels(uid)
> - statusfolder.savemessagelabels(uid, labels, mtime=self.getmessagemtime(uid))
> -
> - # dstfolder is not GmailMaildir.
> - except NotImplementedError:
> + # check that target folder supports labels.
> + if not dstfolder.storeslabels():
> + self.ui.warn("Repository '%s' does not support labels." % dstfolder.repository.name)
Ditto.
> return
>
> + labels = dstfolder.getmessagelabels(uid)
> + statusfolder.savemessagelabels(uid, labels, mtime=self.getmessagemtime(uid))
> +
> def syncmessagesto_labels(self, dstfolder, statusfolder):
> """Pass 4: Label Synchronization (Gmail only)
>
> @@ -233,95 +239,95 @@ class GmailMaildirFolder(MaildirFolder):
> dellabellist = {}
> uidlist = []
>
> - try:
> - # filter uids (fast)
> - for uid in self.getmessageuidlist():
> - # bail out on CTRL-C or SIGTERM
> - if offlineimap.accounts.Account.abort_NOW_signal.is_set():
> - break
> -
> - # Ignore messages with negative UIDs missed by pass 1 and
> - # don't do anything if the message has been deleted remotely
> - if uid < 0 or not dstfolder.uidexists(uid):
> - continue
> -
> - selfmtime = self.getmessagemtime(uid)
> -
> - if statusfolder.uidexists(uid):
> - statusmtime = statusfolder.getmessagemtime(uid)
> - else:
> - statusmtime = 0
> -
> - if selfmtime > statusmtime:
> - uidlist.append(uid)
> -
> -
> - self.ui.collectingdata(uidlist, self)
> - # This can be slow if there is a lot of modified files
> - for uid in uidlist:
> - # bail out on CTRL-C or SIGTERM
> - if offlineimap.accounts.Account.abort_NOW_signal.is_set():
> - break
> -
> - selflabels = self.getmessagelabels(uid)
> -
> - if statusfolder.uidexists(uid):
> - statuslabels = statusfolder.getmessagelabels(uid)
> - else:
> - statuslabels = set()
> -
> - addlabels = selflabels - statuslabels
> - dellabels = statuslabels - selflabels
> -
> - for lb in addlabels:
> - if not lb in addlabellist:
> - addlabellist[lb] = []
> - addlabellist[lb].append(uid)
> -
> - for lb in dellabels:
> - if not lb in dellabellist:
> - dellabellist[lb] = []
> - dellabellist[lb].append(uid)
> -
> - for lb, uids in addlabellist.items():
> - # bail out on CTRL-C or SIGTERM
> - if offlineimap.accounts.Account.abort_NOW_signal.is_set():
> - break
> -
> - self.ui.addinglabels(uids, lb, dstfolder)
> - if self.repository.account.dryrun:
> - continue #don't actually add in a dryrun
> - dstfolder.addmessageslabels(uids, set([lb]))
> - statusfolder.addmessageslabels(uids, set([lb]))
> -
> - for lb, uids in dellabellist.items():
> - # bail out on CTRL-C or SIGTERM
> - if offlineimap.accounts.Account.abort_NOW_signal.is_set():
> - break
> -
> - self.ui.deletinglabels(uids, lb, dstfolder)
> - if self.repository.account.dryrun:
> - continue #don't actually remove in a dryrun
> - dstfolder.deletemessageslabels(uids, set([lb]))
> - statusfolder.deletemessageslabels(uids, set([lb]))
> -
> - # Update mtimes on StatusFolder. It is done last to be safe. If something els fails
> - # and the mtime is not updated, the labels will still be synced next time.
> - mtimes = {}
> - for uid in uidlist:
> - # bail out on CTRL-C or SIGTERM
> - if offlineimap.accounts.Account.abort_NOW_signal.is_set():
> - break
> -
> - if self.repository.account.dryrun:
> - continue #don't actually update statusfolder
> -
> - filename = self.messagelist[uid]['filename']
> - filepath = os.path.join(self.getfullname(), filename)
> - mtimes[uid] = long(os.stat(filepath).st_mtime)
> -
> - # finally update statusfolder in a single DB transaction
> - statusfolder.savemessagesmtimebulk(mtimes)
> -
> - except NotImplementedError:
> - self.ui.warn("Can't sync labels. You need to configure a remote repository of type Gmail.")
> + # check that target folder supports labels.
> + if not dstfolder.storeslabels():
> + self.ui.warn("Repository '%s' does not support labels." % dstfolder.repository.name)
> + return
> +
> + # filter uids (fast)
> + for uid in self.getmessageuidlist():
> + # bail out on CTRL-C or SIGTERM
> + if offlineimap.accounts.Account.abort_NOW_signal.is_set():
> + break
> +
> + # Ignore messages with negative UIDs missed by pass 1 and
> + # don't do anything if the message has been deleted remotely
> + if uid < 0 or not dstfolder.uidexists(uid):
> + continue
> +
> + selfmtime = self.getmessagemtime(uid)
> +
> + if statusfolder.uidexists(uid):
> + statusmtime = statusfolder.getmessagemtime(uid)
> + else:
> + statusmtime = 0
> +
> + if selfmtime > statusmtime:
> + uidlist.append(uid)
> +
> + self.ui.collectingdata(uidlist, self)
> + # This can be slow if there is a lot of modified files
> + for uid in uidlist:
> + # bail out on CTRL-C or SIGTERM
> + if offlineimap.accounts.Account.abort_NOW_signal.is_set():
> + break
> +
> + selflabels = self.getmessagelabels(uid)
> +
> + if statusfolder.uidexists(uid):
> + statuslabels = statusfolder.getmessagelabels(uid)
> + else:
> + statuslabels = set()
> +
> + addlabels = selflabels - statuslabels
> + dellabels = statuslabels - selflabels
> +
> + for lb in addlabels:
> + if not lb in addlabellist:
> + addlabellist[lb] = []
> + addlabellist[lb].append(uid)
> +
> + for lb in dellabels:
> + if not lb in dellabellist:
> + dellabellist[lb] = []
> + dellabellist[lb].append(uid)
> +
> + for lb, uids in addlabellist.items():
> + # bail out on CTRL-C or SIGTERM
> + if offlineimap.accounts.Account.abort_NOW_signal.is_set():
> + break
> +
> + self.ui.addinglabels(uids, lb, dstfolder)
> + if self.repository.account.dryrun:
> + continue #don't actually add in a dryrun
> + dstfolder.addmessageslabels(uids, set([lb]))
> + statusfolder.addmessageslabels(uids, set([lb]))
> +
> + for lb, uids in dellabellist.items():
> + # bail out on CTRL-C or SIGTERM
> + if offlineimap.accounts.Account.abort_NOW_signal.is_set():
> + break
> +
> + self.ui.deletinglabels(uids, lb, dstfolder)
> + if self.repository.account.dryrun:
> + continue #don't actually remove in a dryrun
> + dstfolder.deletemessageslabels(uids, set([lb]))
> + statusfolder.deletemessageslabels(uids, set([lb]))
> +
> + # Update mtimes on StatusFolder. It is done last to be safe. If something els fails
> + # and the mtime is not updated, the labels will still be synced next time.
> + mtimes = {}
> + for uid in uidlist:
> + # bail out on CTRL-C or SIGTERM
> + if offlineimap.accounts.Account.abort_NOW_signal.is_set():
> + break
> +
> + if self.repository.account.dryrun:
> + continue #don't actually update statusfolder
> +
> + filename = self.messagelist[uid]['filename']
> + filepath = os.path.join(self.getfullname(), filename)
> + mtimes[uid] = long(os.stat(filepath).st_mtime)
> +
> + # finally update statusfolder in a single DB transaction
> + statusfolder.savemessagesmtimebulk(mtimes)
> diff --git a/offlineimap/folder/LocalStatus.py b/offlineimap/folder/LocalStatus.py
> index 78f2134..2c3d9bb 100644
> --- a/offlineimap/folder/LocalStatus.py
> +++ b/offlineimap/folder/LocalStatus.py
> @@ -43,6 +43,10 @@ class LocalStatusFolder(BaseFolder):
> def storesmessages(self):
> return 0
>
> + def storeslabels(self):
> + """Should be true for any backend that is able to store message labels."""
> + return True
> +
> def isnewfolder(self):
> return not os.path.exists(self.filename)
>
> diff --git a/offlineimap/folder/LocalStatusSQLite.py b/offlineimap/folder/LocalStatusSQLite.py
> index 64adcd1..d0e897f 100644
> --- a/offlineimap/folder/LocalStatusSQLite.py
> +++ b/offlineimap/folder/LocalStatusSQLite.py
> @@ -97,6 +97,10 @@ class LocalStatusSQLiteFolder(BaseFolder):
> def storesmessages(self):
> return False
>
> + def storeslabels(self):
> + """Should be true for any backend that is able to store message labels."""
> + return True
> +
> def getfullname(self):
> return self.filename
>
> @@ -338,7 +342,7 @@ class LocalStatusSQLiteFolder(BaseFolder):
> def savemessageslabelsbulk(self, labels):
> """
> Saves labels from a dictionary in a single database operation.
> -
> +
> """
> data = [(', '.join(sorted(l)), uid) for uid, l in labels.items()]
> self.__sql_write('UPDATE status SET labels=? WHERE id=?', data, executemany=True)
--
Nicolas Sebrecht
More information about the OfflineIMAP-project
mailing list