Use a queue to communicate matches from browse to matches (#207)

Closes #202 Reviewed-on: #207
2024-08-31 14:42:46 +02:00 · 2024-08-31 14:42:46 +02:00 · 6333b7a131
commit 6333b7a131
parent cda1487734
4 changed files with 108 additions and 145 deletions
--- a/src/tui/app/machine/browse.rs
+++ b/src/tui/app/machine/browse.rs
@ -1,4 +1,4 @@
-use std::{thread, time};
+use std::{sync::mpsc, thread, time};

 use musichoard::collection::musicbrainz::IMusicBrainzRef;

@ -91,7 +91,7 @@ impl IAppInteractBrowse for AppMachine<AppBrowse> {
            }
        };

-        let mut matches = vec![];
+        let (matches_tx, matches_rx) = mpsc::channel::<AppMatchesInfo>();

        match artist.musicbrainz {
            Some(ref mbid) => {
@ -104,7 +104,9 @@ impl IAppInteractBrowse for AppMachine<AppBrowse> {
                    }

                    match self.inner.musicbrainz.search_release_group(arid, album) {
-                        Ok(list) => matches.push(AppMatchesInfo::album(album.clone(), list)),
+                        Ok(list) => matches_tx
+                            .send(AppMatchesInfo::album(album.clone(), list))
+                            .expect("send fails only if receiver is disconnected"),
                        Err(err) => return AppMachine::error(self.inner, err.to_string()).into(),
                    }

@ -114,12 +116,14 @@ impl IAppInteractBrowse for AppMachine<AppBrowse> {
                }
            }
            None => match self.inner.musicbrainz.search_artist(artist) {
-                Ok(list) => matches.push(AppMatchesInfo::artist(artist.clone(), list)),
+                Ok(list) => matches_tx
+                    .send(AppMatchesInfo::artist(artist.clone(), list))
+                    .expect("send fails only if receiver is disconnected"),
                Err(err) => return AppMachine::error(self.inner, err.to_string()).into(),
            },
        };

-        AppMachine::matches(self.inner, matches).into()
+        AppMachine::matches(self.inner, matches_rx).into()
    }

    fn no_op(self) -> Self::APP {
@ -135,7 +139,8 @@ mod tests {
    use crate::tui::{
        app::{
            machine::tests::{inner, inner_with_mb, music_hoard},
-            Category, IAppAccess, IAppInteract, IAppInteractMatches,
+            AppPublicAlbumMatches, AppPublicArtistMatches, AppPublicMatchesInfo, Category,
+            IAppAccess, IAppInteract, IAppInteractMatches,
        },
        lib::interface::musicbrainz::{self, Match, MockIMusicBrainz},
        testmod::COLLECTION,
@ -258,19 +263,11 @@ mod tests {

        let public_matches = public.state.unwrap_matches();

-        assert_eq!(
-            public_matches
-                .matches
-                .as_ref()
-                .unwrap()
-                .album_ref()
-                .matching,
-            &album_1
-        );
-        assert_eq!(
-            public_matches.matches.as_ref().unwrap().album_ref().list,
-            matches_1.as_slice()
-        );
+        let expected = Some(AppPublicMatchesInfo::Album(AppPublicAlbumMatches {
+            matching: &album_1,
+            list: &matches_1,
+        }));
+        assert_eq!(public_matches.matches, expected);

        let mut app = app.unwrap_matches().select();

@ -279,19 +276,11 @@ mod tests {

        let public_matches = public.state.unwrap_matches();

-        assert_eq!(
-            public_matches
-                .matches
-                .as_ref()
-                .unwrap()
-                .album_ref()
-                .matching,
-            &album_4
-        );
-        assert_eq!(
-            public_matches.matches.as_ref().unwrap().album_ref().list,
-            matches_4.as_slice()
-        );
+        let expected = Some(AppPublicMatchesInfo::Album(AppPublicAlbumMatches {
+            matching: &album_4,
+            list: &matches_4,
+        }));
+        assert_eq!(public_matches.matches, expected);

        let app = app.unwrap_matches().select();
        app.unwrap_browse();
@ -335,19 +324,11 @@ mod tests {

        let public_matches = public.state.unwrap_matches();

-        assert_eq!(
-            public_matches
-                .matches
-                .as_ref()
-                .unwrap()
-                .artist_ref()
-                .matching,
-            &artist
-        );
-        assert_eq!(
-            public_matches.matches.as_ref().unwrap().artist_ref().list,
-            matches.as_slice()
-        );
+        let expected = Some(AppPublicMatchesInfo::Artist(AppPublicArtistMatches {
+            matching: &artist,
+            list: &matches,
+        }));
+        assert_eq!(public_matches.matches, expected);

        let app = app.unwrap_matches().select();
        app.unwrap_browse();
--- a/src/tui/app/machine/matches.rs
+++ b/src/tui/app/machine/matches.rs
@ -1,4 +1,4 @@
-use std::cmp;
+use std::{cmp, sync::mpsc};

 use musichoard::collection::{album::Album, artist::Artist};

@ -101,30 +101,20 @@ impl<'app> From<&'app AppMatchesInfo> for AppPublicMatchesInfo<'app> {
 }

 pub struct AppMatches {
-    matches_info_vec: Vec<AppMatchesInfo>,
-    index: Option<usize>,
+    matches_rx: mpsc::Receiver<AppMatchesInfo>,
+    current: Option<AppMatchesInfo>,
    state: WidgetState,
 }

 impl AppMachine<AppMatches> {
-    pub fn matches(inner: AppInner, matches_info_vec: Vec<AppMatchesInfo>) -> Self {
-        let mut index = None;
-        let mut state = WidgetState::default();
-        if let Some(matches_info) = matches_info_vec.first() {
-            index = Some(0);
-            if !matches_info.is_empty() {
-                state.list.select(Some(0));
-            }
-        }
-
-        AppMachine {
-            inner,
-            state: AppMatches {
-                matches_info_vec,
-                index,
-                state,
-            },
-        }
+    pub fn matches(inner: AppInner, matches_rx: mpsc::Receiver<AppMatchesInfo>) -> Self {
+        let mut state = AppMatches {
+            matches_rx,
+            current: None,
+            state: WidgetState::default(),
+        };
+        state.next_matches_info();
+        AppMachine { inner, state }
    }
 }

@ -136,15 +126,10 @@ impl From<AppMachine<AppMatches>> for App {

 impl<'a> From<&'a mut AppMachine<AppMatches>> for AppPublic<'a> {
    fn from(machine: &'a mut AppMachine<AppMatches>) -> Self {
-        let matches = machine
-            .state
-            .index
-            .map(|index| (&machine.state.matches_info_vec[index]).into());
-
        AppPublic {
            inner: (&mut machine.inner).into(),
            state: AppState::Matches(AppPublicMatches {
-                matches,
+                matches: machine.state.current.as_ref().map(Into::into),
                state: &mut machine.state.state,
            }),
        }
@ -155,8 +140,8 @@ impl IAppInteractMatches for AppMachine<AppMatches> {
    type APP = App;

    fn prev_match(mut self) -> Self::APP {
-        if let Some(list_index) = self.state.state.list.selected() {
-            let result = list_index.saturating_sub(1);
+        if let Some(index) = self.state.state.list.selected() {
+            let result = index.saturating_sub(1);
            self.state.state.list.select(Some(result));
        }

@ -164,13 +149,12 @@ impl IAppInteractMatches for AppMachine<AppMatches> {
    }

    fn next_match(mut self) -> Self::APP {
-        if let Some(list_index) = self.state.state.list.selected() {
-            let result = list_index.saturating_add(1);
+        if let Some(index) = self.state.state.list.selected() {
+            let result = index.saturating_add(1);
            let to = cmp::min(
                result,
-                self.state.matches_info_vec[self.state.index.unwrap()]
-                    .len()
-                    .saturating_sub(1),
+                // selected() implies current exists
+                self.state.current.as_ref().unwrap().len().saturating_sub(1),
            );
            self.state.state.list.select(Some(to));
        }
@ -179,18 +163,17 @@ impl IAppInteractMatches for AppMachine<AppMatches> {
    }

    fn select(mut self) -> Self::APP {
-        self.state.index = self.state.index.map(|i| i.saturating_add(1));
+        self.state.next_matches_info();
+        match self.state.current {
+            Some(ref matches_info) => {
                self.state.state = WidgetState::default();
-        if let Some(index) = self.state.index {
-            if let Some(matches_info) = self.state.matches_info_vec.get(index) {
                if !matches_info.is_empty() {
                    self.state.state.list.select(Some(0));
                }
-                return self.into();
+                self.into()
            }
+            None => AppMachine::browse(self.inner).into(),
        }
-
-        AppMachine::browse(self.inner).into()
    }

    fn abort(self) -> Self::APP {
@ -202,8 +185,29 @@ impl IAppInteractMatches for AppMachine<AppMatches> {
    }
 }

+trait IAppInteractMatchesPrivate {
+    fn next_matches_info(&mut self);
+}
+
+impl IAppInteractMatchesPrivate for AppMatches {
+    fn next_matches_info(&mut self) {
+        // FIXME: try_recv might not be appropriate for asynchronous version.
+        (self.current, self.state) = match self.matches_rx.try_recv() {
+            Ok(next_match) => {
+                let mut state = WidgetState::default();
+                if !next_match.is_empty() {
+                    state.list.select(Some(0));
+                }
+                (Some(next_match), state)
+            }
+            Err(_) => (None, WidgetState::default()),
+        }
+    }
+}
+
 #[cfg(test)]
 mod tests {
+    use mpsc::Receiver;
    use musichoard::collection::{
        album::{AlbumDate, AlbumId, AlbumPrimaryType, AlbumSecondaryType},
        artist::ArtistId,
@ -216,15 +220,6 @@ mod tests {

    use super::*;

-    impl AppMatchesInfo {
-        fn album_ref(&self) -> &AppAlbumMatchesInfo {
-            match self {
-                Self::Album(a) => a,
-                Self::Artist(_) => panic!(),
-            }
-        }
-    }
-
    fn artist_matches_info_vec() -> Vec<AppMatchesInfo> {
        let artist_1 = Artist::new(ArtistId::new("Artist 1"));

@ -284,14 +279,21 @@ mod tests {
        vec![matches_info_1, matches_info_2]
    }

+    fn receiver(matches_info_vec: Vec<AppMatchesInfo>) -> Receiver<AppMatchesInfo> {
+        let (tx, rx) = mpsc::channel();
+        for matches_info in matches_info_vec.into_iter() {
+            tx.send(matches_info).unwrap();
+        }
+        rx
+    }
+
    #[test]
    fn create_empty() {
-        let matches = AppMachine::matches(inner(music_hoard(vec![])), vec![]);
+        let matches = AppMachine::matches(inner(music_hoard(vec![])), receiver(vec![]));

        let widget_state = WidgetState::default();

-        assert_eq!(matches.state.matches_info_vec, vec![]);
-        assert_eq!(matches.state.index, None);
+        assert_eq!(matches.state.current, None);
        assert_eq!(matches.state.state, widget_state);

        let mut app = matches.no_op();
@ -305,63 +307,55 @@ mod tests {
    #[test]
    fn create_nonempty() {
        let matches_info_vec = album_matches_info_vec();
-        let matches = AppMachine::matches(inner(music_hoard(vec![])), matches_info_vec.clone());
+        let matches = AppMachine::matches(
+            inner(music_hoard(vec![])),
+            receiver(matches_info_vec.clone()),
+        );

        let mut widget_state = WidgetState::default();
        widget_state.list.select(Some(0));

-        assert_eq!(matches.state.matches_info_vec, matches_info_vec);
-        assert_eq!(matches.state.index, Some(0));
+        assert_eq!(matches.state.current.as_ref(), Some(&matches_info_vec[0]));
        assert_eq!(matches.state.state, widget_state);

        let mut app = matches.no_op();
        let public = app.get();
        let public_matches = public.state.unwrap_matches();

-        assert_eq!(
-            public_matches
-                .matches
-                .as_ref()
-                .unwrap()
-                .album_ref()
-                .matching,
-            &matches_info_vec[0].album_ref().matching
-        );
-        assert_eq!(
-            public_matches.matches.as_ref().unwrap().album_ref().list,
-            matches_info_vec[0].album_ref().list.as_slice()
-        );
+        assert_eq!(public_matches.matches, Some((&matches_info_vec[0]).into()));
        assert_eq!(public_matches.state, &widget_state);
    }

    fn matches_flow(matches_info_vec: Vec<AppMatchesInfo>) {
-        let matches = AppMachine::matches(inner(music_hoard(vec![])), matches_info_vec.clone());
+        let matches = AppMachine::matches(
+            inner(music_hoard(vec![])),
+            receiver(matches_info_vec.clone()),
+        );

        let mut widget_state = WidgetState::default();
        widget_state.list.select(Some(0));

-        assert_eq!(matches.state.matches_info_vec, matches_info_vec);
-        assert_eq!(matches.state.index, Some(0));
+        assert_eq!(matches.state.current.as_ref(), Some(&matches_info_vec[0]));
        assert_eq!(matches.state.state, widget_state);

        let matches = matches.prev_match().unwrap_matches();

-        assert_eq!(matches.state.index, Some(0));
+        assert_eq!(matches.state.current.as_ref(), Some(&matches_info_vec[0]));
        assert_eq!(matches.state.state.list.selected(), Some(0));

        let matches = matches.next_match().unwrap_matches();

-        assert_eq!(matches.state.index, Some(0));
+        assert_eq!(matches.state.current.as_ref(), Some(&matches_info_vec[0]));
        assert_eq!(matches.state.state.list.selected(), Some(1));

        let matches = matches.next_match().unwrap_matches();

-        assert_eq!(matches.state.index, Some(0));
+        assert_eq!(matches.state.current.as_ref(), Some(&matches_info_vec[0]));
        assert_eq!(matches.state.state.list.selected(), Some(1));

        let matches = matches.select().unwrap_matches();

-        assert_eq!(matches.state.index, Some(1));
+        assert_eq!(matches.state.current.as_ref(), Some(&matches_info_vec[1]));
        assert_eq!(matches.state.state.list.selected(), Some(0));

        // And it's done
@ -381,13 +375,15 @@ mod tests {
    #[test]
    fn matches_abort() {
        let matches_info_vec = album_matches_info_vec();
-        let matches = AppMachine::matches(inner(music_hoard(vec![])), matches_info_vec.clone());
+        let matches = AppMachine::matches(
+            inner(music_hoard(vec![])),
+            receiver(matches_info_vec.clone()),
+        );

        let mut widget_state = WidgetState::default();
        widget_state.list.select(Some(0));

-        assert_eq!(matches.state.matches_info_vec, matches_info_vec);
-        assert_eq!(matches.state.index, Some(0));
+        assert_eq!(matches.state.current.as_ref(), Some(&matches_info_vec[0]));
        assert_eq!(matches.state.state, widget_state);

        matches.abort().unwrap_browse();
@ -395,17 +391,16 @@ mod tests {

    #[test]
    fn matches_select_empty() {
-        let matches = AppMachine::matches(inner(music_hoard(vec![])), vec![]);
+        let matches = AppMachine::matches(inner(music_hoard(vec![])), receiver(vec![]));

-        assert_eq!(matches.state.matches_info_vec, vec![]);
-        assert_eq!(matches.state.index, None);
+        assert_eq!(matches.state.current, None);

        matches.select().unwrap_browse();
    }

    #[test]
    fn no_op() {
-        let matches = AppMachine::matches(inner(music_hoard(vec![])), vec![]);
+        let matches = AppMachine::matches(inner(music_hoard(vec![])), receiver(vec![]));
        let app = matches.no_op();
        app.unwrap_matches();
    }
--- a/src/tui/app/machine/mod.rs
+++ b/src/tui/app/machine/mod.rs
@ -143,6 +143,8 @@ impl<'a> From<&'a mut AppInner> for AppPublicInner<'a> {

 #[cfg(test)]
 mod tests {
+    use std::sync::mpsc;
+
    use musichoard::collection::Collection;

    use crate::tui::{
@ -311,7 +313,8 @@ mod tests {
        let mut app = App::new(music_hoard_init(vec![]), mb_api());
        assert!(app.is_running());

-        app = AppMachine::matches(app.unwrap_browse().inner, vec![]).into();
+        let (_, rx) = mpsc::channel();
+        app = AppMachine::matches(app.unwrap_browse().inner, rx).into();

        let state = app.state();
        assert!(matches!(state, AppState::Matches(_)));
--- a/src/tui/app/mod.rs
+++ b/src/tui/app/mod.rs
@ -165,22 +165,6 @@ impl<BS, IS, RS, SS, MS, ES, CS> AppState<BS, IS, RS, SS, MS, ES, CS> {
 mod tests {
    use super::*;

-    impl<'app> AppPublicMatchesInfo<'app> {
-        pub fn artist_ref(&self) -> &AppPublicArtistMatches<'app> {
-            match self {
-                Self::Artist(m) => m,
-                _ => panic!(),
-            }
-        }
-
-        pub fn album_ref(&self) -> &AppPublicAlbumMatches<'app> {
-            match self {
-                Self::Album(m) => m,
-                _ => panic!(),
-            }
-        }
-    }
-
    #[test]
    fn app_is_state() {
        let state = AppPublicState::Search("get rekt");