Merge recent changes (savepoint fixes) from 3.4 branch.

Original checkin comments follow:

r30168 | tim_one | 2005-04-25 14:17:37 -0400 (Mon, 25 Apr 2005) | 2 lines
   M /ZODB/branches/3.4/NEWS.txt
   ...

An internal 3.4a5 release, to incorporate savepoint fixes.

r30165 | jim | 2005-04-25 12:29:28 -0400 (Mon, 25 Apr 2005) | 11 lines
   M /ZODB/branches/3.4/src/transaction/_transaction.py
   M /ZODB/branches/3.4/src/transaction/savepoint.txt
   M /ZODB/branches/3.4/src/transaction/tests/test_savepoint.py

Fixed a bug in savepoint rollback.  It's not enough to rollback
just the savepoint being rolled back because later savepoints
might involved data managers that hadn't joined when the savepoint
being rolled back was created.

Now, when a data manager joins and we have savepoints, we create a
data manager savepoint for the new data manager and add the
datamanager savepoint to all previous transaction savepoints.  Note
that this data manager savepoint can be a special savepoint that just
calls abort on the data manager when it is rolled back.

r30164 | tim_one | 2005-04-25 11:16:20 -0400 (Mon, 25 Apr 2005) | 2 lines
r30163 | tim_one | 2005-04-25 11:08:37 -0400 (Mon, 25 Apr 2005) | 2 lines
r30162 | tim_one | 2005-04-25 11:06:51 -0400 (Mon, 25 Apr 2005) | 2 lines
   M /ZODB/branches/3.4/src/transaction/interfaces.py

Grammar, spelling, English.
Close unterminated sentences.
Trim trailing whitespace.

r30161 | jim | 2005-04-25 10:51:16 -0400 (Mon, 25 Apr 2005) | 10 lines
   M /ZODB/branches/3.4/src/transaction/interfaces.py

Removed a "self" argument. self is normally not shown in interfaces.

Removed the freeme argument.  This argument is part of the
implementation, not the public interface.

Removed the subtransaction argument.  Although it is still supported,
it isn't part of the pblic interface.

Added missing documentation of the savepoint method.

r30160 | jim | 2005-04-25 10:41:08 -0400 (Mon, 25 Apr 2005) | 2 lines
   M /ZODB/branches/3.4/src/transaction/interfaces.py

Removed some stale discussion of subtransactions.

r30147 | jim | 2005-04-24 11:26:39 -0400 (Sun, 24 Apr 2005) | 7 lines
   M /ZODB/branches/3.4/src/transaction/_transaction.py
   M /ZODB/branches/3.4/src/transaction/savepoint.txt

Make transactions uncommitable if savepoint rollback fails.

Added demonstration of transaction non-commitability after savepoint
or savepoint rollback failure.

Updated "previous commit failed" error to "previous operation failed".

r30146 | jim | 2005-04-24 11:26:37 -0400 (Sun, 24 Apr 2005) | 5 lines
   M /ZODB/branches/3.4/src/ZODB/Connection.py
   M /ZODB/branches/3.4/src/transaction/interfaces.py
   M /ZODB/branches/3.4/src/transaction/tests/savepointsample.py

Refined interfaces to distinguish between data-manager savepoints and
transaction savepoints.

Updated some interface declarations.

r30145 | jim | 2005-04-24 10:48:15 -0400 (Sun, 24 Apr 2005) | 2 lines
   M /ZODB/branches/3.4/src/ZODB/tests/testConnectionSavepoint.py

added explanatory text

r30144 | jim | 2005-04-24 10:35:49 -0400 (Sun, 24 Apr 2005) | 2 lines
Changed paths:
   M /ZODB/branches/3.4/NEWS.txt

Updated to reflect savepoints.

NEWS.txt

@@ -3,10 +3,29 @@ What's new in ZODB3 3.5a0?
 Release date: DD-MMM-YYYY
 
 
+What's new in ZODB3 3.4a5?
+==========================
+Release date: 25-Apr-2005
+
+This was an internal release, to fix problems with the new savepoint feature.
+
+
 What's new in ZODB3 3.4a4?
 ==========================
 Release date: 23-Apr-2005
 
