From 2bbe464f59e2808a35f2d8d2bc938e05b6ef8af2 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 24 Jul 2020 17:53:22 +0530 Subject: [PATCH 01/71] feat: Allow skip tables during backups --- frappe/utils/backups.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/frappe/utils/backups.py b/frappe/utils/backups.py index 3b905de6bd..d0867e40e6 100644 --- a/frappe/utils/backups.py +++ b/frappe/utils/backups.py @@ -148,7 +148,11 @@ class BackupGenerator: args = dict([item[0], frappe.utils.esc(str(item[1]), '$ ')] for item in self.__dict__.copy().items()) - cmd_string = """mysqldump --single-transaction --quick --lock-tables=false -u %(user)s -p%(password)s %(db_name)s -h %(db_host)s -P %(db_port)s | gzip > %(backup_path_db)s """ % args + if self.verbose: + print("Skipping Tables: {0}\n".format(", ".join(frappe.conf.ignore_tables_in_backup))) + + args["skip_tables"] = " ".join(["--ignore-table={0}.{1}".format(frappe.conf.db_name, table) for table in frappe.conf.ignore_tables_in_backup]) + cmd_string = """mysqldump --single-transaction --quick --lock-tables=false -u %(user)s -p%(password)s %(db_name)s -h %(db_host)s -P %(db_port)s %(skip_tables)s | gzip > %(backup_path_db)s """ % args if self.db_type == 'postgres': cmd_string = "pg_dump postgres://{user}:{password}@{db_host}:{db_port}/{db_name} | gzip > {backup_path_db}".format( From befea91e6d5b7081960807976107f823ab559d18 Mon Sep 17 00:00:00 2001 From: gavin Date: Mon, 27 Jul 2020 11:01:01 +0530 Subject: [PATCH 02/71] fix: empty conf value for ignore_tables_in_backup Co-authored-by: Faris Ansari --- frappe/utils/backups.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/utils/backups.py b/frappe/utils/backups.py index d0867e40e6..e475cef276 100644 --- a/frappe/utils/backups.py +++ b/frappe/utils/backups.py @@ -151,7 +151,7 @@ class BackupGenerator: if self.verbose: print("Skipping Tables: {0}\n".format(", ".join(frappe.conf.ignore_tables_in_backup))) - args["skip_tables"] = " ".join(["--ignore-table={0}.{1}".format(frappe.conf.db_name, table) for table in frappe.conf.ignore_tables_in_backup]) + args["skip_tables"] = " ".join(["--ignore-table={0}.{1}".format(frappe.conf.db_name, table) for table in frappe.conf.ignore_tables_in_backup or []]) cmd_string = """mysqldump --single-transaction --quick --lock-tables=false -u %(user)s -p%(password)s %(db_name)s -h %(db_host)s -P %(db_port)s %(skip_tables)s | gzip > %(backup_path_db)s """ % args if self.db_type == 'postgres': From 6cb6a18b48a050efc7ee1e307f180d3e4ba55ec3 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 23 Sep 2020 13:18:04 +0530 Subject: [PATCH 03/71] feat: Ability to exclude/include certain doctypes from conf or via CLI --- frappe/commands/site.py | 18 +++++++- frappe/utils/backups.py | 92 +++++++++++++++++++++++++++++++++-------- 2 files changed, 90 insertions(+), 20 deletions(-) diff --git a/frappe/commands/site.py b/frappe/commands/site.py index c5008b32a5..a727341b73 100755 --- a/frappe/commands/site.py +++ b/frappe/commands/site.py @@ -387,10 +387,13 @@ def use(site, sites_path='.'): @click.command('backup') @click.option('--with-files', default=False, is_flag=True, help="Take backup with files") +@click.option('--ignore-backup-conf', default=False, is_flag=True, help="Ignore excludes/includes set in config") +@click.option('--include', default="", type=str, help="Specify the DocTypes to backup seperated by commas") +@click.option('--exclude', default="", type=str, help="Specify the DocTypes to not backup seperated by commas") @click.option('--verbose', default=False, is_flag=True) @pass_context def backup(context, with_files=False, backup_path_db=None, backup_path_files=None, - backup_path_private_files=None, quiet=False, verbose=False): + backup_path_private_files=None, quiet=False, verbose=False, ignore_backup_conf=False, include="", exclude=""): "Backup" from frappe.utils.backups import scheduled_backup verbose = verbose or context.verbose @@ -399,10 +402,21 @@ def backup(context, with_files=False, backup_path_db=None, backup_path_files=Non try: frappe.init(site=site) frappe.connect() - odb = scheduled_backup(ignore_files=not with_files, backup_path_db=backup_path_db, backup_path_files=backup_path_files, backup_path_private_files=backup_path_private_files, force=True, verbose=verbose) + odb = scheduled_backup( + ignore_files=not with_files, + backup_path_db=backup_path_db, + backup_path_files=backup_path_files, + backup_path_private_files=backup_path_private_files, + ignore_conf=ignore_backup_conf, + include_doctypes=include, + exclude_doctypes=exclude, + verbose=verbose, + force=True, + ) except Exception as e: if verbose: print("Backup failed for {0}. Database or site_config.json may be corrupted".format(site)) + print(e) exit_code = 1 continue diff --git a/frappe/utils/backups.py b/frappe/utils/backups.py index 4d29134abd..d3a063cd89 100644 --- a/frappe/utils/backups.py +++ b/frappe/utils/backups.py @@ -18,6 +18,7 @@ from frappe.utils import cstr, get_url, now_datetime # backup variable for backwards compatibility verbose = False _verbose = verbose +base_tables = ["__Auth", "__global_search", "__UserSettings"] class BackupGenerator: @@ -29,7 +30,7 @@ class BackupGenerator: """ def __init__(self, db_name, user, password, backup_path_db=None, backup_path_files=None, backup_path_private_files=None, db_host="localhost", db_port=None, verbose=False, - db_type='mariadb', backup_path_conf=None): + db_type='mariadb', backup_path_conf=None, ignore_conf=False, include_doctypes="", exclude_doctypes=""): global _verbose self.db_host = db_host self.db_port = db_port @@ -41,6 +42,9 @@ class BackupGenerator: self.backup_path_db = backup_path_db self.backup_path_files = backup_path_files self.backup_path_private_files = backup_path_private_files + self.ignore_conf = ignore_conf + self.include_doctypes = include_doctypes + self.exclude_doctypes = exclude_doctypes if not self.db_type: self.db_type = 'mariadb' @@ -54,6 +58,7 @@ class BackupGenerator: self.site_slug = site.replace('.', '_') self.verbose = verbose self.setup_backup_directory() + self.setup_backup_tables() _verbose = verbose def setup_backup_directory(self): @@ -68,6 +73,35 @@ class BackupGenerator: dir = os.path.dirname(file_path) os.makedirs(dir, exist_ok=True) + def setup_backup_tables(self): + """Sets self.backup_includes, self.backup_excludes based on passed args + """ + def get_tables(doctypes): + tables = [] + for doctype in doctypes: + if doctype: + if doctype.startswith("tab"): + tables.append(doctype) + else: + tables.append("tab" + doctype) + return tables + + passed_tables = { + "include": get_tables(self.include_doctypes.strip().split(",")), + "exclude": get_tables(self.exclude_doctypes.strip().split(",")) + } + conf_tables = { + "include": get_tables(frappe.conf.backup.get("includes", [])) + base_tables, + "exclude": get_tables(frappe.conf.backup.get("excludes", [])) + } + + self.backup_includes = passed_tables["include"] + self.backup_excludes = passed_tables["exclude"] + + if not self.ignore_conf: + self.backup_includes = self.backup_includes or conf_tables["include"] + self.backup_excludes = self.backup_excludes or conf_tables["exclude"] + @property def site_config_backup_path(self): # For backwards compatibility @@ -189,23 +223,42 @@ class BackupGenerator: args = dict([item[0], frappe.utils.esc(str(item[1]), '$ ')] for item in self.__dict__.copy().items()) - if self.verbose: - print("Skipping Tables: {0}\n".format(", ".join(frappe.conf.ignore_tables_in_backup))) - - args["skip_tables"] = " ".join(["--ignore-table={0}.{1}".format(frappe.conf.db_name, table) for table in frappe.conf.ignore_tables_in_backup or []]) - cmd_string = """mysqldump --single-transaction --quick --lock-tables=false -u %(user)s -p%(password)s %(db_name)s -h %(db_host)s -P %(db_port)s %(skip_tables)s | gzip > %(backup_path_db)s """ % args + if self.backup_includes: + print("Backing Up Tables: {0}\n".format(", ".join(self.backup_includes))) + elif self.backup_excludes: + print("Skipping Tables: {0}\n".format(", ".join(self.backup_excludes))) if self.db_type == 'postgres': - cmd_string = "pg_dump postgres://{user}:{password}@{db_host}:{db_port}/{db_name} | gzip > {backup_path_db}".format( - user=args.get('user'), - password=args.get('password'), - db_host=args.get('db_host'), - db_port=args.get('db_port'), - db_name=args.get('db_name'), - backup_path_db=args.get('backup_path_db') - ) + if self.backup_includes: + args["include"] = " ".join(["--table='{0}'".format(table) for table in self.backup_includes]) + elif self.backup_excludes: + args["exclude"] = " ".join(["--exclude-table='{0}'".format(table) for table in self.backup_excludes]) - err, out = frappe.utils.execute_in_shell(cmd_string) + cmd_string = "pg_dump postgres://{user}:{password}@{db_host}:{db_port}/{db_name} {include} {exclude} | gzip > {backup_path_db}" + + else: + if self.backup_includes: + args["include"] = " ".join(["'{0}'".format(x) for x in self.backup_includes]) + + elif self.backup_excludes: + args["exclude"] = " ".join(["--ignore-table='{0}.{1}'".format(frappe.conf.db_name, table) for table in self.backup_excludes]) + + cmd_string = "mysqldump --single-transaction --quick --lock-tables=false -u {user} -p{password} {db_name} -h {db_host} -P {db_port} {include} {exclude} | gzip > {backup_path_db}" + + command = cmd_string.format( + user=args.get('user'), + password=args.get('password'), + db_host=args.get('db_host'), + db_port=args.get('db_port'), + db_name=args.get('db_name'), + backup_path_db=args.get('backup_path_db'), + exclude=args.get('exclude', ''), + include=args.get('include', '') + ) + + print(command) + + err, out = frappe.utils.execute_in_shell(command) def send_email(self): """ @@ -279,14 +332,14 @@ def fetch_latest_backups(): } -def scheduled_backup(older_than=6, ignore_files=False, backup_path_db=None, backup_path_files=None, backup_path_private_files=None, force=False, verbose=False): +def scheduled_backup(older_than=6, ignore_files=False, backup_path_db=None, backup_path_files=None, backup_path_private_files=None, force=False, verbose=False, ignore_conf=False, include_doctypes="", exclude_doctypes=""): """this function is called from scheduler deletes backups older than 7 days takes backup""" - odb = new_backup(older_than, ignore_files, backup_path_db=backup_path_db, backup_path_files=backup_path_files, force=force, verbose=verbose) + odb = new_backup(older_than, ignore_files, backup_path_db=backup_path_db, backup_path_files=backup_path_files, force=force, verbose=verbose, ignore_conf=ignore_conf, include_doctypes=include_doctypes, exclude_doctypes=exclude_doctypes) return odb -def new_backup(older_than=6, ignore_files=False, backup_path_db=None, backup_path_files=None, backup_path_private_files=None, force=False, verbose=False): +def new_backup(older_than=6, ignore_files=False, backup_path_db=None, backup_path_files=None, backup_path_private_files=None, force=False, verbose=False, ignore_conf=False, include_doctypes="", exclude_doctypes=""): delete_temp_backups(older_than = frappe.conf.keep_backups_for_hours or 24) odb = BackupGenerator(frappe.conf.db_name, frappe.conf.db_name,\ frappe.conf.db_password, @@ -295,6 +348,9 @@ def new_backup(older_than=6, ignore_files=False, backup_path_db=None, backup_pat db_host = frappe.db.host, db_port = frappe.db.port, db_type = frappe.conf.db_type, + ignore_conf=ignore_conf, + include_doctypes=include_doctypes, + exclude_doctypes=exclude_doctypes, verbose=verbose) odb.get_backup(older_than, ignore_files, force=force) return odb From bb2a1910e86c959286f461ededdf7678dffa6dfc Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 24 Sep 2020 11:34:43 +0530 Subject: [PATCH 04/71] fix: Handle excludes or includes explicitly * Take backup only if doctype exists * Show mysql command only if verbose is passed --- frappe/utils/backups.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/frappe/utils/backups.py b/frappe/utils/backups.py index d3a063cd89..b8d1da299a 100644 --- a/frappe/utils/backups.py +++ b/frappe/utils/backups.py @@ -76,10 +76,11 @@ class BackupGenerator: def setup_backup_tables(self): """Sets self.backup_includes, self.backup_excludes based on passed args """ + existing_doctypes = set([x.name for x in frappe.get_all("DocType")]) def get_tables(doctypes): tables = [] for doctype in doctypes: - if doctype: + if doctype and doctype in existing_doctypes: if doctype.startswith("tab"): tables.append(doctype) else: @@ -98,7 +99,7 @@ class BackupGenerator: self.backup_includes = passed_tables["include"] self.backup_excludes = passed_tables["exclude"] - if not self.ignore_conf: + if not (self.backup_includes or self.backup_excludes) and not self.ignore_conf: self.backup_includes = self.backup_includes or conf_tables["include"] self.backup_excludes = self.backup_excludes or conf_tables["exclude"] @@ -239,7 +240,6 @@ class BackupGenerator: else: if self.backup_includes: args["include"] = " ".join(["'{0}'".format(x) for x in self.backup_includes]) - elif self.backup_excludes: args["exclude"] = " ".join(["--ignore-table='{0}.{1}'".format(frappe.conf.db_name, table) for table in self.backup_excludes]) @@ -256,7 +256,8 @@ class BackupGenerator: include=args.get('include', '') ) - print(command) + if self.verbose: + print(command) err, out = frappe.utils.execute_in_shell(command) From bde7adeb2f597ef59d7c173ad5265b4b1fab6c27 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 7 Oct 2020 14:06:28 +0530 Subject: [PATCH 05/71] style: Black + flake8 --- frappe/utils/backups.py | 445 ++++++++++++++++++++++++++++------------ 1 file changed, 314 insertions(+), 131 deletions(-) diff --git a/frappe/utils/backups.py b/frappe/utils/backups.py index 25147f2085..b59f5526e9 100644 --- a/frappe/utils/backups.py +++ b/frappe/utils/backups.py @@ -2,7 +2,6 @@ # MIT License. See license.txt # imports - standard imports -import json import os from calendar import timegm from datetime import datetime @@ -25,12 +24,31 @@ base_tables = ["__Auth", "__global_search", "__UserSettings"] class BackupGenerator: """ - This class contains methods to perform On Demand Backup + This class contains methods to perform On Demand Backup - To initialize, specify (db_name, user, password, db_file_name=None, db_host="localhost") - If specifying db_file_name, also append ".sql.gz" + To initialize, specify (db_name, user, password, db_file_name=None, db_host="localhost") + If specifying db_file_name, also append ".sql.gz" """ - def __init__(self, db_name, user, password, backup_path=None, backup_path_db=None, backup_path_files=None, backup_path_private_files=None, db_host="localhost", db_port=None, db_type='mariadb', backup_path_conf=None, ignore_conf=False, compress_files=False, include_doctypes="", exclude_doctypes="", verbose=False): + + def __init__( + self, + db_name, + user, + password, + backup_path=None, + backup_path_db=None, + backup_path_files=None, + backup_path_private_files=None, + db_host="localhost", + db_port=None, + db_type="mariadb", + backup_path_conf=None, + ignore_conf=False, + compress_files=False, + include_doctypes="", + exclude_doctypes="", + verbose=False, + ): global _verbose self.compress_files = compress_files or compress self.db_host = db_host @@ -49,22 +67,29 @@ class BackupGenerator: self.exclude_doctypes = exclude_doctypes if not self.db_type: - self.db_type = 'mariadb' + self.db_type = "mariadb" - if not self.db_port and self.db_type == 'mariadb': - self.db_port = 3306 - elif not self.db_port and self.db_type == 'postgres': - self.db_port = 5432 + if not self.db_port: + if self.db_type == "mariadb": + self.db_port = 3306 + if self.db_type == "postgres": + self.db_port = 5432 site = frappe.local.site or frappe.generate_hash(length=8) - self.site_slug = site.replace('.', '_') + self.site_slug = site.replace(".", "_") self.verbose = verbose self.setup_backup_directory() self.setup_backup_tables() _verbose = verbose def setup_backup_directory(self): - specified = self.backup_path or self.backup_path_db or self.backup_path_files or self.backup_path_private_files or self.backup_path_conf + specified = ( + self.backup_path + or self.backup_path_db + or self.backup_path_files + or self.backup_path_private_files + or self.backup_path_conf + ) if not specified: backups_folder = get_backup_path() @@ -74,15 +99,22 @@ class BackupGenerator: if self.backup_path: os.makedirs(self.backup_path, exist_ok=True) - for file_path in set([self.backup_path_files, self.backup_path_db, self.backup_path_private_files, self.backup_path_conf]): + for file_path in set( + [ + self.backup_path_files, + self.backup_path_db, + self.backup_path_private_files, + self.backup_path_conf, + ] + ): if file_path: dir = os.path.dirname(file_path) os.makedirs(dir, exist_ok=True) def setup_backup_tables(self): - """Sets self.backup_includes, self.backup_excludes based on passed args - """ + """Sets self.backup_includes, self.backup_excludes based on passed args""" existing_doctypes = set([x.name for x in frappe.get_all("DocType")]) + def get_tables(doctypes): tables = [] for doctype in doctypes: @@ -95,11 +127,11 @@ class BackupGenerator: passed_tables = { "include": get_tables(self.include_doctypes.strip().split(",")), - "exclude": get_tables(self.exclude_doctypes.strip().split(",")) + "exclude": get_tables(self.exclude_doctypes.strip().split(",")), } conf_tables = { "include": get_tables(frappe.conf.backup.get("includes", [])) + base_tables, - "exclude": get_tables(frappe.conf.backup.get("excludes", [])) + "exclude": get_tables(frappe.conf.backup.get("excludes", [])), } self.backup_includes = passed_tables["include"] @@ -112,24 +144,43 @@ class BackupGenerator: @property def site_config_backup_path(self): # For backwards compatibility - click.secho("BackupGenerator.site_config_backup_path has been deprecated in favour of BackupGenerator.backup_path_conf", fg="yellow") + click.secho( + "BackupGenerator.site_config_backup_path has been deprecated in favour of" + " BackupGenerator.backup_path_conf", + fg="yellow", + ) return getattr(self, "backup_path_conf", None) def get_backup(self, older_than=24, ignore_files=False, force=False): """ - Takes a new dump if existing file is old - and sends the link to the file as email + Takes a new dump if existing file is old + and sends the link to the file as email """ - #Check if file exists and is less than a day old - #If not Take Dump + # Check if file exists and is less than a day old + # If not Take Dump if not force: - last_db, last_file, last_private_file, site_config_backup_path = self.get_recent_backup(older_than) + ( + last_db, + last_file, + last_private_file, + site_config_backup_path, + ) = self.get_recent_backup(older_than) else: - last_db, last_file, last_private_file, site_config_backup_path = False, False, False, False + last_db, last_file, last_private_file, site_config_backup_path = ( + False, + False, + False, + False, + ) - self.todays_date = now_datetime().strftime('%Y%m%d_%H%M%S') + self.todays_date = now_datetime().strftime("%Y%m%d_%H%M%S") - if not (self.backup_path_conf and self.backup_path_db and self.backup_path_files and self.backup_path_private_files): + if not ( + self.backup_path_conf + and self.backup_path_db + and self.backup_path_files + and self.backup_path_private_files + ): self.set_backup_file_name() if not (last_db and last_file and last_private_file and site_config_backup_path): @@ -145,7 +196,7 @@ class BackupGenerator: self.backup_path_conf = site_config_backup_path def set_backup_file_name(self): - #Generate a random name using today's date and a 8 digit random number + # Generate a random name using today's date and a 8 digit random number for_conf = self.todays_date + "-" + self.site_slug + "-site_config_backup.json" for_db = self.todays_date + "-" + self.site_slug + "-database.sql.gz" ext = "tgz" if self.compress_files else "tar" @@ -191,8 +242,7 @@ class BackupGenerator: return file_path latest_backups = { - file_type: get_latest(pattern) - for file_type, pattern in file_type_slugs.items() + file_type: get_latest(pattern) for file_type, pattern in file_type_slugs.items() } recent_backups = { @@ -208,32 +258,40 @@ class BackupGenerator: def zip_files(self): # For backwards compatibility - pre v13 - click.secho("BackupGenerator.zip_files has been deprecated in favour of BackupGenerator.backup_files", fg="yellow") + click.secho( + "BackupGenerator.zip_files has been deprecated in favour of" + " BackupGenerator.backup_files", + fg="yellow", + ) return self.backup_files() def get_summary(self): summary = { "config": { "path": self.backup_path_conf, - "size": get_file_size(self.backup_path_conf, format=True) + "size": get_file_size(self.backup_path_conf, format=True), }, "database": { "path": self.backup_path_db, - "size": get_file_size(self.backup_path_db, format=True) - } + "size": get_file_size(self.backup_path_db, format=True), + }, } - if os.path.exists(self.backup_path_files) and os.path.exists(self.backup_path_private_files): - summary.update({ - "public": { - "path": self.backup_path_files, - "size": get_file_size(self.backup_path_files, format=True) - }, - "private": { - "path": self.backup_path_private_files, - "size": get_file_size(self.backup_path_private_files, format=True) + if os.path.exists(self.backup_path_files) and os.path.exists( + self.backup_path_private_files + ): + summary.update( + { + "public": { + "path": self.backup_path_files, + "size": get_file_size(self.backup_path_files, format=True), + }, + "private": { + "path": self.backup_path_private_files, + "size": get_file_size(self.backup_path_private_files, format=True), + }, } - }) + ) return summary @@ -249,13 +307,17 @@ class BackupGenerator: for folder in ("public", "private"): files_path = frappe.get_site_path(folder, "files") - backup_path = self.backup_path_files if folder=="public" else self.backup_path_private_files + backup_path = ( + self.backup_path_files if folder == "public" else self.backup_path_private_files + ) if self.compress_files: cmd_string = "tar cf - {1} | gzip > {0}" else: cmd_string = "tar -cf {0} {1}" - output = subprocess.check_output(cmd_string.format(backup_path, files_path), shell=True) + output = subprocess.check_output( + cmd_string.format(backup_path, files_path), shell=True + ) if self.verbose and output: print(output.decode("utf8")) @@ -271,39 +333,57 @@ class BackupGenerator: import frappe.utils # escape reserved characters - args = dict([item[0], frappe.utils.esc(str(item[1]), '$ ')] - for item in self.__dict__.copy().items()) + args = dict( + [item[0], frappe.utils.esc(str(item[1]), "$ ")] + for item in self.__dict__.copy().items() + ) if self.backup_includes: print("Backing Up Tables: {0}\n".format(", ".join(self.backup_includes))) elif self.backup_excludes: print("Skipping Tables: {0}\n".format(", ".join(self.backup_excludes))) - if self.db_type == 'postgres': + if self.db_type == "postgres": if self.backup_includes: - args["include"] = " ".join(["--table='{0}'".format(table) for table in self.backup_includes]) + args["include"] = " ".join( + ["--table='{0}'".format(table) for table in self.backup_includes] + ) elif self.backup_excludes: - args["exclude"] = " ".join(["--exclude-table='{0}'".format(table) for table in self.backup_excludes]) + args["exclude"] = " ".join( + ["--exclude-table='{0}'".format(table) for table in self.backup_excludes] + ) - cmd_string = "pg_dump postgres://{user}:{password}@{db_host}:{db_port}/{db_name} {include} {exclude} | gzip > {backup_path_db}" + cmd_string = ( + "pg_dump postgres://{user}:{password}@{db_host}:{db_port}/{db_name}" + " {include} {exclude} | gzip > {backup_path_db}" + ) else: if self.backup_includes: args["include"] = " ".join(["'{0}'".format(x) for x in self.backup_includes]) elif self.backup_excludes: - args["exclude"] = " ".join(["--ignore-table='{0}.{1}'".format(frappe.conf.db_name, table) for table in self.backup_excludes]) + args["exclude"] = " ".join( + [ + "--ignore-table='{0}.{1}'".format(frappe.conf.db_name, table) + for table in self.backup_excludes + ] + ) - cmd_string = "mysqldump --single-transaction --quick --lock-tables=false -u {user} -p{password} {db_name} -h {db_host} -P {db_port} {include} {exclude} | gzip > {backup_path_db}" + cmd_string = ( + "mysqldump --single-transaction --quick --lock-tables=false -u {user}" + " -p{password} {db_name} -h {db_host} -P {db_port} {include} {exclude}" + " | gzip > {backup_path_db}" + ) command = cmd_string.format( - user=args.get('user'), - password=args.get('password'), - db_host=args.get('db_host'), - db_port=args.get('db_port'), - db_name=args.get('db_name'), - backup_path_db=args.get('backup_path_db'), - exclude=args.get('exclude', ''), - include=args.get('include', '') + user=args.get("user"), + password=args.get("password"), + db_host=args.get("db_host"), + db_port=args.get("db_port"), + db_name=args.get("db_name"), + backup_path_db=args.get("backup_path_db"), + exclude=args.get("exclude", ""), + include=args.get("include", ""), ) if self.verbose: @@ -313,13 +393,17 @@ class BackupGenerator: def send_email(self): """ - Sends the link to backup file located at erpnext/backups + Sends the link to backup file located at erpnext/backups """ from frappe.email import get_system_managers recipient_list = get_system_managers() - db_backup_url = get_url(os.path.join('backups', os.path.basename(self.backup_path_db))) - files_backup_url = get_url(os.path.join('backups', os.path.basename(self.backup_path_files))) + db_backup_url = get_url( + os.path.join("backups", os.path.basename(self.backup_path_db)) + ) + files_backup_url = get_url( + os.path.join("backups", os.path.basename(self.backup_path_files)) + ) msg = """Hello, @@ -331,11 +415,13 @@ Your backups are ready to be downloaded. This link will be valid for 24 hours. A new backup will be available for download only after 24 hours.""" % { "db_backup_url": db_backup_url, - "files_backup_url": files_backup_url + "files_backup_url": files_backup_url, } datetime_str = datetime.fromtimestamp(os.stat(self.backup_path_db).st_ctime) - subject = datetime_str.strftime("%d/%m/%Y %H:%M:%S") + """ - Backup ready to be downloaded""" + subject = ( + datetime_str.strftime("%d/%m/%Y %H:%M:%S") + """ - Backup ready to be downloaded""" + ) frappe.sendmail(recipients=recipient_list, msg=msg, subject=subject) return recipient_list @@ -344,16 +430,25 @@ download only after 24 hours.""" % { @frappe.whitelist() def get_backup(): """ - This function is executed when the user clicks on - Toos > Download Backup + This function is executed when the user clicks on + Toos > Download Backup """ delete_temp_backups() - odb = BackupGenerator(frappe.conf.db_name, frappe.conf.db_name,\ - frappe.conf.db_password, db_host = frappe.db.host,\ - db_type=frappe.conf.db_type, db_port=frappe.conf.db_port) + odb = BackupGenerator( + frappe.conf.db_name, + frappe.conf.db_name, + frappe.conf.db_password, + db_host=frappe.db.host, + db_type=frappe.conf.db_type, + db_port=frappe.conf.db_port, + ) odb.get_backup() recipient_list = odb.send_email() - frappe.msgprint(_("Download link for your backup will be emailed on the following email address: {0}").format(', '.join(recipient_list))) + frappe.msgprint( + _( + "Download link for your backup will be emailed on the following email address: {0}" + ).format(", ".join(recipient_list)) + ) @frappe.whitelist() @@ -375,44 +470,86 @@ def fetch_latest_backups(): ) database, public, private, config = odb.get_recent_backup(older_than=24 * 30) - return { - "database": database, - "public": public, - "private": private, - "config": config - } + return {"database": database, "public": public, "private": private, "config": config} -def scheduled_backup(older_than=6, ignore_files=False, backup_path=None, backup_path_db=None, backup_path_files=None, backup_path_private_files=None, backup_path_conf=None, ignore_conf=False, include_doctypes="", exclude_doctypes="", compress=False, force=False, verbose=False): +def scheduled_backup( + older_than=6, + ignore_files=False, + backup_path=None, + backup_path_db=None, + backup_path_files=None, + backup_path_private_files=None, + backup_path_conf=None, + ignore_conf=False, + include_doctypes="", + exclude_doctypes="", + compress=False, + force=False, + verbose=False, +): """this function is called from scheduler - deletes backups older than 7 days - takes backup""" - odb = new_backup(older_than=older_than, ignore_files=ignore_files, backup_path=None, backup_path_db=backup_path_db, backup_path_files=backup_path_files, backup_path_private_files=backup_path_private_files, backup_path_conf=backup_path_conf, ignore_conf=ignore_conf, include_doctypes=include_doctypes, exclude_doctypes=exclude_doctypes, compress=compress, force=force, verbose=verbose) + deletes backups older than 7 days + takes backup""" + odb = new_backup( + older_than=older_than, + ignore_files=ignore_files, + backup_path=None, + backup_path_db=backup_path_db, + backup_path_files=backup_path_files, + backup_path_private_files=backup_path_private_files, + backup_path_conf=backup_path_conf, + ignore_conf=ignore_conf, + include_doctypes=include_doctypes, + exclude_doctypes=exclude_doctypes, + compress=compress, + force=force, + verbose=verbose, + ) return odb -def new_backup(older_than=6, ignore_files=False, backup_path=None, backup_path_db=None, backup_path_files=None, backup_path_private_files=None, backup_path_conf=None, ignore_conf=False, include_doctypes="", exclude_doctypes="", compress=False, force=False, verbose=False): - delete_temp_backups(older_than = frappe.conf.keep_backups_for_hours or 24) - odb = BackupGenerator(frappe.conf.db_name, frappe.conf.db_name,\ - frappe.conf.db_password, - db_host = frappe.db.host, - db_port = frappe.db.port, - db_type = frappe.conf.db_type, - backup_path=backup_path, - backup_path_db=backup_path_db, - backup_path_files=backup_path_files, - backup_path_private_files=backup_path_private_files, - backup_path_conf=backup_path_conf, - ignore_conf=ignore_conf, - include_doctypes=include_doctypes, - exclude_doctypes=exclude_doctypes, - verbose=verbose, - compress_files=compress) + +def new_backup( + older_than=6, + ignore_files=False, + backup_path=None, + backup_path_db=None, + backup_path_files=None, + backup_path_private_files=None, + backup_path_conf=None, + ignore_conf=False, + include_doctypes="", + exclude_doctypes="", + compress=False, + force=False, + verbose=False, +): + delete_temp_backups(older_than=frappe.conf.keep_backups_for_hours or 24) + odb = BackupGenerator( + frappe.conf.db_name, + frappe.conf.db_name, + frappe.conf.db_password, + db_host=frappe.db.host, + db_port=frappe.db.port, + db_type=frappe.conf.db_type, + backup_path=backup_path, + backup_path_db=backup_path_db, + backup_path_files=backup_path_files, + backup_path_private_files=backup_path_private_files, + backup_path_conf=backup_path_conf, + ignore_conf=ignore_conf, + include_doctypes=include_doctypes, + exclude_doctypes=exclude_doctypes, + verbose=verbose, + compress_files=compress, + ) odb.get_backup(older_than, ignore_files, force=force) return odb + def delete_temp_backups(older_than=24): """ - Cleans up the backup_link_path directory by deleting files older than 24 hours + Cleans up the backup_link_path directory by deleting files older than 24 hours """ backup_path = get_backup_path() if os.path.exists(backup_path): @@ -422,54 +559,72 @@ def delete_temp_backups(older_than=24): if is_file_old(this_file_path, older_than): os.remove(this_file_path) + def is_file_old(db_file_name, older_than=24): - """ - Checks if file exists and is older than specified hours - Returns -> - True: file does not exist or file is old - False: file is new - """ - if os.path.isfile(db_file_name): - from datetime import timedelta - #Get timestamp of the file - file_datetime = datetime.fromtimestamp\ - (os.stat(db_file_name).st_ctime) - if datetime.today() - file_datetime >= timedelta(hours = older_than): - if _verbose: - print("File is old") - return True - else: - if _verbose: - print("File is recent") - return False + """ + Checks if file exists and is older than specified hours + Returns -> + True: file does not exist or file is old + False: file is new + """ + if os.path.isfile(db_file_name): + from datetime import timedelta + + # Get timestamp of the file + file_datetime = datetime.fromtimestamp(os.stat(db_file_name).st_ctime) + if datetime.today() - file_datetime >= timedelta(hours=older_than): + if _verbose: + print("File is old") + return True else: if _verbose: - print("File does not exist") - return True + print("File is recent") + return False + else: + if _verbose: + print("File does not exist") + return True + def get_backup_path(): backup_path = frappe.utils.get_site_path(conf.get("backup_path", "private/backups")) return backup_path -def backup(with_files=False, backup_path_db=None, backup_path_files=None, backup_path_private_files=None, backup_path_conf=None, quiet=False): + +def backup( + with_files=False, + backup_path_db=None, + backup_path_files=None, + backup_path_private_files=None, + backup_path_conf=None, + quiet=False, +): "Backup" - odb = scheduled_backup(ignore_files=not with_files, backup_path_db=backup_path_db, backup_path_files=backup_path_files, backup_path_private_files=backup_path_private_files, backup_path_conf=backup_path_conf, force=True) + odb = scheduled_backup( + ignore_files=not with_files, + backup_path_db=backup_path_db, + backup_path_files=backup_path_files, + backup_path_private_files=backup_path_private_files, + backup_path_conf=backup_path_conf, + force=True, + ) return { "backup_path_db": odb.backup_path_db, "backup_path_files": odb.backup_path_files, - "backup_path_private_files": odb.backup_path_private_files + "backup_path_private_files": odb.backup_path_private_files, } if __name__ == "__main__": """ - is_file_old db_name user password db_host db_type db_port - get_backup db_name user password db_host db_type db_port + is_file_old db_name user password db_host db_type db_port + get_backup db_name user password db_host db_type db_port """ import sys + cmd = sys.argv[1] - db_type = 'mariadb' + db_type = "mariadb" try: db_type = sys.argv[6] except IndexError: @@ -482,19 +637,47 @@ if __name__ == "__main__": pass if cmd == "is_file_old": - odb = BackupGenerator(sys.argv[2], sys.argv[3], sys.argv[4], sys.argv[5] or "localhost", db_type=db_type, db_port=db_port) + odb = BackupGenerator( + sys.argv[2], + sys.argv[3], + sys.argv[4], + sys.argv[5] or "localhost", + db_type=db_type, + db_port=db_port, + ) is_file_old(odb.db_file_name) if cmd == "get_backup": - odb = BackupGenerator(sys.argv[2], sys.argv[3], sys.argv[4], sys.argv[5] or "localhost", db_type=db_type, db_port=db_port) + odb = BackupGenerator( + sys.argv[2], + sys.argv[3], + sys.argv[4], + sys.argv[5] or "localhost", + db_type=db_type, + db_port=db_port, + ) odb.get_backup() if cmd == "take_dump": - odb = BackupGenerator(sys.argv[2], sys.argv[3], sys.argv[4], sys.argv[5] or "localhost", db_type=db_type, db_port=db_port) + odb = BackupGenerator( + sys.argv[2], + sys.argv[3], + sys.argv[4], + sys.argv[5] or "localhost", + db_type=db_type, + db_port=db_port, + ) odb.take_dump() if cmd == "send_email": - odb = BackupGenerator(sys.argv[2], sys.argv[3], sys.argv[4], sys.argv[5] or "localhost", db_type=db_type, db_port=db_port) + odb = BackupGenerator( + sys.argv[2], + sys.argv[3], + sys.argv[4], + sys.argv[5] or "localhost", + db_type=db_type, + db_port=db_port, + ) odb.send_email("abc.sql.gz") if cmd == "delete_temp_backups": From bd6f52305eb1e48f370217de89608a16976e207c Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 7 Oct 2020 14:32:18 +0530 Subject: [PATCH 06/71] fix: Show traceback if verbose --- frappe/commands/site.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/frappe/commands/site.py b/frappe/commands/site.py index dc5d8b838e..446166d44c 100755 --- a/frappe/commands/site.py +++ b/frappe/commands/site.py @@ -422,6 +422,8 @@ def backup(context, with_files=False, backup_path=None, backup_path_db=None, bac ) except Exception: click.secho("Backup failed for Site {0}. Database or site_config.json may be corrupted".format(site), fg="red") + if verbose: + print(frappe.get_traceback()) exit_code = 1 continue From 7519ba5e1c81a721bc248762b28f03657b12210a Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 7 Oct 2020 14:32:40 +0530 Subject: [PATCH 07/71] fix: Backup tables correctly ? --- frappe/utils/backups.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/frappe/utils/backups.py b/frappe/utils/backups.py index b59f5526e9..5a91ad656e 100644 --- a/frappe/utils/backups.py +++ b/frappe/utils/backups.py @@ -129,9 +129,12 @@ class BackupGenerator: "include": get_tables(self.include_doctypes.strip().split(",")), "exclude": get_tables(self.exclude_doctypes.strip().split(",")), } + specified_tables = get_tables(frappe.conf.get("backup", {}).get("includes", [])) + include_tables = (specified_tables + base_tables) if specified_tables else [] + conf_tables = { - "include": get_tables(frappe.conf.backup.get("includes", [])) + base_tables, - "exclude": get_tables(frappe.conf.backup.get("excludes", [])), + "include": include_tables, + "exclude": get_tables(frappe.conf.get("backup", {}).get("excludes", [])), } self.backup_includes = passed_tables["include"] @@ -387,7 +390,7 @@ class BackupGenerator: ) if self.verbose: - print(command) + print(command + "\n") err, out = frappe.utils.execute_in_shell(command) From 7d638d298c098036fcf509d3a6086418644e4b57 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 7 Oct 2020 14:33:02 +0530 Subject: [PATCH 08/71] chore: remove old comment that lied --- frappe/utils/backups.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/frappe/utils/backups.py b/frappe/utils/backups.py index 5a91ad656e..5b1f6b0616 100644 --- a/frappe/utils/backups.py +++ b/frappe/utils/backups.py @@ -619,10 +619,6 @@ def backup( if __name__ == "__main__": - """ - is_file_old db_name user password db_host db_type db_port - get_backup db_name user password db_host db_type db_port - """ import sys cmd = sys.argv[1] From 26387401b39517f61fd50f965a5bf84c99e9817a Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 7 Oct 2020 21:06:23 +0530 Subject: [PATCH 09/71] test: Added tests for bench backup skip tables * Added utils for coloured outputs * Fixed bug during last branch update --- frappe/tests/test_commands.py | 112 +++++++++++++++++++++++++++++++++- frappe/utils/backups.py | 2 +- 2 files changed, 110 insertions(+), 4 deletions(-) diff --git a/frappe/tests/test_commands.py b/frappe/tests/test_commands.py index e7e639c775..39d76aaba5 100644 --- a/frappe/tests/test_commands.py +++ b/frappe/tests/test_commands.py @@ -1,10 +1,13 @@ # Copyright (c) 2020, Frappe Technologies Pvt. Ltd. and Contributors # imports - standard imports +import json import os import shlex import subprocess +import sys import unittest +import gzip from glob import glob # imports - module imports @@ -12,12 +15,66 @@ import frappe from frappe.utils.backups import fetch_latest_backups +# TODO: check frappe.cli.coloured_output to set coloured output! + +def supports_color(): + """ + Returns True if the running system's terminal supports color, and False + otherwise. + """ + plat = sys.platform + supported_platform = plat != 'Pocket PC' and (plat != 'win32' or 'ANSICON' in os.environ) + # isatty is not always implemented, #6223. + is_a_tty = hasattr(sys.stdout, 'isatty') and sys.stdout.isatty() + return supported_platform and is_a_tty + + +class color(dict): + nc = '\033[0m' + blue = '\033[94m' + green = '\033[92m' + yellow = '\033[93m' + red = '\033[91m' + silver = '\033[90m' + + def __getattr__(self, key): + if supports_color(): + ret = self.get(key) + else: + ret = "" + return ret + + def clean(value): - if isinstance(value, (bytes, str)): - value = value.decode().strip() + """Strips and converts bytes to str + + Args: + value ([type]): [description] + + Returns: + [type]: [description] + """ + if isinstance(value, bytes): + value = value.decode() + if isinstance(value, str): + value = value.strip() return value +def exists_in_backup(doctypes, file): + """Checks if the list of doctypes exist in the database.sql.gz file supplied + + Args: + doctypes (list): List of DocTypes to be checked + file (str): Path of the database file + + Returns: + bool: True if all tables exist + """ + with gzip.open(file, 'rb') as f: + content = f.read().decode("utf8") + return all(["CREATE TABLE `tab{}`".format(doctype).lower() in content.lower() for doctype in doctypes]) + class BaseTestCommands(unittest.TestCase): def execute(self, command, kwargs=None): site = {"site": frappe.local.site} @@ -25,7 +82,8 @@ class BaseTestCommands(unittest.TestCase): kwargs.update(site) else: kwargs = site - command = command.replace("\n", " ").format(**kwargs) + command = " ".join(command.split()).format(**kwargs) + print("{0}$ {1}{2}".format(color.silver, command, color.nc)) command = shlex.split(command) self._proc = subprocess.run(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE) self.stdout = clean(self._proc.stdout) @@ -54,6 +112,21 @@ class TestCommands(BaseTestCommands): self.assertEquals(self.stdout[1:-1], frappe.bold(text='DocType')) def test_backup(self): + backup = { + "includes": { + "includes": [ + "ToDo", + "Note", + ] + }, + "excludes": { + "excludes": [ + "Activity Log", + "Access Log", + "Error Log" + ] + } + } home = os.path.expanduser("~") site_backup_path = frappe.utils.get_site_path("private", "backups") @@ -119,3 +192,36 @@ class TestCommands(BaseTestCommands): # test 6: take a backup with --verbose self.execute("bench --site {site} backup --verbose") self.assertEquals(self.returncode, 0) + + # test 7: take a backup with frappe.conf.backup.includes + self.execute("bench --site {site} set-config backup '{includes}' --as-dict", {"includes": json.dumps(backup["includes"])}) + self.execute("bench --site {site} backup --verbose") + self.assertEquals(self.returncode, 0) + database = fetch_latest_backups()["database"] + self.assertTrue(exists_in_backup(backup["includes"]["includes"], database)) + + # test 8: take a backup with frappe.conf.backup.excludes + self.execute("bench --site {site} set-config backup '{excludes}' --as-dict", {"excludes": json.dumps(backup["excludes"])}) + self.execute("bench --site {site} backup --verbose") + self.assertEquals(self.returncode, 0) + database = fetch_latest_backups()["database"] + self.assertFalse(exists_in_backup(backup["excludes"]["excludes"], database)) + self.assertTrue(exists_in_backup(backup["includes"]["includes"], database)) + + # test 9: take a backup with --include (with frappe.conf.excludes still set) + self.execute("bench --site {site} backup --include '{include}'", {"include": ",".join(backup["includes"]["includes"])}) + self.assertEquals(self.returncode, 0) + database = fetch_latest_backups()["database"] + self.assertTrue(exists_in_backup(backup["includes"]["includes"], database)) + + # test 10: take a backup with --exclude + self.execute("bench --site {site} backup --exclude '{exclude}'", {"exclude": ",".join(backup["excludes"]["excludes"])}) + self.assertEquals(self.returncode, 0) + database = fetch_latest_backups()["database"] + self.assertFalse(exists_in_backup(backup["excludes"]["excludes"], database)) + + # test 11: take a backup with --ignore-backup-conf + self.execute("bench --site {site} backup --ignore-backup-conf") + self.assertEquals(self.returncode, 0) + database = fetch_latest_backups()["database"] + self.assertTrue(exists_in_backup(backup["excludes"]["excludes"], database)) diff --git a/frappe/utils/backups.py b/frappe/utils/backups.py index 5b1f6b0616..43dd7c17f1 100644 --- a/frappe/utils/backups.py +++ b/frappe/utils/backups.py @@ -497,7 +497,7 @@ def scheduled_backup( odb = new_backup( older_than=older_than, ignore_files=ignore_files, - backup_path=None, + backup_path=backup_path, backup_path_db=backup_path_db, backup_path_files=backup_path_files, backup_path_private_files=backup_path_private_files, From ff1f1e755da30f028ff52e7b878a3ee48905e4ca Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 8 Oct 2020 15:45:15 +0530 Subject: [PATCH 10/71] fix: Dynamic summary spacing --- frappe/utils/backups.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/frappe/utils/backups.py b/frappe/utils/backups.py index 43dd7c17f1..1ae976807a 100644 --- a/frappe/utils/backups.py +++ b/frappe/utils/backups.py @@ -302,8 +302,12 @@ class BackupGenerator: backup_summary = self.get_summary() print("Backup Summary for {0} at {1}".format(frappe.local.site, now())) + title = max([len(x) for x in backup_summary]) + path = max([len(x["path"]) for x in backup_summary.values()]) + for _type, info in backup_summary.items(): - print("{0:8}: {1:85} {2}".format(_type.title(), info["path"], info["size"])) + template = "{{0:{0}}}: {{1:{1}}} {{2}}".format(title, path) + print(template.format(_type.title(), info["path"], info["size"])) def backup_files(self): import subprocess From 104186906f3ecf8abe130c35e34c4c4ba9b21e36 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 8 Oct 2020 16:23:14 +0530 Subject: [PATCH 11/71] feat: Show command execution summary in message format --- frappe/tests/test_commands.py | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/frappe/tests/test_commands.py b/frappe/tests/test_commands.py index 39d76aaba5..b63a886a2f 100644 --- a/frappe/tests/test_commands.py +++ b/frappe/tests/test_commands.py @@ -82,14 +82,25 @@ class BaseTestCommands(unittest.TestCase): kwargs.update(site) else: kwargs = site - command = " ".join(command.split()).format(**kwargs) - print("{0}$ {1}{2}".format(color.silver, command, color.nc)) - command = shlex.split(command) + self.command = " ".join(command.split()).format(**kwargs) + print("{0}$ {1}{2}".format(color.silver, self.command, color.nc)) + command = shlex.split(self.command) self._proc = subprocess.run(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE) self.stdout = clean(self._proc.stdout) self.stderr = clean(self._proc.stderr) self.returncode = clean(self._proc.returncode) + def _formatMessage(self, msg, standardMsg): + output = super(BaseTestCommands, self)._formatMessage(msg, standardMsg) + cmd_execution_summary = "\n".join([ + "-" * 70, + "Last Command Execution Summary:", + "Command: {}".format(self.command) if self.command else "", + "Standard Output: {}".format(self.stdout) if self.stdout else "", + "Standard Error: {}".format(self.stderr) if self.stderr else "", + "Return Code: {}".format(self.returncode) if self.returncode else "", + ]).strip() + return "{}\n\n{}".format(output, cmd_execution_summary) class TestCommands(BaseTestCommands): def test_execute(self): From 44a09191b38f13682ef6f8a0ce21058ce3c25756 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 8 Oct 2020 18:56:29 +0530 Subject: [PATCH 12/71] fix: Use public schema for postgres site tx @surajshetty3416 --- frappe/tests/test_commands.py | 9 +++++++-- frappe/utils/backups.py | 2 +- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/frappe/tests/test_commands.py b/frappe/tests/test_commands.py index b63a886a2f..e7cb9f5d40 100644 --- a/frappe/tests/test_commands.py +++ b/frappe/tests/test_commands.py @@ -71,9 +71,14 @@ def exists_in_backup(doctypes, file): Returns: bool: True if all tables exist """ - with gzip.open(file, 'rb') as f: + predicate = ( + 'CREATE TABLE public."tab{}"' + if frappe.conf.db_type == "postgres" + else "CREATE TABLE `tab{}`" + ) + with gzip.open(file, "rb") as f: content = f.read().decode("utf8") - return all(["CREATE TABLE `tab{}`".format(doctype).lower() in content.lower() for doctype in doctypes]) + return all([predicate.format(doctype).lower() in content.lower() for doctype in doctypes]) class BaseTestCommands(unittest.TestCase): def execute(self, command, kwargs=None): diff --git a/frappe/utils/backups.py b/frappe/utils/backups.py index 1ae976807a..ab5783006c 100644 --- a/frappe/utils/backups.py +++ b/frappe/utils/backups.py @@ -353,7 +353,7 @@ class BackupGenerator: if self.db_type == "postgres": if self.backup_includes: args["include"] = " ".join( - ["--table='{0}'".format(table) for table in self.backup_includes] + ["--table='public.\"{0}\"'".format(table) for table in self.backup_includes] ) elif self.backup_excludes: args["exclude"] = " ".join( From 368a031da3b389bb65b859a3165cc9640bf56e0b Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 9 Oct 2020 11:19:16 +0530 Subject: [PATCH 13/71] fix: Update postgres exlcude table data --- frappe/tests/test_commands.py | 2 +- frappe/utils/backups.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/tests/test_commands.py b/frappe/tests/test_commands.py index e7cb9f5d40..19fd90e99c 100644 --- a/frappe/tests/test_commands.py +++ b/frappe/tests/test_commands.py @@ -72,7 +72,7 @@ def exists_in_backup(doctypes, file): bool: True if all tables exist """ predicate = ( - 'CREATE TABLE public."tab{}"' + 'COPY public."tab{}"' if frappe.conf.db_type == "postgres" else "CREATE TABLE `tab{}`" ) diff --git a/frappe/utils/backups.py b/frappe/utils/backups.py index ab5783006c..552debbaa1 100644 --- a/frappe/utils/backups.py +++ b/frappe/utils/backups.py @@ -357,7 +357,7 @@ class BackupGenerator: ) elif self.backup_excludes: args["exclude"] = " ".join( - ["--exclude-table='{0}'".format(table) for table in self.backup_excludes] + ["--exclude-table-data='public.\"{0}\"'".format(table) for table in self.backup_excludes] ) cmd_string = ( From 044da8a5757be170cce29a7f986ef8a1af919fac Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Fri, 30 Oct 2020 14:04:03 +0530 Subject: [PATCH 14/71] fix: create auto_repeat field if docfield/custom field does not exist --- frappe/core/doctype/doctype/doctype.py | 3 ++- frappe/custom/doctype/customize_form/customize_form.py | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index 8a9c130fbe..1d15de9ab5 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -570,7 +570,8 @@ class DocType(Document): def make_repeatable(self): """If allow_auto_repeat is set, add auto_repeat custom field.""" if self.allow_auto_repeat: - if not frappe.db.exists('Custom Field', {'fieldname': 'auto_repeat', 'dt': self.name}): + if not frappe.db.exists('Custom Field', {'fieldname': 'auto_repeat', 'dt': self.name}) and \ + not frappe.db.exists('DocField', {'fieldname': 'auto_repeat', 'parent': self.name}): insert_after = self.fields[len(self.fields) - 1].fieldname df = dict(fieldname='auto_repeat', label='Auto Repeat', fieldtype='Link', options='Auto Repeat', insert_after=insert_after, read_only=1, no_copy=1, print_hide=1) create_custom_field(self.name, df) diff --git a/frappe/custom/doctype/customize_form/customize_form.py b/frappe/custom/doctype/customize_form/customize_form.py index 61ecdd88b9..f3e67bba7b 100644 --- a/frappe/custom/doctype/customize_form/customize_form.py +++ b/frappe/custom/doctype/customize_form/customize_form.py @@ -76,8 +76,8 @@ class CustomizeForm(Document): def create_auto_repeat_custom_field_if_requried(self, meta): if self.allow_auto_repeat: - if not frappe.db.exists('Custom Field', {'fieldname': 'auto_repeat', - 'dt': self.doc_type}): + if not frappe.db.exists('Custom Field', {'fieldname': 'auto_repeat', 'dt': self.doc_type}) and \ + not frappe.db.exists('DocField', {'fieldname': 'auto_repeat', 'parent': self.name}): insert_after = self.fields[len(self.fields) - 1].fieldname df = dict( fieldname='auto_repeat', From 0245b0ff4fd03162f68ab040b4a80bd528e03a93 Mon Sep 17 00:00:00 2001 From: prssanna Date: Wed, 4 Nov 2020 16:22:16 +0530 Subject: [PATCH 15/71] fix: delete prepared reports in batches --- .../prepared_report/prepared_report.py | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/frappe/core/doctype/prepared_report/prepared_report.py b/frappe/core/doctype/prepared_report/prepared_report.py index 2c02d99dad..2c4d933440 100644 --- a/frappe/core/doctype/prepared_report/prepared_report.py +++ b/frappe/core/doctype/prepared_report/prepared_report.py @@ -89,20 +89,18 @@ def delete_expired_prepared_reports(): 'creation': ['<', frappe.utils.add_days(frappe.utils.now(), -expiry_period)] }) - args = { - 'reports': prepared_reports_to_delete, - 'limit': 50 - } - - enqueue(method=delete_prepared_reports, job_name="delete_prepared_reports", **args) + batches = frappe.utils.create_batch(prepared_reports_to_delete, 50) + for batch in batches: + args = { + 'reports': batch, + } + enqueue(method=delete_prepared_reports, job_name="delete_prepared_reports", **args) @frappe.whitelist() -def delete_prepared_reports(reports, limit=None): +def delete_prepared_reports(reports): reports = frappe.parse_json(reports) - for index, doc in enumerate(reports): - if limit and index == limit: - return - frappe.delete_doc('Prepared Report', doc['name'], ignore_permissions=True) + for report in reports: + frappe.delete_doc('Prepared Report', report['name'], ignore_permissions=True) def create_json_gz_file(data, dt, dn): # Storing data in CSV file causes information loss From 476f21eb4dfdcb93a17582b76fd052ee6c3c3d3a Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Thu, 5 Nov 2020 18:26:36 +0530 Subject: [PATCH 16/71] fix: load doc from db in get_transitions --- frappe/model/workflow.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/frappe/model/workflow.py b/frappe/model/workflow.py index 7239b202bd..72ce8c9ce4 100644 --- a/frappe/model/workflow.py +++ b/frappe/model/workflow.py @@ -29,6 +29,8 @@ def get_transitions(doc, workflow = None, raise_exception=False): if doc.is_new(): return [] + doc.load_from_db() + frappe.has_permission(doc, 'read', throw=True) roles = frappe.get_roles() From af1ed2f0bcd964fb60381fc802092b9466ce5a20 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 9 Nov 2020 18:50:03 +0530 Subject: [PATCH 17/71] refactor: Move _new_site and extract_sql_from_archive from frappe.commands.site module to frappe.installer --- frappe/commands/site.py | 73 ++----------------------- frappe/installer.py | 115 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 117 insertions(+), 71 deletions(-) diff --git a/frappe/commands/site.py b/frappe/commands/site.py index 8af0b422ba..5305502b17 100755 --- a/frappe/commands/site.py +++ b/frappe/commands/site.py @@ -9,7 +9,7 @@ import click import frappe from frappe.commands import get_site, pass_context from frappe.exceptions import SiteNotSpecifiedError -from frappe.utils import get_site_path, touch_file +from frappe.installer import _new_site @click.command('new-site') @@ -42,57 +42,6 @@ def new_site(site, mariadb_root_username=None, mariadb_root_password=None, admin if len(frappe.utils.get_sites()) == 1: use(site) -def _new_site(db_name, site, mariadb_root_username=None, mariadb_root_password=None, - admin_password=None, verbose=False, install_apps=None, source_sql=None, force=False, - no_mariadb_socket=False, reinstall=False, db_password=None, db_type=None, db_host=None, - db_port=None, new_site=False): - """Install a new Frappe site""" - - if not force and os.path.exists(site): - print('Site {0} already exists'.format(site)) - sys.exit(1) - - if no_mariadb_socket and not db_type == "mariadb": - print('--no-mariadb-socket requires db_type to be set to mariadb.') - sys.exit(1) - - if not db_name: - import hashlib - db_name = '_' + hashlib.sha1(site.encode()).hexdigest()[:16] - - from frappe.commands.scheduler import _is_scheduler_enabled - from frappe.installer import install_db, make_site_dirs - from frappe.installer import install_app as _install_app - import frappe.utils.scheduler - - frappe.init(site=site) - - try: - - # enable scheduler post install? - enable_scheduler = _is_scheduler_enabled() - except Exception: - enable_scheduler = False - - make_site_dirs() - - installing = touch_file(get_site_path('locks', 'installing.lock')) - - install_db(root_login=mariadb_root_username, root_password=mariadb_root_password, db_name=db_name, - admin_password=admin_password, verbose=verbose, source_sql=source_sql, force=force, reinstall=reinstall, - db_password=db_password, db_type=db_type, db_host=db_host, db_port=db_port, no_mariadb_socket=no_mariadb_socket) - apps_to_install = ['frappe'] + (frappe.conf.get("install_apps") or []) + (list(install_apps) or []) - for app in apps_to_install: - _install_app(app, verbose=verbose, set_as_patched=not source_sql) - - os.remove(installing) - - frappe.utils.scheduler.toggle_scheduler(enable_scheduler) - frappe.db.commit() - - scheduler_status = "disabled" if frappe.utils.scheduler.is_scheduler_disabled() else "enabled" - print("*** Scheduler is", scheduler_status, "***") - @click.command('restore') @click.argument('sql-file-path') @@ -107,25 +56,9 @@ def _new_site(db_name, site, mariadb_root_username=None, mariadb_root_password=N @pass_context def restore(context, sql_file_path, mariadb_root_username=None, mariadb_root_password=None, db_name=None, verbose=None, install_app=None, admin_password=None, force=None, with_public_files=None, with_private_files=None): "Restore site database from an sql file" - from frappe.installer import extract_sql_gzip, extract_files, is_downgrade + from frappe.installer import extract_sql_from_archive, extract_files, is_downgrade force = context.force or force - - # Extract the gzip file if user has passed *.sql.gz file instead of *.sql file - if not os.path.exists(sql_file_path): - base_path = '..' - sql_file_path = os.path.join(base_path, sql_file_path) - if not os.path.exists(sql_file_path): - print('Invalid path {0}'.format(sql_file_path[3:])) - sys.exit(1) - elif sql_file_path.startswith(os.sep): - base_path = os.sep - else: - base_path = '.' - - if sql_file_path.endswith('sql.gz'): - decompressed_file_name = extract_sql_gzip(os.path.abspath(sql_file_path)) - else: - decompressed_file_name = sql_file_path + decompressed_file_name = extract_sql_from_archive(sql_file_path) site = get_site(context) frappe.init(site=site) diff --git a/frappe/installer.py b/frappe/installer.py index df767a3294..b1420e5d9f 100755 --- a/frappe/installer.py +++ b/frappe/installer.py @@ -3,8 +3,90 @@ import json import os -from frappe.defaults import _clear_cache +import sys + import frappe +from frappe.defaults import _clear_cache + + +def _new_site( + db_name, + site, + mariadb_root_username=None, + mariadb_root_password=None, + admin_password=None, + verbose=False, + install_apps=None, + source_sql=None, + force=False, + no_mariadb_socket=False, + reinstall=False, + db_password=None, + db_type=None, + db_host=None, + db_port=None, + new_site=False, +): + """Install a new Frappe site""" + + if not force and os.path.exists(site): + print("Site {0} already exists".format(site)) + sys.exit(1) + + if no_mariadb_socket and not db_type == "mariadb": + print("--no-mariadb-socket requires db_type to be set to mariadb.") + sys.exit(1) + + if not db_name: + import hashlib + db_name = "_" + hashlib.sha1(site.encode()).hexdigest()[:16] + + frappe.init(site=site) + + from frappe.commands.scheduler import _is_scheduler_enabled + from frappe.utils import get_site_path, scheduler, touch_file + + try: + # enable scheduler post install? + enable_scheduler = _is_scheduler_enabled() + except Exception: + enable_scheduler = False + + make_site_dirs() + + installing = touch_file(get_site_path("locks", "installing.lock")) + + install_db( + root_login=mariadb_root_username, + root_password=mariadb_root_password, + db_name=db_name, + admin_password=admin_password, + verbose=verbose, + source_sql=source_sql, + force=force, + reinstall=reinstall, + db_password=db_password, + db_type=db_type, + db_host=db_host, + db_port=db_port, + no_mariadb_socket=no_mariadb_socket, + ) + apps_to_install = ( + ["frappe"] + (frappe.conf.get("install_apps") or []) + (list(install_apps) or []) + ) + + for app in apps_to_install: + install_app(app, verbose=verbose, set_as_patched=not source_sql) + + os.remove(installing) + + scheduler.toggle_scheduler(enable_scheduler) + frappe.db.commit() + + scheduler_status = ( + "disabled" if frappe.utils.scheduler.is_scheduler_disabled() else "enabled" + ) + print("*** Scheduler is", scheduler_status, "***") def install_db(root_login="root", root_password=None, db_name=None, source_sql=None, @@ -331,6 +413,37 @@ def remove_missing_apps(): frappe.db.set_global("installed_apps", json.dumps(installed_apps)) +def extract_sql_from_archive(sql_file_path): + """Return the path of an SQL file if the passed argument is the path of a gzipped + SQL file or an SQL file path. The path may be absolute or relative from the bench + root directory or the sites sub-directory. + + Args: + sql_file_path (str): Path of the SQL file + + Returns: + str: Path of the decompressed SQL file + """ + # Extract the gzip file if user has passed *.sql.gz file instead of *.sql file + if not os.path.exists(sql_file_path): + base_path = '..' + sql_file_path = os.path.join(base_path, sql_file_path) + if not os.path.exists(sql_file_path): + print('Invalid path {0}'.format(sql_file_path[3:])) + sys.exit(1) + elif sql_file_path.startswith(os.sep): + base_path = os.sep + else: + base_path = '.' + + if sql_file_path.endswith('sql.gz'): + decompressed_file_name = extract_sql_gzip(os.path.abspath(sql_file_path)) + else: + decompressed_file_name = sql_file_path + + return decompressed_file_name + + def extract_sql_gzip(sql_gz_path): import subprocess From 7b1fa59a29ec59535976a53bd10528fe80f6e18c Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 9 Nov 2020 18:51:33 +0530 Subject: [PATCH 18/71] feat: Restore partial backups via bench partial-restore --- frappe/commands/site.py | 17 ++++++++++++++++- frappe/database/db_manager.py | 1 - frappe/installer.py | 23 +++++++++++++++++++++++ 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/frappe/commands/site.py b/frappe/commands/site.py index 5305502b17..fd247b4182 100755 --- a/frappe/commands/site.py +++ b/frappe/commands/site.py @@ -91,6 +91,20 @@ def restore(context, sql_file_path, mariadb_root_username=None, mariadb_root_pas success_message = "Site {0} has been restored{1}".format(site, " with files" if (with_public_files or with_private_files) else "") click.secho(success_message, fg="green") +@click.command('partial-restore') +@click.argument('sql-file-path') +@click.option("--verbose", "-v", is_flag=True) +@pass_context +def partial_restore(context, sql_file_path, verbose): + from frappe.installer import partial_restore + verbose = context.verbose or verbose + + site = get_site(context) + frappe.init(site=site) + frappe.connect(site=site) + partial_restore(sql_file_path, verbose) + frappe.destroy() + @click.command('reinstall') @click.option('--admin-password', help='Administrator Password for reinstalled site') @click.option('--mariadb-root-username', help='Root username for MariaDB') @@ -650,5 +664,6 @@ commands = [ stop_recording, add_to_hosts, start_ngrok, - build_search_index + build_search_index, + partial_restore ] diff --git a/frappe/database/db_manager.py b/frappe/database/db_manager.py index 3345fce735..da1f584f57 100644 --- a/frappe/database/db_manager.py +++ b/frappe/database/db_manager.py @@ -3,7 +3,6 @@ import frappe class DbManager: - def __init__(self, db): """ Pass root_conn here for access to all databases. diff --git a/frappe/installer.py b/frappe/installer.py index b1420e5d9f..2bbf0421e3 100755 --- a/frappe/installer.py +++ b/frappe/installer.py @@ -519,3 +519,26 @@ def is_downgrade(sql_file_path, verbose=False): print("Your site will be downgraded from Frappe {0} to {1}".format(current_version, backup_version)) return downgrade + + +def partial_restore(sql_file_path, verbose=False): + sql_file = extract_sql_from_archive(sql_file_path) + + if frappe.conf.db_type in (None, "mariadb"): + from frappe.database.mariadb.setup_db import import_db_from_sql + elif frappe.conf.db_type == "postgres": + from frappe.database.postgres.setup_db import import_db_from_sql + import warnings + from click import style + warn = style( + "Delete the tables you want to restore manually before attempting" + " partial restore operation for PostreSQL databases", + fg="yellow" + ) + warnings.warn(warn) + + import_db_from_sql(source_sql=sql_file, verbose=verbose) + + # Removing temporarily created file + if sql_file != sql_file_path: + os.remove(sql_file) From ce45343355a4f429e772c83636fe7f2c438345e1 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 9 Nov 2020 18:53:14 +0530 Subject: [PATCH 19/71] feat: Add aliases for bench backup * --include is the same as --only, -i * --exclude is the same as -e --- frappe/commands/site.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/commands/site.py b/frappe/commands/site.py index fd247b4182..646984dc8b 100755 --- a/frappe/commands/site.py +++ b/frappe/commands/site.py @@ -325,8 +325,8 @@ def use(site, sites_path='.'): @click.command('backup') @click.option('--with-files', default=False, is_flag=True, help="Take backup with files") -@click.option('--include', default="", type=str, help="Specify the DocTypes to backup seperated by commas") -@click.option('--exclude', default="", type=str, help="Specify the DocTypes to not backup seperated by commas") +@click.option('--include', '--only', '-i', default="", type=str, help="Specify the DocTypes to backup seperated by commas") +@click.option('--exclude', '-e', default="", type=str, help="Specify the DocTypes to not backup seperated by commas") @click.option('--backup-path', default=None, help="Set path for saving all the files in this operation") @click.option('--backup-path-db', default=None, help="Set path for saving database file") @click.option('--backup-path-files', default=None, help="Set path for saving public file") From b371c7005359e202c68adc4a2abd09030f5366e2 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 10 Nov 2020 11:11:19 +0530 Subject: [PATCH 20/71] refactor: Meaningful variable names --- frappe/database/db_manager.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/frappe/database/db_manager.py b/frappe/database/db_manager.py index da1f584f57..b8ffae519b 100644 --- a/frappe/database/db_manager.py +++ b/frappe/database/db_manager.py @@ -65,10 +65,10 @@ class DbManager: esc = make_esc('$ ') from distutils.spawn import find_executable - pipe = find_executable('pv') - if pipe: - pipe = '{pipe} {source} |'.format( - pipe=pipe, + pv = find_executable('pv') + if pv: + pipe = '{pv} {source} |'.format( + pv=pv, source=source ) source = '' @@ -77,7 +77,7 @@ class DbManager: source = '< {source}'.format(source=source) if pipe: - print('Creating Database...') + print('Restoring Database file...') command = '{pipe} mysql -u {user} -p{password} -h{host} ' + ('-P{port}' if frappe.db.port else '') + ' {target} {source}' command = command.format( From e8fdaa195bf3fe276be58c455d4fc3bad56970a8 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 10 Nov 2020 09:11:19 +0530 Subject: [PATCH 21/71] refactor: PostgreSQL module methods correspond to MariaDB * Added bootstrap_database, import_db_from_sql function APIs similar to MariaDB implementations --- frappe/database/postgres/setup_db.py | 29 +++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/frappe/database/postgres/setup_db.py b/frappe/database/postgres/setup_db.py index f53872db82..6f2ba7a1b7 100644 --- a/frappe/database/postgres/setup_db.py +++ b/frappe/database/postgres/setup_db.py @@ -10,6 +10,23 @@ def setup_database(force, source_sql=None, verbose=False): root_conn.sql("CREATE user {0} password '{1}'".format(frappe.conf.db_name, frappe.conf.db_password)) root_conn.sql("GRANT ALL PRIVILEGES ON DATABASE `{0}` TO {0}".format(frappe.conf.db_name)) + root_conn.close() + + bootstrap_database(frappe.conf.db_name, verbose, source_sql=source_sql) + frappe.connect() + +def bootstrap_database(db_name, verbose, source_sql=None): + frappe.connect(db_name=db_name) + import_db_from_sql(source_sql, verbose) + frappe.connect(db_name=db_name) + if not 'tabDefaultValue' in frappe.db.get_tables(): + print('''Database not installed, this can due to lack of permission, or that the database name exists. + Check your mysql root password, or use --force to reinstall''') + sys.exit(1) + +def import_db_from_sql(source_sql=None, verbose=False): + if verbose: + print("Starting Database Import...") # we can't pass psql password in arguments in postgresql as mysql. So # set password connection parameter in environment variable @@ -19,15 +36,21 @@ def setup_database(force, source_sql=None, verbose=False): if not source_sql: source_sql = os.path.join(os.path.dirname(__file__), 'framework_postgres.sql') - subprocess.check_output([ + command = [ 'psql', frappe.conf.db_name, '-h', frappe.conf.db_host or 'localhost', '-p', str(frappe.conf.db_port or '5432'), '-U', frappe.conf.db_name, '-f', source_sql - ], env=subprocess_env) + ] - frappe.connect() + if verbose: + print(" ".join(command)) + + subprocess.check_output(command, env=subprocess_env) + + if verbose: + print(f"Imported from Database File: {source_sql}") def setup_help_database(help_db_name): root_conn = get_root_connection() From 19f87c36e51d57c2d7b96d20b43e919c7d4289e1 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 9 Nov 2020 18:57:21 +0530 Subject: [PATCH 22/71] fix: Marked is_downgrade function as only MariaDB compatible --- frappe/installer.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/frappe/installer.py b/frappe/installer.py index 2bbf0421e3..b73f3f1d6e 100755 --- a/frappe/installer.py +++ b/frappe/installer.py @@ -118,9 +118,9 @@ def install_db(root_login="root", root_password=None, db_name=None, source_sql=N def install_app(name, verbose=False, set_as_patched=True): from frappe.core.doctype.scheduled_job_type.scheduled_job_type import sync_jobs - from frappe.utils.fixtures import sync_fixtures from frappe.model.sync import sync_for from frappe.modules.utils import sync_customizations + from frappe.utils.fixtures import sync_fixtures frappe.flags.in_install = name frappe.flags.ignore_in_install = False @@ -458,9 +458,10 @@ def extract_sql_gzip(sql_gz_path): return decompressed_file + def extract_files(site_name, file_path, folder_name): - import subprocess import shutil + import subprocess # Need to do frappe.init to maintain the site locals frappe.init(site=site_name) @@ -488,6 +489,12 @@ def extract_files(site_name, file_path, folder_name): def is_downgrade(sql_file_path, verbose=False): """checks if input db backup will get downgraded on current bench""" + + # This function is only tested with mariadb + # TODO: Add postgres support + if frappe.conf.db_type not in (None, "mariadb"): + return False + from semantic_version import Version head = "INSERT INTO `tabInstalled Application` VALUES" From 142b6009fe0d2b609a9f0c40a4404a7dfb16ac72 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 10 Nov 2020 10:42:03 +0530 Subject: [PATCH 23/71] fix: Better error message on missing table --- frappe/database/mariadb/setup_db.py | 18 ++++++++++++++---- frappe/database/postgres/setup_db.py | 13 ++++++++++--- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/frappe/database/mariadb/setup_db.py b/frappe/database/mariadb/setup_db.py index a4e4d624ae..9b73d77171 100644 --- a/frappe/database/mariadb/setup_db.py +++ b/frappe/database/mariadb/setup_db.py @@ -1,7 +1,7 @@ from __future__ import unicode_literals import frappe -import os, sys +import os from frappe.database.db_manager import DbManager expected_settings_10_2_earlier = { @@ -86,6 +86,8 @@ def drop_user_and_database(db_name, root_login, root_password): dbman.drop_database(db_name) def bootstrap_database(db_name, verbose, source_sql=None): + import sys + frappe.connect(db_name=db_name) if not check_database_settings(): print('Database settings do not match expected values; stopping database setup.') @@ -94,9 +96,17 @@ def bootstrap_database(db_name, verbose, source_sql=None): import_db_from_sql(source_sql, verbose) frappe.connect(db_name=db_name) - if not 'tabDefaultValue' in frappe.db.get_tables(): - print('''Database not installed, this can due to lack of permission, or that the database name exists. - Check your mysql root password, or use --force to reinstall''') + if 'tabDefaultValue' not in frappe.db.get_tables(): + from click import secho + + secho( + "Table 'tabDefaultValue' missing in the restored site. " + "Database not installed correctly, this can due to lack of " + "permission, or that the database name exists. Check your mysql" + " root password, validity of the backup file or use --force to" + " reinstall", + fg="red" + ) sys.exit(1) def import_db_from_sql(source_sql=None, verbose=False): diff --git a/frappe/database/postgres/setup_db.py b/frappe/database/postgres/setup_db.py index 6f2ba7a1b7..109ed0469e 100644 --- a/frappe/database/postgres/setup_db.py +++ b/frappe/database/postgres/setup_db.py @@ -19,9 +19,16 @@ def bootstrap_database(db_name, verbose, source_sql=None): frappe.connect(db_name=db_name) import_db_from_sql(source_sql, verbose) frappe.connect(db_name=db_name) - if not 'tabDefaultValue' in frappe.db.get_tables(): - print('''Database not installed, this can due to lack of permission, or that the database name exists. - Check your mysql root password, or use --force to reinstall''') + if 'tabDefaultValue' not in frappe.db.get_tables(): + import sys + from click import secho + + secho( + "Table 'tabDefaultValue' missing in the restored site. " + "This may be due to incorrect permissions or the result of a restore from a bad backup file. " + "Database not installed correctly.", + fg="red" + ) sys.exit(1) def import_db_from_sql(source_sql=None, verbose=False): From 853acf6dd05176729ed033ec36658557ea31ce76 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 10 Nov 2020 10:51:23 +0530 Subject: [PATCH 24/71] fix: Add "partial" tag in the backup file following site name to indicate its nature --- frappe/utils/backups.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/frappe/utils/backups.py b/frappe/utils/backups.py index 552debbaa1..7856c8d953 100644 --- a/frappe/utils/backups.py +++ b/frappe/utils/backups.py @@ -65,6 +65,7 @@ class BackupGenerator: self.ignore_conf = ignore_conf self.include_doctypes = include_doctypes self.exclude_doctypes = exclude_doctypes + self.partial = False if not self.db_type: self.db_type = "mariadb" @@ -144,6 +145,8 @@ class BackupGenerator: self.backup_includes = self.backup_includes or conf_tables["include"] self.backup_excludes = self.backup_excludes or conf_tables["exclude"] + self.partial = (self.backup_includes or self.backup_excludes) and not self.ignore_conf + @property def site_config_backup_path(self): # For backwards compatibility @@ -199,7 +202,10 @@ class BackupGenerator: self.backup_path_conf = site_config_backup_path def set_backup_file_name(self): - # Generate a random name using today's date and a 8 digit random number + if self.partial: + # slugs postfixed with partial won't get returned by get_recent_backup + self.site_slug = self.site_slug + "-partial" + for_conf = self.todays_date + "-" + self.site_slug + "-site_config_backup.json" for_db = self.todays_date + "-" + self.site_slug + "-database.sql.gz" ext = "tgz" if self.compress_files else "tar" From b4e17b9f95f65900b9c287d8a14985fba114065b Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 10 Nov 2020 10:52:10 +0530 Subject: [PATCH 25/71] fix: Give more information about file to match verbosity --- frappe/utils/backups.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/frappe/utils/backups.py b/frappe/utils/backups.py index 7856c8d953..f44bcd40e8 100644 --- a/frappe/utils/backups.py +++ b/frappe/utils/backups.py @@ -573,29 +573,29 @@ def delete_temp_backups(older_than=24): os.remove(this_file_path) -def is_file_old(db_file_name, older_than=24): +def is_file_old(file_path, older_than=24): """ Checks if file exists and is older than specified hours Returns -> True: file does not exist or file is old False: file is new """ - if os.path.isfile(db_file_name): + if os.path.isfile(file_path): from datetime import timedelta # Get timestamp of the file - file_datetime = datetime.fromtimestamp(os.stat(db_file_name).st_ctime) + file_datetime = datetime.fromtimestamp(os.stat(file_path).st_ctime) if datetime.today() - file_datetime >= timedelta(hours=older_than): if _verbose: - print("File is old") + print(f"File {file_path} is older than {older_than} hours") return True else: if _verbose: - print("File is recent") + print(f"File {file_path} is recent") return False else: if _verbose: - print("File does not exist") + print(f"File {file_path} does not exist") return True From a073a595448a2459e3d220e6406a46291f462daa Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 11 Nov 2020 09:11:19 +0530 Subject: [PATCH 26/71] fix: Add partial slug only for database file --- frappe/utils/backups.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/frappe/utils/backups.py b/frappe/utils/backups.py index f44bcd40e8..e2fe4c559a 100644 --- a/frappe/utils/backups.py +++ b/frappe/utils/backups.py @@ -202,16 +202,14 @@ class BackupGenerator: self.backup_path_conf = site_config_backup_path def set_backup_file_name(self): - if self.partial: - # slugs postfixed with partial won't get returned by get_recent_backup - self.site_slug = self.site_slug + "-partial" - - for_conf = self.todays_date + "-" + self.site_slug + "-site_config_backup.json" - for_db = self.todays_date + "-" + self.site_slug + "-database.sql.gz" + # backups with the partial tag won't get returned by get_recent_backup + partial = "-partial" if self.partial else "" ext = "tgz" if self.compress_files else "tar" - for_public_files = self.todays_date + "-" + self.site_slug + "-files." + ext - for_private_files = self.todays_date + "-" + self.site_slug + "-private-files." + ext + for_conf = f"{self.todays_date}-{self.site_slug}-site_config_backup.json" + for_db = f"{self.todays_date}-{self.site_slug}{partial}-database.sql.gz" + for_public_files = f"{self.todays_date}-{self.site_slug}-files.{ext}" + for_private_files = f"{self.todays_date}-{self.site_slug}-private-files.{ext}" backup_path = self.backup_path or get_backup_path() if not self.backup_path_conf: From 3fed5c72553e2404123beb63ba10405cbf934536 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 10 Nov 2020 13:11:34 +0530 Subject: [PATCH 27/71] test: Add tests for bench partial-restore --- frappe/tests/test_commands.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/frappe/tests/test_commands.py b/frappe/tests/test_commands.py index 283f9c01a7..f5cdd6b775 100644 --- a/frappe/tests/test_commands.py +++ b/frappe/tests/test_commands.py @@ -12,6 +12,7 @@ from glob import glob # imports - module imports import frappe +from frappe.utils import add_to_date, now from frappe.utils.backups import fetch_latest_backups import frappe.recorder @@ -243,6 +244,30 @@ class TestCommands(BaseTestCommands): database = fetch_latest_backups()["database"] self.assertTrue(exists_in_backup(backup["excludes"]["excludes"], database)) + def test_partial_restore(self): + _now = now() + for num in range(10): + frappe.get_doc({ + "doctype": "ToDo", + "date": add_to_date(_now, days=num), + "description": frappe.mock("paragraph") + }).insert() + todo_count = frappe.db.count("ToDo") + + # check if todos exist, create a partial backup and see if the state is the same after restore + self.assertIsNot(todo_count, 0) + self.execute("bench --site {site} backup --only 'ToDo'") + db_path = fetch_latest_backups(partial=True)["database"] + self.assertTrue("partial" in db_path) + + frappe.db.sql_ddl("DROP TABLE IF EXISTS `tabToDo`") + frappe.db.commit() + + self.execute("bench --site {site} partial-restore {path}", {"path": db_path}) + self.assertEquals(self.returncode, 0) + frappe.db.commit() + self.assertEquals(frappe.db.count("ToDo"), todo_count) + def test_recorder(self): frappe.recorder.stop() From 34af8cb32601993c069a862f1902aac53d848e69 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 12 Nov 2020 09:11:19 +0530 Subject: [PATCH 28/71] fix: Show partial backups when flag set * in fetch_latest_backups whitelisted API * BackupGenerator.get_recent_backup --- frappe/tests/test_commands.py | 8 ++++---- frappe/utils/backups.py | 9 ++++----- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/frappe/tests/test_commands.py b/frappe/tests/test_commands.py index f5cdd6b775..1063307b76 100644 --- a/frappe/tests/test_commands.py +++ b/frappe/tests/test_commands.py @@ -215,27 +215,27 @@ class TestCommands(BaseTestCommands): self.execute("bench --site {site} set-config backup '{includes}' --as-dict", {"includes": json.dumps(backup["includes"])}) self.execute("bench --site {site} backup --verbose") self.assertEquals(self.returncode, 0) - database = fetch_latest_backups()["database"] + database = fetch_latest_backups(partial=True)["database"] self.assertTrue(exists_in_backup(backup["includes"]["includes"], database)) # test 8: take a backup with frappe.conf.backup.excludes self.execute("bench --site {site} set-config backup '{excludes}' --as-dict", {"excludes": json.dumps(backup["excludes"])}) self.execute("bench --site {site} backup --verbose") self.assertEquals(self.returncode, 0) - database = fetch_latest_backups()["database"] + database = fetch_latest_backups(partial=True)["database"] self.assertFalse(exists_in_backup(backup["excludes"]["excludes"], database)) self.assertTrue(exists_in_backup(backup["includes"]["includes"], database)) # test 9: take a backup with --include (with frappe.conf.excludes still set) self.execute("bench --site {site} backup --include '{include}'", {"include": ",".join(backup["includes"]["includes"])}) self.assertEquals(self.returncode, 0) - database = fetch_latest_backups()["database"] + database = fetch_latest_backups(partial=True)["database"] self.assertTrue(exists_in_backup(backup["includes"]["includes"], database)) # test 10: take a backup with --exclude self.execute("bench --site {site} backup --exclude '{exclude}'", {"exclude": ",".join(backup["excludes"]["excludes"])}) self.assertEquals(self.returncode, 0) - database = fetch_latest_backups()["database"] + database = fetch_latest_backups(partial=True)["database"] self.assertFalse(exists_in_backup(backup["excludes"]["excludes"], database)) # test 11: take a backup with --ignore-backup-conf diff --git a/frappe/utils/backups.py b/frappe/utils/backups.py index e2fe4c559a..178d213e59 100644 --- a/frappe/utils/backups.py +++ b/frappe/utils/backups.py @@ -202,7 +202,6 @@ class BackupGenerator: self.backup_path_conf = site_config_backup_path def set_backup_file_name(self): - # backups with the partial tag won't get returned by get_recent_backup partial = "-partial" if self.partial else "" ext = "tgz" if self.compress_files else "tar" @@ -221,11 +220,11 @@ class BackupGenerator: if not self.backup_path_private_files: self.backup_path_private_files = os.path.join(backup_path, for_private_files) - def get_recent_backup(self, older_than): + def get_recent_backup(self, older_than, partial=False): backup_path = get_backup_path() file_type_slugs = { - "database": "*-{}-database.sql.gz", + "database": "*-{{}}-{}database.sql.gz".format('*' if partial else ''), "public": "*-{}-files.tar", "private": "*-{}-private-files.tar", "config": "*-{}-site_config_backup.json", @@ -463,7 +462,7 @@ def get_backup(): @frappe.whitelist() -def fetch_latest_backups(): +def fetch_latest_backups(partial=False): """Fetches paths of the latest backup taken in the last 30 days Only for: System Managers @@ -479,7 +478,7 @@ def fetch_latest_backups(): db_type=frappe.conf.db_type, db_port=frappe.conf.db_port, ) - database, public, private, config = odb.get_recent_backup(older_than=24 * 30) + database, public, private, config = odb.get_recent_backup(older_than=24 * 30, partial=partial) return {"database": database, "public": public, "private": private, "config": config} From a22cd461ac85764477a39dc1e5ed613125b4e0bf Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 10 Nov 2020 13:34:48 +0530 Subject: [PATCH 29/71] test: Commit after insert, not before count check --- frappe/tests/test_commands.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/tests/test_commands.py b/frappe/tests/test_commands.py index 1063307b76..d19a62d788 100644 --- a/frappe/tests/test_commands.py +++ b/frappe/tests/test_commands.py @@ -252,6 +252,7 @@ class TestCommands(BaseTestCommands): "date": add_to_date(_now, days=num), "description": frappe.mock("paragraph") }).insert() + frappe.db.commit() todo_count = frappe.db.count("ToDo") # check if todos exist, create a partial backup and see if the state is the same after restore @@ -265,7 +266,6 @@ class TestCommands(BaseTestCommands): self.execute("bench --site {site} partial-restore {path}", {"path": db_path}) self.assertEquals(self.returncode, 0) - frappe.db.commit() self.assertEquals(frappe.db.count("ToDo"), todo_count) def test_recorder(self): From a8427c735e97501416b565f1493928a0828c6b67 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 10 Nov 2020 18:01:16 +0530 Subject: [PATCH 30/71] fix: Dont take backup in dry run + other fixes --- frappe/installer.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/frappe/installer.py b/frappe/installer.py index 51113beae8..9807421e98 100755 --- a/frappe/installer.py +++ b/frappe/installer.py @@ -135,7 +135,7 @@ def remove_app(app_name, dry_run=False, yes=False, no_backup=False, force=False) if not confirm: return - if not no_backup: + if not (dry_run or no_backup): from frappe.utils.backups import scheduled_backup print("Backing up...") scheduled_backup(ignore_files=True) @@ -173,13 +173,15 @@ def remove_app(app_name, dry_run=False, yes=False, no_backup=False, force=False) if not dry_run: remove_from_installed_apps(app_name) - for doctype in set(drop_doctypes): - print("* dropping Table for '{0}'...".format(doctype)) + for doctype in set(drop_doctypes): + print("* dropping Table for '{0}'...".format(doctype)) + if not dry_run: frappe.db.sql_ddl("drop table `tab{0}`".format(doctype)) + if not dry_run: frappe.db.commit() - click.secho("Uninstalled App {0} from Site {1}".format(app_name, frappe.local.site), fg="green") + click.secho("Uninstalled App {0} from Site {1}".format(app_name, frappe.local.site), fg="green") frappe.flags.in_uninstall = False From b3a487242ff52feb6b30e94750b64d32171772f7 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 10 Nov 2020 18:04:44 +0530 Subject: [PATCH 31/71] fix: Use get_all instead of get_list --- frappe/installer.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/installer.py b/frappe/installer.py index 9807421e98..dd03783869 100755 --- a/frappe/installer.py +++ b/frappe/installer.py @@ -147,7 +147,7 @@ def remove_app(app_name, dry_run=False, yes=False, no_backup=False, force=False) for module_name in modules: print("Deleting Module '{0}'".format(module_name)) - for doctype in frappe.get_list("DocType", filters={"module": module_name}, fields=["name", "issingle"]): + for doctype in frappe.get_all("DocType", filters={"module": module_name}, fields=["name", "issingle"]): print("* removing DocType '{0}'...".format(doctype.name)) if not dry_run: @@ -161,7 +161,7 @@ def remove_app(app_name, dry_run=False, yes=False, no_backup=False, force=False) doctypes_with_linked_modules = ordered_doctypes + [doctype.parent for doctype in linked_doctypes if doctype.parent not in ordered_doctypes] for doctype in doctypes_with_linked_modules: - for record in frappe.get_list(doctype, filters={"module": module_name}): + for record in frappe.get_all(doctype, filters={"module": module_name}): print("* removing {0} '{1}'...".format(doctype, record.name)) if not dry_run: frappe.delete_doc(doctype, record.name) From 5facf0fd1c45f812f7224d852e11ad63bbeff8c8 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 10 Nov 2020 18:17:54 +0530 Subject: [PATCH 32/71] style: Use f-strings, pluck and Black --- frappe/installer.py | 49 +++++++++++++++++++++++++++++---------------- 1 file changed, 32 insertions(+), 17 deletions(-) diff --git a/frappe/installer.py b/frappe/installer.py index dd03783869..7f630cbe57 100755 --- a/frappe/installer.py +++ b/frappe/installer.py @@ -121,34 +121,41 @@ def remove_from_installed_apps(app_name): def remove_app(app_name, dry_run=False, yes=False, no_backup=False, force=False): """Remove app and all linked to the app's module with the app from a site.""" import click + site = frappe.local.site # dont allow uninstall app if not installed unless forced if not force: if app_name not in frappe.get_installed_apps(): - click.secho("App {0} not installed on Site {1}".format(app_name, frappe.local.site), fg="yellow") + click.secho(f"App {app_name} not installed on Site {site}", fg="yellow") return - print("Uninstalling App {0} from Site {1}...".format(app_name, frappe.local.site)) + print(f"Uninstalling App {app_name} from Site {site}...") if not dry_run and not yes: - confirm = click.confirm("All doctypes (including custom), modules related to this app will be deleted. Are you sure you want to continue?") + confirm = click.confirm( + "All doctypes (including custom), modules related to this app will be" + " deleted. Are you sure you want to continue?" + ) if not confirm: return if not (dry_run or no_backup): from frappe.utils.backups import scheduled_backup + print("Backing up...") scheduled_backup(ignore_files=True) frappe.flags.in_uninstall = True drop_doctypes = [] - modules = (x.name for x in frappe.get_all("Module Def", filters={"app_name": app_name})) + modules = frappe.get_all("Module Def", filters={"app_name": app_name}, pluck="name") for module_name in modules: - print("Deleting Module '{0}'".format(module_name)) + print(f"Deleting Module '{module_name}'") - for doctype in frappe.get_all("DocType", filters={"module": module_name}, fields=["name", "issingle"]): - print("* removing DocType '{0}'...".format(doctype.name)) + for doctype in frappe.get_all( + "DocType", filters={"module": module_name}, fields=["name", "issingle"] + ): + print(f"* removing DocType '{doctype.name}'...") if not dry_run: frappe.delete_doc("DocType", doctype.name) @@ -156,17 +163,25 @@ def remove_app(app_name, dry_run=False, yes=False, no_backup=False, force=False) if not doctype.issingle: drop_doctypes.append(doctype.name) - linked_doctypes = frappe.get_all("DocField", filters={"fieldtype": "Link", "options": "Module Def"}, fields=['parent']) + linked_doctypes = frappe.get_all( + "DocField", filters={"fieldtype": "Link", "options": "Module Def"}, fields=["parent"] + ) ordered_doctypes = ["Desk Page", "Report", "Page", "Web Form"] - doctypes_with_linked_modules = ordered_doctypes + [doctype.parent for doctype in linked_doctypes if doctype.parent not in ordered_doctypes] - + all_doctypes_with_linked_modules = ordered_doctypes + [ + doctype.parent + for doctype in linked_doctypes + if doctype.parent not in ordered_doctypes + ] + doctypes_with_linked_modules = [ + x for x in all_doctypes_with_linked_modules if frappe.db.exists("DocType", x) + ] for doctype in doctypes_with_linked_modules: - for record in frappe.get_all(doctype, filters={"module": module_name}): - print("* removing {0} '{1}'...".format(doctype, record.name)) + for record in frappe.get_all(doctype, filters={"module": module_name}, pluck="name"): + print(f"* removing {doctype} '{record}'...") if not dry_run: - frappe.delete_doc(doctype, record.name) + frappe.delete_doc(doctype, record) - print("* removing Module Def '{0}'...".format(module_name)) + print(f"* removing Module Def '{module_name}'...") if not dry_run: frappe.delete_doc("Module Def", module_name) @@ -174,14 +189,14 @@ def remove_app(app_name, dry_run=False, yes=False, no_backup=False, force=False) remove_from_installed_apps(app_name) for doctype in set(drop_doctypes): - print("* dropping Table for '{0}'...".format(doctype)) + print(f"* dropping Table for '{doctype}'...") if not dry_run: - frappe.db.sql_ddl("drop table `tab{0}`".format(doctype)) + frappe.db.sql_ddl(f"drop table `tab{doctype}`") if not dry_run: frappe.db.commit() - click.secho("Uninstalled App {0} from Site {1}".format(app_name, frappe.local.site), fg="green") + click.secho(f"Uninstalled App {app_name} from Site {site}", fg="green") frappe.flags.in_uninstall = False From d28fb7ff5e7c74a516a9c2bf5ac44fb1535165cc Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 10 Nov 2020 18:37:21 +0530 Subject: [PATCH 33/71] fix: ignore on_trash, delete comment on dt --- frappe/installer.py | 11 +++++------ frappe/model/delete_doc.py | 14 ++++++++++---- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/frappe/installer.py b/frappe/installer.py index 7f630cbe57..ac411e2667 100755 --- a/frappe/installer.py +++ b/frappe/installer.py @@ -121,6 +121,7 @@ def remove_from_installed_apps(app_name): def remove_app(app_name, dry_run=False, yes=False, no_backup=False, force=False): """Remove app and all linked to the app's module with the app from a site.""" import click + site = frappe.local.site # dont allow uninstall app if not installed unless forced @@ -158,7 +159,7 @@ def remove_app(app_name, dry_run=False, yes=False, no_backup=False, force=False) print(f"* removing DocType '{doctype.name}'...") if not dry_run: - frappe.delete_doc("DocType", doctype.name) + frappe.delete_doc("DocType", doctype.name, ignore_on_trash=True) if not doctype.issingle: drop_doctypes.append(doctype.name) @@ -179,14 +180,11 @@ def remove_app(app_name, dry_run=False, yes=False, no_backup=False, force=False) for record in frappe.get_all(doctype, filters={"module": module_name}, pluck="name"): print(f"* removing {doctype} '{record}'...") if not dry_run: - frappe.delete_doc(doctype, record) + frappe.delete_doc(doctype, record, ignore_on_trash=True) print(f"* removing Module Def '{module_name}'...") if not dry_run: - frappe.delete_doc("Module Def", module_name) - - if not dry_run: - remove_from_installed_apps(app_name) + frappe.delete_doc("Module Def", module_name, ignore_on_trash=True) for doctype in set(drop_doctypes): print(f"* dropping Table for '{doctype}'...") @@ -194,6 +192,7 @@ def remove_app(app_name, dry_run=False, yes=False, no_backup=False, force=False) frappe.db.sql_ddl(f"drop table `tab{doctype}`") if not dry_run: + remove_from_installed_apps(app_name) frappe.db.commit() click.secho(f"Uninstalled App {app_name} from Site {site}", fg="green") diff --git a/frappe/model/delete_doc.py b/frappe/model/delete_doc.py index a38470e3f5..862abe375c 100644 --- a/frappe/model/delete_doc.py +++ b/frappe/model/delete_doc.py @@ -335,19 +335,25 @@ def clear_timeline_references(link_doctype, link_name): WHERE `tabCommunication Link`.link_doctype=%s AND `tabCommunication Link`.link_name=%s""", (link_doctype, link_name)) def insert_feed(doc): - from frappe.utils import get_fullname - - if frappe.flags.in_install or frappe.flags.in_import or getattr(doc, "no_feed_on_delete", False): + if ( + frappe.flags.in_install + or frappe.flags.in_uninstall + or frappe.flags.in_import + or getattr(doc, "no_feed_on_delete", False) + ): return + from frappe.utils import get_fullname + frappe.get_doc({ "doctype": "Comment", "comment_type": "Deleted", "reference_doctype": doc.doctype, "subject": "{0} {1}".format(_(doc.doctype), doc.name), - "full_name": get_fullname(doc.owner) + "full_name": get_fullname(doc.owner), }).insert(ignore_permissions=True) + def delete_controllers(doctype, module): """ Delete controller code in the doctype folder From 8fef0e0fcd2d885db59fcaee8c181d5fa41fd781 Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Thu, 12 Nov 2020 15:38:34 +0530 Subject: [PATCH 34/71] fix: TypeError on saving report with child table --- frappe/core/doctype/report/report.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/frappe/core/doctype/report/report.py b/frappe/core/doctype/report/report.py index 9d30409a2a..01c32bcb57 100644 --- a/frappe/core/doctype/report/report.py +++ b/frappe/core/doctype/report/report.py @@ -61,8 +61,9 @@ class Report(Document): def set_doctype_roles(self): if not self.get('roles') and self.is_standard == 'No': meta = frappe.get_meta(self.ref_doctype) - roles = [{'role': d.role} for d in meta.permissions if d.permlevel==0] - self.set('roles', roles) + if not meta.istable: + roles = [{'role': d.role} for d in meta.permissions if d.permlevel==0] + self.set('roles', roles) def is_permitted(self): """Returns true if Has Role is not set or the user is allowed.""" From b1b1fbbce2871f7c8b45f6c34f4f079d8fe561e5 Mon Sep 17 00:00:00 2001 From: conncampbell Date: Thu, 12 Nov 2020 08:04:00 -0700 Subject: [PATCH 35/71] feat: Web form fields support for property depends on fields (read-only and mandatory) --- frappe/website/doctype/web_form/web_form.js | 2 ++ .../web_form_field/web_form_field.json | 27 ++++++++++++++++++- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/frappe/website/doctype/web_form/web_form.js b/frappe/website/doctype/web_form/web_form.js index 6b4442ccaf..369f684192 100644 --- a/frappe/website/doctype/web_form/web_form.js +++ b/frappe/website/doctype/web_form/web_form.js @@ -59,6 +59,8 @@ frappe.ui.form.on("Web Form", { default: field.default, read_only: field.read_only, depends_on: field.depends_on, + mandatory_depends_on: field.mandatory_depends_on, + read_only_depends_on: field.read_only_depends_on, hidden: field.hidden, description: field.description }); diff --git a/frappe/website/doctype/web_form_field/web_form_field.json b/frappe/website/doctype/web_form_field/web_form_field.json index ef7b1c4ddf..72fcccf555 100644 --- a/frappe/website/doctype/web_form_field/web_form_field.json +++ b/frappe/website/doctype/web_form_field/web_form_field.json @@ -18,6 +18,10 @@ "options", "max_length", "max_value", + "property_depends_on_section", + "mandatory_depends_on", + "column_break_16", + "read_only_depends_on", "section_break_6", "description", "column_break_8", @@ -117,11 +121,32 @@ "fieldname": "default", "fieldtype": "Data", "label": "Default" + }, + { + "fieldname": "property_depends_on_section", + "fieldtype": "Section Break", + "label": "Property Depends On" + }, + { + "fieldname": "mandatory_depends_on", + "fieldtype": "Code", + "label": "Mandatory Depends On", + "options": "JS" + }, + { + "fieldname": "column_break_16", + "fieldtype": "Column Break" + }, + { + "fieldname": "read_only_depends_on", + "fieldtype": "Code", + "label": "Read Only Depends On", + "options": "JS" } ], "istable": 1, "links": [], - "modified": "2020-05-13 13:35:08.454427", + "modified": "2020-11-10 23:20:44.354862", "modified_by": "Administrator", "module": "Website", "name": "Web Form Field", From 0f13fe7da5e7158830ab77896a76116e71b02b64 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 13 Nov 2020 15:13:55 +0530 Subject: [PATCH 36/71] fix: Validate SQL files properly --- frappe/installer.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/frappe/installer.py b/frappe/installer.py index 6745a92345..3edea185cf 100755 --- a/frappe/installer.py +++ b/frappe/installer.py @@ -415,23 +415,29 @@ def validate_database_sql(path, _raise=True): path (str): Path of the decompressed SQL file _raise (bool, optional): Raise exception if invalid file. Defaults to True. """ - to_raise = False + empty_file = False + missing_table = True + error_message = "" if not os.path.getsize(path): error_message = f"{path} is an empty file!" - to_raise = True + empty_file = True - if not _raise: + # dont bother checking if empty file + if not empty_file: with open(path, "r") as f: for line in f: if 'tabDefaultValue' in line: - error_message = "Table `tabDefaultValue` not found in file." - to_raise = True + missing_table = False + break + + if missing_table: + error_message = "Table `tabDefaultValue` not found in file." if error_message: import click click.secho(error_message, fg="red") - if _raise and to_raise: + if _raise and (missing_table or empty_file): raise frappe.InvalidDatabaseFile From 14fe6ccfa3e12afbe94ead781aa3179f79f20a2f Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 13 Nov 2020 19:42:38 +0530 Subject: [PATCH 37/71] fix: Add is_partial check in bench restore --- frappe/commands/site.py | 35 ++++++++++++++++++++++++++++++++--- frappe/installer.py | 8 ++++++++ frappe/utils/backups.py | 1 + 3 files changed, 41 insertions(+), 3 deletions(-) diff --git a/frappe/commands/site.py b/frappe/commands/site.py index c12afcb7ef..b55e7b7253 100755 --- a/frappe/commands/site.py +++ b/frappe/commands/site.py @@ -56,17 +56,41 @@ def new_site(site, mariadb_root_username=None, mariadb_root_password=None, admin @pass_context def restore(context, sql_file_path, mariadb_root_username=None, mariadb_root_password=None, db_name=None, verbose=None, install_app=None, admin_password=None, force=None, with_public_files=None, with_private_files=None): "Restore site database from an sql file" - from frappe.installer import extract_sql_from_archive, extract_files, is_downgrade, validate_database_sql + from frappe.installer import ( + extract_sql_from_archive, + extract_files, + is_downgrade, + is_partial, + validate_database_sql + ) + force = context.force or force decompressed_file_name = extract_sql_from_archive(sql_file_path) + # check if partial backup + if is_partial(decompressed_file_name): + click.secho( + "Partial Backup file detected. You cannot use a partial file to restore a Frappe Site.", + fg="red" + ) + click.secho( + "Use `bench partial-restore` to restore a partial backup to an existing site.", + fg="yellow" + ) + sys.exit(1) + + # check if valid SQL file validate_database_sql(decompressed_file_name, _raise=force) + site = get_site(context) frappe.init(site=site) # dont allow downgrading to older versions of frappe without force if not force and is_downgrade(decompressed_file_name, verbose=True): - warn_message = "This is not recommended and may lead to unexpected behaviour. Do you want to continue anyway?" + warn_message = ( + "This is not recommended and may lead to unexpected behaviour. " + "Do you want to continue anyway?" + ) click.confirm(warn_message, abort=True) _new_site(frappe.conf.db_name, site, mariadb_root_username=mariadb_root_username, @@ -89,9 +113,13 @@ def restore(context, sql_file_path, mariadb_root_username=None, mariadb_root_pas if decompressed_file_name != sql_file_path: os.remove(decompressed_file_name) - success_message = "Site {0} has been restored{1}".format(site, " with files" if (with_public_files or with_private_files) else "") + success_message = "Site {0} has been restored{1}".format( + site, + " with files" if (with_public_files or with_private_files) else "" + ) click.secho(success_message, fg="green") + @click.command('partial-restore') @click.argument('sql-file-path') @click.option("--verbose", "-v", is_flag=True) @@ -106,6 +134,7 @@ def partial_restore(context, sql_file_path, verbose): partial_restore(sql_file_path, verbose) frappe.destroy() + @click.command('reinstall') @click.option('--admin-password', help='Administrator Password for reinstalled site') @click.option('--mariadb-root-username', help='Root username for MariaDB') diff --git a/frappe/installer.py b/frappe/installer.py index 2285d0011a..32a61f35fb 100755 --- a/frappe/installer.py +++ b/frappe/installer.py @@ -528,6 +528,14 @@ def is_downgrade(sql_file_path, verbose=False): return downgrade +def is_partial(sql_file_path): + with open(sql_file_path) as f: + header = " ".join([f.readline() for _ in range(5)]) + if "Partial Backup" in header: + return True + return False + + def partial_restore(sql_file_path, verbose=False): sql_file = extract_sql_from_archive(sql_file_path) diff --git a/frappe/utils/backups.py b/frappe/utils/backups.py index 178d213e59..fe045be81f 100644 --- a/frappe/utils/backups.py +++ b/frappe/utils/backups.py @@ -6,6 +6,7 @@ import os from calendar import timegm from datetime import datetime from glob import glob +import gzip # imports - third party imports import click From 2fd5a82bbdd970089e209229eaa93304493c8ab5 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 13 Nov 2020 19:43:07 +0530 Subject: [PATCH 38/71] fix: Add header content in database backups --- frappe/utils/backups.py | 41 ++++++++++++++++++++++++++++++----------- 1 file changed, 30 insertions(+), 11 deletions(-) diff --git a/frappe/utils/backups.py b/frappe/utils/backups.py index fe045be81f..b11c73c2b2 100644 --- a/frappe/utils/backups.py +++ b/frappe/utils/backups.py @@ -342,17 +342,36 @@ class BackupGenerator: def take_dump(self): import frappe.utils + from frappe.utils.change_log import get_app_branch + + database_header_content = [ + f"Backup generated by Frappe {frappe.__version__} on branch {get_app_branch('frappe') or 'N/A'}", + "", + ] # escape reserved characters - args = dict( + args = frappe._dict( [item[0], frappe.utils.esc(str(item[1]), "$ ")] for item in self.__dict__.copy().items() ) if self.backup_includes: - print("Backing Up Tables: {0}\n".format(", ".join(self.backup_includes))) + backup_info = ("Backing Up Tables: ", ", ".join(self.backup_includes)) elif self.backup_excludes: - print("Skipping Tables: {0}\n".format(", ".join(self.backup_excludes))) + backup_info = ("Skipping Tables: ", ", ".join(self.backup_excludes)) + + if self.partial: + print(''.join(backup_info), "\n") + database_header_content.extend([ + f"Partial Backup of Frappe Site {frappe.local.site}", + ("Backup contains: " if self.backup_includes else "Backup excludes: ") + backup_info[1], + "", + ]) + + generated_header = "\n".join([f"-- {x}" for x in database_header_content]) + "\n" + + with gzip.open(args.backup_path_db, "wt") as f: + f.write(generated_header) if self.db_type == "postgres": if self.backup_includes: @@ -366,7 +385,7 @@ class BackupGenerator: cmd_string = ( "pg_dump postgres://{user}:{password}@{db_host}:{db_port}/{db_name}" - " {include} {exclude} | gzip > {backup_path_db}" + " {include} {exclude} | gzip >> {backup_path_db}" ) else: @@ -383,16 +402,16 @@ class BackupGenerator: cmd_string = ( "mysqldump --single-transaction --quick --lock-tables=false -u {user}" " -p{password} {db_name} -h {db_host} -P {db_port} {include} {exclude}" - " | gzip > {backup_path_db}" + " | gzip >> {backup_path_db}" ) command = cmd_string.format( - user=args.get("user"), - password=args.get("password"), - db_host=args.get("db_host"), - db_port=args.get("db_port"), - db_name=args.get("db_name"), - backup_path_db=args.get("backup_path_db"), + user=args.user, + password=args.password, + db_host=args.db_host, + db_port=args.db_port, + db_name=args.db_name, + backup_path_db=args.backup_path_db, exclude=args.get("exclude", ""), include=args.get("include", ""), ) From 22b752ac21c69cf2f50df9bfae472c1dacf0128a Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Sat, 14 Nov 2020 11:11:19 +0530 Subject: [PATCH 39/71] test: Add tests for bench restore --- frappe/tests/test_commands.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/frappe/tests/test_commands.py b/frappe/tests/test_commands.py index 3f19abdae4..422dbf2930 100644 --- a/frappe/tests/test_commands.py +++ b/frappe/tests/test_commands.py @@ -201,9 +201,7 @@ class TestCommands(BaseTestCommands): # test 5: take a backup with --compress self.execute("bench --site {site} backup --with-files --compress") - self.assertEquals(self.returncode, 0) - compressed_files = glob(site_backup_path + "/*.tgz") self.assertGreater(len(compressed_files), 0) @@ -244,6 +242,18 @@ class TestCommands(BaseTestCommands): database = fetch_latest_backups()["database"] self.assertTrue(exists_in_backup(backup["excludes"]["excludes"], database)) + def test_restore(self): + # test 1: bench restore from full backup + self.execute("bench --site {site} backup --ignore-backup-conf") + database = fetch_latest_backups()["database"] + self.execute("bench --site {site} restore {database}", {"database": database}) + + # test 2: restore from partial backup + self.execute("bench --site {site} backup --exclude 'ToDo") + database = fetch_latest_backups(partial=True)["database"] + self.execute("bench --site {site} restore {database}", {"database": database}) + self.assertEquals(self.returncode, 1) + def test_partial_restore(self): _now = now() for num in range(10): From e8c6a7afc2ad7d70bdf5daf3bb3cae382f088410 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Sat, 14 Nov 2020 14:48:39 +0530 Subject: [PATCH 40/71] fix: Manage private images via get_local_image --- frappe/core/doctype/file/file.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/frappe/core/doctype/file/file.py b/frappe/core/doctype/file/file.py index b8bed89a4d..2a4e1983c9 100755 --- a/frappe/core/doctype/file/file.py +++ b/frappe/core/doctype/file/file.py @@ -612,7 +612,12 @@ def get_extension(filename, extn, content): return extn def get_local_image(file_url): - file_path = frappe.get_site_path("public", file_url.lstrip("/")) + if file_url.startswith("/private"): + file_url_path = (file_url.lstrip("/"), ) + else: + file_url_path = ("public", file_url.lstrip("/")) + + file_path = frappe.get_site_path(*file_url_path) try: image = Image.open(file_path) From 2bc477331d2eab4a90a62982c53bff746cb50358 Mon Sep 17 00:00:00 2001 From: Mohammad Hasnain Date: Mon, 16 Nov 2020 13:59:56 +0530 Subject: [PATCH 41/71] refactor: cleanup --- .../public/js/frappe/form/controls/comment.js | 2 +- .../public/js/frappe/form/footer/timeline.js | 52 ++++++++++++------- 2 files changed, 34 insertions(+), 20 deletions(-) diff --git a/frappe/public/js/frappe/form/controls/comment.js b/frappe/public/js/frappe/form/controls/comment.js index a64df56bca..d00c915065 100644 --- a/frappe/public/js/frappe/form/controls/comment.js +++ b/frappe/public/js/frappe/form/controls/comment.js @@ -60,7 +60,7 @@ frappe.ui.form.ControlComment = frappe.ui.form.ControlTextEditor.extend({ update_state() { const value = this.get_value(); - if (strip_html(value).trim() != "") { + if (strip_html(value).trim() != "" || value.includes('img')) { this.button.removeClass('btn-default').addClass('btn-primary'); } else { this.button.addClass('btn-default').removeClass('btn-primary'); diff --git a/frappe/public/js/frappe/form/footer/timeline.js b/frappe/public/js/frappe/form/footer/timeline.js index 84f34d4757..2e390dc3cb 100644 --- a/frappe/public/js/frappe/form/footer/timeline.js +++ b/frappe/public/js/frappe/form/footer/timeline.js @@ -30,7 +30,7 @@ frappe.ui.form.Timeline = class Timeline { render_input: true, only_input: true, on_submit: (val) => { - if(strip_html(val).trim() != "") { + if(strip_html(val).trim() != "" || val.includes('img')) { this.insert_comment(val, this.comment_area.button); } } @@ -547,10 +547,14 @@ frappe.ui.form.Timeline = class Timeline { log.color = 'dark'; log.sender = log.owner; log.comment_type = 'Milestone'; - log.content = __('{0} changed {1} to {2}', [ - frappe.user.full_name(log.owner).bold(), - frappe.meta.get_label(this.frm.doctype, log.track_field), - log.value.bold()]); + log.content = __( + '{0} changed {1} to {2}', + [ + frappe.user.full_name(log.owner).bold(), + frappe.meta.get_label(this.frm.doctype, log.track_field), + log.value.bold() + ] + ); return log; }); return milestones; @@ -613,11 +617,14 @@ frappe.ui.form.Timeline = class Timeline { const field_display_status = frappe.perm.get_field_display_status(df, null, me.frm.perm); if (field_display_status === 'Read' || field_display_status === 'Write') { - parts.push(__('{0} from {1} to {2}', [ - __(df.label), - me.format_content_for_timeline(p[1]), - me.format_content_for_timeline(p[2]) - ])); + parts.push(__( + '{0} from {1} to {2}', + [ + __(df.label), + me.format_content_for_timeline(p[1]), + me.format_content_for_timeline(p[2]) + ] + )); } } } @@ -648,13 +655,18 @@ frappe.ui.form.Timeline = class Timeline { null, me.frm.perm); if (field_display_status === 'Read' || field_display_status === 'Write') { - parts.push(__('{0} from {1} to {2} in row #{3}', [ - frappe.meta.get_label(me.frm.fields_dict[row[0]].grid.doctype, - p[0]), - me.format_content_for_timeline(p[1]), - me.format_content_for_timeline(p[2]), - row[1] - ])); + parts.push(__( + '{0} from {1} to {2} in row #{3}', + [ + frappe.meta.get_label( + me.frm.fields_dict[row[0]].grid.doctype, + p[0] + ), + me.format_content_for_timeline(p[1]), + me.format_content_for_timeline(p[2]), + row[1] + ] + )); } } return parts.length < 3; @@ -691,8 +703,10 @@ frappe.ui.form.Timeline = class Timeline { return p; }); if (parts.length) { - out.push(me.get_version_comment(version, __("{0} rows for {1}", - [__(key), parts.join(', ')]))); + out.push(me.get_version_comment(version, __( + "{0} rows for {1}", + [__(key), parts.join(', ')] + ))); } } }); From 4443dab3cbc4510574e306ba84d114e13ac8e6dc Mon Sep 17 00:00:00 2001 From: Mohammad Hasnain Mohsin Rajan Date: Mon, 16 Nov 2020 14:03:00 +0530 Subject: [PATCH 42/71] refactor: solve sider issues --- frappe/public/js/frappe/form/footer/timeline.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/form/footer/timeline.js b/frappe/public/js/frappe/form/footer/timeline.js index 2e390dc3cb..c23b6d8127 100644 --- a/frappe/public/js/frappe/form/footer/timeline.js +++ b/frappe/public/js/frappe/form/footer/timeline.js @@ -30,7 +30,7 @@ frappe.ui.form.Timeline = class Timeline { render_input: true, only_input: true, on_submit: (val) => { - if(strip_html(val).trim() != "" || val.includes('img')) { + if (strip_html(val).trim() != "" || val.includes('img')) { this.insert_comment(val, this.comment_area.button); } } From c4e57926f9899a3c0d1a0922aea651f66a3032a9 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 17 Nov 2020 12:33:21 +0530 Subject: [PATCH 43/71] feat(postgres): Show restore progress if pv is available --- frappe/database/postgres/setup_db.py | 50 +++++++++++++++++----------- 1 file changed, 31 insertions(+), 19 deletions(-) diff --git a/frappe/database/postgres/setup_db.py b/frappe/database/postgres/setup_db.py index 109ed0469e..bf344c3aa5 100644 --- a/frappe/database/postgres/setup_db.py +++ b/frappe/database/postgres/setup_db.py @@ -1,5 +1,7 @@ -import frappe, subprocess, os -from six.moves import input +import os + +import frappe + def setup_database(force, source_sql=None, verbose=False): root_conn = get_root_connection() @@ -19,6 +21,7 @@ def bootstrap_database(db_name, verbose, source_sql=None): frappe.connect(db_name=db_name) import_db_from_sql(source_sql, verbose) frappe.connect(db_name=db_name) + if 'tabDefaultValue' not in frappe.db.get_tables(): import sys from click import secho @@ -32,32 +35,40 @@ def bootstrap_database(db_name, verbose, source_sql=None): sys.exit(1) def import_db_from_sql(source_sql=None, verbose=False): - if verbose: - print("Starting Database Import...") + from shlex import split + from shutil import which + from subprocess import run, PIPE # we can't pass psql password in arguments in postgresql as mysql. So # set password connection parameter in environment variable subprocess_env = os.environ.copy() subprocess_env['PGPASSWORD'] = str(frappe.conf.db_password) + # bootstrap db if not source_sql: source_sql = os.path.join(os.path.dirname(__file__), 'framework_postgres.sql') - command = [ - 'psql', frappe.conf.db_name, - '-h', frappe.conf.db_host or 'localhost', - '-p', str(frappe.conf.db_port or '5432'), - '-U', frappe.conf.db_name, - '-f', source_sql - ] + pv = which('pv') + + _command = ( + f"psql {frappe.conf.db_name} " + f"-h {frappe.conf.db_host or 'localhost'} -p {str(frappe.conf.db_port or '5432')} " + f"-U {frappe.conf.db_name}" + ) + + if pv: + command = f"{pv} {source_sql} | " + _command + else: + command = _command + f"-f {source_sql}" + + print("Restoring Database file...") + if verbose: + print(command) + + restore_proc = run(command, env=subprocess_env, shell=True, stdout=PIPE) if verbose: - print(" ".join(command)) - - subprocess.check_output(command, env=subprocess_env) - - if verbose: - print(f"Imported from Database File: {source_sql}") + print(f"\nSTDOUT by psql:\n{restore_proc.stdout.decode()}\nImported from Database File: {source_sql}") def setup_help_database(help_db_name): root_conn = get_root_connection() @@ -68,19 +79,20 @@ def setup_help_database(help_db_name): root_conn.sql("GRANT ALL PRIVILEGES ON DATABASE `{0}` TO {0}".format(help_db_name)) def get_root_connection(root_login=None, root_password=None): - import getpass if not frappe.local.flags.root_connection: if not root_login: root_login = frappe.conf.get("root_login") or None if not root_login: + from six.moves import input root_login = input("Enter postgres super user: ") if not root_password: root_password = frappe.conf.get("root_password") or None if not root_password: - root_password = getpass.getpass("Postgres super user password: ") + from getpass import getpass + root_password = getpass("Postgres super user password: ") frappe.local.flags.root_connection = frappe.database.get_db(user=root_login, password=root_password) From 84e2fcd6a8b7bdc0ec6f6eaabe1b4b6d7582cf3c Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 17 Nov 2020 12:33:59 +0530 Subject: [PATCH 44/71] fix: Add missing quote --- frappe/tests/test_commands.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/tests/test_commands.py b/frappe/tests/test_commands.py index 422dbf2930..3c5fddeef0 100644 --- a/frappe/tests/test_commands.py +++ b/frappe/tests/test_commands.py @@ -249,7 +249,7 @@ class TestCommands(BaseTestCommands): self.execute("bench --site {site} restore {database}", {"database": database}) # test 2: restore from partial backup - self.execute("bench --site {site} backup --exclude 'ToDo") + self.execute("bench --site {site} backup --exclude 'ToDo'") database = fetch_latest_backups(partial=True)["database"] self.execute("bench --site {site} restore {database}", {"database": database}) self.assertEquals(self.returncode, 1) From 3b5ec6aa88fa94a83aa00358fb2f85f7e72e0ab5 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 17 Nov 2020 12:45:55 +0530 Subject: [PATCH 45/71] fix: Sider + improper command ending --- frappe/database/postgres/setup_db.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frappe/database/postgres/setup_db.py b/frappe/database/postgres/setup_db.py index bf344c3aa5..3ee6b6a286 100644 --- a/frappe/database/postgres/setup_db.py +++ b/frappe/database/postgres/setup_db.py @@ -35,7 +35,6 @@ def bootstrap_database(db_name, verbose, source_sql=None): sys.exit(1) def import_db_from_sql(source_sql=None, verbose=False): - from shlex import split from shutil import which from subprocess import run, PIPE @@ -59,7 +58,7 @@ def import_db_from_sql(source_sql=None, verbose=False): if pv: command = f"{pv} {source_sql} | " + _command else: - command = _command + f"-f {source_sql}" + command = _command + f" -f {source_sql}" print("Restoring Database file...") if verbose: From 7d091c19575a28f28803f2062a840d9c88d81432 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Mon, 19 Oct 2020 12:04:03 +0530 Subject: [PATCH 46/71] fix: Move attachment limit validation to file uploader --- frappe/public/js/frappe/file_uploader/index.js | 6 ++++++ frappe/public/js/frappe/form/form.js | 6 +----- frappe/public/js/frappe/form/sidebar/attachments.js | 2 +- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/frappe/public/js/frappe/file_uploader/index.js b/frappe/public/js/frappe/file_uploader/index.js index 62a7bff822..5336cd7c52 100644 --- a/frappe/public/js/frappe/file_uploader/index.js +++ b/frappe/public/js/frappe/file_uploader/index.js @@ -15,7 +15,13 @@ export default class FileUploader { allow_multiple, as_dataurl, disable_file_browser, + frm } = {}) { + if (frm && frm.attachments.max_reached()) { + frappe.throw(__("Maximum Attachment Limit for this record reached.")); + return; + } + if (!wrapper) { this.make_dialog(); } else { diff --git a/frappe/public/js/frappe/form/form.js b/frappe/public/js/frappe/form/form.js index bb9e8c22d1..90b628f269 100644 --- a/frappe/public/js/frappe/form/form.js +++ b/frappe/public/js/frappe/form/form.js @@ -232,14 +232,10 @@ frappe.ui.form.Form = class FrappeForm { throw "attach error"; } - if(me.attachments.max_reached()) { - frappe.msgprint(__("Maximum Attachment Limit for this record reached.")); - throw "attach error"; - } - new frappe.ui.FileUploader({ doctype: me.doctype, docname: me.docname, + frm: me, files: dataTransfer.files, folder: 'Home/Attachments', on_success(file_doc) { diff --git a/frappe/public/js/frappe/form/sidebar/attachments.js b/frappe/public/js/frappe/form/sidebar/attachments.js index 165527e281..dc0c839737 100644 --- a/frappe/public/js/frappe/form/sidebar/attachments.js +++ b/frappe/public/js/frappe/form/sidebar/attachments.js @@ -140,7 +140,6 @@ frappe.ui.form.Attachments = Class.extend({ }); }, new_attachment: function(fieldname) { - var me = this; if (this.dialog) { // remove upload dialog this.dialog.$wrapper.remove(); @@ -149,6 +148,7 @@ frappe.ui.form.Attachments = Class.extend({ new frappe.ui.FileUploader({ doctype: this.frm.doctype, docname: this.frm.docname, + frm: this.frm, folder: 'Home/Attachments', on_success: (file_doc) => { this.attachment_uploaded(file_doc); From b33461bb53f05233017d27f19d83846c59e235e4 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Mon, 19 Oct 2020 12:04:57 +0530 Subject: [PATCH 47/71] feat: Add server-side validation for attachment limit --- frappe/core/doctype/file/file.py | 21 +++++++++++++++++++++ frappe/exceptions.py | 1 + 2 files changed, 22 insertions(+) diff --git a/frappe/core/doctype/file/file.py b/frappe/core/doctype/file/file.py index b8bed89a4d..9b0b3f3fd1 100755 --- a/frappe/core/doctype/file/file.py +++ b/frappe/core/doctype/file/file.py @@ -93,6 +93,7 @@ class File(Document): self.set_is_private() self.set_file_name() self.validate_duplicate_entry() + self.validate_attachment_limit() self.validate_folder() if not self.file_url and not self.flags.ignore_file_validate: @@ -140,6 +141,26 @@ class File(Document): if self.file_url and (self.is_private != self.file_url.startswith('/private')): frappe.throw(_('Invalid file URL. Please contact System Administrator.')) + def validate_attachment_limit(self): + attachment_limit = 0 + if self.attached_to_doctype and self.attached_to_name: + attachment_limit = frappe.get_meta(self.attached_to_doctype).max_attachments + + if attachment_limit: + current_attachment_count = len(frappe.get_all('File', filters={ + 'attached_to_doctype': self.attached_to_doctype, + 'attached_to_name': self.attached_to_name, + }, limit=(attachment_limit + 1))) + + if current_attachment_count >= attachment_limit: + frappe.throw(_("Attachment Limit reached for {0} {1}.").format( + self.attached_to_doctype, + self.attached_to_name + ), + exc=frappe.exceptions.AttachmentLimitReached, + title=_('Maximum Attachment Limit Reached') + ) + def set_folder_name(self): """Make parent folders if not exists based on reference doctype and name""" if self.attached_to_doctype and not self.folder: diff --git a/frappe/exceptions.py b/frappe/exceptions.py index 267f5410af..81fccf5880 100644 --- a/frappe/exceptions.py +++ b/frappe/exceptions.py @@ -106,6 +106,7 @@ class InvalidDates(ValidationError): pass class DataTooLongException(ValidationError): pass class FileAlreadyAttachedException(Exception): pass class DocumentAlreadyRestored(Exception): pass +class AttachmentLimitReached(Exception): pass # OAuth exceptions class InvalidAuthorizationHeader(CSRFTokenError): pass class InvalidAuthorizationPrefix(CSRFTokenError): pass From 23efc9919ab24c58d60256a02fb067e5e11ff9d0 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Mon, 19 Oct 2020 12:37:02 +0530 Subject: [PATCH 48/71] fix: Update validation message --- frappe/core/doctype/file/file.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/core/doctype/file/file.py b/frappe/core/doctype/file/file.py index 9b0b3f3fd1..bb7765e055 100755 --- a/frappe/core/doctype/file/file.py +++ b/frappe/core/doctype/file/file.py @@ -153,12 +153,12 @@ class File(Document): }, limit=(attachment_limit + 1))) if current_attachment_count >= attachment_limit: - frappe.throw(_("Attachment Limit reached for {0} {1}.").format( + frappe.throw(_("Maximum Attachment Limit reached for {0} {1}.").format( self.attached_to_doctype, self.attached_to_name ), exc=frappe.exceptions.AttachmentLimitReached, - title=_('Maximum Attachment Limit Reached') + title=_('Attachment Limit Reached') ) def set_folder_name(self): From a2471ace211831c644454f13df8a4b384b251af9 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Mon, 19 Oct 2020 12:43:14 +0530 Subject: [PATCH 49/71] fix: Update attachment limit --- frappe/public/js/frappe/file_uploader/index.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/file_uploader/index.js b/frappe/public/js/frappe/file_uploader/index.js index 5336cd7c52..36de853b8f 100644 --- a/frappe/public/js/frappe/file_uploader/index.js +++ b/frappe/public/js/frappe/file_uploader/index.js @@ -18,7 +18,10 @@ export default class FileUploader { frm } = {}) { if (frm && frm.attachments.max_reached()) { - frappe.throw(__("Maximum Attachment Limit for this record reached.")); + frappe.throw({ + title: __("Attachment Limit Reached"), + message: __("Maximum attachment limit for this record reached."), + }); return; } From 42fc8fb25c8ec59d99468d99a316518aff62f158 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Mon, 19 Oct 2020 13:34:04 +0530 Subject: [PATCH 50/71] fix: Convert limit to integer --- frappe/core/doctype/file/file.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/core/doctype/file/file.py b/frappe/core/doctype/file/file.py index bb7765e055..a8a999ff9a 100755 --- a/frappe/core/doctype/file/file.py +++ b/frappe/core/doctype/file/file.py @@ -144,13 +144,13 @@ class File(Document): def validate_attachment_limit(self): attachment_limit = 0 if self.attached_to_doctype and self.attached_to_name: - attachment_limit = frappe.get_meta(self.attached_to_doctype).max_attachments + attachment_limit = cint(frappe.get_meta(self.attached_to_doctype).max_attachments) if attachment_limit: current_attachment_count = len(frappe.get_all('File', filters={ 'attached_to_doctype': self.attached_to_doctype, 'attached_to_name': self.attached_to_name, - }, limit=(attachment_limit + 1))) + }, limit=attachment_limit + 1)) if current_attachment_count >= attachment_limit: frappe.throw(_("Maximum Attachment Limit reached for {0} {1}.").format( From e64483ad89b7d6af385256aaccb9a8acbcff3b0a Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Mon, 19 Oct 2020 13:35:16 +0530 Subject: [PATCH 51/71] test: Add test for attachment limit validation --- frappe/core/doctype/file/test_file.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/frappe/core/doctype/file/test_file.py b/frappe/core/doctype/file/test_file.py index 85397ea1ee..cb44d33781 100644 --- a/frappe/core/doctype/file/test_file.py +++ b/frappe/core/doctype/file/test_file.py @@ -160,6 +160,29 @@ class TestSameContent(unittest.TestCase): def test_saved_content(self): self.assertFalse(os.path.exists(get_files_path(self.dup_filename))) + def test_attachment_limit(self): + doctype, docname = make_test_doc() + from frappe.custom.doctype.property_setter.property_setter import make_property_setter + limit_property = make_property_setter('ToDo', None, 'max_attachments', 1, 'int', for_doctype=True) + _file1 = frappe.get_doc({ + "doctype": "File", + "file_name": 'test-attachment', + "attached_to_doctype": doctype, + "attached_to_name": docname, + "content": 'test'}) + + _file1.insert() + + _file2 = frappe.get_doc({ + "doctype": "File", + "file_name": 'test-attachment', + "attached_to_doctype": doctype, + "attached_to_name": docname, + "content": 'test2'}) + + self.assertRaises(frappe.exceptions.AttachmentLimitReached, _file2.insert) + limit_property.delete() + frappe.clear_cache(doctype='ToDo') def tearDown(self): # File gets deleted on rollback, so blank From a7264405b5a09ac5f7d4121e914a570a5a6de2b6 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Tue, 20 Oct 2020 10:27:16 +0530 Subject: [PATCH 52/71] style: Fix formatting --- frappe/core/doctype/file/file.py | 6 +++--- frappe/core/doctype/file/test_file.py | 14 ++++++++------ 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/frappe/core/doctype/file/file.py b/frappe/core/doctype/file/file.py index a8a999ff9a..74c6c3f22e 100755 --- a/frappe/core/doctype/file/file.py +++ b/frappe/core/doctype/file/file.py @@ -153,9 +153,9 @@ class File(Document): }, limit=attachment_limit + 1)) if current_attachment_count >= attachment_limit: - frappe.throw(_("Maximum Attachment Limit reached for {0} {1}.").format( - self.attached_to_doctype, - self.attached_to_name + frappe.throw( + _("Maximum Attachment Limit reached for {0} {1}.").format( + self.attached_to_doctype, self.attached_to_name ), exc=frappe.exceptions.AttachmentLimitReached, title=_('Attachment Limit Reached') diff --git a/frappe/core/doctype/file/test_file.py b/frappe/core/doctype/file/test_file.py index cb44d33781..e627558680 100644 --- a/frappe/core/doctype/file/test_file.py +++ b/frappe/core/doctype/file/test_file.py @@ -164,23 +164,25 @@ class TestSameContent(unittest.TestCase): doctype, docname = make_test_doc() from frappe.custom.doctype.property_setter.property_setter import make_property_setter limit_property = make_property_setter('ToDo', None, 'max_attachments', 1, 'int', for_doctype=True) - _file1 = frappe.get_doc({ + file1 = frappe.get_doc({ "doctype": "File", "file_name": 'test-attachment', "attached_to_doctype": doctype, "attached_to_name": docname, - "content": 'test'}) + "content": 'test' + }) - _file1.insert() + file1.insert() - _file2 = frappe.get_doc({ + file2 = frappe.get_doc({ "doctype": "File", "file_name": 'test-attachment', "attached_to_doctype": doctype, "attached_to_name": docname, - "content": 'test2'}) + "content": 'test2' + }) - self.assertRaises(frappe.exceptions.AttachmentLimitReached, _file2.insert) + self.assertRaises(frappe.exceptions.AttachmentLimitReached, file2.insert) limit_property.delete() frappe.clear_cache(doctype='ToDo') From 1685afc49eed63a370ec98880e6c34266ad1e5db Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Mon, 26 Oct 2020 13:53:01 +0530 Subject: [PATCH 53/71] fix: Update limit validation message --- frappe/core/doctype/file/file.py | 4 ++-- .../public/js/frappe/file_uploader/index.js | 9 ++------- .../js/frappe/form/sidebar/attachments.js | 20 +++++++++++-------- 3 files changed, 16 insertions(+), 17 deletions(-) diff --git a/frappe/core/doctype/file/file.py b/frappe/core/doctype/file/file.py index 74c6c3f22e..48247860b3 100755 --- a/frappe/core/doctype/file/file.py +++ b/frappe/core/doctype/file/file.py @@ -154,8 +154,8 @@ class File(Document): if current_attachment_count >= attachment_limit: frappe.throw( - _("Maximum Attachment Limit reached for {0} {1}.").format( - self.attached_to_doctype, self.attached_to_name + _("Maximum Attachment Limit of {0} has been reached for {1} {2}.").format( + frappe.bold(attachment_limit), self.attached_to_doctype, self.attached_to_name ), exc=frappe.exceptions.AttachmentLimitReached, title=_('Attachment Limit Reached') diff --git a/frappe/public/js/frappe/file_uploader/index.js b/frappe/public/js/frappe/file_uploader/index.js index 36de853b8f..646f60715a 100644 --- a/frappe/public/js/frappe/file_uploader/index.js +++ b/frappe/public/js/frappe/file_uploader/index.js @@ -17,13 +17,8 @@ export default class FileUploader { disable_file_browser, frm } = {}) { - if (frm && frm.attachments.max_reached()) { - frappe.throw({ - title: __("Attachment Limit Reached"), - message: __("Maximum attachment limit for this record reached."), - }); - return; - } + + frm && frm.attachments.max_reached(true); if (!wrapper) { this.make_dialog(); diff --git a/frappe/public/js/frappe/form/sidebar/attachments.js b/frappe/public/js/frappe/form/sidebar/attachments.js index dc0c839737..56b484e7c4 100644 --- a/frappe/public/js/frappe/form/sidebar/attachments.js +++ b/frappe/public/js/frappe/form/sidebar/attachments.js @@ -16,15 +16,19 @@ frappe.ui.form.Attachments = Class.extend({ this.add_attachment_wrapper = this.parent.find(".add_attachment").parent(); this.attachments_label = this.parent.find(".attachments-label"); }, - max_reached: function() { - // no of attachments - var n = Object.keys(this.get_attachments()).length; - - // button if the number of attachments is less than max - if(n < this.frm.meta.max_attachments || !this.frm.meta.max_attachments) { - return false; + max_reached: function(raise_exception=false) { + const attachment_count = Object.keys(this.get_attachments()).length; + const attachment_limit = this.frm.meta.max_attachments; + if (attachment_limit && attachment_count >= attachment_limit) { + if (raise_exception) { + frappe.throw({ + title: __("Attachment Limit Reached"), + message: __("Maximum attachment limit of {0} has been reached.", [cstr(attachment_limit).bold()]), + }); + } + return true; } - return true; + return false; }, refresh: function() { var me = this; From 291179904615480bcaaa623e9b2bc055188b8ed9 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 17 Nov 2020 15:14:23 +0530 Subject: [PATCH 54/71] fix: Re-write restore tests to process another site --- frappe/tests/test_commands.py | 36 +++++++++++++++++++++++++++++------ 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/frappe/tests/test_commands.py b/frappe/tests/test_commands.py index 3c5fddeef0..e282f1027d 100644 --- a/frappe/tests/test_commands.py +++ b/frappe/tests/test_commands.py @@ -243,15 +243,39 @@ class TestCommands(BaseTestCommands): self.assertTrue(exists_in_backup(backup["excludes"]["excludes"], database)) def test_restore(self): + # step 0: create a site to run the test on + global_config = { + "admin_password": frappe.conf.admin_password, + "root_login": frappe.conf.root_login, + "root_password": frappe.conf.root_password, + "db_type": frappe.conf.db_type + } + site_data = { + "another_site": f"{frappe.local.site}-restore.test", + **global_config + } + for key, value in global_config.items(): + if value: + self.execute(f"bench set-config {key} {value} -g") + self.execute("bench new-site {another_site} --admin-password {admin_password} --db-type {db_type} --force", site_data) + # test 1: bench restore from full backup - self.execute("bench --site {site} backup --ignore-backup-conf") - database = fetch_latest_backups()["database"] - self.execute("bench --site {site} restore {database}", {"database": database}) + self.execute("bench --site {another_site} backup --ignore-backup-conf", site_data) + self.execute("bench --site {another_site} execute frappe.utils.backups.fetch_latest_backups", site_data) + site_data.update({"database": json.loads(self.stdout)["database"]}) + self.execute("bench --site {another_site} restore {database}", site_data) # test 2: restore from partial backup - self.execute("bench --site {site} backup --exclude 'ToDo'") - database = fetch_latest_backups(partial=True)["database"] - self.execute("bench --site {site} restore {database}", {"database": database}) + self.execute("bench --site {another_site} backup --exclude 'ToDo'", site_data) + site_data.update({"kw": "\"{'partial':True}\""}) + self.execute( + "bench --site {another_site} execute frappe.utils.backups.fetch_latest_backups --kwargs {kw}", + site_data + ) + site_data.update({ + "database": json.loads(self.stdout)["database"] + }) + self.execute("bench --site {another_site} restore {database}", site_data) self.assertEquals(self.returncode, 1) def test_partial_restore(self): From f1cd3388ba0b71a52cf87bd7e1af16f4e834b35f Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 17 Nov 2020 16:28:15 +0530 Subject: [PATCH 55/71] style: Black-ish + fixed typos + Optimized imports --- frappe/build.py | 2 +- .../frappe_providers/frappecloud.py | 2 +- frappe/tests/test_commands.py | 83 ++++++++++++------- 3 files changed, 53 insertions(+), 34 deletions(-) diff --git a/frappe/build.py b/frappe/build.py index f14b250a92..f47a7cb32b 100644 --- a/frappe/build.py +++ b/frappe/build.py @@ -105,7 +105,7 @@ def download_frappe_assets(verbose=True): if frappe_head: try: url = get_assets_link(frappe_head) - click.secho("Retreiving assets...", fg="yellow") + click.secho("Retrieving assets...", fg="yellow") prefix = mkdtemp(prefix="frappe-assets-", suffix=frappe_head) assets_archive = download_file(url, prefix) print("\n{0} Downloaded Frappe assets from {1}".format(green('✔'), url)) diff --git a/frappe/integrations/frappe_providers/frappecloud.py b/frappe/integrations/frappe_providers/frappecloud.py index e09f09a44b..f60344ee8f 100644 --- a/frappe/integrations/frappe_providers/frappecloud.py +++ b/frappe/integrations/frappe_providers/frappecloud.py @@ -6,7 +6,7 @@ import frappe def frappecloud_migrator(local_site): - print("Retreiving Site Migrator...") + print("Retrieving Site Migrator...") remote_site = frappe.conf.frappecloud_url or "frappecloud.com" request_url = "https://{}/api/method/press.api.script".format(remote_site) request = requests.get(request_url) diff --git a/frappe/tests/test_commands.py b/frappe/tests/test_commands.py index e282f1027d..8c76ce2f48 100644 --- a/frappe/tests/test_commands.py +++ b/frappe/tests/test_commands.py @@ -1,24 +1,24 @@ # Copyright (c) 2020, Frappe Technologies Pvt. Ltd. and Contributors # imports - standard imports +import gzip import json import os import shlex import subprocess import sys import unittest -import gzip -from glob import glob +import glob # imports - module imports import frappe +import frappe.recorder +from frappe.installer import add_to_installed_apps from frappe.utils import add_to_date, now from frappe.utils.backups import fetch_latest_backups -import frappe.recorder # TODO: check frappe.cli.coloured_output to set coloured output! - def supports_color(): """ Returns True if the running system's terminal supports color, and False @@ -32,12 +32,12 @@ def supports_color(): class color(dict): - nc = '\033[0m' - blue = '\033[94m' - green = '\033[92m' - yellow = '\033[93m' - red = '\033[91m' - silver = '\033[90m' + nc = "\033[0m" + blue = "\033[94m" + green = "\033[92m" + yellow = "\033[93m" + red = "\033[91m" + silver = "\033[90m" def __getattr__(self, key): if supports_color(): @@ -82,6 +82,7 @@ def exists_in_backup(doctypes, file): content = f.read().decode("utf8") return all([predicate.format(doctype).lower() in content.lower() for doctype in doctypes]) + class BaseTestCommands(unittest.TestCase): def execute(self, command, kwargs=None): site = {"site": frappe.local.site} @@ -109,6 +110,7 @@ class BaseTestCommands(unittest.TestCase): ]).strip() return "{}\n\n{}".format(output, cmd_execution_summary) + class TestCommands(BaseTestCommands): def test_execute(self): # test 1: execute a command expecting a numeric output @@ -127,7 +129,7 @@ class TestCommands(BaseTestCommands): # The returned value has quotes which have been trimmed for the test self.execute("""bench --site {site} execute frappe.bold --kwargs '{{"text": "DocType"}}'""") self.assertEquals(self.returncode, 0) - self.assertEquals(self.stdout[1:-1], frappe.bold(text='DocType')) + self.assertEquals(self.stdout[1:-1], frappe.bold(text="DocType")) def test_backup(self): backup = { @@ -184,16 +186,19 @@ class TestCommands(BaseTestCommands): "db_path": "database.sql.gz", "files_path": "public.tar", "private_path": "private.tar", - "conf_path": "config.json" + "conf_path": "config.json", }.items() } - self.execute("""bench + self.execute( + """bench --site {site} backup --with-files --backup-path-db {db_path} --backup-path-files {files_path} --backup-path-private-files {private_path} - --backup-path-conf {conf_path}""", kwargs) + --backup-path-conf {conf_path}""", + kwargs, + ) self.assertEquals(self.returncode, 0) for path in kwargs.values(): @@ -202,7 +207,7 @@ class TestCommands(BaseTestCommands): # test 5: take a backup with --compress self.execute("bench --site {site} backup --with-files --compress") self.assertEquals(self.returncode, 0) - compressed_files = glob(site_backup_path + "/*.tgz") + compressed_files = glob.glob(site_backup_path + "/*.tgz") self.assertGreater(len(compressed_files), 0) # test 6: take a backup with --verbose @@ -210,14 +215,20 @@ class TestCommands(BaseTestCommands): self.assertEquals(self.returncode, 0) # test 7: take a backup with frappe.conf.backup.includes - self.execute("bench --site {site} set-config backup '{includes}' --as-dict", {"includes": json.dumps(backup["includes"])}) + self.execute( + "bench --site {site} set-config backup '{includes}' --as-dict", + {"includes": json.dumps(backup["includes"])}, + ) self.execute("bench --site {site} backup --verbose") self.assertEquals(self.returncode, 0) database = fetch_latest_backups(partial=True)["database"] self.assertTrue(exists_in_backup(backup["includes"]["includes"], database)) # test 8: take a backup with frappe.conf.backup.excludes - self.execute("bench --site {site} set-config backup '{excludes}' --as-dict", {"excludes": json.dumps(backup["excludes"])}) + self.execute( + "bench --site {site} set-config backup '{excludes}' --as-dict", + {"excludes": json.dumps(backup["excludes"])}, + ) self.execute("bench --site {site} backup --verbose") self.assertEquals(self.returncode, 0) database = fetch_latest_backups(partial=True)["database"] @@ -225,13 +236,19 @@ class TestCommands(BaseTestCommands): self.assertTrue(exists_in_backup(backup["includes"]["includes"], database)) # test 9: take a backup with --include (with frappe.conf.excludes still set) - self.execute("bench --site {site} backup --include '{include}'", {"include": ",".join(backup["includes"]["includes"])}) + self.execute( + "bench --site {site} backup --include '{include}'", + {"include": ",".join(backup["includes"]["includes"])}, + ) self.assertEquals(self.returncode, 0) database = fetch_latest_backups(partial=True)["database"] self.assertTrue(exists_in_backup(backup["includes"]["includes"], database)) # test 10: take a backup with --exclude - self.execute("bench --site {site} backup --exclude '{exclude}'", {"exclude": ",".join(backup["excludes"]["excludes"])}) + self.execute( + "bench --site {site} backup --exclude '{exclude}'", + {"exclude": ",".join(backup["excludes"]["excludes"])}, + ) self.assertEquals(self.returncode, 0) database = fetch_latest_backups(partial=True)["database"] self.assertFalse(exists_in_backup(backup["excludes"]["excludes"], database)) @@ -248,20 +265,24 @@ class TestCommands(BaseTestCommands): "admin_password": frappe.conf.admin_password, "root_login": frappe.conf.root_login, "root_password": frappe.conf.root_password, - "db_type": frappe.conf.db_type - } - site_data = { - "another_site": f"{frappe.local.site}-restore.test", - **global_config + "db_type": frappe.conf.db_type, } + site_data = {"another_site": f"{frappe.local.site}-restore.test", **global_config} for key, value in global_config.items(): if value: self.execute(f"bench set-config {key} {value} -g") - self.execute("bench new-site {another_site} --admin-password {admin_password} --db-type {db_type} --force", site_data) + self.execute( + "bench new-site {another_site} --admin-password {admin_password} --db-type" + " {db_type}", + site_data, + ) # test 1: bench restore from full backup self.execute("bench --site {another_site} backup --ignore-backup-conf", site_data) - self.execute("bench --site {another_site} execute frappe.utils.backups.fetch_latest_backups", site_data) + self.execute( + "bench --site {another_site} execute frappe.utils.backups.fetch_latest_backups", + site_data, + ) site_data.update({"database": json.loads(self.stdout)["database"]}) self.execute("bench --site {another_site} restore {database}", site_data) @@ -269,12 +290,11 @@ class TestCommands(BaseTestCommands): self.execute("bench --site {another_site} backup --exclude 'ToDo'", site_data) site_data.update({"kw": "\"{'partial':True}\""}) self.execute( - "bench --site {another_site} execute frappe.utils.backups.fetch_latest_backups --kwargs {kw}", - site_data + "bench --site {another_site} execute" + " frappe.utils.backups.fetch_latest_backups --kwargs {kw}", + site_data, ) - site_data.update({ - "database": json.loads(self.stdout)["database"] - }) + site_data.update({"database": json.loads(self.stdout)["database"]}) self.execute("bench --site {another_site} restore {database}", site_data) self.assertEquals(self.returncode, 1) @@ -314,7 +334,6 @@ class TestCommands(BaseTestCommands): self.assertEqual(frappe.recorder.status(), False) def test_remove_from_installed_apps(self): - from frappe.installer import add_to_installed_apps app = "test_remove_app" add_to_installed_apps(app) From 39da1e0363b0d15d4f2c9eb779ea3eb093d960fc Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 18 Nov 2020 01:31:18 +0530 Subject: [PATCH 56/71] fix: Raise error if executable not found in PATH --- frappe/exceptions.py | 3 ++- frappe/utils/backups.py | 29 +++++++++++++++++++++++------ 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/frappe/exceptions.py b/frappe/exceptions.py index 267f5410af..9b63b23787 100644 --- a/frappe/exceptions.py +++ b/frappe/exceptions.py @@ -110,4 +110,5 @@ class DocumentAlreadyRestored(Exception): pass class InvalidAuthorizationHeader(CSRFTokenError): pass class InvalidAuthorizationPrefix(CSRFTokenError): pass class InvalidAuthorizationToken(CSRFTokenError): pass -class InvalidDatabaseFile(ValidationError): pass \ No newline at end of file +class InvalidDatabaseFile(ValidationError): pass +class ExecutableNotFound(FileNotFoundError): pass diff --git a/frappe/utils/backups.py b/frappe/utils/backups.py index b11c73c2b2..3ae300d3c4 100644 --- a/frappe/utils/backups.py +++ b/frappe/utils/backups.py @@ -2,11 +2,12 @@ # MIT License. See license.txt # imports - standard imports +import gzip import os from calendar import timegm from datetime import datetime from glob import glob -import gzip +from shutil import which # imports - third party imports import click @@ -14,7 +15,7 @@ import click # imports - module imports import frappe from frappe import _, conf -from frappe.utils import get_url, now, now_datetime, get_file_size +from frappe.utils import get_file_size, get_url, now, now_datetime # backup variable for backwards compatibility verbose = False @@ -344,6 +345,20 @@ class BackupGenerator: import frappe.utils from frappe.utils.change_log import get_app_branch + db_exc = { + "mariadb": ("mysqldump", which("mysqldump")), + "postgres": ("pg_dump", which("pg_dump")), + }[self.db_type] + gzip_exc = which("gzip") + + if not (gzip_exc and db_exc[1]): + _exc = "gzip" if not gzip_exc else db_exc[0] + frappe.throw( + f"{_exc} not found in PATH! This is required to take a backup.", + exc=frappe.ExecutableNotFound + ) + db_exc = db_exc[0] + database_header_content = [ f"Backup generated by Frappe {frappe.__version__} on branch {get_app_branch('frappe') or 'N/A'}", "", @@ -384,8 +399,8 @@ class BackupGenerator: ) cmd_string = ( - "pg_dump postgres://{user}:{password}@{db_host}:{db_port}/{db_name}" - " {include} {exclude} | gzip >> {backup_path_db}" + "{db_exc} postgres://{user}:{password}@{db_host}:{db_port}/{db_name}" + " {include} {exclude} | {gzip} >> {backup_path_db}" ) else: @@ -400,20 +415,22 @@ class BackupGenerator: ) cmd_string = ( - "mysqldump --single-transaction --quick --lock-tables=false -u {user}" + "{db_exc} --single-transaction --quick --lock-tables=false -u {user}" " -p{password} {db_name} -h {db_host} -P {db_port} {include} {exclude}" - " | gzip >> {backup_path_db}" + " | {gzip} >> {backup_path_db}" ) command = cmd_string.format( user=args.user, password=args.password, + db_exc=db_exc, db_host=args.db_host, db_port=args.db_port, db_name=args.db_name, backup_path_db=args.backup_path_db, exclude=args.get("exclude", ""), include=args.get("include", ""), + gzip=gzip_exc, ) if self.verbose: From b90980406c4ac7474a3acf27e91afef5027ad34d Mon Sep 17 00:00:00 2001 From: prssanna Date: Tue, 17 Nov 2020 18:53:27 +0530 Subject: [PATCH 57/71] fix: increase batch limit --- frappe/core/doctype/prepared_report/prepared_report.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/core/doctype/prepared_report/prepared_report.py b/frappe/core/doctype/prepared_report/prepared_report.py index 2c4d933440..1d0d6ebb09 100644 --- a/frappe/core/doctype/prepared_report/prepared_report.py +++ b/frappe/core/doctype/prepared_report/prepared_report.py @@ -89,7 +89,7 @@ def delete_expired_prepared_reports(): 'creation': ['<', frappe.utils.add_days(frappe.utils.now(), -expiry_period)] }) - batches = frappe.utils.create_batch(prepared_reports_to_delete, 50) + batches = frappe.utils.create_batch(prepared_reports_to_delete, 100) for batch in batches: args = { 'reports': batch, From 7f012b73b42616e8b622745831fa31e0212d678c Mon Sep 17 00:00:00 2001 From: Mohammad Hasnain Date: Tue, 17 Nov 2020 19:18:44 +0530 Subject: [PATCH 58/71] feat: solve translation string issues --- .../public/js/frappe/form/footer/timeline.js | 36 +++---------------- 1 file changed, 4 insertions(+), 32 deletions(-) diff --git a/frappe/public/js/frappe/form/footer/timeline.js b/frappe/public/js/frappe/form/footer/timeline.js index c23b6d8127..159ab8a61b 100644 --- a/frappe/public/js/frappe/form/footer/timeline.js +++ b/frappe/public/js/frappe/form/footer/timeline.js @@ -547,14 +547,7 @@ frappe.ui.form.Timeline = class Timeline { log.color = 'dark'; log.sender = log.owner; log.comment_type = 'Milestone'; - log.content = __( - '{0} changed {1} to {2}', - [ - frappe.user.full_name(log.owner).bold(), - frappe.meta.get_label(this.frm.doctype, log.track_field), - log.value.bold() - ] - ); + log.content = __('{0} changed {1} to {2}', [ frappe.user.full_name(log.owner).bold(), frappe.meta.get_label(this.frm.doctype, log.track_field), log.value.bold()]); return log; }); return milestones; @@ -617,14 +610,7 @@ frappe.ui.form.Timeline = class Timeline { const field_display_status = frappe.perm.get_field_display_status(df, null, me.frm.perm); if (field_display_status === 'Read' || field_display_status === 'Write') { - parts.push(__( - '{0} from {1} to {2}', - [ - __(df.label), - me.format_content_for_timeline(p[1]), - me.format_content_for_timeline(p[2]) - ] - )); + parts.push(__('{0} from {1} to {2}', [ __(df.label), me.format_content_for_timeline(p[1]), me.format_content_for_timeline(p[2])])); } } } @@ -655,18 +641,7 @@ frappe.ui.form.Timeline = class Timeline { null, me.frm.perm); if (field_display_status === 'Read' || field_display_status === 'Write') { - parts.push(__( - '{0} from {1} to {2} in row #{3}', - [ - frappe.meta.get_label( - me.frm.fields_dict[row[0]].grid.doctype, - p[0] - ), - me.format_content_for_timeline(p[1]), - me.format_content_for_timeline(p[2]), - row[1] - ] - )); + parts.push(__('{0} from {1} to {2} in row #{3}', [ frappe.meta.get_label( me.frm.fields_dict[row[0]].grid.doctype, p[0]), me.format_content_for_timeline(p[1]), me.format_content_for_timeline(p[2]), row[1] ])); } } return parts.length < 3; @@ -703,10 +678,7 @@ frappe.ui.form.Timeline = class Timeline { return p; }); if (parts.length) { - out.push(me.get_version_comment(version, __( - "{0} rows for {1}", - [__(key), parts.join(', ')] - ))); + out.push(me.get_version_comment(version, __("{0} rows for {1}", [__(key), parts.join(', ')]))); } } }); From f37789280b9f75fffc7e392da28da00c635a9ab9 Mon Sep 17 00:00:00 2001 From: Himanshu Date: Wed, 18 Nov 2020 10:47:33 +0530 Subject: [PATCH 59/71] fix(user defaults): Set user selected timezone in user defaults (#11902) Co-authored-by: Sahil Khan Co-authored-by: Sahil Khan --- frappe/core/doctype/user/user.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/frappe/core/doctype/user/user.py b/frappe/core/doctype/user/user.py index 2c5865fb69..7309528da6 100644 --- a/frappe/core/doctype/user/user.py +++ b/frappe/core/doctype/user/user.py @@ -13,7 +13,7 @@ from frappe.utils.user import get_system_managers from bs4 import BeautifulSoup import frappe.permissions import frappe.share - +import frappe.defaults from frappe.website.utils import is_signup_enabled from frappe.utils.background_jobs import enqueue @@ -107,6 +107,10 @@ class User(Document): ) if self.name not in ('Administrator', 'Guest') and not self.user_image: frappe.enqueue('frappe.core.doctype.user.user.update_gravatar', name=self.name) + + # Set user selected timezone + if self.time_zone: + frappe.defaults.set_default("time_zone", self.time_zone, self.name) def has_website_permission(self, ptype, user, verbose=False): """Returns true if current user is the session user""" @@ -1129,4 +1133,4 @@ def check_password_reset_limit(user, rate_limit): frappe.throw(_("You have reached the hourly limit for generating password reset links. Please try again later.")) def get_generated_link_count(user): - return cint(frappe.cache().hget("password_reset_link_count", user)) or 0 \ No newline at end of file + return cint(frappe.cache().hget("password_reset_link_count", user)) or 0 From cb6c704671dfe96063d3e59bef7e18f84430890a Mon Sep 17 00:00:00 2001 From: Saurabh Date: Mon, 2 Nov 2020 16:43:36 +0530 Subject: [PATCH 60/71] fix: Define chunk size based on backup file size to avoid timeout issues (#11526) Co-authored-by: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com> (cherry picked from commit 5020bbe4f68abaf5f9c019efff6105573d2ed5c0) # Conflicts: # frappe/integrations/offsite_backup_utils.py --- .../dropbox_settings/dropbox_settings.py | 5 ++-- frappe/integrations/offsite_backup_utils.py | 28 ++++++++++++++++++- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/frappe/integrations/doctype/dropbox_settings/dropbox_settings.py b/frappe/integrations/doctype/dropbox_settings/dropbox_settings.py index 6b95a3f5bf..71445b44d7 100644 --- a/frappe/integrations/doctype/dropbox_settings/dropbox_settings.py +++ b/frappe/integrations/doctype/dropbox_settings/dropbox_settings.py @@ -9,7 +9,7 @@ import frappe import os from frappe import _ from frappe.model.document import Document -from frappe.integrations.offsite_backup_utils import get_latest_backup_file, send_email, validate_file_size +from frappe.integrations.offsite_backup_utils import get_latest_backup_file, send_email, validate_file_size, get_chunk_site from frappe.integrations.utils import make_post_request from frappe.utils import (cint, get_request_site_address, get_files_path, get_backups_path, get_url, encode) @@ -167,8 +167,9 @@ def upload_file_to_dropbox(filename, folder, dropbox_client): return create_folder_if_not_exists(folder, dropbox_client) - chunk_size = 15 * 1024 * 1024 file_size = os.path.getsize(encode(filename)) + chunk_size = get_chunk_site(file_size) + mode = (dropbox.files.WriteMode.overwrite) f = open(encode(filename), 'rb') diff --git a/frappe/integrations/offsite_backup_utils.py b/frappe/integrations/offsite_backup_utils.py index db176538e4..39ffbd2c0e 100644 --- a/frappe/integrations/offsite_backup_utils.py +++ b/frappe/integrations/offsite_backup_utils.py @@ -6,7 +6,11 @@ from __future__ import unicode_literals import frappe import glob import os +<<<<<<< HEAD from frappe.utils import split_emails, get_backups_path +======= +from frappe.utils import split_emails, now_datetime, cint +>>>>>>> 5020bbe4f6... fix: Define chunk size based on backup file size to avoid timeout issues (#11526) def send_email(success, service_name, doctype, email_field, error_status=None): @@ -81,6 +85,22 @@ def get_file_size(file_path, unit): return file_size +def get_chunk_site(file_size): + ''' this function will return chunk size in megabytes based on file size ''' + + file_size_in_gb = cint(file_size/1024/1024) + + MB = 1024 * 1024 + if file_size_in_gb > 5000: + return 200 * MB + elif file_size_in_gb >= 3000: + return 150 * MB + elif file_size_in_gb >= 1000: + return 100 * MB + elif file_size_in_gb >= 500: + return 50 * MB + else: + return 15 * MB def validate_file_size(): frappe.flags.create_new_backup = True @@ -97,5 +117,11 @@ def generate_files_backup(): frappe.conf.db_password, db_host = frappe.db.host, db_type=frappe.conf.db_type, db_port=frappe.conf.db_port) +<<<<<<< HEAD backup.set_backup_file_name() - backup.zip_files() \ No newline at end of file + backup.zip_files() +======= + odb.todays_date = now_datetime().strftime('%Y%m%d_%H%M%S') + odb.set_backup_file_name() + odb.zip_files() +>>>>>>> 5020bbe4f6... fix: Define chunk size based on backup file size to avoid timeout issues (#11526) From 887f4b0d614ea05b77980ed6af8bfb73e62db3e0 Mon Sep 17 00:00:00 2001 From: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com> Date: Wed, 18 Nov 2020 13:54:57 +0530 Subject: [PATCH 61/71] fix: Resolve conflicts --- frappe/integrations/offsite_backup_utils.py | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/frappe/integrations/offsite_backup_utils.py b/frappe/integrations/offsite_backup_utils.py index 39ffbd2c0e..ebbeb41cce 100644 --- a/frappe/integrations/offsite_backup_utils.py +++ b/frappe/integrations/offsite_backup_utils.py @@ -6,12 +6,7 @@ from __future__ import unicode_literals import frappe import glob import os -<<<<<<< HEAD -from frappe.utils import split_emails, get_backups_path -======= -from frappe.utils import split_emails, now_datetime, cint ->>>>>>> 5020bbe4f6... fix: Define chunk size based on backup file size to avoid timeout issues (#11526) - +from frappe.utils import split_emails, get_backups_path, cint def send_email(success, service_name, doctype, email_field, error_status=None): recipients = get_recipients(doctype, email_field) @@ -117,11 +112,5 @@ def generate_files_backup(): frappe.conf.db_password, db_host = frappe.db.host, db_type=frappe.conf.db_type, db_port=frappe.conf.db_port) -<<<<<<< HEAD backup.set_backup_file_name() backup.zip_files() -======= - odb.todays_date = now_datetime().strftime('%Y%m%d_%H%M%S') - odb.set_backup_file_name() - odb.zip_files() ->>>>>>> 5020bbe4f6... fix: Define chunk size based on backup file size to avoid timeout issues (#11526) From 189c6cb7715156de759ce2d60310e328dd719789 Mon Sep 17 00:00:00 2001 From: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com> Date: Wed, 18 Nov 2020 14:01:14 +0530 Subject: [PATCH 62/71] style: Remove unused import --- frappe/integrations/offsite_backup_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/integrations/offsite_backup_utils.py b/frappe/integrations/offsite_backup_utils.py index ebbeb41cce..48a2c89107 100644 --- a/frappe/integrations/offsite_backup_utils.py +++ b/frappe/integrations/offsite_backup_utils.py @@ -6,7 +6,7 @@ from __future__ import unicode_literals import frappe import glob import os -from frappe.utils import split_emails, get_backups_path, cint +from frappe.utils import split_emails, cint def send_email(success, service_name, doctype, email_field, error_status=None): recipients = get_recipients(doctype, email_field) From edbb26d73edb0873785502556c1634467cc9d4b1 Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Fri, 20 Nov 2020 11:49:45 +0530 Subject: [PATCH 63/71] fix: display style removed from emails --- frappe/utils/html_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/utils/html_utils.py b/frappe/utils/html_utils.py index 302813645e..6fdd383eb9 100644 --- a/frappe/utils/html_utils.py +++ b/frappe/utils/html_utils.py @@ -34,7 +34,7 @@ def clean_email_html(html): 'margin', 'margin-top', 'margin-bottom', 'margin-left', 'margin-right', 'padding', 'padding-top', 'padding-bottom', 'padding-left', 'padding-right', 'font-size', 'font-weight', 'font-family', 'text-decoration', - 'line-height', 'text-align', 'vertical-align' + 'line-height', 'text-align', 'vertical-align', 'display' ], protocols=['cid', 'http', 'https', 'mailto', 'data'], strip=True, strip_comments=True) From 6bfe86d1272a7a9cc0585d17dc53e486654fec03 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Fri, 20 Nov 2020 17:40:20 +0100 Subject: [PATCH 64/71] feat: translate oauth confirmation dialog --- .../templates/includes/oauth_confirmation.html | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/frappe/templates/includes/oauth_confirmation.html b/frappe/templates/includes/oauth_confirmation.html index 73425af036..3fbbb75971 100644 --- a/frappe/templates/includes/oauth_confirmation.html +++ b/frappe/templates/includes/oauth_confirmation.html @@ -1,7 +1,7 @@ {% if not error %}
-

{{ client_id }} wants to access the following details from your account

+

{{ _("{} wants to access the following details from your account").format(client_id) }}

    @@ -11,10 +11,10 @@
  • - +
  • - +
@@ -22,24 +22,24 @@ {% else %}
-

Authorization error for {{ client_id }}

+

{{ _("Authorization error for {}.").format(client_id) }}

-

An unexpected error occurred while authorizing {{ client_id }}.

+

{{ _("An unexpected error occurred while authorizing {}.").format(client_id) }}

{{ error }}

  • - +
From 5a52bc73effc615f2720303d7b0c7fe30621af8b Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Fri, 20 Nov 2020 17:40:56 +0100 Subject: [PATCH 65/71] fix: cookie value --- frappe/oauth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/oauth.py b/frappe/oauth.py index bf225ac118..09af5ad809 100644 --- a/frappe/oauth.py +++ b/frappe/oauth.py @@ -148,7 +148,7 @@ class OAuthWebRequestValidator(RequestValidator): print("Failed body authentication: Application %s does not exist".format(cid=request.client_id)) cookie_dict = get_cookie_dict_from_headers(request) - user_id = unquote(cookie_dict['user_id']) if 'user_id' in cookie_dict else "Guest" + user_id = unquote(cookie_dict.get('user_id').value) if 'user_id' in cookie_dict else "Guest" return frappe.session.user == user_id def authenticate_client_id(self, client_id, request, *args, **kwargs): From 9fb635828fe900d8e0eef7fb6ae6e4735586bc03 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Fri, 20 Nov 2020 17:44:35 +0100 Subject: [PATCH 66/71] refactor: oauth2 --- frappe/integrations/oauth2.py | 120 +++++++++++++++++----------------- 1 file changed, 60 insertions(+), 60 deletions(-) diff --git a/frappe/integrations/oauth2.py b/frappe/integrations/oauth2.py index c8dfc52c95..59d0278d79 100644 --- a/frappe/integrations/oauth2.py +++ b/frappe/integrations/oauth2.py @@ -1,41 +1,50 @@ from __future__ import unicode_literals -import frappe, json -from frappe.oauth import OAuthWebRequestValidator, WebApplicationServer + +import hashlib +import json +import jwt +from werkzeug.urls import url_fix from oauthlib.oauth2 import FatalClientError, OAuth2Error -from werkzeug import url_fix from six.moves.urllib.parse import quote, urlencode, urlparse -from frappe.integrations.doctype.oauth_provider_settings.oauth_provider_settings import get_oauth_settings + +import frappe from frappe import _ +from frappe.oauth import OAuthWebRequestValidator, WebApplicationServer +from frappe.integrations.doctype.oauth_provider_settings.oauth_provider_settings import get_oauth_settings def get_oauth_server(): if not getattr(frappe.local, 'oauth_server', None): oauth_validator = OAuthWebRequestValidator() - frappe.local.oauth_server = WebApplicationServer(oauth_validator) + frappe.local.oauth_server = WebApplicationServer(oauth_validator) return frappe.local.oauth_server -def get_urlparams_from_kwargs(param_kwargs): +def sanitize_kwargs(param_kwargs): arguments = param_kwargs - if arguments.get("data"): - arguments.pop("data") - if arguments.get("cmd"): - arguments.pop("cmd") + arguments.pop('data', None) + arguments.pop('cmd', None) - return urlencode(arguments) + return arguments @frappe.whitelist() def approve(*args, **kwargs): r = frappe.request - uri = url_fix(r.url.replace("+"," ")) - http_method = r.method - body = r.get_data() - headers = r.headers try: - scopes, frappe.flags.oauth_credentials = get_oauth_server().validate_authorization_request(uri, http_method, body, headers) + scopes, frappe.flags.oauth_credentials = get_oauth_server().validate_authorization_request( + r.url, + r.method, + r.get_data(), + r.headers + ) - headers, body, status = get_oauth_server().create_authorization_response(uri=frappe.flags.oauth_credentials['redirect_uri'], \ - body=body, headers=headers, scopes=scopes, credentials=frappe.flags.oauth_credentials) + headers, body, status = get_oauth_server().create_authorization_response( + uri=frappe.flags.oauth_credentials['redirect_uri'], + body=r.get_data(), + headers=r.headers, + scopes=scopes, + credentials=frappe.flags.oauth_credentials + ) uri = headers.get('Location', None) frappe.local.response["type"] = "redirect" @@ -47,34 +56,28 @@ def approve(*args, **kwargs): return e @frappe.whitelist(allow_guest=True) -def authorize(*args, **kwargs): - #Fetch provider URL from settings - oauth_settings = get_oauth_settings() - params = get_urlparams_from_kwargs(kwargs) - request_url = urlparse(frappe.request.url) - success_url = request_url.scheme + "://" + request_url.netloc + "/api/method/frappe.integrations.oauth2.approve?" + params +def authorize(**kwargs): + success_url = "/api/method/frappe.integrations.oauth2.approve?" + urlencode(sanitize_kwargs(kwargs)) failure_url = frappe.form_dict["redirect_uri"] + "?error=access_denied" - if frappe.session['user']=='Guest': + if frappe.session.user == 'Guest': #Force login, redirect to preauth again. frappe.local.response["type"] = "redirect" - frappe.local.response["location"] = "/login?redirect-to=/api/method/frappe.integrations.oauth2.authorize?" + quote(params.replace("+"," ")) - - elif frappe.session['user']!='Guest': + frappe.local.response["location"] = "/login?" + urlencode({'redirect-to': frappe.request.url}) + else: try: r = frappe.request - uri = url_fix(r.url) - http_method = r.method - body = r.get_data() - headers = r.headers - - scopes, frappe.flags.oauth_credentials = get_oauth_server().validate_authorization_request(uri, http_method, body, headers) + scopes, frappe.flags.oauth_credentials = get_oauth_server().validate_authorization_request( + r.url, + r.method, + r.get_data(), + r.headers + ) skip_auth = frappe.db.get_value("OAuth Client", frappe.flags.oauth_credentials['client_id'], "skip_authorization") unrevoked_tokens = frappe.get_all("OAuth Bearer Token", filters={"status":"Active"}) - if skip_auth or (oauth_settings["skip_authorization"] == "Auto" and len(unrevoked_tokens)): - + if skip_auth or (get_oauth_settings().skip_authorization == "Auto" and unrevoked_tokens): frappe.local.response["type"] = "redirect" frappe.local.response["location"] = success_url else: @@ -87,7 +90,6 @@ def authorize(*args, **kwargs): }) resp_html = frappe.render_template("templates/includes/oauth_confirmation.html", response_html_params) frappe.respond_as_web_page("Confirm Access", resp_html) - except FatalClientError as e: return e except OAuth2Error as e: @@ -95,20 +97,20 @@ def authorize(*args, **kwargs): @frappe.whitelist(allow_guest=True) def get_token(*args, **kwargs): - r = frappe.request - - uri = url_fix(r.url) - http_method = r.method - body = r.form - headers = r.headers - #Check whether frappe server URL is set frappe_server_url = frappe.db.get_value("Social Login Key", "frappe", "base_url") or None if not frappe_server_url: frappe.throw(_("Please set Base URL in Social Login Key for Frappe")) try: - headers, body, status = get_oauth_server().create_token_response(uri, http_method, body, headers, frappe.flags.oauth_credentials) + r = frappe.request + headers, body, status = get_oauth_server().create_token_response( + r.url, + r.method, + r.form, + r.headers, + frappe.flags.oauth_credentials + ) out = frappe._dict(json.loads(body)) if not out.error and "openid" in out.scope: token_user = frappe.db.get_value("OAuth Bearer Token", out.access_token, "user") @@ -116,7 +118,7 @@ def get_token(*args, **kwargs): client_secret = frappe.db.get_value("OAuth Client", token_client, "client_secret") if token_user in ["Guest", "Administrator"]: frappe.throw(_("Logged in as Guest or Administrator")) - import hashlib + id_token_header = { "typ":"jwt", "alg":"HS256" @@ -128,9 +130,10 @@ def get_token(*args, **kwargs): "iss": frappe_server_url, "at_hash": frappe.oauth.calculate_at_hash(out.access_token, hashlib.sha256) } - import jwt + id_token_encoded = jwt.encode(id_token, client_secret, algorithm='HS256', headers=id_token_header) - out.update({"id_token":str(id_token_encoded)}) + out.update({"id_token": str(id_token_encoded)}) + frappe.local.response = out except FatalClientError as e: @@ -140,12 +143,12 @@ def get_token(*args, **kwargs): @frappe.whitelist(allow_guest=True) def revoke_token(*args, **kwargs): r = frappe.request - uri = url_fix(r.url) - http_method = r.method - body = r.form - headers = r.headers - - headers, body, status = get_oauth_server().create_revocation_response(uri, headers=headers, body=body, http_method=http_method) + headers, body, status = get_oauth_server().create_revocation_response( + r.url, + headers=r.headers, + body=r.form, + http_method=r.method + ) frappe.local.response['http_status_code'] = status if status == 200: @@ -174,15 +177,12 @@ def openid_profile(*args, **kwargs): "email": name, "picture": picture }) - + frappe.local.response = user_profile def validate_url(url_string): try: result = urlparse(url_string) - if result.scheme and result.scheme in ["http", "https", "ftp", "ftps"]: - return True - else: - return False + return result.scheme and result.scheme in ["http", "https", "ftp", "ftps"] except: - return False \ No newline at end of file + return False From bea4f6e11b5058c7aa457141f2fd0aa59b27615b Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Fri, 20 Nov 2020 18:03:17 +0100 Subject: [PATCH 67/71] fix: remove unused import --- frappe/integrations/oauth2.py | 1 - 1 file changed, 1 deletion(-) diff --git a/frappe/integrations/oauth2.py b/frappe/integrations/oauth2.py index 59d0278d79..7570a50127 100644 --- a/frappe/integrations/oauth2.py +++ b/frappe/integrations/oauth2.py @@ -3,7 +3,6 @@ from __future__ import unicode_literals import hashlib import json import jwt -from werkzeug.urls import url_fix from oauthlib.oauth2 import FatalClientError, OAuth2Error from six.moves.urllib.parse import quote, urlencode, urlparse From e93a38f912d8fb10f4610173d218508b7a4459b7 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Fri, 20 Nov 2020 19:02:33 +0100 Subject: [PATCH 68/71] refactor: move encode_params from test to oauth2.py --- frappe/integrations/oauth2.py | 17 ++++++++++++++--- frappe/tests/test_oauth20.py | 11 +---------- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/frappe/integrations/oauth2.py b/frappe/integrations/oauth2.py index 7570a50127..a750c8328c 100644 --- a/frappe/integrations/oauth2.py +++ b/frappe/integrations/oauth2.py @@ -2,9 +2,10 @@ from __future__ import unicode_literals import hashlib import json +from urllib.parse import quote, urlencode, urlparse + import jwt from oauthlib.oauth2 import FatalClientError, OAuth2Error -from six.moves.urllib.parse import quote, urlencode, urlparse import frappe from frappe import _ @@ -56,13 +57,13 @@ def approve(*args, **kwargs): @frappe.whitelist(allow_guest=True) def authorize(**kwargs): - success_url = "/api/method/frappe.integrations.oauth2.approve?" + urlencode(sanitize_kwargs(kwargs)) + success_url = "/api/method/frappe.integrations.oauth2.approve?" + encode_params(sanitize_kwargs(kwargs)) failure_url = frappe.form_dict["redirect_uri"] + "?error=access_denied" if frappe.session.user == 'Guest': #Force login, redirect to preauth again. frappe.local.response["type"] = "redirect" - frappe.local.response["location"] = "/login?" + urlencode({'redirect-to': frappe.request.url}) + frappe.local.response["location"] = "/login?" + encode_params({'redirect-to': frappe.request.url}) else: try: r = frappe.request @@ -185,3 +186,13 @@ def validate_url(url_string): return result.scheme and result.scheme in ["http", "https", "ftp", "ftps"] except: return False + +def encode_params(params): + """ + Encode a dict of params into a query string. + + Use `quote_via=urllib.parse.quote` so that whitespaces will be encoded as + `%20` instead of as `+`. This is needed because oauthlib cannot handle `+` + as a whitespace. + """ + return urlencode(params, quote_via=quote) diff --git a/frappe/tests/test_oauth20.py b/frappe/tests/test_oauth20.py index f4ecc8a68d..e2213145b7 100644 --- a/frappe/tests/test_oauth20.py +++ b/frappe/tests/test_oauth20.py @@ -6,6 +6,7 @@ import unittest, frappe, requests, time from frappe.test_runner import make_test_records from six.moves.urllib.parse import urlparse, parse_qs, urljoin from urllib.parse import urlencode, quote +from frappe.integrations.oauth2 import encode_params class TestOAuth20(unittest.TestCase): @@ -232,13 +233,3 @@ def login(session): def get_full_url(endpoint): """Turn '/endpoint' into 'http://127.0.0.1:8000/endpoint'.""" return urljoin(frappe.utils.get_url(), endpoint) - -def encode_params(params): - """ - Encode a dict of params into a query string. - - Use `quote_via=urllib.parse.quote` so that whitespaces will be encoded as - `%20` instead of as `+`. This is needed because oauthlib cannot handle `+` - as a whitespace. - """ - return urlencode(params, quote_via=quote) From 0312b9d67cd9ab45cf4f7e342e100dbb87ad1d26 Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Tue, 17 Nov 2020 19:20:44 +0530 Subject: [PATCH 69/71] feat: check if auto_repeat field is already present --- .../doctype/customize_form/customize_form.py | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/frappe/custom/doctype/customize_form/customize_form.py b/frappe/custom/doctype/customize_form/customize_form.py index cf674082ab..67b4f05856 100644 --- a/frappe/custom/doctype/customize_form/customize_form.py +++ b/frappe/custom/doctype/customize_form/customize_form.py @@ -76,17 +76,20 @@ class CustomizeForm(Document): def create_auto_repeat_custom_field_if_requried(self, meta): if self.allow_auto_repeat: - if not frappe.db.exists('Custom Field', {'fieldname': 'auto_repeat', 'dt': self.doc_type}) and \ - not frappe.db.exists('DocField', {'fieldname': 'auto_repeat', 'parent': self.name}): - insert_after = self.fields[len(self.fields) - 1].fieldname - df = dict( - fieldname='auto_repeat', - label='Auto Repeat', - fieldtype='Link', - options='Auto Repeat', - insert_after=insert_after, - read_only=1, no_copy=1, print_hide=1) - create_custom_field(self.doc_type, df) + all_fields = [df.fieldname for df in meta.fields] + + if "auto_repeat" in all_fields: + return + + insert_after = self.fields[len(self.fields) - 1].fieldname + create_custom_field(self.doc_type, dict( + fieldname='auto_repeat', + label='Auto Repeat', + fieldtype='Link', + options='Auto Repeat', + insert_after=insert_after, + read_only=1, no_copy=1, print_hide=1 + )) def get_name_translation(self): From 019fca9ef773e006a57069b8137a1043593da174 Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Mon, 23 Nov 2020 11:33:16 +0530 Subject: [PATCH 70/71] fix: typo in function name --- frappe/custom/doctype/customize_form/customize_form.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/custom/doctype/customize_form/customize_form.py b/frappe/custom/doctype/customize_form/customize_form.py index 67b4f05856..60ae65091d 100644 --- a/frappe/custom/doctype/customize_form/customize_form.py +++ b/frappe/custom/doctype/customize_form/customize_form.py @@ -39,7 +39,7 @@ class CustomizeForm(Document): translation = self.get_name_translation() self.label = translation.translated_text if translation else '' - self.create_auto_repeat_custom_field_if_requried(meta) + self.create_auto_repeat_custom_field_if_required(meta) # NOTE doc (self) is sent to clientside by run_method @@ -74,7 +74,7 @@ class CustomizeForm(Document): for d in meta.get(fieldname): self.append(fieldname, d) - def create_auto_repeat_custom_field_if_requried(self, meta): + def create_auto_repeat_custom_field_if_required(self, meta): if self.allow_auto_repeat: all_fields = [df.fieldname for df in meta.fields] From 44413e9ba6010a88b25114c67fb12275f0de803f Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Mon, 23 Nov 2020 11:34:49 +0530 Subject: [PATCH 71/71] chore: add docstrings --- frappe/custom/doctype/customize_form/customize_form.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/frappe/custom/doctype/customize_form/customize_form.py b/frappe/custom/doctype/customize_form/customize_form.py index 60ae65091d..82513783c7 100644 --- a/frappe/custom/doctype/customize_form/customize_form.py +++ b/frappe/custom/doctype/customize_form/customize_form.py @@ -75,6 +75,9 @@ class CustomizeForm(Document): self.append(fieldname, d) def create_auto_repeat_custom_field_if_required(self, meta): + ''' + Create auto repeat custom field if it's not already present + ''' if self.allow_auto_repeat: all_fields = [df.fieldname for df in meta.fields]