SVN: ZODB/trunk/src/ZODB/ Fixed bug:

Log message for revision 103176:
  Fixed bug:
    Savepoint blob data wasn't cleaned up after a transaction abort.
    https://bugs.launchpad.net/zodb/+bug/323067
  

Changed:
  U   ZODB/trunk/src/ZODB/Connection.py
  U   ZODB/trunk/src/ZODB/tests/blob_transaction.txt
  U   ZODB/trunk/src/ZODB/tests/testblob.py

-=-
Modified: ZODB/trunk/src/ZODB/Connection.py
===================================================================
--- ZODB/trunk/src/ZODB/Connection.py	2009-08-24 18:17:54 UTC (rev 103175)
+++ ZODB/trunk/src/ZODB/Connection.py	2009-08-24 18:17:56 UTC (rev 103176)
@@ -31,7 +31,7 @@
 from ZODB.interfaces import IConnection
 from ZODB.interfaces import IBlobStorage
 from ZODB.interfaces import IMVCCStorage
-from ZODB.blob import Blob, rename_or_copy_blob
+from ZODB.blob import Blob, rename_or_copy_blob, remove_committed_dir
 from transaction.interfaces import ISavepointDataManager
 from transaction.interfaces import IDataManagerSavepoint
 from transaction.interfaces import ISynchronizer
@@ -1239,11 +1239,16 @@
         self.index = {}
         self.creating = {}
 
+        self._blob_dir = None
+
     def __len__(self):
         return len(self.index)
 
     def close(self):
         self._file.close()
+        if self._blob_dir is not None:
+            remove_committed_dir(self._blob_dir)
+            self._blob_dir = None
 
     def load(self, oid, version):
         pos = self.index.get(oid)
@@ -1307,12 +1312,19 @@
             return ZODB.blob.BlobFile(blob_filename, 'r', blob)
 
     def _getBlobPath(self):
-        return os.path.join(self.temporaryDirectory(), 'savepoints')
+        blob_dir = self._blob_dir
+        if blob_dir is None:
+            blob_dir = tempfile.mkdtemp(dir=self.temporaryDirectory(),
+                                        prefix='savepoints')
+            self._blob_dir = blob_dir
+        return blob_dir
 
     def _getCleanFilename(self, oid, tid):
-        return os.path.join(self._getBlobPath(),
-                            "%s-%s%s" % (utils.oid_repr(oid), utils.tid_repr(tid), SAVEPOINT_SUFFIX,)
-                            )
+        return os.path.join(
+            self._getBlobPath(),
+            "%s-%s%s" % (utils.oid_repr(oid), utils.tid_repr(tid),
+                         SAVEPOINT_SUFFIX,)
+            )
 
     def temporaryDirectory(self):
         return self._storage.temporaryDirectory()

Modified: ZODB/trunk/src/ZODB/tests/blob_transaction.txt
===================================================================
--- ZODB/trunk/src/ZODB/tests/blob_transaction.txt	2009-08-24 18:17:54 UTC (rev 103175)
+++ ZODB/trunk/src/ZODB/tests/blob_transaction.txt	2009-08-24 18:17:56 UTC (rev 103176)
@@ -231,18 +231,22 @@
     >>> root5['blob'].open("r").read()
     "I'm a happy blob. And I'm singing."
 
-Savepoints store the blobs in the `savepoints` directory in the temporary
+Savepoints store the blobs in temporary directories in the temporary
 directory of the blob storage:
 
-    >>> os.listdir(os.path.join(blob_dir, 'tmp', 'savepoints'))
-    ['0x...-0x....spb']
-    >>> transaction.commit()
+    >>> len([name for name in os.listdir(os.path.join(blob_dir, 'tmp'))
+    ...      if name.startswith('savepoint')])
+    1
+    >>> os.listdir(os.path.join(blob_dir, 'tmp'))[0].startswith('savepoint')
+    True
 
 After committing the transaction, the temporary savepoint files are moved to
 the committed location again:
 
-    >>> os.listdir(os.path.join(blob_dir, 'tmp', 'savepoints'))
-    []
+    >>> transaction.commit()
+    >>> len([name for name in os.listdir(os.path.join(blob_dir, 'tmp'))
+    ...      if name.startswith('savepoint')])
+    0
 
 We support non-optimistic savepoints too:
 
@@ -251,28 +255,24 @@
     "I'm a happy blob. And I'm singing. And I'm dancing."
     >>> savepoint = transaction.savepoint()
 
