diff --git a/src/bin/musichoard-edit.rs b/src/bin/musichoard-edit.rs index 1881574..58f322a 100644 --- a/src/bin/musichoard-edit.rs +++ b/src/bin/musichoard-edit.rs @@ -206,7 +206,7 @@ impl MusicBrainzCommand { fn handle(self, music_hoard: &mut MH, artist_name: &str) { match self { MusicBrainzCommand::Set(musicbrainz_value) => music_hoard - .set_artist_musicbrainz(ArtistId::new(artist_name), musicbrainz_value.url) + .set_artist_musicbrainz_from_url(ArtistId::new(artist_name), musicbrainz_value.url) .expect("failed to set MusicBrainz URL"), MusicBrainzCommand::Clear => music_hoard .clear_artist_musicbrainz(ArtistId::new(artist_name)) diff --git a/src/core/collection/album.rs b/src/core/collection/album.rs index 59e48a8..7d92bd9 100644 --- a/src/core/collection/album.rs +++ b/src/core/collection/album.rs @@ -204,13 +204,29 @@ impl AlbumMeta { self.seq = AlbumSeq::default(); } - pub fn set_musicbrainz_ref(&mut self, mbref: MbAlbumRef) { - self.musicbrainz.replace(mbref); + pub fn set_musicbrainz_ref(&mut self, mbref: MbRefOption) { + _ = mem::replace(&mut self.musicbrainz, mbref); } pub fn clear_musicbrainz_ref(&mut self) { self.musicbrainz.take(); } + + pub fn set_primary_type(&mut self, primary_type: Option) { + _ = mem::replace(&mut self.primary_type, primary_type); + } + + pub fn clear_primary_type(&mut self) { + self.primary_type.take(); + } + + pub fn set_secondary_types(&mut self, secondary_types: Vec) { + self.secondary_types = secondary_types; + } + + pub fn clear_secondary_types(&mut self) { + self.secondary_types.clear(); + } } impl PartialOrd for AlbumMeta { @@ -368,20 +384,20 @@ mod tests { assert_eq!(album.meta.musicbrainz, expected); // Setting a URL on an album. - album - .meta - .set_musicbrainz_ref(MbAlbumRef::from_url_str(MUSICBRAINZ).unwrap()); + album.meta.set_musicbrainz_ref(MbRefOption::Some( + MbAlbumRef::from_url_str(MUSICBRAINZ).unwrap(), + )); expected.replace(MbAlbumRef::from_url_str(MUSICBRAINZ).unwrap()); assert_eq!(album.meta.musicbrainz, expected); - album - .meta - .set_musicbrainz_ref(MbAlbumRef::from_url_str(MUSICBRAINZ).unwrap()); + album.meta.set_musicbrainz_ref(MbRefOption::Some( + MbAlbumRef::from_url_str(MUSICBRAINZ).unwrap(), + )); assert_eq!(album.meta.musicbrainz, expected); - album - .meta - .set_musicbrainz_ref(MbAlbumRef::from_url_str(MUSICBRAINZ_2).unwrap()); + album.meta.set_musicbrainz_ref(MbRefOption::Some( + MbAlbumRef::from_url_str(MUSICBRAINZ_2).unwrap(), + )); expected.replace(MbAlbumRef::from_url_str(MUSICBRAINZ_2).unwrap()); assert_eq!(album.meta.musicbrainz, expected); diff --git a/src/core/collection/artist.rs b/src/core/collection/artist.rs index 2a9ff05..53aa878 100644 --- a/src/core/collection/artist.rs +++ b/src/core/collection/artist.rs @@ -92,8 +92,8 @@ impl ArtistMeta { self.sort.take(); } - pub fn set_musicbrainz_ref(&mut self, mbref: MbArtistRef) { - self.musicbrainz.replace(mbref); + pub fn set_musicbrainz_ref(&mut self, mbref: MbRefOption) { + self.musicbrainz = mbref } pub fn clear_musicbrainz_ref(&mut self) { @@ -259,20 +259,20 @@ mod tests { assert_eq!(artist.meta.musicbrainz, expected); // Setting a URL on an artist. - artist - .meta - .set_musicbrainz_ref(MbArtistRef::from_url_str(MUSICBRAINZ).unwrap()); + artist.meta.set_musicbrainz_ref(MbRefOption::Some( + MbArtistRef::from_url_str(MUSICBRAINZ).unwrap(), + )); expected.replace(MbArtistRef::from_url_str(MUSICBRAINZ).unwrap()); assert_eq!(artist.meta.musicbrainz, expected); - artist - .meta - .set_musicbrainz_ref(MbArtistRef::from_url_str(MUSICBRAINZ).unwrap()); + artist.meta.set_musicbrainz_ref(MbRefOption::Some( + MbArtistRef::from_url_str(MUSICBRAINZ).unwrap(), + )); assert_eq!(artist.meta.musicbrainz, expected); - artist - .meta - .set_musicbrainz_ref(MbArtistRef::from_url_str(MUSICBRAINZ_2).unwrap()); + artist.meta.set_musicbrainz_ref(MbRefOption::Some( + MbArtistRef::from_url_str(MUSICBRAINZ_2).unwrap(), + )); expected.replace(MbArtistRef::from_url_str(MUSICBRAINZ_2).unwrap()); assert_eq!(artist.meta.musicbrainz, expected); diff --git a/src/core/musichoard/database.rs b/src/core/musichoard/database.rs index 8c377c8..000f4d6 100644 --- a/src/core/musichoard/database.rs +++ b/src/core/musichoard/database.rs @@ -1,12 +1,18 @@ -use crate::core::{ +use crate::{ collection::{ - album::{Album, AlbumId, AlbumSeq}, - artist::{Artist, ArtistId}, - musicbrainz::MbArtistRef, - Collection, + album::{AlbumPrimaryType, AlbumSecondaryType}, + musicbrainz::{MbAlbumRef, MbRefOption}, + }, + core::{ + collection::{ + album::{Album, AlbumId, AlbumSeq}, + artist::{Artist, ArtistId}, + musicbrainz::MbArtistRef, + Collection, + }, + interface::database::IDatabase, + musichoard::{base::IMusicHoardBasePrivate, Error, MusicHoard, NoDatabase}, }, - interface::database::IDatabase, - musichoard::{base::IMusicHoardBasePrivate, Error, MusicHoard, NoDatabase}, }; pub trait IMusicHoardDatabase { @@ -22,7 +28,12 @@ pub trait IMusicHoardDatabase { ) -> Result<(), Error>; fn clear_artist_sort>(&mut self, artist_id: Id) -> Result<(), Error>; - fn set_artist_musicbrainz, S: AsRef>( + fn set_artist_musicbrainz>( + &mut self, + artist_id: Id, + mbid: MbRefOption, + ) -> Result<(), Error>; + fn set_artist_musicbrainz_from_url, S: AsRef>( &mut self, artist_id: Id, url: S, @@ -54,6 +65,17 @@ pub trait IMusicHoardDatabase { property: S, ) -> Result<(), Error>; + fn set_album_musicbrainz, AlbumIdRef: AsRef>( + &mut self, + artist_id: Id, + album_id: AlbumIdRef, + mbid: MbRefOption, + ) -> Result<(), Error>; + fn clear_album_musicbrainz, AlbumIdRef: AsRef>( + &mut self, + artist_id: Id, + album_id: AlbumIdRef, + ) -> Result<(), Error>; fn set_album_seq, AlbumIdRef: AsRef>( &mut self, artist_id: ArtistIdRef, @@ -65,6 +87,28 @@ pub trait IMusicHoardDatabase { artist_id: ArtistIdRef, album_id: AlbumIdRef, ) -> Result<(), Error>; + fn set_album_primary_type, AlbumIdRef: AsRef>( + &mut self, + artist_id: Id, + album_id: AlbumIdRef, + primary_type: Option, + ) -> Result<(), Error>; + fn clear_album_primary_type, AlbumIdRef: AsRef>( + &mut self, + artist_id: Id, + album_id: AlbumIdRef, + ) -> Result<(), Error>; + fn set_album_secondary_types, AlbumIdRef: AsRef>( + &mut self, + artist_id: Id, + album_id: AlbumIdRef, + secondary_types: Vec, + ) -> Result<(), Error>; + fn clear_album_secondary_types, AlbumIdRef: AsRef>( + &mut self, + artist_id: Id, + album_id: AlbumIdRef, + ) -> Result<(), Error>; } impl IMusicHoardDatabase for MusicHoard { @@ -120,14 +164,24 @@ impl IMusicHoardDatabase for MusicHoard, S: AsRef>( + fn set_artist_musicbrainz>( + &mut self, + artist_id: Id, + mbid: MbRefOption, + ) -> Result<(), Error> { + self.update_artist(artist_id.as_ref(), |artist| { + artist.meta.set_musicbrainz_ref(mbid) + }) + } + + fn set_artist_musicbrainz_from_url, S: AsRef>( &mut self, artist_id: Id, url: S, ) -> Result<(), Error> { let mb = MbArtistRef::from_url_str(url)?; self.update_artist(artist_id.as_ref(), |artist| { - artist.meta.set_musicbrainz_ref(mb) + artist.meta.set_musicbrainz_ref(MbRefOption::Some(mb)) }) } @@ -183,6 +237,27 @@ impl IMusicHoardDatabase for MusicHoard, AlbumIdRef: AsRef>( + &mut self, + artist_id: Id, + album_id: AlbumIdRef, + mbid: MbRefOption, + ) -> Result<(), Error> { + self.update_album(artist_id.as_ref(), album_id.as_ref(), |album| { + album.meta.set_musicbrainz_ref(mbid) + }) + } + + fn clear_album_musicbrainz, AlbumIdRef: AsRef>( + &mut self, + artist_id: Id, + album_id: AlbumIdRef, + ) -> Result<(), Error> { + self.update_album(artist_id.as_ref(), album_id.as_ref(), |album| { + album.meta.clear_musicbrainz_ref() + }) + } + fn set_album_seq, AlbumIdRef: AsRef>( &mut self, artist_id: ArtistIdRef, @@ -194,7 +269,6 @@ impl IMusicHoardDatabase for MusicHoard IMusicHoardDatabase for MusicHoard, AlbumIdRef: AsRef>( + &mut self, + artist_id: Id, + album_id: AlbumIdRef, + primary_type: Option, + ) -> Result<(), Error> { + self.update_album(artist_id.as_ref(), album_id.as_ref(), |album| { + album.meta.set_primary_type(primary_type) + }) + } + + fn clear_album_primary_type, AlbumIdRef: AsRef>( + &mut self, + artist_id: Id, + album_id: AlbumIdRef, + ) -> Result<(), Error> { + self.update_album(artist_id.as_ref(), album_id.as_ref(), |album| { + album.meta.clear_primary_type() + }) + } + + fn set_album_secondary_types, AlbumIdRef: AsRef>( + &mut self, + artist_id: Id, + album_id: AlbumIdRef, + secondary_types: Vec, + ) -> Result<(), Error> { + self.update_album(artist_id.as_ref(), album_id.as_ref(), |album| { + album.meta.set_secondary_types(secondary_types) + }) + } + + fn clear_album_secondary_types, AlbumIdRef: AsRef>( + &mut self, + artist_id: Id, + album_id: AlbumIdRef, + ) -> Result<(), Error> { + self.update_album(artist_id.as_ref(), album_id.as_ref(), |album| { + album.meta.clear_secondary_types() + }) + } } pub trait IMusicHoardDatabasePrivate { @@ -272,24 +387,34 @@ impl MusicHoard { self.update_artist_and(artist_id, fn_artist, |_| {}) } - fn update_album_and( + fn update_album_and( &mut self, artist_id: &ArtistId, album_id: &AlbumId, fn_album: FnAlbum, fn_artist: FnArtist, - fn_coll: FnColl, ) -> Result<(), Error> where FnAlbum: FnOnce(&mut Album), FnArtist: FnOnce(&mut Artist), - FnColl: FnOnce(&mut Collection), { let artist = Self::get_artist_mut_or_err(&mut self.pre_commit, artist_id)?; let album = Self::get_album_mut_or_err(artist, album_id)?; fn_album(album); fn_artist(artist); - self.update_collection(fn_coll) + self.update_collection(|_| {}) + } + + fn update_album( + &mut self, + artist_id: &ArtistId, + album_id: &AlbumId, + fn_album: FnAlbum, + ) -> Result<(), Error> + where + FnAlbum: FnOnce(&mut Album), + { + self.update_album_and(artist_id, album_id, fn_album, |_| {}) } } @@ -423,7 +548,7 @@ mod tests { assert!(music_hoard.add_artist(artist_id.clone()).is_ok()); let actual_err = music_hoard - .set_artist_musicbrainz(&artist_id, MUSICBUTLER) + .set_artist_musicbrainz_from_url(&artist_id, MUSICBUTLER) .unwrap_err(); let expected_err = Error::CollectionError(format!( "an error occurred when processing a URL: invalid artist MusicBrainz URL: {MUSICBUTLER}" @@ -449,13 +574,13 @@ mod tests { // Setting a URL on an artist not in the collection is an error. assert!(music_hoard - .set_artist_musicbrainz(&artist_id_2, MUSICBRAINZ) + .set_artist_musicbrainz_from_url(&artist_id_2, MUSICBRAINZ) .is_err()); assert_eq!(music_hoard.collection[0].meta.musicbrainz, expected); // Setting a URL on an artist. assert!(music_hoard - .set_artist_musicbrainz(&artist_id, MUSICBRAINZ) + .set_artist_musicbrainz_from_url(&artist_id, MUSICBRAINZ) .is_ok()); expected.replace(MbArtistRef::from_url_str(MUSICBRAINZ).unwrap()); assert_eq!(music_hoard.collection[0].meta.musicbrainz, expected); diff --git a/src/tui/app/machine/browse_state.rs b/src/tui/app/machine/browse_state.rs index 7f26314..a215740 100644 --- a/src/tui/app/machine/browse_state.rs +++ b/src/tui/app/machine/browse_state.rs @@ -73,7 +73,7 @@ impl IAppInteractBrowse for AppMachine { } fn fetch_musicbrainz(self) -> Self::APP { - AppMachine::app_fetch_new(self.inner) + AppMachine::app_fetch_first(self.inner) } } diff --git a/src/tui/app/machine/fetch_state.rs b/src/tui/app/machine/fetch_state.rs index cf53244..59730b3 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::AlbumMeta, - artist::{Artist, ArtistMeta}, + artist::{Artist, ArtistId, ArtistMeta}, musicbrainz::{IMusicBrainzRef, MbRefOption, Mbid}, }; @@ -46,12 +46,27 @@ 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) } - pub fn app_fetch_new(inner: AppInner) -> App { + pub fn app_fetch_first(inner: AppInner) -> App { + Self::app_fetch_new(inner, true) + } + + fn app_fetch_new(inner: AppInner, first: bool) -> App { let coll = inner.music_hoard.get_collection(); let artist = match inner.selection.state_artist(coll) { Some(artist_state) => &coll[artist_state.index], @@ -61,16 +76,38 @@ impl AppMachine { }; let (fetch_tx, fetch_rx) = mpsc::channel::(); - if let Err(err) = Self::submit_fetch_job(&*inner.musicbrainz, fetch_tx, artist) { - return AppMachine::error_state(inner, err.to_string()).into(); - } - let fetch = FetchState::new(fetch_rx); - AppMachine::app_fetch(inner, fetch, true) + match Self::submit_fetch_job(&*inner.musicbrainz, fetch_tx, artist) { + Ok(()) => AppMachine::fetch_state(inner, fetch).into(), + Err(FetchError::NothingToFetch) => { + if first { + AppMachine::match_state(inner, MatchState::new(None, fetch)).into() + } else { + AppMachine::browse_state(inner).into() + } + } + Err(FetchError::SubmitError(daemon_err)) => { + AppMachine::error_state(inner, daemon_err.to_string()).into() + } + } } - pub fn app_fetch_next(inner: AppInner, fetch: FetchState) -> App { - Self::app_fetch(inner, fetch, false) + pub fn app_fetch_next(inner: AppInner, mut fetch: FetchState) -> App { + match fetch.try_recv() { + Ok(fetch_result) => match fetch_result { + Ok(next_match) => { + let current = Some(next_match); + AppMachine::match_state(inner, MatchState::new(current, fetch)).into() + } + 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 => Self::app_fetch_new(inner, false), + }, + } } pub fn app_lookup_artist( @@ -86,10 +123,13 @@ impl AppMachine { pub fn app_lookup_album( inner: AppInner, fetch: FetchState, + artist_id: &ArtistId, album: &AlbumMeta, mbid: Mbid, ) -> App { - let f = Self::submit_lookup_release_group_job; + 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) } @@ -111,48 +151,33 @@ impl AppMachine { Self::app_fetch_next(inner, fetch) } - fn app_fetch(inner: AppInner, mut fetch: FetchState, first: bool) -> App { - match fetch.try_recv() { - Ok(fetch_result) => match fetch_result { - Ok(next_match) => { - let current = Some(next_match); - AppMachine::match_state(inner, MatchState::new(current, fetch)).into() - } - 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 => { - if first { - AppMachine::match_state(inner, MatchState::new(None, fetch)).into() - } else { - AppMachine::browse_state(inner).into() - } - } - }, - } - } - fn submit_fetch_job( musicbrainz: &dyn IMbJobSender, result_sender: ResultSender, artist: &Artist, - ) -> Result<(), DaemonError> { + ) -> Result<(), FetchError> { let requests = match artist.meta.musicbrainz { MbRefOption::Some(ref arid) => { let arid = arid.mbid(); let albums = artist.albums.iter(); albums .filter(|album| matches!(album.meta.musicbrainz, MbRefOption::None)) - .map(|album| MbParams::search_release_group(arid.clone(), album.meta.clone())) + .map(|album| { + MbParams::search_release_group( + artist.meta.id.clone(), + arid.clone(), + album.meta.clone(), + ) + }) .collect() } - MbRefOption::CannotHaveMbid => return Ok(()), + MbRefOption::CannotHaveMbid => VecDeque::new(), MbRefOption::None => VecDeque::from([MbParams::search_artist(artist.meta.clone())]), }; - musicbrainz.submit_background_job(result_sender, requests) + if requests.is_empty() { + return Err(FetchError::NothingToFetch); + } + Ok(musicbrainz.submit_background_job(result_sender, requests)?) } fn submit_lookup_artist_job( @@ -168,10 +193,15 @@ impl AppMachine { fn submit_lookup_release_group_job( musicbrainz: &dyn IMbJobSender, result_sender: ResultSender, + artist_id: &ArtistId, album: &AlbumMeta, mbid: Mbid, ) -> Result<(), DaemonError> { - let requests = VecDeque::from([MbParams::lookup_release_group(album.clone(), mbid)]); + let requests = VecDeque::from([MbParams::lookup_release_group( + artist_id.clone(), + album.clone(), + mbid, + )]); musicbrainz.submit_foreground_job(result_sender, requests) } } @@ -207,7 +237,11 @@ impl IAppEventFetch for AppMachine { #[cfg(test)] mod tests { use mockall::predicate; - use musichoard::collection::{album::AlbumMeta, artist::ArtistMeta, musicbrainz::Mbid}; + use musichoard::collection::{ + album::AlbumMeta, + artist::{ArtistId, ArtistMeta}, + musicbrainz::Mbid, + }; use crate::tui::{ app::{ @@ -257,18 +291,23 @@ mod tests { #[test] fn fetch_no_artist() { - let app = AppMachine::app_fetch_new(inner(music_hoard(vec![]))); + let app = AppMachine::app_fetch_first(inner(music_hoard(vec![]))); assert!(matches!(app.state(), AppState::Error(_))); } fn search_release_group_expectation( job_sender: &mut MockIMbJobSender, - arid: &Mbid, + artist_id: &ArtistId, + artist_mbid: &Mbid, albums: &[AlbumMeta], ) { let mut requests = VecDeque::new(); for album in albums.iter() { - requests.push_back(MbParams::search_release_group(arid.clone(), album.clone())); + requests.push_back(MbParams::search_release_group( + artist_id.clone(), + artist_mbid.clone(), + album.clone(), + )); } job_sender .expect_submit_background_job() @@ -281,13 +320,19 @@ mod tests { fn fetch_albums() { let mut mb_job_sender = MockIMbJobSender::new(); - let arid: Mbid = "11111111-1111-1111-1111-111111111111".try_into().unwrap(); + let artist_id = COLLECTION[1].meta.id.clone(); + let artist_mbid: Mbid = "11111111-1111-1111-1111-111111111111".try_into().unwrap(); let album_1_meta = COLLECTION[1].albums[0].meta.clone(); let album_4_meta = COLLECTION[1].albums[3].meta.clone(); // Other albums have an MBID and so they will be skipped. - search_release_group_expectation(&mut mb_job_sender, &arid, &[album_1_meta, album_4_meta]); + search_release_group_expectation( + &mut mb_job_sender, + &artist_id, + &artist_mbid, + &[album_1_meta, album_4_meta], + ); let music_hoard = music_hoard(COLLECTION.to_owned()); let inner = AppInner::new(music_hoard, mb_job_sender); @@ -300,8 +345,16 @@ mod tests { assert!(matches!(app, AppState::Match(_))); } - fn lookup_album_expectation(job_sender: &mut MockIMbJobSender, album: &AlbumMeta) { - let requests = VecDeque::from([MbParams::lookup_release_group(album.clone(), mbid())]); + fn lookup_album_expectation( + job_sender: &mut MockIMbJobSender, + artist_id: &ArtistId, + album: &AlbumMeta, + ) { + let requests = VecDeque::from([MbParams::lookup_release_group( + artist_id.clone(), + album.clone(), + mbid(), + )]); job_sender .expect_submit_foreground_job() .with(predicate::always(), predicate::eq(requests)) @@ -313,8 +366,9 @@ mod tests { fn lookup_album() { 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, &album); + lookup_album_expectation(&mut mb_job_sender, &artist_id, &album); let music_hoard = music_hoard(COLLECTION.to_owned()); let inner = AppInner::new(music_hoard, mb_job_sender); @@ -322,7 +376,7 @@ mod tests { let (_fetch_tx, fetch_rx) = mpsc::channel(); let fetch = FetchState::new(fetch_rx); - AppMachine::app_lookup_album(inner, fetch, &album, mbid()); + AppMachine::app_lookup_album(inner, fetch, &artist_id, &album, mbid()); } fn search_artist_expectation(job_sender: &mut MockIMbJobSender, artist: &ArtistMeta) { @@ -442,7 +496,7 @@ mod tests { let inner = inner(music_hoard(COLLECTION.clone())); let fetch = FetchState::new(rx); - let mut app = AppMachine::app_fetch(inner, fetch, true); + let mut app = AppMachine::app_fetch_next(inner, fetch); assert!(matches!(app, AppState::Match(_))); let public = app.get(); @@ -465,7 +519,7 @@ mod tests { let inner = inner(music_hoard(COLLECTION.clone())); let fetch = FetchState::new(rx); - let app = AppMachine::app_fetch(inner, fetch, true); + let app = AppMachine::app_fetch_next(inner, fetch); assert!(matches!(app, AppState::Error(_))); } @@ -475,7 +529,7 @@ mod tests { let inner = inner(music_hoard(COLLECTION.clone())); let fetch = FetchState::new(rx); - let app = AppMachine::app_fetch(inner, fetch, true); + let app = AppMachine::app_fetch_next(inner, fetch); assert!(matches!(app, AppState::Fetch(_))); } @@ -485,7 +539,7 @@ mod tests { let inner = inner(music_hoard(COLLECTION.clone())); let fetch = FetchState::new(rx); - let app = AppMachine::app_fetch(inner, fetch, true); + let app = AppMachine::app_fetch_next(inner, fetch); assert!(matches!(app, AppState::Match(_))); } @@ -504,7 +558,7 @@ mod tests { let inner = inner(music_hoard(COLLECTION.clone())); let fetch = FetchState::new(rx); - let app = AppMachine::app_fetch(inner, fetch, true); + let app = AppMachine::app_fetch_next(inner, fetch); assert!(matches!(app, AppState::Fetch(_))); let artist = COLLECTION[3].meta.clone(); diff --git a/src/tui/app/machine/match_state.rs b/src/tui/app/machine/match_state.rs index d633319..b90e979 100644 --- a/src/tui/app/machine/match_state.rs +++ b/src/tui/app/machine/match_state.rs @@ -1,13 +1,100 @@ use std::cmp; -use musichoard::collection::musicbrainz::Mbid; - -use crate::tui::app::{ - machine::{fetch_state::FetchState, input::Input, App, AppInner, AppMachine}, - AlbumMatches, AppPublicState, AppState, ArtistMatches, IAppInteractMatch, ListOption, - LookupOption, MatchStateInfo, MatchStatePublic, MissOption, SearchOption, WidgetState, +use musichoard::collection::{ + album::AlbumMeta, + artist::{ArtistId, ArtistMeta}, + musicbrainz::{MbRefOption, Mbid}, }; +use crate::tui::{ + app::{ + machine::{fetch_state::FetchState, input::Input, App, AppInner, AppMachine}, + AlbumMatches, AppPublicState, AppState, ArtistMatches, IAppInteractMatch, ListOption, + LookupOption, MatchStateInfo, MatchStatePublic, MissOption, SearchOption, WidgetState, + }, + lib::IMusicHoard, +}; + +impl LookupOption { + fn set( + self, + music_hoard: &mut dyn IMusicHoard, + meta: &ArtistMeta, + ) -> Result<(), musichoard::Error> { + let mbref = match self { + LookupOption::Match(lookup) => lookup.item.musicbrainz, + LookupOption::None(MissOption::CannotHaveMbid) => MbRefOption::CannotHaveMbid, + _ => panic!(), + }; + music_hoard.set_artist_musicbrainz(&meta.id, mbref) + } +} + +impl SearchOption { + fn set( + self, + music_hoard: &mut dyn IMusicHoard, + meta: &ArtistMeta, + ) -> Result<(), musichoard::Error> { + let mbref = match self { + SearchOption::Match(search) => search.item.musicbrainz, + SearchOption::None(MissOption::CannotHaveMbid) => MbRefOption::CannotHaveMbid, + _ => panic!(), + }; + music_hoard.set_artist_musicbrainz(&meta.id, mbref) + } +} + +impl LookupOption { + fn set( + self, + music_hoard: &mut dyn IMusicHoard, + artist: &ArtistId, + meta: &AlbumMeta, + ) -> Result<(), musichoard::Error> { + let (mbref, primary_type, secondary_types) = match self { + LookupOption::Match(lookup) => ( + lookup.item.musicbrainz, + lookup.item.primary_type, + lookup.item.secondary_types, + ), + LookupOption::None(MissOption::CannotHaveMbid) => { + (MbRefOption::CannotHaveMbid, None, Vec::new()) + } + _ => panic!(), + }; + music_hoard.set_album_musicbrainz(artist, &meta.id, mbref)?; + music_hoard.set_album_primary_type(artist, &meta.id, primary_type)?; + music_hoard.set_album_secondary_types(artist, &meta.id, secondary_types)?; + Ok(()) + } +} + +impl SearchOption { + fn set( + self, + music_hoard: &mut dyn IMusicHoard, + artist: &ArtistId, + meta: &AlbumMeta, + ) -> Result<(), musichoard::Error> { + let (mbref, primary_type, secondary_types) = match self { + SearchOption::Match(lookup) => ( + lookup.item.musicbrainz, + lookup.item.primary_type, + lookup.item.secondary_types, + ), + SearchOption::None(MissOption::CannotHaveMbid) => { + (MbRefOption::CannotHaveMbid, None, Vec::new()) + } + _ => panic!(), + }; + music_hoard.set_album_musicbrainz(artist, &meta.id, mbref)?; + music_hoard.set_album_primary_type(artist, &meta.id, primary_type)?; + music_hoard.set_album_secondary_types(artist, &meta.id, secondary_types)?; + Ok(()) + } +} + impl ListOption { fn len(&self) -> usize { match self { @@ -42,6 +129,35 @@ impl ListOption { } } +impl ListOption { + fn set( + self, + music_hoard: &mut dyn IMusicHoard, + meta: &ArtistMeta, + index: usize, + ) -> Result<(), musichoard::Error> { + match self { + ListOption::Lookup(mut list) => list.swap_remove(index).set(music_hoard, meta), + ListOption::Search(mut list) => list.swap_remove(index).set(music_hoard, meta), + } + } +} + +impl ListOption { + fn set( + self, + music_hoard: &mut dyn IMusicHoard, + artist: &ArtistId, + meta: &AlbumMeta, + index: usize, + ) -> Result<(), musichoard::Error> { + match self { + ListOption::Lookup(mut list) => list.swap_remove(index).set(music_hoard, artist, meta), + ListOption::Search(mut list) => list.swap_remove(index).set(music_hoard, artist, meta), + } + } +} + impl ArtistMatches { fn len(&self) -> usize { self.list.len() @@ -58,6 +174,10 @@ impl ArtistMatches { fn is_manual_input_mbid(&self, index: usize) -> bool { self.list.is_manual_input_mbid(index) } + + fn set(self, music_hoard: &mut dyn IMusicHoard, index: usize) -> Result<(), musichoard::Error> { + self.list.set(music_hoard, &self.matching, index) + } } impl AlbumMatches { @@ -76,6 +196,11 @@ impl AlbumMatches { fn is_manual_input_mbid(&self, index: usize) -> bool { self.list.is_manual_input_mbid(index) } + + fn set(self, music_hoard: &mut dyn IMusicHoard, index: usize) -> Result<(), musichoard::Error> { + self.list + .set(music_hoard, &self.artist, &self.matching, index) + } } impl MatchStateInfo { @@ -106,6 +231,13 @@ impl MatchStateInfo { Self::Album(a) => a.is_manual_input_mbid(index), } } + + fn set(self, music_hoard: &mut dyn IMusicHoard, index: usize) -> Result<(), musichoard::Error> { + match self { + Self::Artist(a) => a.set(music_hoard, index), + Self::Album(a) => a.set(music_hoard, index), + } + } } pub struct MatchState { @@ -146,8 +278,15 @@ impl AppMachine { AppMachine::app_lookup_artist(self.inner, self.state.fetch, matching, mbid) } MatchStateInfo::Album(album_matches) => { + let artist_id = &album_matches.artist; let matching = &album_matches.matching; - AppMachine::app_lookup_album(self.inner, self.state.fetch, matching, mbid) + AppMachine::app_lookup_album( + self.inner, + self.state.fetch, + artist_id, + matching, + mbid, + ) } } } @@ -197,6 +336,7 @@ impl IAppInteractMatch for AppMachine { fn select(mut self) -> Self::APP { if let Some(index) = self.state.state.list.selected() { // selected() implies current exists + if self .state .current @@ -207,7 +347,17 @@ impl IAppInteractMatch for AppMachine { self.input.replace(Input::default()); return self.into(); } + + if let Err(err) = self + .state + .current + .unwrap() + .set(&mut *self.inner.music_hoard, index) + { + return AppMachine::error_state(self.inner, err.to_string()).into(); + } } + AppMachine::app_fetch_next(self.inner, self.state.fetch) } @@ -279,6 +429,7 @@ mod tests { } fn album_match() -> MatchStateInfo { + let artist_id = ArtistId::new("Artist"); let album = AlbumMeta::new( AlbumId::new("Album"), AlbumDate::new(Some(1990), Some(5), None), @@ -295,10 +446,11 @@ mod tests { let album_match_2 = Match::new(100, album_2); let list = vec![album_match_1.clone(), album_match_2.clone()]; - MatchStateInfo::album_search(album, list) + MatchStateInfo::album_search(artist_id, album, list) } fn album_lookup() -> MatchStateInfo { + let artist_id = ArtistId::new("Artist"); let album = AlbumMeta::new( AlbumId::new("Album"), AlbumDate::new(Some(1990), Some(5), None), @@ -306,7 +458,7 @@ mod tests { vec![AlbumSecondaryType::Live, AlbumSecondaryType::Compilation], ); let lookup = Lookup::new(album.clone()); - MatchStateInfo::album_lookup(album, lookup) + MatchStateInfo::album_lookup(artist_id, album, lookup) } fn fetch_state() -> FetchState { @@ -518,15 +670,21 @@ mod tests { #[test] fn select_manual_input_album() { let mut mb_job_sender = MockIMbJobSender::new(); + let artist_id = ArtistId::new("Artist"); let album = AlbumMeta::new("Album", 1990, None, vec![]); - let requests = VecDeque::from([MbParams::lookup_release_group(album.clone(), mbid())]); + let requests = VecDeque::from([MbParams::lookup_release_group( + artist_id.clone(), + album.clone(), + mbid(), + )]); mb_job_sender .expect_submit_foreground_job() .with(predicate::always(), predicate::eq(requests)) .return_once(|_, _| Ok(())); let matches_vec: Vec> = vec![]; - let album_match = MatchStateInfo::album_search(album.clone(), matches_vec); + let album_match = + MatchStateInfo::album_search(artist_id.clone(), album.clone(), matches_vec); let matches = AppMachine::match_state( inner_with_mb(music_hoard(vec![]), mb_job_sender), match_state(Some(album_match)), diff --git a/src/tui/app/mod.rs b/src/tui/app/mod.rs index 67c8a35..e36a195 100644 --- a/src/tui/app/mod.rs +++ b/src/tui/app/mod.rs @@ -4,7 +4,11 @@ mod selection; pub use machine::App; pub use selection::{Category, Delta, Selection, WidgetState}; -use musichoard::collection::{album::AlbumMeta, artist::ArtistMeta, Collection}; +use musichoard::collection::{ + album::AlbumMeta, + artist::{ArtistId, ArtistMeta}, + Collection, +}; use crate::tui::lib::interface::musicbrainz::api::Match; @@ -220,6 +224,7 @@ pub struct ArtistMatches { #[derive(Clone, Debug, PartialEq, Eq)] pub struct AlbumMatches { + pub artist: ArtistId, pub matching: AlbumMeta, pub list: ListOption, } @@ -240,11 +245,16 @@ impl MatchStateInfo { } pub fn album_search>>( + artist: ArtistId, matching: AlbumMeta, list: Vec, ) -> Self { let list = ListOption::Search(list.into_iter().map(Into::into).collect()); - MatchStateInfo::Album(AlbumMatches { matching, list }) + MatchStateInfo::Album(AlbumMatches { + artist, + matching, + list, + }) } pub fn artist_lookup>>(matching: ArtistMeta, item: M) -> Self { @@ -252,9 +262,17 @@ impl MatchStateInfo { MatchStateInfo::Artist(ArtistMatches { matching, list }) } - pub fn album_lookup>>(matching: AlbumMeta, item: M) -> Self { + pub fn album_lookup>>( + artist: ArtistId, + matching: AlbumMeta, + item: M, + ) -> Self { let list = ListOption::Lookup(vec![item.into()]); - MatchStateInfo::Album(AlbumMatches { matching, list }) + MatchStateInfo::Album(AlbumMatches { + artist, + matching, + list, + }) } } diff --git a/src/tui/lib/external/musicbrainz/daemon/mod.rs b/src/tui/lib/external/musicbrainz/daemon/mod.rs index 5db771c..91981c6 100644 --- a/src/tui/lib/external/musicbrainz/daemon/mod.rs +++ b/src/tui/lib/external/musicbrainz/daemon/mod.rs @@ -245,15 +245,15 @@ impl JobInstance { .map(|rv| MatchStateInfo::artist_lookup(params.artist, rv)), LookupParams::ReleaseGroup(params) => musicbrainz .lookup_release_group(¶ms.mbid) - .map(|rv| MatchStateInfo::album_lookup(params.album, rv)), + .map(|rv| MatchStateInfo::album_lookup(params.artist_id, params.album, rv)), }, 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.arid, ¶ms.album) - .map(|rv| MatchStateInfo::album_search(params.album, rv)), + .search_release_group(¶ms.artist_mbid, ¶ms.album) + .map(|rv| MatchStateInfo::album_search(params.artist_id, params.album, rv)), }, }; self.return_result(event_sender, result) @@ -315,7 +315,7 @@ mod tests { use mockall::{predicate, Sequence}; use musichoard::collection::{ album::AlbumMeta, - artist::ArtistMeta, + artist::{ArtistId, ArtistMeta}, musicbrainz::{IMusicBrainzRef, MbRefOption, Mbid}, }; @@ -397,9 +397,10 @@ 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 mbid = mbid(); - VecDeque::from([MbParams::lookup_release_group(album, mbid)]) + VecDeque::from([MbParams::lookup_release_group(artist_id, album, mbid)]) } fn search_artist_requests() -> VecDeque { @@ -421,15 +422,20 @@ mod tests { let mbref = mb_ref_opt_as_ref(&COLLECTION[1].meta.musicbrainz); let arid = mb_ref_opt_unwrap(mbref).mbid().clone(); + let artist_id = COLLECTION[1].meta.id.clone(); let album_1 = COLLECTION[1].albums[0].meta.clone(); let album_4 = COLLECTION[1].albums[3].meta.clone(); VecDeque::from([ - MbParams::search_release_group(arid.clone(), album_1), - MbParams::search_release_group(arid.clone(), album_4), + MbParams::search_release_group(artist_id.clone(), arid.clone(), album_1), + MbParams::search_release_group(artist_id.clone(), arid.clone(), album_4), ]) } + fn album_artist_id() -> ArtistId { + COLLECTION[1].meta.id.clone() + } + fn album_arid_expectation() -> Mbid { let mbref = mb_ref_opt_as_ref(&COLLECTION[1].meta.musicbrainz); mb_ref_opt_unwrap(mbref).mbid().clone() @@ -611,7 +617,11 @@ mod tests { assert_eq!(result, Ok(())); let result = result_receiver.try_recv().unwrap(); - assert_eq!(result, Ok(MatchStateInfo::album_lookup(album, lookup))); + let artist_id = album_artist_id(); + assert_eq!( + result, + Ok(MatchStateInfo::album_lookup(artist_id, album, lookup)) + ); } fn search_artist_expectation( @@ -701,11 +711,27 @@ mod tests { let result = daemon.execute_next_job(); assert_eq!(result, Ok(())); - let result = result_receiver.try_recv().unwrap(); - assert_eq!(result, Ok(MatchStateInfo::album_search(album_1, matches_1))); + let artist_id = album_artist_id(); let result = result_receiver.try_recv().unwrap(); - assert_eq!(result, Ok(MatchStateInfo::album_search(album_4, matches_4))); + assert_eq!( + result, + Ok(MatchStateInfo::album_search( + artist_id.clone(), + album_1, + matches_1 + )) + ); + + let result = result_receiver.try_recv().unwrap(); + assert_eq!( + result, + Ok(MatchStateInfo::album_search( + artist_id.clone(), + album_4, + matches_4 + )) + ); } #[test] diff --git a/src/tui/lib/interface/musicbrainz/daemon/mod.rs b/src/tui/lib/interface/musicbrainz/daemon/mod.rs index 964ec56..0a2b468 100644 --- a/src/tui/lib/interface/musicbrainz/daemon/mod.rs +++ b/src/tui/lib/interface/musicbrainz/daemon/mod.rs @@ -1,6 +1,10 @@ use std::{collections::VecDeque, fmt, sync::mpsc}; -use musichoard::collection::{album::AlbumMeta, artist::ArtistMeta, musicbrainz::Mbid}; +use musichoard::collection::{ + album::AlbumMeta, + artist::{ArtistId, ArtistMeta}, + musicbrainz::Mbid, +}; use crate::tui::{app::MatchStateInfo, lib::interface::musicbrainz::api::Error as MbApiError}; @@ -60,6 +64,7 @@ pub struct LookupArtistParams { #[derive(Clone, Debug, PartialEq, Eq)] pub struct LookupReleaseGroupParams { + pub artist_id: ArtistId, pub album: AlbumMeta, pub mbid: Mbid, } @@ -77,7 +82,8 @@ pub struct SearchArtistParams { #[derive(Clone, Debug, PartialEq, Eq)] pub struct SearchReleaseGroupParams { - pub arid: Mbid, + pub artist_id: ArtistId, + pub artist_mbid: Mbid, pub album: AlbumMeta, } @@ -86,8 +92,9 @@ impl MbParams { MbParams::Lookup(LookupParams::Artist(LookupArtistParams { artist, mbid })) } - pub fn lookup_release_group(album: AlbumMeta, mbid: Mbid) -> Self { + pub fn lookup_release_group(artist_id: ArtistId, album: AlbumMeta, mbid: Mbid) -> Self { MbParams::Lookup(LookupParams::ReleaseGroup(LookupReleaseGroupParams { + artist_id, album, mbid, })) @@ -97,9 +104,10 @@ impl MbParams { MbParams::Search(SearchParams::Artist(SearchArtistParams { artist })) } - pub fn search_release_group(arid: Mbid, album: AlbumMeta) -> Self { + pub fn search_release_group(artist_id: ArtistId, artist_mbid: Mbid, album: AlbumMeta) -> Self { MbParams::Search(SearchParams::ReleaseGroup(SearchReleaseGroupParams { - arid, + artist_id, + artist_mbid, album, })) } diff --git a/src/tui/lib/mod.rs b/src/tui/lib/mod.rs index 31643b2..ff89baa 100644 --- a/src/tui/lib/mod.rs +++ b/src/tui/lib/mod.rs @@ -2,7 +2,12 @@ pub mod external; pub mod interface; use musichoard::{ - collection::Collection, + collection::{ + album::{AlbumId, AlbumPrimaryType, AlbumSecondaryType}, + artist::ArtistId, + musicbrainz::{MbAlbumRef, MbArtistRef, MbRefOption}, + Collection, + }, interface::{database::IDatabase, library::ILibrary}, IMusicHoardBase, IMusicHoardDatabase, IMusicHoardLibrary, MusicHoard, }; @@ -15,6 +20,30 @@ pub trait IMusicHoard { fn rescan_library(&mut self) -> Result<(), musichoard::Error>; fn reload_database(&mut self) -> Result<(), musichoard::Error>; fn get_collection(&self) -> &Collection; + + fn set_artist_musicbrainz( + &mut self, + id: &ArtistId, + mbref: MbRefOption, + ) -> Result<(), musichoard::Error>; + fn set_album_musicbrainz( + &mut self, + artist_id: &ArtistId, + album_id: &AlbumId, + mbref: MbRefOption, + ) -> Result<(), musichoard::Error>; + fn set_album_primary_type( + &mut self, + artist_id: &ArtistId, + album_id: &AlbumId, + primary_type: Option, + ) -> Result<(), musichoard::Error>; + fn set_album_secondary_types( + &mut self, + artist_id: &ArtistId, + album_id: &AlbumId, + secondary_types: Vec, + ) -> Result<(), musichoard::Error>; } // GRCOV_EXCL_START @@ -30,5 +59,50 @@ impl IMusicHoard for MusicHoard &Collection { ::get_collection(self) } + + fn set_artist_musicbrainz( + &mut self, + id: &ArtistId, + mbref: MbRefOption, + ) -> Result<(), musichoard::Error> { + ::set_artist_musicbrainz(self, id, mbref) + } + + fn set_album_musicbrainz( + &mut self, + artist_id: &ArtistId, + album_id: &AlbumId, + mbref: MbRefOption, + ) -> Result<(), musichoard::Error> { + ::set_album_musicbrainz(self, artist_id, album_id, mbref) + } + + fn set_album_primary_type( + &mut self, + artist_id: &ArtistId, + album_id: &AlbumId, + primary_type: Option, + ) -> Result<(), musichoard::Error> { + ::set_album_primary_type( + self, + artist_id, + album_id, + primary_type, + ) + } + + fn set_album_secondary_types( + &mut self, + artist_id: &ArtistId, + album_id: &AlbumId, + secondary_types: Vec, + ) -> Result<(), musichoard::Error> { + ::set_album_secondary_types( + self, + artist_id, + album_id, + secondary_types, + ) + } } // GRCOV_EXCL_STOP diff --git a/src/tui/ui/mod.rs b/src/tui/ui/mod.rs index 94c441b..ec058dc 100644 --- a/src/tui/ui/mod.rs +++ b/src/tui/ui/mod.rs @@ -373,6 +373,10 @@ mod tests { info } + fn album_artist_id() -> ArtistId { + ArtistId::new("Artist") + } + fn album_meta() -> AlbumMeta { AlbumMeta::new( AlbumId::new("An Album"), @@ -383,21 +387,23 @@ mod tests { } fn album_matches() -> MatchStateInfo { + let artist_id = album_artist_id(); let album = album_meta(); let album_match = Match::new(80, album.clone()); let list = vec![album_match.clone(), album_match.clone()]; - let mut info = MatchStateInfo::album_search(album, list); + let mut info = MatchStateInfo::album_search(artist_id, album, list); info.push_cannot_have_mbid(); info.push_manual_input_mbid(); info } fn album_lookup() -> MatchStateInfo { + let artist_id = album_artist_id(); let album = album_meta(); let album_lookup = Lookup::new(album.clone()); - let mut info = MatchStateInfo::album_lookup(album, album_lookup); + let mut info = MatchStateInfo::album_lookup(artist_id, album, album_lookup); info.push_cannot_have_mbid(); info.push_manual_input_mbid(); info