From 5c8856d66ea02f6d35e026fd8131d92d111f64e7 Mon Sep 17 00:00:00 2001 From: Abhishek Saxena <33656173+saxenabhishek@users.noreply.github.com> Date: Tue, 12 Apr 2022 10:37:25 +0530 Subject: [PATCH] refactor: db.sql calls to frappe.qb (#16107) # Changes - Introduces `subqry` class to use in where clause when there is a non-column condition. eg. > .where(subqry(no_of_roles) == 0) - Convert SQL queries to frappe.qb # Testing Functions with query refactors - frappe.boot.get_user_pages_or_reports() -> Same output of `get_bootinfo()` as develop - frappe.boot.get_unseen_notes() -> Forms the same query as develop ```sql SELECT `name`,`title`,`content`,`notify_on_every_login` FROM `tabNote` WHERE `notify_on_every_login`=1 AND `expire_notification_on`>'2022-03-30 01:10:53.393874' AND (SELECT `nsb`.`user` FROM `tabNote Seen By` `nsb` WHERE `nsb`.`parent`=`tabNote`.`name`) NOT IN ('Administrator') ``` - frappe.installer._delete_doctypes() -> installed and uninsalled a dummy app to drop tables ### Not tested - frappe.make_property_setter() - frappe.realtime.get_pending_tasks_for_doc() [whitelist method] - frappe.sessions.Session.start() - frappe.twofactor.cache_2fa_data() --- frappe/__init__.py | 7 ++- frappe/boot.py | 113 +++++++++++++++++++--------------- frappe/query_builder/terms.py | 1 + frappe/realtime.py | 5 -- frappe/sessions.py | 18 +++--- frappe/twofactor.py | 17 +++-- 6 files changed, 86 insertions(+), 75 deletions(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index f92f76c98c..08dcb9bf3f 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -1290,7 +1290,12 @@ def make_property_setter(args, ignore_validate=False, validate_fields_for_doctyp {'parent': 'DocField', 'fieldname': args.property}, 'fieldtype') or 'Data' if not args.doctype: - doctype_list = db.sql_list('select distinct parent from tabDocField where fieldname=%s', args.fieldname) + DocField_doctype = qb.DocType("DocField") + doctype_list = ( + qb.from_(DocField_doctype).select(DocField_doctype.parent) + .where(DocField_doctype.fieldname == args.fieldname).distinct() + ).run(as_list=True) + else: doctype_list = [args.doctype] diff --git a/frappe/boot.py b/frappe/boot.py index b5008f778a..ae5bdfa8c0 100644 --- a/frappe/boot.py +++ b/frappe/boot.py @@ -18,6 +18,10 @@ from frappe.social.doctype.energy_point_log.energy_point_log import get_energy_p from frappe.model.base_document import get_controller from frappe.core.doctype.navbar_settings.navbar_settings import get_navbar_settings, get_app_logo from frappe.utils import get_time_zone, add_user_info +from frappe.query_builder import DocType +from frappe.query_builder.functions import Count +from frappe.query_builder.terms import subqry + def get_bootinfo(): """build and return boot info""" @@ -129,43 +133,53 @@ def get_user_pages_or_reports(parent, cache=False): roles = frappe.get_roles() has_role = {} - column = get_column(parent) + + page = DocType("Page") + report = DocType("Report") + + if parent == "Report": + columns = (report.name.as_("title"), report.ref_doctype, report.report_type) + else: + columns = (page.title.as_("title"), ) + + + customRole = DocType("Custom Role") + hasRole = DocType("Has Role") + parentTable = DocType(parent) # get pages or reports set on custom role - pages_with_custom_roles = frappe.db.sql(""" - select - `tabCustom Role`.{field} as name, - `tabCustom Role`.modified, - `tabCustom Role`.ref_doctype, - {column} - from `tabCustom Role`, `tabHas Role`, `tab{parent}` - where - `tabHas Role`.parent = `tabCustom Role`.name - and `tab{parent}`.name = `tabCustom Role`.{field} - and `tabCustom Role`.{field} is not null - and `tabHas Role`.role in ({roles}) - """.format(field=parent.lower(), parent=parent, column=column, - roles = ', '.join(['%s']*len(roles))), roles, as_dict=1) + pages_with_custom_roles = ( + frappe.qb.from_(customRole).from_(hasRole).from_(parentTable) + .select(customRole[parent.lower()].as_("name"), customRole.modified, customRole.ref_doctype, *columns) + .where( + (hasRole.parent == customRole.name) + & (parentTable.name == customRole[parent.lower()]) + & (customRole[parent.lower()].isnotnull()) + & (hasRole.role.isin(roles))) + ).run(as_dict=True) for p in pages_with_custom_roles: has_role[p.name] = {"modified":p.modified, "title": p.title, "ref_doctype": p.ref_doctype} - pages_with_standard_roles = frappe.db.sql(""" - select distinct - `tab{parent}`.name as name, - `tab{parent}`.modified, - {column} - from `tabHas Role`, `tab{parent}` - where - `tabHas Role`.role in ({roles}) - and `tabHas Role`.parent = `tab{parent}`.name - and `tab{parent}`.`name` not in ( - select `tabCustom Role`.{field} from `tabCustom Role` - where `tabCustom Role`.{field} is not null) - {condition} - """.format(parent=parent, column=column, roles = ', '.join(['%s']*len(roles)), - field=parent.lower(), condition="and `tabReport`.disabled=0" if parent == "Report" else ""), - roles, as_dict=True) + subq = ( + frappe.qb.from_(customRole).select(customRole[parent.lower()]) + .where(customRole[parent.lower()].isnotnull()) + ) + + pages_with_standard_roles = ( + frappe.qb.from_(hasRole).from_(parentTable) + .select(parentTable.name.as_("name"), parentTable.modified, *columns) + .where( + (hasRole.role.isin(roles)) + & (hasRole.parent == parentTable.name) + & (parentTable.name.notin(subq)) + ).distinct() + ) + + if parent == "Report": + pages_with_standard_roles = pages_with_standard_roles.where(report.disabled == 0) + + pages_with_standard_roles = pages_with_standard_roles.run(as_dict=True) for p in pages_with_standard_roles: if p.name not in has_role: @@ -173,16 +187,16 @@ def get_user_pages_or_reports(parent, cache=False): if parent == "Report": has_role[p.name].update({'ref_doctype': p.ref_doctype}) + no_of_roles = (frappe.qb.from_(hasRole).select(Count("*")) + .where(hasRole.parent == parentTable.name) + ) + # pages with no role are allowed if parent =="Page": - pages_with_no_roles = frappe.db.sql(""" - select - `tab{parent}`.name, `tab{parent}`.modified, {column} - from `tab{parent}` - where - (select count(*) from `tabHas Role` - where `tabHas Role`.parent=`tab{parent}`.`name`) = 0 - """.format(parent=parent, column=column), as_dict=1) + + pages_with_no_roles = (frappe.qb.from_(parentTable).select(parentTable.name, parentTable.modified, *columns) + .where(subqry(no_of_roles) == 0) + ).run(as_dict=True) for p in pages_with_no_roles: if p.name not in has_role: @@ -201,13 +215,6 @@ def get_user_pages_or_reports(parent, cache=False): _cache.set_value('has_role:' + parent, has_role, frappe.session.user, 21600) return has_role -def get_column(doctype): - column = "`tabPage`.title as title" - if doctype == "Report": - column = "`tabReport`.`name` as title, `tabReport`.ref_doctype, `tabReport`.report_type" - - return column - def load_translations(bootinfo): messages = frappe.get_lang_dict("boot") @@ -271,10 +278,16 @@ def load_print_css(bootinfo, print_settings): bootinfo.print_css = frappe.www.printview.get_print_style(print_settings.print_style or "Redesign", for_legacy=True) def get_unseen_notes(): - return frappe.db.sql('''select `name`, title, content, notify_on_every_login from `tabNote` where notify_on_login=1 - and expire_notification_on > %s and %s not in - (select user from `tabNote Seen By` nsb - where nsb.parent=`tabNote`.name)''', (frappe.utils.now(), frappe.session.user), as_dict=True) + note = DocType("Note") + nsb = DocType("Note Seen By").as_("nsb") + + return ( + frappe.qb.from_(note).select(note.name, note.title, note.content, note.notify_on_every_login) + .where( + (note.notify_on_every_login == 1) + & (note.expire_notification_on > frappe.utils.now()) + & (subqry(frappe.qb.from_(nsb).select(nsb.user).where(nsb.parent == note.name)).notin([frappe.session.user]))) + ).run(as_dict=1) def get_success_action(): return frappe.get_all("Success Action", fields=["*"]) diff --git a/frappe/query_builder/terms.py b/frappe/query_builder/terms.py index d3785e049a..aee6bf029e 100644 --- a/frappe/query_builder/terms.py +++ b/frappe/query_builder/terms.py @@ -103,6 +103,7 @@ class ParameterizedFunction(Function): return function_sql + class subqry(Criterion): def __init__(self, subq: QueryBuilder, alias: Optional[str] = None,) -> None: super().__init__(alias) diff --git a/frappe/realtime.py b/frappe/realtime.py index 940a3220a4..dc47599923 100644 --- a/frappe/realtime.py +++ b/frappe/realtime.py @@ -9,11 +9,6 @@ import redis redis_server = None -@frappe.whitelist() -def get_pending_tasks_for_doc(doctype, docname): - return frappe.db.sql_list("select name from `tabAsync Task` where status in ('Queued', 'Running') and reference_doctype=%s and reference_name=%s", (doctype, docname)) - - def publish_progress(percent, title=None, doctype=None, docname=None, description=None): publish_realtime('progress', {'percent': percent, 'title': title, 'description': description}, user=frappe.session.user, doctype=doctype, docname=docname) diff --git a/frappe/sessions.py b/frappe/sessions.py index 6a5771b617..4bbcaaa2ae 100644 --- a/frappe/sessions.py +++ b/frappe/sessions.py @@ -244,16 +244,14 @@ class Session: # update user user = frappe.get_doc("User", self.data['user']) - frappe.db.sql("""UPDATE `tabUser` - SET - last_login = %(now)s, - last_ip = %(ip)s, - last_active = %(now)s - WHERE name=%(name)s""", { - 'now': frappe.utils.now(), - 'ip': frappe.local.request_ip, - 'name': self.data['user'] - }) + user_doctype=frappe.qb.DocType("User") + (frappe.qb.update(user_doctype) + .set(user_doctype.last_login, frappe.utils.now()) + .set(user_doctype.last_ip, frappe.local.request_ip) + .set(user_doctype.last_active, frappe.utils.now()) + .where(user_doctype.name == self.data['user']) + ).run() + user.run_notifications("before_change") user.run_notifications("on_update") frappe.db.commit() diff --git a/frappe/twofactor.py b/frappe/twofactor.py index bd49d588b0..bb063faf5a 100644 --- a/frappe/twofactor.py +++ b/frappe/twofactor.py @@ -84,22 +84,21 @@ def two_factor_is_enabled_for_(user): return False if isinstance(user, str): - user = frappe.get_doc('User', user) + user = frappe.get_doc("User", user) - roles = [frappe.db.escape(d.role) for d in user.roles or []] - roles.append("'All'") + roles = [d.role for d in user.roles or []] + ["All"] - query = """SELECT `name` - FROM `tabRole` - WHERE `two_factor_auth`= 1 - AND `name` IN ({0}) - LIMIT 1""".format(", ".join(roles)) + role_doctype = frappe.qb.DocType("Role") + no_of_users = frappe.db.count(role_doctype, filters= + ((role_doctype.two_factor_auth == 1) & (role_doctype.name.isin(roles))), + ) - if len(frappe.db.sql(query)) > 0: + if int(no_of_users) > 0: return True return False + def get_otpsecret_for_(user): '''Set OTP Secret for user even if not set.''' otp_secret = frappe.db.get_default(user + '_otpsecret')