Fixed #24147 -- Prevented managers leaking model during migrations

Thanks Tim Graham for the review.

2 files changed, 53 insertions, 3 deletions

diff --git a/django/db/migrations/state.py b/django/db/migrations/state.py
index 84b8c0dbe9..7eb1e5b8b9 100644
--- a/django/db/migrations/state.py
+++ b/django/db/migrations/state.py
@@ -400,6 +400,19 @@ class ModelState(object):
             field_class = import_string(path)
             yield name, field_class(*args, **kwargs)
 
+    def construct_managers(self):
+        "Deep-clone the managers using deconstruction"
+        # Sort all managers by their creation counter
+        sorted_managers = sorted(self.managers, key=lambda v: v[1].creation_counter)
+        for mgr_name, manager in sorted_managers:
+            as_manager, manager_path, qs_path, args, kwargs = manager.deconstruct()
+            if as_manager:
+                qs_class = import_string(qs_path)
+                yield mgr_name, qs_class.as_manager()
+            else:
+                manager_class = import_string(manager_path)
+                yield mgr_name, manager_class(*args, **kwargs)
+
     def clone(self):
         "Returns an exact copy of this ModelState"
         return self.__class__(
@@ -408,7 +421,7 @@ class ModelState(object):
             fields=list(self.construct_fields()),
             options=dict(self.options),
             bases=self.bases,
-            managers=self.managers,
+            managers=list(self.construct_managers()),
         )
 
     def render(self, apps):
@@ -431,8 +444,7 @@ class ModelState(object):
         body['__module__'] = "__fake__"
 
         # Restore managers
-        for mgr_name, manager in self.managers:
-            body[mgr_name] = manager
+        body.update(self.construct_managers())
 
         # Then, make a Model object (apps.register_model is called in __new__)
         return type(
diff --git a/tests/migrations/test_state.py b/tests/migrations/test_state.py
index e069649810..6410477b36 100644
--- a/tests/migrations/test_state.py
+++ b/tests/migrations/test_state.py
@@ -1,5 +1,6 @@
 from django.apps.registry import Apps
 from django.db import models
+from django.db.migrations.operations import RemoveField
 from django.db.migrations.state import ProjectState, ModelState, InvalidBasesError
 from django.test import TestCase
 
@@ -505,6 +506,43 @@ class StateTests(TestCase):
             ["id", "author"],
         )
 
+    def test_manager_refer_correct_model_version(self):
+        """
+        #24147 - Tests that managers refer to the correct version of a
+        historical model
+        """
+        project_state = ProjectState()
+        project_state.add_model(ModelState(
+            app_label="migrations",
+            name="Tag",
+            fields=[
+                ("id", models.AutoField(primary_key=True)),
+                ("hidden", models.BooleanField()),
+            ],
+            managers=[
+                ('food_mgr', FoodManager('a', 'b')),
+                ('food_qs', FoodQuerySet.as_manager()),
+            ]
+        ))
+
+        old_model = project_state.apps.get_model('migrations', 'tag')
+
+        new_state = project_state.clone()
+        operation = RemoveField("tag", "hidden")
+        operation.state_forwards("migrations", new_state)
+
+        new_model = new_state.apps.get_model('migrations', 'tag')
+
+        self.assertIsNot(old_model, new_model)
+        self.assertIs(old_model, old_model.food_mgr.model)
+        self.assertIs(old_model, old_model.food_qs.model)
+        self.assertIs(new_model, new_model.food_mgr.model)
+        self.assertIs(new_model, new_model.food_qs.model)
+        self.assertIsNot(old_model.food_mgr, new_model.food_mgr)
+        self.assertIsNot(old_model.food_qs, new_model.food_qs)
+        self.assertIsNot(old_model.food_mgr.model, new_model.food_mgr.model)
+        self.assertIsNot(old_model.food_qs.model, new_model.food_qs.model)
+
 
 class ModelStateTests(TestCase):
     def test_custom_model_base(self):