From 88786afbffc5c095f6491e080afea394f63bb44a Mon Sep 17 00:00:00 2001 From: Markus Holtermann Date: Tue, 13 Jan 2015 18:18:23 +0100 Subject: [PATCH] Fixed #24147 -- Prevented managers leaking model during migrations Thanks Tim Graham for the review. --- django/db/migrations/state.py | 18 +++++++++++++--- tests/migrations/test_state.py | 38 ++++++++++++++++++++++++++++++++++ 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):