[Sync framework] Disable contacts content change triggered syncs if sync interval set to manual only (#1569)

* Fix lint error Signed-off-by: Sunik Kupfer <kupfer@bitfire.at> * Show manual sync interval setting in UI Signed-off-by: Sunik Kupfer <kupfer@bitfire.at> * Disable contacts content change triggered syncs if set to manual; update kdoc Signed-off-by: Sunik Kupfer <kupfer@bitfire.at> * Update comments and kdoc Signed-off-by: Sunik Kupfer <kupfer@bitfire.at> * Automatically close provider Signed-off-by: Sunik Kupfer <kupfer@bitfire.at> * Explicitly handle special case * Rename updateAutomaticSync to updateSyncFrameworkSetting; adjust comments Signed-off-by: Sunik Kupfer <kupfer@bitfire.at> --------- Signed-off-by: Sunik Kupfer <kupfer@bitfire.at> Co-authored-by: Ricki Hirner <hirner@bitfire.at>
2025-12-23 23:17:50 -05:00 · 2025-07-09 11:13:54 +02:00
parent 62dc73c2a0
commit 904c8ba29b
6 changed files with 62 additions and 32 deletions
--- a/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalAddressBook.kt
+++ b/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalAddressBook.kt
@@ -23,6 +23,7 @@ import at.bitfire.davdroid.repository.DavServiceRepository
 import at.bitfire.davdroid.resource.LocalAddressBook.Companion.USER_DATA_READ_ONLY
 import at.bitfire.davdroid.resource.workaround.ContactDirtyVerifier
 import at.bitfire.davdroid.settings.AccountSettings
+import at.bitfire.davdroid.sync.SyncDataType
 import at.bitfire.davdroid.sync.SyncFrameworkIntegration
 import at.bitfire.davdroid.sync.account.SystemAccountUtils
 import at.bitfire.davdroid.sync.account.setAndVerifyUserData
@@ -223,15 +224,18 @@ open class LocalAddressBook @AssistedInject constructor(


    /**
-     * Makes contacts of this address book available to be synced and activates synchronization upon
-     * contact data changes.
+     * Enables or disables sync on content changes for the address book account based on the current sync
+     * interval account setting.
     */
    fun updateSyncFrameworkSettings() {
-        // Enable sync-ability of contacts
-        syncFramework.enableSyncAbility(addressBookAccount, ContactsContract.AUTHORITY)
+        val accountSettings = accountSettingsFactory.create(account)
+        val syncInterval = accountSettings.getSyncInterval(SyncDataType.CONTACTS)

-        // Changes in contact data should trigger syncs
-        syncFramework.enableSyncOnContentChange(addressBookAccount, ContactsContract.AUTHORITY)
+        // Enable/Disable content triggered syncs for the address book account.
+        if (syncInterval != null)
+            syncFramework.enableSyncOnContentChange(addressBookAccount, ContactsContract.AUTHORITY)
+        else
+            syncFramework.disableSyncAbility(addressBookAccount, ContactsContract.AUTHORITY)
    }


--- a/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalAddressBookStore.kt
+++ b/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalAddressBookStore.kt
@@ -152,7 +152,7 @@ class LocalAddressBookStore @Inject constructor(
            localCollection.readOnly = nowReadOnly
        }

-        // make sure it will still be synchronized when contacts are updated
+        // Update automatic synchronization
        localCollection.updateSyncFrameworkSettings()
    }

--- a/app/src/main/kotlin/at/bitfire/davdroid/settings/AccountSettings.kt
+++ b/app/src/main/kotlin/at/bitfire/davdroid/settings/AccountSettings.kt
@@ -177,7 +177,7 @@ class AccountSettings @AssistedInject constructor(
            SyncDataType.EVENTS -> KEY_SYNC_INTERVAL_CALENDARS
            SyncDataType.TASKS -> KEY_SYNC_INTERVAL_TASKS
        }
-        val newValue = if (seconds == null) SYNC_INTERVAL_MANUALLY else seconds
+        val newValue = seconds ?: SYNC_INTERVAL_MANUALLY
        accountManager.setAndVerifyUserData(account, key, newValue.toString())

        automaticSyncManager.updateAutomaticSync(account, dataType)
--- a/app/src/main/kotlin/at/bitfire/davdroid/sync/AutomaticSyncManager.kt
+++ b/app/src/main/kotlin/at/bitfire/davdroid/sync/AutomaticSyncManager.kt
@@ -6,8 +6,10 @@ package at.bitfire.davdroid.sync

 import android.accounts.Account
 import android.provider.CalendarContract
+import android.provider.ContactsContract
 import at.bitfire.davdroid.db.Service
 import at.bitfire.davdroid.repository.DavServiceRepository
+import at.bitfire.davdroid.resource.LocalAddressBookStore
 import at.bitfire.davdroid.settings.AccountSettings
 import at.bitfire.davdroid.sync.worker.SyncWorkerManager
 import kotlinx.coroutines.runBlocking
@@ -27,6 +29,7 @@ import javax.inject.Provider
 */
 class AutomaticSyncManager @Inject constructor(
    private val accountSettingsFactory: AccountSettings.Factory,
+    private val localAddressBookStore: LocalAddressBookStore,
    private val serviceRepository: DavServiceRepository,
    private val syncFramework: SyncFrameworkIntegration,
    private val tasksAppManager: Provider<TasksAppManager>,
@@ -39,15 +42,18 @@ class AutomaticSyncManager @Inject constructor(
    private fun disableAutomaticSync(account: Account, dataType: SyncDataType) {
        workerManager.disablePeriodic(account, dataType)

-        for (authority in dataType.possibleAuthorities())
+        for (authority in dataType.possibleAuthorities()) {
            syncFramework.disableSyncAbility(account, authority)
+            // no need to disable content-triggered sync, as it can't be active when sync-ability is disabled
+        }
    }

    /**
-     * Enables automatic synchronization for the given account and data type and sets it to the given interval:
+     * Enables/Disables automatic synchronization for the given account and data type and sets it to the given interval,
+     * based on sync interval setting in account settings:
     *
-     * 1. Sets up periodic sync for the given data type with the given interval.
-     * 2. Enables sync in the sync framework for the given data type and sets up periodic sync with the given interval.
+     * 1. Enables/Disables periodic sync worker for the given data type with the given interval.
+     * 2. Enables/Disables sync in the sync framework and enables or disables content-triggered syncs for the given data type
     *
     * @param account   the account to synchronize
     * @param dataType  the data type to synchronize
@@ -58,31 +64,44 @@ class AutomaticSyncManager @Inject constructor(
    ) {
        val accountSettings = accountSettingsFactory.create(account)
        val syncInterval = accountSettings.getSyncInterval(dataType)
+
+        // 1. Update sync workers (needs already updated sync interval in AccountSettings).
        if (syncInterval != null) {
-            // update sync workers (needs already updated sync interval in AccountSettings)
            val wifiOnly = accountSettings.getSyncWifiOnly()
            workerManager.enablePeriodic(account, dataType, syncInterval, wifiOnly)
        } else
            workerManager.disablePeriodic(account, dataType)

-        // also enable/disable content-triggered syncs
-        val possibleAuthorities = dataType.possibleAuthorities()
-        val authority: String? = when (dataType) {
-            // Content triggered sync of contacts is handled per address book account in
-            // [LocalAddressBook.updateSyncFrameworkSettings()]
-            SyncDataType.CONTACTS -> null
-            SyncDataType.EVENTS -> CalendarContract.AUTHORITY
-            SyncDataType.TASKS -> tasksAppManager.get().currentProvider()?.authority
+        // 2. Enable/disable content-triggered syncs.
+        if (dataType == SyncDataType.CONTACTS) {
+            // Contact updates are handled by their respective address book accounts, so we must always
+            // disable the content-triggered sync for the main account.
+            syncFramework.disableSyncAbility(account, ContactsContract.AUTHORITY)
+
+            // pass through request to update all existing address books
+            localAddressBookStore.acquireContentProvider()?.use { provider ->
+                for (addressBookAccount in localAddressBookStore.getAll(account, provider))
+                    addressBookAccount.updateSyncFrameworkSettings()
+            }
+
+        } else {
+            // everything but contacts
+            val possibleAuthorities = dataType.possibleAuthorities()
+            val authority: String? = when (dataType) {
+                SyncDataType.CONTACTS -> throw IllegalStateException()  // handled above
+                SyncDataType.EVENTS -> CalendarContract.AUTHORITY
+                SyncDataType.TASKS -> tasksAppManager.get().currentProvider()?.authority
+            }
+            if (authority != null && syncInterval != null) {
+                // enable given authority, but completely disable all other possible authorities
+                // (for instance, tasks apps which are not the current task app)
+                syncFramework.enableSyncOnContentChange(account, authority)
+                for (disableAuthority in possibleAuthorities - authority)
+                    syncFramework.disableSyncAbility(account, disableAuthority)
+            } else
+                for (authority in possibleAuthorities)
+                    syncFramework.disableSyncOnContentChange(account, authority)
        }
-        if (authority != null && syncInterval != null) {
-            // enable given authority, but completely disable all other possible authorities
-            // (for instance, tasks apps which are not the current task app)
-            syncFramework.enableSyncOnContentChange(account, authority)
-            for (disableAuthority in possibleAuthorities - authority)
-                syncFramework.disableSyncAbility(account, disableAuthority)
-        } else
-            for (authority in possibleAuthorities)
-                syncFramework.disableSyncOnContentChange(account, authority)
    }

    /**
@@ -101,7 +120,8 @@ class AutomaticSyncManager @Inject constructor(
    /**
     * Updates automatic synchronization of the given account and data type according to the account services and settings.
     *
-     * If there's a [Service] for the given account and data type, automatic sync is enabled (with details from [AccountSettings]).
+     * If there's a [Service] for the given account and data type, automatic sync may be enabled if sync interval is set
+     * in [AccountSettings].
     * Otherwise, automatic synchronization is disabled.
     *
     * @param account   account for which automatic synchronization shall be updated
--- a/app/src/main/kotlin/at/bitfire/davdroid/sync/SyncFrameworkIntegration.kt
+++ b/app/src/main/kotlin/at/bitfire/davdroid/sync/SyncFrameworkIntegration.kt
@@ -57,6 +57,8 @@ class SyncFrameworkIntegration @Inject constructor(

    /**
     * Disable this account/provider to be syncable.
+     *
+     * If an authority is not syncable, this implies that there's no sync on content changes, too.
     */
    fun disableSyncAbility(account: Account, authority: String) {
        logger.fine("Disabling sync framework for account=$account, authority=$authority")
@@ -72,6 +74,9 @@ class SyncFrameworkIntegration @Inject constructor(

    /**
     * Enable syncing on content (contact, calendar event or task) changes.
+     *
+     * This implies that the [authority] is syncable, so this method makes the [authority]
+     * syncable if required.
     */
    fun enableSyncOnContentChange(account: Account, authority: String) {
        if (!isSyncable(account, authority))
--- a/app/src/main/kotlin/at/bitfire/davdroid/ui/account/AccountSettingsScreen.kt
+++ b/app/src/main/kotlin/at/bitfire/davdroid/ui/account/AccountSettingsScreen.kt
@@ -57,6 +57,7 @@ import androidx.compose.ui.unit.dp
 import androidx.hilt.navigation.compose.hiltViewModel
 import at.bitfire.davdroid.Constants
 import at.bitfire.davdroid.R
+import at.bitfire.davdroid.settings.AccountSettings.Companion.SYNC_INTERVAL_MANUALLY
 import at.bitfire.davdroid.settings.Credentials
 import at.bitfire.davdroid.ui.AppTheme
 import at.bitfire.davdroid.ui.composable.ActionCard
@@ -505,7 +506,7 @@ fun SyncIntervalSetting(
        MultipleChoiceInputDialog(
            title = stringResource(name),
            namesAndValues = syncIntervalNames.zip(syncIntervalSeconds),
-            initialValue = syncInterval.toString(),
+            initialValue = (syncInterval ?: SYNC_INTERVAL_MANUALLY).toString(),
            onValueSelected = { newValue ->
                try {
                    val seconds = newValue.toLong()