clarify · smyrman · Mar 23, 2022 · Mar 22, 2022 · Mar 22, 2022
diff --git a/schema/query/predicate_parser.go b/schema/query/predicate_parser.go
@@ -167,72 +167,76 @@ func (p *predicateParser) parseSubExpressions() ([]Expression, error) {
 //   {$in: ["foo", "bar"]}
 func (p *predicateParser) parseCommand(field string) (Expression, error) {
 	oldPos := p.pos
-	if p.expect('{') {
-		p.eatWhitespaces()
-		if p.expect('}') {
-			// Empty dict must be parsed as a value
-			goto VALUE
-		}
+	and := make(And, 0, 1)
+	var nonOps []string
+
+	if !p.expect('{') {
+		// Non-object is treated as value.
+		goto VALUE
+	}
+	p.eatWhitespaces()
+
+	if p.expect('}') {
+		// Empty object is treated as value.
+		goto VALUE
+	}
+
+	// Parse content of non-empty object to look for known operators. If there
+	// are no known operators, we will treat it as a value. If all operators are
+	// known, we will treat it as a set of operator comparisons to be joined by
+	// logical AND. If there is a mix of operator and non-operator fields, we
+	// will respond with an error.
+	for {
 		label, err := p.parseLabel()
 		if err != nil {
 			return nil, err
 		}
 		p.eatWhitespaces()
+
+		var next Expression
 		switch label {
 		case opExists:
 			v, err := p.parseBool()
 			if err != nil {
 				return nil, fmt.Errorf("%s: %v", label, err)
 			}
-			p.eatWhitespaces()
-			if !p.expect('}') {
-				return nil, fmt.Errorf("%s: expected '}' got %q", label, p.peek())
-			}
 			if v {
-				return &Exist{Field: field}, nil
+				next = &Exist{Field: field}
+			} else {
+				next = &NotExist{Field: field}
 			}
-			return &NotExist{Field: field}, nil
-		case opIn, opNotIn:
+		case opIn:
 			values, err := p.parseValues()
 			if err != nil {
 				return nil, fmt.Errorf("%s: %v", label, err)
 			}
-			p.eatWhitespaces()
-			if !p.expect('}') {
-				return nil, fmt.Errorf("%s: expected '}' got %q", label, p.peek())
-			}
-			if label == opIn {
-				return &In{Field: field, Values: values}, nil
+			next = &In{Field: field, Values: values}
+		case opNotIn:
 			values, err := p.parseValues()
+			if err != nil {
+				return nil, fmt.Errorf("%s: %v", label, err)
 			}
-			return &NotIn{Field: field, Values: values}, nil
+			next = &NotIn{Field: field, Values: values}
 		case opNotEqual:
 			value, err := p.parseValue()
 			if err != nil {
 				return nil, fmt.Errorf("%s: %v", label, err)
 			}
-			p.eatWhitespaces()
-			if !p.expect('}') {
-				return nil, fmt.Errorf("%s: expected '}' got %q", label, p.peek())
-			}
-			return &NotEqual{Field: field, Value: value}, nil
+			next = &NotEqual{Field: field, Value: value}
 		case opLowerThan, opLowerOrEqual, opGreaterThan, opGreaterOrEqual:
 			value, err := p.parseValue()
 			if err != nil {
 				return nil, fmt.Errorf("%s: %v", label, err)
 			}
-			p.eatWhitespaces()
-			if !p.expect('}') {
-				return nil, fmt.Errorf("%s: expected '}' got %q", label, p.peek())
-			}
 			switch label {
 			case opLowerThan:
-				return &LowerThan{Field: field, Value: value}, nil
+				next = &LowerThan{Field: field, Value: value}
 			case opLowerOrEqual:
-				return &LowerOrEqual{Field: field, Value: value}, nil
+				next = &LowerOrEqual{Field: field, Value: value}
 			case opGreaterThan:
-				return &GreaterThan{Field: field, Value: value}, nil
+				next = &GreaterThan{Field: field, Value: value}
 			case opGreaterOrEqual:
-				return &GreaterOrEqual{Field: field, Value: value}, nil
+				next = &GreaterOrEqual{Field: field, Value: value}
 			}
 		case opRegex:
 			str, err := p.parseString()
@@ -243,22 +247,47 @@ func (p *predicateParser) parseCommand(field string) (Expression, error) {
 			if err != nil {
 				return nil, fmt.Errorf("%s: invalid regex: %v", label, err)
 			}
-			p.eatWhitespaces()
-			if !p.expect('}') {
-				return nil, fmt.Errorf("%s: expected '}' got %q", label, p.peek())
-			}
-			return &Regex{Field: field, Value: re}, nil
+			next = &Regex{Field: field, Value: re}
 		case opElemMatch:
 			exps, err := p.parseExpressions()
 			if err != nil {
 				return nil, fmt.Errorf("%s: %v", label, err)
 			}
+			next = &ElemMatch{Field: field, Exps: exps}
+		default:
+			// Track unknown operator; if all operators are unknown, we will
+			// fallback to a value comparison.
+			_, err := p.parseValue()
+			if err != nil {
+				return nil, fmt.Errorf("%s: %v", label, err)
+			}
+			nonOps = append(nonOps, label)
+		}
+		if next != nil {
+			and = append(and, next)
+		}
+		p.eatWhitespaces()
+		switch {
+		case p.expect('}'):
 			p.eatWhitespaces()
-			if !p.expect('}') {
-				return nil, fmt.Errorf("%s: expected '}' got %q", label, p.peek())
+			switch {
+			case len(and) == 0:
+				// Object is either empty or without any recognized operators.
+				goto VALUE
+			case len(nonOps) > 0:
+				// Combination of recognized and non-recognized operators.
+				return nil, fmt.Errorf("invalid operators: %v", nonOps)
+			case len(and) == 1:
+				// Single operator.
+				return and[0], nil
+			default:
+				// Multiple operators.
+				return &and, nil
 			}
-			return &ElemMatch{Field: field, Exps: exps}, nil
+		case !p.expect(','):
+			return nil, fmt.Errorf("%s: expected '}' or ',' got %q", label, p.peek())
 		}
+		p.eatWhitespaces()
 	}
 VALUE:
 	// If the current position is not a dictionary ({}) or is a dictionary with

diff --git a/schema/query/predicate_parser_test.go b/schema/query/predicate_parser_test.go
@@ -87,6 +87,16 @@ func TestParse(t *testing.T) {
 			Predicate{&GreaterThan{Field: "baz", Value: float64(1)}},
 			nil,
 		},
+		{
+			`{"baz": {"$gt": 1, "$lt": 2}}`,
+			Predicate{
+				&And{
+					&GreaterThan{Field: "baz", Value: float64(1)},
+					&LowerThan{Field: "baz", Value: float64(2)},
+				},
+			},
+			nil,
+		},
 		{
 			`{"$or": [{"foo": "bar"}, {"foo": "baz"}]}`,
 			Predicate{&Or{&Equal{Field: "foo", Value: "bar"}, &Equal{Field: "foo", Value: "baz"}}},
@@ -182,27 +192,27 @@ func TestParse(t *testing.T) {
 		{
 			`{"foo": {"$exists": true`,
 			Predicate{},
-			errors.New("char 24: foo: $exists: expected '}' got '\\x00'"),
+			errors.New("char 24: foo: $exists: expected '}' or ',' got '\\x00'"),
 		},
 		{
 			`{"foo": {"$in": []`,
 			Predicate{},
-			errors.New("char 18: foo: $in: expected '}' got '\\x00'"),
+			errors.New("char 18: foo: $in: expected '}' or ',' got '\\x00'"),
 		},
 		{
 			`{"foo": {"$ne": "bar"`,
 			Predicate{},
-			errors.New("char 21: foo: $ne: expected '}' got '\\x00'"),
+			errors.New("char 21: foo: $ne: expected '}' or ',' got '\\x00'"),
 		},
 		{
 			`{"foo": {"$regex": "."`,
 			Predicate{},
-			errors.New("char 22: foo: $regex: expected '}' got '\\x00'"),
+			errors.New("char 22: foo: $regex: expected '}' or ',' got '\\x00'"),
 		},
 		{
 			`{"foo": {"$gt": 1`,
 			Predicate{},
-			errors.New("char 17: foo: $gt: expected '}' got '\\x00'"),
+			errors.New("char 17: foo: $gt: expected '}' or ',' got '\\x00'"),
 		},
 		{
 			`{"foo": {"$exists`,
@@ -270,6 +280,16 @@ func TestParse(t *testing.T) {
 			Predicate{},
 			errors.New("char 16: bar: $in: expected '[' got '\"'"),
 		},
+		{
+			`{"baz": {"$gt": 1, "foo": "bar", "bar": "baz"}}`,
+			nil,
+			errors.New("char 46: baz: invalid operators: [foo bar]"),
+		},
+		{
+			`{"baz": {"foo": "bar", "bar": "baz", "$gt": 1}}`,
+			nil,
+			errors.New("char 46: baz: invalid operators: [foo bar]"),
+		},
 		{
 			`{"$or": "foo"}`,
 			Predicate{},
@@ -375,7 +395,7 @@ func TestParse(t *testing.T) {
 		t.Run(strings.Replace(tt.query, " ", "", -1), func(t *testing.T) {
 			t.Parallel()
 			got, err := ParsePredicate(tt.query)
-			if !reflect.DeepEqual(err, tt.err) {
+			if !equalErrorText(err, tt.err) {
 				t.Errorf("unexpected error for `%v`\ngot:  %v\nwant: %v", tt.query, err, tt.err)
 			}
 			if err == nil && !reflect.DeepEqual(got, tt.want) {

diff --git a/schema/query/predicate_validator_test.go b/schema/query/predicate_validator_test.go
@@ -76,7 +76,7 @@ func TestPrepare(t *testing.T) {
 			t.Errorf("Unexpected parse error for `%v`: %v", tt.query, err)
 			continue
 		}
-		if err = q.Prepare(s); !reflect.DeepEqual(err, tt.err) {
+		if err = q.Prepare(s); !equalErrorText(err, tt.err) {
 			t.Errorf("Unexpected error for `%v`:\ngot:  %v\nwant: %v", tt.query, err, tt.err)
 		}
 		if !reflect.DeepEqual(q, tt.want) {
@@ -175,7 +175,7 @@ func TestPrepareErrors(t *testing.T) {
 			t.Errorf("Unexpected parse error for `%v`: %v", tt.query, err)
 			continue
 		}
-		if err = q.Prepare(s); !reflect.DeepEqual(err, tt.want) {
+		if err = q.Prepare(s); !equalErrorText(err, tt.want) {
 			t.Errorf("Unexpected error for `%v`:\ngot:  %v\nwant: %v", tt.query, err, tt.want)
 		}
 	}

diff --git a/schema/query/projection_evaluator_test.go b/schema/query/projection_evaluator_test.go
@@ -5,7 +5,6 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"reflect"
 	"sort"
 	"testing"
 
@@ -163,16 +162,16 @@ func TestProjectionEval(t *testing.T) {
 			},
 		}},
 		subResources: map[string]resource{
-			"cnx": resource{
+			"cnx": {
 				validator: cnxShema,
 				payloads: map[string]map[string]interface{}{
-					"1": map[string]interface{}{"id": "1", "name": "first"},
-					"2": map[string]interface{}{"id": "2", "name": "second", "ref": "a"},
-					"3": map[string]interface{}{"id": "3", "name": "third", "ref": "b"},
-					"4": map[string]interface{}{"id": "4", "name": "forth", "ref": "a"},
+					"1": {"id": "1", "name": "first"},
+					"2": {"id": "2", "name": "second", "ref": "a"},
+					"3": {"id": "3", "name": "third", "ref": "b"},
+					"4": {"id": "4", "name": "forth", "ref": "a"},
 				},
 			},
-			"cnx2": resource{
+			"cnx2": {
 				validator: schema.Schema{Fields: schema.Fields{
 					"id":   {},
 					"name": {},
@@ -186,21 +185,21 @@ func TestProjectionEval(t *testing.T) {
 					},
 				}},
 				subResources: map[string]resource{
-					"cnx3": resource{
+					"cnx3": {
 						validator: cnxShema,
 						payloads: map[string]map[string]interface{}{
-							"6": map[string]interface{}{"id": "6", "name": "first"},
-							"7": map[string]interface{}{"id": "7", "name": "second", "ref": "a"},
-							"8": map[string]interface{}{"id": "8", "name": "third", "ref": "b"},
-							"9": map[string]interface{}{"id": "9", "name": "forth", "ref": "c"},
+							"6": {"id": "6", "name": "first"},
+							"7": {"id": "7", "name": "second", "ref": "a"},
+							"8": {"id": "8", "name": "third", "ref": "b"},
+							"9": {"id": "9", "name": "forth", "ref": "c"},
 						},
 					},
 				},
 				payloads: map[string]map[string]interface{}{
-					"a": map[string]interface{}{"id": "a", "name": "first"},
-					"b": map[string]interface{}{"id": "b", "name": "second", "ref": "2"},
-					"c": map[string]interface{}{"id": "c", "name": "third", "ref": "3"},
-					"d": map[string]interface{}{"id": "d", "name": "forth", "ref": "4"},
+					"a": {"id": "a", "name": "first"},
+					"b": {"id": "b", "name": "second", "ref": "2"},
+					"c": {"id": "c", "name": "third", "ref": "3"},
+					"d": {"id": "d", "name": "forth", "ref": "4"},
 				},
 			},
 		},
@@ -553,7 +552,7 @@ func TestProjectionEval(t *testing.T) {
 				t.Errorf("Invalid JSON payload: %v", err)
 			}
 			payload, err = pr.Eval(ctx, payload, r)
-			if !reflect.DeepEqual(err, tc.err) {
+			if !equalErrorText(err, tc.err) {
 				t.Errorf("Eval return error: %v, wanted: %v", err, tc.err)
 			}
 			if err != nil {

diff --git a/schema/query/projection_parser_test.go b/schema/query/projection_parser_test.go
@@ -253,7 +253,7 @@ func TestParseProjection(t *testing.T) {
 		}
 		t.Run(tc.projection, func(t *testing.T) {
 			pr, err := ParseProjection(tc.projection)
-			if !reflect.DeepEqual(err, tc.err) {
+			if !equalErrorText(err, tc.err) {
 				t.Errorf("ParseProjection error:\ngot:  %v\nwant: %v", err, tc.err)
 			}
 			if err != nil {

diff --git a/schema/query/projection_validator_test.go b/schema/query/projection_validator_test.go
@@ -2,7 +2,6 @@ package query
 
 import (
 	"errors"
-	"reflect"
 	"testing"
 
 	"github.com/clarify/rested/schema"
@@ -94,7 +93,7 @@ func TestProjectionValidate(t *testing.T) {
 			if err != nil {
 				t.Errorf("ParseProjection unexpected error: %v", err)
 			}
-			if err = pr.Validate(s); !reflect.DeepEqual(err, tc.err) {
+			if err = pr.Validate(s); !equalErrorText(err, tc.err) {
 				t.Errorf("Projection.Validate error = %v, wanted: %v", err, tc.err)
 			}
 		})

diff --git a/schema/query/sort_test.go b/schema/query/sort_test.go
@@ -40,7 +40,7 @@ func TestParseSort(t *testing.T) {
 		}
 		t.Run(tt.sort, func(t *testing.T) {
 			got, err := ParseSort(tt.sort)
-			if !reflect.DeepEqual(err, tt.err) {
+			if !equalErrorText(err, tt.err) {
 				t.Errorf("unexpected error:\ngot:  %v\nwant: %v", err, tt.err)
 			}
 			if err == nil && !reflect.DeepEqual(got, tt.want) {
@@ -70,7 +70,7 @@ func TestSortValidate(t *testing.T) {
 			if err != nil {
 				t.Errorf("unexpected parse error: %v", err)
 			}
-			if err := sort.Validate(s); !reflect.DeepEqual(err, tt.err) {
+			if err := sort.Validate(s); !equalErrorText(err, tt.err) {
 				t.Errorf("unexpected validate error:\ngot:  %#v\nwant: %#v", err, tt.err)
 			}
 		})

diff --git a/schema/query/utils_test.go b/schema/query/utils_test.go
@@ -54,3 +54,13 @@ func TestGetField(t *testing.T) {
 		})
 	}
 }
+
+func equalErrorText(got, want error) bool {
+	if got == nil {
+		return want == nil
+	}
+	if want == nil {
+		return got == nil
+	}
+	return got.Error() == want.Error()
+}