internal/labels: labeler: modify GitHub

Add the code that actually changes the labels on GitHub.

For #64.

Change-Id: Iecf061427dec626713e4f9f303c29cf67734cad1
Reviewed-on: https://go-review.googlesource.com/c/oscar/+/637855
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Tatiana Bradley <[email protected]>

internal/github/testing.go

@@ -79,6 +79,14 @@ func (e *TestingEdit) String() string {
 			return fmt.Sprintf("PostIssueComment(%s#%d, %s)", e.Project, e.Issue, js)
 		}
 		return fmt.Sprintf("EditIssueComment(%s#%d.%d, %s)", e.Project, e.Issue, e.Comment, js)
+
+	case e.LabelChanges != nil:
+		js, _ := json.Marshal(e.LabelChanges)
+		return fmt.Sprintf("EditLabel(%s#%d, %s)", e.Project, e.Issue, js)
+
+	case e.Label.Name != "":
+		js, _ := json.Marshal(e.Label)
+		return fmt.Sprintf("CreateLabel(%s#%d, %s)", e.Project, e.Issue, js)
 	}
 	return "?"
 }

internal/labels/labeler.go

@@ -6,9 +6,13 @@ package labels
 
 import (
 	"context"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"log/slog"
+	"maps"
+	"slices"
+	"strings"
 	"time"
 
 	"golang.org/x/oscar/internal/actions"
@@ -134,13 +138,18 @@ func (l *Labeler) Run(ctx context.Context) error {
 			l.slog.Error("labels.Labeler", "issue", e.Issue, "event", e, "error", err)
 			continue
 		}
+		eg := slog.Group("event",
+			"dbtime", e.DBTime,
+			"project", e.Project,
+			"issue", e.Issue,
+			"json", string(e.JSON))
 		if advance {
 			l.watcher.MarkOld(e.DBTime)
 			// Flush immediately to make sure we don't re-post if interrupted later in the loop.
 			l.watcher.Flush()
-			l.slog.Info("labels.Labeler advanced watcher", "latest", l.watcher.Latest(), "event", e)
+			l.slog.Info("labels.Labeler advanced watcher", "latest", l.watcher.Latest(), eg)
 		} else {
-			l.slog.Info("labels.Labeler watcher not advanced", "latest", l.watcher.Latest(), "event", e)
+			l.slog.Info("labels.Labeler watcher not advanced", "latest", l.watcher.Latest(), eg)
 		}
 	}
 	return nil
@@ -266,13 +275,69 @@ type actioner struct {
 }
 
 func (ar *actioner) Run(ctx context.Context, data []byte) ([]byte, error) {
-	// TODO: implement
-	return nil, errors.New("unimplemented")
+	return ar.l.runFromActionLog(ctx, data)
 }
 
 func (ar *actioner) ForDisplay(data []byte) string {
-	// TODO: implement
-	return ""
+	var a action
+	if err := json.Unmarshal(data, &a); err != nil {
+		return fmt.Sprintf("ERROR: %v", err)
+	}
+	return a.Issue.HTMLURL + "\n" + strings.Join(a.NewLabels, ", ")
+}
+
+// runFromActionLog is called by actions.Run to execute an action.
+// It decodes the action, calls [Labeler.runAction], then encodes the result.
+func (l *Labeler) runFromActionLog(ctx context.Context, data []byte) ([]byte, error) {
+	var a action
+	if err := json.Unmarshal(data, &a); err != nil {
+		return nil, err
+	}
+	res, err := l.runAction(ctx, &a)
+	if err != nil {
+		return nil, err
+	}
+	return storage.JSON(res), nil
+}
+
+// runAction runs the given action.
+func (l *Labeler) runAction(ctx context.Context, a *action) (*result, error) {
+	// When updating an issue in GitHub, we must provide all the labels, both the
+	// existing and the new.
+	//
+	// There is an HTTP mechanism for atomic test-and-set, using the If-Match header
+	// with an ETag. Unfortunately, GitHub does not support it: it returns a 412
+	// Precondition Failed if it sees that header, then makes the change regardless
+	// of whether the ETags match. So the best we can do is read the existing labels
+	// and immediately write the new ones.
+	issue, err := l.github.DownloadIssue(ctx, a.Issue.URL)
+	if err != nil {
+		return nil, fmt.Errorf("Labeler, download %s: %w", a.Issue.URL, err)
+	}
+
+	// Compute the union of the old and new label names.
+	oldLabels := map[string]bool{}
+	for _, lab := range issue.Labels {
+		oldLabels[lab.Name] = true
+	}
+	newLabels := maps.Clone(oldLabels)
+	for _, name := range a.NewLabels {
+		newLabels[name] = true
+	}
+	if maps.Equal(oldLabels, newLabels) {
+		// Nothing to do.
+		return &result{issue.URL}, nil
+	}
+	labelNames := slices.Collect(maps.Keys(newLabels))
+	// Sort for determinism in tests.
+	slices.Sort(labelNames)
+
+	err = l.github.EditIssue(ctx, a.Issue, &github.IssueChanges{Labels: &labelNames})
+	// If GitHub returns an error, add it to the action log for this action.
+	if err != nil {
+		return nil, fmt.Errorf("Labeler: edit %s: %w", a.Issue.URL, err)
+	}
+	return &result{URL: issue.URL}, nil
 }
 
 // logKey returns the key for the event in the action log. This is only a portion

