Skip to content

Commit

Permalink
Sync worker management: move logic out of companion object (#1056)
Browse files Browse the repository at this point in the history
* Sync worker management: move logic from companion object to new class

* Fix tests

* Move re-sync inputs from [OneTimeSyncWorker] to [BaseSyncWorker] as they're processed there

* Remove useless Companion
  • Loading branch information
rfc2822 authored Oct 7, 2024
1 parent 196bfb3 commit fc7f42c
Show file tree
Hide file tree
Showing 18 changed files with 436 additions and 287 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,11 @@ import androidx.work.Configuration
import androidx.work.WorkInfo
import androidx.work.WorkManager
import androidx.work.testing.WorkManagerTestInitHelper
import at.bitfire.davdroid.repository.DavCollectionRepository
import at.bitfire.davdroid.repository.DavServiceRepository
import at.bitfire.davdroid.settings.AccountSettings
import at.bitfire.davdroid.sync.account.TestAccountAuthenticator
import at.bitfire.davdroid.sync.worker.OneTimeSyncWorker
import at.bitfire.davdroid.sync.worker.SyncWorkerManager
import dagger.hilt.android.qualifiers.ApplicationContext
import dagger.hilt.android.testing.HiltAndroidRule
import dagger.hilt.android.testing.HiltAndroidTest
Expand All @@ -39,21 +42,33 @@ import org.junit.Rule
import org.junit.Test
import org.junit.rules.Timeout
import java.util.concurrent.Executors
import java.util.logging.Logger
import javax.inject.Inject
import javax.inject.Provider
import kotlin.coroutines.cancellation.CancellationException

@HiltAndroidTest
class SyncAdapterServicesTest {

lateinit var account: Account

@Inject
lateinit var accountSettingsFactory: AccountSettings.Factory

@Inject
lateinit var collectionRepository: DavCollectionRepository

@Inject
@ApplicationContext
lateinit var context: Context

@Inject
lateinit var syncAdapterProvider: Provider<SyncAdapterService.SyncAdapter>
lateinit var logger: Logger

@Inject
lateinit var serviceRepository: DavServiceRepository

@Inject
lateinit var syncConditionsFactory: SyncConditions.Factory

@Inject
lateinit var workerFactory: HiltWorkerFactory
Expand Down Expand Up @@ -87,14 +102,29 @@ class SyncAdapterServicesTest {
}


private fun syncAdapter(
syncWorkerManager: SyncWorkerManager
): SyncAdapterService.SyncAdapter =
SyncAdapterService.SyncAdapter(
accountSettingsFactory = accountSettingsFactory,
collectionRepository = collectionRepository,
serviceRepository = serviceRepository,
context = context,
logger = logger,
syncConditionsFactory = syncConditionsFactory,
syncWorkerManager = syncWorkerManager
)


@Test
fun testSyncAdapter_onPerformSync_cancellation() {
val syncAdapter = syncAdapterProvider.get()
val syncWorkerManager = mockk<SyncWorkerManager>()
val syncAdapter = syncAdapter(syncWorkerManager = syncWorkerManager)
val workManager = WorkManager.getInstance(context)

mockkObject(OneTimeSyncWorker, workManager) {
mockkObject(workManager) {
// don't actually create a worker
every { OneTimeSyncWorker.enqueue(any(), any(), any()) } returns "TheSyncWorker"
every { syncWorkerManager.enqueueOneTime(any(), any()) } returns "TheSyncWorker"

// assume worker takes a long time
every { workManager.getWorkInfosForUniqueWorkFlow("TheSyncWorker") } just Awaits
Expand All @@ -115,12 +145,13 @@ class SyncAdapterServicesTest {

@Test
fun testSyncAdapter_onPerformSync_returnsAfterTimeout() {
val syncAdapter = syncAdapterProvider.get()
val syncWorkerManager = mockk<SyncWorkerManager>()
val syncAdapter = syncAdapter(syncWorkerManager = syncWorkerManager)
val workManager = WorkManager.getInstance(context)

mockkObject(OneTimeSyncWorker, workManager) {
mockkObject(workManager) {
// don't actually create a worker
every { OneTimeSyncWorker.enqueue(any(), any(), any()) } returns "TheSyncWorker"
every { syncWorkerManager.enqueueOneTime(any(), any()) } returns "TheSyncWorker"

// assume worker takes a long time
every { workManager.getWorkInfosForUniqueWorkFlow("TheSyncWorker") } just Awaits
Expand All @@ -136,12 +167,13 @@ class SyncAdapterServicesTest {

@Test
fun testSyncAdapter_onPerformSync_runsInTime() {
val syncAdapter = syncAdapterProvider.get()
val syncWorkerManager = mockk<SyncWorkerManager>()
val syncAdapter = syncAdapter(syncWorkerManager = syncWorkerManager)
val workManager = WorkManager.getInstance(context)

mockkObject(OneTimeSyncWorker, workManager) {
mockkObject(workManager) {
// don't actually create a worker
every { OneTimeSyncWorker.enqueue(any(), any(), any()) } returns "TheSyncWorker"
every { syncWorkerManager.enqueueOneTime(any(), any()) } returns "TheSyncWorker"

// assume worker immediately returns with success
val success = mockk<WorkInfo>()
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import androidx.work.WorkerParameters
import androidx.work.testing.TestListenableWorkerBuilder
import androidx.work.testing.WorkManagerTestInitHelper
import androidx.work.workDataOf
import at.bitfire.davdroid.TestUtils.workScheduledOrRunning
import at.bitfire.davdroid.sync.account.TestAccountAuthenticator
import at.bitfire.davdroid.test.R
import dagger.hilt.android.qualifiers.ApplicationContext
Expand All @@ -27,7 +26,6 @@ import io.mockk.mockkObject
import io.mockk.verify
import kotlinx.coroutines.runBlocking
import org.junit.After
import org.junit.Assert.assertFalse
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Rule
Expand Down Expand Up @@ -69,21 +67,6 @@ class PeriodicSyncWorkerTest {
}


@Test
fun enable_enqueuesPeriodicWorker() {
PeriodicSyncWorker.enable(context, account, CalendarContract.AUTHORITY, 60, false)
val workerName = PeriodicSyncWorker.workerName(account, CalendarContract.AUTHORITY)
assertTrue(workScheduledOrRunning(context, workerName))
}

@Test
fun disable_removesPeriodicWorker() {
PeriodicSyncWorker.enable(context, account, CalendarContract.AUTHORITY, 60, false)
PeriodicSyncWorker.disable(context, account, CalendarContract.AUTHORITY)
val workerName = PeriodicSyncWorker.workerName(account, CalendarContract.AUTHORITY)
assertFalse(workScheduledOrRunning(context, workerName))
}

@Test
fun doWork_cancelsItselfOnInvalidAccount() {
val invalidAccount = Account("invalid", testContext.getString(R.string.account_type_test))
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
/*
* Copyright © All Contributors. See LICENSE and AUTHORS in the root directory for details.
*/

package at.bitfire.davdroid.sync.worker

import android.accounts.Account
import android.content.Context
import android.provider.CalendarContract
import android.util.Log
import androidx.hilt.work.HiltWorkerFactory
import androidx.work.Configuration
import androidx.work.testing.WorkManagerTestInitHelper
import at.bitfire.davdroid.TestUtils
import at.bitfire.davdroid.TestUtils.workScheduledOrRunning
import at.bitfire.davdroid.sync.account.TestAccountAuthenticator
import dagger.hilt.android.qualifiers.ApplicationContext
import dagger.hilt.android.testing.HiltAndroidRule
import dagger.hilt.android.testing.HiltAndroidTest
import org.junit.After
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Rule
import org.junit.Test
import javax.inject.Inject

@HiltAndroidTest
class SyncWorkerManagerTest {

@get:Rule
val hiltRule = HiltAndroidRule(this)

@Inject
@ApplicationContext
lateinit var context: Context

@Inject
lateinit var syncWorkerManager: SyncWorkerManager

@Inject
lateinit var workerFactory: HiltWorkerFactory

lateinit var account: Account

@Before
fun setUp() {
hiltRule.inject()

// Initialize WorkManager for instrumentation tests.
val config = Configuration.Builder()
.setMinimumLoggingLevel(Log.DEBUG)
.setWorkerFactory(workerFactory)
.build()
WorkManagerTestInitHelper.initializeTestWorkManager(context, config)

account = TestAccountAuthenticator.create()
}

@After
fun tearDown() {
TestAccountAuthenticator.remove(account)
}


// one-time sync workers

@Test
fun testEnqueueOneTime() {
val workerName = OneTimeSyncWorker.workerName(account, CalendarContract.AUTHORITY)
assertFalse(TestUtils.workScheduledOrRunningOrSuccessful(context, workerName))

val returnedName = syncWorkerManager.enqueueOneTime(account, CalendarContract.AUTHORITY)
assertEquals(workerName, returnedName)
assertTrue(TestUtils.workScheduledOrRunningOrSuccessful(context, workerName))
}


// periodic sync workers

@Test
fun enablePeriodic() {
syncWorkerManager.enablePeriodic(account, CalendarContract.AUTHORITY, 60, false).result.get()

val workerName = PeriodicSyncWorker.workerName(account, CalendarContract.AUTHORITY)
assertTrue(workScheduledOrRunning(context, workerName))
}

@Test
fun disablePeriodic() {
syncWorkerManager.enablePeriodic(account, CalendarContract.AUTHORITY, 60, false).result.get()
syncWorkerManager.disablePeriodic(account, CalendarContract.AUTHORITY).result.get()

val workerName = PeriodicSyncWorker.workerName(account, CalendarContract.AUTHORITY)
assertFalse(workScheduledOrRunning(context, workerName))
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import at.bitfire.davdroid.repository.AccountRepository
import at.bitfire.davdroid.repository.DavCollectionRepository
import at.bitfire.davdroid.repository.DavServiceRepository
import at.bitfire.davdroid.repository.PreferenceRepository
import at.bitfire.davdroid.sync.worker.OneTimeSyncWorker
import at.bitfire.davdroid.sync.worker.SyncWorkerManager
import dagger.hilt.android.AndroidEntryPoint
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
Expand Down Expand Up @@ -40,6 +40,9 @@ class UnifiedPushReceiver: MessagingReceiver() {
@Inject
lateinit var parsePushMessage: PushMessageParser

@Inject
lateinit var syncWorkerManager: SyncWorkerManager


override fun onNewEndpoint(context: Context, endpoint: String, instance: String) {
// remember new endpoint
Expand Down Expand Up @@ -71,14 +74,14 @@ class UnifiedPushReceiver: MessagingReceiver() {
collectionRepository.getSyncableByTopic(topic)?.let { collection ->
serviceRepository.get(collection.serviceId)?.let { service ->
val account = accountRepository.fromName(service.accountName)
OneTimeSyncWorker.enqueueAllAuthorities(context, account)
syncWorkerManager.enqueueOneTimeAllAuthorities(account)
}
}

} else {
logger.warning("Got push message without topic, syncing all accounts")
for (account in accountRepository.getAll())
OneTimeSyncWorker.enqueueAllAuthorities(context, account)
syncWorkerManager.enqueueOneTimeAllAuthorities(account)

}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import at.bitfire.davdroid.sync.TasksAppManager
import at.bitfire.davdroid.sync.account.AccountUtils
import at.bitfire.davdroid.sync.account.AccountsCleanupWorker
import at.bitfire.davdroid.sync.worker.BaseSyncWorker
import at.bitfire.davdroid.sync.worker.PeriodicSyncWorker
import at.bitfire.davdroid.sync.worker.SyncWorkerManager
import at.bitfire.vcard4android.GroupMethod
import dagger.Lazy
import dagger.hilt.android.qualifiers.ApplicationContext
Expand All @@ -52,6 +52,7 @@ class AccountRepository @Inject constructor(
private val logger: Logger,
private val settingsManager: SettingsManager,
private val serviceRepository: DavServiceRepository,
private val syncWorkerManager: SyncWorkerManager,
private val tasksAppManager: Lazy<TasksAppManager>
) {

Expand Down Expand Up @@ -237,7 +238,7 @@ class AccountRepository @Inject constructor(

// disable periodic syncs for old account
syncIntervals.forEach { (authority, _) ->
PeriodicSyncWorker.disable(context, oldAccount, authority)
syncWorkerManager.disablePeriodic(oldAccount, authority)
}

// update account name references in database
Expand Down
Loading

0 comments on commit fc7f42c

Please sign in to comment.