HTTP - Remove "duplicate-response" error

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"
+}

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"
+}

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: {

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) {

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(
       `

packages/openapi3/src/openapi.ts

@@ -795,60 +795,15 @@ function createOAPIEmitter(program: Program, options: ResolvedOpenAPI3EmitterOpt
       content: {},
       statusCode: statusCode,
     };
-
     const schemaMap = new Map<string, any[]>();
     for (const response of responses) {
       if (response.description && response.description !== openApiResponse.description) {
         openApiResponse.description = openApiResponse.description
           ? `${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<HttpOperationResponse>) {
     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<string, any[]> | undefined = undefined
+  ) {
+    schemaMap ??= new Map<string, any[]>();
+    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) {

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.",
+        },
+      });
+    });
+  });
 });