Implement schema-first OpenAPI definition and validation #3
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/implement-openapi-schema-first"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Pull request overview
This PR shifts the API to a schema-first approach by introducing an OpenAPI 3 spec, generating Go models from it, and validating incoming requests via OpenAPI middleware. It also replaces the previous swagger annotation-based approach and introduces a built-in Swagger UI for interactive documentation.
Changes:
openapi-spec.yaml+oapi-codegenoutput (pkg/api) and wire request validation middleware into the server.api.*types and use explicit mapping helpers.AuthenticationFuncimplementation and serve Swagger UI + spec endpoints.Reviewed changes
Copilot reviewed 29 out of 39 changed files in this pull request and generated 12 comments.
Show a summary per file
/v1/*; uses generated request/response models + mapping helpers./v1/*; uses generated request/response models + mapping helpers./swagger/doc.json.{"message": ...}to match OpenAPI schema./v1/*; uses generated request/response models + mapping helpers.go:generateforoapi-codegenagainstopenapi-spec.yaml.oapi-codegenconfig: models + embedded spec.go tool sqlcgeneration (removes swag generation).subtosubject.pkg/docs(swag-based docs removed).💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
@ -0,0 +1,882 @@openapi: 3.0.3AttachFileToVersionRequest allows
fileIdminimum 0. The service/API expects positive IDs (gt=0), so the schema should useminimum: 1to prevent invalid requests from passing OpenAPI validation.DetachFileFromVersionRequest allows
fileIdminimum 0. The service/API expects positive IDs (gt=0), so the schema should useminimum: 1to match the previous validation rules.@ -0,0 +399,4 @@operationId: createUsersummary: Create a new usertags:- usersPOST /api/v1/users is missing a
securityrequirement. With the OAPI validator middleware, this makes user creation unauthenticated, which differs from prior behavior (e2e tests still send a Bearer token). Addsecurity: - OpenIdConnect: []if this must remain protected.@ -0,0 +424,4 @@operationId: getUserByIdsummary: Get a user by IDtags:- usersGET /api/v1/users/{userId} is missing a
securityrequirement, making it publicly accessible under the OAPI validator middleware. Addsecurity: - OpenIdConnect: []if user lookup should require authentication.@ -0,0 +492,4 @@schema:type: integerformat: int64example: 100QueryLimit schema doesn't express the 1..100 range enforced by ParsePagination (values outside are silently coerced to 100). Add
minimum: 1andmaximum: 100so invalid values are rejected at the OpenAPI validation layer.@ -0,0 +502,4 @@required: falseschema:type: integerformat: int64QueryOffset schema doesn't express the non-negative constraint enforced by ParsePagination (negative offsets are coerced to 0). Add
minimum: 0so invalid values are rejected by OpenAPI validation instead of silently coerced.@ -0,0 +724,4 @@projectId:type: integerformat: int64example: 1CreateVersionRequest does not constrain
projectIdto be > 0, but the previous request validation enforced gt=0. Addminimum: 1forprojectIdso schema validation matches the service expectations.@ -0,0 +859,4 @@type: objectrequired:- name- emailCreateUserRequest requires
emailVerified. Previously this could be omitted and default to false (Go bool zero value). If you want backward compatibility, remove it from the required list and set a schema default of false.@ -0,0 +95,4 @@token, err := jwt.ParseString(unverifiedToken, jwt.WithVerify(false))if errors.Is(err, jwt.TokenExpiredError()) {return errors.New("token expired"), TokenContext{}GetToken parses the JWT with signature verification disabled (jwt.WithVerify(false)). If this helper is ever called without a prior authenticated/validated request, forged tokens would be accepted and their claims trusted. Prefer verifying the token with JWKS here, or make the function explicitly unverified (naming/docs + keep usage tightly scoped).
@ -0,0 +19,4 @@w.WriteHeader(http.StatusOK)err := json.NewEncoder(w).Encode(openapi)if err != nil {log.Printf("failed to write swagger spec: %v", err)Swagger spec handlers write the status code before encoding and do not set a Content-Type. Consider setting Content-Type (application/json / application/yaml) and encoding first so you can still return a 500 on encoder failures instead of partially writing a 200 response.
@ -42,19 +38,12 @@ func (h *Handler) RegisterRoutes(r chi.Router) {})GetMe maps any auth.GetToken error to a 400 "invalid request payload" response. Missing/invalid Authorization should return an authentication error (typically 401), and the error message should reflect the token problem rather than the request body.
CreateUser responds with HTTP 200, but the OpenAPI spec for POST /api/v1/users declares a 201 response (and the other create handlers use StatusCreated). Align the handler status code with the contract (or update the spec if 200 is intended).
Issues
0 New issues
0 Accepted issues
Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code
See analysis details on SonarQube Cloud