[1.7.x] Fixed #24110 -- Rewrote migration unapply to preserve intermediate states

Backport of fdc2cc948725866212a9bcc97b9b7cf21bb49b90 and be158e36251df0b07556657da47cdaf10913c57a from master

3 files changed, 45 insertions, 26 deletions

diff --git a/django/db/migrations/migration.py b/django/db/migrations/migration.py
index 80bdb2f126..79f6bfb4e5 100644
--- a/django/db/migrations/migration.py
+++ b/django/db/migrations/migration.py
@@ -115,29 +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 operation 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))
-            new_state = project_state.clone()
+            # 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.append((operation, project_state, new_state))
-            project_state = new_state
-        # Now run them in reverse
-        to_run.reverse()
+            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..cc824b0ef6 100644
--- a/docs/releases/1.7.3.txt
+++ b/docs/releases/1.7.3.txt
@@ -26,3 +26,6 @@ Bugfixes
   (:ticket:`24097`).
 
 * Added correct formats for Greek (``el``) (:ticket:`23967`).
+
+* Fixed a migration crash when unapplying a migration where multiple operations
+  interact with the same model (:ticket:`24110`).
diff --git a/tests/migrations/test_operations.py b/tests/migrations/test_operations.py
index ed4bf9423d..435a3f2668 100644
--- a/tests/migrations/test_operations.py
+++ b/tests/migrations/test_operations.py
@@ -413,27 +413,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: