[PATCH 3/3] Revamp Syncing strategy.

Sebastian Spaeth Sebastian at SSpaeth.de
Thu Jan 20 12:16:53 UTC 2011


The previous syncing strategy was a bit intrasparent and confusing. This
is an attempt to clean it up. Changes 1-3 are concerned
with changes in accounts.py. The rest is implemented in folder/Base.py.

1) Do away with the previous different code paths depending on
whether there is a LocalStatus file or not (the isnewfolder() test). We
always use the same strategy now.

This strategy is simply:
a) Sync remote to local folder first
b) Sync local to remote

Where each sync implies a 4 pass strategy as before (explained below).

2) Don't delete messages on LOCAL which don't exist on REMOTE right at
the beginning. This prevented us from keeping local messages rather than
redownloading everything once LocalStatus got corrupted or deleted.

3) No need to separately sync the statusfolder anymore, we update that
one simultanously with the destfolders...

3) Simplified the sync functions by only taking one destdir, we
never used more anyway. This makes the code better to understand.

4) Added plenty of code comments while I was going through to make sure it
is easy to understand.

A folder sync strategy is mostly like it was before:
        Pass1: Transfer new local messages
               Upload msg with negative/no UIDs to dstfolder. dstfolder
               should assign that message a new UID. Update statusfolder.
        Pass2: Copy existing messages
               Copy messages in self, but not statusfolder to dstfolder
               if not already in dstfolder. Update statusfolder.
        Pass3: Remove deleted messages
               Get all UIDS in statusfolder but not self. These are
               messages that we have locally deleted. Delete those from
               dstfolder and statusfolder.
        Pass4: Synchronize flag changes
               Compare flags in self with those in statusfolder. If msg
               has a valid UID and exists on dstfolder (has not e.g.
               been deleted there), sync the flag change to dstfolder
               and statusfolder.

The user visible implications of this change should be limited:
 - Blowing away LocalStatus will not require you to redownload ALL of
 your mails if you still have the local Maildir. It will simply recreate
 LocalStatus.

Signed-off-by: Sebastian Spaeth <Sebastian at SSpaeth.de>
---
 offlineimap/accounts.py    |   25 +---
 offlineimap/folder/Base.py |  322 ++++++++++++++++++++++++--------------------
 2 files changed, 181 insertions(+), 166 deletions(-)

