diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 25905b2..345c70e 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -34,6 +34,10 @@ val outlookOAuthClientId: String = secrets.getProperty( "04e4aa5e-ed1f-47f9-b567-b99a0b29b3df", ) +// Optional release signing, configured via git-ignored secrets.properties. When absent, release +// builds fall back to the debug key (installable for testing, but not publishable). +val releaseStoreFile: String? = secrets.getProperty("RELEASE_STORE_FILE") + android { namespace = "org.libremail" compileSdk = 37 @@ -55,6 +59,17 @@ android { manifestPlaceholders["appAuthRedirectScheme"] = gmailRedirectScheme } + signingConfigs { + if (releaseStoreFile != null) { + create("release") { + storeFile = file(releaseStoreFile) + storePassword = secrets.getProperty("RELEASE_STORE_PASSWORD") + keyAlias = secrets.getProperty("RELEASE_KEY_ALIAS") + keyPassword = secrets.getProperty("RELEASE_KEY_PASSWORD") + } + } + } + buildTypes { release { isMinifyEnabled = true @@ -62,9 +77,13 @@ android { getDefaultProguardFile("proguard-android-optimize.txt"), "proguard-rules.pro", ) - // Sign release builds with the debug key so they're installable for testing. - // A public release would configure a dedicated upload/release keystore here. - signingConfig = signingConfigs.getByName("debug") + // Use a dedicated release keystore when configured in secrets.properties; otherwise fall + // back to the debug key so the build is still installable for local testing. + signingConfig = if (releaseStoreFile != null) { + signingConfigs.getByName("release") + } else { + signingConfigs.getByName("debug") + } } } @@ -87,6 +106,11 @@ android { } } +// Export Room schemas so migrations can be validated by instrumented MigrationTestHelper tests. +ksp { + arg("room.schemaLocation", "$projectDir/schemas") +} + dependencies { implementation(libs.androidx.core.ktx) implementation(libs.androidx.lifecycle.runtime.ktx) diff --git a/app/schemas/org.libremail.data.local.LibreMailDatabase/7.json b/app/schemas/org.libremail.data.local.LibreMailDatabase/7.json new file mode 100644 index 0000000..1da2740 --- /dev/null +++ b/app/schemas/org.libremail.data.local.LibreMailDatabase/7.json @@ -0,0 +1,404 @@ +{ + "formatVersion": 1, + "database": { + "version": 7, + "identityHash": "05d29b502db9e50c01ddcdf986be1660", + "entities": [ + { + "tableName": "accounts", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` TEXT NOT NULL, `email` TEXT NOT NULL, `displayName` TEXT NOT NULL, `authType` TEXT NOT NULL, `imap_host` TEXT NOT NULL, `imap_port` INTEGER NOT NULL, `imap_security` TEXT NOT NULL, `smtp_host` TEXT NOT NULL, `smtp_port` INTEGER NOT NULL, `smtp_security` TEXT NOT NULL, PRIMARY KEY(`id`))", + "fields": [ + { + "fieldPath": "id", + "columnName": "id", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "email", + "columnName": "email", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "displayName", + "columnName": "displayName", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "authType", + "columnName": "authType", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "imap.host", + "columnName": "imap_host", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "imap.port", + "columnName": "imap_port", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "imap.security", + "columnName": "imap_security", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "smtp.host", + "columnName": "smtp_host", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "smtp.port", + "columnName": "smtp_port", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "smtp.security", + "columnName": "smtp_security", + "affinity": "TEXT", + "notNull": true + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "id" + ] + } + }, + { + "tableName": "messages", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` TEXT NOT NULL, `accountId` TEXT NOT NULL, `sender` TEXT NOT NULL, `senderEmail` TEXT NOT NULL, `subject` TEXT NOT NULL, `snippet` TEXT NOT NULL, `body` TEXT NOT NULL, `isHtml` INTEGER NOT NULL, `timestampMillis` INTEGER NOT NULL, `isRead` INTEGER NOT NULL, `isStarred` INTEGER NOT NULL, `inInbox` INTEGER NOT NULL, `bodyFetched` INTEGER NOT NULL, PRIMARY KEY(`id`))", + "fields": [ + { + "fieldPath": "id", + "columnName": "id", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "accountId", + "columnName": "accountId", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "sender", + "columnName": "sender", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "senderEmail", + "columnName": "senderEmail", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "subject", + "columnName": "subject", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "snippet", + "columnName": "snippet", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "body", + "columnName": "body", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "isHtml", + "columnName": "isHtml", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "timestampMillis", + "columnName": "timestampMillis", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "isRead", + "columnName": "isRead", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "isStarred", + "columnName": "isStarred", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "inInbox", + "columnName": "inInbox", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "bodyFetched", + "columnName": "bodyFetched", + "affinity": "INTEGER", + "notNull": true + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "id" + ] + }, + "indices": [ + { + "name": "index_messages_accountId", + "unique": false, + "columnNames": [ + "accountId" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_messages_accountId` ON `${TABLE_NAME}` (`accountId`)" + }, + { + "name": "index_messages_timestampMillis", + "unique": false, + "columnNames": [ + "timestampMillis" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_messages_timestampMillis` ON `${TABLE_NAME}` (`timestampMillis`)" + } + ] + }, + { + "tableName": "credentials", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`accountId` TEXT NOT NULL, `encryptedSecret` TEXT NOT NULL, PRIMARY KEY(`accountId`))", + "fields": [ + { + "fieldPath": "accountId", + "columnName": "accountId", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "encryptedSecret", + "columnName": "encryptedSecret", + "affinity": "TEXT", + "notNull": true + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "accountId" + ] + } + }, + { + "tableName": "attachments", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`messageId` TEXT NOT NULL, `partIndex` INTEGER NOT NULL, `filename` TEXT NOT NULL, `mimeType` TEXT NOT NULL, `sizeBytes` INTEGER NOT NULL, PRIMARY KEY(`messageId`, `partIndex`), FOREIGN KEY(`messageId`) REFERENCES `messages`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE )", + "fields": [ + { + "fieldPath": "messageId", + "columnName": "messageId", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "partIndex", + "columnName": "partIndex", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "filename", + "columnName": "filename", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "mimeType", + "columnName": "mimeType", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "sizeBytes", + "columnName": "sizeBytes", + "affinity": "INTEGER", + "notNull": true + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "messageId", + "partIndex" + ] + }, + "indices": [ + { + "name": "index_attachments_messageId", + "unique": false, + "columnNames": [ + "messageId" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_attachments_messageId` ON `${TABLE_NAME}` (`messageId`)" + } + ], + "foreignKeys": [ + { + "table": "messages", + "onDelete": "CASCADE", + "onUpdate": "NO ACTION", + "columns": [ + "messageId" + ], + "referencedColumns": [ + "id" + ] + } + ] + }, + { + "tableName": "outbox", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` TEXT NOT NULL, `accountId` TEXT NOT NULL, `toAddresses` TEXT NOT NULL, `ccAddresses` TEXT NOT NULL, `subject` TEXT NOT NULL, `body` TEXT NOT NULL, `createdAt` INTEGER NOT NULL, `lastError` TEXT, PRIMARY KEY(`id`))", + "fields": [ + { + "fieldPath": "id", + "columnName": "id", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "accountId", + "columnName": "accountId", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "toAddresses", + "columnName": "toAddresses", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "ccAddresses", + "columnName": "ccAddresses", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "subject", + "columnName": "subject", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "body", + "columnName": "body", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "createdAt", + "columnName": "createdAt", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "lastError", + "columnName": "lastError", + "affinity": "TEXT" + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "id" + ] + } + }, + { + "tableName": "drafts", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` TEXT NOT NULL, `accountId` TEXT, `toAddresses` TEXT NOT NULL, `ccAddresses` TEXT NOT NULL, `subject` TEXT NOT NULL, `body` TEXT NOT NULL, `updatedAt` INTEGER NOT NULL, `attachments` TEXT NOT NULL, PRIMARY KEY(`id`))", + "fields": [ + { + "fieldPath": "id", + "columnName": "id", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "accountId", + "columnName": "accountId", + "affinity": "TEXT" + }, + { + "fieldPath": "toAddresses", + "columnName": "toAddresses", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "ccAddresses", + "columnName": "ccAddresses", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "subject", + "columnName": "subject", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "body", + "columnName": "body", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "updatedAt", + "columnName": "updatedAt", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "attachments", + "columnName": "attachments", + "affinity": "TEXT", + "notNull": true + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "id" + ] + } + } + ], + "setupQueries": [ + "CREATE TABLE IF NOT EXISTS room_master_table (id INTEGER PRIMARY KEY,identity_hash TEXT)", + "INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, '05d29b502db9e50c01ddcdf986be1660')" + ] + } +} \ No newline at end of file diff --git a/app/src/androidTest/kotlin/org/libremail/data/local/LibreMailDatabaseTest.kt b/app/src/androidTest/kotlin/org/libremail/data/local/LibreMailDatabaseTest.kt new file mode 100644 index 0000000..0c106c9 --- /dev/null +++ b/app/src/androidTest/kotlin/org/libremail/data/local/LibreMailDatabaseTest.kt @@ -0,0 +1,79 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.local + +import android.content.Context +import androidx.room.Room +import androidx.test.core.app.ApplicationProvider +import androidx.test.ext.junit.runners.AndroidJUnit4 +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.runBlocking +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.libremail.data.local.entity.AttachmentEntity +import org.libremail.data.local.entity.MessageEntity + +/** + * Schema-behavior tests on the real (v7) Room database. (Migrations from versions before + * exportSchema was enabled can't be replayed with MigrationTestHelper, since their schema JSONs + * were never exported; exportSchema is now on so future migrations can be tested.) + */ +@RunWith(AndroidJUnit4::class) +class LibreMailDatabaseTest { + + private lateinit var db: LibreMailDatabase + + @Before + fun setUp() { + val context = ApplicationProvider.getApplicationContext() + db = Room.inMemoryDatabaseBuilder(context, LibreMailDatabase::class.java).build() + } + + @After + fun tearDown() = db.close() + + private fun message(id: String, body: String = "") = MessageEntity( + id = id, + accountId = "acct", + sender = "Ada", + senderEmail = "ada@example.org", + subject = "Hi", + snippet = "", + body = body, + timestampMillis = 1_000L, + isRead = false, + isStarred = false, + ) + + @Test + fun deletingMessageCascadesToItsAttachments() = runBlocking { + val messageDao = db.messageDao() + val attachmentDao = db.attachmentDao() + messageDao.insertNew(listOf(message("acct:1"))) + attachmentDao.insert(listOf(AttachmentEntity("acct:1", 0, "report.pdf", "application/pdf", 10))) + assertEquals(1, attachmentDao.observeForMessage("acct:1").first().size) + + messageDao.deleteById("acct:1") + + assertTrue( + "attachment rows must cascade-delete with their message", + attachmentDao.observeForMessage("acct:1").first().isEmpty(), + ) + } + + @Test + fun searchRowsAreNotInboxAndAreCleared() = runBlocking { + val messageDao = db.messageDao() + messageDao.insertNew(listOf(message("acct:1").copy(inInbox = true))) + messageDao.insertNew(listOf(message("acct:2").copy(inInbox = false))) + + assertEquals(listOf("acct:1"), messageDao.getInboxIdsForAccount("acct")) + + messageDao.deleteSearchRows() + val remaining = messageDao.observeAll().first().map { it.id } + assertEquals(listOf("acct:1"), remaining) + } +} diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index 958ae71..28a6672 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -11,7 +11,8 @@ pushEnabled && hasAccounts } .distinctUntilChanged() - .collect { active -> if (active) idlePushManager.start() else idlePushManager.stop() } + .collect { active -> + pushShouldBeActive = active + if (active) idlePushManager.start() else idlePushManager.stop() + } } } + + /** + * Re-attempts starting the IDLE push service. A start from the background can be blocked + * (ForegroundServiceStartNotAllowedException) and is swallowed; because the active/inactive + * state hasn't changed, the collector above won't retry, so the foreground (MainActivity) calls + * this to recover. Safe to call repeatedly — starting an already-running service is a no-op. + */ + fun ensurePushStarted() { + if (pushShouldBeActive) idlePushManager.start() + } } diff --git a/app/src/main/kotlin/org/libremail/MainActivity.kt b/app/src/main/kotlin/org/libremail/MainActivity.kt index 5e7b9f7..069fd6a 100644 --- a/app/src/main/kotlin/org/libremail/MainActivity.kt +++ b/app/src/main/kotlin/org/libremail/MainActivity.kt @@ -27,6 +27,12 @@ class MainActivity : ComponentActivity() { @Inject lateinit var settingsRepository: SettingsRepository + override fun onStart() { + super.onStart() + // Foreground: recover IDLE push if a background start was previously blocked. + (application as? LibreMailApplication)?.ensurePushStarted() + } + override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) enableEdgeToEdge() diff --git a/app/src/main/kotlin/org/libremail/auth/GmailAuthManager.kt b/app/src/main/kotlin/org/libremail/auth/GmailAuthManager.kt index 153466c..3a32dba 100644 --- a/app/src/main/kotlin/org/libremail/auth/GmailAuthManager.kt +++ b/app/src/main/kotlin/org/libremail/auth/GmailAuthManager.kt @@ -93,7 +93,11 @@ class GmailAuthManager @Inject constructor( } } } - return FreshToken(accessToken = accessToken, authStateJson = authState.jsonSerializeString()) + return FreshToken( + accessToken = accessToken, + authStateJson = authState.jsonSerializeString(), + accessTokenExpiry = authState.accessTokenExpirationTime, + ) } finally { service.dispose() } diff --git a/app/src/main/kotlin/org/libremail/auth/OAuthResult.kt b/app/src/main/kotlin/org/libremail/auth/OAuthResult.kt index ca968a0..08cae16 100644 --- a/app/src/main/kotlin/org/libremail/auth/OAuthResult.kt +++ b/app/src/main/kotlin/org/libremail/auth/OAuthResult.kt @@ -13,4 +13,6 @@ data class OAuthResult( data class FreshToken( val accessToken: String, val authStateJson: String, + /** Epoch-millis expiry of [accessToken], when the provider reported one (for caching). */ + val accessTokenExpiry: Long? = null, ) diff --git a/app/src/main/kotlin/org/libremail/auth/OutlookAuthManager.kt b/app/src/main/kotlin/org/libremail/auth/OutlookAuthManager.kt index 6c635da..ed2cafd 100644 --- a/app/src/main/kotlin/org/libremail/auth/OutlookAuthManager.kt +++ b/app/src/main/kotlin/org/libremail/auth/OutlookAuthManager.kt @@ -121,6 +121,7 @@ class OutlookAuthManager @Inject constructor( return FreshToken( accessToken = tokenResponse.accessToken.orEmpty(), authStateJson = authState.jsonSerializeString(), + accessTokenExpiry = tokenResponse.accessTokenExpirationTime, ) } finally { service.dispose() diff --git a/app/src/main/kotlin/org/libremail/data/local/LibreMailDatabase.kt b/app/src/main/kotlin/org/libremail/data/local/LibreMailDatabase.kt index ab908ec..42dc24d 100644 --- a/app/src/main/kotlin/org/libremail/data/local/LibreMailDatabase.kt +++ b/app/src/main/kotlin/org/libremail/data/local/LibreMailDatabase.kt @@ -25,8 +25,8 @@ import org.libremail.data.local.entity.OutboxEntity OutboxEntity::class, DraftEntity::class, ], - version = 6, - exportSchema = false, + version = 7, + exportSchema = true, ) abstract class LibreMailDatabase : RoomDatabase() { abstract fun messageDao(): MessageDao diff --git a/app/src/main/kotlin/org/libremail/data/local/Mappers.kt b/app/src/main/kotlin/org/libremail/data/local/Mappers.kt index 4212ac5..9dcb167 100644 --- a/app/src/main/kotlin/org/libremail/data/local/Mappers.kt +++ b/app/src/main/kotlin/org/libremail/data/local/Mappers.kt @@ -7,6 +7,8 @@ import org.libremail.data.local.entity.DraftEntity import org.libremail.data.local.entity.MessageEntity import org.libremail.data.local.entity.OutboxEntity import org.libremail.data.local.entity.ServerConfigEmbedded +import org.json.JSONArray +import org.json.JSONObject import org.libremail.domain.model.Account import org.libremail.domain.model.Attachment import org.libremail.domain.model.Draft @@ -15,6 +17,7 @@ import org.libremail.domain.model.AuthType import org.libremail.domain.model.ImapConnectionParams import org.libremail.domain.model.MailSecurity import org.libremail.domain.model.Message +import org.libremail.domain.model.OutgoingAttachment import org.libremail.domain.model.ServerConfig import org.libremail.domain.model.SmtpParams import org.libremail.mail.AttachmentPart @@ -38,7 +41,11 @@ internal fun Account.toEntity(): AccountEntity = AccountEntity( smtp = ServerConfigEmbedded(smtp.host, smtp.port, smtp.security.name), ) -internal fun Account.toImapParams(secret: String, useXoauth2: Boolean): ImapConnectionParams = +internal fun Account.toImapParams( + secret: String, + useXoauth2: Boolean, + strictStartTls: Boolean = true, +): ImapConnectionParams = ImapConnectionParams( host = imap.host, port = imap.port, @@ -46,9 +53,14 @@ internal fun Account.toImapParams(secret: String, useXoauth2: Boolean): ImapConn username = email, secret = secret, useXoauth2 = useXoauth2, + strictStartTls = strictStartTls, ) -internal fun Account.toSmtpParams(secret: String, useXoauth2: Boolean): SmtpParams = +internal fun Account.toSmtpParams( + secret: String, + useXoauth2: Boolean, + strictStartTls: Boolean = true, +): SmtpParams = SmtpParams( host = smtp.host, port = smtp.port, @@ -56,6 +68,7 @@ internal fun Account.toSmtpParams(secret: String, useXoauth2: Boolean): SmtpPara username = email, secret = secret, useXoauth2 = useXoauth2, + strictStartTls = strictStartTls, ) internal fun MessageEntity.toDomain(): Message = Message( @@ -70,9 +83,10 @@ internal fun MessageEntity.toDomain(): Message = Message( timestampMillis = timestampMillis, isRead = isRead, isStarred = isStarred, + inInbox = inInbox, ) -internal fun FetchedMessage.toEntity(accountId: String): MessageEntity = MessageEntity( +internal fun FetchedMessage.toEntity(accountId: String, inInbox: Boolean = true): MessageEntity = MessageEntity( id = "$accountId:$uid", accountId = accountId, sender = sender, @@ -84,6 +98,8 @@ internal fun FetchedMessage.toEntity(accountId: String): MessageEntity = Message timestampMillis = timestampMillis, isRead = isRead, isStarred = isFlagged, + inInbox = inInbox, + bodyFetched = false, ) internal fun AttachmentEntity.toDomain(): Attachment = Attachment( @@ -110,6 +126,7 @@ internal fun DraftEntity.toDomain(): Draft = Draft( subject = subject, body = body, updatedAt = updatedAt, + attachments = attachments.toOutgoingAttachments(), ) internal fun Draft.toEntity(): DraftEntity = DraftEntity( @@ -120,8 +137,28 @@ internal fun Draft.toEntity(): DraftEntity = DraftEntity( subject = subject, body = body, updatedAt = updatedAt, + attachments = attachments.toJson(), ) +/** Serializes draft attachments as a JSON array of {uri, name} objects ("" when empty). */ +private fun List.toJson(): String { + if (isEmpty()) return "" + val array = JSONArray() + forEach { array.put(JSONObject().put("uri", it.uri).put("name", it.name)) } + return array.toString() +} + +private fun String.toOutgoingAttachments(): List { + if (isBlank()) return emptyList() + return runCatching { + val array = JSONArray(this) + (0 until array.length()).map { i -> + val obj = array.getJSONObject(i) + OutgoingAttachment(obj.getString("uri"), obj.optString("name")) + } + }.getOrDefault(emptyList()) +} + internal fun OutboxEntity.toDomain(): OutboxMessage = OutboxMessage( id = id, to = toAddresses, diff --git a/app/src/main/kotlin/org/libremail/data/local/Migrations.kt b/app/src/main/kotlin/org/libremail/data/local/Migrations.kt index bf119df..6f1dcb7 100644 --- a/app/src/main/kotlin/org/libremail/data/local/Migrations.kt +++ b/app/src/main/kotlin/org/libremail/data/local/Migrations.kt @@ -4,6 +4,17 @@ package org.libremail.data.local import androidx.room.migration.Migration import androidx.sqlite.db.SupportSQLiteDatabase +/** v1 -> v2: add the encrypted-credentials table (preserves existing accounts/messages). */ +val MIGRATION_1_2 = object : Migration(1, 2) { + override fun migrate(db: SupportSQLiteDatabase) { + db.execSQL( + "CREATE TABLE IF NOT EXISTS `credentials` (" + + "`accountId` TEXT NOT NULL, `encryptedSecret` TEXT NOT NULL, " + + "PRIMARY KEY(`accountId`))", + ) + } +} + /** v2 -> v3: add the [isHtml] flag to cached messages (preserves existing data). */ val MIGRATION_2_3 = object : Migration(2, 3) { override fun migrate(db: SupportSQLiteDatabase) { @@ -49,3 +60,70 @@ val MIGRATION_5_6 = object : Migration(5, 6) { ) } } + +/** + * v6 -> v7 (preserves existing data). Rebuilds three tables to converge on Room's canonical + * schema and add new columns: + * - `messages`: drop the stray `DEFAULT 0` that MIGRATION_2_3 left on `isHtml` (so upgraded and + * fresh installs validate identically), and add `inInbox` (server-search hits are kept out of + * the inbox) and `bodyFetched` (distinguishes "not fetched yet" from "fetched, empty body"). + * - `attachments`: add an ON DELETE CASCADE foreign key to `messages` so attachment rows can no + * longer be orphaned, dropping any pre-existing orphans in the process. + * - `drafts`: add the `attachments` column so a draft round-trips its attachments. + */ +val MIGRATION_6_7 = object : Migration(6, 7) { + override fun migrate(db: SupportSQLiteDatabase) { + // messages + db.execSQL( + "CREATE TABLE `messages_new` (" + + "`id` TEXT NOT NULL, `accountId` TEXT NOT NULL, `sender` TEXT NOT NULL, " + + "`senderEmail` TEXT NOT NULL, `subject` TEXT NOT NULL, `snippet` TEXT NOT NULL, " + + "`body` TEXT NOT NULL, `isHtml` INTEGER NOT NULL, `timestampMillis` INTEGER NOT NULL, " + + "`isRead` INTEGER NOT NULL, `isStarred` INTEGER NOT NULL, `inInbox` INTEGER NOT NULL, " + + "`bodyFetched` INTEGER NOT NULL, PRIMARY KEY(`id`))", + ) + db.execSQL( + "INSERT INTO `messages_new` " + + "SELECT id, accountId, sender, senderEmail, subject, snippet, body, isHtml, " + + "timestampMillis, isRead, isStarred, 1, (CASE WHEN body <> '' THEN 1 ELSE 0 END) " + + "FROM `messages`", + ) + db.execSQL("DROP TABLE `messages`") + db.execSQL("ALTER TABLE `messages_new` RENAME TO `messages`") + db.execSQL("CREATE INDEX `index_messages_accountId` ON `messages` (`accountId`)") + db.execSQL("CREATE INDEX `index_messages_timestampMillis` ON `messages` (`timestampMillis`)") + + // attachments (now with an ON DELETE CASCADE FK; orphans are dropped by the WHERE filter) + db.execSQL( + "CREATE TABLE `attachments_new` (" + + "`messageId` TEXT NOT NULL, `partIndex` INTEGER NOT NULL, `filename` TEXT NOT NULL, " + + "`mimeType` TEXT NOT NULL, `sizeBytes` INTEGER NOT NULL, " + + "PRIMARY KEY(`messageId`, `partIndex`), " + + "FOREIGN KEY(`messageId`) REFERENCES `messages`(`id`) " + + "ON UPDATE NO ACTION ON DELETE CASCADE)", + ) + db.execSQL( + "INSERT INTO `attachments_new` " + + "SELECT messageId, partIndex, filename, mimeType, sizeBytes FROM `attachments` " + + "WHERE messageId IN (SELECT id FROM `messages`)", + ) + db.execSQL("DROP TABLE `attachments`") + db.execSQL("ALTER TABLE `attachments_new` RENAME TO `attachments`") + db.execSQL("CREATE INDEX `index_attachments_messageId` ON `attachments` (`messageId`)") + + // drafts + db.execSQL( + "CREATE TABLE `drafts_new` (" + + "`id` TEXT NOT NULL, `accountId` TEXT, `toAddresses` TEXT NOT NULL, " + + "`ccAddresses` TEXT NOT NULL, `subject` TEXT NOT NULL, `body` TEXT NOT NULL, " + + "`updatedAt` INTEGER NOT NULL, `attachments` TEXT NOT NULL, PRIMARY KEY(`id`))", + ) + db.execSQL( + "INSERT INTO `drafts_new` " + + "SELECT id, accountId, toAddresses, ccAddresses, subject, body, updatedAt, '' " + + "FROM `drafts`", + ) + db.execSQL("DROP TABLE `drafts`") + db.execSQL("ALTER TABLE `drafts_new` RENAME TO `drafts`") + } +} diff --git a/app/src/main/kotlin/org/libremail/data/local/dao/AttachmentDao.kt b/app/src/main/kotlin/org/libremail/data/local/dao/AttachmentDao.kt index 1899b9e..b016c6e 100644 --- a/app/src/main/kotlin/org/libremail/data/local/dao/AttachmentDao.kt +++ b/app/src/main/kotlin/org/libremail/data/local/dao/AttachmentDao.kt @@ -20,10 +20,6 @@ interface AttachmentDao { @Query("DELETE FROM attachments WHERE messageId = :messageId") suspend fun deleteForMessage(messageId: String) - /** Deletes attachment rows for every message of an account (ids are "accountId:uid"). */ - @Query("DELETE FROM attachments WHERE messageId LIKE :accountPrefix") - suspend fun deleteByAccountPrefix(accountPrefix: String) - /** Replaces the cached attachment list for a message in one transaction. */ @Transaction suspend fun replaceForMessage(messageId: String, attachments: List) { diff --git a/app/src/main/kotlin/org/libremail/data/local/dao/MessageDao.kt b/app/src/main/kotlin/org/libremail/data/local/dao/MessageDao.kt index 446feb8..bf2fc39 100644 --- a/app/src/main/kotlin/org/libremail/data/local/dao/MessageDao.kt +++ b/app/src/main/kotlin/org/libremail/data/local/dao/MessageDao.kt @@ -16,29 +16,36 @@ interface MessageDao { @Query("SELECT * FROM messages WHERE id = :id LIMIT 1") suspend fun getById(id: String): MessageEntity? - @Query("SELECT id FROM messages WHERE accountId = :accountId") - suspend fun getIdsForAccount(accountId: String): List + /** Ids of an account's inbox rows (excludes transient server-search hits). */ + @Query("SELECT id FROM messages WHERE accountId = :accountId AND inInbox = 1") + suspend fun getInboxIdsForAccount(accountId: String): List - /** Inserts only new messages, leaving existing rows (and their cached bodies) intact. */ + /** Inserts only new messages, leaving existing rows (and their cached bodies/flags) intact. */ @Insert(onConflict = OnConflictStrategy.IGNORE) suspend fun insertNew(messages: List) - /** Refreshes header/flag columns from the server without touching the cached body. */ + /** + * Refreshes the display fields from the server without touching the cached body, the local + * read/star flags (which may hold an optimistic change the server hasn't reflected yet), or the + * inbox membership. + */ @Query( "UPDATE messages SET sender = :sender, senderEmail = :senderEmail, subject = :subject, " + - "timestampMillis = :timestampMillis, isRead = :isRead, isStarred = :isStarred WHERE id = :id", + "timestampMillis = :timestampMillis WHERE id = :id", ) - suspend fun updateHeader( + suspend fun updateHeaderContent( id: String, sender: String, senderEmail: String, subject: String, timestampMillis: Long, - isRead: Boolean, - isStarred: Boolean, ) - @Query("UPDATE messages SET body = :body, isHtml = :isHtml, snippet = :snippet WHERE id = :id") + /** Marks rows as belonging to the inbox (e.g. a former search-only row that the sync now returns). */ + @Query("UPDATE messages SET inInbox = 1 WHERE id IN (:ids)") + suspend fun markInInbox(ids: List) + + @Query("UPDATE messages SET body = :body, isHtml = :isHtml, snippet = :snippet, bodyFetched = 1 WHERE id = :id") suspend fun updateBody(id: String, body: String, isHtml: Boolean, snippet: String) @Query("UPDATE messages SET isRead = :isRead WHERE id = :id") @@ -53,6 +60,15 @@ interface MessageDao { @Query("DELETE FROM messages WHERE accountId = :accountId") suspend fun deleteByAccount(accountId: String) - @Query("DELETE FROM messages WHERE accountId = :accountId AND id NOT IN (:keepIds)") - suspend fun deleteNotIn(accountId: String, keepIds: List) + /** Clears only an account's inbox rows (leaves any in-flight search-only rows). */ + @Query("DELETE FROM messages WHERE accountId = :accountId AND inInbox = 1") + suspend fun deleteInboxByAccount(accountId: String) + + /** Drops inbox rows for an account that are no longer present on the server. */ + @Query("DELETE FROM messages WHERE accountId = :accountId AND inInbox = 1 AND id NOT IN (:keepIds)") + suspend fun deleteInboxNotIn(accountId: String, keepIds: List) + + /** Removes transient server-search hits (called when search closes). */ + @Query("DELETE FROM messages WHERE inInbox = 0") + suspend fun deleteSearchRows() } diff --git a/app/src/main/kotlin/org/libremail/data/local/entity/AttachmentEntity.kt b/app/src/main/kotlin/org/libremail/data/local/entity/AttachmentEntity.kt index 9713c14..ecbc181 100644 --- a/app/src/main/kotlin/org/libremail/data/local/entity/AttachmentEntity.kt +++ b/app/src/main/kotlin/org/libremail/data/local/entity/AttachmentEntity.kt @@ -2,12 +2,21 @@ package org.libremail.data.local.entity import androidx.room.Entity +import androidx.room.ForeignKey import androidx.room.Index /** Cached metadata for one attachment part of a message (the bytes are fetched on demand). */ @Entity( tableName = "attachments", primaryKeys = ["messageId", "partIndex"], + foreignKeys = [ + ForeignKey( + entity = MessageEntity::class, + parentColumns = ["id"], + childColumns = ["messageId"], + onDelete = ForeignKey.CASCADE, + ), + ], indices = [Index("messageId")], ) data class AttachmentEntity( diff --git a/app/src/main/kotlin/org/libremail/data/local/entity/DraftEntity.kt b/app/src/main/kotlin/org/libremail/data/local/entity/DraftEntity.kt index c368a0b..ed6dd3b 100644 --- a/app/src/main/kotlin/org/libremail/data/local/entity/DraftEntity.kt +++ b/app/src/main/kotlin/org/libremail/data/local/entity/DraftEntity.kt @@ -14,4 +14,6 @@ data class DraftEntity( val subject: String, val body: String, val updatedAt: Long, + /** JSON array of the draft's attachments ([uri, name] pairs); empty string when there are none. */ + val attachments: String = "", ) diff --git a/app/src/main/kotlin/org/libremail/data/local/entity/MessageEntity.kt b/app/src/main/kotlin/org/libremail/data/local/entity/MessageEntity.kt index 46f5496..c157e38 100644 --- a/app/src/main/kotlin/org/libremail/data/local/entity/MessageEntity.kt +++ b/app/src/main/kotlin/org/libremail/data/local/entity/MessageEntity.kt @@ -21,4 +21,8 @@ data class MessageEntity( val timestampMillis: Long, val isRead: Boolean, val isStarred: Boolean, + /** True for inbox-synced rows; false for transient server-search hits (purged on search close). */ + val inInbox: Boolean = true, + /** True once the body has been fetched from the server (distinguishes "not fetched" from "empty body"). */ + val bodyFetched: Boolean = false, ) diff --git a/app/src/main/kotlin/org/libremail/data/repository/AccountRepositoryImpl.kt b/app/src/main/kotlin/org/libremail/data/repository/AccountRepositoryImpl.kt index 48d03d4..785958a 100644 --- a/app/src/main/kotlin/org/libremail/data/repository/AccountRepositoryImpl.kt +++ b/app/src/main/kotlin/org/libremail/data/repository/AccountRepositoryImpl.kt @@ -6,7 +6,6 @@ import javax.inject.Singleton import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.map import org.libremail.data.local.dao.AccountDao -import org.libremail.data.local.dao.AttachmentDao import org.libremail.data.local.dao.MessageDao import org.libremail.data.local.toDomain import org.libremail.data.local.toEntity @@ -22,7 +21,6 @@ import org.libremail.mail.ImapClient class AccountRepositoryImpl @Inject constructor( private val accountDao: AccountDao, private val messageDao: MessageDao, - private val attachmentDao: AttachmentDao, private val credentialStore: CredentialStore, private val imapClient: ImapClient, private val syncScheduler: SyncScheduler, @@ -42,19 +40,6 @@ class AccountRepositoryImpl @Inject constructor( folders } - override suspend fun addGmailAccount( - email: String, - accessToken: String, - authStateJson: String, - ): Result> = runCatching { - val account = Account.gmail(email) - val folders = imapClient.listFolders(account.toImapParams(secret = accessToken, useXoauth2 = true)) - accountDao.upsert(account.toEntity()) - credentialStore.saveSecret(account.id, authStateJson) - syncScheduler.syncNow() - folders - } - override suspend fun addOutlookAccount( email: String, accessToken: String, @@ -71,8 +56,7 @@ class AccountRepositoryImpl @Inject constructor( override suspend fun deleteAccount(id: String) { accountDao.deleteById(id) credentialStore.delete(id) - // Remove the account's cached mail so it disappears from the (unified) inbox. - attachmentDao.deleteByAccountPrefix("$id:%") + // Remove the account's cached mail (attachment rows cascade via the foreign key). messageDao.deleteByAccount(id) } } diff --git a/app/src/main/kotlin/org/libremail/data/repository/MailRepositoryImpl.kt b/app/src/main/kotlin/org/libremail/data/repository/MailRepositoryImpl.kt index 82cd959..9e531fd 100644 --- a/app/src/main/kotlin/org/libremail/data/repository/MailRepositoryImpl.kt +++ b/app/src/main/kotlin/org/libremail/data/repository/MailRepositoryImpl.kt @@ -55,7 +55,7 @@ class MailRepositoryImpl @Inject constructor( val account = accountDao.getById(entity.accountId)?.toDomain() if (account != null) { val params = connectionFactory.imapParamsFor(account) - if (entity.body.isBlank()) { + if (!entity.bodyFetched) { val content = imapClient.fetchBodyMarkingSeen(params, uidOf(id)) messageDao.updateBody(id, content.body, content.isHtml, snippetOf(content.body)) attachmentDao.replaceForMessage(id, content.attachments.map { it.toEntity(id) }) @@ -112,12 +112,16 @@ class MailRepositoryImpl @Inject constructor( sendScheduler.sendNow() } - /** Copies the picked attachment URIs into the outbox message's own directory for the worker. */ + /** + * Copies the picked attachment URIs into the outbox message's own directory for the worker. + * Each attachment goes in its own index-named subdirectory so the send worker can restore the + * original order (a flat listing's order is unspecified), keeping the file's real name intact. + */ private fun copyAttachments(outboxId: String, attachments: List) { if (attachments.isEmpty()) return - val dir = File(context.cacheDir, "outbox/$outboxId").apply { mkdirs() } - attachments.forEach { attachment -> + attachments.forEachIndexed { index, attachment -> val safeName = attachment.name.substringAfterLast('/').substringAfterLast('\\').ifBlank { "attachment" } + val dir = File(context.cacheDir, "outbox/$outboxId/$index").apply { mkdirs() } runCatching { context.contentResolver.openInputStream(Uri.parse(attachment.uri))?.use { input -> File(dir, safeName).outputStream().use { output -> input.copyTo(output) } @@ -150,23 +154,25 @@ class MailRepositoryImpl @Inject constructor( val account = entity.toDomain() runCatching { val results = imapClient.search(connectionFactory.imapParamsFor(account), query, SEARCH_LIMIT) - val entities = results.map { it.toEntity(account.id) } + // Mark hits as non-inbox so they show only while searching (and never overwrite the + // inbox membership of a row that is genuinely in the inbox). + val entities = results.map { it.toEntity(account.id, inInbox = false) } messageDao.insertNew(entities) entities.forEach { - messageDao.updateHeader( + messageDao.updateHeaderContent( id = it.id, sender = it.sender, senderEmail = it.senderEmail, subject = it.subject, timestampMillis = it.timestampMillis, - isRead = it.isRead, - isStarred = it.isStarred, ) } } } } + override suspend fun clearSearchResults() = messageDao.deleteSearchRows() + /** Writes downloaded bytes to a private cache file that the FileProvider can share. */ private fun saveToCache(attachment: DownloadedAttachment): File { val dir = File(context.cacheDir, "attachments").apply { mkdirs() } diff --git a/app/src/main/kotlin/org/libremail/data/security/KeystoreCrypto.kt b/app/src/main/kotlin/org/libremail/data/security/KeystoreCrypto.kt index a209045..86ecc1d 100644 --- a/app/src/main/kotlin/org/libremail/data/security/KeystoreCrypto.kt +++ b/app/src/main/kotlin/org/libremail/data/security/KeystoreCrypto.kt @@ -19,7 +19,11 @@ import javax.inject.Singleton @Singleton class KeystoreCrypto @Inject constructor() { - private fun secretKey(): SecretKey { + private val keyLock = Any() + + // Synchronized so two concurrent first-run encrypts can't both generate a key under the same + // alias — the second would overwrite the first, leaving the first secret undecryptable. + private fun secretKey(): SecretKey = synchronized(keyLock) { val keyStore = KeyStore.getInstance(ANDROID_KEYSTORE).apply { load(null) } (keyStore.getEntry(KEY_ALIAS, null) as? KeyStore.SecretKeyEntry)?.let { return it.secretKey } @@ -34,7 +38,7 @@ class KeystoreCrypto @Inject constructor() { .setKeySize(256) .build(), ) - return generator.generateKey() + generator.generateKey() } /** Returns Base64(iv || ciphertext). */ diff --git a/app/src/main/kotlin/org/libremail/data/sync/MailConnectionFactory.kt b/app/src/main/kotlin/org/libremail/data/sync/MailConnectionFactory.kt index 1d220e3..8e01972 100644 --- a/app/src/main/kotlin/org/libremail/data/sync/MailConnectionFactory.kt +++ b/app/src/main/kotlin/org/libremail/data/sync/MailConnectionFactory.kt @@ -1,57 +1,93 @@ // SPDX-License-Identifier: GPL-3.0-or-later package org.libremail.data.sync +import java.util.concurrent.ConcurrentHashMap import javax.inject.Inject import javax.inject.Singleton +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock import org.libremail.auth.FreshToken import org.libremail.auth.GmailAuthManager import org.libremail.auth.OutlookAuthManager import org.libremail.data.local.toImapParams import org.libremail.data.local.toSmtpParams import org.libremail.data.security.CredentialStore +import org.libremail.data.settings.SettingsRepository import org.libremail.domain.model.Account import org.libremail.domain.model.AuthType import org.libremail.domain.model.ImapConnectionParams import org.libremail.domain.model.SmtpParams -/** Resolves an account's stored credential (refreshing the Gmail token when needed) into connection params. */ +/** Resolves an account's stored credential (refreshing OAuth tokens when needed) into connection params. */ @Singleton class MailConnectionFactory @Inject constructor( private val credentialStore: CredentialStore, private val gmailAuthManager: GmailAuthManager, private val outlookAuthManager: OutlookAuthManager, + private val settingsRepository: SettingsRepository, ) { + private data class CachedToken(val token: String, val expiry: Long?) + + /** Per-account lock so concurrent refreshes can't redeem the same (rotating) refresh token twice. */ + private val refreshMutexes = ConcurrentHashMap() + + /** In-memory access-token cache keyed by "accountId|scope", to avoid redeeming a still-valid token. */ + private val tokenCache = ConcurrentHashMap() + suspend fun imapParamsFor(account: Account): ImapConnectionParams = - account.toImapParams(resolveSecret(account), account.authType != AuthType.PASSWORD_IMAP) + account.toImapParams(resolveSecret(account), account.authType != AuthType.PASSWORD_IMAP, strictStartTls()) suspend fun smtpParamsFor(account: Account): SmtpParams = - account.toSmtpParams(resolveSecret(account), account.authType != AuthType.PASSWORD_IMAP) + account.toSmtpParams(resolveSecret(account), account.authType != AuthType.PASSWORD_IMAP, strictStartTls()) /** A fresh Microsoft Graph access token for the primary Outlook (sendMail) send path. */ - suspend fun graphTokenFor(account: Account): String { - val stored = credentialStore.loadSecret(account.id) - ?: error("No stored credentials for ${account.email}") - return refreshedToken(account.id, stored, outlookAuthManager::freshGraphToken) + suspend fun graphTokenFor(account: Account): String = + cachedAccessToken(account.id, SCOPE_GRAPH, outlookAuthManager::freshGraphToken) + + private suspend fun resolveSecret(account: Account): String = when (account.authType) { + AuthType.PASSWORD_IMAP -> + credentialStore.loadSecret(account.id) ?: error("No stored credentials for ${account.email}") + AuthType.OAUTH_GMAIL -> + cachedAccessToken(account.id, SCOPE_GMAIL, gmailAuthManager::freshAccessToken) + AuthType.OAUTH_OUTLOOK -> + cachedAccessToken(account.id, SCOPE_OUTLOOK, outlookAuthManager::freshOutlookToken) } - private suspend fun resolveSecret(account: Account): String { - val stored = credentialStore.loadSecret(account.id) - ?: error("No stored credentials for ${account.email}") - return when (account.authType) { - AuthType.PASSWORD_IMAP -> stored - AuthType.OAUTH_GMAIL -> refreshedToken(account.id, stored, gmailAuthManager::freshAccessToken) - AuthType.OAUTH_OUTLOOK -> refreshedToken(account.id, stored, outlookAuthManager::freshOutlookToken) + /** + * Returns a cached access token while it is still valid, otherwise refreshes under the account's + * lock (re-checking the cache first, so a concurrent caller redeems only once) and persists the + * updated AuthState. + */ + private suspend fun cachedAccessToken( + accountId: String, + scope: String, + refresh: suspend (String) -> FreshToken, + ): String { + validCachedToken(accountId, scope)?.let { return it } + // 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 fresh = refresh(stored) + if (fresh.authStateJson != stored) credentialStore.saveSecret(accountId, fresh.authStateJson) + tokenCache["$accountId|$scope"] = CachedToken(fresh.accessToken, fresh.accessTokenExpiry) + fresh.accessToken } } - /** Refreshes an OAuth access token, persisting the updated AuthState when it changes. */ - private suspend fun refreshedToken( - accountId: String, - stored: String, - refresh: suspend (String) -> FreshToken, - ): String { - val fresh = refresh(stored) - if (fresh.authStateJson != stored) credentialStore.saveSecret(accountId, fresh.authStateJson) - return fresh.accessToken + private fun validCachedToken(accountId: String, scope: String): String? { + val cached = tokenCache["$accountId|$scope"] ?: return null + val expiry = cached.expiry ?: return null // unknown expiry — don't trust the cache + return cached.token.takeIf { expiry - EXPIRY_BUFFER_MS > System.currentTimeMillis() } + } + + private suspend fun strictStartTls(): Boolean = !settingsRepository.settings.first().allowStartTls + + private companion object { + const val SCOPE_GMAIL = "gmail" + const val SCOPE_OUTLOOK = "outlook" + const val SCOPE_GRAPH = "graph" + const val EXPIRY_BUFFER_MS = 60_000L } } diff --git a/app/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt b/app/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt index 490fc16..4e8a193 100644 --- a/app/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt +++ b/app/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt @@ -3,9 +3,12 @@ package org.libremail.data.sync import javax.inject.Inject import javax.inject.Singleton +import kotlinx.coroutines.NonCancellable +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock +import kotlinx.coroutines.withContext import org.libremail.data.local.dao.AccountDao import org.libremail.data.local.dao.MessageDao -import org.libremail.data.local.entity.MessageEntity import org.libremail.data.local.toDomain import org.libremail.data.local.toEntity import org.libremail.data.settings.SettingsRepository @@ -23,66 +26,80 @@ class MailSyncer @Inject constructor( private val settingsRepository: SettingsRepository, private val notifier: MailNotifier, ) { + // Serializes all syncing: syncAll/syncAccount are invoked concurrently by the periodic worker, + // pull-to-refresh, one-shot syncs, and one IDLE watcher per account. Without this, two runs can + // both compute the same message as "new" (double-notify) or let a stale deleteInboxNotIn snapshot + // delete a row another run just inserted. + private val syncMutex = Mutex() + /** Syncs every account. Succeeds if at least one account synced (or there are none). */ - suspend fun syncAll(): Result { + suspend fun syncAll(): Result = syncMutex.withLock { val accounts = accountDao.getAll() - if (accounts.isEmpty()) return Result.success(0) + if (accounts.isEmpty()) return@withLock Result.success(0) var total = 0 var firstError: Throwable? = null var anySuccess = false - val newMessages = mutableListOf() for (entity in accounts) { - syncAccount(entity.toDomain()).fold( - onSuccess = { result -> - total += result.fetched - newMessages += result.newMessages + syncAccountInternal(entity.toDomain()).fold( + onSuccess = { fetched -> + total += fetched anySuccess = true }, onFailure = { error -> if (firstError == null) firstError = error }, ) } - - if (newMessages.isNotEmpty() && settingsRepository.isNewMailNotificationsEnabled()) { - notifier.notifyNewMail(newMessages.sortedByDescending { it.timestampMillis }) - } - return if (anySuccess || firstError == null) Result.success(total) else Result.failure(firstError!!) + if (anySuccess || firstError == null) Result.success(total) else Result.failure(firstError) } - private suspend fun syncAccount(account: Account): Result = runCatching { + /** Syncs one account — used by the per-account IDLE watcher so a single push doesn't re-sync all. */ + suspend fun syncAccount(accountId: String): Result = syncMutex.withLock { + val entity = accountDao.getById(accountId) ?: return@withLock Result.success(0) + syncAccountInternal(entity.toDomain()) + } + + private suspend fun syncAccountInternal(account: Account): Result = runCatching { val params = connectionFactory.imapParamsFor(account) - val fetched = imapClient.fetchRecentInbox(params, INBOX_LIMIT) + val fetched = imapClient.fetchRecentInbox(params, INBOX_LIMIT) // cancellable network I/O val entities = fetched.map { it.toEntity(account.id) } - val existingIds = messageDao.getIdsForAccount(account.id).toHashSet() - // Don't notify on the very first sync of an account (would announce the whole inbox). - val newMessages = if (existingIds.isEmpty()) { - emptyList() - } else { - entities.filter { it.id !in existingIds && !it.isRead } - } - - if (entities.isEmpty()) { - messageDao.deleteByAccount(account.id) - } else { - messageDao.insertNew(entities) - entities.forEach { - messageDao.updateHeader( - id = it.id, - sender = it.sender, - senderEmail = it.senderEmail, - subject = it.subject, - timestampMillis = it.timestampMillis, - isRead = it.isRead, - isStarred = it.isStarred, - ) + // Persist and notify atomically with respect to cancellation: an IDLE renewal that cancels + // mid-sync must not drop a notification (the rows would then look "already seen" next time). + withContext(NonCancellable) { + val existingIds = messageDao.getInboxIdsForAccount(account.id).toHashSet() + // Don't notify on the very first sync of an account (would announce the whole inbox). + val newMessages = if (existingIds.isEmpty()) { + emptyList() + } else { + entities.filter { it.id !in existingIds && !it.isRead } } - messageDao.deleteNotIn(account.id, entities.map { it.id }) - } - AccountSyncResult(fetched = fetched.size, newMessages = newMessages) - } - private data class AccountSyncResult(val fetched: Int, val newMessages: List) + if (entities.isEmpty()) { + messageDao.deleteInboxByAccount(account.id) + } else { + val ids = entities.map { it.id } + messageDao.insertNew(entities) + // Mark every fetched message as inbox (upgrades any former search-only row) and refresh + // its display fields — without touching cached bodies or optimistic read/star flags. + messageDao.markInInbox(ids) + entities.forEach { + messageDao.updateHeaderContent( + id = it.id, + sender = it.sender, + senderEmail = it.senderEmail, + subject = it.subject, + timestampMillis = it.timestampMillis, + ) + } + messageDao.deleteInboxNotIn(account.id, ids) + } + + if (newMessages.isNotEmpty() && settingsRepository.isNewMailNotificationsEnabled()) { + notifier.notifyNewMail(newMessages.sortedByDescending { it.timestampMillis }) + } + } + fetched.size + } private companion object { const val INBOX_LIMIT = 50 diff --git a/app/src/main/kotlin/org/libremail/data/sync/SendWorker.kt b/app/src/main/kotlin/org/libremail/data/sync/SendWorker.kt index 7cc18c4..f4df276 100644 --- a/app/src/main/kotlin/org/libremail/data/sync/SendWorker.kt +++ b/app/src/main/kotlin/org/libremail/data/sync/SendWorker.kt @@ -8,16 +8,18 @@ import androidx.work.WorkerParameters import dagger.assisted.Assisted import dagger.assisted.AssistedInject import java.io.File +import kotlin.coroutines.cancellation.CancellationException import org.libremail.data.local.dao.AccountDao import org.libremail.data.local.dao.OutboxDao import org.libremail.data.local.toDomain import org.libremail.domain.model.Account import org.libremail.domain.model.AuthType import org.libremail.domain.model.OutgoingMessage +import org.libremail.mail.GraphSendException import org.libremail.mail.GraphSender import org.libremail.mail.SmtpSender -/** Drains the outbox: sends each queued message over SMTP, deleting it on success. */ +/** Drains the outbox: sends each queued message over Graph/SMTP, deleting it on success. */ @HiltWorker class SendWorker @AssistedInject constructor( @Assisted appContext: Context, @@ -50,7 +52,7 @@ class SendWorker @AssistedInject constructor( subject = entity.subject, body = entity.body, ) - val files = attachmentDir.listFiles()?.toList().orEmpty() + val files = orderedAttachments(attachmentDir) if (account.authType == AuthType.OAUTH_OUTLOOK) { sendOutlook(account, message, files) } else { @@ -62,8 +64,15 @@ class SendWorker @AssistedInject constructor( attachmentDir.deleteRecursively() }, onFailure = { e -> - outboxDao.setError(entity.id, e.message) - anyFailed = true + if (e is GraphSendException && e.mayHaveSent) { + // Graph may already have delivered this; auto-retrying (or any other send) + // would duplicate it, so leave it queued with a clear status and let the + // user decide. Not counted as a failure, so WorkManager won't auto-retry. + outboxDao.setError(entity.id, "Send status unknown — check your Sent folder, then retry or cancel") + } else { + outboxDao.setError(entity.id, e.message) + anyFailed = true + } }, ) } @@ -71,12 +80,31 @@ class SendWorker @AssistedInject constructor( return if (anyFailed) Result.retry() else Result.success() } - /** Outlook prefers Microsoft Graph; fall back to SMTP (XOAUTH2) if the Graph send fails. */ + /** + * Outlook prefers Microsoft Graph. Fall back to SMTP only when Graph definitely did NOT send + * (a rejection, a pre-send/transport error, or a token failure); never fall back when the Graph + * request may already have been accepted, or the message would be sent twice. + */ private suspend fun sendOutlook(account: Account, message: OutgoingMessage, files: List) { - runCatching { - graphSender.send(connectionFactory.graphTokenFor(account), message, files) - }.getOrElse { + try { + val token = connectionFactory.graphTokenFor(account) + graphSender.send(token, message, files) + } catch (e: GraphSendException) { + if (e.mayHaveSent) throw e + smtpSender.send(connectionFactory.smtpParamsFor(account), from = account.email, message = message, attachments = files) + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + // Graph was never reached (e.g. token refresh failed) — SMTP cannot duplicate it. smtpSender.send(connectionFactory.smtpParamsFor(account), from = account.email, message = message, attachments = files) } } + + /** Attachments are staged one-per-indexed-subdirectory so their original order is preserved. */ + private fun orderedAttachments(dir: File): List = + dir.listFiles() + ?.filter { it.isDirectory } + ?.sortedBy { it.name.toIntOrNull() ?: Int.MAX_VALUE } + ?.mapNotNull { it.listFiles()?.firstOrNull() } + .orEmpty() } diff --git a/app/src/main/kotlin/org/libremail/di/DatabaseModule.kt b/app/src/main/kotlin/org/libremail/di/DatabaseModule.kt index b962769..c6facf3 100644 --- a/app/src/main/kotlin/org/libremail/di/DatabaseModule.kt +++ b/app/src/main/kotlin/org/libremail/di/DatabaseModule.kt @@ -10,10 +10,12 @@ import dagger.hilt.android.qualifiers.ApplicationContext import dagger.hilt.components.SingletonComponent import javax.inject.Singleton import org.libremail.data.local.LibreMailDatabase +import org.libremail.data.local.MIGRATION_1_2 import org.libremail.data.local.MIGRATION_2_3 import org.libremail.data.local.MIGRATION_3_4 import org.libremail.data.local.MIGRATION_4_5 import org.libremail.data.local.MIGRATION_5_6 +import org.libremail.data.local.MIGRATION_6_7 import org.libremail.data.local.dao.AccountDao import org.libremail.data.local.dao.AttachmentDao import org.libremail.data.local.dao.CredentialDao @@ -29,9 +31,17 @@ object DatabaseModule { @Singleton fun provideDatabase(@ApplicationContext context: Context): LibreMailDatabase = Room.databaseBuilder(context, LibreMailDatabase::class.java, "libremail.db") - .addMigrations(MIGRATION_2_3, MIGRATION_3_4, MIGRATION_4_5, MIGRATION_5_6) - // Safety net for unforeseen schema jumps during early development. - .fallbackToDestructiveMigration(dropAllTables = true) + .addMigrations( + MIGRATION_1_2, + MIGRATION_2_3, + MIGRATION_3_4, + MIGRATION_4_5, + MIGRATION_5_6, + MIGRATION_6_7, + ) + // No destructive fallback: the migration chain is complete, and silently dropping the + // accounts/credentials/mail tables would lose stored secrets. A missing migration should + // fail loudly in testing instead. .build() @Provides diff --git a/app/src/main/kotlin/org/libremail/domain/model/Account.kt b/app/src/main/kotlin/org/libremail/domain/model/Account.kt index e68ebde..3728156 100644 --- a/app/src/main/kotlin/org/libremail/domain/model/Account.kt +++ b/app/src/main/kotlin/org/libremail/domain/model/Account.kt @@ -22,16 +22,6 @@ data class Account( val smtp: ServerConfig, ) { companion object { - /** A Gmail account with Google's standard IMAP/SMTP endpoints. */ - fun gmail(email: String, displayName: String = email): Account = Account( - id = "gmail:$email", - email = email, - displayName = displayName.ifBlank { email }, - authType = AuthType.OAUTH_GMAIL, - imap = ServerConfig("imap.gmail.com", 993, MailSecurity.SSL_TLS), - smtp = ServerConfig("smtp.gmail.com", 465, MailSecurity.SSL_TLS), - ) - /** An Outlook/Microsoft account using the unified office365 endpoints (personal + M365). */ fun outlook(email: String, displayName: String = email): Account = Account( id = "outlook:$email", diff --git a/app/src/main/kotlin/org/libremail/domain/model/Draft.kt b/app/src/main/kotlin/org/libremail/domain/model/Draft.kt index 8592f65..e6cecd9 100644 --- a/app/src/main/kotlin/org/libremail/domain/model/Draft.kt +++ b/app/src/main/kotlin/org/libremail/domain/model/Draft.kt @@ -9,4 +9,5 @@ data class Draft( val subject: String, val body: String, val updatedAt: Long, + val attachments: List = emptyList(), ) diff --git a/app/src/main/kotlin/org/libremail/domain/model/ImapConnectionParams.kt b/app/src/main/kotlin/org/libremail/domain/model/ImapConnectionParams.kt index 1c6789d..91d41c8 100644 --- a/app/src/main/kotlin/org/libremail/domain/model/ImapConnectionParams.kt +++ b/app/src/main/kotlin/org/libremail/domain/model/ImapConnectionParams.kt @@ -10,4 +10,6 @@ data class ImapConnectionParams( /** Password, app-password, or — when [useXoauth2] is true — an OAuth access token. */ val secret: String, val useXoauth2: Boolean, + /** When true (default), a STARTTLS upgrade must succeed; when false it is best-effort. */ + val strictStartTls: Boolean = true, ) diff --git a/app/src/main/kotlin/org/libremail/domain/model/Message.kt b/app/src/main/kotlin/org/libremail/domain/model/Message.kt index d25e7d6..0d9becc 100644 --- a/app/src/main/kotlin/org/libremail/domain/model/Message.kt +++ b/app/src/main/kotlin/org/libremail/domain/model/Message.kt @@ -13,4 +13,6 @@ data class Message( val timestampMillis: Long, val isRead: Boolean, val isStarred: Boolean, + /** True for messages synced as part of the inbox; false for transient server-search hits. */ + val inInbox: Boolean = true, ) diff --git a/app/src/main/kotlin/org/libremail/domain/model/SmtpParams.kt b/app/src/main/kotlin/org/libremail/domain/model/SmtpParams.kt index dfade3f..bd90c6a 100644 --- a/app/src/main/kotlin/org/libremail/domain/model/SmtpParams.kt +++ b/app/src/main/kotlin/org/libremail/domain/model/SmtpParams.kt @@ -10,4 +10,6 @@ data class SmtpParams( /** Password, app-password, or — when [useXoauth2] is true — an OAuth access token. */ val secret: String, val useXoauth2: Boolean, + /** When true (default), a STARTTLS upgrade must succeed; when false it is best-effort. */ + val strictStartTls: Boolean = true, ) diff --git a/app/src/main/kotlin/org/libremail/domain/repository/AccountRepository.kt b/app/src/main/kotlin/org/libremail/domain/repository/AccountRepository.kt index 3967431..a571b2c 100644 --- a/app/src/main/kotlin/org/libremail/domain/repository/AccountRepository.kt +++ b/app/src/main/kotlin/org/libremail/domain/repository/AccountRepository.kt @@ -15,9 +15,6 @@ interface AccountRepository { /** Verify, then persist, a password/app-password IMAP account. Returns the folders found. */ suspend fun addImapAccount(account: Account, password: String): Result> - /** Verify (via XOAUTH2), then persist, a Gmail account. Returns the folders found. */ - suspend fun addGmailAccount(email: String, accessToken: String, authStateJson: String): Result> - /** Verify (via XOAUTH2), then persist, an Outlook account. Returns the folders found. */ suspend fun addOutlookAccount(email: String, accessToken: String, authStateJson: String): Result> diff --git a/app/src/main/kotlin/org/libremail/domain/repository/MailRepository.kt b/app/src/main/kotlin/org/libremail/domain/repository/MailRepository.kt index 7102aa2..bee54b6 100644 --- a/app/src/main/kotlin/org/libremail/domain/repository/MailRepository.kt +++ b/app/src/main/kotlin/org/libremail/domain/repository/MailRepository.kt @@ -46,4 +46,7 @@ interface MailRepository { /** Fetches server-side search matches into the cache so the message list can surface them. */ suspend fun searchServer(query: String) + + /** Drops transient server-search hits from the cache (called when search is dismissed). */ + suspend fun clearSearchResults() } diff --git a/app/src/main/kotlin/org/libremail/mail/GraphSender.kt b/app/src/main/kotlin/org/libremail/mail/GraphSender.kt index a45d17b..7f62514 100644 --- a/app/src/main/kotlin/org/libremail/mail/GraphSender.kt +++ b/app/src/main/kotlin/org/libremail/mail/GraphSender.kt @@ -1,7 +1,9 @@ // SPDX-License-Identifier: GPL-3.0-or-later package org.libremail.mail +import jakarta.mail.internet.InternetAddress import java.io.File +import java.io.IOException import java.net.HttpURLConnection import java.net.URL import java.util.Base64 @@ -13,6 +15,17 @@ import org.json.JSONArray import org.json.JSONObject import org.libremail.domain.model.OutgoingMessage +/** + * Thrown when a Graph `sendMail` attempt fails. [mayHaveSent] is true only when the request was + * fully transmitted but the response could not be read — in that case the message may already be on + * its way, so callers must NOT retry or fall back to another transport (doing so would duplicate it). + */ +class GraphSendException( + message: String, + val mayHaveSent: Boolean, + cause: Throwable? = null, +) : Exception(message, cause) + /** * Sends mail via Microsoft Graph `me/sendMail` — Microsoft's preferred send path for Outlook / * Microsoft 365, used in place of SMTP. Authenticated with a Graph access token (Bearer). @@ -35,12 +48,24 @@ class GraphSender @Inject constructor() { setRequestProperty("Content-Type", "application/json; charset=utf-8") } try { - connection.outputStream.use { it.write(payload.toByteArray(Charsets.UTF_8)) } - val code = connection.responseCode + // Failure here means the request never reached Graph — safe to fall back/retry. + try { + connection.outputStream.use { it.write(payload.toByteArray(Charsets.UTF_8)) } + } catch (e: IOException) { + throw GraphSendException("Graph sendMail could not be transmitted", mayHaveSent = false, cause = e) + } + // The request was fully sent; if we can't read the response, Graph may already have + // accepted and sent it — do not fall back to SMTP or the message would be duplicated. + val code = try { + connection.responseCode + } catch (e: IOException) { + throw GraphSendException("Graph sendMail sent but no response received", mayHaveSent = true, cause = e) + } if (code !in 200..299) { val body = (connection.errorStream ?: connection.inputStream) ?.bufferedReader()?.use { it.readText() }.orEmpty() - error("Graph sendMail failed (HTTP $code): ${body.take(500)}") + // An explicit non-2xx means Graph rejected (did not send) — safe to fall back. + throw GraphSendException("Graph sendMail failed (HTTP $code): ${body.take(500)}", mayHaveSent = false) } } finally { connection.disconnect() @@ -77,14 +102,20 @@ internal fun buildSendMailPayload(message: OutgoingMessage, attachments: List` — and commas + * inside quoted display names — produce a valid bare `address` (plus an optional `name`). + */ private fun recipientsJson(addresses: String): JSONArray { val array = JSONArray() - addresses.split(",", ";") - .map { it.trim() } - .filter { it.isNotEmpty() } - .forEach { address -> - array.put(JSONObject().put("emailAddress", JSONObject().put("address", address))) - } + val parsed = runCatching { InternetAddress.parse(addresses, false) }.getOrNull() ?: emptyArray() + parsed.forEach { addr -> + val email = addr.address?.trim().orEmpty() + if (email.isEmpty()) return@forEach + val emailAddress = JSONObject().put("address", email) + addr.personal?.takeIf { it.isNotBlank() }?.let { emailAddress.put("name", it) } + array.put(JSONObject().put("emailAddress", emailAddress)) + } return array } diff --git a/app/src/main/kotlin/org/libremail/mail/ImapClient.kt b/app/src/main/kotlin/org/libremail/mail/ImapClient.kt index 44f7f1e..105f41d 100644 --- a/app/src/main/kotlin/org/libremail/mail/ImapClient.kt +++ b/app/src/main/kotlin/org/libremail/mail/ImapClient.kt @@ -215,8 +215,14 @@ class ImapClient @Inject constructor() { val protocol = if (params.security == MailSecurity.SSL_TLS) "imaps" else "imap" val store = Session.getInstance(buildProps(protocol, params)).getStore(protocol) store.connect(params.host, params.port, params.username, params.secret) - val inbox = store.getFolder("INBOX") as IMAPFolder - inbox.open(Folder.READ_ONLY) + // Close the just-connected store if opening the folder fails, so a failed connect in the + // IDLE reconnect loop can't leak connections until the server's per-account limit is hit. + val inbox = try { + (store.getFolder("INBOX") as IMAPFolder).also { it.open(Folder.READ_ONLY) } + } catch (e: Throwable) { + runCatching { store.close() } + throw e + } Log.d(TAG, "IDLE connected for ${params.username}") val pushes = Channel(Channel.CONFLATED) @@ -345,7 +351,7 @@ class ImapClient @Inject constructor() { put("mail.$protocol.writetimeout", TIMEOUT_MS) if (params.security == MailSecurity.STARTTLS) { put("mail.$protocol.starttls.enable", "true") - put("mail.$protocol.starttls.required", "true") + put("mail.$protocol.starttls.required", params.strictStartTls.toString()) } if (params.useXoauth2) { put("mail.$protocol.auth.mechanisms", "XOAUTH2") diff --git a/app/src/main/kotlin/org/libremail/mail/SmtpSender.kt b/app/src/main/kotlin/org/libremail/mail/SmtpSender.kt index dc7c612..1ed7ec7 100644 --- a/app/src/main/kotlin/org/libremail/mail/SmtpSender.kt +++ b/app/src/main/kotlin/org/libremail/mail/SmtpSender.kt @@ -43,7 +43,7 @@ class SmtpSender @Inject constructor() { } if (params.security == MailSecurity.STARTTLS) { put("mail.$protocol.starttls.enable", "true") - put("mail.$protocol.starttls.required", "true") + put("mail.$protocol.starttls.required", params.strictStartTls.toString()) } if (params.useXoauth2) { put("mail.$protocol.auth.mechanisms", "XOAUTH2") diff --git a/app/src/main/kotlin/org/libremail/notifications/MailNotifier.kt b/app/src/main/kotlin/org/libremail/notifications/MailNotifier.kt index beadb85..83ceb4a 100644 --- a/app/src/main/kotlin/org/libremail/notifications/MailNotifier.kt +++ b/app/src/main/kotlin/org/libremail/notifications/MailNotifier.kt @@ -29,39 +29,61 @@ class MailNotifier @Inject constructor( fun notifyNewMail(messages: List) { if (messages.isEmpty() || !hasPermission()) return ensureChannel() + val manager = NotificationManagerCompat.from(context) + val contentIntent = contentIntent() - val title = if (messages.size == 1) { - messages.first().sender - } else { - context.getString(R.string.notif_new_mail_count, messages.size) - } - val text = if (messages.size == 1) { - messages.first().subject - } else { - messages.joinToString(", ") { it.sender } + // One notification per message, keyed by a stable id, so a later batch never overwrites an + // earlier, still-unacknowledged one. setOnlyAlertOnce avoids re-buzzing for the same message. + messages.forEach { message -> + val notification = NotificationCompat.Builder(context, CHANNEL_ID) + .setSmallIcon(R.drawable.ic_launcher_monochrome) + .setContentTitle(message.sender) + .setContentText(message.subject) + .setStyle(NotificationCompat.BigTextStyle().bigText(message.subject)) + .setCategory(NotificationCompat.CATEGORY_EMAIL) + .setAutoCancel(true) + .setOnlyAlertOnce(true) + .setGroup(GROUP_KEY) + .setContentIntent(contentIntent) + .build() + manager.notify(notificationId(message.id), notification) } + // Group summary (the system shows it only once two or more children are present). + val summary = NotificationCompat.Builder(context, CHANNEL_ID) + .setSmallIcon(R.drawable.ic_launcher_monochrome) + .setContentTitle(context.getString(R.string.notif_channel_new_mail)) + .setStyle( + NotificationCompat.InboxStyle().also { style -> + messages.take(SUMMARY_LINES).forEach { style.addLine("${it.sender}: ${it.subject}") } + }, + ) + .setCategory(NotificationCompat.CATEGORY_EMAIL) + .setAutoCancel(true) + .setOnlyAlertOnce(true) + .setGroup(GROUP_KEY) + .setGroupSummary(true) + .setContentIntent(contentIntent) + .build() + manager.notify(SUMMARY_ID, summary) + } + + private fun contentIntent(): PendingIntent { val intent = Intent(context, MainActivity::class.java).apply { flags = Intent.FLAG_ACTIVITY_SINGLE_TOP or Intent.FLAG_ACTIVITY_CLEAR_TOP } - val pendingIntent = PendingIntent.getActivity( + return PendingIntent.getActivity( context, 0, intent, PendingIntent.FLAG_IMMUTABLE or PendingIntent.FLAG_UPDATE_CURRENT, ) + } - val notification = NotificationCompat.Builder(context, CHANNEL_ID) - .setSmallIcon(R.drawable.ic_launcher_monochrome) - .setContentTitle(title) - .setContentText(text) - .setStyle(NotificationCompat.BigTextStyle().bigText(text)) - .setCategory(NotificationCompat.CATEGORY_EMAIL) - .setAutoCancel(true) - .setContentIntent(pendingIntent) - .build() - - NotificationManagerCompat.from(context).notify(NOTIFICATION_ID, notification) + /** Stable per-message id distinct from the summary id, so each message gets its own notification. */ + private fun notificationId(messageId: String): Int { + val hash = messageId.hashCode() + return if (hash == SUMMARY_ID) hash + 1 else hash } private fun hasPermission(): Boolean = @@ -79,6 +101,8 @@ class MailNotifier @Inject constructor( private companion object { const val CHANNEL_ID = "new_mail" - const val NOTIFICATION_ID = 1001 + const val GROUP_KEY = "org.libremail.NEW_MAIL" + const val SUMMARY_ID = 1001 + const val SUMMARY_LINES = 5 } } diff --git a/app/src/main/kotlin/org/libremail/push/IdleService.kt b/app/src/main/kotlin/org/libremail/push/IdleService.kt index 46247ff..d8f82a0 100644 --- a/app/src/main/kotlin/org/libremail/push/IdleService.kt +++ b/app/src/main/kotlin/org/libremail/push/IdleService.kt @@ -92,7 +92,8 @@ class IdleService : Service() { try { val params = connectionFactory.imapParamsFor(account) withTimeoutOrNull(IDLE_RENEWAL_MS) { - imapClient.idle(params) { mailSyncer.syncAll() } + // Sync just this account on its own push — not every account. + imapClient.idle(params) { mailSyncer.syncAccount(account.id) } } backoffMs = INITIAL_BACKOFF_MS } catch (e: CancellationException) { diff --git a/app/src/main/kotlin/org/libremail/ui/accountsetup/AccountSetupViewModel.kt b/app/src/main/kotlin/org/libremail/ui/accountsetup/AccountSetupViewModel.kt index 0248f57..787e5ea 100644 --- a/app/src/main/kotlin/org/libremail/ui/accountsetup/AccountSetupViewModel.kt +++ b/app/src/main/kotlin/org/libremail/ui/accountsetup/AccountSetupViewModel.kt @@ -11,11 +11,10 @@ import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch -import org.libremail.auth.GmailAuthManager import org.libremail.auth.OutlookAuthManager import org.libremail.domain.repository.AccountRepository -/** Stage of an account-setup attempt, shared by the Gmail and manual flows. */ +/** Stage of an account-setup attempt, shared by the Outlook and manual flows. */ enum class SetupStatus { IDLE, CONNECTING, DONE } data class AccountSetupUiState( @@ -25,7 +24,6 @@ data class AccountSetupUiState( @HiltViewModel class AccountSetupViewModel @Inject constructor( - private val authManager: GmailAuthManager, private val outlookAuthManager: OutlookAuthManager, private val accountRepository: AccountRepository, ) : ViewModel() { @@ -33,27 +31,6 @@ class AccountSetupViewModel @Inject constructor( private val _state = MutableStateFlow(AccountSetupUiState()) val state: StateFlow = _state.asStateFlow() - val isGmailConfigured: Boolean get() = authManager.isConfigured - - fun gmailAuthIntent(): Intent = authManager.createAuthIntent() - - fun onGmailResult(data: Intent?) { - if (data == null) { - _state.update { it.copy(error = "Google sign-in was cancelled") } - return - } - viewModelScope.launch { - _state.update { it.copy(status = SetupStatus.CONNECTING, error = null) } - runCatching { - val oauth = authManager.exchangeToken(data) - accountRepository.addGmailAccount(oauth.email, oauth.accessToken, oauth.authStateJson).getOrThrow() - }.fold( - onSuccess = { _state.update { it.copy(status = SetupStatus.DONE) } }, - onFailure = { e -> _state.update { it.copy(status = SetupStatus.IDLE, error = e.message ?: "Gmail sign-in failed") } }, - ) - } - } - val isOutlookConfigured: Boolean get() = outlookAuthManager.isConfigured fun outlookAuthIntent(): Intent = outlookAuthManager.createAuthIntent() diff --git a/app/src/main/kotlin/org/libremail/ui/compose/ComposeViewModel.kt b/app/src/main/kotlin/org/libremail/ui/compose/ComposeViewModel.kt index 1fa05f0..7c640e3 100644 --- a/app/src/main/kotlin/org/libremail/ui/compose/ComposeViewModel.kt +++ b/app/src/main/kotlin/org/libremail/ui/compose/ComposeViewModel.kt @@ -13,6 +13,7 @@ import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.receiveAsFlow import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.flow.update @@ -44,7 +45,7 @@ data class ComposeUiState( class ComposeViewModel @Inject constructor( savedStateHandle: SavedStateHandle, private val mailRepository: MailRepository, - accountRepository: AccountRepository, + private val accountRepository: AccountRepository, private val contactsRepository: ContactsRepository, ) : ViewModel() { @@ -69,6 +70,9 @@ class ComposeViewModel @Inject constructor( private var searchJob: Job? = null + /** Guards against double-navigation and against saving a draft for an already-sent message. */ + @Volatile private var navigated = false + init { if (draftId != null) { viewModelScope.launch { @@ -80,6 +84,7 @@ class ComposeViewModel @Inject constructor( subject = draft.subject, body = draft.body, fromAccountId = draft.accountId ?: it.fromAccountId, + attachments = draft.attachments, ) } } @@ -125,15 +130,25 @@ class ComposeViewModel @Inject constructor( /** Leaving the screen: keep a draft if there's anything worth keeping, then close. */ fun onExit() { + if (navigated) return viewModelScope.launch { - saveOrDeleteDraft() - _finished.send(Unit) + // Don't save a draft for a message that's mid-send (send() will finish the screen). + if (!_state.value.sending) saveOrDeleteDraft() + finish() } } + /** Closes the screen exactly once, so send() and a stray back-press can't double-pop. */ + private suspend fun finish() { + if (navigated) return + navigated = true + _finished.send(Unit) + } + private suspend fun saveOrDeleteDraft() { val s = _state.value - val hasContent = s.to.isNotBlank() || s.cc.isNotBlank() || s.subject.isNotBlank() || s.body.isNotBlank() + val hasContent = s.to.isNotBlank() || s.cc.isNotBlank() || s.subject.isNotBlank() || + s.body.isNotBlank() || s.attachments.isNotEmpty() when { hasContent -> mailRepository.saveDraft( Draft( @@ -144,6 +159,7 @@ class ComposeViewModel @Inject constructor( subject = s.subject, body = s.body, updatedAt = System.currentTimeMillis(), + attachments = s.attachments, ), ) draftId != null -> mailRepository.deleteDraft(draftId) // an existing draft was emptied out @@ -151,23 +167,28 @@ class ComposeViewModel @Inject constructor( } fun send() { - val s = _state.value - val account = accounts.value.firstOrNull { it.id == s.fromAccountId } ?: accounts.value.firstOrNull() - when { - account == null -> _state.update { it.copy(error = "Add an account first") } - s.to.isBlank() -> _state.update { it.copy(error = "Add a recipient") } - else -> viewModelScope.launch { - _state.update { it.copy(sending = true, error = null) } - mailRepository.sendMessage( - OutgoingMessage(account.id, s.to, s.cc, s.subject, s.body, s.attachments), - ).fold( - onSuccess = { - draftId?.let { mailRepository.deleteDraft(it) } - _state.update { it.copy(sending = false) } - _finished.send(Unit) - }, - onFailure = { e -> _state.update { it.copy(sending = false, error = e.message ?: "Could not send") } }, - ) + viewModelScope.launch { + val s = _state.value + // Await the account list if it hasn't emitted yet, so an early tap doesn't wrongly + // report "Add an account first". + val available = accounts.value.ifEmpty { accountRepository.observeAccounts().first() } + val account = available.firstOrNull { it.id == s.fromAccountId } ?: available.firstOrNull() + when { + account == null -> _state.update { it.copy(error = "Add an account first") } + s.to.isBlank() -> _state.update { it.copy(error = "Add a recipient") } + else -> { + _state.update { it.copy(sending = true, error = null) } + mailRepository.sendMessage( + OutgoingMessage(account.id, s.to, s.cc, s.subject, s.body, s.attachments), + ).fold( + onSuccess = { + draftId?.let { mailRepository.deleteDraft(it) } + _state.update { it.copy(sending = false) } + finish() + }, + onFailure = { e -> _state.update { it.copy(sending = false, error = e.message ?: "Could not send") } }, + ) + } } } } diff --git a/app/src/main/kotlin/org/libremail/ui/mailbox/MailboxViewModel.kt b/app/src/main/kotlin/org/libremail/ui/mailbox/MailboxViewModel.kt index 47fa449..6f18500 100644 --- a/app/src/main/kotlin/org/libremail/ui/mailbox/MailboxViewModel.kt +++ b/app/src/main/kotlin/org/libremail/ui/mailbox/MailboxViewModel.kt @@ -51,7 +51,9 @@ class MailboxViewModel @Inject constructor( val q = query.trim() all.filter { message -> (accountId == null || message.accountId == accountId) && - (q.isEmpty() || message.matchesSearch(q)) + // Outside of search show only inbox rows; while searching show every match, + // including transient server-search hits that aren't in the inbox. + (if (q.isEmpty()) message.inInbox else message.matchesSearch(q)) } }.stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), emptyList()) @@ -99,6 +101,8 @@ class MailboxViewModel @Inject constructor( fun closeSearch() { _searchActive.value = false _searchQuery.value = "" + // Drop the transient server-search hits so they don't linger in the inbox. + viewModelScope.launch { mailRepository.clearSearchResults() } } fun onSearchQuery(query: String) { diff --git a/app/src/main/kotlin/org/libremail/ui/navigation/Routes.kt b/app/src/main/kotlin/org/libremail/ui/navigation/Routes.kt index 507152c..d65e6dc 100644 --- a/app/src/main/kotlin/org/libremail/ui/navigation/Routes.kt +++ b/app/src/main/kotlin/org/libremail/ui/navigation/Routes.kt @@ -14,7 +14,7 @@ object Routes { const val READER_ARG_ID = "messageId" const val READER_PATTERN = "reader/{$READER_ARG_ID}" - fun reader(messageId: String) = "reader/$messageId" + fun reader(messageId: String) = "reader/${Uri.encode(messageId)}" const val COMPOSE_ARG_TO = "to" const val COMPOSE_ARG_SUBJECT = "subject" diff --git a/app/src/main/kotlin/org/libremail/ui/reader/HtmlBody.kt b/app/src/main/kotlin/org/libremail/ui/reader/HtmlBody.kt index b8cac5a..2e6b74b 100644 --- a/app/src/main/kotlin/org/libremail/ui/reader/HtmlBody.kt +++ b/app/src/main/kotlin/org/libremail/ui/reader/HtmlBody.kt @@ -9,6 +9,8 @@ import android.webkit.WebSettings import android.webkit.WebView import android.webkit.WebViewClient import androidx.compose.runtime.Composable +import androidx.compose.runtime.remember +import androidx.compose.runtime.mutableStateOf import androidx.compose.ui.Modifier import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.viewinterop.AndroidView @@ -26,6 +28,9 @@ fun HtmlBody( modifier: Modifier = Modifier, ) { val context = LocalContext.current + // Tracks the content actually loaded so recompositions (star/attachment state changes) don't + // reload the page and throw away the user's scroll position. + val lastLoaded = remember { mutableStateOf?>(null) } AndroidView( modifier = modifier, factory = { ctx -> @@ -45,7 +50,17 @@ fun HtmlBody( webViewClient = object : WebViewClient() { override fun shouldOverrideUrlLoading(view: WebView?, request: WebResourceRequest?): Boolean { val url = request?.url ?: return false - runCatching { context.startActivity(Intent(Intent.ACTION_VIEW, url)) } + // Only open ordinary web/mail links, and only on an actual user tap — never + // auto-launch arbitrary intent:/market:/custom-scheme URIs (e.g. via a + // meta-refresh in a malicious email) or navigate inside the WebView. + val scheme = url.scheme?.lowercase() + if (scheme != "http" && scheme != "https" && scheme != "mailto") return true + if (!request.hasGesture()) return true + runCatching { + context.startActivity( + Intent(Intent.ACTION_VIEW, url).addFlags(Intent.FLAG_ACTIVITY_NEW_TASK), + ) + } return true } } @@ -53,7 +68,11 @@ fun HtmlBody( }, update = { webView -> webView.settings.blockNetworkLoads = !loadRemoteImages - webView.loadDataWithBaseURL(null, wrapHtml(html), "text/html", "UTF-8", null) + val key = html to loadRemoteImages + if (lastLoaded.value != key) { + lastLoaded.value = key + webView.loadDataWithBaseURL(null, wrapHtml(html), "text/html", "UTF-8", null) + } }, ) } diff --git a/app/src/main/kotlin/org/libremail/ui/reader/ReaderViewModel.kt b/app/src/main/kotlin/org/libremail/ui/reader/ReaderViewModel.kt index 313dcc3..c80efa3 100644 --- a/app/src/main/kotlin/org/libremail/ui/reader/ReaderViewModel.kt +++ b/app/src/main/kotlin/org/libremail/ui/reader/ReaderViewModel.kt @@ -11,9 +11,11 @@ import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.receiveAsFlow import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch +import org.libremail.data.settings.SettingsRepository import org.libremail.domain.model.Attachment import org.libremail.domain.model.Message import org.libremail.domain.repository.MailRepository @@ -39,6 +41,7 @@ sealed interface ReaderEvent { class ReaderViewModel @Inject constructor( savedStateHandle: SavedStateHandle, private val repository: MailRepository, + private val settingsRepository: SettingsRepository, ) : ViewModel() { private val messageId: String = checkNotNull(savedStateHandle[Routes.READER_ARG_ID]) @@ -50,6 +53,12 @@ class ReaderViewModel @Inject constructor( val events = _events.receiveAsFlow() init { + // Honor the global "load remote images by default" preference. + viewModelScope.launch { + if (settingsRepository.settings.first().loadRemoteImages) { + _state.update { it.copy(loadRemoteImages = true) } + } + } viewModelScope.launch { repository.openMessage(messageId).fold( onSuccess = { message -> _state.update { it.copy(loading = false, message = message) } }, diff --git a/app/src/main/res/xml/data_extraction_rules.xml b/app/src/main/res/xml/data_extraction_rules.xml new file mode 100644 index 0000000..39841b1 --- /dev/null +++ b/app/src/main/res/xml/data_extraction_rules.xml @@ -0,0 +1,17 @@ + + + + + + + + + + + diff --git a/app/src/test/kotlin/org/libremail/mail/GraphSenderTest.kt b/app/src/test/kotlin/org/libremail/mail/GraphSenderTest.kt index 1eed584..2ac0a78 100644 --- a/app/src/test/kotlin/org/libremail/mail/GraphSenderTest.kt +++ b/app/src/test/kotlin/org/libremail/mail/GraphSenderTest.kt @@ -34,6 +34,28 @@ class GraphSenderTest { assertEquals("c@z.com", cc.getJSONObject(0).getJSONObject("emailAddress").getString("address")) } + @Test + fun `recipients parse RFC822 display names into bare addresses`() { + val json = JSONObject( + buildSendMailPayload( + message(to = "John Doe ", cc = "\"Doe, Jane\" "), + emptyList(), + ), + ) + val msg = json.getJSONObject("message") + + val to = msg.getJSONArray("toRecipients") + assertEquals(1, to.length()) + val toAddress = to.getJSONObject(0).getJSONObject("emailAddress") + assertEquals("john@example.com", toAddress.getString("address")) + assertEquals("John Doe", toAddress.getString("name")) + + // The comma inside the quoted display name must not be treated as an address separator. + val cc = msg.getJSONArray("ccRecipients") + assertEquals(1, cc.length()) + assertEquals("jane@example.com", cc.getJSONObject(0).getJSONObject("emailAddress").getString("address")) + } + @Test fun `payload omits cc when blank and encodes attachments as base64`() { val file = File.createTempFile("graph-att", ".txt").apply { writeText("hello") } diff --git a/secrets.properties.example b/secrets.properties.example index 80dd531..48df0a9 100644 --- a/secrets.properties.example +++ b/secrets.properties.example @@ -5,3 +5,14 @@ # the Authorization Code + PKCE login flow; no client secret is required for an # installed Android app. GMAIL_OAUTH_CLIENT_ID= + +# Optional: Microsoft (Outlook) OAuth public client id. A working default ships with the build; +# set this only to use your own Azure app registration. +#OUTLOOK_OAUTH_CLIENT_ID= + +# Optional: release signing. When these are set, release builds are signed with this keystore; +# otherwise they fall back to the debug key (installable for testing, but not publishable). +#RELEASE_STORE_FILE=/absolute/path/to/release.keystore +#RELEASE_STORE_PASSWORD= +#RELEASE_KEY_ALIAS= +#RELEASE_KEY_PASSWORD=