Merge pull request #6235 from grzesiek2010/COLLECT-6221

Fixed displaying progress indicator while loading layers
getodk · Jul 1, 2024 · aea1106 · aea1106
2 parents 1729fa4 + 83335af
commit aea1106
Show file tree

Hide file tree

Showing 6 changed files with 58 additions and 22 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
@@ -3,6 +3,7 @@ package org.odk.collect.androidshared.async
 import org.odk.collect.androidshared.livedata.MutableNonNullLiveData
 import org.odk.collect.androidshared.livedata.NonNullLiveData
 import org.odk.collect.async.Scheduler
+import java.util.concurrent.atomic.AtomicInteger
 import java.util.function.Consumer
 import java.util.function.Supplier
 
@@ -11,10 +12,15 @@ class TrackableWorker(private val scheduler: Scheduler) {
     private val _isWorking = MutableNonNullLiveData(false)
     val isWorking: NonNullLiveData<Boolean> = _isWorking
 
+    private var activeBackgroundJobsCounter = AtomicInteger(0)
+
     fun <T> immediate(background: Supplier<T>, foreground: Consumer<T>) {
+        activeBackgroundJobsCounter.incrementAndGet()
         _isWorking.value = true
         scheduler.immediate(background) { result ->
-            _isWorking.value = false
+            if (activeBackgroundJobsCounter.decrementAndGet() == 0) {
+                _isWorking.value = false
+            }
             foreground.accept(result)
         }
     }

diff --git a/androidshared/src/test/java/org/odk/collect/androidshared/async/TrackableWorkerTest.kt b/androidshared/src/test/java/org/odk/collect/androidshared/async/TrackableWorkerTest.kt
@@ -0,0 +1,28 @@
+package org.odk.collect.androidshared.async
+
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import org.hamcrest.CoreMatchers.equalTo
+import org.hamcrest.MatcherAssert.assertThat
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.odk.collect.testshared.FakeScheduler
+
+@RunWith(AndroidJUnit4::class)
+class TrackableWorkerTest {
+    private val scheduler = FakeScheduler()
+    private val trackableWorker = TrackableWorker(scheduler)
+
+    @Test
+    fun `TrackableWorker counts work in progress`() {
+        trackableWorker.immediate {}
+        trackableWorker.immediate {}
+
+        scheduler.runFirstBackground()
+        scheduler.runFirstForeground()
+        assertThat(trackableWorker.isWorking.value, equalTo(true))
+
+        scheduler.runFirstBackground()
+        scheduler.runFirstForeground()
+        assertThat(trackableWorker.isWorking.value, equalTo(false))
+    }
+}
diff --git a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImporter.kt b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImporter.kt
@@ -56,7 +56,7 @@ class OfflineMapLayersImporter(
         super.onViewCreated(view, savedInstanceState)
         val binding = OfflineMapLayersImporterBinding.bind(view)
 
-        viewModel.isLoading.observe(this) { isLoading ->
+        viewModel.trackableWorker.isWorking.observe(this) { isLoading ->
             if (isLoading) {
                 binding.addLayerButton.isEnabled = false
                 binding.layers.visibility = View.GONE

diff --git a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPicker.kt b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPicker.kt
@@ -122,7 +122,7 @@ class OfflineMapLayersPicker(
     override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
         val binding = OfflineMapLayersPickerBinding.bind(view)
 
-        sharedViewModel.isLoading.observe(this) { isLoading ->
+        sharedViewModel.trackableWorker.isWorking.observe(this) { isLoading ->
             if (isLoading) {
                 binding.progressIndicator.visibility = View.VISIBLE
                 binding.layers.visibility = View.GONE

diff --git a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersViewModel.kt b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersViewModel.kt
@@ -6,6 +6,7 @@ import androidx.lifecycle.LiveData
 import androidx.lifecycle.MutableLiveData
 import androidx.lifecycle.ViewModel
 import org.odk.collect.analytics.Analytics
+import org.odk.collect.androidshared.async.TrackableWorker
 import org.odk.collect.androidshared.data.Consumable
 import org.odk.collect.androidshared.system.copyToFile
 import org.odk.collect.androidshared.system.getFileExtension
@@ -19,11 +20,10 @@ import java.io.File
 
 class OfflineMapLayersViewModel(
     private val referenceLayerRepository: ReferenceLayerRepository,
-    private val scheduler: Scheduler,
+    scheduler: Scheduler,
     private val settingsProvider: SettingsProvider
 ) : ViewModel() {
-    private val _isLoading = MutableLiveData<Boolean>()
-    val isLoading: LiveData<Boolean> = _isLoading
+    val trackableWorker = TrackableWorker(scheduler)
 
     private val _existingLayers = MutableLiveData<List<ReferenceLayer>>()
     val existingLayers: LiveData<List<ReferenceLayer>> = _existingLayers
@@ -38,20 +38,16 @@ class OfflineMapLayersViewModel(
     }
 
     private fun loadExistingLayers() {
-        _isLoading.value = true
-        scheduler.immediate(
+        trackableWorker.immediate(
             background = {
                 val layers = referenceLayerRepository.getAll().sortedBy { it.name }
-                _isLoading.postValue(false)
                 _existingLayers.postValue(layers)
-            },
-            foreground = { }
+            }
         )
     }
 
     fun loadLayersToImport(uris: List<Uri>, context: Context) {
-        _isLoading.value = true
-        scheduler.immediate(
+        trackableWorker.immediate(
             background = {
                 tempLayersDir = TempFiles.createTempDir().also {
                     it.deleteOnExit()
@@ -67,7 +63,6 @@ class OfflineMapLayersViewModel(
                         }
                     }
                 }
-                _isLoading.postValue(false)
                 _layersToImport.postValue(
                     Consumable(
                         LayersToImport(
@@ -77,14 +72,12 @@ class OfflineMapLayersViewModel(
                         )
                     )
                 )
-            },
-            foreground = { }
+            }
         )
     }
 
     fun importNewLayers(shared: Boolean) {
-        _isLoading.value = true
-        scheduler.immediate(
+        trackableWorker.immediate(
             background = {
                 val layers = tempLayersDir.listFiles()
                 logImport(layers)
@@ -93,7 +86,6 @@ class OfflineMapLayersViewModel(
                     referenceLayerRepository.addLayer(it, shared)
                 }
                 tempLayersDir.delete()
-                _isLoading.postValue(false)
             },
             foreground = {
                 loadExistingLayers()
@@ -106,15 +98,13 @@ class OfflineMapLayersViewModel(
     }
 
     fun deleteLayer(layerId: String) {
-        _isLoading.value = true
-        scheduler.immediate {
+        trackableWorker.immediate {
             if (settingsProvider.getUnprotectedSettings().getString(ProjectKeys.KEY_REFERENCE_LAYER) == layerId) {
                 settingsProvider.getUnprotectedSettings().save(ProjectKeys.KEY_REFERENCE_LAYER, null)
             }
 
             referenceLayerRepository.delete(layerId)
             _existingLayers.postValue(_existingLayers.value?.filter { it.id != layerId })
-            _isLoading.postValue(false)
         }
     }
 

diff --git a/test-shared/src/main/java/org/odk/collect/testshared/FakeScheduler.kt b/test-shared/src/main/java/org/odk/collect/testshared/FakeScheduler.kt
@@ -77,6 +77,12 @@ class FakeScheduler : Scheduler {
         return flow.flowOn(backgroundDispatcher)
     }
 
+    fun runFirstForeground() {
+        if (foregroundTasks.isNotEmpty()) {
+            foregroundTasks.removeFirst().run()
+        }
+    }
+
     fun runForeground() {
         while (foregroundTasks.isNotEmpty()) {
             foregroundTasks.remove().run()
@@ -103,6 +109,12 @@ class FakeScheduler : Scheduler {
         }
     }
 
+    fun runFirstBackground() {
+        if (backgroundTasks.isNotEmpty()) {
+            backgroundTasks.removeFirst().run()
+        }
+    }
+
     fun runBackground() {
         while (backgroundTasks.isNotEmpty()) {
             backgroundTasks.remove().run()