diff --git a/offlineimap/accounts.py b/offlineimap/accounts.py
index 667d2b2..2bd3666 100644
--- a/offlineimap/accounts.py
+++ b/offlineimap/accounts.py
@@ -345,29 +345,14 @@ def syncfolder(accountname, remoterepos, remotefolder, localrepos,
         ui.messagelistloaded(remoterepos, remotefolder,
                              len(remotefolder.getmessagelist().keys()))
 
-
-        #
-
-        if not statusfolder.isnewfolder():
-            # Delete local copies of remote messages.  This way,
-            # if a message's flag is modified locally but it has been
-            # deleted remotely, we'll delete it locally.  Otherwise, we
-            # try to modify a deleted message's flags!  This step
-            # need only be taken if a statusfolder is present; otherwise,
-            # there is no action taken *to* the remote repository.
-
-            remotefolder.syncmessagesto_delete(localfolder, [localfolder,
-                                                             statusfolder])
-            ui.syncingmessages(localrepos, localfolder, remoterepos, remotefolder)
-            localfolder.syncmessagesto(statusfolder, [remotefolder, statusfolder])
-
         # Synchronize remote changes.
         ui.syncingmessages(remoterepos, remotefolder, localrepos, localfolder)
-        remotefolder.syncmessagesto(localfolder, [localfolder, statusfolder])
+        remotefolder.syncmessagesto(localfolder, statusfolder)
+        # Synchronize local changes
+        ui.syncingmessages(localrepos, localfolder, remoterepos, remotefolder)
+        localfolder.syncmessagesto(remotefolder, statusfolder)
+
 
-        # Make sure the status folder is up-to-date.
-        ui.syncingmessages(localrepos, localfolder, statusrepos, statusfolder)
-        localfolder.syncmessagesto(statusfolder)
         statusfolder.save()
         localrepos.restore_atime()
     except:
diff --git a/offlineimap/folder/Base.py b/offlineimap/folder/Base.py
index f4c80a0..91524ba 100644
--- a/offlineimap/folder/Base.py
+++ b/offlineimap/folder/Base.py
@@ -215,68 +215,89 @@ class BaseFolder:
         for uid in uidlist:
             self.deletemessage(uid)
 
-    def syncmessagesto_neguid_msg(self, uid, dest, applyto, register = 1):
-        if register:
-            UIBase.getglobalui().registerthread(self.getaccountname())
-        UIBase.getglobalui().copyingmessage(uid, self, applyto)
+    def syncmessagesto_neguid_msg(self, uid, dstfolder, statusfolder,
+                                  register = 1):
+        """Copy a single message from self to dests.
+
+        This is called by meth:`syncmessagesto_neguid`, possibly in a
+        new thread. It does not return anything.
+
+        :param dstfolder: A BaseFolder-derived instance
+        :param statusfolder: A LocalStatusFolder instance
+        :param register: If True, output that a new thread was created
+                         (and register it with the ui)."""
         successobject = None
         successuid = None
+
+        if register:
+            UIBase.getglobalui().registerthread(self.getaccountname())
+        UIBase.getglobalui().copyingmessage(uid, self, [dstfolder])
+
         message = self.getmessage(uid)
         flags = self.getmessageflags(uid)
         rtime = self.getmessagetime(uid)
-        for tryappend in applyto:
-            successuid = tryappend.savemessage(uid, message, flags, rtime)
-            if successuid >= 0:
-                successobject = tryappend
-                break
-        # Did we succeed?
-        if successobject != None:
-            if successuid:       # Only if IMAP actually assigned a UID
-                # Copy the message to the other remote servers.
-                for appendserver in \
-                        [x for x in applyto if x != successobject]:
-                    appendserver.savemessage(successuid, message, flags, rtime)
-                    # Copy to its new name on the local server and delete
-                    # the one without a UID.
-                    self.savemessage(successuid, message, flags, rtime)
-            self.deletemessage(uid) # It'll be re-downloaded.
-        else:
-            # Did not find any server to take this message.  Ignore.
-            pass
+
+        #Save messages to dstfolder and see if a valid UID was returned
+        successuid = dstfolder.savemessage(uid, message, flags, rtime)
+
+        #Succeeded? -> IMAP actually assigned a UID
+        #If successuid remained negative, no server was willing to assign us
+        #an UID. Ignore message.
+        if successuid >= 0:
+            # Copy the message to the statusfolder
+            statusfolder.savemessage(successuid, message, flags, rtime)
+            # Copy to its new name son the local server and delete
+            # the one without a UID.
+            self.savemessage(successuid, message, flags, rtime)
+            #TODO: above means, we read in the message and write it out
+            #to the very same dir with a different UID. Investigate if we
+            #cannot simply rename it.
+            
+            # delete the negative uid message. We have it with a good UID now.
+            self.deletemessage(uid)
         
 
-    def syncmessagesto_neguid(self, dest, applyto):
+    def syncmessagesto_neguid(self, dstfolder, statusfolder):
         """Pass 1 of folder synchronization.
 
-        Look for messages in self with a negative uid.  These are messages in
-        Maildirs that were not added by us.  Try to add them to the dests,
-        and once that succeeds, get the UID, add it to the others for real,
-        add it to local for real, and delete the fake one."""
+        Look for messages in self with a negative uid.  These are
+        messages in Maildirs that were not added by us.  Try to add
+        them to the dstfolder, and once that succeeds, get the UID,
+        add it to the statusfolder for real, add it to local for real,
+        and delete the fake one.
+
+        :param dstfolder: A BaseFolder-derived instance
+        :param statusfolder: A LocalStatusFolder instance"""
 
         uidlist = [uid for uid in self.getmessagelist().keys() if uid < 0]
         threads = []
 
-        usethread = None
-        if applyto != None:
-            usethread = applyto[0]
-        
         for uid in uidlist:
-            if usethread and usethread.suggeststhreads():
-                usethread.waitforthread()
+            if dstfolder.suggeststhreads():
+                dstfolder.waitforthread()
                 thread = InstanceLimitedThread(\
-                    usethread.getcopyinstancelimit(),
+                    dstfolder.getcopyinstancelimit(),
                     target = self.syncmessagesto_neguid_msg,
                     name = "New msg sync from %s" % self.getvisiblename(),
-                    args = (uid, dest, applyto))
+                    args = (uid, dstfolder, statusfolder))
                 thread.setDaemon(1)
                 thread.start()
                 threads.append(thread)
             else:
-                self.syncmessagesto_neguid_msg(uid, dest, applyto, register = 0)
+                self.syncmessagesto_neguid_msg(uid, dstfolder, statusfolder,
+                                               register = 0)
+        #wait for all uploads to finish
         for thread in threads:
             thread.join()
 
-    def copymessageto(self, uid, applyto, register = 1):
+    def copymessageto(self, uid, dstfolder, statusfolder, register = 1):
+        """Copies a message from self to dst if needed, updating the status
+
+        :param uid: uid of the message to be copied.
+        :param dstfolder: A BaseFolder-derived instance
+        :param statusfolder: A LocalStatusFolder instance
+        :param register: whether we should register a new thread."
+        :returns: Nothing on success, or raises an Exception."""
         # Sometimes, it could be the case that if a sync takes awhile,
         # a message might be deleted from the maildir before it can be
         # synced to the status cache.  This is only a problem with
@@ -285,79 +306,86 @@ class BaseFolder:
         try:
             if register:
                 UIBase.getglobalui().registerthread(self.getaccountname())
-            UIBase.getglobalui().copyingmessage(uid, self, applyto)
-            message = ''
-            # If any of the destinations actually stores the message body,
-            # load it up.
-            
-            for object in applyto:
-                if object.storesmessages():
-                    message = self.getmessage(uid)
-                    break
+
+            message = None
             flags = self.getmessageflags(uid)
             rtime = self.getmessagetime(uid)
-            for object in applyto:
-                newuid = object.savemessage(uid, message, flags, rtime)
-                if newuid > 0 and newuid != uid:
-                    # Change the local uid.
-                    self.savemessage(newuid, message, flags, rtime)
-                    self.deletemessage(uid)
-                    uid = newuid
-        except:
-            UIBase.getglobalui().warn("ERROR attempting to copy message " + str(uid) \
-                 + " for account " + self.getaccountname() + ":" + str(sys.exc_info()[1]))
-        
+            
+            if dstfolder.uidexists(uid):
+                # dst already has a message with that UID, so only update status
+                statusfolder.savemessage(uid, None, flags, rtime)
+                return
 
-    def syncmessagesto_copy(self, dest, applyto):
-        """Pass 2 of folder synchronization.
+            # really need to copy to dst...
+            UIBase.getglobalui().copyingmessage(uid, self, [dstfolder])
 
-        Look for messages present in self but not in dest.  If any, add
-        them to dest."""
+            # If any of the destinations actually stores the message body,
+            # load it up.            
+            if dstfolder.storesmessages():
+                message = self.getmessage(uid)
+
+            newuid = dstfolder.savemessage(uid, message, flags, rtime)
+            if newuid > 0 and newuid != uid:
+                # Change the local uid.
+                self.savemessage(newuid, message, flags, rtime)
+                self.deletemessage(uid)
+                uid = newuid
+            statusfolder.savemessage(uid, message, flags, rtime)
+        except:
+            UIBase.getglobalui().warn("ERROR attempting to copy message " + \
+                 str(uid) + " for account " + self.getaccountname() + \
+                 ":" + str(sys.exc_info()[1]))
+            raise
+
+    def syncmessagesto_copy(self, dstfolder, statusfolder):
+        """Pass 2 of folder synchronization. Copy messages in `self` but not `dst`
+
+        1) Look for messages present in self but not in statusfolder.
+        2) invoke copymessageto() on those which:
+            - Updates statusfolder
+            - If dstfolder doesn't have it yet, add them to dstfolder."""
         threads = []
