Distinguish NoLibrary/NoDatabase from EmptyLibrary/EmptyDatabase (#97)

Closes #95

Reviewed-on: #97

src/bin/musichoard-edit.rs

@@ -4,8 +4,7 @@ use structopt::{clap::AppSettings, StructOpt};
 
 use musichoard::{
     database::json::{backend::JsonDatabaseFileBackend, JsonDatabase},
-    library::NoLibrary,
-    ArtistId, MusicHoard,
+    ArtistId, MusicHoard, MusicHoardBuilder, NoLibrary,
 };
 
 type MH = MusicHoard<NoLibrary, JsonDatabase<JsonDatabaseFileBackend>>;
@@ -157,12 +156,9 @@ impl ArtistCommand {
 fn main() {
     let opt = Opt::from_args();
 
-    let lib: Option<NoLibrary> = None;
-    let db = Some(JsonDatabase::new(JsonDatabaseFileBackend::new(
-        &opt.database_file_path,
-    )));
+    let db = JsonDatabase::new(JsonDatabaseFileBackend::new(&opt.database_file_path));
 
-    let mut music_hoard = MusicHoard::new(lib, db);
+    let mut music_hoard = MusicHoardBuilder::default().set_database(db).build();
     music_hoard
         .load_from_database()
         .expect("failed to load database");

src/database/mod.rs

@@ -20,16 +20,16 @@ pub trait IDatabase {
     fn save<S: Serialize + 'static>(&mut self, collection: &S) -> Result<(), SaveError>;
 }
 
-/// Non-implementation defined to make generics easier.
-pub struct NoDatabase {}
+/// Null database implementation of [`IDatabase`].
+pub struct NullDatabase;
 
-impl IDatabase for NoDatabase {
+impl IDatabase for NullDatabase {
     fn load<D: DeserializeOwned + 'static>(&self, _collection: &mut D) -> Result<(), LoadError> {
-        panic!()
+        Ok(())
     }
 
     fn save<S: Serialize + 'static>(&mut self, _collection: &S) -> Result<(), SaveError> {
-        panic!()
+        Ok(())
     }
 }
 
@@ -89,22 +89,21 @@ impl From<std::io::Error> for SaveError {
 mod tests {
     use std::io;
 
-    use super::{IDatabase, LoadError, NoDatabase, SaveError};
+    use super::{IDatabase, LoadError, NullDatabase, SaveError};
 
     #[test]
-    #[should_panic]
     fn no_database_load() {
-        let database = NoDatabase {};
+        let database = NullDatabase;
         let mut collection: Vec<String> = vec![];
-        _ = database.load(&mut collection);
+        assert!(database.load(&mut collection).is_ok());
+        assert!(collection.is_empty());
     }
 
     #[test]
-    #[should_panic]
     fn no_database_save() {
-        let mut database = NoDatabase {};
+        let mut database = NullDatabase;
         let collection: Vec<String> = vec![];
-        _ = database.save(&collection);
+        assert!(database.save(&collection).is_ok());
     }
 
     #[test]

src/lib.rs

@@ -693,11 +693,15 @@ impl From<InvalidUrlError> for Error {
 /// The Music Hoard. It is responsible for pulling information from both the library and the
 /// database, ensuring its consistent and writing back any changes.
 pub struct MusicHoard<LIB, DB> {
-    library: Option<LIB>,
-    database: Option<DB>,
     collection: Collection,
+    library: LIB,
+    database: DB,
 }
 
+// Unit structs to ensure library/database are not compiled in.
+pub struct NoLibrary;
+pub struct NoDatabase;
+
 macro_rules! music_hoard_unique_url_dispatch {
     ($field:ident) => {
         paste! {
@@ -773,13 +777,13 @@ macro_rules! music_hoard_multi_url_dispatch {
     };
 }
 
-impl<LIB: ILibrary, DB: IDatabase> MusicHoard<LIB, DB> {
+impl<LIB, DB> MusicHoard<LIB, DB> {
     /// Create a new [`MusicHoard`] with the provided [`ILibrary`] and [`IDatabase`].
-    pub fn new(library: Option<LIB>, database: Option<DB>) -> Self {
+    pub fn new(library: LIB, database: DB) -> Self {
         MusicHoard {
+            collection: vec![],
             library,
             database,
-            collection: vec![],
         }
     }
 
@@ -787,37 +791,6 @@ impl<LIB: ILibrary, DB: IDatabase> MusicHoard<LIB, DB> {
         &self.collection
     }
 
-    pub fn rescan_library(&mut self) -> Result<(), Error> {
-        if let Some(ref mut library) = self.library {
-            let items = library.list(&Query::new())?;
-            let mut library_collection = Self::items_to_artists(items);
-            Self::sort(&mut library_collection);
-
-            let collection = mem::take(&mut self.collection);
-            self.collection = Self::merge(library_collection, collection);
-        }
-        Ok(())
-    }
-
-    pub fn load_from_database(&mut self) -> Result<(), Error> {
-        if let Some(ref mut database) = self.database {
-            let mut database_collection: Collection = vec![];
-            database.load(&mut database_collection)?;
-            Self::sort(&mut database_collection);
-
-            let collection = mem::take(&mut self.collection);
-            self.collection = Self::merge(collection, database_collection);
-        }
-        Ok(())
-    }
-
-    pub fn save_to_database(&mut self) -> Result<(), Error> {
-        if let Some(ref mut database) = self.database {
-            database.save(&self.collection)?;
-        }
-        Ok(())
-    }
-
     pub fn add_artist<ID: Into<ArtistId>>(&mut self, artist_id: ID) {
         let artist_id: ArtistId = artist_id.into();
 
@@ -940,6 +913,84 @@ impl<LIB: ILibrary, DB: IDatabase> MusicHoard<LIB, DB> {
     }
 }
 
+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);
+
+        let collection = mem::take(&mut self.collection);
+        self.collection = Self::merge(library_collection, collection);
+
+        Ok(())
+    }
+}
+
+impl<LIB, DB: IDatabase> MusicHoard<LIB, DB> {
+    pub fn load_from_database(&mut self) -> Result<(), Error> {
+        let mut database_collection: Collection = vec![];
+        self.database.load(&mut database_collection)?;
+        Self::sort(&mut database_collection);
+
+        let collection = mem::take(&mut self.collection);
+        self.collection = Self::merge(collection, database_collection);
+
+        Ok(())
+    }
+
+    pub fn save_to_database(&mut self) -> Result<(), Error> {
+        self.database.save(&self.collection)?;
+        Ok(())
+    }
+}
+
+/// Builder for [`MusicHoard`]. Its purpose is to make it easier to set various combinations of
+/// library/database or their absence.
+pub struct MusicHoardBuilder<LIB, DB> {
+    library: LIB,
+    database: DB,
+}
+
+impl Default for MusicHoardBuilder<NoLibrary, NoDatabase> {
+    /// Create a [`MusicHoardBuilder`].
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
+impl MusicHoardBuilder<NoLibrary, NoDatabase> {
+    /// Create a [`MusicHoardBuilder`].
+    pub fn new() -> Self {
+        MusicHoardBuilder {
+            library: NoLibrary,
+            database: NoDatabase,
+        }
+    }
+}
+
+impl<LIB, DB> MusicHoardBuilder<LIB, DB> {
+    /// Set a library for [`MusicHoard`].
+    pub fn set_library<NEWLIB: ILibrary>(self, library: NEWLIB) -> MusicHoardBuilder<NEWLIB, DB> {
+        MusicHoardBuilder {
+            library,
+            database: self.database,
+        }
+    }
+
+    /// Set a database for [`MusicHoard`].
+    pub fn set_database<NEWDB: IDatabase>(self, database: NEWDB) -> MusicHoardBuilder<LIB, NEWDB> {
+        MusicHoardBuilder {
+            library: self.library,
+            database,
+        }
+    }
+
+    /// Build [`MusicHoard`] with the currently set library and database.
+    pub fn build(self) -> MusicHoard<LIB, DB> {
+        MusicHoard::new(self.library, self.database)
+    }
+}
+
 #[cfg(test)]
 #[macro_use]
 mod testlib;
@@ -949,10 +1000,7 @@ mod tests {
     use mockall::predicate;
     use once_cell::sync::Lazy;
 
-    use crate::{
-        database::{MockIDatabase, NoDatabase},
-        library::{MockILibrary, NoLibrary},
-    };
+    use crate::{database::MockIDatabase, library::MockILibrary};
 
     use super::*;
 
@@ -1063,7 +1111,7 @@ mod tests {
     fn artist_new_delete() {
         let artist_id = ArtistId::new("an artist");
         let artist_id_2 = ArtistId::new("another artist");
-        let mut music_hoard = MusicHoard::<NoLibrary, NoDatabase>::new(None, None);
+        let mut music_hoard = MusicHoardBuilder::default().build();
 
         let mut expected: Vec<Artist> = vec![];
 
@@ -1085,7 +1133,7 @@ mod tests {
     #[test]
     fn collection_error() {
         let artist_id = ArtistId::new("an artist");
-        let mut music_hoard = MusicHoard::<NoLibrary, NoDatabase>::new(None, None);
+        let mut music_hoard = MusicHoardBuilder::default().build();
 
         let actual_err = music_hoard
             .add_musicbrainz_url(&artist_id, QOBUZ)
@@ -1100,7 +1148,7 @@ mod tests {
     fn add_remove_musicbrainz_url() {
         let artist_id = ArtistId::new("an artist");
         let artist_id_2 = ArtistId::new("another artist");
-        let mut music_hoard = MusicHoard::<NoLibrary, NoDatabase>::new(None, None);
+        let mut music_hoard = MusicHoardBuilder::default().build();
 
         music_hoard.add_artist(artist_id.clone());
 
@@ -1171,7 +1219,7 @@ mod tests {
     fn set_clear_musicbrainz_url() {
         let artist_id = ArtistId::new("an artist");
         let artist_id_2 = ArtistId::new("another artist");
-        let mut music_hoard = MusicHoard::<NoLibrary, NoDatabase>::new(None, None);
+        let mut music_hoard = MusicHoardBuilder::default().build();
 
         music_hoard.add_artist(artist_id.clone());
 
@@ -1226,7 +1274,7 @@ mod tests {
     fn add_remove_musicbutler_urls() {
         let artist_id = ArtistId::new("an artist");
         let artist_id_2 = ArtistId::new("another artist");
-        let mut music_hoard = MusicHoard::<NoLibrary, NoDatabase>::new(None, None);
+        let mut music_hoard = MusicHoardBuilder::default().build();
 
         music_hoard.add_artist(artist_id.clone());
 
@@ -1356,7 +1404,7 @@ mod tests {
     fn set_clear_musicbutler_urls() {
         let artist_id = ArtistId::new("an artist");
         let artist_id_2 = ArtistId::new("another artist");
-        let mut music_hoard = MusicHoard::<NoLibrary, NoDatabase>::new(None, None);
+        let mut music_hoard = MusicHoardBuilder::default().build();
 
         music_hoard.add_artist(artist_id.clone());
 
@@ -1419,7 +1467,7 @@ mod tests {
     fn add_remove_bandcamp_urls() {
         let artist_id = ArtistId::new("an artist");
         let artist_id_2 = ArtistId::new("another artist");
-        let mut music_hoard = MusicHoard::<NoLibrary, NoDatabase>::new(None, None);
+        let mut music_hoard = MusicHoardBuilder::default().build();
 
         music_hoard.add_artist(artist_id.clone());
 
@@ -1549,7 +1597,7 @@ mod tests {
     fn set_clear_bandcamp_urls() {
         let artist_id = ArtistId::new("an artist");
         let artist_id_2 = ArtistId::new("another artist");
-        let mut music_hoard = MusicHoard::<NoLibrary, NoDatabase>::new(None, None);
+        let mut music_hoard = MusicHoardBuilder::default().build();
 
         music_hoard.add_artist(artist_id.clone());
 
@@ -1612,7 +1660,7 @@ mod tests {
     fn add_remove_qobuz_url() {
         let artist_id = ArtistId::new("an artist");
         let artist_id_2 = ArtistId::new("another artist");
-        let mut music_hoard = MusicHoard::<NoLibrary, NoDatabase>::new(None, None);
+        let mut music_hoard = MusicHoardBuilder::default().build();
 
         music_hoard.add_artist(artist_id.clone());
 
@@ -1663,7 +1711,7 @@ mod tests {
     fn set_clear_qobuz_url() {
         let artist_id = ArtistId::new("an artist");
         let artist_id_2 = ArtistId::new("another artist");
-        let mut music_hoard = MusicHoard::<NoLibrary, NoDatabase>::new(None, None);
+        let mut music_hoard = MusicHoardBuilder::default().build();
 
         music_hoard.add_artist(artist_id.clone());
 
@@ -1802,7 +1850,7 @@ mod tests {
         let mut expected = COLLECTION.to_owned();
         expected.sort_unstable();
 
-        let merged = MusicHoard::<MockILibrary, MockIDatabase>::merge(left.clone(), right);
+        let merged = MusicHoard::<NoLibrary, NoDatabase>::merge(left.clone(), right);
         assert_eq!(expected, merged);
     }
 
@@ -1816,7 +1864,7 @@ mod tests {
         let mut expected = COLLECTION.to_owned();
         expected.sort_unstable();
 
-        let merged = MusicHoard::<MockILibrary, MockIDatabase>::merge(left.clone(), right);
+        let merged = MusicHoard::<NoLibrary, NoDatabase>::merge(left.clone(), right);
         assert_eq!(expected, merged);
     }
 
@@ -1834,7 +1882,10 @@ mod tests {
             .times(1)
             .return_once(|_| library_result);
 
-        let mut music_hoard = MusicHoard::new(Some(library), Some(database));
+        let mut music_hoard = MusicHoardBuilder::default()
+            .set_library(library)
+            .set_database(database)
+            .build();
 
         music_hoard.rescan_library().unwrap();
         assert_eq!(
@@ -1861,7 +1912,10 @@ mod tests {
             .times(1)
             .return_once(|_| library_result);
 
-        let mut music_hoard = MusicHoard::new(Some(library), Some(database));
+        let mut music_hoard = MusicHoardBuilder::default()
+            .set_library(library)
+            .set_database(database)
+            .build();
 
         music_hoard.rescan_library().unwrap();
         assert_eq!(
@@ -1888,7 +1942,10 @@ mod tests {
             .times(1)
             .return_once(|_| library_result);
 
-        let mut music_hoard = MusicHoard::new(Some(library), Some(database));
+        let mut music_hoard = MusicHoardBuilder::default()
+            .set_library(library)
+            .set_database(database)
+            .build();
 
         music_hoard.rescan_library().unwrap();
         assert_eq!(music_hoard.get_collection(), &expected);
@@ -1907,7 +1964,10 @@ mod tests {
                 Ok(())
             });
 
-        let mut music_hoard = MusicHoard::new(Some(library), Some(database));
+        let mut music_hoard = MusicHoardBuilder::default()
+            .set_library(library)
+            .set_database(database)
+            .build();
 
         music_hoard.load_from_database().unwrap();
         assert_eq!(music_hoard.get_collection(), &*COLLECTION);
@@ -1936,7 +1996,10 @@ mod tests {
             .times(1)
             .return_once(|_: &Collection| database_result);
 
-        let mut music_hoard = MusicHoard::new(Some(library), Some(database));
+        let mut music_hoard = MusicHoardBuilder::default()
+            .set_library(library)
+            .set_database(database)
+            .build();
 
         music_hoard.rescan_library().unwrap();
         assert_eq!(
@@ -1946,15 +2009,6 @@ mod tests {
         music_hoard.save_to_database().unwrap();
     }
 
-    #[test]
-    fn library_not_provided() {
-        let library: Option<NoLibrary> = None;
-        let database = Some(MockIDatabase::new());
-        let mut music_hoard = MusicHoard::new(library, database);
-
-        assert!(music_hoard.rescan_library().is_ok());
-    }
-
     #[test]
     fn library_error() {
         let mut library = MockILibrary::new();
@@ -1967,7 +2021,10 @@ mod tests {
             .times(1)
             .return_once(|_| library_result);
 
-        let mut music_hoard = MusicHoard::new(Some(library), Some(database));
+        let mut music_hoard = MusicHoardBuilder::default()
+            .set_library(library)
+            .set_database(database)
+            .build();
 
         let actual_err = music_hoard.rescan_library().unwrap_err();
         let expected_err =
@@ -1977,16 +2034,6 @@ mod tests {
         assert_eq!(actual_err.to_string(), expected_err.to_string());
     }
 
-    #[test]
-    fn database_not_provided() {
-        let library = Some(MockILibrary::new());
-        let database: Option<NoDatabase> = None;
-        let mut music_hoard = MusicHoard::new(library, database);
-
-        assert!(music_hoard.load_from_database().is_ok());
-        assert!(music_hoard.save_to_database().is_ok());
-    }
-
     #[test]
     fn database_load_error() {
         let library = MockILibrary::new();
@@ -1999,7 +2046,10 @@ mod tests {
             .times(1)
             .return_once(|_: &mut Collection| database_result);
 
-        let mut music_hoard = MusicHoard::new(Some(library), Some(database));
+        let mut music_hoard = MusicHoardBuilder::default()
+            .set_library(library)
+            .set_database(database)
+            .build();
 
         let actual_err = music_hoard.load_from_database().unwrap_err();
         let expected_err = Error::DatabaseError(
@@ -2022,7 +2072,10 @@ mod tests {
             .times(1)
             .return_once(|_: &Collection| database_result);
 
-        let mut music_hoard = MusicHoard::new(Some(library), Some(database));
+        let mut music_hoard = MusicHoardBuilder::default()
+            .set_library(library)
+            .set_database(database)
+            .build();
 
         let actual_err = music_hoard.save_to_database().unwrap_err();
         let expected_err = Error::DatabaseError(

src/library/mod.rs

@@ -17,12 +17,12 @@ pub trait ILibrary {
     fn list(&mut self, query: &Query) -> Result<Vec<Item>, Error>;
 }
 
-/// Non-implementation defined to make generics easier.
-pub struct NoLibrary {}
+/// Null library implementation for [`ILibrary`].
+pub struct NullLibrary;
 
-impl ILibrary for NoLibrary {
+impl ILibrary for NullLibrary {
     fn list(&mut self, _query: &Query) -> Result<Vec<Item>, Error> {
-        panic!()
+        Ok(vec![])
     }
 }
 
@@ -136,13 +136,12 @@ impl From<Utf8Error> for Error {
 mod tests {
     use std::io;
 
-    use super::{Error, Field, ILibrary, NoLibrary, Query};
+    use super::{Error, Field, ILibrary, NullLibrary, Query};
 
     #[test]
-    #[should_panic]
     fn no_library_list() {
-        let mut library = NoLibrary {};
-        _ = library.list(&Query::default());
+        let mut library = NullLibrary;
+        assert!(library.list(&Query::default()).unwrap().is_empty());
     }
 
     #[test]

src/main.rs

@@ -2,9 +2,9 @@ use std::fs::OpenOptions;
 use std::path::PathBuf;
 use std::{ffi::OsString, io};
 
-use musichoard::database::NoDatabase;
-use musichoard::library::NoLibrary;
-use musichoard::Collection;
+use musichoard::database::NullDatabase;
+use musichoard::library::{ILibrary, NullLibrary};
+use musichoard::{Collection, MusicHoardBuilder};
 use ratatui::{backend::CrosstermBackend, Terminal};
 use structopt::StructOpt;
 
@@ -13,14 +13,11 @@ use musichoard::{
         json::{backend::JsonDatabaseFileBackend, JsonDatabase},
         IDatabase,
     },
-    library::{
-        beets::{
-            executor::{ssh::BeetsLibrarySshExecutor, BeetsLibraryProcessExecutor},
-            BeetsLibrary,
-        },
-        ILibrary,
+    library::beets::{
+        executor::{ssh::BeetsLibrarySshExecutor, BeetsLibraryProcessExecutor},
+        BeetsLibrary,
     },
-    MusicHoard,
+    NoLibrary,
 };
 
 mod tui;
@@ -49,8 +46,8 @@ struct Opt {
     no_database: bool,
 }
 
-fn with<LIB: ILibrary, DB: IDatabase>(lib: Option<LIB>, db: Option<DB>) {
-    let music_hoard = MusicHoard::new(lib, db);
+fn with<LIB: ILibrary, DB: IDatabase>(builder: MusicHoardBuilder<LIB, DB>) {
+    let music_hoard = builder.build();
 
     // Initialize the terminal user interface.
     let backend = CrosstermBackend::new(io::stdout());
@@ -66,9 +63,9 @@ fn with<LIB: ILibrary, DB: IDatabase>(lib: Option<LIB>, db: Option<DB>) {
     Tui::run(terminal, ui, handler, listener).expect("failed to run tui");
 }
 
-fn with_database<DB: IDatabase>(opt: Opt, db: Option<DB>) {
+fn with_database<DB: IDatabase>(opt: Opt, builder: MusicHoardBuilder<NoLibrary, DB>) {
     if opt.no_library {
-        with(None::<NoLibrary>, db);
+        with(builder.set_library(NullLibrary));
     } else if let Some(uri) = opt.beets_ssh_uri {
         let uri = uri.into_string().expect("invalid SSH URI");
         let beets_config_file_path = opt
@@ -79,18 +76,19 @@ fn with_database<DB: IDatabase>(opt: Opt, db: Option<DB>) {
         let lib_exec = BeetsLibrarySshExecutor::new(uri)
             .expect("failed to initialise beets")
             .config(beets_config_file_path);
-        with(Some(BeetsLibrary::new(lib_exec)), db);
+        with(builder.set_library(BeetsLibrary::new(lib_exec)));
     } else {
         let lib_exec = BeetsLibraryProcessExecutor::default().config(opt.beets_config_file_path);
-        with(Some(BeetsLibrary::new(lib_exec)), db);
+        with(builder.set_library(BeetsLibrary::new(lib_exec)));
     }
 }
 
 fn main() {
     let opt = Opt::from_args();
+    let builder = MusicHoardBuilder::default();
 
     if opt.no_database {
-        with_database(opt, None::<NoDatabase>);
+        with_database(opt, builder.set_database(NullDatabase));
     } else {
         // Create an empty database file if it does not exist.
         match OpenOptions::new()
@@ -111,7 +109,7 @@ fn main() {
         }
 
         let db_exec = JsonDatabaseFileBackend::new(&opt.database_file_path);
-        with_database(opt, Some(JsonDatabase::new(db_exec)));
+        with_database(opt, builder.set_database(JsonDatabase::new(db_exec)));
     };
 }
 

tests/lib.rs

@@ -30,7 +30,7 @@ fn merge_library_then_database() {
     let backend = JsonDatabaseFileBackend::new(&*database::json::DATABASE_TEST_FILE);
     let database = JsonDatabase::new(backend);
 
-    let mut music_hoard = MusicHoard::new(Some(library), Some(database));
+    let mut music_hoard = MusicHoard::new(library, database);
 
     music_hoard.rescan_library().unwrap();
     music_hoard.load_from_database().unwrap();
@@ -55,7 +55,7 @@ fn merge_database_then_library() {
     let backend = JsonDatabaseFileBackend::new(&*database::json::DATABASE_TEST_FILE);
     let database = JsonDatabase::new(backend);
 
-    let mut music_hoard = MusicHoard::new(Some(library), Some(database));
+    let mut music_hoard = MusicHoard::new(library, database);
 
     music_hoard.load_from_database().unwrap();
     music_hoard.rescan_library().unwrap();