public inbox for [email protected]
help / color / mirror / Atom feedFrom: Rahul Shirsat <[email protected]>
To: Akshay Joshi <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: SonarQube Fixes #4 Database & Foreign Tables
Date: Thu, 21 Jan 2021 17:24:01 +0530
Message-ID: <CAKtn9dN2O-g-PwKT8qYaAqMS3ohkZ76JpvBPSR2_qnQ7Tjsarg@mail.gmail.com> (raw)
In-Reply-To: <CANxoLDcnOgBoR5yc4E3ugtmF3dT71+eHJ63RPHd0OvMnf9tE-Q@mail.gmail.com>
References: <CAKtn9dPC12071j=ESFRBFsdKsEOtM3sZ6e3UwZj7y_hnrnyb4Q@mail.gmail.com>
<CANxoLDcnOgBoR5yc4E3ugtmF3dT71+eHJ63RPHd0OvMnf9tE-Q@mail.gmail.com>
On Thu, Jan 21, 2021 at 2:47 PM Akshay Joshi <[email protected]>
wrote:
> Hi Rahul
>
> Following are the review comments:
>
> - Change the name of the function 'get_pg_db_properties'. The function
> returns lastsysoid and datistemplate only and I assume it is applicable for
> PG/EPAS both.
>
> *Done!*
>
> - Add comments for all the new functions introduce in the patch.
>
> *Added!*
>
> On Thu, Jan 21, 2021 at 2:36 PM Rahul Shirsat <
> [email protected]> wrote:
>
>> Hi Hackers,
>>
>> Please find the attached patch which resolves the sonar qube issues
>> relating:
>>
>>
>> 1. *Foreign Tables* -*Refactor this function to reduce its Cognitive
>> Complexity from 67 to the 15 allowed.*
>> 2. *Database* -*Refactor this function to reduce its Cognitive
>> Complexity from 17 to the 15 allowed.*
>>
>>
>> --
>> *Rahul Shirsat*
>> Senior Software Engineer | EnterpriseDB Corporation.
>>
>
>
> --
> *Thanks & Regards*
> *Akshay Joshi*
> *pgAdmin Hacker | Principal Software Architect*
> *EDB Postgres <http://edbpostgres.com>*
>
> *Mobile: +91 976-788-8246*
>
--
*Rahul Shirsat*
Senior Software Engineer | EnterpriseDB Corporation.
Attachments:
[application/octet-stream] sonar_issues_database_foreign_tables_v2.patch (11.9K, 3-sonar_issues_database_foreign_tables_v2.patch)
download | inline diff:
diff --git a/web/pgadmin/browser/server_groups/servers/databases/__init__.py b/web/pgadmin/browser/server_groups/servers/databases/__init__.py
index aedc83e86..87323b342 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/__init__.py
+++ b/web/pgadmin/browser/server_groups/servers/databases/__init__.py
@@ -21,7 +21,8 @@ import pgadmin.browser.server_groups.servers as servers
from config import PG_DEFAULT_DRIVER
from pgadmin.browser.collection import CollectionNodeModule
from pgadmin.browser.server_groups.servers.databases.utils import \
- parse_sec_labels_from_db, parse_variables_from_db
+ parse_sec_labels_from_db, parse_variables_from_db, \
+ get_attributes_from_db_info
from pgadmin.browser.server_groups.servers.utils import parse_priv_from_db, \
parse_priv_to_db
from pgadmin.browser.utils import PGChildNodeView
@@ -191,16 +192,15 @@ class DatabaseView(PGChildNodeView):
# If connection to database is not allowed then
# provide generic connection
if kwargs['did'] in self.manager.db_info:
- self._db = self.manager.db_info[kwargs['did']]
- self.datlastsysoid = self._db['datlastsysoid']
- if self._db['datallowconn'] is False:
+
+ self.datlastsysoid, self.datistemplate, \
+ datallowconn = \
+ get_attributes_from_db_info(self.manager, kwargs)
+
+ if datallowconn is False:
self.conn = self.manager.connection()
self.db_allow_connection = False
- self.datistemplate = \
- self.manager.db_info[kwargs['did']][
- 'datistemplate'] if 'datistemplate' in \
- self.manager.db_info[kwargs['did']] else False
else:
self.conn = self.manager.connection()
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/foreign_tables/__init__.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/foreign_tables/__init__.py
index 11d3a3db5..bd67aea7c 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/foreign_tables/__init__.py
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/foreign_tables/__init__.py
@@ -237,83 +237,30 @@ class ForeignTableView(PGChildNodeView, DataTypeReader,
@wraps(f)
def wrap(self, **kwargs):
- data = {}
-
if request.data:
req = json.loads(request.data, encoding='utf-8')
else:
req = request.args or request.form
- if 'foid' not in kwargs:
- required_args = [
- 'name',
- 'ftsrvname'
- ]
-
- for arg in required_args:
- if arg not in req or req[arg] == '':
- return make_json_response(
- status=410,
- success=0,
- errormsg=gettext(
- "Could not find the required parameter ({})."
- ).format(arg)
- )
+ invalid, arg = self._check_valid_foid_input(kwargs, req)
+
+ if invalid:
+ return make_json_response(
+ status=410,
+ success=0,
+ errormsg=gettext(
+ "Could not find the required parameter ({})."
+ ).format(arg)
+ )
try:
- list_params = []
if request.method == 'GET':
list_params = ['constraints', 'columns', 'ftoptions',
'seclabels', 'inherits', 'acl']
else:
list_params = ['inherits']
- for key in req:
- if (
- key in list_params and req[key] != '' and
- req[key] is not None
- ):
- # Coverts string into python list as expected.
- data[key] = []
- if not isinstance(req[key], list) and req[key]:
- data[key] = json.loads(req[key], encoding='utf-8')
-
- if key == 'inherits':
- # Convert Table ids from unicode/string to int
- # and make tuple for 'IN' query.
- inherits = tuple([int(x) for x in data[key]])
-
- if len(inherits) == 1:
- # Python tupple has , after the first param
- # in case of single parameter.
- # So, we need to make it tuple explicitly.
- inherits = "(" + str(inherits[0]) + ")"
- if inherits:
- # Fetch Table Names from their respective Ids,
- # as we need Table names to generate the SQL.
- SQL = render_template(
- "/".join([self.template_path,
- self._GET_TABLES_SQL]),
- attrelid=inherits)
- status, res = self.conn.execute_dict(SQL)
-
- if not status:
- return internal_server_error(errormsg=res)
-
- if 'inherits' in res['rows'][0]:
- data[key] = res['rows'][0]['inherits']
- else:
- data[key] = []
-
- elif key == 'typnotnull':
- if req[key] == 'true' or req[key] is True:
- data[key] = True
- elif req[key] == 'false' or req[key] is False:
- data[key] = False
- else:
- data[key] = ''
- else:
- data[key] = req[key]
+ data = self._validate_req(req, list_params)
except Exception as e:
return internal_server_error(errormsg=str(e))
@@ -323,6 +270,102 @@ class ForeignTableView(PGChildNodeView, DataTypeReader,
return wrap
+ @staticmethod
+ def _check_valid_foid_input(kwargs, req):
+
+ """
+ check for valid Foreign Table id
+ :param kwargs: user input
+ :param req: request object
+ """
+
+ if 'foid' not in kwargs:
+ required_args = [
+ 'name',
+ 'ftsrvname'
+ ]
+
+ for arg in required_args:
+ if arg not in req or req[arg] == '':
+ return True, arg
+ return False, ''
+
+ def _validate_req(self, req, list_params):
+
+ """
+ Validate & convert the string to desired output format
+ :param req: request data
+ :param list_params: prepared list of inherit, constraints, etc.
+ :return: data
+ """
+
+ data = {}
+
+ try:
+ for key in req:
+ if (
+ key in list_params and req[key] != '' and
+ req[key] is not None
+ ):
+ # Coverts string into python list as expected.
+ data[key] = []
+ self._convert_string_to_list(req, data, key)
+
+ elif key == 'typnotnull':
+ if req[key] == 'true' or req[key] is True:
+ data[key] = True
+ elif req[key] == 'false' or req[key] is False:
+ data[key] = False
+ else:
+ data[key] = ''
+ else:
+ data[key] = req[key]
+
+ except Exception as e:
+ current_app.logger.exception(e)
+ raise e
+
+ return data
+
+ def _convert_string_to_list(self, req, data, key):
+
+ """
+ Convert the string with utf-8 base to list
+ :param req: request data
+ :param data: output
+ :param key: index for data
+ """
+
+ if not isinstance(req[key], list) and req[key]:
+ data[key] = json.loads(req[key], encoding='utf-8')
+
+ if key == 'inherits':
+ # Convert Table ids from unicode/string to int
+ # and make tuple for 'IN' query.
+ inherits = tuple([int(x) for x in data[key]])
+
+ if len(inherits) == 1:
+ # Python tupple has , after the first param
+ # in case of single parameter.
+ # So, we need to make it tuple explicitly.
+ inherits = "(" + str(inherits[0]) + ")"
+ if inherits:
+ # Fetch Table Names from their respective Ids,
+ # as we need Table names to generate the SQL.
+ SQL = render_template(
+ "/".join([self.template_path,
+ self._GET_TABLES_SQL]),
+ attrelid=inherits)
+ status, res = self.conn.execute_dict(SQL)
+
+ if not status:
+ return internal_server_error(errormsg=res)
+
+ if 'inherits' in res['rows'][0]:
+ data[key] = res['rows'][0]['inherits']
+ else:
+ data[key] = []
+
def check_precondition(f):
"""
Works as a decorator.
@@ -344,14 +387,10 @@ class ForeignTableView(PGChildNodeView, DataTypeReader,
if self.manager.db_info is not None and \
kwargs['did'] in self.manager.db_info else 0
- self.datistemplate = False
- if (
- self.manager.db_info is not None and
- kwargs['did'] in self.manager.db_info and
- 'datistemplate' in self.manager.db_info[kwargs['did']]
- ):
- self.datistemplate = self.manager.db_info[
- kwargs['did']]['datistemplate']
+ self.datistemplate = \
+ self.manager.db_info[kwargs['did']]['datistemplate'] \
+ if self.manager.db_info is not None and \
+ kwargs['did'] in self.manager.db_info else False
# Set template path for sql scripts depending
# on the server version.
diff --git a/web/pgadmin/browser/server_groups/servers/databases/utils.py b/web/pgadmin/browser/server_groups/servers/databases/utils.py
index 2eb12b265..30f3d9e42 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/utils.py
+++ b/web/pgadmin/browser/server_groups/servers/databases/utils.py
@@ -105,3 +105,26 @@ def _check_var_type(var_value, var_name, row):
var_dict['database'] = row['db_name']
return var_dict
+
+
+def get_attributes_from_db_info(manager, kwargs):
+
+ """
+ Function to get attributes from db_info, send default values if not found.
+ :param manager: DB manager
+ :param kwargs: user input
+ :return: db_info attributes
+ """
+
+ if 'did' in kwargs and kwargs['did'] in manager.db_info:
+
+ datlastsysoid = manager.db_info[kwargs['did']]['datlastsysoid'] \
+ if 'datlastsysoid' in manager.db_info[kwargs['did']] else 0
+ datistemplate = manager.db_info[kwargs['did']]['datistemplate'] \
+ if 'datistemplate' in manager.db_info[kwargs['did']] else False
+ datallowconn = manager.db_info[kwargs['did']]['datallowconn'] \
+ if 'datallowconn' in manager.db_info[kwargs['did']] else False
+
+ return datlastsysoid, datistemplate, datallowconn
+ else:
+ return 0, False, True
view thread (4+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected]
Subject: Re: SonarQube Fixes #4 Database & Foreign Tables
In-Reply-To: <CAKtn9dN2O-g-PwKT8qYaAqMS3ohkZ76JpvBPSR2_qnQ7Tjsarg@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox