perf: faster generate_hash (backport #18825) (#18851)

* perf: faster generate_hash (cherry picked from commit fcaa16b) * refactor: remove `txt` param from generate_hash use (cherry picked from commit f34f703) # Conflicts: # frappe/tests/test_modules.py * chore: conflicts Co-authored-by: Ankush Menat <ankush@frappe.io> Co-authored-by: Ankush Menat <ankushmenat@gmail.com>
frappe · Nov 12, 2022 · bb5c7d5 · bb5c7d5
1 parent 9eaec5c
commit bb5c7d5
Show file tree

Hide file tree

Showing 14 changed files with 26 additions and 32 deletions.
diff --git a/frappe/__init__.py b/frappe/__init__.py
@@ -1019,19 +1019,15 @@ def get_precision(
 	return get_field_precision(get_meta(doctype).get_field(fieldname), doc, currency)
 
 
-def generate_hash(txt: str | None = None, length: int | None = None) -> str:
-	"""Generates random hash for given text + current timestamp + random string."""
-	import hashlib
-	import time
-
-	from .utils import random_string
-
-	digest = hashlib.sha224(
-		((txt or "") + repr(time.time()) + repr(random_string(8))).encode()
-	).hexdigest()
-	if length:
-		digest = digest[:length]
-	return digest
+def generate_hash(txt: str | None = None, length: int = 56) -> str:
+	"""Generate random hash using best available randomness source."""
+	import math
+	import secrets
+
+	if not length:
+		length = 56
+
+	return secrets.token_hex(math.ceil(length / 2))[:length]
 
 
 def reset_metadata_version():

diff --git a/frappe/core/doctype/data_export/exporter.py b/frappe/core/doctype/data_export/exporter.py
@@ -410,7 +410,7 @@ def add_data_row(self, rows, dt, parentfield, doc, rowidx):
 				row[_column_start_end.start + i + 1] = value
 
 	def build_response_as_excel(self):
-		filename = frappe.generate_hash("", 10)
+		filename = frappe.generate_hash(length=10)
 		with open(filename, "wb") as f:
 			f.write(cstr(self.writer.getvalue()).encode("utf-8"))
 		f = open(filename)

diff --git a/frappe/core/doctype/data_import/test_importer.py b/frappe/core/doctype/data_import/test_importer.py
@@ -97,7 +97,7 @@ def test_data_import_without_mandatory_values(self):
 	def test_data_import_update(self):
 		existing_doc = frappe.get_doc(
 			doctype=doctype_name,
-			title=frappe.generate_hash(doctype_name, 8),
+			title=frappe.generate_hash(length=8),
 			table_field_1=[{"child_title": "child title to update"}],
 		)
 		existing_doc.save()

diff --git a/frappe/model/naming.py b/frappe/model/naming.py
@@ -277,7 +277,7 @@ def make_autoname(key="", doctype="", doc=""):
 	                DE/09/01/0001 where 09 is the year, 01 is the month and 0001 is the series
 	"""
 	if key == "hash":
-		return frappe.generate_hash(doctype, 10)
+		return frappe.generate_hash(length=10)
 
 	series = NamingSeries(key)
 	return series.generate_next_name(doc)

diff --git a/frappe/patches/v11_0/remove_skip_for_doctype.py b/frappe/patches/v11_0/remove_skip_for_doctype.py
@@ -60,7 +60,7 @@ def execute():
 					# Maintain sequence (name, user, allow, for_value, applicable_for, apply_to_all_doctypes, creation, modified)
 					new_user_permissions_list.append(
 						(
-							frappe.generate_hash("", 10),
+							frappe.generate_hash(length=10),
 							user_permission.user,
 							user_permission.allow,
 							user_permission.for_value,

diff --git a/frappe/patches/v12_0/move_email_and_phone_to_child_table.py b/frappe/patches/v12_0/move_email_and_phone_to_child_table.py
@@ -27,7 +27,7 @@ def execute():
 			email_values.append(
 				(
 					1,
-					frappe.generate_hash(contact_detail.email_id, 10),
+					frappe.generate_hash(length=10),
 					contact_detail.email_id,
 					"email_ids",
 					"Contact",
@@ -44,7 +44,7 @@ def execute():
 			phone_values.append(
 				(
 					phone_counter,
-					frappe.generate_hash(contact_detail.email_id, 10),
+					frappe.generate_hash(length=10),
 					contact_detail.phone,
 					"phone_nos",
 					"Contact",
@@ -63,7 +63,7 @@ def execute():
 			phone_values.append(
 				(
 					phone_counter,
-					frappe.generate_hash(contact_detail.email_id, 10),
+					frappe.generate_hash(length=10),
 					contact_detail.mobile_no,
 					"phone_nos",
 					"Contact",

diff --git a/frappe/patches/v12_0/setup_tags.py b/frappe/patches/v12_0/setup_tags.py
@@ -28,7 +28,7 @@ def execute():
 
 				tag_list.append((tag.strip(), time, time, "Administrator"))
 
-				tag_link_name = frappe.generate_hash(_user_tags.name + tag.strip() + doctype.name, 10)
+				tag_link_name = frappe.generate_hash(length=10)
 				tag_links.append(
 					(tag_link_name, doctype.name, _user_tags.name, tag.strip(), time, time, "Administrator")
 				)

diff --git a/frappe/templates/includes/navbar/navbar_items.html b/frappe/templates/includes/navbar/navbar_items.html
@@ -3,7 +3,7 @@
 
 {% if parent %}
 
-{%- set dropdown_id = 'id-' + frappe.utils.generate_hash('Dropdown', 12) -%}
+{%- set dropdown_id = 'id-' + frappe.utils.generate_hash(length=12) -%}
 <li class="nav-item dropdown {% if submenu %} dropdown-submenu {% endif %}">
 	<a class="nav-link dropdown-toggle" href="#" id="{{ dropdown_id }}" role="button"
 		data-toggle="dropdown" aria-haspopup="true" aria-expanded="false">
@@ -16,7 +16,7 @@
 	</ul>
 </li>
 {% else %}
-{%- set dropdown_id = 'id-' + frappe.utils.generate_hash('Dropdown', 12) -%}
+{%- set dropdown_id = 'id-' + frappe.utils.generate_hash(length=12) -%}
 <li class="dropdown {% if submenu %} dropdown-submenu {% endif %}">
 	<a class="dropdown-item dropdown-toggle" href="#" id="{{ dropdown_id }}" role="button"
 		data-toggle="dropdown" aria-haspopup="true" aria-expanded="false">

diff --git a/frappe/utils/jinja_globals.py b/frappe/utils/jinja_globals.py
@@ -92,9 +92,7 @@ def web_blocks(blocks):
 def get_dom_id(seed=None):
 	from frappe import generate_hash
 
-	if not seed:
-		seed = "DOM"
-	return "id-" + generate_hash(seed, 12)
+	return "id-" + generate_hash(12)
 
 
 def include_script(path, preload=True):

diff --git a/frappe/utils/oauth.py b/frappe/utils/oauth.py
@@ -266,7 +266,7 @@ def update_oauth_user(user, data, provider):
 				"email": get_email(data),
 				"gender": gender,
 				"enabled": 1,
-				"new_password": frappe.generate_hash(get_email(data)),
+				"new_password": frappe.generate_hash(),
 				"location": data.get("location"),
 				"user_type": "Website User",
 				"user_image": data.get("picture") or data.get("avatar_url"),

diff --git a/frappe/website/doctype/website_theme/website_theme.py b/frappe/website/doctype/website_theme/website_theme.py
@@ -75,7 +75,7 @@ def generate_bootstrap_theme(self):
 			self.delete_old_theme_files(folder_path)
 
 		# add a random suffix
-		suffix = frappe.generate_hash("Website Theme", 8) if self.custom else "style"
+		suffix = frappe.generate_hash(length=8) if self.custom else "style"
 		file_name = frappe.scrub(self.name) + "_" + suffix + ".css"
 		output_path = join_path(folder_path, file_name)
 

diff --git a/...bsite/web_template/section_with_collapsible_content/section_with_collapsible_content.html b/...bsite/web_template/section_with_collapsible_content/section_with_collapsible_content.html
@@ -7,7 +7,7 @@ <h2 class="section-title">{{ _(title) }}</h2>
 	<div class="collapsible-items">
 		{%- for item in items -%}
 		<div class="collapsible-item">
-			{%- set collapse_id = 'id-' + frappe.utils.generate_hash('Collapse', 12) -%}
+			{%- set collapse_id = 'id-' + frappe.utils.generate_hash(length=12) -%}
 			<a class="collapsible-title" data-toggle="collapse" href="#{{ collapse_id }}" role="button"
 				aria-expanded="false" aria-controls="{{ collapse_id }}">
 				<div class="collapsible-item-title">{{ _(item.title) }}</div>

diff --git a/frappe/website/web_template/section_with_tabs/section_with_tabs.html b/frappe/website/web_template/section_with_tabs/section_with_tabs.html
@@ -11,8 +11,8 @@ <h2 class="section-title">{{ title }}</h2>
 
 	{%- for index in ['1', '2', '3', '4', '5', '6'] -%}
 
-	{%- set buttonid = 'id-' + frappe.utils.generate_hash('TabButton', 12) -%}
-	{%- set panelid = 'id-' + frappe.utils.generate_hash('TabPanel', 12) -%}
+	{%- set buttonid = 'id-' + frappe.utils.generate_hash(length=12) -%}
+	{%- set panelid = 'id-' + frappe.utils.generate_hash(length=12) -%}
 
 	{%- set tab = {
 		'title': values['tab_' + index + '_title'],

diff --git a/frappe/website/web_template/slideshow/slideshow.html b/frappe/website/web_template/slideshow/slideshow.html
@@ -1,6 +1,6 @@
 {%- set slideshow = frappe.get_doc('Website Slideshow', website_slideshow) -%}
 {%- set slides = slideshow.slideshow_items -%}
-{%- set slideshow_id = 'id-' + frappe.utils.generate_hash('Slideshow', 12) -%}
+{%- set slideshow_id = 'id-' + frappe.utils.generate_hash(length=12) -%}
 
 {{ slideshow.header or '' }}