Sync.poll doesn't retry if the request fails for some reason :… (#1166)

* [1164] Sync.poll doesn't retry if the request fails for some reason : bug fixed * [1164] Sync.poll doesn't retry if the request fails for some reason : bug fixed * [1164] Sync.poll doesn't retry if the request fails for some reason : bug fixd * [1164] Retry mechanism added for sync API poll * [1164] periodic and one time request builder are desugarred to make function reusable * [1164] periodic and one time request builder are desugarred to make function reusable * [1164] spotless check applied * Review point updated * Review point updated * code cleanup
google · Apr 12, 2022 · e35492b · e35492b
1 parent 801bec1
commit e35492b
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 30 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
@@ -21,9 +21,7 @@ import androidx.work.Data
 import androidx.work.ExistingPeriodicWorkPolicy
 import androidx.work.ExistingWorkPolicy
 import androidx.work.OneTimeWorkRequest
-import androidx.work.OneTimeWorkRequestBuilder
 import androidx.work.PeriodicWorkRequest
-import androidx.work.PeriodicWorkRequestBuilder
 import androidx.work.WorkManager
 import com.google.android.fhir.FhirEngine
 import com.google.android.fhir.FhirEngineProvider
@@ -72,7 +70,7 @@ object Sync {
       .enqueueUniqueWork(
         SyncWorkType.DOWNLOAD.workerName,
         ExistingWorkPolicy.KEEP,
-        createOneTimeWorkRequest<W>(retryConfiguration)
+        createOneTimeWorkRequest(retryConfiguration, W::class.java)
       )
   }
   /**
@@ -82,41 +80,44 @@ object Sync {
    */
   inline fun <reified W : FhirSyncWorker> periodicSync(
     context: Context,
-    periodicSyncConfiguration: PeriodicSyncConfiguration,
+    periodicSyncConfiguration: PeriodicSyncConfiguration
   ) {
 
     WorkManager.getInstance(context)
       .enqueueUniquePeriodicWork(
         SyncWorkType.DOWNLOAD.workerName,
         ExistingPeriodicWorkPolicy.KEEP,
-        createPeriodicWorkRequest<W>(periodicSyncConfiguration)
+        createPeriodicWorkRequest(periodicSyncConfiguration, W::class.java)
       )
   }
 
   @PublishedApi
