Added an option to control whether .old files are kept when packing

file storages.

Also fixed a bug in handling the create option with blob support.

src/CHANGES.txt

@@ -30,6 +30,8 @@ New Features
 
 - FileStorage now supports blobs directly.
 
+- You can now control whether FileStorages keep .old files when packing.
+
 3.9.0a8 (2008-12-15)
 ====================
 

src/ZODB/FileStorage/FileStorage.py

@@ -107,7 +107,8 @@ class FileStorage(
     _pack_is_in_progress = False
 
     def __init__(self, file_name, create=False, read_only=False, stop=None,
-                 quota=None, pack_gc=True, packer=None, blob_dir=None):
+                 quota=None, pack_gc=True, pack_keep_old=True, packer=None,
+                 blob_dir=None):
 
         if read_only:
             self._is_read_only = True
@@ -131,6 +132,7 @@ class FileStorage(
         self._file_name = file_name
 
         self._pack_gc = pack_gc
+        self.pack_keep_old = pack_keep_old
         if packer is not None:
             self.packer = packer
 
@@ -203,12 +205,16 @@ class FileStorage(
 
         self._quota = quota
 
-        self.blob_dir = blob_dir
         if blob_dir:
+            self.blob_dir = os.path.abspath(blob_dir)
+            if create and os.path.exists(self.blob_dir):
+                ZODB.blob.remove_committed_dir(self.blob_dir)
+                
             self._blob_init(blob_dir)
             zope.interface.alsoProvides(self,
                                         ZODB.interfaces.IBlobStorageRestoreable)
         else:
+            self.blob_dir = None
             self._blob_init_no_blobs()
 
     def copyTransactionsFrom(self, other):
@@ -1085,6 +1091,8 @@ class FileStorage(
 
                 # OK, we're beyond the point of no return
                 os.rename(self._file_name + '.pack', self._file_name)
+                if not self.pack_keep_old:
+                    os.remove(oldpath)
                 self._file = open(self._file_name, 'r+b')
                 self._initIndex(index, self._tindex)
                 self._pos = opos
@@ -1107,7 +1115,7 @@ class FileStorage(
         lblob_dir = len(self.blob_dir)
         fshelper = self.fshelper
         old = self.blob_dir+'.old'
-        os.mkdir(old, 0777)
+        link_or_copy = ZODB.blob.link_or_copy
 
         # Helper to clean up dirs left empty after moving things to old
         def maybe_remove_empty_dir_containing(path):
@@ -1118,17 +1126,23 @@ class FileStorage(
                 os.rmdir(path)
                 maybe_remove_empty_dir_containing(path)
 
-        # Helper that moves a oid dir or revision file to the old dir.
-        def move(path):
-            dest = os.path.dirname(old+path[lblob_dir:])
-            if not os.path.exists(dest):
-                os.makedirs(dest, 0700)
-            os.rename(path, old+path[lblob_dir:])
-            maybe_remove_empty_dir_containing(path)
+        if self.pack_keep_old:
+            # Helpers that move oid dir or revision file to the old dir.
+            os.mkdir(old, 0777)
+            link_or_copy(os.path.join(self.blob_dir, '.layout'),
+                         os.path.join(old, '.layout'))
+            def handle_file(path):
+                dest = os.path.dirname(old+path[lblob_dir:])
+                if not os.path.exists(dest):
+                    os.makedirs(dest, 0700)
+                os.rename(path, old+path[lblob_dir:])
+            handle_dir = handle_file
+        else:
+            # Helpers that remove an oid dir or revision file.
+            handle_file = ZODB.blob.remove_committed
+            handle_dir = ZODB.blob.remove_committed_dir
             
-        # Fist step: "remove" oids or revisions by moving them to .old
-        # (Later, when we add an option to not keep old files, we'll
-        # be able to simply remove.)
+        # Fist step: move or remove oids or revisions
         for line in open(os.path.join(self.blob_dir, '.removed')):
             line = line.strip().decode('hex')
 
@@ -1138,7 +1152,8 @@ class FileStorage(
                 if not os.path.exists(path):
                     # Hm, already gone. Odd.
                     continue
-                move(path)
+                handle_dir(path)
+                maybe_remove_empty_dir_containing(path)
                 continue
             
             if len(line) != 16:
@@ -1149,10 +1164,16 @@ class FileStorage(
             if not os.path.exists(path):
                 # Hm, already gone. Odd.
                 continue
-            move(path)
+            handle_file(path)
+            assert not os.path.exists(path)
+            maybe_remove_empty_dir_containing(path)
+
+        os.remove(os.path.join(self.blob_dir, '.removed'))
+
+        if not self.pack_keep_old:
+            return
             
         # Second step, copy remaining files.
-        link_or_copy = ZODB.blob.link_or_copy
         for path, dir_names, file_names in os.walk(self.blob_dir):
             for file_name in file_names:
                 if not file_name.endswith('.blob'):

src/ZODB/FileStorage/tests.py

@@ -11,15 +11,93 @@
 # FOR A PARTICULAR PURPOSE.
 #
 ##############################################################################
-import unittest
+
 from zope.testing import doctest
+
+import os
+import time
+import transaction
+import unittest
+import ZODB.FileStorage
 import ZODB.tests.util
 
+def pack_keep_old():
+    """Should a copy of the database be kept?
+    
+The pack_keep_old constructor argument controls whether a .old file (and .old directory for blobs is kept.)
+
+    >>> fs = ZODB.FileStorage.FileStorage('data.fs', blob_dir='blobs')
+    >>> db = ZODB.DB(fs)
+    >>> conn = db.open()
+    >>> import ZODB.blob
+    >>> conn.root()[1] = ZODB.blob.Blob()
+    >>> conn.root()[1].open('w').write('some data')
+    >>> conn.root()[2] = ZODB.blob.Blob()
+    >>> conn.root()[2].open('w').write('some data')
+    >>> transaction.commit()
+    >>> conn.root()[1].open('w').write('some other data')
+    >>> del conn.root()[2]
+    >>> transaction.commit()
+    >>> old_size = os.stat('data.fs').st_size
+    >>> def get_blob_size(d):
+    ...     result = 0
+    ...     for path, dirs, file_names in os.walk(d):
+    ...         for file_name in file_names:
+    ...             result += os.stat(os.path.join(path, file_name)).st_size
+    ...     return result
+    >>> blob_size = get_blob_size('blobs')
+
+    >>> db.pack(time.time()+1)
+    >>> packed_size = os.stat('data.fs').st_size
+    >>> packed_size < old_size
+    True
+    >>> os.stat('data.fs.old').st_size == old_size
+    True
+
+    >>> packed_blob_size = get_blob_size('blobs')
+    >>> packed_blob_size < blob_size
+    True
+    >>> get_blob_size('blobs.old') == blob_size
+    True
+    >>> db.close()
+
+    
+    >>> fs = ZODB.FileStorage.FileStorage('data.fs', blob_dir='blobs',
+    ...                                   create=True, pack_keep_old=False)
+    >>> db = ZODB.DB(fs)
+    >>> conn = db.open()
+    >>> conn.root()[1] = ZODB.blob.Blob()
+    >>> conn.root()[1].open('w').write('some data')
+    >>> conn.root()[2] = ZODB.blob.Blob()
+    >>> conn.root()[2].open('w').write('some data')
+    >>> transaction.commit()
+    >>> conn.root()[1].open('w').write('some other data')
+    >>> del conn.root()[2]
+    >>> transaction.commit()
+
+    >>> db.pack(time.time()+1)
+    >>> os.stat('data.fs').st_size == packed_size
+    True
+    >>> os.path.exists('data.fs.old')
+    False
+    >>> get_blob_size('blobs') == packed_blob_size
+    True
+    >>> os.path.exists('blobs.old')
+    False
+    >>> db.close()
+    
+    
+    """
+
+
 def test_suite():
     return unittest.TestSuite((
         doctest.DocFileSuite(
             'zconfig.txt',
             setUp=ZODB.tests.util.setUp, tearDown=ZODB.tests.util.tearDown,
             ),
+        doctest.DocTestSuite(
+            setUp=ZODB.tests.util.setUp, tearDown=ZODB.tests.util.tearDown,
+            ),
         ))
 

src/ZODB/FileStorage/zconfig.txt

@@ -39,15 +39,17 @@ blob-dir
     >>> os.path.basename(fs.blob_dir)
     'blobs'
 
-    >>> fs.close()
-
 create
     Flag that indicates whether the storage should be truncated if
     it already exists.
 
-    To demonstrate this, we'll first write some dataL
+    To demonstrate this, we'll first write some data:
 
-    >>> db = ZODB.DB('my.fs') # writes object 0
+    >>> db = ZODB.DB(fs)
+    >>> conn = db.open()
+    >>> import ZODB.blob, transaction
+    >>> conn.root()[1] = ZODB.blob.Blob()
+    >>> transaction.commit()
     >>> db.close()
 
     Then reopen with the create option:
@@ -55,6 +57,7 @@ create
     >>> fs = ZODB.config.storageFromString("""
     ... <filestorage>
     ...     path my.fs
+    ...     blob-dir blobs
     ...     create true
     ... </filestorage>
     ... """)
@@ -66,6 +69,9 @@ create
     ...
     POSKeyError: 0x00
 
+    >>> sorted(os.listdir('blobs'))
+    ['.layout', 'tmp']
+
     >>> fs.close()
 
 read-only
@@ -111,7 +117,7 @@ packer
     some information about it's arguments:
 
     >>> def packer(storage, referencesf, stop, gc):
-    ...     print referencesf, storage is fs, gc
+    ...     print referencesf, storage is fs, gc, storage.pack_keep_old
     >>> ZODB.FileStorage.config_demo_printing_packer = packer
 
     >>> fs = ZODB.config.storageFromString("""
@@ -124,7 +130,7 @@ packer
     >>> import time
     >>> db = ZODB.DB(fs) # writes object 0
     >>> fs.pack(time.time(), 42)
-    42 True True
+    42 True True True
 
     >>> fs.close()
 
@@ -143,13 +149,29 @@ pack-gc
     ... """)
 
     >>> fs.pack(time.time(), 42)
-    42 True False
+    42 True False True
 
     Note that if we pass the gc option to pack, then this will
     override the value set in the configuration:
 
     >>> fs.pack(time.time(), 42, gc=True)
-    42 True True
+    42 True True True
+    
+    >>> fs.close()
+
+pack-keep-old
+    If false, then old files aren't kept when packing
+
+    >>> fs = ZODB.config.storageFromString("""
+    ... <filestorage>
+    ...     path my.fs
+    ...     packer ZODB.FileStorage.config_demo_printing_packer
+    ...     pack-keep-old false
+    ... </filestorage>
+    ... """)
+
+    >>> fs.pack(time.time(), 42)
+    42 True True False
     
     >>> fs.close()
 

src/ZODB/component.xml

@@ -59,6 +59,12 @@
          databases.
       </description>
     </key>
+    <key name="pack-keep-old" datatype="boolean" default="true">
+      <description>
+         If true, a copy of the database before packing is kept in a
+         ".old" file.
+      </description>
+    </key>
   </sectiontype>
 
   <sectiontype name="mappingstorage" datatype=".MappingStorage"

src/ZODB/config.py

@@ -11,9 +11,7 @@
 # FOR A PARTICULAR PURPOSE
 #
 ##############################################################################
-"""Open database and storage from a configuration.
-
-$Id$"""
+"""Open database and storage from a configuration."""
 
 import os
 from cStringIO import StringIO
@@ -147,6 +145,7 @@ class FileStorage(BaseConfig):
                            read_only=self.config.read_only,
                            quota=self.config.quota,
                            pack_gc=self.config.pack_gc,
+                           pack_keep_old=self.config.pack_keep_old,
                            blob_dir=self.config.blob_dir,
                            **options)