From ca8972f8c30876ed71148259145b4b1254631790 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arne=20Schl=C3=BCter?= Date: Wed, 30 May 2018 13:34:38 +0200 Subject: [PATCH 1/4] Make sure to always run tests in development --- README.md | 18 ++++++++++-------- package.json | 14 ++++++-------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index fc2d4a6..07ae2ea 100644 --- a/README.md +++ b/README.md @@ -21,10 +21,10 @@ To build the project make sure you have Node.js (v6.0.0), npm and Java 8 install ``` # after cloning the project, first install all dependencies $ npm install -# start a continuous build with hot-code-reloading; first build takes a while. open http://localhost:8080 + +# start a continuous build with hot-code-reloading and continuous testing +# first build takes a while. open http://localhost:8080 $ npm run dev -# build and optimize the code once for production -$ npm run build ``` **Note:** In dev mode this project comes with re-frame-10x. You can hit `Ctrl + h` to display the overlay and have a time traveling debugger. @@ -36,17 +36,19 @@ This project uses [karma](https://karma-runner.github.io/) for tests. Make sure ``` # run tests once $ npm test -# run tests continuously, watching for changes -$ npm run test:watch ``` ## Build artifacts -Everything you need to serve the app can be found inside the `public` folder. -## Deploy to github +## Deployment ``` -# will build everything and publish everything in /public via gh-pages +# build and optimize the code once for production +$ npm run build + +# runs npm run build and publishes everything via gh-pages $ npm run deploy ``` + +All build artifacts will be output in `/public`. Don't change anything in there as changes will be overwritten. diff --git a/package.json b/package.json index f869bfa..c9dca08 100644 --- a/package.json +++ b/package.json @@ -4,21 +4,19 @@ "description": "Airsonic UI written with re-frame", "main": "index.js", "scripts": { - "test": "run-s test:compile-once test:run-once", - "test:compile-once": "shadow-cljs compile test", - "test:run-once": "karma start --single-run", - "test:compile-watch": "shadow-cljs watch test", - "test:run-watch": "karma start --reporters growl,progress --auto-watch", - "test:watch": "npm-run-all test:compile-once -p test:compile-watch test:run-watch", "build:cljs": "shadow-cljs release app", "build:html": "sed 's/\"\\/app\\//\".\\/app\\//g' src/html/index.html > public/index.html", "build:sass": "node-sass --output-style compressed src/sass/app.sass public/app/style.css", "build": "rm -r public/*; run-p build:*; ", "deploy": "npm run build && gh-pages -d public", - "dev:cljs": "shadow-cljs watch app", + "dev:cljs": "shadow-cljs watch app test", "dev:html": "sed 's/\"\\.\\/app\\//\"\\/app\\//g' src/html/index.html > public/index.html", "dev:sass": "npm run build:sass; node-sass -w src/sass/app.sass public/app/style.css", - "dev": "run-p dev:*" + "dev:test": "karma start --reporters growl,progress --auto-watch", + "dev": "npm-run-all test:compile -p dev:*", + "test": "run-s test:compile test:run", + "test:compile": "shadow-cljs compile test", + "test:run": "karma start --single-run" }, "author": "Arne Schlüter", "license": "ISC", From ed060e55b662d883e780f3f3fe5ffe303fd970a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arne=20Schl=C3=BCter?= Date: Wed, 30 May 2018 14:45:11 +0200 Subject: [PATCH 2/4] Add tests for auth process --- README.md | 2 ++ src/cljs/airsonic_ui/db.cljs | 3 +- src/cljs/airsonic_ui/events.cljs | 42 ++++++++++++++------------ src/cljs/airsonic_ui/subs.cljs | 2 +- test/cljs/airsonic_ui/events_test.cljs | 28 +++++++++++++++++ 5 files changed, 54 insertions(+), 23 deletions(-) create mode 100644 test/cljs/airsonic_ui/events_test.cljs diff --git a/README.md b/README.md index 07ae2ea..c610beb 100644 --- a/README.md +++ b/README.md @@ -38,6 +38,8 @@ This project uses [karma](https://karma-runner.github.io/) for tests. Make sure $ npm test ``` +**Note:** If you want nice console output in your tests, make sure to `(enable-console-print!)`. You can call `println` afterwards like you're used to. + ## Build artifacts diff --git a/src/cljs/airsonic_ui/db.cljs b/src/cljs/airsonic_ui/db.cljs index 16f6fd5..7f3b701 100644 --- a/src/cljs/airsonic_ui/db.cljs +++ b/src/cljs/airsonic_ui/db.cljs @@ -2,6 +2,5 @@ (:require [airsonic-ui.routes :as routes])) (def default-db - {:active-requests 0 - ;; because navigate! executes asynchronously we force to display the login screen first + {;; because navigate! executes asynchronously we force to display the login screen first :current-route [routes/default-route]}) diff --git a/src/cljs/airsonic_ui/events.cljs b/src/cljs/airsonic_ui/events.cljs index 0c46fef..4e91a91 100644 --- a/src/cljs/airsonic_ui/events.cljs +++ b/src/cljs/airsonic_ui/events.cljs @@ -18,31 +18,33 @@ (fn [_] db/default-db)) -;; this is called with user and password to try and see if the credentials are -;; correct; if yes, ::auth-success will be fired +;; auth logic + +(defn authenticate + "Tries to authenticate a user by pinging the server with credentials, saving + them when the request was succesful." + [{:keys [db]} [_ user pass server]] + {:db (assoc db :server 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-verified user pass] + :on-failure [::api-failure]}}) (re-frame/reg-event-fx - ::authenticate - (fn [{:keys [db]} [_ user pass server]] - {:db (-> (update db :active-requests inc) - (assoc :server server)) - :http-xhrio {:method :get - :uri (api/url server "ping" {:u user :p pass}) - :response-format (ajax/json-response-format {:keywords? true}) - :on-success [::auth-success user pass] - :on-failure [::api-failure]}})) + ::authenticate authenticate) -;; TODO: Test that credentials are associated +(defn credentials-verified + "Gets called after the server indicates that the credentials entered by a user + are correct (see `authenticate`)." + [{:keys [db]} [_ user pass response]] + (let [login {:u user :p pass}] + {:routes/set-credentials login + :db (assoc db :login login) + :dispatch [::logged-in]})) (re-frame/reg-event-fx - ::auth-success - (fn [{:keys [db]} [_ user pass response]] - ;; TODO: Handle failures differently - (let [login {:u user :p pass}] - {:routes/set-credentials login - :db (-> (update db :active-requests #(max (dec %) 0)) - (assoc :login login)) - :dispatch [::logged-in]}))) + ::credentials-verified credentials-verified) ;; TODO: We have to find another solution for this once we have routes that ;; don't require a login but have the bottom controls diff --git a/src/cljs/airsonic_ui/subs.cljs b/src/cljs/airsonic_ui/subs.cljs index 1891d2d..9d31228 100644 --- a/src/cljs/airsonic_ui/subs.cljs +++ b/src/cljs/airsonic_ui/subs.cljs @@ -2,8 +2,8 @@ (:require [re-frame.core :as re-frame])) ;; can be used to query the user's credentials -;; TODO: Organize login credentials and server location differently (i.e. together) +;; FIXME: this is used for cover images and it's quite ugly tbh (re-frame/reg-sub ::login (fn [db] diff --git a/test/cljs/airsonic_ui/events_test.cljs b/test/cljs/airsonic_ui/events_test.cljs new file mode 100644 index 0000000..1debe49 --- /dev/null +++ b/test/cljs/airsonic_ui/events_test.cljs @@ -0,0 +1,28 @@ +(ns airsonic-ui.events-test + (:require [cljs.test :refer [deftest testing is]] + [clojure.string :as str] + [airsonic-ui.events :as events])) + +(enable-console-print!) + +(deftest authentication + (testing "Credential verification" + (let [server "https://localhost" + fx (events/authenticate {:db {}} [:_ "user" "pass" server]) + request (:http-xhrio fx)] + (testing "uses correct server url" + (is (str/starts-with? (:uri request) server)) + (is (str/includes? (:uri request) "/ping"))) + (testing "saves the given server location" + (is (= server (get-in fx [:db :server])))) + (testing "invokes correct success callback" + (is (= ::events/credentials-verified (first (:on-success request))))))) + (testing "On succesfull response" + (let [fx (events/credentials-verified {:db {}} [:_ "user" "pass"]) + credentials {:u "user" :p "pass"}] + (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 :login])))) + (testing "the login process is finalized" + (is (= [::events/logged-in] (:dispatch fx))))))) From b480676cef86c34d5d6401891bab9bf47e58ba67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arne=20Schl=C3=BCter?= Date: Wed, 30 May 2018 18:38:40 +0200 Subject: [PATCH 3/4] Remember login credentials --- shadow-cljs.edn | 1 + src/cljs/airsonic_ui/core.cljs | 8 ++++- src/cljs/airsonic_ui/events.cljs | 50 +++++++++++++++++++------- src/cljs/airsonic_ui/subs.cljs | 4 +-- test/cljs/airsonic_ui/events_test.cljs | 5 +-- 5 files changed, 51 insertions(+), 17 deletions(-) diff --git a/shadow-cljs.edn b/shadow-cljs.edn index 8d41c32..ace816b 100644 --- a/shadow-cljs.edn +++ b/shadow-cljs.edn @@ -6,6 +6,7 @@ [[reagent "0.7.0"] [re-frame "0.10.5"] [day8.re-frame/http-fx "0.1.6"] + [akiroz.re-frame/storage "0.1.2"] [funcool/bide "1.6.0"] ;; debugging [day8.re-frame/re-frame-10x "0.3.2-react16"] diff --git a/src/cljs/airsonic_ui/core.cljs b/src/cljs/airsonic_ui/core.cljs index 17bd202..692a633 100644 --- a/src/cljs/airsonic_ui/core.cljs +++ b/src/cljs/airsonic_ui/core.cljs @@ -1,8 +1,11 @@ (ns airsonic-ui.core (:require [reagent.core :as reagent] [re-frame.core :as re-frame] + ;; 3rd party effects / coeffects [day8.re-frame.http-fx] - [airsonic-ui.audio] ; <- just registers effects + [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] @@ -19,6 +22,9 @@ (defn ^:export init [] (routes/start-routing!) + (storage/reg-co-fx! :airsonic-ui {:fx :store + :cofx :store}) (re-frame/dispatch-sync [::events/initialize-db]) + (re-frame/dispatch [::events/try-remember-user]) (dev-setup) (mount-root)) diff --git a/src/cljs/airsonic_ui/events.cljs b/src/cljs/airsonic_ui/events.cljs index 4e91a91..6b82793 100644 --- a/src/cljs/airsonic_ui/events.cljs +++ b/src/cljs/airsonic_ui/events.cljs @@ -22,9 +22,10 @@ (defn authenticate "Tries to authenticate a user by pinging the server with credentials, saving - them when the request was succesful." + them when the request was succesful. Bypasses the request when a user saved + their credentials." [{:keys [db]} [_ user pass server]] - {:db (assoc db :server server) + {:db (assoc-in db [:credentials :server] server) :http-xhrio {:method :get :uri (api/url server "ping" {:u user :p pass}) :response-format (ajax/json-response-format {:keywords? true}) @@ -34,17 +35,34 @@ (re-frame/reg-event-fx ::authenticate authenticate) +(defn try-remember-user + "Enables skipping the auth request when credentials are saved in the + local storage; otherwise has no effect" + [{:keys [db store]} [_]] + (when-let [credentials (:credentials store)] + {:db (assoc-in db [:credentials :server] (:server credentials)) + :dispatch [::credentials-verified (:u credentials) (:p credentials) nil]})) + +(re-frame/reg-event-fx + ::try-remember-user + [(re-frame/inject-cofx :store)] + try-remember-user) + (defn credentials-verified "Gets called after the server indicates that the credentials entered by a user - are correct (see `authenticate`)." - [{:keys [db]} [_ user pass response]] - (let [login {:u user :p pass}] - {:routes/set-credentials login - :db (assoc db :login login) + are correct (see `authenticate`)" + [{:keys [db store]} [_event user pass _response]] + (let [auth {:u user :p pass} + credentials (merge (:credentials db) auth)] + {:routes/set-credentials auth + :store {:credentials credentials} + :db (assoc db :credentials credentials) :dispatch [::logged-in]})) (re-frame/reg-event-fx - ::credentials-verified credentials-verified) + ::credentials-verified + [(re-frame/inject-cofx :store)] + credentials-verified) ;; TODO: We have to find another solution for this once we have routes that ;; don't require a login but have the bottom controls @@ -65,11 +83,15 @@ ;; TODO: Move these in the future? events.cljs should just do wiring. We could ;; implement api.cljs as a completely independent module. +(defn- api-url [db endpoint params] + (let [creds (:credentials db)] + (api/url (:server creds) endpoint (merge params (select-keys creds [:u :p]))))) + (re-frame/reg-event-fx :api-request (fn [{:keys [db]} [_ endpoint k params]] {:http-xhrio {:method :get - :uri (api/url (:server db) endpoint (merge params (:login db))) + :uri (api-url db endpoint params) :response-format (ajax/json-response-format {:keywords? true}) :on-success [::api-success k] :on-failure [::api-failure]}})) @@ -90,11 +112,15 @@ ; TODO: Make play, next and previous a bit prettier and more DRY +(defn- song-url [db song] + (let [creds (:credentials db)] + (api/song-url (:server creds) (select-keys creds [:u :p]) song))) + (re-frame/reg-event-fx ; sets up the db, starts to play a song and adds the rest to a playlist ::play-songs (fn [{:keys [db]} [_ songs song]] - {:play-song (api/song-url (:server db) (:login db) song) + {:play-song (song-url db song) :db (-> db (assoc-in [:currently-playing :item] song) (assoc-in [:currently-playing :playlist] songs))})) @@ -106,7 +132,7 @@ current (-> db :currently-playing :item) next (first (rest (drop-while #(not= % current) playlist)))] (when next - {:play-song (api/song-url (:server db) (:login db) next) + {:play-song (song-url db next) :db (assoc-in db [:currently-playing :item] next)})))) (re-frame/reg-event-fx @@ -116,7 +142,7 @@ current (-> db :currently-playing :item) previous (last (take-while #(not= % current) playlist))] (when previous - {:play-song (api/song-url (:server db) (:login db) previous) + {:play-song (song-url db previous) :db (assoc-in db [:currently-playing :item] previous)})))) (re-frame/reg-event-fx diff --git a/src/cljs/airsonic_ui/subs.cljs b/src/cljs/airsonic_ui/subs.cljs index 9d31228..a83f1f0 100644 --- a/src/cljs/airsonic_ui/subs.cljs +++ b/src/cljs/airsonic_ui/subs.cljs @@ -7,12 +7,12 @@ (re-frame/reg-sub ::login (fn [db] - (:login db))) + (select-keys (:credentials db) [:u :p]))) (re-frame/reg-sub ::server (fn [db] - (:server db))) + (get-in db [:credentials :server]))) ;; current hashbang diff --git a/test/cljs/airsonic_ui/events_test.cljs b/test/cljs/airsonic_ui/events_test.cljs index 1debe49..85a97ec 100644 --- a/test/cljs/airsonic_ui/events_test.cljs +++ b/test/cljs/airsonic_ui/events_test.cljs @@ -14,7 +14,7 @@ (is (str/starts-with? (:uri request) server)) (is (str/includes? (:uri request) "/ping"))) (testing "saves the given server location" - (is (= server (get-in fx [:db :server])))) + (is (= server (get-in fx [:db :credentials :server])))) (testing "invokes correct success callback" (is (= ::events/credentials-verified (first (:on-success request))))))) (testing "On succesfull response" @@ -23,6 +23,7 @@ (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 :login])))) + (is (= credentials (-> (get-in fx [:db :credentials]) + (select-keys [:u :p]))))) (testing "the login process is finalized" (is (= [::events/logged-in] (:dispatch fx))))))) From 8eecb4e6f09ade0f46b207e692c49f261ad75bc1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arne=20Schl=C3=BCter?= Date: Wed, 30 May 2018 18:51:44 +0200 Subject: [PATCH 4/4] Add tests for try-remember-user --- test/cljs/airsonic_ui/events_test.cljs | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/test/cljs/airsonic_ui/events_test.cljs b/test/cljs/airsonic_ui/events_test.cljs index 85a97ec..218e8a1 100644 --- a/test/cljs/airsonic_ui/events_test.cljs +++ b/test/cljs/airsonic_ui/events_test.cljs @@ -18,12 +18,28 @@ (testing "invokes correct success callback" (is (= ::events/credentials-verified (first (:on-success request))))))) (testing "On succesfull response" - (let [fx (events/credentials-verified {:db {}} [:_ "user" "pass"]) - credentials {:u "user" :p "pass"}] + (let [creds-before {:server "https://localhost"} + fx (events/credentials-verified {:db {:credentials creds-before}} + [:_ "user" "pass"]) + auth {:u "user" :p "pass"}] (testing "credentials are sent to the router for access rights" - (is (= credentials (:routes/set-credentials fx)))) + (is (= auth (:routes/set-credentials fx)))) (testing "credentials are saved in the global state" - (is (= credentials (-> (get-in fx [:db :credentials]) + (is (= auth (-> (get-in fx [:db :credentials]) (select-keys [:u :p]))))) + (testing "credentials are persisted together with the server address" + (is (= (merge creds-before auth) (get-in fx [:store :credentials])))) (testing "the login process is finalized" - (is (= [::events/logged-in] (:dispatch fx))))))) + (is (= [::events/logged-in] (:dispatch fx)))))) + (testing "When remembering previous login data" + (let [credentials {:server "http://localhost" + :u "another-user" + :p "some_random_password123"} + fx (events/try-remember-user {:store {:credentials credentials}})] + (testing "the auth request is skipped" + (is (nil? (:http-xhrio fx)))) + (testing "we get sent straight to the home page" + (is (= ::events/credentials-verified (first (:dispatch fx))))))) + (testing "When there's no previous login data" + (testing "remembering has no effect" + (is (nil? (events/try-remember-user {}))))))