Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed #35405 -- Used @cached_property in FieldCacheMixin. #18101

Merged
merged 1 commit into from
May 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion django/contrib/contenttypes/fields.py
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions docs/releases/5.1.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
=======================
Expand Down
75 changes: 75 additions & 0 deletions tests/model_fields/test_mixins.py
Original file line number Diff line number Diff line change
@@ -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):
sarahboyce marked this conversation as resolved.
Show resolved Hide resolved
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)