Migrate threading model to Coroutines - long running branch #89
Conversation
This file contains 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
This class is relatively simple and difficult to test because of `java.utils.Timer`. That's why I decided to make migration to Kotlin right away, without unit tests coverage first.
The returned `Boolean` wasn't used anywhere.
Added a new test to validate the behavior of the FlushManager's queue when two timers are initiated sequentially. The test asserts that the queue flushes correctly as per the time of the first timer's start.
To give SDK time to prepare and trigger the HTTP request. 100ms is to small timeout for CI emulator. It's enough for local builds and Firebase Test Lab tests. There's no business logic mistake - just the CI emulator is extremly slow.
to `LocalStorageRepository`
The logic of validating queue is no longer relevant as discussed internally. This commits removes `QueueManager` completely, and replaces it with a thread-safe `InMemoryBuffer`, which holds newly recorded events and, as soon as possible, adds them to local persistence. The new event not added to persistence right away, because the local storage file might be occupied/locked by, e.g. process/coroutine of sending events. We do not want to make consumer of the library to wait for adding a new event ever. Hence, the `InMemoryRepository#buffer`.
By using mutual exclusion (`Mutex`). Also, remove the `persistEvent` method, as it's no longer needed actually.
Also, add a name for easier identification while debugging
To reduce overhead of constant loop, the `InMemoryBuffer` will check `buffer` list every second.
As `inMemoryBuffer` triggers asynchronous operation, this PR adds a listener that asserts starting flush queue after adding an event.
50 events batches are no longer a thing since we removed `QueueManager`
50 events batches are no longer a thing since we removed `QueueManager`
Actually, it was never working as we never were saving any value to those `SharedPreferences`. This change seems fine because we rely in vast majority of cases on AdvertisingId - only in case of lack of Google Play Services we fallback to `ANDROID_ID` which is the same until factory reset. Closes: #59
Makes creating fake objects less problematic by removing a need to call real object constructor.
Makes creating fake objects less problematic by removing a need to call real object constructor.
Makes creating fake objects less problematic by removing a need to call real object constructor.
Makes creating fake objects less problematic by removing a need to call real object constructor.
The `FlushManager` eventually invokes `SendEvents` which checks for the size of stored queue anyways. This change reduces unnecessary complexity and overhead. More context: #92 (comment)
This change decouples `ParselyFlushManager` and `ParselyTracker`. It also makes `FlushManagerTest` resistant to `ParselyTracker` implementation changes.
To improve readability and align with `eventsToSend.isEmpty()` check
Now, instead of multiple locks in case of removing or inserting data, we do the same with a single lock. See: #92 (comment)
If stored queue will be empty, on next execution of `FlushQueue`, the manager will be stopped anyway. #92 (comment)
Coroutines: send events flow
Engagement manager to coroutines
Refactor retrieving advertisement id to Coroutines
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
This PR groups PRs migrated the SDK from
AsyncTask/java.utils.TimerAPIs to Kotlin Coroutines. The migration has a few goals in mindContains changes from:
FlushManagerto Kotlin Coroutines #86