From 73d5eb808435bcf27ebc935847196ac9e97b6ddc Mon Sep 17 00:00:00 2001 From: Adam Johnson Date: Fri, 23 Feb 2024 22:50:09 +0000 Subject: [PATCH] Fixed #35241 -- Cached model's full parent list. co-authored-by: Keryn Knight co-authored-by: Natalia <124304+nessita@users.noreply.github.com> co-authored-by: David Smith co-authored-by: Paolo Melchiorre --- django/contrib/admin/helpers.py | 2 +- django/db/models/base.py | 10 +++++----- django/db/models/deletion.py | 4 +--- django/db/models/expressions.py | 2 +- django/db/models/options.py | 16 ++++++++++++---- django/db/models/query.py | 2 +- django/db/models/query_utils.py | 4 ++-- django/db/models/sql/compiler.py | 4 ++-- django/forms/models.py | 14 ++++++-------- tests/model_meta/tests.py | 16 ++++++++++------ 10 files changed, 41 insertions(+), 33 deletions(-) diff --git a/django/contrib/admin/helpers.py b/django/contrib/admin/helpers.py index 90ca7affc8..c4613fa24e 100644 --- a/django/contrib/admin/helpers.py +++ b/django/contrib/admin/helpers.py @@ -504,7 +504,7 @@ class InlineAdminForm(AdminForm): # 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 ) ) diff --git a/django/db/models/base.py b/django/db/models/base.py index 75328e0749..9dda7cbff9 100644 --- a/django/db/models/base.py +++ b/django/db/models/base.py @@ -1368,7 +1368,7 @@ class Model(AltersData, metaclass=ModelBase): 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) @@ -1397,7 +1397,7 @@ class Model(AltersData, metaclass=ModelBase): # 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: @@ -1546,7 +1546,7 @@ class Model(AltersData, metaclass=ModelBase): 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 @@ -1855,7 +1855,7 @@ class Model(AltersData, metaclass=ModelBase): 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: @@ -1875,7 +1875,7 @@ class Model(AltersData, metaclass=ModelBase): # 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 diff --git a/django/db/models/deletion.py b/django/db/models/deletion.py index 022dec940b..fd3d290a96 100644 --- a/django/db/models/deletion.py +++ b/django/db/models/deletion.py @@ -305,13 +305,11 @@ class Collector: 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 diff --git a/django/db/models/expressions.py b/django/db/models/expressions.py index e59dfbe249..c7fe3b638a 100644 --- a/django/db/models/expressions.py +++ b/django/db/models/expressions.py @@ -1202,7 +1202,7 @@ class RawSQL(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( diff --git a/django/db/models/options.py b/django/db/models/options.py index e63a81c2d8..7e2896a6a2 100644 --- a/django/db/models/options.py +++ b/django/db/models/options.py @@ -692,16 +692,24 @@ class Options: 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): """ diff --git a/django/db/models/query.py b/django/db/models/query.py index 94819758dd..cb5c63c0d1 100644 --- a/django/db/models/query.py +++ b/django/db/models/query.py @@ -788,7 +788,7 @@ class QuerySet(AltersData): # 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: diff --git a/django/db/models/query_utils.py b/django/db/models/query_utils.py index e1041b9653..7162c4fea9 100644 --- a/django/db/models/query_utils.py +++ b/django/db/models/query_utils.py @@ -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 diff --git a/django/db/models/sql/compiler.py b/django/db/models/sql/compiler.py index 9a0d2eb4e7..b36125c762 100644 --- a/django/db/models/sql/compiler.py +++ b/django/db/models/sql/compiler.py @@ -1391,7 +1391,7 @@ class SQLCompiler: 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, @@ -1402,7 +1402,7 @@ class SQLCompiler: # 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 ) ], } diff --git a/django/forms/models.py b/django/forms/models.py index 4b11d5af8c..4cda4e534e 100644 --- a/django/forms/models.py +++ b/django/forms/models.py @@ -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( @@ -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 ) ) ] diff --git a/tests/model_meta/tests.py b/tests/model_meta/tests.py index fef82661cd..0aa04d760d 100644 --- a/tests/model_meta/tests.py +++ b/tests/model_meta/tests.py @@ -325,15 +325,19 @@ class RelationTreeTests(SimpleTestCase): ) -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):