Merge pull request #5867 from seadowg/db-anr

Fix DB ANRs during form entry
getodk · Jan 22, 2024 · a57d49b · a57d49b
2 parents f463746 + f5fd65c
commit a57d49b
Show file tree

Hide file tree

Showing 38 changed files with 937 additions and 771 deletions.
diff --git a/androidshared/src/main/java/org/odk/collect/androidshared/async/TrackableWorker.kt b/androidshared/src/main/java/org/odk/collect/androidshared/async/TrackableWorker.kt
@@ -18,4 +18,13 @@ class TrackableWorker(private val scheduler: Scheduler) {
             foreground.accept(result)
         }
     }
+
+    fun immediate(background: Runnable) {
+        immediate(
+            background = {
+                background.run()
+            },
+            foreground = {}
+        )
+    }
 }
diff --git a/androidtest/src/main/java/org/odk/collect/androidtest/LiveDataTestUtils.kt b/androidtest/src/main/java/org/odk/collect/androidtest/LiveDataTestUtils.kt
@@ -22,19 +22,18 @@ fun <T> LiveData<T>.getOrAwaitValue(
 ): T {
     var data: T? = null
     val latch = CountDownLatch(1)
-    val observer = object : Observer<T> {
-        override fun onChanged(o: T) {
-            data = o
-            latch.countDown()
-            this@getOrAwaitValue.removeObserver(this)
-        }
+    val observer = Observer<T> { o ->
+        data = o
+        latch.countDown()
     }
-    this.observeForever(observer)
 
+    this.observeForever(observer)
     afterObserve.invoke()
 
     // Don't wait indefinitely if the LiveData is not set.
-    if (!latch.await(time, timeUnit)) {
+    if (latch.await(time, timeUnit)) {
+        this.removeObserver(observer)
+    } else {
         this.removeObserver(observer)
         throw TimeoutException("LiveData value was never set.")
     }

diff --git a/async/src/main/java/org/odk/collect/async/CoroutineScheduler.kt b/async/src/main/java/org/odk/collect/async/CoroutineScheduler.kt
@@ -2,6 +2,8 @@ package org.odk.collect.async
 
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.delay
+import kotlinx.coroutines.flow.Flow
+import kotlinx.coroutines.flow.flowOn
 import kotlinx.coroutines.isActive
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.withContext
@@ -18,9 +20,15 @@ open class CoroutineScheduler(private val foregroundContext: CoroutineContext, p
         }
     }
 
-    override fun immediate(foreground: Runnable) {
-        CoroutineScope(foregroundContext).launch {
-            foreground.run()
+    override fun immediate(background: Boolean, runnable: Runnable) {
+        val context = if (background) {
+            backgroundContext
+        } else {
+            foregroundContext
+        }
+
+        CoroutineScope(context).launch {
+            runnable.run()
         }
     }
 
@@ -37,6 +45,10 @@ open class CoroutineScheduler(private val foregroundContext: CoroutineContext, p
         return ScopeCancellable(repeatScope)
     }
 
+    override fun <T> flowOnBackground(flow: Flow<T>): Flow<T> {
+        return flow.flowOn(backgroundContext)
+    }
+
     override fun cancelAllDeferred() {
         throw UnsupportedOperationException()
     }

diff --git a/async/src/main/java/org/odk/collect/async/Scheduler.kt b/async/src/main/java/org/odk/collect/async/Scheduler.kt
@@ -1,5 +1,6 @@
 package org.odk.collect.async
 
+import kotlinx.coroutines.flow.Flow
 import java.util.function.Consumer
 import java.util.function.Supplier
 
@@ -22,9 +23,9 @@ interface Scheduler {
     fun <T> immediate(background: Supplier<T>, foreground: Consumer<T>)
 
     /**
-     * Run work in the foreground. Cancelled if application closed.
+     * Run work in the foreground or background. Cancelled if application closed.
      */
-    fun immediate(foreground: Runnable)
+    fun immediate(background: Boolean = false, runnable: Runnable)
 
     /**
      * Schedule a task to run in the background even if the app isn't running. The task
@@ -74,4 +75,10 @@ interface Scheduler {
     fun repeat(foreground: Runnable, repeatPeriod: Long): Cancellable
 
     fun cancelAllDeferred()
+
+    fun <T> flowOnBackground(flow: Flow<T>): Flow<T>
+}
+
+fun <T> Flow<T>.flowOnBackground(scheduler: Scheduler): Flow<T> {
+    return scheduler.flowOnBackground(this)
 }
diff --git a/async/src/main/java/org/odk/collect/async/SchedulerAsyncTaskMimic.kt b/async/src/main/java/org/odk/collect/async/SchedulerAsyncTaskMimic.kt
@@ -70,7 +70,7 @@ abstract class SchedulerAsyncTaskMimic<Params, Progress, Result>(private val sch
 
     protected fun publishProgress(vararg values: Progress) {
         scheduler.immediate(
-            foreground = { onProgressUpdate(*values) }
+            runnable = { onProgressUpdate(*values) }
         )
     }
 }
diff --git a/...oidTest/java/org/odk/collect/android/feature/formentry/ExternalSecondaryInstanceTest.java b/...oidTest/java/org/odk/collect/android/feature/formentry/ExternalSecondaryInstanceTest.java
@@ -8,7 +8,6 @@
 import org.junit.runner.RunWith;
 import org.odk.collect.android.support.rules.CollectTestRule;
 import org.odk.collect.android.support.rules.TestRuleChain;
-import org.odk.collect.android.support.pages.MainMenuPage;
 
 import java.util.Collections;
 
@@ -32,15 +31,24 @@ public class ExternalSecondaryInstanceTest {
             .around(rule);
 
     @Test
-    public void displaysAllOptionsFromSecondaryInstance() {
-        //TestCase1
-        new MainMenuPage()
-                .copyForm("external_select_10.xml", Collections.singletonList("external_data_10.xml"))
-                .startBlankForm("external select 10")
-                .clickOnText("a")
-                .swipeToNextQuestion("Second")
-                .assertText("aa")
-                .assertText("ab")
-                .assertText("ac");
+    public void displaysAllOptionsFromXMLSecondaryInstance() {
+        rule.startAtMainMenu()
+                .copyForm("external_select.xml", Collections.singletonList("external_data.xml"))
+                .startBlankForm("external select")
+                .assertQuestion("First")
+                .assertText("One")
+                .assertText("Two")
+                .assertText("Three");
+    }
+
+    @Test
+    public void displaysAllOptionsFromCSVSecondaryInstance() {
+        rule.startAtMainMenu()
+                .copyForm("external_select_csv.xml", Collections.singletonList("external_data.csv"))
+                .startBlankForm("external select")
+                .assertQuestion("First")
+                .assertText("One")
+                .assertText("Two")
+                .assertText("Three");
     }
 }
diff --git a/...formentry/DynamicPreLoadedDataPullTest.kt → ...icpreload/DynamicPreLoadedDataPullTest.kt b/...formentry/DynamicPreLoadedDataPullTest.kt → ...icpreload/DynamicPreLoadedDataPullTest.kt
@@ -1,4 +1,4 @@
-package org.odk.collect.android.feature.formentry
+package org.odk.collect.android.feature.formentry.dynamicpreload
 
 import org.junit.Rule
 import org.junit.Test

diff --git a/...p/src/androidTest/java/org/odk/collect/android/feature/formmanagement/MatchExactlyTest.kt b/...p/src/androidTest/java/org/odk/collect/android/feature/formmanagement/MatchExactlyTest.kt
@@ -180,6 +180,6 @@ class MatchExactlyTest {
             .enableMatchExactly()
             .enableManualUpdates()
 
-        assertThat(testDependencies.scheduler.deferredTasks, `is`(empty()))
+        assertThat(testDependencies.scheduler.getDeferredTasks(), `is`(empty()))
     }
 }
diff --git a/...dTest/java/org/odk/collect/android/feature/formmanagement/PreviouslyDownloadedOnlyTest.kt b/...dTest/java/org/odk/collect/android/feature/formmanagement/PreviouslyDownloadedOnlyTest.kt
@@ -153,6 +153,6 @@ class PreviouslyDownloadedOnlyTest {
             .enablePreviouslyDownloadedOnlyUpdates()
             .enableManualUpdates()
 
-        assertThat(testDependencies.scheduler.deferredTasks, equalTo(emptyList()))
+        assertThat(testDependencies.scheduler.getDeferredTasks(), equalTo(emptyList()))
     }
 }
diff --git a/...c/androidTest/java/org/odk/collect/android/feature/settings/FormManagementSettingsTest.kt b/...c/androidTest/java/org/odk/collect/android/feature/settings/FormManagementSettingsTest.kt
@@ -8,7 +8,6 @@ import org.junit.Rule
 import org.junit.Test
 import org.junit.rules.RuleChain
 import org.junit.runner.RunWith
-import org.odk.collect.android.R
 import org.odk.collect.android.support.TestDependencies
 import org.odk.collect.android.support.pages.MainMenuPage
 import org.odk.collect.android.support.rules.CollectTestRule
@@ -27,7 +26,7 @@ class FormManagementSettingsTest {
 
     @Test
     fun whenMatchExactlyEnabled_changingAutomaticUpdateFrequency_changesTaskFrequency() {
-        var deferredTasks = testDependencies.scheduler.deferredTasks
+        var deferredTasks = testDependencies.scheduler.getDeferredTasks()
 
         assertThat(deferredTasks, `is`(empty()))
 
@@ -38,14 +37,14 @@ class FormManagementSettingsTest {
             .clickUpdateForms()
             .clickOption(org.odk.collect.strings.R.string.match_exactly)
 
-        deferredTasks = testDependencies.scheduler.deferredTasks
+        deferredTasks = testDependencies.scheduler.getDeferredTasks()
 
         assertThat(deferredTasks.size, `is`(1))
 
         val matchExactlyTag = deferredTasks[0].tag
 
         page.clickAutomaticUpdateFrequency().clickOption(org.odk.collect.strings.R.string.every_one_hour)
-        deferredTasks = testDependencies.scheduler.deferredTasks
+        deferredTasks = testDependencies.scheduler.getDeferredTasks()
 
         assertThat(deferredTasks.size, `is`(1))
         assertThat(deferredTasks[0].tag, `is`(matchExactlyTag))
@@ -54,7 +53,7 @@ class FormManagementSettingsTest {
 
     @Test
     fun whenPreviouslyDownloadedOnlyEnabled_changingAutomaticUpdateFrequency_changesTaskFrequency() {
-        var deferredTasks = testDependencies.scheduler.deferredTasks
+        var deferredTasks = testDependencies.scheduler.getDeferredTasks()
 
         assertThat(deferredTasks, `is`(empty()))
 
@@ -65,14 +64,14 @@ class FormManagementSettingsTest {
             .clickUpdateForms()
             .clickOption(org.odk.collect.strings.R.string.previously_downloaded_only)
 
-        deferredTasks = testDependencies.scheduler.deferredTasks
+        deferredTasks = testDependencies.scheduler.getDeferredTasks()
 
         assertThat(deferredTasks.size, `is`(1))
 
         val previouslyDownloadedTag = deferredTasks[0].tag
         page.clickAutomaticUpdateFrequency().clickOption(org.odk.collect.strings.R.string.every_one_hour)
 
-        deferredTasks = testDependencies.scheduler.deferredTasks
+        deferredTasks = testDependencies.scheduler.getDeferredTasks()
 
         assertThat(deferredTasks.size, `is`(1))
         assertThat(deferredTasks[0].tag, `is`(previouslyDownloadedTag))