public inbox for [email protected]  
help / color / mirror / Atom feed
From: Aditya Toshniwal <[email protected]>
To: pgadmin-hackers <[email protected]>
Subject: [pgAdmin][SonarQube] Reduce cognitive complexity
Date: Thu, 3 Sep 2020 15:38:05 +0530
Message-ID: <CAM9w-_nozC6DZmtwX6bYj17=t_Ev7jETJ3D0HAhc_oDSyhXLKA@mail.gmail.com> (raw)

Hi Hackers,

Attached is the patch to reduce cognitive complexity as below:
web/pgadmin/misc/file_manager/__init__.py 20 to 15
web/pgadmin/tools/grant_wizard/__init__.py 55 to 15
web/pgadmin/tools/sqleditor/__init__.py 33 to 15

Please review.

-- 
Thanks,
Aditya Toshniwal
pgAdmin hacker | Sr. Software Engineer | *edbpostgres.com*
<http://edbpostgres.com;
"Don't Complain about Heat, Plant a TREE"


Attachments:

  [application/octet-stream] sonarqube.grant.patch (17.0K, 3-sonarqube.grant.patch)
  download | inline diff:
diff --git a/web/pgadmin/misc/file_manager/__init__.py b/web/pgadmin/misc/file_manager/__init__.py
index 955f98073..a66880d2c 100644
--- a/web/pgadmin/misc/file_manager/__init__.py
+++ b/web/pgadmin/misc/file_manager/__init__.py
@@ -672,30 +672,29 @@ class Filemanager(object):
 
     @staticmethod
     def check_access_permission(in_dir, path):
+        if not config.SERVER_MODE:
+            return
 
-        if config.SERVER_MODE:
-            if in_dir is None:
-                in_dir = ""
-            orig_path = Filemanager.get_abs_path(in_dir, path)
+        in_dir = '' if in_dir is None else in_dir
+        orig_path = Filemanager.get_abs_path(in_dir, path)
 
-            # This translates path with relative path notations
-            # like ./ and ../ to absolute path.
-            orig_path = os.path.abspath(orig_path)
+        # This translates path with relative path notations
+        # like ./ and ../ to absolute path.
+        orig_path = os.path.abspath(orig_path)
 
-            if in_dir:
-                if _platform == 'win32':
-                    if in_dir[-1] == '\\' or in_dir[-1] == '/':
-                        in_dir = in_dir[:-1]
-                else:
-                    if in_dir[-1] == '/':
-                        in_dir = in_dir[:-1]
-
-            # Do not allow user to access outside his storage dir
-            # in server mode.
-            if not orig_path.startswith(in_dir):
-                raise InternalServerError(
-                    gettext("Access denied ({0})").format(path))
-        return True
+        if in_dir:
+            if _platform == 'win32':
+                if in_dir[-1] == '\\' or in_dir[-1] == '/':
+                    in_dir = in_dir[:-1]
+            else:
+                if in_dir[-1] == '/':
+                    in_dir = in_dir[:-1]
+
+        # Do not allow user to access outside his storage dir
+        # in server mode.
+        if not orig_path.startswith(in_dir):
+            raise InternalServerError(
+                gettext("Access denied ({0})").format(path))
 
     @staticmethod
     def get_abs_path(in_dir, path):
diff --git a/web/pgadmin/tools/grant_wizard/__init__.py b/web/pgadmin/tools/grant_wizard/__init__.py
index ee62c4c6a..5c8687b2a 100644
--- a/web/pgadmin/tools/grant_wizard/__init__.py
+++ b/web/pgadmin/tools/grant_wizard/__init__.py
@@ -174,6 +174,79 @@ def acl_list(sid, did):
         mimetype="application/json")
 
 
