diff --git a/modules/drivers/bigquery/test/metabase/driver/bigquery_test.clj b/modules/drivers/bigquery/test/metabase/driver/bigquery_test.clj
index be726da68acdc94893151a80de0002a800a7907c..75315afb626aa1692d4f9b321a056ac74efe2af7 100644
--- a/modules/drivers/bigquery/test/metabase/driver/bigquery_test.clj
+++ b/modules/drivers/bigquery/test/metabase/driver/bigquery_test.clj
@@ -160,9 +160,9 @@
"ORDER BY `name` ASC")
;; normally for test purposes BigQuery doesn't support foreign keys so override the function that checks that and
;; make it return `true` so this test proceeds as expected
- (with-redefs [driver/supports? (constantly true)]
- (tu/with-temp-vals-in-db 'Field (data/id :venues :category_id) {:fk_target_field_id (data/id :categories :id)
- :special_type "type/FK"}
+ (with-redefs [driver/supports? (constantly true)]
+ (tu/with-temp-vals-in-db Field (data/id :venues :category_id) {:fk_target_field_id (data/id :categories :id)
+ :special_type "type/FK"}
(let [results (qp/process-query
{:database (data/id)
:type "query"
diff --git a/modules/drivers/oracle/test/metabase/driver/oracle_test.clj b/modules/drivers/oracle/test/metabase/driver/oracle_test.clj
index 2440ff936769d2573b72e4631c15e0b36130b96a..a5c9901543b11712db020001a094abc8a1007382 100644
--- a/modules/drivers/oracle/test/metabase/driver/oracle_test.clj
+++ b/modules/drivers/oracle/test/metabase/driver/oracle_test.clj
@@ -1,7 +1,7 @@
(ns metabase.driver.oracle-test
"Tests for specific behavior of the Oracle driver."
(:require [clojure.java.jdbc :as jdbc]
- [expectations :refer :all]
+ [expectations :refer [expect]]
[metabase
[driver :as driver]
[query-processor :as qp]
diff --git a/src/metabase/api/embed.clj b/src/metabase/api/embed.clj
index 99a755c07f6ea57fd9702862e98b6e059b3b0c31..7ad440fa3bf912b910f4ffebd35430188b5fb549 100644
--- a/src/metabase/api/embed.clj
+++ b/src/metabase/api/embed.clj
@@ -357,6 +357,7 @@
[token dashcard-id card-id & query-params]
(card-for-signed-token token dashcard-id card-id query-params ))
+
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | FieldValues, Search, Remappings |
;;; +----------------------------------------------------------------------------------------------------------------+
diff --git a/src/metabase/driver.clj b/src/metabase/driver.clj
index 33e6f8b682ba5f5b9f16a5ed86a80437f40255e7..85503dfff1d861f2be04afbbe2fa473df3fe02e3 100644
--- a/src/metabase/driver.clj
+++ b/src/metabase/driver.clj
@@ -123,7 +123,7 @@
(when-not (registered? driver)
(throw (Exception. (str (tru "Driver not registered after loading: {0}" driver))))))))))
-(defn the-driver
+(s/defn the-driver
"Like Clojure core `the-ns`. Converts argument to a keyword, then loads and registers the driver if not already done,
throwing an Exception if it fails or is invalid. Returns keyword.
@@ -140,7 +140,7 @@
(the-driver :postgres) ; -> :postgres
(the-driver :baby) ; -> Exception"
- [driver]
+ [driver :- (s/cond-pre s/Str s/Keyword)]
(let [driver (keyword driver)]
(load-driver-namespace-if-needed driver)
driver))
diff --git a/src/metabase/driver/sql_jdbc/execute.clj b/src/metabase/driver/sql_jdbc/execute.clj
index 76eabd5c5d8ad12e4948843553045256caa0ae12..e204976465c3d2962153b36dd9f7d742c0e8da96 100644
--- a/src/metabase/driver/sql_jdbc/execute.clj
+++ b/src/metabase/driver/sql_jdbc/execute.clj
@@ -147,16 +147,20 @@
;;; | Running Queries |
;;; +----------------------------------------------------------------------------------------------------------------+
+;; TODO - this is pretty similar to what `jdbc/with-db-connection` does, but not exactly the same. See if we can
+;; switch to using that instead?
+(defn- do-with-ensured-connection [db f]
+ (if-let [conn (jdbc/db-find-connection db)]
+ (f conn)
+ (with-open [conn (jdbc/get-connection db)]
+ (f conn))))
+
(defmacro ^:private with-ensured-connection
"In many of the clojure.java.jdbc functions, it checks to see if there's already a connection open before opening a
new one. This macro checks to see if one is open, or will open a new one. Will bind the connection to `conn-sym`."
{:style/indent 1}
- [[conn-sym db] & body]
- `(let [db# ~db]
- (if-let [~conn-sym (jdbc/db-find-connection db#)]
- (do ~@body)
- (with-open [~conn-sym (jdbc/get-connection db#)]
- ~@body))))
+ [[conn-binding db] & body]
+ `(do-with-ensured-connection ~db (fn [~conn-binding] ~@body)))
(defn- cancelable-run-query
"Runs `sql` in such a way that it can be interrupted via a `future-cancel`"
@@ -206,10 +210,11 @@
and rethrowing the exception as an Exception with a nicely formatted error message."
{:style/indent 0}
[f]
- (try (f)
- (catch SQLException e
- (log/error (jdbc/print-sql-exception-chain e))
- (throw (Exception. (exception->nice-error-message e))))))
+ (try
+ (f)
+ (catch SQLException e
+ (log/error (jdbc/print-sql-exception-chain e))
+ (throw (Exception. (exception->nice-error-message e) e)))))
(defn- do-with-auto-commit-disabled
"Disable auto-commit for this transaction, and make the transaction `rollback-only`, which means when the
@@ -222,8 +227,9 @@
(.setAutoCommit (jdbc/get-connection conn) false)
;; TODO - it would be nice if we could also `.setReadOnly` on the transaction as well, but that breaks setting the
;; timezone. Is there some way we can have our cake and eat it too?
- (try (f)
- (finally (.rollback (jdbc/get-connection conn)))))
+ (try
+ (f)
+ (finally (.rollback (jdbc/get-connection conn)))))
(defn- do-in-transaction [connection f]
(jdbc/with-db-transaction [transaction-connection connection]
diff --git a/test/metabase/api/permissions_test.clj b/test/metabase/api/permissions_test.clj
index 95df641f818d1fe817c149784a93106faabb5558..3ee4b2bad1d910e7e58cb58423598cf63b25501f 100644
--- a/test/metabase/api/permissions_test.clj
+++ b/test/metabase/api/permissions_test.clj
@@ -18,7 +18,10 @@
(expect
#{{:id (u/get-id (group/all-users)), :name "All Users", :member_count 3}
{:id (u/get-id (group/admin)), :name "Administrators", :member_count 1}}
- (fetch-groups))
+ ;; make sure test users are created first, otherwise we're possibly going to have some WEIRD results
+ (do
+ (test-users/create-users-if-needed!)
+ (fetch-groups)))
;; The endpoint should however return empty groups!
(tt/expect-with-temp [PermissionsGroup [group]]
@@ -34,9 +37,11 @@
#{{:first_name "Crowberto", :last_name "Corv", :email "crowberto@metabase.com", :user_id (test-users/user->id :crowberto), :membership_id true}
{:first_name "Lucky", :last_name "Pigeon", :email "lucky@metabase.com", :user_id (test-users/user->id :lucky), :membership_id true}
{:first_name "Rasta", :last_name "Toucan", :email "rasta@metabase.com", :user_id (test-users/user->id :rasta), :membership_id true}}
- (set
- (for [member (:members ((test-users/user->client :crowberto) :get 200 (str "permissions/group/" (u/get-id (group/all-users)))))]
- (update member :membership_id some?))))
+ (do
+ (test-users/create-users-if-needed!)
+ (set
+ (for [member (:members ((test-users/user->client :crowberto) :get 200 (str "permissions/group/" (u/get-id (group/all-users)))))]
+ (update member :membership_id some?)))))
;; make sure we can update the perms graph from the API
@@ -46,6 +51,7 @@
(data/id :users) :none
(data/id :venues) :all}
(tt/with-temp PermissionsGroup [group]
+ (test-users/create-users-if-needed!)
((test-users/user->client :crowberto) :put 200 "permissions/graph"
(assoc-in (perms/graph)
[:groups (u/get-id group) (data/id) :schemas]
diff --git a/test/metabase/api/setup_test.clj b/test/metabase/api/setup_test.clj
index 2d6d43913d397e8b54f3231f3c8e08aefc49eb57..4d7a4972771e4260fe4310333c88a4c742fdc82e 100644
--- a/test/metabase/api/setup_test.clj
+++ b/test/metabase/api/setup_test.clj
@@ -1,11 +1,12 @@
(ns metabase.api.setup-test
"Tests for /api/setup endpoints."
- (:require [expectations :refer :all]
+ (:require [expectations :refer [expect]]
[metabase
[http-client :as http]
[public-settings :as public-settings]
[setup :as setup]]
[metabase.models.user :refer [User]]
+ [metabase.test.data.users :as test-users]
[metabase.test.util :as tu]
[toucan.db :as db]))
@@ -13,20 +14,25 @@
;; Check that we can create a new superuser via setup-token
(let [email (tu/random-email)]
(expect
- [true
- email]
+ {:uuid? true, :admin-email email}
(try
+ ;; make sure the default test users are created before running this test, otherwise we're going to run into
+ ;; issues if it attempts to delete this user and it is the only admin test user
+ (test-users/create-users-if-needed!)
(tu/with-temporary-setting-values [admin-email nil]
- [(tu/is-uuid-string? (:id (http/client :post 200 "setup" {:token (setup/create-token!)
+ {:uuid?
+ (tu/is-uuid-string? (:id (http/client :post 200 "setup" {:token (setup/create-token!)
:prefs {:site_name "Metabase Test"}
:user {:first_name (tu/random-name)
:last_name (tu/random-name)
:email email
:password "anythingUP12!!"}})))
+
+ :admin-email
(do
;; reset our setup token
(setup/create-token!)
- (public-settings/admin-email))])
+ (public-settings/admin-email))})
(finally
(db/delete! User :email email)))))
diff --git a/test/metabase/test/util.clj b/test/metabase/test/util.clj
index 64c56312f2d935139e6af127ea2a4062bc71c46a..c9b7dca0f02a933704ab5d552ae3531aa9a50021 100644
--- a/test/metabase/test/util.clj
+++ b/test/metabase/test/util.clj
@@ -305,7 +305,8 @@
[model object-or-id column->temp-value f]
;; use low-level `query` and `execute` functions here, because Toucan `select` and `update` functions tend to do
;; things like add columns like `common_name` that don't actually exist, causing subsequent update to fail
- (let [[original-column->value] (db/query {:select (keys column->temp-value)
+ (let [model (db/resolve-model model)
+ [original-column->value] (db/query {:select (keys column->temp-value)
:from [model]
:where [:= :id (u/get-id object-or-id)]})]
(try