render loading UI for search page

README.md

@@ -222,17 +222,19 @@ The `spotify_player authenticate` command runs every required flow in one go, fo
 
 Every request to the Spotify Web API is attributed to a Spotify _application_, identified by a **client ID**. The client ID — not your account — determines the [API quota](https://developer.spotify.com/documentation/web-api/concepts/rate-limits) you are subject to.
 
-By default, `spotify_player` uses [ncspot](https://github.com/hrkfdn/ncspot)'s client ID. This is intentional: that client ID is registered in [extended quota mode](https://developer.spotify.com/documentation/web-api/concepts/quota-modes) and predates Spotify's [November 2024 Web API changes](https://developer.spotify.com/blog/2024-11-27-changes-to-the-web-api). As a result it has a much higher rate limit and access to endpoints (browse, personalized content, generated playlists, …) that newly-registered applications can no longer use.
+By default, `spotify_player` uses [ncspot](https://github.com/hrkfdn/ncspot)'s client ID. This client ID is shared by many users, so its API quota can be exhausted by aggregate usage and cause `429 Too Many Requests` responses. **Registering and configuring your own client ID is strongly recommended** so routine requests use a quota dedicated to your Spotify application.
+
+The ncspot client ID remains available as a fallback because it is registered in [extended quota mode](https://developer.spotify.com/documentation/web-api/concepts/quota-modes) and predates Spotify's [November 2024 Web API changes](https://developer.spotify.com/blog/2024-11-27-changes-to-the-web-api). It can access endpoints (browse, personalized content, generated playlists, …) that newly-registered applications can no longer use.
 
 When a custom `client_id` is configured, `spotify-player` sends most Web API requests through that client first. If Spotify rejects custom-client request with any `4xx` response, the request is attempted once through ncspot. Successful requests and failures outside the `4xx` range do not trigger fallback.
 
 The custom client uses no request middleware. The ncspot client stores `Retry-After` durations and retries rate-limited GET requests up to two times by default; mutation requests are never delayed or retried by the middleware. Configure the ncspot retry count with `api_rate_limit_retries`; see the [configuration documentation](https://github.com/aome510/spotify-player/blob/master/docs/config.md) for details.
 
 ### Using a custom client ID
 
-A custom client ID can be used when you prefer requests to be attributed to your own Spotify application. Newly registered applications use restricted default quota mode, so endpoints unavailable to the custom client transparently fall back to ncspot.
+Use a custom client ID to avoid competing for the shared ncspot client's rate limit. Most requests will be attributed to your own Spotify application instead. Newly registered applications use restricted default quota mode, so endpoints unavailable to the custom client transparently fall back to ncspot.
 
-If you do need one, [register an application](https://developer.spotify.com/dashboard) on the Spotify developer dashboard, add your `login_redirect_uri` (default `http://127.0.0.1:8989/login`) to the app's allowed redirect URIs, then set `client_id` (or `client_id_command`) in `app.toml`. See the [Client id command](https://github.com/aome510/spotify-player/blob/master/docs/config.md#client-id-command) section of the configuration docs for details.
+To configure one, [register an application](https://developer.spotify.com/dashboard) on the Spotify developer dashboard, add your `login_redirect_uri` (default `http://127.0.0.1:8989/login`) to the app's allowed redirect URIs, then set `client_id` (or `client_id_command`) in `app.toml`. See the [Client id command](https://github.com/aome510/spotify-player/blob/master/docs/config.md#client-id-command) section of the configuration docs for details.
 
 After changing the client ID, re-run `spotify_player authenticate` to refresh the custom and fallback tokens.
 

docs/config.md

@@ -32,7 +32,7 @@ spotify_player -o device.volume=80 -o theme=dracula
 | --------------------------------- | --------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------- |
 | `client_id`                       | Primary Spotify client ID for API access; rejected requests can fall back to ncspot (see notes).    | See code (default: ncspot's client ID)                                 |
 | `client_id_command`               | Shell command that outputs client ID to stdout (overrides `client_id`).                             | `None`                                                                 |
-| `ncspot_only_get_endpoints`       | Endpoint prefixes for GET requests that should always use the ncspot client.                        | `["me/playlists", "search", "playlists/"]`                             |
+| `ncspot_only_get_endpoints`       | Endpoint prefixes for GET requests that should always use the ncspot client.                        | `["me/playlists", "playlists/"]`                                       |
 | `login_redirect_uri`              | Redirect URI for authentication.                                                                    | `http://127.0.0.1:8989/login`                                          |
 | `client_port`                     | Port for the application's client to handle CLI commands.                                           | `8080`                                                                 |
 | `log_folder`                      | Path to store log files.                                                                            | `None`                                                                 |

spotify_player/src/client/mod.rs

@@ -638,18 +638,13 @@ impl AppClient {
                         .insert(uri, ctx, *TTL_CACHE_DURATION);
                 }
             }
-            ClientRequest::Search(query) => {
-                if !state.data.read().caches.search.contains_key(&query) {
-                    let results = self.search(&query).await?;
-
-                    state
-                        .data
-                        .write()
-                        .caches
-                        .search
-                        .insert(query, results, *TTL_CACHE_DURATION);
+            ClientRequest::Search(query) => match self.search(&query).await {
+                Ok(results) => state.data.write().caches.complete_search(query, results),
+                Err(err) => {
+                    state.data.write().caches.fail_search(query);
+                    return Err(err);
                 }
-            }
+            },
 
             ClientRequest::AddPlayableToQueue(playable_id) => {
                 self.add_item_to_queue(playable_id, None).await?;
@@ -1152,7 +1147,7 @@ impl AppClient {
                 ],
                 None,
                 None,
-                None,
+                Some(10),
                 None,
             )
             .await?;

spotify_player/src/client/spotify.rs

@@ -570,35 +570,6 @@ mod tests {
         );
     }
 
-    #[tokio::test]
-    async fn search_uses_ncspot_without_calling_primary() {
-        let server = MockServer::start().await;
-        Mock::given(method("GET"))
-            .and(path("/v1/search"))
-            .and(header("authorization", "Bearer primary-token"))
-            .respond_with(ResponseTemplate::new(200).set_body_string("primary"))
-            .expect(0)
-            .mount(&server)
-            .await;
-        Mock::given(method("GET"))
-            .and(path("/v1/search"))
-            .and(header("authorization", "Bearer fallback-token"))
-            .respond_with(ResponseTemplate::new(200).set_body_string("ncspot"))
-            .expect(1)
-            .mount(&server)
-            .await;
-
-        let client = WebApiClient::new(
-            client_with_token(&server, false, "primary-token").await,
-            Some(client_with_token(&server, true, "fallback-token").await),
-        );
-
-        assert_eq!(
-            client.api_get("search", &Query::new()).await.unwrap(),
-            "ncspot"
-        );
-    }
-
     #[tokio::test]
     async fn configured_endpoint_prefix_uses_ncspot_without_calling_primary() {
         let server = MockServer::start().await;

spotify_player/src/config/mod.rs

@@ -6,8 +6,7 @@ const DEFAULT_CACHE_FOLDER: &str = ".cache/spotify-player";
 const APP_CONFIG_FILE: &str = "app.toml";
 const THEME_CONFIG_FILE: &str = "theme.toml";
 const KEYMAP_CONFIG_FILE: &str = "keymap.toml";
-pub(crate) const DEFAULT_NCSPOT_ONLY_GET_ENDPOINTS: &[&str] =
-    &["me/playlists", "search", "playlists/"];
+pub(crate) const DEFAULT_NCSPOT_ONLY_GET_ENDPOINTS: &[&str] = &["me/playlists", "playlists/"];
 
 use anyhow::{anyhow, Result};
 use config_parser2::{config_parser_impl, ConfigParse, ConfigParser};

spotify_player/src/event/page.rs

@@ -215,8 +215,16 @@ fn handle_key_sequence_for_search_page(
             return match &key_sequence.keys[0] {
                 Key::None(crossterm::event::KeyCode::Enter) => {
                     if !line_input.is_empty() {
-                        *current_query = line_input.get_text();
-                        client_pub.send(ClientRequest::Search(line_input.get_text()))?;
+                        let query = line_input.get_text();
+                        current_query.clone_from(&query);
+
+                        if state.data.write().caches.begin_search(&query) {
+                            if let Err(err) = client_pub.send(ClientRequest::Search(query.clone()))
+                            {
+                                state.data.write().caches.fail_search(query);
+                                return Err(err.into());
+                            }
+                        }
                     }
                     Ok(true)
                 }
@@ -247,7 +255,7 @@ fn handle_key_sequence_for_search_page(
     };
 
     let data = state.data.read();
-    let search_results = data.caches.search.get(current_query);
+    let search_results = data.caches.search_results(current_query);
 
     match focus_state {
         SearchFocusState::Input => anyhow::bail!("user's search input should be handled before"),

spotify_player/src/state/data.rs

@@ -45,10 +45,17 @@ pub struct UserData {
     pub saved_tracks: HashMap<String, Track>,
 }
 
+#[derive(Debug)]
+pub enum SearchCacheEntry {
+    Loading,
+    Ready(SearchResults),
+    Failed,
+}
+
 /// the application's in-memory caches
 pub struct MemoryCaches {
     pub context: ttl_cache::TtlCache<String, Context>,
-    pub search: ttl_cache::TtlCache<String, SearchResults>,
+    pub search: ttl_cache::TtlCache<String, SearchCacheEntry>,
     pub lyrics: ttl_cache::TtlCache<String, Option<Lyrics>>,
     pub genres: ttl_cache::TtlCache<String, Vec<String>>,
     #[cfg(feature = "image")]
@@ -73,6 +80,42 @@ impl MemoryCaches {
             images: ttl_cache::TtlCache::new(64),
         }
     }
+
+    pub fn begin_search(&mut self, query: &str) -> bool {
+        let should_begin = matches!(
+            self.search.get(query),
+            None | Some(SearchCacheEntry::Failed)
+        );
+
+        if should_begin {
+            self.search.insert(
+                query.to_string(),
+                SearchCacheEntry::Loading,
+                *TTL_CACHE_DURATION,
+            );
+        }
+
+        should_begin
+    }
+
+    pub fn complete_search(&mut self, query: String, results: SearchResults) {
+        self.search
+            .insert(query, SearchCacheEntry::Ready(results), *TTL_CACHE_DURATION);
+    }
+
+    pub fn fail_search(&mut self, query: String) {
+        if matches!(self.search.get(&query), Some(SearchCacheEntry::Loading)) {
+            self.search
+                .insert(query, SearchCacheEntry::Failed, *TTL_CACHE_DURATION);
+        }
+    }
+
+    pub fn search_results(&self, query: &str) -> Option<&SearchResults> {
+        match self.search.get(query) {
+            Some(SearchCacheEntry::Ready(results)) => Some(results),
+            _ => None,
+        }
+    }
 }
 
 impl AppData {
@@ -242,3 +285,43 @@ where
         None
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::{MemoryCaches, SearchCacheEntry, SearchResults};
+
+    #[test]
+    fn search_lifecycle_suppresses_duplicate_requests() {
+        let mut caches = MemoryCaches::new();
+
+        assert!(caches.begin_search("query"));
+        assert!(matches!(
+            caches.search.get("query"),
+            Some(SearchCacheEntry::Loading)
+        ));
+        assert!(!caches.begin_search("query"));
+
+        caches.complete_search("query".to_string(), SearchResults::default());
+
+        assert!(caches.search_results("query").is_some());
+        assert!(!caches.begin_search("query"));
+    }
+
+    #[test]
+    fn failed_search_can_be_retried() {
+        let mut caches = MemoryCaches::new();
+
+        assert!(caches.begin_search("query"));
+        caches.fail_search("query".to_string());
+
+        assert!(matches!(
+            caches.search.get("query"),
+            Some(SearchCacheEntry::Failed)
+        ));
+        assert!(caches.begin_search("query"));
+        assert!(matches!(
+            caches.search.get("query"),
+            Some(SearchCacheEntry::Loading)
+        ));
+    }
+}

spotify_player/src/ui/page.rs

@@ -6,7 +6,10 @@ use std::{
 use chrono_humanize::HumanTime;
 use ratatui::text::Line;
 
-use crate::{state::Episode, utils::format_duration};
+use crate::{
+    state::{Episode, SearchCacheEntry},
+    utils::format_duration,
+};
 
 use super::{
     config, utils, utils::construct_and_render_block, Album, Alignment, Artist, ArtistFocusState,
@@ -57,16 +60,51 @@ pub fn render_search_page(
         _ => return,
     };
 
-    let search_results = data.caches.search.get(current_query);
+    let search_entry = data.caches.search.get(current_query);
+    let search_results = data.caches.search_results(current_query);
 
     // 2. Construct the page's layout
     let rect = construct_and_render_block("Search", &ui.theme, Borders::ALL, frame, rect);
 
+    let status = match search_entry {
+        Some(SearchCacheEntry::Loading) => Some("Loading..."),
+        Some(SearchCacheEntry::Failed) => Some("Search failed"),
+        _ => None,
+    };
+
     // search input's layout
-    let chunks = Layout::vertical([Constraint::Length(1), Constraint::Fill(0)]).split(rect);
-    let search_input_rect = chunks[0];
+    let input_height = if status.is_some() { 2 } else { 1 };
+    let chunks =
+        Layout::vertical([Constraint::Length(input_height), Constraint::Fill(0)]).split(rect);
+    let search_input_rect = if status.is_some() {
+        construct_and_render_block("", &ui.theme, Borders::BOTTOM, frame, chunks[0])
+    } else {
+        chunks[0]
+    };
     let rect = chunks[1];
 
+    let PageState::Search { line_input, .. } = ui.current_page_mut() else {
+        return;
+    };
+    frame.render_widget(
+        line_input.widget(is_active && focus_state == SearchFocusState::Input),
+        search_input_rect,
+    );
+
+    if let Some(status) = status {
+        let status_rect = Layout::vertical([
+            Constraint::Fill(1),
+            Constraint::Length(1),
+            Constraint::Fill(1),
+        ])
+        .split(rect)[1];
+        frame.render_widget(
+            Paragraph::new(status).alignment(Alignment::Center),
+            status_rect,
+        );
+        return;
+    }
+
     // track/album/artist/playlist/show/episode search results layout
     let chunks = match ui.orientation {
         // 1x6
@@ -226,19 +264,11 @@ pub fn render_search_page(
     // 4. Render the page's widgets
     // Need mutable access to the list/table states stored inside the page state for rendering.
     let PageState::Search {
-        state: page_state,
-        line_input,
-        ..
+        state: page_state, ..
     } = ui.current_page_mut()
     else {
         return;
     };
-
-    // Render the query input box
-    frame.render_widget(
-        line_input.widget(is_active && focus_state == SearchFocusState::Input),
-        search_input_rect,
-    );
     utils::render_list_window(
         frame,
         track_list,