wojtek/musichoard
1
0
Fork 0
Code Issues 17 Pull Requests 1 Actions Packages Projects 1 Releases 4 Wiki Activity

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
Conversation 0 Commits 3 Files Changed 1 +105 -73
1 changed files with 105 additions and 73 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

178
src/tui/app/machine/match_state.rs
View File

@ -212,6 +212,10 @@ impl AppMachine<MatchState> {
mh.set_artist_mb_ref(&matches.matching.id, tuple.mb_ref)
}
fn filter_mb_ref(left: &AlbumMbRef, right: &AlbumMbRef) -> bool {
(left == right) && !matches!(left, AlbumMbRef::CannotHaveMbid)
}
fn select_album(
inner: &mut AppInner,
matches: &AlbumMatches,
@ -225,7 +229,7 @@ impl AppMachine<MatchState> {
let iter = artist.albums.iter();
let filtered = iter
.filter(|album| album.meta.id != matches.matching)
.filter(|album| album.meta.id.mb_ref == tuple.mb_ref);
.filter(|album| Self::filter_mb_ref(&album.meta.id.mb_ref, &tuple.mb_ref));
clashing.extend(filtered.map(|a| &a.meta.id).cloned())
}
@ -323,6 +327,7 @@ mod tests {
Album, AlbumDate, AlbumId, AlbumInfo, AlbumMeta, AlbumPrimaryType, AlbumSecondaryType,
},
artist::{Artist, ArtistId, ArtistMeta},
Collection,
};
use crate::tui::{
@ -604,6 +609,42 @@ mod tests {
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]
fn set_album_info() {
let matches_info = album_match();
@ -612,45 +653,22 @@ mod tests {
let app_matches = MatchState::new(matches_info.clone(), FetchState::search(rx));
let collection = vec![];
let mut music_hoard = music_hoard(collection.clone());
let mut mh = music_hoard(collection.clone());
match matches_info {
EntityMatches::Artist(_) => panic!(),
EntityMatches::Album(matches) => {
let mut album_id = album_id();
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(()));
set_album_info_expectations(&mut mh, collection, matches, album_id(), None)
}
}
let matches = AppMachine::match_state(inner(music_hoard), app_matches);
let matches = AppMachine::match_state(inner(mh), app_matches);
matches.select().unwrap_fetch();
}
#[test]
fn set_album_info_with_clashing_mbid() {
let matches_info = album_match();
fn albums_with_clashing_mbid(
clashing_id: AlbumId,
last_id: AlbumId,
) -> (Collection, Collection) {
// The collection below should trigger MH to remove the album with the same MBID but not
// matching album_id.
@ -662,66 +680,56 @@ mod tests {
// (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 mb_ref = AlbumMbRef::Some(mbid().into());
let removed = Album::new(AlbumId::new("Album: Not the Same").with_mb_ref(mb_ref.clone()));
artist.albums.push(removed.clone());
let mb_ref = clashing_id.mb_ref.clone();
artist.albums.push(Album::new(clashing_id));
// (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());
artist.albums.push(Album::new(last_id));
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 app_matches = MatchState::new(matches_info.clone(), FetchState::search(rx));
let collection = vec![artist];
let mut reduced = collection.clone();
reduced[0].albums.retain(|a| a.meta.id.mb_ref != mb_ref);
let mb_ref = AlbumMbRef::Some(mbid().into());
let clashing = AlbumId::new("Album: Not the Same").with_mb_ref(mb_ref.clone());
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.
let mut music_hoard = MockIMusicHoard::new();
let mut mh = MockIMusicHoard::new();
let mut seq = Sequence::new();
music_hoard
.expect_get_filtered()
mh.expect_get_filtered()
.times(2)
.in_sequence(&mut seq)
.return_const(collection.clone());
music_hoard
.expect_get_filtered()
mh.expect_get_filtered()
.times(1)
.in_sequence(&mut seq)
.return_const(reduced.clone());
match matches_info {
EntityMatches::Artist(_) => panic!(),
EntityMatches::Album(matches) => {
let artist = matches.artist.clone();
let mut seq = Sequence::new();
music_hoard
.expect_get_collection()
.times(1)
.in_sequence(&mut seq)
.return_const(collection.clone());
music_hoard
.expect_remove_album()
.times(1)
.with(eq(artist), eq(removed.meta.id.clone()))
.in_sequence(&mut seq)
.return_once(|_, _| Ok(()));
music_hoard
.expect_merge_album_info()
.times(1)
.in_sequence(&mut seq)
.return_once(|_, _, _| Ok(()));
music_hoard
.expect_set_album_mb_ref()
.times(1)
.in_sequence(&mut seq)
.return_once(|_, _, _| Ok(()));
}
EntityMatches::Album(matches) => set_album_info_expectations(
&mut mh,
collection.clone(),
matches,
album_id(),
Some(clashing.clone()),
),
}
let mut matches = AppMachine::match_state(inner(music_hoard), app_matches);
let mut matches = AppMachine::match_state(inner(mh), app_matches);
// Make sure the last album is selected.
let selection = &mut matches.inner.selection;
@ -732,7 +740,7 @@ mod tests {
// Check that the correct album is selected.
let state = selection.state_album(&collection).unwrap();
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();
@ -740,7 +748,31 @@ mod tests {
let selection = &mut fetch.inner.selection;
let state = selection.state_album(&reduced).unwrap();
let album = &reduced[0].albums[state.index];
assert_eq!(album.meta, selected.meta);
assert_eq!(album.meta.id, selected);
}
#[test]
fn set_album_info_with_clashing_cannot_have_mbid() {
let matches_info = album_match();
let (_tx, rx) = mpsc::channel();
let app_matches = MatchState::new(matches_info.clone(), FetchState::search(rx));
let mb_ref = AlbumMbRef::CannotHaveMbid;
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 {
EntityMatches::Artist(_) => panic!(),
EntityMatches::Album(matches) => {
set_album_info_expectations(&mut mh, collection, matches, album_id(), None)
}
}
let matches = AppMachine::match_state(inner(mh), app_matches);
matches.select().unwrap_fetch();
}
#[test]