Move fetch into threads

src/main.rs

@@ -73,7 +73,7 @@ struct DbOpt {
 fn with<Database: IDatabase + 'static, Library: ILibrary + 'static>(
     builder: MusicHoardBuilder<Database, Library>,
 ) {
-    let music_hoard = Box::new(builder.build().expect("failed to initialise MusicHoard"));
+    let music_hoard = builder.build().expect("failed to initialise MusicHoard");
 
     // Initialize the terminal user interface.
     let backend = CrosstermBackend::new(io::stdout());
@@ -86,7 +86,7 @@ fn with<Database: IDatabase + 'static, Library: ILibrary + 'static>(
     let http =
         MusicBrainzHttp::new(MUSICHOARD_HTTP_USER_AGENT).expect("failed to initialise HTTP client");
     let client = MusicBrainzClient::new(http);
-    let musicbrainz = Box::new(MusicBrainz::new(client));
+    let musicbrainz = MusicBrainz::new(client);
 
     let app = App::new(music_hoard, musicbrainz);
     let ui = Ui;

src/tui/app/machine/browse.rs

@@ -1,4 +1,7 @@
-use std::{sync::mpsc, thread, time};
+use std::{
+    sync::{mpsc, Arc, Mutex},
+    thread, time,
+};
 
 use musichoard::collection::{
     album::AlbumMeta,
@@ -12,7 +15,7 @@ use crate::tui::{
         selection::{Delta, ListSelection},
         AppMatchesInfo, AppPublic, AppState, IAppInteractBrowse,
     },
-    lib::interface::musicbrainz::Error as MbError,
+    lib::interface::musicbrainz::{Error as MbError, IMusicBrainz},
 };
 
 pub struct AppBrowse;
@@ -89,7 +92,7 @@ impl IAppInteractBrowse for AppMachine<AppBrowse> {
         AppMachine::search(self.inner, orig).into()
     }
 
-    fn fetch_musicbrainz(mut self) -> Self::APP {
+    fn fetch_musicbrainz(self) -> Self::APP {
         let coll = self.inner.music_hoard.get_collection();
         let artist = match self.inner.selection.state_artist(coll) {
             Some(artist_state) => &coll[artist_state.index],
@@ -102,13 +105,16 @@ impl IAppInteractBrowse for AppMachine<AppBrowse> {
 
         match artist.meta.musicbrainz {
             Some(ref arid) => {
-                self.fetch_albums(
+                let musicbrainz = Arc::clone(&self.inner.musicbrainz);
-                    matches_tx,
+                let arid = arid.mbid().clone();
-                    arid.mbid().clone(),
+                let albums = artist.albums.iter().map(|a| &a.meta).cloned().collect();
-                    artist.albums.iter().map(|a| &a.meta).cloned().collect(),
+                thread::spawn(|| Self::fetch_albums(musicbrainz, matches_tx, arid, albums));
-                );
+            }
+            None => {
+                let musicbrainz = Arc::clone(&self.inner.musicbrainz);
+                let artist = artist.meta.clone();
+                thread::spawn(|| Self::fetch_artist(musicbrainz, matches_tx, artist));
             }
-            None => self.fetch_artist(matches_tx, artist.meta.clone()),
         };
 
         AppMachine::app_matches(self.inner, matches_rx)
@@ -125,31 +131,49 @@ pub type FetchSender = mpsc::Sender<FetchResult>;
 pub type FetchReceiver = mpsc::Receiver<FetchResult>;
 
 trait IAppInteractBrowsePrivate {
-    fn fetch_artist(&mut self, matches_tx: FetchSender, artist: ArtistMeta);
+    fn fetch_artist(
-    fn fetch_albums(&mut self, matches_tx: FetchSender, arid: Mbid, albums: Vec<AlbumMeta>);
+        musicbrainz: Arc<Mutex<dyn IMusicBrainz + Send>>,
+        matches_tx: FetchSender,
+        artist: ArtistMeta,
+    );
+    fn fetch_albums(
+        musicbrainz: Arc<Mutex<dyn IMusicBrainz + Send>>,
+        matches_tx: FetchSender,
+        arid: Mbid,
+        albums: Vec<AlbumMeta>,
+    );
 }
 
 impl IAppInteractBrowsePrivate for AppMachine<AppBrowse> {
-    fn fetch_artist(&mut self, matches_tx: FetchSender, artist: ArtistMeta) {
+    fn fetch_artist(
-        let musicbrainz = &mut self.inner.musicbrainz;
+        musicbrainz: Arc<Mutex<dyn IMusicBrainz + Send>>,
-        let result = musicbrainz
+        matches_tx: FetchSender,
-            .search_artist(&artist)
+        artist: ArtistMeta,
-            .map(|list| AppMatchesInfo::artist(artist, list));
+    ) {
-        matches_tx.send(result).expect("receiver is disconnected");
+        let result = musicbrainz.lock().unwrap().search_artist(&artist);
+        let result = result.map(|list| AppMatchesInfo::artist(artist, list));
+        matches_tx.send(result).ok();
     }
 
-    fn fetch_albums(&mut self, matches_tx: FetchSender, arid: Mbid, albums: Vec<AlbumMeta>) {
+    fn fetch_albums(
+        musicbrainz: Arc<Mutex<dyn IMusicBrainz + Send>>,
+        matches_tx: FetchSender,
+        arid: Mbid,
+        albums: Vec<AlbumMeta>,
+    ) {
+        let mut musicbrainz = musicbrainz.lock().unwrap();
         let mut album_iter = albums.into_iter().peekable();
         while let Some(album) = album_iter.next() {
             if album.musicbrainz.is_some() {
                 continue;
             }
 
-            let musicbrainz = &mut self.inner.musicbrainz;
+            let result = musicbrainz.search_release_group(&arid, &album);
-            let result = musicbrainz
+            let result = result.map(|list| AppMatchesInfo::album(album, list));
-                .search_release_group(&arid, &album)
+            if matches_tx.send(result).is_err() {
-                .map(|list| AppMatchesInfo::album(album, list));
+                // If receiver disconnects just drop the rest.
-            matches_tx.send(result).expect("receiver is disconnected");
+                return;
+            }
 
             if album_iter.peek().is_some() {
                 thread::sleep(time::Duration::from_secs(1));
@@ -247,7 +271,7 @@ mod tests {
 
     #[test]
     fn fetch_musicbrainz() {
-        let mut mb_api = Box::new(MockIMusicBrainz::new());
+        let mut mb_api = MockIMusicBrainz::new();
 
         let arid: Mbid = "11111111-1111-1111-1111-111111111111".try_into().unwrap();
         let album_1 = COLLECTION[1].albums[0].meta.clone();
@@ -328,7 +352,7 @@ mod tests {
 
     #[test]
     fn fetch_musicbrainz_no_artist_mbid() {
-        let mut mb_api = Box::new(MockIMusicBrainz::new());
+        let mut mb_api = MockIMusicBrainz::new();
 
         let artist = COLLECTION[3].meta.clone();
 
@@ -372,7 +396,7 @@ mod tests {
 
     #[test]
     fn fetch_musicbrainz_artist_api_error() {
-        let mut mb_api = Box::new(MockIMusicBrainz::new());
+        let mut mb_api = MockIMusicBrainz::new();
 
         let error = Err(musicbrainz::Error::RateLimit);
 
@@ -394,7 +418,7 @@ mod tests {
 
     #[test]
     fn fetch_musicbrainz_album_api_error() {
-        let mut mb_api = Box::new(MockIMusicBrainz::new());
+        let mut mb_api = MockIMusicBrainz::new();
 
         let error = Err(musicbrainz::Error::RateLimit);
 
@@ -409,6 +433,45 @@ mod tests {
         app.unwrap_error();
     }
 
+    #[test]
+    fn fetch_musicbrainz_artist_receiver_disconnect() {
+        let (tx, rx) = mpsc::channel::<FetchResult>();
+        drop(rx);
+
+        let mut mb_api = MockIMusicBrainz::new();
+
+        mb_api
+            .expect_search_artist()
+            .times(1)
+            .return_once(|_| Ok(vec![]));
+
+        // We only check it does not panic and that it doesn't call the API more than once.
+        AppMachine::fetch_artist(Arc::new(Mutex::new(mb_api)), tx, COLLECTION[3].meta.clone());
+    }
+
+    #[test]
+    fn fetch_musicbrainz_albums_receiver_disconnect() {
+        let (tx, rx) = mpsc::channel::<FetchResult>();
+        drop(rx);
+
+        let mut mb_api = MockIMusicBrainz::new();
+
+        mb_api
+            .expect_search_release_group()
+            .times(1)
+            .return_once(|_, _| Ok(vec![]));
+
+        // We only check it does not panic and that it doesn't call the API more than once.
+        let mbref = &COLLECTION[1].meta.musicbrainz;
+        let albums = &COLLECTION[1].albums;
+        AppMachine::fetch_albums(
+            Arc::new(Mutex::new(mb_api)),
+            tx,
+            mbref.as_ref().unwrap().mbid().clone(),
+            albums.iter().map(|a| &a.meta).cloned().collect(),
+        );
+    }
+
     #[test]
     fn no_op() {
         let browse = AppMachine::browse(inner(music_hoard(vec![])));

src/tui/app/machine/matches.rs

@@ -1,13 +1,11 @@
 use std::cmp;
 
 use crate::tui::app::{
-    machine::{App, AppInner, AppMachine},
+    machine::{browse::FetchReceiver, App, AppInner, AppMachine},
     AppAlbumMatches, AppArtistMatches, AppMatchesInfo, AppPublic, AppPublicMatches, AppState,
     IAppInteractMatches, MatchOption, WidgetState,
 };
 
-use super::browse::FetchReceiver;
-
 impl AppArtistMatches {
     fn len(&self) -> usize {
         self.list.len()
@@ -51,7 +49,7 @@ pub struct AppMatches {
 }
 
 impl AppMatches {
-    fn new(matches_rx: FetchReceiver) -> Self {
+    fn empty(matches_rx: FetchReceiver) -> Self {
         AppMatches {
             matches_rx,
             current: None,
@@ -66,7 +64,7 @@ impl AppMachine<AppMatches> {
     }
 
     pub fn app_matches(inner: AppInner, matches_rx: FetchReceiver) -> App {
-        AppMachine::matches(inner, AppMatches::new(matches_rx)).fetch()
+        AppMachine::matches(inner, AppMatches::empty(matches_rx)).fetch_first()
     }
 }
 
@@ -115,7 +113,7 @@ impl IAppInteractMatches for AppMachine<AppMatches> {
     }
 
     fn select(self) -> Self::APP {
-        self.fetch()
+        self.fetch_next()
     }
 
     fn abort(self) -> Self::APP {
@@ -131,11 +129,21 @@ trait IAppInteractMatchesPrivate
 where
     Self: Sized,
 {
-    fn fetch(self) -> App;
+    fn fetch_first(self) -> App;
+    fn fetch_next(self) -> App;
+    fn fetch(self, first: bool) -> App;
 }
 
 impl IAppInteractMatchesPrivate for AppMachine<AppMatches> {
-    fn fetch(mut self) -> App {
+    fn fetch_first(self) -> App {
+        self.fetch(true)
+    }
+
+    fn fetch_next(self) -> App {
+        self.fetch(false)
+    }
+
+    fn fetch(mut self, first: bool) -> App {
         match self.state.matches_rx.recv() {
             Ok(fetch_result) => match fetch_result {
                 Ok(mut next_match) => {
@@ -147,7 +155,13 @@ impl IAppInteractMatchesPrivate for AppMachine<AppMatches> {
                 Err(err) => AppMachine::error(self.inner, format!("fetch failed: {err}")).into(),
             },
             // only happens when the sender disconnects which means it finished its job
-            Err(_) => AppMachine::browse(self.inner).into(),
+            Err(_) => {
+                if first {
+                    AppMachine::matches(self.inner, AppMatches::empty(self.state.matches_rx)).into()
+                } else {
+                    AppMachine::browse(self.inner).into()
+                }
+            }
         }
     }
 }

src/tui/app/machine/mod.rs

@@ -6,6 +6,8 @@ mod matches;
 mod reload;
 mod search;
 
+use std::sync::{Arc, Mutex};
+
 use crate::tui::{
     app::{selection::Selection, AppPublic, AppPublicInner, AppState, IAppAccess, IAppInteract},
     lib::{interface::musicbrainz::IMusicBrainz, IMusicHoard},
@@ -37,21 +39,24 @@ pub struct AppMachine<STATE> {
 pub struct AppInner {
     running: bool,
     music_hoard: Box<dyn IMusicHoard>,
-    musicbrainz: Box<dyn IMusicBrainz>,
+    musicbrainz: Arc<Mutex<dyn IMusicBrainz + Send>>,
     selection: Selection,
 }
 
 impl App {
-    pub fn new(mut music_hoard: Box<dyn IMusicHoard>, mb_api: Box<dyn IMusicBrainz>) -> Self {
+    pub fn new<MH: IMusicHoard + 'static, MB: IMusicBrainz + Send + 'static>(
+        mut music_hoard: MH,
+        musicbrainz: MB,
+    ) -> Self {
         let init_result = Self::init(&mut music_hoard);
-        let inner = AppInner::new(music_hoard, mb_api);
+        let inner = AppInner::new(music_hoard, musicbrainz);
         match init_result {
             Ok(()) => AppMachine::browse(inner).into(),
             Err(err) => AppMachine::critical(inner, err.to_string()).into(),
         }
     }
 
-    fn init(music_hoard: &mut Box<dyn IMusicHoard>) -> Result<(), musichoard::Error> {
+    fn init<MH: IMusicHoard>(music_hoard: &mut MH) -> Result<(), musichoard::Error> {
         music_hoard.rescan_library()?;
         Ok(())
     }
@@ -121,12 +126,15 @@ impl IAppAccess for App {
 }
 
 impl AppInner {
-    pub fn new(music_hoard: Box<dyn IMusicHoard>, musicbrainz: Box<dyn IMusicBrainz>) -> Self {
+    pub fn new<MH: IMusicHoard + 'static, MB: IMusicBrainz + Send + 'static>(
+        music_hoard: MH,
+        musicbrainz: MB,
+    ) -> Self {
         let selection = Selection::new(music_hoard.get_collection());
         AppInner {
             running: true,
-            music_hoard,
+            music_hoard: Box::new(music_hoard),
-            musicbrainz,
+            musicbrainz: Arc::new(Mutex::new(musicbrainz)),
             selection,
         }
     }
@@ -205,14 +213,14 @@ mod tests {
         }
     }
 
-    pub fn music_hoard(collection: Collection) -> Box<MockIMusicHoard> {
+    pub fn music_hoard(collection: Collection) -> MockIMusicHoard {
-        let mut music_hoard = Box::new(MockIMusicHoard::new());
+        let mut music_hoard = MockIMusicHoard::new();
         music_hoard.expect_get_collection().return_const(collection);
 
         music_hoard
     }
 
-    fn music_hoard_init(collection: Collection) -> Box<MockIMusicHoard> {
+    fn music_hoard_init(collection: Collection) -> MockIMusicHoard {
         let mut music_hoard = music_hoard(collection);
 
         music_hoard
@@ -223,18 +231,15 @@ mod tests {
         music_hoard
     }
 
-    fn mb_api() -> Box<MockIMusicBrainz> {
+    fn mb_api() -> MockIMusicBrainz {
-        Box::new(MockIMusicBrainz::new())
+        MockIMusicBrainz::new()
     }
 
-    pub fn inner(music_hoard: Box<MockIMusicHoard>) -> AppInner {
+    pub fn inner(music_hoard: MockIMusicHoard) -> AppInner {
         AppInner::new(music_hoard, mb_api())
     }
 
-    pub fn inner_with_mb(
+    pub fn inner_with_mb(music_hoard: MockIMusicHoard, mb_api: MockIMusicBrainz) -> AppInner {
-        music_hoard: Box<MockIMusicHoard>,
-        mb_api: Box<MockIMusicBrainz>,
-    ) -> AppInner {
         AppInner::new(music_hoard, mb_api)
     }
 
@@ -365,7 +370,7 @@ mod tests {
 
     #[test]
     fn init_error() {
-        let mut music_hoard = Box::new(MockIMusicHoard::new());
+        let mut music_hoard = MockIMusicHoard::new();
 
         music_hoard
             .expect_rescan_library()

src/tui/mod.rs

@@ -192,8 +192,8 @@ mod tests {
         Terminal::new(backend).unwrap()
     }
 
-    fn music_hoard(collection: Collection) -> Box<MockIMusicHoard> {
+    fn music_hoard(collection: Collection) -> MockIMusicHoard {
-        let mut music_hoard = Box::new(MockIMusicHoard::new());
+        let mut music_hoard = MockIMusicHoard::new();
 
         music_hoard.expect_reload_database().returning(|| Ok(()));
         music_hoard.expect_rescan_library().returning(|| Ok(()));
@@ -203,7 +203,7 @@ mod tests {
     }
 
     fn app(collection: Collection) -> App {
-        App::new(music_hoard(collection), Box::new(MockIMusicBrainz::new()))
+        App::new(music_hoard(collection), MockIMusicBrainz::new())
     }
 
     fn listener() -> MockIEventListener {