Fix: bug causes the subscription not updated correctly with a 400 error

[jinliu9508]

Description

One Line Summary

This PR fixes the issue that causes the subscription to not update correctly or results in a 400 error when calling Login, also handles this bad state when migrating from previous any version.

Details

Motivation

There is an edge case that when the app calls Login with an external ID and forced close within 5 seconds in a fresh install, all future Login calls will be responded with a 400 error and any update to the subscription will fail. This PR aims to prevent this scenario by updating the operation queue correctly and handle the bad state from migration.

Scope

• OperationRepo: no longer holds up the operation processing before the PostCreateDelay. The waiting for the PostCreateDelay is now integrated into delayBeforeNextExecution at the end of the execution.
• LoginUserOperationExecutor: can now detects if a Login request has no existing onesignalID or externalID but there exists an externalID in the identity model store. When this happens, it will modify the Login request to include the externalID from the model store.

Testing

Manual testing

###Steps to reproduce
This bug can happen strictly when a Login with an externalID is called after the anonymous Login response is received, but the app is force-closed before the 5 seconds PostCreateDelay has reached. One way to reproduce this issue is:

1. Fresh install the app.
2. Right after the subscription is created or receiving a 200 code from the Create User request, call Login with an external ID.
3. Quickly force close the app. Make sure step 2 and 3 are done within 5 seconds.
4. Re-open the app, observe that a 400 error code is received and any updates like subscribing is not updated in the dashboard.

###New Installs
Step 1-3 same as above
4. Re-open the app, wait for the server response and observe that Login with the external ID is successful.

###Updates
Step 1-3 same as above
4. upgrade to this version without uninstalling the previous version or clearing the storage, observe that Login with the externalID is successful.

Affected code checklist

• Notifications
  • Display
  • Open
  • Push Processing
  • Confirm Deliveries
• Outcomes
• Sessions
• In-App Messaging
• REST API requests
• Public API changes

Checklist

Overview

• I have filled out all REQUIRED sections above
• PR does one thing
  • If it is hard to explain how any codes changes are related to each other then it most likely needs to be more than one PR
• Any Public API changes are explained in the PR details and conform to existing APIs

Testing

• I have included test coverage for these changes, or explained why they are not needed
• All automated tests pass, or I explained why that is not possible
• I have personally tested this on my device, or explained why that is not possible

Final pass

• Code is as readable as possible.
  • Simplify with less code, followed by splitting up code into well named functions and variables, followed by adding comments to the code.
• I have reviewed this PR myself, ensuring it meets each checklist item
  • WIP (Work In Progress) is ok, but explain what is still in progress and what you would like feedback on. Start the PR title with "WIP" to indicate this.

---

This change is 

[jkasten2]

Let's add a test too

[jkasten2]

Could there be an issue with properties being set on the wrong user?
Example:

1. Login(A)
2. setTag(A)
3. Login(B)
4. setTag(B)

Could this cause User B to get both tag A and B, when it was intended for User A to get tag A and User B to get Tag B?

[jinliu9508]

I just tested this scenario and this is how the manual test is setup:

1. Fresh install on main branch, right after the anonymous user is created but before the post create delay, programmatically call Login(A), setTag(A), Login(B), setTag(B) altogether and close the app.
2. Switch to this branch and update the app.
3. Run the app and observe in the dashboard that both users are created successfully and each with the correct tag.

After analyzing the log closely, the code in this if statement will actually never hit for the scenario. OperationRepo will process the operations in this order:

1. Create subscription
2. Login anonymously
3. Refresh user (anonymous)
4. Login A AND transfer subscription
5. Set Alias for A but got 409
6. Refresh user (A)
7. Set Tag for A
8. Login B AND transfer subscription
9. Set Tag for B

Since operations like setting tags does not go with the Login call in one request and the handler for 409 conflict will take care of the IDs, this will not cause the user to be assigned with the wrong tag.

Further more, I tested out that this bug only happen when Login is interrupted with no other operations like set tag or another Login within a few seconds after app starts up.