diff --git a/arazzo/coverage_test.go b/arazzo/coverage_test.go index cd4b3e5d..d8e2a25d 100644 --- a/arazzo/coverage_test.go +++ b/arazzo/coverage_test.go @@ -392,6 +392,16 @@ func TestEvaluateSimpleOperand_Integer(t *testing.T) { assert.Equal(t, int64(42), val) } +func TestEvaluateSimpleOperand_ZeroAndOneAreIntegers(t *testing.T) { + val, err := evaluateSimpleOperand("0", nil, nil) + require.NoError(t, err) + assert.Equal(t, int64(0), val) + + val, err = evaluateSimpleOperand("1", nil, nil) + require.NoError(t, err) + assert.Equal(t, int64(1), val) +} + func TestEvaluateSimpleOperand_NegativeInteger(t *testing.T) { val, err := evaluateSimpleOperand("-5", nil, nil) require.NoError(t, err) diff --git a/arazzo/criterion.go b/arazzo/criterion.go index 99dfd261..076a4c2f 100644 --- a/arazzo/criterion.go +++ b/arazzo/criterion.go @@ -8,9 +8,11 @@ import ( "regexp" "strconv" "strings" + "unicode" "github.com/pb33f/jsonpath/pkg/jsonpath" jsonpathconfig "github.com/pb33f/jsonpath/pkg/jsonpath/config" + "github.com/pb33f/libopenapi/arazzo/expression" high "github.com/pb33f/libopenapi/datamodel/high/arazzo" ) @@ -101,33 +103,9 @@ func evaluateSimpleConditionString(condition string, exprCtx *expression.Context if trimmed == "" { return false, nil } - - if b, err := strconv.ParseBool(trimmed); err == nil { - return b, nil - } - - leftRaw, op, rightRaw, found := expression.SplitSimpleCondition(trimmed) - if found { - left, err := evaluateSimpleOperand(leftRaw, exprCtx, caches) - if err != nil { - return false, err - } - right, err := evaluateSimpleOperand(rightRaw, exprCtx, caches) - if err != nil { - return false, err - } - return compareSimpleValues(left, right, op) - } - - val, err := evaluateSimpleOperand(trimmed, exprCtx, caches) - if err != nil { - return false, err - } - b, ok := val.(bool) - if !ok { - return false, fmt.Errorf("simple condition %q did not evaluate to a boolean", condition) - } - return b, nil + // Compound simple conditions may combine literals, operators, and runtime + // expressions with &&, ||, and parentheses (Arazzo 1.0.1 Criterion Object). + return evaluateBooleanExpr(trimmed, exprCtx, caches) } func evaluateSimpleOperand(operand string, exprCtx *expression.Context, caches *criterionCaches) (any, error) { @@ -145,20 +123,42 @@ func evaluateSimpleOperand(operand string, exprCtx *expression.Context, caches * return op[1 : len(op)-1], nil } - if b, err := strconv.ParseBool(op); err == nil { - return b, nil - } + // Parse numbers before bools. strconv.ParseBool accepts "0" and "1", which + // would make `$response.body#/count > 0` compare a number to false. if i, err := strconv.ParseInt(op, 10, 64); err == nil { return i, nil } if f, err := strconv.ParseFloat(op, 64); err == nil { return f, nil } + if op == "true" || op == "True" || op == "TRUE" { + return true, nil + } + if op == "false" || op == "False" || op == "FALSE" { + return false, nil + } return op, nil } func compareSimpleValues(left, right any, op string) (bool, error) { + // A null or missing operand has no ordering, so only equality is meaningful. + // Without this, nil falls through to the string comparison below as "", + // which sorts after digits and makes `$response.body#/count > 0` pass on a 204. + if left == nil || right == nil { + bothNil := left == nil && right == nil + switch op { + case "==": + return bothNil, nil + case "!=": + return !bothNil, nil + case ">", "<", ">=", "<=": + return false, nil + default: + return false, fmt.Errorf("unsupported operator %q", op) + } + } + if ln, lok := numericValue(left); lok { if rn, rok := numericValue(right); rok { switch op { @@ -352,3 +352,271 @@ func sprintValue(v any) string { return fmt.Sprintf("%v", v) } } + +type boolTokenKind int + +const ( + tokEOF boolTokenKind = iota + tokOperand + tokAnd + tokOr + tokEq + tokNeq + tokGt + tokLt + tokGte + tokLte + tokLParen + tokRParen +) + +type boolToken struct { + kind boolTokenKind + raw string +} + +func evaluateBooleanExpr(condition string, exprCtx *expression.Context, caches *criterionCaches) (bool, error) { + tokens, err := tokenizeSimpleCondition(condition) + if err != nil { + return false, err + } + p := &boolParser{ + tokens: tokens, + exprCtx: exprCtx, + caches: caches, + input: condition, + } + value, err := p.parseOr() + if err != nil { + return false, err + } + if p.peek().kind != tokEOF { + return false, fmt.Errorf("unexpected token %q in simple condition %q", p.peek().raw, condition) + } + return value, nil +} + +type boolParser struct { + tokens []boolToken + pos int + exprCtx *expression.Context + caches *criterionCaches + input string +} + +func (p *boolParser) peek() boolToken { + if p.pos >= len(p.tokens) { + return boolToken{kind: tokEOF} + } + return p.tokens[p.pos] +} + +func (p *boolParser) next() boolToken { + tok := p.peek() + if tok.kind != tokEOF { + p.pos++ + } + return tok +} + +func (p *boolParser) parseOr() (bool, error) { + left, err := p.parseAnd() + if err != nil { + return false, err + } + for p.peek().kind == tokOr { + p.next() + right, err := p.parseAnd() + if err != nil { + return false, err + } + left = left || right + } + return left, nil +} + +func (p *boolParser) parseAnd() (bool, error) { + left, err := p.parseComparison() + if err != nil { + return false, err + } + for p.peek().kind == tokAnd { + p.next() + right, err := p.parseComparison() + if err != nil { + return false, err + } + left = left && right + } + return left, nil +} + +func (p *boolParser) parseComparison() (bool, error) { + left, err := p.parsePrimary() + if err != nil { + return false, err + } + if op, ok := comparisonOp(p.peek().kind); ok { + p.next() + right, err := p.parsePrimary() + if err != nil { + return false, err + } + return compareSimpleValues(left, right, op) + } + // A missing runtime value (parsePrimary maps it to nil) or an explicit null + // used as a bare operand is falsy, so `$statusCode == 204 || $response.body#/flag` + // still passes on an empty 204 response. + if left == nil { + return false, nil + } + b, ok := left.(bool) + if !ok { + return false, fmt.Errorf("simple condition %q did not evaluate to a boolean", p.input) + } + return b, nil +} + +func (p *boolParser) parsePrimary() (any, error) { + tok := p.next() + switch tok.kind { + case tokLParen: + value, err := p.parseOr() + if err != nil { + return nil, err + } + if p.next().kind != tokRParen { + return nil, fmt.Errorf("missing closing parenthesis in simple condition %q", p.input) + } + return value, nil + case tokOperand: + value, err := evaluateSimpleOperand(tok.raw, p.exprCtx, p.caches) + if err != nil && isOptionalRuntimeValueError(err) { + return nil, nil + } + return value, err + default: + return nil, fmt.Errorf("unexpected token %q in simple condition %q", tok.raw, p.input) + } +} + +func comparisonOp(kind boolTokenKind) (string, bool) { + switch kind { + case tokEq: + return "==", true + case tokNeq: + return "!=", true + case tokGte: + return ">=", true + case tokLte: + return "<=", true + case tokGt: + return ">", true + case tokLt: + return "<", true + default: + return "", false + } +} + +func tokenizeSimpleCondition(input string) ([]boolToken, error) { + var tokens []boolToken + i := 0 + for i < len(input) { + for i < len(input) && unicode.IsSpace(rune(input[i])) { + i++ + } + if i >= len(input) { + break + } + rest := input[i:] + switch { + case strings.HasPrefix(rest, "&&"): + tokens = append(tokens, boolToken{kind: tokAnd, raw: "&&"}) + i += 2 + case strings.HasPrefix(rest, "||"): + tokens = append(tokens, boolToken{kind: tokOr, raw: "||"}) + i += 2 + case strings.HasPrefix(rest, "=="): + tokens = append(tokens, boolToken{kind: tokEq, raw: "=="}) + i += 2 + case strings.HasPrefix(rest, "!="): + tokens = append(tokens, boolToken{kind: tokNeq, raw: "!="}) + i += 2 + case strings.HasPrefix(rest, ">="): + tokens = append(tokens, boolToken{kind: tokGte, raw: ">="}) + i += 2 + case strings.HasPrefix(rest, "<="): + tokens = append(tokens, boolToken{kind: tokLte, raw: "<="}) + i += 2 + case rest[0] == '>': + tokens = append(tokens, boolToken{kind: tokGt, raw: ">"}) + i++ + case rest[0] == '<': + tokens = append(tokens, boolToken{kind: tokLt, raw: "<"}) + i++ + case rest[0] == '(': + tokens = append(tokens, boolToken{kind: tokLParen, raw: "("}) + i++ + case rest[0] == ')': + tokens = append(tokens, boolToken{kind: tokRParen, raw: ")"}) + i++ + default: + start := i + if rest[0] == '\'' || rest[0] == '"' { + q := rest[0] + i++ + for i < len(input) && input[i] != q { + i++ + } + if i >= len(input) { + return nil, fmt.Errorf("unterminated string in simple condition %q", input) + } + i++ + } else if rest[0] == '$' { + i++ + for i < len(input) && !isBoolOpStart(input[i:]) && input[i] != '(' && input[i] != ')' && !unicode.IsSpace(rune(input[i])) { + i++ + } + } else { + for i < len(input) && !isBoolOpStart(input[i:]) && input[i] != '(' && input[i] != ')' && !unicode.IsSpace(rune(input[i])) { + i++ + } + } + tokens = append(tokens, boolToken{kind: tokOperand, raw: input[start:i]}) + } + } + return tokens, nil +} + +func isBoolOpStart(s string) bool { + if s == "" { + return false + } + return strings.HasPrefix(s, "&&") || + strings.HasPrefix(s, "||") || + strings.HasPrefix(s, "==") || + strings.HasPrefix(s, "!=") || + strings.HasPrefix(s, ">=") || + strings.HasPrefix(s, "<=") || + s[0] == '>' || + s[0] == '<' +} + +// isOptionalRuntimeValueError must only be called with a non-nil error. +func isOptionalRuntimeValueError(err error) bool { + msg := err.Error() + // Only values that legitimately vary per response (headers, params, body + // pointers) are optional. Unknown steps, workflows, inputs, outputs, or + // components are authoring errors and must still fail. + return strings.Contains(msg, "request header ") || + strings.Contains(msg, "request query parameter ") || + strings.Contains(msg, "request path parameter ") || + strings.Contains(msg, "response header ") || + strings.Contains(msg, "response query parameter ") || + strings.Contains(msg, "response path parameter ") || + strings.Contains(msg, "JSON pointer segment ") || + strings.Contains(msg, "no response body available") || + strings.Contains(msg, "no request body available") || + strings.Contains(msg, "no response headers available") +} diff --git a/arazzo/criterion_test.go b/arazzo/criterion_test.go index 85e219b4..cd5875cf 100644 --- a/arazzo/criterion_test.go +++ b/arazzo/criterion_test.go @@ -6,6 +6,7 @@ package arazzo import ( "testing" + "github.com/pb33f/go-yaml" "github.com/pb33f/libopenapi/arazzo/expression" high "github.com/pb33f/libopenapi/datamodel/high/arazzo" "github.com/pb33f/testify/assert" @@ -35,3 +36,204 @@ func TestEvaluateCriterion_SimpleCondition_StringComparison(t *testing.T) { require.NoError(t, err) assert.True(t, ok) } + +func TestEvaluateCriterion_SimpleCondition_BooleanOperators(t *testing.T) { + criterion := &high.Criterion{ + Condition: `$statusCode == 204 || ($statusCode == 200 && ($response.body#/challengeName == 'NEW_PASSWORD_REQUIRED' || $response.body#/challengeName == 'SOFTWARE_TOKEN_MFA'))`, + } + + ok, err := EvaluateCriterion(criterion, &expression.Context{StatusCode: 204}) + require.NoError(t, err) + assert.True(t, ok, "204 with an empty body should pass") + + ok, err = EvaluateCriterion(criterion, &expression.Context{ + StatusCode: 200, + ResponseBody: yamlMapping(t, map[string]any{"challengeName": "NEW_PASSWORD_REQUIRED"}), + }) + require.NoError(t, err) + assert.True(t, ok) + + ok, err = EvaluateCriterion(criterion, &expression.Context{ + StatusCode: 200, + ResponseBody: yamlMapping(t, map[string]any{"challengeName": "MFA_SETUP"}), + }) + require.NoError(t, err) + assert.False(t, ok, "unsupported challenge must fail") +} + +func TestEvaluateCriterion_NumericLiteralZeroIsNotBoolean(t *testing.T) { + body := yamlMapping(t, map[string]any{ + "pagination": map[string]any{"totalCount": 1}, + }) + ok, err := EvaluateCriterion(&high.Criterion{ + Condition: "$response.body#/pagination/totalCount > 0", + }, &expression.Context{ResponseBody: body}) + require.NoError(t, err) + assert.True(t, ok) + + empty := yamlMapping(t, map[string]any{ + "pagination": map[string]any{"totalCount": 0}, + }) + ok, err = EvaluateCriterion(&high.Criterion{ + Condition: "$response.body#/pagination/totalCount > 0", + }, &expression.Context{ResponseBody: empty}) + require.NoError(t, err) + assert.False(t, ok) +} + +func yamlMapping(t *testing.T, v any) *yaml.Node { + t.Helper() + b, err := yaml.Marshal(v) + require.NoError(t, err) + var node yaml.Node + require.NoError(t, yaml.Unmarshal(b, &node)) + if node.Kind == yaml.DocumentNode && len(node.Content) > 0 { + return node.Content[0] + } + return &node +} + +// A missing runtime value resolves to null. Null has no ordering, so every +// ordering comparison against it must fail; only == and != are meaningful. +func TestEvaluateCriterion_MissingValueComparisons(t *testing.T) { + cases := []struct { + condition string + want bool + }{ + {"$response.body#/count > 0", false}, + {"$response.body#/count >= 0", false}, + {"$response.body#/count < 5", false}, + {"$response.body#/count <= 0", false}, + {"$response.body#/count == 0", false}, + {"$response.body#/count != 0", true}, + {"$response.body#/name == ''", false}, + {"$response.header.X-Count > 0", false}, + {"$statusCode == 204 && $response.body#/count > 0", false}, + {"$statusCode == 204 || $response.body#/count > 0", true}, + {"$response.body#/count > 0 || $statusCode == 204", true}, + } + for _, tc := range cases { + t.Run(tc.condition, func(t *testing.T) { + ok, err := EvaluateCriterion(&high.Criterion{Condition: tc.condition}, + &expression.Context{StatusCode: 204}) + require.NoError(t, err) + assert.Equal(t, tc.want, ok) + }) + } +} + +// An explicit JSON null in a body has no ordering and is falsy as a bare operand. +func TestEvaluateCriterion_NullBodyValueComparisons(t *testing.T) { + body := yamlMapping(t, map[string]any{"count": nil}) + ctx := &expression.Context{StatusCode: 200, ResponseBody: body} + + ok, err := EvaluateCriterion(&high.Criterion{Condition: "$response.body#/count > 0"}, ctx) + require.NoError(t, err) + assert.False(t, ok) + + ok, err = EvaluateCriterion(&high.Criterion{Condition: "$response.body#/count != 0"}, ctx) + require.NoError(t, err) + assert.True(t, ok) + + // A bare null operand is falsy, leaving the rest of the condition to decide. + ok, err = EvaluateCriterion(&high.Criterion{Condition: "$response.body#/count"}, ctx) + require.NoError(t, err) + assert.False(t, ok) + + ok, err = EvaluateCriterion(&high.Criterion{Condition: "$response.body#/count || $statusCode == 200"}, ctx) + require.NoError(t, err) + assert.True(t, ok) +} + +func TestCompareSimpleValues_Nil(t *testing.T) { + ok, err := compareSimpleValues(nil, nil, "==") + require.NoError(t, err) + assert.True(t, ok) + + ok, err = compareSimpleValues(nil, nil, "!=") + require.NoError(t, err) + assert.False(t, ok) + + ok, err = compareSimpleValues(nil, int64(1), ">") + require.NoError(t, err) + assert.False(t, ok) + + _, err = compareSimpleValues(nil, int64(1), "~=") + assert.Error(t, err) +} + +// A missing value used as a bare boolean operand is falsy rather than an error, +// so the rest of the condition still decides the outcome. +func TestEvaluateCriterion_MissingBareOperandIsFalse(t *testing.T) { + cases := []struct { + condition string + want bool + }{ + {"$response.body#/flag", false}, + {"$response.header.X-Flag", false}, + {"$statusCode == 204 || $response.body#/flag", true}, + {"$response.body#/flag || $statusCode == 204", true}, + {"$statusCode == 204 && $response.body#/flag", false}, + {"($response.body#/flag) || $statusCode == 204", true}, + } + for _, tc := range cases { + t.Run(tc.condition, func(t *testing.T) { + ok, err := EvaluateCriterion(&high.Criterion{Condition: tc.condition}, + &expression.Context{StatusCode: 204}) + require.NoError(t, err) + assert.Equal(t, tc.want, ok) + }) + } +} + +// A present but non-boolean bare operand is still an authoring error. +func TestEvaluateCriterion_NonBooleanBareOperandErrors(t *testing.T) { + _, err := EvaluateCriterion(&high.Criterion{Condition: "$statusCode"}, + &expression.Context{StatusCode: 204}) + require.Error(t, err) + assert.Contains(t, err.Error(), "did not evaluate to a boolean") +} + +func TestEvaluateCriterion_SimpleConditionSyntaxErrors(t *testing.T) { + cases := []struct { + condition string + errText string + }{ + {"$statusCode == 'abc", "unterminated string"}, + {"$statusCode == 204 )", `unexpected token ")"`}, + {"($statusCode == 204", "missing closing parenthesis"}, + {"&& $statusCode == 204", `unexpected token "&&"`}, + {"$statusCode ==", "unexpected token"}, + // Non-optional errors still propagate from every position in the parse. + {"($steps.nope.outputs.x == 1)", `step "nope" not found`}, + {"$statusCode == 204 || $steps.nope.outputs.x == 1", `step "nope" not found`}, + {"$statusCode == 204 && $steps.nope.outputs.x == 1", `step "nope" not found`}, + {"$steps.nope.outputs.x == 1 || $statusCode == 204", `step "nope" not found`}, + {"$statusCode == $steps.nope.outputs.x", `step "nope" not found`}, + } + for _, tc := range cases { + t.Run(tc.condition, func(t *testing.T) { + _, err := EvaluateCriterion(&high.Criterion{Condition: tc.condition}, + &expression.Context{StatusCode: 204, Steps: map[string]*expression.StepContext{}}) + require.Error(t, err) + assert.Contains(t, err.Error(), tc.errText) + }) + } +} + +func TestTokenizeSimpleCondition_TrailingWhitespace(t *testing.T) { + tokens, err := tokenizeSimpleCondition("$statusCode == 204 \t") + require.NoError(t, err) + require.Len(t, tokens, 3) + assert.Equal(t, tokOperand, tokens[0].kind) + assert.Equal(t, tokEq, tokens[1].kind) + assert.Equal(t, "204", tokens[2].raw) +} + +func TestIsBoolOpStart(t *testing.T) { + assert.False(t, isBoolOpStart("")) + assert.False(t, isBoolOpStart("abc")) + for _, s := range []string{"&&", "||", "==", "!=", ">=", "<=", ">", "<"} { + assert.True(t, isBoolOpStart(s+" x"), s) + } +} diff --git a/arazzo/execution_preflight_test.go b/arazzo/execution_preflight_test.go index 61da7f21..f55a212e 100644 --- a/arazzo/execution_preflight_test.go +++ b/arazzo/execution_preflight_test.go @@ -461,3 +461,77 @@ func TestEngine_WorkflowOutputsAreNotPartiallyPopulatedOnEvaluationFailure(t *te assert.Empty(t, exprCtx.Outputs, "a failed workflow output set must not leak into the expression context") } + +// On an empty response (e.g. HTTP 204) a missing body or header value publishes a +// nil output instead of failing the step. +func TestEngine_StepOutputsAreNilForMissingResponseValues(t *testing.T) { + engine := NewEngine(nil, nil, nil) + outputs := orderedmap.New[string, *high.OutputValue]() + outputs.Set("id", high.NewExpressionOutputValue("$response.body#/id")) + outputs.Set("location", high.NewExpressionOutputValue("$response.header.Location")) + outputs.Set("status", high.NewExpressionOutputValue("$statusCode")) + + stepResult := &StepResult{Outputs: make(map[string]any)} + err := engine.populateStepOutputs( + &high.Step{StepId: "step", Outputs: outputs}, + &high.Workflow{WorkflowId: "workflow"}, + stepResult, + &expression.Context{StatusCode: 204}, + ) + require.NoError(t, err) + require.Len(t, stepResult.Outputs, 3) + assert.Contains(t, stepResult.Outputs, "id") + assert.Nil(t, stepResult.Outputs["id"]) + assert.Contains(t, stepResult.Outputs, "location") + assert.Nil(t, stepResult.Outputs["location"]) + assert.Equal(t, 204, stepResult.Outputs["status"]) +} + +// Workflow outputs stage nil values for missing response data and commit them to both +// the result and the expression context. +func TestEngine_WorkflowOutputsAreNilForMissingResponseValues(t *testing.T) { + engine := NewEngine(nil, nil, nil) + outputs := orderedmap.New[string, *high.OutputValue]() + outputs.Set("id", high.NewExpressionOutputValue("$response.body#/id")) + outputs.Set("status", high.NewExpressionOutputValue("$statusCode")) + + workflowResult := &WorkflowResult{Outputs: make(map[string]any)} + exprCtx := &expression.Context{StatusCode: 204, Outputs: make(map[string]any)} + + err := engine.populateWorkflowOutputs( + &high.Workflow{WorkflowId: "workflow", Outputs: outputs}, + workflowResult, + exprCtx, + ) + require.NoError(t, err) + for _, got := range []map[string]any{workflowResult.Outputs, exprCtx.Outputs} { + require.Len(t, got, 2) + assert.Contains(t, got, "id") + assert.Nil(t, got["id"]) + assert.Equal(t, 204, got["status"]) + } +} + +// A missing response value staged before a real failure must not be published either. +func TestEngine_WorkflowOutputsWithMissingValueStayAtomicOnFailure(t *testing.T) { + engine := NewEngine(nil, nil, nil) + outputs := orderedmap.New[string, *high.OutputValue]() + outputs.Set("id", high.NewExpressionOutputValue("$response.body#/id")) + outputs.Set("bad", high.NewExpressionOutputValue("$steps.nonexistent.outputs.missing")) + + workflowResult := &WorkflowResult{Outputs: make(map[string]any)} + exprCtx := &expression.Context{ + StatusCode: 204, + Steps: map[string]*expression.StepContext{}, + Outputs: make(map[string]any), + } + + err := engine.populateWorkflowOutputs( + &high.Workflow{WorkflowId: "workflow", Outputs: outputs}, + workflowResult, + exprCtx, + ) + require.Error(t, err) + assert.Empty(t, workflowResult.Outputs) + assert.Empty(t, exprCtx.Outputs) +} diff --git a/arazzo/step.go b/arazzo/step.go index 1e93d6f1..64742f0a 100644 --- a/arazzo/step.go +++ b/arazzo/step.go @@ -476,6 +476,11 @@ func (e *Engine) populateStepOutputs( } value, err := e.evaluateStringValue(expressionValue, exprCtx) if err != nil { + // Missing body/header values are valid on empty responses (e.g. HTTP 204). + if isOptionalRuntimeValueError(err) { + result.Outputs[name] = nil + continue + } clear(result.Outputs) return fmt.Errorf("failed to evaluate output %q for step %q: %w", name, step.StepId, err) } @@ -499,6 +504,11 @@ func (e *Engine) populateWorkflowOutputs(wf *high.Workflow, result *WorkflowResu } value, err := e.evaluateStringValue(expressionValue, exprCtx) if err != nil { + // Missing body/header values are valid on empty responses (e.g. HTTP 204). + if isOptionalRuntimeValueError(err) { + staged = append(staged, stagedOutput{name: name}) + continue + } return fmt.Errorf("failed to evaluate output %q for workflow %q: %w", name, wf.WorkflowId, err) } staged = append(staged, stagedOutput{name: name, value: value})