diff --git a/common/djangoapps/util/db.py b/common/djangoapps/util/db.py index bc1e02d4647d854a5908e02160d40adeb686fb01..3a1320db0d778c509ea70edec89bdc7cf7c8da3f 100644 --- a/common/djangoapps/util/db.py +++ b/common/djangoapps/util/db.py @@ -18,136 +18,6 @@ OUTER_ATOMIC_CACHE_NAME = 'db.outer_atomic' MYSQL_MAX_INT = (2 ** 31) - 1 -class CommitOnSuccessManager(object): - """ - This class implements the commit_on_success() API that was available till Django 1.5. - - An instance can be used either as a decorator or as a context manager. However, it - cannot be nested inside an atomic block. - - It is mostly taken from https://github.com/django/django/blob/1.8.5/django/db/transaction.py#L110-L277 - but instead of using save points commits all pending queries at the end of a block. - - The goal is to behave as close as possible to: - https://github.com/django/django/blob/1.4.22/django/db/transaction.py#L263-L289 - """ - - # Tests in TestCase subclasses are wrapped in an atomic block to speed up database restoration. - # So we must disabled this manager. - # https://github.com/django/django/blob/1.8.5/django/core/handlers/base.py#L129-L132 - ENABLED = True - - def __init__(self, using, read_committed=False): - self.using = using - self.read_committed = read_committed - - def __enter__(self): - - if not self.ENABLED: - return - - connection = transaction.get_connection(self.using) - - if connection.in_atomic_block: - raise transaction.TransactionManagementError('Cannot be inside an atomic block.') - - if getattr(connection, 'commit_on_success_block_level', 0) == 0: - connection.commit_on_success_block_level = 1 - - # This will set the transaction isolation level to READ COMMITTED for the next transaction. - if self.read_committed is True: - if connection.vendor == 'mysql': - cursor = connection.cursor() - cursor.execute("SET TRANSACTION ISOLATION LEVEL READ COMMITTED") - - # We aren't in a transaction yet; create one. - # The usual way to start a transaction is to turn autocommit off. - # However, some database adapters (namely sqlite3) don't handle - # transactions and savepoints properly when autocommit is off. - # In such cases, start an explicit transaction instead, which has - # the side-effect of disabling autocommit. - if connection.features.autocommits_when_autocommit_is_off: - connection._start_transaction_under_autocommit() # pylint: disable=protected-access - connection.autocommit = False - else: - connection.set_autocommit(False) - else: - if self.read_committed is True: - raise transaction.TransactionManagementError('Cannot change isolation level when nested.') - - connection.commit_on_success_block_level += 1 - - def __exit__(self, exc_type, exc_value, traceback): - - if not self.ENABLED: - return - - connection = transaction.get_connection(self.using) - - try: - if exc_type is None: - # Commit transaction - try: - connection.commit() - except DatabaseError: - try: - connection.rollback() - except Error: - # An error during rollback means that something - # went wrong with the connection. Drop it. - connection.close() - raise - else: - # Roll back transaction - try: - connection.rollback() - except Error: - # An error during rollback means that something - # went wrong with the connection. Drop it. - connection.close() - - finally: - connection.commit_on_success_block_level -= 1 - - # Outermost block exit when autocommit was enabled. - if connection.commit_on_success_block_level == 0: - if connection.features.autocommits_when_autocommit_is_off: - connection.autocommit = True - else: - connection.set_autocommit(True) - - def __call__(self, func): - @wraps(func) - def decorated(*args, **kwds): # pylint: disable=missing-docstring - with self: - return func(*args, **kwds) - return decorated - - -def commit_on_success(using=None, read_committed=False): - """ - This function implements the commit_on_success() API that was available till Django 1.5. - - It can be used either as a decorator or as a context manager. However, it - cannot be nested inside an atomic block. - - If the wrapped function or block returns a response the transaction is committed - and if it raises an exception the transaction is rolled back. - - Arguments: - using (str): the name of the database. - read_committed (bool): Whether to use read committed isolation level. - - Raises: - TransactionManagementError: if already inside an atomic block. - """ - if callable(using): - return CommitOnSuccessManager(DEFAULT_DB_ALIAS, read_committed)(using) - # Decorator: @commit_on_success(...) or context manager: with commit_on_success(...): ... - else: - return CommitOnSuccessManager(using, read_committed) - - @contextmanager def enable_named_outer_atomic(*names): """ diff --git a/common/djangoapps/util/testing.py b/common/djangoapps/util/testing.py index fc158fe85a42b38cc0ae747fd6bd0b8902931e83..33169cf90c07fb6cbc886e2135156e17f57cb083 100644 --- a/common/djangoapps/util/testing.py +++ b/common/djangoapps/util/testing.py @@ -12,7 +12,7 @@ from django.test import TestCase from django.urls import clear_url_caches, resolve from mock import patch -from util.db import CommitOnSuccessManager, OuterAtomic +from util.db import OuterAtomic if six.PY3: from importlib import reload # pylint: disable=no-name-in-module,redefined-builtin @@ -156,7 +156,6 @@ def patch_testcase(): """ Method that performs atomic-entering accounting. """ - CommitOnSuccessManager.ENABLED = False OuterAtomic.ALLOW_NESTED = True if not hasattr(OuterAtomic, 'atomic_for_testcase_calls'): OuterAtomic.atomic_for_testcase_calls = 0 @@ -174,7 +173,6 @@ def patch_testcase(): """ Method that performs atomic-rollback accounting. """ - CommitOnSuccessManager.ENABLED = True OuterAtomic.ALLOW_NESTED = False OuterAtomic.atomic_for_testcase_calls -= 1 return wrapped_func(*args, **kwargs) diff --git a/common/djangoapps/util/tests/test_db.py b/common/djangoapps/util/tests/test_db.py index 7e28152671210e1e8077c972791bfa9915ac452d..9c4e72ce62d18f6c2d20c4cf043a18f45d4f4410 100644 --- a/common/djangoapps/util/tests/test_db.py +++ b/common/djangoapps/util/tests/test_db.py @@ -15,7 +15,7 @@ from django.test.utils import override_settings from django.utils.six import StringIO from six.moves import range -from util.db import commit_on_success, enable_named_outer_atomic, generate_int_id, outer_atomic +from util.db import enable_named_outer_atomic, generate_int_id, outer_atomic def do_nothing(): @@ -26,7 +26,7 @@ def do_nothing(): @ddt.ddt class TransactionManagersTestCase(TransactionTestCase): """ - Tests commit_on_success and outer_atomic. + Tests outer_atomic. Note: This TestCase only works with MySQL. @@ -35,15 +35,11 @@ class TransactionManagersTestCase(TransactionTestCase): DECORATORS = { 'outer_atomic': outer_atomic(), 'outer_atomic_read_committed': outer_atomic(read_committed=True), - 'commit_on_success': commit_on_success(), - 'commit_on_success_read_committed': commit_on_success(read_committed=True), } @ddt.data( ('outer_atomic', IntegrityError, None, True), ('outer_atomic_read_committed', type(None), False, True), - ('commit_on_success', IntegrityError, None, True), - ('commit_on_success_read_committed', type(None), False, True), ) @ddt.unpack def test_concurrent_requests(self, transaction_decorator_name, exception_class, created_in_1, created_in_2): @@ -120,27 +116,6 @@ class TransactionManagersTestCase(TransactionTestCase): with outer_atomic(): outer_atomic()(do_nothing)() - def test_commit_on_success_nesting(self): - """ - Test that commit_on_success raises an error if it is nested inside - atomic or if the isolation level is changed when it is nested - inside another commit_on_success. - """ - # pylint: disable=not-callable - - if connection.vendor != 'mysql': - raise unittest.SkipTest('Only works on MySQL.') - - commit_on_success(read_committed=True)(do_nothing)() - - with self.assertRaisesRegex(TransactionManagementError, 'Cannot change isolation level when nested.'): - with commit_on_success(): - commit_on_success(read_committed=True)(do_nothing)() - - with self.assertRaisesRegex(TransactionManagementError, 'Cannot be inside an atomic block.'): - with atomic(): - commit_on_success(read_committed=True)(do_nothing)() - def test_named_outer_atomic_nesting(self): """ Test that a named outer_atomic raises an error only if nested in