summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAdam Johnson <me@adamj.eu>2024-05-03 17:18:06 +0100
committernessita <124304+nessita@users.noreply.github.com>2024-05-21 16:19:29 -0300
commitb9838c65ec2fee36c1fd0d46494ba44da27a9b34 (patch)
tree27ce165d7e8db3efc34204170abc320c8f284c25
parent15fff62d5d8f28b6b61d63341535c9aafc64c865 (diff)
Fixed #35405 -- Converted get_cache_name into a cached property in FieldCacheMixin.
FieldCacheMixin is used by related fields to track their cached values. This work migrates get_cache_name() to be a cached property to optimize performance by reducing unnecessary function calls when working with related fields, given that its value remains constant. Co-authored-by: Simon Charette <charette.s@gmail.com> Co-authored-by: Sarah Boyce <42296566+sarahboyce@users.noreply.github.com> Co-authored-by: Natalia <124304+nessita@users.noreply.github.com>
-rw-r--r--django/contrib/contenttypes/fields.py3
-rw-r--r--django/db/models/fields/mixins.py33
-rw-r--r--django/db/models/fields/related.py3
-rw-r--r--django/db/models/fields/related_descriptors.py10
-rw-r--r--django/db/models/fields/reverse_related.py3
-rw-r--r--docs/internals/deprecation.txt2
-rw-r--r--docs/releases/5.1.txt2
-rw-r--r--tests/model_fields/test_mixins.py75
8 files changed, 117 insertions, 14 deletions
diff --git a/django/contrib/contenttypes/fields.py b/django/contrib/contenttypes/fields.py
index 770f88265c..a3e87f6ed4 100644
--- a/django/contrib/contenttypes/fields.py
+++ b/django/contrib/contenttypes/fields.py
@@ -140,7 +140,8 @@ class GenericForeignKey(FieldCacheMixin, Field):
else:
return []
- def get_cache_name(self):
+ @cached_property
+ def cache_name(self):
return self.name
def get_content_type(self, obj=None, id=None, using=None, model=None):
diff --git a/django/db/models/fields/mixins.py b/django/db/models/fields/mixins.py
index e7f282210e..9f2809dfc8 100644
--- a/django/db/models/fields/mixins.py
+++ b/django/db/models/fields/mixins.py
@@ -1,31 +1,52 @@
+import warnings
+
from django.core import checks
+from django.utils.deprecation import RemovedInDjango60Warning
+from django.utils.functional import cached_property
NOT_PROVIDED = object()
class FieldCacheMixin:
- """Provide an API for working with the model's fields value cache."""
+ """
+ An API for working with the model's fields value cache.
+
+ Subclasses must set self.cache_name to a unique entry for the cache -
+ typically the field’s name.
+ """
+ # RemovedInDjango60Warning.
def get_cache_name(self):
raise NotImplementedError
- def get_cached_value(self, instance, default=NOT_PROVIDED):
+ @cached_property
+ def cache_name(self):
+ # RemovedInDjango60Warning: when the deprecation ends, replace with:
+ # raise NotImplementedError
cache_name = self.get_cache_name()
+ warnings.warn(
+ f"Override {self.__class__.__qualname__}.cache_name instead of "
+ "get_cache_name().",
+ RemovedInDjango60Warning,
+ )
+ return cache_name
+
+ def get_cached_value(self, instance, default=NOT_PROVIDED):
try:
- return instance._state.fields_cache[cache_name]
+ return instance._state.fields_cache[self.cache_name]
except KeyError:
if default is NOT_PROVIDED:
raise
return default
def is_cached(self, instance):
- return self.get_cache_name() in instance._state.fields_cache
+ return self.cache_name in instance._state.fields_cache
def set_cached_value(self, instance, value):
- instance._state.fields_cache[self.get_cache_name()] = value
+ instance._state.fields_cache[self.cache_name] = value
def delete_cached_value(self, instance):
- del instance._state.fields_cache[self.get_cache_name()]
+ del instance._state.fields_cache[self.cache_name]
class CheckFieldDefaultMixin:
diff --git a/django/db/models/fields/related.py b/django/db/models/fields/related.py
index 3e4bfe34c1..7d42d1ea38 100644
--- a/django/db/models/fields/related.py
+++ b/django/db/models/fields/related.py
@@ -509,7 +509,8 @@ class RelatedField(FieldCacheMixin, Field):
)
return target_fields[0]
- def get_cache_name(self):
+ @cached_property
+ def cache_name(self):
return self.name
diff --git a/django/db/models/fields/related_descriptors.py b/django/db/models/fields/related_descriptors.py
index c7848ee63a..bc288c47ec 100644
--- a/django/db/models/fields/related_descriptors.py
+++ b/django/db/models/fields/related_descriptors.py
@@ -210,7 +210,7 @@ class ForwardManyToOneDescriptor:
rel_obj_attr,
instance_attr,
True,
- self.field.get_cache_name(),
+ self.field.cache_name,
False,
)
@@ -486,7 +486,7 @@ class ReverseOneToOneDescriptor:
rel_obj_attr,
instance_attr,
True,
- self.related.get_cache_name(),
+ self.related.cache_name,
False,
)
@@ -744,7 +744,7 @@ def create_reverse_many_to_one_manager(superclass, rel):
def _remove_prefetched_objects(self):
try:
self.instance._prefetched_objects_cache.pop(
- self.field.remote_field.get_cache_name()
+ self.field.remote_field.cache_name
)
except (AttributeError, KeyError):
pass # nothing to clear from cache
@@ -760,7 +760,7 @@ def create_reverse_many_to_one_manager(superclass, rel):
)
try:
return self.instance._prefetched_objects_cache[
- self.field.remote_field.get_cache_name()
+ self.field.remote_field.cache_name
]
except (AttributeError, KeyError):
queryset = super().get_queryset()
@@ -798,7 +798,7 @@ def create_reverse_many_to_one_manager(superclass, rel):
if not self.field.is_cached(rel_obj):
instance = instances_dict[rel_obj_attr(rel_obj)]
setattr(rel_obj, self.field.name, instance)
- cache_name = self.field.remote_field.get_cache_name()
+ cache_name = self.field.remote_field.cache_name
return queryset, rel_obj_attr, instance_attr, False, cache_name, False
def add(self, *objs, bulk=True):
diff --git a/django/db/models/fields/reverse_related.py b/django/db/models/fields/reverse_related.py
index c9f9232ff7..a39bfd07da 100644
--- a/django/db/models/fields/reverse_related.py
+++ b/django/db/models/fields/reverse_related.py
@@ -248,7 +248,8 @@ class ForeignObjectRel(FieldCacheMixin):
def path_infos(self):
return self.get_path_info()
- def get_cache_name(self):
+ @cached_property
+ def cache_name(self):
"""
Return the name of the cache key to use for storing an instance of the
forward model on the reverse model.
diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt
index 4f89481ac7..1a74a2a46b 100644
--- a/docs/internals/deprecation.txt
+++ b/docs/internals/deprecation.txt
@@ -82,6 +82,8 @@ details on these changes.
* The ``OS_OPEN_FLAGS`` attribute of
:class:`~django.core.files.storage.FileSystemStorage` will be removed.
+* The ``get_cache_name()`` method of ``FieldCacheMixin`` will be removed.
+
.. _deprecation-removed-in-5.1:
5.1
diff --git a/docs/releases/5.1.txt b/docs/releases/5.1.txt
index 388487b322..bb5e4f3fe4 100644
--- a/docs/releases/5.1.txt
+++ b/docs/releases/5.1.txt
@@ -474,6 +474,8 @@ Miscellaneous
overwriting files in storage, set the new
:attr:`~django.core.files.storage.FileSystemStorage.allow_overwrite` option
to ``True`` instead.
+* The ``get_cache_name()`` method of ``FieldCacheMixin`` is deprecated in favor
+ of the ``cache_name`` cached property.
Features removed in 5.1
=======================
diff --git a/tests/model_fields/test_mixins.py b/tests/model_fields/test_mixins.py
new file mode 100644
index 0000000000..5ccfac4d78
--- /dev/null
+++ b/tests/model_fields/test_mixins.py
@@ -0,0 +1,75 @@
+from django.db.models.fields.mixins import FieldCacheMixin
+from django.test import SimpleTestCase
+from django.utils.deprecation import RemovedInDjango60Warning
+from django.utils.functional import cached_property
+
+from .models import Foo
+
+
+# RemovedInDjango60Warning.
+class ExampleOld(FieldCacheMixin):
+ def get_cache_name(self):
+ return "example"
+
+
+class Example(FieldCacheMixin):
+ @cached_property
+ def cache_name(self):
+ return "example"
+
+
+class FieldCacheMixinTests(SimpleTestCase):
+ def setUp(self):
+ self.instance = Foo()
+ self.field = Example()
+
+ # RemovedInDjango60Warning: when the deprecation ends, replace with:
+ # def test_cache_name_not_implemented(self):
+ # with self.assertRaises(NotImplementedError):
+ # FieldCacheMixin().cache_name
+ def test_get_cache_name_not_implemented(self):
+ with self.assertRaises(NotImplementedError):
+ FieldCacheMixin().get_cache_name()
+
+ # RemovedInDjango60Warning.
+ def test_get_cache_name_deprecated(self):
+ msg = "Override ExampleOld.cache_name instead of get_cache_name()."
+ with self.assertWarnsMessage(RemovedInDjango60Warning, msg):
+ result = ExampleOld().cache_name
+ self.assertEqual(result, "example")
+
+ def test_cache_name(self):
+ result = Example().cache_name
+ self.assertEqual(result, "example")
+
+ def test_get_cached_value_missing(self):
+ with self.assertRaises(KeyError):
+ self.field.get_cached_value(self.instance)
+
+ def test_get_cached_value_default(self):
+ default = object()
+ result = self.field.get_cached_value(self.instance, default=default)
+ self.assertIs(result, default)
+
+ def test_get_cached_value_after_set(self):
+ value = object()
+
+ self.field.set_cached_value(self.instance, value)
+ result = self.field.get_cached_value(self.instance)
+
+ self.assertIs(result, value)
+
+ def test_is_cached_false(self):
+ result = self.field.is_cached(self.instance)
+ self.assertFalse(result)
+
+ def test_is_cached_true(self):
+ self.field.set_cached_value(self.instance, 1)
+ result = self.field.is_cached(self.instance)
+ self.assertTrue(result)
+
+ def test_delete_cached_value(self):
+ self.field.set_cached_value(self.instance, 1)
+ self.field.delete_cached_value(self.instance)
+ result = self.field.is_cached(self.instance)
+ self.assertFalse(result)