From f43ad6961bf5932f2b4b0ea23ca244847feb84ad Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Thu, 27 Jun 2024 08:55:41 +0200 Subject: [PATCH 1/4] Updated TrackableWorker to count jobs --- .../odk/collect/androidshared/async/TrackableWorker.kt | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) 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 index b6ad2e521d0..6bf67213303 100644 --- 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 = _isWorking + private var counter = AtomicInteger(0) + fun immediate(background: Supplier, foreground: Consumer) { + counter.incrementAndGet() _isWorking.value = true scheduler.immediate(background) { result -> - _isWorking.value = false + if (counter.decrementAndGet() == 0) { + _isWorking.value = false + } foreground.accept(result) } } From 94b2a7ca87561f4301e5a58302ec65b0f95b01ea Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Thu, 27 Jun 2024 08:58:27 +0200 Subject: [PATCH 2/4] Added tests --- .../async/TrackableWorkerTest.kt | 28 +++++++++++++++++++ .../odk/collect/testshared/FakeScheduler.kt | 12 ++++++++ 2 files changed, 40 insertions(+) create mode 100644 androidshared/src/test/java/org/odk/collect/androidshared/async/TrackableWorkerTest.kt 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 new file mode 100644 index 00000000000..4a66f7fc614 --- /dev/null +++ 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/test-shared/src/main/java/org/odk/collect/testshared/FakeScheduler.kt b/test-shared/src/main/java/org/odk/collect/testshared/FakeScheduler.kt index b4a3f153a03..3646e5f2223 100644 --- 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() From fd6a5e3c54c8a36f4f03b6c86a144cc0f56d566b Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Thu, 27 Jun 2024 08:59:09 +0200 Subject: [PATCH 3/4] Naming improvements --- .../org/odk/collect/androidshared/async/TrackableWorker.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 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 index 6bf67213303..2360b5fd63c 100644 --- a/androidshared/src/main/java/org/odk/collect/androidshared/async/TrackableWorker.kt +++ b/androidshared/src/main/java/org/odk/collect/androidshared/async/TrackableWorker.kt @@ -12,13 +12,13 @@ class TrackableWorker(private val scheduler: Scheduler) { private val _isWorking = MutableNonNullLiveData(false) val isWorking: NonNullLiveData = _isWorking - private var counter = AtomicInteger(0) + private var activeBackgroundJobsCounter = AtomicInteger(0) fun immediate(background: Supplier, foreground: Consumer) { - counter.incrementAndGet() + activeBackgroundJobsCounter.incrementAndGet() _isWorking.value = true scheduler.immediate(background) { result -> - if (counter.decrementAndGet() == 0) { + if (activeBackgroundJobsCounter.decrementAndGet() == 0) { _isWorking.value = false } foreground.accept(result) From 83335af55581e50a2e14e19bcce73d7523c3ebd9 Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Thu, 27 Jun 2024 09:06:42 +0200 Subject: [PATCH 4/4] Use TrackableWorker in OfflineMapLayersViewModel --- .../maps/layers/OfflineMapLayersImporter.kt | 2 +- .../maps/layers/OfflineMapLayersPicker.kt | 2 +- .../maps/layers/OfflineMapLayersViewModel.kt | 28 ++++++------------- 3 files changed, 11 insertions(+), 21 deletions(-) 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 index 20e7d92d0ae..ea8b68a0d6f 100644 --- 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 index 3ae5df46827..6c04efa9963 100644 --- a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPicker.kt +++ b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPicker.kt @@ -124,7 +124,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 index 1842dbe325d..e0ed74994a6 100644 --- 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() - val isLoading: LiveData = _isLoading + val trackableWorker = TrackableWorker(scheduler) private val _existingLayers = MutableLiveData>() val existingLayers: LiveData> = _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, 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) } }