From 47ac56ac58c48d488fd0bb3caa36e35b46fa57c6 Mon Sep 17 00:00:00 2001 From: Santosh Pingle Date: Tue, 16 Apr 2024 17:26:17 +0530 Subject: [PATCH 1/4] currentSyncJobStatus when sync state in workmanager is null. --- .../java/com/google/android/fhir/sync/Sync.kt | 43 ++++++++++++------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/engine/src/main/java/com/google/android/fhir/sync/Sync.kt b/engine/src/main/java/com/google/android/fhir/sync/Sync.kt index 3563bc687a..06de985e4b 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/Sync.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/Sync.kt @@ -26,7 +26,9 @@ import androidx.work.PeriodicWorkRequest import androidx.work.WorkInfo import androidx.work.WorkInfo.State.CANCELLED import androidx.work.WorkInfo.State.ENQUEUED +import androidx.work.WorkInfo.State.FAILED import androidx.work.WorkInfo.State.RUNNING +import androidx.work.WorkInfo.State.SUCCEEDED import androidx.work.WorkManager import androidx.work.hasKeyWithValueOfType import com.google.android.fhir.FhirEngineProvider @@ -269,28 +271,37 @@ object Sync { } /** - * Only call this API when `syncJobStatusFromWorkManager` is null. Create a [CurrentSyncJobStatus] - * from `syncJobStatusFromDataStore` if it is not null; otherwise, create it from - * [WorkInfo.State]. + * Creates a terminal states of [CurrentSyncJobStatus] from [syncJobStatusFromDataStore]; and + * intermediate states of [CurrentSyncJobStatus] from [WorkInfo.State]. + * + * Note : Only call this API when `syncJobStatusFromWorkManager` is null. */ private fun handleNullWorkManagerStatusForOneTimeSync( workInfoState: WorkInfo.State, syncJobStatusFromDataStore: SyncJobStatus?, ): CurrentSyncJobStatus = - syncJobStatusFromDataStore?.let { - when (it) { - is SyncJobStatus.Succeeded -> Succeeded(it.timestamp) - is SyncJobStatus.Failed -> Failed(it.timestamp) - else -> error("Inconsistent terminal syncJobStatus : $syncJobStatusFromDataStore") - } + when (workInfoState) { + ENQUEUED -> Enqueued + RUNNING -> Running(SyncJobStatus.Started()) + SUCCEEDED -> + syncJobStatusFromDataStore?.let { + when (it) { + is SyncJobStatus.Succeeded -> Succeeded(it.timestamp) + else -> error("Inconsistent terminal syncJobStatus : $syncJobStatusFromDataStore") + } + } + ?: error("Inconsistent terminal syncJobStatus.") + FAILED -> + syncJobStatusFromDataStore?.let { + when (it) { + is SyncJobStatus.Failed -> Failed(it.timestamp) + else -> error("Inconsistent terminal syncJobStatus : $syncJobStatusFromDataStore") + } + } + ?: error("Inconsistent terminal syncJobStatus.") + CANCELLED -> Cancelled + else -> error("Inconsistent WorkInfo.State: $workInfoState.") } - ?: when (workInfoState) { - RUNNING -> Running(SyncJobStatus.Started()) - ENQUEUED -> Enqueued - CANCELLED -> Cancelled - // syncJobStatusFromDataStore should not be null for SUCCEEDED, FAILED. - else -> error("Inconsistent WorkInfo.State: $workInfoState.") - } /** * Only call this API when syncJobStatusFromWorkManager is null. Create a [CurrentSyncJobStatus] From 4fb9562146c7c7847fe74b5432bf8591f33e8a39 Mon Sep 17 00:00:00 2001 From: Santosh Pingle Date: Thu, 18 Apr 2024 13:35:44 +0530 Subject: [PATCH 2/4] unit tests. --- .../android/fhir/sync/SyncInstrumentedTest.kt | 87 ++++++++++++++++++- 1 file changed, 86 insertions(+), 1 deletion(-) diff --git a/engine/src/androidTest/java/com/google/android/fhir/sync/SyncInstrumentedTest.kt b/engine/src/androidTest/java/com/google/android/fhir/sync/SyncInstrumentedTest.kt index 2aaa61c386..b148e1a3ea 100644 --- a/engine/src/androidTest/java/com/google/android/fhir/sync/SyncInstrumentedTest.kt +++ b/engine/src/androidTest/java/com/google/android/fhir/sync/SyncInstrumentedTest.kt @@ -19,6 +19,7 @@ package com.google.android.fhir.sync import android.content.Context import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.test.platform.app.InstrumentationRegistry +import androidx.work.BackoffPolicy import androidx.work.Constraints import androidx.work.WorkInfo import androidx.work.WorkManager @@ -104,12 +105,49 @@ class SyncInstrumentedTest { assertThat(states.last()).isInstanceOf(CurrentSyncJobStatus.Succeeded::class.java) } + @Test + fun oneTime_worker_nextExecutionAfterSucceeded() { + WorkManagerTestInitHelper.initializeTestWorkManager(context) + val states = mutableListOf() + val nextExecutionStates = mutableListOf() + runBlocking { + Sync.oneTimeSync(context = context) + .transformWhile { + states.add(it) + emit(it is CurrentSyncJobStatus.Succeeded) + it !is CurrentSyncJobStatus.Succeeded + } + .shareIn(this, SharingStarted.Eagerly, 5) + + Sync.oneTimeSync(context = context) + .transformWhile { + nextExecutionStates.add(it) + emit(it is CurrentSyncJobStatus.Succeeded) + it !is CurrentSyncJobStatus.Succeeded + } + .shareIn(this, SharingStarted.Eagerly, 5) + } + assertThat(states.first()).isInstanceOf(CurrentSyncJobStatus.Running::class.java) + assertThat(states.last()).isInstanceOf(CurrentSyncJobStatus.Succeeded::class.java) + assertThat(nextExecutionStates.first()).isInstanceOf(CurrentSyncJobStatus.Running::class.java) + } + @Test fun oneTime_worker_failedSyncState() { WorkManagerTestInitHelper.initializeTestWorkManager(context) val states = mutableListOf() runBlocking { - Sync.oneTimeSync(context = context) + Sync.oneTimeSync( + context = context, + RetryConfiguration( + BackoffCriteria( + BackoffPolicy.LINEAR, + 30, + TimeUnit.SECONDS, + ), + 0, + ), + ) .transformWhile { states.add(it) emit(it is CurrentSyncJobStatus.Failed) @@ -121,6 +159,53 @@ class SyncInstrumentedTest { assertThat(states.last()).isInstanceOf(CurrentSyncJobStatus.Failed::class.java) } + @Test + fun oneTime_worker_nextExecutionAfterFailed() { + WorkManagerTestInitHelper.initializeTestWorkManager(context) + val states = mutableListOf() + val nextExecutionStates = mutableListOf() + runBlocking { + Sync.oneTimeSync( + context = context, + RetryConfiguration( + BackoffCriteria( + BackoffPolicy.LINEAR, + 30, + TimeUnit.SECONDS, + ), + 0, + ), + ) + .transformWhile { + states.add(it) + emit(it is CurrentSyncJobStatus.Failed) + it !is CurrentSyncJobStatus.Failed + } + .shareIn(this, SharingStarted.Eagerly, 5) + + Sync.oneTimeSync( + context = context, + RetryConfiguration( + BackoffCriteria( + BackoffPolicy.LINEAR, + 30, + TimeUnit.SECONDS, + ), + 0, + ), + ) + .transformWhile { + nextExecutionStates.add(it) + emit(it is CurrentSyncJobStatus.Failed) + it !is CurrentSyncJobStatus.Failed + } + .shareIn(this, SharingStarted.Eagerly, 5) + } + assertThat(states.first()).isInstanceOf(CurrentSyncJobStatus.Running::class.java) + assertThat(states.last()).isInstanceOf(CurrentSyncJobStatus.Failed::class.java) + assertThat(nextExecutionStates.first()).isInstanceOf(CurrentSyncJobStatus.Running::class.java) + } + @Test fun periodic_worker_periodicSyncState() { WorkManagerTestInitHelper.initializeTestWorkManager(context) From 13ffe96773af2d6f571dda41745a0f1fec61d1b9 Mon Sep 17 00:00:00 2001 From: Santosh Pingle Date: Thu, 9 May 2024 10:51:21 +0530 Subject: [PATCH 3/4] update kdoc --- engine/src/main/java/com/google/android/fhir/sync/Sync.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/src/main/java/com/google/android/fhir/sync/Sync.kt b/engine/src/main/java/com/google/android/fhir/sync/Sync.kt index 06de985e4b..bdd29020c6 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/Sync.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/Sync.kt @@ -271,7 +271,7 @@ object Sync { } /** - * Creates a terminal states of [CurrentSyncJobStatus] from [syncJobStatusFromDataStore]; and + * Creates terminal states of [CurrentSyncJobStatus] from [syncJobStatusFromDataStore]; and * intermediate states of [CurrentSyncJobStatus] from [WorkInfo.State]. * * Note : Only call this API when `syncJobStatusFromWorkManager` is null. From edd1d2220fa7b5e1b646337bf9ee80e942c982fe Mon Sep 17 00:00:00 2001 From: Santosh Pingle Date: Fri, 5 Jul 2024 13:50:58 +0530 Subject: [PATCH 4/4] address review comments. --- .../android/fhir/sync/SyncInstrumentedTest.kt | 4 ++-- .../java/com/google/android/fhir/sync/Sync.kt | 23 +++++++++---------- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/engine/src/androidTest/java/com/google/android/fhir/sync/SyncInstrumentedTest.kt b/engine/src/androidTest/java/com/google/android/fhir/sync/SyncInstrumentedTest.kt index b148e1a3ea..c449cd3df0 100644 --- a/engine/src/androidTest/java/com/google/android/fhir/sync/SyncInstrumentedTest.kt +++ b/engine/src/androidTest/java/com/google/android/fhir/sync/SyncInstrumentedTest.kt @@ -106,7 +106,7 @@ class SyncInstrumentedTest { } @Test - fun oneTime_worker_nextExecutionAfterSucceeded() { + fun oneTimeSync_currentSyncJobStatusSucceeded_nextCurrentSyncJobStatusShouldBeRunning() { WorkManagerTestInitHelper.initializeTestWorkManager(context) val states = mutableListOf() val nextExecutionStates = mutableListOf() @@ -160,7 +160,7 @@ class SyncInstrumentedTest { } @Test - fun oneTime_worker_nextExecutionAfterFailed() { + fun oneTimeSync_currentSyncJobStatusFailed_nextCurrentSyncJobStatusShouldBeRunning() { WorkManagerTestInitHelper.initializeTestWorkManager(context) val states = mutableListOf() val nextExecutionStates = mutableListOf() diff --git a/engine/src/main/java/com/google/android/fhir/sync/Sync.kt b/engine/src/main/java/com/google/android/fhir/sync/Sync.kt index bdd29020c6..47cf9f0e1e 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/Sync.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/Sync.kt @@ -108,7 +108,16 @@ object Sync { return combineSyncStateForPeriodicSync(context, uniqueWorkName, flow) } - /** Gets the worker info for the [FhirSyncWorker] */ + /** + * Retrieves the work information for a specific unique work name as a flow of pairs containing + * the work state and the corresponding progress data if available. + * + * @param context The application context. + * @param workName The unique name of the work to retrieve information for. + * @return A flow emitting pairs of [WorkInfo.State] and [SyncJobStatus]. The flow will emit only + * when the progress data contains a non-empty key-value map and includes a key of type [String] + * with the name "StateType". + */ @PublishedApi internal fun getWorkerInfo(context: Context, workName: String) = WorkManager.getInstance(context) @@ -304,20 +313,10 @@ object Sync { } /** - * Only call this API when syncJobStatusFromWorkManager is null. Create a [CurrentSyncJobStatus] + * Only call this API when syncJobStatus From WorkManager is null. Create a [CurrentSyncJobStatus] * from [WorkInfo.State]. (Note: syncJobStatusFromDataStore is updated as lastSynJobStatus, which * is the terminalSyncJobStatus.) */ - private fun handleNullWorkManagerStatusForPeriodicSync( - workInfoState: WorkInfo.State, - ): CurrentSyncJobStatus = - when (workInfoState) { - RUNNING -> Running(SyncJobStatus.Started()) - ENQUEUED -> Enqueued - CANCELLED -> Cancelled - else -> error("Inconsistent WorkInfo.State in periodic sync : $workInfoState.") - } - private fun handleNullWorkManagerStatusForPeriodicSync( workInfoState: WorkInfo.State, syncJobStatusFromDataStore: SyncJobStatus?,