Fetch panics if a list shortens with a selection beyond the new length #258
@ -6,13 +6,11 @@ use musichoard::collection::{
|
|||||||
musicbrainz::{MbRefOption, Mbid},
|
musicbrainz::{MbRefOption, Mbid},
|
||||||
};
|
};
|
||||||
|
|
||||||
use crate::tui::{
|
use crate::tui::app::{
|
||||||
app::{
|
machine::{fetch_state::FetchState, input::Input, App, AppInner, AppMachine},
|
||||||
machine::{fetch_state::FetchState, input::Input, App, AppInner, AppMachine},
|
selection::KeySelection,
|
||||||
AlbumMatches, AppPublicState, AppState, ArtistMatches, Delta, EntityMatches,
|
AlbumMatches, AppPublicState, AppState, ArtistMatches, Delta, EntityMatches, IAppInteractMatch,
|
||||||
IAppInteractMatch, MatchOption, MatchStatePublic, WidgetState,
|
MatchOption, MatchStatePublic, WidgetState,
|
||||||
},
|
|
||||||
lib::IMusicHoard,
|
|
||||||
};
|
};
|
||||||
|
|
||||||
struct ArtistInfoTuple {
|
struct ArtistInfoTuple {
|
||||||
@ -205,20 +203,21 @@ impl AppMachine<MatchState> {
|
|||||||
}
|
}
|
||||||
|
|
||||||
fn select_artist(
|
fn select_artist(
|
||||||
mh: &mut Box<dyn IMusicHoard>,
|
inner: &mut AppInner,
|
||||||
matches: &ArtistMatches,
|
matches: &ArtistMatches,
|
||||||
tuple: ArtistInfoTuple,
|
tuple: ArtistInfoTuple,
|
||||||
) -> Result<(), musichoard::Error> {
|
) -> Result<(), musichoard::Error> {
|
||||||
|
let mh = &mut inner.music_hoard;
|
||||||
mh.merge_artist_info(&matches.matching.id, tuple.info)?;
|
mh.merge_artist_info(&matches.matching.id, tuple.info)?;
|
||||||
mh.set_artist_mb_ref(&matches.matching.id, tuple.mb_ref)
|
mh.set_artist_mb_ref(&matches.matching.id, tuple.mb_ref)
|
||||||
}
|
}
|
||||||
|
|
||||||
fn select_album(
|
fn select_album(
|
||||||
mh: &mut Box<dyn IMusicHoard>,
|
inner: &mut AppInner,
|
||||||
matches: &AlbumMatches,
|
matches: &AlbumMatches,
|
||||||
tuple: AlbumInfoTuple,
|
tuple: AlbumInfoTuple,
|
||||||
) -> Result<(), musichoard::Error> {
|
) -> Result<(), musichoard::Error> {
|
||||||
let coll = mh.get_collection();
|
let coll = inner.music_hoard.get_collection();
|
||||||
let mut clashing = vec![];
|
let mut clashing = vec![];
|
||||||
if let Some(artist) = coll.iter().find(|artist| artist.meta.id == matches.artist) {
|
if let Some(artist) = coll.iter().find(|artist| artist.meta.id == matches.artist) {
|
||||||
// While we expect only one, there is nothing stopping anybody from having multiple
|
// While we expect only one, there is nothing stopping anybody from having multiple
|
||||||
@ -231,8 +230,16 @@ impl AppMachine<MatchState> {
|
|||||||
}
|
}
|
||||||
|
|
||||||
for album in clashing.into_iter() {
|
for album in clashing.into_iter() {
|
||||||
mh.remove_album(&matches.artist, &album)?;
|
let coll = inner.music_hoard.get_filtered();
|
||||||
|
let selection = KeySelection::get(coll, &inner.selection);
|
||||||
|
|
||||||
|
inner.music_hoard.remove_album(&matches.artist, &album)?;
|
||||||
|
|
||||||
|
let coll = inner.music_hoard.get_filtered();
|
||||||
|
inner.selection.select_by_id(coll, selection);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
let mh = &mut inner.music_hoard;
|
||||||
mh.merge_album_info(&matches.artist, &matches.matching, tuple.info)?;
|
mh.merge_album_info(&matches.artist, &matches.matching, tuple.info)?;
|
||||||
mh.set_album_mb_ref(&matches.artist, &matches.matching, tuple.mb_ref)
|
mh.set_album_mb_ref(&matches.artist, &matches.matching, tuple.mb_ref)
|
||||||
}
|
}
|
||||||
@ -279,14 +286,14 @@ impl IAppInteractMatch for AppMachine<MatchState> {
|
|||||||
fn select(mut self) -> Self::APP {
|
fn select(mut self) -> Self::APP {
|
||||||
let index = self.state.state.list.selected().unwrap();
|
let index = self.state.state.list.selected().unwrap();
|
||||||
|
|
||||||
let mh = &mut self.inner.music_hoard;
|
let inner = &mut self.inner;
|
||||||
let result = match self.state.current {
|
let result = match self.state.current {
|
||||||
EntityMatches::Artist(ref mut matches) => match matches.list.extract_info(index) {
|
EntityMatches::Artist(ref mut matches) => match matches.list.extract_info(index) {
|
||||||
InfoOption::Info(tuple) => Self::select_artist(mh, matches, tuple),
|
InfoOption::Info(tuple) => Self::select_artist(inner, matches, tuple),
|
||||||
InfoOption::NeedInput => return self.get_input(),
|
InfoOption::NeedInput => return self.get_input(),
|
||||||
},
|
},
|
||||||
EntityMatches::Album(ref mut matches) => match matches.list.extract_info(index) {
|
EntityMatches::Album(ref mut matches) => match matches.list.extract_info(index) {
|
||||||
InfoOption::Info(tuple) => Self::select_album(mh, matches, tuple),
|
InfoOption::Info(tuple) => Self::select_album(inner, matches, tuple),
|
||||||
InfoOption::NeedInput => return self.get_input(),
|
InfoOption::NeedInput => return self.get_input(),
|
||||||
},
|
},
|
||||||
};
|
};
|
||||||
@ -323,9 +330,12 @@ mod tests {
|
|||||||
machine::tests::{inner, inner_with_mb, input_event, music_hoard},
|
machine::tests::{inner, inner_with_mb, input_event, music_hoard},
|
||||||
IApp, IAppAccess, IAppInput,
|
IApp, IAppAccess, IAppInput,
|
||||||
},
|
},
|
||||||
lib::interface::musicbrainz::{
|
lib::{
|
||||||
api::Entity,
|
interface::musicbrainz::{
|
||||||
daemon::{MbParams, MockIMbJobSender},
|
api::Entity,
|
||||||
|
daemon::{MbParams, MockIMbJobSender},
|
||||||
|
},
|
||||||
|
MockIMusicHoard,
|
||||||
},
|
},
|
||||||
};
|
};
|
||||||
|
|
||||||
@ -653,14 +663,34 @@ 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 = AlbumMbRef::Some(mbid().into());
|
||||||
let removed = Album::new(AlbumId::new("Not the Same").with_mb_ref(mb_ref));
|
let removed = Album::new(AlbumId::new("Album: Not the Same").with_mb_ref(mb_ref.clone()));
|
||||||
artist.albums.push(removed.clone());
|
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 collection = vec![artist];
|
||||||
let mut music_hoard = music_hoard(collection.clone());
|
let mut reduced = collection.clone();
|
||||||
|
reduced[0].albums.retain(|a| a.meta.id.mb_ref != mb_ref);
|
||||||
|
|
||||||
|
// We need a custom music_hoard to override the get_filtered expectations.
|
||||||
|
let mut music_hoard = MockIMusicHoard::new();
|
||||||
|
let mut seq = Sequence::new();
|
||||||
|
music_hoard
|
||||||
|
.expect_get_filtered()
|
||||||
|
.times(2)
|
||||||
|
.in_sequence(&mut seq)
|
||||||
|
.return_const(collection.clone());
|
||||||
|
music_hoard
|
||||||
|
.expect_get_filtered()
|
||||||
|
.times(1)
|
||||||
|
.in_sequence(&mut seq)
|
||||||
|
.return_const(reduced.clone());
|
||||||
|
|
||||||
match matches_info {
|
match matches_info {
|
||||||
EntityMatches::Artist(_) => panic!(),
|
EntityMatches::Artist(_) => panic!(),
|
||||||
EntityMatches::Album(matches) => {
|
EntityMatches::Album(matches) => {
|
||||||
@ -671,7 +701,7 @@ mod tests {
|
|||||||
.expect_get_collection()
|
.expect_get_collection()
|
||||||
.times(1)
|
.times(1)
|
||||||
.in_sequence(&mut seq)
|
.in_sequence(&mut seq)
|
||||||
.return_const(collection);
|
.return_const(collection.clone());
|
||||||
music_hoard
|
music_hoard
|
||||||
.expect_remove_album()
|
.expect_remove_album()
|
||||||
.times(1)
|
.times(1)
|
||||||
@ -691,8 +721,26 @@ mod tests {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
let matches = AppMachine::match_state(inner(music_hoard), app_matches);
|
let mut matches = AppMachine::match_state(inner(music_hoard), app_matches);
|
||||||
matches.select().unwrap_fetch();
|
|
||||||
|
// Make sure the last album is selected.
|
||||||
|
let selection = &mut matches.inner.selection;
|
||||||
|
selection.increment_category();
|
||||||
|
selection.increment_selection(&collection, Delta::Line);
|
||||||
|
selection.increment_selection(&collection, Delta::Line);
|
||||||
|
|
||||||
|
// 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);
|
||||||
|
|
||||||
|
let mut fetch = matches.select().unwrap_fetch();
|
||||||
|
|
||||||
|
// Check that the correct album is still selected in the reduced collection.
|
||||||
|
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);
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
|
Loading…
x
Reference in New Issue
Block a user