Fetching and then instantly reloading library loses data #256

Merged
wojtek merged 6 commits from 254---fetching-and-then-instantly-reloading-library-loses-data into main 2025-01-05 11:06:05 +01:00
5 changed files with 40 additions and 39 deletions
Showing only changes of commit d2e3fd4cd8 - Show all commits

View File

@ -33,7 +33,7 @@ pub trait IMusicHoardBasePrivate {
fn sort_artists(collection: &mut [Artist]); fn sort_artists(collection: &mut [Artist]);
fn sort_albums_and_tracks<'a, C: Iterator<Item = &'a mut Artist>>(collection: C); fn sort_albums_and_tracks<'a, C: Iterator<Item = &'a mut Artist>>(collection: C);
fn merge_collections(&self) -> Collection; fn merge_collections<It: IntoIterator<Item = Artist>>(&self, database: It) -> Collection;
fn filter_collection(&self) -> Collection; fn filter_collection(&self) -> Collection;
fn filter_artist(&self, artist: &Artist) -> Option<Artist>; fn filter_artist(&self, artist: &Artist) -> Option<Artist>;
@ -69,19 +69,19 @@ impl<Database, Library> IMusicHoardBasePrivate for MusicHoard<Database, Library>
} }
} }
fn merge_collections(&self) -> Collection { fn merge_collections<It: IntoIterator<Item = Artist>>(&self, database: It) -> Collection {
let mut primary = NormalMap::<Artist>::new(); let mut primary_map = NormalMap::<Artist>::new();
let mut secondary = NormalMap::<Artist>::new(); let mut secondary_map = NormalMap::<Artist>::new();
for artist in self.library_cache.iter().cloned() { 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() { for artist in database.into_iter() {
secondary.insert(string::normalize_string(&artist.meta.id.name), artist); 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.sort_unstable();
collection collection
@ -168,21 +168,19 @@ mod tests {
let mut mh = MusicHoard { let mut mh = MusicHoard {
library_cache: left.clone(), library_cache: left.clone(),
database_cache: right.clone(),
..Default::default() ..Default::default()
}; };
mh.collection = mh.merge_collections(); mh.collection = mh.merge_collections(right.clone());
assert_eq!(expected, mh.collection); assert_eq!(expected, mh.collection);
// The merge is completely non-overlapping so it should be commutative. // The merge is completely non-overlapping so it should be commutative.
let mut mh = MusicHoard { let mut mh = MusicHoard {
library_cache: right.clone(), library_cache: right.clone(),
database_cache: left.clone(),
..Default::default() ..Default::default()
}; };
mh.collection = mh.merge_collections(); mh.collection = mh.merge_collections(left.clone());
assert_eq!(expected, mh.collection); assert_eq!(expected, mh.collection);
} }
@ -198,21 +196,19 @@ mod tests {
let mut mh = MusicHoard { let mut mh = MusicHoard {
library_cache: left.clone(), library_cache: left.clone(),
database_cache: right.clone(),
..Default::default() ..Default::default()
}; };
mh.collection = mh.merge_collections(); mh.collection = mh.merge_collections(right.clone());
assert_eq!(expected, mh.collection); assert_eq!(expected, mh.collection);
// The merge does not overwrite any data so it should be commutative. // The merge does not overwrite any data so it should be commutative.
let mut mh = MusicHoard { let mut mh = MusicHoard {
library_cache: right.clone(), library_cache: right.clone(),
database_cache: left.clone(),
..Default::default() ..Default::default()
}; };
mh.collection = mh.merge_collections(); mh.collection = mh.merge_collections(left.clone());
assert_eq!(expected, mh.collection); assert_eq!(expected, mh.collection);
} }
@ -241,21 +237,19 @@ mod tests {
let mut mh = MusicHoard { let mut mh = MusicHoard {
library_cache: left.clone(), library_cache: left.clone(),
database_cache: right.clone(),
..Default::default() ..Default::default()
}; };
mh.collection = mh.merge_collections(); mh.collection = mh.merge_collections(right.clone());
assert_eq!(expected, mh.collection); assert_eq!(expected, mh.collection);
// The merge overwrites the sort data, but no data is erased so it should be commutative. // The merge overwrites the sort data, but no data is erased so it should be commutative.
let mut mh = MusicHoard { let mut mh = MusicHoard {
library_cache: right.clone(), library_cache: right.clone(),
database_cache: left.clone(),
..Default::default() ..Default::default()
}; };
mh.collection = mh.merge_collections(); mh.collection = mh.merge_collections(left.clone());
assert_eq!(expected, mh.collection); assert_eq!(expected, mh.collection);
} }
@ -273,11 +267,10 @@ mod tests {
let mut mh = MusicHoard { let mut mh = MusicHoard {
library_cache: left.clone(), library_cache: left.clone(),
database_cache: right.clone(),
..Default::default() ..Default::default()
}; };
mh.collection = mh.merge_collections(); mh.collection = mh.merge_collections(right.clone());
} }
#[test] #[test]
@ -294,11 +287,10 @@ mod tests {
let mut mh = MusicHoard { let mut mh = MusicHoard {
library_cache: left.clone(), library_cache: left.clone(),
database_cache: right.clone(),
..Default::default() ..Default::default()
}; };
mh.collection = mh.merge_collections(); mh.collection = mh.merge_collections(right.clone());
} }
#[test] #[test]
@ -318,11 +310,10 @@ mod tests {
let mut mh = MusicHoard { let mut mh = MusicHoard {
library_cache: left.clone(), library_cache: left.clone(),
database_cache: right.clone(),
..Default::default() ..Default::default()
}; };
mh.collection = mh.merge_collections(); mh.collection = mh.merge_collections(right.clone());
assert_eq!(expected, mh.collection); assert_eq!(expected, mh.collection);
} }

