fix(contacts): serialize sync/teardown and stop retrying on lost permission
- The periodic job and the immediate 'Sync now'/foreground runs had no mutual exclusion, so two overlapping reconciles could each see no managed calendar and both create one (duplicate 'Birthdays', doubled events); likewise a disable-teardown racing an in-flight sync got its calendars recreated right after deletion. A lifecycle Mutex now makes sync() and teardown() atomic, and sync() re-reads the enabled flag inside the lock so a teardown always wins. - The foreground resume trigger shared a unique work name with enable/'Sync now' under ExistingWorkPolicy.REPLACE, so a debounced foreground enqueue could cancel-and-swallow a pending enable sync (feature on, no calendars for up to a day). It now uses its own work name. - doWork() mapped every exception to retry(), so a revoked calendar permission retried with backoff forever and never surfaced. A SecurityException now parks the feature in the stalled state instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -30,6 +30,7 @@ object SpecialDatesScheduler {
|
|||||||
|
|
||||||
private const val WORK_NAME = "special-dates-sync"
|
private const val WORK_NAME = "special-dates-sync"
|
||||||
private const val WORK_NAME_NOW = "special-dates-sync-now"
|
private const val WORK_NAME_NOW = "special-dates-sync-now"
|
||||||
|
private const val WORK_NAME_FOREGROUND = "special-dates-sync-foreground"
|
||||||
private const val KEY_FOREGROUND = "foreground"
|
private const val KEY_FOREGROUND = "foreground"
|
||||||
|
|
||||||
/** Enqueue (or cancel) the daily periodic reconcile to match [enabled]. */
|
/** Enqueue (or cancel) the daily periodic reconcile to match [enabled]. */
|
||||||
@@ -38,6 +39,7 @@ object SpecialDatesScheduler {
|
|||||||
if (!enabled) {
|
if (!enabled) {
|
||||||
workManager.cancelUniqueWork(WORK_NAME)
|
workManager.cancelUniqueWork(WORK_NAME)
|
||||||
workManager.cancelUniqueWork(WORK_NAME_NOW)
|
workManager.cancelUniqueWork(WORK_NAME_NOW)
|
||||||
|
workManager.cancelUniqueWork(WORK_NAME_FOREGROUND)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
val request = PeriodicWorkRequestBuilder<SpecialDatesSyncWorker>(1, TimeUnit.DAYS)
|
val request = PeriodicWorkRequestBuilder<SpecialDatesSyncWorker>(1, TimeUnit.DAYS)
|
||||||
@@ -48,15 +50,19 @@ object SpecialDatesScheduler {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Run one reconcile immediately. [foreground] runs are debounced (the app can
|
* Run one reconcile immediately. [foreground] runs (the app resuming) are
|
||||||
* resume often); enable/"Sync now" runs ([foreground] false) always sync.
|
* debounced inside the worker and use their own work name, so a frequent
|
||||||
|
* foreground resync can never REPLACE — and swallow — a pending enable /
|
||||||
|
* "Sync now" run, which always syncs. The engine serializes the two if they
|
||||||
|
* overlap.
|
||||||
*/
|
*/
|
||||||
fun runNow(context: Context, foreground: Boolean = false) {
|
fun runNow(context: Context, foreground: Boolean = false) {
|
||||||
val request = OneTimeWorkRequestBuilder<SpecialDatesSyncWorker>()
|
val request = OneTimeWorkRequestBuilder<SpecialDatesSyncWorker>()
|
||||||
.setInputData(Data.Builder().putBoolean(KEY_FOREGROUND, foreground).build())
|
.setInputData(Data.Builder().putBoolean(KEY_FOREGROUND, foreground).build())
|
||||||
.build()
|
.build()
|
||||||
|
val name = if (foreground) WORK_NAME_FOREGROUND else WORK_NAME_NOW
|
||||||
WorkManager.getInstance(context)
|
WorkManager.getInstance(context)
|
||||||
.enqueueUniqueWork(WORK_NAME_NOW, ExistingWorkPolicy.REPLACE, request)
|
.enqueueUniqueWork(name, ExistingWorkPolicy.REPLACE, request)
|
||||||
}
|
}
|
||||||
|
|
||||||
internal const val INPUT_FOREGROUND = KEY_FOREGROUND
|
internal const val INPUT_FOREGROUND = KEY_FOREGROUND
|
||||||
@@ -107,6 +113,13 @@ class SpecialDatesSyncWorker(
|
|||||||
}
|
}
|
||||||
if (foreground) prefs.setSpecialDatesLastForegroundSync(now)
|
if (foreground) prefs.setSpecialDatesLastForegroundSync(now)
|
||||||
Result.success()
|
Result.success()
|
||||||
|
} catch (e: SecurityException) {
|
||||||
|
// A revoked calendar/contacts permission won't fix itself on retry —
|
||||||
|
// park the feature in a stalled state (surfaced in settings) instead
|
||||||
|
// of retrying with backoff forever.
|
||||||
|
Log.w(TAG, "Special-dates sync lacks a required permission", e)
|
||||||
|
prefs.recordSpecialDatesRun(now, SpecialDatesStalledReason.PermissionRevoked)
|
||||||
|
Result.success()
|
||||||
} catch (e: Exception) {
|
} catch (e: Exception) {
|
||||||
Log.w(TAG, "Special-dates sync failed", e)
|
Log.w(TAG, "Special-dates sync failed", e)
|
||||||
Result.retry()
|
Result.retry()
|
||||||
|
|||||||
@@ -13,10 +13,12 @@ import de.jeanlucmakiola.calendula.domain.SimpleRecurrence
|
|||||||
import de.jeanlucmakiola.calendula.domain.contacts.ContactSpecialDate
|
import de.jeanlucmakiola.calendula.domain.contacts.ContactSpecialDate
|
||||||
import de.jeanlucmakiola.calendula.domain.contacts.SpecialDateType
|
import de.jeanlucmakiola.calendula.domain.contacts.SpecialDateType
|
||||||
import de.jeanlucmakiola.calendula.domain.contacts.anchorDate
|
import de.jeanlucmakiola.calendula.domain.contacts.anchorDate
|
||||||
import de.jeanlucmakiola.calendula.domain.contacts.managedEventUid
|
import de.jeanlucmakiola.calendula.domain.contacts.managedUid
|
||||||
import de.jeanlucmakiola.calendula.domain.contacts.renderSpecialDateTitle
|
import de.jeanlucmakiola.calendula.domain.contacts.renderSpecialDateTitle
|
||||||
import de.jeanlucmakiola.calendula.domain.toRRule
|
import de.jeanlucmakiola.calendula.domain.toRRule
|
||||||
import kotlinx.coroutines.flow.first
|
import kotlinx.coroutines.flow.first
|
||||||
|
import kotlinx.coroutines.sync.Mutex
|
||||||
|
import kotlinx.coroutines.sync.withLock
|
||||||
import kotlinx.datetime.LocalDateTime
|
import kotlinx.datetime.LocalDateTime
|
||||||
import kotlinx.datetime.LocalTime
|
import kotlinx.datetime.LocalTime
|
||||||
import java.time.ZoneId
|
import java.time.ZoneId
|
||||||
@@ -64,11 +66,19 @@ class SpecialDatesSyncEngine @Inject constructor(
|
|||||||
private val spec: SpecialDatesCalendarSpec,
|
private val spec: SpecialDatesCalendarSpec,
|
||||||
) {
|
) {
|
||||||
|
|
||||||
|
// Serializes calendar-lifecycle work: two overlapping syncs (the daily job
|
||||||
|
// racing a "Sync now"/foreground run) would each see no managed calendar and
|
||||||
|
// both create one; a teardown racing an in-flight sync would delete calendars
|
||||||
|
// the sync then recreates. Holding this for the whole of sync()/teardown()
|
||||||
|
// makes those check-then-act sequences atomic, and — because sync() re-reads
|
||||||
|
// the enabled flag inside the lock — a teardown always wins the race.
|
||||||
|
private val lifecycleMutex = Mutex()
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Reconcile every enabled type against the device's contacts. Returns why it
|
* Reconcile every enabled type against the device's contacts. Returns why it
|
||||||
* stopped early (disabled / permission gone) or [SpecialDatesSyncResult.Success].
|
* stopped early (disabled / permission gone) or [SpecialDatesSyncResult.Success].
|
||||||
*/
|
*/
|
||||||
suspend fun sync(): SpecialDatesSyncResult {
|
suspend fun sync(): SpecialDatesSyncResult = lifecycleMutex.withLock {
|
||||||
if (!prefs.specialDatesEnabled.first()) return SpecialDatesSyncResult.Disabled
|
if (!prefs.specialDatesEnabled.first()) return SpecialDatesSyncResult.Disabled
|
||||||
if (!contacts.hasPermission()) return SpecialDatesSyncResult.PermissionMissing
|
if (!contacts.hasPermission()) return SpecialDatesSyncResult.PermissionMissing
|
||||||
|
|
||||||
@@ -91,7 +101,7 @@ class SpecialDatesSyncEngine @Inject constructor(
|
|||||||
reminderCtx = reminderCtx,
|
reminderCtx = reminderCtx,
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
return SpecialDatesSyncResult.Success
|
SpecialDatesSyncResult.Success
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -117,7 +127,7 @@ class SpecialDatesSyncEngine @Inject constructor(
|
|||||||
}
|
}
|
||||||
|
|
||||||
/** Delete every managed calendar and forget its id — used when the feature is turned off. */
|
/** Delete every managed calendar and forget its id — used when the feature is turned off. */
|
||||||
suspend fun teardown() {
|
suspend fun teardown() = lifecycleMutex.withLock {
|
||||||
calendars.findManagedCalendars().forEach { calendars.deleteCalendar(it.id) }
|
calendars.findManagedCalendars().forEach { calendars.deleteCalendar(it.id) }
|
||||||
SpecialDateType.entries.forEach { prefs.setSpecialDatesCalendarId(it, null) }
|
SpecialDateType.entries.forEach { prefs.setSpecialDatesCalendarId(it, null) }
|
||||||
}
|
}
|
||||||
@@ -211,7 +221,7 @@ class SpecialDatesSyncEngine @Inject constructor(
|
|||||||
val dtStartMillis = form.toWriteTimes(ZoneId.systemDefault()).dtStartMillis
|
val dtStartMillis = form.toWriteTimes(ZoneId.systemDefault()).dtStartMillis
|
||||||
return BuiltManagedEvent(
|
return BuiltManagedEvent(
|
||||||
desired = ManagedEventDesired(
|
desired = ManagedEventDesired(
|
||||||
uid = managedEventUid(type, sd.lookupKey),
|
uid = sd.managedUid(),
|
||||||
title = title,
|
title = title,
|
||||||
dtStartMillis = dtStartMillis,
|
dtStartMillis = dtStartMillis,
|
||||||
rrule = YEARLY_RRULE,
|
rrule = YEARLY_RRULE,
|
||||||
|
|||||||
Reference in New Issue
Block a user