Skip to content

Commit

Permalink
internal/gaby: allow multiple github projects
Browse files Browse the repository at this point in the history
Go project has repositories that run separate issue trackers.
(e.g. vscode-go, vulndb, ...) Gaby should be able to support
cross-repo search and correlation.

For #51

Change-Id: I103812460414b9f1628442608c55337c93a18703
Reviewed-on: https://go-review.googlesource.com/c/oscar/+/628016
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Zvonimir Pavlinovic <[email protected]>
  • Loading branch information
hyangah committed Nov 15, 2024
1 parent 0937c9d commit 1397365
Show file tree
Hide file tree
Showing 8 changed files with 996 additions and 29 deletions.
7 changes: 4 additions & 3 deletions internal/gaby/github_event.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"errors"
"fmt"
"net/http"
"slices"

"golang.org/x/oscar/internal/actions"
"golang.org/x/oscar/internal/docs"
Expand All @@ -34,15 +35,15 @@ import (
// or [gabyFlags.enablechanges] is false.)
//
// handleGitHubEvent returns an error if any of the syncs or actions fails,
// of if the webhook request is invalid according to [github.ValidateWebhookRequest].
// or if the webhook request is invalid according to [github.ValidateWebhookRequest].
func (g *Gaby) handleGitHubEvent(r *http.Request, fl *gabyFlags) (handled bool, err error) {
event, err := github.ValidateWebhookRequest(r, g.secret)
if err != nil {
return false, fmt.Errorf("%w: %v", errInvalidWebhookRequest, err)
}

if event.Project() != g.githubProject {
g.slog.Warn("unexpected webhook request", "webhook_project", event.Project(), "gaby_project", g.githubProject, "event", event)
if !slices.Contains(g.githubProjects, event.Project()) {
g.slog.Warn("unexpected webhook request", "webhook_project", event.Project(), "gaby_project", g.githubProjects, "event", event)
return false, nil
}

Expand Down
43 changes: 32 additions & 11 deletions internal/gaby/github_event_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ import (
"golang.org/x/oscar/internal/testutil"
)

const testProject = "rsc/tmp"
const (
testProject = "rsc/tmp"
testProject2 = "rsc/markdown"
)

func TestHandleGitHubEvent(t *testing.T) {
fl := &gabyFlags{
Expand Down Expand Up @@ -81,6 +84,21 @@ func TestHandleGitHubEvent(t *testing.T) {
payloadType: github.WebhookEventTypeIssueComment,
wantHandled: true,
},
{
// Second project is handled too.
name: "new issue comment2",
payload: &github.WebhookIssueCommentEvent{
Action: github.WebhookIssueCommentActionCreated,
Issue: github.Issue{
Number: 3,
},
Repository: github.Repository{
Project: testProject2,
},
},
payloadType: github.WebhookEventTypeIssueComment,
wantHandled: true,
},
{
// Incorrect project skips the event but doesn't return an error.
name: "wrong project",
Expand Down Expand Up @@ -125,24 +143,26 @@ func testGaby(t *testing.T, secret secret.DB) *Gaby {

rp := related.New(lg, db, gh, vdb, dc, "related")
rp.EnableProject(testProject)
rp.EnableProject(testProject2)
rp.EnablePosts()

cf := commentfix.New(lg, gh, db, "fix")
cf.EnableProject(testProject)
cf.EnableProject(testProject2)
// No fixes yet.
cf.EnableEdits()

return &Gaby{
githubProject: testProject,
github: gh,
vector: vdb,
secret: secret,
db: db,
slog: lg,
embed: emb,
docs: dc,
commentFixer: cf,
relatedPoster: rp,
githubProjects: []string{testProject, testProject2},
github: gh,
vector: vdb,
secret: secret,
db: db,
slog: lg,
embed: emb,
docs: dc,
commentFixer: cf,
relatedPoster: rp,
}
}

Expand All @@ -166,6 +186,7 @@ func testGHClient(t *testing.T, check func(error), lg *slog.Logger, db storage.D
}
c := github.New(lg, db, sdb, rr.Client())
check(c.Add(testProject))
check(c.Add(testProject2))

return c
}
19 changes: 13 additions & 6 deletions internal/gaby/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ type Gaby struct {
cloud bool // running on Cloud Run
meta map[string]string // any metadata we want to expose
addr string // address to serve HTTP on
githubProject string // github project to monitor and update
githubProjects []string // github projects to monitor and update
gerritProjects []string // gerrit projects to monitor and update
googleGroups []string // google groups to monitor and update

Expand Down Expand Up @@ -113,17 +113,19 @@ func main() {
slogLevel: level,
http: http.DefaultClient,
addr: "localhost:4229", // 4229 = gaby on a phone
githubProject: "golang/go",
githubProjects: []string{"golang/go"},
gerritProjects: []string{"go"},
googleGroups: []string{"golang-nuts"},
}

shutdown := g.initGCP()
shutdown := g.initGCP() // sets up g.db, g.vector, g.secret, ...
defer shutdown()

g.github = github.New(g.slog, g.db, g.secret, g.http)
g.disc = discussion.New(g.ctx, g.slog, g.secret, g.db)
_ = g.disc.Add(g.githubProject) // only needed once per g.db lifetime
for _, project := range g.githubProjects {
_ = g.disc.Add(project) // FIXME: ignore the error. g.disc.Add returns an error if the project already exists in the db.
}

g.gerrit = gerrit.New("go-review.googlesource.com", g.slog, g.db, g.secret, g.http)
for _, project := range g.gerritProjects {
Expand Down Expand Up @@ -157,14 +159,19 @@ func main() {
}

cf := commentfix.New(g.slog, g.github, g.db, "gerritlinks")
cf.EnableProject(g.githubProject)
for _, proj := range g.githubProjects {
cf.EnableProject(proj)
}
cf.AutoLink(`\bCL ([0-9]+)\b`, "https://go.dev/cl/$1")
cf.ReplaceURL(`\Qhttps://go-review.git.corp.google.com/\E`, "https://go-review.googlesource.com/")
cf.EnableEdits()
g.commentFixer = cf

rp := related.New(g.slog, g.db, g.github, g.vector, g.docs, "related")
rp.EnableProject(g.githubProject)
for _, proj := range g.githubProjects {
rp.EnableProject(proj)
}
// TODO(hyangah): shouldn't these rules be configured differently for different github projects?
rp.SkipBodyContains("— [watchflakes](https://go.dev/wiki/Watchflakes)")
rp.SkipTitlePrefix("x/tools/gopls: release version v")
rp.SkipTitleSuffix(" backport]")
Expand Down
7 changes: 4 additions & 3 deletions internal/gaby/overview.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"context"
"fmt"
"net/http"
"slices"
"strconv"
"strings"
"time"
Expand Down Expand Up @@ -142,10 +143,10 @@ func (g *Gaby) populateOverviewPage(r *http.Request) overviewPage {
p.Error = fmt.Errorf("invalid form value: %v", err)
return p
}
if proj == "" {
proj = g.githubProject // default to golang/go
if proj == "" && len(g.githubProjects) > 0 {
proj = g.githubProjects[0] // default to first project.
}
if g.githubProject != proj {
if !slices.Contains(g.githubProjects, proj) {
p.Error = fmt.Errorf("invalid form value (unrecognized project): %q", p.Form.Query)
return p
}
Expand Down
20 changes: 18 additions & 2 deletions internal/gaby/overview_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func TestPopulateOverviewPage(t *testing.T) {

// Add test data relevant to this test.
project := "hello/world"
g.githubProject = project
g.githubProjects = []string{project}
g.github.Add(project)

iss1 := &github.Issue{
Expand Down Expand Up @@ -161,7 +161,7 @@ func TestPopulateOverviewPage(t *testing.T) {
},
},
{
name: "error",
name: "error/unknownIssue",
r: &http.Request{
Form: map[string][]string{
"q": {"3"}, // not in DB
Expand All @@ -176,6 +176,22 @@ func TestPopulateOverviewPage(t *testing.T) {
Error: cmpopts.AnyError,
},
},
{
name: "error/unknownProject",
r: &http.Request{
Form: map[string][]string{
"q": {"unknown/project#3"}, // not in DB
"t": {relatedOverviewType},
},
},
want: overviewPage{
Form: overviewForm{
Query: "unknown/project#3",
OverviewType: relatedOverviewType,
},
Error: cmpopts.AnyError,
},
},
} {
t.Run(tc.name, func(t *testing.T) {
got := g.populateOverviewPage(tc.r)
Expand Down
7 changes: 4 additions & 3 deletions internal/gaby/rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package main
import (
"fmt"
"net/http"
"slices"

"github.com/google/safehtml"
"github.com/google/safehtml/template"
Expand Down Expand Up @@ -57,10 +58,10 @@ func (g *Gaby) populateRulesPage(r *http.Request) rulesPage {
p.Error = fmt.Errorf("invalid form value %q: %w", form.Query, err).Error()
return p
}
if proj == "" {
proj = g.githubProject // default to golang/go
if proj == "" && len(g.githubProjects) > 0 {
proj = g.githubProjects[0] // default to first project
}
if g.githubProject != proj {
if !slices.Contains(g.githubProjects, proj) {
p.Error = fmt.Errorf("invalid form value (unrecognized project): %q", form.Query).Error()
return p
}
Expand Down

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion internal/github/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ func (c *Client) SyncProject(ctx context.Context, project string) (err error) {
// Load sync state.
var proj projectSync
if val, ok := c.db.Get(key); !ok {
return fmt.Errorf("missing project")
return fmt.Errorf("missing project %v", project)
} else if err := json.Unmarshal(val, &proj); err != nil {
return err
}
Expand Down

0 comments on commit 1397365

Please sign in to comment.