diff --git a/graphql/mutation_resolver.go b/graphql/mutation_resolver.go index 7fe677194b..e775d58c02 100644 --- a/graphql/mutation_resolver.go +++ b/graphql/mutation_resolver.go @@ -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 @@ -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 @@ -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 diff --git a/model/task/db.go b/model/task/db.go index 09d8148d90..88dec2b1c8 100644 --- a/model/task/db.go +++ b/model/task/db.go @@ -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) } diff --git a/model/task/task_annotations.go b/model/task/task_annotations.go index 5262641815..adc79e7113 100644 --- a/model/task/task_annotations.go +++ b/model/task/task_annotations.go @@ -1,6 +1,7 @@ package task import ( + "context" "time" "github.com/evergreen-ci/evergreen/db" @@ -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) @@ -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) @@ -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(), @@ -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, @@ -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(), @@ -155,7 +156,7 @@ 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 @@ -163,7 +164,7 @@ func UpsertAnnotation(a *annotations.TaskAnnotation, userDisplayName string) err // 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) @@ -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) } } @@ -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 diff --git a/model/task/task_annotations_test.go b/model/task/task_annotations_test.go index cc031265dd..da36c1846a 100644 --- a/model/task/task_annotations_test.go +++ b/model/task/task_annotations_test.go @@ -1,9 +1,11 @@ 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" @@ -11,11 +13,14 @@ import ( ) 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) @@ -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) @@ -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) @@ -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) @@ -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"}} @@ -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) @@ -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) @@ -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"}} @@ -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) @@ -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) @@ -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) @@ -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) @@ -195,17 +216,17 @@ 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) @@ -213,7 +234,7 @@ func TestPatchIssue(t *testing.T) { 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) diff --git a/rest/route/annotations.go b/rest/route/annotations.go index e06c34cccd..f087a8b64a 100644 --- a/rest/route/annotations.go +++ b/rest/route/annotations.go @@ -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")) } @@ -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) }