Skip to content

Commit

Permalink
Sync workers: use data type enum instead of specific authority (#1177)
Browse files Browse the repository at this point in the history
* Introduce SyncDataType for Workers

* Fix tests, remove SyncWorkerManager.enqueueOneTime(authority)

* TasksAppManager.currentProviderFlow(): return Flow instead of StateFlow

* Simplify TasksAppManager and TasksAppWatcher

* [WIP] AutomaticSyncManager

* [WIP] AutomaticSyncManager

* AccountSettings: optimize imports

* SyncWorkManager: remove deprecated methods

* AutomaticSyncManager: disable unused authorities in sync framework

* Add migration draft

* AccountSettings: minor changes

* Migration, BaseSyncWorker: use sync data type

* Tests, set default sync interval when tasks app is installed, notify on missing tasks app permission during sync

* Remove deprecated AccountSettings methods

* AccountSettings: actually increase version number

* Use automaticSyncManager.updateAutomaticSync where applicable; better handle manual sync interval

* KDoc

* Remove deprecated SyncWorkerManager.syncAuthorities; fix cancelAllWork

* AccountSettings: minor changes

* TasksAppManager: show notification on missing permissions; always update automatic syncs

* AutomaticSyncWorker: only provide updateAutomaticSync() as public method

* AccountSettings: simplify setSyncInterval

* AutomaticSyncManager: disable automatic task sync when no tasks provider is available
  • Loading branch information
rfc2822 authored Dec 26, 2024
1 parent 98578fe commit f503ce5
Show file tree
Hide file tree
Showing 31 changed files with 589 additions and 431 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package at.bitfire.davdroid.settings

import android.accounts.AccountManager
import android.content.Context
import at.bitfire.davdroid.TestUtils
import at.bitfire.davdroid.sync.account.TestAccountAuthenticator
import dagger.hilt.android.qualifiers.ApplicationContext
import dagger.hilt.android.testing.HiltAndroidRule
Expand All @@ -32,6 +33,7 @@ class AccountSettingsTest {
@Before
fun setUp() {
hiltRule.inject()
TestUtils.setUpWorkManager(context)
}


Expand All @@ -50,7 +52,7 @@ class AccountSettingsTest {
accountSettingsFactory.create(account, abortOnMissingMigration = true)

val accountManager = AccountManager.get(context)
val version = accountManager.getUserData(account, AccountSettings.KEY_SETTINGS_VERSION).toIntOrNull()
val version = accountManager.getUserData(account, AccountSettings.KEY_SETTINGS_VERSION).toInt()
assertEquals(AccountSettings.CURRENT_VERSION, version)
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
/*
* Copyright © All Contributors. See LICENSE and AUTHORS in the root directory for details.
*/

package at.bitfire.davdroid.settings.migration

import android.accounts.Account
import android.content.Context
import android.util.Log
import androidx.hilt.work.HiltWorkerFactory
import androidx.work.Configuration
import androidx.work.WorkManager
import androidx.work.testing.WorkManagerTestInitHelper
import at.bitfire.davdroid.R
import at.bitfire.davdroid.sync.AutomaticSyncManager
import dagger.hilt.android.qualifiers.ApplicationContext
import dagger.hilt.android.testing.HiltAndroidRule
import dagger.hilt.android.testing.HiltAndroidTest
import io.mockk.MockKAnnotations
import io.mockk.impl.annotations.InjectMockKs
import io.mockk.impl.annotations.MockK
import io.mockk.impl.annotations.SpyK
import io.mockk.mockkObject
import io.mockk.unmockkAll
import io.mockk.verify
import org.junit.After
import org.junit.Before
import org.junit.Rule
import org.junit.Test
import javax.inject.Inject

@HiltAndroidTest
class AccountSettingsMigration19Test {

@Inject @ApplicationContext
@SpyK
lateinit var context: Context

@MockK(relaxed = true)
lateinit var automaticSyncManager: AutomaticSyncManager

@InjectMockKs
lateinit var migration: AccountSettingsMigration19

@Inject
lateinit var workerFactory: HiltWorkerFactory

@get:Rule
val hiltRule = HiltAndroidRule(this)


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

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

MockKAnnotations.init(this)
}

@After
fun tearDown() {
unmockkAll()
}


@Test
fun testMigrate_CancelsOldWorkersAndUpdatesAutomaticSync() {
val workManager = WorkManager.getInstance(context)
mockkObject(workManager)

val account = Account("Some", "Test")
migration.migrate(account)

val addressBookAuthority = context.getString(R.string.address_books_authority)
verify {
workManager.cancelUniqueWork("periodic-sync $addressBookAuthority Test/Some")
workManager.cancelUniqueWork("periodic-sync com.android.calendar Test/Some")
workManager.cancelUniqueWork("periodic-sync at.techbee.jtx.provider Test/Some")
workManager.cancelUniqueWork("periodic-sync org.dmfs.tasks Test/Some")
workManager.cancelUniqueWork("periodic-sync org.tasks.opentasks Test/Some")

automaticSyncManager.updateAutomaticSync(account)
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ import android.accounts.AccountManager
import android.content.Context
import android.os.Bundle
import androidx.hilt.work.HiltWorkerFactory
import androidx.work.WorkerFactory
import androidx.work.WorkerParameters
import androidx.work.testing.TestListenableWorkerBuilder
import at.bitfire.davdroid.R
import at.bitfire.davdroid.TestUtils
Expand Down Expand Up @@ -141,7 +139,7 @@ class AccountsCleanupWorkerTest {

@Test
fun testCleanUpAddressBooks_keepsAddressBookWithAccount() {
TestAccountAuthenticator.provide() { account ->
TestAccountAuthenticator.provide { account ->
// Create address book account _with_ corresponding account and verify
val userData = Bundle(2).apply {
putString(LocalAddressBook.USER_DATA_ACCOUNT_NAME, account.name)
Expand Down Expand Up @@ -172,9 +170,4 @@ class AccountsCleanupWorkerTest {
return db.serviceDao().get(serviceId)!!
}

private fun workerFactory() = object : WorkerFactory() {
override fun createWorker(appContext: Context, workerClassName: String, workerParameters: WorkerParameters) =
accountsCleanupWorkerFactory.create(appContext, workerParameters)
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ package at.bitfire.davdroid.sync.worker

import android.accounts.Account
import android.content.Context
import android.provider.CalendarContract
import androidx.test.platform.app.InstrumentationRegistry
import androidx.work.ListenableWorker
import androidx.work.WorkManager
Expand All @@ -15,6 +14,7 @@ import androidx.work.WorkerParameters
import androidx.work.testing.TestListenableWorkerBuilder
import androidx.work.workDataOf
import at.bitfire.davdroid.TestUtils
import at.bitfire.davdroid.sync.SyncDataType
import at.bitfire.davdroid.sync.account.TestAccountAuthenticator
import at.bitfire.davdroid.test.R
import dagger.hilt.android.qualifiers.ApplicationContext
Expand Down Expand Up @@ -66,7 +66,7 @@ class PeriodicSyncWorkerTest {

// Run PeriodicSyncWorker as TestWorker
val inputData = workDataOf(
BaseSyncWorker.INPUT_AUTHORITY to CalendarContract.AUTHORITY,
BaseSyncWorker.INPUT_DATA_TYPE to SyncDataType.EVENTS.toString(),
BaseSyncWorker.INPUT_ACCOUNT_NAME to invalidAccount.name,
BaseSyncWorker.INPUT_ACCOUNT_TYPE to invalidAccount.type
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ package at.bitfire.davdroid.sync.worker

import android.accounts.Account
import android.content.Context
import android.provider.CalendarContract
import androidx.hilt.work.HiltWorkerFactory
import at.bitfire.davdroid.TestUtils
import at.bitfire.davdroid.TestUtils.workScheduledOrRunning
import at.bitfire.davdroid.sync.SyncDataType
import at.bitfire.davdroid.sync.account.TestAccountAuthenticator
import dagger.hilt.android.qualifiers.ApplicationContext
import dagger.hilt.android.testing.HiltAndroidRule
Expand Down Expand Up @@ -59,10 +59,10 @@ class SyncWorkerManagerTest {

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

val returnedName = syncWorkerManager.enqueueOneTime(account, CalendarContract.AUTHORITY)
val returnedName = syncWorkerManager.enqueueOneTime(account, SyncDataType.EVENTS)
assertEquals(workerName, returnedName)
assertTrue(TestUtils.workScheduledOrRunningOrSuccessful(context, workerName))
}
Expand All @@ -72,18 +72,18 @@ class SyncWorkerManagerTest {

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

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

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

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import android.content.Intent
import androidx.core.app.NotificationCompat
import androidx.core.app.NotificationManagerCompat
import at.bitfire.davdroid.R
import at.bitfire.davdroid.sync.SyncDataType
import at.bitfire.davdroid.ui.NotificationRegistry
import at.bitfire.davdroid.ui.account.AccountActivity
import dagger.hilt.android.qualifiers.ApplicationContext
Expand All @@ -20,16 +21,16 @@ class PushNotificationManager @Inject constructor(
/**
* Generates the notification ID for a push notification.
*/
private fun notificationId(account: Account, authority: String): Int {
return account.name.hashCode() + account.type.hashCode() + authority.hashCode()
private fun notificationId(account: Account, dataType: SyncDataType): Int {
return account.name.hashCode() + account.type.hashCode() + dataType.hashCode()
}

/**
* Sends a notification to inform the user that a push notification has been received, the
* sync has been scheduled, but it still has not run.
*/
fun notify(account: Account, authority: String) {
notificationRegistry.notifyIfPossible(notificationId(account, authority)) {
fun notify(account: Account, dataType: SyncDataType) {
notificationRegistry.notifyIfPossible(notificationId(account, dataType)) {
NotificationCompat.Builder(context, notificationRegistry.CHANNEL_STATUS)
.setSmallIcon(R.drawable.ic_sync)
.setContentTitle(context.getString(R.string.sync_notification_pending_push_title))
Expand Down Expand Up @@ -57,9 +58,9 @@ class PushNotificationManager @Inject constructor(
* Once the sync has been started, the notification is no longer needed and can be dismissed.
* It's safe to call this method even if the notification has not been shown.
*/
fun dismiss(account: Account, authority: String) {
fun dismiss(account: Account, dataType: SyncDataType) {
NotificationManagerCompat.from(context)
.cancel(notificationId(account, authority))
.cancel(notificationId(account, dataType))
}

}
Loading

0 comments on commit f503ce5

Please sign in to comment.