perf(ftp): reduce number of ftp commands run in install preparation (#63)

* perf(ftp): use MLST or LIST first to determine if path exists over ftp

* perf(ftp): optimistically check directories from target path up when creating directory

* fix(ftp): skip . and .. in ReadDir

* perf(remote): parallelize old mod removal

* perf(remote): parallelize install validation

* perf(remote): remove unnecessary validation in GetGameVersion

* pref(remote): reduce amount of Validate and GetPlatform calls

* chore: remove unnecessary error handling

cli/disk/ftp.go

@@ -3,12 +3,15 @@ package disk
 import (
 	"bytes"
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"log"
 	"log/slog"
+	"net/textproto"
 	"net/url"
-	"path/filepath"
+	"path"
+	"slices"
 	"strings"
 	"time"
 
@@ -91,7 +94,86 @@ func testFTP(u *url.URL, options ...ftp.DialOption) (*ftp.ServerConn, bool, erro
 	return c, false, nil
 }
 
-func (l *ftpDisk) Exists(path string) (bool, error) {
+func (l *ftpDisk) existsWithLock(res *puddle.Resource[*ftp.ServerConn], p string) (bool, error) {
+	slog.Debug("checking if file exists", slog.String("path", clean(p)), slog.String("schema", "ftp"))
+
+	var protocolError *textproto.Error
+
+	_, err := res.Value().GetEntry(clean(p))
+	if err == nil {
+		return true, nil
+	}
+
+	if errors.As(err, &protocolError) {
+		switch protocolError.Code {
+		case ftp.StatusFileUnavailable:
+			return false, nil
+		case ftp.StatusNotImplemented:
+			// GetEntry uses MLST, which might not be supported by the server.
+			// Even though in this case the error is not coming from the server,
+			// the ftp library still returns it as a protocol error.
+		default:
+			// We won't handle any other kind of error, such as
+			// * temporary errors (4xx) - should be retried after a while, so we won't deal with the delay
+			// * connection errors (x2x) - can't really do anything about them
+			// * authentication errors (x3x) - can't do anything about them
+			return false, fmt.Errorf("failed to get path info: %w", err)
+		}
+	} else {
+		// This is a non-protocol error, so we can't be sure what it means.
+		return false, fmt.Errorf("failed to get path info: %w", err)
+	}
+
+	// In case MLST is not supported, we can try to LIST the target path.
+	// We can be sure that List() will actually execute LIST and not MLSD,
+	// since MLST was not supported in the previous step.
+	entries, err := res.Value().List(clean(p))
+	if err == nil {
+		if len(entries) > 0 {
+			// Some server implementations return an empty list for a nonexistent path,
+			// so we cannot be sure that no error means a directory exists unless it also contains some items.
+			// For files, when they exist, they will be listed as a single entry.
+			// TODO: so far the servers (just one) this was happening on also listed . and .. for valid dirs, because it was using `LIST -a`. Is that behaviour consistent that we can rely on it?
+			return true, nil
+		}
+	} else {
+		if errors.As(err, &protocolError) {
+			if protocolError.Code == ftp.StatusFileUnavailable {
+				return false, nil
+			}
+		}
+		// We won't handle any other kind of error, see above.
+		return false, fmt.Errorf("failed to list path: %w", err)
+	}
+
+	// If we got here, either the path is an empty directory,
+	// or it does not exist and the server is a weird implementation.
+
+	// List the parent directory to determine if the path exists
+	dir, err := l.readDirLock(res, path.Dir(clean(p)))
+	if err == nil {
+		found := false
+		for _, entry := range dir {
+			if entry.Name() == path.Base(clean(p)) {
+				found = true
+				break
+			}
+		}
+
+		return found, nil
+	}
+
+	if errors.As(err, &protocolError) {
+		if protocolError.Code == ftp.StatusFileUnavailable {
+			return false, nil
+		}
+	}
+
+	// We won't handle any other kind of error, see above.
+	return false, fmt.Errorf("failed to list parent path: %w", err)
+}
+
+func (l *ftpDisk) Exists(p string) (bool, error) {
 	res, err := l.acquire()
 	if err != nil {
 		return false, err
@@ -99,49 +181,7 @@ func (l *ftpDisk) Exists(path string) (bool, error) {
 
 	defer res.Release()
 
-	slog.Debug("checking if file exists", slog.String("path", clean(path)), slog.String("schema", "ftp"))
+	return l.existsWithLock(res, p)
-
-	split := strings.Split(clean(path)[1:], "/")
-	for _, s := range split[:len(split)-1] {
-		dir, err := l.readDirLock(res, "")
-		if err != nil {
-			return false, err
-		}
-
-		currentDir, _ := res.Value().CurrentDir()
-
-		foundDir := false
-		for _, entry := range dir {
-			if entry.IsDir() && entry.Name() == s {
-				foundDir = true
-				break
-			}
-		}
-
-		if !foundDir {
-			return false, nil
-		}
-
-		slog.Debug("entering directory", slog.String("dir", s), slog.String("cwd", currentDir), slog.String("schema", "ftp"))
-		if err := res.Value().ChangeDir(s); err != nil {
-			return false, fmt.Errorf("failed to enter directory: %w", err)
-		}
-	}
-
-	dir, err := l.readDirLock(res, "")
-	if err != nil {
-		return false, fmt.Errorf("failed listing directory: %w", err)
-	}
-
-	found := false
-	for _, entry := range dir {
-		if entry.Name() == clean(filepath.Base(path)) {
-			found = true
-			break
-		}
-	}
-
-	return found, nil
 }
 
 func (l *ftpDisk) Read(path string) ([]byte, error) {
@@ -203,7 +243,7 @@ func (l *ftpDisk) Remove(path string) error {
 	return nil
 }
 
-func (l *ftpDisk) MkDir(path string) error {
+func (l *ftpDisk) MkDir(p string) error {
 	res, err := l.acquire()
 	if err != nil {
 		return err
@@ -211,34 +251,47 @@ func (l *ftpDisk) MkDir(path string) error {
 
 	defer res.Release()
 
-	split := strings.Split(clean(path)[1:], "/")
+	lastExistingDir := clean(p)
-	for _, s := range split {
+	for lastExistingDir != "/" && lastExistingDir != "." {
-		dir, err := l.readDirLock(res, "")
+		foundDir, err := l.existsWithLock(res, lastExistingDir)
 		if err != nil {
 			return err
 		}
 
-		currentDir, _ := res.Value().CurrentDir()
+		if foundDir {
-
-		foundDir := false
-		for _, entry := range dir {
-			if entry.IsDir() && entry.Name() == s {
-				foundDir = true
 			break
 		}
+
+		lastExistingDir = path.Dir(lastExistingDir)
 	}
 
-		if !foundDir {
+	remainingDirs := clean(p)
-			slog.Debug("making directory", slog.String("dir", s), slog.String("cwd", currentDir), slog.String("schema", "ftp"))
+
+	if lastExistingDir != "/" && lastExistingDir != "." {
+		remainingDirs = strings.TrimPrefix(remainingDirs, lastExistingDir)
+	}
+
+	if len(remainingDirs) == 0 {
+		// Already exists
+		return nil
+	}
+
+	if err := res.Value().ChangeDir(lastExistingDir); err != nil {
+		return fmt.Errorf("failed to enter directory: %w", err)
+	}
+
+	split := strings.Split(clean(remainingDirs)[1:], "/")
+	for _, s := range split {
+		slog.Debug("making directory", slog.String("dir", s), slog.String("cwd", lastExistingDir), slog.String("schema", "ftp"))
 		if err := res.Value().MakeDir(s); err != nil {
 			return fmt.Errorf("failed to make directory: %w", err)
 		}
-		}
 
-		slog.Debug("entering directory", slog.String("dir", s), slog.String("cwd", currentDir), slog.String("schema", "ftp"))
+		slog.Debug("entering directory", slog.String("dir", s), slog.String("cwd", lastExistingDir), slog.String("schema", "ftp"))
 		if err := res.Value().ChangeDir(s); err != nil {
 			return fmt.Errorf("failed to enter directory: %w", err)
 		}
+		lastExistingDir = path.Join(lastExistingDir, s)
 	}
 
 	return nil
@@ -252,7 +305,14 @@ func (l *ftpDisk) ReadDir(path string) ([]Entry, error) {
 
 	defer res.Release()
 
-	return l.readDirLock(res, path)
+	entries, err := l.readDirLock(res, path)
+	if err != nil {
+		return nil, err
+	}
+	entries = slices.DeleteFunc(entries, func(i Entry) bool {
+		return i.Name() == "." || i.Name() == ".."
+	})
+	return entries, nil
 }
 
 func (l *ftpDisk) readDirLock(res *puddle.Resource[*ftp.ServerConn], path string) ([]Entry, error) {

cli/installations.go

@@ -183,6 +183,8 @@ func (i *Installations) DeleteInstallation(installPath string) error {
 	return nil
 }
 
+var rootExecutables = []string{"FactoryGame.exe", "FactoryServer.sh", "FactoryServer.exe"}
+
 func (i *Installation) Validate(ctx *GlobalContext) error {
 	found := false
 	for _, p := range ctx.Profiles.Profiles {
@@ -203,31 +205,25 @@ func (i *Installation) Validate(ctx *GlobalContext) error {
 
 	foundExecutable := false
 
-	exists, err := d.Exists(filepath.Join(i.BasePath(), "FactoryGame.exe"))
+	var checkWait errgroup.Group
+
+	for _, executable := range rootExecutables {
+		e := executable
+		checkWait.Go(func() error {
+			exists, err := d.Exists(filepath.Join(i.BasePath(), e))
 			if !exists {
 				if err != nil {
-			return fmt.Errorf("failed reading FactoryGame.exe: %w", err)
+					return fmt.Errorf("failed reading %s: %w", e, err)
 				}
 			} else {
 				foundExecutable = true
 			}
+			return nil
+		})
+	}
 
-	exists, err = d.Exists(filepath.Join(i.BasePath(), "FactoryServer.sh"))
+	if err = checkWait.Wait(); err != nil {
-	if !exists {
+		return err //nolint:wrapcheck
-		if err != nil {
-			return fmt.Errorf("failed reading FactoryServer.sh: %w", err)
-		}
-	} else {
-		foundExecutable = true
-	}
-
-	exists, err = d.Exists(filepath.Join(i.BasePath(), "FactoryServer.exe"))
-	if !exists {
-		if err != nil {
-			return fmt.Errorf("failed reading FactoryServer.exe: %w", err)
-		}
-	} else {
-		foundExecutable = true
 	}
 
 	if !foundExecutable {
@@ -243,26 +239,18 @@ var (
 	matchAllCap     = regexp.MustCompile(`([a-z\d])([A-Z])`)
 )
 
-func (i *Installation) LockFilePath(ctx *GlobalContext) (string, error) {
+func (i *Installation) lockFilePath(ctx *GlobalContext, platform *Platform) string {
-	platform, err := i.GetPlatform(ctx)
-	if err != nil {
-		return "", err
-	}
-
 	lockFileName := ctx.Profiles.Profiles[i.Profile].Name
 	lockFileName = matchFirstCap.ReplaceAllString(lockFileName, "${1}_${2}")
 	lockFileName = matchAllCap.ReplaceAllString(lockFileName, "${1}_${2}")
 	lockFileName = lockFileCleaner.ReplaceAllLiteralString(lockFileName, "-")
 	lockFileName = strings.ToLower(lockFileName) + "-lock.json"
 
-	return filepath.Join(i.BasePath(), platform.LockfilePath, lockFileName), nil
+	return filepath.Join(i.BasePath(), platform.LockfilePath, lockFileName)
 }
 
-func (i *Installation) LockFile(ctx *GlobalContext) (*resolver.LockFile, error) {
+func (i *Installation) lockfile(ctx *GlobalContext, platform *Platform) (*resolver.LockFile, error) {
-	lockfilePath, err := i.LockFilePath(ctx)
+	lockfilePath := i.lockFilePath(ctx, platform)
-	if err != nil {
-		return nil, err
-	}
 
 	d, err := i.GetDisk()
 	if err != nil {
@@ -291,11 +279,8 @@ func (i *Installation) LockFile(ctx *GlobalContext) (*resolver.LockFile, error)
 	return lockFile, nil
 }
 
-func (i *Installation) WriteLockFile(ctx *GlobalContext, lockfile *resolver.LockFile) error {
+func (i *Installation) writeLockFile(ctx *GlobalContext, platform *Platform, lockfile *resolver.LockFile) error {
-	lockfilePath, err := i.LockFilePath(ctx)
+	lockfilePath := i.lockFilePath(ctx, platform)
-	if err != nil {
-		return err
-	}
 
 	d, err := i.GetDisk()
 	if err != nil {
@@ -341,15 +326,15 @@ func (i *Installation) Wipe() error {
 	return nil
 }
 
-func (i *Installation) ResolveProfile(ctx *GlobalContext) (*resolver.LockFile, error) {
+func (i *Installation) resolveProfile(ctx *GlobalContext, platform *Platform) (*resolver.LockFile, error) {
-	lockFile, err := i.LockFile(ctx)
+	lockFile, err := i.lockfile(ctx, platform)
 	if err != nil {
 		return nil, err
 	}
 
 	depResolver := resolver.NewDependencyResolver(ctx.Provider, viper.GetString("api-base"))
 
-	gameVersion, err := i.GetGameVersion(ctx)
+	gameVersion, err := i.getGameVersion(platform)
 	if err != nil {
 		return nil, fmt.Errorf("failed to detect game version: %w", err)
 	}
@@ -359,13 +344,37 @@ func (i *Installation) ResolveProfile(ctx *GlobalContext) (*resolver.LockFile, e
 		return nil, fmt.Errorf("could not resolve mods: %w", err)
 	}
 
-	if err := i.WriteLockFile(ctx, lockfile); err != nil {
+	if err := i.writeLockFile(ctx, platform, lockfile); err != nil {
 		return nil, fmt.Errorf("failed to write lockfile: %w", err)
 	}
 
 	return lockfile, nil
 }
 
+func (i *Installation) GetGameVersion(ctx *GlobalContext) (int, error) {
+	platform, err := i.GetPlatform(ctx)
+	if err != nil {
+		return 0, err
+	}
+	return i.getGameVersion(platform)
+}
+
+func (i *Installation) LockFile(ctx *GlobalContext) (*resolver.LockFile, error) {
+	platform, err := i.GetPlatform(ctx)
+	if err != nil {
+		return nil, err
+	}
+	return i.lockfile(ctx, platform)
+}
+
+func (i *Installation) WriteLockFile(ctx *GlobalContext, lockfile *resolver.LockFile) error {
+	platform, err := i.GetPlatform(ctx)
+	if err != nil {
+		return err
+	}
+	return i.writeLockFile(ctx, platform, lockfile)
+}
+
 type InstallUpdateType string
 
 var (
@@ -387,10 +396,6 @@ type InstallUpdateItem struct {
 }
 
 func (i *Installation) Install(ctx *GlobalContext, updates chan<- InstallUpdate) error {
-	if err := i.Validate(ctx); err != nil {
-		return fmt.Errorf("failed to validate installation: %w", err)
-	}
-
 	platform, err := i.GetPlatform(ctx)
 	if err != nil {
 		return fmt.Errorf("failed to detect platform: %w", err)
@@ -400,7 +405,7 @@ func (i *Installation) Install(ctx *GlobalContext, updates chan<- InstallUpdate)
 
 	if !i.Vanilla {
 		var err error
-		lockfile, err = i.ResolveProfile(ctx)
+		lockfile, err = i.resolveProfile(ctx, platform)
 		if err != nil {
 			return fmt.Errorf("failed to resolve lockfile: %w", err)
 		}
@@ -421,25 +426,35 @@ func (i *Installation) Install(ctx *GlobalContext, updates chan<- InstallUpdate)
 		return fmt.Errorf("failed to read mods directory: %w", err)
 	}
 
+	var deleteWait errgroup.Group
 	for _, entry := range dir {
 		if entry.IsDir() {
 			if _, ok := lockfile.Mods[entry.Name()]; !ok {
-				modDir := filepath.Join(modsDirectory, entry.Name())
+				modName := entry.Name()
+				modDir := filepath.Join(modsDirectory, modName)
+				deleteWait.Go(func() error {
 					exists, err := d.Exists(filepath.Join(modDir, ".smm"))
 					if err != nil {
 						return err
 					}
 
 					if exists {
-					slog.Info("deleting mod", slog.String("mod_reference", entry.Name()))
+						slog.Info("deleting mod", slog.String("mod_reference", modName))
 						if err := d.Remove(modDir); err != nil {
 							return fmt.Errorf("failed to delete mod directory: %w", err)
 						}
 					}
+
+					return nil
+				})
 			}
 		}
 	}
 
+	if err := deleteWait.Wait(); err != nil {
+		return fmt.Errorf("failed to remove old mods: %w", err)
+	}
+
 	slog.Info("starting installation", slog.Int("concurrency", viper.GetInt("concurrent-downloads")), slog.String("path", i.Path))
 
 	errg := errgroup.Group{}
@@ -523,18 +538,19 @@ func (i *Installation) Install(ctx *GlobalContext, updates chan<- InstallUpdate)
 }
 
 func (i *Installation) UpdateMods(ctx *GlobalContext, mods []string) error {
-	if err := i.Validate(ctx); err != nil {
+	platform, err := i.GetPlatform(ctx)
-		return fmt.Errorf("failed to validate installation: %w", err)
+	if err != nil {
+		return err
 	}
 
-	lockFile, err := i.LockFile(ctx)
+	lockFile, err := i.lockfile(ctx, platform)
 	if err != nil {
 		return fmt.Errorf("failed to read lock file: %w", err)
 	}
 
 	resolver := resolver.NewDependencyResolver(ctx.Provider, viper.GetString("api-base"))
 
-	gameVersion, err := i.GetGameVersion(ctx)
+	gameVersion, err := i.getGameVersion(platform)
 	if err != nil {
 		return fmt.Errorf("failed to detect game version: %w", err)
 	}
@@ -553,7 +569,7 @@ func (i *Installation) UpdateMods(ctx *GlobalContext, mods []string) error {
 		return fmt.Errorf("failed to resolve dependencies: %w", err)
 	}
 
-	if err := i.WriteLockFile(ctx, newLockFile); err != nil {
+	if err := i.writeLockFile(ctx, platform, newLockFile); err != nil {
 		return fmt.Errorf("failed to write lock file: %w", err)
 	}
 
@@ -667,30 +683,13 @@ type gameVersionFile struct {
 	IsPromotedBuild      int    `json:"IsPromotedBuild"`
 }
 
-func (i *Installation) GetGameVersion(ctx *GlobalContext) (int, error) {
+func (i *Installation) getGameVersion(platform *Platform) (int, error) {
-	if err := i.Validate(ctx); err != nil {
-		return 0, fmt.Errorf("failed to validate installation: %w", err)
-	}
-
-	platform, err := i.GetPlatform(ctx)
-	if err != nil {
-		return 0, err
-	}
-
 	d, err := i.GetDisk()
 	if err != nil {
 		return 0, err
 	}
 
 	fullPath := filepath.Join(i.BasePath(), platform.VersionPath)
-	exists, err := d.Exists(fullPath)
-	if err != nil {
-		return 0, err
-	}
-
-	if !exists {
-		return 0, errors.New("game version file does not exist")
-	}
 
 	file, err := d.Read(fullPath)
 	if err != nil {