+This was an internal release, to create a tag for use in Zope 2.8b1 and
+Zope3 development.
+
+transaction
+-----------
+
+Transactions now support savepoints.  Savepoints allow changes to be
+periodically checkpointed within a transaction.  You can then
+rollback to a previously created savepoint.  See
+``transaction/savepoint.txt``.
+
+
 ZEO
 ---
 
@@ -67,6 +86,12 @@ this out required adding ``last=`` everywhere, or removing it everywhere.
 Since recursion isn't actually needed, and there was no other use for
 ``last=``, removing it everywhere was the obvious choice.
 
+Support for ZODB4 savepoint-aware data managers has been dropped
+----------------------------------------------------------------
+
+In adding savepoint support, we dropped the attempted support
+for ZODB4 data managers that support savepoints.  We don't think that
+this will affect anyone.
 
 What's new in ZODB3 3.4a3?
 ==========================

src/ZODB/Connection.py

@@ -27,7 +27,7 @@ from persistent import PickleCache
 # interfaces
 from persistent.interfaces import IPersistentDataManager
 from ZODB.interfaces import IConnection
-from transaction.interfaces import IDataManager
+from transaction.interfaces import ISavepointDataManager, IDataManagerSavepoint
 from zope.interface import implements
 
 import transaction
@@ -59,7 +59,7 @@ def resetCaches():
 class Connection(ExportImport, object):
     """Connection to ZODB for loading and storing objects."""
 
-    implements(IConnection, IDataManager, IPersistentDataManager)
+    implements(IConnection, ISavepointDataManager, IPersistentDataManager)
 
     _storage = _normal_storage = _savepoint_storage = None
 
@@ -319,7 +319,7 @@ class Connection(ExportImport, object):
     ##########################################################################
 
     ##########################################################################
-    # Data manager (IDataManager) methods
+    # Data manager (ISavepointDataManager) methods
 
     def abort(self, transaction):
         """Abort a transaction and forget all changes."""
@@ -638,7 +638,7 @@ class Connection(ExportImport, object):
         """Return a consistent sort key for this connection."""
         return "%s:%s" % (self._storage.sortKey(), id(self))
 
-    # Data manager (IDataManager) methods
+    # Data manager (ISavepointDataManager) methods
     ##########################################################################
 
     ##########################################################################
@@ -1061,6 +1061,8 @@ class Connection(ExportImport, object):
 
 class Savepoint:
 
+    implements(IDataManagerSavepoint)
+
     def __init__(self, datamanager, state):
         self.datamanager = datamanager
         self.state = state

src/ZODB/tests/testConnectionSavepoint.py

@@ -20,7 +20,20 @@ from zope.testing import doctest
 import persistent.dict, transaction
 
 def testAddingThenModifyThenAbort():
-    """
+    """\
+
+We ran into a problem in which abort failed after adding an object in
+a savepoint and then modifying the object. The problem was that, on
+commit, the savepoint was aborted before the modifications were
+aborted.  Because the object was added in the savepoint, it's _p_oid
+and _p_jar were cleared when the savepoint was aborted.  The object
+was in the registered-object list.  There's an invariant for this
+lists that states that all objects in the list should have an oid and
+(correct) jar.
+
+The fix was to abort work done after he savepoint before aborting the
+savepoint.
+    
     >>> import ZODB.tests.util
     >>> db = ZODB.tests.util.DB()
     >>> connection = db.open()
