Add Swagger for API endpoints #234
Conversation
WalkthroughThis change introduces Swagger/OpenAPI support to the project. It adds a new OpenAPI 3.0 specification file for the VisionBoard API, defines endpoints for health and report generation, and integrates Swagger validation middleware into the HTTP server. The server initialization logic is refactored to support Swagger, including request and response validation, Swagger UI, and logging of the Swagger docs URL. The configuration is updated to support a Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ExpressApp
participant SwaggerValidator
participant APIHandler
Client->>ExpressApp: HTTP Request to /api/v1/*
ExpressApp->>SwaggerValidator: Validate request/response (strict mode)
SwaggerValidator-->>ExpressApp: Pass/fail validation
ExpressApp->>APIHandler: Route to endpoint handler
APIHandler-->>ExpressApp: Response
ExpressApp-->>Client: HTTP Response
Assessment against linked issues
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
src/httpServer/swagger/api-v1.yml (2)
34-34: Remove trailing whitespace.There are trailing spaces on this line that should be removed.
- +🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 34-34: trailing spaces
(trailing-spaces)
93-93: Add newline at end of file.Add a newline character at the end of the file to comply with YAML best practices.
components: schemas: {} +🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 93-93: no new line character at the end of file
(new-line-at-end-of-file)
src/httpServer/index.js (3)
11-12: Pin the Swagger validator version to avoid unintended breaking changes
swagger-endpoint-validatoris imported without an explicit semver range. Because the library is still < 1.0 (and even minor bumps may contain breaking API changes), pullinglateston every CI run can introduce non-deterministic production builds.- "swagger-endpoint-validator": "*" + "swagger-endpoint-validator": "1.4.3" // example, replace with the version you’ve tested againstLocking the dependency (or using a caret for a known-good major version) keeps builds reproducible and shields you from surprise validator failures.
51-55:iconsis not a recognised option forserve-static
serve-staticsilently ignores unknown options; theiconsflag only exists inserve-index. Keeping it here may confuse future maintainers.- app.use('/assets', serveStatic(publicPath, { - index: false, - dotfiles: 'deny', - icons: true - })) + app.use('/assets', serveStatic(publicPath, { + index: false, + dotfiles: 'deny' + }))If directory listings with icons are desired, layer
serve-indexinstead.
58-64: PreventERR_HTTP_HEADERS_SENTwhen the error handler is invoked late
If an error bubbles up after the response headers were already flushed, writing a second status line will crash the process. Safeguard withres.headersSentand delegate to Express’ default handler.- app.use((err, req, res, next) => { - logger.error(`Server error: ${err.message}`, { stack: err.stack }) - res.status(500).json({ - error: 'Internal Server Error', - message: 'Check server logs for more details' - }) - }) + app.use((err, req, res, next) => { + logger.error(`Server error: ${err.message}`, { stack: err.stack }) + if (res.headersSent) return next(err) + res.status(500).json({ + error: 'Internal Server Error', + message: 'Check server logs for more details' + }) + })This avoids double-send crashes and still preserves the clean JSON error format.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
knexfile.js(1 hunks)package.json(2 hunks)playwright.config.js(1 hunks)src/httpServer/index.js(3 hunks)src/httpServer/swagger/api-v1.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
playwright.config.js (1)
Learnt from: UlisesGascon
PR: OpenPathfinder/visionBoard#233
File: playwright.config.js:59-59
Timestamp: 2025-05-03T07:11:12.696Z
Learning: In the visionBoard repository, the team has deliberately chosen to run Playwright tests without explicitly setting NODE_ENV=test in the webServer command, as part of a change that implements automatic database migrations on server start.
🧬 Code Graph Analysis (2)
knexfile.js (1)
src/config/index.js (1)
dbSettings(3-22)
src/httpServer/index.js (3)
server.js (2)
server(1-1)serverInstance(4-4)src/utils/index.js (1)
logger(16-35)src/config/index.js (1)
staticServer(28-31)
🪛 YAMLlint (1.35.1)
src/httpServer/swagger/api-v1.yml
[error] 34-34: trailing spaces
(trailing-spaces)
[error] 93-93: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (7)
knexfile.js (1)
6-7: LGTM! Good addition of a test environment configuration.Adding a dedicated test environment configuration is a good practice that helps separate test and development environments. This change ensures the database settings are consistent when the application runs in test mode.
package.json (2)
24-28: Good practice: Consistent NODE_ENV=test setting for all test scripts.Setting NODE_ENV=test for all Playwright test scripts ensures a consistent testing environment and aligns with the test configuration added in knexfile.js.
65-65: LGTM! Appropriate dependency addition for Swagger integration.The swagger-endpoint-validator package is correctly added to dependencies with a proper semver range (^4.1.0) to support the Swagger documentation and validation features.
playwright.config.js (1)
63-66: Potential inconsistency with prior team decision.This change sets NODE_ENV=test in the webServer configuration, which is consistent with the updated test scripts in package.json. However, a previous learning indicates the team deliberately chose not to set NODE_ENV=test in the webServer command in PR #233.
Can you clarify if this change intentionally overrides the previous decision to not set NODE_ENV in the webServer configuration? If this is intentional, consider documenting the reason for the change.
src/httpServer/swagger/api-v1.yml (3)
1-6: Well-structured OpenAPI specification with proper metadata.The OpenAPI 3.0 specification includes appropriate metadata with title, description, and version information for the VisionBoard API.
8-33: Good implementation of health check endpoint.The health check endpoint is well-defined with clear descriptions, appropriate tags, and a properly structured response schema that includes status and timestamp fields.
35-90: Well-structured report generation endpoint with proper response handling.The report generation endpoint is well-documented with appropriate descriptions, tags, and distinct response schemas for success (202) and failure (500) scenarios. The schemas properly define required fields and use appropriate data types and formats.
- Ignores the Swagger-UI assets in the validation - Add body-parsing middleware before Swagger & routers - Prevent `ERR_HTTP_HEADERS_SENT` when the error handler is invoked late - icons is not an option for `serve-static`
closes #232
Summary by CodeRabbit
New Features
Chores
Refactor