Fixed a couple of blob storage issues:

- The "lawn" layout was being selected by default if the root of
  the blob directory happened to contain a hidden file or directory
  such as ".svn".  Now hidden files and directories are ignored
  when choosing the default layout.

- BlobStorage was not compatible with MVCC storages because the
  wrappers were being removed by each database connection.  There
  was also a problem with subtransactions.  Fixed.

src/CHANGES.txt

@@ -22,6 +22,14 @@ Bugs Fixed
 - zeopack was less flexible than it was before.  -h should default to
   local host.
 
+- The "lawn" layout was being selected by default if the root of
+  the blob directory happened to contain a hidden file or directory
+  such as ".svn".  Now hidden files and directories are ignored
+  when choosing the default layout.
+
+- BlobStorage was not compatible with MVCC storages because the
+  wrappers were being removed by each database connection.  Fixed.
+
 3.9.0b2 (2009-06-11)
 ====================
 

src/ZODB/Connection.py

@@ -334,10 +334,6 @@ class Connection(ExportImport, object):
 
     def invalidate(self, tid, oids):
         """Notify the Connection that transaction 'tid' invalidated oids."""
-        if self._mvcc_storage:
-            # Inter-connection invalidation is not needed when the
-            # storage provides MVCC.
-            return
         if self.before is not None:
             # this is an historical connection.  Invalidations are irrelevant.
             return
@@ -771,6 +767,10 @@ class Connection(ExportImport, object):
         """Indicate confirmation that the transaction is done."""
 
         def callback(tid):
+            if self._mvcc_storage:
+                # Inter-connection invalidation is not needed when the
+                # storage provides MVCC.
+                return
             d = dict.fromkeys(self._modified)
             self._db.invalidate(tid, d, self)
 #       It's important that the storage calls the passed function

src/ZODB/blob.py

@@ -502,23 +502,30 @@ def auto_layout_select(path):
     # A heuristic to look at a path and determine which directory layout to
     # use.
     layout_marker = os.path.join(path, LAYOUT_MARKER)
-    if not os.path.exists(path):
+    if os.path.exists(layout_marker):
+        layout = open(layout_marker, 'rb').read()
+        layout = layout.strip()
+        log('Blob directory `%s` has layout marker set. '
+            'Selected `%s` layout. ' % (path, layout), level=logging.DEBUG)
+    elif not os.path.exists(path):
         log('Blob directory %s does not exist. '
             'Selected `bushy` layout. ' % path)
         layout = 'bushy'
-    elif len(os.listdir(path)) == 0:
+    else:
+        # look for a non-hidden file in the directory
+        has_files = False
+        for name in os.listdir(path):
+            if not name.startswith('.'):
+                has_files = True
+                break
+        if not has_files:
             log('Blob directory `%s` is unused and has no layout marker set. '
                 'Selected `bushy` layout. ' % path)
             layout = 'bushy'
-    elif LAYOUT_MARKER not in os.listdir(path):
+        else:
             log('Blob directory `%s` is used but has no layout marker set. '
                 'Selected `lawn` layout. ' % path)
             layout = 'lawn'
-    else:
-        layout = open(layout_marker, 'rb').read()
-        layout = layout.strip()
-        log('Blob directory `%s` has layout marker set. '
-            'Selected `%s` layout. ' % (path, layout), level=logging.DEBUG)
     return layout
 
 
@@ -861,6 +868,18 @@ class BlobStorage(SpecificationDecoratorBase):
             self._lock_release()
         return undo_serial, keys
 
+    @non_overridable
+    def new_instance(self):
+        """Implementation of IMVCCStorage.new_instance.
+
+        This method causes all storage instances to be wrapped with
+        a blob storage wrapper.
+        """
+        base_dir = self.fshelper.base_dir
+        s = getProxiedObject(self).new_instance()
+        res = BlobStorage(base_dir, s)
+        return res
+
 
 for name, v in BlobStorageMixin.__dict__.items():
     if isinstance(v, type(BlobStorageMixin.__dict__['storeBlob'])):

src/ZODB/tests/blob_layout.txt

@@ -106,9 +106,9 @@ already been used to create a lawn structure.
 'lawn'
 >>> shutil.rmtree('blobs')
 
