Open PaulOlteanu opened 1 year ago
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.
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
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
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.
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 👍
Message to comment on stale issues. If none provided, will not mark issues stale
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