fix(#303, #240): fall back to an account-less request for accounts the app can't see

app/src/androidTest/java/de/jeanlucmakiola/calendula/data/calendar/CalendarRepositorySmokeTest.kt

@@ -37,6 +37,7 @@ class CalendarRepositorySmokeTest {
             CalendarPrefs(store),
             SettingsPrefs(store),
             Dispatchers.IO,
+            AndroidSyncRequester(context),
         )
     }
 

app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarRepositoryImpl.kt

@@ -94,7 +94,7 @@ class CalendarRepositoryImpl @Inject constructor(
      * multi-row save or a whole import asks once per account.
      */
     private suspend fun <T> write(
-        syncCalendars: suspend () -> Collection<Long?> = { emptyList() },
+        syncCalendars: suspend () -> Collection<Long?>,
         block: suspend () -> T,
     ): T = withContext(io) {
         val accounts = syncAccountsOf(syncCalendars)
@@ -103,10 +103,12 @@ class CalendarRepositoryImpl @Inject constructor(
         } finally {
             refresh()
             writes.tryEmit(Unit)
-            accounts.forEach { requestSync(it, userInitiated = false) }
+            requestSyncs(accounts, userInitiated = false)
         }
     }
 
+    private val noSync: suspend () -> Collection<Long?> = { emptyList() }
+
     private suspend fun syncAccountsOf(calendarIds: suspend () -> Collection<Long?>): Set<SyncAccount> =
         try {
             calendarIds().filterNotNull().distinct()
@@ -138,7 +140,26 @@ class CalendarRepositoryImpl @Inject constructor(
         return candidates.filterTo(linkedSetOf()) { it.type in adapterTypes }
     }
 
-    private fun requestSync(account: SyncAccount, userInitiated: Boolean): Boolean = try {
+    /**
+     * Ask each of [accounts] once; those the app can't name (see
+     * [SyncRequester.isAddressable]) share one request for every calendar
+     * account. Returns how many of [accounts] were asked.
+     */
+    private fun requestSyncs(accounts: Set<SyncAccount>, userInitiated: Boolean): Int {
+        val (direct, hidden) = accounts.partition { isAddressable(it) }
+        var sent = direct.count { requestSync(it, userInitiated) }
+        if (hidden.isNotEmpty() && requestSync(null, userInitiated)) sent += hidden.size
+        return sent
+    }
+
+    private fun isAddressable(account: SyncAccount): Boolean = try {
+        syncRequester.isAddressable(account)
+    } catch (e: Exception) {
+        Log.w(TAG, "Account visibility unreadable", e)
+        false
+    }
+
+    private fun requestSync(account: SyncAccount?, userInitiated: Boolean): Boolean = try {
         syncRequester.requestSync(account, userInitiated)
         true
     } catch (e: Exception) {
@@ -170,7 +191,7 @@ class CalendarRepositoryImpl @Inject constructor(
             return@withContext SyncRequestResult.Failed
         }
         if (accounts.isEmpty()) return@withContext SyncRequestResult.NothingToSync
-        val sent = accounts.count { requestSync(it, userInitiated = true) }
+        val sent = requestSyncs(accounts, userInitiated = true)
         when (sent) {
             accounts.size -> SyncRequestResult.Requested
             0 -> SyncRequestResult.Failed
@@ -302,7 +323,7 @@ class CalendarRepositoryImpl @Inject constructor(
         displayName: String,
         color: Int,
         description: String?,
-    ): Long = write {
+    ): Long = write(noSync) {
         dataSource.createLocalCalendar(displayName, color, description)
     }
 
@@ -311,13 +332,14 @@ class CalendarRepositoryImpl @Inject constructor(
         displayName: String,
         color: Int,
         description: String?,
-    ) = write { dataSource.updateCalendar(id, displayName, color, description) }
+    ) = write(noSync) { dataSource.updateCalendar(id, displayName, color, description) }
 
     override suspend fun deleteCalendar(id: Long) =
-        write { dataSource.deleteCalendar(id) }
+        write(noSync) { dataSource.deleteCalendar(id) }
 
     override suspend fun setCalendarsVisible(ids: Collection<Long>, visible: Boolean) =
-        write {
+        // VISIBLE is device-local: nothing for an adapter to upload.
+        write(noSync) {
             val canWrite = dataSource.canWriteCalendars()
             if (canWrite) ids.forEach { dataSource.setCalendarVisible(it, visible) }
             // Nothing of ours is left waiting for the provider once the write

app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/SyncRequester.kt

@@ -1,9 +1,13 @@
 package de.jeanlucmakiola.calendula.data.calendar
 
 import android.accounts.Account
+import android.accounts.AccountManager
 import android.content.ContentResolver
+import android.content.Context
+import android.os.Build
 import android.os.Bundle
 import android.provider.CalendarContract
+import dagger.hilt.android.qualifiers.ApplicationContext
 import de.jeanlucmakiola.calendula.domain.CalendarSource
 import javax.inject.Inject
 import javax.inject.Singleton
@@ -24,27 +28,40 @@ interface SyncRequester {
     fun adapterAccountTypes(): Set<String>
 
     /**
-     * Schedule a sync of [account]; may throw if the system refuses. [userInitiated]
-     * marks it manual, which also overrides master-sync-off — only for a user's tap.
+     * Whether a request naming [account] reaches the system. From Android 13 one
+     * for an account the app can't see is dropped without a word.
      */
-    fun requestSync(account: SyncAccount, userInitiated: Boolean)
+    fun isAddressable(account: SyncAccount): Boolean
+
+    /**
+     * Schedule a sync of [account], or of every calendar account when null; may
+     * throw if the system refuses. [userInitiated] marks it manual, which also
+     * overrides master-sync-off — only for a user's tap.
+     */
+    fun requestSync(account: SyncAccount?, userInitiated: Boolean)
 }
 
 @Singleton
-class AndroidSyncRequester @Inject constructor() : SyncRequester {
+class AndroidSyncRequester @Inject constructor(
+    @ApplicationContext private val context: Context,
+) : SyncRequester {
     // Lists only adapters in packages visible to the app — hence the SyncAdapter <queries> entry.
     override fun adapterAccountTypes(): Set<String> =
         ContentResolver.getSyncAdapterTypes()
             .filter { it.authority == CalendarContract.AUTHORITY }
             .mapTo(mutableSetOf()) { it.accountType }
 
-    override fun requestSync(account: SyncAccount, userInitiated: Boolean) {
+    override fun isAddressable(account: SyncAccount): Boolean =
+        Build.VERSION.SDK_INT < Build.VERSION_CODES.TIRAMISU ||
+            AccountManager.get(context).getAccountsByType(account.type).any { it.name == account.name }
+
+    override fun requestSync(account: SyncAccount?, userInitiated: Boolean) {
         val extras = Bundle().apply {
             putBoolean(ContentResolver.SYNC_EXTRAS_EXPEDITED, true)
             if (userInitiated) putBoolean(ContentResolver.SYNC_EXTRAS_MANUAL, true)
         }
         ContentResolver.requestSync(
-            Account(account.name, account.type),
+            account?.let { Account(it.name, it.type) },
             CalendarContract.AUTHORITY,
             extras,
         )

app/src/main/java/de/jeanlucmakiola/calendula/ui/common/CalendarDrawer.kt

@@ -169,7 +169,7 @@ private fun SyncNowRow(sync: DrawerSync) {
         } else {
             null
         },
-        onClick = { if (!sync.inFlight) sync.onSync() },
+        onClick = sync.onSync.takeIf { !sync.inFlight },
     )
 }
 

app/src/main/java/de/jeanlucmakiola/calendula/ui/common/SyncViewModel.kt

@@ -46,8 +46,6 @@ class SyncViewModel @Inject constructor(
     fun requestSync() {
         if (_inFlight.value) return
         _inFlight.value = true
-        // A repeat receipt must land as a new one, restarting the chip.
-        _result.value = null
         viewModelScope.launch {
             try {
                 // Listening before asking, so a sync that lands at once still counts.

app/src/test/java/de/jeanlucmakiola/calendula/TestRepositories.kt

@@ -51,22 +51,27 @@ fun testRepository(
 /**
  * Records every sync request; [accepts] false models the system refusing all,
  * [refusedTypes] refusing those account types, [adapterTypes] the installed
- * calendar sync adapters (null: a fixed set covering the test accounts).
+ * calendar sync adapters (null: a fixed set covering the test accounts),
+ * [hiddenTypes] account types the app can't address by name.
  */
 class RecordingSyncRequester(
     var accepts: Boolean = true,
     var adapterTypes: Set<String>? = null,
     var refusedTypes: Set<String> = emptySet(),
+    var hiddenTypes: Set<String> = emptySet(),
 ) : SyncRequester {
-    data class Request(val account: SyncAccount, val userInitiated: Boolean)
+    /** [account] null: one request for every calendar account. */
+    data class Request(val account: SyncAccount?, val userInitiated: Boolean)
 
     val requests = mutableListOf<Request>()
 
     override fun adapterAccountTypes(): Set<String> =
         adapterTypes ?: setOf("bitfire.at.davdroid", "com.google", "LOCAL")
 
-    override fun requestSync(account: SyncAccount, userInitiated: Boolean) {
-        if (!accepts || account.type in refusedTypes) throw SecurityException("refused")
+    override fun isAddressable(account: SyncAccount): Boolean = account.type !in hiddenTypes
+
+    override fun requestSync(account: SyncAccount?, userInitiated: Boolean) {
+        if (!accepts || account?.type in refusedTypes) throw SecurityException("refused")
         requests += Request(account, userInitiated)
     }
 }

app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/SyncNudgeTest.kt

@@ -220,4 +220,18 @@ class SyncNudgeTest {
         assertThat(repo.requestSync()).isEqualTo(SyncRequestResult.PartlyFailed)
         assertThat(sync.requests).containsExactly(Request(dav, true))
     }
+
+    @Test
+    fun `accounts the app can't see share one request for every calendar account`(@TempDir dir: Path) = runTest {
+        val sync = RecordingSyncRequester(hiddenTypes = setOf(dav.type, google.type))
+        val repo = testRepository(fake(), dir, Dispatchers.Unconfined, syncRequester = sync)
+
+        repo.moveEvent(10L, 3L, form(1L), form(3L))
+        assertThat(sync.requests).containsExactly(Request(null, false))
+
+        sync.requests.clear()
+        sync.hiddenTypes = setOf(google.type)
+        assertThat(repo.requestSync()).isEqualTo(SyncRequestResult.Requested)
+        assertThat(sync.requests).containsExactly(Request(dav, true), Request(null, true)).inOrder()
+    }
 }

docs/ARCHITECTURE.md

@@ -17,8 +17,10 @@ the package list (recurring writes, save conflicts, reminder delivery).
    a manual request from the drawer's "Sync now" (#240), which like the
    system's own "Sync now" also runs for accounts whose auto-sync is off. Both
    go through `SyncRequester` and only ask account types with a calendar sync
-   adapter (visible to the app via a `<queries>` entry); whether and when the
-   adapter syncs stays its call.
+   adapter (visible to the app via a `<queries>` entry). From Android 13 a
+   request naming an account the app can't see is silently dropped, so those
+   accounts share one account-less request, which covers every calendar
+   account. Whether and when the adapter syncs stays its call.
 2. **Observer-driven UI.** A `ContentObserver` on the provider triggers
    re-queries; every screen recomposes from fresh provider state. After a
    write, nothing is patched by hand — the views refresh from the provider.