diff --git a/django/db/migrations/state.py b/django/db/migrations/state.py index b5ce3e78c4..43454fd99c 100644 --- a/django/db/migrations/state.py +++ b/django/db/migrations/state.py @@ -143,6 +143,12 @@ class ModelState(object): # Sanity-check that fields is NOT a dict. It must be ordered. if isinstance(self.fields, dict): raise ValueError("ModelState.fields cannot be a dict - it must be a list of 2-tuples.") + # Sanity-check that fields are NOT already bound to a model. + for name, field in fields: + if hasattr(field, 'model'): + raise ValueError( + 'ModelState.fields cannot be bound to a model - "%s" is.' % name + ) @classmethod def from_model(cls, model): @@ -226,19 +232,19 @@ class ModelState(object): bases, ) - def clone(self): - "Returns an exact copy of this ModelState" - # We deep-clone the fields using deconstruction - fields = [] + def construct_fields(self): + "Deep-clone the fields using deconstruction" for name, field in self.fields: _, path, args, kwargs = field.deconstruct() field_class = import_string(path) - fields.append((name, field_class(*args, **kwargs))) - # Now make a copy + yield name, field_class(*args, **kwargs) + + def clone(self): + "Returns an exact copy of this ModelState" return self.__class__( app_label=self.app_label, name=self.name, - fields=fields, + fields=list(self.construct_fields()), options=dict(self.options), bases=self.bases, ) @@ -260,7 +266,7 @@ class ModelState(object): except LookupError: raise InvalidBasesError("Cannot resolve one or more bases from %r" % (self.bases,)) # Turn fields into a dict for the body, add other bits - body = dict(self.fields) + body = dict(self.construct_fields()) body['Meta'] = meta body['__module__'] = "__fake__" # Then, make a Model object diff --git a/tests/migrations/test_autodetector.py b/tests/migrations/test_autodetector.py index 88b67d2c23..1fdab21061 100644 --- a/tests/migrations/test_autodetector.py +++ b/tests/migrations/test_autodetector.py @@ -241,7 +241,7 @@ class AutodetectorTests(TestCase): action = migration.operations[0] self.assertEqual(action.__class__.__name__, "AlterField") self.assertEqual(action.name, "author") - self.assertEqual(action.field.rel.to.__name__, "Writer") + self.assertEqual(action.field.rel.to, "testapp.Writer") def test_rename_model_with_renamed_rel_field(self): """ @@ -278,7 +278,7 @@ class AutodetectorTests(TestCase): action = migration.operations[1] self.assertEqual(action.__class__.__name__, "AlterField") self.assertEqual(action.name, "writer") - self.assertEqual(action.field.rel.to.__name__, "Writer") + self.assertEqual(action.field.rel.to, "testapp.Writer") def test_fk_dependency(self): "Tests that having a ForeignKey automatically adds a dependency" diff --git a/tests/migrations/test_state.py b/tests/migrations/test_state.py index ab65630105..9be04f3dfc 100644 --- a/tests/migrations/test_state.py +++ b/tests/migrations/test_state.py @@ -356,7 +356,7 @@ class StateTests(TestCase): project_state = ProjectState() project_state.add_model_state(ModelState.from_model(TestModel)) with self.assertRaises(ValueError): - rendered_state = project_state.render() + project_state.render() # If we include the real app it should succeed project_state = ProjectState(real_apps=["contenttypes"]) @@ -372,3 +372,20 @@ class ModelStateTests(TestCase): def test_custom_model_base(self): state = ModelState.from_model(ModelWithCustomBase) self.assertEqual(state.bases, (models.Model,)) + + def test_bound_field_sanity_check(self): + field = models.CharField(max_length=1) + field.model = models.Model + with self.assertRaisesMessage(ValueError, + 'ModelState.fields cannot be bound to a model - "field" is.'): + ModelState('app', 'Model', [('field', field)]) + + def test_fields_immutability(self): + """ + Tests that rendering a model state doesn't alter its internal fields. + """ + apps = Apps() + field = models.CharField(max_length=1) + state = ModelState('app', 'Model', [('name', field)]) + Model = state.render(apps) + self.assertNotEqual(Model._meta.get_field('name'), field)