[PATCH 2/2] Implement change_message_uid

Sebastian Spaeth Sebastian at SSpaeth.de
Mon Aug 22 16:10:54 BST 2011


Previously, assigning a new UID to a mapped IMAP or Maildir repository
was done by loading the "local" item, saving it under a new UID and
deleting the old one. This involved lots of disk activity for nothing
more than an effective file rename in Maildirs, and lots of network
usage in the MappedUID cases.

We do this on every upload from a local to a remote item, so that can
potentially be quite expensive. This patch lets backends that support it
(Maildir, MappedUID) efficiently rename the file rather than having to
read the mail content, write it out as a new file and delete the old
file. This speeds up uploads from Maildir and the MappedUID server.

Signed-off-by: Sebastian Spaeth <Sebastian at SSpaeth.de>
---
 Changelog.draft.rst           |    3 ++
 offlineimap/folder/Base.py    |   46 +++++++++++++++++++++------------------
 offlineimap/folder/IMAP.py    |   10 +++++++-
 offlineimap/folder/Maildir.py |   47 ++++++++++++++++++++++++++--------------
 offlineimap/folder/UIDMaps.py |   25 +++++++++++++++++++++
 5 files changed, 92 insertions(+), 39 deletions(-)

diff --git a/Changelog.draft.rst b/Changelog.draft.rst
index 74b6368..d005f15 100644
--- a/Changelog.draft.rst
+++ b/Changelog.draft.rst
@@ -25,6 +25,9 @@ Changes
 * Remove the configurability of the Blinkenlights statuschar. It
   cluttered the main configuration file for little gain.
 * Updated bundled imaplib2 to version 2.28.
+* Uploading of Messages from Maildir and IMAP<->IMAP has been made more
+  efficient by renaming files/mapping entries, rather than actually
+  loading and saving the message under a new UID.
 
 Bug Fixes
 ---------
diff --git a/offlineimap/folder/Base.py b/offlineimap/folder/Base.py
index 09837ff..84c07c9 100644
--- a/offlineimap/folder/Base.py
+++ b/offlineimap/folder/Base.py
@@ -211,6 +211,15 @@ class BaseFolder(object):
     def deletemessagesflags(self, uidlist, flags):
         for uid in uidlist:
             self.deletemessageflags(uid, flags)
+            
+    def change_message_uid(self, uid, new_uid):
+        """Change the message from existing uid to new_uid
+
+        If the backend supports it (IMAP does not).       
+        :param new_uid: (optional) If given, the old UID will be changed
+            to a new UID. This allows backends efficient renaming of
+            messages if the UID has changed."""
+        raise NotImplementedException
 
     def deletemessage(self, uid):
         raise NotImplementedException
@@ -254,20 +263,15 @@ class BaseFolder(object):
             #remained negative, no server was willing to assign us an
             #UID. If newid is 0, saving succeeded, but we could not
             #retrieve the new UID. Ignore message in this case.
-            newuid = dstfolder.savemessage(uid, message, flags, rtime)
-
-            if newuid > 0:
-                if newuid != uid:
+            new_uid = dstfolder.savemessage(uid, message, flags, rtime)
+            if new_uid > 0:
+                if new_uid != uid:
+                    # Got new UID, change the local uid to match the new one.
+                    self.change_message_uid(uid, new_uid)
+                    statusfolder.deletemessage(uid)
                     # Got new UID, change the local uid.
-                    #TODO: Maildir could do this with a rename rather than
-                    #load/save/del operation, IMPLEMENT a changeuid()
-                    #function or so.
-                    self.savemessage(newuid, message, flags, rtime)
-                    self.deletemessage(uid)
-                    uid = newuid
                 # Save uploaded status in the statusfolder
-                statusfolder.savemessage(uid, message, flags, rtime)
-    
+                statusfolder.savemessage(new_uid, message, flags, rtime)
             elif newuid == 0:
                 # Message was stored to dstfolder, but we can't find it's UID
                 # This means we can't link current message to the one created
