Refresh fixes

[marioortizmanero]

Description

Some fixes for #262:

• Improved threading example
• New threading example for async with tasks
• Fix and tidy up the examples/with_auto_reauth.rs example
• Moves auto_reauth and refresh_token to the trait, since we already have refetch_token available.
• Avoids unbounded recursivity (stack overflow)
• Simplifies everything a lot by only refreshing the token automatically when the request is being made, and not whenever it's accessed to. Thus, auto_reauth is only called once, and we can avoid having two token getters (one that refreshes and another that doesn't).
• Token::is_expired now includes a margin of 10 seconds
• Token::can_reauth was wrong: it checked if refresh_token was Some(), but that's not true in the case of client credentials.

Motivation and Context

See #262

Dependencies

None

Type of change

Please delete options that are not relevant.

• Bug fix (non-breaking change which fixes an issue)
• 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?

You can run the new test with:

cargo run --example tasks --features=cli,env-file

[AndreasBackx]

Just wanted to say thanks for continuing this work! Just updated a small project of mine and I'm so happy caching and auto refresh works (haven't tried the auto refresh just yet, but caching does work). ❤️ Noticed the stack overflow issue and then I saw you fixed it 5 minutes before I saw it. 😅

[marioortizmanero]

@AndreasBackx thanks a bunch! These messages are really encouraging :) I'm really proud of how the interface of rspotify has turned out to be.

If you have any issues let us know! I think we'll release the new version very soon, probably ignoring #221 for now.

[ramsayleung]

@marioortizmanero Thanks for your contribution, you have done such tons of brilliant improvement

[ramsayleung]

Is it necessary to close write channel by calling drop explicitly? It seems wr will be dropped automatically when it goes out of scope?

[marioortizmanero]

Try removing it and see how it never ends. The thing is that we have a write channel per task/thread, and then also the original one in the main function, which we cloned and passed to the tasks/threads. Since the loop (which is in the main function) waits until all write channels have been dropped, if we don't manually drop the one in main it will never end.

Another way to do it would be with a function or scoping, but I figured this was simple enough.

[ramsayleung]

It's important to not leave the token locked

How do not leave the token locked? The should_reauth acquired the lock before calling refresh_token, and refresh_token needs to acquire the lock internally, but the lock has been held by auto_reauth function, a deadlock will occur at the end?

[marioortizmanero]

No, the thing is that in should_reauth the lock is dropped after turning it into a boolean because it's a single statement.

If it was a two step process and we fist locked the mutex in a separate variable and then checked should_reauth, then we'd have a problem because the mutex is still in scope.

This is kinda what happened before. We had an if let statement that checked the value of the mutex, and inside that we accessed the mutex again. But by then the mutex hadn't been dropped.

Actually, the previous way of doing it might work as well because since we're matching against the token in the if let we don't really have the mutex in scope anymore. But I think this leaves it clearer that there's no mutex anymore.

[ramsayleung]

No, the thing is that in should_reauth the lock is dropped after turning it into a boolean because it's a single statement.

This sounds like a temporary value doesn't live long enough. You did a great job