From 26b4008bbc2628519db78792e29244274caaa879 Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Thu, 23 Apr 2026 15:57:47 +0200 Subject: [PATCH 1/3] Allow in-progress form upload to be interrupted on cancel --- .../InstancesDataService.kt | 11 ++-- .../send/InstanceSubmitter.kt | 57 +++++++++++++++---- .../send/InstanceUploaderViewModel.kt | 2 - .../odk/collect/shared/locks/ChangeLock.kt | 12 ++++ 4 files changed, 64 insertions(+), 18 deletions(-) diff --git a/collect_app/src/main/java/org/odk/collect/android/instancemanagement/InstancesDataService.kt b/collect_app/src/main/java/org/odk/collect/android/instancemanagement/InstancesDataService.kt index 318db463542..40ba1f36a5e 100644 --- a/collect_app/src/main/java/org/odk/collect/android/instancemanagement/InstancesDataService.kt +++ b/collect_app/src/main/java/org/odk/collect/android/instancemanagement/InstancesDataService.kt @@ -213,7 +213,7 @@ class InstancesDataService( } } - fun sendInstances( + suspend fun sendInstances( projectId: String, instances: List, referrer: String, @@ -221,14 +221,13 @@ class InstancesDataService( cancelAfterAuthException: Boolean, externalDeleteAfterUpload: Boolean?, defaultSuccessMessage: String, - ensureActive: () -> Unit, onProgress: (current: Int, total: Int) -> Unit = { _, _ -> } ): List { val projectDependencyModule = projectDependencyModuleFactory.create(projectId) - return projectDependencyModule.instancesLock.withLock { acquiredLock: Boolean -> + return projectDependencyModule.instancesLock.withLockSuspend { acquiredLock: Boolean -> if (acquiredLock) { - instanceSubmitter.submitInstances( + val result = instanceSubmitter.submitInstancesSuspend( projectId, instances, referrer, @@ -236,9 +235,11 @@ class InstancesDataService( cancelAfterAuthException, externalDeleteAfterUpload, defaultSuccessMessage, - ensureActive, onProgress ) + update(projectId) + + result } else { emptyList() } diff --git a/collect_app/src/main/java/org/odk/collect/android/instancemanagement/send/InstanceSubmitter.kt b/collect_app/src/main/java/org/odk/collect/android/instancemanagement/send/InstanceSubmitter.kt index 99b2fa2f7a1..516d4041452 100644 --- a/collect_app/src/main/java/org/odk/collect/android/instancemanagement/send/InstanceSubmitter.kt +++ b/collect_app/src/main/java/org/odk/collect/android/instancemanagement/send/InstanceSubmitter.kt @@ -1,5 +1,8 @@ package org.odk.collect.android.instancemanagement.send +import kotlinx.coroutines.currentCoroutineContext +import kotlinx.coroutines.ensureActive +import kotlinx.coroutines.runInterruptible import org.odk.collect.android.instancemanagement.InstanceDeleter import org.odk.collect.android.projects.ProjectDependencyModule import org.odk.collect.android.utilities.InstanceAutoDeleteChecker @@ -20,15 +23,46 @@ class InstanceSubmitter( ) { fun submitInstances( + projectId: String, + toUpload: List + ): List { + val projectDependencyModule = projectDependencyFactory.create(projectId) + val formsRepository = projectDependencyModule.formsRepository + val instancesRepository = projectDependencyModule.instancesRepository + val generalSettings = projectDependencyModule.generalSettings + + val uploadResults = mutableListOf() + val deviceId = propertyManager.getSingularProperty(PROPMGR_DEVICE_ID) + + val sortedInstances = toUpload.sortedBy { it.finalizationDate } + for (instance in sortedInstances) { + try { + val resultMessage = instanceUploader.uploadOneSubmission(projectId, instance, deviceId, null, "") + uploadResults.add(InstanceUploadResult.Success(instance, resultMessage)) + + deleteInstance(instance, formsRepository, instancesRepository, generalSettings, null) + } catch (e: FormUploadException) { + Timber.d(e) + uploadResults.add(InstanceUploadResult.Error(instance, e)) + + if (e is FormUploadAuthRequestedException) { + break + } + } + } + + return uploadResults + } + + suspend fun submitInstancesSuspend( projectId: String, toUpload: List, - referrer: String = "", - overrideURL: String? = null, - cancelAfterAuthException: Boolean = false, - externalDeleteAfterUpload: Boolean? = null, - defaultSuccessMessage: String? = null, - ensureActive: () -> Unit = {}, - onProgress: (current: Int, total: Int) -> Unit = { _, _ -> } + referrer: String, + overrideURL: String?, + cancelAfterAuthException: Boolean, + externalDeleteAfterUpload: Boolean?, + defaultSuccessMessage: String?, + onProgress: (current: Int, total: Int) -> Unit ): List { val projectDependencyModule = projectDependencyFactory.create(projectId) val formsRepository = projectDependencyModule.formsRepository @@ -40,13 +74,14 @@ class InstanceSubmitter( val sortedInstances = toUpload.sortedBy { it.finalizationDate } for ((index, instance) in sortedInstances.withIndex()) { - ensureActive() - onProgress( index + 1, sortedInstances.size) + currentCoroutineContext().ensureActive() + onProgress(index + 1, sortedInstances.size) try { - val resultMessage = instanceUploader.uploadOneSubmission(projectId, instance, deviceId, overrideURL, referrer) + val resultMessage = runInterruptible { + instanceUploader.uploadOneSubmission(projectId, instance, deviceId, overrideURL, referrer) + } uploadResults.add(InstanceUploadResult.Success(instance, resultMessage ?: defaultSuccessMessage)) - deleteInstance(instance, formsRepository, instancesRepository, generalSettings, externalDeleteAfterUpload) } catch (e: FormUploadException) { Timber.d(e) diff --git a/collect_app/src/main/java/org/odk/collect/android/instancemanagement/send/InstanceUploaderViewModel.kt b/collect_app/src/main/java/org/odk/collect/android/instancemanagement/send/InstanceUploaderViewModel.kt index 5c4ec76dcd1..07e2fcc5fb4 100644 --- a/collect_app/src/main/java/org/odk/collect/android/instancemanagement/send/InstanceUploaderViewModel.kt +++ b/collect_app/src/main/java/org/odk/collect/android/instancemanagement/send/InstanceUploaderViewModel.kt @@ -6,7 +6,6 @@ import androidx.lifecycle.MutableLiveData import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import kotlinx.coroutines.Job -import kotlinx.coroutines.ensureActive import kotlinx.coroutines.launch import org.odk.collect.android.instancemanagement.InstancesDataService import org.odk.collect.android.utilities.WebCredentialsUtils @@ -55,7 +54,6 @@ class InstanceUploadViewModel( true, externalDeleteAfterUpload, defaultSuccessMessage, - { coroutineContext.ensureActive() } ) { current, total -> _state.postValue(UploadState.Progress(current, total)) } diff --git a/shared/src/main/java/org/odk/collect/shared/locks/ChangeLock.kt b/shared/src/main/java/org/odk/collect/shared/locks/ChangeLock.kt index 15c6a0c0462..9af1896a151 100644 --- a/shared/src/main/java/org/odk/collect/shared/locks/ChangeLock.kt +++ b/shared/src/main/java/org/odk/collect/shared/locks/ChangeLock.kt @@ -20,6 +20,18 @@ interface ChangeLock { } } + suspend fun withLockSuspend(function: suspend (Boolean) -> T): T { + val acquired = tryLock(DEFAULT_TOKEN) + + return try { + function(acquired) + } finally { + if (acquired) { + unlock(DEFAULT_TOKEN) + } + } + } + fun tryLock(token: Any): Boolean fun unlock(token: Any) From 8cf66ef3da7962c3fda30fd9c93c9e9bd901fb85 Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Thu, 23 Apr 2026 15:57:56 +0200 Subject: [PATCH 2/3] Improve tests --- .../send/InstanceUploadViewModelTest.kt | 100 +++++++++++++++++- 1 file changed, 98 insertions(+), 2 deletions(-) diff --git a/collect_app/src/test/java/org/odk/collect/android/instancemanagement/send/InstanceUploadViewModelTest.kt b/collect_app/src/test/java/org/odk/collect/android/instancemanagement/send/InstanceUploadViewModelTest.kt index 57ae5efde6e..e7ec7f937ae 100644 --- a/collect_app/src/test/java/org/odk/collect/android/instancemanagement/send/InstanceUploadViewModelTest.kt +++ b/collect_app/src/test/java/org/odk/collect/android/instancemanagement/send/InstanceUploadViewModelTest.kt @@ -26,7 +26,99 @@ class InstanceUploadViewModelTest { private lateinit var viewModel: InstanceUploadViewModel @Test - fun `submitted instance is deleted even when upload is cancelled`() { + fun `instance is not submitted when upload process is canceled during execution`() { + val form = FormFixtures.form("1") + val formsRepository = InMemFormsRepository().apply { + save(form) + } + + val instance = Instance.Builder() + .dbId(1) + .formId(form.formId) + .formVersion(form.version) + .status(Instance.STATUS_COMPLETE) + .finalizationDate(1) + .build() + + val instancesRepository = InMemInstancesRepository().apply { + save(instance) + } + + val projectsDependencyModuleFactory = CachingProjectDependencyModuleFactory { projectId -> + ProjectDependencyModule( + projectId, + { InMemSettings() }, + { formsRepository }, + { instancesRepository }, + mock(), + { ChangeLocks(BooleanChangeLock(), BooleanChangeLock()) }, + mock(), + mock(), + mock(), + mock(), + mock() + ) + } + + val submittedInstances = mutableListOf() + + val instanceUploader = object : InstanceUploader { + override fun uploadOneSubmission( + projectId: String, + instance: Instance, + deviceId: String?, + overrideURL: String?, + referrer: String + ): String { + viewModel.cancel() + Thread.sleep(1000) + + submittedInstances.add(instance.dbId) + instancesRepository.save( + Instance.Builder(instance) + .status(Instance.STATUS_SUBMITTED) + .build() + ) + return "Success" + } + } + + val instancesSubmitter = InstanceSubmitter( + instanceUploader, + projectsDependencyModuleFactory, + mock() + ) + val instancesDataService = InstancesDataService( + AppState(), + mock(), + projectsDependencyModuleFactory, + mock(), + instancesSubmitter + ) {} + + val dispatcherProvider = TestDispatcherProvider() + viewModel = InstanceUploadViewModel( + dispatcherProvider, + mock(), + instancesRepository, + instancesDataService, + "projectId", + "", + null, + null, + null, + null, + "Success", + "Waiting" + ) + viewModel.upload(listOf(instance.dbId)) + dispatcherProvider.runBackground() + + assertThat(submittedInstances.isEmpty(), equalTo(true)) + } + + @Test + fun `submitted instances are deleted when upload process is canceled during execution`() { val form = FormFixtures.form("1") val formsRepository = InMemFormsRepository().apply { save(form) @@ -75,6 +167,7 @@ class InstanceUploadViewModelTest { val submittedInstances = mutableListOf() + var progress = 0; val instanceUploader = object : InstanceUploader { override fun uploadOneSubmission( projectId: String, @@ -83,13 +176,16 @@ class InstanceUploadViewModelTest { overrideURL: String?, referrer: String ): String { + if (++progress == 2) { + viewModel.cancel() + } + submittedInstances.add(instance.dbId) instancesRepository.save( Instance.Builder(instance) .status(Instance.STATUS_SUBMITTED) .build() ) - viewModel.cancel() return "Success" } } From 6ef33a0eaaeeacc9b518ba4ad94fd817d22c360e Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Thu, 23 Apr 2026 16:08:31 +0200 Subject: [PATCH 3/3] Rename fun --- .../collect/android/instancemanagement/InstancesDataService.kt | 2 +- .../android/instancemanagement/send/InstanceSubmitter.kt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/collect_app/src/main/java/org/odk/collect/android/instancemanagement/InstancesDataService.kt b/collect_app/src/main/java/org/odk/collect/android/instancemanagement/InstancesDataService.kt index 40ba1f36a5e..247fbd196c1 100644 --- a/collect_app/src/main/java/org/odk/collect/android/instancemanagement/InstancesDataService.kt +++ b/collect_app/src/main/java/org/odk/collect/android/instancemanagement/InstancesDataService.kt @@ -227,7 +227,7 @@ class InstancesDataService( return projectDependencyModule.instancesLock.withLockSuspend { acquiredLock: Boolean -> if (acquiredLock) { - val result = instanceSubmitter.submitInstancesSuspend( + val result = instanceSubmitter.submitInstances( projectId, instances, referrer, diff --git a/collect_app/src/main/java/org/odk/collect/android/instancemanagement/send/InstanceSubmitter.kt b/collect_app/src/main/java/org/odk/collect/android/instancemanagement/send/InstanceSubmitter.kt index 516d4041452..128ca04f03e 100644 --- a/collect_app/src/main/java/org/odk/collect/android/instancemanagement/send/InstanceSubmitter.kt +++ b/collect_app/src/main/java/org/odk/collect/android/instancemanagement/send/InstanceSubmitter.kt @@ -54,7 +54,7 @@ class InstanceSubmitter( return uploadResults } - suspend fun submitInstancesSuspend( + suspend fun submitInstances( projectId: String, toUpload: List, referrer: String,