From c7471bda0e1ed2136e84fae064cab055c0775e81 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sat, 31 Aug 2024 21:26:39 +0200 Subject: [PATCH 1/6] Split artist/album metadata from collection --- src/core/collection/album.rs | 70 +++-- src/core/collection/artist.rs | 134 +++++---- src/core/musichoard/base.rs | 22 +- src/core/musichoard/database.rs | 40 +-- src/core/musichoard/library.rs | 22 +- src/core/testmod.rs | 4 +- src/external/database/json/mod.rs | 2 +- src/external/database/serde/deserialize.rs | 26 +- src/external/database/serde/serialize.rs | 14 +- src/testmod/full.rs | 280 ++++++++++--------- src/testmod/library.rs | 212 ++++++++------ src/tui/app/machine/browse.rs | 4 +- src/tui/app/machine/matches.rs | 4 +- src/tui/app/machine/search.rs | 6 +- src/tui/lib/external/musicbrainz/mod.rs | 6 +- src/tui/testmod.rs | 4 +- src/tui/ui/browse.rs | 14 +- src/tui/ui/display.rs | 16 +- src/tui/ui/info.rs | 10 +- tests/database/json.rs | 2 +- tests/testlib.rs | 308 +++++++++++---------- 21 files changed, 658 insertions(+), 542 deletions(-) diff --git a/src/core/collection/album.rs b/src/core/collection/album.rs index e272894..ab2403e 100644 --- a/src/core/collection/album.rs +++ b/src/core/collection/album.rs @@ -12,20 +12,26 @@ use crate::core::collection::{ /// An album is a collection of tracks that were released together. #[derive(Clone, Debug, PartialEq, Eq)] pub struct Album { + pub meta: AlbumMeta, + pub tracks: Vec, +} + +/// Album metadata. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct AlbumMeta { pub id: AlbumId, pub date: AlbumDate, pub seq: AlbumSeq, pub musicbrainz: Option, pub primary_type: Option, pub secondary_types: Vec, - pub tracks: Vec, } impl WithId for Album { type Id = AlbumId; fn id(&self) -> &Self::Id { - &self.id + &self.meta.id } } @@ -139,18 +145,20 @@ impl Album { secondary_types: Vec, ) -> Self { Album { - id: id.into(), - date: date.into(), - seq: AlbumSeq::default(), - musicbrainz: None, - primary_type, - secondary_types, + meta: AlbumMeta { + id: id.into(), + date: date.into(), + seq: AlbumSeq::default(), + musicbrainz: None, + primary_type, + secondary_types, + }, tracks: vec![], } } pub fn get_sort_key(&self) -> (&AlbumDate, &AlbumSeq, &AlbumId) { - (&self.date, &self.seq, &self.id) + (&self.meta.date, &self.meta.seq, &self.meta.id) } pub fn get_status(&self) -> AlbumStatus { @@ -158,19 +166,19 @@ impl Album { } pub fn set_seq(&mut self, seq: AlbumSeq) { - self.seq = seq; + self.meta.seq = seq; } pub fn clear_seq(&mut self) { - self.seq = AlbumSeq::default(); + self.meta.seq = AlbumSeq::default(); } pub fn set_musicbrainz_ref(&mut self, mbref: MbAlbumRef) { - _ = self.musicbrainz.insert(mbref); + _ = self.meta.musicbrainz.insert(mbref); } pub fn clear_musicbrainz_ref(&mut self) { - _ = self.musicbrainz.take(); + _ = self.meta.musicbrainz.take(); } } @@ -188,12 +196,14 @@ impl Ord for Album { impl Merge for Album { fn merge_in_place(&mut self, other: Self) { - assert_eq!(self.id, other.id); - self.seq = std::cmp::max(self.seq, other.seq); + assert_eq!(self.meta.id, other.meta.id); + self.meta.seq = std::cmp::max(self.meta.seq, other.meta.seq); - self.musicbrainz = self.musicbrainz.take().or(other.musicbrainz); - self.primary_type = self.primary_type.take().or(other.primary_type); - self.secondary_types.merge_in_place(other.secondary_types); + self.meta.musicbrainz = self.meta.musicbrainz.take().or(other.meta.musicbrainz); + self.meta.primary_type = self.meta.primary_type.take().or(other.meta.primary_type); + self.meta + .secondary_types + .merge_in_place(other.meta.secondary_types); let tracks = mem::take(&mut self.tracks); self.tracks = MergeSorted::new(tracks.into_iter(), other.tracks.into_iter()).collect(); @@ -266,28 +276,28 @@ mod tests { fn set_clear_seq() { let mut album = Album::new("An album", AlbumDate::default(), None, vec![]); - assert_eq!(album.seq, AlbumSeq(0)); + assert_eq!(album.meta.seq, AlbumSeq(0)); // Setting a seq on an album. album.set_seq(AlbumSeq(6)); - assert_eq!(album.seq, AlbumSeq(6)); + assert_eq!(album.meta.seq, AlbumSeq(6)); album.set_seq(AlbumSeq(6)); - assert_eq!(album.seq, AlbumSeq(6)); + assert_eq!(album.meta.seq, AlbumSeq(6)); album.set_seq(AlbumSeq(8)); - assert_eq!(album.seq, AlbumSeq(8)); + assert_eq!(album.meta.seq, AlbumSeq(8)); // Clearing seq. album.clear_seq(); - assert_eq!(album.seq, AlbumSeq(0)); + assert_eq!(album.meta.seq, AlbumSeq(0)); } #[test] fn merge_album_no_overlap() { let left = FULL_COLLECTION[0].albums[0].to_owned(); let mut right = FULL_COLLECTION[0].albums[1].to_owned(); - right.id = left.id.clone(); + right.meta.id = left.meta.id.clone(); let mut expected = left.clone(); expected.tracks.append(&mut right.tracks.clone()); @@ -305,7 +315,7 @@ mod tests { fn merge_album_overlap() { let mut left = FULL_COLLECTION[0].albums[0].to_owned(); let mut right = FULL_COLLECTION[0].albums[1].to_owned(); - right.id = left.id.clone(); + right.meta.id = left.meta.id.clone(); left.tracks.push(right.tracks[0].clone()); left.tracks.sort_unstable(); @@ -328,23 +338,23 @@ mod tests { let mut album = Album::new(AlbumId::new("an album"), AlbumDate::default(), None, vec![]); let mut expected: Option = None; - assert_eq!(album.musicbrainz, expected); + assert_eq!(album.meta.musicbrainz, expected); // Setting a URL on an album. album.set_musicbrainz_ref(MbAlbumRef::from_url_str(MUSICBRAINZ).unwrap()); _ = expected.insert(MbAlbumRef::from_url_str(MUSICBRAINZ).unwrap()); - assert_eq!(album.musicbrainz, expected); + assert_eq!(album.meta.musicbrainz, expected); album.set_musicbrainz_ref(MbAlbumRef::from_url_str(MUSICBRAINZ).unwrap()); - assert_eq!(album.musicbrainz, expected); + assert_eq!(album.meta.musicbrainz, expected); album.set_musicbrainz_ref(MbAlbumRef::from_url_str(MUSICBRAINZ_2).unwrap()); _ = expected.insert(MbAlbumRef::from_url_str(MUSICBRAINZ_2).unwrap()); - assert_eq!(album.musicbrainz, expected); + assert_eq!(album.meta.musicbrainz, expected); // Clearing URLs. album.clear_musicbrainz_ref(); _ = expected.take(); - assert_eq!(album.musicbrainz, expected); + assert_eq!(album.meta.musicbrainz, expected); } } diff --git a/src/core/collection/artist.rs b/src/core/collection/artist.rs index dd0c851..c19d8b0 100644 --- a/src/core/collection/artist.rs +++ b/src/core/collection/artist.rs @@ -13,18 +13,24 @@ use crate::core::collection::{ /// An artist. #[derive(Clone, Debug, PartialEq, Eq)] pub struct Artist { + pub meta: ArtistMeta, + pub albums: Vec, +} + +/// Artist metadata. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct ArtistMeta { pub id: ArtistId, pub sort: Option, pub musicbrainz: Option, pub properties: HashMap>, - pub albums: Vec, } impl WithId for Artist { type Id = ArtistId; fn id(&self) -> &Self::Id { - &self.id + &self.meta.id } } @@ -38,32 +44,34 @@ impl Artist { /// Create new [`Artist`] with the given [`ArtistId`]. pub fn new>(id: Id) -> Self { Artist { - id: id.into(), - sort: None, - musicbrainz: None, - properties: HashMap::new(), + meta: ArtistMeta { + id: id.into(), + sort: None, + musicbrainz: None, + properties: HashMap::new(), + }, albums: vec![], } } pub fn get_sort_key(&self) -> (&ArtistId,) { - (self.sort.as_ref().unwrap_or(&self.id),) + (self.meta.sort.as_ref().unwrap_or(&self.meta.id),) } pub fn set_sort_key>(&mut self, sort: SORT) { - self.sort = Some(sort.into()); + self.meta.sort = Some(sort.into()); } pub fn clear_sort_key(&mut self) { - _ = self.sort.take(); + _ = self.meta.sort.take(); } pub fn set_musicbrainz_ref(&mut self, mbref: MbArtistRef) { - _ = self.musicbrainz.insert(mbref); + _ = self.meta.musicbrainz.insert(mbref); } pub fn clear_musicbrainz_ref(&mut self) { - _ = self.musicbrainz.take(); + _ = self.meta.musicbrainz.take(); } // In the functions below, it would be better to use `contains` instead of `iter().any`, but for @@ -71,7 +79,7 @@ impl Artist { // https://stackoverflow.com/questions/48985924/why-does-a-str-not-coerce-to-a-string-when-using-veccontains pub fn add_to_property + Into>(&mut self, property: S, values: Vec) { - match self.properties.get_mut(property.as_ref()) { + match self.meta.properties.get_mut(property.as_ref()) { Some(container) => { container.append( &mut values @@ -82,7 +90,7 @@ impl Artist { ); } None => { - self.properties.insert( + self.meta.properties.insert( property.into(), values.into_iter().map(|s| s.into()).collect(), ); @@ -91,23 +99,23 @@ impl Artist { } pub fn remove_from_property>(&mut self, property: S, values: Vec) { - if let Some(container) = self.properties.get_mut(property.as_ref()) { + if let Some(container) = self.meta.properties.get_mut(property.as_ref()) { container.retain(|val| !values.iter().any(|x| x.as_ref() == val)); if container.is_empty() { - self.properties.remove(property.as_ref()); + self.meta.properties.remove(property.as_ref()); } } } pub fn set_property + Into>(&mut self, property: S, values: Vec) { - self.properties.insert( + self.meta.properties.insert( property.into(), values.into_iter().map(|s| s.into()).collect(), ); } pub fn clear_property>(&mut self, property: S) { - self.properties.remove(property.as_ref()); + self.meta.properties.remove(property.as_ref()); } } @@ -125,11 +133,11 @@ impl Ord for Artist { impl Merge for Artist { fn merge_in_place(&mut self, other: Self) { - assert_eq!(self.id, other.id); + assert_eq!(self.meta.id, other.meta.id); - self.sort = self.sort.take().or(other.sort); - self.musicbrainz = self.musicbrainz.take().or(other.musicbrainz); - self.properties.merge_in_place(other.properties); + self.meta.sort = self.meta.sort.take().or(other.meta.sort); + self.meta.musicbrainz = self.meta.musicbrainz.take().or(other.meta.musicbrainz); + self.meta.properties.merge_in_place(other.meta.properties); let albums = mem::take(&mut self.albums); self.albums = MergeCollections::merge_iter(albums, other.albums); @@ -181,32 +189,32 @@ mod tests { let mut artist = Artist::new(&artist_id.name); - assert_eq!(artist.id, artist_id); - assert_eq!(artist.sort, None); + assert_eq!(artist.meta.id, artist_id); + assert_eq!(artist.meta.sort, None); assert_eq!(artist.get_sort_key(), (&artist_id,)); assert!(artist < Artist::new(sort_id_1.clone())); assert!(artist < Artist::new(sort_id_2.clone())); artist.set_sort_key(sort_id_1.clone()); - assert_eq!(artist.id, artist_id); - assert_eq!(artist.sort.as_ref(), Some(&sort_id_1)); + assert_eq!(artist.meta.id, artist_id); + assert_eq!(artist.meta.sort.as_ref(), Some(&sort_id_1)); assert_eq!(artist.get_sort_key(), (&sort_id_1,)); assert!(artist > Artist::new(artist_id.clone())); assert!(artist < Artist::new(sort_id_2.clone())); artist.set_sort_key(sort_id_2.clone()); - assert_eq!(artist.id, artist_id); - assert_eq!(artist.sort.as_ref(), Some(&sort_id_2)); + assert_eq!(artist.meta.id, artist_id); + assert_eq!(artist.meta.sort.as_ref(), Some(&sort_id_2)); assert_eq!(artist.get_sort_key(), (&sort_id_2,)); assert!(artist > Artist::new(artist_id.clone())); assert!(artist > Artist::new(sort_id_1.clone())); artist.clear_sort_key(); - assert_eq!(artist.id, artist_id); - assert_eq!(artist.sort, None); + assert_eq!(artist.meta.id, artist_id); + assert_eq!(artist.meta.sort, None); assert_eq!(artist.get_sort_key(), (&artist_id,)); assert!(artist < Artist::new(sort_id_1.clone())); assert!(artist < Artist::new(sort_id_2.clone())); @@ -217,24 +225,24 @@ mod tests { let mut artist = Artist::new(ArtistId::new("an artist")); let mut expected: Option = None; - assert_eq!(artist.musicbrainz, expected); + assert_eq!(artist.meta.musicbrainz, expected); // Setting a URL on an artist. artist.set_musicbrainz_ref(MbArtistRef::from_url_str(MUSICBRAINZ).unwrap()); _ = expected.insert(MbArtistRef::from_url_str(MUSICBRAINZ).unwrap()); - assert_eq!(artist.musicbrainz, expected); + assert_eq!(artist.meta.musicbrainz, expected); artist.set_musicbrainz_ref(MbArtistRef::from_url_str(MUSICBRAINZ).unwrap()); - assert_eq!(artist.musicbrainz, expected); + assert_eq!(artist.meta.musicbrainz, expected); artist.set_musicbrainz_ref(MbArtistRef::from_url_str(MUSICBRAINZ_2).unwrap()); _ = expected.insert(MbArtistRef::from_url_str(MUSICBRAINZ_2).unwrap()); - assert_eq!(artist.musicbrainz, expected); + assert_eq!(artist.meta.musicbrainz, expected); // Clearing URLs. artist.clear_musicbrainz_ref(); _ = expected.take(); - assert_eq!(artist.musicbrainz, expected); + assert_eq!(artist.meta.musicbrainz, expected); } #[test] @@ -242,70 +250,70 @@ mod tests { let mut artist = Artist::new(ArtistId::new("an artist")); let mut expected: Vec = vec![]; - assert!(artist.properties.is_empty()); + assert!(artist.meta.properties.is_empty()); // Adding a single URL. artist.add_to_property("MusicButler", vec![MUSICBUTLER]); expected.push(MUSICBUTLER.to_owned()); - assert_eq!(artist.properties.get("MusicButler"), Some(&expected)); + assert_eq!(artist.meta.properties.get("MusicButler"), Some(&expected)); // Adding a URL that already exists is ok, but does not do anything. artist.add_to_property("MusicButler", vec![MUSICBUTLER]); - assert_eq!(artist.properties.get("MusicButler"), Some(&expected)); + assert_eq!(artist.meta.properties.get("MusicButler"), Some(&expected)); // Adding another single URL. artist.add_to_property("MusicButler", vec![MUSICBUTLER_2]); expected.push(MUSICBUTLER_2.to_owned()); - assert_eq!(artist.properties.get("MusicButler"), Some(&expected)); + assert_eq!(artist.meta.properties.get("MusicButler"), Some(&expected)); artist.add_to_property("MusicButler", vec![MUSICBUTLER_2]); - assert_eq!(artist.properties.get("MusicButler"), Some(&expected)); + assert_eq!(artist.meta.properties.get("MusicButler"), Some(&expected)); // Removing a URL. artist.remove_from_property("MusicButler", vec![MUSICBUTLER]); expected.retain(|url| url != MUSICBUTLER); - assert_eq!(artist.properties.get("MusicButler"), Some(&expected)); + assert_eq!(artist.meta.properties.get("MusicButler"), Some(&expected)); // Removing URls that do not exist is okay, they will be ignored. artist.remove_from_property("MusicButler", vec![MUSICBUTLER]); - assert_eq!(artist.properties.get("MusicButler"), Some(&expected)); + assert_eq!(artist.meta.properties.get("MusicButler"), Some(&expected)); // Removing a URL. artist.remove_from_property("MusicButler", vec![MUSICBUTLER_2]); expected.retain(|url| url.as_str() != MUSICBUTLER_2); - assert!(artist.properties.is_empty()); + assert!(artist.meta.properties.is_empty()); artist.remove_from_property("MusicButler", vec![MUSICBUTLER_2]); - assert!(artist.properties.is_empty()); + assert!(artist.meta.properties.is_empty()); // Adding URLs if some exist is okay, they will be ignored. artist.add_to_property("MusicButler", vec![MUSICBUTLER]); expected.push(MUSICBUTLER.to_owned()); - assert_eq!(artist.properties.get("MusicButler"), Some(&expected)); + assert_eq!(artist.meta.properties.get("MusicButler"), Some(&expected)); artist.add_to_property("MusicButler", vec![MUSICBUTLER, MUSICBUTLER_2]); expected.push(MUSICBUTLER_2.to_owned()); - assert_eq!(artist.properties.get("MusicButler"), Some(&expected)); + assert_eq!(artist.meta.properties.get("MusicButler"), Some(&expected)); // Removing URLs if some do not exist is okay, they will be ignored. artist.remove_from_property("MusicButler", vec![MUSICBUTLER]); expected.retain(|url| url.as_str() != MUSICBUTLER); - assert_eq!(artist.properties.get("MusicButler"), Some(&expected)); + assert_eq!(artist.meta.properties.get("MusicButler"), Some(&expected)); artist.remove_from_property("MusicButler", vec![MUSICBUTLER, MUSICBUTLER_2]); expected.retain(|url| url.as_str() != MUSICBUTLER_2); - assert!(artist.properties.is_empty()); + assert!(artist.meta.properties.is_empty()); // Adding mutliple URLs without clashes. artist.add_to_property("MusicButler", vec![MUSICBUTLER, MUSICBUTLER_2]); expected.push(MUSICBUTLER.to_owned()); expected.push(MUSICBUTLER_2.to_owned()); - assert_eq!(artist.properties.get("MusicButler"), Some(&expected)); + assert_eq!(artist.meta.properties.get("MusicButler"), Some(&expected)); // Removing multiple URLs without clashes. artist.remove_from_property("MusicButler", vec![MUSICBUTLER, MUSICBUTLER_2]); expected.clear(); - assert!(artist.properties.is_empty()); + assert!(artist.meta.properties.is_empty()); } #[test] @@ -313,40 +321,43 @@ mod tests { let mut artist = Artist::new(ArtistId::new("an artist")); let mut expected: Vec = vec![]; - assert!(artist.properties.is_empty()); + assert!(artist.meta.properties.is_empty()); // Set URLs. artist.set_property("MusicButler", vec![MUSICBUTLER]); expected.push(MUSICBUTLER.to_owned()); - assert_eq!(artist.properties.get("MusicButler"), Some(&expected)); + assert_eq!(artist.meta.properties.get("MusicButler"), Some(&expected)); artist.set_property("MusicButler", vec![MUSICBUTLER_2]); expected.clear(); expected.push(MUSICBUTLER_2.to_owned()); - assert_eq!(artist.properties.get("MusicButler"), Some(&expected)); + assert_eq!(artist.meta.properties.get("MusicButler"), Some(&expected)); artist.set_property("MusicButler", vec![MUSICBUTLER, MUSICBUTLER_2]); expected.clear(); expected.push(MUSICBUTLER.to_owned()); expected.push(MUSICBUTLER_2.to_owned()); - assert_eq!(artist.properties.get("MusicButler"), Some(&expected)); + assert_eq!(artist.meta.properties.get("MusicButler"), Some(&expected)); // Clear URLs. artist.clear_property("MusicButler"); expected.clear(); - assert!(artist.properties.is_empty()); + assert!(artist.meta.properties.is_empty()); } #[test] fn merge_artist_no_overlap() { let left = FULL_COLLECTION[0].to_owned(); let mut right = FULL_COLLECTION[1].to_owned(); - right.id = left.id.clone(); - right.musicbrainz = None; - right.properties = HashMap::new(); + right.meta.id = left.meta.id.clone(); + right.meta.musicbrainz = None; + right.meta.properties = HashMap::new(); let mut expected = left.clone(); - expected.properties = expected.properties.merge(right.clone().properties); + expected.meta.properties = expected + .meta + .properties + .merge(right.clone().meta.properties); expected.albums.append(&mut right.albums.clone()); expected.albums.sort_unstable(); @@ -362,12 +373,15 @@ mod tests { fn merge_artist_overlap() { let mut left = FULL_COLLECTION[0].to_owned(); let mut right = FULL_COLLECTION[1].to_owned(); - right.id = left.id.clone(); + right.meta.id = left.meta.id.clone(); left.albums.push(right.albums[0].clone()); left.albums.sort_unstable(); let mut expected = left.clone(); - expected.properties = expected.properties.merge(right.clone().properties); + expected.meta.properties = expected + .meta + .properties + .merge(right.clone().meta.properties); expected.albums.append(&mut right.albums.clone()); expected.albums.sort_unstable(); expected.albums.dedup(); diff --git a/src/core/musichoard/base.rs b/src/core/musichoard/base.rs index 1919924..70baae9 100644 --- a/src/core/musichoard/base.rs +++ b/src/core/musichoard/base.rs @@ -59,14 +59,14 @@ impl IMusicHoardBasePrivate for MusicHoard } fn get_artist<'a>(collection: &'a Collection, artist_id: &ArtistId) -> Option<&'a Artist> { - collection.iter().find(|a| &a.id == artist_id) + collection.iter().find(|a| &a.meta.id == artist_id) } fn get_artist_mut<'a>( collection: &'a mut Collection, artist_id: &ArtistId, ) -> Option<&'a mut Artist> { - collection.iter_mut().find(|a| &a.id == artist_id) + collection.iter_mut().find(|a| &a.meta.id == artist_id) } fn get_artist_mut_or_err<'a>( @@ -79,7 +79,7 @@ impl IMusicHoardBasePrivate for MusicHoard } fn get_album_mut<'a>(artist: &'a mut Artist, album_id: &AlbumId) -> Option<&'a mut Album> { - artist.albums.iter_mut().find(|a| &a.id == album_id) + artist.albums.iter_mut().find(|a| &a.meta.id == album_id) } fn get_album_mut_or_err<'a>( @@ -115,7 +115,7 @@ mod tests { library_cache: left .clone() .into_iter() - .map(|a| (a.id.clone(), a)) + .map(|a| (a.meta.id.clone(), a)) .collect(), database_cache: right.clone(), ..Default::default() @@ -129,7 +129,7 @@ mod tests { library_cache: right .clone() .into_iter() - .map(|a| (a.id.clone(), a)) + .map(|a| (a.meta.id.clone(), a)) .collect(), database_cache: left.clone(), ..Default::default() @@ -153,7 +153,7 @@ mod tests { library_cache: left .clone() .into_iter() - .map(|a| (a.id.clone(), a)) + .map(|a| (a.meta.id.clone(), a)) .collect(), database_cache: right.clone(), ..Default::default() @@ -167,7 +167,7 @@ mod tests { library_cache: right .clone() .into_iter() - .map(|a| (a.id.clone(), a)) + .map(|a| (a.meta.id.clone(), a)) .collect(), database_cache: left.clone(), ..Default::default() @@ -191,20 +191,20 @@ mod tests { assert!(right.first().unwrap() > left.first().unwrap()); let artist_sort = Some(ArtistId::new("Album_Artist 0")); - right[0].sort = artist_sort.clone(); + right[0].meta.sort = artist_sort.clone(); assert!(right.first().unwrap() < left.first().unwrap()); // The result of the merge should be the same list of artists, but with the last artist now // in first place. let mut expected = left.to_owned(); - expected.last_mut().as_mut().unwrap().sort = artist_sort.clone(); + expected.last_mut().as_mut().unwrap().meta.sort = artist_sort.clone(); expected.rotate_right(1); let mut mh = MusicHoard { library_cache: left .clone() .into_iter() - .map(|a| (a.id.clone(), a)) + .map(|a| (a.meta.id.clone(), a)) .collect(), database_cache: right.clone(), ..Default::default() @@ -218,7 +218,7 @@ mod tests { library_cache: right .clone() .into_iter() - .map(|a| (a.id.clone(), a)) + .map(|a| (a.meta.id.clone(), a)) .collect(), database_cache: left.clone(), ..Default::default() diff --git a/src/core/musichoard/database.rs b/src/core/musichoard/database.rs index 8303144..9926966 100644 --- a/src/core/musichoard/database.rs +++ b/src/core/musichoard/database.rs @@ -91,7 +91,9 @@ impl IMusicHoardDatabase for MusicHoard>(&mut self, artist_id: Id) -> Result<(), Error> { self.update_collection(|collection| { - let index_opt = collection.iter().position(|a| &a.id == artist_id.as_ref()); + let index_opt = collection + .iter() + .position(|a| &a.meta.id == artist_id.as_ref()); if let Some(index) = index_opt { collection.remove(index); } @@ -434,29 +436,29 @@ mod tests { assert!(music_hoard.add_artist(artist_id.clone()).is_ok()); let mut expected: Option = None; - assert_eq!(music_hoard.collection[0].musicbrainz, expected); + assert_eq!(music_hoard.collection[0].meta.musicbrainz, expected); // Setting a URL on an artist not in the collection is an error. assert!(music_hoard .set_artist_musicbrainz(&artist_id_2, MUSICBRAINZ) .is_err()); - assert_eq!(music_hoard.collection[0].musicbrainz, expected); + assert_eq!(music_hoard.collection[0].meta.musicbrainz, expected); // Setting a URL on an artist. assert!(music_hoard .set_artist_musicbrainz(&artist_id, MUSICBRAINZ) .is_ok()); _ = expected.insert(MbArtistRef::from_url_str(MUSICBRAINZ).unwrap()); - assert_eq!(music_hoard.collection[0].musicbrainz, expected); + assert_eq!(music_hoard.collection[0].meta.musicbrainz, expected); // Clearing URLs on an artist that does not exist is an error. assert!(music_hoard.clear_artist_musicbrainz(&artist_id_2).is_err()); - assert_eq!(music_hoard.collection[0].musicbrainz, expected); + assert_eq!(music_hoard.collection[0].meta.musicbrainz, expected); // Clearing URLs. assert!(music_hoard.clear_artist_musicbrainz(&artist_id).is_ok()); _ = expected.take(); - assert_eq!(music_hoard.collection[0].musicbrainz, expected); + assert_eq!(music_hoard.collection[0].meta.musicbrainz, expected); } #[test] @@ -472,13 +474,13 @@ mod tests { assert!(music_hoard.add_artist(artist_id.clone()).is_ok()); let mut expected: Vec = vec![]; - assert!(music_hoard.collection[0].properties.is_empty()); + assert!(music_hoard.collection[0].meta.properties.is_empty()); // Adding URLs to an artist not in the collection is an error. assert!(music_hoard .add_to_artist_property(&artist_id_2, "MusicButler", vec![MUSICBUTLER]) .is_err()); - assert!(music_hoard.collection[0].properties.is_empty()); + assert!(music_hoard.collection[0].meta.properties.is_empty()); // Adding mutliple URLs without clashes. assert!(music_hoard @@ -487,7 +489,7 @@ mod tests { expected.push(MUSICBUTLER.to_owned()); expected.push(MUSICBUTLER_2.to_owned()); assert_eq!( - music_hoard.collection[0].properties.get("MusicButler"), + music_hoard.collection[0].meta.properties.get("MusicButler"), Some(&expected) ); @@ -496,7 +498,7 @@ mod tests { .remove_from_artist_property(&artist_id_2, "MusicButler", vec![MUSICBUTLER]) .is_err()); assert_eq!( - music_hoard.collection[0].properties.get("MusicButler"), + music_hoard.collection[0].meta.properties.get("MusicButler"), Some(&expected) ); @@ -509,7 +511,7 @@ mod tests { ) .is_ok()); expected.clear(); - assert!(music_hoard.collection[0].properties.is_empty()); + assert!(music_hoard.collection[0].meta.properties.is_empty()); } #[test] @@ -525,13 +527,13 @@ mod tests { assert!(music_hoard.add_artist(artist_id.clone()).is_ok()); let mut expected: Vec = vec![]; - assert!(music_hoard.collection[0].properties.is_empty()); + assert!(music_hoard.collection[0].meta.properties.is_empty()); // Seting URL on an artist not in the collection is an error. assert!(music_hoard .set_artist_property(&artist_id_2, "MusicButler", vec![MUSICBUTLER]) .is_err()); - assert!(music_hoard.collection[0].properties.is_empty()); + assert!(music_hoard.collection[0].meta.properties.is_empty()); // Set URLs. assert!(music_hoard @@ -541,7 +543,7 @@ mod tests { expected.push(MUSICBUTLER.to_owned()); expected.push(MUSICBUTLER_2.to_owned()); assert_eq!( - music_hoard.collection[0].properties.get("MusicButler"), + music_hoard.collection[0].meta.properties.get("MusicButler"), Some(&expected) ); @@ -555,7 +557,7 @@ mod tests { .clear_artist_property(&artist_id, "MusicButler") .is_ok()); expected.clear(); - assert!(music_hoard.collection[0].properties.is_empty()); + assert!(music_hoard.collection[0].meta.properties.is_empty()); } #[test] @@ -581,17 +583,17 @@ mod tests { database.expect_save().times(2).returning(|_| Ok(())); let mut music_hoard = MusicHoard::database(database).unwrap(); - assert_eq!(music_hoard.collection[0].albums[0].seq, AlbumSeq(0)); + assert_eq!(music_hoard.collection[0].albums[0].meta.seq, AlbumSeq(0)); // Seting seq on an album not belonging to the artist is an error. assert!(music_hoard .set_album_seq(&artist_id, &album_id_2, 6) .is_err()); - assert_eq!(music_hoard.collection[0].albums[0].seq, AlbumSeq(0)); + assert_eq!(music_hoard.collection[0].albums[0].meta.seq, AlbumSeq(0)); // Set seq. assert!(music_hoard.set_album_seq(&artist_id, &album_id, 6).is_ok()); - assert_eq!(music_hoard.collection[0].albums[0].seq, AlbumSeq(6)); + assert_eq!(music_hoard.collection[0].albums[0].meta.seq, AlbumSeq(6)); // Clearing seq on an album that does not exist is an error. assert!(music_hoard @@ -600,7 +602,7 @@ mod tests { // Clear seq. assert!(music_hoard.clear_album_seq(&artist_id, &album_id).is_ok()); - assert_eq!(music_hoard.collection[0].albums[0].seq, AlbumSeq(0)); + assert_eq!(music_hoard.collection[0].albums[0].meta.seq, AlbumSeq(0)); } #[test] diff --git a/src/core/musichoard/library.rs b/src/core/musichoard/library.rs index d2e9888..47e9357 100644 --- a/src/core/musichoard/library.rs +++ b/src/core/musichoard/library.rs @@ -86,17 +86,17 @@ impl MusicHoard { .or_insert_with(|| Artist::new(artist_id)), }; - if artist.sort.is_some() { - if artist_sort.is_some() && (artist.sort != artist_sort) { + if artist.meta.sort.is_some() { + if artist_sort.is_some() && (artist.meta.sort != artist_sort) { return Err(Error::CollectionError(format!( "multiple album_artist_sort found for artist '{}': '{}' != '{}'", - artist.id, - artist.sort.as_ref().unwrap(), + artist.meta.id, + artist.meta.sort.as_ref().unwrap(), artist_sort.as_ref().unwrap() ))); } } else if artist_sort.is_some() { - artist.sort = artist_sort; + artist.meta.sort = artist_sort; } // Do a linear search as few artists have more than a handful of albums. Search from the @@ -105,7 +105,7 @@ impl MusicHoard { .albums .iter_mut() .rev() - .find(|album| album.id == album_id) + .find(|album| album.meta.id == album_id) { Some(album) => album.tracks.push(track), None => { @@ -197,13 +197,13 @@ mod tests { assert!(music_hoard.get_collection()[0] .albums .iter() - .any(|album| album.id.title == "album_title a.a")); + .any(|album| album.meta.id.title == "album_title a.a")); music_hoard.rescan_library().unwrap(); assert!(!music_hoard.get_collection()[0] .albums .iter() - .any(|album| album.id.title == "album_title a.a")); + .any(|album| album.meta.id.title == "album_title a.a")); } #[test] @@ -234,8 +234,8 @@ mod tests { let mut library = MockILibrary::new(); let mut expected = LIBRARY_COLLECTION.to_owned(); - let removed_album_id = expected[0].albums[0].id.clone(); - let clashed_album_id = &expected[1].albums[0].id; + let removed_album_id = expected[0].albums[0].meta.id.clone(); + let clashed_album_id = &expected[1].albums[0].meta.id; let mut items = LIBRARY_ITEMS.to_owned(); for item in items @@ -245,7 +245,7 @@ mod tests { item.album_title = clashed_album_id.title.clone(); } - expected[0].albums[0].id = clashed_album_id.clone(); + expected[0].albums[0].meta.id = clashed_album_id.clone(); let library_input = Query::new(); let library_result = Ok(items); diff --git a/src/core/testmod.rs b/src/core/testmod.rs index 7503316..2b0db1d 100644 --- a/src/core/testmod.rs +++ b/src/core/testmod.rs @@ -2,8 +2,8 @@ use once_cell::sync::Lazy; use std::collections::HashMap; use crate::core::collection::{ - album::{Album, AlbumId, AlbumPrimaryType, AlbumSeq}, - artist::{Artist, ArtistId}, + album::{Album, AlbumId, AlbumMeta, AlbumPrimaryType, AlbumSeq}, + artist::{Artist, ArtistId, ArtistMeta}, musicbrainz::{MbAlbumRef, MbArtistRef}, track::{Track, TrackFormat, TrackId, TrackNum, TrackQuality}, }; diff --git a/src/external/database/json/mod.rs b/src/external/database/json/mod.rs index 3347c5c..924220e 100644 --- a/src/external/database/json/mod.rs +++ b/src/external/database/json/mod.rs @@ -83,7 +83,7 @@ mod tests { let mut expected = FULL_COLLECTION.to_owned(); for artist in expected.iter_mut() { for album in artist.albums.iter_mut() { - album.date = AlbumDate::default(); + album.meta.date = AlbumDate::default(); album.tracks.clear(); } } diff --git a/src/external/database/serde/deserialize.rs b/src/external/database/serde/deserialize.rs index 82cb220..ea63e2c 100644 --- a/src/external/database/serde/deserialize.rs +++ b/src/external/database/serde/deserialize.rs @@ -3,7 +3,7 @@ use std::{collections::HashMap, fmt}; use serde::{de::Visitor, Deserialize, Deserializer}; use crate::{ - collection::musicbrainz::Mbid, + collection::{album::AlbumMeta, artist::ArtistMeta, musicbrainz::Mbid}, core::collection::{ album::{Album, AlbumDate, AlbumId, AlbumSeq}, artist::{Artist, ArtistId}, @@ -86,10 +86,12 @@ impl<'de> Deserialize<'de> for DeserializeMbid { impl From for Artist { fn from(artist: DeserializeArtist) -> Self { Artist { - id: ArtistId::new(artist.name), - sort: artist.sort.map(ArtistId::new), - musicbrainz: artist.musicbrainz.map(Into::::into).map(Into::into), - properties: artist.properties, + meta: ArtistMeta { + id: ArtistId::new(artist.name), + sort: artist.sort.map(ArtistId::new), + musicbrainz: artist.musicbrainz.map(Into::::into).map(Into::into), + properties: artist.properties, + }, albums: artist.albums.into_iter().map(Into::into).collect(), } } @@ -98,12 +100,14 @@ impl From for Artist { impl From for Album { fn from(album: DeserializeAlbum) -> Self { Album { - id: AlbumId { title: album.title }, - date: AlbumDate::default(), - seq: AlbumSeq(album.seq), - musicbrainz: album.musicbrainz.map(Into::::into).map(Into::into), - primary_type: album.primary_type.map(Into::into), - secondary_types: album.secondary_types.into_iter().map(Into::into).collect(), + meta: AlbumMeta { + id: AlbumId { title: album.title }, + date: AlbumDate::default(), + seq: AlbumSeq(album.seq), + musicbrainz: album.musicbrainz.map(Into::::into).map(Into::into), + primary_type: album.primary_type.map(Into::into), + secondary_types: album.secondary_types.into_iter().map(Into::into).collect(), + }, tracks: vec![], } } diff --git a/src/external/database/serde/serialize.rs b/src/external/database/serde/serialize.rs index 874a01c..8abf5f3 100644 --- a/src/external/database/serde/serialize.rs +++ b/src/external/database/serde/serialize.rs @@ -52,13 +52,15 @@ impl<'a> Serialize for SerializeMbid<'a> { impl<'a> From<&'a Artist> for SerializeArtist<'a> { fn from(artist: &'a Artist) -> Self { SerializeArtist { - name: &artist.id.name, - sort: artist.sort.as_ref().map(|id| id.name.as_ref()), + name: &artist.meta.id.name, + sort: artist.meta.sort.as_ref().map(|id| id.name.as_ref()), musicbrainz: artist + .meta .musicbrainz .as_ref() .map(|mbref| SerializeMbid(mbref.mbid())), properties: artist + .meta .properties .iter() .map(|(k, v)| (k.as_ref(), v)) @@ -71,14 +73,16 @@ impl<'a> From<&'a Artist> for SerializeArtist<'a> { impl<'a> From<&'a Album> for SerializeAlbum<'a> { fn from(album: &'a Album) -> Self { SerializeAlbum { - title: &album.id.title, - seq: album.seq.0, + title: &album.meta.id.title, + seq: album.meta.seq.0, musicbrainz: album + .meta .musicbrainz .as_ref() .map(|mbref| SerializeMbid(mbref.mbid())), - primary_type: album.primary_type.map(Into::into), + primary_type: album.meta.primary_type.map(Into::into), secondary_types: album + .meta .secondary_types .iter() .copied() diff --git a/src/testmod/full.rs b/src/testmod/full.rs index b91f846..1a8c5ae 100644 --- a/src/testmod/full.rs +++ b/src/testmod/full.rs @@ -2,35 +2,39 @@ macro_rules! full_collection { () => { vec![ Artist { - id: ArtistId { - name: "Album_Artist ‘A’".to_string(), + meta: ArtistMeta { + id: ArtistId { + name: "Album_Artist ‘A’".to_string(), + }, + sort: None, + musicbrainz: Some(MbArtistRef::from_url_str( + "https://musicbrainz.org/artist/00000000-0000-0000-0000-000000000000" + ).unwrap()), + properties: HashMap::from([ + (String::from("MusicButler"), vec![ + String::from("https://www.musicbutler.io/artist-page/000000000"), + ]), + (String::from("Qobuz"), vec![ + String::from( + "https://www.qobuz.com/nl-nl/interpreter/artist-a/download-streaming-albums", + ) + ]), + ]), }, - sort: None, - musicbrainz: Some(MbArtistRef::from_url_str( - "https://musicbrainz.org/artist/00000000-0000-0000-0000-000000000000" - ).unwrap()), - properties: HashMap::from([ - (String::from("MusicButler"), vec![ - String::from("https://www.musicbutler.io/artist-page/000000000"), - ]), - (String::from("Qobuz"), vec![ - String::from( - "https://www.qobuz.com/nl-nl/interpreter/artist-a/download-streaming-albums", - ) - ]), - ]), albums: vec![ Album { - id: AlbumId { - title: "album_title a.a".to_string(), + meta: AlbumMeta { + id: AlbumId { + title: "album_title a.a".to_string(), + }, + date: 1998.into(), + seq: AlbumSeq(1), + musicbrainz: Some(MbAlbumRef::from_url_str( + "https://musicbrainz.org/release-group/00000000-0000-0000-0000-000000000000" + ).unwrap()), + primary_type: Some(AlbumPrimaryType::Album), + secondary_types: vec![], }, - date: 1998.into(), - seq: AlbumSeq(1), - musicbrainz: Some(MbAlbumRef::from_url_str( - "https://musicbrainz.org/release-group/00000000-0000-0000-0000-000000000000" - ).unwrap()), - primary_type: Some(AlbumPrimaryType::Album), - secondary_types: vec![], tracks: vec![ Track { id: TrackId { @@ -82,14 +86,16 @@ macro_rules! full_collection { ], }, Album { - id: AlbumId { - title: "album_title a.b".to_string(), + meta: AlbumMeta { + id: AlbumId { + title: "album_title a.b".to_string(), + }, + date: (2015, 4).into(), + seq: AlbumSeq(1), + musicbrainz: None, + primary_type: Some(AlbumPrimaryType::Album), + secondary_types: vec![], }, - date: (2015, 4).into(), - seq: AlbumSeq(1), - musicbrainz: None, - primary_type: Some(AlbumPrimaryType::Album), - secondary_types: vec![], tracks: vec![ Track { id: TrackId { @@ -118,37 +124,41 @@ macro_rules! full_collection { ], }, Artist { - id: ArtistId { - name: "Album_Artist ‘B’".to_string(), + meta: ArtistMeta { + id: ArtistId { + name: "Album_Artist ‘B’".to_string(), + }, + sort: None, + musicbrainz: Some(MbArtistRef::from_url_str( + "https://musicbrainz.org/artist/11111111-1111-1111-1111-111111111111" + ).unwrap()), + properties: HashMap::from([ + (String::from("MusicButler"), vec![ + String::from("https://www.musicbutler.io/artist-page/111111111"), + String::from("https://www.musicbutler.io/artist-page/111111112"), + ]), + (String::from("Bandcamp"), vec![ + String::from("https://artist-b.bandcamp.com/") + ]), + (String::from("Qobuz"), vec![ + String::from( + "https://www.qobuz.com/nl-nl/interpreter/artist-b/download-streaming-albums", + ) + ]), + ]), }, - sort: None, - musicbrainz: Some(MbArtistRef::from_url_str( - "https://musicbrainz.org/artist/11111111-1111-1111-1111-111111111111" - ).unwrap()), - properties: HashMap::from([ - (String::from("MusicButler"), vec![ - String::from("https://www.musicbutler.io/artist-page/111111111"), - String::from("https://www.musicbutler.io/artist-page/111111112"), - ]), - (String::from("Bandcamp"), vec![ - String::from("https://artist-b.bandcamp.com/") - ]), - (String::from("Qobuz"), vec![ - String::from( - "https://www.qobuz.com/nl-nl/interpreter/artist-b/download-streaming-albums", - ) - ]), - ]), albums: vec![ Album { - id: AlbumId { - title: "album_title b.a".to_string(), + meta: AlbumMeta { + id: AlbumId { + title: "album_title b.a".to_string(), + }, + date: (2003, 6, 6).into(), + seq: AlbumSeq(1), + musicbrainz: None, + primary_type: Some(AlbumPrimaryType::Album), + secondary_types: vec![], }, - date: (2003, 6, 6).into(), - seq: AlbumSeq(1), - musicbrainz: None, - primary_type: Some(AlbumPrimaryType::Album), - secondary_types: vec![], tracks: vec![ Track { id: TrackId { @@ -178,16 +188,18 @@ macro_rules! full_collection { ], }, Album { - id: AlbumId { - title: "album_title b.b".to_string(), + meta: AlbumMeta { + id: AlbumId { + title: "album_title b.b".to_string(), + }, + date: 2008.into(), + seq: AlbumSeq(3), + musicbrainz: Some(MbAlbumRef::from_url_str( + "https://musicbrainz.org/release-group/11111111-1111-1111-1111-111111111111" + ).unwrap()), + primary_type: Some(AlbumPrimaryType::Album), + secondary_types: vec![], }, - date: 2008.into(), - seq: AlbumSeq(3), - musicbrainz: Some(MbAlbumRef::from_url_str( - "https://musicbrainz.org/release-group/11111111-1111-1111-1111-111111111111" - ).unwrap()), - primary_type: Some(AlbumPrimaryType::Album), - secondary_types: vec![], tracks: vec![ Track { id: TrackId { @@ -217,16 +229,18 @@ macro_rules! full_collection { ], }, Album { - id: AlbumId { - title: "album_title b.c".to_string(), + meta: AlbumMeta { + id: AlbumId { + title: "album_title b.c".to_string(), + }, + date: 2009.into(), + seq: AlbumSeq(2), + musicbrainz: Some(MbAlbumRef::from_url_str( + "https://musicbrainz.org/release-group/11111111-1111-1111-1111-111111111112" + ).unwrap()), + primary_type: Some(AlbumPrimaryType::Album), + secondary_types: vec![], }, - date: 2009.into(), - seq: AlbumSeq(2), - musicbrainz: Some(MbAlbumRef::from_url_str( - "https://musicbrainz.org/release-group/11111111-1111-1111-1111-111111111112" - ).unwrap()), - primary_type: Some(AlbumPrimaryType::Album), - secondary_types: vec![], tracks: vec![ Track { id: TrackId { @@ -256,14 +270,16 @@ macro_rules! full_collection { ], }, Album { - id: AlbumId { - title: "album_title b.d".to_string(), + meta: AlbumMeta { + id: AlbumId { + title: "album_title b.d".to_string(), + }, + date: 2015.into(), + seq: AlbumSeq(4), + musicbrainz: None, + primary_type: Some(AlbumPrimaryType::Album), + secondary_types: vec![], }, - date: 2015.into(), - seq: AlbumSeq(4), - musicbrainz: None, - primary_type: Some(AlbumPrimaryType::Album), - secondary_types: vec![], tracks: vec![ Track { id: TrackId { @@ -295,26 +311,30 @@ macro_rules! full_collection { ], }, Artist { - id: ArtistId { - name: "The Album_Artist ‘C’".to_string(), + meta: ArtistMeta { + id: ArtistId { + name: "The Album_Artist ‘C’".to_string(), + }, + sort: Some(ArtistId { + name: "Album_Artist ‘C’, The".to_string(), + }), + musicbrainz: Some(MbArtistRef::from_url_str( + "https://musicbrainz.org/artist/11111111-1111-1111-1111-111111111111" + ).unwrap()), + properties: HashMap::new(), }, - sort: Some(ArtistId { - name: "Album_Artist ‘C’, The".to_string(), - }), - musicbrainz: Some(MbArtistRef::from_url_str( - "https://musicbrainz.org/artist/11111111-1111-1111-1111-111111111111" - ).unwrap()), - properties: HashMap::new(), albums: vec![ Album { - id: AlbumId { - title: "album_title c.a".to_string(), + meta: AlbumMeta { + id: AlbumId { + title: "album_title c.a".to_string(), + }, + date: 1985.into(), + seq: AlbumSeq(0), + musicbrainz: None, + primary_type: Some(AlbumPrimaryType::Album), + secondary_types: vec![], }, - date: 1985.into(), - seq: AlbumSeq(0), - musicbrainz: None, - primary_type: Some(AlbumPrimaryType::Album), - secondary_types: vec![], tracks: vec![ Track { id: TrackId { @@ -344,14 +364,16 @@ macro_rules! full_collection { ], }, Album { - id: AlbumId { - title: "album_title c.b".to_string(), + meta: AlbumMeta { + id: AlbumId { + title: "album_title c.b".to_string(), + }, + date: 2018.into(), + seq: AlbumSeq(0), + musicbrainz: None, + primary_type: Some(AlbumPrimaryType::Album), + secondary_types: vec![], }, - date: 2018.into(), - seq: AlbumSeq(0), - musicbrainz: None, - primary_type: Some(AlbumPrimaryType::Album), - secondary_types: vec![], tracks: vec![ Track { id: TrackId { @@ -383,22 +405,26 @@ macro_rules! full_collection { ], }, Artist { - id: ArtistId { - name: "Album_Artist ‘D’".to_string(), + meta: ArtistMeta { + id: ArtistId { + name: "Album_Artist ‘D’".to_string(), + }, + sort: None, + musicbrainz: None, + properties: HashMap::new(), }, - sort: None, - musicbrainz: None, - properties: HashMap::new(), albums: vec![ Album { - id: AlbumId { - title: "album_title d.a".to_string(), + meta: AlbumMeta { + id: AlbumId { + title: "album_title d.a".to_string(), + }, + date: 1995.into(), + seq: AlbumSeq(0), + musicbrainz: None, + primary_type: Some(AlbumPrimaryType::Album), + secondary_types: vec![], }, - date: 1995.into(), - seq: AlbumSeq(0), - musicbrainz: None, - primary_type: Some(AlbumPrimaryType::Album), - secondary_types: vec![], tracks: vec![ Track { id: TrackId { @@ -428,14 +454,16 @@ macro_rules! full_collection { ], }, Album { - id: AlbumId { - title: "album_title d.b".to_string(), + meta: AlbumMeta { + id: AlbumId { + title: "album_title d.b".to_string(), + }, + date: 2028.into(), + seq: AlbumSeq(0), + musicbrainz: None, + primary_type: Some(AlbumPrimaryType::Album), + secondary_types: vec![], }, - date: 2028.into(), - seq: AlbumSeq(0), - musicbrainz: None, - primary_type: Some(AlbumPrimaryType::Album), - secondary_types: vec![], tracks: vec![ Track { id: TrackId { diff --git a/src/testmod/library.rs b/src/testmod/library.rs index 7011083..1456f6c 100644 --- a/src/testmod/library.rs +++ b/src/testmod/library.rs @@ -3,22 +3,26 @@ macro_rules! library_collection { () => { vec![ Artist { - id: ArtistId { - name: "Album_Artist ‘A’".to_string(), + meta: ArtistMeta { + id: ArtistId { + name: "Album_Artist ‘A’".to_string(), + }, + sort: None, + musicbrainz: None, + properties: HashMap::new(), }, - sort: None, - musicbrainz: None, - properties: HashMap::new(), albums: vec![ Album { - id: AlbumId { - title: "album_title a.a".to_string(), + meta: AlbumMeta { + id: AlbumId { + title: "album_title a.a".to_string(), + }, + date: 1998.into(), + seq: AlbumSeq(0), + musicbrainz: None, + primary_type: None, + secondary_types: vec![], }, - date: 1998.into(), - seq: AlbumSeq(0), - musicbrainz: None, - primary_type: None, - secondary_types: vec![], tracks: vec![ Track { id: TrackId { @@ -70,14 +74,16 @@ macro_rules! library_collection { ], }, Album { - id: AlbumId { - title: "album_title a.b".to_string(), + meta: AlbumMeta { + id: AlbumId { + title: "album_title a.b".to_string(), + }, + date: (2015, 4).into(), + seq: AlbumSeq(0), + musicbrainz: None, + primary_type: None, + secondary_types: vec![], }, - date: (2015, 4).into(), - seq: AlbumSeq(0), - musicbrainz: None, - primary_type: None, - secondary_types: vec![], tracks: vec![ Track { id: TrackId { @@ -106,22 +112,26 @@ macro_rules! library_collection { ], }, Artist { - id: ArtistId { - name: "Album_Artist ‘B’".to_string(), + meta: ArtistMeta { + id: ArtistId { + name: "Album_Artist ‘B’".to_string(), + }, + sort: None, + musicbrainz: None, + properties: HashMap::new(), }, - sort: None, - musicbrainz: None, - properties: HashMap::new(), albums: vec![ Album { - id: AlbumId { - title: "album_title b.a".to_string(), + meta: AlbumMeta { + id: AlbumId { + title: "album_title b.a".to_string(), + }, + date: (2003, 6, 6).into(), + seq: AlbumSeq(0), + musicbrainz: None, + primary_type: None, + secondary_types: vec![], }, - date: (2003, 6, 6).into(), - seq: AlbumSeq(0), - musicbrainz: None, - primary_type: None, - secondary_types: vec![], tracks: vec![ Track { id: TrackId { @@ -151,14 +161,16 @@ macro_rules! library_collection { ], }, Album { - id: AlbumId { - title: "album_title b.b".to_string(), + meta: AlbumMeta { + id: AlbumId { + title: "album_title b.b".to_string(), + }, + date: 2008.into(), + seq: AlbumSeq(0), + musicbrainz: None, + primary_type: None, + secondary_types: vec![], }, - date: 2008.into(), - seq: AlbumSeq(0), - musicbrainz: None, - primary_type: None, - secondary_types: vec![], tracks: vec![ Track { id: TrackId { @@ -188,14 +200,16 @@ macro_rules! library_collection { ], }, Album { - id: AlbumId { - title: "album_title b.c".to_string(), + meta: AlbumMeta { + id: AlbumId { + title: "album_title b.c".to_string(), + }, + date: 2009.into(), + seq: AlbumSeq(0), + musicbrainz: None, + primary_type: None, + secondary_types: vec![], }, - date: 2009.into(), - seq: AlbumSeq(0), - musicbrainz: None, - primary_type: None, - secondary_types: vec![], tracks: vec![ Track { id: TrackId { @@ -225,14 +239,16 @@ macro_rules! library_collection { ], }, Album { - id: AlbumId { - title: "album_title b.d".to_string(), + meta: AlbumMeta { + id: AlbumId { + title: "album_title b.d".to_string(), + }, + date: 2015.into(), + seq: AlbumSeq(0), + musicbrainz: None, + primary_type: None, + secondary_types: vec![], }, - date: 2015.into(), - seq: AlbumSeq(0), - musicbrainz: None, - primary_type: None, - secondary_types: vec![], tracks: vec![ Track { id: TrackId { @@ -264,24 +280,28 @@ macro_rules! library_collection { ], }, Artist { - id: ArtistId { - name: "The Album_Artist ‘C’".to_string(), + meta: ArtistMeta { + id: ArtistId { + name: "The Album_Artist ‘C’".to_string(), + }, + sort: Some(ArtistId { + name: "Album_Artist ‘C’, The".to_string(), + }), + musicbrainz: None, + properties: HashMap::new(), }, - sort: Some(ArtistId { - name: "Album_Artist ‘C’, The".to_string(), - }), - musicbrainz: None, - properties: HashMap::new(), albums: vec![ Album { - id: AlbumId { - title: "album_title c.a".to_string(), + meta: AlbumMeta { + id: AlbumId { + title: "album_title c.a".to_string(), + }, + date: 1985.into(), + seq: AlbumSeq(0), + musicbrainz: None, + primary_type: None, + secondary_types: vec![], }, - date: 1985.into(), - seq: AlbumSeq(0), - musicbrainz: None, - primary_type: None, - secondary_types: vec![], tracks: vec![ Track { id: TrackId { @@ -311,14 +331,16 @@ macro_rules! library_collection { ], }, Album { - id: AlbumId { - title: "album_title c.b".to_string(), + meta: AlbumMeta { + id: AlbumId { + title: "album_title c.b".to_string(), + }, + date: 2018.into(), + seq: AlbumSeq(0), + musicbrainz: None, + primary_type: None, + secondary_types: vec![], }, - date: 2018.into(), - seq: AlbumSeq(0), - musicbrainz: None, - primary_type: None, - secondary_types: vec![], tracks: vec![ Track { id: TrackId { @@ -350,22 +372,26 @@ macro_rules! library_collection { ], }, Artist { - id: ArtistId { - name: "Album_Artist ‘D’".to_string(), + meta: ArtistMeta { + id: ArtistId { + name: "Album_Artist ‘D’".to_string(), + }, + sort: None, + musicbrainz: None, + properties: HashMap::new(), }, - sort: None, - musicbrainz: None, - properties: HashMap::new(), albums: vec![ Album { - id: AlbumId { - title: "album_title d.a".to_string(), + meta: AlbumMeta { + id: AlbumId { + title: "album_title d.a".to_string(), + }, + date: 1995.into(), + seq: AlbumSeq(0), + musicbrainz: None, + primary_type: None, + secondary_types: vec![], }, - date: 1995.into(), - seq: AlbumSeq(0), - musicbrainz: None, - primary_type: None, - secondary_types: vec![], tracks: vec![ Track { id: TrackId { @@ -395,14 +421,16 @@ macro_rules! library_collection { ], }, Album { - id: AlbumId { - title: "album_title d.b".to_string(), + meta: AlbumMeta { + id: AlbumId { + title: "album_title d.b".to_string(), + }, + date: 2028.into(), + seq: AlbumSeq(0), + musicbrainz: None, + primary_type: None, + secondary_types: vec![], }, - date: 2028.into(), - seq: AlbumSeq(0), - musicbrainz: None, - primary_type: None, - secondary_types: vec![], tracks: vec![ Track { id: TrackId { diff --git a/src/tui/app/machine/browse.rs b/src/tui/app/machine/browse.rs index 951d430..cd07769 100644 --- a/src/tui/app/machine/browse.rs +++ b/src/tui/app/machine/browse.rs @@ -93,13 +93,13 @@ impl IAppInteractBrowse for AppMachine { let (matches_tx, matches_rx) = mpsc::channel::(); - match artist.musicbrainz { + match artist.meta.musicbrainz { Some(ref mbid) => { let arid = mbid.mbid(); let mut album_iter = artist.albums.iter().peekable(); while let Some(album) = album_iter.next() { - if album.musicbrainz.is_some() { + if album.meta.musicbrainz.is_some() { continue; } diff --git a/src/tui/app/machine/matches.rs b/src/tui/app/machine/matches.rs index adeaea8..e632ffc 100644 --- a/src/tui/app/machine/matches.rs +++ b/src/tui/app/machine/matches.rs @@ -194,8 +194,8 @@ mod tests { let album_match_1_1 = Match::new(100, album_1_1); let mut album_1_2 = album_1.clone(); - album_1_2.id.title.push_str(" extra title part"); - album_1_2.secondary_types.pop(); + album_1_2.meta.id.title.push_str(" extra title part"); + album_1_2.meta.secondary_types.pop(); let album_match_1_2 = Match::new(100, album_1_2); let list = vec![album_match_1_1.clone(), album_match_1_2.clone()]; diff --git a/src/tui/app/machine/search.rs b/src/tui/app/machine/search.rs index c13f6a5..17ed189 100644 --- a/src/tui/app/machine/search.rs +++ b/src/tui/app/machine/search.rs @@ -181,10 +181,10 @@ impl IAppInteractSearchPrivate for AppMachine { } fn predicate_artists(case_sens: bool, char_sens: bool, search: &str, probe: &Artist) -> bool { - let name = Self::normalize_search(&probe.id.name, !case_sens, !char_sens); + let name = Self::normalize_search(&probe.meta.id.name, !case_sens, !char_sens); let mut result = name.starts_with(search); - if let Some(ref probe_sort) = probe.sort { + if let Some(ref probe_sort) = probe.meta.sort { if !result { let name = Self::normalize_search(&probe_sort.name, !case_sens, !char_sens); result = name.starts_with(search); @@ -195,7 +195,7 @@ impl IAppInteractSearchPrivate for AppMachine { } fn predicate_albums(case_sens: bool, char_sens: bool, search: &str, probe: &Album) -> bool { - Self::predicate_title(case_sens, char_sens, search, &probe.id.title) + Self::predicate_title(case_sens, char_sens, search, &probe.meta.id.title) } fn predicate_tracks(case_sens: bool, char_sens: bool, search: &str, probe: &Track) -> bool { diff --git a/src/tui/lib/external/musicbrainz/mod.rs b/src/tui/lib/external/musicbrainz/mod.rs index 903c48c..3a7673f 100644 --- a/src/tui/lib/external/musicbrainz/mod.rs +++ b/src/tui/lib/external/musicbrainz/mod.rs @@ -33,7 +33,7 @@ impl MusicBrainz { impl IMusicBrainz for MusicBrainz { fn search_artist(&mut self, artist: &Artist) -> Result>, Error> { - let query = SearchArtistRequest::new().string(&artist.id.name); + let query = SearchArtistRequest::new().string(&artist.meta.id.name); let mb_response = self.client.search_artist(query)?; @@ -51,14 +51,14 @@ impl IMusicBrainz for MusicBrainz { ) -> 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); + let date = AlbumDate::new(album.meta.date.year, None, None); let query = SearchReleaseGroupRequest::new() .arid(arid) .and() .first_release_date(&date) .and() - .release_group(&album.id.title); + .release_group(&album.meta.id.title); let mb_response = self.client.search_release_group(query)?; diff --git a/src/tui/testmod.rs b/src/tui/testmod.rs index 5340087..a5a6710 100644 --- a/src/tui/testmod.rs +++ b/src/tui/testmod.rs @@ -1,8 +1,8 @@ use std::collections::HashMap; use musichoard::collection::{ - album::{Album, AlbumId, AlbumPrimaryType, AlbumSeq}, - artist::{Artist, ArtistId}, + album::{Album, AlbumId, AlbumMeta, AlbumPrimaryType, AlbumSeq}, + artist::{Artist, ArtistId, ArtistMeta}, musicbrainz::{MbAlbumRef, MbArtistRef}, track::{Track, TrackFormat, TrackId, TrackNum, TrackQuality}, }; diff --git a/src/tui/ui/browse.rs b/src/tui/ui/browse.rs index 616a5fe..22df107 100644 --- a/src/tui/ui/browse.rs +++ b/src/tui/ui/browse.rs @@ -120,7 +120,7 @@ impl<'a, 'b> ArtistState<'a, 'b> { let list = List::new( artists .iter() - .map(|a| ListItem::new(a.id.name.as_str())) + .map(|a| ListItem::new(a.meta.id.name.as_str())) .collect::>(), ); @@ -158,12 +158,12 @@ impl<'a, 'b> AlbumState<'a, 'b> { Date: {}\n\ Type: {}\n\ Status: {}", - album.map(|a| a.id.title.as_str()).unwrap_or(""), + album.map(|a| a.meta.id.title.as_str()).unwrap_or(""), album - .map(|a| UiDisplay::display_date(&a.date, &a.seq)) + .map(|a| UiDisplay::display_date(&a.meta.date, &a.meta.seq)) .unwrap_or_default(), album - .map(|a| UiDisplay::display_type(&a.primary_type, &a.secondary_types)) + .map(|a| UiDisplay::display_type(&a.meta.primary_type, &a.meta.secondary_types)) .unwrap_or_default(), album .map(|a| UiDisplay::display_album_status(&a.get_status())) @@ -180,10 +180,10 @@ impl<'a, 'b> AlbumState<'a, 'b> { fn to_list_item(album: &Album) -> ListItem { let line = match album.get_status() { - AlbumStatus::None => Line::raw(album.id.title.as_str()), + AlbumStatus::None => Line::raw(album.meta.id.title.as_str()), AlbumStatus::Owned(format) => match format { - TrackFormat::Mp3 => Line::styled(album.id.title.as_str(), UiColor::FG_WARN), - TrackFormat::Flac => Line::styled(album.id.title.as_str(), UiColor::FG_GOOD), + 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), }, }; ListItem::new(line) diff --git a/src/tui/ui/display.rs b/src/tui/ui/display.rs index 78f0927..1d15413 100644 --- a/src/tui/ui/display.rs +++ b/src/tui/ui/display.rs @@ -99,14 +99,14 @@ impl UiDisplay { } pub fn display_artist_matching(artist: &Artist) -> String { - format!("Matching artist: {}", &artist.id.name) + format!("Matching artist: {}", &artist.meta.id.name) } pub fn display_album_matching(album: &Album) -> String { format!( "Matching album: {} | {}", - UiDisplay::display_album_date(&album.date), - &album.id.title + UiDisplay::display_album_date(&album.meta.date), + &album.meta.id.title ) } @@ -128,7 +128,7 @@ impl UiDisplay { match match_option { MatchOption::Match(match_artist) => format!( "{}{} ({}%)", - &match_artist.item.id.name, + &match_artist.item.meta.id.name, &match_artist .disambiguation .as_ref() @@ -144,11 +144,11 @@ impl UiDisplay { match match_option { MatchOption::Match(match_album) => format!( "{:010} | {} [{}] ({}%)", - UiDisplay::display_album_date(&match_album.item.date), - &match_album.item.id.title, + UiDisplay::display_album_date(&match_album.item.meta.date), + &match_album.item.meta.id.title, UiDisplay::display_type( - &match_album.item.primary_type, - &match_album.item.secondary_types + &match_album.item.meta.primary_type, + &match_album.item.meta.secondary_types ), match_album.score, ), diff --git a/src/tui/ui/info.rs b/src/tui/ui/info.rs index 41a2e23..a604e99 100644 --- a/src/tui/ui/info.rs +++ b/src/tui/ui/info.rs @@ -72,12 +72,12 @@ impl<'a> ArtistOverlay<'a> { "Artist: {}\n\n{item_indent}\ MusicBrainz: {}\n{item_indent}\ Properties: {}", - artist.map(|a| a.id.name.as_str()).unwrap_or(""), + artist.map(|a| a.meta.id.name.as_str()).unwrap_or(""), artist - .and_then(|a| a.musicbrainz.as_ref().map(|mb| mb.url().as_str())) + .and_then(|a| a.meta.musicbrainz.as_ref().map(|mb| mb.url().as_str())) .unwrap_or(""), Self::opt_hashmap_to_string( - artist.map(|a| &a.properties), + artist.map(|a| &a.meta.properties), &double_item_indent, &double_list_indent ), @@ -100,9 +100,9 @@ impl<'a> AlbumOverlay<'a> { let properties = Paragraph::new(format!( "Album: {}\n\n{item_indent}\ MusicBrainz: {}", - album.map(|a| a.id.title.as_str()).unwrap_or(""), + album.map(|a| a.meta.id.title.as_str()).unwrap_or(""), album - .and_then(|a| a.musicbrainz.as_ref().map(|mb| mb.url().as_str())) + .and_then(|a| a.meta.musicbrainz.as_ref().map(|mb| mb.url().as_str())) .unwrap_or(""), )); diff --git a/tests/database/json.rs b/tests/database/json.rs index e803557..2ee6755 100644 --- a/tests/database/json.rs +++ b/tests/database/json.rs @@ -18,7 +18,7 @@ fn expected() -> Collection { let mut expected = COLLECTION.to_owned(); for artist in expected.iter_mut() { for album in artist.albums.iter_mut() { - album.date = AlbumDate::default(); + album.meta.date = AlbumDate::default(); album.tracks.clear(); } } diff --git a/tests/testlib.rs b/tests/testlib.rs index 3261e0a..09f964d 100644 --- a/tests/testlib.rs +++ b/tests/testlib.rs @@ -2,8 +2,8 @@ use once_cell::sync::Lazy; use std::collections::HashMap; use musichoard::collection::{ - album::{Album, AlbumId, AlbumPrimaryType, AlbumSecondaryType, AlbumSeq}, - artist::{Artist, ArtistId}, + album::{Album, AlbumId, AlbumMeta, AlbumPrimaryType, AlbumSecondaryType, AlbumSeq}, + artist::{Artist, ArtistId, ArtistMeta}, musicbrainz::MbArtistRef, track::{Track, TrackFormat, TrackId, TrackNum, TrackQuality}, Collection, @@ -12,35 +12,39 @@ use musichoard::collection::{ pub static COLLECTION: Lazy> = Lazy::new(|| -> Collection { vec![ Artist { - id: ArtistId { - name: String::from("Аркона"), - }, - sort: Some(ArtistId{ - name: String::from("Arkona") - }), - musicbrainz: Some(MbArtistRef::from_url_str( - "https://musicbrainz.org/artist/baad262d-55ef-427a-83c7-f7530964f212" - ).unwrap()), - properties: HashMap::from([ - (String::from("MusicButler"), vec![ - String::from("https://www.musicbutler.io/artist-page/283448581"), - ]), - (String::from("Bandcamp"), vec![ - String::from("https://arkonamoscow.bandcamp.com/"), - ]), - (String::from("Qobuz"), vec![String::from( - "https://www.qobuz.com/nl-nl/interpreter/arkona/download-streaming-albums", - )]), - ]), - albums: vec![Album { - id: AlbumId { - title: String::from("Slovo"), + meta: ArtistMeta { + id: ArtistId { + name: String::from("Аркона"), + }, + sort: Some(ArtistId{ + name: String::from("Arkona") + }), + musicbrainz: Some(MbArtistRef::from_url_str( + "https://musicbrainz.org/artist/baad262d-55ef-427a-83c7-f7530964f212" + ).unwrap()), + properties: HashMap::from([ + (String::from("MusicButler"), vec![ + String::from("https://www.musicbutler.io/artist-page/283448581"), + ]), + (String::from("Bandcamp"), vec![ + String::from("https://arkonamoscow.bandcamp.com/"), + ]), + (String::from("Qobuz"), vec![String::from( + "https://www.qobuz.com/nl-nl/interpreter/arkona/download-streaming-albums", + )]), + ]), + }, + albums: vec![Album { + meta: AlbumMeta { + id: AlbumId { + title: String::from("Slovo"), + }, + date: 2011.into(), + seq: AlbumSeq(0), + musicbrainz: None, + primary_type: Some(AlbumPrimaryType::Album), + secondary_types: vec![], }, - date: 2011.into(), - seq: AlbumSeq(0), - musicbrainz: None, - primary_type: Some(AlbumPrimaryType::Album), - secondary_types: vec![], tracks: vec![ Track { id: TrackId { @@ -200,31 +204,35 @@ pub static COLLECTION: Lazy> = Lazy::new(|| -> Collection { }], }, Artist { - id: ArtistId { - name: String::from("Eluveitie"), - }, - sort: None, - musicbrainz: Some(MbArtistRef::from_url_str( - "https://musicbrainz.org/artist/8000598a-5edb-401c-8e6d-36b167feaf38" - ).unwrap()), - properties: HashMap::from([ - (String::from("MusicButler"), vec![ - String::from("https://www.musicbutler.io/artist-page/269358403"), + meta: ArtistMeta { + id: ArtistId { + name: String::from("Eluveitie"), + }, + sort: None, + musicbrainz: Some(MbArtistRef::from_url_str( + "https://musicbrainz.org/artist/8000598a-5edb-401c-8e6d-36b167feaf38" + ).unwrap()), + properties: HashMap::from([ + (String::from("MusicButler"), vec![ + String::from("https://www.musicbutler.io/artist-page/269358403"), + ]), + (String::from("Qobuz"), vec![String::from( + "https://www.qobuz.com/nl-nl/interpreter/eluveitie/download-streaming-albums", + )]), ]), - (String::from("Qobuz"), vec![String::from( - "https://www.qobuz.com/nl-nl/interpreter/eluveitie/download-streaming-albums", - )]), - ]), + }, albums: vec![ Album { - id: AlbumId { - title: String::from("Vên [re‐recorded]"), + meta: AlbumMeta { + id: AlbumId { + title: String::from("Vên [re‐recorded]"), + }, + date: 2004.into(), + seq: AlbumSeq(0), + musicbrainz: None, + primary_type: Some(AlbumPrimaryType::Ep), + secondary_types: vec![], }, - date: 2004.into(), - seq: AlbumSeq(0), - musicbrainz: None, - primary_type: Some(AlbumPrimaryType::Ep), - secondary_types: vec![], tracks: vec![ Track { id: TrackId { @@ -295,14 +303,16 @@ pub static COLLECTION: Lazy> = Lazy::new(|| -> Collection { ], }, Album { - id: AlbumId { - title: String::from("Slania"), + meta: AlbumMeta { + id: AlbumId { + title: String::from("Slania"), + }, + date: 2008.into(), + seq: AlbumSeq(0), + musicbrainz: None, + primary_type: Some(AlbumPrimaryType::Album), + secondary_types: vec![], }, - date: 2008.into(), - seq: AlbumSeq(0), - musicbrainz: None, - primary_type: Some(AlbumPrimaryType::Album), - secondary_types: vec![], tracks: vec![ Track { id: TrackId { @@ -441,30 +451,34 @@ pub static COLLECTION: Lazy> = Lazy::new(|| -> Collection { ], }, Artist { - id: ArtistId { - name: String::from("Frontside"), - }, - sort: None, - musicbrainz: Some(MbArtistRef::from_url_str( - "https://musicbrainz.org/artist/3a901353-fccd-4afd-ad01-9c03f451b490" - ).unwrap()), - properties: HashMap::from([ - (String::from("MusicButler"), vec![ - String::from("https://www.musicbutler.io/artist-page/826588800"), - ]), - (String::from("Qobuz"), vec![String::from( - "https://www.qobuz.com/nl-nl/interpreter/frontside/download-streaming-albums", - )]), - ]), - albums: vec![Album { - id: AlbumId { - title: String::from("…nasze jest królestwo, potęga i chwała na wieki…"), + meta: ArtistMeta { + id: ArtistId { + name: String::from("Frontside"), + }, + sort: None, + musicbrainz: Some(MbArtistRef::from_url_str( + "https://musicbrainz.org/artist/3a901353-fccd-4afd-ad01-9c03f451b490" + ).unwrap()), + properties: HashMap::from([ + (String::from("MusicButler"), vec![ + String::from("https://www.musicbutler.io/artist-page/826588800"), + ]), + (String::from("Qobuz"), vec![String::from( + "https://www.qobuz.com/nl-nl/interpreter/frontside/download-streaming-albums", + )]), + ]), + }, + albums: vec![Album { + meta: AlbumMeta { + id: AlbumId { + title: String::from("…nasze jest królestwo, potęga i chwała na wieki…"), + }, + date: 2001.into(), + seq: AlbumSeq(0), + musicbrainz: None, + primary_type: Some(AlbumPrimaryType::Album), + secondary_types: vec![], }, - date: 2001.into(), - seq: AlbumSeq(0), - musicbrainz: None, - primary_type: Some(AlbumPrimaryType::Album), - secondary_types: vec![], tracks: vec![ Track { id: TrackId { @@ -591,32 +605,36 @@ pub static COLLECTION: Lazy> = Lazy::new(|| -> Collection { }], }, Artist { - id: ArtistId { - name: String::from("Heaven’s Basement"), - }, - sort: Some(ArtistId { - name: String::from("Heaven’s Basement"), - }), - musicbrainz: Some(MbArtistRef::from_url_str( - "https://musicbrainz.org/artist/c2c4d56a-d599-4a18-bd2f-ae644e2198cc" - ).unwrap()), - properties: HashMap::from([ - (String::from("MusicButler"), vec![ - String::from("https://www.musicbutler.io/artist-page/291158685"), - ]), - (String::from("Qobuz"), vec![String::from( - "https://www.qobuz.com/nl-nl/interpreter/heaven-s-basement/download-streaming-albums", - )]), - ]), - albums: vec![Album { - id: AlbumId { - title: String::from("Paper Plague"), + meta: ArtistMeta { + id: ArtistId { + name: String::from("Heaven’s Basement"), + }, + sort: Some(ArtistId { + name: String::from("Heaven’s Basement"), + }), + musicbrainz: Some(MbArtistRef::from_url_str( + "https://musicbrainz.org/artist/c2c4d56a-d599-4a18-bd2f-ae644e2198cc" + ).unwrap()), + properties: HashMap::from([ + (String::from("MusicButler"), vec![ + String::from("https://www.musicbutler.io/artist-page/291158685"), + ]), + (String::from("Qobuz"), vec![String::from( + "https://www.qobuz.com/nl-nl/interpreter/heaven-s-basement/download-streaming-albums", + )]), + ]), + }, + albums: vec![Album { + meta: AlbumMeta { + id: AlbumId { + title: String::from("Paper Plague"), + }, + date: 2011.into(), + seq: AlbumSeq(0), + musicbrainz: None, + primary_type: None, + secondary_types: vec![], }, - date: 2011.into(), - seq: AlbumSeq(0), - musicbrainz: None, - primary_type: None, - secondary_types: vec![], tracks: vec![ Track { id: TrackId { @@ -631,14 +649,16 @@ pub static COLLECTION: Lazy> = Lazy::new(|| -> Collection { }, ], }, Album { - id: AlbumId { - title: String::from("Unbreakable"), + meta: AlbumMeta { + id: AlbumId { + title: String::from("Unbreakable"), + }, + date: 2011.into(), + seq: AlbumSeq(0), + musicbrainz: None, + primary_type: Some(AlbumPrimaryType::Album), + secondary_types: vec![], }, - date: 2011.into(), - seq: AlbumSeq(0), - musicbrainz: None, - primary_type: Some(AlbumPrimaryType::Album), - secondary_types: vec![], tracks: vec![ Track { id: TrackId { @@ -721,31 +741,35 @@ pub static COLLECTION: Lazy> = Lazy::new(|| -> Collection { }], }, Artist { - id: ArtistId { - name: String::from("Metallica"), - }, - sort: None, - musicbrainz: Some(MbArtistRef::from_url_str( - "https://musicbrainz.org/artist/65f4f0c5-ef9e-490c-aee3-909e7ae6b2ab" - ).unwrap()), - properties: HashMap::from([ - (String::from("MusicButler"), vec![ - String::from("https://www.musicbutler.io/artist-page/3996865"), + meta: ArtistMeta { + id: ArtistId { + name: String::from("Metallica"), + }, + sort: None, + musicbrainz: Some(MbArtistRef::from_url_str( + "https://musicbrainz.org/artist/65f4f0c5-ef9e-490c-aee3-909e7ae6b2ab" + ).unwrap()), + properties: HashMap::from([ + (String::from("MusicButler"), vec![ + String::from("https://www.musicbutler.io/artist-page/3996865"), + ]), + (String::from("Qobuz"), vec![String::from( + "https://www.qobuz.com/nl-nl/interpreter/metallica/download-streaming-albums", + )]), ]), - (String::from("Qobuz"), vec![String::from( - "https://www.qobuz.com/nl-nl/interpreter/metallica/download-streaming-albums", - )]), - ]), + }, albums: vec![ Album { - id: AlbumId { - title: String::from("Ride the Lightning"), + meta: AlbumMeta { + id: AlbumId { + title: String::from("Ride the Lightning"), + }, + date: 1984.into(), + seq: AlbumSeq(0), + musicbrainz: None, + primary_type: Some(AlbumPrimaryType::Album), + secondary_types: vec![], }, - date: 1984.into(), - seq: AlbumSeq(0), - musicbrainz: None, - primary_type: Some(AlbumPrimaryType::Album), - secondary_types: vec![], tracks: vec![ Track { id: TrackId { @@ -838,14 +862,16 @@ pub static COLLECTION: Lazy> = Lazy::new(|| -> Collection { ], }, Album { - id: AlbumId { - title: String::from("S&M"), + meta: AlbumMeta { + id: AlbumId { + title: String::from("S&M"), + }, + date: 1999.into(), + seq: AlbumSeq(0), + musicbrainz: None, + primary_type: Some(AlbumPrimaryType::Album), + secondary_types: vec![AlbumSecondaryType::Live], }, - date: 1999.into(), - seq: AlbumSeq(0), - musicbrainz: None, - primary_type: Some(AlbumPrimaryType::Album), - secondary_types: vec![AlbumSecondaryType::Live], tracks: vec![ Track { id: TrackId { -- 2.45.2 From c04fdcd23ceef08c918661219cb212d33ec52017 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sat, 31 Aug 2024 21:51:38 +0200 Subject: [PATCH 2/6] Use only metadata when collections not needed --- src/core/collection/album.rs | 92 ++++++++++---- src/core/collection/artist.rs | 155 ++++++++++++++++------- src/tui/app/machine/browse.rs | 33 +++-- src/tui/app/machine/matches.rs | 16 +-- src/tui/app/mod.rs | 18 +-- src/tui/lib/external/musicbrainz/mod.rs | 24 ++-- src/tui/lib/interface/musicbrainz/mod.rs | 8 +- src/tui/ui/display.rs | 28 ++-- src/tui/ui/matches.rs | 10 +- src/tui/ui/mod.rs | 16 +-- 10 files changed, 257 insertions(+), 143 deletions(-) diff --git a/src/core/collection/album.rs b/src/core/collection/album.rs index ab2403e..1854d7d 100644 --- a/src/core/collection/album.rs +++ b/src/core/collection/album.rs @@ -145,20 +145,13 @@ impl Album { secondary_types: Vec, ) -> Self { Album { - meta: AlbumMeta { - id: id.into(), - date: date.into(), - seq: AlbumSeq::default(), - musicbrainz: None, - primary_type, - secondary_types, - }, + meta: AlbumMeta::new(id, date, primary_type, secondary_types), tracks: vec![], } } pub fn get_sort_key(&self) -> (&AlbumDate, &AlbumSeq, &AlbumId) { - (&self.meta.date, &self.meta.seq, &self.meta.id) + self.meta.get_sort_key() } pub fn get_status(&self) -> AlbumStatus { @@ -166,19 +159,19 @@ impl Album { } pub fn set_seq(&mut self, seq: AlbumSeq) { - self.meta.seq = seq; + self.meta.set_seq(seq); } pub fn clear_seq(&mut self) { - self.meta.seq = AlbumSeq::default(); + self.meta.clear_seq(); } pub fn set_musicbrainz_ref(&mut self, mbref: MbAlbumRef) { - _ = self.meta.musicbrainz.insert(mbref); + self.meta.set_musicbrainz_ref(mbref); } pub fn clear_musicbrainz_ref(&mut self) { - _ = self.meta.musicbrainz.take(); + self.meta.clear_musicbrainz_ref(); } } @@ -190,26 +183,79 @@ impl PartialOrd for Album { impl Ord for Album { fn cmp(&self, other: &Self) -> std::cmp::Ordering { - self.get_sort_key().cmp(&other.get_sort_key()) + self.meta.cmp(&other.meta) } } impl Merge for Album { fn merge_in_place(&mut self, other: Self) { - assert_eq!(self.meta.id, other.meta.id); - self.meta.seq = std::cmp::max(self.meta.seq, other.meta.seq); - - self.meta.musicbrainz = self.meta.musicbrainz.take().or(other.meta.musicbrainz); - self.meta.primary_type = self.meta.primary_type.take().or(other.meta.primary_type); - self.meta - .secondary_types - .merge_in_place(other.meta.secondary_types); - + self.meta.merge_in_place(other.meta); let tracks = mem::take(&mut self.tracks); self.tracks = MergeSorted::new(tracks.into_iter(), other.tracks.into_iter()).collect(); } } +impl AlbumMeta { + pub fn new, Date: Into>( + id: Id, + date: Date, + primary_type: Option, + secondary_types: Vec, + ) -> Self { + AlbumMeta { + id: id.into(), + date: date.into(), + seq: AlbumSeq::default(), + musicbrainz: None, + primary_type, + secondary_types, + } + } + + pub fn get_sort_key(&self) -> (&AlbumDate, &AlbumSeq, &AlbumId) { + (&self.date, &self.seq, &self.id) + } + + pub fn set_seq(&mut self, seq: AlbumSeq) { + self.seq = seq; + } + + pub fn clear_seq(&mut self) { + self.seq = AlbumSeq::default(); + } + + pub fn set_musicbrainz_ref(&mut self, mbref: MbAlbumRef) { + _ = self.musicbrainz.insert(mbref); + } + + pub fn clear_musicbrainz_ref(&mut self) { + _ = self.musicbrainz.take(); + } +} + +impl PartialOrd for AlbumMeta { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl Ord for AlbumMeta { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + self.get_sort_key().cmp(&other.get_sort_key()) + } +} + +impl Merge for AlbumMeta { + fn merge_in_place(&mut self, other: Self) { + assert_eq!(self.id, other.id); + self.seq = std::cmp::max(self.seq, other.seq); + + self.musicbrainz = self.musicbrainz.take().or(other.musicbrainz); + self.primary_type = self.primary_type.take().or(other.primary_type); + self.secondary_types.merge_in_place(other.secondary_types); + } +} + impl> From for AlbumId { fn from(value: S) -> Self { AlbumId::new(value) diff --git a/src/core/collection/artist.rs b/src/core/collection/artist.rs index c19d8b0..964a746 100644 --- a/src/core/collection/artist.rs +++ b/src/core/collection/artist.rs @@ -44,34 +44,29 @@ impl Artist { /// Create new [`Artist`] with the given [`ArtistId`]. pub fn new>(id: Id) -> Self { Artist { - meta: ArtistMeta { - id: id.into(), - sort: None, - musicbrainz: None, - properties: HashMap::new(), - }, + meta: ArtistMeta::new(id), albums: vec![], } } pub fn get_sort_key(&self) -> (&ArtistId,) { - (self.meta.sort.as_ref().unwrap_or(&self.meta.id),) + self.meta.get_sort_key() } pub fn set_sort_key>(&mut self, sort: SORT) { - self.meta.sort = Some(sort.into()); + self.meta.set_sort_key(sort); } pub fn clear_sort_key(&mut self) { - _ = self.meta.sort.take(); + self.meta.clear_sort_key(); } pub fn set_musicbrainz_ref(&mut self, mbref: MbArtistRef) { - _ = self.meta.musicbrainz.insert(mbref); + self.meta.set_musicbrainz_ref(mbref); } pub fn clear_musicbrainz_ref(&mut self) { - _ = self.meta.musicbrainz.take(); + self.meta.clear_musicbrainz_ref(); } // In the functions below, it would be better to use `contains` instead of `iter().any`, but for @@ -79,43 +74,19 @@ impl Artist { // https://stackoverflow.com/questions/48985924/why-does-a-str-not-coerce-to-a-string-when-using-veccontains pub fn add_to_property + Into>(&mut self, property: S, values: Vec) { - match self.meta.properties.get_mut(property.as_ref()) { - Some(container) => { - container.append( - &mut values - .into_iter() - .filter(|val| !container.iter().any(|x| x == val.as_ref())) - .map(|val| val.into()) - .collect(), - ); - } - None => { - self.meta.properties.insert( - property.into(), - values.into_iter().map(|s| s.into()).collect(), - ); - } - } + self.meta.add_to_property(property, values); } pub fn remove_from_property>(&mut self, property: S, values: Vec) { - if let Some(container) = self.meta.properties.get_mut(property.as_ref()) { - container.retain(|val| !values.iter().any(|x| x.as_ref() == val)); - if container.is_empty() { - self.meta.properties.remove(property.as_ref()); - } - } + self.meta.remove_from_property(property, values); } pub fn set_property + Into>(&mut self, property: S, values: Vec) { - self.meta.properties.insert( - property.into(), - values.into_iter().map(|s| s.into()).collect(), - ); + self.meta.set_property(property, values); } pub fn clear_property>(&mut self, property: S) { - self.meta.properties.remove(property.as_ref()); + self.meta.clear_property(property); } } @@ -127,23 +98,115 @@ impl PartialOrd for Artist { impl Ord for Artist { fn cmp(&self, other: &Self) -> std::cmp::Ordering { - self.get_sort_key().cmp(&other.get_sort_key()) + self.meta.cmp(&other.meta) } } impl Merge for Artist { fn merge_in_place(&mut self, other: Self) { - assert_eq!(self.meta.id, other.meta.id); - - self.meta.sort = self.meta.sort.take().or(other.meta.sort); - self.meta.musicbrainz = self.meta.musicbrainz.take().or(other.meta.musicbrainz); - self.meta.properties.merge_in_place(other.meta.properties); - + self.meta.merge_in_place(other.meta); let albums = mem::take(&mut self.albums); self.albums = MergeCollections::merge_iter(albums, other.albums); } } +impl ArtistMeta { + pub fn new>(id: Id) -> Self { + ArtistMeta { + id: id.into(), + sort: None, + musicbrainz: None, + properties: HashMap::new(), + } + } + + pub fn get_sort_key(&self) -> (&ArtistId,) { + (self.sort.as_ref().unwrap_or(&self.id),) + } + + pub fn set_sort_key>(&mut self, sort: SORT) { + self.sort = Some(sort.into()); + } + + pub fn clear_sort_key(&mut self) { + _ = self.sort.take(); + } + + pub fn set_musicbrainz_ref(&mut self, mbref: MbArtistRef) { + _ = self.musicbrainz.insert(mbref); + } + + pub fn clear_musicbrainz_ref(&mut self) { + _ = self.musicbrainz.take(); + } + + // In the functions below, it would be better to use `contains` instead of `iter().any`, but for + // type reasons that does not work: + // https://stackoverflow.com/questions/48985924/why-does-a-str-not-coerce-to-a-string-when-using-veccontains + + pub fn add_to_property + Into>(&mut self, property: S, values: Vec) { + match self.properties.get_mut(property.as_ref()) { + Some(container) => { + container.append( + &mut values + .into_iter() + .filter(|val| !container.iter().any(|x| x == val.as_ref())) + .map(|val| val.into()) + .collect(), + ); + } + None => { + self.properties.insert( + property.into(), + values.into_iter().map(|s| s.into()).collect(), + ); + } + } + } + + pub fn remove_from_property>(&mut self, property: S, values: Vec) { + if let Some(container) = self.properties.get_mut(property.as_ref()) { + container.retain(|val| !values.iter().any(|x| x.as_ref() == val)); + if container.is_empty() { + self.properties.remove(property.as_ref()); + } + } + } + + pub fn set_property + Into>(&mut self, property: S, values: Vec) { + self.properties.insert( + property.into(), + values.into_iter().map(|s| s.into()).collect(), + ); + } + + pub fn clear_property>(&mut self, property: S) { + self.properties.remove(property.as_ref()); + } +} + +impl PartialOrd for ArtistMeta { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl Ord for ArtistMeta { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + self.get_sort_key().cmp(&other.get_sort_key()) + } +} + +impl Merge for ArtistMeta { + fn merge_in_place(&mut self, other: Self) { + assert_eq!(self.id, other.id); + + self.sort = self.sort.take().or(other.sort); + self.musicbrainz = self.musicbrainz.take().or(other.musicbrainz); + self.properties.merge_in_place(other.properties); + } +} + impl> From for ArtistId { fn from(value: S) -> Self { ArtistId::new(value) diff --git a/src/tui/app/machine/browse.rs b/src/tui/app/machine/browse.rs index cd07769..344e3ea 100644 --- a/src/tui/app/machine/browse.rs +++ b/src/tui/app/machine/browse.rs @@ -103,9 +103,13 @@ impl IAppInteractBrowse for AppMachine { continue; } - match self.inner.musicbrainz.search_release_group(arid, album) { + match self + .inner + .musicbrainz + .search_release_group(arid, &album.meta) + { Ok(list) => matches_tx - .send(AppMatchesInfo::album(album.clone(), list)) + .send(AppMatchesInfo::album(album.meta.clone(), list)) .expect("send fails only if receiver is disconnected"), Err(err) => return AppMachine::error(self.inner, err.to_string()).into(), } @@ -115,9 +119,9 @@ impl IAppInteractBrowse for AppMachine { } } } - None => match self.inner.musicbrainz.search_artist(artist) { + None => match self.inner.musicbrainz.search_artist(&artist.meta) { Ok(list) => matches_tx - .send(AppMatchesInfo::artist(artist.clone(), list)) + .send(AppMatchesInfo::artist(artist.meta.clone(), list)) .expect("send fails only if receiver is disconnected"), Err(err) => return AppMachine::error(self.inner, err.to_string()).into(), }, @@ -134,7 +138,7 @@ impl IAppInteractBrowse for AppMachine { #[cfg(test)] mod tests { use mockall::{predicate, Sequence}; - use musichoard::collection::{album::Album, artist::Artist, musicbrainz::Mbid}; + use musichoard::collection::{album::AlbumMeta, artist::ArtistMeta, musicbrainz::Mbid}; use crate::tui::{ app::{ @@ -223,8 +227,8 @@ mod tests { let mut mb_api = Box::new(MockIMusicBrainz::new()); let arid: Mbid = "11111111-1111-1111-1111-111111111111".try_into().unwrap(); - let album_1 = COLLECTION[1].albums[0].clone(); - let album_4 = COLLECTION[1].albums[3].clone(); + 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()); @@ -233,8 +237,8 @@ mod tests { let matches_1 = vec![album_match_1_1.clone(), album_match_1_2.clone()]; let matches_4 = vec![album_match_4_1.clone(), album_match_4_2.clone()]; - let result_1: Result>, musicbrainz::Error> = Ok(matches_1.clone()); - let result_4: Result>, musicbrainz::Error> = Ok(matches_4.clone()); + let result_1: Result>, musicbrainz::Error> = Ok(matches_1.clone()); + let result_4: Result>, musicbrainz::Error> = Ok(matches_4.clone()); // Other albums have an MBID and so they will be skipped. let mut seq = Sequence::new(); @@ -263,7 +267,7 @@ mod tests { let public_matches = public.state.unwrap_matches(); - let mut matches_1: Vec> = + let mut matches_1: Vec> = matches_1.into_iter().map(Into::into).collect(); matches_1.push(MatchOption::CannotHaveMbid); let expected = Some(AppMatchesInfo::Album(AppAlbumMatches { @@ -279,7 +283,7 @@ mod tests { let public_matches = public.state.unwrap_matches(); - let mut matches_4: Vec> = + let mut matches_4: Vec> = matches_4.into_iter().map(Into::into).collect(); matches_4.push(MatchOption::CannotHaveMbid); let expected = Some(AppMatchesInfo::Album(AppAlbumMatches { @@ -303,13 +307,13 @@ mod tests { fn fetch_musicbrainz_no_artist_mbid() { let mut mb_api = Box::new(MockIMusicBrainz::new()); - let artist = COLLECTION[3].clone(); + 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 matches = vec![artist_match_1.clone(), artist_match_2.clone()]; - let result: Result>, musicbrainz::Error> = Ok(matches.clone()); + let result: Result>, musicbrainz::Error> = Ok(matches.clone()); mb_api .expect_search_artist() @@ -330,7 +334,8 @@ mod tests { let public_matches = public.state.unwrap_matches(); - let mut matches: Vec> = matches.into_iter().map(Into::into).collect(); + let mut matches: Vec> = + matches.into_iter().map(Into::into).collect(); matches.push(MatchOption::CannotHaveMbid); let expected = Some(AppMatchesInfo::Artist(AppArtistMatches { matching: artist.clone(), diff --git a/src/tui/app/machine/matches.rs b/src/tui/app/machine/matches.rs index e632ffc..a945120 100644 --- a/src/tui/app/machine/matches.rs +++ b/src/tui/app/machine/matches.rs @@ -144,8 +144,8 @@ impl IAppInteractMatchesPrivate for AppMatches { mod tests { use mpsc::Receiver; use musichoard::collection::{ - album::{Album, AlbumDate, AlbumId, AlbumPrimaryType, AlbumSecondaryType}, - artist::{Artist, ArtistId}, + album::{AlbumDate, AlbumId, AlbumMeta, AlbumPrimaryType, AlbumSecondaryType}, + artist::{ArtistId, ArtistMeta}, }; use crate::tui::{ @@ -159,7 +159,7 @@ mod tests { use super::*; fn artist_matches_info_vec() -> Vec { - let artist_1 = Artist::new(ArtistId::new("Artist 1")); + let artist_1 = ArtistMeta::new(ArtistId::new("Artist 1")); let artist_1_1 = artist_1.clone(); let artist_match_1_1 = Match::new(100, artist_1_1); @@ -171,7 +171,7 @@ mod tests { let list = vec![artist_match_1_1.clone(), artist_match_1_2.clone()]; let matches_info_1 = AppMatchesInfo::artist(artist_1.clone(), list); - let artist_2 = Artist::new(ArtistId::new("Artist 2")); + let artist_2 = ArtistMeta::new(ArtistId::new("Artist 2")); let artist_2_1 = artist_1.clone(); let album_match_2_1 = Match::new(100, artist_2_1); @@ -183,7 +183,7 @@ mod tests { } fn album_matches_info_vec() -> Vec { - let album_1 = Album::new( + let album_1 = AlbumMeta::new( AlbumId::new("Album 1"), AlbumDate::new(Some(1990), Some(5), None), Some(AlbumPrimaryType::Album), @@ -194,14 +194,14 @@ mod tests { let album_match_1_1 = Match::new(100, album_1_1); let mut album_1_2 = album_1.clone(); - album_1_2.meta.id.title.push_str(" extra title part"); - album_1_2.meta.secondary_types.pop(); + album_1_2.id.title.push_str(" extra title part"); + album_1_2.secondary_types.pop(); let album_match_1_2 = Match::new(100, album_1_2); let list = vec![album_match_1_1.clone(), album_match_1_2.clone()]; let matches_info_1 = AppMatchesInfo::album(album_1.clone(), list); - let album_2 = Album::new( + let album_2 = AlbumMeta::new( AlbumId::new("Album 2"), AlbumDate::new(Some(2001), None, None), Some(AlbumPrimaryType::Album), diff --git a/src/tui/app/mod.rs b/src/tui/app/mod.rs index 3279b17..4d14aef 100644 --- a/src/tui/app/mod.rs +++ b/src/tui/app/mod.rs @@ -4,7 +4,7 @@ mod selection; pub use machine::App; pub use selection::{Category, Delta, Selection, WidgetState}; -use musichoard::collection::{album::Album, artist::Artist, Collection}; +use musichoard::collection::{album::AlbumMeta, artist::ArtistMeta, Collection}; use crate::tui::lib::interface::musicbrainz::Match; @@ -143,14 +143,14 @@ impl From> for MatchOption { #[derive(Clone, Debug, PartialEq, Eq)] pub struct AppArtistMatches { - pub matching: Artist, - pub list: Vec>, + pub matching: ArtistMeta, + pub list: Vec>, } #[derive(Clone, Debug, PartialEq, Eq)] pub struct AppAlbumMatches { - pub matching: Album, - pub list: Vec>, + pub matching: AlbumMeta, + pub list: Vec>, } #[derive(Clone, Debug, PartialEq, Eq)] @@ -160,13 +160,13 @@ pub enum AppMatchesInfo { } impl AppMatchesInfo { - pub fn artist>>(matching: Artist, list: Vec) -> Self { - let list: Vec> = list.into_iter().map(Into::into).collect(); + pub fn artist>>(matching: ArtistMeta, list: Vec) -> Self { + let list: Vec> = list.into_iter().map(Into::into).collect(); AppMatchesInfo::Artist(AppArtistMatches { matching, list }) } - pub fn album>>(matching: Album, list: Vec) -> Self { - let list: Vec> = list.into_iter().map(Into::into).collect(); + pub fn album>>(matching: AlbumMeta, list: Vec) -> Self { + let list: Vec> = list.into_iter().map(Into::into).collect(); AppMatchesInfo::Album(AppAlbumMatches { matching, list }) } } diff --git a/src/tui/lib/external/musicbrainz/mod.rs b/src/tui/lib/external/musicbrainz/mod.rs index 3a7673f..295b435 100644 --- a/src/tui/lib/external/musicbrainz/mod.rs +++ b/src/tui/lib/external/musicbrainz/mod.rs @@ -2,8 +2,8 @@ use musichoard::{ collection::{ - album::{Album, AlbumDate}, - artist::Artist, + album::{AlbumDate, AlbumMeta}, + artist::ArtistMeta, musicbrainz::Mbid, }, external::musicbrainz::{ @@ -32,8 +32,8 @@ impl MusicBrainz { } impl IMusicBrainz for MusicBrainz { - fn search_artist(&mut self, artist: &Artist) -> Result>, Error> { - let query = SearchArtistRequest::new().string(&artist.meta.id.name); + 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)?; @@ -47,18 +47,18 @@ impl IMusicBrainz for MusicBrainz { fn search_release_group( &mut self, arid: &Mbid, - album: &Album, - ) -> Result>, Error> { + album: &AlbumMeta, + ) -> 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.meta.date.year, None, None); + let date = AlbumDate::new(album.date.year, None, None); let query = SearchReleaseGroupRequest::new() .arid(arid) .and() .first_release_date(&date) .and() - .release_group(&album.meta.id.title); + .release_group(&album.id.title); let mb_response = self.client.search_release_group(query)?; @@ -70,8 +70,8 @@ impl IMusicBrainz for MusicBrainz { } } -fn from_search_artist_response_artist(entity: SearchArtistResponseArtist) -> Match { - let mut artist = Artist::new(entity.name); +fn from_search_artist_response_artist(entity: SearchArtistResponseArtist) -> Match { + let mut artist = ArtistMeta::new(entity.name); if let Some(sort) = entity.sort { artist.set_sort_key(sort); } @@ -87,8 +87,8 @@ fn from_search_artist_response_artist(entity: SearchArtistResponseArtist) -> Mat fn from_search_release_group_response_release_group( entity: SearchReleaseGroupResponseReleaseGroup, -) -> Match { - let mut album = Album::new( +) -> Match { + let mut album = AlbumMeta::new( entity.title, entity.first_release_date, Some(entity.primary_type), diff --git a/src/tui/lib/interface/musicbrainz/mod.rs b/src/tui/lib/interface/musicbrainz/mod.rs index 5679d44..5e162fe 100644 --- a/src/tui/lib/interface/musicbrainz/mod.rs +++ b/src/tui/lib/interface/musicbrainz/mod.rs @@ -3,17 +3,17 @@ #[cfg(test)] use mockall::automock; -use musichoard::collection::{album::Album, artist::Artist, musicbrainz::Mbid}; +use musichoard::collection::{album::AlbumMeta, artist::ArtistMeta, musicbrainz::Mbid}; /// Trait for interacting with the MusicBrainz API. #[cfg_attr(test, automock)] pub trait IMusicBrainz { - fn search_artist(&mut self, name: &Artist) -> Result>, Error>; + fn search_artist(&mut self, name: &ArtistMeta) -> Result>, Error>; fn search_release_group( &mut self, arid: &Mbid, - album: &Album, - ) -> Result>, Error>; + album: &AlbumMeta, + ) -> Result>, Error>; } #[derive(Clone, Debug, PartialEq, Eq)] diff --git a/src/tui/ui/display.rs b/src/tui/ui/display.rs index 1d15413..ef0f4f3 100644 --- a/src/tui/ui/display.rs +++ b/src/tui/ui/display.rs @@ -1,6 +1,6 @@ use musichoard::collection::{ - album::{Album, AlbumDate, AlbumPrimaryType, AlbumSecondaryType, AlbumSeq, AlbumStatus}, - artist::Artist, + album::{AlbumDate, AlbumMeta, AlbumPrimaryType, AlbumSecondaryType, AlbumSeq, AlbumStatus}, + artist::ArtistMeta, track::{TrackFormat, TrackQuality}, }; @@ -98,15 +98,15 @@ impl UiDisplay { } } - pub fn display_artist_matching(artist: &Artist) -> String { - format!("Matching artist: {}", &artist.meta.id.name) + pub fn display_artist_matching(artist: &ArtistMeta) -> String { + format!("Matching artist: {}", &artist.id.name) } - pub fn display_album_matching(album: &Album) -> String { + pub fn display_album_matching(album: &AlbumMeta) -> String { format!( "Matching album: {} | {}", - UiDisplay::display_album_date(&album.meta.date), - &album.meta.id.title + UiDisplay::display_album_date(&album.date), + &album.id.title ) } @@ -124,11 +124,11 @@ impl UiDisplay { } } - pub fn display_artist_match(match_option: &MatchOption) -> String { + pub fn display_artist_match(match_option: &MatchOption) -> String { match match_option { MatchOption::Match(match_artist) => format!( "{}{} ({}%)", - &match_artist.item.meta.id.name, + &match_artist.item.id.name, &match_artist .disambiguation .as_ref() @@ -140,15 +140,15 @@ impl UiDisplay { } } - pub fn display_album_match(match_option: &MatchOption) -> String { + pub fn display_album_match(match_option: &MatchOption) -> String { match match_option { MatchOption::Match(match_album) => format!( "{:010} | {} [{}] ({}%)", - UiDisplay::display_album_date(&match_album.item.meta.date), - &match_album.item.meta.id.title, + UiDisplay::display_album_date(&match_album.item.date), + &match_album.item.id.title, UiDisplay::display_type( - &match_album.item.meta.primary_type, - &match_album.item.meta.secondary_types + &match_album.item.primary_type, + &match_album.item.secondary_types ), match_album.score, ), diff --git a/src/tui/ui/matches.rs b/src/tui/ui/matches.rs index 91bd158..20fe0ae 100644 --- a/src/tui/ui/matches.rs +++ b/src/tui/ui/matches.rs @@ -1,4 +1,4 @@ -use musichoard::collection::{album::Album, artist::Artist}; +use musichoard::collection::{album::AlbumMeta, artist::ArtistMeta}; use ratatui::widgets::{List, ListItem}; use crate::tui::{ @@ -32,8 +32,8 @@ impl<'a, 'b> MatchesState<'a, 'b> { } fn artists( - matching: &Artist, - matches: &[MatchOption], + matching: &ArtistMeta, + matches: &[MatchOption], state: &'b mut WidgetState, ) -> Self { let matching = UiDisplay::display_artist_matching(matching); @@ -54,8 +54,8 @@ impl<'a, 'b> MatchesState<'a, 'b> { } fn albums( - matching: &Album, - matches: &[MatchOption], + matching: &AlbumMeta, + matches: &[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 8e5fa70..3855b7b 100644 --- a/src/tui/ui/mod.rs +++ b/src/tui/ui/mod.rs @@ -177,8 +177,8 @@ impl IUi for Ui { #[cfg(test)] mod tests { use musichoard::collection::{ - album::{AlbumDate, AlbumId, AlbumPrimaryType, AlbumSecondaryType}, - artist::{Artist, ArtistId}, + album::{AlbumDate, AlbumId, AlbumMeta, AlbumPrimaryType, AlbumSecondaryType}, + artist::{Artist, ArtistId, ArtistMeta}, }; use crate::tui::{ @@ -224,14 +224,14 @@ mod tests { } } - fn artist_matches(matching: Artist, list: Vec>) -> AppMatchesInfo { - let mut list: Vec> = list.into_iter().map(Into::into).collect(); + fn artist_matches(matching: ArtistMeta, list: Vec>) -> AppMatchesInfo { + let mut list: Vec> = list.into_iter().map(Into::into).collect(); list.push(MatchOption::CannotHaveMbid); AppMatchesInfo::artist(matching, list) } - fn album_matches(matching: Album, list: Vec>) -> AppMatchesInfo { - let mut list: Vec> = list.into_iter().map(Into::into).collect(); + fn album_matches(matching: AlbumMeta, list: Vec>) -> AppMatchesInfo { + let mut list: Vec> = list.into_iter().map(Into::into).collect(); list.push(MatchOption::CannotHaveMbid); AppMatchesInfo::album(matching, list) } @@ -328,7 +328,7 @@ mod tests { let mut terminal = terminal(); - let artist = Artist::new(ArtistId::new("an artist")); + let artist = ArtistMeta::new(ArtistId::new("an artist")); let artist_match = Match { score: 80, item: artist.clone(), @@ -357,7 +357,7 @@ mod tests { let mut terminal = terminal(); - let album = Album::new( + let album = AlbumMeta::new( AlbumId::new("An Album"), AlbumDate::new(Some(1990), Some(5), None), Some(AlbumPrimaryType::Album), -- 2.45.2 From 13a31e0d7f1e0a6c2b7fc4d5476ccbebb1857840 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sat, 31 Aug 2024 22:20:39 +0200 Subject: [PATCH 3/6] Coverage --- src/core/collection/album.rs | 1 + src/core/collection/artist.rs | 12 ++++++++---- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/core/collection/album.rs b/src/core/collection/album.rs index 1854d7d..cc77cf8 100644 --- a/src/core/collection/album.rs +++ b/src/core/collection/album.rs @@ -316,6 +316,7 @@ mod tests { assert_ne!(album_1, album_2); assert!(album_1 < album_2); + assert!(album_1.meta < album_2.meta); } #[test] diff --git a/src/core/collection/artist.rs b/src/core/collection/artist.rs index 964a746..697a00a 100644 --- a/src/core/collection/artist.rs +++ b/src/core/collection/artist.rs @@ -257,6 +257,8 @@ mod tests { assert_eq!(artist.get_sort_key(), (&artist_id,)); assert!(artist < Artist::new(sort_id_1.clone())); assert!(artist < Artist::new(sort_id_2.clone())); + assert!(artist.meta < ArtistMeta::new(sort_id_1.clone())); + assert!(artist.meta < ArtistMeta::new(sort_id_2.clone())); artist.set_sort_key(sort_id_1.clone()); @@ -265,22 +267,24 @@ mod tests { assert_eq!(artist.get_sort_key(), (&sort_id_1,)); assert!(artist > Artist::new(artist_id.clone())); assert!(artist < Artist::new(sort_id_2.clone())); + assert!(artist.meta > ArtistMeta::new(artist_id.clone())); + assert!(artist.meta < ArtistMeta::new(sort_id_2.clone())); artist.set_sort_key(sort_id_2.clone()); assert_eq!(artist.meta.id, artist_id); assert_eq!(artist.meta.sort.as_ref(), Some(&sort_id_2)); assert_eq!(artist.get_sort_key(), (&sort_id_2,)); - assert!(artist > Artist::new(artist_id.clone())); - assert!(artist > Artist::new(sort_id_1.clone())); + assert!(artist.meta > ArtistMeta::new(artist_id.clone())); + assert!(artist.meta > ArtistMeta::new(sort_id_1.clone())); artist.clear_sort_key(); assert_eq!(artist.meta.id, artist_id); assert_eq!(artist.meta.sort, None); assert_eq!(artist.get_sort_key(), (&artist_id,)); - assert!(artist < Artist::new(sort_id_1.clone())); - assert!(artist < Artist::new(sort_id_2.clone())); + assert!(artist.meta < ArtistMeta::new(sort_id_1.clone())); + assert!(artist.meta < ArtistMeta::new(sort_id_2.clone())); } #[test] -- 2.45.2 From 563782f82d61fdc576d1948dcf25e52509eabfd0 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sat, 31 Aug 2024 22:38:53 +0200 Subject: [PATCH 4/6] Clarify item vs meta operations --- src/core/collection/album.rs | 46 ++++------ src/core/collection/artist.rs | 146 ++++++++++++++++---------------- src/core/musichoard/database.rs | 26 +++--- src/tui/app/selection/album.rs | 5 +- src/tui/app/selection/artist.rs | 5 +- 5 files changed, 111 insertions(+), 117 deletions(-) diff --git a/src/core/collection/album.rs b/src/core/collection/album.rs index cc77cf8..2ca28b6 100644 --- a/src/core/collection/album.rs +++ b/src/core/collection/album.rs @@ -150,29 +150,9 @@ impl Album { } } - pub fn get_sort_key(&self) -> (&AlbumDate, &AlbumSeq, &AlbumId) { - self.meta.get_sort_key() - } - pub fn get_status(&self) -> AlbumStatus { AlbumStatus::from_tracks(&self.tracks) } - - pub fn set_seq(&mut self, seq: AlbumSeq) { - self.meta.set_seq(seq); - } - - pub fn clear_seq(&mut self) { - self.meta.clear_seq(); - } - - pub fn set_musicbrainz_ref(&mut self, mbref: MbAlbumRef) { - self.meta.set_musicbrainz_ref(mbref); - } - - pub fn clear_musicbrainz_ref(&mut self) { - self.meta.clear_musicbrainz_ref(); - } } impl PartialOrd for Album { @@ -306,13 +286,13 @@ mod tests { title: String::from("album z"), }; let mut album_1 = Album::new(album_id_1, date.clone(), None, vec![]); - album_1.set_seq(AlbumSeq(1)); + album_1.meta.set_seq(AlbumSeq(1)); let album_id_2 = AlbumId { title: String::from("album a"), }; let mut album_2 = Album::new(album_id_2, date.clone(), None, vec![]); - album_2.set_seq(AlbumSeq(2)); + album_2.meta.set_seq(AlbumSeq(2)); assert_ne!(album_1, album_2); assert!(album_1 < album_2); @@ -326,17 +306,17 @@ mod tests { assert_eq!(album.meta.seq, AlbumSeq(0)); // Setting a seq on an album. - album.set_seq(AlbumSeq(6)); + album.meta.set_seq(AlbumSeq(6)); assert_eq!(album.meta.seq, AlbumSeq(6)); - album.set_seq(AlbumSeq(6)); + album.meta.set_seq(AlbumSeq(6)); assert_eq!(album.meta.seq, AlbumSeq(6)); - album.set_seq(AlbumSeq(8)); + album.meta.set_seq(AlbumSeq(8)); assert_eq!(album.meta.seq, AlbumSeq(8)); // Clearing seq. - album.clear_seq(); + album.meta.clear_seq(); assert_eq!(album.meta.seq, AlbumSeq(0)); } @@ -388,19 +368,25 @@ mod tests { assert_eq!(album.meta.musicbrainz, expected); // Setting a URL on an album. - album.set_musicbrainz_ref(MbAlbumRef::from_url_str(MUSICBRAINZ).unwrap()); + album + .meta + .set_musicbrainz_ref(MbAlbumRef::from_url_str(MUSICBRAINZ).unwrap()); _ = expected.insert(MbAlbumRef::from_url_str(MUSICBRAINZ).unwrap()); assert_eq!(album.meta.musicbrainz, expected); - album.set_musicbrainz_ref(MbAlbumRef::from_url_str(MUSICBRAINZ).unwrap()); + album + .meta + .set_musicbrainz_ref(MbAlbumRef::from_url_str(MUSICBRAINZ).unwrap()); assert_eq!(album.meta.musicbrainz, expected); - album.set_musicbrainz_ref(MbAlbumRef::from_url_str(MUSICBRAINZ_2).unwrap()); + album + .meta + .set_musicbrainz_ref(MbAlbumRef::from_url_str(MUSICBRAINZ_2).unwrap()); _ = expected.insert(MbAlbumRef::from_url_str(MUSICBRAINZ_2).unwrap()); assert_eq!(album.meta.musicbrainz, expected); // Clearing URLs. - album.clear_musicbrainz_ref(); + album.meta.clear_musicbrainz_ref(); _ = expected.take(); assert_eq!(album.meta.musicbrainz, expected); } diff --git a/src/core/collection/artist.rs b/src/core/collection/artist.rs index 697a00a..93e4a1a 100644 --- a/src/core/collection/artist.rs +++ b/src/core/collection/artist.rs @@ -48,46 +48,6 @@ impl Artist { albums: vec![], } } - - pub fn get_sort_key(&self) -> (&ArtistId,) { - self.meta.get_sort_key() - } - - pub fn set_sort_key>(&mut self, sort: SORT) { - self.meta.set_sort_key(sort); - } - - pub fn clear_sort_key(&mut self) { - self.meta.clear_sort_key(); - } - - pub fn set_musicbrainz_ref(&mut self, mbref: MbArtistRef) { - self.meta.set_musicbrainz_ref(mbref); - } - - pub fn clear_musicbrainz_ref(&mut self) { - self.meta.clear_musicbrainz_ref(); - } - - // In the functions below, it would be better to use `contains` instead of `iter().any`, but for - // type reasons that does not work: - // https://stackoverflow.com/questions/48985924/why-does-a-str-not-coerce-to-a-string-when-using-veccontains - - pub fn add_to_property + Into>(&mut self, property: S, values: Vec) { - self.meta.add_to_property(property, values); - } - - pub fn remove_from_property>(&mut self, property: S, values: Vec) { - self.meta.remove_from_property(property, values); - } - - pub fn set_property + Into>(&mut self, property: S, values: Vec) { - self.meta.set_property(property, values); - } - - pub fn clear_property>(&mut self, property: S) { - self.meta.clear_property(property); - } } impl PartialOrd for Artist { @@ -254,37 +214,41 @@ mod tests { assert_eq!(artist.meta.id, artist_id); assert_eq!(artist.meta.sort, None); - assert_eq!(artist.get_sort_key(), (&artist_id,)); - assert!(artist < Artist::new(sort_id_1.clone())); - assert!(artist < Artist::new(sort_id_2.clone())); + assert_eq!(artist.meta.get_sort_key(), (&artist_id,)); assert!(artist.meta < ArtistMeta::new(sort_id_1.clone())); assert!(artist.meta < ArtistMeta::new(sort_id_2.clone())); + assert!(artist < Artist::new(sort_id_1.clone())); + assert!(artist < Artist::new(sort_id_2.clone())); - artist.set_sort_key(sort_id_1.clone()); + artist.meta.set_sort_key(sort_id_1.clone()); assert_eq!(artist.meta.id, artist_id); assert_eq!(artist.meta.sort.as_ref(), Some(&sort_id_1)); - assert_eq!(artist.get_sort_key(), (&sort_id_1,)); - assert!(artist > Artist::new(artist_id.clone())); - assert!(artist < Artist::new(sort_id_2.clone())); + assert_eq!(artist.meta.get_sort_key(), (&sort_id_1,)); assert!(artist.meta > ArtistMeta::new(artist_id.clone())); assert!(artist.meta < ArtistMeta::new(sort_id_2.clone())); + assert!(artist > Artist::new(artist_id.clone())); + assert!(artist < Artist::new(sort_id_2.clone())); - artist.set_sort_key(sort_id_2.clone()); + artist.meta.set_sort_key(sort_id_2.clone()); assert_eq!(artist.meta.id, artist_id); assert_eq!(artist.meta.sort.as_ref(), Some(&sort_id_2)); - assert_eq!(artist.get_sort_key(), (&sort_id_2,)); + assert_eq!(artist.meta.get_sort_key(), (&sort_id_2,)); assert!(artist.meta > ArtistMeta::new(artist_id.clone())); assert!(artist.meta > ArtistMeta::new(sort_id_1.clone())); + assert!(artist > Artist::new(artist_id.clone())); + assert!(artist > Artist::new(sort_id_1.clone())); - artist.clear_sort_key(); + artist.meta.clear_sort_key(); assert_eq!(artist.meta.id, artist_id); assert_eq!(artist.meta.sort, None); - assert_eq!(artist.get_sort_key(), (&artist_id,)); + assert_eq!(artist.meta.get_sort_key(), (&artist_id,)); assert!(artist.meta < ArtistMeta::new(sort_id_1.clone())); assert!(artist.meta < ArtistMeta::new(sort_id_2.clone())); + assert!(artist < Artist::new(sort_id_1.clone())); + assert!(artist < Artist::new(sort_id_2.clone())); } #[test] @@ -295,19 +259,25 @@ mod tests { assert_eq!(artist.meta.musicbrainz, expected); // Setting a URL on an artist. - artist.set_musicbrainz_ref(MbArtistRef::from_url_str(MUSICBRAINZ).unwrap()); + artist + .meta + .set_musicbrainz_ref(MbArtistRef::from_url_str(MUSICBRAINZ).unwrap()); _ = expected.insert(MbArtistRef::from_url_str(MUSICBRAINZ).unwrap()); assert_eq!(artist.meta.musicbrainz, expected); - artist.set_musicbrainz_ref(MbArtistRef::from_url_str(MUSICBRAINZ).unwrap()); + artist + .meta + .set_musicbrainz_ref(MbArtistRef::from_url_str(MUSICBRAINZ).unwrap()); assert_eq!(artist.meta.musicbrainz, expected); - artist.set_musicbrainz_ref(MbArtistRef::from_url_str(MUSICBRAINZ_2).unwrap()); + artist + .meta + .set_musicbrainz_ref(MbArtistRef::from_url_str(MUSICBRAINZ_2).unwrap()); _ = expected.insert(MbArtistRef::from_url_str(MUSICBRAINZ_2).unwrap()); assert_eq!(artist.meta.musicbrainz, expected); // Clearing URLs. - artist.clear_musicbrainz_ref(); + artist.meta.clear_musicbrainz_ref(); _ = expected.take(); assert_eq!(artist.meta.musicbrainz, expected); } @@ -320,65 +290,93 @@ mod tests { assert!(artist.meta.properties.is_empty()); // Adding a single URL. - artist.add_to_property("MusicButler", vec![MUSICBUTLER]); + artist + .meta + .add_to_property("MusicButler", vec![MUSICBUTLER]); expected.push(MUSICBUTLER.to_owned()); assert_eq!(artist.meta.properties.get("MusicButler"), Some(&expected)); // Adding a URL that already exists is ok, but does not do anything. - artist.add_to_property("MusicButler", vec![MUSICBUTLER]); + artist + .meta + .add_to_property("MusicButler", vec![MUSICBUTLER]); assert_eq!(artist.meta.properties.get("MusicButler"), Some(&expected)); // Adding another single URL. - artist.add_to_property("MusicButler", vec![MUSICBUTLER_2]); + artist + .meta + .add_to_property("MusicButler", vec![MUSICBUTLER_2]); expected.push(MUSICBUTLER_2.to_owned()); assert_eq!(artist.meta.properties.get("MusicButler"), Some(&expected)); - artist.add_to_property("MusicButler", vec![MUSICBUTLER_2]); + artist + .meta + .add_to_property("MusicButler", vec![MUSICBUTLER_2]); assert_eq!(artist.meta.properties.get("MusicButler"), Some(&expected)); // Removing a URL. - artist.remove_from_property("MusicButler", vec![MUSICBUTLER]); + artist + .meta + .remove_from_property("MusicButler", vec![MUSICBUTLER]); expected.retain(|url| url != MUSICBUTLER); assert_eq!(artist.meta.properties.get("MusicButler"), Some(&expected)); // Removing URls that do not exist is okay, they will be ignored. - artist.remove_from_property("MusicButler", vec![MUSICBUTLER]); + artist + .meta + .remove_from_property("MusicButler", vec![MUSICBUTLER]); assert_eq!(artist.meta.properties.get("MusicButler"), Some(&expected)); // Removing a URL. - artist.remove_from_property("MusicButler", vec![MUSICBUTLER_2]); + artist + .meta + .remove_from_property("MusicButler", vec![MUSICBUTLER_2]); expected.retain(|url| url.as_str() != MUSICBUTLER_2); assert!(artist.meta.properties.is_empty()); - artist.remove_from_property("MusicButler", vec![MUSICBUTLER_2]); + artist + .meta + .remove_from_property("MusicButler", vec![MUSICBUTLER_2]); assert!(artist.meta.properties.is_empty()); // Adding URLs if some exist is okay, they will be ignored. - artist.add_to_property("MusicButler", vec![MUSICBUTLER]); + artist + .meta + .add_to_property("MusicButler", vec![MUSICBUTLER]); expected.push(MUSICBUTLER.to_owned()); assert_eq!(artist.meta.properties.get("MusicButler"), Some(&expected)); - artist.add_to_property("MusicButler", vec![MUSICBUTLER, MUSICBUTLER_2]); + artist + .meta + .add_to_property("MusicButler", vec![MUSICBUTLER, MUSICBUTLER_2]); expected.push(MUSICBUTLER_2.to_owned()); assert_eq!(artist.meta.properties.get("MusicButler"), Some(&expected)); // Removing URLs if some do not exist is okay, they will be ignored. - artist.remove_from_property("MusicButler", vec![MUSICBUTLER]); + artist + .meta + .remove_from_property("MusicButler", vec![MUSICBUTLER]); expected.retain(|url| url.as_str() != MUSICBUTLER); assert_eq!(artist.meta.properties.get("MusicButler"), Some(&expected)); - artist.remove_from_property("MusicButler", vec![MUSICBUTLER, MUSICBUTLER_2]); + artist + .meta + .remove_from_property("MusicButler", vec![MUSICBUTLER, MUSICBUTLER_2]); expected.retain(|url| url.as_str() != MUSICBUTLER_2); assert!(artist.meta.properties.is_empty()); // Adding mutliple URLs without clashes. - artist.add_to_property("MusicButler", vec![MUSICBUTLER, MUSICBUTLER_2]); + artist + .meta + .add_to_property("MusicButler", vec![MUSICBUTLER, MUSICBUTLER_2]); expected.push(MUSICBUTLER.to_owned()); expected.push(MUSICBUTLER_2.to_owned()); assert_eq!(artist.meta.properties.get("MusicButler"), Some(&expected)); // Removing multiple URLs without clashes. - artist.remove_from_property("MusicButler", vec![MUSICBUTLER, MUSICBUTLER_2]); + artist + .meta + .remove_from_property("MusicButler", vec![MUSICBUTLER, MUSICBUTLER_2]); expected.clear(); assert!(artist.meta.properties.is_empty()); } @@ -391,23 +389,25 @@ mod tests { assert!(artist.meta.properties.is_empty()); // Set URLs. - artist.set_property("MusicButler", vec![MUSICBUTLER]); + artist.meta.set_property("MusicButler", vec![MUSICBUTLER]); expected.push(MUSICBUTLER.to_owned()); assert_eq!(artist.meta.properties.get("MusicButler"), Some(&expected)); - artist.set_property("MusicButler", vec![MUSICBUTLER_2]); + artist.meta.set_property("MusicButler", vec![MUSICBUTLER_2]); expected.clear(); expected.push(MUSICBUTLER_2.to_owned()); assert_eq!(artist.meta.properties.get("MusicButler"), Some(&expected)); - artist.set_property("MusicButler", vec![MUSICBUTLER, MUSICBUTLER_2]); + artist + .meta + .set_property("MusicButler", vec![MUSICBUTLER, MUSICBUTLER_2]); expected.clear(); expected.push(MUSICBUTLER.to_owned()); expected.push(MUSICBUTLER_2.to_owned()); assert_eq!(artist.meta.properties.get("MusicButler"), Some(&expected)); // Clear URLs. - artist.clear_property("MusicButler"); + artist.meta.clear_property("MusicButler"); expected.clear(); assert!(artist.meta.properties.is_empty()); } diff --git a/src/core/musichoard/database.rs b/src/core/musichoard/database.rs index 9926966..25287ac 100644 --- a/src/core/musichoard/database.rs +++ b/src/core/musichoard/database.rs @@ -107,7 +107,7 @@ impl IMusicHoardDatabase for MusicHoard Result<(), Error> { self.update_artist_and( artist_id.as_ref(), - |artist| artist.set_sort_key(artist_sort), + |artist| artist.meta.set_sort_key(artist_sort), |collection| Self::sort_artists(collection), ) } @@ -115,7 +115,7 @@ impl IMusicHoardDatabase for MusicHoard>(&mut self, artist_id: Id) -> Result<(), Error> { self.update_artist_and( artist_id.as_ref(), - |artist| artist.clear_sort_key(), + |artist| artist.meta.clear_sort_key(), |collection| Self::sort_artists(collection), ) } @@ -126,14 +126,18 @@ impl IMusicHoardDatabase for MusicHoard Result<(), Error> { let mb = MbArtistRef::from_url_str(url)?; - self.update_artist(artist_id.as_ref(), |artist| artist.set_musicbrainz_ref(mb)) + self.update_artist(artist_id.as_ref(), |artist| { + artist.meta.set_musicbrainz_ref(mb) + }) } fn clear_artist_musicbrainz>( &mut self, artist_id: Id, ) -> Result<(), Error> { - self.update_artist(artist_id.as_ref(), |artist| artist.clear_musicbrainz_ref()) + self.update_artist(artist_id.as_ref(), |artist| { + artist.meta.clear_musicbrainz_ref() + }) } fn add_to_artist_property, S: AsRef + Into>( @@ -143,7 +147,7 @@ impl IMusicHoardDatabase for MusicHoard, ) -> Result<(), Error> { self.update_artist(artist_id.as_ref(), |artist| { - artist.add_to_property(property, values) + artist.meta.add_to_property(property, values) }) } @@ -154,7 +158,7 @@ impl IMusicHoardDatabase for MusicHoard, ) -> Result<(), Error> { self.update_artist(artist_id.as_ref(), |artist| { - artist.remove_from_property(property, values) + artist.meta.remove_from_property(property, values) }) } @@ -165,7 +169,7 @@ impl IMusicHoardDatabase for MusicHoard, ) -> Result<(), Error> { self.update_artist(artist_id.as_ref(), |artist| { - artist.set_property(property, values) + artist.meta.set_property(property, values) }) } @@ -174,7 +178,9 @@ impl IMusicHoardDatabase for MusicHoard Result<(), Error> { - self.update_artist(artist_id.as_ref(), |artist| artist.clear_property(property)) + self.update_artist(artist_id.as_ref(), |artist| { + artist.meta.clear_property(property) + }) } fn set_album_seq, AlbumIdRef: AsRef>( @@ -186,7 +192,7 @@ impl IMusicHoardDatabase for MusicHoard IMusicHoardDatabase for MusicHoard) { if let Some(album) = album { - let result = albums.binary_search_by(|a| a.get_sort_key().cmp(&album.get_sort_key())); + let result = + albums.binary_search_by(|a| a.meta.get_sort_key().cmp(&album.get_sort_key())); match result { Ok(index) => self.reinitialise_with_index(albums, index, album.track), Err(index) => self.reinitialise_with_index(albums, index, None), @@ -169,7 +170,7 @@ impl KeySelectAlbum { pub fn get(albums: &[Album], selection: &AlbumSelection) -> Option { selection.state.list.selected().map(|index| { let album = &albums[index]; - let key = album.get_sort_key(); + let key = album.meta.get_sort_key(); KeySelectAlbum { key: (key.0.to_owned(), key.1.to_owned(), key.2.to_owned()), track: KeySelectTrack::get(&album.tracks, &selection.track), diff --git a/src/tui/app/selection/artist.rs b/src/tui/app/selection/artist.rs index aa96d25..55877b7 100644 --- a/src/tui/app/selection/artist.rs +++ b/src/tui/app/selection/artist.rs @@ -29,7 +29,8 @@ impl ArtistSelection { pub fn reinitialise(&mut self, artists: &[Artist], active: Option) { if let Some(active) = active { - let result = artists.binary_search_by(|a| a.get_sort_key().cmp(&active.get_sort_key())); + let result = + artists.binary_search_by(|a| a.meta.get_sort_key().cmp(&active.get_sort_key())); match result { Ok(index) => self.reinitialise_with_index(artists, index, active.album), Err(index) => self.reinitialise_with_index(artists, index, None), @@ -202,7 +203,7 @@ impl KeySelectArtist { pub fn get(artists: &[Artist], selection: &ArtistSelection) -> Option { selection.state.list.selected().map(|index| { let artist = &artists[index]; - let key = artist.get_sort_key(); + let key = artist.meta.get_sort_key(); KeySelectArtist { key: (key.0.to_owned(),), album: KeySelectAlbum::get(&artist.albums, &selection.album), -- 2.45.2 From e8b5532a1bff4a58a9b268dd4f65cedecd13ba19 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sat, 31 Aug 2024 22:50:01 +0200 Subject: [PATCH 5/6] More explicit conversion --- src/tui/app/machine/matches.rs | 12 ++++++- src/tui/lib/external/musicbrainz/mod.rs | 44 +++++++++++++----------- src/tui/lib/interface/musicbrainz/mod.rs | 14 -------- 3 files changed, 35 insertions(+), 35 deletions(-) diff --git a/src/tui/app/machine/matches.rs b/src/tui/app/machine/matches.rs index a945120..26c2774 100644 --- a/src/tui/app/machine/matches.rs +++ b/src/tui/app/machine/matches.rs @@ -158,6 +158,16 @@ mod tests { use super::*; + impl Match { + pub fn new(score: u8, item: T) -> Self { + Match { + score, + item, + disambiguation: None, + } + } + } + fn artist_matches_info_vec() -> Vec { let artist_1 = ArtistMeta::new(ArtistId::new("Artist 1")); @@ -166,7 +176,7 @@ mod tests { let artist_1_2 = artist_1.clone(); let mut artist_match_1_2 = Match::new(100, artist_1_2); - artist_match_1_2.set_disambiguation("some disambiguation"); + artist_match_1_2.disambiguation = Some(String::from("some disambiguation")); let list = vec![artist_match_1_1.clone(), artist_match_1_2.clone()]; let matches_info_1 = AppMatchesInfo::artist(artist_1.clone(), list); diff --git a/src/tui/lib/external/musicbrainz/mod.rs b/src/tui/lib/external/musicbrainz/mod.rs index 295b435..00de146 100644 --- a/src/tui/lib/external/musicbrainz/mod.rs +++ b/src/tui/lib/external/musicbrainz/mod.rs @@ -1,8 +1,10 @@ //! Module for interacting with the [MusicBrainz API](https://musicbrainz.org/doc/MusicBrainz_API). +use std::collections::HashMap; + use musichoard::{ collection::{ - album::{AlbumDate, AlbumMeta}, + album::{AlbumDate, AlbumMeta, AlbumSeq}, artist::ArtistMeta, musicbrainz::Mbid, }, @@ -71,30 +73,32 @@ impl IMusicBrainz for MusicBrainz { } fn from_search_artist_response_artist(entity: SearchArtistResponseArtist) -> Match { - let mut artist = ArtistMeta::new(entity.name); - if let Some(sort) = entity.sort { - artist.set_sort_key(sort); + Match { + score: entity.score, + item: ArtistMeta { + id: entity.name.into(), + sort: entity.sort.map(Into::into), + musicbrainz: Some(entity.id.into()), + properties: HashMap::new(), + }, + disambiguation: entity.disambiguation, } - artist.set_musicbrainz_ref(entity.id.into()); - - let mut artist_match = Match::new(entity.score, artist); - if let Some(disambiguation) = entity.disambiguation { - artist_match.set_disambiguation(disambiguation); - } - - artist_match } fn from_search_release_group_response_release_group( entity: SearchReleaseGroupResponseReleaseGroup, ) -> Match { - let mut album = AlbumMeta::new( - entity.title, - entity.first_release_date, - Some(entity.primary_type), - entity.secondary_types.unwrap_or_default(), - ); - album.set_musicbrainz_ref(entity.id.into()); - Match::new(entity.score, album) + Match { + score: entity.score, + item: AlbumMeta { + id: entity.title.into(), + date: entity.first_release_date, + seq: AlbumSeq::default(), + musicbrainz: Some(entity.id.into()), + primary_type: Some(entity.primary_type), + secondary_types: entity.secondary_types.unwrap_or_default(), + }, + disambiguation: None, + } } // GRCOV_EXCL_STOP diff --git a/src/tui/lib/interface/musicbrainz/mod.rs b/src/tui/lib/interface/musicbrainz/mod.rs index 5e162fe..1bb540e 100644 --- a/src/tui/lib/interface/musicbrainz/mod.rs +++ b/src/tui/lib/interface/musicbrainz/mod.rs @@ -23,18 +23,4 @@ pub struct Match { pub disambiguation: Option, } -impl Match { - pub fn new(score: u8, item: T) -> Self { - Match { - score, - item, - disambiguation: None, - } - } - - pub fn set_disambiguation>(&mut self, disambiguation: S) { - self.disambiguation = Some(disambiguation.into()) - } -} - pub type Error = musichoard::external::musicbrainz::api::Error; -- 2.45.2 From be9901cb245f5be35d41348d18c68b38d2201ee4 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sat, 31 Aug 2024 22:50:39 +0200 Subject: [PATCH 6/6] Clippy --- src/tui/lib/external/musicbrainz/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tui/lib/external/musicbrainz/mod.rs b/src/tui/lib/external/musicbrainz/mod.rs index 00de146..a7502eb 100644 --- a/src/tui/lib/external/musicbrainz/mod.rs +++ b/src/tui/lib/external/musicbrainz/mod.rs @@ -76,7 +76,7 @@ fn from_search_artist_response_artist(entity: SearchArtistResponseArtist) -> Mat Match { score: entity.score, item: ArtistMeta { - id: entity.name.into(), + id: entity.name, sort: entity.sort.map(Into::into), musicbrainz: Some(entity.id.into()), properties: HashMap::new(), @@ -91,7 +91,7 @@ fn from_search_release_group_response_release_group( Match { score: entity.score, item: AlbumMeta { - id: entity.title.into(), + id: entity.title, date: entity.first_release_date, seq: AlbumSeq::default(), musicbrainz: Some(entity.id.into()), -- 2.45.2