Added some assertions to enforce the atomicity of atomic.

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:

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

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

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.

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
-<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
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~
 

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
-<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
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~
 

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
-<transactions-changes-from-1.5>` to determine if you need to make changes in
+<transactions-upgrading-from-1.5>` 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 <transactions-changes-from-1.5>`. You should review your code
+  management docs <transactions-upgrading-from-1.5>`. 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 <transactions-upgrading-from-1.5>`.
+
 Changes to :ttag:`cycle` and :ttag:`firstof`
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 

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):

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):

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)

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)

tests/transactions/tests.py

 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

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")