changeset 3863:6d2f6f954c08

caching: improve comments/docstrings, better exception handling, use file-like api for contents() and update(), calculate cache file name only once, cleanup
author Thomas Waldmann <tw AT waldmann-edv DOT de>
date Wed, 16 Jul 2008 19:20:23 +0200
parents 2526b5b8411f
children 6dbeb3669091
files MoinMoin/caching.py
diffstat 1 files changed, 63 insertions(+), 80 deletions(-) [+]
line wrap: on
line diff
--- a/MoinMoin/caching.py	Wed Jul 16 17:53:40 2008 +0200
+++ b/MoinMoin/caching.py	Wed Jul 16 19:20:23 2008 +0200
@@ -71,25 +71,32 @@
         self.arena_dir = get_arena_dir(request, arena, scope)
         if not os.path.exists(self.arena_dir):
             os.makedirs(self.arena_dir)
+        self._fname = os.path.join(self.arena_dir, key)
+        
         if self.locking:
             self.lock_dir = os.path.join(self.arena_dir, '__lock__')
             self.rlock = lock.LazyReadLock(self.lock_dir, 60.0)
             self.wlock = lock.LazyWriteLock(self.lock_dir, 60.0)
+
         # used by file-like api:
-        self._fileobj = None
-        self._lock = None
+        self._lock = None  # either self.rlock or self.wlock
+        self._fileobj = None  # open cache file object
+        self._tmp_fname = None  # name of temporary file (used for write)
+        self._mode = None  # mode of open file object
+
 
     def _filename(self):
-        return os.path.join(self.arena_dir, self.key)
+        # DEPRECATED - please use file-like api
+        return self._fname
 
     def exists(self):
-        return os.path.exists(self._filename())
+        return os.path.exists(self._fname)
 
     def mtime(self):
         # DEPRECATED for checking a changed on-disk cache, please use
         # self.uid() for this, see below
         try:
-            return os.path.getmtime(self._filename())
+            return os.path.getmtime(self._fname)
         except (IOError, OSError):
             return 0
 
@@ -98,7 +105,7 @@
 
             See docstring of MoinMoin.util.filesys.fuid for details.
         """
-        return filesys.fuid(self._filename())
+        return filesys.fuid(self._fname)
 
     def needsUpdate(self, filename, attachdir=None):
         # following code is not necessary. will trigger exception and give same result
@@ -106,7 +113,7 @@
         #    return 1
 
         try:
-            ctime = os.path.getmtime(self._filename())
+            ctime = os.path.getmtime(self._fname)
             ftime = os.path.getmtime(filename)
         except os.error:
             return 1
@@ -123,68 +130,55 @@
 
         return needsupdate
 
-#    def copyto(self, filename):
-#        # currently unused function
-#        import shutil
-#        tmpfname = self._tmpfilename()
-#        fname = self._filename()
-#        if not self.locking or self.locking and self.wlock.acquire(1.0):
-#            try:
-#                shutil.copyfile(filename, tmpfname)
-#                # this is either atomic or happening with real locks set:
-#                filesys.rename(tmpfname, fname)
-#            finally:
-#                if self.locking:
-#                    self.wlock.release()
-#        else:
-#            logging.error("Can't acquire write lock in %s" % self.lock_dir)
-
     def _determine_locktype(self, mode):
         """ return the correct lock object for a specific file access mode """
-        if 'r' in mode:
-            lock = self.rlock
-        if 'w' in mode or 'a' in mode:
-            lock = self.wlock
+        if self.locking:
+            if 'r' in mode:
+                lock = self.rlock
+            if 'w' in mode or 'a' in mode:
+                lock = self.wlock
+        else:
+            lock = None
         return lock
 
     # file-like interface ----------------------------------------------------
 
     def open(self, filename=None, mode='r', bufsize=-1):
-        """ open the cache file for reading/writing
+        """ open the cache for reading/writing
 
-        @param filename: should be None (default - means to use self._filename())
-        @param mode: 'r' (read, default), 'w' (write), 'a' (append)
+        @param filename: must be None (default - automatically determine filename)
+        @param mode: 'r' (read, default), 'w' (write)
                      Note: if mode does not include 'b' (binary), it will be
                            automatically changed to include 'b'.
         @param bufsize: size of read/write buffer (default: -1 meaning automatic)
         @return: None (the opened file object is kept in self._fileobj and used
                  implicitely by read/write/close functions of CacheEntry object.
         """