View File

@ -69,7 +69,6 @@ impl MusicHoard<NoDatabase, NoLibrary> {
collection: vec![], collection: vec![],
pre_commit: vec![], pre_commit: vec![],
database: NoDatabase, database: NoDatabase,
database_cache: vec![],
library: NoLibrary, library: NoLibrary,
library_cache: vec![], library_cache: vec![],
} }
@ -92,7 +91,6 @@ impl<Library: ILibrary> MusicHoard<NoDatabase, Library> {
collection: vec![], collection: vec![],
pre_commit: vec![], pre_commit: vec![],
database: NoDatabase, database: NoDatabase,
database_cache: vec![],
library, library,
library_cache: vec![], library_cache: vec![],
} }
@ -115,7 +113,6 @@ impl<Database: IDatabase> MusicHoard<Database, NoLibrary> {
collection: vec![], collection: vec![],
pre_commit: vec![], pre_commit: vec![],
database, database,
database_cache: vec![],
library: NoLibrary, library: NoLibrary,
library_cache: vec![], library_cache: vec![],
}; };
@ -140,7 +137,6 @@ impl<Database: IDatabase, Library: ILibrary> MusicHoard<Database, Library> {
collection: vec![], collection: vec![],
pre_commit: vec![], pre_commit: vec![],
database, database,
database_cache: vec![],
library, library,
library_cache: vec![], library_cache: vec![],
}; };

View File

@ -116,10 +116,10 @@ pub trait IMusicHoardDatabase {
impl<Database: IDatabase, Library> IMusicHoardDatabase for MusicHoard<Database, Library> { impl<Database: IDatabase, Library> IMusicHoardDatabase for MusicHoard<Database, Library> {
fn reload_database(&mut self) -> Result<(), Error> { fn reload_database(&mut self) -> Result<(), Error> {
self.database_cache = self.database.load()?; let mut database_cache = self.database.load()?;
Self::sort_albums_and_tracks(self.database_cache.iter_mut()); 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.filtered = self.filter_collection();
self.pre_commit = self.collection.clone(); self.pre_commit = self.collection.clone();

View File

@ -23,25 +23,28 @@ pub trait IMusicHoardLibrary {
impl<Library: ILibrary> IMusicHoardLibrary for MusicHoard<NoDatabase, Library> { impl<Library: ILibrary> IMusicHoardLibrary for MusicHoard<NoDatabase, Library> {
fn rescan_library(&mut self) -> Result<(), Error> { fn rescan_library(&mut self) -> Result<(), Error> {
self.pre_commit = self.rescan_library_inner()?; self.pre_commit = self.rescan_library_inner(vec![])?;
self.commit() self.commit()
} }
} }
impl<Database: IDatabase, Library: ILibrary> IMusicHoardLibrary for MusicHoard<Database, Library> { impl<Database: IDatabase, Library: ILibrary> IMusicHoardLibrary for MusicHoard<Database, Library> {
fn rescan_library(&mut self) -> Result<(), Error> { 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() self.commit()
} }
} }
impl<Database, Library: ILibrary> MusicHoard<Database, Library> { impl<Database, Library: ILibrary> MusicHoard<Database, Library> {
fn rescan_library_inner(&mut self) -> Result<Collection, Error> { fn rescan_library_inner(&mut self, database: Collection) -> Result<Collection, Error> {
let items = self.library.list(&Query::new())?; let items = self.library.list(&Query::new())?;
self.library_cache = Self::items_to_artists(items)?; self.library_cache = Self::items_to_artists(items)?;
Self::sort_albums_and_tracks(self.library_cache.iter_mut()); 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<Item>) -> Result<Collection, Error> { fn items_to_artists(items: Vec<Item>) -> Result<Collection, Error> {
@ -152,11 +155,23 @@ mod tests {
.times(1) .times(1)
.return_once(|_| library_result); .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 database
.expect_save() .expect_save()
.with(predicate::eq(&*LIBRARY_COLLECTION)) .with(predicate::eq(&*LIBRARY_COLLECTION))
.times(1) .times(1)
.in_sequence(&mut seq)
.return_once(|_| Ok(())); .return_once(|_| Ok(()));
let mut music_hoard = MusicHoard::new(database, library).unwrap(); let mut music_hoard = MusicHoard::new(database, library).unwrap();

View File

@ -32,7 +32,6 @@ pub struct MusicHoard<Database, Library> {
collection: Collection, collection: Collection,
pre_commit: Collection, pre_commit: Collection,
database: Database, database: Database,
database_cache: Collection,
library: Library, library: Library,
library_cache: Collection, library_cache: Collection,
} }