fixes for code review of rs#274

Author: Tsvetelin Pantev

README.md

@@ -998,7 +998,7 @@ The same example with flags:
 However, keep in mind that Storers have to support regular expression and depending on the implementation of the storage handler the accepted syntax may vary.
 An error of `ErrNotImplemented` will be returned for those storage back-ends which do not support the `$regex` operator.
 
-The operator `$not` functions as an opposite operator to `$regex`
+The operator `$not` functions as an opposite operator to `$regex`. Unlike MongoDB, we do not allow `$not` as a general negation operator.
 
 The `$elemMatch` operator matches documents that contain an array field with at least one element that matches all the specified query criteria.
 ```go

schema/query/predicate.go

@@ -477,13 +477,14 @@ func (e LowerOrEqual) String() string {
 
 // Regex matches values that match to a specified regular expression.
 type Regex struct {
-	Field string
-	Value *regexp.Regexp
+	Field   string
+	Value   *regexp.Regexp
+	Negated bool
 }
 
 // Match implements Expression interface.
 func (e Regex) Match(payload map[string]interface{}) bool {
-	return e.Value.MatchString(payload[e.Field].(string))
+	return e.Value.MatchString(payload[e.Field].(string)) != e.Negated
 }
 
 // Prepare implements Expression interface.
@@ -494,7 +495,13 @@ func (e *Regex) Prepare(validator schema.Validator) error {
 
 // String implements Expression interface.
 func (e Regex) String() string {
-	return quoteField(e.Field) + ": {" + opRegex + ": " + valueString(e.Value) + "}"
+	var regexOperation string
+	if e.Negated {
+		regexOperation = opNot
+	} else {
+		regexOperation = opRegex
+	}
+	return quoteField(e.Field) + ": {" + regexOperation + ": " + valueString(e.Value) + "}"
 }
 
 // ElemMatch matches object values specified in an array.
@@ -553,24 +560,3 @@ func (e ElemMatch) String() string {
 	}
 	return quoteField(e.Field) + ": {" + opElemMatch + ": {" + strings.Join(s, ", ") + "}}"
 }
-
-type Not struct {
-	Field string
-	Value *regexp.Regexp
-}
-
-// Match implements Expression interface.
-func (e Not) Match(payload map[string]interface{}) bool {
-	return !e.Value.MatchString(payload[e.Field].(string))
-}
-
-// Prepare implements Expression interface.
-func (e *Not) Prepare(validator schema.Validator) error {
-	_, err := prepareValue(e.Field, e.Value.String(), validator)
-	return err
-}
-
-// String implements Expression interface.
-func (e Not) String() string {
-	return quoteField(e.Field) + ": {" + opNot + ": \"" + e.Value.String() + "\"}"
-}

schema/query/predicate_parser.go

@@ -234,16 +234,24 @@ func (p *predicateParser) parseCommand(field string) (Expression, error) {
 			case opGreaterOrEqual:
 				return &GreaterOrEqual{Field: field, Value: value}, nil
 			}
-		case opRegex:
-			re, err := p.parseRegex(label)
+		case opRegex, opNot:
+			str, err := p.parseString()
 			if err != nil {
-				return nil, err
+				return nil, fmt.Errorf("%s: %v", label, err)
+			}
+			re, err := regexp.Compile(str)
+			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
+			negated := false
+			if label == opNot {
+				negated = true
+			}
+			return &Regex{Field: field, Value: re, Negated: negated}, nil
 		case opElemMatch:
 			exps, err := p.parseExpressions()
 			if err != nil {
@@ -254,16 +262,6 @@ func (p *predicateParser) parseCommand(field string) (Expression, error) {
 				return nil, fmt.Errorf("%s: expected '}' got %q", label, p.peek())
 			}
 			return &ElemMatch{Field: field, Exps: exps}, nil
-		case opNot:
-			re, err := p.parseRegex(label)
-			if err != nil {
-				return nil, err
-			}
-			p.eatWhitespaces()
-			if !p.expect('}') {
-				return nil, fmt.Errorf("%s: expected '}' got %q", label, p.peek())
-			}
-			return &Not{Field: field, Value: re}, nil
 		}
 	}
 VALUE:
@@ -530,15 +528,3 @@ func (p *predicateParser) eatWhitespaces() {
 		break
 	}
 }
-
-func (p *predicateParser) parseRegex(label string) (*regexp.Regexp, error) {
-	str, err := p.parseString()
-	if err != nil {
-		return nil, fmt.Errorf("%s: %v", label, err)
-	}
-	re, err := regexp.Compile(str)
-	if err != nil {
-		return nil, fmt.Errorf("%s: invalid regex: %v", label, err)
-	}
-	return re, nil
-}

schema/query/predicate_parser_test.go

@@ -105,7 +105,7 @@ func TestParse(t *testing.T) {
 		},
 		{
 			`{"foo": {"$not": "regex.+awesome"}}`,
-			Predicate{&Not{Field: "foo", Value: regexp.MustCompile("regex.+awesome")}},
+			Predicate{&Regex{Field: "foo", Value: regexp.MustCompile("regex.+awesome"), Negated: true}},
 			nil,
 		},
 		{