diff --git a/DB-SYNC.md b/DB-SYNC.md index c8a76e7..ac89933 100644 --- a/DB-SYNC.md +++ b/DB-SYNC.md @@ -54,6 +54,13 @@ This document covers the mail-to-database sync layer only, not the UI. optimistic local update; `flushPendingChanges` drains the queue over a single IMAP connection at the start of each sync cycle. - Sent messages are appended to the Sent folder after SMTP delivery. +- IMAP move remap: after a `MOVE` is flushed, the local row id is rewritten in + place using the RFC 4315 `COPYUID` response code (UIDPLUS); if the server + doesn't support UIDPLUS, the new UID is looked up via `UID SEARCH HEADER + Message-ID …` in the destination mailbox. Cached bodies (`email_bodies`), + threads, queued pending changes, and undo entries follow the new id. + Deletion reconciliation skips rows whose `move`/`snooze`/`unsnooze` is still + in `pending_changes` so the optimistic local move isn't wiped mid-flight. - Sync retries use exponential backoff after failures. ### Cross-protocol diff --git a/SYNC.md b/SYNC.md index 3389202..7e81ad2 100644 --- a/SYNC.md +++ b/SYNC.md @@ -81,6 +81,17 @@ start() On each run, only UIDs greater than `lastUid` are fetched. If `uidValidity` changes the full folder is re-scanned and the checkpoint is reset. +**IMAP move remap** — IMAP UIDs are mailbox-scoped, so a moved message gets a new UID in +its destination folder. When a `move`/`snooze`/`unsnooze` change is flushed, the local row +id (`accountId:mailboxPath:uid`) is rewritten in place to point at the new UID. The new +UID is taken from the RFC 4315 `COPYUID` response code returned by `MOVE`; if the server +does not advertise `UIDPLUS`, a `UID SEARCH HEADER Message-ID …` in the destination +mailbox is used as a fallback. `email_bodies`, `threads`, `pending_changes`, and +`undo_actions` rows that reference the old id are updated atomically so cached bodies and +pending undo operations keep tracking the same physical message. Deletion reconciliation +also skips rows whose move is still queued, so the optimistic local move never gets +wiped mid-flight. + **IDLE cap** — IDLE sessions are limited to 25 minutes per the RFC. The loop also wakes immediately if `syncNow()` is called (e.g. user pulls-to-refresh). diff --git a/lib/data/repositories/email_repository_impl.dart b/lib/data/repositories/email_repository_impl.dart index c6ebbc6..4dded15 100644 --- a/lib/data/repositories/email_repository_impl.dart +++ b/lib/data/repositories/email_repository_impl.dart @@ -6,6 +6,7 @@ import 'dart:math' as math; import 'package:drift/drift.dart'; import 'package:enough_mail/enough_mail.dart' as imap; import 'package:http/http.dart' as http; +import 'package:meta/meta.dart'; import 'package:path/path.dart' as p; import 'package:path_provider/path_provider.dart'; @@ -728,6 +729,14 @@ class EmailRepositoryImpl implements EmailRepository { await _saveSyncState(accountId, resourceType, jsonEncode(data)); } + @visibleForTesting + Future reconcileDeletedImapForTest( + String accountId, + String mailboxPath, + List serverUids, + ) => + _reconcileDeletedImap(accountId, mailboxPath, serverUids); + Future _reconcileDeletedImap( String accountId, String mailboxPath, @@ -752,10 +761,28 @@ class EmailRepositoryImpl implements EmailRepository { return; } + // Email IDs that still have a queued move/snooze/unsnooze waiting to be + // flushed. The optimistic local move has already updated mailbox_path, so + // these rows look orphaned from both the old and new mailbox until the + // server applies the change and we remap to the destination UID. Skipping + // them here avoids wiping the row mid-flight. + final inFlightIds = await (_db.selectOnly(_db.pendingChanges) + ..addColumns([_db.pendingChanges.resourceId]) + ..where( + _db.pendingChanges.accountId.equals(accountId) & + _db.pendingChanges.changeType.isIn( + const ['move', 'snooze', 'unsnooze'], + ), + )) + .map((row) => row.read(_db.pendingChanges.resourceId)!) + .get(); + final inFlightSet = inFlightIds.toSet(); + final serverUidSet = serverUids.toSet(); final affectedThreads = {}; for (final row in localRows) { if (!serverUidSet.contains(row.uid)) { + if (inFlightSet.contains(row.id)) continue; affectedThreads.add(row.threadId ?? row.id); await (_db.delete(_db.emails)..where((t) => t.id.equals(row.id))).go(); } @@ -2317,7 +2344,15 @@ class EmailRepositoryImpl implements EmailRepository { ? await client.uidMarkFlagged(seq) : await client.uidMarkUnflagged(seq); case 'move': - await client.uidMove(seq, targetMailboxPath: payload['dest'] as String); + final dest = payload['dest'] as String; + final result = await client.uidMove(seq, targetMailboxPath: dest); + await _remapEmailAfterImapMove( + client, + oldId: row.resourceId, + sourceUid: uid, + destMailboxPath: dest, + moveResult: result, + ); case 'delete': await client.uidMarkDeleted(seq); await client.uidExpunge(seq); @@ -2332,7 +2367,14 @@ class EmailRepositoryImpl implements EmailRepository { await client.createMailbox(dest); } catch (_) {} await client.uidStore(seq, [keyword], action: imap.StoreAction.add); - await client.uidMove(seq, targetMailboxPath: dest); + final snoozeResult = await client.uidMove(seq, targetMailboxPath: dest); + await _remapEmailAfterImapMove( + client, + oldId: row.resourceId, + sourceUid: uid, + destMailboxPath: dest, + moveResult: snoozeResult, + ); case 'unsnooze': final dest = payload['dest'] as String; try { @@ -2351,7 +2393,151 @@ class EmailRepositoryImpl implements EmailRepository { ); } } - await client.uidMove(seq, targetMailboxPath: dest); + final unsnoozeResult = + await client.uidMove(seq, targetMailboxPath: dest); + await _remapEmailAfterImapMove( + client, + oldId: row.resourceId, + sourceUid: uid, + destMailboxPath: dest, + moveResult: unsnoozeResult, + ); + } + } + + /// Rewrites the local row identity after an IMAP MOVE so the cache keeps + /// tracking the same physical message under its new (mailbox, UID). + /// + /// The new UID is taken from the RFC 4315 `COPYUID` response code first + /// (every modern server advertises `UIDPLUS`). If that's missing we fall + /// back to `UID SEARCH HEADER Message-ID …` in the destination mailbox. + /// When neither yields a UID we leave the row in place; the next sync + /// cycle will re-fetch it as a new message and reconciliation will drop + /// the stale source-side row. + Future _remapEmailAfterImapMove( + imap.ImapClient client, { + required String oldId, + required int sourceUid, + required String destMailboxPath, + required imap.GenericImapResult moveResult, + }) async { + final row = await (_db.select(_db.emails)..where((t) => t.id.equals(oldId))) + .getSingleOrNull(); + if (row == null) return; + + final newUid = _resolveCopyUid(moveResult, sourceUid) ?? + await _searchUidByMessageId( + client, + destMailboxPath, + row.messageId, + ); + if (newUid == null) { + log( + '_remapEmailAfterImapMove: could not resolve new UID for $oldId ' + 'after move to $destMailboxPath (no COPYUID, ' + 'messageId=${row.messageId}); row will be re-fetched on next sync', + ); + return; + } + + final newId = '${row.accountId}:$destMailboxPath:$newUid'; + if (newId == oldId) return; + + await _db.transaction(() async { + await _db.customStatement('PRAGMA defer_foreign_keys = ON'); + + await _db.customStatement( + 'UPDATE email_bodies SET email_id = ?1 WHERE email_id = ?2', + [newId, oldId], + ); + + await (_db.update(_db.emails)..where((t) => t.id.equals(oldId))).write( + EmailsCompanion( + id: Value(newId), + uid: Value(newUid), + mailboxPath: Value(destMailboxPath), + ), + ); + + await (_db.update(_db.pendingChanges) + ..where((t) => t.resourceId.equals(oldId))) + .write(PendingChangesCompanion(resourceId: Value(newId))); + + // threads.latest_email_id is a plain equality match; threads.email_ids_json + // is a JSON array of email IDs — both are safe to update via REPLACE() + // because email IDs are unique opaque strings. + await _db.customStatement( + 'UPDATE threads SET latest_email_id = ?1 ' + 'WHERE latest_email_id = ?2', + [newId, oldId], + ); + await _db.customStatement( + 'UPDATE threads SET email_ids_json = ' + 'REPLACE(email_ids_json, ?1, ?2) ' + 'WHERE email_ids_json LIKE ?3', + ['"$oldId"', '"$newId"', '%"$oldId"%'], + ); + + // UndoAction.toJson() embeds email IDs as quoted JSON strings in both + // emailIds and originalEmails[].id, so the same REPLACE() works. + await _db.customStatement( + 'UPDATE undo_actions SET data_json = ' + 'REPLACE(data_json, ?1, ?2) ' + 'WHERE data_json LIKE ?3', + ['"$oldId"', '"$newId"', '%"$oldId"%'], + ); + }); + + // Rebuild thread aggregates in both mailboxes from the now-updated emails. + final threadId = row.threadId ?? newId; + await _updateThread(row.accountId, row.mailboxPath, threadId); + await _updateThread(row.accountId, destMailboxPath, threadId); + } + + /// Extracts the destination UID for [sourceUid] from a MOVE/COPY result's + /// `COPYUID` response code (RFC 4315). Returns null when the server did not + /// advertise UIDPLUS or the response code is malformed. + int? _resolveCopyUid(imap.GenericImapResult result, int sourceUid) { + final code = result.responseCodeCopyUid; + if (code == null) return null; + try { + final sources = code.originalSequence?.toList(); + final targets = code.targetSequence.toList(); + if (sources == null) { + // Some servers omit the source set when only one message moved. + return targets.length == 1 ? targets.first : null; + } + final idx = sources.indexOf(sourceUid); + if (idx < 0 || idx >= targets.length) return null; + return targets[idx]; + } catch (_) { + return null; + } + } + + /// Looks up the UID of a message in [mailboxPath] by its RFC 2822 + /// `Message-ID` header. Used as a fallback when the server doesn't + /// support UIDPLUS so we can still relink the local row after a move. + Future _searchUidByMessageId( + imap.ImapClient client, + String mailboxPath, + String? messageId, + ) async { + if (messageId == null || messageId.isEmpty) return null; + try { + await client.selectMailboxByPath(mailboxPath); + // RFC 3501 SEARCH HEADER uses an astring for the value; quoting is safe + // for typical Message-ID syntax (no embedded quotes or backslashes). + final escaped = messageId.replaceAll(r'\', r'\\').replaceAll('"', r'\"'); + final result = await client.uidSearchMessages( + searchCriteria: 'HEADER Message-ID "$escaped"', + ); + final uids = result.matchingSequence?.toList() ?? const []; + if (uids.isEmpty) return null; + return uids.reduce((a, b) => a > b ? a : b); + } catch (e) { + log('_searchUidByMessageId failed for $messageId in $mailboxPath: $e'); + return null; } } diff --git a/pubspec.lock b/pubspec.lock index 2c91fa6..95b3562 100644 --- a/pubspec.lock +++ b/pubspec.lock @@ -680,7 +680,7 @@ packages: source: hosted version: "0.13.0" meta: - dependency: transitive + dependency: "direct main" description: name: meta sha256: "1741988757a65eb6b36abe716829688cf01910bbf91c34354ff7ec1c3de2b349" diff --git a/pubspec.yaml b/pubspec.yaml index 9ae4995..b4907e9 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -67,6 +67,9 @@ dependencies: share_plus: ^13.1.0 device_info_plus: ^13.1.0 + # @visibleForTesting annotation used in lib/data/repositories. + meta: ^1.16.0 + dev_dependencies: flutter_test: sdk: flutter diff --git a/test/unit/email_repository_impl_test.dart b/test/unit/email_repository_impl_test.dart index 055b0fe..4414cd5 100644 --- a/test/unit/email_repository_impl_test.dart +++ b/test/unit/email_repository_impl_test.dart @@ -1109,6 +1109,242 @@ void main() { expect(spy.movedToMailbox, 'Snoozed'); }, ); + + test( + 'move flush remaps local id/uid from COPYUID and rewrites cached bodies', + () async { + final spy = SnoozeSpyImapClient( + copyUidValidity: 1, + copyUidSourceToTarget: const {5: 42}, + ); + final r = _makeRepos(imapConnect: (_, __, ___) async => spy); + await r.accounts.addAccount(_account, 'pw'); + + const oldId = 'acc-1:INBOX:5'; + await r.db.into(r.db.emails).insert( + EmailsCompanion.insert( + id: oldId, + accountId: 'acc-1', + mailboxPath: 'Archive', // already optimistically moved + uid: 5, + receivedAt: DateTime(2024), + messageId: const Value(''), + threadId: const Value('thr-1'), + ), + ); + await r.db.into(r.db.emailBodies).insert( + EmailBodiesCompanion.insert( + emailId: oldId, + textBody: const Value('cached body'), + ), + ); + await r.db.into(r.db.pendingChanges).insert( + PendingChangesCompanion.insert( + accountId: 'acc-1', + resourceType: 'Email', + resourceId: oldId, + changeType: 'move', + payload: jsonEncode({ + 'uid': 5, + 'mailboxPath': 'INBOX', + 'dest': 'Archive', + }), + createdAt: DateTime.now(), + ), + ); + + await r.emails.flushPendingChanges('acc-1', 'pw'); + + // Pending change drained. + expect(await r.db.select(r.db.pendingChanges).get(), isEmpty); + + // Old id is gone; new id reflects destination mailbox + new UID. + expect(await r.emails.getEmail(oldId), isNull); + const newId = 'acc-1:Archive:42'; + final moved = await r.emails.getEmail(newId); + expect(moved, isNotNull); + expect(moved!.uid, 42); + expect(moved.mailboxPath, 'Archive'); + + // Body cache follows the new id. + final bodies = await r.db.select(r.db.emailBodies).get(); + expect(bodies, hasLength(1)); + expect(bodies.first.emailId, newId); + expect(bodies.first.textBody, 'cached body'); + }, + ); + + test( + 'move flush falls back to UID SEARCH HEADER Message-ID without UIDPLUS', + () async { + const messageId = ''; + const criteria = 'HEADER Message-ID "$messageId"'; + final spy = SnoozeSpyImapClient( + // No copyUidValidity → no COPYUID in the MOVE response. + searchResults: const { + criteria: [99], + }, + ); + final r = _makeRepos(imapConnect: (_, __, ___) async => spy); + await r.accounts.addAccount(_account, 'pw'); + + const oldId = 'acc-1:INBOX:5'; + await r.db.into(r.db.emails).insert( + EmailsCompanion.insert( + id: oldId, + accountId: 'acc-1', + mailboxPath: 'Archive', + uid: 5, + receivedAt: DateTime(2024), + messageId: const Value(messageId), + ), + ); + await r.db.into(r.db.pendingChanges).insert( + PendingChangesCompanion.insert( + accountId: 'acc-1', + resourceType: 'Email', + resourceId: oldId, + changeType: 'move', + payload: jsonEncode({ + 'uid': 5, + 'mailboxPath': 'INBOX', + 'dest': 'Archive', + }), + createdAt: DateTime.now(), + ), + ); + + await r.emails.flushPendingChanges('acc-1', 'pw'); + + expect(spy.lastSearchCriteria, criteria); + const newId = 'acc-1:Archive:99'; + final moved = await r.emails.getEmail(newId); + expect(moved, isNotNull); + expect(moved!.uid, 99); + }, + ); + + test( + 'move flush rewrites pending undo_actions referencing the old id', + () async { + final spy = SnoozeSpyImapClient( + copyUidValidity: 1, + copyUidSourceToTarget: const {5: 42}, + ); + final r = _makeRepos(imapConnect: (_, __, ___) async => spy); + await r.accounts.addAccount(_account, 'pw'); + + const oldId = 'acc-1:INBOX:5'; + await r.db.into(r.db.emails).insert( + EmailsCompanion.insert( + id: oldId, + accountId: 'acc-1', + mailboxPath: 'Archive', + uid: 5, + receivedAt: DateTime(2024), + ), + ); + await r.db.into(r.db.pendingChanges).insert( + PendingChangesCompanion.insert( + accountId: 'acc-1', + resourceType: 'Email', + resourceId: oldId, + changeType: 'move', + payload: jsonEncode({ + 'uid': 5, + 'mailboxPath': 'INBOX', + 'dest': 'Archive', + }), + createdAt: DateTime.now(), + ), + ); + // An undo entry created when the user did the move, referencing oldId + // in both emailIds and originalEmails[].id. + await r.db.into(r.db.undoActions).insert( + UndoActionsCompanion.insert( + id: 'undo-1', + accountId: 'acc-1', + dataJson: jsonEncode({ + 'id': 'undo-1', + 'accountId': 'acc-1', + 'type': 'move', + 'emailIds': [oldId], + 'sourceMailboxPath': 'INBOX', + 'destinationMailboxPath': 'Archive', + 'timestamp': DateTime(2024).toIso8601String(), + 'originalEmails': [ + { + 'id': oldId, + 'accountId': 'acc-1', + 'mailboxPath': 'INBOX', + 'uid': 5, + 'receivedAt': DateTime(2024).toIso8601String(), + 'from': [], + 'to': [], + 'cc': [], + 'isSeen': false, + 'isFlagged': false, + 'hasAttachment': false, + }, + ], + }), + createdAt: DateTime(2024), + ), + ); + + await r.emails.flushPendingChanges('acc-1', 'pw'); + + const newId = 'acc-1:Archive:42'; + final stored = await r.db.select(r.db.undoActions).getSingle(); + final json = jsonDecode(stored.dataJson) as Map; + expect(json['emailIds'], [newId]); + expect( + (json['originalEmails'] as List).first as Map, + containsPair('id', newId), + ); + }, + ); + + test( + 'reconciliation skips rows with a pending move so they are not wiped', + () async { + final r = _makeRepos(); + await r.accounts.addAccount(_account, 'pw'); + + const oldId = 'acc-1:INBOX:5'; + await r.db.into(r.db.emails).insert( + EmailsCompanion.insert( + id: oldId, + accountId: 'acc-1', + mailboxPath: 'Archive', // optimistically moved + uid: 5, + receivedAt: DateTime(2024), + ), + ); + await r.db.into(r.db.pendingChanges).insert( + PendingChangesCompanion.insert( + accountId: 'acc-1', + resourceType: 'Email', + resourceId: oldId, + changeType: 'move', + payload: jsonEncode({ + 'uid': 5, + 'mailboxPath': 'INBOX', + 'dest': 'Archive', + }), + createdAt: DateTime.now(), + ), + ); + + // Run the deletion-reconciliation pass with a destination snapshot + // that does NOT contain UID 5 — the row would be wiped without the + // in-flight guard. + await r.emails + .reconcileDeletedImapForTest('acc-1', 'Archive', const []); + + expect(await r.emails.getEmail(oldId), isNotNull); + }, + ); }); group('Snooze', () { diff --git a/test/unit/fake_imap.dart b/test/unit/fake_imap.dart index 0df8b84..bcb5c52 100644 --- a/test/unit/fake_imap.dart +++ b/test/unit/fake_imap.dart @@ -19,9 +19,25 @@ class FakeImapClient extends imap.ImapClient { /// Spy IMAP client that records snooze-related operations and succeeds silently. class SnoozeSpyImapClient extends FakeImapClient { + SnoozeSpyImapClient({ + this.copyUidValidity, + this.copyUidSourceToTarget = const {}, + this.searchResults = const {}, + }); + String? selectedMailbox; String? createdMailbox; String? movedToMailbox; + String? lastSearchCriteria; + + /// When non-null, `uidMove` returns a `COPYUID` response code built from + /// these mappings (sourceUid → destinationUid) for the moved sequence. + final int? copyUidValidity; + final Map copyUidSourceToTarget; + + /// Maps a `UID SEARCH HEADER Message-ID …` search criteria (the literal + /// IMAP atom incl. quotes) to the UIDs the fake should return. + final Map> searchResults; imap.Mailbox _fakeMailbox(String path) => imap.Mailbox( encodedName: path, @@ -63,7 +79,33 @@ class SnoozeSpyImapClient extends FakeImapClient { String? targetMailboxPath, }) async { movedToMailbox = targetMailboxPath; - return imap.GenericImapResult(); + final result = imap.GenericImapResult(); + if (copyUidValidity != null && copyUidSourceToTarget.isNotEmpty) { + final sources = sequence.toList(); + final mapped = sources + .where(copyUidSourceToTarget.containsKey) + .map((uid) => copyUidSourceToTarget[uid]!) + .toList(); + if (mapped.isNotEmpty) { + final src = sources.join(','); + final dst = mapped.join(','); + result.responseCode = 'COPYUID $copyUidValidity $src $dst'; + } + } + return result; + } + + @override + Future uidSearchMessages({ + String searchCriteria = 'UNSEEN', + List? returnOptions, + Duration? responseTimeout, + }) async { + lastSearchCriteria = searchCriteria; + final hits = searchResults[searchCriteria] ?? const []; + final result = imap.SearchImapResult() + ..matchingSequence = imap.MessageSequence.fromIds(hits, isUid: true); + return result; } @override