diff --git a/frappe/__init__.py b/frappe/__init__.py index 8a8b70afe3..3ca2082ddb 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -35,6 +35,7 @@ from frappe.query_builder import ( patch_query_execute, patch_query_aggregation, ) +from frappe.utils.data import cstr __version__ = '14.0.0-dev' @@ -214,6 +215,7 @@ def init(site, sites_path=None, new_site=False): local.cache = {} local.document_cache = {} local.meta_cache = {} + local.autoincremented_doctypes = set() local.form_dict = _dict() local.session = _dict() local.dev_server = _dev_server @@ -1001,7 +1003,7 @@ def get_module(modulename): def scrub(txt): """Returns sluggified string. e.g. `Sales Order` becomes `sales_order`.""" - return txt.replace(' ', '_').replace('-', '_').lower() + return cstr(txt).replace(' ', '_').replace('-', '_').lower() def unscrub(txt): """Returns titlified string. e.g. `sales_order` becomes `Sales Order`.""" diff --git a/frappe/core/doctype/data_export/exporter.py b/frappe/core/doctype/data_export/exporter.py index 79570d5048..9f1492af19 100644 --- a/frappe/core/doctype/data_export/exporter.py +++ b/frappe/core/doctype/data_export/exporter.py @@ -324,7 +324,7 @@ class DataExporter: d = doc.copy() meta = frappe.get_meta(dt) if self.all_doctypes: - d.name = '"'+ d.name+'"' + d.name = f'"{d.name}"' if len(rows) < rowidx + 1: rows.append([""] * (len(self.columns) + 1)) diff --git a/frappe/core/doctype/doctype/doctype.js b/frappe/core/doctype/doctype/doctype.js index f250a6a109..88cc5577a6 100644 --- a/frappe/core/doctype/doctype/doctype.js +++ b/frappe/core/doctype/doctype/doctype.js @@ -61,6 +61,13 @@ frappe.ui.form.on('DocType', { frm.events.set_naming_rule_description(frm); }, + istable: (frm) => { + if (frm.doc.istable && frm.is_new()) { + frm.set_value('autoname', 'autoincrement'); + frm.set_value('allow_rename', 0); + } + }, + naming_rule: function(frm) { // set the "autoname" property based on naming_rule if (frm.doc.naming_rule && !frm.__from_autoname) { @@ -70,6 +77,10 @@ frappe.ui.form.on('DocType', { if (frm.doc.naming_rule=='Set by user') { frm.set_value('autoname', 'Prompt'); + } else if (frm.doc.naming_rule === 'Autoincrement') { + frm.set_value('autoname', 'autoincrement'); + // set allow rename to be false when using autoincrement + frm.set_value('allow_rename', 0); } else if (frm.doc.naming_rule=='By fieldname') { frm.set_value('autoname', 'field:'); } else if (frm.doc.naming_rule=='By "Naming Series" field') { @@ -91,6 +102,7 @@ frappe.ui.form.on('DocType', { set_naming_rule_description(frm) { let naming_rule_description = { 'Set by user': '', + 'Autoincrement': 'Uses Auto Increment feature of database.
WARNING: After using this option, any other naming option will not be accessible.', 'By fieldname': 'Format: field:[fieldname]. Valid fieldname must exist', 'By "Naming Series" field': 'Format: naming_series:[fieldname]. Fieldname called naming_series must exist', 'Expression': 'Format: format:EXAMPLE-{MM}morewords{fieldname1}-{fieldname2}-{#####} - Replace all braced words (fieldnames, date words (DD, MM, YY), series) with their value. Outside braces, any characters can be used.', @@ -111,6 +123,8 @@ frappe.ui.form.on('DocType', { frm.__from_autoname = true; if (frm.doc.autoname.toLowerCase() === 'prompt') { frm.set_value('naming_rule', 'Set by user'); + } else if (frm.doc.autoname.toLowerCase() === 'autoincrement') { + frm.set_value('naming_rule', 'Autoincrement'); } else if (frm.doc.autoname.startsWith('field:')) { frm.set_value('naming_rule', 'By fieldname'); } else if (frm.doc.autoname.startsWith('naming_series:')) { diff --git a/frappe/core/doctype/doctype/doctype.json b/frappe/core/doctype/doctype/doctype.json index 2bba4127bb..8169a59566 100644 --- a/frappe/core/doctype/doctype/doctype.json +++ b/frappe/core/doctype/doctype/doctype.json @@ -208,7 +208,7 @@ "label": "Naming" }, { - "description": "Naming Options:\n
  1. field:[fieldname] - By Field
  2. naming_series: - By Naming Series (field called naming_series must be present
  3. Prompt - Prompt user for a name
  4. [series] - Series by prefix (separated by a dot); for example PRE.#####
    5. \n
  5. format:EXAMPLE-{MM}morewords{fieldname1}-{fieldname2}-{#####} - Replace all braced words (fieldnames, date words (DD, MM, YY), series) with their value. Outside braces, any characters can be used.
", + "description": "Naming Options:\n
  1. field:[fieldname] - By Field
  2. autoincrement - Uses Databases' Auto Increment feature
  3. naming_series: - By Naming Series (field called naming_series must be present
  4. Prompt - Prompt user for a name
  5. [series] - Series by prefix (separated by a dot); for example PRE.#####
    6. \n
  6. format:EXAMPLE-{MM}morewords{fieldname1}-{fieldname2}-{#####} - Replace all braced words (fieldnames, date words (DD, MM, YY), series) with their value. Outside braces, any characters can be used.
", "fieldname": "autoname", "fieldtype": "Data", "label": "Auto Name", @@ -216,6 +216,7 @@ "oldfieldtype": "Data" }, { + "depends_on": "eval:doc.naming_rule !== \"Autoincrement\"", "fieldname": "name_case", "fieldtype": "Select", "label": "Name Case", @@ -282,6 +283,7 @@ }, { "default": "1", + "depends_on": "eval:doc.naming_rule !== \"Autoincrement\"", "fieldname": "allow_rename", "fieldtype": "Check", "label": "Allow Rename", @@ -565,7 +567,7 @@ "fieldtype": "Select", "label": "Naming Rule", "length": 40, - "options": "\nSet by user\nBy fieldname\nBy \"Naming Series\" field\nExpression\nExpression (old style)\nRandom\nBy script" + "options": "\nSet by user\nAutoincrement\nBy fieldname\nBy \"Naming Series\" field\nExpression\nExpression (old style)\nRandom\nBy script" }, { "fieldname": "migration_hash", @@ -593,6 +595,7 @@ ], "icon": "fa fa-bolt", "idx": 6, + "index_web_pages_for_search": 1, "links": [ { "group": "Views", @@ -670,10 +673,11 @@ "link_fieldname": "reference_doctype" } ], - "modified": "2022-01-07 16:07:06.196534", + "modified": "2022-02-15 21:47:16.467217", "modified_by": "Administrator", "module": "Core", "name": "DocType", + "naming_rule": "Set by user", "owner": "Administrator", "permissions": [ { @@ -703,5 +707,6 @@ "show_name_in_global_search": 1, "sort_field": "modified", "sort_order": "DESC", + "states": [], "track_changes": 1 } \ No newline at end of file diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index 5f82abac1f..d1401cc91e 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -60,6 +60,7 @@ class DocType(Document): self.check_developer_mode() + self.validate_autoname() self.validate_name() self.set_defaults_for_single_and_table() @@ -714,6 +715,19 @@ class DocType(Document): self.name) return max_idx and max_idx[0][0] or 0 + def validate_autoname(self): + if not self.is_new(): + doc_before_save = self.get_doc_before_save() + if doc_before_save: + if (self.autoname == "autoincrement" and doc_before_save.autoname != "autoincrement") \ + or (self.autoname != "autoincrement" and doc_before_save.autoname == "autoincrement"): + frappe.throw(_("Cannot change to/from Autoincrement naming rule")) + + else: + if self.autoname == "autoincrement": + self.allow_rename = 0 + frappe.local.autoincremented_doctypes.add(self.name) + def validate_name(self, name=None): if not name: name = self.name diff --git a/frappe/core/doctype/doctype/test_doctype.py b/frappe/core/doctype/doctype/test_doctype.py index cb22f581c6..dc6d14b451 100644 --- a/frappe/core/doctype/doctype/test_doctype.py +++ b/frappe/core/doctype/doctype/test_doctype.py @@ -505,7 +505,23 @@ class TestDocType(unittest.TestCase): dt.delete() -def new_doctype(name, unique=0, depends_on='', fields=None): + def test_autoincremented_doctype_transition(self): + frappe.delete_doc("testy_autoinc_dt") + dt = new_doctype("testy_autoinc_dt", autoincremented=True).insert(ignore_permissions=True) + dt.autoname = "hash" + + try: + dt.save(ignore_permissions=True) + except frappe.ValidationError as e: + self.assertEqual(e.args[0], "Cannot change to/from Autoincrement naming rule") + else: + self.fail("Shouldnt be possible to transition autoincremented doctype to any other naming rule") + finally: + # cleanup + dt.delete(ignore_permissions=True) + + +def new_doctype(name, unique=0, depends_on='', fields=None, autoincremented=False): doc = frappe.get_doc({ "doctype": "DocType", "module": "Core", @@ -521,7 +537,8 @@ def new_doctype(name, unique=0, depends_on='', fields=None): "role": "System Manager", "read": 1, }], - "name": name + "name": name, + "autoname": "autoincrement" if autoincremented else "" }) if fields: diff --git a/frappe/core/doctype/server_script/test_server_script.py b/frappe/core/doctype/server_script/test_server_script.py index d9381bcd16..aa4507b858 100644 --- a/frappe/core/doctype/server_script/test_server_script.py +++ b/frappe/core/doctype/server_script/test_server_script.py @@ -112,7 +112,10 @@ class TestServerScript(unittest.TestCase): self.assertEqual(frappe.get_doc('Server Script', 'test_return_value').execute_method(), 'hello') def test_permission_query(self): - self.assertTrue('where (1 = 1)' in frappe.db.get_list('ToDo', run=False)) + if frappe.conf.db_type == "mariadb": + self.assertTrue('where (1 = 1)' in frappe.db.get_list('ToDo', run=False)) + else: + self.assertTrue('where (1 = \'1\')' in frappe.db.get_list('ToDo', run=False)) self.assertTrue(isinstance(frappe.db.get_list('ToDo'), list)) def test_attribute_error(self): diff --git a/frappe/database/database.py b/frappe/database/database.py index a4eca64d8d..1251a323d3 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -142,8 +142,6 @@ class Database(object): self.log_query(query, values, debug, explain) if values!=(): - if isinstance(values, dict): - values = dict(values) # MySQL-python==1.2.5 hack! if not isinstance(values, (dict, tuple, list)): diff --git a/frappe/database/mariadb/schema.py b/frappe/database/mariadb/schema.py index fd4bfc6dd0..3b7aa443f2 100644 --- a/frappe/database/mariadb/schema.py +++ b/frappe/database/mariadb/schema.py @@ -1,12 +1,16 @@ import frappe from frappe import _ from frappe.database.schema import DBTable +from frappe.database.sequence import create_sequence +from frappe.model import log_types + class MariaDBTable(DBTable): def create(self): additional_definitions = "" engine = self.meta.get("engine") or "InnoDB" varchar_len = frappe.db.VARCHAR_LEN + name_column = f"name varchar({varchar_len}) primary key" # columns column_defs = self.get_column_definitions() @@ -29,9 +33,27 @@ class MariaDBTable(DBTable): ) ) + ',\n' + # creating sequence(s) + if (not self.meta.issingle and self.meta.autoname == "autoincrement")\ + or self.doctype in log_types: + + # NOTE: using a very small cache - as during backup, if the sequence was used in anyform, + # it drops the cache and uses the next non cached value in setval func and + # puts that in the backup file, which will start the counter + # from that value when inserting any new record in the doctype. + # By default the cache is 1000 which will mess up the sequence when + # using the system after a restore. + # issue link: https://jira.mariadb.org/browse/MDEV-21786 + create_sequence(self.doctype, check_not_exists=True, cache=50) + + # NOTE: not used nextval func as default as the ability to restore + # database with sequences has bugs in mariadb and gives a scary error. + # issue link: https://jira.mariadb.org/browse/MDEV-21786 + name_column = "name bigint primary key" + # create table query = f"""create table `{self.table_name}` ( - name varchar({varchar_len}) not null primary key, + {name_column}, creation datetime(6), modified datetime(6), modified_by varchar({varchar_len}), diff --git a/frappe/database/postgres/database.py b/frappe/database/postgres/database.py index a20ffe17a5..eb3e33d39c 100644 --- a/frappe/database/postgres/database.py +++ b/frappe/database/postgres/database.py @@ -99,8 +99,13 @@ class PostgresDatabase(Database): return db_size[0].get('database_size') # pylint: disable=W0221 - def sql(self, query, *args, **kwargs): - return super(PostgresDatabase, self).sql(modify_query(query), *args, **kwargs) + def sql(self, query, values=(), *args, **kwargs): + return super(PostgresDatabase, self).sql( + modify_query(query), + modify_values(values), + *args, + **kwargs + ) def get_tables(self, cached=True): return [d[0] for d in self.sql("""select table_name @@ -333,10 +338,45 @@ def modify_query(query): if re.search('from tab', query, flags=re.IGNORECASE): query = re.sub(r'from tab([\w-]*)', r'from "tab\1"', query, flags=re.IGNORECASE) + # only find int (with/without signs), ignore decimals (with/without signs), ignore hashes (which start with numbers), + # drop .0 from decimals and add quotes around them + # + # >>> query = "c='abcd' , a >= 45, b = -45.0, c = 40, d=4500.0, e=3500.53, f=40psdfsd, g=9092094312, h=12.00023" + # >>> re.sub(r"([=><]+)\s*(?!\d+[a-zA-Z])(?![+-]?\d+\.\d\d+)([+-]?\d+)(\.0)?", r"\1 '\2'", query) + # "c='abcd' , a >= '45', b = '-45', c = '40', d= '4500', e=3500.53, f=40psdfsd, g= '9092094312', h=12.00023 + + query = re.sub(r"([=><]+)\s*(?!\d+[a-zA-Z])(?![+-]?\d+\.\d\d+)([+-]?\d+)(\.0)?", r"\1 '\2'", query) return query +def modify_values(values): + def stringify_value(value): + if isinstance(value, int): + value = str(value) + elif isinstance(value, float): + truncated_float = int(value) + if value == truncated_float: + value = str(truncated_float) + + return value + + if not values: + return values + + if isinstance(values, dict): + for k, v in values.items(): + values[k] = stringify_value(v) + elif isinstance(values, (tuple, list)): + new_values = [] + for val in values: + new_values.append(stringify_value(val)) + values = new_values + else: + values = stringify_value(values) + + return values + def replace_locate_with_strpos(query): # strpos is the locate equivalent in postgres if re.search(r'locate\(', query, flags=re.IGNORECASE): - query = re.sub(r'locate\(([^,]+),([^)]+)\)', r'strpos(\2, \1)', query, flags=re.IGNORECASE) + query = re.sub(r'locate\(([^,]+),([^)]+)(\)?)\)', r'strpos(\2\3, \1)', query, flags=re.IGNORECASE) return query diff --git a/frappe/database/postgres/schema.py b/frappe/database/postgres/schema.py index bb7ff20a26..b09f73300e 100644 --- a/frappe/database/postgres/schema.py +++ b/frappe/database/postgres/schema.py @@ -2,10 +2,14 @@ import frappe from frappe import _ from frappe.utils import cint, flt from frappe.database.schema import DBTable, get_definition +from frappe.database.sequence import create_sequence +from frappe.model import log_types + class PostgresTable(DBTable): def create(self): varchar_len = frappe.db.VARCHAR_LEN + name_column = f"name varchar({varchar_len}) primary key" additional_definitions = "" # columns @@ -26,9 +30,21 @@ class PostgresTable(DBTable): ) ) + # creating sequence(s) + if (not self.meta.issingle and self.meta.autoname == "autoincrement")\ + or self.doctype in log_types: + + # The sequence cache is per connection. + # Since we're opening and closing connections for every transaction this results in skipping the cache + # to the next non-cached value hence not using cache in postgres. + # ref: https://stackoverflow.com/questions/21356375/postgres-9-0-4-sequence-skipping-numbers + create_sequence(self.doctype, check_not_exists=True) + name_column = "name bigint primary key" + + # TODO: set docstatus length # create table frappe.db.sql(f"""create table `{self.table_name}` ( - name varchar({varchar_len}) not null primary key, + {name_column}, creation timestamp(6), modified timestamp(6), modified_by varchar({varchar_len}), diff --git a/frappe/database/sequence.py b/frappe/database/sequence.py new file mode 100644 index 0000000000..c89ba468bc --- /dev/null +++ b/frappe/database/sequence.py @@ -0,0 +1,76 @@ +from frappe import db, scrub + + +def create_sequence( + doctype_name: str, + *, + slug: str = "_id_seq", + check_not_exists: bool = False, + cycle: bool = False, + cache: int = 0, + start_value: int = 0, + increment_by: int = 0, + min_value: int = 0, + max_value: int = 0 +) -> str: + + query = "create sequence" + sequence_name = scrub(doctype_name + slug) + + if check_not_exists: + query += " if not exists" + + query += f" {sequence_name}" + + if cache: + query += f" cache {cache}" + else: + # in postgres, the default is cache 1 + if db.db_type == "mariadb": + query += " nocache" + + if start_value: + # default is 1 + query += f" start with {start_value}" + + if increment_by: + # default is 1 + query += f" increment by {increment_by}" + + if min_value: + # default is 1 + query += f" min value {min_value}" + + if max_value: + query += f" max value {max_value}" + + if not cycle: + if db.db_type == "mariadb": + query += " nocycle" + else: + query += " cycle" + + db.sql(query) + + return sequence_name + + +def get_next_val(doctype_name: str, slug: str = "_id_seq") -> int: + if db.db_type == "postgres": + return db.sql(f"select nextval(\'\"{scrub(doctype_name + slug)}\"\')")[0][0] + return db.sql(f"select nextval(`{scrub(doctype_name + slug)}`)")[0][0] + + +def set_next_val( + doctype_name: str, + next_val: int, + *, + slug: str = "_id_seq", + is_val_used :bool = False +) -> None: + + is_val_used = 0 if not is_val_used else 1 + if db.db_type == "postgres": + db.sql(f"SELECT SETVAL(\'\"{scrub(doctype_name + slug)}\"\', {next_val}, {is_val_used})") + else: + db.sql(f"SELECT SETVAL(`{scrub(doctype_name + slug)}`, {next_val}, {is_val_used})") diff --git a/frappe/desk/form/load.py b/frappe/desk/form/load.py index b5dfacb1d6..901e7a3d5e 100644 --- a/frappe/desk/form/load.py +++ b/frappe/desk/form/load.py @@ -10,6 +10,7 @@ import frappe.desk.form.meta from frappe.model.utils.user_settings import get_user_settings from frappe.permissions import get_doc_permissions from frappe.desk.form.document_follow import is_document_followed +from frappe.utils.data import cstr from frappe import _ from frappe import _dict from urllib.parse import quote @@ -356,7 +357,7 @@ def get_document_email(doctype, name): return None email = email.split("@") - return "{0}+{1}+{2}@{3}".format(email[0], quote(doctype), quote(name), email[1]) + return "{0}+{1}+{2}@{3}".format(email[0], quote(doctype), quote(cstr(name)), email[1]) def get_automatic_email_link(): return frappe.db.get_value("Email Account", {"enable_incoming": 1, "enable_automatic_linking": 1}, "email_id") diff --git a/frappe/desk/search.py b/frappe/desk/search.py index 3b76953ed1..b54ea46268 100644 --- a/frappe/desk/search.py +++ b/frappe/desk/search.py @@ -257,7 +257,7 @@ def scrub_custom_query(query, key, txt): def relevance_sorter(key, query, as_dict): value = _(key.name if as_dict else key[0]) return ( - value.lower().startswith(query.lower()) is not True, + cstr(value).lower().startswith(query.lower()) is not True, value ) diff --git a/frappe/email/doctype/newsletter/test_newsletter.py b/frappe/email/doctype/newsletter/test_newsletter.py index 8c1f803a46..b091c31c74 100644 --- a/frappe/email/doctype/newsletter/test_newsletter.py +++ b/frappe/email/doctype/newsletter/test_newsletter.py @@ -51,7 +51,7 @@ class TestNewsletterMixin: "reference_name": newsletter, }) frappe.delete_doc("Newsletter", newsletter) - frappe.db.delete("Newsletter Email Group", newsletter) + frappe.db.delete("Newsletter Email Group", {"parent": newsletter}) newsletters.remove(newsletter) def setup_email_group(self): diff --git a/frappe/event_streaming/doctype/event_update_log/event_update_log.py b/frappe/event_streaming/doctype/event_update_log/event_update_log.py index f4871be312..cd5100623c 100644 --- a/frappe/event_streaming/doctype/event_update_log/event_update_log.py +++ b/frappe/event_streaming/doctype/event_update_log/event_update_log.py @@ -203,12 +203,17 @@ def get_unread_update_logs(consumer_name, dt, dn): SELECT update_log.name FROM `tabEvent Update Log` update_log - JOIN `tabEvent Update Log Consumer` consumer ON consumer.parent = update_log.name + JOIN `tabEvent Update Log Consumer` consumer ON consumer.parent = %(log_name)s WHERE consumer.consumer = %(consumer)s AND update_log.ref_doctype = %(dt)s AND update_log.docname = %(dn)s - """, {'consumer': consumer_name, "dt": dt, "dn": dn}, as_dict=0)] + """, { + "consumer": consumer_name, + "dt": dt, + "dn": dn, + "log_name": "update_log.name" if frappe.conf.db_type == "mariadb" else "CAST(update_log.name AS VARCHAR)" + }, as_dict=0)] logs = frappe.get_all( 'Event Update Log', diff --git a/frappe/frappeclient.py b/frappe/frappeclient.py index 59db38584c..7a1587aae0 100644 --- a/frappe/frappeclient.py +++ b/frappe/frappeclient.py @@ -7,6 +7,7 @@ import json import requests import frappe +from frappe.utils.data import cstr class AuthError(Exception): @@ -122,7 +123,7 @@ class FrappeClient(object): '''Update a remote document :param doc: dict or Document object to be updated remotely. `name` is mandatory for this''' - url = self.url + "/api/resource/" + doc.get("doctype") + "/" + doc.get("name") + url = self.url + "/api/resource/" + doc.get("doctype") + "/" + cstr(doc.get("name")) res = self.session.put(url, data={"data":frappe.as_json(doc)}, verify=self.verify, headers=self.headers) return frappe._dict(self.post_process(res)) @@ -207,7 +208,7 @@ class FrappeClient(object): if fields: params["fields"] = json.dumps(fields) - res = self.session.get(self.url + "/api/resource/" + doctype + "/" + name, + res = self.session.get(self.url + "/api/resource/" + doctype + "/" + cstr(name), params=params, verify=self.verify, headers=self.headers) return self.post_process(res) diff --git a/frappe/model/base_document.py b/frappe/model/base_document.py index 8a81aa5610..3564b1ae11 100644 --- a/frappe/model/base_document.py +++ b/frappe/model/base_document.py @@ -475,7 +475,7 @@ class BaseDocument(object): d = self.get_valid_dict(convert_dates_to_str=True, ignore_nulls = self.doctype in DOCTYPES_FOR_DOCTYPE) # don't update name, as case might've been changed - name = d['name'] + name = cstr(d['name']) del d['name'] columns = list(d) diff --git a/frappe/model/db_query.py b/frappe/model/db_query.py index a6b96e8fb5..ba1f157607 100644 --- a/frappe/model/db_query.py +++ b/frappe/model/db_query.py @@ -164,7 +164,8 @@ class DatabaseQuery(object): # left join parent, child tables for child in self.tables[1:]: - args.tables += f" {self.join} {child} on ({child}.parent = {self.tables[0]}.name)" + parent_name = self.cast_autoincremented_name(f"{self.tables[0]}.name") + args.tables += f" {self.join} {child} on ({child}.parent = {parent_name})" if self.grouped_or_conditions: self.conditions.append(f"({' or '.join(self.grouped_or_conditions)})") @@ -318,21 +319,63 @@ class DatabaseQuery(object): ] # add tables from fields if self.fields: - for field in self.fields: - if not ("tab" in field and "." in field) or any(x for x in sql_functions if x in field): + for i, field in enumerate(self.fields): + if not ("tab" in field and "." in field): + continue + + # add cast in locate/strpos + func_found = False + for func in sql_functions: + if func in field.lower(): + self.fields[i] = self.cast_autoincremented_name(field, func) + func_found = True + break + + if func_found: continue table_name = field.split('.')[0] if table_name.lower().startswith('group_concat('): table_name = table_name[13:] - if table_name.lower().startswith('ifnull('): - table_name = table_name[7:] if not table_name[0]=='`': table_name = f"`{table_name}`" if table_name not in self.tables: self.append_table(table_name) + def cast_autoincremented_name(self, column: str, sql_function: str = "",) -> str: + if frappe.db.db_type == "postgres": + if "name" in column.lower(): + if "cast(" not in column.lower() or "::" not in column: + if not sql_function: + return f"cast({column} as varchar)" + + elif sql_function == "locate(": + return re.sub( + r'locate\(([^,]+),([^)]+)\)', + r'locate(\1, cast(\2 as varchar))', + column, + flags=re.IGNORECASE + ) + + elif sql_function == "strpos(": + return re.sub( + r'strpos\(([^,]+),([^)]+)\)', + r'strpos(cast(\1 as varchar), \2)', + column, + flags=re.IGNORECASE + ) + + elif sql_function == "ifnull(": + return re.sub( + r"ifnull\(([^,]+)", + r"ifnull(cast(\1 as varchar)", + column, + flags=re.IGNORECASE + ) + + return column + def append_table(self, table_name): self.tables.append(table_name) doctype = table_name[4:-1] @@ -423,6 +466,8 @@ class DatabaseQuery(object): ifnull(`tabDocType`.`fieldname`, fallback) operator "value" """ + # TODO: refactor + from frappe.boot import get_additional_filters_from_hooks additional_filters_config = get_additional_filters_from_hooks() f = get_filter(self.doctype, f, additional_filters_config) @@ -432,15 +477,16 @@ class DatabaseQuery(object): self.append_table(tname) if 'ifnull(' in f.fieldname: - column_name = f.fieldname + column_name = self.cast_autoincremented_name(f.fieldname, "ifnull(") else: - column_name = f"{tname}.{f.fieldname}" - - can_be_null = True + column_name = self.cast_autoincremented_name(f"{tname}.{f.fieldname}") if f.operator.lower() in additional_filters_config: f.update(get_additional_filter_field(additional_filters_config, f, f.value)) + meta = frappe.get_meta(f.doctype) + can_be_null = True + # prepare in condition if f.operator.lower() in ('ancestors of', 'descendants of', 'not ancestors of', 'not descendants of'): values = f.value or '' @@ -449,12 +495,8 @@ class DatabaseQuery(object): # if not isinstance(values, (list, tuple)): # values = values.split(",") - ref_doctype = f.doctype - - if frappe.get_meta(f.doctype).get_field(f.fieldname) is not None : - ref_doctype = frappe.get_meta(f.doctype).get_field(f.fieldname).options - - result=[] + field = meta.get_field(f.fieldname) + ref_doctype = field.options if field else f.doctype lft, rgt = '', '' if f.value: @@ -474,29 +516,30 @@ class DatabaseQuery(object): }, order_by='`lft` DESC') fallback = "''" - value = [frappe.db.escape((v.name or '').strip(), percent=False) for v in result] + value = [frappe.db.escape((cstr(v.name) or '').strip(), percent=False) for v in result] if len(value): value = f"({', '.join(value)})" else: value = "('')" + # changing operator to IN as the above code fetches all the parent / child values and convert into tuple # which can be directly used with IN operator to query. f.operator = 'not in' if f.operator.lower() in ('not ancestors of', 'not descendants of') else 'in' - elif f.operator.lower() in ('in', 'not in'): values = f.value or '' if isinstance(values, str): values = values.split(",") fallback = "''" - value = [frappe.db.escape((v or '').strip(), percent=False) for v in values] + value = [frappe.db.escape((cstr(v) or '').strip(), percent=False) for v in values] if len(value): value = f"({', '.join(value)})" else: value = "('')" + else: - df = frappe.get_meta(f.doctype).get("fields", {"fieldname": f.fieldname}) + df = meta.get("fields", {"fieldname": f.fieldname}) df = df[0] if df else None if df and df.fieldtype in ("Check", "Float", "Int", "Currency", "Percent"): @@ -513,7 +556,8 @@ class DatabaseQuery(object): fallback = "'0001-01-01 00:00:00'" elif f.operator.lower() in ('between') and \ - (f.fieldname in ('creation', 'modified') or (df and (df.fieldtype=="Date" or df.fieldtype=="Datetime"))): + (f.fieldname in ('creation', 'modified') or + (df and (df.fieldtype=="Date" or df.fieldtype=="Datetime"))): value = get_between_date_filter(f.value, df) fallback = "'0001-01-01 00:00:00'" @@ -528,7 +572,7 @@ class DatabaseQuery(object): fallback = "''" can_be_null = True - if 'ifnull' not in column_name: + if 'ifnull' not in column_name.lower(): column_name = f'ifnull({column_name}, {fallback})' elif df and df.fieldtype=="Date": @@ -570,7 +614,7 @@ class DatabaseQuery(object): value = f"{tname}.{quote}{f.value.name}{quote}" # escape value - elif isinstance(value, str) and not f.operator.lower() == 'between': + elif isinstance(value, str) and f.operator.lower() != 'between': value = f"{frappe.db.escape(value, percent=False)}" if ( diff --git a/frappe/model/delete_doc.py b/frappe/model/delete_doc.py index ef73a349cc..f055cd79d0 100644 --- a/frappe/model/delete_doc.py +++ b/frappe/model/delete_doc.py @@ -158,7 +158,7 @@ def update_naming_series(doc): and getattr(doc, "naming_series", None): revert_series_if_last(doc.naming_series, doc.name, doc) - elif doc.meta.autoname.split(":")[0] not in ("Prompt", "field", "hash"): + elif doc.meta.autoname.split(":")[0] not in ("Prompt", "field", "hash", "autoincrement"): revert_series_if_last(doc.meta.autoname, doc.name, doc) def delete_from_table(doctype, name, ignore_doctypes, doc): diff --git a/frappe/model/naming.py b/frappe/model/naming.py index 9024b3d7b4..9ba7f11563 100644 --- a/frappe/model/naming.py +++ b/frappe/model/naming.py @@ -1,14 +1,18 @@ # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors # License: MIT. See LICENSE -from typing import Optional +from typing import Optional, TYPE_CHECKING import frappe from frappe import _ +from frappe.database.sequence import get_next_val from frappe.utils import now_datetime, cint, cstr import re from frappe.model import log_types from frappe.query_builder import DocType +if TYPE_CHECKING: + from frappe.model.meta import Meta + def set_new_name(doc): """ @@ -24,7 +28,8 @@ def set_new_name(doc): doc.run_method("before_naming") - autoname = frappe.get_meta(doc.doctype).autoname or "" + meta = frappe.get_meta(doc.doctype) + autoname = meta.autoname or "" if autoname.lower() != "prompt" and not frappe.flags.in_import: doc.name = None @@ -36,6 +41,10 @@ def set_new_name(doc): elif getattr(doc.meta, "issingle", False): doc.name = doc.doctype + elif is_autoincremented(doc.doctype, meta): + doc.name = get_next_val(doc.doctype) + return + elif getattr(doc.meta, "istable", False): doc.name = make_autoname("hash", doc.doctype) @@ -67,6 +76,28 @@ def set_new_name(doc): frappe.get_meta(doc.doctype).get_field("name_case") ) +def is_autoincremented(doctype: str, meta: "Meta" = None): + if doctype in frappe.local.autoincremented_doctypes: + return True + + elif doctype in log_types: + if frappe.db.sql( + f"""select data_type FROM information_schema.columns + where column_name = 'name' and table_name = 'tab{doctype}'""" + )[0][0] == "bigint": + frappe.local.autoincremented_doctypes.add(doctype) + return True + + else: + if not meta: + meta = frappe.get_meta(doctype) + + if meta.autoname == "autoincrement": + frappe.local.autoincremented_doctypes.add(doctype) + return True + + return False + def set_name_from_naming_options(autoname, doc): """ Get a name based on the autoname field option diff --git a/frappe/public/js/frappe/form/form.js b/frappe/public/js/frappe/form/form.js index 56e909dd0c..6eed16bd28 100644 --- a/frappe/public/js/frappe/form/form.js +++ b/frappe/public/js/frappe/form/form.js @@ -248,7 +248,7 @@ frappe.ui.form.Form = class FrappeForm { // on main doc frappe.model.on(me.doctype, "*", function(fieldname, value, doc) { // set input - if(doc.name===me.docname) { + if (cstr(doc.name) === me.docname) { me.dirty(); let field = me.fields_dict[fieldname]; @@ -1215,7 +1215,7 @@ frappe.ui.form.Form = class FrappeForm { } is_dirty() { - return !!this.doc.__unsaved; + return this.doc.__unsaved; } is_new() { diff --git a/frappe/public/js/frappe/list/list_view.js b/frappe/public/js/frappe/list/list_view.js index 98553a4a3e..d7502d4e96 100644 --- a/frappe/public/js/frappe/list/list_view.js +++ b/frappe/public/js/frappe/list/list_view.js @@ -915,7 +915,7 @@ frappe.views.ListView = class ListView extends frappe.views.BaseList { return this.settings.get_form_link(doc); } - const docname = doc.name.match(/[%'"#\s]/) + const docname = cstr(doc.name).match(/[%'"#\s]/) ? encodeURIComponent(doc.name) : doc.name; diff --git a/frappe/realtime.py b/frappe/realtime.py index e0f64d32fb..940a3220a4 100644 --- a/frappe/realtime.py +++ b/frappe/realtime.py @@ -2,6 +2,7 @@ # License: MIT. See LICENSE import frappe +from frappe.utils.data import cstr import os import redis @@ -118,7 +119,7 @@ def get_user_info(): } def get_doc_room(doctype, docname): - return ''.join([frappe.local.site, ':doc:', doctype, '/', docname]) + return ''.join([frappe.local.site, ':doc:', doctype, '/', cstr(docname)]) def get_user_room(user): return ''.join([frappe.local.site, ':user:', user]) diff --git a/frappe/tests/test_db.py b/frappe/tests/test_db.py index bbd09590be..ab85f28af3 100644 --- a/frappe/tests/test_db.py +++ b/frappe/tests/test_db.py @@ -562,3 +562,50 @@ class TestDDLCommandsPost(unittest.TestCase): """, ) self.assertEquals(len(indexs_in_table), 1) + + @run_only_if(db_type_is.POSTGRES) + def test_modify_query(self): + from frappe.database.postgres.database import modify_query + + query = "select * from `tabtree b` where lft > 13 and rgt <= 16 and name =1.0 and parent = 4134qrsdc and isgroup = 1.00045" + self.assertEqual( + "select * from \"tabtree b\" where lft > \'13\' and rgt <= '16' and name = '1' and parent = 4134qrsdc and isgroup = 1.00045", + modify_query(query) + ) + + query = "select locate(\".io\", \"frappe.io\"), locate(\"3\", cast(3 as varchar)), locate(\"3\", 3::varchar)" + self.assertEqual( + "select strpos( \"frappe.io\", \".io\"), strpos( cast(3 as varchar), \"3\"), strpos( 3::varchar, \"3\")", + modify_query(query) + ) + + @run_only_if(db_type_is.POSTGRES) + def test_modify_values(self): + from frappe.database.postgres.database import modify_values + + self.assertEqual( + {"abcd": "23", "efgh": "23", "ijkl": 23.0345, "mnop": "wow"}, + modify_values({"abcd": 23, "efgh": 23.0, "ijkl": 23.0345, "mnop": "wow"}) + ) + self.assertEqual( + ["23", "23", 23.00004345, "wow"], + modify_values((23, 23.0, 23.00004345, "wow")) + ) + + def test_sequence_table_creation(self): + from frappe.core.doctype.doctype.test_doctype import new_doctype + + dt = new_doctype("autoinc_dt_seq_test", autoincremented=True).insert(ignore_permissions=True) + + if frappe.db.db_type == "postgres": + self.assertTrue( + frappe.db.sql("""select sequence_name FROM information_schema.sequences + where sequence_name ilike 'autoinc_dt_seq_test%'""")[0][0] + ) + else: + self.assertTrue( + frappe.db.sql("""select data_type FROM information_schema.tables + where table_type = 'SEQUENCE' and table_name like 'autoinc_dt_seq_test%'""")[0][0] + ) + + dt.delete(ignore_permissions=True) diff --git a/frappe/tests/test_db_query.py b/frappe/tests/test_db_query.py index a53134064e..2d5edd4025 100644 --- a/frappe/tests/test_db_query.py +++ b/frappe/tests/test_db_query.py @@ -494,6 +494,27 @@ class TestReportview(unittest.TestCase): response = execute_cmd("frappe.desk.reportview.get") self.assertListEqual(response["keys"], ["field_label", "field_name", "_aggregate_column", 'columns']) + def test_cast_autoincremented_name(self): + from frappe.core.doctype.doctype.test_doctype import new_doctype + + dt = new_doctype("autoinc_dt_test", autoincremented=True).insert(ignore_permissions=True) + + query = DatabaseQuery("autoinc_dt_test").execute( + fields=["locate('1', `tabautoinc_dt_test`.`name`)", "`tabautoinc_dt_test`.`name`"], + filters={"name": 1}, + run=False + ) + + if frappe.db.db_type == "postgres": + self.assertTrue("strpos( cast( \"tabautoinc_dt_test\".\"name\" as varchar), \'1\')" in query) + self.assertTrue("where cast(\"tabautoinc_dt_test\".name as varchar) = \'1\'" in query) + else: + self.assertTrue("locate(\'1\', `tabautoinc_dt_test`.`name`)" in query) + self.assertTrue("where `tabautoinc_dt_test`.name = 1" in query) + + dt.delete(ignore_permissions=True) + + def add_child_table_to_blog_post(): child_table = frappe.get_doc({ 'doctype': 'DocType', diff --git a/frappe/tests/test_naming.py b/frappe/tests/test_naming.py index 3e1120dc79..0c5387ccf2 100644 --- a/frappe/tests/test_naming.py +++ b/frappe/tests/test_naming.py @@ -245,6 +245,17 @@ class TestNaming(unittest.TestCase): }) self.assertRaises(frappe.ValidationError, tag.insert) + def test_autoincremented_naming(self): + from frappe.core.doctype.doctype.test_doctype import new_doctype + + doctype = "autoinc_doctype" + frappe.generate_hash(length=5) + dt = new_doctype(doctype, autoincremented=True).insert(ignore_permissions=True) + + for i in range(1, 20): + self.assertEqual(frappe.new_doc(doctype).save(ignore_permissions=True).name, i) + + dt.delete(ignore_permissions=True) + def make_invalid_todo(): frappe.get_doc({ diff --git a/frappe/utils/data.py b/frappe/utils/data.py index 50c71bdc2e..212ae8eba6 100644 --- a/frappe/utils/data.py +++ b/frappe/utils/data.py @@ -1494,7 +1494,7 @@ def expand_relative_urls(html): return html def quoted(url): - return cstr(quote(encode(url), safe=b"~@#$&()*!+=:;,.?/'")) + return cstr(quote(encode(cstr(url)), safe=b"~@#$&()*!+=:;,.?/'")) def quote_urls(html): def _quote_url(match): diff --git a/frappe/utils/diff.py b/frappe/utils/diff.py index ac0e1b7439..2574f47fbd 100644 --- a/frappe/utils/diff.py +++ b/frappe/utils/diff.py @@ -1,14 +1,15 @@ import json from difflib import unified_diff -from typing import List +from typing import List, Union import frappe from frappe.utils import pretty_date +from frappe.utils.data import cstr @frappe.whitelist() def get_version_diff( - from_version: str, to_version: str, fieldname: str = "script" + from_version: Union[int, str], to_version: Union[int, str], fieldname: str = "script" ) -> List[str]: before, before_timestamp = _get_value_from_version(from_version, fieldname) @@ -23,15 +24,15 @@ def get_version_diff( diff = unified_diff( before, after, - fromfile=from_version, - tofile=to_version, + fromfile=cstr(from_version), + tofile=cstr(to_version), fromfiledate=before_timestamp, tofiledate=after_timestamp, ) return list(diff) -def _get_value_from_version(version_name: str, fieldname: str): +def _get_value_from_version(version_name: Union[int, str], fieldname: str): version = frappe.get_list( "Version", fields=["data", "modified"], filters={"name": version_name} ) diff --git a/frappe/utils/global_search.py b/frappe/utils/global_search.py index 7b591dff45..22938671a6 100644 --- a/frappe/utils/global_search.py +++ b/frappe/utils/global_search.py @@ -9,6 +9,8 @@ import os from frappe.utils import cint, strip_html_tags from frappe.utils.html_utils import unescape_html from frappe.model.base_document import get_controller +from frappe.utils.data import cstr + def setup_global_search_table(): """ @@ -251,7 +253,7 @@ def update_global_search(doc): if hasattr(doc, 'is_website_published') and doc.meta.allow_guest_to_view: published = 1 if doc.is_website_published() else 0 - title = (doc.get_title() or '')[:int(frappe.db.VARCHAR_LEN)] + title = (cstr(doc.get_title()) or '')[:int(frappe.db.VARCHAR_LEN)] route = doc.get('route') if doc else '' value = dict(