[PR #129] [MERGED] Multiple clients via features #260

New issue

Closed

opened 2026-02-27 20:24:00 +03:00 by kerem · 0 comments

kerem commented

2026-02-27 20:24:00 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/ramsayleung/rspotify/pull/129
Author: @marioortizmanero
Created: 9/20/2020
Status: ✅ Merged
Merged: 10/18/2020
Merged by: @ramsayleung

Base: master ← Head: multiple-clients

📝 Commits (10+)

e4f85f8 First attempt with Deref
d0e7a5f Formatting and Cargo.toml fix
279c138 Implemented run_blocking macro
7532b3f Fix tokio import for tests, small cleanup
e783eae Implemented macro for a single function
d3e821c Now with docs too
9c783dd Single impl block, documentation
675aeb7 More clear formatting
578c13b Applied to all endpoints with a vim macro!
2c29637 Fixed docstrings

📊 Changes

34 files changed (+3048 additions, -5183 deletions)

View changed files

📝 .github/workflows/ci.yml (+18 -4)
📝 CHANGELOG.md (+27 -6)
📝 Cargo.toml (+52 -31)
📝 README.md (+24 -0)
📝 examples/album.rs (+31 -12)
➖ examples/blocking/device.rs (+0 -39)
➖ examples/blocking/me.rs (+0 -39)
➖ examples/blocking/search.rs (+0 -98)
➖ examples/blocking/seek_track.rs (+0 -41)
📝 examples/current_user_recently_played.rs (+40 -30)
➕ examples/oauth_tokens.rs (+45 -0)
📝 examples/track.rs (+31 -12)
📝 examples/tracks.rs (+29 -12)
➕ examples/ureq/device.rs (+48 -0)
➕ examples/ureq/me.rs (+47 -0)
➕ examples/ureq/search.rs (+108 -0)
➕ examples/ureq/seek_track.rs (+49 -0)
📝 examples/webapp/Cargo.toml (+1 -1)
📝 examples/webapp/src/main.rs (+107 -80)
📝 examples/with_refresh_token.rs (+49 -37)

...and 14 more files

📄 Description

This is a second attempt for fixing #112. The original idea at #120 was to have a blocking module that would call the async implementations with a blocking runtime in order to avoid code duplication. This turned up to be a bad idea, because it wasn't as easy and flexible as I thought. This new approach uses the maybe-async crate, which makes it relatively easy to use multiple HTTP clients in the same crate with features. Instead of having a blocking module, the user may configure what client is used like so: rspotify = { version = "...", default_features = false, features = "client-ureq" } (client-reqwest is still the default).

Advantages of this approach:

Much cleaner implementation for the "blocking module"
More flexible than a switch between sync and async code, since multiple clients for each of these can be configured. For now it's only two. This flexibility also makes rspotify faster and less bloated, since the blocking module now uses ureq instead of reqwest, which is much more minimal.
Just as fast to compile: you'd think that the original approach would be faster to compile because it doesn't need a macro to work. But in the end the macro is necessary to make development less of a pain, so both solutions use macros. I haven't tested this but I'm sure the difference isn't noticeable.
The endpoint function signatures aren't limited: the original approach required a macro_rules! macro. These are quick and easy to implement, but supporting generic parameters and other compex stuff can get out of hands pretty quickly. This avoids any workarounds for that.
The derive_deref dependency isn't necessary.

Disadvantages of this approach:

Only one client may be used at once
New dependency: maybe-async. It's just a procedural macro so no performance impact whatsoever. This also requires async_trait.

Required changes and progress

Async client with reqwest
Blocking client with ureq
Rewrite OAuth
Update documentation
Update tests, make them available for all clients
Update basic examples
Update the webapp example
Open an issue at maybe_async about the parenthesis warnings when using ureq: https://github.com/fMeow/maybe-async-rs/issues/1

Read the CHANGELOG.md additions for more changes (most of them are from #127). I had to rewrite the fetch_access_token logic so that it can use the HTTP client in Spotify, and I ended up improving lots of other stuff I saw. (this is still very WIP, along with the first two points of this section).

I've left lots of TODOs that can be discussed after this is merged. I'd like to finish this ASAP so that other changes can be made without merge conflicts and without having to apply them for the blocking module as well.

Also see the examples, some of them have been modified to show how the API works now.

Closes #109
Closes #112
Closes #116
Closes #131

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/ramsayleung/rspotify/pull/129 **Author:** [@marioortizmanero](https://github.com/marioortizmanero) **Created:** 9/20/2020 **Status:** ✅ Merged **Merged:** 10/18/2020 **Merged by:** [@ramsayleung](https://github.com/ramsayleung) **Base:** `master` ← **Head:** `multiple-clients` --- ### 📝 Commits (10+) - [`e4f85f8`](https://github.com/ramsayleung/rspotify/commit/e4f85f8286d496f2f2ba30c9ad882e3c4bc3f2f5) First attempt with Deref - [`d0e7a5f`](https://github.com/ramsayleung/rspotify/commit/d0e7a5f4c6f7482b647eb71e8117a389610dd152) Formatting and Cargo.toml fix - [`279c138`](https://github.com/ramsayleung/rspotify/commit/279c1387ac0f91dfa627f9dc90ca230b8fe90f6e) Implemented run_blocking macro - [`7532b3f`](https://github.com/ramsayleung/rspotify/commit/7532b3f60eb4271fa4fba877f4137f55d9a784e1) Fix tokio import for tests, small cleanup - [`e783eae`](https://github.com/ramsayleung/rspotify/commit/e783eae34ec135aa6389e8ca22026b3090a3dc69) Implemented macro for a single function - [`d3e821c`](https://github.com/ramsayleung/rspotify/commit/d3e821c5ddb72e348e63a5bad5e7df60b133c083) Now with docs too - [`9c783dd`](https://github.com/ramsayleung/rspotify/commit/9c783dd0db967e5128eeeb200b3c6c0e669bd7f6) Single impl block, documentation - [`675aeb7`](https://github.com/ramsayleung/rspotify/commit/675aeb7d0567269c40f65880f8f55ef0e592d8c8) More clear formatting - [`578c13b`](https://github.com/ramsayleung/rspotify/commit/578c13ba92f1242efaa99053765cc694079dc7f6) Applied to all endpoints with a vim macro! - [`2c29637`](https://github.com/ramsayleung/rspotify/commit/2c2963744a83b8afb7b207598e330c35ea91c8ce) Fixed docstrings ### 📊 Changes **34 files changed** (+3048 additions, -5183 deletions) <details> <summary>View changed files</summary> 📝 `.github/workflows/ci.yml` (+18 -4) 📝 `CHANGELOG.md` (+27 -6) 📝 `Cargo.toml` (+52 -31) 📝 `README.md` (+24 -0) 📝 `examples/album.rs` (+31 -12) ➖ `examples/blocking/device.rs` (+0 -39) ➖ `examples/blocking/me.rs` (+0 -39) ➖ `examples/blocking/search.rs` (+0 -98) ➖ `examples/blocking/seek_track.rs` (+0 -41) 📝 `examples/current_user_recently_played.rs` (+40 -30) ➕ `examples/oauth_tokens.rs` (+45 -0) 📝 `examples/track.rs` (+31 -12) 📝 `examples/tracks.rs` (+29 -12) ➕ `examples/ureq/device.rs` (+48 -0) ➕ `examples/ureq/me.rs` (+47 -0) ➕ `examples/ureq/search.rs` (+108 -0) ➕ `examples/ureq/seek_track.rs` (+49 -0) 📝 `examples/webapp/Cargo.toml` (+1 -1) 📝 `examples/webapp/src/main.rs` (+107 -80) 📝 `examples/with_refresh_token.rs` (+49 -37) _...and 14 more files_ </details> ### 📄 Description This is a second attempt for fixing #112. The original idea at #120 was to have a `blocking` module that would call the async implementations with a blocking runtime in order to avoid code duplication. This turned up to be a bad idea, because it wasn't as easy and flexible as I thought. This new approach uses the [`maybe-async`](https://docs.rs/maybe-async/0.2.0/maybe_async/) crate, which makes it relatively easy to use multiple HTTP clients in the same crate with features. Instead of having a `blocking` module, the user may configure what client is used like so: `rspotify = { version = "...", default_features = false, features = "client-ureq" }` (`client-reqwest` is still the default). ## Advantages of this approach: * Much cleaner implementation for the "blocking module" * More flexible than a switch between sync and async code, since multiple clients for each of these can be configured. For now it's only two. This flexibility also makes rspotify faster and less bloated, since the blocking module now uses `ureq` instead of `reqwest`, which is much more minimal. * Just as fast to compile: you'd think that the original approach would be faster to compile because it doesn't *need* a macro to work. But in the end the macro is necessary to make development less of a pain, so both solutions use macros. I haven't tested this but I'm sure the difference isn't noticeable. * The endpoint function signatures aren't limited: the original approach required a `macro_rules!` macro. These are quick and easy to implement, but supporting generic parameters and other compex stuff can get out of hands pretty quickly. This avoids any workarounds for that. * The `derive_deref` dependency isn't necessary. ## Disadvantages of this approach: * Only one client may be used at once * New dependency: [`maybe-async`](https://docs.rs/maybe-async/0.2.0/maybe_async/). It's just a procedural macro so no performance impact whatsoever. This also requires `async_trait`. ## Required changes and progress - [x] Async client with `reqwest` - [x] Blocking client with `ureq` - [x] Rewrite OAuth - [x] Update documentation - [x] Update tests, make them available for all clients - [x] Update basic examples - [x] Update the `webapp` example - [x] Open an issue at `maybe_async` about the parenthesis warnings when using `ureq`: https://github.com/fMeow/maybe-async-rs/issues/1 Read the CHANGELOG.md additions for more changes (most of them are from #127). I had to rewrite the `fetch_access_token` logic so that it can use the HTTP client in `Spotify`, and I ended up improving lots of other stuff I saw. (this is still very WIP, along with the first two points of this section). I've left lots of `TODO`s that can be discussed after this is merged. I'd like to finish this ASAP so that other changes can be made without merge conflicts and without having to apply them for the blocking module as well. Also see the examples, some of them have been modified to show how the API works now. * Closes #109 * Closes #112 * Closes #116 * Closes #131 --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>