Merge remote-tracking branch 'refs/remotes/upstream/master'

# Conflicts:
#	docstore/driver/driver.go
#	docstore/drivertest/drivertest.go
#	docstore/gcpfirestore/testdata/TestConformance/ActionsOnStructNoRev.replay
#	docstore/gcpfirestore/testdata/TestConformance/ActionsWithCompositeID.replay
#	docstore/gcpfirestore/testdata/TestConformance/As/verify_As.replay
#	docstore/gcpfirestore/testdata/TestConformance/As/verify_As_returns_false_when_passed_nil.replay
#	docstore/gcpfirestore/testdata/TestConformance/BeforeDo.replay
#	docstore/gcpfirestore/testdata/TestConformance/BeforeQuery.replay
#	docstore/gcpfirestore/testdata/TestConformance/Create.replay
#	docstore/gcpfirestore/testdata/TestConformance/Data.replay
#	docstore/gcpfirestore/testdata/TestConformance/Delete.replay
#	docstore/gcpfirestore/testdata/TestConformance/ExampleInDoc.replay
#	docstore/gcpfirestore/testdata/TestConformance/Get.replay
#	docstore/gcpfirestore/testdata/TestConformance/GetQuery.replay
#	docstore/gcpfirestore/testdata/TestConformance/GetQueryKeyField.replay
#	docstore/gcpfirestore/testdata/TestConformance/MultipleActions.replay
#	docstore/gcpfirestore/testdata/TestConformance/Proto.replay
#	docstore/gcpfirestore/testdata/TestConformance/Put.replay
#	docstore/gcpfirestore/testdata/TestConformance/Replace.replay
#	docstore/gcpfirestore/testdata/TestConformance/SerializeRevision.replay
#	docstore/gcpfirestore/testdata/TestConformance/Update.replay
#	docstore/memdocstore/query.go
#	docstore/mongodocstore/query.go
#	docstore/query.go
#	go.mod

docstore/driver/driver.go

