src/lib.rs

@@ -813,7 +813,7 @@ impl<LIB, DB> MusicHoard<LIB, DB> {
 
         if self.get_artist(&artist_id).is_none() {
             let new_artist = vec![Artist::new(artist_id)];
-            Self::merge_in_place(&mut self.collection, new_artist);
+            self.merge_with_secondary(new_artist);
         }
     }
 
@@ -872,18 +872,36 @@ impl<LIB, DB> MusicHoard<LIB, DB> {
         }
     }
 
-    fn merge_in_place(primary: &mut Vec<Artist>, secondary: Vec<Artist>) {
+    fn merge_with_primary(&mut self, primary: Vec<Artist>) {
-        let mut primary_map: HashMap<ArtistId, Artist> =
+        let primary_map: HashMap<ArtistId, Artist> =
-            primary.drain(..).map(|a| (a.id.clone(), a)).collect();
+            primary.into_iter().map(|a| (a.id.clone(), a)).collect();
+        let collection = mem::take(&mut self.collection);
+        self.collection = Self::merge_collections(primary_map, collection);
+    }
+
+    fn merge_with_secondary<SEC: IntoIterator<Item = Artist>>(&mut self, secondary: SEC) {
+        let primary_map: HashMap<ArtistId, Artist> = self
+            .collection
+            .drain(..)
+            .map(|a| (a.id.clone(), a))
+            .collect();
+        self.collection = Self::merge_collections(primary_map, secondary);
+    }
+
+    fn merge_collections<SEC: IntoIterator<Item = Artist>>(
+        mut primary: HashMap<ArtistId, Artist>,
+        secondary: SEC,
+    ) -> Collection {
         for secondary_artist in secondary.into_iter() {
-            if let Some(ref mut primary_artist) = primary_map.get_mut(&secondary_artist.id) {
+            if let Some(ref mut primary_artist) = primary.get_mut(&secondary_artist.id) {
                 primary_artist.merge_in_place(secondary_artist);
             } else {
-                primary_map.insert(secondary_artist.id.clone(), secondary_artist);
+                primary.insert(secondary_artist.id.clone(), secondary_artist);
             }
         }
-        primary.extend(primary_map.into_values());
+        let mut collection: Collection = primary.into_values().collect();
-        Self::sort_artists(primary);
+        Self::sort_artists(&mut collection);
+        collection
     }
 
     fn items_to_artists(items: Vec<Item>) -> Result<Vec<Artist>, Error> {
@@ -990,9 +1008,7 @@ impl<LIB: ILibrary, DB> MusicHoard<LIB, DB> {
         let mut library_collection = Self::items_to_artists(items)?;
         Self::sort_albums_and_tracks(&mut library_collection);
 
-        Self::merge_in_place(&mut library_collection, mem::take(&mut self.collection));
+        self.merge_with_primary(library_collection);
-        mem::swap(&mut library_collection, &mut self.collection);
-
         Ok(())
     }
 }
@@ -1003,8 +1019,7 @@ impl<LIB, DB: IDatabase> MusicHoard<LIB, DB> {
         let mut database_collection = self.database.load()?;
         Self::sort_albums_and_tracks(&mut database_collection);
 
-        Self::merge_in_place(&mut self.collection, database_collection);
+        self.merge_with_secondary(database_collection);
-
         Ok(())
     }
 
@@ -1978,13 +1993,17 @@ mod tests {
         let mut expected = COLLECTION.to_owned();
         expected.sort_unstable();
 
-        let mut merged = left.clone();
+        let merged = MusicHoard::<NoLibrary, NoDatabase>::merge_collections(
-        MusicHoard::<NoLibrary, NoDatabase>::merge_in_place(&mut merged, right.clone());
+            left.clone().into_iter().map(|a| (a.id.clone(), a)).collect(),
+            right.clone(),
+        );
         assert_eq!(expected, merged);
 
         // The merge is completele non-overlapping so it should be commutative.
-        let mut merged = right.clone();
+        let merged = MusicHoard::<NoLibrary, NoDatabase>::merge_collections(
-        MusicHoard::<NoLibrary, NoDatabase>::merge_in_place(&mut merged, left.clone());
+            right.clone().into_iter().map(|a| (a.id.clone(), a)).collect(),
+            left.clone(),
+        );
         assert_eq!(expected, merged);
     }
 
@@ -1998,13 +2017,17 @@ mod tests {
         let mut expected = COLLECTION.to_owned();
         expected.sort_unstable();
 
-        let mut merged = left.clone();
+        let merged = MusicHoard::<NoLibrary, NoDatabase>::merge_collections(
-        MusicHoard::<NoLibrary, NoDatabase>::merge_in_place(&mut merged, right.clone());
+            left.clone().into_iter().map(|a| (a.id.clone(), a)).collect(),
+            right.clone(),
+        );
         assert_eq!(expected, merged);
 
         // The merge does not overwrite any data so it should be commutative.
-        let mut merged = right.clone();
+        let merged = MusicHoard::<NoLibrary, NoDatabase>::merge_collections(
-        MusicHoard::<NoLibrary, NoDatabase>::merge_in_place(&mut merged, left.clone());
+            right.clone().into_iter().map(|a| (a.id.clone(), a)).collect(),
+            left.clone(),
+        );
         assert_eq!(expected, merged);
     }
 
@@ -2031,14 +2054,18 @@ mod tests {
         expected.last_mut().as_mut().unwrap().sort = artist_sort.clone();
         expected.rotate_right(1);
 
-        let mut merged = left.clone();
+        let merged = MusicHoard::<NoLibrary, NoDatabase>::merge_collections(
-        MusicHoard::<NoLibrary, NoDatabase>::merge_in_place(&mut merged, right.clone());
+            left.clone().into_iter().map(|a| (a.id.clone(), a)).collect(),
+            right.clone(),
+        );
         assert_eq!(expected.len(), merged.len());
         assert_eq!(expected, merged);
 
         // The merge overwrites the sort data, but no data is erased so it should be commutative.
-        let mut merged = right.clone();
+        let merged = MusicHoard::<NoLibrary, NoDatabase>::merge_collections(
-        MusicHoard::<NoLibrary, NoDatabase>::merge_in_place(&mut merged, left.clone());
+            right.clone().into_iter().map(|a| (a.id.clone(), a)).collect(),
+            left.clone(),
+        );
         assert_eq!(expected.len(), merged.len());
         assert_eq!(expected, merged);
     }