-Again, the savepoint creates a new file for the blob state in the savepoints
-directory:
+Again, the savepoint creates a new savepoints directory:
 
-    >>> os.listdir(os.path.join(blob_dir, 'tmp', 'savepoints'))
-    ['0x...-0x....spb']
+    >>> len([name for name in os.listdir(os.path.join(blob_dir, 'tmp'))
+    ...      if name.startswith('savepoint')])
+    1
 
     >>> root5['blob'].open("w").write(" And the weather is beautiful.")
     >>> savepoint.rollback()
 
-XXX Currently, savepoint state of blobs remains after a rollback:
-
-    >>> os.listdir(os.path.join(blob_dir, 'tmp', 'savepoints'))
-    ['0x...-0x....spb']
-
     >>> root5['blob'].open("r").read()
     "I'm a happy blob. And I'm singing. And I'm dancing."
     >>> transaction.abort()
 
-XXX Currently, savepoint state of blobs remains even after an abort:
+The savepoint blob directory gets cleaned up on an abort:
 
-    >>> os.listdir(os.path.join(blob_dir, 'tmp', 'savepoints'))
-    ['0x...-0x....spb']
+    >>> len([name for name in os.listdir(os.path.join(blob_dir, 'tmp'))
+    ...      if name.startswith('savepoint')])
+    0
 
 Reading Blobs outside of a transaction
 --------------------------------------

Modified: ZODB/trunk/src/ZODB/tests/testblob.py
===================================================================
--- ZODB/trunk/src/ZODB/tests/testblob.py	2009-08-24 18:17:54 UTC (rev 103175)
+++ ZODB/trunk/src/ZODB/tests/testblob.py	2009-08-24 18:17:56 UTC (rev 103176)
@@ -15,23 +15,17 @@
 from pickle import Pickler
 from pickle import Unpickler
 from StringIO import StringIO
-from ZConfig import ConfigurationSyntaxError
-from ZODB.blob import Blob, BlobStorage
+from ZODB.blob import Blob
 from ZODB.DB import DB
 from ZODB.FileStorage import FileStorage
-from ZODB import utils
 from ZODB.tests.testConfig import ConfigTestBase
 from zope.testing import doctest
 
-import base64
 import os
 import random
 import re
-import shutil
-import stat
 import struct
 import sys
-import sys
 import time
 import transaction
 import unittest
@@ -538,6 +532,59 @@
     >>> bs.close()
     """
 
+def savepoint_isolation():
+    """Make sure savepoint data is distinct accross transactions
+
+    >>> bs = create_storage()
+    >>> db = DB(bs)
+    >>> conn = db.open()
+    >>> conn.root.b = ZODB.blob.Blob('initial')
+    >>> transaction.commit()
+    >>> conn.root.b.open('w').write('1')
+    >>> _ = transaction.savepoint()
+    >>> tm = transaction.TransactionManager()
+    >>> conn2 = db.open(transaction_manager=tm)
+    >>> conn2.root.b.open('w').write('2')
+    >>> _ = tm.savepoint()
+    >>> conn.root.b.open().read()
+    '1'
+    >>> conn2.root.b.open().read()
+    '2'
+    >>> transaction.abort()
+    >>> tm.commit()
+    >>> conn.sync()
+    >>> conn.root.b.open().read()
+    '2'
+    >>> db.close()
+    """
+
+def savepoint_cleanup():
+    """Make sure savepoint data gets cleaned up.
+
+    >>> bs = create_storage()
+    >>> tdir = bs.temporaryDirectory()
+    >>> os.listdir(tdir)
+    []
+
+    >>> db = DB(bs)
+    >>> conn = db.open()
+    >>> conn.root.b = ZODB.blob.Blob('initial')
+    >>> _ = transaction.savepoint()
+    >>> len(os.listdir(tdir))
+    1
+    >>> transaction.abort()
+    >>> os.listdir(tdir)
+    []
+    >>> conn.root.b = ZODB.blob.Blob('initial')
+    >>> transaction.commit()
+    >>> conn.root.b.open('w').write('1')
+    >>> _ = transaction.savepoint()
+    >>> transaction.abort()
+    >>> os.listdir(tdir)
+    []
+    """
+
+
 def setUp(test):
     ZODB.tests.util.setUp(test)
     test.globs['rmtree'] = zope.testing.setupstack.rmtree