Conversation
🦋 Changeset detectedLatest commit: 1d010dc The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 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 |
📝 WalkthroughWalkthroughAdds OpenAPI v3.2 support including a new SwaggerUiProvider implementation, schema/type merging utilities, path-item merge logic, v3.2-specific option types, many tests, and public exports for v3.2 provider/controller types. Changes
Sequence DiagramsequenceDiagram
participant Container as Inversify Container
participant Provider as SwaggerUiProvider
participant Metadata as Reflection Metadata Utils
participant PathMerge as mergeOpenApiPathItemObjectIntoOpenApiPaths
participant SchemaMerge as mergeOpenApiTypeSchema
participant ControllerFactory as buildSwaggerUiController
Container->>Provider: list bound controller types
Provider->>Metadata: getSchemaMetadata(controllerType)
loop for each controller
Provider->>Metadata: extract controller-level OpenAPI metadata
loop for each method
Provider->>PathMerge: mergeOpenApiPathItemObjectIntoOpenApiPaths(target,path,operation)
PathMerge-->>Provider: updated target.paths
end
alt controller references exist
Provider->>SchemaMerge: mergeOpenApiTypeSchema(components, refType)
SchemaMerge-->>Provider: components.schemas updated
end
end
Provider->>ControllerFactory: buildSwaggerUiController(options)
ControllerFactory-->>Provider: ControllerClass
Provider->>Container: bind ControllerClass to container
Container-->>Provider: binding complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
packages/framework/http/libraries/open-api/src/metadata/actions/v3Dot2/mergeOpenApiPathItemObjectIntoOpenApiPaths.spec.ts (1)
10-374: Align this suite with the repo's spec conventions.
describe.each('having OpenApi3Dot2Object with %s', ...)skips the usual method/input/flow layering and inlines a lot of repeated OpenAPI fixtures. Wrapping the helper under its own describe scopes and moving these builders into fixture classes would make this match the rest of the repo. As per coding guidelines,**/*.spec.ts: Create unit tests using the four-layer describe structure: Class → Method → Input → Flow scopes,**/*.{spec,int.spec}.ts: Name tests using 'when called, and [condition]' pattern with descriptive test names, and**/*.{spec,int.spec}.ts: Create reusable test fixtures as classes with static methods following fixture guidelines from docs/testing/fixtures.md.🤖 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/actions/v3Dot2/mergeOpenApiPathItemObjectIntoOpenApiPaths.spec.ts` around lines 10 - 374, Test suite uses a single describe.each with inlined fixtures rather than the repo's four-layer describe structure and reusable fixtures; refactor the suite around the function mergeOpenApiPathItemObjectIntoOpenApiPaths by adding nested describe blocks (Class → Method → Input → Flow) instead of the current describe.each, move the OpenApi3Dot2Object/path/pathItem builders into a fixture class with static factory methods (per docs/testing/fixtures.md), and rename tests to the "when called, and [condition]" pattern so each case becomes a clear nested "when" flow with descriptive it blocks.packages/framework/http/libraries/open-api/src/metadata/calculations/v3Dot2/getSchemaMetadata.spec.ts (1)
14-102: Reshape this spec to the repo's unit-test layout.The suite jumps straight from function scope into scenario scopes and repeats the same
OpenApiSchemaMetadatafixture in both branches. Please split it into the standard describe layers and move the repeated setup into fixture helpers/classes. As per coding guidelines,**/*.spec.ts: Create unit tests using the four-layer describe structure: Class → Method → Input → Flow scopes, and**/*.{spec,int.spec}.ts: Create reusable test fixtures as classes with static methods following fixture guidelines from docs/testing/fixtures.md.🤖 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/getSchemaMetadata.spec.ts` around lines 14 - 102, The spec file for getSchemaMetadata needs to follow the repo's four-layer test layout and extract repeated fixtures into a reusable fixture class; refactor the describe blocks to Class → Method → Input → Flow (e.g., top-level describe for the module/class, nested describe for getSchemaMetadata, then describe for "when getOwnReflectMetadata returns undefined/SchemaMetadata"), move the duplicated OpenApiSchemaMetadata object into a fixture helper (create a fixture class with static builder methods per docs/testing/fixtures.md) and replace direct instantiation in both flows, and keep/verify mocks for buildDefaultSchemaMetadata and getOwnReflectMetadata using the same referenced symbols (getSchemaMetadata, OpenApiSchemaMetadata, buildDefaultSchemaMetadata, getOwnReflectMetadata, schemaOpenApiMetadataReflectKey) so tests assert the same behaviors but conform to the four-layer structure.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/framework/http/libraries/open-api/src/metadata/actions/v3Dot2/mergeOpenApiPathItemObjectIntoOpenApiPaths.ts`:
- Around line 18-24: mergeOpenApiPathItemObjectIntoOpenApiPaths currently
assigns openApiPathItemObject directly into target.paths[path], leaving a live
reference that later deepMerge calls can mutate; instead, create a deep clone of
openApiPathItemObject before assigning (e.g., via structuredClone/deepClone
utility or JSON round-trip) so subsequent calls to deepMerge operate on a copy,
not the original metadata object, ensuring target.paths[path] receives an
independent object and original openApiPathItemObject remains unchanged.
In
`@packages/framework/http/libraries/open-api/src/openApi/actions/v3Dot2/mergeOpenApiTypeSchema.spec.ts`:
- Around line 668-773: The test surfaces a regression where
mergeOpenApiTypeSchema stops processing after the first non-well-known property,
so update mergeOpenApiTypeSchema to iterate all properties (not return/exit
early) and for each property: call getSchemaMetadata(propertyType), call
tryBuildSchemaFromWellKnownType(propertyType) and if undefined add a $ref entry
for the escaped name (using escapeJsonPointerFragments), add the property to the
target schema's properties map, accumulate required flags into the target
schema.required array, and union any schemaMetadata.references into the
top-level references set so multiple unresolved properties and references are
handled; ensure functions/variables referenced in the diff
(mergeOpenApiTypeSchema, getSchemaMetadata, tryBuildSchemaFromWellKnownType,
escapeJsonPointerFragments, schemasObject) are updated to remove early return
and process all properties.
In
`@packages/framework/http/libraries/open-api/src/openApi/actions/v3Dot2/mergeOpenApiTypeSchema.ts`:
- Around line 58-71: The early "return" inside mergeOpenApiTypeSchema when
schemaFromWellKnownType === undefined prematurely exits processing for the
entire type; replace that "return" with "continue" so the loop (in
mergeOpenApiTypeSchema) proceeds to the next propertyKey, allowing assignment of
required fields and accumulation of schemaMetadata.references; update any
affected local logic that expects the loop to continue (references:
mergeOpenApiTypeSchema, schemaFromWellKnownType, jsonSchemaProperties,
propertyKey, propertySchemaMetadata, schemaMetadata.references, required).
In
`@packages/framework/http/libraries/open-api/src/openApi/services/v3Dot2/SwaggerUiProvider.spec.ts`:
- Around line 52-69: The test currently shares a single mutable OpenApi object
via optionsFixture.api.openApiObject created in beforeAll, but
SwaggerUiProvider.provide mutates that object (e.g., initializes
components.schemas), making tests order-dependent; change the spec to produce a
fresh options fixture per test (use beforeEach) or implement a reusable fixture
class with a static factory method that returns a new options object (including
a new OpenApi3Dot2Object for optionsFixture.api.openApiObject) for each
scenario, and update usages of optionsFixture and controllerTypeFixture so each
test calls that factory before invoking SwaggerUiProvider.provide or
BaseSwaggerUiController to avoid cross-test mutation and follow the repository’s
fixture guidelines.
In
`@packages/framework/http/libraries/open-api/src/openApi/services/v3Dot2/SwaggerUiProvider.ts`:
- Around line 36-44: The RequestMethodType.All mapping in SwaggerUiProvider.ts
omits the new OpenApi3Dot2PathItemObjectMethodKeys entries for "query" and
"trace", making those v3.2 operations unreachable; update the
RequestMethodType.All array in SwaggerUiProvider (the provider that builds v3.2
paths) to include 'query' and 'trace' alongside the existing verbs so the
provider emits all OpenApi3Dot2PathItemObjectMethodKeys and the corresponding
tests (e.g., SwaggerUiProvider.spec.ts) will exercise those operations.
---
Nitpick comments:
In
`@packages/framework/http/libraries/open-api/src/metadata/actions/v3Dot2/mergeOpenApiPathItemObjectIntoOpenApiPaths.spec.ts`:
- Around line 10-374: Test suite uses a single describe.each with inlined
fixtures rather than the repo's four-layer describe structure and reusable
fixtures; refactor the suite around the function
mergeOpenApiPathItemObjectIntoOpenApiPaths by adding nested describe blocks
(Class → Method → Input → Flow) instead of the current describe.each, move the
OpenApi3Dot2Object/path/pathItem builders into a fixture class with static
factory methods (per docs/testing/fixtures.md), and rename tests to the "when
called, and [condition]" pattern so each case becomes a clear nested "when" flow
with descriptive it blocks.
In
`@packages/framework/http/libraries/open-api/src/metadata/calculations/v3Dot2/getSchemaMetadata.spec.ts`:
- Around line 14-102: The spec file for getSchemaMetadata needs to follow the
repo's four-layer test layout and extract repeated fixtures into a reusable
fixture class; refactor the describe blocks to Class → Method → Input → Flow
(e.g., top-level describe for the module/class, nested describe for
getSchemaMetadata, then describe for "when getOwnReflectMetadata returns
undefined/SchemaMetadata"), move the duplicated OpenApiSchemaMetadata object
into a fixture helper (create a fixture class with static builder methods per
docs/testing/fixtures.md) and replace direct instantiation in both flows, and
keep/verify mocks for buildDefaultSchemaMetadata and getOwnReflectMetadata using
the same referenced symbols (getSchemaMetadata, OpenApiSchemaMetadata,
buildDefaultSchemaMetadata, getOwnReflectMetadata,
schemaOpenApiMetadataReflectKey) so tests assert the same behaviors but conform
to the four-layer structure.
🪄 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: d3905749-6700-499e-91a9-34e71b9d8436
📒 Files selected for processing (14)
.changeset/metal-worms-know.mdpackages/framework/http/libraries/open-api/src/common/models/FilteredByValueType.spec-d.tspackages/framework/http/libraries/open-api/src/common/models/FilteredByValueType.tspackages/framework/http/libraries/open-api/src/metadata/actions/v3Dot2/mergeOpenApiPathItemObjectIntoOpenApiPaths.spec.tspackages/framework/http/libraries/open-api/src/metadata/actions/v3Dot2/mergeOpenApiPathItemObjectIntoOpenApiPaths.tspackages/framework/http/libraries/open-api/src/metadata/calculations/v3Dot2/getSchemaMetadata.spec.tspackages/framework/http/libraries/open-api/src/metadata/calculations/v3Dot2/getSchemaMetadata.tspackages/framework/http/libraries/open-api/src/openApi/actions/v3Dot2/mergeOpenApiTypeSchema.spec.tspackages/framework/http/libraries/open-api/src/openApi/actions/v3Dot2/mergeOpenApiTypeSchema.tspackages/framework/http/libraries/open-api/src/openApi/models/v3Dot2/SwaggerUiProviderApiOptions.tspackages/framework/http/libraries/open-api/src/openApi/models/v3Dot2/SwaggerUiProviderOptions.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.ts
...libraries/open-api/src/metadata/actions/v3Dot2/mergeOpenApiPathItemObjectIntoOpenApiPaths.ts
Show resolved
Hide resolved
.../framework/http/libraries/open-api/src/openApi/actions/v3Dot2/mergeOpenApiTypeSchema.spec.ts
Show resolved
Hide resolved
packages/framework/http/libraries/open-api/src/openApi/actions/v3Dot2/mergeOpenApiTypeSchema.ts
Outdated
Show resolved
Hide resolved
| let optionsFixture: SwaggerUiProviderOptions; | ||
|
|
||
| let controllerTypeFixture: Newable< | ||
| BaseSwaggerUiController<OpenApi3Dot2Object> | ||
| >; | ||
|
|
||
| beforeAll(() => { | ||
| optionsFixture = { | ||
| api: { | ||
| openApiObject: {} as OpenApi3Dot2Object, | ||
| path: '/path/fixture', | ||
| }, | ||
| ui: Symbol() as unknown as SwaggerUiProviderUiOptions, | ||
| }; | ||
| controllerTypeFixture = Symbol() as unknown as Newable< | ||
| BaseSwaggerUiController<OpenApi3Dot2Object> | ||
| >; | ||
| }); |
There was a problem hiding this comment.
Don't share a mutable openApiObject across the whole suite.
optionsFixture.api.openApiObject is created once here, but SwaggerUiProvider.provide() mutates it in place on every call — including the empty-metadata path, which initializes components.schemas. That makes later cases order-dependent and can hide regressions around existing paths/components. Please build a fresh options fixture per scenario instead of reusing this singleton.
As per coding guidelines, **/*.{spec,int.spec}.ts: Create reusable test fixtures as classes with static methods following fixture guidelines from docs/testing/fixtures.md`.
🤖 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/v3Dot2/SwaggerUiProvider.spec.ts`
around lines 52 - 69, The test currently shares a single mutable OpenApi object
via optionsFixture.api.openApiObject created in beforeAll, but
SwaggerUiProvider.provide mutates that object (e.g., initializes
components.schemas), making tests order-dependent; change the spec to produce a
fresh options fixture per test (use beforeEach) or implement a reusable fixture
class with a static factory method that returns a new options object (including
a new OpenApi3Dot2Object for optionsFixture.api.openApiObject) for each
scenario, and update usages of optionsFixture and controllerTypeFixture so each
test calls that factory before invoking SwaggerUiProvider.provide or
BaseSwaggerUiController to avoid cross-test mutation and follow the repository’s
fixture guidelines.
packages/framework/http/libraries/open-api/src/openApi/services/v3Dot2/SwaggerUiProvider.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
packages/framework/http/libraries/open-api/src/metadata/calculations/v3Dot1/getSchemaMetadata.spec.ts (1)
14-16: Prefer a static fixture method over a static getterUse
OpenApiSchemaMetadataFixtures.any()instead ofget anyto align with the fixture convention for spec files.Suggested refactor
class OpenApiSchemaMetadataFixtures { - public static get any(): OpenApiSchemaMetadata { + public static any(): OpenApiSchemaMetadata { return { customAttributes: undefined, name: 'TestSchema', properties: new Map(), references: new Set(), schema: undefined, }; } } @@ - schemaMetadataFixture = OpenApiSchemaMetadataFixtures.any; + schemaMetadataFixture = OpenApiSchemaMetadataFixtures.any(); @@ - schemaMetadataFixture = OpenApiSchemaMetadataFixtures.any; + schemaMetadataFixture = OpenApiSchemaMetadataFixtures.any();As per coding guidelines
**/*.{spec,int.spec}.ts: Create reusable test fixtures as classes with static methods following fixture guidelines from docs/testing/fixtures.md.Also applies to: 42-42, 77-77
🤖 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/getSchemaMetadata.spec.ts` around lines 14 - 16, The test fixture class OpenApiSchemaMetadataFixtures currently exposes a static getter named "get any" — change this to a static method any() (i.e., replace the static getter with a static function named any) so callers use OpenApiSchemaMetadataFixtures.any() to align with the project fixture convention; update all occurrences of the getter (including the other similar fixtures in this file) and any tests that reference the getter to call the method instead.packages/framework/http/libraries/open-api/src/metadata/actions/v3Dot1/mergeOpenApiPathItemObjectIntoOpenApiPaths.spec.ts (3)
113-427: Add one regression case for reference isolation after merge.Given the clone fix, please add a test that mutates the original
openApiPathItemObjectafter calling merge and assertsresult.paths['/path']does not change.🤖 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/actions/v3Dot1/mergeOpenApiPathItemObjectIntoOpenApiPaths.spec.ts` around lines 113 - 427, Add a regression test to the mergeOpenApiPathItemObjectIntoOpenApiPaths.spec.ts suite that verifies the merge produces an isolated clone: call mergeOpenApiPathItemObjectIntoOpenApiPaths with a fixture and a local variable openApiPathItemObject, mutate openApiPathItemObject after the call (e.g., change a property or push into an array like tags or parameters), and assert that result.paths['/path'] (the returned merged object) did not change; reference the mergeOpenApiPathItemObjectIntoOpenApiPaths function, the local openApiPathItemObject used in the test, and the result variable to locate where to add this case.
10-111: Use static fixture methods instead of static getters.The fixture class is a good addition, but this segment uses static getters while the repo guideline asks for static methods.
Suggested refactor
class OpenApi3Dot1ObjectFixtures { - public static get withNoPaths(): OpenApi3Dot1Object { + public static withNoPaths(): OpenApi3Dot1Object { return { info: { title: 'My awesome API', version: '1.0.0', }, openapi: '3.1.0', }; } - public static get withEmptyPaths(): OpenApi3Dot1Object { + public static withEmptyPaths(): OpenApi3Dot1Object { return { - ...OpenApi3Dot1ObjectFixtures.withNoPaths, + ...OpenApi3Dot1ObjectFixtures.withNoPaths(), paths: {}, }; } }As per coding guidelines
**/*.{spec,int.spec}.ts: "Create reusable test fixtures as classes with static methods following fixture guidelines from docs/testing/fixtures.md".🤖 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/actions/v3Dot1/mergeOpenApiPathItemObjectIntoOpenApiPaths.spec.ts` around lines 10 - 111, Convert the static getters on the OpenApi3Dot1ObjectFixtures class into static methods: change each "public static get withNoPaths/withEmptyPaths/withExistingPathEntry/withOverlappingMethods/withArrayTags/withNestedResponses/withMixedArrayAndObjectOperations" into "public static withNoPaths(): OpenApi3Dot1Object { ... }" (and similarly for the others) and return the same objects; then update any test calls to invoke these as methods (e.g. OpenApi3Dot1ObjectFixtures.withNoPaths() instead of the property access) so fixtures follow the repo guideline of using static fixture methods.
115-425: Align scenario describe names with the requiredwhen called, and ...convention.Current input scopes use
"having ..."; please rename these scopes to the required"when called, and [condition]"pattern for consistency with repo test conventions.As per coding guidelines
**/*.{spec,int.spec,spec-d}.ts: "Name tests using 'when called, and [condition]' pattern with descriptive test names".🤖 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/actions/v3Dot1/mergeOpenApiPathItemObjectIntoOpenApiPaths.spec.ts` around lines 115 - 425, Rename the outer scenario describe titles that start with "having an OpenApi3Dot1Object ..." to follow the "when called, and [condition]" convention used across tests; update each describe surrounding the mergeOpenApiPathItemObjectIntoOpenApiPaths calls (e.g., the blocks using OpenApi3Dot1ObjectFixtures.withNoPaths, withEmptyPaths, withExistingPathEntry, withOverlappingMethods, withArrayTags, withNestedResponses, withMixedArrayAndObjectOperations) so the describe string reads "when called, and [brief condition]" (for example "when called, and there are no paths", "when called, and paths is empty", "when called, and an existing path entry exists", etc.) ensuring the inner "when called" blocks remain consistent.packages/framework/http/libraries/open-api/src/openApi/actions/v3Dot2/mergeOpenApiTypeSchema.spec.ts (1)
58-1107: Extract reusable metadata/schema fixtures instead of rebuilding them inline.The suite repeatedly hand-builds the same
schemasObjectFixture,OpenApiSchemaMetadata, and referenced-type shapes. Moving those into fixture classes/static builders would make the new edge cases easier to extend and keep each flow block focused on behavior instead of setup.As per coding guidelines, "Create reusable test fixtures as classes with static methods following fixture guidelines from docs/testing/fixtures.md".
🤖 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/actions/v3Dot2/mergeOpenApiTypeSchema.spec.ts` around lines 58 - 1107, Tests repeatedly recreate identical test data (schemasObjectFixture, OpenApiSchemaMetadata instances, property/referenced-type shapes) making setup verbose; refactor by extracting reusable test fixtures: create a Fixture class with static builders like Fixture.createSchemasObject(), Fixture.createSchemaMetadata({name, schema, properties, references}), Fixture.createPropertySchema(), and Fixture.createReferencedTypeName()/createReferencedTypeConstructor() and use these in places that call getSchemaMetadataMock, getOwnReflectMetadataMock, escapeJsonPointerFragmentsMock, and mergeOpenApiTypeSchema; update each describe block to call the appropriate Fixture static method(s) instead of inlining object construction and import the Fixture into the spec so all repeated structures are centralized and reusable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/framework/http/libraries/open-api/src/metadata/actions/v3Dot2/mergeOpenApiPathItemObjectIntoOpenApiPaths.spec.ts`:
- Line 17: The fixture OpenApi3Dot2ObjectFixtures in
mergeOpenApiPathItemObjectIntoOpenApiPaths.spec.ts is using the wrong OpenAPI
version string; change the openapi property value from '3.1.0' to '3.2.0' so the
fixture matches its name and the v3.2 tests (update the openapi: field inside
OpenApi3Dot2ObjectFixtures).
In
`@packages/framework/http/libraries/open-api/src/openApi/actions/v3Dot2/mergeOpenApiTypeSchema.ts`:
- Around line 80-81: In mergeOpenApiTypeSchema.ts inside the
mergeOpenApiTypeSchema logic, ensure that when returning the base schema (the
branch around Line 102-103) you merge and preserve
OpenApiSchemaMetadata.customAttributes onto the returned schema instead of
dropping them, and when setting jsonSchema.required (the branch around Line
80-81) union the existing jsonSchema.required with requiredProperties rather
than overwriting so caller-supplied required entries are retained; update the
code paths that handle schema-only returns and required merging in
mergeOpenApiTypeSchema to shallow-merge customAttributes and to compute required
= Array.from(new Set([...(existingRequired||[]), ...requiredProperties])) before
assigning.
---
Nitpick comments:
In
`@packages/framework/http/libraries/open-api/src/metadata/actions/v3Dot1/mergeOpenApiPathItemObjectIntoOpenApiPaths.spec.ts`:
- Around line 113-427: Add a regression test to the
mergeOpenApiPathItemObjectIntoOpenApiPaths.spec.ts suite that verifies the merge
produces an isolated clone: call mergeOpenApiPathItemObjectIntoOpenApiPaths with
a fixture and a local variable openApiPathItemObject, mutate
openApiPathItemObject after the call (e.g., change a property or push into an
array like tags or parameters), and assert that result.paths['/path'] (the
returned merged object) did not change; reference the
mergeOpenApiPathItemObjectIntoOpenApiPaths function, the local
openApiPathItemObject used in the test, and the result variable to locate where
to add this case.
- Around line 10-111: Convert the static getters on the
OpenApi3Dot1ObjectFixtures class into static methods: change each "public static
get
withNoPaths/withEmptyPaths/withExistingPathEntry/withOverlappingMethods/withArrayTags/withNestedResponses/withMixedArrayAndObjectOperations"
into "public static withNoPaths(): OpenApi3Dot1Object { ... }" (and similarly
for the others) and return the same objects; then update any test calls to
invoke these as methods (e.g. OpenApi3Dot1ObjectFixtures.withNoPaths() instead
of the property access) so fixtures follow the repo guideline of using static
fixture methods.
- Around line 115-425: Rename the outer scenario describe titles that start with
"having an OpenApi3Dot1Object ..." to follow the "when called, and [condition]"
convention used across tests; update each describe surrounding the
mergeOpenApiPathItemObjectIntoOpenApiPaths calls (e.g., the blocks using
OpenApi3Dot1ObjectFixtures.withNoPaths, withEmptyPaths, withExistingPathEntry,
withOverlappingMethods, withArrayTags, withNestedResponses,
withMixedArrayAndObjectOperations) so the describe string reads "when called,
and [brief condition]" (for example "when called, and there are no paths", "when
called, and paths is empty", "when called, and an existing path entry exists",
etc.) ensuring the inner "when called" blocks remain consistent.
In
`@packages/framework/http/libraries/open-api/src/metadata/calculations/v3Dot1/getSchemaMetadata.spec.ts`:
- Around line 14-16: The test fixture class OpenApiSchemaMetadataFixtures
currently exposes a static getter named "get any" — change this to a static
method any() (i.e., replace the static getter with a static function named any)
so callers use OpenApiSchemaMetadataFixtures.any() to align with the project
fixture convention; update all occurrences of the getter (including the other
similar fixtures in this file) and any tests that reference the getter to call
the method instead.
In
`@packages/framework/http/libraries/open-api/src/openApi/actions/v3Dot2/mergeOpenApiTypeSchema.spec.ts`:
- Around line 58-1107: Tests repeatedly recreate identical test data
(schemasObjectFixture, OpenApiSchemaMetadata instances, property/referenced-type
shapes) making setup verbose; refactor by extracting reusable test fixtures:
create a Fixture class with static builders like Fixture.createSchemasObject(),
Fixture.createSchemaMetadata({name, schema, properties, references}),
Fixture.createPropertySchema(), and
Fixture.createReferencedTypeName()/createReferencedTypeConstructor() and use
these in places that call getSchemaMetadataMock, getOwnReflectMetadataMock,
escapeJsonPointerFragmentsMock, and mergeOpenApiTypeSchema; update each describe
block to call the appropriate Fixture static method(s) instead of inlining
object construction and import the Fixture into the spec so all repeated
structures are centralized and reusable.
🪄 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: f9d191d1-015b-49a5-903a-2c7e460fd0e7
📒 Files selected for processing (11)
.changeset/eager-adults-fail.mdpackages/framework/http/libraries/open-api/src/metadata/actions/v3Dot1/mergeOpenApiPathItemObjectIntoOpenApiPaths.spec.tspackages/framework/http/libraries/open-api/src/metadata/actions/v3Dot1/mergeOpenApiPathItemObjectIntoOpenApiPaths.tspackages/framework/http/libraries/open-api/src/metadata/actions/v3Dot2/mergeOpenApiPathItemObjectIntoOpenApiPaths.spec.tspackages/framework/http/libraries/open-api/src/metadata/actions/v3Dot2/mergeOpenApiPathItemObjectIntoOpenApiPaths.tspackages/framework/http/libraries/open-api/src/metadata/calculations/v3Dot1/getSchemaMetadata.spec.tspackages/framework/http/libraries/open-api/src/metadata/calculations/v3Dot2/getSchemaMetadata.spec.tspackages/framework/http/libraries/open-api/src/openApi/actions/v3Dot1/mergeOpenApiTypeSchema.spec.tspackages/framework/http/libraries/open-api/src/openApi/actions/v3Dot1/mergeOpenApiTypeSchema.tspackages/framework/http/libraries/open-api/src/openApi/actions/v3Dot2/mergeOpenApiTypeSchema.spec.tspackages/framework/http/libraries/open-api/src/openApi/actions/v3Dot2/mergeOpenApiTypeSchema.ts
✅ Files skipped from review due to trivial changes (2)
- .changeset/eager-adults-fail.md
- packages/framework/http/libraries/open-api/src/metadata/calculations/v3Dot2/getSchemaMetadata.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/framework/http/libraries/open-api/src/metadata/actions/v3Dot2/mergeOpenApiPathItemObjectIntoOpenApiPaths.ts
...ries/open-api/src/metadata/actions/v3Dot2/mergeOpenApiPathItemObjectIntoOpenApiPaths.spec.ts
Outdated
Show resolved
Hide resolved
packages/framework/http/libraries/open-api/src/openApi/actions/v3Dot2/mergeOpenApiTypeSchema.ts
Outdated
Show resolved
Hide resolved
…agger-ui-provider
update module to handle metadata with both customAttributes and schema
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/framework/http/libraries/open-api/src/metadata/actions/v3Dot2/mergeOpenApiPathItemObjectIntoOpenApiPaths.spec.ts (2)
10-111: Use static fixture methods instead of static getters.Fixture factories are implemented as
getaccessors. In this repo’s test conventions, these should be static methods for reusable fixtures.♻️ Suggested refactor
class OpenApi3Dot2ObjectFixtures { - public static get withNoPaths(): OpenApi3Dot2Object { + public static withNoPaths(): OpenApi3Dot2Object { return { info: { title: 'My awesome API', version: '1.0.0', }, openapi: '3.2.0', }; }-OpenApi3Dot2ObjectFixtures.withNoPaths +OpenApi3Dot2ObjectFixtures.withNoPaths()As per coding guidelines:
**/*.{spec,int.spec}.ts: Create reusable test fixtures as classes with static methods following fixture guidelines from docs/testing/fixtures.md.🤖 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/actions/v3Dot2/mergeOpenApiPathItemObjectIntoOpenApiPaths.spec.ts` around lines 10 - 111, OpenApi3Dot2ObjectFixtures uses static getters (withNoPaths, withEmptyPaths, withExistingPathEntry, withOverlappingMethods, withArrayTags, withNestedResponses, withMixedArrayAndObjectOperations); change each `public static get <name>(): OpenApi3Dot2Object { ... }` into a static method `public static <name>(): OpenApi3Dot2Object { ... }` that returns the same fixture object, and update any tests calling the fixtures from property access (OpenApi3Dot2ObjectFixtures.withNoPaths) to method calls (OpenApi3Dot2ObjectFixtures.withNoPaths()) so fixtures follow the repo convention for reusable test factories.
115-425: Align describe names with the required “when called, and …” pattern.
describescopes (e.g., Line [115], Line [140], Line [165]) currently usehaving ...+ nestedwhen called. Please normalize towhen called, and [condition]for the scoped context names to match the repo convention consistently.Based on learnings: in spec files, use the
when called, and [condition]pattern fordescribeblocks; and as per coding guidelines:**/*.{spec,int.spec,spec-d}.tstest naming should follow that pattern.🤖 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/actions/v3Dot2/mergeOpenApiPathItemObjectIntoOpenApiPaths.spec.ts` around lines 115 - 425, The top-level describe scopes in the test file use "having ..." plus a nested "when called" instead of the required "when called, and [condition]" pattern; update the describe titles that wrap the calls to mergeOpenApiPathItemObjectIntoOpenApiPaths (e.g., the scopes currently named "having an OpenApi3Dot2Object with no paths", "having an OpenApi3Dot2Object with empty paths", "having an OpenApi3Dot2Object with existing path entry", "having an OpenApi3Dot2Object with overlapping methods", "having an OpenApi3Dot2Object with array merging (tags)", "having an OpenApi3Dot2Object with nested object merging", and "having an OpenApi3Dot2Object with mixed array and object operations") to follow the convention by using a single describe string per scope of the form "when called, and [condition]" (for example: "when called, and the OpenApi3Dot2Object has no paths") so the nested structure remains the same and tests still call mergeOpenApiPathItemObjectIntoOpenApiPaths with OpenApi3Dot2ObjectFixtures and the same path/pathItemObject.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@packages/framework/http/libraries/open-api/src/metadata/actions/v3Dot2/mergeOpenApiPathItemObjectIntoOpenApiPaths.spec.ts`:
- Around line 10-111: OpenApi3Dot2ObjectFixtures uses static getters
(withNoPaths, withEmptyPaths, withExistingPathEntry, withOverlappingMethods,
withArrayTags, withNestedResponses, withMixedArrayAndObjectOperations); change
each `public static get <name>(): OpenApi3Dot2Object { ... }` into a static
method `public static <name>(): OpenApi3Dot2Object { ... }` that returns the
same fixture object, and update any tests calling the fixtures from property
access (OpenApi3Dot2ObjectFixtures.withNoPaths) to method calls
(OpenApi3Dot2ObjectFixtures.withNoPaths()) so fixtures follow the repo
convention for reusable test factories.
- Around line 115-425: The top-level describe scopes in the test file use
"having ..." plus a nested "when called" instead of the required "when called,
and [condition]" pattern; update the describe titles that wrap the calls to
mergeOpenApiPathItemObjectIntoOpenApiPaths (e.g., the scopes currently named
"having an OpenApi3Dot2Object with no paths", "having an OpenApi3Dot2Object with
empty paths", "having an OpenApi3Dot2Object with existing path entry", "having
an OpenApi3Dot2Object with overlapping methods", "having an OpenApi3Dot2Object
with array merging (tags)", "having an OpenApi3Dot2Object with nested object
merging", and "having an OpenApi3Dot2Object with mixed array and object
operations") to follow the convention by using a single describe string per
scope of the form "when called, and [condition]" (for example: "when called, and
the OpenApi3Dot2Object has no paths") so the nested structure remains the same
and tests still call mergeOpenApiPathItemObjectIntoOpenApiPaths with
OpenApi3Dot2ObjectFixtures and the same path/pathItemObject.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 29cb7adf-a18e-4b76-8861-5b79252315e1
📒 Files selected for processing (5)
packages/framework/http/libraries/open-api/src/metadata/actions/v3Dot2/mergeOpenApiPathItemObjectIntoOpenApiPaths.spec.tspackages/framework/http/libraries/open-api/src/openApi/actions/v3Dot1/mergeOpenApiTypeSchema.spec.tspackages/framework/http/libraries/open-api/src/openApi/actions/v3Dot1/mergeOpenApiTypeSchema.tspackages/framework/http/libraries/open-api/src/openApi/actions/v3Dot2/mergeOpenApiTypeSchema.spec.tspackages/framework/http/libraries/open-api/src/openApi/actions/v3Dot2/mergeOpenApiTypeSchema.ts
✅ Files skipped from review due to trivial changes (1)
- packages/framework/http/libraries/open-api/src/openApi/actions/v3Dot2/mergeOpenApiTypeSchema.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/framework/http/libraries/open-api/src/openApi/actions/v3Dot1/mergeOpenApiTypeSchema.spec.ts
- packages/framework/http/libraries/open-api/src/openApi/actions/v3Dot1/mergeOpenApiTypeSchema.ts
- packages/framework/http/libraries/open-api/src/openApi/actions/v3Dot2/mergeOpenApiTypeSchema.spec.ts
Added
SwaggerUiProvider.Changed
FilteredByValueTypeto properly handle v3.2 typing edge case.Summary by CodeRabbit
New Features
Bug Fixes / Improvements
Tests
Chores