@@ -35,7 +48,20 @@ def testAddingThenModifyThenAbort():
 """
 
 def testModifyThenSavePointThenModifySomeMoreThenCommit():
-    """
+    """\
+
+We got conflict errors when we committed after we modified an object
+in a savepoint and then modified it some more after the last
+savepoint.
+
+The problem was that we were effectively commiting the object twice --
+when commiting the current data and when committing the savepoint.
+The fix was to first make a new savepoint to move new changes to the
+savepoint storage and *then* to commit the savepoint storage. (This is
+similar to thr strategy that was used for subtransactions prior to
+savepoints.)
+
+
     >>> import ZODB.tests.util
     >>> db = ZODB.tests.util.DB()
     >>> connection = db.open()

src/transaction/_transaction.py

@@ -231,17 +231,17 @@ class Transaction(object):
 
     # Raise TransactionFailedError, due to commit()/join()/register()
     # getting called when the current transaction has already suffered
-    # a commit failure.
-    def _prior_commit_failed(self):
+    # a commit/savepoint failure.
+    def _prior_operation_failed(self):
         from ZODB.POSException import TransactionFailedError
         assert self._failure_traceback is not None
-        raise TransactionFailedError("commit() previously failed, "
-                "with this traceback:\n\n%s" %
+        raise TransactionFailedError("An operation previously failed, "
+                "with traceback:\n\n%s" %
                 self._failure_traceback.getvalue())
 
     def join(self, resource):
         if self.status is Status.COMMITFAILED:
-            self._prior_commit_failed() # doesn't return
+            self._prior_operation_failed() # doesn't return
 
         if self.status is not Status.ACTIVE:
             # TODO: Should it be possible to join a committing transaction?
@@ -261,15 +261,13 @@ class Transaction(object):
 
     def savepoint(self, optimistic=False):
         if self.status is Status.COMMITFAILED:
-            self._prior_commit_failed() # doesn't return, it raises
+            self._prior_operation_failed() # doesn't return, it raises
 
         try:
-            savepoint = Savepoint(optimistic)
-            for resource in self._resources:
-                savepoint.join(resource)
+            savepoint = Savepoint(self, optimistic, *self._resources)
         except:
             self._cleanup(self._resources)
-            self._saveCommitishError() # doesn't return, it raises!
+            self._saveCommitishError() # reraises!
             
         if self._last_savepoint is not None:
             savepoint.previous = self._last_savepoint
@@ -330,7 +328,7 @@ class Transaction(object):
             return
         
         if self.status is Status.COMMITFAILED:
-            self._prior_commit_failed() # doesn't return
+            self._prior_operation_failed() # doesn't return
 
         self._callBeforeCommitHooks()
 
@@ -598,13 +596,14 @@ class Savepoint:
     """
     interface.implements(interfaces.ISavepoint)
 
-    def __init__(self, optimistic):
-        self._savepoints = []
+    def __init__(self, transaction, optimistic, *resources):
+        self.transaction = transaction
+        self._savepoints = savepoints = []
         self.valid = True
         self.next = self.previous = None
         self.optimistic = optimistic
 
-    def join(self, datamanager):
+        for datamanager in resources:
             try:
                 savepoint = datamanager.savepoint
             except AttributeError:
@@ -614,14 +613,35 @@ class Savepoint:
             else:
                 savepoint = savepoint()
                 
+            savepoints.append(savepoint)
+    
+    def join(self, datamanager):
+        
+        # A data manager has joined a transaction *after* a savepoint
+        # was created.  A couple of things are different in this case:
+        
+        # 1. We need to add it's savepoint to all previous savepoints.
+        # so that if they are rolled back, we roll this was back too.
+        
+        # 2. We don't actualy need to ask it for a savepoint.  Because
+        # is just joining, then we can abort it if there is an error,
+        # so we use an AbortSavepoint.
+
+        savepoint = AbortSavepoint(datamanager, self.transaction)
+        while self is not None:
             self._savepoints.append(savepoint)
+            self = self.previous
 
     def rollback(self):
         if not self.valid:
             raise interfaces.InvalidSavepointRollbackError
         self._invalidate_next()
+        try:
             for savepoint in self._savepoints:
                 savepoint.rollback()
+        except:
+            # Mark the transaction as failed
+            self.transaction._saveCommitishError() # reraises!
 
     def _invalidate_next(self):
         self.valid = False
@@ -633,6 +653,15 @@ class Savepoint:
         if self.previous is not None:
             self.previous._invalidate_previous()
 
+class AbortSavepoint:
+
+    def __init__(self, datamanager, transaction):
+        self.datamanager = datamanager
+        self.transaction = transaction
+
+    def rollback(self):
+        self.datamanager.abort(self.transaction)
+
 class NoRollbackSavepoint:
 
     def __init__(self, datamanager):

src/transaction/interfaces.py

