Implement move-directory command, and use it in Simple Blender Render

internal/manager/job_compilers/job_compilers_test.go

@@ -84,8 +84,8 @@ func TestSimpleBlenderRenderHappy(t *testing.T) {
 
 	settings := sj.Settings.AdditionalProperties
 
-	// Tasks should have been created to render the frames: 1-3, 4-6, 7-9, 10, video-encoding
-	assert.Equal(t, 5, len(aj.Tasks))
+	// Tasks should have been created to render the frames: 1-3, 4-6, 7-9, 10, video-encoding, and cleanup
+	assert.Len(t, aj.Tasks, 6)
 	t0 := aj.Tasks[0]
 	expectCliArgs := []interface{}{ // They are strings, but Goja doesn't know that and will produce an []interface{}.
 		"--render-output", "/render/sprites/farm_output/promo/square_ellie/square_ellie.lighting_light_breakdown2__intermediate-2006-01-02_090405/######",
@@ -165,8 +165,8 @@ func TestSimpleBlenderRenderWindowsPaths(t *testing.T) {
 
 	settings := sj.Settings.AdditionalProperties
 
-	// Tasks should have been created to render the frames: 1-3, 4-6, 7-9, 10, video-encoding
-	assert.Equal(t, 5, len(aj.Tasks))
+	// Tasks should have been created to render the frames: 1-3, 4-6, 7-9, 10, video-encoding, and cleanup
+	assert.Len(t, aj.Tasks, 6)
 	t0 := aj.Tasks[0]
 	expectCliArgs := []interface{}{ // They are strings, but Goja doesn't know that and will produce an []interface{}.
 		// The render output is constructed by the job compiler, and thus transforms to forward slashes.
@@ -221,8 +221,8 @@ func TestSimpleBlenderRenderOutputPathFieldReplacement(t *testing.T) {
 	// The job compiler should have replaced the {timestamp} and {ext} fields.
 	assert.Equal(t, "/root/2006-01-02_090405/jobname/######", aj.Settings["render_output_path"])
 
-	// Tasks should have been created to render the frames: 1-3, 4-6, 7-9, 10, video-encoding
-	assert.Equal(t, 5, len(aj.Tasks))
+	// Tasks should have been created to render the frames: 1-3, 4-6, 7-9, 10, video-encoding, and cleanup
+	assert.Len(t, aj.Tasks, 6)
 	t0 := aj.Tasks[0]
 	expectCliArgs := []interface{}{ // They are strings, but Goja doesn't know that and will produce an []interface{}.
 		"--render-output", "/root/2006-01-02_090405/jobname__intermediate-2006-01-02_090405/######",

internal/manager/job_compilers/scripts/simple_blender_render.js

@@ -72,8 +72,10 @@ function compileJob(job) {
     const settings = job.settings;
     const renderTasks = authorRenderTasks(settings, renderDir, renderOutput);
     const videoTask = authorCreateVideoTask(settings, renderDir);
+    const cleanupTask = authorCleanupTask(finalDir, renderDir);
 
     for (const rt of renderTasks) {
+        cleanupTask.addDependency(rt);
         job.addTask(rt);
     }
     if (videoTask) {
@@ -81,8 +83,10 @@ function compileJob(job) {
         for (const rt of renderTasks) {
             videoTask.addDependency(rt);
         }
+        cleanupTask.addDependency(videoTask);
         job.addTask(videoTask);
     }
+    job.addTask(cleanupTask);
 }
 
 // Do field replacement on the render output path.
@@ -156,6 +160,16 @@ function authorCreateVideoTask(settings, renderDir) {
     return task;
 }
 
+function authorCleanupTask(finalDir, renderDir) {
+    const task = author.Task("move-to-final", "file-management");
+    const command = author.Command("move-directory", {
+        src: renderDir,
+        dest: finalDir,
+    });
+    task.addCommand(command);
+    return task;
+}
+
 // Return file name extension, including period, like '.png' or '.mkv'.
 function guessOutputFileExtension(settings) {
     switch (settings.images_or_video) {

internal/worker/command_exe.go

@@ -69,9 +69,15 @@ func NewCommandExecutor(cli CommandLineRunner, listener CommandListener, timeSer
 	// switch statement) makes it possible to do things like reporting the list of
 	// supported commands.
 	ce.registry = map[string]commandCallable{
+		// misc
 		"echo":  ce.cmdEcho,
 		"sleep": ce.cmdSleep,
+
+		// blender
 		"blender-render": ce.cmdBlenderRender,
+
+		// file-management
+		"move-directory": ce.cmdMoveDirectory,
 	}
 
 	return ce

internal/worker/command_file_mgmt.go

@@ -0,0 +1,152 @@
+package worker
+
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+/* This file contains the commands in the "file-management" type group. */
+
+import (
+	"context"
+	"fmt"
+	"os"
+	"path/filepath"
+	"regexp"
+	"strconv"
+	"time"
+
+	"github.com/rs/zerolog"
+
+	"git.blender.org/flamenco/pkg/api"
+)
+
+// cmdMoveDirectory executes the "move-directory" command.
+// It moves directory 'src' to 'dest'; if 'dest' already exists, it's moved to 'dest-{timestamp}'.
+func (ce *CommandExecutor) cmdMoveDirectory(ctx context.Context, logger zerolog.Logger, taskID string, cmd api.Command) error {
+	var src, dest string
+	var ok bool
+
+	if src, ok = cmdParameter[string](cmd, "src"); !ok || src == "" {
+		logger.Warn().Interface("command", cmd).Msg("missing 'src' parameter")
+		return fmt.Errorf("missing 'src' parameter: %+v", cmd.Parameters)
+	}
+	if dest, ok = cmdParameter[string](cmd, "dest"); !ok || dest == "" {
+		logger.Warn().Interface("command", cmd).Msg("missing 'dest' parameter")
+		return fmt.Errorf("missing 'dest' parameter: %+v", cmd.Parameters)
+	}
+
+	logger = logger.With().
+		Str("src", src).
+		Str("dest", dest).
+		Logger()
+	if !fileExists(src) {
+		logger.Warn().Msg("source path does not exist, not moving anything")
+
+		msg := fmt.Sprintf("%s: source path %q does not exist, not moving anything", cmd.Name, src)
+		if err := ce.listener.LogProduced(ctx, taskID, msg); err != nil {
+			return err
+		}
+		return fmt.Errorf(msg)
+	}
+
+	if fileExists(dest) {
+		backup, err := timestampedPath(dest)
+		if err != nil {
+			logger.Error().Err(err).Str("path", dest).Msg("unable to determine timestamp of directory")
+			return err
+		}
+
+		if fileExists(backup) {
+			logger.Debug().Str("backup", backup).Msg("backup destination also exists, finding one that does not")
+			backup, err = uniquePath(backup)
+			if err != nil {
+				return err
+			}
+		}
+
+		logger.Info().
+			Str("toBackup", backup).
+			Msg("dest directory exists, moving to backup")
+		if err := ce.moveAndLog(ctx, taskID, cmd.Name, dest, backup); err != nil {
+			return err
+		}
+	}
+
+	// self._log.info("Moving %s to %s", src, dest)
+	// await self.worker.register_log(
+	// 		"%s: Moving %s to %s", self.command_name, src, dest
+	// )
+	// src.rename(dest)
+	logger.Info().Msg("moving directory")
+	return ce.moveAndLog(ctx, taskID, cmd.Name, src, dest)
+}
+
+// moveAndLog renames a file/directory from `src` to `dest`, and logs the moveAndLog.
+// The other parameters are just for logging.
+func (ce *CommandExecutor) moveAndLog(ctx context.Context, taskID, cmdName, src, dest string) error {
+	msg := fmt.Sprintf("%s: moving %q to %q", cmdName, src, dest)
+	if err := ce.listener.LogProduced(ctx, taskID, msg); err != nil {
+		return err
+	}
+
+	if err := os.Rename(src, dest); err != nil {
+		msg := fmt.Sprintf("%s: could not move %q to %q: %v", cmdName, src, dest, err)
+		if err := ce.listener.LogProduced(ctx, taskID, msg); err != nil {
+			return err
+		}
+		return err
+	}
+
+	return nil
+}
+
+func fileExists(filename string) bool {
+	_, err := os.Stat(filename)
+	return !os.IsNotExist(err)
+}
+
+// timestampedPath returns the path with its modification time appended to the name.
+func timestampedPath(filepath string) (string, error) {
+	stat, err := os.Stat(filepath)
+	if err != nil {
+		return "", fmt.Errorf("getting mtime of %s: %w", filepath, err)
+	}
+
+	// Round away the milliseconds, as those aren't all that interesting.
+	// Uniqueness can ensured by calling unique_path() later.
+	mtime := stat.ModTime().Round(time.Second)
+
+	iso := mtime.Local().Format("2006-01-02_150405") // YYYY-MM-DD_HHMMSS
+	return fmt.Sprintf("%s-%s", filepath, iso), nil
+}
+
+// uniquePath returns the path, or if it exists, the path with a unique suffix.
+func uniquePath(path string) (string, error) {
+	matches, err := filepath.Glob(path + "-*")
+	if err != nil {
+		return "", err
+	}
+
+	suffixRe, err := regexp.Compile("-([0-9]+)$")
+	if err != nil {
+		return "", fmt.Errorf("compiling regular expression: %w", err)
+	}
+
+	var maxSuffix int64
+	for _, path := range matches {
+		matches := suffixRe.FindStringSubmatch(path)
+		if len(matches) < 2 {
+			continue
+		}
+		suffix := matches[1]
+		value, err := strconv.ParseInt(suffix, 10, 64)
+		if err != nil {
+			// Non-numeric suffixes are fine; they just don't count for this function.
+			continue
+		}
+
+		if value > maxSuffix {
+			maxSuffix = value
+		}
+	}
+
+	return fmt.Sprintf("%s-%03d", path, maxSuffix+1), nil
+}

internal/worker/command_file_mgmt_test.go

@@ -0,0 +1,270 @@
+package worker
+
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+import (
+	"context"
+	"fmt"
+	"io/fs"
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"git.blender.org/flamenco/pkg/api"
+	"github.com/golang/mock/gomock"
+	"github.com/stretchr/testify/assert"
+)
+
+type cmdMoveDirFixture struct {
+	mockCtrl *gomock.Controller
+	ce       *CommandExecutor
+	mocks    *CommandExecutorMocks
+	ctx      context.Context
+
+	temppath string
+	cwd      string
+}
+
+const (
+	taskID     = "90e9d656-e201-4ef0-b6b0-c80684fafa27"
+	sourcePath = "render/output/here__intermediate"
+	destPath   = "render/output/here"
+)
+
+func TestCmdMoveDirectoryNonExistentSourceDir(t *testing.T) {
+	f := newCmdMoveDirectoryFixture(t)
+	defer f.finish(t)
+
+	f.mocks.listener.EXPECT().LogProduced(gomock.Any(), taskID,
+		"move-directory: source path \"render/output/here__intermediate\" does not exist, not moving anything")
+	err := f.run()
+	assert.Error(t, err)
+}
+
+func TestCmdMoveDirectoryHappy(t *testing.T) {
+	f := newCmdMoveDirectoryFixture(t)
+	defer f.finish(t)
+
+	ensureDirExists(sourcePath)
+	fileCreateEmpty(filepath.Join(sourcePath, "testfile.txt"))
+
+	f.mocks.listener.EXPECT().LogProduced(gomock.Any(), taskID,
+		"move-directory: moving \"render/output/here__intermediate\" to \"render/output/here\"")
+	err := f.run()
+	assert.NoError(t, err)
+
+	assert.NoDirExists(t, sourcePath)
+	assert.DirExists(t, destPath)
+	assert.NoFileExists(t, filepath.Join(sourcePath, "testfile.txt"))
+	assert.FileExists(t, filepath.Join(destPath, "testfile.txt"))
+}
+
+func TestCmdMoveDirectoryExistingDest(t *testing.T) {
+	f := newCmdMoveDirectoryFixture(t)
+	defer f.finish(t)
+
+	mtime, err := time.Parse(time.RFC3339, "2006-01-02T15:04:05-07:00")
+	assert.NoError(t, err)
+
+	ensureDirExists(sourcePath)
+	ensureDirExists(destPath)
+	fileCreateEmpty(filepath.Join(sourcePath, "sourcefile.txt"))
+	fileCreateEmpty(filepath.Join(destPath, "destfile.txt"))
+
+	// Change the atime/mtime of the directory after creating the files, otherwise
+	// it'll reset to "now".
+	if err := os.Chtimes(destPath, mtime, mtime); err != nil {
+		t.Fatalf("changing dir time: %v", err)
+	}
+
+	// This cannot be a hard-coded string, as the test would fail in other timezones.
+	backupDir := destPath + "-2006-01-02_" + mtime.Local().Format("150405")
+
+	// Just a sanity check.
+	ts, err := timestampedPath(destPath)
+	assert.NoError(t, err)
+	if !assert.Equal(t, backupDir, ts, "the test's sanity check failed") {
+		t.FailNow()
+	}
+
+	f.mocks.listener.EXPECT().LogProduced(gomock.Any(), taskID,
+		fmt.Sprintf("move-directory: moving \"render/output/here\" to %q", backupDir))
+	f.mocks.listener.EXPECT().LogProduced(gomock.Any(), taskID,
+		"move-directory: moving \"render/output/here__intermediate\" to \"render/output/here\"")
+
+	assert.NoError(t, f.run())
+
+	assert.NoDirExists(t, sourcePath)
+	assert.DirExists(t, destPath)
+	assert.DirExists(t, backupDir, "old dest dir should have been moved to new location")
+	assert.NoFileExists(t, filepath.Join(sourcePath, "sourcefile.txt"))
+	assert.FileExists(t, filepath.Join(destPath, "sourcefile.txt"))
+	assert.FileExists(t, filepath.Join(backupDir, "destfile.txt"))
+}
+
+func TestCmdMoveDirectoryExistingDestAndBackup(t *testing.T) {
+	f := newCmdMoveDirectoryFixture(t)
+	defer f.finish(t)
+
+	mtime, err := time.Parse(time.RFC3339, "2006-01-02T15:04:05-07:00")
+	assert.NoError(t, err)
+
+	ensureDirExists(sourcePath)
+	ensureDirExists(destPath)
+	fileCreateEmpty(filepath.Join(sourcePath, "sourcefile.txt"))
+	fileCreateEmpty(filepath.Join(destPath, "destfile.txt"))
+
+	// This cannot be a hard-coded string, as the test would fail in other timezones.
+	backupDir := destPath + "-2006-01-02_" + mtime.Local().Format("150405")
+	ensureDirExists(backupDir)
+	ensureDirExists(backupDir + "-046")
+	fileCreateEmpty(filepath.Join(backupDir, "backupfile.txt"))
+
+	// uniqueDir is where 'dest' will end up, because 'backupDir' already existed beforehand.
+	uniqueDir := backupDir + "-047"
+
+	// Change the atime/mtime of the directory after creating the files, otherwise
+	// it'll reset to "now".
+	if err := os.Chtimes(destPath, mtime, mtime); err != nil {
+		t.Fatalf("changing dir time: %v", err)
+	}
+
+	f.mocks.listener.EXPECT().LogProduced(gomock.Any(), taskID,
+		fmt.Sprintf("move-directory: moving \"render/output/here\" to %q", uniqueDir))
+	f.mocks.listener.EXPECT().LogProduced(gomock.Any(), taskID,
+		"move-directory: moving \"render/output/here__intermediate\" to \"render/output/here\"")
+
+	assert.NoError(t, f.run())
+
+	assert.NoDirExists(t, sourcePath)
+	assert.DirExists(t, destPath)
+	assert.DirExists(t, backupDir, "the backup directory should not have been removed")
+	assert.DirExists(t, uniqueDir, "old dest dir should have been moved to new unique location")
+
+	assert.NoFileExists(t, filepath.Join(sourcePath, "sourcefile.txt"))
+	assert.FileExists(t, filepath.Join(destPath, "sourcefile.txt"))
+	assert.FileExists(t, filepath.Join(backupDir, "backupfile.txt"), "the original backup directory should not have been touched")
+	assert.FileExists(t, filepath.Join(uniqueDir, "destfile.txt"), "the dest dir should have been moved to a unique dir")
+}
+
+func TestTimestampedPathFile(t *testing.T) {
+	f := newCmdMoveDirectoryFixture(t)
+	defer f.finish(t)
+
+	mtime, err := time.Parse(time.RFC3339, "2006-01-02T15:04:05-07:00")
+	assert.NoError(t, err)
+
+	fileCreateEmpty("somefile.txt")
+	os.Chtimes("somefile.txt", mtime, mtime)
+
+	newpath, err := timestampedPath("somefile.txt")
+
+	// This cannot be a hard-coded string, as the test would fail in other timezones.
+	expect := fmt.Sprintf("somefile.txt-2006-01-02_%s", mtime.Local().Format("150405"))
+
+	assert.NoError(t, err)
+	assert.Equal(t, expect, newpath)
+}
+
+func TestTimestampedPathDir(t *testing.T) {
+	f := newCmdMoveDirectoryFixture(t)
+	defer f.finish(t)
+
+	mtime, err := time.Parse(time.RFC3339, "2006-01-02T15:04:05-07:00")
+	assert.NoError(t, err)
+
+	os.Mkdir("somedir", os.ModePerm)
+	os.Chtimes("somedir", mtime, mtime)
+
+	newpath, err := timestampedPath("somedir")
+
+	// This cannot be a hard-coded string, as the test would fail in other timezones.
+	expect := fmt.Sprintf("somedir-2006-01-02_%s", mtime.Local().Format("150405"))
+
+	assert.NoError(t, err)
+	assert.Equal(t, expect, newpath)
+}
+
+func TestUniquePath(t *testing.T) {
+	f := newCmdMoveDirectoryFixture(t)
+	defer f.finish(t)
+
+	fileCreateEmpty("thefile.txt")
+	fileCreateEmpty("thefile.txt-1")
+	fileCreateEmpty("thefile.txt-003")
+	fileCreateEmpty("thefile.txt-46")
+
+	newpath, err := uniquePath("thefile.txt")
+	assert.NoError(t, err)
+	assert.Equal(t, "thefile.txt-047", newpath)
+
+	// Test with existing suffix longer than 3 digits.
+	fileCreateEmpty("thefile.txt-10327")
+	newpath, err = uniquePath("thefile.txt")
+	assert.NoError(t, err)
+	assert.Equal(t, "thefile.txt-10328", newpath)
+}
+
+func newCmdMoveDirectoryFixture(t *testing.T) cmdMoveDirFixture {
+	mockCtrl := gomock.NewController(t)
+	ce, mocks := testCommandExecutor(t, mockCtrl)
+
+	temppath, err := os.MkdirTemp("", "test-move-directory")
+	if err != nil {
+		t.Fatalf("unable to create temp dir: %v", err)
+	}
+
+	cwd, err := os.Getwd()
+	if err != nil {
+		t.Fatalf("getcw: %v", err)
+	}
+
+	if err := os.Chdir(temppath); err != nil {
+		t.Fatalf("chdir(%s): %v", temppath, err)
+	}
+
+	return cmdMoveDirFixture{
+		mockCtrl: mockCtrl,
+		ce:       ce,
+		mocks:    mocks,
+		ctx:      context.Background(),
+		temppath: temppath,
+		cwd:      cwd,
+	}
+}
+
+func (f cmdMoveDirFixture) finish(t *testing.T) {
+	if err := os.Chdir(f.cwd); err != nil {
+		t.Fatalf("chdir(%s): %v", f.cwd, err)
+	}
+
+	os.RemoveAll(f.temppath)
+	f.mockCtrl.Finish()
+}
+
+func (f cmdMoveDirFixture) run() error {
+	cmd := api.Command{
+		Name: "move-directory",
+		Parameters: map[string]interface{}{
+			"src":  sourcePath,
+			"dest": destPath,
+		},
+	}
+	return f.ce.Run(f.ctx, taskID, cmd)
+}
+
+func ensureDirExists(dirpath string) {
+	if err := os.MkdirAll(dirpath, fs.ModePerm); err != nil {
+		panic(fmt.Sprintf("unable to create dir %s: %v", dirpath, err))
+	}
+}
+
+func fileCreateEmpty(filename string) {
+	file, err := os.OpenFile(filename, os.O_CREATE|os.O_APPEND|os.O_RDONLY, 0666)
+
+	if err != nil {
+		panic(err.Error())
+	}
+	file.Close()
+}