@@ -278,11 +282,11 @@ class BaseFolder(object):
                 self.deletemessage(uid)
             else:
                 raise OfflineImapError("Trying to save msg (uid %d) on folder "
-                                  "%s returned invalid uid %d" % \
-                                      (uid,
-                                       dstfolder.getvisiblename(),
-                                       newuid),
+                                       "%s returned invalid uid %d" % (uid,
+                                       dstfolder.getvisiblename(), new_uid),
                                        OfflineImapError.ERROR.MESSAGE)
+        except (KeyboardInterrupt): # bubble up CTRL-C
+            raise
         except OfflineImapError, e:
             if e.severity > OfflineImapError.ERROR.MESSAGE:
                 raise # buble severe errors up
@@ -290,10 +294,9 @@ class BaseFolder(object):
         except Exception, e:
             self.ui.error(e, "Copying message %s [acc: %s]:\n %s" %\
                               (uid, self.getaccountname(),
-                               traceback.format_exc()))
+                               exc_info()[2]))
             raise    #raise on unknown errors, so we can fix those
 
-
     def syncmessagesto_copy(self, dstfolder, statusfolder):
         """Pass1: Copy locally existing messages not on the other side
 
@@ -436,7 +439,8 @@ class BaseFolder(object):
                     raise
                 self.ui.error(e, exc_info()[2])
             except Exception, e:
-                self.ui.error(e, msg = "ERROR attempting to sync folder %s "
-                             "[acc: %s]:\n  %s" (self, self.getaccountname(),
-                                                 traceback.format_exc()))
+                self.ui.error(e, exc_info()[2],
+                              msg = "ERROR attempting to sync folder %s "
+                              "[acc: %s]:\n  %s" % (self, self.getaccountname(),
+                                                  traceback.format_exc()))
                 raise # raise unknown Exceptions so we can fix them
diff --git a/offlineimap/folder/IMAP.py b/offlineimap/folder/IMAP.py
index 40eb31c..11f828e 100644
--- a/offlineimap/folder/IMAP.py
+++ b/offlineimap/folder/IMAP.py
@@ -584,8 +584,8 @@ class IMAPFolder(BaseFolder):
         self.ui.debug('imap', 'savemessage: returning new UID %d' % uid)
         return uid
 
-
     def savemessageflags(self, uid, flags):
+        """Change a message's flags to `flags`."""
         imapobj = self.imapserver.acquireconnection()
         try:
             try:
@@ -671,6 +671,14 @@ class IMAPFolder(BaseFolder):
             elif operation == '-':
                 self.messagelist[uid]['flags'] -= flags
 
+    def change_message_uid(self, uid, new_uid):
+        """Change the message from existing uid to new_uid
+
+        If the backend supports it. IMAP does not and will throw errors.""" 
+        raise OfflineImapError('IMAP backend cannot change a messages UID from '
+                               '%d to %d' % (uid, new_uid),
+                               OfflineImapError.ERROR.MESSAGE)
+        
     def deletemessage(self, uid):
         self.deletemessages_noconvert([uid])
 
diff --git a/offlineimap/folder/Maildir.py b/offlineimap/folder/Maildir.py
index 05d5ee7..a5b447d 100644
--- a/offlineimap/folder/Maildir.py
+++ b/offlineimap/folder/Maildir.py
@@ -265,7 +265,7 @@ class MaildirFolder(BaseFolder):
              os.getpid(),
              socket.gethostname(),
              uid,
