diff --git a/frappe/tests/test_caching.py b/frappe/tests/test_caching.py index c390a7e500..90cb818a37 100644 --- a/frappe/tests/test_caching.py +++ b/frappe/tests/test_caching.py @@ -110,6 +110,7 @@ class TestRedisCache(FrappeAPITestCase): self.assertEqual(function_call_count, 1) time.sleep(CACHE_TTL * 1.5) + frappe.local.cache.clear() self.assertEqual(calculate_area(10), 314) self.assertEqual(function_call_count, 2) diff --git a/frappe/tests/test_perf.py b/frappe/tests/test_perf.py index d441ac65ce..ecc511abc7 100644 --- a/frappe/tests/test_perf.py +++ b/frappe/tests/test_perf.py @@ -32,6 +32,7 @@ from frappe.tests import IntegrationTestCase from frappe.tests.test_api import FrappeAPITestCase from frappe.tests.test_query_builder import run_only_if from frappe.utils import cint +from frappe.utils.caching import redis_cache from frappe.website.path_resolver import PathResolver TEST_USER = "test@example.com" @@ -201,9 +202,26 @@ class TestPerformance(IntegrationTestCase): def test_get_doc_cache_calls(self): frappe.get_doc("User", "Administrator") - with self.assertRedisCallCounts(1): + with self.assertRedisCallCounts(0): frappe.get_doc("User", "Administrator") + def test_local_caching(self): + frappe.get_cached_doc("User", "Administrator") + with self.assertRedisCallCounts(0): + frappe.get_cached_doc("User", "Administrator") + + def test_redis_cache_calls(self): + redis_cached_func() # warmup + + # Repeat call should use locally cached value + with self.assertRedisCallCounts(0): + redis_cached_func() + + frappe.local.cache.clear() + # Without local cache - only one call required + with self.assertRedisCallCounts(1): + redis_cached_func() + def test_one_time_setup(self): site = frappe.local.site frappe.init(site, force=True) @@ -245,3 +263,8 @@ class TestOverheadCalls(FrappeAPITestCase): self.get(self.resource("User", "Administrator"), {"sid": sid}) with self.assertRedisCallCounts(19), self.assertQueryCount(self.BASE_SQL_CALLS + 1 + tables): self.get(self.resource("User", "Administrator"), {"sid": sid}) + + +@redis_cache +def redis_cached_func(): + return 42 diff --git a/frappe/utils/caching.py b/frappe/utils/caching.py index 70da13f079..05c78d5a31 100644 --- a/frappe/utils/caching.py +++ b/frappe/utils/caching.py @@ -164,8 +164,15 @@ def redis_cache(ttl: int | None = 3600, user: str | bool | None = None, shared: @wraps(func) def redis_cache_wrapper(*args, **kwargs): func_call_key = func_key + "::" + str(__generate_request_cache_key(args, kwargs)) + cached_val = frappe.cache.get_value(func_call_key, user=user, shared=shared) + if cached_val is not None: + return cached_val + + # Edge Case: None can mean two things: cache miss or the result itself is `None` + # RedisWrapper doesn't give us any way to handle this cleanly. if frappe.cache.exists(func_call_key, user=user, shared=shared): - return frappe.cache.get_value(func_call_key, user=user, shared=shared) + return None + val = func(*args, **kwargs) ttl = getattr(func, "ttl", 3600) frappe.cache.set_value(func_call_key, val, expires_in_sec=ttl, user=user, shared=shared) diff --git a/frappe/utils/redis_wrapper.py b/frappe/utils/redis_wrapper.py index 7d127d0843..023c57f09b 100644 --- a/frappe/utils/redis_wrapper.py +++ b/frappe/utils/redis_wrapper.py @@ -60,8 +60,7 @@ class RedisWrapper(redis.Redis): """ key = self.make_key(key, user, shared) - if not expires_in_sec: - frappe.local.cache[key] = val + frappe.local.cache[key] = val with suppress(redis.exceptions.ConnectionError): self.set(name=key, value=pickle.dumps(val), ex=expires_in_sec)