From e07e59fa1510ad3f84d5f07d542e0e575e107310 Mon Sep 17 00:00:00 2001 From: iscai-msft Date: Mon, 25 Mar 2024 17:15:42 -0400 Subject: [PATCH 1/2] get tests passing --- .../src/interfaces.ts | 5 +- .../src/internal-utils.ts | 12 ++-- .../src/package.ts | 36 +++++----- .../test/package.test.ts | 67 ++++++++++++------- 4 files changed, 71 insertions(+), 49 deletions(-) diff --git a/packages/typespec-client-generator-core/src/interfaces.ts b/packages/typespec-client-generator-core/src/interfaces.ts index e81c9c1606..3d851178b3 100644 --- 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; // we will use string to represent status code range - exceptions: Record; // we will use string to represent status code range + responses: Map; + exceptions: Map; } /** diff --git a/packages/typespec-client-generator-core/src/internal-utils.ts b/packages/typespec-client-generator-core/src/internal-utils.ts index e5d92c84f7..5995ad4a23 100644 --- 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): { +function getAllResponseBodiesAndNonBodyExists( + responses: Map +): { 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): SdkType[] { +export function getAllResponseBodies( + responses: Map +): 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 index 1997b2afd1..6e9aa4a19b 100644 --- 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( methodParameters: SdkMethodParameter[] ): [SdkHttpOperation, readonly Diagnostic[]] { const diagnostics = createDiagnosticCollector(); - const [responses, exceptions] = diagnostics.pipe( + const { responses, exceptions } = diagnostics.pipe( getSdkServiceResponseAndExceptions(context, httpOperation) ); const parameters = httpOperation.parameters.parameters @@ -236,8 +241,8 @@ function getSdkHttpOperation( }); } } - 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, httpOperation: HttpOperation ): [ - [Record, Record], + { + responses: Map; + exceptions: Map; + }, readonly Diagnostic[], ] { const diagnostics = createDiagnosticCollector(); - const responses: Record = {}; - const exceptions: Record = {}; + const responses: Map = new Map(); + const exceptions: Map = 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 index 7999129f4f..cf31bf5572 100644 --- 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,9 +1769,10 @@ 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, @@ -1775,7 +1780,8 @@ describe("typespec-client-generator-core: package", () => { ); 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); From 9d88ad029bd51d74e7a7906a2f30b308503a661b Mon Sep 17 00:00:00 2001 From: iscai-msft Date: Mon, 25 Mar 2024 17:16:14 -0400 Subject: [PATCH 2/2] add changeset --- .chronus/changes/response_structure-2024-2-25-17-16-10.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .chronus/changes/response_structure-2024-2-25-17-16-10.md 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 new file mode 100644 index 0000000000..958516cd79 --- /dev/null +++ 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 \ No newline at end of file