[GH-ISSUE #218] Upgrading to v0.11: questions and support #72

Closed
opened 2026-02-27 20:22:56 +03:00 by kerem · 33 comments
Owner

Originally created by @marioortizmanero on GitHub (Jun 19, 2021).
Original GitHub issue: https://github.com/ramsayleung/rspotify/issues/218

Rspotify's v0.11 update introduces a lot of breaking changes. Thus, I think an issue like this might help with upgrading.

First, check out the upgrading guide in the changelog: CHANGELOG.md. If you have any questions or need any help please let us know!

Originally created by @marioortizmanero on GitHub (Jun 19, 2021). Original GitHub issue: https://github.com/ramsayleung/rspotify/issues/218 Rspotify's v0.11 update introduces a lot of breaking changes. Thus, I think an issue like this might help with upgrading. First, check out the upgrading guide in the changelog: [`CHANGELOG.md`](https://github.com/ramsayleung/rspotify/blob/master/CHANGELOG.md). If you have any questions or need any help please let us know!
kerem 2026-02-27 20:22:56 +03:00
Author
Owner

@marioortizmanero commented on GitHub (Jun 19, 2021):

Do note that this version hasn't been released yet, but those using the master branch might consider this useful in the meanwhile.

<!-- gh-comment-id:864392320 --> @marioortizmanero commented on GitHub (Jun 19, 2021): Do note that this version hasn't been released yet, but those using the `master` branch might consider this useful in the meanwhile.
Author
Owner

@flip1995 commented on GitHub (Oct 5, 2021):

Is there an ETA when 0.11 will get released? What issues are blocking the release and can I help to address those?

<!-- gh-comment-id:934213602 --> @flip1995 commented on GitHub (Oct 5, 2021): Is there an ETA when 0.11 will get released? What issues are blocking the release and can I help to address those?
Author
Owner

@marioortizmanero commented on GitHub (Oct 5, 2021):

Hi @flip1995! The problem is that I've recently started Uni classes again and I haven't had time to push the release, but it's really really close. What's pending is:

  • Automatic reauthentication: whenever @ramsayleung has time, he has to remake the #224 PR as #255 indicates. Not sure if he's already working on it or not.
  • Merge the currently open PRs, which should be really quick & easy:
    • #257, ready for merging I think
    • #258, I have to apply a few fixes before ramsay can review & merge it
    • #261, requires a review
  • #221, which is not trivial. Unlike I thought, it is possible to fix, but it requires updating the maybe_async crate as indicated by https://github.com/fMeow/maybe-async-rs/issues/6. I already provided a base design in the latest comment, and I was waiting for opinions before implementing it, but the author is probably busy.
    The thing is that this issue is going to take a considerable amount of time to fix, and it'll single-handedly block the rspotify release at some point. I could just skip it and release the new version before fixing it, but (1) it will introduce breaking changes, and most importantly, (2) those who have rspotify twice in their dependency graph with async and sync will break. Not sure how important these two items are as opposed to releasing the new version finally, honestly.

If you can help on any of these go ahead (just let us know to avoid repeated work), and if you have any questions do ask them. The most fun one is probably fixing maybe_async because writing proc macros is pretty cool imo.

<!-- gh-comment-id:934657454 --> @marioortizmanero commented on GitHub (Oct 5, 2021): Hi @flip1995! The problem is that I've recently started Uni classes again and I haven't had time to push the release, but it's *really really close*. What's pending is: * Automatic reauthentication: whenever @ramsayleung has time, he has to remake the #224 PR as #255 indicates. Not sure if he's already working on it or not. * Merge the currently open PRs, which should be really quick & easy: * #257, ready for merging I think * #258, I have to apply a few fixes before ramsay can review & merge it * #261, requires a review * #221, which is not trivial. Unlike I thought, it *is* possible to fix, but it requires updating the `maybe_async` crate as indicated by https://github.com/fMeow/maybe-async-rs/issues/6. I already provided [a base design in the latest comment](https://github.com/fMeow/maybe-async-rs/issues/6#issuecomment-924961897), and I was waiting for opinions before implementing it, but the author is probably busy. The thing is that this issue is going to take a considerable amount of time to fix, and it'll single-handedly block the rspotify release at some point. I could just skip it and release the new version before fixing it, but (1) it will introduce breaking changes, and most importantly, (2) those who have rspotify twice in their dependency graph with async and sync will break. Not sure how important these two items are as opposed to releasing the new version finally, honestly. If you can help on any of these go ahead (just let us know to avoid repeated work), and if you have any questions do ask them. The most fun one is probably fixing `maybe_async` because writing proc macros is pretty cool imo.
Author
Owner

@flip1995 commented on GitHub (Oct 6, 2021):

Thanks for the summary! Sadly I'm also a bit low on time, so I can't make any promises. I may have some time to look into the async issue in about 2 weeks.

<!-- gh-comment-id:935691050 --> @flip1995 commented on GitHub (Oct 6, 2021): Thanks for the summary! Sadly I'm also a bit low on time, so I can't make any promises. I may have some time to look into the async issue in about 2 weeks.
Author
Owner

@ramsayleung commented on GitHub (Oct 13, 2021):

Since all PRs have been merged, I think, we are ready to release a new version crate or a pre-version crate now :)

PS: ooh, it reminds me that there is something left to be updated, some docs are not up-to-date, for example, the diagram of trait hierarchy. I should update them.

