Fix OpenAPI3 component response conversion to emit reusable response models - #11735
Fix OpenAPI3 component response conversion to emit reusable response models#11735Vincent Biret (baywet) with Copilot wants to merge 7 commits into
Conversation
|
Azure Pipelines: Successfully started running 1 pipeline(s). 1 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Co-authored-by: baywet <7905502+baywet@users.noreply.github.com>
|
Copilot add the changelog entry for this fix |
commit: |
There was a problem hiding this comment.
Pull request overview
This PR updates the OpenAPI 3 importer to treat #/components/responses/* references as first-class reusable TypeSpec models (under a Responses namespace) instead of inlining ad-hoc response objects at each operation.
Changes:
- Collect referenced component responses during conversion and emit them as reusable
Responses.*models. - Update response return-type generation to reference those reusable models when an operation response is a component
$ref. - Add a regression test asserting that a referenced component response produces a
Responsesnamespace/model and is used as the operation return type.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| packages/openapi3/test/tsp-openapi3/convert-openapi3-doc.test.ts | Adds a regression test for emitting a reusable response model for #/components/responses/* refs. |
| packages/openapi3/src/cli/actions/convert/transforms/transforms.ts | Adds component-response collection and model emission under a Responses namespace. |
| packages/openapi3/src/cli/actions/convert/generators/generate-types.ts | Teaches $ref name generation to scope component response refs under Responses.*. |
| packages/openapi3/src/cli/actions/convert/generators/generate-response-expressions.ts | Routes component response $ref return types to the generated Responses.* models. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| const ref = responseObject.$ref as string; | ||
| if (seenResponseRefs.has(ref)) continue; | ||
| seenResponseRefs.add(ref); | ||
|
|
There was a problem hiding this comment.
Fixed in 66cd2f6. The transform now records the status code each component response model was generated for (Context.registerComponentResponseStatusCode), and the response expression generator only reuses the shared model when the referencing operation response uses that same status code — otherwise the response is generated inline with its own @statusCode. Added a test covering the same $ref under 429 and 503.
| if ("$ref" in props.response && props.response.$ref.startsWith("#/components/responses/")) { | ||
| return [context.getRefName(props.response.$ref, props.operationScope)]; | ||
| } |
| function convertStatusCodeToProperty(statusCode: string): TypeSpecModelProperty { | ||
| const schema: SupportedOpenAPISchema = { type: "integer", format: "int32" }; | ||
|
|
||
| if (statusCode === "1XX") { | ||
| schema.minimum = 100; |
There was a problem hiding this comment.
Extracted both helpers (plus StatusCodes and isValidLiteralStatusCode) into a shared utils/response-properties.ts and updated both call sites to use it. 93d84a8
| paths: { | ||
| "/endpoint": { | ||
| get: { | ||
| operationId: "endpoint", | ||
| responses: { |
There was a problem hiding this comment.
Added a second operation (/other-endpoint) referencing the same #/components/responses/TooManyRequests ref, and asserted both operations return Responses.TooManyRequests and that the model is generated only once. Commit d0f5abf.
There was a problem hiding this comment.
Correction: the change is in commit 02645c4.
Co-authored-by: baywet <7905502+baywet@users.noreply.github.com>
Co-authored-by: baywet <7905502+baywet@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (4)
packages/openapi3/src/cli/actions/convert/transforms/transforms.ts:359
seenResponseRefsdedupes by component response$refonly, but the emitted model embeds the operation status code (viagetResponseProperties(statusCode, ...)). If the same#/components/responses/...is referenced under different status codes (allowed by OpenAPI), the first encountered status code will be baked into the single generated model and subsequent uses will silently get the wrong@statusCode.
for (const [statusCode, response] of Object.entries(operationResponses)) {
const responseObject = response as any;
if (
!responseObject ||
typeof responseObject !== "object" ||
!("$ref" in responseObject) ||
typeof responseObject.$ref !== "string" ||
!responseObject.$ref.startsWith("#/components/responses/")
) {
continue;
}
const ref = responseObject.$ref as string;
if (seenResponseRefs.has(ref)) continue;
seenResponseRefs.add(ref);
const componentResponse = context.getByRef<OpenAPI3Response>(ref);
if (!componentResponse) continue;
const { name, scope } = getScopeAndName(ref.slice("#/components/responses/".length));
const namespace = [...scope];
namespace.unshift("Responses");
dataTypes.push({
kind: "model",
name,
scope: namespace,
decorators: [],
doc: componentResponse.description,
properties: getResponseProperties(statusCode, componentResponse, context),
});
packages/openapi3/src/cli/actions/convert/generators/generate-response-expressions.ts:50
- The early return for
#/components/responses/...refs bypasses all existing status-code logic in this function, includingdefaulthandling (GeneratedHelpers.DefaultResponse<...>/@error) and per-media-type expansion (adding acontentTypeheader for non-application/jsoncontent). As a result, an operation likeresponses: { default: { $ref: "#/components/responses/X" } }will no longer produce a default/error response type, and component responses with multiple content types will be flattened to whatever the component model generation chose.
if ("$ref" in props.response && props.response.$ref.startsWith("#/components/responses/")) {
return [context.getRefName(props.response.$ref, props.operationScope)];
}
packages/openapi3/test/tsp-openapi3/convert-openapi3-doc.test.ts:157
- This test covers the basic
429component response ref case, but it doesn’t exercise edge cases introduced by the new control flow: (1)$refused under thedefaultresponse key (should preserve default/error (@error) semantics), and (2) the same component response ref reused across multiple operations/status codes (should not bake in the first-seen status code). Adding coverage for those cases would help prevent regressions.
it("creates reusable response models for referenced component responses", async () => {
const tsp = await convertOpenAPI3Document({
openapi: version,
info: {
title: "Example API",
version: "1.0.0",
},
paths: {
"/endpoint": {
get: {
operationId: "endpoint",
responses: {
"429": {
$ref: "#/components/responses/TooManyRequests",
},
},
},
},
},
components: {
responses: {
TooManyRequests: {
description: "The request was rejected because a rate limit was exceeded.",
headers: {
"Retry-After": {
description: "Retry delay seconds.",
required: false,
schema: {
type: "integer",
minimum: 1,
},
},
},
content: {
"application/json": {
schema: {
$ref: "#/components/schemas/ErrorResponse",
},
},
},
},
},
schemas: {
ErrorResponse: {
type: "object",
properties: {
message: {
type: "string",
description: "A human-readable message.",
},
},
},
},
},
} as any);
strictEqual(tsp.includes("namespace Responses"), true, "Expected generated response namespace: " + tsp);
strictEqual(tsp.includes("model TooManyRequests {"), true, "Expected response model: " + tsp);
strictEqual(tsp.includes("@statusCode statusCode"), true, "Expected status code property: " + tsp);
strictEqual(tsp.includes('@header("Retry-After")'), true, "Expected retry-after header: " + tsp);
strictEqual(tsp.includes("@body body:"), true, "Expected response body property: " + tsp);
strictEqual(tsp.includes("ErrorResponse"), true, "Expected body schema reference: " + tsp);
strictEqual(
tsp.includes("op endpoint(): Responses.TooManyRequests;"),
true,
"Expected operation to use the generated response model: " + tsp,
);
});
packages/openapi3/src/cli/actions/convert/transforms/transforms.ts:316
responsesis declared but never used; this adds dead code and can hide future mistakes (e.g. thinking the variable is consulted when it isn’t).
export function transformComponentResponses(context: Context, dataTypes: TypeSpecDataTypes[]): void {
const responses = context.openApi3Doc.components?.responses;
if (!responses) return;
Co-authored-by: baywet <7905502+baywet@users.noreply.github.com>
|
You can try these changes here
|
Co-authored-by: baywet <7905502+baywet@users.noreply.github.com>
Co-authored-by: baywet <7905502+baywet@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
packages/openapi3/src/cli/actions/convert/transforms/transforms.ts:322
transformComponentResponsesdeclaresresponsesonly to null-check it, but never uses the variable afterward. This will trigger unused-local lint/TS checks and can fail CI. Prefer checking the property directly without introducing an unused binding.
const responses = context.openApi3Doc.components?.responses;
if (!responses) return;
packages/openapi3/test/tsp-openapi3/convert-openapi3-doc.test.ts:201
- The new regression tests cover reuse/dedupe and the “different status code” fallback, but there’s no coverage for a component response ref used under the
defaultstatus code. Givendefaultresponses have special handling (GeneratedHelpers.DefaultResponseand@error), add a test that adefault: { $ref: "#/components/responses/..." }response preserves the default-response emission semantics rather than reusing aResponses.*model.
it("does not reuse a component response model for a different status code", async () => {
const tsp = await convertOpenAPI3Document({
openapi: version,
info: {
title: "Example API",
version: "1.0.0",
},
paths: {
"/endpoint": {
get: {
operationId: "endpoint",
| for (const [statusCode, response] of Object.entries(operationResponses)) { | ||
| const responseObject = response as any; | ||
| if ( | ||
| !responseObject || | ||
| typeof responseObject !== "object" || | ||
| !("$ref" in responseObject) || | ||
| typeof responseObject.$ref !== "string" || | ||
| !responseObject.$ref.startsWith("#/components/responses/") | ||
| ) { | ||
| continue; | ||
| } |
The OpenAPI3 import path was flattening referenced component responses into ad hoc inline response objects, which duplicated response metadata across operations and prevented reusable response models. This made shared component responses harder to maintain and could produce awkward TypeSpec definitions when the same response was referenced repeatedly.
Summary
#/components/responses/...references instead of inlining the response body at each operation.Responsesand preserves the status code, headers, and body schema from the component response.What changed
application/jsonpayload when multiple content types exist.Example
This keeps component responses reusable and reduces duplication while preserving the semantics of the original OpenAPI description.