From a705903b9393e4fa8b801e2f343a8478e4516922 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Thu, 2 Jan 2025 11:20:18 +0100 Subject: [PATCH] Complete unit tests for artist --- src/core/collection/artist.rs | 125 +++++++++++++++++++++++----------- 1 file changed, 85 insertions(+), 40 deletions(-) diff --git a/src/core/collection/artist.rs b/src/core/collection/artist.rs index 239089c..1c0e332 100644 --- a/src/core/collection/artist.rs +++ b/src/core/collection/artist.rs @@ -56,6 +56,64 @@ impl Artist { albums: vec![], } } + + fn merge_album_with_lib_id( + primary_albums: &mut Vec, + mut secondary_album: Album, + ) -> Option { + if let lib_id @ AlbumLibId::Some(_) | lib_id @ AlbumLibId::Singleton = + secondary_album.meta.id.lib_id + { + if let Some(ref mut primary_album) = primary_albums + .iter_mut() + .find(|album| album.meta.id.lib_id == lib_id) + { + primary_album.merge_in_place(secondary_album); + return None; + } + + secondary_album.meta.id.lib_id = AlbumLibId::None; + } + + Some(secondary_album) + } + + fn merge_albums_with_lib_id( + primary_albums: &mut Vec, + mut secondary_albums: Vec, + ) -> HashMap> { + let mut secondary_without_id = HashMap::>::new(); + for secondary_album in secondary_albums.drain(..) { + if let Some(secondary_album) = + Artist::merge_album_with_lib_id(primary_albums, secondary_album) + { + secondary_without_id + .entry(secondary_album.meta.id.title.clone()) + .or_default() + .push(secondary_album); + } + } + secondary_without_id + } + + fn merge_albums_with_title( + primary_albums: &mut Vec, + mut secondary_without_id: HashMap>, + ) { + for (title, mut secondary_albums) in secondary_without_id.drain() { + match primary_albums + .iter_mut() + .find(|album| album.meta.id.title == title) + { + Some(ref mut primary_album) => { + // We do not support merging multiple DB albums with same title yet. + assert_eq!(secondary_albums.len(), 1); + primary_album.merge_in_place(secondary_albums.pop().unwrap()) + } + None => primary_albums.append(&mut secondary_albums), + } + } + } } impl PartialOrd for Artist { @@ -75,45 +133,11 @@ impl Merge for Artist { self.meta.merge_in_place(other.meta); let mut primary_albums = mem::take(&mut self.albums); - let mut secondary_albums = other.albums; + let secondary_albums = other.albums; - let mut secondary_without_id = HashMap::>::new(); - for mut secondary_album in secondary_albums.drain(..) { - match secondary_album.meta.id.lib_id { - lib_id @ AlbumLibId::Some(_) | lib_id @ AlbumLibId::Singleton => { - match primary_albums - .iter_mut() - .find(|album| album.meta.id.lib_id == lib_id) - { - Some(ref mut primary_album) => { - primary_album.merge_in_place(secondary_album) - } - None => { - secondary_album.meta.id.lib_id = AlbumLibId::None; - primary_albums.push(secondary_album); - } - } - } - AlbumLibId::None => secondary_without_id - .entry(secondary_album.meta.id.title.clone()) - .or_default() - .push(secondary_album), - } - } - - for (title, mut secondary_albums) in secondary_without_id.drain() { - match primary_albums - .iter_mut() - .find(|album| album.meta.id.title == title) - { - Some(ref mut primary_album) => { - // We do not support merging multiple DB albums with same title yet. - assert_eq!(secondary_albums.len(), 1); - primary_album.merge_in_place(secondary_albums.pop().unwrap()) - } - None => primary_albums.append(&mut secondary_albums), - } - } + let secondary_without_id = + Artist::merge_albums_with_lib_id(&mut primary_albums, secondary_albums); + Artist::merge_albums_with_title(&mut primary_albums, secondary_without_id); primary_albums.sort_unstable(); self.albums = primary_albums; @@ -454,6 +478,12 @@ mod tests { assert!(info.properties.is_empty()); } + fn reset_lib_id(albums: &mut [Album]) { + for album in albums.iter_mut() { + album.meta.id.lib_id = AlbumLibId::None; + } + } + #[test] fn merge_artist_no_overlap() { let left = FULL_COLLECTION[0].to_owned(); @@ -468,13 +498,25 @@ mod tests { .info .properties .merge(right.clone().meta.info.properties); - expected.albums.append(&mut right.albums.clone()); + + // If an album in the secondary (right) collection has a lib id but there is no match in the + // primary (left) collection then it is removed. + let mut right_albums_without_lib_id = right.albums.clone(); + reset_lib_id(&mut right_albums_without_lib_id); + expected.albums.extend(right_albums_without_lib_id); expected.albums.sort_unstable(); let merged = left.clone().merge(right.clone()); assert_eq!(expected, merged); - // Non-overlapping merge should be commutative. + // Non-overlapping merge should be commutative. Except for lib id which will be erased from + // the albums in the secondary collection. + expected.albums = right.albums.clone(); + let mut left_albums_without_lib_id = left.albums.clone(); + reset_lib_id(&mut left_albums_without_lib_id); + expected.albums.extend(left_albums_without_lib_id); + expected.albums.sort_unstable(); + let merged = right.clone().merge(left.clone()); assert_eq!(expected, merged); } @@ -487,6 +529,9 @@ mod tests { left.albums.push(right.albums[0].clone()); left.albums.sort_unstable(); + // Albums on right without a match on the left will lose their lib id. + reset_lib_id(&mut right.albums[1..]); + let mut expected = left.clone(); expected.meta.info.properties = expected .meta