Fixed #24215 -- Refactored lazy model operations

This adds a new method, Apps.lazy_model_operation(), and a helper function,
lazy_related_operation(), which together supersede add_lazy_relation() and
make lazy model operations the responsibility of the App registry. This
system no longer uses the class_prepared signal.
This commit is contained in:
Alex Hill 2015-03-03 16:43:56 +08:00 committed by Tim Graham
parent 0f6f80c2e7
commit 720ff740e7
10 changed files with 231 additions and 165 deletions

View File

@ -2,6 +2,7 @@ import sys
import threading
import warnings
from collections import Counter, OrderedDict, defaultdict
from functools import partial
from django.core.exceptions import AppRegistryNotReady, ImproperlyConfigured
from django.utils import lru_cache
@ -45,8 +46,10 @@ class Apps(object):
# Lock for thread-safe population.
self._lock = threading.Lock()
# Pending lookups for lazy relations.
self._pending_lookups = {}
# Maps ("app_label", "modelname") tuples to lists of functions to be
# called when the corresponding model is ready. Used by this class's
# `lazy_model_operation()` and `do_pending_operations()` methods.
self._pending_operations = defaultdict(list)
# Populate apps and models, unless it's the master registry.
if installed_apps is not None:
@ -207,6 +210,7 @@ class Apps(object):
"Conflicting '%s' models in application '%s': %s and %s." %
(model_name, app_label, app_models[model_name], model))
app_models[model_name] = model
self.do_pending_operations(model)
self.clear_cache()
def is_installed(self, app_name):
@ -332,5 +336,42 @@ class Apps(object):
for model in app_config.get_models(include_auto_created=True):
model._meta._expire_cache()
def lazy_model_operation(self, function, *model_keys):
"""
Take a function and a number of ("app_label", "modelname") tuples, and
when all the corresponding models have been imported and registered,
call the function with the model classes as its arguments.
The function passed to this method must accept exactly n models as
arguments, where n=len(model_keys).
"""
# If this function depends on more than one model, we recursively turn
# it into a chain of functions that accept a single model argument and
# pass each in turn to lazy_model_operation.
model_key, more_models = model_keys[0], model_keys[1:]
if more_models:
supplied_fn = function
def function(model):
next_function = partial(supplied_fn, model)
self.lazy_model_operation(next_function, *more_models)
# If the model is already loaded, pass it to the function immediately.
# Otherwise, delay execution until the class is prepared.
try:
model_class = self.get_registered_model(*model_key)
except LookupError:
self._pending_operations[model_key].append(function)
else:
function(model_class)
def do_pending_operations(self, model):
"""
Take a newly-prepared model and pass it to each function waiting for
it. This is called at the very end of `Apps.register_model()`.
"""
key = model._meta.app_label, model._meta.model_name
for function in self._pending_operations.pop(key, []):
function(model)
apps = Apps(installed_apps=None)

View File

@ -8,10 +8,9 @@ from django.apps.registry import Apps, apps as global_apps
from django.conf import settings
from django.db import models
from django.db.models.fields.proxy import OrderWrt
from django.db.models.fields.related import (
RECURSIVE_RELATIONSHIP_CONSTANT, do_pending_lookups,
)
from django.db.models.fields.related import RECURSIVE_RELATIONSHIP_CONSTANT
from django.db.models.options import DEFAULT_NAMES, normalize_together
from django.db.models.utils import make_model_tuple
from django.utils import six
from django.utils.encoding import force_text, smart_text
from django.utils.functional import cached_property
@ -214,22 +213,21 @@ class StateApps(Apps):
self.render_multiple(list(models.values()) + self.real_models)
# If there are some lookups left, see if we can first resolve them
# ourselves - sometimes fields are added after class_prepared is sent
for lookup_model, operations in self._pending_lookups.items():
# ourselves - sometimes fields are added after a model is registered
for lookup_model in self._pending_operations:
try:
model = self.get_model(lookup_model[0], lookup_model[1])
model = self.get_model(*lookup_model)
except LookupError:
app_label = "%s.%s" % (lookup_model[0], lookup_model[1])
if app_label == settings.AUTH_USER_MODEL and ignore_swappable:
if lookup_model == make_model_tuple(settings.AUTH_USER_MODEL) and ignore_swappable:
continue
# Raise an error with a best-effort helpful message
# (only for the first issue). Error message should look like:
# "ValueError: Lookup failed for model referenced by
# field migrations.Book.author: migrations.Author"
msg = "Lookup failed for model referenced by field {field}: {model[0]}.{model[1]}"
raise ValueError(msg.format(field=operations[0][1], model=lookup_model))
msg = "Lookup failed for model: {model[0]}.{model[1]}"
raise ValueError(msg.format(model=lookup_model))
else:
do_pending_lookups(model)
self.do_pending_operations(model)
def render_multiple(self, model_states):
# We keep trying to render the models in a loop, ignoring invalid
@ -277,6 +275,7 @@ class StateApps(Apps):
self.app_configs[app_label] = AppConfigStub(app_label)
self.app_configs[app_label].models = OrderedDict()
self.app_configs[app_label].models[model._meta.model_name] = model
self.do_pending_operations(model)
self.clear_cache()
def unregister_model(self, app_label, model_name):

