From 22d23a988f8af119b3c7795dc58797ab91daa4bc Mon Sep 17 00:00:00 2001
From: Jeff Evans <jeff303@users.noreply.github.com>
Date: Tue, 19 Oct 2021 20:44:57 -0500
Subject: [PATCH] Prevent duplicate connection properties (#18359)
Removing duplicate property declaration from presto-jdbc driver YAML
Add test that executes against all drivers to confirm that no duplicate names come out of connection-properties
Change the way the test runs to avoid needing to initialize test data namespace (to make googleanalytics happy)
Unskip repro Cypress test
---
.../admin/databases/add-presto.cy.spec.js | 2 +-
.../resources/metabase-plugin.yaml | 3 ---
test/metabase/driver_test.clj | 21 ++++++++++++++++++-
3 files changed, 21 insertions(+), 5 deletions(-)
diff --git a/frontend/test/metabase/scenarios/admin/databases/add-presto.cy.spec.js b/frontend/test/metabase/scenarios/admin/databases/add-presto.cy.spec.js
index db61cd7c3e8..943584c8e1a 100644
--- a/frontend/test/metabase/scenarios/admin/databases/add-presto.cy.spec.js
+++ b/frontend/test/metabase/scenarios/admin/databases/add-presto.cy.spec.js
@@ -1,6 +1,6 @@
import { restore, popover } from "__support__/e2e/cypress";
-describe.skip("admin > database > add > Presto", () => {
+describe("admin > database > add > Presto", () => {
beforeEach(() => {
restore();
cy.signInAsAdmin();
diff --git a/modules/drivers/presto-jdbc/resources/metabase-plugin.yaml b/modules/drivers/presto-jdbc/resources/metabase-plugin.yaml
index dc9b19ab4c0..41037b5808b 100644
--- a/modules/drivers/presto-jdbc/resources/metabase-plugin.yaml
+++ b/modules/drivers/presto-jdbc/resources/metabase-plugin.yaml
@@ -30,9 +30,6 @@ driver:
- password
- required: false
- ssl
- - merge:
- - additional-options
- - placeholder: "trustServerCertificate=false"
- name: kerberos
type: boolean
display-name: Authenticate with Kerberos?
diff --git a/test/metabase/driver_test.clj b/test/metabase/driver_test.clj
index 79bd3bf5fad..ac09810f2c2 100644
--- a/test/metabase/driver_test.clj
+++ b/test/metabase/driver_test.clj
@@ -2,7 +2,8 @@
(:require [clojure.test :refer :all]
[metabase.driver :as driver]
[metabase.driver.impl :as impl]
- [metabase.plugins.classloader :as classloader]))
+ [metabase.plugins.classloader :as classloader]
+ [metabase.test.data.env :as tx.env]))
(driver/register! ::test-driver, :abstract? true)
@@ -36,3 +37,21 @@
(is (driver/available? ::test-driver))
(is (driver/available? "metabase.driver-test/test-driver")
"`driver/available?` should work for if `driver` is a string -- see #10135")))
+
+(deftest unique-connection-property-test
+ ;; abnormal usage here; we are not using the regular mt/test-driver or mt/test-drivers, because those involve
+ ;; initializing the driver and test data namespaces, which don't necessarily exist for all drivers (ex:
+ ;; googleanalytics), and besides which, we don't actually need sample data or test extensions for this test itself
+
+ ;; so instead, just iterate through all drivers currently set to test by the environment, and check their
+ ;; connection-properties; between all the different CI driver runs, this should cover everything
+ (doseq [d (tx.env/test-drivers)]
+ (testing (str d " has entirely unique connection property names")
+ (let [props (driver/connection-properties d)
+ props-by-name (group-by :name props)]
+ (is (= (count props) (count props-by-name))
+ (format "Property(s) with duplicate name: %s" (-> (filter (fn [[_ props]]
+ (> (count props) 1))
+ props-by-name)
+ vec
+ pr-str)))))))
--
GitLab