rspotify icon indicating copy to clipboard operation
rspotify copied to clipboard

Provide a user-settable callback for when tokens are updated

Open PaulOlteanu opened this issue 2 years ago • 6 comments

Is your feature request related to a problem? Please describe

I'm creating a web-app that stores users' tokens in a database. After performing a Spotify request, I don't have a way to know if the token was refreshed, and therefore have no way to store the updated tokens in my database.

Describe the solution you'd like

I'd like a config option on the clients where I could provide a callback function that could be called when the token is refreshed, so that I can save it to my database.

I feel that this solution is much more "library-like" than the current one that optionally saves the token to a file.

Describe alternatives you've considered

One alternative that I've considered (that I don't think is very good) is to set a field on the client when the token is changed, that can be unset by a user once they've handled the new token.

Additional context

I could try to look into implementing this if we think it's a good thing to add to rspotify

PaulOlteanu avatar Apr 19 '23 23:04 PaulOlteanu

Good point!

provide a callback function that could be called when the token is refreshed

It's a great idea, and in order to keep compatibility and doesn't affect the existing user, the behavior of the default callback function should save the token into a file.

ramsayleung avatar Apr 21 '23 15:04 ramsayleung

I think it may be better to have this be a breaking change rather than try to keep the previous behaviour of reading/writing the token.

To me it seem strange to have that kind of stuff happening from a library (and in general I personally feel that there's other things that don't really belong as part of a library such as the env-file feature - I think the end user should be doing that themselves if they need envfile support).

Obviously I don't want to start conributing by dictating what should and shouldn't be done, but I thought I'd just share my opinion. Perhaps this could/should be discussed on a separate issue

PaulOlteanu avatar Apr 24 '23 17:04 PaulOlteanu

I've opened a draft PR in case anyone has some early feedback. I'm going to look into trying to use generics rather than trait objects to see how that impacts the ergonomics for the user. It would mean the clients would be generic over the functions the user provides as callbacks.

If anyone has opinions on either option (dyn Fn vs impl Fn) I'd love to hear them

PaulOlteanu avatar Apr 28 '23 00:04 PaulOlteanu

I think it may be better to have this be a breaking change rather than try to keep the previous behaviour of reading/writing the token.

To me it seem strange to have that kind of stuff happening from a library (and in general I personally feel that there's other things that don't really belong as part of a library such as the env-file feature - I think the end user should be doing that themselves if they need envfile support).

I agree with your point, it's better to be a breaking change rather than try to keep the previous behavior of reading/writing the token. But in order to minimize the impact of making a breaking change for user/developer, I prefer to provide a feature named token-file or other names, to provide a feature to user to keep everything unchanged, which is disabled by default.

ramsayleung avatar Apr 28 '23 02:04 ramsayleung

Yeah, apologies for not finishing this up myself sooner.

Anyway I found that due to the way generics are handled we wouldn't be able to make the clients generic over the callback type because Rust would require the function to implement Debug even if we store it in some wrapper type.

I linked some tests I wrote in your PR, thanks for finishing this up 👍

PaulOlteanu avatar Jun 01 '23 23:06 PaulOlteanu

Message to comment on stale issues. If none provided, will not mark issues stale

github-actions[bot] avatar Nov 29 '23 02:11 github-actions[bot]