SyncAdapterServices: Use a coroutine scope to cancel waiting on framework request (#977)

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

* Added tests
This commit is contained in:
Ricki Hirner 2024-08-14 14:00:54 +02:00 committed by GitHub
parent 5d4c9c8d94
commit 70f6f2603e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 181 additions and 21 deletions

View file

@ -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())
}
}
}

View file

@ -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()