Fixed #22659 -- Prevent model states from sharing field instances.

Thanks to Trac alias tbartelmess for the report and the test project.
This commit is contained in:
Simon Charette 2014-05-22 18:51:11 -04:00
parent f384b638e2
commit 7a38f88922
3 changed files with 34 additions and 11 deletions

View File

@ -143,6 +143,12 @@ class ModelState(object):
# Sanity-check that fields is NOT a dict. It must be ordered. # Sanity-check that fields is NOT a dict. It must be ordered.
if isinstance(self.fields, dict): if isinstance(self.fields, dict):
raise ValueError("ModelState.fields cannot be a dict - it must be a list of 2-tuples.") 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 @classmethod
def from_model(cls, model): def from_model(cls, model):
@ -226,19 +232,19 @@ class ModelState(object):
bases, bases,
) )
def clone(self): def construct_fields(self):
"Returns an exact copy of this ModelState" "Deep-clone the fields using deconstruction"
# We deep-clone the fields using deconstruction
fields = []
for name, field in self.fields: for name, field in self.fields:
_, path, args, kwargs = field.deconstruct() _, path, args, kwargs = field.deconstruct()
field_class = import_string(path) field_class = import_string(path)
fields.append((name, field_class(*args, **kwargs))) yield name, field_class(*args, **kwargs)
# Now make a copy
def clone(self):
"Returns an exact copy of this ModelState"
return self.__class__( return self.__class__(
app_label=self.app_label, app_label=self.app_label,
name=self.name, name=self.name,
fields=fields, fields=list(self.construct_fields()),
options=dict(self.options), options=dict(self.options),
bases=self.bases, bases=self.bases,
) )
@ -260,7 +266,7 @@ class ModelState(object):
except LookupError: except LookupError:
raise InvalidBasesError("Cannot resolve one or more bases from %r" % (self.bases,)) raise InvalidBasesError("Cannot resolve one or more bases from %r" % (self.bases,))
# Turn fields into a dict for the body, add other bits # Turn fields into a dict for the body, add other bits
body = dict(self.fields) body = dict(self.construct_fields())
body['Meta'] = meta body['Meta'] = meta
body['__module__'] = "__fake__" body['__module__'] = "__fake__"
# Then, make a Model object # Then, make a Model object

View File

@ -241,7 +241,7 @@ class AutodetectorTests(TestCase):
action = migration.operations[0] action = migration.operations[0]
self.assertEqual(action.__class__.__name__, "AlterField") self.assertEqual(action.__class__.__name__, "AlterField")
self.assertEqual(action.name, "author") 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): def test_rename_model_with_renamed_rel_field(self):
""" """
@ -278,7 +278,7 @@ class AutodetectorTests(TestCase):
action = migration.operations[1] action = migration.operations[1]
self.assertEqual(action.__class__.__name__, "AlterField") self.assertEqual(action.__class__.__name__, "AlterField")
self.assertEqual(action.name, "writer") 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): def test_fk_dependency(self):
"Tests that having a ForeignKey automatically adds a dependency" "Tests that having a ForeignKey automatically adds a dependency"

View File

@ -356,7 +356,7 @@ class StateTests(TestCase):
project_state = ProjectState() project_state = ProjectState()
project_state.add_model_state(ModelState.from_model(TestModel)) project_state.add_model_state(ModelState.from_model(TestModel))
with self.assertRaises(ValueError): with self.assertRaises(ValueError):
rendered_state = project_state.render() project_state.render()
# If we include the real app it should succeed # If we include the real app it should succeed
project_state = ProjectState(real_apps=["contenttypes"]) project_state = ProjectState(real_apps=["contenttypes"])
@ -372,3 +372,20 @@ class ModelStateTests(TestCase):
def test_custom_model_base(self): def test_custom_model_base(self):
state = ModelState.from_model(ModelWithCustomBase) state = ModelState.from_model(ModelWithCustomBase)
self.assertEqual(state.bases, (models.Model,)) 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)