rmdir: ensure proper sequence of operations

Ensure the proper sequence of opening and removing a directory. Fix a
potential race between the OpenDirectory (issued by dirlist.Select())
and the RemoveDirectory messages when removing a directory.

Due to the delay in the current dirlist.Select() function, the
RemoveDirectory message can arrive at the backend before the directory
was changed to a different one. This can cause an error on some imap
servers and problems with the watcher on the maildir backends.

Introduce dirlist.Open() that accepts a callback function so that the
operations can be performed in proper sequence. Dirlist.Select() is now
a wrapper call to dirlist.Open().

Reported-by: inwit <inwit@sindominio.net>
Signed-off-by: Koni Marti <koni.marti@gmail.com>
Acked-by: Robin Jarry <robin@jarry.cc>
Tested-by: Inwit <inwit@sindominio.net>

4 files changed, 70 insertions, 17 deletions

diff --git a/app/dirlist.go b/app/dirlist.go
index d77ace42..708eb824 100644
--- a/app/dirlist.go
+++ b/app/dirlist.go
@@ -26,6 +26,7 @@ type DirectoryLister interface {
 
 	Selected() string
 	Select(string)
+	Open(string, time.Duration, func(types.WorkerMessage))
 
 	Update(types.WorkerMessage)
 	List() []string
@@ -172,11 +173,16 @@ func (dirlist *DirectoryList) ExpandFolder() {
 }
 
 func (dirlist *DirectoryList) Select(name string) {
+	dirlist.Open(name, dirlist.UiConfig(name).DirListDelay, nil)
+}
+
+func (dirlist *DirectoryList) Open(name string, delay time.Duration,
+	cb func(types.WorkerMessage),
+) {
 	dirlist.selecting = name
 
 	dirlist.cancel()
 	dirlist.ctx, dirlist.cancel = context.WithCancel(context.Background())
-	delay := dirlist.UiConfig(name).DirListDelay
 
 	go func(ctx context.Context) {
 		defer log.PanicHandler()
@@ -198,6 +204,9 @@ func (dirlist *DirectoryList) Select(name string) {
 					case *types.Cancelled:
 						log.Debugf("OpenDirectory cancelled")
 					}
+					if cb != nil {
+						cb(msg)
+					}
 				})
 		case <-ctx.Done():
 			log.Tracef("dirlist: skip %s", name)
diff --git a/app/dirtree.go b/app/dirtree.go
index 50f5797f..6a3b34c0 100644
--- a/app/dirtree.go
+++ b/app/dirtree.go
@@ -5,6 +5,7 @@ import (
 	"sort"
 	"strconv"
 	"strings"
+	"time"
 
 	"git.sr.ht/~rjarry/aerc/config"
 	"git.sr.ht/~rjarry/aerc/lib"
@@ -50,11 +51,14 @@ func (dt *DirectoryTree) Update(msg types.WorkerMessage) {
 	switch msg := msg.(type) {
 
 	case *types.Done:
-		switch msg.InResponseTo().(type) {
+		switch resp := msg.InResponseTo().(type) {
 		case *types.RemoveDirectory, *types.ListDirectories, *types.CreateDirectory:
 			dt.DirectoryList.Update(msg)
 			dt.buildTree()
 			dt.Invalidate()
+		case *types.OpenDirectory:
+			dt.reindex(resp.Directory)
+			dt.DirectoryList.Update(msg)
 		default:
 			dt.DirectoryList.Update(msg)
 		}
@@ -196,7 +200,7 @@ func (dt *DirectoryTree) SelectedMsgStore() (*lib.MessageStore, bool) {
 	return dt.DirectoryList.SelectedMsgStore()
 }
 
-func (dt *DirectoryTree) Select(name string) {
+func (dt *DirectoryTree) reindex(name string) {
 	idx := findString(dt.treeDirs, name)
 	if idx >= 0 {
 		selIdx, node := dt.getTreeNode(uint32(idx))
@@ -205,14 +209,24 @@ func (dt *DirectoryTree) Select(name string) {
 			dt.listIdx = selIdx
 		}
 	}
+}
 
+func (dt *DirectoryTree) Select(name string) {
 	if name == "" {
 		return
 	}
-
+	dt.reindex(name)
 	dt.DirectoryList.Select(name)
 }
 
+func (dt *DirectoryTree) Open(name string, delay time.Duration, cb func(types.WorkerMessage)) {
+	if name == "" {
+		return
+	}
+	dt.reindex(name)
+	dt.DirectoryList.Open(name, delay, cb)
+}
+
 func (dt *DirectoryTree) NextPrev(delta int) {
 	newIdx := dt.listIdx
 	ndirs := len(dt.list)
diff --git a/commands/account/mkdir.go b/commands/account/mkdir.go
index af3d1045..7e49ad8b 100644
--- a/commands/account/mkdir.go
+++ b/commands/account/mkdir.go
@@ -41,13 +41,15 @@ func (m MakeDir) Execute(args []string) error {
 	if acct == nil {
 		return errors.New("No account selected")
 	}
+	previous := acct.SelectedDirectory()
 	acct.Worker().PostAction(&types.CreateDirectory{
 		Directory: m.Folder,
 	}, func(msg types.WorkerMessage) {
 		switch msg := msg.(type) {
 		case *types.Done:
 			app.PushStatus("Directory created.", 10*time.Second)
-			acct.Directories().Select(m.Folder)
+			history[acct.Name()] = previous
+			acct.Directories().Open(m.Folder, 0, nil)
 		case *types.Error:
 			app.PushError(msg.Error.Error())
 		}
diff --git a/commands/account/rmdir.go b/commands/account/rmdir.go
index 398f375f..6590a819 100644
--- a/commands/account/rmdir.go
+++ b/commands/account/rmdir.go
@@ -36,12 +36,30 @@ func (r RemoveDir) Execute(args []string) error {
 	dirFound := false
 
 	if oldDir, ok := history[acct.Name()]; ok {
-		if oldDir != curDir {
+		present := false
+		for _, dir := range acct.Directories().List() {
+			if dir == oldDir {
+				present = true
+				break
+			}
+		}
+		if oldDir != curDir && present {
 			newDir = oldDir
 			dirFound = true
 		}
 	}
 
+	defaultDir := acct.AccountConfig().Default
+	if !dirFound && defaultDir != curDir {
+		for _, dir := range acct.Directories().List() {
+			if defaultDir == dir {
+				newDir = dir
+				dirFound = true
+				break
+			}
+		}
+	}
+
 	if !dirFound {
 		for _, dir := range acct.Directories().List() {
 			if dir != curDir {
@@ -56,20 +74,30 @@ func (r RemoveDir) Execute(args []string) error {
 		return errors.New("No directory to move to afterwards!")
 	}
 
-	acct.Directories().Select(newDir)
-
-	acct.Worker().PostAction(&types.RemoveDirectory{
-		Directory: curDir,
-		Quiet:     r.Force,
-	}, func(msg types.WorkerMessage) {
-		switch msg := msg.(type) {
+	acct.Directories().Open(newDir, 0, func(msg types.WorkerMessage) {
+		switch msg.(type) {
 		case *types.Done:
-			app.PushStatus("Directory removed.", 10*time.Second)
+			break
 		case *types.Error:
-			app.PushError(msg.Error.Error())
-		case *types.Unsupported:
-			app.PushError(":rmdir is not supported by the backend.")
+			app.PushError("Could not change directory")
+			acct.Directories().Open(curDir, 0, nil)
+			return
+		default:
+			return
 		}
+		acct.Worker().PostAction(&types.RemoveDirectory{
+			Directory: curDir,
+			Quiet:     r.Force,
+		}, func(msg types.WorkerMessage) {
+			switch msg := msg.(type) {
+			case *types.Done:
+				app.PushStatus("Directory removed.", 10*time.Second)
+			case *types.Error:
+				app.PushError(msg.Error.Error())
+			case *types.Unsupported:
+				app.PushError(":rmdir is not supported by the backend.")
+			}
+		})
 	})
 
 	return nil