-        
-	dest_messagelist = dest.getmessagelist()
-        for uid in self.getmessagelist().keys():
-            if uid < 0:                 # Ignore messages that pass 1 missed.
-                continue
-            if not uid in dest_messagelist:
-                if self.suggeststhreads():
-                    self.waitforthread()
-                    thread = InstanceLimitedThread(\
-                        self.getcopyinstancelimit(),
-                        target = self.copymessageto,
-                        name = "Copy message %d from %s" % (uid,
-                                                            self.getvisiblename()),
-                        args = (uid, applyto))
-                    thread.setDaemon(1)
-                    thread.start()
-                    threads.append(thread)
-                else:
-                    self.copymessageto(uid, applyto, register = 0)
+
+        copylist = filter(lambda uid: uid>=0 and not \
+                              statusfolder.uidexists(uid),
+                            self.getmessageuidlist())
+        for uid in copylist:
+            if self.suggeststhreads():
+                self.waitforthread()
+                thread = InstanceLimitedThread(\
+                    self.getcopyinstancelimit(),
+                    target = self.copymessageto,
+                    name = "Copy message %d from %s" % (uid,
+                                                        self.getvisiblename()),
+                    args = (uid, dstfolder, statusfolder))
+                thread.setDaemon(1)
+                thread.start()
+                threads.append(thread)
+            else:
+                self.copymessageto(uid, dstfolder, statusfolder, register = 0)
+
         for thread in threads:
             thread.join()
 
