[GH-ISSUE #203] Endpoints with lists of Ids don't accept multiple of them #64

New issue

Closed

opened 2026-02-27 20:22:54 +03:00 by kerem · 5 comments

kerem commented 2026-02-27 20:22:54 +03:00

Owner

Copy link

Originally created by @marioortizmanero on GitHub (Apr 20, 2021).
Original GitHub issue: https://github.com/ramsayleung/rspotify/issues/203

Describe the bug

I just realized that Ids don't really work in lists. We'd have to use Box, I think, to support for example items of both episodes and tracks. Otherwise, with a snippet like this:

use rspotify::client::SpotifyBuilder;
use rspotify::model::{EpisodeId, TrackId};
use rspotify::oauth2::CredentialsBuilder;

#[tokio::main]
async fn main() {
    env_logger::init();

    let creds = CredentialsBuilder::from_env().build().unwrap();
    let mut spotify = SpotifyBuilder::default()
        .credentials(creds)
        .build()
        .unwrap();

    spotify.request_client_token().await.unwrap();

    let uris = &[
        EpisodeId::from_uri("spotify:episode:7zC6yTmY67f32WOShs0qbb").unwrap(),
        EpisodeId::from_uri("spotify:episode:3hCrwz19QlXY0myYh0XgmL").unwrap(),
        TrackId::from_uri("spotify:track:0hAMkY2kwdXPPDfQ1e3BmJ").unwrap()
    ];
    let resp = spotify.start_uris_playback(uris, None, None, None).await;

    println!("Response: {:#?}", resp);
}

You get:

error[E0308]: mismatched types
  --> examples/broken.rs:20:9
   |
20 |         TrackId::from_uri("spotify:track:0hAMkY2kwdXPPDfQ1e3BmJ").unwrap()
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected enum `rspotify::model::idtypes::Episode`, found enum `rspotify::model::idtypes::
Track`
   |
   = note: expected reference `&Id<rspotify::model::idtypes::Episode>`
              found reference `&Id<rspotify::model::idtypes::Track>`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0308`.
error: could not compile `rspotify`

To learn more, run the command again with --verbose.

This is also relatively dangerous because if the user tries to initialize the Ids with Id::from_uri instead of the specific EpisodeId:.from_uri or TrackId::from_uri, they will all be converted to the type of the first one in the list and fail at the unwrap.

Originally created by @marioortizmanero on GitHub (Apr 20, 2021). Original GitHub issue: https://github.com/ramsayleung/rspotify/issues/203 **Describe the bug** I just realized that Ids don't really work in lists. We'd have to use `Box`, I think, to support for example items of both episodes and tracks. Otherwise, with a snippet like this: ```rust use rspotify::client::SpotifyBuilder; use rspotify::model::{EpisodeId, TrackId}; use rspotify::oauth2::CredentialsBuilder; #[tokio::main] async fn main() { env_logger::init(); let creds = CredentialsBuilder::from_env().build().unwrap(); let mut spotify = SpotifyBuilder::default() .credentials(creds) .build() .unwrap(); spotify.request_client_token().await.unwrap(); let uris = &[ EpisodeId::from_uri("spotify:episode:7zC6yTmY67f32WOShs0qbb").unwrap(), EpisodeId::from_uri("spotify:episode:3hCrwz19QlXY0myYh0XgmL").unwrap(), TrackId::from_uri("spotify:track:0hAMkY2kwdXPPDfQ1e3BmJ").unwrap() ]; let resp = spotify.start_uris_playback(uris, None, None, None).await; println!("Response: {:#?}", resp); } ``` You get: ``` error[E0308]: mismatched types --> examples/broken.rs:20:9 | 20 | TrackId::from_uri("spotify:track:0hAMkY2kwdXPPDfQ1e3BmJ").unwrap() | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected enum `rspotify::model::idtypes::Episode`, found enum `rspotify::model::idtypes:: Track` | = note: expected reference `&Id<rspotify::model::idtypes::Episode>` found reference `&Id<rspotify::model::idtypes::Track>` error: aborting due to previous error For more information about this error, try `rustc --explain E0308`. error: could not compile `rspotify` To learn more, run the command again with --verbose. ``` This is also relatively dangerous because if the user tries to initialize the Ids with `Id::from_uri` instead of the specific `EpisodeId:.from_uri` or `TrackId::from_uri`, they will all be converted to the type of the first one in the list and fail at the `unwrap`.

kerem 2026-02-27 20:22:54 +03:00

• closed this issue
• added the
  help wanted
  bug
  labels

kerem commented 2026-02-27 20:22:55 +03:00

Author

Owner

Copy link

@marioortizmanero commented on GitHub (Jul 14, 2021):

The design for IDs is a bit wrong now that I've tried to actually use it... The thing is that you don't always know at compile-time what kind of ID is being used. So for example, this is impossible to do right now:

let id = match x {
    Track(t) => TrackId::from_uri(&t.uri).unwrap(),
    Episode(t) => EpisodeId::from_uri(&t.uri).unwrap(),
};

client.start_uris_playback(id, /* ... */).await.unwrap();

Because the arms of the match statement have different types.

Any opinions, @kstep ? Sorry for bringing this up so late :(

<!-- gh-comment-id:880116887 --> @marioortizmanero commented on GitHub (Jul 14, 2021): The design for IDs is a bit wrong now that I've tried to actually use it... The thing is that you don't always know at compile-time what kind of ID is being used. So for example, this is impossible to do right now: ```rust let id = match x { Track(t) => TrackId::from_uri(&t.uri).unwrap(), Episode(t) => EpisodeId::from_uri(&t.uri).unwrap(), }; client.start_uris_playback(id, /* ... */).await.unwrap(); ``` Because the arms of the `match` statement have different types. Any opinions, @kstep ? Sorry for bringing this up so late :(

kerem commented 2026-02-27 20:22:55 +03:00

Author

Owner

Copy link

@marioortizmanero commented on GitHub (Aug 24, 2021):

Here's my reasoning behind all of this, step by step so that anyone else can understand it as well. If I'm wrong at any point please let me know!

So we currently have an Id struct that is generic over its type, Id<T>. And for functions like start_uris_playback, we need T to take different values in a Vec/equivalent (as a simplification over the impl IntoIterator<...> thing). We also want to be able to have T be checked at runtime.

• The first idea that comes to my mind is dynamic dispatching; Id<dyn T> might work.

  • In order for Ìd<dyn T> to work, where T: IdType:
    1. IdType must be object safe, so that dyn IdType is possible
    2. Id should be defined with pub struct Id<T: IdType + ?Sized>, so that it may take a dyn IdType directly (just like how Box works). Otherwise we'd have to use Id<Box<dyn IdType>>, which wouldn't work either because Box<dyn IdType> doesn't implement IdType.
  • Problems:
    1. Those requirements are basically what a Box<T> is. But Box<T> performs a memory allocation, so Id<T> wouldn't be a zero-cost abstraction at all.
    2. The trait IdType simply contains const TYPE: Type; in its definition. Which means IdType is not object-safe at all. We would have to make IdType a trait that initializes T and then calls get_type(&self) -> Type, which would be type-safe.
  • In summary, I think we're walking in circles with Id<T>.

• Another solution would be what I've already implemented in #241. Simply add an Unknown variant that doesn't check types at all. You can then have Vec<Id<Unknown>>, which would allow any kind of ID to be passed as its generic parameter. Though this sounds like a cool solution for being able to have IDs defined at runtime (Id<Unknown>), with it you can't enforce the type safety Id<T> is supposedly useful for over a &str.

  While this is my favorite solution so far, I think there must be something else that's better.

• Based on the last solution, we could turn traits like PlayableIdType into types (say Playable) just like Artist or Track. One could then convert Id<Track> into Id<Playable> with id.into() in order to pass it as a parameter. That way no generics are involved at all over T; it's always going to be a known type.

  The main problem of this solution is that there is some data loss as well. For example the Display implementation of Id<T> uses T::TYPE to print its URI. What exactly would happen with Id<Playable>? It would print the URI spotify:playable:xxxx, which is not a valid URI.

• A different approach would be to scratch Id<T> and just use ArtistId and similars directly. What I mean is that we wouldn't have a single type for IDs called Id<T> that implements its logic. We could make ArtistId and similars, and implement the trait the same way clients work. The main component would be a trait that implements all the logic, say Id, and a struct for each type (ArtistId, etc) that implements the trait. With this one could be generic over Id directly, have Box<dyn Id> for runtime usage, and Vec<Box>` for multiple values, which is exactly what we need.

  The main problems are:

  1. We can't use type inference to our benefit; Id::from_uri can't work with this solution; ArtistId::from_uri would have to be used explicitly.
  2. We need the trait Id always available when using IDs; we'd have to add it to the prelude. Considering the user needs to import the prelude to use the clients anyway I don't think it's a big deal

  So this solution is basically the same as how clients work. We have a big trait that does everything, and we let Rust "clone" the code for each of our implementations. It's also basically the newtype pattern, but with a common trait for all the newtypes.

Sorry for the long text lol. Just brainstorming. I'll try to implement my favorite solutions in PRs and showcase how they work.

<!-- gh-comment-id:904486109 --> @marioortizmanero commented on GitHub (Aug 24, 2021): Here's my reasoning behind all of this, step by step so that anyone else can understand it as well. If I'm wrong at any point please let me know! So we currently have an `Id` struct that is generic over its type, `Id<T>`. And for functions like `start_uris_playback`, we need `T` to take different values in a `Vec`/equivalent (as a simplification over the `impl IntoIterator<...>` thing). We also want to be able to have `T` be checked at runtime. * The first idea that comes to my mind is dynamic dispatching; `Id<dyn T>` might work. * In order for `Ìd<dyn T>` to work, where `T: IdType`: 1. `IdType` must be object safe, so that `dyn IdType` is possible 2. `Id` should be defined with `pub struct Id<T: IdType + ?Sized>`, so that it may take a `dyn IdType` directly (just like how `Box` works). Otherwise we'd have to use `Id<Box<dyn IdType>>`, which wouldn't work either because `Box<dyn IdType>` doesn't implement `IdType`. * Problems: 1. Those requirements are basically what a `Box<T>` is. But `Box<T>` performs a memory allocation, so `Id<T>` wouldn't be a zero-cost abstraction at all. 2. The trait `IdType` simply contains `const TYPE: Type;` in its definition. Which means **`IdType` is not object-safe at all**. We would have to make `IdType` a trait that initializes `T` and then calls `get_type(&self) -> Type`, which *would* be type-safe. * In summary, I think we're walking in circles with `Id<T>`. * Another solution would be what I've already implemented in #241. Simply add an `Unknown` variant that doesn't check types at all. You can then have `Vec<Id<Unknown>>`, which would allow any kind of ID to be passed as its generic parameter. Though this sounds like a cool solution for being able to have IDs defined at runtime (`Id<Unknown>`), with it you can't enforce the type safety `Id<T>` is supposedly useful for over a `&str`. While this is my favorite solution so far, I think there must be something else that's better. * Based on the last solution, we could turn **traits** like `PlayableIdType` into **types** (say `Playable`) just like `Artist` or `Track`. One could then convert `Id<Track>` into `Id<Playable>` with `id.into()` in order to pass it as a parameter. That way no generics are involved at all over `T`; it's always going to be a known type. The main problem of this solution is that there is some data loss as well. For example the `Display` implementation of `Id<T>` uses `T::TYPE` to print its URI. What exactly would happen with `Id<Playable>`? It would print the URI `spotify:playable:xxxx`, which is not a valid URI. * A different approach would be to scratch `Id<T>` and just use `ArtistId` and similars directly. What I mean is that we wouldn't have a single type for IDs called `Id<T>` that implements its logic. We could make `ArtistId` and similars, and implement the trait the same way clients work. The main component would be a trait that implements all the logic, say `Id`, and a struct for each type (`ArtistId`, etc) that implements the trait. With this one could be generic over `Id` directly, have `Box<dyn Id> for runtime usage, and `Vec<Box<dyn Id>>` for multiple values, which is exactly what we need. The main problems are: 1. We can't use type inference to our benefit; `Id::from_uri` can't work with this solution; `ArtistId::from_uri` would have to be used explicitly. 2. We need the trait `Id` always available when using IDs; we'd have to add it to the prelude. Considering the user needs to import the prelude to use the clients anyway I don't think it's a big deal So this solution is basically the same as how clients work. We have a big trait that does everything, and we let Rust "clone" the code for each of our implementations. It's also basically the newtype pattern, but with a common trait for all the newtypes. Sorry for the long text lol. Just brainstorming. I'll try to implement my favorite solutions in PRs and showcase how they work.

kerem commented 2026-02-27 20:22:55 +03:00

Author

Owner

Copy link

@marioortizmanero commented on GitHub (Aug 24, 2021):

Ugh. Unfortunately literally none of these ideas work. This is what happens when trying to implement From to convert any Id<T> into Id<Unknown>:

error[E0119]: conflicting implementations of trait `std::convert::From<&idtypes::Id<idtypes::Unknown>>` for type `&idtypes::Id<idtypes::Unknown>`
   --> rspotify-model/src/idtypes.rs:152:1
    |
152 | impl<T> From<&Id<Unknown>> for &Id<T> {
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: conflicting implementation in crate `core`:
            - impl<T> From<T> for T;

For more information about this error, try `rustc --explain E0119`.
error: could not compile `rspotify-model` due to previous error

So in that case the user would have to do:

let id1 = ArtistId::from_uri("...").unwrap();
let id2 = UnknownId::from_id(id1.id()).unwrap();

Which is way uglier and has more overhead than:

let id1 = ArtistId::from_uri("...").unwrap();
let id2: UnknownId = id1.into();

<!-- gh-comment-id:904968769 --> @marioortizmanero commented on GitHub (Aug 24, 2021): Ugh. Unfortunately literally none of these ideas work. This is what happens when trying to implement `From` to convert any `Id<T>` into `Id<Unknown>`: ``` error[E0119]: conflicting implementations of trait `std::convert::From<&idtypes::Id<idtypes::Unknown>>` for type `&idtypes::Id<idtypes::Unknown>` --> rspotify-model/src/idtypes.rs:152:1 | 152 | impl<T> From<&Id<Unknown>> for &Id<T> { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: conflicting implementation in crate `core`: - impl<T> From<T> for T; For more information about this error, try `rustc --explain E0119`. error: could not compile `rspotify-model` due to previous error ``` So in that case the user would have to do: ```rust let id1 = ArtistId::from_uri("...").unwrap(); let id2 = UnknownId::from_id(id1.id()).unwrap(); ``` Which is way uglier and has more overhead than: ```rust let id1 = ArtistId::from_uri("...").unwrap(); let id2: UnknownId = id1.into(); ```

kerem commented 2026-02-27 20:22:55 +03:00

Author

Owner

Copy link

@ramsayleung commented on GitHub (Aug 25, 2021):

It turns out that the zero cost abstractions isn't that easy to reach out, it comes with unexpected obstacles :)

<!-- gh-comment-id:905113707 --> @ramsayleung commented on GitHub (Aug 25, 2021): It turns out that the zero cost abstractions isn't that easy to reach out, it comes with unexpected obstacles :)

kerem commented 2026-02-27 20:22:55 +03:00

Author

Owner

Copy link

@marioortizmanero commented on GitHub (Aug 25, 2021):

Yeah, it's very hard to mix both compile time and runtime. I'm trying a new approach that consists of mixing two of the ideas I explained here. I'll try to have multiple types for Id instead of a single Id<T> so that there's no generics instead, AND I'll add UnknownId/similars as IDs as well. The difference is that since the Id types are different, it is possible to implement conversions from them (instead of using dyn, which is impossible given the Id trait).

I'll explain more if I get it right, though it still has a couple drawbacks.

<!-- gh-comment-id:905273595 --> @marioortizmanero commented on GitHub (Aug 25, 2021): Yeah, it's very hard to mix both compile time and runtime. I'm trying a new approach that consists of mixing two of the ideas I explained here. I'll try to have multiple types for `Id` instead of a single `Id<T>` so that there's no generics instead, AND I'll add `UnknownId`/similars as IDs as well. The difference is that since the `Id` types are different, it is possible to implement conversions from them (instead of using `dyn`, which is impossible given the `Id` trait). I'll explain more if I get it right, though it still has a couple drawbacks.

Sign in to join this conversation.

No Branch/Tag specified

Branches Tags

master

ramsay/upgrade-0.16.1

dependabot/cargo/sha2-0.11.0

ramsay/upgrade-to-v0.16

ramsay/deprecate-removed-renamed-field

ramsay/migrate-to-new-endpoint

ramsay/deprecate-removed-endpoints

dependabot/cargo/strum-0.28.0

dependabot/cargo/reqwest-middleware-0.5.1

dependabot/cargo/reqwest-0.13.1

ramsay/upgrade-to-0.15.3

ramsay/upgrade-to-0.15.2

dependabot/cargo/ureq-3.1.2

ramsay/upgrade-dependencies

ramsay/upgrade-to-0.15.1

ramsay/fix-malformed-json

ramsay/upgrade-to-v0.15.0

ramsay/allow-enum-unknown-variant

ramsay/upgrade-to-0.14

ramsay/add_tcplistern_for_callback

ramsay/fix-clippy-error

ramsay/update-doc

ramsay/fix-endless-sequence

ramsay/publish-crate-workflow-test

ramsay/fix-pkce-refresh-token

ramsay/pkce_auth_refresh_token

ramsay_add_test_for_collectionyourepisodes

better-webapp

ramsay_fix_token_expired

percent-search

refresh_token_rwlock

ramsay_convert_result_inside_endpoint

streaming

v0.16.1

v0.16.0

v0.15.3

v0.15.2

v0.15.1

v0.15.0

v0.14.0

v0.13.3

v0.13.2

v0.13.1

v0.13.0

v0.12.0

v0.11.7

v0.11.6

v0.10.0

v0.11.5

v0.11.5.0

v0.11.4

v0.11.3

v0.11.2

v0.11.1

v0.11.0

v0.11.5.1

v0.9

v0.9-alpha

Labels

Clear labels   

Stale

  

bug

  

discussion

  

enhancement

  

good first issue

  

good first issue

  

help wanted

  

pull-request

Mirrored from GitHub Pull Request

  

question

No labels

Stale

bug

discussion

enhancement

good first issue

good first issue

help wanted

pull-request

question

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

starred/rspotify#64

No description provided.