From 7e63999edf48b4a57f0c59df553941137c7ceb09 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sat, 5 Oct 2024 20:43:33 +0200 Subject: [PATCH 01/15] Add browse to TUI API and other simplifcations --- examples/musicbrainz_api/browse.rs | 4 - src/external/musicbrainz/api/browse.rs | 4 +- src/external/musicbrainz/api/mod.rs | 4 +- src/external/musicbrainz/api/search/mod.rs | 4 +- src/tui/app/machine/fetch_state.rs | 10 +- src/tui/app/machine/match_state.rs | 132 ++++++------------ src/tui/app/machine/mod.rs | 7 +- src/tui/app/mod.rs | 37 ++--- src/tui/lib/external/musicbrainz/api/mod.rs | 121 +++++++++------- .../lib/external/musicbrainz/daemon/mod.rs | 22 +-- src/tui/lib/interface/musicbrainz/api/mod.rs | 32 +++-- src/tui/ui/display.rs | 62 +++----- src/tui/ui/match_state.rs | 24 +--- src/tui/ui/mod.rs | 10 +- 14 files changed, 203 insertions(+), 270 deletions(-) diff --git a/examples/musicbrainz_api/browse.rs b/examples/musicbrainz_api/browse.rs index af8d968..20dcc5c 100644 --- a/examples/musicbrainz_api/browse.rs +++ b/examples/musicbrainz_api/browse.rs @@ -66,14 +66,10 @@ 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), diff --git a/src/external/musicbrainz/api/browse.rs b/src/external/musicbrainz/api/browse.rs index 9d1a21d..dc63ed8 100644 --- a/src/external/musicbrainz/api/browse.rs +++ b/src/external/musicbrainz/api/browse.rs @@ -16,8 +16,8 @@ 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 { diff --git a/src/external/musicbrainz/api/mod.rs b/src/external/musicbrainz/api/mod.rs index 2f61f75..9f374da 100644 --- a/src/external/musicbrainz/api/mod.rs +++ b/src/external/musicbrainz/api/mod.rs @@ -63,8 +63,8 @@ impl MusicBrainzClient { #[derive(Default)] pub struct PageSettings { - pub limit: Option, - pub offset: Option, + limit: Option, + offset: Option, } impl PageSettings { diff --git a/src/external/musicbrainz/api/search/mod.rs b/src/external/musicbrainz/api/search/mod.rs index 6fcb4cc..f76c010 100644 --- a/src/external/musicbrainz/api/search/mod.rs +++ b/src/external/musicbrainz/api/search/mod.rs @@ -28,8 +28,8 @@ 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 { diff --git a/src/tui/app/machine/fetch_state.rs b/src/tui/app/machine/fetch_state.rs index c8a15b3..94aaefc 100644 --- a/src/tui/app/machine/fetch_state.rs +++ b/src/tui/app/machine/fetch_state.rs @@ -290,11 +290,7 @@ mod tests { machine::tests::{inner, music_hoard}, Delta, IApp, IAppAccess, IAppInteractBrowse, MatchOption, MatchStateInfo, }, - lib::interface::musicbrainz::{ - self, - api::{Lookup, Match}, - daemon::MockIMbJobSender, - }, + lib::interface::musicbrainz::{self, api::Match, daemon::MockIMbJobSender}, testmod::COLLECTION, }; @@ -320,7 +316,7 @@ mod tests { assert_eq!(fetch.try_recv(), Err(TryRecvError::Empty)); - let lookup = Lookup::new(artist.clone()); + let lookup = Match::item(artist.clone()); let lookup_result = MatchStateInfo::artist_lookup(artist.clone(), lookup); lookup_tx.send(Ok(lookup_result.clone())).unwrap(); @@ -605,7 +601,7 @@ 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 = Match::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); diff --git a/src/tui/app/machine/match_state.rs b/src/tui/app/machine/match_state.rs index 19e0b3c..844c862 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, IAppInteractMatch, MatchOption, + MatchStateInfo, 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 into_info(&self, info: Self::InfoType) -> InfoOption; } enum InfoOption { @@ -35,86 +32,46 @@ 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 into_info(&self, mut info: Self::InfoType) -> InfoOption { + match self { + MatchOption::Some(option) => info.musicbrainz = option.item.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 into_info(&self, mut info: Self::InfoType) -> InfoOption { 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.item.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, info: Self::InfoType) -> 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, info: Self::InfoType) -> InfoOption { + self.get(index).unwrap().into_info(info) } } @@ -124,11 +81,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,11 +95,11 @@ 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); } } @@ -314,7 +271,7 @@ mod tests { IApp, IAppAccess, IAppInput, }, lib::interface::musicbrainz::{ - api::{Lookup, Match}, + api::Match, daemon::{MbParams, MockIMbJobSender}, }, }; @@ -322,18 +279,9 @@ mod tests { use super::*; impl Match { - pub fn new(score: u8, item: T) -> Self { + pub fn with_score(item: T, score: u8) -> Self { Match { - score, - item, - disambiguation: None, - } - } - } - - impl Lookup { - pub fn new(item: T) -> Self { - Lookup { + score: Some(score), item, disambiguation: None, } @@ -354,10 +302,10 @@ mod tests { let artist = artist_meta(); let artist_1 = artist.clone(); - let artist_match_1 = Match::new(100, artist_1); + let artist_match_1 = Match::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 = Match::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()]; @@ -366,7 +314,7 @@ mod tests { fn artist_lookup() -> MatchStateInfo { let artist = artist_meta(); - let lookup = Lookup::new(artist.clone()); + let lookup = Match::item(artist.clone()); MatchStateInfo::artist_lookup(artist, lookup) } @@ -387,12 +335,12 @@ mod tests { let album = album_meta(); let album_1 = album.clone(); - let album_match_1 = Match::new(100, album_1); + let album_match_1 = Match::with_score(album_1, 100); let mut album_2 = album.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 = Match::with_score(album_2, 100); let list = vec![album_match_1.clone(), album_match_2.clone()]; MatchStateInfo::album_search(artist_id, album, list) @@ -401,7 +349,7 @@ mod tests { fn album_lookup() -> MatchStateInfo { let artist_id = ArtistId::new("Artist"); let album = album_meta(); - let lookup = Lookup::new(album.clone()); + let lookup = Match::item(album.clone()); MatchStateInfo::album_lookup(artist_id, album, lookup) } diff --git a/src/tui/app/machine/mod.rs b/src/tui/app/machine/mod.rs index 9873c25..6747d01 100644 --- a/src/tui/app/machine/mod.rs +++ b/src/tui/app/machine/mod.rs @@ -226,10 +226,7 @@ mod tests { use crate::tui::{ app::{AppState, IApp, IAppInput, IAppInteractBrowse, InputEvent, MatchStateInfo}, - lib::{ - interface::musicbrainz::{api::Lookup, daemon::MockIMbJobSender}, - MockIMusicHoard, - }, + lib::{interface::musicbrainz::{api::Match, daemon::MockIMbJobSender}, MockIMusicHoard}, }; use super::*; @@ -520,7 +517,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 = MatchStateInfo::artist_lookup(artist.clone(), Match::item(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..637c8df 100644 --- a/src/tui/app/mod.rs +++ b/src/tui/app/mod.rs @@ -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(Match), 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: Match) -> Self { MatchOption::Some(value) } } @@ -237,14 +229,14 @@ 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 list: Vec>, } #[derive(Clone, Debug, PartialEq, Eq)] @@ -254,20 +246,20 @@ pub enum MatchStateInfo { } impl MatchStateInfo { - pub fn artist_search>>>( + pub fn artist_search>>( matching: ArtistMeta, list: Vec, ) -> Self { - let list = ListOption::Search(list.into_iter().map(Into::into).collect()); + let list = list.into_iter().map(Into::into).collect(); MatchStateInfo::Artist(ArtistMatches { matching, list }) } - pub fn album_search>>>( + pub fn album_search>>( artist: ArtistId, matching: AlbumMeta, list: Vec, ) -> Self { - let list = ListOption::Search(list.into_iter().map(Into::into).collect()); + let list = list.into_iter().map(Into::into).collect(); MatchStateInfo::Album(AlbumMatches { artist, matching, @@ -275,20 +267,17 @@ impl MatchStateInfo { }) } - pub fn artist_lookup>>>( - matching: ArtistMeta, - item: M, - ) -> Self { - let list = ListOption::Lookup(vec![item.into()]); + pub fn artist_lookup>>(matching: ArtistMeta, item: M) -> Self { + let list = vec![item.into()]; MatchStateInfo::Artist(ArtistMatches { matching, list }) } - pub fn album_lookup>>>( + pub fn album_lookup>>( artist: ArtistId, matching: AlbumMeta, item: M, ) -> Self { - let list = ListOption::Lookup(vec![item.into()]); + let list = vec![item.into()]; MatchStateInfo::Album(AlbumMatches { artist, matching, diff --git a/src/tui/lib/external/musicbrainz/api/mod.rs b/src/tui/lib/external/musicbrainz/api/mod.rs index 35156c5..6724289 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::{Error, IMusicBrainz, Match, Paged}; // 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)?; @@ -92,52 +93,75 @@ 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 PageSettings, + ) -> Result>>, Error> { + let request = BrowseReleaseGroupRequest::artist(artist); + + let mb_response = self.client.browse_release_group(&request, paging)?; + + let page_count = mb_response.release_groups.len(); + let next = mb_response.page.next_page_offset(page_count); + let item = from_browse_release_group_response(mb_response); + + Ok(Paged { item, next }) } } -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(entity: LookupArtistResponse) -> Match { + let (item, disambiguation) = from_mb_artist_meta(entity.meta); + Match { + score: None, + item, + disambiguation, + } +} + +fn from_lookup_release_group_response(entity: LookupReleaseGroupResponse) -> Match { + Match { + score: None, + item: from_mb_release_group_meta(entity.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); + let (item, disambiguation) = from_mb_artist_meta(entity.meta); 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, + score: Some(entity.score), + item, + disambiguation, } } @@ -145,18 +169,17 @@ 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(), - }, - }, + score: Some(entity.score), + item: from_mb_release_group_meta(entity.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(|meta| Match::item(meta)).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..b793322 100644 --- a/src/tui/lib/external/musicbrainz/daemon/mod.rs +++ b/src/tui/lib/external/musicbrainz/daemon/mod.rs @@ -321,7 +321,7 @@ mod tests { use crate::tui::{ event::{Event, EventError, MockIFetchCompleteEventSender}, - lib::interface::musicbrainz::api::{Lookup, Match, MockIMusicBrainz}, + lib::interface::musicbrainz::api::{Match, MockIMusicBrainz}, testmod::COLLECTION, }; @@ -411,8 +411,8 @@ mod tests { 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 = Match::with_score(artist.clone(), 100); + let artist_match_2 = Match::with_score(artist.clone(), 50); let matches = vec![artist_match_1.clone(), artist_match_2.clone()]; (artist, matches) @@ -445,8 +445,8 @@ mod tests { 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 = Match::with_score(album_1.clone(), 100); + let album_match_1_2 = Match::with_score(album_4.clone(), 50); let matches_1 = vec![album_match_1_1.clone(), album_match_1_2.clone()]; (album_1, matches_1) @@ -456,8 +456,8 @@ mod tests { 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 = Match::with_score(album_4.clone(), 100); + let album_match_4_2 = Match::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 +539,7 @@ mod tests { fn lookup_artist_expectation( musicbrainz: &mut MockIMusicBrainz, mbid: &Mbid, - lookup: &Lookup, + lookup: &Match, ) { let result = Ok(lookup.clone()); musicbrainz @@ -554,7 +554,7 @@ mod tests { let mut musicbrainz = musicbrainz(); let mbid = mbid(); let artist = COLLECTION[3].meta.clone(); - let lookup = Lookup::new(artist.clone()); + let lookup = Match::item(artist.clone()); lookup_artist_expectation(&mut musicbrainz, &mbid, &lookup); let mut event_sender = event_sender(); @@ -581,7 +581,7 @@ mod tests { fn lookup_release_group_expectation( musicbrainz: &mut MockIMusicBrainz, mbid: &Mbid, - lookup: &Lookup, + lookup: &Match, ) { let result = Ok(lookup.clone()); musicbrainz @@ -596,7 +596,7 @@ mod tests { let mut musicbrainz = musicbrainz(); let mbid = mbid(); let album = COLLECTION[1].albums[0].meta.clone(); - let lookup = Lookup::new(album.clone()); + let lookup = Match::item(album.clone()); lookup_release_group_expectation(&mut musicbrainz, &mbid, &lookup); let mut event_sender = event_sender(); diff --git a/src/tui/lib/interface/musicbrainz/api/mod.rs b/src/tui/lib/interface/musicbrainz/api/mod.rs index 62917c0..2cc618b 100644 --- a/src/tui/lib/interface/musicbrainz/api/mod.rs +++ b/src/tui/lib/interface/musicbrainz/api/mod.rs @@ -3,32 +3,48 @@ #[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::{NextPage, 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 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>; + fn browse_release_group( + &mut self, + artist: &Mbid, + paging: &mut PageSettings, + ) -> Result>>, Error>; } #[derive(Clone, Debug, PartialEq, Eq)] pub struct Match { - pub score: u8, + pub score: Option, pub item: T, pub disambiguation: Option, } -#[derive(Clone, Debug, PartialEq, Eq)] -pub struct Lookup { +impl Match { + pub fn item(item: T) -> Self { + Match { + score: None, + item, + disambiguation: None, + } + } +} + +pub struct Paged { pub item: T, - pub disambiguation: Option, + pub next: NextPage, } pub type Error = musichoard::external::musicbrainz::api::Error; diff --git a/src/tui/ui/display.rs b/src/tui/ui/display.rs index 30c3950..cfc78df 100644 --- a/src/tui/ui/display.rs +++ b/src/tui/ui/display.rs @@ -5,10 +5,7 @@ use musichoard::collection::{ track::{TrackFormat, TrackQuality}, }; -use crate::tui::{ - app::{MatchOption, MatchStateInfo}, - lib::interface::musicbrainz::api::{Lookup, Match}, -}; +use crate::tui::app::{MatchOption, MatchStateInfo}; pub struct UiDisplay; @@ -129,23 +126,24 @@ impl UiDisplay { } } - pub fn display_search_option_artist(match_option: &MatchOption>) -> String { - match match_option { - MatchOption::Some(match_artist) => format!( - "{} ({}%)", - Self::display_option_artist(&match_artist.item, &match_artist.disambiguation), - match_artist.score, - ), - MatchOption::CannotHaveMbid => Self::display_cannot_have_mbid().to_string(), - MatchOption::ManualInputMbid => Self::display_manual_input_mbid().to_string(), - } + pub fn display_match_option_artist(match_option: &MatchOption) -> String { + Self::display_match_option(Self::display_option_artist, match_option) } - 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) - } + 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!( + "{}{}", + display_fn(&match_artist.item, &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(), } @@ -163,27 +161,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 +170,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..741ec26 100644 --- a/src/tui/ui/match_state.rs +++ b/src/tui/ui/match_state.rs @@ -2,7 +2,7 @@ use musichoard::collection::{album::AlbumMeta, artist::ArtistMeta}; use ratatui::widgets::{List, ListItem}; use crate::tui::{ - app::{ListOption, MatchStateInfo, WidgetState}, + app::{MatchOption, MatchStateInfo, WidgetState}, ui::display::UiDisplay, }; @@ -22,19 +22,12 @@ impl<'a, 'b> MatchOverlay<'a, 'b> { fn artists( matching: &ArtistMeta, - matches: &'a ListOption, + matches: &'a Vec>, 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, @@ -45,19 +38,12 @@ impl<'a, 'b> MatchOverlay<'a, 'b> { fn albums( matching: &AlbumMeta, - matches: &'a ListOption, + matches: &'a Vec>, 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/mod.rs b/src/tui/ui/mod.rs index 5cf579c..4827a3d 100644 --- a/src/tui/ui/mod.rs +++ b/src/tui/ui/mod.rs @@ -204,7 +204,7 @@ mod tests { use crate::tui::{ app::{AppPublic, AppPublicInner, Delta, MatchStatePublic}, - lib::interface::musicbrainz::api::{Lookup, Match}, + lib::interface::musicbrainz::api::Match, testmod::COLLECTION, tests::terminal, }; @@ -331,7 +331,7 @@ mod tests { fn artist_matches() -> MatchStateInfo { let artist = artist_meta(); - let artist_match = Match::new(80, artist.clone()); + let artist_match = Match::with_score(artist.clone(), 80); let list = vec![artist_match.clone(), artist_match.clone()]; let mut info = MatchStateInfo::artist_search(artist, list); @@ -342,7 +342,7 @@ mod tests { fn artist_lookup() -> MatchStateInfo { let artist = artist_meta(); - let artist_lookup = Lookup::new(artist.clone()); + let artist_lookup = Match::item(artist.clone()); let mut info = MatchStateInfo::artist_lookup(artist, artist_lookup); info.push_cannot_have_mbid(); @@ -369,7 +369,7 @@ 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 album_match = Match::with_score(album.clone(), 80); let list = vec![album_match.clone(), album_match.clone()]; let mut info = MatchStateInfo::album_search(artist_id, album, list); @@ -381,7 +381,7 @@ mod tests { fn album_lookup() -> MatchStateInfo { let artist_id = album_artist_id(); let album = album_meta(); - let album_lookup = Lookup::new(album.clone()); + let album_lookup = Match::item(album.clone()); let mut info = MatchStateInfo::album_lookup(artist_id, album, album_lookup); info.push_cannot_have_mbid(); -- 2.45.2 From 898d2e1469041e302e1a2b909d7f90e58b227f78 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sat, 5 Oct 2024 21:06:04 +0200 Subject: [PATCH 02/15] Rename Match to Entity to broaden applicability --- src/tui/app/machine/fetch_state.rs | 10 ++-- src/tui/app/machine/match_state.rs | 30 +++++------ src/tui/app/machine/mod.rs | 7 ++- src/tui/app/mod.rs | 8 +-- src/tui/lib/external/musicbrainz/api/mod.rs | 50 +++++++++---------- .../lib/external/musicbrainz/daemon/mod.rs | 32 ++++++------ src/tui/lib/interface/musicbrainz/api/mod.rs | 22 ++++---- src/tui/ui/display.rs | 2 +- src/tui/ui/match_state.rs | 4 +- src/tui/ui/mod.rs | 10 ++-- 10 files changed, 89 insertions(+), 86 deletions(-) diff --git a/src/tui/app/machine/fetch_state.rs b/src/tui/app/machine/fetch_state.rs index 94aaefc..bd48b2a 100644 --- a/src/tui/app/machine/fetch_state.rs +++ b/src/tui/app/machine/fetch_state.rs @@ -290,7 +290,7 @@ mod tests { machine::tests::{inner, music_hoard}, Delta, IApp, IAppAccess, IAppInteractBrowse, MatchOption, MatchStateInfo, }, - lib::interface::musicbrainz::{self, api::Match, daemon::MockIMbJobSender}, + lib::interface::musicbrainz::{self, api::Entity, daemon::MockIMbJobSender}, testmod::COLLECTION, }; @@ -310,13 +310,13 @@ mod tests { let artist = COLLECTION[3].meta.clone(); - let matches: Vec> = vec![]; + let matches: Vec> = vec![]; let fetch_result = MatchStateInfo::artist_search(artist.clone(), matches); fetch_tx.send(Ok(fetch_result.clone())).unwrap(); assert_eq!(fetch.try_recv(), Err(TryRecvError::Empty)); - let lookup = Match::item(artist.clone()); + let lookup = Entity::new(artist.clone()); let lookup_result = MatchStateInfo::artist_lookup(artist.clone(), lookup); lookup_tx.send(Ok(lookup_result.clone())).unwrap(); @@ -601,7 +601,7 @@ mod tests { let (tx, rx) = mpsc::channel::(); let artist = COLLECTION[3].meta.clone(); - let artist_match = Match::with_score(COLLECTION[2].meta.clone(), 80); + 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); @@ -677,7 +677,7 @@ mod tests { assert!(matches!(app, AppState::Fetch(_))); let artist = COLLECTION[3].meta.clone(); - let match_info = MatchStateInfo::artist_search::>(artist, vec![]); + let match_info = MatchStateInfo::artist_search::>(artist, vec![]); let fetch_result = Ok(match_info); tx.send(fetch_result).unwrap(); diff --git a/src/tui/app/machine/match_state.rs b/src/tui/app/machine/match_state.rs index 844c862..82f497e 100644 --- a/src/tui/app/machine/match_state.rs +++ b/src/tui/app/machine/match_state.rs @@ -37,7 +37,7 @@ impl GetInfo for MatchOption { fn into_info(&self, mut info: Self::InfoType) -> InfoOption { match self { - MatchOption::Some(option) => info.musicbrainz = option.item.info.musicbrainz.clone(), + MatchOption::Some(option) => info.musicbrainz = option.entity.info.musicbrainz.clone(), MatchOption::CannotHaveMbid => info.musicbrainz = MbRefOption::CannotHaveMbid, MatchOption::ManualInputMbid => return InfoOption::NeedInput, } @@ -50,7 +50,7 @@ impl GetInfo for MatchOption { fn into_info(&self, mut info: Self::InfoType) -> InfoOption { match self { - MatchOption::Some(option) => info = option.item.info.clone(), + MatchOption::Some(option) => info = option.entity.info.clone(), MatchOption::CannotHaveMbid => info.musicbrainz = MbRefOption::CannotHaveMbid, MatchOption::ManualInputMbid => return InfoOption::NeedInput, } @@ -271,18 +271,18 @@ mod tests { IApp, IAppAccess, IAppInput, }, lib::interface::musicbrainz::{ - api::Match, + api::Entity, daemon::{MbParams, MockIMbJobSender}, }, }; use super::*; - impl Match { - pub fn with_score(item: T, score: u8) -> Self { - Match { + impl Entity { + pub fn with_score(entity: T, score: u8) -> Self { + Entity { score: Some(score), - item, + entity, disambiguation: None, } } @@ -302,10 +302,10 @@ mod tests { let artist = artist_meta(); let artist_1 = artist.clone(); - let artist_match_1 = Match::with_score(artist_1, 100); + let artist_match_1 = Entity::with_score(artist_1, 100); let artist_2 = artist.clone(); - let mut artist_match_2 = Match::with_score(artist_2, 100); + 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()]; @@ -314,7 +314,7 @@ mod tests { fn artist_lookup() -> MatchStateInfo { let artist = artist_meta(); - let lookup = Match::item(artist.clone()); + let lookup = Entity::new(artist.clone()); MatchStateInfo::artist_lookup(artist, lookup) } @@ -335,12 +335,12 @@ mod tests { let album = album_meta(); let album_1 = album.clone(); - let album_match_1 = Match::with_score(album_1, 100); + let album_match_1 = Entity::with_score(album_1, 100); let mut album_2 = album.clone(); album_2.id.title.push_str(" extra title part"); album_2.info.secondary_types.pop(); - let album_match_2 = Match::with_score(album_2, 100); + 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) @@ -349,7 +349,7 @@ mod tests { fn album_lookup() -> MatchStateInfo { let artist_id = ArtistId::new("Artist"); let album = album_meta(); - let lookup = Match::item(album.clone()); + let lookup = Entity::new(album.clone()); MatchStateInfo::album_lookup(artist_id, album, lookup) } @@ -597,7 +597,7 @@ mod tests { .with(predicate::always(), predicate::eq(requests)) .return_once(|_, _| Ok(())); - let matches_vec: Vec> = vec![]; + let matches_vec: Vec> = vec![]; let artist_match = MatchStateInfo::artist_search(artist.clone(), matches_vec); let matches = AppMachine::match_state( inner_with_mb(music_hoard(vec![]), mb_job_sender), @@ -630,7 +630,7 @@ 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); let matches = AppMachine::match_state( diff --git a/src/tui/app/machine/mod.rs b/src/tui/app/machine/mod.rs index 6747d01..2f270f9 100644 --- a/src/tui/app/machine/mod.rs +++ b/src/tui/app/machine/mod.rs @@ -226,7 +226,10 @@ mod tests { use crate::tui::{ app::{AppState, IApp, IAppInput, IAppInteractBrowse, InputEvent, MatchStateInfo}, - lib::{interface::musicbrainz::{api::Match, daemon::MockIMbJobSender}, MockIMusicHoard}, + lib::{ + interface::musicbrainz::{api::Entity, daemon::MockIMbJobSender}, + MockIMusicHoard, + }, }; use super::*; @@ -517,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(), Match::item(artist.clone())); + let info = MatchStateInfo::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 637c8df..10e3b22 100644 --- a/src/tui/app/mod.rs +++ b/src/tui/app/mod.rs @@ -11,7 +11,7 @@ use musichoard::collection::{ Collection, }; -use crate::tui::lib::interface::musicbrainz::api::Match; +use crate::tui::lib::interface::musicbrainz::api::Entity; pub enum AppState { Browse(B), @@ -215,13 +215,13 @@ pub type InputPublic<'app> = &'app tui_input::Input; #[derive(Clone, Debug, PartialEq, Eq)] pub enum MatchOption { - Some(Match), + Some(Entity), CannotHaveMbid, ManualInputMbid, } -impl From> for MatchOption { - fn from(value: Match) -> Self { +impl From> for MatchOption { + fn from(value: Entity) -> Self { MatchOption::Some(value) } } diff --git a/src/tui/lib/external/musicbrainz/api/mod.rs b/src/tui/lib/external/musicbrainz/api/mod.rs index 6724289..115e0be 100644 --- a/src/tui/lib/external/musicbrainz/api/mod.rs +++ b/src/tui/lib/external/musicbrainz/api/mod.rs @@ -25,7 +25,7 @@ use musichoard::{ }, }; -use crate::tui::lib::interface::musicbrainz::api::{Error, IMusicBrainz, Match, Paged}; +use crate::tui::lib::interface::musicbrainz::api::{Entity, Error, IMusicBrainz, Paged}; // GRCOV_EXCL_START pub struct MusicBrainz { @@ -39,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)?; @@ -47,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)?; @@ -55,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(); @@ -72,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); @@ -98,7 +98,7 @@ impl IMusicBrainz for MusicBrainz { &mut self, artist: &Mbid, paging: &mut PageSettings, - ) -> Result>>, Error> { + ) -> Result>>, Error> { let request = BrowseReleaseGroupRequest::artist(artist); let mb_response = self.client.browse_release_group(&request, paging)?; @@ -139,47 +139,47 @@ fn from_mb_release_group_meta(meta: MbReleaseGroupMeta) -> AlbumMeta { } } -fn from_lookup_artist_response(entity: LookupArtistResponse) -> Match { - let (item, disambiguation) = from_mb_artist_meta(entity.meta); - Match { +fn from_lookup_artist_response(response: LookupArtistResponse) -> Entity { + let (entity, disambiguation) = from_mb_artist_meta(response.meta); + Entity { score: None, - item, + entity, disambiguation, } } -fn from_lookup_release_group_response(entity: LookupReleaseGroupResponse) -> Match { - Match { +fn from_lookup_release_group_response(response: LookupReleaseGroupResponse) -> Entity { + Entity { score: None, - item: from_mb_release_group_meta(entity.meta), + entity: from_mb_release_group_meta(response.meta), disambiguation: None, } } -fn from_search_artist_response_artist(entity: SearchArtistResponseArtist) -> Match { - let (item, disambiguation) = from_mb_artist_meta(entity.meta); - Match { - score: Some(entity.score), - item, +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: Some(entity.score), - item: from_mb_release_group_meta(entity.meta), + 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> { +) -> Vec> { let rgs = entity.release_groups.into_iter(); let metas = rgs.map(from_mb_release_group_meta); - metas.map(|meta| Match::item(meta)).collect() + 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 b793322..2007f35 100644 --- a/src/tui/lib/external/musicbrainz/daemon/mod.rs +++ b/src/tui/lib/external/musicbrainz/daemon/mod.rs @@ -321,7 +321,7 @@ mod tests { use crate::tui::{ event::{Event, EventError, MockIFetchCompleteEventSender}, - lib::interface::musicbrainz::api::{Match, MockIMusicBrainz}, + lib::interface::musicbrainz::api::{Entity, MockIMusicBrainz}, testmod::COLLECTION, }; @@ -408,11 +408,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::with_score(artist.clone(), 100); - let artist_match_2 = Match::with_score(artist.clone(), 50); + 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) @@ -441,23 +441,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::with_score(album_1.clone(), 100); - let album_match_1_2 = Match::with_score(album_4.clone(), 50); + 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::with_score(album_4.clone(), 100); - let album_match_4_2 = Match::with_score(album_1.clone(), 30); + 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 +539,7 @@ mod tests { fn lookup_artist_expectation( musicbrainz: &mut MockIMusicBrainz, mbid: &Mbid, - lookup: &Match, + lookup: &Entity, ) { let result = Ok(lookup.clone()); musicbrainz @@ -554,7 +554,7 @@ mod tests { let mut musicbrainz = musicbrainz(); let mbid = mbid(); let artist = COLLECTION[3].meta.clone(); - let lookup = Match::item(artist.clone()); + let lookup = Entity::new(artist.clone()); lookup_artist_expectation(&mut musicbrainz, &mbid, &lookup); let mut event_sender = event_sender(); @@ -581,7 +581,7 @@ mod tests { fn lookup_release_group_expectation( musicbrainz: &mut MockIMusicBrainz, mbid: &Mbid, - lookup: &Match, + lookup: &Entity, ) { let result = Ok(lookup.clone()); musicbrainz @@ -596,7 +596,7 @@ mod tests { let mut musicbrainz = musicbrainz(); let mbid = mbid(); let album = COLLECTION[1].albums[0].meta.clone(); - let lookup = Match::item(album.clone()); + let lookup = Entity::new(album.clone()); lookup_release_group_expectation(&mut musicbrainz, &mbid, &lookup); let mut event_sender = event_sender(); @@ -627,7 +627,7 @@ mod tests { fn search_artist_expectation( musicbrainz: &mut MockIMusicBrainz, artist: &ArtistMeta, - matches: &[Match], + matches: &[Entity], ) { let result = Ok(matches.to_owned()); musicbrainz @@ -669,7 +669,7 @@ mod tests { seq: &mut Sequence, arid: &Mbid, album: &AlbumMeta, - matches: &[Match], + matches: &[Entity], ) { let result = Ok(matches.to_owned()); musicbrainz diff --git a/src/tui/lib/interface/musicbrainz/api/mod.rs b/src/tui/lib/interface/musicbrainz/api/mod.rs index 2cc618b..cf70913 100644 --- a/src/tui/lib/interface/musicbrainz/api/mod.rs +++ b/src/tui/lib/interface/musicbrainz/api/mod.rs @@ -10,33 +10,33 @@ use musichoard::{ #[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 PageSettings, - ) -> Result>>, Error>; + ) -> Result>>, Error>; } #[derive(Clone, Debug, PartialEq, Eq)] -pub struct Match { +pub struct Entity { pub score: Option, - pub item: T, + pub entity: T, pub disambiguation: Option, } -impl Match { - pub fn item(item: T) -> Self { - Match { +impl Entity { + pub fn new(entity: T) -> Self { + Entity { score: None, - item, + entity, disambiguation: None, } } diff --git a/src/tui/ui/display.rs b/src/tui/ui/display.rs index cfc78df..6684fff 100644 --- a/src/tui/ui/display.rs +++ b/src/tui/ui/display.rs @@ -141,7 +141,7 @@ impl UiDisplay { match match_option { MatchOption::Some(match_artist) => format!( "{}{}", - display_fn(&match_artist.item, &match_artist.disambiguation), + display_fn(&match_artist.entity, &match_artist.disambiguation), Self::display_option_score(match_artist.score), ), MatchOption::CannotHaveMbid => Self::display_cannot_have_mbid().to_string(), diff --git a/src/tui/ui/match_state.rs b/src/tui/ui/match_state.rs index 741ec26..59f9dae 100644 --- a/src/tui/ui/match_state.rs +++ b/src/tui/ui/match_state.rs @@ -22,7 +22,7 @@ impl<'a, 'b> MatchOverlay<'a, 'b> { fn artists( matching: &ArtistMeta, - matches: &'a Vec>, + matches: &'a [MatchOption], state: &'b mut WidgetState, ) -> Self { let matching = UiDisplay::display_artist_matching(matching); @@ -38,7 +38,7 @@ impl<'a, 'b> MatchOverlay<'a, 'b> { fn albums( matching: &AlbumMeta, - matches: &'a Vec>, + matches: &'a [MatchOption], state: &'b mut WidgetState, ) -> Self { let matching = UiDisplay::display_album_matching(matching); diff --git a/src/tui/ui/mod.rs b/src/tui/ui/mod.rs index 4827a3d..5dc4c8e 100644 --- a/src/tui/ui/mod.rs +++ b/src/tui/ui/mod.rs @@ -204,7 +204,7 @@ mod tests { use crate::tui::{ app::{AppPublic, AppPublicInner, Delta, MatchStatePublic}, - lib::interface::musicbrainz::api::Match, + lib::interface::musicbrainz::api::Entity, testmod::COLLECTION, tests::terminal, }; @@ -331,7 +331,7 @@ mod tests { fn artist_matches() -> MatchStateInfo { let artist = artist_meta(); - let artist_match = Match::with_score(artist.clone(), 80); + 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); @@ -342,7 +342,7 @@ mod tests { fn artist_lookup() -> MatchStateInfo { let artist = artist_meta(); - let artist_lookup = Match::item(artist.clone()); + let artist_lookup = Entity::new(artist.clone()); let mut info = MatchStateInfo::artist_lookup(artist, artist_lookup); info.push_cannot_have_mbid(); @@ -369,7 +369,7 @@ mod tests { fn album_matches() -> MatchStateInfo { let artist_id = album_artist_id(); let album = album_meta(); - let album_match = Match::with_score(album.clone(), 80); + let album_match = Entity::with_score(album.clone(), 80); let list = vec![album_match.clone(), album_match.clone()]; let mut info = MatchStateInfo::album_search(artist_id, album, list); @@ -381,7 +381,7 @@ mod tests { fn album_lookup() -> MatchStateInfo { let artist_id = album_artist_id(); let album = album_meta(); - let album_lookup = Match::item(album.clone()); + let album_lookup = Entity::new(album.clone()); let mut info = MatchStateInfo::album_lookup(artist_id, album, album_lookup); info.push_cannot_have_mbid(); -- 2.45.2 From 46d38d9274719f72dc05e140ccd5c81b2c252027 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sat, 5 Oct 2024 21:29:08 +0200 Subject: [PATCH 03/15] Another renaming --- src/tui/app/machine/fetch_state.rs | 14 ++--- src/tui/app/machine/match_state.rs | 62 +++++++++---------- src/tui/app/machine/mod.rs | 4 +- src/tui/app/mod.rs | 14 ++--- .../lib/external/musicbrainz/daemon/mod.rs | 22 +++---- .../lib/interface/musicbrainz/daemon/mod.rs | 4 +- src/tui/ui/display.rs | 8 +-- src/tui/ui/match_state.rs | 8 +-- src/tui/ui/minibuffer.rs | 2 +- src/tui/ui/mod.rs | 30 ++++----- 10 files changed, 85 insertions(+), 83 deletions(-) diff --git a/src/tui/app/machine/fetch_state.rs b/src/tui/app/machine/fetch_state.rs index bd48b2a..740a3da 100644 --- a/src/tui/app/machine/fetch_state.rs +++ b/src/tui/app/machine/fetch_state.rs @@ -288,7 +288,7 @@ mod tests { use crate::tui::{ app::{ machine::tests::{inner, music_hoard}, - Delta, IApp, IAppAccess, IAppInteractBrowse, MatchOption, MatchStateInfo, + Delta, EntityMatches, IApp, IAppAccess, IAppInteractBrowse, MatchOption, }, lib::interface::musicbrainz::{self, api::Entity, daemon::MockIMbJobSender}, testmod::COLLECTION, @@ -311,13 +311,13 @@ mod tests { let artist = COLLECTION[3].meta.clone(); let matches: Vec> = vec![]; - let fetch_result = MatchStateInfo::artist_search(artist.clone(), matches); + let fetch_result = EntityMatches::artist_search(artist.clone(), matches); fetch_tx.send(Ok(fetch_result.clone())).unwrap(); assert_eq!(fetch.try_recv(), Err(TryRecvError::Empty)); let lookup = Entity::new(artist.clone()); - let lookup_result = MatchStateInfo::artist_lookup(artist.clone(), lookup); + let lookup_result = EntityMatches::artist_lookup(artist.clone(), lookup); lookup_tx.send(Ok(lookup_result.clone())).unwrap(); assert_eq!(fetch.try_recv(), Ok(Ok(lookup_result))); @@ -603,7 +603,7 @@ mod tests { let artist = COLLECTION[3].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()]); + EntityMatches::artist_search(artist.clone(), vec![artist_match.clone()]); let fetch_result = Ok(artist_match_info); tx.send(fetch_result).unwrap(); @@ -619,8 +619,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] @@ -677,7 +677,7 @@ mod tests { assert!(matches!(app, AppState::Fetch(_))); let artist = COLLECTION[3].meta.clone(); - let match_info = MatchStateInfo::artist_search::>(artist, vec![]); + let match_info = EntityMatches::artist_search::>(artist, vec![]); let fetch_result = Ok(match_info); tx.send(fetch_result).unwrap(); diff --git a/src/tui/app/machine/match_state.rs b/src/tui/app/machine/match_state.rs index 82f497e..859e946 100644 --- a/src/tui/app/machine/match_state.rs +++ b/src/tui/app/machine/match_state.rs @@ -8,8 +8,8 @@ use musichoard::collection::{ use crate::tui::app::{ machine::{fetch_state::FetchState, input::Input, App, AppInner, AppMachine}, - AlbumMatches, AppPublicState, AppState, ArtistMatches, Delta, IAppInteractMatch, MatchOption, - MatchStateInfo, MatchStatePublic, WidgetState, + AlbumMatches, AppPublicState, AppState, ArtistMatches, Delta, EntityMatches, IAppInteractMatch, + MatchOption, MatchStatePublic, WidgetState, }; trait GetInfoMeta { @@ -103,7 +103,7 @@ impl AlbumMatches { } } -impl MatchStateInfo { +impl EntityMatches { fn len(&self) -> usize { match self { Self::Artist(a) => a.len(), @@ -127,13 +127,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(); @@ -158,11 +158,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( @@ -191,7 +191,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, }) } @@ -225,14 +225,14 @@ impl IAppInteractMatch for AppMachine { let mh = &mut self.inner.music_hoard; let result = match self.state.current { - MatchStateInfo::Artist(ref mut matches) => { + EntityMatches::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(), } } - MatchStateInfo::Album(ref mut matches) => { + EntityMatches::Album(ref mut matches) => { let info: AlbumInfo = matches.matching.info.clone(); match matches.list.extract_info(index, info) { InfoOption::Info(info) => { @@ -298,7 +298,7 @@ mod tests { meta } - fn artist_match() -> MatchStateInfo { + fn artist_match() -> EntityMatches { let artist = artist_meta(); let artist_1 = artist.clone(); @@ -309,13 +309,13 @@ mod tests { 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 = Entity::new(artist.clone()); - MatchStateInfo::artist_lookup(artist, lookup) + EntityMatches::artist_lookup(artist, lookup) } fn album_meta() -> AlbumMeta { @@ -330,7 +330,7 @@ mod tests { ) } - fn album_match() -> MatchStateInfo { + fn album_match() -> EntityMatches { let artist_id = ArtistId::new("Artist"); let album = album_meta(); @@ -343,14 +343,14 @@ mod tests { 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, list) } - fn album_lookup() -> MatchStateInfo { + fn album_lookup() -> EntityMatches { let artist_id = ArtistId::new("Artist"); let album = album_meta(); let lookup = Entity::new(album.clone()); - MatchStateInfo::album_lookup(artist_id, album, lookup) + EntityMatches::album_lookup(artist_id, album, lookup) } fn fetch_state() -> FetchState { @@ -358,7 +358,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()) } @@ -379,11 +379,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)); @@ -391,7 +391,7 @@ 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; @@ -401,7 +401,7 @@ mod tests { .times(1) .return_once(|_, _, _| Ok(())); } - MatchStateInfo::Artist(_) => { + EntityMatches::Artist(_) => { let mut info = artist_meta().info; info.musicbrainz = MbRefOption::CannotHaveMbid; music_hoard @@ -485,8 +485,8 @@ 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() @@ -509,8 +509,8 @@ mod tests { let mut music_hoard = music_hoard(vec![]); match matches_info { - MatchStateInfo::Artist(_) => panic!(), - MatchStateInfo::Album(matches) => { + EntityMatches::Artist(_) => panic!(), + EntityMatches::Album(matches) => { let meta = album_meta(); music_hoard .expect_set_album_info() @@ -533,8 +533,8 @@ mod tests { let mut music_hoard = music_hoard(vec![]); match matches_info { - MatchStateInfo::Album(_) => panic!(), - MatchStateInfo::Artist(_) => { + EntityMatches::Album(_) => panic!(), + EntityMatches::Artist(_) => { music_hoard.expect_set_artist_info().return_once(|_, _| { Err(musichoard::Error::DatabaseError(String::from("error"))) }); @@ -598,7 +598,7 @@ mod tests { .return_once(|_, _| Ok(())); let matches_vec: Vec> = vec![]; - let artist_match = MatchStateInfo::artist_search(artist.clone(), matches_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), @@ -632,7 +632,7 @@ mod tests { 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.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 2f270f9..660f922 100644 --- a/src/tui/app/machine/mod.rs +++ b/src/tui/app/machine/mod.rs @@ -225,7 +225,7 @@ mod tests { }; use crate::tui::{ - app::{AppState, IApp, IAppInput, IAppInteractBrowse, InputEvent, MatchStateInfo}, + app::{AppState, EntityMatches, IApp, IAppInput, IAppInteractBrowse, InputEvent}, lib::{ 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(), Entity::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 10e3b22..6c9ae22 100644 --- a/src/tui/app/mod.rs +++ b/src/tui/app/mod.rs @@ -240,18 +240,18 @@ pub struct AlbumMatches { } #[derive(Clone, Debug, PartialEq, Eq)] -pub enum MatchStateInfo { +pub enum EntityMatches { Artist(ArtistMatches), Album(AlbumMatches), } -impl MatchStateInfo { +impl EntityMatches { pub fn artist_search>>( matching: ArtistMeta, list: Vec, ) -> Self { let list = list.into_iter().map(Into::into).collect(); - MatchStateInfo::Artist(ArtistMatches { matching, list }) + EntityMatches::Artist(ArtistMatches { matching, list }) } pub fn album_search>>( @@ -260,7 +260,7 @@ impl MatchStateInfo { list: Vec, ) -> Self { let list = list.into_iter().map(Into::into).collect(); - MatchStateInfo::Album(AlbumMatches { + EntityMatches::Album(AlbumMatches { artist, matching, list, @@ -269,7 +269,7 @@ impl MatchStateInfo { pub fn artist_lookup>>(matching: ArtistMeta, item: M) -> Self { let list = vec![item.into()]; - MatchStateInfo::Artist(ArtistMatches { matching, list }) + EntityMatches::Artist(ArtistMatches { matching, list }) } pub fn album_lookup>>( @@ -278,7 +278,7 @@ impl MatchStateInfo { item: M, ) -> Self { let list = vec![item.into()]; - MatchStateInfo::Album(AlbumMatches { + EntityMatches::Album(AlbumMatches { artist, matching, list, @@ -287,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/daemon/mod.rs b/src/tui/lib/external/musicbrainz/daemon/mod.rs index 2007f35..ecef237 100644 --- a/src/tui/lib/external/musicbrainz/daemon/mod.rs +++ b/src/tui/lib/external/musicbrainz/daemon/mod.rs @@ -1,7 +1,7 @@ use std::{collections::VecDeque, sync::mpsc, thread, time}; use crate::tui::{ - app::MatchStateInfo, + app::EntityMatches, event::IFetchCompleteEventSender, lib::interface::musicbrainz::{ api::{Error as ApiError, IMusicBrainz}, @@ -242,18 +242,18 @@ impl JobInstance { MbParams::Lookup(lookup) => match lookup { LookupParams::Artist(params) => musicbrainz .lookup_artist(¶ms.mbid) - .map(|rv| MatchStateInfo::artist_lookup(params.artist, rv)), + .map(|rv| EntityMatches::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)), + .map(|rv| EntityMatches::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)), + .map(|rv| EntityMatches::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)), + .map(|rv| EntityMatches::album_search(params.artist_id, params.album, rv)), }, }; self.return_result(event_sender, result) @@ -262,7 +262,7 @@ impl JobInstance { fn return_result( &mut self, event_sender: &mut dyn IFetchCompleteEventSender, - result: Result, + result: Result, ) -> Result<(), JobInstanceError> { self.result_sender .send(result) @@ -575,7 +575,7 @@ mod tests { assert_eq!(result, Ok(())); let result = result_receiver.try_recv().unwrap(); - assert_eq!(result, Ok(MatchStateInfo::artist_lookup(artist, lookup))); + assert_eq!(result, Ok(EntityMatches::artist_lookup(artist, lookup))); } fn lookup_release_group_expectation( @@ -620,7 +620,7 @@ mod tests { let artist_id = album_artist_id(); assert_eq!( result, - Ok(MatchStateInfo::album_lookup(artist_id, album, lookup)) + Ok(EntityMatches::album_lookup(artist_id, album, lookup)) ); } @@ -661,7 +661,7 @@ mod tests { assert_eq!(result, Ok(())); let result = result_receiver.try_recv().unwrap(); - assert_eq!(result, Ok(MatchStateInfo::artist_search(artist, matches))); + assert_eq!(result, Ok(EntityMatches::artist_search(artist, matches))); } fn search_release_group_expectation( @@ -716,7 +716,7 @@ mod tests { let result = result_receiver.try_recv().unwrap(); assert_eq!( result, - Ok(MatchStateInfo::album_search( + Ok(EntityMatches::album_search( artist_id.clone(), album_1, matches_1 @@ -726,7 +726,7 @@ mod tests { let result = result_receiver.try_recv().unwrap(); assert_eq!( result, - Ok(MatchStateInfo::album_search( + Ok(EntityMatches::album_search( artist_id.clone(), album_4, matches_4 diff --git a/src/tui/lib/interface/musicbrainz/daemon/mod.rs b/src/tui/lib/interface/musicbrainz/daemon/mod.rs index 0a2b468..c4a4166 100644 --- a/src/tui/lib/interface/musicbrainz/daemon/mod.rs +++ b/src/tui/lib/interface/musicbrainz/daemon/mod.rs @@ -6,7 +6,7 @@ use musichoard::collection::{ 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,7 +26,7 @@ impl fmt::Display for Error { } } -pub type MbApiResult = Result; +pub type MbApiResult = Result; pub type ResultSender = mpsc::Sender; #[cfg_attr(test, automock)] diff --git a/src/tui/ui/display.rs b/src/tui/ui/display.rs index 6684fff..4fb692b 100644 --- a/src/tui/ui/display.rs +++ b/src/tui/ui/display.rs @@ -5,7 +5,7 @@ use musichoard::collection::{ track::{TrackFormat, TrackQuality}, }; -use crate::tui::app::{MatchOption, MatchStateInfo}; +use crate::tui::app::{EntityMatches, MatchOption}; pub struct UiDisplay; @@ -119,10 +119,10 @@ impl UiDisplay { ) } - 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), } } diff --git a/src/tui/ui/match_state.rs b/src/tui/ui/match_state.rs index 59f9dae..59c11d5 100644 --- a/src/tui/ui/match_state.rs +++ b/src/tui/ui/match_state.rs @@ -2,7 +2,7 @@ use musichoard::collection::{album::AlbumMeta, artist::ArtistMeta}; use ratatui::widgets::{List, ListItem}; use crate::tui::{ - app::{MatchOption, MatchStateInfo, WidgetState}, + app::{EntityMatches, MatchOption, WidgetState}, ui::display::UiDisplay, }; @@ -13,10 +13,10 @@ 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), } } 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 5dc4c8e..b9a65f4 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), _ => {} @@ -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 = 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 = 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 @@ -366,24 +368,24 @@ mod tests { ) } - fn album_matches() -> MatchStateInfo { + fn album_matches() -> EntityMatches { let artist_id = album_artist_id(); let album = album_meta(); let album_match = Entity::with_score(album.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, 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 = Entity::new(album.clone()); - let mut info = MatchStateInfo::album_lookup(artist_id, album, album_lookup); + let mut info = EntityMatches::album_lookup(artist_id, album, album_lookup); info.push_cannot_have_mbid(); info.push_manual_input_mbid(); info @@ -403,13 +405,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, -- 2.45.2 From b51fa3493559e2237812cf22aeb34242201681aa Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sat, 5 Oct 2024 21:41:25 +0200 Subject: [PATCH 04/15] Add return type for browse --- src/tui/app/machine/fetch_state.rs | 23 ++++++++----- .../lib/external/musicbrainz/daemon/mod.rs | 34 +++++++++++++------ .../lib/interface/musicbrainz/daemon/mod.rs | 14 +++++++- 3 files changed, 52 insertions(+), 19 deletions(-) diff --git a/src/tui/app/machine/fetch_state.rs b/src/tui/app/machine/fetch_state.rs index 740a3da..0b1e3e2 100644 --- a/src/tui/app/machine/fetch_state.rs +++ b/src/tui/app/machine/fetch_state.rs @@ -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, @@ -311,13 +318,13 @@ mod tests { let artist = COLLECTION[3].meta.clone(); let matches: Vec> = vec![]; - let fetch_result = EntityMatches::artist_search(artist.clone(), matches); + 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 = Entity::new(artist.clone()); - let lookup_result = EntityMatches::artist_lookup(artist.clone(), lookup); + 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))); @@ -604,7 +611,7 @@ mod tests { let artist_match = Entity::with_score(COLLECTION[2].meta.clone(), 80); let artist_match_info = EntityMatches::artist_search(artist.clone(), vec![artist_match.clone()]); - let fetch_result = Ok(artist_match_info); + let fetch_result = Ok(MbReturn::Match(artist_match_info)); tx.send(fetch_result).unwrap(); let inner = inner(music_hoard(COLLECTION.clone())); @@ -678,7 +685,7 @@ mod tests { let artist = COLLECTION[3].meta.clone(); let match_info = EntityMatches::artist_search::>(artist, vec![]); - let fetch_result = Ok(match_info); + 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/lib/external/musicbrainz/daemon/mod.rs b/src/tui/lib/external/musicbrainz/daemon/mod.rs index ecef237..02a5440 100644 --- a/src/tui/lib/external/musicbrainz/daemon/mod.rs +++ b/src/tui/lib/external/musicbrainz/daemon/mod.rs @@ -5,7 +5,9 @@ use crate::tui::{ event::IFetchCompleteEventSender, lib::interface::musicbrainz::{ api::{Error as ApiError, IMusicBrainz}, - daemon::{Error, IMbJobSender, LookupParams, MbParams, ResultSender, SearchParams}, + daemon::{ + Error, IMbJobSender, LookupParams, MbParams, MbReturn, ResultSender, SearchParams, + }, }, }; @@ -256,13 +258,13 @@ impl JobInstance { .map(|rv| EntityMatches::album_search(params.artist_id, params.album, rv)), }, }; - self.return_result(event_sender, result) + self.return_result(event_sender, result.map(MbReturn::Match)) } fn return_result( &mut self, event_sender: &mut dyn IFetchCompleteEventSender, - result: Result, + result: Result, ) -> Result<(), JobInstanceError> { self.result_sender .send(result) @@ -575,7 +577,12 @@ mod tests { assert_eq!(result, Ok(())); let result = result_receiver.try_recv().unwrap(); - assert_eq!(result, Ok(EntityMatches::artist_lookup(artist, lookup))); + assert_eq!( + result, + Ok(MbReturn::Match(EntityMatches::artist_lookup( + artist, lookup + ))) + ); } fn lookup_release_group_expectation( @@ -620,7 +627,9 @@ mod tests { let artist_id = album_artist_id(); assert_eq!( result, - Ok(EntityMatches::album_lookup(artist_id, album, lookup)) + Ok(MbReturn::Match(EntityMatches::album_lookup( + artist_id, album, lookup + ))) ); } @@ -661,7 +670,12 @@ mod tests { assert_eq!(result, Ok(())); let result = result_receiver.try_recv().unwrap(); - assert_eq!(result, Ok(EntityMatches::artist_search(artist, matches))); + assert_eq!( + result, + Ok(MbReturn::Match(EntityMatches::artist_search( + artist, matches + ))) + ); } fn search_release_group_expectation( @@ -716,21 +730,21 @@ mod tests { let result = result_receiver.try_recv().unwrap(); assert_eq!( result, - Ok(EntityMatches::album_search( + Ok(MbReturn::Match(EntityMatches::album_search( artist_id.clone(), album_1, matches_1 - )) + ))) ); let result = result_receiver.try_recv().unwrap(); assert_eq!( result, - Ok(EntityMatches::album_search( + Ok(MbReturn::Match(EntityMatches::album_search( artist_id.clone(), album_4, matches_4 - )) + ))) ); } diff --git a/src/tui/lib/interface/musicbrainz/daemon/mod.rs b/src/tui/lib/interface/musicbrainz/daemon/mod.rs index c4a4166..bfb414f 100644 --- a/src/tui/lib/interface/musicbrainz/daemon/mod.rs +++ b/src/tui/lib/interface/musicbrainz/daemon/mod.rs @@ -26,9 +26,21 @@ 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 { + Artist(Vec), + Album(Vec), +} + #[cfg_attr(test, automock)] pub trait IMbJobSender { fn submit_foreground_job( -- 2.45.2 From b01af5ff18b7b077e30a1ac2f8276b1235147283 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sat, 5 Oct 2024 23:17:59 +0200 Subject: [PATCH 05/15] Implement browse in daemon --- examples/musicbrainz_api/browse.rs | 2 +- src/external/musicbrainz/api/mod.rs | 17 +++- .../lib/external/musicbrainz/daemon/mod.rs | 88 ++++++++++++++----- src/tui/lib/interface/musicbrainz/api/mod.rs | 24 +++++ .../lib/interface/musicbrainz/daemon/mod.rs | 12 ++- 5 files changed, 113 insertions(+), 30 deletions(-) diff --git a/examples/musicbrainz_api/browse.rs b/examples/musicbrainz_api/browse.rs index 20dcc5c..0bb288e 100644 --- a/examples/musicbrainz_api/browse.rs +++ b/examples/musicbrainz_api/browse.rs @@ -72,7 +72,7 @@ fn main() { println!("Release groups in this response: {count}"); match response.page.next_page_offset(count) { - NextPage::Offset(next_offset) => paging.with_offset(next_offset), + NextPage::Offset(next_offset) => paging.set_offset(next_offset), NextPage::Complete => break, } diff --git a/src/external/musicbrainz/api/mod.rs b/src/external/musicbrainz/api/mod.rs index 9f374da..daad181 100644 --- a/src/external/musicbrainz/api/mod.rs +++ b/src/external/musicbrainz/api/mod.rs @@ -61,7 +61,7 @@ impl MusicBrainzClient { } } -#[derive(Default)] +#[derive(Debug, Default)] pub struct PageSettings { limit: Option, offset: Option, @@ -79,7 +79,12 @@ 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.offset = Some(offset); + self + } + + pub fn set_offset(&mut self, offset: usize) { self.offset = Some(offset); } } @@ -91,6 +96,10 @@ pub enum NextPage { } impl NextPage { + pub fn new() -> NextPage { + NextPage::Offset(0) + } + pub fn next_page_offset(offset: usize, total_count: usize, page_count: usize) -> NextPage { let next_offset = offset + page_count; if next_offset < total_count { @@ -387,14 +396,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/tui/lib/external/musicbrainz/daemon/mod.rs b/src/tui/lib/external/musicbrainz/daemon/mod.rs index 02a5440..f168e54 100644 --- a/src/tui/lib/external/musicbrainz/daemon/mod.rs +++ b/src/tui/lib/external/musicbrainz/daemon/mod.rs @@ -1,12 +1,15 @@ use std::{collections::VecDeque, sync::mpsc, thread, time}; +use musichoard::external::musicbrainz::api::{NextPage, PageSettings}; + use crate::tui::{ app::EntityMatches, event::IFetchCompleteEventSender, lib::interface::musicbrainz::{ - api::{Error as ApiError, IMusicBrainz}, + api::{Error as ApiError, IMusicBrainz, Paged}, daemon::{ - Error, IMbJobSender, LookupParams, MbParams, MbReturn, ResultSender, SearchParams, + BrowseParams, EntityList, Error, IMbJobSender, LookupParams, MbParams, MbReturn, + ResultSender, SearchParams, }, }, }; @@ -45,6 +48,7 @@ enum JobPriority { struct JobInstance { result_sender: ResultSender, requests: VecDeque, + paging: Option, } #[derive(Debug, PartialEq, Eq)] @@ -64,6 +68,7 @@ impl JobInstance { JobInstance { result_sender, requests, + paging: None, } } } @@ -223,8 +228,17 @@ 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)? + self.paging = match self.requests.front() { + Some(params) => { + let result_sender = &mut self.result_sender; + let paging = self.paging.take(); + Self::execute(musicbrainz, result_sender, event_sender, ¶ms, paging)? + } + None => None, + }; + + if self.paging.is_none() { + self.requests.pop_front(); } if self.requests.is_empty() { @@ -235,38 +249,64 @@ impl JobInstance { } fn execute( - &mut self, musicbrainz: &mut dyn IMusicBrainz, + result_sender: &mut ResultSender, event_sender: &mut dyn IFetchCompleteEventSender, - api_params: MbParams, - ) -> Result<(), JobInstanceError> { + api_params: &MbParams, + paging: Option, + ) -> Result, JobInstanceError> { + let mut paging = match paging { + Some(paging) => paging, + None => PageSettings::with_max_limit(), + }; + let mut next_page = NextPage::Complete; + let result = match api_params { MbParams::Lookup(lookup) => match lookup { - LookupParams::Artist(params) => musicbrainz - .lookup_artist(¶ms.mbid) - .map(|rv| EntityMatches::artist_lookup(params.artist, rv)), - LookupParams::ReleaseGroup(params) => musicbrainz - .lookup_release_group(¶ms.mbid) - .map(|rv| EntityMatches::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.clone(), rv) + }) + } + } + .map(MbReturn::Match), MbParams::Search(search) => match search { - SearchParams::Artist(params) => musicbrainz - .search_artist(¶ms.artist) - .map(|rv| EntityMatches::artist_search(params.artist, rv)), - SearchParams::ReleaseGroup(params) => musicbrainz - .search_release_group(¶ms.artist_mbid, ¶ms.album) - .map(|rv| EntityMatches::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.clone(), rv) + }), + } + .map(MbReturn::Match), + MbParams::Browse(browse) => match browse { + BrowseParams::ReleaseGroup(params) => Paged::map_paged_result( + musicbrainz.browse_release_group(¶ms.artist, &mut paging), + |ents| EntityList::Album(ents.into_iter().map(|rg| rg.entity).collect()), + &mut next_page, + ), + } + .map(MbReturn::Fetch), }; - self.return_result(event_sender, result.map(MbReturn::Match)) + Self::return_result(result_sender, event_sender, result)?; + + Ok(match next_page { + NextPage::Offset(offset) => Some(paging.with_offset(offset)), + NextPage::Complete => None, + }) } fn return_result( - &mut self, + result_sender: &mut ResultSender, event_sender: &mut dyn IFetchCompleteEventSender, result: Result, ) -> Result<(), JobInstanceError> { - self.result_sender + result_sender .send(result) .map_err(|_| JobInstanceError::ReturnChannelDisconnected)?; diff --git a/src/tui/lib/interface/musicbrainz/api/mod.rs b/src/tui/lib/interface/musicbrainz/api/mod.rs index cf70913..8b7c431 100644 --- a/src/tui/lib/interface/musicbrainz/api/mod.rs +++ b/src/tui/lib/interface/musicbrainz/api/mod.rs @@ -1,5 +1,7 @@ //! Module for accessing MusicBrainz metadata. +use std::mem; + #[cfg(test)] use mockall::automock; @@ -47,4 +49,26 @@ pub struct Paged { pub next: NextPage, } +// pub fn map U>(self, op: F) -> Result { +// match self { +// Ok(t) => Ok(op(t)), +// Err(e) => Err(e), +// } +// } + +impl Paged { + pub fn map_paged_result U>( + result: Result, E>, + op: F, + next: &mut NextPage, + ) -> Result { + match result { + Ok(paged) => { + _ = mem::replace(next, paged.next); + Ok(op(paged.item)) + } + Err(err) => Err(err), + } + } +} 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 bfb414f..63dd915 100644 --- a/src/tui/lib/interface/musicbrainz/daemon/mod.rs +++ b/src/tui/lib/interface/musicbrainz/daemon/mod.rs @@ -37,7 +37,6 @@ pub enum MbReturn { #[derive(Clone, Debug, PartialEq, Eq)] pub enum EntityList { - Artist(Vec), Album(Vec), } @@ -60,6 +59,7 @@ pub trait IMbJobSender { pub enum MbParams { Lookup(LookupParams), Search(SearchParams), + Browse(BrowseParams), } #[derive(Clone, Debug, PartialEq, Eq)] @@ -99,6 +99,16 @@ pub struct SearchReleaseGroupParams { pub album: AlbumMeta, } +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum BrowseParams { + 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 })) -- 2.45.2 From 029af582d6c364011ff1b6103ecc45ee9b2d8033 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 6 Oct 2024 10:05:23 +0200 Subject: [PATCH 06/15] fix --- src/tui/lib/interface/musicbrainz/api/mod.rs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/tui/lib/interface/musicbrainz/api/mod.rs b/src/tui/lib/interface/musicbrainz/api/mod.rs index 8b7c431..f211cc9 100644 --- a/src/tui/lib/interface/musicbrainz/api/mod.rs +++ b/src/tui/lib/interface/musicbrainz/api/mod.rs @@ -49,13 +49,6 @@ pub struct Paged { pub next: NextPage, } -// pub fn map U>(self, op: F) -> Result { -// match self { -// Ok(t) => Ok(op(t)), -// Err(e) => Err(e), -// } -// } - impl Paged { pub fn map_paged_result U>( result: Result, E>, -- 2.45.2 From f61b357845dad9eb8ea2414eee8a11e2a592fad5 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 6 Oct 2024 10:41:06 +0200 Subject: [PATCH 07/15] Minor improvements --- src/tui/lib/external/musicbrainz/api/mod.rs | 6 +++--- src/tui/lib/external/musicbrainz/daemon/mod.rs | 16 +++++++++------- src/tui/lib/interface/musicbrainz/api/mod.rs | 11 ++++++----- 3 files changed, 18 insertions(+), 15 deletions(-) diff --git a/src/tui/lib/external/musicbrainz/api/mod.rs b/src/tui/lib/external/musicbrainz/api/mod.rs index 115e0be..d2e0ccc 100644 --- a/src/tui/lib/external/musicbrainz/api/mod.rs +++ b/src/tui/lib/external/musicbrainz/api/mod.rs @@ -98,16 +98,16 @@ impl IMusicBrainz for MusicBrainz { &mut self, artist: &Mbid, paging: &mut PageSettings, - ) -> Result>>, Error> { + ) -> Result>, Error> { let request = BrowseReleaseGroupRequest::artist(artist); let mb_response = self.client.browse_release_group(&request, paging)?; let page_count = mb_response.release_groups.len(); let next = mb_response.page.next_page_offset(page_count); - let item = from_browse_release_group_response(mb_response); + let items = from_browse_release_group_response(mb_response); - Ok(Paged { item, next }) + Ok(Paged { items, next }) } } diff --git a/src/tui/lib/external/musicbrainz/daemon/mod.rs b/src/tui/lib/external/musicbrainz/daemon/mod.rs index f168e54..e58acca 100644 --- a/src/tui/lib/external/musicbrainz/daemon/mod.rs +++ b/src/tui/lib/external/musicbrainz/daemon/mod.rs @@ -285,20 +285,22 @@ impl JobInstance { } .map(MbReturn::Match), MbParams::Browse(browse) => match browse { - BrowseParams::ReleaseGroup(params) => Paged::map_paged_result( - musicbrainz.browse_release_group(¶ms.artist, &mut paging), - |ents| EntityList::Album(ents.into_iter().map(|rg| rg.entity).collect()), - &mut next_page, - ), + BrowseParams::ReleaseGroup(params) => { + let paged = musicbrainz.browse_release_group(¶ms.artist, &mut paging); + let result = Paged::map_paged_result(paged, |rg| rg.entity, &mut next_page); + result.map(EntityList::Album) + } } .map(MbReturn::Fetch), }; Self::return_result(result_sender, event_sender, result)?; - Ok(match next_page { + let next_page_settings = match next_page { NextPage::Offset(offset) => Some(paging.with_offset(offset)), NextPage::Complete => None, - }) + }; + + Ok(next_page_settings) } fn return_result( diff --git a/src/tui/lib/interface/musicbrainz/api/mod.rs b/src/tui/lib/interface/musicbrainz/api/mod.rs index f211cc9..440d43c 100644 --- a/src/tui/lib/interface/musicbrainz/api/mod.rs +++ b/src/tui/lib/interface/musicbrainz/api/mod.rs @@ -14,6 +14,7 @@ use musichoard::{ pub trait IMusicBrainz { fn lookup_artist(&mut self, mbid: &Mbid) -> Result, Error>; fn lookup_release_group(&mut self, mbid: &Mbid) -> Result, Error>; + // TODO: Also make it out Paged fn search_artist(&mut self, artist: &ArtistMeta) -> Result>, Error>; fn search_release_group( &mut self, @@ -24,7 +25,7 @@ pub trait IMusicBrainz { &mut self, artist: &Mbid, paging: &mut PageSettings, - ) -> Result>>, Error>; + ) -> Result>, Error>; } #[derive(Clone, Debug, PartialEq, Eq)] @@ -45,20 +46,20 @@ impl Entity { } pub struct Paged { - pub item: T, + pub items: Vec, pub next: NextPage, } impl Paged { - pub fn map_paged_result U>( + pub fn map_paged_result U>( result: Result, E>, op: F, next: &mut NextPage, - ) -> Result { + ) -> Result, E> { match result { Ok(paged) => { _ = mem::replace(next, paged.next); - Ok(op(paged.item)) + Ok(paged.items.into_iter().map(op).collect()) } Err(err) => Err(err), } -- 2.45.2 From f7e215eedfe0de641dc874390d3706046b221e0b Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 6 Oct 2024 11:14:33 +0200 Subject: [PATCH 08/15] Better deamon --- examples/musicbrainz_api/browse.rs | 10 ++--- src/external/musicbrainz/api/browse.rs | 8 ++-- src/external/musicbrainz/api/mod.rs | 28 ++++-------- src/external/musicbrainz/api/search/mod.rs | 8 ++-- src/tui/lib/external/musicbrainz/api/mod.rs | 14 +++--- .../lib/external/musicbrainz/daemon/mod.rs | 44 +++++++------------ src/tui/lib/interface/musicbrainz/api/mod.rs | 29 ++---------- 7 files changed, 45 insertions(+), 96 deletions(-) diff --git a/examples/musicbrainz_api/browse.rs b/examples/musicbrainz_api/browse.rs index 0bb288e..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, }, }; @@ -71,10 +71,10 @@ fn main() { println!("Release groups in this response: {count}"); - match response.page.next_page_offset(count) { - NextPage::Offset(next_offset) => paging.set_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/external/musicbrainz/api/browse.rs b/src/external/musicbrainz/api/browse.rs index dc63ed8..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, @@ -21,8 +21,8 @@ pub struct BrowseReleaseGroupPage { } 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 daad181..a16da76 100644 --- a/src/external/musicbrainz/api/mod.rs +++ b/src/external/musicbrainz/api/mod.rs @@ -87,25 +87,13 @@ impl PageSettings { 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 new() -> NextPage { - NextPage::Offset(0) - } - - 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 } } } @@ -370,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] diff --git a/src/external/musicbrainz/api/search/mod.rs b/src/external/musicbrainz/api/search/mod.rs index f76c010..a0f78e7 100644 --- a/src/external/musicbrainz/api/search/mod.rs +++ b/src/external/musicbrainz/api/search/mod.rs @@ -23,8 +23,6 @@ use crate::external::musicbrainz::{ IMusicBrainzHttp, }; -use super::NextPage; - #[derive(Clone, Copy, Debug, Deserialize, PartialEq, Eq)] #[serde(rename_all(deserialize = "kebab-case"))] pub struct SearchPage { @@ -33,8 +31,8 @@ pub struct SearchPage { } 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/lib/external/musicbrainz/api/mod.rs b/src/tui/lib/external/musicbrainz/api/mod.rs index d2e0ccc..6c2c7c1 100644 --- a/src/tui/lib/external/musicbrainz/api/mod.rs +++ b/src/tui/lib/external/musicbrainz/api/mod.rs @@ -25,7 +25,7 @@ use musichoard::{ }, }; -use crate::tui::lib::interface::musicbrainz::api::{Entity, Error, IMusicBrainz, Paged}; +use crate::tui::lib::interface::musicbrainz::api::{Entity, Error, IMusicBrainz}; // GRCOV_EXCL_START pub struct MusicBrainz { @@ -97,17 +97,17 @@ impl IMusicBrainz for MusicBrainz { fn browse_release_group( &mut self, artist: &Mbid, - paging: &mut PageSettings, - ) -> Result>, Error> { + paging: &mut Option, + ) -> Result>, Error> { let request = BrowseReleaseGroupRequest::artist(artist); - let mb_response = self.client.browse_release_group(&request, paging)?; + 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(); - let next = mb_response.page.next_page_offset(page_count); - let items = from_browse_release_group_response(mb_response); + *paging = mb_response.page.next_page(page, page_count); - Ok(Paged { items, next }) + Ok(from_browse_release_group_response(mb_response)) } } diff --git a/src/tui/lib/external/musicbrainz/daemon/mod.rs b/src/tui/lib/external/musicbrainz/daemon/mod.rs index e58acca..a32df55 100644 --- a/src/tui/lib/external/musicbrainz/daemon/mod.rs +++ b/src/tui/lib/external/musicbrainz/daemon/mod.rs @@ -1,12 +1,12 @@ use std::{collections::VecDeque, sync::mpsc, thread, time}; -use musichoard::external::musicbrainz::api::{NextPage, PageSettings}; +use musichoard::external::musicbrainz::api::PageSettings; use crate::tui::{ app::EntityMatches, event::IFetchCompleteEventSender, lib::interface::musicbrainz::{ - api::{Error as ApiError, IMusicBrainz, Paged}, + api::{Error as ApiError, IMusicBrainz}, daemon::{ BrowseParams, EntityList, Error, IMbJobSender, LookupParams, MbParams, MbReturn, ResultSender, SearchParams, @@ -228,13 +228,10 @@ impl JobInstance { event_sender: &mut dyn IFetchCompleteEventSender, ) -> Result { // self.requests can be empty if the caller submits an empty job. - self.paging = match self.requests.front() { - Some(params) => { - let result_sender = &mut self.result_sender; - let paging = self.paging.take(); - Self::execute(musicbrainz, result_sender, event_sender, ¶ms, paging)? - } - None => None, + 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, ¶ms, paging)?; }; if self.paging.is_none() { @@ -253,13 +250,11 @@ impl JobInstance { result_sender: &mut ResultSender, event_sender: &mut dyn IFetchCompleteEventSender, api_params: &MbParams, - paging: Option, - ) -> Result, JobInstanceError> { - let mut paging = match paging { - Some(paging) => paging, - None => PageSettings::with_max_limit(), - }; - let mut next_page = NextPage::Complete; + paging: &mut Option, + ) -> Result<(), JobInstanceError> { + if paging.is_none() { + *paging = Some(PageSettings::with_max_limit()); + } let result = match api_params { MbParams::Lookup(lookup) => match lookup { @@ -285,22 +280,13 @@ impl JobInstance { } .map(MbReturn::Match), MbParams::Browse(browse) => match browse { - BrowseParams::ReleaseGroup(params) => { - let paged = musicbrainz.browse_release_group(¶ms.artist, &mut paging); - let result = Paged::map_paged_result(paged, |rg| rg.entity, &mut next_page); - result.map(EntityList::Album) - } + BrowseParams::ReleaseGroup(params) => 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(result_sender, event_sender, result)?; - - let next_page_settings = match next_page { - NextPage::Offset(offset) => Some(paging.with_offset(offset)), - NextPage::Complete => None, - }; - - Ok(next_page_settings) + Self::return_result(result_sender, event_sender, result) } fn return_result( diff --git a/src/tui/lib/interface/musicbrainz/api/mod.rs b/src/tui/lib/interface/musicbrainz/api/mod.rs index 440d43c..11889fc 100644 --- a/src/tui/lib/interface/musicbrainz/api/mod.rs +++ b/src/tui/lib/interface/musicbrainz/api/mod.rs @@ -1,20 +1,17 @@ //! Module for accessing MusicBrainz metadata. -use std::mem; - #[cfg(test)] use mockall::automock; use musichoard::{ collection::{album::AlbumMeta, artist::ArtistMeta, musicbrainz::Mbid}, - external::musicbrainz::api::{NextPage, PageSettings}, + external::musicbrainz::api::PageSettings, }; #[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>; - // TODO: Also make it out Paged fn search_artist(&mut self, artist: &ArtistMeta) -> Result>, Error>; fn search_release_group( &mut self, @@ -24,8 +21,8 @@ pub trait IMusicBrainz { fn browse_release_group( &mut self, artist: &Mbid, - paging: &mut PageSettings, - ) -> Result>, Error>; + paging: &mut Option, + ) -> Result>, Error>; } #[derive(Clone, Debug, PartialEq, Eq)] @@ -45,24 +42,4 @@ impl Entity { } } -pub struct Paged { - pub items: Vec, - pub next: NextPage, -} - -impl Paged { - pub fn map_paged_result U>( - result: Result, E>, - op: F, - next: &mut NextPage, - ) -> Result, E> { - match result { - Ok(paged) => { - _ = mem::replace(next, paged.next); - Ok(paged.items.into_iter().map(op).collect()) - } - Err(err) => Err(err), - } - } -} pub type Error = musichoard::external::musicbrainz::api::Error; -- 2.45.2 From 246f35e7156780b2af98e625c5207c991a8698e3 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 6 Oct 2024 12:47:14 +0200 Subject: [PATCH 09/15] Clippy lints --- src/tui/app/machine/match_state.rs | 8 ++++---- src/tui/lib/external/musicbrainz/daemon/mod.rs | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/tui/app/machine/match_state.rs b/src/tui/app/machine/match_state.rs index 859e946..5866b21 100644 --- a/src/tui/app/machine/match_state.rs +++ b/src/tui/app/machine/match_state.rs @@ -24,7 +24,7 @@ impl GetInfoMeta for AlbumMeta { trait GetInfo { type InfoType; - fn into_info(&self, info: Self::InfoType) -> InfoOption; + fn get_info(&self, info: Self::InfoType) -> InfoOption; } enum InfoOption { @@ -35,7 +35,7 @@ enum InfoOption { impl GetInfo for MatchOption { type InfoType = ArtistInfo; - fn into_info(&self, mut info: Self::InfoType) -> InfoOption { + fn get_info(&self, mut info: Self::InfoType) -> InfoOption { match self { MatchOption::Some(option) => info.musicbrainz = option.entity.info.musicbrainz.clone(), MatchOption::CannotHaveMbid => info.musicbrainz = MbRefOption::CannotHaveMbid, @@ -48,7 +48,7 @@ impl GetInfo for MatchOption { impl GetInfo for MatchOption { type InfoType = AlbumInfo; - fn into_info(&self, mut info: Self::InfoType) -> InfoOption { + fn get_info(&self, mut info: Self::InfoType) -> InfoOption { match self { MatchOption::Some(option) => info = option.entity.info.clone(), MatchOption::CannotHaveMbid => info.musicbrainz = MbRefOption::CannotHaveMbid, @@ -71,7 +71,7 @@ where type InfoType = T::InfoType; fn extract_info(&self, index: usize, info: Self::InfoType) -> InfoOption { - self.get(index).unwrap().into_info(info) + self.get(index).unwrap().get_info(info) } } diff --git a/src/tui/lib/external/musicbrainz/daemon/mod.rs b/src/tui/lib/external/musicbrainz/daemon/mod.rs index a32df55..9e7f5cc 100644 --- a/src/tui/lib/external/musicbrainz/daemon/mod.rs +++ b/src/tui/lib/external/musicbrainz/daemon/mod.rs @@ -231,7 +231,7 @@ impl JobInstance { 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, ¶ms, paging)?; + Self::execute(musicbrainz, result_sender, event_sender, params, paging)?; }; if self.paging.is_none() { -- 2.45.2 From 5abac79414cd62e1a899cfa3dd8e1c4d3c35a6d0 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 6 Oct 2024 13:31:15 +0200 Subject: [PATCH 10/15] Complete lints --- src/core/collection/mod.rs | 4 +- src/core/musichoard/base.rs | 3 +- src/core/musichoard/database.rs | 34 ++++---- src/tui/app/machine/fetch_state.rs | 20 ++--- src/tui/app/machine/match_state.rs | 80 ++++++++++--------- src/tui/app/mod.rs | 8 +- .../lib/external/musicbrainz/daemon/mod.rs | 19 ++--- .../lib/interface/musicbrainz/daemon/mod.rs | 10 ++- src/tui/lib/mod.rs | 17 ++-- src/tui/ui/display.rs | 12 ++- src/tui/ui/match_state.rs | 7 +- src/tui/ui/mod.rs | 22 +++-- 12 files changed, 128 insertions(+), 108 deletions(-) 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/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/tui/app/machine/fetch_state.rs b/src/tui/app/machine/fetch_state.rs index 0b1e3e2..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}, }; @@ -151,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( @@ -243,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) @@ -468,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 @@ -487,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); @@ -496,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) { diff --git a/src/tui/app/machine/match_state.rs b/src/tui/app/machine/match_state.rs index 5866b21..e29c5b8 100644 --- a/src/tui/app/machine/match_state.rs +++ b/src/tui/app/machine/match_state.rs @@ -24,7 +24,7 @@ impl GetInfoMeta for AlbumMeta { trait GetInfo { type InfoType; - fn get_info(&self, info: Self::InfoType) -> InfoOption; + fn get_info(&self) -> InfoOption; } enum InfoOption { @@ -35,7 +35,8 @@ enum InfoOption { impl GetInfo for MatchOption { type InfoType = ArtistInfo; - fn get_info(&self, mut info: Self::InfoType) -> InfoOption { + 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, @@ -48,7 +49,8 @@ impl GetInfo for MatchOption { impl GetInfo for MatchOption { type InfoType = AlbumInfo; - fn get_info(&self, mut info: Self::InfoType) -> InfoOption { + fn get_info(&self) -> InfoOption { + let mut info = AlbumInfo::default(); match self { MatchOption::Some(option) => info = option.entity.info.clone(), MatchOption::CannotHaveMbid => info.musicbrainz = MbRefOption::CannotHaveMbid, @@ -60,7 +62,7 @@ impl GetInfo for MatchOption { trait ExtractInfo { type InfoType; - fn extract_info(&self, index: usize, info: Self::InfoType) -> InfoOption; + fn extract_info(&self, index: usize) -> InfoOption; } impl ExtractInfo for Vec> @@ -70,8 +72,8 @@ where { type InfoType = T::InfoType; - fn extract_info(&self, index: usize, info: Self::InfoType) -> InfoOption { - self.get(index).unwrap().get_info(info) + fn extract_info(&self, index: usize) -> InfoOption { + self.get(index).unwrap().get_info() } } @@ -225,22 +227,16 @@ impl IAppInteractMatch for AppMachine { let mh = &mut self.inner.music_hoard; let result = match self.state.current { - EntityMatches::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) } - } - EntityMatches::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 { @@ -318,9 +314,13 @@ mod tests { 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()), @@ -332,25 +332,27 @@ mod tests { 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_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 = Entity::with_score(album_2, 100); let list = vec![album_match_1.clone(), album_match_2.clone()]; - EntityMatches::album_search(artist_id, album, list) + EntityMatches::album_search(artist_id, album_id, list) } fn album_lookup() -> EntityMatches { let artist_id = ArtistId::new("Artist"); - let album = album_meta(); - let lookup = Entity::new(album.clone()); - EntityMatches::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 { @@ -393,10 +395,10 @@ mod tests { match matches_info { EntityMatches::Album(_) => { let album_id = AlbumId::new("Album"); - let mut info = album_meta().info; + let mut info = album_meta(album_id.clone()).info; info.musicbrainz = MbRefOption::CannotHaveMbid; 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(())); @@ -405,7 +407,7 @@ mod tests { let mut info = artist_meta().info; info.musicbrainz = MbRefOption::CannotHaveMbid; music_hoard - .expect_set_artist_info() + .expect_merge_artist_info() .with(eq(artist_id.clone()), eq(info)) .times(1) .return_once(|_, _| Ok(())); @@ -489,7 +491,7 @@ mod tests { 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(())); @@ -511,9 +513,9 @@ mod tests { match matches_info { EntityMatches::Artist(_) => panic!(), EntityMatches::Album(matches) => { - let meta = album_meta(); + 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(())); @@ -535,7 +537,7 @@ mod tests { match matches_info { EntityMatches::Album(_) => panic!(), EntityMatches::Artist(_) => { - music_hoard.expect_set_artist_info().return_once(|_, _| { + music_hoard.expect_merge_artist_info().return_once(|_, _| { Err(musichoard::Error::DatabaseError(String::from("error"))) }); } @@ -622,7 +624,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 @@ -632,7 +634,7 @@ mod tests { let matches_vec: Vec> = vec![]; let album_match = - EntityMatches::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/mod.rs b/src/tui/app/mod.rs index 6c9ae22..bce93ce 100644 --- a/src/tui/app/mod.rs +++ b/src/tui/app/mod.rs @@ -6,7 +6,7 @@ use ratatui::widgets::ListState; pub use selection::{Category, Selection}; use musichoard::collection::{ - album::AlbumMeta, + album::{AlbumId, AlbumMeta}, artist::{ArtistId, ArtistMeta}, Collection, }; @@ -235,7 +235,7 @@ pub struct ArtistMatches { #[derive(Clone, Debug, PartialEq, Eq)] pub struct AlbumMatches { pub artist: ArtistId, - pub matching: AlbumMeta, + pub matching: AlbumId, pub list: Vec>, } @@ -256,7 +256,7 @@ impl EntityMatches { pub fn album_search>>( artist: ArtistId, - matching: AlbumMeta, + matching: AlbumId, list: Vec, ) -> Self { let list = list.into_iter().map(Into::into).collect(); @@ -274,7 +274,7 @@ impl EntityMatches { pub fn album_lookup>>( artist: ArtistId, - matching: AlbumMeta, + matching: AlbumId, item: M, ) -> Self { let list = vec![item.into()]; diff --git a/src/tui/lib/external/musicbrainz/daemon/mod.rs b/src/tui/lib/external/musicbrainz/daemon/mod.rs index 9e7f5cc..0b5b80c 100644 --- a/src/tui/lib/external/musicbrainz/daemon/mod.rs +++ b/src/tui/lib/external/musicbrainz/daemon/mod.rs @@ -263,7 +263,7 @@ impl JobInstance { .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.clone(), rv) + EntityMatches::album_lookup(p.artist_id.clone(), p.album_id.clone(), rv) }) } } @@ -275,7 +275,7 @@ impl JobInstance { SearchParams::ReleaseGroup(p) => musicbrainz .search_release_group(&p.artist_mbid, &p.album) .map(|rv| { - EntityMatches::album_search(p.artist_id.clone(), p.album.clone(), rv) + EntityMatches::album_search(p.artist_id.clone(), p.album.id.clone(), rv) }), } .map(MbReturn::Match), @@ -428,9 +428,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 { @@ -630,8 +630,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 = Entity::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(); @@ -656,7 +657,7 @@ mod tests { assert_eq!( result, Ok(MbReturn::Match(EntityMatches::album_lookup( - artist_id, album, lookup + artist_id, album_id, lookup ))) ); } @@ -760,7 +761,7 @@ mod tests { result, Ok(MbReturn::Match(EntityMatches::album_search( artist_id.clone(), - album_1, + album_1.id, matches_1 ))) ); @@ -770,7 +771,7 @@ mod tests { result, Ok(MbReturn::Match(EntityMatches::album_search( artist_id.clone(), - album_4, + album_4.id, matches_4 ))) ); diff --git a/src/tui/lib/interface/musicbrainz/daemon/mod.rs b/src/tui/lib/interface/musicbrainz/daemon/mod.rs index 63dd915..24d0ff6 100644 --- a/src/tui/lib/interface/musicbrainz/daemon/mod.rs +++ b/src/tui/lib/interface/musicbrainz/daemon/mod.rs @@ -1,7 +1,7 @@ use std::{collections::VecDeque, fmt, sync::mpsc}; use musichoard::collection::{ - album::AlbumMeta, + album::{AlbumId, AlbumMeta}, artist::{ArtistId, ArtistMeta}, musicbrainz::Mbid, }; @@ -59,6 +59,7 @@ pub trait IMbJobSender { pub enum MbParams { Lookup(LookupParams), Search(SearchParams), + #[allow(dead_code)] // TODO: remove with completion of #160 Browse(BrowseParams), } @@ -77,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, } @@ -101,6 +102,7 @@ pub struct SearchReleaseGroupParams { #[derive(Clone, Debug, PartialEq, Eq)] pub enum BrowseParams { + #[allow(dead_code)] // TODO: remove with completion of #160 ReleaseGroup(BrowseReleaseGroupParams), } @@ -114,10 +116,10 @@ impl MbParams { 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, })) } 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 4fb692b..0210688 100644 --- a/src/tui/ui/display.rs +++ b/src/tui/ui/display.rs @@ -1,5 +1,7 @@ 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}, @@ -111,12 +113,8 @@ 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: &EntityMatches) -> String { diff --git a/src/tui/ui/match_state.rs b/src/tui/ui/match_state.rs index 59c11d5..99725b1 100644 --- a/src/tui/ui/match_state.rs +++ b/src/tui/ui/match_state.rs @@ -1,4 +1,7 @@ -use musichoard::collection::{album::AlbumMeta, artist::ArtistMeta}; +use musichoard::collection::{ + album::{AlbumId, AlbumMeta}, + artist::ArtistMeta, +}; use ratatui::widgets::{List, ListItem}; use crate::tui::{ @@ -37,7 +40,7 @@ impl<'a, 'b> MatchOverlay<'a, 'b> { } fn albums( - matching: &AlbumMeta, + matching: &AlbumId, matches: &'a [MatchOption], state: &'b mut WidgetState, ) -> Self { diff --git a/src/tui/ui/mod.rs b/src/tui/ui/mod.rs index b9a65f4..a633a0f 100644 --- a/src/tui/ui/mod.rs +++ b/src/tui/ui/mod.rs @@ -356,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, @@ -370,11 +374,12 @@ mod tests { fn album_matches() -> EntityMatches { let artist_id = album_artist_id(); - let album = album_meta(); - let album_match = Entity::with_score(album.clone(), 80); + 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 = EntityMatches::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 @@ -382,10 +387,11 @@ mod tests { fn album_lookup() -> EntityMatches { let artist_id = album_artist_id(); - let album = album_meta(); - let album_lookup = Entity::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 = EntityMatches::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 -- 2.45.2 From dd9141352c9df26cadd863bdbeea37c57323712f Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 6 Oct 2024 13:38:07 +0200 Subject: [PATCH 11/15] Fix lib unit tests --- src/core/collection/musicbrainz.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/core/collection/musicbrainz.rs b/src/core/collection/musicbrainz.rs index 19597b6..d89ee94 100644 --- a/src/core/collection/musicbrainz.rs +++ b/src/core/collection/musicbrainz.rs @@ -48,8 +48,15 @@ pub enum MbRefOption { impl MbRefOption { pub fn or(self, optb: MbRefOption) -> MbRefOption { match self { - x @ MbRefOption::Some(_) => x, - MbRefOption::CannotHaveMbid | MbRefOption::None => optb, + opta @ MbRefOption::Some(_) => opta, + opta @ MbRefOption::CannotHaveMbid => { + if matches!(optb, MbRefOption::Some(_)) { + optb + } else { + opta + } + } + MbRefOption::None => optb, } } -- 2.45.2 From 17d94e59f18295b60c74e9828412dcf044eb4f4e Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 6 Oct 2024 14:31:14 +0200 Subject: [PATCH 12/15] Fix all unit tests --- src/core/collection/musicbrainz.rs | 11 ++---- src/tui/app/machine/match_state.rs | 12 ++++-- .../lib/external/musicbrainz/daemon/mod.rs | 39 ++++++++----------- 3 files changed, 28 insertions(+), 34 deletions(-) diff --git a/src/core/collection/musicbrainz.rs b/src/core/collection/musicbrainz.rs index d89ee94..d6c4d02 100644 --- a/src/core/collection/musicbrainz.rs +++ b/src/core/collection/musicbrainz.rs @@ -49,13 +49,10 @@ impl MbRefOption { pub fn or(self, optb: MbRefOption) -> MbRefOption { match self { opta @ MbRefOption::Some(_) => opta, - opta @ MbRefOption::CannotHaveMbid => { - if matches!(optb, MbRefOption::Some(_)) { - optb - } else { - opta - } - } + opta @ MbRefOption::CannotHaveMbid => match optb { + MbRefOption::Some(_) => optb, + MbRefOption::CannotHaveMbid | MbRefOption::None => opta, + }, MbRefOption::None => optb, } } diff --git a/src/tui/app/machine/match_state.rs b/src/tui/app/machine/match_state.rs index e29c5b8..5b453a6 100644 --- a/src/tui/app/machine/match_state.rs +++ b/src/tui/app/machine/match_state.rs @@ -395,8 +395,10 @@ mod tests { match matches_info { EntityMatches::Album(_) => { let album_id = AlbumId::new("Album"); - let mut info = album_meta(album_id.clone()).info; - info.musicbrainz = MbRefOption::CannotHaveMbid; + let info = AlbumInfo { + musicbrainz: MbRefOption::CannotHaveMbid, + ..Default::default() + }; music_hoard .expect_merge_album_info() .with(eq(artist_id.clone()), eq(album_id.clone()), eq(info)) @@ -404,8 +406,10 @@ mod tests { .return_once(|_, _, _| Ok(())); } EntityMatches::Artist(_) => { - let mut info = artist_meta().info; - info.musicbrainz = MbRefOption::CannotHaveMbid; + let info = ArtistInfo { + musicbrainz: MbRefOption::CannotHaveMbid, + ..Default::default() + }; music_hoard .expect_merge_artist_info() .with(eq(artist_id.clone()), eq(info)) diff --git a/src/tui/lib/external/musicbrainz/daemon/mod.rs b/src/tui/lib/external/musicbrainz/daemon/mod.rs index 0b5b80c..fa29297 100644 --- a/src/tui/lib/external/musicbrainz/daemon/mod.rs +++ b/src/tui/lib/external/musicbrainz/daemon/mod.rs @@ -252,10 +252,6 @@ impl JobInstance { api_params: &MbParams, paging: &mut Option, ) -> Result<(), JobInstanceError> { - if paging.is_none() { - *paging = Some(PageSettings::with_max_limit()); - } - let result = match api_params { MbParams::Lookup(lookup) => match lookup { LookupParams::Artist(p) => musicbrainz @@ -280,15 +276,24 @@ impl JobInstance { } .map(MbReturn::Match), MbParams::Browse(browse) => match browse { - BrowseParams::ReleaseGroup(params) => musicbrainz - .browse_release_group(¶ms.artist, paging) - .map(|rv| EntityList::Album(rv.into_iter().map(|rg| rg.entity).collect())), + 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(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( result_sender: &mut ResultSender, event_sender: &mut dyn IFetchCompleteEventSender, @@ -757,24 +762,12 @@ mod tests { let artist_id = album_artist_id(); let result = result_receiver.try_recv().unwrap(); - assert_eq!( - result, - Ok(MbReturn::Match(EntityMatches::album_search( - artist_id.clone(), - album_1.id, - 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(MbReturn::Match(EntityMatches::album_search( - artist_id.clone(), - album_4.id, - matches_4 - ))) - ); + let matches = EntityMatches::album_search(artist_id.clone(), album_4.id, matches_4); + assert_eq!(result, Ok(MbReturn::Match(matches))); } #[test] -- 2.45.2 From 9c2bbd2397bc0bed2b38bbfa911af8f162fbcc0f Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 6 Oct 2024 15:10:50 +0200 Subject: [PATCH 13/15] Complete daemon unit tests --- README.md | 2 +- src/external/musicbrainz/api/mod.rs | 4 +- .../lib/external/musicbrainz/daemon/mod.rs | 87 +++++++++++++++++++ .../lib/interface/musicbrainz/daemon/mod.rs | 7 ++ 4 files changed, 97 insertions(+), 3 deletions(-) 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/src/external/musicbrainz/api/mod.rs b/src/external/musicbrainz/api/mod.rs index a16da76..dc86686 100644 --- a/src/external/musicbrainz/api/mod.rs +++ b/src/external/musicbrainz/api/mod.rs @@ -61,7 +61,7 @@ impl MusicBrainzClient { } } -#[derive(Debug, Default)] +#[derive(Copy, Clone, Debug, Default, PartialEq, Eq)] pub struct PageSettings { limit: Option, offset: Option, @@ -80,7 +80,7 @@ impl PageSettings { } pub fn with_offset(mut self, offset: usize) -> Self { - self.offset = Some(offset); + self.set_offset(offset); self } diff --git a/src/tui/lib/external/musicbrainz/daemon/mod.rs b/src/tui/lib/external/musicbrainz/daemon/mod.rs index fa29297..744b75a 100644 --- a/src/tui/lib/external/musicbrainz/daemon/mod.rs +++ b/src/tui/lib/external/musicbrainz/daemon/mod.rs @@ -467,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() } @@ -609,6 +615,9 @@ 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, @@ -657,6 +666,9 @@ 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!( @@ -703,6 +715,9 @@ 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, @@ -759,6 +774,9 @@ 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(); @@ -828,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/daemon/mod.rs b/src/tui/lib/interface/musicbrainz/daemon/mod.rs index 24d0ff6..1c54b80 100644 --- a/src/tui/lib/interface/musicbrainz/daemon/mod.rs +++ b/src/tui/lib/interface/musicbrainz/daemon/mod.rs @@ -135,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)] -- 2.45.2 From e6c440ecc585783f78da86ec43bdfa66589126ba Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 6 Oct 2024 15:21:14 +0200 Subject: [PATCH 14/15] Simplify code --- src/core/collection/musicbrainz.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/core/collection/musicbrainz.rs b/src/core/collection/musicbrainz.rs index d6c4d02..575778a 100644 --- a/src/core/collection/musicbrainz.rs +++ b/src/core/collection/musicbrainz.rs @@ -47,13 +47,9 @@ pub enum MbRefOption { impl MbRefOption { pub fn or(self, optb: MbRefOption) -> MbRefOption { - match self { - opta @ MbRefOption::Some(_) => opta, - opta @ MbRefOption::CannotHaveMbid => match optb { - MbRefOption::Some(_) => optb, - MbRefOption::CannotHaveMbid | MbRefOption::None => opta, - }, - MbRefOption::None => optb, + match (&self, &optb) { + (MbRefOption::Some(_), _) | (MbRefOption::CannotHaveMbid, MbRefOption::None) => self, + _ => optb, } } -- 2.45.2 From 2c9884296a8ce0c77c8b0b795eed3d10b3e534ee Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 6 Oct 2024 15:24:51 +0200 Subject: [PATCH 15/15] Exclude unimplemented! from coverage --- .gitea/workflows/gitea-ci.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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/ -- 2.45.2