-    def syncmessagesto_delete(self, dest, applyto):
+    def syncmessagesto_delete(self, dstfolder, statusfolder):
         """Pass 3 of folder synchronization.
 
-        Look for message present in dest but not in self.
-        If any, delete them."""
-        deletelist = []
-	self_messagelist = self.getmessagelist()
-        for uid in dest.getmessagelist().keys():
-            if uid < 0:
-                continue
-            if not uid in self_messagelist:
-                deletelist.append(uid)
+        Look for message present in statusfolder but not in self.
+        If any, delete them from dstfolder and statusfolder."""
+        deletelist = filter(lambda uid: uid>=0 and not self.uidexists(uid),
+                            statusfolder.getmessageuidlist())
         if len(deletelist):
-            UIBase.getglobalui().deletingmessages(deletelist, applyto)
-            for object in applyto:
-                object.deletemessages(deletelist)
+            UIBase.getglobalui().deletingmessages(deletelist, [dstfolder])
+            # delete in statusfolder first to play safe. In case of abort, we
+            # won't lose message, we will just retransmit some unneccessary
+            for folder in [statusfolder, dstfolder]:
+                folder.deletemessages(deletelist)
 
-    def syncmessagesto_flags(self, dest, applyto):
+    def syncmessagesto_flags(self, dstfolder, statusfolder):
         """Pass 4 of folder synchronization.
 
         Look for any flag matching issues -- set dest message to have the
-        same flags that we have."""
+        same flags that we have (if it exists)."""
 
         # As an optimization over previous versions, we store up which flags
         # are being used for an add or a delete.  For each flag, we store
@@ -368,67 +396,69 @@ class BaseFolder:
 
         addflaglist = {}
         delflaglist = {}
-        
-        for uid in self.getmessagelist().keys():
-            if uid < 0:                 # Ignore messages missed by pass 1
+        for uid in self.getmessageuidlist():
+            # Ignore messages with negative UIDs missed by pass 1
+            # also don't do anything if the message has been deleted remotely
+            if uid < 0 or not dstfolder.uidexists(uid):
                 continue
-            selfflags = self.getmessageflags(uid)
-            destflags = dest.getmessageflags(uid)
 
-            addflags = [x for x in selfflags if x not in destflags]
+            selfflags = self.getmessageflags(uid)
+            statusflags = statusfolder.getmessageflags(uid)
+            #if we could not get message flags from LocalStatus, assume empty.
+            if statusflags is None:
+                statusflags = []
+            addflags = [x for x in selfflags if x not in statusflags]
 
             for flag in addflags:
                 if not flag in addflaglist:
                     addflaglist[flag] = []
                 addflaglist[flag].append(uid)
 
