Open
Conversation
Add @inversifyjs/http-openapi-validation with: - Validate parameter decorator - OpenApiValidationPipe for v3.1 and v3.2 - Ajv-based schema validation with format support - Content-type resolution from request headers - Subpath exports for v3.1 (default) and v3.2
Add end-to-end integration tests for: - v3.1 body validation (valid/invalid requests) - v3.2 body validation (valid/invalid requests) - Content-type fallback (single content type) - Content-type ambiguity error (multiple types)
Add changesets for: - @inversifyjs/http-core (minor): new public exports - @inversifyjs/http-open-api (minor): getter + metadata exports - @inversifyjs/http-openapi-validation (minor): new package
- add validation-docs/openapi/ with introduction and API pages - add code examples for OpenApiValidationPipe and Validate - add integration test for OpenAPI validation code example - add http-open-api, http-openapi-validation devDeps to examples
🦋 Changeset detectedLatest commit: d5e1698 The changes in this PR will be included in the next version bump. This PR includes changesets to release 15 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds a new package Changes
Sequence DiagramsequenceDiagram
actor Client
participant Pipe as OpenApiValidationPipe
participant Core as http-core utils
participant OpenAPI as SwaggerUiProvider / OpenAPI Object
participant AJV as Ajv Validator
Client->>Pipe: execute(input, metadata)
Pipe->>Core: getControllerMethodParameterMetadataList(target, method)
Core-->>Pipe: parameter metadata
Pipe->>Core: getOwnReflectMetadata(OpenAPI validate marker)
Core-->>Pipe: validate enabled?
alt not enabled
Pipe-->>Client: return input
else enabled
Pipe->>OpenAPI: resolve openApiObject + operation/requestBody/content
OpenAPI-->>Pipe: schema JSON Pointer
Pipe->>AJV: ensure Ajv initialized & get validator for Pointer
AJV-->>Pipe: validator function
Pipe->>AJV: validate(input)
alt validation passes
Pipe-->>Client: return input
else validation fails
Pipe-->>Client: throw InversifyValidationError
end
end
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly Related PRs
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (4)
packages/framework/http/libraries/open-api/src/metadata/calculations/v3Dot1/getControllerOpenApiMetadata.ts (1)
6-13: Consider adding JSDoc documentation for this public API.This function is part of the package's public API surface. Adding JSDoc documentation would improve developer experience by providing inline documentation about the function's purpose, parameters, and return value.
📝 Suggested JSDoc
+/** + * Retrieves OpenAPI metadata for a controller. + * + * `@param` target - The controller class to retrieve metadata from + * `@returns` The controller's OpenAPI metadata if present, otherwise undefined + */ export function getControllerOpenApiMetadata( target: object, ): ControllerOpenApiMetadata | undefined {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/framework/http/libraries/open-api/src/metadata/calculations/v3Dot1/getControllerOpenApiMetadata.ts` around lines 6 - 13, Add JSDoc for the exported getControllerOpenApiMetadata function: document its purpose (retrieves controller OpenAPI metadata from the target via reflect metadata), describe the parameter `target: object` (the controller/class whose metadata is read), mention the return type `ControllerOpenApiMetadata | undefined` and what undefined means (no metadata found), and reference the internal helpers/keys used (`getOwnReflectMetadata`, `controllerOpenApiMetadataReflectKey`) for clarity; place the JSDoc immediately above the getControllerOpenApiMetadata declaration and keep it concise and consistent with other public API docs in the package.packages/framework/http/libraries/open-api/src/metadata/calculations/v3Dot2/getControllerOpenApiMetadata.ts (1)
6-13: Consider adding JSDoc documentation for this public API.This function is part of the package's public API surface. Adding JSDoc documentation would improve developer experience and maintain consistency with the v3Dot1 counterpart.
📝 Suggested JSDoc
+/** + * Retrieves OpenAPI metadata for a controller. + * + * `@param` target - The controller class to retrieve metadata from + * `@returns` The controller's OpenAPI metadata if present, otherwise undefined + */ export function getControllerOpenApiMetadata( target: object, ): ControllerOpenApiMetadata | undefined {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/framework/http/libraries/open-api/src/metadata/calculations/v3Dot2/getControllerOpenApiMetadata.ts` around lines 6 - 13, Add a JSDoc comment for the public function getControllerOpenApiMetadata describing its purpose (retrieves controller OpenAPI metadata via reflect metadata), document the parameter target (object) and the return type ControllerOpenApiMetadata | undefined, and mention that it uses getOwnReflectMetadata with controllerOpenApiMetadataReflectKey; follow the style and wording used in the v3Dot1 counterpart to keep consistency across versions.packages/framework/http/libraries/open-api/src/metadata/calculations/v3Dot1/getControllerOpenApiMetadata.spec.ts (1)
11-70: Align this spec with the repository’s required test structure/fixtures.The assertions are good, but the unit test shape should follow the required 4-layer
describehierarchy and fixture-class pattern instead of ad-hocletfixtures inbeforeAll.As per coding guidelines,
**/*.spec.tsmust use the four-layer describe structure (Class → Method → Input → Flow scopes), and**/*.{spec,int.spec}.tsshould use reusable fixture classes with static methods.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/framework/http/libraries/open-api/src/metadata/calculations/v3Dot1/getControllerOpenApiMetadata.spec.ts` around lines 11 - 70, Refactor the spec for getControllerOpenApiMetadata to follow the repository’s 4-layer describe hierarchy (Class → Method → Input → Flow) and replace ad-hoc let fixtures with a reusable fixture class that exposes static factory methods; specifically, wrap tests in nested describes for the target class, the getControllerOpenApiMetadata method, the input case (metadata present vs undefined), and the execution flow, and create a fixture class (e.g., ControllerFixture) with static methods to produce targetFixture and metadataFixture; update test bodies to call vitest.mocked(getOwnReflectMetadata).mockReturnValueOnce(...) and assert calls and returns against controllerOpenApiMetadataReflectKey as before, but using the new fixture class and describe structure to comply with the required pattern.packages/framework/http/libraries/openapi-validation/src/v3Dot2/pipes/OpenApiValidationPipe.ts (1)
35-233: Consider extracting shared logic between v3.1 and v3.2 implementations.The v3Dot1 and v3Dot2
OpenApiValidationPipeimplementations are nearly identical, differing only in import paths, type names, andSCHEMA_ID. A generic base class or shared utility functions could reduce duplication and simplify maintenance.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/framework/http/libraries/openapi-validation/src/v3Dot2/pipes/OpenApiValidationPipe.ts` around lines 35 - 233, The two implementations of OpenApiValidationPipe (v3Dot1 and v3Dot2) duplicate logic; extract the common behavior into a shared base class or utilities and have the version-specific classes only supply differences (types, import SCHEMA_ID, and content-type mapping). Concretely, create a BaseOpenApiValidationPipe that contains execute, `#getOrInitAjv`, and `#resolveContentType` logic using generic type parameters or abstract getters for the OpenAPI object type and SCHEMA_ID, then refactor the v3Dot2 OpenApiValidationPipe to extend BaseOpenApiValidationPipe and override/implement only the version-specific pieces (e.g., providing this.#openApiObject type, SCHEMA_ID constant, and any type aliases), doing the same for v3Dot1 so both implementations reuse the shared logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/config.json:
- Line 12: The package "@inversifyjs/http-openapi-validation" in the fixed group
has version 0.1.0 which mismatches the group's 3.0.0; open the package.json for
the `@inversifyjs/http-openapi-validation` package and change the "version" field
from "0.1.0" to "3.0.0" so it matches the other fixed-group packages referenced
in .changeset/config.json, then verify the package name
"@inversifyjs/http-openapi-validation" is unchanged and commit the updated
package.json.
In
`@packages/docs/tools/inversify-validation-code-examples/src/examples/v1/openApiValidation/apiIntroduction.int.spec.ts`:
- Around line 15-16: The afterAll teardown assumes server was set by beforeAll
and will throw if beforeAll failed early; update the afterAll blocks that
reference server (e.g., the server variable used alongside beforeAll/afterAll)
to guard the teardown by checking the server is truthy before calling its
close/shutdown method (e.g., if (server) await server.close()), and apply the
same guard to the other afterAll at the other occurrence (lines referenced
around 47-49) so teardown does not raise a secondary error when setup failed.
In
`@packages/framework/http/libraries/open-api/src/openApi/services/v3Dot1/SwaggerUiProvider.spec.ts`:
- Around line 71-87: The test suite for SwaggerUiProvider.openApiObject must
follow the Class → Method → Input → Flow describe structure and use the 'when
called, and [condition]' naming pattern; update the nested describes so the
outermost is SwaggerUiProvider, the next targets .openApiObject, then an
input-level describe (e.g., 'with optionsFixture'), and the flow-level describe
should be 'when called, and [condition]' (or simply 'when called, and no special
options' if nothing else), and keep the existing assertions using result and
optionsFixture.api.openApiObject; ensure the describe strings reference
SwaggerUiProvider and .openApiObject so the test name aligns with project
conventions.
In
`@packages/framework/http/libraries/open-api/src/openApi/services/v3Dot1/SwaggerUiProvider.ts`:
- Around line 70-72: The openApiObject getter currently returns
this.#options.api.openApiObject without verifying initialization; add a guard
using the provider's internal flag (this.#provided) to throw a clear error if
openApiObject is accessed before provide() has been called. Update the
SwaggerUiProvider class's openApiObject getter to check this.#provided and throw
a descriptive Error (e.g., "OpenAPI object not provided — call provide() first")
when false, otherwise return this.#options.api.openApiObject.
In
`@packages/framework/http/libraries/open-api/src/openApi/services/v3Dot2/SwaggerUiProvider.spec.ts`:
- Around line 71-87: The test for the SwaggerUiProvider.openApiObject getter
must follow the four-layer describe convention (Class → Method → Input → Flow);
update the spec so the top-level describe names the class (SwaggerUiProvider),
the next describes the 'openApiObject' getter, add an Input-level describe for
the fixture/input context (e.g. 'when options contains api.openApiObject'
referencing optionsFixture), and change the Flow-level describe to a descriptive
condition (e.g. 'when called and the option is present') so the it() assertion
remains the same (expect(result).toBe(optionsFixture.api.openApiObject)); keep
references to SwaggerUiProvider, openApiObject, and optionsFixture to locate the
code.
In `@packages/framework/http/libraries/openapi-validation/package.json`:
- Around line 26-27: The package.json's devDependencies entry for ajv
("devDependencies.ajv") is pinned to 8.17.1 which does not satisfy the declared
peerDependencies.ajv range (^8.18.0); update devDependencies.ajv to a version
that matches the peer range (e.g., ^8.18.0 or a specific 8.18.x/8.19.x release)
so local tests run against a supported AJV version and the peer constraint is
honored.
- Around line 12-14: The package.json pins two internal packages to fixed
versions; change the dependencies for "@inversifyjs/prototype-utils" and
"@inversifyjs/reflect-metadata-utils" to use the workspace protocol
(workspace:*) like "@inversifyjs/validation-common" so they reference the
monorepo packages; update the dependency values for the exact keys
"@inversifyjs/prototype-utils" and "@inversifyjs/reflect-metadata-utils" to
"workspace:*".
In
`@packages/framework/http/libraries/openapi-validation/src/v3Dot1/pipes/OpenApiValidationPipe.int.spec.ts`:
- Around line 285-297: The test description in OpenApiValidationPipe.int.spec.ts
is incorrect: the describe currently says "no Content-Type header" but the test
sends a 'Content-Type': 'application/json' header and actually exercises the
case where the pipe has no requestContentTypeProvider and falls back to the
OpenAPI single declared content type; update the describe string to accurately
reflect this (e.g., mention "requestContentTypeProvider undefined / falls back
to single declared content type" or "with single content type and Content-Type
header present") so the description matches the behavior exercised by the test
that uses the POST /users request and the response variable.
In
`@packages/framework/http/libraries/openapi-validation/src/v3Dot1/pipes/OpenApiValidationPipe.ts`:
- Around line 206-217: The content-type check in OpenApiValidationPipe (inside
the rawContentType !== undefined branch) compares the lowercased request media
type (baseContentType) against declaredContentTypes in their original casing
which can cause mismatches; normalize declaredContentTypes by mapping each entry
to a trimmed, lowercased form (e.g., normalizedDeclared =
declaredContentTypes.map(...) ) and use
normalizedDeclared.includes(baseContentType) for the comparison, then throw the
same InversifyValidationError (InversifyValidationErrorKind.validationFailed) if
not included and keep returning baseContentType.
In
`@packages/framework/http/libraries/openapi-validation/src/v3Dot2/pipes/OpenApiValidationPipe.int.spec.ts`:
- Around line 58-91: The promise returned for bootstrapping the Server currently
only accepts a resolve callback and throws inside the httpServer.listen
callback, which won't reject the outer Promise and lacks an 'error' listener for
listen/start failures; update the Promise signature to accept both resolve and
reject, replace the throw when address is null/string with reject(new
Error(...)), attach an httpServer.once('error', reject) before calling
httpServer.listen and remove that listener (or use httpServer.off) once the
listen callback succeeds so startup errors properly reject the outer Promise,
and ensure any listeners are cleaned up on successful resolve.
In
`@packages/framework/http/libraries/openapi-validation/src/v3Dot2/pipes/OpenApiValidationPipe.spec.ts`:
- Around line 30-49: The test claims "parameter metadata is undefined" but the
mock returns an empty array; change the mocked return of
getControllerMethodParameterMetadataList from [] to undefined
(vitest.mocked(getControllerMethodParameterMetadataList).mockReturnValueOnce(undefined))
so the OpenApiValidationPipe branch for undefined metadata is exercised; update
any related expectations/assertions in this spec (OpenApiValidationPipe.spec.ts)
to reflect the undefined-path behavior if needed.
In
`@packages/framework/http/libraries/openapi-validation/src/v3Dot2/pipes/OpenApiValidationPipe.ts`:
- Around line 206-217: OpenApiValidationPipe currently lowercases the
request-derived baseContentType but compares it against declaredContentTypes
which retain original casing, causing mismatches for differently cased media
types; update the check in the validation block (the code that computes
baseMediaType/baseContentType and uses declaredContentTypes.includes(...)) to
perform a case-insensitive comparison by normalizing declaredContentTypes (e.g.,
map to lowercase/trim) or by comparing baseContentType to each declared entry
lowercased, and keep the thrown InversifyValidationError behavior but use the
normalized values when matching.
---
Nitpick comments:
In
`@packages/framework/http/libraries/open-api/src/metadata/calculations/v3Dot1/getControllerOpenApiMetadata.spec.ts`:
- Around line 11-70: Refactor the spec for getControllerOpenApiMetadata to
follow the repository’s 4-layer describe hierarchy (Class → Method → Input →
Flow) and replace ad-hoc let fixtures with a reusable fixture class that exposes
static factory methods; specifically, wrap tests in nested describes for the
target class, the getControllerOpenApiMetadata method, the input case (metadata
present vs undefined), and the execution flow, and create a fixture class (e.g.,
ControllerFixture) with static methods to produce targetFixture and
metadataFixture; update test bodies to call
vitest.mocked(getOwnReflectMetadata).mockReturnValueOnce(...) and assert calls
and returns against controllerOpenApiMetadataReflectKey as before, but using the
new fixture class and describe structure to comply with the required pattern.
In
`@packages/framework/http/libraries/open-api/src/metadata/calculations/v3Dot1/getControllerOpenApiMetadata.ts`:
- Around line 6-13: Add JSDoc for the exported getControllerOpenApiMetadata
function: document its purpose (retrieves controller OpenAPI metadata from the
target via reflect metadata), describe the parameter `target: object` (the
controller/class whose metadata is read), mention the return type
`ControllerOpenApiMetadata | undefined` and what undefined means (no metadata
found), and reference the internal helpers/keys used (`getOwnReflectMetadata`,
`controllerOpenApiMetadataReflectKey`) for clarity; place the JSDoc immediately
above the getControllerOpenApiMetadata declaration and keep it concise and
consistent with other public API docs in the package.
In
`@packages/framework/http/libraries/open-api/src/metadata/calculations/v3Dot2/getControllerOpenApiMetadata.ts`:
- Around line 6-13: Add a JSDoc comment for the public function
getControllerOpenApiMetadata describing its purpose (retrieves controller
OpenAPI metadata via reflect metadata), document the parameter target (object)
and the return type ControllerOpenApiMetadata | undefined, and mention that it
uses getOwnReflectMetadata with controllerOpenApiMetadataReflectKey; follow the
style and wording used in the v3Dot1 counterpart to keep consistency across
versions.
In
`@packages/framework/http/libraries/openapi-validation/src/v3Dot2/pipes/OpenApiValidationPipe.ts`:
- Around line 35-233: The two implementations of OpenApiValidationPipe (v3Dot1
and v3Dot2) duplicate logic; extract the common behavior into a shared base
class or utilities and have the version-specific classes only supply differences
(types, import SCHEMA_ID, and content-type mapping). Concretely, create a
BaseOpenApiValidationPipe that contains execute, `#getOrInitAjv`, and
`#resolveContentType` logic using generic type parameters or abstract getters for
the OpenAPI object type and SCHEMA_ID, then refactor the v3Dot2
OpenApiValidationPipe to extend BaseOpenApiValidationPipe and override/implement
only the version-specific pieces (e.g., providing this.#openApiObject type,
SCHEMA_ID constant, and any type aliases), doing the same for v3Dot1 so both
implementations reuse the shared logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 729742ee-fad4-4e73-a0e5-414c85d0733f
📒 Files selected for processing (44)
.changeset/add-openapi-body-validation-http-core.md.changeset/add-openapi-body-validation-http-open-api.md.changeset/add-openapi-body-validation-new-package.md.changeset/config.jsonpackages/docs/services/inversify-framework-site/blog/2026-04-04-openapi-body-validation/index.mdxpackages/docs/services/inversify-framework-site/validation-docs/openapi/_category_.jsonpackages/docs/services/inversify-framework-site/validation-docs/openapi/api.mdxpackages/docs/services/inversify-framework-site/validation-docs/openapi/introduction.mdxpackages/docs/tools/inversify-validation-code-examples/package.jsonpackages/docs/tools/inversify-validation-code-examples/src/examples/v1/openApiValidation/apiIntroduction.int.spec.tspackages/docs/tools/inversify-validation-code-examples/src/examples/v1/openApiValidation/apiIntroduction.tspackages/docs/tools/inversify-validation-code-examples/src/examples/v1/openApiValidation/apiOpenApiValidationPipe.tspackages/docs/tools/inversify-validation-code-examples/src/examples/v1/openApiValidation/apiValidateDecorator.tspackages/framework/http/libraries/core/src/index.tspackages/framework/http/libraries/open-api/src/index.tspackages/framework/http/libraries/open-api/src/metadata/calculations/v3Dot1/getControllerOpenApiMetadata.spec.tspackages/framework/http/libraries/open-api/src/metadata/calculations/v3Dot1/getControllerOpenApiMetadata.tspackages/framework/http/libraries/open-api/src/metadata/calculations/v3Dot2/getControllerOpenApiMetadata.spec.tspackages/framework/http/libraries/open-api/src/metadata/calculations/v3Dot2/getControllerOpenApiMetadata.tspackages/framework/http/libraries/open-api/src/openApi/services/v3Dot1/SwaggerUiProvider.spec.tspackages/framework/http/libraries/open-api/src/openApi/services/v3Dot1/SwaggerUiProvider.tspackages/framework/http/libraries/open-api/src/openApi/services/v3Dot2/SwaggerUiProvider.spec.tspackages/framework/http/libraries/open-api/src/openApi/services/v3Dot2/SwaggerUiProvider.tspackages/framework/http/libraries/open-api/src/v3Dot2.tspackages/framework/http/libraries/openapi-validation/.gitignorepackages/framework/http/libraries/openapi-validation/.lintstagedrc.jsonpackages/framework/http/libraries/openapi-validation/eslint.config.mjspackages/framework/http/libraries/openapi-validation/package.jsonpackages/framework/http/libraries/openapi-validation/prettier.config.mjspackages/framework/http/libraries/openapi-validation/src/common/decorators/Validate.spec.tspackages/framework/http/libraries/openapi-validation/src/common/decorators/Validate.tspackages/framework/http/libraries/openapi-validation/src/common/reflectMetadata/openApiValidationMetadataReflectKey.tspackages/framework/http/libraries/openapi-validation/src/index.tspackages/framework/http/libraries/openapi-validation/src/v3Dot1/pipes/OpenApiValidationPipe.int.spec.tspackages/framework/http/libraries/openapi-validation/src/v3Dot1/pipes/OpenApiValidationPipe.spec.tspackages/framework/http/libraries/openapi-validation/src/v3Dot1/pipes/OpenApiValidationPipe.tspackages/framework/http/libraries/openapi-validation/src/v3Dot2.tspackages/framework/http/libraries/openapi-validation/src/v3Dot2/pipes/OpenApiValidationPipe.int.spec.tspackages/framework/http/libraries/openapi-validation/src/v3Dot2/pipes/OpenApiValidationPipe.spec.tspackages/framework/http/libraries/openapi-validation/src/v3Dot2/pipes/OpenApiValidationPipe.tspackages/framework/http/libraries/openapi-validation/stryker.config.mjspackages/framework/http/libraries/openapi-validation/tsconfig.esm.jsonpackages/framework/http/libraries/openapi-validation/tsconfig.jsonpackages/framework/http/libraries/openapi-validation/vitest.config.mjs
...rsify-validation-code-examples/src/examples/v1/openApiValidation/apiIntroduction.int.spec.ts
Show resolved
Hide resolved
...ages/framework/http/libraries/open-api/src/openApi/services/v3Dot1/SwaggerUiProvider.spec.ts
Show resolved
Hide resolved
Comment on lines
+70
to
+72
| public get openApiObject(): OpenApi3Dot1Object { | ||
| return this.#options.api.openApiObject; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether openApiObject is consumed before provide() in call sites.
# Expected: no cases where swaggerProvider.openApiObject is passed into validators before swaggerProvider.provide(container).
rg -n -C6 --type=ts --type=tsx 'swaggerProvider\.openApiObject|swaggerProvider\.provide\('Repository: inversify/monorepo
Length of output: 89
🏁 Script executed:
#!/bin/bash
# Re-run with correct rg syntax - rg doesn't support 'tsx' as separate type
rg -n -C6 'swaggerProvider\.openApiObject|swaggerProvider\.provide\(' --type tsRepository: inversify/monorepo
Length of output: 20225
🏁 Script executed:
#!/bin/bash
# Read the SwaggerUiProvider.ts file to understand the implementation
cat -n packages/framework/http/libraries/open-api/src/openApi/services/v3Dot1/SwaggerUiProvider.tsRepository: inversify/monorepo
Length of output: 10932
🏁 Script executed:
#!/bin/bash
# Search for openApiObject getter and provide() method in the file
rg -n -A5 -B5 'openApiObject|provide\(' packages/framework/http/libraries/open-api/src/openApi/services/v3Dot1/SwaggerUiProvider.ts --type tsRepository: inversify/monorepo
Length of output: 1006
Add a guard to prevent accessing the OpenAPI spec before initialization.
The openApiObject getter at line 70 lacks a check for the #provided state. While all current call sites properly invoke provide() before accessing the getter, adding a guard improves defensive programming and prevents misuse:
Proposed fix
public get openApiObject(): OpenApi3Dot1Object {
+ if (!this.#provided) {
+ throw new Error('Cannot access openApiObject before provide()');
+ }
+
return this.#options.api.openApiObject;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public get openApiObject(): OpenApi3Dot1Object { | |
| return this.#options.api.openApiObject; | |
| } | |
| public get openApiObject(): OpenApi3Dot1Object { | |
| if (!this.#provided) { | |
| throw new Error('Cannot access openApiObject before provide()'); | |
| } | |
| return this.#options.api.openApiObject; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/framework/http/libraries/open-api/src/openApi/services/v3Dot1/SwaggerUiProvider.ts`
around lines 70 - 72, The openApiObject getter currently returns
this.#options.api.openApiObject without verifying initialization; add a guard
using the provider's internal flag (this.#provided) to throw a clear error if
openApiObject is accessed before provide() has been called. Update the
SwaggerUiProvider class's openApiObject getter to check this.#provided and throw
a descriptive Error (e.g., "OpenAPI object not provided — call provide() first")
when false, otherwise return this.#options.api.openApiObject.
...ages/framework/http/libraries/open-api/src/openApi/services/v3Dot2/SwaggerUiProvider.spec.ts
Show resolved
Hide resolved
Comment on lines
+285
to
+297
| describe('when a valid POST /users request is made with single content type and no Content-Type header', () => { | ||
| let response: Response; | ||
|
|
||
| beforeAll(async () => { | ||
| response = await fetch( | ||
| `http://${server.host}:${server.port.toString()}/users`, | ||
| { | ||
| body: JSON.stringify({ name: 'Alice' }), | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| method: 'POST', | ||
| }, | ||
| ); | ||
| }); |
There was a problem hiding this comment.
Test description is misleading: request does include Content-Type header.
The describe block states "no Content-Type header" but line 293 sends 'Content-Type': 'application/json'. The actual scenario being tested is that the pipe has no requestContentTypeProvider and falls back to the single declared content type in the OpenAPI spec.
Consider updating the description to match the actual behavior:
Suggested description fix
- describe('when a valid POST /users request is made with single content type and no Content-Type header', () => {
+ describe('when a valid POST /users request is made with single declared content type and no content-type provider', () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/framework/http/libraries/openapi-validation/src/v3Dot1/pipes/OpenApiValidationPipe.int.spec.ts`
around lines 285 - 297, The test description in
OpenApiValidationPipe.int.spec.ts is incorrect: the describe currently says "no
Content-Type header" but the test sends a 'Content-Type': 'application/json'
header and actually exercises the case where the pipe has no
requestContentTypeProvider and falls back to the OpenAPI single declared content
type; update the describe string to accurately reflect this (e.g., mention
"requestContentTypeProvider undefined / falls back to single declared content
type" or "with single content type and Content-Type header present") so the
description matches the behavior exercised by the test that uses the POST /users
request and the response variable.
Comment on lines
+206
to
+217
| if (rawContentType !== undefined) { | ||
| const baseMediaType: string = rawContentType.split(';')[0] ?? ''; | ||
| const baseContentType: string = baseMediaType.trim().toLowerCase(); | ||
|
|
||
| if (!declaredContentTypes.includes(baseContentType)) { | ||
| throw new InversifyValidationError( | ||
| InversifyValidationErrorKind.validationFailed, | ||
| `Unsupported content type: ${baseContentType}. Supported: ${declaredContentTypes.join(', ')}`, | ||
| ); | ||
| } | ||
|
|
||
| return baseContentType; |
There was a problem hiding this comment.
Content-type matching may fail due to case sensitivity.
Same issue as in the v3Dot2 implementation: the case-insensitive comparison between the request's content type (lowercased) and declared content types (original case) may fail for specs with mixed-case media types.
Proposed fix for case-insensitive comparison
const rawContentType: string | undefined =
this.#requestContentTypeProvider?.();
if (rawContentType !== undefined) {
const baseMediaType: string = rawContentType.split(';')[0] ?? '';
const baseContentType: string = baseMediaType.trim().toLowerCase();
- if (!declaredContentTypes.includes(baseContentType)) {
+ const matchedContentType: string | undefined = declaredContentTypes.find(
+ (ct: string): boolean => ct.toLowerCase() === baseContentType,
+ );
+
+ if (matchedContentType === undefined) {
throw new InversifyValidationError(
InversifyValidationErrorKind.validationFailed,
`Unsupported content type: ${baseContentType}. Supported: ${declaredContentTypes.join(', ')}`,
);
}
- return baseContentType;
+ return matchedContentType;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (rawContentType !== undefined) { | |
| const baseMediaType: string = rawContentType.split(';')[0] ?? ''; | |
| const baseContentType: string = baseMediaType.trim().toLowerCase(); | |
| if (!declaredContentTypes.includes(baseContentType)) { | |
| throw new InversifyValidationError( | |
| InversifyValidationErrorKind.validationFailed, | |
| `Unsupported content type: ${baseContentType}. Supported: ${declaredContentTypes.join(', ')}`, | |
| ); | |
| } | |
| return baseContentType; | |
| if (rawContentType !== undefined) { | |
| const baseMediaType: string = rawContentType.split(';')[0] ?? ''; | |
| const baseContentType: string = baseMediaType.trim().toLowerCase(); | |
| const matchedContentType: string | undefined = declaredContentTypes.find( | |
| (ct: string): boolean => ct.toLowerCase() === baseContentType, | |
| ); | |
| if (matchedContentType === undefined) { | |
| throw new InversifyValidationError( | |
| InversifyValidationErrorKind.validationFailed, | |
| `Unsupported content type: ${baseContentType}. Supported: ${declaredContentTypes.join(', ')}`, | |
| ); | |
| } | |
| return matchedContentType; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/framework/http/libraries/openapi-validation/src/v3Dot1/pipes/OpenApiValidationPipe.ts`
around lines 206 - 217, The content-type check in OpenApiValidationPipe (inside
the rawContentType !== undefined branch) compares the lowercased request media
type (baseContentType) against declaredContentTypes in their original casing
which can cause mismatches; normalize declaredContentTypes by mapping each entry
to a trimmed, lowercased form (e.g., normalizedDeclared =
declaredContentTypes.map(...) ) and use
normalizedDeclared.includes(baseContentType) for the comparison, then throw the
same InversifyValidationError (InversifyValidationErrorKind.validationFailed) if
not included and keep returning baseContentType.
Comment on lines
+58
to
+91
| return new Promise<Server>( | ||
| (resolve: (value: Server | PromiseLike<Server>) => void) => { | ||
| httpServer.listen(0, '127.0.0.1', () => { | ||
| const address: AddressInfo | string | null = httpServer.address(); | ||
|
|
||
| if (address === null || typeof address === 'string') { | ||
| throw new Error('Failed to get server address'); | ||
| } | ||
|
|
||
| const server: Server = { | ||
| host: address.address, | ||
| port: address.port, | ||
| shutdown: async (): Promise<void> => { | ||
| await new Promise<void>( | ||
| ( | ||
| resolve: (value: void | PromiseLike<void>) => void, | ||
| reject: (reason?: unknown) => void, | ||
| ) => { | ||
| httpServer.close((error: Error | undefined) => { | ||
| if (error !== undefined) { | ||
| reject(error); | ||
| } else { | ||
| resolve(); | ||
| } | ||
| }); | ||
| }, | ||
| ); | ||
| }, | ||
| }; | ||
|
|
||
| resolve(server); | ||
| }); | ||
| }, | ||
| ); |
There was a problem hiding this comment.
Server bootstrap promise can hang or crash on startup failures
Line 63 throws inside an async listen callback, which does not safely reject the outer Promise, and there is no error listener for listen failures. This can cause flaky hangs/crashes in the integration suite.
Proposed fix
- return new Promise<Server>(
- (resolve: (value: Server | PromiseLike<Server>) => void) => {
- httpServer.listen(0, '127.0.0.1', () => {
+ return new Promise<Server>(
+ (
+ resolve: (value: Server | PromiseLike<Server>) => void,
+ reject: (reason?: unknown) => void,
+ ) => {
+ const onError = (error: Error): void => {
+ reject(error);
+ };
+
+ httpServer.once('error', onError);
+ httpServer.listen(0, '127.0.0.1', () => {
+ httpServer.off('error', onError);
const address: AddressInfo | string | null = httpServer.address();
if (address === null || typeof address === 'string') {
- throw new Error('Failed to get server address');
+ reject(new Error('Failed to get server address'));
+ return;
}
const server: Server = {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/framework/http/libraries/openapi-validation/src/v3Dot2/pipes/OpenApiValidationPipe.int.spec.ts`
around lines 58 - 91, The promise returned for bootstrapping the Server
currently only accepts a resolve callback and throws inside the
httpServer.listen callback, which won't reject the outer Promise and lacks an
'error' listener for listen/start failures; update the Promise signature to
accept both resolve and reject, replace the throw when address is null/string
with reject(new Error(...)), attach an httpServer.once('error', reject) before
calling httpServer.listen and remove that listener (or use httpServer.off) once
the listen callback succeeds so startup errors properly reject the outer
Promise, and ensure any listeners are cleaned up on successful resolve.
Comment on lines
+30
to
+49
| describe('when called, and parameter metadata is undefined', () => { | ||
| let pipe: OpenApiValidationPipe; | ||
| let inputFixture: unknown; | ||
| let metadataFixture: PipeMetadata; | ||
| let result: unknown; | ||
|
|
||
| beforeAll(() => { | ||
| inputFixture = { name: 'test' }; | ||
| metadataFixture = { | ||
| methodName: 'create', | ||
| parameterIndex: 0, | ||
| targetClass: class {} as NewableFunction, | ||
| }; | ||
|
|
||
| pipe = new OpenApiValidationPipe({} as OpenApi3Dot2Object); | ||
|
|
||
| vitest | ||
| .mocked(getControllerMethodParameterMetadataList) | ||
| .mockReturnValueOnce([]); | ||
|
|
There was a problem hiding this comment.
Test case doesn’t exercise the branch its title describes
Line 30 says the metadata is undefined, but Line 48 returns []. This scenario currently validates the empty-list path, not the undefined path.
Proposed fix
- vitest
- .mocked(getControllerMethodParameterMetadataList)
- .mockReturnValueOnce([]);
+ vitest
+ .mocked(getControllerMethodParameterMetadataList)
+ .mockReturnValueOnce(undefined);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| describe('when called, and parameter metadata is undefined', () => { | |
| let pipe: OpenApiValidationPipe; | |
| let inputFixture: unknown; | |
| let metadataFixture: PipeMetadata; | |
| let result: unknown; | |
| beforeAll(() => { | |
| inputFixture = { name: 'test' }; | |
| metadataFixture = { | |
| methodName: 'create', | |
| parameterIndex: 0, | |
| targetClass: class {} as NewableFunction, | |
| }; | |
| pipe = new OpenApiValidationPipe({} as OpenApi3Dot2Object); | |
| vitest | |
| .mocked(getControllerMethodParameterMetadataList) | |
| .mockReturnValueOnce([]); | |
| describe('when called, and parameter metadata is undefined', () => { | |
| let pipe: OpenApiValidationPipe; | |
| let inputFixture: unknown; | |
| let metadataFixture: PipeMetadata; | |
| let result: unknown; | |
| beforeAll(() => { | |
| inputFixture = { name: 'test' }; | |
| metadataFixture = { | |
| methodName: 'create', | |
| parameterIndex: 0, | |
| targetClass: class {} as NewableFunction, | |
| }; | |
| pipe = new OpenApiValidationPipe({} as OpenApi3Dot2Object); | |
| vitest | |
| .mocked(getControllerMethodParameterMetadataList) | |
| .mockReturnValueOnce(undefined); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/framework/http/libraries/openapi-validation/src/v3Dot2/pipes/OpenApiValidationPipe.spec.ts`
around lines 30 - 49, The test claims "parameter metadata is undefined" but the
mock returns an empty array; change the mocked return of
getControllerMethodParameterMetadataList from [] to undefined
(vitest.mocked(getControllerMethodParameterMetadataList).mockReturnValueOnce(undefined))
so the OpenApiValidationPipe branch for undefined metadata is exercised; update
any related expectations/assertions in this spec (OpenApiValidationPipe.spec.ts)
to reflect the undefined-path behavior if needed.
Comment on lines
+206
to
+217
| if (rawContentType !== undefined) { | ||
| const baseMediaType: string = rawContentType.split(';')[0] ?? ''; | ||
| const baseContentType: string = baseMediaType.trim().toLowerCase(); | ||
|
|
||
| if (!declaredContentTypes.includes(baseContentType)) { | ||
| throw new InversifyValidationError( | ||
| InversifyValidationErrorKind.validationFailed, | ||
| `Unsupported content type: ${baseContentType}. Supported: ${declaredContentTypes.join(', ')}`, | ||
| ); | ||
| } | ||
|
|
||
| return baseContentType; |
There was a problem hiding this comment.
Content-type matching may fail due to case sensitivity.
The baseContentType is lowercased (line 208), but declaredContentTypes preserves the original casing from the OpenAPI spec. Per RFC 7231, media types are case-insensitive, so 'application/json' and 'Application/JSON' should match.
Proposed fix for case-insensitive comparison
const rawContentType: string | undefined =
this.#requestContentTypeProvider?.();
if (rawContentType !== undefined) {
const baseMediaType: string = rawContentType.split(';')[0] ?? '';
const baseContentType: string = baseMediaType.trim().toLowerCase();
- if (!declaredContentTypes.includes(baseContentType)) {
+ const matchedContentType: string | undefined = declaredContentTypes.find(
+ (ct: string): boolean => ct.toLowerCase() === baseContentType,
+ );
+
+ if (matchedContentType === undefined) {
throw new InversifyValidationError(
InversifyValidationErrorKind.validationFailed,
`Unsupported content type: ${baseContentType}. Supported: ${declaredContentTypes.join(', ')}`,
);
}
- return baseContentType;
+ return matchedContentType;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/framework/http/libraries/openapi-validation/src/v3Dot2/pipes/OpenApiValidationPipe.ts`
around lines 206 - 217, OpenApiValidationPipe currently lowercases the
request-derived baseContentType but compares it against declaredContentTypes
which retain original casing, causing mismatches for differently cased media
types; update the check in the validation block (the code that computes
baseMediaType/baseContentType and uses declaredContentTypes.includes(...)) to
perform a case-insensitive comparison by normalizing declaredContentTypes (e.g.,
map to lowercase/trim) or by comparing baseContentType to each declared entry
lowercased, and keep the thrown InversifyValidationError behavior but use the
normalized values when matching.
Contributor
Benchmarks results:> root@ benchmarks:http:cjs:low /home/runner/work/monorepo/monorepo
> pnpm run --filter "@inversifyjs/http-benchmarks" benchmarks:run:cjs:low
> @inversifyjs/http-benchmarks@ benchmarks:run:cjs:low /home/runner/work/monorepo/monorepo/packages/framework/http/tools/http-benchmarks
> BUILD_TARGET=cjs VUS=128 ./bin/run-cjs.cjs
[CJS] Running benchmarks...
Express Basic Get Request
┌─────────┬───────────────────────────┬──────────────────┬──────────────────┬────────────────────────┬─────────┐
│ (index) │ Task name │ Latency avg (ms) │ Latency med (ms) │ Throughput avg (req/s) │ Samples │
├─────────┼───────────────────────────┼──────────────────┼──────────────────┼────────────────────────┼─────────┤
│ 0 │ 'currentInversifyExpress' │ '6.999' │ '6.506' │ '9010.848' │ 90217 │
│ 1 │ 'express' │ '6.529' │ '6.230' │ '9686.443' │ 96992 │
│ 2 │ 'NestJSExpress' │ '7.188' │ '7.020' │ '8809.173' │ 88213 │
└─────────┴───────────────────────────┴──────────────────┴──────────────────┴────────────────────────┴─────────┘
currentInversifyExpress vs express Speedup: 0.930x
currentInversifyExpress vs NestJSExpress Speedup: 1.023x
Express v4 Basic Get Request
┌─────────┬────────────────────────────┬──────────────────┬──────────────────┬────────────────────────┬─────────┐
│ (index) │ Task name │ Latency avg (ms) │ Latency med (ms) │ Throughput avg (req/s) │ Samples │
├─────────┼────────────────────────────┼──────────────────┼──────────────────┼────────────────────────┼─────────┤
│ 0 │ 'currentInversifyExpress4' │ '7.137' │ '6.825' │ '8869.882' │ 88822 │
│ 1 │ 'express4' │ '6.834' │ '6.474' │ '9258.860' │ 92699 │
└─────────┴────────────────────────────┴──────────────────┴──────────────────┴────────────────────────┴─────────┘
currentInversifyExpress4 vs express4 Speedup: 0.958x
Fastify Basic Get Request
┌─────────┬───────────────────────────┬──────────────────┬──────────────────┬────────────────────────┬─────────┐
│ (index) │ Task name │ Latency avg (ms) │ Latency med (ms) │ Throughput avg (req/s) │ Samples │
├─────────┼───────────────────────────┼──────────────────┼──────────────────┼────────────────────────┼─────────┤
│ 0 │ 'currentInversifyFastify' │ '4.136' │ '3.611' │ '15111.427' │ 151249 │
│ 1 │ 'fastify' │ '3.774' │ '3.315' │ '16494.594' │ 165114 │
│ 2 │ 'NestJSFastify' │ '4.295' │ '3.787' │ '14469.561' │ 144820 │
└─────────┴───────────────────────────┴──────────────────┴──────────────────┴────────────────────────┴─────────┘
currentInversifyFastify vs fastify Speedup: 0.916x
currentInversifyFastify vs NestJSFastify Speedup: 1.044x
Hono Basic Get Request
┌─────────┬────────────────────────┬──────────────────┬──────────────────┬────────────────────────┬─────────┐
│ (index) │ Task name │ Latency avg (ms) │ Latency med (ms) │ Throughput avg (req/s) │ Samples │
├─────────┼────────────────────────┼──────────────────┼──────────────────┼────────────────────────┼─────────┤
│ 0 │ 'currentInversifyHono' │ '4.568' │ '4.117' │ '13701.084' │ 137149 │
│ 1 │ 'hono' │ '3.995' │ '3.558' │ '15610.128' │ 156244 │
└─────────┴────────────────────────┴──────────────────┴──────────────────┴────────────────────────┴─────────┘
currentInversifyHono vs hono Speedup: 0.878x
uWebSockets Basic Get Request
┌─────────┬───────────────────────────────┬──────────────────┬──────────────────┬────────────────────────┬─────────┐
│ (index) │ Task name │ Latency avg (ms) │ Latency med (ms) │ Throughput avg (req/s) │ Samples │
├─────────┼───────────────────────────────┼──────────────────┼──────────────────┼────────────────────────┼─────────┤
│ 0 │ 'currentInversifyUwebsockets' │ '2.385' │ '1.685' │ '25352.125' │ 253890 │
│ 1 │ 'uwebsockets' │ '2.304' │ '1.691' │ '26556.561' │ 265667 │
└─────────┴───────────────────────────────┴──────────────────┴──────────────────┴────────────────────────┴─────────┘
currentInversifyUwebsockets vs uwebsockets Speedup: 0.955x |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
See #1701.
Summary by CodeRabbit
New Features
API Changes
Documentation
Tests