[Zodb-checkins] SVN: ZODB/branches/ctheune-blobszerocopy/src/ZODB/Blobs/ - providing test and code for first edge case of error handling

Christian Theune ct at gocept.com
Wed Mar 7 17:43:06 EST 2007


Log message for revision 73041:
   - providing test and code for first edge case of error handling
  
  

Changed:
  U   ZODB/branches/ctheune-blobszerocopy/src/ZODB/Blobs/Blob.py
  U   ZODB/branches/ctheune-blobszerocopy/src/ZODB/Blobs/tests/consume.txt

-=-
Modified: ZODB/branches/ctheune-blobszerocopy/src/ZODB/Blobs/Blob.py
===================================================================
--- ZODB/branches/ctheune-blobszerocopy/src/ZODB/Blobs/Blob.py	2007-03-07 21:43:31 UTC (rev 73040)
+++ ZODB/branches/ctheune-blobszerocopy/src/ZODB/Blobs/Blob.py	2007-03-07 22:43:06 UTC (rev 73041)
@@ -38,6 +38,9 @@
  
     zope.interface.implements(IBlob)
 
+    # Binding this to an attribute allows overriding it in the unit tests
+    _os_link = os.link
+
     _p_blob_readers = 0
     _p_blob_writers = 0
     _p_blob_uncommitted = None  # Filename of the uncommitted (dirty) data
@@ -56,9 +59,6 @@
 
     def open(self, mode="r"):
         """Returns a file(-like) object representing blob data."""
-
-        tempdir = os.environ.get('ZODB_BLOB_TEMPDIR', tempfile.gettempdir())
-
         result = None
 
         if (mode.startswith("r") or mode=="U"):
@@ -76,7 +76,9 @@
                 raise BlobError("Already opened for reading.")
 
             self._p_blob_writers += 1
-            result = BlobFile(self._get_uncommitted_filename(), mode, self)
+            if self._p_blob_uncommitted is None:
+                self._create_uncommitted_file()
+            result = BlobFile(self._p_blob_uncommitted, mode, self)
 
         elif mode.startswith("a"):
             if self._p_blob_readers != 0:
@@ -84,13 +86,13 @@
 
             if self._p_blob_uncommitted is None:
                 # Create a new working copy
-                uncommitted = BlobFile(self._get_uncommitted_filename(), mode, self)
+                uncommitted = BlobFile(self._create_uncommitted_file(), mode, self)
                 # NOTE: _p_blob data appears by virtue of Connection._setstate
                 utils.cp(file(self._p_blob_data), uncommitted)
                 uncommitted.seek(0)
             else:
                 # Re-use existing working copy
-                uncommitted = BlobFile(self._get_uncommitted_filename(), mode, self)
+                uncommitted = BlobFile(self._p_blob_uncommitted, mode, self)
 
             self._p_blob_writers += 1
             result = uncommitted
@@ -150,14 +152,43 @@
             raise BlobError("Already opened for writing.")
         if self._p_blob_readers != 0:
             raise BlobError("Already opened for reading.")
-        target = self._get_uncommitted_filename()
-        # XXX What if link fails and the target was removed? We should do a rename and
-        #  maybe name it back if link gives an exception.
-        if os.path.exists(target):
+
+        previous_uncommitted = bool(self._p_blob_uncommitted)
+        if previous_uncommitted:
+            # If we have uncommitted data, we move it aside for now
+            # in case the consumption doesn't work.
+            target = self._p_blob_uncommitted
+            target_aside = target+".aside"
+            os.rename(target, target_aside)
+        else:
+            target = self._create_uncommitted_file()
+            # We need to unlink the freshly created target again
+            # to allow link() to do its job
             os.unlink(target)
-        # XXX what if link() fails
-        os.link(filename, target)
 
+        try:
+            self._os_link(filename, target)
+        except:
+            # Recover from the failed consumption: First remove the file, it
+            # might exist and mark the pointer to the uncommitted file.
+            self._p_blob_uncommitted = None
+            if os.path.exists(target):
+                os.unlink(target)
+
+            # If there was a file moved aside, bring it back including the pointer to
+            # the uncommitted file.
+            if previous_uncommitted:
+                os.rename(target_aside, target)
+                self._p_blob_uncommitted = target
+
+            # Re-raise the exception to make the application aware of it.
+            raise
+        else:
+            if previous_uncommitted:
+                # The relinking worked so we can remove the data that we had 
+                # set aside.
+                os.unlink(target_aside)
+
     # utility methods
 
     def _current_filename(self):
@@ -165,14 +196,10 @@
         # Connection._setstate
         return self._p_blob_uncommitted or self._p_blob_data
 
-    def _get_uncommitted_filename(self):
-        """Return the filename for existing uncommitted data
-        or generate a new filename and set it as the current filename
-        for uncomitted data.
-        """
-        if self._p_blob_uncommitted is None:
-            tempdir = os.environ.get('ZODB_BLOB_TEMPDIR', tempfile.gettempdir())
-            self._p_blob_uncommitted = utils.mktemp(dir=tempdir)
+    def _create_uncommitted_file(self):
+        assert self._p_blob_uncommitted is None, "Uncommitted file already exists."
+        tempdir = os.environ.get('ZODB_BLOB_TEMPDIR', tempfile.gettempdir())
+        self._p_blob_uncommitted = utils.mktemp(dir=tempdir)
         return self._p_blob_uncommitted
 
     def _change(self):

Modified: ZODB/branches/ctheune-blobszerocopy/src/ZODB/Blobs/tests/consume.txt
===================================================================
--- ZODB/branches/ctheune-blobszerocopy/src/ZODB/Blobs/tests/consume.txt	2007-03-07 21:43:31 UTC (rev 73040)
+++ ZODB/branches/ctheune-blobszerocopy/src/ZODB/Blobs/tests/consume.txt	2007-03-07 22:43:06 UTC (rev 73041)
@@ -65,3 +65,40 @@
     >>> blob_read = blob.open('r')
     >>> blob_read.read()
     'I am another blob.'
+
+
+Edge cases
+==========
+
+There are some edge cases what happens when the link() operation fails. We simulate this in different states:
+
+Case 1: We don't have uncommitted data, but the link operation fails. The
+exception will be re-raised and the target file will not exist::
+
+    >>> input = tempfile.NamedTemporaryFile()
+    >>> input.write('Some data')
+    >>> input.flush()
+
+    >>> def failing_link(self, filename):
+    ...   raise Exception("I can't link.")
+
+    >>> blob = Blob()
+    >>> blob.open('r')
+    Traceback (most recent call last):
+    BlobError: Blob does not exist.
+
+    >>> blob._os_link = failing_link
+    >>> blob.consumeFile(input.name)
+    Traceback (most recent call last):
+    Exception: I can't link.
+
+The blob did not exist before, so it shouldn't exist now::
+
+    >>> blob.open('r')
+    Traceback (most recent call last):
+    BlobError: Blob does not exist.
+
+Case 2: We thave uncommitted data, but the link operation fails. The
+exception will be re-raised and the target file will not exist::
+
+    2. uncommitted data exist, the link fails, the uncommitted data is still there



More information about the Zodb-checkins mailing list