From 563eaf04998da3ef88de4553f54eccd900a12805 Mon Sep 17 00:00:00 2001 From: valtron Date: Mon, 15 Sep 2014 10:17:28 -0600 Subject: [PATCH] [1.7.x] Fixed #23474 -- Prevented migrating backwards from unapplying the wrong migrations. Backport of abcf28a076 from master --- django/db/migrations/executor.py | 14 ++++--- docs/releases/1.7.1.txt | 3 ++ tests/migrations/test_executor.py | 38 +++++++++++++++++++ .../0001_initial.py | 8 ++++ .../0002_second.py | 9 +++++ .../__init__.py | 0 .../0001_initial.py | 16 ++++++++ .../__init__.py | 0 8 files changed, 83 insertions(+), 5 deletions(-) create mode 100644 tests/migrations/test_migrations_backwards_deps_1/0001_initial.py create mode 100644 tests/migrations/test_migrations_backwards_deps_1/0002_second.py create mode 100644 tests/migrations/test_migrations_backwards_deps_1/__init__.py create mode 100644 tests/migrations2/test_migrations_backwards_deps_2/0001_initial.py create mode 100644 tests/migrations2/test_migrations_backwards_deps_2/__init__.py diff --git a/django/db/migrations/executor.py b/django/db/migrations/executor.py index b6275690f91..503ccddc956 100644 --- a/django/db/migrations/executor.py +++ b/django/db/migrations/executor.py @@ -36,11 +36,15 @@ class MigrationExecutor(object): # If the migration is already applied, do backwards mode, # otherwise do forwards mode. elif target in applied: - backwards_plan = self.loader.graph.backwards_plan(target)[:-1] - # We only do this if the migration is not the most recent one - # in its app - that is, another migration with the same app - # label is in the backwards plan - if any(node[0] == target[0] for node in backwards_plan): + app_label = target[0] + next_migration_prefix = str(int(target[1][:4]) + 1) + try: + next_migration = self.loader.get_migration_by_prefix(app_label, next_migration_prefix) + except KeyError: + # If `target` is the most recent one in its app, there is nothing to do. + pass + else: + backwards_plan = self.loader.graph.backwards_plan(next_migration) for migration in backwards_plan: if migration in applied: plan.append((self.loader.graph.nodes[migration], True)) diff --git a/docs/releases/1.7.1.txt b/docs/releases/1.7.1.txt index f5f3f515a93..8438b7f6d28 100644 --- a/docs/releases/1.7.1.txt +++ b/docs/releases/1.7.1.txt @@ -46,3 +46,6 @@ Bugfixes * Allowed migrations to work with ``app_label``\s that have the same last part (e.g. ``django.contrib.auth`` and ``vendor.auth``) (:ticket:`23483`). + +* Fixed bug in migrations that could cause unexpected data loss when executing + a backwards or no-op migration (:ticket:`23474`). diff --git a/tests/migrations/test_executor.py b/tests/migrations/test_executor.py index d07eecfcef1..4631b805776 100644 --- a/tests/migrations/test_executor.py +++ b/tests/migrations/test_executor.py @@ -231,3 +231,41 @@ class ExecutorTests(MigrationTestBase): executor.migrate([("migrations", None)]) self.assertTableNotExists("migrations_author") self.assertTableNotExists("migrations_tribble") + + @override_settings( + MIGRATION_MODULES={ + "migrations": "migrations.test_migrations_backwards_deps_1", + "migrations2": "migrations2.test_migrations_backwards_deps_2", + }, + ) + def test_backwards_deps(self): + """ + #23474 - Migrating backwards shouldn't cause the wrong migrations to be + unapplied. + + Migration dependencies (x -> y === y depends on x): + m.0001 -+-> m.0002 + +-> m2.0001 + + 1) Migrate m2 to 0001, causing { m.0001, m2.0002 } to be applied. + 2) Migrate m to 0001. m.0001 has already been applied, so this should + be a noop. + """ + executor = MigrationExecutor(connection) + executor.migrate([("migrations2", "0001_initial")]) + try: + self.assertTableExists("migrations2_example") + # Rebuild the graph to reflect the new DB state + executor.loader.build_graph() + self.assertEqual( + executor.migration_plan([("migrations", "0001_initial")]), + [], + ) + executor.migrate([("migrations", "0001_initial")]) + self.assertTableExists("migrations2_example") + finally: + # And migrate back to clean up the database + executor.loader.build_graph() + executor.migrate([("migrations", None)]) + self.assertTableNotExists("migrations_author") + self.assertTableNotExists("migrations_tribble") diff --git a/tests/migrations/test_migrations_backwards_deps_1/0001_initial.py b/tests/migrations/test_migrations_backwards_deps_1/0001_initial.py new file mode 100644 index 00000000000..8e1925b4723 --- /dev/null +++ b/tests/migrations/test_migrations_backwards_deps_1/0001_initial.py @@ -0,0 +1,8 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + operations = [] diff --git a/tests/migrations/test_migrations_backwards_deps_1/0002_second.py b/tests/migrations/test_migrations_backwards_deps_1/0002_second.py new file mode 100644 index 00000000000..512f32e97c6 --- /dev/null +++ b/tests/migrations/test_migrations_backwards_deps_1/0002_second.py @@ -0,0 +1,9 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [('migrations', '0001_initial')] + operations = [] diff --git a/tests/migrations/test_migrations_backwards_deps_1/__init__.py b/tests/migrations/test_migrations_backwards_deps_1/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/migrations2/test_migrations_backwards_deps_2/0001_initial.py b/tests/migrations2/test_migrations_backwards_deps_2/0001_initial.py new file mode 100644 index 00000000000..0b6eafb02f9 --- /dev/null +++ b/tests/migrations2/test_migrations_backwards_deps_2/0001_initial.py @@ -0,0 +1,16 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [('migrations', '0001_initial')] + operations = [ + migrations.CreateModel( + "Example", + [ + ("id", models.AutoField(primary_key=True)), + ], + ), + ] diff --git a/tests/migrations2/test_migrations_backwards_deps_2/__init__.py b/tests/migrations2/test_migrations_backwards_deps_2/__init__.py new file mode 100644 index 00000000000..e69de29bb2d