SVN: ZODB/branches/jim-clean-up-tpc/src/Z Clean up tpc semantics to allow multiple tpc_begin and tpc_finish calls for the same transaction.

Log message for revision 107022:
  Clean up tpc semantics to allow multiple tpc_begin and tpc_finish calls for the same transaction.

Changed:
  U   ZODB/branches/jim-clean-up-tpc/src/ZEO/ClientStorage.py
  U   ZODB/branches/jim-clean-up-tpc/src/ZEO/tests/ConnectionTests.py
  U   ZODB/branches/jim-clean-up-tpc/src/ZODB/BaseStorage.py
  U   ZODB/branches/jim-clean-up-tpc/src/ZODB/DemoStorage.py
  U   ZODB/branches/jim-clean-up-tpc/src/ZODB/MappingStorage.py
  U   ZODB/branches/jim-clean-up-tpc/src/ZODB/tests/BasicStorage.py
  U   ZODB/branches/jim-clean-up-tpc/src/ZODB/tests/Synchronization.py

-=-
Modified: ZODB/branches/jim-clean-up-tpc/src/ZEO/ClientStorage.py
===================================================================
--- ZODB/branches/jim-clean-up-tpc/src/ZEO/ClientStorage.py	2009-12-23 21:18:21 UTC (rev 107021)
+++ ZODB/branches/jim-clean-up-tpc/src/ZEO/ClientStorage.py	2009-12-23 21:18:56 UTC (rev 107022)
@@ -1087,17 +1087,19 @@
         if self._is_read_only:
             raise POSException.ReadOnlyError()
         self._tpc_cond.acquire()
