From 27602174aedf03cde43f0716dd2d441400655453 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= <sybren@blender.org>
Date: Thu, 21 Jul 2022 16:28:38 +0200
Subject: [PATCH] Variable replacement: fix issue replacing vars in nested
 lists

An array-of-strings in Go can become an array-of-`interface{}` when
converted to JSON and back again. Such cases are now handled properly.
---
 internal/manager/api_impl/varrepl.go      | 13 +++++++++
 internal/manager/api_impl/varrepl_test.go | 34 +++++++++++++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/internal/manager/api_impl/varrepl.go b/internal/manager/api_impl/varrepl.go
index d1d192fa..6c74118d 100644
--- a/internal/manager/api_impl/varrepl.go
+++ b/internal/manager/api_impl/varrepl.go
@@ -30,6 +30,19 @@ func replaceTaskVariables(replacer VariableReplacer, task api.AssignedTask, work
 					replaced[idx] = repl(v[idx])
 				}
 				task.Commands[cmdIndex].Parameters[key] = replaced
+
+			case []interface{}:
+				replaced := make([]interface{}, len(v))
+				for idx := range v {
+					switch itemValue := v[idx].(type) {
+					case string:
+						replaced[idx] = repl(itemValue)
+					default:
+						replaced[idx] = itemValue
+					}
+				}
+				task.Commands[cmdIndex].Parameters[key] = replaced
+
 			default:
 				continue
 			}
diff --git a/internal/manager/api_impl/varrepl_test.go b/internal/manager/api_impl/varrepl_test.go
index ce2023af..df8559d1 100644
--- a/internal/manager/api_impl/varrepl_test.go
+++ b/internal/manager/api_impl/varrepl_test.go
@@ -3,6 +3,7 @@ package api_impl
 // SPDX-License-Identifier: GPL-3.0-or-later
 
 import (
+	"encoding/json"
 	"runtime"
 	"testing"
 
@@ -64,6 +65,21 @@ func TestReplaceVariables(t *testing.T) {
 	assert.Equal(t, 3, replacedTask.Commands[1].Parameters["{blender}"])
 }
 
+func TestReplaceVariablesInterfaceArrays(t *testing.T) {
+	worker := persistence.Worker{Platform: "linux"}
+	conf := config.GetTestConfig()
+
+	task := jsonWash(varreplTestTask())
+	replacedTask := replaceTaskVariables(&conf, task, worker)
+
+	// Due to the conversion via JSON, arrays of strings are now arrays of
+	// interface{} and still need to be handled properly.
+	assert.Equal(t,
+		[]interface{}{"--render-out", "/shared/flamenco/render/long/sybren/blender-cloud-addon/flamenco-test__intermediate/render-smpl-0001-0084-frm-######"},
+		replacedTask.Commands[2].Parameters["args"],
+	)
+}
+
 func TestReplacePathsWindows(t *testing.T) {
 	worker := persistence.Worker{Platform: "windows"}
 	task := varreplTestTask()
@@ -137,3 +153,21 @@ func TestReplaceJobsVariable(t *testing.T) {
 		assert.Equal(t, expectPath, replacedTask.Commands[2].Parameters["filepath"])
 	}
 }
+
+// jsonWash converts the given value to JSON and back.
+// This makes sure the types are as closed to what the API will handle as
+// possible, making the difference between "array of strings" and "array of
+// interface{}s that happen to be strings".
+func jsonWash[T any](value T) T {
+	bytes, err := json.Marshal(value)
+	if err != nil {
+		panic(err)
+	}
+	var jsonWashedValue T
+	err = json.Unmarshal(bytes, &jsonWashedValue)
+	if err != nil {
+		panic(err)
+	}
+
+	return jsonWashedValue
+}