Skip to content

Commit

Permalink
Merge pull request frappe#28992 from ankush/perf/client_side_redis_cache
Browse files Browse the repository at this point in the history
feat(cache): server-assisted client-side caching
  • Loading branch information
ankush authored Jan 7, 2025
2 parents 146adbe + 06816f2 commit 939f832
Show file tree
Hide file tree
Showing 22 changed files with 441 additions and 117 deletions.
47 changes: 21 additions & 26 deletions frappe/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,12 @@
from frappe.model.document import Document
from frappe.query_builder.builder import MariaDB, Postgres
from frappe.types.lazytranslatedstring import _LazyTranslate
from frappe.utils.redis_wrapper import RedisWrapper
from frappe.utils.redis_wrapper import ClientCache, RedisWrapper

controllers: dict[str, "Document"] = {}
local = Local()
cache: Optional["RedisWrapper"] = None
client_cache: Optional["ClientCache"] = None
STANDARD_USERS = ("Guest", "Administrator")

_one_time_setup: dict[str, bool] = {}
Expand Down Expand Up @@ -397,14 +398,16 @@ def destroy():

def setup_redis_cache_connection():
"""Defines `frappe.cache` as `RedisWrapper` instance"""
from frappe.utils.redis_wrapper import setup_cache
from frappe.utils.redis_wrapper import ClientCache, setup_cache

global cache
global client_cache

with _redis_init_lock:
# We need to check again since someone else might have setup connection before us.
if not cache:
cache = setup_cache()
client_cache = ClientCache()


def get_traceback(with_context: bool = False) -> str:
Expand Down Expand Up @@ -970,6 +973,7 @@ def clear_cache(user: str | None = None, doctype: str | None = None):

if (not doctype and not user) or doctype == "DocType":
frappe.utils.caching._SITE_CACHE.clear()
frappe.client_cache.clear_cache()