<!-- gh-comment-id:941831990 --> @ramsayleung commented on GitHub (Oct 13, 2021): Since all PRs have been merged, I think, we are ready to release a new version crate or a pre-version crate now :) PS: ooh, it reminds me that there is something left to be updated, some docs are not up-to-date, for example, the diagram of trait hierarchy. I should update them.
Author
Owner

@marioortizmanero commented on GitHub (Oct 13, 2021):

I think once we merge your PR we're ready, as long as everyone is OK leaving #221 for a future version.

<!-- gh-comment-id:942017530 --> @marioortizmanero commented on GitHub (Oct 13, 2021): I think once we merge your PR we're ready, as long as everyone is OK leaving #221 for a future version.
Author
Owner

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

Shall we? Do you want to do the honors, Ramsay?

<!-- gh-comment-id:943367453 --> @marioortizmanero commented on GitHub (Oct 14, 2021): Shall we? Do you want to do the honors, Ramsay?
Author
Owner

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

Three attempts later, we've finally released v0.11.2! The changes from v0.11.0 to v0.11.2 are just fixes for the builds in docs.rs. https://docs.rs/rspotify/0.11.2/rspotify/

Cheers!

<!-- gh-comment-id:943549038 --> @marioortizmanero commented on GitHub (Oct 14, 2021): Three attempts later, we've finally released v0.11.2! The changes from v0.11.0 to v0.11.2 are just fixes for the builds in `docs.rs`. https://docs.rs/rspotify/0.11.2/rspotify/ Cheers!
Author
Owner

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

Also, here's my (super long) blog post about the new version: https://nullderef.com/blog/web-api-client/. It tells the whole story and showcases some of the cool features we've added. Thanks again to everyone who made this release possible ❤️

<!-- gh-comment-id:943551469 --> @marioortizmanero commented on GitHub (Oct 14, 2021): Also, here's my (super long) blog post about the new version: https://nullderef.com/blog/web-api-client/. It tells the whole story and showcases some of the cool features we've added. Thanks again to everyone who made this release possible ❤️
Author
Owner

@aome510 commented on GitHub (Oct 14, 2021):

@marioortizmanero wow finally! Nice work, everyone who is involved 🎉

I have been waiting for v0.11.0 since I first started spotify-player. Currently, I have to make a work around with the blocking APIs to be able to use tokio v1. I guess I can start a big migration now 😅

Bookmarked the blog post! I haven't noticed you're the guy behind nullderef until now. Really enjoyed your previous blogs about Rust.

