cleanup event module and implement follow-up support for podcast (#619)

Author: aome510

spotify_player/src/client/handlers.rs

@@ -14,6 +14,7 @@ use super::ClientRequest;
 
 struct PlayerEventHandlerState {
     add_track_to_queue_req_timer: std::time::Instant,
+    get_context_timer: std::time::Instant,
 }
 
 /// starts the client's request handler
@@ -112,6 +113,7 @@ fn handle_playback_change_event(
 fn handle_page_change_event(
     state: &SharedState,
     client_pub: &flume::Sender<ClientRequest>,
+    handler_state: &mut PlayerEventHandlerState,
 ) -> anyhow::Result<()> {
     match state.ui.lock().current_page_mut() {
         PageState::Context {
@@ -124,8 +126,10 @@ fn handle_page_change_event(
                 ContextPageType::CurrentPlaying => state.player.read().playing_context_id(),
             };
 
-            // update the context state and request new data when moving to a new context page
-            if *id != expected_id {
+            let new_id = if *id == expected_id {
+                false
+            } else {
+                // update the context state and request new data when moving to a new context page
                 tracing::info!("Current context ID ({:?}) is different from the expected ID ({:?}), update the context state", id, expected_id);
 
                 *id = expected_id;
@@ -145,14 +149,21 @@ fn handle_page_change_event(
                         *page_state = None;
                     }
                 }
-            }
+                true
+            };
 
             // request new context's data if not found in memory
+            // To avoid making too many requests, only request if context id is changed
+            // or it's been a while since the last request.
             if let Some(id) = id {
                 if !matches!(id, ContextId::Tracks(_))
                     && !state.data.read().caches.context.contains_key(&id.uri())
+                    && (new_id
+                        || handler_state.get_context_timer.elapsed()
+                            > std::time::Duration::from_secs(5))
                 {
                     client_pub.send(ClientRequest::GetContext(id.clone()))?;
+                    handler_state.get_context_timer = std::time::Instant::now();
                 }
             }
         }
@@ -190,7 +201,8 @@ fn handle_player_event(
     client_pub: &flume::Sender<ClientRequest>,
     handler_state: &mut PlayerEventHandlerState,
 ) -> anyhow::Result<()> {
-    handle_page_change_event(state, client_pub).context("handle page change event")?;
+    handle_page_change_event(state, client_pub, handler_state)
+        .context("handle page change event")?;
     handle_playback_change_event(state, client_pub, handler_state)
         .context("handle playback change event")?;
 
@@ -228,6 +240,7 @@ pub async fn start_player_event_watchers(
     let refresh_duration = std::time::Duration::from_secs(1);
     let mut handler_state = PlayerEventHandlerState {
         add_track_to_queue_req_timer: std::time::Instant::now(),
+        get_context_timer: std::time::Instant::now(),
     };
 
     loop {

spotify_player/src/client/mod.rs

@@ -880,12 +880,13 @@ impl Client {
                     self.start_context_playback(PlayContextId::from(id), device_id, offset, None)
                         .await?;
                 }
+                ContextId::Show(id) => {
+                    self.start_context_playback(PlayContextId::from(id), device_id, offset, None)
+                        .await?;
+                }
                 ContextId::Tracks(_) => {
                     anyhow::bail!("`StartPlayback` request for `tracks` context is not supported")
                 }
-                ContextId::Show(_) => {
-                    anyhow::bail!("`StartPlayback` request for `show` context is not supported")
-                }
             },
             Playback::URIs(ids, offset) => {
                 self.start_uris_playback(ids, device_id, offset, None)

spotify_player/src/event/mod.rs

@@ -38,8 +38,7 @@ pub fn start_event_handler(state: &SharedState, client_pub: &flume::Sender<Clien
         if let Err(err) = match event {
             crossterm::event::Event::Mouse(event) => handle_mouse_event(event, client_pub, state),
             crossterm::event::Event::Resize(columns, rows) => {
-                let mut state = state.ui.lock();
-                state.orientation = Orientation::from_size(columns, rows);
+                state.ui.lock().orientation = Orientation::from_size(columns, rows);
                 Ok(())
             }
             crossterm::event::Event::Key(event) => {
@@ -55,7 +54,7 @@ pub fn start_event_handler(state: &SharedState, client_pub: &flume::Sender<Clien
             }
             _ => Ok(()),
         } {
-            tracing::error!("Failed to handle event: {err:#}");
+            tracing::error!("Failed to handle terminal event: {err:#}");
         }
     }
 }
@@ -475,7 +474,7 @@ fn handle_show_actions_on_artist(
 }
 
 /// Handle a global action, currently this is only used to target
-/// the currently playing song instead of the selection.
+/// the currently playing item instead of the selection.
 fn handle_global_action(
     action: Action,
     target: ActionTarget,

spotify_player/src/event/page.rs

@@ -57,24 +57,24 @@ fn handle_action_for_library_page(
     match focus_state {
         LibraryFocusState::Playlists => window::handle_action_for_selected_item(
             action,
-            ui.search_filtered_items(&data.user_data.folder_playlists_items(folder_id))
+            &ui.search_filtered_items(&data.user_data.folder_playlists_items(folder_id))
                 .into_iter()
                 .copied()
-                .collect(),
+                .collect::<Vec<_>>(),
             &data,
             ui,
             client_pub,
         ),
         LibraryFocusState::SavedAlbums => window::handle_action_for_selected_item(
             action,
-            ui.search_filtered_items(&data.user_data.saved_albums),
+            &ui.search_filtered_items(&data.user_data.saved_albums),
             &data,
             ui,
             client_pub,
         ),
         LibraryFocusState::FollowedArtists => window::handle_action_for_selected_item(
             action,
-            ui.search_filtered_items(&data.user_data.followed_artists),
+            &ui.search_filtered_items(&data.user_data.followed_artists),
             &data,
             ui,
             client_pub,
@@ -98,28 +98,30 @@ fn handle_command_for_library_page(
             _ => anyhow::bail!("expect a library page state"),
         };
         match focus_state {
-            LibraryFocusState::Playlists => window::handle_command_for_playlist_list_window(
+            LibraryFocusState::Playlists => Ok(window::handle_command_for_playlist_list_window(
                 command,
-                ui.search_filtered_items(&data.user_data.folder_playlists_items(folder_id))
+                &ui.search_filtered_items(&data.user_data.folder_playlists_items(folder_id))
                     .into_iter()
                     .copied()
-                    .collect(),
+                    .collect::<Vec<_>>(),
                 &data,
                 ui,
-            ),
+            )),
             LibraryFocusState::SavedAlbums => window::handle_command_for_album_list_window(
                 command,
-                ui.search_filtered_items(&data.user_data.saved_albums),
+                &ui.search_filtered_items(&data.user_data.saved_albums),
                 &data,
                 ui,
                 client_pub,
             ),
-            LibraryFocusState::FollowedArtists => window::handle_command_for_artist_list_window(
-                command,
-                ui.search_filtered_items(&data.user_data.followed_artists),
-                &data,
-                ui,
-            ),
+            LibraryFocusState::FollowedArtists => {
+                Ok(window::handle_command_for_artist_list_window(
+                    command,
+                    &ui.search_filtered_items(&data.user_data.followed_artists),
+                    &data,
+                    ui,
+                ))
+            }
         }
     }
 }
@@ -171,75 +173,74 @@ fn handle_key_sequence_for_search_page(
     match focus_state {
         SearchFocusState::Input => anyhow::bail!("user's search input should be handled before"),
         SearchFocusState::Tracks => {
-            let tracks = match search_results {
-                Some(s) => s.tracks.iter().collect(),
-                None => Vec::new(),
-            };
+            let tracks = search_results
+                .map(|s| s.tracks.iter().collect::<Vec<_>>())
+                .unwrap_or_default();
 
             match found_keymap {
                 CommandOrAction::Command(command) => window::handle_command_for_track_list_window(
-                    command, client_pub, tracks, &data, ui,
+                    command, client_pub, &tracks, &data, ui,
                 ),
                 CommandOrAction::Action(action, ActionTarget::SelectedItem) => {
-                    window::handle_action_for_selected_item(action, tracks, &data, ui, client_pub)
+                    window::handle_action_for_selected_item(action, &tracks, &data, ui, client_pub)
                 }
                 CommandOrAction::Action(..) => Ok(false),
             }
         }
         SearchFocusState::Artists => {
             let artists = search_results
-                .map(|s| s.artists.iter().collect())
+                .map(|s| s.artists.iter().collect::<Vec<_>>())
                 .unwrap_or_default();
 
             match found_keymap {
-                CommandOrAction::Command(command) => {
-                    window::handle_command_for_artist_list_window(command, artists, &data, ui)
-                }
+                CommandOrAction::Command(command) => Ok(
+                    window::handle_command_for_artist_list_window(command, &artists, &data, ui),
+                ),
                 CommandOrAction::Action(action, ActionTarget::SelectedItem) => {
-                    window::handle_action_for_selected_item(action, artists, &data, ui, client_pub)
+                    window::handle_action_for_selected_item(action, &artists, &data, ui, client_pub)
                 }
                 CommandOrAction::Action(..) => Ok(false),
             }
         }
         SearchFocusState::Albums => {
             let albums = search_results
-                .map(|s| s.albums.iter().collect())
+                .map(|s| s.albums.iter().collect::<Vec<_>>())
                 .unwrap_or_default();
 
             match found_keymap {
                 CommandOrAction::Command(command) => window::handle_command_for_album_list_window(
-                    command, albums, &data, ui, client_pub,
+                    command, &albums, &data, ui, client_pub,
                 ),
                 CommandOrAction::Action(action, ActionTarget::SelectedItem) => {
-                    window::handle_action_for_selected_item(action, albums, &data, ui, client_pub)
+                    window::handle_action_for_selected_item(action, &albums, &data, ui, client_pub)
                 }
                 CommandOrAction::Action(..) => Ok(false),
             }
         }
         SearchFocusState::Playlists => {
-            let playlists: Vec<PlaylistFolderItem> = search_results
+            let playlists = search_results
                 .map(|s| {
                     s.playlists
                         .iter()
                         .map(|p| PlaylistFolderItem::Playlist(p.clone()))
-                        .collect()
+                        .collect::<Vec<_>>()
                 })
                 .unwrap_or_default();
-            let playlist_refs = playlists.iter().collect();
+            let playlist_refs = playlists.iter().collect::<Vec<_>>();
 
             match found_keymap {
                 CommandOrAction::Command(command) => {
-                    window::handle_command_for_playlist_list_window(
+                    Ok(window::handle_command_for_playlist_list_window(
                         command,
-                        playlist_refs,
+                        &playlist_refs,
                         &data,
                         ui,
-                    )
+                    ))
                 }
                 CommandOrAction::Action(action, ActionTarget::SelectedItem) => {
                     window::handle_action_for_selected_item(
                         action,
-                        playlist_refs,
+                        &playlist_refs,
                         &data,
                         ui,
                         client_pub,
@@ -258,7 +259,7 @@ fn handle_key_sequence_for_search_page(
                     window::handle_command_for_show_list_window(command, &shows, &data, ui),
                 ),
                 CommandOrAction::Action(action, ActionTarget::SelectedItem) => {
-                    window::handle_action_for_selected_item(action, shows, &data, ui, client_pub)
+                    window::handle_action_for_selected_item(action, &shows, &data, ui, client_pub)
                 }
                 CommandOrAction::Action(..) => Ok(false),
             }
@@ -276,7 +277,9 @@ fn handle_key_sequence_for_search_page(
                     )
                 }
                 CommandOrAction::Action(action, ActionTarget::SelectedItem) => {
-                    window::handle_action_for_selected_item(action, episodes, &data, ui, client_pub)
+                    window::handle_action_for_selected_item(
+                        action, &episodes, &data, ui, client_pub,
+                    )
                 }
                 CommandOrAction::Action(..) => Ok(false),
             }