local.role_permissions = {}
if hasattr(local, "request_cache"):
Expand Down Expand Up @@ -1079,11 +1083,11 @@ def has_website_permission(doc=None, ptype="read", user=None, verbose=False, doc

def is_table(doctype: str) -> bool:
"""Return True if `istable` property (indicating child Table) is set for given DocType."""

def get_tables():
return db.get_values("DocType", filters={"istable": 1}, order_by=None, pluck=True)

tables = cache.get_value("is_table", get_tables)
key = "is_table"
tables = client_cache.get_value(key)
if tables is None:
tables = db.get_values("DocType", filters={"istable": 1}, order_by=None, pluck=True)
client_cache.set_value(key, tables)
return doctype in tables


Expand Down Expand Up @@ -1588,16 +1592,18 @@ def get_hooks(
:param app_name: Filter by app."""

if app_name:
hooks = _dict(_load_app_hooks(app_name))
hooks = _load_app_hooks(app_name)
else:
if conf.developer_mode:
hooks = _dict(_load_app_hooks())
hooks = _load_app_hooks()
else:
hooks = _dict(cache.get_value("app_hooks", _load_app_hooks))

hooks = client_cache.get_value("app_hooks")
if hooks is None:
hooks = _load_app_hooks()
client_cache.set_value("app_hooks", hooks)
if hook:
return hooks.get(hook, ([] if default == "_KEEP_DEFAULT_LIST" else default))
return hooks
return _dict(hooks)


def append_hook(target, key, value):
Expand Down Expand Up @@ -1631,7 +1637,7 @@ def setup_module_map(include_all_apps: bool = True) -> None:
if include_all_apps:
local.app_modules = cache.get_value("app_modules")
else:
local.app_modules = cache.get_value("installed_app_modules")
local.app_modules = client_cache.get_value("installed_app_modules")

if not local.app_modules:
local.app_modules = {}
Expand All @@ -1649,7 +1655,7 @@ def setup_module_map(include_all_apps: bool = True) -> None:
if include_all_apps:
cache.set_value("app_modules", local.app_modules)
else:
cache.set_value("installed_app_modules", local.app_modules)
client_cache.set_value("installed_app_modules", local.app_modules)

# Init module_app (reverse mapping)
local.module_app = {}
Expand Down Expand Up @@ -2340,18 +2346,6 @@ def get_website_settings(key):
return local.website_settings.get(key)


def get_system_settings(key: str):
"""Return the value associated with the given `key` from System Settings DocType."""
if not (system_settings := getattr(local, "system_settings", None)):
try:
local.system_settings = system_settings = get_cached_doc("System Settings")
except DoesNotExistError: # possible during new install
clear_last_message()
return

return system_settings.get(key)


def get_active_domains():
from frappe.core.doctype.domain_settings.domain_settings import get_active_domains

Expand Down Expand Up @@ -2487,6 +2481,7 @@ def wrapper(*args, **kwargs):

# Backward compatibility
from frappe.config import get_common_site_config, get_site_config
from frappe.core.doctype.system_settings.system_settings import get_system_settings
from frappe.utils.error import log_error

frappe._optimizations.optimize_all()
12 changes: 3 additions & 9 deletions frappe/api/v2.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
"""

import json
from contextlib import suppress
from typing import Any

from werkzeug.routing import Rule
Expand Down Expand Up @@ -39,15 +38,10 @@ def handle_rpc_call(method: str, doctype: str | None = None):
method = hook
break

method_exists = False
with suppress(Exception):
method_exists = bool(frappe.get_attr(method))

# via server script
if not method_exists:
server_script = get_server_script_map().get("_api", {}).get(method)
if server_script:
return run_server_script(server_script)
server_script = get_server_script_map().get("_api", {}).get(method)
if server_script:
return run_server_script(server_script)

try:
method = frappe.get_attr(method)
Expand Down
9 changes: 6 additions & 3 deletions frappe/cache_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ def get_doctype_map_key(doctype):
)

doctype_cache_keys = (
"doctype_meta",
"doctype_form_meta",
"table_columns",
"last_modified",
Expand Down Expand Up @@ -110,9 +109,10 @@ def clear_global_cache():

def clear_defaults_cache(user=None):
if user:
frappe.cache.hdel("defaults", [user, *common_default_keys])
for key in [user, *common_default_keys]:
frappe.client_cache.delete_value(f"defaults::{key}")
elif frappe.flags.in_install != "frappe":
frappe.cache.delete_value("defaults")
frappe.client_cache.delete_keys("defaults::*")


def clear_doctype_cache(doctype=None):
Expand All @@ -126,6 +126,7 @@ def clear_doctype_cache(doctype=None):

def _clear_doctype_cache_from_redis(doctype: str | None = None):
from frappe.desk.notifications import delete_notification_count_for
from frappe.model.meta import clear_meta_cache

to_del = ["is_table", "doctype_modules"]

Expand All @@ -134,6 +135,7 @@ def _clear_doctype_cache_from_redis(doctype: str | None = None):
def clear_single(dt):
frappe.clear_document_cache(dt)
frappe.cache.hdel_names(doctype_cache_keys, dt)
clear_meta_cache(dt)

clear_single(doctype)

Expand All @@ -157,6 +159,7 @@ def clear_single(dt):
# clear all
to_del += doctype_cache_keys
to_del += frappe.cache.get_keys("document_cache::")
clear_meta_cache()

frappe.cache.delete_value(to_del)

Expand Down
2 changes: 1 addition & 1 deletion frappe/core/doctype/language/language.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ def before_rename(self, old, new, merge=False):

def on_update(self):
frappe.cache.delete_value("languages_with_name")
frappe.cache.delete_value("languages")
frappe.client_cache.delete_value("languages")
self.update_user_defaults()

def update_user_defaults(self):
Expand Down
4 changes: 2 additions & 2 deletions frappe/core/doctype/server_script/server_script.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,11 @@ def on_update(self):
self.sync_scheduled_job_type()

def clear_cache(self):
frappe.cache.delete_value("server_script_map")
frappe.client_cache.delete_value("server_script_map")
return super().clear_cache()

def on_trash(self):
frappe.cache.delete_value("server_script_map")
frappe.client_cache.delete_value("server_script_map")
if self.script_type == "Scheduler Event":
for job in self.scheduled_jobs:
scheduled_job_type: ScheduledJobType = frappe.get_doc("Scheduled Job Type", job.name)
Expand Down
4 changes: 2 additions & 2 deletions frappe/core/doctype/server_script/server_script_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ def get_server_script_map():
if frappe.flags.in_patch and not frappe.db.table_exists("Server Script"):
return {}

script_map = frappe.cache.get_value("server_script_map")
script_map = frappe.client_cache.get_value("server_script_map")
if script_map is None:
script_map = {"permission_query": {}}
enabled_server_scripts = frappe.get_all(
Expand All @@ -83,6 +83,6 @@ def get_server_script_map():
else:
script_map.setdefault("_api", {})[script.api_method] = script.name

frappe.cache.set_value("server_script_map", script_map)
frappe.client_cache.set_value("server_script_map", script_map)

return script_map
4 changes: 2 additions & 2 deletions frappe/core/doctype/server_script/test_server_script.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,10 +135,10 @@ def setUpClass(cls):
def tearDownClass(cls):
frappe.db.commit()
frappe.db.truncate("Server Script")
frappe.cache.delete_value("server_script_map")
frappe.client_cache.delete_value("server_script_map")

def setUp(self):
frappe.cache.delete_value("server_script_map")
frappe.client_cache.delete_value("server_script_map")

def test_doctype_event(self):
todo = frappe.get_doc(doctype="ToDo", description="hello").insert()
Expand Down
29 changes: 26 additions & 3 deletions frappe/core/doctype/system_settings/system_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,9 +176,7 @@ def validate_file_extensions(self):

def on_update(self):
self.set_defaults()

frappe.cache.delete_value("system_settings")
frappe.cache.delete_value("time_zone")
clear_system_settings_cache()

if frappe.flags.update_last_reset_password_date:
update_last_reset_password_date()
Expand Down Expand Up @@ -220,3 +218,28 @@ def load():
defaults[df.fieldname] = all_defaults.get(df.fieldname)

return {"timezones": get_all_timezones(), "defaults": defaults}


cache_key = frappe.get_document_cache_key("System Settings", "System Settings")


def get_system_settings(key: str):
"""Return the value associated with the given `key` from System Settings DocType."""
if not (system_settings := getattr(frappe.local, "system_settings", None)):
try:
system_settings = frappe.client_cache.get_value(cache_key)
if not system_settings:
system_settings = frappe.get_doc("System Settings")
frappe.client_cache.set_value(cache_key, system_settings)
frappe.local.system_settings = system_settings
except frappe.DoesNotExistError: # possible during new install
frappe.clear_last_message()
return

return system_settings.get(key)


def clear_system_settings_cache():
frappe.client_cache.delete_value(cache_key)
frappe.cache.delete_value("system_settings")
frappe.cache.delete_value("time_zone")
58 changes: 29 additions & 29 deletions frappe/core/doctype/user/test_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
from frappe.frappeclient import FrappeClient
from frappe.model.delete_doc import delete_doc
from frappe.tests import IntegrationTestCase, UnitTestCase
from frappe.tests.classes.context_managers import change_settings
from frappe.tests.test_api import FrappeAPITestCase
from frappe.utils import get_url

Expand Down Expand Up @@ -193,37 +194,36 @@ def test_delete_user(self):

def test_password_strength(self):
# Test Password without Password Strength Policy
frappe.db.set_single_value("System Settings", "enable_password_policy", 0)

# password policy is disabled, test_password_strength should be ignored
result = test_password_strength("test_password")
self.assertFalse(result.get("feedback", None))
with change_settings("System Settings", enable_password_policy=0):
# password policy is disabled, test_password_strength should be ignored
result = test_password_strength("test_password")
self.assertFalse(result.get("feedback", None))

# Test Password with Password Strenth Policy Set
frappe.db.set_single_value("System Settings", "enable_password_policy", 1)
frappe.db.set_single_value("System Settings", "minimum_password_score", 2)

# Score 1; should now fail
result = test_password_strength("bee2ve")
self.assertEqual(result["feedback"]["password_policy_validation_passed"], False)
self.assertRaises(frappe.exceptions.ValidationError, handle_password_test_fail, result["feedback"])
self.assertRaises(
frappe.exceptions.ValidationError, handle_password_test_fail, result
) # test backwards compatibility

# Score 4; should pass
result = test_password_strength("Eastern_43A1W")
self.assertEqual(result["feedback"]["password_policy_validation_passed"], True)

# test password strength while saving user with new password
user = frappe.get_doc("User", "test@example.com")
frappe.flags.in_test = False
user.new_password = "password"
self.assertRaises(frappe.exceptions.ValidationError, user.save)
user.reload()
user.new_password = "Eastern_43A1W"
user.save()
frappe.flags.in_test = True
with change_settings("System Settings", enable_password_policy=1, minimum_password_score=2):
# Score 1; should now fail
result = test_password_strength("bee2ve")
self.assertEqual(result["feedback"]["password_policy_validation_passed"], False)
self.assertRaises(
frappe.exceptions.ValidationError, handle_password_test_fail, result["feedback"]
)
self.assertRaises(
frappe.exceptions.ValidationError, handle_password_test_fail, result
) # test backwards compatibility

# Score 4; should pass
result = test_password_strength("Eastern_43A1W")
self.assertEqual(result["feedback"]["password_policy_validation_passed"], True)

# test password strength while saving user with new password
user = frappe.get_doc("User", "test@example.com")
frappe.flags.in_test = False
user.new_password = "password"
self.assertRaises(frappe.exceptions.ValidationError, user.save)
user.reload()
user.new_password = "Eastern_43A1W"
user.save()
frappe.flags.in_test = True

def test_comment_mentions(self):
comment = """
Expand Down
6 changes: 4 additions & 2 deletions frappe/defaults.py
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,9 @@ def clear_default(key=None, value=None, parent=None, name=None, parenttype=None)

def get_defaults_for(parent="__default"):
"""get all defaults"""
defaults = frappe.cache.hget("defaults", parent)

key = f"defaults::{parent}"
defaults = frappe.client_cache.get_value(key)

if defaults is None:
# sort descending because first default must get precedence
Expand All @@ -260,7 +262,7 @@ def get_defaults_for(parent="__default"):
elif d.defvalue is not None:
defaults[d.defkey] = d.defvalue

frappe.cache.hset("defaults", parent, defaults)
frappe.client_cache.set_value(key, defaults)

return defaults

Expand Down
2 changes: 1 addition & 1 deletion frappe/desk/reportview.py
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ def is_standard(fieldname):
return fieldname in default_fields or fieldname in optional_fields or fieldname in child_table_fields


@lru_cache
@lru_cache(maxsize=1024)
def extract_fieldnames(field):
from frappe.database.schema import SPECIAL_CHAR_PATTERN

Expand Down
Loading

0 comments on commit 939f832

Please sign in to comment.