Merge pull request #43 from zopefoundation/explicit-mode

Explicit mode

CHANGES.rst

 Changes
 =======
 
+2.1.0 (unreleased)
+------------------
+
+Added a transaction-manager explicit mode. Explicit mode makes some
+kinds of application bugs easier to detect and potentially allows data
+managers to manage resources more efficiently.
+
+(This addresses https://github.com/zopefoundation/transaction/issues/35.)
+
 2.0.3 (2016-11-17)
 ------------------
 

docs/index.rst

@@ -86,6 +86,45 @@ commit.  It calls afterCompletion() when a top-level transaction is
 committed or aborted.  The methods are passed the current Transaction
 as their only argument.
 
+Explicit vs implicit transactions
+---------------------------------
+
+By default, transactions are implicitly managed.  Calling ``begin()``
+on a transaction manager implicitly aborts the previous transaction
+and calling ``commit()`` or ``abort()`` implicitly begins a new
+one. This behavior can be convenient for interactive use, but invites
+subtle bugs:
+
+- Calling begin() without realizing that there are outstanding changes
+  that will be aborted.
+
+- Interacting with a database without controlling transactions, in
+  which case changes may be unexpectedly discarded.
+
+For applications, including frameworks that control transactions,
+transaction managers provide an optional explicit mode.  Transaction
+managers have an ``explicit`` constructor keyword argument that, if
+True puts the transaction manager in explicit mode.  In explicit mode:
+
+- It is an error to call ``get()``, ``commit()``, ``abort()``,
+  ``doom()``, ``isDoomed``, or ``savepoint()`` without a preceding
+  ``begin()`` call.  Doing so will raise a ``NoTransaction``
+  exception.
+
+- It is an error to call ``begin()`` after a previous ``begin()``
+  without an intervening ``commit()`` or ``abort()`` call.  Doing so
+  will raise an ``AlreadyInTransaction`` exception.
+
+In explicit mode, bugs like those mentioned above are much easier to
+avoid because they cause explicit exceptions that can typically be
+caught in development.
+
+An additional benefit of explicit mode is that it can allow data
+managers to manage resources more efficiently.
+
+Transaction managers have an explicit attribute that can be queried to
+determine if explicit mode is enabled.
+
 Contents:
 
 .. toctree::

transaction/_manager.py

@@ -21,7 +21,9 @@ import threading
 
 from zope.interface import implementer
 
+from transaction.interfaces import AlreadyInTransaction
 from transaction.interfaces import ITransactionManager
+from transaction.interfaces import NoTransaction
 from transaction.interfaces import TransientError
 from transaction.weakset import WeakSet
 from transaction._compat import reraise
@@ -59,7 +61,8 @@ def _new_transaction(txn, synchs):
 @implementer(ITransactionManager)
 class TransactionManager(object):
 
-    def __init__(self):
+    def __init__(self, explicit=False):
+        self.explicit = explicit
         self._txn = None
         self._synchs = WeakSet()
 
@@ -67,6 +70,8 @@ class TransactionManager(object):
         """ See ITransactionManager.
         """
         if self._txn is not None:
+            if self.explicit:
+                raise AlreadyInTransaction()
             self._txn.abort()
         txn = self._txn = Transaction(self._synchs, self)
         _new_transaction(txn, self._synchs)
@@ -78,6 +83,8 @@ class TransactionManager(object):
         """ See ITransactionManager.
         """
         if self._txn is None:
+            if self.explicit:
+                raise NoTransaction()
             self._txn = Transaction(self._synchs, self)
         return self._txn
 

transaction/interfaces.py

@@ -21,40 +21,70 @@ class ITransactionManager(Interface):
     Applications use transaction managers to establish transaction boundaries.
     """
 
+    explicit = Attribute(
+        """Explicit mode indicator.
+
+        This is true if the transaction manager is in explicit mode.
+        In explicit mode, transactions must be begun explicitly, by
+        calling ``begin()`` and ended explicitly by calling
+        ``commit()`` or ``abort()``.
+        """)
+
+
     def begin():
-        """Explicitly begin a new transaction.
+        """Explicitly begin and return a new transaction.
 
-        If an existing transaction is in progress, it will be aborted.
+        If an existing transaction is in progress and the transaction
+        manager not in explicit mode, the previous transaction will be
+        aborted.  If an existing transaction is in progress and the
+        transaction manager is in explicit mode, an
+        ``AlreadyInTransaction`` exception will be raised..
 
         The ``newTransaction`` method of registered synchronizers is called,
         passing the new transaction object.
 
-        Note that transactions may be started implicitly without
-        calling ``begin``. In that case, ``newTransaction`` isn't
-        called because the transaction manager doesn't know when to
-        call it.  The transaction is likely to have begun long before
-        the transaction manager is involved. (Conceivably the ``commit`` and
-        ``abort`` methods could call ``begin``, but they don't.)
+        Note that when not in explicit mode, transactions may be
+        started implicitly without calling ``begin``. In that case,
+        ``newTransaction`` isn't called because the transaction
+        manager doesn't know when to call it.  The transaction is
+        likely to have begun long before the transaction manager is
+        involved. (Conceivably the ``commit`` and ``abort`` methods
+        could call ``begin``, but they don't.)
         """
 
     def get():
         """Get the current transaction.
+
+        In explicit mode, if a transaction hasn't begun, a
+        ``NoTransaction`` exception will be raised.
         """
 
     def commit():
         """Commit the current transaction.
+
+        In explicit mode, if a transaction hasn't begun, a
+        ``NoTransaction`` exception will be raised.
         """
 
     def abort():
         """Abort the current transaction.
+
+        In explicit mode, if a transaction hasn't begun, a
+        ``NoTransaction`` exception will be raised.
         """
 
     def doom():
         """Doom the current transaction.
+
+        In explicit mode, if a transaction hasn't begun, a
+        ``NoTransaction`` exception will be raised.
         """
 
     def isDoomed():
         """Returns True if the current transaction is doomed, otherwise False.
+
+        In explicit mode, if a transaction hasn't begun, a
+        ``NoTransaction`` exception will be raised.
         """
 
     def savepoint(optimistic=False):
@@ -65,6 +95,9 @@ class ITransactionManager(Interface):
         raised if the savepoint is rolled back.
 
         An ISavepoint object is returned.
+
+        In explicit mode, if a transaction hasn't begun, a
+        ``NoTransaction`` exception will be raised.
         """
 
     def registerSynch(synch):
@@ -525,3 +558,20 @@ class TransientError(TransactionError):
 
     It's possible that retrying the transaction will succeed.
     """
+
+class NoTransaction(TransactionError):
+    """No transaction has been defined
+
+    An application called an operation on a transaction manager that
+    affects an exciting transaction, but no transaction was begun.
+    The transaction manager was in explicit mode, so a new transaction
+    was not explicitly created.
+    """
+
+class AlreadyInTransaction(TransactionError):
+    """Attempt to create a new transaction without ending a preceding one
+
+    An application called ``begin()`` on a transaction manager in
+    explicit mode, without committing or aborting the previous
+    transaction.
+    """

transaction/tests/test__manager.py

@@ -711,6 +711,42 @@ class AttemptTests(unittest.TestCase):
         self.assertFalse(manager.committed)
         self.assertTrue(manager.aborted)
 
+    def test_explicit_mode(self):
+        from .. import TransactionManager
+        from ..interfaces import AlreadyInTransaction, NoTransaction
+
+        tm = TransactionManager()
+        self.assertFalse(tm.explicit)
+
+        tm = TransactionManager(explicit=True)
+        self.assertTrue(tm.explicit)
+        for name in 'get', 'commit', 'abort', 'doom', 'isDoomed', 'savepoint':
+            with self.assertRaises(NoTransaction):
+                getattr(tm, name)()
+
+        t = tm.begin()
+        with self.assertRaises(AlreadyInTransaction):
+            tm.begin()
+
+        self.assertTrue(t is tm.get())
+
+        self.assertFalse(tm.isDoomed())
+        tm.doom()
+        self.assertTrue(tm.isDoomed())
+        tm.abort()
+
+        for name in 'get', 'commit', 'abort', 'doom', 'isDoomed', 'savepoint':
+            with self.assertRaises(NoTransaction):
+                getattr(tm, name)()
+
+        t = tm.begin()
+        self.assertFalse(tm.isDoomed())
+        with self.assertRaises(AlreadyInTransaction):
+            tm.begin()
+        tm.savepoint()
+        tm.commit()
+
+
 
 class DummyManager(object):
     entered = False