-            delflags = [x for x in destflags if x not in selfflags]
+            delflags = [x for x in statusflags if x not in selfflags]
             for flag in delflags:
                 if not flag in delflaglist:
                     delflaglist[flag] = []
                 delflaglist[flag].append(uid)
 
-        for object in applyto:
-            for flag in addflaglist.keys():
-                UIBase.getglobalui().addingflags(addflaglist[flag], flag, [object])
-                object.addmessagesflags(addflaglist[flag], [flag])
-            for flag in delflaglist.keys():
-                UIBase.getglobalui().deletingflags(delflaglist[flag], flag, [object])
-                object.deletemessagesflags(delflaglist[flag], [flag])
-                
-    def syncmessagesto(self, dest, applyto = None):
-        """Syncs messages in this folder to the destination.
-        If applyto is specified, it should be a list of folders (don't forget
-        to include dest!) to which all write actions should be applied.
-        It defaults to [dest] if not specified.  It is important that
-        the UID generator be listed first in applyto; that is, the other
-        applyto ones should be the ones that "copy" the main action."""
-        if applyto == None:
-            applyto = [dest]
+        for flag in addflaglist.keys():
+            UIBase.getglobalui().addingflags(addflaglist[flag], flag, [dstfolder])
+            dstfolder.addmessagesflags(addflaglist[flag], [flag])
+            statusfolder.addmessagesflags(addflaglist[flag], [flag])
 
-        try:
-            self.syncmessagesto_neguid(dest, applyto)
-        except:
-            UIBase.getglobalui().warn("ERROR attempting to handle negative uids " \
-                + "for account " + self.getaccountname() + ":" + str(sys.exc_info()[1]))
-
-        #all threads launched here are in try / except clauses when they copy anyway...
-        self.syncmessagesto_copy(dest, applyto)
-
-        try:
-            self.syncmessagesto_delete(dest, applyto)
-        except:
-            UIBase.getglobalui().warn("ERROR attempting to delete messages " \
-                + "for account " + self.getaccountname() + ":" + str(sys.exc_info()[1]))
-
-        # Now, the message lists should be identical wrt the uids present.
-        # (except for potential negative uids that couldn't be placed
-        # anywhere)
-
-        try:
-            self.syncmessagesto_flags(dest, applyto)
-        except:
-            UIBase.getglobalui().warn("ERROR attempting to sync flags " \
-                + "for account " + self.getaccountname() + ":" + str(sys.exc_info()[1]))
-        
-            
+        for flag in delflaglist.keys():
+            UIBase.getglobalui().deletingflags(delflaglist[flag], flag, [dstfolder])
+            dstfolder.deletemessagesflags(delflaglist[flag], [flag])
+            statusfolder.deletemessagesflags(delflaglist[flag], [flag])
+                
+    def syncmessagesto(self, dstfolder, statusfolder):
+        """Syncs messages in this folder to the destination dstfolder.
+              
+        Syncsteps are:
+
+        Pass1: Upload msg with negative/no UIDs to dstfolder. dstfolder
+               might assign that message a new UID. Update statusfolder.
+        Pass2: Copy messages in self, but not statusfolder to dstfolder
+               if not already in dstfolder. Update statusfolder.
+
+        Pass3: Delete messages in de
+          Now, the message lists should be identical wrt the uids present.
+          (except for potential negative uids that couldn't be placed
+          anywhere)
+         Pass4: Synchronize flag changes
+
+        :param dstfolder: Folderinstance to sync the msgs to.
+        :param statusfolder: LocalStatus instance to sync against.
+        """
+        ui = UIBase.getglobalui()
+        passes = [('uploading negative UIDs', self.syncmessagesto_neguid),
+                  ('copying messages'       , self.syncmessagesto_copy),
+                  ('deleting messages'      , self.syncmessagesto_delete),
+                  ('syncing flags'          , self.syncmessagesto_flags)]
+
+        for (passdesc, action) in passes:
+            try:
+                action(dstfolder, statusfolder)
+            except:
+                ui.warn("ERROR in folder sync (%s) [account %s]: %s"%\
+                        (passdesc, self.getaccountname(), sys.exc_info()[1]))
+                raise
-- 
1.7.1




More information about the OfflineIMAP-project mailing list