[Fix] Prevent operations added to queue while it's processing from being executed without delay

[jkasten2]

Description

One Line Summary

Fixes new operations added to OperationRepo's queue while its executing from skipping the opRepoExecutionInterval delay.

Details

Motivation

Preventing a misbehaving app adding operations (such as addTag()) in a tight loop, causing a number of back-to-back network calls to OneSignal's backend.

Scope

Only affects OperationRepo's batching logic.

New OperationRepo Concept - Buckets

   /** *** Buckets ***
     * Purpose: Bucketing is a pattern we are using to help save network
     * calls. It works together with opRepoExecutionInterval to define
     * a time window operations can be added to the bucket.
     *
     * When enqueue() is called it creates a new OperationQueueItem with it's
     * bucket = enqueueIntoBucket. Just before we start processing a bucket we
     * enqueueIntoBucket++, this ensures anything new that comes in while
     * executing doesn't cause it to skip the opRepoExecutionInterval delay.
     *
     * NOTE: Bucketing only effects the starting operation we grab.
     *       The reason is we still want getGroupableOperations() to find
     *       other operations it can execute in one go (same network call).
     *       It's more efficient overall, as it lowers the total number of
     *       network calls.
     */

Future Considerations

There is still a problem if someone rapidity adds login operations which switches users. (such calling login() with different external_ids, or login() and logout() over and over). They would all be in the same bucket and each would be its own network call and we would call them back-to-back without a delay. This isn't a problem with things like addTag, since those can be grouped into one network call.
Ideas to address this:

1. Maybe we always wait after creating a user then?
   • We probably already need to do this to account for the 404 problem.
     • 404 problem meaning the backend bug where it can return 404 if doing an operation on something you just created
2. maxCount - We only process x number of operations back-to-back in a bucket. Then we enforce the delay.
3. rateLimit - Only make x number of network calls per y amount of time.

Related

This improves on the PR #2033, which fixed opRepoExecutionInterval from being skipped anytime two operations were called back-to-back.

Testing

Unit testing

1. Commit 0988977 with failing test to prove the batching issue
   • Failing test results
2. Commit 0f718b7 adding bucketing feature
   • Test now passes
   • add a eba7081 ensuring we don't have a off-by-one errors
3. Additional tests that cover lines changed (had no existing tests for lines)
   • starting OperationModelStore should be processed, following normal delay rules
   • ensure results from executeOperations are added to beginning of the queue

Manual testing

Manually tested on an Android 14 emulator, tested to ensure prompting for notifications works and updates the subscription correctly. As well as logging into a new user.

Also tested running the following code no longer reproduces the orignal issue:

    private void reproducesOperationRepoLoopingTooQuickly() {
        new Thread(() -> {
            while (true) {
                OneSignal.getUser().addTag("test", "Test");
                OneSignal.getUser().removeTag("test");
                OneSignal.getUser().addTag("test", "Test");
                OneSignal.getUser().removeTag("test");
                try {
                    Thread.sleep(1);
                } catch (InterruptedException e) {
                    throw new RuntimeException(e);
                }
            }
        }).start();
    }

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 

[nan-li]

Tested manually by enqueuing lots of addTags while operations are executing and requests are made every second or less. After PR changes, spam enqueuing makes requests every 5 seconds only.

Also tested with device offline, cached operations and any cached operation items initialize with bucket = 0. Behavior appears correct.

[jinliu9508]

Manually tested with increment of bucket ensuring the logic works correctly. Also tested other normal operations including login, push, logout to make sure it doesn't break existing features.

[jkasten2]

@nan-li @jinliu9508 appreciate both of you testing as well! And for the code review