From 727d4548718a96867d5eb9ecc9daff752518b6c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arne=20Schl=C3=BCter?= Date: Wed, 1 Aug 2018 11:39:24 +0200 Subject: [PATCH] Move navigation to interceptor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Squashed commit of the following: commit c8bf5e0cb4fd95935e06dc46dda38256f5bb970f Author: Arne Schlüter Date: Wed Aug 1 11:37:43 2018 +0200 Start credential verification only if there are previous credentials commit 61e6f2e7f2fb4d01e59c71c5980b1b761fa0bf83 Author: Arne Schlüter Date: Wed Aug 1 10:22:31 2018 +0200 Make `dispatches?` helper return a boolean commit 4dc10acd5f1eae616d62c24e3cb9685e4e595f04 Author: Arne Schlüter Date: Wed Aug 1 09:19:49 2018 +0200 Add joker for linting commit 7069febff0ed49be5c60e6787bfc9dc5b758917b Author: Arne Schlüter Date: Tue Jul 31 14:17:41 2018 +0200 Implement navigation as interceptor FIXME: Unauthorized access doesn't redirect to `#/login?redirect=...` commit 60f9f03dd86f48234133e76dd57c067afb7a74d4 Author: Arne Schlüter Date: Wed Jul 18 19:35:47 2018 +0200 Make booting explicit and prepare for :navigate interceptor --- .joker | 1 + src/cljs/airsonic_ui/core.cljs | 1 - src/cljs/airsonic_ui/db.cljs | 3 +- src/cljs/airsonic_ui/events.cljs | 127 +++++++++-------- src/cljs/airsonic_ui/routes.cljs | 44 +++--- src/cljs/airsonic_ui/subs.cljs | 25 ++-- src/cljs/airsonic_ui/views.cljs | 39 ++--- src/cljs/airsonic_ui/views/login.cljs | 2 +- test/cljs/airsonic_ui/events_test.cljs | 141 +++++++++++-------- test/cljs/airsonic_ui/fixtures.cljs | 4 + test/cljs/airsonic_ui/routes_test.cljs | 9 ++ test/cljs/airsonic_ui/subs_test.cljs | 56 +++----- test/cljs/airsonic_ui/test_helpers.cljs | 2 +- test/cljs/airsonic_ui/test_helpers_test.cljs | 18 +-- 14 files changed, 254 insertions(+), 218 deletions(-) create mode 100644 .joker diff --git a/.joker b/.joker new file mode 100644 index 0000000..8d9e936 --- /dev/null +++ b/.joker @@ -0,0 +1 @@ +{:known-macros [cljs.test/deftest]} \ No newline at end of file diff --git a/src/cljs/airsonic_ui/core.cljs b/src/cljs/airsonic_ui/core.cljs index 7ce6781..d61493e 100644 --- a/src/cljs/airsonic_ui/core.cljs +++ b/src/cljs/airsonic_ui/core.cljs @@ -6,7 +6,6 @@ [akiroz.re-frame.storage :as storage] ;; our app [airsonic-ui.audio] ; <- just registers effects here - [airsonic-ui.routes :as routes] [airsonic-ui.events :as events] [airsonic-ui.views :as views] [airsonic-ui.config :as config])) diff --git a/src/cljs/airsonic_ui/db.cljs b/src/cljs/airsonic_ui/db.cljs index 26d54e0..02f3ee4 100644 --- a/src/cljs/airsonic_ui/db.cljs +++ b/src/cljs/airsonic_ui/db.cljs @@ -1,4 +1,5 @@ (ns airsonic-ui.db) (def default-db - {:notifications (sorted-map)}) + {:is-booting? true + :notifications (sorted-map)}) diff --git a/src/cljs/airsonic_ui/events.cljs b/src/cljs/airsonic_ui/events.cljs index 2861009..1bd0529 100644 --- a/src/cljs/airsonic_ui/events.cljs +++ b/src/cljs/airsonic_ui/events.cljs @@ -24,85 +24,83 @@ ;; * sending out the appropriate requests ;; --- +(defn initialize-app + [{{:keys [credentials]} :store} _] + (let [effects {:db db/default-db + :routes/start-routing nil}] + (if (not (empty? credentials)) + (assoc effects :dispatch [:credentials/verify credentials]) + effects))) + (re-frame/reg-event-fx ::initialize-app - (fn [_] - {:db db/default-db - :dispatch [:init-flow/restore-previous-session]})) - -(defn restore-previous-session - "See comment above for different steps; what's important here is that we check - for a previous session before anything else, otherwise we might run into auth - troubles with our router." - [{:keys [db store]} _] - (let [credentials (:credentials store)] - {:dispatch-n [(if credentials - [:init-flow/credentials-found credentials] - [:init-flow/credentials-not-found])] - :routes/start-routing nil})) - -(re-frame/reg-event-fx - :init-flow/restore-previous-session [(re-frame/inject-cofx :store)] - restore-previous-session) + initialize-app) -(defn credentials-found [_ [_ {:keys [u p server]}]] - {:dispatch [:credentials/verification-request u p server]}) +(defn verify-credentials + "Initializes the whole authentication chain when we have locally stored + credentials that look plausible." + [_ [_ credentials]] + ;; TODO: spec this + (if (every? string? ((juxt :u :p :server) credentials)) + {:dispatch [:credentials/send-authentication-request credentials]})) -(re-frame/reg-event-fx :init-flow/credentials-found credentials-found) - -;; we don't do anything special here, it's just for the sake of clarity - -(defn credentials-not-found - [cofx _] - (assoc-in cofx [:db :credentials] :credentials/not-found)) - -(re-frame/reg-event-fx :init-flow/credentials-not-found credentials-not-found) +(re-frame/reg-event-fx :credentials/verify verify-credentials) ;; --- ;; auth logic ;; --- -(defn credentials-verification-request +(defn user-login + "Gets called after the user clicked on the login button" + [cofx [_ user pass server]] + (let [credentials {:u user, :p pass, :server server, :verified? false}] + (-> (assoc-in cofx [:db :credentials] credentials) + (assoc :dispatch [:credentials/send-authentication-request credentials])))) + +(re-frame/reg-event-fx :credentials/user-login user-login) + +(defn authentication-request "Tries to authenticate a user by pinging the server with credentials, saving them when the request was successful. Bypasses the request when a user saved their credentials." - [_ [_ user pass server]] - {:http-xhrio {:method :get - :uri (api/url server "ping" {:u user :p pass}) - :response-format (ajax/json-response-format {:keywords? true}) - :on-success [:credentials/verification-response user pass server] - :on-failure [:credentials/verification-failure]}}) + [cofx [_ credentials]] + (assoc cofx :http-xhrio {:method :get + :uri (api/url (:server credentials) "ping" (select-keys credentials [:u :p])) + :response-format (ajax/json-response-format {:keywords? true}) + :on-success [:credentials/authentication-response credentials] + :on-failure [:api/bad-response]})) -(re-frame/reg-event-fx :credentials/verification-request credentials-verification-request) +(re-frame/reg-event-fx :credentials/send-authentication-request authentication-request) -(defn credentials-verification-response +(defn authentication-response "Since we don't get real status codes, we have to look into the server's response and see whether we actually sent the correct credentials" - [fx [_ user pass server response]] - {:dispatch (if (api/is-error? response) - [:credentials/verification-failure response] - [:credentials/verified user pass server])}) + [fx [_ credentials response]] + (assoc fx :dispatch (if (api/is-error? response) + [:credentials/authentication-failure response] + [:credentials/authentication-success (assoc credentials :verified? true)]))) -(re-frame/reg-event-fx :credentials/verification-response credentials-verification-response) +(re-frame/reg-event-fx :credentials/authentication-response authentication-response) -(defn credentials-verification-failure [fx [_ response]] - (-> (assoc-in fx [:db :credentials] :credentials/verification-failure) - (assoc :dispatch [:notification/show :error (api/error-msg (api/->exception response))]))) +(defn authentication-failure + "Removes all stored credentials and displays potential api errors to the user" + [fx [_ response]] + (-> (assoc fx :dispatch [:notification/show :error (api/error-msg (api/->exception response))]) + (update :store dissoc :credentials) + (update :db dissoc :credentials))) -(re-frame/reg-event-fx :credentials/verification-failure credentials-verification-failure) +(re-frame/reg-event-fx :credentials/authentication-failure authentication-failure) -(defn credentials-verified +(defn authentication-success "Gets called after the server indicates that the credentials entered by a user are correct (see `credentials-verification-request`)" - [{:keys [db]} [_ user pass server]] - (let [credentials {:u user :p pass :server server}] - {:routes/set-credentials credentials - :store {:credentials credentials} - :db (assoc db :credentials credentials) - :dispatch [::logged-in]})) + [{:keys [db]} [_ credentials]] + {:store {:credentials credentials} + :db (assoc db :credentials (assoc credentials :verified? true)) + :dispatch [::logged-in]}) -(re-frame/reg-event-fx :credentials/verified credentials-verified) +(re-frame/reg-event-fx :credentials/authentication-success authentication-success) ;; TODO: We have to find another solution for this once we have routes that ;; don't require a login but have the bottom controls @@ -112,11 +110,11 @@ (fn [_] (.. js/document -documentElement -classList (add "has-navbar-fixed-bottom")))) - (defn logged-in [cofx _] - (let [redirect (or (get-in cofx [:routes/from-query-param :redirect]) [::routes/main])] - {:routes/navigate redirect + (let [redirect (or (get-in cofx [:routes/from-query-param :redirect]) + [::routes/main])] + {:dispatch [:routes/do-navigation redirect] :show-nav-bar nil})) (re-frame/reg-event-fx @@ -128,12 +126,12 @@ "Clears all credentials and redirects the user to the login page" [cofx [_ & args]] (let [args (apply hash-map args)] - {:routes/navigate (if-let [redirect (:redirect-to args)] - [::routes/login {} {:redirect (routes/encode-route redirect)}] - [::routes/login]) - :routes/unset-credentials nil + {:dispatch [:routes/do-navigation (if-let [redirect (:redirect-to args)] + [::routes/login {} {:redirect (routes/encode-route redirect)}] + [::routes/login])] :store nil - :db (merge (:db cofx) db/default-db {:credentials :credentials/logged-out})})) + :db (-> (merge (:db cofx) db/default-db) + (dissoc :credentials))})) (re-frame/reg-event-fx ::logout logout) @@ -223,8 +221,9 @@ ;; --- (re-frame/reg-event-fx - :routes/navigation + :routes/did-navigate (fn [{:keys [db]} [_ route params query]] + ;; FIXME: This leads to an ugly "unregistered event handler `nil`" error ;; all the naviagation logic is in routes.cljs; all we need to do here ;; is say what actually happens once we've navigated succesfully {:db (assoc db :current-route [route params query]) diff --git a/src/cljs/airsonic_ui/routes.cljs b/src/cljs/airsonic_ui/routes.cljs index f169bce..17c0506 100644 --- a/src/cljs/airsonic_ui/routes.cljs +++ b/src/cljs/airsonic_ui/routes.cljs @@ -7,7 +7,7 @@ (def router (r/router [["/" ::login] - ["/hello" ::main] + ["/main" ::main] ["/artist/:id" ::artist-view] ["/album/:id" ::album-view]])) @@ -46,31 +46,39 @@ ;; holding credentials, which is necessary to restrict certain routes, and the ;; last one is used for actual navigation -(def credentials (atom nil)) +;; the event to initialize navigation is implemented so the coeffect map is +;; returned unaltered, we just need access to the current app database for +;; authentication, which we get with an interceptor -(re-frame/reg-fx - :routes/set-credentials - (fn [credentials'] - (reset! credentials credentials'))) +(def ^:private credentials (atom nil)) -(re-frame/reg-fx - :routes/unset-credentials - (fn [] - (reset! credentials nil))) +(def do-navigation + "An interceptor which performs the navigation after looking up current + credentials in the app database" + (re-frame.core/->interceptor + :id :routes/do-navigation + :after (fn do-navigation [context] + (let [[_ & [route]] (get-in context [:coeffects :event]) + ;; because :routes/do-navigation is both an event handler and + ;; an interceptor, we know that when handling the event (see + ;; below) the credentials aren't altered anymore + credentials'(get-in context [:coeffects :db :credentials])] + (println "calling do-navigation with" route credentials') + (reset! credentials credentials') + (apply r/navigate! router route) + context)))) -(re-frame/reg-fx - :routes/navigate - (fn [[route-id params query]] - (println "calling ::navigate with" route-id params query) - (r/navigate! router route-id params query))) +(re-frame/reg-event-fx :routes/do-navigation do-navigation (fn [& _] nil)) (defn can-access? [route] - (or (not (protected-routes route)) @credentials)) + (or (not (protected-routes route)) + (:verified? @credentials))) (defn on-navigate [route-id params query] + (println "on-navigate is called" route-id params query credentials) (if (can-access? route-id) - (re-frame/dispatch [:routes/navigation route-id params query]) + (re-frame/dispatch [:routes/did-navigate route-id params query]) (re-frame/dispatch [:routes/unauthorized route-id params query]))) (defn encode-route @@ -89,11 +97,13 @@ [] (r/match router (subs (.. js/window -location -hash) 1))) +;; add the current route to our coeffect map (re-frame/reg-cofx :routes/current-route (fn [coeffects _] (assoc coeffects :routes/current-route (current-route)))) +;; add route into from a URL parameter to our coeffect map (re-frame/reg-cofx :routes/from-query-param (fn [coeffects param] diff --git a/src/cljs/airsonic_ui/subs.cljs b/src/cljs/airsonic_ui/subs.cljs index 61ab0c6..a4d2a22 100644 --- a/src/cljs/airsonic_ui/subs.cljs +++ b/src/cljs/airsonic_ui/subs.cljs @@ -3,19 +3,18 @@ [airsonic-ui.utils.api :as api])) (defn is-booting? - "Predicate to tell whether our app is still in the process of initialization" - [{:keys [credentials]} _] - (and (not (map? credentials)) - (not (#{:credentials/not-found :credentials/verification-failure :credentials/logged-out} credentials)))) + "The boot process starts with setting up routing and continues if we found + previous credentials and ends when we receive a response from the server." + [db _] + ;; so either we don't have any credentials or they are not verified + (or (empty? (:current-route db)) + (and (not (empty? (:credentials db))) + (not (get-in db [:credentials :verified?]))))) (re-frame/reg-sub ::is-booting? is-booting?) -;; can be used to query the user's credentials - -(re-frame/reg-sub - ::credentials - (fn [db _] - (:credentials db))) +(defn credentials [db _] (:credentials db)) +(re-frame/reg-sub ::credentials credentials) (re-frame/reg-sub ::user @@ -65,7 +64,5 @@ ;; user notifications -(re-frame/reg-sub - ::notifications - (fn [db _] - (:notifications db))) +(defn notifications [db _] (:notifications db)) +(re-frame/reg-sub ::notifications notifications) diff --git a/src/cljs/airsonic_ui/views.cljs b/src/cljs/airsonic_ui/views.cljs index 184741d..3d7e172 100644 --- a/src/cljs/airsonic_ui/views.cljs +++ b/src/cljs/airsonic_ui/views.cljs @@ -1,12 +1,10 @@ (ns airsonic-ui.views (:require [re-frame.core :refer [dispatch subscribe]] - [airsonic-ui.config :as config] [airsonic-ui.routes :as routes :refer [url-for]] [airsonic-ui.events :as events] [airsonic-ui.subs :as subs] [airsonic-ui.views.notifications :refer [notification-list]] - [airsonic-ui.views.loading-spinner :refer [loading-spinner]] [airsonic-ui.views.breadcrumbs :refer [breadcrumbs]] [airsonic-ui.views.bottom-bar :refer [bottom-bar]] [airsonic-ui.views.login :refer [login-form]] @@ -49,30 +47,33 @@ ;; putting everything together -(defn app [route params query] +(defn app [route-id params query] (let [user @(subscribe [::subs/user]) content @(subscribe [::subs/current-content])] - (if (= route ::routes/login) - [login-form] - [:div - [:main.columns - [:div.column.is-2.sidebar - [sidebar user]] - [:div.column - [:section.section - [breadcrumbs content] - (case route - ::routes/main [most-recent content] - ::routes/artist-view [artist-detail content] - ::routes/album-view [album-detail content])]]] - [bottom-bar]]))) + [:div + [:main.columns + [:div.column.is-2.sidebar + [sidebar user]] + [:div.column + [:section.section + [breadcrumbs content] + (case route-id + ::routes/main [most-recent content] + ::routes/artist-view [artist-detail content] + ::routes/album-view [album-detail content])]]] + [bottom-bar]])) (defn main-panel [] (let [notifications @(subscribe [::subs/notifications]) is-booting? @(subscribe [::subs/is-booting?]) - [route params query] @(subscribe [::subs/current-route])] + [route-id params query] @(subscribe [::subs/current-route])] + (println "route-id" route-id (case route-id + ::routes/login "::routes/login" + "something else")) [:div [notification-list notifications] (if is-booting? [:div.app-loading>div.loader] - [app route params query] )])) + (case route-id + ::routes/login [login-form] + [app route-id params query]))])) diff --git a/src/cljs/airsonic_ui/views/login.cljs b/src/cljs/airsonic_ui/views/login.cljs index ebb1770..728bfe1 100644 --- a/src/cljs/airsonic_ui/views/login.cljs +++ b/src/cljs/airsonic_ui/views/login.cljs @@ -16,7 +16,7 @@ server (r/atom (.. js/window -location -origin)) submit (fn [e] (.preventDefault e) - (dispatch [:credentials/verification-request @user @pass @server]))] + (dispatch [:credentials/user-login @user @pass @server]))] (fn [] [:section.hero.is-fullheight>div.hero-body [:div.container.has-text-centered>div.column.is-4.is-offset-4 diff --git a/test/cljs/airsonic_ui/events_test.cljs b/test/cljs/airsonic_ui/events_test.cljs index a9d654f..2cfbbd0 100644 --- a/test/cljs/airsonic_ui/events_test.cljs +++ b/test/cljs/airsonic_ui/events_test.cljs @@ -2,83 +2,112 @@ (:require [cljs.test :refer [deftest testing is]] [clojure.string :as str] [airsonic-ui.test-helpers :refer [dispatches?]] - [airsonic-ui.fixtures :refer [responses]] + [airsonic-ui.fixtures :as fixtures] [airsonic-ui.db :as db] [airsonic-ui.routes :as routes] - [airsonic-ui.events :as events])) + [airsonic-ui.events :as events] + [airsonic-ui.subs :as subs])) (enable-console-print!) -(deftest session-restoration - (letfn [(no-previous-session [] - (events/restore-previous-session {} [:_])) - (has-previous-session [] - (events/restore-previous-session {:store {:credentials {:u "test" - :p "test" - :server "https://demo.airsonic.io/"}}} [:_]))] - (testing "Should initialize routing after checking for previous credentials" - (is (contains? (no-previous-session) :routes/start-routing)) - (is (contains? (has-previous-session) :routes/start-routing))) - (testing "Should indicate success or failure" - (is (true? (dispatches? (no-previous-session) :init-flow/credentials-not-found))) - (is (true? (dispatches? (has-previous-session) :init-flow/credentials-found)))) - (testing "Should send an auth request on success" - (is (true? (dispatches? (events/credentials-found {} [:_]) :credentials/verification-request)))))) +;; the event tests are actually quite nice to write: +;; because everything in re-frame is described as data, we pass on coeffects +;; to event handler after event handler and check if the final coeffect map +;; looks as expected. -(deftest authentication - (testing "Server ping for verifications" - (let [server "https://localhost" - fx (events/credentials-verification-request {} [:_ "user" "pass" server]) - request (:http-xhrio fx)] - (testing "uses correct server url" - (let [uri (:uri request)] - (is (true? (str/starts-with? uri server))) - (is (true? (str/includes? uri "/ping"))) - (is (true? (str/includes? uri "p=pass"))) - (is (true? (str/includes? uri "u=user"))))) - (testing "invokes correct success callback" - (is (= :credentials/verification-response (first (:on-success request))))))) - (testing "Auth response" - (testing "verification for bad responses" - (let [ev [:_ "user" "pass" "https://localhost"] - invalid-credentials (events/credentials-verification-response {} (conj ev (:auth-failure responses))) - verification-failure (events/credentials-verification-failure {} [:_ (:auth-failure responses)])] - (is (true? (dispatches? invalid-credentials :credentials/verification-failure)) "fails for bad responses") - (is (true? (dispatches? verification-failure :notification/show)) "shows the failure the the user"))) - (let [server "https://localhost" - fx (events/credentials-verification-response {} [:_ "username" "password" server (:auth-success responses)])] - (is (true? (dispatches? fx [:credentials/verified "username" "password" server]))))) - (testing "On succesful response" - (let [credentials {:u "user" :p "pass" :server "https://localhost"} - fx (events/credentials-verified {} [:_ (:u credentials) (:p credentials) (:server credentials)])] - (testing "credentials are sent to the router for access rights" - (is (= credentials (:routes/set-credentials fx)))) - (testing "credentials are saved in the global state" - (is (= credentials (get-in fx [:db :credentials])))) - (testing "the login process is finalized" - (is (true? (dispatches? fx ::events/logged-in))))))) +(defn no-previous-session [] (events/initialize-app {} [::events/initialize-app])) +(defn has-previous-session [] (-> {:store {:credentials fixtures/credentials}} + (events/initialize-app [::events/initialize-app]))) + +(deftest app-initialization + (testing "Should set up notifications" + (is (map? (subs/notifications (:db (no-previous-session)) + [::subs/notifications]))) + (is (map? (subs/notifications (:db (has-previous-session)) + [::subs/notifications])))) + (testing "Should set up the default database") + (testing "Should initialize credential verification" + (is (false? (dispatches? (no-previous-session) :credentials/verify))) + (is (true? (dispatches? (has-previous-session) [:credentials/verify fixtures/credentials])))) + (testing "Should initialize the router" + (is (contains? (no-previous-session) :routes/start-routing)) + (is (contains? (has-previous-session) :routes/start-routing)))) + +(deftest credential-verification + (testing "Should fail when there are no credentials" + (is (false? (dispatches? (-> (no-previous-session) + (events/verify-credentials [:credentials/verify nil])) [::subs/is-booting?])))) + (testing "Should happen server-side when we have credentials" + (let [cofx (-> (has-previous-session) + (events/verify-credentials [:credentials/verify fixtures/credentials]))] + (is (true? (dispatches? cofx :credentials/send-authentication-request))))) + (testing "Should verify the structure of credentials" + (let [empty-creds {:store {:credentials {}}}] + (is (false? (boolean (dispatches? empty-creds :credentials/send-authentication-request))))) + (let [malformed {:store {:credentials {:xyz #{12 34 56}}}}] + (is (false? (boolean (dispatches? malformed :credentials/send-authentication-request))))))) + +(deftest authentication-request + (let [event [:credentials/send-authentication-request fixtures/credentials] + fx (events/authentication-request {} event) + request (:http-xhrio fx)] + (testing "uses correct server url" + (let [uri (:uri request)] + (is (true? (str/starts-with? uri (:server fixtures/credentials)))) + (is (true? (str/includes? uri "/ping"))) + (is (true? (str/includes? uri (str "p=" (:p fixtures/credentials))))) + (is (true? (str/includes? uri (str "u=" (:u fixtures/credentials))))))) + (testing "invokes correct callback on server response" + (is (= [:credentials/authentication-response fixtures/credentials] (:on-success request)))) + (testing "invokes correct callback when server is not reachable" + (is (= [:api/bad-response] (:on-failure request)))))) + +(deftest authentication-response + (testing "On success" + (let [cofx (-> (has-previous-session) + (events/authentication-response [:credentials/authentication-response (:auth-success fixtures/responses)]) + (events/authentication-success [:credentials/authentication-success]))] + (testing "should mark the credentials as verified" + (is (true? (get-in cofx [:db :credentials :verified?])))))) + (testing "On failure" + (let [cofx (-> (has-previous-session) + (events/authentication-response [:credentials/authentication-response (:auth-failure fixtures/responses)]) + (events/authentication-failure [:credentials/authentication-failure (:auth-failure fixtures/responses)]))] + (testing "should display a notification to the user" + (is (true? (dispatches? cofx :notification/show))))))) + +(deftest manual-login + (let [{:keys [u p server]} fixtures/credentials + credentials (assoc fixtures/credentials :verified? false) + effect (events/user-login {} [:credentials/user-login u p server])] + (testing "Should save the credentials as unverified" + + (is (= credentials (get-in effect [:db :credentials])))) + (testing "Should start the authentication request" + (is (true? (dispatches? effect [:credentials/send-authentication-request credentials])))))) (deftest logout (let [fx (events/logout {} [:_])] (testing "Should clear all stored data" (is (nil? (:store fx)))) (testing "Should redirect to the login screen" - (is (= [::routes/login] (:routes/navigate fx)))) - (testing "Should unset authentication in the router" - (is (contains? fx :routes/unset-credentials))) + (is (dispatches? fx [:routes/do-navigation [::routes/login]]))) (testing "Should reset the app-db" - (is (= (every? #(= (get db/default-db %) (get-in fx [:db %])) (keys db/default-db)))))) + (is (= db/default-db (:db fx))))) (testing "Should be able to keep a redirection parameter" (let [redirect [:route {:with-data #{1 2 3 4 5}}] - fx (events/logout {} [:_ :redirect-to redirect])] - (is (= [::routes/login {:redirect redirect}]))))) + navigation-event (:dispatch (events/logout {} [:_ :redirect-to redirect]))] + (is (= :routes/do-navigation (first navigation-event))) + (let [[route-id _ query] (second navigation-event)] + (is (= ::routes/login route-id)) + (is (contains? query :redirect)))))) (defn- first-notification [fx] (-> (get-in fx [:db :notifications]) vals first)) (deftest api-interaction (testing "Should show an error notification when airsonic responds with an error" - (let [fx (events/good-api-response {} [:_ (:error responses)])] + (let [fx (events/good-api-response {} [:_ (:error fixtures/responses)])] (is (= :error (-> fx :dispatch second)))))) (deftest user-notifications diff --git a/test/cljs/airsonic_ui/fixtures.cljs b/test/cljs/airsonic_ui/fixtures.cljs index 56aadef..30061ca 100644 --- a/test/cljs/airsonic_ui/fixtures.cljs +++ b/test/cljs/airsonic_ui/fixtures.cljs @@ -1,5 +1,9 @@ (ns airsonic-ui.fixtures) +(def credentials {:u "username" + :p "cleartext-password" + :server "https://demo.airsonic.io"}) + (def responses {:error {:subsonic-response {:error {:code 50 :message "Incompatible Airsonic REST protocol version. Server must upgrade."} diff --git a/test/cljs/airsonic_ui/routes_test.cljs b/test/cljs/airsonic_ui/routes_test.cljs index cfc6e51..055862f 100644 --- a/test/cljs/airsonic_ui/routes_test.cljs +++ b/test/cljs/airsonic_ui/routes_test.cljs @@ -5,6 +5,15 @@ (def fixtures {:default [::route {:some :data} {:some-more true}]}) +#_(deftest permission-checking + (testing "Should succeed for unprotected routes" + (testing "without credentials") + (testing "with unverified credentials")) + (testing "Should fail for protected routes" + (testing "without credentials") + (testing "with unverified credentials")) + (testing "Should succeed for protected routes with verified credentials")) + (deftest route-encoding (testing "Should return a string with hash-compatible characters" (let [encoded (routes/encode-route (:default fixtures))] diff --git a/test/cljs/airsonic_ui/subs_test.cljs b/test/cljs/airsonic_ui/subs_test.cljs index fc145fc..684027d 100644 --- a/test/cljs/airsonic_ui/subs_test.cljs +++ b/test/cljs/airsonic_ui/subs_test.cljs @@ -1,41 +1,27 @@ (ns airsonic-ui.subs-test (:require [cljs.test :refer [deftest testing is]] - [airsonic-ui.db :as db] - [airsonic-ui.fixtures :refer [song] :as fixtures] + [airsonic-ui.fixtures :as fixtures] [airsonic-ui.utils.api :as api] - [airsonic-ui.events :as ev] [airsonic-ui.subs :as subs])) -(def creds {:credentials {:u "test" - :p "test" - :server "https://demo.airsonic.io/"}}) - -(deftest is-booting - (testing "Should be true when provided the initial state" - (is (true? (subs/is-booting? db/default-db [:_])))) - (testing "Should be true when we have credentials but no response yet" - (is (true? (-> (ev/restore-previous-session {:store creds} [:_]) - (ev/credentials-found [:_]) - :db - (subs/is-booting? [:_]))))) - (testing "Should be false when the login screen is shown" - (is (false? (-> (ev/restore-previous-session {} [:_]) - (ev/credentials-not-found [:_]) - :db - (subs/is-booting? [:_]))))) - (let [{:keys [u p server]} (:credentials creds)] - (testing "Should be false after we verified our credentials with the server" - (is (false? (-> (ev/credentials-verified {:db {}} [:_ u p server]) - :db - (subs/is-booting? [:_]))))) - (testing "Should be false after the server rejected our credentials" - (is (false? (-> (ev/credentials-verification-failure {} [:_ (:auth-failure fixtures/responses)]) - :db - (subs/is-booting? [:_])))))) - (testing "Should be false when a user logged out voluntarily" - (is (false? (-> (ev/logout {} [:_]) - :db - (subs/is-booting? [:_])))))) +(deftest booting + (let [route [:some-route nil nil] + verified-credentials (assoc fixtures/credentials :verified? true) + is-booting? (fn is-booting? [db] + (subs/is-booting? db [:subs/is-booting?]))] + (testing "Should be false when we don't have previous credentials" + (is (not (is-booting? {:current-route route}))) + (is (not (is-booting? {:current-route route + :credentials {}}))) ) + (testing "Should be true when we have unverified credentials" + (is (true? (is-booting? {:current-route route + :credentials fixtures/credentials})))) + (testing "Should be false when we have verified credentials" + (is (not (is-booting? {:current-route route + :credentials verified-credentials})))) + (testing "Should be true when routing is not yet set up" + (is (true? (is-booting? {:current-route nil + :credentials verified-credentials})))))) (deftest cover-images (let [credentials {:server "https://foo.bar" @@ -44,6 +30,6 @@ (testing "Should give the correct path once the credentials are set" (is (= (api/cover-url (:server credentials) (select-keys credentials [:u :p]) - song + fixtures/song 48) - (subs/cover-url [credentials] [:_ song 48])))))) + (subs/cover-url [credentials] [:subs/cover-image fixtures/song 48])))))) diff --git a/test/cljs/airsonic_ui/test_helpers.cljs b/test/cljs/airsonic_ui/test_helpers.cljs index 18f8718..05a58a9 100644 --- a/test/cljs/airsonic_ui/test_helpers.cljs +++ b/test/cljs/airsonic_ui/test_helpers.cljs @@ -5,4 +5,4 @@ be a whole vector or a keyword which is interpreted as the event name." [cofx ev] (let [all-events (conj (get cofx :dispatch-n []) (:dispatch cofx))] - (some #(= ev (if (vector? ev) % (first %))) all-events))) + (boolean (some #(= ev (if (vector? ev) % (first %))) all-events)))) diff --git a/test/cljs/airsonic_ui/test_helpers_test.cljs b/test/cljs/airsonic_ui/test_helpers_test.cljs index e4abf52..3b3ad76 100644 --- a/test/cljs/airsonic_ui/test_helpers_test.cljs +++ b/test/cljs/airsonic_ui/test_helpers_test.cljs @@ -4,14 +4,14 @@ (deftest dispatch-helper (testing "single dispatch" - (is (not (dispatches? {} :foo))) - (is (dispatches? {:dispatch [:foo 1 2 3]} :foo)) - (is (not (dispatches? {:dispatch [:foo 1 2 3]} :bar))) - (is (dispatches? {:dispatch [:foo 1 2 3]} [:foo 1 2 3])) - (is (not (dispatches? {:dispatch [:foo 1 2 3]} [:bar 2 3])))) + (is (false? (dispatches? {} :foo))) + (is (true? (dispatches? {:dispatch [:foo 1 2 3]} :foo))) + (is (false? (dispatches? {:dispatch [:foo 1 2 3]} :bar))) + (is (true? (dispatches? {:dispatch [:foo 1 2 3]} [:foo 1 2 3]))) + (is (false? (dispatches? {:dispatch [:foo 1 2 3]} [:bar 2 3])))) (testing "multiple dispatch" - (is (not (dispatches? {:dispatch-n [[:bar]]} :foo))) - (is (dispatches? {:dispatch-n [[:foo 1 2 3]]} :foo)) - (is (not (dispatches? {:dispatch-n [[:foo 1 2 3]]} :bar))) + (is (false? (dispatches? {:dispatch-n [[:bar]]} :foo))) + (is (true? (dispatches? {:dispatch-n [[:foo 1 2 3]]} :foo))) + (is (false? (dispatches? {:dispatch-n [[:foo 1 2 3]]} :bar))) (is (dispatches? {:dispatch-n [[:foo 1 2 3]]} [:foo 1 2 3])) - (is (not (dispatches? {:dispatch-n [[:foo 1 2 3]]} [:bar 2 3]))))) + (is (false? (dispatches? {:dispatch-n [[:foo 1 2 3]]} [:bar 2 3])))))