public inbox for [email protected]help / color / mirror / Atom feed
[pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file. 7+ messages / 3 participants [nested] [flat]
* [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file. @ 2021-06-03 06:39 Nikhil Mohite <[email protected]> 2021-06-03 08:17 ` Re: [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file. Dave Page <[email protected]> 0 siblings, 1 reply; 7+ messages in thread From: Nikhil Mohite @ 2021-06-03 06:39 UTC (permalink / raw) To: pgadmin-hackers Hi Hackers, Please find the attached patch for RM-6460 <https://redmine.postgresql.org/issues/6460;: Need a mechanism to detect a corrupt/broken config DB file. 1. Added checks if all tables added in the model are present in SQLite DB or not. 2. If migrations fail it will backup older file and try migrations with the newly created file. (User will get notification on UI for the location of the backup file and newly created.) 3. If the user deleted any table from SQLite DB pgAdmin will not run on the next restart and it will add the missing table list in the logs. -- *Thanks & Regards,* *Nikhil Mohite* *Software Engineer.* *EDB Postgres* <https://www.enterprisedb.com/; *Mob.No: +91-7798364578.* Attachments: [application/octet-stream] RM_6460.patch (9.4K, 3-RM_6460.patch) download | inline diff: diff --git a/web/pgadmin/__init__.py b/web/pgadmin/__init__.py index 6e395f42..9aa47d0d 100644 --- a/web/pgadmin/__init__.py +++ b/web/pgadmin/__init__.py @@ -14,6 +14,8 @@ import os import sys import re import ipaddress +import traceback + from types import MethodType from collections import defaultdict from importlib import import_module @@ -38,8 +40,8 @@ from pgadmin.utils import PgAdminModule, driver, KeyManager from pgadmin.utils.preferences import Preferences from pgadmin.utils.session import create_session_interface, pga_unauthorised from pgadmin.utils.versioned_template_loader import VersionedTemplateLoader -from datetime import timedelta -from pgadmin.setup import get_version, set_version +from datetime import timedelta, datetime +from pgadmin.setup import get_version, set_version, check_db_tables from pgadmin.utils.ajax import internal_server_error, make_json_response from pgadmin.utils.csrf import pgCSRFProtect from pgadmin import authenticate @@ -356,18 +358,48 @@ def create_app(app_name=None): # If version not available, user must have aborted. Tables are not # created and so its an empty db - if not os.path.exists(SQLITE_PATH) or get_version() == -1: + version = get_version() + if not os.path.exists(SQLITE_PATH) or version == -1: # If running in cli mode then don't try to upgrade, just raise # the exception if not cli_mode: - db_upgrade(app) + try: + db_upgrade(app) + os.environ['CORRUPTED_DB_BACKUP_FILE'] = '' + except Exception as e: + backup_file_name = "{0}.{1}".format( + SQLITE_PATH, datetime.now().strftime('%Y%m%d%H%M%S')) + os.rename(SQLITE_PATH, backup_file_name) + app.logger.error('Exception in database migration: ' + '{0}'.format(e)) + app.logger.info('Creating new database file.') + try: + db_upgrade(app) + os.environ[ + 'CORRUPTED_DB_BACKUP_FILE'] = backup_file_name + app.logger.info('Database migration completed.') + except Exception as e: + app.logger.error('Database migration failed') + app.logger.error(traceback.format_exc()) + raise RuntimeError('Migration failed') + + # check all tables are present in the db. + is_db_error, invalid_tb_names = check_db_tables() + if is_db_error: + app.logger.error( + 'Table(s) {0} not present in the' + ' db'.format(invalid_tb_names)) + raise RuntimeError( + 'Exception in database tables, Table(s) {0} ' + 'not present'.format(invalid_tb_names)) + else: if not os.path.exists(SQLITE_PATH): raise FileNotFoundError( 'SQLite database file "' + SQLITE_PATH + '" does not exists.') - raise RuntimeError('Specified SQLite database file ' - 'is not valid.') + raise RuntimeError( + 'Specified SQLite database file is not valid.') else: schema_version = get_version() @@ -376,6 +408,17 @@ def create_app(app_name=None): if CURRENT_SCHEMA_VERSION >= schema_version: db_upgrade(app) + # check all tables are present in the db. + is_db_error, invalid_tb_names = check_db_tables() + if is_db_error: + app.logger.error( + 'Table(s) "{0}" not present in the ' + 'database'.format(invalid_tb_names)) + raise RuntimeError( + 'Exception in database tables, Table(s) "{0}" not present' + ' in the database.'.format( + invalid_tb_names)) + # Update schema version to the latest if CURRENT_SCHEMA_VERSION > schema_version: set_version(CURRENT_SCHEMA_VERSION) diff --git a/web/pgadmin/browser/__init__.py b/web/pgadmin/browser/__init__.py index edbd491e..5a6a1c46 100644 --- a/web/pgadmin/browser/__init__.py +++ b/web/pgadmin/browser/__init__.py @@ -342,6 +342,7 @@ class BrowserModule(PgAdminModule): list: a list of url endpoints exposed to the client. """ return [BROWSER_INDEX, 'browser.nodes', + 'browser.check_corrupted_db_file', 'browser.check_master_password', 'browser.set_master_password', 'browser.reset_master_password', @@ -950,6 +951,19 @@ def form_master_password_response(existing=True, present=False, errmsg=None): }) [email protected]("/check_corrupted_db_file", + endpoint="check_corrupted_db_file", methods=["GET"]) +def check_corrupted_db_file(): + """ + Get the corrupted database file path. + """ + file_location = os.environ['CORRUPTED_DB_BACKUP_FILE'] \ + if 'CORRUPTED_DB_BACKUP_FILE' in os.environ else '' + # reset the corrupted db file path in env. + os.environ['CORRUPTED_DB_BACKUP_FILE'] = '' + return make_json_response(data=file_location) + + @blueprint.route("/master_password", endpoint="check_master_password", methods=["GET"]) def check_master_password(): diff --git a/web/pgadmin/browser/static/js/browser.js b/web/pgadmin/browser/static/js/browser.js index 46dfb4f1..ba84ca41 100644 --- a/web/pgadmin/browser/static/js/browser.js +++ b/web/pgadmin/browser/static/js/browser.js @@ -594,7 +594,7 @@ define('pgadmin.browser', [ }, 300000); obj.set_master_password(''); - + obj.check_corrupted_db_file(); obj.Events.on('pgadmin:browser:tree:add', obj.onAddTreeNode, obj); obj.Events.on('pgadmin:browser:tree:update', obj.onUpdateTreeNode, obj); obj.Events.on('pgadmin:browser:tree:refresh', obj.onRefreshTreeNode, obj); @@ -607,7 +607,35 @@ define('pgadmin.browser', [ obj.register_to_activity_listener(document); obj.start_inactivity_timeout_daemon(); }, + check_corrupted_db_file: function() { + $.ajax({ + url: url_for('browser.check_corrupted_db_file'), + type: 'GET', + dataType: 'json', + contentType: 'application/json', + }).done((res)=> { + if(res.data.length > 0) { + + Alertify.alert( + 'Warning', + 'pgAdmin detected unrecoverable corruption in it\'s SQLite configuration database. ' + + 'The database has been backed up and recreated with default settings. '+ + 'It may be possible to recover data such as query history manually from '+ + 'the original/corrupt file using a tool such as DB Browser for SQLite if desired.'+ + '<br><br>Original file: ' + res.data + '<br>Replacement file: ' + + res.data.substring(0, res.data.length - 14) + ) + .set({'closable': true, + 'onok': function() { + }, + }); + + } + }).fail(function(xhr, status, error) { + Alertify.alert(error); + }); + }, init_master_password: function() { let self = this; // Master password dialog diff --git a/web/pgadmin/setup/__init__.py b/web/pgadmin/setup/__init__.py index 04571e9e..779a7217 100644 --- a/web/pgadmin/setup/__init__.py +++ b/web/pgadmin/setup/__init__.py @@ -11,3 +11,4 @@ from .user_info import user_info from .db_version import get_version, set_version from .db_upgrade import db_upgrade from .data_directory import create_app_data_directory +from .db_table_check import check_db_tables diff --git a/web/pgadmin/setup/db_table_check.py b/web/pgadmin/setup/db_table_check.py new file mode 100644 index 00000000..4da86c27 --- /dev/null +++ b/web/pgadmin/setup/db_table_check.py @@ -0,0 +1,32 @@ +########################################################################## +# +# pgAdmin 4 - PostgreSQL Tools +# +# Copyright (C) 2013 - 2021, The pgAdmin Development Team +# This software is released under the PostgreSQL Licence +# +########################################################################## + +from pgadmin.model import Version +from pgadmin.model import db + + +def get_db_table_names(): + db_table_names = db.metadata.tables.keys() if db.metadata.tables else 0 + return db_table_names + + +def check_db_tables(): + is_error = False + invalid_tb_names = list() + db_table_names = get_db_table_names() + # check table is actually present in the db. + for table_name in db_table_names: + if not db.engine.dialect.has_table(db.engine, table_name): + invalid_tb_names.append(table_name) + is_error = True + + if is_error: + return True, invalid_tb_names + else: + return False, invalid_tb_names diff --git a/web/pgadmin/setup/db_version.py b/web/pgadmin/setup/db_version.py index ad2811a1..50607583 100644 --- a/web/pgadmin/setup/db_version.py +++ b/web/pgadmin/setup/db_version.py @@ -16,7 +16,10 @@ def get_version(): except Exception: return -1 - return version.value + if version: + return version.value + else: + return -1 def set_version(new_version): ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file. 2021-06-03 06:39 [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file. Nikhil Mohite <[email protected]> @ 2021-06-03 08:17 ` Dave Page <[email protected]> 2021-06-03 09:01 ` Re: [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file. Nikhil Mohite <[email protected]> 0 siblings, 1 reply; 7+ messages in thread From: Dave Page @ 2021-06-03 08:17 UTC (permalink / raw) To: Nikhil Mohite <[email protected]>; +Cc: pgadmin-hackers Hi On Thu, Jun 3, 2021 at 7:39 AM Nikhil Mohite <[email protected]> wrote: > Hi Hackers, > > Please find the attached patch for RM-6460 > <https://redmine.postgresql.org/issues/6460;: Need a mechanism to detect > a corrupt/broken config DB file. > > 1. Added checks if all tables added in the model are present in SQLite DB > or not. > 2. If migrations fail it will backup older file and try migrations with > the newly created file. > (User will get notification on UI for the location of the backup file and > newly created.) > 3. If the user deleted any table from SQLite DB pgAdmin will not run on > the next restart and it will add the missing table list in the logs. > Surely if any tables have been deleted, it'll fail the check in point 1? -- Dave Page Blog: https://pgsnake.blogspot.com Twitter: @pgsnake EDB: https://www.enterprisedb.com ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file. 2021-06-03 06:39 [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file. Nikhil Mohite <[email protected]> 2021-06-03 08:17 ` Re: [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file. Dave Page <[email protected]> @ 2021-06-03 09:01 ` Nikhil Mohite <[email protected]> 2021-06-03 09:19 ` Re: [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file. Dave Page <[email protected]> 0 siblings, 1 reply; 7+ messages in thread From: Nikhil Mohite @ 2021-06-03 09:01 UTC (permalink / raw) To: Dave Page <[email protected]>; +Cc: pgadmin-hackers Hi Dave, On Thu, Jun 3, 2021 at 1:47 PM Dave Page <[email protected]> wrote: > Hi > > On Thu, Jun 3, 2021 at 7:39 AM Nikhil Mohite < > [email protected]> wrote: > >> Hi Hackers, >> >> Please find the attached patch for RM-6460 >> <https://redmine.postgresql.org/issues/6460;: Need a mechanism to >> detect a corrupt/broken config DB file. >> >> 1. Added checks if all tables added in the model are present in SQLite DB >> or not. >> 2. If migrations fail it will backup older file and try migrations with >> the newly created file. >> (User will get notification on UI for the location of the backup file and >> newly created.) >> 3. If the user deleted any table from SQLite DB pgAdmin will not run on >> the next restart and it will add the missing table list in the logs. >> > > Surely if any tables have been deleted, it'll fail the check in point 1? > Yes, but if the user deletes any table while pgAdmin is running then it will fail when the user tries to run pgAdmin next time. (If we remove the table from the model it will not check particular table is present in DB or not. ) > > -- > Dave Page > Blog: https://pgsnake.blogspot.com > Twitter: @pgsnake > > EDB: https://www.enterprisedb.com > > Regards, Nikhil Mohite. ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file. 2021-06-03 06:39 [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file. Nikhil Mohite <[email protected]> 2021-06-03 08:17 ` Re: [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file. Dave Page <[email protected]> 2021-06-03 09:01 ` Re: [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file. Nikhil Mohite <[email protected]> @ 2021-06-03 09:19 ` Dave Page <[email protected]> 2021-06-03 09:40 ` Re: [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file. Nikhil Mohite <[email protected]> 0 siblings, 1 reply; 7+ messages in thread From: Dave Page @ 2021-06-03 09:19 UTC (permalink / raw) To: Nikhil Mohite <[email protected]>; +Cc: pgadmin-hackers Hi On Thu, Jun 3, 2021 at 10:01 AM Nikhil Mohite < [email protected]> wrote: > Hi Dave, > > On Thu, Jun 3, 2021 at 1:47 PM Dave Page <[email protected]> wrote: > >> Hi >> >> On Thu, Jun 3, 2021 at 7:39 AM Nikhil Mohite < >> [email protected]> wrote: >> >>> Hi Hackers, >>> >>> Please find the attached patch for RM-6460 >>> <https://redmine.postgresql.org/issues/6460;: Need a mechanism to >>> detect a corrupt/broken config DB file. >>> >>> 1. Added checks if all tables added in the model are present in SQLite >>> DB or not. >>> 2. If migrations fail it will backup older file and try migrations with >>> the newly created file. >>> (User will get notification on UI for the location of the backup file >>> and newly created.) >>> 3. If the user deleted any table from SQLite DB pgAdmin will not run on >>> the next restart and it will add the missing table list in the logs. >>> >> >> Surely if any tables have been deleted, it'll fail the check in point 1? >> > Yes, but if the user deletes any table while pgAdmin is running then it > will fail when the user tries to run pgAdmin next time. > Right - but how is the end result of that different from a failed migration that left a table missing? Either way, the end result is the same, and should be handled the same way; i.e. backup/recreate the DB, then warn the user as soon as the UI is up. I believe the process is simple (and maybe this is what is done and this is just a language issue - I haven't read the patch in detail): - On startup, attempt to create the database/run any migrations as appropriate. - Check that all tables exist, and the schema version is correct. - If there's a problem, backup the file, create a new one from scratch, and warn the user. One thing I did notice when skimming the patch - we have this in the code: + raise RuntimeError( + 'Specified SQLite database file is not valid.') The *user* didn't specify a SQLite database file (nor do they care that it's SQLite at this point). That (and any other similar messages) should probably be rephrased to say something like: "The configuration database file is not valid." Thanks! > (If we remove the table from the model it will not check particular table > is present in DB or not. ) > >> >> -- >> Dave Page >> Blog: https://pgsnake.blogspot.com >> Twitter: @pgsnake >> >> EDB: https://www.enterprisedb.com >> >> Regards, > Nikhil Mohite. > -- Dave Page Blog: https://pgsnake.blogspot.com Twitter: @pgsnake EDB: https://www.enterprisedb.com ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file. 2021-06-03 06:39 [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file. Nikhil Mohite <[email protected]> 2021-06-03 08:17 ` Re: [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file. Dave Page <[email protected]> 2021-06-03 09:01 ` Re: [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file. Nikhil Mohite <[email protected]> 2021-06-03 09:19 ` Re: [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file. Dave Page <[email protected]> @ 2021-06-03 09:40 ` Nikhil Mohite <[email protected]> 2021-06-03 10:21 ` Re: [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file. Nikhil Mohite <[email protected]> 0 siblings, 1 reply; 7+ messages in thread From: Nikhil Mohite @ 2021-06-03 09:40 UTC (permalink / raw) To: Dave Page <[email protected]>; +Cc: pgadmin-hackers Hi Dave, On Thu, Jun 3, 2021 at 2:49 PM Dave Page <[email protected]> wrote: > Hi > > On Thu, Jun 3, 2021 at 10:01 AM Nikhil Mohite < > [email protected]> wrote: > >> Hi Dave, >> >> On Thu, Jun 3, 2021 at 1:47 PM Dave Page <[email protected]> wrote: >> >>> Hi >>> >>> On Thu, Jun 3, 2021 at 7:39 AM Nikhil Mohite < >>> [email protected]> wrote: >>> >>>> Hi Hackers, >>>> >>>> Please find the attached patch for RM-6460 >>>> <https://redmine.postgresql.org/issues/6460;: Need a mechanism to >>>> detect a corrupt/broken config DB file. >>>> >>>> 1. Added checks if all tables added in the model are present in SQLite >>>> DB or not. >>>> 2. If migrations fail it will backup older file and try migrations with >>>> the newly created file. >>>> (User will get notification on UI for the location of the backup file >>>> and newly created.) >>>> 3. If the user deleted any table from SQLite DB pgAdmin will not run on >>>> the next restart and it will add the missing table list in the logs. >>>> >>> >>> Surely if any tables have been deleted, it'll fail the check in point 1? >>> >> Yes, but if the user deletes any table while pgAdmin is running then it >> will fail when the user tries to run pgAdmin next time. >> > > Right - but how is the end result of that different from a failed > migration that left a table missing? Either way, the end result is the > same, and should be handled the same way; i.e. backup/recreate the DB, then > warn the user as soon as the UI is up. > Yes - It should be the same in both conditions, I will send an updated patch for this. > > > I believe the process is simple (and maybe this is what is done and this > is just a language issue - I haven't read the patch in detail): > > - On startup, attempt to create the database/run any migrations as > appropriate. > - Check that all tables exist, and the schema version is correct. > - If there's a problem, backup the file, create a new one from scratch, > and warn the user. > > One thing I did notice when skimming the patch - we have this in the code: > > + raise RuntimeError( > + 'Specified SQLite database file is not valid.') > > The *user* didn't specify a SQLite database file (nor do they care that > it's SQLite at this point). That (and any other similar messages) should > probably be rephrased to say something like: > > "The configuration database file is not valid." > > Thanks! > > > >> (If we remove the table from the model it will not check particular table >> is present in DB or not. ) >> >>> >>> -- >>> Dave Page >>> Blog: https://pgsnake.blogspot.com >>> Twitter: @pgsnake >>> >>> EDB: https://www.enterprisedb.com >>> >>> Regards, >> Nikhil Mohite. >> > > > -- > Dave Page > Blog: https://pgsnake.blogspot.com > Twitter: @pgsnake > > EDB: https://www.enterprisedb.com > > Regards, Nikhil Mohite ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file. 2021-06-03 06:39 [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file. Nikhil Mohite <[email protected]> 2021-06-03 08:17 ` Re: [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file. Dave Page <[email protected]> 2021-06-03 09:01 ` Re: [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file. Nikhil Mohite <[email protected]> 2021-06-03 09:19 ` Re: [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file. Dave Page <[email protected]> 2021-06-03 09:40 ` Re: [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file. Nikhil Mohite <[email protected]> @ 2021-06-03 10:21 ` Nikhil Mohite <[email protected]> 2021-06-08 14:27 ` Re: [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file. Akshay Joshi <[email protected]> 0 siblings, 1 reply; 7+ messages in thread From: Nikhil Mohite @ 2021-06-03 10:21 UTC (permalink / raw) To: pgadmin-hackers; +Cc: Dave Page <[email protected]> Hi Team, PFA updated patch (v2) On Thu, Jun 3, 2021 at 3:10 PM Nikhil Mohite <[email protected]> wrote: > Hi Dave, > > On Thu, Jun 3, 2021 at 2:49 PM Dave Page <[email protected]> wrote: > >> Hi >> >> On Thu, Jun 3, 2021 at 10:01 AM Nikhil Mohite < >> [email protected]> wrote: >> >>> Hi Dave, >>> >>> On Thu, Jun 3, 2021 at 1:47 PM Dave Page <[email protected]> wrote: >>> >>>> Hi >>>> >>>> On Thu, Jun 3, 2021 at 7:39 AM Nikhil Mohite < >>>> [email protected]> wrote: >>>> >>>>> Hi Hackers, >>>>> >>>>> Please find the attached patch for RM-6460 >>>>> <https://redmine.postgresql.org/issues/6460;: Need a mechanism to >>>>> detect a corrupt/broken config DB file. >>>>> >>>>> 1. Added checks if all tables added in the model are present in SQLite >>>>> DB or not. >>>>> 2. If migrations fail it will backup older file and try migrations >>>>> with the newly created file. >>>>> (User will get notification on UI for the location of the backup file >>>>> and newly created.) >>>>> 3. If the user deleted any table from SQLite DB pgAdmin will not run >>>>> on the next restart and it will add the missing table list in the logs. >>>>> >>>> >>>> Surely if any tables have been deleted, it'll fail the check in point 1? >>>> >>> Yes, but if the user deletes any table while pgAdmin is running then it >>> will fail when the user tries to run pgAdmin next time. >>> >> >> Right - but how is the end result of that different from a failed >> migration that left a table missing? Either way, the end result is the >> same, and should be handled the same way; i.e. backup/recreate the DB, then >> warn the user as soon as the UI is up. >> > Yes - It should be the same in both conditions, I will send an updated > patch for this. > >> > >> I believe the process is simple (and maybe this is what is done and this >> is just a language issue - I haven't read the patch in detail): >> >> - On startup, attempt to create the database/run any migrations as >> appropriate. >> - Check that all tables exist, and the schema version is correct. >> - If there's a problem, backup the file, create a new one from scratch, >> and warn the user. >> >> One thing I did notice when skimming the patch - we have this in the code: >> >> + raise RuntimeError( >> + 'Specified SQLite database file is not valid.') >> >> The *user* didn't specify a SQLite database file (nor do they care that >> it's SQLite at this point). That (and any other similar messages) should >> probably be rephrased to say something like: >> >> "The configuration database file is not valid." >> > >> Thanks! >> >> >> >>> (If we remove the table from the model it will not check particular >>> table is present in DB or not. ) >>> >>>> >>>> -- >>>> Dave Page >>>> Blog: https://pgsnake.blogspot.com >>>> Twitter: @pgsnake >>>> >>>> EDB: https://www.enterprisedb.com >>>> >>>> Regards, >>> Nikhil Mohite. >>> >> >> >> -- >> Dave Page >> Blog: https://pgsnake.blogspot.com >> Twitter: @pgsnake >> >> EDB: https://www.enterprisedb.com >> >> Regards, > Nikhil Mohite > Regards, Nikhil Mohite Attachments: [application/octet-stream] RM_6460_v2.patch (9.6K, 3-RM_6460_v2.patch) download | inline diff: diff --git a/web/pgadmin/__init__.py b/web/pgadmin/__init__.py index 6e395f42..d0c4a25e 100644 --- a/web/pgadmin/__init__.py +++ b/web/pgadmin/__init__.py @@ -14,6 +14,8 @@ import os import sys import re import ipaddress +import traceback + from types import MethodType from collections import defaultdict from importlib import import_module @@ -38,8 +40,8 @@ from pgadmin.utils import PgAdminModule, driver, KeyManager from pgadmin.utils.preferences import Preferences from pgadmin.utils.session import create_session_interface, pga_unauthorised from pgadmin.utils.versioned_template_loader import VersionedTemplateLoader -from datetime import timedelta -from pgadmin.setup import get_version, set_version +from datetime import timedelta, datetime +from pgadmin.setup import get_version, set_version, check_db_tables from pgadmin.utils.ajax import internal_server_error, make_json_response from pgadmin.utils.csrf import pgCSRFProtect from pgadmin import authenticate @@ -349,6 +351,44 @@ def create_app(app_name=None): ########################################################################## # Upgrade the schema (if required) ########################################################################## + def backup_db_file(): + """ + Create a backup of the current database file + and create new database file with default settings. + """ + backup_file_name = "{0}.{1}".format( + SQLITE_PATH, datetime.now().strftime('%Y%m%d%H%M%S')) + os.rename(SQLITE_PATH, backup_file_name) + app.logger.error('Exception in database migration.') + app.logger.info('Creating new database file.') + try: + db_upgrade(app) + os.environ[ + 'CORRUPTED_DB_BACKUP_FILE'] = backup_file_name + app.logger.info('Database migration completed.') + except Exception as e: + app.logger.error('Database migration failed') + app.logger.error(traceback.format_exc()) + raise RuntimeError('Migration failed') + + def upgrade_db(): + """ + Execute the migrations. + """ + try: + db_upgrade(app) + os.environ['CORRUPTED_DB_BACKUP_FILE'] = '' + except Exception as e: + backup_db_file() + + # check all tables are present in the db. + is_db_error, invalid_tb_names = check_db_tables() + if is_db_error: + app.logger.error( + 'Table(s) {0} are missing in the' + ' database'.format(invalid_tb_names)) + backup_db_file() + with app.app_context(): # Run migration for the first time i.e. create database from config import SQLITE_PATH @@ -356,25 +396,34 @@ def create_app(app_name=None): # If version not available, user must have aborted. Tables are not # created and so its an empty db - if not os.path.exists(SQLITE_PATH) or get_version() == -1: + version = get_version() + if not os.path.exists(SQLITE_PATH) or version == -1: # If running in cli mode then don't try to upgrade, just raise # the exception if not cli_mode: - db_upgrade(app) + upgrade_db() else: if not os.path.exists(SQLITE_PATH): raise FileNotFoundError( 'SQLite database file "' + SQLITE_PATH + '" does not exists.') - raise RuntimeError('Specified SQLite database file ' - 'is not valid.') + raise RuntimeError( + 'The configuration database file is not valid.') else: schema_version = get_version() # Run migration if current schema version is greater than the # schema version stored in version table if CURRENT_SCHEMA_VERSION >= schema_version: - db_upgrade(app) + upgrade_db() + else: + # check all tables are present in the db. + is_db_error, invalid_tb_names = check_db_tables() + if is_db_error: + app.logger.error( + 'Table(s) {0} are missing in the' + ' database'.format(invalid_tb_names)) + backup_db_file() # Update schema version to the latest if CURRENT_SCHEMA_VERSION > schema_version: diff --git a/web/pgadmin/browser/__init__.py b/web/pgadmin/browser/__init__.py index edbd491e..5a6a1c46 100644 --- a/web/pgadmin/browser/__init__.py +++ b/web/pgadmin/browser/__init__.py @@ -342,6 +342,7 @@ class BrowserModule(PgAdminModule): list: a list of url endpoints exposed to the client. """ return [BROWSER_INDEX, 'browser.nodes', + 'browser.check_corrupted_db_file', 'browser.check_master_password', 'browser.set_master_password', 'browser.reset_master_password', @@ -950,6 +951,19 @@ def form_master_password_response(existing=True, present=False, errmsg=None): }) [email protected]("/check_corrupted_db_file", + endpoint="check_corrupted_db_file", methods=["GET"]) +def check_corrupted_db_file(): + """ + Get the corrupted database file path. + """ + file_location = os.environ['CORRUPTED_DB_BACKUP_FILE'] \ + if 'CORRUPTED_DB_BACKUP_FILE' in os.environ else '' + # reset the corrupted db file path in env. + os.environ['CORRUPTED_DB_BACKUP_FILE'] = '' + return make_json_response(data=file_location) + + @blueprint.route("/master_password", endpoint="check_master_password", methods=["GET"]) def check_master_password(): diff --git a/web/pgadmin/browser/static/js/browser.js b/web/pgadmin/browser/static/js/browser.js index 46dfb4f1..ba84ca41 100644 --- a/web/pgadmin/browser/static/js/browser.js +++ b/web/pgadmin/browser/static/js/browser.js @@ -594,7 +594,7 @@ define('pgadmin.browser', [ }, 300000); obj.set_master_password(''); - + obj.check_corrupted_db_file(); obj.Events.on('pgadmin:browser:tree:add', obj.onAddTreeNode, obj); obj.Events.on('pgadmin:browser:tree:update', obj.onUpdateTreeNode, obj); obj.Events.on('pgadmin:browser:tree:refresh', obj.onRefreshTreeNode, obj); @@ -607,7 +607,35 @@ define('pgadmin.browser', [ obj.register_to_activity_listener(document); obj.start_inactivity_timeout_daemon(); }, + check_corrupted_db_file: function() { + $.ajax({ + url: url_for('browser.check_corrupted_db_file'), + type: 'GET', + dataType: 'json', + contentType: 'application/json', + }).done((res)=> { + if(res.data.length > 0) { + + Alertify.alert( + 'Warning', + 'pgAdmin detected unrecoverable corruption in it\'s SQLite configuration database. ' + + 'The database has been backed up and recreated with default settings. '+ + 'It may be possible to recover data such as query history manually from '+ + 'the original/corrupt file using a tool such as DB Browser for SQLite if desired.'+ + '<br><br>Original file: ' + res.data + '<br>Replacement file: ' + + res.data.substring(0, res.data.length - 14) + ) + .set({'closable': true, + 'onok': function() { + }, + }); + + } + }).fail(function(xhr, status, error) { + Alertify.alert(error); + }); + }, init_master_password: function() { let self = this; // Master password dialog diff --git a/web/pgadmin/setup/__init__.py b/web/pgadmin/setup/__init__.py index 04571e9e..779a7217 100644 --- a/web/pgadmin/setup/__init__.py +++ b/web/pgadmin/setup/__init__.py @@ -11,3 +11,4 @@ from .user_info import user_info from .db_version import get_version, set_version from .db_upgrade import db_upgrade from .data_directory import create_app_data_directory +from .db_table_check import check_db_tables diff --git a/web/pgadmin/setup/db_table_check.py b/web/pgadmin/setup/db_table_check.py new file mode 100644 index 00000000..4da86c27 --- /dev/null +++ b/web/pgadmin/setup/db_table_check.py @@ -0,0 +1,32 @@ +########################################################################## +# +# pgAdmin 4 - PostgreSQL Tools +# +# Copyright (C) 2013 - 2021, The pgAdmin Development Team +# This software is released under the PostgreSQL Licence +# +########################################################################## + +from pgadmin.model import Version +from pgadmin.model import db + + +def get_db_table_names(): + db_table_names = db.metadata.tables.keys() if db.metadata.tables else 0 + return db_table_names + + +def check_db_tables(): + is_error = False + invalid_tb_names = list() + db_table_names = get_db_table_names() + # check table is actually present in the db. + for table_name in db_table_names: + if not db.engine.dialect.has_table(db.engine, table_name): + invalid_tb_names.append(table_name) + is_error = True + + if is_error: + return True, invalid_tb_names + else: + return False, invalid_tb_names diff --git a/web/pgadmin/setup/db_version.py b/web/pgadmin/setup/db_version.py index ad2811a1..50607583 100644 --- a/web/pgadmin/setup/db_version.py +++ b/web/pgadmin/setup/db_version.py @@ -16,7 +16,10 @@ def get_version(): except Exception: return -1 - return version.value + if version: + return version.value + else: + return -1 def set_version(new_version): ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file. 2021-06-03 06:39 [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file. Nikhil Mohite <[email protected]> 2021-06-03 08:17 ` Re: [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file. Dave Page <[email protected]> 2021-06-03 09:01 ` Re: [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file. Nikhil Mohite <[email protected]> 2021-06-03 09:19 ` Re: [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file. Dave Page <[email protected]> 2021-06-03 09:40 ` Re: [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file. Nikhil Mohite <[email protected]> 2021-06-03 10:21 ` Re: [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file. Nikhil Mohite <[email protected]> @ 2021-06-08 14:27 ` Akshay Joshi <[email protected]> 0 siblings, 0 replies; 7+ messages in thread From: Akshay Joshi @ 2021-06-08 14:27 UTC (permalink / raw) To: Nikhil Mohite <[email protected]>; +Cc: pgadmin-hackers; Dave Page <[email protected]> Thanks, the patch applied. On Thu, Jun 3, 2021 at 3:52 PM Nikhil Mohite <[email protected]> wrote: > Hi Team, > > PFA updated patch (v2) > On Thu, Jun 3, 2021 at 3:10 PM Nikhil Mohite < > [email protected]> wrote: > >> Hi Dave, >> >> On Thu, Jun 3, 2021 at 2:49 PM Dave Page <[email protected]> wrote: >> >>> Hi >>> >>> On Thu, Jun 3, 2021 at 10:01 AM Nikhil Mohite < >>> [email protected]> wrote: >>> >>>> Hi Dave, >>>> >>>> On Thu, Jun 3, 2021 at 1:47 PM Dave Page <[email protected]> wrote: >>>> >>>>> Hi >>>>> >>>>> On Thu, Jun 3, 2021 at 7:39 AM Nikhil Mohite < >>>>> [email protected]> wrote: >>>>> >>>>>> Hi Hackers, >>>>>> >>>>>> Please find the attached patch for RM-6460 >>>>>> <https://redmine.postgresql.org/issues/6460;: Need a mechanism to >>>>>> detect a corrupt/broken config DB file. >>>>>> >>>>>> 1. Added checks if all tables added in the model are present in >>>>>> SQLite DB or not. >>>>>> 2. If migrations fail it will backup older file and try migrations >>>>>> with the newly created file. >>>>>> (User will get notification on UI for the location of the backup file >>>>>> and newly created.) >>>>>> 3. If the user deleted any table from SQLite DB pgAdmin will not run >>>>>> on the next restart and it will add the missing table list in the logs. >>>>>> >>>>> >>>>> Surely if any tables have been deleted, it'll fail the check in point >>>>> 1? >>>>> >>>> Yes, but if the user deletes any table while pgAdmin is running then it >>>> will fail when the user tries to run pgAdmin next time. >>>> >>> >>> Right - but how is the end result of that different from a failed >>> migration that left a table missing? Either way, the end result is the >>> same, and should be handled the same way; i.e. backup/recreate the DB, then >>> warn the user as soon as the UI is up. >>> >> Yes - It should be the same in both conditions, I will send an updated >> patch for this. >> > >>> >> >>> I believe the process is simple (and maybe this is what is done and this >>> is just a language issue - I haven't read the patch in detail): >>> >>> - On startup, attempt to create the database/run any migrations as >>> appropriate. >>> - Check that all tables exist, and the schema version is correct. >>> - If there's a problem, backup the file, create a new one from scratch, >>> and warn the user. >>> >>> One thing I did notice when skimming the patch - we have this in the >>> code: >>> >>> + raise RuntimeError( >>> + 'Specified SQLite database file is not valid.') >>> >>> The *user* didn't specify a SQLite database file (nor do they care that >>> it's SQLite at this point). That (and any other similar messages) should >>> probably be rephrased to say something like: >>> >>> "The configuration database file is not valid." >>> >> >>> Thanks! >>> >>> >>> >>>> (If we remove the table from the model it will not check particular >>>> table is present in DB or not. ) >>>> >>>>> >>>>> -- >>>>> Dave Page >>>>> Blog: https://pgsnake.blogspot.com >>>>> Twitter: @pgsnake >>>>> >>>>> EDB: https://www.enterprisedb.com >>>>> >>>>> Regards, >>>> Nikhil Mohite. >>>> >>> >>> >>> -- >>> Dave Page >>> Blog: https://pgsnake.blogspot.com >>> Twitter: @pgsnake >>> >>> EDB: https://www.enterprisedb.com >>> >>> Regards, >> Nikhil Mohite >> > Regards, > Nikhil Mohite > -- *Thanks & Regards* *Akshay Joshi* *pgAdmin Hacker | Principal Software Architect* *EDB Postgres <http://edbpostgres.com>* *Mobile: +91 976-788-8246* ^ permalink raw reply [nested|flat] 7+ messages in thread
end of thread, other threads:[~2021-06-08 14:27 UTC | newest] Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2021-06-03 06:39 [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file. Nikhil Mohite <[email protected]> 2021-06-03 08:17 ` Dave Page <[email protected]> 2021-06-03 09:01 ` Nikhil Mohite <[email protected]> 2021-06-03 09:19 ` Dave Page <[email protected]> 2021-06-03 09:40 ` Nikhil Mohite <[email protected]> 2021-06-03 10:21 ` Nikhil Mohite <[email protected]> 2021-06-08 14:27 ` Akshay Joshi <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox