Fix splitting of $compute parameter based on comma separator. Implementation previously incorrectly split on all commas including those located within a single $compute item such as to separate function arguments.

Author: migoff875

compute_parser.go

@@ -2,6 +2,8 @@ package godata
 
 import (
 	"context"
+	"errors"
+	"fmt"
 	"regexp"
 	"strings"
 )
@@ -18,25 +20,40 @@ type ComputeItem struct {
 	Field string     // The name of the computed dynamic property.
 }
 
+// GlobalAllTokenParser is a Tokenizer which matches all tokens and ignores none. It differs from the
+// GlobalExpressionTokenizer which ignores whitespace tokens.
+var GlobalAllTokenParser *Tokenizer
+
+func init() {
+	t := NewExpressionParser().tokenizer
+	t.TokenMatchers = append(t.IgnoreMatchers, t.TokenMatchers...)
+	t.IgnoreMatchers = nil
+	GlobalAllTokenParser = t
+}
+
 func ParseComputeString(ctx context.Context, compute string) (*GoDataComputeQuery, error) {
-	items := strings.Split(compute, ",")
+	items, err := SplitComputeItems(compute)
+	if err != nil {
+		return nil, err
+	}
 
 	result := make([]*ComputeItem, 0)
+	fields := map[string]struct{}{}
 
 	for _, v := range items {
 		v = strings.TrimSpace(v)
 		parts := strings.Split(v, computeAsSeparator)
 		if len(parts) != 2 {
 			return nil, &GoDataError{
 				ResponseCode: 400,
-				Message:      "Invalid $compute query option",
+				Message:      "Invalid $compute query format",
 			}
 		}
 		field := strings.TrimSpace(parts[1])
 		if !computeFieldRegex.MatchString(field) {
 			return nil, &GoDataError{
 				ResponseCode: 400,
-				Message:      "Invalid $compute query option",
+				Message:      "Invalid $compute field name",
 			}
 		}
 
@@ -45,7 +62,7 @@ func ParseComputeString(ctx context.Context, compute string) (*GoDataComputeQuer
 			case *GoDataError:
 				return nil, &GoDataError{
 					ResponseCode: e.ResponseCode,
-					Message:      "Invalid $compute query option",
+					Message:      fmt.Sprintf("Invalid $compute query option, %s", e.Message),
 					Cause:        e,
 				}
 			default:
@@ -62,6 +79,16 @@ func ParseComputeString(ctx context.Context, compute string) (*GoDataComputeQuer
 					Message:      "Invalid $compute query option",
 				}
 			}
+
+			if _, ok := fields[field]; ok {
+				return nil, &GoDataError{
+					ResponseCode: 400,
+					Message:      "Invalid $compute, duplicate field name",
+				}
+			}
+
+			fields[field] = struct{}{}
+
 			result = append(result, &ComputeItem{
 				Tree:  tree.Tree,
 				Field: field,
@@ -71,3 +98,52 @@ func ParseComputeString(ctx context.Context, compute string) (*GoDataComputeQuer
 
 	return &GoDataComputeQuery{result, compute}, nil
 }
+
+// SplitComputeItems splits the input string based on the comma delimiter. It does so with awareness as to
+// which commas delimit $compute items and which ones are an inline part of the item, such as a separator
+// for function arguments.
+//
+// For example the input "someFunc(one,two) as three, 1 add 2 as four" results in the
+// output ["someFunc(one,two) as three", "1 add 2 as four"]
+func SplitComputeItems(in string) ([]string, error) {
+
+	var ret []string
+
+	tokens, err := GlobalAllTokenParser.Tokenize(context.Background(), in)
+	if err != nil {
+		return nil, err
+	}
+
+	item := strings.Builder{}
+	parenGauge := 0
+
+	for _, v := range tokens {
+		switch v.Type {
+		case ExpressionTokenOpenParen:
+			parenGauge++
+		case ExpressionTokenCloseParen:
+			if parenGauge == 0 {
+				return nil, errors.New("unmatched parentheses")
+			}
+			parenGauge--
+		case ExpressionTokenComma:
+			if parenGauge == 0 {
+				ret = append(ret, item.String())
+				item.Reset()
+				continue
+			}
+		}
+
+		item.WriteString(v.Value)
+	}
+
+	if parenGauge != 0 {
+		return nil, errors.New("unmatched parentheses")
+	}
+
+	if item.Len() > 0 {
+		ret = append(ret, item.String())
+	}
+
+	return ret, nil
+}

compute_parser_test.go

@@ -0,0 +1,65 @@
+package godata
+
+import (
+	"context"
+	"strings"
+	"testing"
+)
+
+func TestParseCompute(t *testing.T) {
+	DefineCustomFunctions([]CustomFunctionInput{
+		{Name: "zeroArgFunc", NumParams: []int{0}},
+		{Name: "oneArgFunc", NumParams: []int{1}},
+		{Name: "twoArgFunc", NumParams: []int{2}},
+	})
+
+	type testcase struct {
+		computeStrings []string
+		shouldPass     bool
+	}
+
+	testCases := []testcase{
+		{[]string{"oldField as newField"}, true},
+		{[]string{"1 as newField"}, true},
+		{[]string{"one add 2 as newField"}, true},
+		{[]string{"one add two as extra/newField"}, true},
+		{[]string{"zeroArgFunc() as newField"}, true},
+		{[]string{"oneArgFunc(one) as newField"}, true},
+		{[]string{"twoArgFunc(one, two) as newField"}, true},
+		{[]string{"twoArgFunc(one, two) as newField", "tolower(three) as  newFieldTwo"}, true},
+
+		// negative cases
+		{[]string{"one add two as newField2"}, false},
+		{[]string{"one add two newField2"}, false},
+		{[]string{""}, false},
+		{[]string{"as"}, false},
+		{[]string{"as newField"}, false},
+		{[]string{"zeroArgFunc() as "}, false},
+	}
+
+	for i, v := range testCases {
+
+		result, err := ParseComputeString(context.Background(), strings.Join(v.computeStrings, ","))
+		if v.shouldPass {
+			if err != nil {
+				t.Errorf("testcase %d: %v", i, err)
+				t.FailNow()
+			}
+			if result == nil {
+				t.Errorf("testcase %d: nil result", i)
+				t.FailNow()
+			}
+
+			if len(result.ComputeItems) != len(v.computeStrings) {
+				t.Errorf("testcase %d: wrong number of result items", i)
+				t.FailNow()
+			}
+		} else {
+			if err == nil {
+				t.Errorf("testcase %d: expected error", i)
+				t.FailNow()
+			}
+		}
+
+	}
+}