From 78766625df4a3328515b94c44c31a74d5d86cef1 Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Thu, 20 Aug 2020 19:37:19 +0530 Subject: [PATCH] jobstore (#15250) * Migration completed * Modify test case * Update en.json * Fix layers * Lint: remove unnecessary use of sprint * trigger CI * fix error * Fixes * fix test Co-authored-by: Rodrigo Villablanca Co-authored-by: Mattermod --- app/job.go | 31 ++- i18n/en.json | 52 +++-- jobs/jobs.go | 70 ++++-- migrations/migrations.go | 4 +- migrations/migrations_test.go | 12 +- store/opentracinglayer/opentracinglayer.go | 26 +-- store/retrylayer/retrylayer.go | 234 ++++++++++++++++++--- store/sqlstore/job_store.go | 99 ++++----- store/store.go | 26 +-- store/storetest/job_store.go | 15 +- store/storetest/mocks/JobStore.go | 130 +++++------- store/timerlayer/timerlayer.go | 26 +-- 12 files changed, 474 insertions(+), 251 deletions(-) diff --git a/app/job.go b/app/job.go index 3b73a05d91..c1388aff4f 100644 --- a/app/job.go +++ b/app/job.go @@ -4,11 +4,26 @@ package app import ( + "errors" + "net/http" + "github.com/mattermost/mattermost-server/v5/model" + "github.com/mattermost/mattermost-server/v5/store" ) func (a *App) GetJob(id string) (*model.Job, *model.AppError) { - return a.Srv().Store.Job().Get(id) + job, err := a.Srv().Store.Job().Get(id) + if err != nil { + var nfErr *store.ErrNotFound + switch { + case errors.As(err, &nfErr): + return nil, model.NewAppError("GetJob", "app.job.get.app_error", nil, nfErr.Error(), http.StatusNotFound) + default: + return nil, model.NewAppError("GetJob", "app.job.get.app_error", nil, err.Error(), http.StatusInternalServerError) + } + } + + return job, nil } func (a *App) GetJobsPage(page int, perPage int) ([]*model.Job, *model.AppError) { @@ -16,7 +31,12 @@ func (a *App) GetJobsPage(page int, perPage int) ([]*model.Job, *model.AppError) } func (a *App) GetJobs(offset int, limit int) ([]*model.Job, *model.AppError) { - return a.Srv().Store.Job().GetAllPage(offset, limit) + jobs, err := a.Srv().Store.Job().GetAllPage(offset, limit) + if err != nil { + return nil, model.NewAppError("GetJobs", "app.job.get_all.app_error", nil, err.Error(), http.StatusInternalServerError) + } + + return jobs, nil } func (a *App) GetJobsByTypePage(jobType string, page int, perPage int) ([]*model.Job, *model.AppError) { @@ -24,7 +44,12 @@ func (a *App) GetJobsByTypePage(jobType string, page int, perPage int) ([]*model } func (a *App) GetJobsByType(jobType string, offset int, limit int) ([]*model.Job, *model.AppError) { - return a.Srv().Store.Job().GetAllByTypePage(jobType, offset, limit) + jobs, err := a.Srv().Store.Job().GetAllByTypePage(jobType, offset, limit) + if err != nil { + return nil, model.NewAppError("GetJobsByType", "app.job.get_all.app_error", nil, err.Error(), http.StatusInternalServerError) + } + + return jobs, nil } func (a *App) CreateJob(job *model.Job) (*model.Job, *model.AppError) { diff --git a/i18n/en.json b/i18n/en.json index 90d3cbfb1c..99293ab9b8 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -4022,6 +4022,30 @@ "id": "app.job.download_export_results_not_enabled", "translation": "DownloadExportResults in config.json is false. Please set this to true to download the results of this job." }, + { + "id": "app.job.get.app_error", + "translation": "Unable to get the job." + }, + { + "id": "app.job.get_all.app_error", + "translation": "Unable to get the jobs." + }, + { + "id": "app.job.get_count_by_status_and_type.app_error", + "translation": "Unable to get the job count by status and type." + }, + { + "id": "app.job.get_newest_job_by_status_and_type.app_error", + "translation": "Unable to get the newest job by status and type." + }, + { + "id": "app.job.save.app_error", + "translation": "Unable to save the job." + }, + { + "id": "app.job.update.app_error", + "translation": "Unable to update the job." + }, { "id": "app.notification.body.intro.direct.full", "translation": "You have a new Direct Message." @@ -7254,34 +7278,6 @@ "id": "store.sql_group.uniqueness_error", "translation": "group member already exists" }, - { - "id": "store.sql_job.delete.app_error", - "translation": "Unable to delete the job." - }, - { - "id": "store.sql_job.get.app_error", - "translation": "Unable to get the job." - }, - { - "id": "store.sql_job.get_all.app_error", - "translation": "Unable to get the jobs." - }, - { - "id": "store.sql_job.get_count_by_status_and_type.app_error", - "translation": "Unable to get the job count by status and type." - }, - { - "id": "store.sql_job.get_newest_job_by_status_and_type.app_error", - "translation": "Unable to get the newest job by status and type." - }, - { - "id": "store.sql_job.save.app_error", - "translation": "Unable to save the job." - }, - { - "id": "store.sql_job.update.app_error", - "translation": "Unable to update the job." - }, { "id": "store.sql_post.analytics_posts_count.app_error", "translation": "Unable to get post counts." diff --git a/jobs/jobs.go b/jobs/jobs.go index 9cb07c7808..4d2df9d4f1 100644 --- a/jobs/jobs.go +++ b/jobs/jobs.go @@ -5,12 +5,13 @@ package jobs import ( "context" - "time" - + "errors" "net/http" + "time" "github.com/mattermost/mattermost-server/v5/mlog" "github.com/mattermost/mattermost-server/v5/model" + "github.com/mattermost/mattermost-server/v5/store" ) const ( @@ -31,18 +32,34 @@ func (srv *JobServer) CreateJob(jobType string, jobData map[string]string) (*mod } if _, err := srv.Store.Job().Save(&job); err != nil { - return nil, err + return nil, model.NewAppError("CreateJob", "app.job.save.app_error", nil, err.Error(), http.StatusInternalServerError) } return &job, nil } func (srv *JobServer) GetJob(id string) (*model.Job, *model.AppError) { - return srv.Store.Job().Get(id) + job, err := srv.Store.Job().Get(id) + if err != nil { + var nfErr *store.ErrNotFound + switch { + case errors.As(err, &nfErr): + return nil, model.NewAppError("GetJob", "app.job.get.app_error", nil, nfErr.Error(), http.StatusNotFound) + default: + return nil, model.NewAppError("GetJob", "app.job.get.app_error", nil, err.Error(), http.StatusInternalServerError) + } + } + + return job, nil } func (srv *JobServer) ClaimJob(job *model.Job) (bool, *model.AppError) { - return srv.Store.Job().UpdateStatusOptimistically(job.Id, model.JOB_STATUS_PENDING, model.JOB_STATUS_IN_PROGRESS) + updated, err := srv.Store.Job().UpdateStatusOptimistically(job.Id, model.JOB_STATUS_PENDING, model.JOB_STATUS_IN_PROGRESS) + if err != nil { + return false, model.NewAppError("ClaimJob", "app.job.update.app_error", nil, err.Error(), http.StatusInternalServerError) + } + + return updated, nil } func (srv *JobServer) SetJobProgress(job *model.Job, progress int64) *model.AppError { @@ -50,21 +67,21 @@ func (srv *JobServer) SetJobProgress(job *model.Job, progress int64) *model.AppE job.Progress = progress if _, err := srv.Store.Job().UpdateOptimistically(job, model.JOB_STATUS_IN_PROGRESS); err != nil { - return err + return model.NewAppError("SetJobProgress", "app.job.update.app_error", nil, err.Error(), http.StatusInternalServerError) } return nil } func (srv *JobServer) SetJobWarning(job *model.Job) *model.AppError { if _, err := srv.Store.Job().UpdateStatus(job.Id, model.JOB_STATUS_WARNING); err != nil { - return err + return model.NewAppError("SetJobWarning", "app.job.update.app_error", nil, err.Error(), http.StatusInternalServerError) } return nil } func (srv *JobServer) SetJobSuccess(job *model.Job) *model.AppError { if _, err := srv.Store.Job().UpdateStatus(job.Id, model.JOB_STATUS_SUCCESS); err != nil { - return err + return model.NewAppError("SetJobSuccess", "app.job.update.app_error", nil, err.Error(), http.StatusInternalServerError) } return nil } @@ -72,7 +89,11 @@ func (srv *JobServer) SetJobSuccess(job *model.Job) *model.AppError { func (srv *JobServer) SetJobError(job *model.Job, jobError *model.AppError) *model.AppError { if jobError == nil { _, err := srv.Store.Job().UpdateStatus(job.Id, model.JOB_STATUS_ERROR) - return err + if err != nil { + return model.NewAppError("SetJobError", "app.job.update.app_error", nil, err.Error(), http.StatusInternalServerError) + } + + return nil } job.Status = model.JOB_STATUS_ERROR @@ -84,16 +105,16 @@ func (srv *JobServer) SetJobError(job *model.Job, jobError *model.AppError) *mod updated, err := srv.Store.Job().UpdateOptimistically(job, model.JOB_STATUS_IN_PROGRESS) if err != nil { - return err + return model.NewAppError("SetJobError", "app.job.update.app_error", nil, err.Error(), http.StatusInternalServerError) } if !updated { updated, err = srv.Store.Job().UpdateOptimistically(job, model.JOB_STATUS_CANCEL_REQUESTED) if err != nil { - return err + return model.NewAppError("SetJobError", "app.job.update.app_error", nil, err.Error(), http.StatusInternalServerError) } if !updated { - return model.NewAppError("Jobs.SetJobError", "jobs.set_job_error.update.error", nil, "id="+job.Id, http.StatusInternalServerError) + return model.NewAppError("SetJobError", "jobs.set_job_error.update.error", nil, "id="+job.Id, http.StatusInternalServerError) } } @@ -102,7 +123,7 @@ func (srv *JobServer) SetJobError(job *model.Job, jobError *model.AppError) *mod func (srv *JobServer) SetJobCanceled(job *model.Job) *model.AppError { if _, err := srv.Store.Job().UpdateStatus(job.Id, model.JOB_STATUS_CANCELED); err != nil { - return err + return model.NewAppError("SetJobCanceled", "app.job.update.app_error", nil, err.Error(), http.StatusInternalServerError) } return nil } @@ -111,7 +132,7 @@ func (srv *JobServer) UpdateInProgressJobData(job *model.Job) *model.AppError { job.Status = model.JOB_STATUS_IN_PROGRESS job.LastActivityAt = model.GetMillis() if _, err := srv.Store.Job().UpdateOptimistically(job, model.JOB_STATUS_IN_PROGRESS); err != nil { - return err + return model.NewAppError("UpdateInProgressJobData", "app.job.update.app_error", nil, err.Error(), http.StatusInternalServerError) } return nil } @@ -119,7 +140,7 @@ func (srv *JobServer) UpdateInProgressJobData(job *model.Job) *model.AppError { func (srv *JobServer) RequestCancellation(jobId string) *model.AppError { updated, err := srv.Store.Job().UpdateStatusOptimistically(jobId, model.JOB_STATUS_PENDING, model.JOB_STATUS_CANCELED) if err != nil { - return err + return model.NewAppError("RequestCancellation", "app.job.update.app_error", nil, err.Error(), http.StatusInternalServerError) } if updated { return nil @@ -127,14 +148,14 @@ func (srv *JobServer) RequestCancellation(jobId string) *model.AppError { updated, err = srv.Store.Job().UpdateStatusOptimistically(jobId, model.JOB_STATUS_IN_PROGRESS, model.JOB_STATUS_CANCEL_REQUESTED) if err != nil { - return err + return model.NewAppError("RequestCancellation", "app.job.update.app_error", nil, err.Error(), http.StatusInternalServerError) } if updated { return nil } - return model.NewAppError("Jobs.RequestCancellation", "jobs.request_cancellation.status.error", nil, "id="+jobId, http.StatusInternalServerError) + return model.NewAppError("RequestCancellation", "jobs.request_cancellation.status.error", nil, "id="+jobId, http.StatusInternalServerError) } func (srv *JobServer) CancellationWatcher(ctx context.Context, jobId string, cancelChan chan interface{}) { @@ -168,7 +189,7 @@ func GenerateNextStartDateTime(now time.Time, nextStartTime time.Time) *time.Tim func (srv *JobServer) CheckForPendingJobsByType(jobType string) (bool, *model.AppError) { count, err := srv.Store.Job().GetCountByStatusAndType(model.JOB_STATUS_PENDING, jobType) if err != nil { - return false, err + return false, model.NewAppError("CheckForPendingJobsByType", "app.job.get_count_by_status_and_type.app_error", nil, err.Error(), http.StatusInternalServerError) } return count > 0, nil } @@ -178,5 +199,16 @@ func (srv *JobServer) GetLastSuccessfulJobByType(jobType string) (*model.Job, *m if jobType == model.JOB_TYPE_MESSAGE_EXPORT { statuses = []string{model.JOB_STATUS_WARNING, model.JOB_STATUS_SUCCESS} } - return srv.Store.Job().GetNewestJobByStatusesAndType(statuses, jobType) + job, err := srv.Store.Job().GetNewestJobByStatusesAndType(statuses, jobType) + if err != nil { + var nfErr *store.ErrNotFound + switch { + case errors.As(err, &nfErr): + return nil, model.NewAppError("GetLastSuccessfulJobByType", "app.job.get_newest_job_by_status_and_type.app_error", nil, nfErr.Error(), http.StatusNotFound) + default: + return nil, model.NewAppError("GetLastSuccessfulJobByType", "app.job.get_newest_job_by_status_and_type.app_error", nil, err.Error(), http.StatusInternalServerError) + } + } + + return job, nil } diff --git a/migrations/migrations.go b/migrations/migrations.go index 7dfc9117bf..ebe9735700 100644 --- a/migrations/migrations.go +++ b/migrations/migrations.go @@ -4,6 +4,8 @@ package migrations import ( + "net/http" + "github.com/mattermost/mattermost-server/v5/app" tjobs "github.com/mattermost/mattermost-server/v5/jobs/interfaces" "github.com/mattermost/mattermost-server/v5/model" @@ -42,7 +44,7 @@ func GetMigrationState(migration string, store store.Store) (string, *model.Job, jobs, err := store.Job().GetAllByType(model.JOB_TYPE_MIGRATIONS) if err != nil { - return "", nil, err + return "", nil, model.NewAppError("GetMigrationState", "app.job.get_all.app_error", nil, err.Error(), http.StatusInternalServerError) } for _, job := range jobs { diff --git a/migrations/migrations_test.go b/migrations/migrations_test.go index 4075b56895..1c45345744 100644 --- a/migrations/migrations_test.go +++ b/migrations/migrations_test.go @@ -56,8 +56,8 @@ func TestGetMigrationState(t *testing.T) { Type: model.JOB_TYPE_MIGRATIONS, } - j1, err = th.App.Srv().Store.Job().Save(j1) - require.Nil(t, err) + j1, nErr = th.App.Srv().Store.Job().Save(j1) + require.Nil(t, nErr) state, job, err = GetMigrationState(migrationKey, th.App.Srv().Store) assert.Nil(t, err) @@ -75,8 +75,8 @@ func TestGetMigrationState(t *testing.T) { Type: model.JOB_TYPE_MIGRATIONS, } - j2, err = th.App.Srv().Store.Job().Save(j2) - require.Nil(t, err) + j2, nErr = th.App.Srv().Store.Job().Save(j2) + require.Nil(t, nErr) state, job, err = GetMigrationState(migrationKey, th.App.Srv().Store) assert.Nil(t, err) @@ -94,8 +94,8 @@ func TestGetMigrationState(t *testing.T) { Type: model.JOB_TYPE_MIGRATIONS, } - j3, err = th.App.Srv().Store.Job().Save(j3) - require.Nil(t, err) + j3, nErr = th.App.Srv().Store.Job().Save(j3) + require.Nil(t, nErr) state, job, err = GetMigrationState(migrationKey, th.App.Srv().Store) assert.Nil(t, err) diff --git a/store/opentracinglayer/opentracinglayer.go b/store/opentracinglayer/opentracinglayer.go index 2b02d39e39..1a3e051711 100644 --- a/store/opentracinglayer/opentracinglayer.go +++ b/store/opentracinglayer/opentracinglayer.go @@ -3883,7 +3883,7 @@ func (s *OpenTracingLayerGroupStore) UpsertMember(groupID string, userID string) return result, err } -func (s *OpenTracingLayerJobStore) Delete(id string) (string, *model.AppError) { +func (s *OpenTracingLayerJobStore) Delete(id string) (string, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "JobStore.Delete") s.Root.Store.SetContext(newCtx) @@ -3901,7 +3901,7 @@ func (s *OpenTracingLayerJobStore) Delete(id string) (string, *model.AppError) { return result, err } -func (s *OpenTracingLayerJobStore) Get(id string) (*model.Job, *model.AppError) { +func (s *OpenTracingLayerJobStore) Get(id string) (*model.Job, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "JobStore.Get") s.Root.Store.SetContext(newCtx) @@ -3919,7 +3919,7 @@ func (s *OpenTracingLayerJobStore) Get(id string) (*model.Job, *model.AppError) return result, err } -func (s *OpenTracingLayerJobStore) GetAllByStatus(status string) ([]*model.Job, *model.AppError) { +func (s *OpenTracingLayerJobStore) GetAllByStatus(status string) ([]*model.Job, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "JobStore.GetAllByStatus") s.Root.Store.SetContext(newCtx) @@ -3937,7 +3937,7 @@ func (s *OpenTracingLayerJobStore) GetAllByStatus(status string) ([]*model.Job, return result, err } -func (s *OpenTracingLayerJobStore) GetAllByType(jobType string) ([]*model.Job, *model.AppError) { +func (s *OpenTracingLayerJobStore) GetAllByType(jobType string) ([]*model.Job, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "JobStore.GetAllByType") s.Root.Store.SetContext(newCtx) @@ -3955,7 +3955,7 @@ func (s *OpenTracingLayerJobStore) GetAllByType(jobType string) ([]*model.Job, * return result, err } -func (s *OpenTracingLayerJobStore) GetAllByTypePage(jobType string, offset int, limit int) ([]*model.Job, *model.AppError) { +func (s *OpenTracingLayerJobStore) GetAllByTypePage(jobType string, offset int, limit int) ([]*model.Job, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "JobStore.GetAllByTypePage") s.Root.Store.SetContext(newCtx) @@ -3973,7 +3973,7 @@ func (s *OpenTracingLayerJobStore) GetAllByTypePage(jobType string, offset int, return result, err } -func (s *OpenTracingLayerJobStore) GetAllPage(offset int, limit int) ([]*model.Job, *model.AppError) { +func (s *OpenTracingLayerJobStore) GetAllPage(offset int, limit int) ([]*model.Job, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "JobStore.GetAllPage") s.Root.Store.SetContext(newCtx) @@ -3991,7 +3991,7 @@ func (s *OpenTracingLayerJobStore) GetAllPage(offset int, limit int) ([]*model.J return result, err } -func (s *OpenTracingLayerJobStore) GetCountByStatusAndType(status string, jobType string) (int64, *model.AppError) { +func (s *OpenTracingLayerJobStore) GetCountByStatusAndType(status string, jobType string) (int64, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "JobStore.GetCountByStatusAndType") s.Root.Store.SetContext(newCtx) @@ -4009,7 +4009,7 @@ func (s *OpenTracingLayerJobStore) GetCountByStatusAndType(status string, jobTyp return result, err } -func (s *OpenTracingLayerJobStore) GetNewestJobByStatusAndType(status string, jobType string) (*model.Job, *model.AppError) { +func (s *OpenTracingLayerJobStore) GetNewestJobByStatusAndType(status string, jobType string) (*model.Job, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "JobStore.GetNewestJobByStatusAndType") s.Root.Store.SetContext(newCtx) @@ -4027,7 +4027,7 @@ func (s *OpenTracingLayerJobStore) GetNewestJobByStatusAndType(status string, jo return result, err } -func (s *OpenTracingLayerJobStore) GetNewestJobByStatusesAndType(statuses []string, jobType string) (*model.Job, *model.AppError) { +func (s *OpenTracingLayerJobStore) GetNewestJobByStatusesAndType(statuses []string, jobType string) (*model.Job, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "JobStore.GetNewestJobByStatusesAndType") s.Root.Store.SetContext(newCtx) @@ -4045,7 +4045,7 @@ func (s *OpenTracingLayerJobStore) GetNewestJobByStatusesAndType(statuses []stri return result, err } -func (s *OpenTracingLayerJobStore) Save(job *model.Job) (*model.Job, *model.AppError) { +func (s *OpenTracingLayerJobStore) Save(job *model.Job) (*model.Job, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "JobStore.Save") s.Root.Store.SetContext(newCtx) @@ -4063,7 +4063,7 @@ func (s *OpenTracingLayerJobStore) Save(job *model.Job) (*model.Job, *model.AppE return result, err } -func (s *OpenTracingLayerJobStore) UpdateOptimistically(job *model.Job, currentStatus string) (bool, *model.AppError) { +func (s *OpenTracingLayerJobStore) UpdateOptimistically(job *model.Job, currentStatus string) (bool, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "JobStore.UpdateOptimistically") s.Root.Store.SetContext(newCtx) @@ -4081,7 +4081,7 @@ func (s *OpenTracingLayerJobStore) UpdateOptimistically(job *model.Job, currentS return result, err } -func (s *OpenTracingLayerJobStore) UpdateStatus(id string, status string) (*model.Job, *model.AppError) { +func (s *OpenTracingLayerJobStore) UpdateStatus(id string, status string) (*model.Job, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "JobStore.UpdateStatus") s.Root.Store.SetContext(newCtx) @@ -4099,7 +4099,7 @@ func (s *OpenTracingLayerJobStore) UpdateStatus(id string, status string) (*mode return result, err } -func (s *OpenTracingLayerJobStore) UpdateStatusOptimistically(id string, currentStatus string, newStatus string) (bool, *model.AppError) { +func (s *OpenTracingLayerJobStore) UpdateStatusOptimistically(id string, currentStatus string, newStatus string) (bool, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "JobStore.UpdateStatusOptimistically") s.Root.Store.SetContext(newCtx) diff --git a/store/retrylayer/retrylayer.go b/store/retrylayer/retrylayer.go index f7d6f240ca..33afb2db2e 100644 --- a/store/retrylayer/retrylayer.go +++ b/store/retrylayer/retrylayer.go @@ -2646,81 +2646,263 @@ func (s *RetryLayerGroupStore) UpsertMember(groupID string, userID string) (*mod } -func (s *RetryLayerJobStore) Delete(id string) (string, *model.AppError) { +func (s *RetryLayerJobStore) Delete(id string) (string, error) { - return s.JobStore.Delete(id) + tries := 0 + for { + result, err := s.JobStore.Delete(id) + if err == nil { + return result, err + } + if !isRepeatableError(err) { + return result, err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return result, err + } + } } -func (s *RetryLayerJobStore) Get(id string) (*model.Job, *model.AppError) { +func (s *RetryLayerJobStore) Get(id string) (*model.Job, error) { - return s.JobStore.Get(id) + tries := 0 + for { + result, err := s.JobStore.Get(id) + if err == nil { + return result, err + } + if !isRepeatableError(err) { + return result, err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return result, err + } + } } -func (s *RetryLayerJobStore) GetAllByStatus(status string) ([]*model.Job, *model.AppError) { +func (s *RetryLayerJobStore) GetAllByStatus(status string) ([]*model.Job, error) { - return s.JobStore.GetAllByStatus(status) + tries := 0 + for { + result, err := s.JobStore.GetAllByStatus(status) + if err == nil { + return result, err + } + if !isRepeatableError(err) { + return result, err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return result, err + } + } } -func (s *RetryLayerJobStore) GetAllByType(jobType string) ([]*model.Job, *model.AppError) { +func (s *RetryLayerJobStore) GetAllByType(jobType string) ([]*model.Job, error) { - return s.JobStore.GetAllByType(jobType) + tries := 0 + for { + result, err := s.JobStore.GetAllByType(jobType) + if err == nil { + return result, err + } + if !isRepeatableError(err) { + return result, err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return result, err + } + } } -func (s *RetryLayerJobStore) GetAllByTypePage(jobType string, offset int, limit int) ([]*model.Job, *model.AppError) { +func (s *RetryLayerJobStore) GetAllByTypePage(jobType string, offset int, limit int) ([]*model.Job, error) { - return s.JobStore.GetAllByTypePage(jobType, offset, limit) + tries := 0 + for { + result, err := s.JobStore.GetAllByTypePage(jobType, offset, limit) + if err == nil { + return result, err + } + if !isRepeatableError(err) { + return result, err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return result, err + } + } } -func (s *RetryLayerJobStore) GetAllPage(offset int, limit int) ([]*model.Job, *model.AppError) { +func (s *RetryLayerJobStore) GetAllPage(offset int, limit int) ([]*model.Job, error) { - return s.JobStore.GetAllPage(offset, limit) + tries := 0 + for { + result, err := s.JobStore.GetAllPage(offset, limit) + if err == nil { + return result, err + } + if !isRepeatableError(err) { + return result, err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return result, err + } + } } -func (s *RetryLayerJobStore) GetCountByStatusAndType(status string, jobType string) (int64, *model.AppError) { +func (s *RetryLayerJobStore) GetCountByStatusAndType(status string, jobType string) (int64, error) { - return s.JobStore.GetCountByStatusAndType(status, jobType) + tries := 0 + for { + result, err := s.JobStore.GetCountByStatusAndType(status, jobType) + if err == nil { + return result, err + } + if !isRepeatableError(err) { + return result, err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return result, err + } + } } -func (s *RetryLayerJobStore) GetNewestJobByStatusAndType(status string, jobType string) (*model.Job, *model.AppError) { +func (s *RetryLayerJobStore) GetNewestJobByStatusAndType(status string, jobType string) (*model.Job, error) { - return s.JobStore.GetNewestJobByStatusAndType(status, jobType) + tries := 0 + for { + result, err := s.JobStore.GetNewestJobByStatusAndType(status, jobType) + if err == nil { + return result, err + } + if !isRepeatableError(err) { + return result, err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return result, err + } + } } -func (s *RetryLayerJobStore) GetNewestJobByStatusesAndType(statuses []string, jobType string) (*model.Job, *model.AppError) { +func (s *RetryLayerJobStore) GetNewestJobByStatusesAndType(statuses []string, jobType string) (*model.Job, error) { - return s.JobStore.GetNewestJobByStatusesAndType(statuses, jobType) + tries := 0 + for { + result, err := s.JobStore.GetNewestJobByStatusesAndType(statuses, jobType) + if err == nil { + return result, err + } + if !isRepeatableError(err) { + return result, err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return result, err + } + } } -func (s *RetryLayerJobStore) Save(job *model.Job) (*model.Job, *model.AppError) { +func (s *RetryLayerJobStore) Save(job *model.Job) (*model.Job, error) { - return s.JobStore.Save(job) + tries := 0 + for { + result, err := s.JobStore.Save(job) + if err == nil { + return result, err + } + if !isRepeatableError(err) { + return result, err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return result, err + } + } } -func (s *RetryLayerJobStore) UpdateOptimistically(job *model.Job, currentStatus string) (bool, *model.AppError) { +func (s *RetryLayerJobStore) UpdateOptimistically(job *model.Job, currentStatus string) (bool, error) { - return s.JobStore.UpdateOptimistically(job, currentStatus) + tries := 0 + for { + result, err := s.JobStore.UpdateOptimistically(job, currentStatus) + if err == nil { + return result, err + } + if !isRepeatableError(err) { + return result, err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return result, err + } + } } -func (s *RetryLayerJobStore) UpdateStatus(id string, status string) (*model.Job, *model.AppError) { +func (s *RetryLayerJobStore) UpdateStatus(id string, status string) (*model.Job, error) { - return s.JobStore.UpdateStatus(id, status) + tries := 0 + for { + result, err := s.JobStore.UpdateStatus(id, status) + if err == nil { + return result, err + } + if !isRepeatableError(err) { + return result, err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return result, err + } + } } -func (s *RetryLayerJobStore) UpdateStatusOptimistically(id string, currentStatus string, newStatus string) (bool, *model.AppError) { +func (s *RetryLayerJobStore) UpdateStatusOptimistically(id string, currentStatus string, newStatus string) (bool, error) { - return s.JobStore.UpdateStatusOptimistically(id, currentStatus, newStatus) + tries := 0 + for { + result, err := s.JobStore.UpdateStatusOptimistically(id, currentStatus, newStatus) + if err == nil { + return result, err + } + if !isRepeatableError(err) { + return result, err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return result, err + } + } } diff --git a/store/sqlstore/job_store.go b/store/sqlstore/job_store.go index 1e538c1861..3dd2fcf122 100644 --- a/store/sqlstore/job_store.go +++ b/store/sqlstore/job_store.go @@ -5,10 +5,11 @@ package sqlstore import ( "database/sql" - "net/http" + "fmt" "strings" sq "github.com/Masterminds/squirrel" + "github.com/pkg/errors" "github.com/mattermost/gorp" "github.com/mattermost/mattermost-server/v5/model" @@ -37,15 +38,15 @@ func (jss SqlJobStore) createIndexesIfNotExists() { jss.CreateIndexIfNotExists("idx_jobs_type", "Jobs", "Type") } -func (jss SqlJobStore) Save(job *model.Job) (*model.Job, *model.AppError) { +func (jss SqlJobStore) Save(job *model.Job) (*model.Job, error) { if err := jss.GetMaster().Insert(job); err != nil { - return nil, model.NewAppError("SqlJobStore.Save", "store.sql_job.save.app_error", nil, "id="+job.Id+", "+err.Error(), http.StatusInternalServerError) + return nil, errors.Wrap(err, "failed to save Job") } return job, nil } -func (jss SqlJobStore) UpdateOptimistically(job *model.Job, currentStatus string) (bool, *model.AppError) { - sql, args, err := jss.getQueryBuilder(). +func (jss SqlJobStore) UpdateOptimistically(job *model.Job, currentStatus string) (bool, error) { + query, args, err := jss.getQueryBuilder(). Update("Jobs"). Set("LastActivityAt", model.GetMillis()). Set("Status", job.Status). @@ -53,17 +54,17 @@ func (jss SqlJobStore) UpdateOptimistically(job *model.Job, currentStatus string Set("Progress", job.Progress). Where(sq.Eq{"Id": job.Id, "Status": currentStatus}).ToSql() if err != nil { - return false, model.NewAppError("SqlJobStore.UpdateOptimistically", "store.sql.build_query.app_error", nil, err.Error(), http.StatusInternalServerError) + return false, errors.Wrap(err, "job_tosql") } - sqlResult, err := jss.GetMaster().Exec(sql, args...) + sqlResult, err := jss.GetMaster().Exec(query, args...) if err != nil { - return false, model.NewAppError("SqlJobStore.UpdateOptimistically", "store.sql_job.update.app_error", nil, "id="+job.Id+", "+err.Error(), http.StatusInternalServerError) + return false, errors.Wrap(err, "failed to update Job") } rows, err := sqlResult.RowsAffected() if err != nil { - return false, model.NewAppError("SqlJobStore.UpdateStatus", "store.sql_job.update.app_error", nil, "id="+job.Id+", "+err.Error(), http.StatusInternalServerError) + return false, errors.Wrap(err, "unable to get rows affected") } if rows != 1 { @@ -73,7 +74,7 @@ func (jss SqlJobStore) UpdateOptimistically(job *model.Job, currentStatus string return true, nil } -func (jss SqlJobStore) UpdateStatus(id string, status string) (*model.Job, *model.AppError) { +func (jss SqlJobStore) UpdateStatus(id string, status string) (*model.Job, error) { job := &model.Job{ Id: id, Status: status, @@ -83,34 +84,34 @@ func (jss SqlJobStore) UpdateStatus(id string, status string) (*model.Job, *mode if _, err := jss.GetMaster().UpdateColumns(func(col *gorp.ColumnMap) bool { return col.ColumnName == "Status" || col.ColumnName == "LastActivityAt" }, job); err != nil { - return nil, model.NewAppError("SqlJobStore.UpdateStatus", "store.sql_job.update.app_error", nil, "id="+id+", "+err.Error(), http.StatusInternalServerError) + return nil, errors.Wrapf(err, "failed to update Job with id=%s", id) } return job, nil } -func (jss SqlJobStore) UpdateStatusOptimistically(id string, currentStatus string, newStatus string) (bool, *model.AppError) { - sql := jss.getQueryBuilder(). +func (jss SqlJobStore) UpdateStatusOptimistically(id string, currentStatus string, newStatus string) (bool, error) { + builder := jss.getQueryBuilder(). Update("Jobs"). Set("LastActivityAt", model.GetMillis()). Set("Status", newStatus). Where(sq.Eq{"Id": id, "Status": currentStatus}) if newStatus == model.JOB_STATUS_IN_PROGRESS { - sql = sql.Set("StartAt", model.GetMillis()) + builder = builder.Set("StartAt", model.GetMillis()) } - query, args, err := sql.ToSql() + query, args, err := builder.ToSql() if err != nil { - return false, model.NewAppError("SqlJobStore.UpdateStatusOptimistically", "store.sql.build_query.app_error", nil, err.Error(), http.StatusInternalServerError) + return false, errors.Wrap(err, "job_tosql") } sqlResult, err := jss.GetMaster().Exec(query, args...) if err != nil { - return false, model.NewAppError("SqlJobStore.UpdateStatusOptimistically", "store.sql_job.update.app_error", nil, "id="+id+", "+err.Error(), http.StatusInternalServerError) + return false, errors.Wrapf(err, "failed to update Job with id=%s", id) } rows, err := sqlResult.RowsAffected() if err != nil { - return false, model.NewAppError("SqlJobStore.UpdateStatus", "store.sql_job.update.app_error", nil, "id="+id+", "+err.Error(), http.StatusInternalServerError) + return false, errors.Wrap(err, "unable to get rows affected") } if rows != 1 { return false, nil @@ -119,25 +120,25 @@ func (jss SqlJobStore) UpdateStatusOptimistically(id string, currentStatus strin return true, nil } -func (jss SqlJobStore) Get(id string) (*model.Job, *model.AppError) { +func (jss SqlJobStore) Get(id string) (*model.Job, error) { query, args, err := jss.getQueryBuilder(). Select("*"). From("Jobs"). Where(sq.Eq{"Id": id}).ToSql() if err != nil { - return nil, model.NewAppError("SqlJobStore.Get", "store.sql.build_query.app_error", nil, err.Error(), http.StatusInternalServerError) + return nil, errors.Wrap(err, "job_tosql") } var status *model.Job if err = jss.GetReplica().SelectOne(&status, query, args...); err != nil { if err == sql.ErrNoRows { - return nil, model.NewAppError("SqlJobStore.Get", "store.sql_job.get.app_error", nil, "Id="+id+", "+err.Error(), http.StatusNotFound) + return nil, store.NewErrNotFound("Job", id) } - return nil, model.NewAppError("SqlJobStore.Get", "store.sql_job.get.app_error", nil, "Id="+id+", "+err.Error(), http.StatusInternalServerError) + return nil, errors.Wrapf(err, "failed to get Job with id=%s", id) } return status, nil } -func (jss SqlJobStore) GetAllPage(offset int, limit int) ([]*model.Job, *model.AppError) { +func (jss SqlJobStore) GetAllPage(offset int, limit int) ([]*model.Job, error) { query, args, err := jss.getQueryBuilder(). Select("*"). From("Jobs"). @@ -145,33 +146,33 @@ func (jss SqlJobStore) GetAllPage(offset int, limit int) ([]*model.Job, *model.A Limit(uint64(limit)). Offset(uint64(offset)).ToSql() if err != nil { - return nil, model.NewAppError("SqlJobStore.GetAllPage", "store.sql.build_query.app_error", nil, err.Error(), http.StatusInternalServerError) + return nil, errors.Wrap(err, "job_tosql") } var statuses []*model.Job if _, err = jss.GetReplica().Select(&statuses, query, args...); err != nil { - return nil, model.NewAppError("SqlJobStore.GetAllPage", "store.sql_job.get_all.app_error", nil, err.Error(), http.StatusInternalServerError) + return nil, errors.Wrap(err, "failed to find Jobs") } return statuses, nil } -func (jss SqlJobStore) GetAllByType(jobType string) ([]*model.Job, *model.AppError) { +func (jss SqlJobStore) GetAllByType(jobType string) ([]*model.Job, error) { query, args, err := jss.getQueryBuilder(). Select("*"). From("Jobs"). Where(sq.Eq{"Type": jobType}). OrderBy("CreateAt DESC").ToSql() if err != nil { - return nil, model.NewAppError("SqlJobStore.GetAllByType", "store.sql.build_query.app_error", nil, err.Error(), http.StatusInternalServerError) + return nil, errors.Wrap(err, "job_tosql") } var statuses []*model.Job if _, err = jss.GetReplica().Select(&statuses, query, args...); err != nil { - return nil, model.NewAppError("SqlJobStore.GetAllByType", "store.sql_job.get_all.app_error", nil, "Type="+jobType+", "+err.Error(), http.StatusInternalServerError) + return nil, errors.Wrapf(err, "failed to find Jobs with type=%s", jobType) } return statuses, nil } -func (jss SqlJobStore) GetAllByTypePage(jobType string, offset int, limit int) ([]*model.Job, *model.AppError) { +func (jss SqlJobStore) GetAllByTypePage(jobType string, offset int, limit int) ([]*model.Job, error) { query, args, err := jss.getQueryBuilder(). Select("*"). From("Jobs"). @@ -180,17 +181,17 @@ func (jss SqlJobStore) GetAllByTypePage(jobType string, offset int, limit int) ( Limit(uint64(limit)). Offset(uint64(offset)).ToSql() if err != nil { - return nil, model.NewAppError("SqlJobStore.GetAllByTypePage", "store.sql.build_query.app_error", nil, err.Error(), http.StatusInternalServerError) + return nil, errors.Wrap(err, "job_tosql") } var statuses []*model.Job if _, err = jss.GetReplica().Select(&statuses, query, args...); err != nil { - return nil, model.NewAppError("SqlJobStore.GetAllByTypePage", "store.sql_job.get_all.app_error", nil, "Type="+jobType+", "+err.Error(), http.StatusInternalServerError) + return nil, errors.Wrapf(err, "failed to find Jobs with type=%s", jobType) } return statuses, nil } -func (jss SqlJobStore) GetAllByStatus(status string) ([]*model.Job, *model.AppError) { +func (jss SqlJobStore) GetAllByStatus(status string) ([]*model.Job, error) { var statuses []*model.Job query, args, err := jss.getQueryBuilder(). Select("*"). @@ -198,19 +199,20 @@ func (jss SqlJobStore) GetAllByStatus(status string) ([]*model.Job, *model.AppEr Where(sq.Eq{"Status": status}). OrderBy("CreateAt ASC").ToSql() if err != nil { - return nil, model.NewAppError("SqlJobStore.GetAllByStatus", "store.sql.build_query.app_error", nil, err.Error(), http.StatusInternalServerError) + return nil, errors.Wrap(err, "job_tosql") } if _, err = jss.GetReplica().Select(&statuses, query, args...); err != nil { - return nil, model.NewAppError("SqlJobStore.GetAllByStatus", "store.sql_job.get_all.app_error", nil, "Status="+status+", "+err.Error(), http.StatusInternalServerError) + return nil, errors.Wrapf(err, "failed to find Jobs with status=%s", status) } return statuses, nil } -func (jss SqlJobStore) GetNewestJobByStatusAndType(status string, jobType string) (*model.Job, *model.AppError) { + +func (jss SqlJobStore) GetNewestJobByStatusAndType(status string, jobType string) (*model.Job, error) { return jss.GetNewestJobByStatusesAndType([]string{status}, jobType) } -func (jss SqlJobStore) GetNewestJobByStatusesAndType(status []string, jobType string) (*model.Job, *model.AppError) { +func (jss SqlJobStore) GetNewestJobByStatusesAndType(status []string, jobType string) (*model.Job, error) { query, args, err := jss.getQueryBuilder(). Select("*"). From("Jobs"). @@ -218,41 +220,44 @@ func (jss SqlJobStore) GetNewestJobByStatusesAndType(status []string, jobType st OrderBy("CreateAt DESC"). Limit(1).ToSql() if err != nil { - return nil, model.NewAppError("SqlJobStore.GetNewestJobByStatusAndType", "store.sql.build_query.app_error", nil, err.Error(), http.StatusInternalServerError) + return nil, errors.Wrap(err, "job_tosql") } var job *model.Job - if err = jss.GetReplica().SelectOne(&job, query, args...); err != nil && err != sql.ErrNoRows { - return nil, model.NewAppError("SqlJobStore.GetNewestJobByStatusAndType", "store.sql_job.get_newest_job_by_status_and_type.app_error", nil, "Status="+strings.Join(status, ",")+", "+err.Error(), http.StatusInternalServerError) + if err = jss.GetReplica().SelectOne(&job, query, args...); err != nil { + if err == sql.ErrNoRows { + return nil, store.NewErrNotFound("Job", fmt.Sprintf("=<%s, %s>", strings.Join(status, ","), jobType)) + } + return nil, errors.Wrapf(err, "failed to find Job with statuses=%s and type=%s", strings.Join(status, ","), jobType) } return job, nil } -func (jss SqlJobStore) GetCountByStatusAndType(status string, jobType string) (int64, *model.AppError) { +func (jss SqlJobStore) GetCountByStatusAndType(status string, jobType string) (int64, error) { query, args, err := jss.getQueryBuilder(). Select("COUNT(*)"). From("Jobs"). Where(sq.Eq{"Status": status, "Type": jobType}).ToSql() if err != nil { - return 0, model.NewAppError("SqlJobStore.GetCountByStatusAndType", "store.sql.build_query.app_error", nil, err.Error(), http.StatusInternalServerError) + return 0, errors.Wrap(err, "job_tosql") } count, err := jss.GetReplica().SelectInt(query, args...) if err != nil { - return int64(0), model.NewAppError("SqlJobStore.GetCountByStatusAndType", "store.sql_job.get_count_by_status_and_type.app_error", nil, "Status="+status+", "+err.Error(), http.StatusInternalServerError) + return int64(0), errors.Wrapf(err, "failed to count Jobs with status=%s and type=%s", status, jobType) } return count, nil } -func (jss SqlJobStore) Delete(id string) (string, *model.AppError) { - sql, args, err := jss.getQueryBuilder(). +func (jss SqlJobStore) Delete(id string) (string, error) { + query, args, err := jss.getQueryBuilder(). Delete("Jobs"). Where(sq.Eq{"Id": id}).ToSql() if err != nil { - return "", model.NewAppError("SqlJobStore.DeleteByType", "store.sql.build_query.app_error", nil, err.Error(), http.StatusInternalServerError) + return "", errors.Wrap(err, "job_tosql") } - if _, err = jss.GetMaster().Exec(sql, args...); err != nil { - return "", model.NewAppError("SqlJobStore.DeleteByType", "store.sql_job.delete.app_error", nil, "id="+id+", "+err.Error(), http.StatusInternalServerError) + if _, err = jss.GetMaster().Exec(query, args...); err != nil { + return "", errors.Wrapf(err, "failed to delete Job with id=%s", id) } return id, nil } diff --git a/store/store.go b/store/store.go index 157091c67d..b67d59a2cd 100644 --- a/store/store.go +++ b/store/store.go @@ -559,19 +559,19 @@ type ReactionStore interface { } type JobStore interface { - Save(job *model.Job) (*model.Job, *model.AppError) - UpdateOptimistically(job *model.Job, currentStatus string) (bool, *model.AppError) - UpdateStatus(id string, status string) (*model.Job, *model.AppError) - UpdateStatusOptimistically(id string, currentStatus string, newStatus string) (bool, *model.AppError) - Get(id string) (*model.Job, *model.AppError) - GetAllPage(offset int, limit int) ([]*model.Job, *model.AppError) - GetAllByType(jobType string) ([]*model.Job, *model.AppError) - GetAllByTypePage(jobType string, offset int, limit int) ([]*model.Job, *model.AppError) - GetAllByStatus(status string) ([]*model.Job, *model.AppError) - GetNewestJobByStatusAndType(status string, jobType string) (*model.Job, *model.AppError) - GetNewestJobByStatusesAndType(statuses []string, jobType string) (*model.Job, *model.AppError) - GetCountByStatusAndType(status string, jobType string) (int64, *model.AppError) - Delete(id string) (string, *model.AppError) + Save(job *model.Job) (*model.Job, error) + UpdateOptimistically(job *model.Job, currentStatus string) (bool, error) + UpdateStatus(id string, status string) (*model.Job, error) + UpdateStatusOptimistically(id string, currentStatus string, newStatus string) (bool, error) + Get(id string) (*model.Job, error) + GetAllPage(offset int, limit int) ([]*model.Job, error) + GetAllByType(jobType string) ([]*model.Job, error) + GetAllByTypePage(jobType string, offset int, limit int) ([]*model.Job, error) + GetAllByStatus(status string) ([]*model.Job, error) + GetNewestJobByStatusAndType(status string, jobType string) (*model.Job, error) + GetNewestJobByStatusesAndType(statuses []string, jobType string) (*model.Job, error) + GetCountByStatusAndType(status string, jobType string) (int64, error) + Delete(id string) (string, error) } type UserAccessTokenStore interface { diff --git a/store/storetest/job_store.go b/store/storetest/job_store.go index 463d79363f..6a51c2b74c 100644 --- a/store/storetest/job_store.go +++ b/store/storetest/job_store.go @@ -4,6 +4,7 @@ package storetest import ( + "errors" "testing" "time" @@ -258,7 +259,9 @@ func testJobStoreGetNewestJobByStatusAndType(t *testing.T, ss store.Store) { assert.EqualValues(t, jobs[0].Id, received.Id) received, err = ss.Job().GetNewestJobByStatusAndType(model.NewId(), model.NewId()) - assert.Nil(t, err) + assert.NotNil(t, err) + var nfErr *store.ErrNotFound + assert.True(t, errors.As(err, &nfErr)) assert.Nil(t, received) } @@ -306,11 +309,14 @@ func testJobStoreGetNewestJobByStatusesAndType(t *testing.T, ss store.Store) { assert.EqualValues(t, jobs[3].Id, received.Id) received, err = ss.Job().GetNewestJobByStatusesAndType([]string{model.NewId(), model.NewId()}, model.NewId()) - assert.Nil(t, err) + assert.NotNil(t, err) + var nfErr *store.ErrNotFound + assert.True(t, errors.As(err, &nfErr)) assert.Nil(t, received) received, err = ss.Job().GetNewestJobByStatusesAndType([]string{status2}, jobType2) - assert.Nil(t, err) + assert.NotNil(t, err) + assert.True(t, errors.As(err, &nfErr)) assert.Nil(t, received) received, err = ss.Job().GetNewestJobByStatusesAndType([]string{status1}, jobType2) @@ -318,7 +324,8 @@ func testJobStoreGetNewestJobByStatusesAndType(t *testing.T, ss store.Store) { assert.EqualValues(t, jobs[2].Id, received.Id) received, err = ss.Job().GetNewestJobByStatusesAndType([]string{}, jobType1) - assert.Nil(t, err) + assert.NotNil(t, err) + assert.True(t, errors.As(err, &nfErr)) assert.Nil(t, received) } diff --git a/store/storetest/mocks/JobStore.go b/store/storetest/mocks/JobStore.go index 394089a79f..659ff0e55f 100644 --- a/store/storetest/mocks/JobStore.go +++ b/store/storetest/mocks/JobStore.go @@ -15,7 +15,7 @@ type JobStore struct { } // Delete provides a mock function with given fields: id -func (_m *JobStore) Delete(id string) (string, *model.AppError) { +func (_m *JobStore) Delete(id string) (string, error) { ret := _m.Called(id) var r0 string @@ -25,20 +25,18 @@ func (_m *JobStore) Delete(id string) (string, *model.AppError) { r0 = ret.Get(0).(string) } - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(string) *model.AppError); ok { + var r1 error + if rf, ok := ret.Get(1).(func(string) error); ok { r1 = rf(id) } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } + r1 = ret.Error(1) } return r0, r1 } // Get provides a mock function with given fields: id -func (_m *JobStore) Get(id string) (*model.Job, *model.AppError) { +func (_m *JobStore) Get(id string) (*model.Job, error) { ret := _m.Called(id) var r0 *model.Job @@ -50,20 +48,18 @@ func (_m *JobStore) Get(id string) (*model.Job, *model.AppError) { } } - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(string) *model.AppError); ok { + var r1 error + if rf, ok := ret.Get(1).(func(string) error); ok { r1 = rf(id) } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } + r1 = ret.Error(1) } return r0, r1 } // GetAllByStatus provides a mock function with given fields: status -func (_m *JobStore) GetAllByStatus(status string) ([]*model.Job, *model.AppError) { +func (_m *JobStore) GetAllByStatus(status string) ([]*model.Job, error) { ret := _m.Called(status) var r0 []*model.Job @@ -75,20 +71,18 @@ func (_m *JobStore) GetAllByStatus(status string) ([]*model.Job, *model.AppError } } - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(string) *model.AppError); ok { + var r1 error + if rf, ok := ret.Get(1).(func(string) error); ok { r1 = rf(status) } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } + r1 = ret.Error(1) } return r0, r1 } // GetAllByType provides a mock function with given fields: jobType -func (_m *JobStore) GetAllByType(jobType string) ([]*model.Job, *model.AppError) { +func (_m *JobStore) GetAllByType(jobType string) ([]*model.Job, error) { ret := _m.Called(jobType) var r0 []*model.Job @@ -100,20 +94,18 @@ func (_m *JobStore) GetAllByType(jobType string) ([]*model.Job, *model.AppError) } } - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(string) *model.AppError); ok { + var r1 error + if rf, ok := ret.Get(1).(func(string) error); ok { r1 = rf(jobType) } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } + r1 = ret.Error(1) } return r0, r1 } // GetAllByTypePage provides a mock function with given fields: jobType, offset, limit -func (_m *JobStore) GetAllByTypePage(jobType string, offset int, limit int) ([]*model.Job, *model.AppError) { +func (_m *JobStore) GetAllByTypePage(jobType string, offset int, limit int) ([]*model.Job, error) { ret := _m.Called(jobType, offset, limit) var r0 []*model.Job @@ -125,20 +117,18 @@ func (_m *JobStore) GetAllByTypePage(jobType string, offset int, limit int) ([]* } } - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(string, int, int) *model.AppError); ok { + var r1 error + if rf, ok := ret.Get(1).(func(string, int, int) error); ok { r1 = rf(jobType, offset, limit) } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } + r1 = ret.Error(1) } return r0, r1 } // GetAllPage provides a mock function with given fields: offset, limit -func (_m *JobStore) GetAllPage(offset int, limit int) ([]*model.Job, *model.AppError) { +func (_m *JobStore) GetAllPage(offset int, limit int) ([]*model.Job, error) { ret := _m.Called(offset, limit) var r0 []*model.Job @@ -150,20 +140,18 @@ func (_m *JobStore) GetAllPage(offset int, limit int) ([]*model.Job, *model.AppE } } - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(int, int) *model.AppError); ok { + var r1 error + if rf, ok := ret.Get(1).(func(int, int) error); ok { r1 = rf(offset, limit) } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } + r1 = ret.Error(1) } return r0, r1 } // GetCountByStatusAndType provides a mock function with given fields: status, jobType -func (_m *JobStore) GetCountByStatusAndType(status string, jobType string) (int64, *model.AppError) { +func (_m *JobStore) GetCountByStatusAndType(status string, jobType string) (int64, error) { ret := _m.Called(status, jobType) var r0 int64 @@ -173,20 +161,18 @@ func (_m *JobStore) GetCountByStatusAndType(status string, jobType string) (int6 r0 = ret.Get(0).(int64) } - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(string, string) *model.AppError); ok { + var r1 error + if rf, ok := ret.Get(1).(func(string, string) error); ok { r1 = rf(status, jobType) } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } + r1 = ret.Error(1) } return r0, r1 } // GetNewestJobByStatusAndType provides a mock function with given fields: status, jobType -func (_m *JobStore) GetNewestJobByStatusAndType(status string, jobType string) (*model.Job, *model.AppError) { +func (_m *JobStore) GetNewestJobByStatusAndType(status string, jobType string) (*model.Job, error) { ret := _m.Called(status, jobType) var r0 *model.Job @@ -198,20 +184,18 @@ func (_m *JobStore) GetNewestJobByStatusAndType(status string, jobType string) ( } } - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(string, string) *model.AppError); ok { + var r1 error + if rf, ok := ret.Get(1).(func(string, string) error); ok { r1 = rf(status, jobType) } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } + r1 = ret.Error(1) } return r0, r1 } // GetNewestJobByStatusesAndType provides a mock function with given fields: statuses, jobType -func (_m *JobStore) GetNewestJobByStatusesAndType(statuses []string, jobType string) (*model.Job, *model.AppError) { +func (_m *JobStore) GetNewestJobByStatusesAndType(statuses []string, jobType string) (*model.Job, error) { ret := _m.Called(statuses, jobType) var r0 *model.Job @@ -223,20 +207,18 @@ func (_m *JobStore) GetNewestJobByStatusesAndType(statuses []string, jobType str } } - var r1 *model.AppError - if rf, ok := ret.Get(1).(func([]string, string) *model.AppError); ok { + var r1 error + if rf, ok := ret.Get(1).(func([]string, string) error); ok { r1 = rf(statuses, jobType) } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } + r1 = ret.Error(1) } return r0, r1 } // Save provides a mock function with given fields: job -func (_m *JobStore) Save(job *model.Job) (*model.Job, *model.AppError) { +func (_m *JobStore) Save(job *model.Job) (*model.Job, error) { ret := _m.Called(job) var r0 *model.Job @@ -248,20 +230,18 @@ func (_m *JobStore) Save(job *model.Job) (*model.Job, *model.AppError) { } } - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(*model.Job) *model.AppError); ok { + var r1 error + if rf, ok := ret.Get(1).(func(*model.Job) error); ok { r1 = rf(job) } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } + r1 = ret.Error(1) } return r0, r1 } // UpdateOptimistically provides a mock function with given fields: job, currentStatus -func (_m *JobStore) UpdateOptimistically(job *model.Job, currentStatus string) (bool, *model.AppError) { +func (_m *JobStore) UpdateOptimistically(job *model.Job, currentStatus string) (bool, error) { ret := _m.Called(job, currentStatus) var r0 bool @@ -271,20 +251,18 @@ func (_m *JobStore) UpdateOptimistically(job *model.Job, currentStatus string) ( r0 = ret.Get(0).(bool) } - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(*model.Job, string) *model.AppError); ok { + var r1 error + if rf, ok := ret.Get(1).(func(*model.Job, string) error); ok { r1 = rf(job, currentStatus) } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } + r1 = ret.Error(1) } return r0, r1 } // UpdateStatus provides a mock function with given fields: id, status -func (_m *JobStore) UpdateStatus(id string, status string) (*model.Job, *model.AppError) { +func (_m *JobStore) UpdateStatus(id string, status string) (*model.Job, error) { ret := _m.Called(id, status) var r0 *model.Job @@ -296,20 +274,18 @@ func (_m *JobStore) UpdateStatus(id string, status string) (*model.Job, *model.A } } - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(string, string) *model.AppError); ok { + var r1 error + if rf, ok := ret.Get(1).(func(string, string) error); ok { r1 = rf(id, status) } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } + r1 = ret.Error(1) } return r0, r1 } // UpdateStatusOptimistically provides a mock function with given fields: id, currentStatus, newStatus -func (_m *JobStore) UpdateStatusOptimistically(id string, currentStatus string, newStatus string) (bool, *model.AppError) { +func (_m *JobStore) UpdateStatusOptimistically(id string, currentStatus string, newStatus string) (bool, error) { ret := _m.Called(id, currentStatus, newStatus) var r0 bool @@ -319,13 +295,11 @@ func (_m *JobStore) UpdateStatusOptimistically(id string, currentStatus string, r0 = ret.Get(0).(bool) } - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(string, string, string) *model.AppError); ok { + var r1 error + if rf, ok := ret.Get(1).(func(string, string, string) error); ok { r1 = rf(id, currentStatus, newStatus) } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } + r1 = ret.Error(1) } return r0, r1 diff --git a/store/timerlayer/timerlayer.go b/store/timerlayer/timerlayer.go index cfa011f30d..388c5004ef 100644 --- a/store/timerlayer/timerlayer.go +++ b/store/timerlayer/timerlayer.go @@ -3535,7 +3535,7 @@ func (s *TimerLayerGroupStore) UpsertMember(groupID string, userID string) (*mod return result, err } -func (s *TimerLayerJobStore) Delete(id string) (string, *model.AppError) { +func (s *TimerLayerJobStore) Delete(id string) (string, error) { start := timemodule.Now() result, err := s.JobStore.Delete(id) @@ -3551,7 +3551,7 @@ func (s *TimerLayerJobStore) Delete(id string) (string, *model.AppError) { return result, err } -func (s *TimerLayerJobStore) Get(id string) (*model.Job, *model.AppError) { +func (s *TimerLayerJobStore) Get(id string) (*model.Job, error) { start := timemodule.Now() result, err := s.JobStore.Get(id) @@ -3567,7 +3567,7 @@ func (s *TimerLayerJobStore) Get(id string) (*model.Job, *model.AppError) { return result, err } -func (s *TimerLayerJobStore) GetAllByStatus(status string) ([]*model.Job, *model.AppError) { +func (s *TimerLayerJobStore) GetAllByStatus(status string) ([]*model.Job, error) { start := timemodule.Now() result, err := s.JobStore.GetAllByStatus(status) @@ -3583,7 +3583,7 @@ func (s *TimerLayerJobStore) GetAllByStatus(status string) ([]*model.Job, *model return result, err } -func (s *TimerLayerJobStore) GetAllByType(jobType string) ([]*model.Job, *model.AppError) { +func (s *TimerLayerJobStore) GetAllByType(jobType string) ([]*model.Job, error) { start := timemodule.Now() result, err := s.JobStore.GetAllByType(jobType) @@ -3599,7 +3599,7 @@ func (s *TimerLayerJobStore) GetAllByType(jobType string) ([]*model.Job, *model. return result, err } -func (s *TimerLayerJobStore) GetAllByTypePage(jobType string, offset int, limit int) ([]*model.Job, *model.AppError) { +func (s *TimerLayerJobStore) GetAllByTypePage(jobType string, offset int, limit int) ([]*model.Job, error) { start := timemodule.Now() result, err := s.JobStore.GetAllByTypePage(jobType, offset, limit) @@ -3615,7 +3615,7 @@ func (s *TimerLayerJobStore) GetAllByTypePage(jobType string, offset int, limit return result, err } -func (s *TimerLayerJobStore) GetAllPage(offset int, limit int) ([]*model.Job, *model.AppError) { +func (s *TimerLayerJobStore) GetAllPage(offset int, limit int) ([]*model.Job, error) { start := timemodule.Now() result, err := s.JobStore.GetAllPage(offset, limit) @@ -3631,7 +3631,7 @@ func (s *TimerLayerJobStore) GetAllPage(offset int, limit int) ([]*model.Job, *m return result, err } -func (s *TimerLayerJobStore) GetCountByStatusAndType(status string, jobType string) (int64, *model.AppError) { +func (s *TimerLayerJobStore) GetCountByStatusAndType(status string, jobType string) (int64, error) { start := timemodule.Now() result, err := s.JobStore.GetCountByStatusAndType(status, jobType) @@ -3647,7 +3647,7 @@ func (s *TimerLayerJobStore) GetCountByStatusAndType(status string, jobType stri return result, err } -func (s *TimerLayerJobStore) GetNewestJobByStatusAndType(status string, jobType string) (*model.Job, *model.AppError) { +func (s *TimerLayerJobStore) GetNewestJobByStatusAndType(status string, jobType string) (*model.Job, error) { start := timemodule.Now() result, err := s.JobStore.GetNewestJobByStatusAndType(status, jobType) @@ -3663,7 +3663,7 @@ func (s *TimerLayerJobStore) GetNewestJobByStatusAndType(status string, jobType return result, err } -func (s *TimerLayerJobStore) GetNewestJobByStatusesAndType(statuses []string, jobType string) (*model.Job, *model.AppError) { +func (s *TimerLayerJobStore) GetNewestJobByStatusesAndType(statuses []string, jobType string) (*model.Job, error) { start := timemodule.Now() result, err := s.JobStore.GetNewestJobByStatusesAndType(statuses, jobType) @@ -3679,7 +3679,7 @@ func (s *TimerLayerJobStore) GetNewestJobByStatusesAndType(statuses []string, jo return result, err } -func (s *TimerLayerJobStore) Save(job *model.Job) (*model.Job, *model.AppError) { +func (s *TimerLayerJobStore) Save(job *model.Job) (*model.Job, error) { start := timemodule.Now() result, err := s.JobStore.Save(job) @@ -3695,7 +3695,7 @@ func (s *TimerLayerJobStore) Save(job *model.Job) (*model.Job, *model.AppError) return result, err } -func (s *TimerLayerJobStore) UpdateOptimistically(job *model.Job, currentStatus string) (bool, *model.AppError) { +func (s *TimerLayerJobStore) UpdateOptimistically(job *model.Job, currentStatus string) (bool, error) { start := timemodule.Now() result, err := s.JobStore.UpdateOptimistically(job, currentStatus) @@ -3711,7 +3711,7 @@ func (s *TimerLayerJobStore) UpdateOptimistically(job *model.Job, currentStatus return result, err } -func (s *TimerLayerJobStore) UpdateStatus(id string, status string) (*model.Job, *model.AppError) { +func (s *TimerLayerJobStore) UpdateStatus(id string, status string) (*model.Job, error) { start := timemodule.Now() result, err := s.JobStore.UpdateStatus(id, status) @@ -3727,7 +3727,7 @@ func (s *TimerLayerJobStore) UpdateStatus(id string, status string) (*model.Job, return result, err } -func (s *TimerLayerJobStore) UpdateStatusOptimistically(id string, currentStatus string, newStatus string) (bool, *model.AppError) { +func (s *TimerLayerJobStore) UpdateStatusOptimistically(id string, currentStatus string, newStatus string) (bool, error) { start := timemodule.Now() result, err := s.JobStore.UpdateStatusOptimistically(id, currentStatus, newStatus)