[PATCH v2] avoid removing of data when user removed a maildir

Nicolas Sebrecht nicolas.s-dev at laposte.net
Wed Jun 22 23:13:07 UTC 2016


When a maildir is removed it must be considered new for the sync. However, the
local cache of the folder remains. This means the sync of the folder removes all
the missing emails.

Avoid loosing of data for users not aware of the local cache by removing any
pre-existing status cache of a folder when we actually want to create the
database.

Improve style.

Github-fix: https://github.com/OfflineIMAP/offlineimap/issues/333
Signed-off-by: Nicolas Sebrecht <nicolas.s-dev at laposte.net>
---

-- interdiff --
  diff --git a/offlineimap/folder/LocalStatus.py b/offlineimap/folder/LocalStatus.py
  index fa882a0..356918f 100644
  --- a/offlineimap/folder/LocalStatus.py
  +++ b/offlineimap/folder/LocalStatus.py
  @@ -166,8 +166,9 @@ class LocalStatusFolder(BaseFolder):
   
           try:
               os.unlink(self.filename)
  -        except OSError:
  -            pass
  +        except OSError as e:
  +            self.ui.debug('', "could not remove file %s: %s"%
  +                (self.filename, e))
   
       def save(self):
           """Save changed data to disk. For this backend it is the same as saveall."""
  diff --git a/offlineimap/folder/LocalStatusSQLite.py b/offlineimap/folder/LocalStatusSQLite.py
  index 94656ea..1806796 100644
  --- a/offlineimap/folder/LocalStatusSQLite.py
  +++ b/offlineimap/folder/LocalStatusSQLite.py
  @@ -93,8 +93,9 @@ class LocalStatusSQLiteFolder(BaseFolder):
   
           try:
               os.unlink(self.filename)
  -        except OSError:
  -            pass
  +        except OSError as e:
  +            self.ui.debug('', "could not remove file %s: %s"%
  +                (self.filename, e))
   
       def storesmessages(self):
           return False
-- /interdiff --

The following changes since commit 52120beb27a718dbec3d88a00b3448fcb5137777:

  man: offlineimapui: minor typo fix (2016-06-19 23:32:01 +0200)

are available in the git repository at:

  https://github.com/nicolas33/offlineimap.git ns/purge-status-2

for you to fetch changes up to d4f5f186442ee4d24052e011e07ad5e55e84b726:

  avoid removing of data when user removed a maildir (2016-06-23 01:08:26 +0200)

----------------------------------------------------------------

 offlineimap/accounts.py                 | 47 ++++++++++++++++-----------------
 offlineimap/folder/LocalStatus.py       | 18 +++++++++----
 offlineimap/folder/LocalStatusSQLite.py |  8 ++++++
 offlineimap/repository/Base.py          | 20 +++++++-------
 offlineimap/repository/LocalStatus.py   | 16 ++++++-----
 5 files changed, 65 insertions(+), 44 deletions(-)

diff --git a/offlineimap/accounts.py b/offlineimap/accounts.py
index 734078f..b24d6c9 100644
--- a/offlineimap/accounts.py
+++ b/offlineimap/accounts.py
@@ -83,10 +83,9 @@ class Account(CustomConfig.ConfigHelperMixin):
         self.name = name
         self.metadatadir = config.getmetadatadir()
         self.localeval = config.getlocaleval()
-        # current :mod:`offlineimap.ui`, can be used for logging:
+        # Current :mod:`offlineimap.ui`, can be used for logging:
         self.ui = getglobalui()
         self.refreshperiod = self.getconffloat('autorefresh', 0.0)
-        # should we run in "dry-run" mode?
         self.dryrun = self.config.getboolean('general', 'dry-run')
         self.quicknum = 0
         if self.refreshperiod == 0.0:
@@ -262,7 +261,7 @@ class SyncableAccount(Account):
                 raise
             return
 
-        # Loop account sync if needed (bail out after 3 failures)
+        # Loop account sync if needed (bail out after 3 failures).
         looping = 3
         while looping:
             self.ui.acct(self)
@@ -329,30 +328,30 @@ class SyncableAccount(Account):
             localrepos = self.localrepos
             statusrepos = self.statusrepos
 
