diff --git a/common/changes/@typespec/http/http-removeDuplicateResponse_2023-04-21-18-28.json b/common/changes/@typespec/http/http-removeDuplicateResponse_2023-04-21-18-28.json new file mode 100644 index 00000000000..2311021e2c6 --- /dev/null +++ b/common/changes/@typespec/http/http-removeDuplicateResponse_2023-04-21-18-28.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@typespec/http", + "comment": "No longer issues an error for multiple different responses for the same status code.", + "type": "none" + } + ], + "packageName": "@typespec/http" +} \ No newline at end of file diff --git a/common/changes/@typespec/openapi3/http-removeDuplicateResponse_2023-04-21-19-53.json b/common/changes/@typespec/openapi3/http-removeDuplicateResponse_2023-04-21-19-53.json new file mode 100644 index 00000000000..3c141c50f23 --- /dev/null +++ b/common/changes/@typespec/openapi3/http-removeDuplicateResponse_2023-04-21-19-53.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@typespec/openapi3", + "comment": "Support multiple responses for the same status code and content type.", + "type": "none" + } + ], + "packageName": "@typespec/openapi3" +} \ No newline at end of file diff --git a/packages/http/src/lib.ts b/packages/http/src/lib.ts index 931d53a3634..6278cbc7947 100644 --- a/packages/http/src/lib.ts +++ b/packages/http/src/lib.ts @@ -75,12 +75,6 @@ const libDefinition = { default: "contentType parameter must be a string literal or union of string literals", }, }, - "duplicate-response": { - severity: "error", - messages: { - default: paramMessage`Multiple return types for content type ${"contentType"} and status code ${"statusCode"}`, - }, - }, "content-type-ignored": { severity: "warning", messages: { diff --git a/packages/http/src/responses.ts b/packages/http/src/responses.ts index 58140729a36..c78e901fae1 100644 --- a/packages/http/src/responses.ts +++ b/packages/http/src/responses.ts @@ -100,19 +100,6 @@ function processResponseType( responses: [], }; - // check for duplicates - for (const contentType of contentTypes) { - if (response.responses.find((x) => x.body?.contentTypes.includes(contentType))) { - diagnostics.add( - createDiagnostic({ - code: "duplicate-response", - format: { statusCode: statusCode.toString(), contentType }, - target: responseType, - }) - ); - } - } - if (bodyType !== undefined) { response.responses.push({ body: { contentTypes: contentTypes, type: bodyType }, headers }); } else if (contentTypes.length > 0) { diff --git a/packages/http/test/responses.test.ts b/packages/http/test/responses.test.ts index 93cfd927feb..0dc9759c344 100644 --- a/packages/http/test/responses.test.ts +++ b/packages/http/test/responses.test.ts @@ -23,28 +23,6 @@ describe("http: responses", () => { expectDiagnostics(diagnostics, [{ code: "@typespec/http/duplicate-body" }]); }); - it("issues diagnostics for return type with duplicate status code", async () => { - const [_, diagnostics] = await compileOperations( - ` - model Foo { - foo: string; - } - model Error { - code: string; - } - @route("/") - namespace root { - @get - op read(): Foo | Error; - } - ` - ); - expectDiagnostics(diagnostics, { - code: "@typespec/http/duplicate-response", - message: "Multiple return types for content type application/json and status code 200", - }); - }); - it("issues diagnostics for invalid content types", async () => { const [_, diagnostics] = await compileOperations( ` diff --git a/packages/openapi3/src/openapi.ts b/packages/openapi3/src/openapi.ts index 9c2f6d66a88..3cb298c34a6 100644 --- a/packages/openapi3/src/openapi.ts +++ b/packages/openapi3/src/openapi.ts @@ -795,7 +795,6 @@ function createOAPIEmitter(program: Program, options: ResolvedOpenAPI3EmitterOpt content: {}, statusCode: statusCode, }; - const schemaMap = new Map(); for (const response of responses) { if (response.description && response.description !== openApiResponse.description) { @@ -803,52 +802,8 @@ function createOAPIEmitter(program: Program, options: ResolvedOpenAPI3EmitterOpt ? `${openApiResponse.description} ${response.description}` : response.description; } - for (const data of response.responses) { - if (data.headers && Object.keys(data.headers).length > 0) { - openApiResponse.headers ??= {}; - // OpenAPI can't represent different headers per content type. - // So we merge headers here, and report any duplicates. - // It may be possible in principle to not error for identically declared - // headers. - for (const [key, value] of Object.entries(data.headers)) { - if (openApiResponse.headers[key]) { - reportDiagnostic(program, { - code: "duplicate-header", - format: { header: key }, - target: response.type, - }); - continue; - } - openApiResponse.headers[key] = getResponseHeader(value); - } - } - - if (data.body !== undefined) { - openApiResponse.content ??= {}; - for (const contentType of data.body.contentTypes) { - const isBinary = isBinaryPayload(data.body.type, contentType); - const schema = isBinary - ? { type: "string", format: "binary" } - : getSchemaOrRef(data.body.type, Visibility.Read); - if (schemaMap.has(contentType)) { - schemaMap.get(contentType)!.push(schema); - } else { - schemaMap.set(contentType, [schema]); - } - } - } - const content: any = {}; - for (const [contentType, schemaArray] of schemaMap) { - if (schemaArray.length === 1) { - content[contentType] = { schema: schemaArray[0] }; - } else { - content[contentType] = { - schema: { oneOf: schemaArray }, - }; - } - } - openApiResponse.content = content; - } + emitResponseHeaders(openApiResponse, response.responses, response.type); + emitResponseContent(openApiResponse, response.responses, schemaMap); if (!openApiResponse.description) { openApiResponse.description = getResponseDescriptionForStatusCode(statusCode); } @@ -858,43 +813,73 @@ function createOAPIEmitter(program: Program, options: ResolvedOpenAPI3EmitterOpt function emitResponseObject(response: Readonly) { const statusCode = getOpenAPIStatuscode(response); - const openapiResponse = currentEndpoint.responses[statusCode] ?? { + const openApiResponse = currentEndpoint.responses[statusCode] ?? { description: response.description ?? getResponseDescriptionForStatusCode(statusCode), }; + emitResponseHeaders(openApiResponse, response.responses, response.type); + emitResponseContent(openApiResponse, response.responses); + currentEndpoint.responses[statusCode] = openApiResponse; + } - for (const data of response.responses) { + function emitResponseHeaders( + obj: any, + responses: http.HttpOperationResponseContent[], + target: Type + ) { + for (const data of responses) { if (data.headers && Object.keys(data.headers).length > 0) { - openapiResponse.headers ??= {}; + obj.headers ??= {}; // OpenAPI can't represent different headers per content type. // So we merge headers here, and report any duplicates. // It may be possible in principle to not error for identically declared // headers. for (const [key, value] of Object.entries(data.headers)) { - if (openapiResponse.headers[key]) { + if (obj.headers[key]) { reportDiagnostic(program, { code: "duplicate-header", format: { header: key }, - target: response.type, + target: target, }); continue; } - openapiResponse.headers[key] = getResponseHeader(value); + obj.headers[key] = getResponseHeader(value); } } + } + } - if (data.body !== undefined) { - openapiResponse.content ??= {}; - for (const contentType of data.body.contentTypes) { - const isBinary = isBinaryPayload(data.body.type, contentType); - const schema = isBinary - ? { type: "string", format: "binary" } - : getSchemaOrRef(data.body.type, Visibility.Read); - openapiResponse.content[contentType] = { schema }; + function emitResponseContent( + obj: any, + responses: http.HttpOperationResponseContent[], + schemaMap: Map | undefined = undefined + ) { + schemaMap ??= new Map(); + for (const data of responses) { + if (data.body === undefined) { + continue; + } + obj.content ??= {}; + for (const contentType of data.body.contentTypes) { + const isBinary = isBinaryPayload(data.body.type, contentType); + const schema = isBinary + ? { type: "string", format: "binary" } + : getSchemaOrRef(data.body.type, Visibility.Read); + if (schemaMap.has(contentType)) { + schemaMap.get(contentType)!.push(schema); + } else { + schemaMap.set(contentType, [schema]); + } + } + for (const [contentType, schema] of schemaMap) { + if (schema.length === 1) { + obj.content[contentType] = { schema: schema[0] }; + } else { + obj.content[contentType] = { + schema: { oneOf: schema }, + }; } } } - - currentEndpoint.responses[statusCode] = openapiResponse; } function getResponseDescriptionForStatusCode(statusCode: string) { diff --git a/packages/openapi3/test/return-types.test.ts b/packages/openapi3/test/return-types.test.ts index 101a77e614f..eae77da1b32 100644 --- a/packages/openapi3/test/return-types.test.ts +++ b/packages/openapi3/test/return-types.test.ts @@ -617,4 +617,112 @@ describe("openapi3: return types", () => { strictEqual(response.content["image/png"].schema.format, "binary"); }); }); + + describe("multiple responses", () => { + it("handles multiple response types for the same status code", async () => { + const res = await openApiFor(` + model A { x: 1 } + model B { y: 1 } + @route("/foo1") op foo1(): A | B ; + `); + const responses = res.paths["/foo1"].get.responses; + deepStrictEqual(responses, { + "200": { + content: { + "application/json": { + schema: { + oneOf: [ + { + $ref: "#/components/schemas/A", + }, + { + $ref: "#/components/schemas/B", + }, + ], + }, + }, + }, + description: "The request has succeeded.", + }, + }); + }); + + it("only merges responses with the same status code", async () => { + const res = await openApiFor(` + model A { x: 1 } + model B { y: 1 } + model C { @statusCode code: 201; z: 1 } + @route("/foo") op foo(): A | B | C ; + `); + const responses = res.paths["/foo"].get.responses; + deepStrictEqual(responses, { + "200": { + content: { + "application/json": { + schema: { + oneOf: [ + { + $ref: "#/components/schemas/A", + }, + { + $ref: "#/components/schemas/B", + }, + ], + }, + }, + }, + description: "The request has succeeded.", + }, + "201": { + content: { + "application/json": { + schema: { + $ref: "#/components/schemas/C", + }, + }, + }, + description: "The request has succeeded and a new resource has been created as a result.", + }, + }); + }); + + it("does not merge error responses", async () => { + const res = await openApiFor(` + model A { x: 1 } + model B { y: 1 } + @error model E { z: 1 } + @route("/foo") op foo(): A | B | E; + `); + const responses = res.paths["/foo"].get.responses; + deepStrictEqual(responses, { + "200": { + content: { + "application/json": { + schema: { + oneOf: [ + { + $ref: "#/components/schemas/A", + }, + { + $ref: "#/components/schemas/B", + }, + ], + }, + }, + }, + description: "The request has succeeded.", + }, + default: { + content: { + "application/json": { + schema: { + $ref: "#/components/schemas/E", + }, + }, + }, + description: "An unexpected error response.", + }, + }); + }); + }); });