From 282a64b4c3d7af443f3534ecfeae6ff73c0acf59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bot=20of=20Thomas=20G=C3=BCttler?= Date: Sun, 7 Jun 2026 05:30:59 +0200 Subject: [PATCH] fix: include mailboxPath in IMAP email ID to prevent UID collisions (#511) --- lib/core/db_schema_version.dart | 2 +- lib/data/db/database.dart | 110 +++++++++++ .../repositories/email_repository_impl.dart | 4 +- test/unit/email_repository_impl_test.dart | 44 +++++ test/unit/migration_test.dart | 181 +++++++++++++++++- 5 files changed, 336 insertions(+), 5 deletions(-) diff --git a/lib/core/db_schema_version.dart b/lib/core/db_schema_version.dart index dd07635..a3cac20 100644 --- a/lib/core/db_schema_version.dart +++ b/lib/core/db_schema_version.dart @@ -1 +1 @@ -const int dbSchemaVersion = 40; +const int dbSchemaVersion = 41; diff --git a/lib/data/db/database.dart b/lib/data/db/database.dart index 93d3939..bded832 100644 --- a/lib/data/db/database.dart +++ b/lib/data/db/database.dart @@ -679,6 +679,116 @@ class AppDatabase extends _$AppDatabase { if (from < 40) { await m.createTable(installedVersions); } + if (from < 41) { + // Fix IMAP email IDs to include mailboxPath, preventing UID + // collisions across mailboxes (IMAP UIDs are mailbox-scoped). + // New format: "accountId:mailboxPath:uid" (was "accountId:uid"). + // + // defer_foreign_keys defers the email_bodies→emails FK check + // to COMMIT so the two tables can be updated sequentially inside + // the migration transaction without a transient FK violation. + await customStatement('PRAGMA defer_foreign_keys = ON'); + + // 1. Remap email_bodies.email_id before emails.id changes. + await customStatement(''' + UPDATE email_bodies + SET email_id = ( + SELECT e.account_id || ':' || e.mailbox_path || ':' || CAST(e.uid AS TEXT) + FROM emails e + JOIN accounts a ON a.id = e.account_id + WHERE e.id = email_bodies.email_id + AND a.account_type = 'imap' + ) + WHERE EXISTS ( + SELECT 1 FROM emails e + JOIN accounts a ON a.id = e.account_id + WHERE e.id = email_bodies.email_id + AND a.account_type = 'imap' + ) + '''); + + // 2. Update emails.thread_id where it was set to the email's own + // id (fallback for messages with no Message-ID header). + await customStatement(''' + UPDATE emails + SET thread_id = account_id || ':' || mailbox_path || ':' || CAST(uid AS TEXT) + WHERE account_id IN (SELECT id FROM accounts WHERE account_type = 'imap') + AND thread_id = id + '''); + + // 3. Update the primary key on emails. + await customStatement(''' + UPDATE emails + SET id = account_id || ':' || mailbox_path || ':' || CAST(uid AS TEXT) + WHERE account_id IN ( + SELECT id FROM accounts WHERE account_type = 'imap' + ) + '''); + + // 5. Rebuild threads for IMAP accounts from the updated email rows. + // The threads table stores denormalised data (latest_email_id, + // email_ids_json) that references email IDs, so it is simpler to + // delete and reconstruct than to patch the JSON in SQL. + await customStatement(''' + DELETE FROM threads + WHERE account_id IN (SELECT id FROM accounts WHERE account_type = 'imap') + '''); + + final imapAccounts = await (select(accounts) + ..where((t) => t.accountType.equals('imap'))) + .get(); + for (final acct in imapAccounts) { + final emailRows = await (select(emails) + ..where((t) => t.accountId.equals(acct.id))) + .get(); + + final groups = >{}; + for (final row in emailRows) { + final key = '${row.mailboxPath}:${row.threadId ?? row.id}'; + groups.putIfAbsent(key, () => []).add(row); + } + + for (final threadEmails in groups.values) { + threadEmails.sort((a, b) { + final da = a.sentAt ?? a.receivedAt; + final db = b.sentAt ?? b.receivedAt; + return da.compareTo(db); + }); + final latest = threadEmails.last; + + final seen = {}; + final participants = >[]; + for (final e in threadEmails) { + final from = jsonDecode(e.fromJson) as List; + for (final a in from.cast>()) { + final email = a['email'] as String; + if (seen.add(email)) { + participants.add({'name': a['name'], 'email': email}); + } + } + } + + await into(threads).insert( + ThreadsCompanion.insert( + id: latest.threadId ?? latest.id, + accountId: latest.accountId, + mailboxPath: latest.mailboxPath, + subject: Value(latest.subject), + latestDate: latest.sentAt ?? latest.receivedAt, + messageCount: Value(threadEmails.length), + hasUnread: Value(threadEmails.any((e) => !e.isSeen)), + isFlagged: Value(threadEmails.any((e) => e.isFlagged)), + participantsJson: Value(jsonEncode(participants)), + preview: Value(latest.preview), + latestEmailId: latest.id, + emailIdsJson: Value( + jsonEncode(threadEmails.map((e) => e.id).toList()), + ), + ), + ); + } + } + } }, ); diff --git a/lib/data/repositories/email_repository_impl.dart b/lib/data/repositories/email_repository_impl.dart index e2ad173..c6ebbc6 100644 --- a/lib/data/repositories/email_repository_impl.dart +++ b/lib/data/repositories/email_repository_impl.dart @@ -561,7 +561,7 @@ class EmailRepositoryImpl implements EmailRepository { for (final msg in result.messages) { final uid = msg.uid; if (uid == null) continue; - final emailId = '${account.id}:$uid'; + final emailId = '${account.id}:$mailboxPath:$uid'; await (_db.update(_db.emails)..where((t) => t.id.equals(emailId))).write( EmailsCompanion( isSeen: Value(msg.flags?.contains(r'\Seen') ?? false), @@ -616,7 +616,7 @@ class EmailRepositoryImpl implements EmailRepository { continue; } bytes += msg.size ?? 0; - final emailId = '${account.id}:$uid'; + final emailId = '${account.id}:$mailboxPath:$uid'; final msgId = envelope.messageId?.trim(); final inReplyTo = envelope.inReplyTo?.trim(); final refs = msg.getHeaderValue('References')?.trim(); diff --git a/test/unit/email_repository_impl_test.dart b/test/unit/email_repository_impl_test.dart index 710990e..3f8abdf 100644 --- a/test/unit/email_repository_impl_test.dart +++ b/test/unit/email_repository_impl_test.dart @@ -262,6 +262,50 @@ void main() { expect(emails.map((e) => e.uid).toList(), [3, 2, 1]); }); + test('same UID in different mailboxes yields independent emails', () async { + // Regression test for the UID collision bug: IMAP UIDs are mailbox-scoped, + // so UID 50 in INBOX and UID 50 in Archive must get distinct local IDs. + final r = _makeRepos(); + await r.accounts.addAccount(_account, 'pw'); + + // New ID format: accountId:mailboxPath:uid + await r.db.into(r.db.emails).insert( + EmailsCompanion.insert( + id: 'acc-1:INBOX:50', + accountId: 'acc-1', + mailboxPath: 'INBOX', + uid: 50, + receivedAt: DateTime(2024), + ), + ); + await r.db.into(r.db.emails).insert( + EmailsCompanion.insert( + id: 'acc-1:Archive:50', + accountId: 'acc-1', + mailboxPath: 'Archive', + uid: 50, + receivedAt: DateTime(2024, 1, 2), + ), + ); + + final inboxEmail = await r.emails.getEmail('acc-1:INBOX:50'); + expect(inboxEmail, isNotNull); + expect(inboxEmail!.mailboxPath, 'INBOX'); + + final archiveEmail = await r.emails.getEmail('acc-1:Archive:50'); + expect(archiveEmail, isNotNull); + expect(archiveEmail!.mailboxPath, 'Archive'); + + final inboxEmails = await r.emails.observeEmails('acc-1', 'INBOX').first; + expect(inboxEmails, hasLength(1)); + expect(inboxEmails.first.id, 'acc-1:INBOX:50'); + + final archiveEmails = + await r.emails.observeEmails('acc-1', 'Archive').first; + expect(archiveEmails, hasLength(1)); + expect(archiveEmails.first.id, 'acc-1:Archive:50'); + }); + test('syncEmails propagates IMAP error', () async { final r = _makeRepos(); await r.accounts.addAccount(_account, 'pw'); diff --git a/test/unit/migration_test.dart b/test/unit/migration_test.dart index c52cfd6..30e1eb5 100644 --- a/test/unit/migration_test.dart +++ b/test/unit/migration_test.dart @@ -14,7 +14,7 @@ void main() { group('Migration', () { test('schemaVersion matches expected value', () async { final db = AppDatabase(NativeDatabase.memory()); - expect(db.schemaVersion, 40); + expect(db.schemaVersion, 41); await db.close(); }); @@ -435,7 +435,184 @@ void main() { }, ); - test('fresh install creates all tables at schemaVersion 40', () async { + test('v40→v41: IMAP email IDs gain mailboxPath segment', () async { + final dbFile = File('test_migration_v40.db'); + if (dbFile.existsSync()) dbFile.deleteSync(); + + final rawDb = sqlite.sqlite3.open(dbFile.path); + rawDb.execute(''' + CREATE TABLE accounts ( + id TEXT NOT NULL PRIMARY KEY, + display_name TEXT NOT NULL, + email TEXT NOT NULL, + imap_host TEXT NOT NULL DEFAULT '', + imap_port INTEGER NOT NULL DEFAULT 993, + imap_ssl INTEGER NOT NULL DEFAULT 1, + smtp_host TEXT NOT NULL DEFAULT '', + smtp_port INTEGER NOT NULL DEFAULT 465, + smtp_ssl INTEGER NOT NULL DEFAULT 1, + account_type TEXT NOT NULL DEFAULT 'imap', + jmap_url TEXT NULL, + username TEXT NOT NULL DEFAULT '', + verbose INTEGER NOT NULL DEFAULT 0, + manage_sieve_host TEXT NOT NULL DEFAULT '', + manage_sieve_port INTEGER NOT NULL DEFAULT 4190, + manage_sieve_ssl INTEGER NOT NULL DEFAULT 1, + manage_sieve_available INTEGER NULL + ) + '''); + rawDb.execute(''' + CREATE TABLE emails ( + id TEXT NOT NULL PRIMARY KEY, + account_id TEXT NOT NULL REFERENCES accounts (id) ON DELETE CASCADE, + mailbox_path TEXT NOT NULL, + uid INTEGER NOT NULL, + subject TEXT NULL, + sent_at INTEGER NULL, + received_at INTEGER NOT NULL, + from_json TEXT NOT NULL DEFAULT '[]', + to_addresses TEXT NOT NULL DEFAULT '[]', + cc_json TEXT NOT NULL DEFAULT '[]', + preview TEXT NULL, + is_seen INTEGER NOT NULL DEFAULT 0, + is_flagged INTEGER NOT NULL DEFAULT 0, + has_attachment INTEGER NOT NULL DEFAULT 0, + thread_id TEXT NULL, + message_id TEXT NULL, + in_reply_to TEXT NULL, + "references" TEXT NULL, + snoozed_until INTEGER NULL, + snoozed_from_mailbox_path TEXT NULL, + list_unsubscribe_header TEXT NULL + ) + '''); + rawDb.execute(''' + CREATE TABLE email_bodies ( + email_id TEXT NOT NULL PRIMARY KEY REFERENCES emails (id) ON DELETE CASCADE, + text_body TEXT NULL, + html_body TEXT NULL, + attachments_json TEXT NOT NULL DEFAULT '[]', + cached_at INTEGER NULL, + headers_json TEXT NULL, + mime_tree_json TEXT NULL + ) + '''); + rawDb.execute(''' + CREATE TABLE threads ( + account_id TEXT NOT NULL REFERENCES accounts (id) ON DELETE CASCADE, + mailbox_path TEXT NOT NULL, + id TEXT NOT NULL, + subject TEXT NULL, + latest_date INTEGER NOT NULL, + message_count INTEGER NOT NULL DEFAULT 1, + has_unread INTEGER NOT NULL DEFAULT 0, + is_flagged INTEGER NOT NULL DEFAULT 0, + participants_json TEXT NOT NULL DEFAULT '[]', + preview TEXT NULL, + latest_email_id TEXT NOT NULL, + email_ids_json TEXT NOT NULL DEFAULT '[]', + PRIMARY KEY (account_id, mailbox_path, id) + ) + '''); + rawDb.execute(''' + CREATE TABLE pending_changes ( + id INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT, + account_id TEXT NOT NULL REFERENCES accounts (id) ON DELETE CASCADE, + resource_type TEXT NOT NULL, + resource_id TEXT NOT NULL, + change_type TEXT NOT NULL, + payload TEXT NOT NULL, + created_at INTEGER NOT NULL, + attempts INTEGER NOT NULL DEFAULT 0, + last_error TEXT NULL + ) + '''); + + // Insert an IMAP account. + rawDb.execute( + "INSERT INTO accounts (id, display_name, email) VALUES ('acc-1', 'Alice', 'alice@example.com')", + ); + + // Two emails with the same UID but in different mailboxes — old format. + final now = DateTime.now().millisecondsSinceEpoch ~/ 1000; + rawDb.execute( + 'INSERT INTO emails (id, account_id, mailbox_path, uid, received_at, thread_id) ' + "VALUES ('acc-1:50', 'acc-1', 'INBOX', 50, $now, 'acc-1:50')", + ); + rawDb.execute( + 'INSERT INTO emails (id, account_id, mailbox_path, uid, received_at) ' + "VALUES ('acc-1:50-arch', 'acc-1', 'Archive', 50, $now)", + ); + // A third email with a Message-ID-based thread_id (should not be changed). + rawDb.execute( + 'INSERT INTO emails (id, account_id, mailbox_path, uid, received_at, thread_id) ' + "VALUES ('acc-1:99', 'acc-1', 'INBOX', 99, $now, '')", + ); + + // Email body for the first email. + rawDb.execute( + "INSERT INTO email_bodies (email_id, text_body) VALUES ('acc-1:50', 'body text')", + ); + + // Thread for the first email (old-format IDs). + rawDb.execute( + 'INSERT INTO threads (account_id, mailbox_path, id, latest_date, latest_email_id, email_ids_json) ' + "VALUES ('acc-1', 'INBOX', 'acc-1:50', $now, 'acc-1:50', '[\"acc-1:50\"]')", + ); + + // A pending change referencing the first email's old ID. + rawDb.execute( + 'INSERT INTO pending_changes (account_id, resource_type, resource_id, change_type, payload, created_at) ' + "VALUES ('acc-1', 'Email', 'acc-1:50', 'flag_seen', '{\"seen\":true}', $now)", + ); + + rawDb.execute('PRAGMA user_version = 40'); + rawDb.close(); + + // Open with Drift to trigger the migration. + final db = AppDatabase(NativeDatabase(dbFile)); + await db.select(db.accounts).get(); + + // emails.id should now use the accountId:mailboxPath:uid format. + final emailRows = await db.select(db.emails).get(); + final emailIds = emailRows.map((r) => r.id).toSet(); + expect(emailIds, contains('acc-1:INBOX:50')); + expect(emailIds, contains('acc-1:Archive:50')); + expect(emailIds, contains('acc-1:INBOX:99')); + // Old-format IDs must be gone. + expect(emailIds, isNot(contains('acc-1:50'))); + expect(emailIds, isNot(contains('acc-1:99'))); + + // email_bodies.email_id must be updated. + final bodyRows = await db.select(db.emailBodies).get(); + expect(bodyRows, hasLength(1)); + expect(bodyRows.first.emailId, 'acc-1:INBOX:50'); + + // thread_id where it was the email's own ID should be updated. + final inboxEmail = emailRows.firstWhere((r) => r.id == 'acc-1:INBOX:50'); + expect(inboxEmail.threadId, 'acc-1:INBOX:50'); + + // thread_id based on a real Message-ID must be left unchanged. + final inboxEmail99 = + emailRows.firstWhere((r) => r.id == 'acc-1:INBOX:99'); + expect(inboxEmail99.threadId, ''); + + // threads must be rebuilt with new-format IDs. + final threadRows = await db.select(db.threads).get(); + final thread = threadRows.firstWhere((t) => t.mailboxPath == 'INBOX'); + expect(thread.latestEmailId, 'acc-1:INBOX:50'); + expect(thread.emailIdsJson, contains('acc-1:INBOX:50')); + + // pending_changes.resource_id is not updated by the migration + // (IMAP operations use payload uid/mailboxPath, so this is safe). + final changeRows = await db.select(db.pendingChanges).get(); + expect(changeRows, hasLength(1)); + + await db.close(); + if (dbFile.existsSync()) dbFile.deleteSync(); + }); + + test('fresh install creates all tables at schemaVersion 41', () async { final db = AppDatabase(NativeDatabase.memory()); await db.select(db.accounts).get();