imap: handle outdated sequence numbers from server upon move/delete

To delete N messages, aerc issues a single EXPUNGE command to the IMAP
server. The server sends back one ExpungeUpdate for each of those N
messages, and aerc reacts to each to update the folder contents.

The problem is that ExpungeUpdate messages contain a sequence number
that aerc uses to find the deleted message's UID, do its thing, and
*rightfully* pop the sequence number from the sequence. This works well
as long as N=1, but not otherwise: after the first ExpungeData, aerc and
the server have a different view of the sequence, and all subsequent
ExpungeData messages reference an *outdated* sequence number (yaaay
distributed systems :-D)), and
- If that number is higher than the current number of messages in the
folder, we log an error.
- Otherwise, we don't update the correct message in the UI :-/

This patch takes a snapshot of the current sequence numbers for all the
UIDs being deleted/moved, and uses it (not the actual sequence map) when
processing ExpungeUpdate messages from the server.

Signed-off-by: Simon Martin <simon@nasilyan.com>
Acked-by: Robin Jarry <robin@jarry.cc>

worker/imap/flags.go

@@ -49,6 +49,9 @@ func (imapw *IMAPWorker) handleDeleteMessages(msg *types.DeleteMessages) {
 	drain := imapw.drainUpdates()
 	defer drain.Close()
 
+	// Snapshot the current seqMap for proper ExpungeData processing.
+	imapw.SnapshotSequenceForExpunge(models.UidToUint32List(msg.Uids))
+
 	item := imap.FormatFlagsOp(imap.AddFlags, true)
 	flags := []interface{}{imap.DeletedFlag}
 	uids := toSeqSet(msg.Uids)

worker/imap/movecopy.go

@@ -3,6 +3,7 @@ package imap
 import (
 	"io"
 
+	"git.sr.ht/~rjarry/aerc/models"
 	"git.sr.ht/~rjarry/aerc/worker/types"
 )
 
@@ -51,6 +52,9 @@ func (imapw *IMAPWorker) handleMoveMessages(msg *types.MoveMessages) {
 	drain := imapw.drainUpdates()
 	defer drain.Close()
 
+	// Snapshot the current seqMap for proper ExpungeData processing.
+	imapw.SnapshotSequenceForExpunge(models.UidToUint32List(msg.Uids))
+
 	uids := toSeqSet(msg.Uids)
 	if err := imapw.client.UidMove(uids, msg.Destination); err != nil {
 		imapw.worker.PostMessage(&types.Error{

worker/imap/seqmap.go

@@ -1,6 +1,7 @@
 package imap
 
 import (
+	"slices"
 	"sort"
 	"sync"
 )
@@ -54,6 +55,20 @@ func (s *SeqMap) Put(uid uint32) {
 	s.lock.Unlock()
 }
 
+func (s *SeqMap) Snapshot(uids []uint32) map[uint32]uint32 {
+	snapshot := make(map[uint32]uint32)
+	s.lock.Lock()
+	for num, uid := range s.m {
+		if slices.Contains(uids, uid) {
+			// IMAP sequence numbers start at 1
+			seqNum := uint32(num) + 1
+			snapshot[seqNum] = uid
+		}
+	}
+	s.lock.Unlock()
+	return snapshot
+}
+
 // Pop removes seqnum from the SeqMap. seqnum must be a valid seqnum, ie
 // [1:size of mailbox]
 func (s *SeqMap) Pop(seqnum uint32) (uint32, bool) {

worker/imap/seqmap_test.go

@@ -82,4 +82,12 @@ func TestSeqMap(t *testing.T) {
 	wg.Wait()
 
 	assert.Equal(0, seqmap.Size())
+
+	// Test snapshotting
+	seqmap.Initialize([]uint32{21, 42, 1107, 1982, 2390, 27892, 32000})
+	snap := seqmap.Snapshot([]uint32{21, 1107, 27892, 1234567})
+	assert.Equal(3, len(snap))
+	assert.Equal(snap[1], uint32(21))
+	assert.Equal(snap[3], uint32(1107))
+	assert.Equal(snap[6], uint32(27892))
 }

worker/imap/worker.go

@@ -70,6 +70,7 @@ type IMAPWorker struct {
 	updates   chan client.Update
 	worker    types.WorkerInteractor
 	seqMap    SeqMap
+	expunging map[uint32]uint32
 	delimiter string
 
 	idler    *idler
@@ -293,7 +294,7 @@ func (w *IMAPWorker) handleImapUpdate(update client.Update) {
 			},
 		}, nil)
 	case *client.ExpungeUpdate:
-		if uid, found := w.seqMap.Pop(update.SeqNum); !found {
+		if uid, found := w.expunging[update.SeqNum]; !found {
 			w.worker.Errorf("ExpungeUpdate unknown seqnum: %d", update.SeqNum)
 		} else {
 			w.worker.PostMessage(&types.MessagesDeleted{
@@ -395,3 +396,7 @@ func (w *IMAPWorker) PathSeparator() string {
 	}
 	return w.delimiter
 }
+
+func (w *IMAPWorker) SnapshotSequenceForExpunge(uids []uint32) {
+	w.expunging = w.seqMap.Snapshot(uids)
+}