feat: post model-sync patches (#15351)

Ability to run a few patches after the doctype model schema is synced.

Read module-level docstring of patch_handler.py for more info.

frappe/installer.py

@@ -346,14 +346,15 @@ def post_install(rebuild_website=False):
 
 
 def set_all_patches_as_completed(app):
-	patch_path = os.path.join(frappe.get_pymodule_path(app), "patches.txt")
-	if os.path.exists(patch_path):
-		for patch in frappe.get_file_items(patch_path):
-			frappe.get_doc({
-				"doctype": "Patch Log",
-				"patch": patch
-			}).insert(ignore_permissions=True)
-		frappe.db.commit()
+	from frappe.modules.patch_handler import get_patches_from_app
+
+	patches = get_patches_from_app(app)
+	for patch in patches:
+		frappe.get_doc({
+			"doctype": "Patch Log",
+			"patch": patch
+		}).insert(ignore_permissions=True)
+	frappe.db.commit()
 
 
 def init_singles():

frappe/migrate.py

@@ -19,6 +19,8 @@ from frappe.modules.utils import sync_customizations
 from frappe.core.doctype.scheduled_job_type.scheduled_job_type import sync_jobs
 from frappe.search.website_search import build_index_for_all_routes
 from frappe.database.schema import add_column
+from frappe.modules.patch_handler import PatchType
+
 
 
 def migrate(verbose=True, skip_failing=False, skip_search_index=False):
@@ -59,16 +61,13 @@ Otherwise, check the server logs and ensure that all the required services are r
 
 		clear_global_cache()
 
-		#run before_migrate hooks
 		for app in frappe.get_installed_apps():
 			for fn in frappe.get_hooks('before_migrate', app_name=app):
 				frappe.get_attr(fn)()
 
-		# run patches
-		frappe.modules.patch_handler.run_all(skip_failing)
-
-		# sync
+		frappe.modules.patch_handler.run_all(skip_failing=skip_failing, patch_type=PatchType.pre_model_sync)
 		frappe.model.sync.sync_all()
+		frappe.modules.patch_handler.run_all(skip_failing=skip_failing, patch_type=PatchType.post_model_sync)
 		frappe.translate.clear_cache()
 		sync_jobs()
 		sync_fixtures()
@@ -78,18 +77,16 @@ Otherwise, check the server logs and ensure that all the required services are r
 
 		frappe.get_doc('Portal Settings', 'Portal Settings').sync_menu()
 
-		# syncs statics
+		# syncs static files
 		clear_website_cache()
 
 		# updating installed applications data
 		frappe.get_single('Installed Applications').update_versions()
 
-		#run after_migrate hooks
 		for app in frappe.get_installed_apps():
 			for fn in frappe.get_hooks('after_migrate', app_name=app):
 				frappe.get_attr(fn)()
 
-		# build web_routes index
 		if not skip_search_index:
 			# Run this last as it updates the current session
 			print('Building search index for {}'.format(frappe.local.site))

frappe/modules/patch_handler.py

@@ -1,37 +1,76 @@
-# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors
+# Copyright (c) 2021, Frappe Technologies Pvt. Ltd. and Contributors
 # License: MIT. See LICENSE
-"""
-	Execute Patch Files
+""" Patch Handler.
+
+This file manages execution of manaully written patches. Patches are script
+that apply changes in database schema or data to accomodate for changes in the
+code.
+
+Ways to specify patches:
+
+1. patches.txt file specifies patches that run before doctype schema
+migration. Each line represents one patch (old format).
+2. patches.txt can alternatively also separate pre and post model sync
+patches by using INI like file format:
+	```patches.txt
+	[pre_model_sync]
+	app.module.patch1
+	app.module.patch2
 
-	To run directly
 
-	python lib/wnf.py patch patch1, patch2 etc
-	python lib/wnf.py patch -f patch1, patch2 etc
+	[post_model_sync]
+	app.module.patch3
+	```
 
-	where patch1, patch2 is module name
+	When different sections are specified patches are executed in this order:
+		1. Run pre_model_sync patches
+		2. Reload/resync all doctype schema
+		3. Run post_model_sync patches
+
+	Hence any patch that just needs to modify data but doesn't depend on
+	old schema should be added to post_model_sync section of file.
+
+3. simple python commands can be added by starting line with `execute:`
+`execute:` example: `execute:print("hello world")`
 """
-import frappe, frappe.permissions, time
 
-class PatchError(Exception): pass
+import configparser
+import time
+from enum import Enum
+from typing import List, Optional
+
+import frappe
 
-def run_all(skip_failing=False):
+
+class PatchError(Exception):
+	pass
+
+
+class PatchType(Enum):
+	pre_model_sync = "pre_model_sync"
+	post_model_sync = "post_model_sync"
+
+
+def run_all(skip_failing: bool = False, patch_type: Optional[PatchType] = None) -> None:
 	"""run all pending patches"""
-	executed = [p[0] for p in frappe.db.sql("""select patch from `tabPatch Log`""")]
+	executed = set(frappe.get_all("Patch Log", fields="patch", pluck="patch"))
 
 	frappe.flags.final_patches = []
 
 	def run_patch(patch):
 		try:
 			if not run_single(patchmodule = patch):
-				log(patch + ': failed: STOPPED')
+				print(patch + ': failed: STOPPED')
 				raise PatchError(patch)
 		except Exception:
 			if not skip_failing:
 				raise
 			else:
-				log('Failed to execute patch')
+				print('Failed to execute patch')
+
+	patches = get_all_patches(patch_type=patch_type)
 
-	for patch in get_all_patches():
+	for patch in patches:
 		if patch and (patch not in executed):
 			run_patch(patch)
 
@@ -40,18 +79,54 @@ def run_all(skip_failing=False):
 		patch = patch.replace('finally:', '')
 		run_patch(patch)
 
-def get_all_patches():
+def get_all_patches(patch_type: Optional[PatchType] = None) -> List[str]:
+
+	if patch_type and not isinstance(patch_type, PatchType):
+		frappe.throw(f"Unsupported patch type specified: {patch_type}")
+
 	patches = []
 	for app in frappe.get_installed_apps():
-		if app == "shopping_cart":
-			continue
-		# 3-to-4 fix
-		if app=="webnotes":
-			app="frappe"
-		patches.extend(frappe.get_file_items(frappe.get_pymodule_path(app, "patches.txt")))
+		patches.extend(get_patches_from_app(app, patch_type=patch_type))
 
 	return patches
 
+def get_patches_from_app(app: str, patch_type: Optional[PatchType] = None) -> List[str]:
+	""" Get patches from an app's patches.txt
+
+		patches.txt can be:
+			1. ini like file with section for different patch_type
+			2. plain text file with each line representing a patch.
+	"""
+
+	patches_txt = frappe.get_pymodule_path(app, "patches.txt")
+
+	try:
+		# Attempt to parse as ini file with pre/post patches
+		# allow_no_value: patches are not key value pairs
+		# delimiters = '\n' to avoid treating default `:` and `=` in execute as k:v delimiter
+		parser = configparser.ConfigParser(allow_no_value=True, delimiters="\n")
+		# preserve case
+		parser.optionxform = str
+		parser.read(patches_txt)
+
+
+		if not patch_type:
+			return [patch for patch in parser[PatchType.pre_model_sync.value]] + \
+					[patch for patch in parser[PatchType.post_model_sync.value]]
+
+		if patch_type.value in parser.sections():
+			return [patch for patch in parser[patch_type.value]]
+		else:
+			frappe.throw(frappe._("Patch type {} not found in patches.txt").format(patch_type))
+
+	except configparser.MissingSectionHeaderError:
+		# treat as old format with each line representing a single patch
+		# backward compatbility with old patches.txt format
+		if not patch_type or patch_type == PatchType.pre_model_sync:
+			return frappe.get_file_items(patches_txt)
+
+	return []
+
 def reload_doc(args):
 	import frappe.modules
 	run_single(method = frappe.modules.reload_doc, methodargs = args)
@@ -73,7 +148,7 @@ def execute_patch(patchmodule, method=None, methodargs=None):
 	frappe.db.begin()
 	start_time = time.time()
 	try:
-		log('Executing {patch} in {site} ({db})'.format(patch=patchmodule or str(methodargs),
+		print('Executing {patch} in {site} ({db})'.format(patch=patchmodule or str(methodargs),
 			site=frappe.local.site, db=frappe.db.cur_db_name))
 		if patchmodule:
 			if patchmodule.startswith("finally:"):
@@ -96,7 +171,7 @@ def execute_patch(patchmodule, method=None, methodargs=None):
 		frappe.db.commit()
 		end_time = time.time()
 		block_user(False)
-		log('Success: Done in {time}s'.format(time = round(end_time - start_time, 3)))
+		print('Success: Done in {time}s'.format(time = round(end_time - start_time, 3)))
 
 	return True
 
@@ -109,10 +184,7 @@ def executed(patchmodule):
 	if patchmodule.startswith('finally:'):
 		# patches are saved without the finally: tag
 		patchmodule = patchmodule.replace('finally:', '')
-	done = frappe.db.get_value("Patch Log", {"patch": patchmodule})
-	# if done:
-	# 	print "Patch %s already executed in %s" % (patchmodule, frappe.db.cur_db_name)
-	return done
+	return frappe.db.get_value("Patch Log", {"patch": patchmodule})
 
 def block_user(block, msg=None):
 	"""stop/start execution till patch is run"""
@@ -128,6 +200,3 @@ def check_session_stopped():
 	if frappe.db.get_global("__session_status")=='stop':
 		frappe.msgprint(frappe.db.get_global("__session_status_message"))
 		raise frappe.SessionStopped('Session Stopped')
-
-def log(msg):
-	print (msg)

frappe/patches.txt

@@ -1,3 +1,4 @@
+[pre_model_sync]
 frappe.patches.v12_0.remove_deprecated_fields_from_doctype #3
 execute:frappe.utils.global_search.setup_global_search_table()
 execute:frappe.reload_doc('core', 'doctype', 'doctype_action', force=True) #2019-09-23
@@ -87,7 +88,6 @@ frappe.patches.v11_0.set_missing_creation_and_modified_value_for_user_permission
 frappe.patches.v11_0.set_default_letter_head_source
 frappe.patches.v12_0.set_primary_key_in_series
 execute:frappe.delete_doc("Page", "modules", ignore_missing=True)
-frappe.patches.v11_0.set_default_letter_head_source
 frappe.patches.v12_0.setup_comments_from_communications
 frappe.patches.v12_0.replace_null_values_in_tables
 frappe.patches.v12_0.reset_home_settings
@@ -123,7 +123,7 @@ frappe.patches.v12_0.remove_parent_and_parenttype_from_print_formats
 frappe.patches.v12_0.remove_example_email_thread_notify
 execute:from frappe.desk.page.setup_wizard.install_fixtures import update_genders;update_genders()
 frappe.patches.v12_0.set_correct_url_in_files
-execute:frappe.reload_doc('core', 'doctype', 'doctype', force=True)
+execute:frappe.reload_doc('core', 'doctype', 'doctype')
 execute:frappe.reload_doc('custom', 'doctype', 'property_setter')
 frappe.patches.v13_0.remove_invalid_options_for_data_fields
 frappe.patches.v13_0.website_theme_custom_scss
@@ -184,12 +184,14 @@ frappe.patches.v13_0.queryreport_columns
 frappe.patches.v13_0.jinja_hook
 frappe.patches.v13_0.update_notification_channel_if_empty
 frappe.patches.v13_0.set_first_day_of_the_week
-frappe.patches.v14_0.drop_data_import_legacy
 frappe.patches.v14_0.rename_cancelled_documents
-frappe.patches.v14_0.copy_mail_data #08.03.21
 frappe.patches.v14_0.update_workspace2 # 20.09.2021
+frappe.patches.v14_0.save_ratings_in_fraction #23-12-2021
+frappe.patches.v14_0.transform_todo_schema
+
+[post_model_sync]
+frappe.patches.v14_0.drop_data_import_legacy
+frappe.patches.v14_0.copy_mail_data #08.03.21
 frappe.patches.v14_0.update_github_endpoints #08-11-2021
 frappe.patches.v14_0.remove_db_aggregation
-frappe.patches.v14_0.save_ratings_in_fraction #23-12-2021
 frappe.patches.v14_0.update_color_names_in_kanban_board_column
-frappe.patches.v14_0.transform_todo_schema

frappe/patches/v14_0/copy_mail_data.py

@@ -3,9 +3,6 @@ import frappe
 
 
 def execute():
-	frappe.reload_doc("email", "doctype", "imap_folder")
-	frappe.reload_doc("email", "doctype", "email_account")
-
 	# patch for all Email Account with the flag use_imap
 	for email_account in frappe.get_list("Email Account", filters={"enable_incoming": 1, "use_imap": 1}):
 		# get all data from Email Account

frappe/patches/v14_0/update_color_names_in_kanban_board_column.py

@@ -5,7 +5,6 @@ from __future__ import unicode_literals
 import frappe
 
 def execute():
-	frappe.reload_doc("desk", "doctype", "kanban_board_column")
 	indicator_map = {
 		'blue': 'Blue',
 		'orange': 'Orange',

frappe/tests/test_patches.py

@@ -15,3 +15,18 @@ class TestPatches(unittest.TestCase):
 				self.assertTrue(frappe.get_attr(patchmodule.split()[0] + ".execute"))
 
 		frappe.flags.in_install = False
+
+	def test_get_patch_list(self):
+		pre = patch_handler.get_patches_from_app("frappe", patch_handler.PatchType.pre_model_sync)
+		post = patch_handler.get_patches_from_app("frappe", patch_handler.PatchType.post_model_sync)
+		all_patches = patch_handler.get_patches_from_app("frappe")
+		self.assertGreater(len(pre), 0)
+		self.assertGreater(len(post), 0)
+
+		self.assertEqual(len(all_patches), len(pre) + len(post))
+
+	def test_all_patches_are_marked_completed(self):
+		all_patches = patch_handler.get_patches_from_app("frappe")
+		finished_patches = frappe.db.count("Patch Log")
+
+		self.assertGreaterEqual(finished_patches, len(all_patches))

frappe/utils/boilerplate.py

@@ -1,6 +1,11 @@
 # Copyright (c) 2021, Frappe Technologies Pvt. Ltd. and Contributors
 # License: MIT. See LICENSE
-import frappe, os, re, git
+
+import git
+import os
+import re
+
+import frappe
 from frappe.utils import touch_file, cstr
 
 def make_boilerplate(dest, app_name, no_git=False):