Replaced openDetatched method with a committed method that returns a

committed file name.

Make sure that uncommitted data gets cleaned up if w blob is GCed
without committing.

src/ZODB/Connection.py

@@ -38,6 +38,7 @@ from zope.interface import implements
 
 import transaction
 
+from ZODB.blob import SAVEPOINT_SUFFIX
 from ZODB.ConflictResolution import ResolvedSerial
 from ZODB.ExportImport import ExportImport
 from ZODB import POSException
@@ -616,7 +617,7 @@ class Connection(ExportImport, object):
                 if obj.opened():
                     raise ValueError("Can't commit with opened blobs.")
                 s = self._storage.storeBlob(oid, serial, p,
-                                            obj._p_blob_uncommitted,
+                                            obj._uncommitted(),
                                             self._version, transaction)
                 # we invalidate the object here in order to ensure
                 # that that the next attribute access of its name
@@ -1170,9 +1171,6 @@ class Savepoint:
     def rollback(self):
         self.datamanager._rollback(self.state)
 
-BLOB_SUFFIX = ".blob"
-BLOB_DIRTY = "store"
-
 class TmpStore:
     """A storage-like thing to support savepoints."""
 
@@ -1271,8 +1269,7 @@ class TmpStore:
 
     def _getCleanFilename(self, oid, tid):
         return os.path.join(self._getBlobPath(oid),
-                            "%s%s" % (utils.tid_repr(tid), 
-                                      BLOB_SUFFIX,)
+                            "%s%s" % (utils.tid_repr(tid), SAVEPOINT_SUFFIX,)
                             )
 
     def temporaryDirectory(self):

src/ZODB/blob.py

@@ -40,6 +40,7 @@ from zope.proxy.decorator import SpecificationDecoratorBase
 logger = logging.getLogger('ZODB.blob')
 
 BLOB_SUFFIX = ".blob"
+SAVEPOINT_SUFFIX = ".spb"
 
 valid_modes = 'r', 'w', 'r+', 'a'
 
@@ -85,9 +86,7 @@ class Blob(persistent.Persistent):
             if f is not None:
                 f.close()
 
-        if (self._p_blob_uncommitted
-            and os.path.exists(self._p_blob_uncommitted)
-            ):
+        if (self._p_blob_uncommitted):
             os.remove(self._p_blob_uncommitted)
 
         super(Blob, self)._p_invalidate()
@@ -159,6 +158,16 @@ class Blob(persistent.Persistent):
 
         return result
 
+    def committed(self):
+        if (self._p_blob_uncommitted
+            or
+            not self._p_blob_committed
+            or
+            self._p_blob_committed.endswith(SAVEPOINT_SUFFIX)
+            ):
+            raise BlobError('Uncommitted changes')
+        return self._p_blob_committed
+
     def openDetached(self, class_=file):
         """Returns a file(-like) object in read mode that can be used
         outside of transaction boundaries.
@@ -234,8 +243,22 @@ class Blob(persistent.Persistent):
             tempdir = self._p_jar.db()._storage.temporaryDirectory()
         else:
             tempdir = tempfile.gettempdir()
-        self._p_blob_uncommitted = utils.mktemp(dir=tempdir)
-        return self._p_blob_uncommitted
+        filename = utils.mktemp(dir=tempdir)
+        self._p_blob_uncommitted = filename
+
+        def cleanup(ref):
+            if os.path.exists(filename):
+                os.remove(filename)
+        
+        self._p_blob_ref = weakref.ref(self, cleanup)
+        return filename
+
+    def _uncommitted(self):
+        # hand uncommitted data to connection, relinquishing responsibility
+        # for it.
+        filename = self._p_blob_uncommitted
+        self._p_blob_uncommitted = self._p_blob_ref = None
+        return filename
 
 class BlobFile(file):
     """A BlobFile that holds a file handle to actual blob data.

src/ZODB/interfaces.py

@@ -910,6 +910,18 @@ class IBlob(Interface):
         mode: Mode to open the file with. Possible values: r,w,r+,a
         """
 
+    def committed():
+        """Return a file name for committed data.
+
+        The returned file name may be opened for reading or handed to
+        other processes for reading.  The file name isn't guarenteed
+        to be valid indefinately.  The file may be removed in the
+        future as a result of garbage collection depending on system
+        configuration.
+
+        A BlobError will be raised if the blob has any uncommitted data.
+        """
+
     def consumeFile(filename):
         """Consume a file.
 

