[PATCH] Make sure folder filters are always respected.

Dave Abrahams dave at boostpro.com
Thu Jul 19 23:18:57 UTC 2012


* Value of sync_this is determined at Folder __init__ time so it is
  never inadvertently skipped.

* Fix omitted check for local folder's sync_this value in
  SyncableAccount.sync()

* Add should_sync_folder() method to Repository

* Eliminate DRY violations

  - Factor out the code to find a local folder given a remote folder

  - Factor out the code that reports on skipped
    folders (sync_this_verbose)
---
 offlineimap/accounts.py           | 26 ++++++++++++++++----------
 offlineimap/folder/Base.py        | 17 +++++++++++++++--
 offlineimap/repository/Base.py    |  5 ++++-
 offlineimap/repository/IMAP.py    |  5 -----
 offlineimap/repository/Maildir.py |  5 -----
 5 files changed, 35 insertions(+), 23 deletions(-)

diff --git a/offlineimap/accounts.py b/offlineimap/accounts.py
index 8b39c7e..a3ba33d 100644
--- a/offlineimap/accounts.py
+++ b/offlineimap/accounts.py
@@ -255,6 +255,12 @@ class SyncableAccount(Account):
                 if looping and self.sleeper() >= 2:
                     looping = 0
 
+    def get_local_folder(self, remotefolder):
+        """Return the corresponding local folder for a given remotefolder"""
+        return self.localrepos.getfolder(
+            remotefolder.getvisiblename().
+            replace(self.remoterepos.getsep(), self.localrepos.getsep()))
+
     def sync(self):
         """Synchronize the account once, then return
 
@@ -300,10 +306,14 @@ class SyncableAccount(Account):
             for remotefolder in remoterepos.getfolders():
                 # check for CTRL-C or SIGTERM
                 if Account.abort_NOW_signal.is_set(): break
-                if not remotefolder.sync_this:
-                    self.ui.debug('', "Not syncing filtered remote folder '%s'"
-                                  "[%s]" % (remotefolder, remoterepos))
-                    continue # Filtered out remote folder
+
+                localfolder = self.get_local_folder(remotefolder)
+
+                if not (
+                    remotefolder.sync_this_verbose()
+                    and localfolder.sync_this_verbose()):
+                    continue
+
                 thread = InstanceLimitedThread(\
                     instancename = 'FOLDER_' + self.remoterepos.getname(),
                     target = syncfolder,
@@ -367,16 +377,12 @@ def syncfolder(account, remotefolder, quick):
     ui.registerthread(account)
     try:
         # Load local folder.
-        localfolder = localrepos.\
-                      getfolder(remotefolder.getvisiblename().\
-                                replace(remoterepos.getsep(), localrepos.getsep()))
+        localfolder = account.get_local_folder(remotefolder)
 
         #Filtered folders on the remote side will not invoke this
         #function, but we need to NOOP if the local folder is filtered
         #out too:
-        if not localfolder.sync_this:
-            ui.debug('', "Not syncing filtered local folder '%s'" \
-                         % localfolder)
+        if not localfolder.sync_this_verbose():
             return
         # Write the mailboxes
         mbnames.add(account.name, localfolder.getname())
diff --git a/offlineimap/folder/Base.py b/offlineimap/folder/Base.py
index aee2215..317e0b4 100644
--- a/offlineimap/folder/Base.py
+++ b/offlineimap/folder/Base.py
@@ -31,8 +31,7 @@ class BaseFolder(object):
         :para name: Path & name of folder minus root or reference
         :para repository: Repository() in which the folder is.
         """
-        self.sync_this = True
-        """Should this folder be included in syncing?"""
+        self._sync_this = repository.should_sync_folder(name)
         self.ui = getglobalui()
         # Top level dir name is always ''
         self.name = name if not name == self.getsep() else ''
@@ -45,6 +44,20 @@ class BaseFolder(object):
             self.visiblename = ''
         self.config = repository.getconfig()
 
+    @property
+    def sync_this(self):
+        return self._sync_this
+
+    def sync_this_verbose(self):
+        if not self.sync_this:
+            locality = 'local' \
+                if self.repository == self.repository.account.localrepos \
+                else 'remote'
+            self.ui.debug(
+                '', "Not syncing filtered %s folder '%s'[%s]"
+                % (locality, self, self.repository))
+        return self.sync_this
+
     def getname(self):
         """Returns name"""
         return self.name
diff --git a/offlineimap/repository/Base.py b/offlineimap/repository/Base.py
index 81050b0..4db85e3 100644
--- a/offlineimap/repository/Base.py
+++ b/offlineimap/repository/Base.py
@@ -146,6 +146,9 @@ class BaseRepository(CustomConfig.ConfigHelperMixin, object):
     def getfolder(self, foldername):
         raise NotImplementedError
 
+    def should_sync_folder(self, foldername):
+        return foldername in self.folderincludes or self.folderfilter(foldername)
+
     def sync_folder_structure(self, dst_repo, status_repo):
         """Syncs the folders in this repository to those in dest.
 
@@ -201,7 +204,7 @@ class BaseRepository(CustomConfig.ConfigHelperMixin, object):
                 # Does nametrans back&forth lead to identical names?
                 # 1) would src repo filter out the new folder name? In this
                 # case don't create it on it:
-                if not self.folderfilter(dst_name_t):
+                if not self.should_sync_folder(dst_name_t):
                     self.ui.debug('', "Not creating folder '%s' (repository '%s"
                         "') as it would be filtered out on that repository." %
                                   (dst_name_t, self))
diff --git a/offlineimap/repository/IMAP.py b/offlineimap/repository/IMAP.py
index 5ad787a..be8c858 100644
--- a/offlineimap/repository/IMAP.py
+++ b/offlineimap/repository/IMAP.py
@@ -287,11 +287,6 @@ class IMAPRepository(BaseRepository):
             foldername = imaputil.dequote(name)
             retval.append(self.getfoldertype()(self.imapserver, foldername,
                                                self))
-            # filter out the folder?
-            if not self.folderfilter(foldername):
-                self.ui.debug('imap', "Filtering out '%s'[%s] due to folderfilt"
-                              "er" % (foldername, self))
-                retval[-1].sync_this = False
         # Add all folderincludes
         if len(self.folderincludes):
             imapobj = self.imapserver.acquireconnection()
diff --git a/offlineimap/repository/Maildir.py b/offlineimap/repository/Maildir.py
index f197002..f2d5581 100644
--- a/offlineimap/repository/Maildir.py
+++ b/offlineimap/repository/Maildir.py
@@ -181,11 +181,6 @@ class MaildirRepository(BaseRepository):
                                                            foldername,
                                                            self.getsep(),
                                                            self))
-                # filter out the folder?
-                if not self.folderfilter(foldername):
-                    self.debug("Filtering out '%s'[%s] due to folderfilt"
-                               "er" % (foldername, self))
-                    retval[-1].sync_this = False
 
             if self.getsep() == '/' and dirname != '':
                 # Recursively check sub-directories for folders too.
-- 
1.7.11.1




More information about the OfflineIMAP-project mailing list