Refactored stacking support:

- explicit push and pop methods

- backward compatible with old behavior.

src/ZODB/DemoStorage.py

@@ -19,7 +19,9 @@ to be layered over a base database.
 The base storage must not change.
 
 """
+import os
 import random
+import weakref
 import tempfile
 import threading
 import ZODB.blob
@@ -36,18 +38,18 @@ class DemoStorage(object):
         ZODB.interfaces.IStorageIteration,
         )
 
-    def __init__(self, name=None, base=None, changes=None,
-                 keep_base_open=False):
-        self._keep_base_open = keep_base_open
+    def __init__(self, name=None, base=None, changes=None):
         if base is None:
             base = ZODB.MappingStorage.MappingStorage()
+            self._temporary_base = True
+        else:
+            self._temporary_base = False
         self.base = base
             
         if changes is None:
             changes = ZODB.MappingStorage.MappingStorage()
             zope.interface.alsoProvides(self, ZODB.interfaces.IBlobStorage)
             self._temporary_changes = True
-            self._blob_dir = None
         else:
             if ZODB.interfaces.IBlobStorage.providedBy(changes):
                 zope.interface.alsoProvides(self, ZODB.interfaces.IBlobStorage)
@@ -61,10 +63,16 @@ class DemoStorage(object):
 
         self._copy_methods_from_changes(changes)
         
+
     def _blobify(self):
-        if self._temporary_changes and self._blob_dir is None:
-            self._blob_dir = tempfile.mkdtemp('blobs')
-            self.changes = ZODB.blob.BlobStorage(self._blob_dir, self.changes)
+        if (self._temporary_changes and
+            isinstance(self.changes, ZODB.MappingStorage.MappingStorage)
+            ):
+            blob_dir = tempfile.mkdtemp('.demoblobs')
+            _temporary_blobdirs[
+                weakref.ref(self, cleanup_temporary_blobdir)
+                ] = blob_dir
+            self.changes = ZODB.blob.BlobStorage(blob_dir, self.changes)
             self._copy_methods_from_changes(self.changes)
             return True
     
@@ -73,11 +81,10 @@ class DemoStorage(object):
         self.changes.cleanup()
 
     def close(self):
-        if not self._keep_base_open:
+        if not self._temporary_base:
             self.base.close()
+        if not self._temporary_changes:
             self.changes.close()
-        if getattr(self, '_blob_dir', ''):
-            ZODB.blob.remove_committed_dir(self._blob_dir)
 
     def _copy_methods_from_changes(self, changes):
         for meth in (
@@ -195,6 +202,13 @@ class DemoStorage(object):
                 pass # The gc arg isn't supported. Don't pack
             raise
 
+    def pop(self):
+        self.changes.close()
+        return self.base
+
+    def push(self, changes=None):
+        return self.__class__(base=self, changes=changes)
+
     def store(self, oid, serial, data, version, transaction):
         assert version=='', "versions aren't supported"
 
@@ -231,3 +245,12 @@ class DemoStorage(object):
             if self._blobify():
                 return self.changes.temporaryDirectory()
             raise
+
+_temporary_blobdirs = {}
+def cleanup_temporary_blobdir(
+    ref,
+    _temporary_blobdirs=_temporary_blobdirs, # Make sure it stays around 
+    ):
+    blob_dir = _temporary_blobdirs.pop(ref, None)
+    if blob_dir and os.path.exists(blob_dir):
+        ZODB.blob.remove_committed_dir(blob_dir)

src/ZODB/DemoStorage.test

@@ -120,37 +120,66 @@ Undo methods are simply copied from the changes storage:
     ...  ]
     [True, True, True, True]
 
+    >>> db.close()
 
-Normally, when we close a demo storage, the changes and base storages
-are closed:
+Storage Stacking
+================
 
-    >>> db.close()
-    >>> base._file.closed
+A common use case is to stack demo storages.  DemoStorage provides
+some helper functions to help with this.  The push method, just
+creates a new demo storage who's base is the original demo storage:
+
+    >>> demo = DemoStorage()
+    >>> demo2 = demo.push()
+    >>> demo2.base is demo
     True
-    >>> changes._file.closed
+
+We can also supply an explicit changes storage, if we wish:
+
+    >>> from ZODB.MappingStorage import MappingStorage
+    >>> changes = MappingStorage()
+    >>> demo3 = demo2.push(changes)
+    >>> demo3.changes is changes, demo3.base is demo2
+    (True, True)
+
+The pop method closes the changes storage and returns the base
+*without* closing it:
+
+    >>> demo3.pop() is demo2
     True
 
-A common use case is to stack multiple DemoStorages, returning to a
-previous state by popping a DemoStorage off the stack.  In this case,
-we want to leave the base storage open:
+    >>> changes.opened()
+    False
 
-    >>> base = FileStorage('base.fs', read_only=True)
-    >>> storage = DemoStorage(base=base, keep_base_open=True)
+Special backward compatibility support
+--------------------------------------
 
-Here, we didn't specify a changes storage.  A MappingStorage was
-automatically created:
+Normally, when a demo storage is closed, it's base and changes
+storage are closed:
 
-    >>> type(storage.changes).__name__
-    'MappingStorage'
+    >>> demo = DemoStorage(base=MappingStorage(), changes=MappingStorage())
+    >>> demo.close()
+    >>> demo.base.opened(), demo.changes.opened()
+    (False, False)
 
-Because we specified the keep_base_open option, the base storage is
-left open when we close the DemoStorage:
+Older versions of DemoStorage didn't have a separate changes storage
+and didn't close or discard their changes when they were closed.  When
+a stack was built solely of demo storages, the close method
+effectively did nothing.  To maintain backward compatibility, when no
+base or changes storage is supplied in the constructor, the underlying
+storage created by the demo storage isn't closed by the demo storage.
+This backward-compatibility is deprecated.
 
-    >>> storage.close()
-    >>> base._file.closed
-    False
-    >>> storage.changes.opened()
-    False
+    >>> demo = DemoStorage()
+    >>> demo.close()
+    >>> demo.changes.opened(), demo.base.opened()
+    (True, True)
+
+    >>> demo = DemoStorage(base=MappingStorage())
+    >>> demo2 = demo.push()
+    >>> demo2.close()
+    >>> demo2.changes.opened(), demo2.base.base.opened()
+    (True, False)
 
 Blob Support
 ============
@@ -236,6 +265,16 @@ storage wrapped around it when necessary:
 
 .. Check that the temporary directory is gone
 
+   For now, it won't go until the storage does.
+
+   >>> transaction.abort()
+   >>> conn.close()
+   >>> blobdir = storage.temporaryDirectory()
+   >>> del db, conn, storage, _
+   
+   >>> import gc
+   >>> _ = gc.collect()
+
    >>> import os
-   >>> os.path.exists(storage.temporaryDirectory())
+   >>> os.path.exists(blobdir)
    False