From 80225d46b1f02a0ee16d04248dea5406239176c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arne=20Schl=C3=BCter?= Date: Wed, 1 Aug 2018 18:36:47 +0200 Subject: [PATCH] Start restructuring audio playback, add some tests for audio Fixes #15 where audio was not stopped on logout --- src/cljs/airsonic_ui/audio.cljs | 73 +++++++++++++++++++- src/cljs/airsonic_ui/events.cljs | 37 +++++----- src/cljs/airsonic_ui/subs.cljs | 15 ---- src/cljs/airsonic_ui/views.cljs | 3 - src/cljs/airsonic_ui/views/bottom_bar.cljs | 20 +++--- test/cljs/airsonic_ui/audio_test.cljs | 40 +++++++++++ test/cljs/airsonic_ui/events_test.cljs | 16 +++-- test/cljs/airsonic_ui/fixtures.cljs | 8 +++ test/cljs/airsonic_ui/test_helpers.cljs | 11 +++ test/cljs/airsonic_ui/test_helpers_test.cljs | 33 +++++---- 10 files changed, 187 insertions(+), 69 deletions(-) create mode 100644 test/cljs/airsonic_ui/audio_test.cljs diff --git a/src/cljs/airsonic_ui/audio.cljs b/src/cljs/airsonic_ui/audio.cljs index 08842b9..297062f 100644 --- a/src/cljs/airsonic_ui/audio.cljs +++ b/src/cljs/airsonic_ui/audio.cljs @@ -20,8 +20,10 @@ (doseq [event ["loadstart" "progress" "play" "timeupdate" "pause"]] (.addEventListener el event #(re-frame/dispatch [:audio/update (->status el)])))) +;; effects to be fired from event handlers + (re-frame/reg-fx - :play-song + :audio/play (fn [song-url] (when-not @audio (reset! audio (js/Audio.)) @@ -31,9 +33,74 @@ (.play @audio))) (re-frame/reg-fx - :toggle-play-pause + :audio/pause (fn [_] - (let [a @audio] + (some-> @audio .pause))) + +(re-frame/reg-fx + :audio/stop + (fn [_] + (when-let [audio @audio] + (.pause audio) + (set! (.-currentTime audio) 0)))) + +(re-frame/reg-fx + :audio/toggle-play-pause + (fn [_] + (if-let [a @audio] (if (.-paused a) (.play a) (.pause a))))) + +;; subscriptions + +(defn summary + "Returns all information about audio that we have" + [db _] + (:audio db)) + +(re-frame/reg-sub :audio/summary summary) + +(defn current-song + "Gives us information about the currently played song as presented by + the airsonic api" + [summary _] + (:current-song summary)) + +(re-frame/reg-sub + :audio/current-song + (fn [_ _] (re-frame/subscribe [:audio/summary])) + current-song) + +(defn playback-status + "Gives us information about the most recently fired html 5 audio event" + [summary _] + (:playback-status summary)) + +(re-frame/reg-sub + :audio/playback-status + (fn [_ _] (re-frame/subscribe [:audio/summary])) + playback-status) + +(defn is-playing? + "Predicate to tell us whether we currently have audio output or not" + [playback-status _] + (and (not (:paused? playback-status)) + (not (:ended? playback-status)))) + +(re-frame/reg-sub + :audio/is-playing? + (fn [_ _] (re-frame/subscribe [:audio/current-playback-status])) + is-playing?) + +(comment + ;; NOTE: Not in use currently + (defn current-playlist + "Lists the complete playlist" + [summary _] + (:playlist summary)) + + (re-frame/reg-sub + :audio/current-playlist + (fn [_ _] (re-frame/subscribe [:audio/summary])) + current-playlist)) diff --git a/src/cljs/airsonic_ui/events.cljs b/src/cljs/airsonic_ui/events.cljs index 1bd0529..dd3e619 100644 --- a/src/cljs/airsonic_ui/events.cljs +++ b/src/cljs/airsonic_ui/events.cljs @@ -130,8 +130,8 @@ [::routes/login {} {:redirect (routes/encode-route redirect)}] [::routes/login])] :store nil - :db (-> (merge (:db cofx) db/default-db) - (dissoc :credentials))})) + :db db/default-db + :audio/stop nil})) (re-frame/reg-event-fx ::logout logout) @@ -180,41 +180,40 @@ ; 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 (song-url db song) - :db (-> db - (assoc-in [:currently-playing :item] song) - (assoc-in [:currently-playing :playlist] songs))})) + {:audio/play (song-url db song) + :db (-> (assoc-in db [:audio :current-song] song) + (assoc-in [:audio :playlist] songs))})) (re-frame/reg-event-fx ::next-song (fn [{:keys [db]} _] - (let [playlist (-> db :currently-playing :playlist) - current (-> db :currently-playing :item) - next (first (rest (drop-while #(not= % current) playlist)))] + (let [playlist (get-in db [:audio :playlist]) + current-song (get-in db [:audio :current-song]) + next (first (rest (drop-while #(not= % current-song) playlist)))] (when next - {:play-song (song-url db next) - :db (assoc-in db [:currently-playing :item] next)})))) + {:audio/play (song-url db next) + :db (assoc-in db [:audio :current-song] next)})))) (re-frame/reg-event-fx ::previous-song (fn [{:keys [db]} _] - (let [playlist (-> db :currently-playing :playlist) - current (-> db :currently-playing :item) - previous (last (take-while #(not= % current) playlist))] + (let [playlist (get-in db [:audio :playlist]) + current-song (get-in db [:audio :current-song]) + previous (last (take-while #(not= % current-song) playlist))] (when previous - {:play-song (song-url db previous) - :db (assoc-in db [:currently-playing :item] previous)})))) + {:audio/play (song-url db previous) + :db (assoc-in db [:audio :current-song] previous)})))) (re-frame/reg-event-fx ::toggle-play-pause (fn [_ _] - {:toggle-play-pause nil})) + {:audio/toggle-play-pause nil})) (re-frame/reg-event-db :audio/update (fn [db [_ status]] - ; we receive this from the player once it's playing - (assoc-in db [:currently-playing :status] status))) + ; this is coming from HTML5 Audio events + (assoc-in db [:audio :playback-status] status))) ;; --- ;; routing diff --git a/src/cljs/airsonic_ui/subs.cljs b/src/cljs/airsonic_ui/subs.cljs index a4d2a22..3dc6e80 100644 --- a/src/cljs/airsonic_ui/subs.cljs +++ b/src/cljs/airsonic_ui/subs.cljs @@ -47,21 +47,6 @@ (fn [db _] (:response db))) -(re-frame/reg-sub - ; returns info on the current song as is (basically the metadata you can read from the file system) - ::currently-playing - (fn [db _] - (:currently-playing db))) - -(re-frame/reg-sub - ::is-playing? - (fn [query-v _] - [(re-frame/subscribe [::currently-playing])]) - (fn [[currently-playing] _] - (let [status (:status currently-playing)] - (and (not (:paused? status)) - (not (:ended? status)))))) - ;; user notifications (defn notifications [db _] (:notifications db)) diff --git a/src/cljs/airsonic_ui/views.cljs b/src/cljs/airsonic_ui/views.cljs index 3d7e172..8b88a1d 100644 --- a/src/cljs/airsonic_ui/views.cljs +++ b/src/cljs/airsonic_ui/views.cljs @@ -67,9 +67,6 @@ (let [notifications @(subscribe [::subs/notifications]) is-booting? @(subscribe [::subs/is-booting?]) [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? diff --git a/src/cljs/airsonic_ui/views/bottom_bar.cljs b/src/cljs/airsonic_ui/views/bottom_bar.cljs index a9ca588..8e14324 100644 --- a/src/cljs/airsonic_ui/views/bottom_bar.cljs +++ b/src/cljs/airsonic_ui/views/bottom_bar.cljs @@ -1,18 +1,17 @@ (ns airsonic-ui.views.bottom-bar (:require [re-frame.core :refer [dispatch subscribe]] [airsonic-ui.events :as events] - [airsonic-ui.subs :as subs] [airsonic-ui.views.cover :refer [cover]] [airsonic-ui.views.icon :refer [icon]])) ;; currently playing / coming next / audio controls... -(defn current-song-info [{:keys [item status]}] +(defn current-song-info [song status] [:article - [:div (:artist item) " - " (:title item)] + [:div (:artist song) " - " (:title song)] ;; FIXME: Sometimes items don't have a duration [:progress.progress.is-tiny {:value (:current-time status) - :max (:duration item)}]]) + :max (:duration song)}]]) (defn playback-controls [is-playing?] [:div.field.has-addons @@ -25,22 +24,23 @@ [icon icon-glyph]]) buttons))]) - (def logo-url "https://airsonic.github.io/airsonic-ui/assets/images/logo/airsonic-light-350x100.png") +(def logo-url "https://airsonic.github.io/airsonic-ui/assets/images/logo/airsonic-light-350x100.png") (defn bottom-bar [] - (let [currently-playing @(subscribe [::subs/currently-playing]) - is-playing? @(subscribe [::subs/is-playing?])] + (let [current-song @(subscribe [:audio/current-song]) + playback-status @(subscribe [:audio/playback-status]) + is-playing? @(subscribe [:audio/is-playing?])] [:nav.navbar.is-fixed-bottom.playback-area [:div.navbar-brand [:div.navbar-item [:img {:src logo-url}]]] [:div.navbar-menu.is-active - (if currently-playing + (if current-song ;; show song info [:section.level.audio-interaction [:div.level-left>article.media - [:div.media-left [cover (:item currently-playing) 48]] - [:div.media-content [current-song-info currently-playing]]] + [:div.media-left [cover current-song 48]] + [:div.media-content [current-song-info current-song playback-status]]] [:div.level-right [playback-controls is-playing?]]] ;; not playing anything [:p.idle-notification "Currently no song selected"])]])) diff --git a/test/cljs/airsonic_ui/audio_test.cljs b/test/cljs/airsonic_ui/audio_test.cljs new file mode 100644 index 0000000..2663774 --- /dev/null +++ b/test/cljs/airsonic_ui/audio_test.cljs @@ -0,0 +1,40 @@ +(ns airsonic-ui.audio-test + (:require [airsonic-ui.audio :as audio] + [airsonic-ui.fixtures :as fixtures] + [airsonic-ui.test-helpers :as helpers] + [cljs.test :refer [deftest testing is]])) + +(enable-console-print!) + +(defn- simulate-playlist [n ] + (repeatedly n #(hash-map :id (rand-int 9999) + :coverArt (rand-int 9999) + :year (+ 1900 (rand-int 118)) + :artist (helpers/rand-str) + :aristId (rand-int 100000) + :title (helpers/rand-str) + :album (helpers/rand-str)))) + +(def fixture + {:audio {:current-song fixtures/song + :playlist (simulate-playlist 20) + :playback-status fixtures/playback-status}}) + +(deftest current-song + (letfn [(current-song [db] + (-> (audio/summary db [:audio/summary]) + (audio/current-song [:audio/current-song])))] + (testing "Should provide information about the song" + (= fixtures/song (current-song fixture))))) + +(deftest playback-status + (letfn [(is-playing? [playback-status] + (audio/is-playing? playback-status [:audio/is-playing?]))] + (testing "Should be shown as not playing when the song is paused or has ended" + (is (not (is-playing? {:paused? true, :ended? false}))) + (is (not (is-playing? {:paused? false, :ended? true})))) + (testing "Should be shown as playing when the song is not paused or finished" + (is (is-playing? {:paused? false, :ended? false}))))) + +#_(deftest current-playlist + (testing "Should show the complete playlist")) diff --git a/test/cljs/airsonic_ui/events_test.cljs b/test/cljs/airsonic_ui/events_test.cljs index 2cfbbd0..804150e 100644 --- a/test/cljs/airsonic_ui/events_test.cljs +++ b/test/cljs/airsonic_ui/events_test.cljs @@ -93,7 +93,9 @@ (testing "Should redirect to the login screen" (is (dispatches? fx [:routes/do-navigation [::routes/login]]))) (testing "Should reset the app-db" - (is (= db/default-db (:db fx))))) + (is (= db/default-db (:db fx)))) + (testing "Should stop currently playing songs" + (is (contains? fx :audio/stop)))) (testing "Should be able to keep a redirection parameter" (let [redirect [:route {:with-data #{1 2 3 4 5}}] navigation-event (:dispatch (events/logout {} [:_ :redirect-to redirect]))] @@ -102,13 +104,15 @@ (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 fixtures/responses)])] - (is (= :error (-> fx :dispatch second)))))) + (let [fx (events/good-api-response {} [:_ (:error fixtures/responses)]) + ev (:dispatch fx)] + (is (= :notification/show (first ev))) + (is (= :error (second ev)))))) + +(defn- first-notification [fx] + (-> (get-in fx [:db :notifications]) vals first)) (deftest user-notifications (testing "Should be able to display a message with an assigned level" diff --git a/test/cljs/airsonic_ui/fixtures.cljs b/test/cljs/airsonic_ui/fixtures.cljs index 30061ca..fc7db2c 100644 --- a/test/cljs/airsonic_ui/fixtures.cljs +++ b/test/cljs/airsonic_ui/fixtures.cljs @@ -43,3 +43,11 @@ :contentType "audio/mpeg", :album "Reincarnations, Pt. 2 - The Remix Chapter 2009 - 2014", :track 14}) + +(def playback-status + {:ended? false + :loop? false + :muted? false + :paused? false + :current-src "https://londe.arnes.space/rest/stream?f=json&c=airsonic-ui-cljs&v=1.15.0&id=9574&u=arne&p=27h-%25bO%5B8-.ys%40SQ%7Bg%24-%5B5NZkX%7Dw%24NNwY%263DPATi%2CgaFoH%40e" + :current-time 3.477029}) diff --git a/test/cljs/airsonic_ui/test_helpers.cljs b/test/cljs/airsonic_ui/test_helpers.cljs index 05a58a9..1762f47 100644 --- a/test/cljs/airsonic_ui/test_helpers.cljs +++ b/test/cljs/airsonic_ui/test_helpers.cljs @@ -6,3 +6,14 @@ [cofx ev] (let [all-events (conj (get cofx :dispatch-n []) (:dispatch cofx))] (boolean (some #(= ev (if (vector? ev) % (first %))) all-events)))) + +(defn rand-str + "Generates a random string; ported from https://stackoverflow.com/a/27747377/2345852" + ([] (rand-str 40)) + ([len] + (let [arr (js/Uint8Array. (/ len 2))] + (.. js/window -crypto (getRandomValues arr)) + (.. js/Array + (from arr #(-> (str 0 (.toString % 16)) + (.substr -2))) + (join ""))))) diff --git a/test/cljs/airsonic_ui/test_helpers_test.cljs b/test/cljs/airsonic_ui/test_helpers_test.cljs index 3b3ad76..ec7153c 100644 --- a/test/cljs/airsonic_ui/test_helpers_test.cljs +++ b/test/cljs/airsonic_ui/test_helpers_test.cljs @@ -1,17 +1,24 @@ (ns airsonic-ui.test-helpers-test (:require [cljs.test :refer [deftest testing is]] - [airsonic-ui.test-helpers :refer [dispatches?]])) + [airsonic-ui.test-helpers :as h])) (deftest dispatch-helper - (testing "single dispatch" - (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 (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 (false? (dispatches? {:dispatch-n [[:foo 1 2 3]]} [:bar 2 3]))))) + (testing "Should identify singly dispatched events" + (is (false? (h/dispatches? {} :foo))) + (is (true? (h/dispatches? {:dispatch [:foo 1 2 3]} :foo))) + (is (false? (h/dispatches? {:dispatch [:foo 1 2 3]} :bar))) + (is (true? (h/dispatches? {:dispatch [:foo 1 2 3]} [:foo 1 2 3]))) + (is (false? (h/dispatches? {:dispatch [:foo 1 2 3]} [:bar 2 3])))) + (testing "Should identify an event along multiple dispatched events" + (is (false? (h/dispatches? {:dispatch-n [[:bar]]} :foo))) + (is (true? (h/dispatches? {:dispatch-n [[:foo 1 2 3]]} :foo))) + (is (false? (h/dispatches? {:dispatch-n [[:foo 1 2 3]]} :bar))) + (is (h/dispatches? {:dispatch-n [[:foo 1 2 3]]} [:foo 1 2 3])) + (is (false? (h/dispatches? {:dispatch-n [[:foo 1 2 3]]} [:bar 2 3]))))) + +(deftest rand-str + (testing "Generates strings" + (is (string? (h/rand-str))) + (is (string? (h/rand-str 20)))) + (testing "Should respect the length for even lengths" + (is (= 124 (count (h/rand-str 124))))))