-        self._midtxn_disconnect = 0
-        while self._transaction is not None:
-            # It is allowable for a client to call two tpc_begins in a
-            # row with the same transaction, and the second of these
-            # must be ignored.
-            if self._transaction == txn:
-                self._tpc_cond.release()
-                return
-            self._tpc_cond.wait(30)
-        self._transaction = txn
-        self._tpc_cond.release()
+        try:
+            self._midtxn_disconnect = 0
+            while self._transaction is not None:
+                # It is allowable for a client to call two tpc_begins in a
+                # row with the same transaction, and the second of these
+                # must be ignored.
+                if self._transaction is txn:
+                    raise POSException.StorageTransactionError(
+                        "Multiple calls to tpc_begin with same transaction")
+                self._tpc_cond.wait(30)
+            self._transaction = txn
+        finally:
+            self._tpc_cond.release()
 
         try:
             self._server.tpc_begin(id(txn), txn.user, txn.description,
@@ -1148,7 +1150,8 @@
     def tpc_finish(self, txn, f=None):
         """Storage API: finish a transaction."""
         if txn is not self._transaction:
-            return
+            raise POSException.StorageTransactionError(
+                "tpc_finish called with wrong transaction.")
         self._load_lock.acquire()
         try:
             if self._midtxn_disconnect:

Modified: ZODB/branches/jim-clean-up-tpc/src/ZEO/tests/ConnectionTests.py
===================================================================
--- ZODB/branches/jim-clean-up-tpc/src/ZEO/tests/ConnectionTests.py	2009-12-23 21:18:21 UTC (rev 107021)
+++ ZODB/branches/jim-clean-up-tpc/src/ZEO/tests/ConnectionTests.py	2009-12-23 21:18:56 UTC (rev 107022)
@@ -1093,7 +1093,6 @@
         self.assertRaises(ConflictError, storage.tpc_vote, t)
         # Even aborting won't help.
         storage.tpc_abort(t)
-        storage.tpc_finish(t)
         # Try again.
         obj.value = 10
         t = Transaction()
@@ -1103,7 +1102,6 @@
         self.assertRaises(ConflictError, storage.tpc_vote, t)
         # Abort this one and try a transaction that should succeed.
         storage.tpc_abort(t)
-        storage.tpc_finish(t)
         # Now do a store.
         obj.value = 11
         t = Transaction()

Modified: ZODB/branches/jim-clean-up-tpc/src/ZODB/BaseStorage.py
===================================================================
--- ZODB/branches/jim-clean-up-tpc/src/ZODB/BaseStorage.py	2009-12-23 21:18:21 UTC (rev 107021)
+++ ZODB/branches/jim-clean-up-tpc/src/ZODB/BaseStorage.py	2009-12-23 21:18:56 UTC (rev 107022)
@@ -221,7 +221,8 @@
         self._lock_acquire()
         try:
             if self._transaction is transaction:
-                return
+                raise POSException.StorageTransactionError(
+                    "Multiple calls to tpc_begin with same transaction")
             self._lock_release()
             self._commit_lock_acquire()
             self._lock_acquire()
@@ -284,7 +285,8 @@
         self._lock_acquire()
         try:
             if transaction is not self._transaction:
-                return
+                raise POSException.StorageTransactionError(
+                    "tpc_finish called with wrong transaction.")
             try:
                 if f is not None:
                     f(self._tid)

Modified: ZODB/branches/jim-clean-up-tpc/src/ZODB/DemoStorage.py
===================================================================
--- ZODB/branches/jim-clean-up-tpc/src/ZODB/DemoStorage.py	2009-12-23 21:18:21 UTC (rev 107021)
+++ ZODB/branches/jim-clean-up-tpc/src/ZODB/DemoStorage.py	2009-12-23 21:18:56 UTC (rev 107022)
@@ -45,7 +45,7 @@
         else:
             self._temporary_base = False
         self.base = base
-            
+
         if changes is None:
             changes = ZODB.MappingStorage.MappingStorage()
             zope.interface.alsoProvides(self, ZODB.interfaces.IBlobStorage)
@@ -82,7 +82,7 @@
             self.changes = ZODB.blob.BlobStorage(blob_dir, self.changes)
             self._copy_methods_from_changes(self.changes)
             return True
-    
+
     def cleanup(self):
         self.base.cleanup()
         self.changes.cleanup()
@@ -95,7 +95,7 @@
 
     def _copy_methods_from_changes(self, changes):
         for meth in (
-            '_lock_acquire', '_lock_release', 
+            '_lock_acquire', '_lock_release',
             'getSize', 'history', 'isReadOnly', 'registerDB',
             'sortKey', 'tpc_transaction', 'tpc_vote',
             ):
@@ -230,7 +230,7 @@
             raise TypeError(
                 "Garbage collection isn't supported"
                 " when there is a base storage.")
-        
+
         try:
             self.changes.pack(t, referencesf, gc=False)
         except TypeError, v:
@@ -262,7 +262,7 @@
                 old = self.base.load(oid, '')[1]
             except ZODB.POSException.POSKeyError:
                 old = serial
-                
+
         if old != serial:
             raise ZODB.POSException.ConflictError(
                 oid=oid, serials=(old, serial)) # XXX untested branch
@@ -309,7 +309,8 @@
     def tpc_begin(self, transaction, *a, **k):
         # The tid argument exists to support testing.
         if transaction is self._transaction:
-            return
+            raise ZODB.POSException.StorageTransactionError(
+                "Multiple calls to tpc_begin with same transaction")
         self._lock_release()
         self._commit_lock.acquire()
         self._lock_acquire()
@@ -320,7 +321,9 @@
     @ZODB.utils.locked
     def tpc_finish(self, transaction, func = lambda tid: None):
         if (transaction is not self._transaction):
-            return
+            raise ZODB.POSException.StorageTransactionError(
+                "tpc_finish called with wrong transaction.")
+
         self._issued_oids.difference_update(self._stored_oids)
         self._stored_oids = set()
         self._transaction = None
@@ -330,7 +333,7 @@
 _temporary_blobdirs = {}
 def cleanup_temporary_blobdir(
     ref,
-    _temporary_blobdirs=_temporary_blobdirs, # Make sure it stays around 
+    _temporary_blobdirs=_temporary_blobdirs, # Make sure it stays around
     ):
     blob_dir = _temporary_blobdirs.pop(ref, None)
     if blob_dir and os.path.exists(blob_dir):

Modified: ZODB/branches/jim-clean-up-tpc/src/ZODB/MappingStorage.py
===================================================================
--- ZODB/branches/jim-clean-up-tpc/src/ZODB/MappingStorage.py	2009-12-23 21:18:21 UTC (rev 107021)
+++ ZODB/branches/jim-clean-up-tpc/src/ZODB/MappingStorage.py	2009-12-23 21:18:56 UTC (rev 107022)
@@ -28,7 +28,7 @@
 import zope.interface
 
 class MappingStorage(object):
-    
+
     zope.interface.implements(
         ZODB.interfaces.IStorage,
         ZODB.interfaces.IStorageIteration,
@@ -50,7 +50,7 @@
 
     ######################################################################
     # Preconditions:
-    
+
     def opened(self):
         """The storage is open
         """
@@ -94,7 +94,7 @@
         if tid_data:
             return tid_data.maxKey()
         raise ZODB.POSException.POSKeyError(oid)
-        
+
     # ZODB.interfaces.IStorage
     @ZODB.utils.locked(opened)
     def history(self, oid, size=1):
@@ -124,7 +124,7 @@
     def iterator(self, start=None, end=None):
         for transaction_record in self._transactions.values(start, end):
             yield transaction_record
-        
+
     # ZODB.interfaces.IStorage
     @ZODB.utils.locked(opened)
     def lastTransaction(self):
@@ -165,7 +165,7 @@
         else:
             raise ZODB.POSException.POSKeyError(oid)
 
-            
+
     # ZODB.interfaces.IStorage
     @ZODB.utils.locked(opened)
     def loadSerial(self, oid, serial):
@@ -189,7 +189,7 @@
     def pack(self, t, referencesf, gc=True):
         if not self._data:
             return
-        
+
         stop = `ZODB.TimeStamp.TimeStamp(*time.gmtime(t)[:5]+(t%60,))`
         if self._last_pack is not None and self._last_pack >= stop:
             if self._last_pack == stop:
@@ -274,7 +274,8 @@
     def tpc_begin(self, transaction, tid=None):
         # The tid argument exists to support testing.
         if transaction is self._transaction:
-            return
+            raise ZODB.POSException.StorageTransactionError(
+                "Multiple calls to tpc_begin with same transaction")
         self._lock_release()
         self._commit_lock.acquire()
         self._lock_acquire()
@@ -292,7 +293,8 @@
     @ZODB.utils.locked(opened)
     def tpc_finish(self, transaction, func = lambda tid: None):
         if (transaction is not self._transaction):
-            return
+            raise ZODB.POSException.StorageTransactionError(
+                "tpc_finish called with wrong transaction.")
 
         tid = self._tid
         func(tid)
@@ -310,7 +312,7 @@
         self._transaction = None
         del self._tdata
         self._commit_lock.release()
- 
+
     # ZEO.interfaces.IServeable
     @ZODB.utils.locked(opened)
     def tpc_transaction(self):

Modified: ZODB/branches/jim-clean-up-tpc/src/ZODB/tests/BasicStorage.py
===================================================================
--- ZODB/branches/jim-clean-up-tpc/src/ZODB/tests/BasicStorage.py	2009-12-23 21:18:21 UTC (rev 107021)
+++ ZODB/branches/jim-clean-up-tpc/src/ZODB/tests/BasicStorage.py	2009-12-23 21:18:56 UTC (rev 107022)
@@ -34,8 +34,8 @@
     def checkBasics(self):
         t = transaction.Transaction()
         self._storage.tpc_begin(t)
-        # This should simply return
-        self._storage.tpc_begin(t)
+        self.assertRaises(POSException.StorageTransactionError,
+                          self._storage.tpc_begin, t)
         # Aborting is easy
         self._storage.tpc_abort(t)
         # Test a few expected exceptions when we're doing operations giving a

Modified: ZODB/branches/jim-clean-up-tpc/src/ZODB/tests/Synchronization.py
===================================================================
--- ZODB/branches/jim-clean-up-tpc/src/ZODB/tests/Synchronization.py	2009-12-23 21:18:21 UTC (rev 107021)
+++ ZODB/branches/jim-clean-up-tpc/src/ZODB/tests/Synchronization.py	2009-12-23 21:18:56 UTC (rev 107022)
@@ -99,19 +99,22 @@
 
     def checkFinishNotCommitting(self):
         t = Transaction()
-        self._storage.tpc_finish(t)
+        self.assertRaises(StorageTransactionError,
+                          self._storage.tpc_finish, t)
         self._storage.tpc_abort(t)
 
     def checkFinishWrongTrans(self):
         t = Transaction()
         self._storage.tpc_begin(t)
-        self._storage.tpc_finish(Transaction())
+        self.assertRaises(StorageTransactionError,
+                          self._storage.tpc_finish, Transaction())
         self._storage.tpc_abort(t)
 
     def checkBeginCommitting(self):
         t = Transaction()
         self._storage.tpc_begin(t)
-        self._storage.tpc_begin(t)
+        self.assertRaises(StorageTransactionError,
+                          self._storage.tpc_begin, t)
         self._storage.tpc_abort(t)
 
     # TODO:  how to check undo?