Fixed #24110 -- Rewrote migration unapply to preserve intermediate states

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:
+            # If it's irreversible, error out
+            if not operation.reversible:
+                raise Migration.IrreversibleError("Operation %s in %s is not reversible" % (operation, self))
+            # 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 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()
-        for operation, to_state, from_state in to_run:
             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):

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`).

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: