Closed sthang closed 4 years ago
@sthang : Thanks for the PR 👏 . Have a couple of questions with your implementation :
I noticed that as part of the createToken
method, you are doing a check to see if the autoRefresh is enabled and if YES, the client sets up the autoRefresh
. This point forward depending on the autoRefreshIntervalInSeconds
field the auto-refresh of tokens goes on until its killed explicitly. But what if there is a call made to OAuthClient.prototype.refresh
explicitly ? Will it process the current refresh request into a Queue ?
You have updated the sample to stopAutoRefresh on demand as shown here. But the actual stopRefresh()
method is missing on the client. were you planning on updating it later?
@abisalehalliprasan : Thanks for the review.
No. If refresh is called explicitly it will be independent of the autoRefresh and has no impact on the timer. I thought about it and made it independent to keep the implementation simple. The alternative is to reset the timer on setInterval when refresh is called outside of autoRefresh. What do you think?
Here's the stopRefresh on the OAuthClient. The sample app is calling it on the oauthClient.
@sthang :
1.) We do not want the explicit token refresh call to coincide with the autoRefresh. The workaround seems fine but there is a one-off edge case where the two API calls might coincide(the probability is very low 😃). I will see if I could implement Queue
logic here.
Could you close this PR and create a PR for the branch autoRefresh
. Apologize for the re-do, I need to test this feature before actually releasing it. Thanks @sthang
No problem. I've updated the Pull request base to autoRefresh
.
Coverage increased (+0.006%) to 88.068% when pulling f5ca941892f4658545ed7763e854f50e48a90cb0 on sthang:develop into 6352c7589fef9d94aa0c4ca5b9b04c21b2066675 on intuit:autoRefresh.