From 47a47a9b5d8e019e5cdce87c2868b82ea54ad405 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 4 Jun 2025 21:05:50 +0530 Subject: [PATCH] refactor!: Change internal datastructure of db.value_cache It's now a defaultdictionary of `[doctype][name/filters][fieldname]` This allows us to implement granular clearing and improve usage of this cache. --- frappe/database/database.py | 45 ++++++++++--------- frappe/deprecation_dumpster.py | 2 +- frappe/model/document.py | 2 +- frappe/tests/classes/context_managers.py | 2 - frappe/tests/classes/integration_test_case.py | 2 +- frappe/tests/test_website.py | 1 - frappe/utils/data.py | 6 +++ .../doctype/blog_post/test_blog_post.py | 4 -- 8 files changed, 32 insertions(+), 32 deletions(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index ce4326b4fb..18ab1d2c94 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -34,7 +34,16 @@ from frappe.exceptions import DoesNotExistError, ImplicitCommitError from frappe.monitor import get_trace_id from frappe.query_builder import Case from frappe.query_builder.functions import Count -from frappe.utils import CallbackManager, cint, get_datetime, get_table_name, getdate, now, sbool +from frappe.utils import ( + CallbackManager, + cint, + get_datetime, + get_table_name, + getdate, + now, + recursive_defaultdict, + sbool, +) from frappe.utils import cast as cast_fieldtype if TYPE_CHECKING: @@ -112,7 +121,7 @@ class Database: self.transaction_writes = 0 self.auto_commit_on_many_writes = 0 - self.value_cache = {} + self.value_cache = recursive_defaultdict() self.logger = frappe.logger("database") self.logger.setLevel("WARNING") @@ -615,11 +624,8 @@ class Database: user = frappe.db.get_values("User", "test@example.com", "*")[0] """ out = None - cache_key = None - if cache and isinstance(filters, str): - cache_key = (doctype, filters, fieldname) - if cache_key in self.value_cache: - return self.value_cache[cache_key] + if cache and isinstance(filters, str) and fieldname in self.value_cache[doctype][filters]: + return self.value_cache[doctype][filters][fieldname] if distinct: order_by = None @@ -701,8 +707,8 @@ class Database: distinct=distinct, ) - if cache and cache_key: - self.value_cache[cache_key] = out + if cache and isinstance(filters, str): + self.value_cache[doctype][filters][fieldname] = out return out @@ -858,8 +864,7 @@ class Database: frappe.qb.into("Singles").columns("doctype", "field", "value").insert(*singles_data).run(debug=debug) frappe.clear_document_cache(doctype, doctype) - if doctype in self.value_cache: - del self.value_cache[doctype] + self.value_cache.pop(doctype, None) def get_single_value(self, doctype: str, fieldname: str, cache: bool = True): """Get property of Single DocType. Cache locally by default @@ -872,10 +877,6 @@ class Database: # Get the default value of the company from the Global Defaults doctype. company = frappe.db.get_single_value('Global Defaults', 'default_company') """ - - if doctype not in self.value_cache: - self.value_cache[doctype] = {} - if cache and fieldname in self.value_cache[doctype]: return self.value_cache[doctype][fieldname] @@ -975,8 +976,7 @@ class Database: query.run(debug=debug) - if dt in self.value_cache: - del self.value_cache[dt] + self.value_cache.pop(dt, None) def bulk_update( self, @@ -1252,10 +1252,10 @@ class Database: def count(self, dt, filters=None, debug=False, cache=False, distinct: bool = True): """Return `COUNT(*)` for given DocType and filters.""" - cache_key = (dt, "COUNT(*)") - if cache and not filters: - if cache_key in self.value_cache: - return self.value_cache[cache_key] + cache_key = "COUNT(*)" + if cache and not filters and cache_key in self.value_cache[dt]: + return self.value_cache[dt][cache_key] + count = frappe.qb.get_query( table=dt, filters=filters, @@ -1263,8 +1263,9 @@ class Database: distinct=distinct, validate_filters=True, ).run(debug=debug)[0][0] + if not filters and cache: - self.value_cache[cache_key] = count + self.value_cache[dt][cache_key] = count return count def estimate_count(self, doctype: str) -> int: diff --git a/frappe/deprecation_dumpster.py b/frappe/deprecation_dumpster.py index 686fbc57c1..91aac5526f 100644 --- a/frappe/deprecation_dumpster.py +++ b/frappe/deprecation_dumpster.py @@ -576,7 +576,7 @@ def get_tests_CompatFrappeTestCase(): traceback.print_stack(limit=10) def _rollback_db(): - frappe.db.value_cache = {} + frappe.db.value_cache.clear() frappe.db.rollback() def _restore_thread_locals(flags): diff --git a/frappe/model/document.py b/frappe/model/document.py index d0061b8bfb..b55fdafd2a 100644 --- a/frappe/model/document.py +++ b/frappe/model/document.py @@ -706,7 +706,7 @@ class Document(BaseDocument, DocRef): ) if self.doctype in frappe.db.value_cache: - del frappe.db.value_cache[self.doctype] + frappe.db.value_cache.pop(self.doctype, None) def set_user_and_timestamp(self): self._original_modified = self.modified diff --git a/frappe/tests/classes/context_managers.py b/frappe/tests/classes/context_managers.py index c9c1b0024d..4ee9fd628e 100644 --- a/frappe/tests/classes/context_managers.py +++ b/frappe/tests/classes/context_managers.py @@ -88,8 +88,6 @@ def change_settings(doctype, settings_dict=None, /, commit=False, **settings) -> for key, value in settings_dict.items(): setattr(settings, key, value) settings.save(ignore_permissions=True) - # singles are cached by default, clear to avoid flake - frappe.db.value_cache[settings] = {} if commit: frappe.db.commit() yield diff --git a/frappe/tests/classes/integration_test_case.py b/frappe/tests/classes/integration_test_case.py index a089f2e0cd..ecb2a47e29 100644 --- a/frappe/tests/classes/integration_test_case.py +++ b/frappe/tests/classes/integration_test_case.py @@ -183,7 +183,7 @@ def _commit_watcher(): def _rollback_db(): - frappe.db.value_cache = {} + frappe.db.value_cache.clear() frappe.db.rollback() diff --git a/frappe/tests/test_website.py b/frappe/tests/test_website.py index 1309579f36..47c9d7548b 100644 --- a/frappe/tests/test_website.py +++ b/frappe/tests/test_website.py @@ -252,7 +252,6 @@ class TestWebsite(IntegrationTestCase): self.assertEqual(response.status_code, 404) def test_printview_page(self): - frappe.db.value_cache[("DocType", "Language", "name")] = (("Language",),) frappe.set_user("Administrator") content = get_response_content("/Language/ru") self.assertIn('
', content) diff --git a/frappe/utils/data.py b/frappe/utils/data.py index 94fa3c642e..12dbe8cbb3 100644 --- a/frappe/utils/data.py +++ b/frappe/utils/data.py @@ -12,6 +12,7 @@ import re import time import typing from code import compile_command +from collections import defaultdict from enum import Enum from functools import lru_cache from typing import Any, Literal, Optional, TypeVar @@ -2711,6 +2712,11 @@ def mock(type, size=1, locale="en"): return squashify(results) +# Recursive default dict with arbitrary levels of nesting +def recursive_defaultdict(): + return defaultdict(recursive_defaultdict) + + # This is used in test to count memory overhead of default imports. def _get_rss_memory_usage(): import psutil diff --git a/frappe/website/doctype/blog_post/test_blog_post.py b/frappe/website/doctype/blog_post/test_blog_post.py index ec9698bb6d..8697d0f9db 100644 --- a/frappe/website/doctype/blog_post/test_blog_post.py +++ b/frappe/website/doctype/blog_post/test_blog_post.py @@ -74,9 +74,6 @@ class TestBlogPost(IntegrationTestCase): category_page_link = next(iter(soup.find_all("a", href=re.compile(blog.blog_category)))) category_page_url = category_page_link["href"] - cached_value = frappe.db.value_cache.get(("DocType", "Blog Post", "name")) - frappe.db.value_cache[("DocType", "Blog Post", "name")] = (("Blog Post",),) - # Visit the category page (by following the link found in above stage) set_request(path=category_page_url) category_page_response = get_response() @@ -85,7 +82,6 @@ class TestBlogPost(IntegrationTestCase): self.assertIn(blog.title, category_page_html) # Cleanup - frappe.db.value_cache[("DocType", "Blog Post", "name")] = cached_value frappe.delete_doc("Blog Post", blog.name) frappe.delete_doc("Blog Category", blog.blog_category)