Skip to content

Commit

Permalink
Fixed #35405 -- Used @cached_property in FieldCacheMixin.
Browse files Browse the repository at this point in the history
co-authored-by: Simon Charette <charette.s@gmail.com>
co-authored-by: Natalia <124304+nessita@users.noreply.github.com>
  • Loading branch information
3 people committed May 3, 2024
1 parent ec85524 commit 644eb89
Show file tree
Hide file tree
Showing 8 changed files with 112 additions and 14 deletions.
3 changes: 2 additions & 1 deletion django/contrib/contenttypes/fields.py
Expand Up @@ -140,7 +140,8 @@ def _check_content_type_field(self):
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):
Expand Down
33 changes: 27 additions & 6 deletions 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:
Expand Down
3 changes: 2 additions & 1 deletion django/db/models/fields/related.py
Expand Up @@ -509,7 +509,8 @@ def target_field(self):
)
return target_fields[0]

def get_cache_name(self):
@cached_property
def cache_name(self):
return self.name


Expand Down
10 changes: 5 additions & 5 deletions django/db/models/fields/related_descriptors.py
Expand Up @@ -210,7 +210,7 @@ def get_prefetch_querysets(self, instances, querysets=None):
rel_obj_attr,
instance_attr,
True,
self.field.get_cache_name(),
self.field.cache_name,
False,
)

Expand Down Expand Up @@ -486,7 +486,7 @@ def get_prefetch_querysets(self, instances, querysets=None):
rel_obj_attr,
instance_attr,
True,
self.related.get_cache_name(),
self.related.cache_name,
False,
)

Expand Down Expand Up @@ -744,7 +744,7 @@ def _apply_rel_filters(self, queryset):
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
Expand All @@ -760,7 +760,7 @@ def get_queryset(self):
)
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()
Expand Down Expand Up @@ -798,7 +798,7 @@ def get_prefetch_querysets(self, instances, querysets=None):
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):
Expand Down
3 changes: 2 additions & 1 deletion django/db/models/fields/reverse_related.py
Expand Up @@ -248,7 +248,8 @@ def get_path_info(self, filtered_relation=None):
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.
Expand Down
2 changes: 2 additions & 0 deletions docs/internals/deprecation.txt
Expand Up @@ -79,6 +79,8 @@ details on these changes.

* The ``check`` keyword argument of ``CheckConstraint`` will be removed.

* The ``get_cache_name()`` method of ``FieldCacheMixin`` will be removed.

.. _deprecation-removed-in-5.1:

5.1
Expand Down
3 changes: 3 additions & 0 deletions docs/releases/5.1.txt
Expand Up @@ -454,6 +454,9 @@ Miscellaneous
* The ``check`` keyword argument of ``CheckConstraint`` is deprecated in favor
of ``condition``.

* The ``get_cache_name()`` method of ``FieldCacheMixin`` is deprecated in favor
of the ``cache_name`` cached property.

Features removed in 5.1
=======================

Expand Down
69 changes: 69 additions & 0 deletions tests/model_fields/test_mixins.py
@@ -0,0 +1,69 @@
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


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()

def test_get_cache_name_not_implemented(self):
with self.assertRaises(NotImplementedError):
FieldCacheMixin().get_cache_name()

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)

0 comments on commit 644eb89

Please sign in to comment.