[cv][api] introduce run.CLQueryBuilder.
Using raw Datastore keys on Run IDs proved very complicated for my
brain, so I decided to encapsulate & test it well.
So, Min .. Max do what one would expect on strings.
And .After() and .Before() are introduced for ordering based on created
time, but limited to specific LUCI project.
R=robertocn, yiwzhang
Change-Id: Ib430e792d0073a22f0a53a88e18255b0b1ea5084
Reviewed-on: https://chromium-review.googlesource.com/c/infra/luci/luci-go/+/3182965
Auto-Submit: Andrii Shyshkalov <tandrii@google.com>
Commit-Queue: Andrii Shyshkalov <tandrii@google.com>
Reviewed-by: Roberto Carrillo <robertocn@chromium.org>
Reviewed-by: Yiwei Zhang <yiwzhang@google.com>
diff --git a/cv/internal/rpc/admin/server.go b/cv/internal/rpc/admin/server.go
index aeab14f..64fe371 100644
--- a/cv/internal/rpc/admin/server.go
+++ b/cv/internal/rpc/admin/server.go
@@ -343,21 +343,16 @@
return nil, nil, err
}
- q := datastore.NewQuery(run.RunCLKind).Eq("IndexedID", cl.ID).Limit(req.GetPageSize()).KeysOnly(true)
+ qb := run.CLQueryBuilder{
+ CLID: cl.ID,
+ Limit: req.GetPageSize(),
+ Project: req.GetProject(), // optional.
+ }
if excl := cursor.GetRun(); excl != "" {
- q = q.Gt("__key__", datastore.MakeKey(ctx, run.RunKind, excl, run.RunCLKind, int64(cl.ID)))
+ qb.Min = common.RunID(excl)
}
- // TODO(tandrii): if req.GetProject() is given, restrict __key__ here.
- var runCLKeys []*datastore.Key
- if err := datastore.GetAll(ctx, q, &runCLKeys); err != nil {
- return nil, nil, errors.Annotate(err, "failed to fetch Runs IDs").Tag(transient.Tag).Err()
- }
-
- runKeys := make([]*datastore.Key, len(runCLKeys))
- for i, k := range runCLKeys {
- runKeys[i] = k.Parent()
- }
- return cl, runKeys, nil
+ runKeys, err := qb.GetAllRunKeys(ctx)
+ return cl, runKeys, err
}
// searchRunsByProject returns Run IDs as Datastore keys, using LUCI Project to
diff --git a/cv/internal/rpc/admin/server_test.go b/cv/internal/rpc/admin/server_test.go
index c61a2a9..64cde3c 100644
--- a/cv/internal/rpc/admin/server_test.go
+++ b/cv/internal/rpc/admin/server_test.go
@@ -470,6 +470,26 @@
total := fetchAll(ctx, req)
So(idsOf(total), ShouldResemble, []string{diffProjectID, laterID, earlierID})
})
+
+ Convey("with CL and project and paging", func() {
+ // Make CL1 included in 3 runs: diffProjectID, laterID, earlierID.
+ So(datastore.Put(ctx,
+ &run.Run{
+ ID: diffProjectID,
+ Status: run.Status_RUNNING,
+ CLs: common.MakeCLIDs(1),
+ },
+ &run.RunCL{Run: datastore.MakeKey(ctx, run.RunKind, diffProjectID), ID: cl1.ID, IndexedID: cl1.ID},
+ ), ShouldBeNil)
+
+ req := &adminpb.SearchRunsRequest{
+ Cl: &adminpb.GetCLRequest{ExternalId: string(cl1.ExternalID)},
+ Project: lProject,
+ PageSize: 1,
+ }
+ total := fetchAll(ctx, req)
+ So(idsOf(total), ShouldResemble, []string{laterID, earlierID})
+ })
})
Convey("runs aross all projects", func() {
diff --git a/cv/internal/run/query.go b/cv/internal/run/query.go
index 9aa609b..29e717d 100644
--- a/cv/internal/run/query.go
+++ b/cv/internal/run/query.go
@@ -18,16 +18,129 @@
"context"
"fmt"
+ "go.chromium.org/luci/common/errors"
+ "go.chromium.org/luci/common/retry/transient"
"go.chromium.org/luci/gae/service/datastore"
+
+ "go.chromium.org/luci/cv/internal/common"
)
// NewQueryWithLUCIProject returns a query for Run entities that belongs
// to the given LUCI Project.
func NewQueryWithLUCIProject(ctx context.Context, project string) *datastore.Query {
+ min, max := rangeOfProjectIDs(project)
+ return datastore.NewQuery(RunKind).
+ Gt("__key__", datastore.MakeKey(ctx, RunKind, min)).
+ Lt("__key__", datastore.MakeKey(ctx, RunKind, max))
+}
+
+// CLQueryBuilder builds datastore.Query for searching Runs of a given CL.
+type CLQueryBuilder struct {
+ // CLID of the CL being searched for. Required.
+ CLID common.CLID
+ // Project optionally restricts Runs to the given LUCI project.
+ Project string
+ // Max restricts query to Runs with ID lexicographically smaller.
+ //
+ // This means query will return union of:
+ // * all Runs created after this Run in the same project,
+ // * all Runs in lexicographically smaller projects,
+ // unless .Project is set to the same project (recommended).
+ Max common.RunID
+ // Min restricts query to Runs with ID lexicographically larger.
+ //
+ // This means query will return union of:
+ // * all Runs created before this Run in the same project,
+ // * all Runs in lexicographically larger projects,
+ // unless .Project is set to the same project (recommended).
+ Min common.RunID
+
+ // Limit limits the number of results if positive. Ignored otherwise.
+ Limit int32
+}
+
+// AfterInProject constrains CLQueryBuilder to Runs created after this Run but
+// belonging to the same LUCI project.
+//
+// Panics if CLQueryBuilder is already constrained to a different LUCI Project.
+func (b CLQueryBuilder) AfterInProject(id common.RunID) CLQueryBuilder {
+ if p := id.LUCIProject(); p != b.Project {
+ if b.Project != "" {
+ panic(fmt.Errorf("invalid CLQueryBuilder.After(%q): .Project is already set to %q", id, b.Project))
+ }
+ b.Project = p
+ }
+ b.Max = id
+ return b
+}
+
+// BeforeInProject constrains CLQueryBuilder to Runs created before this Run but
+// belonging to the same LUCI project.
+//
+// Panics if CLQueryBuilder is already constrained to a different LUCI Project.
+func (b CLQueryBuilder) BeforeInProject(id common.RunID) CLQueryBuilder {
+ if p := id.LUCIProject(); p != b.Project {
+ if b.Project != "" {
+ panic(fmt.Errorf("invalid CLQueryBuilder.Before(%q): .Project is already set to %q", id, b.Project))
+ }
+ b.Project = p
+ }
+ b.Min = id
+ return b
+}
+
+// BuildKeysOnly returns keys-only query on RunCL entities.
+//
+// It's exposed primarily for debugging reasons.
+func (b CLQueryBuilder) BuildKeysOnly(ctx context.Context) *datastore.Query {
+ q := datastore.NewQuery(RunCLKind).Eq("IndexedID", b.CLID).KeysOnly(true)
+
+ if b.Limit > 0 {
+ q.Limit(b.Limit)
+ }
+
+ min := string(b.Min)
+ max := string(b.Max)
+ if b.Project != "" {
+ prMin, prMax := rangeOfProjectIDs(b.Project)
+ if min == "" || min < prMin {
+ min = prMin
+ }
+ if max == "" || max > prMax {
+ max = prMax
+ }
+ }
+ if min != "" {
+ q = q.Gt("__key__", datastore.MakeKey(ctx, RunKind, min, RunCLKind, int64(b.CLID)))
+ }
+ if max != "" {
+ q = q.Lt("__key__", datastore.MakeKey(ctx, RunKind, max, RunCLKind, int64(b.CLID)))
+ }
+
+ return q
+}
+
+func (b CLQueryBuilder) GetAllRunKeys(ctx context.Context) ([]*datastore.Key, error) {
+ // Fetch RunCL keys.
+ var keys []*datastore.Key
+ if err := datastore.GetAll(ctx, b.BuildKeysOnly(ctx), &keys); err != nil {
+ return nil, errors.Annotate(err, "failed to fetch Runs IDs").Tag(transient.Tag).Err()
+ }
+
+ // Replace each RunCL key with its parent (Run) key.
+ for i := range keys {
+ keys[i] = keys[i].Parent()
+ }
+ return keys, nil
+}
+
+// rangeOfProjectIDs returns (min..max) non-existent Run IDs, such that
+// the following
+// min < $RunID < max
+// for all valid $RunID belonging to the given project.
+func rangeOfProjectIDs(project string) (string, string) {
// ID starts with the LUCI Project name followed by '/' and a 13-digit
// number. So it must be lexicographically greater than "project/0" and
// less than "project/:" (int(':') == int('9') + 1).
- return datastore.NewQuery(RunKind).
- Gt("__key__", datastore.MakeKey(ctx, RunKind, fmt.Sprintf("%s/0", project))).
- Lt("__key__", datastore.MakeKey(ctx, RunKind, fmt.Sprintf("%s/:", project)))
+ return project + "/0", project + "/:"
}
diff --git a/cv/internal/run/query_test.go b/cv/internal/run/query_test.go
new file mode 100644
index 0000000..67a5404
--- /dev/null
+++ b/cv/internal/run/query_test.go
@@ -0,0 +1,142 @@
+// Copyright 2021 The LUCI Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package run
+
+import (
+ "testing"
+ "time"
+
+ "go.chromium.org/luci/gae/service/datastore"
+
+ "go.chromium.org/luci/cv/internal/common"
+ "go.chromium.org/luci/cv/internal/cvtesting"
+
+ . "github.com/smartystreets/goconvey/convey"
+)
+
+func TestCLQueryBuilder(t *testing.T) {
+ t.Parallel()
+
+ Convey("CLQueryBuilder works", t, func() {
+ ct := cvtesting.Test{}
+ ctx, cancel := ct.SetUp()
+ defer cancel()
+
+ getAll := func(qb CLQueryBuilder) (out common.RunIDs) {
+ keys, err := qb.GetAllRunKeys(ctx)
+ So(err, ShouldBeNil)
+ for _, k := range keys {
+ out = append(out, common.RunID(k.StringID()))
+ }
+ return out
+ }
+
+ makeRun := func(proj string, delay time.Duration, clids ...common.CLID) common.RunID {
+ createdAt := ct.Clock.Now().Add(delay)
+ runID := common.MakeRunID(proj, createdAt, 1, []byte{0, byte(delay / time.Millisecond)})
+ So(datastore.Put(ctx, &Run{ID: runID, CLs: clids}), ShouldBeNil)
+ for _, clid := range clids {
+ So(datastore.Put(ctx, &RunCL{
+ Run: datastore.MakeKey(ctx, RunKind, string(runID)),
+ ID: clid,
+ IndexedID: clid,
+ }), ShouldBeNil)
+ }
+ return runID
+ }
+
+ clA, clB, clZ := common.CLID(1), common.CLID(2), common.CLID(3)
+
+ // RunID below are ordered lexicographically.
+ bond9 := makeRun("bond", 9*time.Millisecond, clA)
+ bond4 := makeRun("bond", 4*time.Millisecond, clA, clB)
+ bond2 := makeRun("bond", 2*time.Millisecond, clA)
+ dart5 := makeRun("dart", 5*time.Millisecond, clA)
+ dart3 := makeRun("dart", 3*time.Millisecond, clA)
+ rust1 := makeRun("rust", 1*time.Millisecond, clA, clB)
+ xero7 := makeRun("xero", 7*time.Millisecond, clA)
+
+ Convey("CL without Runs", func() {
+ qb := CLQueryBuilder{CLID: clZ}
+ So(getAll(qb), ShouldResemble, common.RunIDs(nil))
+ })
+
+ Convey("CL with some Runs", func() {
+ qb := CLQueryBuilder{CLID: clB}
+ So(getAll(qb), ShouldResemble, common.RunIDs{bond4, rust1})
+ })
+
+ Convey("CL with all Runs", func() {
+ qb := CLQueryBuilder{CLID: clA}
+ So(getAll(qb), ShouldResemble, common.RunIDs{bond9, bond4, bond2, dart5, dart3, rust1, xero7})
+ })
+
+ Convey("Filter by Project", func() {
+ qb := CLQueryBuilder{CLID: clA, Project: "bond"}
+ So(getAll(qb), ShouldResemble, common.RunIDs{bond9, bond4, bond2})
+ })
+
+ Convey("Filtering by Project and Min with diff project", func() {
+ qb := CLQueryBuilder{CLID: clA, Project: "dart", Min: bond4}
+ So(getAll(qb), ShouldResemble, common.RunIDs{dart5, dart3})
+
+ qb = CLQueryBuilder{CLID: clA, Project: "dart", Min: rust1}
+ _, err := qb.BuildKeysOnly(ctx).Finalize()
+ So(err, ShouldEqual, datastore.ErrNullQuery)
+ })
+
+ Convey("Filtering by Project and Max with diff project", func() {
+ qb := CLQueryBuilder{CLID: clA, Project: "dart", Max: xero7}
+ So(getAll(qb), ShouldResemble, common.RunIDs{dart5, dart3})
+
+ qb = CLQueryBuilder{CLID: clA, Project: "dart", Max: bond4}
+ _, err := qb.BuildKeysOnly(ctx).Finalize()
+ So(err, ShouldEqual, datastore.ErrNullQuery)
+ })
+
+ Convey("Before", func() {
+ qb := CLQueryBuilder{CLID: clA}.BeforeInProject(bond9)
+ So(getAll(qb), ShouldResemble, common.RunIDs{bond4, bond2})
+
+ qb = CLQueryBuilder{CLID: clA}.BeforeInProject(bond4)
+ So(getAll(qb), ShouldResemble, common.RunIDs{bond2})
+
+ qb = CLQueryBuilder{CLID: clA}.BeforeInProject(bond2)
+ So(getAll(qb), ShouldResemble, common.RunIDs(nil))
+ })
+
+ Convey("After", func() {
+ qb := CLQueryBuilder{CLID: clA}.AfterInProject(bond2)
+ So(getAll(qb), ShouldResemble, common.RunIDs{bond9, bond4})
+
+ qb = CLQueryBuilder{CLID: clA}.AfterInProject(bond4)
+ So(getAll(qb), ShouldResemble, common.RunIDs{bond9})
+
+ qb = CLQueryBuilder{CLID: clA}.AfterInProject(bond9)
+ So(getAll(qb), ShouldResemble, common.RunIDs(nil))
+ })
+
+ Convey("After and Before", func() {
+ qb := CLQueryBuilder{CLID: clA}.AfterInProject(bond2).BeforeInProject(bond9)
+ So(getAll(qb), ShouldResemble, common.RunIDs{bond4})
+ })
+
+ Convey("Invalid usage panics", func() {
+ So(func() { CLQueryBuilder{CLID: clA, Project: "dart"}.BeforeInProject(bond2) }, ShouldPanic)
+ So(func() { CLQueryBuilder{CLID: clA, Project: "dart"}.AfterInProject(bond2) }, ShouldPanic)
+ So(func() { CLQueryBuilder{CLID: clA}.AfterInProject(dart3).BeforeInProject(xero7) }, ShouldPanic)
+ })
+ })
+}
diff --git a/cv/internal/userhtml/common.go b/cv/internal/userhtml/common.go
index 9363ff1..b940623 100644
--- a/cv/internal/userhtml/common.go
+++ b/cv/internal/userhtml/common.go
@@ -81,6 +81,9 @@
"Split": func(s string) []string {
return strings.Split(s, "\n")
},
+ "SplitSlash": func(s string) []string {
+ return strings.Split(s, "/")
+ },
"Title": func(s string) string {
return strings.Title(strings.ToLower(strings.Replace(s, "_", " ", -1)))
},
