From 0d7e6bb5558dc3f5523ecc4c43b26e83786d5720 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 29 Sep 2024 12:38:38 +0200 Subject: [PATCH] Allow fetching of a single album (#226) Closes #225 Reviewed-on: https://git.thenineworlds.net/wojtek/musichoard/pulls/226 --- src/tui/app/machine/fetch_state.rs | 132 +++++++++++++++++++++++++++-- 1 file changed, 124 insertions(+), 8 deletions(-) diff --git a/src/tui/app/machine/fetch_state.rs b/src/tui/app/machine/fetch_state.rs index 74c9b2c..c8a15b3 100644 --- a/src/tui/app/machine/fetch_state.rs +++ b/src/tui/app/machine/fetch_state.rs @@ -1,5 +1,6 @@ use std::{ collections::VecDeque, + slice, sync::mpsc::{self, TryRecvError}, }; @@ -12,7 +13,7 @@ use musichoard::collection::{ use crate::tui::{ app::{ machine::{match_state::MatchState, App, AppInner, AppMachine}, - AppPublicState, AppState, IAppEventFetch, IAppInteractFetch, + AppPublicState, AppState, Category, IAppEventFetch, IAppInteractFetch, }, lib::interface::musicbrainz::daemon::{ Error as DaemonError, IMbJobSender, MbApiResult, MbParams, ResultSender, @@ -68,16 +69,42 @@ impl AppMachine { 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_state(inner, "cannot fetch: no artist selected").into() + let err = "cannot fetch artist: no artist selected"; + return AppMachine::error_state(inner, err).into(); } }; let (fetch_tx, fetch_rx) = mpsc::channel::(); let fetch = FetchState::new(fetch_rx); - match Self::submit_fetch_job(&*inner.musicbrainz, fetch_tx, artist) { + + let mb = &*inner.musicbrainz; + let result = match inner.selection.category() { + Category::Artist => Self::submit_search_artist_job(mb, fetch_tx, artist), + _ => { + let arid = match artist.meta.info.musicbrainz { + MbRefOption::Some(ref mbref) => mbref, + _ => { + let err = "cannot fetch album: artist has no MBID"; + return AppMachine::error_state(inner, err).into(); + } + }; + let album = match inner.selection.state_album(coll) { + Some(album_state) => &artist.albums[album_state.index], + None => { + let err = "cannot fetch album: no album selected"; + return AppMachine::error_state(inner, err).into(); + } + }; + let artist_id = &artist.meta.id; + Self::submit_search_release_group_job(mb, fetch_tx, artist_id, arid, album) + } + }; + + match result { Ok(()) => AppMachine::fetch_state(inner, fetch).into(), Err(FetchError::NothingToFetch) => AppMachine::browse_state(inner).into(), Err(FetchError::SubmitError(daemon_err)) => { @@ -144,17 +171,17 @@ impl AppMachine { Self::app_fetch_next(inner, fetch) } - fn submit_fetch_job( + fn submit_search_artist_job( musicbrainz: &dyn IMbJobSender, result_sender: ResultSender, artist: &Artist, ) -> Result<(), FetchError> { let requests = match artist.meta.info.musicbrainz { MbRefOption::Some(ref arid) => { - Self::fetch_albums_requests(&artist.meta.id, arid, &artist.albums) + Self::search_albums_requests(&artist.meta.id, arid, &artist.albums) } MbRefOption::CannotHaveMbid => VecDeque::new(), - MbRefOption::None => Self::fetch_artist_request(&artist.meta), + MbRefOption::None => Self::search_artist_request(&artist.meta), }; if requests.is_empty() { return Err(FetchError::NothingToFetch); @@ -162,7 +189,21 @@ impl AppMachine { Ok(musicbrainz.submit_background_job(result_sender, requests)?) } - fn fetch_albums_requests( + fn submit_search_release_group_job( + musicbrainz: &dyn IMbJobSender, + result_sender: ResultSender, + artist_id: &ArtistId, + artist_mbid: &MbArtistRef, + album: &Album, + ) -> Result<(), FetchError> { + if !matches!(album.meta.info.musicbrainz, MbRefOption::None) { + return Err(FetchError::NothingToFetch); + } + let requests = Self::search_albums_requests(artist_id, artist_mbid, slice::from_ref(album)); + Ok(musicbrainz.submit_background_job(result_sender, requests)?) + } + + fn search_albums_requests( artist: &ArtistId, arid: &MbArtistRef, albums: &[Album], @@ -177,7 +218,7 @@ impl AppMachine { .collect() } - fn fetch_artist_request(meta: &ArtistMeta) -> VecDeque { + fn search_artist_request(meta: &ArtistMeta) -> VecDeque { VecDeque::from([MbParams::search_artist(meta.clone())]) } @@ -317,6 +358,81 @@ mod tests { .return_once(|_, _| Ok(())); } + #[test] + fn fetch_single_album() { + let mut mb_job_sender = MockIMbJobSender::new(); + + let artist_id = COLLECTION[1].meta.id.clone(); + let artist_mbid: Mbid = "11111111-1111-1111-1111-111111111111".try_into().unwrap(); + + let album_meta = COLLECTION[1].albums[0].meta.clone(); + + search_release_group_expectation( + &mut mb_job_sender, + &artist_id, + &artist_mbid, + &[album_meta], + ); + + let music_hoard = music_hoard(COLLECTION.to_owned()); + let inner = AppInner::new(music_hoard, mb_job_sender); + + // Use second artist and have album selected to match the expectation. + let browse = AppMachine::browse_state(inner); + let browse = browse.increment_selection(Delta::Line).unwrap_browse(); + let app = browse.increment_category(); + + let app = app.unwrap_browse().fetch_musicbrainz(); + assert!(matches!(app, AppState::Fetch(_))); + } + + #[test] + fn fetch_single_album_nothing_to_fetch() { + let music_hoard = music_hoard(COLLECTION.to_owned()); + let inner = inner(music_hoard); + + // Use second artist, have second album selected (has MBID) to match the expectation. + let browse = AppMachine::browse_state(inner); + let browse = browse.increment_selection(Delta::Line).unwrap_browse(); + let browse = browse.increment_category().unwrap_browse(); + let app = browse.increment_selection(Delta::Line); + + let app = app.unwrap_browse().fetch_musicbrainz(); + assert!(matches!(app, AppState::Browse(_))); + } + + #[test] + fn fetch_single_album_no_artist_mbid() { + let music_hoard = music_hoard(COLLECTION.to_owned()); + let inner = inner(music_hoard); + + // Use third artist and have album selected 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_category(); + + let app = app.unwrap_browse().fetch_musicbrainz(); + assert!(matches!(app, AppState::Error(_))); + } + + #[test] + fn fetch_single_album_no_album() { + let mut collection = COLLECTION.to_owned(); + collection[1].albums.clear(); + + let music_hoard = music_hoard(collection); + let inner = inner(music_hoard); + + // Use second artist and have album selected to match the expectation. + let browse = AppMachine::browse_state(inner); + let browse = browse.increment_selection(Delta::Line).unwrap_browse(); + let app = browse.increment_category(); + + let app = app.unwrap_browse().fetch_musicbrainz(); + assert!(matches!(app, AppState::Error(_))); + } + #[test] fn fetch_albums() { let mut mb_job_sender = MockIMbJobSender::new();