-  internal inline fun <reified W : FhirSyncWorker> createOneTimeWorkRequest(
-    retryConfiguration: RetryConfiguration?
+  internal inline fun <W : FhirSyncWorker> createOneTimeWorkRequest(
+    retryConfiguration: RetryConfiguration?,
+    clazz: Class<W>
   ): OneTimeWorkRequest {
-    val oneTimeWorkRequest = OneTimeWorkRequestBuilder<W>()
+    val oneTimeWorkRequestBuilder = OneTimeWorkRequest.Builder(clazz)
     retryConfiguration?.let {
-      oneTimeWorkRequest.setBackoffCriteria(
+      oneTimeWorkRequestBuilder.setBackoffCriteria(
         it.backoffCriteria.backoffPolicy,
         it.backoffCriteria.backoffDelay,
         it.backoffCriteria.timeUnit
       )
-      oneTimeWorkRequest.setInputData(
+      oneTimeWorkRequestBuilder.setInputData(
         Data.Builder().putInt(MAX_RETRIES_ALLOWED, it.maxRetries).build()
       )
     }
-    return oneTimeWorkRequest.build()
+    return oneTimeWorkRequestBuilder.build()
   }
 
   @PublishedApi
-  internal inline fun <reified W : FhirSyncWorker> createPeriodicWorkRequest(
-    periodicSyncConfiguration: PeriodicSyncConfiguration
+  internal inline fun <W : FhirSyncWorker> createPeriodicWorkRequest(
+    periodicSyncConfiguration: PeriodicSyncConfiguration,
+    clazz: Class<W>
   ): PeriodicWorkRequest {
     val periodicWorkRequestBuilder =
-      PeriodicWorkRequestBuilder<W>(
+      PeriodicWorkRequest.Builder(
+          clazz,
           periodicSyncConfiguration.repeat.interval,
           periodicSyncConfiguration.repeat.timeUnit
         )

diff --git a/engine/src/main/java/com/google/android/fhir/sync/SyncJobImpl.kt b/engine/src/main/java/com/google/android/fhir/sync/SyncJobImpl.kt
@@ -19,7 +19,6 @@ package com.google.android.fhir.sync
 import android.content.Context
 import androidx.lifecycle.asFlow
 import androidx.work.ExistingPeriodicWorkPolicy
-import androidx.work.PeriodicWorkRequest
 import androidx.work.WorkInfo
 import androidx.work.WorkManager
 import androidx.work.hasKeyWithValueOfType
@@ -55,15 +54,7 @@ class SyncJobImpl(private val context: Context) : SyncJob {
 
     Timber.d("Configuring polling for $workerUniqueName")
 
-    val periodicWorkRequest =
-      PeriodicWorkRequest.Builder(
-          clazz,
-          periodicSyncConfiguration.repeat.interval,
-          periodicSyncConfiguration.repeat.timeUnit
-        )
-        .setConstraints(periodicSyncConfiguration.syncConstraints)
-        .build()
-
+    val periodicWorkRequest = Sync.createPeriodicWorkRequest(periodicSyncConfiguration, clazz)
     val workManager = WorkManager.getInstance(context)
 
     val flow = stateFlow()

diff --git a/engine/src/test/java/com/google/android/fhir/sync/SyncTest.kt b/engine/src/test/java/com/google/android/fhir/sync/SyncTest.kt
@@ -43,8 +43,9 @@ class SyncTest {
   @Test
   fun createOneTimeWorkRequestWithRetryConfiguration_shouldHave3MaxTries() {
     val workRequest =
-      Sync.createOneTimeWorkRequest<PassingPeriodicSyncWorker>(
-        RetryConfiguration(BackoffCriteria(BackoffPolicy.LINEAR, 30, TimeUnit.SECONDS), 3)
+      Sync.createOneTimeWorkRequest(
+        RetryConfiguration(BackoffCriteria(BackoffPolicy.LINEAR, 30, TimeUnit.SECONDS), 3),
+        PassingPeriodicSyncWorker::class.java
       )
     assertThat(workRequest.workSpec.backoffPolicy).isEqualTo(BackoffPolicy.LINEAR)
     assertThat(workRequest.workSpec.backoffDelayDuration).isEqualTo(TimeUnit.SECONDS.toMillis(30))
@@ -53,7 +54,7 @@ class SyncTest {
 
   @Test
   fun createOneTimeWorkRequest_withoutRetryConfiguration_shouldHaveZeroMaxTries() {
-    val workRequest = Sync.createOneTimeWorkRequest<PassingPeriodicSyncWorker>(null)
+    val workRequest = Sync.createOneTimeWorkRequest(null, PassingPeriodicSyncWorker::class.java)
     assertThat(workRequest.workSpec.input.getInt(MAX_RETRIES_ALLOWED, 0)).isEqualTo(0)
     //    Not checking [workRequest.workSpec.backoffPolicy] and
     // [workRequest.workSpec.backoffDelayDuration] as they have default values.
@@ -62,12 +63,13 @@ class SyncTest {
   @Test
   fun createPeriodicWorkRequest_withRetryConfiguration_shouldHave3MaxTries() {
     val workRequest =
-      Sync.createPeriodicWorkRequest<PassingPeriodicSyncWorker>(
+      Sync.createPeriodicWorkRequest(
         PeriodicSyncConfiguration(
           repeat = RepeatInterval(20, TimeUnit.MINUTES),
           retryConfiguration =
             RetryConfiguration(BackoffCriteria(BackoffPolicy.LINEAR, 30, TimeUnit.SECONDS), 3)
         ),
+        PassingPeriodicSyncWorker::class.java
       )
     assertThat(workRequest.workSpec.intervalDuration).isEqualTo(TimeUnit.MINUTES.toMillis(20))
     assertThat(workRequest.workSpec.backoffPolicy).isEqualTo(BackoffPolicy.LINEAR)
@@ -78,11 +80,12 @@ class SyncTest {
   @Test
   fun createPeriodicWorkRequest_withoutRetryConfiguration_shouldHaveZeroMaxRetries() {
     val workRequest =
-      Sync.createPeriodicWorkRequest<PassingPeriodicSyncWorker>(
+      Sync.createPeriodicWorkRequest(
         PeriodicSyncConfiguration(
           repeat = RepeatInterval(20, TimeUnit.MINUTES),
           retryConfiguration = null
-        )
+        ),
+        PassingPeriodicSyncWorker::class.java
       )
     assertThat(workRequest.workSpec.intervalDuration).isEqualTo(TimeUnit.MINUTES.toMillis(20))
     assertThat(workRequest.workSpec.input.getInt(MAX_RETRIES_ALLOWED, 0)).isEqualTo(0)