From bd8dd8bae5db689b71aa104cdd7b1603ee3996d0 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sat, 13 Jan 2024 23:59:59 +0100 Subject: [PATCH] Artists cannot be merged with MergeSorted --- src/lib.rs | 95 ++++++++++++++++++++++++++++++++---------------------- 1 file changed, 57 insertions(+), 38 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index a782173..11778a4 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -287,9 +287,8 @@ impl Ord for Track { } impl Merge for Track { - fn merge(self, other: Self) -> Self { + fn merge_in_place(&mut self, other: Self) { assert_eq!(self.id, other.id); - self } } @@ -320,10 +319,10 @@ impl Ord for Album { } impl Merge for Album { - fn merge(mut self, other: Self) -> Self { + fn merge_in_place(&mut self, other: Self) { assert_eq!(self.id, other.id); - self.tracks = MergeSorted::new(self.tracks.into_iter(), other.tracks.into_iter()).collect(); - self + let tracks = mem::take(&mut self.tracks); + self.tracks = MergeSorted::new(tracks.into_iter(), other.tracks.into_iter()).collect(); } } @@ -361,13 +360,11 @@ pub struct ArtistProperties { } impl Merge for ArtistProperties { - fn merge(mut self, other: Self) -> Self { - self.musicbrainz = Self::merge_opts(self.musicbrainz, other.musicbrainz); - self.musicbutler = Self::merge_vecs(self.musicbutler, other.musicbutler); - self.bandcamp = Self::merge_vecs(self.bandcamp, other.bandcamp); - self.qobuz = Self::merge_opts(self.qobuz, other.qobuz); - - self + fn merge_in_place(&mut self, other: Self) { + self.musicbrainz = self.musicbrainz.take().or(other.musicbrainz); + Self::merge_vecs(&mut self.musicbutler, other.musicbutler); + Self::merge_vecs(&mut self.bandcamp, other.bandcamp); + self.qobuz = self.qobuz.take().or(other.qobuz); } } @@ -566,12 +563,12 @@ impl Ord for Artist { } impl Merge for Artist { - fn merge(mut self, other: Self) -> Self { + fn merge_in_place(&mut self, other: Self) { assert_eq!(self.id, other.id); - self.sort = Self::merge_opts(self.sort, other.sort); - self.properties = self.properties.merge(other.properties); - self.albums = MergeSorted::new(self.albums.into_iter(), other.albums.into_iter()).collect(); - self + self.sort = self.sort.take().or(other.sort); + 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(); } } @@ -579,20 +576,20 @@ impl Merge for Artist { pub type Collection = Vec; trait Merge { - fn merge(self, other: Self) -> Self; + fn merge_in_place(&mut self, other: Self); - fn merge_opts(this: Option, other: Option) -> Option { - match &this { - Some(_) => this, - None => other, - } + fn merge(mut self, other: Self) -> Self + where + Self: Sized, + { + self.merge_in_place(other); + self } - fn merge_vecs(mut this: Vec, mut other: Vec) -> Vec { + fn merge_vecs(this: &mut Vec, mut other: Vec) { this.append(&mut other); this.sort_unstable(); this.dedup(); - this } } @@ -816,9 +813,7 @@ impl MusicHoard { if self.get_artist(&artist_id).is_none() { let new_artist = vec![Artist::new(artist_id)]; - - let collection = mem::take(&mut self.collection); - self.collection = Self::merge(collection, new_artist); + Self::merge_in_place(&mut self.collection, new_artist); } } @@ -869,8 +864,31 @@ impl MusicHoard { } } - fn merge(primary: Vec, secondary: Vec) -> Vec { - MergeSorted::new(primary.into_iter(), secondary.into_iter()).collect() + fn sort_artists(collection: &mut [Artist]) { + collection.sort_unstable(); + } + + fn sort_albums_and_tracks(collection: &mut [Artist]) { + for artist in collection.iter_mut() { + artist.albums.sort_unstable(); + for album in artist.albums.iter_mut() { + album.tracks.sort_unstable(); + } + } + } + + fn merge_in_place(primary: &mut Vec, secondary: Vec) { + let mut primary_map: HashMap = + primary.drain(..).map(|a| (a.id.clone(), a)).collect(); + for secondary_artist in secondary.into_iter() { + if let Some(ref mut primary_artist) = primary_map.get_mut(&secondary_artist.id) { + primary_artist.merge_in_place(secondary_artist); + } else { + primary_map.insert(secondary_artist.id.clone(), secondary_artist); + } + } + primary.extend(primary_map.into_values()); + Self::sort_artists(primary); } fn items_to_artists(items: Vec) -> Result, Error> { @@ -975,10 +993,10 @@ impl MusicHoard { pub fn rescan_library(&mut self) -> Result<(), Error> { let items = self.library.list(&Query::new())?; let mut library_collection = Self::items_to_artists(items)?; - Self::sort(&mut library_collection); + Self::sort_albums_and_tracks(&mut library_collection); - let collection = mem::take(&mut self.collection); - self.collection = Self::merge(library_collection, collection); + Self::merge_in_place(&mut library_collection, mem::take(&mut self.collection)); + mem::swap(&mut library_collection, &mut self.collection); Ok(()) } @@ -988,10 +1006,9 @@ impl MusicHoard { /// Load the database and merge with the in-memory collection. pub fn load_from_database(&mut self) -> Result<(), Error> { let mut database_collection = self.database.load()?; - Self::sort(&mut database_collection); + Self::sort_albums_and_tracks(&mut database_collection); - let collection = mem::take(&mut self.collection); - self.collection = Self::merge(collection, database_collection); + Self::merge_in_place(&mut self.collection, database_collection); Ok(()) } @@ -1957,7 +1974,8 @@ mod tests { let mut expected = COLLECTION.to_owned(); expected.sort_unstable(); - let merged = MusicHoard::::merge(left.clone(), right); + let mut merged = left; + MusicHoard::::merge_in_place(&mut merged, right); assert_eq!(expected, merged); } @@ -1971,7 +1989,8 @@ mod tests { let mut expected = COLLECTION.to_owned(); expected.sort_unstable(); - let merged = MusicHoard::::merge(left.clone(), right); + let mut merged = left; + MusicHoard::::merge_in_place(&mut merged, right); assert_eq!(expected, merged); }