From 18f6eb36124b62bd80bf72c4b9a3212b162352e1 Mon Sep 17 00:00:00 2001
From: Jeff Evans <jeff303@users.noreply.github.com>
Date: Mon, 16 Aug 2021 16:33:42 -0500
Subject: [PATCH] Make DB name in c3p0 name more robust (#17447)
Consult any of these keys when trying to determine the "name" portion of the c3p0 data: :db :dbname :sid :catalog
Update existing `c3p0-datasource-name-test` to more robustly check the different portions of the c3p0 dataSourceName
---
src/metabase/driver/sql_jdbc/connection.clj | 11 +++++++++--
test/metabase/driver/sql_jdbc/connection_test.clj | 12 +++++++-----
2 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/src/metabase/driver/sql_jdbc/connection.clj b/src/metabase/driver/sql_jdbc/connection.clj
index 00e64f5618c..eb9878e2915 100644
--- a/src/metabase/driver/sql_jdbc/connection.clj
+++ b/src/metabase/driver/sql_jdbc/connection.clj
@@ -77,8 +77,15 @@
;;
;; Kill idle connections above the minPoolSize after 5 minutes.
"maxIdleTimeExcessConnections" (* 5 60)
- ;; Set the data source name so that the c3p0 JMX bean has a useful identifier
- "dataSourceName" (format "db-%d-%s-%s" (u/the-id database) (name driver) (-> database :details :db))})
+ ;; Set the data source name so that the c3p0 JMX bean has a useful identifier, which incorporates the DB ID, driver,
+ ;; and name; to find a "name" in the details, just look for the first key that is set and could make sense, rather
+ ;; than introducing a new driver level multimethod just for this
+ "dataSourceName" (format "db-%d-%s-%s" (u/the-id database) (name driver) (->> database
+ :details
+ ((some-fn :db
+ :dbname
+ :sid
+ :catalog))))})
(defn- create-pool!
"Create a new C3P0 `ComboPooledDataSource` for connecting to the given `database`."
diff --git a/test/metabase/driver/sql_jdbc/connection_test.clj b/test/metabase/driver/sql_jdbc/connection_test.clj
index 285cf3e7595..4617cefc3ba 100644
--- a/test/metabase/driver/sql_jdbc/connection_test.clj
+++ b/test/metabase/driver/sql_jdbc/connection_test.clj
@@ -65,11 +65,13 @@
(deftest c3p0-datasource-name-test
(mt/test-drivers (sql-jdbc.tu/sql-jdbc-drivers)
(testing "The dataSourceName c3p0 property is set properly for a database"
- (let [db (mt/db)
- props (sql-jdbc.conn/data-warehouse-connection-pool-properties driver/*driver* db)
- expected (format "db-%d-%s-%s" (u/the-id db) (name driver/*driver*) (-> db :details :db))]
- (is (= expected
- (get props "dataSourceName")))))))
+ (let [db (mt/db)
+ props (sql-jdbc.conn/data-warehouse-connection-pool-properties driver/*driver* db)
+ [_ db-nm] (re-matches (re-pattern (format "^db-%d-%s-(.*)$" (u/the-id db) (name driver/*driver*)))
+ (get props "dataSourceName"))]
+ (is (some? db-nm))
+ ;; ensure that, for any sql-jdbc drier anyway, we found *some* DB name to use in this String
+ (is (not= db-nm "null"))))))
(deftest connection-pool-invalidated-on-details-change
(mt/test-drivers (sql-jdbc.tu/sql-jdbc-drivers)
--
GitLab