From abc0212b28a0c584c8e1e0b6cc9ff21113b482c6 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 6 Oct 2024 15:33:55 +0200 Subject: [PATCH 01/14] Remove dead_code allows --- src/tui/lib/interface/musicbrainz/daemon/mod.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/tui/lib/interface/musicbrainz/daemon/mod.rs b/src/tui/lib/interface/musicbrainz/daemon/mod.rs index 1c54b80..eda65c9 100644 --- a/src/tui/lib/interface/musicbrainz/daemon/mod.rs +++ b/src/tui/lib/interface/musicbrainz/daemon/mod.rs @@ -59,7 +59,6 @@ pub trait IMbJobSender { pub enum MbParams { Lookup(LookupParams), Search(SearchParams), - #[allow(dead_code)] // TODO: remove with completion of #160 Browse(BrowseParams), } @@ -102,7 +101,6 @@ pub struct SearchReleaseGroupParams { #[derive(Clone, Debug, PartialEq, Eq)] pub enum BrowseParams { - #[allow(dead_code)] // TODO: remove with completion of #160 ReleaseGroup(BrowseReleaseGroupParams), } @@ -136,7 +134,6 @@ impl MbParams { })) } - #[allow(dead_code)] // TODO: to be removed by completion of #160 pub fn browse_release_group(artist: Mbid) -> Self { MbParams::Browse(BrowseParams::ReleaseGroup(BrowseReleaseGroupParams { artist, -- 2.47.1 From 07b3e66f2571125769180b2b4c6c26b577f7efb7 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 6 Oct 2024 15:43:06 +0200 Subject: [PATCH 02/14] Some renaming --- src/tui/app/machine/fetch_state.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/tui/app/machine/fetch_state.rs b/src/tui/app/machine/fetch_state.rs index e5e1de2..746f8b0 100644 --- a/src/tui/app/machine/fetch_state.rs +++ b/src/tui/app/machine/fetch_state.rs @@ -20,16 +20,16 @@ use crate::tui::{ }, }; -pub type FetchReceiver = mpsc::Receiver; +pub type MbApiReceiver = mpsc::Receiver; pub struct FetchState { - fetch_rx: FetchReceiver, - lookup_rx: Option, + search_rx: MbApiReceiver, + lookup_rx: Option, } impl FetchState { - pub fn new(fetch_rx: FetchReceiver) -> Self { + pub fn new(search_rx: MbApiReceiver) -> Self { FetchState { - fetch_rx, + search_rx, lookup_rx: None, } } @@ -43,7 +43,7 @@ impl FetchState { } } } - self.fetch_rx.try_recv() + self.search_rx.try_recv() } } -- 2.47.1 From 134165a2974b3d10254c13624d1f035b6428a7c7 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 6 Oct 2024 16:11:59 +0200 Subject: [PATCH 03/14] Some rearrangements --- src/tui/app/machine/fetch_state.rs | 61 ++++++++++++++++++------------ src/tui/app/machine/match_state.rs | 10 ++--- src/tui/app/machine/mod.rs | 4 +- 3 files changed, 44 insertions(+), 31 deletions(-) diff --git a/src/tui/app/machine/fetch_state.rs b/src/tui/app/machine/fetch_state.rs index 746f8b0..0a23524 100644 --- a/src/tui/app/machine/fetch_state.rs +++ b/src/tui/app/machine/fetch_state.rs @@ -22,28 +22,41 @@ use crate::tui::{ pub type MbApiReceiver = mpsc::Receiver; pub struct FetchState { - search_rx: MbApiReceiver, + search_rx: Option, lookup_rx: Option, + fetch_rx: Option, +} + +macro_rules! try_recv_mb_api_receiver { + ($rx:expr) => { + if let Some(rx) = &($rx) { + match rx.try_recv() { + x @ Ok(_) | x @ Err(TryRecvError::Empty) => return x, + Err(TryRecvError::Disconnected) => { + ($rx).take(); + } + } + } + }; } impl FetchState { - pub fn new(search_rx: MbApiReceiver) -> Self { + pub fn search(search_rx: MbApiReceiver) -> Self { FetchState { - search_rx, + search_rx: Some(search_rx), lookup_rx: None, + fetch_rx: None, } } fn try_recv(&mut self) -> Result { - if let Some(lookup_rx) = &self.lookup_rx { - match lookup_rx.try_recv() { - x @ Ok(_) | x @ Err(TryRecvError::Empty) => return x, - Err(TryRecvError::Disconnected) => { - self.lookup_rx.take(); - } - } + try_recv_mb_api_receiver!(self.lookup_rx); + try_recv_mb_api_receiver!(self.search_rx); + + match &self.fetch_rx { + Some(fetch_rx) => fetch_rx.try_recv(), + None => Err(TryRecvError::Disconnected), } - self.search_rx.try_recv() } } @@ -79,7 +92,7 @@ impl AppMachine { }; let (fetch_tx, fetch_rx) = mpsc::channel::(); - let fetch = FetchState::new(fetch_rx); + let fetch = FetchState::search(fetch_rx); let mb = &*inner.musicbrainz; let result = match inner.selection.category() { @@ -203,10 +216,10 @@ impl AppMachine { artist_mbid: &MbArtistRef, album: &Album, ) -> Result<(), FetchError> { - if !matches!(album.meta.info.musicbrainz, MbRefOption::None) { + let requests = Self::search_albums_requests(artist_id, artist_mbid, slice::from_ref(album)); + if requests.is_empty() { 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)?) } @@ -312,7 +325,7 @@ mod tests { let (fetch_tx, fetch_rx) = mpsc::channel(); let (lookup_tx, lookup_rx) = mpsc::channel(); - let mut fetch = FetchState::new(fetch_rx); + let mut fetch = FetchState::search(fetch_rx); fetch.lookup_rx.replace(lookup_rx); let artist = COLLECTION[3].meta.clone(); @@ -494,7 +507,7 @@ mod tests { let inner = AppInner::new(music_hoard, mb_job_sender); let (_fetch_tx, fetch_rx) = mpsc::channel(); - let fetch = FetchState::new(fetch_rx); + let fetch = FetchState::search(fetch_rx); AppMachine::app_lookup_album(inner, fetch, &artist_id, &album_id, mbid()); } @@ -548,7 +561,7 @@ mod tests { let inner = AppInner::new(music_hoard, mb_job_sender); let (_fetch_tx, fetch_rx) = mpsc::channel(); - let fetch = FetchState::new(fetch_rx); + let fetch = FetchState::search(fetch_rx); AppMachine::app_lookup_artist(inner, fetch, &artist, mbid()); } @@ -597,7 +610,7 @@ mod tests { let inner = AppInner::new(music_hoard, mb_job_sender); let (_fetch_tx, fetch_rx) = mpsc::channel(); - let fetch = FetchState::new(fetch_rx); + let fetch = FetchState::search(fetch_rx); let app = AppMachine::app_lookup_artist(inner, fetch, &artist, mbid()); assert!(matches!(app, AppState::Error(_))); @@ -615,7 +628,7 @@ mod tests { tx.send(fetch_result).unwrap(); let inner = inner(music_hoard(COLLECTION.clone())); - let fetch = FetchState::new(rx); + let fetch = FetchState::search(rx); let mut app = AppMachine::app_fetch_next(inner, fetch); assert!(matches!(app, AppState::Match(_))); @@ -638,7 +651,7 @@ mod tests { tx.send(fetch_result).unwrap(); let inner = inner(music_hoard(COLLECTION.clone())); - let fetch = FetchState::new(rx); + let fetch = FetchState::search(rx); let app = AppMachine::app_fetch_next(inner, fetch); assert!(matches!(app, AppState::Error(_))); } @@ -648,7 +661,7 @@ mod tests { let (_tx, rx) = mpsc::channel::(); let inner = inner(music_hoard(COLLECTION.clone())); - let fetch = FetchState::new(rx); + let fetch = FetchState::search(rx); let app = AppMachine::app_fetch_next(inner, fetch); assert!(matches!(app, AppState::Fetch(_))); } @@ -668,7 +681,7 @@ mod tests { collection[0].albums.clear(); let (_, rx) = mpsc::channel::(); - let fetch = FetchState::new(rx); + let fetch = FetchState::search(rx); let app = AppMachine::app_fetch_next(inner(music_hoard(collection)), fetch); assert!(matches!(app, AppState::Browse(_))); @@ -679,7 +692,7 @@ mod tests { let (tx, rx) = mpsc::channel::(); let inner = inner(music_hoard(COLLECTION.clone())); - let fetch = FetchState::new(rx); + let fetch = FetchState::search(rx); let app = AppMachine::app_fetch_next(inner, fetch); assert!(matches!(app, AppState::Fetch(_))); @@ -696,7 +709,7 @@ mod tests { fn abort() { let (_, rx) = mpsc::channel::(); - let fetch = FetchState::new(rx); + let fetch = FetchState::search(rx); let app = AppMachine::fetch_state(inner(music_hoard(COLLECTION.clone())), fetch); let app = app.abort(); diff --git a/src/tui/app/machine/match_state.rs b/src/tui/app/machine/match_state.rs index 5b453a6..8a28fd4 100644 --- a/src/tui/app/machine/match_state.rs +++ b/src/tui/app/machine/match_state.rs @@ -357,7 +357,7 @@ mod tests { fn fetch_state() -> FetchState { let (_, rx) = mpsc::channel(); - FetchState::new(rx) + FetchState::search(rx) } fn match_state(match_state_info: EntityMatches) -> MatchState { @@ -388,7 +388,7 @@ mod tests { fn match_state_flow(mut matches_info: EntityMatches, len: usize) { // tx must exist for rx to return Empty rather than Disconnected. let (_tx, rx) = mpsc::channel(); - let app_matches = MatchState::new(matches_info.clone(), FetchState::new(rx)); + let app_matches = MatchState::new(matches_info.clone(), FetchState::search(rx)); let mut music_hoard = music_hoard(vec![]); let artist_id = ArtistId::new("Artist"); @@ -487,7 +487,7 @@ mod tests { let matches_info = artist_match(); let (_tx, rx) = mpsc::channel(); - let app_matches = MatchState::new(matches_info.clone(), FetchState::new(rx)); + let app_matches = MatchState::new(matches_info.clone(), FetchState::search(rx)); let mut music_hoard = music_hoard(vec![]); match matches_info { @@ -511,7 +511,7 @@ mod tests { let matches_info = album_match(); let (_tx, rx) = mpsc::channel(); - let app_matches = MatchState::new(matches_info.clone(), FetchState::new(rx)); + let app_matches = MatchState::new(matches_info.clone(), FetchState::search(rx)); let mut music_hoard = music_hoard(vec![]); match matches_info { @@ -535,7 +535,7 @@ mod tests { let matches_info = artist_match(); let (_tx, rx) = mpsc::channel(); - let app_matches = MatchState::new(matches_info.clone(), FetchState::new(rx)); + let app_matches = MatchState::new(matches_info.clone(), FetchState::search(rx)); let mut music_hoard = music_hoard(vec![]); match matches_info { diff --git a/src/tui/app/machine/mod.rs b/src/tui/app/machine/mod.rs index 660f922..19f930c 100644 --- a/src/tui/app/machine/mod.rs +++ b/src/tui/app/machine/mod.rs @@ -493,7 +493,7 @@ mod tests { let (_, rx) = mpsc::channel(); let inner = app.unwrap_browse().inner; - let state = FetchState::new(rx); + let state = FetchState::search(rx); app = AppMachine::new(inner, state).into(); let state = app.state(); @@ -518,7 +518,7 @@ mod tests { assert!(app.is_running()); let (_, rx) = mpsc::channel(); - let fetch = FetchState::new(rx); + let fetch = FetchState::search(rx); let artist = ArtistMeta::new(ArtistId::new("Artist")); let info = EntityMatches::artist_lookup(artist.clone(), Entity::new(artist.clone())); app = -- 2.47.1 From 3763abfb1785f3f5207a98824cd47e955ff90233 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 6 Oct 2024 16:39:58 +0200 Subject: [PATCH 04/14] Streamline code --- src/tui/app/machine/fetch_state.rs | 107 +++++++++++------------------ 1 file changed, 41 insertions(+), 66 deletions(-) diff --git a/src/tui/app/machine/fetch_state.rs b/src/tui/app/machine/fetch_state.rs index 0a23524..dd8cbb2 100644 --- a/src/tui/app/machine/fetch_state.rs +++ b/src/tui/app/machine/fetch_state.rs @@ -60,17 +60,6 @@ impl FetchState { } } -enum FetchError { - NothingToFetch, - SubmitError(DaemonError), -} - -impl From for FetchError { - fn from(value: DaemonError) -> Self { - FetchError::SubmitError(value) - } -} - impl AppMachine { fn fetch_state(inner: AppInner, state: FetchState) -> Self { AppMachine::new(inner, state) @@ -81,48 +70,21 @@ 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 => { - let err = "cannot fetch artist: no artist selected"; - return AppMachine::error_state(inner, err).into(); - } + let requests = match Self::search_job(&inner) { + Ok(requests) => requests, + Err(err) => return AppMachine::error_state(inner, err.to_string()).into(), }; + if requests.is_empty() { + return AppMachine::browse_state(inner).into(); + } + let (fetch_tx, fetch_rx) = mpsc::channel::(); let fetch = FetchState::search(fetch_rx); - 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 { + match inner.musicbrainz.submit_background_job(fetch_tx, requests) { Ok(()) => AppMachine::fetch_state(inner, fetch).into(), - Err(FetchError::NothingToFetch) => AppMachine::browse_state(inner).into(), - Err(FetchError::SubmitError(daemon_err)) => { - AppMachine::error_state(inner, daemon_err.to_string()).into() - } + Err(err) => AppMachine::error_state(inner, err.to_string()).into(), } } @@ -191,36 +153,49 @@ impl AppMachine { Self::app_fetch_next(inner, fetch) } - fn submit_search_artist_job( - musicbrainz: &dyn IMbJobSender, - result_sender: ResultSender, - artist: &Artist, - ) -> Result<(), FetchError> { - let requests = match artist.meta.info.musicbrainz { + fn search_job(inner: &AppInner) -> Result, &'static str> { + let coll = inner.music_hoard.get_collection(); + + let artist = match inner.selection.state_artist(coll) { + Some(artist_state) => &coll[artist_state.index], + None => return Err("cannot fetch: no artist selected"), + }; + + let requests = match inner.selection.category() { + Category::Artist => Self::search_artist_job(artist), + _ => { + let arid = match artist.meta.info.musicbrainz { + MbRefOption::Some(ref mbref) => mbref, + _ => return Err("cannot fetch album: artist has no MBID"), + }; + let album = match inner.selection.state_album(coll) { + Some(album_state) => &artist.albums[album_state.index], + None => return Err("cannot fetch album: no album selected"), + }; + let artist_id = &artist.meta.id; + Self::search_release_group_job(artist_id, arid, album) + } + }; + + Ok(requests) + } + + fn search_artist_job(artist: &Artist) -> VecDeque { + match artist.meta.info.musicbrainz { MbRefOption::Some(ref arid) => { Self::search_albums_requests(&artist.meta.id, arid, &artist.albums) } MbRefOption::CannotHaveMbid => VecDeque::new(), MbRefOption::None => Self::search_artist_request(&artist.meta), - }; - if requests.is_empty() { - return Err(FetchError::NothingToFetch); } - Ok(musicbrainz.submit_background_job(result_sender, requests)?) } - fn submit_search_release_group_job( - musicbrainz: &dyn IMbJobSender, - result_sender: ResultSender, + fn search_release_group_job( artist_id: &ArtistId, artist_mbid: &MbArtistRef, album: &Album, - ) -> Result<(), FetchError> { - let requests = Self::search_albums_requests(artist_id, artist_mbid, slice::from_ref(album)); - if requests.is_empty() { - return Err(FetchError::NothingToFetch); - } - Ok(musicbrainz.submit_background_job(result_sender, requests)?) + ) -> VecDeque { + Self::search_albums_requests(artist_id, artist_mbid, slice::from_ref(album)) } fn search_albums_requests( -- 2.47.1 From ec61b2d50ae06183edc1ca1eb27ea9569b28632d Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 6 Oct 2024 16:49:31 +0200 Subject: [PATCH 05/14] Minor --- src/tui/app/machine/fetch_state.rs | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/src/tui/app/machine/fetch_state.rs b/src/tui/app/machine/fetch_state.rs index dd8cbb2..563ace4 100644 --- a/src/tui/app/machine/fetch_state.rs +++ b/src/tui/app/machine/fetch_state.rs @@ -49,6 +49,14 @@ impl FetchState { } } + pub fn fetch(fetch_rx: MbApiReceiver) -> Self { + FetchState { + search_rx: None, + lookup_rx: None, + fetch_rx: Some(fetch_rx), + } + } + fn try_recv(&mut self) -> Result { try_recv_mb_api_receiver!(self.lookup_rx); try_recv_mb_api_receiver!(self.search_rx); @@ -79,10 +87,10 @@ impl AppMachine { return AppMachine::browse_state(inner).into(); } - let (fetch_tx, fetch_rx) = mpsc::channel::(); - let fetch = FetchState::search(fetch_rx); + let (search_tx, search_rx) = mpsc::channel::(); + let fetch = FetchState::search(search_rx); - match inner.musicbrainz.submit_background_job(fetch_tx, requests) { + match inner.musicbrainz.submit_background_job(search_tx, requests) { Ok(()) => AppMachine::fetch_state(inner, fetch).into(), Err(err) => AppMachine::error_state(inner, err.to_string()).into(), } @@ -98,7 +106,13 @@ impl AppMachine { }, Err(recv_err) => match recv_err { TryRecvError::Empty => AppMachine::fetch_state(inner, fetch).into(), - TryRecvError::Disconnected => Self::app_fetch_new(inner), + TryRecvError::Disconnected => { + if fetch.fetch_rx.is_some() { + AppMachine::browse_state(inner).into() + } else { + Self::app_fetch_new(inner) + } + } }, } } @@ -198,6 +212,10 @@ impl AppMachine { Self::search_albums_requests(artist_id, artist_mbid, slice::from_ref(album)) } + fn search_artist_request(meta: &ArtistMeta) -> VecDeque { + VecDeque::from([MbParams::search_artist(meta.clone())]) + } + fn search_albums_requests( artist: &ArtistId, arid: &MbArtistRef, @@ -213,10 +231,6 @@ impl AppMachine { .collect() } - fn search_artist_request(meta: &ArtistMeta) -> VecDeque { - VecDeque::from([MbParams::search_artist(meta.clone())]) - } - fn submit_lookup_artist_job( musicbrainz: &dyn IMbJobSender, result_sender: ResultSender, -- 2.47.1 From 028d77928ad2ee2df3e027f96dc619c09a7d6307 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 6 Oct 2024 17:14:28 +0200 Subject: [PATCH 06/14] Submit browse jobs --- src/tui/app/machine/fetch_state.rs | 97 +++++++++++++++++++----------- 1 file changed, 62 insertions(+), 35 deletions(-) diff --git a/src/tui/app/machine/fetch_state.rs b/src/tui/app/machine/fetch_state.rs index 563ace4..7b8ef79 100644 --- a/src/tui/app/machine/fetch_state.rs +++ b/src/tui/app/machine/fetch_state.rs @@ -27,6 +27,11 @@ pub struct FetchState { fetch_rx: Option, } +struct SubmitJob { + fetch: FetchState, + requests: VecDeque, +} + macro_rules! try_recv_mb_api_receiver { ($rx:expr) => { if let Some(rx) = &($rx) { @@ -78,24 +83,62 @@ impl AppMachine { } fn app_fetch_new(inner: AppInner) -> App { - let requests = match Self::search_job(&inner) { - Ok(requests) => requests, + let (tx, rx) = mpsc::channel::(); + let job = match Self::fetch_job(&inner, rx) { + Ok(job) => job, Err(err) => return AppMachine::error_state(inner, err.to_string()).into(), }; - if requests.is_empty() { + if job.requests.is_empty() { return AppMachine::browse_state(inner).into(); } - let (search_tx, search_rx) = mpsc::channel::(); - let fetch = FetchState::search(search_rx); - - match inner.musicbrainz.submit_background_job(search_tx, requests) { - Ok(()) => AppMachine::fetch_state(inner, fetch).into(), + match inner.musicbrainz.submit_background_job(tx, job.requests) { + Ok(()) => AppMachine::fetch_state(inner, job.fetch).into(), Err(err) => AppMachine::error_state(inner, err.to_string()).into(), } } + fn fetch_job(inner: &AppInner, rx: MbApiReceiver) -> Result { + let coll = inner.music_hoard.get_collection(); + + let artist = match inner.selection.state_artist(coll) { + Some(artist_state) => &coll[artist_state.index], + None => return Err("cannot fetch: no artist selected"), + }; + + let requests = match inner.selection.category() { + Category::Artist => { + let fetch: FetchState; + let mut requests = Self::search_artist_job(artist); + if requests.is_empty() { + fetch = FetchState::fetch(rx); + requests = Self::browse_release_group_job(&artist.meta.info.musicbrainz); + } else { + fetch = FetchState::search(rx); + } + SubmitJob { fetch, requests } + } + _ => { + let arid = match artist.meta.info.musicbrainz { + MbRefOption::Some(ref mbref) => mbref, + _ => return Err("cannot fetch album: artist has no MBID"), + }; + let album = match inner.selection.state_album(coll) { + Some(album_state) => &artist.albums[album_state.index], + None => return Err("cannot fetch album: no album selected"), + }; + let artist_id = &artist.meta.id; + SubmitJob { + fetch: FetchState::search(rx), + requests: Self::search_release_group_job(artist_id, arid, album), + } + } + }; + + Ok(requests) + } + pub fn app_fetch_next(inner: AppInner, mut fetch: FetchState) -> App { match fetch.try_recv() { Ok(fetch_result) => match fetch_result { @@ -167,33 +210,6 @@ impl AppMachine { Self::app_fetch_next(inner, fetch) } - fn search_job(inner: &AppInner) -> Result, &'static str> { - let coll = inner.music_hoard.get_collection(); - - let artist = match inner.selection.state_artist(coll) { - Some(artist_state) => &coll[artist_state.index], - None => return Err("cannot fetch: no artist selected"), - }; - - let requests = match inner.selection.category() { - Category::Artist => Self::search_artist_job(artist), - _ => { - let arid = match artist.meta.info.musicbrainz { - MbRefOption::Some(ref mbref) => mbref, - _ => return Err("cannot fetch album: artist has no MBID"), - }; - let album = match inner.selection.state_album(coll) { - Some(album_state) => &artist.albums[album_state.index], - None => return Err("cannot fetch album: no album selected"), - }; - let artist_id = &artist.meta.id; - Self::search_release_group_job(artist_id, arid, album) - } - }; - - Ok(requests) - } - fn search_artist_job(artist: &Artist) -> VecDeque { match artist.meta.info.musicbrainz { MbRefOption::Some(ref arid) => { @@ -231,6 +247,17 @@ impl AppMachine { .collect() } + fn browse_release_group_job(mbopt: &MbRefOption) -> VecDeque { + match mbopt { + MbRefOption::Some(mbref) => Self::browse_release_group_request(mbref), + _ => VecDeque::new(), + } + } + + fn browse_release_group_request(mbref: &MbArtistRef) -> VecDeque { + VecDeque::from([MbParams::browse_release_group(mbref.mbid().clone())]) + } + fn submit_lookup_artist_job( musicbrainz: &dyn IMbJobSender, result_sender: ResultSender, -- 2.47.1 From dde8268eaa3084307173abec78c20f5769b6e38d Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 6 Oct 2024 17:38:25 +0200 Subject: [PATCH 07/14] Correct fetch behaviour --- src/tui/app/machine/fetch_state.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tui/app/machine/fetch_state.rs b/src/tui/app/machine/fetch_state.rs index 7b8ef79..14cd586 100644 --- a/src/tui/app/machine/fetch_state.rs +++ b/src/tui/app/machine/fetch_state.rs @@ -165,7 +165,7 @@ impl AppMachine { MbReturn::Match(next_match) => { AppMachine::match_state(inner, MatchState::new(next_match, fetch)).into() } - _ => unimplemented!(), + MbReturn::Fetch(_) => Self::app_fetch_next(inner, fetch), } } -- 2.47.1 From 12b27655e3ee7886862363faf660b2c934ad3b7f Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 6 Oct 2024 17:48:50 +0200 Subject: [PATCH 08/14] Congest the code --- src/tui/app/machine/fetch_state.rs | 35 +++++++++++++----------------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/src/tui/app/machine/fetch_state.rs b/src/tui/app/machine/fetch_state.rs index 14cd586..b08a96f 100644 --- a/src/tui/app/machine/fetch_state.rs +++ b/src/tui/app/machine/fetch_state.rs @@ -140,32 +140,27 @@ impl AppMachine { } pub fn app_fetch_next(inner: AppInner, mut fetch: FetchState) -> App { - match fetch.try_recv() { - Ok(fetch_result) => match fetch_result { - Ok(retval) => Self::handle_mb_api_return(inner, fetch, retval), - Err(fetch_err) => { - AppMachine::error_state(inner, format!("fetch failed: {fetch_err}")).into() - } - }, - Err(recv_err) => match recv_err { - TryRecvError::Empty => AppMachine::fetch_state(inner, fetch).into(), - TryRecvError::Disconnected => { + loop { + let app: App = match fetch.try_recv() { + Ok(fetch_result) => match fetch_result { + Ok(MbReturn::Match(next_match)) => { + AppMachine::match_state(inner, MatchState::new(next_match, fetch)).into() + } + Ok(MbReturn::Fetch(_)) => continue, + Err(fetch_err) => { + AppMachine::error_state(inner, format!("fetch failed: {fetch_err}")).into() + } + }, + Err(TryRecvError::Empty) => AppMachine::fetch_state(inner, fetch).into(), + Err(TryRecvError::Disconnected) => { if fetch.fetch_rx.is_some() { AppMachine::browse_state(inner).into() } else { Self::app_fetch_new(inner) } } - }, - } - } - - fn handle_mb_api_return(inner: AppInner, fetch: FetchState, retval: MbReturn) -> App { - match retval { - MbReturn::Match(next_match) => { - AppMachine::match_state(inner, MatchState::new(next_match, fetch)).into() - } - MbReturn::Fetch(_) => Self::app_fetch_next(inner, fetch), + }; + return app; } } -- 2.47.1 From 73aab07d9b5af2aaf9debb8ea52e44172120f8cd Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 6 Oct 2024 20:01:09 +0200 Subject: [PATCH 09/14] Works --- src/core/collection/album.rs | 4 +- src/core/musichoard/base.rs | 5 +++ src/core/musichoard/database.rs | 58 ++++++++++++++++++++++++++--- src/tui/app/machine/fetch_state.rs | 59 ++++++++++++++++++++++++++++-- src/tui/lib/mod.rs | 16 +++++++- 5 files changed, 132 insertions(+), 10 deletions(-) diff --git a/src/core/collection/album.rs b/src/core/collection/album.rs index 27c171e..fdccb1f 100644 --- a/src/core/collection/album.rs +++ b/src/core/collection/album.rs @@ -258,7 +258,9 @@ impl Merge for AlbumInfo { fn merge_in_place(&mut self, other: Self) { self.musicbrainz = self.musicbrainz.take().or(other.musicbrainz); self.primary_type = self.primary_type.take().or(other.primary_type); - self.secondary_types.merge_in_place(other.secondary_types); + if self.secondary_types.is_empty() { + self.secondary_types = other.secondary_types; + } } } diff --git a/src/core/musichoard/base.rs b/src/core/musichoard/base.rs index 22f0d4d..71496e9 100644 --- a/src/core/musichoard/base.rs +++ b/src/core/musichoard/base.rs @@ -34,6 +34,7 @@ pub trait IMusicHoardBasePrivate { artist_id: &ArtistId, ) -> Result<&'a mut Artist, Error>; + fn get_album<'a>(artist: &'a mut Artist, album_id: &AlbumId) -> Option<&'a Album>; fn get_album_mut<'a>(artist: &'a mut Artist, album_id: &AlbumId) -> Option<&'a mut Album>; fn get_album_mut_or_err<'a>( artist: &'a mut Artist, @@ -79,6 +80,10 @@ impl IMusicHoardBasePrivate for MusicHoard }) } + fn get_album<'a>(artist: &'a mut Artist, album_id: &AlbumId) -> Option<&'a Album> { + artist.albums.iter().find(|a| &a.meta.id == album_id) + } + fn get_album_mut<'a>(artist: &'a mut Artist, album_id: &AlbumId) -> Option<&'a mut Album> { artist.albums.iter_mut().find(|a| &a.meta.id == album_id) } diff --git a/src/core/musichoard/database.rs b/src/core/musichoard/database.rs index 606c54d..9e95ab1 100644 --- a/src/core/musichoard/database.rs +++ b/src/core/musichoard/database.rs @@ -1,7 +1,11 @@ use std::mem; use crate::{ - collection::{album::AlbumInfo, artist::ArtistInfo, merge::Merge}, + collection::{ + album::{AlbumInfo, AlbumMeta}, + artist::ArtistInfo, + merge::Merge, + }, core::{ collection::{ album::{Album, AlbumId, AlbumSeq}, @@ -57,6 +61,17 @@ pub trait IMusicHoardDatabase { property: S, ) -> Result<(), Error>; + fn add_album>( + &mut self, + artist_id: ArtistIdRef, + album_meta: AlbumMeta, + ) -> Result<(), Error>; + fn remove_album, AlbumIdRef: AsRef>( + &mut self, + artist_id: ArtistIdRef, + album_id: AlbumIdRef, + ) -> Result<(), Error>; + fn set_album_seq, AlbumIdRef: AsRef>( &mut self, artist_id: ArtistIdRef, @@ -68,15 +83,15 @@ pub trait IMusicHoardDatabase { artist_id: ArtistIdRef, album_id: AlbumIdRef, ) -> Result<(), Error>; - fn merge_album_info, AlbumIdRef: AsRef>( + fn merge_album_info, AlbumIdRef: AsRef>( &mut self, - artist_id: Id, + artist_id: ArtistIdRef, album_id: AlbumIdRef, info: AlbumInfo, ) -> Result<(), Error>; - fn clear_album_info, AlbumIdRef: AsRef>( + fn clear_album_info, AlbumIdRef: AsRef>( &mut self, - artist_id: Id, + artist_id: ArtistIdRef, album_id: AlbumIdRef, ) -> Result<(), Error>; } @@ -194,6 +209,39 @@ impl IMusicHoardDatabase for MusicHoard>( + &mut self, + artist_id: ArtistIdRef, + album_meta: AlbumMeta, + ) -> Result<(), Error> { + let album = Album { + meta: album_meta, + tracks: vec![], + }; + self.update_artist(artist_id.as_ref(), |artist| { + if Self::get_album(artist, &album.meta.id).is_none() { + artist.albums.push(album); + artist.albums.sort_unstable(); + } + }) + } + + fn remove_album, AlbumIdRef: AsRef>( + &mut self, + artist_id: ArtistIdRef, + album_id: AlbumIdRef, + ) -> Result<(), Error> { + self.update_artist(artist_id.as_ref(), |artist| { + let index_opt = artist + .albums + .iter() + .position(|a| &a.meta.id == album_id.as_ref()); + if let Some(index) = index_opt { + artist.albums.remove(index); + } + }) + } + fn set_album_seq, AlbumIdRef: AsRef>( &mut self, artist_id: ArtistIdRef, diff --git a/src/tui/app/machine/fetch_state.rs b/src/tui/app/machine/fetch_state.rs index b08a96f..1015d44 100644 --- a/src/tui/app/machine/fetch_state.rs +++ b/src/tui/app/machine/fetch_state.rs @@ -13,10 +13,12 @@ use musichoard::collection::{ use crate::tui::{ app::{ machine::{match_state::MatchState, App, AppInner, AppMachine}, + selection::KeySelection, AppPublicState, AppState, Category, IAppEventFetch, IAppInteractFetch, }, lib::interface::musicbrainz::daemon::{ - Error as DaemonError, IMbJobSender, MbApiResult, MbParams, MbReturn, ResultSender, + EntityList, Error as DaemonError, IMbJobSender, MbApiResult, MbParams, MbReturn, + ResultSender, }, }; @@ -139,14 +141,19 @@ impl AppMachine { Ok(requests) } - pub fn app_fetch_next(inner: AppInner, mut fetch: FetchState) -> App { + pub fn app_fetch_next(mut inner: AppInner, mut fetch: FetchState) -> App { loop { let app: App = match fetch.try_recv() { Ok(fetch_result) => match fetch_result { Ok(MbReturn::Match(next_match)) => { AppMachine::match_state(inner, MatchState::new(next_match, fetch)).into() } - Ok(MbReturn::Fetch(_)) => continue, + Ok(MbReturn::Fetch(list)) => { + match Self::apply_fetch_results(&mut inner, list) { + Ok(()) => continue, + Err(err) => AppMachine::error_state(inner, err.to_string()).into(), + } + } Err(fetch_err) => { AppMachine::error_state(inner, format!("fetch failed: {fetch_err}")).into() } @@ -164,6 +171,52 @@ impl AppMachine { } } + fn apply_fetch_results( + inner: &mut AppInner, + list: EntityList, + ) -> Result<(), musichoard::Error> { + match list { + EntityList::Album(new_albums) => { + let coll = inner.music_hoard.get_collection(); + let artist_state = inner.selection.state_artist(coll).unwrap(); + + let artist = &coll[artist_state.index]; + let artist_id = &coll[artist_state.index].meta.id.clone(); + let old_albums = &artist.albums; + + let mut to_be_added = vec![]; + for new in new_albums.into_iter() { + let mut exists = false; + for old in old_albums.iter() { + if matches!(old.meta.info.musicbrainz, MbRefOption::Some(_)) { + if new.info.musicbrainz == old.meta.info.musicbrainz { + exists = true; + break; + } + } + } + + if !exists { + to_be_added.push(new); + } + } + + let previous = + KeySelection::get(inner.music_hoard.get_collection(), &inner.selection); + + for new in to_be_added.into_iter() { + inner.music_hoard.add_album(&artist_id, new)?; + } + + inner + .selection + .select_by_id(inner.music_hoard.get_collection(), previous); + + Ok(()) + } + } + } + pub fn app_lookup_artist( inner: AppInner, fetch: FetchState, diff --git a/src/tui/lib/mod.rs b/src/tui/lib/mod.rs index 59cfd41..7470a3a 100644 --- a/src/tui/lib/mod.rs +++ b/src/tui/lib/mod.rs @@ -3,7 +3,7 @@ pub mod interface; use musichoard::{ collection::{ - album::{AlbumId, AlbumInfo}, + album::{AlbumId, AlbumInfo, AlbumMeta}, artist::{ArtistId, ArtistInfo}, Collection, }, @@ -20,6 +20,12 @@ pub trait IMusicHoard { fn reload_database(&mut self) -> Result<(), musichoard::Error>; fn get_collection(&self) -> &Collection; + fn add_album( + &mut self, + artist_id: &ArtistId, + album_meta: AlbumMeta, + ) -> Result<(), musichoard::Error>; + fn merge_artist_info( &mut self, id: &ArtistId, @@ -47,6 +53,14 @@ impl IMusicHoard for MusicHoard::get_collection(self) } + fn add_album( + &mut self, + artist_id: &ArtistId, + album_meta: AlbumMeta, + ) -> Result<(), musichoard::Error> { + ::add_album(self, artist_id, album_meta) + } + fn merge_artist_info( &mut self, id: &ArtistId, -- 2.47.1 From 799de9c003ea74e023e9b7563ceff7c4e393f45d Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 6 Oct 2024 22:15:46 +0200 Subject: [PATCH 10/14] Clean up a bit --- src/core/collection/musicbrainz.rs | 8 ++++ src/tui/app/machine/fetch_state.rs | 65 +++++++++++++++--------------- 2 files changed, 40 insertions(+), 33 deletions(-) diff --git a/src/core/collection/musicbrainz.rs b/src/core/collection/musicbrainz.rs index 575778a..3b24af5 100644 --- a/src/core/collection/musicbrainz.rs +++ b/src/core/collection/musicbrainz.rs @@ -46,6 +46,14 @@ pub enum MbRefOption { } impl MbRefOption { + pub fn is_some(&self) -> bool { + matches!(self, MbRefOption::Some(_)) + } + + pub fn is_none(&self) -> bool { + matches!(self, MbRefOption::None) + } + pub fn or(self, optb: MbRefOption) -> MbRefOption { match (&self, &optb) { (MbRefOption::Some(_), _) | (MbRefOption::CannotHaveMbid, MbRefOption::None) => self, diff --git a/src/tui/app/machine/fetch_state.rs b/src/tui/app/machine/fetch_state.rs index 1015d44..f10e15a 100644 --- a/src/tui/app/machine/fetch_state.rs +++ b/src/tui/app/machine/fetch_state.rs @@ -5,7 +5,7 @@ use std::{ }; use musichoard::collection::{ - album::{Album, AlbumId}, + album::{Album, AlbumId, AlbumMeta}, artist::{Artist, ArtistId, ArtistMeta}, musicbrainz::{IMusicBrainzRef, MbArtistRef, MbRefOption, Mbid}, }; @@ -176,45 +176,44 @@ impl AppMachine { list: EntityList, ) -> Result<(), musichoard::Error> { match list { - EntityList::Album(new_albums) => { - let coll = inner.music_hoard.get_collection(); - let artist_state = inner.selection.state_artist(coll).unwrap(); + EntityList::Album(fetch_albums) => Self::apply_album_results(inner, fetch_albums), + } + } - let artist = &coll[artist_state.index]; - let artist_id = &coll[artist_state.index].meta.id.clone(); - let old_albums = &artist.albums; + fn apply_album_results( + inner: &mut AppInner, + fetch_albums: Vec, + ) -> Result<(), musichoard::Error> { + let coll = inner.music_hoard.get_collection(); - let mut to_be_added = vec![]; - for new in new_albums.into_iter() { - let mut exists = false; - for old in old_albums.iter() { - if matches!(old.meta.info.musicbrainz, MbRefOption::Some(_)) { - if new.info.musicbrainz == old.meta.info.musicbrainz { - exists = true; - break; - } - } - } + let artist_state = inner.selection.state_artist(coll).unwrap(); + let artist = &coll[artist_state.index]; + let selection = KeySelection::get(coll, &inner.selection); - if !exists { - to_be_added.push(new); - } - } + let artist_id = &artist.meta.id.clone(); + for new in Self::new_albums(fetch_albums, &artist.albums).into_iter() { + inner.music_hoard.add_album(artist_id, new)?; + } - let previous = - KeySelection::get(inner.music_hoard.get_collection(), &inner.selection); + let coll = inner.music_hoard.get_collection(); + inner.selection.select_by_id(coll, selection); - for new in to_be_added.into_iter() { - inner.music_hoard.add_album(&artist_id, new)?; - } + Ok(()) + } - inner - .selection - .select_by_id(inner.music_hoard.get_collection(), previous); - - Ok(()) + fn new_albums(fetch_albums: Vec, albums: &[Album]) -> Vec { + let mut new_albums = vec![]; + for alb in fetch_albums.into_iter() { + let existing = albums.iter().find(|old| Self::album_match(&old.meta, &alb)); + if existing.is_none() { + new_albums.push(alb); } } + new_albums + } + + fn album_match(old: &AlbumMeta, new: &AlbumMeta) -> bool { + old.info.musicbrainz.is_some() && (old.info.musicbrainz == new.info.musicbrainz) } pub fn app_lookup_artist( @@ -288,7 +287,7 @@ impl AppMachine { let arid = arid.mbid(); albums .iter() - .filter(|album| matches!(album.meta.info.musicbrainz, MbRefOption::None)) + .filter(|album| album.meta.info.musicbrainz.is_none()) .map(|album| { MbParams::search_release_group(artist.clone(), arid.clone(), album.meta.clone()) }) -- 2.47.1 From 264a3d52cb38fda2f4ddc5f0dd358855a927c2f2 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Mon, 30 Dec 2024 20:17:53 +0100 Subject: [PATCH 11/14] Fix unit tests --- src/tui/app/machine/fetch_state.rs | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/tui/app/machine/fetch_state.rs b/src/tui/app/machine/fetch_state.rs index f10e15a..12fac9e 100644 --- a/src/tui/app/machine/fetch_state.rs +++ b/src/tui/app/machine/fetch_state.rs @@ -370,7 +370,7 @@ mod tests { use crate::tui::{ app::{ - machine::tests::{inner, music_hoard}, + machine::tests::{inner, inner_with_mb, music_hoard}, Delta, EntityMatches, IApp, IAppAccess, IAppInteractBrowse, MatchOption, }, lib::interface::musicbrainz::{self, api::Entity, daemon::MockIMbJobSender}, @@ -734,8 +734,18 @@ mod tests { let mut collection = COLLECTION.clone(); collection[0].albums.clear(); - let app = AppMachine::app_fetch_first(inner(music_hoard(collection))); - assert!(matches!(app, AppState::Browse(_))); + let requests = AppMachine::browse_release_group_job(&collection[0].meta.info.musicbrainz); + + let mut mb_job_sender = MockIMbJobSender::new(); + mb_job_sender + .expect_submit_background_job() + .with(predicate::always(), predicate::eq(requests)) + .times(1) + .return_once(|_, _| Ok(())); + + let inner = inner_with_mb(music_hoard(collection), mb_job_sender); + let app = AppMachine::app_fetch_first(inner); + assert!(matches!(app, AppState::Fetch(_))); } #[test] @@ -743,11 +753,11 @@ mod tests { let mut collection = COLLECTION.clone(); collection[0].albums.clear(); - let (_, rx) = mpsc::channel::(); + let (_tx, rx) = mpsc::channel::(); let fetch = FetchState::search(rx); let app = AppMachine::app_fetch_next(inner(music_hoard(collection)), fetch); - assert!(matches!(app, AppState::Browse(_))); + assert!(matches!(app, AppState::Fetch(_))); } #[test] -- 2.47.1 From 6464f8300dc483615cf5b79da91c9012198c94eb Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Mon, 30 Dec 2024 22:00:31 +0100 Subject: [PATCH 12/14] Unit test core --- src/core/collection/album.rs | 51 +++++++++++++-------- src/core/musichoard/database.rs | 72 ++++++++++++++++++++++++------ src/core/musichoard/library.rs | 2 +- src/tui/app/machine/match_state.rs | 12 +++-- src/tui/ui/mod.rs | 14 +++--- 5 files changed, 103 insertions(+), 48 deletions(-) diff --git a/src/core/collection/album.rs b/src/core/collection/album.rs index fdccb1f..762a8e0 100644 --- a/src/core/collection/album.rs +++ b/src/core/collection/album.rs @@ -144,17 +144,26 @@ impl AlbumStatus { } impl Album { - pub fn new, Date: Into>( - id: Id, - date: Date, + pub fn new>(id: Id) -> Self { + Album { + meta: AlbumMeta::new(id), + tracks: vec![], + } + } + + pub fn with_date>(mut self, date: Date) -> Self { + self.meta.date = date.into(); + self + } + + pub fn with_type, Date: Into>( + mut self, primary_type: Option, secondary_types: Vec, ) -> Self { - let info = AlbumInfo::new(MbRefOption::None, primary_type, secondary_types); - Album { - meta: AlbumMeta::new(id, date, info), - tracks: vec![], - } + self.meta.info.primary_type = primary_type; + self.meta.info.secondary_types = secondary_types; + self } pub fn get_status(&self) -> AlbumStatus { @@ -183,19 +192,25 @@ impl Merge for Album { } impl AlbumMeta { - pub fn new, Date: Into>( - id: Id, - date: Date, - info: AlbumInfo, - ) -> Self { + pub fn new>(id: Id) -> Self { AlbumMeta { id: id.into(), - date: date.into(), + date: AlbumDate::default(), seq: AlbumSeq::default(), - info, + info: AlbumInfo::default(), } } + pub fn with_date>(mut self, date: Date) -> Self { + self.date = date.into(); + self + } + + pub fn with_info(mut self, info: AlbumInfo) -> Self { + self.info = info; + self + } + pub fn get_sort_key(&self) -> (&AlbumDate, &AlbumSeq, &AlbumId) { (&self.date, &self.seq, &self.id) } @@ -313,13 +328,13 @@ mod tests { let album_id_1 = AlbumId { title: String::from("album z"), }; - let mut album_1 = Album::new(album_id_1, date.clone(), None, vec![]); + let mut album_1 = Album::new(album_id_1).with_date(date.clone()); album_1.meta.set_seq(AlbumSeq(1)); let album_id_2 = AlbumId { title: String::from("album a"), }; - let mut album_2 = Album::new(album_id_2, date.clone(), None, vec![]); + let mut album_2 = Album::new(album_id_2).with_date(date.clone()); album_2.meta.set_seq(AlbumSeq(2)); assert_ne!(album_1, album_2); @@ -329,7 +344,7 @@ mod tests { #[test] fn set_clear_seq() { - let mut album = Album::new("An album", AlbumDate::default(), None, vec![]); + let mut album = Album::new("An album"); assert_eq!(album.meta.seq, AlbumSeq(0)); diff --git a/src/core/musichoard/database.rs b/src/core/musichoard/database.rs index 9e95ab1..21837d1 100644 --- a/src/core/musichoard/database.rs +++ b/src/core/musichoard/database.rs @@ -392,7 +392,7 @@ mod tests { musicbrainz::{MbArtistRef, MbRefOption}, }, core::{ - collection::{album::AlbumDate, artist::ArtistId}, + collection::artist::ArtistId, interface::database::{self, MockIDatabase}, musichoard::{base::IMusicHoardBase, NoLibrary}, testmod::FULL_COLLECTION, @@ -652,6 +652,62 @@ mod tests { assert!(music_hoard.collection[0].meta.info.properties.is_empty()); } + #[test] + fn album_new_delete() { + let album_id = AlbumId::new("an album"); + let album_meta = AlbumMeta::new(album_id.clone()); + let album_id_2 = AlbumId::new("another album"); + let album_meta_2 = AlbumMeta::new(album_id_2); + + let collection = FULL_COLLECTION.to_owned(); + let artist_id = collection[0].meta.id.clone(); + let mut with_album = collection.clone(); + with_album[0].albums.push(Album::new(album_id)); + with_album[0].albums.sort_unstable(); + + let mut database = MockIDatabase::new(); + let mut seq = Sequence::new(); + database + .expect_load() + .times(1) + .times(1) + .in_sequence(&mut seq) + .returning(|| Ok(FULL_COLLECTION.to_owned())); + database + .expect_save() + .times(1) + .in_sequence(&mut seq) + .with(predicate::eq(with_album.clone())) + .returning(|_| Ok(())); + database + .expect_save() + .times(1) + .in_sequence(&mut seq) + .with(predicate::eq(collection.clone())) + .returning(|_| Ok(())); + + let mut music_hoard = MusicHoard::database(database).unwrap(); + assert_eq!(music_hoard.collection, collection); + + assert!(music_hoard + .add_album(&artist_id, album_meta.clone()) + .is_ok()); + assert_eq!(music_hoard.collection, with_album); + + assert!(music_hoard + .add_album(&artist_id, album_meta.clone()) + .is_ok()); + assert_eq!(music_hoard.collection, with_album); + + assert!(music_hoard + .remove_album(&artist_id, &album_meta_2.id) + .is_ok()); + assert_eq!(music_hoard.collection, with_album); + + assert!(music_hoard.remove_album(&artist_id, &album_meta.id).is_ok()); + assert_eq!(music_hoard.collection, collection); + } + #[test] fn set_clear_album_seq() { let mut database = MockIDatabase::new(); @@ -661,12 +717,7 @@ mod tests { let album_id_2 = AlbumId::new("another album"); let mut database_result = vec![Artist::new(artist_id.clone())]; - database_result[0].albums.push(Album::new( - album_id.clone(), - AlbumDate::default(), - None, - vec![], - )); + database_result[0].albums.push(Album::new(album_id.clone())); database .expect_load() @@ -706,12 +757,7 @@ mod tests { let album_id_2 = AlbumId::new("another album"); let mut database_result = vec![Artist::new(artist_id.clone())]; - database_result[0].albums.push(Album::new( - album_id.clone(), - AlbumDate::default(), - None, - vec![], - )); + database_result[0].albums.push(Album::new(album_id.clone())); database .expect_load() diff --git a/src/core/musichoard/library.rs b/src/core/musichoard/library.rs index 36b5f1e..5272b6d 100644 --- a/src/core/musichoard/library.rs +++ b/src/core/musichoard/library.rs @@ -109,7 +109,7 @@ impl MusicHoard { { Some(album) => album.tracks.push(track), None => { - let mut album = Album::new(album_id, album_date, None, vec![]); + let mut album = Album::new(album_id).with_date(album_date); album.tracks.push(track); artist.albums.push(album); } diff --git a/src/tui/app/machine/match_state.rs b/src/tui/app/machine/match_state.rs index 8a28fd4..f339f60 100644 --- a/src/tui/app/machine/match_state.rs +++ b/src/tui/app/machine/match_state.rs @@ -319,15 +319,13 @@ mod tests { } fn album_meta(id: AlbumId) -> AlbumMeta { - AlbumMeta::new( - id, - AlbumDate::new(Some(1990), Some(5), None), - AlbumInfo::new( + AlbumMeta::new(id) + .with_date(AlbumDate::new(Some(1990), Some(5), None)) + .with_info(AlbumInfo::new( MbRefOption::Some(mbid().into()), Some(AlbumPrimaryType::Album), vec![AlbumSecondaryType::Live, AlbumSecondaryType::Compilation], - ), - ) + )) } fn album_match() -> EntityMatches { @@ -625,7 +623,7 @@ mod tests { fn select_manual_input_album() { let mut mb_job_sender = MockIMbJobSender::new(); let artist_id = ArtistId::new("Artist"); - let album = AlbumMeta::new("Album", 1990, AlbumInfo::default()); + let album = AlbumMeta::new("Album").with_date(1990); let requests = VecDeque::from([MbParams::lookup_release_group( artist_id.clone(), album.id.clone(), diff --git a/src/tui/ui/mod.rs b/src/tui/ui/mod.rs index 7c21c68..3258e86 100644 --- a/src/tui/ui/mod.rs +++ b/src/tui/ui/mod.rs @@ -289,9 +289,7 @@ mod tests { #[test] fn empty_album() { let mut artists: Vec = vec![Artist::new(ArtistId::new("An artist"))]; - artists[0] - .albums - .push(Album::new("An album", AlbumDate::default(), None, vec![])); + artists[0].albums.push(Album::new("An album")); let mut selection = Selection::new(&artists); draw_test_suite(&artists, &mut selection); @@ -361,15 +359,13 @@ mod tests { } fn album_meta(id: AlbumId) -> AlbumMeta { - AlbumMeta::new( - id, - AlbumDate::new(Some(1990), Some(5), None), - AlbumInfo::new( + AlbumMeta::new(id) + .with_date(AlbumDate::new(Some(1990), Some(5), None)) + .with_info(AlbumInfo::new( MbRefOption::None, Some(AlbumPrimaryType::Album), vec![AlbumSecondaryType::Live, AlbumSecondaryType::Compilation], - ), - ) + )) } fn album_matches() -> EntityMatches { -- 2.47.1 From 62918ff3ca894976a1e62991f6a92840f45e561a Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Mon, 30 Dec 2024 23:15:00 +0100 Subject: [PATCH 13/14] Add a unit test --- src/tui/app/machine/fetch_state.rs | 31 +++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/src/tui/app/machine/fetch_state.rs b/src/tui/app/machine/fetch_state.rs index 12fac9e..862dd65 100644 --- a/src/tui/app/machine/fetch_state.rs +++ b/src/tui/app/machine/fetch_state.rs @@ -729,20 +729,23 @@ mod tests { assert!(matches!(app, AppState::Fetch(_))); } - #[test] - fn recv_err_empty_first() { - let mut collection = COLLECTION.clone(); - collection[0].albums.clear(); - - let requests = AppMachine::browse_release_group_job(&collection[0].meta.info.musicbrainz); - + fn browse_release_group_expectation(artist: &Artist) -> MockIMbJobSender { + let requests = AppMachine::browse_release_group_job(&artist.meta.info.musicbrainz); let mut mb_job_sender = MockIMbJobSender::new(); mb_job_sender .expect_submit_background_job() .with(predicate::always(), predicate::eq(requests)) .times(1) .return_once(|_, _| Ok(())); + mb_job_sender + } + #[test] + fn recv_err_empty_first() { + let mut collection = COLLECTION.clone(); + collection[0].albums.clear(); + + let mb_job_sender = browse_release_group_expectation(&collection[0]); let inner = inner_with_mb(music_hoard(collection), mb_job_sender); let app = AppMachine::app_fetch_first(inner); assert!(matches!(app, AppState::Fetch(_))); @@ -760,6 +763,20 @@ mod tests { assert!(matches!(app, AppState::Fetch(_))); } + #[test] + fn recv_err_disconnected_next() { + let mut collection = COLLECTION.clone(); + collection[0].albums.clear(); + + let (_, rx) = mpsc::channel::(); + let fetch = FetchState::search(rx); + + let mb_job_sender = browse_release_group_expectation(&collection[0]); + let inner = inner_with_mb(music_hoard(collection), mb_job_sender); + let app = AppMachine::app_fetch_next(inner, fetch); + assert!(matches!(app, AppState::Fetch(_))); + } + #[test] fn empty_first_then_ready() { let (tx, rx) = mpsc::channel::(); -- 2.47.1 From c162d399d8b3b4fde1392d260219c183c3cd6d57 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Mon, 30 Dec 2024 23:37:54 +0100 Subject: [PATCH 14/14] Complete code coverage --- src/core/collection/album.rs | 10 ----- src/tui/app/machine/fetch_state.rs | 60 ++++++++++++++++++++++++++++-- 2 files changed, 57 insertions(+), 13 deletions(-) diff --git a/src/core/collection/album.rs b/src/core/collection/album.rs index 762a8e0..6356692 100644 --- a/src/core/collection/album.rs +++ b/src/core/collection/album.rs @@ -156,16 +156,6 @@ impl Album { self } - pub fn with_type, Date: Into>( - mut self, - primary_type: Option, - secondary_types: Vec, - ) -> Self { - self.meta.info.primary_type = primary_type; - self.meta.info.secondary_types = secondary_types; - self - } - pub fn get_status(&self) -> AlbumStatus { AlbumStatus::from_tracks(&self.tracks) } diff --git a/src/tui/app/machine/fetch_state.rs b/src/tui/app/machine/fetch_state.rs index 862dd65..2029fd5 100644 --- a/src/tui/app/machine/fetch_state.rs +++ b/src/tui/app/machine/fetch_state.rs @@ -680,7 +680,7 @@ mod tests { } #[test] - fn recv_ok_fetch_ok() { + fn recv_ok_match_ok() { let (tx, rx) = mpsc::channel::(); let artist = COLLECTION[3].meta.clone(); @@ -707,7 +707,7 @@ mod tests { } #[test] - fn recv_ok_fetch_err() { + fn recv_ok_search_err() { let (tx, rx) = mpsc::channel::(); let fetch_result = Err(musicbrainz::api::Error::RateLimit); @@ -719,6 +719,60 @@ mod tests { assert!(matches!(app, AppState::Error(_))); } + #[test] + fn recv_ok_fetch_ok() { + let collection = COLLECTION.clone(); + + let (tx, rx) = mpsc::channel::(); + let fetch = FetchState::fetch(rx); + + let artist_id = collection[0].meta.id.clone(); + let old_album = collection[0].albums[0].meta.clone(); + let new_album = AlbumMeta::new(AlbumId::new("some new album")); + + let release_group_fetch = EntityList::Album(vec![old_album.clone(), new_album.clone()]); + let fetch_result = Ok(MbReturn::Fetch(release_group_fetch)); + tx.send(fetch_result).unwrap(); + drop(tx); + + let mut music_hoard = music_hoard(collection); + music_hoard + .expect_add_album() + .with(predicate::eq(artist_id), predicate::eq(new_album)) + .times(1) + .return_once(|_, _| Ok(())); + + let app = AppMachine::app_fetch_next(inner(music_hoard), fetch); + assert!(matches!(app, AppState::Browse(_))); + } + + #[test] + fn recv_ok_fetch_ok_add_album_err() { + let collection = COLLECTION.clone(); + + let (tx, rx) = mpsc::channel::(); + let fetch = FetchState::fetch(rx); + + let artist_id = collection[0].meta.id.clone(); + let old_album = collection[0].albums[0].meta.clone(); + let new_album = AlbumMeta::new(AlbumId::new("some new album")); + + let release_group_fetch = EntityList::Album(vec![old_album.clone(), new_album.clone()]); + let fetch_result = Ok(MbReturn::Fetch(release_group_fetch)); + tx.send(fetch_result).unwrap(); + drop(tx); + + let mut music_hoard = music_hoard(collection); + music_hoard + .expect_add_album() + .with(predicate::eq(artist_id), predicate::eq(new_album)) + .times(1) + .return_once(|_, _| Err(musichoard::Error::CollectionError(String::from("get rekt")))); + + let app = AppMachine::app_fetch_next(inner(music_hoard), fetch); + assert!(matches!(app, AppState::Error(_))); + } + #[test] fn recv_err_empty() { let (_tx, rx) = mpsc::channel::(); @@ -764,7 +818,7 @@ mod tests { } #[test] - fn recv_err_disconnected_next() { + fn recv_err_disconnected_search_next() { let mut collection = COLLECTION.clone(); collection[0].albums.clear(); -- 2.47.1