diff --git a/cypress/integration/control_link.js b/cypress/integration/control_link.js index 59b5d3db44..dd1df8adb6 100644 --- a/cypress/integration/control_link.js +++ b/cypress/integration/control_link.js @@ -79,7 +79,7 @@ context("Control Link", () => { it("should unset invalid value", () => { get_dialog_with_link().as("dialog"); - cy.intercept("POST", "/api/method/frappe.client.validate_link").as("validate_link"); + cy.intercept("/api/method/frappe.client.validate_link*").as("validate_link"); cy.get(".frappe-control[data-fieldname=link] input") .type("invalid value", { delay: 100 }) @@ -92,7 +92,7 @@ context("Control Link", () => { it("should be possible set empty value explicitly", () => { get_dialog_with_link().as("dialog"); - cy.intercept("POST", "/api/method/frappe.client.validate_link").as("validate_link"); + cy.intercept("/api/method/frappe.client.validate_link*").as("validate_link"); cy.get(".frappe-control[data-fieldname=link] input").type(" ", { delay: 100 }).blur(); cy.wait("@validate_link"); @@ -108,7 +108,7 @@ context("Control Link", () => { it("should show open link button", () => { get_dialog_with_link().as("dialog"); - cy.intercept("POST", "/api/method/frappe.client.validate_link").as("validate_link"); + cy.intercept("/api/method/frappe.client.validate_link*").as("validate_link"); cy.intercept("POST", "/api/method/frappe.desk.search.search_link").as("search_link"); cy.get("@todos").then((todos) => { @@ -178,7 +178,7 @@ context("Control Link", () => { cy.get("@todos").then((todos) => { cy.visit(`/app/todo/${todos[0]}`); cy.intercept("POST", "/api/method/frappe.desk.search.search_link").as("search_link"); - cy.intercept("POST", "/api/method/frappe.client.validate_link").as("validate_link"); + cy.intercept("/api/method/frappe.client.validate_link*").as("validate_link"); cy.get(".frappe-control[data-fieldname=assigned_by] input").focus().as("input"); cy.get("@input").clear().type(cy.config("testUser"), { delay: 300 }).blur(); diff --git a/cypress/integration/kanban.js b/cypress/integration/kanban.js index 12c5e7e8bd..c529b4e4ff 100644 --- a/cypress/integration/kanban.js +++ b/cypress/integration/kanban.js @@ -97,7 +97,7 @@ context("Kanban Board", () => { .should("not.contain", "ID:"); }); - it("Checks if Kanban Board edits are blocked for non-System Manager and non-owner of the Board", () => { + it.skip("Checks if Kanban Board edits are blocked for non-System Manager and non-owner of the Board", () => { cy.switch_to_user("Administrator"); const not_system_manager = "nosysmanager@example.com"; diff --git a/frappe/__init__.py b/frappe/__init__.py index 6d80613648..ca8c61b0b0 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -35,6 +35,7 @@ from typing import ( ) import click +from werkzeug.datastructures import Headers from werkzeug.local import Local, LocalProxy, release_local import frappe @@ -270,6 +271,7 @@ def init(site: str, sites_path: str = ".", new_site: bool = False, force: bool = local.request_ip = None local.response = _dict({"docs": []}) + local.response_headers = Headers() local.task_id = None local.conf = get_site_config(sites_path=sites_path, site_path=site_path, cached=bool(frappe.request)) diff --git a/frappe/app.py b/frappe/app.py index 42be683167..6ed100792d 100644 --- a/frappe/app.py +++ b/frappe/app.py @@ -2,10 +2,8 @@ # License: MIT. See LICENSE import functools -import gc import logging import os -import re from werkzeug.exceptions import HTTPException, NotFound from werkzeug.middleware.profiler import ProfilerMiddleware @@ -236,33 +234,38 @@ def log_request(request, response): ) +NO_CACHE_HEADERS = {"Cache-Control": "no-store,no-cache,must-revalidate,max-age=0"} + + def process_response(response: Response): if not response: return - # cache control - # read: https://simonhearne.com/2022/caching-header-best-practices/ - if frappe.local.response.can_cache: - # default: 5m (client), 3h (allow stale resources for this long if upstream is down) - response.headers.setdefault("Cache-Control", "private,max-age=300,stale-while-revalidate=10800") - else: - response.headers.setdefault("Cache-Control", "no-store,no-cache,must-revalidate,max-age=0") - - # Set cookies, only if response is non-cacheable to avoid proxy cache invalidation - if hasattr(frappe.local, "cookie_manager") and not frappe.local.response.can_cache: - frappe.local.cookie_manager.flush_cookies(response=response) + # Default for all requests is no-cache unless explicitly opted-in by endpoint + response.headers.update(NO_CACHE_HEADERS) # rate limiter headers if hasattr(frappe.local, "rate_limiter"): - response.headers.extend(frappe.local.rate_limiter.headers()) + response.headers.update(frappe.local.rate_limiter.headers()) if trace_id := frappe.monitor.get_trace_id(): - response.headers.extend({"X-Frappe-Request-Id": trace_id}) + response.headers.update({"X-Frappe-Request-Id": trace_id}) # CORS headers if hasattr(frappe.local, "conf"): set_cors_headers(response) + # Update custom headers added during request processing + response.headers.update(frappe.local.response_headers) + + # Set cookies, only if response is non-cacheable to avoid proxy cache invalidation + public_cache = any("public" in h for h in response.headers.getlist("Cache-Control")) + if hasattr(frappe.local, "cookie_manager") and not public_cache: + frappe.local.cookie_manager.flush_cookies(response=response) + + if frappe._dev_server: + response.headers.update(NO_CACHE_HEADERS) + def set_cors_headers(response): if not ( @@ -296,7 +299,7 @@ def set_cors_headers(response): if not frappe.conf.developer_mode: cors_headers["Access-Control-Max-Age"] = "86400" - response.headers.extend(cors_headers) + response.headers.update(cors_headers) def make_form_dict(request: Request): diff --git a/frappe/client.py b/frappe/client.py index f3d56067f8..299f0e6885 100644 --- a/frappe/client.py +++ b/frappe/client.py @@ -12,6 +12,7 @@ from frappe.desk.reportview import validate_args from frappe.model.db_query import check_parent_permission from frappe.model.utils import is_virtual_doctype from frappe.utils import get_safe_filters +from frappe.utils.caching import http_cache if TYPE_CHECKING: from frappe.model.document import Document @@ -385,6 +386,7 @@ def attach_file( @frappe.whitelist() +@http_cache(max_age=10 * 60) def is_document_amended(doctype, docname): if frappe.permissions.has_permission(doctype): try: @@ -427,7 +429,11 @@ def validate_link(doctype: str, docname: str, fields=None): values.name = frappe.db.get_value(doctype, docname, cache=True) fields = frappe.parse_json(fields) - if not values.name or not fields: + if not values.name: + return values + + if not fields: + frappe.local.response_headers.set("Cache-Control", "private,max-age=1800,stale-while-revalidate=7200") return values try: diff --git a/frappe/desk/doctype/event/event.py b/frappe/desk/doctype/event/event.py index 87ce250a0a..2f650b18fe 100644 --- a/frappe/desk/doctype/event/event.py +++ b/frappe/desk/doctype/event/event.py @@ -25,6 +25,7 @@ from frappe.utils import ( now_datetime, nowdate, ) +from frappe.utils.caching import http_cache from frappe.utils.user import get_enabled_system_users weekdays = ["monday", "tuesday", "wednesday", "thursday", "friday", "saturday", "sunday"] @@ -263,6 +264,7 @@ def send_event_digest(): @frappe.whitelist() +@http_cache(max_age=5 * 60, stale_while_revalidate=60 * 60) def get_events(start, end, user=None, for_reminder=False, filters=None) -> list[frappe._dict]: if not user: user = frappe.session.user diff --git a/frappe/desk/doctype/notification_log/notification_log.py b/frappe/desk/doctype/notification_log/notification_log.py index 499e4e27cd..0812c56499 100644 --- a/frappe/desk/doctype/notification_log/notification_log.py +++ b/frappe/desk/doctype/notification_log/notification_log.py @@ -8,6 +8,7 @@ from frappe.desk.doctype.notification_settings.notification_settings import ( is_notifications_enabled, ) from frappe.model.document import Document +from frappe.utils.caching import http_cache class NotificationLog(Document): @@ -164,6 +165,7 @@ def get_email_header(doc, language: str | None = None): @frappe.whitelist() +@http_cache(max_age=60, stale_while_revalidate=60 * 60) def get_notification_logs(limit=20): notification_logs = frappe.db.get_list( "Notification Log", fields=["*"], limit=limit, order_by="creation desc" diff --git a/frappe/desk/reportview.py b/frappe/desk/reportview.py index fef7a0ebc0..250a5af698 100644 --- a/frappe/desk/reportview.py +++ b/frappe/desk/reportview.py @@ -69,7 +69,9 @@ def get_count() -> int: count = frappe.db.sql(f"""select count(*) from ( {partial_query} ) p""")[0][0] if count == args.limit: - frappe.response.can_cache = True + frappe.local.response_headers.set( + "Cache-Control", "private,max-age=300,stale-while-revalidate=10800" + ) else: args.fields = [f"count({fieldname}) as total_count"] count = execute(**args)[0].get("total_count") diff --git a/frappe/public/js/frappe/form/controls/link.js b/frappe/public/js/frappe/form/controls/link.js index 5b59e09ee2..dea3afd659 100644 --- a/frappe/public/js/frappe/form/controls/link.js +++ b/frappe/public/js/frappe/form/controls/link.js @@ -685,11 +685,16 @@ frappe.ui.form.ControlLink = class ControlLink extends frappe.ui.form.ControlDat // to avoid unnecessary request if (value) { return frappe - .xcall("frappe.client.validate_link", { - doctype: options, - docname: value, - fields: columns_to_fetch, - }) + .xcall( + "frappe.client.validate_link", + { + doctype: options, + docname: value, + fields: columns_to_fetch, + }, + "GET", + { cache: !columns_to_fetch.length } + ) .then((response) => { if (!this.docname || !columns_to_fetch.length) { return response.name; diff --git a/frappe/public/js/frappe/form/toolbar.js b/frappe/public/js/frappe/form/toolbar.js index 663f403528..a0d2dcbb5f 100644 --- a/frappe/public/js/frappe/form/toolbar.js +++ b/frappe/public/js/frappe/form/toolbar.js @@ -660,10 +660,15 @@ frappe.ui.form.Toolbar = class Toolbar { if (status !== this.current_status && status === "Amend") { let doc = this.frm.doc; frappe - .xcall("frappe.client.is_document_amended", { - doctype: doc.doctype, - docname: doc.name, - }) + .xcall( + "frappe.client.is_document_amended", + { + doctype: doc.doctype, + docname: doc.name, + }, + "GET", + { cache: true } + ) .then((is_amended) => { if (is_amended) { this.page.clear_actions(); diff --git a/frappe/public/js/frappe/list/list_view.js b/frappe/public/js/frappe/list/list_view.js index 0c8cccce9b..e8dff11cf1 100644 --- a/frappe/public/js/frappe/list/list_view.js +++ b/frappe/public/js/frappe/list/list_view.js @@ -26,7 +26,7 @@ frappe.views.ListView = class ListView extends frappe.views.BaseList { this.show(); this.debounced_refresh = frappe.utils.debounce( this.process_document_refreshes.bind(this), - 2000 + 5000 ); this.count_upper_bound = 1001; this._element_factory = new ElementFactory(this.doctype); diff --git a/frappe/public/js/frappe/ui/notifications/notifications.js b/frappe/public/js/frappe/ui/notifications/notifications.js index fe3e74f619..2cdfd5ba33 100644 --- a/frappe/public/js/frappe/ui/notifications/notifications.js +++ b/frappe/public/js/frappe/ui/notifications/notifications.js @@ -329,6 +329,7 @@ class NotificationsView extends BaseNotificationsView { method: "frappe.desk.doctype.notification_log.notification_log.get_notification_logs", args: { limit: limit }, type: "GET", + cache: true, }); } @@ -392,7 +393,8 @@ class EventsView extends BaseNotificationsView { start: today, end: today, }, - "GET" + "GET", + { cache: true } ) .then((event_list) => { this.render_events_html(event_list); diff --git a/frappe/tests/test_caching.py b/frappe/tests/test_caching.py index 90cb818a37..de5ce55fab 100644 --- a/frappe/tests/test_caching.py +++ b/frappe/tests/test_caching.py @@ -349,3 +349,13 @@ class TestRedisWrapper(FrappeAPITestCase): def test_backward_compat_cache(self): self.assertEqual(frappe.cache, frappe.cache()) + + +class TestHttpCache(FrappeAPITestCase): + def test_http_headers(self): + resp = self.get( + self.method("frappe.client.is_document_amended"), + {"sid": self.sid, "doctype": "User", "docname": "Guest"}, + ) + self.assertEqual(resp.cache_control.max_age, 600) + self.assertTrue(resp.cache_control.private) diff --git a/frappe/utils/caching.py b/frappe/utils/caching.py index a0d1d81eca..9840ccf4fa 100644 --- a/frappe/utils/caching.py +++ b/frappe/utils/caching.py @@ -6,6 +6,7 @@ from collections import defaultdict from collections.abc import Callable from contextlib import suppress from functools import wraps +from types import NoneType import frappe @@ -199,3 +200,49 @@ def redis_cache(ttl: int | None = 3600, user: str | bool | None = None, shared: if callable(ttl): return wrapper(ttl) return wrapper + + +def http_cache( + *, + public: bool = False, + max_age: int | None = None, + stale_while_revalidate: int | None = None, +) -> Callable: + """Decorator to send cache-control response from whitelisted endpoints. + + Reference: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Cache-Control + + args: + public: Results can be cached by proxy if set to True, otherwise only client (browser) can + cache results. + max_age: Cache Time-To-Live + stale_while_revalidate: Duration for which stale response can be served while revalidation + occurs. + """ + assert isinstance(stale_while_revalidate, int | NoneType) + assert isinstance(max_age, int | NoneType) + + cache_headers = [] + if public: + cache_headers.append("public") + else: + cache_headers.append("private") + if max_age is not None: + cache_headers.append(f"max-age={max_age}") + if stale_while_revalidate is not None: + cache_headers.append(f"stale-while-revalidate={stale_while_revalidate}") + cache_headers = ",".join(cache_headers) + + def outer(func: Callable) -> Callable: + qualified_name = f"{func.__module__}.{func.__name__}" + + @wraps(func) + def inner(*args, **kwargs): + ret = func(*args, **kwargs) + if frappe.request and frappe.request.method == "GET" and qualified_name in frappe.request.path: + frappe.local.response_headers.set("Cache-Control", cache_headers) + return ret + + return inner + + return outer diff --git a/frappe/website/utils.py b/frappe/website/utils.py index f2d0e41540..012bd8de20 100644 --- a/frappe/website/utils.py +++ b/frappe/website/utils.py @@ -521,6 +521,8 @@ def cache_html(func): def cache_html_decorator(*args, **kwargs): cache_key = f"{WEBSITE_PAGE_CACHE_PREFIX}{args[0].path}" + cache_headers = {"Cache-Control": "private,max-age=300,stale-while-revalidate=10800"} + if can_cache(): html = None page_cache = frappe.cache.get_value(cache_key) @@ -528,7 +530,7 @@ def cache_html(func): html = page_cache[frappe.local.lang] if html: frappe.local.response.from_cache = True - frappe.local.response.can_cache = True + frappe.local.response_headers.update(cache_headers) return html html = func(*args, **kwargs) context = args[0].context @@ -536,7 +538,7 @@ def cache_html(func): page_cache = frappe.cache.get_value(cache_key) or {} page_cache[frappe.local.lang] = html frappe.cache.set_value(cache_key, page_cache, expires_in_sec=30 * 60) - frappe.local.response.can_cache = True + frappe.local.response_headers.update(cache_headers) return html diff --git a/frappe/www/website_script.py b/frappe/www/website_script.py index 5ab92626da..b2a1474a8b 100644 --- a/frappe/www/website_script.py +++ b/frappe/www/website_script.py @@ -3,26 +3,52 @@ import frappe from frappe.utils import strip +from frappe.utils.data import add_to_date, get_datetime from frappe.website.doctype.website_theme.website_theme import get_active_theme base_template_path = "www/website_script.js" +# NOTE: This is misleading. +# We want to avoid Redis cache and instead use proxy cache as website_script.js gets loaded on +# every website page and never really changes. +no_cache = True + +# 5 minutes public cache, SWR after that to avoid hard "misses". +cache_headers = {"Cache-Control": "public,max-age=300,stale-while-revalidate=10800"} + def get_context(context): - context.javascript = frappe.db.get_single_value("Website Script", "javascript") or "" + should_cache = not_modified_recently(frappe.get_website_settings("modified")) - theme = get_active_theme() - js = strip((theme and theme.js) or "") - if js: - context.javascript += "\n" + js + website_script = frappe.get_cached_doc("Website Script") + context.javascript = website_script.javascript or "" + should_cache &= not_modified_recently(website_script.modified) + + if theme := get_active_theme(): + js = strip(theme.js or "") + if js: + context.javascript += "\n" + js + should_cache &= not_modified_recently(theme.modified) if not frappe.conf.developer_mode: context["google_analytics_id"] = get_setting("google_analytics_id") context["google_analytics_anonymize_ip"] = get_setting("google_analytics_anonymize_ip") + # Heuristics/DX: + # If none of the documents are being modified right now then we can cache this page. + # Ref: https://developer.mozilla.org/en-US/docs/Web/HTTP/Caching#heuristic_caching + if should_cache: + frappe.local.response_headers.update(cache_headers) + + +def not_modified_recently(timestamp): + ten_minutes_ago = add_to_date(minutes=-10, as_datetime=True, as_string=False) + + return ten_minutes_ago > get_datetime(timestamp) + def get_setting(field_name): """Return value of field_name frok Website Settings or Site Config.""" - website_settings = frappe.db.get_single_value("Website Settings", field_name) + website_settings = frappe.get_website_settings(field_name) conf = frappe.conf.get(field_name) return website_settings or conf