From 28a124ca47791d4a42d02668d524c2f83d5b3d37 Mon Sep 17 00:00:00 2001 From: Daizy Date: Sat, 22 Oct 2022 13:31:34 +0530 Subject: [PATCH 1/5] perf: cache document naming rule to avoid multiple db call --- frappe/model/naming.py | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/frappe/model/naming.py b/frappe/model/naming.py index d9dc0ee48c..163633b54e 100644 --- a/frappe/model/naming.py +++ b/frappe/model/naming.py @@ -235,13 +235,21 @@ def set_naming_from_document_naming_rule(doc): if doc.doctype in log_types: return - # ignore_ddl if naming is not yet bootstrapped - for d in frappe.get_all( - "Document Naming Rule", - dict(document_type=doc.doctype, disabled=0), - order_by="priority desc", - ignore_ddl=True, - ): + def _get_document_naming_rule(): + # ignore_ddl if naming is not yet bootstrapped + + return frappe.get_all( + "Document Naming Rule", + {"document_type": doc.doctype, "disabled": 0}, + order_by="priority desc", + ignore_ddl=True, + ) + + document_naming_rules = frappe.cache().hget( + "document_naming_rule", doc.doctype, _get_document_naming_rule + ) + + for d in document_naming_rules: frappe.get_cached_doc("Document Naming Rule", d.name).apply(doc) if doc.name: break From 78d30905ac34a294a2f0db946c85f05987a47eac Mon Sep 17 00:00:00 2001 From: Daizy Date: Mon, 24 Oct 2022 13:32:48 +0530 Subject: [PATCH 2/5] refactor: `get_doctype_map()` using single query and use generator for caching --- frappe/cache_manager.py | 29 +++++++------------ .../document_naming_rule.py | 6 ++++ frappe/model/naming.py | 17 ++++------- 3 files changed, 22 insertions(+), 30 deletions(-) diff --git a/frappe/cache_manager.py b/frappe/cache_manager.py index d4ce92f384..5862d532d8 100644 --- a/frappe/cache_manager.py +++ b/frappe/cache_manager.py @@ -8,11 +8,14 @@ from frappe.desk.notifications import clear_notifications, delete_notification_c common_default_keys = ["__default", "__global"] -doctype_map_keys = ( - "energy_point_rule_map", - "assignment_rule_map", - "milestone_tracker_map", -) +doctypes_for_mapping = { + "Energy Point Rule", + "Assignment Rule", + "Milestone Tracker", + "Document Naming Rule", +} + +doctype_map_keys = tuple(f"{frappe.scrub(doctype)}_map" for doctype in doctypes_for_mapping) bench_cache_keys = ("assets_json",) @@ -163,21 +166,11 @@ def clear_controller_cache(doctype=None): def get_doctype_map(doctype, name, filters=None, order_by=None): cache = frappe.cache() cache_key = frappe.scrub(doctype) + "_map" - doctype_map = cache.hget(cache_key, name) - if doctype_map is not None: - # cached, return - items = json.loads(doctype_map) - else: - # non cached, build cache - try: - items = frappe.get_all(doctype, filters=filters, order_by=order_by) - cache.hset(cache_key, name, json.dumps(items)) - except frappe.db.TableMissingError: - # executed from inside patch, ignore - items = [] + def _get_items(): + return frappe.get_all(doctype, filters=filters, order_by=order_by, ignore_ddl=True) - return items + return cache.hget(cache_key, name, _get_items) def clear_doctype_map(doctype, name): diff --git a/frappe/core/doctype/document_naming_rule/document_naming_rule.py b/frappe/core/doctype/document_naming_rule/document_naming_rule.py index 3fecf26ade..6d32893ceb 100644 --- a/frappe/core/doctype/document_naming_rule/document_naming_rule.py +++ b/frappe/core/doctype/document_naming_rule/document_naming_rule.py @@ -12,6 +12,12 @@ class DocumentNamingRule(Document): def validate(self): self.validate_fields_in_conditions() + def on_update(self): + frappe.cache_manager.clear_doctype_map("Document Naming Rule", self.document_type) + + def on_trash(self): + frappe.cache_manager.clear_doctype_map("Document Naming Rule", self.document_type) + def validate_fields_in_conditions(self): if self.has_value_changed("document_type"): docfields = [x.fieldname for x in frappe.get_meta(self.document_type).fields] diff --git a/frappe/model/naming.py b/frappe/model/naming.py index 163633b54e..6f7db5cf9d 100644 --- a/frappe/model/naming.py +++ b/frappe/model/naming.py @@ -235,18 +235,11 @@ def set_naming_from_document_naming_rule(doc): if doc.doctype in log_types: return - def _get_document_naming_rule(): - # ignore_ddl if naming is not yet bootstrapped - - return frappe.get_all( - "Document Naming Rule", - {"document_type": doc.doctype, "disabled": 0}, - order_by="priority desc", - ignore_ddl=True, - ) - - document_naming_rules = frappe.cache().hget( - "document_naming_rule", doc.doctype, _get_document_naming_rule + document_naming_rules = frappe.cache_manager.get_doctype_map( + "Document Naming Rule", + doc.doctype, + filters={"document_type": doc.doctype, "disabled": 0}, + order_by="priority desc", ) for d in document_naming_rules: From 380a638f3cd8182e857f87497bb73672b4acfaac Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Fri, 28 Oct 2022 16:18:39 +0530 Subject: [PATCH 3/5] chore: refactor duplicate code --- frappe/cache_manager.py | 21 +++++++++++-------- .../document_naming_rule.py | 7 +++++-- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/frappe/cache_manager.py b/frappe/cache_manager.py index 5862d532d8..47b753833a 100644 --- a/frappe/cache_manager.py +++ b/frappe/cache_manager.py @@ -15,7 +15,12 @@ doctypes_for_mapping = { "Document Naming Rule", } -doctype_map_keys = tuple(f"{frappe.scrub(doctype)}_map" for doctype in doctypes_for_mapping) + +def get_doctype_map_key(doctype): + return frappe.scrub(doctype) + "_map" + + +doctype_map_keys = tuple(map(get_doctype_map_key, doctypes_for_mapping)) bench_cache_keys = ("assets_json",) @@ -40,7 +45,7 @@ global_cache_keys = ( "sitemap_routes", "db_tables", "server_script_autocompletion_items", -) + doctype_map_keys +) user_cache_keys = ( "bootinfo", @@ -164,13 +169,11 @@ def clear_controller_cache(doctype=None): def get_doctype_map(doctype, name, filters=None, order_by=None): - cache = frappe.cache() - cache_key = frappe.scrub(doctype) + "_map" - - def _get_items(): - return frappe.get_all(doctype, filters=filters, order_by=order_by, ignore_ddl=True) - - return cache.hget(cache_key, name, _get_items) + return frappe.cache().hget( + get_doctype_map_key(doctype), + name, + lambda: frappe.get_all(doctype, filters=filters, order_by=order_by, ignore_ddl=True), + ) def clear_doctype_map(doctype, name): diff --git a/frappe/core/doctype/document_naming_rule/document_naming_rule.py b/frappe/core/doctype/document_naming_rule/document_naming_rule.py index 6d32893ceb..3da2d0105a 100644 --- a/frappe/core/doctype/document_naming_rule/document_naming_rule.py +++ b/frappe/core/doctype/document_naming_rule/document_naming_rule.py @@ -12,11 +12,14 @@ class DocumentNamingRule(Document): def validate(self): self.validate_fields_in_conditions() - def on_update(self): + def clear_doctype_map(self): frappe.cache_manager.clear_doctype_map("Document Naming Rule", self.document_type) + def on_update(self): + self.clear_doctype_map() + def on_trash(self): - frappe.cache_manager.clear_doctype_map("Document Naming Rule", self.document_type) + self.clear_doctype_map() def validate_fields_in_conditions(self): if self.has_value_changed("document_type"): From 85f6f1e01df375692aade92ac55e89b8e9a989b4 Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Fri, 28 Oct 2022 16:25:46 +0530 Subject: [PATCH 4/5] fix: dont clear doctype map keys when clearing doctype cache --- frappe/cache_manager.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/cache_manager.py b/frappe/cache_manager.py index 47b753833a..eeddef1865 100644 --- a/frappe/cache_manager.py +++ b/frappe/cache_manager.py @@ -45,7 +45,7 @@ global_cache_keys = ( "sitemap_routes", "db_tables", "server_script_autocompletion_items", -) +) + doctype_map_keys user_cache_keys = ( "bootinfo", @@ -74,7 +74,7 @@ doctype_cache_keys = ( "notifications", "workflow", "data_import_column_header_map", -) + doctype_map_keys +) def clear_user_cache(user=None): From e82e4d1a7344808bbff9a566fda76a6d71f4496a Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Fri, 28 Oct 2022 16:35:20 +0530 Subject: [PATCH 5/5] fix: use property instead of hardcoding --- .../core/doctype/document_naming_rule/document_naming_rule.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/core/doctype/document_naming_rule/document_naming_rule.py b/frappe/core/doctype/document_naming_rule/document_naming_rule.py index 3da2d0105a..598de98dbb 100644 --- a/frappe/core/doctype/document_naming_rule/document_naming_rule.py +++ b/frappe/core/doctype/document_naming_rule/document_naming_rule.py @@ -13,7 +13,7 @@ class DocumentNamingRule(Document): self.validate_fields_in_conditions() def clear_doctype_map(self): - frappe.cache_manager.clear_doctype_map("Document Naming Rule", self.document_type) + frappe.cache_manager.clear_doctype_map(self.doctype, self.document_type) def on_update(self): self.clear_doctype_map()