Use FilterEntity instead of 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 {