Fetch loses albums with CannotHaveMbid as mb_ref #259

Merged
wojtek merged 3 commits from 255---fetch-loses-albums-with-cannothavembid-as-mb_ref into main 2025-01-05 14:55:36 +01:00
Showing only changes of commit 8b2d7fb07a - Show all commits

View File

@ -327,6 +327,7 @@ mod tests {
Album, AlbumDate, AlbumId, AlbumInfo, AlbumMeta, AlbumPrimaryType, AlbumSecondaryType, Album, AlbumDate, AlbumId, AlbumInfo, AlbumMeta, AlbumPrimaryType, AlbumSecondaryType,
}, },
artist::{Artist, ArtistId, ArtistMeta}, artist::{Artist, ArtistId, ArtistMeta},
Collection,
}; };
use crate::tui::{ use crate::tui::{
@ -608,6 +609,42 @@ mod tests {
matches.select().unwrap_fetch(); matches.select().unwrap_fetch();
} }
fn set_album_info_expectations(
mh: &mut MockIMusicHoard,
collection: Collection,
matches: AlbumMatches,
mut album_id: AlbumId,
removed_id: Option<AlbumId>,
) {
let meta = album_meta(album_id.clone());
let mb_ref = album_id.mb_ref.clone();
album_id.clear_mb_ref();
let artist = matches.artist.clone();
let mut seq = Sequence::new();
mh.expect_get_collection()
.times(1)
.in_sequence(&mut seq)
.return_const(collection);
if let Some(removed_id) = removed_id {
mh.expect_remove_album()
.times(1)
.with(eq(artist.clone()), eq(removed_id))
.in_sequence(&mut seq)
.return_once(|_, _| Ok(()));
}
mh.expect_merge_album_info()
.with(eq(artist.clone()), eq(album_id.clone()), eq(meta.info))
.times(1)
.in_sequence(&mut seq)
.return_once(|_, _, _| Ok(()));
mh.expect_set_album_mb_ref()
.with(eq(artist.clone()), eq(album_id.clone()), eq(mb_ref))
.times(1)
.in_sequence(&mut seq)
.return_once(|_, _, _| Ok(()));
}
#[test] #[test]
fn set_album_info() { fn set_album_info() {
let matches_info = album_match(); let matches_info = album_match();
@ -616,45 +653,22 @@ mod tests {
let app_matches = MatchState::new(matches_info.clone(), FetchState::search(rx)); let app_matches = MatchState::new(matches_info.clone(), FetchState::search(rx));
let collection = vec![]; let collection = vec![];
let mut music_hoard = music_hoard(collection.clone()); let mut mh = music_hoard(collection.clone());
match matches_info { match matches_info {
EntityMatches::Artist(_) => panic!(), EntityMatches::Artist(_) => panic!(),
EntityMatches::Album(matches) => { EntityMatches::Album(matches) => {
let mut album_id = album_id(); set_album_info_expectations(&mut mh, collection, matches, album_id(), None)
let meta = album_meta(album_id.clone());
let mb_ref = album_id.mb_ref.clone();
album_id.clear_mb_ref();
let artist = matches.artist.clone();
let mut seq = Sequence::new();
music_hoard
.expect_get_collection()
.times(1)
.in_sequence(&mut seq)
.return_const(collection);
music_hoard
.expect_merge_album_info()
.with(eq(artist.clone()), eq(album_id.clone()), eq(meta.info))
.times(1)
.in_sequence(&mut seq)
.return_once(|_, _, _| Ok(()));
music_hoard
.expect_set_album_mb_ref()
.with(eq(artist.clone()), eq(album_id.clone()), eq(mb_ref))
.times(1)
.in_sequence(&mut seq)
.return_once(|_, _, _| Ok(()));
} }
} }
let matches = AppMachine::match_state(inner(music_hoard), app_matches); let matches = AppMachine::match_state(inner(mh), app_matches);
matches.select().unwrap_fetch(); matches.select().unwrap_fetch();
} }
#[test] fn albums_with_clashing_mbid(
fn set_album_info_with_clashing_mbid() { clashing_id: AlbumId,
let matches_info = album_match(); last_id: AlbumId,
) -> (Collection, Collection) {
// The collection below should trigger MH to remove the album with the same MBID but not // The collection below should trigger MH to remove the album with the same MBID but not
// matching album_id. // matching album_id.
@ -666,20 +680,31 @@ mod tests {
// (3) An album with a different album_id than the selected one. // (3) An album with a different album_id than the selected one.
// (4) This album has an MBID that is identical to that of the selected match. // (4) This album has an MBID that is identical to that of the selected match.
let mb_ref = AlbumMbRef::Some(mbid().into()); let mb_ref = clashing_id.mb_ref.clone();
let removed = Album::new(AlbumId::new("Album: Not the Same").with_mb_ref(mb_ref.clone())); artist.albums.push(Album::new(clashing_id));
artist.albums.push(removed.clone());
// (5) An album after the one that will be removed. Selection must remain on it. // (5) An album after the one that will be removed. Selection must remain on it.
let selected = Album::new(AlbumId::new("Album: Z")); artist.albums.push(Album::new(last_id));
artist.albums.push(selected.clone());
let collection = vec![artist];
let mut reduced = collection.clone();
reduced[0].albums.retain(|a| a.meta.id.mb_ref != mb_ref);
(collection, reduced)
}
#[test]
fn set_album_info_with_clashing_mbid() {
let matches_info = album_match();
let (_tx, rx) = mpsc::channel(); let (_tx, rx) = mpsc::channel();
let app_matches = MatchState::new(matches_info.clone(), FetchState::search(rx)); let app_matches = MatchState::new(matches_info.clone(), FetchState::search(rx));
let collection = vec![artist]; let mb_ref = AlbumMbRef::Some(mbid().into());
let mut reduced = collection.clone(); let clashing = AlbumId::new("Album: Not the Same").with_mb_ref(mb_ref.clone());
reduced[0].albums.retain(|a| a.meta.id.mb_ref != mb_ref); let selected = AlbumId::new("Album: Z");
let (collection, reduced) = albums_with_clashing_mbid(clashing.clone(), selected.clone());
// We need a custom music_hoard to override the get_filtered expectations. // We need a custom music_hoard to override the get_filtered expectations.
let mut mh = MockIMusicHoard::new(); let mut mh = MockIMusicHoard::new();
@ -695,28 +720,13 @@ mod tests {
match matches_info { match matches_info {
EntityMatches::Artist(_) => panic!(), EntityMatches::Artist(_) => panic!(),
EntityMatches::Album(matches) => { EntityMatches::Album(matches) => set_album_info_expectations(
let artist = matches.artist.clone(); &mut mh,
collection.clone(),
let mut seq = Sequence::new(); matches,
mh.expect_get_collection() album_id(),
.times(1) Some(clashing.clone()),
.in_sequence(&mut seq) ),
.return_const(collection.clone());
mh.expect_remove_album()
.times(1)
.with(eq(artist), eq(removed.meta.id.clone()))
.in_sequence(&mut seq)
.return_once(|_, _| Ok(()));
mh.expect_merge_album_info()
.times(1)
.in_sequence(&mut seq)
.return_once(|_, _, _| Ok(()));
mh.expect_set_album_mb_ref()
.times(1)
.in_sequence(&mut seq)
.return_once(|_, _, _| Ok(()));
}
} }
let mut matches = AppMachine::match_state(inner(mh), app_matches); let mut matches = AppMachine::match_state(inner(mh), app_matches);
@ -730,7 +740,7 @@ mod tests {
// Check that the correct album is selected. // Check that the correct album is selected.
let state = selection.state_album(&collection).unwrap(); let state = selection.state_album(&collection).unwrap();
let album = &collection[0].albums[state.index]; let album = &collection[0].albums[state.index];
assert_eq!(album.meta, selected.meta); assert_eq!(album.meta.id, selected);
let mut fetch = matches.select().unwrap_fetch(); let mut fetch = matches.select().unwrap_fetch();
@ -738,54 +748,26 @@ mod tests {
let selection = &mut fetch.inner.selection; let selection = &mut fetch.inner.selection;
let state = selection.state_album(&reduced).unwrap(); let state = selection.state_album(&reduced).unwrap();
let album = &reduced[0].albums[state.index]; let album = &reduced[0].albums[state.index];
assert_eq!(album.meta, selected.meta); assert_eq!(album.meta.id, selected);
} }
#[test] #[test]
fn set_album_info_with_clashing_cannot_have_mbid() { fn set_album_info_with_clashing_cannot_have_mbid() {
let matches_info = album_match(); let matches_info = album_match();
// The collection below should trigger MH to remove the album with the same MBID but not
// matching album_id.
// (1) Same artist as matches_info.
let mut artist = Artist::new(ArtistId::new("Artist"));
// (2) An album with the same album_id as the selected one.
artist.albums.push(Album::new(AlbumId::new("Album")));
// (3) An album with a different album_id than the selected one.
// (4) This album has an MBID that is identical to that of the selected match.
let removed =
Album::new(AlbumId::new("Album: Not the Same").with_mb_ref(AlbumMbRef::CannotHaveMbid));
artist.albums.push(removed.clone());
// (5) An album after the one that will be removed. Selection must remain on it.
let selected = Album::new(AlbumId::new("Album: Z"));
artist.albums.push(selected.clone());
let (_tx, rx) = mpsc::channel(); let (_tx, rx) = mpsc::channel();
let app_matches = MatchState::new(matches_info.clone(), FetchState::search(rx)); let app_matches = MatchState::new(matches_info.clone(), FetchState::search(rx));
let collection = vec![artist]; let mb_ref = AlbumMbRef::CannotHaveMbid;
let mut mh = music_hoard(collection.clone()); let clashing = AlbumId::new("Album: Not the Same").with_mb_ref(mb_ref);
let selected = AlbumId::new("Album: Z");
let (collection, _) = albums_with_clashing_mbid(clashing.clone(), selected.clone());
let mut mh = music_hoard(collection.clone());
match matches_info { match matches_info {
EntityMatches::Artist(_) => panic!(), EntityMatches::Artist(_) => panic!(),
EntityMatches::Album(_) => { EntityMatches::Album(matches) => {
let mut seq = Sequence::new(); set_album_info_expectations(&mut mh, collection, matches, album_id(), None)
mh.expect_get_collection()
.times(1)
.in_sequence(&mut seq)
.return_const(collection.clone());
mh.expect_merge_album_info()
.times(1)
.in_sequence(&mut seq)
.return_once(|_, _, _| Ok(()));
mh.expect_set_album_mb_ref()
.times(1)
.in_sequence(&mut seq)
.return_once(|_, _, _| Ok(()));
} }
} }