From fdc2cc948725866212a9bcc97b9b7cf21bb49b90 Mon Sep 17 00:00:00 2001 From: Markus Holtermann Date: Sat, 10 Jan 2015 15:18:06 +0100 Subject: [PATCH] Fixed #24110 -- Rewrote migration unapply to preserve intermediate states --- django/db/migrations/migration.py | 41 ++++++++++++++++++----------- docs/releases/1.7.3.txt | 2 ++ tests/migrations/test_operations.py | 28 ++++++++++++-------- 3 files changed, 45 insertions(+), 26 deletions(-) diff --git a/django/db/migrations/migration.py b/django/db/migrations/migration.py index 168c43c565..061f37c391 100644 --- a/django/db/migrations/migration.py +++ b/django/db/migrations/migration.py @@ -115,28 +115,39 @@ class Migration(object): Takes a project_state representing all migrations prior to this one and a schema_editor for a live database and applies the migration in a reverse order. + + The backwards migration process consists of two phases: + + 1. The intermediate states from right before the first until right + after the last opertion inside this migration are preserved. + 2. The operations are applied in reverse order using the states + recorded in step 1. """ - # We need to pre-calculate the stack of project states + # Construct all the intermediate states we need for a reverse migration to_run = [] + new_state = project_state + # Phase 1 for operation in self.operations: - # If this operation cannot be represented as SQL, place a comment - # there instead - if collect_sql and not operation.reduces_to_sql: - schema_editor.collected_sql.append("--") - schema_editor.collected_sql.append("-- MIGRATION NOW PERFORMS OPERATION THAT CANNOT BE " - "WRITTEN AS SQL:") - schema_editor.collected_sql.append("-- %s" % operation.describe()) - schema_editor.collected_sql.append("--") - continue # If it's irreversible, error out if not operation.reversible: raise Migration.IrreversibleError("Operation %s in %s is not reversible" % (operation, self)) - old_state = project_state.clone() - operation.state_forwards(self.app_label, project_state) - to_run.append((operation, old_state, project_state)) - # Now run them in reverse - to_run.reverse() + # Preserve new state from previous run to not tamper the same state + # over all operations + new_state = new_state.clone() + old_state = new_state.clone() + operation.state_forwards(self.app_label, new_state) + to_run.insert(0, (operation, old_state, new_state)) + + # Phase 2 for operation, to_state, from_state in to_run: + if collect_sql: + if not operation.reduces_to_sql: + schema_editor.collected_sql.append("--") + schema_editor.collected_sql.append("-- MIGRATION NOW PERFORMS OPERATION THAT CANNOT BE " + "WRITTEN AS SQL:") + schema_editor.collected_sql.append("-- %s" % operation.describe()) + schema_editor.collected_sql.append("--") + continue if not schema_editor.connection.features.can_rollback_ddl and operation.atomic: # We're forcing a transaction on a non-transactional-DDL backend with atomic(schema_editor.connection.alias): diff --git a/docs/releases/1.7.3.txt b/docs/releases/1.7.3.txt index 3a651f0bd9..3b748b636a 100644 --- a/docs/releases/1.7.3.txt +++ b/docs/releases/1.7.3.txt @@ -26,3 +26,5 @@ Bugfixes (:ticket:`24097`). * Added correct formats for Greek (``el``) (:ticket:`23967`). + +* Fixed a migration crash when unapplying a migration (:ticket:`24110`). diff --git a/tests/migrations/test_operations.py b/tests/migrations/test_operations.py index 4f46d1a015..a491d9c280 100644 --- a/tests/migrations/test_operations.py +++ b/tests/migrations/test_operations.py @@ -465,27 +465,33 @@ class OperationTests(OperationTestBase): # Test the state alteration operation = migrations.RenameModel("Pony", "Horse") self.assertEqual(operation.describe(), "Rename model Pony to Horse") - new_state = project_state.clone() - operation.state_forwards("test_rnmo", new_state) - self.assertNotIn(("test_rnmo", "pony"), new_state.models) - self.assertIn(("test_rnmo", "horse"), new_state.models) - # Remember, RenameModel also repoints all incoming FKs and M2Ms - self.assertEqual("test_rnmo.Horse", new_state.models["test_rnmo", "rider"].fields[1][1].rel.to) - # Test the database alteration + # Test initial state and database + self.assertIn(("test_rnmo", "pony"), project_state.models) + self.assertNotIn(("test_rnmo", "horse"), project_state.models) self.assertTableExists("test_rnmo_pony") self.assertTableNotExists("test_rnmo_horse") if connection.features.supports_foreign_keys: self.assertFKExists("test_rnmo_rider", ["pony_id"], ("test_rnmo_pony", "id")) self.assertFKNotExists("test_rnmo_rider", ["pony_id"], ("test_rnmo_horse", "id")) - with connection.schema_editor() as editor: - operation.database_forwards("test_rnmo", editor, project_state, new_state) + # Migrate forwards + new_state = project_state.clone() + new_state = self.apply_operations("test_rnmo", new_state, [operation]) + # Test new state and database + self.assertNotIn(("test_rnmo", "pony"), new_state.models) + self.assertIn(("test_rnmo", "horse"), new_state.models) + # RenameModel also repoints all incoming FKs and M2Ms + self.assertEqual("test_rnmo.Horse", new_state.models["test_rnmo", "rider"].fields[1][1].rel.to) self.assertTableNotExists("test_rnmo_pony") self.assertTableExists("test_rnmo_horse") if connection.features.supports_foreign_keys: self.assertFKNotExists("test_rnmo_rider", ["pony_id"], ("test_rnmo_pony", "id")) self.assertFKExists("test_rnmo_rider", ["pony_id"], ("test_rnmo_horse", "id")) - # And test reversal - self.unapply_operations("test_rnmo", project_state, [operation]) + # Migrate backwards + original_state = self.unapply_operations("test_rnmo", project_state, [operation]) + # Test original state and database + self.assertIn(("test_rnmo", "pony"), original_state.models) + self.assertNotIn(("test_rnmo", "horse"), original_state.models) + self.assertEqual("Pony", original_state.models["test_rnmo", "rider"].fields[1][1].rel.to) self.assertTableExists("test_rnmo_pony") self.assertTableNotExists("test_rnmo_horse") if connection.features.supports_foreign_keys: