From 6c79a13641ea2e3ebeb0d235daac090d8bb42711 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 29 Mar 2024 16:06:20 +0530 Subject: [PATCH 1/4] feat: UUID naming support --- frappe/core/doctype/doctype/doctype.json | 4 ++-- frappe/core/doctype/doctype/doctype.py | 1 + frappe/database/mariadb/schema.py | 3 +++ frappe/model/naming.py | 12 +++++++----- frappe/public/js/frappe/doctype/index.js | 1 + pyproject.toml | 1 + 6 files changed, 15 insertions(+), 7 deletions(-) diff --git a/frappe/core/doctype/doctype/doctype.json b/frappe/core/doctype/doctype/doctype.json index 0f97b077ab..c285c6ffec 100644 --- a/frappe/core/doctype/doctype/doctype.json +++ b/frappe/core/doctype/doctype/doctype.json @@ -570,7 +570,7 @@ "fieldtype": "Select", "label": "Naming Rule", "length": 40, - "options": "\nSet by user\nAutoincrement\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\nUUID\nBy script" }, { "fieldname": "migration_hash", @@ -750,7 +750,7 @@ "link_fieldname": "reference_doctype" } ], - "modified": "2024-03-23 16:03:21.405959", + "modified": "2024-03-29 16:09:26.114720", "modified_by": "Administrator", "module": "Core", "name": "DocType", diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index 7237992fdf..7c29792778 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -147,6 +147,7 @@ class DocType(Document): "Expression", "Expression (old style)", "Random", + "UUID", "By script", ] nsm_parent_field: DF.Data | None diff --git a/frappe/database/mariadb/schema.py b/frappe/database/mariadb/schema.py index 73a92dfa4a..1c047474e7 100644 --- a/frappe/database/mariadb/schema.py +++ b/frappe/database/mariadb/schema.py @@ -47,6 +47,9 @@ class MariaDBTable(DBTable): # issue link: https://jira.mariadb.org/browse/MDEV-20070 name_column = "name bigint primary key" + elif not self.meta.issingle and self.meta.autoname == "UUID": + name_column = "name uuid primary key" + additional_definitions = ",\n".join(additional_definitions) # create table diff --git a/frappe/model/naming.py b/frappe/model/naming.py index 7dabb3d80d..cd7066672a 100644 --- a/frappe/model/naming.py +++ b/frappe/model/naming.py @@ -4,11 +4,12 @@ import base64 import datetime import re -import struct import time from collections.abc import Callable from typing import TYPE_CHECKING, Optional +from uuid_utils import uuid7 + import frappe from frappe import _ from frappe.model import log_types @@ -149,6 +150,10 @@ def set_new_name(doc): doc.name = frappe.db.get_next_sequence_val(doc.doctype) return + if meta.autoname == "UUID": + doc.name = str(uuid7()) + return + if getattr(doc, "amended_from", None): _set_amended_name(doc) if doc.name: @@ -179,10 +184,7 @@ def is_autoincremented(doctype: str, meta: Optional["Meta"] = None) -> bool: if not meta: meta = frappe.get_meta(doctype) - if not getattr(meta, "issingle", False) and meta.autoname == "autoincrement": - return True - - return False + return not getattr(meta, "issingle", False) and meta.autoname == "autoincrement" def set_name_from_naming_options(autoname, doc): diff --git a/frappe/public/js/frappe/doctype/index.js b/frappe/public/js/frappe/doctype/index.js index 5b072d6bfa..4d8e437c1b 100644 --- a/frappe/public/js/frappe/doctype/index.js +++ b/frappe/public/js/frappe/doctype/index.js @@ -78,6 +78,7 @@ frappe.model.DocTypeController = class DocTypeController extends frappe.ui.form. Expression: "format:", "Expression (sld style)": "", Random: "hash", + UUID: "UUID", "By script": "", }; this.frm.set_value( diff --git a/pyproject.toml b/pyproject.toml index 14289d1a72..734ffef6dc 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -73,6 +73,7 @@ dependencies = [ "terminaltables~=3.1.10", "traceback-with-variables~=2.0.4", "typing_extensions>=4.6.1,<5", + "uuid-utils~=0.6.1", "xlrd~=2.0.1", "zxcvbn~=4.4.28", "markdownify~=0.11.6", From 1ec4d658fcf0f54f9a0b1699b58324996ab67258 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 29 Mar 2024 17:37:37 +0530 Subject: [PATCH 2/4] fix: Allow setting UUID to application code --- frappe/model/naming.py | 19 ++++++++++++++++--- frappe/tests/test_document.py | 2 +- frappe/tests/test_naming.py | 24 ++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 4 deletions(-) diff --git a/frappe/model/naming.py b/frappe/model/naming.py index cd7066672a..ddbea52c1d 100644 --- a/frappe/model/naming.py +++ b/frappe/model/naming.py @@ -7,8 +7,9 @@ import re import time from collections.abc import Callable from typing import TYPE_CHECKING, Optional +from uuid import UUID -from uuid_utils import uuid7 +import uuid_utils import frappe from frappe import _ @@ -40,6 +41,10 @@ class InvalidNamingSeriesError(frappe.ValidationError): pass +class InvalidUUIDValue(frappe.ValidationError): + pass + + class NamingSeries: __slots__ = ("series",) @@ -143,7 +148,7 @@ def set_new_name(doc): meta = frappe.get_meta(doc.doctype) autoname = meta.autoname or "" - if autoname.lower() != "prompt" and not frappe.flags.in_import: + if autoname.lower() not in ("prompt", "uuid") and not frappe.flags.in_import: doc.name = None if is_autoincremented(doc.doctype, meta): @@ -151,7 +156,15 @@ def set_new_name(doc): return if meta.autoname == "UUID": - doc.name = str(uuid7()) + if not doc.name: + doc.name = str(uuid_utils.uuid7()) + elif isinstance(doc.name, UUID | uuid_utils.UUID): + doc.name = str(doc.name) + elif isinstance(doc.name, str): # validate + try: + UUID(doc.name) + except ValueError: + frappe.throw(_("Invalid value specified for UUID: {}").format(doc.name), InvalidUUIDValue) return if getattr(doc, "amended_from", None): diff --git a/frappe/tests/test_document.py b/frappe/tests/test_document.py index a707312103..dd76970903 100644 --- a/frappe/tests/test_document.py +++ b/frappe/tests/test_document.py @@ -9,7 +9,7 @@ from frappe.app import make_form_dict from frappe.core.doctype.doctype.test_doctype import new_doctype from frappe.desk.doctype.note.note import Note from frappe.model.naming import make_autoname, parse_naming_series, revert_series_if_last -from frappe.tests.utils import FrappeTestCase, timeout +from frappe.tests.utils import FrappeTestCase from frappe.utils import cint, now_datetime, set_request from frappe.website.serve import get_response diff --git a/frappe/tests/test_naming.py b/frappe/tests/test_naming.py index aa2c091fac..b2e3c3a18a 100644 --- a/frappe/tests/test_naming.py +++ b/frappe/tests/test_naming.py @@ -2,13 +2,16 @@ # License: MIT. See LICENSE import time +from uuid import UUID +import uuid_utils from tenacity import retry, retry_if_exception_type, stop_after_attempt, wait_full_jitter import frappe from frappe.core.doctype.doctype.test_doctype import new_doctype from frappe.model.naming import ( InvalidNamingSeriesError, + InvalidUUIDValue, NamingSeries, append_number_if_name_exists, determine_consecutive_week_number, @@ -407,6 +410,27 @@ class TestNaming(FrappeTestCase): names.append(make_autoname("hash")) self.assertEqual(names, sorted(names)) + def test_uuid_naming(self): + uuid_doctype = new_doctype(autoname="UUID").insert().name + self.assertEqual("uuid", frappe.db.get_column_type(uuid_doctype, "name")) + + # Auto set names + document = frappe.new_doc(uuid_doctype).insert() + uid = UUID(document.name) + self.assertEqual(uid.version, 7) # Default version + + # Applications can specify UUID themselves, useful for APIs to set name themselves. + for uid in (uuid_utils.uuid4(), uuid_utils.uuid7()): + doc = frappe.new_doc(uuid_doctype, name=uid).insert() + self.assertEqual(doc.name, str(uid)) + + # Can specify valid UUID strings too + for uid in (uuid_utils.uuid4(), uuid_utils.uuid7()): + doc = frappe.new_doc(uuid_doctype, name=str(uid)).insert() + self.assertEqual(doc.name, str(uid)) + + self.assertRaises(InvalidUUIDValue, frappe.new_doc(uuid_doctype, name="XYZ").insert) + def parse_naming_series_variable(doc, variable): if variable == "PM": From 8cc6c31f0e63308bc6344ed90b53fd58af57fb6a Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 29 Mar 2024 17:56:07 +0530 Subject: [PATCH 3/4] fix: Allow switching between UUID and VARCHAR - VARCHAR -> UUID = Only allowed when table is empty for now - UUID -> VARCHAR is not lossy, can be done anytime. --- frappe/database/mariadb/schema.py | 20 ++++++++++++++++++++ frappe/tests/test_db_update.py | 13 +++++++++++++ 2 files changed, 33 insertions(+) diff --git a/frappe/database/mariadb/schema.py b/frappe/database/mariadb/schema.py index 1c047474e7..d1ec513026 100644 --- a/frappe/database/mariadb/schema.py +++ b/frappe/database/mariadb/schema.py @@ -79,6 +79,9 @@ class MariaDBTable(DBTable): f"MODIFY `{col.fieldname}` {col.get_definition(for_modification=True)}" for col in columns_to_modify ] + if alter_pk := self.alter_primary_key(): + modify_column_query.append(alter_pk) + modify_column_query.extend( [f"ADD UNIQUE INDEX IF NOT EXISTS {col.fieldname} (`{col.fieldname}`)" for col in self.add_unique] ) @@ -141,3 +144,20 @@ class MariaDBTable(DBTable): ) raise + + def alter_primary_key(self) -> str | None: + # If there are no values in table allow migrating to UUID from varchar + autoname = self.meta.autoname + if autoname == "UUID" and frappe.db.get_column_type(self.doctype, "name") != "uuid": + if not frappe.db.get_value(self.doctype, {}, order_by=None): + return "modify name uuid" + else: + frappe.throw( + _("Primary key of doctype {0} can not be changed as there are existing values.").format( + self.doctype + ) + ) + + # Reverting from UUID to VARCHAR + if autoname != "UUID" and frappe.db.get_column_type(self.doctype, "name") == "uuid": + return f"modify name varchar({frappe.db.VARCHAR_LEN})" diff --git a/frappe/tests/test_db_update.py b/frappe/tests/test_db_update.py index 780be92fda..27c05ce285 100644 --- a/frappe/tests/test_db_update.py +++ b/frappe/tests/test_db_update.py @@ -139,6 +139,19 @@ class TestDBUpdate(FrappeTestCase): doctype.delete() frappe.db.commit() + def test_uuid_varchar_migration(self): + doctype = new_doctype().insert() + doctype.autoname = "UUID" + doctype.save() + self.assertEqual(frappe.db.get_column_type(doctype.name, "name"), "uuid") + + doc = frappe.new_doc(doctype.name).insert() + + doctype.autoname = "hash" + doctype.save() + self.assertEqual(frappe.db.get_column_type(doctype.name, "name"), "varchar(140)") + doc.reload() # ensure that docs are still accesible + def get_fieldtype_from_def(field_def): fieldtuple = frappe.db.type_map.get(field_def.fieldtype, ("", 0)) From 8c6e3a56c69353f242c870071bec7b39558e7931 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 29 Mar 2024 20:56:34 +0530 Subject: [PATCH 4/4] feat: UUID name implementation for postgres --- frappe/database/postgres/schema.py | 23 +++++++++++++++++++++++ frappe/tests/test_db_update.py | 3 ++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/frappe/database/postgres/schema.py b/frappe/database/postgres/schema.py index 018de93f41..926f9e0edd 100644 --- a/frappe/database/postgres/schema.py +++ b/frappe/database/postgres/schema.py @@ -34,6 +34,9 @@ class PostgresTable(DBTable): frappe.db.create_sequence(self.doctype, check_not_exists=True) name_column = "name bigint primary key" + elif not self.meta.issingle and self.meta.autoname == "UUID": + name_column = "name uuid primary key" + # TODO: set docstatus length # create table frappe.db.sql( @@ -91,6 +94,9 @@ class PostgresTable(DBTable): ) ) + if alter_pk := self.alter_primary_key(): + query.append(alter_pk) + for col in self.set_default: if col.fieldname == "name": continue @@ -181,3 +187,20 @@ class PostgresTable(DBTable): ) else: raise e + + def alter_primary_key(self) -> str | None: + # If there are no values in table allow migrating to UUID from varchar + autoname = self.meta.autoname + if autoname == "UUID" and frappe.db.get_column_type(self.doctype, "name") != "uuid": + if not frappe.db.get_value(self.doctype, {}, order_by=None): + return "alter column `name` TYPE uuid USING name::uuid" + else: + frappe.throw( + _("Primary key of doctype {0} can not be changed as there are existing values.").format( + self.doctype + ) + ) + + # Reverting from UUID to VARCHAR + if autoname != "UUID" and frappe.db.get_column_type(self.doctype, "name") == "uuid": + return f"alter column `name` TYPE varchar({frappe.db.VARCHAR_LEN})" diff --git a/frappe/tests/test_db_update.py b/frappe/tests/test_db_update.py index 27c05ce285..68a22e0de2 100644 --- a/frappe/tests/test_db_update.py +++ b/frappe/tests/test_db_update.py @@ -149,7 +149,8 @@ class TestDBUpdate(FrappeTestCase): doctype.autoname = "hash" doctype.save() - self.assertEqual(frappe.db.get_column_type(doctype.name, "name"), "varchar(140)") + varchar = "varchar" if frappe.db.db_type == "mariadb" else "character varying" + self.assertIn(varchar, frappe.db.get_column_type(doctype.name, "name")) doc.reload() # ensure that docs are still accesible