From 96b2da1753e7f9ff3476f7ce505347b7d3a09b53 Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Fri, 3 Jul 2026 01:33:48 -0500 Subject: [PATCH] feat(compose): inline images end to end (picker to SMTP/Graph) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wire inline images through the whole send pipeline. Compose UI: an image-picker (image/*, persistable URI grant, mirroring the attachment picker) behind a new toolbar button appended at the END of the toolbar — after the block/link buttons and the font/size/align controls — so it never shifts the bullet button the compose E2E taps without scrolling. Picking an image adds an inline OutgoingAttachment and hands the editor a PendingInlineImage, which RichTextEditing.insertImage drops as a [image: name] token + RichImage(contentId) at the caret. Deleting the token drops the image: onBodyChange reconciles inline attachments against the body's surviving cid: references. Inline images are tracked in ComposeUiState alongside regular attachments but kept out of the attachment-chip row. Domain/persistence: OutgoingAttachment gains contentId/isInline; the shared draft/outbox attachment JSON carries them (drafts need no migration — an older draft reads back as a plain attachment). The outbox stages files by index as before but now also stores per-file {contentId,isInline} metadata in a new OutboxEntity.attachments column (Room 17 -> 18, MIGRATION_17_18, DEFAULT '' per the bccAddresses precedent so fresh-install == migrated; 18.json committed). The send worker pairs each staged file with its metadata by index (with a positional fallback for messages queued before the column existed). SMTP (SmtpSender): inline images wrap the body in a multipart/related, each with a Content-ID matching the HTML's cid: and inline disposition; regular attachments keep today's multipart/mixed shape. Graph (GraphSender): inline fileAttachments carry isInline + contentId. Tests: GreenMail asserts multipart/related with a Content-ID matching the cid; GraphSenderTest asserts the inline payload; a mapper test proves an inline image's cid<->file pairing round-trips a draft save/reopen; RichTextEditing tests cover insertImage (token/RichImage placement + offset shift + html round-trip); MigrationTest gains migrate17To18. Reader-side cid: rendering stays out of scope (follow-up). Closes #77 Co-Authored-By: Claude Opus 4.8 --- .../18.json | 467 ++++++++++++++++++ .../org/libremail/data/local/MigrationTest.kt | 24 + .../libremail/data/local/LibreMailDatabase.kt | 2 +- .../org/libremail/data/local/Mappers.kt | 28 +- .../org/libremail/data/local/Migrations.kt | 15 + .../data/local/entity/OutboxEntity.kt | 7 + .../data/repository/MailRepositoryImpl.kt | 4 + .../org/libremail/data/sync/SendWorker.kt | 36 +- .../kotlin/org/libremail/di/DatabaseModule.kt | 2 + .../libremail/domain/model/OutgoingMessage.kt | 16 +- .../kotlin/org/libremail/mail/GraphSender.kt | 106 ++-- .../org/libremail/mail/SendableAttachment.kt | 16 + .../kotlin/org/libremail/mail/SmtpSender.kt | 53 +- .../org/libremail/richtext/RichTextEditing.kt | 26 + .../org/libremail/ui/compose/ComposeScreen.kt | 17 +- .../libremail/ui/compose/ComposeViewModel.kt | 45 +- .../libremail/ui/compose/RichTextEditor.kt | 46 ++ app/src/main/res/values/strings.xml | 1 + .../local/OutgoingAttachmentMapperTest.kt | 67 +++ .../org/libremail/mail/GraphSenderTest.kt | 22 +- .../org/libremail/mail/SmtpSenderTest.kt | 40 +- .../libremail/richtext/RichTextEditingTest.kt | 23 + 22 files changed, 979 insertions(+), 84 deletions(-) create mode 100644 app/schemas/org.libremail.data.local.LibreMailDatabase/18.json create mode 100644 app/src/main/kotlin/org/libremail/mail/SendableAttachment.kt create mode 100644 app/src/test/kotlin/org/libremail/data/local/OutgoingAttachmentMapperTest.kt diff --git a/app/schemas/org.libremail.data.local.LibreMailDatabase/18.json b/app/schemas/org.libremail.data.local.LibreMailDatabase/18.json new file mode 100644 index 0000000..95e5656 --- /dev/null +++ b/app/schemas/org.libremail.data.local.LibreMailDatabase/18.json @@ -0,0 +1,467 @@ +{ + "formatVersion": 1, + "database": { + "version": 18, + "identityHash": "f4b80b1d988222a691f7aeea4d45e37e", + "entities": [ + { + "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, `folder` TEXT NOT NULL DEFAULT 'INBOX', `inInbox` INTEGER NOT NULL, `bodyFetched` INTEGER NOT NULL, `uid` INTEGER NOT NULL DEFAULT 0, 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": "folder", + "columnName": "folder", + "affinity": "TEXT", + "notNull": true, + "defaultValue": "'INBOX'" + }, + { + "fieldPath": "inInbox", + "columnName": "inInbox", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "bodyFetched", + "columnName": "bodyFetched", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "uid", + "columnName": "uid", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "0" + } + ], + "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`)" + }, + { + "name": "index_messages_accountId_folder_uid", + "unique": false, + "columnNames": [ + "accountId", + "folder", + "uid" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_messages_accountId_folder_uid` ON `${TABLE_NAME}` (`accountId`, `folder`, `uid`)" + } + ] + }, + { + "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, `contentId` TEXT, 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 + }, + { + "fieldPath": "contentId", + "columnName": "contentId", + "affinity": "TEXT" + } + ], + "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, `bccAddresses` TEXT NOT NULL DEFAULT '', `subject` TEXT NOT NULL, `body` TEXT NOT NULL, `createdAt` INTEGER NOT NULL, `lastError` TEXT, `bodyHtml` TEXT, `attachments` TEXT NOT NULL DEFAULT '', 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": "bccAddresses", + "columnName": "bccAddresses", + "affinity": "TEXT", + "notNull": true, + "defaultValue": "''" + }, + { + "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" + }, + { + "fieldPath": "bodyHtml", + "columnName": "bodyHtml", + "affinity": "TEXT" + }, + { + "fieldPath": "attachments", + "columnName": "attachments", + "affinity": "TEXT", + "notNull": true, + "defaultValue": "''" + } + ], + "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, `bccAddresses` TEXT NOT NULL DEFAULT '', `subject` TEXT NOT NULL, `body` TEXT NOT NULL, `updatedAt` INTEGER NOT NULL, `attachments` TEXT NOT NULL, `bodyHtml` TEXT, 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": "bccAddresses", + "columnName": "bccAddresses", + "affinity": "TEXT", + "notNull": true, + "defaultValue": "''" + }, + { + "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 + }, + { + "fieldPath": "bodyHtml", + "columnName": "bodyHtml", + "affinity": "TEXT" + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "id" + ] + } + }, + { + "tableName": "folders", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`accountId` TEXT NOT NULL, `fullName` TEXT NOT NULL, `displayName` TEXT NOT NULL, `role` TEXT NOT NULL, `selectable` INTEGER NOT NULL, `sortOrder` INTEGER NOT NULL, `specialUse` INTEGER NOT NULL DEFAULT 0, `hierarchyDelimiter` TEXT, PRIMARY KEY(`accountId`, `fullName`))", + "fields": [ + { + "fieldPath": "accountId", + "columnName": "accountId", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "fullName", + "columnName": "fullName", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "displayName", + "columnName": "displayName", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "role", + "columnName": "role", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "selectable", + "columnName": "selectable", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "sortOrder", + "columnName": "sortOrder", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "specialUse", + "columnName": "specialUse", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "0" + }, + { + "fieldPath": "hierarchyDelimiter", + "columnName": "hierarchyDelimiter", + "affinity": "TEXT" + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "accountId", + "fullName" + ] + } + }, + { + "tableName": "backfill_progress", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`accountId` TEXT NOT NULL, `folder` TEXT NOT NULL, `nextBeforeUid` INTEGER NOT NULL, `complete` INTEGER NOT NULL, PRIMARY KEY(`accountId`, `folder`))", + "fields": [ + { + "fieldPath": "accountId", + "columnName": "accountId", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "folder", + "columnName": "folder", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "nextBeforeUid", + "columnName": "nextBeforeUid", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "complete", + "columnName": "complete", + "affinity": "INTEGER", + "notNull": true + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "accountId", + "folder" + ] + } + } + ], + "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, 'f4b80b1d988222a691f7aeea4d45e37e')" + ] + } +} \ No newline at end of file diff --git a/app/src/androidTest/kotlin/org/libremail/data/local/MigrationTest.kt b/app/src/androidTest/kotlin/org/libremail/data/local/MigrationTest.kt index 8a4ec5b..e2939a8 100644 --- a/app/src/androidTest/kotlin/org/libremail/data/local/MigrationTest.kt +++ b/app/src/androidTest/kotlin/org/libremail/data/local/MigrationTest.kt @@ -200,6 +200,30 @@ class MigrationTest { db.close() } + /** v17 -> v18 (issue #77): `outbox.attachments` appears defaulting to '' and the queued row survives. */ + @Test + fun migrate17To18_addsEmptyAttachmentsToOutbox() { + helper.createDatabase(TEST_DB, 17).apply { + // v16 dropped the account tables, so a bare outbox row is all this migration needs. + execSQL( + "INSERT INTO outbox (id, accountId, toAddresses, ccAddresses, bccAddresses, subject, body, " + + "createdAt, lastError, bodyHtml) VALUES ('out-1', 'acct', 'bob@example.org', '', '', " + + "'Queued', 'Body', 3000, NULL, NULL)", + ) + close() + } + + val db = helper.runMigrationsAndValidate(TEST_DB, 18, true, MIGRATION_17_18) + + db.query("SELECT subject, attachments FROM outbox WHERE id = 'out-1'").use { c -> + assertTrue("the queued outbox row must survive", c.moveToFirst()) + assertEquals("Queued", c.getString(0)) + assertEquals("existing outbox rows read empty attachment metadata", "", c.getString(1)) + assertFalse("only the one pre-upgrade outbox row must survive", c.moveToNext()) + } + db.close() + } + /** The newest schema JSON exported to app/schemas (shipped to the test APK as assets). */ private fun latestExportedSchemaVersion(): Int { val schemaFolder = checkNotNull(LibreMailDatabase::class.java.canonicalName) 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 2494c04..af42bfc 100644 --- a/app/src/main/kotlin/org/libremail/data/local/LibreMailDatabase.kt +++ b/app/src/main/kotlin/org/libremail/data/local/LibreMailDatabase.kt @@ -35,7 +35,7 @@ import org.libremail.data.local.entity.OutboxEntity FolderEntity::class, BackfillProgressEntity::class, ], - version = 17, + version = 18, exportSchema = true, ) abstract class LibreMailDatabase : RoomDatabase() { 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 0854815..cc464cb 100644 --- a/app/src/main/kotlin/org/libremail/data/local/Mappers.kt +++ b/app/src/main/kotlin/org/libremail/data/local/Mappers.kt @@ -218,25 +218,41 @@ internal fun Draft.toEntity(): DraftEntity = DraftEntity( subject = subject, body = body, updatedAt = updatedAt, - attachments = attachments.toJson(), + attachments = attachments.toOutgoingAttachmentsJson(), bodyHtml = bodyHtml, ) -/** Serializes draft attachments as a JSON array of {uri, name} objects ("" when empty). */ -private fun List.toJson(): String { +/** + * Serializes outgoing attachments as a JSON array of `{uri, name, contentId?, isInline?}` objects + * ("" when empty). Shared by the drafts column and the outbox column, so an inline image's + * cid↔file pairing survives a draft save/reopen and a queued send alike. The two optional keys are + * omitted for a plain attachment, so an old draft (written before inline images) reads back with + * `contentId = null` / `isInline = false`. + */ +internal fun List.toOutgoingAttachmentsJson(): String { if (isEmpty()) return "" val array = JSONArray() - forEach { array.put(JSONObject().put("uri", it.uri).put("name", it.name)) } + forEach { attachment -> + val obj = JSONObject().put("uri", attachment.uri).put("name", attachment.name) + attachment.contentId?.let { obj.put("contentId", it) } + if (attachment.isInline) obj.put("isInline", true) + array.put(obj) + } return array.toString() } -private fun String.toOutgoingAttachments(): List { +internal 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")) + OutgoingAttachment( + uri = obj.getString("uri"), + name = obj.optString("name"), + contentId = obj.optString("contentId").takeIf { it.isNotEmpty() }, + isInline = obj.optBoolean("isInline", false), + ) } }.getOrDefault(emptyList()) } 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 566d0a4..1777f53 100644 --- a/app/src/main/kotlin/org/libremail/data/local/Migrations.kt +++ b/app/src/main/kotlin/org/libremail/data/local/Migrations.kt @@ -344,3 +344,18 @@ val MIGRATION_16_17 = object : Migration(16, 17) { db.execSQL("ALTER TABLE `attachments` ADD COLUMN `contentId` TEXT") } } + +/** + * v17 -> v18: inline-image support for outgoing mail (issue #77; preserves existing data). Adds an + * `attachments` column to `outbox` holding JSON metadata (`{uri, name, contentId?, isInline?}`, one + * entry per staged file in index order) so the send worker can pair an inline image's `Content-ID` + * with its staged file. `DEFAULT ''` matches the entity's `@ColumnInfo(defaultValue = "")` so the + * fresh-install schema validates identically to the migrated one (the MIGRATION_9_10 `bccAddresses` + * pattern); a message queued before the upgrade reads back "" and its staged files are still sent as + * plain attachments (the send worker's positional fallback). + */ +val MIGRATION_17_18 = object : Migration(17, 18) { + override fun migrate(db: SupportSQLiteDatabase) { + db.execSQL("ALTER TABLE `outbox` ADD COLUMN `attachments` TEXT NOT NULL DEFAULT ''") + } +} diff --git a/app/src/main/kotlin/org/libremail/data/local/entity/OutboxEntity.kt b/app/src/main/kotlin/org/libremail/data/local/entity/OutboxEntity.kt index 7481b2d..4b796d9 100644 --- a/app/src/main/kotlin/org/libremail/data/local/entity/OutboxEntity.kt +++ b/app/src/main/kotlin/org/libremail/data/local/entity/OutboxEntity.kt @@ -19,4 +19,11 @@ data class OutboxEntity( val lastError: String? = null, /** HTML form of [body] when composed with formatting; null sends `text/plain` only. */ val bodyHtml: String? = null, + /** + * JSON metadata for the staged attachments, in staging-index order (the same + * `{uri, name, contentId?, isInline?}` shape drafts use), or "" when there are none. The bytes + * live on disk under `cacheDir/outbox///`; this column carries each part's `Content-ID` + * and inline flag so the send worker can pair an inline image's cid with its file. + */ + @ColumnInfo(defaultValue = "") val attachments: String = "", ) 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 669f59f..8463729 100644 --- a/app/src/main/kotlin/org/libremail/data/repository/MailRepositoryImpl.kt +++ b/app/src/main/kotlin/org/libremail/data/repository/MailRepositoryImpl.kt @@ -32,6 +32,7 @@ import org.libremail.data.local.entity.MessageRouting import org.libremail.data.local.entity.OutboxEntity import org.libremail.data.local.toDomain import org.libremail.data.local.toEntity +import org.libremail.data.local.toOutgoingAttachmentsJson import org.libremail.data.settings.AccountSettingsRepository import org.libremail.data.settings.SignatureRepository import org.libremail.data.sync.MailConnectionFactory @@ -404,6 +405,9 @@ class MailRepositoryImpl @Inject constructor( body = outgoing.body, createdAt = System.currentTimeMillis(), bodyHtml = outgoing.bodyHtml, + // Persist the cid↔file pairing (in staging-index order) so the send worker can attach + // an inline image with its Content-ID even though the files are looked up by index. + attachments = outgoing.attachments.toOutgoingAttachmentsJson(), ), ) sendScheduler.sendNow() 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 29f7ac2..bdd1eba 100644 --- a/app/src/main/kotlin/org/libremail/data/sync/SendWorker.kt +++ b/app/src/main/kotlin/org/libremail/data/sync/SendWorker.kt @@ -12,12 +12,15 @@ import dagger.assisted.AssistedInject import org.libremail.data.local.dao.AccountDao import org.libremail.data.local.dao.OutboxDao import org.libremail.data.local.toDomain +import org.libremail.data.local.toOutgoingAttachments import org.libremail.data.security.EncryptedCacheGuard import org.libremail.domain.model.Account import org.libremail.domain.model.AuthType +import org.libremail.domain.model.OutgoingAttachment import org.libremail.domain.model.OutgoingMessage import org.libremail.mail.GraphSendException import org.libremail.mail.GraphSender +import org.libremail.mail.SendableAttachment import org.libremail.mail.SmtpSender import java.io.File import kotlin.coroutines.cancellation.CancellationException @@ -69,15 +72,15 @@ class SendWorker @AssistedInject constructor( body = entity.body, bodyHtml = entity.bodyHtml, ) - val files = orderedAttachments(attachmentDir) + val attachments = stagedAttachments(attachmentDir, entity.attachments.toOutgoingAttachments()) if (account.authType == AuthType.OAUTH_OUTLOOK) { - sendOutlook(connectionFactory, account, message, files) + sendOutlook(connectionFactory, account, message, attachments) } else { smtpSender.send( connectionFactory.smtpParamsFor(account), from = account.email, message = message, - attachments = files, + attachments = attachments, ) } }.fold( @@ -114,18 +117,18 @@ class SendWorker @AssistedInject constructor( connectionFactory: MailConnectionFactory, account: Account, message: OutgoingMessage, - files: List, + attachments: List, ) { try { val token = connectionFactory.graphTokenFor(account) - graphSender.send(token, message, files) + graphSender.send(token, message, attachments) } catch (e: GraphSendException) { if (e.mayHaveSent) throw e smtpSender.send( connectionFactory.smtpParamsFor(account), from = account.email, message = message, - attachments = files, + attachments = attachments, ) } catch (e: CancellationException) { throw e @@ -136,13 +139,30 @@ class SendWorker @AssistedInject constructor( connectionFactory.smtpParamsFor(account), from = account.email, message = message, - attachments = files, + attachments = attachments, ) } } + /** + * Pairs each staged file with its [metadata] entry by staging index, so an inline image keeps its + * `Content-ID`/inline flag alongside its file. A missing file (a copy that failed) is skipped. + * When [metadata] is empty — a message queued before the outbox carried attachment metadata, or a + * genuinely attachment-less one — falls back to restoring the staged files positionally as plain + * attachments, so an in-flight message from before the upgrade still sends its files. + */ + private fun stagedAttachments(dir: File, metadata: List): List { + if (metadata.isEmpty()) { + return orderedAttachmentFiles(dir).map { SendableAttachment(it) } + } + return metadata.mapIndexedNotNull { index, meta -> + val file = File(dir, index.toString()).listFiles()?.firstOrNull() ?: return@mapIndexedNotNull null + SendableAttachment(file, contentId = meta.contentId, isInline = meta.isInline) + } + } + /** Attachments are staged one-per-indexed-subdirectory so their original order is preserved. */ - private fun orderedAttachments(dir: File): List = dir.listFiles() + private fun orderedAttachmentFiles(dir: File): List = dir.listFiles() ?.filter { it.isDirectory } ?.sortedBy { it.name.toIntOrNull() ?: Int.MAX_VALUE } ?.mapNotNull { it.listFiles()?.firstOrNull() } diff --git a/app/src/main/kotlin/org/libremail/di/DatabaseModule.kt b/app/src/main/kotlin/org/libremail/di/DatabaseModule.kt index 30a82f6..4143f96 100644 --- a/app/src/main/kotlin/org/libremail/di/DatabaseModule.kt +++ b/app/src/main/kotlin/org/libremail/di/DatabaseModule.kt @@ -23,6 +23,7 @@ import org.libremail.data.local.MIGRATION_13_14 import org.libremail.data.local.MIGRATION_14_15 import org.libremail.data.local.MIGRATION_15_16 import org.libremail.data.local.MIGRATION_16_17 +import org.libremail.data.local.MIGRATION_17_18 import org.libremail.data.local.MIGRATION_1_2 import org.libremail.data.local.MIGRATION_2_3 import org.libremail.data.local.MIGRATION_3_4 @@ -65,6 +66,7 @@ object DatabaseModule { MIGRATION_14_15, MIGRATION_15_16, MIGRATION_16_17, + MIGRATION_17_18, ) // No destructive fallback: the migration chain is complete, and silently dropping the // mail/message tables would lose cached data. A missing migration should fail loudly in diff --git a/app/src/main/kotlin/org/libremail/domain/model/OutgoingMessage.kt b/app/src/main/kotlin/org/libremail/domain/model/OutgoingMessage.kt index 7ee8892..d1c4bc5 100644 --- a/app/src/main/kotlin/org/libremail/domain/model/OutgoingMessage.kt +++ b/app/src/main/kotlin/org/libremail/domain/model/OutgoingMessage.kt @@ -19,5 +19,17 @@ data class OutgoingMessage( val attachments: List = emptyList(), ) -/** A file the user attached, referenced by its content-URI string until it is sent. */ -data class OutgoingAttachment(val uri: String, val name: String) +/** + * A file the user attached, referenced by its content-URI string until it is sent. + * + * An inline image (inserted into the body, not listed as a separate attachment) has [isInline] true + * and a [contentId]: the body's HTML references it as ``, and the sender + * emits the part with a matching `Content-ID` (SMTP `multipart/related`) or `isInline`+`contentId` + * (Graph). A regular attachment leaves both at their defaults. + */ +data class OutgoingAttachment( + val uri: String, + val name: String, + val contentId: String? = null, + val isInline: Boolean = false, +) diff --git a/app/src/main/kotlin/org/libremail/mail/GraphSender.kt b/app/src/main/kotlin/org/libremail/mail/GraphSender.kt index 2071e71..936dc54 100644 --- a/app/src/main/kotlin/org/libremail/mail/GraphSender.kt +++ b/app/src/main/kotlin/org/libremail/mail/GraphSender.kt @@ -7,7 +7,6 @@ import kotlinx.coroutines.withContext import org.json.JSONArray import org.json.JSONObject import org.libremail.domain.model.OutgoingMessage -import java.io.File import java.io.IOException import java.net.HttpURLConnection import java.net.URL @@ -30,48 +29,51 @@ class GraphSendException(message: String, val mayHaveSent: Boolean, cause: Throw @Singleton class GraphSender @Inject constructor() { - suspend fun send(accessToken: String, message: OutgoingMessage, attachments: List = emptyList()) = - withContext(Dispatchers.IO) { - val payload = buildSendMailPayload(message, attachments) - val connection = (URL(SEND_MAIL_URL).openConnection() as HttpURLConnection).apply { - requestMethod = "POST" - connectTimeout = TIMEOUT_MS - readTimeout = TIMEOUT_MS - doOutput = true - setRequestProperty("Authorization", "Bearer $accessToken") - setRequestProperty("Content-Type", "application/json; charset=utf-8") - } - try { - // 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 HTTP_OK_MIN..HTTP_OK_MAX) { - val body = (connection.errorStream ?: connection.inputStream) - ?.bufferedReader()?.use { it.readText() }.orEmpty() - // An explicit non-2xx means Graph rejected (did not send) — safe to fall back. - throw GraphSendException( - "Graph sendMail failed (HTTP $code): ${body.take(ERROR_BODY_LIMIT)}", - mayHaveSent = false, - ) - } - } finally { - connection.disconnect() - } + suspend fun send( + accessToken: String, + message: OutgoingMessage, + attachments: List = emptyList(), + ) = withContext(Dispatchers.IO) { + val payload = buildSendMailPayload(message, attachments) + val connection = (URL(SEND_MAIL_URL).openConnection() as HttpURLConnection).apply { + requestMethod = "POST" + connectTimeout = TIMEOUT_MS + readTimeout = TIMEOUT_MS + doOutput = true + setRequestProperty("Authorization", "Bearer $accessToken") + setRequestProperty("Content-Type", "application/json; charset=utf-8") } + try { + // 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 HTTP_OK_MIN..HTTP_OK_MAX) { + val body = (connection.errorStream ?: connection.inputStream) + ?.bufferedReader()?.use { it.readText() }.orEmpty() + // An explicit non-2xx means Graph rejected (did not send) — safe to fall back. + throw GraphSendException( + "Graph sendMail failed (HTTP $code): ${body.take(ERROR_BODY_LIMIT)}", + mayHaveSent = false, + ) + } + } finally { + connection.disconnect() + } + } private companion object { const val SEND_MAIL_URL = "https://graph.microsoft.com/v1.0/me/sendMail" @@ -83,7 +85,7 @@ class GraphSender @Inject constructor() { } /** Builds the Graph `sendMail` JSON body (a pure function, so it is unit-testable without a network). */ -internal fun buildSendMailPayload(message: OutgoingMessage, attachments: List): String { +internal fun buildSendMailPayload(message: OutgoingMessage, attachments: List): String { // Graph carries a single body object: send HTML when the message was formatted (Outlook renders // it and derives its own plaintext), otherwise plain text so unformatted mail is unchanged. val body = if (message.bodyHtml != null) { @@ -103,13 +105,17 @@ internal fun buildSendMailPayload(message: OutgoingMessage, attachments: List - items.put( - JSONObject() - .put("@odata.type", "#microsoft.graph.fileAttachment") - .put("name", file.name) - .put("contentBytes", Base64.getEncoder().encodeToString(file.readBytes())), - ) + attachments.forEach { attachment -> + val obj = JSONObject() + .put("@odata.type", "#microsoft.graph.fileAttachment") + .put("name", attachment.file.name) + .put("contentBytes", Base64.getEncoder().encodeToString(attachment.file.readBytes())) + // An inline image the HTML references as `cid:contentId` — Graph renders it in the body + // rather than listing it as a downloadable attachment. + if (attachment.isInlineImage) { + obj.put("isInline", true).put("contentId", attachment.contentId) + } + items.put(obj) } mail.put("attachments", items) } diff --git a/app/src/main/kotlin/org/libremail/mail/SendableAttachment.kt b/app/src/main/kotlin/org/libremail/mail/SendableAttachment.kt new file mode 100644 index 0000000..7caa4a1 --- /dev/null +++ b/app/src/main/kotlin/org/libremail/mail/SendableAttachment.kt @@ -0,0 +1,16 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.mail + +import java.io.File + +/** + * One staged attachment ready to send: the on-disk [file] plus how it should be attached. An inline + * image has [isInline] true and a non-null [contentId] — the body's HTML references it as + * `cid:contentId`, so the SMTP sender emits it inside a `multipart/related` with a matching + * `Content-ID` + inline disposition, and the Graph sender marks the payload `isInline` with that + * `contentId`. A regular attachment leaves both defaults and is attached exactly as before. + */ +data class SendableAttachment(val file: File, val contentId: String? = null, val isInline: Boolean = false) { + /** True only for a genuine inline image (inline flag set and a content id to reference it by). */ + val isInlineImage: Boolean get() = isInline && contentId != null +} diff --git a/app/src/main/kotlin/org/libremail/mail/SmtpSender.kt b/app/src/main/kotlin/org/libremail/mail/SmtpSender.kt index 7ce2d18..8e4f650 100644 --- a/app/src/main/kotlin/org/libremail/mail/SmtpSender.kt +++ b/app/src/main/kotlin/org/libremail/mail/SmtpSender.kt @@ -13,7 +13,6 @@ import kotlinx.coroutines.withContext import org.libremail.domain.model.MailSecurity import org.libremail.domain.model.OutgoingMessage import org.libremail.domain.model.SmtpParams -import java.io.File import java.util.Date import java.util.Properties import javax.inject.Inject @@ -27,7 +26,7 @@ class SmtpSender @Inject constructor() { params: SmtpParams, from: String, message: OutgoingMessage, - attachments: List = emptyList(), + attachments: List = emptyList(), ) = withContext(Dispatchers.IO) { val protocol = if (params.security == MailSecurity.SSL_TLS) "smtps" else "smtp" val props = Properties().apply { @@ -78,22 +77,52 @@ class SmtpSender @Inject constructor() { } /** - * Sets the message body: + * Sets the message body and attaches files: * - plaintext-only ([OutgoingMessage.bodyHtml] null): a single `text/plain` part, exactly as * before, so unformatted mail is unchanged on the wire; * - formatted: a `multipart/alternative` of `text/plain` (fallback, first) + `text/html` * (preferred, last, per RFC 2046). - * When there are attachments the body is nested inside a `multipart/mixed` as its first part. + * + * Inline images (referenced by the HTML as `cid:…`) wrap the body in a `multipart/related` so a + * client can resolve those references; each carries a `Content-ID` and inline disposition. Regular + * attachments keep today's shape — the body (related or plain) becomes the first part of a + * `multipart/mixed`, followed by the attachment parts. */ - private fun applyBody(mime: MimeMessage, message: OutgoingMessage, attachments: List) { - if (attachments.isEmpty()) { - setBody(mime, message) - return + private fun applyBody(mime: MimeMessage, message: OutgoingMessage, attachments: List) { + val inline = attachments.filter { it.isInlineImage } + val regular = attachments.filterNot { it.isInlineImage } + when { + inline.isEmpty() && regular.isEmpty() -> setBody(mime, message) + regular.isEmpty() -> mime.setContent(relatedBody(message, inline)) + else -> { + val mixed = MimeMultipart("mixed") + mixed.addBodyPart(bodyPart(message, inline)) + regular.forEach { mixed.addBodyPart(MimeBodyPart().apply { attachFile(it.file) }) } + mime.setContent(mixed) + } } - val mixed = MimeMultipart("mixed") - mixed.addBodyPart(MimeBodyPart().also { setBody(it, message) }) - attachments.forEach { file -> mixed.addBodyPart(MimeBodyPart().apply { attachFile(file) }) } - mime.setContent(mixed) + } + + /** The body part for a `multipart/mixed`: the plain/alternative body, wrapped in related if inline. */ + private fun bodyPart(message: OutgoingMessage, inline: List): MimeBodyPart = + if (inline.isEmpty()) { + MimeBodyPart().also { setBody(it, message) } + } else { + MimeBodyPart().apply { setContent(relatedBody(message, inline)) } + } + + /** A `multipart/related`: the body (plain/alternative) followed by each inline image part. */ + private fun relatedBody(message: OutgoingMessage, inline: List): MimeMultipart = + MimeMultipart("related").apply { + addBodyPart(MimeBodyPart().also { setBody(it, message) }) + inline.forEach { addBodyPart(inlinePart(it)) } + } + + /** An inline image part: attached bytes, a `Content-ID` the HTML's `cid:` matches, inline disposition. */ + private fun inlinePart(attachment: SendableAttachment): MimeBodyPart = MimeBodyPart().apply { + attachFile(attachment.file) + contentID = "<${attachment.contentId}>" + setDisposition(MimeBodyPart.INLINE) } /** Writes the body onto [part]: plain text, or a text/plain + text/html alternative when formatted. */ diff --git a/app/src/main/kotlin/org/libremail/richtext/RichTextEditing.kt b/app/src/main/kotlin/org/libremail/richtext/RichTextEditing.kt index 82468bc..ed67e10 100644 --- a/app/src/main/kotlin/org/libremail/richtext/RichTextEditing.kt +++ b/app/src/main/kotlin/org/libremail/richtext/RichTextEditing.kt @@ -141,6 +141,32 @@ object RichTextEditing { } return aligns.distinct().singleOrNull() } + + /** + * Inserts an inline image at [at]: a visible [imageToken] backed by a [RichImage] carrying + * [contentId] and [name]. Every span/link/alignment/image at or after the insertion point shifts + * by the token's length (a run the caret sits inside grows to keep covering the same text), and + * the returned selection lands just past the inserted token. Deleting the token later removes the + * [RichImage] with it, since the model derives its images from the tokens in the text. + */ + fun insertImage(content: RichTextContent, at: Int, contentId: String, name: String): EditResult { + val pos = at.coerceIn(0, content.text.length) + val token = imageToken(name) + val len = token.length + val newText = content.text.substring(0, pos) + token + content.text.substring(pos) + fun shiftStart(o: Int): Int = if (o >= pos) o + len else o + fun shiftEnd(o: Int): Int = if (o > pos) o + len else o + val shiftedImages = content.images.map { it.copy(start = shiftStart(it.start), end = shiftEnd(it.end)) } + val images = (shiftedImages + RichImage(pos, pos + len, contentId, name)).sortedBy { it.start } + val updated = content.copy( + text = newText, + spans = content.spans.map { it.copy(start = shiftStart(it.start), end = shiftEnd(it.end)) }, + links = content.links.map { it.copy(start = shiftStart(it.start), end = shiftEnd(it.end)) }, + alignments = content.alignments.map { it.copy(start = shiftStart(it.start), end = shiftEnd(it.end)) }, + images = images, + ) + return EditResult(updated, pos + len, pos + len) + } } private fun insertFor(marker: BlockMarker, ordinal: Int): String = when (marker) { diff --git a/app/src/main/kotlin/org/libremail/ui/compose/ComposeScreen.kt b/app/src/main/kotlin/org/libremail/ui/compose/ComposeScreen.kt index 3c196dd..fdc0aad 100644 --- a/app/src/main/kotlin/org/libremail/ui/compose/ComposeScreen.kt +++ b/app/src/main/kotlin/org/libremail/ui/compose/ComposeScreen.kt @@ -97,6 +97,17 @@ fun ComposeScreen(onBack: () -> Unit, viewModel: ComposeViewModel = hiltViewMode ) } + // Inline-image picker (image/*). Mirrors the attachment picker's persistable grant so a draft can + // reopen the image later; the ViewModel then hands the editor a token to drop at the caret. + val imagePicker = rememberLauncherForActivityResult(ActivityResultContracts.OpenDocument()) { uri -> + uri?.let { + runCatching { + context.contentResolver.takePersistableUriPermission(it, Intent.FLAG_GRANT_READ_URI_PERMISSION) + } + viewModel.onImagePicked(it.toString(), queryFileName(context, it)) + } + } + // Reflect the current READ_CONTACTS grant without ever prompting: the request now lives in the // onboarding contacts step (#127) and the Settings entry (#129), so compose only reads state. // Re-checked on resume so enabling autocomplete later (e.g. from Settings) takes effect the next @@ -179,7 +190,8 @@ fun ComposeScreen(onBack: () -> Unit, viewModel: ComposeViewModel = hiltViewMode modifier = Modifier.fillMaxWidth(), ) AttachmentsSection( - attachments = state.attachments, + // Inline images live in the body (as tokens), not as separate attachment chips. + attachments = state.attachments.filterNot { it.isInline }, highlight = state.highlightAttach, onHighlightShown = viewModel::consumeAttachHighlight, onAttach = { attachmentPicker.launch(arrayOf("*/*")) }, @@ -193,6 +205,9 @@ fun ComposeScreen(onBack: () -> Unit, viewModel: ComposeViewModel = hiltViewMode label = stringResource(R.string.compose_body), modifier = Modifier.fillMaxWidth().weight(1f), resolveFont = FontRegistry::resolveFontFamily, + onPickImage = { imagePicker.launch(arrayOf("image/*")) }, + pendingImage = state.pendingInlineImage, + onImageInserted = viewModel::onInlineImageInserted, ) } 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 c0cacd3..413a637 100644 --- a/app/src/main/kotlin/org/libremail/ui/compose/ComposeViewModel.kt +++ b/app/src/main/kotlin/org/libremail/ui/compose/ComposeViewModel.kt @@ -60,8 +60,17 @@ data class ComposeUiState( val showAttachmentPrompt: Boolean = false, /** Draw the eye to the attach button — the user answered the attachment prompt with "Yes". */ val highlightAttach: Boolean = false, + /** + * A just-picked inline image waiting for the editor to drop its token at the caret. The editor + * consumes it (via [ComposeViewModel.onInlineImageInserted]) once inserted; a transient signal, so + * it is excluded from the autosaved [DraftContent]. + */ + val pendingInlineImage: PendingInlineImage? = null, ) +/** A picked inline image the editor should insert: the token to show and the [contentId] to link it by. */ +data class PendingInlineImage(val contentId: String, val name: String) + /** The persisted draft fields; autosave fires only when one of these actually changes (#177). */ private data class DraftContent( val to: String, @@ -198,8 +207,42 @@ class ComposeViewModel @Inject constructor( * The rich editor reports the current body in both forms: [plain] (also the plaintext fallback) * and [html], which is null when the content carries no formatting so the message stays * plaintext-only. Both are held for sending and for saving the draft. + * + * Inline images are reconciled against the body here: an inline attachment is kept only while its + * `cid:` is still referenced by the HTML (or is the one currently being inserted), so deleting an + * image's `[image: …]` token drops the image itself. Regular attachments are never touched. */ - fun onBodyChange(plain: String, html: String?) = _state.update { it.copy(body = plain, bodyHtml = html) } + fun onBodyChange(plain: String, html: String?) = _state.update { s -> + val referenced = referencedContentIds(html) + val keptAttachments = s.attachments.filter { attachment -> + !attachment.isInline || + attachment.contentId in referenced || + attachment.contentId == s.pendingInlineImage?.contentId + } + s.copy(body = plain, bodyHtml = html, attachments = keptAttachments) + } + + /** The content ids the body's HTML still references (`cid:…`), used to prune deleted inline images. */ + private fun referencedContentIds(html: String?): Set = + html?.let { RichTextHtml.fromHtml(it).images.mapTo(mutableSetOf()) { image -> image.contentId } }.orEmpty() + + /** + * A picked inline image: track it as an inline attachment (alongside regular ones) and hand the + * editor a [PendingInlineImage] to drop at the caret. The generated content id ties the body's + * `cid:` reference to the attachment the sender emits with a matching `Content-ID`. + */ + fun onImagePicked(uri: String, name: String) { + val contentId = "img-${UUID.randomUUID()}@libremail" + _state.update { + it.copy( + attachments = it.attachments + OutgoingAttachment(uri, name, contentId, isInline = true), + pendingInlineImage = PendingInlineImage(contentId, name), + ) + } + } + + /** The editor has inserted the pending image's token; clear the transient signal. */ + fun onInlineImageInserted() = _state.update { it.copy(pendingInlineImage = null) } fun selectFrom(accountId: String) { viewModelScope.launch { applySignature(accountId) } diff --git a/app/src/main/kotlin/org/libremail/ui/compose/RichTextEditor.kt b/app/src/main/kotlin/org/libremail/ui/compose/RichTextEditor.kt index 06951e8..cc48459 100644 --- a/app/src/main/kotlin/org/libremail/ui/compose/RichTextEditor.kt +++ b/app/src/main/kotlin/org/libremail/ui/compose/RichTextEditor.kt @@ -19,6 +19,7 @@ import androidx.compose.material3.OutlinedTextField import androidx.compose.material3.Text import androidx.compose.material3.TextButton import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember @@ -102,6 +103,9 @@ fun RichTextBodyField( label: String, modifier: Modifier = Modifier, resolveFont: (String) -> FontFamily? = { null }, + onPickImage: (() -> Unit)? = null, + pendingImage: PendingInlineImage? = null, + onImageInserted: () -> Unit = {}, ) { val linkColor = MaterialTheme.colorScheme.primary val seed = remember { seedContent(body, bodyHtml) } @@ -132,6 +136,16 @@ fun RichTextBodyField( var showFontColorPicker by remember { mutableStateOf(false) } var showHighlightPicker by remember { mutableStateOf(false) } + // A picked image (chosen in ComposeScreen) is inserted here at the live caret — the editor owns the + // cursor — then reported handled so the caller clears it. Keyed on the image so re-picking the same + // file (a new instance) still fires. + LaunchedEffect(pendingImage) { + pendingImage?.let { image -> + emit(applyImageInsert(value, image.contentId, image.name, linkColor, resolveFont)) + onImageInserted() + } + } + Column(modifier) { FormattingToolbar( value = value, @@ -140,6 +154,7 @@ fun RichTextBodyField( onLink = { showLinkDialog = true }, onFontColor = { showFontColorPicker = true }, onHighlight = { showHighlightPicker = true }, + onInsertImage = onPickImage, onFontSize = { pt -> emit( if (pt != null) { @@ -245,6 +260,7 @@ private fun FormattingToolbar( onFontSize: (Int?) -> Unit, onAlignment: (RichAlign) -> Unit, onFont: (String?) -> Unit, + onInsertImage: (() -> Unit)?, ) { val content = value.annotatedString.toRichContent() val start = value.selection.min @@ -339,6 +355,16 @@ private fun FormattingToolbar( selected = RichTextEditing.alignmentAt(content, start, end), onSelect = onAlignment, ) + // The image button is appended last (like the other trailing controls) so it never shifts the + // block buttons the compose E2E taps without scrolling. Only shown when the host wires a picker. + if (onInsertImage != null) { + FormatButton( + label = "🖼", + description = stringResource(R.string.format_image), + active = false, + onClick = onInsertImage, + ) + } } } @@ -534,6 +560,26 @@ internal fun applyAlignment( return TextFieldValue(updated.toAnnotatedString(linkColor, resolveFont), value.selection) } +/** Inserts an inline-image token at the caret and lands the selection just past it (for toolbar wiring). */ +internal fun applyImageInsert( + value: TextFieldValue, + contentId: String, + name: String, + linkColor: Color, + resolveFont: (String) -> FontFamily? = { null }, +): TextFieldValue { + val result = RichTextEditing.insertImage( + value.annotatedString.toRichContent(), + value.selection.min, + contentId, + name, + ) + return TextFieldValue( + result.content.toAnnotatedString(linkColor, resolveFont), + TextRange(result.selectionStart, result.selectionEnd), + ) +} + /** * Clears every span of style kind [kind] over the selection - the color pickers' "no color"/"none" * entry, which must remove the style outright regardless of its value. [applyStyle] cannot do this: diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index a6016d0..f3a647d 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -129,6 +129,7 @@ Align end Font Default + Insert image Drafts diff --git a/app/src/test/kotlin/org/libremail/data/local/OutgoingAttachmentMapperTest.kt b/app/src/test/kotlin/org/libremail/data/local/OutgoingAttachmentMapperTest.kt new file mode 100644 index 0000000..4c640c8 --- /dev/null +++ b/app/src/test/kotlin/org/libremail/data/local/OutgoingAttachmentMapperTest.kt @@ -0,0 +1,67 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.local + +import org.junit.Test +import org.libremail.data.local.entity.DraftEntity +import org.libremail.domain.model.Draft +import org.libremail.domain.model.OutgoingAttachment +import kotlin.test.assertEquals + +/** + * The draft attachment JSON mapper must preserve an inline image's cid↔file pairing across a save and + * reopen (#77), while still reading an older draft (written before inline images) as a plain + * attachment. The same `{uri, name, contentId?, isInline?}` serialization also backs the outbox + * column, so this pins the wire format both use. + */ +class OutgoingAttachmentMapperTest { + + @Test + fun `draft attachments including an inline image round-trip through the entity`() { + val draft = Draft( + id = "d1", + accountId = "acct", + to = "bob@example.org", + cc = "", + subject = "With image", + body = "See [image: cat.png]", + updatedAt = 1_000L, + bodyHtml = "

See \"cat.png\"

", + attachments = listOf( + OutgoingAttachment("content://docs/report", "report.pdf"), + OutgoingAttachment("content://images/cat", "cat.png", contentId = "cat@libremail", isInline = true), + ), + ) + + val restored = draft.toEntity().toDomain() + + assertEquals(draft.attachments, restored.attachments) + assertEquals(draft.bodyHtml, restored.bodyHtml) + } + + @Test + fun `a draft json written before inline images reads back as a plain attachment`() { + val legacy = DraftEntity( + id = "d", + accountId = null, + toAddresses = "x@y.org", + ccAddresses = "", + bccAddresses = "", + subject = "", + body = "", + updatedAt = 0L, + attachments = """[{"uri":"content://f","name":"a.txt"}]""", + bodyHtml = null, + ) + + assertEquals( + OutgoingAttachment("content://f", "a.txt", contentId = null, isInline = false), + legacy.toDomain().attachments.single(), + ) + } + + @Test + fun `no attachments serialize to the empty string`() { + assertEquals("", emptyList().toOutgoingAttachmentsJson()) + assertEquals(emptyList(), "".toOutgoingAttachments()) + } +} diff --git a/app/src/test/kotlin/org/libremail/mail/GraphSenderTest.kt b/app/src/test/kotlin/org/libremail/mail/GraphSenderTest.kt index fb6a58a..087899d 100644 --- a/app/src/test/kotlin/org/libremail/mail/GraphSenderTest.kt +++ b/app/src/test/kotlin/org/libremail/mail/GraphSenderTest.kt @@ -93,7 +93,8 @@ class GraphSenderTest { fun `payload omits cc and bcc when blank and encodes attachments as base64`() { val file = File.createTempFile("graph-att", ".txt").apply { writeText("hello") } try { - val msg = JSONObject(buildSendMailPayload(message(to = "a@x.com"), listOf(file))).getJSONObject("message") + val msg = JSONObject(buildSendMailPayload(message(to = "a@x.com"), listOf(SendableAttachment(file)))) + .getJSONObject("message") assertFalse(msg.has("ccRecipients")) assertFalse(msg.has("bccRecipients")) @@ -103,8 +104,27 @@ class GraphSenderTest { assertEquals("#microsoft.graph.fileAttachment", attachment.getString("@odata.type")) assertEquals(file.name, attachment.getString("name")) assertEquals("aGVsbG8=", attachment.getString("contentBytes")) // base64("hello") + // A plain attachment carries no inline markers. + assertFalse(attachment.has("isInline")) + assertFalse(attachment.has("contentId")) } finally { file.delete() } } + + @Test + fun `an inline image attachment is marked isInline with its contentId`() { + val image = File.createTempFile("graph-inline", ".png").apply { writeText("PNGDATA") } + try { + val message = message(to = "a@x.com").copy(bodyHtml = "

") + val inline = SendableAttachment(image, contentId = "logo@libremail", isInline = true) + val attachment = JSONObject(buildSendMailPayload(message, listOf(inline))) + .getJSONObject("message").getJSONArray("attachments").getJSONObject(0) + assertEquals(image.name, attachment.getString("name")) + assertTrue(attachment.getBoolean("isInline")) + assertEquals("logo@libremail", attachment.getString("contentId")) + } finally { + image.delete() + } + } } diff --git a/app/src/test/kotlin/org/libremail/mail/SmtpSenderTest.kt b/app/src/test/kotlin/org/libremail/mail/SmtpSenderTest.kt index 424fdf4..b3fe3d1 100644 --- a/app/src/test/kotlin/org/libremail/mail/SmtpSenderTest.kt +++ b/app/src/test/kotlin/org/libremail/mail/SmtpSenderTest.kt @@ -117,7 +117,7 @@ class SmtpSenderTest { body = "Body", bodyHtml = "

Body

", ), - attachments = listOf(file), + attachments = listOf(SendableAttachment(file)), ) greenMail.waitForIncomingEmail(1) @@ -130,6 +130,42 @@ class SmtpSenderTest { file.delete() } + @Test + fun `an inline image is sent as multipart related with a matching Content-ID`() = runTest { + val image = File.createTempFile("libremail-inline", ".png").apply { writeText("PNGDATA") } + val params = SmtpParams( + host = "127.0.0.1", + port = greenMail.smtp.port, + security = MailSecurity.NONE, + username = "sender@example.org", + secret = "secret", + useXoauth2 = false, + ) + + sender.send( + params = params, + from = "sender@example.org", + message = OutgoingMessage( + accountId = "x", + to = "bob@example.org", + subject = "Inline", + body = "See image", + bodyHtml = "

See \"logo\"

", + ), + attachments = listOf(SendableAttachment(image, contentId = "logo@libremail", isInline = true)), + ) + + greenMail.waitForIncomingEmail(1) + val received = greenMail.receivedMessages.single() + assertTrue(received.contentType.contains("multipart/related", ignoreCase = true), received.contentType) + val raw = GreenMailUtil.getWholeMessage(received) + // The HTML's cid reference and the image part's Content-ID must name the same content id. + assertTrue(raw.contains("cid:logo@libremail"), "html must reference the cid") + assertTrue(raw.contains(""), "inline part must carry a matching Content-ID") + assertTrue(raw.contains("multipart/alternative", ignoreCase = true), "the body stays an alternative") + image.delete() + } + @Test fun `send delivers a message with an attachment`() = runTest { val file = File.createTempFile("libremail-report", ".txt").apply { writeText("quarterly numbers") } @@ -151,7 +187,7 @@ class SmtpSenderTest { subject = "With file", body = "See the attached report.", ), - attachments = listOf(file), + attachments = listOf(SendableAttachment(file)), ) greenMail.waitForIncomingEmail(1) diff --git a/app/src/test/kotlin/org/libremail/richtext/RichTextEditingTest.kt b/app/src/test/kotlin/org/libremail/richtext/RichTextEditingTest.kt index 128ac26..8d4d238 100644 --- a/app/src/test/kotlin/org/libremail/richtext/RichTextEditingTest.kt +++ b/app/src/test/kotlin/org/libremail/richtext/RichTextEditingTest.kt @@ -232,4 +232,27 @@ class RichTextEditingTest { fun `alignmentAt treats a plain paragraph as START`() { assertEquals(RichAlign.START, RichTextEditing.alignmentAt(RichTextContent("plain"), 0, 5)) } + + // --- insertImage --- + + @Test + fun `insertImage drops a token and RichImage at the caret and shifts later spans`() { + val base = RichTextContent("ab", spans = listOf(RichSpan(1, 2, RichStyle.Bold))) + val result = RichTextEditing.insertImage(base, 1, "cid1@x", "pic") + val token = imageToken("pic") + assertEquals("a${token}b", result.content.text) + assertEquals(listOf(RichImage(1, 1 + token.length, "cid1@x", "pic")), result.content.images) + // The bold run that began at the caret shifts entirely past the inserted token. + assertEquals(listOf(RichSpan(1 + token.length, 2 + token.length, RichStyle.Bold)), result.content.spans) + assertEquals(1 + token.length, result.selectionStart) + assertEquals(1 + token.length, result.selectionEnd) + } + + @Test + fun `insertImage output round-trips its image through html`() { + val result = RichTextEditing.insertImage(RichTextContent("hi"), 2, "c@x", "cat.png") + val restored = RichTextHtml.fromHtml(RichTextHtml.toHtml(result.content)) + assertEquals(result.content.text, restored.text) + assertEquals(result.content.images, restored.images) + } }