From 6c098e5d132026587cc3515e64e35ce0cd395bf4 Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Sat, 15 Jun 2024 00:30:58 +0200 Subject: [PATCH 1/3] Removed redundant super calls --- .../org/odk/collect/maps/layers/OfflineMapLayersImporter.kt | 2 -- .../java/org/odk/collect/maps/layers/OfflineMapLayersPicker.kt | 2 -- .../odk/collect/material/MaterialFullScreenDialogFragment.java | 2 -- 3 files changed, 6 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 a3ca996b4ba..c9ffcfccdc6 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 @@ -47,8 +47,6 @@ class OfflineMapLayersImporter( } override fun onViewCreated(view: View, savedInstanceState: Bundle?) { - super.onViewCreated(view, savedInstanceState) - viewModel.isLoading.observe(this) { isLoading -> if (isLoading) { binding.addLayerButton.isEnabled = false 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 483c98883b8..9784b200061 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 @@ -101,8 +101,6 @@ class OfflineMapLayersPicker( } override fun onViewCreated(view: View, savedInstanceState: Bundle?) { - super.onViewCreated(view, savedInstanceState) - sharedViewModel.isLoading.observe(this) { isLoading -> if (isLoading) { binding.progressIndicator.visibility = View.VISIBLE diff --git a/material/src/main/java/org/odk/collect/material/MaterialFullScreenDialogFragment.java b/material/src/main/java/org/odk/collect/material/MaterialFullScreenDialogFragment.java index 15290c98604..7a2a2f8404a 100644 --- a/material/src/main/java/org/odk/collect/material/MaterialFullScreenDialogFragment.java +++ b/material/src/main/java/org/odk/collect/material/MaterialFullScreenDialogFragment.java @@ -51,8 +51,6 @@ public void handleOnBackPressed() { @Override public void onViewCreated(View view, Bundle savedInstanceState) { - super.onViewCreated(view, savedInstanceState); - if (getToolbar() != null) { getToolbar().setNavigationOnClickListener(v -> { onCloseClicked(); From be76300bafda53e4b26ff032be40758a05e89959 Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Sat, 15 Jun 2024 00:38:58 +0200 Subject: [PATCH 2/3] Use TrackableWorker --- .../maps/layers/OfflineMapLayersImporter.kt | 2 +- .../maps/layers/OfflineMapLayersPicker.kt | 2 +- .../maps/layers/OfflineMapLayersViewModel.kt | 60 +++++++------------ 3 files changed, 25 insertions(+), 39 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 c9ffcfccdc6..c76205b43e4 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 @@ -47,7 +47,7 @@ class OfflineMapLayersImporter( } override fun onViewCreated(view: View, savedInstanceState: Bundle?) { - 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 9784b200061..dec57862327 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 @@ -101,7 +101,7 @@ class OfflineMapLayersPicker( } override fun onViewCreated(view: View, savedInstanceState: Bundle?) { - 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 893104dc060..1fb52d495b2 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 @@ -5,6 +5,7 @@ import android.net.Uri import androidx.lifecycle.LiveData import androidx.lifecycle.MutableLiveData import androidx.lifecycle.ViewModel +import org.odk.collect.androidshared.async.TrackableWorker import org.odk.collect.androidshared.system.copyToFile import org.odk.collect.androidshared.system.getFileExtension import org.odk.collect.androidshared.system.getFileName @@ -16,11 +17,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 @@ -35,51 +35,39 @@ class OfflineMapLayersViewModel( } private fun loadExistingLayers() { - _isLoading.value = true - scheduler.immediate( - background = { - val layers = referenceLayerRepository.getAll() - _isLoading.postValue(false) - _existingLayers.postValue(layers) - }, - foreground = { } - ) + trackableWorker.immediate { + val layers = referenceLayerRepository.getAll() + _existingLayers.postValue(layers) + } } fun loadLayersToImport(uris: List, context: Context) { - _isLoading.value = true - scheduler.immediate( - background = { - tempLayersDir = TempFiles.createTempDir().also { - it.deleteOnExit() - } - val layers = mutableListOf() - uris.forEach { uri -> - if (uri.getFileExtension(context) == MbtilesFile.FILE_EXTENSION) { - uri.getFileName(context)?.let { fileName -> - val layerFile = File(tempLayersDir, fileName).also { file -> - uri.copyToFile(context, file) - } - layers.add(ReferenceLayer(layerFile.absolutePath, layerFile, MbtilesFile.readName(layerFile) ?: layerFile.name)) + trackableWorker.immediate { + tempLayersDir = TempFiles.createTempDir().also { + it.deleteOnExit() + } + val layers = mutableListOf() + uris.forEach { uri -> + if (uri.getFileExtension(context) == MbtilesFile.FILE_EXTENSION) { + uri.getFileName(context)?.let { fileName -> + val layerFile = File(tempLayersDir, fileName).also { file -> + uri.copyToFile(context, file) } + layers.add(ReferenceLayer(layerFile.absolutePath, layerFile, MbtilesFile.readName(layerFile) ?: layerFile.name)) } } - _isLoading.postValue(false) - _layersToImport.postValue(layers) - }, - foreground = { } - ) + } + _layersToImport.postValue(layers) + } } fun importNewLayers(shared: Boolean) { - _isLoading.value = true - scheduler.immediate( + trackableWorker.immediate( background = { tempLayersDir.listFiles()?.forEach { referenceLayerRepository.addLayer(it, shared) } tempLayersDir.delete() - _isLoading.postValue(false) }, foreground = { loadExistingLayers() @@ -92,11 +80,9 @@ class OfflineMapLayersViewModel( } fun deleteLayer(layerId: String) { - _isLoading.value = true - scheduler.immediate { + trackableWorker.immediate { referenceLayerRepository.delete(layerId) _existingLayers.postValue(_existingLayers.value?.filter { it.id != layerId }) - _isLoading.postValue(false) } } } From 2cb44919e140ed433d93a0bd975ca303d8f33553 Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Sat, 15 Jun 2024 00:46:51 +0200 Subject: [PATCH 3/3] Pulled out TestRegistry --- .../maps/layers/OfflineMapLayersPickerTest.kt | 41 +++++++++++-------- 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersPickerTest.kt b/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersPickerTest.kt index 79d107708af..b3418e5ca3f 100644 --- a/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersPickerTest.kt +++ b/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersPickerTest.kt @@ -56,19 +56,7 @@ class OfflineMapLayersPickerTest { private val settingsProvider = InMemSettingsProvider() private val externalWebPageHelper = mock() - private val uris = mutableListOf() - private val testRegistry = object : ActivityResultRegistry() { - override fun onLaunch( - requestCode: Int, - contract: ActivityResultContract, - input: I, - options: ActivityOptionsCompat? - ) { - assertThat(contract, instanceOf(ActivityResultContracts.GetMultipleContents()::class.java)) - assertThat(input, equalTo("*/*")) - dispatchResult(requestCode, uris) - } - } + private val testRegistry = TestRegistry() @get:Rule val fragmentScenarioLauncherRule = FragmentScenarioLauncherRule( @@ -311,7 +299,7 @@ class OfflineMapLayersPickerTest { fun `clicking the 'add layer' and selecting layers displays the confirmation dialog`() { val scenario = launchFragment() - uris.add(Uri.parse("blah")) + testRegistry.addUris(Uri.parse("blah")) EspressoHelpers.clickOnText(string.add_layer) scenario.onFragment { @@ -345,8 +333,7 @@ class OfflineMapLayersPickerTest { scheduler.flush() - uris.add(file1.toUri()) - uris.add(file2.toUri()) + testRegistry.addUris(file1.toUri(), file2.toUri()) EspressoHelpers.clickOnText(string.add_layer) scheduler.flush() @@ -370,8 +357,7 @@ class OfflineMapLayersPickerTest { scheduler.flush() - uris.add(file1.toUri()) - uris.add(file2.toUri()) + testRegistry.addUris(file1.toUri(), file2.toUri()) EspressoHelpers.clickOnText(string.add_layer) scheduler.flush() @@ -579,4 +565,23 @@ class OfflineMapLayersPickerTest { private fun launchFragment(): FragmentScenario { return fragmentScenarioLauncherRule.launchInContainer(OfflineMapLayersPicker::class.java) } + + private class TestRegistry : ActivityResultRegistry() { + val uris = mutableListOf() + + override fun onLaunch( + requestCode: Int, + contract: ActivityResultContract, + input: I, + options: ActivityOptionsCompat? + ) { + assertThat(contract, instanceOf(ActivityResultContracts.GetMultipleContents()::class.java)) + assertThat(input, equalTo("*/*")) + dispatchResult(requestCode, uris) + } + + fun addUris(vararg uris: Uri) { + this.uris.addAll(uris) + } + } }