Add retry for auto send #6071
Add retry for auto send #6071
Conversation
|
|
||
| private val projectDependencyProvider = projectsDependencyProviderFactory.create(project.uuid) | ||
|
|
||
| private val httpInterface = mock<OpenRosaHttpInterface>() |
There was a problem hiding this comment.
I think we'd ideally be setting our boundary at a level more similar to a reworked InstanceUploader that resembles our FormSource interface, but it wasn't painful to just use the OpenRosaHttpInterface (as we just need an HTTP failure here). That's a further refactor for another day!
seadowg
force-pushed
the
auto-send-retty
branch
from
b9f1cd1 to
252f9c2
Compare
seadowg
force-pushed
the
auto-send-retty
branch
from
252f9c2 to
0400b96
Compare
|
@grzesiek2010 It'd be good to prioritise this as I'm needing some of the changes here in the next set of offline entities work. |
Almost done reviewing the changes. I will finish soon. |
collect_app/src/main/java/org/odk/collect/android/injection/config/AppDependencyModule.java
Outdated
Show resolved
Hide resolved
| @@ -77,6 +77,11 @@ interface Scheduler { | |||
| fun cancelAllDeferred() | |||
|
|
|||
| fun <T> flowOnBackground(flow: Flow<T>): Flow<T> | |||
|
|
|||
| enum class NetworkType { | |||
There was a problem hiding this comment.
Can we specify the third type WIFI_AND_CELLULAR so that we don't have to support null values?
There was a problem hiding this comment.
I was thinking that it's really a constraint so it can be on one of the two network types or null (no constraint). Would you prefer it if the param on networkDeferred was networkConstraint (and it takes an optional NetworkType still) instead of networkType?
There was a problem hiding this comment.
yeah maybe it would make more sense.
| @@ -67,9 +68,17 @@ public void cancelUpdates(String projectId) { | |||
|
|
|||
| @Override | |||
| public void scheduleSubmit(String projectId) { | |||
| Scheduler.NetworkType networkType = null; | |||
| Settings settings = settingsProvider.getUnprotectedSettings(projectId); | |||
| if (settings.getString(ProjectKeys.KEY_AUTOSEND).equals("wifi_only")) { | |||
There was a problem hiding this comment.
I think it would be nice to create consts for cellular_only and cellular_only as we use those values ina couple of places.
collect_app/src/test/java/org/odk/collect/android/backgroundwork/AutoSendTaskSpecTest.kt
Outdated
Show resolved
Hide resolved
| return parse(context, setting) | ||
| } | ||
|
|
||
| private inline fun <reified T> parse( |
There was a problem hiding this comment.
This is a pretty intense method signature, but it prevents us needing to create a parse for each of these enum settings (which I can see us adding more of).
We could actually also have a Settings.getEnum extension that would remove the need for the getFormUpdateMode/getAutoSend methods, but this ends up not being callable from Java sadly (due to the use of inline/reified).
|
Tested with Success! Verified cases:
|
|
Tested with Success! |
|
Tested with Success! |
Closes #5946
I ended up doing some restructuring here to make everything easier to change/test. This mostly involved moving the auto send logic to
InstancesDataServiceand also using (new) network constraints inSchedulerfor the wifi/celluar/both options instead of checking during the background job.Why is this the best possible solution? Were any other approaches considered?
New tests and verified manually.
How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?
How auto send jobs get scheduled has been changed, so it's worth playing around with them in different wifi/cellular/both setups to check everything still works as expected.
If auto send fails, it should now retry 1 minute later and then it should "back off" exponentially after that (each subsequent retry will be further apart). Retries will happen infinitely until all finalised forms have been sent.
Before submitting this PR, please make sure you have:
./gradlew connectedAndroidTest(or./gradlew testLab) and confirmed all checks still pass