From 0d74bdaf0c39feb8ec303dbbdbcadba70e46eecb Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Mon, 30 Sep 2013 10:14:22 +0200 Subject: Fixed #21134 -- Prevented queries in broken transactions. Backport of 728548e4 from master. Squashed commit of the following: commit 63ddb271a44df389b2c302e421fc17b7f0529755 Author: Aymeric Augustin Date: Sun Sep 29 22:51:00 2013 +0200 Clarified interactions between atomic and exceptions. commit 2899ec299228217c876ba3aa4024e523a41c8504 Author: Aymeric Augustin Date: Sun Sep 22 22:45:32 2013 +0200 Fixed TransactionManagementError in tests. Previous commit introduced an additional check to prevent running queries in transactions that will be rolled back, which triggered a few failures in the tests. In practice using transaction.atomic instead of the low-level savepoint APIs was enough to fix the problems. commit 4a639b059ea80aeb78f7f160a7d4b9f609b9c238 Author: Aymeric Augustin Date: Tue Sep 24 22:24:17 2013 +0200 Allowed nesting constraint_checks_disabled inside atomic. Since MySQL handles transactions loosely, this isn't a problem. commit 2a4ab1cb6e83391ff7e25d08479e230ca564bfef Author: Aymeric Augustin Date: Sat Sep 21 18:43:12 2013 +0200 Prevented running queries in transactions that will be rolled back. This avoids a counter-intuitive behavior in an edge case on databases with non-atomic transaction semantics. It prevents using savepoint_rollback() inside an atomic block without calling set_rollback(False) first, which is backwards-incompatible in tests. Refs #21134. commit 8e3db393853c7ac64a445b66e57f3620a3fde7b0 Author: Aymeric Augustin Date: Sun Sep 22 22:14:17 2013 +0200 Replaced manual savepoints by atomic blocks. This ensures the rollback flag is handled consistently in internal APIs. --- django/contrib/sessions/backends/db.py | 5 ++-- django/db/backends/__init__.py | 9 +++++++ django/db/backends/mysql/base.py | 9 ++++++- django/db/backends/oracle/base.py | 1 + django/db/backends/sqlite3/base.py | 1 + django/db/backends/util.py | 47 +++++++++++++++++++++++----------- django/db/models/query.py | 6 ++--- django/db/transaction.py | 9 ++++--- 8 files changed, 60 insertions(+), 27 deletions(-) (limited to 'django') diff --git a/django/contrib/sessions/backends/db.py b/django/contrib/sessions/backends/db.py index 206fca2700..7be99c3e16 100644 --- a/django/contrib/sessions/backends/db.py +++ b/django/contrib/sessions/backends/db.py @@ -58,12 +58,11 @@ class SessionStore(SessionBase): expire_date=self.get_expiry_date() ) using = router.db_for_write(Session, instance=obj) - sid = transaction.savepoint(using=using) try: - obj.save(force_insert=must_create, using=using) + with transaction.atomic(using=using): + obj.save(force_insert=must_create, using=using) except IntegrityError: if must_create: - transaction.savepoint_rollback(sid, using=using) raise CreateError raise diff --git a/django/db/backends/__init__.py b/django/db/backends/__init__.py index f06682f630..55ed1f0a16 100644 --- a/django/db/backends/__init__.py +++ b/django/db/backends/__init__.py @@ -359,6 +359,12 @@ class BaseDatabaseWrapper(object): raise TransactionManagementError( "This is forbidden when an 'atomic' block is active.") + def validate_no_broken_transaction(self): + if self.needs_rollback: + raise TransactionManagementError( + "An error occurred in the current transaction. You can't " + "execute queries until the end of the 'atomic' block.") + def abort(self): """ Roll back any ongoing transaction and clean the transaction state @@ -626,6 +632,9 @@ class BaseDatabaseFeatures(object): # when autocommit is disabled? http://bugs.python.org/issue8145#msg109965 autocommits_when_autocommit_is_off = False + # Does the backend prevent running SQL queries in broken transactions? + atomic_transactions = True + # Does the backend support 'pyformat' style ("... %(name)s ...", {'name': value}) # parameter passing? Note this can be provided by the backend even if not # supported by the Python driver diff --git a/django/db/backends/mysql/base.py b/django/db/backends/mysql/base.py index fa64175ffc..d760418aaf 100644 --- a/django/db/backends/mysql/base.py +++ b/django/db/backends/mysql/base.py @@ -166,6 +166,7 @@ class DatabaseFeatures(BaseDatabaseFeatures): requires_explicit_null_ordering_when_grouping = True allows_primary_key_0 = False uses_savepoints = True + atomic_transactions = False def __init__(self, connection): super(DatabaseFeatures, self).__init__(connection) @@ -470,7 +471,13 @@ class DatabaseWrapper(BaseDatabaseWrapper): """ Re-enable foreign key checks after they have been disabled. """ - self.cursor().execute('SET foreign_key_checks=1') + # Override needs_rollback in case constraint_checks_disabled is + # nested inside transaction.atomic. + self.needs_rollback, needs_rollback = False, self.needs_rollback + try: + self.cursor().execute('SET foreign_key_checks=1') + finally: + self.needs_rollback = needs_rollback def check_constraints(self, table_names=None): """ diff --git a/django/db/backends/oracle/base.py b/django/db/backends/oracle/base.py index 9e7dd03fc2..46f89ffd2f 100644 --- a/django/db/backends/oracle/base.py +++ b/django/db/backends/oracle/base.py @@ -89,6 +89,7 @@ class DatabaseFeatures(BaseDatabaseFeatures): has_bulk_insert = True supports_tablespaces = True supports_sequence_reset = False + atomic_transactions = False class DatabaseOperations(BaseDatabaseOperations): compiler_module = "django.db.backends.oracle.compiler" diff --git a/django/db/backends/sqlite3/base.py b/django/db/backends/sqlite3/base.py index 2255cc2a8c..89c9ef1afa 100644 --- a/django/db/backends/sqlite3/base.py +++ b/django/db/backends/sqlite3/base.py @@ -101,6 +101,7 @@ class DatabaseFeatures(BaseDatabaseFeatures): has_bulk_insert = True can_combine_inserts_with_and_without_auto_increment_pk = False autocommits_when_autocommit_is_off = True + atomic_transactions = False supports_paramstyle_pyformat = False @cached_property diff --git a/django/db/backends/util.py b/django/db/backends/util.py index 43ceff095b..2820007b1c 100644 --- a/django/db/backends/util.py +++ b/django/db/backends/util.py @@ -19,14 +19,9 @@ class CursorWrapper(object): self.cursor = cursor self.db = db - SET_DIRTY_ATTRS = frozenset(['execute', 'executemany', 'callproc']) - WRAP_ERROR_ATTRS = frozenset([ - 'callproc', 'close', 'execute', 'executemany', - 'fetchone', 'fetchmany', 'fetchall', 'nextset']) + WRAP_ERROR_ATTRS = frozenset(['fetchone', 'fetchmany', 'fetchall', 'nextset']) def __getattr__(self, attr): - if attr in CursorWrapper.SET_DIRTY_ATTRS: - self.db.set_dirty() cursor_attr = getattr(self.cursor, attr) if attr in CursorWrapper.WRAP_ERROR_ATTRS: return self.db.wrap_database_errors(cursor_attr) @@ -36,18 +31,42 @@ class CursorWrapper(object): def __iter__(self): return iter(self.cursor) + # The following methods cannot be implemented in __getattr__, because the + # code must run when the method is invoked, not just when it is accessed. -class CursorDebugWrapper(CursorWrapper): + def callproc(self, procname, params=None): + self.db.validate_no_broken_transaction() + self.db.set_dirty() + with self.db.wrap_database_errors: + if params is None: + return self.cursor.callproc(procname) + else: + return self.cursor.callproc(procname, params) def execute(self, sql, params=None): + self.db.validate_no_broken_transaction() self.db.set_dirty() + with self.db.wrap_database_errors: + if params is None: + return self.cursor.execute(sql) + else: + return self.cursor.execute(sql, params) + + def executemany(self, sql, param_list): + self.db.validate_no_broken_transaction() + self.db.set_dirty() + with self.db.wrap_database_errors: + return self.cursor.executemany(sql, param_list) + + +class CursorDebugWrapper(CursorWrapper): + + # XXX callproc isn't instrumented at this time. + + def execute(self, sql, params=None): start = time() try: - with self.db.wrap_database_errors: - if params is None: - # params default might be backend specific - return self.cursor.execute(sql) - return self.cursor.execute(sql, params) + return super(CursorDebugWrapper, self).execute(sql, params) finally: stop = time() duration = stop - start @@ -61,11 +80,9 @@ class CursorDebugWrapper(CursorWrapper): ) def executemany(self, sql, param_list): - self.db.set_dirty() start = time() try: - with self.db.wrap_database_errors: - return self.cursor.executemany(sql, param_list) + return super(CursorDebugWrapper, self).executemany(sql, param_list) finally: stop = time() duration = stop - start diff --git a/django/db/models/query.py b/django/db/models/query.py index 31b79ed0a2..1075407ae3 100644 --- a/django/db/models/query.py +++ b/django/db/models/query.py @@ -376,12 +376,10 @@ class QuerySet(object): params = dict((k, v) for k, v in kwargs.items() if LOOKUP_SEP not in k) params.update(defaults) obj = self.model(**params) - sid = transaction.savepoint(using=self.db) - obj.save(force_insert=True, using=self.db) - transaction.savepoint_commit(sid, using=self.db) + with transaction.atomic(using=self.db): + obj.save(force_insert=True, using=self.db) return obj, True except DatabaseError: - transaction.savepoint_rollback(sid, using=self.db) exc_info = sys.exc_info() try: return self.get(**lookup), False diff --git a/django/db/transaction.py b/django/db/transaction.py index 15da007ec9..2639569122 100644 --- a/django/db/transaction.py +++ b/django/db/transaction.py @@ -16,14 +16,15 @@ import warnings from functools import wraps -from django.db import connections, DatabaseError, DEFAULT_DB_ALIAS +from django.db import ( + connections, DEFAULT_DB_ALIAS, + DatabaseError, ProgrammingError) from django.utils.decorators import available_attrs -class TransactionManagementError(Exception): +class TransactionManagementError(ProgrammingError): """ - This exception is thrown when something bad happens with transaction - management. + This exception is thrown when transaction management is used improperly. """ pass -- cgit v1.3