diff --git a/README.md b/README.md index 6d5dc24..a3c8975 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,10 @@ ### Creating a Connection Pool -You can create a C3P0 connection pool with any `clojure.java.jdbc` connection spec map. (Currently, only maps with `:subname` and `:subprotocol` are supported.) `connection-pool-spec` will return a `clojure.java.jdbc` connection spec you can use directly with JDBC: +#### From a `clojure.java.jdbc` spec + +You can create a C3P0 connection pool with any `clojure.java.jdbc` connection spec map with `:subname` and +`:subprotocol` keys. `connection-pool-spec` will return a `clojure.java.jdbc` connection spec you can use directly: ```clj (require '[clojure.java.jdbc :as jdbc] @@ -22,6 +25,18 @@ You can create a C3P0 connection pool with any `clojure.java.jdbc` connection sp (You will almost certainly want to store your pool somewhere, such as in an atom). +#### From a JDBC URL String: + +You can create a pooled `DataSource` (e.g., for use with [`next-jdbc`](https://github.com/seancorfield/next-jdbc)) by calling `pooled-data-source-from-url`: + +```clj +(require '[next.jdbc :as jdbc] + '[metabase.connection-pool :as connection-pool]) + +(with-open [connection (jdbc/get-connection (connection-pool/pooled-data-source-from-url "jdbc:postgres:cam@localhost:3000/my_db"))] + (reduce my-fn init-value (jdbc/plan connection ["SELECT *"]))) +``` + ### Configuring the connection pool You can set connection pool options such as size in a `c3p0.properties` file, or by passing them as a map to `connection-pool-spec`: diff --git a/project.clj b/project.clj index d3d74b1..fa2729e 100644 --- a/project.clj +++ b/project.clj @@ -1,4 +1,4 @@ -(defproject metabase/connection-pool "1.0.3" +(defproject metabase/connection-pool "1.1.0" :description "Connection pools for JDBC databases. Simple wrapper around C3P0." :url "https://github.com/metabase/connection-pool" :min-lein-version "2.5.0" @@ -24,6 +24,8 @@ [com.h2database/h2 "1.4.197"] [pjstadig/humane-test-output "0.9.0"]] + :global-vars {*warn-on-reflection* true} + :jvm-opts ["-Xverify:none"]} diff --git a/src/metabase/connection_pool.clj b/src/metabase/connection_pool.clj index 34b9749..6ae087b 100644 --- a/src/metabase/connection_pool.clj +++ b/src/metabase/connection_pool.clj @@ -22,17 +22,19 @@ (reify DataSource (getConnection [_] (.connect driver jdbc-url properties)) + (getConnection [_ username password] - (doseq [[k v] {"user" username, "password" password}] - (if (some? k) - (.setProperty properties k (name v)) - (.remove properties k))) - (.connect driver jdbc-url properties))))) + (let [properties (or properties (Properties.))] + (doseq [[k v] {"user" username, "password" password}] + (if (some? v) + (.setProperty properties k (name v)) + (.remove properties k))) + (.connect driver jdbc-url properties)))))) ;;; ------------------------------------------- Creating Connection Pools -------------------------------------------- -(defn- map->properties +(defn map->properties "Create a `Properties` object from a JDBC connection spec map. Properties objects are maps of String -> String, so all keys and values are converted to Strings appropriately." ^Properties [m] @@ -46,16 +48,23 @@ (defn- spec->properties ^Properties [spec] (map->properties (dissoc spec :classname :subprotocol :subname))) -(defn ^:private unpooled-data-source - ^DataSource [{:keys [subname subprotocol], :as spec}] - (proxy-data-source (format "jdbc:%s:%s" subprotocol subname) (spec->properties spec))) +(defn- unpooled-data-source + (^DataSource [{:keys [subname subprotocol], :as spec}] + (proxy-data-source (format "jdbc:%s:%s" subprotocol subname) (spec->properties spec))) -(defn- pooled-data-source ^DataSource - ([spec] + (^DataSource [driver {:keys [subname subprotocol], :as spec}] + (proxy-data-source driver (format "jdbc:%s:%s" subprotocol subname) (spec->properties spec)))) + +(defn pooled-data-source + "Create a new pooled DataSource from a `clojure.java.jdbc` spec." + (^DataSource [spec] (DataSources/pooledDataSource (unpooled-data-source spec))) - ([spec pool-properties-map] - (DataSources/pooledDataSource (unpooled-data-source spec) (map->properties pool-properties-map)))) + (^DataSource [spec pool-properties-map] + (DataSources/pooledDataSource (unpooled-data-source spec) (map->properties pool-properties-map))) + + (^DataSource [driver spec pool-properties-map] + (DataSources/pooledDataSource (unpooled-data-source driver spec) (map->properties pool-properties-map)))) (defn connection-pool-spec "Create a new connection pool for a JDBC `spec` and return a spec for it. Optionally pass a map of connection pool @@ -65,8 +74,21 @@ {:datasource (pooled-data-source spec)}) ([spec pool-properties-map] - {:datasource (pooled-data-source spec pool-properties-map)})) + {:datasource (pooled-data-source spec pool-properties-map)}) + + ([driver spec pool-properties-map] + {:datasource (pooled-data-source driver spec pool-properties-map)})) + +(defn pooled-data-source-from-url + "Create a new pooled DataSource from a JDBC URL string." + (^DataSource [url] + (DataSources/pooledDataSource (proxy-data-source url nil))) + + (^DataSource [url pool-properties-map] + (DataSources/pooledDataSource (proxy-data-source url nil) (map->properties pool-properties-map))) + (^DataSource [driver url pool-properties-map] + (DataSources/pooledDataSource (proxy-data-source driver url nil) (map->properties pool-properties-map)))) (defn destroy-connection-pool! "Immediately release all resources held by a connection pool." diff --git a/test/metabase/connection_pool_test.clj b/test/metabase/connection_pool_test.clj index 1afa610..63581fa 100644 --- a/test/metabase/connection_pool_test.clj +++ b/test/metabase/connection_pool_test.clj @@ -1,19 +1,78 @@ (ns metabase.connection-pool-test - (:require [clojure.test :as t] + (:require [clojure.test :refer :all] [metabase.connection-pool :as connection-pool])) (def ^:private spec {:classname "org.h2.Driver", :subprotocol "h2", :subname "mem:db"}) -(t/deftest properties-test - (t/testing "Options passed in to `connection-pool-spec` should get parsed correctly" +(deftest properties-test + (testing "Options passed in to `connection-pool-spec` should get parsed correctly" (let [description (-> (connection-pool/connection-pool-spec spec {"acquireIncrement" 1 "testConnectionOnCheckin" true}) :datasource str)] - (t/is (= "acquireIncrement -> 1" - (re-find #"acquireIncrement -> \d" description)) - "numeric options should get converted correctly") - (t/is (= "testConnectionOnCheckin -> true" - (re-find #"testConnectionOnCheckin -> \w+" description)) - "boolean options should get converted correctly")))) + (is (= "acquireIncrement -> 1" + (re-find #"acquireIncrement -> \d" description)) + "numeric options should get converted correctly") + (is (= "testConnectionOnCheckin -> true" + (re-find #"testConnectionOnCheckin -> \w+" description)) + "boolean options should get converted correctly")))) + +(deftest map->properties-test + (testing "Properties should be converted to strings" + ;; Properties are equality-comparable to maps + (is (= {"A" "true", "B" "false", "C" "100"} + (connection-pool/map->properties {:A "true", "B" false, "C" 100}))))) + +(defrecord ^:private FakeConnection [url props] + java.sql.Connection) + +(defrecord ^:private FakeDriver [] + java.sql.Driver + (connect [_ url props] + (FakeConnection. url props))) + +(defn- proxy-data-source ^javax.sql.DataSource [& args] + (apply #'connection-pool/proxy-data-source args)) + +(deftest proxy-data-source-test + (testing "Make sure we can create a data source with an explicit driver instance" + (is (= (FakeConnection. "jdbc:my-fake-db:localhost" nil) + (.getConnection (proxy-data-source (FakeDriver.) "jdbc:my-fake-db:localhost" nil))))) + + (testing "Make sure username/password are set when using the 3-arg getConnection method" + (doseq [props [(java.util.Properties.) nil (doto (java.util.Properties.) + (.setProperty "password" "abc") + (.setProperty "user" "cam-2"))]] + (testing (format "with initial properties = %s" (pr-str props)) + (is (= (FakeConnection. "jdbc:my-fake-db:localhost" {"password" "passw0rd", "user" "cam"}) + (.getConnection (proxy-data-source (FakeDriver.) "jdbc:my-fake-db:localhost" props) + "cam" + "passw0rd")))))) + + (testing "passing nil username/password to 3-arg getConnection method should existing props" + (let [props (doto (java.util.Properties.) + (.setProperty "password" "abc") + (.setProperty "user" "cam-2"))] + (is (= (FakeConnection. "jdbc:my-fake-db:localhost" {}) + (.getConnection (proxy-data-source (FakeDriver.) "jdbc:my-fake-db:localhost" props) + nil + nil)))))) + +(deftest connection-pool-spec-test + (let [{:keys [^javax.sql.DataSource datasource]} (connection-pool/connection-pool-spec + {:subprotocol "h2", :subname "mem:in-memory"})] + (with-open [conn (.getConnection datasource) + stmt (.prepareStatement conn "SELECT 1 AS one;") + rset (.executeQuery stmt)] + (.next rset) + (is (= 1 + (.getObject rset 1)))))) + +(deftest pooled-data-source-from-url-test + (with-open [conn (.getConnection (connection-pool/pooled-data-source-from-url "jdbc:h2:mem:in-memory")) + stmt (.prepareStatement conn "SELECT 1 AS one;") + rset (.executeQuery stmt)] + (.next rset) + (is (= 1 + (.getObject rset 1)))))