Skip to content

Commit

Permalink
Add annotations updates
Browse files Browse the repository at this point in the history
  • Loading branch information
sophstad committed Dec 18, 2024
1 parent 302b875 commit 06c0893
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 40 deletions.
8 changes: 4 additions & 4 deletions graphql/mutation_resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func (r *mutationResolver) AddAnnotationIssue(ctx context.Context, taskID string
return false, InputValidationError.Send(ctx, fmt.Sprintf("issue does not have valid URL: %s", err.Error()))
}
if isIssue {
if err := task.AddIssueToAnnotation(taskID, execution, *issue, usr.Username()); err != nil {
if err := task.AddIssueToAnnotation(ctx, taskID, execution, *issue, usr.Username()); err != nil {
return false, InternalServerError.Send(ctx, fmt.Sprintf("couldn't add issue: %s", err.Error()))
}
return true, nil
Expand Down Expand Up @@ -97,12 +97,12 @@ func (r *mutationResolver) MoveAnnotationIssue(ctx context.Context, taskID strin
usr := mustHaveUser(ctx)
issue := restModel.APIIssueLinkToService(apiIssue)
if isIssue {
if err := task.MoveIssueToSuspectedIssue(taskID, execution, *issue, usr.Username()); err != nil {
if err := task.MoveIssueToSuspectedIssue(ctx, taskID, execution, *issue, usr.Username()); err != nil {
return false, InternalServerError.Send(ctx, fmt.Sprintf("couldn't move issue to suspected issues: %s", err.Error()))
}
return true, nil
} else {
if err := task.MoveSuspectedIssueToIssue(taskID, execution, *issue, usr.Username()); err != nil {
if err := task.MoveSuspectedIssueToIssue(ctx, taskID, execution, *issue, usr.Username()); err != nil {
return false, InternalServerError.Send(ctx, fmt.Sprintf("couldn't move issue to suspected issues: %s", err.Error()))
}
return true, nil
Expand All @@ -117,7 +117,7 @@ func (r *mutationResolver) RemoveAnnotationIssue(ctx context.Context, taskID str
}
issue := restModel.APIIssueLinkToService(apiIssue)
if isIssue {
if err := task.RemoveIssueFromAnnotation(taskID, execution, *issue); err != nil {
if err := task.RemoveIssueFromAnnotation(ctx, taskID, execution, *issue); err != nil {
return false, InternalServerError.Send(ctx, fmt.Sprintf("couldn't delete issue: %s", err.Error()))
}
return true, nil
Expand Down
14 changes: 9 additions & 5 deletions model/task/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -2750,12 +2750,16 @@ func (t *Task) IncNumNextTaskDispatches() error {

// UpdateHasAnnotations updates a task's HasAnnotations flag, indicating if there
// are any annotations with populated IssuesKey for its id / execution pair.
func UpdateHasAnnotations(taskId string, execution int, hasAnnotations bool) error {
err := UpdateOne(
func UpdateHasAnnotations(ctx context.Context, taskId string, execution int, hasAnnotations bool) error {
err := UpdateOneContext(
ctx,
ByIdAndExecution(taskId, execution),
bson.M{"$set": bson.M{
HasAnnotationsKey: hasAnnotations,
}})
[]bson.M{
bson.M{"$set": bson.M{
HasAnnotationsKey: hasAnnotations,
}},
addDisplayStatusCache,
})
return errors.Wrapf(err, "updating HasAnnotations field for task '%s'", taskId)
}

Expand Down
27 changes: 14 additions & 13 deletions model/task/task_annotations.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package task

import (
"context"
"time"

"github.com/evergreen-ci/evergreen/db"
Expand All @@ -14,7 +15,7 @@ import (
// MoveIssueToSuspectedIssue removes an issue from an existing annotation and adds it to its suspected issues,
// and unsets its associated task document as having annotations if this was the last issue removed from the
// annotation.
func MoveIssueToSuspectedIssue(taskId string, taskExecution int, issue annotations.IssueLink, username string) error {
func MoveIssueToSuspectedIssue(ctx context.Context, taskId string, taskExecution int, issue annotations.IssueLink, username string) error {
newIssue := issue
newIssue.Source = &annotations.Source{Requester: annotations.UIRequester, Author: username, Time: time.Now()}
q := annotations.ByTaskIdAndExecution(taskId, taskExecution)
Expand All @@ -37,14 +38,14 @@ func MoveIssueToSuspectedIssue(taskId string, taskExecution int, issue annotatio
return errors.Wrapf(err, "finding and modifying task annotation for execution %d of task '%s'", taskExecution, taskId)
}
if len(annotation.Issues) == 0 {
return UpdateHasAnnotations(taskId, taskExecution, false)
return UpdateHasAnnotations(ctx, taskId, taskExecution, false)
}
return nil
}

// MoveSuspectedIssueToIssue removes a suspected issue from an existing annotation and adds it to its issues,
// and marks its associated task document as having annotations.
func MoveSuspectedIssueToIssue(taskId string, taskExecution int, issue annotations.IssueLink, username string) error {
func MoveSuspectedIssueToIssue(ctx context.Context, taskId string, taskExecution int, issue annotations.IssueLink, username string) error {
newIssue := issue
newIssue.Source = &annotations.Source{Requester: annotations.UIRequester, Author: username, Time: time.Now()}
q := annotations.ByTaskIdAndExecution(taskId, taskExecution)
Expand All @@ -59,12 +60,12 @@ func MoveSuspectedIssueToIssue(taskId string, taskExecution int, issue annotatio
); err != nil {
return err
}
return UpdateHasAnnotations(taskId, taskExecution, true)
return UpdateHasAnnotations(ctx, taskId, taskExecution, true)
}

// AddIssueToAnnotation adds an issue onto an existing annotation and marks its associated task document
// as having annotations.
func AddIssueToAnnotation(taskId string, execution int, issue annotations.IssueLink, username string) error {
func AddIssueToAnnotation(ctx context.Context, taskId string, execution int, issue annotations.IssueLink, username string) error {
issue.Source = &annotations.Source{
Author: username,
Time: time.Now(),
Expand All @@ -79,12 +80,12 @@ func AddIssueToAnnotation(taskId string, execution int, issue annotations.IssueL
); err != nil {
return errors.Wrapf(err, "adding task annotation issue for task '%s'", taskId)
}
return UpdateHasAnnotations(taskId, execution, true)
return UpdateHasAnnotations(ctx, taskId, execution, true)
}

// RemoveIssueFromAnnotation removes an issue from an existing annotation, and unsets its
// associated task document as having annotations if this was the last issue removed from the annotation.
func RemoveIssueFromAnnotation(taskId string, execution int, issue annotations.IssueLink) error {
func RemoveIssueFromAnnotation(ctx context.Context, taskId string, execution int, issue annotations.IssueLink) error {
annotation := &annotations.TaskAnnotation{}
_, err := db.FindAndModify(
annotations.Collection,
Expand All @@ -100,14 +101,14 @@ func RemoveIssueFromAnnotation(taskId string, execution int, issue annotations.I
return errors.Wrapf(err, "finding and removing issue for task annotation for execution %d of task '%s'", execution, taskId)
}
if len(annotation.Issues) == 0 {
return UpdateHasAnnotations(taskId, execution, false)
return UpdateHasAnnotations(ctx, taskId, execution, false)
}
return nil
}

// UpsertAnnotation upserts a task annotation, and marks its associated task document
// as having annotations if the upsert includes a non-nil Issues field.
func UpsertAnnotation(a *annotations.TaskAnnotation, userDisplayName string) error {
func UpsertAnnotation(ctx context.Context, a *annotations.TaskAnnotation, userDisplayName string) error {
source := &annotations.Source{
Author: userDisplayName,
Time: time.Now(),
Expand Down Expand Up @@ -155,15 +156,15 @@ func UpsertAnnotation(a *annotations.TaskAnnotation, userDisplayName string) err

if a.Issues != nil {
hasAnnotations := len(a.Issues) > 0
return UpdateHasAnnotations(a.TaskId, a.TaskExecution, hasAnnotations)
return UpdateHasAnnotations(ctx, a.TaskId, a.TaskExecution, hasAnnotations)
}

return nil
}

// PatchAnnotation adds issues onto existing annotations, and marks its associated task document
// as having annotations if the patch includes a non-nil Issues field.
func PatchAnnotation(a *annotations.TaskAnnotation, userDisplayName string, upsert bool) error {
func PatchAnnotation(ctx context.Context, a *annotations.TaskAnnotation, userDisplayName string, upsert bool) error {
existingAnnotation, err := annotations.FindOneByTaskIdAndExecution(a.TaskId, a.TaskExecution)
if err != nil {
return errors.Wrapf(err, "finding annotation for task '%s' and execution %d", a.TaskId, a.TaskExecution)
Expand All @@ -172,7 +173,7 @@ func PatchAnnotation(a *annotations.TaskAnnotation, userDisplayName string, upse
if !upsert {
return errors.Errorf("annotation for task '%s' and execution %d not found", a.TaskId, a.TaskExecution)
} else {
return UpsertAnnotation(a, userDisplayName)
return UpsertAnnotation(ctx, a, userDisplayName)
}
}

Expand Down Expand Up @@ -201,7 +202,7 @@ func PatchAnnotation(a *annotations.TaskAnnotation, userDisplayName string, upse

if a.Issues != nil {
hasAnnotations := len(a.Issues) > 0
return UpdateHasAnnotations(a.TaskId, a.TaskExecution, hasAnnotations)
return UpdateHasAnnotations(ctx, a.TaskId, a.TaskExecution, hasAnnotations)
}

return nil
Expand Down
53 changes: 37 additions & 16 deletions model/task/task_annotations_test.go
Original file line number Diff line number Diff line change
@@ -1,21 +1,26 @@
package task

import (
"context"
"testing"

"github.com/evergreen-ci/birch"
"github.com/evergreen-ci/evergreen"
"github.com/evergreen-ci/evergreen/db"
"github.com/evergreen-ci/evergreen/model/annotations"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestAddIssueToAnnotation(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

assert.NoError(t, db.ClearCollections(annotations.Collection, Collection))
task := Task{Id: "t1"}
assert.NoError(t, task.Insert())
issue := annotations.IssueLink{URL: "https://issuelink.com", IssueKey: "EVG-1234", ConfidenceScore: float64(91.23)}
assert.NoError(t, AddIssueToAnnotation("t1", 0, issue, "annie.black"))
assert.NoError(t, AddIssueToAnnotation(ctx, "t1", 0, issue, "annie.black"))

annotation, err := annotations.FindOneByTaskIdAndExecution("t1", 0)
assert.NoError(t, err)
Expand All @@ -27,7 +32,7 @@ func TestAddIssueToAnnotation(t *testing.T) {
assert.Equal(t, "annie.black", annotation.Issues[0].Source.Author)
assert.Equal(t, float64(91.23), annotation.Issues[0].ConfidenceScore)

assert.NoError(t, AddIssueToAnnotation("t1", 0, issue, "not.annie.black"))
assert.NoError(t, AddIssueToAnnotation(ctx, "t1", 0, issue, "not.annie.black"))
annotation, err = annotations.FindOneByTaskIdAndExecution("t1", 0)
assert.NoError(t, err)
assert.NotNil(t, annotation)
Expand All @@ -39,19 +44,23 @@ func TestAddIssueToAnnotation(t *testing.T) {
require.NoError(t, err)
require.NotNil(t, dbTask)
assert.True(t, dbTask.HasAnnotations)
assert.Equal(t, dbTask.DisplayStatusCache, evergreen.TaskKnownIssue)
}

func TestRemoveIssueFromAnnotation(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

issue1 := annotations.IssueLink{URL: "https://issuelink.com", IssueKey: "EVG-1234", Source: &annotations.Source{Author: "annie.black"}}
issue2 := annotations.IssueLink{URL: "https://issuelink.com", IssueKey: "EVG-1234", Source: &annotations.Source{Author: "not.annie.black"}}
assert.NoError(t, db.ClearCollections(annotations.Collection, Collection))
a := annotations.TaskAnnotation{TaskId: "t1", Issues: []annotations.IssueLink{issue1, issue2}}
assert.NoError(t, a.Upsert())
task := Task{Id: "t1", HasAnnotations: true}
task := Task{Id: "t1", HasAnnotations: true, Status: evergreen.TaskFailed, DisplayStatusCache: evergreen.TaskKnownIssue}
assert.NoError(t, task.Insert())

// Task should still have annotations key set after first issue is removed
assert.NoError(t, RemoveIssueFromAnnotation("t1", 0, issue1))
assert.NoError(t, RemoveIssueFromAnnotation(ctx, "t1", 0, issue1))
annotationFromDB, err := annotations.FindOneByTaskIdAndExecution("t1", 0)
assert.NoError(t, err)
assert.NotNil(t, annotationFromDB)
Expand All @@ -61,9 +70,10 @@ func TestRemoveIssueFromAnnotation(t *testing.T) {
require.NoError(t, err)
require.NotNil(t, dbTask)
assert.True(t, dbTask.HasAnnotations)
assert.Equal(t, evergreen.TaskKnownIssue, dbTask.DisplayStatusCache)

// Removing the second issue should mark the task as no longer having annotations
assert.NoError(t, RemoveIssueFromAnnotation("t1", 0, issue2))
assert.NoError(t, RemoveIssueFromAnnotation(ctx, "t1", 0, issue2))
annotationFromDB, err = annotations.FindOneByTaskIdAndExecution("t1", 0)
assert.NoError(t, err)
assert.NotNil(t, annotationFromDB)
Expand All @@ -72,9 +82,13 @@ func TestRemoveIssueFromAnnotation(t *testing.T) {
require.NoError(t, err)
require.NotNil(t, dbTask)
assert.False(t, dbTask.HasAnnotations)
assert.Equal(t, evergreen.TaskFailed, dbTask.DisplayStatusCache)
}

func TestMoveIssueToSuspectedIssue(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

issue1 := annotations.IssueLink{URL: "https://issuelink.com", IssueKey: "EVG-1234", Source: &annotations.Source{Author: "this will be overridden"}}
issue2 := annotations.IssueLink{URL: "https://issuelink.com", IssueKey: "EVG-2345", Source: &annotations.Source{Author: "evergreen user"}}
issue3 := annotations.IssueLink{URL: "https://issuelink.com", IssueKey: "EVG-3456", Source: &annotations.Source{Author: "different user"}}
Expand All @@ -84,7 +98,7 @@ func TestMoveIssueToSuspectedIssue(t *testing.T) {
task := Task{Id: "t1", HasAnnotations: true}
assert.NoError(t, task.Insert())

assert.NoError(t, MoveIssueToSuspectedIssue(a.TaskId, a.TaskExecution, issue1, "someone new"))
assert.NoError(t, MoveIssueToSuspectedIssue(ctx, a.TaskId, a.TaskExecution, issue1, "someone new"))
annotationFromDB, err := annotations.FindOneByTaskIdAndExecution(a.TaskId, a.TaskExecution)
assert.NoError(t, err)
assert.NotNil(t, annotationFromDB)
Expand All @@ -100,7 +114,7 @@ func TestMoveIssueToSuspectedIssue(t *testing.T) {
assert.True(t, dbTask.HasAnnotations)

// Removing the second issue should mark the task as no longer having annotations
assert.NoError(t, MoveIssueToSuspectedIssue(a.TaskId, a.TaskExecution, issue2, "someone else new"))
assert.NoError(t, MoveIssueToSuspectedIssue(ctx, a.TaskId, a.TaskExecution, issue2, "someone else new"))
annotationFromDB, err = annotations.FindOneByTaskIdAndExecution(a.TaskId, a.TaskExecution)
assert.NoError(t, err)
assert.NotNil(t, annotationFromDB)
Expand All @@ -116,6 +130,9 @@ func TestMoveIssueToSuspectedIssue(t *testing.T) {
}

func TestMoveSuspectedIssueToIssue(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

issue1 := annotations.IssueLink{URL: "https://issuelink.com", IssueKey: "EVG-1234", Source: &annotations.Source{Author: "this will be overridden"}}
issue2 := annotations.IssueLink{URL: "https://issuelink.com", IssueKey: "EVG-2345", Source: &annotations.Source{Author: "evergreen user"}}
issue3 := annotations.IssueLink{URL: "https://issuelink.com", IssueKey: "EVG-3456", Source: &annotations.Source{Author: "different user"}}
Expand All @@ -126,7 +143,7 @@ func TestMoveSuspectedIssueToIssue(t *testing.T) {
a := annotations.TaskAnnotation{TaskId: "t1", SuspectedIssues: []annotations.IssueLink{issue1, issue2}, Issues: []annotations.IssueLink{issue3}}
assert.NoError(t, a.Upsert())

assert.NoError(t, MoveSuspectedIssueToIssue(a.TaskId, a.TaskExecution, issue1, "someone new"))
assert.NoError(t, MoveSuspectedIssueToIssue(ctx, a.TaskId, a.TaskExecution, issue1, "someone new"))
annotationFromDB, err := annotations.FindOneByTaskIdAndExecution("t1", 0)
assert.NoError(t, err)
assert.NotNil(t, annotationFromDB)
Expand All @@ -139,17 +156,21 @@ func TestMoveSuspectedIssueToIssue(t *testing.T) {
require.NoError(t, err)
require.NotNil(t, dbTask)
assert.True(t, dbTask.HasAnnotations)
assert.Equal(t, evergreen.TaskKnownIssue, dbTask.DisplayStatusCache)
}

func TestPatchIssue(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

assert.NoError(t, db.ClearCollections(annotations.Collection, Collection))
t1 := Task{Id: "t1"}
assert.NoError(t, t1.Insert())
issue1 := annotations.IssueLink{URL: "https://issuelink.com", IssueKey: "EVG-1234", ConfidenceScore: float64(91.23)}
assert.NoError(t, AddIssueToAnnotation("t1", 0, issue1, "bynn.lee"))
assert.NoError(t, AddIssueToAnnotation(ctx, "t1", 0, issue1, "bynn.lee"))
issue2 := annotations.IssueLink{URL: "https://issuelink.com", IssueKey: "EVG-2345"}
a := annotations.TaskAnnotation{TaskId: "t1", TaskExecution: 0, SuspectedIssues: []annotations.IssueLink{issue2}}
assert.NoError(t, PatchAnnotation(&a, "not bynn", true))
assert.NoError(t, PatchAnnotation(ctx, &a, "not bynn", true))

annotation, err := annotations.FindOneByTaskIdAndExecution(a.TaskId, a.TaskExecution)
assert.NoError(t, err)
Expand All @@ -169,7 +190,7 @@ func TestPatchIssue(t *testing.T) {

issue3 := annotations.IssueLink{URL: "https://issuelink.com", IssueKey: "EVG-3456"}
insert := annotations.TaskAnnotation{TaskId: "t1", TaskExecution: 1, SuspectedIssues: []annotations.IssueLink{issue3}}
assert.NoError(t, PatchAnnotation(&insert, "insert", true))
assert.NoError(t, PatchAnnotation(ctx, &insert, "insert", true))
annotation, err = annotations.FindOneByTaskIdAndExecution(insert.TaskId, insert.TaskExecution)
assert.NoError(t, err)
assert.NotNil(t, annotation)
Expand All @@ -181,7 +202,7 @@ func TestPatchIssue(t *testing.T) {
assert.Equal(t, "EVG-3456", annotation.SuspectedIssues[0].IssueKey)

upsert := annotations.TaskAnnotation{TaskId: "t1", TaskExecution: 2, Note: &annotations.Note{Message: "should work"}, SuspectedIssues: []annotations.IssueLink{issue3}}
assert.NoError(t, PatchAnnotation(&upsert, "upsert", true))
assert.NoError(t, PatchAnnotation(ctx, &upsert, "upsert", true))
annotation, err = annotations.FindOneByTaskIdAndExecution(upsert.TaskId, upsert.TaskExecution)
assert.NoError(t, err)
assert.NotNil(t, annotation)
Expand All @@ -195,25 +216,25 @@ func TestPatchIssue(t *testing.T) {
assert.Equal(t, "should work", annotation.Note.Message)

badInsert := annotations.TaskAnnotation{TaskId: "t1", TaskExecution: 1, Note: &annotations.Note{Message: "shouldn't work"}}
assert.Error(t, PatchAnnotation(&badInsert, "error out", true))
assert.Error(t, PatchAnnotation(ctx, &badInsert, "error out", true))

badInsert2 := annotations.TaskAnnotation{TaskId: "t1", TaskExecution: 1, Metadata: &birch.Document{}}
assert.Error(t, PatchAnnotation(&badInsert2, "error out", false))
assert.Error(t, PatchAnnotation(ctx, &badInsert2, "error out", false))

// Check that HasAnnotations field is correctly in sync when patching issues array.
t2 := Task{Id: "t2"}
assert.NoError(t, t2.Insert())

annotationUpdate := annotations.TaskAnnotation{TaskId: "t2", TaskExecution: 0, Issues: []annotations.IssueLink{issue3}}
assert.NoError(t, PatchAnnotation(&annotationUpdate, "jane.smith", true))
assert.NoError(t, PatchAnnotation(ctx, &annotationUpdate, "jane.smith", true))

foundTask, err := FindOneId(t2.Id)
require.NoError(t, err)
require.NotNil(t, foundTask)
assert.Equal(t, true, foundTask.HasAnnotations)

annotationUpdate = annotations.TaskAnnotation{TaskId: "t2", TaskExecution: 0, Issues: []annotations.IssueLink{}}
assert.NoError(t, PatchAnnotation(&annotationUpdate, "jane.smith", true))
assert.NoError(t, PatchAnnotation(ctx, &annotationUpdate, "jane.smith", true))

foundTask, err = FindOneId(t2.Id)
require.NoError(t, err)
Expand Down
4 changes: 2 additions & 2 deletions rest/route/annotations.go
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ func (h *annotationByTaskPutHandler) Parse(ctx context.Context, r *http.Request)
}

func (h *annotationByTaskPutHandler) Run(ctx context.Context) gimlet.Responder {
err := task.UpsertAnnotation(restModel.APITaskAnnotationToService(*h.annotation), h.user.DisplayName())
err := task.UpsertAnnotation(ctx, restModel.APITaskAnnotationToService(*h.annotation), h.user.DisplayName())
if err != nil {
return gimlet.NewJSONInternalErrorResponse(errors.Wrap(err, "updating annotation"))
}
Expand Down Expand Up @@ -398,7 +398,7 @@ func (h *annotationByTaskPatchHandler) Parse(ctx context.Context, r *http.Reques
}

func (h *annotationByTaskPatchHandler) Run(ctx context.Context) gimlet.Responder {
err := task.PatchAnnotation(restModel.APITaskAnnotationToService(*h.annotation), h.user.DisplayName(), h.upsert)
err := task.PatchAnnotation(ctx, restModel.APITaskAnnotationToService(*h.annotation), h.user.DisplayName(), h.upsert)
if err != nil {
gimlet.NewJSONInternalErrorResponse(err)
}
Expand Down

0 comments on commit 06c0893

Please sign in to comment.