From ce3571e393eeb869acdf541a1099d39524e286c1 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sat, 4 Jan 2025 23:26:03 +0100 Subject: [PATCH 1/6] Code fix --- src/core/interface/database/mod.rs | 8 +++---- src/core/musichoard/database.rs | 35 +++++++++++++++++------------- src/core/musichoard/library.rs | 2 +- src/external/database/json/mod.rs | 5 +++-- 4 files changed, 28 insertions(+), 22 deletions(-) diff --git a/src/core/interface/database/mod.rs b/src/core/interface/database/mod.rs index b60575c..dd93d4d 100644 --- a/src/core/interface/database/mod.rs +++ b/src/core/interface/database/mod.rs @@ -13,8 +13,8 @@ pub trait IDatabase { /// Load collection from the database. fn load(&self) -> Result; - /// Save collection to the database. - fn save(&mut self, collection: &Collection) -> Result<(), SaveError>; + /// Save collection to the database. Return the written data. + fn save(&mut self, collection: &Collection) -> Result; } /// Null database implementation of [`IDatabase`]. @@ -25,8 +25,8 @@ impl IDatabase for NullDatabase { Ok(vec![]) } - fn save(&mut self, _collection: &Collection) -> Result<(), SaveError> { - Ok(()) + fn save(&mut self, _collection: &Collection) -> Result { + Ok(vec![]) } } diff --git a/src/core/musichoard/database.rs b/src/core/musichoard/database.rs index 990871c..392546c 100644 --- a/src/core/musichoard/database.rs +++ b/src/core/musichoard/database.rs @@ -374,9 +374,14 @@ impl IMusicHoardDatabasePrivate for MusicHoard { impl IMusicHoardDatabasePrivate for MusicHoard { fn commit(&mut self) -> Result<(), Error> { if self.collection != self.pre_commit { - if let Err(err) = self.database.save(&self.pre_commit) { - self.pre_commit = self.collection.clone(); - return Err(err.into()); + match self.database.save(&self.pre_commit) { + Ok(collection) => { + self.database_cache = collection; + } + Err(err) => { + self.pre_commit = self.collection.clone(); + return Err(err.into()); + } } self.collection = self.pre_commit.clone(); self.filtered = self.filter_collection(); @@ -496,13 +501,13 @@ mod tests { .times(1) .in_sequence(&mut seq) .with(predicate::eq(with_artist.clone())) - .returning(|_| Ok(())); + .returning(|_| Ok(vec![])); database .expect_save() .times(1) .in_sequence(&mut seq) .with(predicate::eq(collection.clone())) - .returning(|_| Ok(())); + .returning(|_| Ok(vec![])); let mut music_hoard = MusicHoard::database(database).unwrap(); assert_eq!(music_hoard.collection, collection); @@ -524,7 +529,7 @@ mod tests { fn artist_sort_set_clear() { let mut database = MockIDatabase::new(); database.expect_load().times(1).returning(|| Ok(vec![])); - database.expect_save().times(4).returning(|_| Ok(())); + database.expect_save().times(4).returning(|_| Ok(vec![])); type MH = MusicHoard; let mut music_hoard: MH = MusicHoard::database(database).unwrap(); @@ -592,7 +597,7 @@ mod tests { fn set_clear_artist_mb_ref() { let mut database = MockIDatabase::new(); database.expect_load().times(1).returning(|| Ok(vec![])); - database.expect_save().times(3).returning(|_| Ok(())); + database.expect_save().times(3).returning(|_| Ok(vec![])); let mut artist_id = ArtistId::new("an artist"); let artist_id_2 = ArtistId::new("another artist"); @@ -638,7 +643,7 @@ mod tests { fn set_clear_artist_info() { let mut database = MockIDatabase::new(); database.expect_load().times(1).returning(|| Ok(vec![])); - database.expect_save().times(3).returning(|_| Ok(())); + database.expect_save().times(3).returning(|_| Ok(vec![])); let artist_id = ArtistId::new("an artist"); let artist_id_2 = ArtistId::new("another artist"); @@ -682,7 +687,7 @@ mod tests { fn add_to_remove_from_property() { let mut database = MockIDatabase::new(); database.expect_load().times(1).returning(|| Ok(vec![])); - database.expect_save().times(3).returning(|_| Ok(())); + database.expect_save().times(3).returning(|_| Ok(vec![])); let artist_id = ArtistId::new("an artist"); let artist_id_2 = ArtistId::new("another artist"); @@ -731,7 +736,7 @@ mod tests { fn set_clear_property() { let mut database = MockIDatabase::new(); database.expect_load().times(1).returning(|| Ok(vec![])); - database.expect_save().times(3).returning(|_| Ok(())); + database.expect_save().times(3).returning(|_| Ok(vec![])); let artist_id = ArtistId::new("an artist"); let artist_id_2 = ArtistId::new("another artist"); @@ -797,13 +802,13 @@ mod tests { .times(1) .in_sequence(&mut seq) .with(predicate::eq(with_album.clone())) - .returning(|_| Ok(())); + .returning(|_| Ok(vec![])); database .expect_save() .times(1) .in_sequence(&mut seq) .with(predicate::eq(collection.clone())) - .returning(|_| Ok(())); + .returning(|_| Ok(vec![])); let mut music_hoard = MusicHoard::database(database).unwrap(); assert_eq!(music_hoard.collection, collection); @@ -842,7 +847,7 @@ mod tests { .expect_load() .times(1) .return_once(|| Ok(database_result)); - database.expect_save().times(2).returning(|_| Ok(())); + database.expect_save().times(2).returning(|_| Ok(vec![])); let mut music_hoard = MusicHoard::database(database).unwrap(); let album = &music_hoard.collection[0].albums[0]; @@ -900,7 +905,7 @@ mod tests { .expect_load() .times(1) .return_once(|| Ok(database_result)); - database.expect_save().times(2).returning(|_| Ok(())); + database.expect_save().times(2).returning(|_| Ok(vec![])); let mut music_hoard = MusicHoard::database(database).unwrap(); assert_eq!(music_hoard.collection[0].albums[0].meta.seq, AlbumSeq(0)); @@ -940,7 +945,7 @@ mod tests { .expect_load() .times(1) .return_once(|| Ok(database_result)); - database.expect_save().times(2).returning(|_| Ok(())); + database.expect_save().times(2).returning(|_| Ok(vec![])); let mut music_hoard = MusicHoard::database(database).unwrap(); let meta = &music_hoard.collection[0].albums[0].meta; diff --git a/src/core/musichoard/library.rs b/src/core/musichoard/library.rs index 70eabe2..86abcee 100644 --- a/src/core/musichoard/library.rs +++ b/src/core/musichoard/library.rs @@ -157,7 +157,7 @@ mod tests { .expect_save() .with(predicate::eq(&*LIBRARY_COLLECTION)) .times(1) - .return_once(|_| Ok(())); + .return_once(|_| Ok(vec![])); let mut music_hoard = MusicHoard::new(database, library).unwrap(); diff --git a/src/external/database/json/mod.rs b/src/external/database/json/mod.rs index 0b7edf9..40762a3 100644 --- a/src/external/database/json/mod.rs +++ b/src/external/database/json/mod.rs @@ -55,11 +55,12 @@ impl IDatabase for JsonDatabase { Ok(database.into()) } - fn save(&mut self, collection: &Collection) -> Result<(), SaveError> { + fn save(&mut self, collection: &Collection) -> Result { let database: SerializeDatabase = collection.into(); let serialized = serde_json::to_string(&database)?; + let deserialized: DeserializeDatabase = serde_json::from_str(&serialized)?; self.backend.write(&serialized)?; - Ok(()) + Ok(deserialized.into()) } } -- 2.47.1 From e64dac6798bf740b077352511198c132c03cca57 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 5 Jan 2025 09:48:09 +0100 Subject: [PATCH 2/6] Revert "Code fix" This reverts commit ce3571e393eeb869acdf541a1099d39524e286c1. --- src/core/interface/database/mod.rs | 8 +++---- src/core/musichoard/database.rs | 35 +++++++++++++----------------- src/core/musichoard/library.rs | 2 +- src/external/database/json/mod.rs | 5 ++--- 4 files changed, 22 insertions(+), 28 deletions(-) diff --git a/src/core/interface/database/mod.rs b/src/core/interface/database/mod.rs index dd93d4d..b60575c 100644 --- a/src/core/interface/database/mod.rs +++ b/src/core/interface/database/mod.rs @@ -13,8 +13,8 @@ pub trait IDatabase { /// Load collection from the database. fn load(&self) -> Result; - /// Save collection to the database. Return the written data. - fn save(&mut self, collection: &Collection) -> Result; + /// Save collection to the database. + fn save(&mut self, collection: &Collection) -> Result<(), SaveError>; } /// Null database implementation of [`IDatabase`]. @@ -25,8 +25,8 @@ impl IDatabase for NullDatabase { Ok(vec![]) } - fn save(&mut self, _collection: &Collection) -> Result { - Ok(vec![]) + fn save(&mut self, _collection: &Collection) -> Result<(), SaveError> { + Ok(()) } } diff --git a/src/core/musichoard/database.rs b/src/core/musichoard/database.rs index 392546c..990871c 100644 --- a/src/core/musichoard/database.rs +++ b/src/core/musichoard/database.rs @@ -374,14 +374,9 @@ impl IMusicHoardDatabasePrivate for MusicHoard { impl IMusicHoardDatabasePrivate for MusicHoard { fn commit(&mut self) -> Result<(), Error> { if self.collection != self.pre_commit { - match self.database.save(&self.pre_commit) { - Ok(collection) => { - self.database_cache = collection; - } - Err(err) => { - self.pre_commit = self.collection.clone(); - return Err(err.into()); - } + if let Err(err) = self.database.save(&self.pre_commit) { + self.pre_commit = self.collection.clone(); + return Err(err.into()); } self.collection = self.pre_commit.clone(); self.filtered = self.filter_collection(); @@ -501,13 +496,13 @@ mod tests { .times(1) .in_sequence(&mut seq) .with(predicate::eq(with_artist.clone())) - .returning(|_| Ok(vec![])); + .returning(|_| Ok(())); database .expect_save() .times(1) .in_sequence(&mut seq) .with(predicate::eq(collection.clone())) - .returning(|_| Ok(vec![])); + .returning(|_| Ok(())); let mut music_hoard = MusicHoard::database(database).unwrap(); assert_eq!(music_hoard.collection, collection); @@ -529,7 +524,7 @@ mod tests { fn artist_sort_set_clear() { let mut database = MockIDatabase::new(); database.expect_load().times(1).returning(|| Ok(vec![])); - database.expect_save().times(4).returning(|_| Ok(vec![])); + database.expect_save().times(4).returning(|_| Ok(())); type MH = MusicHoard; let mut music_hoard: MH = MusicHoard::database(database).unwrap(); @@ -597,7 +592,7 @@ mod tests { fn set_clear_artist_mb_ref() { let mut database = MockIDatabase::new(); database.expect_load().times(1).returning(|| Ok(vec![])); - database.expect_save().times(3).returning(|_| Ok(vec![])); + database.expect_save().times(3).returning(|_| Ok(())); let mut artist_id = ArtistId::new("an artist"); let artist_id_2 = ArtistId::new("another artist"); @@ -643,7 +638,7 @@ mod tests { fn set_clear_artist_info() { let mut database = MockIDatabase::new(); database.expect_load().times(1).returning(|| Ok(vec![])); - database.expect_save().times(3).returning(|_| Ok(vec![])); + database.expect_save().times(3).returning(|_| Ok(())); let artist_id = ArtistId::new("an artist"); let artist_id_2 = ArtistId::new("another artist"); @@ -687,7 +682,7 @@ mod tests { fn add_to_remove_from_property() { let mut database = MockIDatabase::new(); database.expect_load().times(1).returning(|| Ok(vec![])); - database.expect_save().times(3).returning(|_| Ok(vec![])); + database.expect_save().times(3).returning(|_| Ok(())); let artist_id = ArtistId::new("an artist"); let artist_id_2 = ArtistId::new("another artist"); @@ -736,7 +731,7 @@ mod tests { fn set_clear_property() { let mut database = MockIDatabase::new(); database.expect_load().times(1).returning(|| Ok(vec![])); - database.expect_save().times(3).returning(|_| Ok(vec![])); + database.expect_save().times(3).returning(|_| Ok(())); let artist_id = ArtistId::new("an artist"); let artist_id_2 = ArtistId::new("another artist"); @@ -802,13 +797,13 @@ mod tests { .times(1) .in_sequence(&mut seq) .with(predicate::eq(with_album.clone())) - .returning(|_| Ok(vec![])); + .returning(|_| Ok(())); database .expect_save() .times(1) .in_sequence(&mut seq) .with(predicate::eq(collection.clone())) - .returning(|_| Ok(vec![])); + .returning(|_| Ok(())); let mut music_hoard = MusicHoard::database(database).unwrap(); assert_eq!(music_hoard.collection, collection); @@ -847,7 +842,7 @@ mod tests { .expect_load() .times(1) .return_once(|| Ok(database_result)); - database.expect_save().times(2).returning(|_| Ok(vec![])); + database.expect_save().times(2).returning(|_| Ok(())); let mut music_hoard = MusicHoard::database(database).unwrap(); let album = &music_hoard.collection[0].albums[0]; @@ -905,7 +900,7 @@ mod tests { .expect_load() .times(1) .return_once(|| Ok(database_result)); - database.expect_save().times(2).returning(|_| Ok(vec![])); + database.expect_save().times(2).returning(|_| Ok(())); let mut music_hoard = MusicHoard::database(database).unwrap(); assert_eq!(music_hoard.collection[0].albums[0].meta.seq, AlbumSeq(0)); @@ -945,7 +940,7 @@ mod tests { .expect_load() .times(1) .return_once(|| Ok(database_result)); - database.expect_save().times(2).returning(|_| Ok(vec![])); + database.expect_save().times(2).returning(|_| Ok(())); let mut music_hoard = MusicHoard::database(database).unwrap(); let meta = &music_hoard.collection[0].albums[0].meta; diff --git a/src/core/musichoard/library.rs b/src/core/musichoard/library.rs index 86abcee..70eabe2 100644 --- a/src/core/musichoard/library.rs +++ b/src/core/musichoard/library.rs @@ -157,7 +157,7 @@ mod tests { .expect_save() .with(predicate::eq(&*LIBRARY_COLLECTION)) .times(1) - .return_once(|_| Ok(vec![])); + .return_once(|_| Ok(())); let mut music_hoard = MusicHoard::new(database, library).unwrap(); diff --git a/src/external/database/json/mod.rs b/src/external/database/json/mod.rs index 40762a3..0b7edf9 100644 --- a/src/external/database/json/mod.rs +++ b/src/external/database/json/mod.rs @@ -55,12 +55,11 @@ impl IDatabase for JsonDatabase { Ok(database.into()) } - fn save(&mut self, collection: &Collection) -> Result { + fn save(&mut self, collection: &Collection) -> Result<(), SaveError> { let database: SerializeDatabase = collection.into(); let serialized = serde_json::to_string(&database)?; - let deserialized: DeserializeDatabase = serde_json::from_str(&serialized)?; self.backend.write(&serialized)?; - Ok(deserialized.into()) + Ok(()) } } -- 2.47.1 From d2e3fd4cd813aac9277f96975250335e1d98ea44 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 5 Jan 2025 10:26:00 +0100 Subject: [PATCH 3/6] Code fix and passing UTs --- src/core/musichoard/base.rs | 43 +++++++++++++-------------------- src/core/musichoard/builder.rs | 4 --- src/core/musichoard/database.rs | 6 ++--- src/core/musichoard/library.rs | 25 +++++++++++++++---- src/core/musichoard/mod.rs | 1 - 5 files changed, 40 insertions(+), 39 deletions(-) diff --git a/src/core/musichoard/base.rs b/src/core/musichoard/base.rs index 08a04c7..c405e9b 100644 --- a/src/core/musichoard/base.rs +++ b/src/core/musichoard/base.rs @@ -33,7 +33,7 @@ pub trait IMusicHoardBasePrivate { fn sort_artists(collection: &mut [Artist]); fn sort_albums_and_tracks<'a, C: Iterator>(collection: C); - fn merge_collections(&self) -> Collection; + fn merge_collections>(&self, database: It) -> Collection; fn filter_collection(&self) -> Collection; fn filter_artist(&self, artist: &Artist) -> Option; @@ -69,19 +69,19 @@ impl IMusicHoardBasePrivate for MusicHoard } } - fn merge_collections(&self) -> Collection { - let mut primary = NormalMap::::new(); - let mut secondary = NormalMap::::new(); + fn merge_collections>(&self, database: It) -> Collection { + let mut primary_map = NormalMap::::new(); + let mut secondary_map = NormalMap::::new(); for artist in self.library_cache.iter().cloned() { - primary.insert(string::normalize_string(&artist.meta.id.name), artist); + primary_map.insert(string::normalize_string(&artist.meta.id.name), artist); } - for artist in self.database_cache.iter().cloned() { - secondary.insert(string::normalize_string(&artist.meta.id.name), artist); + for artist in database.into_iter() { + secondary_map.insert(string::normalize_string(&artist.meta.id.name), artist); } - let mut collection = MergeCollections::merge_by_name(primary, secondary); + let mut collection = MergeCollections::merge_by_name(primary_map, secondary_map); collection.sort_unstable(); collection @@ -168,21 +168,19 @@ mod tests { let mut mh = MusicHoard { library_cache: left.clone(), - database_cache: right.clone(), ..Default::default() }; - mh.collection = mh.merge_collections(); + mh.collection = mh.merge_collections(right.clone()); assert_eq!(expected, mh.collection); // The merge is completely non-overlapping so it should be commutative. let mut mh = MusicHoard { library_cache: right.clone(), - database_cache: left.clone(), ..Default::default() }; - mh.collection = mh.merge_collections(); + mh.collection = mh.merge_collections(left.clone()); assert_eq!(expected, mh.collection); } @@ -198,21 +196,19 @@ mod tests { let mut mh = MusicHoard { library_cache: left.clone(), - database_cache: right.clone(), ..Default::default() }; - mh.collection = mh.merge_collections(); + mh.collection = mh.merge_collections(right.clone()); assert_eq!(expected, mh.collection); // The merge does not overwrite any data so it should be commutative. let mut mh = MusicHoard { library_cache: right.clone(), - database_cache: left.clone(), ..Default::default() }; - mh.collection = mh.merge_collections(); + mh.collection = mh.merge_collections(left.clone()); assert_eq!(expected, mh.collection); } @@ -241,21 +237,19 @@ mod tests { let mut mh = MusicHoard { library_cache: left.clone(), - database_cache: right.clone(), ..Default::default() }; - mh.collection = mh.merge_collections(); + mh.collection = mh.merge_collections(right.clone()); assert_eq!(expected, mh.collection); // The merge overwrites the sort data, but no data is erased so it should be commutative. let mut mh = MusicHoard { library_cache: right.clone(), - database_cache: left.clone(), ..Default::default() }; - mh.collection = mh.merge_collections(); + mh.collection = mh.merge_collections(left.clone()); assert_eq!(expected, mh.collection); } @@ -273,11 +267,10 @@ mod tests { let mut mh = MusicHoard { library_cache: left.clone(), - database_cache: right.clone(), ..Default::default() }; - mh.collection = mh.merge_collections(); + mh.collection = mh.merge_collections(right.clone()); } #[test] @@ -294,11 +287,10 @@ mod tests { let mut mh = MusicHoard { library_cache: left.clone(), - database_cache: right.clone(), ..Default::default() }; - mh.collection = mh.merge_collections(); + mh.collection = mh.merge_collections(right.clone()); } #[test] @@ -318,11 +310,10 @@ mod tests { let mut mh = MusicHoard { library_cache: left.clone(), - database_cache: right.clone(), ..Default::default() }; - mh.collection = mh.merge_collections(); + mh.collection = mh.merge_collections(right.clone()); assert_eq!(expected, mh.collection); } diff --git a/src/core/musichoard/builder.rs b/src/core/musichoard/builder.rs index 1901a9f..d030fd7 100644 --- a/src/core/musichoard/builder.rs +++ b/src/core/musichoard/builder.rs @@ -69,7 +69,6 @@ impl MusicHoard { collection: vec![], pre_commit: vec![], database: NoDatabase, - database_cache: vec![], library: NoLibrary, library_cache: vec![], } @@ -92,7 +91,6 @@ impl MusicHoard { collection: vec![], pre_commit: vec![], database: NoDatabase, - database_cache: vec![], library, library_cache: vec![], } @@ -115,7 +113,6 @@ impl MusicHoard { collection: vec![], pre_commit: vec![], database, - database_cache: vec![], library: NoLibrary, library_cache: vec![], }; @@ -140,7 +137,6 @@ impl MusicHoard { collection: vec![], pre_commit: vec![], database, - database_cache: vec![], library, library_cache: vec![], }; diff --git a/src/core/musichoard/database.rs b/src/core/musichoard/database.rs index 990871c..cf6c6f3 100644 --- a/src/core/musichoard/database.rs +++ b/src/core/musichoard/database.rs @@ -116,10 +116,10 @@ pub trait IMusicHoardDatabase { impl IMusicHoardDatabase for MusicHoard { fn reload_database(&mut self) -> Result<(), Error> { - self.database_cache = self.database.load()?; - Self::sort_albums_and_tracks(self.database_cache.iter_mut()); + let mut database_cache = self.database.load()?; + Self::sort_albums_and_tracks(database_cache.iter_mut()); - self.collection = self.merge_collections(); + self.collection = self.merge_collections(database_cache); self.filtered = self.filter_collection(); self.pre_commit = self.collection.clone(); diff --git a/src/core/musichoard/library.rs b/src/core/musichoard/library.rs index 70eabe2..2107cdb 100644 --- a/src/core/musichoard/library.rs +++ b/src/core/musichoard/library.rs @@ -23,25 +23,28 @@ pub trait IMusicHoardLibrary { impl IMusicHoardLibrary for MusicHoard { fn rescan_library(&mut self) -> Result<(), Error> { - self.pre_commit = self.rescan_library_inner()?; + self.pre_commit = self.rescan_library_inner(vec![])?; self.commit() } } impl IMusicHoardLibrary for MusicHoard { fn rescan_library(&mut self) -> Result<(), Error> { - self.pre_commit = self.rescan_library_inner()?; + let mut database_cache = self.database.load()?; + Self::sort_albums_and_tracks(database_cache.iter_mut()); + + self.pre_commit = self.rescan_library_inner(database_cache)?; self.commit() } } impl MusicHoard { - fn rescan_library_inner(&mut self) -> Result { + fn rescan_library_inner(&mut self, database: Collection) -> Result { let items = self.library.list(&Query::new())?; self.library_cache = Self::items_to_artists(items)?; Self::sort_albums_and_tracks(self.library_cache.iter_mut()); - Ok(self.merge_collections()) + Ok(self.merge_collections(database)) } fn items_to_artists(items: Vec) -> Result { @@ -152,11 +155,23 @@ mod tests { .times(1) .return_once(|_| library_result); - database.expect_load().times(1).returning(|| Ok(vec![])); + // The database contents are not relevant in this test. + let mut seq = Sequence::new(); + database + .expect_load() + .times(1) + .in_sequence(&mut seq) + .returning(|| Ok(vec![])); + database + .expect_load() + .times(1) + .in_sequence(&mut seq) + .returning(|| Ok(vec![])); database .expect_save() .with(predicate::eq(&*LIBRARY_COLLECTION)) .times(1) + .in_sequence(&mut seq) .return_once(|_| Ok(())); let mut music_hoard = MusicHoard::new(database, library).unwrap(); diff --git a/src/core/musichoard/mod.rs b/src/core/musichoard/mod.rs index 3b3230d..bfe9c04 100644 --- a/src/core/musichoard/mod.rs +++ b/src/core/musichoard/mod.rs @@ -32,7 +32,6 @@ pub struct MusicHoard { collection: Collection, pre_commit: Collection, database: Database, - database_cache: Collection, library: Library, library_cache: Collection, } -- 2.47.1 From 180a2edd83316cac0c60c61e5b2593ea32ad920a Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 5 Jan 2025 10:45:38 +0100 Subject: [PATCH 4/6] Make database loads explicit by the MH user --- src/bin/musichoard-edit.rs | 8 +++--- src/core/musichoard/builder.rs | 34 ++++++++++--------------- src/core/musichoard/database.rs | 44 +++++++++++++++++---------------- src/core/musichoard/library.rs | 7 +----- src/main.rs | 2 +- tests/lib.rs | 4 +-- 6 files changed, 44 insertions(+), 55 deletions(-) diff --git a/src/bin/musichoard-edit.rs b/src/bin/musichoard-edit.rs index 3941786c..d6a1533 100644 --- a/src/bin/musichoard-edit.rs +++ b/src/bin/musichoard-edit.rs @@ -249,9 +249,9 @@ fn main() { let db = JsonDatabase::new(JsonDatabaseFileBackend::new(&opt.database_file_path)); - let mut music_hoard = MusicHoardBuilder::default() - .set_database(db) - .build() - .expect("failed to initialise MusicHoard"); + let mut music_hoard = MusicHoardBuilder::default().set_database(db).build(); + music_hoard + .reload_database() + .expect("failed to load MusicHoard database"); opt.command.handle(&mut music_hoard); } diff --git a/src/core/musichoard/builder.rs b/src/core/musichoard/builder.rs index d030fd7..fd285d3 100644 --- a/src/core/musichoard/builder.rs +++ b/src/core/musichoard/builder.rs @@ -1,8 +1,6 @@ use crate::core::{ interface::{database::IDatabase, library::ILibrary}, - musichoard::{ - database::IMusicHoardDatabase, CollectionFilter, Error, MusicHoard, NoDatabase, NoLibrary, - }, + musichoard::{CollectionFilter, MusicHoard, NoDatabase, NoLibrary}, }; /// Builder for [`MusicHoard`]. Its purpose is to make it easier to set various combinations of @@ -99,15 +97,15 @@ impl MusicHoard { impl MusicHoardBuilder { /// Build [`MusicHoard`] with the currently set library and database. - pub fn build(self) -> Result, Error> { + pub fn build(self) -> MusicHoard { MusicHoard::database(self.database) } } impl MusicHoard { /// Create a new [`MusicHoard`] with the provided [`IDatabase`] and no library. - pub fn database(database: Database) -> Result { - let mut mh = MusicHoard { + pub fn database(database: Database) -> Self { + MusicHoard { filter: CollectionFilter::default(), filtered: vec![], collection: vec![], @@ -115,23 +113,21 @@ impl MusicHoard { database, library: NoLibrary, library_cache: vec![], - }; - mh.reload_database()?; - Ok(mh) + } } } impl MusicHoardBuilder { /// Build [`MusicHoard`] with the currently set library and database. - pub fn build(self) -> Result, Error> { + pub fn build(self) -> MusicHoard { MusicHoard::new(self.database, self.library) } } impl MusicHoard { /// Create a new [`MusicHoard`] with the provided [`ILibrary`] and [`IDatabase`]. - pub fn new(database: Database, library: Library) -> Result { - let mut mh = MusicHoard { + pub fn new(database: Database, library: Library) -> Self { + MusicHoard { filter: CollectionFilter::default(), filtered: vec![], collection: vec![], @@ -139,18 +135,16 @@ impl MusicHoard { database, library, library_cache: vec![], - }; - mh.reload_database()?; - Ok(mh) + } } } #[cfg(test)] mod tests { - use crate::core::{ + use crate::{core::{ interface::{database::NullDatabase, library::NullLibrary}, musichoard::library::IMusicHoardLibrary, - }; + }, IMusicHoardDatabase}; use super::*; @@ -177,8 +171,7 @@ mod tests { fn no_library_with_database() { let mut mh = MusicHoardBuilder::default() .set_database(NullDatabase) - .build() - .unwrap(); + .build(); assert!(mh.reload_database().is_ok()); } @@ -187,8 +180,7 @@ mod tests { let mut mh = MusicHoardBuilder::default() .set_library(NullLibrary) .set_database(NullDatabase) - .build() - .unwrap(); + .build(); assert!(mh.rescan_library().is_ok()); assert!(mh.reload_database().is_ok()); } diff --git a/src/core/musichoard/database.rs b/src/core/musichoard/database.rs index cf6c6f3..ae9e58e 100644 --- a/src/core/musichoard/database.rs +++ b/src/core/musichoard/database.rs @@ -504,7 +504,8 @@ mod tests { .with(predicate::eq(collection.clone())) .returning(|_| Ok(())); - let mut music_hoard = MusicHoard::database(database).unwrap(); + let mut music_hoard = MusicHoard::database(database); + music_hoard.reload_database().unwrap(); assert_eq!(music_hoard.collection, collection); assert!(music_hoard.add_artist(artist_id.clone()).is_ok()); @@ -523,11 +524,10 @@ mod tests { #[test] fn artist_sort_set_clear() { let mut database = MockIDatabase::new(); - database.expect_load().times(1).returning(|| Ok(vec![])); database.expect_save().times(4).returning(|_| Ok(())); type MH = MusicHoard; - let mut music_hoard: MH = MusicHoard::database(database).unwrap(); + let mut music_hoard: MH = MusicHoard::database(database); let artist_1_id = ArtistId::new("the artist"); let artist_1_sort = String::from("artist, the"); @@ -574,9 +574,8 @@ mod tests { #[test] fn collection_error() { - let mut database = MockIDatabase::new(); - database.expect_load().times(1).returning(|| Ok(vec![])); - let mut music_hoard = MusicHoard::database(database).unwrap(); + let database = MockIDatabase::new(); + let mut music_hoard = MusicHoard::database(database); let artist_id = ArtistId::new("an artist"); let actual_err = music_hoard @@ -591,12 +590,11 @@ mod tests { #[test] fn set_clear_artist_mb_ref() { let mut database = MockIDatabase::new(); - database.expect_load().times(1).returning(|| Ok(vec![])); database.expect_save().times(3).returning(|_| Ok(())); let mut artist_id = ArtistId::new("an artist"); let artist_id_2 = ArtistId::new("another artist"); - let mut music_hoard = MusicHoard::database(database).unwrap(); + let mut music_hoard = MusicHoard::database(database); assert!(music_hoard.add_artist(artist_id.clone()).is_ok()); @@ -637,12 +635,11 @@ mod tests { #[test] fn set_clear_artist_info() { let mut database = MockIDatabase::new(); - database.expect_load().times(1).returning(|| Ok(vec![])); database.expect_save().times(3).returning(|_| Ok(())); let artist_id = ArtistId::new("an artist"); let artist_id_2 = ArtistId::new("another artist"); - let mut music_hoard = MusicHoard::database(database).unwrap(); + let mut music_hoard = MusicHoard::database(database); assert!(music_hoard.add_artist(artist_id.clone()).is_ok()); @@ -681,12 +678,11 @@ mod tests { #[test] fn add_to_remove_from_property() { let mut database = MockIDatabase::new(); - database.expect_load().times(1).returning(|| Ok(vec![])); database.expect_save().times(3).returning(|_| Ok(())); let artist_id = ArtistId::new("an artist"); let artist_id_2 = ArtistId::new("another artist"); - let mut music_hoard = MusicHoard::database(database).unwrap(); + let mut music_hoard = MusicHoard::database(database); assert!(music_hoard.add_artist(artist_id.clone()).is_ok()); @@ -730,12 +726,11 @@ mod tests { #[test] fn set_clear_property() { let mut database = MockIDatabase::new(); - database.expect_load().times(1).returning(|| Ok(vec![])); database.expect_save().times(3).returning(|_| Ok(())); let artist_id = ArtistId::new("an artist"); let artist_id_2 = ArtistId::new("another artist"); - let mut music_hoard = MusicHoard::database(database).unwrap(); + let mut music_hoard = MusicHoard::database(database); assert!(music_hoard.add_artist(artist_id.clone()).is_ok()); @@ -805,7 +800,8 @@ mod tests { .with(predicate::eq(collection.clone())) .returning(|_| Ok(())); - let mut music_hoard = MusicHoard::database(database).unwrap(); + let mut music_hoard = MusicHoard::database(database); + music_hoard.reload_database().unwrap(); assert_eq!(music_hoard.collection, collection); assert!(music_hoard @@ -844,7 +840,8 @@ mod tests { .return_once(|| Ok(database_result)); database.expect_save().times(2).returning(|_| Ok(())); - let mut music_hoard = MusicHoard::database(database).unwrap(); + let mut music_hoard = MusicHoard::database(database); + music_hoard.reload_database().unwrap(); let album = &music_hoard.collection[0].albums[0]; assert_eq!(album.meta.id.mb_ref, AlbumMbRef::None); @@ -902,7 +899,8 @@ mod tests { .return_once(|| Ok(database_result)); database.expect_save().times(2).returning(|_| Ok(())); - let mut music_hoard = MusicHoard::database(database).unwrap(); + let mut music_hoard = MusicHoard::database(database); + music_hoard.reload_database().unwrap(); assert_eq!(music_hoard.collection[0].albums[0].meta.seq, AlbumSeq(0)); // Seting seq on an album not belonging to the artist is an error. @@ -941,7 +939,8 @@ mod tests { .times(1) .return_once(|| Ok(database_result)); database.expect_save().times(2).returning(|_| Ok(())); - let mut music_hoard = MusicHoard::database(database).unwrap(); + let mut music_hoard = MusicHoard::database(database); + music_hoard.reload_database().unwrap(); let meta = &music_hoard.collection[0].albums[0].meta; assert_eq!(meta.info.primary_type, None); @@ -986,7 +985,8 @@ mod tests { .times(1) .return_once(|| Ok(FULL_COLLECTION.to_owned())); - let music_hoard = MusicHoard::database(database).unwrap(); + let mut music_hoard = MusicHoard::database(database); + music_hoard.reload_database().unwrap(); assert_eq!(music_hoard.get_collection(), &*FULL_COLLECTION); } @@ -1002,7 +1002,8 @@ mod tests { .times(1) .return_once(|| database_result); - let actual_err = MusicHoard::database(database).unwrap_err(); + let mut music_hoard = MusicHoard::database(database); + let actual_err = music_hoard.reload_database().unwrap_err(); let expected_err = Error::DatabaseError( database::LoadError::IoError(String::from("I/O error")).to_string(), ); @@ -1023,7 +1024,8 @@ mod tests { .times(1) .return_once(|_: &Collection| database_result); - let mut music_hoard = MusicHoard::database(database).unwrap(); + let mut music_hoard = MusicHoard::database(database); + music_hoard.reload_database().unwrap(); let actual_err = music_hoard .add_artist(ArtistId::new("an artist")) diff --git a/src/core/musichoard/library.rs b/src/core/musichoard/library.rs index 2107cdb..d0deb30 100644 --- a/src/core/musichoard/library.rs +++ b/src/core/musichoard/library.rs @@ -157,11 +157,6 @@ mod tests { // The database contents are not relevant in this test. let mut seq = Sequence::new(); - database - .expect_load() - .times(1) - .in_sequence(&mut seq) - .returning(|| Ok(vec![])); database .expect_load() .times(1) @@ -174,7 +169,7 @@ mod tests { .in_sequence(&mut seq) .return_once(|_| Ok(())); - let mut music_hoard = MusicHoard::new(database, library).unwrap(); + let mut music_hoard = MusicHoard::new(database, library); music_hoard.rescan_library().unwrap(); assert_eq!(music_hoard.get_collection(), &*LIBRARY_COLLECTION); diff --git a/src/main.rs b/src/main.rs index c3cb2be..b7efbfa 100644 --- a/src/main.rs +++ b/src/main.rs @@ -97,7 +97,7 @@ fn default_filter() -> CollectionFilter { fn with( builder: MusicHoardBuilder, ) { - let mut music_hoard = builder.build().expect("failed to initialise MusicHoard"); + let mut music_hoard = builder.build(); music_hoard.set_filter(default_filter()); // Initialize the terminal user interface. diff --git a/tests/lib.rs b/tests/lib.rs index 72f6417..3ae0b9e 100644 --- a/tests/lib.rs +++ b/tests/lib.rs @@ -31,7 +31,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(database, library).unwrap(); + let mut music_hoard = MusicHoard::new(database, library); music_hoard.rescan_library().unwrap(); music_hoard.reload_database().unwrap(); @@ -54,7 +54,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(database, library).unwrap(); + let mut music_hoard = MusicHoard::new(database, library); music_hoard.reload_database().unwrap(); music_hoard.rescan_library().unwrap(); -- 2.47.1 From c842b1d98d328aed76aff1dbc823e204a3a8db0e Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 5 Jan 2025 10:46:15 +0100 Subject: [PATCH 5/6] Lint --- src/core/musichoard/builder.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/core/musichoard/builder.rs b/src/core/musichoard/builder.rs index fd285d3..46c9f4c 100644 --- a/src/core/musichoard/builder.rs +++ b/src/core/musichoard/builder.rs @@ -141,10 +141,13 @@ impl MusicHoard { #[cfg(test)] mod tests { - use crate::{core::{ - interface::{database::NullDatabase, library::NullLibrary}, - musichoard::library::IMusicHoardLibrary, - }, IMusicHoardDatabase}; + use crate::{ + core::{ + interface::{database::NullDatabase, library::NullLibrary}, + musichoard::library::IMusicHoardLibrary, + }, + IMusicHoardDatabase, + }; use super::*; -- 2.47.1 From 1bcd1ba1e1d3c67369c85485690a867288f08c6f Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 5 Jan 2025 11:01:40 +0100 Subject: [PATCH 6/6] Variable rename --- src/core/musichoard/base.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/core/musichoard/base.rs b/src/core/musichoard/base.rs index c405e9b..e8daf36 100644 --- a/src/core/musichoard/base.rs +++ b/src/core/musichoard/base.rs @@ -70,18 +70,18 @@ impl IMusicHoardBasePrivate for MusicHoard } fn merge_collections>(&self, database: It) -> Collection { - let mut primary_map = NormalMap::::new(); - let mut secondary_map = NormalMap::::new(); + let mut primary = NormalMap::::new(); + let mut secondary = NormalMap::::new(); for artist in self.library_cache.iter().cloned() { - primary_map.insert(string::normalize_string(&artist.meta.id.name), artist); + primary.insert(string::normalize_string(&artist.meta.id.name), artist); } for artist in database.into_iter() { - secondary_map.insert(string::normalize_string(&artist.meta.id.name), artist); + secondary.insert(string::normalize_string(&artist.meta.id.name), artist); } - let mut collection = MergeCollections::merge_by_name(primary_map, secondary_map); + let mut collection = MergeCollections::merge_by_name(primary, secondary); collection.sort_unstable(); collection -- 2.47.1