From d2fcd77f2eafe0979eb65e4d7113a5a0cb2aee29 Mon Sep 17 00:00:00 2001
From: Cam Saul <cam@getluckybird.com>
Date: Wed, 13 May 2015 16:45:52 -0700
Subject: [PATCH] :heart_eyes_cat: Add support for new-style connection details
maps for Postgres to Clojure backend. Add some extra unit tests for funsies
---
.dir-locals.el | 1 +
src/metabase/driver/postgres.clj | 33 ++++++++----
test/metabase/driver/postgres_test.clj | 70 ++++++++++++++++++++++++++
3 files changed, 95 insertions(+), 9 deletions(-)
create mode 100644 test/metabase/driver/postgres_test.clj
diff --git a/.dir-locals.el b/.dir-locals.el
index 6ef64b82e46..c633e019cd1 100644
--- a/.dir-locals.el
+++ b/.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)
diff --git a/src/metabase/driver/postgres.clj b/src/metabase/driver/postgres.clj
index 3b03dd8c407..70fa2046758 100644
--- a/src/metabase/driver/postgres.clj
+++ b/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)
diff --git a/test/metabase/driver/postgres_test.clj b/test/metabase/driver/postgres_test.clj
new file mode 100644
index 00000000000..5defacf9d29
--- /dev/null
+++ b/test/metabase/driver/postgres_test.clj
@@ -0,0 +1,70 @@
+(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"}))
--
GitLab