diff --git a/src/main.rs b/src/main.rs index 3fa803b..4577ea6 100644 --- a/src/main.rs +++ b/src/main.rs @@ -4,7 +4,7 @@ extern crate test; mod tui; -use std::{ffi::OsString, fs::OpenOptions, io, path::PathBuf}; +use std::{ffi::OsString, fs::OpenOptions, io, path::PathBuf, thread}; use ratatui::{backend::CrosstermBackend, Terminal}; use structopt::StructOpt; @@ -25,7 +25,10 @@ use musichoard::{ MusicHoardBuilder, NoDatabase, NoLibrary, }; -use tui::{App, EventChannel, EventHandler, EventListener, MusicBrainz, Tui, Ui}; +use tui::{ + App, EventChannel, EventHandler, EventListener, JobChannel, MusicBrainz, MusicBrainzDaemon, + Tui, Ui, +}; const MUSICHOARD_HTTP_USER_AGENT: &str = concat!( "MusicHoard/", @@ -91,10 +94,13 @@ fn with( let listener = EventListener::new(listener_sender); let handler = EventHandler::new(channel.receiver()); - let app = App::new(music_hoard, musicbrainz, app_sender); + let mb_job_channel = JobChannel::new(); + + let app = App::new(music_hoard, mb_job_channel.sender()); let ui = Ui; // Run the TUI application. + thread::spawn(|| MusicBrainzDaemon::run(musicbrainz, mb_job_channel.receiver(), app_sender)); Tui::run(terminal, app, ui, handler, listener).expect("failed to run tui"); } diff --git a/src/tui/app/machine/browse_state.rs b/src/tui/app/machine/browse_state.rs index 06c8cfb..7f26314 100644 --- a/src/tui/app/machine/browse_state.rs +++ b/src/tui/app/machine/browse_state.rs @@ -84,7 +84,7 @@ mod tests { machine::tests::{inner, inner_with_mb, music_hoard}, Category, IApp, IAppAccess, }, - lib::interface::musicbrainz::MockIMusicBrainz, + lib::interface::musicbrainz::daemon::MockIMbJobSender, testmod::COLLECTION, }; @@ -162,9 +162,16 @@ mod tests { #[test] fn fetch_musicbrainz() { - let mb_api = MockIMusicBrainz::new(); - let browse = - AppMachine::browse_state(inner_with_mb(music_hoard(COLLECTION.to_owned()), mb_api)); + let mut mb_job_sender = MockIMbJobSender::new(); + mb_job_sender + .expect_submit_background_job() + .times(1) + .returning(|_, _| Ok(())); + + let browse = AppMachine::browse_state(inner_with_mb( + music_hoard(COLLECTION.to_owned()), + mb_job_sender, + )); // Use the second artist for this test. let browse = browse.increment_selection(Delta::Line).unwrap_browse(); diff --git a/src/tui/app/machine/fetch_state.rs b/src/tui/app/machine/fetch_state.rs index da01165..73e5964 100644 --- a/src/tui/app/machine/fetch_state.rs +++ b/src/tui/app/machine/fetch_state.rs @@ -1,41 +1,31 @@ use std::{ - sync::{ - mpsc::{self, TryRecvError}, - Arc, Mutex, - }, - thread, time, + collections::VecDeque, + sync::mpsc::{self, TryRecvError}, }; -use musichoard::collection::{ - album::AlbumMeta, - artist::{Artist, ArtistMeta}, - musicbrainz::{IMusicBrainzRef, Mbid}, -}; +use musichoard::collection::{artist::Artist, musicbrainz::IMusicBrainzRef}; use crate::tui::{ app::{ machine::{match_state::MatchState, App, AppInner, AppMachine}, - AppPublicState, AppState, IAppEventFetch, IAppInteractFetch, MatchStateInfo, + AppPublicState, AppState, IAppEventFetch, IAppInteractFetch, + }, + lib::interface::musicbrainz::daemon::{ + Error as DaemonError, IMbJobSender, MbApiResult, MbParams, ResultSender, }, - event::{Event, EventSender}, - lib::interface::musicbrainz::{self, Error as MbError, IMusicBrainz}, }; pub struct FetchState { fetch_rx: FetchReceiver, } +pub type FetchReceiver = mpsc::Receiver; impl FetchState { pub fn new(fetch_rx: FetchReceiver) -> Self { FetchState { fetch_rx } } } -pub type FetchError = MbError; -pub type FetchResult = Result; -pub type FetchSender = mpsc::Sender; -pub type FetchReceiver = mpsc::Receiver; - impl AppMachine { fn fetch_state(inner: AppInner, state: FetchState) -> Self { AppMachine::new(inner, state) @@ -50,8 +40,10 @@ impl AppMachine { } }; - let (fetch_tx, fetch_rx) = mpsc::channel::(); - Self::spawn_fetch_thread(&inner, artist, fetch_tx); + let (fetch_tx, fetch_rx) = mpsc::channel::(); + if let Err(err) = Self::submit_fetch_job(&*inner.musicbrainz, fetch_tx, artist) { + return AppMachine::error_state(inner, err.to_string()).into(); + } let fetch = FetchState::new(fetch_rx); AppMachine::app_fetch(inner, fetch, true) @@ -85,78 +77,23 @@ impl AppMachine { } } - fn spawn_fetch_thread( - inner: &AppInner, + fn submit_fetch_job( + musicbrainz: &dyn IMbJobSender, + result_sender: ResultSender, artist: &Artist, - tx: FetchSender, - ) -> thread::JoinHandle<()> { - match artist.meta.musicbrainz { + ) -> Result<(), DaemonError> { + let requests = 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)) + let arid = arid.mbid(); + let albums = artist.albums.iter(); + albums + .filter(|album| album.meta.musicbrainz.is_none()) + .map(|album| MbParams::search_release_group(arid.clone(), album.meta.clone())) + .collect() } - 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| MatchStateInfo::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| MatchStateInfo::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(()) + None => VecDeque::from([MbParams::search_artist(artist.meta.clone())]), + }; + musicbrainz.submit_background_job(result_sender, requests) } } @@ -190,18 +127,16 @@ impl IAppEventFetch for AppMachine { #[cfg(test)] mod tests { - use mockall::{predicate, Sequence}; - use musichoard::collection::artist::ArtistMeta; + use mockall::predicate; + use musichoard::collection::{album::AlbumMeta, artist::ArtistMeta, musicbrainz::Mbid}; use crate::tui::{ app::{ machine::tests::{inner, music_hoard}, - AlbumMatches, ArtistMatches, IApp, + Delta, IApp, IAppAccess, IAppInteractBrowse, MatchOption, MatchStateInfo, }, - event::EventReceiver, - lib::interface::musicbrainz::{self, Match, MockIMusicBrainz}, + lib::interface::musicbrainz::{self, api::Match, daemon::MockIMbJobSender}, testmod::COLLECTION, - EventChannel, }; use super::*; @@ -212,210 +147,121 @@ mod tests { 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, + job_sender: &mut MockIMbJobSender, arid: &Mbid, - album: &AlbumMeta, - matches: &[Match], + albums: &[AlbumMeta], ) { - let result = Ok(matches.to_owned()); - api.expect_search_release_group() - .with(predicate::eq(arid.clone()), predicate::eq(album.clone())) + let mut requests = VecDeque::new(); + for album in albums.iter() { + requests.push_back(MbParams::search_release_group(arid.clone(), album.clone())); + } + job_sender + .expect_submit_background_job() + .with(predicate::always(), predicate::eq(requests)) .times(1) - .in_sequence(seq) - .return_once(|_, _| result); + .return_once(|_, _| Ok(())); } #[test] fn fetch_albums() { - let mut mb_api = MockIMusicBrainz::new(); + let mut mb_job_sender = MockIMbJobSender::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(); + let album_1_meta = COLLECTION[1].albums[0].meta.clone(); + let album_4_meta = COLLECTION[1].albums[3].meta.clone(); // 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); + search_release_group_expectation(&mut mb_job_sender, &arid, &[album_1_meta, album_4_meta]); 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 inner = AppInner::new(music_hoard, mb_job_sender); - 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(); + // Use second artist to match the expectation. + let browse = AppMachine::browse_state(inner); + let app = browse.increment_selection(Delta::Line); - assert_eq!(events_rx.try_recv().unwrap(), Event::FetchResultReady); - let result = fetch_rx.try_recv().unwrap(); - let expected = Ok(MatchStateInfo::Album(AlbumMatches { - 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(MatchStateInfo::Album(AlbumMatches { - matching: album_4.clone(), - list: matches_4.iter().cloned().map(Into::into).collect(), - })); - assert_eq!(result, expected); + let app = app.unwrap_browse().fetch_musicbrainz(); + assert!(matches!(app, AppState::Match(_))); } - 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())) + fn search_artist_expectation(job_sender: &mut MockIMbJobSender, artist: &ArtistMeta) { + let requests = VecDeque::from([MbParams::search_artist(artist.clone())]); + job_sender + .expect_submit_background_job() + .with(predicate::always(), predicate::eq(requests)) .times(1) - .in_sequence(seq) - .return_once(|_| result); + .return_once(|_, _| Ok(())); } #[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(MatchStateInfo::Artist(ArtistMatches { - 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(MatchStateInfo::Album(AlbumMatches { - 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 mut mb_job_sender = MockIMbJobSender::new(); let artist = COLLECTION[3].meta.clone(); - let fetch_result = Ok(MatchStateInfo::artist::>(artist, vec![])); - tx.send(fetch_result).unwrap(); + search_artist_expectation(&mut mb_job_sender, &artist); - let inner = inner(music_hoard(COLLECTION.clone())); - let fetch = FetchState::new(rx); - let app = AppMachine::app_fetch(inner, fetch, true); + let music_hoard = music_hoard(COLLECTION.to_owned()); + let inner = AppInner::new(music_hoard, mb_job_sender); + + // Use fourth artist to match the expectation. + let browse = AppMachine::browse_state(inner); + let browse = browse.increment_selection(Delta::Line).unwrap_browse(); + let browse = browse.increment_selection(Delta::Line).unwrap_browse(); + let app = browse.increment_selection(Delta::Line); + + let app = app.unwrap_browse().fetch_musicbrainz(); assert!(matches!(app, AppState::Match(_))); } #[test] - fn recv_ok_fetch_err() { - let (tx, rx) = mpsc::channel::(); + fn fetch_artist_job_sender_err() { + let mut mb_job_sender = MockIMbJobSender::new(); - let fetch_result = Err(musicbrainz::Error::RateLimit); + mb_job_sender + .expect_submit_background_job() + .return_once(|_, _| Err(DaemonError::JobChannelDisconnected)); + + let music_hoard = music_hoard(COLLECTION.to_owned()); + let inner = AppInner::new(music_hoard, mb_job_sender); + let browse = AppMachine::browse_state(inner); + + let app = browse.fetch_musicbrainz(); + assert!(matches!(app, AppState::Error(_))); + } + + #[test] + fn recv_ok_fetch_ok() { + let (tx, rx) = mpsc::channel::(); + + let artist = COLLECTION[3].meta.clone(); + let artist_match = Match::new(80, COLLECTION[2].meta.clone()); + let artist_match_info = MatchStateInfo::artist(artist.clone(), vec![artist_match.clone()]); + let fetch_result = Ok(artist_match_info); + tx.send(fetch_result).unwrap(); + + let inner = inner(music_hoard(COLLECTION.clone())); + let fetch = FetchState::new(rx); + let mut app = AppMachine::app_fetch(inner, fetch, true); + assert!(matches!(app, AppState::Match(_))); + + let public = app.get(); + let match_state = public.state.unwrap_match(); + let match_options = vec![ + artist_match.into(), + MatchOption::CannotHaveMbid, + MatchOption::ManualInputMbid, + ]; + let expected = MatchStateInfo::artist(artist, match_options); + assert_eq!(match_state.info, Some(expected).as_ref()); + } + + #[test] + fn recv_ok_fetch_err() { + let (tx, rx) = mpsc::channel::(); + + let fetch_result = Err(musicbrainz::api::Error::RateLimit); tx.send(fetch_result).unwrap(); let inner = inner(music_hoard(COLLECTION.clone())); @@ -426,7 +272,7 @@ mod tests { #[test] fn recv_err_empty() { - let (_tx, rx) = mpsc::channel::(); + let (_tx, rx) = mpsc::channel::(); let inner = inner(music_hoard(COLLECTION.clone())); let fetch = FetchState::new(rx); @@ -436,7 +282,7 @@ mod tests { #[test] fn recv_err_disconnected_first() { - let (_, rx) = mpsc::channel::(); + let (_, rx) = mpsc::channel::(); let inner = inner(music_hoard(COLLECTION.clone())); let fetch = FetchState::new(rx); @@ -446,7 +292,7 @@ mod tests { #[test] fn recv_err_disconnected_next() { - let (_, rx) = mpsc::channel::(); + let (_, rx) = mpsc::channel::(); let fetch = FetchState::new(rx); let app = AppMachine::app_fetch_next(inner(music_hoard(COLLECTION.clone())), fetch); @@ -455,7 +301,7 @@ mod tests { #[test] fn empty_first_then_ready() { - let (tx, rx) = mpsc::channel::(); + let (tx, rx) = mpsc::channel::(); let inner = inner(music_hoard(COLLECTION.clone())); let fetch = FetchState::new(rx); @@ -472,7 +318,7 @@ mod tests { #[test] fn abort() { - let (_, rx) = mpsc::channel::(); + let (_, rx) = mpsc::channel::(); let fetch = FetchState::new(rx); let app = AppMachine::fetch_state(inner(music_hoard(COLLECTION.clone())), fetch); diff --git a/src/tui/app/machine/input.rs b/src/tui/app/machine/input.rs index 0494235..17f22a1 100644 --- a/src/tui/app/machine/input.rs +++ b/src/tui/app/machine/input.rs @@ -58,7 +58,7 @@ impl IAppInput for AppInputMode { #[cfg(test)] mod tests { use crate::tui::app::{ - machine::tests::{events, mb_api, music_hoard_init}, + machine::tests::{mb_job_sender, music_hoard_init}, IApp, }; @@ -74,7 +74,7 @@ mod tests { #[test] fn handle_input() { - let mut app = App::new(music_hoard_init(vec![]), mb_api(), events()); + let mut app = App::new(music_hoard_init(vec![]), mb_job_sender()); app.input_mut().replace(Input::default()); let input = app.mode().unwrap_input(); diff --git a/src/tui/app/machine/match_state.rs b/src/tui/app/machine/match_state.rs index efcf7b7..362fa96 100644 --- a/src/tui/app/machine/match_state.rs +++ b/src/tui/app/machine/match_state.rs @@ -179,7 +179,7 @@ mod tests { machine::tests::{inner, music_hoard}, IApp, IAppAccess, IAppInput, }, - lib::interface::musicbrainz::Match, + lib::interface::musicbrainz::api::Match, }; use super::*; diff --git a/src/tui/app/machine/mod.rs b/src/tui/app/machine/mod.rs index f7c5d08..35b6542 100644 --- a/src/tui/app/machine/mod.rs +++ b/src/tui/app/machine/mod.rs @@ -8,15 +8,12 @@ mod match_state; mod reload_state; mod search_state; -use std::sync::{Arc, Mutex}; - use crate::tui::{ app::{ selection::Selection, AppMode, AppPublic, AppPublicInner, AppPublicState, AppState, IApp, IAppAccess, IAppBase, IAppState, }, - event::EventSender, - lib::{interface::musicbrainz::IMusicBrainz, IMusicHoard}, + lib::{interface::musicbrainz::daemon::IMbJobSender, IMusicHoard}, }; use browse_state::BrowseState; @@ -49,9 +46,8 @@ pub struct AppMachine { pub struct AppInner { running: bool, music_hoard: Box, - musicbrainz: Arc>, + musicbrainz: Box, selection: Selection, - events: EventSender, } macro_rules! app_field_ref { @@ -85,13 +81,12 @@ macro_rules! app_field_mut { } impl App { - pub fn new( + 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, events); + let inner = AppInner::new(music_hoard, musicbrainz); match init_result { Ok(()) => AppMachine::browse_state(inner).into(), Err(err) => AppMachine::critical_state(inner, err.to_string()).into(), @@ -174,18 +169,16 @@ impl IAppAccess for App { } impl AppInner { - pub fn new( + pub fn new( music_hoard: MH, musicbrainz: MB, - events: EventSender, ) -> Self { let selection = Selection::new(music_hoard.get_collection()); AppInner { running: true, music_hoard: Box::new(music_hoard), - musicbrainz: Arc::new(Mutex::new(musicbrainz)), + musicbrainz: Box::new(musicbrainz), selection, - events, } } } @@ -230,8 +223,7 @@ mod tests { use crate::tui::{ app::{AppState, IApp, IAppInput, IAppInteractBrowse}, - lib::{interface::musicbrainz::MockIMusicBrainz, MockIMusicHoard}, - EventChannel, + lib::{interface::musicbrainz::daemon::MockIMbJobSender, MockIMusicHoard}, }; use super::*; @@ -348,25 +340,24 @@ mod tests { music_hoard } - pub fn mb_api() -> MockIMusicBrainz { - MockIMusicBrainz::new() - } - - pub fn events() -> EventSender { - EventChannel::new().sender() + pub fn mb_job_sender() -> MockIMbJobSender { + MockIMbJobSender::new() } pub fn inner(music_hoard: MockIMusicHoard) -> AppInner { - AppInner::new(music_hoard, mb_api(), events()) + AppInner::new(music_hoard, mb_job_sender()) } - pub fn inner_with_mb(music_hoard: MockIMusicHoard, mb_api: MockIMusicBrainz) -> AppInner { - AppInner::new(music_hoard, mb_api, events()) + pub fn inner_with_mb( + music_hoard: MockIMusicHoard, + mb_job_sender: MockIMbJobSender, + ) -> AppInner { + AppInner::new(music_hoard, mb_job_sender) } #[test] fn input_mode() { - let app = App::new(music_hoard_init(vec![]), mb_api(), events()); + let app = App::new(music_hoard_init(vec![]), mb_job_sender()); assert!(app.is_running()); let mode = app.mode(); @@ -393,7 +384,7 @@ mod tests { #[test] fn state_browse() { - let mut app = App::new(music_hoard_init(vec![]), mb_api(), events()); + let mut app = App::new(music_hoard_init(vec![]), mb_job_sender()); assert!(app.is_running()); let state = app.state(); @@ -414,7 +405,7 @@ mod tests { #[test] fn state_info() { - let mut app = App::new(music_hoard_init(vec![]), mb_api(), events()); + let mut app = App::new(music_hoard_init(vec![]), mb_job_sender()); assert!(app.is_running()); app = app.unwrap_browse().show_info_overlay(); @@ -437,7 +428,7 @@ mod tests { #[test] fn state_reload() { - let mut app = App::new(music_hoard_init(vec![]), mb_api(), events()); + let mut app = App::new(music_hoard_init(vec![]), mb_job_sender()); assert!(app.is_running()); app = app.unwrap_browse().show_reload_menu(); @@ -460,7 +451,7 @@ mod tests { #[test] fn state_search() { - let mut app = App::new(music_hoard_init(vec![]), mb_api(), events()); + let mut app = App::new(music_hoard_init(vec![]), mb_job_sender()); assert!(app.is_running()); app = app.unwrap_browse().begin_search(); @@ -483,7 +474,7 @@ mod tests { #[test] fn state_fetch() { - let mut app = App::new(music_hoard_init(vec![]), mb_api(), events()); + let mut app = App::new(music_hoard_init(vec![]), mb_job_sender()); assert!(app.is_running()); let (_, rx) = mpsc::channel(); @@ -509,7 +500,7 @@ mod tests { #[test] fn state_match() { - let mut app = App::new(music_hoard_init(vec![]), mb_api(), events()); + let mut app = App::new(music_hoard_init(vec![]), mb_job_sender()); assert!(app.is_running()); let (_, rx) = mpsc::channel(); @@ -535,7 +526,7 @@ mod tests { #[test] fn state_error() { - let mut app = App::new(music_hoard_init(vec![]), mb_api(), events()); + let mut app = App::new(music_hoard_init(vec![]), mb_job_sender()); assert!(app.is_running()); app = AppMachine::error_state(app.unwrap_browse().inner, "get rekt").into(); @@ -558,7 +549,7 @@ mod tests { #[test] fn state_critical() { - let mut app = App::new(music_hoard_init(vec![]), mb_api(), events()); + let mut app = App::new(music_hoard_init(vec![]), mb_job_sender()); assert!(app.is_running()); app = AppMachine::critical_state(app.unwrap_browse().inner, "get rekt").into(); @@ -589,7 +580,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(), events()); + let app = App::new(music_hoard, mb_job_sender()); assert!(app.is_running()); app.unwrap_critical(); } diff --git a/src/tui/app/mod.rs b/src/tui/app/mod.rs index dd6b3a9..39ec665 100644 --- a/src/tui/app/mod.rs +++ b/src/tui/app/mod.rs @@ -6,7 +6,7 @@ pub use selection::{Category, Delta, Selection, WidgetState}; use musichoard::collection::{album::AlbumMeta, artist::ArtistMeta, Collection}; -use crate::tui::lib::interface::musicbrainz::Match; +use crate::tui::lib::interface::musicbrainz::api::Match; pub enum AppState { Browse(B), diff --git a/src/tui/event.rs b/src/tui/event.rs index eaa90e1..34c5bb1 100644 --- a/src/tui/event.rs +++ b/src/tui/event.rs @@ -2,6 +2,9 @@ use crossterm::event::KeyEvent; use std::fmt; use std::sync::mpsc; +#[cfg(test)] +use mockall::automock; + #[derive(Debug)] pub enum EventError { Send(Event), @@ -42,7 +45,7 @@ impl From for EventError { #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum Event { Key(KeyEvent), - FetchResultReady, + FetchComplete, } pub struct EventChannel { @@ -50,6 +53,15 @@ pub struct EventChannel { receiver: mpsc::Receiver, } +pub trait IKeyEventSender { + fn send_key(&self, key_event: KeyEvent) -> Result<(), EventError>; +} + +#[cfg_attr(test, automock)] +pub trait IFetchCompleteEventSender { + fn send_fetch_complete(&self) -> Result<(), EventError>; +} + #[derive(Clone)] pub struct EventSender { sender: mpsc::Sender, @@ -78,9 +90,15 @@ impl EventChannel { } } -impl EventSender { - pub fn send(&self, event: Event) -> Result<(), EventError> { - Ok(self.sender.send(event)?) +impl IKeyEventSender for EventSender { + fn send_key(&self, key_event: KeyEvent) -> Result<(), EventError> { + Ok(self.sender.send(Event::Key(key_event))?) + } +} + +impl IFetchCompleteEventSender for EventSender { + fn send_fetch_complete(&self) -> Result<(), EventError> { + Ok(self.sender.send(Event::FetchComplete)?) } } @@ -108,13 +126,13 @@ mod tests { let channel = EventChannel::new(); let sender = channel.sender(); let receiver = channel.receiver(); - let event = Event::Key(KeyEvent::new(KeyCode::Up, KeyModifiers::empty())); + let key_event = KeyEvent::new(KeyCode::Up, KeyModifiers::empty()); - let result = sender.send(event); + let result = sender.send_key(key_event); assert!(result.is_ok()); drop(receiver); - let result = sender.send(event); + let result = sender.send_key(key_event); assert!(result.is_err()); } @@ -123,9 +141,9 @@ mod tests { let channel = EventChannel::new(); let sender = channel.sender(); let receiver = channel.receiver(); - let event = Event::Key(KeyEvent::new(KeyCode::Up, KeyModifiers::empty())); + let key_event = KeyEvent::new(KeyCode::Up, KeyModifiers::empty()); - sender.send(event).unwrap(); + sender.send_key(key_event).unwrap(); let result = receiver.recv(); assert!(result.is_ok()); @@ -139,12 +157,11 @@ mod tests { 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(); + sender.send_fetch_complete().unwrap(); let result = receiver.try_recv(); assert!(result.is_ok()); diff --git a/src/tui/handler.rs b/src/tui/handler.rs index 9adea18..cf04371 100644 --- a/src/tui/handler.rs +++ b/src/tui/handler.rs @@ -28,7 +28,7 @@ trait IEventHandlerPrivate { fn handle_error_key_event(app: ::ErrorState, key_event: KeyEvent) -> APP; fn handle_critical_key_event(app: ::CriticalState, key_event: KeyEvent) -> APP; - fn handle_fetch_result_ready_event(app: APP) -> APP; + fn handle_fetch_complete_event(app: APP) -> APP; fn handle_input_key_event>(app: Input, key_event: KeyEvent) -> APP; } @@ -48,7 +48,7 @@ impl IEventHandler for EventHandler { 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), + Event::FetchComplete => Self::handle_fetch_complete_event(app), }) } } @@ -92,7 +92,7 @@ impl IEventHandlerPrivate for EventHandler { } } - fn handle_fetch_result_ready_event(app: APP) -> APP { + fn handle_fetch_complete_event(app: APP) -> APP { match app.state() { AppState::Browse(state) => state.no_op(), AppState::Info(state) => state.no_op(), diff --git a/src/tui/lib/external/musicbrainz/api/mod.rs b/src/tui/lib/external/musicbrainz/api/mod.rs new file mode 100644 index 0000000..3730382 --- /dev/null +++ b/src/tui/lib/external/musicbrainz/api/mod.rs @@ -0,0 +1,104 @@ +//! Module for interacting with the [MusicBrainz API](https://musicbrainz.org/doc/MusicBrainz_API). + +use std::collections::HashMap; + +use musichoard::{ + collection::{ + album::{AlbumDate, AlbumMeta, AlbumSeq}, + artist::ArtistMeta, + musicbrainz::Mbid, + }, + external::musicbrainz::{ + api::{ + search::{ + SearchArtistRequest, SearchArtistResponseArtist, SearchReleaseGroupRequest, + SearchReleaseGroupResponseReleaseGroup, + }, + MusicBrainzClient, + }, + IMusicBrainzHttp, + }, +}; + +use crate::tui::lib::interface::musicbrainz::api::{Error, IMusicBrainz, Match}; + +// GRCOV_EXCL_START +pub struct MusicBrainz { + client: MusicBrainzClient, +} + +impl MusicBrainz { + pub fn new(client: MusicBrainzClient) -> Self { + MusicBrainz { client } + } +} + +impl IMusicBrainz for MusicBrainz { + fn search_artist(&mut self, artist: &ArtistMeta) -> Result>, Error> { + let query = SearchArtistRequest::new().string(&artist.id.name); + + let mb_response = self.client.search_artist(query)?; + + Ok(mb_response + .artists + .into_iter() + .map(from_search_artist_response_artist) + .collect()) + } + + fn search_release_group( + &mut self, + arid: &Mbid, + album: &AlbumMeta, + ) -> Result>, Error> { + // Some release groups may have a promotional early release messing up the search. Searching + // with just the year should be enough anyway. + let date = AlbumDate::new(album.date.year, None, None); + + let query = SearchReleaseGroupRequest::new() + .arid(arid) + .and() + .first_release_date(&date) + .and() + .release_group(&album.id.title); + + let mb_response = self.client.search_release_group(query)?; + + Ok(mb_response + .release_groups + .into_iter() + .map(from_search_release_group_response_release_group) + .collect()) + } +} + +fn from_search_artist_response_artist(entity: SearchArtistResponseArtist) -> Match { + Match { + score: entity.score, + item: ArtistMeta { + id: entity.name, + sort: entity.sort.map(Into::into), + musicbrainz: Some(entity.id.into()), + properties: HashMap::new(), + }, + disambiguation: entity.disambiguation, + } +} + +fn from_search_release_group_response_release_group( + entity: SearchReleaseGroupResponseReleaseGroup, +) -> Match { + Match { + score: entity.score, + item: AlbumMeta { + id: entity.title, + date: entity.first_release_date, + seq: AlbumSeq::default(), + musicbrainz: Some(entity.id.into()), + primary_type: Some(entity.primary_type), + secondary_types: entity.secondary_types.unwrap_or_default(), + }, + disambiguation: None, + } +} +// GRCOV_EXCL_STOP diff --git a/src/tui/lib/external/musicbrainz/daemon/mod.rs b/src/tui/lib/external/musicbrainz/daemon/mod.rs new file mode 100644 index 0000000..686f3ff --- /dev/null +++ b/src/tui/lib/external/musicbrainz/daemon/mod.rs @@ -0,0 +1,705 @@ +use std::{collections::VecDeque, sync::mpsc, thread, time}; + +use crate::tui::{ + app::MatchStateInfo, + event::IFetchCompleteEventSender, + lib::interface::musicbrainz::{ + api::{Error as ApiError, IMusicBrainz}, + daemon::{Error, IMbJobSender, MbParams, ResultSender, SearchParams}, + }, +}; + +pub struct MusicBrainzDaemon { + musicbrainz: Box, + job_receiver: mpsc::Receiver, + job_queue: JobQueue, + event_sender: Box, +} + +struct JobQueue { + foreground_queue: VecDeque, + background_queue: VecDeque, +} + +#[derive(Debug)] +struct Job { + priority: JobPriority, + instance: JobInstance, +} + +impl Job { + pub fn new(priority: JobPriority, instance: JobInstance) -> Self { + Job { priority, instance } + } +} + +#[derive(Debug)] +enum JobPriority { + #[cfg(test)] + Foreground, + Background, +} + +#[derive(Debug)] +struct JobInstance { + result_sender: ResultSender, + requests: VecDeque, +} + +#[derive(Debug, PartialEq, Eq)] +enum JobInstanceStatus { + Continue, + Complete, +} + +#[derive(Debug, PartialEq, Eq)] +enum JobInstanceError { + ReturnChannelDisconnected, + EventChannelDisconnected, +} + +impl JobInstance { + fn new(result_sender: ResultSender, requests: VecDeque) -> Self { + JobInstance { + result_sender, + requests, + } + } +} + +#[derive(Debug, PartialEq, Eq)] +enum JobError { + JobQueueEmpty, + EventChannelDisconnected, +} + +pub struct JobChannel { + sender: mpsc::Sender, + receiver: mpsc::Receiver, +} + +pub struct JobSender { + sender: mpsc::Sender, +} + +pub struct JobReceiver { + receiver: mpsc::Receiver, +} + +impl JobChannel { + pub fn new() -> Self { + let (sender, receiver) = mpsc::channel(); + JobChannel { receiver, sender } + } + + pub fn sender(&self) -> JobSender { + JobSender { + sender: self.sender.clone(), + } + } + + pub fn receiver(self) -> JobReceiver { + JobReceiver { + receiver: self.receiver, + } + } +} + +impl IMbJobSender for JobSender { + fn submit_background_job( + &self, + result_sender: ResultSender, + requests: VecDeque, + ) -> Result<(), Error> { + self.send_background_job(result_sender, requests) + } +} + +impl JobSender { + fn send_background_job( + &self, + result_sender: ResultSender, + requests: VecDeque, + ) -> Result<(), Error> { + self.send_job(JobPriority::Background, result_sender, requests) + } + + fn send_job( + &self, + priority: JobPriority, + result_sender: ResultSender, + requests: VecDeque, + ) -> Result<(), Error> { + let instance = JobInstance::new(result_sender, requests); + let job = Job::new(priority, instance); + self.sender.send(job).or(Err(Error::JobChannelDisconnected)) + } +} + +impl MusicBrainzDaemon { + // GRCOV_EXCL_START + pub fn run( + musicbrainz: MB, + job_receiver: JobReceiver, + event_sender: ES, + ) -> Result<(), Error> { + let daemon = MusicBrainzDaemon { + musicbrainz: Box::new(musicbrainz), + job_receiver: job_receiver.receiver, + job_queue: JobQueue::new(), + event_sender: Box::new(event_sender), + }; + daemon.main() + } + + fn main(mut self) -> Result<(), Error> { + loop { + self.enqueue_all_pending_jobs()?; + match self.execute_next_job() { + Ok(()) => { + // Sleep for one second. Required by MB API rate limiting. Assume all other + // processing takes negligible time such that regardless of how much other + // processing there is to be done, this one second sleep is necessary. + thread::sleep(time::Duration::from_secs(1)); + } + Err(JobError::JobQueueEmpty) => { + self.wait_for_jobs()?; + } + Err(JobError::EventChannelDisconnected) => { + return Err(Error::EventChannelDisconnected); + } + } + } + } + // GRCOV_EXCL_STOP + + fn enqueue_all_pending_jobs(&mut self) -> Result<(), Error> { + loop { + match self.job_receiver.try_recv() { + Ok(job) => self.job_queue.push_back(job), + Err(mpsc::TryRecvError::Empty) => return Ok(()), + Err(mpsc::TryRecvError::Disconnected) => { + return Err(Error::JobChannelDisconnected); + } + } + } + } + + fn execute_next_job(&mut self) -> Result<(), JobError> { + if let Some(instance) = self.job_queue.front_mut() { + let result = instance.execute_next(&mut *self.musicbrainz, &mut *self.event_sender); + match result { + Ok(JobInstanceStatus::Continue) => {} + Ok(JobInstanceStatus::Complete) + | Err(JobInstanceError::ReturnChannelDisconnected) => { + self.job_queue.pop_front(); + } + Err(JobInstanceError::EventChannelDisconnected) => { + return Err(JobError::EventChannelDisconnected) + } + } + return Ok(()); + } + Err(JobError::JobQueueEmpty) + } + + fn wait_for_jobs(&mut self) -> Result<(), Error> { + assert!(self.job_queue.is_empty()); + + match self.job_receiver.recv() { + Ok(job) => self.job_queue.push_back(job), + Err(mpsc::RecvError) => return Err(Error::JobChannelDisconnected), + } + + Ok(()) + } +} + +impl JobInstance { + fn execute_next( + &mut self, + musicbrainz: &mut dyn IMusicBrainz, + event_sender: &mut dyn IFetchCompleteEventSender, + ) -> Result { + // self.requests can be empty if the caller submits an empty job. + if let Some(params) = self.requests.pop_front() { + self.execute(musicbrainz, event_sender, params)? + } + + if self.requests.is_empty() { + Ok(JobInstanceStatus::Complete) + } else { + Ok(JobInstanceStatus::Continue) + } + } + + fn execute( + &mut self, + musicbrainz: &mut dyn IMusicBrainz, + event_sender: &mut dyn IFetchCompleteEventSender, + api_params: MbParams, + ) -> Result<(), JobInstanceError> { + match api_params { + MbParams::Search(search) => match search { + SearchParams::Artist(params) => { + let result = musicbrainz.search_artist(¶ms.artist); + let result = result.map(|list| MatchStateInfo::artist(params.artist, list)); + self.return_result(event_sender, result) + } + SearchParams::ReleaseGroup(params) => { + let result = musicbrainz.search_release_group(¶ms.arid, ¶ms.album); + let result = result.map(|list| MatchStateInfo::album(params.album, list)); + self.return_result(event_sender, result) + } + }, + } + } + + fn return_result( + &mut self, + event_sender: &mut dyn IFetchCompleteEventSender, + result: Result, + ) -> Result<(), JobInstanceError> { + self.result_sender + .send(result) + .map_err(|_| JobInstanceError::ReturnChannelDisconnected)?; + + // 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. + event_sender + .send_fetch_complete() + .map_err(|_| JobInstanceError::EventChannelDisconnected)?; + + Ok(()) + } +} + +impl JobQueue { + fn new() -> Self { + JobQueue { + foreground_queue: VecDeque::new(), + background_queue: VecDeque::new(), + } + } + + fn is_empty(&self) -> bool { + self.foreground_queue.is_empty() && self.background_queue.is_empty() + } + + fn front_mut(&mut self) -> Option<&mut JobInstance> { + self.foreground_queue + .front_mut() + .or_else(|| self.background_queue.front_mut()) + } + + fn pop_front(&mut self) -> Option { + self.foreground_queue + .pop_front() + .or_else(|| self.background_queue.pop_front()) + } + + #[cfg(test)] + fn push_back(&mut self, job: Job) { + match job.priority { + JobPriority::Foreground => self.foreground_queue.push_back(job.instance), + JobPriority::Background => self.background_queue.push_back(job.instance), + } + } + + // GRCOV_EXCL_START + #[cfg(not(test))] + fn push_back(&mut self, job: Job) { + match job.priority { + JobPriority::Background => self.background_queue.push_back(job.instance), + } + } + // GRCOV_EXCL_STOP +} + +#[cfg(test)] +mod tests { + use mockall::{predicate, Sequence}; + use musichoard::collection::{ + album::AlbumMeta, + artist::ArtistMeta, + musicbrainz::{IMusicBrainzRef, Mbid}, + }; + + use crate::tui::{ + event::{Event, EventError, MockIFetchCompleteEventSender}, + lib::interface::musicbrainz::api::{Match, MockIMusicBrainz}, + testmod::COLLECTION, + }; + + use super::*; + + fn musicbrainz() -> MockIMusicBrainz { + MockIMusicBrainz::new() + } + + fn job_channel() -> (JobSender, JobReceiver) { + let channel = JobChannel::new(); + let sender = channel.sender(); + let receiver = channel.receiver(); + (sender, receiver) + } + + fn event_sender() -> MockIFetchCompleteEventSender { + MockIFetchCompleteEventSender::new() + } + + fn fetch_complete_expectation(event_sender: &mut MockIFetchCompleteEventSender, times: usize) { + event_sender + .expect_send_fetch_complete() + .times(times) + .returning(|| Ok(())); + } + + fn daemon(job_receiver: JobReceiver) -> MusicBrainzDaemon { + MusicBrainzDaemon { + musicbrainz: Box::new(musicbrainz()), + job_receiver: job_receiver.receiver, + job_queue: JobQueue::new(), + event_sender: Box::new(event_sender()), + } + } + + fn daemon_with( + musicbrainz: MockIMusicBrainz, + job_receiver: JobReceiver, + event_sender: MockIFetchCompleteEventSender, + ) -> MusicBrainzDaemon { + MusicBrainzDaemon { + musicbrainz: Box::new(musicbrainz), + job_receiver: job_receiver.receiver, + job_queue: JobQueue::new(), + event_sender: Box::new(event_sender), + } + } + + fn search_artist_requests() -> VecDeque { + let artist = COLLECTION[3].meta.clone(); + VecDeque::from([MbParams::search_artist(artist)]) + } + + 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_albums_requests() -> VecDeque { + let mbref = COLLECTION[1].meta.musicbrainz.as_ref(); + let arid = mbref.unwrap().mbid().clone(); + + let album_1 = COLLECTION[1].albums[0].meta.clone(); + let album_4 = COLLECTION[1].albums[3].meta.clone(); + + VecDeque::from([ + MbParams::search_release_group(arid.clone(), album_1), + MbParams::search_release_group(arid.clone(), album_4), + ]) + } + + fn album_arid_expectation() -> Mbid { + let mbref = COLLECTION[1].meta.musicbrainz.as_ref(); + mbref.unwrap().mbid().clone() + } + + 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) + } + + #[test] + fn enqueue_job_channel_disconnected() { + let (_, job_receiver) = job_channel(); + let mut daemon = daemon(job_receiver); + let result = daemon.enqueue_all_pending_jobs(); + assert_eq!(result, Err(Error::JobChannelDisconnected)); + } + + #[test] + fn wait_for_job_channel_disconnected() { + let (_, job_receiver) = job_channel(); + let mut daemon = daemon(job_receiver); + let result = daemon.wait_for_jobs(); + assert_eq!(result, Err(Error::JobChannelDisconnected)); + } + + #[test] + fn enqueue_job_queue_empty() { + let (_job_sender, job_receiver) = job_channel(); + let mut daemon = daemon(job_receiver); + let result = daemon.enqueue_all_pending_jobs(); + assert_eq!(result, Ok(())); + } + + #[test] + fn enqueue_job() { + let (job_sender, job_receiver) = job_channel(); + let mut daemon = daemon(job_receiver); + + let requests = search_artist_requests(); + let (result_sender, _) = mpsc::channel(); + let result = job_sender.submit_background_job(result_sender, requests.clone()); + assert_eq!(result, Ok(())); + + let result = daemon.enqueue_all_pending_jobs(); + assert_eq!(result, Ok(())); + + assert_eq!(daemon.job_queue.pop_front().unwrap().requests, requests); + } + + #[test] + fn wait_for_jobs_job() { + let (job_sender, job_receiver) = job_channel(); + let mut daemon = daemon(job_receiver); + + let requests = search_artist_requests(); + let (result_sender, _) = mpsc::channel(); + let result = job_sender.submit_background_job(result_sender, requests.clone()); + assert_eq!(result, Ok(())); + + let result = daemon.wait_for_jobs(); + assert_eq!(result, Ok(())); + + assert_eq!(daemon.job_queue.pop_front().unwrap().requests, requests); + } + + #[test] + fn execute_empty() { + let (_job_sender, job_receiver) = job_channel(); + let mut daemon = daemon(job_receiver); + + let (result_sender, _) = mpsc::channel(); + let mut instance = JobInstance::new(result_sender, VecDeque::new()); + let result = instance.execute_next(&mut musicbrainz(), &mut event_sender()); + assert_eq!(result, Ok(JobInstanceStatus::Complete)); + + let result = daemon.enqueue_all_pending_jobs(); + assert_eq!(result, Ok(())); + + let result = daemon.execute_next_job(); + assert_eq!(result, Err(JobError::JobQueueEmpty)); + } + + fn search_artist_expectation( + musicbrainz: &mut MockIMusicBrainz, + artist: &ArtistMeta, + matches: &[Match], + ) { + let result = Ok(matches.to_owned()); + musicbrainz + .expect_search_artist() + .with(predicate::eq(artist.clone())) + .times(1) + .return_once(|_| result); + } + + #[test] + fn execute_search_artist() { + let mut musicbrainz = musicbrainz(); + let (artist, matches) = artist_expectations(); + search_artist_expectation(&mut musicbrainz, &artist, &matches); + + let mut event_sender = event_sender(); + fetch_complete_expectation(&mut event_sender, 1); + + let (job_sender, job_receiver) = job_channel(); + let mut daemon = daemon_with(musicbrainz, job_receiver, event_sender); + + let requests = search_artist_requests(); + let (result_sender, result_receiver) = mpsc::channel(); + let result = job_sender.submit_background_job(result_sender, requests); + assert_eq!(result, Ok(())); + + let result = daemon.enqueue_all_pending_jobs(); + assert_eq!(result, Ok(())); + + let result = daemon.execute_next_job(); + assert_eq!(result, Ok(())); + + let result = result_receiver.try_recv().unwrap(); + assert_eq!(result, Ok(MatchStateInfo::artist(artist, matches))); + } + + fn search_release_group_expectation( + musicbrainz: &mut MockIMusicBrainz, + seq: &mut Sequence, + arid: &Mbid, + album: &AlbumMeta, + matches: &[Match], + ) { + let result = Ok(matches.to_owned()); + musicbrainz + .expect_search_release_group() + .with(predicate::eq(arid.clone()), predicate::eq(album.clone())) + .times(1) + .in_sequence(seq) + .return_once(|_, _| result); + } + + #[test] + fn execute_search_release_groups() { + let mut musicbrainz = musicbrainz(); + let arid = album_arid_expectation(); + let (album_1, matches_1) = album_expectations_1(); + let (album_4, matches_4) = album_expectations_4(); + + let mut seq = Sequence::new(); + search_release_group_expectation(&mut musicbrainz, &mut seq, &arid, &album_1, &matches_1); + search_release_group_expectation(&mut musicbrainz, &mut seq, &arid, &album_4, &matches_4); + + let mut event_sender = event_sender(); + fetch_complete_expectation(&mut event_sender, 2); + + let (job_sender, job_receiver) = job_channel(); + let mut daemon = daemon_with(musicbrainz, job_receiver, event_sender); + + let requests = search_albums_requests(); + let (result_sender, result_receiver) = mpsc::channel(); + let result = job_sender.submit_background_job(result_sender, requests); + assert_eq!(result, Ok(())); + + let result = daemon.enqueue_all_pending_jobs(); + assert_eq!(result, Ok(())); + + let result = daemon.execute_next_job(); + assert_eq!(result, Ok(())); + + let result = daemon.execute_next_job(); + assert_eq!(result, Ok(())); + + let result = result_receiver.try_recv().unwrap(); + assert_eq!(result, Ok(MatchStateInfo::album(album_1, matches_1))); + + let result = result_receiver.try_recv().unwrap(); + assert_eq!(result, Ok(MatchStateInfo::album(album_4, matches_4))); + } + + #[test] + fn execute_search_release_groups_result_disconnect() { + let mut musicbrainz = musicbrainz(); + let arid = album_arid_expectation(); + let (album_1, matches_1) = album_expectations_1(); + + let mut seq = Sequence::new(); + search_release_group_expectation(&mut musicbrainz, &mut seq, &arid, &album_1, &matches_1); + + let mut event_sender = event_sender(); + fetch_complete_expectation(&mut event_sender, 0); + + let (job_sender, job_receiver) = job_channel(); + let mut daemon = daemon_with(musicbrainz, job_receiver, event_sender); + + let requests = search_albums_requests(); + let (result_sender, _) = mpsc::channel(); + let result = job_sender.submit_background_job(result_sender, requests); + assert_eq!(result, Ok(())); + + let result = daemon.enqueue_all_pending_jobs(); + assert_eq!(result, Ok(())); + + let result = daemon.execute_next_job(); + assert_eq!(result, Ok(())); + + // Two albums were submitted, but as one job. If the first one fails due to the + // result_channel disconnecting, all remaining requests in that job are dropped. + assert!(daemon.job_queue.pop_front().is_none()); + } + + #[test] + fn execute_search_artist_event_disconnect() { + let mut musicbrainz = musicbrainz(); + let (artist, matches) = artist_expectations(); + search_artist_expectation(&mut musicbrainz, &artist, &matches); + + let mut event_sender = event_sender(); + event_sender + .expect_send_fetch_complete() + .times(1) + .return_once(|| Err(EventError::Send(Event::FetchComplete))); + + let (job_sender, job_receiver) = job_channel(); + let mut daemon = daemon_with(musicbrainz, job_receiver, event_sender); + + let requests = search_artist_requests(); + let (result_sender, _result_receiver) = mpsc::channel(); + let result = job_sender.submit_background_job(result_sender, requests); + assert_eq!(result, Ok(())); + + let result = daemon.enqueue_all_pending_jobs(); + assert_eq!(result, Ok(())); + + let result = daemon.execute_next_job(); + assert_eq!(result, Err(JobError::EventChannelDisconnected)); + } + + #[test] + fn job_queue() { + let mut queue = JobQueue::new(); + assert!(queue.is_empty()); + assert_eq!(queue.foreground_queue.len(), 0); + assert_eq!(queue.background_queue.len(), 0); + + let (result_sender, _) = mpsc::channel(); + let bg = Job::new( + JobPriority::Background, + JobInstance::new(result_sender, VecDeque::new()), + ); + queue.push_back(bg); + + assert!(!queue.is_empty()); + assert_eq!(queue.foreground_queue.len(), 0); + assert_eq!(queue.background_queue.len(), 1); + + let (result_sender, _) = mpsc::channel(); + let fg = Job::new( + JobPriority::Foreground, + JobInstance::new(result_sender, VecDeque::new()), + ); + queue.push_back(fg); + + assert!(!queue.is_empty()); + assert_eq!(queue.foreground_queue.len(), 1); + assert_eq!(queue.background_queue.len(), 1); + + let instance = queue.pop_front(); + assert!(instance.is_some()); + assert!(!queue.is_empty()); + assert_eq!(queue.foreground_queue.len(), 0); + assert_eq!(queue.background_queue.len(), 1); + + let instance = queue.pop_front(); + assert!(instance.is_some()); + assert!(queue.is_empty()); + assert_eq!(queue.foreground_queue.len(), 0); + assert_eq!(queue.background_queue.len(), 0); + + let instance = queue.pop_front(); + assert!(instance.is_none()); + assert!(queue.is_empty()); + } +} diff --git a/src/tui/lib/external/musicbrainz/mod.rs b/src/tui/lib/external/musicbrainz/mod.rs index a7502eb..de2c343 100644 --- a/src/tui/lib/external/musicbrainz/mod.rs +++ b/src/tui/lib/external/musicbrainz/mod.rs @@ -1,104 +1,2 @@ -//! Module for interacting with the [MusicBrainz API](https://musicbrainz.org/doc/MusicBrainz_API). - -use std::collections::HashMap; - -use musichoard::{ - collection::{ - album::{AlbumDate, AlbumMeta, AlbumSeq}, - artist::ArtistMeta, - musicbrainz::Mbid, - }, - external::musicbrainz::{ - api::{ - search::{ - SearchArtistRequest, SearchArtistResponseArtist, SearchReleaseGroupRequest, - SearchReleaseGroupResponseReleaseGroup, - }, - MusicBrainzClient, - }, - IMusicBrainzHttp, - }, -}; - -use crate::tui::lib::interface::musicbrainz::{Error, IMusicBrainz, Match}; - -// GRCOV_EXCL_START -pub struct MusicBrainz { - client: MusicBrainzClient, -} - -impl MusicBrainz { - pub fn new(client: MusicBrainzClient) -> Self { - MusicBrainz { client } - } -} - -impl IMusicBrainz for MusicBrainz { - fn search_artist(&mut self, artist: &ArtistMeta) -> Result>, Error> { - let query = SearchArtistRequest::new().string(&artist.id.name); - - let mb_response = self.client.search_artist(query)?; - - Ok(mb_response - .artists - .into_iter() - .map(from_search_artist_response_artist) - .collect()) - } - - fn search_release_group( - &mut self, - arid: &Mbid, - album: &AlbumMeta, - ) -> Result>, Error> { - // Some release groups may have a promotional early release messing up the search. Searching - // with just the year should be enough anyway. - let date = AlbumDate::new(album.date.year, None, None); - - let query = SearchReleaseGroupRequest::new() - .arid(arid) - .and() - .first_release_date(&date) - .and() - .release_group(&album.id.title); - - let mb_response = self.client.search_release_group(query)?; - - Ok(mb_response - .release_groups - .into_iter() - .map(from_search_release_group_response_release_group) - .collect()) - } -} - -fn from_search_artist_response_artist(entity: SearchArtistResponseArtist) -> Match { - Match { - score: entity.score, - item: ArtistMeta { - id: entity.name, - sort: entity.sort.map(Into::into), - musicbrainz: Some(entity.id.into()), - properties: HashMap::new(), - }, - disambiguation: entity.disambiguation, - } -} - -fn from_search_release_group_response_release_group( - entity: SearchReleaseGroupResponseReleaseGroup, -) -> Match { - Match { - score: entity.score, - item: AlbumMeta { - id: entity.title, - date: entity.first_release_date, - seq: AlbumSeq::default(), - musicbrainz: Some(entity.id.into()), - primary_type: Some(entity.primary_type), - secondary_types: entity.secondary_types.unwrap_or_default(), - }, - disambiguation: None, - } -} -// GRCOV_EXCL_STOP +pub mod api; +pub mod daemon; diff --git a/src/tui/lib/interface/musicbrainz/api/mod.rs b/src/tui/lib/interface/musicbrainz/api/mod.rs new file mode 100644 index 0000000..3c674e7 --- /dev/null +++ b/src/tui/lib/interface/musicbrainz/api/mod.rs @@ -0,0 +1,26 @@ +//! Module for accessing MusicBrainz metadata. + +#[cfg(test)] +use mockall::automock; + +use musichoard::collection::{album::AlbumMeta, artist::ArtistMeta, musicbrainz::Mbid}; + +/// Trait for interacting with the MusicBrainz API. +#[cfg_attr(test, automock)] +pub trait IMusicBrainz { + fn search_artist(&mut self, artist: &ArtistMeta) -> Result>, Error>; + fn search_release_group( + &mut self, + arid: &Mbid, + album: &AlbumMeta, + ) -> Result>, Error>; +} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct Match { + pub score: u8, + pub item: T, + pub disambiguation: Option, +} + +pub type Error = musichoard::external::musicbrainz::api::Error; diff --git a/src/tui/lib/interface/musicbrainz/daemon/mod.rs b/src/tui/lib/interface/musicbrainz/daemon/mod.rs new file mode 100644 index 0000000..7647f99 --- /dev/null +++ b/src/tui/lib/interface/musicbrainz/daemon/mod.rs @@ -0,0 +1,81 @@ +use std::{collections::VecDeque, fmt, sync::mpsc}; + +use musichoard::collection::{album::AlbumMeta, artist::ArtistMeta, musicbrainz::Mbid}; + +use crate::tui::{app::MatchStateInfo, lib::interface::musicbrainz::api::Error as MbApiError}; + +#[cfg(test)] +use mockall::automock; + +#[derive(Debug, PartialEq, Eq)] +pub enum Error { + EventChannelDisconnected, + JobChannelDisconnected, +} + +impl fmt::Display for Error { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + Error::EventChannelDisconnected => write!(f, "the event channel is disconnected"), + Error::JobChannelDisconnected => write!(f, "the job channel is disconnected"), + } + } +} + +pub type MbApiResult = Result; +pub type ResultSender = mpsc::Sender; + +#[cfg_attr(test, automock)] +pub trait IMbJobSender { + fn submit_background_job( + &self, + result_sender: ResultSender, + requests: VecDeque, + ) -> Result<(), Error>; +} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum MbParams { + Search(SearchParams), +} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum SearchParams { + Artist(SearchArtistParams), + ReleaseGroup(SearchReleaseGroupParams), +} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct SearchArtistParams { + pub artist: ArtistMeta, +} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct SearchReleaseGroupParams { + pub arid: Mbid, + pub album: AlbumMeta, +} + +impl MbParams { + pub fn search_artist(artist: ArtistMeta) -> Self { + MbParams::Search(SearchParams::Artist(SearchArtistParams { artist })) + } + + pub fn search_release_group(arid: Mbid, album: AlbumMeta) -> Self { + MbParams::Search(SearchParams::ReleaseGroup(SearchReleaseGroupParams { + arid, + album, + })) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn errors() { + assert!(!format!("{}", Error::EventChannelDisconnected).is_empty()); + assert!(!format!("{}", Error::JobChannelDisconnected).is_empty()); + } +} diff --git a/src/tui/lib/interface/musicbrainz/mod.rs b/src/tui/lib/interface/musicbrainz/mod.rs index 1bb540e..de2c343 100644 --- a/src/tui/lib/interface/musicbrainz/mod.rs +++ b/src/tui/lib/interface/musicbrainz/mod.rs @@ -1,26 +1,2 @@ -//! Module for accessing MusicBrainz metadata. - -#[cfg(test)] -use mockall::automock; - -use musichoard::collection::{album::AlbumMeta, artist::ArtistMeta, musicbrainz::Mbid}; - -/// Trait for interacting with the MusicBrainz API. -#[cfg_attr(test, automock)] -pub trait IMusicBrainz { - fn search_artist(&mut self, name: &ArtistMeta) -> Result>, Error>; - fn search_release_group( - &mut self, - arid: &Mbid, - album: &AlbumMeta, - ) -> Result>, Error>; -} - -#[derive(Clone, Debug, PartialEq, Eq)] -pub struct Match { - pub score: u8, - pub item: T, - pub disambiguation: Option, -} - -pub type Error = musichoard::external::musicbrainz::api::Error; +pub mod api; +pub mod daemon; diff --git a/src/tui/listener.rs b/src/tui/listener.rs index f49a147..5ea816b 100644 --- a/src/tui/listener.rs +++ b/src/tui/listener.rs @@ -4,7 +4,7 @@ use std::thread; #[cfg(test)] use mockall::automock; -use crate::tui::event::{Event, EventError, EventSender}; +use crate::tui::event::{EventError, IKeyEventSender}; #[cfg_attr(test, automock)] pub trait IEventListener { @@ -12,13 +12,15 @@ pub trait IEventListener { } pub struct EventListener { - events: EventSender, + event_sender: Box, } // GRCOV_EXCL_START impl EventListener { - pub fn new(events: EventSender) -> Self { - EventListener { events } + pub fn new(event_sender: ES) -> Self { + EventListener { + event_sender: Box::new(event_sender), + } } } @@ -32,7 +34,7 @@ impl IEventListener for EventListener { match event::read() { Ok(event) => { if let Err(err) = match event { - CrosstermEvent::Key(e) => self.events.send(Event::Key(e)), + CrosstermEvent::Key(e) => self.event_sender.send_key(e), _ => Ok(()), } { return err; diff --git a/src/tui/mod.rs b/src/tui/mod.rs index d7ebffd..2a2fd80 100644 --- a/src/tui/mod.rs +++ b/src/tui/mod.rs @@ -8,7 +8,10 @@ mod ui; pub use app::App; pub use event::EventChannel; pub use handler::EventHandler; -pub use lib::external::musicbrainz::MusicBrainz; +pub use lib::external::musicbrainz::{ + api::MusicBrainz, + daemon::{JobChannel, MusicBrainzDaemon}, +}; pub use listener::EventListener; pub use ui::Ui; @@ -174,8 +177,7 @@ mod testmod; mod tests { use std::{io, thread}; - use event::EventSender; - use lib::interface::musicbrainz::MockIMusicBrainz; + use lib::interface::musicbrainz::daemon::MockIMbJobSender; use ratatui::{backend::TestBackend, Terminal}; use musichoard::collection::Collection; @@ -203,12 +205,8 @@ mod tests { music_hoard } - fn events() -> EventSender { - EventChannel::new().sender() - } - fn app(collection: Collection) -> App { - App::new(music_hoard(collection), MockIMusicBrainz::new(), events()) + App::new(music_hoard(collection), MockIMbJobSender::new()) } fn listener() -> MockIEventListener { diff --git a/src/tui/ui/mod.rs b/src/tui/ui/mod.rs index b87ddfa..fbce47c 100644 --- a/src/tui/ui/mod.rs +++ b/src/tui/ui/mod.rs @@ -207,7 +207,7 @@ mod tests { use crate::tui::{ app::{AppPublic, AppPublicInner, Delta, MatchOption, MatchStatePublic}, - lib::interface::musicbrainz::Match, + lib::interface::musicbrainz::api::Match, testmod::COLLECTION, tests::terminal, };