From 3d447f324eff4dc66b0cb859c7d2e82d507f9f8e Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 29 Sep 2024 16:31:36 +0200 Subject: [PATCH 1/9] Browse API integration --- Cargo.toml | 5 + examples/musicbrainz_api/browse.rs | 98 ++++++++++++++++ examples/musicbrainz_api/lookup.rs | 2 - examples/musicbrainz_api/search.rs | 2 - src/external/musicbrainz/api/browse.rs | 111 ++++++++++++++++++ src/external/musicbrainz/api/lookup.rs | 4 +- src/external/musicbrainz/api/mod.rs | 7 +- .../musicbrainz/api/search/release_group.rs | 2 +- src/tui/lib/external/musicbrainz/api/mod.rs | 4 +- 9 files changed, 223 insertions(+), 12 deletions(-) create mode 100644 examples/musicbrainz_api/browse.rs create mode 100644 src/external/musicbrainz/api/browse.rs diff --git a/Cargo.toml b/Cargo.toml index 19b269e..4582b62 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -47,6 +47,11 @@ required-features = ["bin", "database-json", "library-beets", "library-beets-ssh name = "musichoard-edit" required-features = ["bin", "database-json"] +[[example]] +name = "musicbrainz-api---browse" +path = "examples/musicbrainz_api/browse.rs" +required-features = ["bin", "musicbrainz"] + [[example]] name = "musicbrainz-api---lookup" path = "examples/musicbrainz_api/lookup.rs" diff --git a/examples/musicbrainz_api/browse.rs b/examples/musicbrainz_api/browse.rs new file mode 100644 index 0000000..397dd3a --- /dev/null +++ b/examples/musicbrainz_api/browse.rs @@ -0,0 +1,98 @@ +use std::{thread, time}; + +use musichoard::{ + collection::musicbrainz::Mbid, + external::musicbrainz::{ + api::{browse::BrowseReleaseGroupRequest, MusicBrainzClient}, + http::MusicBrainzHttp, + }, +}; +use structopt::StructOpt; +use uuid::Uuid; + +const USER_AGENT: &str = concat!( + "MusicHoard---examples---musicbrainz-api---browse/", + env!("CARGO_PKG_VERSION"), + " ( musichoard@thenineworlds.net )" +); + +#[derive(StructOpt)] +struct Opt { + #[structopt(subcommand)] + entity: OptEntity, +} + +#[derive(StructOpt)] +enum OptEntity { + #[structopt(about = "Browse release groups")] + ReleaseGroup(OptReleaseGroup), +} + +#[derive(StructOpt)] +enum OptReleaseGroup { + #[structopt(about = "Browse release groups of an artist")] + Artist(OptMbid), +} + +#[derive(StructOpt)] +struct OptMbid { + #[structopt(help = "MBID of the entity")] + mbid: Uuid, +} + +fn main() { + let opt = Opt::from_args(); + + println!("USER_AGENT: {USER_AGENT}"); + + let http = MusicBrainzHttp::new(USER_AGENT).expect("failed to create API client"); + let mut client = MusicBrainzClient::new(http); + + match opt.entity { + OptEntity::ReleaseGroup(opt_release_group) => match opt_release_group { + OptReleaseGroup::Artist(opt_mbid) => { + let mbid: Mbid = opt_mbid.mbid.into(); + let mut request = BrowseReleaseGroupRequest::artist(&mbid).with_max_limit(); + + let mut response_counts: Vec = Vec::new(); + + loop { + let response = client + .browse_release_group(&request) + .expect("failed to make API call"); + + for rg in response.release_groups.iter() { + println!("{rg:?}\n"); + } + + let offset = response.release_group_offset; + let count = response.release_groups.len(); + response_counts.push(count); + let total = response.release_group_count; + + println!("Release group offset : {offset}"); + println!("Release groups in this response: {count}"); + println!("Release groups in total : {total}"); + + let next_offset = offset + count; + if next_offset == total { + break; + } + request.with_offset(next_offset); + + thread::sleep(time::Duration::from_secs(1)); + } + + println!( + "Total: {}={} release groups", + response_counts + .iter() + .map(|i| i.to_string()) + .collect::>() + .join("+"), + response_counts.iter().sum::(), + ); + } + }, + } +} diff --git a/examples/musicbrainz_api/lookup.rs b/examples/musicbrainz_api/lookup.rs index 29e02ee..e3590b3 100644 --- a/examples/musicbrainz_api/lookup.rs +++ b/examples/musicbrainz_api/lookup.rs @@ -1,5 +1,3 @@ -#![allow(non_snake_case)] - use musichoard::{ collection::musicbrainz::Mbid, external::musicbrainz::{ diff --git a/examples/musicbrainz_api/search.rs b/examples/musicbrainz_api/search.rs index 96fd893..7f4d2f6 100644 --- a/examples/musicbrainz_api/search.rs +++ b/examples/musicbrainz_api/search.rs @@ -1,5 +1,3 @@ -#![allow(non_snake_case)] - use std::{num::ParseIntError, str::FromStr}; use musichoard::{ diff --git a/src/external/musicbrainz/api/browse.rs b/src/external/musicbrainz/api/browse.rs new file mode 100644 index 0000000..9a3ef17 --- /dev/null +++ b/src/external/musicbrainz/api/browse.rs @@ -0,0 +1,111 @@ +use std::fmt; + +use serde::Deserialize; + +use crate::{ + collection::musicbrainz::Mbid, + external::musicbrainz::{ + api::{Error, MusicBrainzClient, MB_BASE_URL}, + IMusicBrainzHttp, + }, +}; + +use super::{MbReleaseGroupMeta, SerdeMbReleaseGroupMeta}; + +const MB_MAX_BROWSE_LIMIT: usize = 100; + +impl MusicBrainzClient { + pub fn browse_release_group( + &mut self, + request: &BrowseReleaseGroupRequest, + ) -> Result { + let entity = &request.entity; + let mbid = request.mbid.uuid().as_hyphenated(); + let limit = request + .limit + .map(|l| format!("&limit={l}")) + .unwrap_or_default(); + let offset = request + .offset + .map(|o| format!("&offset={o}")) + .unwrap_or_default(); + + let url = format!("{MB_BASE_URL}/release-group?{entity}={mbid}{limit}{offset}"); + + let response: DeserializeBrowseReleaseGroupResponse = self.http.get(&url)?; + Ok(response.into()) + } +} + +pub struct BrowseReleaseGroupRequest<'a> { + entity: BrowseReleaseGroupRequestEntity, + mbid: &'a Mbid, + limit: Option, + offset: Option, +} + +enum BrowseReleaseGroupRequestEntity { + Artist, +} + +impl fmt::Display for BrowseReleaseGroupRequestEntity { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + BrowseReleaseGroupRequestEntity::Artist => write!(f, "artist"), + } + } +} + +impl<'a> BrowseReleaseGroupRequest<'a> { + pub fn artist(mbid: &'a Mbid) -> Self { + BrowseReleaseGroupRequest { + entity: BrowseReleaseGroupRequestEntity::Artist, + mbid, + limit: None, + offset: None, + } + } + + #[must_use] + pub const fn with_limit(mut self, limit: usize) -> Self { + self.limit = Some(limit); + self + } + + #[must_use] + pub const fn with_max_limit(self) -> Self { + self.with_limit(MB_MAX_BROWSE_LIMIT) + } + + pub fn with_offset(&mut self, offset: usize) { + self.offset = Some(offset); + } +} + +#[derive(Debug, PartialEq, Eq)] +pub struct BrowseReleaseGroupResponse { + pub release_group_offset: usize, + pub release_group_count: usize, + pub release_groups: Vec, +} + +#[derive(Deserialize)] +#[serde(rename_all(deserialize = "kebab-case"))] +struct DeserializeBrowseReleaseGroupResponse { + release_group_offset: usize, + release_group_count: usize, + release_groups: Option>, +} + +impl From for BrowseReleaseGroupResponse { + fn from(value: DeserializeBrowseReleaseGroupResponse) -> Self { + BrowseReleaseGroupResponse { + release_group_offset: value.release_group_offset, + release_group_count: value.release_group_count, + release_groups: value + .release_groups + .map(|rgs| rgs.into_iter().map(Into::into).collect()) + .unwrap_or_default(), + } + } +} diff --git a/src/external/musicbrainz/api/lookup.rs b/src/external/musicbrainz/api/lookup.rs index 46be188..3bfc5c7 100644 --- a/src/external/musicbrainz/api/lookup.rs +++ b/src/external/musicbrainz/api/lookup.rs @@ -152,7 +152,7 @@ mod tests { id: SerdeMbid("11111111-1111-1111-1111-111111111111".try_into().unwrap()), title: String::from("an album"), first_release_date: SerdeAlbumDate((1986, 4).into()), - primary_type: SerdeAlbumPrimaryType(AlbumPrimaryType::Album), + primary_type: Some(SerdeAlbumPrimaryType(AlbumPrimaryType::Album)), secondary_types: Some(vec![SerdeAlbumSecondaryType( AlbumSecondaryType::Compilation, )]), @@ -192,7 +192,7 @@ mod tests { id: SerdeMbid("11111111-1111-1111-1111-111111111111".try_into().unwrap()), title: String::from("an album"), first_release_date: SerdeAlbumDate((1986, 4).into()), - primary_type: SerdeAlbumPrimaryType(AlbumPrimaryType::Album), + primary_type: Some(SerdeAlbumPrimaryType(AlbumPrimaryType::Album)), secondary_types: Some(vec![SerdeAlbumSecondaryType( AlbumSecondaryType::Compilation, )]), diff --git a/src/external/musicbrainz/api/mod.rs b/src/external/musicbrainz/api/mod.rs index 70e9299..b269ee1 100644 --- a/src/external/musicbrainz/api/mod.rs +++ b/src/external/musicbrainz/api/mod.rs @@ -11,6 +11,7 @@ use crate::{ external::musicbrainz::HttpError, }; +pub mod browse; pub mod lookup; pub mod search; @@ -91,7 +92,7 @@ pub struct MbReleaseGroupMeta { pub id: Mbid, pub title: String, pub first_release_date: AlbumDate, - pub primary_type: AlbumPrimaryType, + pub primary_type: Option, pub secondary_types: Option>, } @@ -101,7 +102,7 @@ pub struct SerdeMbReleaseGroupMeta { id: SerdeMbid, title: String, first_release_date: SerdeAlbumDate, - primary_type: SerdeAlbumPrimaryType, + primary_type: Option, secondary_types: Option>, } @@ -111,7 +112,7 @@ impl From for MbReleaseGroupMeta { id: value.id.into(), title: value.title, first_release_date: value.first_release_date.into(), - primary_type: value.primary_type.into(), + primary_type: value.primary_type.map(Into::into), secondary_types: value .secondary_types .map(|v| v.into_iter().map(Into::into).collect()), diff --git a/src/external/musicbrainz/api/search/release_group.rs b/src/external/musicbrainz/api/search/release_group.rs index d33d558..6e65c26 100644 --- a/src/external/musicbrainz/api/search/release_group.rs +++ b/src/external/musicbrainz/api/search/release_group.rs @@ -115,7 +115,7 @@ mod tests { id: SerdeMbid("11111111-1111-1111-1111-111111111111".try_into().unwrap()), title: String::from("an album"), first_release_date: SerdeAlbumDate((1986, 4).into()), - primary_type: SerdeAlbumPrimaryType(AlbumPrimaryType::Album), + primary_type: Some(SerdeAlbumPrimaryType(AlbumPrimaryType::Album)), secondary_types: Some(vec![SerdeAlbumSecondaryType(AlbumSecondaryType::Live)]), }, }; diff --git a/src/tui/lib/external/musicbrainz/api/mod.rs b/src/tui/lib/external/musicbrainz/api/mod.rs index f248f1d..b597cdb 100644 --- a/src/tui/lib/external/musicbrainz/api/mod.rs +++ b/src/tui/lib/external/musicbrainz/api/mod.rs @@ -115,7 +115,7 @@ fn from_lookup_release_group_response(entity: LookupReleaseGroupResponse) -> Loo seq: AlbumSeq::default(), info: AlbumInfo { musicbrainz: MbRefOption::Some(entity.meta.id.into()), - primary_type: Some(entity.meta.primary_type), + primary_type: entity.meta.primary_type, secondary_types: entity.meta.secondary_types.unwrap_or_default(), }, }, @@ -150,7 +150,7 @@ fn from_search_release_group_response_release_group( seq: AlbumSeq::default(), info: AlbumInfo { musicbrainz: MbRefOption::Some(entity.meta.id.into()), - primary_type: Some(entity.meta.primary_type), + primary_type: entity.meta.primary_type, secondary_types: entity.meta.secondary_types.unwrap_or_default(), }, }, -- 2.45.2 From 6083e829247e1a2f42861c85b3c1f0a552202af0 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 29 Sep 2024 16:33:35 +0200 Subject: [PATCH 2/9] fix --- src/external/musicbrainz/api/browse.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/external/musicbrainz/api/browse.rs b/src/external/musicbrainz/api/browse.rs index 9a3ef17..d79ffe0 100644 --- a/src/external/musicbrainz/api/browse.rs +++ b/src/external/musicbrainz/api/browse.rs @@ -5,13 +5,11 @@ use serde::Deserialize; use crate::{ collection::musicbrainz::Mbid, external::musicbrainz::{ - api::{Error, MusicBrainzClient, MB_BASE_URL}, + api::{Error, MbReleaseGroupMeta, MusicBrainzClient, SerdeMbReleaseGroupMeta, MB_BASE_URL}, IMusicBrainzHttp, }, }; -use super::{MbReleaseGroupMeta, SerdeMbReleaseGroupMeta}; - const MB_MAX_BROWSE_LIMIT: usize = 100; impl MusicBrainzClient { -- 2.45.2 From 70e510a1de6f12d1acdff7bd78ba8d518841ef12 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 29 Sep 2024 16:44:19 +0200 Subject: [PATCH 3/9] Unit tests --- src/external/musicbrainz/api/browse.rs | 94 +++++++++++++++++++++++++- 1 file changed, 93 insertions(+), 1 deletion(-) diff --git a/src/external/musicbrainz/api/browse.rs b/src/external/musicbrainz/api/browse.rs index d79ffe0..ce9ba36 100644 --- a/src/external/musicbrainz/api/browse.rs +++ b/src/external/musicbrainz/api/browse.rs @@ -87,7 +87,7 @@ pub struct BrowseReleaseGroupResponse { pub release_groups: Vec, } -#[derive(Deserialize)] +#[derive(Clone, Deserialize)] #[serde(rename_all(deserialize = "kebab-case"))] struct DeserializeBrowseReleaseGroupResponse { release_group_offset: usize, @@ -107,3 +107,95 @@ impl From for BrowseReleaseGroupResponse } } } + +#[cfg(test)] +mod tests { + use mockall::{predicate, Sequence}; + + use crate::{ + collection::album::{AlbumPrimaryType, AlbumSecondaryType}, + external::musicbrainz::{ + api::{SerdeAlbumDate, SerdeAlbumPrimaryType, SerdeAlbumSecondaryType, SerdeMbid}, + MockIMusicBrainzHttp, + }, + }; + + use super::*; + + #[test] + fn browse_release_group() { + let mbid = "00000000-0000-0000-0000-000000000000"; + let mut http = MockIMusicBrainzHttp::new(); + + let de_release_group_offset = 24; + let de_release_group_count = 302; + let de_meta = SerdeMbReleaseGroupMeta { + id: SerdeMbid("11111111-1111-1111-1111-111111111111".try_into().unwrap()), + title: String::from("an album"), + first_release_date: SerdeAlbumDate((1986, 4).into()), + primary_type: Some(SerdeAlbumPrimaryType(AlbumPrimaryType::Album)), + secondary_types: Some(vec![SerdeAlbumSecondaryType( + AlbumSecondaryType::Compilation, + )]), + }; + let de_response = DeserializeBrowseReleaseGroupResponse { + release_group_offset: de_release_group_offset, + release_group_count: de_release_group_count, + release_groups: Some(vec![de_meta.clone()]), + }; + + let response = BrowseReleaseGroupResponse { + release_group_offset: de_release_group_offset, + release_group_count: de_release_group_count, + release_groups: vec![de_meta.clone().into()], + }; + + let mut seq = Sequence::new(); + + let url = format!("https://musicbrainz.org/ws/2/release-group?artist={mbid}",); + let expect_response = de_response.clone(); + http.expect_get() + .times(1) + .in_sequence(&mut seq) + .with(predicate::eq(url)) + .return_once(|_| Ok(expect_response)); + + let url = format!( + "https://musicbrainz.org/ws/2/release-group?artist={mbid}&limit={MB_MAX_BROWSE_LIMIT}", + ); + let expect_response = de_response.clone(); + http.expect_get() + .times(1) + .in_sequence(&mut seq) + .with(predicate::eq(url)) + .return_once(|_| Ok(expect_response)); + + let url = format!( + "https://musicbrainz.org/ws/2/release-group?artist={mbid}&limit={MB_MAX_BROWSE_LIMIT}&offset={offset}", + offset = de_release_group_offset, + ); + let expect_response = de_response.clone(); + http.expect_get() + .times(1) + .in_sequence(&mut seq) + .with(predicate::eq(url)) + .return_once(|_| Ok(expect_response)); + + let mut client = MusicBrainzClient::new(http); + + let mbid: Mbid = mbid.try_into().unwrap(); + + let request = BrowseReleaseGroupRequest::artist(&mbid); + let result = client.browse_release_group(&request).unwrap(); + assert_eq!(result, response); + + let request = request.with_max_limit(); + let result = client.browse_release_group(&request).unwrap(); + assert_eq!(result, response); + + let mut request = request; + request.with_offset(de_release_group_offset); + let result = client.browse_release_group(&request).unwrap(); + assert_eq!(result, response); + } +} -- 2.45.2 From 2d88e45d252527724890b968b92619f9bf23d4d9 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 29 Sep 2024 16:50:24 +0200 Subject: [PATCH 4/9] MB API requests as references --- examples/musicbrainz_api/lookup.rs | 4 ++-- examples/musicbrainz_api/search.rs | 4 ++-- src/external/musicbrainz/api/lookup.rs | 8 ++++---- src/external/musicbrainz/api/search/artist.rs | 2 +- src/external/musicbrainz/api/search/mod.rs | 2 +- src/external/musicbrainz/api/search/release_group.rs | 6 +++--- src/tui/lib/external/musicbrainz/api/mod.rs | 8 ++++---- 7 files changed, 17 insertions(+), 17 deletions(-) diff --git a/examples/musicbrainz_api/lookup.rs b/examples/musicbrainz_api/lookup.rs index e3590b3..139c2cf 100644 --- a/examples/musicbrainz_api/lookup.rs +++ b/examples/musicbrainz_api/lookup.rs @@ -62,7 +62,7 @@ fn main() { } let response = client - .lookup_artist(request) + .lookup_artist(&request) .expect("failed to make API call"); println!("{response:#?}"); @@ -72,7 +72,7 @@ fn main() { let request = LookupReleaseGroupRequest::new(&mbid); let response = client - .lookup_release_group(request) + .lookup_release_group(&request) .expect("failed to make API call"); println!("{response:#?}"); diff --git a/examples/musicbrainz_api/search.rs b/examples/musicbrainz_api/search.rs index 7f4d2f6..c5cb9b2 100644 --- a/examples/musicbrainz_api/search.rs +++ b/examples/musicbrainz_api/search.rs @@ -107,7 +107,7 @@ fn main() { println!("Query: {query}"); let matches = client - .search_artist(query) + .search_artist(&query) .expect("failed to make API call"); println!("{matches:#?}"); @@ -139,7 +139,7 @@ fn main() { println!("Query: {query}"); let matches = client - .search_release_group(query) + .search_release_group(&query) .expect("failed to make API call"); println!("{matches:#?}"); diff --git a/src/external/musicbrainz/api/lookup.rs b/src/external/musicbrainz/api/lookup.rs index 3bfc5c7..82197d5 100644 --- a/src/external/musicbrainz/api/lookup.rs +++ b/src/external/musicbrainz/api/lookup.rs @@ -14,7 +14,7 @@ use super::{MbArtistMeta, MbReleaseGroupMeta, SerdeMbArtistMeta, SerdeMbReleaseG impl MusicBrainzClient { pub fn lookup_artist( &mut self, - request: LookupArtistRequest, + request: &LookupArtistRequest, ) -> Result { let mut include: Vec = vec![]; @@ -35,7 +35,7 @@ impl MusicBrainzClient { pub fn lookup_release_group( &mut self, - request: LookupReleaseGroupRequest, + request: &LookupReleaseGroupRequest, ) -> Result { let url = format!( "{MB_BASE_URL}/release-group/{mbid}", @@ -177,7 +177,7 @@ mod tests { let mbid: Mbid = "00000000-0000-0000-0000-000000000000".try_into().unwrap(); let mut request = LookupArtistRequest::new(&mbid); request.include_release_groups(); - let result = client.lookup_artist(request).unwrap(); + let result = client.lookup_artist(&request).unwrap(); assert_eq!(result, response); } @@ -214,7 +214,7 @@ mod tests { let mbid: Mbid = "00000000-0000-0000-0000-000000000000".try_into().unwrap(); let request = LookupReleaseGroupRequest::new(&mbid); - let result = client.lookup_release_group(request).unwrap(); + let result = client.lookup_release_group(&request).unwrap(); assert_eq!(result, response); } diff --git a/src/external/musicbrainz/api/search/artist.rs b/src/external/musicbrainz/api/search/artist.rs index 12d1bf1..37780a7 100644 --- a/src/external/musicbrainz/api/search/artist.rs +++ b/src/external/musicbrainz/api/search/artist.rs @@ -128,7 +128,7 @@ mod tests { let query = SearchArtistRequest::new().string(name); - let matches = client.search_artist(query).unwrap(); + let matches = client.search_artist(&query).unwrap(); assert_eq!(matches, response); } } diff --git a/src/external/musicbrainz/api/search/mod.rs b/src/external/musicbrainz/api/search/mod.rs index 564063b..985528a 100644 --- a/src/external/musicbrainz/api/search/mod.rs +++ b/src/external/musicbrainz/api/search/mod.rs @@ -27,7 +27,7 @@ macro_rules! impl_search_entity { paste! { pub fn []( &mut self, - query: [] + query: &[] ) -> Result<[], Error> { let query: String = form_urlencoded::byte_serialize(format!("{query}").as_bytes()).collect(); diff --git a/src/external/musicbrainz/api/search/release_group.rs b/src/external/musicbrainz/api/search/release_group.rs index 6e65c26..7dddfa8 100644 --- a/src/external/musicbrainz/api/search/release_group.rs +++ b/src/external/musicbrainz/api/search/release_group.rs @@ -161,7 +161,7 @@ mod tests { let query = SearchReleaseGroupRequest::new().string(title); - let matches = client.search_release_group(query).unwrap(); + let matches = client.search_release_group(&query).unwrap(); assert_eq!(matches, response); } @@ -198,7 +198,7 @@ mod tests { .and() .first_release_date(&date); - let matches = client.search_release_group(query).unwrap(); + let matches = client.search_release_group(&query).unwrap(); assert_eq!(matches, response); } @@ -226,7 +226,7 @@ mod tests { let query = SearchReleaseGroupRequest::new().rgid(&rgid); - let matches = client.search_release_group(query).unwrap(); + let matches = client.search_release_group(&query).unwrap(); assert_eq!(matches, response); } } diff --git a/src/tui/lib/external/musicbrainz/api/mod.rs b/src/tui/lib/external/musicbrainz/api/mod.rs index b597cdb..c7b7ac9 100644 --- a/src/tui/lib/external/musicbrainz/api/mod.rs +++ b/src/tui/lib/external/musicbrainz/api/mod.rs @@ -41,7 +41,7 @@ impl IMusicBrainz for MusicBrainz { fn lookup_artist(&mut self, mbid: &Mbid) -> Result, Error> { let request = LookupArtistRequest::new(mbid); - let mb_response = self.client.lookup_artist(request)?; + let mb_response = self.client.lookup_artist(&request)?; Ok(from_lookup_artist_response(mb_response)) } @@ -49,7 +49,7 @@ impl IMusicBrainz for MusicBrainz { fn lookup_release_group(&mut self, mbid: &Mbid) -> Result, Error> { let request = LookupReleaseGroupRequest::new(mbid); - let mb_response = self.client.lookup_release_group(request)?; + let mb_response = self.client.lookup_release_group(&request)?; Ok(from_lookup_release_group_response(mb_response)) } @@ -57,7 +57,7 @@ impl IMusicBrainz for MusicBrainz { fn search_artist(&mut self, artist: &ArtistMeta) -> Result>, Error> { let query = SearchArtistRequest::new().string(&artist.id.name); - let mb_response = self.client.search_artist(query)?; + let mb_response = self.client.search_artist(&query)?; Ok(mb_response .artists @@ -82,7 +82,7 @@ impl IMusicBrainz for MusicBrainz { .and() .release_group(&album.id.title); - let mb_response = self.client.search_release_group(query)?; + let mb_response = self.client.search_release_group(&query)?; Ok(mb_response .release_groups -- 2.45.2 From e2c103b4d7b826944c5cf785780865c4073d45e8 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 29 Sep 2024 17:31:36 +0200 Subject: [PATCH 5/9] Generalise paging --- examples/musicbrainz_api/browse.rs | 9 ++- examples/musicbrainz_api/search.rs | 8 +- src/external/musicbrainz/api/browse.rs | 81 ++++--------------- src/external/musicbrainz/api/mod.rs | 57 +++++++++++++ src/external/musicbrainz/api/search/artist.rs | 5 +- src/external/musicbrainz/api/search/mod.rs | 8 +- .../musicbrainz/api/search/release_group.rs | 13 +-- src/tui/lib/external/musicbrainz/api/mod.rs | 8 +- 8 files changed, 104 insertions(+), 85 deletions(-) diff --git a/examples/musicbrainz_api/browse.rs b/examples/musicbrainz_api/browse.rs index 397dd3a..cff2548 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}, + api::{browse::BrowseReleaseGroupRequest, MusicBrainzClient, PageSettings}, http::MusicBrainzHttp, }, }; @@ -52,13 +52,14 @@ fn main() { OptEntity::ReleaseGroup(opt_release_group) => match opt_release_group { OptReleaseGroup::Artist(opt_mbid) => { let mbid: Mbid = opt_mbid.mbid.into(); - let mut request = BrowseReleaseGroupRequest::artist(&mbid).with_max_limit(); + let request = BrowseReleaseGroupRequest::artist(&mbid); + let mut paging = PageSettings::with_max_limit(); let mut response_counts: Vec = Vec::new(); loop { let response = client - .browse_release_group(&request) + .browse_release_group(&request, &paging) .expect("failed to make API call"); for rg in response.release_groups.iter() { @@ -78,7 +79,7 @@ fn main() { if next_offset == total { break; } - request.with_offset(next_offset); + paging.with_offset(next_offset); thread::sleep(time::Duration::from_secs(1)); } diff --git a/examples/musicbrainz_api/search.rs b/examples/musicbrainz_api/search.rs index c5cb9b2..5d6f44f 100644 --- a/examples/musicbrainz_api/search.rs +++ b/examples/musicbrainz_api/search.rs @@ -5,7 +5,7 @@ use musichoard::{ external::musicbrainz::{ api::{ search::{SearchArtistRequest, SearchReleaseGroupRequest}, - MusicBrainzClient, + MusicBrainzClient, PageSettings, }, http::MusicBrainzHttp, }, @@ -106,8 +106,9 @@ fn main() { println!("Query: {query}"); + let paging = PageSettings::default(); let matches = client - .search_artist(&query) + .search_artist(&query, &paging) .expect("failed to make API call"); println!("{matches:#?}"); @@ -138,8 +139,9 @@ fn main() { println!("Query: {query}"); + let paging = PageSettings::default(); let matches = client - .search_release_group(&query) + .search_release_group(&query, &paging) .expect("failed to make API call"); println!("{matches:#?}"); diff --git a/src/external/musicbrainz/api/browse.rs b/src/external/musicbrainz/api/browse.rs index ce9ba36..0156629 100644 --- a/src/external/musicbrainz/api/browse.rs +++ b/src/external/musicbrainz/api/browse.rs @@ -5,30 +5,27 @@ use serde::Deserialize; use crate::{ collection::musicbrainz::Mbid, external::musicbrainz::{ - api::{Error, MbReleaseGroupMeta, MusicBrainzClient, SerdeMbReleaseGroupMeta, MB_BASE_URL}, + api::{ + Error, MbReleaseGroupMeta, MusicBrainzClient, PageSettings, SerdeMbReleaseGroupMeta, + MB_BASE_URL, + }, IMusicBrainzHttp, }, }; -const MB_MAX_BROWSE_LIMIT: usize = 100; +use super::ApiDisplay; impl MusicBrainzClient { pub fn browse_release_group( &mut self, request: &BrowseReleaseGroupRequest, + paging: &PageSettings, ) -> Result { let entity = &request.entity; let mbid = request.mbid.uuid().as_hyphenated(); - let limit = request - .limit - .map(|l| format!("&limit={l}")) - .unwrap_or_default(); - let offset = request - .offset - .map(|o| format!("&offset={o}")) - .unwrap_or_default(); + let page = ApiDisplay::format_page_settings(paging); - let url = format!("{MB_BASE_URL}/release-group?{entity}={mbid}{limit}{offset}"); + let url = format!("{MB_BASE_URL}/release-group?{entity}={mbid}{page}"); let response: DeserializeBrowseReleaseGroupResponse = self.http.get(&url)?; Ok(response.into()) @@ -38,8 +35,6 @@ impl MusicBrainzClient { pub struct BrowseReleaseGroupRequest<'a> { entity: BrowseReleaseGroupRequestEntity, mbid: &'a Mbid, - limit: Option, - offset: Option, } enum BrowseReleaseGroupRequestEntity { @@ -59,25 +54,8 @@ impl<'a> BrowseReleaseGroupRequest<'a> { BrowseReleaseGroupRequest { entity: BrowseReleaseGroupRequestEntity::Artist, mbid, - limit: None, - offset: None, } } - - #[must_use] - pub const fn with_limit(mut self, limit: usize) -> Self { - self.limit = Some(limit); - self - } - - #[must_use] - pub const fn with_max_limit(self) -> Self { - self.with_limit(MB_MAX_BROWSE_LIMIT) - } - - pub fn with_offset(&mut self, offset: usize) { - self.offset = Some(offset); - } } #[derive(Debug, PartialEq, Eq)] @@ -110,12 +88,15 @@ impl From for BrowseReleaseGroupResponse #[cfg(test)] mod tests { - use mockall::{predicate, Sequence}; + use mockall::predicate; use crate::{ collection::album::{AlbumPrimaryType, AlbumSecondaryType}, external::musicbrainz::{ - api::{SerdeAlbumDate, SerdeAlbumPrimaryType, SerdeAlbumSecondaryType, SerdeMbid}, + api::{ + SerdeAlbumDate, SerdeAlbumPrimaryType, SerdeAlbumSecondaryType, SerdeMbid, + MB_MAX_PAGE_LIMIT, + }, MockIMusicBrainzHttp, }, }; @@ -150,34 +131,12 @@ mod tests { release_groups: vec![de_meta.clone().into()], }; - let mut seq = Sequence::new(); - - let url = format!("https://musicbrainz.org/ws/2/release-group?artist={mbid}",); - let expect_response = de_response.clone(); - http.expect_get() - .times(1) - .in_sequence(&mut seq) - .with(predicate::eq(url)) - .return_once(|_| Ok(expect_response)); - let url = format!( - "https://musicbrainz.org/ws/2/release-group?artist={mbid}&limit={MB_MAX_BROWSE_LIMIT}", + "https://musicbrainz.org/ws/2/release-group?artist={mbid}&limit={MB_MAX_PAGE_LIMIT}", ); let expect_response = de_response.clone(); http.expect_get() .times(1) - .in_sequence(&mut seq) - .with(predicate::eq(url)) - .return_once(|_| Ok(expect_response)); - - let url = format!( - "https://musicbrainz.org/ws/2/release-group?artist={mbid}&limit={MB_MAX_BROWSE_LIMIT}&offset={offset}", - offset = de_release_group_offset, - ); - let expect_response = de_response.clone(); - http.expect_get() - .times(1) - .in_sequence(&mut seq) .with(predicate::eq(url)) .return_once(|_| Ok(expect_response)); @@ -186,16 +145,8 @@ mod tests { let mbid: Mbid = mbid.try_into().unwrap(); let request = BrowseReleaseGroupRequest::artist(&mbid); - let result = client.browse_release_group(&request).unwrap(); - assert_eq!(result, response); - - let request = request.with_max_limit(); - let result = client.browse_release_group(&request).unwrap(); - assert_eq!(result, response); - - let mut request = request; - request.with_offset(de_release_group_offset); - let result = client.browse_release_group(&request).unwrap(); + let paging = PageSettings::with_max_limit(); + let result = client.browse_release_group(&request, &paging).unwrap(); assert_eq!(result, response); } } diff --git a/src/external/musicbrainz/api/mod.rs b/src/external/musicbrainz/api/mod.rs index b269ee1..34ab3cd 100644 --- a/src/external/musicbrainz/api/mod.rs +++ b/src/external/musicbrainz/api/mod.rs @@ -17,6 +17,8 @@ pub mod search; const MB_BASE_URL: &str = "https://musicbrainz.org/ws/2"; const MB_RATE_LIMIT_CODE: u16 = 503; +const MB_MAX_PAGE_LIMIT: usize = 100; + #[derive(Debug, PartialEq, Eq)] pub enum Error { /// The HTTP client failed. @@ -59,6 +61,29 @@ impl MusicBrainzClient { } } +#[derive(Default)] +pub struct PageSettings { + pub limit: Option, + pub offset: Option, +} + +impl PageSettings { + pub fn with_limit(limit: usize) -> Self { + PageSettings { + limit: Some(limit), + ..Default::default() + } + } + + pub fn with_max_limit() -> Self { + Self::with_limit(MB_MAX_PAGE_LIMIT) + } + + pub fn with_offset(&mut self, offset: usize) { + self.offset = Some(offset); + } +} + #[derive(Clone, Debug, PartialEq, Eq)] pub struct MbArtistMeta { pub id: Mbid, @@ -123,6 +148,18 @@ impl From for MbReleaseGroupMeta { pub struct ApiDisplay; impl ApiDisplay { + fn format_page_settings(paging: &PageSettings) -> String { + let limit = paging + .limit + .map(|l| format!("&limit={l}")) + .unwrap_or_default(); + let offset = paging + .offset + .map(|o| format!("&offset={o}")) + .unwrap_or_default(); + format!("{limit}{offset}") + } + fn format_album_date(date: &AlbumDate) -> String { match date.year { Some(year) => match date.month { @@ -305,6 +342,26 @@ mod tests { assert!(!format!("{unk_err:?}").is_empty()); } + #[test] + fn format_page_settings() { + let paging = PageSettings::default(); + assert_eq!(ApiDisplay::format_page_settings(&paging), ""); + + let paging = PageSettings::with_max_limit(); + assert_eq!(ApiDisplay::format_page_settings(&paging), "&limit=100"); + + let mut paging = PageSettings::with_limit(45); + paging.with_offset(145); + assert_eq!( + ApiDisplay::format_page_settings(&paging), + "&limit=45&offset=145" + ); + + let mut paging = PageSettings::default(); + paging.with_offset(26); + assert_eq!(ApiDisplay::format_page_settings(&paging), "&offset=26"); + } + #[test] fn format_album_date() { assert_eq!( diff --git a/src/external/musicbrainz/api/search/artist.rs b/src/external/musicbrainz/api/search/artist.rs index 37780a7..8eaa4ea 100644 --- a/src/external/musicbrainz/api/search/artist.rs +++ b/src/external/musicbrainz/api/search/artist.rs @@ -70,7 +70,7 @@ mod tests { use mockall::predicate; use crate::external::musicbrainz::{ - api::{MusicBrainzClient, SerdeMbid}, + api::{MusicBrainzClient, PageSettings, SerdeMbid}, MockIMusicBrainzHttp, }; @@ -128,7 +128,8 @@ mod tests { let query = SearchArtistRequest::new().string(name); - let matches = client.search_artist(&query).unwrap(); + let paging = PageSettings::default(); + let matches = client.search_artist(&query, &paging).unwrap(); assert_eq!(matches, response); } } diff --git a/src/external/musicbrainz/api/search/mod.rs b/src/external/musicbrainz/api/search/mod.rs index 985528a..c4c1521 100644 --- a/src/external/musicbrainz/api/search/mod.rs +++ b/src/external/musicbrainz/api/search/mod.rs @@ -17,7 +17,7 @@ use crate::external::musicbrainz::{ artist::DeserializeSearchArtistResponse, release_group::DeserializeSearchReleaseGroupResponse, }, - Error, MusicBrainzClient, MB_BASE_URL, + ApiDisplay, Error, MusicBrainzClient, PageSettings, MB_BASE_URL, }, IMusicBrainzHttp, }; @@ -27,11 +27,13 @@ macro_rules! impl_search_entity { paste! { pub fn []( &mut self, - query: &[] + query: &[], + paging: &PageSettings, ) -> Result<[], Error> { let query: String = form_urlencoded::byte_serialize(format!("{query}").as_bytes()).collect(); - let url = format!("{MB_BASE_URL}/{entity}?query={query}", entity = $entity); + let page = ApiDisplay::format_page_settings(paging); + let url = format!("{MB_BASE_URL}/{entity}?query={query}{page}", entity = $entity); let response: [] = self.http.get(&url)?; Ok(response.into()) diff --git a/src/external/musicbrainz/api/search/release_group.rs b/src/external/musicbrainz/api/search/release_group.rs index 7dddfa8..be48a7f 100644 --- a/src/external/musicbrainz/api/search/release_group.rs +++ b/src/external/musicbrainz/api/search/release_group.rs @@ -99,8 +99,8 @@ mod tests { collection::album::{AlbumPrimaryType, AlbumSecondaryType}, external::musicbrainz::{ api::{ - MusicBrainzClient, SerdeAlbumDate, SerdeAlbumPrimaryType, SerdeAlbumSecondaryType, - SerdeMbid, + MusicBrainzClient, PageSettings, SerdeAlbumDate, SerdeAlbumPrimaryType, + SerdeAlbumSecondaryType, SerdeMbid, }, MockIMusicBrainzHttp, }, @@ -161,7 +161,8 @@ mod tests { let query = SearchReleaseGroupRequest::new().string(title); - let matches = client.search_release_group(&query).unwrap(); + let paging = PageSettings::default(); + let matches = client.search_release_group(&query, &paging).unwrap(); assert_eq!(matches, response); } @@ -198,7 +199,8 @@ mod tests { .and() .first_release_date(&date); - let matches = client.search_release_group(&query).unwrap(); + let paging = PageSettings::default(); + let matches = client.search_release_group(&query, &paging).unwrap(); assert_eq!(matches, response); } @@ -226,7 +228,8 @@ mod tests { let query = SearchReleaseGroupRequest::new().rgid(&rgid); - let matches = client.search_release_group(&query).unwrap(); + let paging = PageSettings::default(); + let matches = client.search_release_group(&query, &paging).unwrap(); assert_eq!(matches, response); } } diff --git a/src/tui/lib/external/musicbrainz/api/mod.rs b/src/tui/lib/external/musicbrainz/api/mod.rs index c7b7ac9..3e4c0cb 100644 --- a/src/tui/lib/external/musicbrainz/api/mod.rs +++ b/src/tui/lib/external/musicbrainz/api/mod.rs @@ -18,7 +18,7 @@ use musichoard::{ SearchArtistRequest, SearchArtistResponseArtist, SearchReleaseGroupRequest, SearchReleaseGroupResponseReleaseGroup, }, - MusicBrainzClient, + MusicBrainzClient, PageSettings, }, IMusicBrainzHttp, }, @@ -57,7 +57,8 @@ impl IMusicBrainz for MusicBrainz { fn search_artist(&mut self, artist: &ArtistMeta) -> Result>, Error> { let query = SearchArtistRequest::new().string(&artist.id.name); - let mb_response = self.client.search_artist(&query)?; + let paging = PageSettings::with_max_limit(); + let mb_response = self.client.search_artist(&query, &paging)?; Ok(mb_response .artists @@ -82,7 +83,8 @@ impl IMusicBrainz for MusicBrainz { .and() .release_group(&album.id.title); - let mb_response = self.client.search_release_group(&query)?; + let paging = PageSettings::with_max_limit(); + let mb_response = self.client.search_release_group(&query, &paging)?; Ok(mb_response .release_groups -- 2.45.2 From 77a97659d17d4acc4f2b908e6720e0b1b7050bfd Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 29 Sep 2024 21:06:29 +0200 Subject: [PATCH 6/9] Complete support for paging --- examples/musicbrainz_api/browse.rs | 13 +++--- src/external/musicbrainz/api/browse.rs | 42 +++++++++++++------ src/external/musicbrainz/api/mod.rs | 16 +++++++ src/external/musicbrainz/api/search/artist.rs | 16 ++++++- src/external/musicbrainz/api/search/mod.rs | 18 ++++++++ .../musicbrainz/api/search/release_group.rs | 16 ++++++- src/tui/lib/external/musicbrainz/api/mod.rs | 4 +- 7 files changed, 101 insertions(+), 24 deletions(-) diff --git a/examples/musicbrainz_api/browse.rs b/examples/musicbrainz_api/browse.rs index cff2548..af8d968 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, PageSettings}, + api::{browse::BrowseReleaseGroupRequest, MusicBrainzClient, NextPage, PageSettings}, http::MusicBrainzHttp, }, }; @@ -66,20 +66,19 @@ fn main() { println!("{rg:?}\n"); } - let offset = response.release_group_offset; + let offset = response.page.release_group_offset; let count = response.release_groups.len(); response_counts.push(count); - let total = response.release_group_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}"); - let next_offset = offset + count; - if next_offset == total { - break; + match response.page.next_page_offset(count) { + NextPage::Offset(next_offset) => paging.with_offset(next_offset), + NextPage::Complete => break, } - paging.with_offset(next_offset); thread::sleep(time::Duration::from_secs(1)); } diff --git a/src/external/musicbrainz/api/browse.rs b/src/external/musicbrainz/api/browse.rs index 0156629..e6299b0 100644 --- a/src/external/musicbrainz/api/browse.rs +++ b/src/external/musicbrainz/api/browse.rs @@ -6,14 +6,31 @@ use crate::{ collection::musicbrainz::Mbid, external::musicbrainz::{ api::{ - Error, MbReleaseGroupMeta, MusicBrainzClient, PageSettings, SerdeMbReleaseGroupMeta, - MB_BASE_URL, + ApiDisplay, Error, MbReleaseGroupMeta, MusicBrainzClient, NextPage, PageSettings, + SerdeMbReleaseGroupMeta, MB_BASE_URL, }, IMusicBrainzHttp, }, }; -use super::ApiDisplay; +#[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, +} + +impl BrowseReleaseGroupPage { + pub fn next_page_offset(&self, page_count: usize) -> NextPage { + NextPage::next_page_offset( + self.release_group_offset, + self.release_group_count, + page_count, + ) + } +} + +pub type SerdeBrowseReleaseGroupPage = BrowseReleaseGroupPage; impl MusicBrainzClient { pub fn browse_release_group( @@ -60,24 +77,22 @@ impl<'a> BrowseReleaseGroupRequest<'a> { #[derive(Debug, PartialEq, Eq)] pub struct BrowseReleaseGroupResponse { - pub release_group_offset: usize, - pub release_group_count: usize, pub release_groups: Vec, + pub page: BrowseReleaseGroupPage, } #[derive(Clone, Deserialize)] #[serde(rename_all(deserialize = "kebab-case"))] struct DeserializeBrowseReleaseGroupResponse { - release_group_offset: usize, - release_group_count: usize, release_groups: Option>, + #[serde(flatten)] + page: SerdeBrowseReleaseGroupPage, } impl From for BrowseReleaseGroupResponse { fn from(value: DeserializeBrowseReleaseGroupResponse) -> Self { BrowseReleaseGroupResponse { - release_group_offset: value.release_group_offset, - release_group_count: value.release_group_count, + page: value.page, release_groups: value .release_groups .map(|rgs| rgs.into_iter().map(Into::into).collect()) @@ -120,14 +135,15 @@ mod tests { )]), }; let de_response = DeserializeBrowseReleaseGroupResponse { - release_group_offset: de_release_group_offset, - release_group_count: de_release_group_count, + page: SerdeBrowseReleaseGroupPage { + release_group_offset: de_release_group_offset, + release_group_count: de_release_group_count, + }, release_groups: Some(vec![de_meta.clone()]), }; let response = BrowseReleaseGroupResponse { - release_group_offset: de_release_group_offset, - release_group_count: de_release_group_count, + page: de_response.page, release_groups: vec![de_meta.clone().into()], }; diff --git a/src/external/musicbrainz/api/mod.rs b/src/external/musicbrainz/api/mod.rs index 34ab3cd..7faf9de 100644 --- a/src/external/musicbrainz/api/mod.rs +++ b/src/external/musicbrainz/api/mod.rs @@ -84,6 +84,22 @@ impl PageSettings { } } +pub enum NextPage { + Offset(usize), + Complete, +} + +impl NextPage { + 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 { + NextPage::Offset(next_offset) + } else { + NextPage::Complete + } + } +} + #[derive(Clone, Debug, PartialEq, Eq)] pub struct MbArtistMeta { pub id: Mbid, diff --git a/src/external/musicbrainz/api/search/artist.rs b/src/external/musicbrainz/api/search/artist.rs index 8eaa4ea..8399c4b 100644 --- a/src/external/musicbrainz/api/search/artist.rs +++ b/src/external/musicbrainz/api/search/artist.rs @@ -3,7 +3,10 @@ use std::fmt; use serde::Deserialize; use crate::external::musicbrainz::api::{ - search::query::{impl_term, EmptyQuery, EmptyQueryJoin, Query, QueryJoin}, + search::{ + query::{impl_term, EmptyQuery, EmptyQueryJoin, Query, QueryJoin}, + SearchPage, SerdeSearchPage, + }, MbArtistMeta, SerdeMbArtistMeta, }; @@ -26,18 +29,22 @@ impl_term!(string, SearchArtist<'a>, String, &'a str); #[derive(Debug, PartialEq, Eq)] pub struct SearchArtistResponse { pub artists: Vec, + pub page: SearchPage, } #[derive(Clone, Deserialize)] #[serde(rename_all(deserialize = "kebab-case"))] pub struct DeserializeSearchArtistResponse { artists: Vec, + #[serde(flatten)] + page: SerdeSearchPage, } impl From for SearchArtistResponse { fn from(value: DeserializeSearchArtistResponse) -> Self { SearchArtistResponse { artists: value.artists.into_iter().map(Into::into).collect(), + page: value.page, } } } @@ -77,6 +84,8 @@ mod tests { use super::*; fn de_response() -> DeserializeSearchArtistResponse { + let de_offset = 24; + let de_count = 124; let de_artist = DeserializeSearchArtistResponseArtist { score: 67, meta: SerdeMbArtistMeta { @@ -88,6 +97,10 @@ mod tests { }; DeserializeSearchArtistResponse { artists: vec![de_artist.clone()], + page: SerdeSearchPage { + offset: de_offset, + count: de_count, + }, } } @@ -101,6 +114,7 @@ mod tests { meta: a.meta.into(), }) .collect(), + page: de_response.page, } } diff --git a/src/external/musicbrainz/api/search/mod.rs b/src/external/musicbrainz/api/search/mod.rs index c4c1521..daefcba 100644 --- a/src/external/musicbrainz/api/search/mod.rs +++ b/src/external/musicbrainz/api/search/mod.rs @@ -9,6 +9,7 @@ pub use release_group::{ }; use paste::paste; +use serde::Deserialize; use url::form_urlencoded; use crate::external::musicbrainz::{ @@ -22,6 +23,23 @@ use crate::external::musicbrainz::{ IMusicBrainzHttp, }; +use super::NextPage; + +#[derive(Clone, Copy, Debug, Deserialize, PartialEq, Eq)] +#[serde(rename_all(deserialize = "kebab-case"))] +pub struct SearchPage { + pub offset: usize, + pub count: usize, +} + +impl SearchPage { + pub fn next_page_offset(&self, page_count: usize) -> NextPage { + NextPage::next_page_offset(self.offset, self.count, page_count) + } +} + +pub type SerdeSearchPage = SearchPage; + macro_rules! impl_search_entity { ($name:ident, $entity:literal) => { paste! { diff --git a/src/external/musicbrainz/api/search/release_group.rs b/src/external/musicbrainz/api/search/release_group.rs index be48a7f..ae36fe2 100644 --- a/src/external/musicbrainz/api/search/release_group.rs +++ b/src/external/musicbrainz/api/search/release_group.rs @@ -5,7 +5,10 @@ use serde::Deserialize; use crate::{ collection::{album::AlbumDate, musicbrainz::Mbid}, external::musicbrainz::api::{ - search::query::{impl_term, EmptyQuery, EmptyQueryJoin, Query, QueryJoin}, + search::{ + query::{impl_term, EmptyQuery, EmptyQueryJoin, Query, QueryJoin}, + SearchPage, SerdeSearchPage, + }, ApiDisplay, MbReleaseGroupMeta, SerdeMbReleaseGroupMeta, }, }; @@ -50,18 +53,22 @@ impl_term!(rgid, SearchReleaseGroup<'a>, Rgid, &'a Mbid); #[derive(Debug, PartialEq, Eq)] pub struct SearchReleaseGroupResponse { pub release_groups: Vec, + pub page: SearchPage, } #[derive(Clone, Deserialize)] #[serde(rename_all(deserialize = "kebab-case"))] pub struct DeserializeSearchReleaseGroupResponse { release_groups: Vec, + #[serde(flatten)] + page: SerdeSearchPage, } impl From for SearchReleaseGroupResponse { fn from(value: DeserializeSearchReleaseGroupResponse) -> Self { SearchReleaseGroupResponse { release_groups: value.release_groups.into_iter().map(Into::into).collect(), + page: value.page, } } } @@ -109,6 +116,8 @@ mod tests { use super::*; fn de_response() -> DeserializeSearchReleaseGroupResponse { + let de_offset = 26; + let de_count = 126; let de_release_group = DeserializeSearchReleaseGroupResponseReleaseGroup { score: 67, meta: SerdeMbReleaseGroupMeta { @@ -121,6 +130,10 @@ mod tests { }; DeserializeSearchReleaseGroupResponse { release_groups: vec![de_release_group.clone()], + page: SerdeSearchPage { + offset: de_offset, + count: de_count, + }, } } @@ -134,6 +147,7 @@ mod tests { meta: rg.meta.into(), }) .collect(), + page: de_response.page, } } diff --git a/src/tui/lib/external/musicbrainz/api/mod.rs b/src/tui/lib/external/musicbrainz/api/mod.rs index 3e4c0cb..35156c5 100644 --- a/src/tui/lib/external/musicbrainz/api/mod.rs +++ b/src/tui/lib/external/musicbrainz/api/mod.rs @@ -57,7 +57,7 @@ impl IMusicBrainz for MusicBrainz { fn search_artist(&mut self, artist: &ArtistMeta) -> Result>, Error> { let query = SearchArtistRequest::new().string(&artist.id.name); - let paging = PageSettings::with_max_limit(); + let paging = PageSettings::default(); let mb_response = self.client.search_artist(&query, &paging)?; Ok(mb_response @@ -83,7 +83,7 @@ impl IMusicBrainz for MusicBrainz { .and() .release_group(&album.id.title); - let paging = PageSettings::with_max_limit(); + let paging = PageSettings::default(); let mb_response = self.client.search_release_group(&query, &paging)?; Ok(mb_response -- 2.45.2 From 66b273bef4303f64f0647d15b96e30e05dd19ada Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 29 Sep 2024 21:17:17 +0200 Subject: [PATCH 7/9] Paging of match page --- src/tui/app/machine/browse_state.rs | 4 +-- src/tui/app/machine/match_state.rs | 40 ++++++++++++++-------------- src/tui/app/mod.rs | 41 ++++++++++++++++++++++++++--- src/tui/app/selection/album.rs | 8 +++--- src/tui/app/selection/artist.rs | 8 +++--- src/tui/app/selection/mod.rs | 39 +++------------------------ src/tui/app/selection/track.rs | 2 +- src/tui/handler.rs | 6 +++-- 8 files changed, 79 insertions(+), 69 deletions(-) diff --git a/src/tui/app/machine/browse_state.rs b/src/tui/app/machine/browse_state.rs index a215740..95c15e0 100644 --- a/src/tui/app/machine/browse_state.rs +++ b/src/tui/app/machine/browse_state.rs @@ -1,7 +1,7 @@ use crate::tui::app::{ machine::{App, AppInner, AppMachine}, - selection::{Delta, ListSelection}, - AppPublicState, AppState, IAppInteractBrowse, + selection::ListSelection, + AppPublicState, AppState, IAppInteractBrowse, Delta }; pub struct BrowseState; diff --git a/src/tui/app/machine/match_state.rs b/src/tui/app/machine/match_state.rs index de68e30..19e0b3c 100644 --- a/src/tui/app/machine/match_state.rs +++ b/src/tui/app/machine/match_state.rs @@ -9,8 +9,8 @@ use musichoard::collection::{ use crate::tui::{ app::{ machine::{fetch_state::FetchState, input::Input, App, AppInner, AppMachine}, - AlbumMatches, AppPublicState, AppState, ArtistMatches, IAppInteractMatch, ListOption, - MatchOption, MatchStateInfo, MatchStatePublic, WidgetState, + AlbumMatches, AppPublicState, AppState, ArtistMatches, Delta, IAppInteractMatch, + ListOption, MatchOption, MatchStateInfo, MatchStatePublic, WidgetState, }, lib::interface::musicbrainz::api::{Lookup, Match}, }; @@ -243,19 +243,19 @@ impl<'a> From<&'a mut MatchState> for AppPublicState<'a> { impl IAppInteractMatch for AppMachine { type APP = App; - fn prev_match(mut self) -> Self::APP { + fn decrement_match(mut self, delta: Delta) -> Self::APP { if let Some(index) = self.state.state.list.selected() { - let result = index.saturating_sub(1); + let result = index.saturating_sub(delta.as_usize(&self.state.state)); self.state.state.list.select(Some(result)); } self.into() } - fn next_match(mut self) -> Self::APP { + fn increment_match(mut self, delta: Delta) -> Self::APP { let index = self.state.state.list.selected().unwrap(); let to = cmp::min( - index.saturating_add(1), + index.saturating_add(delta.as_usize(&self.state.state)), self.state.current.len().saturating_sub(1), ); self.state.state.list.select(Some(to)); @@ -473,38 +473,38 @@ mod tests { assert_eq!(matches.state.current, matches_info); assert_eq!(matches.state.state, widget_state); - let matches = matches.prev_match().unwrap_match(); + let matches = matches.decrement_match(Delta::Line).unwrap_match(); assert_eq!(matches.state.current, matches_info); assert_eq!(matches.state.state.list.selected(), Some(0)); let mut matches = matches; for ii in 1..len { - matches = matches.next_match().unwrap_match(); + matches = matches.increment_match(Delta::Line).unwrap_match(); assert_eq!(matches.state.current, matches_info); assert_eq!(matches.state.state.list.selected(), Some(ii)); } // Next is CannotHaveMBID - let matches = matches.next_match().unwrap_match(); + let matches = matches.increment_match(Delta::Line).unwrap_match(); assert_eq!(matches.state.current, matches_info); assert_eq!(matches.state.state.list.selected(), Some(len)); // Next is ManualInputMbid - let matches = matches.next_match().unwrap_match(); + let matches = matches.increment_match(Delta::Line).unwrap_match(); assert_eq!(matches.state.current, matches_info); assert_eq!(matches.state.state.list.selected(), Some(len + 1)); - let matches = matches.next_match().unwrap_match(); + let matches = matches.increment_match(Delta::Line).unwrap_match(); assert_eq!(matches.state.current, matches_info); assert_eq!(matches.state.state.list.selected(), Some(len + 1)); // Go prev_match first as selecting on manual input does not go back to fetch. - let matches = matches.prev_match().unwrap_match(); + let matches = matches.decrement_match(Delta::Line).unwrap_match(); matches.select().unwrap_fetch(); } @@ -619,10 +619,10 @@ mod tests { AppMachine::match_state(inner(music_hoard(vec![])), match_state(album_match())); // album_match has two matches which means that the fourth option should be manual input. - let matches = matches.next_match().unwrap_match(); - let matches = matches.next_match().unwrap_match(); - let matches = matches.next_match().unwrap_match(); - let matches = matches.next_match().unwrap_match(); + let matches = matches.increment_match(Delta::Line).unwrap_match(); + let matches = matches.increment_match(Delta::Line).unwrap_match(); + let matches = matches.increment_match(Delta::Line).unwrap_match(); + let matches = matches.increment_match(Delta::Line).unwrap_match(); let app = matches.select(); @@ -657,8 +657,8 @@ mod tests { ); // There are no matches which means that the second option should be manual input. - let matches = matches.next_match().unwrap_match(); - let matches = matches.next_match().unwrap_match(); + let matches = matches.increment_match(Delta::Line).unwrap_match(); + let matches = matches.increment_match(Delta::Line).unwrap_match(); let mut app = matches.select(); app = input_mbid(app); @@ -691,8 +691,8 @@ mod tests { ); // There are no matches which means that the second option should be manual input. - let matches = matches.next_match().unwrap_match(); - let matches = matches.next_match().unwrap_match(); + let matches = matches.increment_match(Delta::Line).unwrap_match(); + let matches = matches.increment_match(Delta::Line).unwrap_match(); let mut app = matches.select(); app = input_mbid(app); diff --git a/src/tui/app/mod.rs b/src/tui/app/mod.rs index a44cc28..e7bd921 100644 --- a/src/tui/app/mod.rs +++ b/src/tui/app/mod.rs @@ -2,7 +2,8 @@ mod machine; mod selection; pub use machine::App; -pub use selection::{Category, Delta, Selection, WidgetState}; +use ratatui::widgets::ListState; +pub use selection::{Category, Selection}; use musichoard::collection::{ album::AlbumMeta, @@ -124,8 +125,8 @@ pub trait IAppEventFetch { pub trait IAppInteractMatch { type APP: IApp; - fn prev_match(self) -> Self::APP; - fn next_match(self) -> Self::APP; + fn decrement_match(self, delta: Delta) -> Self::APP; + fn increment_match(self, delta: Delta) -> Self::APP; fn select(self) -> Self::APP; fn abort(self) -> Self::APP; @@ -159,6 +160,40 @@ pub trait IAppInteractError { fn dismiss_error(self) -> Self::APP; } +#[derive(Clone, Debug, Default)] +pub struct WidgetState { + pub list: ListState, + pub height: usize, +} + +impl PartialEq for WidgetState { + fn eq(&self, other: &Self) -> bool { + self.list.selected().eq(&other.list.selected()) && self.height.eq(&other.height) + } +} + +impl WidgetState { + #[must_use] + pub const fn with_selected(mut self, selected: Option) -> Self { + self.list = self.list.with_selected(selected); + self + } +} + +pub enum Delta { + Line, + Page, +} + +impl Delta { + fn as_usize(&self, state: &WidgetState) -> usize { + match self { + Delta::Line => 1, + Delta::Page => state.height.saturating_sub(1), + } + } +} + // It would be preferable to have a getter for each field separately. However, the selection field // needs to be mutably accessible requiring a mutable borrow of the entire struct if behind a trait. // This in turn complicates simultaneous field access since only a single mutable borrow is allowed. diff --git a/src/tui/app/selection/album.rs b/src/tui/app/selection/album.rs index 2f89967..15cd1da 100644 --- a/src/tui/app/selection/album.rs +++ b/src/tui/app/selection/album.rs @@ -5,9 +5,11 @@ use musichoard::collection::{ track::Track, }; -use crate::tui::app::selection::{ - track::{KeySelectTrack, TrackSelection}, - Delta, SelectionState, WidgetState, +use crate::tui::app::{ + selection::{ + track::{KeySelectTrack, TrackSelection}, + SelectionState, + }, Delta, WidgetState }; #[derive(Clone, Debug, PartialEq)] diff --git a/src/tui/app/selection/artist.rs b/src/tui/app/selection/artist.rs index 045455a..110bd66 100644 --- a/src/tui/app/selection/artist.rs +++ b/src/tui/app/selection/artist.rs @@ -6,9 +6,11 @@ use musichoard::collection::{ track::Track, }; -use crate::tui::app::selection::{ - album::{AlbumSelection, KeySelectAlbum}, - Delta, SelectionState, WidgetState, +use crate::tui::app::{ + selection::{ + album::{AlbumSelection, KeySelectAlbum}, + SelectionState, + }, Delta, WidgetState }; #[derive(Clone, Debug, PartialEq)] diff --git a/src/tui/app/selection/mod.rs b/src/tui/app/selection/mod.rs index da31713..947db24 100644 --- a/src/tui/app/selection/mod.rs +++ b/src/tui/app/selection/mod.rs @@ -5,7 +5,10 @@ mod track; use musichoard::collection::{album::Album, artist::Artist, track::Track, Collection}; use ratatui::widgets::ListState; -use artist::{ArtistSelection, KeySelectArtist}; +use crate::tui::app::{ + selection::artist::{ArtistSelection, KeySelectArtist}, + Delta, WidgetState, +}; #[derive(Copy, Clone, Debug, PartialEq, Eq)] pub enum Category { @@ -24,40 +27,6 @@ pub struct SelectionState<'a, T> { pub index: usize, } -#[derive(Clone, Debug, Default)] -pub struct WidgetState { - pub list: ListState, - pub height: usize, -} - -impl PartialEq for WidgetState { - fn eq(&self, other: &Self) -> bool { - self.list.selected().eq(&other.list.selected()) && self.height.eq(&other.height) - } -} - -impl WidgetState { - #[must_use] - pub const fn with_selected(mut self, selected: Option) -> Self { - self.list = self.list.with_selected(selected); - self - } -} - -pub enum Delta { - Line, - Page, -} - -impl Delta { - fn as_usize(&self, state: &WidgetState) -> usize { - match self { - Delta::Line => 1, - Delta::Page => state.height.saturating_sub(1), - } - } -} - impl Selection { pub fn new(artists: &[Artist]) -> Self { Selection { diff --git a/src/tui/app/selection/track.rs b/src/tui/app/selection/track.rs index adec55b..1fa18d5 100644 --- a/src/tui/app/selection/track.rs +++ b/src/tui/app/selection/track.rs @@ -2,7 +2,7 @@ use std::cmp; use musichoard::collection::track::{Track, TrackId, TrackNum}; -use crate::tui::app::selection::{Delta, SelectionState, WidgetState}; +use crate::tui::app::{selection::SelectionState, Delta, WidgetState}; #[derive(Clone, Debug, PartialEq)] pub struct TrackSelection { diff --git a/src/tui/handler.rs b/src/tui/handler.rs index cf04371..f046da3 100644 --- a/src/tui/handler.rs +++ b/src/tui/handler.rs @@ -212,8 +212,10 @@ impl IEventHandlerPrivate for EventHandler { // Abort. KeyCode::Esc | KeyCode::Char('q') | KeyCode::Char('Q') => app.abort(), // Select. - KeyCode::Up => app.prev_match(), - KeyCode::Down => app.next_match(), + KeyCode::Up => app.decrement_match(Delta::Line), + KeyCode::Down => app.increment_match(Delta::Line), + KeyCode::PageUp => app.decrement_match(Delta::Page), + KeyCode::PageDown => app.increment_match(Delta::Page), KeyCode::Enter => app.select(), // Othey keys. _ => app.no_op(), -- 2.45.2 From 03c1b2b9ae2c605820ab73606c59767bbc08a75d Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 29 Sep 2024 21:29:20 +0200 Subject: [PATCH 8/9] Complete unit tests --- src/external/musicbrainz/api/browse.rs | 14 ++++++++++++-- src/external/musicbrainz/api/mod.rs | 20 ++++++++++++++++++++ src/external/musicbrainz/api/search/mod.rs | 17 +++++++++++++++++ 3 files changed, 49 insertions(+), 2 deletions(-) diff --git a/src/external/musicbrainz/api/browse.rs b/src/external/musicbrainz/api/browse.rs index e6299b0..9d1a21d 100644 --- a/src/external/musicbrainz/api/browse.rs +++ b/src/external/musicbrainz/api/browse.rs @@ -109,8 +109,8 @@ mod tests { collection::album::{AlbumPrimaryType, AlbumSecondaryType}, external::musicbrainz::{ api::{ - SerdeAlbumDate, SerdeAlbumPrimaryType, SerdeAlbumSecondaryType, SerdeMbid, - MB_MAX_PAGE_LIMIT, + tests::next_page_test, SerdeAlbumDate, SerdeAlbumPrimaryType, + SerdeAlbumSecondaryType, SerdeMbid, MB_MAX_PAGE_LIMIT, }, MockIMusicBrainzHttp, }, @@ -118,6 +118,16 @@ mod tests { use super::*; + #[test] + fn browse_release_group_next_page() { + let page = BrowseReleaseGroupPage { + release_group_offset: 5, + release_group_count: 45, + }; + + next_page_test(|val| page.next_page_offset(val)); + } + #[test] fn browse_release_group() { let mbid = "00000000-0000-0000-0000-000000000000"; diff --git a/src/external/musicbrainz/api/mod.rs b/src/external/musicbrainz/api/mod.rs index 7faf9de..2f61f75 100644 --- a/src/external/musicbrainz/api/mod.rs +++ b/src/external/musicbrainz/api/mod.rs @@ -84,6 +84,7 @@ impl PageSettings { } } +#[derive(Debug, PartialEq, Eq)] pub enum NextPage { Offset(usize), Complete, @@ -358,6 +359,25 @@ mod tests { assert!(!format!("{unk_err:?}").is_empty()); } + pub fn next_page_test(mut f: Fn) + where + Fn: FnMut(usize) -> NextPage, + { + let next = f(20); + assert_eq!(next, NextPage::Offset(25)); + + let next = f(40); + assert_eq!(next, NextPage::Complete); + + let next = f(100); + assert_eq!(next, NextPage::Complete); + } + + #[test] + fn next_page() { + next_page_test(|val| NextPage::next_page_offset(5, 45, val)); + } + #[test] fn format_page_settings() { let paging = PageSettings::default(); diff --git a/src/external/musicbrainz/api/search/mod.rs b/src/external/musicbrainz/api/search/mod.rs index daefcba..6fcb4cc 100644 --- a/src/external/musicbrainz/api/search/mod.rs +++ b/src/external/musicbrainz/api/search/mod.rs @@ -64,3 +64,20 @@ impl MusicBrainzClient { impl_search_entity!(Artist, "artist"); impl_search_entity!(ReleaseGroup, "release-group"); } + +#[cfg(test)] +mod tests { + use crate::external::musicbrainz::api::tests::next_page_test; + + use super::*; + + #[test] + fn search_next_page() { + let page = SearchPage { + offset: 5, + count: 45, + }; + + next_page_test(|val| page.next_page_offset(val)); + } +} -- 2.45.2 From a8443c378c0eb2160c96eaf1a4b2665749530f12 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 29 Sep 2024 21:29:52 +0200 Subject: [PATCH 9/9] Lints --- src/tui/app/machine/browse_state.rs | 2 +- src/tui/app/selection/album.rs | 3 ++- src/tui/app/selection/artist.rs | 3 ++- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/tui/app/machine/browse_state.rs b/src/tui/app/machine/browse_state.rs index 95c15e0..00e9cca 100644 --- a/src/tui/app/machine/browse_state.rs +++ b/src/tui/app/machine/browse_state.rs @@ -1,7 +1,7 @@ use crate::tui::app::{ machine::{App, AppInner, AppMachine}, selection::ListSelection, - AppPublicState, AppState, IAppInteractBrowse, Delta + AppPublicState, AppState, Delta, IAppInteractBrowse, }; pub struct BrowseState; diff --git a/src/tui/app/selection/album.rs b/src/tui/app/selection/album.rs index 15cd1da..d4e8f8d 100644 --- a/src/tui/app/selection/album.rs +++ b/src/tui/app/selection/album.rs @@ -9,7 +9,8 @@ use crate::tui::app::{ selection::{ track::{KeySelectTrack, TrackSelection}, SelectionState, - }, Delta, WidgetState + }, + Delta, WidgetState, }; #[derive(Clone, Debug, PartialEq)] diff --git a/src/tui/app/selection/artist.rs b/src/tui/app/selection/artist.rs index 110bd66..851bd52 100644 --- a/src/tui/app/selection/artist.rs +++ b/src/tui/app/selection/artist.rs @@ -10,7 +10,8 @@ use crate::tui::app::{ selection::{ album::{AlbumSelection, KeySelectAlbum}, SelectionState, - }, Delta, WidgetState + }, + Delta, WidgetState, }; #[derive(Clone, Debug, PartialEq)] -- 2.45.2