internal/labels/labeler_test.go

@@ -8,6 +8,7 @@ import (
 	"context"
 	"encoding/json"
 	"maps"
+	"reflect"
 	"slices"
 	"testing"
 
@@ -72,11 +73,13 @@ func TestRun(t *testing.T) {
 	lg := testutil.Slogger(t)
 	db := storage.MemDB()
 	gh := github.New(lg, db, nil, nil)
-	gh.Testing().AddLabel(project, github.Label{Name: "bug"})
+	gh.Testing().AddLabel(project, github.Label{Name: "Bug"})
+	gh.Testing().AddLabel(project, github.Label{Name: "Other"})
 	gh.Testing().AddIssue(project, &github.Issue{
 		Number: 1,
 		Title:  "title",
 		Body:   "body",
+		Labels: []github.Label{{Name: "Other"}},
 	})
 	gh.Testing().AddIssue("other/project", &github.Issue{
 		Number: 2,
@@ -98,7 +101,7 @@ func TestRun(t *testing.T) {
 	var got action
 	check(json.Unmarshal(entries[0].Action, &got))
 	if got.Issue.Number != 1 || !slices.Equal(got.NewLabels, []string{"Bug"}) {
-		t.Errorf("got (%d, %v), want (1, [Bug])", got.Issue.Number, got.NewLabels)
+		t.Errorf("got (%d, %v), want (1, [bug])", got.Issue.Number, got.NewLabels)
 	}
 
 	// Second time, nothing should happen.
@@ -107,4 +110,33 @@ func TestRun(t *testing.T) {
 	if g := len(entries); g != 0 {
 		t.Fatalf("got %d actions, want 0", g)
 	}
+
+	// Run the action, look for changes.
+	check(actions.Run(ctx, lg, db))
+	gotEdits := gh.Testing().Edits()
+	wantEdits := []*github.TestingEdit{
+		{
+			Project:      "golang/go",
+			Issue:        1,
+			IssueChanges: &github.IssueChanges{Labels: &[]string{"Bug", "Other"}},
+		},
+	}
+	wi := 0
+	for _, ge := range gotEdits {
+		// Ignore the changes from syncLabels.
+		if ge.LabelChanges != nil || ge.Label.Name != "" {
+			continue
+		}
+		if wi >= len(wantEdits) {
+			t.Errorf("unexpected edit %s", ge)
+			continue
+		}
+		if we := wantEdits[wi]; !reflect.DeepEqual(ge, we) {
+			t.Errorf("\ngot  %s\nwant %s", ge, we)
+		}
+		wi++
+	}
+	if wi != len(wantEdits) {
+		t.Fatal("not enough edits")
+	}
 }