+def _get_rows_for_type(conn, ntype, server_prop, node_id):
+    """
+    Used internally by properties to get rows for an object type
+    :param conn: connection object
+    :param ntype: object type
+    :param server_prop: server properties
+    :param node_id: oid
+    :return: status, execute response
+    """
+    function_sql_url = '/sql/function.sql'
+    status, res = True, []
+
+    if ntype in ['function']:
+        sql = render_template("/".join(
+            [server_prop['template_path'], function_sql_url]),
+            node_id=node_id, type='function')
+
+        status, res = conn.execute_dict(sql)
+    # Fetch procedures only if server type is EPAS or PG >= 11
+    elif len(server_prop) > 0 and (
+        server_prop['server_type'] == 'ppas' or (
+            server_prop['server_type'] == 'pg' and
+            server_prop['version'] >= 11000
+        )
+    ) and ntype in ['procedure']:
+        sql = render_template("/".join(
+            [server_prop['template_path'], function_sql_url]),
+            node_id=node_id, type='procedure')
+
+        status, res = conn.execute_dict(sql)
+
+    # Fetch trigger functions
+    elif ntype in ['trigger_function']:
+        sql = render_template("/".join(
+            [server_prop['template_path'], function_sql_url]),
+            node_id=node_id, type='trigger_function')
+        status, res = conn.execute_dict(sql)
+
+    # Fetch Sequences against schema
+    elif ntype in ['sequence']:
+        sql = render_template("/".join(
+            [server_prop['template_path'], '/sql/sequence.sql']),
+            node_id=node_id)
+
+        status, res = conn.execute_dict(sql)
+
+    # Fetch Tables against schema
+    elif ntype in ['table']:
+        sql = render_template("/".join(
+            [server_prop['template_path'], '/sql/table.sql']),
+            node_id=node_id)
+
+        status, res = conn.execute_dict(sql)
+
+    # Fetch Views against schema
+    elif ntype in ['view']:
+        sql = render_template("/".join(
+            [server_prop['template_path'], '/sql/view.sql']),
+            node_id=node_id, node_type='v')
+
+        status, res = conn.execute_dict(sql)
+
+    # Fetch Materialzed Views against schema
+    elif ntype in ['mview']:
+        sql = render_template("/".join(
+            [server_prop['template_path'], '/sql/view.sql']),
+            node_id=node_id, node_type='m')
+
+        status, res = conn.execute_dict(sql)
+
+    return status, res
+
+
 @blueprint.route(
     '/<int:sid>/<int:did>/<int:node_id>/<node_type>/',
     methods=['GET'], endpoint='objects'
@@ -185,7 +258,6 @@ def properties(sid, did, node_id, node_type):
        and render into selection page of wizard
     """
 
-    function_sql_url = '/sql/function.sql'
     get_schema_sql_url = '/sql/get_schemas.sql'
 
     # unquote encoded url parameter
@@ -198,130 +270,66 @@ def properties(sid, did, node_id, node_type):
     manager = get_driver(PG_DEFAULT_DRIVER).connection_manager(sid)
     conn = manager.connection(did=did)
 
-    node_types = []
     show_sysobj = blueprint.show_system_objects().get()
     if node_type == 'database':
-        # Fetch list of schemas
-        # Get sys_obj_values and get list of schemas
-        ntype = 'schema'
         sql = render_template("/".join(
             [server_prop['template_path'], get_schema_sql_url]),
             show_sysobj=show_sysobj)
-        status, res = conn.execute_dict(sql)
-
-        if not status:
-            return internal_server_error(errormsg=res)
-        node_types = res['rows']
+        ntype = 'schema'
     else:
         sql = render_template("/".join(
             [server_prop['template_path'], get_schema_sql_url]),
-            nspid=node_id, show_sysobj=False)
-        status, res = conn.execute_dict(sql)
+            show_sysobj=show_sysobj, nspid=node_id)
+        ntype = node_type
+
+    status, res = conn.execute_dict(sql)
+
+    if not status:
+        return internal_server_error(errormsg=res)
+    node_types = res['rows']
 
+    def _append_rows(status, res, disp_type):
         if not status:
-            return internal_server_error(errormsg=res)
-        node_types = res['rows']
-        ntype = node_type
+            current_app.logger.error(res)
+            failed_objects.append(disp_type)
+        else:
+            res_data.extend(res['rows'])
 
     for row in node_types:
         if 'oid' in row:
             node_id = row['oid']
 
-        # Fetch functions against schema
-        if ntype in ['schema', 'function']:
-            sql = render_template("/".join(
-                [server_prop['template_path'], function_sql_url]),
-                node_id=node_id, type='function')
-
-            status, res = conn.execute_dict(sql)
-            if not status:
-                current_app.logger.error(res)
-                failed_objects.append('function')
-            else:
-                res_data.extend(res['rows'])
-
-        # Fetch procedures only if server type is EPAS or PG >= 11
-        if (len(server_prop) > 0 and
-            (server_prop['server_type'] == 'ppas' or
-             (server_prop['server_type'] == 'pg' and
-              server_prop['version'] >= 11000)) and
-                ntype in ['schema', 'procedure']):
-            sql = render_template("/".join(
-                [server_prop['template_path'], function_sql_url]),
-                node_id=node_id, type='procedure')
-
-            status, res = conn.execute_dict(sql)
-
-            if not status:
-                current_app.logger.error(res)
-                failed_objects.append('procedure')
-            else:
-                res_data.extend(res['rows'])
-
-        # Fetch trigger functions
-        if ntype in ['schema', 'trigger_function']:
-            sql = render_template("/".join(
-                [server_prop['template_path'], function_sql_url]),
-                node_id=node_id, type='trigger_function')
-            status, res = conn.execute_dict(sql)
-
-            if not status:
-                current_app.logger.error(res)
-                failed_objects.append('trigger function')
-            else:
-                res_data.extend(res['rows'])
-
-        # Fetch Sequences against schema
-        if ntype in ['schema', 'sequence']:
-            sql = render_template("/".join(
-                [server_prop['template_path'], '/sql/sequence.sql']),
-                node_id=node_id)
-
-            status, res = conn.execute_dict(sql)
-            if not status:
-                current_app.logger.error(res)
-                failed_objects.append('sequence')
-            else:
-                res_data.extend(res['rows'])
-
-        # Fetch Tables against schema
-        if ntype in ['schema', 'table']:
-            sql = render_template("/".join(
-                [server_prop['template_path'], '/sql/table.sql']),
-                node_id=node_id)
-
-            status, res = conn.execute_dict(sql)
-            if not status:
-                current_app.logger.error(res)
-                failed_objects.append('table')
-            else:
-                res_data.extend(res['rows'])
-
-        # Fetch Views against schema
-        if ntype in ['schema', 'view']:
-            sql = render_template("/".join(
-                [server_prop['template_path'], '/sql/view.sql']),
-                node_id=node_id, node_type='v')
-
-            status, res = conn.execute_dict(sql)
-            if not status:
-                current_app.logger.error(res)
-                failed_objects.append('view')
-            else:
-                res_data.extend(res['rows'])
-
-        # Fetch Materialzed Views against schema
-        if ntype in ['schema', 'mview']:
-            sql = render_template("/".join(
-                [server_prop['template_path'], '/sql/view.sql']),
-                node_id=node_id, node_type='m')
-
-            status, res = conn.execute_dict(sql)
-            if not status:
-                current_app.logger.error(res)
-                failed_objects.append('materialized view')
-            else:
-                res_data.extend(res['rows'])
+        if ntype == 'schema':
+            status, res = _get_rows_for_type(
+                conn, 'function', server_prop, node_id)
+            _append_rows(status, res, 'function')
+
+            status, res = _get_rows_for_type(
+                conn, 'procedure', server_prop, node_id)
+            _append_rows(status, res, 'procedure')
+
+            status, res = _get_rows_for_type(
+                conn, 'trigger_function', server_prop, node_id)
+            _append_rows(status, res, 'trigger function')
+
+            status, res = _get_rows_for_type(
+                conn, 'sequence', server_prop, node_id)
+            _append_rows(status, res, 'sequence')
+
+            status, res = _get_rows_for_type(
+                conn, 'table', server_prop, node_id)
+            _append_rows(status, res, 'table')
+
+            status, res = _get_rows_for_type(
+                conn, 'view', server_prop, node_id)
+            _append_rows(status, res, 'view')
+
+            status, res = _get_rows_for_type(
+                conn, 'mview', server_prop, node_id)
+            _append_rows(status, res, 'materialized view')
+        else:
+            status, res = _get_rows_for_type(conn, ntype, server_prop, node_id)
+            _append_rows(status, res, 'function')
 
     msg = None
     if len(failed_objects) > 0:
diff --git a/web/pgadmin/tools/sqleditor/__init__.py b/web/pgadmin/tools/sqleditor/__init__.py
index 014192763..b16c72e0c 100644
--- a/web/pgadmin/tools/sqleditor/__init__.py
+++ b/web/pgadmin/tools/sqleditor/__init__.py
@@ -1316,76 +1316,79 @@ def save_file():
 )
 @login_required
 def start_query_download_tool(trans_id):
-    sync_conn = None
     (status, error_msg, sync_conn, trans_obj,
      session_obj) = check_transaction_status(trans_id)
 
-    if status and sync_conn is not None and \
-       trans_obj is not None and session_obj is not None:
+    if not status or sync_conn is None or trans_obj is None or \
+            session_obj is None:
+        return internal_server_error(
+            errormsg=gettext("Transaction status check failed.")
+        )
 
-        data = request.values if request.values else None
-        try:
-            if data and 'query' in data:
-                sql = data['query']
+    data = request.values if request.values else None
+    if data is None or (data and 'query' not in data):
+        return make_json_response(
+            status=410,
+            success=0,
+            errormsg=gettext(
+                "Could not find the required parameter (query)."
+            )
+        )
 
-                # This returns generator of records.
-                status, gen = sync_conn.execute_on_server_as_csv(
-                    sql, records=2000
-                )
+    try:
+        sql = data['query']
 
-                if not status:
-                    return make_json_response(
-                        data={
-                            'status': status, 'result': gen
-                        }
-                    )
-
-                r = Response(
-                    gen(
-                        quote=blueprint.csv_quoting.get(),
-                        quote_char=blueprint.csv_quote_char.get(),
-                        field_separator=blueprint.csv_field_separator.get(),
-                        replace_nulls_with=blueprint.replace_nulls_with.get()
-                    ),
-                    mimetype='text/csv' if
-                    blueprint.csv_field_separator.get() == ','
-                    else 'text/plain'
-                )
+        # This returns generator of records.
+        status, gen = sync_conn.execute_on_server_as_csv(
+            sql, records=2000
+        )
 
-                if 'filename' in data and data['filename'] != "":
-                    filename = data['filename']
-                else:
-                    import time
-                    filename = '{0}.{1}'. \
-                        format(int(time.time()), 'csv' if blueprint.
-                               csv_field_separator.get() == ',' else 'txt')
-
-                # We will try to encode report file name with latin-1
-                # If it fails then we will fallback to default ascii file name
-                # werkzeug only supports latin-1 encoding supported values
-                try:
-                    tmp_file_name = filename
-                    tmp_file_name.encode('latin-1', 'strict')
-                except UnicodeEncodeError:
-                    filename = "download.csv"
-
-                r.headers[
-                    "Content-Disposition"
-                ] = "attachment;filename={0}".format(filename)
-
-                return r
-        except (ConnectionLost, SSHTunnelConnectionLost):
-            raise
-        except Exception as e:
-            current_app.logger.error(e)
-            err_msg = "Error: {0}".format(
-                e.strerror if hasattr(e, 'strerror') else str(e))
-            return internal_server_error(errormsg=err_msg)
-    else:
-        return internal_server_error(
-            errormsg=gettext("Transaction status check failed.")
+        if not status:
+            return make_json_response(
+                data={
+                    'status': status, 'result': gen
+                }
+            )
+
+        r = Response(
+            gen(
+                quote=blueprint.csv_quoting.get(),
+                quote_char=blueprint.csv_quote_char.get(),
+                field_separator=blueprint.csv_field_separator.get(),
+                replace_nulls_with=blueprint.replace_nulls_with.get()
+            ),
+            mimetype='text/csv' if
+            blueprint.csv_field_separator.get() == ','
+            else 'text/plain'
         )
 
+        import time
+        extn = 'csv' if blueprint.csv_field_separator.get() == ',' else 'txt'
+        filename = data['filename'] if data.get('filename', '') != "" else \
+            '{0}.{1}'.format(int(time.time()), extn)
+
+        # We will try to encode report file name with latin-1
+        # If it fails then we will fallback to default ascii file name
+        # werkzeug only supports latin-1 encoding supported values
+        try:
+            tmp_file_name = filename
+            tmp_file_name.encode('latin-1', 'strict')
+        except UnicodeEncodeError:
+            filename = "download.csv"
+
+        r.headers[
+            "Content-Disposition"
+        ] = "attachment;filename={0}".format(filename)
+
+        return r
+    except (ConnectionLost, SSHTunnelConnectionLost):
+        raise
+    except Exception as e:
+        current_app.logger.error(e)
+        err_msg = "Error: {0}".format(
+            e.strerror if hasattr(e, 'strerror') else str(e))
+        return internal_server_error(errormsg=err_msg)
+
 
 @blueprint.route(
     '/status/<int:trans_id>',


view thread (9+ 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]
  Subject: Re: [pgAdmin][SonarQube] Reduce cognitive complexity
  In-Reply-To: <CAM9w-_nozC6DZmtwX6bYj17=t_Ev7jETJ3D0HAhc_oDSyhXLKA@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