From 0d8803d35dbf84645e55def625b95799e1f15ae3 Mon Sep 17 00:00:00 2001 From: Alex Clarke Date: Wed, 6 Nov 2024 15:50:47 -0700 Subject: [PATCH] fix(ui): Fixed a bug that would freeze all user input while background network requests were running --- src/app/app_tests.rs | 40 +++++++++++++++++++++++++++---- src/app/mod.rs | 7 +++++- src/app/radarr/mod.rs | 29 +++++++++-------------- src/app/radarr/radarr_tests.rs | 43 +++++++++++----------------------- src/main.rs | 6 +---- src/network/mod.rs | 5 +--- 6 files changed, 68 insertions(+), 62 deletions(-) diff --git a/src/app/app_tests.rs b/src/app/app_tests.rs index e2b429f..82af22a 100644 --- a/src/app/app_tests.rs +++ b/src/app/app_tests.rs @@ -87,7 +87,11 @@ mod tests { #[test] fn test_reset_cancellation_token() { - let mut app = App::default(); + let mut app = App { + is_loading: true, + should_refresh: false, + ..App::default() + }; app.cancellation_token.cancel(); assert!(app.cancellation_token.is_cancelled()); @@ -96,6 +100,8 @@ mod tests { assert!(!app.cancellation_token.is_cancelled()); assert!(!new_token.is_cancelled()); + assert!(!app.is_loading); + assert!(app.should_refresh); } #[test] @@ -145,6 +151,29 @@ mod tests { assert_eq!(app.error.text, test_string); } + #[tokio::test] + async fn test_dispatch_network_event() { + let (sync_network_tx, mut sync_network_rx) = mpsc::channel::(500); + + let mut app = App { + tick_until_poll: 2, + network_tx: Some(sync_network_tx), + ..App::default() + }; + + assert_eq!(app.tick_count, 0); + + app + .dispatch_network_event(RadarrEvent::GetStatus.into()) + .await; + + assert_eq!( + sync_network_rx.recv().await.unwrap(), + RadarrEvent::GetStatus.into() + ); + assert_eq!(app.tick_count, 0); + } + #[tokio::test] async fn test_on_tick_first_render() { let (sync_network_tx, mut sync_network_rx) = mpsc::channel::(500); @@ -158,6 +187,7 @@ mod tests { assert_eq!(app.tick_count, 0); app.on_tick(true).await; + assert_eq!( sync_network_rx.recv().await.unwrap(), RadarrEvent::GetQualityProfiles.into() @@ -170,6 +200,10 @@ mod tests { sync_network_rx.recv().await.unwrap(), RadarrEvent::GetRootFolders.into() ); + assert_eq!( + sync_network_rx.recv().await.unwrap(), + RadarrEvent::GetDownloads.into() + ); assert_eq!( sync_network_rx.recv().await.unwrap(), RadarrEvent::GetOverview.into() @@ -182,10 +216,6 @@ mod tests { sync_network_rx.recv().await.unwrap(), RadarrEvent::GetMovies.into() ); - assert_eq!( - sync_network_rx.recv().await.unwrap(), - RadarrEvent::GetDownloads.into() - ); assert!(!app.is_routing); assert!(!app.should_refresh); assert_eq!(app.tick_count, 1); diff --git a/src/app/mod.rs b/src/app/mod.rs index 970877c..87faa8e 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -56,7 +56,10 @@ impl<'a> App<'a> { pub async fn dispatch_network_event(&mut self, action: NetworkEvent) { debug!("Dispatching network event: {action:?}"); - self.is_loading = true; + if !self.should_refresh { + self.is_loading = true; + } + if let Some(network_tx) = &self.network_tx { if let Err(e) = network_tx.send(action).await { self.is_loading = false; @@ -113,6 +116,8 @@ impl<'a> App<'a> { pub fn reset_cancellation_token(&mut self) -> CancellationToken { self.cancellation_token = CancellationToken::new(); + self.should_refresh = true; + self.is_loading = false; self.cancellation_token.clone() } diff --git a/src/app/radarr/mod.rs b/src/app/radarr/mod.rs index 11ddd4d..3ffc43a 100644 --- a/src/app/radarr/mod.rs +++ b/src/app/radarr/mod.rs @@ -142,35 +142,22 @@ impl<'a> App<'a> { is_first_render: bool, ) { if is_first_render { - self - .dispatch_network_event(RadarrEvent::GetQualityProfiles.into()) - .await; - self - .dispatch_network_event(RadarrEvent::GetTags.into()) - .await; - self - .dispatch_network_event(RadarrEvent::GetRootFolders.into()) - .await; - self - .dispatch_network_event(RadarrEvent::GetOverview.into()) - .await; - self - .dispatch_network_event(RadarrEvent::GetStatus.into()) - .await; + self.refresh_metadata().await; self.dispatch_by_radarr_block(&active_radarr_block).await; } if self.should_refresh { self.dispatch_by_radarr_block(&active_radarr_block).await; + self.refresh_metadata().await; } if self.is_routing { if !self.should_refresh { self.cancellation_token.cancel(); + } else { + self.dispatch_by_radarr_block(&active_radarr_block).await; + self.refresh_metadata().await; } - - self.dispatch_by_radarr_block(&active_radarr_block).await; - self.refresh_metadata().await; } if self.tick_count % self.tick_until_poll == 0 { @@ -191,6 +178,12 @@ impl<'a> App<'a> { self .dispatch_network_event(RadarrEvent::GetDownloads.into()) .await; + self + .dispatch_network_event(RadarrEvent::GetOverview.into()) + .await; + self + .dispatch_network_event(RadarrEvent::GetStatus.into()) + .await; } async fn populate_movie_collection_table(&mut self) { diff --git a/src/app/radarr/radarr_tests.rs b/src/app/radarr/radarr_tests.rs index 90a1d97..286d21f 100644 --- a/src/app/radarr/radarr_tests.rs +++ b/src/app/radarr/radarr_tests.rs @@ -508,6 +508,14 @@ mod tests { sync_network_rx.recv().await.unwrap(), RadarrEvent::GetDownloads.into() ); + assert_eq!( + sync_network_rx.recv().await.unwrap(), + RadarrEvent::GetOverview.into() + ); + assert_eq!( + sync_network_rx.recv().await.unwrap(), + RadarrEvent::GetStatus.into() + ); assert!(app.is_loading); } @@ -529,6 +537,10 @@ mod tests { sync_network_rx.recv().await.unwrap(), RadarrEvent::GetRootFolders.into() ); + assert_eq!( + sync_network_rx.recv().await.unwrap(), + RadarrEvent::GetDownloads.into() + ); assert_eq!( sync_network_rx.recv().await.unwrap(), RadarrEvent::GetOverview.into() @@ -537,10 +549,6 @@ mod tests { sync_network_rx.recv().await.unwrap(), RadarrEvent::GetStatus.into() ); - assert_eq!( - sync_network_rx.recv().await.unwrap(), - RadarrEvent::GetDownloads.into() - ); assert!(app.is_loading); assert!(!app.data.radarr_data.prompt_confirm); } @@ -549,6 +557,7 @@ mod tests { async fn test_radarr_on_tick_routing() { let (mut app, mut sync_network_rx) = construct_app_unit(); app.is_routing = true; + app.should_refresh = true; app .radarr_on_tick(ActiveRadarrBlock::Downloads, false) @@ -574,13 +583,12 @@ mod tests { sync_network_rx.recv().await.unwrap(), RadarrEvent::GetDownloads.into() ); - assert!(app.is_loading); assert!(!app.data.radarr_data.prompt_confirm); } #[tokio::test] async fn test_radarr_on_tick_routing_while_long_request_is_running_should_cancel_request() { - let (mut app, mut sync_network_rx) = construct_app_unit(); + let (mut app, _) = construct_app_unit(); app.is_routing = true; app.should_refresh = false; @@ -588,28 +596,6 @@ mod tests { .radarr_on_tick(ActiveRadarrBlock::Downloads, false) .await; - assert_eq!( - sync_network_rx.recv().await.unwrap(), - RadarrEvent::GetDownloads.into() - ); - assert_eq!( - sync_network_rx.recv().await.unwrap(), - RadarrEvent::GetQualityProfiles.into() - ); - assert_eq!( - sync_network_rx.recv().await.unwrap(), - RadarrEvent::GetTags.into() - ); - assert_eq!( - sync_network_rx.recv().await.unwrap(), - RadarrEvent::GetRootFolders.into() - ); - assert_eq!( - sync_network_rx.recv().await.unwrap(), - RadarrEvent::GetDownloads.into() - ); - assert!(app.is_loading); - assert!(!app.data.radarr_data.prompt_confirm); assert!(app.cancellation_token.is_cancelled()); } @@ -626,7 +612,6 @@ mod tests { sync_network_rx.recv().await.unwrap(), RadarrEvent::GetDownloads.into() ); - assert!(app.is_loading); assert!(app.should_refresh); assert!(!app.data.radarr_data.prompt_confirm); } diff --git a/src/main.rs b/src/main.rs index e4281ca..18728cb 100644 --- a/src/main.rs +++ b/src/main.rs @@ -158,11 +158,7 @@ async fn start_networking( while network_rx.try_recv().is_ok() { // Discard the message } - { - /* Wrapped in its own block so we don't lock the app arc early, - so UI is still processed */ - network.reset_cancellation_token().await; - } + network.reset_cancellation_token().await; } } } diff --git a/src/network/mod.rs b/src/network/mod.rs index da944e3..e3c252e 100644 --- a/src/network/mod.rs +++ b/src/network/mod.rs @@ -75,10 +75,7 @@ impl<'a, 'b> Network<'a, 'b> { } pub(super) async fn reset_cancellation_token(&mut self) { - let mut app = self.app.lock().await; - self.cancellation_token = app.reset_cancellation_token(); - app.should_refresh = true; - app.is_loading = false; + self.cancellation_token = self.app.lock().await.reset_cancellation_token(); } async fn handle_request(