[PR #379] [CLOSED] Much simpler interface based on webapp example #422

New issue

Closed

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

kerem commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/ramsayleung/rspotify/pull/379
Author: @marioortizmanero
Created: 12/29/2022
Status: ❌ Closed

Base: master ← Head: better-webapp

📝 Commits (5)

23a08e7 Much simpler webapp
e1acfbe Refactor auth process & token mutex
4d850d0 Small improvements
494f0af No need to handle token cache manually in prompt
ab6740d More clear is_token_valid

📊 Changes

13 files changed (+217 additions, -307 deletions)

View changed files

📝 Cargo.toml (+1 -0)
📝 examples/webapp/Cargo.toml (+1 -1)
📝 examples/webapp/src/main.rs (+95 -125)
📝 src/auth_code.rs (+15 -10)
📝 src/auth_code_pkce.rs (+15 -10)
📝 src/client_creds.rs (+4 -30)
📝 src/clients/base.rs (+79 -17)
📝 src/clients/oauth.rs (+6 -79)
📝 src/lib.rs (+0 -1)
➖ src/sync/blocking.rs (+0 -1)
➖ src/sync/futures.rs (+0 -16)
➖ src/sync/mod.rs (+0 -16)
📝 tests/test_with_oauth.rs (+1 -1)

📄 Description

Description

This PR simplifies rspotify's usage based on the webapp example. I realized that we were really handing the token cache ourselves, instead of being automatic. We had lots of unnecessary boilerplate, which I've now simplified thanks to a refactored auth flow:

I've moved read_token_cache into BaseClient and generalized it to all kinds of clients through the method is_token_valid. Note that valid doesn't mean that it's ready to be used, just that it matches the flow, and that it could be used to reauthenticate (i.e., even if expired, we have a refresh token).
I've modified write_token_cache so that it needs to have the token passed, instead of only using self.token. Not only does this avoid unnecessary locks in some places (and weird code because we don't want to lock inside a lock), but also it's more consistent with the way read_cache_token works.
We now check the cache file once at the first request in the client. If it's valid, we set it as the client's token. Note that it might be valid but not refreshed, so the user will need to make sure both token_cached and token_refreshing are enabled. Otherwise, they'll have to take care of that themselves.

Motivation and Context

This closes #324

Dependencies

parking_lot makes token's usage simpler by simply not dealing with lock poisoning manually. We can avoid unwrap for every lock we have. Furthermore, I've removed the async variant, as many sources indicate that a regular mutex is usually better anyway.

Type of change

Please delete options that are not relevant.

Bug fix (non-breaking change which fixes an issue)

How has this been tested?

Ran it locally

Is this change properly documented?

Yes.

_{🔄 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/379 **Author:** [@marioortizmanero](https://github.com/marioortizmanero) **Created:** 12/29/2022 **Status:** ❌ Closed **Base:** `master` ← **Head:** `better-webapp` --- ### 📝 Commits (5) - [`23a08e7`](https://github.com/ramsayleung/rspotify/commit/23a08e7de2500a8e4abac27d4d7632d267acb4fa) Much simpler webapp - [`e1acfbe`](https://github.com/ramsayleung/rspotify/commit/e1acfbe3d30fabf76b5cd8150603c268fecd38f2) Refactor auth process & token mutex - [`4d850d0`](https://github.com/ramsayleung/rspotify/commit/4d850d0c512b8e23b7c7d8e03e03b8ea9b3f3868) Small improvements - [`494f0af`](https://github.com/ramsayleung/rspotify/commit/494f0af0751589b799743dacd44b3838aaafa737) No need to handle token cache manually in prompt - [`ab6740d`](https://github.com/ramsayleung/rspotify/commit/ab6740db11560133e2089b6b7ac69c650ce79ed9) More clear `is_token_valid` ### 📊 Changes **13 files changed** (+217 additions, -307 deletions) <details> <summary>View changed files</summary> 📝 `Cargo.toml` (+1 -0) 📝 `examples/webapp/Cargo.toml` (+1 -1) 📝 `examples/webapp/src/main.rs` (+95 -125) 📝 `src/auth_code.rs` (+15 -10) 📝 `src/auth_code_pkce.rs` (+15 -10) 📝 `src/client_creds.rs` (+4 -30) 📝 `src/clients/base.rs` (+79 -17) 📝 `src/clients/oauth.rs` (+6 -79) 📝 `src/lib.rs` (+0 -1) ➖ `src/sync/blocking.rs` (+0 -1) ➖ `src/sync/futures.rs` (+0 -16) ➖ `src/sync/mod.rs` (+0 -16) 📝 `tests/test_with_oauth.rs` (+1 -1) </details> ### 📄 Description ## Description This PR simplifies `rspotify`'s usage based on the webapp example. I realized that we were really handing the token cache ourselves, instead of being automatic. We had lots of unnecessary boilerplate, which I've now simplified thanks to a refactored auth flow: - I've moved `read_token_cache` into `BaseClient` and generalized it to all kinds of clients through the method `is_token_valid`. Note that valid doesn't mean that it's ready to be used, just that it matches the flow, and that it could be used to reauthenticate (i.e., even if expired, we have a refresh token). - I've modified `write_token_cache` so that it needs to have the `token` passed, instead of only using `self.token`. Not only does this avoid unnecessary locks in some places (and weird code because we don't want to lock inside a lock), but also it's more consistent with the way `read_cache_token` works. - We now check the cache file once at the first request in the client. If it's valid, we set it as the client's token. Note that it might be valid but not refreshed, so the user will need to make sure both `token_cached` and `token_refreshing` are enabled. Otherwise, they'll have to take care of that themselves. ## Motivation and Context This closes #324 ## Dependencies `parking_lot` makes `token`'s usage simpler by simply not dealing with lock poisoning manually. We can avoid `unwrap` for every lock we have. Furthermore, I've removed the async variant, as many sources indicate that a regular mutex is usually better anyway. ## Type of change Please delete options that are not relevant. - [x] Bug fix (non-breaking change which fixes an issue) ## How has this been tested? Ran it locally ## Is this change properly documented? Yes. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>