@@ -19,12 +19,11 @@ $Id$
 import zope.interface
 
 class ITransactionManager(zope.interface.Interface):
-    """An object that manages a sequence of transactions
+    """An object that manages a sequence of transactions.
 
     Applications use transaction managers to establish transaction boundaries.
     """
 
-
     def begin():
         """Begin a new transaction.
 
@@ -36,11 +35,21 @@ class ITransactionManager(zope.interface.Interface):
         """
 
     def commit():
-        """Commit the current transaction
+        """Commit the current transaction.
         """
 
-    def abort(self):
-        """Abort the current transaction
+    def abort():
+        """Abort the current transaction.
+        """
+
+    def savepoint(optimistic=False):
+        """Create a savepoint from the current transaction.
+
+        If the optimistic argument is true, then data managers that
+        don't support savepoints can be used, but an error will be
+        raised if the savepoint is rolled back.
+
+        An ISavepoint object is returned.
         """
 
     def registerSynch(synch):
@@ -48,7 +57,6 @@ class ITransactionManager(zope.interface.Interface):
 
         Synchronizers are notified at the beginning and end of
         transaction completion.
-        
         """
 
     def unregisterSynch(synch):
@@ -56,7 +64,6 @@ class ITransactionManager(zope.interface.Interface):
 
         Synchronizers are notified at the beginning and end of
         transaction completion.
-        
         """
 
 class ITransaction(zope.interface.Interface):
@@ -91,34 +98,43 @@ class ITransaction(zope.interface.Interface):
         raise an exception, or truncate the value).
         """)
 
-    def commit(subtransaction=None):
+    def commit():
         """Finalize the transaction.
 
         This executes the two-phase commit algorithm for all
         IDataManager objects associated with the transaction.
         """
 
-    def abort(subtransaction=0, freeme=1):
+    def abort():
         """Abort the transaction.
 
         This is called from the application.  This can only be called
         before the two-phase commit protocol has been started.
         """
 
+    def savepoint(optimistic=False):
+        """Create a savepoint.
+
+        If the optimistic argument is true, then data managers that don't
+        support savepoints can be used, but an error will be raised if the
+        savepoint is rolled back.
+
+        An ISavepoint object is returned.
+        """
+
     def join(datamanager):
         """Add a datamanager to the transaction.
 
-        The if the data manager supports savepoints, it must call this
-        *before* making any changes.  If the transaction has had any
-        savepoints, then it will take a savepoint of the data manager
-        when join is called and this savepoint must reflct the state
-        of the data manager before any changes that caused the data
-        manager to join the transaction.
+        If the data manager supports savepoints, it must call join *before*
+        making any changes:  if the transaction has made any savepoints, then
+        the transaction will take a savepoint of the data manager when join
+        is called, and this savepoint must reflect the state of the data
+        manager before any changes that caused the data manager to join the
+        transaction.
 
         The datamanager must implement the
         transactions.interfaces.IDataManager interface, and be
         adaptable to ZODB.interfaces.IDataManager.
-        
         """
 
     def note(text):
@@ -195,16 +211,12 @@ class IDataManager(zope.interface.Interface):
     """Objects that manage transactional storage.
 
     These objects may manage data for other objects, or they may manage
-    non-object storages, such as relational databases.
-
-    IDataManagerOriginal is the interface currently provided by ZODB
-    database connections, but the intent is to move to the newer
-    IDataManager.
+    non-object storages, such as relational databases.  For example,
+    a ZODB.Connection.
 
-    Note that when data are modified, data managers should join a
-    transaction so that data can be committed when the user commits
+    Note that when some data is modified, that data's data manager should
+    join a transaction so that data can be committed when the user commits
     the transaction.
-
     """
 
     # Two-phase commit protocol.  These methods are called by the
@@ -225,17 +237,6 @@ class IDataManager(zope.interface.Interface):
 
         transaction is the ITransaction instance associated with the
         transaction being committed.