-        if self._fileobj:
-            # bug-possibility: this doesn't check, if there is any lock on the file
-            # we assume this, as the first call to open should
-            # acquire the lock.
-            return
-
-        if filename is None:
-            filename = self._filename()
-        else:
-            raise Exception('caching: giving a filename is not supported (yet?)')
+        assert self._fileobj is None, 'caching: trying to open an already opened cache'
+        assert filename is None, 'caching: giving a filename is not supported (yet?)'
 
         self._lock = self._determine_locktype(mode)
 
         if 'b' not in mode:
             mode += 'b'  # we want to use binary mode, ever!
+        self._mode = mode  # for self.close()
 
         if not self.locking or self.locking and self._lock.acquire(1.0):
             try:
-                self._fileobj = open(filename, mode, bufsize)
-            except IOError:
+                if 'r' in mode:
+                    self._fileobj = open(self._fname, mode, bufsize)
+                elif 'w' in mode:
+                    # we do not write content to old inode, but to a new file
+                    # so we don't need to lock when we just want to read the file
+                    # (at least on POSIX, this works)
+                    fd, self._tmp_fname = tempfile.mkstemp('.tmp', self.key, self.arena_dir)
+                    self._fileobj = os.fdopen(fd, mode, bufsize)
+                else:
+                    raise ValueError("caching: mode does not contain 'r' or 'w'")
+            finally:
                 if self.locking:
                     self._lock.release()
                     self._lock = None
-                logging.error("Can't open cache file %s" % filename)
-                raise
         else:
             logging.error("Can't acquire read/write lock in %s" % self.lock_dir)
 
@@ -209,6 +203,10 @@
         if self._fileobj:
             self._fileobj.close()
             self._fileobj = None
+            if 'w' in self._mode:
+                filesys.chmod(self._tmp_fname, 0666 & config.umask) # fix mode that mkstemp chose
+                # this is either atomic or happening with real locks set:
+                filesys.rename(self._tmp_fname, self._fname)
 
         if self._lock:
             if self.locking:
@@ -219,30 +217,34 @@
 
     def update(self, content):
         try:
-            fname = self._filename()
             if self.use_pickle:
                 content = pickle.dumps(content, PICKLE_PROTOCOL)
             elif self.use_encode:
                 content = content.encode(config.charset)
-            if not self.locking or self.locking and self.wlock.acquire(1.0):
-                try:
-                    # we do not write content to old inode, but to a new file
-                    # so we don't need to lock when we just want to read the file
-                    # (at least on POSIX, this works)
-                    tmp_handle, tmp_fname = tempfile.mkstemp('.tmp', self.key, self.arena_dir)
-                    os.write(tmp_handle, content)
-                    os.close(tmp_handle)
-                    # this is either atomic or happening with real locks set:
-                    filesys.rename(tmp_fname, fname)
-                    filesys.chmod(fname, 0666 & config.umask) # fix mode that mkstemp chose
-                finally:
-                    if self.locking:
-                        self.wlock.release()
-            else:
-                logging.error("Can't acquire write lock in %s" % self.lock_dir)
+
+            try:
+                self.open(mode='w')
+                self.write(content)
+            finally:
+                self.close()
         except (pickle.PicklingError, OSError, IOError, ValueError), err:
             raise CacheError(str(err))
 
+    def content(self):
+        try:
+            try:
+                self.open(mode='r')
+                data = self.read()
+            finally:
+                self.close()
+            if self.use_pickle:
+                data = pickle.loads(data)
+            elif self.use_encode:
+                data = data.decode(config.charset)
+            return data
+        except (pickle.UnpicklingError, IOError, EOFError, ValueError), err:
+            raise CacheError(str(err))
+    
     def remove(self):
         if not self.locking or self.locking and self.wlock.acquire(1.0):
             try:
@@ -256,23 +258,4 @@
         else:
             logging.error("Can't acquire write lock in %s" % self.lock_dir)
 
-    def content(self):
-        try:
-            if not self.locking or self.locking and self.rlock.acquire(1.0):
-                try:
-                    f = open(self._filename(), 'rb')
-                    data = f.read()
-                    f.close()
-                finally:
-                    if self.locking:
-                        self.rlock.release()
-            else:
-                logging.error("Can't acquire read lock in %s" % self.lock_dir)
-            if self.use_pickle:
-                data = pickle.loads(data)
-            elif self.use_encode:
-                data = data.decode(config.charset)
-            return data
-        except (pickle.UnpicklingError, IOError, EOFError, ValueError), err:
-            raise CacheError(str(err))