<!-- gh-comment-id:943622577 --> @aome510 commented on GitHub (Oct 14, 2021): @marioortizmanero wow finally! Nice work, everyone who is involved 🎉 I have been waiting for `v0.11.0` since I first started [`spotify-player`](https://github.com/aome510/spotify-player). Currently, I have to make a work around with the blocking APIs to be able to use `tokio v1`. I guess I can start a big migration now 😅 Bookmarked the blog post! I haven't noticed you're the guy behind [nullderef](https://nullderef.com) until now. Really enjoyed your previous blogs about Rust.
Author
Owner

@hrkfdn commented on GitHub (Nov 6, 2021):

Hey there. Congratulations to your release and thanks for all the effort. I'm currently migrating ncspot to 0.11.x. and I'm struggling a little with the new ItemPositions struct. I have a collection of playable IDs and playlist positions and would like to dynamically create ItemPositions based on those as in the following example:

use rspotify::model::{Id, ItemPositions, PlayableId, TrackId};

fn main() {
    let track_ids = ["foo", "bar"];
    let positions = [1u32, 2u32];
 
    // will not compile
    let ids = track_ids.iter().zip(positions.iter()).map(|(id, pos)| ItemPositions {
        id: &TrackId::from_id(id).unwrap(),
        positions: &[pos.clone()]
    });

    // compiles fine
    let test = ItemPositions {
        id: &TrackId::from_id("test").unwrap(),
        positions: &[1]
    };
}

The borrow checker returns the following error:

error[E0515]: cannot return value referencing temporary value
  --> src/main.rs:8:70
   |
8  |       let ids = track_ids.iter().zip(positions.iter()).map(|(id, pos)| ItemPositions {
   |  ______________________________________________________________________^
9  | |         id: &TrackId::from_id(id).unwrap(),
   | |              ----------------------------- temporary value created here
10 | |         positions: &[pos.clone()]
11 | |     });
   | |_____^ returns a value referencing data owned by the current function

error[E0515]: cannot return value referencing temporary value
  --> src/main.rs:8:70
   |
8  |       let ids = track_ids.iter().zip(positions.iter()).map(|(id, pos)| ItemPositions {
   |  ______________________________________________________________________^
9  | |         id: &TrackId::from_id(id).unwrap(),
10 | |         positions: &[pos.clone()]
   | |                     ------------- temporary value created here
11 | |     });
   | |_____^ returns a value referencing data owned by the current function

I'm a little bit at loss here. It works fine if I pass static literals, but I can't manage to fulfill the memory lifetime requirements for dynamic IDs/positions. Any ideas?

<!-- gh-comment-id:962492604 --> @hrkfdn commented on GitHub (Nov 6, 2021): Hey there. Congratulations to your release and thanks for all the effort. I'm currently migrating ncspot to 0.11.x. and I'm struggling a little with the new `ItemPositions` struct. I have a collection of playable IDs and playlist positions and would like to dynamically create `ItemPositions` based on those as in the following example: ```rust use rspotify::model::{Id, ItemPositions, PlayableId, TrackId}; fn main() { let track_ids = ["foo", "bar"]; let positions = [1u32, 2u32]; // will not compile let ids = track_ids.iter().zip(positions.iter()).map(|(id, pos)| ItemPositions { id: &TrackId::from_id(id).unwrap(), positions: &[pos.clone()] }); // compiles fine let test = ItemPositions { id: &TrackId::from_id("test").unwrap(), positions: &[1] }; } ``` The borrow checker returns the following error: ``` error[E0515]: cannot return value referencing temporary value --> src/main.rs:8:70 | 8 | let ids = track_ids.iter().zip(positions.iter()).map(|(id, pos)| ItemPositions { | ______________________________________________________________________^ 9 | | id: &TrackId::from_id(id).unwrap(), | | ----------------------------- temporary value created here 10 | | positions: &[pos.clone()] 11 | | }); | |_____^ returns a value referencing data owned by the current function error[E0515]: cannot return value referencing temporary value --> src/main.rs:8:70 | 8 | let ids = track_ids.iter().zip(positions.iter()).map(|(id, pos)| ItemPositions { | ______________________________________________________________________^ 9 | | id: &TrackId::from_id(id).unwrap(), 10 | | positions: &[pos.clone()] | | ------------- temporary value created here 11 | | }); | |_____^ returns a value referencing data owned by the current function ``` I'm a little bit at loss here. It works fine if I pass static literals, but I can't manage to fulfill the memory lifetime requirements for dynamic IDs/positions. Any ideas?
Author
Owner

@marioortizmanero commented on GitHub (Nov 6, 2021):

ItemPositions contains borrowed data, so you must construct their values beforehand into owned types. This will work:

use rspotify::model::{Id, ItemPositions, PlayableId, TrackId};

fn main() {
    let track_ids = ["foo", "bar"];
    let positions = [1u32, 2u32];
 
    let track_ids = track_ids.iter().map(|id| TrackId::from_id(id).unwrap()).collect::<Vec<_>>();
    let positions = positions.iter().map(|pos| [*pos]).collect::<Vec<_>>();
    let ids = track_ids.iter().zip(positions.iter()).map(|(id, pos)| ItemPositions {
        id,
        positions: pos
    });
}

Not sure if it's the best way to do it, but try with something like that.

<!-- gh-comment-id:962513873 --> @marioortizmanero commented on GitHub (Nov 6, 2021): `ItemPositions` contains borrowed data, so you must construct their values beforehand into owned types. This will work: ```rust use rspotify::model::{Id, ItemPositions, PlayableId, TrackId}; fn main() { let track_ids = ["foo", "bar"]; let positions = [1u32, 2u32]; let track_ids = track_ids.iter().map(|id| TrackId::from_id(id).unwrap()).collect::<Vec<_>>(); let positions = positions.iter().map(|pos| [*pos]).collect::<Vec<_>>(); let ids = track_ids.iter().zip(positions.iter()).map(|(id, pos)| ItemPositions { id, positions: pos }); } ``` Not sure if it's the best way to do it, but try with something like that.
Author
Owner

@hrkfdn commented on GitHub (Nov 7, 2021):

I just realized my example wasn't very good and the problem was actually in the creation of PlayableId trait objects which I didn't include in the example 🤦 Your advice still applied, thanks a lot!

<!-- gh-comment-id:962530331 --> @hrkfdn commented on GitHub (Nov 7, 2021): I just realized my example wasn't very good and the problem was actually in the creation of `PlayableId` trait objects which I didn't include in the example :facepalm: Your advice still applied, thanks a lot!
Author
Owner

@hrkfdn commented on GitHub (Dec 12, 2021):

Hey again, one more question. Previously I used to specify the limit and offset values to implement a "Show more results" button in ncspot (see screenshow below). I would like to use the iterable results, but I can't think of a way to stop iteration if the end of a page is reached. Do you have any ideas on how I could tackle this or plans to expose this information?

Screenshot from 2021-12-12 12-00-16

<!-- gh-comment-id:991876895 --> @hrkfdn commented on GitHub (Dec 12, 2021): Hey again, one more question. Previously I used to specify the limit and offset values to implement a "Show more results" button in ncspot (see screenshow below). I would like to use the iterable results, but I can't think of a way to stop iteration if the end of a page is reached. Do you have any ideas on how I could tackle this or plans to expose this information? ![Screenshot from 2021-12-12 12-00-16](https://user-images.githubusercontent.com/19884/145709781-db1e0e29-803d-4c6e-afc1-76886f1a630c.png)
Author
Owner

@marioortizmanero commented on GitHub (Dec 12, 2021):

Assuming you know the length of the page, you can use .take(length), see https://doc.rust-lang.org/std/iter/struct.Take.html. Is that what you needed?

<!-- gh-comment-id:991879835 --> @marioortizmanero commented on GitHub (Dec 12, 2021): Assuming you know the length of the page, you can use `.take(length)`, see https://doc.rust-lang.org/std/iter/struct.Take.html. Is that what you needed?
Author
Owner

@buzzneon commented on GitHub (Dec 27, 2021):

Good afternoon!

Really enjoying the changes that 0.11 brings, and I have almost my entire project converted (only took a few hours). Thanks for the hard work!

I'm stuck on one thing though: in 0.10 we had user_playlist_tracks which returned a page of tracks, and we could keep hitting that same method to get all the pages. I don't seem to see an equivalent in 0.11; there is user_playlist, but that returns a FullPlaylist with just a single page of tracks.

Any advice?
Thanks!

<!-- gh-comment-id:1001735004 --> @buzzneon commented on GitHub (Dec 27, 2021): Good afternoon! Really enjoying the changes that 0.11 brings, and I have almost my entire project converted (only took a few hours). Thanks for the hard work! I'm stuck on one thing though: in 0.10 we had `user_playlist_tracks` which returned a page of tracks, and we could keep hitting that same method to get all the pages. I don't seem to see an equivalent in 0.11; there is `user_playlist`, but that returns a `FullPlaylist` with just a single page of tracks. Any advice? Thanks!
Author
Owner

@marioortizmanero commented on GitHub (Dec 27, 2021):

I'm very glad you've liked the changes so far. I was a bit nervous we'd get lynched at the beginning for having so many breaking changes haha.

user_playlist_tracks suffered many changes:

<!-- gh-comment-id:1001742764 --> @marioortizmanero commented on GitHub (Dec 27, 2021): I'm very glad you've liked the changes so far. I was a bit nervous we'd get lynched at the beginning for having so many breaking changes haha. `user_playlist_tracks` suffered many changes: * It was first renamed to `playlist_tracks` because the I think the endpoint itself changed (if you're already authorized you don't really need to indicate the user ID, so it makes sense). * Then, I renamed it to `playlist_items` because it actually works for episodes as well. You're probably looking for that: https://docs.rs/rspotify/latest/rspotify/clients/base/trait.BaseClient.html#method.playlist_items * If you want to have access to manual paging instead of an iterator/stream, you can use `playlist_items_manual`: https://docs.rs/rspotify/latest/rspotify/clients/base/trait.BaseClient.html#method.playlist_items_manual
Author
Owner

@buzzneon commented on GitHub (Dec 27, 2021):

Thanks so much @marioortizmanero !

I can't believe it was right under my nose - I even went spelunking through the source and I couldn't find it 🤦‍♂️

Using playlist_items works like a charm! (Also, I 100% agree with it being renamed). Don't feel bad about making breaking changes, it's often necessary for progress .. sometimes a building needs to be gutted a bit before it can really be expanded upon.

Wishing you the best for the New Years!

<!-- gh-comment-id:1001748742 --> @buzzneon commented on GitHub (Dec 27, 2021): Thanks so much @marioortizmanero ! I can't believe it was right under my nose - I even went spelunking through the source and I couldn't find it :man_facepalming: Using `playlist_items` works like a charm! (Also, I 100% agree with it being renamed). Don't feel bad about making breaking changes, it's often necessary for progress .. sometimes a building needs to be gutted a bit before it can really be expanded upon. Wishing you the best for the New Years!
Author
Owner

@buzzneon commented on GitHub (Dec 29, 2021):

I think I may have found a bug .. Querying for devices, sometimes I get the following error:

json parse error: unknown variant `TV`, expected one of `Computer`, `Tablet`, `Smartphone`, `Speaker`, `Tv`, `Avr`, `Stb`, `AudioDongle`, `GameConsole`, `CastVideo`, `CastAudio`, `Automobile`, `Unknown` at line 8 column 17"

The device in question is a Roku 3 running the Spotify app. What's weird is that sometimes it does work .. so I'm not sure if, when it works, it returns Tv, or one of the other types. Next time it works, I'll try and get the returned payload.

The error is being generated by: convert_result.

Here's the full payload returned from the Spotify API:

{
  "devices": [
    {
      "id": "c88dcf36-f1f5-5590-8516-efc4e989cb64",
      "is_active": true,
      "is_private_session": false,
      "is_restricted": false,
      "name": "Living Room Roku",
      "type": "TV",
      "volume_percent": 99
    }
  ]
}
<!-- gh-comment-id:1002714637 --> @buzzneon commented on GitHub (Dec 29, 2021): I think I may have found a bug .. Querying for devices, _sometimes_ I get the following error: ``` json parse error: unknown variant `TV`, expected one of `Computer`, `Tablet`, `Smartphone`, `Speaker`, `Tv`, `Avr`, `Stb`, `AudioDongle`, `GameConsole`, `CastVideo`, `CastAudio`, `Automobile`, `Unknown` at line 8 column 17" ``` The device in question is a Roku 3 running the Spotify app. What's weird is that sometimes it does work .. so I'm not sure if, when it works, it returns `Tv`, or one of the other types. Next time it works, I'll try and get the returned payload. The error is being generated by: [convert_result](https://github.com/ramsayleung/rspotify/blob/c824b15a3751f4bbf99e5535d24e08050832785c/src/clients/mod.rs#L14). Here's the full payload returned from the Spotify API: ``` { "devices": [ { "id": "c88dcf36-f1f5-5590-8516-efc4e989cb64", "is_active": true, "is_private_session": false, "is_restricted": false, "name": "Living Room Roku", "type": "TV", "volume_percent": 99 } ] } ```
Author
Owner

@marioortizmanero commented on GitHub (Dec 29, 2021):

Unfortunately the device type is not very well documented: https://developer.spotify.com/documentation/web-api/reference/#/operations/get-a-users-available-devices. It doesn't even mention "TV" or "Tv". I would say this is a problem on Spotify's side, but we can surely fix it easily.

Case insensitive deserialization was brought up in serde (https://github.com/serde-rs/serde/issues/586), and it ended up being added to serde_aux: https://docs.rs/serde-aux/3.0.1/serde_aux/container_attributes/fn.deserialize_struct_case_insensitive.html

In order to not pull that entire dependency we can just copy-paste it into the rspotify_model crate. I'll make a PR. Not sure how this can be reproduced but I guess it makes sense.

Nvm, it's easier to just use serde(alias)

<!-- gh-comment-id:1002763010 --> @marioortizmanero commented on GitHub (Dec 29, 2021): Unfortunately the device type is not very well documented: https://developer.spotify.com/documentation/web-api/reference/#/operations/get-a-users-available-devices. It doesn't even mention "TV" or "Tv". I would say this is a problem on Spotify's side, but we can surely fix it easily. ~~Case insensitive deserialization was brought up in `serde` (https://github.com/serde-rs/serde/issues/586), and it ended up being added to `serde_aux`: https://docs.rs/serde-aux/3.0.1/serde_aux/container_attributes/fn.deserialize_struct_case_insensitive.html~~ ~~In order to not pull that entire dependency we can just copy-paste it into the `rspotify_model` crate. I'll make a PR. Not sure how this can be reproduced but I guess it makes sense.~~ Nvm, it's easier to just use `serde(alias)`
Author
Owner

@buzzneon commented on GitHub (Dec 29, 2021):

Great news, thanks again @marioortizmanero ! 😄 👍

<!-- gh-comment-id:1002767870 --> @buzzneon commented on GitHub (Dec 29, 2021): Great news, thanks again @marioortizmanero ! :smile: :+1:
Author
Owner

@buzzneon commented on GitHub (Dec 30, 2021):

For completeness - the fix in master works like a charm, thanks again! 😄

<!-- gh-comment-id:1003132293 --> @buzzneon commented on GitHub (Dec 30, 2021): For completeness - the fix in `master` works like a charm, thanks again! :smile:
Author
Owner

@marioortizmanero commented on GitHub (Dec 30, 2021):

Awesome, thanks for the bug report!

<!-- gh-comment-id:1003177294 --> @marioortizmanero commented on GitHub (Dec 30, 2021): Awesome, thanks for the bug report!
Author
Owner

@buzzneon commented on GitHub (Feb 14, 2022):

Hi @marioortizmanero , it looks like a similar issue exists for the device type avr:

ParseJson(Error("unknown variant `AVR`, expected one of `Computer`, `Tablet`, `Smartphone`, `Speaker`, `Tv`, `Avr`, `Stb`, `AudioDongle`, `GameConsole`, `CastVideo`, `CastAudio`, `Automobile`, `Unknown`", line: 8, column: 18))
<!-- gh-comment-id:1039362014 --> @buzzneon commented on GitHub (Feb 14, 2022): Hi @marioortizmanero , it looks like a similar issue exists for the device type `avr`: ``` ParseJson(Error("unknown variant `AVR`, expected one of `Computer`, `Tablet`, `Smartphone`, `Speaker`, `Tv`, `Avr`, `Stb`, `AudioDongle`, `GameConsole`, `CastVideo`, `CastAudio`, `Automobile`, `Unknown`", line: 8, column: 18)) ```
Author
Owner

@ramsayleung commented on GitHub (Feb 15, 2022):

I have created a PR to fix this problem, you could retry after this PR merged :)

<!-- gh-comment-id:1039772431 --> @ramsayleung commented on GitHub (Feb 15, 2022): I have created a PR to fix this problem, you could retry after this PR merged :)
Author
Owner