-
-        subtransaction is a Boolean flag indicating whether the
-        two-phase commit is being invoked for a subtransaction.
-
-        Important note: Subtransactions are modelled in the sense that
-        when you commit a subtransaction, subsequent commits should be
-        for subtransactions as well.  That is, there must be a
-        commit_sub() call between a tpc_begin() call with the
-        subtransaction flag set to true and a tpc_begin() with the
-        flag set to false.
-
         """
 
     def commit(transaction):
@@ -263,7 +264,7 @@ class IDataManager(zope.interface.Interface):
         """
 
     def tpc_vote(transaction):
-        """Verify that a data manager can commit the transaction
+        """Verify that a data manager can commit the transaction.
 
         This is the last chance for a data manager to vote 'no'.  A
         data manager votes 'no' by raising an exception.
@@ -290,7 +291,7 @@ class IDataManager(zope.interface.Interface):
         """
 
     def sortKey():
-        """Return a key to use for ordering registered DataManagers
+        """Return a key to use for ordering registered DataManagers.
 
         ZODB uses a global sort order to prevent deadlock when it commits
         transactions involving multiple resource managers.  The resource
@@ -308,32 +309,47 @@ class IDataManager(zope.interface.Interface):
 class ISavepointDataManager(IDataManager):
 
     def savepoint():
-        """Return a savepoint (ISavepoint)
+        """Return a data-manager savepoint (IDataManagerSavepoint).
         """
 
-class ISavepoint(zope.interface.Interface):
-    """A transaction savepoint
+class IDataManagerSavepoint(zope.interface.Interface):
+    """Savepoint for data-manager changes for use in transaction savepoints.
+
+    Datamanager savepoints are used by, and only by, transaction savepoints.
+
+    Note that data manager savepoints don't have any notion of, or
+    responsibility for, validity.  It isn't the responsibility of
+    data-manager savepoints to prevent multiple rollbacks or rollbacks after
+    transaction termination.  Preventing invalid savepoint rollback is the
+    responsibility of transaction rollbacks. Application code should never
+    use data-manager savepoints.
     """
 
     def rollback():
-        """Rollback any work done since the savepoint
+        """Rollback any work done since the savepoint.
+        """
+
+class ISavepoint(zope.interface.Interface):
+    """A transaction savepoint.
+    """
 
-        An InvalidSavepointRollbackError is raised if the savepoint
-        isn't valid.
+    def rollback():
+        """Rollback any work done since the savepoint.
 
+        InvalidSavepointRollbackError is raised if the savepoint isn't valid.
         """
 
     valid = zope.interface.Attribute(
         "Boolean indicating whether the savepoint is valid")
 
 class InvalidSavepointRollbackError(Exception):
-    """Attempt to rollback an invalid savepoint
+    """Attempt to rollback an invalid savepoint.
 
     A savepoint may be invalid because:
 
-    - The surrounding transaction has committed or aborted
+    - The surrounding transaction has committed or aborted.
 
