[PR #224] [CLOSED] Support re-authenticate automatically and refresh token when it expired. #327

New issue

Closed

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

kerem commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/ramsayleung/rspotify/pull/224
Author: @ramsayleung
Created: 7/10/2021
Status: ❌ Closed

Base: master ← Head: ramsay_support_refresh_token

📝 Commits (10+)

092b153 Change function signature of get_token and get_token_mut to async.
21a9339 Change the function signature of get_token and get_token_mut
bdbf3ef Remove mut from get_token_mut.
b8a6abd Remove mut from request_token and refresh_token
c54ec04 Automatically re-authenticate.
03050a0 Optimze get_token flow.
c50c2dd Add a new function auto_reauth for OAuthClient trait.
04c88b8 Fix compile error.
b97552c webapp example upgrade rocket to 0.5
c563f5d update refresh-token.rs.

📊 Changes

21 files changed (+594 additions, -168 deletions)

View changed files

📝 .env (+3 -3)
📝 .gitignore (+2 -0)
📝 CHANGELOG.md (+3 -1)
📝 Cargo.toml (+10 -0)
📝 examples/oauth_tokens.rs (+6 -3)
📝 examples/ureq/search.rs (+1 -1)
➕ examples/ureq/threading.rs (+45 -0)
📝 examples/webapp/Cargo.toml (+3 -3)
📝 examples/webapp/src/main.rs (+37 -36)
➕ examples/with_auto_reauth.rs (+111 -0)
📝 examples/with_refresh_token.rs (+4 -12)
📝 src/auth_code.rs (+63 -30)
📝 src/auth_code_pkce.rs (+60 -25)
📝 src/client_creds.rs (+92 -12)
📝 src/clients/base.rs (+81 -12)
📝 src/clients/mod.rs (+3 -3)
📝 src/clients/oauth.rs (+47 -8)
📝 src/lib.rs (+11 -1)
📝 tests/test_oauth2.rs (+4 -4)
📝 tests/test_with_credential.rs (+1 -1)

...and 1 more files

📄 Description

Description

Fixed #223 and #4, since @marioortizmanero already implemented the cache token feature, what I need to do is implement the re-authenticate feature and refresh token when it expired.

To be honest, it's a little hard to implement than I expect before. The major problem is that to maintain a mutable token inside the immutable client.

Cell<T> doesn't work as expected, since it requires the types that implement Copy trait, but Token doesn't implement the Copy trait and has no way to do so. Since the String type doesn't implement.
Mutex<T> seems to be unnecessary, for that we don't need to handle the shared-state between threads(or we should do so?)
RefCell<T> is great, it looks pretty suitable for our case, but it also follows Rust borrow-check rules, even though the check delays to runtime:

At any given time, you can have either (but not both of) one mutable reference or any number of immutable references.

So the following code will fail:

let c = RefCell::new(5);
let m = c.borrow();

let b = c.borrow_mut(); // this causes a panic

Thus, we need to take good care of the borrowed immutable reference and borrowed mutable reference.

And there are a little TODO items I left to discuss, I am not sure which way should we choose:

I change the function signature of request_token/refresh_token from &mut self to &self, so we have an immutable client now. But the trade-off is we use borrow_mut internally, which means we leave the immutable or mutable borrows checked at runtime, we are in charge of compiler's jobs. It looks like a landmine, will bomb someday by mistake.
Same as above, there is no way to return reference hold by RefCell, so I changed the function signature of get_token and get_token_mut, to return the wrapper type from RefCell.
In this commit c54ec04 (#224), maybe_async failed to compile when is_sync feature gate is set, it seems it can't reduce the async move, I am not sure about that.

I also upgrade the Rocket crate to 0.5-rc for the webapp example, now it can compile with the stable rustc compiler.

Motivation and Context

Dependencies

Nope

Type of change

Please delete options that are not relevant.

New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

How Has This Been Tested?

Test A: examples/with_refresh_token

_{🔄 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/224 **Author:** [@ramsayleung](https://github.com/ramsayleung) **Created:** 7/10/2021 **Status:** ❌ Closed **Base:** `master` ← **Head:** `ramsay_support_refresh_token` --- ### 📝 Commits (10+) - [`092b153`](https://github.com/ramsayleung/rspotify/commit/092b15398e245a9d8d9780e929992074b6e4327b) Change function signature of `get_token` and `get_token_mut` to async. - [`21a9339`](https://github.com/ramsayleung/rspotify/commit/21a9339073988649aac5b89ffc757b2d00735608) Change the function signature of `get_token` and `get_token_mut` - [`bdbf3ef`](https://github.com/ramsayleung/rspotify/commit/bdbf3ef142200f243b912e27b79da27b53ebffbe) Remove `mut` from `get_token_mut`. - [`b8a6abd`](https://github.com/ramsayleung/rspotify/commit/b8a6abd27cb1502081e38c125672bd827277d659) Remove `mut` from `request_token` and `refresh_token` - [`c54ec04`](https://github.com/ramsayleung/rspotify/commit/c54ec047ea1220d4be9af2536fa9d5fdad50b56d) Automatically re-authenticate. - [`03050a0`](https://github.com/ramsayleung/rspotify/commit/03050a0e5d40ca4753e71923d356f28f4dfb0b77) Optimze get_token flow. - [`c50c2dd`](https://github.com/ramsayleung/rspotify/commit/c50c2dd25e4487aa63b55b3bfd077965a54b236a) Add a new function `auto_reauth` for OAuthClient trait. - [`04c88b8`](https://github.com/ramsayleung/rspotify/commit/04c88b8224264c72f03cb72c6c2144eafe0926cd) Fix compile error. - [`b97552c`](https://github.com/ramsayleung/rspotify/commit/b97552c98706d765e3eb5a540f36c6b9ad91c538) webapp example upgrade rocket to 0.5 - [`c563f5d`](https://github.com/ramsayleung/rspotify/commit/c563f5d2887b7bc295eb5c935299a8d71206bf8e) update refresh-token.rs. ### 📊 Changes **21 files changed** (+594 additions, -168 deletions) <details> <summary>View changed files</summary> 📝 `.env` (+3 -3) 📝 `.gitignore` (+2 -0) 📝 `CHANGELOG.md` (+3 -1) 📝 `Cargo.toml` (+10 -0) 📝 `examples/oauth_tokens.rs` (+6 -3) 📝 `examples/ureq/search.rs` (+1 -1) ➕ `examples/ureq/threading.rs` (+45 -0) 📝 `examples/webapp/Cargo.toml` (+3 -3) 📝 `examples/webapp/src/main.rs` (+37 -36) ➕ `examples/with_auto_reauth.rs` (+111 -0) 📝 `examples/with_refresh_token.rs` (+4 -12) 📝 `src/auth_code.rs` (+63 -30) 📝 `src/auth_code_pkce.rs` (+60 -25) 📝 `src/client_creds.rs` (+92 -12) 📝 `src/clients/base.rs` (+81 -12) 📝 `src/clients/mod.rs` (+3 -3) 📝 `src/clients/oauth.rs` (+47 -8) 📝 `src/lib.rs` (+11 -1) 📝 `tests/test_oauth2.rs` (+4 -4) 📝 `tests/test_with_credential.rs` (+1 -1) _...and 1 more files_ </details> ### 📄 Description ## Description Fixed #223 and #4, since @marioortizmanero already implemented the cache token feature, what I need to do is implement the re-authenticate feature and refresh token when it expired. To be honest, it's a little hard to implement than I expect before. The major problem is that to maintain a mutable token inside the immutable client. + `Cell<T>` doesn't work as expected, since it requires the types that implement `Copy` trait, but `Token` doesn't implement the `Copy` trait and has no way to do so. Since the String type doesn't implement. + `Mutex<T>` seems to be unnecessary, for that we don't need to handle the shared-state between threads(or we should do so?) + `RefCell<T>` is great, it looks pretty suitable for our case, but it also follows Rust borrow-check rules, even though the check delays to runtime: > At any given time, you can have either (but not both of) one mutable reference or any number of immutable references. So the following code will fail: ```rust let c = RefCell::new(5); let m = c.borrow(); let b = c.borrow_mut(); // this causes a panic ``` Thus, we need to take good care of the borrowed immutable reference and borrowed mutable reference. And there are a little `TODO` items I left to discuss, I am not sure which way should we choose: + I change the function signature of `request_token/refresh_token` from `&mut self` to `&self`, so we have an immutable client now. But the trade-off is we use `borrow_mut` internally, which means we leave the immutable or mutable borrows checked at runtime, we are in charge of compiler's jobs. It looks like a landmine, will bomb someday by mistake. + Same as above, there is no way to return reference hold by `RefCell`, so I changed the function signature of `get_token` and `get_token_mut`, to return the wrapper type from `RefCell`. + In this commit [`c54ec04` (#224)](https://github.com/ramsayleung/rspotify/pull/224/commits/c54ec047ea1220d4be9af2536fa9d5fdad50b56d), `maybe_async` failed to compile when `is_sync` feature gate is set, it seems it can't reduce the `async move`, I am not sure about that. I also upgrade the `Rocket` crate to `0.5-rc` for the `webapp` example, now it can compile with the stable `rustc` compiler. ## Motivation and Context + #176 + #4 + #223 ## Dependencies Nope ## Type of change Please delete options that are not relevant. - [x] New feature (non-breaking change which adds functionality) - [x] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [x] This change requires a documentation update ## How Has This Been Tested? - [x] Test A: examples/with_refresh_token --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>