diff --git a/api/checks/api.go b/api/checks/api.go index e4ecf545dd2..f2f0208caba 100644 --- a/api/checks/api.go +++ b/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 } diff --git a/api/checks/suites.go b/api/checks/suites.go index c28243be763..f1a93a91af7 100644 --- a/api/checks/suites.go +++ b/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 { diff --git a/api/pending_test_runs.go b/api/pending_test_runs.go index 811a50fe530..85370192c58 100644 --- a/api/pending_test_runs.go +++ b/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: diff --git a/api/screenshot/model.go b/api/screenshot/model.go index 49b9af9a987..5383282fdf9 100644 --- a/api/screenshot/model.go +++ b/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) diff --git a/api/test_history.go b/api/test_history.go index 2cdc15b2c9e..5956536942f 100644 --- a/api/test_history.go +++ b/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) diff --git a/api/versions.go b/api/versions.go index 5d638de5018..0493d0ab1c6 100644 --- a/api/versions.go +++ b/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*/), } } diff --git a/shared/datastore.go b/shared/datastore.go index 791b7c20c76..ed2cc4666d0 100644 --- a/shared/datastore.go +++ b/shared/datastore.go @@ -36,7 +36,7 @@ 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 @@ -44,6 +44,18 @@ type Query interface { 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 diff --git a/shared/datastore_cloud.go b/shared/datastore_cloud.go index 40980916d4d..6973a7ee364 100644 --- a/shared/datastore_cloud.go +++ b/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} +} diff --git a/shared/test_run_query.go b/shared/test_run_query.go index 45584007eaa..48e1ac69c2a 100644 --- a/shared/test_run_query.go +++ b/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 {