SyncAdapterServices: Use a coroutine scope to cancel waiting on framework request

app/src/androidTest/kotlin/at/bitfire/davdroid/sync/SyncAdapterServicesTest.kt

@@ -0,0 +1,159 @@
+/*
+ * Copyright © All Contributors. See LICENSE and AUTHORS in the root directory for details.
+ */
+
+package at.bitfire.davdroid.sync
+
+import android.accounts.Account
+import android.content.Context
+import android.content.SyncResult
+import android.os.Bundle
+import android.provider.CalendarContract
+import android.util.Log
+import androidx.hilt.work.HiltWorkerFactory
+import androidx.work.Configuration
+import androidx.work.WorkInfo
+import androidx.work.WorkManager
+import androidx.work.testing.WorkManagerTestInitHelper
+import at.bitfire.davdroid.sync.account.TestAccountAuthenticator
+import at.bitfire.davdroid.sync.worker.OneTimeSyncWorker
+import dagger.hilt.android.qualifiers.ApplicationContext
+import dagger.hilt.android.testing.HiltAndroidRule
+import dagger.hilt.android.testing.HiltAndroidTest
+import io.mockk.Awaits
+import io.mockk.coEvery
+import io.mockk.every
+import io.mockk.just
+import io.mockk.mockk
+import io.mockk.mockkObject
+import io.mockk.mockkStatic
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.delay
+import kotlinx.coroutines.flow.flow
+import kotlinx.coroutines.launch
+import kotlinx.coroutines.runBlocking
+import kotlinx.coroutines.withTimeout
+import org.junit.After
+import org.junit.Before
+import org.junit.Rule
+import org.junit.Test
+import org.junit.rules.Timeout
+import java.util.concurrent.Executors
+import javax.inject.Inject
+import javax.inject.Provider
+import kotlin.coroutines.cancellation.CancellationException
+
+@HiltAndroidTest
+class SyncAdapterServicesTest {
+
+    lateinit var account: Account
+
+    @Inject
+    @ApplicationContext
+    lateinit var context: Context
+
+    @Inject
+    lateinit var syncAdapterProvider: Provider<SyncAdapterService.SyncAdapter>
+
+    @Inject
+    lateinit var workerFactory: HiltWorkerFactory
+
+    @get:Rule
+    val hiltRule = HiltAndroidRule(this)
+
+    // test methods should run quickly and not wait 60 seconds for a sync timeout or something like that
+    @get:Rule
+    val timeoutRule: Timeout = Timeout.seconds(5)
+
+
+    @Before
+    fun setUp() {
+        hiltRule.inject()
+
+        account = TestAccountAuthenticator.create()
+
+        // Initialize WorkManager for instrumentation tests.
+        val config = Configuration.Builder()
+            .setMinimumLoggingLevel(Log.DEBUG)
+            .setWorkerFactory(workerFactory)
+            .setTaskExecutor(Executors.newSingleThreadExecutor())
+            .build()
+        WorkManagerTestInitHelper.initializeTestWorkManager(context, config)
+    }
+
+    @After
+    fun tearDown() {
+        TestAccountAuthenticator.remove(account)
+    }
+
+
+    @Test
+    fun testSyncAdapter_onPerformSync_cancellation() {
+        val syncAdapter = syncAdapterProvider.get()
+        val workManager = WorkManager.getInstance(context)
+
+        mockkObject(OneTimeSyncWorker, workManager) {
+            // don't actually create a worker
+            every { OneTimeSyncWorker.enqueue(any(), any(), any()) } returns "TheSyncWorker"
+
+            // assume worker takes a long time
+            every { workManager.getWorkInfosForUniqueWorkFlow("TheSyncWorker") } just Awaits
+
+            runBlocking {
+                val sync = launch {
+                    syncAdapter.onPerformSync(account, Bundle(), CalendarContract.AUTHORITY, mockk(), SyncResult())
+                }
+
+                // simulate incoming cancellation from sync framework
+                syncAdapter.onSyncCanceled()
+
+                // wait for sync to finish (should happen immediately)
+                sync.join()
+            }
+        }
+    }
+
+    @Test
+    fun testSyncAdapter_onPerformSync_returnsAfterTimeout() {
+        val syncAdapter = syncAdapterProvider.get()
+        val workManager = WorkManager.getInstance(context)
+
+        mockkObject(OneTimeSyncWorker, workManager) {
+            // don't actually create a worker
+            every { OneTimeSyncWorker.enqueue(any(), any(), any()) } returns "TheSyncWorker"
+
+            // assume worker takes a long time
+            every { workManager.getWorkInfosForUniqueWorkFlow("TheSyncWorker") } just Awaits
+
+            mockkStatic("kotlinx.coroutines.TimeoutKt") {   // mock global extension function
+                // immediate timeout (instead of really waiting)
+                coEvery { withTimeout(any<Long>(), any<suspend CoroutineScope.() -> Unit>()) } throws CancellationException("Simulated timeout")
+
+                syncAdapter.onPerformSync(account, Bundle(), CalendarContract.AUTHORITY, mockk(), SyncResult())
+            }
+        }
+    }
+
+    @Test
+    fun testSyncAdapter_onPerformSync_runsInTime() {
+        val syncAdapter = syncAdapterProvider.get()
+        val workManager = WorkManager.getInstance(context)
+
+        mockkObject(OneTimeSyncWorker, workManager) {
+            // don't actually create a worker
+            every { OneTimeSyncWorker.enqueue(any(), any(), any()) } returns "TheSyncWorker"
+
+            // assume worker immediately returns with success
+            val success = mockk<WorkInfo>()
+            every { success.state } returns WorkInfo.State.SUCCEEDED
+            every { workManager.getWorkInfosForUniqueWorkFlow("TheSyncWorker") } returns flow {
+                emit(listOf(success))
+                delay(60000)    // keep the flow active
+            }
+
+            // should just run
+            syncAdapter.onPerformSync(account, Bundle(), CalendarContract.AUTHORITY, mockk(), SyncResult())
+        }
+    }
+
+}

