From 94e5dcf5dfce53b44ce5ec5d97e1053e58441264 Mon Sep 17 00:00:00 2001
From: metamben <103100869+metamben@users.noreply.github.com>
Date: Tue, 14 Jun 2022 22:33:31 +0300
Subject: [PATCH] Make H2 error messages refer to the correct field (#23338)
* Use (str ...) forms to translate multi-line messages
* Make H2 error messages refer to the correct field
---
src/metabase/driver/h2.clj | 4 +--
src/metabase/driver/util.clj | 45 +++++++++++++++++------------
test/metabase/api/database_test.clj | 5 ++--
test/metabase/api/setup_test.clj | 5 ++--
4 files changed, 32 insertions(+), 27 deletions(-)
diff --git a/src/metabase/driver/h2.clj b/src/metabase/driver/h2.clj
index 13138cf4a0e..cbbf81625dc 100644
--- a/src/metabase/driver/h2.clj
+++ b/src/metabase/driver/h2.clj
@@ -108,10 +108,10 @@
[_ message]
(condp re-matches message
#"^A file path that is implicitly relative to the current working directory is not allowed in the database URL .*$"
- :cannot-connect-check-host-and-port
+ :implicitly-relative-db-file-path
#"^Database .* not found .*$"
- :cannot-connect-check-host-and-port
+ :db-file-not-found
#"^Wrong user name or password .*$"
:username-or-password-incorrect
diff --git a/src/metabase/driver/util.clj b/src/metabase/driver/util.clj
index a56a66664b6..4c73f5baa9b 100644
--- a/src/metabase/driver/util.clj
+++ b/src/metabase/driver/util.clj
@@ -24,23 +24,26 @@
"Generic error messages that drivers should return in their implementation
of [[metabase.driver/humanize-connection-error-message]]."
{:cannot-connect-check-host-and-port
- {:message [(deferred-tru "Hmm, we couldn''t connect to the database.")
- " "
- (deferred-tru "Make sure your Host and Port settings are correct")]
+ {:message (deferred-tru
+ (str "Hmm, we couldn''t connect to the database."
+ " "
+ "Make sure your Host and Port settings are correct"))
:errors {:host (deferred-tru "check your host settings")
:port (deferred-tru "check your port settings")}}
:ssh-tunnel-auth-fail
- {:message [(deferred-tru "We couldn''t connect to the SSH tunnel host.")
- " "
- (deferred-tru "Check the Username and Password.")]
+ {:message (deferred-tru
+ (str "We couldn''t connect to the SSH tunnel host."
+ " "
+ "Check the Username and Password."))
:errors {:tunnel-user (deferred-tru "check your username")
:tunnel-pass (deferred-tru "check your password")}}
:ssh-tunnel-connection-fail
- {:message [(deferred-tru "We couldn''t connect to the SSH tunnel host.")
- " "
- (deferred-tru "Check the Host and Port.")]
+ {:message (deferred-tru
+ (str "We couldn''t connect to the SSH tunnel host."
+ " "
+ "Check the Host and Port."))
:errors {:tunnel-host (deferred-tru "check your host settings")
:tunnel-port (deferred-tru "check your port settings")}}
@@ -49,9 +52,10 @@
:errors {:dbname (deferred-tru "check your database name settings")}}
:invalid-hostname
- {:message [(deferred-tru "It looks like your Host is invalid.")
- " "
- (deferred-tru "Please double-check it and try again.")]
+ {:message (deferred-tru
+ (str "It looks like your Host is invalid."
+ " "
+ "Please double-check it and try again."))
:errors {:host (deferred-tru "check your host settings")}}
:password-incorrect
@@ -82,18 +86,21 @@
:requires-ssl
{:message (deferred-tru "Server appears to require SSL - please enable SSL below")
- :errors {:ssl (deferred-tru "please enable SSL")}}})
+ :errors {:ssl (deferred-tru "please enable SSL")}}
-(defn- force-tr [text-or-vector]
- (if (vector? text-or-vector)
- (apply str text-or-vector)
- (str text-or-vector)))
+ :implicitly-relative-db-file-path
+ {:message (deferred-tru "Implicitly relative file paths are not allowed.")
+ :errors {:db (deferred-tru "check your connection string")}}
+
+ :db-file-not-found
+ {:message (deferred-tru "Database cannot be found.")
+ :errors {:db (deferred-tru "check your connection string")}}})
(defn- tr-connection-error-messages [error-type-kw]
(when-let [message (connection-error-messages error-type-kw)]
(cond-> message
- (contains? message :message) (update :message force-tr)
- (contains? message :errors) (update :errors update-vals force-tr))))
+ (contains? message :message) (update :message str)
+ (contains? message :errors) (update :errors update-vals str))))
(comment mdb.connection/keep-me) ; used for [[memoize/ttl]]
diff --git a/test/metabase/api/database_test.clj b/test/metabase/api/database_test.clj
index 6567a2e3fc4..88b6fa26744 100644
--- a/test/metabase/api/database_test.clj
+++ b/test/metabase/api/database_test.clj
@@ -793,9 +793,8 @@
(testing "invalid database connection details"
(testing "calling test-connection-details directly"
- (is (= {:errors {:host "check your host settings"
- :port "check your port settings"}
- :message "Hmm, we couldn't connect to the database. Make sure your Host and Port settings are correct"
+ (is (= {:errors {:db "check your connection string"}
+ :message "Implicitly relative file paths are not allowed."
:valid false}
(#'api.database/test-connection-details "h2" {:db "ABC"}))))
diff --git a/test/metabase/api/setup_test.clj b/test/metabase/api/setup_test.clj
index 0cd2f0141a9..f415410cb71 100644
--- a/test/metabase/api/setup_test.clj
+++ b/test/metabase/api/setup_test.clj
@@ -378,9 +378,8 @@
(client/client :post 400 "setup/validate" {:token (setup/setup-token)}))))
(testing "should validate that database connection works"
- (is (= {:errors {:host "check your host settings"
- :port "check your port settings"}
- :message "Hmm, we couldn't connect to the database. Make sure your Host and Port settings are correct"}
+ (is (= {:errors {:db "check your connection string"},
+ :message "Database cannot be found."}
(client/client :post 400 "setup/validate" {:token (setup/setup-token)
:details {:engine "h2"
:details {:db "file:///tmp/fake.db"}}}))))
--
GitLab