diff --git a/src/metabase/api/session.clj b/src/metabase/api/session.clj
index 618cadd91551c7f2bfd48d2fb745864a4d601a67..3b27ad0ee0c7c9ec1abcc6cd5f435bd36999d16e 100644
--- a/src/metabase/api/session.clj
+++ b/src/metabase/api/session.clj
@@ -174,8 +174,11 @@
(when-not (:last_login user)
(email/send-user-joined-admin-notification-email! (User user-id)))
;; after a successful password update go ahead and offer the client a new session that they can use
- {:success true
- :session_id (create-session! user)})
+ (let [session-id (create-session! user)]
+ (mw.session/set-session-cookie
+ {:success true
+ :session_id (str session-id)}
+ session-id)))
(api/throw-invalid-param-exception :password (tru "Invalid reset token"))))
diff --git a/src/metabase/api/setup.clj b/src/metabase/api/setup.clj
index a76b677f6c89744801b3051b74625952882b37af..34db8770dfb673b615de27f0f434e6026c971562 100644
--- a/src/metabase/api/setup.clj
+++ b/src/metabase/api/setup.clj
@@ -77,7 +77,7 @@
(setup/clear-token!)
;; then we create a session right away because we want our new user logged in to continue the setup process
(db/insert! Session
- :id (str session-id)
+ :id (str session-id)
:user_id (:id new-user))
;; notify that we've got a new user in the system AND that this user logged in
(events/publish-event! :user-create {:user_id (:id new-user)})
diff --git a/src/metabase/middleware/session.clj b/src/metabase/middleware/session.clj
index c47a2c5418d0d09851a51ce3fdf3404081af4df9..ecb64c7b7e731674106d1493bf6a25100d2a07b4 100644
--- a/src/metabase/middleware/session.clj
+++ b/src/metabase/middleware/session.clj
@@ -13,7 +13,8 @@
[schema.core :as s]
[toucan.db :as db])
(:import java.net.URL
- java.util.UUID))
+ java.util.UUID
+ org.joda.time.DateTime))
(def ^:private ^String metabase-session-cookie "metabase.SESSION_ID")
(def ^:private ^String metabase-session-header "x-metabase-session")
@@ -40,14 +41,14 @@
(defn clear-session-cookie
"Add a header to `response` to clear the current Metabase session cookie."
[response]
- (println "response:" response "->" (resp/set-cookie response nil {:expires "01 Jan 1970 00:00:00 GMT"})) ; NOCOMMIT
- #_(resp/set-cookie response nil {:expires "01 Jan 1970 00:00:00 GMT"}))
+ (resp/set-cookie response metabase-session-cookie nil {:expires (DateTime. 0)}))
(defn- wrap-session-id* [{:keys [cookies headers] :as request}]
- (if-let [session-id (or (get-in cookies [metabase-session-cookie :value])
- (headers metabase-session-header))]
- (assoc request :metabase-session-id session-id)
- request))
+ (let [session-id (or (get-in cookies [metabase-session-cookie :value])
+ (headers metabase-session-header))]
+ (if (seq session-id)
+ (assoc request :metabase-session-id session-id)
+ request)))
(defn wrap-session-id
"Middleware that sets the `:metabase-session-id` keyword on the request if a session id can be found.
diff --git a/test/expectation_options.clj b/test/expectation_options.clj
index f9dbc19aa97febe1b371e6ae586e0614ec33eec4..635fa87d7c302b8ce38fcdbcd79a0eaf48e4ba4b 100644
--- a/test/expectation_options.clj
+++ b/test/expectation_options.clj
@@ -1,5 +1,53 @@
(ns expectation-options
- "Namespace expectations will automatically load before running a tests")
+ "Namespace expectations will automatically load before running a tests"
+ (:require [clojure
+ [data :as data]
+ [set :as set]]
+ [expectations :as expectations]
+ [metabase.util :as u]))
+
+;;; ---------------------------------------- Expectations Framework Settings -----------------------------------------
+
+;; ## EXPECTATIONS FORMATTING OVERRIDES
+
+;; These overrides the methods Expectations usually uses for printing failed tests.
+;; These are basically the same as the original implementations, but they colorize and pretty-print the
+;; output, which makes it an order of magnitude easier to read, especially for tests that compare a
+;; lot of data, like Query Processor or API tests.
+(defn- format-failure [e a str-e str-a]
+ {:type :fail
+ :expected-message (when-let [in-e (first (data/diff e a))]
+ (format "\nin expected, not actual:\n%s" (u/pprint-to-str 'green in-e)))
+ :actual-message (when-let [in-a (first (data/diff a e))]
+ (format "\nin actual, not expected:\n%s" (u/pprint-to-str 'red in-a)))
+ :raw [str-e str-a]
+ :result ["\nexpected:\n"
+ (u/pprint-to-str 'green e)
+ "\nwas:\n"
+ (u/pprint-to-str 'red a)]})
+
+(defmethod expectations/compare-expr :expectations/maps [e a str-e str-a]
+ (let [[in-e in-a] (data/diff e a)]
+ (if (and (nil? in-e) (nil? in-a))
+ {:type :pass}
+ (format-failure e a str-e str-a))))
+
+(defmethod expectations/compare-expr :expectations/sets [e a str-e str-a]
+ (format-failure e a str-e str-a))
+
+(defmethod expectations/compare-expr :expectations/sequentials [e a str-e str-a]
+ (let [diff-fn (fn [e a] (seq (set/difference (set e) (set a))))]
+ (assoc (format-failure e a str-e str-a)
+ :message (cond
+ (and (= (set e) (set a))
+ (= (count e) (count a))
+ (= (count e) (count (set a)))) "lists appear to contain the same items with different ordering"
+ (and (= (set e) (set a))
+ (< (count e) (count a))) "some duplicate items in actual are not expected"
+ (and (= (set e) (set a))
+ (> (count e) (count a))) "some duplicate items in expected are not actual"
+ (< (count e) (count a)) "actual is larger than expected"
+ (> (count e) (count a)) "expected is larger than actual"))))
;;; ---------------------------------------------- check-for-slow-tests ----------------------------------------------
diff --git a/test/metabase/api/session_test.clj b/test/metabase/api/session_test.clj
index e46dbce332db07230d68b2032d6cf932cf7d4260..acf23547c4d115f22d91b63c723659d870f32249 100644
--- a/test/metabase/api/session_test.clj
+++ b/test/metabase/api/session_test.clj
@@ -13,11 +13,12 @@
[metabase.test
[data :refer :all]
[util :as tu]]
- [metabase.test.data.users :refer :all]
+ [metabase.test.data.users :as test-users :refer :all]
[metabase.test.integrations.ldap :refer [expect-with-ldap-server]]
[metabase.test.util.log :as tu.log]
[toucan.db :as db]
- [toucan.util.test :as tt]))
+ [toucan.util.test :as tt])
+ (:import java.util.UUID))
;; ## POST /api/session
;; Test that we can login
@@ -61,10 +62,16 @@
;; Test that we can logout
(expect
nil
- (let [{session_id :id} ((user->client :rasta) :post 200 "session" (user->credentials :rasta))]
- (assert session_id)
- ((user->client :rasta) :delete 204 "session" :session_id session_id)
- (Session session_id)))
+ (do
+ ;; clear out cached session tokens so next time we make an API request it log in & we'll know we have a valid
+ ;; Session
+ (test-users/clear-cached-session-tokens!)
+ (let [session-id (test-users/username->token :rasta)]
+ ;; Ok, calling the logout endpoint should delete the Session in the DB. Don't worry, `test-users` will log back
+ ;; in on the next API call
+ ((user->client :rasta) :delete 204 "session")
+ ;; check whether it's still there -- should be GONE
+ (Session session-id))))
;; ## POST /api/session/forgot_password
@@ -234,18 +241,15 @@
[:first_name :last_name :email]))))
-;;; tests for google-auth-fetch-or-create-user!
-
-(defn- is-session? [session]
- (u/ignore-exceptions
- (tu/is-uuid-string? (:id session))))
+;;; --------------------------------------- google-auth-fetch-or-create-user! ----------------------------------------
;; test that an existing user can log in with Google auth even if the auto-create accounts domain is different from
;; their account should return a Session
(expect
+ UUID
(tt/with-temp User [user {:email "cam@sf-toucannery.com"}]
(tu/with-temporary-setting-values [google-auth-auto-create-accounts-domain "metabase.com"]
- (is-session? (#'session-api/google-auth-fetch-or-create-user! "Cam" "Saül" "cam@sf-toucannery.com")))))
+ (#'session-api/google-auth-fetch-or-create-user! "Cam" "Saul" "cam@sf-toucannery.com"))))
;; test that a user that doesn't exist with a *different* domain than the auto-create accounts domain gets an
;; exception
@@ -258,11 +262,14 @@
;; test that a user that doesn't exist with the *same* domain as the auto-create accounts domain means a new user gets
;; created
(expect
+ UUID
(et/with-fake-inbox
(tu/with-temporary-setting-values [google-auth-auto-create-accounts-domain "sf-toucannery.com"
admin-email "rasta@toucans.com"]
- (u/prog1 (is-session? (#'session-api/google-auth-fetch-or-create-user! "Rasta" "Toucan" "rasta@sf-toucannery.com"))
- (db/delete! User :email "rasta@sf-toucannery.com"))))) ; clean up after ourselves
+ (try
+ (#'session-api/google-auth-fetch-or-create-user! "Rasta" "Toucan" "rasta@sf-toucannery.com")
+ (finally
+ (db/delete! User :email "rasta@sf-toucannery.com")))))) ; clean up after ourselves
;;; ------------------------------------------- TESTS FOR LDAP AUTH STUFF --------------------------------------------
diff --git a/test/metabase/test/data/users.clj b/test/metabase/test/data/users.clj
index edfa9d35e6e7c34bf5a76cf6a6e8a77d51802d38..a714ccf73ba52d20d4d0f0eca21b1e9fb664822b 100644
--- a/test/metabase/test/data/users.clj
+++ b/test/metabase/test/data/users.clj
@@ -8,8 +8,10 @@
[metabase.core.initialization-status :as init-status]
[metabase.middleware.session :as mw.session]
[metabase.models.user :as user :refer [User]]
+ [schema.core :as s]
[toucan.db :as db])
- (:import clojure.lang.ExceptionInfo))
+ (:import clojure.lang.ExceptionInfo
+ metabase.models.user.UserInstance))
;;; ------------------------------------------------ User Definitions ------------------------------------------------
@@ -42,9 +44,12 @@
:password "birdseed"
:active false}})
-(def ^:private ^:const usernames
+(def ^:private usernames
(set (keys user->info)))
+(def ^:private TestUserName
+ (apply s/enum usernames))
+
;;; ------------------------------------------------- Test User Fns --------------------------------------------------
(defn- wait-for-initiailization
@@ -67,8 +72,8 @@
(defn- fetch-or-create-user!
"Create User if they don't already exist and return User."
[& {:keys [email first last password superuser active]
- :or {superuser false
- active true}}]
+ :or {superuser false
+ active true}}]
{:pre [(string? email) (string? first) (string? last) (string? password) (m/boolean? superuser) (m/boolean? active)]}
(wait-for-initiailization)
(or (User :email email)
@@ -82,19 +87,18 @@
:is_active active)))
-(defn fetch-user
+(s/defn fetch-user :- UserInstance
"Fetch the User object associated with USERNAME. Creates user if needed.
(fetch-user :rasta) -> {:id 100 :first_name \"Rasta\" ...}"
- [username]
- {:pre [(contains? usernames username)]}
+ [username :- TestUserName]
(m/mapply fetch-or-create-user! (user->info username)))
-(defn create-users-if-needed!
+(s/defn create-users-if-needed!
"Force creation of the test users if they don't already exist."
([]
(apply create-users-if-needed! usernames))
- ([& usernames]
+ ([& usernames :- [TestUserName]]
(doseq [username usernames]
;; fetch-user will force creation of users
(fetch-user username))))
@@ -104,15 +108,15 @@
(user->id :rasta) -> 4"
(memoize
- (fn [username]
+ (s/fn :- s/Int [username :- TestUserName]
{:pre [(contains? usernames username)]}
- (:id (fetch-user username)))))
+ (u/get-id (fetch-user username)))))
-(defn user->credentials
+(s/defn user->credentials :- {:username (s/pred u/email?), :password s/Str}
"Return a map with `:username` and `:password` for User with USERNAME.
(user->credentials :rasta) -> {:username \"rasta@metabase.com\", :password \"blueberries\"}"
- [username]
+ [username :- TestUserName]
{:pre [(contains? usernames username)]}
(let [{:keys [email password]} (user->info username)]
{:username email
@@ -128,9 +132,9 @@
(defonce ^:private tokens (atom {}))
-(defn username->token
+(s/defn username->token :- u/uuid-regex
"Return cached session token for a test User, logging in first if needed."
- [username]
+ [username :- TestUserName]
(or (@tokens username)
(u/prog1 (http/authenticate (user->credentials username))
(swap! tokens assoc username <>))
@@ -154,18 +158,18 @@
(clear-cached-session-tokens!)
(apply client-fn username args)))))
-(defn user->client
+(s/defn user->client :- (s/pred fn?)
"Returns a `metabase.http-client/client` partially bound with the credentials for User with USERNAME.
In addition, it forces lazy creation of the User if needed.
((user->client) :get 200 \"meta/table\")"
- [username]
+ [username :- TestUserName]
(create-users-if-needed! username)
(partial client-fn username))
-(defn do-with-test-user
+(s/defn do-with-test-user
"Call `f` with various `metabase.api.common` dynamic vars bound to the test User named by `user-kwd`."
- [user-kwd f]
+ [user-kwd :- TestUserName, f :- (s/pred fn?)]
((mw.session/bind-current-user (fn [_ respond _] (respond (f))))
(let [user-id (user->id user-kwd)]
{:metabase-user-id user-id
diff --git a/test/metabase/test/util.clj b/test/metabase/test/util.clj
index c9b7dca0f02a933704ab5d552ae3531aa9a50021..c25c43af3269491db83ad9289a5e72bc07a1d498 100644
--- a/test/metabase/test/util.clj
+++ b/test/metabase/test/util.clj
@@ -3,11 +3,11 @@
(:require [cheshire.core :as json]
[clj-time.core :as time]
[clojure
- [string :as s]
+ [string :as str]
[walk :as walk]]
[clojure.tools.logging :as log]
[clojurewerkz.quartzite.scheduler :as qs]
- [expectations :refer :all]
+ [expectations :as expectations :refer [expect]]
[metabase
[driver :as driver]
[task :as task]
@@ -35,11 +35,35 @@
[metabase.test.data :as data]
[metabase.test.data.dataset-definitions :as defs]
[metabase.util.date :as du]
+ [schema.core :as s]
[toucan.db :as db]
[toucan.util.test :as test])
(:import org.apache.log4j.Logger
[org.quartz CronTrigger JobDetail JobKey Scheduler Trigger]))
+;; record type for testing that results match a Schema
+(defrecord SchemaExpectation [schema]
+ expectations/CustomPred
+ (expect-fn [_ actual]
+ (nil? (s/check schema actual)))
+ (expected-message [_ _ _ _]
+ (str "Result did not match schema:\n"
+ (u/pprint-to-str (s/explain schema))))
+ (actual-message [_ actual _ _]
+ (str "Was:\n"
+ (u/pprint-to-str actual)))
+ (message [_ actual _ _]
+ (u/pprint-to-str (s/check schema actual))))
+
+(defmacro expect-schema
+ "Like `expect`, but checks that results match a schema."
+ {:style/indent 0}
+ [expected actual]
+ `(expect
+ (SchemaExpectation. ~expected)
+ ~actual))
+
+
;;; ---------------------------------------------------- match-$ -----------------------------------------------------
(defn- $->prop
@@ -108,8 +132,8 @@
(every-pred (some-fn keyword? string?)
(some-fn #{:id :created_at :updated_at :last_analyzed :created-at :updated-at :field-value-id :field-id
:fields_hash :date_joined :date-joined :last_login :dimension-id :human-readable-field-id}
- #(s/ends-with? % "_id")
- #(s/ends-with? % "_at")))
+ #(str/ends-with? % "_id")
+ #(str/ends-with? % "_at")))
data))
([pred data]
(walk/prewalk (fn [maybe-map]
diff --git a/test/metabase/test_setup.clj b/test/metabase/test_setup.clj
index 8d23f9cbad0f968c106e12e004ed2779dcdbe8f3..ed47f29be45afdbc1bace2c24287cb9bc747fe9e 100644
--- a/test/metabase/test_setup.clj
+++ b/test/metabase/test_setup.clj
@@ -1,12 +1,8 @@
(ns metabase.test-setup
"Functions that run before + after unit tests (setup DB, start web server, load test data)."
- (:require [clojure
- [data :as data]
- [set :as set]
- [string :as str]]
- [clojure.java.io :as io]
+ (:require [clojure.java.io :as io]
+ [clojure.string :as str]
[clojure.tools.logging :as log]
- [expectations :refer :all]
[metabase
[db :as mdb]
[handler :as handler]
@@ -20,50 +16,6 @@
[metabase.test.data.env :as tx.env]
[yaml.core :as yaml]))
-;;; ---------------------------------------- Expectations Framework Settings -----------------------------------------
-
-;; ## EXPECTATIONS FORMATTING OVERRIDES
-
-;; These overrides the methods Expectations usually uses for printing failed tests.
-;; These are basically the same as the original implementations, but they colorize and pretty-print the
-;; output, which makes it an order of magnitude easier to read, especially for tests that compare a
-;; lot of data, like Query Processor or API tests.
-(defn- format-failure [e a str-e str-a]
- {:type :fail
- :expected-message (when-let [in-e (first (data/diff e a))]
- (format "\nin expected, not actual:\n%s" (u/pprint-to-str 'green in-e)))
- :actual-message (when-let [in-a (first (data/diff a e))]
- (format "\nin actual, not expected:\n%s" (u/pprint-to-str 'red in-a)))
- :raw [str-e str-a]
- :result ["\nexpected:\n"
- (u/pprint-to-str 'green e)
- "\nwas:\n"
- (u/pprint-to-str 'red a)]})
-
-(defmethod compare-expr :expectations/maps [e a str-e str-a]
- (let [[in-e in-a] (data/diff e a)]
- (if (and (nil? in-e) (nil? in-a))
- {:type :pass}
- (format-failure e a str-e str-a))))
-
-(defmethod compare-expr :expectations/sets [e a str-e str-a]
- (format-failure e a str-e str-a))
-
-(defmethod compare-expr :expectations/sequentials [e a str-e str-a]
- (let [diff-fn (fn [e a] (seq (set/difference (set e) (set a))))]
- (assoc (format-failure e a str-e str-a)
- :message (cond
- (and (= (set e) (set a))
- (= (count e) (count a))
- (= (count e) (count (set a)))) "lists appear to contain the same items with different ordering"
- (and (= (set e) (set a))
- (< (count e) (count a))) "some duplicate items in actual are not expected"
- (and (= (set e) (set a))
- (> (count e) (count a))) "some duplicate items in expected are not actual"
- (< (count e) (count a)) "actual is larger than expected"
- (> (count e) (count a)) "expected is larger than actual"))))
-
-
;;; ------------------------------- Functions That Get Ran On Test Suite Start / Stop --------------------------------
(defn- driver-plugin-manifest [driver]
diff --git a/test/metabase/util_test.clj b/test/metabase/util_test.clj
index 5a278d0aea3d19e0e3bef6fc28646f23408ab94c..d84afec808059a58d567c1795fa9d95b65c1bdf2 100644
--- a/test/metabase/util_test.clj
+++ b/test/metabase/util_test.clj
@@ -65,7 +65,7 @@
(expect "cam_s_awesome_toucan_emporium" (slugify "Cam's awesome toucan emporium"))
(expect "frequently_used_cards" (slugify "Frequently-Used Cards"))
;; check that diactrics get removed
-(expect "cam_saul_s_toucannery" (slugify "Cam Saül's Toucannery"))
+(expect "cam_saul_s_toucannery" (slugify "Cam Saul's Toucannery"))
(expect "toucans_dislike_pinatas___" (slugify "toucans dislike piñatas :("))
;; check that non-ASCII characters get URL-encoded (so we can support non-Latin alphabet languages; see #3818)
(expect "%E5%8B%87%E5%A3%AB" (slugify "勇士")) ; go dubs