Add a test for loadBlob.

src/ZEO/ClientStorage.py

@@ -314,6 +314,11 @@ class ClientStorage(object):
         # is executing.
         self._lock = threading.Lock()
 
+        # XXX need to check for POSIX-ness here
+        if (os.stat(blob_dir).st_mode & 077) != 0:
+            log2('Blob dir %s has insecure mode setting' % blob_dir,
+                 level=logging.WARNING)
+
         self.blob_dir = blob_dir
 
         # Initialize locks
@@ -950,7 +955,8 @@ class ClientStorage(object):
             tempfile.write(chunk)
 
         tempfile.close()
-        utils.best_rename(tempfilename, blob_filename)
+        # XXX will fail on Windows if file is open
+        os.rename(tempfilename, blob_filename)
         return blob_filename
 
     def loadBlob(self, oid, serial, version):
@@ -968,7 +974,7 @@ class ClientStorage(object):
         blob_filename = self._getCleanFilename(oid, serial)
         # Case 1: Blob is available already, just use it
         if os.path.exists(blob_filename):
-                return blob_filename
+            return blob_filename
 
         # Case 2,3: Blob might still be downloading or not there yet
 
@@ -981,7 +987,7 @@ class ClientStorage(object):
         self.blob_status_lock.acquire()
         try:
             if not self.blob_status.has_key(oid):
-                self.blob_status[lock_key] = Lock()
+                self.blob_status[lock_key] = self.getBlobLock()
             lock = self.blob_status[lock_key]
         finally:
             self.blob_status_lock.release()
@@ -1006,10 +1012,13 @@ class ClientStorage(object):
             # making the creation of this status lock non-atomic (see above)
             self.blob_status_lock.acquire()
             try:
-                del self.blob_status_lock[lock_key]
+                del self.blob_status[lock_key]
             finally:
                 self.blob_status_lock.release()
-        
+
+    def getBlobLock(self):
+        # indirection to support unit testing
+        return Lock()
 
     def tpc_vote(self, txn):
         """Storage API: vote on a transaction."""

src/ZEO/tests/testZEO.py

@@ -252,6 +252,57 @@ class BlobAdaptedFileStorageTests(GenericTests):
         self.assert_(os.path.exists(filename))
         self.assertEqual(somedata, open(filename).read())
         
+    def checkLoadBlob(self):
+        from ZODB.tests.StorageTestBase import zodb_pickle, ZERO, \
+             handle_serials
+        oid = self._storage.new_oid()
+        serial = ZERO
+        version = ''
+        filename = self._storage._getCleanFilename(oid, serial)
+
+        class Dummy:
+            def __init__(self):
+                self.acquired = 0
+                self.released = 0
+            def acquire(self):
+                self.acquired += 1
+            def release(self):
+                self.released += 1
+
+        class statusdict(dict):
+            def __init__(self):
+                self.added = []
+                self.removed = []
+                
+            def __setitem__(self, k, v):
+                self.added.append(k)
+                super(dict, self).__setitem__(k, v)
+
+            def __delitem__(self, k):
+                self.removed.append(k)
+                super(dict, self).__delitem__(k)
+
+        # ensure that we do locking properly
+        thestatuslock = self._storage.blob_status_lock = Dummy()
+        thebloblock = Dummy()
+
+        def getBlobLock(self):
+            return thebloblock
+
+        # override getBlobLock to test that locking is performed
+        self._storage.getBlobLock = getBlobLock
+        thestatusdict = self._storage.blob_status = statusdict()
+
+        filename = self._storage.loadBlob(oid, serial, version)
+
+        self.assertEqual(thestatuslock.acquired, 2)
+        self.assertEqual(thestatuslock.released, 2)
+        
+        self.assertEqual(thebloblock.acquired, 1)
+        self.assertEqual(thebloblock.released, 1)
+
+        self.assertEqual(thestatusdict.added, (oid, serial))
+        self.assertEqual(thestatusdict.removed, (oid, serial))
 
 test_classes = [FileStorageTests, MappingStorageTests,
                 BlobAdaptedFileStorageTests]