Artists cannot be merged with MergeSorted

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<Artist>;
 
 trait Merge {
-    fn merge(self, other: Self) -> Self;
+    fn merge_in_place(&mut self, other: Self);
 
-    fn merge_opts<T>(this: Option<T>, other: Option<T>) -> Option<T> {
-        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<T: Ord + Eq>(mut this: Vec<T>, mut other: Vec<T>) -> Vec<T> {
+    fn merge_vecs<T: Ord + Eq>(this: &mut Vec<T>, mut other: Vec<T>) {
         this.append(&mut other);
         this.sort_unstable();
         this.dedup();
-        this
     }
 }
 
@@ -816,9 +813,7 @@ impl<LIB, DB> MusicHoard<LIB, DB> {
 
         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<LIB, DB> MusicHoard<LIB, DB> {
         }
     }
 
-    fn merge(primary: Vec<Artist>, secondary: Vec<Artist>) -> Vec<Artist> {
-        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<Artist>, secondary: Vec<Artist>) {
+        let mut primary_map: HashMap<ArtistId, Artist> =
+            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<Item>) -> Result<Vec<Artist>, Error> {
@@ -975,10 +993,10 @@ impl<LIB: ILibrary, DB> MusicHoard<LIB, DB> {
     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<LIB, DB: IDatabase> MusicHoard<LIB, DB> {
     /// 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::<NoLibrary, NoDatabase>::merge(left.clone(), right);
+        let mut merged = left;
+        MusicHoard::<NoLibrary, NoDatabase>::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::<NoLibrary, NoDatabase>::merge(left.clone(), right);
+        let mut merged = left;
+        MusicHoard::<NoLibrary, NoDatabase>::merge_in_place(&mut merged, right);
         assert_eq!(expected, merged);
     }