Add support for new-style connection details maps for Postgres to

Clojure backend. Add some extra unit tests for funsies

.dir-locals.el

@@ -31,6 +31,7 @@
                               (org-perms-case 1)
                               (pdoseq 1)
                               (qp-expect-with-all-drivers 1)
+                              (resolve-private-fns 1)
                               (symbol-macrolet 1)
                               (sync-in-context 2)
                               (upd 2)

src/metabase/driver/postgres.clj

@@ -90,19 +90,31 @@
       (rename-keys {:dbname :db})
       kdb/postgres))
 
-(defn- database->connection-details [database]
-  (let [details (-<>> database :details :conn_str             ; get conn str like "password=corvus user=corvus ..."
-                      (s/split <> #" ")                       ; split into k=v pairs
-                      (map (fn [pair]                          ; convert to {:k v} pairs
-                             (let [[k v] (s/split pair #"=")]
-                               {(keyword k) v})))
-                      (reduce conj {}))                       ; combine into single dict
-        {:keys [host port]} details]
+(defn- is-legacy-conn-details?
+  "Is DETAILS-MAP a legacy map (i.e., does it only contain `conn_str`)?"
+  [details-map]
+  {:pre [(map? details-map)]}
+  (not (:dbname details-map)))
+
+(defn- parse-legacy-conn-str
+  "Parse a legacy `database.details.conn_str` CONNECTION-STRING."
+  [connection-string]
+  {:pre [(string? connection-string)]}
+  (-<>> connection-string
+        (s/split <> #" ")               ; split into k=v pairs
+        (map (fn [pair]                  ; convert to {:k v} pairs
+               (let [[k v] (s/split pair #"=")]
+                 {(keyword k) v})))
+        (reduce conj {})))
+
+(defn- database->connection-details [{:keys [details]}]
+  (let [{:keys [host port] :as details} (if (is-legacy-conn-details? details) (parse-legacy-conn-str (:conn_str details))
+                                            details)]
     (-> details
         (assoc :host       host
                :make-pool? false
                :db-type    :postgres                          ; What purpose is this serving?
-               :ssl        (:ssl (:details database))
+               :ssl        (:ssl details)
                :port       (Integer/parseInt port))
         (rename-keys {:dbname :db}))))
 
@@ -122,3 +134,6 @@
     :database->connection-details        database->connection-details
     :sql-string-length-fn                :CHAR_LENGTH
     :timezone->set-timezone-sql          timezone->set-timezone-sql}))
+
+(use 'metabase.db)
+(use 'metabase.models.database)

test/metabase/driver/postgres_test.clj

+(ns metabase.driver.postgres-test
+  (:require [expectations :refer :all]
+            [metabase.driver.postgres :refer :all]
+            [metabase.test.util :refer [resolve-private-fns]]))
+
+(resolve-private-fns metabase.driver.postgres
+  connection-details->connection-spec
+  database->connection-details)
+
+;; # Check that database->connection details still works whether we're dealing with new-style or legacy details
+;; ## new-style
+(expect {:db "bird_sightings"
+         :db-type :postgres
+         :make-pool? false
+         :ssl false
+         :port 5432
+         :host "localhost"
+         :user "camsaul"}
+  (database->connection-details {:details {:ssl    false
+                                           :host   "localhost"
+                                           :port   "5432"
+                                           :dbname "bird_sightings"
+                                           :user   "camsaul"}}))
+
+;; ## legacy
+(expect {:db "bird_sightings"
+         :ssl nil
+         :db-type :postgres
+         :make-pool? false
+         :user "camsaul"
+         :port 5432
+         :host "localhost"}
+  (database->connection-details {:details {:conn_str "host=localhost port=5432 dbname=bird_sightings user=camsaul"}}))
+
+
+;; # Check that SSL params get added the connection details in the way we'd like
+;; ## no SSL -- this should *not* include the key :ssl (regardless of its value) since that will cause the PG driver to use SSL anyway
+(expect
+    {:user "camsaul"
+     :host "localhost"
+     :port "5432"
+     :db "bird_sightings"
+     :classname "org.postgresql.Driver"
+     :subprotocol "postgresql"
+     :subname "//localhost:5432/bird_sightings"
+     :make-pool? true}
+  (connection-details->connection-spec {:ssl    false
+                                        :host   "localhost"
+                                        :port   "5432"
+                                        :dbname "bird_sightings"
+                                        :user   "camsaul"}))
+
+;; ## ssl - check that expected params get added
+(expect
+    {:ssl true
+     :make-pool? true
+     :db "bird_sightings"
+     :sslmode "require"
+     :classname "org.postgresql.Driver"
+     :port "5432"
+     :subprotocol "postgresql"
+     :host "localhost"
+     :user "camsaul"
+     :sslfactory "org.postgresql.ssl.NonValidatingFactory"
+     :subname "//localhost:5432/bird_sightings"}
+  (connection-details->connection-spec {:ssl    true
+                                        :host   "localhost"
+                                        :port   "5432"
+                                        :dbname "bird_sightings"
+                                        :user   "camsaul"}))