Refs #18012 -- Made proxy and concrete model reverse fields consistent.

Prior to this change proxy models reverse fields didn't include the
reverse fields pointing to their concrete model.
This commit is contained in:
Simon Charette 2015-10-07 15:29:20 -04:00
parent 211486f3ab
commit 5b980897f2
6 changed files with 113 additions and 14 deletions

View File

@ -55,7 +55,7 @@ def get_candidate_relations_to_delete(opts):
# The candidate relations are the ones that come from N-1 and 1-1 relations. # The candidate relations are the ones that come from N-1 and 1-1 relations.
# N-N (i.e., many-to-many) relations aren't candidates for deletion. # N-N (i.e., many-to-many) relations aren't candidates for deletion.
return ( return (
f for f in opts.concrete_model._meta.get_fields(include_hidden=True) f for f in opts.get_fields(include_hidden=True)
if f.auto_created and not f.concrete and (f.one_to_one or f.one_to_many) if f.auto_created and not f.concrete and (f.one_to_one or f.one_to_many)
) )

View File

@ -552,14 +552,10 @@ class Options(object):
is set as a property on every model. is set as a property on every model.
""" """
related_objects_graph = defaultdict(list) related_objects_graph = defaultdict(list)
# Map of concrete models to all options of models it represents.
# Including its options and all its proxy model ones.
concrete_model_classes = defaultdict(list)
all_models = self.apps.get_models(include_auto_created=True) all_models = self.apps.get_models(include_auto_created=True)
for model in all_models: for model in all_models:
opts = model._meta opts = model._meta
concrete_model_classes[opts.concrete_model].append(opts)
# Abstract model's fields are copied to child models, hence we will # Abstract model's fields are copied to child models, hence we will
# see the fields from the child models. # see the fields from the child models.
if opts.abstract: if opts.abstract:
@ -570,7 +566,7 @@ class Options(object):
) )
for f in fields_with_relations: for f in fields_with_relations:
if not isinstance(f.remote_field.model, six.string_types): if not isinstance(f.remote_field.model, six.string_types):
related_objects_graph[f.remote_field.model._meta].append(f) related_objects_graph[f.remote_field.model._meta.concrete_model._meta].append(f)
for model in all_models: for model in all_models:
# Set the relation_tree using the internal __dict__. In this way # Set the relation_tree using the internal __dict__. In this way
@ -578,9 +574,7 @@ class Options(object):
# __dict__ takes precedence over a data descriptor (such as # __dict__ takes precedence over a data descriptor (such as
# @cached_property). This means that the _meta._relation_tree is # @cached_property). This means that the _meta._relation_tree is
# only called if related_objects is not in __dict__. # only called if related_objects is not in __dict__.
related_objects = list(chain.from_iterable( related_objects = related_objects_graph[model._meta.concrete_model._meta]
related_objects_graph[opts] for opts in concrete_model_classes[model]
))
model._meta.__dict__['_relation_tree'] = related_objects model._meta.__dict__['_relation_tree'] = related_objects
# It seems it is possible that self is not in all_models, so guard # It seems it is possible that self is not in all_models, so guard
# against that with default for get(). # against that with default for get().
@ -674,10 +668,10 @@ class Options(object):
for obj in parent._meta._get_fields( for obj in parent._meta._get_fields(
forward=forward, reverse=reverse, include_parents=include_parents, forward=forward, reverse=reverse, include_parents=include_parents,
include_hidden=include_hidden, seen_models=seen_models): include_hidden=include_hidden, seen_models=seen_models):
if hasattr(obj, 'parent_link') and obj.parent_link: if getattr(obj, 'parent_link', False) and obj.model != self.concrete_model:
continue continue
fields.append(obj) fields.append(obj)
if reverse: if reverse and not self.proxy:
# Tree is computed once and cached until the app cache is expired. # Tree is computed once and cached until the app cache is expired.
# It is composed of a list of fields pointing to the current model # It is composed of a list of fields pointing to the current model
# from other models. # from other models.

View File

@ -238,6 +238,15 @@ But it didn't prohibit nested non-relation fields as it does now::
... ...
FieldError: Non-relational field given in select_related: 'name' FieldError: Non-relational field given in select_related: 'name'
``_meta.get_fields()`` returns consistent reverse fields for proxy models
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Before Django 1.10, the :meth:`~django.db.models.options.Options.get_fields`
method returned different reverse fields when called on a proxy model compared
to its proxied concrete class. This inconsistency was fixed by returning the
full set of fields pointing to a concrete class or one of its proxies in both
cases.
Miscellaneous Miscellaneous
~~~~~~~~~~~~~ ~~~~~~~~~~~~~

View File

@ -101,6 +101,10 @@ class ProxyPerson(Person):
proxy = True proxy = True
class PersonThroughProxySubclass(ProxyPerson):
pass
class Relating(models.Model): class Relating(models.Model):
# ForeignKey to BasePerson # ForeignKey to BasePerson

View File

@ -1,4 +1,6 @@
from .models import AbstractPerson, BasePerson, Person, Relating, Relation from .models import (
AbstractPerson, BasePerson, Person, ProxyPerson, Relating, Relation,
)
TEST_RESULTS = { TEST_RESULTS = {
'get_all_field_names': { 'get_all_field_names': {
@ -329,11 +331,30 @@ TEST_RESULTS = {
('Relating_people_hidden+', None), ('Relating_people_hidden+', None),
('followers_concrete', None), ('followers_concrete', None),
('friends_inherited_rel_+', None), ('friends_inherited_rel_+', None),
('personthroughproxysubclass', None),
('relating_people', None), ('relating_people', None),
('relating_person', None), ('relating_person', None),
('relating_proxyperson', None), ('relating_proxyperson', None),
('relating_proxyperson_hidden+', None), ('relating_proxyperson_hidden+', None),
), ),
ProxyPerson: (
('+', Person),
('_relating_people_hidden_+', Person),
('Person_following_inherited+', Person),
('Person_following_inherited+', Person),
('Person_friends_inherited+', Person),
('Person_friends_inherited+', Person),
('Person_m2m_inherited+', Person),
('Relating_people+', Person),
('Relating_people_hidden+', Person),
('followers_concrete', Person),
('friends_inherited_rel_+', Person),
('personthroughproxysubclass', Person),
('relating_people', Person),
('relating_person', Person),
('relating_proxyperson', Person),
('relating_proxyperson_hidden+', Person),
),
BasePerson: ( BasePerson: (
('+', None), ('+', None),
('_relating_basepeople_hidden_+', None), ('_relating_basepeople_hidden_+', None),
@ -366,6 +387,9 @@ TEST_RESULTS = {
('+', None), ('+', None),
('+', None), ('+', None),
('+', None), ('+', None),
('+', None),
('+', None),
('+', None),
('BasePerson_m2m_abstract+', None), ('BasePerson_m2m_abstract+', None),
('BasePerson_m2m_base+', None), ('BasePerson_m2m_base+', None),
('Person_m2m_inherited+', None), ('Person_m2m_inherited+', None),
@ -411,6 +435,7 @@ TEST_RESULTS = {
('friends_abstract_rel_+', BasePerson), ('friends_abstract_rel_+', BasePerson),
('friends_base_rel_+', BasePerson), ('friends_base_rel_+', BasePerson),
('friends_inherited_rel_+', None), ('friends_inherited_rel_+', None),
('personthroughproxysubclass', None),
('relating_basepeople', BasePerson), ('relating_basepeople', BasePerson),
('relating_baseperson', BasePerson), ('relating_baseperson', BasePerson),
('relating_people', None), ('relating_people', None),
@ -418,6 +443,44 @@ TEST_RESULTS = {
('relating_proxyperson', None), ('relating_proxyperson', None),
('relating_proxyperson_hidden+', None), ('relating_proxyperson_hidden+', None),
), ),
ProxyPerson: (
('+', BasePerson),
('+', Person),
('_relating_basepeople_hidden_+', BasePerson),
('_relating_people_hidden_+', Person),
('BasePerson_following_abstract+', BasePerson),
('BasePerson_following_abstract+', BasePerson),
('BasePerson_following_base+', BasePerson),
('BasePerson_following_base+', BasePerson),
('BasePerson_friends_abstract+', BasePerson),
('BasePerson_friends_abstract+', BasePerson),
('BasePerson_friends_base+', BasePerson),
('BasePerson_friends_base+', BasePerson),
('BasePerson_m2m_abstract+', BasePerson),
('BasePerson_m2m_base+', BasePerson),
('Person_following_inherited+', Person),
('Person_following_inherited+', Person),
('Person_friends_inherited+', Person),
('Person_friends_inherited+', Person),
('Person_m2m_inherited+', Person),
('Relating_basepeople+', BasePerson),
('Relating_basepeople_hidden+', BasePerson),
('Relating_people+', Person),
('Relating_people_hidden+', Person),
('followers_abstract', BasePerson),
('followers_base', BasePerson),
('followers_concrete', Person),
('friends_abstract_rel_+', BasePerson),
('friends_base_rel_+', BasePerson),
('friends_inherited_rel_+', Person),
('personthroughproxysubclass', Person),
('relating_basepeople', BasePerson),
('relating_baseperson', BasePerson),
('relating_people', Person),
('relating_person', Person),
('relating_proxyperson', Person),
('relating_proxyperson_hidden+', Person),
),
BasePerson: ( BasePerson: (
('+', None), ('+', None),
('_relating_basepeople_hidden_+', None), ('_relating_basepeople_hidden_+', None),
@ -450,6 +513,9 @@ TEST_RESULTS = {
('+', None), ('+', None),
('+', None), ('+', None),
('+', None), ('+', None),
('+', None),
('+', None),
('+', None),
('BasePerson_m2m_abstract+', None), ('BasePerson_m2m_abstract+', None),
('BasePerson_m2m_base+', None), ('BasePerson_m2m_base+', None),
('Person_m2m_inherited+', None), ('Person_m2m_inherited+', None),
@ -467,10 +533,18 @@ TEST_RESULTS = {
'get_all_related_objects_with_model_local': { 'get_all_related_objects_with_model_local': {
Person: ( Person: (
('followers_concrete', None), ('followers_concrete', None),
('personthroughproxysubclass', None),
('relating_person', None), ('relating_person', None),
('relating_people', None), ('relating_people', None),
('relating_proxyperson', None), ('relating_proxyperson', None),
), ),
ProxyPerson: (
('followers_concrete', Person),
('personthroughproxysubclass', Person),
('relating_person', Person),
('relating_people', Person),
('relating_proxyperson', Person),
),
BasePerson: ( BasePerson: (
('followers_abstract', None), ('followers_abstract', None),
('followers_base', None), ('followers_base', None),
@ -497,10 +571,22 @@ TEST_RESULTS = {
('relating_baseperson', BasePerson), ('relating_baseperson', BasePerson),
('relating_basepeople', BasePerson), ('relating_basepeople', BasePerson),
('followers_concrete', None), ('followers_concrete', None),
('personthroughproxysubclass', None),
('relating_person', None), ('relating_person', None),
('relating_people', None), ('relating_people', None),
('relating_proxyperson', None), ('relating_proxyperson', None),
), ),
ProxyPerson: (
('followers_abstract', BasePerson),
('followers_base', BasePerson),
('relating_baseperson', BasePerson),
('relating_basepeople', BasePerson),
('followers_concrete', Person),
('personthroughproxysubclass', Person),
('relating_person', Person),
('relating_people', Person),
('relating_proxyperson', Person),
),
BasePerson: ( BasePerson: (
('followers_abstract', None), ('followers_abstract', None),
('followers_base', None), ('followers_base', None),

View File

@ -111,7 +111,10 @@ class RelatedObjectsTests(OptionsBaseTests):
for field in model._meta.get_fields() for field in model._meta.get_fields()
if field.auto_created and not field.concrete if field.auto_created and not field.concrete
] ]
self.assertEqual(self._map_related_query_names(objects), expected) self.assertEqual(
sorted(self._map_related_query_names(objects), key=self.key_name),
sorted(expected, key=self.key_name),
)
def test_related_objects_local(self): def test_related_objects_local(self):
result_key = 'get_all_related_objects_with_model_local' result_key = 'get_all_related_objects_with_model_local'
@ -121,7 +124,10 @@ class RelatedObjectsTests(OptionsBaseTests):
for field in model._meta.get_fields(include_parents=False) for field in model._meta.get_fields(include_parents=False)
if field.auto_created and not field.concrete if field.auto_created and not field.concrete
] ]
self.assertEqual(self._map_related_query_names(objects), expected) self.assertEqual(
sorted(self._map_related_query_names(objects), key=self.key_name),
sorted(expected, key=self.key_name),
)
def test_related_objects_include_hidden(self): def test_related_objects_include_hidden(self):
result_key = 'get_all_related_objects_with_model_hidden' result_key = 'get_all_related_objects_with_model_hidden'