Fixed #24591 -- Optimized cloning of ModelState objects.

Changed ModelState.clone() to create a shallow copy of self.fields
and self.managers.
This commit is contained in:
Marten Kenbeek 2015-04-06 20:17:13 +02:00 committed by Tim Graham
parent c331eeb89c
commit 1a1f16d67d
3 changed files with 42 additions and 4 deletions

View File

@ -312,12 +312,23 @@ 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: for name, field in fields:
# Sanity-check that fields are NOT already bound to a model.
if hasattr(field, 'model'): if hasattr(field, 'model'):
raise ValueError( raise ValueError(
'ModelState.fields cannot be bound to a model - "%s" is.' % name 'ModelState.fields cannot be bound to a model - "%s" is.' % name
) )
# Sanity-check that relation fields are NOT referring to a model class.
if field.is_relation and hasattr(field.related_model, '_meta'):
raise ValueError(
'ModelState.fields cannot refer to a model class - "%s.to" does. '
'Use a string reference instead.' % name
)
if field.many_to_many and hasattr(field.remote_field.through, '_meta'):
raise ValueError(
'ModelState.fields cannot refer to a model class - "%s.through" does. '
'Use a string reference instead.' % name
)
@cached_property @cached_property
def name_lower(self): def name_lower(self):
@ -502,10 +513,10 @@ class ModelState(object):
return self.__class__( return self.__class__(
app_label=self.app_label, app_label=self.app_label,
name=self.name, name=self.name,
fields=list(self.construct_fields()), fields=list(self.fields),
options=dict(self.options), options=dict(self.options),
bases=self.bases, bases=self.bases,
managers=list(self.construct_managers()), managers=list(self.managers),
) )
def render(self, apps): def render(self, apps):

View File

@ -402,7 +402,8 @@ You can take this template and work from it, though we suggest looking at the
built-in Django operations in ``django.db.migrations.operations`` - they're built-in Django operations in ``django.db.migrations.operations`` - they're
easy to read and cover a lot of the example usage of semi-internal aspects easy to read and cover a lot of the example usage of semi-internal aspects
of the migration framework like ``ProjectState`` and the patterns used to get of the migration framework like ``ProjectState`` and the patterns used to get
historical models. historical models, as well as ``ModelState`` and the patterns used to mutate
historical models in ``state_forwards()``.
Some things to note: Some things to note:
@ -421,6 +422,16 @@ Some things to note:
operations; this is part of the autodetection code and does not matter for operations; this is part of the autodetection code and does not matter for
custom operations. custom operations.
.. warning::
For performance reasons, the :class:`~django.db.models.Field` instances in
``ModelState.fields`` are reused across migrations. You must never change
the attributes on these instances. If you need to mutate a field in
``state_forwards()``, you must remove the old instance from
``ModelState.fields`` and add a new instance in its place. The same is true
for the :class:`~django.db.models.Manager` instances in
``ModelState.managers``.
As a simple example, let's make an operation that loads PostgreSQL extensions As a simple example, let's make an operation that loads PostgreSQL extensions
(which contain some of PostgreSQL's more exciting features). It's simple enough; (which contain some of PostgreSQL's more exciting features). It's simple enough;
there's no model state changes, and all it does is run one command:: there's no model state changes, and all it does is run one command::

View File

@ -10,6 +10,7 @@ from django.test import SimpleTestCase, TestCase, override_settings
from .models import ( from .models import (
FoodManager, FoodQuerySet, ModelWithCustomBase, NoMigrationFoodManager, FoodManager, FoodQuerySet, ModelWithCustomBase, NoMigrationFoodManager,
UnicodeModel,
) )
@ -695,6 +696,21 @@ class ModelStateTests(TestCase):
'ModelState.fields cannot be bound to a model - "field" is.'): 'ModelState.fields cannot be bound to a model - "field" is.'):
ModelState('app', 'Model', [('field', field)]) ModelState('app', 'Model', [('field', field)])
def test_sanity_check_to(self):
field = models.ForeignKey(UnicodeModel)
with self.assertRaisesMessage(ValueError,
'ModelState.fields cannot refer to a model class - "field.to" does. '
'Use a string reference instead.'):
ModelState('app', 'Model', [('field', field)])
def test_sanity_check_through(self):
field = models.ManyToManyField('UnicodeModel')
field.remote_field.through = UnicodeModel
with self.assertRaisesMessage(ValueError,
'ModelState.fields cannot refer to a model class - "field.through" does. '
'Use a string reference instead.'):
ModelState('app', 'Model', [('field', field)])
def test_fields_immutability(self): def test_fields_immutability(self):
""" """
Tests that rendering a model state doesn't alter its internal fields. Tests that rendering a model state doesn't alter its internal fields.