From 7ebe17f0b0575226d9ab16697c17c1ada5a42a9b Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Fri, 3 Jan 2025 21:00:21 +0100 Subject: [PATCH] Ignore bootleg release groups (#247) Closes #239 Reviewed-on: https://git.thenineworlds.net/wojtek/musichoard/pulls/247 --- src/core/collection/album.rs | 14 ++-- src/external/musicbrainz/api/browse.rs | 86 +++++++++++++++++++-- src/tui/lib/external/musicbrainz/api/mod.rs | 2 +- src/tui/ui/browse_state.rs | 14 ++-- src/tui/ui/display.rs | 62 ++++++++------- 5 files changed, 128 insertions(+), 50 deletions(-) diff --git a/src/core/collection/album.rs b/src/core/collection/album.rs index f27192a..308e901 100644 --- a/src/core/collection/album.rs +++ b/src/core/collection/album.rs @@ -138,16 +138,16 @@ pub enum AlbumSecondaryType { } /// The album's ownership status. -pub enum AlbumStatus { +pub enum AlbumOwnership { None, Owned(TrackFormat), } -impl AlbumStatus { - pub fn from_tracks(tracks: &[Track]) -> AlbumStatus { +impl AlbumOwnership { + pub fn from_tracks(tracks: &[Track]) -> AlbumOwnership { match tracks.iter().map(|t| t.quality.format).min() { - Some(format) => AlbumStatus::Owned(format), - None => AlbumStatus::None, + Some(format) => AlbumOwnership::Owned(format), + None => AlbumOwnership::None, } } } @@ -165,8 +165,8 @@ impl Album { self } - pub fn get_status(&self) -> AlbumStatus { - AlbumStatus::from_tracks(&self.tracks) + pub fn get_ownership(&self) -> AlbumOwnership { + AlbumOwnership::from_tracks(&self.tracks) } } diff --git a/src/external/musicbrainz/api/browse.rs b/src/external/musicbrainz/api/browse.rs index c62dc14..2a3125d 100644 --- a/src/external/musicbrainz/api/browse.rs +++ b/src/external/musicbrainz/api/browse.rs @@ -33,17 +33,28 @@ impl BrowseReleaseGroupPage { pub type SerdeBrowseReleaseGroupPage = BrowseReleaseGroupPage; impl MusicBrainzClient { + fn browse_release_group_url( + request: &BrowseReleaseGroupRequest, + paging: &PageSettings, + ) -> String { + let entity = &request.entity; + let mbid = request.mbid.uuid().as_hyphenated(); + let status = request + .release_group_status + .as_ref() + .map(|s| format!("&release-group-status={s}")) + .unwrap_or_default(); + let page = ApiDisplay::format_page_settings(paging); + + format!("{MB_BASE_URL}/release-group?{entity}={mbid}{status}{page}") + } + pub fn browse_release_group( &mut self, request: &BrowseReleaseGroupRequest, paging: &PageSettings, ) -> Result { - let entity = &request.entity; - let mbid = request.mbid.uuid().as_hyphenated(); - let page = ApiDisplay::format_page_settings(paging); - - let url = format!("{MB_BASE_URL}/release-group?{entity}={mbid}{page}"); - + let url = Self::browse_release_group_url(request, paging); let response: DeserializeBrowseReleaseGroupResponse = self.http.get(&url)?; Ok(response.into()) } @@ -52,6 +63,7 @@ impl MusicBrainzClient { pub struct BrowseReleaseGroupRequest<'a> { entity: BrowseReleaseGroupRequestEntity, mbid: &'a Mbid, + release_group_status: Option, } enum BrowseReleaseGroupRequestEntity { @@ -61,7 +73,21 @@ enum BrowseReleaseGroupRequestEntity { impl fmt::Display for BrowseReleaseGroupRequestEntity { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - BrowseReleaseGroupRequestEntity::Artist => write!(f, "artist"), + Self::Artist => write!(f, "artist"), + } + } +} + +enum BrowseReleaseGroupRequestReleaseGroupStatus { + WebsiteDefault, + All, +} + +impl fmt::Display for BrowseReleaseGroupRequestReleaseGroupStatus { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::WebsiteDefault => write!(f, "website-default"), + Self::All => write!(f, "all"), } } } @@ -71,8 +97,20 @@ impl<'a> BrowseReleaseGroupRequest<'a> { BrowseReleaseGroupRequest { entity: BrowseReleaseGroupRequestEntity::Artist, mbid, + release_group_status: None, } } + + pub fn filter_status_website_default(mut self) -> Self { + self.release_group_status = + Some(BrowseReleaseGroupRequestReleaseGroupStatus::WebsiteDefault); + self + } + + pub fn filter_status_all(mut self) -> Self { + self.release_group_status = Some(BrowseReleaseGroupRequestReleaseGroupStatus::All); + self + } } #[derive(Debug, PartialEq, Eq)] @@ -175,4 +213,38 @@ mod tests { let result = client.browse_release_group(&request, &paging).unwrap(); assert_eq!(result, response); } + + #[test] + fn browse_release_group_filter_status() { + type Client = MusicBrainzClient; + let mbid_str = "00000000-0000-0000-0000-000000000000"; + let mbid: Mbid = mbid_str.try_into().unwrap(); + let paging = PageSettings::with_max_limit(); + + let request = BrowseReleaseGroupRequest::artist(&mbid); + let url = format!( + "https://musicbrainz.org/ws/2/release-group\ + ?artist={mbid_str}\ + &limit={MB_MAX_PAGE_LIMIT}", + ); + assert_eq!(Client::browse_release_group_url(&request, &paging), url); + + let request = BrowseReleaseGroupRequest::artist(&mbid).filter_status_website_default(); + let url = format!( + "https://musicbrainz.org/ws/2/release-group\ + ?artist={mbid_str}\ + &release-group-status=website-default\ + &limit={MB_MAX_PAGE_LIMIT}", + ); + assert_eq!(Client::browse_release_group_url(&request, &paging), url); + + let request = BrowseReleaseGroupRequest::artist(&mbid).filter_status_all(); + let url = format!( + "https://musicbrainz.org/ws/2/release-group\ + ?artist={mbid_str}\ + &release-group-status=all\ + &limit={MB_MAX_PAGE_LIMIT}", + ); + assert_eq!(Client::browse_release_group_url(&request, &paging), url); + } } diff --git a/src/tui/lib/external/musicbrainz/api/mod.rs b/src/tui/lib/external/musicbrainz/api/mod.rs index f7e893c..d648b8d 100644 --- a/src/tui/lib/external/musicbrainz/api/mod.rs +++ b/src/tui/lib/external/musicbrainz/api/mod.rs @@ -99,7 +99,7 @@ impl IMusicBrainz for MusicBrainz { artist: &Mbid, paging: &mut Option, ) -> Result>, Error> { - let request = BrowseReleaseGroupRequest::artist(artist); + let request = BrowseReleaseGroupRequest::artist(artist).filter_status_website_default(); let page = paging.take().unwrap_or_default(); let mb_response = self.client.browse_release_group(&request, &page)?; diff --git a/src/tui/ui/browse_state.rs b/src/tui/ui/browse_state.rs index 72254ff..a99e195 100644 --- a/src/tui/ui/browse_state.rs +++ b/src/tui/ui/browse_state.rs @@ -1,5 +1,5 @@ use musichoard::collection::{ - album::{Album, AlbumStatus}, + album::{Album, AlbumOwnership}, artist::Artist, track::{Track, TrackFormat}, }; @@ -163,19 +163,19 @@ impl<'a, 'b> AlbumState<'a, 'b> { "Title: {}\n\ Date: {}\n\ Type: {}\n\ - Status: {}", + Ownership: {}", album.map(|a| a.meta.id.title.as_str()).unwrap_or(""), album .map(|a| UiDisplay::display_date(&a.meta.date, &a.meta.seq)) .unwrap_or_default(), album - .map(|a| UiDisplay::display_type( + .map(|a| UiDisplay::display_album_type( &a.meta.info.primary_type, &a.meta.info.secondary_types )) .unwrap_or_default(), album - .map(|a| UiDisplay::display_album_status(&a.get_status())) + .map(|a| UiDisplay::display_album_ownership(&a.get_ownership())) .unwrap_or("") )); @@ -188,9 +188,9 @@ impl<'a, 'b> AlbumState<'a, 'b> { } fn to_list_item(album: &Album) -> ListItem { - let line = match album.get_status() { - AlbumStatus::None => Line::raw(album.meta.id.title.as_str()), - AlbumStatus::Owned(format) => match format { + let line = match album.get_ownership() { + AlbumOwnership::None => Line::raw(album.meta.id.title.as_str()), + AlbumOwnership::Owned(format) => match format { TrackFormat::Mp3 => Line::styled(album.meta.id.title.as_str(), UiColor::FG_WARN), TrackFormat::Flac => Line::styled(album.meta.id.title.as_str(), UiColor::FG_GOOD), }, diff --git a/src/tui/ui/display.rs b/src/tui/ui/display.rs index 68c2281..1c91792 100644 --- a/src/tui/ui/display.rs +++ b/src/tui/ui/display.rs @@ -1,7 +1,7 @@ use musichoard::collection::{ album::{ - AlbumDate, AlbumId, AlbumLibId, AlbumMeta, AlbumPrimaryType, AlbumSecondaryType, AlbumSeq, - AlbumStatus, + AlbumDate, AlbumId, AlbumLibId, AlbumMeta, AlbumOwnership, AlbumPrimaryType, + AlbumSecondaryType, AlbumSeq, }, artist::ArtistMeta, musicbrainz::{IMusicBrainzRef, MbRefOption}, @@ -50,19 +50,19 @@ impl UiDisplay { } } - pub fn display_type( + pub fn display_album_type( primary: &Option, secondary: &Vec, ) -> String { match primary { Some(ref primary) => { if secondary.is_empty() { - Self::display_primary_type(primary).to_string() + Self::display_album_primary_type(primary).to_string() } else { format!( "{} ({})", - Self::display_primary_type(primary), - Self::display_secondary_types(secondary) + Self::display_album_primary_type(primary), + Self::display_album_secondary_types(secondary) ) } } @@ -70,7 +70,7 @@ impl UiDisplay { } } - pub fn display_primary_type(value: &AlbumPrimaryType) -> &'static str { + pub fn display_album_primary_type(value: &AlbumPrimaryType) -> &'static str { match value { AlbumPrimaryType::Album => "Album", AlbumPrimaryType::Single => "Single", @@ -80,7 +80,7 @@ impl UiDisplay { } } - pub fn display_secondary_types(values: &Vec) -> String { + pub fn display_album_secondary_types(values: &Vec) -> String { let mut types: Vec<&'static str> = vec![]; for value in values { match value { @@ -101,10 +101,10 @@ impl UiDisplay { types.join(", ") } - pub fn display_album_status(status: &AlbumStatus) -> &'static str { + pub fn display_album_ownership(status: &AlbumOwnership) -> &'static str { match status { - AlbumStatus::None => "None", - AlbumStatus::Owned(format) => match format { + AlbumOwnership::None => "None", + AlbumOwnership::Owned(format) => match format { TrackFormat::Mp3 => "MP3", TrackFormat::Flac => "FLAC", }, @@ -173,7 +173,7 @@ impl UiDisplay { "{:010} | {} [{}]", UiDisplay::display_album_date(&album.date), album.id.title, - UiDisplay::display_type(&album.info.primary_type, &album.info.secondary_types), + UiDisplay::display_album_type(&album.info.primary_type, &album.info.secondary_types), ) } @@ -224,30 +224,33 @@ mod tests { } #[test] - fn display_primary_type() { + fn display_album_primary_type() { assert_eq!( - UiDisplay::display_primary_type(&AlbumPrimaryType::Album), + UiDisplay::display_album_primary_type(&AlbumPrimaryType::Album), "Album" ); assert_eq!( - UiDisplay::display_primary_type(&AlbumPrimaryType::Single), + UiDisplay::display_album_primary_type(&AlbumPrimaryType::Single), "Single" ); - assert_eq!(UiDisplay::display_primary_type(&AlbumPrimaryType::Ep), "EP"); assert_eq!( - UiDisplay::display_primary_type(&AlbumPrimaryType::Broadcast), + UiDisplay::display_album_primary_type(&AlbumPrimaryType::Ep), + "EP" + ); + assert_eq!( + UiDisplay::display_album_primary_type(&AlbumPrimaryType::Broadcast), "Broadcast" ); assert_eq!( - UiDisplay::display_primary_type(&AlbumPrimaryType::Other), + UiDisplay::display_album_primary_type(&AlbumPrimaryType::Other), "Other" ); } #[test] - fn display_secondary_types() { + fn display_album_secondary_types() { assert_eq!( - UiDisplay::display_secondary_types(&vec![ + UiDisplay::display_album_secondary_types(&vec![ AlbumSecondaryType::Compilation, AlbumSecondaryType::Soundtrack, AlbumSecondaryType::Spokenword, @@ -267,14 +270,14 @@ mod tests { } #[test] - fn display_type() { - assert_eq!(UiDisplay::display_type(&None, &vec![]), ""); + fn display_album_type() { + assert_eq!(UiDisplay::display_album_type(&None, &vec![]), ""); assert_eq!( - UiDisplay::display_type(&Some(AlbumPrimaryType::Album), &vec![]), + UiDisplay::display_album_type(&Some(AlbumPrimaryType::Album), &vec![]), "Album" ); assert_eq!( - UiDisplay::display_type( + UiDisplay::display_album_type( &Some(AlbumPrimaryType::Album), &vec![AlbumSecondaryType::Live, AlbumSecondaryType::Compilation] ), @@ -283,14 +286,17 @@ mod tests { } #[test] - fn display_album_status() { - assert_eq!(UiDisplay::display_album_status(&AlbumStatus::None), "None"); + fn display_album_ownership() { assert_eq!( - UiDisplay::display_album_status(&AlbumStatus::Owned(TrackFormat::Mp3)), + UiDisplay::display_album_ownership(&AlbumOwnership::None), + "None" + ); + assert_eq!( + UiDisplay::display_album_ownership(&AlbumOwnership::Owned(TrackFormat::Mp3)), "MP3" ); assert_eq!( - UiDisplay::display_album_status(&AlbumStatus::Owned(TrackFormat::Flac)), + UiDisplay::display_album_ownership(&AlbumOwnership::Owned(TrackFormat::Flac)), "FLAC" ); }