@buzzneon commented on GitHub (Feb 17, 2022):

That worked! Thanks so much!

Sorry for the delay in getting back to you, it was a busy weekend.

<!-- gh-comment-id:1042467518 --> @buzzneon commented on GitHub (Feb 17, 2022): That worked! Thanks so much! Sorry for the delay in getting back to you, it was a busy weekend.
Author
Owner

@sewnie commented on GitHub (Mar 13, 2022):

image
i feel like it is appropriate to post this here as the main library for handling API in spotify-tui is rspotify

<!-- gh-comment-id:1066061035 --> @sewnie commented on GitHub (Mar 13, 2022): ![image](https://user-images.githubusercontent.com/47404953/158053397-90806b15-b225-44e3-b529-20d5bd995b4e.png) i feel like it is appropriate to post this here as the main library for handling API in spotify-tui is rspotify
Author
Owner

@marioortizmanero commented on GitHub (Mar 13, 2022):

That is indeed an error in rspotify, thanks for reporting. I assume it's the Type enum in Context, in CurrentPlaybackContext. We could fix it by adding a (badly documented) collection variant. But would it be possible for you to share the full error, just to make sure?

Edit: can confirm that tekore also has this variant: github.com/felix-hilden/tekore@d1200964f5/tekore/_model/context.py (L5)

<!-- gh-comment-id:1066092080 --> @marioortizmanero commented on GitHub (Mar 13, 2022): That is indeed an error in rspotify, thanks for reporting. I assume it's the `Type` enum in `Context`, in `CurrentPlaybackContext`. We could fix it by adding a (badly documented) `collection` variant. But would it be possible for you to share the full error, just to make sure? Edit: can confirm that tekore also has this variant: https://github.com/felix-hilden/tekore/blob/d1200964f50d88e99e42ada1748769de64228dc7/tekore/_model/context.py#L5
Author
Owner

@sewnie commented on GitHub (Mar 13, 2022):

But would it be possible for you to share the full error, just to make sure?

it is quite long but sure.
image
i hope this doesn't have some dangerous information 👻

<!-- gh-comment-id:1066096002 --> @sewnie commented on GitHub (Mar 13, 2022): > But would it be possible for you to share the full error, just to make sure? it is quite long but sure. ![image](https://user-images.githubusercontent.com/47404953/158060460-a2254c7f-9c87-4f13-b25b-61e53f20d3fc.png) i hope this doesn't have some _dangerous_ information :ghost:
Author
Owner

@marioortizmanero commented on GitHub (Mar 13, 2022):

You did well, the only sensitive thing may be what you censored. I can confirm that it's a problem in Type, which should be fixed with #306. Thanks again for the help!

<!-- gh-comment-id:1066097680 --> @marioortizmanero commented on GitHub (Mar 13, 2022): You did well, the only sensitive thing may be what you censored. I can confirm that it's a problem in `Type`, which should be fixed with #306. Thanks again for the help!
Author
Owner

@eladyn commented on GitHub (Apr 12, 2022):

Hi! I'm currently trying to port this code to the new rspotify version and I'm having some problems finding a clean way to parse IDs / URIs.

This method receives a spotify URI from user interaction and should start playback for the given URI. In the current version, it just uses start_playback and passes in a single context URI or a track URI depending on wether it contains spotify:track, in both cases just as a String.

This is no longer that easy. Not only has Spotify added shows, from which one can play episodes (spotify:episode, I assume), the tricky bit for me is the new way to parse Ids. To improve usage, the start_playback method was additionally split up into start_uris_playback and start_context_playback.

Due to the new way, Ids are represented in the crate, namely as different structs that implement common traits such as Id, PlayContextId or PlayableId, I need to somehow parse my string into one of the types and call the correct one of the two methods.

I'm currently doing something similar to the following and I'm wondering, if and how this could be achieved in a better way:

  1. parse the String more or less the same it is done here
  2. depending on the Type of the parsed Uri, decide I want to turn the id into a PlayContextId or a PlayableId (or rather something that implements the trait)
    • Track or Episode:
      Parse the Id with the proper method (TrackId::from_id(...) or EpisodeId::from_id(...)) and put it into a Box (ugh, heap allocation 😃), so that I can treat those two different things as Box<dyn PlayableId>.
    • everything else:
      Parse the id with the proper method (see above), put it into a Box and insert that into a custom newtype struct, which takes a Box<dyn PlayContextId>. This struct implements PlayContextId itself (although not completely, it would panic on _type_static and from_id_unchecked).
      The reason I (believe to) have to do this, is that start_context_playback is generic over its context_uri parameter and requires it to implement PlayContextId. As such, I can't just use a Box<dyn PlayContextId>, since that does not implement the trait itself. This feels really hacky.
The whole code can be found here
let mv_device_name = device_name.clone();
let sp_client = Arc::clone(&spotify_api_client);
b.method("OpenUri", ("uri",), (), move |_, _, (uri,): (String,)| {
    struct AnyContextId(Box<dyn PlayContextId>);

    impl Id for AnyContextId {
        fn id(&self) -> &str {
            self.0.id()
        }

        fn _type(&self) -> Type {
            self.0._type()
        }

        fn _type_static() -> Type
        where
            Self: Sized,
        {
            unreachable!("never called");
        }

        unsafe fn from_id_unchecked(_id: &str) -> Self
        where
            Self: Sized,
        {
            unreachable!("never called");
        }
    }
    impl PlayContextId for AnyContextId {}

    enum Uri {
        Playable(Box<dyn PlayableId>),
        Context(AnyContextId),
    }

    impl Uri {
        fn from_id(id_type: Type, id: &str) -> Result<Uri, IdError> {
            use Uri::*;
            let uri = match id_type {
                Type::Track => Playable(Box::new(TrackId::from_id(id)?)),
                Type::Episode => Playable(Box::new(EpisodeId::from_id(id)?)),
                Type::Artist => Context(AnyContextId(Box::new(ArtistId::from_id(id)?))),
                Type::Album => Context(AnyContextId(Box::new(AlbumId::from_id(id)?))),
                Type::Playlist => Context(AnyContextId(Box::new(PlaylistId::from_id(id)?))),
                Type::Show => Context(AnyContextId(Box::new(ShowId::from_id(id)?))),
                Type::User | Type::Collection => Err(IdError::InvalidType)?,
            };
            Ok(uri)
        }
    }

    // parsing the uri
    let mut chars = uri
        .strip_prefix("spotify")
        .ok_or(MethodErr::invalid_arg(&uri))?
        .chars();

    let sep = match chars.next() {
        Some(ch) if ch == '/' || ch == ':' => ch,
        _ => return Err(MethodErr::invalid_arg(&uri)),
    };
    let rest = chars.as_str();

    let (id_type, id) = rest
        .rsplit_once(sep)
        .and_then(|(id_type, id)| Some((id_type.parse::<Type>().ok()?, id)))
        .ok_or(MethodErr::invalid_arg(&uri))?;

    let uri = Uri::from_id(id_type, id).map_err(|_| MethodErr::invalid_arg(&uri))?;

    // spotifyd specific things
    let device_name = utf8_percent_encode(&mv_device_name, NON_ALPHANUMERIC).to_string();
    let device_id = sp_client.device().ok().and_then(|devices| {
        devices.into_iter().find_map(|d| {
            if d.is_active && d.name == device_name {
                d.id
            } else {
                None
            }
        })
    });

    // call the appropriate method
    match uri {
        Uri::Playable(id) => {
            let _ = sp_client.start_uris_playback(
                Some(id.as_ref()),
                device_id.as_deref(),
                Some(Offset::for_position(0)),
                None,
            );
        }
        Uri::Context(id) => {
            let _ = sp_client.start_context_playback(
                &id,
                device_id.as_deref(),
                Some(Offset::for_position(0)),
                None,
            );
        }
    }
    Ok(())
});

Sorry, if this issue is not the right place to ask this question, feel free to move it elsewhere, if not. Some of the issues I encountered might not be "Upgrading to v0.11" specific since the last crate version that spotifyd used, was 0.8. (😬)

Some of my problems should be fixed with https://github.com/ramsayleung/rspotify/pull/305, although things like parsing an Id that I know nothing about I would still have to implement myself. If you'd like me to, I can add my thoughts on that over there.

Anyway, thanks for this great library and sorry for this massive wall of text! 😅

<!-- gh-comment-id:1097240398 --> @eladyn commented on GitHub (Apr 12, 2022): Hi! I'm currently trying to port [this code](https://github.com/Spotifyd/spotifyd/blob/ae6dac7a54f899316674ba57ce4a0f9890cd2b1c/src/dbus_mpris.rs) to the new `rspotify` version and I'm having some problems finding a clean way to parse IDs / URIs. [This method](https://github.com/Spotifyd/spotifyd/blob/ae6dac7a54f899316674ba57ce4a0f9890cd2b1c/src/dbus_mpris.rs#L271-L294) receives a spotify URI from user interaction and should start playback for the given URI. In the current version, it just uses [`start_playback`](https://docs.rs/rspotify/0.8.0/rspotify/spotify/client/struct.Spotify.html#method.start_playback) and passes in a single context URI or a track URI depending on wether it contains `spotify:track`, in both cases just as a `String`. This is no longer that easy. Not only has Spotify added shows, from which one can play episodes (`spotify:episode`, I assume), the tricky bit for me is the new way to parse `Id`s. To improve usage, the `start_playback` method was additionally split up into [`start_uris_playback`](https://docs.rs/rspotify/latest/rspotify/clients/oauth/trait.OAuthClient.html#method.start_uris_playback) and [`start_context_playback`](https://docs.rs/rspotify/latest/rspotify/clients/oauth/trait.OAuthClient.html#method.start_context_playback). Due to the new way, `Id`s are represented in the crate, namely as different structs that implement common traits such as `Id`, `PlayContextId` or `PlayableId`, I need to somehow parse my string into one of the types and call the correct one of the two methods. I'm currently doing something similar to the following and I'm wondering, if and how this could be achieved in a better way: 1. parse the `String` more or less the same it is done [here](https://github.com/ramsayleung/rspotify/blob/0b4f3aa1058b961f307b7b6798b3cb875b30d24e/rspotify-model/src/idtypes.rs#L213-L226) 2. depending on the [`Type`](https://docs.rs/rspotify-model/0.11.5/rspotify_model/enums/types/enum.Type.html) of the parsed Uri, decide I want to turn the id into a `PlayContextId` or a `PlayableId` (or rather something that implements the trait) - **`Track` or `Episode`**: Parse the Id with the proper method (`TrackId::from_id(...)` or `EpisodeId::from_id(...)`) and put it into a `Box` (ugh, heap allocation :smiley:), so that I can treat those two different things as `Box<dyn PlayableId>`. - **everything else**: Parse the id with the proper method (see above), put it into a `Box` and insert that into a custom `newtype` struct, which takes a `Box<dyn PlayContextId>`. This struct implements `PlayContextId` itself (although not completely, it would panic on `_type_static` and `from_id_unchecked`). The reason I (believe to) have to do this, is that [`start_context_playback`](https://docs.rs/rspotify/latest/rspotify/clients/oauth/trait.OAuthClient.html#method.start_context_playback) is generic over its `context_uri` parameter and requires it to implement `PlayContextId`. As such, I can't just use a `Box<dyn PlayContextId>`, since that does not implement the trait itself. This feels really hacky. <summary>The whole code can be found here</summary> <details> ```rust let mv_device_name = device_name.clone(); let sp_client = Arc::clone(&spotify_api_client); b.method("OpenUri", ("uri",), (), move |_, _, (uri,): (String,)| { struct AnyContextId(Box<dyn PlayContextId>); impl Id for AnyContextId { fn id(&self) -> &str { self.0.id() } fn _type(&self) -> Type { self.0._type() } fn _type_static() -> Type where Self: Sized, { unreachable!("never called"); } unsafe fn from_id_unchecked(_id: &str) -> Self where Self: Sized, { unreachable!("never called"); } } impl PlayContextId for AnyContextId {} enum Uri { Playable(Box<dyn PlayableId>), Context(AnyContextId), } impl Uri { fn from_id(id_type: Type, id: &str) -> Result<Uri, IdError> { use Uri::*; let uri = match id_type { Type::Track => Playable(Box::new(TrackId::from_id(id)?)), Type::Episode => Playable(Box::new(EpisodeId::from_id(id)?)), Type::Artist => Context(AnyContextId(Box::new(ArtistId::from_id(id)?))), Type::Album => Context(AnyContextId(Box::new(AlbumId::from_id(id)?))), Type::Playlist => Context(AnyContextId(Box::new(PlaylistId::from_id(id)?))), Type::Show => Context(AnyContextId(Box::new(ShowId::from_id(id)?))), Type::User | Type::Collection => Err(IdError::InvalidType)?, }; Ok(uri) } } // parsing the uri let mut chars = uri .strip_prefix("spotify") .ok_or(MethodErr::invalid_arg(&uri))? .chars(); let sep = match chars.next() { Some(ch) if ch == '/' || ch == ':' => ch, _ => return Err(MethodErr::invalid_arg(&uri)), }; let rest = chars.as_str(); let (id_type, id) = rest .rsplit_once(sep) .and_then(|(id_type, id)| Some((id_type.parse::<Type>().ok()?, id))) .ok_or(MethodErr::invalid_arg(&uri))?; let uri = Uri::from_id(id_type, id).map_err(|_| MethodErr::invalid_arg(&uri))?; // spotifyd specific things let device_name = utf8_percent_encode(&mv_device_name, NON_ALPHANUMERIC).to_string(); let device_id = sp_client.device().ok().and_then(|devices| { devices.into_iter().find_map(|d| { if d.is_active && d.name == device_name { d.id } else { None } }) }); // call the appropriate method match uri { Uri::Playable(id) => { let _ = sp_client.start_uris_playback( Some(id.as_ref()), device_id.as_deref(), Some(Offset::for_position(0)), None, ); } Uri::Context(id) => { let _ = sp_client.start_context_playback( &id, device_id.as_deref(), Some(Offset::for_position(0)), None, ); } } Ok(()) }); ``` </details> Sorry, if this issue is not the right place to ask this question, feel free to move it elsewhere, if not. Some of the issues I encountered might not be "Upgrading to v0.11" specific since the last crate version that `spotifyd` used, was `0.8`. (:grimacing:) Some of my problems should be fixed with https://github.com/ramsayleung/rspotify/pull/305, although things like parsing an Id that I know nothing about I would still have to implement myself. If you'd like me to, I can add my thoughts on that over there. Anyway, thanks for this great library and sorry for this massive wall of text! :sweat_smile:
Author
Owner

@marioortizmanero commented on GitHub (Apr 26, 2022):

Hi @eladyn, just wanted to let you know that I wasn't able to answer yet because I haven't had any free time (and it will continue that way until around the end of May, unfortunately).

I can agree with you that the design of Id types is indeed not perfect. I put a lot of effort into considering the different alternatives we had, and chose dyn because it seemed like the most "idiomatic" and "natural" way to approach the problem. However, it still had a few downsides:

  1. You need an owned type internally (String). I think this could be "fixed" by using Cow<str> but I am currently not sure.
  2. It's awkward to cast back to the original Id type from a dyn
  3. Its implementation is intricate and relies on macros

Note that you may not need to box your Id if you don't plan on keeping it after the function. You can use a &dyn instead.

I agree that the resulting code is indeed overly complex, but only because you have to implement your own Id type. Handling both cases of start_context_playback and start_uris_playback is inherently cumbersome if you want to do it in a type-safe way. As you said, you know nothing about the Id at that point. Therefore, I would consider this issue fixed if we managed to remove the custom Id part.

I'm glad you at least got it working, though. If it's fine by you, once either I or Ramsay have more time, we can look into the issue and fix it properly in a future version.

<!-- gh-comment-id:1109208573 --> @marioortizmanero commented on GitHub (Apr 26, 2022): Hi @eladyn, just wanted to let you know that I wasn't able to answer yet because I haven't had any free time (and it will continue that way until around the end of May, unfortunately). I can agree with you that the design of `Id` types is indeed not perfect. I put a lot of effort into considering the different alternatives we had, and chose `dyn` because it seemed like the most "idiomatic" and "natural" way to approach the problem. However, it still had a few downsides: 1. You need an owned type internally (`String`). I think this could be "fixed" by using `Cow<str>` but I am currently not sure. 2. It's awkward to cast back to the original `Id` type from a `dyn` 3. Its implementation is intricate and relies on macros Note that you may not need to box your Id if you don't plan on keeping it after the function. You can use a `&dyn` instead. I agree that the resulting code is indeed overly complex, but only because you have to implement your own `Id` type. Handling both cases of `start_context_playback` and `start_uris_playback` is inherently cumbersome if you want to do it in a type-safe way. As you said, you know nothing about the Id at that point. Therefore, I would consider this issue fixed if we managed to remove the custom Id part. I'm glad you at least got it working, though. If it's fine by you, once either I or Ramsay have more time, we can look into the issue and fix it properly in a future version.
Author
Owner

@eladyn commented on GitHub (Apr 28, 2022):

Thank you for the answer! I don't have much time myself currently, so no hurries. I might have a look at the code I wrote again some time in the future and hopefully find a way to improve it a bit (maybe apply your suggestion about the Box thing). However, good to know that I wasn't just too dumb and didn't overlook an obvious solution. 😀

<!-- gh-comment-id:1111990170 --> @eladyn commented on GitHub (Apr 28, 2022): Thank you for the answer! I don't have much time myself currently, so no hurries. I might have a look at the code I wrote again some time in the future and hopefully find a way to improve it a bit (maybe apply your suggestion about the `Box` thing). However, good to know that I wasn't just too dumb and didn't overlook an obvious solution. :grinning:
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
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#72
No description provided.