Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MLIBZ-2719: Prevent multiple calls to refresh token #330

Merged
merged 6 commits into from
Oct 26, 2018

Conversation

heyzooi
Copy link
Contributor

@heyzooi heyzooi commented Oct 22, 2018

Description

Prevent multiple calls to refresh token which was causing a race condition

Changes

  • User, Client and HttpRequest operations are queued when a 401 response is returned

Tests

  • Unit test added to reproduce a multiple calls being made when the token needs to be refreshed

@heyzooi heyzooi self-assigned this Oct 22, 2018
if let user = self.credential as? User {
user.logout()
DispatchQueue.global(qos: .default).async {
guard !client.refreshingToken else {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the client.refreshingToken made thread-safe because it is being accessed and set from within the DispatchQueue?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, so this is a serial queue and I'm calling wait() on the DispatchGroup so the combination of both makes the entire refresh token process thread-safe

@codecov-io
Copy link

codecov-io commented Oct 23, 2018

Codecov Report

Merging #330 into develop will increase coverage by 0.05%.
The diff coverage is 98.55%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #330      +/-   ##
===========================================
+ Coverage     86.3%   86.35%   +0.05%     
===========================================
  Files           70       70              
  Lines         8431     8470      +39     
===========================================
+ Hits          7276     7314      +38     
- Misses        1155     1156       +1
Flag Coverage Δ
#Mac 84.52% <98.55%> (+0.14%) ⬆️
#iOS 86.29% <98.55%> (-0.02%) ⬇️
Impacted Files Coverage Δ
Kinvey/Kinvey/Client.swift 86% <ø> (ø) ⬆️
Kinvey/Kinvey/User.swift 87.59% <100%> (+0.02%) ⬆️
Kinvey/Kinvey/HttpRequest.swift 83.45% <98.5%> (+2.12%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9cc84cf...893f22b. Read the comment docs.

@heyzooi heyzooi merged commit c465c82 into develop Oct 26, 2018
@heyzooi heyzooi deleted the feature/MLIBZ-2719-refresh_token_race_condition branch October 26, 2018 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants