[PR #321] [MERGED] Remove unnecessary mutable references in clients #397

New issue

Closed

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

kerem commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/ramsayleung/rspotify/pull/321
Author: @Spanfile
Created: 5/31/2022
Status: ✅ Merged
Merged: 6/2/2022
Merged by: @marioortizmanero

Base: master ← Head: master

📝 Commits (2)

42ffc65 Remove unnecessary mutable references in clients
345993e Remove now unnecessary muts in examples

📊 Changes

20 files changed (+25 additions, -25 deletions)

View changed files

📝 examples/auth_code.rs (+1 -1)
📝 examples/client_creds.rs (+1 -1)
📝 examples/oauth_tokens.rs (+1 -1)
📝 examples/pagination_async.rs (+1 -1)
📝 examples/pagination_manual.rs (+1 -1)
📝 examples/pagination_sync.rs (+1 -1)
📝 examples/tasks.rs (+1 -1)
📝 examples/ureq/device.rs (+1 -1)
📝 examples/ureq/me.rs (+1 -1)
📝 examples/ureq/search.rs (+1 -1)
📝 examples/ureq/seek_track.rs (+1 -1)
📝 examples/ureq/threading.rs (+1 -1)
📝 examples/webapp/src/main.rs (+3 -3)
📝 examples/with_auto_reauth.rs (+2 -2)
📝 examples/with_refresh_token.rs (+1 -1)
📝 src/auth_code.rs (+1 -1)
📝 src/auth_code_pkce.rs (+1 -1)
📝 src/client_creds.rs (+1 -1)
📝 src/clients/oauth.rs (+3 -3)
📝 tests/test_with_credential.rs (+1 -1)

📄 Description

Description

OAuthClient::request_token takes self as &mut, yet all of its implementors are designed for an async environment, where inner mutability for their fields is required through Arc<Mutex<_>>, as is the case for the token fields, thus self doesn't actually have to be a mutable reference. By making the reference immutable, it is much more comfortable to use the clients in application code, since there's no need to wrap them in e.g. a Mutex or an RwLock only for the one case where a mutable self is needlessly required.

However, AuthCodePkceSpotify::get_authorize_url is still left as is to take self as &mut since it has to set its verifier field, and converting that field to Arc<Mutex<_>> and converting the function into async would be a breaking change, hence I've left it out of this PR.

Motivation and Context

I've been using the library recently and ran into this ergonomics issue.

Dependencies

None.

Type of change

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

How has this been tested?

The crate test suite ran succesfully. I removed one unneeded mut in tests/test_with_credential.rs.

Is this change properly documented?

I'm unsure if this is a breaking change, since existing application code will still work, it would just be beneficial for them to adapt to not requiring mutability anymore. If someone has implemented OAuthClient for their own type, though, it will be a breaking change since the trait signature has changed.

_{🔄 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/321 **Author:** [@Spanfile](https://github.com/Spanfile) **Created:** 5/31/2022 **Status:** ✅ Merged **Merged:** 6/2/2022 **Merged by:** [@marioortizmanero](https://github.com/marioortizmanero) **Base:** `master` ← **Head:** `master` --- ### 📝 Commits (2) - [`42ffc65`](https://github.com/ramsayleung/rspotify/commit/42ffc659911fe503d5be851814c2766d5de34a6f) Remove unnecessary mutable references in clients - [`345993e`](https://github.com/ramsayleung/rspotify/commit/345993e72b7c76f13bb95a12cbb31070e82c7835) Remove now unnecessary muts in examples ### 📊 Changes **20 files changed** (+25 additions, -25 deletions) <details> <summary>View changed files</summary> 📝 `examples/auth_code.rs` (+1 -1) 📝 `examples/client_creds.rs` (+1 -1) 📝 `examples/oauth_tokens.rs` (+1 -1) 📝 `examples/pagination_async.rs` (+1 -1) 📝 `examples/pagination_manual.rs` (+1 -1) 📝 `examples/pagination_sync.rs` (+1 -1) 📝 `examples/tasks.rs` (+1 -1) 📝 `examples/ureq/device.rs` (+1 -1) 📝 `examples/ureq/me.rs` (+1 -1) 📝 `examples/ureq/search.rs` (+1 -1) 📝 `examples/ureq/seek_track.rs` (+1 -1) 📝 `examples/ureq/threading.rs` (+1 -1) 📝 `examples/webapp/src/main.rs` (+3 -3) 📝 `examples/with_auto_reauth.rs` (+2 -2) 📝 `examples/with_refresh_token.rs` (+1 -1) 📝 `src/auth_code.rs` (+1 -1) 📝 `src/auth_code_pkce.rs` (+1 -1) 📝 `src/client_creds.rs` (+1 -1) 📝 `src/clients/oauth.rs` (+3 -3) 📝 `tests/test_with_credential.rs` (+1 -1) </details> ### 📄 Description ## Description `OAuthClient::request_token` takes self as `&mut`, yet all of its implementors are designed for an async environment, where inner mutability for their fields is required through `Arc<Mutex<_>>`, as is the case for the token fields, thus self doesn't actually have to be a mutable reference. By making the reference immutable, it is much more comfortable to use the clients in application code, since there's no need to wrap them in e.g. a Mutex or an RwLock only for the one case where a mutable self is needlessly required. However, `AuthCodePkceSpotify::get_authorize_url` is still left as is to take self as `&mut` since it has to set its `verifier` field, and converting that field to `Arc<Mutex<_>>` and converting the function into `async` would be a breaking change, hence I've left it out of this PR. ## Motivation and Context I've been using the library recently and ran into this ergonomics issue. ## Dependencies None. ## Type of change - [x] Bug fix (non-breaking change which fixes an issue) ## How has this been tested? The crate test suite ran succesfully. I removed one unneeded `mut` in `tests/test_with_credential.rs`. ## Is this change properly documented? I'm unsure if this is a breaking change, since existing application code will still work, it would just be beneficial for them to adapt to not requiring mutability anymore. If someone has implemented `OAuthClient` for their own type, though, it will be a breaking change since the trait signature has changed. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>