Merge branch 'main' into perf-322-batched-persistbatch
This commit is contained in:
+90
-25
@@ -6,9 +6,17 @@
|
||||
# Spec: docs/ci/mergify-integration-spec.md + docs/ci/mergify.yml.proposed
|
||||
# (issues #407 / #408).
|
||||
# Schema: https://docs.mergify.com/configuration/file-format/
|
||||
# Verified against the LIVE Mergify docs on 2026-07-06 (queue rules,
|
||||
# the queue action, priority rules, parallel checks, batches, setup and
|
||||
# lifecycle pages) — the config format evolves, so this is not from memory.
|
||||
# Verified against the LIVE Mergify docs on 2026-07-07 (file-format, queue
|
||||
# rules, priority, merge-queue lifecycle/setup/batches, and the
|
||||
# merge-protections auto-merge pages) — the config format evolves, so this is
|
||||
# not from memory.
|
||||
# 2026-07-07 CHANGE: auto-queueing migrated OFF the `pull_request_rules`
|
||||
# queue-action path (which no longer auto-queues — a green matching PR just
|
||||
# reported "Merge queue is ready — use `@Mergifyio queue`" and sat there) ONTO
|
||||
# `merge_protections_settings.auto_merge_conditions` (see that block below).
|
||||
# The old `autoqueue`/queue-action auto path is DEPRECATED and "will stop
|
||||
# working on 2026-07-16" (docs.mergify.com/merge-queue/rules). This changes only
|
||||
# the TRIGGER; the queue's merge semantics (below) are untouched.
|
||||
# ============================================================================
|
||||
#
|
||||
# WHAT THIS DOES
|
||||
@@ -31,6 +39,23 @@
|
||||
# * The single required status check stays "CI passed" — the exact `name:` of the
|
||||
# `ci-passed` job in .github/workflows/ci.yml. NOT "ci-passed". A wrong name means
|
||||
# PRs queue but never merge.
|
||||
# * IN-PLACE CHECKS, not speculative draft-PR checks. GitHub's strict
|
||||
# `required_status_checks` ruleset (require-branches-up-to-date) rejects
|
||||
# speculative checks outright — Mergify surfaced this as a "Configuration not
|
||||
# compatible with `required_status_checks` ruleset rule" check on #422. The fix
|
||||
# (per Mergify: docs.mergify.com/merge-queue/rules) is to make Mergify validate
|
||||
# each PR IN PLACE, on the real PR branch, which requires ALL THREE of:
|
||||
# (a) `merge_queue.max_parallel_checks: 1` (below),
|
||||
# (b) every `queue_rules[].batch_size: 1` (below), and
|
||||
# (c) `queue_rules.default.queue_conditions` IDENTICAL (same conditions, same
|
||||
# order) to `queue_rules.default.merge_conditions` — i.e. no "two-step CI"
|
||||
# where the conditions to ENTER the queue differ from the conditions to
|
||||
# MERGE. Mergify runs three condition sets, sequentially:
|
||||
# `merge_protections_settings.auto_merge_conditions` (TRIGGERS auto-queueing)
|
||||
# → `queue_conditions` (validates a PR's queue ENTRY) → `merge_conditions`
|
||||
# (validates the MERGE). Omitting `queue_conditions` — as this config first
|
||||
# did — reads as a two-step-CI mismatch and re-trips the incompatibility
|
||||
# check, so we keep all three lists identical. Do not let them drift apart.
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# queue_rules — how a queued PR is validated and merged.
|
||||
@@ -38,16 +63,35 @@
|
||||
queue_rules:
|
||||
- name: default
|
||||
# Final merge gate. Merge ONLY when the single required context is green (the exact
|
||||
# same check branch protection requires), the PR is not a draft, has no merge
|
||||
# conflicts, and is not flagged `broken`. NOTE: branch protection requires 0
|
||||
# same check branch protection requires), the PR targets `main`, is not a draft, has
|
||||
# no merge conflicts, and is not flagged `broken`. NOTE: branch protection requires 0
|
||||
# approvals here (the active repository ruleset sets required_approving_review_count
|
||||
# = 0), so there is deliberately NO `#approved-reviews-by` condition — adding one
|
||||
# would wedge the solo-maintainer flow, where nobody can approve their own PR.
|
||||
merge_conditions:
|
||||
- check-success = CI passed
|
||||
#
|
||||
# IN-PLACE CHECKS: `queue_conditions` (what a PR must satisfy to ENTER/stay in the
|
||||
# queue) MUST be IDENTICAL (same conditions, same order) to `merge_conditions` (what
|
||||
# it must satisfy to MERGE) below. When those two lists match — plus batch_size 1 and
|
||||
# max_parallel_checks 1 — Mergify validates each PR IN PLACE on the real PR branch
|
||||
# instead of running speculative draft-PR checks, which is what GitHub's strict
|
||||
# `required_status_checks` ruleset (require-branches-up-to-date) demands. Omitting
|
||||
# `queue_conditions` (as this config originally did) is treated as a "two-step CI"
|
||||
# mismatch and Mergify flags the ruleset as incompatible. Keep the three lists here —
|
||||
# `queue_conditions`, `merge_conditions`, and
|
||||
# `merge_protections_settings.auto_merge_conditions` — all identical; if any diverge,
|
||||
# Mergify's ruleset-compatibility check fails again.
|
||||
queue_conditions:
|
||||
- base = main
|
||||
- -draft
|
||||
- -conflict
|
||||
- label != broken
|
||||
- check-success = CI passed
|
||||
merge_conditions:
|
||||
- base = main
|
||||
- -draft
|
||||
- -conflict
|
||||
- label != broken
|
||||
- check-success = CI passed
|
||||
# SERIAL: exactly one PR per merge. No batching (Phase 2 / #410). One merge commit
|
||||
# per PR, which is what lets require-up-to-date stay literally ON.
|
||||
batch_size: 1
|
||||
@@ -125,22 +169,43 @@ priority_rules:
|
||||
priority: 1000
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# pull_request_rules — WHICH PRs enter the queue.
|
||||
# The `queue` action is what actually ADDS a PR to the merge queue: per
|
||||
# docs.mergify.com/merge-queue/lifecycle, queue_conditions alone do NOT auto-queue a PR —
|
||||
# a queue action (or an `@mergifyio queue` command / auto_merge) is required, otherwise the
|
||||
# "Mergify Merge Queue" check sits permanently pending. A PR is queued as soon as it is green
|
||||
# on "CI passed", targets `main`, is not a draft, has no conflicts, and is not flagged
|
||||
# `broken`. `broken` / `draft` PRs are never queued.
|
||||
# merge_protections_settings — WHICH PRs are AUTOMATICALLY added to the queue.
|
||||
#
|
||||
# This REPLACES the old `pull_request_rules` `queue` action. That action no longer
|
||||
# auto-queues in current Mergify: a green, matching PR just reported "Merge queue is
|
||||
# ready — use `@Mergifyio queue`" and sat there forever (never merged). Automatic
|
||||
# queueing now lives in `auto_merge_conditions` under `merge_protections_settings`. The
|
||||
# old `queue_rules[].autoqueue` field (and the queue-action auto path) is DEPRECATED and
|
||||
# "will stop working on 2026-07-16. Use `auto_merge_conditions` in
|
||||
# `merge_protections_settings` instead" (docs.mergify.com/merge-queue/rules).
|
||||
#
|
||||
# `auto_merge_conditions` accepts `true` (auto-queue every mergeable PR) or, as here,
|
||||
# "a list of conditions to restrict the audience" (docs.mergify.com/configuration/
|
||||
# file-format). We give the SAME set the old queue action used, so EXACTLY the same PRs
|
||||
# auto-queue: green on "CI passed", targeting `main`, not a draft, no conflicts, not
|
||||
# `broken`.
|
||||
#
|
||||
# WHY THIS PRESERVES require-up-to-date: this changes only the TRIGGER (manual →
|
||||
# automatic). It does NOT touch how the queue validates or merges — batch_size 1,
|
||||
# merge_method merge, and max_parallel_checks 1 above are unchanged — and those are the
|
||||
# settings that interact with require-up-to-date (only BATCHING, batch_size > 1, forces
|
||||
# that checkbox OFF; see the invariants header + docs.mergify.com/merge-queue/batches).
|
||||
# When a merge queue is configured, a matched PR is auto-QUEUED, not merged directly:
|
||||
# "Every PR is auto-queued. The merge queue then handles routing and merging"
|
||||
# (docs.mergify.com/merge-protections/auto-merge) — so it still goes through the serial
|
||||
# queue, gets updated onto the latest `main`, re-runs CI, and merges on the real green
|
||||
# "CI passed". Mergify also auto-reads GitHub branch protection (the required "CI passed"
|
||||
# check + require-up-to-date) and injects it as a merge condition, so the GitHub gate is
|
||||
# enforced on top of queue_rules.merge_conditions.
|
||||
#
|
||||
# This list MUST stay IDENTICAL (same conditions, same order) to
|
||||
# `queue_rules.default.queue_conditions` and `.merge_conditions` above — see the note
|
||||
# there and the IN-PLACE CHECKS hard invariant at the top of this file.
|
||||
# ---------------------------------------------------------------------------
|
||||
pull_request_rules:
|
||||
- name: Queue green, non-draft, non-conflicting PRs targeting main
|
||||
conditions:
|
||||
- base = main
|
||||
- -draft
|
||||
- -conflict
|
||||
- label != broken
|
||||
- check-success = CI passed
|
||||
actions:
|
||||
queue:
|
||||
name: default
|
||||
merge_protections_settings:
|
||||
auto_merge_conditions:
|
||||
- base = main
|
||||
- -draft
|
||||
- -conflict
|
||||
- label != broken
|
||||
- check-success = CI passed
|
||||
|
||||
+64
@@ -0,0 +1,64 @@
|
||||
// SPDX-License-Identifier: GPL-3.0-or-later
|
||||
package org.libremail.data.local
|
||||
|
||||
import android.content.Context
|
||||
import androidx.test.core.app.ApplicationProvider
|
||||
import androidx.test.ext.junit.runners.AndroidJUnit4
|
||||
import org.junit.After
|
||||
import org.junit.Assert.assertFalse
|
||||
import org.junit.Before
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import java.io.File
|
||||
|
||||
/**
|
||||
* On-device cover for the issue-#359 keyed-open probe ([DatabaseEncryption.probeKeyedOpen]). The probe
|
||||
* is the fix for gap 2: it reaches the REAL `SQLiteConnection.nativeOpen` — the exact #359 crash site —
|
||||
* from inside `DatabaseProvisioner`'s fail-closed handler, so an open-time `UnsatisfiedLinkError` on an
|
||||
* incompatible device is caught and converted to `CacheEncryptionUnavailableException` before Room's
|
||||
* later deferred open can crash on it uncaught.
|
||||
*
|
||||
* This runs the real SQLCipher native path (mocked out of the JVM unit tests), asserting two things a
|
||||
* device is needed for: on a compatible device the probe opens+closes the keyed database WITHOUT
|
||||
* throwing, and — on every device — it leaves no file behind (it opens a throwaway sibling, never the
|
||||
* real cache, and cleans up its sidecars). All data is synthetic; nothing here is PII.
|
||||
*/
|
||||
@RunWith(AndroidJUnit4::class)
|
||||
class DatabaseEncryptionProbeInstrumentedTest {
|
||||
|
||||
private val context = ApplicationProvider.getApplicationContext<Context>()
|
||||
private val cacheName = "probe_test_cache.db"
|
||||
private val cacheFile: File get() = context.getDatabasePath(cacheName)
|
||||
|
||||
// 64 hex chars == a 32-byte SQLCipher passphrase, matching DatabaseKeyStore's format.
|
||||
private val passphrase = "0123456789abcdef".repeat(4)
|
||||
|
||||
@Before
|
||||
@After
|
||||
fun clean() {
|
||||
cacheFile.parentFile
|
||||
?.listFiles { f -> f.name.startsWith(cacheName) }
|
||||
?.forEach { it.delete() }
|
||||
}
|
||||
|
||||
@Test
|
||||
fun probeKeyedOpen_reachesNativeOpen_thenLeavesNoFileBehind() {
|
||||
// The provisioner loads the native library immediately before probing (probeKeyedOpen's
|
||||
// precondition, kept out of the probe so the load happens exactly once per open); mirror that here.
|
||||
DatabaseEncryption.ensureNativeLibraryLoaded()
|
||||
// On a compatible device this returns normally (keyed nativeOpen succeeds); on an incompatible one
|
||||
// it throws UnsatisfiedLinkError here — the exact #359 signature the provisioner now fails closed
|
||||
// on. Either way, no probe file may survive.
|
||||
DatabaseEncryption.probeKeyedOpen(cacheFile, passphrase)
|
||||
|
||||
val dir = cacheFile.parentFile!!
|
||||
listOf("", "-wal", "-shm", "-journal").forEach { suffix ->
|
||||
assertFalse(
|
||||
"probe left a '$cacheName.openprobe$suffix' file behind",
|
||||
File(dir, "$cacheName.openprobe$suffix").exists(),
|
||||
)
|
||||
}
|
||||
// The probe uses a throwaway sibling, so it must never create the real cache file.
|
||||
assertFalse("probe must not create the real cache file", cacheFile.exists())
|
||||
}
|
||||
}
|
||||
+144
@@ -0,0 +1,144 @@
|
||||
// SPDX-License-Identifier: GPL-3.0-or-later
|
||||
package org.libremail.data.repository
|
||||
|
||||
import android.content.Context
|
||||
import androidx.room.Room
|
||||
import androidx.test.core.app.ApplicationProvider
|
||||
import androidx.test.ext.junit.runners.AndroidJUnit4
|
||||
import io.mockk.coEvery
|
||||
import io.mockk.mockk
|
||||
import io.mockk.unmockkAll
|
||||
import kotlinx.coroutines.CompletableDeferred
|
||||
import kotlinx.coroutines.Dispatchers
|
||||
import kotlinx.coroutines.launch
|
||||
import kotlinx.coroutines.runBlocking
|
||||
import kotlinx.coroutines.withTimeout
|
||||
import org.junit.After
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Before
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.libremail.data.attachment.AttachmentUriGrants
|
||||
import org.libremail.data.local.AccountDatabase
|
||||
import org.libremail.data.local.dao.BackfillProgressDao
|
||||
import org.libremail.data.local.dao.DraftDao
|
||||
import org.libremail.data.local.dao.FolderDao
|
||||
import org.libremail.data.local.dao.MessageDao
|
||||
import org.libremail.data.security.CredentialStore
|
||||
import org.libremail.data.security.KeystoreCrypto
|
||||
import org.libremail.data.settings.AccountSettingsRepository
|
||||
import org.libremail.data.sync.SyncScheduler
|
||||
import org.libremail.domain.model.Account
|
||||
import org.libremail.domain.model.AuthType
|
||||
import org.libremail.domain.model.MailSecurity
|
||||
import org.libremail.domain.model.ServerConfig
|
||||
import org.libremail.mail.FetchedFolder
|
||||
import org.libremail.mail.ImapClient
|
||||
import org.libremail.notifications.MailNotifier
|
||||
|
||||
/**
|
||||
* On-device proof of the #403 fix against real SQLite and the real Keystore-backed [CredentialStore]:
|
||||
* when [AccountRepositoryImpl] adds an account, its credential must be resolvable the instant the new
|
||||
* account row becomes observable — the exact moment the push watchers (LibreMailApplication's collector
|
||||
* and [org.libremail.push.IdleService.reconcileWatchers], both keyed on the *accounts* table) react.
|
||||
*
|
||||
* The JVM `AccountRepositoryImplTest` pins the call ORDER with `coVerifyOrder`; this drives the same
|
||||
* production code against a real in-memory [AccountDatabase] (real `accountDao` + real `credentialDao`
|
||||
* via a real [CredentialStore]/[KeystoreCrypto]) so the transaction-commit ordering — not just the call
|
||||
* ordering — is exercised. A background collector reads the credential the moment the account first
|
||||
* appears, reproducing the reactive watcher; with the secret committed before the row it always
|
||||
* resolves. Non-DB collaborators (IMAP, scheduler, notifier, settings) are mocked the same way
|
||||
* `WorkerCacheLockDeferralInstrumentedTest` fakes its non-framework collaborators — never a framework
|
||||
* `Context`, which is the real application context.
|
||||
*/
|
||||
@RunWith(AndroidJUnit4::class)
|
||||
class AccountAddCredentialOrderingInstrumentedTest {
|
||||
|
||||
private val context: Context = ApplicationProvider.getApplicationContext()
|
||||
|
||||
private lateinit var db: AccountDatabase
|
||||
private lateinit var credentialStore: CredentialStore
|
||||
private lateinit var imapClient: ImapClient
|
||||
private lateinit var repository: AccountRepositoryImpl
|
||||
|
||||
@Before
|
||||
fun setUp() {
|
||||
db = Room.inMemoryDatabaseBuilder(context, AccountDatabase::class.java).build()
|
||||
credentialStore = CredentialStore(KeystoreCrypto(), db.credentialDao())
|
||||
imapClient = mockk()
|
||||
coEvery { imapClient.listFolders(any()) } returns listOf(
|
||||
FetchedFolder("INBOX", "INBOX", emptyList(), selectable = true),
|
||||
)
|
||||
repository = AccountRepositoryImpl(
|
||||
context = context,
|
||||
accountDao = db.accountDao(),
|
||||
messageDao = mockk<MessageDao>(relaxed = true),
|
||||
folderDao = mockk<FolderDao>(relaxed = true),
|
||||
backfillProgressDao = mockk<BackfillProgressDao>(relaxed = true),
|
||||
draftDao = mockk<DraftDao>(relaxed = true),
|
||||
credentialStore = credentialStore,
|
||||
imapClient = imapClient,
|
||||
syncScheduler = mockk<SyncScheduler>(relaxed = true),
|
||||
accountSettingsRepository = mockk<AccountSettingsRepository>(relaxed = true),
|
||||
mailNotifier = mockk<MailNotifier>(relaxed = true),
|
||||
attachmentUriGrants = mockk<AttachmentUriGrants>(relaxed = true),
|
||||
)
|
||||
}
|
||||
|
||||
@After
|
||||
fun tearDown() {
|
||||
db.close()
|
||||
unmockkAll()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun newAccountRowIsObservableOnlyAfterItsCredentialIsResolvable() = runBlocking {
|
||||
val account = imapAccount()
|
||||
|
||||
// A reactive watcher: read the stored secret the moment the new account row first appears in the
|
||||
// observed accounts list — exactly what IdleService.reconcileWatchers does before opening IDLE.
|
||||
val credentialAtFirstSight = CompletableDeferred<String?>()
|
||||
val observer = launch(Dispatchers.IO) {
|
||||
repository.observeAccounts().collect { accounts ->
|
||||
if (accounts.any { it.id == account.id } && !credentialAtFirstSight.isCompleted) {
|
||||
credentialAtFirstSight.complete(credentialStore.loadSecret(account.id))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
repository.addImapAccount(account, PASSWORD).getOrThrow()
|
||||
|
||||
assertEquals(
|
||||
"the credential must be resolvable the instant the account row becomes observable (#403)",
|
||||
PASSWORD,
|
||||
withTimeout(TIMEOUT_MS) { credentialAtFirstSight.await() },
|
||||
)
|
||||
observer.cancel()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun addImapAccountLeavesTheCredentialResolvableForTheStoredAccount() = runBlocking {
|
||||
val account = imapAccount()
|
||||
|
||||
repository.addImapAccount(account, PASSWORD).getOrThrow()
|
||||
|
||||
// Durability backstop: the account is persisted AND its secret round-trips through the real
|
||||
// Keystore-sealed store, so any later push (re)start resolves it rather than hitting a hard miss.
|
||||
assertEquals(PASSWORD, credentialStore.loadSecret(account.id))
|
||||
assertEquals(account.id, db.accountDao().getById(account.id)?.id)
|
||||
}
|
||||
|
||||
private fun imapAccount(id: String = "imap:ada@example.org") = Account(
|
||||
id = id,
|
||||
email = "ada@example.org",
|
||||
displayName = "Ada",
|
||||
authType = AuthType.PASSWORD_IMAP,
|
||||
imap = ServerConfig("imap.example.org", 993, MailSecurity.SSL_TLS),
|
||||
smtp = ServerConfig("smtp.example.org", 587, MailSecurity.STARTTLS),
|
||||
)
|
||||
|
||||
private companion object {
|
||||
const val PASSWORD = "app-password"
|
||||
const val TIMEOUT_MS = 5_000L
|
||||
}
|
||||
}
|
||||
@@ -134,6 +134,20 @@ class OnboardingFlowTest {
|
||||
navController.navigate(Routes.onboardingAppPassword(provider.key))
|
||||
},
|
||||
onManualSetup = {},
|
||||
onPickOutlook = { navController.navigate(Routes.ONBOARDING_OUTLOOK_IMAP) },
|
||||
viewModel = viewModel,
|
||||
)
|
||||
}
|
||||
composable(Routes.ONBOARDING_OUTLOOK_IMAP) {
|
||||
val viewModel = remember { AccountSetupViewModel(outlookAuthManager, accountRepo) }
|
||||
OutlookImapNoticeScreen(
|
||||
onBack = { navController.popBackStack() },
|
||||
onAccountAdded = { id ->
|
||||
onboarding.onAccountAdded(id)
|
||||
navController.navigate(Routes.ONBOARDING_ADD_ANOTHER) {
|
||||
popUpTo(Routes.ONBOARDING_PICKER)
|
||||
}
|
||||
},
|
||||
viewModel = viewModel,
|
||||
)
|
||||
}
|
||||
@@ -252,6 +266,23 @@ class OnboardingFlowTest {
|
||||
composeTestRule.onNodeWithText("E2E first message").assertIsDisplayed()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun outlookPick_showsImapNoticeBeforeAuth() {
|
||||
setOnboardingContent(FakeAccountRepository(), FakeMailRepository())
|
||||
|
||||
// Welcome → picker → tap Outlook.
|
||||
composeTestRule.onNodeWithText(string(R.string.onboarding_add_account)).performClick()
|
||||
waitForText(string(R.string.account_setup_outlook))
|
||||
composeTestRule.onNodeWithText(string(R.string.account_setup_outlook)).performClick()
|
||||
|
||||
// Picking Outlook lands on the IMAP-enablement notice BEFORE any OAuth browser opens (#411):
|
||||
// the interstitial's question is shown and its bottom "Sign in" button (which would continue
|
||||
// the existing Outlook auth flow) is present.
|
||||
waitForText(string(R.string.outlook_imap_question))
|
||||
composeTestRule.onNodeWithText(string(R.string.outlook_imap_question)).assertIsDisplayed()
|
||||
composeTestRule.onNodeWithText(string(R.string.outlook_imap_sign_in)).performScrollTo().assertIsDisplayed()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun yahooSetup_hasNoTwoFactorHelpLink() {
|
||||
setOnboardingContent(FakeAccountRepository(), FakeMailRepository())
|
||||
|
||||
+121
@@ -0,0 +1,121 @@
|
||||
// SPDX-License-Identifier: GPL-3.0-or-later
|
||||
package org.libremail.ui.onboarding
|
||||
|
||||
import android.app.Activity
|
||||
import android.app.Instrumentation
|
||||
import android.content.Context
|
||||
import android.content.Intent
|
||||
import androidx.activity.ComponentActivity
|
||||
import androidx.compose.ui.test.assertIsDisplayed
|
||||
import androidx.compose.ui.test.junit4.createAndroidComposeRule
|
||||
import androidx.compose.ui.test.onNodeWithText
|
||||
import androidx.compose.ui.test.performClick
|
||||
import androidx.compose.ui.test.performScrollTo
|
||||
import androidx.test.espresso.intent.Intents
|
||||
import androidx.test.espresso.intent.matcher.IntentMatchers.hasAction
|
||||
import androidx.test.espresso.intent.matcher.IntentMatchers.hasComponent
|
||||
import androidx.test.espresso.intent.matcher.IntentMatchers.hasData
|
||||
import androidx.test.espresso.intent.matcher.UriMatchers.hasHost
|
||||
import androidx.test.ext.junit.runners.AndroidJUnit4
|
||||
import androidx.test.platform.app.InstrumentationRegistry
|
||||
import net.openid.appauth.AuthorizationManagementActivity
|
||||
import org.hamcrest.CoreMatchers.allOf
|
||||
import org.hamcrest.CoreMatchers.equalTo
|
||||
import org.junit.Rule
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.libremail.R
|
||||
import org.libremail.auth.OutlookAuthManager
|
||||
import org.libremail.ui.FakeAccountRepository
|
||||
import org.libremail.ui.accountsetup.AccountSetupViewModel
|
||||
import org.libremail.ui.theme.LibreMailTheme
|
||||
|
||||
/**
|
||||
* End-to-end UI test for the pre-auth Outlook IMAP-enablement notice (#411). Drives the real
|
||||
* [OutlookImapNoticeScreen] + [AccountSetupViewModel] over a [FakeAccountRepository]: the IMAP
|
||||
* question and both outbound links render, tapping a help link fires the browser `ACTION_VIEW`
|
||||
* intent, and tapping the bottom "Sign in" button starts the existing Microsoft OAuth (AppAuth)
|
||||
* flow. Both launches are asserted with Espresso-Intents (mirroring `AccountPickerScreenTest`), so no
|
||||
* real browser ever opens; the interstitial → OAuth navigation in the full onboarding graph is
|
||||
* covered by `OnboardingFlowTest`.
|
||||
*/
|
||||
@RunWith(AndroidJUnit4::class)
|
||||
class OutlookImapNoticeScreenTest {
|
||||
|
||||
@get:Rule
|
||||
val composeTestRule = createAndroidComposeRule<ComponentActivity>()
|
||||
|
||||
private val context: Context =
|
||||
InstrumentationRegistry.getInstrumentation().targetContext.applicationContext
|
||||
|
||||
private fun string(resId: Int) = composeTestRule.activity.getString(resId)
|
||||
|
||||
private fun setContent(onAccountAdded: (String) -> Unit = {}) {
|
||||
val viewModel = AccountSetupViewModel(OutlookAuthManager(context), FakeAccountRepository())
|
||||
composeTestRule.setContent {
|
||||
LibreMailTheme(darkTheme = false, dynamicColor = false) {
|
||||
OutlookImapNoticeScreen(onBack = {}, onAccountAdded = onAccountAdded, viewModel = viewModel)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun showsImapQuestion_bothLinks_andSignIn() {
|
||||
setContent()
|
||||
|
||||
composeTestRule.onNodeWithText(string(R.string.outlook_imap_question)).performScrollTo().assertIsDisplayed()
|
||||
composeTestRule.onNodeWithText(string(R.string.outlook_imap_help)).performScrollTo().assertIsDisplayed()
|
||||
composeTestRule.onNodeWithText(string(R.string.outlook_imap_settings)).performScrollTo().assertIsDisplayed()
|
||||
composeTestRule.onNodeWithText(string(R.string.outlook_imap_sign_in)).performScrollTo().assertIsDisplayed()
|
||||
}
|
||||
|
||||
/**
|
||||
* Tapping the "How to enable IMAP" link opens Microsoft's help article via
|
||||
* [androidx.compose.ui.platform.UriHandler], which starts an `ACTION_VIEW` intent. Stubbing that
|
||||
* intent both proves the tap launched it and stops a real browser from opening on the device.
|
||||
*/
|
||||
@Test
|
||||
fun tappingImapHelpLink_opensTheMicrosoftArticle() {
|
||||
setContent()
|
||||
|
||||
Intents.init()
|
||||
try {
|
||||
Intents.intending(hasAction(Intent.ACTION_VIEW))
|
||||
.respondWith(Instrumentation.ActivityResult(Activity.RESULT_CANCELED, null))
|
||||
|
||||
composeTestRule.onNodeWithText(string(R.string.outlook_imap_help))
|
||||
.performScrollTo()
|
||||
.performClick()
|
||||
|
||||
Intents.intended(allOf(hasAction(Intent.ACTION_VIEW), hasData(hasHost(equalTo("support.microsoft.com")))))
|
||||
} finally {
|
||||
Intents.release()
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Tapping "Sign in" must continue the existing Outlook OAuth flow — i.e. fire the AppAuth
|
||||
* authorization intent (mirroring `AccountPickerScreenTest.tappingOutlook_...`). AppAuth wraps its
|
||||
* browser launch in an intent targeting [AuthorizationManagementActivity]; that component name is
|
||||
* the guaranteed, browser-independent signature of the launch. Stubbing a canceled result stops
|
||||
* that activity from ever resuming and opening a real browser.
|
||||
*/
|
||||
@Test
|
||||
fun tappingSignIn_launchesTheAppAuthBrowserIntent() {
|
||||
setContent()
|
||||
|
||||
Intents.init()
|
||||
try {
|
||||
Intents.intending(hasComponent(AuthorizationManagementActivity::class.java.name))
|
||||
.respondWith(Instrumentation.ActivityResult(Activity.RESULT_CANCELED, null))
|
||||
|
||||
composeTestRule.onNodeWithText(string(R.string.outlook_imap_sign_in))
|
||||
.performScrollTo()
|
||||
.performClick()
|
||||
|
||||
Intents.intended(hasComponent(AuthorizationManagementActivity::class.java.name))
|
||||
} finally {
|
||||
Intents.release()
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -22,3 +22,19 @@ class CacheEncryptionUnavailableException(cause: Throwable) :
|
||||
"Encrypted cache unavailable: the SQLCipher native library failed to load on this device",
|
||||
cause,
|
||||
)
|
||||
|
||||
/**
|
||||
* True when this throwable (or anything in its cause chain) signals that SQLCipher's native library is
|
||||
* unavailable on this device (issue #359): a [CacheEncryptionUnavailableException] the provisioner raised
|
||||
* when it failed closed, or — defensively — a bare [LinkageError] (an open-time `UnsatisfiedLinkError` at
|
||||
* `SQLiteConnection.nativeOpen` that reached a headless entry point before the provisioner wrapped it).
|
||||
*
|
||||
* Headless entry points that inject the Room cache directly — the WorkManager workers and `IdleService` —
|
||||
* have no UI gate (that is `CacheEncryptionGate`'s job), so they consult this to treat such a failure as a
|
||||
* soft defer/skip (a worker retries; the push service stops) instead of crashing. A later launch may load
|
||||
* the library and recover, so deferring rather than failing hard is correct. The whole cause chain is
|
||||
* walked because coroutine stack-trace recovery can re-wrap the throwable as it crosses the database-open
|
||||
* boundary (the same reason [DatabaseProvisioner]'s own tests assert on the cause chain, not the instance).
|
||||
*/
|
||||
fun Throwable.isCacheEncryptionUnavailable(): Boolean = generateSequence(this) { it.cause }
|
||||
.any { it is CacheEncryptionUnavailableException || it is LinkageError }
|
||||
|
||||
@@ -115,8 +115,45 @@ object DatabaseEncryption {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Opens a throwaway keyed SQLCipher database next to [cacheFile] and immediately closes it, purely to
|
||||
* reach `SQLiteConnection.nativeOpen` — the exact call site of the #359 crash — from inside
|
||||
* [DatabaseProvisioner]'s fail-closed handler. [ensureNativeLibraryLoaded]
|
||||
* (`System.loadLibrary("sqlcipher")`) can succeed on a device whose bundled `.so` is otherwise
|
||||
* incompatible, yet the JNI-bound `nativeOpen` still be unresolved; that only surfaces when a keyed
|
||||
* database is actually opened, which Room does LATER via its deferred open helper
|
||||
* ([org.libremail.di.DatabaseModule]) — outside any handler. Probing the real open here lets the
|
||||
* provisioner catch that [LinkageError] and fail closed BEFORE Room reaches it.
|
||||
*
|
||||
* Deliberately opens a sibling throwaway file (never the real cache) so it can never create, mutate,
|
||||
* or leave `-wal`/`-shm` sidecars on the cache, then deletes the probe and its sidecars in a `finally`.
|
||||
* Mirrors `SqlCipherOpenSpikeTest`'s stage-B keyed-open probe (issue #359).
|
||||
*
|
||||
* PRECONDITION: the caller must have already loaded the native library (via [ensureNativeLibraryLoaded]).
|
||||
* The sole production caller — [DatabaseProvisioner]'s encrypted branch — does so on the line above its
|
||||
* probe call. Kept out of here on purpose so the provisioner loads the library exactly ONCE per open
|
||||
* (the `ensureNativeLibraryLoaded()`-exactly-once invariant its instrumented tests pin), not twice.
|
||||
*/
|
||||
fun probeKeyedOpen(cacheFile: File, passphrase: String) {
|
||||
val dir = cacheFile.parentFile ?: error("cache database file has no parent directory")
|
||||
val probe = File(dir, cacheFile.name + PROBE_SUFFIX)
|
||||
try {
|
||||
SQLiteDatabase.openOrCreateDatabase(
|
||||
probe.absolutePath,
|
||||
passphrase.toByteArray(Charsets.US_ASCII),
|
||||
null, // no CursorFactory
|
||||
null, // no DatabaseErrorHandler
|
||||
).close()
|
||||
} finally {
|
||||
listOf("", "-wal", "-shm", "-journal").forEach { File(dir, probe.name + it).delete() }
|
||||
}
|
||||
}
|
||||
|
||||
private const val TAG = "LibreMailDbCrypto"
|
||||
|
||||
// Suffix of the throwaway file [probeKeyedOpen] opens to reach nativeOpen without touching the cache.
|
||||
private const val PROBE_SUFFIX = ".openprobe"
|
||||
|
||||
// The 16-byte magic that opens every plaintext SQLite file: "SQLite format 3" + a NUL terminator.
|
||||
// Spelled out as bytes to keep the trailing NUL unambiguous.
|
||||
private val SQLITE_HEADER = byteArrayOf(
|
||||
|
||||
@@ -104,35 +104,45 @@ class DatabaseProvisioner internal constructor(
|
||||
keyStore.clearClearPending()
|
||||
}
|
||||
|
||||
// One-time move of accounts/credentials/settings/signatures into the non-auth AccountDatabase
|
||||
// (issue #111). MUST run before the cache opens: opening it applies MIGRATION_15_16, which drops
|
||||
// the moved tables. Runs AFTER the wipe above so an unrecoverable-key cache is gone first
|
||||
// (nothing left to move) and we never block waiting on a passphrase we can't get.
|
||||
accountDataMigrator.migrateIfNeeded()
|
||||
|
||||
// Opt-in at-rest encryption of the local cache (off by default). The conversion runs here —
|
||||
// before the database is opened — so it never races an open connection; toggling the setting
|
||||
// therefore takes effect on the next app start. The passphrase source is resolved from which
|
||||
// seal actually exists (DatabaseKeyStore.resolvePassphrase), NOT from the app-lock setting (a
|
||||
// separate DataStore that can disagree). When app-lock is ON the sealing key is auth-bound, so
|
||||
// resolvePassphrase waits on PassphraseSession until the user authenticates — which is why this
|
||||
// must never run on the main thread while the cache is locked (issue #93).
|
||||
val settings = settingsRepository.settings.first()
|
||||
// FAIL CLOSED (issue #359, security rework of #367). SQLCipher's native library can fail to LOAD
|
||||
// (UnsatisfiedLinkError from ensureNativeLibraryLoaded) OR to LINK (the library loads, but the
|
||||
// JNI-bound SQLiteConnection.nativeOpen is unresolved and throws only when a keyed database is
|
||||
// actually opened — the exact #359 signature). EVERY startup step that touches that library must
|
||||
// therefore run inside this ONE handler, so any such LinkageError becomes a single fail-closed
|
||||
// signal rather than escaping as a raw crash. On that signal we deliberately do NOT silently
|
||||
// degrade to an unencrypted cache (that would defeat the user's opt-in encryption): we never open
|
||||
// plaintext, wipe the on-disk ciphertext, or write the encryptCache setting — we raise a distinct
|
||||
// exception the startup UI (CacheEncryptionGate) catches to show the encryption error gate. It is
|
||||
// NOT memoized (a throw skips prepareCache's `.also { prepared = it }`), so the next launch
|
||||
// re-attempts and recovers automatically if the library later loads.
|
||||
//
|
||||
// The catch stays typed LinkageError ONLY. It must NOT be broadened to Exception/Throwable: the
|
||||
// migrator below deliberately throws a NON-linkage error ("crash-loop rather than lose data") on
|
||||
// an unexpected copy failure, and that — like any other non-linkage error — must still propagate
|
||||
// uncaught so we never drop the not-yet-copied source tables.
|
||||
return try {
|
||||
// One-time move of accounts/credentials/settings/signatures into the non-auth AccountDatabase
|
||||
// (issue #111). MUST run before the cache opens: opening it applies MIGRATION_15_16, which drops
|
||||
// the moved tables. Runs AFTER the wipe above so an unrecoverable-key cache is gone first
|
||||
// (nothing left to move) and we never block waiting on a passphrase we can't get. Runs INSIDE
|
||||
// this handler (issue #359 gap 1): its copyAccountTables loads SQLCipher and does a keyed
|
||||
// openOrCreateDatabase + ATTACH … KEY (a real nativeOpen) even when encryption is OFF, so a
|
||||
// LinkageError there used to escape this handler entirely and crash-loop.
|
||||
accountDataMigrator.migrateIfNeeded()
|
||||
|
||||
// Opt-in at-rest encryption of the local cache (off by default). The conversion runs here —
|
||||
// before the database is opened — so it never races an open connection; toggling the setting
|
||||
// therefore takes effect on the next app start. The passphrase source is resolved from which
|
||||
// seal actually exists (DatabaseKeyStore.resolvePassphrase), NOT from the app-lock setting (a
|
||||
// separate DataStore that can disagree). When app-lock is ON the sealing key is auth-bound, so
|
||||
// resolvePassphrase waits on PassphraseSession until the user authenticates — which is why this
|
||||
// must never run on the main thread while the cache is locked (issue #93).
|
||||
val settings = settingsRepository.settings.first()
|
||||
resolveOpenMode(settings, dbFile)
|
||||
} catch (nativeLoadFailure: LinkageError) {
|
||||
// FAIL CLOSED (issue #359, security rework of #367). SQLCipher's native library could not be
|
||||
// loaded/linked (e.g. UnsatisfiedLinkError at SQLiteConnection.nativeOpen or from
|
||||
// ensureNativeLibraryLoaded), so the encrypted cache cannot be opened OR converted. We must
|
||||
// NOT silently degrade to an unencrypted cache (that would defeat the user's opt-in
|
||||
// encryption), so we deliberately do NOT: open plaintext, wipe the on-disk ciphertext, or
|
||||
// write the encryptCache setting. Instead raise a distinct signal the startup UI catches to
|
||||
// show the encryption error gate. This throw is NOT memoized (it skips prepareCache's
|
||||
// `.also { prepared = it }`), so the next launch re-attempts and recovers automatically if
|
||||
// the library later loads.
|
||||
AppLog.w(
|
||||
TAG,
|
||||
"SQLCipher native library failed to load; failing closed (encrypted cache unavailable)",
|
||||
"SQLCipher native library failed to load or link; failing closed (encrypted cache unavailable)",
|
||||
nativeLoadFailure,
|
||||
)
|
||||
throw CacheEncryptionUnavailableException(nativeLoadFailure)
|
||||
@@ -159,6 +169,13 @@ class DatabaseProvisioner internal constructor(
|
||||
// load the keyed open reaches SQLiteConnection.nativeOpen with no library loaded and
|
||||
// crashes with UnsatisfiedLinkError on every cold start once encryption is enabled.
|
||||
DatabaseEncryption.ensureNativeLibraryLoaded()
|
||||
// Probe the REAL keyed open HERE (issue #359 gap 2), inside the fail-closed handler.
|
||||
// ensureNativeLibraryLoaded() above only loads the .so; the keyed nativeOpen that can still
|
||||
// throw on an incompatible device fires LATER, in DatabaseModule's DeferredOpenHelperFactory
|
||||
// AFTER prepareCache() returns — outside any handler — so an open-time UnsatisfiedLinkError
|
||||
// there would escape uncaught. Reaching a keyed nativeOpen now converts that LinkageError to
|
||||
// CacheEncryptionUnavailableException before Room's deferred open can crash on it.
|
||||
DatabaseEncryption.probeKeyedOpen(dbFile, passphrase)
|
||||
CacheOpenMode.Encrypted(passphrase)
|
||||
}
|
||||
|
||||
|
||||
@@ -24,6 +24,8 @@ import org.libremail.domain.model.ImapConnectionParams
|
||||
import org.libremail.domain.repository.AccountRepository
|
||||
import org.libremail.mail.ImapClient
|
||||
import org.libremail.notifications.MailNotifier
|
||||
import org.libremail.reporting.AppLog
|
||||
import org.libremail.reporting.accountLogRef
|
||||
import javax.inject.Inject
|
||||
import javax.inject.Singleton
|
||||
|
||||
@@ -53,10 +55,17 @@ class AccountRepositoryImpl @Inject constructor(
|
||||
|
||||
override suspend fun addImapAccount(account: Account, password: String): Result<List<String>> = runCatching {
|
||||
val folders = imapClient.listFolders(account.toImapParams(secret = password, useXoauth2 = false))
|
||||
// Persist the credential BEFORE inserting the account row (#403). Both LibreMailApplication's
|
||||
// push collector and IdleService.reconcileWatchers react to the *accounts* table; committing the
|
||||
// secret first guarantees any watcher that observes the new row can already resolve it, instead
|
||||
// of firing a transient "No stored credentials" IDLE miss on every account add. The credentials
|
||||
// table has no foreign key to accounts, so it can be written first; account_settings does (FK),
|
||||
// so ensureDefaults must still follow the account row.
|
||||
credentialStore.saveSecret(account.id, password)
|
||||
accountDao.insertAtEnd(account.toEntity())
|
||||
accountSettingsRepository.ensureDefaults(account.id)
|
||||
credentialStore.saveSecret(account.id, password)
|
||||
mailNotifier.ensureAccountChannel(account)
|
||||
AppLog.i(TAG, "IMAP account added ${accountLogRef(account.id)}; credential persisted before account row")
|
||||
syncScheduler.syncNow()
|
||||
syncScheduler.backfillNow() // start caching this account's full history in the background (#12)
|
||||
folders.map { it.fullName }
|
||||
@@ -69,10 +78,15 @@ class AccountRepositoryImpl @Inject constructor(
|
||||
): Result<List<String>> = runCatching {
|
||||
val account = Account.outlook(email)
|
||||
val folders = imapClient.listFolders(account.toImapParams(secret = accessToken, useXoauth2 = true))
|
||||
// Persist the durable AuthState BEFORE the account row (#403) — same ordering rationale as
|
||||
// addImapAccount: the push watchers observe the accounts table, so the secret must be committed
|
||||
// first for the newly-observed account to resolve. account_settings' FK still needs the row, so
|
||||
// ensureDefaults follows the insert.
|
||||
credentialStore.saveSecret(account.id, authStateJson)
|
||||
accountDao.insertAtEnd(account.toEntity())
|
||||
accountSettingsRepository.ensureDefaults(account.id)
|
||||
credentialStore.saveSecret(account.id, authStateJson)
|
||||
mailNotifier.ensureAccountChannel(account)
|
||||
AppLog.i(TAG, "Outlook account added ${accountLogRef(account.id)}; credential persisted before account row")
|
||||
syncScheduler.syncNow()
|
||||
syncScheduler.backfillNow() // start caching this account's full history in the background (#12)
|
||||
folders.map { it.fullName }
|
||||
@@ -113,4 +127,8 @@ class AccountRepositoryImpl @Inject constructor(
|
||||
if (accountId != null) backfillProgressDao.deleteForAccount(accountId) else backfillProgressDao.deleteAll()
|
||||
syncScheduler.backfillNow()
|
||||
}
|
||||
|
||||
private companion object {
|
||||
const val TAG = "AccountRepository"
|
||||
}
|
||||
}
|
||||
|
||||
@@ -10,6 +10,7 @@ import dagger.assisted.Assisted
|
||||
import dagger.assisted.AssistedInject
|
||||
import kotlinx.coroutines.CancellationException
|
||||
import org.libremail.BuildConfig
|
||||
import org.libremail.data.local.isCacheEncryptionUnavailable
|
||||
import org.libremail.data.security.EncryptedCacheGuard
|
||||
import org.libremail.reporting.AppLog
|
||||
|
||||
@@ -60,7 +61,14 @@ class BackfillWorker @AssistedInject constructor(
|
||||
},
|
||||
onFailure = { error ->
|
||||
if (error is CancellationException) throw error
|
||||
AppLog.w(TAG, "backfill worker: retry", error)
|
||||
// A DB open that fails because SQLCipher's native library is unavailable (issue #359) lands
|
||||
// here (thrown inside the runCatching above); log it distinctly but still defer softly — a
|
||||
// later launch may load the library and recover — instead of a generic retry.
|
||||
if (error.isCacheEncryptionUnavailable()) {
|
||||
AppLog.w(TAG, "backfill deferred: encrypted cache unavailable (SQLCipher native library)", error)
|
||||
} else {
|
||||
AppLog.w(TAG, "backfill worker: retry", error)
|
||||
}
|
||||
Result.retry()
|
||||
},
|
||||
)
|
||||
|
||||
@@ -0,0 +1,24 @@
|
||||
// SPDX-License-Identifier: GPL-3.0-or-later
|
||||
package org.libremail.data.sync
|
||||
|
||||
import androidx.work.ListenableWorker.Result
|
||||
import org.libremail.data.local.isCacheEncryptionUnavailable
|
||||
import org.libremail.reporting.AppLog
|
||||
|
||||
/**
|
||||
* Runs [block] and, if opening the Room cache fails because SQLCipher's native library is unavailable on
|
||||
* this device (issue #359 — a `CacheEncryptionUnavailableException` the provisioner raised, or defensively
|
||||
* a bare `LinkageError` at `nativeOpen`), logs a PII-free breadcrumb under [tag] and returns
|
||||
* [Result.retry] rather than letting the failure crash the worker. WorkManager workers inject the cache
|
||||
* directly, with no UI gate; a later launch may load the library and recover, so a soft retry — not a hard
|
||||
* failure — is correct. Every other throwable (including `CancellationException`) propagates unchanged.
|
||||
*
|
||||
* `inline` so the (suspending) [block] runs in the worker's own coroutine context, mirroring `runCatching`.
|
||||
*/
|
||||
internal inline fun retryIfEncryptedCacheUnavailable(tag: String, block: () -> Result): Result = try {
|
||||
block()
|
||||
} catch (failure: Throwable) {
|
||||
if (!failure.isCacheEncryptionUnavailable()) throw failure
|
||||
AppLog.w(tag, "deferred: encrypted cache unavailable (SQLCipher native library)", failure)
|
||||
Result.retry()
|
||||
}
|
||||
@@ -51,7 +51,7 @@ class MailConnectionFactory @Inject constructor(
|
||||
|
||||
private suspend fun resolveSecret(account: Account): String = when (account.authType) {
|
||||
AuthType.PASSWORD_IMAP ->
|
||||
credentialStore.loadSecret(account.id) ?: error("No stored credentials for ${account.email}")
|
||||
credentialStore.loadSecret(account.id) ?: throw MissingCredentialsException()
|
||||
AuthType.OAUTH_OUTLOOK ->
|
||||
cachedAccessToken(account.id, SCOPE_OUTLOOK, outlookAuthManager::freshOutlookToken)
|
||||
}
|
||||
@@ -70,7 +70,7 @@ class MailConnectionFactory @Inject constructor(
|
||||
// computeIfAbsent (not getOrPut) so concurrent first-callers share one mutex per account.
|
||||
return refreshMutexes.computeIfAbsent(accountId) { Mutex() }.withLock {
|
||||
validCachedToken(accountId, scope)?.let { return@withLock it }
|
||||
val stored = credentialStore.loadSecret(accountId) ?: error("No stored credentials for $accountId")
|
||||
val stored = credentialStore.loadSecret(accountId) ?: throw MissingCredentialsException()
|
||||
val fresh = refresh(stored)
|
||||
if (fresh.authStateJson != stored) credentialStore.saveSecret(accountId, fresh.authStateJson)
|
||||
tokenCache["$accountId|$scope"] = CachedToken(fresh.accessToken, fresh.accessTokenExpiry)
|
||||
|
||||
@@ -0,0 +1,20 @@
|
||||
// SPDX-License-Identifier: GPL-3.0-or-later
|
||||
package org.libremail.data.sync
|
||||
|
||||
/**
|
||||
* Thrown by [MailConnectionFactory] when an account has no stored credential to resolve.
|
||||
*
|
||||
* It extends [IllegalStateException] — the type [kotlin.error] previously raised here — so existing
|
||||
* callers that treat a missing credential as a hard failure are unaffected. What it adds is a type a
|
||||
* caller can catch *specifically*: the IMAP IDLE watcher ([org.libremail.push.IdleService]) tolerates
|
||||
* the brief account-add write race (#403), where a reactive observer of the accounts table can see a
|
||||
* newly-added account a beat before its secret has finished persisting, by catching this and deferring
|
||||
* instead of logging a connection failure. Genuinely-absent credentials still surface as an error to
|
||||
* every other caller.
|
||||
*
|
||||
* The message is deliberately PII-free (no email, host, or account id): an account id embeds the raw
|
||||
* email address, so it must never appear in an exception message that could reach Logcat. A caller that
|
||||
* needs to name the account in a log line uses [org.libremail.reporting.accountLogRef] on the id it
|
||||
* already holds.
|
||||
*/
|
||||
class MissingCredentialsException : IllegalStateException("No stored credentials for account")
|
||||
@@ -8,6 +8,7 @@ import androidx.work.WorkerParameters
|
||||
import dagger.Lazy
|
||||
import dagger.assisted.Assisted
|
||||
import dagger.assisted.AssistedInject
|
||||
import org.libremail.data.local.isCacheEncryptionUnavailable
|
||||
import org.libremail.data.security.EncryptedCacheGuard
|
||||
import org.libremail.reporting.AppLog
|
||||
|
||||
@@ -39,7 +40,14 @@ class PruneWorker @AssistedInject constructor(
|
||||
Result.success()
|
||||
},
|
||||
onFailure = { error ->
|
||||
AppLog.w(TAG, "prune worker: retry", error)
|
||||
// A DB open that fails because SQLCipher's native library is unavailable (issue #359) lands
|
||||
// here (it is thrown inside the runCatching above); log it distinctly but still defer softly
|
||||
// — a later launch may load the library and recover — instead of a generic retry.
|
||||
if (error.isCacheEncryptionUnavailable()) {
|
||||
AppLog.w(TAG, "prune deferred: encrypted cache unavailable (SQLCipher native library)", error)
|
||||
} else {
|
||||
AppLog.w(TAG, "prune worker: retry", error)
|
||||
}
|
||||
Result.retry()
|
||||
},
|
||||
)
|
||||
|
||||
@@ -52,6 +52,13 @@ class SendWorker @AssistedInject constructor(
|
||||
|
||||
override suspend fun doWork(): Result {
|
||||
if (cacheGuard.isCacheLocked()) return Result.retry()
|
||||
// Resolving the Lazy DB deps below opens the Room cache; if SQLCipher's native library is
|
||||
// unavailable on this device (issue #359) that open throws — with no UI gate here, treat it as a
|
||||
// soft retry rather than letting it crash the worker.
|
||||
return retryIfEncryptedCacheUnavailable(TAG) { drainOutbox() }
|
||||
}
|
||||
|
||||
private suspend fun drainOutbox(): Result {
|
||||
val outboxDao = this.outboxDao.get()
|
||||
val accountDao = this.accountDao.get()
|
||||
val connectionFactory = this.connectionFactory.get()
|
||||
|
||||
@@ -28,16 +28,21 @@ class SyncWorker @AssistedInject constructor(
|
||||
AppLog.i(TAG, "sync deferred: cache locked")
|
||||
return Result.retry()
|
||||
}
|
||||
return mailSyncer.get().syncAll().fold(
|
||||
onSuccess = {
|
||||
AppLog.i(TAG, "sync worker: success")
|
||||
Result.success()
|
||||
},
|
||||
onFailure = { error ->
|
||||
AppLog.w(TAG, "sync worker: retry", error)
|
||||
Result.retry()
|
||||
},
|
||||
)
|
||||
// syncAll()'s first DB access (and thus Room's deferred cache open) can throw if SQLCipher's
|
||||
// native library is unavailable on this device (issue #359) — that open fires OUTSIDE syncAll's own
|
||||
// runCatching, so without this guard it would escape doWork. Treat it as a soft retry, not a crash.
|
||||
return retryIfEncryptedCacheUnavailable(TAG) {
|
||||
mailSyncer.get().syncAll().fold(
|
||||
onSuccess = {
|
||||
AppLog.i(TAG, "sync worker: success")
|
||||
Result.success()
|
||||
},
|
||||
onFailure = { error ->
|
||||
AppLog.w(TAG, "sync worker: retry", error)
|
||||
Result.retry()
|
||||
},
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
private companion object {
|
||||
|
||||
@@ -27,10 +27,12 @@ import kotlinx.coroutines.isActive
|
||||
import kotlinx.coroutines.launch
|
||||
import kotlinx.coroutines.withTimeoutOrNull
|
||||
import org.libremail.data.local.dao.AccountDao
|
||||
import org.libremail.data.local.isCacheEncryptionUnavailable
|
||||
import org.libremail.data.local.toDomain
|
||||
import org.libremail.data.security.EncryptedCacheGuard
|
||||
import org.libremail.data.sync.MailConnectionFactory
|
||||
import org.libremail.data.sync.MailSyncer
|
||||
import org.libremail.data.sync.MissingCredentialsException
|
||||
import org.libremail.data.sync.PushMode
|
||||
import org.libremail.data.sync.SyncResourcePolicy
|
||||
import org.libremail.data.sync.SyncScheduler
|
||||
@@ -110,7 +112,20 @@ class IdleService : Service() {
|
||||
stopSelf()
|
||||
return@launch
|
||||
}
|
||||
reconcileWatchers()
|
||||
// Headless tolerance for issue #359: this service injects the Room cache with NO UI gate
|
||||
// (CacheEncryptionGate wraps only MainActivity), so if SQLCipher's native library is unavailable
|
||||
// on this device reconcileWatchers()'s first DB access throws — a CacheEncryptionUnavailableException
|
||||
// from the provisioner, or defensively a bare LinkageError at nativeOpen. Left uncaught, a child of
|
||||
// this SupervisorJob would route it to the app's default handler and crash the process. Treat it
|
||||
// like the cache-locked case: log PII-free and stop. The app's CacheEncryptionGate surfaces the
|
||||
// error to the user, and a later restart re-probes and recovers if the library loads.
|
||||
try {
|
||||
reconcileWatchers()
|
||||
} catch (unavailable: Throwable) {
|
||||
if (!unavailable.isCacheEncryptionUnavailable()) throw unavailable
|
||||
AppLog.w(TAG, "encrypted cache unavailable (SQLCipher native lib); deferring IDLE push", unavailable)
|
||||
stopSelf()
|
||||
}
|
||||
}
|
||||
// The reuse cache (issue #357 Part 2) keeps interactive/sync IMAP connections warm; sweep
|
||||
// them so a socket that has gone idle past the reuse timeout is closed rather than left
|
||||
@@ -301,6 +316,16 @@ class IdleService : Service() {
|
||||
backoffMs = INITIAL_BACKOFF_MS
|
||||
} catch (e: CancellationException) {
|
||||
throw e
|
||||
} catch (ignored: MissingCredentialsException) {
|
||||
// #403: a just-added account can be observed here a beat before its secret finishes
|
||||
// persisting. AccountRepository now commits the secret before the account row, so this is
|
||||
// rare — but tolerate any residual race as a transient miss: defer quietly and re-check
|
||||
// soon, WITHOUT the warn + exponential backoff a real connection drop gets. A genuinely
|
||||
// absent credential simply keeps deferring (no mail, but no error noise) until it appears
|
||||
// or the account is removed. The sentinel exception carries no diagnostic value beyond the
|
||||
// message below, so it is intentionally not re-logged with its (empty) trace.
|
||||
AppLog.i(TAG, "IDLE deferred ${accountLogRef(account.id)}: credentials not yet persisted")
|
||||
delay(CREDENTIALS_DEFER_RETRY_MS)
|
||||
} catch (e: Exception) {
|
||||
AppLog.w(TAG, "IDLE for ${accountLogRef(account.id)} dropped; retrying in ${backoffMs}ms", e)
|
||||
delay(backoffMs)
|
||||
@@ -332,6 +357,12 @@ class IdleService : Service() {
|
||||
const val INITIAL_BACKOFF_MS = 5_000L
|
||||
const val MAX_BACKOFF_MS = 5 * 60_000L
|
||||
|
||||
// #403: how long to wait before re-checking after a transient missing-credential miss on a
|
||||
// just-added account. Short — the account-add write race resolves in milliseconds once the
|
||||
// secret commit lands — and deliberately flat (no exponential escalation) because this is an
|
||||
// expected persist-ordering blip, not a connection failure.
|
||||
const val CREDENTIALS_DEFER_RETRY_MS = 1_000L
|
||||
|
||||
// Cadence of the reuse-cache idle-eviction sweep (issue #357 Part 2). Tighter than the reuse
|
||||
// idle timeout so an idle socket is closed shortly after it crosses it.
|
||||
const val REUSE_EVICTION_SWEEP_MS = 2 * 60_000L
|
||||
|
||||
@@ -42,6 +42,7 @@ import org.libremail.ui.onboarding.ContactsAccessScreen
|
||||
import org.libremail.ui.onboarding.LicenseScreen
|
||||
import org.libremail.ui.onboarding.OnboardingViewModel
|
||||
import org.libremail.ui.onboarding.OnboardingWelcomeScreen
|
||||
import org.libremail.ui.onboarding.OutlookImapNoticeScreen
|
||||
import org.libremail.ui.outbox.OutboxScreen
|
||||
import org.libremail.ui.reader.ReaderScreen
|
||||
import org.libremail.ui.reporting.ProblemReportsScreen
|
||||
@@ -344,29 +345,50 @@ private fun NavGraphBuilder.onboardingGraph(navController: NavHostController, li
|
||||
navController.navigate(Routes.onboardingAppPassword(provider.key))
|
||||
},
|
||||
onManualSetup = { navController.navigate(Routes.ONBOARDING_MANUAL) },
|
||||
// Onboarding interposes the IMAP-enablement notice before Microsoft sign-in (#411);
|
||||
// the notice screen then runs the same Outlook OAuth flow the picker would inline.
|
||||
onPickOutlook = { navController.navigate(Routes.ONBOARDING_OUTLOOK_IMAP) },
|
||||
)
|
||||
}
|
||||
composable(
|
||||
route = Routes.ONBOARDING_APP_PASSWORD_PATTERN,
|
||||
arguments = listOf(navArgument(Routes.APP_PASSWORD_ARG_PROVIDER) { type = NavType.StringType }),
|
||||
) { entry ->
|
||||
val onboarding = onboardingViewModel(navController, entry)
|
||||
AppPasswordSetupScreen(
|
||||
onBack = navController::popBackStack,
|
||||
onAccountAdded = { id -> onboarding.completeAdd(navController, id) },
|
||||
)
|
||||
}
|
||||
composable(Routes.ONBOARDING_MANUAL) { entry ->
|
||||
val onboarding = onboardingViewModel(navController, entry)
|
||||
ManualSetupScreen(
|
||||
onBack = navController::popBackStack,
|
||||
onAccountAdded = { id -> onboarding.completeAdd(navController, id) },
|
||||
)
|
||||
}
|
||||
onboardingSetupDestinations(navController)
|
||||
onboardingFinishDestinations(navController)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The account-setup destinations reached from the vendor picker: the pre-auth Outlook IMAP-enablement
|
||||
* notice (#411), the guided app-password form, and manual IMAP/SMTP setup. Split out of
|
||||
* [onboardingGraph] so each stays a readable length; all share the graph-scoped [OnboardingViewModel]
|
||||
* and report a completed add through [completeAdd], which drops the setup screen and advances to the
|
||||
* "add another?" prompt.
|
||||
*/
|
||||
private fun NavGraphBuilder.onboardingSetupDestinations(navController: NavHostController) {
|
||||
composable(Routes.ONBOARDING_OUTLOOK_IMAP) { entry ->
|
||||
val onboarding = onboardingViewModel(navController, entry)
|
||||
OutlookImapNoticeScreen(
|
||||
onBack = navController::popBackStack,
|
||||
onAccountAdded = { id -> onboarding.completeAdd(navController, id) },
|
||||
)
|
||||
}
|
||||
composable(
|
||||
route = Routes.ONBOARDING_APP_PASSWORD_PATTERN,
|
||||
arguments = listOf(navArgument(Routes.APP_PASSWORD_ARG_PROVIDER) { type = NavType.StringType }),
|
||||
) { entry ->
|
||||
val onboarding = onboardingViewModel(navController, entry)
|
||||
AppPasswordSetupScreen(
|
||||
onBack = navController::popBackStack,
|
||||
onAccountAdded = { id -> onboarding.completeAdd(navController, id) },
|
||||
)
|
||||
}
|
||||
composable(Routes.ONBOARDING_MANUAL) { entry ->
|
||||
val onboarding = onboardingViewModel(navController, entry)
|
||||
ManualSetupScreen(
|
||||
onBack = navController::popBackStack,
|
||||
onAccountAdded = { id -> onboarding.completeAdd(navController, id) },
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The tail of onboarding: the "add another?" prompt and the optional contacts + battery opt-in steps.
|
||||
* Split out of [onboardingGraph] so each stays a readable length; all share the graph-scoped
|
||||
|
||||
@@ -57,6 +57,9 @@ import org.libremail.domain.model.MailProvider
|
||||
*
|
||||
* @param onAccountAdded invoked with the new account id when the *inline* Outlook flow completes.
|
||||
* The app-password and manual paths report their own completion from their own screens.
|
||||
* @param onPickOutlook when non-null, tapping Outlook delegates here instead of launching auth
|
||||
* inline — onboarding uses it to first show the pre-auth IMAP-enablement notice (#411). Left null
|
||||
* for the standalone "Add account" entry, which keeps launching Microsoft sign-in directly.
|
||||
*/
|
||||
@OptIn(ExperimentalMaterial3Api::class)
|
||||
@Composable
|
||||
@@ -65,6 +68,7 @@ fun AccountPickerScreen(
|
||||
onAccountAdded: (String) -> Unit,
|
||||
onPickProvider: (MailProvider) -> Unit,
|
||||
onManualSetup: () -> Unit,
|
||||
onPickOutlook: (() -> Unit)? = null,
|
||||
viewModel: AccountSetupViewModel = hiltViewModel(),
|
||||
) {
|
||||
val state by viewModel.state.collectAsStateWithLifecycle()
|
||||
@@ -74,6 +78,19 @@ fun AccountPickerScreen(
|
||||
ActivityResultContracts.StartActivityForResult(),
|
||||
) { result -> viewModel.onOutlookResult(result.data) }
|
||||
|
||||
// Launches Microsoft sign-in in place (the standalone "Add account" behaviour). Onboarding
|
||||
// overrides the Outlook tap with [onPickOutlook] to interpose the IMAP-enablement notice (#411),
|
||||
// which then runs this exact same flow from its "Sign in" button.
|
||||
val launchOutlookInline: () -> Unit = {
|
||||
viewModel.outlookAuthIntent().fold(
|
||||
onSuccess = { intent ->
|
||||
runCatching { outlookLauncher.launch(intent) }
|
||||
.onFailure { viewModel.onOutlookLaunchFailed(it) }
|
||||
},
|
||||
onFailure = { viewModel.onOutlookLaunchFailed(it) },
|
||||
)
|
||||
}
|
||||
|
||||
LaunchedEffect(state.status, state.addedAccountId) {
|
||||
if (state.status == SetupStatus.DONE) {
|
||||
state.addedAccountId?.let(onAccountAdded)
|
||||
@@ -124,15 +141,7 @@ fun AccountPickerScreen(
|
||||
icon = Icons.Filled.Email,
|
||||
label = stringResource(R.string.account_setup_outlook),
|
||||
enabled = !busy,
|
||||
onClick = {
|
||||
viewModel.outlookAuthIntent().fold(
|
||||
onSuccess = { intent ->
|
||||
runCatching { outlookLauncher.launch(intent) }
|
||||
.onFailure { viewModel.onOutlookLaunchFailed(it) }
|
||||
},
|
||||
onFailure = { viewModel.onOutlookLaunchFailed(it) },
|
||||
)
|
||||
},
|
||||
onClick = onPickOutlook ?: launchOutlookInline,
|
||||
)
|
||||
MailProvider.entries.forEach { provider ->
|
||||
ProviderRow(
|
||||
|
||||
@@ -39,6 +39,14 @@ object Routes {
|
||||
const val ONBOARDING_LICENSE = "onboarding/license"
|
||||
const val ONBOARDING_WELCOME = "onboarding/welcome"
|
||||
const val ONBOARDING_PICKER = "onboarding/picker"
|
||||
|
||||
// Pre-auth interstitial shown when the user picks Outlook during onboarding (#411): new
|
||||
// personal outlook.com accounts ship with IMAP OFF by default, so OAuth can succeed while the
|
||||
// IMAP AUTHENTICATE step later fails. This screen asks the user to confirm IMAP is on and links
|
||||
// Microsoft's help/settings pages before its bottom "Sign in" button continues the existing
|
||||
// Outlook OAuth flow. Onboarding-only; the standalone "Add account" picker still launches auth
|
||||
// inline (the reactive complement for that path is #390).
|
||||
const val ONBOARDING_OUTLOOK_IMAP = "onboarding/outlook_imap"
|
||||
const val ONBOARDING_MANUAL = "onboarding/manual"
|
||||
const val ONBOARDING_ADD_ANOTHER = "onboarding/add_another"
|
||||
|
||||
|
||||
@@ -0,0 +1,213 @@
|
||||
// SPDX-License-Identifier: GPL-3.0-or-later
|
||||
package org.libremail.ui.onboarding
|
||||
|
||||
import androidx.activity.compose.rememberLauncherForActivityResult
|
||||
import androidx.activity.result.contract.ActivityResultContracts
|
||||
import androidx.compose.foundation.background
|
||||
import androidx.compose.foundation.layout.Box
|
||||
import androidx.compose.foundation.layout.Column
|
||||
import androidx.compose.foundation.layout.Spacer
|
||||
import androidx.compose.foundation.layout.fillMaxSize
|
||||
import androidx.compose.foundation.layout.fillMaxWidth
|
||||
import androidx.compose.foundation.layout.height
|
||||
import androidx.compose.foundation.layout.padding
|
||||
import androidx.compose.foundation.layout.size
|
||||
import androidx.compose.foundation.rememberScrollState
|
||||
import androidx.compose.foundation.verticalScroll
|
||||
import androidx.compose.material.icons.Icons
|
||||
import androidx.compose.material.icons.automirrored.filled.ArrowBack
|
||||
import androidx.compose.material.icons.filled.Email
|
||||
import androidx.compose.material3.Button
|
||||
import androidx.compose.material3.CircularProgressIndicator
|
||||
import androidx.compose.material3.ExperimentalMaterial3Api
|
||||
import androidx.compose.material3.Icon
|
||||
import androidx.compose.material3.IconButton
|
||||
import androidx.compose.material3.MaterialTheme
|
||||
import androidx.compose.material3.OutlinedButton
|
||||
import androidx.compose.material3.Scaffold
|
||||
import androidx.compose.material3.SnackbarHost
|
||||
import androidx.compose.material3.SnackbarHostState
|
||||
import androidx.compose.material3.Text
|
||||
import androidx.compose.material3.TopAppBar
|
||||
import androidx.compose.runtime.Composable
|
||||
import androidx.compose.runtime.LaunchedEffect
|
||||
import androidx.compose.runtime.getValue
|
||||
import androidx.compose.runtime.remember
|
||||
import androidx.compose.runtime.rememberCoroutineScope
|
||||
import androidx.compose.ui.Alignment
|
||||
import androidx.compose.ui.Modifier
|
||||
import androidx.compose.ui.platform.LocalUriHandler
|
||||
import androidx.compose.ui.res.stringResource
|
||||
import androidx.compose.ui.unit.dp
|
||||
import androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel
|
||||
import androidx.lifecycle.compose.collectAsStateWithLifecycle
|
||||
import kotlinx.coroutines.launch
|
||||
import org.libremail.R
|
||||
import org.libremail.reporting.AppLog
|
||||
import org.libremail.ui.accountsetup.AccountSetupViewModel
|
||||
import org.libremail.ui.accountsetup.SetupStatus
|
||||
|
||||
/**
|
||||
* Pre-auth interstitial shown when the user picks Outlook during onboarding (#411), before the
|
||||
* Microsoft OAuth browser opens. New personal outlook.com accounts ship with IMAP **OFF** by
|
||||
* default, so OAuth can succeed while the later IMAP `AUTHENTICATE` step fails — a confusing
|
||||
* dead-end (the *reactive* complement is #390). This screen asks the user to confirm IMAP is on
|
||||
* first, links Microsoft's help article and the Outlook IMAP settings page, and only then continues
|
||||
* the **existing** Outlook OAuth flow from its bottom "Sign in" button.
|
||||
*
|
||||
* The sign-in wiring is identical to the picker's Outlook row: build the AppAuth intent via
|
||||
* [AccountSetupViewModel.outlookAuthIntent], launch it, and hand the redirect back to
|
||||
* [AccountSetupViewModel.onOutlookResult]; a completed add is reported through [onAccountAdded]. No
|
||||
* PII is logged — the account address is embedded in the id and never touched here (see
|
||||
* [AccountSetupViewModel.onOutlookResult] for the add breadcrumb).
|
||||
*
|
||||
* @param onBack returns to the vendor picker (e.g. to choose a different provider).
|
||||
* @param onAccountAdded invoked with the new account id once the Outlook flow completes.
|
||||
*/
|
||||
@OptIn(ExperimentalMaterial3Api::class)
|
||||
@Composable
|
||||
fun OutlookImapNoticeScreen(
|
||||
onBack: () -> Unit,
|
||||
onAccountAdded: (String) -> Unit,
|
||||
viewModel: AccountSetupViewModel = hiltViewModel(),
|
||||
) {
|
||||
val state by viewModel.state.collectAsStateWithLifecycle()
|
||||
val snackbarHostState = remember { SnackbarHostState() }
|
||||
val uriHandler = LocalUriHandler.current
|
||||
val scope = rememberCoroutineScope()
|
||||
// Resolved up front so the non-composable failure handler can use it.
|
||||
val openFailedMessage = stringResource(R.string.app_password_open_failed)
|
||||
|
||||
val outlookLauncher = rememberLauncherForActivityResult(
|
||||
ActivityResultContracts.StartActivityForResult(),
|
||||
) { result -> viewModel.onOutlookResult(result.data) }
|
||||
|
||||
// One-shot breadcrumb so a debug report shows the user reached the pre-auth IMAP notice (#411).
|
||||
LaunchedEffect(Unit) { AppLog.i(TAG, "Outlook IMAP notice shown") }
|
||||
|
||||
LaunchedEffect(state.status, state.addedAccountId) {
|
||||
if (state.status == SetupStatus.DONE) {
|
||||
state.addedAccountId?.let(onAccountAdded)
|
||||
}
|
||||
}
|
||||
LaunchedEffect(state.error) {
|
||||
state.error?.let {
|
||||
snackbarHostState.showSnackbar(it)
|
||||
viewModel.consumeError()
|
||||
}
|
||||
}
|
||||
|
||||
// openUri throws when no browser/handler is installed; surface that as a snackbar, not a crash.
|
||||
val openUrl: (String) -> Unit = { url ->
|
||||
runCatching { uriHandler.openUri(url) }
|
||||
.onFailure { scope.launch { snackbarHostState.showSnackbar(openFailedMessage) } }
|
||||
}
|
||||
|
||||
val busy = state.status == SetupStatus.CONNECTING
|
||||
|
||||
Scaffold(
|
||||
topBar = {
|
||||
TopAppBar(
|
||||
title = { Text(stringResource(R.string.outlook_imap_title)) },
|
||||
navigationIcon = {
|
||||
IconButton(onClick = onBack) {
|
||||
Icon(
|
||||
Icons.AutoMirrored.Filled.ArrowBack,
|
||||
contentDescription = stringResource(R.string.action_back),
|
||||
)
|
||||
}
|
||||
},
|
||||
)
|
||||
},
|
||||
snackbarHost = { SnackbarHost(snackbarHostState) },
|
||||
) { padding ->
|
||||
Box(Modifier.fillMaxSize().padding(padding)) {
|
||||
Column(
|
||||
modifier = Modifier
|
||||
.fillMaxSize()
|
||||
.verticalScroll(rememberScrollState())
|
||||
.padding(24.dp),
|
||||
) {
|
||||
Icon(
|
||||
Icons.Filled.Email,
|
||||
contentDescription = null,
|
||||
modifier = Modifier.size(64.dp),
|
||||
tint = MaterialTheme.colorScheme.primary,
|
||||
)
|
||||
Spacer(Modifier.height(20.dp))
|
||||
Text(
|
||||
text = stringResource(R.string.outlook_imap_question),
|
||||
style = MaterialTheme.typography.headlineSmall,
|
||||
)
|
||||
Spacer(Modifier.height(12.dp))
|
||||
Text(
|
||||
text = stringResource(R.string.outlook_imap_body),
|
||||
style = MaterialTheme.typography.bodyLarge,
|
||||
color = MaterialTheme.colorScheme.onSurfaceVariant,
|
||||
)
|
||||
Spacer(Modifier.height(24.dp))
|
||||
OutlinedButton(
|
||||
onClick = {
|
||||
AppLog.i(TAG, "Outlook IMAP help article opened")
|
||||
openUrl(IMAP_HELP_URL)
|
||||
},
|
||||
modifier = Modifier.fillMaxWidth(),
|
||||
) {
|
||||
Text(stringResource(R.string.outlook_imap_help))
|
||||
}
|
||||
Spacer(Modifier.height(8.dp))
|
||||
OutlinedButton(
|
||||
onClick = {
|
||||
AppLog.i(TAG, "Outlook IMAP settings page opened")
|
||||
openUrl(IMAP_SETTINGS_URL)
|
||||
},
|
||||
modifier = Modifier.fillMaxWidth(),
|
||||
) {
|
||||
Text(stringResource(R.string.outlook_imap_settings))
|
||||
}
|
||||
Spacer(Modifier.height(32.dp))
|
||||
// Bottom of the visual hierarchy (#411): continues the existing Outlook OAuth flow,
|
||||
// unchanged. Append any new controls AFTER this so onboarding E2E clicks stay stable.
|
||||
Button(
|
||||
onClick = {
|
||||
AppLog.i(TAG, "Outlook sign-in continued from IMAP notice")
|
||||
viewModel.outlookAuthIntent().fold(
|
||||
onSuccess = { intent ->
|
||||
runCatching { outlookLauncher.launch(intent) }
|
||||
.onFailure { viewModel.onOutlookLaunchFailed(it) }
|
||||
},
|
||||
onFailure = { viewModel.onOutlookLaunchFailed(it) },
|
||||
)
|
||||
},
|
||||
enabled = !busy,
|
||||
modifier = Modifier.fillMaxWidth(),
|
||||
) {
|
||||
Text(stringResource(R.string.outlook_imap_sign_in))
|
||||
}
|
||||
}
|
||||
if (busy) {
|
||||
Box(
|
||||
modifier = Modifier
|
||||
.fillMaxSize()
|
||||
.background(MaterialTheme.colorScheme.scrim.copy(alpha = 0.32f)),
|
||||
contentAlignment = Alignment.Center,
|
||||
) {
|
||||
CircularProgressIndicator()
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private const val TAG = "OutlookImapNotice"
|
||||
|
||||
// Microsoft's canonical "POP, IMAP, and SMTP settings for Outlook.com" support article — the
|
||||
// authoritative walkthrough for switching IMAP on (verified 2026-07, issue #411).
|
||||
private const val IMAP_HELP_URL =
|
||||
"https://support.microsoft.com/en-us/office/pop-imap-and-smtp-settings-for-outlook-com-" +
|
||||
"d088b986-291d-42b8-9564-9c414e2aa040"
|
||||
|
||||
// Deep link to the Outlook.com POP/IMAP settings page ("Let devices and apps use IMAP"). If
|
||||
// Microsoft changes the options path this still lands the user in Outlook.com mail settings; the
|
||||
// help article above is the durable fallback.
|
||||
private const val IMAP_SETTINGS_URL = "https://outlook.live.com/mail/0/options/mail/accounts/popImap"
|
||||
@@ -205,6 +205,14 @@
|
||||
<string name="account_setup_other">Other (IMAP/SMTP)</string>
|
||||
<string name="account_setup_subtitle">Choose your email provider to get started.</string>
|
||||
|
||||
<!-- Onboarding: pre-auth Outlook IMAP-enablement notice, shown before Microsoft sign-in (#411) -->
|
||||
<string name="outlook_imap_title">Turn on IMAP for Outlook</string>
|
||||
<string name="outlook_imap_question">Have you enabled IMAP for your Outlook account?</string>
|
||||
<string name="outlook_imap_body">New Outlook.com accounts often have IMAP switched off. LibreMail needs IMAP turned on to receive your mail — please enable it before signing in.</string>
|
||||
<string name="outlook_imap_help">How to enable IMAP for Outlook</string>
|
||||
<string name="outlook_imap_settings">Open Outlook IMAP settings</string>
|
||||
<string name="outlook_imap_sign_in">Sign in</string>
|
||||
|
||||
<!-- App-password guided setup (Gmail/Yahoo/iCloud/AOL) -->
|
||||
<string name="app_password_title">Connect %1$s</string>
|
||||
<string name="app_password_unknown_provider">Unknown email provider.</string>
|
||||
|
||||
+54
@@ -0,0 +1,54 @@
|
||||
// SPDX-License-Identifier: GPL-3.0-or-later
|
||||
package org.libremail.data.local
|
||||
|
||||
import kotlinx.coroutines.CancellationException
|
||||
import org.junit.Test
|
||||
import kotlin.test.assertFalse
|
||||
import kotlin.test.assertTrue
|
||||
|
||||
/**
|
||||
* [isCacheEncryptionUnavailable] is the shared classifier the headless entry points (WorkManager workers
|
||||
* and `IdleService`) use to decide whether a database-open failure is the issue-#359 "SQLCipher native
|
||||
* library unavailable" condition — which they defer/skip softly — versus any other failure, which they
|
||||
* handle normally. It must recognise the provisioner's [CacheEncryptionUnavailableException] AND a bare
|
||||
* [LinkageError] (defensive), even when wrapped several layers deep (coroutine stack-trace recovery
|
||||
* re-wraps the throwable across the open boundary), and reject everything else — including
|
||||
* [CancellationException], which must always propagate.
|
||||
*/
|
||||
class CacheEncryptionUnavailableExceptionTest {
|
||||
|
||||
@Test
|
||||
fun `recognises a direct CacheEncryptionUnavailableException`() {
|
||||
val failure = CacheEncryptionUnavailableException(UnsatisfiedLinkError("nativeOpen"))
|
||||
assertTrue(failure.isCacheEncryptionUnavailable())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `recognises a bare LinkageError`() {
|
||||
assertTrue(UnsatisfiedLinkError("SQLiteConnection.nativeOpen").isCacheEncryptionUnavailable())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `recognises a CacheEncryptionUnavailableException wrapped deep in the cause chain`() {
|
||||
val wrapped = RuntimeException(
|
||||
"room open failed",
|
||||
IllegalStateException("delegate", CacheEncryptionUnavailableException(UnsatisfiedLinkError())),
|
||||
)
|
||||
assertTrue(wrapped.isCacheEncryptionUnavailable())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `recognises a LinkageError nested as a cause`() {
|
||||
assertTrue(RuntimeException("open", UnsatisfiedLinkError("nativeOpen")).isCacheEncryptionUnavailable())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `rejects an unrelated exception`() {
|
||||
assertFalse(IllegalStateException("database is locked").isCacheEncryptionUnavailable())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `rejects a CancellationException so cancellation still propagates`() {
|
||||
assertFalse(CancellationException("job cancelled").isCacheEncryptionUnavailable())
|
||||
}
|
||||
}
|
||||
@@ -71,6 +71,7 @@ class DatabaseProvisionerTest {
|
||||
every { DatabaseEncryption.ensureEncrypted(any(), any()) } just Runs
|
||||
every { DatabaseEncryption.ensurePlaintext(any(), any()) } just Runs
|
||||
every { DatabaseEncryption.ensureNativeLibraryLoaded() } just Runs
|
||||
every { DatabaseEncryption.probeKeyedOpen(any(), any()) } just Runs
|
||||
every { settingsRepository.settings } returns flowOf(AppSettings())
|
||||
|
||||
coEvery { keyStore.isClearPending() } returns false
|
||||
@@ -104,6 +105,10 @@ class DatabaseProvisionerTest {
|
||||
// load only rode on that conversion, Room's keyed open would hit nativeOpen with no .so loaded
|
||||
// and throw UnsatisfiedLinkError on every cold start.
|
||||
verify(exactly = 1) { DatabaseEncryption.ensureNativeLibraryLoaded() }
|
||||
// Issue #359 gap 2: the encrypted branch also probes a REAL keyed open (nativeOpen) inside the
|
||||
// fail-closed handler, so an open-time UnsatisfiedLinkError is caught here rather than escaping
|
||||
// Room's later deferred open.
|
||||
verify(exactly = 1) { DatabaseEncryption.probeKeyedOpen(any(), PASSPHRASE) }
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -147,6 +152,53 @@ class DatabaseProvisionerTest {
|
||||
coVerify(exactly = 0) { settingsRepository.setEncryptCache(any()) }
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a native-library load failure in the account migrator fails closed`() = runTest {
|
||||
// Issue #359 gap 1 (PRIMARY): AccountDataMigrator.copyAccountTables loads SQLCipher and does a
|
||||
// keyed openOrCreateDatabase + ATTACH … KEY (a real nativeOpen) even when encryption is OFF, so a
|
||||
// LinkageError there must fail closed too. It used to ESCAPE the handler because migrateIfNeeded()
|
||||
// ran OUTSIDE the try/catch (crash-loop). Un-mock the `just Runs` default (which hid the gap) and
|
||||
// make the migrator throw the exact #359 error — encryption stays at its default OFF here to prove
|
||||
// the migrator drags EVERY upgrader through the native library regardless of the setting.
|
||||
coEvery { accountDataMigrator.migrateIfNeeded() } throws
|
||||
UnsatisfiedLinkError("dlopen failed: libsqlcipher.so is not 16 KB aligned")
|
||||
|
||||
val error = assertFailsWith<CacheEncryptionUnavailableException> { provisioner().prepareCache() }
|
||||
|
||||
assertTrue(
|
||||
generateSequence(error.cause) { it.cause }.any { it is LinkageError },
|
||||
"the native-load LinkageError must be preserved as the cause, not surface as a raw LinkageError",
|
||||
)
|
||||
// Fail CLOSED, not open: nothing wiped, no seal reset, the setting untouched.
|
||||
verify(exactly = 0) { DatabaseFiles.clear(any()) }
|
||||
coVerify(exactly = 0) { keyStore.resetSealedPassphrase() }
|
||||
coVerify(exactly = 0) { settingsRepository.setEncryptCache(any()) }
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a nativeOpen failure while probing the encrypted open fails closed`() = runTest {
|
||||
every { settingsRepository.settings } returns flowOf(AppSettings(encryptCache = true, appLock = false))
|
||||
// Issue #359 gap 2 (SECONDARY): loadLibrary succeeds, but the REAL keyed open throws an
|
||||
// UnsatisfiedLinkError at SQLiteConnection.nativeOpen — the exact #359 signature. The provisioner
|
||||
// probes that open INSIDE the fail-closed handler, so it converts here rather than letting Room's
|
||||
// later deferred open (DatabaseModule) crash uncaught.
|
||||
every { DatabaseEncryption.probeKeyedOpen(any(), any()) } throws
|
||||
UnsatisfiedLinkError("SQLiteConnection.nativeOpen")
|
||||
|
||||
val error = assertFailsWith<CacheEncryptionUnavailableException> { provisioner().prepareCache() }
|
||||
|
||||
assertTrue(
|
||||
generateSequence(error.cause) { it.cause }.any { it is LinkageError },
|
||||
"the nativeOpen LinkageError must be preserved as the cause",
|
||||
)
|
||||
// The library loaded fine (loadLibrary was not the failure); it was the keyed nativeOpen probe.
|
||||
verify(exactly = 1) { DatabaseEncryption.ensureNativeLibraryLoaded() }
|
||||
verify(exactly = 1) { DatabaseEncryption.probeKeyedOpen(any(), PASSPHRASE) }
|
||||
// No fail-open side effects.
|
||||
verify(exactly = 0) { DatabaseFiles.clear(any()) }
|
||||
coVerify(exactly = 0) { keyStore.resetSealedPassphrase() }
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a native-library load failure is not memoized and retries on the next open`() = runTest {
|
||||
every { settingsRepository.settings } returns flowOf(AppSettings(encryptCache = true, appLock = false))
|
||||
@@ -237,8 +289,10 @@ class DatabaseProvisionerTest {
|
||||
verify(exactly = 0) { DatabaseEncryption.ensureEncrypted(any(), any()) }
|
||||
verify(exactly = 0) { DatabaseEncryption.ensurePlaintext(any(), any()) }
|
||||
// A plaintext cache opens with the framework helper, never SQLCipher, so it must not touch the
|
||||
// native library — the counterpart to the encrypted path's mandatory load above.
|
||||
// native library — the counterpart to the encrypted path's mandatory load above — nor probe a
|
||||
// keyed open (issue #359: the probe belongs to the encrypted branch only).
|
||||
verify(exactly = 0) { DatabaseEncryption.ensureNativeLibraryLoaded() }
|
||||
verify(exactly = 0) { DatabaseEncryption.probeKeyedOpen(any(), any()) }
|
||||
}
|
||||
|
||||
@Test
|
||||
|
||||
@@ -2,17 +2,23 @@
|
||||
package org.libremail.data.repository
|
||||
|
||||
import android.content.Context
|
||||
import android.util.Log
|
||||
import app.cash.turbine.test
|
||||
import io.mockk.Runs
|
||||
import io.mockk.coEvery
|
||||
import io.mockk.coVerify
|
||||
import io.mockk.coVerifyOrder
|
||||
import io.mockk.every
|
||||
import io.mockk.just
|
||||
import io.mockk.mockk
|
||||
import io.mockk.mockkStatic
|
||||
import io.mockk.slot
|
||||
import io.mockk.unmockkAll
|
||||
import io.mockk.verify
|
||||
import kotlinx.coroutines.flow.flowOf
|
||||
import kotlinx.coroutines.test.runTest
|
||||
import org.junit.After
|
||||
import org.junit.Before
|
||||
import org.junit.Test
|
||||
import org.libremail.data.attachment.AttachmentUriGrants
|
||||
import org.libremail.data.attachmentCacheDir
|
||||
@@ -76,6 +82,17 @@ class AccountRepositoryImplTest {
|
||||
attachmentUriGrants = attachmentUriGrants,
|
||||
)
|
||||
|
||||
// addImapAccount/addOutlookAccount now breadcrumb via AppLog (#403); android.util.Log is a no-op
|
||||
// stub under plain JVM tests, so mock it class-wide so no test crashes on the unmocked method.
|
||||
@Before
|
||||
fun setUp() {
|
||||
mockkStatic(Log::class)
|
||||
every { Log.i(any(), any()) } returns 0
|
||||
}
|
||||
|
||||
@After
|
||||
fun tearDown() = unmockkAll()
|
||||
|
||||
@Test
|
||||
fun `observeAccounts maps the stored account rows to domain models`() = runTest {
|
||||
every { accountDao.observeAll() } returns flowOf(listOf(accountEntity()))
|
||||
@@ -134,6 +151,40 @@ class AccountRepositoryImplTest {
|
||||
verify { syncScheduler.backfillNow() }
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `addImapAccount persists the credential before the account row (issue 403)`() = runTest {
|
||||
val account = account()
|
||||
coEvery { imapClient.listFolders(any()) } returns listOf(
|
||||
FetchedFolder("INBOX", "INBOX", emptyList(), selectable = true),
|
||||
)
|
||||
coEvery { accountDao.insertAtEnd(any()) } just Runs
|
||||
|
||||
repository.addImapAccount(account, "app-password").getOrThrow()
|
||||
|
||||
// The push collector (LibreMailApplication) and IdleService.reconcileWatchers both react to the
|
||||
// accounts table; the secret must be committed FIRST so a watcher observing the new row can
|
||||
// resolve it, rather than logging a transient "No stored credentials" IDLE miss on every add.
|
||||
coVerifyOrder {
|
||||
credentialStore.saveSecret(account.id, "app-password")
|
||||
accountDao.insertAtEnd(any())
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `addOutlookAccount persists the credential before the account row (issue 403)`() = runTest {
|
||||
coEvery { imapClient.listFolders(any()) } returns listOf(
|
||||
FetchedFolder("INBOX", "INBOX", emptyList(), selectable = true),
|
||||
)
|
||||
coEvery { accountDao.insertAtEnd(any()) } just Runs
|
||||
|
||||
repository.addOutlookAccount("me@outlook.com", "access-token", "{authstate}").getOrThrow()
|
||||
|
||||
coVerifyOrder {
|
||||
credentialStore.saveSecret("outlook:me@outlook.com", "{authstate}")
|
||||
accountDao.insertAtEnd(any())
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `addImapAccount persists nothing when the initial folder list fails`() = runTest {
|
||||
coEvery { imapClient.listFolders(any()) } throws RuntimeException("bad credentials")
|
||||
|
||||
@@ -15,6 +15,7 @@ import kotlinx.coroutines.test.runTest
|
||||
import org.junit.After
|
||||
import org.junit.Before
|
||||
import org.junit.Test
|
||||
import org.libremail.data.local.CacheEncryptionUnavailableException
|
||||
import org.libremail.data.security.EncryptedCacheGuard
|
||||
import org.libremail.reporting.AppLog
|
||||
import org.libremail.reporting.RingLogBuffer
|
||||
@@ -86,6 +87,22 @@ class BackfillWorkerTest {
|
||||
assertEquals(Result.retry(), worker().doWork())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `defers with a retry and a distinct breadcrumb when the encrypted cache is unavailable`() = runTest {
|
||||
// Issue #359: the DB open fails because SQLCipher's native library is unavailable. It lands in the
|
||||
// worker's runCatching (thrown inside runBackfill()), so it retries — but it must now log a DISTINCT
|
||||
// breadcrumb (not the generic retry, and not mistaken for a cancellation) and still never crash.
|
||||
coEvery { cacheGuard.isCacheLocked() } returns false
|
||||
coEvery { backfiller.runBackfill(any()) } throws
|
||||
CacheEncryptionUnavailableException(UnsatisfiedLinkError("SQLiteConnection.nativeOpen"))
|
||||
|
||||
assertEquals(Result.retry(), worker().doWork())
|
||||
|
||||
val entry = logBuffer.snapshot().single()
|
||||
assertEquals('W', entry.level)
|
||||
assertTrue(entry.message.startsWith("backfill deferred: encrypted cache unavailable"), entry.message)
|
||||
}
|
||||
|
||||
// --- issue #329: AppLog breadcrumbs ---------------------------------------------------------
|
||||
|
||||
@Test
|
||||
|
||||
@@ -80,7 +80,9 @@ class MailConnectionFactoryTest {
|
||||
fun `a missing password credential is a hard error`() = runTest {
|
||||
coEvery { credentialStore.loadSecret("acct") } returns null
|
||||
|
||||
assertFailsWith<IllegalStateException> { factory().imapParamsFor(passwordAccount) }
|
||||
// A typed MissingCredentialsException (#403), not a bare IllegalStateException, so the IMAP IDLE
|
||||
// watcher can catch it specifically and defer on the account-add write race.
|
||||
assertFailsWith<MissingCredentialsException> { factory().imapParamsFor(passwordAccount) }
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -170,6 +172,6 @@ class MailConnectionFactoryTest {
|
||||
fun `a missing OAuth credential is a hard error`() = runTest {
|
||||
coEvery { credentialStore.loadSecret(outlookAccount.id) } returns null
|
||||
|
||||
assertFailsWith<IllegalStateException> { factory().graphTokenFor(outlookAccount) }
|
||||
assertFailsWith<MissingCredentialsException> { factory().graphTokenFor(outlookAccount) }
|
||||
}
|
||||
}
|
||||
|
||||
@@ -15,6 +15,7 @@ import kotlinx.coroutines.test.runTest
|
||||
import org.junit.After
|
||||
import org.junit.Before
|
||||
import org.junit.Test
|
||||
import org.libremail.data.local.CacheEncryptionUnavailableException
|
||||
import org.libremail.data.security.EncryptedCacheGuard
|
||||
import org.libremail.reporting.AppLog
|
||||
import org.libremail.reporting.RingLogBuffer
|
||||
@@ -84,6 +85,23 @@ class PruneWorkerTest {
|
||||
assertEquals(Result.retry(), worker().doWork())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `defers with a retry and a distinct breadcrumb when the encrypted cache is unavailable`() = runTest {
|
||||
// Issue #359: the DB open fails because SQLCipher's native library is unavailable. It lands in the
|
||||
// worker's runCatching (thrown inside prune()), so it already retried — but it must now log a
|
||||
// DISTINCT breadcrumb so a debug report shows "encrypted cache unavailable" rather than a generic
|
||||
// retry, and still never crash.
|
||||
coEvery { cacheGuard.isCacheLocked() } returns false
|
||||
coEvery { pruner.prune(any()) } throws
|
||||
CacheEncryptionUnavailableException(UnsatisfiedLinkError("SQLiteConnection.nativeOpen"))
|
||||
|
||||
assertEquals(Result.retry(), worker().doWork())
|
||||
|
||||
val entry = logBuffer.snapshot().single()
|
||||
assertEquals('W', entry.level)
|
||||
assertTrue(entry.message.startsWith("prune deferred: encrypted cache unavailable"), entry.message)
|
||||
}
|
||||
|
||||
// --- issue #329: AppLog breadcrumbs ---------------------------------------------------------
|
||||
|
||||
@Test
|
||||
|
||||
@@ -19,6 +19,7 @@ import org.junit.After
|
||||
import org.junit.Before
|
||||
import org.junit.Test
|
||||
import org.libremail.data.attachment.AttachmentUriGrants
|
||||
import org.libremail.data.local.CacheEncryptionUnavailableException
|
||||
import org.libremail.data.local.dao.AccountDao
|
||||
import org.libremail.data.local.dao.OutboxDao
|
||||
import org.libremail.data.local.entity.AccountEntity
|
||||
@@ -139,6 +140,21 @@ class SendWorkerTest {
|
||||
verify(exactly = 0) { lazyGrants.get() }
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `defers with a retry when the encrypted cache is unavailable, without crashing`() = runTest {
|
||||
// Cache unlocked (setUp), so the worker resolves its Lazy DB deps and reads the outbox; that first
|
||||
// DB access (Room's deferred cache open) throws because SQLCipher's native library is unavailable
|
||||
// on this device (issue #359). It is OUTSIDE any per-message runCatching, so without the guard it
|
||||
// would escape doWork — the guard turns it into a soft retry with a PII-free breadcrumb.
|
||||
coEvery { outboxDao.getAll() } throws
|
||||
CacheEncryptionUnavailableException(UnsatisfiedLinkError("SQLiteConnection.nativeOpen"))
|
||||
|
||||
assertEquals(Result.retry(), worker().doWork())
|
||||
|
||||
val messages = logBuffer.snapshot().map { it.message }
|
||||
assertTrue(messages.any { it.startsWith("deferred: encrypted cache unavailable") }, "messages=$messages")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `an empty outbox succeeds without sending`() = runTest {
|
||||
coEvery { outboxDao.getAll() } returns emptyList()
|
||||
|
||||
@@ -15,6 +15,7 @@ import kotlinx.coroutines.test.runTest
|
||||
import org.junit.After
|
||||
import org.junit.Before
|
||||
import org.junit.Test
|
||||
import org.libremail.data.local.CacheEncryptionUnavailableException
|
||||
import org.libremail.data.security.EncryptedCacheGuard
|
||||
import org.libremail.reporting.AppLog
|
||||
import org.libremail.reporting.RingLogBuffer
|
||||
@@ -82,6 +83,24 @@ class SyncWorkerTest {
|
||||
assertEquals(ListenableWorker.Result.retry(), worker().doWork())
|
||||
}
|
||||
|
||||
// --- issue #359: headless tolerance when SQLCipher's native library is unavailable ------------
|
||||
|
||||
@Test
|
||||
fun `defers with a retry when the encrypted cache is unavailable, without crashing`() = runTest {
|
||||
coEvery { cacheGuard.isCacheLocked() } returns false
|
||||
// The cache is unlocked, so the worker resolves MailSyncer; its first DB access (Room's deferred
|
||||
// cache open) throws because SQLCipher's native library is unavailable on this device (issue #359).
|
||||
// That open is OUTSIDE syncAll's own runCatching, so without the guard it would escape doWork.
|
||||
coEvery { mailSyncer.syncAll() } throws
|
||||
CacheEncryptionUnavailableException(UnsatisfiedLinkError("SQLiteConnection.nativeOpen"))
|
||||
|
||||
assertEquals(ListenableWorker.Result.retry(), worker().doWork())
|
||||
|
||||
val entry = logBuffer.snapshot().single()
|
||||
assertEquals('W', entry.level)
|
||||
assertTrue(entry.message.startsWith("deferred: encrypted cache unavailable"), entry.message)
|
||||
}
|
||||
|
||||
// --- issue #329: AppLog breadcrumbs ---------------------------------------------------------
|
||||
|
||||
@Test
|
||||
|
||||
@@ -101,6 +101,7 @@ class AccountPickerScreenJvmTest {
|
||||
onAccountAdded: (String) -> Unit = {},
|
||||
onPickProvider: (MailProvider) -> Unit = {},
|
||||
onManualSetup: () -> Unit = {},
|
||||
onPickOutlook: (() -> Unit)? = null,
|
||||
) {
|
||||
composeTestRule.setContent {
|
||||
CompositionLocalProvider(
|
||||
@@ -113,6 +114,7 @@ class AccountPickerScreenJvmTest {
|
||||
onAccountAdded = onAccountAdded,
|
||||
onPickProvider = onPickProvider,
|
||||
onManualSetup = onManualSetup,
|
||||
onPickOutlook = onPickOutlook,
|
||||
viewModel = viewModel,
|
||||
)
|
||||
}
|
||||
@@ -185,6 +187,20 @@ class AccountPickerScreenJvmTest {
|
||||
verify { vm.onOutlookLaunchFailed(any()) }
|
||||
}
|
||||
|
||||
@Test
|
||||
fun tappingOutlook_whenOnPickOutlookProvided_delegatesInsteadOfLaunching() {
|
||||
// Onboarding passes onPickOutlook to interpose the IMAP-enablement notice (#411): the tap
|
||||
// must route there, NOT build/launch the auth intent inline.
|
||||
val vm = viewModel()
|
||||
var outlookPicked = false
|
||||
setContent(vm, onPickOutlook = { outlookPicked = true })
|
||||
|
||||
composeTestRule.onNodeWithText(string(R.string.account_setup_outlook)).performClick()
|
||||
|
||||
assertTrue("Outlook tap must delegate to onPickOutlook", outlookPicked)
|
||||
verify(exactly = 0) { vm.outlookAuthIntent() }
|
||||
}
|
||||
|
||||
@Test
|
||||
fun doneStatus_reportsTheNewAccountIdToOnAccountAdded() {
|
||||
var addedId: String? = null
|
||||
|
||||
@@ -0,0 +1,246 @@
|
||||
// SPDX-License-Identifier: GPL-3.0-or-later
|
||||
package org.libremail.ui.onboarding
|
||||
|
||||
import android.content.ActivityNotFoundException
|
||||
import android.content.Context
|
||||
import android.content.Intent
|
||||
import androidx.activity.compose.LocalActivityResultRegistryOwner
|
||||
import androidx.activity.result.ActivityResultRegistry
|
||||
import androidx.activity.result.ActivityResultRegistryOwner
|
||||
import androidx.activity.result.contract.ActivityResultContract
|
||||
import androidx.compose.runtime.CompositionLocalProvider
|
||||
import androidx.compose.ui.platform.LocalUriHandler
|
||||
import androidx.compose.ui.platform.UriHandler
|
||||
import androidx.compose.ui.semantics.ProgressBarRangeInfo
|
||||
import androidx.compose.ui.test.assertIsDisplayed
|
||||
import androidx.compose.ui.test.assertIsNotEnabled
|
||||
import androidx.compose.ui.test.hasProgressBarRangeInfo
|
||||
import androidx.compose.ui.test.junit4.v2.createComposeRule
|
||||
import androidx.compose.ui.test.onAllNodesWithText
|
||||
import androidx.compose.ui.test.onNodeWithContentDescription
|
||||
import androidx.compose.ui.test.onNodeWithText
|
||||
import androidx.compose.ui.test.performClick
|
||||
import androidx.core.app.ActivityOptionsCompat
|
||||
import androidx.lifecycle.Lifecycle
|
||||
import androidx.lifecycle.LifecycleOwner
|
||||
import androidx.lifecycle.LifecycleRegistry
|
||||
import androidx.lifecycle.compose.LocalLifecycleOwner
|
||||
import io.mockk.every
|
||||
import io.mockk.mockk
|
||||
import io.mockk.verify
|
||||
import kotlinx.coroutines.flow.MutableStateFlow
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Rule
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.libremail.R
|
||||
import org.libremail.ui.accountsetup.AccountSetupUiState
|
||||
import org.libremail.ui.accountsetup.AccountSetupViewModel
|
||||
import org.libremail.ui.accountsetup.SetupStatus
|
||||
import org.libremail.ui.theme.LibreMailTheme
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
import org.robolectric.RuntimeEnvironment
|
||||
import org.robolectric.annotation.Config
|
||||
import org.robolectric.annotation.GraphicsMode
|
||||
|
||||
/**
|
||||
* Robolectric JVM Compose test for the pre-auth Outlook IMAP-enablement notice (#411). Drives the
|
||||
* real [OutlookImapNoticeScreen] on the JVM via the v2 `createComposeRule()` under
|
||||
* [RobolectricTestRunner] — no emulator — so the screen counts toward JaCoCo's JVM-testable surface.
|
||||
* The instrumented [OutlookImapNoticeScreenTest] stays as the on-device E2E (with Espresso-Intents it
|
||||
* owns the real browser/AppAuth launch this JVM rule cannot safely surface).
|
||||
*
|
||||
* [AccountSetupViewModel] is mocked (its own logic is covered by `AccountSetupViewModelTest`); a
|
||||
* recording [UriHandler] captures the help/settings link launches and a no-op
|
||||
* [ActivityResultRegistry] lets the "Sign in" launcher register/launch without a real activity.
|
||||
*/
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
@GraphicsMode(GraphicsMode.Mode.NATIVE)
|
||||
// A tall display so the whole scrolling column fits Robolectric's small default viewport, keeping
|
||||
// every control on-screen for assertIsDisplayed / performClick without scrolling.
|
||||
@Config(sdk = [36], qualifiers = "+w411dp-h2000dp")
|
||||
class OutlookImapNoticeScreenJvmTest {
|
||||
|
||||
@get:Rule
|
||||
val composeTestRule = createComposeRule()
|
||||
|
||||
private val context: Context get() = RuntimeEnvironment.getApplication()
|
||||
|
||||
private fun string(resId: Int): String = context.getString(resId)
|
||||
|
||||
/** RESUMED owner so `collectAsStateWithLifecycle` collects the view-model state. */
|
||||
private val resumedOwner = object : LifecycleOwner {
|
||||
private val registry =
|
||||
LifecycleRegistry.createUnsafe(this).apply { currentState = Lifecycle.State.RESUMED }
|
||||
override val lifecycle: Lifecycle get() = registry
|
||||
}
|
||||
|
||||
/** A no-op registry so the "Sign in" launcher can register/launch without a real activity. */
|
||||
private val noopRegistryOwner = object : ActivityResultRegistryOwner {
|
||||
override val activityResultRegistry = object : ActivityResultRegistry() {
|
||||
override fun <I, O> onLaunch(
|
||||
requestCode: Int,
|
||||
contract: ActivityResultContract<I, O>,
|
||||
input: I,
|
||||
options: ActivityOptionsCompat?,
|
||||
) {
|
||||
// Intentionally never dispatch a result: the launch is a no-op in this JVM test.
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/** Records outbound link launches so the screen's help/settings links are exercised. */
|
||||
private val openedUrls = mutableListOf<String>()
|
||||
private val recordingUriHandler = object : UriHandler {
|
||||
override fun openUri(uri: String) {
|
||||
openedUrls.add(uri)
|
||||
}
|
||||
}
|
||||
|
||||
/** A handler that always fails, to drive the "couldn't open your browser" snackbar path. */
|
||||
private val throwingUriHandler = object : UriHandler {
|
||||
override fun openUri(uri: String): Unit = throw ActivityNotFoundException("no browser")
|
||||
}
|
||||
|
||||
private fun viewModel(state: AccountSetupUiState = AccountSetupUiState()): AccountSetupViewModel {
|
||||
val vm = mockk<AccountSetupViewModel>(relaxed = true)
|
||||
every { vm.state } returns MutableStateFlow(state)
|
||||
return vm
|
||||
}
|
||||
|
||||
private fun setContent(
|
||||
viewModel: AccountSetupViewModel,
|
||||
uriHandler: UriHandler = recordingUriHandler,
|
||||
onBack: () -> Unit = {},
|
||||
onAccountAdded: (String) -> Unit = {},
|
||||
) {
|
||||
composeTestRule.setContent {
|
||||
CompositionLocalProvider(
|
||||
LocalLifecycleOwner provides resumedOwner,
|
||||
LocalActivityResultRegistryOwner provides noopRegistryOwner,
|
||||
LocalUriHandler provides uriHandler,
|
||||
) {
|
||||
LibreMailTheme(darkTheme = false, dynamicColor = false) {
|
||||
OutlookImapNoticeScreen(
|
||||
onBack = onBack,
|
||||
onAccountAdded = onAccountAdded,
|
||||
viewModel = viewModel,
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun rendersImapQuestion_bothLinks_andSignIn() {
|
||||
setContent(viewModel())
|
||||
|
||||
composeTestRule.onNodeWithText(string(R.string.outlook_imap_title)).assertIsDisplayed()
|
||||
composeTestRule.onNodeWithText(string(R.string.outlook_imap_question)).assertIsDisplayed()
|
||||
composeTestRule.onNodeWithText(string(R.string.outlook_imap_body)).assertIsDisplayed()
|
||||
composeTestRule.onNodeWithText(string(R.string.outlook_imap_help)).assertIsDisplayed()
|
||||
composeTestRule.onNodeWithText(string(R.string.outlook_imap_settings)).assertIsDisplayed()
|
||||
composeTestRule.onNodeWithText(string(R.string.outlook_imap_sign_in)).assertIsDisplayed()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun tappingHelpLink_opensTheMicrosoftArticle() {
|
||||
setContent(viewModel())
|
||||
|
||||
composeTestRule.onNodeWithText(string(R.string.outlook_imap_help)).performClick()
|
||||
|
||||
assertTrue(
|
||||
"Help link must open a support.microsoft.com article",
|
||||
openedUrls.any { it.startsWith("https://support.microsoft.com/") },
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun tappingSettingsLink_opensOutlookSettings() {
|
||||
setContent(viewModel())
|
||||
|
||||
composeTestRule.onNodeWithText(string(R.string.outlook_imap_settings)).performClick()
|
||||
|
||||
assertTrue(
|
||||
"Settings link must open an outlook.live.com settings page",
|
||||
openedUrls.any { it.startsWith("https://outlook.live.com/") },
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun tappingSignIn_buildsTheAuthIntentAndLaunchesIt() {
|
||||
val vm = viewModel()
|
||||
// A real (empty) Intent is safe here: the no-op registry never actually starts it.
|
||||
every { vm.outlookAuthIntent() } returns Result.success(Intent())
|
||||
setContent(vm)
|
||||
|
||||
composeTestRule.onNodeWithText(string(R.string.outlook_imap_sign_in)).performClick()
|
||||
|
||||
verify { vm.outlookAuthIntent() }
|
||||
}
|
||||
|
||||
@Test
|
||||
fun tappingSignIn_whenTheIntentCannotBeBuilt_reportsLaunchFailure() {
|
||||
val vm = viewModel()
|
||||
every { vm.outlookAuthIntent() } returns Result.failure(ActivityNotFoundException("no browser"))
|
||||
setContent(vm)
|
||||
|
||||
composeTestRule.onNodeWithText(string(R.string.outlook_imap_sign_in)).performClick()
|
||||
|
||||
verify { vm.onOutlookLaunchFailed(any()) }
|
||||
}
|
||||
|
||||
@Test
|
||||
fun doneStatus_reportsTheNewAccountIdToOnAccountAdded() {
|
||||
var addedId: String? = null
|
||||
setContent(
|
||||
viewModel(AccountSetupUiState(status = SetupStatus.DONE, addedAccountId = "outlook:me@outlook.com")),
|
||||
onAccountAdded = { addedId = it },
|
||||
)
|
||||
|
||||
composeTestRule.waitUntil(5_000) { addedId != null }
|
||||
assertEquals("outlook:me@outlook.com", addedId)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun anError_isSurfacedAsASnackbar() {
|
||||
setContent(viewModel(AccountSetupUiState(error = "Microsoft sign-in failed")))
|
||||
|
||||
composeTestRule.waitUntil(5_000) {
|
||||
composeTestRule.onAllNodesWithText("Microsoft sign-in failed").fetchSemanticsNodes().isNotEmpty()
|
||||
}
|
||||
composeTestRule.onNodeWithText("Microsoft sign-in failed").assertIsDisplayed()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun connectingStatus_showsBusySpinner_andDisablesSignIn() {
|
||||
setContent(viewModel(AccountSetupUiState(status = SetupStatus.CONNECTING)))
|
||||
|
||||
composeTestRule.onNode(hasProgressBarRangeInfo(ProgressBarRangeInfo.Indeterminate)).assertIsDisplayed()
|
||||
composeTestRule.onNodeWithText(string(R.string.outlook_imap_sign_in)).assertIsNotEnabled()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun tappingBack_invokesOnBack() {
|
||||
var backed = false
|
||||
setContent(viewModel(), onBack = { backed = true })
|
||||
|
||||
composeTestRule.onNodeWithContentDescription(string(R.string.action_back)).performClick()
|
||||
|
||||
assertTrue(backed)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun openingALink_whenNoBrowser_surfacesTheOpenFailedSnackbar() {
|
||||
setContent(viewModel(), uriHandler = throwingUriHandler)
|
||||
|
||||
composeTestRule.onNodeWithText(string(R.string.outlook_imap_help)).performClick()
|
||||
|
||||
val openFailed = string(R.string.app_password_open_failed)
|
||||
composeTestRule.waitUntil(5_000) {
|
||||
composeTestRule.onAllNodesWithText(openFailed).fetchSemanticsNodes().isNotEmpty()
|
||||
}
|
||||
composeTestRule.onNodeWithText(openFailed).assertIsDisplayed()
|
||||
}
|
||||
}
|
||||
@@ -74,6 +74,9 @@ style:
|
||||
# log via AppLog, so their unit tests mockkStatic(Log) too.
|
||||
- '**/data/repository/MailRepositoryImplTest.kt'
|
||||
- '**/data/repository/MailRepositoryImplCoverageTest.kt'
|
||||
# Account-add breadcrumb (issue #403): addImapAccount/addOutlookAccount log via AppLog, so this
|
||||
# suite mockkStatic(Log) so the calls don't crash on the throwing JVM stub.
|
||||
- '**/data/repository/AccountRepositoryImplTest.kt'
|
||||
- '**/ui/reader/ReaderViewModelTest.kt'
|
||||
- '**/ui/reader/ReaderViewModelActionsTest.kt'
|
||||
MagicNumber:
|
||||
|
||||
Reference in New Issue
Block a user