fix(filesystem): backup dir exist causes moving directory failure

api/filesystem/filesystem.go

@@ -932,9 +932,13 @@ func FileExists(filePath string) (bool, error) {
 
 // SafeCopyDirectory copies a directory from src to dst in a safe way.
 func (service *Service) SafeMoveDirectory(originalPath, newPath string) error {
+	return safeMoveDirectory(originalPath, newPath, CopyDir)
+}
+
+func safeMoveDirectory(src, dst string, copyDirFunc func(string, string, bool) error) error {
 	// 1. Backup the source directory to a different folder
-	backupDir := fmt.Sprintf("%s-%s", filepath.Dir(originalPath), "backup")
-	err := MoveDirectory(originalPath, backupDir)
+	backupDir := fmt.Sprintf("%s-%s", src, "backup")
+	err := MoveDirectory(src, backupDir)
 	if err != nil {
 		return fmt.Errorf("failed to backup source directory: %w", err)
 	}
@@ -942,25 +946,25 @@ func (service *Service) SafeMoveDirectory(originalPath, newPath string) error {
 	defer func() {
 		if err != nil {
 			// If an error occurred, rollback the backup directory
-			restoreErr := restoreBackup(originalPath, backupDir)
+			restoreErr := restoreBackup(src, backupDir)
 			if restoreErr != nil {
 				log.Warn().Err(restoreErr).Msg("failed to restore backup during creating versioning folder")
 			}
 		}
+
+		// 3. Delete the backup directory
+		err = os.RemoveAll(backupDir)
+		if err != nil {
+			log.Warn().Err(err).Msg("failed to remove backup directory")
+		}
 	}()
 
 	// 2. Copy the backup directory to the destination directory
-	err = CopyDir(backupDir, newPath, false)
+	err = copyDirFunc(backupDir, dst, false)
 	if err != nil {
 		return fmt.Errorf("failed to copy backup directory to destination directory: %w", err)
 	}
 
-	// 3. Delete the backup directory
-	err = os.RemoveAll(backupDir)
-	if err != nil {
-		return fmt.Errorf("failed to delete backup directory: %w", err)
-	}
-
 	return nil
 }
 
@@ -991,7 +995,11 @@ func MoveDirectory(originalPath, newPath string) error {
 	}
 
 	if alreadyExists {
-		return errors.New("Target path already exists")
+		log.Info().Msgf("Target path already exists, removing it: %s", newPath)
+		err := os.RemoveAll(newPath)
+		if err != nil {
+			log.Warn().Err(err).Msgf("Failed to remove existing target path: %s", newPath)
+		}
 	}
 
 	return os.Rename(originalPath, newPath)

api/filesystem/filesystem_test.go

@@ -1,6 +1,7 @@
 package filesystem
 
 import (
+	"errors"
 	"os"
 	"path"
 	"testing"
@@ -20,3 +21,62 @@ func createService(t *testing.T) *Service {
 
 	return service
 }
+
+func prepareDir(t *testing.T, dir string) error {
+	err := os.MkdirAll(path.Join(dir, "data", "compose", "1"), 0755)
+	if err != nil {
+		return err
+	}
+
+	file, err := os.Create(path.Join(dir, "data", "compose", "1", "docker-compose.yml"))
+	if err != nil {
+		return err
+	}
+
+	file.Close()
+	return nil
+}
+
+func TestSafeSafeMoveDirectory(t *testing.T) {
+	t.Run("able to migrate original folder when the backup folder already exists", func(t *testing.T) {
+		testDir := path.Join(t.TempDir(), t.Name())
+		err := prepareDir(t, testDir)
+		assert.NoError(t, err, "prepareDir should not fail")
+		defer os.RemoveAll(testDir)
+
+		err = os.MkdirAll(path.Join(testDir, "data", "compose", "1-backup"), 0755)
+		assert.NoError(t, err, "create backupdir should not fail")
+		assert.DirExists(t, path.Join(testDir, "data", "compose", "1-backup"), "backupdir should exist")
+
+		src := path.Join(testDir, "data", "compose", "1")
+		dst := path.Join(testDir, "data", "compose", "1", "v1")
+		err = safeMoveDirectory(src, dst, CopyDir)
+		assert.NoError(t, err, "safeMoveDirectory should not fail")
+
+		assert.FileExists(t, path.Join(testDir, "data", "compose", "1", "v1", "docker-compose.yml"), "docker-compose.yml should be migrated")
+		assert.NoDirExists(t, path.Join(testDir, "data", "compose", "1-backup"), "backupdir should not exist")
+
+	})
+
+	t.Run("original folder can be restored if error occurs", func(t *testing.T) {
+		testDir := path.Join(t.TempDir(), t.Name())
+		err := prepareDir(t, testDir)
+		assert.NoError(t, err, "prepareDir should not fail")
+		defer os.RemoveAll(testDir)
+
+		src := path.Join(testDir, "data", "compose", "1")
+		dst := path.Join(testDir, "data", "compose", "1", "v1")
+
+		copyDirFunc := func(string, string, bool) error {
+			return errors.New("mock copy dir error")
+		}
+		err = safeMoveDirectory(src, dst, copyDirFunc)
+		assert.Error(t, err, "safeMoveDirectory should fail")
+
+		assert.FileExists(t, path.Join(testDir, "data", "compose", "1", "docker-compose.yml"), "original folder should be restored")
+		assert.NoDirExists(t, path.Join(testDir, "data", "compose", "1", "v1"), "the target folder should not exist")
+		assert.NoFileExists(t, path.Join(testDir, "data", "compose", "1", "v1", "docker-compose.yml"), "docker-compose.yml should not be migrated")
+		assert.NoDirExists(t, path.Join(testDir, "data", "compose", "1-backup"), "backupdir should not exist")
+	})
+
+}