diff --git a/app/src/androidTest/java/de/jeanlucmakiola/calendula/data/calendar/CalendarRepositorySmokeTest.kt b/app/src/androidTest/java/de/jeanlucmakiola/calendula/data/calendar/CalendarRepositorySmokeTest.kt index d6eea3f1..308ca955 100644 --- a/app/src/androidTest/java/de/jeanlucmakiola/calendula/data/calendar/CalendarRepositorySmokeTest.kt +++ b/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), ) } diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarRepositoryImpl.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarRepositoryImpl.kt index 8903e4b4..f760bc89 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarRepositoryImpl.kt +++ b/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 write( - syncCalendars: suspend () -> Collection = { emptyList() }, + syncCalendars: suspend () -> Collection, 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 = { emptyList() } + private suspend fun syncAccountsOf(calendarIds: suspend () -> Collection): Set = 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, 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, 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 diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/SyncRequester.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/SyncRequester.kt index ddcd20df..f010bbe6 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/SyncRequester.kt +++ b/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 /** - * 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 entry. override fun adapterAccountTypes(): Set = 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, ) diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/CalendarDrawer.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/CalendarDrawer.kt index 602bf99a..46de5fdf 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/CalendarDrawer.kt +++ b/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 }, ) } diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/SyncViewModel.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/SyncViewModel.kt index b80a822a..4c05144c 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/SyncViewModel.kt +++ b/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. diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/TestRepositories.kt b/app/src/test/java/de/jeanlucmakiola/calendula/TestRepositories.kt index 44eb1ae0..b838d682 100644 --- a/app/src/test/java/de/jeanlucmakiola/calendula/TestRepositories.kt +++ b/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? = null, var refusedTypes: Set = emptySet(), + var hiddenTypes: Set = 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() override fun adapterAccountTypes(): Set = 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) } } diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/SyncNudgeTest.kt b/app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/SyncNudgeTest.kt index 8a347768..f040b36e 100644 --- a/app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/SyncNudgeTest.kt +++ b/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() + } } diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 426ca7da..e913d0bb 100644 --- a/docs/ARCHITECTURE.md +++ b/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 `` entry); whether and when the - adapter syncs stays its call. + adapter (visible to the app via a `` 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.