From 7aacde84f2b499d9c35741cbfccb621af6b48903 Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Sat, 2 Mar 2013 20:25:25 +0100 Subject: [PATCH 01/32] Made transaction.managed a no-op and deprecated it. enter_transaction_management() was nearly always followed by managed(). In three places it wasn't, but they will all be refactored eventually. The "forced" keyword argument avoids introducing behavior changes until then. This is mostly backwards-compatible, except, of course, for managed itself. There's a minor difference in _enter_transaction_management: the top self.transaction_state now contains the new 'managed' state rather than the previous one. Django doesn't access self.transaction_state in _enter_transaction_management. --- django/core/management/commands/loaddata.py | 1 - django/db/backends/__init__.py | 29 +++++---------------- django/db/models/deletion.py | 2 +- django/db/models/query.py | 4 +-- django/db/transaction.py | 18 +++++-------- django/middleware/transaction.py | 1 - django/test/testcases.py | 4 --- docs/internals/deprecation.txt | 6 +++-- tests/delete_regress/tests.py | 4 +-- tests/middleware/tests.py | 6 +---- tests/requests/tests.py | 2 -- tests/select_for_update/tests.py | 3 --- tests/serializers/tests.py | 1 - tests/transactions_regress/tests.py | 5 ---- 14 files changed, 22 insertions(+), 64 deletions(-) diff --git a/django/core/management/commands/loaddata.py b/django/core/management/commands/loaddata.py index ed47b8fbf1..77b9a44a43 100644 --- a/django/core/management/commands/loaddata.py +++ b/django/core/management/commands/loaddata.py @@ -75,7 +75,6 @@ class Command(BaseCommand): if commit: transaction.commit_unless_managed(using=self.using) transaction.enter_transaction_management(using=self.using) - transaction.managed(True, using=self.using) class SingleZipReader(zipfile.ZipFile): def __init__(self, *args, **kwargs): diff --git a/django/db/backends/__init__.py b/django/db/backends/__init__.py index fe26c98baf..f11ee35260 100644 --- a/django/db/backends/__init__.py +++ b/django/db/backends/__init__.py @@ -234,7 +234,7 @@ class BaseDatabaseWrapper(object): ##### Generic transaction management methods ##### - def enter_transaction_management(self, managed=True): + def enter_transaction_management(self, managed=True, forced=False): """ Enters transaction management for a running thread. It must be balanced with the appropriate leave_transaction_management call, since the actual state is @@ -243,12 +243,14 @@ class BaseDatabaseWrapper(object): The state and dirty flag are carried over from the surrounding block or from the settings, if there is no surrounding block (dirty is always false when no current block is running). + + If you switch off transaction management and there is a pending + commit/rollback, the data will be commited, unless "forced" is True. """ - if self.transaction_state: - self.transaction_state.append(self.transaction_state[-1]) - else: - self.transaction_state.append(settings.TRANSACTIONS_MANAGED) + self.transaction_state.append(managed) self._enter_transaction_management(managed) + if not managed and self.is_dirty() and not forced: + self.commit() def leave_transaction_management(self): """ @@ -314,22 +316,6 @@ class BaseDatabaseWrapper(object): return self.transaction_state[-1] return settings.TRANSACTIONS_MANAGED - def managed(self, flag=True): - """ - Puts the transaction manager into a manual state: managed transactions have - to be committed explicitly by the user. If you switch off transaction - management and there is a pending commit/rollback, the data will be - commited. - """ - top = self.transaction_state - if top: - top[-1] = flag - if not flag and self.is_dirty(): - self.commit() - else: - raise TransactionManagementError("This code isn't under transaction " - "management") - def commit_unless_managed(self): """ Commits changes if the system is not in managed transaction mode. @@ -574,7 +560,6 @@ class BaseDatabaseFeatures(object): # otherwise autocommit will cause the confimation to # fail. self.connection.enter_transaction_management() - self.connection.managed(True) cursor = self.connection.cursor() cursor.execute('CREATE TABLE ROLLBACK_TEST (X INT)') self.connection.commit() diff --git a/django/db/models/deletion.py b/django/db/models/deletion.py index 81f74923c2..93ef0006cb 100644 --- a/django/db/models/deletion.py +++ b/django/db/models/deletion.py @@ -54,7 +54,7 @@ def force_managed(func): @wraps(func) def decorated(self, *args, **kwargs): if not transaction.is_managed(using=self.using): - transaction.enter_transaction_management(using=self.using) + transaction.enter_transaction_management(using=self.using, forced=True) forced_managed = True else: forced_managed = False diff --git a/django/db/models/query.py b/django/db/models/query.py index 30be30ca43..b41007ee4f 100644 --- a/django/db/models/query.py +++ b/django/db/models/query.py @@ -443,7 +443,7 @@ class QuerySet(object): connection = connections[self.db] fields = self.model._meta.local_fields if not transaction.is_managed(using=self.db): - transaction.enter_transaction_management(using=self.db) + transaction.enter_transaction_management(using=self.db, forced=True) forced_managed = True else: forced_managed = False @@ -582,7 +582,7 @@ class QuerySet(object): query = self.query.clone(sql.UpdateQuery) query.add_update_values(kwargs) if not transaction.is_managed(using=self.db): - transaction.enter_transaction_management(using=self.db) + transaction.enter_transaction_management(using=self.db, forced=True) forced_managed = True else: forced_managed = False diff --git a/django/db/transaction.py b/django/db/transaction.py index 809f14f628..09ce2abbd2 100644 --- a/django/db/transaction.py +++ b/django/db/transaction.py @@ -12,6 +12,8 @@ Managed transactions don't do those commits, but will need some kind of manual or implicit commits or rollbacks. """ +import warnings + from functools import wraps from django.db import connections, DEFAULT_DB_ALIAS @@ -49,7 +51,7 @@ def abort(using=None): """ get_connection(using).abort() -def enter_transaction_management(managed=True, using=None): +def enter_transaction_management(managed=True, using=None, forced=False): """ Enters transaction management for a running thread. It must be balanced with the appropriate leave_transaction_management call, since the actual state is @@ -59,7 +61,7 @@ def enter_transaction_management(managed=True, using=None): from the settings, if there is no surrounding block (dirty is always false when no current block is running). """ - get_connection(using).enter_transaction_management(managed) + get_connection(using).enter_transaction_management(managed, forced) def leave_transaction_management(using=None): """ @@ -105,13 +107,8 @@ def is_managed(using=None): return get_connection(using).is_managed() def managed(flag=True, using=None): - """ - Puts the transaction manager into a manual state: managed transactions have - to be committed explicitly by the user. If you switch off transaction - management and there is a pending commit/rollback, the data will be - commited. - """ - get_connection(using).managed(flag) + warnings.warn("'managed' no longer serves a purpose.", + PendingDeprecationWarning, stacklevel=2) def commit_unless_managed(using=None): """ @@ -224,7 +221,6 @@ def autocommit(using=None): """ def entering(using): enter_transaction_management(managed=False, using=using) - managed(False, using=using) def exiting(exc_value, using): leave_transaction_management(using=using) @@ -240,7 +236,6 @@ def commit_on_success(using=None): """ def entering(using): enter_transaction_management(using=using) - managed(True, using=using) def exiting(exc_value, using): try: @@ -268,7 +263,6 @@ def commit_manually(using=None): """ def entering(using): enter_transaction_management(using=using) - managed(True, using=using) def exiting(exc_value, using): leave_transaction_management(using=using) diff --git a/django/middleware/transaction.py b/django/middleware/transaction.py index 4440f377a7..b5a07a02b7 100644 --- a/django/middleware/transaction.py +++ b/django/middleware/transaction.py @@ -10,7 +10,6 @@ class TransactionMiddleware(object): def process_request(self, request): """Enters transaction management""" transaction.enter_transaction_management() - transaction.managed(True) def process_exception(self, request, exception): """Rolls back the database and leaves transaction management""" diff --git a/django/test/testcases.py b/django/test/testcases.py index 44ddb624d6..7f6b1a49ba 100644 --- a/django/test/testcases.py +++ b/django/test/testcases.py @@ -67,7 +67,6 @@ real_commit = transaction.commit real_rollback = transaction.rollback real_enter_transaction_management = transaction.enter_transaction_management real_leave_transaction_management = transaction.leave_transaction_management -real_managed = transaction.managed real_abort = transaction.abort def nop(*args, **kwargs): @@ -78,7 +77,6 @@ def disable_transaction_methods(): transaction.rollback = nop transaction.enter_transaction_management = nop transaction.leave_transaction_management = nop - transaction.managed = nop transaction.abort = nop def restore_transaction_methods(): @@ -86,7 +84,6 @@ def restore_transaction_methods(): transaction.rollback = real_rollback transaction.enter_transaction_management = real_enter_transaction_management transaction.leave_transaction_management = real_leave_transaction_management - transaction.managed = real_managed transaction.abort = real_abort @@ -833,7 +830,6 @@ class TestCase(TransactionTestCase): for db_name in self._databases_names(): transaction.enter_transaction_management(using=db_name) - transaction.managed(True, using=db_name) disable_transaction_methods() from django.contrib.sites.models import Site diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index b5173af298..296f908a5b 100644 --- a/docs/internals/deprecation.txt +++ b/docs/internals/deprecation.txt @@ -339,8 +339,6 @@ these changes. * ``Model._meta.module_name`` was renamed to ``model_name``. -* The private API ``django.db.close_connection`` will be removed. - * Remove the backward compatible shims introduced to rename ``get_query_set`` and similar queryset methods. This affects the following classes: ``BaseModelAdmin``, ``ChangeList``, ``BaseCommentNode``, @@ -350,6 +348,10 @@ these changes. * Remove the backward compatible shims introduced to rename the attributes ``ChangeList.root_query_set`` and ``ChangeList.query_set``. +* The private API ``django.db.close_connection`` will be removed. + +* The private API ``django.transaction.managed`` will be removed. + 2.0 --- diff --git a/tests/delete_regress/tests.py b/tests/delete_regress/tests.py index 9fcc19ba71..e88c95e229 100644 --- a/tests/delete_regress/tests.py +++ b/tests/delete_regress/tests.py @@ -22,9 +22,7 @@ class DeleteLockingTest(TransactionTestCase): self.conn2 = new_connections[DEFAULT_DB_ALIAS] # Put both DB connections into managed transaction mode transaction.enter_transaction_management() - transaction.managed(True) self.conn2.enter_transaction_management() - self.conn2.managed(True) def tearDown(self): # Close down the second connection. @@ -335,7 +333,7 @@ class Ticket19102Tests(TestCase): ).select_related('orgunit').delete() self.assertFalse(Login.objects.filter(pk=self.l1.pk).exists()) self.assertTrue(Login.objects.filter(pk=self.l2.pk).exists()) - + @skipUnlessDBFeature("update_can_self_select") def test_ticket_19102_defer(self): with self.assertNumQueries(1): diff --git a/tests/middleware/tests.py b/tests/middleware/tests.py index 4e5fd8ea6b..122371c02c 100644 --- a/tests/middleware/tests.py +++ b/tests/middleware/tests.py @@ -692,7 +692,6 @@ class TransactionMiddlewareTest(TransactionTestCase): def test_managed_response(self): transaction.enter_transaction_management() - transaction.managed(True) Band.objects.create(name='The Beatles') self.assertTrue(transaction.is_dirty()) TransactionMiddleware().process_response(self.request, self.response) @@ -700,8 +699,7 @@ class TransactionMiddlewareTest(TransactionTestCase): self.assertEqual(Band.objects.count(), 1) def test_unmanaged_response(self): - transaction.enter_transaction_management() - transaction.managed(False) + transaction.enter_transaction_management(False) self.assertEqual(Band.objects.count(), 0) TransactionMiddleware().process_response(self.request, self.response) self.assertFalse(transaction.is_managed()) @@ -711,7 +709,6 @@ class TransactionMiddlewareTest(TransactionTestCase): def test_exception(self): transaction.enter_transaction_management() - transaction.managed(True) Band.objects.create(name='The Beatles') self.assertTrue(transaction.is_dirty()) TransactionMiddleware().process_exception(self.request, None) @@ -726,7 +723,6 @@ class TransactionMiddlewareTest(TransactionTestCase): raise IntegrityError() connections[DEFAULT_DB_ALIAS].commit = raise_exception transaction.enter_transaction_management() - transaction.managed(True) Band.objects.create(name='The Beatles') self.assertTrue(transaction.is_dirty()) with self.assertRaises(IntegrityError): diff --git a/tests/requests/tests.py b/tests/requests/tests.py index 4fdc17618b..2803d7995b 100644 --- a/tests/requests/tests.py +++ b/tests/requests/tests.py @@ -576,7 +576,6 @@ class DatabaseConnectionHandlingTests(TransactionTestCase): # Make sure there is an open connection connection.cursor() connection.enter_transaction_management() - connection.managed(True) signals.request_finished.send(sender=response._handler_class) self.assertEqual(len(connection.transaction_state), 0) @@ -585,7 +584,6 @@ class DatabaseConnectionHandlingTests(TransactionTestCase): connection.settings_dict['CONN_MAX_AGE'] = 0 connection.enter_transaction_management() - connection.managed(True) connection.set_dirty() # Test that the rollback doesn't succeed (for example network failure # could cause this). diff --git a/tests/select_for_update/tests.py b/tests/select_for_update/tests.py index b9716bd797..c2fa22705a 100644 --- a/tests/select_for_update/tests.py +++ b/tests/select_for_update/tests.py @@ -25,7 +25,6 @@ class SelectForUpdateTests(TransactionTestCase): def setUp(self): transaction.enter_transaction_management() - transaction.managed(True) self.person = Person.objects.create(name='Reinhardt') # We have to commit here so that code in run_select_for_update can @@ -37,7 +36,6 @@ class SelectForUpdateTests(TransactionTestCase): new_connections = ConnectionHandler(settings.DATABASES) self.new_connection = new_connections[DEFAULT_DB_ALIAS] self.new_connection.enter_transaction_management() - self.new_connection.managed(True) # We need to set settings.DEBUG to True so we can capture # the output SQL to examine. @@ -162,7 +160,6 @@ class SelectForUpdateTests(TransactionTestCase): # We need to enter transaction management again, as this is done on # per-thread basis transaction.enter_transaction_management() - transaction.managed(True) people = list( Person.objects.all().select_for_update(nowait=nowait) ) diff --git a/tests/serializers/tests.py b/tests/serializers/tests.py index 34d0f5f1b1..a96a1af748 100644 --- a/tests/serializers/tests.py +++ b/tests/serializers/tests.py @@ -268,7 +268,6 @@ class SerializersTransactionTestBase(object): # within a transaction in order to test forward reference # handling. transaction.enter_transaction_management() - transaction.managed(True) objs = serializers.deserialize(self.serializer_name, self.fwd_ref_str) with connection.constraint_checks_disabled(): for obj in objs: diff --git a/tests/transactions_regress/tests.py b/tests/transactions_regress/tests.py index 6ba04892cd..0af7605339 100644 --- a/tests/transactions_regress/tests.py +++ b/tests/transactions_regress/tests.py @@ -223,7 +223,6 @@ class TestNewConnection(TransactionTestCase): def test_commit_unless_managed_in_managed(self): cursor = connection.cursor() connection.enter_transaction_management() - transaction.managed(True) cursor.execute("INSERT into transactions_regress_mod (fld) values (2)") connection.commit_unless_managed() self.assertTrue(connection.is_dirty()) @@ -280,7 +279,6 @@ class TestPostgresAutocommitAndIsolation(TransactionTestCase): def test_transaction_management(self): transaction.enter_transaction_management() - transaction.managed(True) self.assertEqual(connection.isolation_level, self._serializable) transaction.leave_transaction_management() @@ -288,7 +286,6 @@ class TestPostgresAutocommitAndIsolation(TransactionTestCase): def test_transaction_stacking(self): transaction.enter_transaction_management() - transaction.managed(True) self.assertEqual(connection.isolation_level, self._serializable) transaction.enter_transaction_management() @@ -302,13 +299,11 @@ class TestPostgresAutocommitAndIsolation(TransactionTestCase): def test_enter_autocommit(self): transaction.enter_transaction_management() - transaction.managed(True) self.assertEqual(connection.isolation_level, self._serializable) list(Mod.objects.all()) self.assertTrue(transaction.is_dirty()) # Enter autocommit mode again. transaction.enter_transaction_management(False) - transaction.managed(False) self.assertFalse(transaction.is_dirty()) self.assertEqual( connection.connection.get_transaction_status(), From f5156194945661d217523d6648dfb9b48707ec95 Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Sat, 2 Mar 2013 13:47:46 +0100 Subject: [PATCH 02/32] Added an API to control database-level autocommit. --- django/db/backends/__init__.py | 14 ++++++++++++++ django/db/backends/creation.py | 6 +++++- django/db/backends/dummy/base.py | 1 + django/db/backends/mysql/base.py | 3 +++ django/db/backends/oracle/base.py | 3 +++ django/db/backends/oracle/creation.py | 3 --- django/db/backends/postgresql_psycopg2/base.py | 8 ++++++++ .../db/backends/postgresql_psycopg2/creation.py | 3 --- django/db/backends/sqlite3/base.py | 11 +++++++++++ django/db/backends/sqlite3/creation.py | 3 --- django/db/transaction.py | 12 ++++++++++++ django/test/testcases.py | 3 +++ docs/internals/deprecation.txt | 7 ++++--- docs/topics/db/transactions.txt | 17 +++++++++++++++++ 14 files changed, 81 insertions(+), 13 deletions(-) diff --git a/django/db/backends/__init__.py b/django/db/backends/__init__.py index f11ee35260..379416fad7 100644 --- a/django/db/backends/__init__.py +++ b/django/db/backends/__init__.py @@ -44,6 +44,7 @@ class BaseDatabaseWrapper(object): self.savepoint_state = 0 # Transaction management related attributes + self.autocommit = False self.transaction_state = [] # Tracks if the connection is believed to be in transaction. This is # set somewhat aggressively, as the DBAPI doesn't make it easy to @@ -232,6 +233,12 @@ class BaseDatabaseWrapper(object): """ pass + def _set_autocommit(self, autocommit): + """ + Backend-specific implementation to enable or disable autocommit. + """ + raise NotImplementedError + ##### Generic transaction management methods ##### def enter_transaction_management(self, managed=True, forced=False): @@ -274,6 +281,13 @@ class BaseDatabaseWrapper(object): raise TransactionManagementError( "Transaction managed block ended with pending COMMIT/ROLLBACK") + def set_autocommit(self, autocommit=True): + """ + Enable or disable autocommit. + """ + self._set_autocommit(autocommit) + self.autocommit = autocommit + def abort(self): """ Roll back any ongoing transaction and clean the transaction state diff --git a/django/db/backends/creation.py b/django/db/backends/creation.py index 70c24bc820..aa4fb82b12 100644 --- a/django/db/backends/creation.py +++ b/django/db/backends/creation.py @@ -1,6 +1,7 @@ import hashlib import sys import time +import warnings from django.conf import settings from django.db.utils import load_backend @@ -466,7 +467,10 @@ class BaseDatabaseCreation(object): anymore by Django code. Kept for compatibility with user code that might use it. """ - pass + warnings.warn( + "set_autocommit was moved from BaseDatabaseCreation to " + "BaseDatabaseWrapper.", PendingDeprecationWarning, stacklevel=2) + return self.connection.set_autocommit() def _prepare_for_test_db_ddl(self): """ diff --git a/django/db/backends/dummy/base.py b/django/db/backends/dummy/base.py index c59f037a27..a8c2d5bada 100644 --- a/django/db/backends/dummy/base.py +++ b/django/db/backends/dummy/base.py @@ -57,6 +57,7 @@ class DatabaseWrapper(BaseDatabaseWrapper): _savepoint_rollback = ignore _enter_transaction_management = complain _leave_transaction_management = ignore + _set_autocommit = complain set_dirty = complain set_clean = complain commit_unless_managed = complain diff --git a/django/db/backends/mysql/base.py b/django/db/backends/mysql/base.py index 400fe6cdac..39fd3695b7 100644 --- a/django/db/backends/mysql/base.py +++ b/django/db/backends/mysql/base.py @@ -445,6 +445,9 @@ class DatabaseWrapper(BaseDatabaseWrapper): except Database.NotSupportedError: pass + def _set_autocommit(self, autocommit): + self.connection.autocommit(autocommit) + def disable_constraint_checking(self): """ Disables foreign key checks, primarily for use in adding rows with forward references. Always returns True, diff --git a/django/db/backends/oracle/base.py b/django/db/backends/oracle/base.py index 60ee1ba632..d895c1583a 100644 --- a/django/db/backends/oracle/base.py +++ b/django/db/backends/oracle/base.py @@ -612,6 +612,9 @@ class DatabaseWrapper(BaseDatabaseWrapper): def _savepoint_commit(self, sid): pass + def _set_autocommit(self, autocommit): + self.connection.autocommit = autocommit + def check_constraints(self, table_names=None): """ To check constraints, we set constraints to immediate. Then, when, we're done we must ensure they diff --git a/django/db/backends/oracle/creation.py b/django/db/backends/oracle/creation.py index aaca74e8d1..5485830bf5 100644 --- a/django/db/backends/oracle/creation.py +++ b/django/db/backends/oracle/creation.py @@ -273,6 +273,3 @@ class DatabaseCreation(BaseDatabaseCreation): settings_dict['NAME'], self._test_database_user(), ) - - def set_autocommit(self): - self.connection.connection.autocommit = True diff --git a/django/db/backends/postgresql_psycopg2/base.py b/django/db/backends/postgresql_psycopg2/base.py index f9af507311..a14844433e 100644 --- a/django/db/backends/postgresql_psycopg2/base.py +++ b/django/db/backends/postgresql_psycopg2/base.py @@ -201,6 +201,14 @@ class DatabaseWrapper(BaseDatabaseWrapper): self.isolation_level = level self.features.uses_savepoints = bool(level) + def _set_autocommit(self, autocommit): + if autocommit: + level = psycopg2.extensions.ISOLATION_LEVEL_AUTOCOMMIT + else: + level = self.settings_dict["OPTIONS"].get('isolation_level', + psycopg2.extensions.ISOLATION_LEVEL_READ_COMMITTED) + self._set_isolation_level(level) + def set_dirty(self): if ((self.transaction_state and self.transaction_state[-1]) or not self.features.uses_autocommit): diff --git a/django/db/backends/postgresql_psycopg2/creation.py b/django/db/backends/postgresql_psycopg2/creation.py index b19926b440..e6400d79a1 100644 --- a/django/db/backends/postgresql_psycopg2/creation.py +++ b/django/db/backends/postgresql_psycopg2/creation.py @@ -78,9 +78,6 @@ class DatabaseCreation(BaseDatabaseCreation): ' text_pattern_ops')) return output - def set_autocommit(self): - self._prepare_for_test_db_ddl() - def _prepare_for_test_db_ddl(self): """Rollback and close the active transaction.""" # Make sure there is an open connection. diff --git a/django/db/backends/sqlite3/base.py b/django/db/backends/sqlite3/base.py index 416a6293f5..9a37dd17fe 100644 --- a/django/db/backends/sqlite3/base.py +++ b/django/db/backends/sqlite3/base.py @@ -355,6 +355,17 @@ class DatabaseWrapper(BaseDatabaseWrapper): if self.settings_dict['NAME'] != ":memory:": BaseDatabaseWrapper.close(self) + def _set_autocommit(self, autocommit): + if autocommit: + level = None + else: + # sqlite3's internal default is ''. It's different from None. + # See Modules/_sqlite/connection.c. + level = '' + # 'isolation_level' is a misleading API. + # SQLite always runs at the SERIALIZABLE isolation level. + self.connection.isolation_level = level + def check_constraints(self, table_names=None): """ Checks each table name in `table_names` for rows with invalid foreign key references. This method is diff --git a/django/db/backends/sqlite3/creation.py b/django/db/backends/sqlite3/creation.py index c90a697e35..a9fb273f7a 100644 --- a/django/db/backends/sqlite3/creation.py +++ b/django/db/backends/sqlite3/creation.py @@ -72,9 +72,6 @@ class DatabaseCreation(BaseDatabaseCreation): # Remove the SQLite database file os.remove(test_database_name) - def set_autocommit(self): - self.connection.connection.isolation_level = None - def test_db_signature(self): """ Returns a tuple that uniquely identifies a test database. diff --git a/django/db/transaction.py b/django/db/transaction.py index 09ce2abbd2..dd48e14bf4 100644 --- a/django/db/transaction.py +++ b/django/db/transaction.py @@ -39,6 +39,18 @@ def get_connection(using=None): using = DEFAULT_DB_ALIAS return connections[using] +def get_autocommit(using=None): + """ + Get the autocommit status of the connection. + """ + return get_connection(using).autocommit + +def set_autocommit(using=None, autocommit=True): + """ + Set the autocommit status of the connection. + """ + return get_connection(using).set_autocommit(autocommit) + def abort(using=None): """ Roll back any ongoing transactions and clean the transaction management diff --git a/django/test/testcases.py b/django/test/testcases.py index 7f6b1a49ba..4b9116e3bc 100644 --- a/django/test/testcases.py +++ b/django/test/testcases.py @@ -63,6 +63,7 @@ def to_list(value): value = [value] return value +real_set_autocommit = transaction.set_autocommit real_commit = transaction.commit real_rollback = transaction.rollback real_enter_transaction_management = transaction.enter_transaction_management @@ -73,6 +74,7 @@ def nop(*args, **kwargs): return def disable_transaction_methods(): + transaction.set_autocommit = nop transaction.commit = nop transaction.rollback = nop transaction.enter_transaction_management = nop @@ -80,6 +82,7 @@ def disable_transaction_methods(): transaction.abort = nop def restore_transaction_methods(): + transaction.set_autocommit = real_set_autocommit transaction.commit = real_commit transaction.rollback = real_rollback transaction.enter_transaction_management = real_enter_transaction_management diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index 296f908a5b..74fbb563f0 100644 --- a/docs/internals/deprecation.txt +++ b/docs/internals/deprecation.txt @@ -348,9 +348,10 @@ these changes. * Remove the backward compatible shims introduced to rename the attributes ``ChangeList.root_query_set`` and ``ChangeList.query_set``. -* The private API ``django.db.close_connection`` will be removed. - -* The private API ``django.transaction.managed`` will be removed. +* The following private APIs will be removed: + - ``django.db.close_connection()`` + - ``django.db.backends.creation.BaseDatabaseCreation.set_autocommit()`` + - ``django.db.transaction.managed()`` 2.0 --- diff --git a/docs/topics/db/transactions.txt b/docs/topics/db/transactions.txt index 11755ff5c5..e145edf149 100644 --- a/docs/topics/db/transactions.txt +++ b/docs/topics/db/transactions.txt @@ -208,6 +208,23 @@ This applies to all database operations, not just write operations. Even if your transaction only reads from the database, the transaction must be committed or rolled back before you complete a request. +.. _managing-autocommit: + +Managing autocommit +=================== + +.. versionadded:: 1.6 + +Django provides a straightforward API to manage the autocommit state of each +database connection, if you need to. + +.. function:: get_autocommit(using=None) + +.. function:: set_autocommit(using=None, autocommit=True) + +These functions take a ``using`` argument which should be the name of a +database. If it isn't provided, Django uses the ``"default"`` database. + .. _deactivate-transaction-management: How to globally deactivate transaction management From 8717b0668caf00ec5e81ef5a1e31b4d7c64eee8a Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Sat, 2 Mar 2013 16:57:56 +0100 Subject: [PATCH 03/32] Separated autocommit and isolation level handling for PostgreSQL. Autocommit cannot be manipulated independently from an open connection. This commit introduces a minor change in behavior: entering transaction management forces opening a databasse connection. This shouldn't be backwards incompatible in any practical use case. --- .../db/backends/postgresql_psycopg2/base.py | 76 ++++++++++--------- tests/transactions_regress/tests.py | 17 ++++- 2 files changed, 53 insertions(+), 40 deletions(-) diff --git a/django/db/backends/postgresql_psycopg2/base.py b/django/db/backends/postgresql_psycopg2/base.py index a14844433e..6211adcbd7 100644 --- a/django/db/backends/postgresql_psycopg2/base.py +++ b/django/db/backends/postgresql_psycopg2/base.py @@ -77,21 +77,21 @@ class DatabaseWrapper(BaseDatabaseWrapper): def __init__(self, *args, **kwargs): super(DatabaseWrapper, self).__init__(*args, **kwargs) + opts = self.settings_dict["OPTIONS"] + RC = psycopg2.extensions.ISOLATION_LEVEL_READ_COMMITTED + self.isolation_level = opts.get('isolation_level', RC) + self.features = DatabaseFeatures(self) - autocommit = self.settings_dict["OPTIONS"].get('autocommit', False) - self.features.uses_autocommit = autocommit - if autocommit: - level = psycopg2.extensions.ISOLATION_LEVEL_AUTOCOMMIT - else: - level = self.settings_dict["OPTIONS"].get('isolation_level', - psycopg2.extensions.ISOLATION_LEVEL_READ_COMMITTED) - self._set_isolation_level(level) self.ops = DatabaseOperations(self) self.client = DatabaseClient(self) self.creation = DatabaseCreation(self) self.introspection = DatabaseIntrospection(self) self.validation = BaseDatabaseValidation(self) + autocommit = opts.get('autocommit', False) + self.features.uses_autocommit = autocommit + self.features.uses_savepoints = not autocommit + def get_connection_params(self): settings_dict = self.settings_dict if not settings_dict['NAME']: @@ -135,11 +135,12 @@ class DatabaseWrapper(BaseDatabaseWrapper): if conn_tz != tz: # Set the time zone in autocommit mode (see #17062) - self.connection.set_isolation_level( - psycopg2.extensions.ISOLATION_LEVEL_AUTOCOMMIT) + self.set_autocommit(True) self.connection.cursor().execute( self.ops.set_time_zone_sql(), [tz]) self.connection.set_isolation_level(self.isolation_level) + if self.features.uses_autocommit: + self.set_autocommit(True) def create_cursor(self): cursor = self.connection.cursor() @@ -172,42 +173,40 @@ class DatabaseWrapper(BaseDatabaseWrapper): Switch the isolation level when needing transaction support, so that the same transaction is visible across all the queries. """ - if self.features.uses_autocommit and managed and not self.isolation_level: - level = self.settings_dict["OPTIONS"].get('isolation_level', - psycopg2.extensions.ISOLATION_LEVEL_READ_COMMITTED) - self._set_isolation_level(level) + if self.connection is None: # Force creating a connection. + self.cursor().close() + if self.features.uses_autocommit and managed and self.autocommit: + self.set_autocommit(False) + self.features.uses_savepoints = True def _leave_transaction_management(self, managed): """ If the normal operating mode is "autocommit", switch back to that when leaving transaction management. """ - if self.features.uses_autocommit and not managed and self.isolation_level: - self._set_isolation_level(psycopg2.extensions.ISOLATION_LEVEL_AUTOCOMMIT) + if self.connection is None: # Force creating a connection. + self.cursor().close() + if self.features.uses_autocommit and not managed and not self.autocommit: + self.rollback() # Must terminate transaction first. + self.set_autocommit(True) + self.features.uses_savepoints = False - def _set_isolation_level(self, level): - """ - Do all the related feature configurations for changing isolation - levels. This doesn't touch the uses_autocommit feature, since that - controls the movement *between* isolation levels. - """ - assert level in range(5) - try: - if self.connection is not None: - self.connection.set_isolation_level(level) - if level == psycopg2.extensions.ISOLATION_LEVEL_AUTOCOMMIT: - self.set_clean() - finally: - self.isolation_level = level - self.features.uses_savepoints = bool(level) + def _set_isolation_level(self, isolation_level): + assert isolation_level in range(1, 5) # Use set_autocommit for level = 0 + if self.psycopg2_version >= (2, 4, 2): + self.connection.set_session(isolation_level=isolation_level) + else: + self.connection.set_isolation_level(isolation_level) def _set_autocommit(self, autocommit): - if autocommit: - level = psycopg2.extensions.ISOLATION_LEVEL_AUTOCOMMIT + if self.psycopg2_version >= (2, 4, 2): + self.connection.autocommit = autocommit else: - level = self.settings_dict["OPTIONS"].get('isolation_level', - psycopg2.extensions.ISOLATION_LEVEL_READ_COMMITTED) - self._set_isolation_level(level) + if autocommit: + level = psycopg2.extensions.ISOLATION_LEVEL_AUTOCOMMIT + else: + level = self.isolation_level + self.connection.set_isolation_level(level) def set_dirty(self): if ((self.transaction_state and self.transaction_state[-1]) or @@ -231,6 +230,11 @@ class DatabaseWrapper(BaseDatabaseWrapper): else: return True + @cached_property + def psycopg2_version(self): + version = psycopg2.__version__.split(' ', 1)[0] + return tuple(int(v) for v in version.split('.')) + @cached_property def pg_version(self): with self.temporary_connection(): diff --git a/tests/transactions_regress/tests.py b/tests/transactions_regress/tests.py index 0af7605339..1b50039965 100644 --- a/tests/transactions_regress/tests.py +++ b/tests/transactions_regress/tests.py @@ -274,31 +274,39 @@ class TestPostgresAutocommitAndIsolation(TransactionTestCase): connections[DEFAULT_DB_ALIAS] = self._old_backend def test_initial_autocommit_state(self): + # Autocommit is activated when the connection is created. + connection.cursor().close() + self.assertTrue(connection.features.uses_autocommit) - self.assertEqual(connection.isolation_level, self._autocommit) + self.assertTrue(connection.autocommit) def test_transaction_management(self): transaction.enter_transaction_management() + self.assertFalse(connection.autocommit) self.assertEqual(connection.isolation_level, self._serializable) transaction.leave_transaction_management() - self.assertEqual(connection.isolation_level, self._autocommit) + self.assertTrue(connection.autocommit) def test_transaction_stacking(self): transaction.enter_transaction_management() + self.assertFalse(connection.autocommit) self.assertEqual(connection.isolation_level, self._serializable) transaction.enter_transaction_management() + self.assertFalse(connection.autocommit) self.assertEqual(connection.isolation_level, self._serializable) transaction.leave_transaction_management() + self.assertFalse(connection.autocommit) self.assertEqual(connection.isolation_level, self._serializable) transaction.leave_transaction_management() - self.assertEqual(connection.isolation_level, self._autocommit) + self.assertTrue(connection.autocommit) def test_enter_autocommit(self): transaction.enter_transaction_management() + self.assertFalse(connection.autocommit) self.assertEqual(connection.isolation_level, self._serializable) list(Mod.objects.all()) self.assertTrue(transaction.is_dirty()) @@ -311,9 +319,10 @@ class TestPostgresAutocommitAndIsolation(TransactionTestCase): list(Mod.objects.all()) self.assertFalse(transaction.is_dirty()) transaction.leave_transaction_management() + self.assertFalse(connection.autocommit) self.assertEqual(connection.isolation_level, self._serializable) transaction.leave_transaction_management() - self.assertEqual(connection.isolation_level, self._autocommit) + self.assertTrue(connection.autocommit) class TestManyToManyAddTransaction(TransactionTestCase): From af9e9386eb6ad22e3fa822574a4f5be4c9c29771 Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Sat, 2 Mar 2013 21:59:55 +0100 Subject: [PATCH 04/32] Enabled autocommit for PostgreSQL. For users who didn't activate autocommit in their database options, this is backwards-incompatible in "non-managed" aka "auto" transaction state. This state now uses database-level autocommit instead of ORM-level autocommit. Also removed the uses_autocommit feature which lost its purpose. --- django/db/backends/__init__.py | 1 - django/db/backends/postgresql_psycopg2/base.py | 14 +++++--------- tests/backends/tests.py | 4 ++-- tests/middleware/tests.py | 5 +++++ tests/transactions_regress/tests.py | 15 +++++++-------- 5 files changed, 19 insertions(+), 20 deletions(-) diff --git a/django/db/backends/__init__.py b/django/db/backends/__init__.py index 379416fad7..afa01158c2 100644 --- a/django/db/backends/__init__.py +++ b/django/db/backends/__init__.py @@ -479,7 +479,6 @@ class BaseDatabaseFeatures(object): can_use_chunked_reads = True can_return_id_from_insert = False has_bulk_insert = False - uses_autocommit = False uses_savepoints = False can_combine_inserts_with_and_without_auto_increment_pk = False diff --git a/django/db/backends/postgresql_psycopg2/base.py b/django/db/backends/postgresql_psycopg2/base.py index 6211adcbd7..93c5f3c677 100644 --- a/django/db/backends/postgresql_psycopg2/base.py +++ b/django/db/backends/postgresql_psycopg2/base.py @@ -88,9 +88,7 @@ class DatabaseWrapper(BaseDatabaseWrapper): self.introspection = DatabaseIntrospection(self) self.validation = BaseDatabaseValidation(self) - autocommit = opts.get('autocommit', False) - self.features.uses_autocommit = autocommit - self.features.uses_savepoints = not autocommit + self.features.uses_savepoints = False def get_connection_params(self): settings_dict = self.settings_dict @@ -139,8 +137,7 @@ class DatabaseWrapper(BaseDatabaseWrapper): self.connection.cursor().execute( self.ops.set_time_zone_sql(), [tz]) self.connection.set_isolation_level(self.isolation_level) - if self.features.uses_autocommit: - self.set_autocommit(True) + self.set_autocommit(not settings.TRANSACTIONS_MANAGED) def create_cursor(self): cursor = self.connection.cursor() @@ -175,7 +172,7 @@ class DatabaseWrapper(BaseDatabaseWrapper): """ if self.connection is None: # Force creating a connection. self.cursor().close() - if self.features.uses_autocommit and managed and self.autocommit: + if managed and self.autocommit: self.set_autocommit(False) self.features.uses_savepoints = True @@ -186,7 +183,7 @@ class DatabaseWrapper(BaseDatabaseWrapper): """ if self.connection is None: # Force creating a connection. self.cursor().close() - if self.features.uses_autocommit and not managed and not self.autocommit: + if not managed and not self.autocommit: self.rollback() # Must terminate transaction first. self.set_autocommit(True) self.features.uses_savepoints = False @@ -209,8 +206,7 @@ class DatabaseWrapper(BaseDatabaseWrapper): self.connection.set_isolation_level(level) def set_dirty(self): - if ((self.transaction_state and self.transaction_state[-1]) or - not self.features.uses_autocommit): + if self.transaction_state and self.transaction_state[-1]: super(DatabaseWrapper, self).set_dirty() def check_constraints(self, table_names=None): diff --git a/tests/backends/tests.py b/tests/backends/tests.py index 103a44684e..5c8a8955eb 100644 --- a/tests/backends/tests.py +++ b/tests/backends/tests.py @@ -302,8 +302,8 @@ class PostgresNewConnectionTest(TestCase): transaction is rolled back. """ @unittest.skipUnless( - connection.vendor == 'postgresql' and connection.isolation_level > 0, - "This test applies only to PostgreSQL without autocommit") + connection.vendor == 'postgresql', + "This test applies only to PostgreSQL") def test_connect_and_rollback(self): new_connections = ConnectionHandler(settings.DATABASES) new_connection = new_connections[DEFAULT_DB_ALIAS] diff --git a/tests/middleware/tests.py b/tests/middleware/tests.py index 122371c02c..17751dd158 100644 --- a/tests/middleware/tests.py +++ b/tests/middleware/tests.py @@ -22,6 +22,7 @@ from django.test.utils import override_settings from django.utils import six from django.utils.encoding import force_str from django.utils.six.moves import xrange +from django.utils.unittest import expectedFailure from .models import Band @@ -698,6 +699,10 @@ class TransactionMiddlewareTest(TransactionTestCase): self.assertFalse(transaction.is_dirty()) self.assertEqual(Band.objects.count(), 1) + # TODO: update this test to account for database-level autocommit. + # Currently it fails under PostgreSQL because connections are never + # marked dirty in non-managed mode. + @expectedFailure def test_unmanaged_response(self): transaction.enter_transaction_management(False) self.assertEqual(Band.objects.count(), 0) diff --git a/tests/transactions_regress/tests.py b/tests/transactions_regress/tests.py index 1b50039965..e6750cddcf 100644 --- a/tests/transactions_regress/tests.py +++ b/tests/transactions_regress/tests.py @@ -4,7 +4,7 @@ from django.db import connection, connections, transaction, DEFAULT_DB_ALIAS, Da from django.db.transaction import commit_on_success, commit_manually, TransactionManagementError from django.test import TransactionTestCase, skipUnlessDBFeature from django.test.utils import override_settings -from django.utils.unittest import skipIf, skipUnless +from django.utils.unittest import skipIf, skipUnless, expectedFailure from .models import Mod, M2mA, M2mB @@ -173,10 +173,6 @@ class TestNewConnection(TransactionTestCase): def setUp(self): self._old_backend = connections[DEFAULT_DB_ALIAS] settings = self._old_backend.settings_dict.copy() - opts = settings['OPTIONS'].copy() - if 'autocommit' in opts: - opts['autocommit'] = False - settings['OPTIONS'] = opts new_backend = self._old_backend.__class__(settings, DEFAULT_DB_ALIAS) connections[DEFAULT_DB_ALIAS] = new_backend @@ -189,6 +185,8 @@ class TestNewConnection(TransactionTestCase): connections[DEFAULT_DB_ALIAS].close() connections[DEFAULT_DB_ALIAS] = self._old_backend + # TODO: update this test to account for database-level autocommit. + @expectedFailure def test_commit(self): """ Users are allowed to commit and rollback connections. @@ -210,6 +208,8 @@ class TestNewConnection(TransactionTestCase): connection.leave_transaction_management() self.assertEqual(orig_dirty, connection._dirty) + # TODO: update this test to account for database-level autocommit. + @expectedFailure def test_commit_unless_managed(self): cursor = connection.cursor() cursor.execute("INSERT into transactions_regress_mod (fld) values (2)") @@ -220,6 +220,8 @@ class TestNewConnection(TransactionTestCase): connection.commit_unless_managed() self.assertFalse(connection.is_dirty()) + # TODO: update this test to account for database-level autocommit. + @expectedFailure def test_commit_unless_managed_in_managed(self): cursor = connection.cursor() connection.enter_transaction_management() @@ -260,7 +262,6 @@ class TestPostgresAutocommitAndIsolation(TransactionTestCase): self._old_backend = connections[DEFAULT_DB_ALIAS] settings = self._old_backend.settings_dict.copy() opts = settings['OPTIONS'].copy() - opts['autocommit'] = True opts['isolation_level'] = ISOLATION_LEVEL_SERIALIZABLE settings['OPTIONS'] = opts new_backend = self._old_backend.__class__(settings, DEFAULT_DB_ALIAS) @@ -276,8 +277,6 @@ class TestPostgresAutocommitAndIsolation(TransactionTestCase): def test_initial_autocommit_state(self): # Autocommit is activated when the connection is created. connection.cursor().close() - - self.assertTrue(connection.features.uses_autocommit) self.assertTrue(connection.autocommit) def test_transaction_management(self): From cd364efa00dba5aa86187aee83e4ed9917e7e93c Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Sun, 3 Mar 2013 15:32:14 +0100 Subject: [PATCH 05/32] Stopped flipping the uses_savepoints feature at runtime. --- django/db/backends/__init__.py | 6 +++--- django/db/backends/postgresql_psycopg2/base.py | 5 +---- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/django/db/backends/__init__.py b/django/db/backends/__init__.py index afa01158c2..ada0e038b4 100644 --- a/django/db/backends/__init__.py +++ b/django/db/backends/__init__.py @@ -161,17 +161,17 @@ class BaseDatabaseWrapper(object): ##### Backend-specific savepoint management methods ##### def _savepoint(self, sid): - if not self.features.uses_savepoints: + if not self.features.uses_savepoints or self.autocommit: return self.cursor().execute(self.ops.savepoint_create_sql(sid)) def _savepoint_rollback(self, sid): - if not self.features.uses_savepoints: + if not self.features.uses_savepoints or self.autocommit: return self.cursor().execute(self.ops.savepoint_rollback_sql(sid)) def _savepoint_commit(self, sid): - if not self.features.uses_savepoints: + if not self.features.uses_savepoints or self.autocommit: return self.cursor().execute(self.ops.savepoint_commit_sql(sid)) diff --git a/django/db/backends/postgresql_psycopg2/base.py b/django/db/backends/postgresql_psycopg2/base.py index 93c5f3c677..385206221d 100644 --- a/django/db/backends/postgresql_psycopg2/base.py +++ b/django/db/backends/postgresql_psycopg2/base.py @@ -49,6 +49,7 @@ class DatabaseFeatures(BaseDatabaseFeatures): has_select_for_update = True has_select_for_update_nowait = True has_bulk_insert = True + uses_savepoints = True supports_tablespaces = True supports_transactions = True can_distinct_on_fields = True @@ -88,8 +89,6 @@ class DatabaseWrapper(BaseDatabaseWrapper): self.introspection = DatabaseIntrospection(self) self.validation = BaseDatabaseValidation(self) - self.features.uses_savepoints = False - def get_connection_params(self): settings_dict = self.settings_dict if not settings_dict['NAME']: @@ -174,7 +173,6 @@ class DatabaseWrapper(BaseDatabaseWrapper): self.cursor().close() if managed and self.autocommit: self.set_autocommit(False) - self.features.uses_savepoints = True def _leave_transaction_management(self, managed): """ @@ -186,7 +184,6 @@ class DatabaseWrapper(BaseDatabaseWrapper): if not managed and not self.autocommit: self.rollback() # Must terminate transaction first. self.set_autocommit(True) - self.features.uses_savepoints = False def _set_isolation_level(self, isolation_level): assert isolation_level in range(1, 5) # Use set_autocommit for level = 0 From 7b4815b455fc99a2661492f91f7242cfb09a7017 Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Thu, 7 Mar 2013 11:50:49 +0100 Subject: [PATCH 06/32] Expressed the dirty flag handling logic in terms of autocommit. --- django/db/backends/__init__.py | 3 ++- django/db/backends/postgresql_psycopg2/base.py | 4 ---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/django/db/backends/__init__.py b/django/db/backends/__init__.py index ada0e038b4..d372384fc5 100644 --- a/django/db/backends/__init__.py +++ b/django/db/backends/__init__.py @@ -311,7 +311,8 @@ class BaseDatabaseWrapper(object): to decide in a managed block of code to decide whether there are open changes waiting for commit. """ - self._dirty = True + if not self.autocommit: + self._dirty = True def set_clean(self): """ diff --git a/django/db/backends/postgresql_psycopg2/base.py b/django/db/backends/postgresql_psycopg2/base.py index 385206221d..cc1d1ae567 100644 --- a/django/db/backends/postgresql_psycopg2/base.py +++ b/django/db/backends/postgresql_psycopg2/base.py @@ -202,10 +202,6 @@ class DatabaseWrapper(BaseDatabaseWrapper): level = self.isolation_level self.connection.set_isolation_level(level) - def set_dirty(self): - if self.transaction_state and self.transaction_state[-1]: - super(DatabaseWrapper, self).set_dirty() - def check_constraints(self, table_names=None): """ To check constraints, we set constraints to immediate. Then, when, we're done we must ensure they From 1617557ae30001cef8a863687d4bcdc28151cd50 Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Sun, 3 Mar 2013 15:43:24 +0100 Subject: [PATCH 07/32] Added BaseDatabaseWrapper.ensure_connection. This API is useful because autocommit cannot be managed without an open connection. --- django/db/backends/__init__.py | 34 +++++++++++++------ .../db/backends/postgresql_psycopg2/base.py | 4 --- 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/django/db/backends/__init__.py b/django/db/backends/__init__.py index d372384fc5..b7f4628e9a 100644 --- a/django/db/backends/__init__.py +++ b/django/db/backends/__init__.py @@ -86,20 +86,33 @@ class BaseDatabaseWrapper(object): """Creates a cursor. Assumes that a connection is established.""" raise NotImplementedError + ##### Backend-specific methods for creating connections ##### + + def connect(self): + """Connects to the database. Assumes that the connection is closed.""" + # Reset parameters defining when to close the connection + max_age = self.settings_dict['CONN_MAX_AGE'] + self.close_at = None if max_age is None else time.time() + max_age + self.errors_occurred = False + # Establish the connection + conn_params = self.get_connection_params() + self.connection = self.get_new_connection(conn_params) + self.init_connection_state() + connection_created.send(sender=self.__class__, connection=self) + + def ensure_connection(self): + """ + Guarantees that a connection to the database is established. + """ + if self.connection is None: + with self.wrap_database_errors(): + self.connect() + ##### Backend-specific wrappers for PEP-249 connection methods ##### def _cursor(self): + self.ensure_connection() with self.wrap_database_errors(): - if self.connection is None: - # Reset parameters defining when to close the connection - max_age = self.settings_dict['CONN_MAX_AGE'] - self.close_at = None if max_age is None else time.time() + max_age - self.errors_occurred = False - # Establish the connection - conn_params = self.get_connection_params() - self.connection = self.get_new_connection(conn_params) - self.init_connection_state() - connection_created.send(sender=self.__class__, connection=self) return self.create_cursor() def _commit(self): @@ -285,6 +298,7 @@ class BaseDatabaseWrapper(object): """ Enable or disable autocommit. """ + self.ensure_connection() self._set_autocommit(autocommit) self.autocommit = autocommit diff --git a/django/db/backends/postgresql_psycopg2/base.py b/django/db/backends/postgresql_psycopg2/base.py index cc1d1ae567..384b38cc58 100644 --- a/django/db/backends/postgresql_psycopg2/base.py +++ b/django/db/backends/postgresql_psycopg2/base.py @@ -169,8 +169,6 @@ class DatabaseWrapper(BaseDatabaseWrapper): Switch the isolation level when needing transaction support, so that the same transaction is visible across all the queries. """ - if self.connection is None: # Force creating a connection. - self.cursor().close() if managed and self.autocommit: self.set_autocommit(False) @@ -179,8 +177,6 @@ class DatabaseWrapper(BaseDatabaseWrapper): If the normal operating mode is "autocommit", switch back to that when leaving transaction management. """ - if self.connection is None: # Force creating a connection. - self.cursor().close() if not managed and not self.autocommit: self.rollback() # Must terminate transaction first. self.set_autocommit(True) From cfc114e00ebe2ac16c37af2ccee1ed8e47247b7a Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Sun, 3 Mar 2013 15:48:52 +0100 Subject: [PATCH 08/32] Removed _enter/_leave_transaction_management. The goal is to make all databases share a common, autocommit-based, implementation. --- django/db/backends/__init__.py | 38 ++++++++----------- django/db/backends/dummy/base.py | 2 - .../db/backends/postgresql_psycopg2/base.py | 17 --------- 3 files changed, 15 insertions(+), 42 deletions(-) diff --git a/django/db/backends/__init__.py b/django/db/backends/__init__.py index b7f4628e9a..26a84e6e60 100644 --- a/django/db/backends/__init__.py +++ b/django/db/backends/__init__.py @@ -231,21 +231,6 @@ class BaseDatabaseWrapper(object): ##### Backend-specific transaction management methods ##### - def _enter_transaction_management(self, managed): - """ - A hook for backend-specific changes required when entering manual - transaction handling. - """ - pass - - def _leave_transaction_management(self, managed): - """ - A hook for backend-specific changes required when leaving manual - transaction handling. Will usually be implemented only when - _enter_transaction_management() is also required. - """ - pass - def _set_autocommit(self, autocommit): """ Backend-specific implementation to enable or disable autocommit. @@ -268,7 +253,10 @@ class BaseDatabaseWrapper(object): commit/rollback, the data will be commited, unless "forced" is True. """ self.transaction_state.append(managed) - self._enter_transaction_management(managed) + + if managed and self.autocommit: + self.set_autocommit(False) + if not managed and self.is_dirty() and not forced: self.commit() @@ -283,17 +271,21 @@ class BaseDatabaseWrapper(object): else: raise TransactionManagementError( "This code isn't under transaction management") - # The _leave_transaction_management hook can change the dirty flag, - # so memoize it. - dirty = self._dirty - # We will pass the next status (after leaving the previous state - # behind) to subclass hook. - self._leave_transaction_management(self.is_managed()) - if dirty: + + # That's the next state -- we already left the previous state behind. + managed = self.is_managed() + + if self._dirty: self.rollback() + if not managed and not self.autocommit: + self.set_autocommit(True) raise TransactionManagementError( "Transaction managed block ended with pending COMMIT/ROLLBACK") + if not managed and not self.autocommit: + self.set_autocommit(True) + + def set_autocommit(self, autocommit=True): """ Enable or disable autocommit. diff --git a/django/db/backends/dummy/base.py b/django/db/backends/dummy/base.py index a8c2d5bada..02f0b6462d 100644 --- a/django/db/backends/dummy/base.py +++ b/django/db/backends/dummy/base.py @@ -55,8 +55,6 @@ class DatabaseWrapper(BaseDatabaseWrapper): _savepoint = ignore _savepoint_commit = complain _savepoint_rollback = ignore - _enter_transaction_management = complain - _leave_transaction_management = ignore _set_autocommit = complain set_dirty = complain set_clean = complain diff --git a/django/db/backends/postgresql_psycopg2/base.py b/django/db/backends/postgresql_psycopg2/base.py index 384b38cc58..6ea2dd3099 100644 --- a/django/db/backends/postgresql_psycopg2/base.py +++ b/django/db/backends/postgresql_psycopg2/base.py @@ -164,23 +164,6 @@ class DatabaseWrapper(BaseDatabaseWrapper): finally: self.set_clean() - def _enter_transaction_management(self, managed): - """ - Switch the isolation level when needing transaction support, so that - the same transaction is visible across all the queries. - """ - if managed and self.autocommit: - self.set_autocommit(False) - - def _leave_transaction_management(self, managed): - """ - If the normal operating mode is "autocommit", switch back to that when - leaving transaction management. - """ - if not managed and not self.autocommit: - self.rollback() # Must terminate transaction first. - self.set_autocommit(True) - def _set_isolation_level(self, isolation_level): assert isolation_level in range(1, 5) # Use set_autocommit for level = 0 if self.psycopg2_version >= (2, 4, 2): From 5e27debc5cba30c84f99151a84c5fd846a65b091 Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Sun, 3 Mar 2013 15:55:11 +0100 Subject: [PATCH 09/32] Enabled database-level autocommit for all backends. This is mostly a documentation change. It has the same backwards-incompatibility consequences as those described for PostgreSQL in a previous commit. --- django/db/backends/__init__.py | 2 + .../db/backends/postgresql_psycopg2/base.py | 1 - docs/ref/databases.txt | 58 +---- docs/ref/request-response.txt | 4 +- docs/releases/1.6.txt | 22 ++ docs/topics/db/sql.txt | 61 ++--- docs/topics/db/transactions.txt | 230 +++++++++++++----- 7 files changed, 238 insertions(+), 140 deletions(-) diff --git a/django/db/backends/__init__.py b/django/db/backends/__init__.py index 26a84e6e60..4031e8f668 100644 --- a/django/db/backends/__init__.py +++ b/django/db/backends/__init__.py @@ -98,6 +98,8 @@ class BaseDatabaseWrapper(object): conn_params = self.get_connection_params() self.connection = self.get_new_connection(conn_params) self.init_connection_state() + if not settings.TRANSACTIONS_MANAGED: + self.set_autocommit() connection_created.send(sender=self.__class__, connection=self) def ensure_connection(self): diff --git a/django/db/backends/postgresql_psycopg2/base.py b/django/db/backends/postgresql_psycopg2/base.py index 6ea2dd3099..5c5f5e185a 100644 --- a/django/db/backends/postgresql_psycopg2/base.py +++ b/django/db/backends/postgresql_psycopg2/base.py @@ -136,7 +136,6 @@ class DatabaseWrapper(BaseDatabaseWrapper): self.connection.cursor().execute( self.ops.set_time_zone_sql(), [tz]) self.connection.set_isolation_level(self.isolation_level) - self.set_autocommit(not settings.TRANSACTIONS_MANAGED) def create_cursor(self): cursor = self.connection.cursor() diff --git a/docs/ref/databases.txt b/docs/ref/databases.txt index 4e435949a2..4dafb3774f 100644 --- a/docs/ref/databases.txt +++ b/docs/ref/databases.txt @@ -69,7 +69,6 @@ even ``0``, because it doesn't make sense to maintain a connection that's unlikely to be reused. This will help keep the number of simultaneous connections to this database small. - The development server creates a new thread for each request it handles, negating the effect of persistent connections. @@ -104,7 +103,8 @@ Optimizing PostgreSQL's configuration Django needs the following parameters for its database connections: - ``client_encoding``: ``'UTF8'``, -- ``default_transaction_isolation``: ``'read committed'``, +- ``default_transaction_isolation``: ``'read committed'`` by default, + or the value set in the connection options (see below), - ``timezone``: ``'UTC'`` when :setting:`USE_TZ` is ``True``, value of :setting:`TIME_ZONE` otherwise. @@ -118,30 +118,16 @@ will do some additional queries to set these parameters. .. _ALTER ROLE: http://www.postgresql.org/docs/current/interactive/sql-alterrole.html -Transaction handling ---------------------- - -:doc:`By default `, Django runs with an open -transaction which it commits automatically when any built-in, data-altering -model function is called. The PostgreSQL backends normally operate the same as -any other Django backend in this respect. - .. _postgresql-autocommit-mode: Autocommit mode -~~~~~~~~~~~~~~~ +--------------- -If your application is particularly read-heavy and doesn't make many -database writes, the overhead of a constantly open transaction can -sometimes be noticeable. For those situations, you can configure Django -to use *"autocommit"* behavior for the connection, meaning that each database -operation will normally be in its own transaction, rather than having -the transaction extend over multiple operations. In this case, you can -still manually start a transaction if you're doing something that -requires consistency across multiple database operations. The -autocommit behavior is enabled by setting the ``autocommit`` key in -the :setting:`OPTIONS` part of your database configuration in -:setting:`DATABASES`:: +.. versionchanged:: 1.6 + +In previous versions of Django, database-level autocommit could be enabled by +setting the ``autocommit`` key in the :setting:`OPTIONS` part of your database +configuration in :setting:`DATABASES`:: DATABASES = { # ... @@ -150,29 +136,11 @@ the :setting:`OPTIONS` part of your database configuration in }, } -In this configuration, Django still ensures that :ref:`delete() -` and :ref:`update() ` -queries run inside a single transaction, so that either all the affected -objects are changed or none of them are. - -.. admonition:: This is database-level autocommit - - This functionality is not the same as the :ref:`autocommit - ` decorator. That decorator is - a Django-level implementation that commits automatically after - data changing operations. The feature enabled using the - :setting:`OPTIONS` option provides autocommit behavior at the - database adapter level. It commits after *every* operation. - -If you are using this feature and performing an operation akin to delete or -updating that requires multiple operations, you are strongly recommended to -wrap you operations in manual transaction handling to ensure data consistency. -You should also audit your existing code for any instances of this behavior -before enabling this feature. It's faster, but it provides less automatic -protection for multi-call operations. +Since Django 1.6, autocommit is turned on by default. This configuration is +ignored and can be safely removed. Isolation level -~~~~~~~~~~~~~~~ +--------------- .. versionadded:: 1.6 @@ -200,7 +168,7 @@ such as ``REPEATABLE READ`` or ``SERIALIZABLE``, set it in the .. _postgresql-isolation-levels: http://www.postgresql.org/docs/devel/static/transaction-iso.html Indexes for ``varchar`` and ``text`` columns -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +-------------------------------------------- When specifying ``db_index=True`` on your model fields, Django typically outputs a single ``CREATE INDEX`` statement. However, if the database type @@ -457,7 +425,7 @@ Savepoints Both the Django ORM and MySQL (when using the InnoDB :ref:`storage engine `) support database :ref:`savepoints `, but this feature wasn't available in -Django until version 1.4 when such supports was added. +Django until version 1.4 when such support was added. If you use the MyISAM storage engine please be aware of the fact that you will receive database-generated errors if you try to use the :ref:`savepoint-related diff --git a/docs/ref/request-response.txt b/docs/ref/request-response.txt index 30f5e87100..6f620e17e2 100644 --- a/docs/ref/request-response.txt +++ b/docs/ref/request-response.txt @@ -814,8 +814,8 @@ generating large CSV files. .. admonition:: Performance considerations Django is designed for short-lived requests. Streaming responses will tie - a worker process and keep a database connection idle in transaction for - the entire duration of the response. This may result in poor performance. + a worker process for the entire duration of the response. This may result + in poor performance. Generally speaking, you should perform expensive tasks outside of the request-response cycle, rather than resorting to a streamed response. diff --git a/docs/releases/1.6.txt b/docs/releases/1.6.txt index d78d594c90..c55ef0ef38 100644 --- a/docs/releases/1.6.txt +++ b/docs/releases/1.6.txt @@ -30,6 +30,18 @@ prevention ` are turned on. If the default templates don't suit your tastes, you can use :ref:`custom project and app templates `. +Improved transaction management +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Django's transaction management was overhauled. Database-level autocommit is +now turned on by default. This makes transaction handling more explicit and +should improve performance. The existing APIs were deprecated, and new APIs +were introduced, as described in :doc:`/topics/db/transactions`. + +Please review carefully the list of :ref:`known backwards-incompatibilities +` to determine if you need to make changes in +your code. + Persistent database connections ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -148,6 +160,16 @@ Backwards incompatible changes in 1.6 deprecation timeline for a given feature, its removal may appear as a backwards incompatible change. +* Database-level autocommit is enabled by default in Django 1.6. While this + doesn't change the general spirit of Django's transaction management, there + are a few known backwards-incompatibities, described in the :ref:`transaction + management docs `. You should review your code + to determine if you're affected. + +* In previous versions, database-level autocommit was only an option for + PostgreSQL, and it was disabled by default. This option is now + :ref:`ignored `. + * The ``django.db.models.query.EmptyQuerySet`` can't be instantiated any more - it is only usable as a marker class for checking if :meth:`~django.db.models.query.QuerySet.none` has been called: diff --git a/docs/topics/db/sql.txt b/docs/topics/db/sql.txt index 6cc174a248..b2161fe65b 100644 --- a/docs/topics/db/sql.txt +++ b/docs/topics/db/sql.txt @@ -201,31 +201,32 @@ perform queries that don't map cleanly to models, or directly execute In these cases, you can always access the database directly, routing around the model layer entirely. -The object ``django.db.connection`` represents the -default database connection, and ``django.db.transaction`` represents the -default database transaction. To use the database connection, call -``connection.cursor()`` to get a cursor object. Then, call -``cursor.execute(sql, [params])`` to execute the SQL and ``cursor.fetchone()`` -or ``cursor.fetchall()`` to return the resulting rows. After performing a data -changing operation, you should then call -``transaction.commit_unless_managed()`` to ensure your changes are committed -to the database. If your query is purely a data retrieval operation, no commit -is required. For example:: +The object ``django.db.connection`` represents the default database +connection. To use the database connection, call ``connection.cursor()`` to +get a cursor object. Then, call ``cursor.execute(sql, [params])`` to execute +the SQL and ``cursor.fetchone()`` or ``cursor.fetchall()`` to return the +resulting rows. + +For example:: + + from django.db import connection def my_custom_sql(): - from django.db import connection, transaction cursor = connection.cursor() - # Data modifying operation - commit required cursor.execute("UPDATE bar SET foo = 1 WHERE baz = %s", [self.baz]) - transaction.commit_unless_managed() - # Data retrieval operation - no commit required cursor.execute("SELECT foo FROM bar WHERE baz = %s", [self.baz]) row = cursor.fetchone() return row +.. versionchanged:: 1.6 + In Django 1.5 and earlier, after performing a data changing operation, you + had to call ``transaction.commit_unless_managed()`` to ensure your changes + were committed to the database. Since Django now defaults to database-level + autocommit, this isn't necessary any longer. + If you are using :doc:`more than one database `, you can use ``django.db.connections`` to obtain the connection (and cursor) for a specific database. ``django.db.connections`` is a dictionary-like @@ -235,7 +236,6 @@ alias:: from django.db import connections cursor = connections['my_db_alias'].cursor() # Your code here... - transaction.commit_unless_managed(using='my_db_alias') By default, the Python DB API will return results without their field names, which means you end up with a ``list`` of values, rather than a @@ -260,27 +260,18 @@ Here is an example of the difference between the two:: >>> dictfetchall(cursor) [{'parent_id': None, 'id': 54360982L}, {'parent_id': None, 'id': 54360880L}] - -.. _transactions-and-raw-sql: - -Transactions and raw SQL ------------------------- - -When you make a raw SQL call, Django will automatically mark the -current transaction as dirty. You must then ensure that the -transaction containing those calls is closed correctly. See :ref:`the -notes on the requirements of Django's transaction handling -` for more details. - Connections and cursors ----------------------- ``connection`` and ``cursor`` mostly implement the standard Python DB-API -described in :pep:`249` (except when it comes to :doc:`transaction handling -`). If you're not familiar with the Python DB-API, note -that the SQL statement in ``cursor.execute()`` uses placeholders, ``"%s"``, -rather than adding parameters directly within the SQL. If you use this -technique, the underlying database library will automatically add quotes and -escaping to your parameter(s) as necessary. (Also note that Django expects the -``"%s"`` placeholder, *not* the ``"?"`` placeholder, which is used by the SQLite -Python bindings. This is for the sake of consistency and sanity.) +described in :pep:`249` — except when it comes to :doc:`transaction handling +`. + +If you're not familiar with the Python DB-API, note that the SQL statement in +``cursor.execute()`` uses placeholders, ``"%s"``, rather than adding +parameters directly within the SQL. If you use this technique, the underlying +database library will automatically escape your parameters as necessary. + +Also note that Django expects the ``"%s"`` placeholder, *not* the ``"?"`` +placeholder, which is used by the SQLite Python bindings. This is for the sake +of consistency and sanity. diff --git a/docs/topics/db/transactions.txt b/docs/topics/db/transactions.txt index e145edf149..93c4a3b11d 100644 --- a/docs/topics/db/transactions.txt +++ b/docs/topics/db/transactions.txt @@ -4,21 +4,24 @@ Managing database transactions .. module:: django.db.transaction -Django gives you a few ways to control how database transactions are managed, -if you're using a database that supports transactions. +Django gives you a few ways to control how database transactions are managed. Django's default transaction behavior ===================================== -Django's default behavior is to run with an open transaction which it -commits automatically when any built-in, data-altering model function is -called. For example, if you call ``model.save()`` or ``model.delete()``, the -change will be committed immediately. +Django's default behavior is to run in autocommit mode. Each query is +immediately committed to the database. :ref:`See below for details +`. -This is much like the auto-commit setting for most databases. As soon as you -perform an action that needs to write to the database, Django produces the -``INSERT``/``UPDATE``/``DELETE`` statements and then does the ``COMMIT``. -There's no implicit ``ROLLBACK``. +.. + Django uses transactions or savepoints automatically to guarantee the + integrity of ORM operations that require multiple queries, especially + :ref:`delete() ` and :ref:`update() + ` queries. + +.. versionchanged:: 1.6 + Previous version of Django featured :ref:`a more complicated default + behavior `. Tying transactions to HTTP requests =================================== @@ -26,7 +29,7 @@ Tying transactions to HTTP requests The recommended way to handle transactions in Web requests is to tie them to the request and response phases via Django's ``TransactionMiddleware``. -It works like this: When a request starts, Django starts a transaction. If the +It works like this. When a request starts, Django starts a transaction. If the response is produced without problems, Django commits any pending transactions. If the view function produces an exception, Django rolls back any pending transactions. @@ -47,11 +50,11 @@ view functions, but also for all middleware modules that come after it. So if you use the session middleware after the transaction middleware, session creation will be part of the transaction. -The various cache middlewares are an exception: -``CacheMiddleware``, :class:`~django.middleware.cache.UpdateCacheMiddleware`, -and :class:`~django.middleware.cache.FetchFromCacheMiddleware` are never -affected. Even when using database caching, Django's cache backend uses its own -database cursor (which is mapped to its own database connection internally). +The various cache middlewares are an exception: ``CacheMiddleware``, +:class:`~django.middleware.cache.UpdateCacheMiddleware`, and +:class:`~django.middleware.cache.FetchFromCacheMiddleware` are never affected. +Even when using database caching, Django's cache backend uses its own database +connection internally. .. note:: @@ -116,7 +119,7 @@ managers, too. .. function:: autocommit Use the ``autocommit`` decorator to switch a view function to Django's - default commit behavior, regardless of the global transaction setting. + default commit behavior. Example:: @@ -195,14 +198,14 @@ managers, too. Requirements for transaction handling ===================================== -Django requires that every transaction that is opened is closed before -the completion of a request. If you are using :func:`autocommit` (the -default commit mode) or :func:`commit_on_success`, this will be done -for you automatically (with the exception of :ref:`executing custom SQL -`). However, if you are manually managing -transactions (using the :func:`commit_manually` decorator), you must -ensure that the transaction is either committed or rolled back before -a request is completed. +Django requires that every transaction that is opened is closed before the +completion of a request. + +If you are using :func:`autocommit` (the default commit mode) or +:func:`commit_on_success`, this will be done for you automatically. However, +if you are manually managing transactions (using the :func:`commit_manually` +decorator), you must ensure that the transaction is either committed or rolled +back before a request is completed. This applies to all database operations, not just write operations. Even if your transaction only reads from the database, the transaction must @@ -231,17 +234,17 @@ How to globally deactivate transaction management ================================================= Control freaks can totally disable all transaction management by setting -:setting:`TRANSACTIONS_MANAGED` to ``True`` in the Django settings file. +:setting:`TRANSACTIONS_MANAGED` to ``True`` in the Django settings file. If +you do this, Django won't enable autocommit. You'll get the regular behavior +of the underlying database library. -If you do this, Django won't provide any automatic transaction management -whatsoever. Middleware will no longer implicitly commit transactions, and -you'll need to roll management yourself. This even requires you to commit -changes done by middleware somewhere else. +This requires you to commit explicitly every transaction, even those started +by Django or by third-party libraries. Thus, this is best used in situations +where you want to run your own transaction-controlling middleware or do +something really strange. -Thus, this is best used in situations where you want to run your own -transaction-controlling middleware or do something really strange. In almost -all situations, you'll be better off using the default behavior, or the -transaction middleware, and only modify selected functions as needed. +In almost all situations, you'll be better off using the default behavior, or +the transaction middleware, and only modify selected functions as needed. .. _topics-db-transactions-savepoints: @@ -308,8 +311,11 @@ The following example demonstrates the use of savepoints:: transaction.commit() +Database-specific notes +======================= + Transactions in MySQL -===================== +--------------------- If you're using MySQL, your tables may or may not support transactions; it depends on your MySQL version and the table types you're using. (By @@ -318,14 +324,14 @@ peculiarities are outside the scope of this article, but the MySQL site has `information on MySQL transactions`_. If your MySQL setup does *not* support transactions, then Django will function -in auto-commit mode: Statements will be executed and committed as soon as +in autocommit mode: Statements will be executed and committed as soon as they're called. If your MySQL setup *does* support transactions, Django will handle transactions as explained in this document. .. _information on MySQL transactions: http://dev.mysql.com/doc/refman/5.0/en/sql-syntax-transactions.html Handling exceptions within PostgreSQL transactions -================================================== +-------------------------------------------------- When a call to a PostgreSQL cursor raises an exception (typically ``IntegrityError``), all subsequent SQL in the same transaction will fail with @@ -338,7 +344,7 @@ force_insert/force_update flag, or invoking custom SQL. There are several ways to recover from this sort of error. Transaction rollback --------------------- +~~~~~~~~~~~~~~~~~~~~ The first option is to roll back the entire transaction. For example:: @@ -355,7 +361,7 @@ made by ``a.save()`` would be lost, even though that operation raised no error itself. Savepoint rollback ------------------- +~~~~~~~~~~~~~~~~~~ If you are using PostgreSQL 8 or later, you can use :ref:`savepoints ` to control the extent of a rollback. @@ -375,25 +381,135 @@ offending operation, rather than the entire transaction. For example:: In this example, ``a.save()`` will not be undone in the case where ``b.save()`` raises an exception. -Database-level autocommit -------------------------- +Under the hood +============== -With PostgreSQL 8.2 or later, there is an advanced option to run PostgreSQL -with :doc:`database-level autocommit `. If you use this option, -there is no constantly open transaction, so it is always possible to continue -after catching an exception. For example:: +.. _autocommit-details: - a.save() # succeeds - try: - b.save() # Could throw exception - except IntegrityError: - pass - c.save() # succeeds +Details on autocommit +--------------------- -.. note:: +In the SQL standards, each SQL query starts a transaction, unless one is +already in progress. Such transactions must then be committed or rolled back. - This is not the same as the :ref:`autocommit decorator - `. When using database level autocommit - there is no database transaction at all. The ``autocommit`` decorator - still uses transactions, automatically committing each transaction when - a database modifying operation occurs. +This isn't always convenient for application developers. To alleviate this +problem, most databases provide an autocommit mode. When autocommit is turned +on, each SQL query is wrapped in its own transaction. In other words, the +transaction is not only automatically started, but also automatically +committed. + +:pep:`249`, the Python Database API Specification v2.0, requires autocommit to +be initially turned off. Django overrides this default and turns autocommit +on. + +To avoid this, you can :ref:`deactivate the transaction management +`, but it isn't recommended. + +.. versionchanged:: 1.6 + Before Django 1.6, autocommit was turned off, and it was emulated by + forcing a commit after write operations in the ORM. + +.. warning:: + + If you're using the database API directly — for instance, you're running + SQL queries with ``cursor.execute()`` — be aware that autocommit is on, + and consider wrapping your operations in a transaction to ensure + consistency. + +.. _transaction-states: + +Transaction management states +----------------------------- + +At any time, each database connection is in one of these two states: + +- **auto mode**: autocommit is enabled; +- **managed mode**: autocommit is disabled. + +Django starts in auto mode. ``TransactionMiddleware``, +:func:`commit_on_success` and :func:`commit_manually` activate managed mode; +:func:`autocommit` activates auto mode. + +Internally, Django keeps a stack of states. Activations and deactivations must +be balanced. + +For example, at the beginning of each HTTP request, ``TransactionMiddleware`` +switches to managed mode; at the end of the request, it commits or rollbacks, +and switches back to auto mode. + +.. admonition:: Nesting decorators / context managers + + :func:`commit_on_success` has two effects: it changes the transaction + state, and defines an atomic transaction block. + + Nesting with :func:`autocommit` and :func:`commit_manually` will give the + expected results in terms of transaction state, but not in terms of + transaction semantics. Most often, the inner block will commit, breaking + the atomicity of the outer block. + +Django currently doesn't provide any APIs to create transactions in auto mode. + +.. _transactions-changes-from-1.5: + +Changes from Django 1.5 and earlier +=================================== + +Since version 1.6, Django uses database-level autocommit in auto mode. + +Previously, it implemented application-level autocommit by triggering a commit +after each ORM write. + +As a consequence, each database query (for instance, an +ORM read) started a transaction that lasted until the next ORM write. Such +"automatic transactions" no longer exist in Django 1.6. + +There are four known scenarios where this is backwards-incompatible. + +Note that managed mode isn't affected at all. This section assumes auto mode. +See the :ref:`description of modes ` above. + +Sequences of custom SQL queries +------------------------------- + +If you're executing several :ref:`custom SQL queries ` +in a row, each one now runs in its own transaction, instead of sharing the +same "automatic transaction". If you need to enforce atomicity, you must wrap +the sequence of queries in :func:`commit_on_success`. + +To check for this problem, look for calls to ``cursor.execute()``. They're +usually followed by a call to ``transaction.commit_unless_managed``, which +isn't necessary any more and should be removed. + +Select for update +----------------- + +If you were relying on "automatic transactions" to provide locking between +:meth:`~django.db.models.query.QuerySet.select_for_update` and a subsequent +write operation — an extremely fragile design, but nonetheless possible — you +must wrap the relevant code in :func:`commit_on_success`. + +Using a high isolation level +---------------------------- + +If you were using the "repeatable read" isolation level or higher, and if you +relied on "automatic transactions" to guarantee consistency between successive +reads, the new behavior is backwards-incompatible. To maintain consistency, +you must wrap such sequences in :func:`commit_on_success`. + +MySQL defaults to "repeatable read" and SQLite to "serializable"; they may be +affected by this problem. + +At the "read committed" isolation level or lower, "automatic transactions" +have no effect on the semantics of any sequence of ORM operations. + +PostgreSQL and Oracle default to "read committed" and aren't affected, unless +you changed the isolation level. + +Using unsupported database features +----------------------------------- + +With triggers, views, or functions, it's possible to make ORM reads result in +database modifications. Django 1.5 and earlier doesn't deal with this case and +it's theoretically possible to observe a different behavior after upgrading to +Django 1.6 or later. In doubt, use :func:`commit_on_success` to enforce +integrity. From 14aa563f51c1a8a2ece52ca6b9e66aed9c89c0fd Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Tue, 5 Mar 2013 20:40:57 +0100 Subject: [PATCH 10/32] Removed superfluous code now that connections use autocommit by default. --- django/db/backends/creation.py | 15 +-------------- .../db/backends/postgresql_psycopg2/creation.py | 8 -------- 2 files changed, 1 insertion(+), 22 deletions(-) diff --git a/django/db/backends/creation.py b/django/db/backends/creation.py index aa4fb82b12..c0d0a5958b 100644 --- a/django/db/backends/creation.py +++ b/django/db/backends/creation.py @@ -383,10 +383,7 @@ class BaseDatabaseCreation(object): qn = self.connection.ops.quote_name - # Create the test database and connect to it. We need to autocommit - # if the database supports it because PostgreSQL doesn't allow - # CREATE/DROP DATABASE statements within transactions. - self._prepare_for_test_db_ddl() + # Create the test database and connect to it. cursor = self.connection.cursor() try: cursor.execute( @@ -454,7 +451,6 @@ class BaseDatabaseCreation(object): # to do so, because it's not allowed to delete a database while being # connected to it. cursor = self.connection.cursor() - self._prepare_for_test_db_ddl() # Wait to avoid "database is being accessed by other users" errors. time.sleep(1) cursor.execute("DROP DATABASE %s" @@ -472,15 +468,6 @@ class BaseDatabaseCreation(object): "BaseDatabaseWrapper.", PendingDeprecationWarning, stacklevel=2) return self.connection.set_autocommit() - def _prepare_for_test_db_ddl(self): - """ - Internal implementation - Hook for tasks that should be performed - before the ``CREATE DATABASE``/``DROP DATABASE`` clauses used by - testing code to create/ destroy test databases. Needed e.g. in - PostgreSQL to rollback and close any active transaction. - """ - pass - def sql_table_creation_suffix(self): """ SQL to append to the end of the test table creation statements. diff --git a/django/db/backends/postgresql_psycopg2/creation.py b/django/db/backends/postgresql_psycopg2/creation.py index e6400d79a1..3ba2158103 100644 --- a/django/db/backends/postgresql_psycopg2/creation.py +++ b/django/db/backends/postgresql_psycopg2/creation.py @@ -77,11 +77,3 @@ class DatabaseCreation(BaseDatabaseCreation): output.append(get_index_sql('%s_%s_like' % (db_table, f.column), ' text_pattern_ops')) return output - - def _prepare_for_test_db_ddl(self): - """Rollback and close the active transaction.""" - # Make sure there is an open connection. - self.connection.cursor() - self.connection.connection.rollback() - self.connection.connection.set_isolation_level( - psycopg2.extensions.ISOLATION_LEVEL_AUTOCOMMIT) From ba5138b1c0253fcf390b7509ad7b954117b3be88 Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Mon, 4 Mar 2013 13:12:59 +0100 Subject: [PATCH 11/32] Deprecated transaction.commit/rollback_unless_managed. Since "unless managed" now means "if database-level autocommit", committing or rolling back doesn't have any effect. Restored transactional integrity in a few places that relied on automatically-started transactions with a transitory API. --- django/contrib/gis/utils/layermapping.py | 4 - django/contrib/sessions/backends/db.py | 1 - django/core/cache/backends/db.py | 7 +- .../management/commands/createcachetable.py | 21 +++-- django/core/management/commands/flush.py | 9 +- django/core/management/commands/loaddata.py | 1 - django/core/management/commands/syncdb.py | 77 ++++++++--------- django/db/__init__.py | 11 --- django/db/backends/__init__.py | 21 ----- django/db/backends/dummy/base.py | 2 - django/db/models/base.py | 84 +++++++++---------- django/db/models/deletion.py | 2 - django/db/models/query.py | 4 - django/db/transaction.py | 27 ++++-- django/test/testcases.py | 19 +---- docs/internals/deprecation.txt | 2 + tests/transactions_regress/tests.py | 32 ------- 17 files changed, 116 insertions(+), 208 deletions(-) diff --git a/django/contrib/gis/utils/layermapping.py b/django/contrib/gis/utils/layermapping.py index e4ea44d0d2..51f70f5350 100644 --- a/django/contrib/gis/utils/layermapping.py +++ b/django/contrib/gis/utils/layermapping.py @@ -555,10 +555,6 @@ class LayerMapping(object): except SystemExit: raise except Exception as msg: - if self.transaction_mode == 'autocommit': - # Rolling back the transaction so that other model saves - # will work. - transaction.rollback_unless_managed() if strict: # Bailing out if the `strict` keyword is set. if not silent: diff --git a/django/contrib/sessions/backends/db.py b/django/contrib/sessions/backends/db.py index 47e89b66e5..30da0b7a10 100644 --- a/django/contrib/sessions/backends/db.py +++ b/django/contrib/sessions/backends/db.py @@ -74,7 +74,6 @@ class SessionStore(SessionBase): @classmethod def clear_expired(cls): Session.objects.filter(expire_date__lt=timezone.now()).delete() - transaction.commit_unless_managed() # At bottom to avoid circular import diff --git a/django/core/cache/backends/db.py b/django/core/cache/backends/db.py index bb91d8cb05..53d7f4d22a 100644 --- a/django/core/cache/backends/db.py +++ b/django/core/cache/backends/db.py @@ -10,7 +10,7 @@ except ImportError: from django.conf import settings from django.core.cache.backends.base import BaseCache -from django.db import connections, router, transaction, DatabaseError +from django.db import connections, router, DatabaseError from django.utils import timezone, six from django.utils.encoding import force_bytes @@ -70,7 +70,6 @@ class DatabaseCache(BaseDatabaseCache): cursor = connections[db].cursor() cursor.execute("DELETE FROM %s " "WHERE cache_key = %%s" % table, [key]) - transaction.commit_unless_managed(using=db) return default value = connections[db].ops.process_clob(row[1]) return pickle.loads(base64.b64decode(force_bytes(value))) @@ -124,10 +123,8 @@ class DatabaseCache(BaseDatabaseCache): [key, b64encoded, connections[db].ops.value_to_db_datetime(exp)]) except DatabaseError: # To be threadsafe, updates/inserts are allowed to fail silently - transaction.rollback_unless_managed(using=db) return False else: - transaction.commit_unless_managed(using=db) return True def delete(self, key, version=None): @@ -139,7 +136,6 @@ class DatabaseCache(BaseDatabaseCache): cursor = connections[db].cursor() cursor.execute("DELETE FROM %s WHERE cache_key = %%s" % table, [key]) - transaction.commit_unless_managed(using=db) def has_key(self, key, version=None): key = self.make_key(key, version=version) @@ -184,7 +180,6 @@ class DatabaseCache(BaseDatabaseCache): table = connections[db].ops.quote_name(self._table) cursor = connections[db].cursor() cursor.execute('DELETE FROM %s' % table) - transaction.commit_unless_managed(using=db) # For backwards compatibility class CacheClass(DatabaseCache): diff --git a/django/core/management/commands/createcachetable.py b/django/core/management/commands/createcachetable.py index 411042ee76..94b6b09400 100644 --- a/django/core/management/commands/createcachetable.py +++ b/django/core/management/commands/createcachetable.py @@ -53,14 +53,13 @@ class Command(LabelCommand): for i, line in enumerate(table_output): full_statement.append(' %s%s' % (line, i < len(table_output)-1 and ',' or '')) full_statement.append(');') - curs = connection.cursor() - try: - curs.execute("\n".join(full_statement)) - except DatabaseError as e: - transaction.rollback_unless_managed(using=db) - raise CommandError( - "Cache table '%s' could not be created.\nThe error was: %s." % - (tablename, force_text(e))) - for statement in index_output: - curs.execute(statement) - transaction.commit_unless_managed(using=db) + with transaction.commit_on_success_unless_managed(): + curs = connection.cursor() + try: + curs.execute("\n".join(full_statement)) + except DatabaseError as e: + raise CommandError( + "Cache table '%s' could not be created.\nThe error was: %s." % + (tablename, force_text(e))) + for statement in index_output: + curs.execute(statement) diff --git a/django/core/management/commands/flush.py b/django/core/management/commands/flush.py index 3bf1e9c672..9bd65e735c 100644 --- a/django/core/management/commands/flush.py +++ b/django/core/management/commands/flush.py @@ -57,18 +57,17 @@ Are you sure you want to do this? if confirm == 'yes': try: - cursor = connection.cursor() - for sql in sql_list: - cursor.execute(sql) + with transaction.commit_on_success_unless_managed(): + cursor = connection.cursor() + for sql in sql_list: + cursor.execute(sql) except Exception as e: - transaction.rollback_unless_managed(using=db) raise CommandError("""Database %s couldn't be flushed. Possible reasons: * The database isn't running or isn't configured correctly. * At least one of the expected database tables doesn't exist. * The SQL was invalid. Hint: Look at the output of 'django-admin.py sqlflush'. That's the SQL this command wasn't able to run. The full error: %s""" % (connection.settings_dict['NAME'], e)) - transaction.commit_unless_managed(using=db) # Emit the post sync signal. This allows individual # applications to respond as if the database had been diff --git a/django/core/management/commands/loaddata.py b/django/core/management/commands/loaddata.py index 77b9a44a43..674e6be7b0 100644 --- a/django/core/management/commands/loaddata.py +++ b/django/core/management/commands/loaddata.py @@ -73,7 +73,6 @@ class Command(BaseCommand): # Start transaction management. All fixtures are installed in a # single transaction to ensure that all references are resolved. if commit: - transaction.commit_unless_managed(using=self.using) transaction.enter_transaction_management(using=self.using) class SingleZipReader(zipfile.ZipFile): diff --git a/django/core/management/commands/syncdb.py b/django/core/management/commands/syncdb.py index 4ce2910fb5..e7e11a8c90 100644 --- a/django/core/management/commands/syncdb.py +++ b/django/core/management/commands/syncdb.py @@ -83,26 +83,25 @@ class Command(NoArgsCommand): # Create the tables for each model if verbosity >= 1: self.stdout.write("Creating tables ...\n") - for app_name, model_list in manifest.items(): - for model in model_list: - # Create the model's database table, if it doesn't already exist. - if verbosity >= 3: - self.stdout.write("Processing %s.%s model\n" % (app_name, model._meta.object_name)) - sql, references = connection.creation.sql_create_model(model, self.style, seen_models) - seen_models.add(model) - created_models.add(model) - for refto, refs in references.items(): - pending_references.setdefault(refto, []).extend(refs) - if refto in seen_models: - sql.extend(connection.creation.sql_for_pending_references(refto, self.style, pending_references)) - sql.extend(connection.creation.sql_for_pending_references(model, self.style, pending_references)) - if verbosity >= 1 and sql: - self.stdout.write("Creating table %s\n" % model._meta.db_table) - for statement in sql: - cursor.execute(statement) - tables.append(connection.introspection.table_name_converter(model._meta.db_table)) - - transaction.commit_unless_managed(using=db) + with transaction.commit_on_success_unless_managed(using=db): + for app_name, model_list in manifest.items(): + for model in model_list: + # Create the model's database table, if it doesn't already exist. + if verbosity >= 3: + self.stdout.write("Processing %s.%s model\n" % (app_name, model._meta.object_name)) + sql, references = connection.creation.sql_create_model(model, self.style, seen_models) + seen_models.add(model) + created_models.add(model) + for refto, refs in references.items(): + pending_references.setdefault(refto, []).extend(refs) + if refto in seen_models: + sql.extend(connection.creation.sql_for_pending_references(refto, self.style, pending_references)) + sql.extend(connection.creation.sql_for_pending_references(model, self.style, pending_references)) + if verbosity >= 1 and sql: + self.stdout.write("Creating table %s\n" % model._meta.db_table) + for statement in sql: + cursor.execute(statement) + tables.append(connection.introspection.table_name_converter(model._meta.db_table)) # Send the post_syncdb signal, so individual apps can do whatever they need # to do at this point. @@ -122,17 +121,16 @@ class Command(NoArgsCommand): if custom_sql: if verbosity >= 2: self.stdout.write("Installing custom SQL for %s.%s model\n" % (app_name, model._meta.object_name)) - try: - for sql in custom_sql: - cursor.execute(sql) - except Exception as e: - self.stderr.write("Failed to install custom SQL for %s.%s model: %s\n" % \ - (app_name, model._meta.object_name, e)) - if show_traceback: - traceback.print_exc() - transaction.rollback_unless_managed(using=db) - else: - transaction.commit_unless_managed(using=db) + with transaction.commit_on_success_unless_managed(using=db): + try: + for sql in custom_sql: + cursor.execute(sql) + except Exception as e: + self.stderr.write("Failed to install custom SQL for %s.%s model: %s\n" % \ + (app_name, model._meta.object_name, e)) + if show_traceback: + traceback.print_exc() + raise else: if verbosity >= 3: self.stdout.write("No custom SQL for %s.%s model\n" % (app_name, model._meta.object_name)) @@ -147,15 +145,14 @@ class Command(NoArgsCommand): if index_sql: if verbosity >= 2: self.stdout.write("Installing index for %s.%s model\n" % (app_name, model._meta.object_name)) - try: - for sql in index_sql: - cursor.execute(sql) - except Exception as e: - self.stderr.write("Failed to install index for %s.%s model: %s\n" % \ - (app_name, model._meta.object_name, e)) - transaction.rollback_unless_managed(using=db) - else: - transaction.commit_unless_managed(using=db) + with transaction.commit_on_success_unless_managed(using=db): + try: + for sql in index_sql: + cursor.execute(sql) + except Exception as e: + self.stderr.write("Failed to install index for %s.%s model: %s\n" % \ + (app_name, model._meta.object_name, e)) + raise # Load initial_data fixtures (unless that has been disabled) if load_initial_data: diff --git a/django/db/__init__.py b/django/db/__init__.py index 60fe8f6ce2..13ba68ba7e 100644 --- a/django/db/__init__.py +++ b/django/db/__init__.py @@ -77,14 +77,3 @@ def close_old_connections(**kwargs): conn.close_if_unusable_or_obsolete() signals.request_started.connect(close_old_connections) signals.request_finished.connect(close_old_connections) - -# Register an event that rolls back the connections -# when a Django request has an exception. -def _rollback_on_exception(**kwargs): - from django.db import transaction - for conn in connections: - try: - transaction.rollback_unless_managed(using=conn) - except DatabaseError: - pass -signals.got_request_exception.connect(_rollback_on_exception) diff --git a/django/db/backends/__init__.py b/django/db/backends/__init__.py index 4031e8f668..848f6df2d6 100644 --- a/django/db/backends/__init__.py +++ b/django/db/backends/__init__.py @@ -339,27 +339,6 @@ class BaseDatabaseWrapper(object): return self.transaction_state[-1] return settings.TRANSACTIONS_MANAGED - def commit_unless_managed(self): - """ - Commits changes if the system is not in managed transaction mode. - """ - self.validate_thread_sharing() - if not self.is_managed(): - self.commit() - self.clean_savepoints() - else: - self.set_dirty() - - def rollback_unless_managed(self): - """ - Rolls back changes if the system is not in managed transaction mode. - """ - self.validate_thread_sharing() - if not self.is_managed(): - self.rollback() - else: - self.set_dirty() - ##### Foreign key constraints checks handling ##### @contextmanager diff --git a/django/db/backends/dummy/base.py b/django/db/backends/dummy/base.py index 02f0b6462d..9a220ffd8b 100644 --- a/django/db/backends/dummy/base.py +++ b/django/db/backends/dummy/base.py @@ -58,8 +58,6 @@ class DatabaseWrapper(BaseDatabaseWrapper): _set_autocommit = complain set_dirty = complain set_clean = complain - commit_unless_managed = complain - rollback_unless_managed = ignore def __init__(self, *args, **kwargs): super(DatabaseWrapper, self).__init__(*args, **kwargs) diff --git a/django/db/models/base.py b/django/db/models/base.py index 543cdfc165..ab0e42d461 100644 --- a/django/db/models/base.py +++ b/django/db/models/base.py @@ -609,48 +609,48 @@ class Model(six.with_metaclass(ModelBase)): if update_fields: non_pks = [f for f in non_pks if f.name in update_fields or f.attname in update_fields] - # First, try an UPDATE. If that doesn't update anything, do an INSERT. - pk_val = self._get_pk_val(meta) - pk_set = pk_val is not None - record_exists = True - manager = cls._base_manager - if pk_set: - # Determine if we should do an update (pk already exists, forced update, - # no force_insert) - if ((force_update or update_fields) or (not force_insert and - manager.using(using).filter(pk=pk_val).exists())): - if force_update or non_pks: - values = [(f, None, (raw and getattr(self, f.attname) or f.pre_save(self, False))) for f in non_pks] - if values: - rows = manager.using(using).filter(pk=pk_val)._update(values) - if force_update and not rows: - raise DatabaseError("Forced update did not affect any rows.") - if update_fields and not rows: - raise DatabaseError("Save with update_fields did not affect any rows.") - else: + with transaction.commit_on_success_unless_managed(using=using): + # First, try an UPDATE. If that doesn't update anything, do an INSERT. + pk_val = self._get_pk_val(meta) + pk_set = pk_val is not None + record_exists = True + manager = cls._base_manager + if pk_set: + # Determine if we should do an update (pk already exists, forced update, + # no force_insert) + if ((force_update or update_fields) or (not force_insert and + manager.using(using).filter(pk=pk_val).exists())): + if force_update or non_pks: + values = [(f, None, (raw and getattr(self, f.attname) or f.pre_save(self, False))) for f in non_pks] + if values: + rows = manager.using(using).filter(pk=pk_val)._update(values) + if force_update and not rows: + raise DatabaseError("Forced update did not affect any rows.") + if update_fields and not rows: + raise DatabaseError("Save with update_fields did not affect any rows.") + else: + record_exists = False + if not pk_set or not record_exists: + if meta.order_with_respect_to: + # If this is a model with an order_with_respect_to + # autopopulate the _order field + field = meta.order_with_respect_to + order_value = manager.using(using).filter(**{field.name: getattr(self, field.attname)}).count() + self._order = order_value + + fields = meta.local_fields + if not pk_set: + if force_update or update_fields: + raise ValueError("Cannot force an update in save() with no primary key.") + fields = [f for f in fields if not isinstance(f, AutoField)] + record_exists = False - if not pk_set or not record_exists: - if meta.order_with_respect_to: - # If this is a model with an order_with_respect_to - # autopopulate the _order field - field = meta.order_with_respect_to - order_value = manager.using(using).filter(**{field.name: getattr(self, field.attname)}).count() - self._order = order_value - fields = meta.local_fields - if not pk_set: - if force_update or update_fields: - raise ValueError("Cannot force an update in save() with no primary key.") - fields = [f for f in fields if not isinstance(f, AutoField)] + update_pk = bool(meta.has_auto_field and not pk_set) + result = manager._insert([self], fields=fields, return_id=update_pk, using=using, raw=raw) - record_exists = False - - update_pk = bool(meta.has_auto_field and not pk_set) - result = manager._insert([self], fields=fields, return_id=update_pk, using=using, raw=raw) - - if update_pk: - setattr(self, meta.pk.attname, result) - transaction.commit_unless_managed(using=using) + if update_pk: + setattr(self, meta.pk.attname, result) # Store the database on which the object was saved self._state.db = using @@ -963,9 +963,9 @@ def method_set_order(ordered_obj, self, id_list, using=None): order_name = ordered_obj._meta.order_with_respect_to.name # FIXME: It would be nice if there was an "update many" version of update # for situations like this. - for i, j in enumerate(id_list): - ordered_obj.objects.filter(**{'pk': j, order_name: rel_val}).update(_order=i) - transaction.commit_unless_managed(using=using) + with transaction.commit_on_success_unless_managed(using=using): + for i, j in enumerate(id_list): + ordered_obj.objects.filter(**{'pk': j, order_name: rel_val}).update(_order=i) def method_get_order(ordered_obj, self): diff --git a/django/db/models/deletion.py b/django/db/models/deletion.py index 93ef0006cb..26f63391d5 100644 --- a/django/db/models/deletion.py +++ b/django/db/models/deletion.py @@ -62,8 +62,6 @@ def force_managed(func): func(self, *args, **kwargs) if forced_managed: transaction.commit(using=self.using) - else: - transaction.commit_unless_managed(using=self.using) finally: if forced_managed: transaction.leave_transaction_management(using=self.using) diff --git a/django/db/models/query.py b/django/db/models/query.py index b41007ee4f..22f71c6aee 100644 --- a/django/db/models/query.py +++ b/django/db/models/query.py @@ -460,8 +460,6 @@ class QuerySet(object): self._batched_insert(objs_without_pk, fields, batch_size) if forced_managed: transaction.commit(using=self.db) - else: - transaction.commit_unless_managed(using=self.db) finally: if forced_managed: transaction.leave_transaction_management(using=self.db) @@ -590,8 +588,6 @@ class QuerySet(object): rows = query.get_compiler(self.db).execute_sql(None) if forced_managed: transaction.commit(using=self.db) - else: - transaction.commit_unless_managed(using=self.db) finally: if forced_managed: transaction.leave_transaction_management(using=self.db) diff --git a/django/db/transaction.py b/django/db/transaction.py index dd48e14bf4..a8e80c6c02 100644 --- a/django/db/transaction.py +++ b/django/db/transaction.py @@ -123,16 +123,12 @@ def managed(flag=True, using=None): PendingDeprecationWarning, stacklevel=2) def commit_unless_managed(using=None): - """ - Commits changes if the system is not in managed transaction mode. - """ - get_connection(using).commit_unless_managed() + warnings.warn("'commit_unless_managed' is now a no-op.", + PendingDeprecationWarning, stacklevel=2) def rollback_unless_managed(using=None): - """ - Rolls back changes if the system is not in managed transaction mode. - """ - get_connection(using).rollback_unless_managed() + warnings.warn("'rollback_unless_managed' is now a no-op.", + PendingDeprecationWarning, stacklevel=2) ############### # Public APIs # @@ -280,3 +276,18 @@ def commit_manually(using=None): leave_transaction_management(using=using) return _transaction_func(entering, exiting, using) + +def commit_on_success_unless_managed(using=None): + """ + Transitory API to preserve backwards-compatibility while refactoring. + """ + if is_managed(using): + def entering(using): + pass + + def exiting(exc_value, using): + set_dirty(using=using) + + return _transaction_func(entering, exiting, using) + else: + return commit_on_success(using) diff --git a/django/test/testcases.py b/django/test/testcases.py index 4b9116e3bc..55673dca25 100644 --- a/django/test/testcases.py +++ b/django/test/testcases.py @@ -157,14 +157,6 @@ class DocTestRunner(doctest.DocTestRunner): doctest.DocTestRunner.__init__(self, *args, **kwargs) self.optionflags = doctest.ELLIPSIS - def report_unexpected_exception(self, out, test, example, exc_info): - doctest.DocTestRunner.report_unexpected_exception(self, out, test, - example, exc_info) - # Rollback, in case of database errors. Otherwise they'd have - # side effects on other tests. - for conn in connections: - transaction.rollback_unless_managed(using=conn) - class _AssertNumQueriesContext(CaptureQueriesContext): def __init__(self, test_case, num, connection): @@ -490,14 +482,10 @@ class TransactionTestCase(SimpleTestCase): conn.ops.sequence_reset_by_name_sql(no_style(), conn.introspection.sequence_list()) if sql_list: - try: + with transaction.commit_on_success_unless_managed(using=db_name): cursor = conn.cursor() for sql in sql_list: cursor.execute(sql) - except Exception: - transaction.rollback_unless_managed(using=db_name) - raise - transaction.commit_unless_managed(using=db_name) def _fixture_setup(self): for db_name in self._databases_names(include_mirrors=False): @@ -537,11 +525,6 @@ class TransactionTestCase(SimpleTestCase): conn.close() def _fixture_teardown(self): - # Roll back any pending transactions in order to avoid a deadlock - # during flush when TEST_MIRROR is used (#18984). - for conn in connections.all(): - conn.rollback_unless_managed() - for db in self._databases_names(include_mirrors=False): call_command('flush', verbosity=0, interactive=False, database=db, skip_validation=True, reset_sequences=False) diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index 74fbb563f0..a81b16278f 100644 --- a/docs/internals/deprecation.txt +++ b/docs/internals/deprecation.txt @@ -352,6 +352,8 @@ these changes. - ``django.db.close_connection()`` - ``django.db.backends.creation.BaseDatabaseCreation.set_autocommit()`` - ``django.db.transaction.managed()`` + - ``django.db.transaction.commit_unless_managed()`` + - ``django.db.transaction.rollback_unless_managed()`` 2.0 --- diff --git a/tests/transactions_regress/tests.py b/tests/transactions_regress/tests.py index e6750cddcf..3d571fba2f 100644 --- a/tests/transactions_regress/tests.py +++ b/tests/transactions_regress/tests.py @@ -208,38 +208,6 @@ class TestNewConnection(TransactionTestCase): connection.leave_transaction_management() self.assertEqual(orig_dirty, connection._dirty) - # TODO: update this test to account for database-level autocommit. - @expectedFailure - def test_commit_unless_managed(self): - cursor = connection.cursor() - cursor.execute("INSERT into transactions_regress_mod (fld) values (2)") - connection.commit_unless_managed() - self.assertFalse(connection.is_dirty()) - self.assertEqual(len(Mod.objects.all()), 1) - self.assertTrue(connection.is_dirty()) - connection.commit_unless_managed() - self.assertFalse(connection.is_dirty()) - - # TODO: update this test to account for database-level autocommit. - @expectedFailure - def test_commit_unless_managed_in_managed(self): - cursor = connection.cursor() - connection.enter_transaction_management() - cursor.execute("INSERT into transactions_regress_mod (fld) values (2)") - connection.commit_unless_managed() - self.assertTrue(connection.is_dirty()) - connection.rollback() - self.assertFalse(connection.is_dirty()) - self.assertEqual(len(Mod.objects.all()), 0) - connection.commit() - connection.leave_transaction_management() - self.assertFalse(connection.is_dirty()) - self.assertEqual(len(Mod.objects.all()), 0) - self.assertTrue(connection.is_dirty()) - connection.commit_unless_managed() - self.assertFalse(connection.is_dirty()) - self.assertEqual(len(Mod.objects.all()), 0) - @skipUnless(connection.vendor == 'postgresql', "This test only valid for PostgreSQL") From 3bdc7a6a70bb030324fdebe9b1dce1fa5358f0c6 Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Mon, 4 Mar 2013 15:24:01 +0100 Subject: [PATCH 12/32] Deprecated transaction.is_managed(). It's synchronized with the autocommit flag. --- django/db/backends/__init__.py | 30 ++++++++++++------------------ django/db/models/deletion.py | 2 +- django/db/models/query.py | 4 ++-- django/db/transaction.py | 12 +++++------- django/middleware/transaction.py | 2 +- docs/internals/deprecation.txt | 1 + tests/middleware/tests.py | 2 +- 7 files changed, 23 insertions(+), 30 deletions(-) diff --git a/django/db/backends/__init__.py b/django/db/backends/__init__.py index 848f6df2d6..499bc32113 100644 --- a/django/db/backends/__init__.py +++ b/django/db/backends/__init__.py @@ -256,11 +256,12 @@ class BaseDatabaseWrapper(object): """ self.transaction_state.append(managed) - if managed and self.autocommit: - self.set_autocommit(False) - if not managed and self.is_dirty() and not forced: self.commit() + self.set_clean() + + if managed == self.autocommit: + self.set_autocommit(not managed) def leave_transaction_management(self): """ @@ -274,19 +275,20 @@ class BaseDatabaseWrapper(object): raise TransactionManagementError( "This code isn't under transaction management") - # That's the next state -- we already left the previous state behind. - managed = self.is_managed() + if self.transaction_state: + managed = self.transaction_state[-1] + else: + managed = settings.TRANSACTIONS_MANAGED if self._dirty: self.rollback() - if not managed and not self.autocommit: - self.set_autocommit(True) + if managed == self.autocommit: + self.set_autocommit(not managed) raise TransactionManagementError( "Transaction managed block ended with pending COMMIT/ROLLBACK") - if not managed and not self.autocommit: - self.set_autocommit(True) - + if managed == self.autocommit: + self.set_autocommit(not managed) def set_autocommit(self, autocommit=True): """ @@ -331,14 +333,6 @@ class BaseDatabaseWrapper(object): self._dirty = False self.clean_savepoints() - def is_managed(self): - """ - Checks whether the transaction manager is in manual or in auto state. - """ - if self.transaction_state: - return self.transaction_state[-1] - return settings.TRANSACTIONS_MANAGED - ##### Foreign key constraints checks handling ##### @contextmanager diff --git a/django/db/models/deletion.py b/django/db/models/deletion.py index 26f63391d5..27184c9350 100644 --- a/django/db/models/deletion.py +++ b/django/db/models/deletion.py @@ -53,7 +53,7 @@ def DO_NOTHING(collector, field, sub_objs, using): def force_managed(func): @wraps(func) def decorated(self, *args, **kwargs): - if not transaction.is_managed(using=self.using): + if transaction.get_autocommit(using=self.using): transaction.enter_transaction_management(using=self.using, forced=True) forced_managed = True else: diff --git a/django/db/models/query.py b/django/db/models/query.py index 22f71c6aee..834fe363b4 100644 --- a/django/db/models/query.py +++ b/django/db/models/query.py @@ -442,7 +442,7 @@ class QuerySet(object): self._for_write = True connection = connections[self.db] fields = self.model._meta.local_fields - if not transaction.is_managed(using=self.db): + if transaction.get_autocommit(using=self.db): transaction.enter_transaction_management(using=self.db, forced=True) forced_managed = True else: @@ -579,7 +579,7 @@ class QuerySet(object): self._for_write = True query = self.query.clone(sql.UpdateQuery) query.add_update_values(kwargs) - if not transaction.is_managed(using=self.db): + if transaction.get_autocommit(using=self.db): transaction.enter_transaction_management(using=self.db, forced=True) forced_managed = True else: diff --git a/django/db/transaction.py b/django/db/transaction.py index a8e80c6c02..49b67f4122 100644 --- a/django/db/transaction.py +++ b/django/db/transaction.py @@ -113,10 +113,8 @@ def clean_savepoints(using=None): get_connection(using).clean_savepoints() def is_managed(using=None): - """ - Checks whether the transaction manager is in manual or in auto state. - """ - return get_connection(using).is_managed() + warnings.warn("'is_managed' is deprecated.", + PendingDeprecationWarning, stacklevel=2) def managed(flag=True, using=None): warnings.warn("'managed' no longer serves a purpose.", @@ -281,7 +279,9 @@ def commit_on_success_unless_managed(using=None): """ Transitory API to preserve backwards-compatibility while refactoring. """ - if is_managed(using): + if get_autocommit(using): + return commit_on_success(using) + else: def entering(using): pass @@ -289,5 +289,3 @@ def commit_on_success_unless_managed(using=None): set_dirty(using=using) return _transaction_func(entering, exiting, using) - else: - return commit_on_success(using) diff --git a/django/middleware/transaction.py b/django/middleware/transaction.py index b5a07a02b7..35f765d99f 100644 --- a/django/middleware/transaction.py +++ b/django/middleware/transaction.py @@ -23,7 +23,7 @@ class TransactionMiddleware(object): def process_response(self, request, response): """Commits and leaves transaction management.""" - if transaction.is_managed(): + if not transaction.get_autocommit(): if transaction.is_dirty(): # Note: it is possible that the commit fails. If the reason is # closed connection or some similar reason, then there is diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index a81b16278f..1c8618713a 100644 --- a/docs/internals/deprecation.txt +++ b/docs/internals/deprecation.txt @@ -351,6 +351,7 @@ these changes. * The following private APIs will be removed: - ``django.db.close_connection()`` - ``django.db.backends.creation.BaseDatabaseCreation.set_autocommit()`` + - ``django.db.transaction.is_managed()`` - ``django.db.transaction.managed()`` - ``django.db.transaction.commit_unless_managed()`` - ``django.db.transaction.rollback_unless_managed()`` diff --git a/tests/middleware/tests.py b/tests/middleware/tests.py index 17751dd158..e704fce342 100644 --- a/tests/middleware/tests.py +++ b/tests/middleware/tests.py @@ -689,7 +689,7 @@ class TransactionMiddlewareTest(TransactionTestCase): def test_request(self): TransactionMiddleware().process_request(self.request) - self.assertTrue(transaction.is_managed()) + self.assertFalse(transaction.get_autocommit()) def test_managed_response(self): transaction.enter_transaction_management() From 918f44e3ae650ff124067425d31c9d3deeba2224 Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Thu, 28 Feb 2013 18:30:03 +0100 Subject: [PATCH 13/32] Moved standard SQL for savepoints in the base backend. These methods are only called when uses_savepoints = True anyway. --- django/db/backends/__init__.py | 6 +++--- django/db/backends/mysql/base.py | 9 --------- django/db/backends/postgresql_psycopg2/operations.py | 9 --------- 3 files changed, 3 insertions(+), 21 deletions(-) diff --git a/django/db/backends/__init__.py b/django/db/backends/__init__.py index 499bc32113..95a7bdc3b9 100644 --- a/django/db/backends/__init__.py +++ b/django/db/backends/__init__.py @@ -863,19 +863,19 @@ class BaseDatabaseOperations(object): "uses_savepoints" feature is True. The "sid" parameter is a string for the savepoint id. """ - raise NotImplementedError + return "SAVEPOINT %s" % self.quote_name(sid) def savepoint_commit_sql(self, sid): """ Returns the SQL for committing the given savepoint. """ - raise NotImplementedError + return "RELEASE SAVEPOINT %s" % self.quote_name(sid) def savepoint_rollback_sql(self, sid): """ Returns the SQL for rolling back the given savepoint. """ - raise NotImplementedError + return "ROLLBACK TO SAVEPOINT %s" % self.quote_name(sid) def set_time_zone_sql(self): """ diff --git a/django/db/backends/mysql/base.py b/django/db/backends/mysql/base.py index 39fd3695b7..f7d07cf4b7 100644 --- a/django/db/backends/mysql/base.py +++ b/django/db/backends/mysql/base.py @@ -355,15 +355,6 @@ class DatabaseOperations(BaseDatabaseOperations): items_sql = "(%s)" % ", ".join(["%s"] * len(fields)) return "VALUES " + ", ".join([items_sql] * num_values) - def savepoint_create_sql(self, sid): - return "SAVEPOINT %s" % sid - - def savepoint_commit_sql(self, sid): - return "RELEASE SAVEPOINT %s" % sid - - def savepoint_rollback_sql(self, sid): - return "ROLLBACK TO SAVEPOINT %s" % sid - class DatabaseWrapper(BaseDatabaseWrapper): vendor = 'mysql' operators = { diff --git a/django/db/backends/postgresql_psycopg2/operations.py b/django/db/backends/postgresql_psycopg2/operations.py index 56535e0865..a210f87ccd 100644 --- a/django/db/backends/postgresql_psycopg2/operations.py +++ b/django/db/backends/postgresql_psycopg2/operations.py @@ -175,15 +175,6 @@ class DatabaseOperations(BaseDatabaseOperations): style.SQL_TABLE(qn(f.m2m_db_table())))) return output - def savepoint_create_sql(self, sid): - return "SAVEPOINT %s" % sid - - def savepoint_commit_sql(self, sid): - return "RELEASE SAVEPOINT %s" % sid - - def savepoint_rollback_sql(self, sid): - return "ROLLBACK TO SAVEPOINT %s" % sid - def prep_for_iexact_query(self, x): return x From e264f67174622f66503e4748e2f71c72a4a5064b Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Fri, 1 Mar 2013 10:09:22 +0100 Subject: [PATCH 14/32] Refactored implementation of savepoints. Prepared for using savepoints within transactions in autocommit mode. --- django/db/backends/__init__.py | 43 +++++++++++++++++++--------------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/django/db/backends/__init__.py b/django/db/backends/__init__.py index 95a7bdc3b9..48190d3c62 100644 --- a/django/db/backends/__init__.py +++ b/django/db/backends/__init__.py @@ -176,54 +176,59 @@ class BaseDatabaseWrapper(object): ##### Backend-specific savepoint management methods ##### def _savepoint(self, sid): - if not self.features.uses_savepoints or self.autocommit: - return self.cursor().execute(self.ops.savepoint_create_sql(sid)) def _savepoint_rollback(self, sid): - if not self.features.uses_savepoints or self.autocommit: - return self.cursor().execute(self.ops.savepoint_rollback_sql(sid)) def _savepoint_commit(self, sid): - if not self.features.uses_savepoints or self.autocommit: - return self.cursor().execute(self.ops.savepoint_commit_sql(sid)) + def _savepoint_allowed(self): + # Savepoints cannot be created outside a transaction + return self.features.uses_savepoints and not self.autocommit + ##### Generic savepoint management methods ##### def savepoint(self): """ - Creates a savepoint (if supported and required by the backend) inside the - current transaction. Returns an identifier for the savepoint that will be - used for the subsequent rollback or commit. + Creates a savepoint inside the current transaction. Returns an + identifier for the savepoint that will be used for the subsequent + rollback or commit. Does nothing if savepoints are not supported. """ + if not self._savepoint_allowed(): + return + thread_ident = thread.get_ident() + tid = str(thread_ident).replace('-', '') self.savepoint_state += 1 - - tid = str(thread_ident).replace('-', '') sid = "s%s_x%d" % (tid, self.savepoint_state) + + self.validate_thread_sharing() self._savepoint(sid) + return sid def savepoint_rollback(self, sid): """ - Rolls back the most recent savepoint (if one exists). Does nothing if - savepoints are not supported. + Rolls back to a savepoint. Does nothing if savepoints are not supported. """ + if not self._savepoint_allowed(): + return + self.validate_thread_sharing() - if self.savepoint_state: - self._savepoint_rollback(sid) + self._savepoint_rollback(sid) def savepoint_commit(self, sid): """ - Commits the most recent savepoint (if one exists). Does nothing if - savepoints are not supported. + Releases a savepoint. Does nothing if savepoints are not supported. """ + if not self._savepoint_allowed(): + return + self.validate_thread_sharing() - if self.savepoint_state: - self._savepoint_commit(sid) + self._savepoint_commit(sid) def clean_savepoints(self): """ From 4b31a6a9e698a26e3e359e2ccf3da1505d114cf1 Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Mon, 4 Mar 2013 15:57:04 +0100 Subject: [PATCH 15/32] Added support for savepoints in SQLite. Technically speaking they aren't usable yet. --- django/db/backends/sqlite3/base.py | 10 +++++++++ docs/ref/databases.txt | 3 +-- docs/topics/db/transactions.txt | 35 ++++++++++++++++++++--------- tests/transactions_regress/tests.py | 4 ++++ 4 files changed, 40 insertions(+), 12 deletions(-) diff --git a/django/db/backends/sqlite3/base.py b/django/db/backends/sqlite3/base.py index 9a37dd17fe..f537860a53 100644 --- a/django/db/backends/sqlite3/base.py +++ b/django/db/backends/sqlite3/base.py @@ -100,6 +100,10 @@ class DatabaseFeatures(BaseDatabaseFeatures): has_bulk_insert = True can_combine_inserts_with_and_without_auto_increment_pk = False + @cached_property + def uses_savepoints(self): + return Database.sqlite_version_info >= (3, 6, 8) + @cached_property def supports_stddev(self): """Confirm support for STDDEV and related stats functions @@ -355,6 +359,12 @@ class DatabaseWrapper(BaseDatabaseWrapper): if self.settings_dict['NAME'] != ":memory:": BaseDatabaseWrapper.close(self) + def _savepoint_allowed(self): + # When 'isolation_level' is None, Django doesn't provide a way to + # create a transaction (yet) so savepoints can't be created. When it + # isn't, sqlite3 commits before each savepoint -- it's a bug. + return False + def _set_autocommit(self, autocommit): if autocommit: level = None diff --git a/docs/ref/databases.txt b/docs/ref/databases.txt index 4dafb3774f..78c1bb3dda 100644 --- a/docs/ref/databases.txt +++ b/docs/ref/databases.txt @@ -424,8 +424,7 @@ Savepoints Both the Django ORM and MySQL (when using the InnoDB :ref:`storage engine `) support database :ref:`savepoints -`, but this feature wasn't available in -Django until version 1.4 when such support was added. +`. If you use the MyISAM storage engine please be aware of the fact that you will receive database-generated errors if you try to use the :ref:`savepoint-related diff --git a/docs/topics/db/transactions.txt b/docs/topics/db/transactions.txt index 93c4a3b11d..e2c8e4e3f5 100644 --- a/docs/topics/db/transactions.txt +++ b/docs/topics/db/transactions.txt @@ -251,11 +251,11 @@ the transaction middleware, and only modify selected functions as needed. Savepoints ========== -A savepoint is a marker within a transaction that enables you to roll back part -of a transaction, rather than the full transaction. Savepoints are available -with the PostgreSQL 8, Oracle and MySQL (when using the InnoDB storage engine) -backends. Other backends provide the savepoint functions, but they're empty -operations -- they don't actually do anything. +A savepoint is a marker within a transaction that enables you to roll back +part of a transaction, rather than the full transaction. Savepoints are +available with the SQLite (≥ 3.6.8), PostgreSQL, Oracle and MySQL (when using +the InnoDB storage engine) backends. Other backends provide the savepoint +functions, but they're empty operations -- they don't actually do anything. Savepoints aren't especially useful if you are using the default ``autocommit`` behavior of Django. However, if you are using @@ -314,6 +314,21 @@ The following example demonstrates the use of savepoints:: Database-specific notes ======================= +Savepoints in SQLite +-------------------- + +While SQLite ≥ 3.6.8 supports savepoints, a flaw in the design of the +:mod:`sqlite3` makes them hardly usable. + +When autocommit is enabled, savepoints don't make sense. When it's disabled, +:mod:`sqlite3` commits implicitly before savepoint-related statement. (It +commits before any statement other than ``SELECT``, ``INSERT``, ``UPDATE``, +``DELETE`` and ``REPLACE``.) + +As a consequence, savepoints are only usable if you start a transaction +manually while in autocommit mode, and Django doesn't provide an API to +achieve that. + Transactions in MySQL --------------------- @@ -363,11 +378,11 @@ itself. Savepoint rollback ~~~~~~~~~~~~~~~~~~ -If you are using PostgreSQL 8 or later, you can use :ref:`savepoints -` to control the extent of a rollback. -Before performing a database operation that could fail, you can set or update -the savepoint; that way, if the operation fails, you can roll back the single -offending operation, rather than the entire transaction. For example:: +You can use :ref:`savepoints ` to control +the extent of a rollback. Before performing a database operation that could +fail, you can set or update the savepoint; that way, if the operation fails, +you can roll back the single offending operation, rather than the entire +transaction. For example:: a.save() # Succeeds, and never undone by savepoint rollback try: diff --git a/tests/transactions_regress/tests.py b/tests/transactions_regress/tests.py index 3d571fba2f..e86db4d0aa 100644 --- a/tests/transactions_regress/tests.py +++ b/tests/transactions_regress/tests.py @@ -309,6 +309,8 @@ class TestManyToManyAddTransaction(TransactionTestCase): class SavepointTest(TransactionTestCase): + @skipIf(connection.vendor == 'sqlite', + "SQLite doesn't support savepoints in managed mode") @skipUnlessDBFeature('uses_savepoints') def test_savepoint_commit(self): @commit_manually @@ -324,6 +326,8 @@ class SavepointTest(TransactionTestCase): work() + @skipIf(connection.vendor == 'sqlite', + "SQLite doesn't support savepoints in managed mode") @skipIf(connection.vendor == 'mysql' and connection.features._mysql_storage_engine == 'MyISAM', "MyISAM MySQL storage engine doesn't support savepoints") From d7bc4fbc94df6c231d71dffa45cf337ff13512ee Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Mon, 4 Mar 2013 22:17:35 +0100 Subject: [PATCH 16/32] Implemented an 'atomic' decorator and context manager. Currently it only works in autocommit mode. Based on @xact by Christophe Pettus. --- AUTHORS | 1 + django/db/backends/__init__.py | 23 ++++- django/db/backends/sqlite3/base.py | 19 +++- django/db/transaction.py | 157 +++++++++++++++++++++++++++-- docs/topics/db/transactions.txt | 97 ++++++++++++++++-- tests/transactions/models.py | 2 +- tests/transactions/tests.py | 154 +++++++++++++++++++++++++++- 7 files changed, 430 insertions(+), 23 deletions(-) diff --git a/AUTHORS b/AUTHORS index 35a316d4c2..3c1cd81639 100644 --- a/AUTHORS +++ b/AUTHORS @@ -434,6 +434,7 @@ answer newbie questions, and generally made Django that much better: Andreas Pelme permonik@mesias.brnonet.cz peter@mymart.com + Christophe Pettus pgross@thoughtworks.com phaedo phil@produxion.net diff --git a/django/db/backends/__init__.py b/django/db/backends/__init__.py index 48190d3c62..818850bf43 100644 --- a/django/db/backends/__init__.py +++ b/django/db/backends/__init__.py @@ -50,6 +50,12 @@ 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' + self.in_atomic_block = 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 @@ -148,7 +154,7 @@ class BaseDatabaseWrapper(object): def commit(self): """ - Does the commit itself and resets the dirty flag. + Commits a transaction and resets the dirty flag. """ self.validate_thread_sharing() self._commit() @@ -156,7 +162,7 @@ class BaseDatabaseWrapper(object): def rollback(self): """ - Does the rollback itself and resets the dirty flag. + Rolls back a transaction and resets the dirty flag. """ self.validate_thread_sharing() self._rollback() @@ -447,6 +453,12 @@ class BaseDatabaseWrapper(object): if must_close: self.close() + def _start_transaction_under_autocommit(self): + """ + Only required when autocommits_when_autocommit_is_off = True. + """ + raise NotImplementedError + class BaseDatabaseFeatures(object): allows_group_by_pk = False @@ -549,6 +561,10 @@ class BaseDatabaseFeatures(object): # Support for the DISTINCT ON clause can_distinct_on_fields = False + # Does the backend decide to commit before SAVEPOINT statements + # when autocommit is disabled? http://bugs.python.org/issue8145#msg109965 + autocommits_when_autocommit_is_off = False + def __init__(self, connection): self.connection = connection @@ -931,6 +947,9 @@ class BaseDatabaseOperations(object): return "BEGIN;" def end_transaction_sql(self, success=True): + """ + Returns the SQL statement required to end a transaction. + """ if not success: return "ROLLBACK;" return "COMMIT;" diff --git a/django/db/backends/sqlite3/base.py b/django/db/backends/sqlite3/base.py index f537860a53..f70c3872a8 100644 --- a/django/db/backends/sqlite3/base.py +++ b/django/db/backends/sqlite3/base.py @@ -99,6 +99,7 @@ class DatabaseFeatures(BaseDatabaseFeatures): supports_mixed_date_datetime_comparisons = False has_bulk_insert = True can_combine_inserts_with_and_without_auto_increment_pk = False + autocommits_when_autocommit_is_off = True @cached_property def uses_savepoints(self): @@ -360,10 +361,12 @@ class DatabaseWrapper(BaseDatabaseWrapper): BaseDatabaseWrapper.close(self) def _savepoint_allowed(self): - # When 'isolation_level' is None, Django doesn't provide a way to - # create a transaction (yet) so savepoints can't be created. When it - # isn't, sqlite3 commits before each savepoint -- it's a bug. - return False + # When 'isolation_level' is not None, sqlite3 commits before each + # savepoint; it's a bug. When it is None, savepoints don't make sense + # because autocommit is enabled. The only exception is inside atomic + # blocks. To work around that bug, on SQLite, atomic starts a + # transaction explicitly rather than simply disable autocommit. + return self.in_atomic_block def _set_autocommit(self, autocommit): if autocommit: @@ -413,6 +416,14 @@ class DatabaseWrapper(BaseDatabaseWrapper): def is_usable(self): return True + def _start_transaction_under_autocommit(self): + """ + Start a transaction explicitly in autocommit mode. + + Staying in autocommit mode works around a bug of sqlite3 that breaks + savepoints when autocommit is disabled. + """ + self.cursor().execute("BEGIN") FORMAT_QMARK_REGEX = re.compile(r'(?`. Tying transactions to HTTP requests -=================================== +----------------------------------- The recommended way to handle transactions in Web requests is to tie them to the request and response phases via Django's ``TransactionMiddleware``. @@ -63,6 +66,85 @@ connection internally. multiple databases and want transaction control over databases other than "default", you will need to write your own transaction middleware. +Controlling transactions explicitly +----------------------------------- + +.. versionadded:: 1.6 + +Django provides a single API to control database transactions. + +.. function:: atomic(using=None) + + This function creates an atomic block for writes to the database. + (Atomicity is the defining property of database transactions.) + + When the block completes successfully, the changes are committed to the + database. When it raises an exception, the changes are rolled back. + + ``atomic`` can be nested. In this case, when an inner block completes + successfully, its effects can still be rolled back if an exception is + raised in the outer block at a later point. + + ``atomic`` takes a ``using`` argument which should be the name of a + database. If this argument isn't provided, Django uses the ``"default"`` + database. + + ``atomic`` is usable both as a decorator:: + + from django.db import transaction + + @transaction.atomic + def viewfunc(request): + # This code executes inside a transaction. + do_stuff() + + and as a context manager:: + + from django.db import transaction + + def viewfunc(request): + # This code executes in autocommit mode (Django's default). + do_stuff() + + with transaction.atomic(): + # This code executes inside a transaction. + do_more_stuff() + + Wrapping ``atomic`` in a try/except block allows for natural handling of + integrity errors:: + + from django.db import IntegrityError, transaction + + @transaction.atomic + def viewfunc(request): + do_stuff() + + try: + with transaction.atomic(): + do_stuff_that_could_fail() + except IntegrityError: + handle_exception() + + do_more_stuff() + + In this example, even if ``do_stuff_that_could_fail()`` causes a database + error by breaking an integrity constraint, you can execute queries in + ``do_more_stuff()``, and the changes from ``do_stuff()`` are still there. + + In order to guarantee atomicity, ``atomic`` disables some APIs. Attempting + 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; + - creates a savepoint when entering an inner ``atomic`` block; + - releases or rolls back to the savepoint when exiting an inner block; + - commits or rolls back the transaction when exiting the outermost block. + .. _transaction-management-functions: Controlling transaction management in views @@ -325,9 +407,8 @@ 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 if you start a transaction -manually while in autocommit mode, and Django doesn't provide an API to -achieve that. +As a consequence, savepoints are only usable inside a transaction ie. inside +an :func:`atomic` block. Transactions in MySQL --------------------- diff --git a/tests/transactions/models.py b/tests/transactions/models.py index 0f8d6b16ec..6c2bcfd23f 100644 --- a/tests/transactions/models.py +++ b/tests/transactions/models.py @@ -22,4 +22,4 @@ class Reporter(models.Model): ordering = ('first_name', 'last_name') def __str__(self): - return "%s %s" % (self.first_name, self.last_name) + return ("%s %s" % (self.first_name, self.last_name)).strip() diff --git a/tests/transactions/tests.py b/tests/transactions/tests.py index a1edf53fcb..14252dd6dc 100644 --- a/tests/transactions/tests.py +++ b/tests/transactions/tests.py @@ -1,11 +1,163 @@ from __future__ import absolute_import +import sys + from django.db import connection, transaction, IntegrityError -from django.test import TransactionTestCase, skipUnlessDBFeature +from django.test import TestCase, TransactionTestCase, skipUnlessDBFeature +from django.utils import six +from django.utils.unittest import skipUnless from .models import Reporter +@skipUnless(connection.features.uses_savepoints, + "'atomic' requires transactions and savepoints.") +class AtomicTests(TransactionTestCase): + """ + Tests for the atomic decorator and context manager. + + The tests make assertions on internal attributes because there isn't a + robust way to ask the database for its current transaction state. + + Since the decorator syntax is converted into a context manager (see the + implementation), there are only a few basic tests with the decorator + syntax and the bulk of the tests use the context manager syntax. + """ + + def test_decorator_syntax_commit(self): + @transaction.atomic + def make_reporter(): + Reporter.objects.create(first_name="Tintin") + make_reporter() + self.assertQuerysetEqual(Reporter.objects.all(), ['']) + + def test_decorator_syntax_rollback(self): + @transaction.atomic + def make_reporter(): + Reporter.objects.create(first_name="Haddock") + raise Exception("Oops, that's his last name") + with six.assertRaisesRegex(self, Exception, "Oops"): + make_reporter() + self.assertQuerysetEqual(Reporter.objects.all(), []) + + def test_alternate_decorator_syntax_commit(self): + @transaction.atomic() + def make_reporter(): + Reporter.objects.create(first_name="Tintin") + make_reporter() + self.assertQuerysetEqual(Reporter.objects.all(), ['']) + + def test_alternate_decorator_syntax_rollback(self): + @transaction.atomic() + def make_reporter(): + Reporter.objects.create(first_name="Haddock") + raise Exception("Oops, that's his last name") + with six.assertRaisesRegex(self, Exception, "Oops"): + make_reporter() + self.assertQuerysetEqual(Reporter.objects.all(), []) + + def test_commit(self): + with transaction.atomic(): + Reporter.objects.create(first_name="Tintin") + self.assertQuerysetEqual(Reporter.objects.all(), ['']) + + def test_rollback(self): + with six.assertRaisesRegex(self, Exception, "Oops"): + with transaction.atomic(): + Reporter.objects.create(first_name="Haddock") + raise Exception("Oops, that's his last name") + self.assertQuerysetEqual(Reporter.objects.all(), []) + + def test_nested_commit_commit(self): + with transaction.atomic(): + Reporter.objects.create(first_name="Tintin") + with transaction.atomic(): + Reporter.objects.create(first_name="Archibald", last_name="Haddock") + self.assertQuerysetEqual(Reporter.objects.all(), + ['', '']) + + def test_nested_commit_rollback(self): + with transaction.atomic(): + Reporter.objects.create(first_name="Tintin") + with six.assertRaisesRegex(self, Exception, "Oops"): + with transaction.atomic(): + Reporter.objects.create(first_name="Haddock") + raise Exception("Oops, that's his last name") + self.assertQuerysetEqual(Reporter.objects.all(), ['']) + + def test_nested_rollback_commit(self): + with six.assertRaisesRegex(self, Exception, "Oops"): + with transaction.atomic(): + Reporter.objects.create(last_name="Tintin") + with transaction.atomic(): + Reporter.objects.create(last_name="Haddock") + raise Exception("Oops, that's his first name") + self.assertQuerysetEqual(Reporter.objects.all(), []) + + def test_nested_rollback_rollback(self): + with six.assertRaisesRegex(self, Exception, "Oops"): + with transaction.atomic(): + Reporter.objects.create(last_name="Tintin") + with six.assertRaisesRegex(self, Exception, "Oops"): + with transaction.atomic(): + Reporter.objects.create(first_name="Haddock") + raise Exception("Oops, that's his last name") + raise Exception("Oops, that's his first name") + self.assertQuerysetEqual(Reporter.objects.all(), []) + + def test_reuse_commit_commit(self): + atomic = transaction.atomic() + with atomic: + Reporter.objects.create(first_name="Tintin") + with atomic: + Reporter.objects.create(first_name="Archibald", last_name="Haddock") + self.assertQuerysetEqual(Reporter.objects.all(), + ['', '']) + + def test_reuse_commit_rollback(self): + atomic = transaction.atomic() + with atomic: + Reporter.objects.create(first_name="Tintin") + with six.assertRaisesRegex(self, Exception, "Oops"): + with atomic: + Reporter.objects.create(first_name="Haddock") + raise Exception("Oops, that's his last name") + self.assertQuerysetEqual(Reporter.objects.all(), ['']) + + def test_reuse_rollback_commit(self): + atomic = transaction.atomic() + with six.assertRaisesRegex(self, Exception, "Oops"): + with atomic: + Reporter.objects.create(last_name="Tintin") + with atomic: + Reporter.objects.create(last_name="Haddock") + raise Exception("Oops, that's his first name") + self.assertQuerysetEqual(Reporter.objects.all(), []) + + def test_reuse_rollback_rollback(self): + atomic = transaction.atomic() + with six.assertRaisesRegex(self, Exception, "Oops"): + with atomic: + Reporter.objects.create(last_name="Tintin") + with six.assertRaisesRegex(self, Exception, "Oops"): + with atomic: + Reporter.objects.create(first_name="Haddock") + raise Exception("Oops, that's his last name") + raise Exception("Oops, that's his first name") + self.assertQuerysetEqual(Reporter.objects.all(), []) + + +class AtomicInsideTransactionTests(AtomicTests): + """All basic tests for atomic should also pass within an existing transaction.""" + + def setUp(self): + self.atomic = transaction.atomic() + self.atomic.__enter__() + + def tearDown(self): + self.atomic.__exit__(*sys.exc_info()) + + class TransactionTests(TransactionTestCase): def create_a_reporter_then_fail(self, first, last): a = Reporter(first_name=first, last_name=last) From 7c46c8d5f27fe305507359588ca0635b6d87c59a Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Mon, 4 Mar 2013 23:26:31 +0100 Subject: [PATCH 17/32] Added some assertions to enforce the atomicity of atomic. --- django/db/__init__.py | 1 + django/db/backends/__init__.py | 15 + django/db/transaction.py | 18 +- docs/internals/deprecation.txt | 4 + docs/releases/1.3-alpha-1.txt | 6 +- docs/releases/1.3.txt | 6 +- docs/releases/1.6.txt | 17 +- docs/topics/db/transactions.txt | 459 ++++++++++++-------------- tests/backends/tests.py | 15 +- tests/fixtures_model_package/tests.py | 11 +- tests/fixtures_regress/tests.py | 7 +- tests/middleware/tests.py | 6 +- tests/transactions/tests.py | 71 +++- tests/transactions_regress/tests.py | 12 +- 14 files changed, 369 insertions(+), 279 deletions(-) diff --git a/django/db/__init__.py b/django/db/__init__.py index 13ba68ba7e..08c901ab7b 100644 --- a/django/db/__init__.py +++ b/django/db/__init__.py @@ -70,6 +70,7 @@ signals.request_started.connect(reset_queries) # their lifetime. NB: abort() doesn't do anything outside of a transaction. def close_old_connections(**kwargs): for conn in connections.all(): + # Remove this when the legacy transaction management goes away. try: conn.abort() except DatabaseError: diff --git a/django/db/backends/__init__.py b/django/db/backends/__init__.py index 818850bf43..346d10198d 100644 --- a/django/db/backends/__init__.py +++ b/django/db/backends/__init__.py @@ -157,6 +157,7 @@ class BaseDatabaseWrapper(object): Commits a transaction and resets the dirty flag. """ self.validate_thread_sharing() + self.validate_no_atomic_block() self._commit() self.set_clean() @@ -165,6 +166,7 @@ class BaseDatabaseWrapper(object): Rolls back a transaction and resets the dirty flag. """ self.validate_thread_sharing() + self.validate_no_atomic_block() self._rollback() self.set_clean() @@ -265,6 +267,8 @@ class BaseDatabaseWrapper(object): If you switch off transaction management and there is a pending commit/rollback, the data will be commited, unless "forced" is True. """ + self.validate_no_atomic_block() + self.transaction_state.append(managed) if not managed and self.is_dirty() and not forced: @@ -280,6 +284,8 @@ class BaseDatabaseWrapper(object): over to the surrounding block, as a commit will commit all changes, even those from outside. (Commits are on connection level.) """ + self.validate_no_atomic_block() + if self.transaction_state: del self.transaction_state[-1] else: @@ -305,10 +311,19 @@ class BaseDatabaseWrapper(object): """ Enable or disable autocommit. """ + self.validate_no_atomic_block() self.ensure_connection() self._set_autocommit(autocommit) self.autocommit = autocommit + def validate_no_atomic_block(self): + """ + Raise an error if an atomic block is active. + """ + if self.in_atomic_block: + raise TransactionManagementError( + "This is forbidden when an 'atomic' block is active.") + def abort(self): """ Roll back any ongoing transaction and clean the transaction state diff --git a/django/db/transaction.py b/django/db/transaction.py index 8126c18a70..eb9d85e274 100644 --- a/django/db/transaction.py +++ b/django/db/transaction.py @@ -367,6 +367,9 @@ def autocommit(using=None): this decorator is useful if you globally activated transaction management in your settings file and want the default behavior in some view functions. """ + warnings.warn("autocommit is deprecated in favor of set_autocommit.", + PendingDeprecationWarning, stacklevel=2) + def entering(using): enter_transaction_management(managed=False, using=using) @@ -382,6 +385,9 @@ def commit_on_success(using=None): a rollback is made. This is one of the most common ways to do transaction control in Web apps. """ + warnings.warn("commit_on_success is deprecated in favor of atomic.", + PendingDeprecationWarning, stacklevel=2) + def entering(using): enter_transaction_management(using=using) @@ -409,6 +415,9 @@ def commit_manually(using=None): own -- it's up to the user to call the commit and rollback functions themselves. """ + warnings.warn("commit_manually is deprecated in favor of set_autocommit.", + PendingDeprecationWarning, stacklevel=2) + def entering(using): enter_transaction_management(using=using) @@ -420,10 +429,15 @@ def commit_manually(using=None): def commit_on_success_unless_managed(using=None): """ Transitory API to preserve backwards-compatibility while refactoring. + + Once the legacy transaction management is fully deprecated, this should + simply be replaced by atomic. 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. """ connection = get_connection(using) - if connection.autocommit and not connection.in_atomic_block: - return commit_on_success(using) + if connection.autocommit or connection.in_atomic_block: + return atomic(using) else: def entering(using): pass diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index 1c8618713a..6c13af7ae4 100644 --- a/docs/internals/deprecation.txt +++ b/docs/internals/deprecation.txt @@ -329,6 +329,10 @@ these changes. 1.8 --- +* The decorators and context managers ``django.db.transaction.autocommit``, + ``commit_on_success`` and ``commit_manually`` will be removed. See + :ref:`transactions-upgrading-from-1.5`. + * The :ttag:`cycle` and :ttag:`firstof` template tags will auto-escape their arguments. In 1.6 and 1.7, this behavior is provided by the version of these tags in the ``future`` template tag library. diff --git a/docs/releases/1.3-alpha-1.txt b/docs/releases/1.3-alpha-1.txt index ba8a4fc557..53d38a006b 100644 --- a/docs/releases/1.3-alpha-1.txt +++ b/docs/releases/1.3-alpha-1.txt @@ -105,16 +105,14 @@ you just won't get any of the nice new unittest2 features. Transaction context managers ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -Users of Python 2.5 and above may now use :ref:`transaction management functions -` as `context managers`_. For example:: +Users of Python 2.5 and above may now use transaction management functions as +`context managers`_. For example:: with transaction.autocommit(): # ... .. _context managers: http://docs.python.org/glossary.html#term-context-manager -For more information, see :ref:`transaction-management-functions`. - Configurable delete-cascade ~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/docs/releases/1.3.txt b/docs/releases/1.3.txt index 4c8dd2f81f..582bceffca 100644 --- a/docs/releases/1.3.txt +++ b/docs/releases/1.3.txt @@ -148,16 +148,14 @@ you just won't get any of the nice new unittest2 features. Transaction context managers ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -Users of Python 2.5 and above may now use :ref:`transaction management functions -` as `context managers`_. For example:: +Users of Python 2.5 and above may now use transaction management functions as +`context managers`_. For example:: with transaction.autocommit(): # ... .. _context managers: http://docs.python.org/glossary.html#term-context-manager -For more information, see :ref:`transaction-management-functions`. - Configurable delete-cascade ~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/docs/releases/1.6.txt b/docs/releases/1.6.txt index c55ef0ef38..cc3bf94ef5 100644 --- a/docs/releases/1.6.txt +++ b/docs/releases/1.6.txt @@ -39,7 +39,7 @@ should improve performance. The existing APIs were deprecated, and new APIs were introduced, as described in :doc:`/topics/db/transactions`. Please review carefully the list of :ref:`known backwards-incompatibilities -` to determine if you need to make changes in +` to determine if you need to make changes in your code. Persistent database connections @@ -163,7 +163,7 @@ Backwards incompatible changes in 1.6 * Database-level autocommit is enabled by default in Django 1.6. While this doesn't change the general spirit of Django's transaction management, there are a few known backwards-incompatibities, described in the :ref:`transaction - management docs `. You should review your code + management docs `. You should review your code to determine if you're affected. * In previous versions, database-level autocommit was only an option for @@ -256,6 +256,19 @@ Backwards incompatible changes in 1.6 Features deprecated in 1.6 ========================== +Transaction management APIs +~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Transaction management was completely overhauled in Django 1.6, and the +current APIs are deprecated: + +- :func:`django.db.transaction.autocommit` +- :func:`django.db.transaction.commit_on_success` +- :func:`django.db.transaction.commit_manually` + +The reasons for this change and the upgrade path are described in the +:ref:`transactions documentation `. + Changes to :ttag:`cycle` and :ttag:`firstof` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/docs/topics/db/transactions.txt b/docs/topics/db/transactions.txt index 2a4cd306c6..91b2cf41b3 100644 --- a/docs/topics/db/transactions.txt +++ b/docs/topics/db/transactions.txt @@ -24,7 +24,7 @@ immediately committed to the database. :ref:`See below for details .. versionchanged:: 1.6 Previous version of Django featured :ref:`a more complicated default - behavior `. + behavior `. Tying transactions to HTTP requests ----------------------------------- @@ -89,7 +89,7 @@ Django provides a single API to control database transactions. database. If this argument isn't provided, Django uses the ``"default"`` database. - ``atomic`` is usable both as a decorator:: + ``atomic`` is usable both as a `decorator`_:: from django.db import transaction @@ -98,7 +98,7 @@ Django provides a single API to control database transactions. # This code executes inside a transaction. do_stuff() - and as a context manager:: + and as a `context manager`_:: from django.db import transaction @@ -110,6 +110,9 @@ Django provides a single API to control database transactions. # This code executes inside a transaction. do_more_stuff() + .. _decorator: http://docs.python.org/glossary.html#term-decorator + .. _context manager: http://docs.python.org/glossary.html#term-context-manager + Wrapping ``atomic`` in a try/except block allows for natural handling of integrity errors:: @@ -145,189 +148,6 @@ Django provides a single API to control database transactions. - releases or rolls back to the savepoint when exiting an inner block; - commits or rolls back the transaction when exiting the outermost block. -.. _transaction-management-functions: - -Controlling transaction management in views -=========================================== - -For most people, implicit request-based transactions work wonderfully. However, -if you need more fine-grained control over how transactions are managed, you can -use a set of functions in ``django.db.transaction`` to control transactions on a -per-function or per-code-block basis. - -These functions, described in detail below, can be used in two different ways: - -* As a decorator_ on a particular function. For example:: - - from django.db import transaction - - @transaction.commit_on_success - def viewfunc(request): - # ... - # this code executes inside a transaction - # ... - -* As a `context manager`_ around a particular block of code:: - - from django.db import transaction - - def viewfunc(request): - # ... - # this code executes using default transaction management - # ... - - with transaction.commit_on_success(): - # ... - # this code executes inside a transaction - # ... - -Both techniques work with all supported version of Python. - -.. _decorator: http://docs.python.org/glossary.html#term-decorator -.. _context manager: http://docs.python.org/glossary.html#term-context-manager - -For maximum compatibility, all of the examples below show transactions using the -decorator syntax, but all of the follow functions may be used as context -managers, too. - -.. note:: - - Although the examples below use view functions as examples, these - decorators and context managers can be used anywhere in your code - that you need to deal with transactions. - -.. _topics-db-transactions-autocommit: - -.. function:: autocommit - - Use the ``autocommit`` decorator to switch a view function to Django's - default commit behavior. - - Example:: - - from django.db import transaction - - @transaction.autocommit - def viewfunc(request): - .... - - @transaction.autocommit(using="my_other_database") - def viewfunc2(request): - .... - - Within ``viewfunc()``, transactions will be committed as soon as you call - ``model.save()``, ``model.delete()``, or any other function that writes to - the database. ``viewfunc2()`` will have this same behavior, but for the - ``"my_other_database"`` connection. - -.. function:: commit_on_success - - Use the ``commit_on_success`` decorator to use a single transaction for all - the work done in a function:: - - from django.db import transaction - - @transaction.commit_on_success - def viewfunc(request): - .... - - @transaction.commit_on_success(using="my_other_database") - def viewfunc2(request): - .... - - If the function returns successfully, then Django will commit all work done - within the function at that point. If the function raises an exception, - though, Django will roll back the transaction. - -.. function:: commit_manually - - Use the ``commit_manually`` decorator if you need full control over - transactions. It tells Django you'll be managing the transaction on your - own. - - Whether you are writing or simply reading from the database, you must - ``commit()`` or ``rollback()`` explicitly or Django will raise a - :exc:`TransactionManagementError` exception. This is required when reading - from the database because ``SELECT`` statements may call functions which - modify tables, and thus it is impossible to know if any data has been - modified. - - Manual transaction management looks like this:: - - from django.db import transaction - - @transaction.commit_manually - def viewfunc(request): - ... - # You can commit/rollback however and whenever you want - transaction.commit() - ... - - # But you've got to remember to do it yourself! - try: - ... - except: - transaction.rollback() - else: - transaction.commit() - - @transaction.commit_manually(using="my_other_database") - def viewfunc2(request): - .... - -.. _topics-db-transactions-requirements: - -Requirements for transaction handling -===================================== - -Django requires that every transaction that is opened is closed before the -completion of a request. - -If you are using :func:`autocommit` (the default commit mode) or -:func:`commit_on_success`, this will be done for you automatically. However, -if you are manually managing transactions (using the :func:`commit_manually` -decorator), you must ensure that the transaction is either committed or rolled -back before a request is completed. - -This applies to all database operations, not just write operations. Even -if your transaction only reads from the database, the transaction must -be committed or rolled back before you complete a request. - -.. _managing-autocommit: - -Managing autocommit -=================== - -.. versionadded:: 1.6 - -Django provides a straightforward API to manage the autocommit state of each -database connection, if you need to. - -.. function:: get_autocommit(using=None) - -.. function:: set_autocommit(using=None, autocommit=True) - -These functions take a ``using`` argument which should be the name of a -database. If it isn't provided, Django uses the ``"default"`` database. - -.. _deactivate-transaction-management: - -How to globally deactivate transaction management -================================================= - -Control freaks can totally disable all transaction management by setting -:setting:`TRANSACTIONS_MANAGED` to ``True`` in the Django settings file. If -you do this, Django won't enable autocommit. You'll get the regular behavior -of the underlying database library. - -This requires you to commit explicitly every transaction, even those started -by Django or by third-party libraries. Thus, this is best used in situations -where you want to run your own transaction-controlling middleware or do -something really strange. - -In almost all situations, you'll be better off using the default behavior, or -the transaction middleware, and only modify selected functions as needed. - .. _topics-db-transactions-savepoints: Savepoints @@ -339,13 +159,19 @@ available with the SQLite (≥ 3.6.8), PostgreSQL, Oracle and MySQL (when using the InnoDB storage engine) backends. Other backends provide the savepoint functions, but they're empty operations -- they don't actually do anything. -Savepoints aren't especially useful if you are using the default -``autocommit`` behavior of Django. However, if you are using -``commit_on_success`` or ``commit_manually``, each open transaction will build -up a series of database operations, awaiting a commit or rollback. If you -issue a rollback, the entire transaction is rolled back. Savepoints provide -the ability to perform a fine-grained rollback, rather than the full rollback -that would be performed by ``transaction.rollback()``. +Savepoints aren't especially useful if you are using autocommit, the default +behavior of Django. However, once you open a transaction with :func:`atomic`, +you build up a series of database operations awaiting a commit or rollback. If +you issue a rollback, the entire transaction is rolled back. Savepoints +provide the ability to perform a fine-grained rollback, rather than the full +rollback that would be performed by ``transaction.rollback()``. + +.. versionchanged:: 1.6 + +When the :func:`atomic` decorator is nested, it creates a savepoint to allow +partial commit or rollback. You're strongly encouraged to use :func:`atomic` +rather than the functions described below, but they're still part of the +public API, and there's no plan to deprecate them. Each of these functions takes a ``using`` argument which should be the name of a database for which the behavior applies. If no ``using`` argument is @@ -374,15 +200,17 @@ The following example demonstrates the use of savepoints:: from django.db import transaction - @transaction.commit_manually + # open a transaction + @transaction.atomic def viewfunc(request): a.save() - # open transaction now contains a.save() + # transaction now contains a.save() + sid = transaction.savepoint() b.save() - # open transaction now contains a.save() and b.save() + # transaction now contains a.save() and b.save() if want_to_keep_b: transaction.savepoint_commit(sid) @@ -391,7 +219,82 @@ The following example demonstrates the use of savepoints:: transaction.savepoint_rollback(sid) # open transaction now contains only a.save() - transaction.commit() +Autocommit +========== + +.. _autocommit-details: + +Why Django uses autocommit +-------------------------- + +In the SQL standards, each SQL query starts a transaction, unless one is +already in progress. Such transactions must then be committed or rolled back. + +This isn't always convenient for application developers. To alleviate this +problem, most databases provide an autocommit mode. When autocommit is turned +on, each SQL query is wrapped in its own transaction. In other words, the +transaction is not only automatically started, but also automatically +committed. + +:pep:`249`, the Python Database API Specification v2.0, requires autocommit to +be initially turned off. Django overrides this default and turns autocommit +on. + +To avoid this, you can :ref:`deactivate the transaction management +`, but it isn't recommended. + +.. versionchanged:: 1.6 + Before Django 1.6, autocommit was turned off, and it was emulated by + forcing a commit after write operations in the ORM. + +.. warning:: + + If you're using the database API directly — for instance, you're running + SQL queries with ``cursor.execute()`` — be aware that autocommit is on, + and consider wrapping your operations in a transaction, with + :func:`atomic`, to ensure consistency. + +.. _managing-autocommit: + +Managing autocommit +------------------- + +.. versionadded:: 1.6 + +Django provides a straightforward API to manage the autocommit state of each +database connection, if you need to. + +.. function:: get_autocommit(using=None) + +.. function:: set_autocommit(using=None, autocommit=True) + +These functions take a ``using`` argument which should be the name of a +database. If it isn't provided, Django uses the ``"default"`` database. + +Autocommit is initially turned on. If you turn it off, it's your +responsibility to restore it. + +: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. + +.. _deactivate-transaction-management: + +Deactivating transaction management +----------------------------------- + +Control freaks can totally disable all transaction management by setting +:setting:`TRANSACTIONS_MANAGED` to ``True`` in the Django settings file. If +you do this, Django won't enable autocommit. You'll get the regular behavior +of the underlying database library. + +This requires you to commit explicitly every transaction, even those started +by Django or by third-party libraries. Thus, this is best used in situations +where you want to run your own transaction-controlling middleware or do +something really strange. + +In almost all situations, you'll be better off using the default behavior, or +the transaction middleware, and only modify selected functions as needed. Database-specific notes ======================= @@ -477,45 +380,57 @@ transaction. For example:: In this example, ``a.save()`` will not be undone in the case where ``b.save()`` raises an exception. -Under the hood -============== +.. _transactions-upgrading-from-1.5: -.. _autocommit-details: +Changes from Django 1.5 and earlier +=================================== -Details on autocommit ---------------------- +The features described below were deprecated in Django 1.6 and will be removed +in Django 1.8. They're documented in order to ease the migration to the new +transaction management APIs. -In the SQL standards, each SQL query starts a transaction, unless one is -already in progress. Such transactions must then be committed or rolled back. +Legacy APIs +----------- -This isn't always convenient for application developers. To alleviate this -problem, most databases provide an autocommit mode. When autocommit is turned -on, each SQL query is wrapped in its own transaction. In other words, the -transaction is not only automatically started, but also automatically -committed. +The following functions, defined in ``django.db.transaction``, provided a way +to control transactions on a per-function or per-code-block basis. They could +be used as decorators or as context managers, and they accepted a ``using`` +argument, exactly like :func:`atomic`. -:pep:`249`, the Python Database API Specification v2.0, requires autocommit to -be initially turned off. Django overrides this default and turns autocommit -on. +.. function:: autocommit -To avoid this, you can :ref:`deactivate the transaction management -`, but it isn't recommended. + Enable Django's default autocommit behavior. -.. versionchanged:: 1.6 - Before Django 1.6, autocommit was turned off, and it was emulated by - forcing a commit after write operations in the ORM. + Transactions will be committed as soon as you call ``model.save()``, + ``model.delete()``, or any other function that writes to the database. -.. warning:: +.. function:: commit_on_success - If you're using the database API directly — for instance, you're running - SQL queries with ``cursor.execute()`` — be aware that autocommit is on, - and consider wrapping your operations in a transaction to ensure - consistency. + Use a single transaction for all the work done in a function. + + If the function returns successfully, then Django will commit all work done + within the function at that point. If the function raises an exception, + though, Django will roll back the transaction. + +.. function:: commit_manually + + Tells Django you'll be managing the transaction on your own. + + Whether you are writing or simply reading from the database, you must + ``commit()`` or ``rollback()`` explicitly or Django will raise a + :exc:`TransactionManagementError` exception. This is required when reading + from the database because ``SELECT`` statements may call functions which + modify tables, and thus it is impossible to know if any data has been + modified. .. _transaction-states: -Transaction management states ------------------------------ +Transaction states +------------------ + +The three functions described above relied on a concept called "transaction +states". This mechanisme was deprecated in Django 1.6, but it's still +available until Django 1.8.. At any time, each database connection is in one of these two states: @@ -529,35 +444,80 @@ Django starts in auto mode. ``TransactionMiddleware``, Internally, Django keeps a stack of states. Activations and deactivations must be balanced. -For example, at the beginning of each HTTP request, ``TransactionMiddleware`` -switches to managed mode; at the end of the request, it commits or rollbacks, +For example, ``commit_on_success`` switches to managed mode when entering the +block of code it controls; when exiting the block, it commits or rollbacks, and switches back to auto mode. -.. admonition:: Nesting decorators / context managers +So :func:`commit_on_success` really has two effects: it changes the +transaction state and it defines an transaction block. Nesting will give the +expected results in terms of transaction state, but not in terms of +transaction semantics. Most often, the inner block will commit, breaking the +atomicity of the outer block. - :func:`commit_on_success` has two effects: it changes the transaction - state, and defines an atomic transaction block. +:func:`autocommit` and :func:`commit_manually` have similar limitations. - Nesting with :func:`autocommit` and :func:`commit_manually` will give the - expected results in terms of transaction state, but not in terms of - transaction semantics. Most often, the inner block will commit, breaking - the atomicity of the outer block. +API changes +----------- -Django currently doesn't provide any APIs to create transactions in auto mode. +Managing transactions +~~~~~~~~~~~~~~~~~~~~~ -.. _transactions-changes-from-1.5: +Starting with Django 1.6, :func:`atomic` is the only supported API for +defining a transaction. Unlike the deprecated APIs, it's nestable and always +guarantees atomicity. -Changes from Django 1.5 and earlier -=================================== +In most cases, it will be a drop-in replacement for :func:`commit_on_success`. + +During the deprecation period, it's possible to use :func:`atomic` within +:func:`autocommit`, :func:`commit_on_success` or :func:`commit_manually`. +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 +~~~~~~~~~~~~~~~~~~~ + +Django 1.6 introduces an explicit :ref:`API for mananging autocommit +`. + +To disable autocommit temporarily, instead of:: + + with transaction.commit_manually(): + # do stuff + +you should now use:: + + transaction.set_autocommit(autocommit=False) + try: + # do stuff + finally: + transaction.set_autocommit(autocommit=True) + +To enable autocommit temporarily, instead of:: + + with transaction.autocommit(): + # do stuff + +you should now use:: + + transaction.set_autocommit(autocommit=True) + try: + # do stuff + finally: + transaction.set_autocommit(autocommit=False) + +Backwards incompatibilities +--------------------------- Since version 1.6, Django uses database-level autocommit in auto mode. - Previously, it implemented application-level autocommit by triggering a commit after each ORM write. -As a consequence, each database query (for instance, an -ORM read) started a transaction that lasted until the next ORM write. Such -"automatic transactions" no longer exist in Django 1.6. +As a consequence, each database query (for instance, an ORM read) started a +transaction that lasted until the next ORM write. Such "automatic +transactions" no longer exist in Django 1.6. There are four known scenarios where this is backwards-incompatible. @@ -565,7 +525,7 @@ Note that managed mode isn't affected at all. This section assumes auto mode. See the :ref:`description of modes ` above. Sequences of custom SQL queries -------------------------------- +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ If you're executing several :ref:`custom SQL queries ` in a row, each one now runs in its own transaction, instead of sharing the @@ -577,20 +537,20 @@ usually followed by a call to ``transaction.commit_unless_managed``, which isn't necessary any more and should be removed. Select for update ------------------ +~~~~~~~~~~~~~~~~~ If you were relying on "automatic transactions" to provide locking between :meth:`~django.db.models.query.QuerySet.select_for_update` and a subsequent write operation — an extremely fragile design, but nonetheless possible — you -must wrap the relevant code in :func:`commit_on_success`. +must wrap the relevant code in :func:`atomic`. Using a high isolation level ----------------------------- +~~~~~~~~~~~~~~~~~~~~~~~~~~~~ If you were using the "repeatable read" isolation level or higher, and if you relied on "automatic transactions" to guarantee consistency between successive -reads, the new behavior is backwards-incompatible. To maintain consistency, -you must wrap such sequences in :func:`commit_on_success`. +reads, the new behavior might be backwards-incompatible. To enforce +consistency, you must wrap such sequences in :func:`atomic`. MySQL defaults to "repeatable read" and SQLite to "serializable"; they may be affected by this problem. @@ -602,10 +562,9 @@ PostgreSQL and Oracle default to "read committed" and aren't affected, unless you changed the isolation level. Using unsupported database features ------------------------------------ +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ With triggers, views, or functions, it's possible to make ORM reads result in database modifications. Django 1.5 and earlier doesn't deal with this case and it's theoretically possible to observe a different behavior after upgrading to -Django 1.6 or later. In doubt, use :func:`commit_on_success` to enforce -integrity. +Django 1.6 or later. In doubt, use :func:`atomic` to enforce integrity. diff --git a/tests/backends/tests.py b/tests/backends/tests.py index 5c8a8955eb..51acbcb07f 100644 --- a/tests/backends/tests.py +++ b/tests/backends/tests.py @@ -522,7 +522,8 @@ class FkConstraintsTests(TransactionTestCase): """ When constraint checks are disabled, should be able to write bad data without IntegrityErrors. """ - with transaction.commit_manually(): + transaction.set_autocommit(autocommit=False) + try: # Create an Article. models.Article.objects.create(headline="Test article", pub_date=datetime.datetime(2010, 9, 4), reporter=self.r) # Retrive it from the DB @@ -536,12 +537,15 @@ class FkConstraintsTests(TransactionTestCase): self.fail("IntegrityError should not have occurred.") finally: transaction.rollback() + finally: + transaction.set_autocommit(autocommit=True) def test_disable_constraint_checks_context_manager(self): """ When constraint checks are disabled (using context manager), should be able to write bad data without IntegrityErrors. """ - with transaction.commit_manually(): + transaction.set_autocommit(autocommit=False) + try: # Create an Article. models.Article.objects.create(headline="Test article", pub_date=datetime.datetime(2010, 9, 4), reporter=self.r) # Retrive it from the DB @@ -554,12 +558,15 @@ class FkConstraintsTests(TransactionTestCase): self.fail("IntegrityError should not have occurred.") finally: transaction.rollback() + finally: + transaction.set_autocommit(autocommit=True) def test_check_constraints(self): """ Constraint checks should raise an IntegrityError when bad data is in the DB. """ - with transaction.commit_manually(): + try: + transaction.set_autocommit(autocommit=False) # Create an Article. models.Article.objects.create(headline="Test article", pub_date=datetime.datetime(2010, 9, 4), reporter=self.r) # Retrive it from the DB @@ -572,6 +579,8 @@ class FkConstraintsTests(TransactionTestCase): connection.check_constraints() finally: transaction.rollback() + finally: + transaction.set_autocommit(autocommit=True) class ThreadTests(TestCase): diff --git a/tests/fixtures_model_package/tests.py b/tests/fixtures_model_package/tests.py index d147fe68a7..894a6c7fde 100644 --- a/tests/fixtures_model_package/tests.py +++ b/tests/fixtures_model_package/tests.py @@ -25,7 +25,8 @@ class SampleTestCase(TestCase): class TestNoInitialDataLoading(TransactionTestCase): def test_syncdb(self): - with transaction.commit_manually(): + transaction.set_autocommit(autocommit=False) + try: Book.objects.all().delete() management.call_command( @@ -35,6 +36,9 @@ class TestNoInitialDataLoading(TransactionTestCase): ) self.assertQuerysetEqual(Book.objects.all(), []) transaction.rollback() + finally: + transaction.set_autocommit(autocommit=True) + def test_flush(self): # Test presence of fixture (flush called by TransactionTestCase) @@ -45,7 +49,8 @@ class TestNoInitialDataLoading(TransactionTestCase): lambda a: a.name ) - with transaction.commit_manually(): + transaction.set_autocommit(autocommit=False) + try: management.call_command( 'flush', verbosity=0, @@ -55,6 +60,8 @@ class TestNoInitialDataLoading(TransactionTestCase): ) self.assertQuerysetEqual(Book.objects.all(), []) transaction.rollback() + finally: + transaction.set_autocommit(autocommit=True) class FixtureTestCase(TestCase): diff --git a/tests/fixtures_regress/tests.py b/tests/fixtures_regress/tests.py index 61dc4460df..f965dd81ac 100644 --- a/tests/fixtures_regress/tests.py +++ b/tests/fixtures_regress/tests.py @@ -684,5 +684,8 @@ class TestTicket11101(TransactionTestCase): @skipUnlessDBFeature('supports_transactions') def test_ticket_11101(self): """Test that fixtures can be rolled back (ticket #11101).""" - ticket_11101 = transaction.commit_manually(self.ticket_11101) - ticket_11101() + transaction.set_autocommit(autocommit=False) + try: + self.ticket_11101() + finally: + transaction.set_autocommit(autocommit=True) diff --git a/tests/middleware/tests.py b/tests/middleware/tests.py index e704fce342..7e26037967 100644 --- a/tests/middleware/tests.py +++ b/tests/middleware/tests.py @@ -24,6 +24,8 @@ from django.utils.encoding import force_str from django.utils.six.moves import xrange from django.utils.unittest import expectedFailure +from transactions.tests import IgnorePendingDeprecationWarningsMixin + from .models import Band @@ -670,11 +672,12 @@ class ETagGZipMiddlewareTest(TestCase): self.assertNotEqual(gzip_etag, nogzip_etag) -class TransactionMiddlewareTest(TransactionTestCase): +class TransactionMiddlewareTest(IgnorePendingDeprecationWarningsMixin, TransactionTestCase): """ Test the transaction middleware. """ def setUp(self): + super(TransactionMiddlewareTest, self).setUp() self.request = HttpRequest() self.request.META = { 'SERVER_NAME': 'testserver', @@ -686,6 +689,7 @@ class TransactionMiddlewareTest(TransactionTestCase): def tearDown(self): transaction.abort() + super(TransactionMiddlewareTest, self).tearDown() def test_request(self): TransactionMiddleware().process_request(self.request) diff --git a/tests/transactions/tests.py b/tests/transactions/tests.py index 14252dd6dc..d6cfd8ae95 100644 --- a/tests/transactions/tests.py +++ b/tests/transactions/tests.py @@ -1,9 +1,10 @@ from __future__ import absolute_import import sys +import warnings from django.db import connection, transaction, IntegrityError -from django.test import TestCase, TransactionTestCase, skipUnlessDBFeature +from django.test import TransactionTestCase, skipUnlessDBFeature from django.utils import six from django.utils.unittest import skipUnless @@ -158,7 +159,69 @@ class AtomicInsideTransactionTests(AtomicTests): self.atomic.__exit__(*sys.exc_info()) -class TransactionTests(TransactionTestCase): +class AtomicInsideLegacyTransactionManagementTests(AtomicTests): + + def setUp(self): + transaction.enter_transaction_management() + + def tearDown(self): + # The tests access the database after exercising 'atomic', making the + # connection dirty; a rollback is required to make it clean. + transaction.rollback() + transaction.leave_transaction_management() + + +@skipUnless(connection.features.uses_savepoints, + "'atomic' requires transactions and savepoints.") +class AtomicErrorsTests(TransactionTestCase): + + def test_atomic_requires_autocommit(self): + transaction.set_autocommit(autocommit=False) + try: + with self.assertRaises(transaction.TransactionManagementError): + with transaction.atomic(): + pass + finally: + transaction.set_autocommit(autocommit=True) + + def test_atomic_prevents_disabling_autocommit(self): + autocommit = transaction.get_autocommit() + with transaction.atomic(): + with self.assertRaises(transaction.TransactionManagementError): + transaction.set_autocommit(autocommit=not autocommit) + # Make sure autocommit wasn't changed. + self.assertEqual(connection.autocommit, autocommit) + + def test_atomic_prevents_calling_transaction_methods(self): + with transaction.atomic(): + with self.assertRaises(transaction.TransactionManagementError): + transaction.commit() + with self.assertRaises(transaction.TransactionManagementError): + transaction.rollback() + + def test_atomic_prevents_calling_transaction_management_methods(self): + with transaction.atomic(): + with self.assertRaises(transaction.TransactionManagementError): + transaction.enter_transaction_management() + with self.assertRaises(transaction.TransactionManagementError): + transaction.leave_transaction_management() + + +class IgnorePendingDeprecationWarningsMixin(object): + + def setUp(self): + super(IgnorePendingDeprecationWarningsMixin, self).setUp() + self.catch_warnings = warnings.catch_warnings() + self.catch_warnings.__enter__() + warnings.filterwarnings("ignore", category=PendingDeprecationWarning) + + def tearDown(self): + self.catch_warnings.__exit__(*sys.exc_info()) + super(IgnorePendingDeprecationWarningsMixin, self).tearDown() + + +class TransactionTests(IgnorePendingDeprecationWarningsMixin, TransactionTestCase): + def create_a_reporter_then_fail(self, first, last): a = Reporter(first_name=first, last_name=last) a.save() @@ -313,7 +376,7 @@ class TransactionTests(TransactionTestCase): ) -class TransactionRollbackTests(TransactionTestCase): +class TransactionRollbackTests(IgnorePendingDeprecationWarningsMixin, TransactionTestCase): def execute_bad_sql(self): cursor = connection.cursor() cursor.execute("INSERT INTO transactions_reporter (first_name, last_name) VALUES ('Douglas', 'Adams');") @@ -330,7 +393,7 @@ class TransactionRollbackTests(TransactionTestCase): self.assertRaises(IntegrityError, execute_bad_sql) transaction.rollback() -class TransactionContextManagerTests(TransactionTestCase): +class TransactionContextManagerTests(IgnorePendingDeprecationWarningsMixin, TransactionTestCase): def create_reporter_and_fail(self): Reporter.objects.create(first_name="Bob", last_name="Holtzman") raise Exception diff --git a/tests/transactions_regress/tests.py b/tests/transactions_regress/tests.py index e86db4d0aa..d5ee62da5e 100644 --- a/tests/transactions_regress/tests.py +++ b/tests/transactions_regress/tests.py @@ -6,10 +6,12 @@ from django.test import TransactionTestCase, skipUnlessDBFeature from django.test.utils import override_settings from django.utils.unittest import skipIf, skipUnless, expectedFailure +from transactions.tests import IgnorePendingDeprecationWarningsMixin + from .models import Mod, M2mA, M2mB -class TestTransactionClosing(TransactionTestCase): +class TestTransactionClosing(IgnorePendingDeprecationWarningsMixin, TransactionTestCase): """ Tests to make sure that transactions are properly closed when they should be, and aren't left pending after operations @@ -166,7 +168,7 @@ class TestTransactionClosing(TransactionTestCase): (connection.settings_dict['NAME'] == ':memory:' or not connection.settings_dict['NAME']), 'Test uses multiple connections, but in-memory sqlite does not support this') -class TestNewConnection(TransactionTestCase): +class TestNewConnection(IgnorePendingDeprecationWarningsMixin, TransactionTestCase): """ Check that new connections don't have special behaviour. """ @@ -211,7 +213,7 @@ class TestNewConnection(TransactionTestCase): @skipUnless(connection.vendor == 'postgresql', "This test only valid for PostgreSQL") -class TestPostgresAutocommitAndIsolation(TransactionTestCase): +class TestPostgresAutocommitAndIsolation(IgnorePendingDeprecationWarningsMixin, TransactionTestCase): """ Tests to make sure psycopg2's autocommit mode and isolation level is restored after entering and leaving transaction management. @@ -292,7 +294,7 @@ class TestPostgresAutocommitAndIsolation(TransactionTestCase): self.assertTrue(connection.autocommit) -class TestManyToManyAddTransaction(TransactionTestCase): +class TestManyToManyAddTransaction(IgnorePendingDeprecationWarningsMixin, TransactionTestCase): def test_manyrelated_add_commit(self): "Test for https://code.djangoproject.com/ticket/16818" a = M2mA.objects.create() @@ -307,7 +309,7 @@ class TestManyToManyAddTransaction(TransactionTestCase): self.assertEqual(a.others.count(), 1) -class SavepointTest(TransactionTestCase): +class SavepointTest(IgnorePendingDeprecationWarningsMixin, TransactionTestCase): @skipIf(connection.vendor == 'sqlite', "SQLite doesn't support savepoints in managed mode") From 09ba70f9f1844680ac0e0b3a7c38ff7113cbdb02 Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Tue, 5 Mar 2013 22:30:55 +0100 Subject: [PATCH 18/32] Made transaction management work even before the first SQL query. Thanks Florian again. --- django/db/backends/__init__.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/django/db/backends/__init__.py b/django/db/backends/__init__.py index 346d10198d..68551aad51 100644 --- a/django/db/backends/__init__.py +++ b/django/db/backends/__init__.py @@ -269,6 +269,8 @@ class BaseDatabaseWrapper(object): """ self.validate_no_atomic_block() + self.ensure_connection() + self.transaction_state.append(managed) if not managed and self.is_dirty() and not forced: @@ -286,6 +288,8 @@ class BaseDatabaseWrapper(object): """ self.validate_no_atomic_block() + self.ensure_connection() + if self.transaction_state: del self.transaction_state[-1] else: From f7245b83bb2df9d66a375d46a4be8de093957fa7 Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Thu, 7 Mar 2013 11:38:21 +0100 Subject: [PATCH 19/32] Implemented atomic_if_autocommit. It disables transaction management entirely when AUTOCOMMIT is False. --- django/db/transaction.py | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/django/db/transaction.py b/django/db/transaction.py index eb9d85e274..0799d23bb6 100644 --- a/django/db/transaction.py +++ b/django/db/transaction.py @@ -308,6 +308,24 @@ def atomic(using=None): return Atomic(using) +def atomic_if_autocommit(using=None): + # 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) + else: + # Bare decorator: @atomic_if_autocommit + if callable(using): + return using + # Decorator: @atomic_if_autocommit(...) + else: + return lambda func: func + + ############################################ # Deprecated decorators / context managers # ############################################ @@ -431,13 +449,13 @@ def commit_on_success_unless_managed(using=None): Transitory API to preserve backwards-compatibility while refactoring. Once the legacy transaction management is fully deprecated, this should - simply be replaced by atomic. 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_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. """ connection = get_connection(using) if connection.autocommit or connection.in_atomic_block: - return atomic(using) + return atomic_if_autocommit(using) else: def entering(using): pass From ac37ed21b3d66dde1748f6edf3279656b0267b70 Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Wed, 6 Mar 2013 11:12:24 +0100 Subject: [PATCH 20/32] Deprecated TransactionMiddleware and TRANSACTIONS_MANAGED. Replaced them with per-database options, for proper multi-db support. Also toned down the recommendation to tie transactions to HTTP requests. Thanks Jeremy for sharing his experience. --- django/core/handlers/base.py | 12 ++- django/db/backends/__init__.py | 4 +- django/db/utils.py | 8 ++ django/middleware/transaction.py | 13 +++- docs/internals/deprecation.txt | 11 ++- docs/ref/middleware.txt | 4 + docs/ref/settings.txt | 30 ++++++++ docs/releases/1.6.txt | 8 +- docs/topics/db/transactions.txt | 122 ++++++++++++++++++++++--------- tests/handlers/tests.py | 30 +++++++- tests/handlers/urls.py | 9 ++- tests/handlers/views.py | 17 +++++ 12 files changed, 217 insertions(+), 51 deletions(-) create mode 100644 tests/handlers/views.py diff --git a/django/core/handlers/base.py b/django/core/handlers/base.py index 0dcd9794c7..5327ce5891 100644 --- a/django/core/handlers/base.py +++ b/django/core/handlers/base.py @@ -6,10 +6,10 @@ import types from django import http from django.conf import settings -from django.core import exceptions from django.core import urlresolvers from django.core import signals from django.core.exceptions import MiddlewareNotUsed, PermissionDenied +from django.db import connections, transaction from django.utils.encoding import force_text from django.utils.module_loading import import_by_path from django.utils import six @@ -65,6 +65,13 @@ class BaseHandler(object): # as a flag for initialization being complete. self._request_middleware = request_middleware + def make_view_atomic(self, view): + if getattr(view, 'transactions_per_request', True): + for db in connections.all(): + if db.settings_dict['ATOMIC_REQUESTS']: + view = transaction.atomic(using=db.alias)(view) + return view + def get_response(self, request): "Returns an HttpResponse object for the given HttpRequest" try: @@ -101,8 +108,9 @@ class BaseHandler(object): break if response is None: + wrapped_callback = self.make_view_atomic(callback) try: - response = callback(request, *callback_args, **callback_kwargs) + response = wrapped_callback(request, *callback_args, **callback_kwargs) except Exception as e: # If the view raised an exception, run it through exception # middleware, and if the exception middleware returns a diff --git a/django/db/backends/__init__.py b/django/db/backends/__init__.py index 68551aad51..2cf75bd528 100644 --- a/django/db/backends/__init__.py +++ b/django/db/backends/__init__.py @@ -104,7 +104,7 @@ class BaseDatabaseWrapper(object): conn_params = self.get_connection_params() self.connection = self.get_new_connection(conn_params) self.init_connection_state() - if not settings.TRANSACTIONS_MANAGED: + if self.settings_dict['AUTOCOMMIT']: self.set_autocommit() connection_created.send(sender=self.__class__, connection=self) @@ -299,7 +299,7 @@ class BaseDatabaseWrapper(object): if self.transaction_state: managed = self.transaction_state[-1] else: - managed = settings.TRANSACTIONS_MANAGED + managed = not self.settings_dict['AUTOCOMMIT'] if self._dirty: self.rollback() diff --git a/django/db/utils.py b/django/db/utils.py index 71b89f93fb..936b42039d 100644 --- a/django/db/utils.py +++ b/django/db/utils.py @@ -2,6 +2,7 @@ from functools import wraps import os import pkgutil from threading import local +import warnings from django.conf import settings from django.core.exceptions import ImproperlyConfigured @@ -158,6 +159,13 @@ class ConnectionHandler(object): except KeyError: raise ConnectionDoesNotExist("The connection %s doesn't exist" % alias) + conn.setdefault('ATOMIC_REQUESTS', False) + if settings.TRANSACTIONS_MANAGED: + warnings.warn( + "TRANSACTIONS_MANAGED is deprecated. Use AUTOCOMMIT instead.", + PendingDeprecationWarning, stacklevel=2) + conn.setdefault('AUTOCOMMIT', False) + conn.setdefault('AUTOCOMMIT', True) conn.setdefault('ENGINE', 'django.db.backends.dummy') if conn['ENGINE'] == 'django.db.backends.' or not conn['ENGINE']: conn['ENGINE'] = 'django.db.backends.dummy' diff --git a/django/middleware/transaction.py b/django/middleware/transaction.py index 35f765d99f..95cc9a21f3 100644 --- a/django/middleware/transaction.py +++ b/django/middleware/transaction.py @@ -1,4 +1,7 @@ -from django.db import transaction +import warnings + +from django.core.exceptions import MiddlewareNotUsed +from django.db import connection, transaction class TransactionMiddleware(object): """ @@ -7,6 +10,14 @@ class TransactionMiddleware(object): commit, the commit is done when a successful response is created. If an exception happens, the database is rolled back. """ + + def __init__(self): + warnings.warn( + "TransactionMiddleware is deprecated in favor of ATOMIC_REQUESTS.", + PendingDeprecationWarning, stacklevel=2) + if connection.settings_dict['ATOMIC_REQUESTS']: + raise MiddlewareNotUsed + def process_request(self, request): """Enters transaction management""" transaction.enter_transaction_management() diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index 6c13af7ae4..19675801e4 100644 --- a/docs/internals/deprecation.txt +++ b/docs/internals/deprecation.txt @@ -329,9 +329,14 @@ these changes. 1.8 --- -* The decorators and context managers ``django.db.transaction.autocommit``, - ``commit_on_success`` and ``commit_manually`` will be removed. See - :ref:`transactions-upgrading-from-1.5`. +* The following transaction management APIs will be removed: + + - ``TransactionMiddleware``, + - the decorators and context managers ``autocommit``, ``commit_on_success``, + and ``commit_manually``, + - the ``TRANSACTIONS_MANAGED`` setting. + + Upgrade paths are described in :ref:`transactions-upgrading-from-1.5`. * The :ttag:`cycle` and :ttag:`firstof` template tags will auto-escape their arguments. In 1.6 and 1.7, this behavior is provided by the version of these diff --git a/docs/ref/middleware.txt b/docs/ref/middleware.txt index 1e6e57f720..20bb2fb751 100644 --- a/docs/ref/middleware.txt +++ b/docs/ref/middleware.txt @@ -205,6 +205,10 @@ Transaction middleware .. class:: TransactionMiddleware +.. versionchanged:: 1.6 + ``TransactionMiddleware`` is deprecated. The documentation of transactions + contains :ref:`upgrade instructions `. + Binds commit and rollback of the default database to the request/response phase. If a view function runs successfully, a commit is done. If it fails with an exception, a rollback is done. diff --git a/docs/ref/settings.txt b/docs/ref/settings.txt index 0cd141bcef..2b80527d8b 100644 --- a/docs/ref/settings.txt +++ b/docs/ref/settings.txt @@ -408,6 +408,30 @@ SQLite. This can be configured using the following:: For other database backends, or more complex SQLite configurations, other options will be required. The following inner options are available. +.. setting:: DATABASE-ATOMIC_REQUESTS + +ATOMIC_REQUESTS +~~~~~~~~~~~~~~~ + +.. versionadded:: 1.6 + +Default: ``False`` + +Set this to ``True`` to wrap each HTTP request in a transaction on this +database. See :ref:`tying-transactions-to-http-requests`. + +.. setting:: DATABASE-AUTOCOMMIT + +AUTOCOMMIT +~~~~~~~~~~ + +.. versionadded:: 1.6 + +Default: ``True`` + +Set this to ``False`` if you want to :ref:`disable Django's transaction +management ` and implement your own. + .. setting:: DATABASE-ENGINE ENGINE @@ -1807,6 +1831,12 @@ to ensure your processes are running in the correct environment. TRANSACTIONS_MANAGED -------------------- +.. deprecated:: 1.6 + + This setting was deprecated because its name is very misleading. Use the + :setting:`AUTOCOMMIT ` key in :setting:`DATABASES` + entries instead. + Default: ``False`` Set this to ``True`` if you want to :ref:`disable Django's transaction diff --git a/docs/releases/1.6.txt b/docs/releases/1.6.txt index cc3bf94ef5..a1fe69229c 100644 --- a/docs/releases/1.6.txt +++ b/docs/releases/1.6.txt @@ -262,9 +262,11 @@ Transaction management APIs Transaction management was completely overhauled in Django 1.6, and the current APIs are deprecated: -- :func:`django.db.transaction.autocommit` -- :func:`django.db.transaction.commit_on_success` -- :func:`django.db.transaction.commit_manually` +- ``django.middleware.transaction.TransactionMiddleware`` +- ``django.db.transaction.autocommit`` +- ``django.db.transaction.commit_on_success`` +- ``django.db.transaction.commit_manually`` +- the ``TRANSACTIONS_MANAGED`` setting The reasons for this change and the upgrade path are described in the :ref:`transactions documentation `. diff --git a/docs/topics/db/transactions.txt b/docs/topics/db/transactions.txt index 91b2cf41b3..37a369a02f 100644 --- a/docs/topics/db/transactions.txt +++ b/docs/topics/db/transactions.txt @@ -26,45 +26,61 @@ immediately committed to the database. :ref:`See below for details Previous version of Django featured :ref:`a more complicated default behavior `. +.. _tying-transactions-to-http-requests: + Tying transactions to HTTP requests ----------------------------------- -The recommended way to handle transactions in Web requests is to tie them to -the request and response phases via Django's ``TransactionMiddleware``. +A common way to handle transactions on the web is to wrap each request in a +transaction. Set :setting:`ATOMIC_REQUESTS ` to +``True`` in the configuration of each database for which you want to enable +this behavior. It works like this. When a request starts, Django starts a transaction. If the -response is produced without problems, Django commits any pending transactions. -If the view function produces an exception, Django rolls back any pending -transactions. +response is produced without problems, Django commits the transaction. If the +view function produces an exception, Django rolls back the transaction. +Middleware always runs outside of this transaction. -To activate this feature, just add the ``TransactionMiddleware`` middleware to -your :setting:`MIDDLEWARE_CLASSES` setting:: +You may perfom partial commits and rollbacks in your view code, typically with +the :func:`atomic` context manager. However, at the end of the view, either +all the changes will be committed, or none of them. - MIDDLEWARE_CLASSES = ( - 'django.middleware.cache.UpdateCacheMiddleware', - 'django.contrib.sessions.middleware.SessionMiddleware', - 'django.middleware.common.CommonMiddleware', - 'django.middleware.transaction.TransactionMiddleware', - 'django.middleware.cache.FetchFromCacheMiddleware', - ) +To disable this behavior for a specific view, you must set the +``transactions_per_request`` attribute of the view function itself to +``False``, like this:: -The order is quite important. The transaction middleware applies not only to -view functions, but also for all middleware modules that come after it. So if -you use the session middleware after the transaction middleware, session -creation will be part of the transaction. + def my_view(request): + do_stuff() + my_view.transactions_per_request = False -The various cache middlewares are an exception: ``CacheMiddleware``, -:class:`~django.middleware.cache.UpdateCacheMiddleware`, and -:class:`~django.middleware.cache.FetchFromCacheMiddleware` are never affected. -Even when using database caching, Django's cache backend uses its own database -connection internally. +.. warning:: -.. note:: + While the simplicity of this transaction model is appealing, it also makes it + inefficient when traffic increases. Opening a transaction for every view has + some overhead. The impact on performance depends on the query patterns of your + application and on how well your database handles locking. - The ``TransactionMiddleware`` only affects the database aliased - as "default" within your :setting:`DATABASES` setting. If you are using - multiple databases and want transaction control over databases other than - "default", you will need to write your own transaction middleware. +.. admonition:: Per-request transactions and streaming responses + + When a view returns a :class:`~django.http.StreamingHttpResponse`, reading + the contents of the response will often execute code to generate the + content. Since the view has already returned, such code runs outside of + the transaction. + + Generally speaking, it isn't advisable to write to the database while + generating a streaming response, since there's no sensible way to handle + errors after starting to send the response. + +In practice, this feature simply wraps every view function in the :func:`atomic` +decorator described below. + +Note that only the execution of your view in enclosed in the transactions. +Middleware run outside of the transaction, and so does the rendering of +template responses. + +.. versionchanged:: 1.6 + Django used to provide this feature via ``TransactionMiddleware``, which is + now deprecated. Controlling transactions explicitly ----------------------------------- @@ -283,18 +299,20 @@ if autocommit is off. Django will also refuse to turn autocommit off when an Deactivating transaction management ----------------------------------- -Control freaks can totally disable all transaction management by setting -:setting:`TRANSACTIONS_MANAGED` to ``True`` in the Django settings file. If -you do this, Django won't enable autocommit. You'll get the regular behavior -of the underlying database library. +You can totally disable Django's transaction management for a given database +by setting :setting:`AUTOCOMMIT ` to ``False`` in its +configuration. If you do this, Django won't enable autocommit, and won't +perform any commits. You'll get the regular behavior of the underlying +database library. This requires you to commit explicitly every transaction, even those started by Django or by third-party libraries. Thus, this is best used in situations where you want to run your own transaction-controlling middleware or do something really strange. -In almost all situations, you'll be better off using the default behavior, or -the transaction middleware, and only modify selected functions as needed. +.. versionchanged:: 1.6 + This used to be controlled by the ``TRANSACTIONS_MANAGED`` setting. + Database-specific notes ======================= @@ -459,6 +477,35 @@ atomicity of the outer block. API changes ----------- +Transaction middleware +~~~~~~~~~~~~~~~~~~~~~~ + +In Django 1.6, ``TransactionMiddleware`` is deprecated and replaced +:setting:`ATOMIC_REQUESTS `. While the general +behavior is the same, there are a few differences. + +With the transaction middleware, it was still possible to switch to autocommit +or to commit explicitly in a view. Since :func:`atomic` guarantees atomicity, +this isn't allowed any longer. + +To avoid wrapping a particular view in a transaction, instead of:: + + @transaction.autocommit + def my_view(request): + do_stuff() + +you must now use this pattern:: + + def my_view(request): + do_stuff() + my_view.transactions_per_request = False + +The transaction middleware applied not only to view functions, but also to +middleware modules that come after it. For instance, if you used the session +middleware after the transaction middleware, session creation was part of the +transaction. :setting:`ATOMIC_REQUESTS ` only +applies to the view itself. + Managing transactions ~~~~~~~~~~~~~~~~~~~~~ @@ -508,6 +555,13 @@ you should now use:: finally: transaction.set_autocommit(autocommit=False) +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`. + Backwards incompatibilities --------------------------- diff --git a/tests/handlers/tests.py b/tests/handlers/tests.py index 6eb9bd23fe..3680eecdd2 100644 --- a/tests/handlers/tests.py +++ b/tests/handlers/tests.py @@ -1,9 +1,8 @@ from django.core.handlers.wsgi import WSGIHandler from django.core.signals import request_started, request_finished -from django.db import close_old_connections -from django.test import RequestFactory, TestCase +from django.db import close_old_connections, connection +from django.test import RequestFactory, TestCase, TransactionTestCase from django.test.utils import override_settings -from django.utils import six class HandlerTests(TestCase): @@ -37,6 +36,31 @@ class HandlerTests(TestCase): self.assertEqual(response.status_code, 400) +class TransactionsPerRequestTests(TransactionTestCase): + urls = 'handlers.urls' + + def test_no_transaction(self): + response = self.client.get('/in_transaction/') + self.assertContains(response, 'False') + + def test_auto_transaction(self): + old_atomic_requests = connection.settings_dict['ATOMIC_REQUESTS'] + try: + connection.settings_dict['ATOMIC_REQUESTS'] = True + response = self.client.get('/in_transaction/') + finally: + connection.settings_dict['ATOMIC_REQUESTS'] = old_atomic_requests + self.assertContains(response, 'True') + + def test_no_auto_transaction(self): + old_atomic_requests = connection.settings_dict['ATOMIC_REQUESTS'] + try: + connection.settings_dict['ATOMIC_REQUESTS'] = True + response = self.client.get('/not_in_transaction/') + finally: + connection.settings_dict['ATOMIC_REQUESTS'] = old_atomic_requests + self.assertContains(response, 'False') + class SignalsTests(TestCase): urls = 'handlers.urls' diff --git a/tests/handlers/urls.py b/tests/handlers/urls.py index 8570f04696..29858055ab 100644 --- a/tests/handlers/urls.py +++ b/tests/handlers/urls.py @@ -1,9 +1,12 @@ from __future__ import unicode_literals from django.conf.urls import patterns, url -from django.http import HttpResponse, StreamingHttpResponse + +from . import views urlpatterns = patterns('', - url(r'^regular/$', lambda request: HttpResponse(b"regular content")), - url(r'^streaming/$', lambda request: StreamingHttpResponse([b"streaming", b" ", b"content"])), + url(r'^regular/$', views.regular), + url(r'^streaming/$', views.streaming), + url(r'^in_transaction/$', views.in_transaction), + url(r'^not_in_transaction/$', views.not_in_transaction), ) diff --git a/tests/handlers/views.py b/tests/handlers/views.py new file mode 100644 index 0000000000..22d9ea4c7d --- /dev/null +++ b/tests/handlers/views.py @@ -0,0 +1,17 @@ +from __future__ import unicode_literals + +from django.db import connection +from django.http import HttpResponse, StreamingHttpResponse + +def regular(request): + return HttpResponse(b"regular content") + +def streaming(request): + return StreamingHttpResponse([b"streaming", b" ", b"content"]) + +def in_transaction(request): + return HttpResponse(str(connection.in_atomic_block)) + +def not_in_transaction(request): + return HttpResponse(str(connection.in_atomic_block)) +not_in_transaction.transactions_per_request = False From 557e40412729c06a2daff34a4cabcb6efb719b32 Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Thu, 7 Mar 2013 14:09:52 +0100 Subject: [PATCH 21/32] Re-ordered functions by deprecation status. --- django/db/transaction.py | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/django/db/transaction.py b/django/db/transaction.py index 0799d23bb6..a6eb0662d4 100644 --- a/django/db/transaction.py +++ b/django/db/transaction.py @@ -39,17 +39,9 @@ def get_connection(using=None): using = DEFAULT_DB_ALIAS return connections[using] -def get_autocommit(using=None): - """ - Get the autocommit status of the connection. - """ - return get_connection(using).autocommit - -def set_autocommit(using=None, autocommit=True): - """ - Set the autocommit status of the connection. - """ - return get_connection(using).set_autocommit(autocommit) +########################### +# Deprecated private APIs # +########################### def abort(using=None): """ @@ -106,12 +98,6 @@ def set_clean(using=None): """ get_connection(using).set_clean() -def clean_savepoints(using=None): - """ - Resets the counter used to generate unique savepoint ids in this thread. - """ - get_connection(using).clean_savepoints() - def is_managed(using=None): warnings.warn("'is_managed' is deprecated.", PendingDeprecationWarning, stacklevel=2) @@ -132,6 +118,18 @@ def rollback_unless_managed(using=None): # Public APIs # ############### +def get_autocommit(using=None): + """ + Get the autocommit status of the connection. + """ + return get_connection(using).autocommit + +def set_autocommit(using=None, autocommit=True): + """ + Set the autocommit status of the connection. + """ + return get_connection(using).set_autocommit(autocommit) + def commit(using=None): """ Commits a transaction and resets the dirty flag. @@ -166,6 +164,11 @@ def savepoint_commit(sid, using=None): """ get_connection(using).savepoint_commit(sid) +def clean_savepoints(using=None): + """ + Resets the counter used to generate unique savepoint ids in this thread. + """ + get_connection(using).clean_savepoints() ################################# # Decorators / context managers # From ffe41591e75fc3acf76c634bdd0899d78e91688d Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Thu, 7 Mar 2013 14:05:32 +0100 Subject: [PATCH 22/32] Updated the documentation for savepoints. Apparently django.db.transaction used to be an object. --- docs/topics/db/transactions.txt | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/docs/topics/db/transactions.txt b/docs/topics/db/transactions.txt index 37a369a02f..bcedd90424 100644 --- a/docs/topics/db/transactions.txt +++ b/docs/topics/db/transactions.txt @@ -193,24 +193,32 @@ Each of these functions takes a ``using`` argument which should be the name of a database for which the behavior applies. If no ``using`` argument is provided then the ``"default"`` database is used. -Savepoints are controlled by three methods on the transaction object: +Savepoints are controlled by three functions in :mod:`django.db.transaction`: -.. method:: transaction.savepoint(using=None) +.. function:: savepoint(using=None) Creates a new savepoint. This marks a point in the transaction that is known to be in a "good" state. - Returns the savepoint ID (sid). + Returns the savepoint ID (``sid``). -.. method:: transaction.savepoint_commit(sid, using=None) +.. function:: savepoint_commit(sid, using=None) - Updates the savepoint to include any operations that have been performed - since the savepoint was created, or since the last commit. + Releases savepoint ``sid``. The changes performed since the savepoint was + created become part of the transaction. -.. method:: transaction.savepoint_rollback(sid, using=None) +.. function:: savepoint_rollback(sid, using=None) - Rolls the transaction back to the last point at which the savepoint was - committed. + Rolls back the transaction to savepoint ``sid``. + +These functions do nothing if savepoints aren't supported or if the database +is in autocommit mode. + +In addition, there's a utility function: + +.. function:: clean_savepoints(using=None) + + Resets the counter used to generate unique savepoint IDs. The following example demonstrates the use of savepoints:: From 17cf29920b3b3a4870c865cb53f6b935187ba4e4 Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Thu, 7 Mar 2013 14:42:51 +0100 Subject: [PATCH 23/32] Added an explanation of transactions and grouped low-level APIs. --- docs/topics/db/transactions.txt | 202 +++++++++++++++++++------------- 1 file changed, 122 insertions(+), 80 deletions(-) diff --git a/docs/topics/db/transactions.txt b/docs/topics/db/transactions.txt index bcedd90424..4cecf896a4 100644 --- a/docs/topics/db/transactions.txt +++ b/docs/topics/db/transactions.txt @@ -164,10 +164,131 @@ Django provides a single API to control database transactions. - releases or rolls back to the savepoint when exiting an inner block; - commits or rolls back the transaction when exiting the outermost block. +Autocommit +========== + +.. _autocommit-details: + +Why Django uses autocommit +-------------------------- + +In the SQL standards, each SQL query starts a transaction, unless one is +already in progress. Such transactions must then be committed or rolled back. + +This isn't always convenient for application developers. To alleviate this +problem, most databases provide an autocommit mode. When autocommit is turned +on, each SQL query is wrapped in its own transaction. In other words, the +transaction is not only automatically started, but also automatically +committed. + +:pep:`249`, the Python Database API Specification v2.0, requires autocommit to +be initially turned off. Django overrides this default and turns autocommit +on. + +To avoid this, you can :ref:`deactivate the transaction management +`, but it isn't recommended. + +.. versionchanged:: 1.6 + Before Django 1.6, autocommit was turned off, and it was emulated by + forcing a commit after write operations in the ORM. + +.. warning:: + + If you're using the database API directly — for instance, you're running + SQL queries with ``cursor.execute()`` — be aware that autocommit is on, + and consider wrapping your operations in a transaction, with + :func:`atomic`, to ensure consistency. + +.. _deactivate-transaction-management: + +Deactivating transaction management +----------------------------------- + +You can totally disable Django's transaction management for a given database +by setting :setting:`AUTOCOMMIT ` to ``False`` in its +configuration. If you do this, Django won't enable autocommit, and won't +perform any commits. You'll get the regular behavior of the underlying +database library. + +This requires you to commit explicitly every transaction, even those started +by Django or by third-party libraries. Thus, this is best used in situations +where you want to run your own transaction-controlling middleware or do +something really strange. + +.. versionchanged:: 1.6 + This used to be controlled by the ``TRANSACTIONS_MANAGED`` setting. + +Low-level APIs +============== + +.. warning:: + + Always prefer :func:`atomic` if possible at all. It accounts for the + idiosyncrasies of each database and prevents invalid operations. + + The low level APIs are only useful if you're implementing your own + transaction management. + +.. _managing-autocommit: + +Autocommit +---------- + +.. versionadded:: 1.6 + +Django provides a straightforward API to manage the autocommit state of each +database connection, if you need to. + +.. function:: get_autocommit(using=None) + +.. function:: set_autocommit(using=None, autocommit=True) + +These functions take a ``using`` argument which should be the name of a +database. If it isn't provided, Django uses the ``"default"`` database. + +Autocommit is initially turned on. If you turn it off, it's your +responsibility to restore it. + +Once you turn autocommit off, you get the default behavior of your database +adapter, and Django won't help you. Although that behavior is specified in +:pep:`249`, implementations of adapters aren't always consistent with one +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. + +Transactions +------------ + +A transaction is an atomic set of database queries. Even if your program +crashes, the database guarantees that either all the changes will be applied, +or none of them. + +Django doesn't provide an API to start a transaction. The expected way to +start a transaction is to disable autocommit with :func:`set_autocommit`. + +Once you're in a transaction, you can choose either to apply the changes +you've performed until this point with :func:`commit`, or to cancel them with +:func:`rollback`. + +.. function:: commit(using=None) + +.. function:: rollback(using=None) + +These functions take a ``using`` argument which should be the name of a +database. If it isn't provided, Django uses the ``"default"`` database. + +Django will refuse to commit or to rollback when an :func:`atomic` block is +active, because that would break atomicity. + .. _topics-db-transactions-savepoints: Savepoints -========== +---------- A savepoint is a marker within a transaction that enables you to roll back part of a transaction, rather than the full transaction. Savepoints are @@ -243,85 +364,6 @@ The following example demonstrates the use of savepoints:: transaction.savepoint_rollback(sid) # open transaction now contains only a.save() -Autocommit -========== - -.. _autocommit-details: - -Why Django uses autocommit --------------------------- - -In the SQL standards, each SQL query starts a transaction, unless one is -already in progress. Such transactions must then be committed or rolled back. - -This isn't always convenient for application developers. To alleviate this -problem, most databases provide an autocommit mode. When autocommit is turned -on, each SQL query is wrapped in its own transaction. In other words, the -transaction is not only automatically started, but also automatically -committed. - -:pep:`249`, the Python Database API Specification v2.0, requires autocommit to -be initially turned off. Django overrides this default and turns autocommit -on. - -To avoid this, you can :ref:`deactivate the transaction management -`, but it isn't recommended. - -.. versionchanged:: 1.6 - Before Django 1.6, autocommit was turned off, and it was emulated by - forcing a commit after write operations in the ORM. - -.. warning:: - - If you're using the database API directly — for instance, you're running - SQL queries with ``cursor.execute()`` — be aware that autocommit is on, - and consider wrapping your operations in a transaction, with - :func:`atomic`, to ensure consistency. - -.. _managing-autocommit: - -Managing autocommit -------------------- - -.. versionadded:: 1.6 - -Django provides a straightforward API to manage the autocommit state of each -database connection, if you need to. - -.. function:: get_autocommit(using=None) - -.. function:: set_autocommit(using=None, autocommit=True) - -These functions take a ``using`` argument which should be the name of a -database. If it isn't provided, Django uses the ``"default"`` database. - -Autocommit is initially turned on. If you turn it off, it's your -responsibility to restore it. - -: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. - -.. _deactivate-transaction-management: - -Deactivating transaction management ------------------------------------ - -You can totally disable Django's transaction management for a given database -by setting :setting:`AUTOCOMMIT ` to ``False`` in its -configuration. If you do this, Django won't enable autocommit, and won't -perform any commits. You'll get the regular behavior of the underlying -database library. - -This requires you to commit explicitly every transaction, even those started -by Django or by third-party libraries. Thus, this is best used in situations -where you want to run your own transaction-controlling middleware or do -something really strange. - -.. versionchanged:: 1.6 - This used to be controlled by the ``TRANSACTIONS_MANAGED`` setting. - - Database-specific notes ======================= From 189fb4e29463a2e1ee1a86a6c29c94f9f9904d6f Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Thu, 7 Mar 2013 23:06:58 +0100 Subject: [PATCH 24/32] Added a note about long-running processes. There isn't much else to say, really. --- docs/topics/db/transactions.txt | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/topics/db/transactions.txt b/docs/topics/db/transactions.txt index 4cecf896a4..8cbe0dccd0 100644 --- a/docs/topics/db/transactions.txt +++ b/docs/topics/db/transactions.txt @@ -164,6 +164,13 @@ Django provides a single API to control database transactions. - releases or rolls back to the savepoint when exiting an inner block; - commits or rolls back the transaction when exiting the outermost block. +.. admonition:: Performance considerations + + Open transactions have a performance cost for your database server. To + minimize this overhead, keep your transactions as short as possible. This + is especially important of you're using :func:`atomic` in long-running + processes, outside of Django's request / response cycle. + Autocommit ========== From 107d9b1d974ae97132751cb1db742fd8e930fb89 Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Fri, 8 Mar 2013 15:44:41 +0100 Subject: [PATCH 25/32] Added an option to disable the creation of savepoints in atomic. --- django/db/backends/__init__.py | 3 ++ django/db/transaction.py | 80 ++++++++++++++++++---------- docs/topics/db/transactions.txt | 10 +++- tests/transactions/tests.py | 93 +++++++++++++++++++++++++++++++++ 4 files changed, 156 insertions(+), 30 deletions(-) diff --git a/django/db/backends/__init__.py b/django/db/backends/__init__.py index 2cf75bd528..2d1de9509e 100644 --- a/django/db/backends/__init__.py +++ b/django/db/backends/__init__.py @@ -52,6 +52,9 @@ class BaseDatabaseWrapper(object): self._dirty = False # Tracks if the connection is in a transaction managed by 'atomic' self.in_atomic_block = False + # 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 diff --git a/django/db/transaction.py b/django/db/transaction.py index a6eb0662d4..be8981f968 100644 --- a/django/db/transaction.py +++ b/django/db/transaction.py @@ -188,8 +188,11 @@ class Atomic(object): __exit__ commits the transaction or releases the savepoint on normal exit, and rolls back the transaction or to the savepoint on exceptions. + It's possible to disable the creation of savepoints if the goal is to + ensure that some code runs within a transaction without creating overhead. + A stack of savepoints identifiers is maintained as an attribute of the - connection. None denotes a plain transaction. + connection. None denotes the absence of a savepoint. This allows reentrancy even if the same AtomicWrapper is reused. For example, it's possible to define `oa = @atomic('other')` and use `@ao` or @@ -198,8 +201,9 @@ class Atomic(object): Since database connections are thread-local, this is thread-safe. """ - def __init__(self, using): + def __init__(self, using, savepoint): self.using = using + self.savepoint = savepoint def _legacy_enter_transaction_management(self, connection): if not connection.in_atomic_block: @@ -228,9 +232,15 @@ class Atomic(object): "'atomic' cannot be used when autocommit is disabled.") if connection.in_atomic_block: - # We're already in a transaction; create a savepoint. - sid = connection.savepoint() - connection.savepoint_ids.append(sid) + # We're already in a transaction; create a savepoint, unless we + # were told not to or we're already waiting for a rollback. The + # second condition avoids creating useless savepoints and prevents + # overwriting needs_rollback until the rollback is performed. + if self.savepoint and not connection.needs_rollback: + sid = connection.savepoint() + connection.savepoint_ids.append(sid) + else: + connection.savepoint_ids.append(None) else: # We aren't in a transaction yet; create one. # The usual way to start a transaction is to turn autocommit off. @@ -244,13 +254,23 @@ class Atomic(object): else: connection.set_autocommit(False) connection.in_atomic_block = True - connection.savepoint_ids.append(None) + connection.needs_rollback = False def __exit__(self, exc_type, exc_value, traceback): connection = get_connection(self.using) - sid = connection.savepoint_ids.pop() - if exc_value is None: - if sid is None: + 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: + try: + connection.savepoint_commit(sid) + except DatabaseError: + connection.savepoint_rollback(sid) + # Remove this when the legacy transaction management goes away. + self._legacy_leave_transaction_management(connection) + raise + else: # Commit transaction connection.in_atomic_block = False try: @@ -265,17 +285,19 @@ class Atomic(object): connection.autocommit = True else: connection.set_autocommit(True) - else: - # Release savepoint - try: - connection.savepoint_commit(sid) - except DatabaseError: - connection.savepoint_rollback(sid) - # Remove this when the legacy transaction management goes away. - self._legacy_leave_transaction_management(connection) - raise else: - if sid is None: + # 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 + else: + connection.savepoint_rollback(sid) + else: # Roll back transaction connection.in_atomic_block = False try: @@ -285,9 +307,6 @@ class Atomic(object): connection.autocommit = True else: connection.set_autocommit(True) - else: - # Roll back to savepoint - connection.savepoint_rollback(sid) # Remove this when the legacy transaction management goes away. self._legacy_leave_transaction_management(connection) @@ -301,17 +320,17 @@ class Atomic(object): return inner -def atomic(using=None): +def atomic(using=None, savepoint=True): # Bare decorator: @atomic -- although the first argument is called # `using`, it's actually the function being decorated. if callable(using): - return Atomic(DEFAULT_DB_ALIAS)(using) + return Atomic(DEFAULT_DB_ALIAS, savepoint)(using) # Decorator: @atomic(...) or context manager: with atomic(...): ... else: - return Atomic(using) + return Atomic(using, savepoint) -def atomic_if_autocommit(using=None): +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. @@ -319,7 +338,7 @@ def atomic_if_autocommit(using=None): autocommit = get_connection(db).settings_dict['AUTOCOMMIT'] if autocommit: - return atomic(using) + return atomic(using, savepoint) else: # Bare decorator: @atomic_if_autocommit if callable(using): @@ -447,7 +466,7 @@ def commit_manually(using=None): return _transaction_func(entering, exiting, using) -def commit_on_success_unless_managed(using=None): +def commit_on_success_unless_managed(using=None, savepoint=False): """ Transitory API to preserve backwards-compatibility while refactoring. @@ -455,10 +474,13 @@ def commit_on_success_unless_managed(using=None): 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. + + 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) + return atomic_if_autocommit(using, savepoint) else: def entering(using): pass diff --git a/docs/topics/db/transactions.txt b/docs/topics/db/transactions.txt index 8cbe0dccd0..d5c22e17f5 100644 --- a/docs/topics/db/transactions.txt +++ b/docs/topics/db/transactions.txt @@ -89,7 +89,7 @@ Controlling transactions explicitly Django provides a single API to control database transactions. -.. function:: atomic(using=None) +.. function:: atomic(using=None, savepoint=True) This function creates an atomic block for writes to the database. (Atomicity is the defining property of database transactions.) @@ -164,6 +164,14 @@ Django provides a single API to control database transactions. - releases or rolls back to the savepoint when exiting an inner block; - commits or rolls back the transaction when exiting the outermost block. + You can disable the creation of savepoints for inner blocks by setting the + ``savepoint`` argument to ``False``. If an exception occurs, Django will + perform the rollback when exiting the first parent block with a savepoint + if there is one, and the outermost block otherwise. Atomicity is still + guaranteed by the outer transaction. This option should only be used if + the overhead of savepoints is noticeable. It has the drawback of breaking + the error handling described above. + .. admonition:: Performance considerations Open transactions have a performance cost for your database server. To diff --git a/tests/transactions/tests.py b/tests/transactions/tests.py index d6cfd8ae95..42a78ad4ba 100644 --- a/tests/transactions/tests.py +++ b/tests/transactions/tests.py @@ -106,6 +106,44 @@ class AtomicTests(TransactionTestCase): raise Exception("Oops, that's his first name") self.assertQuerysetEqual(Reporter.objects.all(), []) + def test_merged_commit_commit(self): + with transaction.atomic(): + Reporter.objects.create(first_name="Tintin") + with transaction.atomic(savepoint=False): + Reporter.objects.create(first_name="Archibald", last_name="Haddock") + self.assertQuerysetEqual(Reporter.objects.all(), + ['', '']) + + def test_merged_commit_rollback(self): + with transaction.atomic(): + Reporter.objects.create(first_name="Tintin") + with six.assertRaisesRegex(self, Exception, "Oops"): + with transaction.atomic(savepoint=False): + Reporter.objects.create(first_name="Haddock") + raise Exception("Oops, that's his last name") + # Writes in the outer block are rolled back too. + self.assertQuerysetEqual(Reporter.objects.all(), []) + + def test_merged_rollback_commit(self): + with six.assertRaisesRegex(self, Exception, "Oops"): + with transaction.atomic(): + Reporter.objects.create(last_name="Tintin") + with transaction.atomic(savepoint=False): + Reporter.objects.create(last_name="Haddock") + raise Exception("Oops, that's his first name") + self.assertQuerysetEqual(Reporter.objects.all(), []) + + def test_merged_rollback_rollback(self): + with six.assertRaisesRegex(self, Exception, "Oops"): + with transaction.atomic(): + Reporter.objects.create(last_name="Tintin") + with six.assertRaisesRegex(self, Exception, "Oops"): + with transaction.atomic(savepoint=False): + Reporter.objects.create(first_name="Haddock") + raise Exception("Oops, that's his last name") + raise Exception("Oops, that's his first name") + self.assertQuerysetEqual(Reporter.objects.all(), []) + def test_reuse_commit_commit(self): atomic = transaction.atomic() with atomic: @@ -171,6 +209,61 @@ class AtomicInsideLegacyTransactionManagementTests(AtomicTests): transaction.leave_transaction_management() +@skipUnless(connection.features.uses_savepoints, + "'atomic' requires transactions and savepoints.") +class AtomicMergeTests(TransactionTestCase): + """Test merging transactions with savepoint=False.""" + + def test_merged_outer_rollback(self): + with transaction.atomic(): + Reporter.objects.create(first_name="Tintin") + with transaction.atomic(savepoint=False): + Reporter.objects.create(first_name="Archibald", last_name="Haddock") + with six.assertRaisesRegex(self, Exception, "Oops"): + with transaction.atomic(savepoint=False): + Reporter.objects.create(first_name="Tournesol") + raise Exception("Oops, that's his last name") + # It wasn't possible to roll back + self.assertEqual(Reporter.objects.count(), 3) + # It wasn't possible to roll back + self.assertEqual(Reporter.objects.count(), 3) + # The outer block must roll back + self.assertQuerysetEqual(Reporter.objects.all(), []) + + def test_merged_inner_savepoint_rollback(self): + with transaction.atomic(): + Reporter.objects.create(first_name="Tintin") + with transaction.atomic(): + Reporter.objects.create(first_name="Archibald", last_name="Haddock") + with six.assertRaisesRegex(self, Exception, "Oops"): + with transaction.atomic(savepoint=False): + Reporter.objects.create(first_name="Tournesol") + raise Exception("Oops, that's his last name") + # It wasn't possible to roll back + self.assertEqual(Reporter.objects.count(), 3) + # The first block with a savepoint must roll back + self.assertEqual(Reporter.objects.count(), 1) + self.assertQuerysetEqual(Reporter.objects.all(), ['']) + + def test_merged_outer_rollback_after_inner_failure_and_inner_success(self): + with transaction.atomic(): + Reporter.objects.create(first_name="Tintin") + # Inner block without a savepoint fails + with six.assertRaisesRegex(self, Exception, "Oops"): + with transaction.atomic(savepoint=False): + Reporter.objects.create(first_name="Haddock") + raise Exception("Oops, that's his last name") + # It wasn't possible to roll back + self.assertEqual(Reporter.objects.count(), 2) + # Inner block with a savepoint succeeds + with transaction.atomic(savepoint=False): + Reporter.objects.create(first_name="Archibald", last_name="Haddock") + # It still wasn't possible to roll back + self.assertEqual(Reporter.objects.count(), 3) + # The outer block must rollback + self.assertQuerysetEqual(Reporter.objects.all(), []) + + @skipUnless(connection.features.uses_savepoints, "'atomic' requires transactions and savepoints.") class AtomicErrorsTests(TransactionTestCase): From 423c0d5e293bf6bd5f859430126269c31313585a Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Thu, 7 Mar 2013 14:47:50 +0100 Subject: [PATCH 26/32] Added a safety net for developers messing with autocommit. --- django/db/backends/__init__.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/django/db/backends/__init__.py b/django/db/backends/__init__.py index 2d1de9509e..cb17e1db61 100644 --- a/django/db/backends/__init__.py +++ b/django/db/backends/__init__.py @@ -417,12 +417,19 @@ class BaseDatabaseWrapper(object): or if it outlived its maximum age. """ if self.connection is not None: + # If the application didn't restore the original autocommit setting, + # don't take chances, drop the connection. + if self.autocommit != self.settings_dict['AUTOCOMMIT']: + self.close() + return + if self.errors_occurred: if self.is_usable(): self.errors_occurred = False else: self.close() return + if self.close_at is not None and time.time() >= self.close_at: self.close() return From 0cee3c0e43e20ca91aca18210c0b813c42407b1b Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Thu, 7 Mar 2013 17:20:15 +0100 Subject: [PATCH 27/32] Updated a test that doesn't make sense with autocommit. --- tests/transactions_regress/tests.py | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/tests/transactions_regress/tests.py b/tests/transactions_regress/tests.py index d5ee62da5e..142c09d3cf 100644 --- a/tests/transactions_regress/tests.py +++ b/tests/transactions_regress/tests.py @@ -4,7 +4,7 @@ from django.db import connection, connections, transaction, DEFAULT_DB_ALIAS, Da from django.db.transaction import commit_on_success, commit_manually, TransactionManagementError from django.test import TransactionTestCase, skipUnlessDBFeature from django.test.utils import override_settings -from django.utils.unittest import skipIf, skipUnless, expectedFailure +from django.utils.unittest import skipIf, skipUnless from transactions.tests import IgnorePendingDeprecationWarningsMixin @@ -187,22 +187,24 @@ class TestNewConnection(IgnorePendingDeprecationWarningsMixin, TransactionTestCa connections[DEFAULT_DB_ALIAS].close() connections[DEFAULT_DB_ALIAS] = self._old_backend - # TODO: update this test to account for database-level autocommit. - @expectedFailure def test_commit(self): """ Users are allowed to commit and rollback connections. """ - # The starting value is False, not None. - self.assertIs(connection._dirty, False) - list(Mod.objects.all()) - self.assertTrue(connection.is_dirty()) - connection.commit() - self.assertFalse(connection.is_dirty()) - list(Mod.objects.all()) - self.assertTrue(connection.is_dirty()) - connection.rollback() - self.assertFalse(connection.is_dirty()) + connection.set_autocommit(False) + try: + # The starting value is False, not None. + self.assertIs(connection._dirty, False) + list(Mod.objects.all()) + self.assertTrue(connection.is_dirty()) + connection.commit() + self.assertFalse(connection.is_dirty()) + list(Mod.objects.all()) + self.assertTrue(connection.is_dirty()) + connection.rollback() + self.assertFalse(connection.is_dirty()) + finally: + connection.set_autocommit(True) def test_enter_exit_management(self): orig_dirty = connection._dirty From 86fd920f6761cbf93a7e5a9eb3c634e0e168a038 Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Thu, 7 Mar 2013 17:20:50 +0100 Subject: [PATCH 28/32] Removed a test that no longer makes any sense. Since unmanaged == autocommit, there's nothing to commit or roll back. --- tests/middleware/tests.py | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/tests/middleware/tests.py b/tests/middleware/tests.py index 7e26037967..74b8ab9601 100644 --- a/tests/middleware/tests.py +++ b/tests/middleware/tests.py @@ -703,19 +703,6 @@ class TransactionMiddlewareTest(IgnorePendingDeprecationWarningsMixin, Transacti self.assertFalse(transaction.is_dirty()) self.assertEqual(Band.objects.count(), 1) - # TODO: update this test to account for database-level autocommit. - # Currently it fails under PostgreSQL because connections are never - # marked dirty in non-managed mode. - @expectedFailure - def test_unmanaged_response(self): - transaction.enter_transaction_management(False) - self.assertEqual(Band.objects.count(), 0) - TransactionMiddleware().process_response(self.request, self.response) - self.assertFalse(transaction.is_managed()) - # The transaction middleware doesn't commit/rollback if management - # has been disabled. - self.assertTrue(transaction.is_dirty()) - def test_exception(self): transaction.enter_transaction_management() Band.objects.create(name='The Beatles') From 4dbd1b2dd8d997f439b0116748994fd538ff893a Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Fri, 8 Mar 2013 11:35:54 +0100 Subject: [PATCH 29/32] Used commit_on_success_unless_managed to make ORM operations atomic. --- django/db/models/deletion.py | 82 +++++++++++++-------------------- django/db/models/query.py | 24 +--------- docs/topics/db/transactions.txt | 9 ++-- 3 files changed, 38 insertions(+), 77 deletions(-) diff --git a/django/db/models/deletion.py b/django/db/models/deletion.py index 27184c9350..a04f05c73b 100644 --- a/django/db/models/deletion.py +++ b/django/db/models/deletion.py @@ -50,24 +50,6 @@ def DO_NOTHING(collector, field, sub_objs, using): pass -def force_managed(func): - @wraps(func) - def decorated(self, *args, **kwargs): - if transaction.get_autocommit(using=self.using): - transaction.enter_transaction_management(using=self.using, forced=True) - forced_managed = True - else: - forced_managed = False - try: - func(self, *args, **kwargs) - if forced_managed: - transaction.commit(using=self.using) - finally: - if forced_managed: - transaction.leave_transaction_management(using=self.using) - return decorated - - class Collector(object): def __init__(self, using): self.using = using @@ -260,7 +242,6 @@ class Collector(object): self.data = SortedDict([(model, self.data[model]) for model in sorted_models]) - @force_managed def delete(self): # sort instance collections for model, instances in self.data.items(): @@ -271,40 +252,41 @@ class Collector(object): # end of a transaction. self.sort() - # send pre_delete signals - for model, obj in self.instances_with_model(): - if not model._meta.auto_created: - signals.pre_delete.send( - sender=model, instance=obj, using=self.using - ) - - # fast deletes - for qs in self.fast_deletes: - qs._raw_delete(using=self.using) - - # update fields - for model, instances_for_fieldvalues in six.iteritems(self.field_updates): - query = sql.UpdateQuery(model) - for (field, value), instances in six.iteritems(instances_for_fieldvalues): - query.update_batch([obj.pk for obj in instances], - {field.name: value}, self.using) - - # reverse instance collections - for instances in six.itervalues(self.data): - instances.reverse() - - # delete instances - for model, instances in six.iteritems(self.data): - query = sql.DeleteQuery(model) - pk_list = [obj.pk for obj in instances] - query.delete_batch(pk_list, self.using) - - if not model._meta.auto_created: - for obj in instances: - signals.post_delete.send( + with transaction.commit_on_success_unless_managed(using=self.using): + # send pre_delete signals + for model, obj in self.instances_with_model(): + if not model._meta.auto_created: + signals.pre_delete.send( sender=model, instance=obj, using=self.using ) + # fast deletes + for qs in self.fast_deletes: + qs._raw_delete(using=self.using) + + # update fields + for model, instances_for_fieldvalues in six.iteritems(self.field_updates): + query = sql.UpdateQuery(model) + for (field, value), instances in six.iteritems(instances_for_fieldvalues): + query.update_batch([obj.pk for obj in instances], + {field.name: value}, self.using) + + # reverse instance collections + for instances in six.itervalues(self.data): + instances.reverse() + + # delete instances + for model, instances in six.iteritems(self.data): + query = sql.DeleteQuery(model) + pk_list = [obj.pk for obj in instances] + query.delete_batch(pk_list, self.using) + + if not model._meta.auto_created: + for obj in instances: + signals.post_delete.send( + sender=model, instance=obj, using=self.using + ) + # update collected instances for model, instances_for_fieldvalues in six.iteritems(self.field_updates): for (field, value), instances in six.iteritems(instances_for_fieldvalues): diff --git a/django/db/models/query.py b/django/db/models/query.py index 834fe363b4..22c7cfba32 100644 --- a/django/db/models/query.py +++ b/django/db/models/query.py @@ -442,12 +442,7 @@ class QuerySet(object): self._for_write = True connection = connections[self.db] fields = self.model._meta.local_fields - if transaction.get_autocommit(using=self.db): - transaction.enter_transaction_management(using=self.db, forced=True) - forced_managed = True - else: - forced_managed = False - try: + with transaction.commit_on_success_unless_managed(using=self.db): if (connection.features.can_combine_inserts_with_and_without_auto_increment_pk and self.model._meta.has_auto_field): self._batched_insert(objs, fields, batch_size) @@ -458,11 +453,6 @@ class QuerySet(object): if objs_without_pk: fields= [f for f in fields if not isinstance(f, AutoField)] self._batched_insert(objs_without_pk, fields, batch_size) - if forced_managed: - transaction.commit(using=self.db) - finally: - if forced_managed: - transaction.leave_transaction_management(using=self.db) return objs @@ -579,18 +569,8 @@ class QuerySet(object): self._for_write = True query = self.query.clone(sql.UpdateQuery) query.add_update_values(kwargs) - if transaction.get_autocommit(using=self.db): - transaction.enter_transaction_management(using=self.db, forced=True) - forced_managed = True - else: - forced_managed = False - try: + with transaction.commit_on_success_unless_managed(using=self.db): rows = query.get_compiler(self.db).execute_sql(None) - if forced_managed: - transaction.commit(using=self.db) - finally: - if forced_managed: - transaction.leave_transaction_management(using=self.db) self._result_cache = None return rows update.alters_data = True diff --git a/docs/topics/db/transactions.txt b/docs/topics/db/transactions.txt index d5c22e17f5..b8fc0d4efa 100644 --- a/docs/topics/db/transactions.txt +++ b/docs/topics/db/transactions.txt @@ -16,11 +16,10 @@ Django's default behavior is to run in autocommit mode. Each query is immediately committed to the database. :ref:`See below for details `. -.. - Django uses transactions or savepoints automatically to guarantee the - integrity of ORM operations that require multiple queries, especially - :ref:`delete() ` and :ref:`update() - ` queries. +Django uses transactions or savepoints automatically to guarantee the +integrity of ORM operations that require multiple queries, especially +:ref:`delete() ` and :ref:`update() +` queries. .. versionchanged:: 1.6 Previous version of Django featured :ref:`a more complicated default From d04964e70d5c6277e79874ba8950e23c224b323b Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Fri, 8 Mar 2013 11:35:15 +0100 Subject: [PATCH 30/32] Used commit_on_success_unless_managed in loaddata. --- django/core/management/commands/loaddata.py | 69 +++++++-------------- 1 file changed, 21 insertions(+), 48 deletions(-) diff --git a/django/core/management/commands/loaddata.py b/django/core/management/commands/loaddata.py index 674e6be7b0..8aa40cf021 100644 --- a/django/core/management/commands/loaddata.py +++ b/django/core/management/commands/loaddata.py @@ -41,8 +41,6 @@ class Command(BaseCommand): self.ignore = options.get('ignore') self.using = options.get('database') - connection = connections[self.using] - if not len(fixture_labels): raise CommandError( "No database fixture specified. Please provide the path of at " @@ -51,13 +49,18 @@ class Command(BaseCommand): self.verbosity = int(options.get('verbosity')) - # commit is a stealth option - it isn't really useful as - # a command line option, but it can be useful when invoking - # loaddata from within another script. - # If commit=True, loaddata will use its own transaction; - # if commit=False, the data load SQL will become part of - # the transaction in place when loaddata was invoked. - commit = options.get('commit', True) + with transaction.commit_on_success_unless_managed(using=self.using): + self.loaddata(fixture_labels) + + # Close the DB connection -- unless we're still in a transaction. This + # is required as a workaround for an edge case in MySQL: if the same + # connection is used to create tables, load data, and query, the query + # can return incorrect results. See Django #7572, MySQL #37735. + if transaction.get_autocommit(self.using): + connections[self.using].close() + + def loaddata(self, fixture_labels): + connection = connections[self.using] # Keep a count of the installed objects and fixtures self.fixture_count = 0 @@ -65,16 +68,6 @@ class Command(BaseCommand): self.fixture_object_count = 0 self.models = set() - # Get a cursor (even though we don't need one yet). This has - # the side effect of initializing the test database (if - # it isn't already initialized). - cursor = connection.cursor() - - # Start transaction management. All fixtures are installed in a - # single transaction to ensure that all references are resolved. - if commit: - transaction.enter_transaction_management(using=self.using) - class SingleZipReader(zipfile.ZipFile): def __init__(self, *args, **kwargs): zipfile.ZipFile.__init__(self, *args, **kwargs) @@ -103,26 +96,17 @@ class Command(BaseCommand): app_fixtures = [os.path.join(os.path.dirname(path), 'fixtures') for path in app_module_paths] + with connection.constraint_checks_disabled(): + for fixture_label in fixture_labels: + self.load_label(fixture_label, app_fixtures) + + # Since we disabled constraint checks, we must manually check for + # any invalid keys that might have been added + table_names = [model._meta.db_table for model in self.models] try: - with connection.constraint_checks_disabled(): - for fixture_label in fixture_labels: - self.load_label(fixture_label, app_fixtures) - - # Since we disabled constraint checks, we must manually check for - # any invalid keys that might have been added - table_names = [model._meta.db_table for model in self.models] - try: - connection.check_constraints(table_names=table_names) - except Exception as e: - e.args = ("Problem installing fixtures: %s" % e,) - raise - - except (SystemExit, KeyboardInterrupt): - raise + connection.check_constraints(table_names=table_names) except Exception as e: - if commit: - transaction.rollback(using=self.using) - transaction.leave_transaction_management(using=self.using) + e.args = ("Problem installing fixtures: %s" % e,) raise # If we found even one object in a fixture, we need to reset the @@ -135,10 +119,6 @@ class Command(BaseCommand): for line in sequence_sql: cursor.execute(line) - if commit: - transaction.commit(using=self.using) - transaction.leave_transaction_management(using=self.using) - if self.verbosity >= 1: if self.fixture_object_count == self.loaded_object_count: self.stdout.write("Installed %d object(s) from %d fixture(s)" % ( @@ -147,13 +127,6 @@ class Command(BaseCommand): self.stdout.write("Installed %d object(s) (of %d) from %d fixture(s)" % ( self.loaded_object_count, self.fixture_object_count, self.fixture_count)) - # Close the DB connection. This is required as a workaround for an - # edge case in MySQL: if the same connection is used to - # create tables, load data, and query, the query can return - # incorrect results. See Django #7572, MySQL #37735. - if commit: - connection.close() - def load_label(self, fixture_label, app_fixtures): parts = fixture_label.split('.') From f32100939e8ea8a2714e45e22467af5df55c8f33 Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Fri, 8 Mar 2013 13:31:14 +0100 Subject: [PATCH 31/32] Abused atomic for transaction handling in TestCase. It isn't necessary to disable set_autocommit since its use is prohibited inside an atomic block. It's still necessary to disable the legacy transaction management methods for backwards compatibility, until they're removed. --- django/test/testcases.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/django/test/testcases.py b/django/test/testcases.py index 55673dca25..83762ddee4 100644 --- a/django/test/testcases.py +++ b/django/test/testcases.py @@ -63,7 +63,6 @@ def to_list(value): value = [value] return value -real_set_autocommit = transaction.set_autocommit real_commit = transaction.commit real_rollback = transaction.rollback real_enter_transaction_management = transaction.enter_transaction_management @@ -74,7 +73,6 @@ def nop(*args, **kwargs): return def disable_transaction_methods(): - transaction.set_autocommit = nop transaction.commit = nop transaction.rollback = nop transaction.enter_transaction_management = nop @@ -82,7 +80,6 @@ def disable_transaction_methods(): transaction.abort = nop def restore_transaction_methods(): - transaction.set_autocommit = real_set_autocommit transaction.commit = real_commit transaction.rollback = real_rollback transaction.enter_transaction_management = real_enter_transaction_management @@ -814,8 +811,11 @@ class TestCase(TransactionTestCase): assert not self.reset_sequences, 'reset_sequences cannot be used on TestCase instances' + self.atomics = {} for db_name in self._databases_names(): - transaction.enter_transaction_management(using=db_name) + self.atomics[db_name] = transaction.atomic(using=db_name) + self.atomics[db_name].__enter__() + # Remove this when the legacy transaction management goes away. disable_transaction_methods() from django.contrib.sites.models import Site @@ -835,10 +835,12 @@ class TestCase(TransactionTestCase): if not connections_support_transactions(): return super(TestCase, self)._fixture_teardown() + # Remove this when the legacy transaction management goes away. restore_transaction_methods() - for db in self._databases_names(): - transaction.rollback(using=db) - transaction.leave_transaction_management(using=db) + for db_name in reversed(self._databases_names()): + # Hack to force a rollback + connections[db_name].needs_rollback = True + self.atomics[db_name].__exit__(None, None, None) def _deferredSkip(condition, reason): From e654180ce2a11ef4c525497d6c40dc542e16806c Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Mon, 11 Mar 2013 15:10:58 +0100 Subject: [PATCH 32/32] Improved the API of set_autocommit. --- django/db/backends/__init__.py | 4 ++-- django/db/backends/creation.py | 2 +- django/db/transaction.py | 2 +- docs/topics/db/transactions.txt | 10 +++++----- tests/backends/tests.py | 12 ++++++------ tests/fixtures_model_package/tests.py | 8 ++++---- tests/fixtures_regress/tests.py | 4 ++-- tests/transactions/tests.py | 6 +++--- 8 files changed, 24 insertions(+), 24 deletions(-) diff --git a/django/db/backends/__init__.py b/django/db/backends/__init__.py index cb17e1db61..09a33eebae 100644 --- a/django/db/backends/__init__.py +++ b/django/db/backends/__init__.py @@ -108,7 +108,7 @@ class BaseDatabaseWrapper(object): self.connection = self.get_new_connection(conn_params) self.init_connection_state() if self.settings_dict['AUTOCOMMIT']: - self.set_autocommit() + self.set_autocommit(True) connection_created.send(sender=self.__class__, connection=self) def ensure_connection(self): @@ -314,7 +314,7 @@ class BaseDatabaseWrapper(object): if managed == self.autocommit: self.set_autocommit(not managed) - def set_autocommit(self, autocommit=True): + def set_autocommit(self, autocommit): """ Enable or disable autocommit. """ diff --git a/django/db/backends/creation.py b/django/db/backends/creation.py index c0d0a5958b..38c284d6d3 100644 --- a/django/db/backends/creation.py +++ b/django/db/backends/creation.py @@ -466,7 +466,7 @@ class BaseDatabaseCreation(object): warnings.warn( "set_autocommit was moved from BaseDatabaseCreation to " "BaseDatabaseWrapper.", PendingDeprecationWarning, stacklevel=2) - return self.connection.set_autocommit() + return self.connection.set_autocommit(True) def sql_table_creation_suffix(self): """ diff --git a/django/db/transaction.py b/django/db/transaction.py index be8981f968..3a4c3f2b8d 100644 --- a/django/db/transaction.py +++ b/django/db/transaction.py @@ -124,7 +124,7 @@ def get_autocommit(using=None): """ return get_connection(using).autocommit -def set_autocommit(using=None, autocommit=True): +def set_autocommit(autocommit, using=None): """ Set the autocommit status of the connection. """ diff --git a/docs/topics/db/transactions.txt b/docs/topics/db/transactions.txt index b8fc0d4efa..b8017e8bfa 100644 --- a/docs/topics/db/transactions.txt +++ b/docs/topics/db/transactions.txt @@ -255,7 +255,7 @@ database connection, if you need to. .. function:: get_autocommit(using=None) -.. function:: set_autocommit(using=None, autocommit=True) +.. function:: set_autocommit(autocommit, using=None) These functions take a ``using`` argument which should be the name of a database. If it isn't provided, Django uses the ``"default"`` database. @@ -600,11 +600,11 @@ To disable autocommit temporarily, instead of:: you should now use:: - transaction.set_autocommit(autocommit=False) + transaction.set_autocommit(False) try: # do stuff finally: - transaction.set_autocommit(autocommit=True) + transaction.set_autocommit(True) To enable autocommit temporarily, instead of:: @@ -613,11 +613,11 @@ To enable autocommit temporarily, instead of:: you should now use:: - transaction.set_autocommit(autocommit=True) + transaction.set_autocommit(True) try: # do stuff finally: - transaction.set_autocommit(autocommit=False) + transaction.set_autocommit(False) Disabling transaction management ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/tests/backends/tests.py b/tests/backends/tests.py index 51acbcb07f..c7f09013d4 100644 --- a/tests/backends/tests.py +++ b/tests/backends/tests.py @@ -522,7 +522,7 @@ class FkConstraintsTests(TransactionTestCase): """ When constraint checks are disabled, should be able to write bad data without IntegrityErrors. """ - transaction.set_autocommit(autocommit=False) + transaction.set_autocommit(False) try: # Create an Article. models.Article.objects.create(headline="Test article", pub_date=datetime.datetime(2010, 9, 4), reporter=self.r) @@ -538,13 +538,13 @@ class FkConstraintsTests(TransactionTestCase): finally: transaction.rollback() finally: - transaction.set_autocommit(autocommit=True) + transaction.set_autocommit(True) def test_disable_constraint_checks_context_manager(self): """ When constraint checks are disabled (using context manager), should be able to write bad data without IntegrityErrors. """ - transaction.set_autocommit(autocommit=False) + transaction.set_autocommit(False) try: # Create an Article. models.Article.objects.create(headline="Test article", pub_date=datetime.datetime(2010, 9, 4), reporter=self.r) @@ -559,14 +559,14 @@ class FkConstraintsTests(TransactionTestCase): finally: transaction.rollback() finally: - transaction.set_autocommit(autocommit=True) + transaction.set_autocommit(True) def test_check_constraints(self): """ Constraint checks should raise an IntegrityError when bad data is in the DB. """ try: - transaction.set_autocommit(autocommit=False) + transaction.set_autocommit(False) # Create an Article. models.Article.objects.create(headline="Test article", pub_date=datetime.datetime(2010, 9, 4), reporter=self.r) # Retrive it from the DB @@ -580,7 +580,7 @@ class FkConstraintsTests(TransactionTestCase): finally: transaction.rollback() finally: - transaction.set_autocommit(autocommit=True) + transaction.set_autocommit(True) class ThreadTests(TestCase): diff --git a/tests/fixtures_model_package/tests.py b/tests/fixtures_model_package/tests.py index 894a6c7fde..c250f647ce 100644 --- a/tests/fixtures_model_package/tests.py +++ b/tests/fixtures_model_package/tests.py @@ -25,7 +25,7 @@ class SampleTestCase(TestCase): class TestNoInitialDataLoading(TransactionTestCase): def test_syncdb(self): - transaction.set_autocommit(autocommit=False) + transaction.set_autocommit(False) try: Book.objects.all().delete() @@ -37,7 +37,7 @@ class TestNoInitialDataLoading(TransactionTestCase): self.assertQuerysetEqual(Book.objects.all(), []) transaction.rollback() finally: - transaction.set_autocommit(autocommit=True) + transaction.set_autocommit(True) def test_flush(self): @@ -49,7 +49,7 @@ class TestNoInitialDataLoading(TransactionTestCase): lambda a: a.name ) - transaction.set_autocommit(autocommit=False) + transaction.set_autocommit(False) try: management.call_command( 'flush', @@ -61,7 +61,7 @@ class TestNoInitialDataLoading(TransactionTestCase): self.assertQuerysetEqual(Book.objects.all(), []) transaction.rollback() finally: - transaction.set_autocommit(autocommit=True) + transaction.set_autocommit(True) class FixtureTestCase(TestCase): diff --git a/tests/fixtures_regress/tests.py b/tests/fixtures_regress/tests.py index f965dd81ac..c76056b93a 100644 --- a/tests/fixtures_regress/tests.py +++ b/tests/fixtures_regress/tests.py @@ -684,8 +684,8 @@ class TestTicket11101(TransactionTestCase): @skipUnlessDBFeature('supports_transactions') def test_ticket_11101(self): """Test that fixtures can be rolled back (ticket #11101).""" - transaction.set_autocommit(autocommit=False) + transaction.set_autocommit(False) try: self.ticket_11101() finally: - transaction.set_autocommit(autocommit=True) + transaction.set_autocommit(True) diff --git a/tests/transactions/tests.py b/tests/transactions/tests.py index 42a78ad4ba..e5a608e583 100644 --- a/tests/transactions/tests.py +++ b/tests/transactions/tests.py @@ -269,19 +269,19 @@ class AtomicMergeTests(TransactionTestCase): class AtomicErrorsTests(TransactionTestCase): def test_atomic_requires_autocommit(self): - transaction.set_autocommit(autocommit=False) + transaction.set_autocommit(False) try: with self.assertRaises(transaction.TransactionManagementError): with transaction.atomic(): pass finally: - transaction.set_autocommit(autocommit=True) + transaction.set_autocommit(True) def test_atomic_prevents_disabling_autocommit(self): autocommit = transaction.get_autocommit() with transaction.atomic(): with self.assertRaises(transaction.TransactionManagementError): - transaction.set_autocommit(autocommit=not autocommit) + transaction.set_autocommit(not autocommit) # Make sure autocommit wasn't changed. self.assertEqual(connection.autocommit, autocommit)