-            # init repos with list of folders, so we have them (and the
-            # folder delimiter etc)
+            # Init repos with list of folders, so we have them (and the
+            # folder delimiter etc).
             remoterepos.getfolders()
             localrepos.getfolders()
 
             remoterepos.sync_folder_structure(localrepos, statusrepos)
-            # replicate the folderstructure between REMOTE to LOCAL
+            # Replicate the folderstructure between REMOTE to LOCAL.
             if not localrepos.getconfboolean('readonly', False):
                 self.ui.syncfolders(remoterepos, localrepos)
 
-            # iterate through all folders on the remote repo and sync
+            # Iterate through all folders on the remote repo and sync.
             for remotefolder in remoterepos.getfolders():
-                # check for CTRL-C or SIGTERM
+                # 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 folder '%s'"
                                   "[%s]"% (remotefolder, remoterepos))
-                    continue # Ignore filtered folder
+                    continue # Ignore filtered folder.
                 localfolder = self.get_local_folder(remotefolder)
                 if not localfolder.sync_this:
                     self.ui.debug('', "Not syncing filtered folder '%s'"
                                  "[%s]"% (localfolder, localfolder.repository))
-                    continue # Ignore filtered folder
+                    continue # Ignore filtered folder.
                 if not globals.options.singlethreading:
                     thread = InstanceLimitedThread(
                         limitNamespace = "%s%s"% (
@@ -366,22 +365,22 @@ class SyncableAccount(Account):
                     folderthreads.append(thread)
                 else:
                     syncfolder(self, remotefolder, quick)
-            # wait for all threads to finish
+            # Wait for all threads to finish.
             for thr in folderthreads:
                 thr.join()
-            # Write out mailbox names if required and not in dry-run mode
+            # Write out mailbox names if required and not in dry-run mode.
             if not self.dryrun:
                 mbnames.write(False)
             localrepos.forgetfolders()
             remoterepos.forgetfolders()
         except:
-            #error while syncing. Drop all connections that we have, they
-            #might be bogus by now (e.g. after suspend)
+            # Error while syncing. Drop all connections that we have, they
+            # might be bogus by now (e.g. after suspend).
             localrepos.dropconnections()
             remoterepos.dropconnections()
             raise
         else:
-            # sync went fine. Hold or drop depending on config
+            # Sync went fine. Hold or drop depending on config.
             localrepos.holdordropconnections()
             remoterepos.holdordropconnections()
 
@@ -389,14 +388,14 @@ class SyncableAccount(Account):
         self.callhook(hook)
 
     def callhook(self, cmd):
-        # check for CTRL-C or SIGTERM and run postsynchook
+        # Check for CTRL-C or SIGTERM and run postsynchook.
         if Account.abort_NOW_signal.is_set():
             return
         if not cmd:
             return
         try:
             self.ui.callhook("Calling hook: " + cmd)
-            if self.dryrun: # don't if we are in dry-run mode
+            if self.dryrun:
                 return
             p = Popen(cmd, shell=True,
                       stdin=PIPE, stdout=PIPE, stderr=PIPE,
@@ -430,7 +429,7 @@ def syncfolder(account, remotefolder, quick):
                 localrepos.restore_atime()
                 return
         else:
-            # Both folders empty, just save new UIDVALIDITY
+            # Both folders empty, just save new UIDVALIDITY.
             localfolder.save_uidvalidity()
             remotefolder.save_uidvalidity()
 
@@ -445,10 +444,10 @@ def syncfolder(account, remotefolder, quick):
 
         localfolder.cachemessagelist(min_date=date)
         check_uid_validity(localfolder, remotefolder, statusfolder)
-        # local messagelist had date restriction applied already. Restrict
+        # Local messagelist had date restriction applied already. Restrict
         # sync to messages with UIDs >= min_uid from this list.
         #
-        # local messagelist might contain new messages (with uid's < 0).
+        # Local messagelist might contain new messages (with uid's < 0).
         positive_uids = [uid for uid in localfolder.getmessageuidlist() if uid > 0]
         if len(positive_uids) > 0:
             remotefolder.cachemessagelist(min_uid=min(positive_uids))
@@ -491,7 +490,7 @@ def syncfolder(account, remotefolder, quick):
                 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
+                # remote counterparts.
                 positive_uids = [uid for uid in list(partial.messagelist.keys()) if uid > 0]
                 if len(positive_uids) > 0:
                     min_uid = min(positive_uids)
@@ -512,7 +511,7 @@ def syncfolder(account, remotefolder, quick):
         # Load local folder.
         localfolder = account.get_local_folder(remotefolder)
 
-        # Write the mailboxes
+        # Write the mailboxes.
         mbnames.add(account.name, localfolder.getname(),
             localrepos.getlocalroot())
 
@@ -535,7 +534,7 @@ def syncfolder(account, remotefolder, quick):
         ui.syncingfolder(remoterepos, remotefolder, localrepos, localfolder)
 
         # Retrieve messagelists, taking into account age-restriction
-        # options
+        # options.
         maxage = localfolder.getmaxage()
         localstart = localfolder.getstartdate()
         remotestart = remotefolder.getstartdate()
@@ -592,7 +591,7 @@ def syncfolder(account, remotefolder, quick):
     except (KeyboardInterrupt, SystemExit):
         raise
     except OfflineImapError as e:
-        # bubble up severe Errors, skip folder otherwise
+        # Bubble up severe Errors, skip folder otherwise.
         if e.severity > OfflineImapError.ERROR.FOLDER:
             raise
         else:
diff --git a/offlineimap/folder/LocalStatus.py b/offlineimap/folder/LocalStatus.py
index 31b04dd..356918f 100644
--- a/offlineimap/folder/LocalStatus.py
+++ b/offlineimap/folder/LocalStatus.py
@@ -1,5 +1,5 @@
 # Local status cache virtual folder
-# Copyright (C) 2002-2015 John Goerzen & contributors
+# Copyright (C) 2002-2016 John Goerzen & contributors
 #
 #    This program is free software; you can redistribute it and/or modify
 #    it under the terms of the GNU General Public License as published by
@@ -18,11 +18,10 @@
 from sys import exc_info
 import os
 import threading
+import six
 
 from .Base import BaseFolder
 
-import six
-
 
 class LocalStatusFolder(BaseFolder):
     """LocalStatus backend implemented as a plain text file."""
@@ -73,8 +72,8 @@ class LocalStatusFolder(BaseFolder):
                 uid = int(uid)
                 flags = set(flags)
             except ValueError as e:
-                errstr = "Corrupt line '%s' in cache file '%s'" % \
-                    (line, self.filename)
+                errstr = ("Corrupt line '%s' in cache file '%s'"%
+                    (line, self.filename))
                 self.ui.warn(errstr)
                 six.reraise(ValueError(errstr), None, exc_info()[2])
             self.messagelist[uid] = self.msglist_item_initializer(uid)
@@ -162,6 +161,15 @@ class LocalStatusFolder(BaseFolder):
     def closefiles(self):
         pass # Closing files is done on a per-transaction basis.
 
+    def purge(self):
+        """Remove any pre-existing database."""
+
+        try:
+            os.unlink(self.filename)
+        except OSError as e:
+            self.ui.debug('', "could not remove file %s: %s"%
+                (self.filename, e))
+
     def save(self):
         """Save changed data to disk. For this backend it is the same as saveall."""
 
diff --git a/offlineimap/folder/LocalStatusSQLite.py b/offlineimap/folder/LocalStatusSQLite.py
index 762e432..1806796 100644
--- a/offlineimap/folder/LocalStatusSQLite.py
+++ b/offlineimap/folder/LocalStatusSQLite.py
@@ -88,6 +88,14 @@ class LocalStatusSQLiteFolder(BaseFolder):
             if version < LocalStatusSQLiteFolder.cur_version:
                 self.__upgrade_db(version)
 
+    def purge(self):
+        """Remove any pre-existing database."""
+
+        try:
+            os.unlink(self.filename)
+        except OSError as e:
+            self.ui.debug('', "could not remove file %s: %s"%
+                (self.filename, e))
 
     def storesmessages(self):
         return False
diff --git a/offlineimap/repository/Base.py b/offlineimap/repository/Base.py
index 8e714ca..e453f7e 100644
--- a/offlineimap/repository/Base.py
+++ b/offlineimap/repository/Base.py
@@ -1,5 +1,5 @@
 # Base repository support
-# Copyright (C) 2002-2015 John Goerzen & contributors
+# Copyright (C) 2002-2016 John Goerzen & contributors
 #
 #    This program is free software; you can redistribute it and/or modify
 #    it under the terms of the GNU General Public License as published by
@@ -222,12 +222,13 @@ class BaseRepository(CustomConfig.ConfigHelperMixin, object):
                         "') as it would be filtered out on that repository."%
                         (dst_name_t, self))
                     continue
-                # get IMAPFolder and see if the reverse nametrans
+                # Get IMAPFolder and see if the reverse nametrans
                 # works fine TODO: getfolder() works only because we
                 # succeed in getting inexisting folders which I would
                 # like to change. Take care!
                 folder = self.getfolder(dst_name_t)
-                # apply reverse nametrans to see if we end up with the same name
+                # Apply reverse nametrans to see if we end up with the same
+                # name.
                 newdst_name = folder.getvisiblename().replace(
                     src_repo.getsep(), dst_repo.getsep())
                 if dst_folder.name != newdst_name:
@@ -239,13 +240,14 @@ class BaseRepository(CustomConfig.ConfigHelperMixin, object):
                         "itories so they lead to identical names if applied bac"
                         "k and forth. 2) Use folderfilter settings on a reposit"
                         "ory to prevent some folders from being created on the "
-                        "other side." % (dst_folder.name, dst_repo, dst_name_t,
-                                         src_repo, newdst_name),
-                                           OfflineImapError.ERROR.REPO)
-                # end sanity check, actually create the folder
+                        "other side."%
+                        (dst_folder.name, dst_repo, dst_name_t,
+                            src_repo, newdst_name),
+                        OfflineImapError.ERROR.REPO)
+                # End sanity check, actually create the folder.
                 try:
                     src_repo.makefolder(dst_name_t)
