fix: don't resurrect locally-deleted IMAP message on next sync
The incremental IMAP sync issued `UID ${lastUid + 1}:*` to look for new
mail. RFC 3501 §6.4.4 reverses `n:*` to `*:n` when n exceeds the largest
UID, so a server with one message at UID 1 and `lastUid=1` returned UID 1
for `UID 2:*` — re-fetching and re-inserting a row the user had just
deleted locally (whose pending change had not yet flushed).
`_fetchAndUpsertImap` now looks up the UIDs in the mailbox that have a
pending `delete` or `move` queued and skips the insert for those. The
existing `UID n:*` query is left intact so freshly-delivered SMTP mail
keeps driving StreamBuilder rebuilds in the E2E flow.
Regression test in `email_repository_imap_test.dart` deletes a synced
message and calls `syncEmails` directly — exactly what the in-app sync
button does — and asserts the row stays gone with the pending change
still queued.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
db05878aca
commit
fb767a8489
@@ -6,6 +6,33 @@ Tasks get moved from next.md to done.md
|
||||
|
||||
## Tasks
|
||||
|
||||
## IMAP delete: locally-deleted message no longer reappears after sync
|
||||
|
||||
User report: deleting an IMAP message removes it from the list, but tapping
|
||||
the sync button before the next background flush makes it pop back in.
|
||||
|
||||
Reproduced in `test/integration/email_repository_imap_test.dart` with a new
|
||||
case `syncEmails after local delete does not resurrect message`: it deletes an
|
||||
email locally (which queues a pending change and drops the cached row), then
|
||||
calls `syncEmails` directly — exactly what the sync button does — and
|
||||
asserts the row stays gone and the pending change stays queued.
|
||||
|
||||
Root cause: the incremental IMAP sync issues `UID ${lastUid + 1}:*` to look
|
||||
for new mail. Per RFC 3501 §6.4.4 a sequence range `n:*` reverses to `*:n`
|
||||
when `n` exceeds the largest UID. With one message at UID 1 and `lastUid=1`,
|
||||
`UID 2:*` reverses to `*:2` and the server returns UID 1, which then gets
|
||||
re-fetched and re-inserted — undoing the optimistic local delete.
|
||||
|
||||
Fix in `lib/data/repositories/email_repository_impl.dart`: in
|
||||
`_fetchAndUpsertImap`, look up the UIDs in this mailbox that have a pending
|
||||
`delete` or `move` queued and skip the insert for those. Keeping the `UID n:*`
|
||||
search untouched preserves the existing E2E flow where re-fetching freshly
|
||||
delivered SMTP messages drives the StreamBuilder rebuild.
|
||||
|
||||
Same protection guards the `move`-on-delete path (when a Trash mailbox is
|
||||
configured) for free, since `moveEmail` enqueues a `move` and drops the cached
|
||||
row in the source mailbox.
|
||||
|
||||
## task deploy-android works end-to-end
|
||||
|
||||
The original "Emulator did not become ready within 120 s" was already resolved in
|
||||
|
||||
@@ -438,6 +438,8 @@ class EmailRepositoryImpl implements EmailRepository {
|
||||
final fetch = sequence.isUidSequence
|
||||
? await client.uidFetchMessages(sequence, fetchItems)
|
||||
: await client.fetchMessages(sequence, fetchItems);
|
||||
final pendingByUid =
|
||||
await _pendingDeleteOrMoveUids(account.id, mailboxPath);
|
||||
var bytes = 0;
|
||||
await _db.transaction(() async {
|
||||
for (final msg in fetch.messages) {
|
||||
@@ -451,6 +453,18 @@ class EmailRepositoryImpl implements EmailRepository {
|
||||
log('IMAP: skipping message with no uid (mailbox=$mailboxPath)');
|
||||
continue;
|
||||
}
|
||||
// Don't resurrect a row the user has already removed locally via a
|
||||
// pending delete or move. The IMAP server still has the message
|
||||
// until the next flushPendingChanges, and `UID lastUid+1:*` can
|
||||
// even return a UID smaller than `lastUid+1` because RFC 3501
|
||||
// §6.4.4 reverses `n:*` to `*:n` when `n` exceeds the largest UID.
|
||||
if (pendingByUid.containsKey(uid)) {
|
||||
log(
|
||||
'IMAP: skipping insert for uid=$uid in $mailboxPath '
|
||||
'(pending ${pendingByUid[uid]})',
|
||||
);
|
||||
continue;
|
||||
}
|
||||
bytes += msg.size ?? 0;
|
||||
final emailId = '${account.id}:$uid';
|
||||
final msgId = envelope.messageId?.trim();
|
||||
@@ -488,6 +502,35 @@ class EmailRepositoryImpl implements EmailRepository {
|
||||
return bytes;
|
||||
}
|
||||
|
||||
// UIDs in [mailboxPath] that have a pending local delete or move queued.
|
||||
// Used by the IMAP fetch path to avoid re-inserting rows the user has
|
||||
// already removed from view but whose change has not yet flushed.
|
||||
Future<Map<int, String>> _pendingDeleteOrMoveUids(
|
||||
String accountId,
|
||||
String mailboxPath,
|
||||
) async {
|
||||
final rows = await (_db.select(_db.pendingChanges)
|
||||
..where(
|
||||
(t) =>
|
||||
t.accountId.equals(accountId) &
|
||||
t.resourceType.equals('Email') &
|
||||
(t.changeType.equals('delete') | t.changeType.equals('move')),
|
||||
))
|
||||
.get();
|
||||
final result = <int, String>{};
|
||||
for (final r in rows) {
|
||||
try {
|
||||
final payload = jsonDecode(r.payload) as Map<String, dynamic>;
|
||||
if (payload['mailboxPath'] != mailboxPath) continue;
|
||||
final uid = payload['uid'];
|
||||
if (uid is int) result[uid] = r.changeType;
|
||||
} catch (_) {
|
||||
// Malformed payload — skip.
|
||||
}
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
Future<Map<String, dynamic>?> _loadImapCheckpoint(
|
||||
String accountId,
|
||||
String resourceType,
|
||||
|
||||
@@ -520,4 +520,39 @@ void main() {
|
||||
await client.logout();
|
||||
}
|
||||
});
|
||||
|
||||
// Regression: the in-app "sync" button calls syncEmails directly and does
|
||||
// not invoke flushPendingChanges itself. After a local delete, the message
|
||||
// is removed from the local DB optimistically but still lives on the server
|
||||
// until the pending change is flushed. If syncEmails runs before that flush,
|
||||
// it must not resurrect the deleted row from the server.
|
||||
test('syncEmails after local delete does not resurrect message', () async {
|
||||
await appendToInbox('delete-and-sync');
|
||||
|
||||
final r = makeRepo();
|
||||
await r.accounts.addAccount(account, userPass);
|
||||
await r.emails.syncEmails('test', 'INBOX');
|
||||
|
||||
final emails = await r.emails.observeEmails('test', 'INBOX').first;
|
||||
expect(emails, hasLength(1));
|
||||
|
||||
// User taps delete in the UI: enqueues a pending change, drops local row.
|
||||
await r.emails.deleteEmail(emails.first.id);
|
||||
expect(await r.emails.observeEmails('test', 'INBOX').first, isEmpty);
|
||||
|
||||
// User taps the sync button: syncEmails runs without flushPendingChanges.
|
||||
await r.emails.syncEmails('test', 'INBOX');
|
||||
|
||||
final after = await r.emails.observeEmails('test', 'INBOX').first;
|
||||
expect(
|
||||
after,
|
||||
isEmpty,
|
||||
reason: 'deleted message must not reappear on next sync',
|
||||
);
|
||||
|
||||
// The pending delete must still be queued for the next flush.
|
||||
final pending = await r.db.select(r.db.pendingChanges).get();
|
||||
expect(pending, hasLength(1));
|
||||
expect(pending.first.changeType, 'delete');
|
||||
});
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user