[SDK-342] - AuthRetry logic #987
Draft
+525
−17
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Includes 1. getRemoteConfiguration checking for the flag. And assuming false if no values exist. Also stores in sharedPreferences to load from it next time. 2. isAutoRetryOnJWTFailure on `IterableAPI` which other classes with use. Not public. 3. Introduced three states which Taskmanager can rely on. VALID, INVALID, UNKNOWN. 4. Listener pub/sub added which will invoke onAuthTokenReady() on listner class. Adding array of AuthTokenReadyListener on Authmanager and its add and remove list methods. 5. Right now authHandler null returns true hoping to bypass jwt. But JWT based request will fail if API needs jwt. This could pile up storage. But this seems like a valid scenario 6. handleAuthTokenSucess sets authtoken to unknown when a new token is set. I hope it should be right approach. Because its new and unknown at this point. 7. `IterableConfig` has final boolean `autoRetryOnJwtFailure`. Its not something I want customers to be able to configure. It should be removed before going to master. It should be internal only variable. 8. IterableRequestTask - has some changes introduced due to the flow not falling under jwt failure. Response code coming has -1 was the root cause. Hence the change around responseCode >= 0 && responseCode < 400 9. `IterableRequestTask` - Line 215 - Still not sure if RequestProcessor check should happen. It does make sense to have it as only offline stored request will be reprocessed. And this feature should only work for those requests going through offline Request processor 10. `IterableRequestTask` - fromJSON method change is something I havent checked properly. I think its for unit testing the changes are done. 11. IterableTaskRunner. Would like to see if I can remove the method - isJWTFailure(responseCode). May be its a small abstraction thats needed. 12. Havent checked unit tests properly yet
| * The remote configuration flag takes precedence when present; | ||
| * otherwise the local {@link IterableConfig#autoRetryOnJwtFailure} value is used. | ||
| */ | ||
| boolean isAutoRetryOnJwtFailure() { |
Contributor
There was a problem hiding this comment.
is there a reason for it not to be private or public?
| try { | ||
| BufferedReader in; | ||
| if (responseCode < 400) { | ||
| if (responseCode >= 0 && responseCode < 400) { |
Contributor
There was a problem hiding this comment.
is there a 0 code that can occur?
| } else { | ||
| in = new BufferedReader( | ||
| new InputStreamReader(urlConnection.getErrorStream())); | ||
| InputStream errorStream = urlConnection.getErrorStream(); |
Contributor
There was a problem hiding this comment.
Isn't it possible to extract just this flow of getting the resultResult to a helper class? this part is quite verbose
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🔹 Jira Ticket(s) if any
✏️ Description
IterableAPIwhich other classes with use. Not public.IterableConfighas final booleanautoRetryOnJwtFailure. Its not something I want customers to be able to configure. It should be removed before going to master. It should be internal only variable.IterableRequestTask- Line 215 - Still not sure if RequestProcessor check should happen. It does make sense to have it as only offline stored request will be reprocessed. And this feature should only work for those requests going through offline Request processorIterableRequestTask- fromJSON method change is something I havent checked properly. I think its for unit testing the changes are done.