From 1fb00bc51388805571e03290d2d35157d7c1204d Mon Sep 17 00:00:00 2001
From: Lily Kuang <lily@preset.io>
Date: Thu, 10 Sep 2020 13:49:14 -0700
Subject: [PATCH] fix(databases): test connection api endpoint (#10824)

* fix test connection with extra

* fix lint and allow_none server_cert

* update test connection tests
---
 superset/databases/commands/test_connection.py |  5 ++---
 superset/databases/schemas.py                  |  8 ++++++--
 tests/databases/api_tests.py                   | 16 +++++++++++++++-
 3 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/superset/databases/commands/test_connection.py b/superset/databases/commands/test_connection.py
index 3bcd5b09237cc..99e6f98721705 100644
--- a/superset/databases/commands/test_connection.py
+++ b/superset/databases/commands/test_connection.py
@@ -18,7 +18,6 @@
 from contextlib import closing
 from typing import Any, Dict, Optional
 
-import simplejson as json
 from flask_appbuilder.security.sqla.models import User
 from sqlalchemy import select
 
@@ -46,9 +45,9 @@ def run(self) -> None:
 
             database = DatabaseDAO.build_db_for_connection_test(
                 server_cert=self._properties.get("server_cert", ""),
-                extra=json.dumps(self._properties.get("extra", {})),
+                extra=self._properties.get("extra", "{}"),
                 impersonate_user=self._properties.get("impersonate_user", False),
-                encrypted_extra=json.dumps(self._properties.get("encrypted_extra", {})),
+                encrypted_extra=self._properties.get("encrypted_extra", "{}"),
             )
             if database is not None:
                 database.set_sqlalchemy_uri(uri)
diff --git a/superset/databases/schemas.py b/superset/databases/schemas.py
index 859eebb9290d6..7dd7bc41f0ed1 100644
--- a/superset/databases/schemas.py
+++ b/superset/databases/schemas.py
@@ -298,10 +298,14 @@ class DatabaseTestConnectionSchema(Schema):
     impersonate_user = fields.Boolean(description=impersonate_user_description)
     extra = fields.String(description=extra_description, validate=extra_validator)
     encrypted_extra = fields.String(
-        description=encrypted_extra_description, validate=encrypted_extra_validator
+        description=encrypted_extra_description,
+        validate=encrypted_extra_validator,
+        allow_none=True,
     )
     server_cert = fields.String(
-        description=server_cert_description, validate=server_cert_validator
+        description=server_cert_description,
+        allow_none=True,
+        validate=server_cert_validator,
     )
     sqlalchemy_uri = fields.String(
         description=sqlalchemy_uri_description,
diff --git a/tests/databases/api_tests.py b/tests/databases/api_tests.py
index e07b7ac8a072f..9dafb87f2f7ac 100644
--- a/tests/databases/api_tests.py
+++ b/tests/databases/api_tests.py
@@ -656,15 +656,24 @@ def test_test_connection(self):
         """
         Database API: Test test connection
         """
+        extra = {
+            "metadata_params": {},
+            "engine_params": {},
+            "metadata_cache_timeout": {},
+            "schemas_allowed_for_csv_upload": [],
+        }
         # need to temporarily allow sqlite dbs, teardown will undo this
         app.config["PREVENT_UNSAFE_DB_CONNECTIONS"] = False
         self.login("admin")
         example_db = get_example_database()
         # validate that the endpoint works with the password-masked sqlalchemy uri
         data = {
-            "sqlalchemy_uri": example_db.safe_sqlalchemy_uri(),
             "database_name": "examples",
+            "encrypted_extra": "{}",
+            "extra": json.dumps(extra),
             "impersonate_user": False,
+            "sqlalchemy_uri": example_db.safe_sqlalchemy_uri(),
+            "server_cert": ssl_certificate,
         }
         url = f"api/v1/database/test_connection"
         rv = self.post_assert_metric(url, data, "test_connection")
@@ -676,6 +685,8 @@ def test_test_connection(self):
             "sqlalchemy_uri": example_db.sqlalchemy_uri_decrypted,
             "database_name": "examples",
             "impersonate_user": False,
+            "extra": json.dumps(extra),
+            "server_cert": None,
         }
         rv = self.post_assert_metric(url, data, "test_connection")
         self.assertEqual(rv.status_code, 200)
@@ -691,6 +702,7 @@ def test_test_connection_failed(self):
             "sqlalchemy_uri": "broken://url",
             "database_name": "examples",
             "impersonate_user": False,
+            "server_cert": None,
         }
         url = f"api/v1/database/test_connection"
         rv = self.post_assert_metric(url, data, "test_connection")
@@ -707,6 +719,7 @@ def test_test_connection_failed(self):
             "sqlalchemy_uri": "mssql+pymssql://url",
             "database_name": "examples",
             "impersonate_user": False,
+            "server_cert": None,
         }
         rv = self.post_assert_metric(url, data, "test_connection")
         self.assertEqual(rv.status_code, 400)
@@ -729,6 +742,7 @@ def test_test_connection_unsafe_uri(self):
             "sqlalchemy_uri": "sqlite:///home/superset/unsafe.db",
             "database_name": "unsafe",
             "impersonate_user": False,
+            "server_cert": None,
         }
         url = f"api/v1/database/test_connection"
         rv = self.post_assert_metric(url, data, "test_connection")