From 79532ea0f21d7502ba487c57e90e8a3de8225ce0 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 15 Jul 2023 20:46:19 +0530 Subject: [PATCH 1/2] fix: incorrect cache clearing of assets - Build version wasn't correctly computed since v14 update of build system. This makes client side cache useless. - We clear cache assuming rapid reloads,but opening new tab also does that. This makes the cache effectively useless for most users. --- frappe/public/js/frappe/assets.js | 2 +- frappe/tests/test_perf.py | 5 +++++ frappe/utils/__init__.py | 2 +- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/frappe/public/js/frappe/assets.js b/frappe/public/js/frappe/assets.js index b8af761ee8..25a959fc50 100644 --- a/frappe/public/js/frappe/assets.js +++ b/frappe/public/js/frappe/assets.js @@ -29,7 +29,7 @@ frappe.assets = { if (localStorage._last_load) { var not_updated_since = new Date() - new Date(localStorage._last_load); - if (not_updated_since < 10000 || not_updated_since > 86400000) { + if (not_updated_since > 86400000) { frappe.assets.clear_local_storage(); } } else { diff --git a/frappe/tests/test_perf.py b/frappe/tests/test_perf.py index e086711f8c..32df6f8d5b 100644 --- a/frappe/tests/test_perf.py +++ b/frappe/tests/test_perf.py @@ -139,3 +139,8 @@ class TestPerformance(FrappeTestCase): PathResolver(path).resolve() with self.assertQueryCount(1): PathResolver(path).resolve() + + def test_consistent_build_version(self): + from frappe.utils import get_build_version + + self.assertEqual(get_build_version(), get_build_version()) diff --git a/frappe/utils/__init__.py b/frappe/utils/__init__.py index 387a4d0c6a..dbe6c5b925 100644 --- a/frappe/utils/__init__.py +++ b/frappe/utils/__init__.py @@ -955,7 +955,7 @@ def get_file_size(path, format=False): def get_build_version(): try: - return str(os.path.getmtime(os.path.join(frappe.local.sites_path, ".build"))) + return str(os.path.getmtime(os.path.join(frappe.local.sites_path, "assets/assets.json"))) except OSError: # .build can sometimes not exist # this is not a major problem so send fallback From 52c2e6d47bcd22d9a0a9dd2e4eb1cb6790478f96 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 15 Jul 2023 21:57:44 +0530 Subject: [PATCH 2/2] fix: Accurately determine rapid reloads Assuming rapid-reload ~= user frustrated with old assets. We weren't actually checking if load was "reload". This cleared cache when tabs were opened and effectively made cache useless. --- frappe/public/js/frappe/assets.js | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/frappe/public/js/frappe/assets.js b/frappe/public/js/frappe/assets.js index 25a959fc50..dcb945bacd 100644 --- a/frappe/public/js/frappe/assets.js +++ b/frappe/public/js/frappe/assets.js @@ -28,8 +28,11 @@ frappe.assets = { } if (localStorage._last_load) { - var not_updated_since = new Date() - new Date(localStorage._last_load); - if (not_updated_since > 86400000) { + let not_updated_since = new Date() - new Date(localStorage._last_load); + // Evict cache every 2 days + // Evict cache if page is reloaded within 10 seconds. Which could be user trying to + // refresh if things feel broken. + if ((not_updated_since < 10000 && is_reload()) || not_updated_since > 2 * 86400000) { frappe.assets.clear_local_storage(); } } else { @@ -184,3 +187,15 @@ frappe.assets = { return path; }, }; + +function is_reload() { + try { + return window.performance + ?.getEntriesByType("navigation") + .map((nav) => nav.type) + .includes("reload"); + } catch (e) { + // Safari probably + return true; + } +}