From ef6241f9b1f33a32fd5288b954797c4ef9918ee6 Mon Sep 17 00:00:00 2001 From: Christopher Poile Date: Tue, 11 Nov 2025 13:21:03 -0500 Subject: [PATCH] [MM-63561] - Job server: Fix read after write (#32527) * fix read after write * clean up tests --- server/channels/jobs/jobs.go | 7 +------ server/channels/jobs/jobs_test.go | 15 +-------------- 2 files changed, 2 insertions(+), 20 deletions(-) diff --git a/server/channels/jobs/jobs.go b/server/channels/jobs/jobs.go index 0a42a2497d6..0a24411826d 100644 --- a/server/channels/jobs/jobs.go +++ b/server/channels/jobs/jobs.go @@ -253,12 +253,7 @@ func (srv *JobServer) RequestCancellation(rctx request.CTX, jobId string) *model } if newJob != nil { if srv.metrics != nil { - job, err := srv.GetJob(rctx, jobId) - if err != nil { - return model.NewAppError("RequestCancellation", "app.job.update.app_error", nil, "", http.StatusInternalServerError).Wrap(err) - } - - srv.metrics.DecrementJobActive(job.Type) + srv.metrics.DecrementJobActive(newJob.Type) } return nil diff --git a/server/channels/jobs/jobs_test.go b/server/channels/jobs/jobs_test.go index 7ebe5677ba6..af71ba27fc3 100644 --- a/server/channels/jobs/jobs_test.go +++ b/server/channels/jobs/jobs_test.go @@ -10,13 +10,11 @@ import ( "os" "testing" - "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "github.com/mattermost/mattermost/server/public/model" "github.com/mattermost/mattermost/server/public/shared/mlog" "github.com/mattermost/mattermost/server/public/shared/request" - "github.com/mattermost/mattermost/server/v8/channels/store" "github.com/mattermost/mattermost/server/v8/channels/store/storetest" "github.com/mattermost/mattermost/server/v8/channels/utils/testutils" "github.com/mattermost/mattermost/server/v8/einterfaces/mocks" @@ -641,17 +639,9 @@ func TestRequestCancellation(t *testing.T) { t.Run("cancelled, job not found", func(t *testing.T) { jobServer, mockStore, _ := makeJobServer(t) - job := &model.Job{ - Id: "job_id", - Type: "job_type", - } - mockStore.JobStore. On("UpdateStatusOptimistically", "job_id", model.JobStatusPending, model.JobStatusCanceled). - Return(job, nil) - mockStore.JobStore. - On("Get", mock.AnythingOfType("*request.Context"), "job_id"). - Return(nil, &store.ErrNotFound{}) + Return(nil, errors.New("failed to update Job with id=job_id")) err := jobServer.RequestCancellation(ctx, "job_id") expectErrorId(t, "app.job.update.app_error", err) @@ -668,9 +658,6 @@ func TestRequestCancellation(t *testing.T) { mockStore.JobStore. On("UpdateStatusOptimistically", "job_id", model.JobStatusPending, model.JobStatusCanceled). Return(job, nil) - mockStore.JobStore. - On("Get", mock.AnythingOfType("*request.Context"), "job_id"). - Return(job, nil) mockMetrics.On("DecrementJobActive", "job_type") err := jobServer.RequestCancellation(ctx, "job_id")