Fixed NPE in case of empty dbname (#141)

* Fixed NPE in case of empty dbname

* Update README

* Add default db name to post-filtered-active-tables

* Update Metabase repo ref in GHA

Author: slvrtrn

.github/deps.edn

@@ -439,7 +439,7 @@
      metabase.api.native-query-snippet-test/create-snippet-api-test
      metabase.api.native-query-snippet-test/update-snippet-api-test
      metabase.api.native-query-snippet-test/read-snippet-api-test
-     metabase.api.notify-test/unauthenticated-test
+     metabase.api.notify-test/authentication-test
      metabase.api.notify-test/not-found-test
      metabase.api.notify-test/post-db-id-test
      metabase.api.permissions-test/groups-list-limit-test
@@ -821,7 +821,9 @@
      metabase.dashboard-subscription-test/mrkdwn-length-limit-test
      metabase.dashboard-subscription-test/use-default-values-test
      metabase.dashboard-subscription-test/dashboard-filter-test
+     metabase.db.connection-pool-setup-test/DbActivityTracker-test
      metabase.db.connection-pool-setup-test/connection-pool-spec-test
+     metabase.db.connection-pool-setup-test/recent-activity-test
      metabase.db.data-migrations-test/fix-click-through-test
      metabase.db.data-migrations-test/migrate-click-through-test
      metabase.db.data-migrations-test/migrate-remove-admin-from-group-mapping-if-needed-test
@@ -1582,6 +1584,7 @@
      metabase.pulse.render.color-test/convert-keywords-test
      metabase.pulse.render.common-test/number-formatting-test
      metabase.pulse.render.datetime-test/format-temporal-string-pair-test
+     metabase.pulse.render.datetime-test/format-temporal-str-column-viz-settings-test
      metabase.pulse.render.datetime-test/format-temporal-str-test
      metabase.pulse.render.js-engine-test/make-context-test
      metabase.pulse.render.js-svg-test/progress-test
@@ -1713,6 +1716,7 @@
      metabase.query-processor.middleware.annotate-test/col-info-combine-parent-field-names-test
      metabase.query-processor.middleware.annotate-test/mbql-cols-nested-queries-test
      metabase.query-processor.middleware.annotate-test/col-info-field-literals-test
+     metabase.query-processor.middleware.annotate-test/preserve-original-join-alias-test
      metabase.query-processor.middleware.auto-bucket-datetimes-test/auto-bucket-in-compound-filter-clause-test
      metabase.query-processor.middleware.auto-bucket-datetimes-test/auto-bucket-by-semantic-type-test
      metabase.query-processor.middleware.auto-bucket-datetimes-test/auto-bucket-unix-timestamp-fields-test
@@ -2079,15 +2083,15 @@
      metabase.query-processor-test.count-where-test/metric-test
      metabase.query-processor-test.count-where-test/breakout-test
      metabase.query-processor-test.date-bucketing-test/week-of-year-and-week-count-should-be-consistent-test
-     ;metabase.query-processor-test.date-bucketing-test/group-by-default-test
+     ;;metabase.query-processor-test.date-bucketing-test/group-by-default-test
      metabase.query-processor-test.date-bucketing-test/legacy-default-datetime-bucketing-test
      metabase.query-processor-test.date-bucketing-test/june-31st-test
      metabase.query-processor-test.date-bucketing-test/sanity-check-test
      metabase.query-processor-test.date-bucketing-test/compile-time-interval-test
      metabase.query-processor-test.date-bucketing-test/filter-by-current-quarter-test
      metabase.query-processor-test.date-bucketing-test/default-bucketing-test
      metabase.query-processor-test.date-bucketing-test/group-by-day-of-month-test
-     ;metabase.query-processor-test.date-bucketing-test/group-by-day-test
+     ;;metabase.query-processor-test.date-bucketing-test/group-by-day-test
      metabase.query-processor-test.date-bucketing-test/group-by-month-test
      metabase.query-processor-test.date-bucketing-test/group-by-quarter-test
      metabase.query-processor-test.date-bucketing-test/group-by-week-test

.github/workflows/check.yml

@@ -17,7 +17,7 @@ jobs:
         uses: actions/checkout@v2
         with:
           repository: metabase/metabase
-          ref: v0.45.1
+          ref: v0.45.2
 
       - name: Checkout Driver Repo
         uses: actions/checkout@v2

CHANGELOG.md

@@ -1,3 +1,9 @@
+# 1.0.3
+
+### Bug fixes
+
+* Fixed NPE that could be thrown by the driver in case of empty database name input.
+
 # 1.0.2
 
 ### Bug fixes

README.md

@@ -66,7 +66,7 @@ Metabase Release | Driver Version
 0.41.3.1         | 0.8.1
 0.42.x           | 0.8.1
 0.44.x           | 0.9.1
-0.45.1           | 1.0.2
+0.45.x           | 1.0.3
 
 ## Creating a Metabase Docker image with ClickHouse driver
 

docker-compose.yml

@@ -32,7 +32,7 @@ services:
     hostname: server.clickhouseconnect.test
 
   metabase:
-    image: metabase/metabase:v0.45.1
+    image: metabase/metabase:v0.45.2
     container_name: metabase-with-clickhouse-driver
     ports:
       - '3000:3000'

resources/metabase-plugin.yaml

@@ -1,6 +1,6 @@
 info:
   name: Metabase ClickHouse Driver
-  version: 1.0.2
+  version: 1.0.3
   description: Allows Metabase to connect to ClickHouse databases.
 contact-info:
   name: ClickHouse

src/metabase/driver/clickhouse.clj

@@ -72,24 +72,28 @@
 (def ^:private excluded-schemas #{"system" "information_schema" "INFORMATION_SCHEMA"})
 (defmethod sql-jdbc.sync/excluded-schemas :clickhouse [_] excluded-schemas)
 
+(def ^:private default-connection-details
+  {:user "default", :password "", :dbname "default", :host "localhost", :port "8123"})
+
 (defmethod sql-jdbc.conn/connection-details->spec :clickhouse
-  [_
-   {:keys [user password dbname host port ssl use-no-proxy]
-    :or
-    {user "default" password "" dbname "default" host "localhost" port "8123"}
-    :as details}]
-  (->
-   {:classname "com.clickhouse.jdbc.ClickHouseDriver"
-    :subprotocol "clickhouse"
-    :subname (str "//" host ":" port "/" dbname)
-    :password password
-    :user user
-    :ssl (boolean ssl)
-    :use_no_proxy (boolean use-no-proxy)
-    :use_server_time_zone_for_dates true
-    ;; temporary hardcode until we get product_name setting with JDBC driver v0.4.0
-    :client_name "metabase/1.0.2 clickhouse-jdbc/0.3.2-patch-11"}
-   (sql-jdbc.common/handle-additional-options details :separator-style :url)))
+  [_ details]
+  ;; ensure defaults merge on top of nils
+  (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]
+    (->
+     {:classname "com.clickhouse.jdbc.ClickHouseDriver"
+      :subprotocol "clickhouse"
+      :subname (str "//" host ":" port "/" dbname)
+      :password password
+      :user user
+      :ssl (boolean ssl)
+      :use_no_proxy (boolean use-no-proxy)
+      :use_server_time_zone_for_dates true
+      ;; temporary hardcode until we get product_name setting with JDBC driver v0.4.0
+      :client_name "metabase/1.0.3 clickhouse-jdbc/0.3.2-patch-11"}
+     (sql-jdbc.common/handle-additional-options details :separator-style :url))))
 
 (defn- to-relative-day-num
   [expr]
@@ -486,7 +490,7 @@
 
 (defn- post-filtered-active-tables
   [^DatabaseMetaData metadata db-name-or-nil]
-  (let [db-name-snake-case (ddl.i/format-name :clickhouse db-name-or-nil)
+  (let [db-name-snake-case (ddl.i/format-name :clickhouse (or db-name-or-nil "default"))
         tables (get-tables metadata db-name-snake-case)]
     (set
      (for [table (filter is-not-excluded-schema tables)]

test/metabase/driver/clickhouse_test.clj

@@ -460,7 +460,12 @@
              :dbname "foo"
              :use-no-proxy true
              :additional-options "sessionTimeout=42"
-             :ssl true})))))
+             :ssl true}))))
+  (testing "nil dbname handling"
+    (is (= default-connection-params
+           (sql-jdbc.conn/connection-details->spec
+            :clickhouse
+            {:dbname nil})))))
 
 (deftest clickhouse-boolean-result-metadata
   (mt/test-driver

test/metabase/test/data/clickhouse.clj

@@ -13,7 +13,7 @@
 
 (sql-jdbc.tx/add-test-extensions! :clickhouse)
 
-(def product-name "metabase/1.0.2 clickhouse-jdbc/0.3.2-patch-11")
+(def product-name "metabase/1.0.3 clickhouse-jdbc/0.3.2-patch-11")
 (def default-connection-params {:classname "com.clickhouse.jdbc.ClickHouseDriver"
                                 :subprotocol "clickhouse"
                                 :subname "//localhost:8123/default"