From 6af837dc87c791f2406dc91875e7a482766920d8 Mon Sep 17 00:00:00 2001 From: Mike Zorn Date: Fri, 9 Aug 2024 13:37:00 -0700 Subject: [PATCH 1/4] Move error to its own file --- internal/dev_server/model/error.go | 23 +++++++++++++++++++++++ internal/dev_server/model/store.go | 21 +-------------------- 2 files changed, 24 insertions(+), 20 deletions(-) create mode 100644 internal/dev_server/model/error.go diff --git a/internal/dev_server/model/error.go b/internal/dev_server/model/error.go new file mode 100644 index 00000000..55d6d463 --- /dev/null +++ b/internal/dev_server/model/error.go @@ -0,0 +1,23 @@ +package model + +import "github.com/pkg/errors" + +type Error struct { + err error + message string +} + +func (e Error) Error() string { + return e.message +} + +func (e Error) Unwrap() error { + return e.err +} + +func NewError(message string) error { + return errors.WithStack(Error{ + err: errors.New(message), + message: message, + }) +} diff --git a/internal/dev_server/model/store.go b/internal/dev_server/model/store.go index a1b644e5..e4c9f565 100644 --- a/internal/dev_server/model/store.go +++ b/internal/dev_server/model/store.go @@ -46,23 +46,4 @@ func StoreMiddleware(store Store) mux.MiddlewareFunc { } var ErrNotFound = errors.New("not found") - -type Error struct { - err error - message string -} - -func (e Error) Error() string { - return e.message -} - -func (e Error) Unwrap() error { - return e.err -} - -func NewError(message string) error { - return errors.WithStack(Error{ - err: errors.New(message), - message: message, - }) -} +var ErrAlreadyExists = errors.New("already exists") From e57288467f69f81fb6ac5cd3c0bea0b90fa6c76d Mon Sep 17 00:00:00 2001 From: Mike Zorn Date: Fri, 9 Aug 2024 13:58:18 -0700 Subject: [PATCH 2/4] DRY up error shapes and add 409 for project conflicts --- internal/dev_server/api/api.yaml | 28 ++++------- internal/dev_server/api/server.gen.go | 68 +++++++++++++-------------- internal/dev_server/api/server.go | 6 +-- 3 files changed, 45 insertions(+), 57 deletions(-) diff --git a/internal/dev_server/api/api.yaml b/internal/dev_server/api/api.yaml index 7a9724c5..9dc4a62f 100644 --- a/internal/dev_server/api/api.yaml +++ b/internal/dev_server/api/api.yaml @@ -74,7 +74,7 @@ paths: 204: description: OK. Project & overrides were removed 404: - $ref: "#/components/responses/NotFoundErrorResp" + $ref: "#/components/responses/ErrorResponse" post: summary: Add the project to the dev server parameters: @@ -97,7 +97,9 @@ paths: 201: $ref: "#/components/responses/Project" 400: - $ref: "#/components/responses/InvalidRequestResponse" + $ref: "#/components/responses/ErrorResponse" + 409: + $ref: "#/components/responses/ErrorResponse" /dev/projects/{projectKey}/overrides/{flagKey}: put: summary: override flag value with value provided in the body @@ -115,7 +117,7 @@ paths: 200: $ref: "#/components/responses/FlagOverride" 400: - $ref: "#/components/responses/InvalidRequestResponse" + $ref: "#/components/responses/ErrorResponse" delete: summary: remove override for flag @@ -222,8 +224,8 @@ components: application/json: schema: $ref: "#/components/schemas/Project" - InvalidRequestResponse: - description: Invalid request response + ErrorResponse: + description: Error response object content: application/json: schema: @@ -237,18 +239,4 @@ components: description: specific error code encountered message: type: string - description: description of the error - NotFoundErrorResp: - description: not found - content: - application/json: - schema: - type: object - required: - - code - - message - properties: - code: - type: string - message: - type: string \ No newline at end of file + description: description of the error \ No newline at end of file diff --git a/internal/dev_server/api/server.gen.go b/internal/dev_server/api/server.gen.go index e869b9aa..5d522be1 100644 --- a/internal/dev_server/api/server.gen.go +++ b/internal/dev_server/api/server.gen.go @@ -70,17 +70,8 @@ type FlagKey = string // ProjectKey defines model for projectKey. type ProjectKey = string -// FlagOverride defines model for FlagOverride. -type FlagOverride struct { - // Override whether or not this is an overridden value or one from the source environment - Override bool `json:"override"` - - // Value value of a feature flag variation - Value FlagValue `json:"value"` -} - -// InvalidRequestResponse defines model for InvalidRequestResponse. -type InvalidRequestResponse struct { +// ErrorResponse defines model for ErrorResponse. +type ErrorResponse struct { // Code specific error code encountered Code string `json:"code"` @@ -88,10 +79,13 @@ type InvalidRequestResponse struct { Message string `json:"message"` } -// NotFoundErrorResp defines model for NotFoundErrorResp. -type NotFoundErrorResp struct { - Code string `json:"code"` - Message string `json:"message"` +// FlagOverride defines model for FlagOverride. +type FlagOverride struct { + // Override whether or not this is an overridden value or one from the source environment + Override bool `json:"override"` + + // Value value of a feature flag variation + Value FlagValue `json:"value"` } // GetDevProjectsProjectKeyParams defines parameters for GetDevProjectsProjectKey. @@ -585,15 +579,7 @@ func HandlerWithOptions(si ServerInterface, options GorillaServerOptions) http.H return r } -type FlagOverrideJSONResponse struct { - // Override whether or not this is an overridden value or one from the source environment - Override bool `json:"override"` - - // Value value of a feature flag variation - Value FlagValue `json:"value"` -} - -type InvalidRequestResponseJSONResponse struct { +type ErrorResponseJSONResponse struct { // Code specific error code encountered Code string `json:"code"` @@ -601,9 +587,12 @@ type InvalidRequestResponseJSONResponse struct { Message string `json:"message"` } -type NotFoundErrorRespJSONResponse struct { - Code string `json:"code"` - Message string `json:"message"` +type FlagOverrideJSONResponse struct { + // Override whether or not this is an overridden value or one from the source environment + Override bool `json:"override"` + + // Value value of a feature flag variation + Value FlagValue `json:"value"` } type ProjectJSONResponse Project @@ -640,7 +629,7 @@ func (response DeleteDevProjectsProjectKey204Response) VisitDeleteDevProjectsPro return nil } -type DeleteDevProjectsProjectKey404JSONResponse struct{ NotFoundErrorRespJSONResponse } +type DeleteDevProjectsProjectKey404JSONResponse struct{ ErrorResponseJSONResponse } func (response DeleteDevProjectsProjectKey404JSONResponse) VisitDeleteDevProjectsProjectKeyResponse(w http.ResponseWriter) error { w.Header().Set("Content-Type", "application/json") @@ -721,9 +710,7 @@ func (response PostDevProjectsProjectKey201JSONResponse) VisitPostDevProjectsPro return json.NewEncoder(w).Encode(response) } -type PostDevProjectsProjectKey400JSONResponse struct { - InvalidRequestResponseJSONResponse -} +type PostDevProjectsProjectKey400JSONResponse struct{ ErrorResponseJSONResponse } func (response PostDevProjectsProjectKey400JSONResponse) VisitPostDevProjectsProjectKeyResponse(w http.ResponseWriter) error { w.Header().Set("Content-Type", "application/json") @@ -732,6 +719,21 @@ func (response PostDevProjectsProjectKey400JSONResponse) VisitPostDevProjectsPro return json.NewEncoder(w).Encode(response) } +type PostDevProjectsProjectKey409JSONResponse struct { + // Code specific error code encountered + Code string `json:"code"` + + // Message description of the error + Message string `json:"message"` +} + +func (response PostDevProjectsProjectKey409JSONResponse) VisitPostDevProjectsProjectKeyResponse(w http.ResponseWriter) error { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(409) + + return json.NewEncoder(w).Encode(response) +} + type DeleteDevProjectsProjectKeyOverridesFlagKeyRequestObject struct { ProjectKey ProjectKey `json:"projectKey"` FlagKey FlagKey `json:"flagKey"` @@ -776,9 +778,7 @@ func (response PutDevProjectsProjectKeyOverridesFlagKey200JSONResponse) VisitPut return json.NewEncoder(w).Encode(response) } -type PutDevProjectsProjectKeyOverridesFlagKey400JSONResponse struct { - InvalidRequestResponseJSONResponse -} +type PutDevProjectsProjectKeyOverridesFlagKey400JSONResponse struct{ ErrorResponseJSONResponse } func (response PutDevProjectsProjectKeyOverridesFlagKey400JSONResponse) VisitPutDevProjectsProjectKeyOverridesFlagKeyResponse(w http.ResponseWriter) error { w.Header().Set("Content-Type", "application/json") diff --git a/internal/dev_server/api/server.go b/internal/dev_server/api/server.go index 66862537..f60c146b 100644 --- a/internal/dev_server/api/server.go +++ b/internal/dev_server/api/server.go @@ -34,7 +34,7 @@ func (s Server) DeleteDevProjectsProjectKey(ctx context.Context, request DeleteD return nil, err } if !deleted { - return DeleteDevProjectsProjectKey404JSONResponse{NotFoundErrorRespJSONResponse{ + return DeleteDevProjectsProjectKey404JSONResponse{ErrorResponseJSONResponse{ Code: "not_found", Message: "project not found", }}, nil @@ -90,7 +90,7 @@ func (s Server) GetDevProjectsProjectKey(ctx context.Context, request GetDevProj func (s Server) PostDevProjectsProjectKey(ctx context.Context, request PostDevProjectsProjectKeyRequestObject) (PostDevProjectsProjectKeyResponseObject, error) { if request.Body.SourceEnvironmentKey == "" { return PostDevProjectsProjectKey400JSONResponse{ - InvalidRequestResponseJSONResponse{ + ErrorResponseJSONResponse{ Code: "invalid_request", Message: "sourceEnvironmentKey is required", }, @@ -248,7 +248,7 @@ func (s Server) PutDevProjectsProjectKeyOverridesFlagKey(ctx context.Context, re if err != nil { if errors.As(err, &model.Error{}) { return PutDevProjectsProjectKeyOverridesFlagKey400JSONResponse{ - InvalidRequestResponseJSONResponse{ + ErrorResponseJSONResponse{ Code: "invalid_request", Message: err.Error(), }, From 3e89370eb93b4fd466a34eb72fbb194b442caeec Mon Sep 17 00:00:00 2001 From: Mike Zorn Date: Fri, 9 Aug 2024 14:21:24 -0700 Subject: [PATCH 3/4] Return already exists error from store layer --- internal/dev_server/db/sqlite.go | 34 ++++++++++++++++++++++++--- internal/dev_server/db/sqlite_test.go | 5 ++++ internal/dev_server/model/store.go | 1 + 3 files changed, 37 insertions(+), 3 deletions(-) diff --git a/internal/dev_server/db/sqlite.go b/internal/dev_server/db/sqlite.go index 73ecc34b..523b4b9f 100644 --- a/internal/dev_server/db/sqlite.go +++ b/internal/dev_server/db/sqlite.go @@ -106,12 +106,36 @@ func (s Sqlite) DeleteDevProject(ctx context.Context, key string) (bool, error) return true, nil } -func (s Sqlite) InsertProject(ctx context.Context, project model.Project) error { +func (s Sqlite) InsertProject(ctx context.Context, project model.Project) (err error) { flagsStateJson, err := json.Marshal(project.AllFlagsState) if err != nil { return errors.Wrap(err, "unable to marshal flags state when writing project") } - _, err = s.database.Exec(` + tx, err := s.database.BeginTx(ctx, nil) + if err != nil { + return + } + defer func() { + if err != nil { + _ = tx.Rollback() + } + }() + + projects, err := tx.QueryContext(ctx, ` +SELECT 1 FROM projects WHERE key = ? +`, project.Key) + if err != nil { + return + } + if projects.Next() { + err = model.ErrAlreadyExists + return + } + err = projects.Close() + if err != nil { + return + } + _, err = tx.Exec(` INSERT INTO projects (key, source_environment_key, context, last_sync_time, flag_state) VALUES (?, ?, ?, ?, ?) `, @@ -121,7 +145,11 @@ VALUES (?, ?, ?, ?, ?) project.LastSyncTime, string(flagsStateJson), ) - return err + if err != nil { + return + } + err = tx.Commit() + return } func (s Sqlite) GetOverridesForProject(ctx context.Context, projectKey string) (model.Overrides, error) { diff --git a/internal/dev_server/db/sqlite_test.go b/internal/dev_server/db/sqlite_test.go index 7bd08264..acbed71f 100644 --- a/internal/dev_server/db/sqlite_test.go +++ b/internal/dev_server/db/sqlite_test.go @@ -58,6 +58,11 @@ func TestDBFunctions(t *testing.T) { actualProjectKeys[proj.Key] = true } + t.Run("InsertProject returns ErrAlreadyExists if the project already exists", func(t *testing.T) { + err := store.InsertProject(ctx, projects[0]) + assert.Equal(t, model.ErrAlreadyExists, err) + }) + t.Run("GetDevProjectKeys returns keys in projects", func(t *testing.T) { keys, err := store.GetDevProjectKeys(ctx) assert.NoError(t, err) diff --git a/internal/dev_server/model/store.go b/internal/dev_server/model/store.go index e4c9f565..2686a1bb 100644 --- a/internal/dev_server/model/store.go +++ b/internal/dev_server/model/store.go @@ -21,6 +21,7 @@ type Store interface { GetDevProject(ctx context.Context, projectKey string) (*Project, error) UpdateProject(ctx context.Context, project Project) (bool, error) DeleteDevProject(ctx context.Context, projectKey string) (bool, error) + // InsertProject inserts the project. If it already exists, ErrAlreadyExists is returned InsertProject(ctx context.Context, project Project) error UpsertOverride(ctx context.Context, override Override) (Override, error) GetOverridesForProject(ctx context.Context, projectKey string) (Overrides, error) From b1ca6938ac3abba4ea5bb2d2c0ea0a700bc2f4e9 Mon Sep 17 00:00:00 2001 From: Mike Zorn Date: Fri, 9 Aug 2024 14:34:19 -0700 Subject: [PATCH 4/4] Return a 409 if the project is present --- internal/dev_server/api/server.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/internal/dev_server/api/server.go b/internal/dev_server/api/server.go index f60c146b..7ae6bfcf 100644 --- a/internal/dev_server/api/server.go +++ b/internal/dev_server/api/server.go @@ -99,7 +99,13 @@ func (s Server) PostDevProjectsProjectKey(ctx context.Context, request PostDevPr store := model.StoreFromContext(ctx) project, err := model.CreateProject(ctx, request.ProjectKey, request.Body.SourceEnvironmentKey, request.Body.Context) - if err != nil { + switch { + case errors.Is(err, model.ErrAlreadyExists): + return PostDevProjectsProjectKey409JSONResponse{ + Code: "conflict", + Message: "project already exists", + }, nil + case err != nil: return nil, err }