Commit eb3efdfb authored by Tim Peters's avatar Tim Peters

Merge rev 27279 from 3.3 branch.

Transaction.begin() didn't do anything.

begin() is supposed to abort the current transaction, but
Transaction.begin() did not.  Calling begin() on a transaction
*manager* worked fine, and is the intended way to do a begin()
in 3.3.  But calling begin() on a Transaction object is still
very easy to do (e.g., the older get_transaction().begin()
spelling still works), and shouldn't be a subtle disaster.
parent 2cc74713
...@@ -2,6 +2,25 @@ What's new in ZODB3 3.3 ? ...@@ -2,6 +2,25 @@ What's new in ZODB3 3.3 ?
========================= =========================
Release date: DD-MMM-YYYY Release date: DD-MMM-YYYY
transaction
-----------
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 BTrees
------ ------
......
...@@ -344,6 +344,77 @@ class ZODBTests(unittest.TestCase): ...@@ -344,6 +344,77 @@ class ZODBTests(unittest.TestCase):
self.obj = DecoyIndependent() self.obj = DecoyIndependent()
self.readConflict() self.readConflict()
def checkTxnBeginImpliesAbort(self):
# begin() should do an abort() first, if needed.
cn = self._db.open()
rt = cn.root()
rt['a'] = 1
transaction.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.begin()
rt = cn.root()
rt['a'] = 2
transaction.commit(1)
transaction.begin()
rt = cn.root()
self.assertRaises(KeyError, rt.__getitem__, 'a')
# One more time, mixing "top level" and subtransaction changes.
transaction.begin()
rt = cn.root()
rt['a'] = 3
transaction.commit(1)
rt['b'] = 4
transaction.begin()
rt = cn.root()
self.assertRaises(KeyError, rt.__getitem__, 'a')
self.assertRaises(KeyError, rt.__getitem__, 'b')
# That used methods of the default transaction *manager*. Alas,
# that's not necessarily the same as using methods of the current
# 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')
# 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()
def test_suite(): def test_suite():
return unittest.makeSuite(ZODBTests, 'check') return unittest.makeSuite(ZODBTests, 'check')
......
...@@ -230,11 +230,14 @@ class Transaction(object): ...@@ -230,11 +230,14 @@ class Transaction(object):
self._resources.append(adapter) self._resources.append(adapter)
def begin(self): def begin(self):
# TODO: I'm not sure how this should be implemented. Not doing if (self._resources or
# anything now, but my best guess is: If nothing has happened self._sub or
# yet, it's fine. Otherwise, abort this transaction and let self._nonsub or
# the txn manager create a new one. self._synchronizers):
pass self.abort()
# Else aborting wouldn't do anything, except if _manager is non-None,
# in which case it would do nothing besides uselessly free() this
# transaction.
def commit(self, subtransaction=False): def commit(self, subtransaction=False):
if not subtransaction and self._sub and self._resources: if not subtransaction and self._sub and self._resources:
...@@ -247,8 +250,6 @@ class Transaction(object): ...@@ -247,8 +250,6 @@ class Transaction(object):
if not subtransaction: if not subtransaction:
for s in self._synchronizers: for s in self._synchronizers:
s.beforeCompletion(self) s.beforeCompletion(self)
if not subtransaction:
self.status = Status.COMMITTING self.status = Status.COMMITTING
self._commitResources(subtransaction) self._commitResources(subtransaction)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment