Skip to content

Commit

Permalink
Fixed #35241 -- Cached model's full parent list.
Browse files Browse the repository at this point in the history
co-authored-by: Keryn Knight <[email protected]>
co-authored-by: Natalia <[email protected]>
co-authored-by: David Smith <[email protected]>
co-authored-by: Paolo Melchiorre <[email protected]>
  • Loading branch information
5 people authored and felixxm committed Feb 26, 2024
1 parent 6e1ece7 commit 73d5eb8
Show file tree
Hide file tree
Showing 10 changed files with 41 additions and 33 deletions.
2 changes: 1 addition & 1 deletion django/contrib/admin/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,7 @@ def needs_explicit_pk_field(self):
# in parents.)
any(
parent._meta.auto_field or not parent._meta.model._meta.pk.editable
for parent in self.form._meta.model._meta.get_parent_list()
for parent in self.form._meta.model._meta.all_parents
)
)

Expand Down
10 changes: 5 additions & 5 deletions django/db/models/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -1368,7 +1368,7 @@ def _get_unique_checks(self, exclude=None, include_meta_constraints=False):
constraints = []
if include_meta_constraints:
constraints = [(self.__class__, self._meta.total_unique_constraints)]
for parent_class in self._meta.get_parent_list():
for parent_class in self._meta.all_parents:
if parent_class._meta.unique_together:
unique_togethers.append(
(parent_class, parent_class._meta.unique_together)
Expand Down Expand Up @@ -1397,7 +1397,7 @@ def _get_unique_checks(self, exclude=None, include_meta_constraints=False):
# the list of checks.

fields_with_class = [(self.__class__, self._meta.local_fields)]
for parent_class in self._meta.get_parent_list():
for parent_class in self._meta.all_parents:
fields_with_class.append((parent_class, parent_class._meta.local_fields))

for model_class, fields in fields_with_class:
Expand Down Expand Up @@ -1546,7 +1546,7 @@ def unique_error_message(self, model_class, unique_check):

def get_constraints(self):
constraints = [(self.__class__, self._meta.constraints)]
for parent_class in self._meta.get_parent_list():
for parent_class in self._meta.all_parents:
if parent_class._meta.constraints:
constraints.append((parent_class, parent_class._meta.constraints))
return constraints
Expand Down Expand Up @@ -1855,7 +1855,7 @@ def _check_field_name_clashes(cls):
used_fields = {} # name or attname -> field

# Check that multi-inheritance doesn't cause field name shadowing.
for parent in cls._meta.get_parent_list():
for parent in cls._meta.all_parents:
for f in parent._meta.local_fields:
clash = used_fields.get(f.name) or used_fields.get(f.attname) or None
if clash:
Expand All @@ -1875,7 +1875,7 @@ def _check_field_name_clashes(cls):
# Check that fields defined in the model don't clash with fields from
# parents, including auto-generated fields like multi-table inheritance
# child accessors.
for parent in cls._meta.get_parent_list():
for parent in cls._meta.all_parents:
for f in parent._meta.get_fields():
if f not in used_fields:
used_fields[f.name] = f
Expand Down
4 changes: 1 addition & 3 deletions django/db/models/deletion.py
Original file line number Diff line number Diff line change
Expand Up @@ -305,13 +305,11 @@ def collect(
if not collect_related:
return

if keep_parents:
parents = set(model._meta.get_parent_list())
model_fast_deletes = defaultdict(list)
protected_objects = defaultdict(list)
for related in get_candidate_relations_to_delete(model._meta):
# Preserve parent reverse relationships if keep_parents=True.
if keep_parents and related.model in parents:
if keep_parents and related.model in model._meta.all_parents:
continue
field = related.field
on_delete = field.remote_field.on_delete
Expand Down
2 changes: 1 addition & 1 deletion django/db/models/expressions.py
Original file line number Diff line number Diff line change
Expand Up @@ -1202,7 +1202,7 @@ def resolve_expression(
):
# Resolve parents fields used in raw SQL.
if query.model:
for parent in query.model._meta.get_parent_list():
for parent in query.model._meta.all_parents:
for parent_field in parent._meta.local_fields:
if parent_field.column.lower() in self.sql.lower():
query.resolve_ref(
Expand Down
16 changes: 12 additions & 4 deletions django/db/models/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -692,16 +692,24 @@ def get_base_chain(self, model):
return res
return []

def get_parent_list(self):
@cached_property
def all_parents(self):
"""
Return all the ancestors of this model as a list ordered by MRO.
Return all the ancestors of this model as a tuple ordered by MRO.
Useful for determining if something is an ancestor, regardless of lineage.
"""
result = OrderedSet(self.parents)
for parent in self.parents:
for ancestor in parent._meta.get_parent_list():
for ancestor in parent._meta.all_parents:
result.add(ancestor)
return list(result)
return tuple(result)

def get_parent_list(self):
"""
Return all the ancestors of this model as a list ordered by MRO.
Backward compatibility method.
"""
return list(self.all_parents)

def get_ancestor_link(self, ancestor):
"""
Expand Down
2 changes: 1 addition & 1 deletion django/db/models/query.py
Original file line number Diff line number Diff line change
Expand Up @@ -788,7 +788,7 @@ def bulk_create(
# model to detect the inheritance pattern ConcreteGrandParent ->
# MultiTableParent -> ProxyChild. Simply checking self.model._meta.proxy
# would not identify that case as involving multiple tables.
for parent in self.model._meta.get_parent_list():
for parent in self.model._meta.all_parents:
if parent._meta.concrete_model is not self.model._meta.concrete_model:
raise ValueError("Can't bulk create a multi-table inherited model")
if not objs:
Expand Down
4 changes: 2 additions & 2 deletions django/db/models/query_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -403,8 +403,8 @@ def check_rel_lookup_compatibility(model, target_opts, field):
def check(opts):
return (
model._meta.concrete_model == opts.concrete_model
or opts.concrete_model in model._meta.get_parent_list()
or model in opts.get_parent_list()
or opts.concrete_model in model._meta.all_parents
or model in opts.all_parents
)

# If the field is a primary key, then doing a query against the field's
Expand Down
4 changes: 2 additions & 2 deletions django/db/models/sql/compiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -1391,7 +1391,7 @@ def get_select_for_update_of_arguments(self):
def _get_parent_klass_info(klass_info):
concrete_model = klass_info["model"]._meta.concrete_model
for parent_model, parent_link in concrete_model._meta.parents.items():
parent_list = parent_model._meta.get_parent_list()
all_parents = parent_model._meta.all_parents
yield {
"model": parent_model,
"field": parent_link,
Expand All @@ -1402,7 +1402,7 @@ def _get_parent_klass_info(klass_info):
# Selected columns from a model or its parents.
if (
self.select[select_index][0].target.model == parent_model
or self.select[select_index][0].target.model in parent_list
or self.select[select_index][0].target.model in all_parents
)
],
}
Expand Down
14 changes: 6 additions & 8 deletions django/forms/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -1214,20 +1214,19 @@ def _get_foreign_key(parent_model, model, fk_name=None, can_fail=False):
fks_to_parent = [f for f in opts.fields if f.name == fk_name]
if len(fks_to_parent) == 1:
fk = fks_to_parent[0]
parent_list = parent_model._meta.get_parent_list()
parent_list.append(parent_model)
all_parents = (*parent_model._meta.all_parents, parent_model)
if (
not isinstance(fk, ForeignKey)
or (
# ForeignKey to proxy models.
fk.remote_field.model._meta.proxy
and fk.remote_field.model._meta.proxy_for_model not in parent_list
and fk.remote_field.model._meta.proxy_for_model not in all_parents
)
or (
# ForeignKey to concrete models.
not fk.remote_field.model._meta.proxy
and fk.remote_field.model != parent_model
and fk.remote_field.model not in parent_list
and fk.remote_field.model not in all_parents
)
):
raise ValueError(
Expand All @@ -1240,18 +1239,17 @@ def _get_foreign_key(parent_model, model, fk_name=None, can_fail=False):
)
else:
# Try to discover what the ForeignKey from model to parent_model is
parent_list = parent_model._meta.get_parent_list()
parent_list.append(parent_model)
all_parents = (*parent_model._meta.all_parents, parent_model)
fks_to_parent = [
f
for f in opts.fields
if isinstance(f, ForeignKey)
and (
f.remote_field.model == parent_model
or f.remote_field.model in parent_list
or f.remote_field.model in all_parents
or (
f.remote_field.model._meta.proxy
and f.remote_field.model._meta.proxy_for_model in parent_list
and f.remote_field.model._meta.proxy_for_model in all_parents
)
)
]
Expand Down
16 changes: 10 additions & 6 deletions tests/model_meta/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -325,15 +325,19 @@ def test_relations_related_objects(self):
)


class ParentListTests(SimpleTestCase):
def test_get_parent_list(self):
self.assertEqual(CommonAncestor._meta.get_parent_list(), [])
self.assertEqual(FirstParent._meta.get_parent_list(), [CommonAncestor])
self.assertEqual(SecondParent._meta.get_parent_list(), [CommonAncestor])
class AllParentsTests(SimpleTestCase):
def test_all_parents(self):
self.assertEqual(CommonAncestor._meta.all_parents, ())
self.assertEqual(FirstParent._meta.all_parents, (CommonAncestor,))
self.assertEqual(SecondParent._meta.all_parents, (CommonAncestor,))
self.assertEqual(
Child._meta.get_parent_list(), [FirstParent, SecondParent, CommonAncestor]
Child._meta.all_parents,
(FirstParent, SecondParent, CommonAncestor),
)

def test_get_parent_list(self):
self.assertEqual(Child._meta.get_parent_list(), list(Child._meta.all_parents))


class PropertyNamesTests(SimpleTestCase):
def test_person(self):
Expand Down

0 comments on commit 73d5eb8

Please sign in to comment.