diff --git a/.github/helper/roulette.py b/.github/helper/roulette.py index f68ef5046f..9165198012 100644 --- a/.github/helper/roulette.py +++ b/.github/helper/roulette.py @@ -5,8 +5,10 @@ import shlex import subprocess import sys import urllib.request +from functools import cache +@cache def fetch_pr_data(pr_number, repo, endpoint): api_url = f"https://api.github.com/repos/{repo}/pulls/{pr_number}" @@ -26,7 +28,16 @@ def get_output(command, shell=True): return subprocess.check_output(command, shell=shell, encoding="utf8").strip() def has_skip_ci_label(pr_number, repo="frappe/frappe"): - return any([label["name"] for label in fetch_pr_data(pr_number, repo, "")["labels"] if label["name"] == "Skip CI"]) + return has_label(pr_number, "Skip CI", repo) + +def has_run_server_tests_label(pr_number, repo="frappe/frappe"): + return has_label(pr_number, "Run Server Tests", repo) + +def has_run_ui_tests_label(pr_number, repo="frappe/frappe"): + return has_label(pr_number, "Run UI Tests", repo) + +def has_label(pr_number, label, repo="frappe/frappe"): + return any([label["name"] for label in fetch_pr_data(pr_number, repo, "")["labels"] if label["name"] == label]) def is_py(file): return file.endswith("py") @@ -77,11 +88,11 @@ if __name__ == "__main__": print("Only docs were updated, stopping build process.") sys.exit(0) - elif only_frontend_code_changed and build_type == "server": + elif only_frontend_code_changed and build_type == "server" and not has_run_server_tests_label(pr_number, repo): print("Only Frontend code was updated; Stopping Python build process.") sys.exit(0) - elif build_type == "ui" and only_py_changed: + elif build_type == "ui" and only_py_changed and not has_run_ui_tests_label(pr_number, repo): print("Only Python code was updated, stopping Cypress build process.") sys.exit(0) diff --git a/frappe/__init__.py b/frappe/__init__.py index 51c6ba3a74..7c71bc8815 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -44,6 +44,7 @@ local = Local() STANDARD_USERS = ("Guest", "Administrator") _dev_server = int(sbool(os.environ.get("DEV_SERVER", False))) +_qb_patched = False if _dev_server: warnings.simplefilter("always", DeprecationWarning) @@ -236,8 +237,10 @@ def init(site, sites_path=None, new_site=False): local.qb = get_query_builder(local.conf.db_type or "mariadb") setup_module_map() - patch_query_execute() - patch_query_aggregation() + + if not _qb_patched: + patch_query_execute() + patch_query_aggregation() local.initialised = True @@ -868,6 +871,10 @@ def clear_cache(user=None, doctype=None): local.role_permissions = {} if hasattr(local, "request_cache"): local.request_cache.clear() + if hasattr(local, "system_settings"): + del local.system_settings + if hasattr(local, "website_settings"): + del local.website_settings def only_has_select_perm(doctype, user=None, ignore_permissions=False): @@ -1091,6 +1098,10 @@ def clear_document_cache(doctype, name): if key in local.document_cache: del local.document_cache[key] cache().hdel("document_cache", key) + if doctype == "System Settings" and hasattr(local, "system_settings"): + delattr(local, "system_settings") + if doctype == "Website Settings" and hasattr(local, "website_settings"): + delattr(local, "website_settings") def get_cached_value(doctype, name, fieldname="name", as_dict=False): @@ -2203,8 +2214,18 @@ def safe_eval(code, eval_globals=None, eval_locals=None): return eval(code, eval_globals, eval_locals) +def get_website_settings(key): + if not hasattr(local, "website_settings"): + local.website_settings = db.get_singles_dict("Website Settings") + + return local.website_settings[key] + + def get_system_settings(key): - return db.get_single_value("System Settings", key, cache=True) + if not hasattr(local, "system_settings"): + local.system_settings = db.get_singles_dict("System Settings") + + return local.system_settings[key] def get_active_domains(): diff --git a/frappe/contacts/address_and_contact.py b/frappe/contacts/address_and_contact.py index 036594926e..a0f742c55a 100644 --- a/frappe/contacts/address_and_contact.py +++ b/frappe/contacts/address_and_contact.py @@ -3,6 +3,7 @@ import functools import re +from typing import Dict, List import frappe from frappe import _ @@ -169,29 +170,34 @@ def delete_contact_and_address(doctype, docname): @frappe.whitelist() @frappe.validate_and_sanitize_search_inputs -def filter_dynamic_link_doctypes(doctype, txt, searchfield, start, page_len, filters): - if not txt: - txt = "" +def filter_dynamic_link_doctypes(txt: str, filters: Dict) -> List[List[str]]: + from frappe.permissions import get_doctypes_with_read - doctypes = frappe.db.get_all( - "DocField", filters=filters, fields=["parent"], distinct=True, as_list=True + txt = txt or "" + filters = filters or {} + TXT_PATTERN = re.compile(f"{txt}.*") + + _doctypes_from_df = frappe.get_all( + "DocField", + filters=filters, + pluck="parent", + distinct=True, + order_by=None, ) + doctypes_from_df = {d for d in _doctypes_from_df if TXT_PATTERN.search(_(d), re.IGNORECASE)} - doctypes = tuple(d for d in doctypes if re.search(txt + ".*", _(d[0]), re.IGNORECASE)) + filters.update({"dt": ("not in", doctypes_from_df)}) + _doctypes_from_cdf = frappe.get_all( + "Custom Field", filters=filters, pluck="dt", distinct=True, order_by=None + ) + doctypes_from_cdf = {d for d in _doctypes_from_cdf if TXT_PATTERN.search(_(d), re.IGNORECASE)} - filters.update({"dt": ("not in", [d[0] for d in doctypes])}) + all_doctypes = doctypes_from_df.union(doctypes_from_cdf) + allowed_doctypes = set(get_doctypes_with_read()) - _doctypes = frappe.db.get_all("Custom Field", filters=filters, fields=["dt"], as_list=True) + valid_doctypes = sorted(all_doctypes.intersection(allowed_doctypes)) - _doctypes = tuple([d for d in _doctypes if re.search(txt + ".*", _(d[0]), re.IGNORECASE)]) - - all_doctypes = [d[0] for d in doctypes + _doctypes] - allowed_doctypes = frappe.permissions.get_doctypes_with_read() - - valid_doctypes = sorted(set(all_doctypes).intersection(set(allowed_doctypes))) - valid_doctypes = [[doctype] for doctype in valid_doctypes] - - return valid_doctypes + return [[doctype] for doctype in valid_doctypes] def set_link_title(doc): diff --git a/frappe/database/database.py b/frappe/database/database.py index 1de22af037..39ca846904 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -1,4 +1,4 @@ -# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors +# Copyright (c) 2022, Frappe Technologies Pvt. Ltd. and Contributors # License: MIT. See LICENSE # Database Module @@ -18,6 +18,7 @@ import frappe import frappe.defaults import frappe.model.meta from frappe import _ +from frappe.exceptions import DoesNotExistError from frappe.model.utils.link_count import flush_local_link_count from frappe.query_builder.functions import Count from frappe.query_builder.utils import DocType @@ -29,6 +30,10 @@ SINGLE_WORD_PATTERN = re.compile(r'([`"]?)(tab([A-Z]\w+))\1') MULTI_WORD_PATTERN = re.compile(r'([`"])(tab([A-Z]\w+)( [A-Z]\w+)+)\1') +def is_query_type(query: str, query_type: Union[str, Tuple[str]]) -> bool: + return query.lstrip().split(maxsplit=1)[0].lower().startswith(query_type) + + class Database(object): """ Open a database connection with the given parmeters, if use_default is True, use the @@ -248,7 +253,7 @@ class Database(object): # debug if debug: - if explain and query.strip().lower().startswith("select"): + if explain and is_query_type(query, "select"): self.explain_query(query, values) frappe.errprint(self.mogrify(query, values)) @@ -305,7 +310,7 @@ class Database(object): could cause the system to hang.""" self.check_implicit_commit(query) - if query and query.strip().lower() in ("commit", "rollback"): + if query and is_query_type(query, ("commit", "rollback")): self.transaction_writes = 0 if query[:6].lower() in ("update", "insert", "delete"): @@ -322,8 +327,7 @@ class Database(object): if ( self.transaction_writes and query - and query.strip().split()[0].lower() - in ["start", "alter", "drop", "create", "begin", "truncate"] + and is_query_type(query, ("start", "alter", "drop", "create", "begin", "truncate")) ): raise Exception("This statement can cause implicit commit") @@ -346,7 +350,7 @@ class Database(object): @staticmethod def clear_db_table_cache(query): - if query and query.strip().split()[0].lower() in {"drop", "create"}: + if query and is_query_type(query, ("drop", "create")): frappe.cache().delete_key("db_tables") @staticmethod @@ -625,14 +629,28 @@ class Database(object): # Get coulmn and value of the single doctype Accounts Settings account_settings = frappe.db.get_singles_dict("Accounts Settings") """ - result = self.query.get_sql( + return_value = frappe._dict() + + try: + meta = frappe.get_meta(doctype) + except DoesNotExistError: + return return_value + + queried_result = self.query.get_sql( "Singles", filters={"doctype": doctype}, fields=["field", "value"], for_update=for_update, - ).run() + ).run(debug=debug) - return frappe._dict(result) + for fieldname, value in queried_result: + if df := meta.get_field(fieldname): + casted_value = cast(df.fieldtype, value) + else: + casted_value = value + return_value[fieldname] = casted_value + + return return_value @staticmethod def get_all(*args, **kwargs): @@ -1207,7 +1225,7 @@ class Database(object): def log_touched_tables(self, query, values=None): if values: query = frappe.safe_decode(self._cursor.mogrify(query, values)) - if query.strip().lower().split()[0] in ("insert", "delete", "update", "alter", "drop", "rename"): + if is_query_type(query, ("insert", "delete", "update", "alter", "drop", "rename")): # single_word_regex is designed to match following patterns # `tabXxx`, tabXxx and "tabXxx" diff --git a/frappe/integrations/doctype/ldap_settings/ldap_settings.py b/frappe/integrations/doctype/ldap_settings/ldap_settings.py index a14124234f..96007ee918 100644 --- a/frappe/integrations/doctype/ldap_settings/ldap_settings.py +++ b/frappe/integrations/doctype/ldap_settings/ldap_settings.py @@ -120,7 +120,7 @@ class LDAPSettings(Document): def get_ldap_client_settings(): # return the settings to be used on the client side. result = {"enabled": False} - ldap = frappe.get_doc("LDAP Settings") + ldap = frappe.get_cached_doc("LDAP Settings") if ldap.enabled: result["enabled"] = True result["method"] = "frappe.integrations.doctype.ldap_settings.ldap_settings.login" diff --git a/frappe/query_builder/utils.py b/frappe/query_builder/utils.py index 69aee9b350..c75e380d49 100644 --- a/frappe/query_builder/utils.py +++ b/frappe/query_builder/utils.py @@ -64,7 +64,6 @@ def patch_query_execute(): This excludes the use of `frappe.db.sql` method while executing the query object """ - from frappe.utils.safe_exec import check_safe_sql_query def execute_query(query, *args, **kwargs): query, params = prepare_query(query) @@ -73,6 +72,8 @@ def patch_query_execute(): def prepare_query(query): import inspect + from frappe.utils.safe_exec import check_safe_sql_query + param_collector = NamedParameterWrapper() query = query.get_sql(param_wrapper=param_collector) if frappe.flags.in_safe_exec and not check_safe_sql_query(query, throw=False): @@ -103,6 +104,7 @@ def patch_query_execute(): builder_class.run = execute_query builder_class.walk = prepare_query + frappe._qb_patched = True def patch_query_aggregation(): @@ -113,3 +115,4 @@ def patch_query_aggregation(): frappe.qb.min = _min frappe.qb.avg = _avg frappe.qb.sum = _sum + frappe._qb_patched = True diff --git a/frappe/sessions.py b/frappe/sessions.py index 6e0ce73732..67b58e1d89 100644 --- a/frappe/sessions.py +++ b/frappe/sessions.py @@ -187,7 +187,7 @@ def get(): bootinfo["translated_search_doctypes"] = frappe.get_hooks("translated_search_doctypes") bootinfo["disable_async"] = frappe.conf.disable_async - bootinfo["setup_complete"] = cint(frappe.db.get_single_value("System Settings", "setup_complete")) + bootinfo["setup_complete"] = cint(frappe.get_system_settings("setup_complete")) bootinfo["desk_theme"] = frappe.db.get_value("User", frappe.session.user, "desk_theme") or "Light" diff --git a/frappe/tests/test_scheduler.py b/frappe/tests/test_scheduler.py index 5161e1e80f..c6b381e487 100644 --- a/frappe/tests/test_scheduler.py +++ b/frappe/tests/test_scheduler.py @@ -1,18 +1,16 @@ +import os import time from unittest import TestCase +from unittest.mock import patch import frappe -from frappe.core.doctype.scheduled_job_type.scheduled_job_type import sync_jobs +from frappe.core.doctype.scheduled_job_type.scheduled_job_type import ScheduledJobType, sync_jobs from frappe.utils import add_days, get_datetime from frappe.utils.background_jobs import enqueue from frappe.utils.doctor import purge_pending_jobs from frappe.utils.scheduler import enqueue_events, is_dormant, schedule_jobs_based_on_activity -def test_timeout(): - time.sleep(100) - - def test_timeout_10(): time.sleep(10) @@ -23,6 +21,11 @@ def test_method(): class TestScheduler(TestCase): def setUp(self): + frappe.db.rollback() + + if not os.environ.get("CI"): + return + purge_pending_jobs() if not frappe.get_all("Scheduled Job Type", limit=1): sync_jobs() @@ -44,15 +47,9 @@ class TestScheduler(TestCase): def test_queue_peeking(self): job = get_test_job() - self.assertTrue(job.enqueue()) - job.db_set("last_execution", "2010-01-01 00:00:00") - frappe.db.commit() - - time.sleep(0.5) - - # 1st job is in the queue (or running), don't enqueue it again - self.assertFalse(job.enqueue()) - frappe.db.delete("Scheduled Job Log", {"scheduled_job_type": job.name}) + with patch.object(job, "is_job_in_queue", return_value=True): + # 1st job is in the queue (or running), don't enqueue it again + self.assertFalse(job.enqueue()) def test_is_dormant(self): self.assertTrue(is_dormant(check_time=get_datetime("2100-01-01 00:00:00"))) @@ -88,22 +85,10 @@ class TestScheduler(TestCase): ) ) - frappe.db.rollback() - def test_job_timeout(self): - return - job = enqueue(test_timeout, timeout=10) - count = 5 - while count > 0: - count -= 1 - time.sleep(5) - if job.get_status() == "failed": - break - - self.assertTrue(job.is_failed) - - -def get_test_job(method="frappe.tests.test_scheduler.test_timeout_10", frequency="All"): +def get_test_job( + method="frappe.tests.test_scheduler.test_timeout_10", frequency="All" +) -> ScheduledJobType: if not frappe.db.exists("Scheduled Job Type", dict(method=method)): job = frappe.get_doc( dict( diff --git a/frappe/utils/background_jobs.py b/frappe/utils/background_jobs.py index ce8e44665a..f49c641673 100755 --- a/frappe/utils/background_jobs.py +++ b/frappe/utils/background_jobs.py @@ -3,7 +3,7 @@ import socket import time from collections import defaultdict from functools import lru_cache -from typing import List +from typing import TYPE_CHECKING, List from uuid import uuid4 import redis @@ -19,6 +19,9 @@ from frappe.utils import cstr, get_bench_id from frappe.utils.commands import log from frappe.utils.redis_queue import RedisQueue +if TYPE_CHECKING: + from rq.job import Job + @lru_cache() def get_queues_timeout(): @@ -52,7 +55,7 @@ def enqueue( *, at_front=False, **kwargs, -): +) -> "Job": """ Enqueue method to be executed using a background worker diff --git a/frappe/website/router.py b/frappe/website/router.py index e9f0d0f09c..8c21501a4e 100644 --- a/frappe/website/router.py +++ b/frappe/website/router.py @@ -8,7 +8,7 @@ import re from werkzeug.routing import Map, NotFound, Rule import frappe -from frappe.website.utils import extract_title +from frappe.website.utils import extract_title, get_frontmatter def get_page_info_from_web_page_with_dynamic_routes(path): @@ -161,26 +161,6 @@ def get_page_info(path, app, start, basepath=None, app_path=None, fname=None): return page_info -def get_frontmatter(string): - """ - Reference: https://github.com/jonbeebe/frontmatter - """ - import yaml - - fmatter = "" - body = "" - result = re.compile(r"^\s*(?:---|\+\+\+)(.*?)(?:---|\+\+\+)\s*(.+)$", re.S | re.M).search(string) - - if result: - fmatter = result.group(1) - body = result.group(2) - - return { - "attributes": yaml.safe_load(fmatter), - "body": body, - } - - def setup_source(page_info): """Get the HTML source of the template""" jenv = frappe.get_jenv() diff --git a/frappe/www/about.py b/frappe/www/about.py index fd2331d9f6..a233bfd311 100644 --- a/frappe/www/about.py +++ b/frappe/www/about.py @@ -7,6 +7,6 @@ sitemap = 1 def get_context(context): - context.doc = frappe.get_doc("About Us Settings", "About Us Settings") + context.doc = frappe.get_cached_doc("About Us Settings") return context diff --git a/frappe/www/app.py b/frappe/www/app.py index f1b62a0899..9a8c80d6b1 100644 --- a/frappe/www/app.py +++ b/frappe/www/app.py @@ -32,8 +32,6 @@ def get_context(context): frappe.db.commit() - desk_theme = frappe.db.get_value("User", frappe.session.user, "desk_theme") - boot_json = frappe.as_json(boot) # remove script tags from boot @@ -52,7 +50,7 @@ def get_context(context): "lang": frappe.local.lang, "sounds": hooks["sounds"], "boot": boot if context.get("for_mobile") else boot_json, - "desk_theme": desk_theme or "Light", + "desk_theme": boot.get("desk_theme") or "Light", "csrf_token": csrf_token, "google_analytics_id": frappe.conf.get("google_analytics_id"), "google_analytics_anonymize_ip": frappe.conf.get("google_analytics_anonymize_ip"), diff --git a/frappe/www/login.py b/frappe/www/login.py index 1b9a8c239a..119dfefcd7 100644 --- a/frappe/www/login.py +++ b/frappe/www/login.py @@ -36,22 +36,14 @@ def get_context(context): frappe.local.flags.redirect_location = redirect_to raise frappe.Redirect - # get settings from site config context.no_header = True context.for_test = "login.html" context["title"] = "Login" context["provider_logins"] = [] - context["disable_signup"] = frappe.utils.cint( - frappe.db.get_single_value("Website Settings", "disable_signup") - ) - context["logo"] = ( - frappe.db.get_single_value("Website Settings", "app_logo") - or frappe.get_hooks("app_logo_url")[-1] - ) + context["disable_signup"] = frappe.utils.cint(frappe.get_website_settings("disable_signup")) + context["logo"] = frappe.get_website_settings("app_logo") or frappe.get_hooks("app_logo_url")[-1] context["app_name"] = ( - frappe.db.get_single_value("Website Settings", "app_name") - or frappe.get_system_settings("app_name") - or _("Frappe") + frappe.get_website_settings("app_name") or frappe.get_system_settings("app_name") or _("Frappe") ) signup_form_template = frappe.get_hooks("signup_form_template") @@ -61,38 +53,41 @@ def get_context(context): path = frappe.get_attr(signup_form_template[-1])() else: path = "frappe/templates/signup.html" + if path: context["signup_form_template"] = frappe.get_template(path).render() - providers = [ - i.name - for i in frappe.get_all("Social Login Key", filters={"enable_social_login": 1}, order_by="name") - ] + providers = frappe.get_all( + "Social Login Key", + filters={"enable_social_login": 1}, + fields=["name", "client_id", "base_url", "provider_name", "icon"], + order_by="name", + ) + for provider in providers: - client_id, base_url = frappe.get_value("Social Login Key", provider, ["client_id", "base_url"]) - client_secret = get_decrypted_password("Social Login Key", provider, "client_secret") - provider_name = frappe.get_value("Social Login Key", provider, "provider_name") + client_secret = get_decrypted_password("Social Login Key", provider.name, "client_secret") + if not client_secret: + continue icon = None - icon_url = frappe.get_value("Social Login Key", provider, "icon") - if icon_url: - if provider_name != "Custom": - icon = "{1}".format(icon_url, provider_name) + if provider.icon: + if provider.provider_name == "Custom": + icon = get_icon_html(provider.icon, small=True) else: - icon = get_icon_html(icon_url, small=True) + icon = f"{provider.provider_name}" - if get_oauth_keys(provider) and client_secret and client_id and base_url: + if provider.client_id and provider.base_url and get_oauth_keys(provider.name): context.provider_logins.append( { - "name": provider, - "provider_name": provider_name, - "auth_url": get_oauth2_authorize_url(provider, redirect_to), + "name": provider.name, + "provider_name": provider.provider_name, + "auth_url": get_oauth2_authorize_url(provider.name, redirect_to), "icon": icon, } ) context["social_login"] = True - ldap_settings = LDAPSettings.get_ldap_client_settings() - context["ldap_settings"] = ldap_settings + + context["ldap_settings"] = LDAPSettings.get_ldap_client_settings() login_label = [_("Email")] @@ -102,7 +97,7 @@ def get_context(context): if frappe.utils.cint(frappe.get_system_settings("allow_login_using_user_name")): login_label.append(_("Username")) - context["login_label"] = " {0} ".format(_("or")).join(login_label) + context["login_label"] = f" {_('or')} ".join(login_label) return context