internal/reviews: add GerritChanges and CollectChangePreds
These are the building blocks that a particular Gerrit project
needs to build a review dashboard.
Change-Id: Ifb803ff66320470647da234b001e6392f963e485
Reviewed-on: https://go-review.googlesource.com/c/oscar/+/619217
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Zvonimir Pavlinovic <zpavlinovic@google.com>
diff --git a/internal/reviews/collect.go b/internal/reviews/collect.go
new file mode 100644
index 0000000..81d1eeb
--- /dev/null
+++ b/internal/reviews/collect.go
@@ -0,0 +1,108 @@
+// Copyright 2024 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package reviews
+
+import (
+ "cmp"
+ "context"
+ "iter"
+ "log/slog"
+ "runtime"
+ "slices"
+ "strconv"
+ "strings"
+ "sync"
+)
+
+// CollectChangePreds reads [Change] values from an iterator and
+// collects them into a slice of [ChangePreds] values.
+// The slice is sorted by the predicate default values.
+func CollectChangePreds(ctx context.Context, lg *slog.Logger, it iter.Seq[Change]) []ChangePreds {
+ // Applying predicates can be time consuming,
+ // and there can be a lot of changes,
+ // so shard the work.
+ count := runtime.GOMAXPROCS(0)
+ chIn := make(chan Change, count)
+ chOut := make(chan ChangePreds, count)
+
+ var wg sync.WaitGroup
+ wg.Add(1)
+ go func() {
+ defer wg.Done()
+ defer close(chIn)
+ for change := range it {
+ select {
+ case chIn <- change:
+ case <-ctx.Done():
+ return
+ }
+ }
+ }()
+
+ wg.Add(count)
+ for range count {
+ go func() {
+ defer wg.Done()
+ for change := range chIn {
+ cp, ok, err := ApplyPredicates(change)
+ if err != nil {
+ // Errors are assumed to be
+ // non-critical. Just log them.
+ lg.Error("error applying predicate", "change", change.ID(), "err", err)
+ }
+ if !ok {
+ continue
+ }
+
+ select {
+ case chOut <- cp:
+ case <-ctx.Done():
+ return
+ }
+ }
+ }()
+ }
+
+ go func() {
+ wg.Wait()
+ close(chOut)
+ }()
+
+ var cps []ChangePreds
+ for cp := range chOut {
+ cps = append(cps, cp)
+ }
+
+ total := func(cp ChangePreds) int {
+ r := 0
+ for _, p := range cp.Predicates {
+ r += p.Score
+ }
+ return r
+ }
+
+ slices.SortFunc(cps, func(a, b ChangePreds) int {
+ if r := cmp.Compare(total(a), total(b)); r != 0 {
+ return -r // negate to sort in descending order
+ }
+ if r := a.Change.Updated().Compare(b.Change.Updated()); r != 0 {
+ return -r // negate to sort newest to oldest
+ }
+
+ // Sort change IDs numerically if we can,
+ // otherwise lexically.
+ aID := a.Change.ID()
+ bID := b.Change.ID()
+ anum, aerr := strconv.Atoi(aID)
+ bnum, berr := strconv.Atoi(bID)
+ if aerr == nil && berr == nil {
+ return cmp.Compare(anum, bnum)
+ } else {
+ return strings.Compare(aID, bID)
+ }
+ })
+
+ return cps
+}
diff --git a/internal/reviews/collect_test.go b/internal/reviews/collect_test.go
new file mode 100644
index 0000000..a3e65be
--- /dev/null
+++ b/internal/reviews/collect_test.go
@@ -0,0 +1,49 @@
+// Copyright 2024 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package reviews
+
+import (
+ "context"
+ "slices"
+ "testing"
+
+ "golang.org/x/oscar/internal/testutil"
+)
+
+func TestCollectChangePreds(t *testing.T) {
+ gc := testGerritClient(t)
+ testutil.Check(t, gc.Testing().LoadTxtar("testdata/gerritchange.txt"))
+
+ ctx := context.Background()
+
+ // Convert a iter.Seq[*GerritChange] into a iter.Seq[Change].
+ changes := func(yield func(Change) bool) {
+ for gc := range GerritChanges(ctx, gc, []string{"test"}, testAccounts()) {
+ if !yield(gc) {
+ return
+ }
+ }
+ }
+
+ cps := CollectChangePreds(ctx, testutil.Slogger(t), changes)
+ if len(cps) != 1 {
+ t.Errorf("CollectChangePreds returned %d entries, want 1", len(cps))
+ }
+ if len(cps) == 0 {
+ return
+ }
+ cp := cps[0]
+ if got := cp.Change.ID(); got != "1" {
+ t.Errorf("CollectChangePreds returned change %s, want 1", got)
+ }
+ var got []string
+ for _, s := range cp.Predicates {
+ got = append(got, s.Name)
+ }
+ want := []string{"authorReviewer"}
+ if !slices.Equal(got, want) {
+ t.Errorf("CollectChangePreds returned change with predicates %v, want %v", got, want)
+ }
+}
diff --git a/internal/reviews/gerrit.go b/internal/reviews/gerrit.go
index d4c57e3..0256d08 100644
--- a/internal/reviews/gerrit.go
+++ b/internal/reviews/gerrit.go
@@ -5,6 +5,8 @@
package reviews
import (
+ "context"
+ "iter"
"slices"
"strconv"
"time"
@@ -156,3 +158,26 @@
return NeedsReview
}
}
+
+// GerritChanges returns an iterator over the [GerritChange]
+// values for the specific projects.
+func GerritChanges(ctx context.Context, cl *gerrit.Client, projects []string, accounts AccountLookup) iter.Seq[*GerritChange] {
+ return func(yield func(*GerritChange) bool) {
+ for _, project := range projects {
+ grc := &GerritReviewClient{
+ GClient: cl,
+ Project: project,
+ Accounts: accounts,
+ }
+ for _, changeFn := range cl.ChangeNumbers(project) {
+ gc := &GerritChange{
+ Client: grc,
+ Change: changeFn(),
+ }
+ if !yield(gc) {
+ return
+ }
+ }
+ }
+ }
+}
diff --git a/internal/reviews/gerrit_test.go b/internal/reviews/gerrit_test.go
index 7f50faa..58b8dad 100644
--- a/internal/reviews/gerrit_test.go
+++ b/internal/reviews/gerrit_test.go
@@ -5,8 +5,10 @@
package reviews
import (
+ "context"
"reflect"
"slices"
+ "sync"
"testing"
"time"
@@ -30,28 +32,9 @@
testutil.Check(t, tc.LoadTxtar(filename))
gch := gc.Change("test", num)
grc := &GerritReviewClient{
- GClient: gc,
- Project: "test",
- Accounts: gerritTestAccountLookup{
- "gopher@golang.org": &gerritTestAccount{
- name: "gopher@golang.org",
- displayName: "gopher",
- authority: AuthorityReviewer,
- commits: 10,
- },
- "maintainer@golang.org": &gerritTestAccount{
- name: "maintainer@golang.org",
- displayName: "maintainer",
- authority: AuthorityMaintainer,
- commits: 10,
- },
- "commenter@golang.org": &gerritTestAccount{
- name: "commenter@golang.org",
- displayName: "commenter",
- authority: AuthorityContributor,
- commits: 10,
- },
- },
+ GClient: gc,
+ Project: "test",
+ Accounts: testAccounts(),
}
change := &GerritChange{
Client: grc,
@@ -148,6 +131,19 @@
t.Errorf("%s got %v, want %v", test.name, got, test.want)
}
}
+
+ ctx := context.Background()
+ it := GerritChanges(ctx, gc, []string{"test"}, testAccounts())
+ got := slices.Collect(it)
+ var gotIDs []string
+ for _, g := range got {
+ gotIDs = append(gotIDs, g.ID())
+ }
+ slices.Sort(gotIDs)
+ wantIDs := []string{"1", "2"}
+ if !slices.Equal(gotIDs, wantIDs) {
+ t.Errorf("GerritChanges returned IDs %v, want %v", gotIDs, wantIDs)
+ }
}
// changeMethod is one of the methods used to retrieve Change values.
@@ -194,3 +190,27 @@
func (gtal gerritTestAccountLookup) Lookup(name string) Account {
return gtal[name]
}
+
+// testAccounts returns an implementation of [AccountLookup] for testing.
+var testAccounts = sync.OnceValue(func() AccountLookup {
+ return gerritTestAccountLookup{
+ "gopher@golang.org": &gerritTestAccount{
+ name: "gopher@golang.org",
+ displayName: "gopher",
+ authority: AuthorityReviewer,
+ commits: 10,
+ },
+ "maintainer@golang.org": &gerritTestAccount{
+ name: "maintainer@golang.org",
+ displayName: "maintainer",
+ authority: AuthorityMaintainer,
+ commits: 10,
+ },
+ "commenter@golang.org": &gerritTestAccount{
+ name: "commenter@golang.org",
+ displayName: "commenter",
+ authority: AuthorityContributor,
+ commits: 10,
+ },
+ }
+})
