From 7937cc16f5fafb33a53b9a9ff286229fcc5ba67f Mon Sep 17 00:00:00 2001 From: Evan Grim Date: Tue, 11 Jul 2017 11:40:18 -0500 Subject: [PATCH] Fixed #28386 -- Made operations within non-atomic migrations honor the operation's atomic flag when migrating backwards. --- AUTHORS | 1 + django/db/migrations/migration.py | 6 ++++-- tests/migrations/test_executor.py | 8 ++++++++ .../test_migrations_atomic_operation/0001_initial.py | 2 +- tests/migrations/test_operations.py | 2 ++ 5 files changed, 16 insertions(+), 3 deletions(-) diff --git a/AUTHORS b/AUTHORS index 8fbf94582c..0f20d4ff6f 100644 --- a/AUTHORS +++ b/AUTHORS @@ -254,6 +254,7 @@ answer newbie questions, and generally made Django that much better: Esdras Beleza Espen Grindhaug Eugene Lazutkin + Evan Grim Fabrice Aneche favo@exoweb.net fdr diff --git a/django/db/migrations/migration.py b/django/db/migrations/migration.py index 3a41703ef8..ffe0b1fb3d 100644 --- a/django/db/migrations/migration.py +++ b/django/db/migrations/migration.py @@ -162,8 +162,10 @@ class Migration: schema_editor.collected_sql.append("--") if not operation.reduces_to_sql: continue - if not schema_editor.connection.features.can_rollback_ddl and operation.atomic: - # We're forcing a transaction on a non-transactional-DDL backend + atomic_operation = operation.atomic or (self.atomic and operation.atomic is not False) + if not schema_editor.atomic_migration and atomic_operation: + # Force a transaction on a non-transactional-DDL backend or an + # atomic operation inside a non-atomic migration. with atomic(schema_editor.connection.alias): operation.database_backwards(self.app_label, schema_editor, from_state, to_state) else: diff --git a/tests/migrations/test_executor.py b/tests/migrations/test_executor.py index d6cc802149..fdbc724faf 100644 --- a/tests/migrations/test_executor.py +++ b/tests/migrations/test_executor.py @@ -126,6 +126,14 @@ class ExecutorTests(MigrationTestBase): migrations_apps = executor.loader.project_state(("migrations", "0001_initial")).apps Editor = migrations_apps.get_model("migrations", "Editor") self.assertFalse(Editor.objects.exists()) + # Record previous migration as successful. + executor.migrate([("migrations", "0001_initial")], fake=True) + # Rebuild the graph to reflect the new DB state. + executor.loader.build_graph() + # Migrating backwards is also atomic. + with self.assertRaisesMessage(RuntimeError, "Abort migration"): + executor.migrate([("migrations", None)]) + self.assertFalse(Editor.objects.exists()) @override_settings(MIGRATION_MODULES={ "migrations": "migrations.test_migrations", diff --git a/tests/migrations/test_migrations_atomic_operation/0001_initial.py b/tests/migrations/test_migrations_atomic_operation/0001_initial.py index 415186423f..47ae3df5e9 100644 --- a/tests/migrations/test_migrations_atomic_operation/0001_initial.py +++ b/tests/migrations/test_migrations_atomic_operation/0001_initial.py @@ -18,5 +18,5 @@ class Migration(migrations.Migration): ("name", models.CharField(primary_key=True, max_length=255)), ], ), - migrations.RunPython(raise_error, atomic=True), + migrations.RunPython(raise_error, reverse_code=raise_error, atomic=True), ] diff --git a/tests/migrations/test_operations.py b/tests/migrations/test_operations.py index 86dbb84411..13b511f3aa 100644 --- a/tests/migrations/test_operations.py +++ b/tests/migrations/test_operations.py @@ -2055,9 +2055,11 @@ class OperationTests(OperationTestBase): with self.assertRaises(ValueError): with connection.schema_editor() as editor: atomic_migration.unapply(project_state, editor) + self.assertEqual(project_state.apps.get_model("test_runpythonatomic", "Pony").objects.count(), 0) with self.assertRaises(ValueError): with connection.schema_editor() as editor: non_atomic_migration.unapply(project_state, editor) + self.assertEqual(project_state.apps.get_model("test_runpythonatomic", "Pony").objects.count(), 1) # Verify deconstruction. definition = non_atomic_migration.operations[0].deconstruct() self.assertEqual(definition[0], "RunPython")