feat(datastore): adds in, not-in, and != query operations

datastore/query.go

@@ -38,6 +38,9 @@ const (
 	equal
 	greaterEq
 	greaterThan
+	in
+	notIn
+	notEqual
 
 	keyFieldName = "__key__"
 )
@@ -48,6 +51,9 @@ var operatorToProto = map[operator]pb.PropertyFilter_Operator{
 	equal:       pb.PropertyFilter_EQUAL,
 	greaterEq:   pb.PropertyFilter_GREATER_THAN_OR_EQUAL,
 	greaterThan: pb.PropertyFilter_GREATER_THAN,
+	in:          pb.PropertyFilter_IN,
+	notIn:       pb.PropertyFilter_NOT_IN,
+	notEqual:    pb.PropertyFilter_NOT_EQUAL,
 }
 
 // filter is a conditional filter on query results.
@@ -171,25 +177,46 @@ func (q *Query) Transaction(t *Transaction) *Query {
 }
 
 // Filter returns a derivative query with a field-based filter.
+//
+// Deprecated: Use the FilterField method instead, which supports the same
+// set of operations (and more).
+//
 // The filterStr argument must be a field name followed by optional space,
-// followed by an operator, one of ">", "<", ">=", "<=", or "=".
+// followed by an operator, one of ">", "<", ">=", "<=", "=", and "!=".
 // Fields are compared against the provided value using the operator.
 // Multiple filters are AND'ed together.
 // Field names which contain spaces, quote marks, or operator characters
 // should be passed as quoted Go string literals as returned by strconv.Quote
 // or the fmt package's %q verb.
 func (q *Query) Filter(filterStr string, value interface{}) *Query {
-	q = q.clone()
+	// TODO( #5977 ): Add better string parsing (or something)
 	filterStr = strings.TrimSpace(filterStr)
 	if filterStr == "" {
 		q.err = fmt.Errorf("datastore: invalid filter %q", filterStr)
 		return q
 	}
+	f := strings.TrimRight(filterStr, " ><=!")
+	op := strings.TrimSpace(filterStr[len(f):])
+	return q.FilterField(f, op, value)
+}
+
+// FilterField returns a derivative query with a field-based filter.
+// The operation parameter takes the following strings: ">", "<", ">=", "<=",
+// "=", "!=", "in", and "not-in".
+// Fields are compared against the provided value using the operator.
+// Multiple filters are AND'ed together.
+// Field names which contain spaces, quote marks, or operator characters
+// should be passed as quoted Go string literals as returned by strconv.Quote
+// or the fmt package's %q verb.
+func (q *Query) FilterField(fieldName, operator string, value interface{}) *Query {
+	q = q.clone()
+
 	f := filter{
-		FieldName: strings.TrimRight(filterStr, " ><=!"),
+		FieldName: fieldName,
 		Value:     value,
 	}
-	switch op := strings.TrimSpace(filterStr[len(f.FieldName):]); op {
+
+	switch o := strings.TrimSpace(operator); o {
 	case "<=":
 		f.Op = lessEq
 	case ">=":
@@ -200,8 +227,14 @@ func (q *Query) Filter(filterStr string, value interface{}) *Query {
 		f.Op = greaterThan
 	case "=":
 		f.Op = equal
+	case "in":
+		f.Op = in
+	case "not-in":
+		f.Op = notIn
+	case "!=":
+		f.Op = notEqual
 	default:
-		q.err = fmt.Errorf("datastore: invalid operator %q in filter %q", op, filterStr)
+		q.err = fmt.Errorf("datastore: invalid operator %q in filter", operator)
 		return q
 	}
 	var err error

datastore/query_test.go

@@ -391,57 +391,61 @@ func TestQueriesAreImmutable(t *testing.T) {
 	}
 }
 
-func TestFilterParser(t *testing.T) {
-	testCases := []struct {
-		filterStr     string
-		wantOK        bool
-		wantFieldName string
-		wantOp        operator
-	}{
-		// Supported ops.
-		{"x<", true, "x", lessThan},
-		{"x <", true, "x", lessThan},
-		{"x  <", true, "x", lessThan},
-		{"   x   <  ", true, "x", lessThan},
-		{"x <=", true, "x", lessEq},
-		{"x =", true, "x", equal},
-		{"x >=", true, "x", greaterEq},
-		{"x >", true, "x", greaterThan},
-		{"in >", true, "in", greaterThan},
-		{"in>", true, "in", greaterThan},
-		// Valid but (currently) unsupported ops.
-		{"x!=", false, "", 0},
-		{"x !=", false, "", 0},
-		{" x  !=  ", false, "", 0},
-		{"x IN", false, "", 0},
-		{"x in", false, "", 0},
-		// Invalid ops.
-		{"x EQ", false, "", 0},
-		{"x lt", false, "", 0},
-		{"x <>", false, "", 0},
-		{"x >>", false, "", 0},
-		{"x ==", false, "", 0},
-		{"x =<", false, "", 0},
-		{"x =>", false, "", 0},
-		{"x !", false, "", 0},
-		{"x ", false, "", 0},
-		{"x", false, "", 0},
-		// Quoted and interesting field names.
-		{"x > y =", true, "x > y", equal},
-		{"` x ` =", true, " x ", equal},
-		{`" x " =`, true, " x ", equal},
-		{`" \"x " =`, true, ` "x `, equal},
-		{`" x =`, false, "", 0},
-		{`" x ="`, false, "", 0},
-		{"` x \" =", false, "", 0},
+type testFilterCase struct {
+	filterStr     string
+	fieldName     string
+	operator      string
+	wantOp        operator
+	wantFieldName string
+}
+
+var (
+	// Supported ops both filters.
+	filterTestCases = []testFilterCase{
+		{"x<", "x", "<", lessThan, "x"},
+		{"x <", "x", "<", lessThan, "x"},
+		{"x  <", "x", "<", lessThan, "x"},
+		{"   x   <  ", "x", "<", lessThan, "x"},
+		{"x <=", "x", "<=", lessEq, "x"},
+		{"x =", "x", "=", equal, "x"},
+		{"x >=", "x", ">=", greaterEq, "x"},
+		{"x >", "x", ">", greaterThan, "x"},
+		{"in >", "in", ">", greaterThan, "in"},
+		{"in>", "in", ">", greaterThan, "in"},
+		{"x!=", "x", "!=", notEqual, "x"},
+		{"x !=", "x", "!=", notEqual, "x"},
+		{" x  !=  ", "x", "!=", notEqual, "x"},
 	}
-	for _, tc := range testCases {
+	// Supported in FilterField only.
+	filterFieldTestCases = []testFilterCase{
+		{"x in", "x", "in", in, "x"},
+		{"x not-in", "x", "not-in", notIn, "x"},
+		{"ins in", "ins", "in", in, "ins"},
+		{"in not-in", "in", "not-in", notIn, "in"},
+	}
+	// Operators not supported in either filter method
+	filterUnsupported = []testFilterCase{
+		{"x IN", "x", "IN", 0, ""},
+		{"x NOT-IN", "x", "NOT-IN", 0, ""},
+		{"x EQ", "x", "EQ", 0, ""},
+		{"x lt", "x", "lt", 0, ""},
+		{"x <>", "x", "<>", 0, ""},
+		{"x >>", "x", ">>", 0, ""},
+		{"x ==", "x", "==", 0, ""},
+		{"x =<", "x", "=<", 0, ""},
+		{"x =>", "x", "=>", 0, ""},
+		{"x !", "x", "!", 0, ""},
+		{"x ", "x", "", 0, ""},
+		{"x", "x", "", 0, ""},
+	}
+)
+
+func TestFilterParser(t *testing.T) {
+	// Success cases
+	for _, tc := range filterTestCases {
 		q := NewQuery("foo").Filter(tc.filterStr, 42)
-		if ok := q.err == nil; ok != tc.wantOK {
-			t.Errorf("%q: ok=%t, want %t", tc.filterStr, ok, tc.wantOK)
-			continue
-		}
-		if !tc.wantOK {
+		if q.err != nil {
+			t.Errorf("%q: error=%v", tc.filterStr, q.err)
 			continue
 		}
 		if len(q.filter) != 1 {
@@ -454,6 +458,62 @@ func TestFilterParser(t *testing.T) {
 			continue
 		}
 	}
+	// Failure cases
+	failureTestCases := append(filterFieldTestCases, filterUnsupported...)
+	for _, tc := range failureTestCases {
+		q := NewQuery("foo").Filter(tc.filterStr, 42)
+		if q.err == nil {
+			t.Errorf("%q: should have thrown error", tc.filterStr)
+		}
+	}
+}
+
+func TestFilterField(t *testing.T) {
+	successTestCases := append(filterTestCases, filterFieldTestCases...)
+	for _, tc := range successTestCases {
+		q := NewQuery("foo").FilterField(tc.fieldName, tc.operator, 42)
+		if q.err != nil {
+			t.Errorf("%q %q: error: %v", tc.fieldName, tc.operator, q.err)
+			continue
+		}
+		if len(q.filter) != 1 {
+			t.Errorf("%q: len=%d, want %d", tc.fieldName, len(q.filter), 1)
+			continue
+		}
+		got, want := q.filter[0], filter{tc.fieldName, tc.wantOp, 42}
+		if got != want {
+			t.Errorf("%q %q: got %v, want %v", tc.fieldName, tc.operator, got, want)
+			continue
+		}
+	}
+	for _, tc := range filterUnsupported {
+		q := NewQuery("foo").Filter(tc.filterStr, 42)
+		if q.err == nil {
+			t.Errorf("%q: should have thrown error", tc.filterStr)
+		}
+	}
+}
+
+func TestUnquote(t *testing.T) {
+	testCases := []struct {
+		input string
+		want  string
+	}{
+		{`" x "`, ` x `},
+		{`"\" \\\"x \""`, `" \"x "`},
+	}
+
+	for _, tc := range testCases {
+		got, err := unquote(tc.input)
+
+		if err != nil {
+			t.Errorf("error parsing field name: %v", err)
+		}
+
+		if got != tc.want {
+			t.Errorf("field name parsing error: \nwant %v,\ngot %v", tc.want, got)
+		}
+	}
 }
 
 func TestNamespaceQuery(t *testing.T) {