Remove address books sync authority and content provider (#877)

sunkup · rfc2822 · web-flow · commit 51f01b215cb3 · 2024-07-11T11:17:09.000+02:00
* Drop address book provider * Drop address book sync adapter service * Replace address books authority with contacts authority * Update kdoc * Revert "Update kdoc" This reverts commit dfb14d4. * Revert "Replace address books authority with contacts authority" This reverts commit 0e15bf1. * Don't enable addressbook sync for main account * Acquire content provider in Syncer * Use contacts authority instead of address book authority when acquiring content provider * Set default sync interval for address book authority again * Minor re-ordering of lines * Add comment, rename variable * Move SyncAdapterServices out of adapter package --------- Co-authored-by: Ricki Hirner <hirner@bitfire.at>
diff --git a/app/src/androidTestOse/kotlin/at/bitfire/davdroid/sync/SyncerTest.kt b/app/src/androidTestOse/kotlin/at/bitfire/davdroid/sync/SyncerTest.kt
@@ -34,7 +34,6 @@ class SyncerTest {
 
     /** use our WebDAV provider as a mock provider because it's our own and we don't need any permissions for it */
     private val mockAuthority = context.getString(R.string.webdav_authority)
-    private val mockProvider = context.contentResolver!!.acquireContentProviderClient(mockAuthority)!!
 
     val account = Account(javaClass.canonicalName, context.getString(R.string.account_type))
 
@@ -47,7 +46,7 @@ class SyncerTest {
     @Test
     fun testOnPerformSync_runsSyncAndSetsClassLoader() {
         val syncer = TestSyncer(context, db)
-        syncer.onPerformSync(account, arrayOf(), mockAuthority, mockProvider, SyncResult())
+        syncer.onPerformSync(account, arrayOf(), mockAuthority, SyncResult())
 
         // check whether onPerformSync() actually calls sync()
         assertEquals(1, syncer.syncCalled.get())
diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml
@@ -177,7 +177,7 @@
                 android:resource="@xml/account_authenticator"/>
         </service>
         <service
-            android:name=".sync.adapter.CalendarsSyncAdapterService"
+            android:name=".sync.CalendarsSyncAdapterService"
             android:exported="true"
             tools:ignore="ExportedService">
             <intent-filter>
@@ -188,7 +188,7 @@
                 android:resource="@xml/sync_calendars"/>
         </service>
         <service
-            android:name=".sync.adapter.JtxSyncAdapterService"
+            android:name=".sync.JtxSyncAdapterService"
             android:exported="true"
             tools:ignore="ExportedService">
             <intent-filter>
@@ -199,7 +199,7 @@
                 android:resource="@xml/sync_notes"/>
         </service>
         <service
-            android:name=".sync.adapter.OpenTasksSyncAdapterService"
+            android:name=".sync.OpenTasksSyncAdapterService"
             android:exported="true"
             tools:ignore="ExportedService">
             <intent-filter>
@@ -210,7 +210,7 @@
                 android:resource="@xml/sync_opentasks"/>
         </service>
         <service
-            android:name=".sync.adapter.TasksOrgSyncAdapterService"
+            android:name=".sync.TasksOrgSyncAdapterService"
             android:exported="true"
             tools:ignore="ExportedService">
             <intent-filter>
@@ -244,25 +244,8 @@
                 android:name="android.accounts.AccountAuthenticator"
                 android:resource="@xml/account_authenticator_address_book"/>
         </service>
-        <provider
-            android:authorities="@string/address_books_authority"
-            android:exported="false"
-            android:label="@string/address_books_authority_title"
-            android:name=".sync.adapter.AddressBookProvider" />
-        <service
-            android:name=".sync.adapter.AddressBooksSyncAdapterService"
-            android:exported="true"
-            tools:ignore="ExportedService">
-            <intent-filter>
-                <action android:name="android.content.SyncAdapter"/>
-            </intent-filter>
-
-            <meta-data
-                android:name="android.content.SyncAdapter"
-                android:resource="@xml/sync_address_books"/>
-        </service>
         <service
-            android:name=".sync.adapter.ContactsSyncAdapterService"
+            android:name=".sync.ContactsSyncAdapterService"
             android:exported="true"
             tools:ignore="ExportedService">
             <intent-filter>
diff --git a/app/src/main/kotlin/at/bitfire/davdroid/repository/AccountRepository.kt b/app/src/main/kotlin/at/bitfire/davdroid/repository/AccountRepository.kt
@@ -90,26 +90,19 @@ class AccountRepository @Inject constructor(
                 // insert CardDAV service
                 val id = insertService(accountName, Service.TYPE_CARDDAV, config.cardDAV)
 
-                // initial CardDAV account settings
+                // initial CardDAV account settings and sync intervals
                 accountSettings.setGroupMethod(groupMethod)
+                accountSettings.setSyncInterval(addrBookAuthority, defaultSyncInterval)
 
                 // start CardDAV service detection (refresh collections)
                 RefreshCollectionsWorker.enqueue(context, id)
-
-                // set default sync interval and enable sync regardless of permissions
-                ContentResolver.setIsSyncable(account, addrBookAuthority, 1)
-                accountSettings.setSyncInterval(addrBookAuthority, defaultSyncInterval)
-            } else
-                ContentResolver.setIsSyncable(account, addrBookAuthority, 0)
+            }
 
             // Configure CalDAV service
             if (config.calDAV != null) {
                 // insert CalDAV service
                 val id = insertService(accountName, Service.TYPE_CALDAV, config.calDAV)
 
-                // start CalDAV service detection (refresh collections)
-                RefreshCollectionsWorker.enqueue(context, id)
-
                 // set default sync interval and enable sync regardless of permissions
                 ContentResolver.setIsSyncable(account, CalendarContract.AUTHORITY, 1)
                 accountSettings.setSyncInterval(CalendarContract.AUTHORITY, defaultSyncInterval)
@@ -123,6 +116,9 @@ class AccountRepository @Inject constructor(
                     Logger.log.info("Tasks provider ${taskProvider.authority} found. Tasks sync enabled.")
                 } else
                     Logger.log.info("No tasks provider found. Did not enable tasks sync.")
+
+                // start CalDAV service detection (refresh collections)
+                RefreshCollectionsWorker.enqueue(context, id)
             } else
                 ContentResolver.setIsSyncable(account, CalendarContract.AUTHORITY, 0)
 
diff --git a/app/src/main/kotlin/at/bitfire/davdroid/settings/AccountSettings.kt b/app/src/main/kotlin/at/bitfire/davdroid/settings/AccountSettings.kt
@@ -217,11 +217,13 @@ class AccountSettings(
      * @return sync interval in seconds; *[SYNC_INTERVAL_MANUALLY]* if manual sync; *null* if not set
      */
     fun getSyncInterval(authority: String): Long? {
-        if (ContentResolver.getIsSyncable(account, authority) <= 0)
+        val addrBookAuthority = context.getString(R.string.address_books_authority)
+
+        if (ContentResolver.getIsSyncable(account, authority) <= 0 && authority != addrBookAuthority)
             return null
 
         val key = when {
-            authority == context.getString(R.string.address_books_authority) ->
+            authority == addrBookAuthority ->
                 KEY_SYNC_INTERVAL_ADDRESSBOOKS
             authority == CalendarContract.AUTHORITY ->
                 KEY_SYNC_INTERVAL_CALENDARS
diff --git a/app/src/main/kotlin/at/bitfire/davdroid/sync/SyncAdapterServices.kt b/app/src/main/kotlin/at/bitfire/davdroid/sync/SyncAdapterServices.kt
@@ -2,7 +2,7 @@
  * Copyright © All Contributors. See LICENSE and AUTHORS in the root directory for details.
  **************************************************************************************************/
 
-package at.bitfire.davdroid.sync.adapter
+package at.bitfire.davdroid.sync
 
 import android.accounts.Account
 import android.app.Service
@@ -134,7 +134,6 @@ abstract class SyncAdapterService: Service() {
 }
 
 // exported sync adapter services; we need a separate class for each authority
-class AddressBooksSyncAdapterService: SyncAdapterService()
 class CalendarsSyncAdapterService: SyncAdapterService()
 class ContactsSyncAdapterService: SyncAdapterService()
 class JtxSyncAdapterService: SyncAdapterService()
diff --git a/app/src/main/kotlin/at/bitfire/davdroid/sync/Syncer.kt b/app/src/main/kotlin/at/bitfire/davdroid/sync/Syncer.kt
@@ -9,7 +9,9 @@ import android.content.ContentProviderClient
 import android.content.Context
 import android.content.SyncResult
 import android.os.DeadObjectException
+import android.provider.ContactsContract
 import at.bitfire.davdroid.InvalidAccountException
+import at.bitfire.davdroid.R
 import at.bitfire.davdroid.db.AppDatabase
 import at.bitfire.davdroid.log.Logger
 import at.bitfire.davdroid.network.HttpClient
@@ -59,19 +61,44 @@ abstract class Syncer(
         account: Account,
         extras: Array<String>,
         authority: String,
-        provider: ContentProviderClient,
         syncResult: SyncResult
     ) {
         Logger.log.log(Level.INFO, "$authority sync of $account initiated", extras.joinToString(", "))
 
+        // use contacts provider for address books
+        val contentAuthority =
+            if (authority == context.getString(R.string.address_books_authority))
+                ContactsContract.AUTHORITY
+            else
+                authority
+
         val accountSettings by lazy { AccountSettings(context, account) }
         val httpClient = lazy { HttpClient.Builder(context, accountSettings).build() }
 
+        // acquire ContentProviderClient of authority to be synced
+        val provider = try {
+            context.contentResolver.acquireContentProviderClient(contentAuthority)
+        } catch (e: SecurityException) {
+            Logger.log.log(Level.WARNING, "Missing permissions for authority $contentAuthority", e)
+            null
+        }
+
+        if (provider == null) {
+            /* Can happen if
+             - we're not allowed to access the content provider, or
+             - the content provider is not available at all, for instance because the respective
+               system app, like "contacts storage" is disabled */
+            Logger.log.warning("Couldn't connect to content provider of authority $contentAuthority")
+            syncResult.stats.numParseExceptions++ // hard sync error
+            return
+        }
+
+        // run sync
         try {
             val runSync = /* ose */ true
 
             if (runSync)
-                sync(account, extras, authority, httpClient, provider, syncResult)
+                sync(account, extras, contentAuthority, httpClient, provider, syncResult)
 
         } catch (e: DeadObjectException) {
             /* May happen when the remote process dies or (since Android 14) when IPC (for instance with the calendar provider)
@@ -83,15 +110,15 @@ abstract class Syncer(
             Logger.log.log(Level.WARNING, "Account was removed during synchronization", e)
 
         } catch (e: Exception) {
-            Logger.log.log(Level.SEVERE, "Couldn't sync $authority", e)
-            syncResult.stats.numParseExceptions++
+            Logger.log.log(Level.SEVERE, "Couldn't sync $contentAuthority", e)
+            syncResult.stats.numParseExceptions++ // Hard sync error
 
         } finally {
             if (httpClient.isInitialized())
                 httpClient.value.close()
             Logger.log.log(
                 Level.INFO,
-                "$authority sync of $account finished",
+                "$contentAuthority sync of $account finished",
                 extras.joinToString(", "))
         }
     }
diff --git a/app/src/main/kotlin/at/bitfire/davdroid/sync/adapter/AddressBookProvider.kt b/app/src/main/kotlin/at/bitfire/davdroid/sync/adapter/AddressBookProvider.kt
diff --git a/app/src/main/kotlin/at/bitfire/davdroid/sync/worker/BaseSyncWorker.kt b/app/src/main/kotlin/at/bitfire/davdroid/sync/worker/BaseSyncWorker.kt
@@ -290,81 +290,68 @@ abstract class BaseSyncWorker(
             // Comes in through SyncAdapterService and is used only by ContactsSyncManager for an Android 7 workaround.
             extras.add(ContentResolver.SYNC_EXTRAS_UPLOAD)
 
-        // acquire ContentProviderClient of authority to be synced
-        try {
-            applicationContext.contentResolver.acquireContentProviderClient(authority)
-        } catch (e: SecurityException) {
-            Logger.log.log(Level.WARNING, "Missing permissions to acquire ContentProviderClient for $authority", e)
-            null
-        }.use { provider ->
-            if (provider == null) {
-                Logger.log.warning("Couldn't acquire ContentProviderClient for $authority")
-                return@withContext Result.failure()
-            }
-
-            val result = SyncResult()
-            // Start syncing. We still use the sync adapter framework's SyncResult to pass the sync results, but this
-            // is only for legacy reasons and can be replaced by an own result class in the future.
-            runInterruptible {
-                syncer.onPerformSync(account, extras.toTypedArray(), authority, provider, result)
-            }
+        val result = SyncResult()
+        // Start syncing. We still use the sync adapter framework's SyncResult to pass the sync results, but this
+        // is only for legacy reasons and can be replaced by an own result class in the future.
+        runInterruptible {
+            syncer.onPerformSync(account, extras.toTypedArray(), authority, result)
+        }
 
-            // Check for errors
-            if (result.hasError()) {
-                val syncResult = Data.Builder()
-                    .putString("syncresult", result.toString())
-                    .putString("syncResultStats", result.stats.toString())
-                    .build()
-
-                val softErrorNotificationTag = account.type + "-" + account.name + "-" + authority
-
-                // On soft errors the sync is retried a few times before considered failed
-                if (result.hasSoftError()) {
-                    Logger.log.warning("Soft error while syncing: result=$result, stats=${result.stats}")
-                    if (runAttemptCount < MAX_RUN_ATTEMPTS) {
-                        val blockDuration = result.delayUntil - System.currentTimeMillis() / 1000
-                        Logger.log.warning("Waiting for $blockDuration seconds, before retrying ...")
-
-                        // We block the SyncWorker here so that it won't be started by the sync framework immediately again.
-                        // This should be replaced by proper work scheduling as soon as we don't depend on the sync framework anymore.
-                        if (blockDuration > 0)
-                            delay(blockDuration * 1000)
-
-                        Logger.log.warning("Retrying on soft error (attempt $runAttemptCount of $MAX_RUN_ATTEMPTS)")
-                        return@withContext Result.retry()
-                    }
-
-                    Logger.log.warning("Max retries on soft errors reached ($runAttemptCount of $MAX_RUN_ATTEMPTS). Treating as failed")
-
-                    notificationManager.notifyIfPossible(
-                        softErrorNotificationTag,
-                        NotificationUtils.NOTIFY_SYNC_ERROR,
-                        NotificationUtils.newBuilder(applicationContext, NotificationUtils.CHANNEL_SYNC_IO_ERRORS)
-                            .setSmallIcon(R.drawable.ic_sync_problem_notify)
-                            .setContentTitle(account.name)
-                            .setContentText(applicationContext.getString(R.string.sync_error_retry_limit_reached))
-                            .setSubText(account.name)
-                            .setOnlyAlertOnce(true)
-                            .setPriority(NotificationCompat.PRIORITY_MIN)
-                            .setCategory(NotificationCompat.CATEGORY_ERROR)
-                            .build()
-                    )
-
-                    return@withContext Result.failure(syncResult)
+        // Check for errors
+        if (result.hasError()) {
+            val syncResult = Data.Builder()
+                .putString("syncresult", result.toString())
+                .putString("syncResultStats", result.stats.toString())
+                .build()
+
+            val softErrorNotificationTag = account.type + "-" + account.name + "-" + authority
+
+            // On soft errors the sync is retried a few times before considered failed
+            if (result.hasSoftError()) {
+                Logger.log.warning("Soft error while syncing: result=$result, stats=${result.stats}")
+                if (runAttemptCount < MAX_RUN_ATTEMPTS) {
+                    val blockDuration = result.delayUntil - System.currentTimeMillis() / 1000
+                    Logger.log.warning("Waiting for $blockDuration seconds, before retrying ...")
+
+                    // We block the SyncWorker here so that it won't be started by the sync framework immediately again.
+                    // This should be replaced by proper work scheduling as soon as we don't depend on the sync framework anymore.
+                    if (blockDuration > 0)
+                        delay(blockDuration * 1000)
+
+                    Logger.log.warning("Retrying on soft error (attempt $runAttemptCount of $MAX_RUN_ATTEMPTS)")
+                    return@withContext Result.retry()
                 }
 
-                // If no soft error found, dismiss sync error notification
-                notificationManager.cancel(
+                Logger.log.warning("Max retries on soft errors reached ($runAttemptCount of $MAX_RUN_ATTEMPTS). Treating as failed")
+
+                notificationManager.notifyIfPossible(
                     softErrorNotificationTag,
-                    NotificationUtils.NOTIFY_SYNC_ERROR
+                    NotificationUtils.NOTIFY_SYNC_ERROR,
+                    NotificationUtils.newBuilder(applicationContext, NotificationUtils.CHANNEL_SYNC_IO_ERRORS)
+                        .setSmallIcon(R.drawable.ic_sync_problem_notify)
+                        .setContentTitle(account.name)
+                        .setContentText(applicationContext.getString(R.string.sync_error_retry_limit_reached))
+                        .setSubText(account.name)
+                        .setOnlyAlertOnce(true)
+                        .setPriority(NotificationCompat.PRIORITY_MIN)
+                        .setCategory(NotificationCompat.CATEGORY_ERROR)
+                        .build()
                 )
 
-                // On a hard error - fail with an error message
-                // Note: SyncManager should have notified the user
-                if (result.hasHardError()) {
-                    Logger.log.warning("Hard error while syncing: result=$result, stats=${result.stats}")
-                    return@withContext Result.failure(syncResult)
-                }
+                return@withContext Result.failure(syncResult)
+            }
+
+            // If no soft error found, dismiss sync error notification
+            notificationManager.cancel(
+                softErrorNotificationTag,
+                NotificationUtils.NOTIFY_SYNC_ERROR
+            )
+
+            // On a hard error - fail with an error message
+            // Note: SyncManager should have notified the user
+            if (result.hasHardError()) {
+                Logger.log.warning("Hard error while syncing: result=$result, stats=${result.stats}")
+                return@withContext Result.failure(syncResult)
             }
         }
 
diff --git a/app/src/main/res/xml/sync_address_books.xml b/app/src/main/res/xml/sync_address_books.xml
