Skip to content

Commit a354403

Browse files
devin-ai-integration[bot]chrisli30github-actions[bot]
authored
Add schema validation for BranchNode gRPC objects (#248)
* Fixes #140 Co-Authored-By: Chris Li <chris.li.2046@gmail.com> * Fix validation to allow empty expressions in if conditions Co-Authored-By: Chris Li <chris.li.2046@gmail.com> * style: Automated gofmt Formatted Go code using gofmt. * Update tests to allow empty expressions in if conditions Co-Authored-By: Chris Li <chris.li.2046@gmail.com> --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: Chris Li <chris.li.2046@gmail.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
1 parent 582f862 commit a354403

2 files changed

Lines changed: 120 additions & 0 deletions

File tree

core/taskengine/vm_runner_branch.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,30 @@ func (r *BranchProcessor) Validate(node *avsproto.BranchNode) error {
3232
return fmt.Errorf("the first condition need to be an if but got :%s", node.Conditions[0].Type)
3333
}
3434

35+
for i, condition := range node.Conditions {
36+
if condition == nil {
37+
return fmt.Errorf("condition at index %d is nil", i)
38+
}
39+
40+
if condition.Id == "" {
41+
return fmt.Errorf("condition at index %d has empty ID", i)
42+
}
43+
44+
if condition.Type == "" {
45+
return fmt.Errorf("condition at index %d has empty type", i)
46+
}
47+
48+
if condition.Type != "if" && condition.Type != "else" {
49+
return fmt.Errorf("condition at index %d has invalid type: %s (must be 'if' or 'else')", i, condition.Type)
50+
}
51+
52+
if condition.Type == "else" && i < len(node.Conditions)-1 {
53+
if r.vm.logger != nil {
54+
r.vm.logger.Warn("'else' condition is not the last one, subsequent conditions will be ignored")
55+
}
56+
}
57+
}
58+
3559
return nil
3660
}
3761

core/taskengine/vm_runner_branch_test.go

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -549,3 +549,99 @@ func TestBranchNodeNoElseSkip(t *testing.T) {
549549
})
550550
}
551551
}
552+
553+
func TestBranchNodeMalformedData(t *testing.T) {
554+
testCases := []struct {
555+
name string
556+
conditions []*avsproto.Condition
557+
expectError bool
558+
errorMsg string
559+
}{
560+
{
561+
name: "nil condition",
562+
conditions: []*avsproto.Condition{
563+
{Id: "condition1", Type: "if", Expression: "a > 10"},
564+
nil,
565+
},
566+
expectError: true,
567+
errorMsg: "condition at index 1 is nil",
568+
},
569+
{
570+
name: "empty condition ID",
571+
conditions: []*avsproto.Condition{
572+
{Id: "condition1", Type: "if", Expression: "a > 10"},
573+
{Id: "", Type: "if", Expression: "a > 5"},
574+
},
575+
expectError: true,
576+
errorMsg: "condition at index 1 has empty ID",
577+
},
578+
{
579+
name: "empty condition type",
580+
conditions: []*avsproto.Condition{
581+
{Id: "condition1", Type: "if", Expression: "a > 10"},
582+
{Id: "condition2", Type: "", Expression: "a > 5"},
583+
},
584+
expectError: true,
585+
errorMsg: "condition at index 1 has empty type",
586+
},
587+
{
588+
name: "invalid condition type",
589+
conditions: []*avsproto.Condition{
590+
{Id: "condition1", Type: "if", Expression: "a > 10"},
591+
{Id: "condition2", Type: "invalid", Expression: "a > 5"},
592+
},
593+
expectError: true,
594+
errorMsg: "condition at index 1 has invalid type",
595+
},
596+
{
597+
name: "empty if expression",
598+
conditions: []*avsproto.Condition{
599+
{Id: "condition1", Type: "if", Expression: "a > 10"},
600+
{Id: "condition2", Type: "if", Expression: ""},
601+
},
602+
expectError: false,
603+
},
604+
{
605+
name: "whitespace if expression",
606+
conditions: []*avsproto.Condition{
607+
{Id: "condition1", Type: "if", Expression: "a > 10"},
608+
{Id: "condition2", Type: "if", Expression: " \t\n "},
609+
},
610+
expectError: false,
611+
},
612+
{
613+
name: "valid conditions",
614+
conditions: []*avsproto.Condition{
615+
{Id: "condition1", Type: "if", Expression: "a > 10"},
616+
{Id: "condition2", Type: "if", Expression: "a > 5"},
617+
{Id: "condition3", Type: "else"},
618+
},
619+
expectError: false,
620+
},
621+
}
622+
623+
for _, tc := range testCases {
624+
t.Run(tc.name, func(t *testing.T) {
625+
vm := NewVM()
626+
processor := NewBranchProcessor(vm)
627+
628+
err := processor.Validate(&avsproto.BranchNode{
629+
Conditions: tc.conditions,
630+
})
631+
632+
if tc.expectError {
633+
if err == nil {
634+
t.Errorf("expected error but got none")
635+
return
636+
}
637+
if !strings.Contains(err.Error(), tc.errorMsg) {
638+
t.Errorf("expected error message to contain '%s', but got: '%s'", tc.errorMsg, err.Error())
639+
}
640+
} else {
641+
if err != nil {
642+
t.Errorf("unexpected error: %v", err)
643+
}
644+
}
645+
})
646+
}
647+
}

0 commit comments

Comments
 (0)