Fixed #19049 -- Ensure that swapped models aren't included in reverse field caches.

Thanks to Ivan Virabyan for the report.
This commit is contained in:
Russell Keith-Magee 2012-10-02 22:52:45 +08:00
parent 5f8b97f9fb
commit 3b6f980bed
5 changed files with 47 additions and 27 deletions

View File

@ -23,7 +23,6 @@ def get_validation_errors(outfile, app=None):
validates all models of all installed apps. Writes errors, if any, to outfile.
Returns number of errors.
"""
from django.conf import settings
from django.db import models, connection
from django.db.models.loading import get_app_errors
from django.db.models.fields.related import RelatedObject
@ -37,7 +36,19 @@ def get_validation_errors(outfile, app=None):
for cls in models.get_models(app):
opts = cls._meta
# Do field-specific validation.
# Check swappable attribute.
if opts.swapped:
try:
app_label, model_name = opts.swapped.split('.')
except ValueError:
e.add(opts, "%s is not of the form 'app_label.app_name'." % opts.swappable)
continue
if not models.get_model(app_label, model_name):
e.add(opts, "Model has been swapped out for '%s' which has not been installed or is abstract." % opts.swapped)
# No need to perform any other validation checks on a swapped model.
continue
# Model isn't swapped; do field-specific validation.
for f in opts.local_fields:
if f.name == 'id' and not f.primary_key and opts.pk.name == 'id':
e.add(opts, '"%s": You can\'t use "id" as a field name, because each model automatically gets an "id" field if none of the fields have primary_key=True. You need to either remove/rename your "id" field or add primary_key=True to a field.' % f.name)
@ -285,16 +296,6 @@ def get_validation_errors(outfile, app=None):
if r.get_accessor_name() == rel_query_name:
e.add(opts, "Reverse query name for m2m field '%s' clashes with related field '%s.%s'. Add a related_name argument to the definition for '%s'." % (f.name, rel_opts.object_name, r.get_accessor_name(), f.name))
# Check swappable attribute.
if opts.swapped:
try:
app_label, model_name = opts.swapped.split('.')
except ValueError:
e.add(opts, "%s is not of the form 'app_label.app_name'." % opts.swappable)
continue
if not models.get_model(app_label, model_name):
e.add(opts, "Model has been swapped out for '%s' which has not been installed or is abstract." % opts.swapped)
# Check ordering attribute.
if opts.ordering:
for field_name in opts.ordering:

View File

@ -1025,8 +1025,8 @@ class ForeignKey(RelatedField, Field):
def contribute_to_related_class(self, cls, related):
# Internal FK's - i.e., those with a related name ending with '+' -
# don't get a related descriptor.
if not self.rel.is_hidden():
# and swapped models don't get a related descriptor.
if not self.rel.is_hidden() and related.model._meta.swapped:
setattr(cls, related.get_accessor_name(), ForeignRelatedObjectsDescriptor(related))
if self.rel.limit_choices_to:
cls._meta.related_fkey_lookups.append(self.rel.limit_choices_to)
@ -1265,8 +1265,8 @@ class ManyToManyField(RelatedField, Field):
def contribute_to_related_class(self, cls, related):
# Internal M2Ms (i.e., those with a related name ending with '+')
# don't get a related descriptor.
if not self.rel.is_hidden():
# and swapped models don't get a related descriptor.
if not self.rel.is_hidden() and not related.model._meta.swapped:
setattr(cls, related.get_accessor_name(), ManyRelatedObjectsDescriptor(related))
# Set up the accessors for the column names on the m2m table

View File

@ -418,13 +418,14 @@ class Options(object):
# Collect also objects which are in relation to some proxy child/parent of self.
proxy_cache = cache.copy()
for klass in get_models(include_auto_created=True, only_installed=False):
for f in klass._meta.local_fields:
if f.rel and not isinstance(f.rel.to, six.string_types):
if self == f.rel.to._meta:
cache[RelatedObject(f.rel.to, klass, f)] = None
proxy_cache[RelatedObject(f.rel.to, klass, f)] = None
elif self.concrete_model == f.rel.to._meta.concrete_model:
proxy_cache[RelatedObject(f.rel.to, klass, f)] = None
if not klass._meta.swapped:
for f in klass._meta.local_fields:
if f.rel and not isinstance(f.rel.to, six.string_types):
if self == f.rel.to._meta:
cache[RelatedObject(f.rel.to, klass, f)] = None
proxy_cache[RelatedObject(f.rel.to, klass, f)] = None
elif self.concrete_model == f.rel.to._meta.concrete_model:
proxy_cache[RelatedObject(f.rel.to, klass, f)] = None
self._related_objects_cache = cache
self._related_objects_proxy_cache = proxy_cache
@ -460,9 +461,12 @@ class Options(object):
else:
cache[obj] = model
for klass in get_models(only_installed=False):
for f in klass._meta.local_many_to_many:
if f.rel and not isinstance(f.rel.to, six.string_types) and self == f.rel.to._meta:
cache[RelatedObject(f.rel.to, klass, f)] = None
if not klass._meta.swapped:
for f in klass._meta.local_many_to_many:
if (f.rel
and not isinstance(f.rel.to, six.string_types)
and self == f.rel.to._meta):
cache[RelatedObject(f.rel.to, klass, f)] = None
if app_cache_ready():
self._related_many_to_many_cache = cache
return cache

View File

@ -303,13 +303,28 @@ class SwappedModel(models.Model):
References to this model *should* raise a validation error.
Requires TEST_SWAPPED_MODEL to be defined in the test environment;
this is guaranteed by the test runner using @override_settings.
The foreign keys and m2m relations on this model *shouldn't*
install related accessors, so there shouldn't be clashes with
the equivalent names on the replacement.
"""
name = models.CharField(max_length=100)
foreign = models.ForeignKey(Target, related_name='swappable_fk_set')
m2m = models.ManyToManyField(Target, related_name='swappable_m2m_set')
class Meta:
swappable = 'TEST_SWAPPED_MODEL'
class ReplacementModel(models.Model):
"""A replacement model for swapping purposes."""
name = models.CharField(max_length=100)
foreign = models.ForeignKey(Target, related_name='swappable_fk_set')
m2m = models.ManyToManyField(Target, related_name='swappable_m2m_set')
class BadSwappableValue(models.Model):
"""A model that can be swapped out; during testing, the swappable
value is not of the format app.model

View File

@ -37,7 +37,7 @@ class InvalidModelTestCase(unittest.TestCase):
# easier to set this up as an override than to require every developer
# to specify a value in their test settings.
@override_settings(
TEST_SWAPPED_MODEL='invalid_models.Target',
TEST_SWAPPED_MODEL='invalid_models.ReplacementModel',
TEST_SWAPPED_MODEL_BAD_VALUE='not-a-model',
TEST_SWAPPED_MODEL_BAD_MODEL='not_an_app.Target',
)