-    - An earlier savepoint in the same transaction has been rolled back
+    - An earlier savepoint in the same transaction has been rolled back.
     """
 
 class ISynchronizer(zope.interface.Interface):
@@ -347,4 +363,3 @@ class ISynchronizer(zope.interface.Interface):
     def afterCompletion(transaction):
         """Hook that is called by the transaction after completing a commit.
         """
-

src/transaction/savepoint.txt

@@ -201,7 +201,7 @@ support savepoints:
     >>> transaction.abort()
     
 However, a flag can be passed to the transaction savepoint method to
-indicate that databases without savepoint support should be tolderated
+indicate that databases without savepoint support should be tolerated
 until a savepoint is roled back.  This allows transactions to proceed
 is there are no reasons to roll back:
 
@@ -212,10 +212,64 @@ is there are no reasons to roll back:
     >>> dm_no_sp['name']
     'sue'
 
-    >>> savepoint = transaction.savepoint(1)
     >>> dm_no_sp['name'] = 'sam'
+    >>> savepoint = transaction.savepoint(1)
     >>> savepoint.rollback()
     Traceback (most recent call last):
     ...
     TypeError: ('Savepoints unsupported', {'name': 'sam'})
 
+Failures
+--------
+
+If a failure occurs when creating or rolling back a savepoint, the
+transaction state will be uncertain and the transaction will become
+uncommitable.  From that point on, most transaction operations,
+including commit, will fail until the transaction is aborted.
+
+In the previous example, we got an error when we tried to rollback the
+savepoint. If we try to commit the transaction, the commit will fail:
+
+    >>> transaction.commit() # doctest: +ELLIPSIS
+    Traceback (most recent call last):
+    ...
+    TransactionFailedError: An operation previously failed, with traceback:
+    ...
+    TypeError: ('Savepoints unsupported', {'name': 'sam'})
+    <BLANKLINE>
+
+We have to abort it to make any progress:
+
+    >>> transaction.abort()
+
+Similarly, in our earlier example, where we tried to take a savepoint
+with a data manager that didn't support savepoints:
+
+    >>> dm_no_sp['name'] = 'sally'
+    >>> dm['name'] = 'sally'
+    >>> savepoint = transaction.savepoint()
+    Traceback (most recent call last):
+    ...
+    TypeError: ('Savepoints unsupported', {'name': 'sue'})
+
+    >>> transaction.commit() # doctest: +ELLIPSIS
+    Traceback (most recent call last):
+    ...
+    TransactionFailedError: An operation previously failed, with traceback:
+    ...
+    TypeError: ('Savepoints unsupported', {'name': 'sue'})
+    <BLANKLINE>
+    
+    >>> transaction.abort()
+ 
+After clearing the transaction with an abort, we can get on with new
+transactions: 
+
+    >>> dm_no_sp['name'] = 'sally'
+    >>> dm['name'] = 'sally'
+    >>> transaction.commit()
+    >>> dm_no_sp['name']
+    'sally'
+    >>> dm['name']
+    'sally'
+    

src/transaction/tests/savepointsample.py

@@ -30,7 +30,7 @@ class SampleDataManager(UserDict.DictMixin):
     This data manager stores named simple values, like strings and numbers.
     """
     
-    interface.implements(transaction.interfaces.ISavepointDataManager)
+    interface.implements(transaction.interfaces.IDataManager)
 
     def __init__(self, transaction_manager = None):
         if transaction_manager is None:
@@ -156,6 +156,8 @@ class SampleSavepointDataManager(SampleDataManager):
     This extends the basic data manager with savepoint support.
     """
 
+    interface.implements(transaction.interfaces.ISavepointDataManager)
+
     def savepoint(self):
         # When we create the savepoint, we save the existing database state
         return SampleSavepoint(self, self.uncommitted.copy())
@@ -166,6 +168,8 @@ class SampleSavepointDataManager(SampleDataManager):
 
 class SampleSavepoint:
 
+    interface.implements(transaction.interfaces.IDataManagerSavepoint)
+
     def __init__(self, data_manager, data):
         self.data_manager = data_manager
         self.data = data

src/transaction/tests/test_savepoint.py

@@ -19,9 +19,49 @@ import unittest
 from zope.testing import doctest
 
 
+def testRollbackRollsbackDataManagersThatJoinedLater():
+    """
+
+A savepoint needs to not just rollback it's savepoints, but needs to
+rollback savepoints for data managers that joined savepoints after the
+savepoint:
+
+    >>> import transaction.tests.savepointsample
+    >>> dm = transaction.tests.savepointsample.SampleSavepointDataManager()
+    >>> dm['name'] = 'bob'
+    >>> sp1 = transaction.savepoint()
+    >>> dm['job'] = 'geek'
+    >>> sp2 = transaction.savepoint()
+    >>> dm['salary'] = 'fun'    
+    >>> dm2 = transaction.tests.savepointsample.SampleSavepointDataManager()
+    >>> dm2['name'] = 'sally'
+
+    >>> 'name' in dm
+    True
+    >>> 'job' in dm
+    True
+    >>> 'salary' in dm
+    True
+    >>> 'name' in dm2
+    True
+
+    >>> sp1.rollback()
+
+    >>> 'name' in dm
+    True
+    >>> 'job' in dm
+    False
+    >>> 'salary' in dm
+    False
+    >>> 'name' in dm2
+    False
+
+"""
+
 def test_suite():
     return unittest.TestSuite((
         doctest.DocFileSuite('../savepoint.txt'),
+        doctest.DocTestSuite(),
         ))
 
 if __name__ == '__main__':