-             md5(self.getvisiblename()).hexdigest())
+             self._foldermd5)
         # open file and write it out
         try:
             fd = os.open(os.path.join(tmpdir, messagename),
@@ -301,26 +301,22 @@ class MaildirFolder(BaseFolder):
         return self.messagelist[uid]['flags']
 
     def savemessageflags(self, uid, flags):
+        """Sets the specified message's flags to the given set."""
         oldfilename = self.messagelist[uid]['filename']
-        dir_prefix, newname = os.path.split(oldfilename)
-        tmpdir = os.path.join(self.getfullname(), 'tmp')
-        if 'S' in flags:
-            # If a message has been seen, it goes into the cur
-            # directory.  CR debian#152482
-            dir_prefix = 'cur'
-        else:
-            dir_prefix = 'new'
-
+        dir_prefix, filename = os.path.split(oldfilename)
+        # If a message has been seen, it goes into 'cur'
+        dir_prefix = 'cur' if 'S' in flags else 'new'
         infostr = self.infosep
-        infomatch = re.search('(' + self.infosep + '.*)$', newname)
+        infomatch = re.search('(' + self.infosep + '.*)$', filename)
         if infomatch:                   # If the info string is present..
             infostr = infomatch.group(1)
-            newname = newname.split(self.infosep)[0] # Strip off the info string.
+            filename = filename.split(self.infosep)[0] # Strip off info string.
+
         infostr = re.sub('2,[A-Z]*', '', infostr)
         infostr += '2,' + ''.join(sorted(flags))
-        newname += infostr
+        filename += infostr
         
-        newfilename = os.path.join(dir_prefix, newname)
+        newfilename = os.path.join(dir_prefix, filename)
         if (newfilename != oldfilename):
             try:
                 os.rename(os.path.join(self.getfullname(), oldfilename),
@@ -333,10 +329,27 @@ class MaildirFolder(BaseFolder):
             self.messagelist[uid]['flags'] = flags
             self.messagelist[uid]['filename'] = newfilename
 
-        # By now, the message had better not be in tmp/ land!
-        final_dir, final_name = os.path.split(self.messagelist[uid]['filename'])
-        assert final_dir != 'tmp'
+    def change_message_uid(self, uid, new_uid):
+        """Change the message from existing uid to new_uid
 
+        This will not update the statusfolder UID, you need to do that yourself.
+        :param new_uid: (optional) If given, the old UID will be changed
+            to a new UID. The Maildir backend can implement this as an efficient rename-"""
+        if not uid in self.messagelist:
+            raise OfflineImapError("Cannot change unknown Maildir UID %s" % uid)
+        if uid == new_uid: return
+
+        oldfilename = self.messagelist[uid]['filename']
+        dir_prefix, filename = os.path.split(oldfilename)
+        prefix, old_uid, fmd5, flags = self._parse_filename(filename)
+        filename = "%s,U=%s,FMD5=%s:2,%s" % (prefix, new_uid, self._foldermd5,
+                                             flags)
+        os.rename(os.path.join(self.getfullname(), oldfilename),
+                  os.path.join(self.getfullname(), dir_prefix, filename))
+        self.messagelist[uid]['uid'] = new_uid
+        self.messagelist[new_uid] = self.messagelist[uid]
+        del self.messagelist[uid]
+        
     def deletemessage(self, uid):
         """Unlinks a message file from the Maildir.
 
diff --git a/offlineimap/folder/UIDMaps.py b/offlineimap/folder/UIDMaps.py
index fa1924d..2684e46 100644
--- a/offlineimap/folder/UIDMaps.py
+++ b/offlineimap/folder/UIDMaps.py
@@ -221,6 +221,31 @@ class MappedIMAPFolder(IMAPFolder):
         self._mb.addmessagesflags(self._uidlist(self.r2l, uidlist),
                                   flags)
 
+    def change_message_uid(self, ruid, new_ruid):
+        """Change the message from existing ruid to new_ruid
+
+        :param new_uid: The old remote UID will be changed to a new
+            UID. The UIDMaps case handles this efficiently by simply
+            changing the mappings file."""
+        if ruid not in self.r2l:
+            raise OfflineImapError("Cannot change unknown Maildir UID %s" % ruid,
+                                   OfflineImapError.ERROR.MESSAGE)
+        if ruid == new_ruid: return  # sanity check shortcut
+        self.maplock.acquire()
+        try:
+            luid = self.r2l[ruid]
+            self.l2r[luid] = new_ruid
+            del self.r2l[ruid]
+            self.r2l[new_ruid] = luid
+            #TODO: diskl2r|r2l are a pain to sync and should be done away with
+            #diskl2r only contains positive UIDs, so wrap in ifs
+            if luid>0: self.diskl2r[luid] = new_ruid
+            if ruid>0: del self.diskr2l[ruid]
+            if new_ruid > 0: self.diskr2l[new_ruid] = luid
+            self._savemaps(dolock = 0)
+        finally:
+            self.maplock.release()
+
     def _mapped_delete(self, uidlist):
         self.maplock.acquire()
         try:
-- 
1.7.4.1





More information about the OfflineIMAP-project mailing list