Skip to content
Snippets Groups Projects
Unverified Commit e14854c9 authored by Cam Saul's avatar Cam Saul Committed by GitHub
Browse files

Better with-temp error messages (#17889) 

* Better with-temp error messages

* Undo wacky indentation
parent 3c83fb6e
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
test/metabase/models/user_test.clj
+ 5
− 3
View file @ e14854c9
......@@ -415,8 +415,9 @@
(is (= "en_US"
(db/select-one-field :locale User :id user-id)))))
(testing "invalid locale"
(is (thrown?
AssertionError
(is (thrown-with-msg?
Exception
#"Assert failed: \(i18n/available-locale\? locale\)"
(mt/with-temp User [{user-id :id} {:locale "en_XX"}])))))
(testing "updating a User"
......@@ -426,8 +427,9 @@
(is (= "en_GB"
(db/select-one-field :locale User :id user-id))))
(testing "invalid locale"
(is (thrown?
(is (thrown-with-msg?
AssertionError
#"Assert failed: \(i18n/available-locale\? locale\)"
(db/update! User user-id :locale "en_XX"))))))))
(deftest normalize-locale-test
......
test/metabase/pulse_test.clj
+ 5
− 6
View file @ e14854c9
......@@ -754,9 +754,8 @@
(send-pulse-created-by-user! user-kw card)))]
(is (= [[1 "2014-04-07T00:00:00Z" 5 12]]
(send-pulse-created-by-user!* :crowberto)))
(is (thrown-with-msg?
clojure.lang.ExceptionInfo
#"^You do not have permissions to view Card [\d,]+."
(mt/suppress-output
(send-pulse-created-by-user!* :rasta)))
"If the current user doesn't have permissions to execute the Card for a Pulse, an Exception should be thrown."))))
(testing "If the current user doesn't have permissions to execute the Card for a Pulse, an Exception should be thrown."
(is (thrown-with-msg?
clojure.lang.ExceptionInfo
#"You do not have permissions to view Card [\d,]+."
(send-pulse-created-by-user!* :rasta)))))))
test/metabase/query_processor/middleware/permissions_test.clj
+ 74
− 63
View file @ e14854c9
......@@ -24,19 +24,20 @@
(defn- check-perms-for-rasta
"Check permissions for `query` with rasta as the current user."
{:style/indent 0}
[query]
(do-with-rasta (fn [] (check-perms query))))
(def ^:private perms-error-msg #"^You do not have permissions to run this query\.")
(def ^:private perms-error-msg #"You do not have permissions to run this query\.")
(deftest native-query-perms-test
(testing "Make sure the NATIVE query fails to run if current user doesn't have perms"
(is (thrown-with-msg? ExceptionInfo perms-error-msg
(check-perms-for-rasta
{:database 1000
:type :native
:native {:query "SELECT * FROM VENUES"}}))))
(is (thrown-with-msg?
ExceptionInfo
perms-error-msg
(check-perms-for-rasta
{:database 1000
:type :native
:native {:query "SELECT * FROM VENUES"}}))))
(testing "...but it should work if user has perms"
(mt/with-temp Database [db]
......@@ -51,15 +52,17 @@
(deftest mbql-query-perms-test
(testing "Make sure the MBQL query fails to run if current user doesn't have perms"
(is (thrown-with-msg? ExceptionInfo perms-error-msg
(mt/with-temp* [Database [db]
Table [table {:db_id (u/the-id db)}]]
;; All users get perms for all new DBs by default
(perms/revoke-data-perms! (perms-group/all-users) (u/the-id db))
(check-perms-for-rasta
{:database (u/the-id db)
:type :query
:query {:source-table (u/the-id table)}})))))
(is (thrown-with-msg?
ExceptionInfo
perms-error-msg
(mt/with-temp* [Database [db]
Table [table {:db_id (u/the-id db)}]]
;; All users get perms for all new DBs by default
(perms/revoke-data-perms! (perms-group/all-users) (u/the-id db))
(check-perms-for-rasta
{:database (u/the-id db)
:type :query
:query {:source-table (u/the-id table)}})))))
(testing "...but it should work if user has perms [MBQL]"
(mt/with-temp* [Database [db]
......@@ -75,11 +78,13 @@
(deftest nested-native-query-test
(testing "Make sure nested native query fails to run if current user doesn't have perms"
(is (thrown-with-msg? ExceptionInfo perms-error-msg
(check-perms-for-rasta
{:database 1000
:type :query
:query {:source-query {:native "SELECT * FROM VENUES"}}}))))
(is (thrown-with-msg?
ExceptionInfo
perms-error-msg
(check-perms-for-rasta
{:database 1000
:type :query
:query {:source-query {:native "SELECT * FROM VENUES"}}}))))
(testing "...but it should work if user has perms [nested native queries]"
(mt/with-temp Database [db]
......@@ -94,15 +99,17 @@
(deftest nested-mbql-query-test
(testing "Make sure nested MBQL query fails to run if current user doesn't have perms"
(is (thrown-with-msg? ExceptionInfo perms-error-msg
(mt/with-temp* [Database [db]
Table [table {:db_id (u/the-id db)}]]
;; All users get perms for all new DBs by default
(perms/revoke-data-perms! (perms-group/all-users) (u/the-id db))
(check-perms-for-rasta
{:database (u/the-id db)
:type :query
:query {:source-query {:source-table (u/the-id table)}}})))))
(is (thrown-with-msg?
ExceptionInfo
perms-error-msg
(mt/with-temp* [Database [db]
Table [table {:db_id (u/the-id db)}]]
;; All users get perms for all new DBs by default
(perms/revoke-data-perms! (perms-group/all-users) (u/the-id db))
(check-perms-for-rasta
{:database (u/the-id db)
:type :query
:query {:source-query {:source-table (u/the-id table)}}})))))
(testing "...but it should work if user has perms [nested MBQL queries]"
(mt/with-temp* [Database [db]
......@@ -117,23 +124,25 @@
(deftest template-tags-referenced-queries-test
(testing "Fails for MBQL query referenced in template tag, when user has no perms to referenced query"
(is (thrown-with-msg? ExceptionInfo perms-error-msg
(mt/with-temp* [Database [db]
Table [table-1 {:db_id (u/the-id db)}]
Table [table-2 {:db_id (u/the-id db)}]
Card [card {:dataset_query {:database (u/the-id db), :type :query,
:query {:source-table (u/the-id table-2)}}}]]
;; All users get perms for all new DBs by default
(perms/revoke-data-perms! (perms-group/all-users) (u/the-id db) nil (u/the-id table-2))
(let [card-id (:id card)
tag-name (str "#" card-id)]
(check-perms-for-rasta
{:database (u/the-id db)
:type :native
:native {:query (format "SELECT * FROM {{%s}} AS x" tag-name)
:template-tags {tag-name
{:id tag-name, :name tag-name, :display-name tag-name,
:type "card", :card card-id}}}}))))))
(is (thrown-with-msg?
ExceptionInfo
perms-error-msg
(mt/with-temp* [Database [db]
Table [table-1 {:db_id (u/the-id db)}]
Table [table-2 {:db_id (u/the-id db)}]
Card [card {:dataset_query {:database (u/the-id db), :type :query,
:query {:source-table (u/the-id table-2)}}}]]
;; All users get perms for all new DBs by default
(perms/revoke-data-perms! (perms-group/all-users) (u/the-id db) nil (u/the-id table-2))
(let [card-id (:id card)
tag-name (str "#" card-id)]
(check-perms-for-rasta
{:database (u/the-id db)
:type :native
:native {:query (format "SELECT * FROM {{%s}} AS x" tag-name)
:template-tags {tag-name
{:id tag-name, :name tag-name, :display-name tag-name,
:type "card", :card card-id}}}}))))))
(testing "...but it should work if user has perms [template tag referenced query]"
(mt/with-temp* [Database [db]
......@@ -159,22 +168,24 @@
:type "card", :card card-id}}}}))))))
(testing "Fails for native query referenced in template tag, when user has no perms to referenced query"
(is (thrown-with-msg? ExceptionInfo perms-error-msg
(mt/with-temp* [Database [db]
Card [card {:dataset_query
{:database (u/the-id db), :type :native,
:native {:query "SELECT 1 AS \"foo\", 2 AS \"bar\", 3 AS \"baz\""}}}]]
;; All users get perms for all new DBs by default
(perms/revoke-data-perms! (perms-group/all-users) (u/the-id db))
(let [card-id (:id card)
tag-name (str "#" card-id)]
(check-perms-for-rasta
{:database (u/the-id db)
:type :native
:native {:query (format "SELECT * FROM {{%s}} AS x" tag-name)
:template-tags {tag-name
{:id tag-name, :name tag-name, :display-name tag-name,
:type "card", :card card-id}}}}))))))
(is (thrown-with-msg?
ExceptionInfo
perms-error-msg
(mt/with-temp* [Database [db]
Card [card {:dataset_query
{:database (u/the-id db), :type :native,
:native {:query "SELECT 1 AS \"foo\", 2 AS \"bar\", 3 AS \"baz\""}}}]]
;; All users get perms for all new DBs by default
(perms/revoke-data-perms! (perms-group/all-users) (u/the-id db))
(let [card-id (:id card)
tag-name (str "#" card-id)]
(check-perms-for-rasta
{:database (u/the-id db)
:type :native
:native {:query (format "SELECT * FROM {{%s}} AS x" tag-name)
:template-tags {tag-name
{:id tag-name, :name tag-name, :display-name tag-name,
:type "card", :card card-id}}}}))))))
(testing "...but it should work if user has perms [template tag referenced query]"
(mt/with-temp* [Database [db]
......
test/metabase/test/redefs.clj
+ 13
− 1
View file @ e14854c9
......@@ -19,7 +19,19 @@
;; TODO -- there's not really a reason that we can't use with-temp in parallel tests -- it depends on the test -- so
;; once we're a little more comfortable with the current parallel stuff we should remove this restriction.
(test-runner.parallel/assert-test-is-not-parallel "with-temp")
(apply orig-do-with-temp args))
;; catch any Exceptions thrown by the original call and rethrow them with some extra context to make them a little
;; easier to debug.
(try
(apply orig-do-with-temp args)
(catch Throwable e
;; only wrap the Exception with additional context if it's not already wrapped. We don't need a bunch of nested
;; `do-with-temp` calls that didn't fail wrapping the Exception from the one that did and adding a bunch of
;; noise
(if (::with-temp-error (ex-data e))
(throw e)
(throw (ex-info (str "with-temp error: " (ex-message e))
{:args args, ::with-temp-error true}
e))))))
(alter-var-root #'tt/do-with-temp (constantly do-with-temp))
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment