From 69a8027935c264d55982ad559f8a681265e2ebf2 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 21 Aug 2022 13:00:34 +0530 Subject: [PATCH 1/6] fix: jump to field shows column/section breaks --- frappe/public/js/frappe/form/layout.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/frappe/public/js/frappe/form/layout.js b/frappe/public/js/frappe/form/layout.js index 770df2a5a9..3bd9e451db 100644 --- a/frappe/public/js/frappe/form/layout.js +++ b/frappe/public/js/frappe/form/layout.js @@ -278,7 +278,10 @@ frappe.ui.form.Layout = class Layout { make_section(df = {}) { this.section_count++; - if (!df.fieldname) df.fieldname = `__section_${this.section_count}`; + if (!df.fieldname) { + df.fieldname = `__section_${this.section_count}`; + df.fieldtype = "Section Break"; + } this.section = new Section( this.current_tab ? this.current_tab.wrapper : this.page, @@ -300,7 +303,10 @@ frappe.ui.form.Layout = class Layout { make_column(df = {}) { this.column_count++; - if (!df.fieldname) df.fieldname = `__column_${this.section_count}`; + if (!df.fieldname) { + df.fieldname = `__column_${this.section_count}`; + df.fieldtype = "Column Break"; + } this.column = new Column(this.section, df); if (df && df.fieldname) { From 04aeeabb2bf4730399adf3985b8f448293debb8b Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 21 Aug 2022 16:42:26 +0530 Subject: [PATCH 2/6] fix(recorder): make whole order button clickable --- frappe/public/js/frappe/recorder/RequestDetail.vue | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/public/js/frappe/recorder/RequestDetail.vue b/frappe/public/js/frappe/recorder/RequestDetail.vue index c07e6220a9..8ee6ff631b 100644 --- a/frappe/public/js/frappe/recorder/RequestDetail.vue +++ b/frappe/public/js/frappe/recorder/RequestDetail.vue @@ -25,8 +25,8 @@
  • {{ column.label }}
    • - From 786df3fbeb1876639cdfa84de7908f33b7a9c990 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 21 Aug 2022 14:31:10 +0530 Subject: [PATCH 3/6] perf: ~33% faster Desk response - hardcode `/app` resolution - use cached website settings everywhere. It was mixing cache and DB everywhere and re-quering same thing (why ?) --- .../website_settings/website_settings.py | 22 ++++++++++--------- .../doctype/website_theme/website_theme.py | 4 ++-- frappe/website/page_renderers/static_page.py | 2 ++ frappe/website/path_resolver.py | 7 ++++++ 4 files changed, 23 insertions(+), 12 deletions(-) diff --git a/frappe/website/doctype/website_settings/website_settings.py b/frappe/website/doctype/website_settings/website_settings.py index fffbd94684..be9b155314 100644 --- a/frappe/website/doctype/website_settings/website_settings.py +++ b/frappe/website/doctype/website_settings/website_settings.py @@ -103,12 +103,12 @@ class WebsiteSettings(Document): def get_website_settings(context=None): hooks = frappe.get_hooks() context = frappe._dict(context or {}) - settings: "WebsiteSettings" = frappe.get_single("Website Settings") + settings: "WebsiteSettings" = frappe.get_cached_doc("Website Settings") context = context.update( { - "top_bar_items": get_items("top_bar_items"), - "footer_items": get_items("footer_items"), + "top_bar_items": modify_header_footer_items(settings.top_bar_items), + "footer_items": modify_header_footer_items(settings.footer_items), "post_login": [ {"label": _("My Account"), "url": "/me"}, {"label": _("Log out"), "url": "/?cmd=web_logout"}, @@ -203,22 +203,24 @@ def get_items(parentfield: str) -> list[dict]: order_by="idx asc", fields="*", ) - top_items = _items.copy() + return modify_header_footer_items(_items) + +def modify_header_footer_items(items: list): + top_items = items.copy() # attach child items to top bar - for item in _items: - if not item["parent_label"]: + for item in items: + if not item.parent_label: continue for top_bar_item in top_items: - if top_bar_item["label"] != item["parent_label"]: + if top_bar_item.label != item.parent_label: continue - if "child_items" not in top_bar_item: + if not top_bar_item.get("child_items"): top_bar_item["child_items"] = [] - top_bar_item["child_items"].append(item) - + top_bar_item.child_items.append(item) break return top_items diff --git a/frappe/website/doctype/website_theme/website_theme.py b/frappe/website/doctype/website_theme/website_theme.py index 442ebe284b..e7636445c2 100644 --- a/frappe/website/doctype/website_theme/website_theme.py +++ b/frappe/website/doctype/website_theme/website_theme.py @@ -133,9 +133,9 @@ class WebsiteTheme(Document): def get_active_theme() -> Optional["WebsiteTheme"]: - if website_theme := frappe.db.get_single_value("Website Settings", "website_theme"): + if website_theme := frappe.get_website_settings("website_theme"): try: - return frappe.get_doc("Website Theme", website_theme) + return frappe.get_cached_doc("Website Theme", website_theme) except frappe.DoesNotExistError: pass diff --git a/frappe/website/page_renderers/static_page.py b/frappe/website/page_renderers/static_page.py index eb862d42bd..1f26de1514 100644 --- a/frappe/website/page_renderers/static_page.py +++ b/frappe/website/page_renderers/static_page.py @@ -12,6 +12,8 @@ UNSUPPORTED_STATIC_PAGE_TYPES = ("html", "md", "js", "xml", "css", "txt", "py", class StaticPage(BaseRenderer): + __slots__ = ("path", "file_path") + def __init__(self, path, http_status_code=None): super().__init__(path=path, http_status_code=http_status_code) self.set_file_path() diff --git a/frappe/website/path_resolver.py b/frappe/website/path_resolver.py index 9015bc7566..36490f5e30 100644 --- a/frappe/website/path_resolver.py +++ b/frappe/website/path_resolver.py @@ -17,6 +17,8 @@ from frappe.website.utils import can_cache, get_home_page class PathResolver: + __slots__ = ("path",) + def __init__(self, path): self.path = path.strip("/ ") @@ -36,6 +38,11 @@ class PathResolver: return frappe.flags.redirect_location, RedirectPage(self.path) endpoint = resolve_path(self.path) + + # WARN: Hardcoded for better performance + if endpoint == "app": + return endpoint, TemplatePage(endpoint, 200) + custom_renderers = self.get_custom_page_renderers() renderers = custom_renderers + [ StaticPage, From 61a9349789607f31cc657e7bbdc652f7b2a77b70 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 21 Aug 2022 16:14:09 +0530 Subject: [PATCH 4/6] perf: use is_virtual_doctype and remove limit This reduces 1 query for each child table read Removed limit cause with 1000+ doctypes in frappe+erpnext this cache will just keep getting trashed for no reason. There's clear upper bound on size so no need to limit it here. --- frappe/model/document.py | 7 ++----- frappe/model/utils/__init__.py | 2 +- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/frappe/model/document.py b/frappe/model/document.py index 5add5fba4e..2a82b5af9a 100644 --- a/frappe/model/document.py +++ b/frappe/model/document.py @@ -15,6 +15,7 @@ from frappe.model import optional_fields, table_fields from frappe.model.base_document import BaseDocument, get_controller from frappe.model.docstatus import DocStatus from frappe.model.naming import set_new_name, validate_name +from frappe.model.utils import is_virtual_doctype from frappe.model.workflow import set_workflow_state_on_action, validate_workflow from frappe.utils import cstr, date_diff, file_lock, flt, get_datetime_str, now from frappe.utils.data import get_absolute_url @@ -154,11 +155,7 @@ class Document(BaseDocument): # Make sure not to query the DB for a child table, if it is a virtual one. # During frappe is installed, the property "is_virtual" is not available in tabDocType, so # we need to filter those cases for the access to frappe.db.get_value() as it would crash otherwise. - if ( - hasattr(self, "doctype") - and not hasattr(self, "module") - and frappe.db.get_value("DocType", df.options, "is_virtual", cache=True) - ): + if hasattr(self, "doctype") and not hasattr(self, "module") and is_virtual_doctype(df.options): self.set(df.fieldname, []) continue diff --git a/frappe/model/utils/__init__.py b/frappe/model/utils/__init__.py index 2220b3904f..bf6804ad05 100644 --- a/frappe/model/utils/__init__.py +++ b/frappe/model/utils/__init__.py @@ -128,6 +128,6 @@ def get_fetch_values(doctype, fieldname, value): return result -@site_cache(maxsize=128) +@site_cache() def is_virtual_doctype(doctype): return frappe.db.get_value("DocType", doctype, "is_virtual") From 48869d506fc705a1f8d1fee1f80c5134df93c96e Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 21 Aug 2022 15:45:13 +0530 Subject: [PATCH 5/6] perf: dont order by for uniq searches --- frappe/desk/doctype/tag/tag.py | 2 +- frappe/desk/form/load.py | 4 +++- frappe/utils/user.py | 2 +- frappe/website/path_resolver.py | 2 +- frappe/www/app.py | 4 +++- 5 files changed, 9 insertions(+), 5 deletions(-) diff --git a/frappe/desk/doctype/tag/tag.py b/frappe/desk/doctype/tag/tag.py index ca167c148e..84239fae6d 100644 --- a/frappe/desk/doctype/tag/tag.py +++ b/frappe/desk/doctype/tag/tag.py @@ -192,4 +192,4 @@ def get_documents_for_tag(tag): @frappe.whitelist() def get_tags_list_for_awesomebar(): - return [t.name for t in frappe.get_list("Tag")] + return frappe.get_list("Tag", pluck="name", order_by=None) diff --git a/frappe/desk/form/load.py b/frappe/desk/form/load.py index d68aab927a..36dfd948f7 100644 --- a/frappe/desk/form/load.py +++ b/frappe/desk/form/load.py @@ -452,7 +452,9 @@ def get_title_values_for_link_and_dynamic_link_fields(doc, link_fields=None): if not meta or not (meta.title_field and meta.show_title_field_in_link): continue - link_title = frappe.db.get_value(doctype, doc.get(field.fieldname), meta.title_field, cache=True) + link_title = frappe.db.get_value( + doctype, doc.get(field.fieldname), meta.title_field, cache=True, order_by=None + ) link_titles.update({doctype + "::" + doc.get(field.fieldname): link_title}) return link_titles diff --git a/frappe/utils/user.py b/frappe/utils/user.py index a9bec0affa..28918f437d 100644 --- a/frappe/utils/user.py +++ b/frappe/utils/user.py @@ -273,7 +273,7 @@ def get_user_fullname(user: str) -> str: def get_fullname_and_avatar(user: str) -> _dict: first_name, last_name, avatar, name = frappe.db.get_value( - "User", user, ["first_name", "last_name", "user_image", "name"] + "User", user, ["first_name", "last_name", "user_image", "name"], order_by=None ) return _dict( { diff --git a/frappe/website/path_resolver.py b/frappe/website/path_resolver.py index 36490f5e30..071b5f9be1 100644 --- a/frappe/website/path_resolver.py +++ b/frappe/website/path_resolver.py @@ -105,7 +105,7 @@ def resolve_redirect(path, query_string=None): ] """ redirects = frappe.get_hooks("website_redirects") - redirects += frappe.db.get_all("Website Route Redirect", ["source", "target"]) + redirects += frappe.get_all("Website Route Redirect", ["source", "target"], order_by=None) if not redirects: return diff --git a/frappe/www/app.py b/frappe/www/app.py index f75fe05c03..6f6b4c88c2 100644 --- a/frappe/www/app.py +++ b/frappe/www/app.py @@ -18,7 +18,9 @@ CLOSING_SCRIPT_TAG_PATTERN = re.compile(r"") def get_context(context): if frappe.session.user == "Guest": frappe.throw(_("Log in to access this page."), frappe.PermissionError) - elif frappe.db.get_value("User", frappe.session.user, "user_type") == "Website User": + elif ( + frappe.db.get_value("User", frappe.session.user, "user_type", order_by=None) == "Website User" + ): frappe.throw(_("You are not permitted to access this page."), frappe.PermissionError) hooks = frappe.get_hooks() From 4241f8c8c0761344eeccf219d125324b0b232f6f Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 21 Aug 2022 18:30:14 +0530 Subject: [PATCH 6/6] perf: simpler/faster preload header computation We parse entire response to find preload headers, instead just use include_style and include_script to include assets directly into preload headers. This shaves off ~13% overhead in response. --- frappe/__init__.py | 1 + frappe/utils/jinja_globals.py | 24 ++++++++++++++++++++++-- frappe/website/utils.py | 30 ++++++++++-------------------- 3 files changed, 33 insertions(+), 22 deletions(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index a796db9a83..53bc1018da 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -240,6 +240,7 @@ def init(site: str, sites_path: str = ".", new_site: bool = False) -> None: local.document_cache = {} local.meta_cache = {} local.form_dict = _dict() + local.preload_assets = {"style": [], "script": []} local.session = _dict() local.dev_server = _dev_server local.qb = get_query_builder(local.conf.db_type or "mariadb") diff --git a/frappe/utils/jinja_globals.py b/frappe/utils/jinja_globals.py index 44835be352..1265bd4d42 100644 --- a/frappe/utils/jinja_globals.py +++ b/frappe/utils/jinja_globals.py @@ -95,13 +95,33 @@ def get_dom_id(seed=None): return "id-" + generate_hash(seed, 12) -def include_script(path): +def include_script(path, preload=True): + """Get path of bundled script files. + + If preload is specified the path will be added to preload headers so browsers can prefetch + assets.""" path = bundled_asset(path) + + if preload: + import frappe + + frappe.local.preload_assets["script"].append(path) + return f'' -def include_style(path, rtl=None): +def include_style(path, rtl=None, preload=True): + """Get path of bundled style files. + + If preload is specified the path will be added to preload headers so browsers can prefetch + assets.""" path = bundled_asset(path) + + if preload: + import frappe + + frappe.local.preload_assets["style"].append(path) + return f'' diff --git a/frappe/website/utils.py b/frappe/website/utils.py index 508026f064..bfb600a8c4 100644 --- a/frappe/website/utils.py +++ b/frappe/website/utils.py @@ -527,7 +527,8 @@ def build_response(path, data, http_status_code, headers: dict | None = None): response.headers["X-Page-Name"] = path.encode("ascii", errors="xmlcharrefreplace") response.headers["X-From-Cache"] = frappe.local.response.from_cache or False - add_preload_headers(response) + add_preload_for_bundled_assets(response) + if headers: for key, val in headers.items(): response.headers[key] = val.encode("ascii", errors="xmlcharrefreplace") @@ -557,29 +558,18 @@ def set_content_type(response, data, path): return data -def add_preload_headers(response): - from bs4 import BeautifulSoup, SoupStrainer +def add_preload_for_bundled_assets(response): - try: - preload = [] - strainer = SoupStrainer(re.compile("script|link")) - soup = BeautifulSoup(response.data, "html.parser", parse_only=strainer) - for elem in soup.find_all("script", src=re.compile(".*")): - preload.append(("script", elem.get("src"))) + links = [] - for elem in soup.find_all("link", rel="stylesheet"): - preload.append(("style", elem.get("href"))) + for css in frappe.local.preload_assets["style"]: + links.append(f"<{css}>; rel=preload; as=style") - links = [] - for _type, link in preload: - links.append(f"<{link}>; rel=preload; as={_type}") + for js in frappe.local.preload_assets["script"]: + links.append(f"<{js}>; rel=preload; as=script") - if links: - response.headers["Link"] = ",".join(links) - except Exception: - import traceback - - traceback.print_exc() + if links: + response.headers["Link"] = ",".join(links) @lru_cache