imap: properly handle out-of-band Unseen flag updates

I noticed that the fix I made via b576ab28 (and the general idea to get
rid of materialized RUE counters) does not work, because we're extremely
unlikely to have all the messages' flags in memory. So the patch worked
"by chance" when updating messages that are in the active message list
(e.g. their headers are already fetched) but not for "out of view"
messages.

This alternative patch "just" captures unsolicited MessageUpdate
messages from the server, and updates the Unseen counter according to
the status reported by the server for that message. This allows to
synchronize our view with the server's.

Fixes: b576ab28 ("properly update UI upon message (un)read in [...]")
Fixes: https://todo.sr.ht/~rjarry/aerc/307
Signed-off-by: Simon Martin <simon@nasilyan.com>
Tested-by: Karel Balej <balejk@matfyz.cz>
Acked-by: Robin Jarry <robin@jarry.cc>

app/account.go

@@ -455,7 +455,6 @@ func (acct *AccountView) onMessage(msg types.WorkerMessage) {
 				acct.msglist.SetStore(store)
 			}
 			store.Update(msg)
-			acct.refreshDirCounts(store.Name)
 			acct.SetStatus(state.Threading(store.ThreadedView()))
 		}
 		if acct.newConn && len(msg.Uids) == 0 {
@@ -478,14 +477,23 @@ func (acct *AccountView) onMessage(msg types.WorkerMessage) {
 		}
 	case *types.MessageInfo:
 		if store, ok := acct.dirlist.SelectedMsgStore(); ok {
-			store.Update(msg)
-			// It this is an update of a message we already know, we'll
-			// have merged the update into the existing message already,
-			// but not the properties materialized outside of the Messages
-			// array, e.g. the Seen and Recent counters; do it now.
-			if dir := acct.dirlist.SelectedDirectory(); dir != nil {
-				acct.refreshDirCounts(dir.Name)
+			if msg.Unsolicited {
+				// This is a server generated message update, e.g. a
+				// notification that a message has changed; this will happen
+				// mostly for flags according to section 2.3.1.1 alinea 4 in
+				// the IMAP4rev1 RFC.
+				if dir := acct.dirlist.SelectedDirectory(); dir != nil {
+					// Our view of Unseen is out-of-sync with the server's;
+					// update it now.
+					if msg.Info.Flags.Has(models.SeenFlag) {
+						dir.Unseen -= 1
+					} else {
+						dir.Unseen += 1
+					}
+					dir.Unseen = acct.ensurePositive(dir.Unseen, "Unseen")
+				}
 			}
+			store.Update(msg)
 		}
 	case *types.MessagesDeleted:
 		if dir := acct.dirlist.SelectedDirectory(); dir != nil {
@@ -570,39 +578,6 @@ func (acct *AccountView) updateDirCounts(destination string, uids []models.UID,
 	}
 }
 
-func (acct *AccountView) refreshDirCounts(destination string) {
-	// Only update the destination destDir if it is initialized
-	if destDir := acct.dirlist.Directory(destination); destDir != nil {
-		store := acct.Store()
-		if store == nil {
-			// This may look a bit of unnecessary paranoid programming, but it
-			// happened once during my manual monkey testing :-)
-			acct.worker.Errorf("No message store for directory %s", destination)
-			return
-		}
-		var count, recent, unseen int
-		for _, msg := range acct.Store().Messages {
-			count++
-			if msg == nil {
-				// Don't trust the store for status if it's not fully loaded
-				// yet.
-				return
-			}
-			if msg.Flags.Has(models.RecentFlag) {
-				recent++
-			}
-			if !msg.Flags.Has(models.SeenFlag) {
-				unseen++
-			}
-		}
-		destDir.Unseen = acct.ensurePositive(unseen, "Unseen")
-		destDir.Recent = acct.ensurePositive(recent, "Recent")
-		destDir.Exists = acct.ensurePositive(count, "Exists")
-	} else {
-		acct.worker.Errorf("Skipping unknown directory %s", destination)
-	}
-}
-
 func (acct *AccountView) SortCriteria(uiConf *config.UIConfig) []*types.SortCriterion {
 	if uiConf == nil {
 		return nil

lib/msgstore.go

@@ -319,6 +319,12 @@ func (store *MessageStore) Update(msg types.WorkerMessage) {
 			if store.selectedUid == msg.Info.Uid {
 				store.onSelect(msg.Info)
 			}
+		} else if msg.Unsolicited {
+			// We received an unsolicited update for a message we don't have
+			// in store; store that update as is to ensure we have the same
+			// "Unseen status" as the server. It will be properly replaced
+			// if/when we need to fetch the message.
+			store.Messages[msg.Info.Uid] = msg.Info
 		}
 		if msg.NeedsFlags {
 			store.Lock()

worker/imap/worker.go

@@ -291,6 +291,7 @@ func (w *IMAPWorker) handleImapUpdate(update client.Update) {
 				InternalDate:  msg.InternalDate,
 				Uid:           models.Uint32ToUid(msg.Uid),
 			},
+			Unsolicited: true,
 		}, nil)
 	case *client.ExpungeUpdate:
 		if uid, found := w.seqMap.Pop(update.SeqNum); !found {

worker/types/messages.go

@@ -254,8 +254,9 @@ type SearchResults struct {
 
 type MessageInfo struct {
 	Message
-	Info       *models.MessageInfo
-	NeedsFlags bool
+	Info        *models.MessageInfo
+	NeedsFlags  bool
+	Unsolicited bool
 }
 
 type FullMessage struct {