From 95ee681229aa8a1957a08e250c4f207742a6194d Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Fri, 12 Jan 2024 21:34:01 +0100 Subject: [PATCH] IDatabase::load should return D not take a mutable reference of it (#99) Closes #96 Reviewed-on: https://git.thenineworlds.net/wojtek/musichoard/pulls/99 --- src/database/json/mod.rs | 18 +++++++----------- src/database/mod.rs | 21 +++++++++------------ src/lib.rs | 10 +++------- src/main.rs | 4 ++-- tests/database/json.rs | 6 ++---- 5 files changed, 23 insertions(+), 36 deletions(-) diff --git a/src/database/json/mod.rs b/src/database/json/mod.rs index 6013fbb..5acce3c 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,12 @@ impl JsonDatabase { } impl IDatabase for JsonDatabase { - fn load(&self, collection: &mut D) -> Result<(), LoadError> { + fn load(&self) -> Result { let serialized = self.backend.read()?; - *collection = serde_json::from_str(&serialized)?; - Ok(()) + Ok(serde_json::from_str(&serialized)?) } - 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(()) @@ -184,8 +182,7 @@ mod tests { let mut backend = MockIJsonDatabaseBackend::new(); backend.expect_read().times(1).return_once(|| result); - let mut read_data: Vec = vec![]; - JsonDatabase::new(backend).load(&mut read_data).unwrap(); + let read_data: Vec = JsonDatabase::new(backend).load().unwrap(); assert_eq!(read_data, expected); } @@ -207,9 +204,8 @@ mod tests { let mut database = JsonDatabase::new(backend); let write_data = COLLECTION.to_owned(); - let mut read_data: Vec = vec![]; database.save(&write_data).unwrap(); - database.load(&mut read_data).unwrap(); + let read_data: Vec = database.load().unwrap(); assert_eq!(write_data, read_data); } diff --git a/src/database/mod.rs b/src/database/mod.rs index 79c63df..13223b4 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) -> Result; /// Save collection to the database. - fn save(&mut self, collection: &S) -> Result<(), SaveError>; + fn save(&mut self, collection: &Collection) -> Result<(), SaveError>; } /// Null database implementation of [`IDatabase`]. pub struct NullDatabase; impl IDatabase for NullDatabase { - fn load(&self, _collection: &mut D) -> Result<(), LoadError> { - Ok(()) + fn load(&self) -> Result { + Ok(vec![]) } - fn save(&mut self, _collection: &S) -> Result<(), SaveError> { + fn save(&mut self, _collection: &Collection) -> Result<(), SaveError> { Ok(()) } } @@ -94,16 +94,13 @@ mod tests { #[test] fn no_database_load() { let database = NullDatabase; - let mut collection: Vec = vec![]; - assert!(database.load(&mut collection).is_ok()); - assert!(collection.is_empty()); + assert!(database.load().unwrap().is_empty()); } #[test] fn no_database_save() { let mut database = NullDatabase; - let collection: Vec = vec![]; - assert!(database.save(&collection).is_ok()); + assert!(database.save(&vec![]).is_ok()); } #[test] diff --git a/src/lib.rs b/src/lib.rs index 4002b47..bbbae7f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -928,8 +928,7 @@ impl MusicHoard { impl MusicHoard { pub fn load_from_database(&mut self) -> Result<(), Error> { - let mut database_collection: Collection = vec![]; - self.database.load(&mut database_collection)?; + let mut database_collection = self.database.load()?; Self::sort(&mut database_collection); let collection = mem::take(&mut self.collection); @@ -1959,10 +1958,7 @@ mod tests { database .expect_load() .times(1) - .return_once(|coll: &mut Collection| { - *coll = COLLECTION.to_owned(); - Ok(()) - }); + .return_once(|| Ok(COLLECTION.to_owned())); let mut music_hoard = MusicHoardBuilder::default() .set_library(library) @@ -2044,7 +2040,7 @@ mod tests { database .expect_load() .times(1) - .return_once(|_: &mut Collection| database_result); + .return_once(|| database_result); let mut music_hoard = MusicHoardBuilder::default() .set_library(library) diff --git a/src/main.rs b/src/main.rs index 0c087d5..5530866 100644 --- a/src/main.rs +++ b/src/main.rs @@ -15,7 +15,7 @@ use musichoard::{ }, ILibrary, NullLibrary, }, - Collection, MusicHoardBuilder, NoDatabase, NoLibrary, + MusicHoardBuilder, NoDatabase, NoLibrary, }; mod tui; @@ -85,7 +85,7 @@ fn with_database(db_opt: DbOpt, builder: MusicHoardBuilder { drop(f); JsonDatabase::new(JsonDatabaseFileBackend::new(&db_opt.database_file_path)) - .save::(&vec![]) + .save(&vec![]) .expect("failed to create empty database"); } Err(e) => match e.kind() { diff --git a/tests/database/json.rs b/tests/database/json.rs index 6a1f698..fd8f2b7 100644 --- a/tests/database/json.rs +++ b/tests/database/json.rs @@ -36,8 +36,7 @@ fn load() { let backend = JsonDatabaseFileBackend::new(&*DATABASE_TEST_FILE); let database = JsonDatabase::new(backend); - let mut read_data: Vec = vec![]; - database.load(&mut read_data).unwrap(); + let read_data: Vec = database.load().unwrap(); let expected = COLLECTION.to_owned(); assert_eq!(read_data, expected); @@ -53,8 +52,7 @@ fn reverse() { let write_data = COLLECTION.to_owned(); database.save(&write_data).unwrap(); - let mut read_data: Vec = vec![]; - database.load(&mut read_data).unwrap(); + let read_data: Vec = database.load().unwrap(); assert_eq!(write_data, read_data); }