JWT improvements Part 1 #718

hardikmashru · 2024-04-03T13:35:27Z

Provide a method to stop JWT retries (regardless of retry policy)
Make it possible to specify a retry policy (on IterableConfig)

🔹 Jira Ticket(s) if any

MOB-XXXX

✏️ Description

Please provide a brief description of what this pull request does.

1. Provide a method to stop JWT retries (regardless of retry policy) 2. Make it possible to specify a retry policy (on IterableConfig)

codecov · 2024-04-03T13:47:14Z

Codecov Report

Attention: Patch coverage is 65.78947% with 13 lines in your changes are missing coverage. Please review.

❗ No coverage uploaded for pull request base (feature/JWTImprovement@e95295f). Click here to learn what that means.

❗ Current head 680b6fa differs from pull request most recent head 3bfe86d. Consider uploading reports for the commit 3bfe86d to get more accurate results

Files	Patch %	Lines
.../com/iterable/iterableapi/IterableAuthManager.java	47.36%	7 Missing and 3 partials ⚠️
.../java/com/iterable/iterableapi/IterableConfig.java	50.00%	2 Missing ⚠️
...ain/java/com/iterable/iterableapi/IterableApi.java	75.00%	1 Missing ⚠️

Additional details and impacted files

@@                    Coverage Diff                    @@
##             feature/JWTImprovement     #718   +/-   ##
=========================================================
  Coverage                          ?   61.73%           
=========================================================
  Files                             ?       76           
  Lines                             ?     4728           
  Branches                          ?      536           
=========================================================
  Hits                              ?     2919           
  Misses                            ?     1506           
  Partials                          ?      303

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Ayyanchira

Added some comments @hardikmashru ! Really great PR.
Can we have a feature branch and first have all the changes merged into the feature branch first? That way we can test the entire branch once ready and then merge to master

Ayyanchira · 2024-04-03T13:38:20Z

app/src/main/java/com/iterable/androidsdk/MainActivity.java

+                .setMaxRetries(4)
+                .setRetryBackoff(RetryPolicy.LINEAR)
+                .setRetryInterval(2L)


This can go under one method -> config.setAuthRetryPolicy

app/src/main/java/com/iterable/androidsdk/MainActivity.java

Ayyanchira · 2024-04-03T13:51:22Z

iterableapi/src/main/java/com/iterable/iterableapi/IterableApi.java

@@ -355,6 +355,7 @@ private void onLogin(@Nullable String authToken) {
            return;
        }

+        pauseAuthRetries(false);


Thinking over - What if we call this method in setAuthToken function?
That way, Not just during logging in, but also when setAuthToken is explicitly called, we can resume the retry functionality.

@Ayyanchira I tried with that first, but setAuthToken is called internally from the SDK in the success scenario and in other methods which would reset the retry and pauseAuthRetry and break the retry logic we have written. Instead I have done this pauseAuthRetries call on the success of the API call so eventually in case of a valid token things will reset correctly.

iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthManager.java

Ayyanchira · 2024-04-03T13:55:33Z

iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthManager.java

