From 2bf73fbb5cef4643886c18dd38c62709806a6c70 Mon Sep 17 00:00:00 2001 From: Vistaar Juneja Date: Wed, 9 Aug 2023 23:56:47 +0100 Subject: [PATCH] add UpdateOptLock --- internal/api/controller/execution/list.go | 4 +- internal/api/controller/execution/update.go | 12 ++- internal/api/controller/pipeline/update.go | 22 +++-- internal/api/controller/secret/update.go | 22 +++-- .../api/controller/space/list_pipelines.go | 12 ++- internal/api/controller/space/list_secrets.go | 12 ++- internal/api/handler/execution/list.go | 6 +- internal/api/handler/space/list_pipelines.go | 6 +- internal/api/handler/space/list_secrets.go | 6 +- internal/api/request/pipeline.go | 19 ---- internal/api/request/secret.go | 10 -- internal/api/request/util.go | 39 +++----- internal/store/database.go | 26 +++-- internal/store/database/execution.go | 97 ++++++++++++------- internal/store/database/pipeline.go | 43 ++++++-- internal/store/database/secret.go | 49 ++++++++-- mocks/mock_client.go | 3 +- mocks/mock_store.go | 3 +- types/execution.go | 6 -- types/pagination.go | 12 +++ types/pipeline.go | 7 -- types/secret.go | 7 -- 22 files changed, 247 insertions(+), 176 deletions(-) create mode 100644 types/pagination.go diff --git a/internal/api/controller/execution/list.go b/internal/api/controller/execution/list.go index 7d9c79488..ef9ad1a0d 100644 --- a/internal/api/controller/execution/list.go +++ b/internal/api/controller/execution/list.go @@ -19,7 +19,7 @@ func (c *Controller) List( session *auth.Session, spaceRef string, pipelineUID string, - filter *types.ExecutionFilter, + pagination types.Pagination, ) ([]types.Execution, int64, error) { space, err := c.spaceStore.FindByRef(ctx, spaceRef) if err != nil { @@ -45,7 +45,7 @@ func (c *Controller) List( return fmt.Errorf("failed to count child executions: %w", err) } - executions, dbErr = c.executionStore.List(ctx, pipeline.ID, filter) + executions, dbErr = c.executionStore.List(ctx, pipeline.ID, pagination) if dbErr != nil { return fmt.Errorf("failed to list child executions: %w", err) } diff --git a/internal/api/controller/execution/update.go b/internal/api/controller/execution/update.go index 820d97adb..0bd8a8959 100644 --- a/internal/api/controller/execution/update.go +++ b/internal/api/controller/execution/update.go @@ -45,9 +45,13 @@ func (c *Controller) Update( return nil, fmt.Errorf("failed to find execution: %w", err) } - if in.Status != "" { - execution.Status = in.Status - } + return c.executionStore.UpdateOptLock(ctx, + execution, func(original *types.Execution) error { + // update values only if provided + if in.Status != "" { + original.Status = in.Status + } - return c.executionStore.Update(ctx, execution) + return nil + }) } diff --git a/internal/api/controller/pipeline/update.go b/internal/api/controller/pipeline/update.go index 5f97b3d9d..7cc75cdc9 100644 --- a/internal/api/controller/pipeline/update.go +++ b/internal/api/controller/pipeline/update.go @@ -42,15 +42,17 @@ func (c *Controller) Update( return nil, fmt.Errorf("could not find pipeline: %w", err) } - if in.Description != "" { - pipeline.Description = in.Description - } - if in.UID != "" { - pipeline.UID = in.UID - } - if in.ConfigPath != "" { - pipeline.ConfigPath = in.ConfigPath - } + return c.pipelineStore.UpdateOptLock(ctx, pipeline, func(pipeline *types.Pipeline) error { + if in.Description != "" { + pipeline.Description = in.Description + } + if in.UID != "" { + pipeline.UID = in.UID + } + if in.ConfigPath != "" { + pipeline.ConfigPath = in.ConfigPath + } - return c.pipelineStore.Update(ctx, pipeline) + return nil + }) } diff --git a/internal/api/controller/secret/update.go b/internal/api/controller/secret/update.go index d495dc082..3d283b11d 100644 --- a/internal/api/controller/secret/update.go +++ b/internal/api/controller/secret/update.go @@ -41,15 +41,17 @@ func (c *Controller) Update( return nil, err } - if in.Description != "" { - secret.Description = in.Description - } - if in.Data != "" { - secret.Data = in.Data // will get encrypted at db layer - } - if in.UID != "" { - secret.UID = in.UID - } + return c.secretStore.UpdateOptLock(ctx, secret, func(original *types.Secret) error { + if in.Description != "" { + original.Description = in.Description + } + if in.Data != "" { + original.Data = in.Data // will get encrypted at db layer + } + if in.UID != "" { + original.UID = in.UID + } - return c.secretStore.Update(ctx, secret) + return nil + }) } diff --git a/internal/api/controller/space/list_pipelines.go b/internal/api/controller/space/list_pipelines.go index 59e709e7a..6a70162c9 100644 --- a/internal/api/controller/space/list_pipelines.go +++ b/internal/api/controller/space/list_pipelines.go @@ -15,8 +15,12 @@ import ( ) // ListPipelines lists the pipelines in a space. -func (c *Controller) ListPipelines(ctx context.Context, session *auth.Session, - spaceRef string, filter *types.PipelineFilter) ([]types.Pipeline, int64, error) { +func (c *Controller) ListPipelines( + ctx context.Context, + session *auth.Session, + spaceRef string, + pagination types.Pagination, +) ([]types.Pipeline, int64, error) { space, err := c.spaceStore.FindByRef(ctx, spaceRef) if err != nil { return nil, 0, fmt.Errorf("failed to find parent space: %w", err) @@ -32,12 +36,12 @@ func (c *Controller) ListPipelines(ctx context.Context, session *auth.Session, err = dbtx.New(c.db).WithTx(ctx, func(ctx context.Context) (err error) { var dbErr error - count, dbErr = c.pipelineStore.Count(ctx, space.ID, filter) + count, dbErr = c.pipelineStore.Count(ctx, space.ID, pagination) if dbErr != nil { return fmt.Errorf("failed to count child executions: %w", err) } - pipelines, dbErr = c.pipelineStore.List(ctx, space.ID, filter) + pipelines, dbErr = c.pipelineStore.List(ctx, space.ID, pagination) if dbErr != nil { return fmt.Errorf("failed to list child executions: %w", err) } diff --git a/internal/api/controller/space/list_secrets.go b/internal/api/controller/space/list_secrets.go index 0e175d798..ff49b06d4 100644 --- a/internal/api/controller/space/list_secrets.go +++ b/internal/api/controller/space/list_secrets.go @@ -15,8 +15,12 @@ import ( ) // ListSecrets lists the secrets in a space. -func (c *Controller) ListSecrets(ctx context.Context, session *auth.Session, - spaceRef string, filter *types.SecretFilter) ([]types.Secret, int64, error) { +func (c *Controller) ListSecrets( + ctx context.Context, + session *auth.Session, + spaceRef string, + pagination types.Pagination, +) ([]types.Secret, int64, error) { space, err := c.spaceStore.FindByRef(ctx, spaceRef) if err != nil { return nil, 0, fmt.Errorf("failed to find parent space: %w", err) @@ -32,12 +36,12 @@ func (c *Controller) ListSecrets(ctx context.Context, session *auth.Session, err = dbtx.New(c.db).WithTx(ctx, func(ctx context.Context) (err error) { var dbErr error - count, dbErr = c.secretStore.Count(ctx, space.ID, filter) + count, dbErr = c.secretStore.Count(ctx, space.ID, pagination) if dbErr != nil { return fmt.Errorf("failed to count child executions: %w", err) } - secrets, dbErr = c.secretStore.List(ctx, space.ID, filter) + secrets, dbErr = c.secretStore.List(ctx, space.ID, pagination) if dbErr != nil { return fmt.Errorf("failed to list child executions: %w", err) } diff --git a/internal/api/handler/execution/list.go b/internal/api/handler/execution/list.go index 3ade46038..eb590f239 100644 --- a/internal/api/handler/execution/list.go +++ b/internal/api/handler/execution/list.go @@ -28,15 +28,15 @@ func HandleList(executionCtrl *execution.Controller) http.HandlerFunc { return } - filter := request.ParseExecutionFilter(r) + pagination := request.ParsePaginationFromRequest(r) - repos, totalCount, err := executionCtrl.List(ctx, session, spaceRef, pipelineUID, filter) + repos, totalCount, err := executionCtrl.List(ctx, session, spaceRef, pipelineUID, pagination) if err != nil { render.TranslatedUserError(w, err) return } - render.Pagination(r, w, filter.Page, filter.Size, int(totalCount)) + render.Pagination(r, w, pagination.Page, pagination.Size, int(totalCount)) render.JSON(w, http.StatusOK, repos) } } diff --git a/internal/api/handler/space/list_pipelines.go b/internal/api/handler/space/list_pipelines.go index 0bdd02633..764145ac4 100644 --- a/internal/api/handler/space/list_pipelines.go +++ b/internal/api/handler/space/list_pipelines.go @@ -22,14 +22,14 @@ func HandleListPipelines(spaceCtrl *space.Controller) http.HandlerFunc { return } - filter := request.ParsePipelineFilter(r) - repos, totalCount, err := spaceCtrl.ListPipelines(ctx, session, spaceRef, filter) + pagination := request.ParsePaginationFromRequest(r) + repos, totalCount, err := spaceCtrl.ListPipelines(ctx, session, spaceRef, pagination) if err != nil { render.TranslatedUserError(w, err) return } - render.Pagination(r, w, filter.Page, filter.Size, int(totalCount)) + render.Pagination(r, w, pagination.Page, pagination.Page, int(totalCount)) render.JSON(w, http.StatusOK, repos) } } diff --git a/internal/api/handler/space/list_secrets.go b/internal/api/handler/space/list_secrets.go index 2209493b2..ab76ee74d 100644 --- a/internal/api/handler/space/list_secrets.go +++ b/internal/api/handler/space/list_secrets.go @@ -23,8 +23,8 @@ func HandleListSecrets(spaceCtrl *space.Controller) http.HandlerFunc { return } - filter := request.ParseSecretFilter(r) - ret, totalCount, err := spaceCtrl.ListSecrets(ctx, session, spaceRef, filter) + pagination := request.ParsePaginationFromRequest(r) + ret, totalCount, err := spaceCtrl.ListSecrets(ctx, session, spaceRef, pagination) if err != nil { render.TranslatedUserError(w, err) return @@ -36,7 +36,7 @@ func HandleListSecrets(spaceCtrl *space.Controller) http.HandlerFunc { secrets = append(secrets, *s.CopyWithoutData()) } - render.Pagination(r, w, filter.Page, filter.Size, int(totalCount)) + render.Pagination(r, w, pagination.Page, pagination.Size, int(totalCount)) render.JSON(w, http.StatusOK, secrets) } } diff --git a/internal/api/request/pipeline.go b/internal/api/request/pipeline.go index 4dbb5abde..d6259ae32 100644 --- a/internal/api/request/pipeline.go +++ b/internal/api/request/pipeline.go @@ -7,8 +7,6 @@ package request import ( "net/http" "net/url" - - "github.com/harness/gitness/types" ) const ( @@ -30,20 +28,3 @@ func GetPipelineRefFromPath(r *http.Request) (string, error) { func GetExecutionNumberFromPath(r *http.Request) (int64, error) { return PathParamAsPositiveInt64(r, ExecutionNumber) } - -// ParsePipelineFilter extracts the pipeline filter from the url. -func ParsePipelineFilter(r *http.Request) *types.PipelineFilter { - return &types.PipelineFilter{ - Query: ParseQuery(r), - Page: ParsePage(r), - Size: ParseLimit(r), - } -} - -// ParseExecutionFilter extracts the execution filter from the url. -func ParseExecutionFilter(r *http.Request) *types.ExecutionFilter { - return &types.ExecutionFilter{ - Page: ParsePage(r), - Size: ParseLimit(r), - } -} diff --git a/internal/api/request/secret.go b/internal/api/request/secret.go index d47b140a5..517ff642d 100644 --- a/internal/api/request/secret.go +++ b/internal/api/request/secret.go @@ -7,8 +7,6 @@ package request import ( "net/http" "net/url" - - "github.com/harness/gitness/types" ) const ( @@ -24,11 +22,3 @@ func GetSecretRefFromPath(r *http.Request) (string, error) { // paths are unescaped return url.PathUnescape(rawRef) } - -// ParseExecutionFilter extracts the execution filter from the url. -func ParseSecretFilter(r *http.Request) *types.SecretFilter { - return &types.SecretFilter{ - Page: ParsePage(r), - Size: ParseLimit(r), - } -} diff --git a/internal/api/request/util.go b/internal/api/request/util.go index 709253395..05fc3c81a 100644 --- a/internal/api/request/util.go +++ b/internal/api/request/util.go @@ -9,6 +9,7 @@ import ( "strconv" "github.com/harness/gitness/internal/api/usererror" + "github.com/harness/gitness/types" "github.com/harness/gitness/types/enum" "github.com/go-chi/chi" @@ -17,11 +18,10 @@ import ( const ( PathParamRemainder = "*" - QueryParamSort = "sort" - QueryParamOrder = "order" - QueryParamQuery = "query" - QueryParamCreatedBy = "created_by" + QueryParamSort = "sort" + QueryParamOrder = "order" + QueryParamQuery = "query" QueryParamState = "state" QueryParamKind = "kind" @@ -166,26 +166,6 @@ func GetRemainderFromPath(r *http.Request) (string, error) { return PathParamOrError(r, PathParamRemainder) } -// PipelineFilter stores pipeline query parameters. -type Pagination struct { - Page int `json:"page"` - Size int `json:"size"` - Query string `json:"query"` - Sort string `json:"sort"` - Order enum.Order `json:"order"` -} - -// ParseExecutionFilter extracts the execution filter from the url. -func ParseFilterFromRequest(r *http.Request) Pagination { - return Pagination{ - Page: ParsePage(r), - Size: ParseLimit(r), - Query: ParseQuery(r), - Sort: ParseSort(r), - Order: ParseOrder(r), - } -} - // ParseQuery extracts the query parameter from the url. func ParseQuery(r *http.Request) string { return r.URL.Query().Get(QueryParamQuery) @@ -224,3 +204,14 @@ func ParseOrder(r *http.Request) enum.Order { func ParseSort(r *http.Request) string { return r.URL.Query().Get(QueryParamSort) } + +// ParsePaginationFromRequest extracts the pagination info from the url. +func ParsePaginationFromRequest(r *http.Request) types.Pagination { + return types.Pagination{ + Page: ParsePage(r), + Size: ParseLimit(r), + Query: ParseQuery(r), + Sort: ParseSort(r), + Order: ParseOrder(r), + } +} diff --git a/internal/store/database.go b/internal/store/database.go index 87fe85cb9..3d066f3fa 100644 --- a/internal/store/database.go +++ b/internal/store/database.go @@ -449,17 +449,21 @@ type ( // Create creates a new pipeline in the datastore. Create(ctx context.Context, pipeline *types.Pipeline) error - // Update tries to update a pipeline in the datastore with optimistic locking. + // Update tries to update a pipeline in the datastore Update(ctx context.Context, pipeline *types.Pipeline) (*types.Pipeline, error) // List lists the pipelines present in a parent space ID in the datastore. - List(ctx context.Context, spaceID int64, filter *types.PipelineFilter) ([]types.Pipeline, error) + List(ctx context.Context, spaceID int64, pagination types.Pagination) ([]types.Pipeline, error) + + // UpdateOptLock updates the pipeline using the optimistic locking mechanism. + UpdateOptLock(ctx context.Context, pipeline *types.Pipeline, + mutateFn func(pipeline *types.Pipeline) error) (*types.Pipeline, error) // Delete deletes a pipeline ID from the datastore. Delete(ctx context.Context, id int64) error // Count the number of pipelines in a space matching the given filter. - Count(ctx context.Context, spaceID int64, filter *types.PipelineFilter) (int64, error) + Count(ctx context.Context, spaceID int64, filter types.Pagination) (int64, error) // DeleteByUID deletes a pipeline with a given UID in a space DeleteByUID(ctx context.Context, spaceID int64, uid string) error @@ -479,7 +483,11 @@ type ( Create(ctx context.Context, secret *types.Secret) error // Count the number of secrets in a space matching the given filter. - Count(ctx context.Context, spaceID int64, filter *types.SecretFilter) (int64, error) + Count(ctx context.Context, spaceID int64, pagination types.Pagination) (int64, error) + + // UpdateOptLock updates the secret using the optimistic locking mechanism. + UpdateOptLock(ctx context.Context, secret *types.Secret, + mutateFn func(secret *types.Secret) error) (*types.Secret, error) // Update tries to update a secret. Update(ctx context.Context, secret *types.Secret) (*types.Secret, error) @@ -491,7 +499,7 @@ type ( DeleteByUID(ctx context.Context, spaceID int64, uid string) error // List lists the secrets in a given space - List(ctx context.Context, spaceID int64, filter *types.SecretFilter) ([]types.Secret, error) + List(ctx context.Context, spaceID int64, filter types.Pagination) ([]types.Secret, error) } ExecutionStore interface { @@ -501,11 +509,15 @@ type ( // Create creates a new execution in the datastore. Create(ctx context.Context, execution *types.Execution) error - // Update tries to update an execution in the datastore with optimistic locking. + // Update tries to update an execution. Update(ctx context.Context, execution *types.Execution) (*types.Execution, error) + // UpdateOptLock updates the execution using the optimistic locking mechanism. + UpdateOptLock(ctx context.Context, exectuion *types.Execution, + mutateFn func(execution *types.Execution) error) (*types.Execution, error) + // List lists the executions for a given pipeline ID - List(ctx context.Context, pipelineID int64, filter *types.ExecutionFilter) ([]types.Execution, error) + List(ctx context.Context, pipelineID int64, pagination types.Pagination) ([]types.Execution, error) // Delete deletes an execution given a pipeline ID and an execution number Delete(ctx context.Context, pipelineID int64, num int64) error diff --git a/internal/store/database/execution.go b/internal/store/database/execution.go index 73e344beb..29c8f53bf 100644 --- a/internal/store/database/execution.go +++ b/internal/store/database/execution.go @@ -151,36 +151,36 @@ const ( executionUpdateStmt = ` UPDATE executions SET - execution_trigger = :execution_trigger, - execution_parent = :execution_parent, - execution_status = :execution_status, - execution_error = :execution_error, - execution_event = :execution_event, - execution_action = :execution_action, - execution_link = :execution_link, - execution_timestamp = :execution_timestamp, - execution_title = :execution_title, - execution_message = :execution_message, - execution_before = :execution_before, - execution_after = :execution_after, - execution_ref = :execution_ref, - execution_source_repo = :execution_source_repo, - execution_source = :execution_source, - execution_target = :execution_target, - execution_author = :execution_author, - execution_author_name = :execution_author_name, - execution_author_email = :execution_author_email, - execution_author_avatar = :execution_author_avatar, - execution_sender = :execution_sender, - execution_params = :execution_params, - execution_cron = :execution_cron, - execution_deploy = :execution_deploy, - execution_deploy_id = :execution_deploy_id, - execution_debug = :execution_debug, - execution_started = :execution_started, - execution_finished = :execution_finished, - execution_updated = :execution_updated, - execution_version = :execution_version + execution_trigger = :execution_trigger + ,execution_parent = :execution_parent + ,execution_status = :execution_status + ,execution_error = :execution_error + ,execution_event = :execution_event + ,execution_action = :execution_action + ,execution_link = :execution_link + ,execution_timestamp = :execution_timestamp + ,execution_title = :execution_title + ,execution_message = :execution_message + ,execution_before = :execution_before + ,execution_after = :execution_after + ,execution_ref = :execution_ref + ,execution_source_repo = :execution_source_repo + ,execution_source = :execution_source + ,execution_target = :execution_target + ,execution_author = :execution_author + ,execution_author_name = :execution_author_name + ,execution_author_email = :execution_author_email + ,execution_author_avatar = :execution_author_avatar + ,execution_sender = :execution_sender + ,execution_params = :execution_params + ,execution_cron = :execution_cron + ,execution_deploy = :execution_deploy + ,execution_deploy_id = :execution_deploy_id + ,execution_debug = :execution_debug + ,execution_started = :execution_started + ,execution_finished = :execution_finished + ,execution_updated = :execution_updated + ,execution_version = :execution_version WHERE execution_id = :execution_id AND execution_version = :execution_version - 1` ) @@ -244,19 +244,50 @@ func (s *executionStore) Update(ctx context.Context, execution *types.Execution) return execution, nil } +// UpdateOptLock updates the pipeline using the optimistic locking mechanism. +func (s *executionStore) UpdateOptLock(ctx context.Context, + execution *types.Execution, + mutateFn func(execution *types.Execution) error) (*types.Execution, error) { + for { + dup := *execution + + fmt.Println(dup.Status) + + err := mutateFn(&dup) + if err != nil { + return nil, err + } + + fmt.Println("dup.Status after: ", dup.Status) + + execution, err = s.Update(ctx, &dup) + if err == nil { + return &dup, nil + } + if !errors.Is(err, gitness_store.ErrVersionConflict) { + return nil, err + } + + execution, err = s.Find(ctx, execution.PipelineID, execution.Number) + if err != nil { + return nil, err + } + } +} + // List lists the executions for a given pipeline ID. func (s *executionStore) List( ctx context.Context, pipelineID int64, - opts *types.ExecutionFilter, + pagination types.Pagination, ) ([]types.Execution, error) { stmt := database.Builder. Select(executionColumns). From("executions"). Where("execution_pipeline_id = ?", fmt.Sprint(pipelineID)) - stmt = stmt.Limit(database.Limit(opts.Size)) - stmt = stmt.Offset(database.Offset(opts.Page, opts.Size)) + stmt = stmt.Limit(database.Limit(pagination.Size)) + stmt = stmt.Offset(database.Offset(pagination.Page, pagination.Size)) sql, args, err := stmt.ToSql() if err != nil { diff --git a/internal/store/database/pipeline.go b/internal/store/database/pipeline.go index 1e36f9615..a09320fdb 100644 --- a/internal/store/database/pipeline.go +++ b/internal/store/database/pipeline.go @@ -178,19 +178,19 @@ func (s *pipelineStore) Update(ctx context.Context, pipeline *types.Pipeline) (* func (s *pipelineStore) List( ctx context.Context, parentID int64, - opts *types.PipelineFilter, + pagination types.Pagination, ) ([]types.Pipeline, error) { stmt := database.Builder. Select(pipelineColumns). From("pipelines"). Where("pipeline_space_id = ?", fmt.Sprint(parentID)) - if opts.Query != "" { - stmt = stmt.Where("LOWER(pipeline_uid) LIKE ?", fmt.Sprintf("%%%s%%", strings.ToLower(opts.Query))) + if pagination.Query != "" { + stmt = stmt.Where("LOWER(pipeline_uid) LIKE ?", fmt.Sprintf("%%%s%%", strings.ToLower(pagination.Query))) } - stmt = stmt.Limit(database.Limit(opts.Size)) - stmt = stmt.Offset(database.Offset(opts.Page, opts.Size)) + stmt = stmt.Limit(database.Limit(pagination.Size)) + stmt = stmt.Offset(database.Offset(pagination.Page, pagination.Size)) sql, args, err := stmt.ToSql() if err != nil { @@ -207,15 +207,42 @@ func (s *pipelineStore) List( return dst, nil } +// UpdateOptLock updates the pipeline using the optimistic locking mechanism. +func (s *pipelineStore) UpdateOptLock(ctx context.Context, + pipeline *types.Pipeline, + mutateFn func(pipeline *types.Pipeline) error) (*types.Pipeline, error) { + for { + dup := *pipeline + + err := mutateFn(&dup) + if err != nil { + return nil, err + } + + pipeline, err = s.Update(ctx, &dup) + if err == nil { + return &dup, nil + } + if !errors.Is(err, gitness_store.ErrVersionConflict) { + return nil, err + } + + pipeline, err = s.Find(ctx, pipeline.ID) + if err != nil { + return nil, err + } + } +} + // Count of pipelines in a space. -func (s *pipelineStore) Count(ctx context.Context, parentID int64, opts *types.PipelineFilter) (int64, error) { +func (s *pipelineStore) Count(ctx context.Context, parentID int64, filter types.Pagination) (int64, error) { stmt := database.Builder. Select("count(*)"). From("pipelines"). Where("pipeline_space_id = ?", parentID) - if opts.Query != "" { - stmt = stmt.Where("pipeline_uid LIKE ?", fmt.Sprintf("%%%s%%", opts.Query)) + if filter.Query != "" { + stmt = stmt.Where("pipeline_uid LIKE ?", fmt.Sprintf("%%%s%%", filter.Query)) } sql, args, err := stmt.ToSql() diff --git a/internal/store/database/secret.go b/internal/store/database/secret.go index a53a10db7..2ce4f5bf9 100644 --- a/internal/store/database/secret.go +++ b/internal/store/database/secret.go @@ -165,19 +165,46 @@ func (s *secretStore) Update(ctx context.Context, secret *types.Secret) (*types. return secret, nil } +// UpdateOptLock updates the pipeline using the optimistic locking mechanism. +func (s *secretStore) UpdateOptLock(ctx context.Context, + secret *types.Secret, + mutateFn func(secret *types.Secret) error) (*types.Secret, error) { + for { + dup := *secret + + err := mutateFn(&dup) + if err != nil { + return nil, err + } + + secret, err = s.Update(ctx, &dup) + if err == nil { + return &dup, nil + } + if !errors.Is(err, gitness_store.ErrVersionConflict) { + return nil, err + } + + secret, err = s.Find(ctx, secret.ID) + if err != nil { + return nil, err + } + } +} + // List lists all the secrets present in a space. -func (s *secretStore) List(ctx context.Context, parentID int64, opts *types.SecretFilter) ([]types.Secret, error) { +func (s *secretStore) List(ctx context.Context, parentID int64, pagination types.Pagination) ([]types.Secret, error) { stmt := database.Builder. Select(secretColumns). From("secrets"). Where("secret_space_id = ?", fmt.Sprint(parentID)) - if opts.Query != "" { - stmt = stmt.Where("LOWER(secret_uid) LIKE ?", fmt.Sprintf("%%%s%%", strings.ToLower(opts.Query))) + if pagination.Query != "" { + stmt = stmt.Where("LOWER(secret_uid) LIKE ?", fmt.Sprintf("%%%s%%", strings.ToLower(pagination.Query))) } - stmt = stmt.Limit(database.Limit(opts.Size)) - stmt = stmt.Offset(database.Offset(opts.Page, opts.Size)) + stmt = stmt.Limit(database.Limit(pagination.Size)) + stmt = stmt.Offset(database.Offset(pagination.Page, pagination.Size)) sql, args, err := stmt.ToSql() if err != nil { @@ -225,14 +252,14 @@ func (s *secretStore) DeleteByUID(ctx context.Context, spaceID int64, uid string } // Count of secrets in a space. -func (s *secretStore) Count(ctx context.Context, parentID int64, opts *types.SecretFilter) (int64, error) { +func (s *secretStore) Count(ctx context.Context, parentID int64, filter types.Pagination) (int64, error) { stmt := database.Builder. Select("count(*)"). From("secrets"). Where("secret_space_id = ?", parentID) - if opts.Query != "" { - stmt = stmt.Where("secret_uid LIKE ?", fmt.Sprintf("%%%s%%", opts.Query)) + if filter.Query != "" { + stmt = stmt.Where("secret_uid LIKE ?", fmt.Sprintf("%%%s%%", filter.Query)) } sql, args, err := stmt.ToSql() @@ -252,6 +279,9 @@ func (s *secretStore) Count(ctx context.Context, parentID int64, opts *types.Sec // helper function returns the same secret with encrypted data. func enc(encrypt encrypt.Encrypter, secret *types.Secret) (*types.Secret, error) { + if secret == nil { + return nil, fmt.Errorf("cannot encrypt a nil secret") + } s := *secret ciphertext, err := encrypt.Encrypt(secret.Data) if err != nil { @@ -263,6 +293,9 @@ func enc(encrypt encrypt.Encrypter, secret *types.Secret) (*types.Secret, error) // helper function returns the same secret with decrypted data. func dec(encrypt encrypt.Encrypter, secret *types.Secret) (*types.Secret, error) { + if secret == nil { + return nil, fmt.Errorf("cannot decrypt a nil secret") + } s := *secret plaintext, err := encrypt.Decrypt([]byte(secret.Data)) if err != nil { diff --git a/mocks/mock_client.go b/mocks/mock_client.go index 3cb746055..b16a6490e 100644 --- a/mocks/mock_client.go +++ b/mocks/mock_client.go @@ -8,10 +8,9 @@ import ( context "context" reflect "reflect" + gomock "github.com/golang/mock/gomock" user "github.com/harness/gitness/internal/api/controller/user" types "github.com/harness/gitness/types" - - gomock "github.com/golang/mock/gomock" ) // MockClient is a mock of Client interface. diff --git a/mocks/mock_store.go b/mocks/mock_store.go index 9310f3729..0af0bbc34 100644 --- a/mocks/mock_store.go +++ b/mocks/mock_store.go @@ -8,10 +8,9 @@ import ( context "context" reflect "reflect" + gomock "github.com/golang/mock/gomock" types "github.com/harness/gitness/types" enum "github.com/harness/gitness/types/enum" - - gomock "github.com/golang/mock/gomock" ) // MockPrincipalStore is a mock of PrincipalStore interface. diff --git a/types/execution.go b/types/execution.go index fde55504e..a66ff6680 100644 --- a/types/execution.go +++ b/types/execution.go @@ -44,9 +44,3 @@ type Execution struct { // TODO: (Vistaar) Add stages // Stages []*Stage `db:"-" json:"stages,omitempty"` } - -// ExecutionFilter stores execution query parameters. -type ExecutionFilter struct { - Page int `json:"page"` - Size int `json:"size"` -} diff --git a/types/pagination.go b/types/pagination.go new file mode 100644 index 000000000..28bd01971 --- /dev/null +++ b/types/pagination.go @@ -0,0 +1,12 @@ +package types + +import "github.com/harness/gitness/types/enum" + +// Pagination stores pagination related params +type Pagination struct { + Page int `json:"page"` + Size int `json:"size"` + Query string `json:"query"` + Sort string `json:"sort"` + Order enum.Order `json:"order"` +} diff --git a/types/pipeline.go b/types/pipeline.go index b368dd5de..019bf99a1 100644 --- a/types/pipeline.go +++ b/types/pipeline.go @@ -21,10 +21,3 @@ type Pipeline struct { Updated int64 `db:"pipeline_updated" json:"updated"` Version int64 `db:"pipeline_version" json:"version"` } - -// PipelineFilter stores pipeline query parameters. -type PipelineFilter struct { - Page int `json:"page"` - Size int `json:"size"` - Query string `json:"query"` -} diff --git a/types/secret.go b/types/secret.go index 6b20e7b5e..f4bfa5b88 100644 --- a/types/secret.go +++ b/types/secret.go @@ -15,13 +15,6 @@ type Secret struct { Version int64 `db:"secret_version" json:"version"` } -// SecretFilter stores secret query parameters. -type SecretFilter struct { - Page int `json:"page"` - Size int `json:"size"` - Query string `json:"query"` -} - // Copy makes a copy of the secret without the value. func (s *Secret) CopyWithoutData() *Secret { return &Secret{