From ffb282758982dfb302bc3b4842906b4d331cab3e Mon Sep 17 00:00:00 2001 From: Jared O'Connell Date: Mon, 5 Feb 2024 18:33:09 -0500 Subject: [PATCH 01/29] Progress towards adding function expressions to the engine --- internal/functions/functions.go | 268 ++++++++++++++++++++++++++++++++ internal/infer/infer.go | 2 +- workflow/executor.go | 13 +- workflow/workflow.go | 2 +- 4 files changed, 281 insertions(+), 4 deletions(-) create mode 100644 internal/functions/functions.go diff --git a/internal/functions/functions.go b/internal/functions/functions.go new file mode 100644 index 00000000..6df96f8c --- /dev/null +++ b/internal/functions/functions.go @@ -0,0 +1,268 @@ +package functions + +import ( + "go.flow.arcalot.io/pluginsdk/schema" + "math" + "regexp" + "strconv" +) + +func getFunctions() map[string]schema.Function { + // Getters + intToFloatFunction := getIntToFloatFunction() + floatToIntFunction := getFloatToIntFunction() + intToStringFunction := getIntToStringFunction() + floatToStringFunction := getFloatToStringFunction() + booleanToStringFunction := getBooleanToStringFunction() + ceilFunction := getCeilFunction() + floorFunction := getFloorFunction() + roundFunction := getRoundFunction() + + // Combine in a map + allFunctions := map[string]schema.Function{ + intToFloatFunction.ID(): intToFloatFunction, + floatToIntFunction.ID(): floatToIntFunction, + intToStringFunction.ID(): intToStringFunction, + floatToStringFunction.ID(): floatToStringFunction, + booleanToStringFunction.ID(): booleanToStringFunction, + ceilFunction.ID(): ceilFunction, + floorFunction.ID(): floorFunction, + roundFunction.ID(): roundFunction, + } + + return allFunctions +} + +func getIntToFloatFunction() schema.Function { + funcSchema, err := schema.NewCallableFunction( + "intToFloat", + []schema.Type{schema.NewIntSchema(nil, nil, nil)}, + schema.NewFloatSchema(nil, nil, nil), + false, + schema.NewDisplayValue( + schema.PointerTo("intToFloat"), + schema.PointerTo("Converts an integer type into a floating point type."), + nil, + ), + func(a int64) float64 { + return float64(a) + }, + ) + if err != nil { + panic(err) + } + return funcSchema +} + +func getFloatToIntFunction() schema.Function { + funcSchema, err := schema.NewCallableFunction( + "floatToInt", + []schema.Type{schema.NewFloatSchema(nil, nil, nil)}, + schema.NewIntSchema(nil, nil, nil), + false, + schema.NewDisplayValue( + schema.PointerTo("floatToInt"), + schema.PointerTo( + "Converts a float type into an integer type by down-casting. "+ + "The value loses any data after the decimal point.\n"+ + "For example, `5.5` becomes `5`", + ), + nil, + ), + func(a float64) int64 { + return int64(a) + }, + ) + if err != nil { + panic(err) + } + return funcSchema +} + +func getIntToStringFunction() schema.Function { + funcSchema, err := schema.NewCallableFunction( + "intToString", + []schema.Type{schema.NewIntSchema(nil, nil, nil)}, + schema.NewStringSchema(nil, nil, regexp.MustCompile(`^(?:0|[1-9]\d*)$`)), + false, + schema.NewDisplayValue( + schema.PointerTo("intToString"), + schema.PointerTo( + "Converts an integer to a string whose characters represent that integer in base-10.\n"+ + "For example, an input of `55` will output \"55\"", + ), + nil, + ), + func(a int64) string { + return strconv.FormatInt(a, 10) + }, + ) + if err != nil { + panic(err) + } + return funcSchema +} + +func getFloatToStringFunction() schema.Function { + funcSchema, err := schema.NewCallableFunction( + "floatToString", + []schema.Type{schema.NewFloatSchema(nil, nil, nil)}, + schema.NewStringSchema(nil, nil, regexp.MustCompile(`^\d+\.\d*$`)), + false, + schema.NewDisplayValue( + schema.PointerTo("floatToString"), + schema.PointerTo( + "Converts a floating point number to a string whose characters"+ + "represent that number in base-10 as as simple decimal.\n"+ + "For example, an input of `5000.5` will output \"5000.5\"", + ), + nil, + ), + func(a float64) string { + return strconv.FormatFloat(a, 'f', -1, 64) + }, + ) + if err != nil { + panic(err) + } + return funcSchema +} + +// TODO: Webb can add a function called floatToStringAdvanced that allows other float formats + +func getBooleanToStringFunction() schema.Function { + funcSchema, err := schema.NewCallableFunction( + "booleanToString", + []schema.Type{schema.NewBoolSchema()}, + schema.NewStringSchema(nil, nil, regexp.MustCompile(`^true|false$`)), + false, + schema.NewDisplayValue( + schema.PointerTo("booleanToString"), + schema.PointerTo( + "Represents `true` as \"true\", and `false` as \"false\".", + ), + nil, + ), + func(a bool) string { + return strconv.FormatBool(a) + }, + ) + if err != nil { + panic(err) + } + return funcSchema +} + +func getStringToIntFunction() schema.Function { + funcSchema, err := schema.NewCallableFunction( + "", + []schema.Type{schema.NewStringSchema(nil, nil, nil)}, + schema.NewStringSchema(nil, nil, regexp.MustCompile(``)), + false, + schema.NewDisplayValue( + schema.PointerTo(""), + schema.PointerTo( + "", + ), + nil, + ), + func(a string) { + return + }, + ) + if err != nil { + panic(err) + } + return funcSchema +} + +// TODO: +// string to float +// string to bool + +func getCeilFunction() schema.Function { + funcSchema, err := schema.NewCallableFunction( + "ceil", + []schema.Type{schema.NewFloatSchema(nil, nil, nil)}, + schema.NewFloatSchema(nil, nil, nil), + false, + schema.NewDisplayValue( + schema.PointerTo("ceil"), + schema.PointerTo( + // Description based on documentation for math.Ceil + "Ceil returns the least integer value greater than or equal to x.\n"+ + "For example `ceil(1.5)` outputs `2.0`, and `ceil(-1.5)` outputs `-1.0`"+ + "Special cases are:\n"+ + "Ceil(±0) = ±0\n"+ + "Ceil(±Inf) = ±Inf\n"+ + "Ceil(NaN) = Na", + ), + nil, + ), + func(a float64) float64 { + return math.Ceil(a) + }, + ) + if err != nil { + panic(err) + } + return funcSchema +} + +func getFloorFunction() schema.Function { + funcSchema, err := schema.NewCallableFunction( + "floor", + []schema.Type{schema.NewFloatSchema(nil, nil, nil)}, + schema.NewFloatSchema(nil, nil, nil), + false, + schema.NewDisplayValue( + schema.PointerTo("floor"), + schema.PointerTo( + // Description based on documentation for math.Floor + "Floor returns the greatest integer value less than or equal to x.\n"+ + "For example `floor(1.5)` outputs `1.0`, and `floor(-1.5)` outputs `-2.0`"+ + "Special cases are:\n"+ + "Ceil(±0) = ±0\n"+ + "Ceil(±Inf) = ±Inf\n"+ + "Ceil(NaN) = Na", + ), + nil, + ), + func(a float64) float64 { + return math.Floor(a) + }, + ) + if err != nil { + panic(err) + } + return funcSchema +} + +func getRoundFunction() schema.Function { + funcSchema, err := schema.NewCallableFunction( + "round", + []schema.Type{schema.NewFloatSchema(nil, nil, nil)}, + schema.NewFloatSchema(nil, nil, nil), + false, + schema.NewDisplayValue( + schema.PointerTo("round"), + schema.PointerTo( + // Description based on documentation for math.Round + "Round returns the nearest integer, rounding half away from zero.\n"+ + "For example `round(1.5)` outputs `2.0`, and `round(-1.5)` outputs `-2.0`"+ + "Special cases are:\n"+ + "Ceil(±0) = ±0\n"+ + "Ceil(±Inf) = ±Inf\n"+ + "Ceil(NaN) = Na", + ), + nil, + ), + func(a float64) float64 { + return math.Round(a) + }, + ) + if err != nil { + panic(err) + } + return funcSchema +} diff --git a/internal/infer/infer.go b/internal/infer/infer.go index 56241b56..1bf917f0 100644 --- a/internal/infer/infer.go +++ b/internal/infer/infer.go @@ -53,7 +53,7 @@ func Scope(data any, internalDataModel *schema.ScopeSchema, workflowContext map[ // Type attempts to infer the data model from the data, possibly evaluating expressions. func Type(data any, internalDataModel *schema.ScopeSchema, workflowContext map[string][]byte) (schema.Type, error) { if expression, ok := data.(expressions.Expression); ok { - expressionType, err := expression.Type(internalDataModel, workflowContext) + expressionType, err := expression.Type(internalDataModel, make(map[string]schema.Function), workflowContext) // TODO if err != nil { return nil, fmt.Errorf("failed to evaluate type of expression %s (%w)", expression.String(), err) } diff --git a/workflow/executor.go b/workflow/executor.go index e3b0d002..44e76815 100644 --- a/workflow/executor.go +++ b/workflow/executor.go @@ -455,7 +455,7 @@ func (e *executor) createTypeStructure(rootSchema schema.Scope, inputField any, if expr, ok := inputField.(expressions.Expression); ok { // Is expression, so evaluate it. e.logger.Debugf("Evaluating expression %s...", expr.String()) - return expr.Type(rootSchema, workflowContext) + return expr.Type(rootSchema, make(map[string]schema.Function), workflowContext) // TODO } v := reflect.ValueOf(inputField) @@ -792,7 +792,16 @@ func (e *executor) prepareDependencies( //nolint:gocognit,gocyclo case reflect.Struct: switch s := stepData.(type) { case expressions.Expression: - dependencies, err := s.Dependencies(outputSchema, workflowContext) + // Evaluate the dependencies of the expression on the data structure. + // Include extraneous is false due to this only being needed for connecting to the appropriate node, + // as opposed to the data provided by that node. + pathUnpackRequirements := expressions.UnpackRequirements{ + ExcludeDataRootPaths: false, + ExcludeFunctionRootPaths: true, // We don't need to setup DAG connections for them. + StopAtTerminals: true, // I don't think we need the extra info. We just need the connection. + IncludeKeys: false, + } + dependencies, err := s.Dependencies(outputSchema, make(map[string]schema.Function), workflowContext, pathUnpackRequirements) // TODO if err != nil { return fmt.Errorf( "failed to evaluate dependencies of the expression %s (%w)", diff --git a/workflow/workflow.go b/workflow/workflow.go index ad7b65bf..6d1372b5 100644 --- a/workflow/workflow.go +++ b/workflow/workflow.go @@ -522,7 +522,7 @@ func (l *loopState) checkForDeadlocks(retries int, wg *sync.WaitGroup) { func (l *loopState) resolveExpressions(inputData any, dataModel any) (any, error) { if expr, ok := inputData.(expressions.Expression); ok { l.logger.Debugf("Evaluating expression %s...", expr.String()) - return expr.Evaluate(dataModel, l.workflowContext) + return expr.Evaluate(dataModel, make(map[string]schema.CallableFunction), l.workflowContext) // TODO } v := reflect.ValueOf(inputData) switch v.Kind() { From 3b498cffc1bcdecbcbd4f070260b58c0b0cf02ff Mon Sep 17 00:00:00 2001 From: Jared O'Connell Date: Wed, 7 Feb 2024 16:12:07 -0500 Subject: [PATCH 02/29] Added function support --- engine.go | 3 +- go.mod | 2 +- go.sum | 4 +- .../functions.go | 226 ++++++++- internal/builtinfunctions/functions_test.go | 471 ++++++++++++++++++ .../step/foreach/provider_example_test.go | 4 +- steps.go | 3 +- workflow/executor.go | 26 +- workflow/executor_example_test.go | 3 +- workflow/executor_test.go | 7 +- workflow/workflow.go | 5 +- workflow/workflow_test.go | 8 +- 12 files changed, 718 insertions(+), 44 deletions(-) rename internal/{functions => builtinfunctions}/functions.go (50%) create mode 100644 internal/builtinfunctions/functions_test.go diff --git a/engine.go b/engine.go index 61ed73ae..5cefe488 100644 --- a/engine.go +++ b/engine.go @@ -6,6 +6,7 @@ import ( "fmt" log "go.arcalot.io/log/v2" "go.flow.arcalot.io/engine/config" + "go.flow.arcalot.io/engine/internal/builtinfunctions" "go.flow.arcalot.io/engine/internal/step" "go.flow.arcalot.io/engine/internal/yaml" "go.flow.arcalot.io/engine/loadfile" @@ -112,7 +113,7 @@ func (w workflowEngine) Parse( } wf.Version = v - executor, err := workflow.NewExecutor(w.logger, w.config, w.stepRegistry) + executor, err := workflow.NewExecutor(w.logger, w.config, w.stepRegistry, builtinfunctions.GetFunctions()) if err != nil { return nil, err } diff --git a/go.mod b/go.mod index 5208bce5..ff86c8a4 100644 --- a/go.mod +++ b/go.mod @@ -11,7 +11,7 @@ require ( go.arcalot.io/log/v2 v2.1.0 go.flow.arcalot.io/deployer v0.5.0 go.flow.arcalot.io/dockerdeployer v0.6.1 - go.flow.arcalot.io/expressions v0.2.1 + go.flow.arcalot.io/expressions v0.3.0 go.flow.arcalot.io/kubernetesdeployer v0.9.1 go.flow.arcalot.io/pluginsdk v0.8.0 go.flow.arcalot.io/podmandeployer v0.8.1 diff --git a/go.sum b/go.sum index 9f8e9611..ca089b67 100644 --- a/go.sum +++ b/go.sum @@ -135,8 +135,8 @@ go.flow.arcalot.io/deployer v0.5.0 h1:yXYogvL3shNBEEoTx9U9CNbfxuf8777uAH5Vn3hv1Y go.flow.arcalot.io/deployer v0.5.0/go.mod h1:whj8wOUursCnfZCt1a7eY5hU3EyOcUG48vM4NeAe5N8= go.flow.arcalot.io/dockerdeployer v0.6.1 h1:oRhxXEeOHmXDQVgtYa95tUpw9qc/M//pbeLjdDYMUxc= go.flow.arcalot.io/dockerdeployer v0.6.1/go.mod h1:Y5Xfg/Fedw/y4LTV0eiJqnsex1mvTbhtP06LCtFkJyo= -go.flow.arcalot.io/expressions v0.2.1 h1:TAAbDrgJJLpmgA5ASyP/KzrXWtpEaQ8JsCPHgpe5kLw= -go.flow.arcalot.io/expressions v0.2.1/go.mod h1:Vw1ScNu4Uyw1/l87LAH8jxe0DyRWwMh+rlfB/BPYDOU= +go.flow.arcalot.io/expressions v0.3.0 h1:PhHiAyRcPLWp3dycZGoY1X3al7yanB9xoxJqyigzxts= +go.flow.arcalot.io/expressions v0.3.0/go.mod h1:WfbAaPUfvAnarly6GO87Cyi6lmWmttMaVS1rR9YDyrU= go.flow.arcalot.io/kubernetesdeployer v0.9.1 h1:AGnJFazehAENXxGMCF0Uc7aG9F0LpvuhoyQFu8deJG0= go.flow.arcalot.io/kubernetesdeployer v0.9.1/go.mod h1:yvxT3VwmyrlIi4422pxl02z4QeU2Gvbjg5aQB17Ye4s= go.flow.arcalot.io/pluginsdk v0.8.0 h1:cShsshrR17ZFLcbgi3aZvqexLttcp3JISFNqPUPuDvA= diff --git a/internal/functions/functions.go b/internal/builtinfunctions/functions.go similarity index 50% rename from internal/functions/functions.go rename to internal/builtinfunctions/functions.go index 6df96f8c..68ae7193 100644 --- a/internal/functions/functions.go +++ b/internal/builtinfunctions/functions.go @@ -1,39 +1,57 @@ -package functions +package builtinfunctions import ( "go.flow.arcalot.io/pluginsdk/schema" "math" "regexp" "strconv" + "strings" ) -func getFunctions() map[string]schema.Function { - // Getters +func GetFunctions() map[string]schema.CallableFunction { + // Simple conversions intToFloatFunction := getIntToFloatFunction() floatToIntFunction := getFloatToIntFunction() intToStringFunction := getIntToStringFunction() floatToStringFunction := getFloatToStringFunction() booleanToStringFunction := getBooleanToStringFunction() + // Parsers, that could fail + stringToIntFunction := getStringToIntFunction() + stringToFloatFunction := getStringToFloatFunction() + stringToBoolFunction := getStringToBoolFunction() + // Math helper functions ceilFunction := getCeilFunction() floorFunction := getFloorFunction() roundFunction := getRoundFunction() + absFunction := getAbsFunction() + // String helper functions + toLowerFunction := getToLowerFunction() + toUpperFunction := getToUpperFunction() + splitStringFunction := getSplitStringFunction() // Combine in a map - allFunctions := map[string]schema.Function{ + allFunctions := map[string]schema.CallableFunction{ intToFloatFunction.ID(): intToFloatFunction, floatToIntFunction.ID(): floatToIntFunction, intToStringFunction.ID(): intToStringFunction, floatToStringFunction.ID(): floatToStringFunction, booleanToStringFunction.ID(): booleanToStringFunction, + stringToIntFunction.ID(): stringToIntFunction, + stringToFloatFunction.ID(): stringToFloatFunction, + stringToBoolFunction.ID(): stringToBoolFunction, ceilFunction.ID(): ceilFunction, floorFunction.ID(): floorFunction, roundFunction.ID(): roundFunction, + absFunction.ID(): absFunction, + toLowerFunction.ID(): toLowerFunction, + toUpperFunction.ID(): toUpperFunction, + splitStringFunction.ID(): splitStringFunction, } return allFunctions } -func getIntToFloatFunction() schema.Function { +func getIntToFloatFunction() schema.CallableFunction { funcSchema, err := schema.NewCallableFunction( "intToFloat", []schema.Type{schema.NewIntSchema(nil, nil, nil)}, @@ -54,7 +72,7 @@ func getIntToFloatFunction() schema.Function { return funcSchema } -func getFloatToIntFunction() schema.Function { +func getFloatToIntFunction() schema.CallableFunction { funcSchema, err := schema.NewCallableFunction( "floatToInt", []schema.Type{schema.NewFloatSchema(nil, nil, nil)}, @@ -79,7 +97,7 @@ func getFloatToIntFunction() schema.Function { return funcSchema } -func getIntToStringFunction() schema.Function { +func getIntToStringFunction() schema.CallableFunction { funcSchema, err := schema.NewCallableFunction( "intToString", []schema.Type{schema.NewIntSchema(nil, nil, nil)}, @@ -103,7 +121,7 @@ func getIntToStringFunction() schema.Function { return funcSchema } -func getFloatToStringFunction() schema.Function { +func getFloatToStringFunction() schema.CallableFunction { funcSchema, err := schema.NewCallableFunction( "floatToString", []schema.Type{schema.NewFloatSchema(nil, nil, nil)}, @@ -130,14 +148,14 @@ func getFloatToStringFunction() schema.Function { // TODO: Webb can add a function called floatToStringAdvanced that allows other float formats -func getBooleanToStringFunction() schema.Function { +func getBooleanToStringFunction() schema.CallableFunction { funcSchema, err := schema.NewCallableFunction( - "booleanToString", + "boolToString", []schema.Type{schema.NewBoolSchema()}, schema.NewStringSchema(nil, nil, regexp.MustCompile(`^true|false$`)), false, schema.NewDisplayValue( - schema.PointerTo("booleanToString"), + schema.PointerTo("boolToString"), schema.PointerTo( "Represents `true` as \"true\", and `false` as \"false\".", ), @@ -153,21 +171,51 @@ func getBooleanToStringFunction() schema.Function { return funcSchema } -func getStringToIntFunction() schema.Function { +func getStringToIntFunction() schema.CallableFunction { funcSchema, err := schema.NewCallableFunction( - "", + "stringToInt", + []schema.Type{schema.NewStringSchema(nil, nil, regexp.MustCompile(`^-?(?:0|[1-9]\d*)$`))}, + schema.NewIntSchema(nil, nil, nil), + true, + schema.NewDisplayValue( + schema.PointerTo("stringToInt"), + schema.PointerTo( + "Interprets the string as a base-10 integer. Can fail if the input is not a valid integer.", + ), + nil, + ), + func(s string) (int64, error) { + return strconv.ParseInt(s, 10, 0) + }, + ) + if err != nil { + panic(err) + } + return funcSchema +} + +func getStringToFloatFunction() schema.CallableFunction { + funcSchema, err := schema.NewCallableFunction( + "stringToFloat", []schema.Type{schema.NewStringSchema(nil, nil, nil)}, - schema.NewStringSchema(nil, nil, regexp.MustCompile(``)), - false, + schema.NewFloatSchema(nil, nil, nil), + true, schema.NewDisplayValue( - schema.PointerTo(""), + schema.PointerTo("stringToFloat"), schema.PointerTo( - "", + "Converts the string s to a 64-bit floating-point number\n\n"+ + "Accepts decimal and hexadecimal floating-point numbers\n"+ + "as defined by the Go syntax for floating point literals\n"+ + "https://go.dev/ref/spec#Floating-point_literals.\n"+ + "If s is well-formed and near a valid floating-point number,\n"+ + "ParseFloat returns the nearest floating-point number rounded\n"+ + "using IEEE754 unbiased rounding.\n\n"+ + "Returns NumError when an invalid input is received.", ), nil, ), - func(a string) { - return + func(s string) (float64, error) { + return strconv.ParseFloat(s, 64) }, ) if err != nil { @@ -176,11 +224,38 @@ func getStringToIntFunction() schema.Function { return funcSchema } -// TODO: -// string to float -// string to bool +func getStringToBoolFunction() schema.CallableFunction { + funcSchema, err := schema.NewCallableFunction( + "stringToBool", + []schema.Type{ + schema.NewStringSchema( + nil, + nil, + regexp.MustCompile(`[Tt]rue|TRUE|[Ff]alse|FALSE|[tTfF]|[01]$`), + )}, + schema.NewBoolSchema(), + true, + schema.NewDisplayValue( + schema.PointerTo("stringToBool"), + schema.PointerTo( + "Interprets the input as a boolean.\n"+ + "Accepts `1`, `t`, `T`, `true`, `TRUE`, and `True` for true.\n"+ + "Accepts `0`, 'f', 'F', 'false', 'FALSE', and 'False' for false.\n"+ + "Returns an error if any other value is input.", + ), + nil, + ), + func(s string) (bool, error) { + return strconv.ParseBool(s) + }, + ) + if err != nil { + panic(err) + } + return funcSchema +} -func getCeilFunction() schema.Function { +func getCeilFunction() schema.CallableFunction { funcSchema, err := schema.NewCallableFunction( "ceil", []schema.Type{schema.NewFloatSchema(nil, nil, nil)}, @@ -209,7 +284,7 @@ func getCeilFunction() schema.Function { return funcSchema } -func getFloorFunction() schema.Function { +func getFloorFunction() schema.CallableFunction { funcSchema, err := schema.NewCallableFunction( "floor", []schema.Type{schema.NewFloatSchema(nil, nil, nil)}, @@ -238,7 +313,7 @@ func getFloorFunction() schema.Function { return funcSchema } -func getRoundFunction() schema.Function { +func getRoundFunction() schema.CallableFunction { funcSchema, err := schema.NewCallableFunction( "round", []schema.Type{schema.NewFloatSchema(nil, nil, nil)}, @@ -266,3 +341,104 @@ func getRoundFunction() schema.Function { } return funcSchema } + +func getAbsFunction() schema.CallableFunction { + funcSchema, err := schema.NewCallableFunction( + "abs", + []schema.Type{schema.NewFloatSchema(nil, nil, nil)}, + schema.NewFloatSchema(nil, nil, nil), + false, + schema.NewDisplayValue( + schema.PointerTo("abs"), + schema.PointerTo( + // Description based on documentation for math.Abs + "abs returns the absolute value of x.\n"+ + "Special cases are:\n"+ + "Ceil(±Inf) = +Inf\n"+ + "Ceil(NaN) = Na", + ), + nil, + ), + func(a float64) float64 { + return math.Abs(a) + }, + ) + if err != nil { + panic(err) + } + return funcSchema +} + +func getToLowerFunction() schema.CallableFunction { + funcSchema, err := schema.NewCallableFunction( + "toLower", + []schema.Type{schema.NewStringSchema(nil, nil, nil)}, + schema.NewStringSchema(nil, nil, nil), + false, + schema.NewDisplayValue( + schema.PointerTo("toLower"), + schema.PointerTo( + "Outputs the input with Unicode letters mapped to their lower case.", + ), + nil, + ), + func(s string) string { + return strings.ToLower(s) + }, + ) + if err != nil { + panic(err) + } + return funcSchema +} + +func getToUpperFunction() schema.CallableFunction { + funcSchema, err := schema.NewCallableFunction( + "toUpper", + []schema.Type{schema.NewStringSchema(nil, nil, nil)}, + schema.NewStringSchema(nil, nil, nil), + false, + schema.NewDisplayValue( + schema.PointerTo("toUpper"), + schema.PointerTo( + "Outputs the input with Unicode letters mapped to their upper case.", + ), + nil, + ), + func(s string) string { + return strings.ToUpper(s) + }, + ) + if err != nil { + panic(err) + } + return funcSchema +} + +func getSplitStringFunction() schema.CallableFunction { + funcSchema, err := schema.NewCallableFunction( + "splitString", + []schema.Type{ + schema.NewStringSchema(nil, nil, nil), + schema.NewStringSchema(nil, nil, nil), + }, + schema.NewListSchema(schema.NewStringSchema(nil, nil, nil), nil, nil), + false, + schema.NewDisplayValue( + schema.PointerTo("strSplit"), + schema.PointerTo( + "Splits the given string with the given separator.\n"+ + " Param 1: The string to split.\n"+ + " Param 2: The separator.\n", + ), + nil, + ), + func(source string, separator string) []string { + return strings.Split(source, separator) + }, + ) + if err != nil { + panic(err) + } + return funcSchema +} diff --git a/internal/builtinfunctions/functions_test.go b/internal/builtinfunctions/functions_test.go new file mode 100644 index 00000000..5eedb51f --- /dev/null +++ b/internal/builtinfunctions/functions_test.go @@ -0,0 +1,471 @@ +package builtinfunctions_test + +import ( + "go.flow.arcalot.io/engine/internal/builtinfunctions" + "math" + "reflect" + "testing" +) + +var testData = map[string]struct { + functionName string + rawArguments []any + returnError bool + expectedResult any +}{ + "int-to-float-positive": { + "intToFloat", + []any{int64(1)}, + false, + 1.0, + }, + "int-to-float-negative": { + "intToFloat", + []any{int64(-1)}, + false, + -1.0, + }, + "int-to-float-negative-zero": { + "intToFloat", + []any{int64(-0)}, + false, + -0.0, + }, + "float-to-int-positive": { + "floatToInt", + []any{1.0}, + false, + int64(1), + }, + "float-to-int-negative": { + "floatToInt", + []any{-1.0}, + false, + int64(-1), + }, + "float-to-int-negative-zero": { + "floatToInt", + []any{-0.0}, + false, + int64(-0), + }, + "int-to-string-positive": { + "intToString", + []any{int64(11)}, + false, + "11", + }, + "int-to-string-zero": { + "intToString", + []any{int64(0)}, + false, + "0", + }, + "int-to-string-negative": { + "intToString", + []any{int64(-11)}, + false, + "-11", + }, + "float-to-string-positive-whole": { + "floatToString", + []any{11.0}, + false, + "11", + }, + "float-to-string-negative-whole": { + "floatToString", + []any{-11.0}, + false, + "-11", + }, + "float-to-string-positive-fractional": { + "floatToString", + []any{11.1}, + false, + "11.1", + }, + "float-to-string-negative-fractional": { + "floatToString", + []any{-11.1}, + false, + "-11.1", + }, + "float-to-string-zero": { + "floatToString", + []any{0.0}, + false, + "0", + }, + "float-to-string-nan": { + "floatToString", + []any{math.NaN()}, + false, + "NaN", + }, + "bool-to-string-true": { + "boolToString", + []any{true}, + false, + "true", + }, + "bool-to-string-false": { + "boolToString", + []any{false}, + false, + "false", + }, + "string-to-int-positive": { + "stringToInt", + []any{"11"}, + false, + int64(11), + }, + "string-to-int-negative": { + "stringToInt", + []any{"-11"}, + false, + int64(-11), + }, + "string-to-int-zero": { + "stringToInt", + []any{"0"}, + false, + int64(0), + }, + "string-to-int-invalid-1": { + "stringToInt", + []any{"zero"}, + true, + nil, + }, + "string-to-int-invalid-2": { + "stringToInt", + []any{"1.0"}, + true, + nil, + }, + "string-to-int-invalid-3": { + "stringToInt", + []any{""}, + true, + nil, + }, + "string-to-float-positive": { + "stringToFloat", + []any{"11.1"}, + false, + 11.1, + }, + "string-to-float-negative": { + "stringToFloat", + []any{"-11.1"}, + false, + -11.1, + }, + "string-to-float-zero": { + "stringToFloat", + []any{"0"}, + false, + 0.0, + }, + "string-to-float-scientific-notation": { + "stringToFloat", + []any{"5e+5"}, + false, + 500000.0, + }, + "string-to-float-error-1": { + "stringToFloat", + []any{""}, + true, + nil, + }, + "string-to-float-error-2": { + "stringToFloat", + []any{"ten"}, + true, + nil, + }, + "string-to-bool-1": { + "stringToBool", + []any{"true"}, + false, + true, + }, + "string-to-bool-2": { + "stringToBool", + []any{"True"}, + false, + true, + }, + "string-to-bool-3": { + "stringToBool", + []any{"TRUE"}, + false, + true, + }, + "string-to-bool-4": { + "stringToBool", + []any{"t"}, + false, + true, + }, + "string-to-bool-5": { + "stringToBool", + []any{"T"}, + false, + true, + }, + "string-to-bool-6": { + "stringToBool", + []any{"1"}, + false, + true, + }, + "string-to-bool-7": { + "stringToBool", + []any{"false"}, + false, + false, + }, + "string-to-bool-8": { + "stringToBool", + []any{"False"}, + false, + false, + }, + "string-to-bool-9": { + "stringToBool", + []any{"FALSE"}, + false, + false, + }, + "string-to-bool-10": { + "stringToBool", + []any{"f"}, + false, + false, + }, + "string-to-bool-11": { + "stringToBool", + []any{"F"}, + false, + false, + }, + "string-to-bool-12": { + "stringToBool", + []any{"0"}, + false, + false, + }, + "string-to-bool-error-1": { + "stringToBool", + []any{""}, + true, + nil, + }, + "string-to-bool-error-2": { + "stringToBool", + []any{"abc"}, + true, + nil, + }, + "ceil-1": { + "ceil", + []any{0.0}, + false, + 0.0, + }, + "ceil-2": { + "ceil", + []any{0.1}, + false, + 1.0, + }, + "ceil-3": { + "ceil", + []any{-0.1}, + false, + 0.0, + }, + "floor-1": { + "floor", + []any{0.0}, + false, + 0.0, + }, + "floor-2": { + "floor", + []any{1.1}, + false, + 1.0, + }, + "floor-3": { + "floor", + []any{2.9999}, + false, + 2.0, + }, + "floor-4": { + "floor", + []any{-0.1}, + false, + -1.0, + }, + "round-1": { + "round", + []any{0.0}, + false, + 0.0, + }, + "round-2": { + "round", + []any{0.1}, + false, + 0.0, + }, + "round-3": { + "round", + []any{0.4}, + false, + 0.0, + }, + "round-4": { + "round", + []any{0.5}, + false, + 1.0, + }, + "round-5": { + "round", + []any{0.9}, + false, + 1.0, + }, + "round-6": { + "round", + []any{-0.9}, + false, + -1.0, + }, + "round-7": { + "round", + []any{-0.5}, + false, + -1.0, + }, + "round-8": { + "round", + []any{-0.01}, + false, + 0.0, + }, + "abs-1": { + "abs", + []any{-0.0}, + false, + 0.0, + }, + "abs-2": { + "abs", + []any{-1.0}, + false, + 1.0, + }, + "abs-3": { + "abs", + []any{1.0}, + false, + 1.0, + }, + "to-lower-1": { + "toLower", + []any{""}, + false, + "", + }, + "to-lower-2": { + "toLower", + []any{"abc"}, + false, + "abc", + }, + "to-lower-3": { + "toLower", + []any{"ABC"}, + false, + "abc", + }, + "to-upper-1": { + "toUpper", + []any{""}, + false, + "", + }, + "to-upper-2": { + "toUpper", + []any{"abc"}, + false, + "ABC", + }, + "to-upper-3": { + "toUpper", + []any{"ABC"}, + false, + "ABC", + }, + "split-string-1": { + "splitString", + []any{"", ","}, + false, + make([]string, 1), // The size appears to matter with DeepEquals + }, + "split-string-2": { + "splitString", + []any{"abc", ","}, + false, + []string{"abc"}, + }, + "split-string-3": { + "splitString", + []any{"a,b", ","}, + false, + []string{"a", "b"}, + }, + "split-string-4": { + "splitString", + []any{"a,b,c,d", ","}, + false, + []string{"a", "b", "c", "d"}, + }, +} + +func TestFunctionsBulk(t *testing.T) { + allFunctions := builtinfunctions.GetFunctions() + for name, tc := range testData { + testCase := tc + t.Run(name, func(t *testing.T) { + functionToTest, funcFound := allFunctions[testCase.functionName] + if !funcFound { + t.Fatalf("Function %q not found", testCase.functionName) + } + + output, err := functionToTest.Call(testCase.rawArguments) + + if testCase.returnError { + if err == nil { + t.Fatalf("expected error in test case %q; error returned nil", name) + } + return + } + if err != nil { + t.Fatalf("unexpected error in test case %q (%s)", name, err.Error()) + } + + if !reflect.DeepEqual(testCase.expectedResult, output) { + t.Fatalf("mismatch for test %q, expected: %v, got: %v", name, testCase.expectedResult, output) + } + }) + } +} diff --git a/internal/step/foreach/provider_example_test.go b/internal/step/foreach/provider_example_test.go index 6bc248a8..e81ef001 100644 --- a/internal/step/foreach/provider_example_test.go +++ b/internal/step/foreach/provider_example_test.go @@ -3,6 +3,7 @@ package foreach_test import ( "context" "fmt" + "go.flow.arcalot.io/engine/internal/builtinfunctions" "go.arcalot.io/lang" "go.arcalot.io/log/v2" @@ -86,6 +87,7 @@ func ExampleNew() { logger, cfg, stepRegistry, + builtinfunctions.GetFunctions(), )) wf := lang.Must2(workflow.NewYAMLConverter(stepRegistry).FromYAML([]byte(mainWorkflow))) preparedWorkflow := lang.Must2(executor.Prepare(wf, map[string][]byte{ @@ -131,5 +133,5 @@ func (f *workflowFactory) createWorkflow(logger log.Logger) (workflow.Executor, if stepR == nil { return nil, fmt.Errorf("YAML converter not available yet, please call the factory function after the engine has initialized") } - return workflow.NewExecutor(logger, f.config, stepR) + return workflow.NewExecutor(logger, f.config, stepR, builtinfunctions.GetFunctions()) } diff --git a/steps.go b/steps.go index 9840f752..e2b53217 100644 --- a/steps.go +++ b/steps.go @@ -5,6 +5,7 @@ import ( "go.arcalot.io/log/v2" deployerRegistry "go.flow.arcalot.io/deployer/registry" "go.flow.arcalot.io/engine/config" + "go.flow.arcalot.io/engine/internal/builtinfunctions" "go.flow.arcalot.io/engine/internal/step" "go.flow.arcalot.io/engine/internal/step/foreach" "go.flow.arcalot.io/engine/internal/step/plugin" @@ -57,5 +58,5 @@ func (f *workflowFactory) createWorkflow(logger log.Logger) (workflow.Executor, if stepR == nil { return nil, fmt.Errorf("YAML converter not available yet, please call the factory function after the engine has initialized") } - return workflow.NewExecutor(logger, f.config, stepR) + return workflow.NewExecutor(logger, f.config, stepR, builtinfunctions.GetFunctions()) } diff --git a/workflow/executor.go b/workflow/executor.go index 44e76815..b560ab3f 100644 --- a/workflow/executor.go +++ b/workflow/executor.go @@ -22,6 +22,7 @@ func NewExecutor( logger log.Logger, config *config.Config, stepRegistry step.Registry, + callableFunctions map[string]schema.CallableFunction, ) (Executor, error) { if logger == nil { return nil, fmt.Errorf("bug: no logger passed to NewExecutor") @@ -29,10 +30,16 @@ func NewExecutor( if stepRegistry == nil { return nil, fmt.Errorf("bug: no step registry passed to NewExecutor") } + functionSchemas := make(map[string]schema.Function, len(callableFunctions)) + for k, v := range callableFunctions { + functionSchemas[k] = v + } return &executor{ - logger: logger.WithLabel("source", "executor"), - stepRegistry: stepRegistry, - config: config, + logger: logger.WithLabel("source", "executor"), + stepRegistry: stepRegistry, + config: config, + callableFunctions: callableFunctions, + callableFunctionSchemas: functionSchemas, }, nil } @@ -67,9 +74,11 @@ type ExecutableWorkflow interface { } type executor struct { - logger log.Logger - config *config.Config - stepRegistry step.Registry + logger log.Logger + config *config.Config + stepRegistry step.Registry + callableFunctionSchemas map[string]schema.Function + callableFunctions map[string]schema.CallableFunction } // Prepare goes through all workflow steps and constructs their schema and input data. @@ -184,6 +193,7 @@ func (e *executor) Prepare(workflow *Workflow, workflowContext map[string][]byte return &executableWorkflow{ logger: e.logger, config: e.config, + callableFunctions: e.callableFunctions, dag: dag, input: typedInput, stepRunData: stepRunData, @@ -455,7 +465,7 @@ func (e *executor) createTypeStructure(rootSchema schema.Scope, inputField any, if expr, ok := inputField.(expressions.Expression); ok { // Is expression, so evaluate it. e.logger.Debugf("Evaluating expression %s...", expr.String()) - return expr.Type(rootSchema, make(map[string]schema.Function), workflowContext) // TODO + return expr.Type(rootSchema, e.callableFunctionSchemas, workflowContext) } v := reflect.ValueOf(inputField) @@ -801,7 +811,7 @@ func (e *executor) prepareDependencies( //nolint:gocognit,gocyclo StopAtTerminals: true, // I don't think we need the extra info. We just need the connection. IncludeKeys: false, } - dependencies, err := s.Dependencies(outputSchema, make(map[string]schema.Function), workflowContext, pathUnpackRequirements) // TODO + dependencies, err := s.Dependencies(outputSchema, e.callableFunctionSchemas, workflowContext, pathUnpackRequirements) if err != nil { return fmt.Errorf( "failed to evaluate dependencies of the expression %s (%w)", diff --git a/workflow/executor_example_test.go b/workflow/executor_example_test.go index b68ab747..e4d6dfe2 100644 --- a/workflow/executor_example_test.go +++ b/workflow/executor_example_test.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "go.flow.arcalot.io/engine/config" + "go.flow.arcalot.io/engine/internal/builtinfunctions" "go.arcalot.io/lang" "go.arcalot.io/log/v2" @@ -38,7 +39,7 @@ func ExampleExecutor() { dummy.New(), )) - executor, err := workflow.NewExecutor(logger, &config.Config{}, stepRegistry) + executor, err := workflow.NewExecutor(logger, &config.Config{}, stepRegistry, builtinfunctions.GetFunctions()) if err != nil { panic(err) } diff --git a/workflow/executor_test.go b/workflow/executor_test.go index 8fe5299f..b8b4053f 100644 --- a/workflow/executor_test.go +++ b/workflow/executor_test.go @@ -4,15 +4,16 @@ import ( "context" "fmt" "go.arcalot.io/assert" + "go.arcalot.io/lang" "go.flow.arcalot.io/deployer" deployerregistry "go.flow.arcalot.io/deployer/registry" "go.flow.arcalot.io/engine/config" + "go.flow.arcalot.io/engine/internal/builtinfunctions" "go.flow.arcalot.io/engine/internal/step" "go.flow.arcalot.io/engine/internal/step/plugin" testimpl "go.flow.arcalot.io/testdeployer" "testing" - "go.arcalot.io/lang" "go.arcalot.io/log/v2" "go.flow.arcalot.io/engine/internal/step/dummy" stepregistry "go.flow.arcalot.io/engine/internal/step/registry" @@ -34,6 +35,7 @@ func getTestImplPreparedWorkflow(t *testing.T, workflowDefinition string) (workf logger, cfg, stepRegistry, + builtinfunctions.GetFunctions(), )) wf := assert.NoErrorR[*workflow.Workflow](t)(workflow.NewYAMLConverter(stepRegistry).FromYAML([]byte(workflowDefinition))) return executor.Prepare(wf, map[string][]byte{}) @@ -49,6 +51,7 @@ func getDummyDeployerPreparedWorkflow(t *testing.T, workflowDefinition string) ( logger, cfg, stepRegistry, + builtinfunctions.GetFunctions(), )) wf := assert.NoErrorR[*workflow.Workflow](t)(workflow.NewYAMLConverter(stepRegistry).FromYAML([]byte(workflowDefinition))) return executor.Prepare(wf, map[string][]byte{}) @@ -279,5 +282,5 @@ func TestDependOnNoOutputs(t *testing.T) { // into the workflow's expression path data structure at preparation time. _, err := getTestImplPreparedWorkflow(t, invalidWaitfor) assert.Error(t, err) - assert.Contains(t, err.Error(), "object wait_1 does not have a property named deploy") + assert.Contains(t, err.Error(), "object wait_1 does not have a property named \"deploy\"") } diff --git a/workflow/workflow.go b/workflow/workflow.go index 6d1372b5..1d15b323 100644 --- a/workflow/workflow.go +++ b/workflow/workflow.go @@ -29,6 +29,7 @@ const ( type executableWorkflow struct { logger log.Logger config *config.Config + callableFunctions map[string]schema.CallableFunction dag dgraph.DirectedGraph[*DAGItem] input schema.Scope stepRunData map[string]map[string]any @@ -75,6 +76,7 @@ func (e *executableWorkflow) Execute(ctx context.Context, serializedInput any) ( WorkflowInputKey: serializedInput, WorkflowStepsKey: map[string]any{}, }, + callableFunctions: e.callableFunctions, dag: e.dag.Clone(), inputsNotified: make(map[string]struct{}, len(e.dag.ListNodes())), runningSteps: make(map[string]step.RunningStep, len(e.dag.ListNodes())), @@ -222,6 +224,7 @@ type loopState struct { // If steps use this, it could cause dependency problems, concurrency problems, and scalability problems. data map[string]any dag dgraph.DirectedGraph[*DAGItem] + callableFunctions map[string]schema.CallableFunction inputsNotified map[string]struct{} runningSteps map[string]step.RunningStep outputDataChannel chan outputDataType @@ -522,7 +525,7 @@ func (l *loopState) checkForDeadlocks(retries int, wg *sync.WaitGroup) { func (l *loopState) resolveExpressions(inputData any, dataModel any) (any, error) { if expr, ok := inputData.(expressions.Expression); ok { l.logger.Debugf("Evaluating expression %s...", expr.String()) - return expr.Evaluate(dataModel, make(map[string]schema.CallableFunction), l.workflowContext) // TODO + return expr.Evaluate(dataModel, l.callableFunctions, l.workflowContext) } v := reflect.ValueOf(inputData) switch v.Kind() { diff --git a/workflow/workflow_test.go b/workflow/workflow_test.go index 18487773..067e614e 100644 --- a/workflow/workflow_test.go +++ b/workflow/workflow_test.go @@ -10,6 +10,7 @@ import ( "go.flow.arcalot.io/deployer" deployerregistry "go.flow.arcalot.io/deployer/registry" "go.flow.arcalot.io/engine/config" + "go.flow.arcalot.io/engine/internal/builtinfunctions" "go.flow.arcalot.io/engine/internal/step" "go.flow.arcalot.io/engine/internal/step/foreach" "go.flow.arcalot.io/engine/internal/step/plugin" @@ -329,6 +330,7 @@ func TestWaitForSerial(t *testing.T) { logger, cfg, stepRegistry, + builtinfunctions.GetFunctions(), )) wf := lang.Must2(workflow.NewYAMLConverter(stepRegistry).FromYAML([]byte(waitForSerialWorkflowDefinition))) preparedWorkflow := lang.Must2(executor.Prepare(wf, map[string][]byte{})) @@ -410,6 +412,7 @@ func TestWaitForStarted(t *testing.T) { logger, cfg, stepRegistry, + builtinfunctions.GetFunctions(), )) wf := lang.Must2(workflow.NewYAMLConverter(stepRegistry).FromYAML([]byte(waitForStartedWorkflowDefinition))) preparedWorkflow := lang.Must2(executor.Prepare(wf, map[string][]byte{})) @@ -516,6 +519,7 @@ func TestWaitForSerial_Foreach(t *testing.T) { logger, cfg, stepRegistry, + builtinfunctions.GetFunctions(), )) wf := lang.Must2(workflow.NewYAMLConverter(stepRegistry).FromYAML([]byte(waitForSerialForeachWf))) preparedWorkflow := lang.Must2(executor.Prepare(wf, map[string][]byte{ @@ -621,6 +625,7 @@ func TestWaitForStarted_Foreach(t *testing.T) { logger, cfg, stepRegistry, + builtinfunctions.GetFunctions(), )) wf := lang.Must2(workflow.NewYAMLConverter(stepRegistry).FromYAML([]byte(waitForStartedForeachWf))) preparedWorkflow := lang.Must2(executor.Prepare(wf, map[string][]byte{ @@ -650,7 +655,7 @@ func (f *workflowFactory) createWorkflow(logger log.Logger) (workflow.Executor, if stepR == nil { return nil, fmt.Errorf("YAML converter not available yet, please call the factory function after the engine has initialized") } - return workflow.NewExecutor(logger, f.config, stepR) + return workflow.NewExecutor(logger, f.config, stepR, builtinfunctions.GetFunctions()) } var missingInputsFailedDeploymentWorkflowDefinition = ` @@ -782,6 +787,7 @@ func TestEarlyContextCancellation(t *testing.T) { logger, cfg, stepRegistry, + builtinfunctions.GetFunctions(), )) wf := lang.Must2(workflow.NewYAMLConverter(stepRegistry).FromYAML([]byte(fiveSecWaitWorkflowDefinition))) preparedWorkflow := lang.Must2(executor.Prepare(wf, map[string][]byte{})) From c36109cac1c0bf682ac46a7767cd8b42b642da21 Mon Sep 17 00:00:00 2001 From: Jared O'Connell Date: Wed, 7 Feb 2024 17:09:35 -0500 Subject: [PATCH 03/29] Fixes. --- internal/builtinfunctions/functions.go | 62 ++++++++------------- internal/builtinfunctions/functions_test.go | 14 ++--- internal/infer/infer.go | 55 +++++++++++++----- internal/infer/infer_test.go | 2 +- workflow/executor.go | 9 ++- 5 files changed, 79 insertions(+), 63 deletions(-) diff --git a/internal/builtinfunctions/functions.go b/internal/builtinfunctions/functions.go index 68ae7193..aaac833f 100644 --- a/internal/builtinfunctions/functions.go +++ b/internal/builtinfunctions/functions.go @@ -1,3 +1,4 @@ +// Package builtinfunctions provides functions available to expressions in workflows. package builtinfunctions import ( @@ -8,6 +9,7 @@ import ( "strings" ) +// GetFunctions returns a map of all functions currently available. func GetFunctions() map[string]schema.CallableFunction { // Simple conversions intToFloatFunction := getIntToFloatFunction() @@ -161,9 +163,7 @@ func getBooleanToStringFunction() schema.CallableFunction { ), nil, ), - func(a bool) string { - return strconv.FormatBool(a) - }, + strconv.FormatBool, // Wrap go standard lib function. ) if err != nil { panic(err) @@ -245,9 +245,7 @@ func getStringToBoolFunction() schema.CallableFunction { ), nil, ), - func(s string) (bool, error) { - return strconv.ParseBool(s) - }, + strconv.ParseBool, // Wrap go standard lib function. ) if err != nil { panic(err) @@ -267,16 +265,14 @@ func getCeilFunction() schema.CallableFunction { // Description based on documentation for math.Ceil "Ceil returns the least integer value greater than or equal to x.\n"+ "For example `ceil(1.5)` outputs `2.0`, and `ceil(-1.5)` outputs `-1.0`"+ - "Special cases are:\n"+ - "Ceil(±0) = ±0\n"+ - "Ceil(±Inf) = ±Inf\n"+ - "Ceil(NaN) = Na", + "Special cases are:\n"+ //nolint:goconst + "ceil(±0) = ±0\n"+ + "ceil(±Inf) = ±Inf\n"+ + "ceil(NaN) = Na", ), nil, ), - func(a float64) float64 { - return math.Ceil(a) - }, + math.Ceil, // Wrap go standard lib function. ) if err != nil { panic(err) @@ -297,15 +293,13 @@ func getFloorFunction() schema.CallableFunction { "Floor returns the greatest integer value less than or equal to x.\n"+ "For example `floor(1.5)` outputs `1.0`, and `floor(-1.5)` outputs `-2.0`"+ "Special cases are:\n"+ - "Ceil(±0) = ±0\n"+ - "Ceil(±Inf) = ±Inf\n"+ - "Ceil(NaN) = Na", + "floor(±0) = ±0\n"+ + "floor(±Inf) = ±Inf\n"+ + "floor(NaN) = Na", ), nil, ), - func(a float64) float64 { - return math.Floor(a) - }, + math.Floor, // Wrap go standard lib function. ) if err != nil { panic(err) @@ -326,15 +320,13 @@ func getRoundFunction() schema.CallableFunction { "Round returns the nearest integer, rounding half away from zero.\n"+ "For example `round(1.5)` outputs `2.0`, and `round(-1.5)` outputs `-2.0`"+ "Special cases are:\n"+ - "Ceil(±0) = ±0\n"+ - "Ceil(±Inf) = ±Inf\n"+ - "Ceil(NaN) = Na", + "round(±0) = ±0\n"+ + "round(±Inf) = ±Inf\n"+ + "round(NaN) = Na", ), nil, ), - func(a float64) float64 { - return math.Round(a) - }, + math.Round, // Wrap go standard lib function. ) if err != nil { panic(err) @@ -354,14 +346,12 @@ func getAbsFunction() schema.CallableFunction { // Description based on documentation for math.Abs "abs returns the absolute value of x.\n"+ "Special cases are:\n"+ - "Ceil(±Inf) = +Inf\n"+ - "Ceil(NaN) = Na", + "abs(±Inf) = +Inf\n"+ + "abs(NaN) = Na", ), nil, ), - func(a float64) float64 { - return math.Abs(a) - }, + math.Abs, // Wrap go standard lib function. ) if err != nil { panic(err) @@ -382,9 +372,7 @@ func getToLowerFunction() schema.CallableFunction { ), nil, ), - func(s string) string { - return strings.ToLower(s) - }, + strings.ToLower, // Wrap go standard lib function. ) if err != nil { panic(err) @@ -405,9 +393,7 @@ func getToUpperFunction() schema.CallableFunction { ), nil, ), - func(s string) string { - return strings.ToUpper(s) - }, + strings.ToUpper, // Wrap go standard lib function. ) if err != nil { panic(err) @@ -433,9 +419,7 @@ func getSplitStringFunction() schema.CallableFunction { ), nil, ), - func(source string, separator string) []string { - return strings.Split(source, separator) - }, + strings.Split, ) if err != nil { panic(err) diff --git a/internal/builtinfunctions/functions_test.go b/internal/builtinfunctions/functions_test.go index 5eedb51f..eef5ff65 100644 --- a/internal/builtinfunctions/functions_test.go +++ b/internal/builtinfunctions/functions_test.go @@ -25,11 +25,11 @@ var testData = map[string]struct { false, -1.0, }, - "int-to-float-negative-zero": { + "int-to-float-zero": { "intToFloat", - []any{int64(-0)}, + []any{int64(0)}, false, - -0.0, + 0.0, }, "float-to-int-positive": { "floatToInt", @@ -43,11 +43,11 @@ var testData = map[string]struct { false, int64(-1), }, - "float-to-int-negative-zero": { + "float-to-int-zero": { "floatToInt", - []any{-0.0}, + []any{0.0}, false, - int64(-0), + int64(0), }, "int-to-string-positive": { "intToString", @@ -363,7 +363,7 @@ var testData = map[string]struct { }, "abs-1": { "abs", - []any{-0.0}, + []any{0.0}, false, 0.0, }, diff --git a/internal/infer/infer.go b/internal/infer/infer.go index 1bf917f0..d0bc394b 100644 --- a/internal/infer/infer.go +++ b/internal/infer/infer.go @@ -14,9 +14,16 @@ import ( ) // OutputSchema either uses the passed output schema or infers the schema from the data model. -func OutputSchema(data any, outputID string, outputSchema *schema.StepOutputSchema, internalDataModel *schema.ScopeSchema, workflowContext map[string][]byte) (*schema.StepOutputSchema, error) { +func OutputSchema( + data any, + outputID string, + outputSchema *schema.StepOutputSchema, + internalDataModel *schema.ScopeSchema, + functions map[string]schema.Function, + workflowContext map[string][]byte, +) (*schema.StepOutputSchema, error) { if outputSchema == nil { - inferredScope, err := Scope(data, internalDataModel, workflowContext) + inferredScope, err := Scope(data, internalDataModel, functions, workflowContext) if err != nil { return nil, fmt.Errorf("unable to infer output schema for %s (%w)", outputID, err) } @@ -30,8 +37,13 @@ func OutputSchema(data any, outputID string, outputSchema *schema.StepOutputSche } // Scope will infer a scope from the data. -func Scope(data any, internalDataModel *schema.ScopeSchema, workflowContext map[string][]byte) (schema.Scope, error) { - dataType, err := Type(data, internalDataModel, workflowContext) +func Scope( + data any, + internalDataModel *schema.ScopeSchema, + functions map[string]schema.Function, + workflowContext map[string][]byte, +) (schema.Scope, error) { + dataType, err := Type(data, internalDataModel, functions, workflowContext) if err != nil { return nil, fmt.Errorf("failed to infer data type (%w)", err) } @@ -51,9 +63,14 @@ func Scope(data any, internalDataModel *schema.ScopeSchema, workflowContext map[ } // Type attempts to infer the data model from the data, possibly evaluating expressions. -func Type(data any, internalDataModel *schema.ScopeSchema, workflowContext map[string][]byte) (schema.Type, error) { +func Type( + data any, + internalDataModel *schema.ScopeSchema, + functions map[string]schema.Function, + workflowContext map[string][]byte, +) (schema.Type, error) { if expression, ok := data.(expressions.Expression); ok { - expressionType, err := expression.Type(internalDataModel, make(map[string]schema.Function), workflowContext) // TODO + expressionType, err := expression.Type(internalDataModel, functions, workflowContext) if err != nil { return nil, fmt.Errorf("failed to evaluate type of expression %s (%w)", expression.String(), err) } @@ -62,9 +79,9 @@ func Type(data any, internalDataModel *schema.ScopeSchema, workflowContext map[s v := reflect.ValueOf(data) switch v.Kind() { case reflect.Map: - return mapType(v, internalDataModel, workflowContext) + return mapType(v, internalDataModel, functions, workflowContext) case reflect.Slice: - return sliceType(v, internalDataModel, workflowContext) + return sliceType(v, internalDataModel, functions, workflowContext) case reflect.String: return schema.NewStringSchema(nil, nil, nil), nil case reflect.Int8: @@ -116,9 +133,10 @@ func Type(data any, internalDataModel *schema.ScopeSchema, workflowContext map[s func mapType( v reflect.Value, internalDataModel *schema.ScopeSchema, + functions map[string]schema.Function, workflowContext map[string][]byte, ) (schema.Type, error) { - keyType, err := sliceItemType(v.MapKeys(), internalDataModel, workflowContext) + keyType, err := sliceItemType(v.MapKeys(), internalDataModel, functions, workflowContext) if err != nil { return nil, fmt.Errorf("failed to infer map key type (%w)", err) } @@ -126,7 +144,7 @@ func mapType( case schema.TypeIDString: fallthrough case schema.TypeIDStringEnum: - return objectType(v, internalDataModel, workflowContext) + return objectType(v, internalDataModel, functions, workflowContext) case schema.TypeIDInt: case schema.TypeIDIntEnum: default: @@ -136,7 +154,7 @@ func mapType( for _, mapKey := range v.MapKeys() { mapValue := v.MapIndex(mapKey) mapValueAny := mapValue.Interface() - valueType, err := Type(mapValueAny, internalDataModel, workflowContext) + valueType, err := Type(mapValueAny, internalDataModel, functions, workflowContext) if err != nil { return nil, fmt.Errorf("failed to infer type of %d (%w)", mapValueAny, err) } @@ -169,11 +187,12 @@ func mapType( func objectType( value reflect.Value, internalDataModel *schema.ScopeSchema, + functions map[string]schema.Function, workflowContext map[string][]byte, ) (schema.Type, error) { properties := make(map[string]*schema.PropertySchema, value.Len()) for _, keyValue := range value.MapKeys() { - propertyType, err := Type(value.MapIndex(keyValue).Interface(), internalDataModel, workflowContext) + propertyType, err := Type(value.MapIndex(keyValue).Interface(), internalDataModel, functions, workflowContext) if err != nil { return nil, fmt.Errorf("failed to infer property %s type (%w)", keyValue.Interface(), err) } @@ -198,13 +217,14 @@ func objectType( func sliceType( v reflect.Value, internalDataModel *schema.ScopeSchema, + functions map[string]schema.Function, workflowContext map[string][]byte, ) (schema.Type, error) { values := make([]reflect.Value, v.Len()) for i := 0; i < v.Len(); i++ { values[i] = v.Index(i) } - foundType, err := sliceItemType(values, internalDataModel, workflowContext) + foundType, err := sliceItemType(values, internalDataModel, functions, workflowContext) if err != nil { return nil, err } @@ -215,12 +235,17 @@ func sliceType( ), nil } -func sliceItemType(values []reflect.Value, internalDataModel *schema.ScopeSchema, workflowContext map[string][]byte) (schema.Type, error) { +func sliceItemType( + values []reflect.Value, + internalDataModel *schema.ScopeSchema, + functions map[string]schema.Function, + workflowContext map[string][]byte, +) (schema.Type, error) { types := make([]schema.Type, len(values)) var foundType schema.Type for i, value := range values { var err error - types[i], err = Type(value.Interface(), internalDataModel, workflowContext) + types[i], err = Type(value.Interface(), internalDataModel, functions, workflowContext) if err != nil { return nil, fmt.Errorf("failed to infer type for item %d (%w)", i, err) } diff --git a/internal/infer/infer_test.go b/internal/infer/infer_test.go index 98002224..e22476fb 100644 --- a/internal/infer/infer_test.go +++ b/internal/infer/infer_test.go @@ -60,7 +60,7 @@ var testData = []testEntry{ func TestInfer(t *testing.T) { for _, entry := range testData { t.Run(entry.name, func(t *testing.T) { - inferredType, err := infer.Type(entry.input, nil, nil) + inferredType, err := infer.Type(entry.input, nil, make(map[string]schema.Function), nil) if err != nil { t.Fatalf("%v", err) } diff --git a/workflow/executor.go b/workflow/executor.go index b560ab3f..50687d44 100644 --- a/workflow/executor.go +++ b/workflow/executor.go @@ -162,7 +162,14 @@ func (e *executor) Prepare(workflow *Workflow, workflowContext map[string][]byte } outputSchema = outputSchemaData.(*schema.StepOutputSchema) } - outputSchema, err = infer.OutputSchema(outputData, outputID, outputSchema, internalDataModel, workflowContext) + outputSchema, err = infer.OutputSchema( + outputData, + outputID, + outputSchema, + internalDataModel, + e.callableFunctionSchemas, + workflowContext, + ) if err != nil { return nil, fmt.Errorf("cannot read/infer workflow output schema for output %s (%w)", outputID, err) } From cc2a01e9be319870dd8e350e24eb5a382dbbc8ef Mon Sep 17 00:00:00 2001 From: Jared O'Connell Date: Fri, 9 Feb 2024 13:06:42 -0500 Subject: [PATCH 04/29] Added workflow test cases with more complex expressions --- go.mod | 2 +- go.sum | 4 +- internal/infer/infer.go | 2 +- .../escaped_characters_workflow.yaml | 30 +++++ workflow/workflow_test.go | 112 ++++++++++++++++++ 5 files changed, 146 insertions(+), 4 deletions(-) create mode 100644 workflow/test_workflows/escaped_characters_workflow.yaml diff --git a/go.mod b/go.mod index ff86c8a4..6c618f71 100644 --- a/go.mod +++ b/go.mod @@ -11,7 +11,7 @@ require ( go.arcalot.io/log/v2 v2.1.0 go.flow.arcalot.io/deployer v0.5.0 go.flow.arcalot.io/dockerdeployer v0.6.1 - go.flow.arcalot.io/expressions v0.3.0 + go.flow.arcalot.io/expressions v0.4.0 go.flow.arcalot.io/kubernetesdeployer v0.9.1 go.flow.arcalot.io/pluginsdk v0.8.0 go.flow.arcalot.io/podmandeployer v0.8.1 diff --git a/go.sum b/go.sum index ca089b67..add0153e 100644 --- a/go.sum +++ b/go.sum @@ -135,8 +135,8 @@ go.flow.arcalot.io/deployer v0.5.0 h1:yXYogvL3shNBEEoTx9U9CNbfxuf8777uAH5Vn3hv1Y go.flow.arcalot.io/deployer v0.5.0/go.mod h1:whj8wOUursCnfZCt1a7eY5hU3EyOcUG48vM4NeAe5N8= go.flow.arcalot.io/dockerdeployer v0.6.1 h1:oRhxXEeOHmXDQVgtYa95tUpw9qc/M//pbeLjdDYMUxc= go.flow.arcalot.io/dockerdeployer v0.6.1/go.mod h1:Y5Xfg/Fedw/y4LTV0eiJqnsex1mvTbhtP06LCtFkJyo= -go.flow.arcalot.io/expressions v0.3.0 h1:PhHiAyRcPLWp3dycZGoY1X3al7yanB9xoxJqyigzxts= -go.flow.arcalot.io/expressions v0.3.0/go.mod h1:WfbAaPUfvAnarly6GO87Cyi6lmWmttMaVS1rR9YDyrU= +go.flow.arcalot.io/expressions v0.4.0 h1:fYr4Ud6GA/dAjj2iaewaVOauZPwY6/QHN8F25Q0+w1Y= +go.flow.arcalot.io/expressions v0.4.0/go.mod h1:WfbAaPUfvAnarly6GO87Cyi6lmWmttMaVS1rR9YDyrU= go.flow.arcalot.io/kubernetesdeployer v0.9.1 h1:AGnJFazehAENXxGMCF0Uc7aG9F0LpvuhoyQFu8deJG0= go.flow.arcalot.io/kubernetesdeployer v0.9.1/go.mod h1:yvxT3VwmyrlIi4422pxl02z4QeU2Gvbjg5aQB17Ye4s= go.flow.arcalot.io/pluginsdk v0.8.0 h1:cShsshrR17ZFLcbgi3aZvqexLttcp3JISFNqPUPuDvA= diff --git a/internal/infer/infer.go b/internal/infer/infer.go index d0bc394b..781f77bc 100644 --- a/internal/infer/infer.go +++ b/internal/infer/infer.go @@ -208,7 +208,7 @@ func objectType( ) } return schema.NewObjectSchema( - generateRandomObjectID(), + "inferred_schema_"+generateRandomObjectID(), properties, ), nil } diff --git a/workflow/test_workflows/escaped_characters_workflow.yaml b/workflow/test_workflows/escaped_characters_workflow.yaml new file mode 100644 index 00000000..d3e8a580 --- /dev/null +++ b/workflow/test_workflows/escaped_characters_workflow.yaml @@ -0,0 +1,30 @@ +version: v0.2.0 +input: + root: RootObject + objects: + RootObject: + id: RootObject + properties: + a: + type: + type_id: string +steps: + no_wait: + plugin: + src: "n/a" + deployment_type: "builtin" + step: wait + input: + wait_time_ms: 0 +outputs: + success: + # GOAL: "\\\\" and "\\" and "'"'" and " " \t Plugin slept for 0 ms. + result_raw_inlined: !expr '`"` + $.input.a + `" and "\\" and "''"''" and " " \t ` + $.steps.no_wait.outputs.success.message' + result_raw_flow_scalar: !expr |- + `"` + $.input.a + `" and "\\" and "'"'" and " " \t ` + $.steps.no_wait.outputs.success.message + result_inlined_single_quote: !expr '"\"" + $.input.a + "\" and \"\\\\\" and \"''\"''\" and \"\t\" \\t " + $.steps.no_wait.outputs.success.message' + result_inlined_double_quote: !expr "'\"' + $.input.a + '\" and \"\\\\\\\\\" and \"\\'\"\\'\" and \"\t\" \\\\t ' + $.steps.no_wait.outputs.success.message" + result_flow_scalar_single_quote: !expr |- + '"' + $.input.a + '" and "\\\\" and "\'"\'" and "\t" \\t ' + $.steps.no_wait.outputs.success.message + result_flow_scalar_double_quote: !expr |- + "\"" + $.input.a + "\" and \"\\\\\" and \"'\"'\" and \"\t\" \\t " + $.steps.no_wait.outputs.success.message diff --git a/workflow/workflow_test.go b/workflow/workflow_test.go index 067e614e..915385eb 100644 --- a/workflow/workflow_test.go +++ b/workflow/workflow_test.go @@ -16,6 +16,7 @@ import ( "go.flow.arcalot.io/engine/internal/step/plugin" stepregistry "go.flow.arcalot.io/engine/internal/step/registry" testimpl "go.flow.arcalot.io/testdeployer" + "os" "testing" "time" @@ -804,3 +805,114 @@ func TestEarlyContextCancellation(t *testing.T) { t.Fatalf("Test execution time is greater than 1000 milliseconds; Is the workflow properly cancelling?") } } + +var justMathWorkflowDefinition = ` +version: v0.2.0 +input: + root: RootObject + objects: + RootObject: + id: RootObject + properties: + a: + type: + type_id: integer + b: + type: + type_id: integer +steps: + no_wait: + plugin: + src: "n/a" + deployment_type: "builtin" + step: wait + input: + wait_time_ms: 0 +outputs: + success: + result: !expr intToString($.input.a + $.input.b) + " and " + $.steps.no_wait.outputs.success.message +` + +func TestWorkflowWithMathAndFunctions(t *testing.T) { + // For this test, we run the minimum amount of steps, and resolve the output with math and functions. + logConfig := log.Config{ + Level: log.LevelDebug, + Destination: log.DestinationStdout, + } + logger := log.New( + logConfig, + ) + cfg := &config.Config{ + Log: logConfig, + } + stepRegistry := NewTestImplStepRegistry(logger, t) + + executor := lang.Must2(workflow.NewExecutor( + logger, + cfg, + stepRegistry, + builtinfunctions.GetFunctions(), + )) + wf := lang.Must2(workflow.NewYAMLConverter(stepRegistry).FromYAML([]byte(justMathWorkflowDefinition))) + preparedWorkflow := lang.Must2(executor.Prepare(wf, map[string][]byte{})) + outputID, outputData, err := preparedWorkflow.Execute(context.Background(), map[string]any{ + "a": int64(2), + "b": int64(2), + }) + assert.NoError(t, err) + assert.Equals(t, outputID, "success") + assert.Equals(t, outputData.(map[any]any), map[any]any{ + "result": "4 and Plugin slept for 0 ms.", + }) +} + +func TestWorkflowWithEscapedCharacters(t *testing.T) { + // For this test, we have escapable characters in the input and in the expressions + // to make sure they are handled properly. + logConfig := log.Config{ + Level: log.LevelDebug, + Destination: log.DestinationStdout, + } + logger := log.New( + logConfig, + ) + cfg := &config.Config{ + Log: logConfig, + } + stepRegistry := NewTestImplStepRegistry(logger, t) + + executor := lang.Must2(workflow.NewExecutor( + logger, + cfg, + stepRegistry, + builtinfunctions.GetFunctions(), + )) + fileData, err := os.ReadFile("./test_workflows/escaped_characters_workflow.yaml") + assert.NoError(t, err) + wf := lang.Must2(workflow.NewYAMLConverter(stepRegistry).FromYAML(fileData)) + preparedWorkflow := lang.Must2(executor.Prepare(wf, map[string][]byte{})) + outputID, outputData, err := preparedWorkflow.Execute(context.Background(), map[string]any{ + "a": `\\\\`, // Start with four. There should still be four in the output. + }) + assert.NoError(t, err) + assert.Equals(t, outputID, "success") + expectedString := `"\\\\" and "\\" and "'"'" and " " \t Plugin slept for 0 ms.` + expectedOutput := map[any]any{ + "result_raw_inlined": expectedString, + "result_raw_flow_scalar": expectedString, + "result_inlined_single_quote": expectedString, + "result_inlined_double_quote": expectedString, + "result_flow_scalar_single_quote": expectedString, + "result_flow_scalar_double_quote": expectedString, + } + outputAsMap := outputData.(map[any]any) + for expectedKey, expectedValue := range expectedOutput { + value, outputHasExpectedKey := outputAsMap[expectedKey] + if !outputHasExpectedKey { + t.Errorf("output missing expected key %q", expectedKey) + } else if value != expectedValue { + t.Errorf("case %q failed; expected (%s), got (%s)", expectedKey, expectedValue, value) + } + + } +} From d8728906987be71c187ea69acb42acf4805ae9f7 Mon Sep 17 00:00:00 2001 From: Jared O'Connell Date: Tue, 13 Feb 2024 18:19:59 -0500 Subject: [PATCH 05/29] Addressed review feedback --- internal/builtinfunctions/functions.go | 62 ++++----- internal/builtinfunctions/functions_test.go | 137 ++++++++++++++++++-- internal/infer/infer.go | 6 +- internal/infer/infer_test.go | 3 +- workflow/executor.go | 6 +- workflow/executor_test.go | 2 +- 6 files changed, 167 insertions(+), 49 deletions(-) diff --git a/internal/builtinfunctions/functions.go b/internal/builtinfunctions/functions.go index aaac833f..caf9ec91 100644 --- a/internal/builtinfunctions/functions.go +++ b/internal/builtinfunctions/functions.go @@ -83,9 +83,9 @@ func getFloatToIntFunction() schema.CallableFunction { schema.NewDisplayValue( schema.PointerTo("floatToInt"), schema.PointerTo( - "Converts a float type into an integer type by down-casting. "+ - "The value loses any data after the decimal point.\n"+ - "For example, `5.5` becomes `5`", + "Converts a float type into an integer type by discarding the fraction."+ + " In other words, it is rounded to the nearest integer towards zero.\n"+ + "For example, `5.5` becomes `5`, and `-1.9` becomes `-1`", ), nil, ), @@ -109,7 +109,7 @@ func getIntToStringFunction() schema.CallableFunction { schema.PointerTo("intToString"), schema.PointerTo( "Converts an integer to a string whose characters represent that integer in base-10.\n"+ - "For example, an input of `55` will output \"55\"", + "For example, an input of `55` will output `\"55\"`", ), nil, ), @@ -134,7 +134,7 @@ func getFloatToStringFunction() schema.CallableFunction { schema.PointerTo( "Converts a floating point number to a string whose characters"+ "represent that number in base-10 as as simple decimal.\n"+ - "For example, an input of `5000.5` will output \"5000.5\"", + "For example, an input of `5000.5` will output `\"5000.5\"`", ), nil, ), @@ -159,7 +159,7 @@ func getBooleanToStringFunction() schema.CallableFunction { schema.NewDisplayValue( schema.PointerTo("boolToString"), schema.PointerTo( - "Represents `true` as \"true\", and `false` as \"false\".", + "Returns `\"true\"` for `true`, and `\"false\"` for `false`.", ), nil, ), @@ -180,7 +180,7 @@ func getStringToIntFunction() schema.CallableFunction { schema.NewDisplayValue( schema.PointerTo("stringToInt"), schema.PointerTo( - "Interprets the string as a base-10 integer. Can fail if the input is not a valid integer.", + "Interprets the string as a base-10 integer. Will fail if the input is not a valid integer.", ), nil, ), @@ -203,14 +203,14 @@ func getStringToFloatFunction() schema.CallableFunction { schema.NewDisplayValue( schema.PointerTo("stringToFloat"), schema.PointerTo( - "Converts the string s to a 64-bit floating-point number\n\n"+ + "Converts the input string to a 64-bit floating-point number\n\n"+ "Accepts decimal and hexadecimal floating-point numbers\n"+ "as defined by the Go syntax for floating point literals\n"+ "https://go.dev/ref/spec#Floating-point_literals.\n"+ - "If s is well-formed and near a valid floating-point number,\n"+ - "ParseFloat returns the nearest floating-point number rounded\n"+ + "If the input is well-formed and near a valid floating-point number,\n"+ + "stringToFloat returns the nearest floating-point number rounded\n"+ "using IEEE754 unbiased rounding.\n\n"+ - "Returns NumError when an invalid input is received.", + "Returns an error when an invalid input is received.", ), nil, ), @@ -231,7 +231,7 @@ func getStringToBoolFunction() schema.CallableFunction { schema.NewStringSchema( nil, nil, - regexp.MustCompile(`[Tt]rue|TRUE|[Ff]alse|FALSE|[tTfF]|[01]$`), + regexp.MustCompile(`(?i)^(?:true|false|[tf]|[01])$`), )}, schema.NewBoolSchema(), true, @@ -239,13 +239,15 @@ func getStringToBoolFunction() schema.CallableFunction { schema.PointerTo("stringToBool"), schema.PointerTo( "Interprets the input as a boolean.\n"+ - "Accepts `1`, `t`, `T`, `true`, `TRUE`, and `True` for true.\n"+ - "Accepts `0`, 'f', 'F', 'false', 'FALSE', and 'False' for false.\n"+ - "Returns an error if any other value is input.", + "Case insensitively accepts `\"1\"`, `\"t\"`, and `\"true\"` for `true`.\n"+ + "Case insensitively accepts `\"0\"`, '\"f\"', and '\"false\"' for `false`.\n"+ + "Returns an error for any other input.", ), nil, ), - strconv.ParseBool, // Wrap go standard lib function. + func(s string) (bool, error) { + return strconv.ParseBool(strings.ToLower(s)) + }, ) if err != nil { panic(err) @@ -263,12 +265,12 @@ func getCeilFunction() schema.CallableFunction { schema.PointerTo("ceil"), schema.PointerTo( // Description based on documentation for math.Ceil - "Ceil returns the least integer value greater than or equal to x.\n"+ + "Ceil returns the least integer value greater than or equal to the input.\n"+ "For example `ceil(1.5)` outputs `2.0`, and `ceil(-1.5)` outputs `-1.0`"+ "Special cases are:\n"+ //nolint:goconst - "ceil(±0) = ±0\n"+ - "ceil(±Inf) = ±Inf\n"+ - "ceil(NaN) = Na", + " ceil(±0) = ±0\n"+ + " ceil(±Inf) = ±Inf\n"+ + " ceil(NaN) = NaN", ), nil, ), @@ -290,12 +292,12 @@ func getFloorFunction() schema.CallableFunction { schema.PointerTo("floor"), schema.PointerTo( // Description based on documentation for math.Floor - "Floor returns the greatest integer value less than or equal to x.\n"+ + "Floor returns the greatest integer value less than or equal to the input.\n"+ "For example `floor(1.5)` outputs `1.0`, and `floor(-1.5)` outputs `-2.0`"+ "Special cases are:\n"+ - "floor(±0) = ±0\n"+ - "floor(±Inf) = ±Inf\n"+ - "floor(NaN) = Na", + " floor(±0) = ±0\n"+ + " floor(±Inf) = ±Inf\n"+ + " floor(NaN) = NaN", ), nil, ), @@ -317,12 +319,12 @@ func getRoundFunction() schema.CallableFunction { schema.PointerTo("round"), schema.PointerTo( // Description based on documentation for math.Round - "Round returns the nearest integer, rounding half away from zero.\n"+ + "Round returns the nearest integer to the input, rounding half away from zero.\n"+ "For example `round(1.5)` outputs `2.0`, and `round(-1.5)` outputs `-2.0`"+ "Special cases are:\n"+ - "round(±0) = ±0\n"+ - "round(±Inf) = ±Inf\n"+ - "round(NaN) = Na", + " round(±0) = ±0\n"+ + " round(±Inf) = ±Inf\n"+ + " round(NaN) = NaN", ), nil, ), @@ -346,8 +348,8 @@ func getAbsFunction() schema.CallableFunction { // Description based on documentation for math.Abs "abs returns the absolute value of x.\n"+ "Special cases are:\n"+ - "abs(±Inf) = +Inf\n"+ - "abs(NaN) = Na", + " abs(±Inf) = +Inf\n"+ + " abs(NaN) = NaN", ), nil, ), diff --git a/internal/builtinfunctions/functions_test.go b/internal/builtinfunctions/functions_test.go index eef5ff65..7a7017fc 100644 --- a/internal/builtinfunctions/functions_test.go +++ b/internal/builtinfunctions/functions_test.go @@ -37,18 +37,42 @@ var testData = map[string]struct { false, int64(1), }, - "float-to-int-negative": { + "float-to-int-negative-whole": { "floatToInt", []any{-1.0}, false, int64(-1), }, + "float-to-int-negative-fractional": { + "floatToInt", + []any{-1.9}, + false, + int64(-1), + }, "float-to-int-zero": { "floatToInt", []any{0.0}, false, int64(0), }, + "float-to-int-positive-infinity": { + "floatToInt", + []any{math.Inf(1)}, + false, + int64(math.MaxInt64), + }, + "float-to-int-negative-infinity": { + "floatToInt", + []any{math.Inf(-1)}, + false, + int64(math.MinInt64), + }, + "float-to-int-nan": { + "floatToInt", + []any{math.NaN()}, + false, + int64(0), + }, "int-to-string-positive": { "intToString", []any{int64(11)}, @@ -103,6 +127,18 @@ var testData = map[string]struct { false, "NaN", }, + "float-to-string-positive-infinity": { + "floatToString", + []any{math.Inf(1)}, + false, + "+Inf", + }, + "float-to-string-negative-infinity": { + "floatToString", + []any{math.Inf(-1)}, + false, + "-Inf", + }, "bool-to-string-true": { "boolToString", []any{true}, @@ -151,6 +187,30 @@ var testData = map[string]struct { true, nil, }, + "string-to-int-invalid-4": { + "stringToInt", + []any{"0b1"}, + true, + nil, + }, + "string-to-int-invalid-5": { + "stringToInt", + []any{"0o1"}, + true, + nil, + }, + "string-to-int-invalid-6": { + "stringToInt", + []any{"0x1"}, + true, + nil, + }, + "string-to-int-invalid-7": { + "stringToInt", + []any{"1_000_000"}, + true, + nil, + }, "string-to-float-positive": { "stringToFloat", []any{"11.1"}, @@ -163,18 +223,48 @@ var testData = map[string]struct { false, -11.1, }, - "string-to-float-zero": { + "string-to-float-zero-1": { "stringToFloat", []any{"0"}, false, 0.0, }, - "string-to-float-scientific-notation": { + "string-to-float-zero-2": { + "stringToFloat", + []any{"-0"}, + false, + 0.0, + }, + "string-to-float-positive-infinity-1": { + "stringToFloat", + []any{"Inf"}, + false, + math.Inf(1), + }, + "string-to-float-positive-infinity-2": { + "stringToFloat", + []any{"+Inf"}, + false, + math.Inf(1), + }, + "string-to-float-negative-infinity": { + "stringToFloat", + []any{"-Inf"}, + false, + math.Inf(-1), + }, + "string-to-float-scientific-notation-1": { "stringToFloat", []any{"5e+5"}, false, 500000.0, }, + "string-to-float-scientific-notation-2": { + "stringToFloat", + []any{"5E-5"}, + false, + 0.00005, + }, "string-to-float-error-1": { "stringToFloat", []any{""}, @@ -187,6 +277,12 @@ var testData = map[string]struct { true, nil, }, + "string-to-float-error-3": { + "stringToFloat", + []any{"5E+500"}, + true, + nil, + }, "string-to-bool-1": { "stringToBool", []any{"true"}, @@ -284,6 +380,12 @@ var testData = map[string]struct { 1.0, }, "ceil-3": { + "ceil", + []any{0.99}, + false, + 1.0, + }, + "ceil-4": { "ceil", []any{-0.1}, false, @@ -397,6 +499,12 @@ var testData = map[string]struct { false, "abc", }, + "to-lower-4": { + "toLower", + []any{"aBc"}, + false, + "abc", + }, "to-upper-1": { "toUpper", []any{""}, @@ -415,6 +523,12 @@ var testData = map[string]struct { false, "ABC", }, + "to-upper-4": { + "toUpper", + []any{"aBc"}, + false, + "ABC", + }, "split-string-1": { "splitString", []any{"", ","}, @@ -439,12 +553,19 @@ var testData = map[string]struct { false, []string{"a", "b", "c", "d"}, }, + "split-string-5": { + "splitString", + []any{"1,,2,", ","}, + false, + []string{"1", "", "2", ""}, + }, } func TestFunctionsBulk(t *testing.T) { allFunctions := builtinfunctions.GetFunctions() for name, tc := range testData { testCase := tc + name := name t.Run(name, func(t *testing.T) { functionToTest, funcFound := allFunctions[testCase.functionName] if !funcFound { @@ -453,13 +574,9 @@ func TestFunctionsBulk(t *testing.T) { output, err := functionToTest.Call(testCase.rawArguments) - if testCase.returnError { - if err == nil { - t.Fatalf("expected error in test case %q; error returned nil", name) - } - return - } - if err != nil { + if testCase.returnError && err == nil { + t.Fatalf("expected error in test case %q; error returned nil", name) + } else if !testCase.returnError && err != nil { t.Fatalf("unexpected error in test case %q (%s)", name, err.Error()) } diff --git a/internal/infer/infer.go b/internal/infer/infer.go index 781f77bc..ac52a384 100644 --- a/internal/infer/infer.go +++ b/internal/infer/infer.go @@ -208,7 +208,7 @@ func objectType( ) } return schema.NewObjectSchema( - "inferred_schema_"+generateRandomObjectID(), + generateRandomObjectID("inferred_schema"), properties, ), nil } @@ -263,10 +263,10 @@ func sliceItemType( var characters = []rune("abcdefghijklmnopqrstuvwxyz0123456789") var objectIDRandom = rand.New(rand.NewSource(time.Now().UnixNano())) //nolint:gosec -func generateRandomObjectID() string { +func generateRandomObjectID(purpose string) string { result := make([]rune, 32) for i := range result { result[i] = characters[objectIDRandom.Intn(len(characters))] } - return string(result) + return purpose + "_" + string(result) } diff --git a/internal/infer/infer_test.go b/internal/infer/infer_test.go index e22476fb..80b35561 100644 --- a/internal/infer/infer_test.go +++ b/internal/infer/infer_test.go @@ -59,8 +59,9 @@ var testData = []testEntry{ func TestInfer(t *testing.T) { for _, entry := range testData { + entry := entry t.Run(entry.name, func(t *testing.T) { - inferredType, err := infer.Type(entry.input, nil, make(map[string]schema.Function), nil) + inferredType, err := infer.Type(entry.input, nil, nil, nil) if err != nil { t.Fatalf("%v", err) } diff --git a/workflow/executor.go b/workflow/executor.go index 50687d44..96928acd 100644 --- a/workflow/executor.go +++ b/workflow/executor.go @@ -809,13 +809,11 @@ func (e *executor) prepareDependencies( //nolint:gocognit,gocyclo case reflect.Struct: switch s := stepData.(type) { case expressions.Expression: - // Evaluate the dependencies of the expression on the data structure. - // Include extraneous is false due to this only being needed for connecting to the appropriate node, - // as opposed to the data provided by that node. + // Evaluate the dependencies of the expression on the main data structure. pathUnpackRequirements := expressions.UnpackRequirements{ ExcludeDataRootPaths: false, ExcludeFunctionRootPaths: true, // We don't need to setup DAG connections for them. - StopAtTerminals: true, // I don't think we need the extra info. We just need the connection. + StopAtTerminals: true, // We need the extra info. We just need the connection. IncludeKeys: false, } dependencies, err := s.Dependencies(outputSchema, e.callableFunctionSchemas, workflowContext, pathUnpackRequirements) diff --git a/workflow/executor_test.go b/workflow/executor_test.go index b8b4053f..fdb90634 100644 --- a/workflow/executor_test.go +++ b/workflow/executor_test.go @@ -282,5 +282,5 @@ func TestDependOnNoOutputs(t *testing.T) { // into the workflow's expression path data structure at preparation time. _, err := getTestImplPreparedWorkflow(t, invalidWaitfor) assert.Error(t, err) - assert.Contains(t, err.Error(), "object wait_1 does not have a property named \"deploy\"") + assert.Contains(t, err.Error(), `object wait_1 does not have a property named "deploy"`) } From 41137ae9119bbca06a12a6e5fe52a3772acba842 Mon Sep 17 00:00:00 2001 From: Jared O'Connell Date: Tue, 13 Feb 2024 18:23:27 -0500 Subject: [PATCH 06/29] Remove unnecessary reference to the function names in descriptions --- internal/builtinfunctions/functions.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/builtinfunctions/functions.go b/internal/builtinfunctions/functions.go index caf9ec91..772b2d34 100644 --- a/internal/builtinfunctions/functions.go +++ b/internal/builtinfunctions/functions.go @@ -265,7 +265,7 @@ func getCeilFunction() schema.CallableFunction { schema.PointerTo("ceil"), schema.PointerTo( // Description based on documentation for math.Ceil - "Ceil returns the least integer value greater than or equal to the input.\n"+ + "Returns the least integer value greater than or equal to the input.\n"+ "For example `ceil(1.5)` outputs `2.0`, and `ceil(-1.5)` outputs `-1.0`"+ "Special cases are:\n"+ //nolint:goconst " ceil(±0) = ±0\n"+ @@ -292,7 +292,7 @@ func getFloorFunction() schema.CallableFunction { schema.PointerTo("floor"), schema.PointerTo( // Description based on documentation for math.Floor - "Floor returns the greatest integer value less than or equal to the input.\n"+ + "Returns the greatest integer value less than or equal to the input.\n"+ "For example `floor(1.5)` outputs `1.0`, and `floor(-1.5)` outputs `-2.0`"+ "Special cases are:\n"+ " floor(±0) = ±0\n"+ @@ -319,7 +319,7 @@ func getRoundFunction() schema.CallableFunction { schema.PointerTo("round"), schema.PointerTo( // Description based on documentation for math.Round - "Round returns the nearest integer to the input, rounding half away from zero.\n"+ + "Returns the nearest integer to the input, rounding half away from zero.\n"+ "For example `round(1.5)` outputs `2.0`, and `round(-1.5)` outputs `-2.0`"+ "Special cases are:\n"+ " round(±0) = ±0\n"+ @@ -346,7 +346,7 @@ func getAbsFunction() schema.CallableFunction { schema.PointerTo("abs"), schema.PointerTo( // Description based on documentation for math.Abs - "abs returns the absolute value of x.\n"+ + "returns the absolute value of x.\n"+ "Special cases are:\n"+ " abs(±Inf) = +Inf\n"+ " abs(NaN) = NaN", From 4c7f591a280f19966552ab596a2c5095012b8311 Mon Sep 17 00:00:00 2001 From: Jared O'Connell Date: Tue, 13 Feb 2024 18:34:58 -0500 Subject: [PATCH 07/29] More description changes --- internal/builtinfunctions/functions.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/builtinfunctions/functions.go b/internal/builtinfunctions/functions.go index 772b2d34..be3d8306 100644 --- a/internal/builtinfunctions/functions.go +++ b/internal/builtinfunctions/functions.go @@ -346,7 +346,7 @@ func getAbsFunction() schema.CallableFunction { schema.PointerTo("abs"), schema.PointerTo( // Description based on documentation for math.Abs - "returns the absolute value of x.\n"+ + "Returns the absolute value of x.\n"+ "Special cases are:\n"+ " abs(±Inf) = +Inf\n"+ " abs(NaN) = NaN", @@ -417,7 +417,7 @@ func getSplitStringFunction() schema.CallableFunction { schema.PointerTo( "Splits the given string with the given separator.\n"+ " Param 1: The string to split.\n"+ - " Param 2: The separator.\n", + " Param 2: The separator.", ), nil, ), From 6c6b7282bb4a8440946397405561be3926b75ae2 Mon Sep 17 00:00:00 2001 From: Jared O'Connell Date: Wed, 14 Feb 2024 11:03:56 -0500 Subject: [PATCH 08/29] Manually add special cases to prevent platform-specific behavior --- internal/builtinfunctions/functions.go | 9 +++++++++ internal/builtinfunctions/functions_test.go | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/internal/builtinfunctions/functions.go b/internal/builtinfunctions/functions.go index be3d8306..223a1f6b 100644 --- a/internal/builtinfunctions/functions.go +++ b/internal/builtinfunctions/functions.go @@ -85,11 +85,20 @@ func getFloatToIntFunction() schema.CallableFunction { schema.PointerTo( "Converts a float type into an integer type by discarding the fraction."+ " In other words, it is rounded to the nearest integer towards zero.\n"+ + "Special cases:\n"+ + " +Inf outputs the maximum 64-bit integer (9223372036854775807)\n"+ + " -Inf and NaN output the minimum 64-bit integer (-9223372036854775808)\n\n"+ "For example, `5.5` becomes `5`, and `-1.9` becomes `-1`", ), nil, ), func(a float64) int64 { + // Define behavior for special cases. The raw cast in go has platform-specific behavior. + if a == math.Inf(1) { + return math.MaxInt64 + } else if a == math.Inf(-1) || math.IsNaN(a) { + return math.MinInt64 + } return int64(a) }, ) diff --git a/internal/builtinfunctions/functions_test.go b/internal/builtinfunctions/functions_test.go index 7a7017fc..cfac607b 100644 --- a/internal/builtinfunctions/functions_test.go +++ b/internal/builtinfunctions/functions_test.go @@ -71,7 +71,7 @@ var testData = map[string]struct { "floatToInt", []any{math.NaN()}, false, - int64(0), + int64(math.MinInt64), }, "int-to-string-positive": { "intToString", From 3ffb44b808378bd734128daf4fa50db77f872dc1 Mon Sep 17 00:00:00 2001 From: Jared O'Connell Date: Wed, 14 Feb 2024 11:45:38 -0500 Subject: [PATCH 09/29] Added negative zeros to test cases --- internal/builtinfunctions/functions_test.go | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/internal/builtinfunctions/functions_test.go b/internal/builtinfunctions/functions_test.go index cfac607b..909d21a9 100644 --- a/internal/builtinfunctions/functions_test.go +++ b/internal/builtinfunctions/functions_test.go @@ -49,12 +49,18 @@ var testData = map[string]struct { false, int64(-1), }, - "float-to-int-zero": { + "float-to-int-positive-zero": { "floatToInt", []any{0.0}, false, int64(0), }, + "float-to-int-negative-zero": { + "floatToInt", + []any{math.Copysign(0.0, -1)}, + false, + int64(0), + }, "float-to-int-positive-infinity": { "floatToInt", []any{math.Inf(1)}, @@ -121,6 +127,12 @@ var testData = map[string]struct { false, "0", }, + "float-to-string-negative-zero": { + "floatToString", + []any{math.Copysign(0.0, -1)}, + false, + "-0", + }, "float-to-string-nan": { "floatToString", []any{math.NaN()}, @@ -233,7 +245,7 @@ var testData = map[string]struct { "stringToFloat", []any{"-0"}, false, - 0.0, + math.Copysign(0.0, -1), }, "string-to-float-positive-infinity-1": { "stringToFloat", From f1500e4f846afc7a715b30734719563c2a607730 Mon Sep 17 00:00:00 2001 From: Jared O'Connell Date: Wed, 14 Feb 2024 11:54:08 -0500 Subject: [PATCH 10/29] Address review comments --- internal/builtinfunctions/functions.go | 7 ++++--- workflow/executor.go | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/internal/builtinfunctions/functions.go b/internal/builtinfunctions/functions.go index 223a1f6b..d903903c 100644 --- a/internal/builtinfunctions/functions.go +++ b/internal/builtinfunctions/functions.go @@ -248,9 +248,10 @@ func getStringToBoolFunction() schema.CallableFunction { schema.PointerTo("stringToBool"), schema.PointerTo( "Interprets the input as a boolean.\n"+ - "Case insensitively accepts `\"1\"`, `\"t\"`, and `\"true\"` for `true`.\n"+ - "Case insensitively accepts `\"0\"`, '\"f\"', and '\"false\"' for `false`.\n"+ - "Returns an error for any other input.", + " accepts `\"1\"`, `\"t\"`, and `\"true\"` for `true`.\n"+ + " accepts `\"0\"`, '\"f\"', and '\"false\"' for `false`.\n"+ + "Returns an error for any other input.\n"+ + "Inputs are case insensitive. So `\"True\"` is interpreted as `\"true\"`, for example.", ), nil, ), diff --git a/workflow/executor.go b/workflow/executor.go index 96928acd..dc87ef35 100644 --- a/workflow/executor.go +++ b/workflow/executor.go @@ -813,7 +813,7 @@ func (e *executor) prepareDependencies( //nolint:gocognit,gocyclo pathUnpackRequirements := expressions.UnpackRequirements{ ExcludeDataRootPaths: false, ExcludeFunctionRootPaths: true, // We don't need to setup DAG connections for them. - StopAtTerminals: true, // We need the extra info. We just need the connection. + StopAtTerminals: true, // We do not need the extra info. We just need the connection. IncludeKeys: false, } dependencies, err := s.Dependencies(outputSchema, e.callableFunctionSchemas, workflowContext, pathUnpackRequirements) From 1f491cd10ff5ec24a82b75b29f455934afa3e489 Mon Sep 17 00:00:00 2001 From: Jared O'Connell Date: Wed, 14 Feb 2024 11:57:15 -0500 Subject: [PATCH 11/29] Change case --- internal/builtinfunctions/functions.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/builtinfunctions/functions.go b/internal/builtinfunctions/functions.go index d903903c..7423fec1 100644 --- a/internal/builtinfunctions/functions.go +++ b/internal/builtinfunctions/functions.go @@ -248,8 +248,8 @@ func getStringToBoolFunction() schema.CallableFunction { schema.PointerTo("stringToBool"), schema.PointerTo( "Interprets the input as a boolean.\n"+ - " accepts `\"1\"`, `\"t\"`, and `\"true\"` for `true`.\n"+ - " accepts `\"0\"`, '\"f\"', and '\"false\"' for `false`.\n"+ + " Accepts `\"1\"`, `\"t\"`, and `\"true\"` for `true`.\n"+ + " Accepts `\"0\"`, '\"f\"', and '\"false\"' for `false`.\n"+ "Returns an error for any other input.\n"+ "Inputs are case insensitive. So `\"True\"` is interpreted as `\"true\"`, for example.", ), From ad7bbaf9305c7689ffecbebd89bdedc6bad18b65 Mon Sep 17 00:00:00 2001 From: Jared O'Connell Date: Thu, 15 Feb 2024 18:07:35 -0500 Subject: [PATCH 12/29] Address review comments --- internal/builtinfunctions/functions.go | 27 ++++++++++++--------- internal/builtinfunctions/functions_test.go | 4 +-- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/internal/builtinfunctions/functions.go b/internal/builtinfunctions/functions.go index 7423fec1..609ce28a 100644 --- a/internal/builtinfunctions/functions.go +++ b/internal/builtinfunctions/functions.go @@ -2,6 +2,7 @@ package builtinfunctions import ( + "fmt" "go.flow.arcalot.io/pluginsdk/schema" "math" "regexp" @@ -79,27 +80,31 @@ func getFloatToIntFunction() schema.CallableFunction { "floatToInt", []schema.Type{schema.NewFloatSchema(nil, nil, nil)}, schema.NewIntSchema(nil, nil, nil), - false, + true, schema.NewDisplayValue( schema.PointerTo("floatToInt"), schema.PointerTo( "Converts a float type into an integer type by discarding the fraction."+ " In other words, it is rounded to the nearest integer towards zero.\n"+ "Special cases:\n"+ - " +Inf outputs the maximum 64-bit integer (9223372036854775807)\n"+ - " -Inf and NaN output the minimum 64-bit integer (-9223372036854775808)\n\n"+ + " +Inf outputs the maximum signed 64-bit integer (9223372036854775807)\n"+ + " -Inf outputs the minimum signed 64-bit integer (-9223372036854775808)\n"+ + " NaN outputs an error\n\n"+ "For example, `5.5` becomes `5`, and `-1.9` becomes `-1`", ), nil, ), - func(a float64) int64 { - // Define behavior for special cases. The raw cast in go has platform-specific behavior. - if a == math.Inf(1) { - return math.MaxInt64 - } else if a == math.Inf(-1) || math.IsNaN(a) { - return math.MinInt64 + func(a float64) (int64, error) { + // Because the type conversion in Go has platform-specific behavior, handle + // the special cases explicitly so that we get consistent, portable results. + if math.IsInf(a, 1) { + return math.MaxInt64, nil + } else if math.IsInf(a, -1) { + return math.MinInt64, nil + } else if math.IsNaN(a) { + return math.MinInt64, fmt.Errorf("attempted to convert a NaN float to an integer") } - return int64(a) + return int64(a), nil }, ) if err != nil { @@ -240,7 +245,7 @@ func getStringToBoolFunction() schema.CallableFunction { schema.NewStringSchema( nil, nil, - regexp.MustCompile(`(?i)^(?:true|false|[tf]|[01])$`), + regexp.MustCompile(`(?i)^(?:true|false|[tf01])$`), )}, schema.NewBoolSchema(), true, diff --git a/internal/builtinfunctions/functions_test.go b/internal/builtinfunctions/functions_test.go index 909d21a9..d9b74d59 100644 --- a/internal/builtinfunctions/functions_test.go +++ b/internal/builtinfunctions/functions_test.go @@ -76,8 +76,8 @@ var testData = map[string]struct { "float-to-int-nan": { "floatToInt", []any{math.NaN()}, - false, - int64(math.MinInt64), + true, + nil, }, "int-to-string-positive": { "intToString", From b4221203297d1a56aa78147545413d28de27738e Mon Sep 17 00:00:00 2001 From: Jared O'Connell Date: Fri, 16 Feb 2024 11:57:58 -0500 Subject: [PATCH 13/29] Address review comments --- internal/builtinfunctions/functions.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/builtinfunctions/functions.go b/internal/builtinfunctions/functions.go index 609ce28a..b38b74b0 100644 --- a/internal/builtinfunctions/functions.go +++ b/internal/builtinfunctions/functions.go @@ -283,7 +283,7 @@ func getCeilFunction() schema.CallableFunction { "Returns the least integer value greater than or equal to the input.\n"+ "For example `ceil(1.5)` outputs `2.0`, and `ceil(-1.5)` outputs `-1.0`"+ "Special cases are:\n"+ //nolint:goconst - " ceil(±0) = ±0\n"+ + " ceil(±0) = ±0.0"+ " ceil(±Inf) = ±Inf\n"+ " ceil(NaN) = NaN", ), @@ -310,7 +310,7 @@ func getFloorFunction() schema.CallableFunction { "Returns the greatest integer value less than or equal to the input.\n"+ "For example `floor(1.5)` outputs `1.0`, and `floor(-1.5)` outputs `-2.0`"+ "Special cases are:\n"+ - " floor(±0) = ±0\n"+ + " floor(±0) = ±0.0\n"+ " floor(±Inf) = ±Inf\n"+ " floor(NaN) = NaN", ), @@ -334,10 +334,10 @@ func getRoundFunction() schema.CallableFunction { schema.PointerTo("round"), schema.PointerTo( // Description based on documentation for math.Round - "Returns the nearest integer to the input, rounding half away from zero.\n"+ + "Returns the nearest integer to the input, rounding one-half away from zero.\n"+ "For example `round(1.5)` outputs `2.0`, and `round(-1.5)` outputs `-2.0`"+ "Special cases are:\n"+ - " round(±0) = ±0\n"+ + " round(±0) = ±0.0\n"+ " round(±Inf) = ±Inf\n"+ " round(NaN) = NaN", ), From 453a458797b493181eadf8e1088ed44f18b469e2 Mon Sep 17 00:00:00 2001 From: Jared O'Connell Date: Fri, 16 Feb 2024 12:22:12 -0500 Subject: [PATCH 14/29] Fix linting error and address review comment. --- internal/builtinfunctions/functions.go | 7 ++++--- internal/builtinfunctions/functions_test.go | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/internal/builtinfunctions/functions.go b/internal/builtinfunctions/functions.go index b38b74b0..6dfaa3b5 100644 --- a/internal/builtinfunctions/functions.go +++ b/internal/builtinfunctions/functions.go @@ -97,11 +97,12 @@ func getFloatToIntFunction() schema.CallableFunction { func(a float64) (int64, error) { // Because the type conversion in Go has platform-specific behavior, handle // the special cases explicitly so that we get consistent, portable results. - if math.IsInf(a, 1) { + switch { + case math.IsInf(a, 1): return math.MaxInt64, nil - } else if math.IsInf(a, -1) { + case math.IsInf(a, -1): return math.MinInt64, nil - } else if math.IsNaN(a) { + case math.IsNaN(a): return math.MinInt64, fmt.Errorf("attempted to convert a NaN float to an integer") } return int64(a), nil diff --git a/internal/builtinfunctions/functions_test.go b/internal/builtinfunctions/functions_test.go index d9b74d59..f8d337af 100644 --- a/internal/builtinfunctions/functions_test.go +++ b/internal/builtinfunctions/functions_test.go @@ -545,7 +545,7 @@ var testData = map[string]struct { "splitString", []any{"", ","}, false, - make([]string, 1), // The size appears to matter with DeepEquals + []string{""}, }, "split-string-2": { "splitString", From b01e7d84c8a06e4b6983c8d4be3ca9052112dfaa Mon Sep 17 00:00:00 2001 From: Jared O'Connell Date: Fri, 16 Feb 2024 13:31:14 -0500 Subject: [PATCH 15/29] Change descriptions for functions --- internal/builtinfunctions/functions.go | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/internal/builtinfunctions/functions.go b/internal/builtinfunctions/functions.go index 6dfaa3b5..514daa9b 100644 --- a/internal/builtinfunctions/functions.go +++ b/internal/builtinfunctions/functions.go @@ -62,7 +62,7 @@ func getIntToFloatFunction() schema.CallableFunction { false, schema.NewDisplayValue( schema.PointerTo("intToFloat"), - schema.PointerTo("Converts an integer type into a floating point type."), + schema.PointerTo("Returns a floating-point representation of the inputted integer."), nil, ), func(a int64) float64 { @@ -84,7 +84,8 @@ func getFloatToIntFunction() schema.CallableFunction { schema.NewDisplayValue( schema.PointerTo("floatToInt"), schema.PointerTo( - "Converts a float type into an integer type by discarding the fraction."+ + "Returns an integer representation of the inputted floating-point number.\n"+ + " It does this by discarding the the fraction.\n"+ " In other words, it is rounded to the nearest integer towards zero.\n"+ "Special cases:\n"+ " +Inf outputs the maximum signed 64-bit integer (9223372036854775807)\n"+ @@ -123,7 +124,7 @@ func getIntToStringFunction() schema.CallableFunction { schema.NewDisplayValue( schema.PointerTo("intToString"), schema.PointerTo( - "Converts an integer to a string whose characters represent that integer in base-10.\n"+ + "Returns a string whose characters represent the inputted integer in base-10.\n"+ "For example, an input of `55` will output `\"55\"`", ), nil, @@ -147,8 +148,8 @@ func getFloatToStringFunction() schema.CallableFunction { schema.NewDisplayValue( schema.PointerTo("floatToString"), schema.PointerTo( - "Converts a floating point number to a string whose characters"+ - "represent that number in base-10 as as simple decimal.\n"+ + "Returns a string whose characters represent the inputted floating-point\n"+ + "number in base-10 without scientific notation.\n"+ "For example, an input of `5000.5` will output `\"5000.5\"`", ), nil, @@ -195,7 +196,8 @@ func getStringToIntFunction() schema.CallableFunction { schema.NewDisplayValue( schema.PointerTo("stringToInt"), schema.PointerTo( - "Interprets the string as a base-10 integer. Will fail if the input is not a valid integer.", + "Returns an integer by interpreting the inputted string as\n"+ + "a base-10 integer. Will fail if the input is not a valid integer.", ), nil, ), @@ -218,7 +220,8 @@ func getStringToFloatFunction() schema.CallableFunction { schema.NewDisplayValue( schema.PointerTo("stringToFloat"), schema.PointerTo( - "Converts the input string to a 64-bit floating-point number\n\n"+ + "Returns a floating point number by interpreting the the input\n"+ + "string as a 64-bit floating-point number\n\n"+ "Accepts decimal and hexadecimal floating-point numbers\n"+ "as defined by the Go syntax for floating point literals\n"+ "https://go.dev/ref/spec#Floating-point_literals.\n"+ @@ -253,7 +256,7 @@ func getStringToBoolFunction() schema.CallableFunction { schema.NewDisplayValue( schema.PointerTo("stringToBool"), schema.PointerTo( - "Interprets the input as a boolean.\n"+ + "Returns a boolean by interpreting the input.\n"+ " Accepts `\"1\"`, `\"t\"`, and `\"true\"` for `true`.\n"+ " Accepts `\"0\"`, '\"f\"', and '\"false\"' for `false`.\n"+ "Returns an error for any other input.\n"+ @@ -386,7 +389,7 @@ func getToLowerFunction() schema.CallableFunction { schema.NewDisplayValue( schema.PointerTo("toLower"), schema.PointerTo( - "Outputs the input with Unicode letters mapped to their lower case.", + "Returns a version of the input with Unicode letters mapped to their lower case form.", ), nil, ), @@ -407,7 +410,7 @@ func getToUpperFunction() schema.CallableFunction { schema.NewDisplayValue( schema.PointerTo("toUpper"), schema.PointerTo( - "Outputs the input with Unicode letters mapped to their upper case.", + "Returns a version of the input with Unicode letters mapped to their upper case form.", ), nil, ), @@ -431,7 +434,8 @@ func getSplitStringFunction() schema.CallableFunction { schema.NewDisplayValue( schema.PointerTo("strSplit"), schema.PointerTo( - "Splits the given string with the given separator.\n"+ + "Returns a list of strings by splitting the given string\n"+ + "with the given separator.\n"+ " Param 1: The string to split.\n"+ " Param 2: The separator.", ), From 912c1b54a2b229e5aa804ffef5d3741f61188d96 Mon Sep 17 00:00:00 2001 From: Jared O'Connell Date: Fri, 16 Feb 2024 13:47:10 -0500 Subject: [PATCH 16/29] Address review comment --- internal/builtinfunctions/functions.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/builtinfunctions/functions.go b/internal/builtinfunctions/functions.go index 514daa9b..7159a5c8 100644 --- a/internal/builtinfunctions/functions.go +++ b/internal/builtinfunctions/functions.go @@ -338,7 +338,7 @@ func getRoundFunction() schema.CallableFunction { schema.PointerTo("round"), schema.PointerTo( // Description based on documentation for math.Round - "Returns the nearest integer to the input, rounding one-half away from zero.\n"+ + "Returns the nearest integer to the input, rounding one-half value away from zero.\n"+ "For example `round(1.5)` outputs `2.0`, and `round(-1.5)` outputs `-2.0`"+ "Special cases are:\n"+ " round(±0) = ±0.0\n"+ From 3b3ee56cf8489f2f1a2d6920dc8fe510cba7e686 Mon Sep 17 00:00:00 2001 From: Jared O'Connell Date: Wed, 21 Feb 2024 18:10:42 -0500 Subject: [PATCH 17/29] Added namespaced scopes for step namespaces in input --- workflow/any.go | 8 +++-- workflow/executor.go | 50 +++++++++++++++++++++++++- workflow/executor_test.go | 76 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 131 insertions(+), 3 deletions(-) diff --git a/workflow/any.go b/workflow/any.go index 0040fa3c..ae66b1b0 100644 --- a/workflow/any.go +++ b/workflow/any.go @@ -46,8 +46,12 @@ func (a *anySchemaWithExpressions) Serialize(data any) (any, error) { return a.checkAndConvert(data) } -func (a *anySchemaWithExpressions) ApplyScope(scope schema.Scope) { - a.anySchema.ApplyScope(scope) +func (a *anySchemaWithExpressions) ApplyScope(scope schema.Scope, namespace string) { + a.anySchema.ApplyScope(scope, namespace) +} + +func (a *anySchemaWithExpressions) ValidateReferences() error { + return a.anySchema.ValidateReferences() } func (a *anySchemaWithExpressions) TypeID() schema.TypeID { diff --git a/workflow/executor.go b/workflow/executor.go index dc87ef35..57c0661e 100644 --- a/workflow/executor.go +++ b/workflow/executor.go @@ -120,6 +120,12 @@ func (e *executor) Prepare(workflow *Workflow, workflowContext map[string][]byte return nil, err } + // Apply step lifecycle objects to the input scope + err = e.applyExternalScopes(stepLifecycles, typedInput) + if err != nil { + return nil, err + } + // Stage 3: Construct an internal data model for the output data model // provided by the steps. This is the schema the expressions evaluate // against. You can use this to do static code analysis on the expressions. @@ -221,7 +227,7 @@ func (e *executor) processInput(workflow *Workflow) (schema.Scope, error) { if !ok { return nil, fmt.Errorf("bug: unserialized input is not a scope") } - typedInput.ApplyScope(typedInput) + typedInput.ApplyScope(typedInput, "") return typedInput, nil } @@ -307,6 +313,48 @@ func (e *executor) processSteps( return runnableSteps, stepOutputProperties, stepLifecycles, stepRunData, nil } +func (e *executor) applyExternalScopes( + stepLifecycles map[string]step.Lifecycle[step.LifecycleStageWithSchema], + typedInput schema.Scope, +) error { + allNamespaces := make(map[string]schema.Scope) + for workflowStepID, stepLifecycle := range stepLifecycles { + // The format will be $.steps.step_name.stage. + // First, get the input schema and output schemas, and apply them. + for _, stage := range stepLifecycle.Stages { + prefix := "$.steps." + workflowStepID + "." + stage.ID + "." + // Apply inputs + // Example: $.steps.wait_step.starting.input + for inputID, inputSchema := range stage.InputSchema { + if inputSchema.TypeID() == schema.TypeIDScope { + allNamespaces[prefix+inputID] = inputSchema.Type().(schema.Scope) + } + } + // Apply outputs + // Example: $.steps.wait_step.outputs.success + for outputID, outputSchema := range stage.Outputs { + allNamespaces[prefix+outputID] = outputSchema.Schema() + } + } + } + for namespace, scope := range allNamespaces { + typedInput.ApplyScope(scope, namespace) + } + err := typedInput.ValidateReferences() + if err == nil { + return nil // Success + } + // Without listing the namespaces, it may be hard to debug a workflow, so add that list to the error. + availableObjects := "" + for namespace, scope := range allNamespaces { + availableObjects += "\n" + namespace + ":" + for objectID, _ := range scope.Objects() { + availableObjects += " " + objectID + } + } + return fmt.Errorf("error validating references for workflow input (%w)\nAvailable namespaces and objects:%s", err, availableObjects) +} + // connectStepDependencies connects the steps based on their expressions. func (e *executor) connectStepDependencies( workflow *Workflow, diff --git a/workflow/executor_test.go b/workflow/executor_test.go index fdb90634..e907e525 100644 --- a/workflow/executor_test.go +++ b/workflow/executor_test.go @@ -284,3 +284,79 @@ func TestDependOnNoOutputs(t *testing.T) { assert.Error(t, err) assert.Contains(t, err.Error(), `object wait_1 does not have a property named "deploy"`) } + +var namespacedScopeWorkflowDef = `--- +version: v0.2.0 +input: + root: RootObject + objects: + RootObject: + id: RootObject + properties: + wait_input_pass_through: + required: true + type: + type_id: ref + id: wait-input + namespace: "$.steps.wait_step.starting.input" +steps: + wait_step: + plugin: + src: "n/a" + deployment_type: builtin + step: wait + input: !expr $.input.wait_input_pass_through +output: + result: !expr $.steps.wait_step.outputs +` + +func TestNamespacedObjectsAvailable(t *testing.T) { + preparedWorkflow := assert.NoErrorR[workflow.ExecutableWorkflow](t)( + getTestImplPreparedWorkflow(t, namespacedScopeWorkflowDef), + ) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + outputID, _, err := preparedWorkflow.Execute(ctx, map[string]any{ + "wait_input_pass_through": map[string]any{ + "wait_time_ms": "0", + }, + }) + if err != nil { + t.Fatalf("Error while executing workflow, %e", err) + } + assert.Equals(t, outputID, "success") +} + +var brokenNamespacedScopeWorkflowDef = `--- +version: v0.2.0 +input: + root: RootObject + objects: + RootObject: + id: RootObject + properties: + wait_input_pass_through: + required: true + type: + type_id: ref + id: wait-input + namespace: "$.steps.wait_step.starting.wrong" +steps: + wait_step: + plugin: + src: "n/a" + deployment_type: builtin + step: wait + input: !expr $.input.wait_input_pass_through +output: + result: !expr $.steps.wait_step.outputs +` + +func TestNamespacedInputWrongNamespace(t *testing.T) { + _, err := getTestImplPreparedWorkflow(t, brokenNamespacedScopeWorkflowDef) + assert.Error(t, err) + // Validate that the error is the expected one. + assert.Contains(t, err.Error(), `could not find an object with ID "wait-input"`) + // Validate that the correct namespace is listed + assert.Contains(t, err.Error(), "$.steps.wait_step.starting.input") +} From 05c45a2652e29536ed26521cfe243ff7e072fe91 Mon Sep 17 00:00:00 2001 From: Jared O'Connell Date: Thu, 22 Feb 2024 11:00:28 -0500 Subject: [PATCH 18/29] Use default namespace constant --- workflow/executor.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/workflow/executor.go b/workflow/executor.go index 57c0661e..137e10d2 100644 --- a/workflow/executor.go +++ b/workflow/executor.go @@ -227,7 +227,7 @@ func (e *executor) processInput(workflow *Workflow) (schema.Scope, error) { if !ok { return nil, fmt.Errorf("bug: unserialized input is not a scope") } - typedInput.ApplyScope(typedInput, "") + typedInput.ApplyScope(typedInput, schema.DEFAULT_NAMESPACE) return typedInput, nil } From ab751ada363ad304f9dd8c10a741a85cafb00939 Mon Sep 17 00:00:00 2001 From: Jared O'Connell Date: Thu, 22 Feb 2024 17:28:08 -0500 Subject: [PATCH 19/29] Apply scopes from sub-workflows --- internal/step/registry/registry.go | 2 +- workflow/executor.go | 10 +++++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/internal/step/registry/registry.go b/internal/step/registry/registry.go index 6811caac..2525271d 100644 --- a/internal/step/registry/registry.go +++ b/internal/step/registry/registry.go @@ -104,7 +104,7 @@ type stepRegistry struct { } func (s stepRegistry) Schema() *schema.OneOfSchema[string] { - return schema.NewOneOfStringSchema[any](s.providerSchemas, "kind") + return schema.NewOneOfStringSchema[any](s.providerSchemas, "kind", false) } func (s stepRegistry) SchemaByKind(kind string) (schema.Object, error) { diff --git a/workflow/executor.go b/workflow/executor.go index 137e10d2..9fd394ea 100644 --- a/workflow/executor.go +++ b/workflow/executor.go @@ -326,8 +326,16 @@ func (e *executor) applyExternalScopes( // Apply inputs // Example: $.steps.wait_step.starting.input for inputID, inputSchema := range stage.InputSchema { - if inputSchema.TypeID() == schema.TypeIDScope { + switch inputSchema.TypeID() { + case schema.TypeIDScope: allNamespaces[prefix+inputID] = inputSchema.Type().(schema.Scope) + case schema.TypeIDList: + // foreach is a list, for example. + listSchema := inputSchema.Type().(schema.UntypedList) + if listSchema.Items().TypeID() == schema.TypeIDScope { + // Apply list item type since it's a scope. + allNamespaces[prefix+inputID] = listSchema.Items().(schema.Scope) + } } } // Apply outputs From 45cc29a711bd942731891a84ed05b08119235da5 Mon Sep 17 00:00:00 2001 From: Jared O'Connell Date: Thu, 14 Mar 2024 00:07:31 -0400 Subject: [PATCH 20/29] Update invalid serialization detector for changes in SDK --- go.mod | 2 +- go.sum | 2 ++ internal/util/invalid_serialization_detector.go | 12 ++++++++---- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/go.mod b/go.mod index e92a3815..c7d48455 100644 --- a/go.mod +++ b/go.mod @@ -13,7 +13,7 @@ require ( go.flow.arcalot.io/dockerdeployer v0.6.1 go.flow.arcalot.io/expressions v0.4.1 go.flow.arcalot.io/kubernetesdeployer v0.9.1 - go.flow.arcalot.io/pluginsdk v0.9.0-beta1 + go.flow.arcalot.io/pluginsdk v0.9.0-beta1.0.20240308171151-a206faaeb58d go.flow.arcalot.io/podmandeployer v0.8.1 go.flow.arcalot.io/pythondeployer v0.6.0 go.flow.arcalot.io/testdeployer v0.6.0 diff --git a/go.sum b/go.sum index 305daeca..42a9bd2c 100644 --- a/go.sum +++ b/go.sum @@ -141,6 +141,8 @@ go.flow.arcalot.io/kubernetesdeployer v0.9.1 h1:AGnJFazehAENXxGMCF0Uc7aG9F0Lpvuh go.flow.arcalot.io/kubernetesdeployer v0.9.1/go.mod h1:yvxT3VwmyrlIi4422pxl02z4QeU2Gvbjg5aQB17Ye4s= go.flow.arcalot.io/pluginsdk v0.9.0-beta1 h1:tJwEp92vRJldHMff29Q8vfQB5a7FHe/nn6vyFTC1sik= go.flow.arcalot.io/pluginsdk v0.9.0-beta1/go.mod h1:7HafTRTFTYRbJ4sS/Vn0CFrHlaBpEoyOX4oNf612XJM= +go.flow.arcalot.io/pluginsdk v0.9.0-beta1.0.20240308171151-a206faaeb58d h1:1vHzzzI2z2G3JFofCpHB40n9P7SKIrbFmvEUKHPS56A= +go.flow.arcalot.io/pluginsdk v0.9.0-beta1.0.20240308171151-a206faaeb58d/go.mod h1:7HafTRTFTYRbJ4sS/Vn0CFrHlaBpEoyOX4oNf612XJM= go.flow.arcalot.io/podmandeployer v0.8.1 h1:40UDjR1XOn3zOhkwfxvG1AvY+di+MKpGGyd1O6jyzWk= go.flow.arcalot.io/podmandeployer v0.8.1/go.mod h1:eC6FVbEQXwArSV+U5NePJ5tNbay1+2ZJDamwvcT4Ef8= go.flow.arcalot.io/pythondeployer v0.6.0 h1:ptAurEJ2u2U127nK6Kk7zTelbkk6ipPqZcwnTmqB9vo= diff --git a/internal/util/invalid_serialization_detector.go b/internal/util/invalid_serialization_detector.go index 26455876..299d713b 100644 --- a/internal/util/invalid_serialization_detector.go +++ b/internal/util/invalid_serialization_detector.go @@ -60,7 +60,7 @@ func (d *InvalidSerializationDetectorSchema) UnserializeType(data any) (string, // ValidateCompatibility ensures that the input data or schema is compatible with // the given InvalidSerializationDetectorSchema. -func (d InvalidSerializationDetectorSchema) ValidateCompatibility(_ any) error { +func (d *InvalidSerializationDetectorSchema) ValidateCompatibility(_ any) error { // For convenience, always return "success". return nil } @@ -90,15 +90,19 @@ func (d *InvalidSerializationDetectorSchema) SerializeType(data string) (any, er } // ApplyScope is for applying a scope to the references. Does not apply to this object. -func (d InvalidSerializationDetectorSchema) ApplyScope(_ schema.Scope) {} +func (d *InvalidSerializationDetectorSchema) ApplyScope(_ schema.Scope, _ string) {} + +func (d *InvalidSerializationDetectorSchema) ValidateReferences() error { + return nil +} // TypeID returns the category of type this type is. Returns string because // the valid states of this type include all strings. -func (d InvalidSerializationDetectorSchema) TypeID() schema.TypeID { +func (d *InvalidSerializationDetectorSchema) TypeID() schema.TypeID { return schema.TypeIDString // This is a subset of a string schema. } // ReflectedType returns the reflect.Type for a string. -func (d InvalidSerializationDetectorSchema) ReflectedType() reflect.Type { +func (d *InvalidSerializationDetectorSchema) ReflectedType() reflect.Type { return reflect.TypeOf("") } From f8480a877498a39127e22dc9d7a76b8e85a6cf20 Mon Sep 17 00:00:00 2001 From: Jared O'Connell Date: Thu, 14 Mar 2024 00:23:42 -0400 Subject: [PATCH 21/29] Fix linting errors --- internal/util/invalid_serialization_detector.go | 2 ++ workflow/executor.go | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/util/invalid_serialization_detector.go b/internal/util/invalid_serialization_detector.go index 299d713b..773c53c9 100644 --- a/internal/util/invalid_serialization_detector.go +++ b/internal/util/invalid_serialization_detector.go @@ -92,6 +92,8 @@ func (d *InvalidSerializationDetectorSchema) SerializeType(data string) (any, er // ApplyScope is for applying a scope to the references. Does not apply to this object. func (d *InvalidSerializationDetectorSchema) ApplyScope(_ schema.Scope, _ string) {} +// ValidateReferences validates that all necessary references from scopes have been applied. +// Does not apply to this object. func (d *InvalidSerializationDetectorSchema) ValidateReferences() error { return nil } diff --git a/workflow/executor.go b/workflow/executor.go index d307ad49..39725c07 100644 --- a/workflow/executor.go +++ b/workflow/executor.go @@ -356,7 +356,7 @@ func (e *executor) applyExternalScopes( availableObjects := "" for namespace, scope := range allNamespaces { availableObjects += "\n" + namespace + ":" - for objectID, _ := range scope.Objects() { + for objectID := range scope.Objects() { availableObjects += " " + objectID } } From 5fb8e94d0d20c7be0a5b4848d9f05c6e4602f92e Mon Sep 17 00:00:00 2001 From: Jared O'Connell Date: Fri, 22 Mar 2024 19:36:08 -0400 Subject: [PATCH 22/29] Improved code and added initial tests for namespaced scopes --- workflow/executor.go | 50 ++++++++++++------- workflow/executor_unit_test.go | 88 ++++++++++++++++++++++++++++++++++ 2 files changed, 120 insertions(+), 18 deletions(-) create mode 100644 workflow/executor_unit_test.go diff --git a/workflow/executor.go b/workflow/executor.go index 39725c07..ae2f2ba0 100644 --- a/workflow/executor.go +++ b/workflow/executor.go @@ -325,30 +325,22 @@ func (e *executor) applyExternalScopes( prefix := "$.steps." + workflowStepID + "." + stage.ID + "." // Apply inputs // Example: $.steps.wait_step.starting.input - for inputID, inputSchema := range stage.InputSchema { - switch inputSchema.TypeID() { - case schema.TypeIDScope: - allNamespaces[prefix+inputID] = inputSchema.Type().(schema.Scope) - case schema.TypeIDList: - // foreach is a list, for example. - listSchema := inputSchema.Type().(schema.UntypedList) - if listSchema.Items().TypeID() == schema.TypeIDScope { - // Apply list item type since it's a scope. - allNamespaces[prefix+inputID] = listSchema.Items().(schema.Scope) - } - } - } + addInputNamespacedScopes(allNamespaces, stage, prefix) // Apply outputs // Example: $.steps.wait_step.outputs.success - for outputID, outputSchema := range stage.Outputs { - allNamespaces[prefix+outputID] = outputSchema.Schema() - } + addOutputNamespacedScopes(allNamespaces, stage, prefix) } } + return ApplyAllNamespaces(allNamespaces, typedInput) +} + +func ApplyAllNamespaces(allNamespaces map[string]schema.Scope, scopeToApplyTo schema.Scope) error { + // Just apply all scopes for namespace, scope := range allNamespaces { - typedInput.ApplyScope(scope, namespace) + scopeToApplyTo.ApplyScope(scope, namespace) } - err := typedInput.ValidateReferences() + // Validate references. If that fails, provide a useful error message to the user. + err := scopeToApplyTo.ValidateReferences() if err == nil { return nil // Success } @@ -363,6 +355,28 @@ func (e *executor) applyExternalScopes( return fmt.Errorf("error validating references for workflow input (%w)\nAvailable namespaces and objects:%s", err, availableObjects) } +func addOutputNamespacedScopes(allNamespaces map[string]schema.Scope, stage step.LifecycleStageWithSchema, prefix string) { + for outputID, outputSchema := range stage.Outputs { + allNamespaces[prefix+outputID] = outputSchema.Schema() + } +} + +func addInputNamespacedScopes(allNamespaces map[string]schema.Scope, stage step.LifecycleStageWithSchema, prefix string) { + for inputID, inputSchema := range stage.InputSchema { + switch inputSchema.TypeID() { + case schema.TypeIDScope: + allNamespaces[prefix+inputID] = inputSchema.Type().(schema.Scope) + case schema.TypeIDList: + // foreach is a list, for example. + listSchema := inputSchema.Type().(schema.UntypedList) + if listSchema.Items().TypeID() == schema.TypeIDScope { + // Apply list item type since it's a scope. + allNamespaces[prefix+inputID] = listSchema.Items().(schema.Scope) + } + } + } +} + // connectStepDependencies connects the steps based on their expressions. func (e *executor) connectStepDependencies( workflow *Workflow, diff --git a/workflow/executor_unit_test.go b/workflow/executor_unit_test.go new file mode 100644 index 00000000..46308ef2 --- /dev/null +++ b/workflow/executor_unit_test.go @@ -0,0 +1,88 @@ +package workflow + +import ( + "go.arcalot.io/assert" + "go.flow.arcalot.io/engine/internal/step" + "go.flow.arcalot.io/pluginsdk/schema" + "testing" +) + +var testLifecycleStage = step.LifecycleStageWithSchema{ + LifecycleStage: step.LifecycleStage{}, + InputSchema: map[string]*schema.PropertySchema{ + "scopeA": schema.NewPropertySchema( + schema.NewScopeSchema(schema.NewObjectSchema("testObjectA", map[string]*schema.PropertySchema{})), + nil, + true, + nil, + nil, + nil, + nil, + nil, + ), + "listA": schema.NewPropertySchema( + schema.NewListSchema( + schema.NewScopeSchema( + schema.NewObjectSchema("testObjectB", map[string]*schema.PropertySchema{}), + ), + nil, + nil, + ), + nil, + true, + nil, + nil, + nil, + nil, + nil, + ), + "intA": schema.NewPropertySchema( + schema.NewIntSchema(nil, nil, nil), + nil, + true, + nil, + nil, + nil, + nil, + nil, + ), + }, + Outputs: map[string]*schema.StepOutputSchema{ + "outputC": schema.NewStepOutputSchema( + schema.NewScopeSchema( + schema.NewObjectSchema("testObjectC", map[string]*schema.PropertySchema{}), + ), + nil, + false, + ), + }, +} + +func TestAddOutputNamespacedScopes(t *testing.T) { + allNamespaces := make(map[string]schema.Scope) + expectedPrefix := "TEST_PREFIX_" + addOutputNamespacedScopes(allNamespaces, testLifecycleStage, expectedPrefix) + expectedOutput := "outputC" + expectedNamespace := expectedPrefix + expectedOutput + assert.Equals(t, len(allNamespaces), 1) + assert.MapContainsKey(t, expectedNamespace, allNamespaces) + assert.Equals(t, "testObjectC", allNamespaces[expectedNamespace].Root()) +} + +func TestAddInputNamespacedScopes(t *testing.T) { + allNamespaces := make(map[string]schema.Scope) + expectedPrefix := "TEST_PREFIX_" + expectedInputs := map[string]string{ + "scopeA": "testObjectA", + "listA": "testObjectB", + } + + addInputNamespacedScopes(allNamespaces, testLifecycleStage, expectedPrefix) + assert.Equals(t, len(allNamespaces), 2) + for expectedInput, expectedObject := range expectedInputs { + expectedNamespace := expectedPrefix + expectedInput + assert.MapContainsKey(t, expectedNamespace, allNamespaces) + assert.Equals(t, expectedObject, allNamespaces[expectedNamespace].Root()) + } + +} From 52ede8c09aeef4b020cf2ba3fc0da882ead35d02 Mon Sep 17 00:00:00 2001 From: Jared O'Connell Date: Fri, 29 Mar 2024 17:36:05 -0400 Subject: [PATCH 23/29] Added unit tests for new ApplyScope functions --- workflow/executor.go | 12 +- workflow/executor_unit_test.go | 277 ++++++++++++++++++++++++++++++++- 2 files changed, 282 insertions(+), 7 deletions(-) diff --git a/workflow/executor.go b/workflow/executor.go index ae2f2ba0..98efc896 100644 --- a/workflow/executor.go +++ b/workflow/executor.go @@ -121,7 +121,7 @@ func (e *executor) Prepare(workflow *Workflow, workflowContext map[string][]byte } // Apply step lifecycle objects to the input scope - err = e.applyExternalScopes(stepLifecycles, typedInput) + err = applyLifecycleScopes(stepLifecycles, typedInput) if err != nil { return nil, err } @@ -313,7 +313,7 @@ func (e *executor) processSteps( return runnableSteps, stepOutputProperties, stepLifecycles, stepRunData, nil } -func (e *executor) applyExternalScopes( +func applyLifecycleScopes( stepLifecycles map[string]step.Lifecycle[step.LifecycleStageWithSchema], typedInput schema.Scope, ) error { @@ -325,16 +325,16 @@ func (e *executor) applyExternalScopes( prefix := "$.steps." + workflowStepID + "." + stage.ID + "." // Apply inputs // Example: $.steps.wait_step.starting.input - addInputNamespacedScopes(allNamespaces, stage, prefix) + addInputNamespacedScopes(allNamespaces, stage, prefix+"input.") // Apply outputs // Example: $.steps.wait_step.outputs.success - addOutputNamespacedScopes(allNamespaces, stage, prefix) + addOutputNamespacedScopes(allNamespaces, stage, prefix+"output.") } } - return ApplyAllNamespaces(allNamespaces, typedInput) + return applyAllNamespaces(allNamespaces, typedInput) } -func ApplyAllNamespaces(allNamespaces map[string]schema.Scope, scopeToApplyTo schema.Scope) error { +func applyAllNamespaces(allNamespaces map[string]schema.Scope, scopeToApplyTo schema.Scope) error { // Just apply all scopes for namespace, scope := range allNamespaces { scopeToApplyTo.ApplyScope(scope, namespace) diff --git a/workflow/executor_unit_test.go b/workflow/executor_unit_test.go index 46308ef2..71db565c 100644 --- a/workflow/executor_unit_test.go +++ b/workflow/executor_unit_test.go @@ -1,4 +1,4 @@ -package workflow +package workflow //nolint:testpackage // Tests private members in the packages import ( "go.arcalot.io/assert" @@ -84,5 +84,280 @@ func TestAddInputNamespacedScopes(t *testing.T) { assert.MapContainsKey(t, expectedNamespace, allNamespaces) assert.Equals(t, expectedObject, allNamespaces[expectedNamespace].Root()) } +} + +func TestApplyAllNamespaces_Pass(t *testing.T) { + // In this test, we will call applyAllNamespaces and validate that the two namespaces were applied. + ref1Schema := schema.NewNamespacedRefSchema("scopeTestObjectB", "test-namespace-1", nil) + ref1Property := schema.NewPropertySchema( + ref1Schema, + nil, + true, + nil, + nil, + nil, + nil, + nil, + ) + ref2Schema := schema.NewNamespacedRefSchema("scopeTestObjectC", "test-namespace-2", nil) + ref2Property := schema.NewPropertySchema( + ref2Schema, + nil, + true, + nil, + nil, + nil, + nil, + nil, + ) + testScope := schema.NewScopeSchema( + schema.NewObjectSchema( + "scopeTestObjectA", + map[string]*schema.PropertySchema{ + "ref-b": ref1Property, + "ref-c": ref2Property, + }, + ), + ) + // Validate that the refs do not have their object caches yet + assert.Error(t, testScope.ValidateReferences()) + assert.Panics(t, func() { + ref1Schema.GetObject() + }) + assert.Panics(t, func() { + ref2Schema.GetObject() + }) + allNamespaces := make(map[string]schema.Scope) + allNamespaces["test-namespace-1"] = schema.NewScopeSchema( + schema.NewObjectSchema( + "scopeTestObjectB", map[string]*schema.PropertySchema{}, + ), + ) + allNamespaces["test-namespace-2"] = schema.NewScopeSchema( + schema.NewObjectSchema( + "scopeTestObjectC", map[string]*schema.PropertySchema{}, + ), + ) + // Call the function under test and validate that all caches are set. + assert.NoError(t, applyAllNamespaces(allNamespaces, testScope)) + assert.NoError(t, testScope.ValidateReferences()) + assert.NotNil(t, ref1Schema.GetObject()) + assert.NotNil(t, ref2Schema.GetObject()) +} +func TestApplyAllNamespaces_MissingNamespace(t *testing.T) { + // Test the validation in the applyAllNamespaces function by missing a namespace. + ref1Schema := schema.NewNamespacedRefSchema("scopeTestObjectB", "test-namespace-1", nil) + ref1Property := schema.NewPropertySchema( + ref1Schema, + nil, + true, + nil, + nil, + nil, + nil, + nil, + ) + ref2Schema := schema.NewNamespacedRefSchema("scopeTestObjectC", "test-namespace-2", nil) + ref2Property := schema.NewPropertySchema( + ref2Schema, + nil, + true, + nil, + nil, + nil, + nil, + nil, + ) + testScope := schema.NewScopeSchema( + schema.NewObjectSchema( + "scopeTestObjectA", + map[string]*schema.PropertySchema{ + "ref-b": ref1Property, + "ref-c": ref2Property, + }, + ), + ) + // Validate that the refs do not have their object caches yet + assert.Error(t, testScope.ValidateReferences()) + assert.Panics(t, func() { + ref1Schema.GetObject() + }) + assert.Panics(t, func() { + ref2Schema.GetObject() + }) + // Only add test-namespace-1 + allNamespaces := make(map[string]schema.Scope) + allNamespaces["test-namespace-1"] = schema.NewScopeSchema( + schema.NewObjectSchema( + "scopeTestObjectB", map[string]*schema.PropertySchema{}, + ), + ) + err := applyAllNamespaces(allNamespaces, testScope) + assert.Error(t, err) + assert.Contains(t, err.Error(), "error validating references") + // Ensure the error message lists the single available object. + assert.Contains(t, err.Error(), "scopeTestObjectB") +} + +func TestApplyAllNamespaces_InvalidObject(t *testing.T) { + // Call applyAllNamespaces with an object reference that doesn't match + // the scope applied. This tests that the existing validation works. + ref1Schema := schema.NewNamespacedRefSchema("scopeTestObjectB-wrong", "test-namespace-1", nil) + ref1Property := schema.NewPropertySchema( + ref1Schema, + nil, + true, + nil, + nil, + nil, + nil, + nil, + ) + testScope := schema.NewScopeSchema( + schema.NewObjectSchema( + "scopeTestObjectA", + map[string]*schema.PropertySchema{ + "ref-b": ref1Property, + }, + ), + ) + allNamespaces := make(map[string]schema.Scope) + allNamespaces["test-namespace-1"] = schema.NewScopeSchema( + schema.NewObjectSchema( + "scopeTestObjectB", map[string]*schema.PropertySchema{}, + ), + ) + assert.PanicsContains( + t, + func() { + _ = applyAllNamespaces(allNamespaces, testScope) + }, + "object 'scopeTestObjectB-wrong' not found", + ) +} + +func TestApplyLifecycleScopes_Basic(t *testing.T) { + // This tests applyLifeCycleScopes by calling the function with a simple lifecycle, + // and validating that all references have their objects cached. + stepLifecycles := make(map[string]step.Lifecycle[step.LifecycleStageWithSchema]) + stepLifecycles["exampleStepID"] = step.Lifecycle[step.LifecycleStageWithSchema]{ + InitialStage: "exampleStageID", + Stages: []step.LifecycleStageWithSchema{ + { + LifecycleStage: step.LifecycleStage{ + ID: "exampleStageID", + }, + InputSchema: map[string]*schema.PropertySchema{ + "exampleField": schema.NewPropertySchema( + schema.NewScopeSchema( + schema.NewObjectSchema( + "scopeTestObjectB", map[string]*schema.PropertySchema{}, + ), + ), + nil, + true, + nil, + nil, + nil, + nil, + nil, + ), + }, + Outputs: map[string]*schema.StepOutputSchema{ + "success": { + SchemaValue: schema.NewScopeSchema( + schema.NewObjectSchema( + "successObject", + map[string]*schema.PropertySchema{ + "exampleField": schema.NewPropertySchema( + schema.NewStringSchema(nil, nil, nil), + schema.NewDisplayValue( + schema.PointerTo("Message"), + nil, + nil, + ), + true, + nil, + nil, + nil, + nil, + nil, + ), + }, + ), + ), + ErrorValue: false, + }, + "error": { + SchemaValue: schema.NewScopeSchema( + schema.NewObjectSchema( + "error", + map[string]*schema.PropertySchema{ + "reason": schema.NewPropertySchema( + schema.NewStringSchema(nil, nil, nil), + schema.NewDisplayValue( + schema.PointerTo("Message"), + nil, + nil, + ), + true, + nil, + nil, + nil, + nil, + nil, + ), + }, + ), + ), + ErrorValue: true, + }, + }, + }, + }, + } + ref1Schema := schema.NewNamespacedRefSchema( + "scopeTestObjectB", + "$.steps.exampleStepID.exampleStageID.input.exampleField", + nil, + ) + ref1Property := schema.NewPropertySchema( + ref1Schema, + nil, + true, + nil, + nil, + nil, + nil, + nil, + ) + ref2Schema := schema.NewNamespacedRefSchema( + "successObject", + "$.steps.exampleStepID.exampleStageID.output.success", + nil, + ) + ref2Property := schema.NewPropertySchema( + ref2Schema, + nil, + true, + nil, + nil, + nil, + nil, + nil, + ) + testScope := schema.NewScopeSchema( + schema.NewObjectSchema( + "scopeTestObjectA", + map[string]*schema.PropertySchema{ + "ref-b": ref1Property, + "ref-c": ref2Property, + }, + ), + ) + assert.NoError(t, applyLifecycleScopes(stepLifecycles, testScope)) + assert.NoError(t, testScope.ValidateReferences()) + assert.NotNil(t, ref1Schema.GetObject()) + assert.NotNil(t, ref2Schema.GetObject()) } From 1bd2e22121c9c46b0fad800e9a17ce5c7ef4c8f0 Mon Sep 17 00:00:00 2001 From: Jared O'Connell Date: Wed, 3 Apr 2024 18:20:14 -0400 Subject: [PATCH 24/29] Added integration tests for namespaced scoped workflows --- workflow/executor.go | 4 +- workflow/executor_unit_test.go | 4 +- .../child_workflow_namespaced_scopes.yaml | 22 ++++++ .../parent_workflow_namespaced_scopes_1.yaml | 23 ++++++ workflow/workflow_test.go | 73 +++++++++++++++++++ 5 files changed, 122 insertions(+), 4 deletions(-) create mode 100644 workflow/test_workflows/child_workflow_namespaced_scopes.yaml create mode 100644 workflow/test_workflows/parent_workflow_namespaced_scopes_1.yaml diff --git a/workflow/executor.go b/workflow/executor.go index 98efc896..24fb28db 100644 --- a/workflow/executor.go +++ b/workflow/executor.go @@ -325,10 +325,10 @@ func applyLifecycleScopes( prefix := "$.steps." + workflowStepID + "." + stage.ID + "." // Apply inputs // Example: $.steps.wait_step.starting.input - addInputNamespacedScopes(allNamespaces, stage, prefix+"input.") + addInputNamespacedScopes(allNamespaces, stage, prefix+"inputs.") // Apply outputs // Example: $.steps.wait_step.outputs.success - addOutputNamespacedScopes(allNamespaces, stage, prefix+"output.") + addOutputNamespacedScopes(allNamespaces, stage, prefix+"outputs.") } } return applyAllNamespaces(allNamespaces, typedInput) diff --git a/workflow/executor_unit_test.go b/workflow/executor_unit_test.go index 71db565c..16e9ea23 100644 --- a/workflow/executor_unit_test.go +++ b/workflow/executor_unit_test.go @@ -319,7 +319,7 @@ func TestApplyLifecycleScopes_Basic(t *testing.T) { } ref1Schema := schema.NewNamespacedRefSchema( "scopeTestObjectB", - "$.steps.exampleStepID.exampleStageID.input.exampleField", + "$.steps.exampleStepID.exampleStageID.inputs.exampleField", nil, ) ref1Property := schema.NewPropertySchema( @@ -334,7 +334,7 @@ func TestApplyLifecycleScopes_Basic(t *testing.T) { ) ref2Schema := schema.NewNamespacedRefSchema( "successObject", - "$.steps.exampleStepID.exampleStageID.output.success", + "$.steps.exampleStepID.exampleStageID.outputs.success", nil, ) ref2Property := schema.NewPropertySchema( diff --git a/workflow/test_workflows/child_workflow_namespaced_scopes.yaml b/workflow/test_workflows/child_workflow_namespaced_scopes.yaml new file mode 100644 index 00000000..64e54730 --- /dev/null +++ b/workflow/test_workflows/child_workflow_namespaced_scopes.yaml @@ -0,0 +1,22 @@ +version: v0.2.0 +input: + root: SubWorkflowInput + objects: + SubWorkflowInput: + id: SubWorkflowInput + properties: + wait_input: + type: + type_id: ref + id: wait-input + namespace: $.steps.simple_wait.starting.inputs.input +steps: + simple_wait: + plugin: + src: "n/a" + deployment_type: "builtin" + step: wait + input: !expr $.input.wait_input +outputs: + success: + simple_wait_output: !expr $.steps.simple_wait.outputs.success \ No newline at end of file diff --git a/workflow/test_workflows/parent_workflow_namespaced_scopes_1.yaml b/workflow/test_workflows/parent_workflow_namespaced_scopes_1.yaml new file mode 100644 index 00000000..250df806 --- /dev/null +++ b/workflow/test_workflows/parent_workflow_namespaced_scopes_1.yaml @@ -0,0 +1,23 @@ +version: v0.2.0 +input: + root: ParentWorkflow1Input + objects: + ParentWorkflow1Input: + id: ParentWorkflow1Input + properties: + sub_workflow_input: + type: + type_id: list + items: + type_id: ref + id: SubWorkflowInput + namespace: $.steps.sub_workflow_loop.execute.inputs.items +steps: + sub_workflow_loop: + kind: foreach + items: !expr $.input.sub_workflow_input + workflow: child_workflow_namespaced_scopes.yaml + parallelism: 1 +outputs: + success: + sub_workflow_result: !expr $.steps.sub_workflow_loop.outputs.success \ No newline at end of file diff --git a/workflow/workflow_test.go b/workflow/workflow_test.go index b70650d7..26cca6bb 100644 --- a/workflow/workflow_test.go +++ b/workflow/workflow_test.go @@ -1005,6 +1005,79 @@ func TestWorkflow_Execute_Error_MalformedOutputSchema(t *testing.T) { assert.Contains(t, err.Error(), "bug: output schema cannot unserialize") } +func TestWorkflowWithNamespacedScopes(t *testing.T) { + // Run a workflow where the input uses a reference to one of the workflow's steps. + fileData, err := os.ReadFile("./test_workflows/child_workflow_namespaced_scopes.yaml") + assert.NoError(t, err) + preparedWorkflow := assert.NoErrorR[workflow.ExecutableWorkflow](t)( + getTestImplPreparedWorkflow(t, string(fileData)), + ) + outputID, _, err := preparedWorkflow.Execute(context.Background(), map[string]any{ + "wait_input": map[string]any{"wait_time_ms": 0}, + }) + assert.NoError(t, err) + assert.Equals(t, outputID, "success") +} + +func TestNestedWorkflowWithNamespacedScopes(t *testing.T) { + // Combine namespaced scopes with foreach. Manually create the registry to allow this. + logConfig := log.Config{ + Level: log.LevelDebug, + Destination: log.DestinationStdout, + } + logger := log.New( + logConfig, + ) + cfg := &config.Config{ + Log: logConfig, + } + factories := workflowFactory{ + config: cfg, + } + deployerRegistry := deployerregistry.New( + deployer.Any(testimpl.NewFactory()), + ) + + pluginProvider := assert.NoErrorR[step.Provider](t)( + plugin.New(logger, deployerRegistry, map[string]interface{}{ + "builtin": map[string]any{ + "deployer_name": "test-impl", + "deploy_time": "0", + }, + }), + ) + stepRegistry, err := stepregistry.New( + pluginProvider, + lang.Must2(foreach.New(logger, factories.createYAMLParser, factories.createWorkflow)), + ) + assert.NoError(t, err) + + factories.stepRegistry = stepRegistry + executor := lang.Must2(workflow.NewExecutor( + logger, + cfg, + stepRegistry, + builtinfunctions.GetFunctions(), + )) + parentWorkflowData, err := os.ReadFile("./test_workflows/parent_workflow_namespaced_scopes_1.yaml") + subWorkflowData, err := os.ReadFile("./test_workflows/child_workflow_namespaced_scopes.yaml") + assert.NoError(t, err) + wf := lang.Must2(workflow.NewYAMLConverter(stepRegistry).FromYAML(parentWorkflowData)) + preparedWorkflow := lang.Must2(executor.Prepare(wf, map[string][]byte{ + "child_workflow_namespaced_scopes.yaml": subWorkflowData, + })) + subWorkflowInput := map[string]any{ + "wait_input": map[string]any{"wait_time_ms": 0}, + } + outputID, _, err := preparedWorkflow.Execute(context.Background(), map[string]any{ + "sub_workflow_input": []map[string]any{ + subWorkflowInput, + }, + }) + assert.NoError(t, err) + assert.Equals(t, outputID, "success") +} + func createTestExecutableWorkflow(t *testing.T, workflowStr string, workflowCtx map[string][]byte) (workflow.ExecutableWorkflow, error) { logConfig := log.Config{ Level: log.LevelDebug, From f2875cf60308de98ccab9488d1f97dd8e9daf4e5 Mon Sep 17 00:00:00 2001 From: Jared O'Connell Date: Fri, 5 Apr 2024 14:59:38 -0400 Subject: [PATCH 25/29] Added namespaces for referenced items in scopes --- workflow/executor.go | 26 +++- workflow/executor_unit_test.go | 73 +++++++++++ .../child_workflow_namespaced_scopes.yaml | 22 ---- .../parent_workflow_namespaced_scopes_1.yaml | 23 ---- workflow/workflow_test.go | 124 +++++++++++++++--- 5 files changed, 200 insertions(+), 68 deletions(-) delete mode 100644 workflow/test_workflows/child_workflow_namespaced_scopes.yaml delete mode 100644 workflow/test_workflows/parent_workflow_namespaced_scopes_1.yaml diff --git a/workflow/executor.go b/workflow/executor.go index 24fb28db..ea8efe26 100644 --- a/workflow/executor.go +++ b/workflow/executor.go @@ -357,7 +357,7 @@ func applyAllNamespaces(allNamespaces map[string]schema.Scope, scopeToApplyTo sc func addOutputNamespacedScopes(allNamespaces map[string]schema.Scope, stage step.LifecycleStageWithSchema, prefix string) { for outputID, outputSchema := range stage.Outputs { - allNamespaces[prefix+outputID] = outputSchema.Schema() + addScopesWithReferences(allNamespaces, outputSchema.Schema(), prefix+outputID) } } @@ -365,13 +365,33 @@ func addInputNamespacedScopes(allNamespaces map[string]schema.Scope, stage step. for inputID, inputSchema := range stage.InputSchema { switch inputSchema.TypeID() { case schema.TypeIDScope: - allNamespaces[prefix+inputID] = inputSchema.Type().(schema.Scope) + addScopesWithReferences(allNamespaces, inputSchema.Type().(schema.Scope), prefix+inputID) case schema.TypeIDList: // foreach is a list, for example. listSchema := inputSchema.Type().(schema.UntypedList) if listSchema.Items().TypeID() == schema.TypeIDScope { // Apply list item type since it's a scope. - allNamespaces[prefix+inputID] = listSchema.Items().(schema.Scope) + itemScope := listSchema.Items().(schema.Scope) + addScopesWithReferences(allNamespaces, itemScope, prefix+inputID) + } + } + } +} + +// Adds the scope to the namespace map, as well as all resolved references from external namespaces. +func addScopesWithReferences(allNamespaces map[string]schema.Scope, scope schema.Scope, prefix string) { + // First, just adds the scope + allNamespaces[prefix] = scope + // Next, checks all properties for resolved references that reference objects outside of this scope. + rootObject := scope.Objects()[scope.Root()] + for propertyID, property := range rootObject.Properties() { + if property.Type().TypeID() == schema.TypeIDRef { + refProperty := property.Type().(schema.Ref) + if refProperty.Namespace() != schema.DEFAULT_NAMESPACE && refProperty.ObjectReady() { + // Found a resolved reference with an object that is not included in the scope. Add it to the map. + var referencedObject any = refProperty.GetObject() + refObjectSchema := referencedObject.(*schema.ObjectSchema) + allNamespaces[prefix+"."+propertyID] = schema.NewScopeSchema(refObjectSchema) } } } diff --git a/workflow/executor_unit_test.go b/workflow/executor_unit_test.go index 16e9ea23..fd84d0ed 100644 --- a/workflow/executor_unit_test.go +++ b/workflow/executor_unit_test.go @@ -86,6 +86,79 @@ func TestAddInputNamespacedScopes(t *testing.T) { } } +func TestAddScopesWithReferences(t *testing.T) { + // Test that the scope itself and the resolved references are added. + allNamespaces := make(map[string]schema.Scope) + internalRef := schema.NewRefSchema("scopeTestObjectA", nil) + internalRefProperty := schema.NewPropertySchema( + internalRef, + nil, + true, + nil, + nil, + nil, + nil, + nil, + ) + externalRef1 := schema.NewNamespacedRefSchema("scopeTestObjectB", "test-namespace-1", nil) + appliedExternalRefProperty := schema.NewPropertySchema( + externalRef1, + nil, + true, + nil, + nil, + nil, + nil, + nil, + ) + externalRef2 := schema.NewNamespacedRefSchema("scopeTestObjectB", "$.test-namespace-2", nil) + rootPrefixAppliedExternalRefProperty := schema.NewPropertySchema( + externalRef2, + nil, + true, + nil, + nil, + nil, + nil, + nil, + ) + externalRef3 := schema.NewNamespacedRefSchema("scopeTestObjectC", "test-namespace-3", nil) + notAppliedExternalRefProperty := schema.NewPropertySchema( + externalRef3, + nil, + true, + nil, + nil, + nil, + nil, + nil, + ) + testScope := schema.NewScopeSchema( + schema.NewObjectSchema( + "scopeTestObjectA", + map[string]*schema.PropertySchema{ + "internalRef": internalRefProperty, + "appliedExternalRef": appliedExternalRefProperty, + "rootPrefixAppliedExternalRef": rootPrefixAppliedExternalRefProperty, + "notAppliedExternalRef": notAppliedExternalRefProperty, + }, + ), + ) + scopeToApply := schema.NewScopeSchema( + schema.NewObjectSchema( + "scopeTestObjectB", map[string]*schema.PropertySchema{}, + ), + ) + testScope.ApplyScope(scopeToApply, "test-namespace-1") + testScope.ApplyScope(scopeToApply, "$.test-namespace-2") + testScope.ApplyScope(testScope, schema.DEFAULT_NAMESPACE) + addScopesWithReferences(allNamespaces, testScope, "$") + assert.Equals(t, len(allNamespaces), 3) + assert.MapContainsKey(t, "$", allNamespaces) + assert.MapContainsKey(t, "$.appliedExternalRef", allNamespaces) + assert.MapContainsKey(t, "$.rootPrefixAppliedExternalRef", allNamespaces) +} + func TestApplyAllNamespaces_Pass(t *testing.T) { // In this test, we will call applyAllNamespaces and validate that the two namespaces were applied. ref1Schema := schema.NewNamespacedRefSchema("scopeTestObjectB", "test-namespace-1", nil) diff --git a/workflow/test_workflows/child_workflow_namespaced_scopes.yaml b/workflow/test_workflows/child_workflow_namespaced_scopes.yaml deleted file mode 100644 index 64e54730..00000000 --- a/workflow/test_workflows/child_workflow_namespaced_scopes.yaml +++ /dev/null @@ -1,22 +0,0 @@ -version: v0.2.0 -input: - root: SubWorkflowInput - objects: - SubWorkflowInput: - id: SubWorkflowInput - properties: - wait_input: - type: - type_id: ref - id: wait-input - namespace: $.steps.simple_wait.starting.inputs.input -steps: - simple_wait: - plugin: - src: "n/a" - deployment_type: "builtin" - step: wait - input: !expr $.input.wait_input -outputs: - success: - simple_wait_output: !expr $.steps.simple_wait.outputs.success \ No newline at end of file diff --git a/workflow/test_workflows/parent_workflow_namespaced_scopes_1.yaml b/workflow/test_workflows/parent_workflow_namespaced_scopes_1.yaml deleted file mode 100644 index 250df806..00000000 --- a/workflow/test_workflows/parent_workflow_namespaced_scopes_1.yaml +++ /dev/null @@ -1,23 +0,0 @@ -version: v0.2.0 -input: - root: ParentWorkflow1Input - objects: - ParentWorkflow1Input: - id: ParentWorkflow1Input - properties: - sub_workflow_input: - type: - type_id: list - items: - type_id: ref - id: SubWorkflowInput - namespace: $.steps.sub_workflow_loop.execute.inputs.items -steps: - sub_workflow_loop: - kind: foreach - items: !expr $.input.sub_workflow_input - workflow: child_workflow_namespaced_scopes.yaml - parallelism: 1 -outputs: - success: - sub_workflow_result: !expr $.steps.sub_workflow_loop.outputs.success \ No newline at end of file diff --git a/workflow/workflow_test.go b/workflow/workflow_test.go index 26cca6bb..4e28e292 100644 --- a/workflow/workflow_test.go +++ b/workflow/workflow_test.go @@ -1005,20 +1005,104 @@ func TestWorkflow_Execute_Error_MalformedOutputSchema(t *testing.T) { assert.Contains(t, err.Error(), "bug: output schema cannot unserialize") } +var childNamespacedScopesWorkflow = ` +version: v0.2.0 +input: + root: SubWorkflowInput + objects: + SubWorkflowInput: + id: SubWorkflowInput + properties: + wait_input_property: + type: + type_id: ref + id: wait-input + namespace: $.steps.simple_wait.starting.inputs.input +steps: + simple_wait: + plugin: + src: "n/a" + deployment_type: "builtin" + step: wait + input: !expr $.input.wait_input_property +outputs: + success: + simple_wait_output: !expr $.steps.simple_wait.outputs.success +` + func TestWorkflowWithNamespacedScopes(t *testing.T) { // Run a workflow where the input uses a reference to one of the workflow's steps. - fileData, err := os.ReadFile("./test_workflows/child_workflow_namespaced_scopes.yaml") - assert.NoError(t, err) preparedWorkflow := assert.NoErrorR[workflow.ExecutableWorkflow](t)( - getTestImplPreparedWorkflow(t, string(fileData)), + getTestImplPreparedWorkflow(t, childNamespacedScopesWorkflow), ) outputID, _, err := preparedWorkflow.Execute(context.Background(), map[string]any{ - "wait_input": map[string]any{"wait_time_ms": 0}, + "wait_input_property": map[string]any{"wait_time_ms": 0}, }) assert.NoError(t, err) assert.Equals(t, outputID, "success") } +var parentNestedNamespacedScopesWorkflow1 = ` +version: v0.2.0 +input: + root: ParentWorkflow1Input + objects: + ParentWorkflow1Input: + id: ParentWorkflow1Input + properties: + sub_workflow_input: + type: + type_id: list + items: + # References the input of the sub-workflow. + type_id: ref + id: SubWorkflowInput + namespace: $.steps.sub_workflow_loop.execute.inputs.items +steps: + sub_workflow_loop: + kind: foreach + items: !expr $.input.sub_workflow_input + workflow: child_workflow_namespaced_scopes.yaml + parallelism: 1 +outputs: + success: + sub_workflow_result: !expr $.steps.sub_workflow_loop.outputs.success +` + +var parentNestedNamespacedScopesWorkflow2 = ` +version: v0.2.0 +input: + root: ParentWorkflow1Input + objects: + ParentWorkflow1Input: + id: ParentWorkflow1Input + properties: + sub_workflow_input: + type: + type_id: list + items: + type_id: ref + id: SubWorkflowInput + SubWorkflowInput: + # Re-constructs the object from the sub-workflow, but references the step object used. + id: SubWorkflowInput + properties: + wait_input_property: + type: + type_id: ref + id: wait-input + namespace: $.steps.sub_workflow_loop.execute.inputs.items.wait_input_property +steps: + sub_workflow_loop: + kind: foreach + items: !expr $.input.sub_workflow_input + workflow: child_workflow_namespaced_scopes.yaml + parallelism: 1 +outputs: + success: + sub_workflow_result: !expr $.steps.sub_workflow_loop.outputs.success +` + func TestNestedWorkflowWithNamespacedScopes(t *testing.T) { // Combine namespaced scopes with foreach. Manually create the registry to allow this. logConfig := log.Config{ @@ -1059,23 +1143,23 @@ func TestNestedWorkflowWithNamespacedScopes(t *testing.T) { stepRegistry, builtinfunctions.GetFunctions(), )) - parentWorkflowData, err := os.ReadFile("./test_workflows/parent_workflow_namespaced_scopes_1.yaml") - subWorkflowData, err := os.ReadFile("./test_workflows/child_workflow_namespaced_scopes.yaml") - assert.NoError(t, err) - wf := lang.Must2(workflow.NewYAMLConverter(stepRegistry).FromYAML(parentWorkflowData)) - preparedWorkflow := lang.Must2(executor.Prepare(wf, map[string][]byte{ - "child_workflow_namespaced_scopes.yaml": subWorkflowData, - })) - subWorkflowInput := map[string]any{ - "wait_input": map[string]any{"wait_time_ms": 0}, + for workflowIndex, parentWorkflow := range []string{parentNestedNamespacedScopesWorkflow1, parentNestedNamespacedScopesWorkflow2} { + t.Logf("running parent workflow %d", workflowIndex) + wf := lang.Must2(workflow.NewYAMLConverter(stepRegistry).FromYAML([]byte(parentWorkflow))) + preparedWorkflow := lang.Must2(executor.Prepare(wf, map[string][]byte{ + "child_workflow_namespaced_scopes.yaml": []byte(childNamespacedScopesWorkflow), + })) + subWorkflowInput := map[string]any{ + "wait_input_property": map[string]any{"wait_time_ms": 0}, + } + outputID, _, err := preparedWorkflow.Execute(context.Background(), map[string]any{ + "sub_workflow_input": []map[string]any{ + subWorkflowInput, + }, + }) + assert.NoError(t, err) + assert.Equals(t, outputID, "success") } - outputID, _, err := preparedWorkflow.Execute(context.Background(), map[string]any{ - "sub_workflow_input": []map[string]any{ - subWorkflowInput, - }, - }) - assert.NoError(t, err) - assert.Equals(t, outputID, "success") } func createTestExecutableWorkflow(t *testing.T, workflowStr string, workflowCtx map[string][]byte) (workflow.ExecutableWorkflow, error) { From e5adf71d8d75159b42fba6b03968ecc1945fac66 Mon Sep 17 00:00:00 2001 From: Jared O'Connell Date: Thu, 11 Apr 2024 15:17:23 -0400 Subject: [PATCH 26/29] Upgrade to go SDK pre-release --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 56237d29..8a14fdc5 100644 --- a/go.mod +++ b/go.mod @@ -13,7 +13,7 @@ require ( go.flow.arcalot.io/dockerdeployer v0.6.1 go.flow.arcalot.io/expressions v0.4.1 go.flow.arcalot.io/kubernetesdeployer v0.9.1 - go.flow.arcalot.io/pluginsdk v0.9.0-beta1.0.20240405185120-23eebaa23805 + go.flow.arcalot.io/pluginsdk v0.10.0-beta1 go.flow.arcalot.io/podmandeployer v0.9.0 go.flow.arcalot.io/pythondeployer v0.6.0 go.flow.arcalot.io/testdeployer v0.6.0 diff --git a/go.sum b/go.sum index 4b48a2a2..76cbc224 100644 --- a/go.sum +++ b/go.sum @@ -143,8 +143,8 @@ go.flow.arcalot.io/expressions v0.4.1 h1:WOl3DtDcWAmPKupwYxJV3bVYKPoMgAmQbECfiUg go.flow.arcalot.io/expressions v0.4.1/go.mod h1:FA/50wX1+0iTgW/dFeeE1yOslZSmfBaMNR4IiMYRwxc= go.flow.arcalot.io/kubernetesdeployer v0.9.1 h1:AGnJFazehAENXxGMCF0Uc7aG9F0LpvuhoyQFu8deJG0= go.flow.arcalot.io/kubernetesdeployer v0.9.1/go.mod h1:yvxT3VwmyrlIi4422pxl02z4QeU2Gvbjg5aQB17Ye4s= -go.flow.arcalot.io/pluginsdk v0.9.0-beta1.0.20240405185120-23eebaa23805 h1:M9he1XNjS1csTyQJEUSbehvqyYAykeBn05kcD9Je9tY= -go.flow.arcalot.io/pluginsdk v0.9.0-beta1.0.20240405185120-23eebaa23805/go.mod h1:7HafTRTFTYRbJ4sS/Vn0CFrHlaBpEoyOX4oNf612XJM= +go.flow.arcalot.io/pluginsdk v0.10.0-beta1 h1:VGhGIJTZiAfb9J9NZGgMPFO4wb2i94PEiZ2e+ZkNz4A= +go.flow.arcalot.io/pluginsdk v0.10.0-beta1/go.mod h1:7HafTRTFTYRbJ4sS/Vn0CFrHlaBpEoyOX4oNf612XJM= go.flow.arcalot.io/podmandeployer v0.9.0 h1:BSN/s8BeEUIIqOQm6/I5LOzAUzYFrVO1/3p1hcbWD4g= go.flow.arcalot.io/podmandeployer v0.9.0/go.mod h1:FWwelCoH0jfQEYQAJ5mzLElZIXlRfyLP4osjcuQ6n30= go.flow.arcalot.io/pythondeployer v0.6.0 h1:ptAurEJ2u2U127nK6Kk7zTelbkk6ipPqZcwnTmqB9vo= From e0555d8b5a6c13a26ab537e804c1b1ca90867cbd Mon Sep 17 00:00:00 2001 From: Jared O'Connell Date: Tue, 16 Apr 2024 12:27:29 -0400 Subject: [PATCH 27/29] Addressed review comments --- workflow/executor.go | 42 +++++++++++++++++----------------- workflow/executor_unit_test.go | 13 +++++------ 2 files changed, 27 insertions(+), 28 deletions(-) diff --git a/workflow/executor.go b/workflow/executor.go index ea8efe26..808cd677 100644 --- a/workflow/executor.go +++ b/workflow/executor.go @@ -319,40 +319,44 @@ func applyLifecycleScopes( ) error { allNamespaces := make(map[string]schema.Scope) for workflowStepID, stepLifecycle := range stepLifecycles { - // The format will be $.steps.step_name.stage. - // First, get the input schema and output schemas, and apply them. for _, stage := range stepLifecycle.Stages { prefix := "$.steps." + workflowStepID + "." + stage.ID + "." // Apply inputs - // Example: $.steps.wait_step.starting.input + // Example: $.steps.wait_step.starting.inputs. addInputNamespacedScopes(allNamespaces, stage, prefix+"inputs.") // Apply outputs - // Example: $.steps.wait_step.outputs.success + // Example: $.steps.wait_step.outputs.outputs.success addOutputNamespacedScopes(allNamespaces, stage, prefix+"outputs.") } } return applyAllNamespaces(allNamespaces, typedInput) } +// applyAllNamespaces applies all namespaces to the given scope. +// This function also validates references, and lists the namespaces +// and their objects in the event of a reference failure. func applyAllNamespaces(allNamespaces map[string]schema.Scope, scopeToApplyTo schema.Scope) error { - // Just apply all scopes for namespace, scope := range allNamespaces { scopeToApplyTo.ApplyScope(scope, namespace) } - // Validate references. If that fails, provide a useful error message to the user. err := scopeToApplyTo.ValidateReferences() if err == nil { return nil // Success } - // Without listing the namespaces, it may be hard to debug a workflow, so add that list to the error. + // Now on the error path. Provide useful debug info. availableObjects := "" for namespace, scope := range allNamespaces { - availableObjects += "\n" + namespace + ":" + availableObjects += "\n\t" + namespace + ":" for objectID := range scope.Objects() { availableObjects += " " + objectID } } - return fmt.Errorf("error validating references for workflow input (%w)\nAvailable namespaces and objects:%s", err, availableObjects) + availableObjects += "\n" // Since this is a multi-line error message, ending with a newline is clearer. + return fmt.Errorf( + "error validating references for workflow input (%w)\nAvailable namespaces and objects:%s", + err, + availableObjects, + ) } func addOutputNamespacedScopes(allNamespaces map[string]schema.Scope, stage step.LifecycleStageWithSchema, prefix string) { @@ -362,18 +366,14 @@ func addOutputNamespacedScopes(allNamespaces map[string]schema.Scope, stage step } func addInputNamespacedScopes(allNamespaces map[string]schema.Scope, stage step.LifecycleStageWithSchema, prefix string) { - for inputID, inputSchema := range stage.InputSchema { - switch inputSchema.TypeID() { - case schema.TypeIDScope: - addScopesWithReferences(allNamespaces, inputSchema.Type().(schema.Scope), prefix+inputID) - case schema.TypeIDList: - // foreach is a list, for example. - listSchema := inputSchema.Type().(schema.UntypedList) - if listSchema.Items().TypeID() == schema.TypeIDScope { - // Apply list item type since it's a scope. - itemScope := listSchema.Items().(schema.Scope) - addScopesWithReferences(allNamespaces, itemScope, prefix+inputID) - } + for inputID, inputSchemaProperty := range stage.InputSchema { + var inputSchemaType = inputSchemaProperty.Type() + // Extract item values from lists (like for ForEach) + if inputSchemaType.TypeID() == schema.TypeIDList { + inputSchemaType = inputSchemaType.(schema.UntypedList).Items() + } + if inputSchemaType.TypeID() == schema.TypeIDScope { + addScopesWithReferences(allNamespaces, inputSchemaType.(schema.Scope), prefix+inputID) } } } diff --git a/workflow/executor_unit_test.go b/workflow/executor_unit_test.go index fd84d0ed..8ac3965a 100644 --- a/workflow/executor_unit_test.go +++ b/workflow/executor_unit_test.go @@ -60,10 +60,9 @@ var testLifecycleStage = step.LifecycleStageWithSchema{ func TestAddOutputNamespacedScopes(t *testing.T) { allNamespaces := make(map[string]schema.Scope) - expectedPrefix := "TEST_PREFIX_" - addOutputNamespacedScopes(allNamespaces, testLifecycleStage, expectedPrefix) - expectedOutput := "outputC" - expectedNamespace := expectedPrefix + expectedOutput + namespacePrefix := "TEST_PREFIX_" + addOutputNamespacedScopes(allNamespaces, testLifecycleStage, namespacePrefix) + expectedNamespace := namespacePrefix + "outputC" assert.Equals(t, len(allNamespaces), 1) assert.MapContainsKey(t, expectedNamespace, allNamespaces) assert.Equals(t, "testObjectC", allNamespaces[expectedNamespace].Root()) @@ -71,16 +70,16 @@ func TestAddOutputNamespacedScopes(t *testing.T) { func TestAddInputNamespacedScopes(t *testing.T) { allNamespaces := make(map[string]schema.Scope) - expectedPrefix := "TEST_PREFIX_" + namespacePrefix := "TEST_PREFIX_" expectedInputs := map[string]string{ "scopeA": "testObjectA", "listA": "testObjectB", } - addInputNamespacedScopes(allNamespaces, testLifecycleStage, expectedPrefix) + addInputNamespacedScopes(allNamespaces, testLifecycleStage, namespacePrefix) assert.Equals(t, len(allNamespaces), 2) for expectedInput, expectedObject := range expectedInputs { - expectedNamespace := expectedPrefix + expectedInput + expectedNamespace := namespacePrefix + expectedInput assert.MapContainsKey(t, expectedNamespace, allNamespaces) assert.Equals(t, expectedObject, allNamespaces[expectedNamespace].Root()) } From 3dd60af823b4fe2dfe6705a0f9a580aae3535090 Mon Sep 17 00:00:00 2001 From: Jared O'Connell Date: Tue, 16 Apr 2024 13:55:25 -0400 Subject: [PATCH 28/29] Clarify stage IDs in comment examples --- workflow/executor.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/workflow/executor.go b/workflow/executor.go index 808cd677..0ee0f55a 100644 --- a/workflow/executor.go +++ b/workflow/executor.go @@ -322,10 +322,10 @@ func applyLifecycleScopes( for _, stage := range stepLifecycle.Stages { prefix := "$.steps." + workflowStepID + "." + stage.ID + "." // Apply inputs - // Example: $.steps.wait_step.starting.inputs. + // Example with stage starting: $.steps.wait_step.starting.inputs. addInputNamespacedScopes(allNamespaces, stage, prefix+"inputs.") // Apply outputs - // Example: $.steps.wait_step.outputs.outputs.success + // Example with stage outputs: $.steps.wait_step.outputs.outputs.success addOutputNamespacedScopes(allNamespaces, stage, prefix+"outputs.") } } From 3cf14b6a00499b981dc55c2d4e56041d9a5a7ce5 Mon Sep 17 00:00:00 2001 From: Jared O'Connell Date: Tue, 16 Apr 2024 15:24:23 -0400 Subject: [PATCH 29/29] Addressed review comments --- workflow/executor.go | 8 +++---- workflow/executor_unit_test.go | 38 ++++++++++++++++++++++++++++++---- 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/workflow/executor.go b/workflow/executor.go index 0ee0f55a..d41aee39 100644 --- a/workflow/executor.go +++ b/workflow/executor.go @@ -322,10 +322,10 @@ func applyLifecycleScopes( for _, stage := range stepLifecycle.Stages { prefix := "$.steps." + workflowStepID + "." + stage.ID + "." // Apply inputs - // Example with stage starting: $.steps.wait_step.starting.inputs. + // Example with stage "starting": $.steps.wait_step.starting.inputs. addInputNamespacedScopes(allNamespaces, stage, prefix+"inputs.") // Apply outputs - // Example with stage outputs: $.steps.wait_step.outputs.outputs.success + // Example with stage "outputs": $.steps.wait_step.outputs.outputs. addOutputNamespacedScopes(allNamespaces, stage, prefix+"outputs.") } } @@ -387,8 +387,8 @@ func addScopesWithReferences(allNamespaces map[string]schema.Scope, scope schema for propertyID, property := range rootObject.Properties() { if property.Type().TypeID() == schema.TypeIDRef { refProperty := property.Type().(schema.Ref) - if refProperty.Namespace() != schema.DEFAULT_NAMESPACE && refProperty.ObjectReady() { - // Found a resolved reference with an object that is not included in the scope. Add it to the map. + if refProperty.Namespace() != schema.DEFAULT_NAMESPACE { + // Found a reference to an object that is not included in the scope. Add it to the map. var referencedObject any = refProperty.GetObject() refObjectSchema := referencedObject.(*schema.ObjectSchema) allNamespaces[prefix+"."+propertyID] = schema.NewScopeSchema(refObjectSchema) diff --git a/workflow/executor_unit_test.go b/workflow/executor_unit_test.go index 8ac3965a..cc533c9e 100644 --- a/workflow/executor_unit_test.go +++ b/workflow/executor_unit_test.go @@ -85,6 +85,36 @@ func TestAddInputNamespacedScopes(t *testing.T) { } } +func TestAddScopesWithMissingCache(t *testing.T) { + allNamespaces := make(map[string]schema.Scope) + externalRef3 := schema.NewNamespacedRefSchema("scopeTestObjectC", "not-applied-namespace", nil) + notAppliedExternalRefProperty := schema.NewPropertySchema( + externalRef3, + nil, + true, + nil, + nil, + nil, + nil, + nil, + ) + testScope := schema.NewScopeSchema( + schema.NewObjectSchema( + "scopeTestObjectA", + map[string]*schema.PropertySchema{ + "notAppliedExternalRef": notAppliedExternalRefProperty, + }, + ), + ) + assert.PanicsContains( + t, + func() { + addScopesWithReferences(allNamespaces, testScope, "$") + }, + "scope with namespace \"not-applied-namespace\" was not applied successfully", + ) +} + func TestAddScopesWithReferences(t *testing.T) { // Test that the scope itself and the resolved references are added. allNamespaces := make(map[string]schema.Scope) @@ -121,9 +151,9 @@ func TestAddScopesWithReferences(t *testing.T) { nil, nil, ) - externalRef3 := schema.NewNamespacedRefSchema("scopeTestObjectC", "test-namespace-3", nil) - notAppliedExternalRefProperty := schema.NewPropertySchema( - externalRef3, + // This one shouldn't add a namespace. + nonRefProperty := schema.NewPropertySchema( + schema.NewStringSchema(nil, nil, nil), nil, true, nil, @@ -139,7 +169,7 @@ func TestAddScopesWithReferences(t *testing.T) { "internalRef": internalRefProperty, "appliedExternalRef": appliedExternalRefProperty, "rootPrefixAppliedExternalRef": rootPrefixAppliedExternalRefProperty, - "notAppliedExternalRef": notAppliedExternalRefProperty, + "nonRefProperty": nonRefProperty, }, ), )