[GH-ISSUE #382] Make search function generic over search types #118

New issue

Closed

opened 2026-02-27 20:23:14 +03:00 by kerem · 1 comment

kerem commented

2026-02-27 20:23:14 +03:00

Owner

Originally created by @alevani on GitHub (Jan 10, 2023).
Original GitHub issue: https://github.com/ramsayleung/rspotify/issues/382

Using 0.11.6

I would like to be able to deserialise the search result into a specific search type. Right now the search() function on ClientCredsSpotify returns a SearchResult by default.

Motivations:
The SearchResult we get back is an enum that you can only extract data from with pattern matching or if-let statement.

if let SearchResult::Playlists(playlist) = result {
        // do something..
}

If you know what search result you expect, this feels like an extra step

I have seen that you have some unused types such as SearchPlaylists. I would suggest making search() generic over these types to be able to extract the data more easily. I have been able to go a bit of the way

 async fn search<T=SearchResult>(
        &self,
        q: &str,
        _type: SearchType,
        market: Option<Market>,
        include_external: Option<IncludeExternal>,
        limit: Option<u32>,
        offset: Option<u32>,
    ) -> ClientResult<T>
    where T: DeserializeOwned
    {
        let limit = limit.map(|s| s.to_string());
        let offset = offset.map(|s| s.to_string());
        let params = build_map([
            ("q", Some(q)),
            ("type", Some(_type.into())),
            ("market", market.map(Into::into)),
            ("include_external", include_external.map(Into::into)),
            ("limit", limit.as_deref()),
            ("offset", offset.as_deref()),
        ]);

        let result = self.endpoint_get("search", &params).await?;
        convert_result(&result)
    }

This works well when specifying a search result type, or when implicitly inferring the type. However, I think the search() function should still return a SearchResults type if no types were inferred by ::<>. My view is that it should default to SearchResult when called like

let result = spotify
    .search(&req.query, req.search_type, None, None, None, None)
    .await?;

Or return T when use like this

let result = spotify
    .search::<SearchPlaylists>(&req.query, req.search_type, None, None, None, None)
    .await?;

Unfortunately, I haven't found a good way to infer the default generic type, and fail to find a neat solution to avoid deserialisation problems.

Describe alternatives you've considered
Changing the SearchResult to a struct with optional fields makes you able to extract the values:

/// Search result of any kind
#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)]
pub struct SearchResult {
    #[serde(rename = "playlists")]
    pub playlists: Option<Page<SimplifiedPlaylist>>,
    #[serde(rename = "albums")]
    pub albums: Option<Page<SimplifiedAlbum>>,
    #[serde(rename = "artists")]
    pub artists: Option<Page<FullArtist>>,
    #[serde(rename = "tracks")]
    pub tracks: Option<Page<FullTrack>>,
    #[serde(rename = "shows")]
    pub shows: Option<Page<SimplifiedShow>>,
    #[serde(rename = "episodes")]
    pub episodes: Option<Page<SimplifiedEpisode>>,
}

But doesn't seem like the most Rust way to do things

Originally created by @alevani on GitHub (Jan 10, 2023). Original GitHub issue: https://github.com/ramsayleung/rspotify/issues/382 Using 0.11.6 I would like to be able to deserialise the search result into a specific search type. Right now the `search()` function on `ClientCredsSpotify` returns a `SearchResult` by default. **Motivations**: The `SearchResult` we get back is an `enum` that you can only extract data from with pattern matching or `if-let` statement. ```Rust if let SearchResult::Playlists(playlist) = result { // do something.. } ``` If you know what search result you expect, this feels like an extra step I have seen that you have some unused types such as `SearchPlaylists`. I would suggest making `search()` generic over these types to be able to extract the data more easily. I have been able to go a bit of the way ```Rust async fn search<T=SearchResult>( &self, q: &str, _type: SearchType, market: Option<Market>, include_external: Option<IncludeExternal>, limit: Option<u32>, offset: Option<u32>, ) -> ClientResult<T> where T: DeserializeOwned { let limit = limit.map(|s| s.to_string()); let offset = offset.map(|s| s.to_string()); let params = build_map([ ("q", Some(q)), ("type", Some(_type.into())), ("market", market.map(Into::into)), ("include_external", include_external.map(Into::into)), ("limit", limit.as_deref()), ("offset", offset.as_deref()), ]); let result = self.endpoint_get("search", &params).await?; convert_result(&result) } ``` This works well when specifying a search result type, or when implicitly inferring the type. However, I think the `search()` function should still return a `SearchResults` type if no types were inferred by `::<>`. My view is that it should default to `SearchResult` when called like ```Rust let result = spotify .search(&req.query, req.search_type, None, None, None, None) .await?; ``` Or return T when use like this ```Rust let result = spotify .search::<SearchPlaylists>(&req.query, req.search_type, None, None, None, None) .await?; ``` Unfortunately, I haven't found a good way to infer the default generic type, and fail to find a neat solution to avoid deserialisation problems. **Describe alternatives you've considered** Changing the `SearchResult` to a struct with optional fields makes you able to extract the values: ```Rust /// Search result of any kind #[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)] pub struct SearchResult { #[serde(rename = "playlists")] pub playlists: Option<Page<SimplifiedPlaylist>>, #[serde(rename = "albums")] pub albums: Option<Page<SimplifiedAlbum>>, #[serde(rename = "artists")] pub artists: Option<Page<FullArtist>>, #[serde(rename = "tracks")] pub tracks: Option<Page<FullTrack>>, #[serde(rename = "shows")] pub shows: Option<Page<SimplifiedShow>>, #[serde(rename = "episodes")] pub episodes: Option<Page<SimplifiedEpisode>>, } ``` But doesn't seem like the most Rust way to do things

kerem

2026-02-27 20:23:14 +03:00

closed this issue
added the
enhancement

Stale
labels

kerem commented

2026-02-27 20:23:14 +03:00

Author

Owner

@github-actions[bot] commented on GitHub (Jun 24, 2023):

Message to comment on stale issues. If none provided, will not mark issues stale

@github-actions[bot] commented on GitHub (Jun 24, 2023): Message to comment on stale issues. If none provided, will not mark issues stale

kerem referenced this issue

2026-02-27 20:23:59 +03:00

[PR #119] [MERGED] Rename environmental variables with prefix RSPOTIFY_ #255