msgstore: properly rebuild message store upon reconnection

I noticed another status synchronization issue between aerc and the IMAP
server when multiple clients are involved *and* the computer running
aerc hibernates. I acknowledge it's probably not very frequent but
that's my day to day ;-)

The scenario is as follows:
- aerc sees new unread emails, and renders them properly
- The computer running aerc goes to sleep
- Those emails get read via another client
- The computer running aerc wakes up, and aerc reconnects to the IMAP
  server and creates a new session.
- aerc does NOT redraw the message list (because no other mail was
  received while it was asleep) and from there on
   - The unread messages remain bold
   - Explicitly marking them read does not do anything because this
     involves a call to the server that already knows the message is
     read, and does not send back a MessageUpdate (that aerc usually
     reacts to to "unbold" the message)

This patch fixes this issue by making sure that we clear the directory
list's message store upon disconnection from the backend, and
recomputing the RUE counts from memory when processing a
types.DirectoryContents, via new AccountView.refreshDirCounts function.
It also leverages this function in the fix I made earlier via
https://lists.sr.ht/~rjarry/aerc-devel/patches/58306, and protects from
showing negative counters in the UI, just in case.

This function is pretty cheap since it's working on memory (we're well
below the millisecond on my largish mailbox - see
https://paste.sr.ht/~simartin/124a5fa4568a4ed8cb6e639c62bb01b2d6803588),
and given the number of synchronization issues I'm meeting with those
counters, my aim is to leverage it more in the future, and ideally get
rid of the RUE counts in models.Directory, and the dance we're doing to
try to keep them up-to-date.

Signed-off-by: Simon Martin <simon@nasilyan.com>
Tested-by: Julio B <julio.bacel@gmail.com>
Acked-by: Robin Jarry <robin@jarry.cc>

app/account.go

@@ -445,6 +445,7 @@ 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 {
@@ -467,26 +468,14 @@ func (acct *AccountView) onMessage(msg types.WorkerMessage) {
 		}
 	case *types.MessageInfo:
 		if store, ok := acct.dirlist.SelectedMsgStore(); ok {
-
-			if existing := store.Messages[msg.Info.Uid]; existing != nil {
-				// 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. Only consider Seen
-				// since it's the only one that can be altered as per
-				// https://datatracker.ietf.org/doc/html/rfc3501#section-2.3.2.
-				old_seen := existing.Flags.Has(models.SeenFlag)
-				new_seen := msg.Info.Flags.Has(models.SeenFlag)
-				dir := acct.dirlist.SelectedDirectory()
-				if old_seen != new_seen && dir != nil {
-					if old_seen && !new_seen {
-						dir.Unseen += 1
-					} else if !old_seen && new_seen {
-						dir.Unseen -= 1
-					}
-				}
-			}
 			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)
+			}
 		}
 	case *types.MessagesDeleted:
 		if dir := acct.dirlist.SelectedDirectory(); dir != nil {
@@ -515,6 +504,14 @@ func (acct *AccountView) onMessage(msg types.WorkerMessage) {
 	acct.setTitle()
 }
 
+func (acct *AccountView) ensurePositive(val int, name string) int {
+	if val < 0 {
+		acct.worker.Errorf("Unexpected negative value (%d) for %s", val, name)
+		return 0
+	}
+	return val
+}
+
 func (acct *AccountView) updateDirCounts(destination string, uids []models.UID, deleted bool) {
 	// Only update the destination destDir if it is initialized
 	if destDir := acct.dirlist.Directory(destination); destDir != nil {
@@ -555,6 +552,40 @@ func (acct *AccountView) updateDirCounts(destination string, uids []models.UID,
 		} else {
 			destDir.Exists -= len(uids)
 		}
+		destDir.Unseen = acct.ensurePositive(destDir.Unseen, "Unseen")
+		destDir.Recent = acct.ensurePositive(destDir.Recent, "Recent")
+		destDir.Exists = acct.ensurePositive(destDir.Exists, "Exists")
+	} else {
+		acct.worker.Errorf("Skipping unknown directory %s", destination)
+	}
+}
+
+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 {
+				continue
+			}
+			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)
 	}

app/dirlist.go

@@ -104,6 +104,7 @@ func (dirlist *DirectoryList) List() []string {
 }
 
 func (dirlist *DirectoryList) ClearList() {
+	dirlist.store = lib.NewDirStore()
 	dirlist.dirs = []string{}
 }