diff --git a/django/core/cache/backends/db.py b/django/core/cache/backends/db.py index f18b05db2f..7749284122 100644 --- a/django/core/cache/backends/db.py +++ b/django/core/cache/backends/db.py @@ -109,7 +109,7 @@ class DatabaseCache(BaseDatabaseCache): if six.PY3: b64encoded = b64encoded.decode('latin1') try: - with transaction.atomic_if_autocommit(using=db): + with transaction.atomic(using=db): cursor.execute("SELECT cache_key, expires FROM %s " "WHERE cache_key = %%s" % table, [key]) result = cursor.fetchone() diff --git a/django/db/backends/__init__.py b/django/db/backends/__init__.py index c797a50733..27b39a3d1f 100644 --- a/django/db/backends/__init__.py +++ b/django/db/backends/__init__.py @@ -50,15 +50,16 @@ class BaseDatabaseWrapper(object): # set somewhat aggressively, as the DBAPI doesn't make it easy to # deduce if the connection is in transaction or not. self._dirty = False - # Tracks if the connection is in a transaction managed by 'atomic' + # Tracks if the connection is in a transaction managed by 'atomic'. self.in_atomic_block = False + # Tracks if the outermost 'atomic' block should commit on exit, + # ie. if autocommit was active on entry. + self.commit_on_exit = True # Tracks if the transaction should be rolled back to the next # available savepoint because of an exception in an inner block. self.needs_rollback = False # List of savepoints created by 'atomic' self.savepoint_ids = [] - # Hack to provide compatibility with legacy transaction management - self._atomic_forced_unmanaged = False # Connection termination related attributes self.close_at = None diff --git a/django/db/transaction.py b/django/db/transaction.py index 1ff1a8437e..0aed0aa4f4 100644 --- a/django/db/transaction.py +++ b/django/db/transaction.py @@ -206,18 +206,6 @@ class Atomic(object): self.using = using self.savepoint = savepoint - def _legacy_enter_transaction_management(self, connection): - if not connection.in_atomic_block: - if connection.transaction_state and connection.transaction_state[-1]: - connection._atomic_forced_unmanaged = True - connection.enter_transaction_management(managed=False) - else: - connection._atomic_forced_unmanaged = False - - def _legacy_leave_transaction_management(self, connection): - if not connection.in_atomic_block and connection._atomic_forced_unmanaged: - connection.leave_transaction_management() - def __enter__(self): connection = get_connection(self.using) @@ -225,12 +213,31 @@ class Atomic(object): # autocommit status. connection.ensure_connection() - # Remove this when the legacy transaction management goes away. - self._legacy_enter_transaction_management(connection) - - if not connection.in_atomic_block and not connection.autocommit: - raise TransactionManagementError( - "'atomic' cannot be used when autocommit is disabled.") + if not connection.in_atomic_block: + # Reset state when entering an outermost atomic block. + connection.commit_on_exit = True + connection.needs_rollback = False + if not connection.autocommit: + # Some database adapters (namely sqlite3) don't handle + # transactions and savepoints properly when autocommit is off. + # Turning autocommit back on isn't an option; it would trigger + # a premature commit. Give up if that happens. + if connection.features.autocommits_when_autocommit_is_off: + raise TransactionManagementError( + "Your database backend doesn't behave properly when " + "autocommit is off. Turn it on before using 'atomic'.") + # When entering an atomic block with autocommit turned off, + # Django should only use savepoints and shouldn't commit. + # This requires at least a savepoint for the outermost block. + if not self.savepoint: + raise TransactionManagementError( + "The outermost 'atomic' block cannot use " + "savepoint = False when autocommit is off.") + # Pretend we're already in an atomic block to bypass the code + # that disables autocommit to enter a transaction, and make a + # note to deal with this case in __exit__. + connection.in_atomic_block = True + connection.commit_on_exit = False if connection.in_atomic_block: # We're already in a transaction; create a savepoint, unless we @@ -255,63 +262,58 @@ class Atomic(object): else: connection.set_autocommit(False) connection.in_atomic_block = True - connection.needs_rollback = False def __exit__(self, exc_type, exc_value, traceback): connection = get_connection(self.using) - if exc_value is None and not connection.needs_rollback: - if connection.savepoint_ids: - # Release savepoint if there is one - sid = connection.savepoint_ids.pop() - if sid is not None: + + if connection.savepoint_ids: + sid = connection.savepoint_ids.pop() + else: + # Prematurely unset this flag to allow using commit or rollback. + connection.in_atomic_block = False + + try: + if exc_value is None and not connection.needs_rollback: + if connection.in_atomic_block: + # Release savepoint if there is one + if sid is not None: + try: + connection.savepoint_commit(sid) + except DatabaseError: + connection.savepoint_rollback(sid) + raise + else: + # Commit transaction try: - connection.savepoint_commit(sid) + connection.commit() except DatabaseError: - connection.savepoint_rollback(sid) - # Remove this when the legacy transaction management goes away. - self._legacy_leave_transaction_management(connection) + connection.rollback() raise else: - # Commit transaction - connection.in_atomic_block = False - try: - connection.commit() - except DatabaseError: - connection.rollback() - # Remove this when the legacy transaction management goes away. - self._legacy_leave_transaction_management(connection) - raise - finally: - if connection.features.autocommits_when_autocommit_is_off: - connection.autocommit = True + # This flag will be set to True again if there isn't a savepoint + # allowing to perform the rollback at this level. + connection.needs_rollback = False + if connection.in_atomic_block: + # Roll back to savepoint if there is one, mark for rollback + # otherwise. + if sid is None: + connection.needs_rollback = True else: - connection.set_autocommit(True) - else: - # This flag will be set to True again if there isn't a savepoint - # allowing to perform the rollback at this level. - connection.needs_rollback = False - if connection.savepoint_ids: - # Roll back to savepoint if there is one, mark for rollback - # otherwise. - sid = connection.savepoint_ids.pop() - if sid is None: - connection.needs_rollback = True + connection.savepoint_rollback(sid) else: - connection.savepoint_rollback(sid) - else: - # Roll back transaction - connection.in_atomic_block = False - try: + # Roll back transaction connection.rollback() - finally: - if connection.features.autocommits_when_autocommit_is_off: - connection.autocommit = True - else: - connection.set_autocommit(True) - - # Remove this when the legacy transaction management goes away. - self._legacy_leave_transaction_management(connection) + finally: + # Outermost block exit when autocommit was enabled. + if not connection.in_atomic_block: + if connection.features.autocommits_when_autocommit_is_off: + connection.autocommit = True + else: + connection.set_autocommit(True) + # Outermost block exit when autocommit was disabled. + elif not connection.savepoint_ids and not connection.commit_on_exit: + connection.in_atomic_block = False def __call__(self, func): @wraps(func, assigned=available_attrs(func)) @@ -331,24 +333,6 @@ def atomic(using=None, savepoint=True): return Atomic(using, savepoint) -def atomic_if_autocommit(using=None, savepoint=True): - # This variant only exists to support the ability to disable transaction - # management entirely in the DATABASES setting. It doesn't care about the - # autocommit state at run time. - db = DEFAULT_DB_ALIAS if callable(using) else using - autocommit = get_connection(db).settings_dict['AUTOCOMMIT'] - - if autocommit: - return atomic(using, savepoint) - else: - # Bare decorator: @atomic_if_autocommit - if callable(using): - return using - # Decorator: @atomic_if_autocommit(...) - else: - return lambda func: func - - ############################################ # Deprecated decorators / context managers # ############################################ @@ -472,16 +456,15 @@ def commit_on_success_unless_managed(using=None, savepoint=False): Transitory API to preserve backwards-compatibility while refactoring. Once the legacy transaction management is fully deprecated, this should - simply be replaced by atomic_if_autocommit. Until then, it's necessary to - avoid making a commit where Django didn't use to, since entering atomic in - managed mode triggers a commmit. + simply be replaced by atomic. Until then, it's necessary to guarantee that + a commit occurs on exit, which atomic doesn't do when it's nested. Unlike atomic, savepoint defaults to False because that's closer to the legacy behavior. """ connection = get_connection(using) if connection.autocommit or connection.in_atomic_block: - return atomic_if_autocommit(using, savepoint) + return atomic(using, savepoint) else: def entering(using): pass diff --git a/docs/topics/db/transactions.txt b/docs/topics/db/transactions.txt index b8017e8bfa..122af14359 100644 --- a/docs/topics/db/transactions.txt +++ b/docs/topics/db/transactions.txt @@ -153,9 +153,6 @@ Django provides a single API to control database transactions. to commit, roll back, or change the autocommit state of the database connection within an ``atomic`` block will raise an exception. - ``atomic`` can only be used in autocommit mode. It will raise an exception - if autocommit is turned off. - Under the hood, Django's transaction management code: - opens a transaction when entering the outermost ``atomic`` block; @@ -171,6 +168,10 @@ Django provides a single API to control database transactions. the overhead of savepoints is noticeable. It has the drawback of breaking the error handling described above. + You may use ``atomic`` when autocommit is turned off. It will only use + savepoints, even for the outermost block, and it will raise an exception + if the outermost block is declared with ``savepoint=False``. + .. admonition:: Performance considerations Open transactions have a performance cost for your database server. To @@ -271,9 +272,8 @@ another. Review the documentation of the adapter you're using carefully. You must ensure that no transaction is active, usually by issuing a :func:`commit` or a :func:`rollback`, before turning autocommit back on. -:func:`atomic` requires autocommit to be turned on; it will raise an exception -if autocommit is off. Django will also refuse to turn autocommit off when an -:func:`atomic` block is active, because that would break atomicity. +Django will refuse to turn autocommit off when an :func:`atomic` block is +active, because that would break atomicity. Transactions ------------ @@ -392,8 +392,11 @@ When autocommit is enabled, savepoints don't make sense. When it's disabled, commits before any statement other than ``SELECT``, ``INSERT``, ``UPDATE``, ``DELETE`` and ``REPLACE``.) -As a consequence, savepoints are only usable inside a transaction ie. inside -an :func:`atomic` block. +This has two consequences: + +- The low level APIs for savepoints are only usable inside a transaction ie. + inside an :func:`atomic` block. +- It's impossible to use :func:`atomic` when autocommit is turned off. Transactions in MySQL --------------------- @@ -584,9 +587,6 @@ During the deprecation period, it's possible to use :func:`atomic` within However, the reverse is forbidden, because nesting the old decorators / context managers breaks atomicity. -If you enter :func:`atomic` while you're in managed mode, it will trigger a -commit to start from a clean slate. - Managing autocommit ~~~~~~~~~~~~~~~~~~~ @@ -623,8 +623,8 @@ Disabling transaction management ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Instead of setting ``TRANSACTIONS_MANAGED = True``, set the ``AUTOCOMMIT`` key -to ``False`` in the configuration of each database, as explained in :ref -:`deactivate-transaction-management`. +to ``False`` in the configuration of each database, as explained in +:ref:`deactivate-transaction-management`. Backwards incompatibilities --------------------------- diff --git a/tests/transactions/tests.py b/tests/transactions/tests.py index e53a320481..aeb9bc3d2c 100644 --- a/tests/transactions/tests.py +++ b/tests/transactions/tests.py @@ -6,7 +6,7 @@ import warnings from django.db import connection, transaction, IntegrityError from django.test import TransactionTestCase, skipUnlessDBFeature from django.utils import six -from django.utils.unittest import skipUnless +from django.utils.unittest import skipIf, skipUnless from .models import Reporter @@ -197,6 +197,23 @@ class AtomicInsideTransactionTests(AtomicTests): self.atomic.__exit__(*sys.exc_info()) +@skipIf(connection.features.autocommits_when_autocommit_is_off, + "This test requires a non-autocommit mode that doesn't autocommit.") +class AtomicWithoutAutocommitTests(AtomicTests): + """All basic tests for atomic should also pass when autocommit is turned off.""" + + def setUp(self): + transaction.set_autocommit(False) + + def tearDown(self): + # The tests access the database after exercising 'atomic', initiating + # a transaction ; a rollback is required before restoring autocommit. + transaction.rollback() + transaction.set_autocommit(True) + + +@skipIf(connection.features.autocommits_when_autocommit_is_off, + "This test requires a non-autocommit mode that doesn't autocommit.") class AtomicInsideLegacyTransactionManagementTests(AtomicTests): def setUp(self): @@ -268,16 +285,7 @@ class AtomicMergeTests(TransactionTestCase): "'atomic' requires transactions and savepoints.") class AtomicErrorsTests(TransactionTestCase): - def test_atomic_requires_autocommit(self): - transaction.set_autocommit(False) - try: - with self.assertRaises(transaction.TransactionManagementError): - with transaction.atomic(): - pass - finally: - transaction.set_autocommit(True) - - def test_atomic_prevents_disabling_autocommit(self): + def test_atomic_prevents_setting_autocommit(self): autocommit = transaction.get_autocommit() with transaction.atomic(): with self.assertRaises(transaction.TransactionManagementError):