consentGiven true forces wake when delaying for retry#2109
Conversation
Retriable errors now use waiter instead of delay and it can only be interrupted by force calls to waiter.wake or waiting the full retry duration.
| } | ||
| } | ||
|
|
||
| override fun forceProcessDeltas() { |
There was a problem hiding this comment.
I think this should be called forceExecuteOperations, since we have the method name executeOperations in this code base.
| while (!wakeMessage.force) { | ||
| val waitedTheFullTime = | ||
| withTimeoutOrNull(delayFor) { | ||
| wakeMessage = waiter.waitForWake() |
There was a problem hiding this comment.
Thinking about this again it would be better to create a different waiter for this. (calling it something like retryWaiter maybe?). After seeing the code again I have concerns how things will interact on the same waiter, as I don't know think calling wake() will chain to both waits. Sorry for the orignal suggestion of using the same waiter could work.
So here I think we can simply this logic, remove waitedTheFullTime as we want to break out of this wait right away if consent becomes true. Then we can call wake() on orignal waiter, without force or a time so it counts it as a "normal" operation, so we can batch other things like tags together.
| withTimeoutOrNull(delayFor) { | ||
| retryWaiter.waitForWake() | ||
| } | ||
| waiter.wake(LoopWaiterMessage(false, 0)) |
There was a problem hiding this comment.
I believe this is a better fit in forceExecuteOperations().
| } | ||
|
|
||
| override fun forceExecuteOperations() { | ||
| retryWaiter.wake(LoopWaiterMessage(true, 0)) |
There was a problem hiding this comment.
nit, we can drop the 2nd param of LoopWaiterMessage, it defaults to 0.
| @@ -1,5 +1,6 @@ | |||
| package com.onesignal.core.internal.operations.impl | |||
|
|
|||
| import android.os.Looper | |||
There was a problem hiding this comment.
It doesn't looks like the new import is used. (it is probably failing ktlint).
emawby
force-pushed
the
fix/cancel_wait_after_consent_given
branch
from
5d901ab to
151ec95
Compare
emawby
changed the title
emawby
force-pushed
the
fix/cancel_wait_after_consent_given
branch
from
f071a47 to
8d98222
Compare
2 participants
Description
One Line Summary
Retriable errors now use waiter instead of delay and it can only be interrupted by force calls to waiter.wake or waiting the
Details
We were previously using delay() to wait to retry failed server requests. This included privacy consent not given. This PR adds a way to interrupt that delay when privacy consent goes from false to true.
We cannot use delay() for this so it uses the waiter instead. This has the side effect of making all retry-able errors use the waiter.
Motivation
We want a way to immediately process deltas when privacy consent goes from false to true.
Scope
Retry-able operations and privacy consent given
Testing
Unit testing
OPTIONAL - Explain unit tests added, if not clear in the code.
Manual testing
Tested not giving privacy consent to ensure we were properly delaying retries. Then tested providing privacy consent and ensured that the operations were sent immediately.
Affected code checklist
Checklist
Overview
Testing
Final pass
This change is