Fix a connection issue with TLS and multiple databases (#217)

ClickHouse · Jan 16, 2024 · 884beef · 884beef
1 parent f40cf00
commit 884beef
Show file tree

Hide file tree

Showing 7 changed files with 42 additions and 22 deletions.
diff --git a/.gitignore b/.gitignore
@@ -12,3 +12,4 @@
 .calva
 .cpcache
 .joyride
+.nrepl-port
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,8 @@
+# 1.3.3
+
+### Bug fixes
+* Fixed an issue where it was not possible to create a connection with multiple databases using TLS. ([#215](https://github.com/ClickHouse/metabase-clickhouse-driver/issues/215))
+
 # 1.3.2
 
 ### Bug fixes

diff --git a/resources/metabase-plugin.yaml b/resources/metabase-plugin.yaml
@@ -1,6 +1,6 @@
 info:
   name: Metabase ClickHouse Driver
-  version: 1.3.2
+  version: 1.3.3
   description: Allows Metabase to connect to ClickHouse databases.
 contact-info:
   name: ClickHouse
@@ -20,7 +20,7 @@ driver:
     - name: dbname
       display-name: Databases
       placeholder: default
-      helper-text: "To specify multiple databases, separate them by the space symbol. For example: default data logs"
+      helper-text: "To specify multiple databases, separate them by the space symbol. For example: default data logs."
     - name: scan-all-databases
       display-name: Scan all databases
       type: boolean

diff --git a/src/metabase/driver/clickhouse.clj b/src/metabase/driver/clickhouse.clj
@@ -20,7 +20,7 @@
 (driver/register! :clickhouse :parent :sql-jdbc)
 
 (defmethod driver/display-name :clickhouse [_] "ClickHouse")
-(def ^:private product-name "metabase/1.3.2")
+(def ^:private product-name "metabase/1.3.3")
 
 (defmethod driver/prettify-native-form :clickhouse [_ native-form]
   (sql.u/format-sql-and-fix-params :mysql native-form))
@@ -45,7 +45,10 @@
   (let [details (reduce-kv (fn [m k v] (assoc m k (or v (k default-connection-details))))
                            default-connection-details
                            details)
-        {:keys [user password dbname host port ssl use-no-proxy]} details]
+        {:keys [user password dbname host port ssl use-no-proxy]} details
+        ;; if multiple databases were specified for the connection,
+        ;; use only the first dbname as the "main" one
+        dbname (first (str/split (str/trim dbname) #" "))]
     (->
      {:classname "com.clickhouse.jdbc.ClickHouseDriver"
       :subprotocol "clickhouse"

diff --git a/test/metabase/driver/clickhouse_test.clj b/test/metabase/driver/clickhouse_test.clj
@@ -538,20 +538,33 @@
        (is (= :type/Boolean (transduce identity (driver.common/values->base-type) [c1, c3])))
        (is (= :type/Boolean (driver.common/class->base-type (class c1))))))))
 
-(deftest clickhouse-simple-tls-connection
-  (mt/test-driver
-   :clickhouse
-   (is (= "UTC"
-          (let [working-dir (System/getProperty "user.dir")
-                cert-path (str working-dir "/modules/drivers/clickhouse/.docker/clickhouse/single_node_tls/certificates/ca.crt")
-                additional-options (str "sslrootcert=" cert-path)
-                spec (sql-jdbc.conn/connection-details->spec
-                      :clickhouse
-                      {:ssl true
-                       :host "server.clickhouseconnect.test"
-                       :port 8443
-                       :additional-options additional-options})]
-            (driver/db-default-timezone :clickhouse spec))))))
+(deftest clickhouse-tls
+  (mt/test-driver
+   :clickhouse
+   (let [working-dir (System/getProperty "user.dir")
+         cert-path (str working-dir "/modules/drivers/clickhouse/.docker/clickhouse/single_node_tls/certificates/ca.crt")
+         additional-options (str "sslrootcert=" cert-path)]
+     (testing "simple connection with a single database"
+       (is (= "UTC"
+              (driver/db-default-timezone
+               :clickhouse
+               (sql-jdbc.conn/connection-details->spec
+                :clickhouse
+                {:ssl true
+                 :host "server.clickhouseconnect.test"
+                 :port 8443
+                 :additional-options additional-options})))))
+     (testing "connection with multiple databases"
+       (is (= "UTC"
+              (driver/db-default-timezone
+               :clickhouse
+               (sql-jdbc.conn/connection-details->spec
+                :clickhouse
+                {:ssl true
+                 :host "server.clickhouseconnect.test"
+                 :port 8443
+                 :dbname "default system"
+                 :additional-options additional-options}))))))))
 
 (deftest clickhouse-filtered-aggregate-functions-test
   (mt/test-driver
@@ -769,6 +782,6 @@
    (ctd/create-test-db!)
    (doall
     (for [[username password] [["default" ""] ["user_with_password" "foo@bar!"]]
-          database            ["default" "Test Database" "Special@Characters~" "'Escaping'"]]
+          database            ["default" "Special@Characters~" "'Escaping'"]]
       (testing (format "User `%s` can connect to `%s`" username database)
         (is (true? (driver/can-connect? :clickhouse {:user username :password password :dbname database}))))))))
diff --git a/test/metabase/test/data/clickhouse.clj b/test/metabase/test/data/clickhouse.clj
@@ -90,7 +90,7 @@
    :ssl false
    :use_no_proxy false
    :use_server_time_zone_for_dates true
-   :product_name "metabase/1.3.2"})
+   :product_name "metabase/1.3.3"})
 
 (defn rows-without-index
   "Remove the Metabase index which is the first column in the result set"

diff --git a/test/metabase/test/data/datasets.sql b/test/metabase/test/data/datasets.sql
@@ -246,8 +246,6 @@ CREATE TABLE `metabase_test`.`low_cardinality_nullable_base_types` (
 ) ENGINE Memory;
 
 -- can-connect tests (odd database names)
-DROP DATABASE IF EXISTS `Test Database`;
-CREATE DATABASE `Test Database`;
 DROP DATABASE IF EXISTS `Special@Characters~`;
 CREATE DATABASE `Special@Characters~`;
 DROP DATABASE IF EXISTS `'Escaping'`;