From 26f9127d84c86b4e5d558cec3f2ff6cfb24cc146 Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Wed, 1 Jul 2026 22:07:51 -0500 Subject: [PATCH] feat(sync): gate full-content fetch on Wi-Fi/battery; IDLE polls at low battery Shared core: BatteryStatusProvider (BatteryManager one-shot + ACTION_BATTERY_CHANGED flow) feeds SyncResourcePolicy, a pure, unit-tested decision object; all gates are runtime-only and self-reverting - no setting is ever mutated. - #88: FetchPolicy now defaults to WIFI_ONLY in both the AppSettings default and the DataStore-read fallback, so fresh installs and never-touched existing installs stop bulk-downloading full content over cellular. An explicitly chosen policy is unaffected. - #89: the aggressive body/attachment prefetch pauses for every FetchPolicy at <=20% battery in BOTH content-prefetch paths - MailSyncer's recent-window prefetch and MailBackfiller's full-history prefetch (#12) - resuming on the next sync once above the threshold; charging exempts. Header sync and backfill header paging (new-mail detection, notifications, history) are untouched. - #90: IdleService watches battery and proactively closes its IDLE connections at <=20%, flipping the foreground notification to say mail is checked every 15 minutes (the always-scheduled periodic sync, re-asserted on entry); IDLE resumes at >=25% or on charger (hysteresis prevents threshold flapping) and catches up missed mail via idle()'s on-connect sync. Co-Authored-By: Claude Fable 5 --- .../data/settings/SettingsRepository.kt | 14 +- .../org/libremail/data/sync/MailBackfiller.kt | 23 ++- .../org/libremail/data/sync/MailSyncer.kt | 20 ++- .../libremail/data/sync/SyncResourcePolicy.kt | 79 ++++++++ .../kotlin/org/libremail/di/PowerModule.kt | 20 +++ .../power/AndroidBatteryStatusProvider.kt | 73 ++++++++ .../libremail/power/BatteryStatusProvider.kt | 32 ++++ .../kotlin/org/libremail/push/IdleService.kt | 77 ++++++-- app/src/main/res/values/strings.xml | 1 + .../data/settings/AppSettingsTest.kt | 40 +++++ .../libremail/data/sync/MailBackfillerTest.kt | 52 +++++- .../data/sync/MailMaintenanceGateTest.kt | 5 + .../org/libremail/data/sync/MailSyncerTest.kt | 52 ++++++ .../data/sync/SyncResourcePolicyTest.kt | 168 ++++++++++++++++++ 14 files changed, 620 insertions(+), 36 deletions(-) create mode 100644 app/src/main/kotlin/org/libremail/data/sync/SyncResourcePolicy.kt create mode 100644 app/src/main/kotlin/org/libremail/di/PowerModule.kt create mode 100644 app/src/main/kotlin/org/libremail/power/AndroidBatteryStatusProvider.kt create mode 100644 app/src/main/kotlin/org/libremail/power/BatteryStatusProvider.kt create mode 100644 app/src/test/kotlin/org/libremail/data/settings/AppSettingsTest.kt create mode 100644 app/src/test/kotlin/org/libremail/data/sync/SyncResourcePolicyTest.kt diff --git a/app/src/main/kotlin/org/libremail/data/settings/SettingsRepository.kt b/app/src/main/kotlin/org/libremail/data/settings/SettingsRepository.kt index f56ffb9..57d6c1d 100644 --- a/app/src/main/kotlin/org/libremail/data/settings/SettingsRepository.kt +++ b/app/src/main/kotlin/org/libremail/data/settings/SettingsRepository.kt @@ -29,9 +29,14 @@ internal val Context.settingsDataStore: DataStore by preferencesDat /** * How aggressively the app downloads message content during sync. - * - [ALWAYS]: fetch full bodies and all attachments on any connection (default). + * - [ALWAYS]: fetch full bodies and all attachments on any connection. * - [WIFI_ONLY]: fetch full content only on an unmetered network; otherwise behave like [ON_DEMAND]. + * The default (#88): with the mailbox's full-history backfill, defaulting to [ALWAYS] would + * silently download every message and attachment over cellular on a fresh install. * - [ON_DEMAND]: sync headers only; fetch a message's body/attachments lazily when it's opened. + * + * Regardless of policy, the prefetch also pauses at low battery — a runtime gate, not a setting + * (see [org.libremail.data.sync.SyncResourcePolicy]). */ enum class FetchPolicy { ALWAYS, WIFI_ONLY, ON_DEMAND } @@ -44,7 +49,7 @@ data class AppSettings( val loadRemoteImages: Boolean = false, val encryptCache: Boolean = false, val includeInBackup: Boolean = false, - val fetchPolicy: FetchPolicy = FetchPolicy.ALWAYS, + val fetchPolicy: FetchPolicy = FetchPolicy.WIFI_ONLY, /** * Global device-only retention defaults (issue #13), applied to accounts that don't override them. * `0` means "keep everything" (the default), matching the fetch-all history behaviour of #12. @@ -79,8 +84,11 @@ internal fun Preferences.toAppSettings(): AppSettings = AppSettings( loadRemoteImages = this[Keys.LOAD_REMOTE_IMAGES] ?: false, encryptCache = this[Keys.ENCRYPT_CACHE] ?: false, includeInBackup = this[Keys.INCLUDE_IN_BACKUP] ?: false, + // The fallback here (not just the AppSettings default) must be WIFI_ONLY so existing installs + // that never touched the setting also pick up the safer default (#88). An explicitly chosen + // policy is stored and always wins. fetchPolicy = this[Keys.FETCH_POLICY]?.let { runCatching { FetchPolicy.valueOf(it) }.getOrNull() } - ?: FetchPolicy.ALWAYS, + ?: FetchPolicy.WIFI_ONLY, retentionCount = this[Keys.RETENTION_COUNT] ?: 0, retentionMonths = this[Keys.RETENTION_MONTHS] ?: 0, ) diff --git a/app/src/main/kotlin/org/libremail/data/sync/MailBackfiller.kt b/app/src/main/kotlin/org/libremail/data/sync/MailBackfiller.kt index 2a200bd..8a2e822 100644 --- a/app/src/main/kotlin/org/libremail/data/sync/MailBackfiller.kt +++ b/app/src/main/kotlin/org/libremail/data/sync/MailBackfiller.kt @@ -17,7 +17,6 @@ import org.libremail.data.local.entity.MessageEntity import org.libremail.data.local.toDomain import org.libremail.data.local.toEntity import org.libremail.data.settings.AccountSettingsRepository -import org.libremail.data.settings.FetchPolicy import org.libremail.data.settings.RetentionPolicy import org.libremail.data.settings.SettingsRepository import org.libremail.data.settings.effectiveRetention @@ -25,6 +24,7 @@ import org.libremail.domain.model.Account import org.libremail.domain.model.ImapConnectionParams import org.libremail.domain.repository.MailRepository import org.libremail.mail.ImapClient +import org.libremail.power.BatteryStatusProvider import javax.inject.Inject import javax.inject.Singleton @@ -50,6 +50,7 @@ class MailBackfiller @Inject constructor( private val connectionFactory: MailConnectionFactory, private val settingsRepository: SettingsRepository, private val accountSettingsRepository: AccountSettingsRepository, + private val batteryStatusProvider: BatteryStatusProvider, private val mailRepository: MailRepository, private val maintenanceGate: MailMaintenanceGate, ) { @@ -169,16 +170,20 @@ class MailBackfiller @Inject constructor( } /** - * Pre-caches each backfilled message's body/attachments per the [FetchPolicy] (headers first, - * bodies per policy — issue #12). Best-effort and cancellable between messages so an interruption - * stops promptly; anything not fetched is filled in lazily when the message is opened. + * Pre-caches each backfilled message's body/attachments per the user's fetch policy (headers + * first, bodies per policy — issue #12), pausing at low battery regardless of policy — the same + * [SyncResourcePolicy.shouldPrefetchContent] gate as [MailSyncer]'s prefetch, so the foreground + * and backfill content paths can never disagree (#88/#89). Header paging above is not gated here: + * WorkManager's battery-not-low constraint on the backfill work is the scheduler-level control. + * Best-effort and cancellable between messages so an interruption stops promptly; anything not + * fetched is filled in lazily when the message is opened. */ private suspend fun prefetchIfEnabled(ids: List) { - val shouldPrefetch = when (settingsRepository.fetchPolicy()) { - FetchPolicy.ALWAYS -> true - FetchPolicy.WIFI_ONLY -> context.isActiveNetworkUnmetered() - FetchPolicy.ON_DEMAND -> false - } + val shouldPrefetch = SyncResourcePolicy.shouldPrefetchContent( + policy = settingsRepository.fetchPolicy(), + unmetered = { context.isActiveNetworkUnmetered() }, + battery = batteryStatusProvider.current(), + ) if (!shouldPrefetch) return for (id in ids) { currentCoroutineContext().ensureActive() diff --git a/app/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt b/app/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt index 425a996..b059125 100644 --- a/app/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt +++ b/app/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt @@ -14,13 +14,13 @@ import org.libremail.data.local.dao.MessageDao import org.libremail.data.local.toDomain import org.libremail.data.local.toEntity import org.libremail.data.settings.AccountSettingsRepository -import org.libremail.data.settings.FetchPolicy import org.libremail.data.settings.SettingsRepository import org.libremail.data.settings.effectiveRetention import org.libremail.domain.model.Account import org.libremail.domain.repository.MailRepository import org.libremail.mail.ImapClient import org.libremail.notifications.MailNotifier +import org.libremail.power.BatteryStatusProvider import javax.inject.Inject import javax.inject.Singleton @@ -34,6 +34,7 @@ class MailSyncer @Inject constructor( private val connectionFactory: MailConnectionFactory, private val settingsRepository: SettingsRepository, private val accountSettingsRepository: AccountSettingsRepository, + private val batteryStatusProvider: BatteryStatusProvider, private val notifier: MailNotifier, private val mailRepository: MailRepository, ) : Syncer { @@ -158,15 +159,18 @@ class MailSyncer @Inject constructor( /** * Aggressively pre-caches each not-yet-fetched message's full content (body + attachments) per the - * user's [FetchPolicy]. Runs outside [syncMutex] so these downloads don't block pull-to-refresh or - * other syncs, and is cancellable between messages so an IDLE renewal stops it promptly. + * user's fetch policy, pausing at low battery regardless of policy — see + * [SyncResourcePolicy.shouldPrefetchContent] (#89). The header sync above is never gated: mail + * keeps arriving, and a skipped prefetch is simply retried on the next sync once battery/network + * recover. Runs outside [syncMutex] so these downloads don't block pull-to-refresh or other syncs, + * and is cancellable between messages so an IDLE renewal stops it promptly. */ private suspend fun prefetchIfEnabled(account: Account, folder: String) { - val shouldPrefetch = when (settingsRepository.fetchPolicy()) { - FetchPolicy.ALWAYS -> true - FetchPolicy.WIFI_ONLY -> context.isActiveNetworkUnmetered() - FetchPolicy.ON_DEMAND -> false - } + val shouldPrefetch = SyncResourcePolicy.shouldPrefetchContent( + policy = settingsRepository.fetchPolicy(), + unmetered = { context.isActiveNetworkUnmetered() }, + battery = batteryStatusProvider.current(), + ) if (!shouldPrefetch) return for (id in messageDao.getUnfetchedIds(account.id, folder)) { currentCoroutineContext().ensureActive() diff --git a/app/src/main/kotlin/org/libremail/data/sync/SyncResourcePolicy.kt b/app/src/main/kotlin/org/libremail/data/sync/SyncResourcePolicy.kt new file mode 100644 index 0000000..13deb34 --- /dev/null +++ b/app/src/main/kotlin/org/libremail/data/sync/SyncResourcePolicy.kt @@ -0,0 +1,79 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.sync + +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.distinctUntilChanged +import kotlinx.coroutines.flow.scan +import org.libremail.data.settings.FetchPolicy +import org.libremail.power.BatteryStatus + +/** How new mail is watched for: a live IMAP IDLE connection, or the 15-minute periodic WorkManager sync. */ +enum class PushMode { IDLE, POLLING } + +/** + * Pure decisions for how sync adapts to device resource constraints (battery level, charging state, + * network metering) — shared by the content-prefetch paths (#88, #89: [MailSyncer]'s recent-window + * prefetch and [MailBackfiller]'s full-history prefetch) and the IMAP IDLE push service (#90). + * Deliberately free of Android types so it is exhaustively unit-testable; the live inputs come from + * [org.libremail.power.BatteryStatusProvider] and the shared [isActiveNetworkUnmetered] check. + * + * These are runtime gates, not settings writes: nothing here mutates a preference, so every effect + * self-reverts as soon as the device leaves the constrained state. + * + * Charging exempts from every battery gate (#89's open question, resolved): a device on power is not + * under battery pressure, so a plugged-in phone at 15% keeps prefetching and keeps IDLE alive. + */ +object SyncResourcePolicy { + + /** At or below this charge (and not charging), battery-sensitive work pauses (#89, #90). */ + const val LOW_BATTERY_PERCENT = 20 + + /** + * Once push has fallen back to polling, IDLE resumes only at or above this charge (or on the + * charger). Strictly above [LOW_BATTERY_PERCENT] so a battery hovering at the threshold cannot + * flap push on and off (hysteresis). + */ + const val PUSH_RESUME_PERCENT = 25 + + /** True when the battery is at or below [LOW_BATTERY_PERCENT] and the device is not charging. */ + fun isBatteryLow(battery: BatteryStatus): Boolean = !battery.isCharging && battery.percent <= LOW_BATTERY_PERCENT + + /** + * Whether to aggressively pre-download full message content (bodies + attachments) after a header + * sync: the user's [FetchPolicy] decides, except at low battery, where the prefetch pauses for + * every policy (#89). [unmetered] is a supplier because only [FetchPolicy.WIFI_ONLY] needs the + * network state — the other policies (and any low-battery pause) never query it. Header sync is + * never gated here — new mail still arrives; a paused prefetch merely defers content caching to + * the next healthy sync (or to on-demand fetch when a message is opened). + */ + fun shouldPrefetchContent(policy: FetchPolicy, unmetered: () -> Boolean, battery: BatteryStatus): Boolean { + if (isBatteryLow(battery)) return false + return when (policy) { + FetchPolicy.ALWAYS -> true + FetchPolicy.WIFI_ONLY -> unmetered() + FetchPolicy.ON_DEMAND -> false + } + } + + /** + * Next push mode given the latest battery snapshot and the [previous] mode: drop to + * [PushMode.POLLING] at low battery; return to [PushMode.IDLE] when charging or once charge + * recovers to [PUSH_RESUME_PERCENT]. Inside the band between the two thresholds the previous mode + * is kept (hysteresis), so jitter around the cutoff can't flap connections. + */ + fun pushMode(battery: BatteryStatus, previous: PushMode): PushMode = when { + battery.isCharging -> PushMode.IDLE + battery.percent <= LOW_BATTERY_PERCENT -> PushMode.POLLING + previous == PushMode.POLLING && battery.percent < PUSH_RESUME_PERCENT -> PushMode.POLLING + else -> PushMode.IDLE + } + + /** + * Folds a stream of battery snapshots into the push mode to run, starting from [initial] + * (assessed with no history, so when collection starts on an already-low battery the very first + * mode is [PushMode.POLLING]). Deduplicated: changes inside the hysteresis band emit nothing. + */ + fun pushModes(initial: BatteryStatus, updates: Flow): Flow = updates + .scan(pushMode(initial, PushMode.IDLE)) { previous, battery -> pushMode(battery, previous) } + .distinctUntilChanged() +} diff --git a/app/src/main/kotlin/org/libremail/di/PowerModule.kt b/app/src/main/kotlin/org/libremail/di/PowerModule.kt new file mode 100644 index 0000000..a072477 --- /dev/null +++ b/app/src/main/kotlin/org/libremail/di/PowerModule.kt @@ -0,0 +1,20 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.di + +import dagger.Binds +import dagger.Module +import dagger.hilt.InstallIn +import dagger.hilt.components.SingletonComponent +import org.libremail.power.AndroidBatteryStatusProvider +import org.libremail.power.BatteryStatusProvider +import javax.inject.Singleton + +/** Bindings for device power/battery state sources. */ +@Module +@InstallIn(SingletonComponent::class) +abstract class PowerModule { + + @Binds + @Singleton + abstract fun bindBatteryStatusProvider(impl: AndroidBatteryStatusProvider): BatteryStatusProvider +} diff --git a/app/src/main/kotlin/org/libremail/power/AndroidBatteryStatusProvider.kt b/app/src/main/kotlin/org/libremail/power/AndroidBatteryStatusProvider.kt new file mode 100644 index 0000000..10ceb54 --- /dev/null +++ b/app/src/main/kotlin/org/libremail/power/AndroidBatteryStatusProvider.kt @@ -0,0 +1,73 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.power + +import android.content.BroadcastReceiver +import android.content.Context +import android.content.Intent +import android.content.IntentFilter +import android.os.BatteryManager +import androidx.core.content.ContextCompat +import dagger.hilt.android.qualifiers.ApplicationContext +import kotlinx.coroutines.channels.awaitClose +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.callbackFlow +import kotlinx.coroutines.flow.distinctUntilChanged +import javax.inject.Inject +import javax.inject.Singleton + +/** + * Reads battery state from [BatteryManager] (one-shot) and from the sticky + * [Intent.ACTION_BATTERY_CHANGED] broadcast (stream). Needs no permission. Anything unreadable + * degrades to [BatteryStatus.ASSUME_OK], so battery gating only ever engages on a definitively low + * battery, never on missing data. + */ +@Singleton +class AndroidBatteryStatusProvider @Inject constructor(@ApplicationContext private val context: Context) : + BatteryStatusProvider { + + override fun current(): BatteryStatus { + val manager = context.getSystemService(BatteryManager::class.java) ?: return BatteryStatus.ASSUME_OK + val percent = manager.getIntProperty(BatteryManager.BATTERY_PROPERTY_CAPACITY) + return BatteryStatus( + percent = if (percent in 0..PERCENT_SCALE) percent else BatteryStatus.ASSUME_OK.percent, + isCharging = manager.isCharging, + ) + } + + /** + * [Intent.ACTION_BATTERY_CHANGED] can only be received by a runtime-registered receiver, so the + * registration lives for exactly as long as the flow is collected. The broadcast is sticky: + * registration returns the latest snapshot, which is emitted immediately. RECEIVER_NOT_EXPORTED + * is correct for a system broadcast — the system is always allowed to deliver to it. + */ + override fun status(): Flow = callbackFlow { + val receiver = object : BroadcastReceiver() { + override fun onReceive(context: Context, intent: Intent) { + trySend(intent.toBatteryStatus()) + } + } + val sticky = ContextCompat.registerReceiver( + context, + receiver, + IntentFilter(Intent.ACTION_BATTERY_CHANGED), + ContextCompat.RECEIVER_NOT_EXPORTED, + ) + trySend(sticky?.toBatteryStatus() ?: current()) + awaitClose { context.unregisterReceiver(receiver) } + }.distinctUntilChanged() + + private fun Intent.toBatteryStatus(): BatteryStatus { + val level = getIntExtra(BatteryManager.EXTRA_LEVEL, -1) + val scale = getIntExtra(BatteryManager.EXTRA_SCALE, -1) + val status = getIntExtra(BatteryManager.EXTRA_STATUS, -1) + val charging = status == BatteryManager.BATTERY_STATUS_CHARGING || status == BatteryManager.BATTERY_STATUS_FULL + return BatteryStatus( + percent = if (level >= 0 && scale > 0) level * PERCENT_SCALE / scale else BatteryStatus.ASSUME_OK.percent, + isCharging = charging, + ) + } + + private companion object { + const val PERCENT_SCALE = 100 + } +} diff --git a/app/src/main/kotlin/org/libremail/power/BatteryStatusProvider.kt b/app/src/main/kotlin/org/libremail/power/BatteryStatusProvider.kt new file mode 100644 index 0000000..e54327b --- /dev/null +++ b/app/src/main/kotlin/org/libremail/power/BatteryStatusProvider.kt @@ -0,0 +1,32 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.power + +import kotlinx.coroutines.flow.Flow + +/** Point-in-time battery snapshot: charge [percent] (0–100) and whether the device is on power. */ +data class BatteryStatus(val percent: Int, val isCharging: Boolean) { + companion object { + /** + * Fallback when the platform can't report battery state (missing service or extras): a full, + * discharging battery, so every battery gate fails open to normal behaviour — bad data can + * never pause sync or push. + */ + val ASSUME_OK = BatteryStatus(percent = 100, isCharging = false) + } +} + +/** + * Battery-state source for the resource-aware sync decisions in + * [org.libremail.data.sync.SyncResourcePolicy]. An interface so those call sites stay unit-testable; + * the live Android implementation is [AndroidBatteryStatusProvider]. + */ +interface BatteryStatusProvider { + /** One-shot snapshot, for point-in-time decisions (e.g. "prefetch content after this sync?"). */ + fun current(): BatteryStatus + + /** + * Stream of snapshots for long-lived consumers (the IDLE push service). Emits the current state + * immediately on collection, then on every battery change, deduplicated. + */ + fun status(): Flow +} diff --git a/app/src/main/kotlin/org/libremail/push/IdleService.kt b/app/src/main/kotlin/org/libremail/push/IdleService.kt index 4b7fd1d..6aeabfa 100644 --- a/app/src/main/kotlin/org/libremail/push/IdleService.kt +++ b/app/src/main/kotlin/org/libremail/push/IdleService.kt @@ -20,6 +20,7 @@ import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.cancel import kotlinx.coroutines.delay import kotlinx.coroutines.flow.collect +import kotlinx.coroutines.flow.combine import kotlinx.coroutines.isActive import kotlinx.coroutines.launch import kotlinx.coroutines.withTimeoutOrNull @@ -28,14 +29,27 @@ import org.libremail.data.local.dao.AccountDao import org.libremail.data.local.toDomain import org.libremail.data.sync.MailConnectionFactory import org.libremail.data.sync.MailSyncer +import org.libremail.data.sync.PushMode +import org.libremail.data.sync.SyncResourcePolicy +import org.libremail.data.sync.SyncScheduler import org.libremail.domain.model.Account import org.libremail.mail.ImapClient +import org.libremail.power.BatteryStatusProvider +import org.libremail.reporting.AppLog import javax.inject.Inject /** * Foreground service that holds a long-lived IMAP IDLE connection per account so the server can * push new mail to us instantly — no polling, and no third-party push service. When IDLE reports * activity we run a normal sync, which writes to Room and fires the new-mail notification. + * + * At low battery (#90) the service stays up but proactively closes every IDLE connection and drops + * to [PushMode.POLLING]: mail then arrives via the always-scheduled 15-minute periodic sync, and the + * persistent notification says so instead of pretending push is live. Once battery recovers (with + * hysteresis — see [SyncResourcePolicy.pushMode]) or the device is plugged in, IDLE resumes and its + * on-connect sync catches up anything that arrived in between. A deliberate, visible fallback beats + * the previous emergent one, where the OS throttled the low-battery socket and the reconnect loop + * burned battery retrying while the notification still claimed instant delivery. */ @AndroidEntryPoint class IdleService : Service() { @@ -48,14 +62,22 @@ class IdleService : Service() { @Inject lateinit var mailSyncer: MailSyncer + @Inject lateinit var batteryStatusProvider: BatteryStatusProvider + + @Inject lateinit var syncScheduler: SyncScheduler + private val scope = CoroutineScope(SupervisorJob() + Dispatchers.IO) private var watching = false + /** The push mode the foreground notification currently reflects. */ + @Volatile + private var shownMode = PushMode.IDLE + /** Active IDLE watcher per account id, so we can start/stop them as accounts change. */ private val watchers = mutableMapOf() override fun onStartCommand(intent: Intent?, flags: Int, startId: Int): Int { - startAsForeground() + startAsForeground(shownMode) if (!watching) { watching = true scope.launch { reconcileWatchers() } @@ -64,17 +86,26 @@ class IdleService : Service() { } /** - * Observes the account list and keeps one IDLE watcher per account: a watcher is started for a - * newly-added account and cancelled when its account is removed (which promptly closes that - * account's IDLE connection). The service is started/stopped by the app based on whether any - * accounts exist, so reaching zero here is just a transient state. + * Observes the account list and the battery-derived [PushMode], and keeps one IDLE watcher per + * account while in [PushMode.IDLE]: a watcher is started for a newly-added account and cancelled + * when its account is removed (which promptly closes that account's IDLE connection). In + * [PushMode.POLLING] no watcher is wanted, so entering it cancels them all — cleanly closing the + * IDLE connections — and leaving it starts them again. The service is started/stopped by the app + * based on whether any accounts exist, so reaching zero here is just a transient state. */ private suspend fun reconcileWatchers() { - accountDao.observeAll().collect { entities -> - val accounts = entities.map { it.toDomain() } - val currentIds = accounts.mapTo(mutableSetOf()) { it.id } - (watchers.keys - currentIds).forEach { id -> watchers.remove(id)?.cancel() } - accounts.forEach { account -> + val pushModes = SyncResourcePolicy.pushModes(batteryStatusProvider.current(), batteryStatusProvider.status()) + combine(accountDao.observeAll(), pushModes) { entities, mode -> + entities.map { it.toDomain() } to mode + }.collect { (accounts, mode) -> + if (mode != shownMode) { + shownMode = mode + onPushModeChanged(mode) + } + val wanted = if (mode == PushMode.POLLING) emptyList() else accounts + val wantedIds = wanted.mapTo(mutableSetOf()) { it.id } + (watchers.keys - wantedIds).forEach { id -> watchers.remove(id)?.cancel() } + wanted.forEach { account -> if (account.id !in watchers) { watchers[account.id] = scope.launch { watchAccount(account) } } @@ -82,6 +113,19 @@ class IdleService : Service() { } } + /** Makes a push-mode change visible (#90): updates the persistent notification and the app log. */ + private fun onPushModeChanged(mode: PushMode) { + startAsForeground(mode) + if (mode == PushMode.POLLING) { + AppLog.i(TAG, "Battery low: pausing IMAP IDLE; mail arrives via 15-minute periodic sync") + // The periodic fallback is scheduled at every app start with KEEP, so this is normally a + // no-op — re-asserted here so the fallback provably exists whenever push is paused. + syncScheduler.schedulePeriodicSync() + } else { + AppLog.i(TAG, "Battery recovered: resuming IMAP IDLE push") + } + } + /** * Holds IDLE for one account, reconnecting with exponential backoff whenever it drops. * Each IDLE session is bounded by [IDLE_RENEWAL_MS]: when it elapses, [withTimeoutOrNull] @@ -117,7 +161,11 @@ class IdleService : Service() { override fun onBind(intent: Intent?): IBinder? = null - private fun startAsForeground() { + /** + * Starts (or, on later calls, updates) the foreground notification. The text tells the truth per + * [mode]: "connected for instant delivery" versus the low-battery 15-minute polling fallback. + */ + private fun startAsForeground(mode: PushMode) { NotificationManagerCompat.from(this).createNotificationChannel( NotificationChannel( CHANNEL_ID, @@ -125,10 +173,15 @@ class IdleService : Service() { NotificationManager.IMPORTANCE_LOW, ), ) + val text = if (mode == PushMode.POLLING) { + getString(R.string.notif_push_status_text_low_battery) + } else { + getString(R.string.notif_push_status_text) + } val notification = NotificationCompat.Builder(this, CHANNEL_ID) .setSmallIcon(R.drawable.ic_launcher_monochrome) .setContentTitle(getString(R.string.notif_push_status_title)) - .setContentText(getString(R.string.notif_push_status_text)) + .setContentText(text) .setOngoing(true) .setShowWhen(false) .setCategory(NotificationCompat.CATEGORY_SERVICE) diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index f2a44fa..0659f71 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -180,6 +180,7 @@ Push (IMAP IDLE) Watching for new mail Connected for instant delivery + Battery low — checking every 15 minutes until it recovers Accounts diff --git a/app/src/test/kotlin/org/libremail/data/settings/AppSettingsTest.kt b/app/src/test/kotlin/org/libremail/data/settings/AppSettingsTest.kt new file mode 100644 index 0000000..65d62dc --- /dev/null +++ b/app/src/test/kotlin/org/libremail/data/settings/AppSettingsTest.kt @@ -0,0 +1,40 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.settings + +import androidx.datastore.preferences.core.emptyPreferences +import androidx.datastore.preferences.core.preferencesOf +import androidx.datastore.preferences.core.stringPreferencesKey +import org.junit.Test +import kotlin.test.assertEquals + +/** + * Full-content prefetch must default to Wi-Fi-only (#88) — both for fresh installs (the [AppSettings] + * default) and for existing installs that never touched the setting (the DataStore-read fallback in + * [toAppSettings]); an explicit user choice always wins. + */ +class AppSettingsTest { + + private val fetchPolicyKey = stringPreferencesKey("fetch_policy") + + @Test + fun `in-memory default fetch policy is WIFI_ONLY`() { + assertEquals(FetchPolicy.WIFI_ONLY, AppSettings().fetchPolicy) + } + + @Test + fun `DataStore fallback for a never-set fetch policy is WIFI_ONLY`() { + assertEquals(FetchPolicy.WIFI_ONLY, emptyPreferences().toAppSettings().fetchPolicy) + } + + @Test + fun `an explicitly stored fetch policy is respected over the default`() { + val prefs = preferencesOf(fetchPolicyKey to FetchPolicy.ALWAYS.name) + assertEquals(FetchPolicy.ALWAYS, prefs.toAppSettings().fetchPolicy) + } + + @Test + fun `an unrecognized stored fetch policy falls back to WIFI_ONLY`() { + val prefs = preferencesOf(fetchPolicyKey to "NOT_A_POLICY") + assertEquals(FetchPolicy.WIFI_ONLY, prefs.toAppSettings().fetchPolicy) + } +} diff --git a/app/src/test/kotlin/org/libremail/data/sync/MailBackfillerTest.kt b/app/src/test/kotlin/org/libremail/data/sync/MailBackfillerTest.kt index d325e9d..c900ba4 100644 --- a/app/src/test/kotlin/org/libremail/data/sync/MailBackfillerTest.kt +++ b/app/src/test/kotlin/org/libremail/data/sync/MailBackfillerTest.kt @@ -35,6 +35,8 @@ import org.libremail.domain.model.ImapConnectionParams import org.libremail.domain.model.MailSecurity import org.libremail.domain.repository.MailRepository import org.libremail.mail.ImapClient +import org.libremail.power.BatteryStatus +import org.libremail.power.BatteryStatusProvider import java.util.Date import java.util.Properties import kotlin.test.assertEquals @@ -189,8 +191,42 @@ class MailBackfillerTest { assertNoDeletes() } + @Test + fun `low battery pauses the backfill content prefetch even for ALWAYS, but not header paging`() = runTest { + appendMessages(60) + seedForegroundWindow() + + backfiller( + AccountSettings("acct"), + fetchPolicy = FetchPolicy.ALWAYS, + battery = BatteryStatus(percent = 15, isCharging = false), + ).runBackfill() + + // #89 gates only the aggressive body/attachment prefetch; history headers keep paging in. + assertEquals(60, distinctCachedUids().size, "header paging itself is not battery-gated") + coVerify(exactly = 0) { requireNotNull(lastMailRepository).prefetchMessage(any()) } + } + + @Test + fun `charging at low percent keeps the backfill content prefetch running`() = runTest { + appendMessages(60) + seedForegroundWindow() + + backfiller( + AccountSettings("acct"), + fetchPolicy = FetchPolicy.ALWAYS, + battery = BatteryStatus(percent = 15, isCharging = true), + ).runBackfill() + + coVerify(atLeast = 1) { requireNotNull(lastMailRepository).prefetchMessage(any()) } + } + /** Builds a backfiller wired to GreenMail with the in-memory fakes and the given account settings. */ - private fun backfiller(accountSettings: AccountSettings): MailBackfiller { + private fun backfiller( + accountSettings: AccountSettings, + fetchPolicy: FetchPolicy = FetchPolicy.ON_DEMAND, + battery: BatteryStatus = BatteryStatus(percent = 100, isCharging = false), + ): MailBackfiller { val accountDao = mockk() coEvery { accountDao.getAll() } returns listOf(accountEntity) @@ -227,12 +263,15 @@ class MailBackfillerTest { coEvery { connectionFactory.imapParamsFor(any()) } returns params() val settingsRepository = mockk() - coEvery { settingsRepository.fetchPolicy() } returns FetchPolicy.ON_DEMAND + coEvery { settingsRepository.fetchPolicy() } returns fetchPolicy every { settingsRepository.settings } returns flowOf(AppSettings()) val accountSettingsRepository = mockk() coEvery { accountSettingsRepository.get("acct") } returns accountSettings + val batteryStatusProvider = mockk { every { current() } returns battery } + val mailRepository = mockk(relaxed = true) + return MailBackfiller( context = mockk(relaxed = true), accountDao = accountDao, @@ -242,12 +281,17 @@ class MailBackfillerTest { connectionFactory = connectionFactory, settingsRepository = settingsRepository, accountSettingsRepository = accountSettingsRepository, - mailRepository = mockk(relaxed = true), + batteryStatusProvider = batteryStatusProvider, + mailRepository = mailRepository, maintenanceGate = MailMaintenanceGate(), - ).also { lastMessageDao = messageDao } + ).also { + lastMessageDao = messageDao + lastMailRepository = mailRepository + } } private var lastMessageDao: MessageDao? = null + private var lastMailRepository: MailRepository? = null /** Seeds the in-memory cache with the newest [WINDOW] headers, mimicking a prior foreground sync. */ private suspend fun seedForegroundWindow() { diff --git a/app/src/test/kotlin/org/libremail/data/sync/MailMaintenanceGateTest.kt b/app/src/test/kotlin/org/libremail/data/sync/MailMaintenanceGateTest.kt index 340d135..ade809d 100644 --- a/app/src/test/kotlin/org/libremail/data/sync/MailMaintenanceGateTest.kt +++ b/app/src/test/kotlin/org/libremail/data/sync/MailMaintenanceGateTest.kt @@ -26,6 +26,8 @@ import org.libremail.domain.model.ImapConnectionParams import org.libremail.domain.model.MailSecurity import org.libremail.mail.FetchedMessage import org.libremail.mail.ImapClient +import org.libremail.power.BatteryStatus +import org.libremail.power.BatteryStatusProvider import java.util.Collections import java.util.concurrent.atomic.AtomicInteger import kotlin.test.assertEquals @@ -167,6 +169,9 @@ class MailMaintenanceGateTest { connectionFactory = connectionFactory, settingsRepository = settingsRepository, accountSettingsRepository = accountSettingsRepository, + batteryStatusProvider = mockk { + every { current() } returns BatteryStatus(percent = 100, isCharging = false) + }, mailRepository = mockk(relaxed = true), maintenanceGate = gate, ) diff --git a/app/src/test/kotlin/org/libremail/data/sync/MailSyncerTest.kt b/app/src/test/kotlin/org/libremail/data/sync/MailSyncerTest.kt index 186834c..1574f54 100644 --- a/app/src/test/kotlin/org/libremail/data/sync/MailSyncerTest.kt +++ b/app/src/test/kotlin/org/libremail/data/sync/MailSyncerTest.kt @@ -26,6 +26,9 @@ import org.libremail.domain.repository.MailRepository import org.libremail.mail.FetchedMessage import org.libremail.mail.ImapClient import org.libremail.notifications.MailNotifier +import org.libremail.power.BatteryStatus +import org.libremail.power.BatteryStatusProvider +import kotlin.test.assertEquals class MailSyncerTest { @@ -48,6 +51,7 @@ class MailSyncerTest { context: Context = mockk(relaxed = true), accountSettings: AccountSettings = AccountSettings("acct"), globalSettings: AppSettings = AppSettings(), + battery: BatteryStatus = BatteryStatus(percent = 100, isCharging = false), ): MailSyncer { val accountDao = mockk() coEvery { accountDao.getById("acct") } returns account @@ -72,11 +76,15 @@ class MailSyncerTest { connectionFactory = connectionFactory, settingsRepository = settingsRepository, accountSettingsRepository = accountSettingsRepository, + batteryStatusProvider = batteryProvider(battery), notifier = mockk(relaxed = true), mailRepository = mailRepository, ) } + private fun batteryProvider(battery: BatteryStatus): BatteryStatusProvider = + mockk { every { current() } returns battery } + @Test fun `ALWAYS policy prefetches unfetched messages after the header sync`() = runTest { val repo = mockk() @@ -146,6 +154,49 @@ class MailSyncerTest { coVerify(exactly = 0) { repo.prefetchMessage(any()) } } + @Test + fun `low battery pauses prefetch even for ALWAYS, leaving the header sync untouched`() = runTest { + val repo = mockk(relaxed = true) + val lowBattery = BatteryStatus(percent = 15, isCharging = false) + + val result = syncer(FetchPolicy.ALWAYS, repo, battery = lowBattery).syncFolder("acct", "INBOX") + + assertEquals(0, result.getOrNull()) // header sync still ran and succeeded + coVerify(exactly = 0) { repo.prefetchMessage(any()) } + } + + @Test + fun `at exactly the 20 percent threshold prefetch is paused`() = runTest { + val repo = mockk(relaxed = true) + val threshold = BatteryStatus(percent = 20, isCharging = false) + + syncer(FetchPolicy.ALWAYS, repo, battery = threshold).syncFolder("acct", "INBOX") + + coVerify(exactly = 0) { repo.prefetchMessage(any()) } + } + + @Test + fun `a low battery on the charger still prefetches`() = runTest { + val repo = mockk() + coEvery { repo.prefetchMessage(any()) } returns Result.success(Unit) + val chargingLow = BatteryStatus(percent = 15, isCharging = true) + + syncer(FetchPolicy.ALWAYS, repo, battery = chargingLow).syncFolder("acct", "INBOX") + + coVerify { repo.prefetchMessage("acct:INBOX:1") } + } + + @Test + fun `prefetch resumes on the next sync once battery recovers`() = runTest { + val repo = mockk() + coEvery { repo.prefetchMessage(any()) } returns Result.success(Unit) + val recovered = BatteryStatus(percent = 21, isCharging = false) + + syncer(FetchPolicy.ALWAYS, repo, battery = recovered).syncFolder("acct", "INBOX") + + coVerify { repo.prefetchMessage("acct:INBOX:1") } + } + @Test fun `notifies for new mail when both global and per-account notifications are enabled`() = runTest { val notifier = mockk(relaxed = true) @@ -200,6 +251,7 @@ class MailSyncerTest { connectionFactory = connectionFactory, settingsRepository = settingsRepository, accountSettingsRepository = accountSettingsRepository, + batteryStatusProvider = batteryProvider(BatteryStatus(percent = 100, isCharging = false)), notifier = notifier, mailRepository = mockk(relaxed = true), ) diff --git a/app/src/test/kotlin/org/libremail/data/sync/SyncResourcePolicyTest.kt b/app/src/test/kotlin/org/libremail/data/sync/SyncResourcePolicyTest.kt new file mode 100644 index 0000000..8d969ee --- /dev/null +++ b/app/src/test/kotlin/org/libremail/data/sync/SyncResourcePolicyTest.kt @@ -0,0 +1,168 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.sync + +import app.cash.turbine.test +import kotlinx.coroutines.flow.MutableSharedFlow +import kotlinx.coroutines.test.runTest +import org.junit.Test +import org.libremail.data.settings.FetchPolicy +import org.libremail.power.BatteryStatus +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +class SyncResourcePolicyTest { + + private fun battery(percent: Int, charging: Boolean = false) = BatteryStatus(percent, charging) + + private fun prefetch(policy: FetchPolicy, unmetered: Boolean = true, battery: BatteryStatus = battery(80)) = + SyncResourcePolicy.shouldPrefetchContent(policy, { unmetered }, battery) + + // ---- isBatteryLow: the shared threshold (#89/#90) ---- + + @Test + fun `battery is low at exactly 20 percent and below, but not at 21`() { + assertTrue(SyncResourcePolicy.isBatteryLow(battery(20))) + assertTrue(SyncResourcePolicy.isBatteryLow(battery(1))) + assertFalse(SyncResourcePolicy.isBatteryLow(battery(21))) + assertFalse(SyncResourcePolicy.isBatteryLow(battery(100))) + } + + @Test + fun `charging exempts from the low-battery state at any percent`() { + assertFalse(SyncResourcePolicy.isBatteryLow(battery(20, charging = true))) + assertFalse(SyncResourcePolicy.isBatteryLow(battery(1, charging = true))) + } + + // ---- shouldPrefetchContent: FetchPolicy x network x battery (#88/#89) ---- + + @Test + fun `healthy battery applies the fetch policy as-is`() { + assertTrue(prefetch(FetchPolicy.ALWAYS, unmetered = false)) + assertTrue(prefetch(FetchPolicy.WIFI_ONLY, unmetered = true)) + assertFalse(prefetch(FetchPolicy.WIFI_ONLY, unmetered = false)) + assertFalse(prefetch(FetchPolicy.ON_DEMAND, unmetered = true)) + } + + @Test + fun `low battery pauses prefetch for every policy`() { + val low = battery(20) + assertFalse(prefetch(FetchPolicy.ALWAYS, unmetered = true, battery = low)) + assertFalse(prefetch(FetchPolicy.WIFI_ONLY, unmetered = true, battery = low)) + assertFalse(prefetch(FetchPolicy.ON_DEMAND, unmetered = true, battery = low)) + } + + @Test + fun `prefetch resumes one percent above the threshold`() { + assertTrue(prefetch(FetchPolicy.ALWAYS, battery = battery(21))) + } + + @Test + fun `charging at low percent still prefetches`() { + assertTrue(prefetch(FetchPolicy.ALWAYS, battery = battery(10, charging = true))) + } + + @Test + fun `charging does not override the network gate`() { + assertFalse(prefetch(FetchPolicy.WIFI_ONLY, unmetered = false, battery = battery(10, charging = true))) + } + + @Test + fun `network state is only consulted for WIFI_ONLY on a healthy battery`() { + val forbidden: () -> Boolean = { error("network must not be consulted") } + assertTrue(SyncResourcePolicy.shouldPrefetchContent(FetchPolicy.ALWAYS, forbidden, battery(80))) + assertFalse(SyncResourcePolicy.shouldPrefetchContent(FetchPolicy.ON_DEMAND, forbidden, battery(80))) + // The battery gate short-circuits even the WIFI_ONLY network check. + assertFalse(SyncResourcePolicy.shouldPrefetchContent(FetchPolicy.WIFI_ONLY, forbidden, battery(10))) + } + + // ---- pushMode: IDLE vs POLLING with hysteresis (#90) ---- + + @Test + fun `push drops to polling at exactly 20 percent`() { + assertEquals(PushMode.POLLING, SyncResourcePolicy.pushMode(battery(20), previous = PushMode.IDLE)) + assertEquals(PushMode.POLLING, SyncResourcePolicy.pushMode(battery(5), previous = PushMode.IDLE)) + } + + @Test + fun `push stays on idle above the threshold`() { + assertEquals(PushMode.IDLE, SyncResourcePolicy.pushMode(battery(21), previous = PushMode.IDLE)) + } + + @Test + fun `inside the hysteresis band the previous mode is kept`() { + for (percent in 21 until SyncResourcePolicy.PUSH_RESUME_PERCENT) { + assertEquals(PushMode.POLLING, SyncResourcePolicy.pushMode(battery(percent), previous = PushMode.POLLING)) + assertEquals(PushMode.IDLE, SyncResourcePolicy.pushMode(battery(percent), previous = PushMode.IDLE)) + } + } + + @Test + fun `push resumes idle at the recovery threshold`() { + assertEquals( + PushMode.IDLE, + SyncResourcePolicy.pushMode(battery(SyncResourcePolicy.PUSH_RESUME_PERCENT), previous = PushMode.POLLING), + ) + } + + @Test + fun `plugging in resumes idle immediately at any percent`() { + val plugged = battery(5, charging = true) + assertEquals(PushMode.IDLE, SyncResourcePolicy.pushMode(plugged, previous = PushMode.POLLING)) + } + + // ---- pushModes: the folded stream the IDLE service consumes (#90) ---- + + @Test + fun `stream starts in idle on a healthy battery and drops to polling at the threshold`() = runTest { + val updates = MutableSharedFlow() + SyncResourcePolicy.pushModes(initial = battery(50), updates = updates).test { + assertEquals(PushMode.IDLE, awaitItem()) + updates.emit(battery(20)) + assertEquals(PushMode.POLLING, awaitItem()) + cancelAndIgnoreRemainingEvents() + } + } + + @Test + fun `stream starts in polling when collection begins on an already-low battery`() = runTest { + val updates = MutableSharedFlow() + SyncResourcePolicy.pushModes(initial = battery(12), updates = updates).test { + assertEquals(PushMode.POLLING, awaitItem()) + cancelAndIgnoreRemainingEvents() + } + } + + @Test + fun `jitter around the threshold does not flap the stream`() = runTest { + val updates = MutableSharedFlow() + SyncResourcePolicy.pushModes(initial = battery(50), updates = updates).test { + assertEquals(PushMode.IDLE, awaitItem()) + updates.emit(battery(20)) + assertEquals(PushMode.POLLING, awaitItem()) + // Bouncing 21 <-> 20 <-> 24 stays inside the hysteresis band: nothing is emitted. + updates.emit(battery(21)) + updates.emit(battery(20)) + updates.emit(battery(24)) + expectNoEvents() + // Only reaching the recovery threshold flips back. + updates.emit(battery(SyncResourcePolicy.PUSH_RESUME_PERCENT)) + assertEquals(PushMode.IDLE, awaitItem()) + cancelAndIgnoreRemainingEvents() + } + } + + @Test + fun `plugging in while polling resumes idle without waiting for the recovery threshold`() = runTest { + val updates = MutableSharedFlow() + SyncResourcePolicy.pushModes(initial = battery(10), updates = updates).test { + assertEquals(PushMode.POLLING, awaitItem()) + updates.emit(battery(10, charging = true)) + assertEquals(PushMode.IDLE, awaitItem()) + // Unplugging below the threshold drops straight back to polling. + updates.emit(battery(10)) + assertEquals(PushMode.POLLING, awaitItem()) + cancelAndIgnoreRemainingEvents() + } + } +}