[tcgc] change response structure to mapping with key number, status c…

…ode range, or * (#515)
Azure · Mar 26, 2024 · c5ef8f3 · c5ef8f3
1 parent 0a17c8f
commit c5ef8f3
Show file tree

Hide file tree

Showing 5 changed files with 78 additions and 49 deletions.
diff --git a/.chronus/changes/response_structure-2024-2-25-17-16-10.md b/.chronus/changes/response_structure-2024-2-25-17-16-10.md
@@ -0,0 +1,7 @@
+---
+changeKind: breaking
+packages:
+  - "@azure-tools/typespec-client-generator-core"
+---
+
+change responses from a record to a mapping of status code, range, or default
diff --git a/packages/typespec-client-generator-core/src/interfaces.ts b/packages/typespec-client-generator-core/src/interfaces.ts
@@ -14,6 +14,7 @@ import {
   HttpAuth,
   HttpOperation,
   HttpOperationResponse,
+  HttpStatusCodeRange,
   HttpVerb,
   Visibility,
 } from "@typespec/http";
@@ -440,8 +441,8 @@ export interface SdkHttpOperation extends SdkServiceOperationBase {
   verb: HttpVerb;
   parameters: (SdkPathParameter | SdkQueryParameter | SdkHeaderParameter)[];
   bodyParams: SdkBodyParameter[]; // array for cases like urlencoded / multipart
-  responses: Record<number | string, SdkHttpResponse>; // we will use string to represent status code range
-  exceptions: Record<number | string, SdkHttpResponse>; // we will use string to represent status code range
+  responses: Map<HttpStatusCodeRange | number, SdkHttpResponse>;
+  exceptions: Map<HttpStatusCodeRange | number | "*", SdkHttpResponse>;
 }
 
 /**

diff --git a/packages/typespec-client-generator-core/src/internal-utils.ts b/packages/typespec-client-generator-core/src/internal-utils.ts
@@ -14,7 +14,7 @@ import {
   ignoreDiagnostics,
   isNullType,
 } from "@typespec/compiler";
-import { HttpOperation } from "@typespec/http";
+import { HttpOperation, HttpStatusCodeRange } from "@typespec/http";
 import { getAddedOnVersions, getRemovedOnVersions, getVersions } from "@typespec/versioning";
 import {
   SdkBuiltInKinds,
@@ -258,13 +258,15 @@ export function getNonNullOptions(type: Union): Type[] {
   return [...type.variants.values()].map((x) => x.type).filter((t) => !isNullType(t));
 }
 
-function getAllResponseBodiesAndNonBodyExists(responses: Record<number, SdkHttpResponse>): {
+function getAllResponseBodiesAndNonBodyExists(
+  responses: Map<HttpStatusCodeRange | number | "*", SdkHttpResponse>
+): {
   allResponseBodies: SdkType[];
   nonBodyExists: boolean;
 } {
   const allResponseBodies: SdkType[] = [];
   let nonBodyExists = false;
-  for (const response of Object.values(responses)) {
+  for (const response of responses.values()) {
     if (response.type) {
       if (response.nullable) {
         nonBodyExists = true;
@@ -277,7 +279,9 @@ function getAllResponseBodiesAndNonBodyExists(responses: Record<number, SdkHttpR
   return { allResponseBodies, nonBodyExists };
 }
 
-export function getAllResponseBodies(responses: Record<number, SdkHttpResponse>): SdkType[] {
+export function getAllResponseBodies(
+  responses: Map<HttpStatusCodeRange | number | "*", SdkHttpResponse>
+): SdkType[] {
   return getAllResponseBodiesAndNonBodyExists(responses).allResponseBodies;
 }
 

diff --git a/packages/typespec-client-generator-core/src/package.ts b/packages/typespec-client-generator-core/src/package.ts
@@ -8,7 +8,12 @@ import {
   getService,
   isErrorModel,
 } from "@typespec/compiler";
-import { HttpOperation, getHeaderFieldName, isContentTypeHeader } from "@typespec/http";
+import {
+  HttpOperation,
+  HttpStatusCodeRange,
+  getHeaderFieldName,
+  isContentTypeHeader,
+} from "@typespec/http";
 import { resolveVersions } from "@typespec/versioning";
 import {
   getAccess,
@@ -197,7 +202,7 @@ function getSdkHttpOperation<TOptions extends object>(
   methodParameters: SdkMethodParameter[]
 ): [SdkHttpOperation, readonly Diagnostic[]] {
   const diagnostics = createDiagnosticCollector();
-  const [responses, exceptions] = diagnostics.pipe(
+  const { responses, exceptions } = diagnostics.pipe(
     getSdkServiceResponseAndExceptions<TOptions, SdkHttpOperation>(context, httpOperation)
   );
   const parameters = httpOperation.parameters.parameters
@@ -236,8 +241,8 @@ function getSdkHttpOperation<TOptions extends object>(
       });
     }
   }
-  const responsesWithBodies = Object.values(responses)
-    .concat(Object.values(exceptions))
+  const responsesWithBodies = [...responses.values()]
+    .concat([...exceptions.values()])
     .filter((r) => r.type);
   if (responsesWithBodies.length > 0 && !headerParams.some((h) => isAcceptHeader(h))) {
     // Always have an accept header if we're returning any response with a body
@@ -416,12 +421,15 @@ function getSdkServiceResponseAndExceptions<
   context: SdkContext<TOptions, TServiceOperation>,
   httpOperation: HttpOperation
 ): [
-  [Record<number | string, SdkHttpResponse>, Record<number | string, SdkHttpResponse>],
+  {
+    responses: Map<HttpStatusCodeRange | number, SdkHttpResponse>;
+    exceptions: Map<HttpStatusCodeRange | number | "*", SdkHttpResponse>;
+  },
   readonly Diagnostic[],
 ] {
   const diagnostics = createDiagnosticCollector();
-  const responses: Record<number | string, SdkHttpResponse> = {};
-  const exceptions: Record<number | string | "*", SdkHttpResponse> = {};
+  const responses: Map<HttpStatusCodeRange | number, SdkHttpResponse> = new Map();
+  const exceptions: Map<HttpStatusCodeRange | number | "*", SdkHttpResponse> = new Map();
   for (const response of httpOperation.responses) {
     const headers: SdkServiceResponseHeader[] = [];
     let body: Type | undefined;
@@ -465,19 +473,13 @@ function getSdkServiceResponseAndExceptions<
       apiVersions: getAvailableApiVersions(context, httpOperation.operation),
       nullable: body ? isNullable(body) : true,
     };
-    let statusCode: number | string = "";
-    if (typeof response.statusCodes === "number" || response.statusCodes === "*") {
-      statusCode = response.statusCodes;
-    } else {
-      statusCode = `${response.statusCodes.start}-${response.statusCodes.end}`;
-    }
-    if (statusCode === "*" || (body && isErrorModel(context.program, body))) {
-      exceptions[statusCode] = sdkResponse;
+    if (response.statusCodes === "*" || (body && isErrorModel(context.program, body))) {
+      exceptions.set(response.statusCodes, sdkResponse);
     } else {
-      responses[statusCode] = sdkResponse;
+      responses.set(response.statusCodes, sdkResponse);
     }
   }
-  return diagnostics.wrap([responses, exceptions]);
+  return diagnostics.wrap({ responses, exceptions });
 }
 
 function getParameterMappingHelper<

diff --git a/packages/typespec-client-generator-core/test/package.test.ts b/packages/typespec-client-generator-core/test/package.test.ts
@@ -941,7 +941,7 @@ describe("typespec-client-generator-core: package", () => {
 
       const serviceOperation = method.operation;
       strictEqual(serviceOperation.bodyParams.length, 0);
-      strictEqual(serviceOperation.exceptions["*"], undefined);
+      strictEqual(serviceOperation.exceptions.get("*"), undefined);
 
       strictEqual(serviceOperation.parameters.length, 1);
       const pathParam = serviceOperation.parameters[0];
@@ -1010,7 +1010,7 @@ describe("typespec-client-generator-core: package", () => {
 
       const serviceOperation = method.operation;
       strictEqual(serviceOperation.bodyParams.length, 0);
-      strictEqual(serviceOperation.exceptions["*"], undefined);
+      strictEqual(serviceOperation.exceptions.get("*"), undefined);
 
       strictEqual(serviceOperation.parameters.length, 1);
       const headerParam = serviceOperation.parameters[0];
@@ -1094,7 +1094,7 @@ describe("typespec-client-generator-core: package", () => {
 
       const serviceOperation = method.operation;
       strictEqual(serviceOperation.bodyParams.length, 0);
-      strictEqual(serviceOperation.exceptions["*"], undefined);
+      strictEqual(serviceOperation.exceptions.get("*"), undefined);
 
       strictEqual(serviceOperation.parameters.length, 1);
       const queryParam = serviceOperation.parameters[0];
@@ -1651,8 +1651,9 @@ describe("typespec-client-generator-core: package", () => {
       strictEqual(serviceContentTypeParam.type.value, "application/json");
       strictEqual(serviceContentTypeParam.type.valueType.kind, "string");
 
-      strictEqual(Object.keys(serviceOperation.responses).length, 1);
-      const response = serviceOperation.responses[200];
+      strictEqual(serviceOperation.responses.size, 1);
+      const response = serviceOperation.responses.get(200);
+      ok(response);
       strictEqual(response.kind, "http");
       strictEqual(response.type, sdkPackage.models[0]);
       strictEqual(response.contentTypes?.length, 1);
@@ -1694,8 +1695,9 @@ describe("typespec-client-generator-core: package", () => {
       strictEqual(serviceContentTypeParam.type.value, "image/png");
       strictEqual(serviceContentTypeParam.type.valueType.kind, "string");
 
-      strictEqual(Object.keys(serviceOperation.responses).length, 1);
-      const response = serviceOperation.responses[200];
+      strictEqual(serviceOperation.responses.size, 1);
+      const response = serviceOperation.responses.get(200);
+      ok(response);
       strictEqual(response.kind, "http");
       strictEqual(sdkPackage.models.length, 0);
       strictEqual(response.contentTypes?.length, 1);
@@ -1724,14 +1726,16 @@ describe("typespec-client-generator-core: package", () => {
       strictEqual(sdkPackage.models.length, 1);
       strictEqual(method.name, "delete");
       const serviceResponses = method.operation.responses;
-      strictEqual(Object.keys(serviceResponses).length, 1);
+      strictEqual(serviceResponses.size, 1);
 
-      const voidResponse = serviceResponses[204];
+      const voidResponse = serviceResponses.get(204);
+      ok(voidResponse);
       strictEqual(voidResponse.kind, "http");
       strictEqual(voidResponse.type, undefined);
       strictEqual(voidResponse.headers.length, 0);
 
-      const errorResponse = method.operation.exceptions["*"];
+      const errorResponse = method.operation.exceptions.get("*");
+      ok(errorResponse);
       strictEqual(errorResponse.kind, "http");
       ok(errorResponse.type);
       strictEqual(errorResponse.type.kind, "model");
@@ -1765,17 +1769,19 @@ describe("typespec-client-generator-core: package", () => {
       strictEqual(sdkPackage.models.length, 2);
       strictEqual(method.name, "create");
       const serviceResponses = method.operation.responses;
-      strictEqual(Object.keys(serviceResponses).length, 1);
+      strictEqual(serviceResponses.size, 1);
 
-      const createResponse = serviceResponses[200];
+      const createResponse = serviceResponses.get(200);
+      ok(createResponse);
       strictEqual(createResponse.kind, "http");
       strictEqual(
         createResponse.type,
         sdkPackage.models.find((x) => x.name === "Widget")
       );
       strictEqual(createResponse.headers.length, 0);
 
-      const errorResponse = method.operation.exceptions["*"];
+      const errorResponse = method.operation.exceptions.get("*");
+      ok(errorResponse);
       strictEqual(errorResponse.kind, "http");
       ok(errorResponse.type);
       strictEqual(errorResponse.type.kind, "model");
@@ -1806,11 +1812,12 @@ describe("typespec-client-generator-core: package", () => {
       strictEqual(sdkPackage.models.length, 1);
       strictEqual(method.name, "operation");
       const serviceResponses = method.operation.responses;
-      strictEqual(Object.keys(serviceResponses).length, 1);
+      strictEqual(serviceResponses.size, 1);
 
       strictEqual(method.parameters.length, 1);
 
-      const createResponse = serviceResponses[200];
+      const createResponse = serviceResponses.get(200);
+      ok(createResponse);
       strictEqual(createResponse.kind, "http");
       strictEqual(
         createResponse.type,
@@ -1852,7 +1859,8 @@ describe("typespec-client-generator-core: package", () => {
       const method = getServiceMethodOfClient(sdkPackage);
       const serviceResponses = method.operation.responses;
 
-      const createResponse = serviceResponses[200];
+      const createResponse = serviceResponses.get(200);
+      ok(createResponse);
       strictEqual(createResponse.headers[0].nullable, true);
       strictEqual(createResponse.nullable, true);
 
@@ -1873,10 +1881,12 @@ describe("typespec-client-generator-core: package", () => {
       const method = getServiceMethodOfClient(sdkPackage);
       const serviceResponses = method.operation.responses;
 
-      const okResponse = serviceResponses[200];
+      const okResponse = serviceResponses.get(200);
+      ok(okResponse);
       strictEqual(okResponse.nullable, false);
 
-      const noContentResponse = serviceResponses[204];
+      const noContentResponse = serviceResponses.get(204);
+      ok(noContentResponse);
       strictEqual(noContentResponse.nullable, true);
 
       strictEqual(method.response.nullable, true);
@@ -1894,9 +1904,10 @@ describe("typespec-client-generator-core: package", () => {
       strictEqual(method.name, "delete");
       strictEqual(method.response.nullable, true);
       const serviceResponses = method.operation.responses;
-      strictEqual(Object.keys(serviceResponses).length, 1);
+      strictEqual(serviceResponses.size, 1);
 
-      const voidResponse = serviceResponses[204];
+      const voidResponse = serviceResponses.get(204);
+      ok(voidResponse);
       strictEqual(voidResponse.kind, "http");
       strictEqual(voidResponse.type, undefined);
       strictEqual(voidResponse.headers.length, 0);
@@ -2640,28 +2651,31 @@ describe("typespec-client-generator-core: package", () => {
       strictEqual(serviceOperation.bodyParams[0].name, "resource");
       strictEqual(serviceOperation.bodyParams[0].type, widgetModel);
 
-      strictEqual(Object.keys(serviceOperation.responses).length, 2);
+      strictEqual(serviceOperation.responses.size, 2);
       const responseHeaders = [
         "Repeatability-Result",
         "ETag",
         "x-ms-client-request-id",
         "Operation-Location",
       ];
-      const response200 = serviceOperation.responses[200];
+      const response200 = serviceOperation.responses.get(200);
+      ok(response200);
       deepStrictEqual(
         response200.headers.map((x) => x.serializedName),
         responseHeaders
       );
       strictEqual(response200.type, widgetModel);
 
-      const response201 = serviceOperation.responses[201];
+      const response201 = serviceOperation.responses.get(201);
+      ok(response201);
       deepStrictEqual(
         response201.headers.map((x) => x.serializedName),
         responseHeaders
       );
       strictEqual(response201.type, widgetModel);
 
-      const exception = serviceOperation.exceptions["*"];
+      const exception = serviceOperation.exceptions.get("*");
+      ok(exception);
       strictEqual(exception.kind, "http");
       ok(exception.type);
       strictEqual(exception.type.kind, "model");
@@ -2744,8 +2758,9 @@ describe("typespec-client-generator-core: package", () => {
         listManufacturers.parameters[1]
       );
 
-      strictEqual(Object.keys(operation.responses).length, 1);
-      const response200 = operation.responses[200];
+      strictEqual(operation.responses.size, 1);
+      const response200 = operation.responses.get(200);
+      ok(response200);
       strictEqual(response200.kind, "http");
       const pagingModel = response200.type;
       ok(pagingModel);