From c77b4acbd42e13bb8a3f1a5b2096fbd754c10ea2 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sat, 2 Mar 2024 18:41:31 +0100 Subject: [PATCH] Fix merging for albums --- src/core/collection/album.rs | 21 ++++++++++++- src/core/collection/artist.rs | 14 +++++++-- src/core/collection/merge.rs | 42 +++++++++++++++++++++++++- src/core/collection/mod.rs | 2 +- src/core/database/json/mod.rs | 12 ++------ src/core/database/json/testmod.rs | 12 ++++---- src/core/database/serde/deserialize.rs | 17 +++++------ src/core/database/serde/mod.rs | 8 ----- src/core/database/serde/serialize.rs | 12 ++++---- src/core/musichoard/musichoard.rs | 16 ++-------- src/tests.rs | 11 +++++-- tests/database/json.rs | 7 +++-- tests/files/database/database.json | 2 +- 13 files changed, 113 insertions(+), 63 deletions(-) diff --git a/src/core/collection/album.rs b/src/core/collection/album.rs index fea4895..03e2acb 100644 --- a/src/core/collection/album.rs +++ b/src/core/collection/album.rs @@ -1,7 +1,7 @@ use std::mem; use crate::core::collection::{ - merge::{Merge, MergeSorted}, + merge::{Merge, MergeSorted, WithId}, track::Track, }; @@ -14,6 +14,14 @@ pub struct Album { pub tracks: Vec, } +impl WithId for Album { + type Id = AlbumId; + + fn id(&self) -> &Self::Id { + &self.id + } +} + /// The album identifier. #[derive(Clone, Debug, PartialEq, PartialOrd, Ord, Eq, Hash)] pub struct AlbumId { @@ -29,6 +37,16 @@ pub struct AlbumDate { pub day: u8, } +impl Default for AlbumDate { + fn default() -> Self { + AlbumDate { + year: 0, + month: AlbumMonth::None, + day: 0, + } + } +} + /// The album's sequence to determine order when two or more albums have the same release date. #[derive(Clone, Copy, Debug, PartialEq, PartialOrd, Ord, Eq, Hash)] pub struct AlbumSeq(pub u8); @@ -92,6 +110,7 @@ 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); let tracks = mem::take(&mut self.tracks); self.tracks = MergeSorted::new(tracks.into_iter(), other.tracks.into_iter()).collect(); } diff --git a/src/core/collection/artist.rs b/src/core/collection/artist.rs index 8081475..ab9d8b0 100644 --- a/src/core/collection/artist.rs +++ b/src/core/collection/artist.rs @@ -9,7 +9,7 @@ use uuid::Uuid; use crate::core::collection::{ album::Album, - merge::{Merge, MergeSorted}, + merge::{Merge, MergeCollections, WithId}, Error, }; @@ -23,6 +23,14 @@ pub struct Artist { pub albums: Vec, } +impl WithId for Artist { + type Id = ArtistId; + + fn id(&self) -> &Self::Id { + &self.id + } +} + /// The artist identifier. #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct ArtistId { @@ -121,11 +129,13 @@ impl Ord for Artist { impl Merge for Artist { 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); + let albums = mem::take(&mut self.albums); - self.albums = MergeSorted::new(albums.into_iter(), other.albums.into_iter()).collect(); + self.albums = MergeCollections::merge_iter(albums, other.albums); } } diff --git a/src/core/collection/merge.rs b/src/core/collection/merge.rs index e2a5fc5..76ed15a 100644 --- a/src/core/collection/merge.rs +++ b/src/core/collection/merge.rs @@ -1,4 +1,4 @@ -use std::{cmp::Ordering, collections::HashMap, hash::Hash, iter::Peekable}; +use std::{cmp::Ordering, collections::HashMap, hash::Hash, iter::Peekable, marker::PhantomData}; /// A trait for merging two objects. The merge is asymmetric with the left argument considered to be /// the primary whose properties are to be kept in case of collisions. @@ -79,3 +79,43 @@ where } } } + +pub trait WithId { + type Id; + + fn id(&self) -> &Self::Id; +} + +pub struct MergeCollections { + _id: PhantomData, + _t: PhantomData, +} + +impl MergeCollections +where + ID: Eq + Hash + Clone, + T: WithId + Merge + Ord, +{ + pub fn merge_iter>(primary: IT, secondary: IT) -> Vec { + let primary = primary + .into_iter() + .map(|item| (item.id().clone(), item)) + .collect(); + Self::merge(primary, secondary) + } + + pub fn merge>(mut primary: HashMap, secondary: IT) -> Vec { + for secondary_item in secondary { + if let Some(ref mut primary_item) = primary.get_mut(secondary_item.id()) { + primary_item.merge_in_place(secondary_item); + } else { + primary.insert(secondary_item.id().clone(), secondary_item); + } + } + + let mut collection: Vec = primary.into_values().collect(); + collection.sort_unstable(); + + collection + } +} diff --git a/src/core/collection/mod.rs b/src/core/collection/mod.rs index ea42e15..ef4386d 100644 --- a/src/core/collection/mod.rs +++ b/src/core/collection/mod.rs @@ -5,7 +5,7 @@ pub mod artist; pub mod track; mod merge; -pub use merge::Merge; +pub use merge::MergeCollections; use std::fmt::{self, Display}; diff --git a/src/core/database/json/mod.rs b/src/core/database/json/mod.rs index 1e59ebd..5eec609 100644 --- a/src/core/database/json/mod.rs +++ b/src/core/database/json/mod.rs @@ -72,11 +72,7 @@ mod tests { use mockall::predicate; use crate::core::{ - collection::{ - album::{AlbumDate, AlbumMonth}, - artist::Artist, - Collection, - }, + collection::{album::AlbumDate, artist::Artist, Collection}, testmod::FULL_COLLECTION, }; @@ -87,11 +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 { - year: 0, - month: AlbumMonth::None, - day: 0, - }; + album.date = AlbumDate::default(); album.tracks.clear(); } } diff --git a/src/core/database/json/testmod.rs b/src/core/database/json/testmod.rs index 84cc693..d328f8e 100644 --- a/src/core/database/json/testmod.rs +++ b/src/core/database/json/testmod.rs @@ -10,8 +10,8 @@ pub static DATABASE_JSON: &str = "{\ \"Qobuz\":[\"https://www.qobuz.com/nl-nl/interpreter/artist-a/download-streaming-albums\"]\ },\ \"albums\":[\ - {\"title\":\"album_title a.a\",\"seq\":0},\ - {\"title\":\"album_title a.b\",\"seq\":0}\ + {\"title\":\"album_title a.a\",\"seq\":1},\ + {\"title\":\"album_title a.b\",\"seq\":1}\ ]\ },\ {\ @@ -27,10 +27,10 @@ pub static DATABASE_JSON: &str = "{\ \"Qobuz\":[\"https://www.qobuz.com/nl-nl/interpreter/artist-b/download-streaming-albums\"]\ },\ \"albums\":[\ - {\"title\":\"album_title b.a\",\"seq\":0},\ - {\"title\":\"album_title b.b\",\"seq\":0},\ - {\"title\":\"album_title b.c\",\"seq\":0},\ - {\"title\":\"album_title b.d\",\"seq\":0}\ + {\"title\":\"album_title b.a\",\"seq\":1},\ + {\"title\":\"album_title b.b\",\"seq\":3},\ + {\"title\":\"album_title b.c\",\"seq\":2},\ + {\"title\":\"album_title b.d\",\"seq\":4}\ ]\ },\ {\ diff --git a/src/core/database/serde/deserialize.rs b/src/core/database/serde/deserialize.rs index ced43e7..8b403ec 100644 --- a/src/core/database/serde/deserialize.rs +++ b/src/core/database/serde/deserialize.rs @@ -4,14 +4,17 @@ use serde::Deserialize; use crate::core::{ collection::{ - album::{Album, AlbumDate, AlbumId, AlbumMonth, AlbumSeq}, + album::{Album, AlbumDate, AlbumId, AlbumSeq}, artist::{Artist, ArtistId, MusicBrainz}, Collection, }, - database::{serde::Database, LoadError}, + database::LoadError, }; -pub type DeserializeDatabase = Database; +#[derive(Debug, Deserialize)] +pub enum DeserializeDatabase { + V20240302(Vec), +} #[derive(Debug, Deserialize)] pub struct DeserializeArtist { @@ -33,7 +36,7 @@ impl TryFrom for Collection { fn try_from(database: DeserializeDatabase) -> Result { match database { - Database::V20240302(collection) | Database::V20240210(collection) => collection + DeserializeDatabase::V20240302(collection) => collection .into_iter() .map(|artist| artist.try_into()) .collect(), @@ -59,11 +62,7 @@ impl From for Album { fn from(album: DeserializeAlbum) -> Self { Album { id: AlbumId { title: album.title }, - date: AlbumDate { - year: 0, - month: AlbumMonth::None, - day: 0, - }, + date: AlbumDate::default(), seq: AlbumSeq(album.seq), tracks: vec![], } diff --git a/src/core/database/serde/mod.rs b/src/core/database/serde/mod.rs index ac41ef7..d0a878f 100644 --- a/src/core/database/serde/mod.rs +++ b/src/core/database/serde/mod.rs @@ -2,11 +2,3 @@ pub mod deserialize; pub mod serialize; - -use serde::{Deserialize, Serialize}; - -#[derive(Debug, Serialize, Deserialize)] -pub enum Database { - V20240302(Vec), - V20240210(Vec), -} diff --git a/src/core/database/serde/serialize.rs b/src/core/database/serde/serialize.rs index 56e477c..5276b54 100644 --- a/src/core/database/serde/serialize.rs +++ b/src/core/database/serde/serialize.rs @@ -2,12 +2,12 @@ use std::collections::BTreeMap; use serde::Serialize; -use crate::core::{ - collection::{album::Album, artist::Artist, Collection}, - database::serde::Database, -}; +use crate::core::collection::{album::Album, artist::Artist, Collection}; -pub type SerializeDatabase<'a> = Database>; +#[derive(Debug, Serialize)] +pub enum SerializeDatabase<'a> { + V20240302(Vec>), +} #[derive(Debug, Serialize)] pub struct SerializeArtist<'a> { @@ -26,7 +26,7 @@ pub struct SerializeAlbum<'a> { impl<'a> From<&'a Collection> for SerializeDatabase<'a> { fn from(collection: &'a Collection) -> Self { - Database::V20240302(collection.iter().map(|artist| artist.into()).collect()) + SerializeDatabase::V20240302(collection.iter().map(Into::into).collect()) } } diff --git a/src/core/musichoard/musichoard.rs b/src/core/musichoard/musichoard.rs index 779f259..8995e86 100644 --- a/src/core/musichoard/musichoard.rs +++ b/src/core/musichoard/musichoard.rs @@ -5,7 +5,7 @@ use crate::core::{ album::{Album, AlbumDate, AlbumId, AlbumSeq}, artist::{Artist, ArtistId}, track::{Track, TrackId, TrackNum, TrackQuality}, - Collection, Merge, + Collection, MergeCollections, }, database::IDatabase, library::{ILibrary, Item, Query}, @@ -73,19 +73,7 @@ impl MusicHoard { } fn merge_collections(&self) -> Collection { - let mut primary = self.library_cache.clone(); - for secondary_artist in self.database_cache.iter().cloned() { - if let Some(ref mut primary_artist) = primary.get_mut(&secondary_artist.id) { - primary_artist.merge_in_place(secondary_artist); - } else { - primary.insert(secondary_artist.id.clone(), secondary_artist); - } - } - - let mut collection: Collection = primary.into_values().collect(); - Self::sort_artists(&mut collection); - - collection + MergeCollections::merge(self.library_cache.clone(), self.database_cache.clone()) } fn items_to_artists(items: Vec) -> Result, Error> { diff --git a/src/tests.rs b/src/tests.rs index 6125799..aa8e5b2 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -457,8 +457,7 @@ macro_rules! full_collection { artist_a.musicbrainz = Some( MusicBrainz::new( "https://musicbrainz.org/artist/00000000-0000-0000-0000-000000000000", - ) - .unwrap(), + ).unwrap(), ); artist_a.properties = HashMap::from([ @@ -472,6 +471,9 @@ macro_rules! full_collection { ]), ]); + artist_a.albums[0].seq = AlbumSeq(1); + artist_a.albums[1].seq = AlbumSeq(1); + let artist_b = iter.next().unwrap(); assert_eq!(artist_b.id.name, "Album_Artist ‘B’"); @@ -494,6 +496,11 @@ macro_rules! full_collection { ]), ]); + artist_b.albums[0].seq = AlbumSeq(1); + artist_b.albums[1].seq = AlbumSeq(3); + artist_b.albums[2].seq = AlbumSeq(2); + artist_b.albums[3].seq = AlbumSeq(4); + let artist_c = iter.next().unwrap(); assert_eq!(artist_c.id.name, "The Album_Artist ‘C’"); diff --git a/tests/database/json.rs b/tests/database/json.rs index 0f9d6b2..f2c6c52 100644 --- a/tests/database/json.rs +++ b/tests/database/json.rs @@ -4,7 +4,7 @@ use once_cell::sync::Lazy; use tempfile::NamedTempFile; use musichoard::{ - collection::{artist::Artist, Collection}, + collection::{album::AlbumDate, artist::Artist, Collection}, database::{ json::{backend::JsonDatabaseFileBackend, JsonDatabase}, IDatabase, @@ -19,7 +19,10 @@ pub static DATABASE_TEST_FILE: Lazy = fn expected() -> Collection { let mut expected = COLLECTION.to_owned(); for artist in expected.iter_mut() { - artist.albums.clear(); + for album in artist.albums.iter_mut() { + album.date = AlbumDate::default(); + album.tracks.clear(); + } } expected } diff --git a/tests/files/database/database.json b/tests/files/database/database.json index 465e2b9..3515a9b 100644 --- a/tests/files/database/database.json +++ b/tests/files/database/database.json @@ -1 +1 @@ -{"V20240210":[{"name":"Аркона","sort":"Arkona","musicbrainz":"https://musicbrainz.org/artist/baad262d-55ef-427a-83c7-f7530964f212","properties":{"Bandcamp":["https://arkonamoscow.bandcamp.com/"],"MusicButler":["https://www.musicbutler.io/artist-page/283448581"],"Qobuz":["https://www.qobuz.com/nl-nl/interpreter/arkona/download-streaming-albums"]}},{"name":"Eluveitie","sort":null,"musicbrainz":"https://musicbrainz.org/artist/8000598a-5edb-401c-8e6d-36b167feaf38","properties":{"MusicButler":["https://www.musicbutler.io/artist-page/269358403"],"Qobuz":["https://www.qobuz.com/nl-nl/interpreter/eluveitie/download-streaming-albums"]}},{"name":"Frontside","sort":null,"musicbrainz":"https://musicbrainz.org/artist/3a901353-fccd-4afd-ad01-9c03f451b490","properties":{"MusicButler":["https://www.musicbutler.io/artist-page/826588800"],"Qobuz":["https://www.qobuz.com/nl-nl/interpreter/frontside/download-streaming-albums"]}},{"name":"Heaven’s Basement","sort":"Heaven’s Basement","musicbrainz":"https://musicbrainz.org/artist/c2c4d56a-d599-4a18-bd2f-ae644e2198cc","properties":{"MusicButler":["https://www.musicbutler.io/artist-page/291158685"],"Qobuz":["https://www.qobuz.com/nl-nl/interpreter/heaven-s-basement/download-streaming-albums"]}},{"name":"Metallica","sort":null,"musicbrainz":"https://musicbrainz.org/artist/65f4f0c5-ef9e-490c-aee3-909e7ae6b2ab","properties":{"MusicButler":["https://www.musicbutler.io/artist-page/3996865"],"Qobuz":["https://www.qobuz.com/nl-nl/interpreter/metallica/download-streaming-albums"]}}]} \ No newline at end of file +{"V20240302":[{"name":"Аркона","sort":"Arkona","musicbrainz":"https://musicbrainz.org/artist/baad262d-55ef-427a-83c7-f7530964f212","properties":{"Bandcamp":["https://arkonamoscow.bandcamp.com/"],"MusicButler":["https://www.musicbutler.io/artist-page/283448581"],"Qobuz":["https://www.qobuz.com/nl-nl/interpreter/arkona/download-streaming-albums"]},"albums":[{"title":"Slovo","seq":0}]},{"name":"Eluveitie","sort":null,"musicbrainz":"https://musicbrainz.org/artist/8000598a-5edb-401c-8e6d-36b167feaf38","properties":{"MusicButler":["https://www.musicbutler.io/artist-page/269358403"],"Qobuz":["https://www.qobuz.com/nl-nl/interpreter/eluveitie/download-streaming-albums"]},"albums":[{"title":"Vên [re‐recorded]","seq":0},{"title":"Slania","seq":0}]},{"name":"Frontside","sort":null,"musicbrainz":"https://musicbrainz.org/artist/3a901353-fccd-4afd-ad01-9c03f451b490","properties":{"MusicButler":["https://www.musicbutler.io/artist-page/826588800"],"Qobuz":["https://www.qobuz.com/nl-nl/interpreter/frontside/download-streaming-albums"]},"albums":[{"title":"…nasze jest królestwo, potęga i chwała na wieki…","seq":0}]},{"name":"Heaven’s Basement","sort":"Heaven’s Basement","musicbrainz":"https://musicbrainz.org/artist/c2c4d56a-d599-4a18-bd2f-ae644e2198cc","properties":{"MusicButler":["https://www.musicbutler.io/artist-page/291158685"],"Qobuz":["https://www.qobuz.com/nl-nl/interpreter/heaven-s-basement/download-streaming-albums"]},"albums":[{"title":"Paper Plague","seq":0},{"title":"Unbreakable","seq":0}]},{"name":"Metallica","sort":null,"musicbrainz":"https://musicbrainz.org/artist/65f4f0c5-ef9e-490c-aee3-909e7ae6b2ab","properties":{"MusicButler":["https://www.musicbutler.io/artist-page/3996865"],"Qobuz":["https://www.qobuz.com/nl-nl/interpreter/metallica/download-streaming-albums"]},"albums":[{"title":"Ride the Lightning","seq":0},{"title":"S&M","seq":0}]}]} \ No newline at end of file