src/ZODB/tests/blob_transaction.txt

@@ -263,7 +263,8 @@ Reading Blobs outside of a transaction
 --------------------------------------
 
 If you want to read from a Blob outside of transaction boundaries (e.g. to
-stream a file to the browser), you can use the openDetached() method::
+stream a file to the browser), committed method to get the name of a
+file that can be opened.
 
     >>> connection6 = database.open()
     >>> root6 = connection6.root()
@@ -273,60 +274,38 @@ stream a file to the browser), you can use the openDetached() method::
     >>> blob_fh.close()
     >>> root6['blob'] = blob
     >>> transaction.commit()
-    >>> blob.openDetached().read()
+    >>> open(blob.committed()).read()
     "I'm a happy blob."
 
-Of course, that doesn't work for empty blobs::
+An exception is raised if we call committed on a blob that has
+uncommitted changes:
 
     >>> blob = Blob()
-    >>> blob.openDetached()
+    >>> blob.committed()
     Traceback (most recent call last):
-        ...
-    BlobError: Blob does not exist.
-
-nor when the Blob is already opened for writing::
+    ...
+    BlobError: Uncommitted changes
 
-    >>> blob = Blob()
-    >>> blob_fh = blob.open("w")
-    >>> blob.openDetached()
+    >>> blob.open('w').write("I'm a happy blob.")
+    >>> root6['blob6'] = blob
+    >>> blob.committed()
     Traceback (most recent call last):
-        ...
-    BlobError: Already opened for writing.
-
-You can also pass a factory to the openDetached method that will be used to
-instantiate the file. This is used for e.g. creating filestream iterators::
-
-    >>> class customfile(file):
-    ...   pass
-    >>> blob_fh.write('Something')
-    >>> blob_fh.close()
-    >>> fh = blob.openDetached(customfile)
-    >>> fh  # doctest: +ELLIPSIS
-    <open file '...', mode 'rb' at 0x...>
-    >>> isinstance(fh, customfile)
-    True
-
-
-Note: Nasty people could use a factory that opens the file for writing. This
-would be evil.
+    ...
+    BlobError: Uncommitted changes
 
-It does work when the transaction was aborted, though::
+    >>> s = transaction.savepoint()
+    >>> blob.committed()
+    Traceback (most recent call last):
+    ...
+    BlobError: Uncommitted changes
 
-    >>> blob = Blob()
-    >>> blob_fh = blob.open("w")
-    >>> blob_fh.write("I'm a happy blob.")
-    >>> blob_fh.close()
-    >>> root6['blob'] = blob
     >>> transaction.commit()
-
-    >>> blob_fh = blob.open("w")
-    >>> blob_fh.write("And I'm singing.")
-    >>> blob_fh.close()
-    >>> transaction.abort()
-    >>> blob.openDetached().read()
+    >>> open(blob.committed()).read()
     "I'm a happy blob."
 
 
+
+
 Teardown
 --------
 

src/ZODB/tests/testblob.py

@@ -265,6 +265,19 @@ class BlobUndoTests(unittest.TestCase):
 
         database.close()
 
+def gc_blob_removes_uncommitted_data():
+    """
+    >>> from ZODB.blob import Blob
+    >>> blob = Blob()
+    >>> blob.open('w').write('x')
+    >>> fname = blob._p_blob_uncommitted
+    >>> os.path.exists(fname)
+    True
+    >>> blob = None
+    >>> os.path.exists(fname)
+    False
+    """
+
 def test_suite():
     suite = unittest.TestSuite()
     suite.addTest(unittest.makeSuite(ZODBBlobConfigTest))
@@ -275,12 +288,13 @@ def test_suite():
         setUp=ZODB.tests.util.setUp,
         tearDown=ZODB.tests.util.tearDown,
         ))
+    suite.addTest(doctest.DocTestSuite(
+        setUp=ZODB.tests.util.setUp,
+        tearDown=ZODB.tests.util.tearDown,
+        ))
     suite.addTest(unittest.makeSuite(BlobUndoTests))
 
     return suite
 
 if __name__ == '__main__':
     unittest.main(defaultTest = 'test_suite')
-
-
-