From 68d5fa69d4f65323cb6374cf55e77655704693ed Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Mon, 10 Apr 2023 21:21:04 +0200 Subject: [PATCH 1/2] Rename types for consistency --- src/database/json.rs | 46 ++++++++++++------------ src/database/mod.rs | 3 ++ src/library/beets.rs | 80 ++++++++++++++++++++++++------------------ src/main.rs | 14 ++++---- tests/database/json.rs | 14 ++++---- tests/library/beets.rs | 14 ++++---- 6 files changed, 95 insertions(+), 76 deletions(-) diff --git a/src/database/json.rs b/src/database/json.rs index d6ad01b..f99381b 100644 --- a/src/database/json.rs +++ b/src/database/json.rs @@ -6,10 +6,10 @@ use std::path::{Path, PathBuf}; use serde::de::DeserializeOwned; use serde::Serialize; -use super::{DatabaseRead, DatabaseWrite}; +use super::{Database, DatabaseRead, DatabaseWrite}; /// Trait for the JSON database backend. -pub trait DatabaseJsonBackend { +pub trait JsonDatabaseBackend { /// Read the JSON string from the backend. fn read(&self) -> Result; @@ -18,18 +18,18 @@ pub trait DatabaseJsonBackend { } /// JSON database. -pub struct DatabaseJson { - backend: Box, +pub struct JsonDatabase { + backend: Box, } -impl DatabaseJson { - /// Create a new JSON database with the provided executor, e.g. [DatabaseJsonFile]. - pub fn new(backend: Box) -> Self { - DatabaseJson { backend } +impl JsonDatabase { + /// Create a new JSON database with the provided backend, e.g. [JsonDatabaseFileBackend]. + pub fn new(backend: Box) -> Self { + JsonDatabase { backend } } } -impl DatabaseRead for DatabaseJson { +impl DatabaseRead for JsonDatabase { fn read(&self, collection: &mut D) -> Result<(), std::io::Error> where D: DeserializeOwned, @@ -40,7 +40,7 @@ impl DatabaseRead for DatabaseJson { } } -impl DatabaseWrite for DatabaseJson { +impl DatabaseWrite for JsonDatabase { fn write(&mut self, collection: &S) -> Result<(), std::io::Error> where S: Serialize, @@ -51,21 +51,23 @@ impl DatabaseWrite for DatabaseJson { } } +impl Database for JsonDatabase {} + /// JSON database that uses a local file for persistent storage. -pub struct DatabaseJsonFile { +pub struct JsonDatabaseFileBackend { path: PathBuf, } -impl DatabaseJsonFile { +impl JsonDatabaseFileBackend { /// Create a database instance that will read/write to the provided path. pub fn new(path: &Path) -> Self { - DatabaseJsonFile { + JsonDatabaseFileBackend { path: path.to_path_buf(), } } } -impl DatabaseJsonBackend for DatabaseJsonFile { +impl JsonDatabaseBackend for JsonDatabaseFileBackend { fn read(&self) -> Result { // Read entire file to memory as for now this is faster than a buffered read from disk: // https://github.com/serde-rs/json/issues/160 @@ -83,11 +85,11 @@ mod tests { use crate::{tests::COLLECTION, Artist, TrackFormat}; - struct DatabaseJsonTest { + struct JsonDatabaseTestBackend { json: String, } - impl DatabaseJsonBackend for DatabaseJsonTest { + impl JsonDatabaseBackend for JsonDatabaseTestBackend { fn read(&self) -> Result { Ok(self.json.clone()) } @@ -165,11 +167,11 @@ mod tests { #[test] fn write() { let write_data = COLLECTION.to_owned(); - let backend = DatabaseJsonTest { + let backend = JsonDatabaseTestBackend { json: artists_to_json(&write_data), }; - DatabaseJson::new(Box::new(backend)) + JsonDatabase::new(Box::new(backend)) .write(&write_data) .unwrap(); } @@ -177,12 +179,12 @@ mod tests { #[test] fn read() { let expected = COLLECTION.to_owned(); - let backend = DatabaseJsonTest { + let backend = JsonDatabaseTestBackend { json: artists_to_json(&expected), }; let mut read_data: Vec = vec![]; - DatabaseJson::new(Box::new(backend)) + JsonDatabase::new(Box::new(backend)) .read(&mut read_data) .unwrap(); @@ -192,10 +194,10 @@ mod tests { #[test] fn reverse() { let expected = COLLECTION.to_owned(); - let backend = DatabaseJsonTest { + let backend = JsonDatabaseTestBackend { json: artists_to_json(&expected), }; - let mut database = DatabaseJson::new(Box::new(backend)); + let mut database = JsonDatabase::new(Box::new(backend)); let write_data = COLLECTION.to_owned(); let mut read_data: Vec = vec![]; diff --git a/src/database/mod.rs b/src/database/mod.rs index 22a4067..dea333d 100644 --- a/src/database/mod.rs +++ b/src/database/mod.rs @@ -20,3 +20,6 @@ pub trait DatabaseWrite { where S: Serialize; } + +/// Trait for database reads and writes. +pub trait Database: DatabaseRead + DatabaseWrite {} diff --git a/src/library/beets.rs b/src/library/beets.rs index 065147a..8a6dc2c 100644 --- a/src/library/beets.rs +++ b/src/library/beets.rs @@ -84,14 +84,14 @@ impl QueryArgsBeets for Query { } /// Trait for invoking beets commands. -pub trait BeetsExecutor { +pub trait BeetsLibraryExecutor { /// Invoke beets with the provided arguments. fn exec(&mut self, arguments: &[String]) -> Result, Error>; } /// Struct for interacting with the music library via beets. -pub struct Beets { - executor: Box, +pub struct BeetsLibrary { + executor: Box, } trait LibraryPrivate { @@ -105,14 +105,14 @@ trait LibraryPrivate { fn list_to_artists(list_output: Vec) -> Result, Error>; } -impl Beets { - /// Create a new beets library instance with the provided executor, e.g. [SystemExecutor]. - pub fn new(executor: Box) -> Beets { - Beets { executor } +impl BeetsLibrary { + /// Create a new beets library with the provided executor, e.g. [BeetsLibrarySystemExecutor]. + pub fn new(executor: Box) -> BeetsLibrary { + BeetsLibrary { executor } } } -impl Library for Beets { +impl Library for BeetsLibrary { fn list(&mut self, query: &Query) -> Result, Error> { let cmd = Self::list_cmd_and_args(query); let output = self.executor.exec(&cmd)?; @@ -126,7 +126,7 @@ macro_rules! list_format_separator { }; } -impl LibraryPrivate for Beets { +impl LibraryPrivate for BeetsLibrary { const CMD_LIST: &'static str = "ls"; const LIST_FORMAT_SEPARATOR: &'static str = list_format_separator!(); const LIST_FORMAT_ARG: &'static str = concat!( @@ -244,19 +244,19 @@ impl LibraryPrivate for Beets { /// database/library. Therefore, all functions that create a [SystemExecutor] or modify which /// library it works with are marked unsafe. It is the caller's responsibility to make sure the /// library is not being concurrently accessed from anywhere else. -pub struct SystemExecutor { +pub struct BeetsLibrarySystemExecutor { bin: String, config: Option, } -impl SystemExecutor { +impl BeetsLibrarySystemExecutor { /// Create a new [SystemExecutor] that uses the provided beets executable. /// /// # Safety /// /// The caller must ensure the library is not being concurrently accessed from anywhere else. pub unsafe fn new(bin: &str) -> Self { - SystemExecutor { + BeetsLibrarySystemExecutor { bin: bin.to_string(), config: None, } @@ -268,7 +268,7 @@ impl SystemExecutor { /// /// The caller must ensure the library is not being concurrently accessed from anywhere else. pub unsafe fn default() -> Self { - SystemExecutor::new("beet") + BeetsLibrarySystemExecutor::new("beet") } /// Update the configuration file for the beets executable. @@ -282,7 +282,7 @@ impl SystemExecutor { } } -impl BeetsExecutor for SystemExecutor { +impl BeetsLibraryExecutor for BeetsLibrarySystemExecutor { fn exec(&mut self, arguments: &[String]) -> Result, Error> { let mut cmd = Command::new(&self.bin); if let Some(ref path) = self.config { @@ -306,12 +306,12 @@ mod tests { use crate::tests::COLLECTION; - struct TestExecutor { + struct BeetsLibraryTestExecutor { arguments: Option>, output: Option, Error>>, } - impl BeetsExecutor for TestExecutor { + impl BeetsLibraryExecutor for BeetsLibraryTestExecutor { fn exec(&mut self, arguments: &[String]) -> Result, Error> { assert_eq!(&self.arguments.take().unwrap(), arguments); self.output.take().unwrap() @@ -332,14 +332,14 @@ mod tests { let track_title = &track.title; let track_artist = &track.artist.join("; "); let track_format = match track.format { - TrackFormat::Flac => Beets::TRACK_FORMAT_FLAC, - TrackFormat::Mp3 => Beets::TRACK_FORMAT_MP3, + TrackFormat::Flac => BeetsLibrary::TRACK_FORMAT_FLAC, + TrackFormat::Mp3 => BeetsLibrary::TRACK_FORMAT_MP3, }; strings.push(format!( "{album_artist}{0}{album_year}{0}{album_title}{0}\ {track_number}{0}{track_title}{0}{track_artist}{0}{track_format}", - Beets::LIST_FORMAT_SEPARATOR, + BeetsLibrary::LIST_FORMAT_SEPARATOR, )); } } @@ -379,11 +379,14 @@ mod tests { #[test] fn test_list_empty() { - let executor = TestExecutor { - arguments: Some(vec!["ls".to_string(), Beets::LIST_FORMAT_ARG.to_string()]), + let executor = BeetsLibraryTestExecutor { + arguments: Some(vec![ + "ls".to_string(), + BeetsLibrary::LIST_FORMAT_ARG.to_string(), + ]), output: Some(Ok(vec![])), }; - let mut beets = Beets::new(Box::new(executor)); + let mut beets = BeetsLibrary::new(Box::new(executor)); let output = beets.list(&Query::default()).unwrap(); let expected: Vec = vec![]; @@ -395,11 +398,14 @@ mod tests { let expected = COLLECTION.to_owned(); let output = artists_to_beets_string(&expected); - let executor = TestExecutor { - arguments: Some(vec!["ls".to_string(), Beets::LIST_FORMAT_ARG.to_string()]), + let executor = BeetsLibraryTestExecutor { + arguments: Some(vec![ + "ls".to_string(), + BeetsLibrary::LIST_FORMAT_ARG.to_string(), + ]), output: Some(Ok(output)), }; - let mut beets = Beets::new(Box::new(executor)); + let mut beets = BeetsLibrary::new(Box::new(executor)); let output = beets.list(&Query::default()).unwrap(); assert_eq!(output, expected); @@ -424,11 +430,14 @@ mod tests { // And the (now) second album's tracks first track comes last. expected[1].albums[0].tracks.rotate_left(1); - let executor = TestExecutor { - arguments: Some(vec!["ls".to_string(), Beets::LIST_FORMAT_ARG.to_string()]), + let executor = BeetsLibraryTestExecutor { + arguments: Some(vec![ + "ls".to_string(), + BeetsLibrary::LIST_FORMAT_ARG.to_string(), + ]), output: Some(Ok(output)), }; - let mut beets = Beets::new(Box::new(executor)); + let mut beets = BeetsLibrary::new(Box::new(executor)); let output = beets.list(&Query::default()).unwrap(); assert_eq!(output, expected); @@ -442,11 +451,14 @@ mod tests { let output = artists_to_beets_string(&expected); - let executor = TestExecutor { - arguments: Some(vec!["ls".to_string(), Beets::LIST_FORMAT_ARG.to_string()]), + let executor = BeetsLibraryTestExecutor { + arguments: Some(vec![ + "ls".to_string(), + BeetsLibrary::LIST_FORMAT_ARG.to_string(), + ]), output: Some(Ok(output)), }; - let mut beets = Beets::new(Box::new(executor)); + let mut beets = BeetsLibrary::new(Box::new(executor)); let output = beets.list(&Query::default()).unwrap(); assert_eq!(output, expected); @@ -459,17 +471,17 @@ mod tests { .track_number(QueryOption::Include(5)) .track_artist(QueryOption::Include(vec![String::from("some.artist")])); - let executor = TestExecutor { + let executor = BeetsLibraryTestExecutor { arguments: Some(vec![ "ls".to_string(), - Beets::LIST_FORMAT_ARG.to_string(), + BeetsLibrary::LIST_FORMAT_ARG.to_string(), String::from("^album:some.album"), String::from("track:5"), String::from("artist:some.artist"), ]), output: Some(Ok(vec![])), }; - let mut beets = Beets::new(Box::new(executor)); + let mut beets = BeetsLibrary::new(Box::new(executor)); let output = beets.list(&query).unwrap(); let expected: Vec = vec![]; diff --git a/src/main.rs b/src/main.rs index ee8abe5..c01e554 100644 --- a/src/main.rs +++ b/src/main.rs @@ -4,11 +4,11 @@ use structopt::StructOpt; use musichoard::{ database::{ - json::{DatabaseJson, DatabaseJsonFile}, + json::{JsonDatabase, JsonDatabaseFileBackend}, DatabaseWrite, }, library::{ - beets::{Beets, SystemExecutor}, + beets::{BeetsLibrary, BeetsLibrarySystemExecutor}, Library, Query, }, }; @@ -36,15 +36,17 @@ struct Opt { fn main() { let opt = Opt::from_args(); - let mut beets = Beets::new(Box::new( - unsafe { SystemExecutor::default().config(opt.beets_config_file_path.as_deref()) }, - )); + let mut beets = BeetsLibrary::new(Box::new(unsafe { + BeetsLibrarySystemExecutor::default().config(opt.beets_config_file_path.as_deref()) + })); let collection = beets .list(&Query::new()) .expect("failed to query the library"); - let mut database = DatabaseJson::new(Box::new(DatabaseJsonFile::new(&opt.database_file_path))); + let mut database = JsonDatabase::new(Box::new(JsonDatabaseFileBackend::new( + &opt.database_file_path, + ))); database .write(&collection) diff --git a/tests/database/json.rs b/tests/database/json.rs index d747841..a31996d 100644 --- a/tests/database/json.rs +++ b/tests/database/json.rs @@ -2,7 +2,7 @@ use std::{fs, path::PathBuf}; use musichoard::{ database::{ - json::{DatabaseJson, DatabaseJsonFile}, + json::{JsonDatabase, JsonDatabaseFileBackend}, DatabaseRead, DatabaseWrite, }, Artist, @@ -19,8 +19,8 @@ static DATABASE_TEST_FILE: Lazy = fn write() { let file = NamedTempFile::new().unwrap(); - let backend = DatabaseJsonFile::new(file.path()); - let mut database = DatabaseJson::new(Box::new(backend)); + let backend = JsonDatabaseFileBackend::new(file.path()); + let mut database = JsonDatabase::new(Box::new(backend)); let write_data = COLLECTION.to_owned(); database.write(&write_data).unwrap(); @@ -33,8 +33,8 @@ fn write() { #[test] fn read() { - let backend = DatabaseJsonFile::new(&*DATABASE_TEST_FILE); - let database = DatabaseJson::new(Box::new(backend)); + let backend = JsonDatabaseFileBackend::new(&*DATABASE_TEST_FILE); + let database = JsonDatabase::new(Box::new(backend)); let mut read_data: Vec = vec![]; database.read(&mut read_data).unwrap(); @@ -47,8 +47,8 @@ fn read() { fn reverse() { let file = NamedTempFile::new().unwrap(); - let backend = DatabaseJsonFile::new(file.path()); - let mut database = DatabaseJson::new(Box::new(backend)); + let backend = JsonDatabaseFileBackend::new(file.path()); + let mut database = JsonDatabase::new(Box::new(backend)); let write_data = COLLECTION.to_owned(); database.write(&write_data).unwrap(); diff --git a/tests/library/beets.rs b/tests/library/beets.rs index a290500..f5539c4 100644 --- a/tests/library/beets.rs +++ b/tests/library/beets.rs @@ -7,7 +7,7 @@ use once_cell::sync::Lazy; use musichoard::{ library::{ - beets::{Beets, SystemExecutor}, + beets::{BeetsLibrary, BeetsLibrarySystemExecutor}, Library, Query, QueryOption, }, Artist, @@ -15,15 +15,15 @@ use musichoard::{ use crate::COLLECTION; -static BEETS_EMPTY_CONFIG: Lazy>> = Lazy::new(|| { - Arc::new(Mutex::new(Beets::new(Box::new(unsafe { - SystemExecutor::default() +static BEETS_EMPTY_CONFIG: Lazy>> = Lazy::new(|| { + Arc::new(Mutex::new(BeetsLibrary::new(Box::new(unsafe { + BeetsLibrarySystemExecutor::default() })))) }); -static BEETS_TEST_CONFIG: Lazy>> = Lazy::new(|| { - Arc::new(Mutex::new(Beets::new(Box::new(unsafe { - SystemExecutor::default().config(Some( +static BEETS_TEST_CONFIG: Lazy>> = Lazy::new(|| { + Arc::new(Mutex::new(BeetsLibrary::new(Box::new(unsafe { + BeetsLibrarySystemExecutor::default().config(Some( &fs::canonicalize("./tests/files/library/config.yml").unwrap(), )) })))) -- 2.45.2 From a0d38454ca5b5a17806539f115cdad3ba9e47405 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Mon, 10 Apr 2023 21:35:40 +0200 Subject: [PATCH 2/2] Rename SystemExecutor to CommandExecutor --- src/database/json.rs | 4 ++-- src/library/beets.rs | 28 ++++++++++++++-------------- src/main.rs | 4 ++-- tests/library/beets.rs | 6 +++--- 4 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/database/json.rs b/src/database/json.rs index f99381b..30496f4 100644 --- a/src/database/json.rs +++ b/src/database/json.rs @@ -53,13 +53,13 @@ impl DatabaseWrite for JsonDatabase { impl Database for JsonDatabase {} -/// JSON database that uses a local file for persistent storage. +/// JSON database backend that uses a local file for persistent storage. pub struct JsonDatabaseFileBackend { path: PathBuf, } impl JsonDatabaseFileBackend { - /// Create a database instance that will read/write to the provided path. + /// Create a [JsonDatabaseFileBackend] that will read/write to the provided path. pub fn new(path: &Path) -> Self { JsonDatabaseFileBackend { path: path.to_path_buf(), diff --git a/src/library/beets.rs b/src/library/beets.rs index 8a6dc2c..c6a3e7b 100644 --- a/src/library/beets.rs +++ b/src/library/beets.rs @@ -89,7 +89,7 @@ pub trait BeetsLibraryExecutor { fn exec(&mut self, arguments: &[String]) -> Result, Error>; } -/// Struct for interacting with the music library via beets. +/// Beets library. pub struct BeetsLibrary { executor: Box, } @@ -106,7 +106,7 @@ trait LibraryPrivate { } impl BeetsLibrary { - /// Create a new beets library with the provided executor, e.g. [BeetsLibrarySystemExecutor]. + /// Create a new beets library with the provided executor, e.g. [BeetsLibraryCommandExecutor]. pub fn new(executor: Box) -> BeetsLibrary { BeetsLibrary { executor } } @@ -236,42 +236,42 @@ impl LibraryPrivate for BeetsLibrary { } } -/// Executor for executing beets commands on the local system. +/// Beets library executor that executes beets commands in their own process. /// /// # Safety /// /// The beets executable is not safe to call concurrently for operations on the same -/// database/library. Therefore, all functions that create a [SystemExecutor] or modify which -/// library it works with are marked unsafe. It is the caller's responsibility to make sure the -/// library is not being concurrently accessed from anywhere else. -pub struct BeetsLibrarySystemExecutor { +/// database/library. Therefore, all functions that create a [BeetsLibraryCommandExecutor] or modify +/// which library it works with are marked unsafe. It is the caller's responsibility to make sure +/// the library is not being concurrently accessed from anywhere else. +pub struct BeetsLibraryCommandExecutor { bin: String, config: Option, } -impl BeetsLibrarySystemExecutor { - /// Create a new [SystemExecutor] that uses the provided beets executable. +impl BeetsLibraryCommandExecutor { + /// Create a new [BeetsLibraryCommandExecutor] that uses the provided beets executable. /// /// # Safety /// /// The caller must ensure the library is not being concurrently accessed from anywhere else. pub unsafe fn new(bin: &str) -> Self { - BeetsLibrarySystemExecutor { + BeetsLibraryCommandExecutor { bin: bin.to_string(), config: None, } } - /// Create a new [SystemExecutor] that uses the system's default beets executable. + /// Create a new [BeetsLibraryCommandExecutor] that uses the system's default beets executable. /// /// # Safety /// /// The caller must ensure the library is not being concurrently accessed from anywhere else. pub unsafe fn default() -> Self { - BeetsLibrarySystemExecutor::new("beet") + BeetsLibraryCommandExecutor::new("beet") } - /// Update the configuration file for the beets executable. + /// Update the configuration file passed to the beets executable. /// /// # Safety /// @@ -282,7 +282,7 @@ impl BeetsLibrarySystemExecutor { } } -impl BeetsLibraryExecutor for BeetsLibrarySystemExecutor { +impl BeetsLibraryExecutor for BeetsLibraryCommandExecutor { fn exec(&mut self, arguments: &[String]) -> Result, Error> { let mut cmd = Command::new(&self.bin); if let Some(ref path) = self.config { diff --git a/src/main.rs b/src/main.rs index c01e554..44c2f3c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -8,7 +8,7 @@ use musichoard::{ DatabaseWrite, }, library::{ - beets::{BeetsLibrary, BeetsLibrarySystemExecutor}, + beets::{BeetsLibrary, BeetsLibraryCommandExecutor}, Library, Query, }, }; @@ -37,7 +37,7 @@ fn main() { let opt = Opt::from_args(); let mut beets = BeetsLibrary::new(Box::new(unsafe { - BeetsLibrarySystemExecutor::default().config(opt.beets_config_file_path.as_deref()) + BeetsLibraryCommandExecutor::default().config(opt.beets_config_file_path.as_deref()) })); let collection = beets diff --git a/tests/library/beets.rs b/tests/library/beets.rs index f5539c4..4b8e9b8 100644 --- a/tests/library/beets.rs +++ b/tests/library/beets.rs @@ -7,7 +7,7 @@ use once_cell::sync::Lazy; use musichoard::{ library::{ - beets::{BeetsLibrary, BeetsLibrarySystemExecutor}, + beets::{BeetsLibrary, BeetsLibraryCommandExecutor}, Library, Query, QueryOption, }, Artist, @@ -17,13 +17,13 @@ use crate::COLLECTION; static BEETS_EMPTY_CONFIG: Lazy>> = Lazy::new(|| { Arc::new(Mutex::new(BeetsLibrary::new(Box::new(unsafe { - BeetsLibrarySystemExecutor::default() + BeetsLibraryCommandExecutor::default() })))) }); static BEETS_TEST_CONFIG: Lazy>> = Lazy::new(|| { Arc::new(Mutex::new(BeetsLibrary::new(Box::new(unsafe { - BeetsLibrarySystemExecutor::default().config(Some( + BeetsLibraryCommandExecutor::default().config(Some( &fs::canonicalize("./tests/files/library/config.yml").unwrap(), )) })))) -- 2.45.2