View File

@ -21,7 +21,8 @@ from django.db.models.constants import LOOKUP_SEP
from django.db.models.deletion import Collector
from django.db.models.fields import AutoField
from django.db.models.fields.related import (
ForeignObjectRel, ManyToOneRel, OneToOneField, add_lazy_relation,
ForeignObjectRel, ManyToOneRel, OneToOneField, lazy_related_operation,
resolve_relation,
)
from django.db.models.manager import ensure_default_manager
from django.db.models.options import Options
@ -29,6 +30,7 @@ from django.db.models.query import Q
from django.db.models.query_utils import (
DeferredAttribute, deferred_class_factory,
)
from django.db.models.utils import make_model_tuple
from django.utils import six
from django.utils.encoding import force_str, force_text
from django.utils.functional import curry
@ -199,8 +201,8 @@ class ModelBase(type):
# Locate OneToOneField instances.
for field in base._meta.local_fields:
if isinstance(field, OneToOneField):
parent_links[field.remote_field.model] = field
related = resolve_relation(new_class, field.remote_field.model)
parent_links[make_model_tuple(related)] = field
# Do the appropriate setup for any model parents.
for base in parents:
original_base = base
@ -223,8 +225,9 @@ class ModelBase(type):
if not base._meta.abstract:
# Concrete classes...
base = base._meta.concrete_model
if base in parent_links:
field = parent_links[base]
base_key = make_model_tuple(base)
if base_key in parent_links:
field = parent_links[base_key]
elif not is_proxy:
attr_name = '%s_ptr' % base._meta.model_name
field = OneToOneField(base, name=attr_name,
@ -305,7 +308,7 @@ class ModelBase(type):
# defer creating accessors on the foreign class until we are
# certain it has been created
def make_foreign_order_accessors(field, model, cls):
def make_foreign_order_accessors(cls, model, field):
setattr(
field.remote_field.model,
'get_%s_order' % cls.__name__.lower(),
@ -316,12 +319,8 @@ class ModelBase(type):
'set_%s_order' % cls.__name__.lower(),
curry(method_set_order, cls)
)
add_lazy_relation(
cls,
opts.order_with_respect_to,
opts.order_with_respect_to.remote_field.model,
make_foreign_order_accessors
)
wrt = opts.order_with_respect_to
lazy_related_operation(make_foreign_order_accessors, cls, wrt.remote_field.model, field=wrt)
# Give the class a docstring -- its definition.
if cls.__doc__ is None:

View File

@ -1,6 +1,7 @@
from __future__ import unicode_literals
import warnings
from functools import partial
from operator import attrgetter
from django import forms
@ -21,6 +22,7 @@ from django.db.models.fields.related_lookups import (
)
from django.db.models.query import QuerySet
from django.db.models.query_utils import PathInfo
from django.db.models.utils import make_model_tuple
from django.utils import six
from django.utils.deprecation import (
RemovedInDjango20Warning, RemovedInDjango21Warning,
@ -32,75 +34,60 @@ from django.utils.translation import ugettext_lazy as _
RECURSIVE_RELATIONSHIP_CONSTANT = 'self'
def add_lazy_relation(cls, field, relation, operation):
def resolve_relation(scope_model, relation):
"""
Adds a lookup on ``cls`` when a related field is defined using a string,
i.e.::
Transform relation into a model or fully-qualified model string of the form
"app_label.ModelName", relative to scope_model.
class MyModel(Model):
fk = ForeignKey("AnotherModel")
This string can be:
* RECURSIVE_RELATIONSHIP_CONSTANT (i.e. "self") to indicate a recursive
relation.
* The name of a model (i.e "AnotherModel") to indicate another model in
the same app.
* An app-label and model name (i.e. "someapp.AnotherModel") to indicate
another model in a different app.
If the other model hasn't yet been loaded -- almost a given if you're using
lazy relationships -- then the relation won't be set up until the
class_prepared signal fires at the end of model initialization.
``operation`` is the work that must be performed once the relation can be
resolved.
The relation argument can be:
* RECURSIVE_RELATIONSHIP_CONSTANT, i.e. the string "self", in which case
the model argument will be returned.
* A bare model name without an app_label, in which case scope_model's
app_label will be prepended.
* An "app_label.ModelName" string.
* A model class, which will be returned unchanged.
"""
# Check for recursive relations
if relation == RECURSIVE_RELATIONSHIP_CONSTANT:
app_label = cls._meta.app_label
model_name = cls.__name__
relation = scope_model
else:
# Look for an "app.Model" relation.
# Look for an "app.Model" relation
if isinstance(relation, six.string_types):
if "." not in relation:
relation = "%s.%s" % (scope_model._meta.app_label, relation)
if isinstance(relation, six.string_types):
try:
app_label, model_name = relation.split(".")
except ValueError:
# If we can't split, assume a model in current app.
app_label = cls._meta.app_label
model_name = relation
else:
# It's actually a model class.
app_label = relation._meta.app_label
model_name = relation._meta.object_name
# Try to look up the related model, and if it's already loaded resolve the
# string right away. If get_registered_model raises a LookupError, it means
# that the related model isn't loaded yet, so we need to pend the relation
# until the class is prepared.
try:
model = cls._meta.apps.get_registered_model(app_label, model_name)
except LookupError:
key = (app_label, model_name)
value = (cls, field, operation)
cls._meta.apps._pending_lookups.setdefault(key, []).append(value)
else:
operation(field, model, cls)
return relation
def do_pending_lookups(sender, **kwargs):
def lazy_related_operation(function, model, *related_models, **kwargs):
"""
Sent from class_prepared to handle pending relations to the sending model.
"""
key = (sender._meta.app_label, sender.__name__)
for cls, field, operation in sender._meta.apps._pending_lookups.pop(key, []):
operation(field, sender, cls)
Schedule `function` to be called once `model` and all `related_models`
have been imported and registered with the app registry. `function` will
be called with the newly-loaded model classes as its positional arguments,
plus any optional keyword arguments.
signals.class_prepared.connect(do_pending_lookups)
The `model` argument must be a model class. Each subsequent positional
argument is another model, or a reference to another model - see
`resolve_relation()` for the various forms these may take. Any relative
references will be resolved relative to `model`.
This is a convenience wrapper for `Apps.lazy_model_operation` - the app
registry model used is the one found in `model._meta.apps`.
"""
models = [model] + [resolve_relation(model, rel) for rel in related_models]
model_keys = (make_model_tuple(m) for m in models)
apps = model._meta.apps
return apps.lazy_model_operation(partial(function, **kwargs), *model_keys)
def add_lazy_relation(cls, field, relation, operation):
warnings.warn(
"add_lazy_relation() has been superseded by lazy_related_operation() "
"and related methods on the Apps class.",
RemovedInDjango21Warning, stacklevel=2)
# Rearrange args for new Apps.lazy_model_operation
function = lambda local, related, field: operation(field, related, local)
lazy_related_operation(function, cls, relation, field=field)
class RelatedField(Field):
@ -289,13 +276,11 @@ class RelatedField(Field):
return None
def contribute_to_class(self, cls, name, virtual_only=False):
sup = super(RelatedField, self)
super(RelatedField, self).contribute_to_class(cls, name, virtual_only=virtual_only)
self.opts = cls._meta
if hasattr(sup, 'contribute_to_class'):
sup.contribute_to_class(cls, name, virtual_only=virtual_only)
if not cls._meta.abstract:
if self.remote_field.related_name:
related_name = force_text(self.remote_field.related_name) % {
@ -303,14 +288,11 @@ class RelatedField(Field):
'app_label': cls._meta.app_label.lower()
}
self.remote_field.related_name = related_name
other = self.remote_field.model
if isinstance(other, six.string_types) or other._meta.pk is None:
def resolve_related_class(field, model, cls):
field.remote_field.model = model
field.do_related_class(model, cls)
add_lazy_relation(cls, self, other, resolve_related_class)
else:
self.do_related_class(other, cls)
def resolve_related_class(model, related, field):
field.remote_field.model = related
field.do_related_class(related, model)
lazy_related_operation(resolve_related_class, cls, self.remote_field.model, field=self)
@property
def swappable_setting(self):
@ -351,8 +333,7 @@ class RelatedField(Field):
def do_related_class(self, other, cls):
self.set_attributes_from_rel()
if not cls._meta.abstract:
self.contribute_to_related_class(other, self.remote_field)
self.contribute_to_related_class(other, self.remote_field)
def get_limit_choices_to(self):
"""
@ -2132,32 +2113,22 @@ class OneToOneField(ForeignKey):
def create_many_to_many_intermediary_model(field, klass):
from django.db import models
managed = True
if isinstance(field.remote_field.model, six.string_types) and field.remote_field.model != RECURSIVE_RELATIONSHIP_CONSTANT:
to_model = field.remote_field.model
to = to_model.split('.')[-1]
def set_managed(field, model, cls):
field.remote_field.through._meta.managed = model._meta.managed or cls._meta.managed
add_lazy_relation(klass, field, to_model, set_managed)
elif isinstance(field.remote_field.model, six.string_types):
to = klass._meta.object_name
to_model = klass
managed = klass._meta.managed
else:
to = field.remote_field.model._meta.object_name
to_model = field.remote_field.model
managed = klass._meta.managed or to_model._meta.managed
def set_managed(model, related, through):
through._meta.managed = model._meta.managed or related._meta.managed
to_model = resolve_relation(klass, field.remote_field.model)
name = '%s_%s' % (klass._meta.object_name, field.name)
if field.remote_field.model == RECURSIVE_RELATIONSHIP_CONSTANT or to == klass._meta.object_name:
from_ = 'from_%s' % to.lower()
to = 'to_%s' % to.lower()
else:
from_ = klass._meta.model_name
to = to.lower()
lazy_related_operation(set_managed, klass, to_model, name)
to = make_model_tuple(to_model)[1]
from_ = klass._meta.model_name
if to == from_:
to = 'to_%s' % to
from_ = 'from_%s' % from_
meta = type(str('Meta'), (object,), {
'db_table': field._get_m2m_db_table(klass._meta),
'managed': managed,
'auto_created': klass,
'app_label': klass._meta.app_label,
'db_tablespace': klass._meta.db_tablespace,
@ -2323,7 +2294,7 @@ class ManyToManyField(RelatedField):
)
# Set some useful local variables
to_model = self.remote_field.model
to_model = resolve_relation(from_model, self.remote_field.model)
from_model_name = from_model._meta.object_name
if isinstance(to_model, six.string_types):
to_model_name = to_model
@ -2657,8 +2628,13 @@ class ManyToManyField(RelatedField):
# 1) There is a manually specified intermediate, or
# 2) The class owning the m2m field is abstract.
# 3) The class owning the m2m field has been swapped out.
if not self.remote_field.through and not cls._meta.abstract and not cls._meta.swapped:
self.remote_field.through = create_many_to_many_intermediary_model(self, cls)
if not cls._meta.abstract:
if self.remote_field.through:
def resolve_through_model(_, model, field):
field.remote_field.through = model
lazy_related_operation(resolve_through_model, cls, self.remote_field.through, field=self)
elif not cls._meta.swapped:
self.remote_field.through = create_many_to_many_intermediary_model(self, cls)
# Add the descriptor for the m2m relation.
setattr(cls, self.name, ManyRelatedObjectsDescriptor(self.remote_field, reverse=False))
@ -2666,13 +2642,6 @@ class ManyToManyField(RelatedField):
# Set up the accessor for the m2m table name for the relation.
self.m2m_db_table = curry(self._get_m2m_db_table, cls._meta)
# Populate some necessary rel arguments so that cross-app relations
# work correctly.
if not cls._meta.abstract and isinstance(self.remote_field.through, six.string_types):
def resolve_through_model(field, model, cls):
field.remote_field.through = model
add_lazy_relation(cls, self, self.remote_field.through, resolve_through_model)
def contribute_to_related_class(self, cls, related):
# Internal M2Ms (i.e., those with a related name ending with '+')
# and swapped models don't get a related descriptor.

18
django/db/models/utils.py Normal file
View File

@ -0,0 +1,18 @@
from django.utils import six
def make_model_tuple(model):
"""
Takes a model or a string of the form "app_label.ModelName" and returns a
corresponding ("app_label", "modelname") tuple. If a tuple is passed in,
it's assumed to be a valid model tuple already and returned unchanged.
"""
if isinstance(model, tuple):
model_tuple = model
elif isinstance(model, six.string_types):
app_label, model_name = model.split(".")
model_tuple = app_label, model_name.lower()
else:
model_tuple = model._meta.app_label, model._meta.model_name
assert len(model_tuple) == 2, "Invalid model representation: %s" % model
return model_tuple

View File

@ -30,6 +30,8 @@ details on these changes.
* ``Field.remote_field.to`` attribute will be removed.
* ``django.db.models.fields.add_lazy_relation`` will be removed.
.. _deprecation-removed-in-2.0:
2.0

View File

@ -372,6 +372,8 @@ Miscellaneous
:class:`~django.forms.SelectDateWidget` in ``django.forms.widgets``
(or simply ``django.forms``) instead.
* Private API ``django.db.models.fields.add_lazy_relation()`` is deprecated.
.. removed-features-1.9:
Features removed in 1.9

View File

@ -259,6 +259,44 @@ class AppsTests(TestCase):
finally:
apps.apps_ready = True
def test_lazy_model_operation(self):
"""
Tests apps.lazy_model_operation().
"""
model_classes = []
initial_pending = set(apps._pending_operations)
def test_func(*models):
model_classes[:] = models
class LazyA(models.Model):
pass
# Test models appearing twice, and models appearing consecutively
model_keys = [('apps', model_name) for model_name in ['lazya', 'lazyb', 'lazyb', 'lazyc', 'lazya']]
apps.lazy_model_operation(test_func, *model_keys)
# LazyModelA shouldn't be waited on since it's already registered,
# and LazyModelC shouldn't be waited on until LazyModelB exists.
self.assertSetEqual(set(apps._pending_operations) - initial_pending, {('apps', 'lazyb')})
# Test that multiple operations can wait on the same model
apps.lazy_model_operation(test_func, ('apps', 'lazyb'))
class LazyB(models.Model):
pass
self.assertListEqual(model_classes, [LazyB])
# Now we are just waiting on LazyModelC.
self.assertSetEqual(set(apps._pending_operations) - initial_pending, {('apps', 'lazyc')})
class LazyC(models.Model):
pass
# Everything should be loaded - make sure the callback was executed properly.
self.assertListEqual(model_classes, [LazyA, LazyB, LazyB, LazyC, LazyA])
class Stub(object):
def __init__(self, **kwargs):

View File

@ -373,14 +373,3 @@ class PrimaryKeyUUIDModel(models.Model):
class RelatedToUUIDModel(models.Model):
uuid_fk = models.ForeignKey('PrimaryKeyUUIDModel')
###############################################################################
# See ticket #24215.
class AbstractForeignFieldsModel(models.Model):
fk = models.ForeignKey('missing.FK')
m2m = models.ManyToManyField('missing.M2M', through='missing.Through')
class Meta:
abstract = True

View File

@ -5,6 +5,7 @@ import unittest
from decimal import Decimal
from django import forms, test
from django.apps import apps
from django.core import checks, validators
from django.core.exceptions import ValidationError
from django.db import IntegrityError, connection, models, transaction
@ -21,12 +22,11 @@ from django.utils import six
from django.utils.functional import lazy
from .models import (
AbstractForeignFieldsModel, Bar, BigD, BigIntegerModel, BigS, BooleanModel,
DataModel, DateTimeModel, Document, FksToBooleans, FkToChar, FloatModel,
Foo, GenericIPAddress, IntegerModel, NullBooleanModel,
PositiveIntegerModel, PositiveSmallIntegerModel, Post, PrimaryKeyCharModel,
RenamedField, SmallIntegerModel, VerboseNameField, Whiz, WhizIter,
WhizIterEmpty,
Bar, BigD, BigIntegerModel, BigS, BooleanModel, DataModel, DateTimeModel,
Document, FksToBooleans, FkToChar, FloatModel, Foo, GenericIPAddress,
IntegerModel, NullBooleanModel, PositiveIntegerModel,
PositiveSmallIntegerModel, Post, PrimaryKeyCharModel, RenamedField,
SmallIntegerModel, VerboseNameField, Whiz, WhizIter, WhizIterEmpty,
)
@ -202,37 +202,46 @@ class ForeignKeyTests(test.TestCase):
rel_name = Bar._meta.get_field('a').remote_field.related_name
self.assertIsInstance(rel_name, six.text_type)
def test_abstract_model_pending_lookups(self):
def test_abstract_model_pending_operations(self):
"""
Foreign key fields declared on abstract models should not add lazy relations to
resolve relationship declared as string. refs #24215
"""
opts = AbstractForeignFieldsModel._meta
to_key = ('missing', 'FK')
fk_lookup = (AbstractForeignFieldsModel, opts.get_field('fk'))
self.assertFalse(
any(lookup[0:2] == fk_lookup for lookup in opts.apps._pending_lookups.get(to_key, [])),
'Pending lookup added for the abstract model foreign key `to` parameter'
pending_ops_before = list(apps._pending_operations.items())
class AbstractForeignKeyModel(models.Model):
fk = models.ForeignKey('missing.FK')
class Meta:
abstract = True
self.assertIs(AbstractForeignKeyModel._meta.apps, apps)
self.assertEqual(
pending_ops_before,
list(apps._pending_operations.items()),
"Pending lookup added for a foreign key on an abstract model"
)
class ManyToManyFieldTests(test.TestCase):
def test_abstract_model_pending_lookups(self):
def test_abstract_model_pending_operations(self):
"""
Many-to-many fields declared on abstract models should not add lazy relations to
resolve relationship declared as string. refs #24215
"""
opts = AbstractForeignFieldsModel._meta
to_key = ('missing', 'M2M')
fk_lookup = (AbstractForeignFieldsModel, opts.get_field('m2m'))
self.assertFalse(
any(lookup[0:2] == fk_lookup for lookup in opts.apps._pending_lookups.get(to_key, [])),
'Pending lookup added for the abstract model many-to-many `to` parameter.'
)
through_key = ('missing', 'Through')
self.assertFalse(
any(lookup[0:2] == fk_lookup for lookup in opts.apps._pending_lookups.get(through_key, [])),
'Pending lookup added for the abstract model many-to-many `through` parameter.'
pending_ops_before = list(apps._pending_operations.items())
class AbstractManyToManyModel(models.Model):
fk = models.ForeignKey('missing.FK')
class Meta:
abstract = True
self.assertIs(AbstractManyToManyModel._meta.apps, apps)
self.assertEqual(
pending_ops_before,
list(apps._pending_operations.items()),
"Pending lookup added for a many-to-many field on an abstract model"
)