From af2438c159264e51b3192ca7471f9d80a62c05ad Mon Sep 17 00:00:00 2001
From: Junda Yang <youngyjd@gmail.com>
Date: Mon, 8 Oct 2018 20:26:26 -0700
Subject: [PATCH] check db extra and metadata params preemptively (#6004)

* check db extra and metadata params preemptively

* flake8

* make use of inspect

(cherry picked from commit f50ed17655fa3bf91911a6a959a26144ae6ed7b4)
---
 superset/connectors/sqla/models.py |  3 ++-
 superset/views/core.py             | 19 ++++++++++++++++++-
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py
index 037eb779a03d7..d19e5b4be6462 100644
--- a/superset/connectors/sqla/models.py
+++ b/superset/connectors/sqla/models.py
@@ -805,7 +805,8 @@ def fetch_metadata(self):
         """Fetches the metadata for the table and merges it in"""
         try:
             table = self.get_sqla_table_object()
-        except Exception:
+        except Exception as e:
+            logging.exception(e)
             raise Exception(_(
                 "Table [{}] doesn't seem to exist in the specified database, "
                 "couldn't fetch column information").format(self.table_name))
diff --git a/superset/views/core.py b/superset/views/core.py
index 3308a6fc73fc8..8b3695817a184 100755
--- a/superset/views/core.py
+++ b/superset/views/core.py
@@ -6,6 +6,7 @@
 from __future__ import unicode_literals
 
 from datetime import datetime, timedelta
+import inspect
 import logging
 import os
 import re
@@ -26,7 +27,7 @@
 from six import text_type
 from six.moves.urllib import parse
 import sqlalchemy as sqla
-from sqlalchemy import and_, create_engine, or_, update
+from sqlalchemy import and_, create_engine, MetaData, or_, update
 from sqlalchemy.engine.url import make_url
 from sqlalchemy.exc import IntegrityError
 from unidecode import unidecode
@@ -261,6 +262,7 @@ class DatabaseView(SupersetModelView, DeleteMixin, YamlExportMixin):  # noqa
     }
 
     def pre_add(self, db):
+        self.check_extra(db)
         db.set_sqlalchemy_uri(db.sqlalchemy_uri)
         security_manager.merge_perm('database_access', db.perm)
         # adding a new database we always want to force refresh schema list
@@ -281,6 +283,21 @@ def pre_delete(self, obj):
     def _delete(self, pk):
         DeleteMixin._delete(self, pk)
 
+    def check_extra(self, db):
+        # this will check whether json.loads(extra) can succeed
+        try:
+            extra = db.get_extra()
+        except Exception as e:
+            raise Exception('Extra field cannot be decoded by JSON. {}'.format(str(e)))
+
+        # this will check whether 'metadata_params' is configured correctly
+        metadata_signature = inspect.signature(MetaData)
+        for key in extra.get('metadata_params', {}):
+            if key not in metadata_signature.parameters:
+                raise Exception('The metadata_params in Extra field '
+                                'is not configured correctly. The key '
+                                '{} is invalid.'.format(key))
+
 
 appbuilder.add_link(
     'Import Dashboards',