From 5d510ff7878231b98946df21b7684a7719a65d72 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 6 Oct 2024 15:32:46 +0200 Subject: [PATCH] Integrate browse API into TUI MB daemon (#230) Part 2 of #160 Reviewed-on: https://git.thenineworlds.net/wojtek/musichoard/pulls/230 --- .gitea/workflows/gitea-ci.yaml | 2 +- README.md | 2 +- examples/musicbrainz_api/browse.rs | 14 +- src/core/collection/mod.rs | 4 +- src/core/collection/musicbrainz.rs | 6 +- src/core/musichoard/base.rs | 3 +- src/core/musichoard/database.rs | 34 ++- src/external/musicbrainz/api/browse.rs | 12 +- src/external/musicbrainz/api/mod.rs | 41 ++- src/external/musicbrainz/api/search/mod.rs | 12 +- src/tui/app/machine/fetch_state.rs | 65 +++-- src/tui/app/machine/match_state.rs | 266 ++++++++---------- src/tui/app/machine/mod.rs | 6 +- src/tui/app/mod.rs | 61 ++-- src/tui/lib/external/musicbrainz/api/mod.rs | 135 +++++---- .../lib/external/musicbrainz/daemon/mod.rs | 247 ++++++++++++---- src/tui/lib/interface/musicbrainz/api/mod.rs | 37 ++- .../lib/interface/musicbrainz/daemon/mod.rs | 43 ++- src/tui/lib/mod.rs | 17 +- src/tui/ui/display.rs | 78 ++--- src/tui/ui/match_state.rs | 37 +-- src/tui/ui/minibuffer.rs | 2 +- src/tui/ui/mod.rs | 54 ++-- 23 files changed, 644 insertions(+), 534 deletions(-) diff --git a/.gitea/workflows/gitea-ci.yaml b/.gitea/workflows/gitea-ci.yaml index dbd86fb..d540eb1 100644 --- a/.gitea/workflows/gitea-ci.yaml +++ b/.gitea/workflows/gitea-ci.yaml @@ -37,7 +37,7 @@ jobs: --ignore "tests/*" --ignore "src/main.rs" --ignore "src/bin/musichoard-edit.rs" - --excl-line "^#\[derive" + --excl-line "^#\[derive|unimplemented\!\(\)" --excl-start "GRCOV_EXCL_START|mod tests \{" --excl-stop "GRCOV_EXCL_STOP" --output-path ./target/debug/coverage/ diff --git a/README.md b/README.md index e88046b..c2aeb62 100644 --- a/README.md +++ b/README.md @@ -50,7 +50,7 @@ grcov codecov/debug/profraw \ --ignore "tests/*" \ --ignore "src/main.rs" \ --ignore "src/bin/musichoard-edit.rs" \ - --excl-line "^#\[derive" \ + --excl-line "^#\[derive|unimplemented\!\(\)" \ --excl-start "GRCOV_EXCL_START|mod tests \{" \ --excl-stop "GRCOV_EXCL_STOP" \ --output-path ./codecov/debug/coverage/ diff --git a/examples/musicbrainz_api/browse.rs b/examples/musicbrainz_api/browse.rs index af8d968..0526a2f 100644 --- a/examples/musicbrainz_api/browse.rs +++ b/examples/musicbrainz_api/browse.rs @@ -3,7 +3,7 @@ use std::{thread, time}; use musichoard::{ collection::musicbrainz::Mbid, external::musicbrainz::{ - api::{browse::BrowseReleaseGroupRequest, MusicBrainzClient, NextPage, PageSettings}, + api::{browse::BrowseReleaseGroupRequest, MusicBrainzClient, PageSettings}, http::MusicBrainzHttp, }, }; @@ -66,19 +66,15 @@ fn main() { println!("{rg:?}\n"); } - let offset = response.page.release_group_offset; let count = response.release_groups.len(); response_counts.push(count); - let total = response.page.release_group_count; - println!("Release group offset : {offset}"); println!("Release groups in this response: {count}"); - println!("Release groups in total : {total}"); - match response.page.next_page_offset(count) { - NextPage::Offset(next_offset) => paging.with_offset(next_offset), - NextPage::Complete => break, - } + paging = match response.page.next_page(paging, count) { + Some(paging) => paging, + None => break, + }; thread::sleep(time::Duration::from_secs(1)); } diff --git a/src/core/collection/mod.rs b/src/core/collection/mod.rs index bd964f9..3dbc419 100644 --- a/src/core/collection/mod.rs +++ b/src/core/collection/mod.rs @@ -2,12 +2,10 @@ pub mod album; pub mod artist; +pub mod merge; pub mod musicbrainz; pub mod track; -mod merge; -pub use merge::MergeCollections; - use std::fmt::{self, Display}; /// The [`Collection`] alias type for convenience. diff --git a/src/core/collection/musicbrainz.rs b/src/core/collection/musicbrainz.rs index 19597b6..575778a 100644 --- a/src/core/collection/musicbrainz.rs +++ b/src/core/collection/musicbrainz.rs @@ -47,9 +47,9 @@ pub enum MbRefOption { impl MbRefOption { pub fn or(self, optb: MbRefOption) -> MbRefOption { - match self { - x @ MbRefOption::Some(_) => x, - MbRefOption::CannotHaveMbid | MbRefOption::None => optb, + match (&self, &optb) { + (MbRefOption::Some(_), _) | (MbRefOption::CannotHaveMbid, MbRefOption::None) => self, + _ => optb, } } diff --git a/src/core/musichoard/base.rs b/src/core/musichoard/base.rs index 53d089a..22f0d4d 100644 --- a/src/core/musichoard/base.rs +++ b/src/core/musichoard/base.rs @@ -2,7 +2,8 @@ use crate::core::{ collection::{ album::{Album, AlbumId}, artist::{Artist, ArtistId}, - Collection, MergeCollections, + merge::MergeCollections, + Collection, }, musichoard::{Error, MusicHoard}, }; diff --git a/src/core/musichoard/database.rs b/src/core/musichoard/database.rs index c384bf9..606c54d 100644 --- a/src/core/musichoard/database.rs +++ b/src/core/musichoard/database.rs @@ -1,5 +1,7 @@ +use std::mem; + use crate::{ - collection::{album::AlbumInfo, artist::ArtistInfo}, + collection::{album::AlbumInfo, artist::ArtistInfo, merge::Merge}, core::{ collection::{ album::{Album, AlbumId, AlbumSeq}, @@ -24,7 +26,7 @@ pub trait IMusicHoardDatabase { ) -> Result<(), Error>; fn clear_artist_sort>(&mut self, artist_id: Id) -> Result<(), Error>; - fn set_artist_info>( + fn merge_artist_info>( &mut self, artist_id: Id, info: ArtistInfo, @@ -66,7 +68,7 @@ pub trait IMusicHoardDatabase { artist_id: ArtistIdRef, album_id: AlbumIdRef, ) -> Result<(), Error>; - fn set_album_info, AlbumIdRef: AsRef>( + fn merge_album_info, AlbumIdRef: AsRef>( &mut self, artist_id: Id, album_id: AlbumIdRef, @@ -132,12 +134,15 @@ impl IMusicHoardDatabase for MusicHoard>( + fn merge_artist_info>( &mut self, artist_id: Id, - info: ArtistInfo, + mut info: ArtistInfo, ) -> Result<(), Error> { - self.update_artist(artist_id.as_ref(), |artist| artist.meta.info = info) + self.update_artist(artist_id.as_ref(), |artist| { + mem::swap(&mut artist.meta.info, &mut info); + artist.meta.info.merge_in_place(info); + }) } fn clear_artist_info>(&mut self, artist_id: Id) -> Result<(), Error> { @@ -216,14 +221,15 @@ impl IMusicHoardDatabase for MusicHoard, AlbumIdRef: AsRef>( + fn merge_album_info, AlbumIdRef: AsRef>( &mut self, artist_id: Id, album_id: AlbumIdRef, - info: AlbumInfo, + mut info: AlbumInfo, ) -> Result<(), Error> { self.update_album(artist_id.as_ref(), album_id.as_ref(), |album| { - album.meta.info = info + mem::swap(&mut album.meta.info, &mut info); + album.meta.info.merge_in_place(info); }) } @@ -457,7 +463,7 @@ mod tests { let artist_id = ArtistId::new("an artist"); let actual_err = music_hoard - .set_artist_info(&artist_id, ArtistInfo::default()) + .merge_artist_info(&artist_id, ArtistInfo::default()) .unwrap_err(); let expected_err = Error::CollectionError(format!("artist '{artist_id}' is not in the collection")); @@ -484,13 +490,13 @@ mod tests { // Setting a URL on an artist not in the collection is an error. assert!(music_hoard - .set_artist_info(&artist_id_2, info.clone()) + .merge_artist_info(&artist_id_2, info.clone()) .is_err()); assert_eq!(music_hoard.collection[0].meta.info.musicbrainz, expected); // Setting a URL on an artist. assert!(music_hoard - .set_artist_info(&artist_id, info.clone()) + .merge_artist_info(&artist_id, info.clone()) .is_ok()); expected.replace(MbArtistRef::from_uuid_str(MBID).unwrap()); assert_eq!(music_hoard.collection[0].meta.info.musicbrainz, expected); @@ -679,14 +685,14 @@ mod tests { // Seting info on an album not belonging to the artist is an error. assert!(music_hoard - .set_album_info(&artist_id, &album_id_2, info.clone()) + .merge_album_info(&artist_id, &album_id_2, info.clone()) .is_err()); let meta = &music_hoard.collection[0].albums[0].meta; assert_eq!(meta.info, AlbumInfo::default()); // Set info. assert!(music_hoard - .set_album_info(&artist_id, &album_id, info.clone()) + .merge_album_info(&artist_id, &album_id, info.clone()) .is_ok()); let meta = &music_hoard.collection[0].albums[0].meta; assert_eq!(meta.info, info); diff --git a/src/external/musicbrainz/api/browse.rs b/src/external/musicbrainz/api/browse.rs index 9d1a21d..c62dc14 100644 --- a/src/external/musicbrainz/api/browse.rs +++ b/src/external/musicbrainz/api/browse.rs @@ -6,7 +6,7 @@ use crate::{ collection::musicbrainz::Mbid, external::musicbrainz::{ api::{ - ApiDisplay, Error, MbReleaseGroupMeta, MusicBrainzClient, NextPage, PageSettings, + ApiDisplay, Error, MbReleaseGroupMeta, MusicBrainzClient, PageSettings, SerdeMbReleaseGroupMeta, MB_BASE_URL, }, IMusicBrainzHttp, @@ -16,13 +16,13 @@ use crate::{ #[derive(Clone, Copy, Debug, Deserialize, PartialEq, Eq)] #[serde(rename_all(deserialize = "kebab-case"))] pub struct BrowseReleaseGroupPage { - pub release_group_offset: usize, - pub release_group_count: usize, + release_group_offset: usize, + release_group_count: usize, } impl BrowseReleaseGroupPage { - pub fn next_page_offset(&self, page_count: usize) -> NextPage { - NextPage::next_page_offset( + pub fn next_page(&self, settings: PageSettings, page_count: usize) -> Option { + settings.next_page( self.release_group_offset, self.release_group_count, page_count, @@ -125,7 +125,7 @@ mod tests { release_group_count: 45, }; - next_page_test(|val| page.next_page_offset(val)); + next_page_test(|val| page.next_page(PageSettings::default(), val)); } #[test] diff --git a/src/external/musicbrainz/api/mod.rs b/src/external/musicbrainz/api/mod.rs index 2f61f75..dc86686 100644 --- a/src/external/musicbrainz/api/mod.rs +++ b/src/external/musicbrainz/api/mod.rs @@ -61,10 +61,10 @@ impl MusicBrainzClient { } } -#[derive(Default)] +#[derive(Copy, Clone, Debug, Default, PartialEq, Eq)] pub struct PageSettings { - pub limit: Option, - pub offset: Option, + limit: Option, + offset: Option, } impl PageSettings { @@ -79,24 +79,21 @@ impl PageSettings { Self::with_limit(MB_MAX_PAGE_LIMIT) } - pub fn with_offset(&mut self, offset: usize) { + pub fn with_offset(mut self, offset: usize) -> Self { + self.set_offset(offset); + self + } + + pub fn set_offset(&mut self, offset: usize) { self.offset = Some(offset); } -} -#[derive(Debug, PartialEq, Eq)] -pub enum NextPage { - Offset(usize), - Complete, -} - -impl NextPage { - pub fn next_page_offset(offset: usize, total_count: usize, page_count: usize) -> NextPage { + pub fn next_page(self, offset: usize, total_count: usize, page_count: usize) -> Option { let next_offset = offset + page_count; if next_offset < total_count { - NextPage::Offset(next_offset) + Some(self.with_offset(next_offset)) } else { - NextPage::Complete + None } } } @@ -361,21 +358,21 @@ mod tests { pub fn next_page_test(mut f: Fn) where - Fn: FnMut(usize) -> NextPage, + Fn: FnMut(usize) -> Option, { let next = f(20); - assert_eq!(next, NextPage::Offset(25)); + assert_eq!(next.unwrap().offset, Some(25)); let next = f(40); - assert_eq!(next, NextPage::Complete); + assert!(next.is_none()); let next = f(100); - assert_eq!(next, NextPage::Complete); + assert!(next.is_none()); } #[test] fn next_page() { - next_page_test(|val| NextPage::next_page_offset(5, 45, val)); + next_page_test(|val| PageSettings::default().next_page(5, 45, val)); } #[test] @@ -387,14 +384,14 @@ mod tests { assert_eq!(ApiDisplay::format_page_settings(&paging), "&limit=100"); let mut paging = PageSettings::with_limit(45); - paging.with_offset(145); + paging.set_offset(145); assert_eq!( ApiDisplay::format_page_settings(&paging), "&limit=45&offset=145" ); let mut paging = PageSettings::default(); - paging.with_offset(26); + paging.set_offset(26); assert_eq!(ApiDisplay::format_page_settings(&paging), "&offset=26"); } diff --git a/src/external/musicbrainz/api/search/mod.rs b/src/external/musicbrainz/api/search/mod.rs index 6fcb4cc..a0f78e7 100644 --- a/src/external/musicbrainz/api/search/mod.rs +++ b/src/external/musicbrainz/api/search/mod.rs @@ -23,18 +23,16 @@ use crate::external::musicbrainz::{ IMusicBrainzHttp, }; -use super::NextPage; - #[derive(Clone, Copy, Debug, Deserialize, PartialEq, Eq)] #[serde(rename_all(deserialize = "kebab-case"))] pub struct SearchPage { - pub offset: usize, - pub count: usize, + offset: usize, + count: usize, } impl SearchPage { - pub fn next_page_offset(&self, page_count: usize) -> NextPage { - NextPage::next_page_offset(self.offset, self.count, page_count) + pub fn next_page(&self, settings: PageSettings, page_count: usize) -> Option { + settings.next_page(self.offset, self.count, page_count) } } @@ -78,6 +76,6 @@ mod tests { count: 45, }; - next_page_test(|val| page.next_page_offset(val)); + next_page_test(|val| page.next_page(PageSettings::default(), val)); } } diff --git a/src/tui/app/machine/fetch_state.rs b/src/tui/app/machine/fetch_state.rs index c8a15b3..e5e1de2 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, AlbumMeta}, + album::{Album, AlbumId}, artist::{Artist, ArtistId, ArtistMeta}, musicbrainz::{IMusicBrainzRef, MbArtistRef, MbRefOption, Mbid}, }; @@ -16,7 +16,7 @@ use crate::tui::{ AppPublicState, AppState, Category, IAppEventFetch, IAppInteractFetch, }, lib::interface::musicbrainz::daemon::{ - Error as DaemonError, IMbJobSender, MbApiResult, MbParams, ResultSender, + Error as DaemonError, IMbJobSender, MbApiResult, MbParams, MbReturn, ResultSender, }, }; @@ -116,9 +116,7 @@ impl AppMachine { pub fn app_fetch_next(inner: AppInner, mut fetch: FetchState) -> App { match fetch.try_recv() { Ok(fetch_result) => match fetch_result { - Ok(next_match) => { - AppMachine::match_state(inner, MatchState::new(next_match, fetch)).into() - } + Ok(retval) => Self::handle_mb_api_return(inner, fetch, retval), Err(fetch_err) => { AppMachine::error_state(inner, format!("fetch failed: {fetch_err}")).into() } @@ -130,6 +128,15 @@ impl AppMachine { } } + 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() + } + _ => unimplemented!(), + } + } + pub fn app_lookup_artist( inner: AppInner, fetch: FetchState, @@ -144,13 +151,13 @@ impl AppMachine { inner: AppInner, fetch: FetchState, artist_id: &ArtistId, - album: &AlbumMeta, + album_id: &AlbumId, mbid: Mbid, ) -> App { let f = |mb: &dyn IMbJobSender, rs, album, mbid| { Self::submit_lookup_release_group_job(mb, rs, artist_id, album, mbid) }; - Self::app_lookup(f, inner, fetch, album, mbid) + Self::app_lookup(f, inner, fetch, album_id, mbid) } fn app_lookup( @@ -236,12 +243,12 @@ impl AppMachine { musicbrainz: &dyn IMbJobSender, result_sender: ResultSender, artist_id: &ArtistId, - album: &AlbumMeta, + album_id: &AlbumId, mbid: Mbid, ) -> Result<(), DaemonError> { let requests = VecDeque::from([MbParams::lookup_release_group( artist_id.clone(), - album.clone(), + album_id.clone(), mbid, )]); musicbrainz.submit_foreground_job(result_sender, requests) @@ -288,13 +295,9 @@ mod tests { use crate::tui::{ app::{ machine::tests::{inner, music_hoard}, - Delta, IApp, IAppAccess, IAppInteractBrowse, MatchOption, MatchStateInfo, - }, - lib::interface::musicbrainz::{ - self, - api::{Lookup, Match}, - daemon::MockIMbJobSender, + Delta, EntityMatches, IApp, IAppAccess, IAppInteractBrowse, MatchOption, }, + lib::interface::musicbrainz::{self, api::Entity, daemon::MockIMbJobSender}, testmod::COLLECTION, }; @@ -314,14 +317,14 @@ mod tests { let artist = COLLECTION[3].meta.clone(); - let matches: Vec> = vec![]; - let fetch_result = MatchStateInfo::artist_search(artist.clone(), matches); + let matches: Vec> = vec![]; + let fetch_result = MbReturn::Match(EntityMatches::artist_search(artist.clone(), matches)); fetch_tx.send(Ok(fetch_result.clone())).unwrap(); assert_eq!(fetch.try_recv(), Err(TryRecvError::Empty)); - let lookup = Lookup::new(artist.clone()); - let lookup_result = MatchStateInfo::artist_lookup(artist.clone(), lookup); + let lookup = Entity::new(artist.clone()); + let lookup_result = MbReturn::Match(EntityMatches::artist_lookup(artist.clone(), lookup)); lookup_tx.send(Ok(lookup_result.clone())).unwrap(); assert_eq!(fetch.try_recv(), Ok(Ok(lookup_result))); @@ -465,11 +468,11 @@ mod tests { fn lookup_album_expectation( job_sender: &mut MockIMbJobSender, artist_id: &ArtistId, - album: &AlbumMeta, + album_id: &AlbumId, ) { let requests = VecDeque::from([MbParams::lookup_release_group( artist_id.clone(), - album.clone(), + album_id.clone(), mbid(), )]); job_sender @@ -484,8 +487,8 @@ mod tests { let mut mb_job_sender = MockIMbJobSender::new(); let artist_id = COLLECTION[1].meta.id.clone(); - let album = COLLECTION[1].albums[0].meta.clone(); - lookup_album_expectation(&mut mb_job_sender, &artist_id, &album); + let album_id = COLLECTION[1].albums[0].meta.id.clone(); + lookup_album_expectation(&mut mb_job_sender, &artist_id, &album_id); let music_hoard = music_hoard(COLLECTION.to_owned()); let inner = AppInner::new(music_hoard, mb_job_sender); @@ -493,7 +496,7 @@ mod tests { let (_fetch_tx, fetch_rx) = mpsc::channel(); let fetch = FetchState::new(fetch_rx); - AppMachine::app_lookup_album(inner, fetch, &artist_id, &album, mbid()); + AppMachine::app_lookup_album(inner, fetch, &artist_id, &album_id, mbid()); } fn search_artist_expectation(job_sender: &mut MockIMbJobSender, artist: &ArtistMeta) { @@ -605,10 +608,10 @@ mod tests { let (tx, rx) = mpsc::channel::(); let artist = COLLECTION[3].meta.clone(); - let artist_match = Match::new(80, COLLECTION[2].meta.clone()); + let artist_match = Entity::with_score(COLLECTION[2].meta.clone(), 80); let artist_match_info = - MatchStateInfo::artist_search(artist.clone(), vec![artist_match.clone()]); - let fetch_result = Ok(artist_match_info); + EntityMatches::artist_search(artist.clone(), vec![artist_match.clone()]); + let fetch_result = Ok(MbReturn::Match(artist_match_info)); tx.send(fetch_result).unwrap(); let inner = inner(music_hoard(COLLECTION.clone())); @@ -623,8 +626,8 @@ mod tests { MatchOption::CannotHaveMbid, MatchOption::ManualInputMbid, ]; - let expected = MatchStateInfo::artist_search(artist, match_options); - assert_eq!(match_state.info, &expected); + let expected = EntityMatches::artist_search(artist, match_options); + assert_eq!(match_state.matches, &expected); } #[test] @@ -681,8 +684,8 @@ mod tests { assert!(matches!(app, AppState::Fetch(_))); let artist = COLLECTION[3].meta.clone(); - let match_info = MatchStateInfo::artist_search::>(artist, vec![]); - let fetch_result = Ok(match_info); + let match_info = EntityMatches::artist_search::>(artist, vec![]); + let fetch_result = Ok(MbReturn::Match(match_info)); tx.send(fetch_result).unwrap(); let app = app.unwrap_fetch().fetch_result_ready(); diff --git a/src/tui/app/machine/match_state.rs b/src/tui/app/machine/match_state.rs index 19e0b3c..5b453a6 100644 --- a/src/tui/app/machine/match_state.rs +++ b/src/tui/app/machine/match_state.rs @@ -6,13 +6,10 @@ use musichoard::collection::{ musicbrainz::{MbRefOption, Mbid}, }; -use crate::tui::{ - app::{ - machine::{fetch_state::FetchState, input::Input, App, AppInner, AppMachine}, - AlbumMatches, AppPublicState, AppState, ArtistMatches, Delta, IAppInteractMatch, - ListOption, MatchOption, MatchStateInfo, MatchStatePublic, WidgetState, - }, - lib::interface::musicbrainz::api::{Lookup, Match}, +use crate::tui::app::{ + machine::{fetch_state::FetchState, input::Input, App, AppInner, AppMachine}, + AlbumMatches, AppPublicState, AppState, ArtistMatches, Delta, EntityMatches, IAppInteractMatch, + MatchOption, MatchStatePublic, WidgetState, }; trait GetInfoMeta { @@ -27,7 +24,7 @@ impl GetInfoMeta for AlbumMeta { trait GetInfo { type InfoType; - fn into_info(self, info: Self::InfoType) -> InfoOption; + fn get_info(&self) -> InfoOption; } enum InfoOption { @@ -35,86 +32,48 @@ enum InfoOption { NeedInput, } -macro_rules! impl_match_option_artist_into_info { - ($holder:ident) => { - impl GetInfo for MatchOption<$holder> { - type InfoType = ArtistInfo; +impl GetInfo for MatchOption { + type InfoType = ArtistInfo; - fn into_info(self, mut info: Self::InfoType) -> InfoOption { - match self { - MatchOption::Some(option) => info.musicbrainz = option.item.info.musicbrainz, - MatchOption::CannotHaveMbid => info.musicbrainz = MbRefOption::CannotHaveMbid, - MatchOption::ManualInputMbid => return InfoOption::NeedInput, - } - InfoOption::Info(info) - } + fn get_info(&self) -> InfoOption { + let mut info = ArtistInfo::default(); + match self { + MatchOption::Some(option) => info.musicbrainz = option.entity.info.musicbrainz.clone(), + MatchOption::CannotHaveMbid => info.musicbrainz = MbRefOption::CannotHaveMbid, + MatchOption::ManualInputMbid => return InfoOption::NeedInput, } - }; + InfoOption::Info(info) + } } -impl_match_option_artist_into_info!(Match); -impl_match_option_artist_into_info!(Lookup); +impl GetInfo for MatchOption { + type InfoType = AlbumInfo; -macro_rules! impl_match_option_album_into_info { - ($holder:ident) => { - impl GetInfo for MatchOption<$holder> { - type InfoType = AlbumInfo; - - fn into_info(self, mut info: Self::InfoType) -> InfoOption { - match self { - MatchOption::Some(option) => info = option.item.info, - MatchOption::CannotHaveMbid => info.musicbrainz = MbRefOption::CannotHaveMbid, - MatchOption::ManualInputMbid => return InfoOption::NeedInput, - } - InfoOption::Info(info) - } - } - }; -} - -impl_match_option_album_into_info!(Match); -impl_match_option_album_into_info!(Lookup); - -impl ListOption { - fn len(&self) -> usize { + fn get_info(&self) -> InfoOption { + let mut info = AlbumInfo::default(); match self { - ListOption::Lookup(list) => list.len(), - ListOption::Search(list) => list.len(), - } - } - - fn push_cannot_have_mbid(&mut self) { - match self { - ListOption::Lookup(list) => list.push(MatchOption::CannotHaveMbid), - ListOption::Search(list) => list.push(MatchOption::CannotHaveMbid), - } - } - - fn push_manual_input_mbid(&mut self) { - match self { - ListOption::Lookup(list) => list.push(MatchOption::ManualInputMbid), - ListOption::Search(list) => list.push(MatchOption::ManualInputMbid), + MatchOption::Some(option) => info = option.entity.info.clone(), + MatchOption::CannotHaveMbid => info.musicbrainz = MbRefOption::CannotHaveMbid, + MatchOption::ManualInputMbid => return InfoOption::NeedInput, } + InfoOption::Info(info) } } trait ExtractInfo { type InfoType; - fn extract_info(&mut self, index: usize, info: Self::InfoType) -> InfoOption; + fn extract_info(&self, index: usize) -> InfoOption; } -impl ExtractInfo for ListOption +impl ExtractInfo for Vec> where - MatchOption>: GetInfo, - MatchOption>: GetInfo, + MatchOption: GetInfo, + MatchOption: GetInfo, { type InfoType = T::InfoType; - fn extract_info(&mut self, index: usize, info: Self::InfoType) -> InfoOption { - match self { - ListOption::Lookup(ref mut list) => list.swap_remove(index).into_info(info), - ListOption::Search(ref mut list) => list.swap_remove(index).into_info(info), - } + fn extract_info(&self, index: usize) -> InfoOption { + self.get(index).unwrap().get_info() } } @@ -124,11 +83,11 @@ impl ArtistMatches { } fn push_cannot_have_mbid(&mut self) { - self.list.push_cannot_have_mbid(); + self.list.push(MatchOption::CannotHaveMbid); } fn push_manual_input_mbid(&mut self) { - self.list.push_manual_input_mbid(); + self.list.push(MatchOption::ManualInputMbid); } } @@ -138,15 +97,15 @@ impl AlbumMatches { } fn push_cannot_have_mbid(&mut self) { - self.list.push_cannot_have_mbid(); + self.list.push(MatchOption::CannotHaveMbid); } fn push_manual_input_mbid(&mut self) { - self.list.push_manual_input_mbid(); + self.list.push(MatchOption::ManualInputMbid); } } -impl MatchStateInfo { +impl EntityMatches { fn len(&self) -> usize { match self { Self::Artist(a) => a.len(), @@ -170,13 +129,13 @@ impl MatchStateInfo { } pub struct MatchState { - current: MatchStateInfo, + current: EntityMatches, state: WidgetState, fetch: FetchState, } impl MatchState { - pub fn new(mut current: MatchStateInfo, fetch: FetchState) -> Self { + pub fn new(mut current: EntityMatches, fetch: FetchState) -> Self { current.push_cannot_have_mbid(); current.push_manual_input_mbid(); @@ -201,11 +160,11 @@ impl AppMachine { Err(err) => return AppMachine::error_state(self.inner, err.to_string()).into(), }; match self.state.current { - MatchStateInfo::Artist(artist_matches) => { + EntityMatches::Artist(artist_matches) => { let matching = &artist_matches.matching; AppMachine::app_lookup_artist(self.inner, self.state.fetch, matching, mbid) } - MatchStateInfo::Album(album_matches) => { + EntityMatches::Album(album_matches) => { let artist_id = &album_matches.artist; let matching = &album_matches.matching; AppMachine::app_lookup_album( @@ -234,7 +193,7 @@ impl From> for App { impl<'a> From<&'a mut MatchState> for AppPublicState<'a> { fn from(state: &'a mut MatchState) -> Self { AppState::Match(MatchStatePublic { - info: &state.current, + matches: &state.current, state: &mut state.state, }) } @@ -268,22 +227,16 @@ impl IAppInteractMatch for AppMachine { let mh = &mut self.inner.music_hoard; let result = match self.state.current { - MatchStateInfo::Artist(ref mut matches) => { - let info: ArtistInfo = matches.matching.info.clone(); - match matches.list.extract_info(index, info) { - InfoOption::Info(info) => mh.set_artist_info(&matches.matching.id, info), - InfoOption::NeedInput => return self.get_input(), + EntityMatches::Artist(ref mut matches) => match matches.list.extract_info(index) { + InfoOption::Info(info) => mh.merge_artist_info(&matches.matching.id, info), + InfoOption::NeedInput => return self.get_input(), + }, + EntityMatches::Album(ref mut matches) => match matches.list.extract_info(index) { + InfoOption::Info(info) => { + mh.merge_album_info(&matches.artist, &matches.matching, info) } - } - MatchStateInfo::Album(ref mut matches) => { - let info: AlbumInfo = matches.matching.info.clone(); - match matches.list.extract_info(index, info) { - InfoOption::Info(info) => { - mh.set_album_info(&matches.artist, &matches.matching.id, info) - } - InfoOption::NeedInput => return self.get_input(), - } - } + InfoOption::NeedInput => return self.get_input(), + }, }; if let Err(err) = result { @@ -314,27 +267,18 @@ mod tests { IApp, IAppAccess, IAppInput, }, lib::interface::musicbrainz::{ - api::{Lookup, Match}, + api::Entity, daemon::{MbParams, MockIMbJobSender}, }, }; use super::*; - impl Match { - pub fn new(score: u8, item: T) -> Self { - Match { - score, - item, - disambiguation: None, - } - } - } - - impl Lookup { - pub fn new(item: T) -> Self { - Lookup { - item, + impl Entity { + pub fn with_score(entity: T, score: u8) -> Self { + Entity { + score: Some(score), + entity, disambiguation: None, } } @@ -350,29 +294,33 @@ mod tests { meta } - fn artist_match() -> MatchStateInfo { + fn artist_match() -> EntityMatches { let artist = artist_meta(); let artist_1 = artist.clone(); - let artist_match_1 = Match::new(100, artist_1); + let artist_match_1 = Entity::with_score(artist_1, 100); let artist_2 = artist.clone(); - let mut artist_match_2 = Match::new(100, artist_2); + let mut artist_match_2 = Entity::with_score(artist_2, 100); artist_match_2.disambiguation = Some(String::from("some disambiguation")); let list = vec![artist_match_1.clone(), artist_match_2.clone()]; - MatchStateInfo::artist_search(artist, list) + EntityMatches::artist_search(artist, list) } - fn artist_lookup() -> MatchStateInfo { + fn artist_lookup() -> EntityMatches { let artist = artist_meta(); - let lookup = Lookup::new(artist.clone()); - MatchStateInfo::artist_lookup(artist, lookup) + let lookup = Entity::new(artist.clone()); + EntityMatches::artist_lookup(artist, lookup) } - fn album_meta() -> AlbumMeta { + fn album_id() -> AlbumId { + AlbumId::new("Album") + } + + fn album_meta(id: AlbumId) -> AlbumMeta { AlbumMeta::new( - AlbumId::new("Album"), + id, AlbumDate::new(Some(1990), Some(5), None), AlbumInfo::new( MbRefOption::Some(mbid().into()), @@ -382,27 +330,29 @@ mod tests { ) } - fn album_match() -> MatchStateInfo { + fn album_match() -> EntityMatches { let artist_id = ArtistId::new("Artist"); - let album = album_meta(); + let album_id = album_id(); + let album_meta = album_meta(album_id.clone()); - let album_1 = album.clone(); - let album_match_1 = Match::new(100, album_1); + let album_1 = album_meta.clone(); + let album_match_1 = Entity::with_score(album_1, 100); - let mut album_2 = album.clone(); + let mut album_2 = album_meta.clone(); album_2.id.title.push_str(" extra title part"); album_2.info.secondary_types.pop(); - let album_match_2 = Match::new(100, album_2); + let album_match_2 = Entity::with_score(album_2, 100); let list = vec![album_match_1.clone(), album_match_2.clone()]; - MatchStateInfo::album_search(artist_id, album, list) + EntityMatches::album_search(artist_id, album_id, list) } - fn album_lookup() -> MatchStateInfo { + fn album_lookup() -> EntityMatches { let artist_id = ArtistId::new("Artist"); - let album = album_meta(); - let lookup = Lookup::new(album.clone()); - MatchStateInfo::album_lookup(artist_id, album, lookup) + let album_id = album_id(); + let album_meta = album_meta(album_id.clone()); + let lookup = Entity::new(album_meta.clone()); + EntityMatches::album_lookup(artist_id, album_id, lookup) } fn fetch_state() -> FetchState { @@ -410,7 +360,7 @@ mod tests { FetchState::new(rx) } - fn match_state(match_state_info: MatchStateInfo) -> MatchState { + fn match_state(match_state_info: EntityMatches) -> MatchState { MatchState::new(match_state_info, fetch_state()) } @@ -431,11 +381,11 @@ mod tests { let public = app.get(); let public_matches = public.state.unwrap_match(); - assert_eq!(public_matches.info, &album_match); + assert_eq!(public_matches.matches, &album_match); assert_eq!(public_matches.state, &widget_state); } - fn match_state_flow(mut matches_info: MatchStateInfo, len: usize) { + 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)); @@ -443,21 +393,25 @@ mod tests { let mut music_hoard = music_hoard(vec![]); let artist_id = ArtistId::new("Artist"); match matches_info { - MatchStateInfo::Album(_) => { + EntityMatches::Album(_) => { let album_id = AlbumId::new("Album"); - let mut info = album_meta().info; - info.musicbrainz = MbRefOption::CannotHaveMbid; + let info = AlbumInfo { + musicbrainz: MbRefOption::CannotHaveMbid, + ..Default::default() + }; music_hoard - .expect_set_album_info() + .expect_merge_album_info() .with(eq(artist_id.clone()), eq(album_id.clone()), eq(info)) .times(1) .return_once(|_, _, _| Ok(())); } - MatchStateInfo::Artist(_) => { - let mut info = artist_meta().info; - info.musicbrainz = MbRefOption::CannotHaveMbid; + EntityMatches::Artist(_) => { + let info = ArtistInfo { + musicbrainz: MbRefOption::CannotHaveMbid, + ..Default::default() + }; music_hoard - .expect_set_artist_info() + .expect_merge_artist_info() .with(eq(artist_id.clone()), eq(info)) .times(1) .return_once(|_, _| Ok(())); @@ -537,11 +491,11 @@ mod tests { let mut music_hoard = music_hoard(vec![]); match matches_info { - MatchStateInfo::Album(_) => panic!(), - MatchStateInfo::Artist(_) => { + EntityMatches::Album(_) => panic!(), + EntityMatches::Artist(_) => { let meta = artist_meta(); music_hoard - .expect_set_artist_info() + .expect_merge_artist_info() .with(eq(meta.id), eq(meta.info)) .times(1) .return_once(|_, _| Ok(())); @@ -561,11 +515,11 @@ mod tests { let mut music_hoard = music_hoard(vec![]); match matches_info { - MatchStateInfo::Artist(_) => panic!(), - MatchStateInfo::Album(matches) => { - let meta = album_meta(); + EntityMatches::Artist(_) => panic!(), + EntityMatches::Album(matches) => { + let meta = album_meta(album_id()); music_hoard - .expect_set_album_info() + .expect_merge_album_info() .with(eq(matches.artist), eq(meta.id), eq(meta.info)) .times(1) .return_once(|_, _, _| Ok(())); @@ -585,9 +539,9 @@ mod tests { let mut music_hoard = music_hoard(vec![]); match matches_info { - MatchStateInfo::Album(_) => panic!(), - MatchStateInfo::Artist(_) => { - music_hoard.expect_set_artist_info().return_once(|_, _| { + EntityMatches::Album(_) => panic!(), + EntityMatches::Artist(_) => { + music_hoard.expect_merge_artist_info().return_once(|_, _| { Err(musichoard::Error::DatabaseError(String::from("error"))) }); } @@ -649,8 +603,8 @@ mod tests { .with(predicate::always(), predicate::eq(requests)) .return_once(|_, _| Ok(())); - let matches_vec: Vec> = vec![]; - let artist_match = MatchStateInfo::artist_search(artist.clone(), matches_vec); + let matches_vec: Vec> = vec![]; + let artist_match = EntityMatches::artist_search(artist.clone(), matches_vec); let matches = AppMachine::match_state( inner_with_mb(music_hoard(vec![]), mb_job_sender), match_state(artist_match), @@ -674,7 +628,7 @@ mod tests { let album = AlbumMeta::new("Album", 1990, AlbumInfo::default()); let requests = VecDeque::from([MbParams::lookup_release_group( artist_id.clone(), - album.clone(), + album.id.clone(), mbid(), )]); mb_job_sender @@ -682,9 +636,9 @@ mod tests { .with(predicate::always(), predicate::eq(requests)) .return_once(|_, _| Ok(())); - let matches_vec: Vec> = vec![]; + let matches_vec: Vec> = vec![]; let album_match = - MatchStateInfo::album_search(artist_id.clone(), album.clone(), matches_vec); + EntityMatches::album_search(artist_id.clone(), album.id.clone(), matches_vec); let matches = AppMachine::match_state( inner_with_mb(music_hoard(vec![]), mb_job_sender), match_state(album_match), diff --git a/src/tui/app/machine/mod.rs b/src/tui/app/machine/mod.rs index 9873c25..660f922 100644 --- a/src/tui/app/machine/mod.rs +++ b/src/tui/app/machine/mod.rs @@ -225,9 +225,9 @@ mod tests { }; use crate::tui::{ - app::{AppState, IApp, IAppInput, IAppInteractBrowse, InputEvent, MatchStateInfo}, + app::{AppState, EntityMatches, IApp, IAppInput, IAppInteractBrowse, InputEvent}, lib::{ - interface::musicbrainz::{api::Lookup, daemon::MockIMbJobSender}, + interface::musicbrainz::{api::Entity, daemon::MockIMbJobSender}, MockIMusicHoard, }, }; @@ -520,7 +520,7 @@ mod tests { let (_, rx) = mpsc::channel(); let fetch = FetchState::new(rx); let artist = ArtistMeta::new(ArtistId::new("Artist")); - let info = MatchStateInfo::artist_lookup(artist.clone(), Lookup::new(artist.clone())); + let info = EntityMatches::artist_lookup(artist.clone(), Entity::new(artist.clone())); app = AppMachine::match_state(app.unwrap_browse().inner, MatchState::new(info, fetch)).into(); diff --git a/src/tui/app/mod.rs b/src/tui/app/mod.rs index e7bd921..bce93ce 100644 --- a/src/tui/app/mod.rs +++ b/src/tui/app/mod.rs @@ -6,12 +6,12 @@ use ratatui::widgets::ListState; pub use selection::{Category, Selection}; use musichoard::collection::{ - album::AlbumMeta, + album::{AlbumId, AlbumMeta}, artist::{ArtistId, ArtistMeta}, Collection, }; -use crate::tui::lib::interface::musicbrainz::api::Match; +use crate::tui::lib::interface::musicbrainz::api::Entity; pub enum AppState { Browse(B), @@ -37,8 +37,6 @@ macro_rules! IAppState { } use IAppState; -use super::lib::interface::musicbrainz::api::Lookup; - pub trait IApp { type BrowseState: IAppBase + IAppInteractBrowse; type InfoState: IAppBase + IAppInteractInfo; @@ -217,19 +215,13 @@ pub type InputPublic<'app> = &'app tui_input::Input; #[derive(Clone, Debug, PartialEq, Eq)] pub enum MatchOption { - Some(T), + Some(Entity), CannotHaveMbid, ManualInputMbid, } -#[derive(Clone, Debug, PartialEq, Eq)] -pub enum ListOption { - Search(Vec>>), - Lookup(Vec>>), -} - -impl From for MatchOption { - fn from(value: T) -> Self { +impl From> for MatchOption { + fn from(value: Entity) -> Self { MatchOption::Some(value) } } @@ -237,59 +229,56 @@ impl From for MatchOption { #[derive(Clone, Debug, PartialEq, Eq)] pub struct ArtistMatches { pub matching: ArtistMeta, - pub list: ListOption, + pub list: Vec>, } #[derive(Clone, Debug, PartialEq, Eq)] pub struct AlbumMatches { pub artist: ArtistId, - pub matching: AlbumMeta, - pub list: ListOption, + pub matching: AlbumId, + pub list: Vec>, } #[derive(Clone, Debug, PartialEq, Eq)] -pub enum MatchStateInfo { +pub enum EntityMatches { Artist(ArtistMatches), Album(AlbumMatches), } -impl MatchStateInfo { - pub fn artist_search>>>( +impl EntityMatches { + pub fn artist_search>>( matching: ArtistMeta, list: Vec, ) -> Self { - let list = ListOption::Search(list.into_iter().map(Into::into).collect()); - MatchStateInfo::Artist(ArtistMatches { matching, list }) + let list = list.into_iter().map(Into::into).collect(); + EntityMatches::Artist(ArtistMatches { matching, list }) } - pub fn album_search>>>( + pub fn album_search>>( artist: ArtistId, - matching: AlbumMeta, + matching: AlbumId, list: Vec, ) -> Self { - let list = ListOption::Search(list.into_iter().map(Into::into).collect()); - MatchStateInfo::Album(AlbumMatches { + let list = list.into_iter().map(Into::into).collect(); + EntityMatches::Album(AlbumMatches { artist, matching, list, }) } - pub fn artist_lookup>>>( - matching: ArtistMeta, - item: M, - ) -> Self { - let list = ListOption::Lookup(vec![item.into()]); - MatchStateInfo::Artist(ArtistMatches { matching, list }) + pub fn artist_lookup>>(matching: ArtistMeta, item: M) -> Self { + let list = vec![item.into()]; + EntityMatches::Artist(ArtistMatches { matching, list }) } - pub fn album_lookup>>>( + pub fn album_lookup>>( artist: ArtistId, - matching: AlbumMeta, + matching: AlbumId, item: M, ) -> Self { - let list = ListOption::Lookup(vec![item.into()]); - MatchStateInfo::Album(AlbumMatches { + let list = vec![item.into()]; + EntityMatches::Album(AlbumMatches { artist, matching, list, @@ -298,7 +287,7 @@ impl MatchStateInfo { } pub struct MatchStatePublic<'app> { - pub info: &'app MatchStateInfo, + pub matches: &'app EntityMatches, pub state: &'app mut WidgetState, } diff --git a/src/tui/lib/external/musicbrainz/api/mod.rs b/src/tui/lib/external/musicbrainz/api/mod.rs index 35156c5..6c2c7c1 100644 --- a/src/tui/lib/external/musicbrainz/api/mod.rs +++ b/src/tui/lib/external/musicbrainz/api/mod.rs @@ -10,6 +10,7 @@ use musichoard::{ }, external::musicbrainz::{ api::{ + browse::{BrowseReleaseGroupRequest, BrowseReleaseGroupResponse}, lookup::{ LookupArtistRequest, LookupArtistResponse, LookupReleaseGroupRequest, LookupReleaseGroupResponse, @@ -18,13 +19,13 @@ use musichoard::{ SearchArtistRequest, SearchArtistResponseArtist, SearchReleaseGroupRequest, SearchReleaseGroupResponseReleaseGroup, }, - MusicBrainzClient, PageSettings, + MbArtistMeta, MbReleaseGroupMeta, MusicBrainzClient, PageSettings, }, IMusicBrainzHttp, }, }; -use crate::tui::lib::interface::musicbrainz::api::{Error, IMusicBrainz, Lookup, Match}; +use crate::tui::lib::interface::musicbrainz::api::{Entity, Error, IMusicBrainz}; // GRCOV_EXCL_START pub struct MusicBrainz { @@ -38,7 +39,7 @@ impl MusicBrainz { } impl IMusicBrainz for MusicBrainz { - fn lookup_artist(&mut self, mbid: &Mbid) -> Result, Error> { + fn lookup_artist(&mut self, mbid: &Mbid) -> Result, Error> { let request = LookupArtistRequest::new(mbid); let mb_response = self.client.lookup_artist(&request)?; @@ -46,7 +47,7 @@ impl IMusicBrainz for MusicBrainz { Ok(from_lookup_artist_response(mb_response)) } - fn lookup_release_group(&mut self, mbid: &Mbid) -> Result, Error> { + fn lookup_release_group(&mut self, mbid: &Mbid) -> Result, Error> { let request = LookupReleaseGroupRequest::new(mbid); let mb_response = self.client.lookup_release_group(&request)?; @@ -54,7 +55,7 @@ impl IMusicBrainz for MusicBrainz { Ok(from_lookup_release_group_response(mb_response)) } - fn search_artist(&mut self, artist: &ArtistMeta) -> Result>, Error> { + fn search_artist(&mut self, artist: &ArtistMeta) -> Result>, Error> { let query = SearchArtistRequest::new().string(&artist.id.name); let paging = PageSettings::default(); @@ -71,7 +72,7 @@ impl IMusicBrainz for MusicBrainz { &mut self, arid: &Mbid, album: &AlbumMeta, - ) -> Result>, Error> { + ) -> 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); @@ -92,71 +93,93 @@ impl IMusicBrainz for MusicBrainz { .map(from_search_release_group_response_release_group) .collect()) } -} -fn from_lookup_artist_response(entity: LookupArtistResponse) -> Lookup { - let sort = Some(entity.meta.sort_name).filter(|s| s != &entity.meta.name); - Lookup { - item: ArtistMeta { - id: entity.meta.name.into(), - sort, - info: ArtistInfo { - musicbrainz: MbRefOption::Some(entity.meta.id.into()), - properties: HashMap::new(), - }, - }, - disambiguation: entity.meta.disambiguation, + fn browse_release_group( + &mut self, + artist: &Mbid, + paging: &mut Option, + ) -> Result>, Error> { + let request = BrowseReleaseGroupRequest::artist(artist); + + let page = paging.take().unwrap_or_default(); + let mb_response = self.client.browse_release_group(&request, &page)?; + + let page_count = mb_response.release_groups.len(); + *paging = mb_response.page.next_page(page, page_count); + + Ok(from_browse_release_group_response(mb_response)) } } -fn from_lookup_release_group_response(entity: LookupReleaseGroupResponse) -> Lookup { - Lookup { - item: AlbumMeta { - id: entity.meta.title.into(), - date: entity.meta.first_release_date, - seq: AlbumSeq::default(), - info: AlbumInfo { - musicbrainz: MbRefOption::Some(entity.meta.id.into()), - primary_type: entity.meta.primary_type, - secondary_types: entity.meta.secondary_types.unwrap_or_default(), +fn from_mb_artist_meta(meta: MbArtistMeta) -> (ArtistMeta, Option) { + let sort = Some(meta.sort_name).filter(|s| s != &meta.name); + ( + ArtistMeta { + id: meta.name.into(), + sort, + info: ArtistInfo { + musicbrainz: MbRefOption::Some(meta.id.into()), + properties: HashMap::new(), }, }, + meta.disambiguation, + ) +} + +fn from_mb_release_group_meta(meta: MbReleaseGroupMeta) -> AlbumMeta { + AlbumMeta { + id: meta.title.into(), + date: meta.first_release_date, + seq: AlbumSeq::default(), + info: AlbumInfo { + musicbrainz: MbRefOption::Some(meta.id.into()), + primary_type: meta.primary_type, + secondary_types: meta.secondary_types.unwrap_or_default(), + }, + } +} + +fn from_lookup_artist_response(response: LookupArtistResponse) -> Entity { + let (entity, disambiguation) = from_mb_artist_meta(response.meta); + Entity { + score: None, + entity, + disambiguation, + } +} + +fn from_lookup_release_group_response(response: LookupReleaseGroupResponse) -> Entity { + Entity { + score: None, + entity: from_mb_release_group_meta(response.meta), disambiguation: None, } } -fn from_search_artist_response_artist(entity: SearchArtistResponseArtist) -> Match { - let sort = Some(entity.meta.sort_name).filter(|s| s != &entity.meta.name); - Match { - score: entity.score, - item: ArtistMeta { - id: entity.meta.name.into(), - sort, - info: ArtistInfo { - musicbrainz: MbRefOption::Some(entity.meta.id.into()), - properties: HashMap::new(), - }, - }, - disambiguation: entity.meta.disambiguation, +fn from_search_artist_response_artist(response: SearchArtistResponseArtist) -> Entity { + let (entity, disambiguation) = from_mb_artist_meta(response.meta); + Entity { + score: Some(response.score), + entity, + disambiguation, } } fn from_search_release_group_response_release_group( - entity: SearchReleaseGroupResponseReleaseGroup, -) -> Match { - Match { - score: entity.score, - item: AlbumMeta { - id: entity.meta.title.into(), - date: entity.meta.first_release_date, - seq: AlbumSeq::default(), - info: AlbumInfo { - musicbrainz: MbRefOption::Some(entity.meta.id.into()), - primary_type: entity.meta.primary_type, - secondary_types: entity.meta.secondary_types.unwrap_or_default(), - }, - }, + response: SearchReleaseGroupResponseReleaseGroup, +) -> Entity { + Entity { + score: Some(response.score), + entity: from_mb_release_group_meta(response.meta), disambiguation: None, } } + +fn from_browse_release_group_response( + entity: BrowseReleaseGroupResponse, +) -> Vec> { + let rgs = entity.release_groups.into_iter(); + let metas = rgs.map(from_mb_release_group_meta); + metas.map(Entity::new).collect() +} // GRCOV_EXCL_STOP diff --git a/src/tui/lib/external/musicbrainz/daemon/mod.rs b/src/tui/lib/external/musicbrainz/daemon/mod.rs index dc75fd2..744b75a 100644 --- a/src/tui/lib/external/musicbrainz/daemon/mod.rs +++ b/src/tui/lib/external/musicbrainz/daemon/mod.rs @@ -1,11 +1,16 @@ use std::{collections::VecDeque, sync::mpsc, thread, time}; +use musichoard::external::musicbrainz::api::PageSettings; + use crate::tui::{ - app::MatchStateInfo, + app::EntityMatches, event::IFetchCompleteEventSender, lib::interface::musicbrainz::{ api::{Error as ApiError, IMusicBrainz}, - daemon::{Error, IMbJobSender, LookupParams, MbParams, ResultSender, SearchParams}, + daemon::{ + BrowseParams, EntityList, Error, IMbJobSender, LookupParams, MbParams, MbReturn, + ResultSender, SearchParams, + }, }, }; @@ -43,6 +48,7 @@ enum JobPriority { struct JobInstance { result_sender: ResultSender, requests: VecDeque, + paging: Option, } #[derive(Debug, PartialEq, Eq)] @@ -62,6 +68,7 @@ impl JobInstance { JobInstance { result_sender, requests, + paging: None, } } } @@ -221,8 +228,14 @@ impl JobInstance { 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 let Some(params) = self.requests.front() { + let result_sender = &mut self.result_sender; + let paging = &mut self.paging; + Self::execute(musicbrainz, result_sender, event_sender, params, paging)?; + }; + + if self.paging.is_none() { + self.requests.pop_front(); } if self.requests.is_empty() { @@ -233,38 +246,60 @@ impl JobInstance { } fn execute( - &mut self, musicbrainz: &mut dyn IMusicBrainz, + result_sender: &mut ResultSender, event_sender: &mut dyn IFetchCompleteEventSender, - api_params: MbParams, + api_params: &MbParams, + paging: &mut Option, ) -> Result<(), JobInstanceError> { let result = match api_params { MbParams::Lookup(lookup) => match lookup { - LookupParams::Artist(params) => musicbrainz - .lookup_artist(¶ms.mbid) - .map(|rv| MatchStateInfo::artist_lookup(params.artist, rv)), - LookupParams::ReleaseGroup(params) => musicbrainz - .lookup_release_group(¶ms.mbid) - .map(|rv| MatchStateInfo::album_lookup(params.artist_id, params.album, rv)), - }, + LookupParams::Artist(p) => musicbrainz + .lookup_artist(&p.mbid) + .map(|rv| EntityMatches::artist_lookup(p.artist.clone(), rv)), + LookupParams::ReleaseGroup(p) => { + musicbrainz.lookup_release_group(&p.mbid).map(|rv| { + EntityMatches::album_lookup(p.artist_id.clone(), p.album_id.clone(), rv) + }) + } + } + .map(MbReturn::Match), MbParams::Search(search) => match search { - SearchParams::Artist(params) => musicbrainz - .search_artist(¶ms.artist) - .map(|rv| MatchStateInfo::artist_search(params.artist, rv)), - SearchParams::ReleaseGroup(params) => musicbrainz - .search_release_group(¶ms.artist_mbid, ¶ms.album) - .map(|rv| MatchStateInfo::album_search(params.artist_id, params.album, rv)), - }, + SearchParams::Artist(p) => musicbrainz + .search_artist(&p.artist) + .map(|rv| EntityMatches::artist_search(p.artist.clone(), rv)), + SearchParams::ReleaseGroup(p) => musicbrainz + .search_release_group(&p.artist_mbid, &p.album) + .map(|rv| { + EntityMatches::album_search(p.artist_id.clone(), p.album.id.clone(), rv) + }), + } + .map(MbReturn::Match), + MbParams::Browse(browse) => match browse { + BrowseParams::ReleaseGroup(params) => { + Self::init_paging_if_none(paging); + musicbrainz + .browse_release_group(¶ms.artist, paging) + .map(|rv| EntityList::Album(rv.into_iter().map(|rg| rg.entity).collect())) + } + } + .map(MbReturn::Fetch), }; - self.return_result(event_sender, result) + Self::return_result(result_sender, event_sender, result) + } + + fn init_paging_if_none(paging: &mut Option) { + if paging.is_none() { + *paging = Some(PageSettings::with_max_limit()); + } } fn return_result( - &mut self, + result_sender: &mut ResultSender, event_sender: &mut dyn IFetchCompleteEventSender, - result: Result, + result: Result, ) -> Result<(), JobInstanceError> { - self.result_sender + result_sender .send(result) .map_err(|_| JobInstanceError::ReturnChannelDisconnected)?; @@ -321,7 +356,7 @@ mod tests { use crate::tui::{ event::{Event, EventError, MockIFetchCompleteEventSender}, - lib::interface::musicbrainz::api::{Lookup, Match, MockIMusicBrainz}, + lib::interface::musicbrainz::api::{Entity, MockIMusicBrainz}, testmod::COLLECTION, }; @@ -398,9 +433,9 @@ mod tests { fn lookup_release_group_requests() -> VecDeque { let artist_id = COLLECTION[1].meta.id.clone(); - let album = COLLECTION[1].albums[0].meta.clone(); + let album_id = COLLECTION[1].albums[0].meta.id.clone(); let mbid = mbid(); - VecDeque::from([MbParams::lookup_release_group(artist_id, album, mbid)]) + VecDeque::from([MbParams::lookup_release_group(artist_id, album_id, mbid)]) } fn search_artist_requests() -> VecDeque { @@ -408,11 +443,11 @@ mod tests { VecDeque::from([MbParams::search_artist(artist)]) } - fn search_artist_expectations() -> (ArtistMeta, Vec>) { + fn search_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 artist_match_1 = Entity::with_score(artist.clone(), 100); + let artist_match_2 = Entity::with_score(artist.clone(), 50); let matches = vec![artist_match_1.clone(), artist_match_2.clone()]; (artist, matches) @@ -432,6 +467,12 @@ mod tests { ]) } + fn browse_albums_requests() -> VecDeque { + let mbref = mb_ref_opt_as_ref(&COLLECTION[1].meta.info.musicbrainz); + let arid = mb_ref_opt_unwrap(mbref).mbid().clone(); + VecDeque::from([MbParams::browse_release_group(arid)]) + } + fn album_artist_id() -> ArtistId { COLLECTION[1].meta.id.clone() } @@ -441,23 +482,23 @@ mod tests { mb_ref_opt_unwrap(mbref).mbid().clone() } - fn search_album_expectations_1() -> (AlbumMeta, Vec>) { + fn search_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 album_match_1_1 = Entity::with_score(album_1.clone(), 100); + let album_match_1_2 = Entity::with_score(album_4.clone(), 50); let matches_1 = vec![album_match_1_1.clone(), album_match_1_2.clone()]; (album_1, matches_1) } - fn search_album_expectations_4() -> (AlbumMeta, Vec>) { + fn search_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 album_match_4_1 = Entity::with_score(album_4.clone(), 100); + let album_match_4_2 = Entity::with_score(album_1.clone(), 30); let matches_4 = vec![album_match_4_1.clone(), album_match_4_2.clone()]; (album_4, matches_4) @@ -539,7 +580,7 @@ mod tests { fn lookup_artist_expectation( musicbrainz: &mut MockIMusicBrainz, mbid: &Mbid, - lookup: &Lookup, + lookup: &Entity, ) { let result = Ok(lookup.clone()); musicbrainz @@ -554,7 +595,7 @@ mod tests { let mut musicbrainz = musicbrainz(); let mbid = mbid(); let artist = COLLECTION[3].meta.clone(); - let lookup = Lookup::new(artist.clone()); + let lookup = Entity::new(artist.clone()); lookup_artist_expectation(&mut musicbrainz, &mbid, &lookup); let mut event_sender = event_sender(); @@ -574,14 +615,22 @@ mod tests { let result = daemon.execute_next_job(); assert_eq!(result, Ok(())); + let result = daemon.execute_next_job(); + assert_eq!(result, Err(JobError::JobQueueEmpty)); + let result = result_receiver.try_recv().unwrap(); - assert_eq!(result, Ok(MatchStateInfo::artist_lookup(artist, lookup))); + assert_eq!( + result, + Ok(MbReturn::Match(EntityMatches::artist_lookup( + artist, lookup + ))) + ); } fn lookup_release_group_expectation( musicbrainz: &mut MockIMusicBrainz, mbid: &Mbid, - lookup: &Lookup, + lookup: &Entity, ) { let result = Ok(lookup.clone()); musicbrainz @@ -595,8 +644,9 @@ mod tests { fn execute_lookup_release_group() { let mut musicbrainz = musicbrainz(); let mbid = mbid(); - let album = COLLECTION[1].albums[0].meta.clone(); - let lookup = Lookup::new(album.clone()); + let album_id = COLLECTION[1].albums[0].meta.id.clone(); + let album_meta = COLLECTION[1].albums[0].meta.clone(); + let lookup = Entity::new(album_meta.clone()); lookup_release_group_expectation(&mut musicbrainz, &mbid, &lookup); let mut event_sender = event_sender(); @@ -616,18 +666,23 @@ mod tests { let result = daemon.execute_next_job(); assert_eq!(result, Ok(())); + let result = daemon.execute_next_job(); + assert_eq!(result, Err(JobError::JobQueueEmpty)); + let result = result_receiver.try_recv().unwrap(); let artist_id = album_artist_id(); assert_eq!( result, - Ok(MatchStateInfo::album_lookup(artist_id, album, lookup)) + Ok(MbReturn::Match(EntityMatches::album_lookup( + artist_id, album_id, lookup + ))) ); } fn search_artist_expectation( musicbrainz: &mut MockIMusicBrainz, artist: &ArtistMeta, - matches: &[Match], + matches: &[Entity], ) { let result = Ok(matches.to_owned()); musicbrainz @@ -660,8 +715,16 @@ mod tests { let result = daemon.execute_next_job(); assert_eq!(result, Ok(())); + let result = daemon.execute_next_job(); + assert_eq!(result, Err(JobError::JobQueueEmpty)); + let result = result_receiver.try_recv().unwrap(); - assert_eq!(result, Ok(MatchStateInfo::artist_search(artist, matches))); + assert_eq!( + result, + Ok(MbReturn::Match(EntityMatches::artist_search( + artist, matches + ))) + ); } fn search_release_group_expectation( @@ -669,7 +732,7 @@ mod tests { seq: &mut Sequence, arid: &Mbid, album: &AlbumMeta, - matches: &[Match], + matches: &[Entity], ) { let result = Ok(matches.to_owned()); musicbrainz @@ -711,27 +774,18 @@ mod tests { let result = daemon.execute_next_job(); assert_eq!(result, Ok(())); + let result = daemon.execute_next_job(); + assert_eq!(result, Err(JobError::JobQueueEmpty)); + let artist_id = album_artist_id(); let result = result_receiver.try_recv().unwrap(); - assert_eq!( - result, - Ok(MatchStateInfo::album_search( - artist_id.clone(), - album_1, - matches_1 - )) - ); + let matches = EntityMatches::album_search(artist_id.clone(), album_1.id, matches_1); + assert_eq!(result, Ok(MbReturn::Match(matches))); let result = result_receiver.try_recv().unwrap(); - assert_eq!( - result, - Ok(MatchStateInfo::album_search( - artist_id.clone(), - album_4, - matches_4 - )) - ); + let matches = EntityMatches::album_search(artist_id.clone(), album_4.id, matches_4); + assert_eq!(result, Ok(MbReturn::Match(matches))); } #[test] @@ -792,6 +846,75 @@ mod tests { assert_eq!(result, Err(JobError::EventChannelDisconnected)); } + fn browse_release_group_expectation( + musicbrainz: &mut MockIMusicBrainz, + seq: &mut Sequence, + mbid: &Mbid, + page: Option, + matches: &[Entity], + next_page: Option, + ) { + let result = Ok(matches.to_owned()); + musicbrainz + .expect_browse_release_group() + .with(predicate::eq(mbid.clone()), predicate::eq(page)) + .times(1) + .in_sequence(seq) + .return_once(move |_, paging| { + *paging = next_page; + result + }); + } + + #[test] + fn execute_browse_release_groups() { + let mut musicbrainz = musicbrainz(); + let arid = album_arid_expectation(); + let (_, matches_1) = search_album_expectations_1(); + let (_, matches_4) = search_album_expectations_4(); + + let mut seq = Sequence::new(); + + let page = Some(PageSettings::with_max_limit()); + let next = Some(PageSettings::with_max_limit().with_offset(10)); + browse_release_group_expectation(&mut musicbrainz, &mut seq, &arid, page, &matches_1, next); + + let page = next; + let next = None; + browse_release_group_expectation(&mut musicbrainz, &mut seq, &arid, page, &matches_4, next); + + 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 = browse_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 = daemon.execute_next_job(); + assert_eq!(result, Err(JobError::JobQueueEmpty)); + + let result = result_receiver.try_recv().unwrap(); + let fetch = EntityList::Album(matches_1.into_iter().map(|m| m.entity).collect()); + assert_eq!(result, Ok(MbReturn::Fetch(fetch))); + + let result = result_receiver.try_recv().unwrap(); + let fetch = EntityList::Album(matches_4.into_iter().map(|m| m.entity).collect()); + assert_eq!(result, Ok(MbReturn::Fetch(fetch))); + } + #[test] fn job_queue() { let mut queue = JobQueue::new(); diff --git a/src/tui/lib/interface/musicbrainz/api/mod.rs b/src/tui/lib/interface/musicbrainz/api/mod.rs index 62917c0..11889fc 100644 --- a/src/tui/lib/interface/musicbrainz/api/mod.rs +++ b/src/tui/lib/interface/musicbrainz/api/mod.rs @@ -3,32 +3,43 @@ #[cfg(test)] use mockall::automock; -use musichoard::collection::{album::AlbumMeta, artist::ArtistMeta, musicbrainz::Mbid}; +use musichoard::{ + collection::{album::AlbumMeta, artist::ArtistMeta, musicbrainz::Mbid}, + external::musicbrainz::api::PageSettings, +}; -/// Trait for interacting with the MusicBrainz API. #[cfg_attr(test, automock)] pub trait IMusicBrainz { - fn lookup_artist(&mut self, mbid: &Mbid) -> Result, Error>; - fn lookup_release_group(&mut self, mbid: &Mbid) -> Result, Error>; - fn search_artist(&mut self, artist: &ArtistMeta) -> Result>, Error>; + fn lookup_artist(&mut self, mbid: &Mbid) -> Result, Error>; + fn lookup_release_group(&mut self, mbid: &Mbid) -> Result, Error>; + fn search_artist(&mut self, artist: &ArtistMeta) -> Result>, Error>; fn search_release_group( &mut self, arid: &Mbid, album: &AlbumMeta, - ) -> Result>, Error>; + ) -> Result>, Error>; + fn browse_release_group( + &mut self, + artist: &Mbid, + paging: &mut Option, + ) -> Result>, Error>; } #[derive(Clone, Debug, PartialEq, Eq)] -pub struct Match { - pub score: u8, - pub item: T, +pub struct Entity { + pub score: Option, + pub entity: T, pub disambiguation: Option, } -#[derive(Clone, Debug, PartialEq, Eq)] -pub struct Lookup { - pub item: T, - pub disambiguation: Option, +impl Entity { + pub fn new(entity: T) -> Self { + Entity { + score: None, + entity, + disambiguation: None, + } + } } 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 index 0a2b468..1c54b80 100644 --- a/src/tui/lib/interface/musicbrainz/daemon/mod.rs +++ b/src/tui/lib/interface/musicbrainz/daemon/mod.rs @@ -1,12 +1,12 @@ use std::{collections::VecDeque, fmt, sync::mpsc}; use musichoard::collection::{ - album::AlbumMeta, + album::{AlbumId, AlbumMeta}, artist::{ArtistId, ArtistMeta}, musicbrainz::Mbid, }; -use crate::tui::{app::MatchStateInfo, lib::interface::musicbrainz::api::Error as MbApiError}; +use crate::tui::{app::EntityMatches, lib::interface::musicbrainz::api::Error as MbApiError}; #[cfg(test)] use mockall::automock; @@ -26,9 +26,20 @@ impl fmt::Display for Error { } } -pub type MbApiResult = Result; +pub type MbApiResult = Result; pub type ResultSender = mpsc::Sender; +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum MbReturn { + Match(EntityMatches), + Fetch(EntityList), +} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum EntityList { + Album(Vec), +} + #[cfg_attr(test, automock)] pub trait IMbJobSender { fn submit_foreground_job( @@ -48,6 +59,8 @@ pub trait IMbJobSender { pub enum MbParams { Lookup(LookupParams), Search(SearchParams), + #[allow(dead_code)] // TODO: remove with completion of #160 + Browse(BrowseParams), } #[derive(Clone, Debug, PartialEq, Eq)] @@ -65,7 +78,7 @@ pub struct LookupArtistParams { #[derive(Clone, Debug, PartialEq, Eq)] pub struct LookupReleaseGroupParams { pub artist_id: ArtistId, - pub album: AlbumMeta, + pub album_id: AlbumId, pub mbid: Mbid, } @@ -87,15 +100,26 @@ pub struct SearchReleaseGroupParams { pub album: AlbumMeta, } +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum BrowseParams { + #[allow(dead_code)] // TODO: remove with completion of #160 + ReleaseGroup(BrowseReleaseGroupParams), +} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct BrowseReleaseGroupParams { + pub artist: Mbid, +} + impl MbParams { pub fn lookup_artist(artist: ArtistMeta, mbid: Mbid) -> Self { MbParams::Lookup(LookupParams::Artist(LookupArtistParams { artist, mbid })) } - pub fn lookup_release_group(artist_id: ArtistId, album: AlbumMeta, mbid: Mbid) -> Self { + pub fn lookup_release_group(artist_id: ArtistId, album_id: AlbumId, mbid: Mbid) -> Self { MbParams::Lookup(LookupParams::ReleaseGroup(LookupReleaseGroupParams { artist_id, - album, + album_id, mbid, })) } @@ -111,6 +135,13 @@ impl MbParams { album, })) } + + #[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, + })) + } } #[cfg(test)] diff --git a/src/tui/lib/mod.rs b/src/tui/lib/mod.rs index 8adc147..59cfd41 100644 --- a/src/tui/lib/mod.rs +++ b/src/tui/lib/mod.rs @@ -20,9 +20,12 @@ pub trait IMusicHoard { fn reload_database(&mut self) -> Result<(), musichoard::Error>; fn get_collection(&self) -> &Collection; - fn set_artist_info(&mut self, id: &ArtistId, info: ArtistInfo) - -> Result<(), musichoard::Error>; - fn set_album_info( + fn merge_artist_info( + &mut self, + id: &ArtistId, + info: ArtistInfo, + ) -> Result<(), musichoard::Error>; + fn merge_album_info( &mut self, artist_id: &ArtistId, album_id: &AlbumId, @@ -44,21 +47,21 @@ impl IMusicHoard for MusicHoard::get_collection(self) } - fn set_artist_info( + fn merge_artist_info( &mut self, id: &ArtistId, info: ArtistInfo, ) -> Result<(), musichoard::Error> { - ::set_artist_info(self, id, info) + ::merge_artist_info(self, id, info) } - fn set_album_info( + fn merge_album_info( &mut self, artist_id: &ArtistId, album_id: &AlbumId, info: AlbumInfo, ) -> Result<(), musichoard::Error> { - ::set_album_info(self, artist_id, album_id, info) + ::merge_album_info(self, artist_id, album_id, info) } } // GRCOV_EXCL_STOP diff --git a/src/tui/ui/display.rs b/src/tui/ui/display.rs index 30c3950..0210688 100644 --- a/src/tui/ui/display.rs +++ b/src/tui/ui/display.rs @@ -1,14 +1,13 @@ use musichoard::collection::{ - album::{AlbumDate, AlbumMeta, AlbumPrimaryType, AlbumSecondaryType, AlbumSeq, AlbumStatus}, + album::{ + AlbumDate, AlbumId, AlbumMeta, AlbumPrimaryType, AlbumSecondaryType, AlbumSeq, AlbumStatus, + }, artist::ArtistMeta, musicbrainz::{IMusicBrainzRef, MbRefOption}, track::{TrackFormat, TrackQuality}, }; -use crate::tui::{ - app::{MatchOption, MatchStateInfo}, - lib::interface::musicbrainz::api::{Lookup, Match}, -}; +use crate::tui::app::{EntityMatches, MatchOption}; pub struct UiDisplay; @@ -114,43 +113,40 @@ impl UiDisplay { format!("Matching artist: {}", &artist.id.name) } - pub fn display_album_matching(album: &AlbumMeta) -> String { - format!( - "Matching album: {} | {}", - UiDisplay::display_album_date(&album.date), - &album.id.title - ) + pub fn display_album_matching(album: &AlbumId) -> String { + format!("Matching album: {}", &album.title) } - pub fn display_matching_info(info: &MatchStateInfo) -> String { + pub fn display_matching_info(info: &EntityMatches) -> String { match info { - MatchStateInfo::Artist(m) => UiDisplay::display_artist_matching(&m.matching), - MatchStateInfo::Album(m) => UiDisplay::display_album_matching(&m.matching), + EntityMatches::Artist(m) => UiDisplay::display_artist_matching(&m.matching), + EntityMatches::Album(m) => UiDisplay::display_album_matching(&m.matching), } } - pub fn display_search_option_artist(match_option: &MatchOption>) -> String { + pub fn display_match_option_artist(match_option: &MatchOption) -> String { + Self::display_match_option(Self::display_option_artist, match_option) + } + + pub fn display_match_option_album(match_option: &MatchOption) -> String { + Self::display_match_option(Self::display_option_album, match_option) + } + + fn display_match_option(display_fn: Fn, match_option: &MatchOption) -> String + where + Fn: FnOnce(&T, &Option) -> String, + { match match_option { MatchOption::Some(match_artist) => format!( - "{} ({}%)", - Self::display_option_artist(&match_artist.item, &match_artist.disambiguation), - match_artist.score, + "{}{}", + display_fn(&match_artist.entity, &match_artist.disambiguation), + Self::display_option_score(match_artist.score), ), MatchOption::CannotHaveMbid => Self::display_cannot_have_mbid().to_string(), MatchOption::ManualInputMbid => Self::display_manual_input_mbid().to_string(), } } - pub fn display_lookup_option_artist(lookup_option: &MatchOption>) -> String { - match lookup_option { - MatchOption::Some(match_artist) => { - Self::display_option_artist(&match_artist.item, &match_artist.disambiguation) - } - MatchOption::CannotHaveMbid => Self::display_cannot_have_mbid().to_string(), - MatchOption::ManualInputMbid => Self::display_manual_input_mbid().to_string(), - } - } - fn display_option_artist(artist: &ArtistMeta, disambiguation: &Option) -> String { format!( "{}{}", @@ -163,27 +159,7 @@ impl UiDisplay { ) } - pub fn display_search_option_album(match_option: &MatchOption>) -> String { - match match_option { - MatchOption::Some(match_album) => format!( - "{} ({}%)", - Self::display_option_album(&match_album.item), - match_album.score, - ), - MatchOption::CannotHaveMbid => Self::display_cannot_have_mbid().to_string(), - MatchOption::ManualInputMbid => Self::display_manual_input_mbid().to_string(), - } - } - - pub fn display_lookup_option_album(lookup_option: &MatchOption>) -> String { - match lookup_option { - MatchOption::Some(match_album) => Self::display_option_album(&match_album.item), - MatchOption::CannotHaveMbid => Self::display_cannot_have_mbid().to_string(), - MatchOption::ManualInputMbid => Self::display_manual_input_mbid().to_string(), - } - } - - fn display_option_album(album: &AlbumMeta) -> String { + fn display_option_album(album: &AlbumMeta, _disambiguation: &Option) -> String { format!( "{:010} | {} [{}]", UiDisplay::display_album_date(&album.date), @@ -192,6 +168,10 @@ impl UiDisplay { ) } + fn display_option_score(score: Option) -> String { + score.map(|s| format!(" ({s}%)")).unwrap_or_default() + } + fn display_cannot_have_mbid() -> &'static str { "-- Cannot have a MusicBrainz Identifier --" } diff --git a/src/tui/ui/match_state.rs b/src/tui/ui/match_state.rs index bf611fe..99725b1 100644 --- a/src/tui/ui/match_state.rs +++ b/src/tui/ui/match_state.rs @@ -1,8 +1,11 @@ -use musichoard::collection::{album::AlbumMeta, artist::ArtistMeta}; +use musichoard::collection::{ + album::{AlbumId, AlbumMeta}, + artist::ArtistMeta, +}; use ratatui::widgets::{List, ListItem}; use crate::tui::{ - app::{ListOption, MatchStateInfo, WidgetState}, + app::{EntityMatches, MatchOption, WidgetState}, ui::display::UiDisplay, }; @@ -13,28 +16,21 @@ pub struct MatchOverlay<'a, 'b> { } impl<'a, 'b> MatchOverlay<'a, 'b> { - pub fn new(info: &'a MatchStateInfo, state: &'b mut WidgetState) -> Self { + pub fn new(info: &'a EntityMatches, state: &'b mut WidgetState) -> Self { match info { - MatchStateInfo::Artist(m) => Self::artists(&m.matching, &m.list, state), - MatchStateInfo::Album(m) => Self::albums(&m.matching, &m.list, state), + EntityMatches::Artist(m) => Self::artists(&m.matching, &m.list, state), + EntityMatches::Album(m) => Self::albums(&m.matching, &m.list, state), } } fn artists( matching: &ArtistMeta, - matches: &'a ListOption, + matches: &'a [MatchOption], state: &'b mut WidgetState, ) -> Self { let matching = UiDisplay::display_artist_matching(matching); - let list = match matches { - ListOption::Search(matches) => { - Self::display_list(UiDisplay::display_search_option_artist, matches) - } - ListOption::Lookup(matches) => { - Self::display_list(UiDisplay::display_lookup_option_artist, matches) - } - }; + let list = Self::display_list(UiDisplay::display_match_option_artist, matches); MatchOverlay { matching, @@ -44,20 +40,13 @@ impl<'a, 'b> MatchOverlay<'a, 'b> { } fn albums( - matching: &AlbumMeta, - matches: &'a ListOption, + matching: &AlbumId, + matches: &'a [MatchOption], state: &'b mut WidgetState, ) -> Self { let matching = UiDisplay::display_album_matching(matching); - let list = match matches { - ListOption::Search(matches) => { - Self::display_list(UiDisplay::display_search_option_album, matches) - } - ListOption::Lookup(matches) => { - Self::display_list(UiDisplay::display_lookup_option_album, matches) - } - }; + let list = Self::display_list(UiDisplay::display_match_option_album, matches); MatchOverlay { matching, diff --git a/src/tui/ui/minibuffer.rs b/src/tui/ui/minibuffer.rs index 7db8361..625de51 100644 --- a/src/tui/ui/minibuffer.rs +++ b/src/tui/ui/minibuffer.rs @@ -65,7 +65,7 @@ impl Minibuffer<'_> { }, AppState::Match(public) => Minibuffer { paragraphs: vec![ - Paragraph::new(UiDisplay::display_matching_info(public.info)), + Paragraph::new(UiDisplay::display_matching_info(public.matches)), Paragraph::new("ctrl+g: abort"), ], columns: 2, diff --git a/src/tui/ui/mod.rs b/src/tui/ui/mod.rs index 5cf579c..a633a0f 100644 --- a/src/tui/ui/mod.rs +++ b/src/tui/ui/mod.rs @@ -18,7 +18,7 @@ use musichoard::collection::{album::Album, Collection}; use crate::tui::{ app::{ - AppPublicState, AppState, Category, IAppAccess, InputPublic, MatchStateInfo, Selection, + AppPublicState, AppState, Category, EntityMatches, IAppAccess, InputPublic, Selection, WidgetState, }, ui::{ @@ -140,7 +140,7 @@ impl Ui { UiWidget::render_overlay_widget("Fetching", fetch_text, area, false, frame) } - fn render_match_overlay(info: &MatchStateInfo, state: &mut WidgetState, frame: &mut Frame) { + fn render_match_overlay(info: &EntityMatches, state: &mut WidgetState, frame: &mut Frame) { let area = OverlayBuilder::default().build(frame.area()); let st = MatchOverlay::new(info, state); UiWidget::render_overlay_list_widget(&st.matching, st.list, st.state, true, area, frame) @@ -182,7 +182,9 @@ impl IUi for Ui { AppState::Info(()) => Self::render_info_overlay(collection, selection, frame), AppState::Reload(()) => Self::render_reload_overlay(frame), AppState::Fetch(()) => Self::render_fetch_overlay(frame), - AppState::Match(public) => Self::render_match_overlay(public.info, public.state, frame), + AppState::Match(public) => { + Self::render_match_overlay(public.matches, public.state, frame) + } AppState::Error(msg) => Self::render_error_overlay("Error", msg, frame), AppState::Critical(msg) => Self::render_error_overlay("Critical Error", msg, frame), _ => {} @@ -204,7 +206,7 @@ mod tests { use crate::tui::{ app::{AppPublic, AppPublicInner, Delta, MatchStatePublic}, - lib::interface::musicbrainz::api::{Lookup, Match}, + lib::interface::musicbrainz::api::Entity, testmod::COLLECTION, tests::terminal, }; @@ -226,7 +228,7 @@ mod tests { AppState::Search(s) => AppState::Search(s), AppState::Fetch(()) => AppState::Fetch(()), AppState::Match(ref mut m) => AppState::Match(MatchStatePublic { - info: m.info, + matches: m.matches, state: m.state, }), AppState::Error(s) => AppState::Error(s), @@ -329,22 +331,22 @@ mod tests { ArtistMeta::new(ArtistId::new("an artist")) } - fn artist_matches() -> MatchStateInfo { + fn artist_matches() -> EntityMatches { let artist = artist_meta(); - let artist_match = Match::new(80, artist.clone()); + let artist_match = Entity::with_score(artist.clone(), 80); let list = vec![artist_match.clone(), artist_match.clone()]; - let mut info = MatchStateInfo::artist_search(artist, list); + let mut info = EntityMatches::artist_search(artist, list); info.push_cannot_have_mbid(); info.push_manual_input_mbid(); info } - fn artist_lookup() -> MatchStateInfo { + fn artist_lookup() -> EntityMatches { let artist = artist_meta(); - let artist_lookup = Lookup::new(artist.clone()); + let artist_lookup = Entity::new(artist.clone()); - let mut info = MatchStateInfo::artist_lookup(artist, artist_lookup); + let mut info = EntityMatches::artist_lookup(artist, artist_lookup); info.push_cannot_have_mbid(); info.push_manual_input_mbid(); info @@ -354,9 +356,13 @@ mod tests { ArtistId::new("Artist") } - fn album_meta() -> AlbumMeta { + fn album_id() -> AlbumId { + AlbumId::new("An Album") + } + + fn album_meta(id: AlbumId) -> AlbumMeta { AlbumMeta::new( - AlbumId::new("An Album"), + id, AlbumDate::new(Some(1990), Some(5), None), AlbumInfo::new( MbRefOption::None, @@ -366,24 +372,26 @@ mod tests { ) } - fn album_matches() -> MatchStateInfo { + fn album_matches() -> EntityMatches { let artist_id = album_artist_id(); - let album = album_meta(); - let album_match = Match::new(80, album.clone()); + let album_id = album_id(); + let album_meta = album_meta(album_id.clone()); + let album_match = Entity::with_score(album_meta.clone(), 80); let list = vec![album_match.clone(), album_match.clone()]; - let mut info = MatchStateInfo::album_search(artist_id, album, list); + let mut info = EntityMatches::album_search(artist_id, album_id, list); info.push_cannot_have_mbid(); info.push_manual_input_mbid(); info } - fn album_lookup() -> MatchStateInfo { + fn album_lookup() -> EntityMatches { let artist_id = album_artist_id(); - let album = album_meta(); - let album_lookup = Lookup::new(album.clone()); + let album_id = album_id(); + let album_meta = album_meta(album_id.clone()); + let album_lookup = Entity::new(album_meta.clone()); - let mut info = MatchStateInfo::album_lookup(artist_id, album, album_lookup); + let mut info = EntityMatches::album_lookup(artist_id, album_id, album_lookup); info.push_cannot_have_mbid(); info.push_manual_input_mbid(); info @@ -403,13 +411,13 @@ mod tests { album_lookup(), ]; - for info in match_state_infos.iter() { + for matches in match_state_infos.iter() { let mut widget_state = WidgetState::default().with_selected(Some(0)); let mut app = AppPublic { inner: public_inner(collection, &mut selection), state: AppState::Match(MatchStatePublic { - info, + matches, state: &mut widget_state, }), input: None,