IDatabase::load should return D not take a mutable reference of it (#99)
All checks were successful
Cargo CI / Build and Test (push) Successful in 1m1s
Cargo CI / Lint (push) Successful in 43s
Cargo CI / Build and Test (pull_request) Successful in 1m1s
Cargo CI / Lint (pull_request) Successful in 45s

Closes #96

Reviewed-on: #99
This commit is contained in:
Wojciech Kozlowski 2024-01-12 21:34:01 +01:00
parent a315bf4229
commit 95ee681229
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.
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<JDB: IJsonDatabaseBackend> 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()?;
*collection = serde_json::from_str(&serialized)?;
Ok(())
Ok(serde_json::from_str(&serialized)?)
}
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)?;
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<Artist> = vec![];
JsonDatabase::new(backend).load(&mut read_data).unwrap();
let read_data: Vec<Artist> = 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<Artist> = vec![];
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);
}

View File

@ -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<D: DeserializeOwned + 'static>(&self, collection: &mut D) -> Result<(), LoadError>;
fn load(&self) -> Result<Collection, LoadError>;
/// 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`].
pub struct NullDatabase;
impl IDatabase for NullDatabase {
fn load<D: DeserializeOwned + 'static>(&self, _collection: &mut D) -> Result<(), LoadError> {
Ok(())
fn load(&self) -> Result<Collection, LoadError> {
Ok(vec![])
}
fn save<S: Serialize + 'static>(&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<String> = 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<String> = vec![];
assert!(database.save(&collection).is_ok());
assert!(database.save(&vec![]).is_ok());
}
#[test]

View File

@ -928,8 +928,7 @@ impl<LIB: ILibrary, DB> MusicHoard<LIB, DB> {
impl<LIB, DB: IDatabase> MusicHoard<LIB, DB> {
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)

View File

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

View File

@ -36,8 +36,7 @@ fn load() {
let backend = JsonDatabaseFileBackend::new(&*DATABASE_TEST_FILE);
let database = JsonDatabase::new(backend);
let mut read_data: Vec<Artist> = vec![];
database.load(&mut read_data).unwrap();
let read_data: Vec<Artist> = 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<Artist> = vec![];
database.load(&mut read_data).unwrap();
let read_data: Vec<Artist> = database.load().unwrap();
assert_eq!(write_data, read_data);
}