@@ -214,7 +214,7 @@ type Query struct {
 // TODO(#1762): support comparison of other types.
 type Filter struct {
 	FieldPath []string    // the field path to filter
-	Op        string      // the operation, supports =, >, >=, <, <=, array-contains, array-contains-any
+	Op        string      // the operation, supports `=`, `>`, `>=`, `<`, `<=`, `in`, `not-in`
 	Value     interface{} // the value to compare using the operation
 }
 

docstore/drivertest/drivertest.go

@@ -249,7 +249,7 @@ func withColl(t *testing.T, h Harness, kind CollectionKind, f func(*testing.T, H
 	}
 	coll := docstore.NewCollection(dc)
 	defer coll.Close()
-	clearCollection(t, coll)
+	ClearCollection(t, coll)
 	f(t, h, coll)
 }
 
@@ -1228,7 +1228,6 @@ type HighScore struct {
 	Player           string
 	Score            int
 	Time             time.Time
-	Tags             []string
 	DocstoreRevision interface{}
 }
 
@@ -1273,14 +1272,14 @@ const (
 )
 
 var highScores = []*HighScore{
-	{game1, "pat", 49, date(3, 13), []string{"tag1"}, nil},
-	{game1, "mel", 60, date(4, 10), []string{"tag2"}, nil},
-	{game1, "andy", 81, date(2, 1), []string{"tag3"}, nil},
-	{game1, "fran", 33, date(3, 19), []string{"tag4"}, nil},
-	{game2, "pat", 120, date(4, 1), []string{"tag2"}, nil},
-	{game2, "billie", 111, date(4, 10), []string{"tag1"}, nil},
-	{game2, "mel", 190, date(4, 18), []string{"tag2"}, nil},
-	{game2, "fran", 33, date(3, 20), []string{"tag1"}, nil},
+	{game1, "pat", 49, date(3, 13), nil},
+	{game1, "mel", 60, date(4, 10), nil},
+	{game1, "andy", 81, date(2, 1), nil},
+	{game1, "fran", 33, date(3, 19), nil},
+	{game2, "pat", 120, date(4, 1), nil},
+	{game2, "billie", 111, date(4, 10), nil},
+	{game2, "mel", 190, date(4, 18), nil},
+	{game2, "fran", 33, date(3, 20), nil},
 }
 
 func addHighScores(t *testing.T, coll *docstore.Collection) {
@@ -1463,21 +1462,6 @@ func testGetQuery(t *testing.T, _ Harness, coll *docstore.Collection) {
 			want:   func(h *HighScore) bool { return h.Game == game1 },
 			before: func(h1, h2 *HighScore) bool { return h1.Player > h2.Player },
 		},
-		{
-			name: "Tags",
-			q:    coll.Query().Where("Tags", "array-contains", "tag1"),
-			want: func(h *HighScore) bool { return contains(h.Tags, "tag1") },
-		},
-		{
-			name: "Tags array-contains-any",
-			q:    coll.Query().Where("Tags", "array-contains-any", []string{"tag1", "tag2"}),
-			want: func(h *HighScore) bool { return contains(h.Tags, "tag1") || contains(h.Tags, "tag2") },
-		},
-		{
-			name: "Player in",
-			q:    coll.Query().Where("Player", "in", []string{"billie", "pat"}),
-			want: func(h *HighScore) bool { return h.Player == "billie" || h.Player == "pat" },
-		},
 		// TODO(jba): add more OrderBy tests.
 		{
 			name:   "AllWithKeyFields",
@@ -1486,7 +1470,6 @@ func testGetQuery(t *testing.T, _ Harness, coll *docstore.Collection) {
 			want: func(h *HighScore) bool {
 				h.Score = 0
 				h.Time = time.Time{}
-				h.Tags = nil
 				return true
 			},
 		},
@@ -1496,7 +1479,6 @@ func testGetQuery(t *testing.T, _ Harness, coll *docstore.Collection) {
 			fields: []docstore.FieldPath{"Game", "Player", "Score", docstore.FieldPath(docstore.DefaultRevisionField)},
 			want: func(h *HighScore) bool {
 				h.Time = time.Time{}
-				h.Tags = nil
 				return true
 			},
 		},
@@ -1559,17 +1541,8 @@ func filterHighScores(hs []*HighScore, f func(*HighScore) bool) []*HighScore {
 	return res
 }
 
-func contains(s []string, v string) bool {
-	for _, s2 := range s {
-		if s2 == v {
-			return true
-		}
-	}
-	return false
-}
-
-// clearCollection delete all documents from this collection after test.
-func clearCollection(fataler interface{ Fatalf(string, ...interface{}) }, coll *docstore.Collection) {
+// ClearCollection delete all documents from this collection after test.
+func ClearCollection(fataler interface{ Fatalf(string, ...interface{}) }, coll *docstore.Collection) {
 	ctx := context.Background()
 	iter := coll.Query().Get(ctx)
 	dels := coll.Actions()
@@ -1969,7 +1942,7 @@ func testAs(t *testing.T, coll *docstore.Collection, st AsTest) {
 	}
 
 	// ErrorCheck
-	doc := &HighScore{game3, "steph", 24, date(4, 25), nil, nil}
+	doc := &HighScore{game3, "steph", 24, date(4, 25), nil}
 	if err := coll.Create(ctx, doc); err != nil {
 		t.Fatal(err)
 	}

docstore/memdocstore/query.go

@@ -79,61 +79,14 @@ func filterMatches(f driver.Filter, doc storedDoc) bool {
 	if err != nil {
 		return false
 	}
-	if f.Op == "array-contains" {
-		return applyArrayComparison(docval, f.Value)
-	}
-	if f.Op == "array-contains-any" {
-		value := reflect.ValueOf(f.Value)
-		if value.Kind() != reflect.Slice {
-			return false
-		}
-
-		for i := 0; i < value.Len(); i++ {
-			if applyArrayComparison(docval, value.Index(i).Interface()) {
-				return true
-			}
-		}
-		return false
-	}
-	if f.Op == "in" {
-		value := reflect.ValueOf(f.Value)
-		if value.Kind() != reflect.Slice {
-			return false
-		}
-		for i := 0; i < value.Len(); i++ {
-			c, ok := compare(docval, value.Index(i).Interface())
-			if ok && c == 0 {
-				return true
-			}
-		}
-		return false
-	}
 	c, ok := compare(docval, f.Value)
 	if !ok {
 		return false
 	}
 	return applyComparison(f.Op, c)
 }
 
-func applyArrayComparison(docval, value interface{}) bool {
-	v := reflect.ValueOf(docval)
-	if v.Kind() != reflect.Slice {
-		return false
-	}
-	s := docval.([]interface{})
-	for _, v := range s {
-		c, ok := compare(v, value)
-		if !ok {
-			return false
-		}
-		if c == 0 {
-			return true
-		}
-	}
-	return false
-}
-
-// op is one of the five permitted docstore operators ("=", "<", etc.)
+// op is one of the permitted docstore operators ("=", "<", etc.)
 // c is the result of strings.Compare or the like.
 // TODO(jba): dedup from gcpfirestore/query?
 func applyComparison(op string, c int) bool {
@@ -148,6 +101,10 @@ func applyComparison(op string, c int) bool {
 		return c >= 0
 	case "<=":
 		return c <= 0
+	case "in":
+		return c == 0
+	case "not-in":
+		return c != 0
 	default:
 		panic("bad op")
 	}
@@ -156,6 +113,21 @@ func applyComparison(op string, c int) bool {
 func compare(x1, x2 interface{}) (int, bool) {
 	v1 := reflect.ValueOf(x1)
 	v2 := reflect.ValueOf(x2)
+	// this is for in/not-in queries.
+	// return 0 if x1 is in slice x2, -1 if not.
+	if v2.Kind() == reflect.Slice {
+		for i := 0; i < v2.Len(); i++ {
+			if c, ok := compare(x1, v2.Index(i).Interface()); ok {
+				if !ok {
+					return 0, false
+				}
+				if c == 0 {
+					return 0, true
+				}
+			}
+		}
+		return -1, true
+	}
 	if v1.Kind() == reflect.String && v2.Kind() == reflect.String {
 		return strings.Compare(v1.String(), v2.String()), true
 	}
@@ -167,12 +139,6 @@ func compare(x1, x2 interface{}) (int, bool) {
 			return driver.CompareTimes(t1, t2), true
 		}
 	}
-	if v1.Kind() == reflect.Bool && v2.Kind() == reflect.Bool {
-		if v1.Bool() == v2.Bool() {
-			return 0, true
-		}
-		return -1, true
-	}
 	return 0, false
 }
 

docstore/mongodocstore/query.go

@@ -70,12 +70,13 @@ func (c *collection) RunGetQuery(ctx context.Context, q *driver.Query) (driver.D
 }
 
 var mongoQueryOps = map[string]string{
-	driver.EqualOp:       "$eq",
-	">":                  "$gt",
-	">=":                 "$gte",
-	"<":                  "$lt",
-	"<=":                 "$lte",
-	"array-contains-any": "$in",
+	driver.EqualOp: "$eq",
+	">":            "$gt",
+	">=":           "$gte",
+	"<":            "$lt",
+	"<=":           "$lte",
+	"in":           "$in",
+	"not-in":       "$nin",
 }
 
 // filtersToBSON converts a []driver.Filter to the MongoDB equivalent, expressed
@@ -104,10 +105,6 @@ func (c *collection) filterToBSON(f driver.Filter) (bson.E, error) {
 	if c.idField != "" && key == c.idField {
 		key = mongoIDField
 	}
-	if f.Op == "array-contains" {
-		f.Op = "array-contains-any"
-		f.Value = []interface{}{f.Value}
-	}
 	val, err := encodeValue(f.Value)
 	if err != nil {
 		return bson.E{}, err

docstore/query.go

@@ -37,8 +37,8 @@ func (c *Collection) Query() *Query {
 }
 
 // Where expresses a condition on the query.
-// Valid ops are: "=", ">", "<", ">=", "<=", "array-contains", "array-contains-any", "in".
-// Valid values are strings, integers, floating-point numbers, boolean (only with "="), slice (only with "array-contains-any" and "in") and time.Time values.
+// Valid ops are: "=", ">", "<", ">=", "<=, "in", "not-in".
+// Valid values are strings, integers, floating-point numbers, and time.Time values.
 func (q *Query) Where(fp FieldPath, op string, value interface{}) *Query {
 	if q.err != nil {
 		return q
@@ -48,11 +48,12 @@ func (q *Query) Where(fp FieldPath, op string, value interface{}) *Query {
 		q.err = err
 		return q
 	}
-	if !validOp[op] {
-		return q.invalidf("invalid filter operator: %q. Use one of: =, >, <, >=, <=, array-contains, array-contains-any", op)
+	validator, ok := validOp[op]
+	if !ok {
+		return q.invalidf("invalid filter operator: %q. Use one of: =, >, <, >=, <=, in, not-in", op)
 	}
-	if !validFilterValue(op, value) {
-		return q.invalidf("invalid filter value: %v for operator %q", value, op)
+	if !validator(value) {
+		return q.invalidf("invalid filter value: %v", value)
 	}
 	q.dq.Filters = append(q.dq.Filters, driver.Filter{
 		FieldPath: pfp,
@@ -62,29 +63,26 @@ func (q *Query) Where(fp FieldPath, op string, value interface{}) *Query {
 	return q
 }
 
-var validOp = map[string]bool{
-	"=":                  true,
-	">":                  true,
-	"<":                  true,
-	">=":                 true,
-	"<=":                 true,
-	"array-contains":     true,
-	"array-contains-any": true,
-	"in":                 true,
+type valueValidator func(interface{}) bool
+
+var validOp = map[string]valueValidator{
+	"=":      validFilterValue,
+	">":      validFilterValue,
+	"<":      validFilterValue,
+	">=":     validFilterValue,
+	"<=":     validFilterValue,
+	"in":     validFilterSlice,
+	"not-in": validFilterSlice,
 }
 
-func validFilterValue(op string, v interface{}) bool {
+func validFilterValue(v interface{}) bool {
 	if v == nil {
 		return false
 	}
 	if _, ok := v.(time.Time); ok {
 		return true
 	}
-	k := reflect.TypeOf(v).Kind()
-	if op == "array-contains-any" || op == "in" {
-		return k == reflect.Slice
-	}
-	switch k {
+	switch reflect.TypeOf(v).Kind() {
 	case reflect.String:
 		return true
 	case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
@@ -93,13 +91,24 @@ func validFilterValue(op string, v interface{}) bool {
 		return true
 	case reflect.Float32, reflect.Float64:
 		return true
-	case reflect.Bool:
-		return op == "="
 	default:
 		return false
 	}
 }
 
+func validFilterSlice(v interface{}) bool {
+	if v == nil || reflect.TypeOf(v).Kind() != reflect.Slice {
+		return false
+	}
+	vv := reflect.ValueOf(v)
+	for i := 0; i < vv.Len(); i++ {
+		if !validFilterValue(vv.Index(i).Interface()) {
+			return false
+		}
+	}
+	return true
+}
+
 // Limit will limit the results to at most n documents.
 // n must be positive.
 // It is an error to specify Limit more than once in a Get query, or