From 1013fb3e7bf59b9b003c6c63e5a09aaae102c032 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Thu, 11 Jan 2024 23:28:25 +0100 Subject: [PATCH 1/2] Replace generics with traits --- src/database/json/mod.rs | 9 +++--- src/database/mod.rs | 18 ++++++------ src/lib.rs | 60 +++++++++++++++++++--------------------- src/main.rs | 16 +++++------ src/tui/lib.rs | 4 +-- tests/lib.rs | 4 +-- 6 files changed, 55 insertions(+), 56 deletions(-) diff --git a/src/database/json/mod.rs b/src/database/json/mod.rs index 6013fbb..aa910e5 100644 --- a/src/database/json/mod.rs +++ b/src/database/json/mod.rs @@ -1,11 +1,10 @@ //! Module for storing MusicHoard data in a JSON file database. -use serde::de::DeserializeOwned; -use serde::Serialize; - #[cfg(test)] use mockall::automock; +use crate::Collection; + use super::{IDatabase, LoadError, SaveError}; pub mod backend; @@ -46,13 +45,13 @@ impl JsonDatabase { } impl IDatabase for JsonDatabase { - fn load(&self, collection: &mut D) -> Result<(), LoadError> { + fn load(&self, collection: &mut Collection) -> Result<(), LoadError> { let serialized = self.backend.read()?; *collection = serde_json::from_str(&serialized)?; Ok(()) } - fn save(&mut self, collection: &S) -> Result<(), SaveError> { + fn save(&mut self, collection: &Collection) -> Result<(), SaveError> { let serialized = serde_json::to_string(&collection)?; self.backend.write(&serialized)?; Ok(()) diff --git a/src/database/mod.rs b/src/database/mod.rs index 6c0c0b4..0c19ac4 100644 --- a/src/database/mod.rs +++ b/src/database/mod.rs @@ -2,11 +2,11 @@ use std::fmt; -use serde::{de::DeserializeOwned, Serialize}; - #[cfg(test)] use mockall::automock; +use crate::Collection; + #[cfg(feature = "database-json")] pub mod json; @@ -14,21 +14,21 @@ pub mod json; #[cfg_attr(test, automock)] pub trait IDatabase { /// Load collection from the database. - fn load(&self, collection: &mut D) -> Result<(), LoadError>; + fn load(&self, collection: &mut Collection) -> Result<(), LoadError>; /// Save collection to the database. - fn save(&mut self, collection: &S) -> Result<(), SaveError>; + fn save(&mut self, collection: &Collection) -> Result<(), SaveError>; } /// Non-implementation defined to make generics easier. pub struct NoDatabase {} impl IDatabase for NoDatabase { - fn load(&self, _collection: &mut D) -> Result<(), LoadError> { + fn load(&self, _collection: &mut Collection) -> Result<(), LoadError> { panic!() } - fn save(&mut self, _collection: &S) -> Result<(), SaveError> { + fn save(&mut self, _collection: &Collection) -> Result<(), SaveError> { panic!() } } @@ -89,13 +89,15 @@ impl From for SaveError { mod tests { use std::io; + use crate::Artist; + use super::{IDatabase, LoadError, NoDatabase, SaveError}; #[test] #[should_panic] fn no_database_load() { let database = NoDatabase {}; - let mut collection: Vec = vec![]; + let mut collection: Vec = vec![]; _ = database.load(&mut collection); } @@ -103,7 +105,7 @@ mod tests { #[should_panic] fn no_database_save() { let mut database = NoDatabase {}; - let collection: Vec = vec![]; + let collection: Vec = vec![]; _ = database.save(&collection); } diff --git a/src/lib.rs b/src/lib.rs index 47c152c..f73cb70 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -692,9 +692,9 @@ 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, +pub struct MusicHoard { + library: Option>, + database: Option>, collection: Collection, } @@ -773,9 +773,9 @@ 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: Option>, database: Option>) -> Self { MusicHoard { library, database, @@ -1063,7 +1063,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 = MusicHoard::new(None, None); let mut expected: Vec = vec![]; @@ -1085,7 +1085,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 = MusicHoard::new(None, None); let actual_err = music_hoard .add_musicbrainz_url(&artist_id, QOBUZ) @@ -1100,7 +1100,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 = MusicHoard::new(None, None); music_hoard.add_artist(artist_id.clone()); @@ -1171,7 +1171,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 = MusicHoard::new(None, None); music_hoard.add_artist(artist_id.clone()); @@ -1226,7 +1226,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 = MusicHoard::new(None, None); music_hoard.add_artist(artist_id.clone()); @@ -1356,7 +1356,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 = MusicHoard::new(None, None); music_hoard.add_artist(artist_id.clone()); @@ -1419,7 +1419,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 = MusicHoard::new(None, None); music_hoard.add_artist(artist_id.clone()); @@ -1549,7 +1549,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 = MusicHoard::new(None, None); music_hoard.add_artist(artist_id.clone()); @@ -1612,7 +1612,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 = MusicHoard::new(None, None); music_hoard.add_artist(artist_id.clone()); @@ -1663,7 +1663,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 = MusicHoard::new(None, None); music_hoard.add_artist(artist_id.clone()); @@ -1802,7 +1802,7 @@ mod tests { let mut expected = COLLECTION.to_owned(); expected.sort_unstable(); - let merged = MusicHoard::::merge(left.clone(), right); + let merged = MusicHoard::merge(left.clone(), right); assert_eq!(expected, merged); } @@ -1816,7 +1816,7 @@ mod tests { let mut expected = COLLECTION.to_owned(); expected.sort_unstable(); - let merged = MusicHoard::::merge(left.clone(), right); + let merged = MusicHoard::merge(left.clone(), right); assert_eq!(expected, merged); } @@ -1834,7 +1834,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(Some(Box::new(library)), Some(Box::new(database))); music_hoard.rescan_library().unwrap(); assert_eq!( @@ -1861,7 +1861,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(Some(Box::new(library)), Some(Box::new(database))); music_hoard.rescan_library().unwrap(); assert_eq!( @@ -1888,7 +1888,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(Some(Box::new(library)), Some(Box::new(database))); music_hoard.rescan_library().unwrap(); assert_eq!(music_hoard.get_collection(), &expected); @@ -1907,7 +1907,7 @@ mod tests { Ok(()) }); - let mut music_hoard = MusicHoard::new(Some(library), Some(database)); + let mut music_hoard = MusicHoard::new(Some(Box::new(library)), Some(Box::new(database))); music_hoard.load_from_database().unwrap(); assert_eq!(music_hoard.get_collection(), &*COLLECTION); @@ -1936,7 +1936,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(Some(Box::new(library)), Some(Box::new(database))); music_hoard.rescan_library().unwrap(); assert_eq!( @@ -1948,9 +1948,8 @@ mod tests { #[test] fn library_not_provided() { - let library: Option = None; - let database = Some(MockIDatabase::new()); - let mut music_hoard = MusicHoard::new(library, database); + let database = MockIDatabase::new(); + let mut music_hoard = MusicHoard::new(None, Some(Box::new(database))); assert!(music_hoard.rescan_library().is_ok()); } @@ -1967,7 +1966,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(Some(Box::new(library)), Some(Box::new(database))); let actual_err = music_hoard.rescan_library().unwrap_err(); let expected_err = @@ -1979,9 +1978,8 @@ mod tests { #[test] fn database_not_provided() { - let library = Some(MockILibrary::new()); - let database: Option = None; - let mut music_hoard = MusicHoard::new(library, database); + let library = MockILibrary::new(); + let mut music_hoard = MusicHoard::new(Some(Box::new(library)), None); assert!(music_hoard.load_from_database().is_ok()); assert!(music_hoard.save_to_database().is_ok()); @@ -1999,7 +1997,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(Some(Box::new(library)), Some(Box::new(database))); let actual_err = music_hoard.load_from_database().unwrap_err(); let expected_err = Error::DatabaseError( @@ -2022,7 +2020,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(Some(Box::new(library)), Some(Box::new(database))); let actual_err = music_hoard.save_to_database().unwrap_err(); let expected_err = Error::DatabaseError( diff --git a/src/main.rs b/src/main.rs index 48cd409..ec90759 100644 --- a/src/main.rs +++ b/src/main.rs @@ -49,7 +49,7 @@ struct Opt { no_database: bool, } -fn with(lib: Option, db: Option) { +fn with(lib: Option>, db: Option>) { let music_hoard = MusicHoard::new(lib, db); // Initialize the terminal user interface. @@ -66,9 +66,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, db: Option>) { if opt.no_library { - with(None::, db); + with(None, db); } 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,10 +79,10 @@ 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(Some(Box::new(BeetsLibrary::new(lib_exec))), db); } else { let lib_exec = BeetsLibraryProcessExecutor::default().config(opt.beets_config_file_path); - with(Some(BeetsLibrary::new(lib_exec)), db); + with(Some(Box::new(BeetsLibrary::new(lib_exec))), db); } } @@ -90,7 +90,7 @@ fn main() { let opt = Opt::from_args(); if opt.no_database { - with_database(opt, None::); + with_database(opt, None); } else { // Create an empty database file if it does not exist. match OpenOptions::new() @@ -101,7 +101,7 @@ fn main() { Ok(f) => { drop(f); JsonDatabase::new(JsonDatabaseFileBackend::new(&opt.database_file_path)) - .save::(&vec![]) + .save(&vec![]) .expect("failed to create empty database"); } Err(e) => match e.kind() { @@ -111,7 +111,7 @@ fn main() { } let db_exec = JsonDatabaseFileBackend::new(&opt.database_file_path); - with_database(opt, Some(JsonDatabase::new(db_exec))); + with_database(opt, Some(Box::new(JsonDatabase::new(db_exec)))); }; } diff --git a/src/tui/lib.rs b/src/tui/lib.rs index 6e0facd..b120bb4 100644 --- a/src/tui/lib.rs +++ b/src/tui/lib.rs @@ -1,4 +1,4 @@ -use musichoard::{database::IDatabase, library::ILibrary, Collection, MusicHoard}; +use musichoard::{Collection, MusicHoard}; #[cfg(test)] use mockall::automock; @@ -12,7 +12,7 @@ pub trait IMusicHoard { } // GRCOV_EXCL_START -impl IMusicHoard for MusicHoard { +impl IMusicHoard for MusicHoard { fn rescan_library(&mut self) -> Result<(), musichoard::Error> { MusicHoard::rescan_library(self) } diff --git a/tests/lib.rs b/tests/lib.rs index f945f38..cbca84b 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(Some(Box::new(library)), Some(Box::new(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(Some(Box::new(library)), Some(Box::new(database))); music_hoard.load_from_database().unwrap(); music_hoard.rescan_library().unwrap(); -- 2.45.2 From 73d01446962be66769c01fb143c248bce08e738a Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Thu, 11 Jan 2024 23:29:03 +0100 Subject: [PATCH 2/2] Tidy up main --- src/main.rs | 49 +++++++++++++++++++++++-------------------------- 1 file changed, 23 insertions(+), 26 deletions(-) diff --git a/src/main.rs b/src/main.rs index ec90759..28793b8 100644 --- a/src/main.rs +++ b/src/main.rs @@ -2,9 +2,6 @@ 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 ratatui::{backend::CrosstermBackend, Terminal}; use structopt::StructOpt; @@ -66,31 +63,11 @@ fn with(lib: Option>, db: Option>) { Tui::run(terminal, ui, handler, listener).expect("failed to run tui"); } -fn with_database(opt: Opt, db: Option>) { - if opt.no_library { - with(None, db); - } else if let Some(uri) = opt.beets_ssh_uri { - let uri = uri.into_string().expect("invalid SSH URI"); - let beets_config_file_path = opt - .beets_config_file_path - .map(|s| s.into_string()) - .transpose() - .expect("failed to extract beets config file path"); - let lib_exec = BeetsLibrarySshExecutor::new(uri) - .expect("failed to initialise beets") - .config(beets_config_file_path); - with(Some(Box::new(BeetsLibrary::new(lib_exec))), db); - } else { - let lib_exec = BeetsLibraryProcessExecutor::default().config(opt.beets_config_file_path); - with(Some(Box::new(BeetsLibrary::new(lib_exec))), db); - } -} - fn main() { let opt = Opt::from_args(); - if opt.no_database { - with_database(opt, None); + let database: Option> = if opt.no_database { + None } else { // Create an empty database file if it does not exist. match OpenOptions::new() @@ -111,8 +88,28 @@ fn main() { } let db_exec = JsonDatabaseFileBackend::new(&opt.database_file_path); - with_database(opt, Some(Box::new(JsonDatabase::new(db_exec)))); + Some(Box::new(JsonDatabase::new(db_exec))) }; + + let library: Option> = if opt.no_library { + None + } else if let Some(uri) = opt.beets_ssh_uri { + let uri = uri.into_string().expect("invalid SSH URI"); + let beets_config_file_path = opt + .beets_config_file_path + .map(|s| s.into_string()) + .transpose() + .expect("failed to extract beets config file path"); + let lib_exec = BeetsLibrarySshExecutor::new(uri) + .expect("failed to initialise beets") + .config(beets_config_file_path); + Some(Box::new(BeetsLibrary::new(lib_exec))) + } else { + let lib_exec = BeetsLibraryProcessExecutor::default().config(opt.beets_config_file_path); + Some(Box::new(BeetsLibrary::new(lib_exec))) + }; + + with(library, database); } #[cfg(test)] -- 2.45.2