diff --git a/src/bin/musichoard-edit.rs b/src/bin/musichoard-edit.rs index 9173ff7..4a38d5d 100644 --- a/src/bin/musichoard-edit.rs +++ b/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>; @@ -157,12 +156,9 @@ impl ArtistCommand { fn main() { let opt = Opt::from_args(); - let lib: Option = 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::new().set_database(db).build(); music_hoard .load_from_database() .expect("failed to load database"); diff --git a/src/database/mod.rs b/src/database/mod.rs index 6c0c0b4..79c63df 100644 --- a/src/database/mod.rs +++ b/src/database/mod.rs @@ -20,16 +20,16 @@ pub trait IDatabase { fn save(&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(&self, _collection: &mut D) -> Result<(), LoadError> { - panic!() + Ok(()) } fn save(&mut self, _collection: &S) -> Result<(), SaveError> { - panic!() + Ok(()) } } @@ -89,22 +89,21 @@ impl From 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 = 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 = vec![]; - _ = database.save(&collection); + assert!(database.save(&collection).is_ok()); } #[test] diff --git a/src/lib.rs b/src/lib.rs index 47c152c..760bc34 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -693,11 +693,15 @@ impl From 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 { - library: Option, - database: Option, 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 MusicHoard { +impl MusicHoard { /// Create a new [`MusicHoard`] with the provided [`ILibrary`] and [`IDatabase`]. - pub fn new(library: Option, database: Option) -> Self { + pub fn new(library: LIB, database: DB) -> Self { MusicHoard { + collection: vec![], library, database, - collection: vec![], } } @@ -787,37 +791,6 @@ impl MusicHoard { &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>(&mut self, artist_id: ID) { let artist_id: ArtistId = artist_id.into(); @@ -940,6 +913,84 @@ impl MusicHoard { } } +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); + + let collection = mem::take(&mut self.collection); + self.collection = Self::merge(library_collection, collection); + + Ok(()) + } +} + +impl MusicHoard { + 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 { + library: LIB, + database: DB, +} + +impl Default for MusicHoardBuilder { + /// Create a [`MusicHoardBuilder`]. + fn default() -> Self { + Self::new() + } +} + +impl MusicHoardBuilder { + /// Create a [`MusicHoardBuilder`]. + pub fn new() -> Self { + MusicHoardBuilder { + library: NoLibrary, + database: NoDatabase, + } + } +} + +impl MusicHoardBuilder { + /// Set a library for [`MusicHoard`]. + pub fn set_library(self, library: NEWLIB) -> MusicHoardBuilder { + MusicHoardBuilder { + library, + database: self.database, + } + } + + /// Set a database for [`MusicHoard`]. + pub fn set_database(self, database: NEWDB) -> MusicHoardBuilder { + MusicHoardBuilder { + library: self.library, + database, + } + } + + /// Build [`MusicHoard`] with the currently set library and database. + pub fn build(self) -> MusicHoard { + 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::::new(None, None); + let mut music_hoard = MusicHoardBuilder::new().build(); let mut expected: Vec = vec![]; @@ -1085,7 +1133,7 @@ mod tests { #[test] fn collection_error() { let artist_id = ArtistId::new("an artist"); - let mut music_hoard = MusicHoard::::new(None, None); + let mut music_hoard = MusicHoardBuilder::new().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::::new(None, None); + let mut music_hoard = MusicHoardBuilder::new().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::::new(None, None); + let mut music_hoard = MusicHoardBuilder::new().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::::new(None, None); + let mut music_hoard = MusicHoardBuilder::new().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::::new(None, None); + let mut music_hoard = MusicHoardBuilder::new().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::::new(None, None); + let mut music_hoard = MusicHoardBuilder::new().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::::new(None, None); + let mut music_hoard = MusicHoardBuilder::new().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::::new(None, None); + let mut music_hoard = MusicHoardBuilder::new().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::::new(None, None); + let mut music_hoard = MusicHoardBuilder::new().build(); music_hoard.add_artist(artist_id.clone()); @@ -1834,7 +1882,7 @@ mod tests { .times(1) .return_once(|_| library_result); - let mut music_hoard = MusicHoard::new(Some(library), Some(database)); + let mut music_hoard = MusicHoard::new(library, database); music_hoard.rescan_library().unwrap(); assert_eq!( @@ -1861,7 +1909,7 @@ mod tests { .times(1) .return_once(|_| library_result); - let mut music_hoard = MusicHoard::new(Some(library), Some(database)); + let mut music_hoard = MusicHoard::new(library, database); music_hoard.rescan_library().unwrap(); assert_eq!( @@ -1888,7 +1936,7 @@ mod tests { .times(1) .return_once(|_| library_result); - let mut music_hoard = MusicHoard::new(Some(library), Some(database)); + let mut music_hoard = MusicHoard::new(library, database); music_hoard.rescan_library().unwrap(); assert_eq!(music_hoard.get_collection(), &expected); @@ -1907,7 +1955,7 @@ mod tests { Ok(()) }); - let mut music_hoard = MusicHoard::new(Some(library), Some(database)); + let mut music_hoard = MusicHoard::new(library, database); music_hoard.load_from_database().unwrap(); assert_eq!(music_hoard.get_collection(), &*COLLECTION); @@ -1936,7 +1984,7 @@ mod tests { .times(1) .return_once(|_: &Collection| database_result); - let mut music_hoard = MusicHoard::new(Some(library), Some(database)); + let mut music_hoard = MusicHoard::new(library, database); music_hoard.rescan_library().unwrap(); assert_eq!( @@ -1946,15 +1994,6 @@ mod tests { music_hoard.save_to_database().unwrap(); } - #[test] - fn library_not_provided() { - let library: Option = 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 +2006,7 @@ mod tests { .times(1) .return_once(|_| library_result); - let mut music_hoard = MusicHoard::new(Some(library), Some(database)); + let mut music_hoard = MusicHoard::new(library, database); let actual_err = music_hoard.rescan_library().unwrap_err(); let expected_err = @@ -1977,16 +2016,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 = 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 +2028,7 @@ mod tests { .times(1) .return_once(|_: &mut Collection| database_result); - let mut music_hoard = MusicHoard::new(Some(library), Some(database)); + let mut music_hoard = MusicHoard::new(library, database); let actual_err = music_hoard.load_from_database().unwrap_err(); let expected_err = Error::DatabaseError( @@ -2022,7 +2051,7 @@ mod tests { .times(1) .return_once(|_: &Collection| database_result); - let mut music_hoard = MusicHoard::new(Some(library), Some(database)); + let mut music_hoard = MusicHoard::new(library, database); let actual_err = music_hoard.save_to_database().unwrap_err(); let expected_err = Error::DatabaseError( diff --git a/src/library/mod.rs b/src/library/mod.rs index 5f43671..cea936e 100644 --- a/src/library/mod.rs +++ b/src/library/mod.rs @@ -17,12 +17,12 @@ pub trait ILibrary { fn list(&mut self, query: &Query) -> Result, 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, Error> { - panic!() + Ok(vec![]) } } @@ -136,13 +136,12 @@ impl From 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] diff --git a/src/main.rs b/src/main.rs index 48cd409..4d43c80 100644 --- a/src/main.rs +++ b/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: Option, db: Option) { - let music_hoard = MusicHoard::new(lib, db); +fn with(builder: MusicHoardBuilder) { + let music_hoard = builder.build(); // Initialize the terminal user interface. let backend = CrosstermBackend::new(io::stdout()); @@ -66,9 +63,9 @@ fn with(lib: Option, db: Option) { Tui::run(terminal, ui, handler, listener).expect("failed to run tui"); } -fn with_database(opt: Opt, db: Option) { +fn with_database(opt: Opt, builder: MusicHoardBuilder) { if opt.no_library { - with(None::, 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(opt: Opt, db: Option) { 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::new(); if opt.no_database { - with_database(opt, None::); + 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))); }; } diff --git a/tests/lib.rs b/tests/lib.rs index f945f38..c1e74bd 100644 --- a/tests/lib.rs +++ b/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();