Fix getuidvalidity crash (UIDVALIDITY returning None)

Rename getuidvalidity -> get_uidvalidity and cache the IMAP result.

1) Start modernizing our function names using more underscores
2) IMAPs implementation of get_uidvalidity was removing the UIDVALIDITY result
   from the imaplib2 result stack, so subsequent calls would return "None".
   As various functions can invoke this, this led to some errors that we
   avoid by caching the current UIDVALIDITY value in the Folder instance.

There are more simplifications and improvements to be made.

Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>

Changelog.draft.rst

@@ -26,3 +26,4 @@ Bug Fixes
 
 * Fix python2.6 compatibility with the TTYUI backend (crash)
 * Fix TTYUI regression from 6.5.2 in refresh loop (crash)
+* Fix crashes related to UIDVALIDITY returning "None"

offlineimap/accounts.py

@@ -378,7 +378,7 @@ def syncfolder(account, remotefolder, quick):
         statusfolder = statusrepos.getfolder(remotefolder.getvisiblename().\
                                              replace(remoterepos.getsep(),
                                                      statusrepos.getsep()))
-        if localfolder.getuidvalidity() == None:
+        if localfolder.get_uidvalidity() == None:
             # This is a new folder, so delete the status cache to be sure
             # we don't have a conflict.
             statusfolder.deletemessagelist()

offlineimap/folder/Base.py

@@ -110,13 +110,14 @@ class BaseFolder(object):
         return basename
 
     def isuidvalidityok(self):
-        """Does the cached UID match the real UID
+        """Tests if the cached UIDVALIDITY match the real current one
 
-        If required it caches the UID. In this case the function is not
-        threadsafe. So don't attempt to call it from concurrent threads."""
+        If required it saves the UIDVALIDITY value. In this case the
+        function is not threadsafe. So don't attempt to call it from
+        concurrent threads."""
 
         if self.getsaveduidvalidity() != None:
-            return self.getsaveduidvalidity() == self.getuidvalidity()
+            return self.getsaveduidvalidity() == self.get_uidvalidity()
         else:
             self.saveuidvalidity()
             return 1
@@ -142,7 +143,7 @@ class BaseFolder(object):
 
         This function is not threadsafe, so don't attempt to call it
         from concurrent threads."""
-        newval = self.getuidvalidity()
+        newval = self.get_uidvalidity()
         uidfilename = self._getuidfilename()
 
         file = open(uidfilename + ".tmp", "wt")
@@ -151,7 +152,11 @@ class BaseFolder(object):
         os.rename(uidfilename + ".tmp", uidfilename)
         self._base_saved_uidvalidity = newval
 
-    def getuidvalidity(self):
+    def get_uidvalidity(self):
+        """Retrieve the current connections UIDVALIDITY value
+
+        This function needs to be implemented by each Backend
+        :returns: UIDVALIDITY as a (long) number"""
         raise NotImplementedException
 
     def cachemessagelist(self):

offlineimap/folder/IMAP.py

@@ -65,17 +65,23 @@ class IMAPFolder(BaseFolder):
     def getcopyinstancelimit(self):
         return 'MSGCOPY_' + self.repository.getname()
 
-    def getuidvalidity(self):
+    def get_uidvalidity(self):
+        """Retrieve the current connections UIDVALIDITY value
+
+        UIDVALIDITY value will be cached on the first call.
+        :returns: The UIDVALIDITY as (long) number."""
+        if hasattr(self, '_uidvalidity'):
+            # use cached value if existing
+            return self._uidvalidity
         imapobj = self.imapserver.acquireconnection()
         try:
-            # SELECT receives UIDVALIDITY response
+            # SELECT (if not already done) and get current UIDVALIDITY
             self.selectro(imapobj)
-            # note: we would want to use .response() here but that
-            # often seems to return [None], even though we have
-            # data. TODO
-            uidval = imapobj._get_untagged_response('UIDVALIDITY')
-            assert uidval != [None], "response('UIDVALIDITY') returned [None]!"
-            return long(uidval[-1])
+            typ, uidval = imapobj.response('UIDVALIDITY')
+            assert uidval != [None] and uidval != None, \
+                "response('UIDVALIDITY') returned [None]!"
+            self._uidvalidity = long(uidval[-1])
+            return self._uidvalidity
         finally:
             self.imapserver.releaseconnection(imapobj)
 

offlineimap/folder/Maildir.py

@@ -83,8 +83,10 @@ class MaildirFolder(BaseFolder):
         """Return the absolute file path to the Maildir folder (sans cur|new)"""
         return self._fullname
 
-    def getuidvalidity(self):
-        """Maildirs have no notion of uidvalidity, so we just return a magic
+    def get_uidvalidity(self):
+        """Retrieve the current connections UIDVALIDITY value
+
+        Maildirs have no notion of uidvalidity, so we just return a magic
         token."""
         return 42
 

offlineimap/ui/Machine.py

@@ -70,7 +70,7 @@ class MachineUI(UIBase):
     def validityproblem(s, folder):
         s._printData('validityproblem', "%s\n%s\n%s\n%s" % \
                 (folder.getname(), folder.getrepository().getname(),
-                 folder.getsaveduidvalidity(), folder.getuidvalidity()))
+                 folder.getsaveduidvalidity(), folder.get_uidvalidity()))
 
     def connecting(s, hostname, port):
         s._printData('connecting', "%s\n%s" % (hostname, str(port)))

offlineimap/ui/UIBase.py

@@ -309,9 +309,9 @@ class UIBase(object):
     def validityproblem(self, folder):
         self.logger.warning("UID validity problem for folder %s (repo %s) "
                             "(saved %d; got %d); skipping it. Please see FAQ "
-                            "and manual how to handle this." % \
+                            "and manual on how to handle this." % \
                (folder, folder.getrepository(),
-                folder.getsaveduidvalidity(), folder.getuidvalidity()))
+                folder.getsaveduidvalidity(), folder.get_uidvalidity()))
 
     def loadmessagelist(self, repos, folder):
         self.logger.debug("Loading message list for %s[%s]" % (