Use FilterEntity instead of Filter

The datastore library has marked Filter as deprecated. By moving to
FilterEntity, we can use more advanced queries as well.

This commit introduces a new EntityFilter type. That type just wraps
datastore.EntityFilter because there are no exposed methods on
datastore.EntityFilter for us to manually declare.

In order to create an EntityFilter, another type called FilterBuilder is
created which will allow consumers to make a particular Filter that
reteurns an EntityFilter.

More about the deprecation here: https://pkg.go.dev/cloud.google.com/go/datastore#Query.Filter

api/checks/api.go

@@ -76,7 +76,8 @@ func (s checksAPIImpl) ScheduleResultsProcessing(sha string, product shared.Prod
 func (s checksAPIImpl) GetSuitesForSHA(sha string) ([]shared.CheckSuite, error) {
 	var suites []shared.CheckSuite
 	store := shared.NewAppEngineDatastore(s.Context(), false)
-	_, err := store.GetAll(store.NewQuery("CheckSuite").Filter("SHA =", sha), &suites)
+	q := store.NewQuery("CheckSuite")
+	_, err := store.GetAll(q.FilterEntity(q.FilterBuilder().PropertyFilter("SHA", "=", sha)), &suites)
 
 	return suites, err
 }

api/checks/suites.go

@@ -20,12 +20,14 @@ func getOrCreateCheckSuite(
 	prNumbers ...int,
 ) (*shared.CheckSuite, error) {
 	ds := shared.NewAppEngineDatastore(ctx, false)
-	query := ds.NewQuery("CheckSuite").
-		Filter("SHA =", sha).
-		Filter("AppID =", appID).
-		Filter("InstallationID =", installationID).
-		Filter("Owner =", owner).
-		Filter("Repo =", repo).
+	query := ds.NewQuery("CheckSuite")
+	filterBuilder := query.FilterBuilder()
+	query = query.
+		FilterEntity(filterBuilder.PropertyFilter("SHA", "=", sha)).
+		FilterEntity(filterBuilder.PropertyFilter("AppID", "=", appID)).
+		FilterEntity(filterBuilder.PropertyFilter("InstallationID", "=", installationID)).
+		FilterEntity(filterBuilder.PropertyFilter("Owner", "=", owner)).
+		FilterEntity(filterBuilder.PropertyFilter("Repo", "=", repo)).
 		KeysOnly()
 	var suite shared.CheckSuite
 	if keys, err := ds.GetAll(query, nil); err != nil {

api/pending_test_runs.go

@@ -23,15 +23,16 @@ func apiPendingTestRunsHandler(w http.ResponseWriter, r *http.Request) {
 
 	filter := strings.ToLower(mux.Vars(r)["filter"])
 	q := store.NewQuery("PendingTestRun")
+	filterBuilder := q.FilterBuilder()
 	switch filter {
 	case "pending":
-		q = q.Order("-Stage").Filter("Stage < ", int(shared.StageValid))
+		q = q.Order("-Stage").FilterEntity(filterBuilder.PropertyFilter("Stage", "<", int(shared.StageValid)))
 	case "invalid":
-		q = q.Filter("Stage = ", int(shared.StageInvalid))
+		q = q.FilterEntity(filterBuilder.PropertyFilter("Stage", "=", int(shared.StageInvalid)))
 	case "empty":
-		q = q.Filter("Stage = ", int(shared.StageEmpty))
+		q = q.FilterEntity(filterBuilder.PropertyFilter("Stage", "=", int(shared.StageEmpty)))
 	case "duplicate":
-		q = q.Filter("Stage = ", int(shared.StageDuplicate))
+		q = q.FilterEntity(filterBuilder.PropertyFilter("Stage", "= ", int(shared.StageDuplicate)))
 	case "":
 		// No-op
 	default:

api/screenshot/model.go

@@ -140,7 +140,7 @@ func RecentScreenshotHashes(
 	for all.Cardinality() < totalLimit {
 		query := ds.NewQuery("Screenshot")
 		for _, l := range labels {
-			query = query.Filter("Labels =", l)
+			query = query.FilterEntity(query.FilterBuilder().PropertyFilter("Labels", "=", l))
 		}
 		query = query.Order("-LastUsed").Limit(totalLimit)
 

api/test_history.go

@@ -54,7 +54,8 @@ func testHistoryHandler(w http.ResponseWriter, r *http.Request) {
 	}
 
 	store := shared.NewAppEngineDatastore(ctx, false)
-	q := store.NewQuery("TestHistoryEntry").Filter("TestName =", reqBody.TestName)
+	q := store.NewQuery("TestHistoryEntry")
+	q = q.FilterEntity(q.FilterBuilder().PropertyFilter("TestName", "=", reqBody.TestName))
 
 	var runs []shared.TestHistoryEntry
 	_, err = store.GetAll(q, &runs)

api/versions.go

@@ -49,10 +49,12 @@ func (h VersionsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 
 	ctx := h.ctx
 	store := shared.NewAppEngineDatastore(ctx, false)
-	query := store.NewQuery("TestRun").Filter("BrowserName =", product.BrowserName)
+	query := store.NewQuery("TestRun")
+	filterBuilder := query.FilterBuilder()
+	query = query.FilterEntity(filterBuilder.PropertyFilter("BrowserName", "=", product.BrowserName))
 	if product.Labels != nil {
 		for label := range product.Labels.Iter() {
-			query = query.Filter("Labels =", label)
+			query = query.FilterEntity(filterBuilder.PropertyFilter("Labels", "=", label))
 		}
 	}
 	distinctQuery := query.Project("BrowserVersion").Distinct()
@@ -61,7 +63,7 @@ func (h VersionsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		queries = []shared.Query{distinctQuery}
 	} else {
 		queries = []shared.Query{
-			query.Filter("BrowserVersion =", product.BrowserVersion).Limit(1),
+			query.FilterEntity(filterBuilder.PropertyFilter("BrowserVersion", "=", product.BrowserVersion)).Limit(1),
 			shared.VersionPrefix(distinctQuery, "BrowserVersion", product.BrowserVersion, false /*desc*/),
 		}
 	}

shared/datastore.go

@@ -36,14 +36,26 @@ type Iterator interface {
 
 // Query abstracts a datastore.Query
 type Query interface {
-	Filter(filterStr string, value interface{}) Query
+	FilterEntity(EntityFilter) Query
 	Project(fields ...string) Query
 	Limit(limit int) Query
 	Offset(offset int) Query
 	Order(order string) Query
 	KeysOnly() Query
 	Distinct() Query
 	Run(Datastore) Iterator
+
+	FilterBuilder() FilterBuilder
+}
+
+// PropertyFilter is a particular filter that filters based on property value.
+type PropertyFilter interface {
+	EntityFilter
+}
+
+// FilterBuilder contains the logic to create different types of filters
+type FilterBuilder interface {
+	PropertyFilter(FieldName string, Operator string, Value interface{}) EntityFilter
 }
 
 // Datastore abstracts a datastore, hiding the distinctions between cloud and

shared/datastore_cloud.go

@@ -197,6 +197,14 @@ type cloudQuery struct {
 	query *datastore.Query
 }
 
+func (q cloudQuery) FilterBuilder() FilterBuilder {
+	return cloudFilterBuilder{}
+}
+
+func (q cloudQuery) FilterEntity(entityFilter EntityFilter) Query {
+	return cloudQuery{q.query.FilterEntity(entityFilter)}
+}
+
 func (q cloudQuery) Filter(filterStr string, value interface{}) Query {
 	return cloudQuery{q.query.Filter(filterStr, value)}
 }
@@ -240,3 +248,16 @@ func (i cloudIterator) Next(dst interface{}) (Key, error) {
 	key, err := i.iter.Next(dst)
 	return cloudKey{key}, err
 }
+
+// EntityFilter wraps datastore.EntityFilter.
+// datastore.EntityFilter does not expose any methods. But using this type
+// allows us to be strict on the filters returned by the FilterBuilder.
+type EntityFilter interface {
+	datastore.EntityFilter
+}
+
+type cloudFilterBuilder struct{}
+
+func (b cloudFilterBuilder) PropertyFilter(FieldName string, Operator string, Value interface{}) EntityFilter {
+	return datastore.PropertyFilter{FieldName: FieldName, Operator: Operator, Value: Value}
+}

shared/test_run_query.go

@@ -119,13 +119,14 @@ func (t testRunQueryImpl) LoadTestRunKeys(
 	log := GetLogger(t.store.Context())
 	result = make(KeysByProduct, len(products))
 	baseQuery := t.store.NewQuery("TestRun")
+	filterBuilder := baseQuery.FilterBuilder()
 	if offset != nil {
 		baseQuery = baseQuery.Offset(*offset)
 	}
 	if labels != nil {
 		labels.Remove("") // Ensure the empty string isn't present.
 		for i := range labels.Iter() {
-			baseQuery = baseQuery.Filter("Labels =", i.(string))
+			baseQuery = baseQuery.FilterEntity(filterBuilder.PropertyFilter("Labels", "=", i.(string)))
 		}
 	}
 	var globalIDFilter mapset.Set
@@ -143,10 +144,10 @@ func (t testRunQueryImpl) LoadTestRunKeys(
 
 	for i, product := range products {
 		var productIDFilter = merge(globalIDFilter, nil)
-		query := baseQuery.Filter("BrowserName =", product.BrowserName)
+		query := baseQuery.FilterEntity(filterBuilder.PropertyFilter("BrowserName", "=", product.BrowserName))
 		if product.Labels != nil {
 			for i := range product.Labels.Iter() {
-				query = query.Filter("Labels =", i.(string))
+				query = query.FilterEntity(filterBuilder.PropertyFilter("Labels", "=", i.(string)))
 			}
 		}
 		if !IsLatest(product.Revision) {
@@ -180,10 +181,10 @@ func (t testRunQueryImpl) LoadTestRunKeys(
 			// TODO(lukebjerring): Indexes + filtering for OS + version.
 			query = query.Order("-TimeStart")
 			if from != nil {
-				query = query.Filter("TimeStart >=", *from)
+				query = query.FilterEntity(filterBuilder.PropertyFilter("TimeStart", ">=", *from))
 			}
 			if to != nil {
-				query = query.Filter("TimeStart <", *to)
+				query = query.FilterEntity(filterBuilder.PropertyFilter("TimeStart", "<", *to))
 			}
 			max := MaxCountMaxValue
 			if limit != nil && *limit < MaxCountMaxValue {
@@ -267,17 +268,18 @@ func (t testRunQueryImpl) GetAlignedRunSHAs(
 	query := t.store.
 		NewQuery("TestRun").
 		Order("-TimeStart")
+	filterBuilder := query.FilterBuilder()
 
 	if labels != nil {
 		for i := range labels.Iter() {
-			query = query.Filter("Labels =", i.(string))
+			query = query.FilterEntity(filterBuilder.PropertyFilter("Labels", "=", i.(string)))
 		}
 	}
 	if from != nil {
-		query = query.Filter("TimeStart >=", *from)
+		query = query.FilterEntity(filterBuilder.PropertyFilter("TimeStart", ">=", *from))
 	}
 	if to != nil {
-		query = query.Filter("TimeStart <", *to)
+		query = query.FilterEntity(filterBuilder.PropertyFilter("TimeStart", "<", *to))
 	}
 
 	productsBySHA := make(map[string]mapset.Set)
@@ -356,16 +358,17 @@ func contains(s []string, x string) bool {
 func loadIDsForRevision(store Datastore, query Query, sha string) (result mapset.Set, err error) {
 	log := GetLogger(store.Context())
 	var revQuery Query
+	filterBuilder := query.FilterBuilder()
 	if len(sha) < 40 {
 		log.Debugf("Finding revisions %s <= SHA < %s", sha, sha+"g")
 		revQuery = query.
 			Order("FullRevisionHash").
 			Limit(MaxCountMaxValue).
-			Filter("FullRevisionHash >=", sha).
-			Filter("FullRevisionHash <", sha+"g") // g > f
+			FilterEntity(filterBuilder.PropertyFilter("FullRevisionHash", ">=", sha)).
+			FilterEntity(filterBuilder.PropertyFilter("FullRevisionHash", "<", sha+"g")) // g > f
 	} else {
 		log.Debugf("Finding exact revision %s", sha)
-		revQuery = query.Filter("FullRevisionHash =", sha[:40])
+		revQuery = query.FilterEntity(filterBuilder.PropertyFilter("FullRevisionHash", "=", sha[:40]))
 	}
 
 	var keys []Key
@@ -393,7 +396,8 @@ func loadIDsForBrowserVersion(store Datastore, query Query, version string) (res
 		result.Add(id)
 	}
 	// By exact match
-	if keys, err = store.GetAll(query.Filter("BrowserVersion =", version).KeysOnly(), nil); err != nil {
+	if keys, err = store.GetAll(query.FilterEntity(
+		query.FilterBuilder().PropertyFilter("BrowserVersion", "=", version)).KeysOnly(), nil); err != nil {
 		return nil, err
 	}
 	for _, id := range GetTestRunIDs(keys) {
@@ -409,11 +413,14 @@ func VersionPrefix(query Query, fieldName, versionPrefix string, desc bool) Quer
 	if desc {
 		order = "-" + order
 	}
+	filterBuilder := query.FilterBuilder()
 	return query.
 		Limit(MaxCountMaxValue).
 		Order(order).
-		Filter(fieldName+" >=", fmt.Sprintf("%s.", versionPrefix)).
-		Filter(fieldName+" <=", fmt.Sprintf("%s.%c", versionPrefix, '9'+1))
+		FilterEntity(
+			filterBuilder.PropertyFilter(fieldName, ">=", fmt.Sprintf("%s.", versionPrefix))).
+		FilterEntity(
+			filterBuilder.PropertyFilter(fieldName, "<=", fmt.Sprintf("%s.%c", versionPrefix, '9'+1)))
 }
 
 func getTestRunRedisKey(id int64) string {