IDatabase::load should return D not take a mutable reference of it #99

Merged
wojtek merged 1 commits from 96---idatabase--load-should-return-d-not-take-a-mutable-reference-of-it into main 2024-01-12 21:34:02 +01:00
5 changed files with 23 additions and 36 deletions

View File

@ -1,11 +1,10 @@
//! Module for storing MusicHoard data in a JSON file database. //! Module for storing MusicHoard data in a JSON file database.
use serde::de::DeserializeOwned;
use serde::Serialize;
#[cfg(test)] #[cfg(test)]
use mockall::automock; use mockall::automock;
use crate::Collection;
use super::{IDatabase, LoadError, SaveError}; use super::{IDatabase, LoadError, SaveError};
pub mod backend; pub mod backend;
@ -46,13 +45,12 @@ impl<JDB: IJsonDatabaseBackend> JsonDatabase<JDB> {
} }
impl<JDB: IJsonDatabaseBackend> IDatabase for JsonDatabase<JDB> { impl<JDB: IJsonDatabaseBackend> IDatabase for JsonDatabase<JDB> {
fn load<D: DeserializeOwned>(&self, collection: &mut D) -> Result<(), LoadError> { fn load(&self) -> Result<Collection, LoadError> {
let serialized = self.backend.read()?; let serialized = self.backend.read()?;
*collection = serde_json::from_str(&serialized)?; Ok(serde_json::from_str(&serialized)?)
Ok(())
} }
fn save<S: Serialize>(&mut self, collection: &S) -> Result<(), SaveError> { fn save(&mut self, collection: &Collection) -> Result<(), SaveError> {
let serialized = serde_json::to_string(&collection)?; let serialized = serde_json::to_string(&collection)?;
self.backend.write(&serialized)?; self.backend.write(&serialized)?;
Ok(()) Ok(())
@ -184,8 +182,7 @@ mod tests {
let mut backend = MockIJsonDatabaseBackend::new(); let mut backend = MockIJsonDatabaseBackend::new();
backend.expect_read().times(1).return_once(|| result); backend.expect_read().times(1).return_once(|| result);
let mut read_data: Vec<Artist> = vec![]; let read_data: Vec<Artist> = JsonDatabase::new(backend).load().unwrap();
JsonDatabase::new(backend).load(&mut read_data).unwrap();
assert_eq!(read_data, expected); assert_eq!(read_data, expected);
} }
@ -207,9 +204,8 @@ mod tests {
let mut database = JsonDatabase::new(backend); let mut database = JsonDatabase::new(backend);
let write_data = COLLECTION.to_owned(); let write_data = COLLECTION.to_owned();
let mut read_data: Vec<Artist> = vec![];
database.save(&write_data).unwrap(); database.save(&write_data).unwrap();
database.load(&mut read_data).unwrap(); let read_data: Vec<Artist> = database.load().unwrap();
assert_eq!(write_data, read_data); assert_eq!(write_data, read_data);
} }

View File

@ -2,11 +2,11 @@
use std::fmt; use std::fmt;
use serde::{de::DeserializeOwned, Serialize};
#[cfg(test)] #[cfg(test)]
use mockall::automock; use mockall::automock;
use crate::Collection;
#[cfg(feature = "database-json")] #[cfg(feature = "database-json")]
pub mod json; pub mod json;
@ -14,21 +14,21 @@ pub mod json;
#[cfg_attr(test, automock)] #[cfg_attr(test, automock)]
pub trait IDatabase { pub trait IDatabase {
/// Load collection from the database. /// Load collection from the database.
fn load<D: DeserializeOwned + 'static>(&self, collection: &mut D) -> Result<(), LoadError>; fn load(&self) -> Result<Collection, LoadError>;
/// Save collection to the database. /// Save collection to the database.
fn save<S: Serialize + 'static>(&mut self, collection: &S) -> Result<(), SaveError>; fn save(&mut self, collection: &Collection) -> Result<(), SaveError>;
} }
/// Null database implementation of [`IDatabase`]. /// Null database implementation of [`IDatabase`].
pub struct NullDatabase; pub struct NullDatabase;
impl IDatabase for NullDatabase { impl IDatabase for NullDatabase {
fn load<D: DeserializeOwned + 'static>(&self, _collection: &mut D) -> Result<(), LoadError> { fn load(&self) -> Result<Collection, LoadError> {
Ok(()) Ok(vec![])
} }
fn save<S: Serialize + 'static>(&mut self, _collection: &S) -> Result<(), SaveError> { fn save(&mut self, _collection: &Collection) -> Result<(), SaveError> {
Ok(()) Ok(())
} }
} }
@ -94,16 +94,13 @@ mod tests {
#[test] #[test]
fn no_database_load() { fn no_database_load() {
let database = NullDatabase; let database = NullDatabase;
let mut collection: Vec<String> = vec![]; assert!(database.load().unwrap().is_empty());
assert!(database.load(&mut collection).is_ok());
assert!(collection.is_empty());
} }
#[test] #[test]
fn no_database_save() { fn no_database_save() {
let mut database = NullDatabase; let mut database = NullDatabase;
let collection: Vec<String> = vec![]; assert!(database.save(&vec![]).is_ok());
assert!(database.save(&collection).is_ok());
} }
#[test] #[test]

View File

@ -928,8 +928,7 @@ impl<LIB: ILibrary, DB> MusicHoard<LIB, DB> {
impl<LIB, DB: IDatabase> MusicHoard<LIB, DB> { impl<LIB, DB: IDatabase> MusicHoard<LIB, DB> {
pub fn load_from_database(&mut self) -> Result<(), Error> { pub fn load_from_database(&mut self) -> Result<(), Error> {
let mut database_collection: Collection = vec![]; let mut database_collection = self.database.load()?;
self.database.load(&mut database_collection)?;
Self::sort(&mut database_collection); Self::sort(&mut database_collection);
let collection = mem::take(&mut self.collection); let collection = mem::take(&mut self.collection);
@ -1959,10 +1958,7 @@ mod tests {
database database
.expect_load() .expect_load()
.times(1) .times(1)
.return_once(|coll: &mut Collection| { .return_once(|| Ok(COLLECTION.to_owned()));
*coll = COLLECTION.to_owned();
Ok(())
});
let mut music_hoard = MusicHoardBuilder::default() let mut music_hoard = MusicHoardBuilder::default()
.set_library(library) .set_library(library)
@ -2044,7 +2040,7 @@ mod tests {
database database
.expect_load() .expect_load()
.times(1) .times(1)
.return_once(|_: &mut Collection| database_result); .return_once(|| database_result);
let mut music_hoard = MusicHoardBuilder::default() let mut music_hoard = MusicHoardBuilder::default()
.set_library(library) .set_library(library)

View File

@ -15,7 +15,7 @@ use musichoard::{
}, },
ILibrary, NullLibrary, ILibrary, NullLibrary,
}, },
Collection, MusicHoardBuilder, NoDatabase, NoLibrary, MusicHoardBuilder, NoDatabase, NoLibrary,
}; };
mod tui; mod tui;
@ -85,7 +85,7 @@ fn with_database<LIB: ILibrary>(db_opt: DbOpt, builder: MusicHoardBuilder<LIB, N
Ok(f) => { Ok(f) => {
drop(f); drop(f);
JsonDatabase::new(JsonDatabaseFileBackend::new(&db_opt.database_file_path)) JsonDatabase::new(JsonDatabaseFileBackend::new(&db_opt.database_file_path))
.save::<Collection>(&vec![]) .save(&vec![])
.expect("failed to create empty database"); .expect("failed to create empty database");
} }
Err(e) => match e.kind() { Err(e) => match e.kind() {

View File

@ -36,8 +36,7 @@ fn load() {
let backend = JsonDatabaseFileBackend::new(&*DATABASE_TEST_FILE); let backend = JsonDatabaseFileBackend::new(&*DATABASE_TEST_FILE);
let database = JsonDatabase::new(backend); let database = JsonDatabase::new(backend);
let mut read_data: Vec<Artist> = vec![]; let read_data: Vec<Artist> = database.load().unwrap();
database.load(&mut read_data).unwrap();
let expected = COLLECTION.to_owned(); let expected = COLLECTION.to_owned();
assert_eq!(read_data, expected); assert_eq!(read_data, expected);
@ -53,8 +52,7 @@ fn reverse() {
let write_data = COLLECTION.to_owned(); let write_data = COLLECTION.to_owned();
database.save(&write_data).unwrap(); database.save(&write_data).unwrap();
let mut read_data: Vec<Artist> = vec![]; let read_data: Vec<Artist> = database.load().unwrap();
database.load(&mut read_data).unwrap();
assert_eq!(write_data, read_data); assert_eq!(write_data, read_data);
} }