Upgrading to v0.11: questions and support

[marioortizmanero]

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!

[marioortizmanero]

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

[flip1995]

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

[marioortizmanero]

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 Support re-authenticate automatically and refresh token when it expired. #224 PR as Fix refreshing token #255 indicates. Not sure if he's already working on it or not.
• Merge the currently open PRs, which should be really quick & easy:
  • Fix the remaining TODOs #257, ready for merging I think
  • HTTP Error cleanup #258, I have to apply a few fixes before ramsay can review & merge it
  • Implement Default when it is possible to #261, requires a review
• Avoiding conflicting features; allow multiple HTTP clients at once #221, which is not trivial. Unlike I thought, it is possible to fix, but it requires updating the maybe_async crate as indicated by async and sync in the same program fMeow/maybe-async-rs#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.

[flip1995]

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.

[ramsayleung]

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.

[marioortizmanero]

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

[marioortizmanero]

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

[marioortizmanero]

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!

[marioortizmanero]

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 ❤️

[aome510]

@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.

[hrkfdn]

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?

[marioortizmanero]

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.

[hrkfdn]

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!

[hrkfdn]

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?

[marioortizmanero]

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?

[buzzneon]

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
    }
  ]
}

[marioortizmanero]

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 (serde-rs/serde#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)

[buzzneon]

Great news, thanks again @marioortizmanero ! 😄 👍

[buzzneon]

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

[marioortizmanero]

Awesome, thanks for the bug report!

[buzzneon]

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))

[ramsayleung]

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

[buzzneon]

That worked! Thanks so much!

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

[apprehensions]

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

[marioortizmanero]

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

[apprehensions]

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

it is quite long but sure.

i hope this doesn't have some dangerous information 👻

[marioortizmanero]

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!

[eladyn]

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. (:grimacing:)

Some of my problems should be fixed with #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! 😅

[marioortizmanero]

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.

[eladyn]

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. 😀