Merge from 3.3 branch.

Officially deprecate Transaction.begin().

NEWS.txt

@@ -20,12 +20,12 @@ Release date: DD-MMM-YYYY
 Connection
 ----------
 
-ZODB intends to raise ConnnectionStateError if an attempt is made to
-close a connection while modifications are pending (the connection is
-involved in a transaction that hasn't been abort()'ed or commit()'ed).
-It was missing the case where the only pending modifications were made
-in subtransactions.  This has been fixed.  If an attempt to close a
-connection with pending subtransactions is made now,
+ZODB intends to raise ConnnectionStateError if an attempt is made to close
+a connection while modifications are pending (the connection is involved in
+a transaction that hasn't been ``abort()``'ed or ``commit()``'ed). It was
+missing the case where the only pending modifications were made in
+subtransactions.  This has been fixed.  If an attempt to close a connection
+with pending subtransactions is made now::
 
     ConnnectionStateError: Cannot close a connection with a pending subtransaction
 
@@ -34,27 +34,48 @@ is raised.
 transaction
 -----------
 
-If ReadConflictError was raised by an attempt to load an object with a
-_p_independent() method that returned false, attempting to commit the
-transaction failed to (re)raise ReadConflictError for that object.  Note
-that ZODB intends to prevent committing a transaction in which a
-ReadConflictError occurred; this was an obscure case it missed.
+- Some explanations of new transaction features in the 3.3a3 news
+  were incorrect, and this news file has been retroactively edited to
+  repair that.  See news for 3.3a3 below.
+
+- If ReadConflictError was raised by an attempt to load an object with a
+  ``_p_independent()`` method that returned false, attempting to commit the
+  transaction failed to (re)raise ReadConflictError for that object.  Note
+  that ZODB intends to prevent committing a transaction in which a
+  ReadConflictError occurred; this was an obscure case it missed.
+
+- Growing pains:  ZODB 3.2 had a bug wherein ``Transaction.begin()`` didn't
+  abort the current transaction if the only pending changes were in a
+  subtransaction.  In ZODB 3.3, it's intended that a transaction manager be
+  used to effect ``begin()`` (instead of invoking ``Transaction.begin()``),
+  and calling ``begin()`` on a transaction manager didn't have this old
+  bug.  However, ``Transaction.begin()`` still exists in 3.3, and it had a
+  worse bug:  it never aborted the transaction (not even if changes were
+  pending outside of subtransactions). ``Transaction.begin()`` has been
+  changed to abort the transaction. ``Transaction.begin()`` is also
+  deprecated.  Don't use it.  Use ``begin()`` on the relevant transaction
+  manager instead.  For example,
+
+      >>> import transaction
+      >>> txn = transaction.begin()  # start a txn using the default TM
+
+  if using the default ThreadTransactionManager (see news for 3.3a3 below).
+  In 3.3, it's intended that a single Transaction object is used for exactly
+  one transaction.  So, unlike as in 3.2, when somtimes Transaction objects
+  were reused across transactions, but sometimes weren't, when you do
+  ``Transaction.begin()`` in 3.3 a brand new transaction object is
+  created.  That's why this use is deprecated.  Code of the form:
+
+      >>> txn = transaction.get()
+      >>> ...
+      >>> txn.begin()
+      >>> ...
+      >>> txn.commit()
+
+  can't work as intended is 3.3, because ``txn`` is no longer the current
+  Transaction object the instant ``txn.begin()`` returns.
 
-Growing pains:  ZODB 3.1 and 3.2 had a bug wherein Transaction.begin()
-didn't abort the current transaction if the only pending changes were in a
-subtransaction.  In ZODB 3.3, it's intended that transaction managers be
-used instead of invoking methods directly on Transaction objects, and
-calling begin() on a transaction manager didn't have this old bug.  However,
-Transaction.begin() still exists in 3.3, and it had a worse bug:  it never
-aborted the transaction (not even if changes were pending outside of
-subtransactions).  Transaction.begin() has been changed to abort the
-transaction, although it's still strongly recommended to invoke begin() on
-the relevant transaction manager instead.  For example,
 
-    import transaction
-    transaction.begin()
-
-if using the default ThreadTransactionManager (see news for 3.3a3 below).
 
 BTrees
 ------
@@ -270,9 +291,9 @@ There is a new transaction package, which provides new interfaces for
 application code and for the interaction between transactions and
 resource managers.
 
-The top-level transaction package has functions commit(), abort(),
-get(), and begin().  They should be used instead of the magic
-get_transaction() builtin, which will be deprecated.  For example:
+The top-level transaction package has functions ``commit()``, ``abort()``,
+``get()``, and ``begin()``.  They should be used instead of the magic
+``get_transaction()`` builtin, which will be deprecated.  For example:
 
     >>> get_transaction().commit()
 
@@ -281,38 +302,38 @@ should now be written as
     >>> import transaction
     >>> transaction.commit()
 
-The new API provides explicit transaction manager objects.  The
-transaction manager (TM) is responsible for associating resource
-managers with a "current" transaction.  It is available as
-`transaction.manager`.  The default TM, implemented by
-ThreadedTransactionManager, assigns each thread its own current
-transaction.  The TransactionManager class assigns all threads to the
-same transaction.
+The new API provides explicit transaction manager objects.  A transaction
+manager (TM) is responsible for associating resource managers with a
+"current" transaction.  The default TM, implemented by class
+``ThreadedTransactionManager``, assigns each thread its own current
+transaction.  This default TM is available as ``transaction.manager``.  The
+``TransactionManager`` class assigns all threads to the same transaction,
+and is an explicit replacement for the ``Connection.setLocalTransaction()``
+method:
 
-A transaction manager instance can be passed as the txn_mgr argument
-to DB.open().  If you do, the connection will use the specified
-transaction manager instead of the default transaction manager.  You
-will need to call commit() and abort() on the transaction manager
-explicitly.  For example:
+A transaction manager instance can be passed as the txn_mgr argument to
+``DB.open()``.  If you do, the connection will use the specified
+transaction manager instead of the default TM.  The current transaction is
+obtained by calling ``get()`` on a TM. For example:
 
     >>> tm = transaction.TransactionManager()
     >>> cn = db.open(txn_mgr=tm)
     [...]
-    >>> tm.commit()
+    >>> tm.get().commit()
 
-The setLocalTransaction() and getTransaction() methods of Connection
-are deprecated.  Use an explicit TM passed via txn_mgr instead.  The
-setLocalTransaction() manager functions still works, but it returns a
-TM instead of a Transaction.
+The ``setLocalTransaction()`` and ``getTransaction()`` methods of
+Connection are deprecated.  Use an explicit TM passed via ``txn_mgr=`` to
+``DB.open()`` instead.  The ``setLocalTransaction()`` method still works,
+but it returns a TM instead of a Transaction.
 
-The TM creates Transaction objects, which are used for exactly one
-transaction.  They have a status() method that returns their current
-state.
+A TM creates Transaction objects, which are used for exactly one
+transaction.  Transaction objects still have ``commit()``, ``abort()``,
+``note()``, ``setUser()``, and ``setExtendedInfo()`` methods.
 
-Resource managers, e.g. Connection or RDB adapter, should use join()
-instead of register().  An object that calls join() manages its own
-resources.  An object that calls register() expects the TM to manage
-the objects.
+Resource managers, e.g. Connection or RDB adapter, should use a
+Transaction's ``join()`` method instead of its ``register()`` method.  An
+object that calls ``join()`` manages its own resources.  An object that
+calls ``register()`` expects the TM to manage the objects.
 
 Data managers written against the ZODB 4 transaction API are now
 supported in ZODB 3.

src/ZODB/tests/testZODB.py

@@ -386,38 +386,57 @@ class ZODBTests(unittest.TestCase):
         # transaction, and, in fact, when this test was written,
         # Transaction.begin() didn't do anything (everything from here
         # down failed).
-        cn = self._db.open()
-        rt = cn.root()
-        rt['a'] = 1
-
-        transaction.get().begin()  # should abort adding 'a' to the root
-        rt = cn.root()
-        self.assertRaises(KeyError, rt.__getitem__, 'a')
-
-        # A longstanding bug:  this didn't work if changes were only in
-        # subtransactions.
-        transaction.get().begin()
-        rt = cn.root()
-        rt['a'] = 2
-        transaction.get().commit(1)
 
-        transaction.get().begin()
-        rt = cn.root()
-        self.assertRaises(KeyError, rt.__getitem__, 'a')
+        # Oh, bleech.  Since Transaction.begin is also deprecated, we have
+        # to goof around suppressing the deprecation warning.
+        import warnings
 
-        # One more time, mixing "top level" and subtransaction changes.
-        transaction.get().begin()
-        rt = cn.root()
-        rt['a'] = 3
-        transaction.get().commit(1)
-        rt['b'] = 4
+        # First verify that Transaction.begin *is* deprecated, by turning
+        # the warning into an error.
+        warnings.filterwarnings("error", category=DeprecationWarning)
+        self.assertRaises(DeprecationWarning, transaction.get().begin)
+        del warnings.filters[0]
 
-        transaction.get().begin()
-        rt = cn.root()
-        self.assertRaises(KeyError, rt.__getitem__, 'a')
-        self.assertRaises(KeyError, rt.__getitem__, 'b')
+        # Now ignore DeprecationWarnings for the duration.  Use a
+        # try/finally block to ensure we reenable DeprecationWarnings
+        # no matter what.
+        warnings.filterwarnings("ignore", category=DeprecationWarning)
+        try:
+            cn = self._db.open()
+            rt = cn.root()
+            rt['a'] = 1
+
+            transaction.get().begin()  # should abort adding 'a' to the root
+            rt = cn.root()
+            self.assertRaises(KeyError, rt.__getitem__, 'a')
+
+            # A longstanding bug:  this didn't work if changes were only in
+            # subtransactions.
+            transaction.get().begin()
+            rt = cn.root()
+            rt['a'] = 2
+            transaction.get().commit(1)
+
+            transaction.get().begin()
+            rt = cn.root()
+            self.assertRaises(KeyError, rt.__getitem__, 'a')
+
+            # One more time, mixing "top level" and subtransaction changes.
+            transaction.get().begin()
+            rt = cn.root()
+            rt['a'] = 3
+            transaction.get().commit(1)
+            rt['b'] = 4
+
+            transaction.get().begin()
+            rt = cn.root()
+            self.assertRaises(KeyError, rt.__getitem__, 'a')
+            self.assertRaises(KeyError, rt.__getitem__, 'b')
+
+            cn.close()
 
-        cn.close()
+        finally:
+            del warnings.filters[0]
 
 def test_suite():
     return unittest.makeSuite(ZODBTests, 'check')

src/transaction/_transaction.py

@@ -136,6 +136,7 @@ XXX This code isn't tested.
 import logging
 import sys
 import thread
+import warnings
 
 _marker = object()
 
@@ -230,6 +231,9 @@ class Transaction(object):
                 self._resources.append(adapter)
 
     def begin(self):
+        warnings.warn("Transaction.begin() should no longer be used; use "
+                      "the begin() method of a transaction manager.",
+                      DeprecationWarning)
         if (self._resources or
               self._sub or
               self._nonsub or