Parallel session refresh

GeoffreyHuck commented 6 months ago

Related to #1022

This PR includes two changes:

When we call the service to refresh with a token that is not the most recent one for the session, we return the most recent one, with its expiration time
When we call the service to refresh with a token that is the most recent one, and if it has been issued less than 5 minutes ago, we just return the current token without refreshing, with the expiration time
(actual refresh) When we call the service to refresh with a token that is the most recent one, and if it as been issued more than 5 minutes ago, we refresh the token

Added a new *.feature file for the new behavior that uses the new testing system
The new testing system now handles sessions and access tokens
Updated the older tests. The times have been updated to make it easier to read, and to make the old tests compliant with the new behaviors.
Updated the service swagger doc

Easier to read commit by commit, with the details in commit descriptions.

The lock `userID` have been changed to be on `sessionID`

There was a lock on userID to prevent multiple refresh concurrently. It was modified to be on the sessionID instead, because we don't want to have a lock waiting on one device because we coincidentally have a refresh on another device for the same user.

I had a bug with it (two last commits), because a test was using the lock manually without clearing it, a another test was blocked because it uses the same sessionID. It's weird to use the lock manually instead of using the implementation function withLock, which would have cleaned it automatically. Anyway, the test was updated to clear the lock, to fix the bug.

codecov[bot] commented 6 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 100.00%. Comparing base (d033fd6) to head (961809d).

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #1034 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 234 234 Lines 14034 14056 +22 ========================================= + Hits 14034 14056 +22 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

smadbe commented 6 months ago

This PR includes two changes:

1. If we try to refresh with a token that is not the most recent one for the session, we return the most recent one, with the expiration time

2. If a session has been refreshed less than 5 minutes ago, we just return the current token without refreshing, with the expiration time

Sorry, it is not clear to me:

if (1) does this without refreshing... if it is without refreshing I don't understand why it is mentioned like the next line, if it is with refresh
(2) Is it "If we try to refresh ... " condition as well?
(2) not clear what "current token" is ... because the current token may be almost expired with a session refreshed less than 5min ago... in which case returning the current token is not ok
as the 2 conditions are not exclusive... what happens if both applies (depends a bit on the answer of my prev questions)

GeoffreyHuck commented 6 months ago

If we try to refresh with a token that is the most recent one, and if it has been issued less than 5 minutes ago, we just return the current token without refreshing, with the expiration time
(actual refresh) If we try to refresh with a token that is the most recent one, and if it as been issued more than 5 minutes ago, we refresh the token

France-ioi / AlgoreaBackend