+    }
+
+    void resetRetryCount() {
+        if (retryCount > 0) retryCount = -1;


Not sure about this.
Why is it not simply assigning the counter to 0?
Is it because there is a retry in progress which will increase the count to 0?

Ayyanchira · 2024-04-03T14:09:57Z

iterableapi/src/main/java/com/iterable/iterableapi/IterableRequestTask.java

@@ -330,6 +330,8 @@ public void run() {

    private void handleSuccessResponse(IterableApiResponse response) {
        IterableApi.getInstance().getAuthManager().resetFailedAuth();
+        IterableApi.getInstance().getAuthManager().pauseAuthRetries(false);


There could be success response for certain APIs which do not go through JWT check in the backend. Resetting auth could be problematic there. One such method is getRemoteConfiguration() and in future - disableDevice

Either a bypass for the above method - getRemoteConfiguration
OR some other logic should be considered

I guess, we can put a check for these APIs and not call the pauseAuthRetries

Ayyanchira · 2024-04-03T14:11:39Z

iterableapi/src/main/java/com/iterable/iterableapi/RetryPolicy.java

Looks like this is in progress. Will wait for the next part to comment on it.

Retry Policy code is already there, I have just used an enum for the values.

hardikmashru · 2024-04-03T14:20:50Z

Added some comments @hardikmashru ! Really great PR. Can we have a feature branch and first have all the changes merged into the feature branch first? That way we can test the entire branch once ready and then merge to master

Thanks. Yes, I can merge all into the one feature branch after other 2 points are done. What will be the name of the branch?

Ayyanchira · 2024-04-03T14:22:34Z

Added some comments @hardikmashru ! Really great PR. Can we have a feature branch and first have all the changes merged into the feature branch first? That way we can test the entire branch once ready and then merge to master

Thanks. Yes, I can merge all into the one feature branch after other 2 points are done. What will be the name of the branch?

-> feature/JWTRetryEnhancement

* JWT improvements 1. Provide a method to stop JWT retries (regardless of retry policy) 2. Make it possible to specify a retry policy (on IterableConfig) * removed sample app changes * sample app revert * sample app revert * updates * Update IterableConfig.java * Update IterableRequestTask.java

Ayyanchira · 2024-04-09T04:33:07Z

@hardikmashru , lets have the base branch be a feature branch instead of master. Changes are looking great.

However, I still see that retries firing off instantly and not respecting time intervals set especially when SDK makes getMessages, getEmbeddedMessages and registerDevice call in the app startup.

merge fixes

Ayyanchira

Added some last comments

iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthManager.java

iterableapi/src/main/java/com/iterable/iterableapi/IterableRequestTask.java

Ayyanchira · 2024-04-19T09:24:53Z

Github actions - Checkstyle and a breaking unit test to be fixed before this can be merged in

This will allow Iterable tester apps to read values from auth Manager for testing purpose

Ayyanchira · 2024-05-13T16:59:18Z

Instrumentation test fails due to yml changes which are there in master branch. And the changes has not propagated to feature/JWTImprovement. Hence skipping the instrumentation test.

JWT improvements

8b95b5c

1. Provide a method to stop JWT retries (regardless of retry policy) 2. Make it possible to specify a retry policy (on IterableConfig)

Ayyanchira reviewed Apr 3, 2024

View reviewed changes

hardikmashru added 3 commits April 3, 2024 20:07

removed sample app changes

01d9fd1

sample app revert

2afca4c

sample app revert

281a19e

evantk91 added the omni-cg PR's from OMNI CG label Apr 3, 2024

hardikmashru and others added 5 commits April 4, 2024 19:17

updates

4dfafb9

Update IterableConfig.java

680b6fa

Update IterableRequestTask.java

8d860c4

Merge branch 'JWT_Imrovement_Part2' into feature/JWTRetryEnhancement

e69ca65

Implemented suggested changes

cd972d3

hardikmashru changed the base branch from master to feature/JWTRetryEnhancement April 11, 2024 14:02

hardikmashru added 4 commits April 15, 2024 17:04

updates for retry mechanism

9cb4909

Update IterableRequestTask.java

95497d8

updates

f8d7b74

further improvements on parallel API calls to respect retrycount

eb136f9

hardikmashru changed the base branch from feature/JWTRetryEnhancement to feature/JWTImprovement April 16, 2024 08:53

hardikmashru and others added 4 commits April 16, 2024 21:25

Merge branch 'feature/JWTRetryEnhancement' into JWT_Imrovement_Part1

71734e0

Update IterableRequestTask.java

c3d1cf4

merge fixes

conflict fixes

0ea96a2

authmanager reset function added

d270495

Ayyanchira reviewed Apr 18, 2024

View reviewed changes

iterableapi/src/main/java/com/iterable/iterableapi/IterableAuthManager.java Outdated Show resolved Hide resolved

iterableapi/src/main/java/com/iterable/iterableapi/IterableRequestTask.java Outdated Show resolved Hide resolved

did suggested changes

fe9a166

style updates

5980927

hardikmashru and others added 9 commits April 19, 2024 15:21

checkstyle fixes

7fad778

Update IterableAuthManager.java

08dfedc

removed Part 2 code

32b9922

Update IterableRequestTask.java

03861a7

Update IterableRequestTask.java

cc80f40

Update IterableRequestTask.java

b6c8a14

handled null and exception scenario for authtoken

95687c2

Making some properties package friendly

4074a89

This will allow Iterable tester apps to read values from auth Manager for testing purpose

Merge branch 'feature/JWTImprovement' into JWT_Imrovement_Part1

3bfe86d

Ayyanchira merged commit ec5ba3d into feature/JWTImprovement May 13, 2024
3 of 4 checks passed

Ayyanchira deleted the JWT_Imrovement_Part1 branch May 13, 2024 16:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JWT improvements Part 1 #718

JWT improvements Part 1 #718

hardikmashru commented Apr 3, 2024

codecov bot commented Apr 3, 2024 •

edited

Loading

Ayyanchira left a comment

Ayyanchira Apr 3, 2024

hardikmashru Apr 3, 2024

Ayyanchira Apr 3, 2024

hardikmashru Apr 3, 2024

Ayyanchira Apr 3, 2024

Ayyanchira Apr 3, 2024

hardikmashru Apr 3, 2024

Ayyanchira Apr 3, 2024

hardikmashru Apr 3, 2024

hardikmashru commented Apr 3, 2024

Ayyanchira commented Apr 3, 2024

Ayyanchira commented Apr 9, 2024

Ayyanchira left a comment

Ayyanchira commented Apr 19, 2024

Ayyanchira commented May 13, 2024

JWT improvements Part 1 #718

JWT improvements Part 1 #718

Conversation

hardikmashru commented Apr 3, 2024

🔹 Jira Ticket(s) if any

✏️ Description

codecov bot commented Apr 3, 2024 • edited Loading

Codecov Report

Ayyanchira left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hardikmashru commented Apr 3, 2024

Ayyanchira commented Apr 3, 2024

Ayyanchira commented Apr 9, 2024

Ayyanchira left a comment

Choose a reason for hiding this comment

Ayyanchira commented Apr 19, 2024

Ayyanchira commented May 13, 2024

codecov bot commented Apr 3, 2024 •

edited

Loading