From f7e215eedfe0de641dc874390d3706046b221e0b Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 6 Oct 2024 11:14:33 +0200 Subject: [PATCH] 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;