-                    src_haschanged = True # Need to refresh list
+                    src_haschanged = True # Need to refresh list.
                 except OfflineImapError as e:
                     self.ui.error(e, exc_info()[2], "Creating folder %s on "
                                   "repository %s" % (dst_name_t, src_repo))
@@ -255,7 +257,7 @@ class BaseRepository(CustomConfig.ConfigHelperMixin, object):
         # Find deleted folders.
         # TODO: We don't delete folders right now.
 
-        #Forget old list of cached folders so we get new ones if needed
+        # Forget old list of cached folders so we get new ones if needed.
         if src_haschanged:
             self.forgetfolders()
         if dst_haschanged:
diff --git a/offlineimap/repository/LocalStatus.py b/offlineimap/repository/LocalStatus.py
index 25fadc0..becbda2 100644
--- a/offlineimap/repository/LocalStatus.py
+++ b/offlineimap/repository/LocalStatus.py
@@ -1,5 +1,5 @@
 # Local status cache repository support
-# Copyright (C) 2002-2015 John Goerzen & contributors
+# Copyright (C) 2002-2016 John Goerzen & contributors
 #
 #    This program is free software; you can redistribute it and/or modify
 #    it under the terms of the GNU General Public License as published by
@@ -61,15 +61,15 @@ class LocalStatusRepository(BaseRepository):
 
     def import_other_backend(self, folder):
         for bk, dic in self.backends.items():
-            # skip folder's own type
+            # Skip folder's own type.
             if dic['class'] == type(folder):
                 continue
 
             repobk = LocalStatusRepository(self.name, self.account)
-            repobk.setup_backend(bk)      # fake the backend
+            repobk.setup_backend(bk)      # Fake the backend.
             folderbk = dic['class'](folder.name, repobk)
 
-            # if backend contains data, import it to folder.
+            # If backend contains data, import it to folder.
             if not folderbk.isnewfolder():
                 self.ui._msg("Migrating LocalStatus cache from %s to %s "
                     "status folder for %s:%s"%
@@ -87,10 +87,14 @@ class LocalStatusRepository(BaseRepository):
         """Create a LocalStatus Folder."""
 
         if self.account.dryrun:
-            return # bail out in dry-run mode
+            return # Bail out in dry-run mode.
 
-        # Create an empty StatusFolder
+        # Create an empty StatusFolder.
         folder = self._instanciatefolder(foldername)
+        # First delete any existing data to make sure we won't consider obsolete
+        # data. This might happen if the user removed the folder (maildir) and
+        # it is re-created afterwards.
+        folder.purge()
         folder.save()
         folder.closefiles()
 
-- 
2.7.4



More information about the OfflineIMAP-project mailing list