diff options
| -rw-r--r-- | django/contrib/admin/options.py | 10 | ||||
| -rw-r--r-- | django/contrib/admin/utils.py | 8 | ||||
| -rw-r--r-- | django/contrib/contenttypes/fields.py | 2 | ||||
| -rw-r--r-- | django/db/models/base.py | 2 | ||||
| -rw-r--r-- | django/db/models/fields/related.py | 40 | ||||
| -rw-r--r-- | django/db/models/fields/related_descriptors.py | 2 | ||||
| -rw-r--r-- | django/db/models/fields/related_lookups.py | 10 | ||||
| -rw-r--r-- | django/db/models/fields/reverse_related.py | 25 | ||||
| -rw-r--r-- | django/db/models/options.py | 2 | ||||
| -rw-r--r-- | django/db/models/sql/query.py | 7 | ||||
| -rw-r--r-- | tests/foreign_object/tests.py | 103 |
11 files changed, 182 insertions, 29 deletions
diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py index 7b2b893c10..66f3396a6b 100644 --- a/django/contrib/admin/options.py +++ b/django/contrib/admin/options.py @@ -400,14 +400,14 @@ class BaseModelAdmin(metaclass=forms.MediaDefiningClass): # model anyways. For example, if you filter on employee__department__id, # then the id value would be found already from employee__department_id. if not prev_field or (prev_field.is_relation and - field not in prev_field.get_path_info()[-1].target_fields): + field not in prev_field.path_infos[-1].target_fields): relation_parts.append(part) - if not getattr(field, 'get_path_info', None): + if not getattr(field, 'path_infos', None): # This is not a relational field, so further parts # must be transforms. break prev_field = field - model = field.get_path_info()[-1].to_opts.model + model = field.path_infos[-1].to_opts.model if len(relation_parts) <= 1: # Either a local field filter, or no fields at all. @@ -1020,9 +1020,9 @@ class ModelAdmin(BaseModelAdmin): return field_name else: prev_field = field - if hasattr(field, 'get_path_info'): + if hasattr(field, 'path_infos'): # Update opts to follow the relation. - opts = field.get_path_info()[-1].to_opts + opts = field.path_infos[-1].to_opts # Otherwise, use the field with icontains. return "%s__icontains" % field_name diff --git a/django/contrib/admin/utils.py b/django/contrib/admin/utils.py index 5821db5ffe..baaaa9e43f 100644 --- a/django/contrib/admin/utils.py +++ b/django/contrib/admin/utils.py @@ -40,9 +40,9 @@ def lookup_spawns_duplicates(opts, lookup_path): # Ignore query lookups. continue else: - if hasattr(field, 'get_path_info'): + if hasattr(field, 'path_infos'): # This field is a relation; update opts to follow the relation. - path_info = field.get_path_info() + path_info = field.path_infos opts = path_info[-1].to_opts if any(path.m2m for path in path_info): # This field is a m2m relation so duplicates must be @@ -435,8 +435,8 @@ class NotRelationField(Exception): def get_model_from_relation(field): - if hasattr(field, 'get_path_info'): - return field.get_path_info()[-1].to_opts.model + if hasattr(field, 'path_infos'): + return field.path_infos[-1].to_opts.model else: raise NotRelationField diff --git a/django/contrib/contenttypes/fields.py b/django/contrib/contenttypes/fields.py index 63ee408bf5..b0b77b01d4 100644 --- a/django/contrib/contenttypes/fields.py +++ b/django/contrib/contenttypes/fields.py @@ -395,7 +395,7 @@ class GenericRelation(ForeignObject): opts = field.remote_field.model._meta parent_field_chain.reverse() for field in parent_field_chain: - path.extend(field.remote_field.get_path_info()) + path.extend(field.remote_field.path_infos) return path def get_path_info(self, filtered_relation=None): diff --git a/django/db/models/base.py b/django/db/models/base.py index 3c46afcf9f..cc03ae1252 100644 --- a/django/db/models/base.py +++ b/django/db/models/base.py @@ -1828,7 +1828,7 @@ class Model(metaclass=ModelBase): else: fld = _cls._meta.get_field(part) if fld.is_relation: - _cls = fld.get_path_info()[-1].to_opts.model + _cls = fld.path_infos[-1].to_opts.model else: _cls = None except (FieldDoesNotExist, AttributeError): diff --git a/django/db/models/fields/related.py b/django/db/models/fields/related.py index 2e8c87c1b1..8533803ba6 100644 --- a/django/db/models/fields/related.py +++ b/django/db/models/fields/related.py @@ -447,7 +447,7 @@ class RelatedField(FieldCacheMixin, Field): When filtering against this relation, return the field on the remote model against which the filtering should happen. """ - target_fields = self.get_path_info()[-1].target_fields + target_fields = self.path_infos[-1].target_fields if len(target_fields) > 1: raise exceptions.FieldError( "The relation has multiple target fields, but only single target field was asked for") @@ -499,6 +499,13 @@ class ForeignObject(RelatedField): self.to_fields = to_fields self.swappable = swappable + def __copy__(self): + obj = super().__copy__() + # Remove any cached PathInfo values. + obj.__dict__.pop('path_infos', None) + obj.__dict__.pop('reverse_path_infos', None) + return obj + def check(self, **kwargs): return [ *super().check(**kwargs), @@ -743,6 +750,10 @@ class ForeignObject(RelatedField): filtered_relation=filtered_relation, )] + @cached_property + def path_infos(self): + return self.get_path_info() + def get_reverse_path_info(self, filtered_relation=None): """Get path from the related model to this field's model.""" opts = self.model._meta @@ -757,6 +768,10 @@ class ForeignObject(RelatedField): filtered_relation=filtered_relation, )] + @cached_property + def reverse_path_infos(self): + return self.get_reverse_path_info() + @classmethod @functools.lru_cache(maxsize=None) def get_lookups(cls): @@ -1541,12 +1556,17 @@ class ManyToManyField(RelatedField): linkfield1 = int_model._meta.get_field(self.m2m_field_name()) linkfield2 = int_model._meta.get_field(self.m2m_reverse_field_name()) if direct: - join1infos = linkfield1.get_reverse_path_info() - join2infos = linkfield2.get_path_info(filtered_relation) + join1infos = linkfield1.reverse_path_infos + if filtered_relation: + join2infos = linkfield2.get_path_info(filtered_relation) + else: + join2infos = linkfield2.path_infos else: - join1infos = linkfield2.get_reverse_path_info() - join2infos = linkfield1.get_path_info(filtered_relation) - + join1infos = linkfield2.reverse_path_infos + if filtered_relation: + join2infos = linkfield1.get_path_info(filtered_relation) + else: + join2infos = linkfield1.path_infos # Get join infos between the last model of join 1 and the first model # of join 2. Assume the only reason these may differ is due to model # inheritance. @@ -1564,9 +1584,17 @@ class ManyToManyField(RelatedField): def get_path_info(self, filtered_relation=None): return self._get_path_info(direct=True, filtered_relation=filtered_relation) + @cached_property + def path_infos(self): + return self.get_path_info() + def get_reverse_path_info(self, filtered_relation=None): return self._get_path_info(direct=False, filtered_relation=filtered_relation) + @cached_property + def reverse_path_infos(self): + return self.get_reverse_path_info() + def _get_m2m_db_table(self, opts): """ Function that can be curried to provide the m2m table name for this diff --git a/django/db/models/fields/related_descriptors.py b/django/db/models/fields/related_descriptors.py index cb77a0c476..ef546e844a 100644 --- a/django/db/models/fields/related_descriptors.py +++ b/django/db/models/fields/related_descriptors.py @@ -599,7 +599,7 @@ def create_reverse_many_to_one_manager(superclass, rel): # for related object id. rel_obj_id = tuple([ getattr(self.instance, target_field.attname) - for target_field in self.field.get_path_info()[-1].target_fields + for target_field in self.field.path_infos[-1].target_fields ]) else: rel_obj_id = getattr(self.instance, target_field.attname) diff --git a/django/db/models/fields/related_lookups.py b/django/db/models/fields/related_lookups.py index 34cca8ba5e..50f8b44158 100644 --- a/django/db/models/fields/related_lookups.py +++ b/django/db/models/fields/related_lookups.py @@ -30,7 +30,7 @@ def get_normalized_value(value, lhs): from django.db.models import Model if isinstance(value, Model): value_list = [] - sources = lhs.output_field.get_path_info()[-1].target_fields + sources = lhs.output_field.path_infos[-1].target_fields for source in sources: while not isinstance(value, source.model) and source.remote_field: source = source.remote_field.model._meta.get_field(source.remote_field.field_name) @@ -55,10 +55,10 @@ class RelatedIn(In): # ForeignKey to IntegerField given value 'abc'. The ForeignKey itself # doesn't have validation for non-integers, so we must run validation # using the target field. - if hasattr(self.lhs.output_field, 'get_path_info'): + if hasattr(self.lhs.output_field, 'path_infos'): # Run the target field's get_prep_value. We can safely assume there is # only one as we don't get to the direct value branch otherwise. - target_field = self.lhs.output_field.get_path_info()[-1].target_fields[-1] + target_field = self.lhs.output_field.path_infos[-1].target_fields[-1] self.rhs = [target_field.get_prep_value(v) for v in self.rhs] return super().get_prep_lookup() @@ -113,10 +113,10 @@ class RelatedLookupMixin: # ForeignKey to IntegerField given value 'abc'. The ForeignKey itself # doesn't have validation for non-integers, so we must run validation # using the target field. - if self.prepare_rhs and hasattr(self.lhs.output_field, 'get_path_info'): + if self.prepare_rhs and hasattr(self.lhs.output_field, 'path_infos'): # Get the target field. We can safely assume there is only one # as we don't get to the direct value branch otherwise. - target_field = self.lhs.output_field.get_path_info()[-1].target_fields[-1] + target_field = self.lhs.output_field.path_infos[-1].target_fields[-1] self.rhs = target_field.get_prep_value(self.rhs) return super().get_prep_lookup() diff --git a/django/db/models/fields/reverse_related.py b/django/db/models/fields/reverse_related.py index 65950590e2..6f0c788bbd 100644 --- a/django/db/models/fields/reverse_related.py +++ b/django/db/models/fields/reverse_related.py @@ -71,7 +71,7 @@ class ForeignObjectRel(FieldCacheMixin): When filtering against this relation, return the field on the remote model against which the filtering should happen. """ - target_fields = self.get_path_info()[-1].target_fields + target_fields = self.path_infos[-1].target_fields if len(target_fields) > 1: raise exceptions.FieldError("Can't use target_field for multicolumn relations.") return target_fields[0] @@ -138,6 +138,18 @@ class ForeignObjectRel(FieldCacheMixin): def __hash__(self): return hash(self.identity) + def __getstate__(self): + state = self.__dict__.copy() + # Delete the path_infos cached property because it can be recalculated + # at first invocation after deserialization. The attribute must be + # removed because subclasses like ManyToOneRel may have a PathInfo + # which contains an intermediate M2M table that's been dynamically + # created and doesn't exist in the .models module. + # This is a reverse relation, so there is no reverse_path_infos to + # delete. + state.pop('path_infos', None) + return state + def get_choices( self, include_blank=True, blank_choice=BLANK_CHOICE_DASH, limit_choices_to=None, ordering=(), @@ -195,7 +207,14 @@ class ForeignObjectRel(FieldCacheMixin): return opts.model_name + ('_set' if self.multiple else '') def get_path_info(self, filtered_relation=None): - return self.field.get_reverse_path_info(filtered_relation) + if filtered_relation: + return self.field.get_reverse_path_info(filtered_relation) + else: + return self.field.reverse_path_infos + + @cached_property + def path_infos(self): + return self.get_path_info() def get_cache_name(self): """ @@ -234,7 +253,7 @@ class ManyToOneRel(ForeignObjectRel): self.field_name = field_name def __getstate__(self): - state = self.__dict__.copy() + state = super().__getstate__() state.pop('related_model', None) return state diff --git a/django/db/models/options.py b/django/db/models/options.py index 0788ba3298..5f0f8f0e5f 100644 --- a/django/db/models/options.py +++ b/django/db/models/options.py @@ -702,7 +702,7 @@ class Options: for i, ancestor in enumerate(chain[:-1]): child = chain[i + 1] link = child._meta.get_ancestor_link(ancestor) - path.extend(link.get_reverse_path_info()) + path.extend(link.reverse_path_infos) return path def _populate_directed_relation_graph(self): diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py index 171e72cfb8..2c5f11cbbf 100644 --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -1528,8 +1528,11 @@ class Query(BaseExpression): path.extend(path_to_parent) cur_names_with_path[1].extend(path_to_parent) opts = path_to_parent[-1].to_opts - if hasattr(field, 'get_path_info'): - pathinfos = field.get_path_info(filtered_relation) + if hasattr(field, 'path_infos'): + if filtered_relation: + pathinfos = field.get_path_info(filtered_relation) + else: + pathinfos = field.path_infos if not allow_many: for inner_pos, p in enumerate(pathinfos): if p.m2m: diff --git a/tests/foreign_object/tests.py b/tests/foreign_object/tests.py index 72d50cad6b..8b61c87e5d 100644 --- a/tests/foreign_object/tests.py +++ b/tests/foreign_object/tests.py @@ -1,4 +1,6 @@ +import copy import datetime +import pickle from operator import attrgetter from django.core.exceptions import FieldError @@ -482,3 +484,104 @@ class TestExtraJoinFilterQ(TestCase): qs = qs.select_related('active_translation_q') with self.assertNumQueries(1): self.assertEqual(qs[0].active_translation_q.title, 'title') + + +class TestCachedPathInfo(TestCase): + def test_equality(self): + """ + The path_infos and reverse_path_infos attributes are equivalent to + calling the get_<method>() with no arguments. + """ + foreign_object = Membership._meta.get_field('person') + self.assertEqual( + foreign_object.path_infos, + foreign_object.get_path_info(), + ) + self.assertEqual( + foreign_object.reverse_path_infos, + foreign_object.get_reverse_path_info(), + ) + + def test_copy_removes_direct_cached_values(self): + """ + Shallow copying a ForeignObject (or a ForeignObjectRel) removes the + object's direct cached PathInfo values. + """ + foreign_object = Membership._meta.get_field('person') + # Trigger storage of cached_property into ForeignObject's __dict__. + foreign_object.path_infos + foreign_object.reverse_path_infos + # The ForeignObjectRel doesn't have reverse_path_infos. + foreign_object.remote_field.path_infos + self.assertIn('path_infos', foreign_object.__dict__) + self.assertIn('reverse_path_infos', foreign_object.__dict__) + self.assertIn('path_infos', foreign_object.remote_field.__dict__) + # Cached value is removed via __getstate__() on ForeignObjectRel + # because no __copy__() method exists, so __reduce_ex__() is used. + remote_field_copy = copy.copy(foreign_object.remote_field) + self.assertNotIn('path_infos', remote_field_copy.__dict__) + # Cached values are removed via __copy__() on ForeignObject for + # consistency of behavior. + foreign_object_copy = copy.copy(foreign_object) + self.assertNotIn('path_infos', foreign_object_copy.__dict__) + self.assertNotIn('reverse_path_infos', foreign_object_copy.__dict__) + # ForeignObjectRel's remains because it's part of a shallow copy. + self.assertIn('path_infos', foreign_object_copy.remote_field.__dict__) + + def test_deepcopy_removes_cached_values(self): + """ + Deep copying a ForeignObject removes the object's cached PathInfo + values, including those of the related ForeignObjectRel. + """ + foreign_object = Membership._meta.get_field('person') + # Trigger storage of cached_property into ForeignObject's __dict__. + foreign_object.path_infos + foreign_object.reverse_path_infos + # The ForeignObjectRel doesn't have reverse_path_infos. + foreign_object.remote_field.path_infos + self.assertIn('path_infos', foreign_object.__dict__) + self.assertIn('reverse_path_infos', foreign_object.__dict__) + self.assertIn('path_infos', foreign_object.remote_field.__dict__) + # Cached value is removed via __getstate__() on ForeignObjectRel + # because no __deepcopy__() method exists, so __reduce_ex__() is used. + remote_field_copy = copy.deepcopy(foreign_object.remote_field) + self.assertNotIn('path_infos', remote_field_copy.__dict__) + # Field.__deepcopy__() internally uses __copy__() on both the + # ForeignObject and ForeignObjectRel, so all cached values are removed. + foreign_object_copy = copy.deepcopy(foreign_object) + self.assertNotIn('path_infos', foreign_object_copy.__dict__) + self.assertNotIn('reverse_path_infos', foreign_object_copy.__dict__) + self.assertNotIn('path_infos', foreign_object_copy.remote_field.__dict__) + + def test_pickling_foreignobjectrel(self): + """ + Pickling a ForeignObjectRel removes the path_infos attribute. + + ForeignObjectRel implements __getstate__(), so copy and pickle modules + both use that, but ForeignObject implements __reduce__() and __copy__() + separately, so doesn't share the same behaviour. + """ + foreign_object_rel = Membership._meta.get_field('person').remote_field + # Trigger storage of cached_property into ForeignObjectRel's __dict__. + foreign_object_rel.path_infos + self.assertIn('path_infos', foreign_object_rel.__dict__) + foreign_object_rel_restored = pickle.loads(pickle.dumps(foreign_object_rel)) + self.assertNotIn('path_infos', foreign_object_rel_restored.__dict__) + + def test_pickling_foreignobject(self): + """ + Pickling a ForeignObject does not remove the cached PathInfo values. + + ForeignObject will always keep the path_infos and reverse_path_infos + attributes within the same process, because of the way + Field.__reduce__() is used for restoring values. + """ + foreign_object = Membership._meta.get_field('person') + # Trigger storage of cached_property into ForeignObjectRel's __dict__ + foreign_object.path_infos + foreign_object.reverse_path_infos + self.assertIn('path_infos', foreign_object.__dict__) + self.assertIn('reverse_path_infos', foreign_object.__dict__) + foreign_object_restored = pickle.loads(pickle.dumps(foreign_object)) + self.assertIn('path_infos', foreign_object_restored.__dict__) + self.assertIn('reverse_path_infos', foreign_object_restored.__dict__) |