Add new unit tests

src/lib.rs

@@ -435,15 +435,6 @@ impl Artist {
         }
     }
 
-    pub fn new_with_sort<ID: Into<ArtistId>, SORT: Into<ArtistId>>(id: ID, sort: SORT) -> Self {
-        Artist {
-            id: id.into(),
-            sort: Some(sort.into()),
-            properties: ArtistProperties::default(),
-            albums: vec![],
-        }
-    }
-
     fn get_sort_key(&self) -> &ArtistId {
         self.sort.as_ref().unwrap_or(&self.id)
     }
@@ -738,7 +729,7 @@ macro_rules! music_hoard_unique_url_dispatch {
                 artist_id: ID,
                 url: S,
             ) -> Result<(), Error> {
-                self.get_artist_or_err(artist_id.as_ref())?.[<add_ $field _url>](url)
+                self.get_artist_mut_or_err(artist_id.as_ref())?.[<add_ $field _url>](url)
             }
 
             pub fn [<remove_ $field _url>]<ID: AsRef<ArtistId>, S: AsRef<str>>(
@@ -746,7 +737,7 @@ macro_rules! music_hoard_unique_url_dispatch {
                 artist_id: ID,
                 url: S,
             ) -> Result<(), Error> {
-                self.get_artist_or_err(artist_id.as_ref())?.[<remove_ $field _url>](url)
+                self.get_artist_mut_or_err(artist_id.as_ref())?.[<remove_ $field _url>](url)
             }
 
             pub fn [<set_ $field _url>]<ID: AsRef<ArtistId>, S: AsRef<str>>(
@@ -754,14 +745,14 @@ macro_rules! music_hoard_unique_url_dispatch {
                 artist_id: ID,
                 url: S,
             ) -> Result<(), Error> {
-                self.get_artist_or_err(artist_id.as_ref())?.[<set_ $field _url>](url)
+                self.get_artist_mut_or_err(artist_id.as_ref())?.[<set_ $field _url>](url)
             }
 
             pub fn [<clear_ $field _url>]<ID: AsRef<ArtistId>>(
                 &mut self,
                 artist_id: ID,
             ) -> Result<(), Error> {
-                self.get_artist_or_err(artist_id.as_ref())?.[<clear_ $field _url>]();
+                self.get_artist_mut_or_err(artist_id.as_ref())?.[<clear_ $field _url>]();
                 Ok(())
             }
         }
@@ -776,7 +767,7 @@ macro_rules! music_hoard_multi_url_dispatch {
                 artist_id: ID,
                 urls: Vec<S>,
             ) -> Result<(), Error> {
-                self.get_artist_or_err(artist_id.as_ref())?.[<add_ $field _urls>](urls)
+                self.get_artist_mut_or_err(artist_id.as_ref())?.[<add_ $field _urls>](urls)
             }
 
             pub fn [<remove_ $field _urls>]<ID: AsRef<ArtistId>, S: AsRef<str>>(
@@ -784,7 +775,7 @@ macro_rules! music_hoard_multi_url_dispatch {
                 artist_id: ID,
                 urls: Vec<S>,
             ) -> Result<(), Error> {
-                self.get_artist_or_err(artist_id.as_ref())?.[<remove_ $field _urls>](urls)
+                self.get_artist_mut_or_err(artist_id.as_ref())?.[<remove_ $field _urls>](urls)
             }
 
             pub fn [<set_ $field _urls>]<ID: AsRef<ArtistId>, S: AsRef<str>>(
@@ -792,13 +783,13 @@ macro_rules! music_hoard_multi_url_dispatch {
                 artist_id: ID,
                 urls: Vec<S>,
             ) -> Result<(), Error> {
-                self.get_artist_or_err(artist_id.as_ref())?.[<set_ $field _urls>](urls)
+                self.get_artist_mut_or_err(artist_id.as_ref())?.[<set_ $field _urls>](urls)
             }
 
             pub fn [<clear_ $field _urls>]<ID: AsRef<ArtistId>>(
                 &mut self, artist_id: ID,
             ) -> Result<(), Error> {
-                self.get_artist_or_err(artist_id.as_ref())?.[<clear_ $field _urls>]();
+                self.get_artist_mut_or_err(artist_id.as_ref())?.[<clear_ $field _urls>]();
                 Ok(())
             }
         }
@@ -847,13 +838,16 @@ impl<LIB, DB> MusicHoard<LIB, DB> {
         artist_id: ID,
         artist_sort: SORT,
     ) -> Result<(), Error> {
-        self.get_artist_or_err(artist_id.as_ref())?
+        self.get_artist_mut_or_err(artist_id.as_ref())?
             .set_sort_key(artist_sort);
+        Self::sort(&mut self.collection);
         Ok(())
     }
 
     pub fn clear_artist_sort<ID: AsRef<ArtistId>>(&mut self, artist_id: ID) -> Result<(), Error> {
-        self.get_artist_or_err(artist_id.as_ref())?.clear_sort_key();
+        self.get_artist_mut_or_err(artist_id.as_ref())?
+            .clear_sort_key();
+        Self::sort(&mut self.collection);
         Ok(())
     }
 
@@ -961,12 +955,16 @@ impl<LIB, DB> MusicHoard<LIB, DB> {
         Ok(artists)
     }
 
-    fn get_artist(&mut self, artist_id: &ArtistId) -> Option<&mut Artist> {
+    fn get_artist(&self, artist_id: &ArtistId) -> Option<&Artist> {
+        self.collection.iter().find(|a| &a.id == artist_id)
+    }
+
+    fn get_artist_mut(&mut self, artist_id: &ArtistId) -> Option<&mut Artist> {
         self.collection.iter_mut().find(|a| &a.id == artist_id)
     }
 
-    fn get_artist_or_err(&mut self, artist_id: &ArtistId) -> Result<&mut Artist, Error> {
+    fn get_artist_mut_or_err(&mut self, artist_id: &ArtistId) -> Result<&mut Artist, Error> {
-        self.get_artist(artist_id).ok_or_else(|| {
+        self.get_artist_mut(artist_id).ok_or_else(|| {
             Error::CollectionError(format!("artist '{}' is not in the collection", artist_id))
         })
     }
@@ -1192,6 +1190,44 @@ mod tests {
         assert_eq!(music_hoard.collection, expected);
     }
 
+    #[test]
+    fn artist_sort_set_clear() {
+        let mut music_hoard = MusicHoardBuilder::default().build();
+
+        let artist_1_id = ArtistId::new("the artist");
+        let artist_1_sort = ArtistId::new("artist, the");
+
+        // Must be after "artist, the", but before "the artist"
+        let artist_2_id = ArtistId::new("b-artist");
+
+        assert!(artist_1_sort < artist_2_id);
+        assert!(artist_2_id < artist_1_id);
+
+        music_hoard.add_artist(artist_1_id.clone());
+        music_hoard.add_artist(artist_2_id.clone());
+
+        let artist_1: &Artist = music_hoard.get_artist(&artist_1_id).unwrap();
+        let artist_2: &Artist = music_hoard.get_artist(&artist_2_id).unwrap();
+
+        assert!(artist_2 < artist_1);
+
+        music_hoard
+            .set_artist_sort(artist_1_id.as_ref(), artist_1_sort.clone())
+            .unwrap();
+
+        let artist_1: &Artist = music_hoard.get_artist(&artist_1_id).unwrap();
+        let artist_2: &Artist = music_hoard.get_artist(&artist_2_id).unwrap();
+
+        assert!(artist_1 < artist_2);
+
+        music_hoard.clear_artist_sort(artist_1_id.as_ref()).unwrap();
+
+        let artist_1: &Artist = music_hoard.get_artist(&artist_1_id).unwrap();
+        let artist_2: &Artist = music_hoard.get_artist(&artist_2_id).unwrap();
+
+        assert!(artist_2 < artist_1);
+    }
+
     #[test]
     fn collection_error() {
         let artist_id = ArtistId::new("an artist");
@@ -2013,6 +2049,42 @@ mod tests {
         assert_eq!(music_hoard.get_collection(), &expected);
     }
 
+    #[test]
+    fn rescan_library_album_artist_sort_clash() {
+        let mut library = MockILibrary::new();
+        let database = MockIDatabase::new();
+
+        let expected = clean_collection(COLLECTION.to_owned());
+        let library_input = Query::new();
+        let mut library_items = artists_to_items(&expected);
+
+        assert_eq!(library_items[0].album_artist, library_items[1].album_artist);
+        library_items[0].album_artist_sort = Some(library_items[0].album_artist.clone());
+        library_items[1].album_artist_sort = Some(
+            library_items[1]
+                .album_artist
+                .clone()
+                .chars()
+                .rev()
+                .collect(),
+        );
+
+        let library_result = Ok(library_items);
+
+        library
+            .expect_list()
+            .with(predicate::eq(library_input))
+            .times(1)
+            .return_once(|_| library_result);
+
+        let mut music_hoard = MusicHoardBuilder::default()
+            .set_library(library)
+            .set_database(database)
+            .build();
+
+        assert!(music_hoard.rescan_library().is_err());
+    }
+
     #[test]
     fn load_database() {
         let library = MockILibrary::new();

src/library/beets/mod.rs

@@ -235,6 +235,7 @@ mod tests {
 
         let mut query = Query::default()
             .exclude(Field::AlbumArtist(String::from("some.albumartist")))
+            .exclude(Field::AlbumArtistSort(String::from("some.albumartist")))
             .include(Field::AlbumYear(3030))
             .include(Field::TrackTitle(String::from("some.track")))
             .exclude(Field::TrackArtist(vec![
@@ -248,6 +249,7 @@ mod tests {
             query,
             vec![
                 String::from("^albumartist:some.albumartist"),
+                String::from("^albumartist_sort:some.albumartist"),
                 String::from("^artist:some.artist.1; some.artist.2"),
                 String::from("title:some.track"),
                 String::from("year:3030"),