From 9d1caffd9cb239e0761dbdc467a290b75c8eefb2 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 8 Sep 2024 23:23:53 +0200 Subject: [PATCH] Handle idle time between fetch results (#212) Closes #211 Reviewed-on: https://git.thenineworlds.net/wojtek/musichoard/pulls/212 --- src/main.rs | 13 +- src/tui/app/machine/browse.rs | 319 +-------------------- src/tui/app/machine/critical.rs | 24 +- src/tui/app/machine/fetch.rs | 486 ++++++++++++++++++++++++++++++++ src/tui/app/machine/info.rs | 11 - src/tui/app/machine/matches.rs | 241 +++++----------- src/tui/app/machine/mod.rs | 133 +++++++-- src/tui/app/machine/reload.rs | 11 - src/tui/app/machine/search.rs | 11 - src/tui/app/mod.rs | 56 ++-- src/tui/event.rs | 34 ++- src/tui/handler.rs | 42 ++- src/tui/mod.rs | 7 +- src/tui/ui/fetch.rs | 9 + src/tui/ui/minibuffer.rs | 15 +- src/tui/ui/mod.rs | 21 +- 16 files changed, 836 insertions(+), 597 deletions(-) create mode 100644 src/tui/app/machine/fetch.rs create mode 100644 src/tui/ui/fetch.rs diff --git a/src/main.rs b/src/main.rs index 73233f3..3fa803b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -79,16 +79,19 @@ fn with( let backend = CrosstermBackend::new(io::stdout()); let terminal = Terminal::new(backend).expect("failed to initialise terminal"); - let channel = EventChannel::new(); - let listener = EventListener::new(channel.sender()); - let handler = EventHandler::new(channel.receiver()); - let http = MusicBrainzHttp::new(MUSICHOARD_HTTP_USER_AGENT).expect("failed to initialise HTTP client"); let client = MusicBrainzClient::new(http); let musicbrainz = MusicBrainz::new(client); - let app = App::new(music_hoard, musicbrainz); + let channel = EventChannel::new(); + let listener_sender = channel.sender(); + let app_sender = channel.sender(); + + let listener = EventListener::new(listener_sender); + let handler = EventHandler::new(channel.receiver()); + + let app = App::new(music_hoard, musicbrainz, app_sender); let ui = Ui; // Run the TUI application. diff --git a/src/tui/app/machine/browse.rs b/src/tui/app/machine/browse.rs index 4b7b6c2..fe6014a 100644 --- a/src/tui/app/machine/browse.rs +++ b/src/tui/app/machine/browse.rs @@ -1,21 +1,7 @@ -use std::{ - sync::{mpsc, Arc, Mutex}, - thread, time, -}; - -use musichoard::collection::{ - album::AlbumMeta, - artist::ArtistMeta, - musicbrainz::{IMusicBrainzRef, Mbid}, -}; - -use crate::tui::{ - app::{ - machine::{App, AppInner, AppMachine}, - selection::{Delta, ListSelection}, - AppMatchesInfo, AppPublic, AppState, IAppInteractBrowse, - }, - lib::interface::musicbrainz::{Error as MbError, IMusicBrainz}, +use crate::tui::app::{ + machine::{App, AppInner, AppMachine}, + selection::{Delta, ListSelection}, + AppPublic, AppState, IAppInteractBrowse, }; pub struct AppBrowse; @@ -93,107 +79,18 @@ impl IAppInteractBrowse for AppMachine { } fn fetch_musicbrainz(self) -> Self::APP { - let coll = self.inner.music_hoard.get_collection(); - let artist = match self.inner.selection.state_artist(coll) { - Some(artist_state) => &coll[artist_state.index], - None => { - return AppMachine::error(self.inner, "cannot fetch: no artist selected").into() - } - }; - - let (matches_tx, matches_rx) = mpsc::channel::(); - - match artist.meta.musicbrainz { - Some(ref arid) => { - let musicbrainz = Arc::clone(&self.inner.musicbrainz); - let arid = arid.mbid().clone(); - let albums = artist.albums.iter().map(|a| &a.meta).cloned().collect(); - thread::spawn(|| Self::fetch_albums(musicbrainz, matches_tx, arid, albums)); - } - None => { - let musicbrainz = Arc::clone(&self.inner.musicbrainz); - let artist = artist.meta.clone(); - thread::spawn(|| Self::fetch_artist(musicbrainz, matches_tx, artist)); - } - }; - - AppMachine::app_matches(self.inner, matches_rx) - } - - fn no_op(self) -> Self::APP { - self.into() - } -} - -pub type FetchError = MbError; -pub type FetchResult = Result; -pub type FetchSender = mpsc::Sender; -pub type FetchReceiver = mpsc::Receiver; - -trait IAppInteractBrowsePrivate { - fn fetch_artist( - musicbrainz: Arc>, - matches_tx: FetchSender, - artist: ArtistMeta, - ); - fn fetch_albums( - musicbrainz: Arc>, - matches_tx: FetchSender, - arid: Mbid, - albums: Vec, - ); -} - -impl IAppInteractBrowsePrivate for AppMachine { - fn fetch_artist( - musicbrainz: Arc>, - matches_tx: FetchSender, - artist: ArtistMeta, - ) { - let result = musicbrainz.lock().unwrap().search_artist(&artist); - let result = result.map(|list| AppMatchesInfo::artist(artist, list)); - matches_tx.send(result).ok(); - } - - fn fetch_albums( - musicbrainz: Arc>, - matches_tx: FetchSender, - arid: Mbid, - albums: Vec, - ) { - let mut musicbrainz = musicbrainz.lock().unwrap(); - let mut album_iter = albums.into_iter().peekable(); - while let Some(album) = album_iter.next() { - if album.musicbrainz.is_some() { - continue; - } - - let result = musicbrainz.search_release_group(&arid, &album); - let result = result.map(|list| AppMatchesInfo::album(album, list)); - if matches_tx.send(result).is_err() { - // If receiver disconnects just drop the rest. - return; - } - - if album_iter.peek().is_some() { - thread::sleep(time::Duration::from_secs(1)); - } - } + AppMachine::app_fetch_new(self.inner) } } #[cfg(test)] mod tests { - use mockall::{predicate, Sequence}; - use musichoard::collection::{album::AlbumMeta, artist::ArtistMeta, musicbrainz::Mbid}; - use crate::tui::{ app::{ machine::tests::{inner, inner_with_mb, music_hoard}, - AppAlbumMatches, AppArtistMatches, AppMatchesInfo, Category, IAppAccess, IAppInteract, - IAppInteractMatches, MatchOption, + Category, IAppAccess, IAppInteract, }, - lib::interface::musicbrainz::{self, Match, MockIMusicBrainz}, + lib::interface::musicbrainz::MockIMusicBrainz, testmod::COLLECTION, }; @@ -271,38 +168,7 @@ mod tests { #[test] fn fetch_musicbrainz() { - let mut mb_api = MockIMusicBrainz::new(); - - let arid: Mbid = "11111111-1111-1111-1111-111111111111".try_into().unwrap(); - let album_1 = COLLECTION[1].albums[0].meta.clone(); - let album_4 = COLLECTION[1].albums[3].meta.clone(); - - let album_match_1_1 = Match::new(100, album_1.clone()); - let album_match_1_2 = Match::new(50, album_4.clone()); - let album_match_4_1 = Match::new(100, album_4.clone()); - let album_match_4_2 = Match::new(30, album_1.clone()); - let matches_1 = vec![album_match_1_1.clone(), album_match_1_2.clone()]; - let matches_4 = vec![album_match_4_1.clone(), album_match_4_2.clone()]; - - let result_1: Result>, musicbrainz::Error> = Ok(matches_1.clone()); - let result_4: Result>, musicbrainz::Error> = Ok(matches_4.clone()); - - // Other albums have an MBID and so they will be skipped. - let mut seq = Sequence::new(); - - mb_api - .expect_search_release_group() - .with(predicate::eq(arid.clone()), predicate::eq(album_1.clone())) - .times(1) - .in_sequence(&mut seq) - .return_once(|_, _| result_1); - mb_api - .expect_search_release_group() - .with(predicate::eq(arid.clone()), predicate::eq(album_4.clone())) - .times(1) - .in_sequence(&mut seq) - .return_once(|_, _| result_4); - + let mb_api = MockIMusicBrainz::new(); let browse = AppMachine::browse(inner_with_mb(music_hoard(COLLECTION.to_owned()), mb_api)); // Use the second artist for this test. @@ -310,172 +176,11 @@ mod tests { let mut app = browse.fetch_musicbrainz(); let public = app.get(); - assert!(matches!(public.state, AppState::Matches(_))); - let public_matches = public.state.unwrap_matches(); - - let mut matches_1: Vec> = - matches_1.into_iter().map(Into::into).collect(); - matches_1.push(MatchOption::CannotHaveMbid); - let expected = Some(AppMatchesInfo::Album(AppAlbumMatches { - matching: album_1.clone(), - list: matches_1.clone(), - })); - assert_eq!(public_matches.matches, expected.as_ref()); - - let mut app = app.unwrap_matches().select(); - - let public = app.get(); - assert!(matches!(public.state, AppState::Matches(_))); - - let public_matches = public.state.unwrap_matches(); - - let mut matches_4: Vec> = - matches_4.into_iter().map(Into::into).collect(); - matches_4.push(MatchOption::CannotHaveMbid); - let expected = Some(AppMatchesInfo::Album(AppAlbumMatches { - matching: album_4.clone(), - list: matches_4.clone(), - })); - assert_eq!(public_matches.matches, expected.as_ref()); - - let app = app.unwrap_matches().select(); - app.unwrap_browse(); - } - - #[test] - fn fetch_musicbrainz_no_artist() { - let browse = AppMachine::browse(inner(music_hoard(vec![]))); - let app = browse.fetch_musicbrainz(); - app.unwrap_error(); - } - - #[test] - fn fetch_musicbrainz_no_artist_mbid() { - let mut mb_api = MockIMusicBrainz::new(); - - let artist = COLLECTION[3].meta.clone(); - - let artist_match_1 = Match::new(100, artist.clone()); - let artist_match_2 = Match::new(50, artist.clone()); - let matches = vec![artist_match_1.clone(), artist_match_2.clone()]; - - let result: Result>, musicbrainz::Error> = Ok(matches.clone()); - - mb_api - .expect_search_artist() - .with(predicate::eq(artist.clone())) - .times(1) - .return_once(|_| result); - - let browse = AppMachine::browse(inner_with_mb(music_hoard(COLLECTION.to_owned()), mb_api)); - - // Use the fourth artist for this test as they have no MBID. - let browse = browse.increment_selection(Delta::Line).unwrap_browse(); - let browse = browse.increment_selection(Delta::Line).unwrap_browse(); - let browse = browse.increment_selection(Delta::Line).unwrap_browse(); - let mut app = browse.fetch_musicbrainz(); - - let public = app.get(); - assert!(matches!(public.state, AppState::Matches(_))); - - let public_matches = public.state.unwrap_matches(); - - let mut matches: Vec> = - matches.into_iter().map(Into::into).collect(); - matches.push(MatchOption::CannotHaveMbid); - let expected = Some(AppMatchesInfo::Artist(AppArtistMatches { - matching: artist.clone(), - list: matches.clone(), - })); - assert_eq!(public_matches.matches, expected.as_ref()); - - let app = app.unwrap_matches().select(); - app.unwrap_browse(); - } - - #[test] - fn fetch_musicbrainz_artist_api_error() { - let mut mb_api = MockIMusicBrainz::new(); - - let error = Err(musicbrainz::Error::RateLimit); - - mb_api - .expect_search_artist() - .times(1) - .return_once(|_| error); - - let browse = AppMachine::browse(inner_with_mb(music_hoard(COLLECTION.to_owned()), mb_api)); - - // Use the fourth artist for this test as they have no MBID. - let browse = browse.increment_selection(Delta::Line).unwrap_browse(); - let browse = browse.increment_selection(Delta::Line).unwrap_browse(); - let browse = browse.increment_selection(Delta::Line).unwrap_browse(); - - let app = browse.fetch_musicbrainz(); - app.unwrap_error(); - } - - #[test] - fn fetch_musicbrainz_album_api_error() { - let mut mb_api = MockIMusicBrainz::new(); - - let error = Err(musicbrainz::Error::RateLimit); - - mb_api - .expect_search_release_group() - .times(1) - .return_once(|_, _| error); - - let browse = AppMachine::browse(inner_with_mb(music_hoard(COLLECTION.to_owned()), mb_api)); - - let app = browse.fetch_musicbrainz(); - app.unwrap_error(); - } - - #[test] - fn fetch_musicbrainz_artist_receiver_disconnect() { - let (tx, rx) = mpsc::channel::(); - drop(rx); - - let mut mb_api = MockIMusicBrainz::new(); - - mb_api - .expect_search_artist() - .times(1) - .return_once(|_| Ok(vec![])); - - // We only check it does not panic and that it doesn't call the API more than once. - AppMachine::fetch_artist(Arc::new(Mutex::new(mb_api)), tx, COLLECTION[3].meta.clone()); - } - - #[test] - fn fetch_musicbrainz_albums_receiver_disconnect() { - let (tx, rx) = mpsc::channel::(); - drop(rx); - - let mut mb_api = MockIMusicBrainz::new(); - - mb_api - .expect_search_release_group() - .times(1) - .return_once(|_, _| Ok(vec![])); - - // We only check it does not panic and that it doesn't call the API more than once. - let mbref = &COLLECTION[1].meta.musicbrainz; - let albums = &COLLECTION[1].albums; - AppMachine::fetch_albums( - Arc::new(Mutex::new(mb_api)), - tx, - mbref.as_ref().unwrap().mbid().clone(), - albums.iter().map(|a| &a.meta).cloned().collect(), + // Because of fetch's threaded behaviour, this unit test cannot expect one or the other. + assert!( + matches!(public.state, AppState::Matches(_)) + || matches!(public.state, AppState::Fetch(_)) ); } - - #[test] - fn no_op() { - let browse = AppMachine::browse(inner(music_hoard(vec![]))); - let app = browse.no_op(); - app.unwrap_browse(); - } } diff --git a/src/tui/app/machine/critical.rs b/src/tui/app/machine/critical.rs index 476ba8a..8206dbb 100644 --- a/src/tui/app/machine/critical.rs +++ b/src/tui/app/machine/critical.rs @@ -1,6 +1,6 @@ use crate::tui::app::{ machine::{App, AppInner, AppMachine}, - AppPublic, AppState, IAppInteractCritical, + AppPublic, AppState, }; pub struct AppCritical { @@ -32,25 +32,3 @@ impl<'a> From<&'a mut AppMachine> for AppPublic<'a> { } } } - -impl IAppInteractCritical for AppMachine { - type APP = App; - - fn no_op(self) -> Self::APP { - self.into() - } -} - -#[cfg(test)] -mod tests { - use crate::tui::app::machine::tests::{inner, music_hoard}; - - use super::*; - - #[test] - fn no_op() { - let critical = AppMachine::critical(inner(music_hoard(vec![])), "get rekt"); - let app = critical.no_op(); - app.unwrap_critical(); - } -} diff --git a/src/tui/app/machine/fetch.rs b/src/tui/app/machine/fetch.rs new file mode 100644 index 0000000..da924bd --- /dev/null +++ b/src/tui/app/machine/fetch.rs @@ -0,0 +1,486 @@ +use std::{ + sync::{ + mpsc::{self, TryRecvError}, + Arc, Mutex, + }, + thread, time, +}; + +use musichoard::collection::{ + album::AlbumMeta, + artist::{Artist, ArtistMeta}, + musicbrainz::{IMusicBrainzRef, Mbid}, +}; + +use crate::tui::{ + app::{ + machine::{App, AppInner, AppMachine}, + AppMatchesInfo, AppPublic, AppState, IAppEventFetch, IAppInteractFetch, + }, + event::{Event, EventSender}, + lib::interface::musicbrainz::{self, Error as MbError, IMusicBrainz}, +}; + +use super::matches::AppMatches; + +pub struct AppFetch { + fetch_rx: FetchReceiver, +} + +impl AppFetch { + pub fn new(fetch_rx: FetchReceiver) -> Self { + AppFetch { fetch_rx } + } +} + +pub type FetchError = MbError; +pub type FetchResult = Result; +pub type FetchSender = mpsc::Sender; +pub type FetchReceiver = mpsc::Receiver; + +impl AppMachine { + fn fetch(inner: AppInner, state: AppFetch) -> Self { + AppMachine { inner, state } + } + + pub fn app_fetch_new(inner: AppInner) -> App { + let coll = inner.music_hoard.get_collection(); + let artist = match inner.selection.state_artist(coll) { + Some(artist_state) => &coll[artist_state.index], + None => return AppMachine::error(inner, "cannot fetch: no artist selected").into(), + }; + + let (fetch_tx, fetch_rx) = mpsc::channel::(); + Self::spawn_fetch_thread(&inner, artist, fetch_tx); + + let fetch = AppFetch::new(fetch_rx); + AppMachine::app_fetch(inner, fetch, true) + } + + pub fn app_fetch_next(inner: AppInner, fetch: AppFetch) -> App { + Self::app_fetch(inner, fetch, false) + } + + fn app_fetch(inner: AppInner, fetch: AppFetch, first: bool) -> App { + match fetch.fetch_rx.try_recv() { + Ok(fetch_result) => match fetch_result { + Ok(next_match) => { + let current = Some(next_match); + AppMachine::matches(inner, AppMatches::new(current, fetch)).into() + } + Err(fetch_err) => { + AppMachine::error(inner, format!("fetch failed: {fetch_err}")).into() + } + }, + Err(recv_err) => match recv_err { + TryRecvError::Empty => AppMachine::fetch(inner, fetch).into(), + TryRecvError::Disconnected => { + if first { + AppMachine::matches(inner, AppMatches::new(None, fetch)).into() + } else { + AppMachine::browse(inner).into() + } + } + }, + } + } + + fn spawn_fetch_thread( + inner: &AppInner, + artist: &Artist, + tx: FetchSender, + ) -> thread::JoinHandle<()> { + match artist.meta.musicbrainz { + Some(ref arid) => { + let musicbrainz = Arc::clone(&inner.musicbrainz); + let events = inner.events.clone(); + let arid = arid.mbid().clone(); + let albums = artist.albums.iter().map(|a| &a.meta).cloned().collect(); + thread::spawn(|| Self::fetch_albums(musicbrainz, tx, events, arid, albums)) + } + None => { + let musicbrainz = Arc::clone(&inner.musicbrainz); + let events = inner.events.clone(); + let artist = artist.meta.clone(); + thread::spawn(|| Self::fetch_artist(musicbrainz, tx, events, artist)) + } + } + } + + fn fetch_artist( + musicbrainz: Arc>, + fetch_tx: FetchSender, + events: EventSender, + artist: ArtistMeta, + ) { + let result = musicbrainz.lock().unwrap().search_artist(&artist); + let result = result.map(|list| AppMatchesInfo::artist(artist, list)); + Self::send_fetch_result(&fetch_tx, &events, result).ok(); + } + + fn fetch_albums( + musicbrainz: Arc>, + fetch_tx: FetchSender, + events: EventSender, + arid: Mbid, + albums: Vec, + ) { + let mut musicbrainz = musicbrainz.lock().unwrap(); + let mut album_iter = albums.into_iter().peekable(); + while let Some(album) = album_iter.next() { + if album.musicbrainz.is_some() { + continue; + } + + let result = musicbrainz.search_release_group(&arid, &album); + let result = result.map(|list| AppMatchesInfo::album(album, list)); + if Self::send_fetch_result(&fetch_tx, &events, result).is_err() { + return; + }; + + if album_iter.peek().is_some() { + thread::sleep(time::Duration::from_secs(1)); + } + } + } + + fn send_fetch_result( + fetch_tx: &FetchSender, + events: &EventSender, + result: Result, + ) -> Result<(), ()> { + // If receiver disconnects just drop the rest. + fetch_tx.send(result).map_err(|_| ())?; + + // If this send fails the event listener is dead. Don't panic as this function runs in a + // detached thread so this might be happening during normal shut down. + events.send(Event::FetchResultReady).map_err(|_| ())?; + + Ok(()) + } +} + +impl From> for App { + fn from(machine: AppMachine) -> Self { + AppState::Fetch(machine) + } +} + +impl<'a> From<&'a mut AppMachine> for AppPublic<'a> { + fn from(machine: &'a mut AppMachine) -> Self { + AppPublic { + inner: (&mut machine.inner).into(), + state: AppState::Fetch(()), + } + } +} + +impl IAppInteractFetch for AppMachine { + type APP = App; + + fn abort(self) -> Self::APP { + AppMachine::browse(self.inner).into() + } +} + +impl IAppEventFetch for AppMachine { + type APP = App; + + fn fetch_result_ready(self) -> Self::APP { + Self::app_fetch_next(self.inner, self.state) + } +} + +#[cfg(test)] +mod tests { + use mockall::{predicate, Sequence}; + use musichoard::collection::artist::ArtistMeta; + + use crate::tui::{ + app::{ + machine::tests::{inner, music_hoard}, + AppAlbumMatches, AppArtistMatches, IAppInteract, + }, + event::EventReceiver, + lib::interface::musicbrainz::{self, Match, MockIMusicBrainz}, + testmod::COLLECTION, + EventChannel, + }; + + use super::*; + + #[test] + fn fetch_no_artist() { + let app = AppMachine::app_fetch_new(inner(music_hoard(vec![]))); + assert!(matches!(app.state(), AppState::Error(_))); + } + + fn event_channel() -> (EventSender, EventReceiver) { + let event_channel = EventChannel::new(); + let events_tx = event_channel.sender(); + let events_rx = event_channel.receiver(); + (events_tx, events_rx) + } + + fn album_expectations_1() -> (AlbumMeta, Vec>) { + let album_1 = COLLECTION[1].albums[0].meta.clone(); + let album_4 = COLLECTION[1].albums[3].meta.clone(); + + let album_match_1_1 = Match::new(100, album_1.clone()); + let album_match_1_2 = Match::new(50, album_4.clone()); + let matches_1 = vec![album_match_1_1.clone(), album_match_1_2.clone()]; + + (album_1, matches_1) + } + + fn album_expectations_4() -> (AlbumMeta, Vec>) { + let album_1 = COLLECTION[1].albums[0].meta.clone(); + let album_4 = COLLECTION[1].albums[3].meta.clone(); + + let album_match_4_1 = Match::new(100, album_4.clone()); + let album_match_4_2 = Match::new(30, album_1.clone()); + let matches_4 = vec![album_match_4_1.clone(), album_match_4_2.clone()]; + + (album_4, matches_4) + } + + fn search_release_group_expectation( + api: &mut MockIMusicBrainz, + seq: &mut Sequence, + arid: &Mbid, + album: &AlbumMeta, + matches: &[Match], + ) { + let result = Ok(matches.to_owned()); + api.expect_search_release_group() + .with(predicate::eq(arid.clone()), predicate::eq(album.clone())) + .times(1) + .in_sequence(seq) + .return_once(|_, _| result); + } + + #[test] + fn fetch_albums() { + let mut mb_api = MockIMusicBrainz::new(); + + let arid: Mbid = "11111111-1111-1111-1111-111111111111".try_into().unwrap(); + + let (album_1, matches_1) = album_expectations_1(); + let (album_4, matches_4) = album_expectations_4(); + + // Other albums have an MBID and so they will be skipped. + let mut seq = Sequence::new(); + + search_release_group_expectation(&mut mb_api, &mut seq, &arid, &album_1, &matches_1); + search_release_group_expectation(&mut mb_api, &mut seq, &arid, &album_4, &matches_4); + + let music_hoard = music_hoard(COLLECTION.to_owned()); + let (events_tx, events_rx) = event_channel(); + let inner = AppInner::new(music_hoard, mb_api, events_tx); + + let (fetch_tx, fetch_rx) = mpsc::channel(); + // Use the second artist for this test. + let handle = AppMachine::spawn_fetch_thread(&inner, &COLLECTION[1], fetch_tx); + handle.join().unwrap(); + + assert_eq!(events_rx.try_recv().unwrap(), Event::FetchResultReady); + let result = fetch_rx.try_recv().unwrap(); + let expected = Ok(AppMatchesInfo::Album(AppAlbumMatches { + matching: album_1.clone(), + list: matches_1.iter().cloned().map(Into::into).collect(), + })); + assert_eq!(result, expected); + + assert_eq!(events_rx.try_recv().unwrap(), Event::FetchResultReady); + let result = fetch_rx.try_recv().unwrap(); + let expected = Ok(AppMatchesInfo::Album(AppAlbumMatches { + matching: album_4.clone(), + list: matches_4.iter().cloned().map(Into::into).collect(), + })); + assert_eq!(result, expected); + } + + fn artist_expectations() -> (ArtistMeta, Vec>) { + let artist = COLLECTION[3].meta.clone(); + + let artist_match_1 = Match::new(100, artist.clone()); + let artist_match_2 = Match::new(50, artist.clone()); + let matches = vec![artist_match_1.clone(), artist_match_2.clone()]; + + (artist, matches) + } + + fn search_artist_expectation( + api: &mut MockIMusicBrainz, + seq: &mut Sequence, + artist: &ArtistMeta, + matches: &[Match], + ) { + let result = Ok(matches.to_owned()); + api.expect_search_artist() + .with(predicate::eq(artist.clone())) + .times(1) + .in_sequence(seq) + .return_once(|_| result); + } + + #[test] + fn fetch_artist() { + let mut mb_api = MockIMusicBrainz::new(); + + let (artist, matches) = artist_expectations(); + let mut seq = Sequence::new(); + search_artist_expectation(&mut mb_api, &mut seq, &artist, &matches); + + let music_hoard = music_hoard(COLLECTION.to_owned()); + let (events_tx, events_rx) = event_channel(); + let inner = AppInner::new(music_hoard, mb_api, events_tx); + + let (fetch_tx, fetch_rx) = mpsc::channel(); + // Use the fourth artist for this test as they have no MBID. + let handle = AppMachine::spawn_fetch_thread(&inner, &COLLECTION[3], fetch_tx); + handle.join().unwrap(); + + assert_eq!(events_rx.try_recv().unwrap(), Event::FetchResultReady); + let result = fetch_rx.try_recv().unwrap(); + let expected = Ok(AppMatchesInfo::Artist(AppArtistMatches { + matching: artist.clone(), + list: matches.iter().cloned().map(Into::into).collect(), + })); + assert_eq!(result, expected); + } + + #[test] + fn fetch_artist_fetch_disconnect() { + let mut mb_api = MockIMusicBrainz::new(); + + let (artist, matches) = artist_expectations(); + let mut seq = Sequence::new(); + search_artist_expectation(&mut mb_api, &mut seq, &artist, &matches); + + let music_hoard = music_hoard(COLLECTION.to_owned()); + let (events_tx, events_rx) = event_channel(); + let inner = AppInner::new(music_hoard, mb_api, events_tx); + + let (fetch_tx, _) = mpsc::channel(); + // Use the fourth artist for this test as they have no MBID. + let handle = AppMachine::spawn_fetch_thread(&inner, &COLLECTION[3], fetch_tx); + handle.join().unwrap(); + + assert!(events_rx.try_recv().is_err()); + } + + #[test] + fn fetch_albums_event_disconnect() { + let mut mb_api = MockIMusicBrainz::new(); + + let arid: Mbid = "11111111-1111-1111-1111-111111111111".try_into().unwrap(); + + let (album_1, matches_1) = album_expectations_1(); + + let mut seq = Sequence::new(); + search_release_group_expectation(&mut mb_api, &mut seq, &arid, &album_1, &matches_1); + + let music_hoard = music_hoard(COLLECTION.to_owned()); + let (events_tx, _) = event_channel(); + let inner = AppInner::new(music_hoard, mb_api, events_tx); + + let (fetch_tx, fetch_rx) = mpsc::channel(); + // Use the second artist for this test. + let handle = AppMachine::spawn_fetch_thread(&inner, &COLLECTION[1], fetch_tx); + handle.join().unwrap(); + + let result = fetch_rx.try_recv().unwrap(); + let expected = Ok(AppMatchesInfo::Album(AppAlbumMatches { + matching: album_1.clone(), + list: matches_1.iter().cloned().map(Into::into).collect(), + })); + assert_eq!(result, expected); + + assert_eq!(fetch_rx.try_recv().unwrap_err(), TryRecvError::Disconnected); + } + + #[test] + fn recv_ok_fetch_ok() { + let (tx, rx) = mpsc::channel::(); + + let artist = COLLECTION[3].meta.clone(); + let fetch_result = Ok(AppMatchesInfo::artist::>(artist, vec![])); + tx.send(fetch_result).unwrap(); + + let inner = inner(music_hoard(COLLECTION.clone())); + let fetch = AppFetch::new(rx); + let app = AppMachine::app_fetch(inner, fetch, true); + assert!(matches!(app, AppState::Matches(_))); + } + + #[test] + fn recv_ok_fetch_err() { + let (tx, rx) = mpsc::channel::(); + + let fetch_result = Err(musicbrainz::Error::RateLimit); + tx.send(fetch_result).unwrap(); + + let inner = inner(music_hoard(COLLECTION.clone())); + let fetch = AppFetch::new(rx); + let app = AppMachine::app_fetch(inner, fetch, true); + assert!(matches!(app, AppState::Error(_))); + } + + #[test] + fn recv_err_empty() { + let (_tx, rx) = mpsc::channel::(); + + let inner = inner(music_hoard(COLLECTION.clone())); + let fetch = AppFetch::new(rx); + let app = AppMachine::app_fetch(inner, fetch, true); + assert!(matches!(app, AppState::Fetch(_))); + } + + #[test] + fn recv_err_disconnected_first() { + let (_, rx) = mpsc::channel::(); + + let inner = inner(music_hoard(COLLECTION.clone())); + let fetch = AppFetch::new(rx); + let app = AppMachine::app_fetch(inner, fetch, true); + assert!(matches!(app, AppState::Matches(_))); + } + + #[test] + fn recv_err_disconnected_next() { + let (_, rx) = mpsc::channel::(); + + let fetch = AppFetch::new(rx); + let app = AppMachine::app_fetch_next(inner(music_hoard(COLLECTION.clone())), fetch); + assert!(matches!(app, AppState::Browse(_))); + } + + #[test] + fn empty_first_then_ready() { + let (tx, rx) = mpsc::channel::(); + + let inner = inner(music_hoard(COLLECTION.clone())); + let fetch = AppFetch::new(rx); + let app = AppMachine::app_fetch(inner, fetch, true); + assert!(matches!(app, AppState::Fetch(_))); + + let artist = COLLECTION[3].meta.clone(); + let fetch_result = Ok(AppMatchesInfo::artist::>(artist, vec![])); + tx.send(fetch_result).unwrap(); + + let app = app.unwrap_fetch().fetch_result_ready(); + assert!(matches!(app, AppState::Matches(_))); + } + + #[test] + fn abort() { + let (_, rx) = mpsc::channel::(); + + let fetch = AppFetch::new(rx); + let app = AppMachine::fetch(inner(music_hoard(COLLECTION.clone())), fetch); + + let app = app.abort(); + assert!(matches!(app, AppState::Browse(_))); + } +} diff --git a/src/tui/app/machine/info.rs b/src/tui/app/machine/info.rs index 6dd90b6..06ddd0b 100644 --- a/src/tui/app/machine/info.rs +++ b/src/tui/app/machine/info.rs @@ -35,10 +35,6 @@ impl IAppInteractInfo for AppMachine { fn hide_info_overlay(self) -> Self::APP { AppMachine::browse(self.inner).into() } - - fn no_op(self) -> Self::APP { - self.into() - } } #[cfg(test)] @@ -53,11 +49,4 @@ mod tests { let app = info.hide_info_overlay(); app.unwrap_browse(); } - - #[test] - fn no_op() { - let info = AppMachine::info(inner(music_hoard(vec![]))); - let app = info.no_op(); - app.unwrap_info(); - } } diff --git a/src/tui/app/machine/matches.rs b/src/tui/app/machine/matches.rs index d3ff9ab..4dbcab3 100644 --- a/src/tui/app/machine/matches.rs +++ b/src/tui/app/machine/matches.rs @@ -1,11 +1,13 @@ use std::cmp; use crate::tui::app::{ - machine::{browse::FetchReceiver, App, AppInner, AppMachine}, + machine::{App, AppInner, AppMachine}, AppAlbumMatches, AppArtistMatches, AppMatchesInfo, AppPublic, AppPublicMatches, AppState, IAppInteractMatches, MatchOption, WidgetState, }; +use super::fetch::AppFetch; + impl AppArtistMatches { fn len(&self) -> usize { self.list.len() @@ -34,7 +36,7 @@ impl AppMatchesInfo { } } - fn push_cannot_have_mbid(&mut self) { + pub fn push_cannot_have_mbid(&mut self) { match self { Self::Artist(a) => a.push_cannot_have_mbid(), Self::Album(a) => a.push_cannot_have_mbid(), @@ -43,29 +45,30 @@ impl AppMatchesInfo { } pub struct AppMatches { - matches_rx: FetchReceiver, current: Option, state: WidgetState, + fetch: AppFetch, } impl AppMatches { - fn empty(matches_rx: FetchReceiver) -> Self { + pub fn new(mut current: Option, fetch: AppFetch) -> Self { + let mut state = WidgetState::default(); + if let Some(ref mut current) = current { + state.list.select(Some(0)); + current.push_cannot_have_mbid(); + } AppMatches { - matches_rx, - current: None, - state: WidgetState::default(), + current, + state, + fetch, } } } impl AppMachine { - fn matches(inner: AppInner, state: AppMatches) -> Self { + pub fn matches(inner: AppInner, state: AppMatches) -> Self { AppMachine { inner, state } } - - pub fn app_matches(inner: AppInner, matches_rx: FetchReceiver) -> App { - AppMachine::matches(inner, AppMatches::empty(matches_rx)).fetch_first() - } } impl From> for App { @@ -113,57 +116,12 @@ impl IAppInteractMatches for AppMachine { } fn select(self) -> Self::APP { - self.fetch_next() + AppMachine::app_fetch_next(self.inner, self.state.fetch) } fn abort(self) -> Self::APP { AppMachine::browse(self.inner).into() } - - fn no_op(self) -> Self::APP { - self.into() - } -} - -trait IAppInteractMatchesPrivate -where - Self: Sized, -{ - fn fetch_first(self) -> App; - fn fetch_next(self) -> App; - fn fetch(self, first: bool) -> App; -} - -impl IAppInteractMatchesPrivate for AppMachine { - fn fetch_first(self) -> App { - self.fetch(true) - } - - fn fetch_next(self) -> App { - self.fetch(false) - } - - fn fetch(mut self, first: bool) -> App { - match self.state.matches_rx.recv() { - Ok(fetch_result) => match fetch_result { - Ok(mut next_match) => { - next_match.push_cannot_have_mbid(); - self.state.current = Some(next_match); - self.state.state.list.select(Some(0)); - AppMachine::matches(self.inner, self.state).into() - } - Err(err) => AppMachine::error(self.inner, format!("fetch failed: {err}")).into(), - }, - // only happens when the sender disconnects which means it finished its job - Err(_) => { - if first { - AppMachine::matches(self.inner, AppMatches::empty(self.state.matches_rx)).into() - } else { - AppMachine::browse(self.inner).into() - } - } - } - } } #[cfg(test)] @@ -195,90 +153,59 @@ mod tests { } } - fn artist_matches_info_vec() -> Vec { - let artist_1 = ArtistMeta::new(ArtistId::new("Artist 1")); + fn artist_match() -> AppMatchesInfo { + let artist = ArtistMeta::new(ArtistId::new("Artist")); - let artist_1_1 = artist_1.clone(); - let artist_match_1_1 = Match::new(100, artist_1_1); + let artist_1 = artist.clone(); + let artist_match_1 = Match::new(100, artist_1); - let artist_1_2 = artist_1.clone(); - let mut artist_match_1_2 = Match::new(100, artist_1_2); - artist_match_1_2.disambiguation = Some(String::from("some disambiguation")); + let artist_2 = artist.clone(); + let mut artist_match_2 = Match::new(100, artist_2); + artist_match_2.disambiguation = Some(String::from("some disambiguation")); - let list = vec![artist_match_1_1.clone(), artist_match_1_2.clone()]; - let matches_info_1 = AppMatchesInfo::artist(artist_1.clone(), list); - - let artist_2 = ArtistMeta::new(ArtistId::new("Artist 2")); - - let artist_2_1 = artist_1.clone(); - let album_match_2_1 = Match::new(100, artist_2_1); - - let list = vec![album_match_2_1.clone()]; - let matches_info_2 = AppMatchesInfo::artist(artist_2.clone(), list); - - vec![matches_info_1, matches_info_2] + let list = vec![artist_match_1.clone(), artist_match_2.clone()]; + AppMatchesInfo::artist(artist, list) } - fn album_matches_info_vec() -> Vec { - let album_1 = AlbumMeta::new( - AlbumId::new("Album 1"), + fn album_match() -> AppMatchesInfo { + let album = AlbumMeta::new( + AlbumId::new("Album"), AlbumDate::new(Some(1990), Some(5), None), Some(AlbumPrimaryType::Album), vec![AlbumSecondaryType::Live, AlbumSecondaryType::Compilation], ); - let album_1_1 = album_1.clone(); - let album_match_1_1 = Match::new(100, album_1_1); + let album_1 = album.clone(); + let album_match_1 = Match::new(100, album_1); - let mut album_1_2 = album_1.clone(); - album_1_2.id.title.push_str(" extra title part"); - album_1_2.secondary_types.pop(); - let album_match_1_2 = Match::new(100, album_1_2); + let mut album_2 = album.clone(); + album_2.id.title.push_str(" extra title part"); + album_2.secondary_types.pop(); + let album_match_2 = Match::new(100, album_2); - let list = vec![album_match_1_1.clone(), album_match_1_2.clone()]; - let matches_info_1 = AppMatchesInfo::album(album_1.clone(), list); - - let album_2 = AlbumMeta::new( - AlbumId::new("Album 2"), - AlbumDate::new(Some(2001), None, None), - Some(AlbumPrimaryType::Album), - vec![], - ); - - let album_2_1 = album_1.clone(); - let album_match_2_1 = Match::new(100, album_2_1); - - let list = vec![album_match_2_1.clone()]; - let matches_info_2 = AppMatchesInfo::album(album_2.clone(), list); - - vec![matches_info_1, matches_info_2] + let list = vec![album_match_1.clone(), album_match_2.clone()]; + AppMatchesInfo::album(album, list) } - fn receiver(matches_info_vec: Vec) -> FetchReceiver { - let (tx, rx) = mpsc::channel(); - for matches_info in matches_info_vec.into_iter() { - tx.send(Ok(matches_info)).unwrap(); - } - rx + fn fetch() -> AppFetch { + let (_, rx) = mpsc::channel(); + AppFetch::new(rx) } - fn push_cannot_have_mbid(matches_info_vec: &mut [AppMatchesInfo]) { - for matches_info in matches_info_vec.iter_mut() { - matches_info.push_cannot_have_mbid(); - } + fn matches(matches_info: Option) -> AppMatches { + AppMatches::new(matches_info, fetch()) } #[test] fn create_empty() { - let matches = - AppMachine::app_matches(inner(music_hoard(vec![])), receiver(vec![])).unwrap_matches(); + let matches = AppMachine::matches(inner(music_hoard(vec![])), matches(None)); let widget_state = WidgetState::default(); assert_eq!(matches.state.current, None); assert_eq!(matches.state.state, widget_state); - let mut app = matches.no_op(); + let mut app: App = matches.into(); let public = app.get(); let public_matches = public.state.unwrap_matches(); @@ -288,96 +215,90 @@ mod tests { #[test] fn create_nonempty() { - let mut matches_info_vec = album_matches_info_vec(); - let matches = AppMachine::app_matches( + let mut album_match = album_match(); + let matches = AppMachine::matches( inner(music_hoard(vec![])), - receiver(matches_info_vec.clone()), - ) - .unwrap_matches(); - push_cannot_have_mbid(&mut matches_info_vec); + matches(Some(album_match.clone())), + ); + album_match.push_cannot_have_mbid(); let mut widget_state = WidgetState::default(); widget_state.list.select(Some(0)); - assert_eq!(matches.state.current.as_ref(), Some(&matches_info_vec[0])); + assert_eq!(matches.state.current.as_ref(), Some(&album_match)); assert_eq!(matches.state.state, widget_state); - let mut app = matches.no_op(); + let mut app: App = matches.into(); let public = app.get(); let public_matches = public.state.unwrap_matches(); - assert_eq!(public_matches.matches, Some(&matches_info_vec[0])); + assert_eq!(public_matches.matches, Some(&album_match)); assert_eq!(public_matches.state, &widget_state); } - fn matches_flow(mut matches_info_vec: Vec) { - let matches = AppMachine::app_matches( - inner(music_hoard(vec![])), - receiver(matches_info_vec.clone()), - ) - .unwrap_matches(); - push_cannot_have_mbid(&mut matches_info_vec); + fn matches_flow(mut matches_info: AppMatchesInfo) { + // tx must exist for rx to return Empty rather than Disconnected. + #[allow(unused_variables)] + let (tx, rx) = mpsc::channel(); + let app_matches = AppMatches::new(Some(matches_info.clone()), AppFetch::new(rx)); + + let matches = AppMachine::matches(inner(music_hoard(vec![])), app_matches); + matches_info.push_cannot_have_mbid(); let mut widget_state = WidgetState::default(); widget_state.list.select(Some(0)); - assert_eq!(matches.state.current.as_ref(), Some(&matches_info_vec[0])); + assert_eq!(matches.state.current.as_ref(), Some(&matches_info)); assert_eq!(matches.state.state, widget_state); let matches = matches.prev_match().unwrap_matches(); - assert_eq!(matches.state.current.as_ref(), Some(&matches_info_vec[0])); + assert_eq!(matches.state.current.as_ref(), Some(&matches_info)); assert_eq!(matches.state.state.list.selected(), Some(0)); let matches = matches.next_match().unwrap_matches(); - assert_eq!(matches.state.current.as_ref(), Some(&matches_info_vec[0])); + assert_eq!(matches.state.current.as_ref(), Some(&matches_info)); assert_eq!(matches.state.state.list.selected(), Some(1)); // Next is CannotHaveMBID let matches = matches.next_match().unwrap_matches(); - assert_eq!(matches.state.current.as_ref(), Some(&matches_info_vec[0])); + assert_eq!(matches.state.current.as_ref(), Some(&matches_info)); assert_eq!(matches.state.state.list.selected(), Some(2)); let matches = matches.next_match().unwrap_matches(); - assert_eq!(matches.state.current.as_ref(), Some(&matches_info_vec[0])); + assert_eq!(matches.state.current.as_ref(), Some(&matches_info)); assert_eq!(matches.state.state.list.selected(), Some(2)); - let matches = matches.select().unwrap_matches(); - - assert_eq!(matches.state.current.as_ref(), Some(&matches_info_vec[1])); - assert_eq!(matches.state.state.list.selected(), Some(0)); - // And it's done - matches.select().unwrap_browse(); + matches.select().unwrap_fetch(); } #[test] fn artist_matches_flow() { - matches_flow(artist_matches_info_vec()); + matches_flow(artist_match()); } #[test] fn album_matches_flow() { - matches_flow(album_matches_info_vec()); + matches_flow(album_match()); } #[test] fn matches_abort() { - let mut matches_info_vec = album_matches_info_vec(); - let matches = AppMachine::app_matches( + let mut album_match = album_match(); + let matches = AppMachine::matches( inner(music_hoard(vec![])), - receiver(matches_info_vec.clone()), - ) - .unwrap_matches(); - push_cannot_have_mbid(&mut matches_info_vec); + matches(Some(album_match.clone())), + ); + album_match.push_cannot_have_mbid(); let mut widget_state = WidgetState::default(); widget_state.list.select(Some(0)); - assert_eq!(matches.state.current.as_ref(), Some(&matches_info_vec[0])); + assert_eq!(matches.state.current.as_ref(), Some(&album_match)); assert_eq!(matches.state.state, widget_state); matches.abort().unwrap_browse(); @@ -385,19 +306,9 @@ mod tests { #[test] fn matches_select_empty() { - let matches = - AppMachine::app_matches(inner(music_hoard(vec![])), receiver(vec![])).unwrap_matches(); - - assert_eq!(matches.state.current, None); - + // Note that what really matters in this test is actually that the transmit channel has + // disconnected and so the receive within AppFetch concludes there are no more matches. + let matches = AppMachine::matches(inner(music_hoard(vec![])), matches(None)); matches.select().unwrap_browse(); } - - #[test] - fn no_op() { - let matches = - AppMachine::app_matches(inner(music_hoard(vec![])), receiver(vec![])).unwrap_matches(); - let app = matches.no_op(); - app.unwrap_matches(); - } } diff --git a/src/tui/app/machine/mod.rs b/src/tui/app/machine/mod.rs index 6aa186f..70ae5b4 100644 --- a/src/tui/app/machine/mod.rs +++ b/src/tui/app/machine/mod.rs @@ -1,6 +1,7 @@ mod browse; mod critical; mod error; +mod fetch; mod info; mod matches; mod reload; @@ -10,22 +11,27 @@ use std::sync::{Arc, Mutex}; use crate::tui::{ app::{selection::Selection, AppPublic, AppPublicInner, AppState, IAppAccess, IAppInteract}, + event::EventSender, lib::{interface::musicbrainz::IMusicBrainz, IMusicHoard}, }; use browse::AppBrowse; use critical::AppCritical; use error::AppError; +use fetch::AppFetch; use info::AppInfo; use matches::AppMatches; use reload::AppReload; use search::AppSearch; +use super::IAppBase; + pub type App = AppState< AppMachine, AppMachine, AppMachine, AppMachine, + AppMachine, AppMachine, AppMachine, AppMachine, @@ -41,15 +47,17 @@ pub struct AppInner { music_hoard: Box, musicbrainz: Arc>, selection: Selection, + events: EventSender, } impl App { pub fn new( mut music_hoard: MH, musicbrainz: MB, + events: EventSender, ) -> Self { let init_result = Self::init(&mut music_hoard); - let inner = AppInner::new(music_hoard, musicbrainz); + let inner = AppInner::new(music_hoard, musicbrainz, events); match init_result { Ok(()) => AppMachine::browse(inner).into(), Err(err) => AppMachine::critical(inner, err.to_string()).into(), @@ -65,9 +73,10 @@ impl App { match self { AppState::Browse(browse) => &browse.inner, AppState::Info(info) => &info.inner, - AppState::Matches(matches) => &matches.inner, AppState::Reload(reload) => &reload.inner, AppState::Search(search) => &search.inner, + AppState::Fetch(fetch) => &fetch.inner, + AppState::Matches(matches) => &matches.inner, AppState::Error(error) => &error.inner, AppState::Critical(critical) => &critical.inner, } @@ -77,9 +86,10 @@ impl App { match self { AppState::Browse(browse) => &mut browse.inner, AppState::Info(info) => &mut info.inner, - AppState::Matches(matches) => &mut matches.inner, AppState::Reload(reload) => &mut reload.inner, AppState::Search(search) => &mut search.inner, + AppState::Fetch(fetch) => &mut fetch.inner, + AppState::Matches(matches) => &mut matches.inner, AppState::Error(error) => &mut error.inner, AppState::Critical(critical) => &mut critical.inner, } @@ -91,6 +101,7 @@ impl IAppInteract for App { type IS = AppMachine; type RS = AppMachine; type SS = AppMachine; + type FS = AppMachine; type MS = AppMachine; type ES = AppMachine; type CS = AppMachine; @@ -106,19 +117,29 @@ impl IAppInteract for App { fn state( self, - ) -> AppState { + ) -> AppState + { self } } +impl> IAppBase for T { + type APP = App; + + fn no_op(self) -> Self::APP { + self.into() + } +} + impl IAppAccess for App { fn get(&mut self) -> AppPublic { match self { AppState::Browse(browse) => browse.into(), AppState::Info(info) => info.into(), - AppState::Matches(matches) => matches.into(), AppState::Reload(reload) => reload.into(), AppState::Search(search) => search.into(), + AppState::Fetch(fetch) => fetch.into(), + AppState::Matches(matches) => matches.into(), AppState::Error(error) => error.into(), AppState::Critical(critical) => critical.into(), } @@ -129,6 +150,7 @@ impl AppInner { pub fn new( music_hoard: MH, musicbrainz: MB, + events: EventSender, ) -> Self { let selection = Selection::new(music_hoard.get_collection()); AppInner { @@ -136,6 +158,7 @@ impl AppInner { music_hoard: Box::new(music_hoard), musicbrainz: Arc::new(Mutex::new(musicbrainz)), selection, + events, } } } @@ -158,11 +181,12 @@ mod tests { use crate::tui::{ app::{AppState, IAppInteract, IAppInteractBrowse}, lib::{interface::musicbrainz::MockIMusicBrainz, MockIMusicHoard}, + EventChannel, }; use super::*; - impl AppState { + impl AppState { pub fn unwrap_browse(self) -> BS { match self { AppState::Browse(browse) => browse, @@ -191,6 +215,13 @@ mod tests { } } + pub fn unwrap_fetch(self) -> FS { + match self { + AppState::Fetch(fetch) => fetch, + _ => panic!(), + } + } + pub fn unwrap_matches(self) -> MS { match self { AppState::Matches(matches) => matches, @@ -235,23 +266,32 @@ mod tests { MockIMusicBrainz::new() } + fn events() -> EventSender { + EventChannel::new().sender() + } + pub fn inner(music_hoard: MockIMusicHoard) -> AppInner { - AppInner::new(music_hoard, mb_api()) + AppInner::new(music_hoard, mb_api(), events()) } pub fn inner_with_mb(music_hoard: MockIMusicHoard, mb_api: MockIMusicBrainz) -> AppInner { - AppInner::new(music_hoard, mb_api) + AppInner::new(music_hoard, mb_api, events()) } #[test] fn state_browse() { - let mut app = App::new(music_hoard_init(vec![]), mb_api()); + let mut app = App::new(music_hoard_init(vec![]), mb_api(), events()); assert!(app.is_running()); let state = app.state(); assert!(matches!(state, AppState::Browse(_))); app = state; + app = app.no_op(); + let state = app.state(); + assert!(matches!(state, AppState::Browse(_))); + app = state; + let public = app.get(); assert!(matches!(public.state, AppState::Browse(_))); @@ -261,7 +301,7 @@ mod tests { #[test] fn state_info() { - let mut app = App::new(music_hoard_init(vec![]), mb_api()); + let mut app = App::new(music_hoard_init(vec![]), mb_api(), events()); assert!(app.is_running()); app = app.unwrap_browse().show_info_overlay(); @@ -270,6 +310,11 @@ mod tests { assert!(matches!(state, AppState::Info(_))); app = state; + app = app.no_op(); + let state = app.state(); + assert!(matches!(state, AppState::Info(_))); + app = state; + let public = app.get(); assert!(matches!(public.state, AppState::Info(_))); @@ -279,7 +324,7 @@ mod tests { #[test] fn state_reload() { - let mut app = App::new(music_hoard_init(vec![]), mb_api()); + let mut app = App::new(music_hoard_init(vec![]), mb_api(), events()); assert!(app.is_running()); app = app.unwrap_browse().show_reload_menu(); @@ -288,6 +333,11 @@ mod tests { assert!(matches!(state, AppState::Reload(_))); app = state; + app = app.no_op(); + let state = app.state(); + assert!(matches!(state, AppState::Reload(_))); + app = state; + let public = app.get(); assert!(matches!(public.state, AppState::Reload(_))); @@ -297,7 +347,7 @@ mod tests { #[test] fn state_search() { - let mut app = App::new(music_hoard_init(vec![]), mb_api()); + let mut app = App::new(music_hoard_init(vec![]), mb_api(), events()); assert!(app.is_running()); app = app.unwrap_browse().begin_search(); @@ -306,6 +356,11 @@ mod tests { assert!(matches!(state, AppState::Search(_))); app = state; + app = app.no_op(); + let state = app.state(); + assert!(matches!(state, AppState::Search(_))); + app = state; + let public = app.get(); assert!(matches!(public.state, AppState::Search(""))); @@ -314,13 +369,45 @@ mod tests { } #[test] - fn state_matches() { - let mut app = App::new(music_hoard_init(vec![]), mb_api()); + fn state_fetch() { + let mut app = App::new(music_hoard_init(vec![]), mb_api(), events()); assert!(app.is_running()); let (_, rx) = mpsc::channel(); - app = AppMachine::app_matches(app.unwrap_browse().inner, rx); + let inner = app.unwrap_browse().inner; + let state = AppFetch::new(rx); + app = AppMachine { inner, state }.into(); + let state = app.state(); + assert!(matches!(state, AppState::Fetch(_))); + app = state; + + app = app.no_op(); + let state = app.state(); + assert!(matches!(state, AppState::Fetch(_))); + app = state; + + let public = app.get(); + assert!(matches!(public.state, AppState::Fetch(_))); + + let app = app.force_quit(); + assert!(!app.is_running()); + } + + #[test] + fn state_matches() { + let mut app = App::new(music_hoard_init(vec![]), mb_api(), events()); + assert!(app.is_running()); + + let (_, rx) = mpsc::channel(); + let fetch = AppFetch::new(rx); + app = AppMachine::matches(app.unwrap_browse().inner, AppMatches::new(None, fetch)).into(); + + let state = app.state(); + assert!(matches!(state, AppState::Matches(_))); + app = state; + + app = app.no_op(); let state = app.state(); assert!(matches!(state, AppState::Matches(_))); app = state; @@ -334,7 +421,7 @@ mod tests { #[test] fn state_error() { - let mut app = App::new(music_hoard_init(vec![]), mb_api()); + let mut app = App::new(music_hoard_init(vec![]), mb_api(), events()); assert!(app.is_running()); app = AppMachine::error(app.unwrap_browse().inner, "get rekt").into(); @@ -343,6 +430,11 @@ mod tests { assert!(matches!(state, AppState::Error(_))); app = state; + app = app.no_op(); + let state = app.state(); + assert!(matches!(state, AppState::Error(_))); + app = state; + let public = app.get(); assert!(matches!(public.state, AppState::Error("get rekt"))); @@ -352,7 +444,7 @@ mod tests { #[test] fn state_critical() { - let mut app = App::new(music_hoard_init(vec![]), mb_api()); + let mut app = App::new(music_hoard_init(vec![]), mb_api(), events()); assert!(app.is_running()); app = AppMachine::critical(app.unwrap_browse().inner, "get rekt").into(); @@ -361,6 +453,11 @@ mod tests { assert!(matches!(state, AppState::Critical(_))); app = state; + app = app.no_op(); + let state = app.state(); + assert!(matches!(state, AppState::Critical(_))); + app = state; + let public = app.get(); assert!(matches!(public.state, AppState::Critical("get rekt"))); @@ -378,7 +475,7 @@ mod tests { .return_once(|| Err(musichoard::Error::LibraryError(String::from("get rekt")))); music_hoard.expect_get_collection().return_const(vec![]); - let app = App::new(music_hoard, mb_api()); + let app = App::new(music_hoard, mb_api(), events()); assert!(app.is_running()); app.unwrap_critical(); } diff --git a/src/tui/app/machine/reload.rs b/src/tui/app/machine/reload.rs index 6659434..41758d2 100644 --- a/src/tui/app/machine/reload.rs +++ b/src/tui/app/machine/reload.rs @@ -53,10 +53,6 @@ impl IAppInteractReload for AppMachine { fn hide_reload_menu(self) -> Self::APP { AppMachine::browse(self.inner).into() } - - fn no_op(self) -> Self::APP { - self.into() - } } trait IAppInteractReloadPrivate { @@ -131,11 +127,4 @@ mod tests { let app = reload.reload_database(); app.unwrap_error(); } - - #[test] - fn no_op() { - let reload = AppMachine::reload(inner(music_hoard(vec![]))); - let app = reload.no_op(); - app.unwrap_reload(); - } } diff --git a/src/tui/app/machine/search.rs b/src/tui/app/machine/search.rs index 17ed189..3a484b5 100644 --- a/src/tui/app/machine/search.rs +++ b/src/tui/app/machine/search.rs @@ -98,10 +98,6 @@ impl IAppInteractSearch for AppMachine { self.inner.selection.select_by_list(self.state.orig); AppMachine::browse(self.inner).into() } - - fn no_op(self) -> Self::APP { - self.into() - } } trait IAppInteractSearchPrivate { @@ -544,13 +540,6 @@ mod tests { let browse = search.cancel_search().unwrap_browse(); assert_eq!(browse.inner.selection.selected(), None); } - - #[test] - fn no_op() { - let search = AppMachine::search(inner(music_hoard(vec![])), orig(None)); - let app = search.no_op(); - app.unwrap_search(); - } } #[cfg(nightly)] diff --git a/src/tui/app/mod.rs b/src/tui/app/mod.rs index 4d14aef..866f9a0 100644 --- a/src/tui/app/mod.rs +++ b/src/tui/app/mod.rs @@ -8,24 +8,26 @@ use musichoard::collection::{album::AlbumMeta, artist::ArtistMeta, Collection}; use crate::tui::lib::interface::musicbrainz::Match; -pub enum AppState { +pub enum AppState { Browse(BS), Info(IS), Reload(RS), Search(SS), + Fetch(FS), Matches(MS), Error(ES), Critical(CS), } pub trait IAppInteract { - type BS: IAppInteractBrowse; - type IS: IAppInteractInfo; - type RS: IAppInteractReload; - type SS: IAppInteractSearch; - type MS: IAppInteractMatches; - type ES: IAppInteractError; - type CS: IAppInteractCritical; + type BS: IAppBase + IAppInteractBrowse; + type IS: IAppBase + IAppInteractInfo; + type RS: IAppBase + IAppInteractReload; + type SS: IAppBase + IAppInteractSearch; + type FS: IAppBase + IAppInteractFetch + IAppEventFetch; + type MS: IAppBase + IAppInteractMatches; + type ES: IAppBase + IAppInteractError; + type CS: IAppBase; fn is_running(&self) -> bool; fn force_quit(self) -> Self; @@ -33,7 +35,13 @@ pub trait IAppInteract { #[allow(clippy::type_complexity)] fn state( self, - ) -> AppState; + ) -> AppState; +} + +pub trait IAppBase { + type APP: IAppInteract; + + fn no_op(self) -> Self::APP; } pub trait IAppInteractBrowse { @@ -53,16 +61,12 @@ pub trait IAppInteractBrowse { fn begin_search(self) -> Self::APP; fn fetch_musicbrainz(self) -> Self::APP; - - fn no_op(self) -> Self::APP; } pub trait IAppInteractInfo { type APP: IAppInteract; fn hide_info_overlay(self) -> Self::APP; - - fn no_op(self) -> Self::APP; } pub trait IAppInteractReload { @@ -71,8 +75,6 @@ pub trait IAppInteractReload { fn reload_library(self) -> Self::APP; fn reload_database(self) -> Self::APP; fn hide_reload_menu(self) -> Self::APP; - - fn no_op(self) -> Self::APP; } pub trait IAppInteractSearch { @@ -83,8 +85,18 @@ pub trait IAppInteractSearch { fn step_back(self) -> Self::APP; fn finish_search(self) -> Self::APP; fn cancel_search(self) -> Self::APP; +} - fn no_op(self) -> Self::APP; +pub trait IAppInteractFetch { + type APP: IAppInteract; + + fn abort(self) -> Self::APP; +} + +pub trait IAppEventFetch { + type APP: IAppInteract; + + fn fetch_result_ready(self) -> Self::APP; } pub trait IAppInteractMatches { @@ -95,8 +107,6 @@ pub trait IAppInteractMatches { fn select(self) -> Self::APP; fn abort(self) -> Self::APP; - - fn no_op(self) -> Self::APP; } pub trait IAppInteractError { @@ -105,12 +115,6 @@ pub trait IAppInteractError { fn dismiss_error(self) -> Self::APP; } -pub trait IAppInteractCritical { - type APP: IAppInteract; - - fn no_op(self) -> Self::APP; -} - // It would be preferable to have a getter for each field separately. However, the selection field // needs to be mutably accessible requiring a mutable borrow of the entire struct if behind a trait. // This in turn complicates simultaneous field access since only a single mutable borrow is allowed. @@ -177,9 +181,9 @@ pub struct AppPublicMatches<'app> { } pub type AppPublicState<'app> = - AppState<(), (), (), &'app str, AppPublicMatches<'app>, &'app str, &'app str>; + AppState<(), (), (), &'app str, (), AppPublicMatches<'app>, &'app str, &'app str>; -impl AppState { +impl AppState { pub fn is_search(&self) -> bool { matches!(self, AppState::Search(_)) } diff --git a/src/tui/event.rs b/src/tui/event.rs index 8143c29..eaa90e1 100644 --- a/src/tui/event.rs +++ b/src/tui/event.rs @@ -33,9 +33,16 @@ impl From for EventError { } } -#[derive(Clone, Copy, Debug)] +impl From for EventError { + fn from(_: mpsc::TryRecvError) -> EventError { + EventError::Recv + } +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum Event { Key(KeyEvent), + FetchResultReady, } pub struct EventChannel { @@ -43,6 +50,7 @@ pub struct EventChannel { receiver: mpsc::Receiver, } +#[derive(Clone)] pub struct EventSender { sender: mpsc::Sender, } @@ -80,6 +88,11 @@ impl EventReceiver { pub fn recv(&self) -> Result { Ok(self.receiver.recv()?) } + + #[cfg(test)] + pub fn try_recv(&self) -> Result { + Ok(self.receiver.try_recv()?) + } } #[cfg(test)] @@ -121,6 +134,25 @@ mod tests { assert!(result.is_err()); } + #[test] + fn event_receiver_try() { + let channel = EventChannel::new(); + let sender = channel.sender(); + let receiver = channel.receiver(); + let event = Event::FetchResultReady; + + let result = receiver.try_recv(); + assert!(result.is_err()); + + sender.send(event).unwrap(); + let result = receiver.try_recv(); + assert!(result.is_ok()); + + drop(sender); + let result = receiver.try_recv(); + assert!(result.is_err()); + } + #[test] fn errors() { let send_err = EventError::Send(Event::Key(KeyEvent { diff --git a/src/tui/handler.rs b/src/tui/handler.rs index 13f4ec1..513a9fd 100644 --- a/src/tui/handler.rs +++ b/src/tui/handler.rs @@ -5,12 +5,14 @@ use mockall::automock; use crate::tui::{ app::{ - AppState, Delta, IAppInteract, IAppInteractBrowse, IAppInteractCritical, IAppInteractError, + AppState, Delta, IAppInteract, IAppInteractBrowse, IAppInteractError, IAppInteractFetch, IAppInteractInfo, IAppInteractMatches, IAppInteractReload, IAppInteractSearch, }, event::{Event, EventError, EventReceiver}, }; +use super::app::{IAppBase, IAppEventFetch}; + #[cfg_attr(test, automock)] pub trait IEventHandler { fn handle_next_event(&self, app: APP) -> Result; @@ -22,9 +24,12 @@ trait IEventHandlerPrivate { fn handle_info_key_event(app: ::IS, key_event: KeyEvent) -> APP; fn handle_reload_key_event(app: ::RS, key_event: KeyEvent) -> APP; fn handle_search_key_event(app: ::SS, key_event: KeyEvent) -> APP; + fn handle_fetch_key_event(app: ::FS, key_event: KeyEvent) -> APP; fn handle_matches_key_event(app: ::MS, key_event: KeyEvent) -> APP; fn handle_error_key_event(app: ::ES, key_event: KeyEvent) -> APP; fn handle_critical_key_event(app: ::CS, key_event: KeyEvent) -> APP; + + fn handle_fetch_result_ready_event(app: APP) -> APP; } pub struct EventHandler { @@ -39,11 +44,11 @@ impl EventHandler { } impl IEventHandler for EventHandler { - fn handle_next_event(&self, mut app: APP) -> Result { - match self.events.recv()? { - Event::Key(key_event) => app = Self::handle_key_event(app, key_event), - }; - Ok(app) + fn handle_next_event(&self, app: APP) -> Result { + Ok(match self.events.recv()? { + Event::Key(key_event) => Self::handle_key_event(app, key_event), + Event::FetchResultReady => Self::handle_fetch_result_ready_event(app), + }) } } @@ -70,6 +75,9 @@ impl IEventHandlerPrivate for EventHandler { AppState::Search(search) => { >::handle_search_key_event(search, key_event) } + AppState::Fetch(fetch) => { + >::handle_fetch_key_event(fetch, key_event) + } AppState::Matches(matches) => { >::handle_matches_key_event(matches, key_event) } @@ -82,6 +90,19 @@ impl IEventHandlerPrivate for EventHandler { } } + fn handle_fetch_result_ready_event(app: APP) -> APP { + match app.state() { + AppState::Browse(browse) => browse.no_op(), + AppState::Info(info) => info.no_op(), + AppState::Reload(reload) => reload.no_op(), + AppState::Search(search) => search.no_op(), + AppState::Fetch(fetch) => fetch.fetch_result_ready(), + AppState::Matches(matches) => matches.no_op(), + AppState::Error(error) => error.no_op(), + AppState::Critical(critical) => critical.no_op(), + } + } + fn handle_browse_key_event(app: ::BS, key_event: KeyEvent) -> APP { match key_event.code { // Exit application on `ESC` or `q`. @@ -161,6 +182,15 @@ impl IEventHandlerPrivate for EventHandler { } } + fn handle_fetch_key_event(app: ::FS, key_event: KeyEvent) -> APP { + match key_event.code { + // Abort. + KeyCode::Esc | KeyCode::Char('q') | KeyCode::Char('Q') => app.abort(), + // Othey keys. + _ => app.no_op(), + } + } + fn handle_matches_key_event(app: ::MS, key_event: KeyEvent) -> APP { match key_event.code { // Abort. diff --git a/src/tui/mod.rs b/src/tui/mod.rs index 131ebb7..d1ce805 100644 --- a/src/tui/mod.rs +++ b/src/tui/mod.rs @@ -174,6 +174,7 @@ mod testmod; mod tests { use std::{io, thread}; + use event::EventSender; use lib::interface::musicbrainz::MockIMusicBrainz; use ratatui::{backend::TestBackend, Terminal}; @@ -202,8 +203,12 @@ mod tests { music_hoard } + fn events() -> EventSender { + EventChannel::new().sender() + } + fn app(collection: Collection) -> App { - App::new(music_hoard(collection), MockIMusicBrainz::new()) + App::new(music_hoard(collection), MockIMusicBrainz::new(), events()) } fn listener() -> MockIEventListener { diff --git a/src/tui/ui/fetch.rs b/src/tui/ui/fetch.rs new file mode 100644 index 0000000..063b059 --- /dev/null +++ b/src/tui/ui/fetch.rs @@ -0,0 +1,9 @@ +use ratatui::widgets::Paragraph; + +pub struct FetchOverlay; + +impl FetchOverlay { + pub fn paragraph<'a>() -> Paragraph<'a> { + Paragraph::new(" -- fetching --") + } +} diff --git a/src/tui/ui/minibuffer.rs b/src/tui/ui/minibuffer.rs index 3e6e0f1..1d92c40 100644 --- a/src/tui/ui/minibuffer.rs +++ b/src/tui/ui/minibuffer.rs @@ -27,7 +27,7 @@ impl Minibuffer<'_> { pub fn new(state: &AppPublicState) -> Self { let columns = 3; let mut mb = match state { - AppState::Browse(_) => Minibuffer { + AppState::Browse(()) => Minibuffer { paragraphs: vec![ Paragraph::new("m: show info overlay"), Paragraph::new("g: show reload menu"), @@ -36,11 +36,11 @@ impl Minibuffer<'_> { ], columns, }, - AppState::Info(_) => Minibuffer { + AppState::Info(()) => Minibuffer { paragraphs: vec![Paragraph::new("m: hide info overlay")], columns, }, - AppState::Reload(_) => Minibuffer { + AppState::Reload(()) => Minibuffer { paragraphs: vec![ Paragraph::new("g: hide reload menu"), Paragraph::new("d: reload database"), @@ -51,12 +51,15 @@ impl Minibuffer<'_> { AppState::Search(ref s) => Minibuffer { paragraphs: vec![ Paragraph::new(format!("I-search: {s}")), - Paragraph::new("ctrl+s: search next".to_string()).alignment(Alignment::Center), - Paragraph::new("ctrl+g: cancel search".to_string()) - .alignment(Alignment::Center), + Paragraph::new("ctrl+s: search next").alignment(Alignment::Center), + Paragraph::new("ctrl+g: cancel search").alignment(Alignment::Center), ], columns, }, + AppState::Fetch(()) => Minibuffer { + paragraphs: vec![Paragraph::new("fetching..."), Paragraph::new("q: abort")], + columns: 2, + }, AppState::Matches(public) => Minibuffer { paragraphs: vec![ Paragraph::new(UiDisplay::display_matching_info(public.matches)), diff --git a/src/tui/ui/mod.rs b/src/tui/ui/mod.rs index 3855b7b..b40d809 100644 --- a/src/tui/ui/mod.rs +++ b/src/tui/ui/mod.rs @@ -1,6 +1,7 @@ mod browse; mod display; mod error; +mod fetch; mod info; mod matches; mod minibuffer; @@ -9,6 +10,7 @@ mod reload; mod style; mod widgets; +use fetch::FetchOverlay; use ratatui::{layout::Rect, widgets::Paragraph, Frame}; use musichoard::collection::{album::Album, Collection}; @@ -125,12 +127,16 @@ impl Ui { .with_width(OverlaySize::Value(39)) .with_height(OverlaySize::Value(4)) .build(frame.size()); - let reload_text = ReloadOverlay::paragraph(); - UiWidget::render_overlay_widget("Reload", reload_text, area, false, frame); } + fn render_fetch_overlay(frame: &mut Frame) { + let area = OverlayBuilder::default().build(frame.size()); + let fetch_text = FetchOverlay::paragraph(); + UiWidget::render_overlay_widget("Fetching", fetch_text, area, false, frame) + } + fn render_matches_overlay( matches: Option<&AppMatchesInfo>, state: &mut WidgetState, @@ -145,9 +151,7 @@ impl Ui { let area = OverlayBuilder::default() .with_height(OverlaySize::Value(4)) .build(frame.size()); - let error_text = ErrorOverlay::paragraph(msg.as_ref()); - UiWidget::render_overlay_widget(title.as_ref(), error_text, area, true, frame); } } @@ -162,11 +166,12 @@ impl IUi for Ui { Self::render_browse_frame(collection, selection, &state, frame); match state { - AppState::Info(_) => Self::render_info_overlay(collection, selection, frame), + AppState::Info(()) => Self::render_info_overlay(collection, selection, frame), + AppState::Reload(()) => Self::render_reload_overlay(frame), + AppState::Fetch(()) => Self::render_fetch_overlay(frame), AppState::Matches(public) => { Self::render_matches_overlay(public.matches, public.state, frame) } - AppState::Reload(_) => Self::render_reload_overlay(frame), AppState::Error(msg) => Self::render_error_overlay("Error", msg, frame), AppState::Critical(msg) => Self::render_error_overlay("Critical Error", msg, frame), _ => {} @@ -203,6 +208,7 @@ mod tests { AppState::Info(()) => AppState::Info(()), AppState::Reload(()) => AppState::Reload(()), AppState::Search(s) => AppState::Search(s), + AppState::Fetch(()) => AppState::Fetch(()), AppState::Matches(ref mut m) => AppState::Matches(AppPublicMatches { matches: m.matches, state: m.state, @@ -254,6 +260,9 @@ mod tests { app.state = AppState::Search(""); terminal.draw(|frame| Ui::render(&mut app, frame)).unwrap(); + app.state = AppState::Fetch(()); + terminal.draw(|frame| Ui::render(&mut app, frame)).unwrap(); + app.state = AppState::Error("get rekt scrub"); terminal.draw(|frame| Ui::render(&mut app, frame)).unwrap();