app/src/main/kotlin/at/bitfire/davdroid/sync/SyncAdapterServices.kt

@@ -23,8 +23,10 @@ import at.bitfire.davdroid.sync.worker.OneTimeSyncWorker
 import dagger.hilt.android.AndroidEntryPoint
 import dagger.hilt.android.qualifiers.ApplicationContext
 import kotlinx.coroutines.CancellationException
-import kotlinx.coroutines.CompletableDeferred
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.cancel
+import kotlinx.coroutines.launch
 import kotlinx.coroutines.runBlocking
 import kotlinx.coroutines.withTimeout
 import java.util.logging.Level
@@ -62,18 +64,13 @@ abstract class SyncAdapterService: Service() {
     ) {
 
         /**
-         * Completable [Boolean], which will be set to
-         *
-         * - `true` when the related sync worker has finished
-         * - `false` when the sync framework has requested cancellation.
-         *
-         * In any case, the sync framework shouldn't be blocked anymore as soon as a
-         * value is available.
+         * Scope used to wait until the synchronization is finished. Will be cancelled when the sync framework
+         * requests cancellation.
          */
-        private val finished = CompletableDeferred<Boolean>()
+        private val waitScope = CoroutineScope(Dispatchers.Default)
 
         override fun onPerformSync(account: Account, extras: Bundle, authority: String, provider: ContentProviderClient, syncResult: SyncResult) {
-            // We seem to have to pass this old SyncFramework extra for an Android 7 workaround
+            // We have to pass this old SyncFramework extra for an Android 7 workaround
             val upload = extras.containsKey(ContentResolver.SYNC_EXTRAS_UPLOAD)
             logger.info("Sync request via sync framework for $account $authority (upload=$upload)")
 
@@ -104,27 +101,31 @@ abstract class SyncAdapterService: Service() {
             logger.fine("Starting OneTimeSyncWorker for $workerAccount $workerAuthority and waiting for it")
             val workerName = OneTimeSyncWorker.enqueue(context, workerAccount, workerAuthority, upload = upload)
 
-            // Because we are not allowed to observe worker state on a background thread, we can not
-            // use it to block the sync adapter. Instead we check periodically whether the sync has
-            // finished, putting the thread to sleep in between checks.
+            /* Because we are not allowed to observe worker state on a background thread, we can not
+            use it to block the sync adapter. Instead we use a Flow to get notified when the sync
+            has finished. */
             val workManager = WorkManager.getInstance(context)
 
             try {
+                val waitJob = waitScope.launch {
+                    // wait for finished worker state
+                    workManager.getWorkInfosForUniqueWorkFlow(workerName).collect { info ->
+                        if (info.any { it.state.isFinished })
+                            cancel("$workerName has finished")
+                    }
+                }
+
                 runBlocking {
                     withTimeout(10 * 60 * 1000) {   // block max. 10 minutes
-                        // wait for finished worker state
-                        workManager.getWorkInfosForUniqueWorkFlow(workerName).collect { info ->
-                            if (info.any { it.state.isFinished })
-                                cancel(CancellationException("$workerName has finished"))
-                        }
+                        waitJob.join()              // wait until worker has finished
                     }
                 }
             } catch (e: CancellationException) {
                 // waiting for work was cancelled, either by timeout or because the worker has finished
-                logger.log(Level.FINE, "Not waiting for OneTimeSyncWorker anymore (this is not an error)", e)
+                logger.fine("Not waiting for OneTimeSyncWorker anymore.")
             }
 
-            logger.fine("Returning to sync framework")
+            logger.info("Returning to sync framework.")
         }
 
         override fun onSecurityException(account: Account, extras: Bundle, authority: String, syncResult: SyncResult) {
@@ -135,7 +136,7 @@ abstract class SyncAdapterService: Service() {
             logger.info("Sync adapter requested cancellation – won't cancel sync, but also won't block sync framework anymore")
 
             // unblock sync framework
-            finished.complete(false)
+            waitScope.cancel()
         }
 
         override fun onSyncCanceled(thread: Thread) = onSyncCanceled()