-4. If the directory does not contain a marker but other files, we assume that
-it was created with an earlier version of the blob implementation and uses our
-`lawn` layout:
+4. If the directory does not contain a marker but other files that are
+not hidden, we assume that it was created with an earlier version of
+the blob implementation and uses our `lawn` layout:
 
 >>> os.mkdir('blobs')
 >>> open(os.path.join('blobs', '0x0101'), 'wb').write('foo')
@@ -116,6 +116,14 @@ it was created with an earlier version of the blob implementation and uses our
 'lawn'
 >>> shutil.rmtree('blobs')
 
+5. If the directory contains only hidden files, use the bushy layout:
+
+>>> os.mkdir('blobs')
+>>> open(os.path.join('blobs', '.svn'), 'wb').write('blah')
+>>> auto_layout_select('blobs')
+'bushy'
+>>> shutil.rmtree('blobs')
+
 
 Directory layout markers
 ========================

src/ZODB/tests/blob_packing.txt

@@ -37,7 +37,7 @@ Put some revisions of a blob object in our database and on the filesystem:
     >>> blob.open('w').write('this is blob data 0')
     >>> root['blob'] = blob
     >>> transaction.commit()
-    >>> tids.append(blob_storage._tid)
+    >>> tids.append(blob._p_serial)
 
     >>> nothing = transaction.begin()
     >>> times.append(new_time())

src/ZODB/tests/blob_transaction.txt

@@ -174,7 +174,7 @@ connections should result in a write conflict error::
     >>> tm2.commit()
     Traceback (most recent call last):
         ...
-    ConflictError: database conflict error (oid 0x01, class ZODB.blob.Blob)
+    ConflictError: database conflict error (oid 0x01, class ZODB.blob.Blob...)
 
 After the conflict, the winning transaction's result is visible on both
 connections::

src/ZODB/tests/testMVCCMappingStorage.py

@@ -18,7 +18,8 @@ from persistent.mapping import PersistentMapping
 import transaction
 from ZODB.DB import DB
 from ZODB.tests.MVCCMappingStorage import MVCCMappingStorage
-
+import ZODB.blob
+import ZODB.tests.testblob
 
 from ZODB.tests import (
     BasicStorage,
@@ -167,9 +168,20 @@ class MVCCMappingStorageTests(
         self._storage.tpc_begin(t)
         self.assertEqual(self._storage._tid, 'zzzzzzzz')
 
+def create_blob_storage(name, blob_dir):
+    s = MVCCMappingStorage(name)
+    return ZODB.blob.BlobStorage(blob_dir, s)
 
 def test_suite():
     suite = unittest.makeSuite(MVCCMappingStorageTests, 'check')
+    # Note: test_packing doesn't work because even though MVCCMappingStorage
+    # retains history, it does not provide undo methods, so the
+    # BlobStorage wrapper calls _packNonUndoing instead of _packUndoing,
+    # causing blobs to get deleted even though object states are retained.
+    suite.addTest(ZODB.tests.testblob.storage_reusable_suite(
+        'MVCCMapping', create_blob_storage,
+        test_undo=False,
+        ))
     return suite
 
 if __name__ == "__main__":

src/ZODB/tests/testblob.py

@@ -476,7 +476,7 @@ def loadblob_tmpstore():
     >>> import transaction
     >>> transaction.commit()
     >>> blob_oid = root['blob']._p_oid
-    >>> tid = blob_storage.lastTransaction()
+    >>> tid = connection._storage.lastTransaction()
 
     Now we open a database with a TmpStore in front:
 
@@ -556,6 +556,7 @@ def setUpBlobAdaptedFileStorage(test):
 def storage_reusable_suite(prefix, factory,
                            test_blob_storage_recovery=False,
                            test_packing=False,
+                           test_undo=True,
                            ):
     """Return a test suite for a generic IBlobStorage.
 
@@ -605,6 +606,7 @@ def storage_reusable_suite(prefix, factory,
 
     if test_blob_storage_recovery:
         add_test_based_on_test_class(RecoveryBlobStorage)
+    if test_undo:
         add_test_based_on_test_class(BlobUndoTests)
 
     suite.layer = ZODB.tests.util.MininalTestLayer(prefix+'BlobTests')