From 0f1e8e5f68b677c1d437f04ac7eb18f3707be3a2 Mon Sep 17 00:00:00 2001 From: iscai-msft Date: Tue, 26 Mar 2024 18:15:43 -0400 Subject: [PATCH 1/4] create http file and add corresponding method params onto params --- .../src/http.ts | 450 ++++++++++++++++++ .../src/interfaces.ts | 10 +- .../src/package.ts | 411 ++++------------ .../src/types.ts | 240 ++-------- .../test/package.test.ts | 139 +++--- 5 files changed, 654 insertions(+), 596 deletions(-) create mode 100644 packages/typespec-client-generator-core/src/http.ts diff --git a/packages/typespec-client-generator-core/src/http.ts b/packages/typespec-client-generator-core/src/http.ts new file mode 100644 index 0000000000..2eba12ca0b --- /dev/null +++ b/packages/typespec-client-generator-core/src/http.ts @@ -0,0 +1,450 @@ +import { + Diagnostic, + ModelProperty, + Type, + createDiagnosticCollector, + ignoreDiagnostics, + isErrorModel, +} from "@typespec/compiler"; +import { + HttpOperation, + getHeaderFieldName, + getHeaderFieldOptions, + getPathParamName, + getQueryParamName, + getQueryParamOptions, + isBody, + isContentTypeHeader, + isHeader, + isPathParam, + isQueryParam, +} from "@typespec/http"; +import { + CollectionFormat, + SdkBodyParameter, + SdkHeaderParameter, + SdkHttpOperation, + SdkHttpParameter, + SdkHttpResponse, + SdkMethodParameter, + SdkModelPropertyType, + SdkParameter, + SdkPathParameter, + SdkQueryParameter, + SdkServiceResponseHeader, + SdkType, +} from "./interfaces.js"; +import { + TCGCContext, + getAvailableApiVersions, + getDocHelper, + isAcceptHeader, + isNullable, +} from "./internal-utils.js"; +import { + addEncodeInfo, + addFormatInfo, + getClientTypeWithDiagnostics, + getSdkModelPropertyTypeBase, +} from "./types.js"; + +export function getSdkHttpOperation( + context: TCGCContext, + httpOperation: HttpOperation, + methodParameters: SdkMethodParameter[] +): [SdkHttpOperation, readonly Diagnostic[]] { + const diagnostics = createDiagnosticCollector(); + const [responses, exceptions] = diagnostics.pipe( + getSdkHttpResponseAndExceptions(context, httpOperation) + ); + const responsesWithBodies = Object.values(responses) + .concat(Object.values(exceptions)) + .filter((r) => r.type); + const parameters = diagnostics.pipe( + getSdkHttpParameters(context, httpOperation, methodParameters, responsesWithBodies[0]) + ); + return diagnostics.wrap({ + __raw: httpOperation, + kind: "http", + path: httpOperation.path, + verb: httpOperation.verb, + ...parameters, + responses, + exceptions, + }); +} + +export function isSdkHttpParameter(context: TCGCContext, type: ModelProperty): boolean { + const program = context.program; + return ( + isPathParam(program, type) || + isQueryParam(program, type) || + isHeader(program, type) || + isBody(program, type) + ); +} + +interface SdkHttpParameters { + parameters: (SdkPathParameter | SdkQueryParameter | SdkHeaderParameter)[]; + bodyParam?: SdkBodyParameter; +} + +function getSdkHttpParameters( + context: TCGCContext, + httpOperation: HttpOperation, + methodParameters: SdkMethodParameter[], + responseBody?: SdkHttpResponse +): [SdkHttpParameters, readonly Diagnostic[]] { + const diagnostics = createDiagnosticCollector(); + const retval: SdkHttpParameters = { + parameters: [], + bodyParam: undefined, + }; + retval.parameters = httpOperation.parameters.parameters + .map((x) => diagnostics.pipe(getSdkHttpParameter(context, x.param))) + .filter( + (x): x is SdkHeaderParameter | SdkQueryParameter | SdkPathParameter => + x.kind === "header" || x.kind === "query" || x.kind === "path" + ); + const headerParams = retval.parameters.filter( + (x): x is SdkHeaderParameter => x.kind === "header" + ); + // add operation info onto body param + const tspBody = httpOperation.parameters.body; + // we add correspondingMethodParams after we create the type, since we need the info on the type + const correspondingMethodParams: SdkModelPropertyType[] = []; + if (tspBody) { + // if there's a param on the body, we can just rely on getSdkHttpParameter + if (tspBody.parameter) { + const getParamResponse = diagnostics.pipe(getSdkHttpParameter(context, tspBody.parameter)); + if (getParamResponse.kind !== "body") throw new Error("blah"); + retval.bodyParam = getParamResponse; + } else { + retval.bodyParam = { + kind: "body", + name: "body", + nameInClient: "body", + description: getDocHelper(context, tspBody.type).description, + details: getDocHelper(context, tspBody.type).details, + onClient: false, + contentTypes: [], + defaultContentType: "application/json", // actual content type info is added later + isApiVersionParam: false, + apiVersions: getAvailableApiVersions(context, tspBody.type), + type: diagnostics.pipe( + getClientTypeWithDiagnostics(context, tspBody.type, httpOperation.operation) + ), + optional: false, + nullable: isNullable(tspBody.type), + correspondingMethodParams, + }; + } + addContentTypeInfoToBodyParam(context, httpOperation, retval.bodyParam); + retval.bodyParam.correspondingMethodParams = getCorrespondingMethodParams( + context, + methodParameters, + retval.bodyParam + ); + } + if ( + retval.bodyParam && + !headerParams.some((h) => h.__raw && isContentTypeHeader(context.program, h.__raw)) + ) { + // if we have a body param and no content type header, we add one + const contentTypeBase = { + ...createContentTypeOrAcceptHeader(retval.bodyParam), + description: `Body parameter's content type. Known values are ${retval.bodyParam.contentTypes}`, + }; + if (!methodParameters.some((m) => m.__raw && isContentTypeHeader(context.program, m.__raw))) { + methodParameters.push({ + ...contentTypeBase, + kind: "method", + }); + } + retval.parameters.push({ + ...contentTypeBase, + kind: "header", + serializedName: "Content-Type", + correspondingMethodParams, + }); + } + if (responseBody && !headerParams.some((h) => isAcceptHeader(h))) { + // If our operation returns a body, we add an accept header if none exist + const acceptBase = { + ...createContentTypeOrAcceptHeader(responseBody), + }; + if (!methodParameters.some((m) => m.name === "accept")) { + methodParameters.push({ + ...acceptBase, + kind: "method", + }); + } + retval.parameters.push({ + ...acceptBase, + kind: "header", + serializedName: "Accept", + correspondingMethodParams, + }); + } + for (const param of retval.parameters) { + param.correspondingMethodParams = getCorrespondingMethodParams( + context, + methodParameters, + param + ); + } + return diagnostics.wrap(retval); +} + +function createContentTypeOrAcceptHeader( + bodyObject: SdkBodyParameter | SdkHttpResponse +): Omit { + const name = bodyObject.kind === "body" ? "contentType" : "accept"; + let type: SdkType = { + kind: "string", + encode: "string", + nullable: false, + }; + // for contentType, we treat it as a constant IFF there's one value and it's application/json. + // this is to prevent a breaking change when a service adds more content types in the future. + // e.g. the service accepting image/png then later image/jpeg should _not_ be a breaking change. + // + // for accept, we treat it as a constant IFF there's a single value. adding more content types + // for this case is considered a breaking change for SDKs so we want to surface it as such. + // e.g. the service returns image/png then later provides the option to return image/jpeg. + if ( + bodyObject.contentTypes && + bodyObject.contentTypes.length === 1 && + (/json/.test(bodyObject.contentTypes[0]) || name === "accept") + ) { + // in this case, we just want a content type of application/json + type = { + nullable: false, + kind: "constant", + value: bodyObject.contentTypes[0], + valueType: type, + }; + } + // No need for clientDefaultValue because it's a constant, it only has one value + return { + type, + nameInClient: name, + name, + apiVersions: bodyObject.apiVersions, + isApiVersionParam: false, + onClient: false, + optional: false, + nullable: false, + }; +} + +function addContentTypeInfoToBodyParam( + context: TCGCContext, + httpOperation: HttpOperation, + bodyParam: SdkBodyParameter +): readonly Diagnostic[] { + const diagnostics = createDiagnosticCollector(); + const tspBody = httpOperation.parameters.body; + if (!tspBody) return diagnostics.diagnostics; + let contentTypes = tspBody.contentTypes; + if (contentTypes.length === 0) { + contentTypes = ["application/json"]; + } + const defaultContentType = contentTypes.includes("application/json") + ? "application/json" + : contentTypes[0]; + bodyParam.contentTypes = contentTypes; + bodyParam.defaultContentType = defaultContentType; + diagnostics.pipe(addEncodeInfo(context, bodyParam.__raw!, bodyParam.type, defaultContentType)); + return diagnostics.diagnostics; +} + +export function getSdkHttpParameter( + context: TCGCContext, + type: ModelProperty, + isMethodParameter: boolean = false +): [SdkHttpParameter, readonly Diagnostic[]] { + const diagnostics = createDiagnosticCollector(); + const base = diagnostics.pipe(getSdkModelPropertyTypeBase(context, type)); + const program = context.program; + const correspondingMethodParams: SdkParameter[] = []; // we set it later in the operation + if (isPathParam(context.program, type) || isMethodParameter) { + // we don't url encode if the type can be assigned to url + const urlEncode = !ignoreDiagnostics( + program.checker.isTypeAssignableTo( + type.type.projectionBase ?? type.type, + program.checker.getStdType("url"), + type.type + ) + ); + return diagnostics.wrap({ + ...base, + kind: "path", + urlEncode, + serializedName: getPathParamName(program, type), + correspondingMethodParams, + optional: false, + }); + } + if (isBody(context.program, type)) { + return diagnostics.wrap({ + ...base, + kind: "body", + contentTypes: ["application/json"], // will update when we get to the operation level + defaultContentType: "application/json", // will update when we get to the operation level + optional: type.optional, + correspondingMethodParams, + }); + } + const headerQueryBase = { + ...base, + optional: type.optional, + collectionFormat: getCollectionFormat(context, type), + correspondingMethodParams, + }; + if (isQueryParam(context.program, type)) { + return diagnostics.wrap({ + ...headerQueryBase, + kind: "query", + serializedName: getQueryParamName(program, type), + }); + } + // has to be a header + return diagnostics.wrap({ + ...headerQueryBase, + kind: "header", + serializedName: getHeaderFieldName(program, type), + }); +} + +function getSdkHttpResponseAndExceptions( + context: TCGCContext, + httpOperation: HttpOperation +): [ + [Record, Record], + readonly Diagnostic[], +] { + const diagnostics = createDiagnosticCollector(); + const responses: Record = {}; + const exceptions: Record = {}; + for (const response of httpOperation.responses) { + const headers: SdkServiceResponseHeader[] = []; + let body: Type | undefined; + let contentTypes: string[] = []; + + for (const innerResponse of response.responses) { + for (const header of Object.values(innerResponse.headers || [])) { + const clientType = diagnostics.pipe(getClientTypeWithDiagnostics(context, header.type)); + const defaultContentType = innerResponse.body?.contentTypes.includes("application/json") + ? "application/json" + : innerResponse.body?.contentTypes[0]; + addEncodeInfo(context, header, clientType, defaultContentType); + addFormatInfo(context, header, clientType); + headers.push({ + __raw: header, + description: getDocHelper(context, header).description, + details: getDocHelper(context, header).details, + serializedName: getHeaderFieldName(context.program, header), + type: clientType, + nullable: isNullable(header.type), + }); + } + if (innerResponse.body) { + if (body && body !== innerResponse.body.type) { + throw new Error("blah"); + } + contentTypes = contentTypes.concat(innerResponse.body.contentTypes); + body = innerResponse.body.type; + } + } + + const sdkResponse: SdkHttpResponse = { + __raw: response, + kind: "http", + type: body ? diagnostics.pipe(getClientTypeWithDiagnostics(context, body)) : undefined, + headers, + contentTypes, + defaultContentType: contentTypes.includes("application/json") + ? "application/json" + : contentTypes[0], + 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; + } else { + responses[statusCode] = sdkResponse; + } + } + return diagnostics.wrap([responses, exceptions]); +} + +export function getCorrespondingMethodParams( + context: TCGCContext, + methodParameters: SdkParameter[], + serviceParam: SdkHttpParameter +): SdkModelPropertyType[] { + if (serviceParam.isApiVersionParam) { + if (!context.__api_version_parameter) throw new Error("No api version on the client"); + return [context.__api_version_parameter]; + } + const correspondingMethodParameter = methodParameters.find((x) => x.name === serviceParam.name); + if (correspondingMethodParameter) { + return [correspondingMethodParameter]; + } + function paramInProperties(param: SdkModelPropertyType, type: SdkType): boolean { + if (type.kind !== "model") return false; + return Array.from(type.properties.values()) + .filter((x) => x.kind === "property") + .map((x) => x.name) + .includes(param.name); + } + const serviceParamType = serviceParam.type; + if (serviceParam.kind === "body" && serviceParamType.kind === "model") { + // Here we have a spread body parameter + const correspondingProperties = methodParameters.filter((x) => + paramInProperties(x, serviceParamType) + ); + const bodyPropertyNames = serviceParamType.properties.filter((x) => + paramInProperties(x, serviceParamType) + ); + if (correspondingProperties.length !== bodyPropertyNames.length) { + throw new Error("Can't find corresponding properties for spread body parameter"); + } + return correspondingProperties; + } + for (const methodParam of methodParameters) { + if (methodParam.type.kind === "model") { + for (const prop of methodParam.type.properties) { + if (prop.name === serviceParam.name) { + return [prop]; + } + } + } + } + throw new Error("Can't find corresponding parameter"); +} + +function getCollectionFormat( + context: TCGCContext, + type: ModelProperty +): CollectionFormat | undefined { + const program = context.program; + const tspCollectionFormat = ( + isQueryParam(program, type) + ? getQueryParamOptions(program, type) + : isHeader(program, type) + ? getHeaderFieldOptions(program, type) + : undefined + )?.format; + if (tspCollectionFormat === "form" || tspCollectionFormat === "simple") { + return undefined; + } + return tspCollectionFormat; +} diff --git a/packages/typespec-client-generator-core/src/interfaces.ts b/packages/typespec-client-generator-core/src/interfaces.ts index e81c9c1606..bb556e8d49 100644 --- a/packages/typespec-client-generator-core/src/interfaces.ts +++ b/packages/typespec-client-generator-core/src/interfaces.ts @@ -368,12 +368,14 @@ export interface SdkHeaderParameter extends SdkModelPropertyTypeBase { kind: "header"; collectionFormat?: CollectionFormat; serializedName: string; + correspondingMethodParams: SdkModelPropertyType[]; } export interface SdkQueryParameter extends SdkModelPropertyTypeBase { kind: "query"; collectionFormat?: CollectionFormat; serializedName: string; + correspondingMethodParams: SdkModelPropertyType[]; } export interface SdkPathParameter extends SdkModelPropertyTypeBase { @@ -381,6 +383,7 @@ export interface SdkPathParameter extends SdkModelPropertyTypeBase { urlEncode: boolean; serializedName: string; optional: false; + correspondingMethodParams: SdkModelPropertyType[]; } export interface SdkBodyParameter extends SdkModelPropertyTypeBase { @@ -388,6 +391,7 @@ export interface SdkBodyParameter extends SdkModelPropertyTypeBase { optional: boolean; contentTypes: string[]; defaultContentType: string; + correspondingMethodParams: SdkModelPropertyType[]; } export type SdkHttpParameter = @@ -439,7 +443,7 @@ export interface SdkHttpOperation extends SdkServiceOperationBase { path: string; verb: HttpVerb; parameters: (SdkPathParameter | SdkQueryParameter | SdkHeaderParameter)[]; - bodyParams: SdkBodyParameter[]; // array for cases like urlencoded / multipart + bodyParam?: 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 } @@ -463,6 +467,10 @@ interface SdkMethodBase { interface SdkServiceMethodBase extends SdkMethodBase { + /** + * @deprecated This property is deprecated. Access .correspondingMethodParams on the service parameters instead + * @param serviceParam + */ getParameterMapping(serviceParam: SdkServiceParameter): SdkModelPropertyType[]; operation: TServiceOperation; parameters: SdkMethodParameter[]; diff --git a/packages/typespec-client-generator-core/src/package.ts b/packages/typespec-client-generator-core/src/package.ts index 1997b2afd1..66d4d43814 100644 --- a/packages/typespec-client-generator-core/src/package.ts +++ b/packages/typespec-client-generator-core/src/package.ts @@ -1,14 +1,13 @@ import { getLroMetadata, getPagedResult } from "@azure-tools/typespec-azure-core"; import { Diagnostic, + ModelProperty, Operation, - Type, createDiagnosticCollector, getNamespaceFullName, getService, - isErrorModel, } from "@typespec/compiler"; -import { HttpOperation, getHeaderFieldName, isContentTypeHeader } from "@typespec/http"; +import { getServers } from "@typespec/http"; import { resolveVersions } from "@typespec/versioning"; import { getAccess, @@ -16,15 +15,14 @@ import { listOperationGroups, listOperationsInOperationGroup, } from "./decorators.js"; +import { getCorrespondingMethodParams, getSdkHttpOperation, getSdkHttpParameter } from "./http.js"; import { - SdkBodyParameter, SdkClient, SdkClientType, SdkContext, + SdkEndpointParameter, + SdkEndpointType, SdkEnumType, - SdkHeaderParameter, - SdkHttpOperation, - SdkHttpResponse, SdkInitializationType, SdkLroPagingServiceMethod, SdkLroServiceMethod, @@ -38,7 +36,6 @@ import { SdkPagingServiceMethod, SdkParameter, SdkPathParameter, - SdkQueryParameter, SdkServiceMethod, SdkServiceOperation, SdkServiceParameter, @@ -47,227 +44,29 @@ import { UsageFlags, } from "./interfaces.js"; import { + TCGCContext, createGeneratedName, getAllResponseBodies, getAvailableApiVersions, getClientNamespaceStringHelper, getDocHelper, getHashForType, - isAcceptHeader, isNullable, } from "./internal-utils.js"; +import { createDiagnostic } from "./lib.js"; import { getClientNamespaceString, getDefaultApiVersion, getHttpOperationWithCache, getLibraryName, - getPropertyNames, } from "./public-utils.js"; import { - addEncodeInfo, - addFormatInfo, getAllModelsWithDiagnostics, getClientTypeWithDiagnostics, getSdkCredentialParameter, - getSdkEndpointParameter, getSdkModelPropertyType, } from "./types.js"; -function getSdkHttpBodyParameters( - context: SdkContext, - httpOperation: HttpOperation, - methodParameters: SdkParameter[] -): [SdkBodyParameter[], readonly Diagnostic[]] { - const diagnostics = createDiagnosticCollector(); - const tspBody = httpOperation.parameters.body; - if (tspBody === undefined) return diagnostics.wrap([]); - let contentTypes = tspBody.contentTypes; - if (contentTypes.length === 0) { - contentTypes = ["application/json"]; - } - const defaultContentType = contentTypes.includes("application/json") - ? "application/json" - : contentTypes[0]; - if (!tspBody.parameter) { - const bodyType = diagnostics.pipe( - getClientTypeWithDiagnostics(context, tspBody.type, httpOperation.operation) - ); - const name = "body"; - return diagnostics.wrap([ - { - kind: "body", - nameInClient: name, - name, - description: getDocHelper(context, tspBody.type).description, - details: getDocHelper(context, tspBody.type).details, - onClient: false, - contentTypes: contentTypes, - defaultContentType: defaultContentType, - isApiVersionParam: false, - apiVersions: getAvailableApiVersions(context, tspBody.type), - type: bodyType, - optional: false, - nullable: isNullable(tspBody.type), - }, - ]); - } - const body = diagnostics.pipe( - getSdkModelPropertyType(context, tspBody.parameter!, { - operation: httpOperation.operation, - defaultContentType: defaultContentType, - }) - ); - const methodBodyParameter = methodParameters.find( - (x) => x.name === getPropertyNames(context, tspBody.parameter!)[0] - ); - if (body.kind !== "body") throw new Error("blah"); - if (methodBodyParameter) { - return diagnostics.wrap([ - { - ...body, - contentTypes, - defaultContentType, - }, - ]); - } else { - // this means that the body parameter is a property on one of the method parameters - for (const methodParameter of methodParameters) { - if (methodParameter.type.kind === "model") { - const bodyProperty = methodParameter.type.properties.find((x) => x.kind === "body"); - if (bodyProperty) { - return diagnostics.wrap([ - { - ...body, - contentTypes, - defaultContentType, - }, - ]); - } - } - } - } - throw new Error("blah"); -} - -function createContentTypeOrAcceptHeader( - bodyObject: SdkBodyParameter | SdkHttpResponse -): Omit { - const name = bodyObject.kind === "body" ? "contentType" : "accept"; - let type: SdkType = { - kind: "string", - encode: "string", - nullable: false, - }; - // for contentType, we treat it as a constant IFF there's one value and it's application/json. - // this is to prevent a breaking change when a service adds more content types in the future. - // e.g. the service accepting image/png then later image/jpeg should _not_ be a breaking change. - // - // for accept, we treat it as a constant IFF there's a single value. adding more content types - // for this case is considered a breaking change for SDKs so we want to surface it as such. - // e.g. the service returns image/png then later provides the option to return image/jpeg. - if ( - bodyObject.contentTypes && - bodyObject.contentTypes.length === 1 && - (/json/.test(bodyObject.contentTypes[0]) || name === "accept") - ) { - // in this case, we just want a content type of application/json - type = { - nullable: false, - kind: "constant", - value: bodyObject.contentTypes[0], - valueType: type, - }; - } - // No need for clientDefaultValue because it's a constant, it only has one value - return { - type, - nameInClient: name, - name, - apiVersions: bodyObject.apiVersions, - isApiVersionParam: false, - onClient: false, - optional: false, - nullable: false, - }; -} - -function getSdkHttpOperation( - context: SdkContext, - httpOperation: HttpOperation, - methodParameters: SdkMethodParameter[] -): [SdkHttpOperation, readonly Diagnostic[]] { - const diagnostics = createDiagnosticCollector(); - const [responses, exceptions] = diagnostics.pipe( - getSdkServiceResponseAndExceptions(context, httpOperation) - ); - const parameters = httpOperation.parameters.parameters - .map((x) => - diagnostics.pipe( - getSdkModelPropertyType(context, x.param, { operation: httpOperation.operation }) - ) - ) - .filter( - (x): x is SdkHeaderParameter | SdkQueryParameter | SdkPathParameter => - x.kind === "header" || x.kind === "query" || x.kind === "path" - ); - const headerParams = parameters.filter((x): x is SdkHeaderParameter => x.kind === "header"); - const bodyParams = diagnostics.pipe( - getSdkHttpBodyParameters(context, httpOperation, methodParameters) - ); - - if ( - bodyParams.length && - !headerParams.some((h) => h.__raw && isContentTypeHeader(context.program, h.__raw)) - ) { - // We will always add a content type parameter if a body is being inputted - const contentTypeBase = { - ...createContentTypeOrAcceptHeader(bodyParams[0]), - description: `Body parameter's content type. Known values are ${bodyParams[0].contentTypes}`, - }; - parameters.push({ - ...contentTypeBase, - kind: "header", - serializedName: "Content-Type", - }); - if (!methodParameters.some((m) => m.__raw && isContentTypeHeader(context.program, m.__raw))) { - methodParameters.push({ - ...contentTypeBase, - kind: "method", - }); - } - } - const responsesWithBodies = Object.values(responses) - .concat(Object.values(exceptions)) - .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 - const acceptBase = { - ...createContentTypeOrAcceptHeader(responsesWithBodies[0]), - }; - parameters.push({ - ...acceptBase, - kind: "header", - serializedName: "Accept", - }); - if (!methodParameters.some((m) => m.name === "accept")) { - methodParameters.push({ - ...acceptBase, - kind: "method", - }); - } - } - return diagnostics.wrap({ - __raw: httpOperation, - kind: "http", - path: httpOperation.path, - verb: httpOperation.verb, - parameters, - bodyParams: bodyParams || [], - responses, - exceptions, - }); -} - function getSdkServiceOperation< TOptions extends object, TServiceOperation extends SdkServiceOperation, @@ -409,126 +208,6 @@ function getSdkMethodResponse( }; } -function getSdkServiceResponseAndExceptions< - TOptions extends object, - TServiceOperation extends SdkServiceOperation, ->( - context: SdkContext, - httpOperation: HttpOperation -): [ - [Record, Record], - readonly Diagnostic[], -] { - const diagnostics = createDiagnosticCollector(); - const responses: Record = {}; - const exceptions: Record = {}; - for (const response of httpOperation.responses) { - const headers: SdkServiceResponseHeader[] = []; - let body: Type | undefined; - let contentTypes: string[] = []; - - for (const innerResponse of response.responses) { - for (const header of Object.values(innerResponse.headers || [])) { - const clientType = diagnostics.pipe(getClientTypeWithDiagnostics(context, header.type)); - const defaultContentType = innerResponse.body?.contentTypes.includes("application/json") - ? "application/json" - : innerResponse.body?.contentTypes[0]; - addEncodeInfo(context, header, clientType, defaultContentType); - addFormatInfo(context, header, clientType); - headers.push({ - __raw: header, - description: getDocHelper(context, header).description, - details: getDocHelper(context, header).details, - serializedName: getHeaderFieldName(context.program, header), - type: clientType, - nullable: isNullable(header.type), - }); - } - if (innerResponse.body) { - if (body && body !== innerResponse.body.type) { - throw new Error("blah"); - } - contentTypes = contentTypes.concat(innerResponse.body.contentTypes); - body = innerResponse.body.type; - } - } - - const sdkResponse: SdkHttpResponse = { - __raw: response, - kind: "http", - type: body ? diagnostics.pipe(getClientTypeWithDiagnostics(context, body)) : undefined, - headers, - contentTypes, - defaultContentType: contentTypes.includes("application/json") - ? "application/json" - : contentTypes[0], - 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; - } else { - responses[statusCode] = sdkResponse; - } - } - return diagnostics.wrap([responses, exceptions]); -} - -function getParameterMappingHelper< - TOptions extends object, - TServiceOperation extends SdkServiceOperation, ->( - context: SdkContext, - method: SdkServiceMethod, - serviceParam: SdkServiceParameter -): SdkModelPropertyType[] { - if (serviceParam.isApiVersionParam) { - if (!context.__api_version_parameter) throw new Error("No api version on the client"); - return [context.__api_version_parameter]; - } - const correspondingMethodParameter = method.parameters.find((x) => x.name === serviceParam.name); - if (correspondingMethodParameter) { - return [correspondingMethodParameter]; - } - function paramInProperties(param: SdkModelPropertyType, type: SdkType): boolean { - if (type.kind !== "model") return false; - return Array.from(type.properties.values()) - .filter((x) => x.kind === "property") - .map((x) => x.name) - .includes(param.name); - } - const serviceParamType = serviceParam.type; - if (serviceParam.kind === "body" && serviceParamType.kind === "model") { - // Here we have a spread body parameter - const correspondingProperties = method.parameters.filter((x) => - paramInProperties(x, serviceParamType) - ); - const bodyPropertyNames = serviceParamType.properties.filter((x) => - paramInProperties(x, serviceParamType) - ); - if (correspondingProperties.length !== bodyPropertyNames.length) { - throw new Error("Can't find corresponding properties for spread body parameter"); - } - return correspondingProperties; - } - for (const methodParam of method.parameters) { - if (methodParam.type.kind === "model") { - for (const prop of methodParam.type.properties) { - if (prop.name === serviceParam.name) { - return [prop]; - } - } - } - } - throw new Error("Can't find corresponding parameter"); -} - function getSdkBasicServiceMethod< TOptions extends object, TServiceOperation extends SdkServiceOperation, @@ -539,7 +218,7 @@ function getSdkBasicServiceMethod< const diagnostics = createDiagnosticCollector(); // when we spread, all of the inputtable properties of our model get flattened onto the method const methodParameters = Array.from(operation.parameters.properties.values()) - .map((x) => diagnostics.pipe(getSdkModelPropertyType(context, x, { isMethodParameter: true }))) + .map((x) => diagnostics.pipe(getSdkMethodParameter(context, x))) .filter((x): x is SdkMethodParameter => x.kind === "method"); // if there's an api version parameter, we want to bubble it up to the client // we don't want it on the method level, but we will keep it on the service operation level @@ -570,7 +249,7 @@ function getSdkBasicServiceMethod< getParameterMapping: function getParameterMapping( serviceParam: SdkServiceParameter ): SdkModelPropertyType[] { - return getParameterMappingHelper(context, this, serviceParam); + return getCorrespondingMethodParams(context, methodParameters, serviceParam); }, getResponseMapping: function getResponseMapping(): string | undefined { return undefined; // currently we only return a value for paging or lro @@ -645,6 +324,17 @@ function getSdkInitializationType< }); } +function getSdkMethodParameter( + context: TCGCContext, + type: ModelProperty +): [SdkMethodParameter, readonly Diagnostic[]] { + const diagnostics = createDiagnosticCollector(); + return diagnostics.wrap({ + ...diagnostics.pipe(getSdkModelPropertyType(context, type)), + kind: "method", + }); +} + function getSdkMethods( context: SdkContext, client: SdkClient, @@ -676,6 +366,67 @@ function getSdkMethods 1) { + // if there is no defined server url, or if there are more than one + // we will return a mandatory endpoint parameter in initialization + type = { + kind: "endpoint", + nullable: false, + templateArguments: [], + }; + } else { + // this means we have one server + const templateArguments: SdkPathParameter[] = []; + type = { + kind: "endpoint", + nullable: false, + serverUrl: servers[0].url, + templateArguments, + }; + for (const param of servers[0].parameters.values()) { + const sdkParam = diagnostics.pipe(getSdkHttpParameter(context, param, true)); + if (sdkParam.kind === "path") { + templateArguments.push(sdkParam); + sdkParam.description = sdkParam.description ?? servers[0].description; + sdkParam.onClient = true; + } else { + diagnostics.add( + createDiagnostic({ + code: "server-param-not-path", + target: param, + format: { + templateArgumentName: sdkParam.name, + templateArgumentType: sdkParam.kind, + }, + }) + ); + } + } + optional = Boolean(servers[0].url.length && templateArguments.every((param) => param.optional)); + } + return diagnostics.wrap({ + kind: "endpoint", + type, + nameInClient: "endpoint", + name: "endpoint", + description: "Service host", + onClient: true, + urlEncode: false, + apiVersions: getAvailableApiVersions(context, client.service), + optional, + isApiVersionParam: false, + nullable: false, + }); +} + function createSdkClientType< TOptions extends object, TServiceOperation extends SdkServiceOperation, diff --git a/packages/typespec-client-generator-core/src/types.ts b/packages/typespec-client-generator-core/src/types.ts index 9157e66c86..715f24c094 100644 --- a/packages/typespec-client-generator-core/src/types.ts +++ b/packages/typespec-client-generator-core/src/types.ts @@ -33,16 +33,8 @@ import { Authentication, Visibility, getAuthentication, - getHeaderFieldName, - getHeaderFieldOptions, - getPathParamName, - getQueryParamName, - getQueryParamOptions, getServers, - isBody, isHeader, - isPathParam, - isQueryParam, isStatusCode, } from "@typespec/http"; import { @@ -58,7 +50,6 @@ import { shouldGenerateConvenient, } from "./decorators.js"; import { - CollectionFormat, SdkArrayType, SdkBodyModelPropertyType, SdkBuiltInKinds, @@ -70,13 +61,11 @@ import { SdkDatetimeType, SdkDictionaryType, SdkDurationType, - SdkEndpointParameter, - SdkEndpointType, SdkEnumType, SdkEnumValueType, SdkModelPropertyType, + SdkModelPropertyTypeBase, SdkModelType, - SdkPathParameter, SdkTupleType, SdkType, SdkUnionType, @@ -109,6 +98,7 @@ import { import { getVersions } from "@typespec/versioning"; import { UnionEnumVariant } from "../../typespec-azure-core/dist/src/helpers/union-enums.js"; +import { getSdkHttpParameter, isSdkHttpParameter } from "./http.js"; import { TCGCContext } from "./internal-utils.js"; function getAnyType(context: TCGCContext, type: Type): SdkBuiltInType { @@ -882,24 +872,6 @@ function getSdkVisibility(context: TCGCContext, type: ModelProperty): Visibility return undefined; } -function getCollectionFormat( - context: TCGCContext, - type: ModelProperty -): CollectionFormat | undefined { - const program = context.program; - const tspCollectionFormat = ( - isQueryParam(program, type) - ? getQueryParamOptions(program, type) - : isHeader(program, type) - ? getHeaderFieldOptions(program, type) - : undefined - )?.format; - if (tspCollectionFormat === "form" || tspCollectionFormat === "simple") { - return undefined; - } - return tspCollectionFormat; -} - function getSdkCredentialType( client: SdkClient, authentication: Authentication @@ -949,31 +921,22 @@ export function getSdkCredentialParameter( }; } -interface GetSdkModelPropertyTypeOptions { - isEndpointParam?: boolean; - operation?: Operation; - isMethodParameter?: boolean; - defaultContentType?: string; -} - -export function getSdkModelPropertyType( +export function getSdkModelPropertyTypeBase( context: TCGCContext, type: ModelProperty, - options: GetSdkModelPropertyTypeOptions = {} -): [SdkModelPropertyType, readonly Diagnostic[]] { + operation?: Operation +): [SdkModelPropertyTypeBase, readonly Diagnostic[]] { const diagnostics = createDiagnosticCollector(); - let propertyType = diagnostics.pipe( - getClientTypeWithDiagnostics(context, type.type, options.operation) - ); - diagnostics.pipe(addEncodeInfo(context, type, propertyType, options.defaultContentType)); + let propertyType = diagnostics.pipe(getClientTypeWithDiagnostics(context, type.type, operation)); + diagnostics.pipe(addEncodeInfo(context, type, propertyType)); addFormatInfo(context, type, propertyType); const knownValues = getKnownValues(context.program, type); if (knownValues) { - propertyType = getSdkEnum(context, knownValues, options.operation); + propertyType = getSdkEnum(context, knownValues, operation); } const docWrapper = getDocHelper(context, type); const name = getPropertyNames(context, type)[0]; - const base = { + return diagnostics.wrap({ __raw: type, description: docWrapper.description, details: docWrapper.details, @@ -981,158 +944,51 @@ export function getSdkModelPropertyType( type: propertyType, nameInClient: name, name, - onClient: false, optional: type.optional, nullable: isNullable(type.type), - }; - const program = context.program; - const headerQueryOptions = { - ...base, - optional: type.optional, - collectionFormat: getCollectionFormat(context, type), - }; - if (options.isMethodParameter) { - return diagnostics.wrap({ - ...base, - kind: "method", - ...updateWithApiVersionInformation(context, type), - optional: type.optional, - }); - } else if (isPathParam(program, type) || options.isEndpointParam) { - // we don't url encode if the type can be assigned to url - const urlEncode = !ignoreDiagnostics( - program.checker.isTypeAssignableTo( - type.type.projectionBase ?? type.type, - program.checker.getStdType("url"), - type.type - ) - ); - return diagnostics.wrap({ - ...base, - kind: "path", - urlEncode, - serializedName: getPathParamName(program, type), - ...updateWithApiVersionInformation(context, type), - optional: false, - }); - } else if (isHeader(program, type)) { - return diagnostics.wrap({ - ...headerQueryOptions, - kind: "header", - serializedName: getHeaderFieldName(program, type), - ...updateWithApiVersionInformation(context, type), - }); - } else if (isQueryParam(program, type)) { - return diagnostics.wrap({ - ...headerQueryOptions, - kind: "query", - serializedName: getQueryParamName(program, type), - ...updateWithApiVersionInformation(context, type), - }); - } else if (isBody(program, type)) { - return diagnostics.wrap({ - ...base, - kind: "body", - contentTypes: ["application/json"], // will update when we get to the operation level - defaultContentType: "application/json", // will update when we get to the operation level - ...updateWithApiVersionInformation(context, type), - optional: type.optional, - }); - } else { - // I'm a body model property - let operationIsMultipart = false; - if (options.operation) { - const httpOperation = getHttpOperationWithCache(context, options.operation); - operationIsMultipart = Boolean( - httpOperation && httpOperation.parameters.body?.contentTypes.includes("multipart/form-data") - ); - } - // Currently we only recognize bytes and list of bytes as potential file inputs - const isBytesInput = - base.type.kind === "bytes" || - (base.type.kind === "array" && base.type.valueType.kind === "bytes"); - if (isBytesInput && operationIsMultipart && getEncode(context.program, type)) { - diagnostics.add( - createDiagnostic({ - code: "encoding-multipart-bytes", - target: type, - }) - ); - } - return diagnostics.wrap({ - ...base, - kind: "property", - optional: type.optional, - visibility: getSdkVisibility(context, type), - discriminator: false, - serializedName: getPropertyNames(context, type)[1], - isMultipartFileInput: isBytesInput && operationIsMultipart, - flatten: shouldFlattenProperty(context, type), - ...updateWithApiVersionInformation(context, type), - }); - } + ...updateWithApiVersionInformation(context, type), + }); } -export function getSdkEndpointParameter( +export function getSdkModelPropertyType( context: TCGCContext, - client: SdkClient -): [SdkEndpointParameter, readonly Diagnostic[]] { + type: ModelProperty, + operation?: Operation +): [SdkModelPropertyType, readonly Diagnostic[]] { const diagnostics = createDiagnosticCollector(); - const servers = getServers(context.program, client.service); - let type: SdkEndpointType; - let optional: boolean = false; - if (servers === undefined || servers.length > 1) { - // if there is no defined server url, or if there are more than one - // we will return a mandatory endpoint parameter in initialization - type = { - kind: "endpoint", - nullable: false, - templateArguments: [], - }; - } else { - // this means we have one server - const templateArguments: SdkPathParameter[] = []; - type = { - kind: "endpoint", - nullable: false, - serverUrl: servers[0].url, - templateArguments, - }; - for (const param of servers[0].parameters.values()) { - const sdkParam = diagnostics.pipe( - getSdkModelPropertyType(context, param, { isEndpointParam: true }) - ); - if (sdkParam.kind === "path") { - templateArguments.push(sdkParam); - sdkParam.description = sdkParam.description ?? servers[0].description; - sdkParam.onClient = true; - } else { - diagnostics.add( - createDiagnostic({ - code: "server-param-not-path", - target: param, - format: { - templateArgumentName: sdkParam.name, - templateArgumentType: sdkParam.kind, - }, - }) - ); - } - } - optional = !!servers[0].url.length && templateArguments.every((param) => param.optional); + const base = diagnostics.pipe(getSdkModelPropertyTypeBase(context, type, operation)); + + if (isSdkHttpParameter(context, type)) return getSdkHttpParameter(context, type); + // I'm a body model property + let operationIsMultipart = false; + if (operation) { + const httpOperation = getHttpOperationWithCache(context, operation); + operationIsMultipart = Boolean( + httpOperation && httpOperation.parameters.body?.contentTypes.includes("multipart/form-data") + ); + } + // Currently we only recognize bytes and list of bytes as potential file inputs + const isBytesInput = + base.type.kind === "bytes" || + (base.type.kind === "array" && base.type.valueType.kind === "bytes"); + if (isBytesInput && operationIsMultipart && getEncode(context.program, type)) { + diagnostics.add( + createDiagnostic({ + code: "encoding-multipart-bytes", + target: type, + }) + ); } return diagnostics.wrap({ - kind: "endpoint", - type, - nameInClient: "endpoint", - name: "endpoint", - description: "Service host", - onClient: true, - urlEncode: false, - apiVersions: getAvailableApiVersions(context, client.service), - optional, - isApiVersionParam: false, - nullable: false, + ...base, + kind: "property", + optional: type.optional, + visibility: getSdkVisibility(context, type), + discriminator: false, + serializedName: getPropertyNames(context, type)[1], + isMultipartFileInput: isBytesInput && operationIsMultipart, + flatten: shouldFlattenProperty(context, type), + ...updateWithApiVersionInformation(context, type), }); } @@ -1151,9 +1007,7 @@ function addPropertiesToModelType( ) { continue; } - const clientProperty = diagnostics.pipe( - getSdkModelPropertyType(context, property, { operation: operation }) - ); + const clientProperty = diagnostics.pipe(getSdkModelPropertyType(context, property, operation)); if (sdkType.properties) { sdkType.properties.push(clientProperty); } else { diff --git a/packages/typespec-client-generator-core/test/package.test.ts b/packages/typespec-client-generator-core/test/package.test.ts index 7999129f4f..96af560bbc 100644 --- a/packages/typespec-client-generator-core/test/package.test.ts +++ b/packages/typespec-client-generator-core/test/package.test.ts @@ -940,7 +940,7 @@ describe("typespec-client-generator-core: package", () => { strictEqual(methodParam.nullable, false); const serviceOperation = method.operation; - strictEqual(serviceOperation.bodyParams.length, 0); + strictEqual(serviceOperation.bodyParam, undefined); strictEqual(serviceOperation.exceptions["*"], undefined); strictEqual(serviceOperation.parameters.length, 1); @@ -960,7 +960,7 @@ describe("typespec-client-generator-core: package", () => { strictEqual(method.response.kind, "method"); strictEqual(method.response.type, undefined); - const correspondingMethodParams = method.getParameterMapping(pathParam); + const correspondingMethodParams = pathParam.correspondingMethodParams; strictEqual(correspondingMethodParams.length, 1); //eslint-disable-next-line deprecation/deprecation strictEqual(pathParam.nameInClient, correspondingMethodParams[0].nameInClient); @@ -1009,7 +1009,7 @@ describe("typespec-client-generator-core: package", () => { strictEqual(methodParam.nullable, false); const serviceOperation = method.operation; - strictEqual(serviceOperation.bodyParams.length, 0); + strictEqual(serviceOperation.bodyParam, undefined); strictEqual(serviceOperation.exceptions["*"], undefined); strictEqual(serviceOperation.parameters.length, 1); @@ -1027,7 +1027,7 @@ describe("typespec-client-generator-core: package", () => { strictEqual(headerParam.collectionFormat, undefined); strictEqual(headerParam.nullable, false); - const correspondingMethodParams = method.getParameterMapping(headerParam); + const correspondingMethodParams = headerParam.correspondingMethodParams; strictEqual(correspondingMethodParams.length, 1); //eslint-disable-next-line deprecation/deprecation strictEqual(headerParam.nameInClient, correspondingMethodParams[0].nameInClient); @@ -1093,7 +1093,7 @@ describe("typespec-client-generator-core: package", () => { strictEqual(methodParam.nullable, false); const serviceOperation = method.operation; - strictEqual(serviceOperation.bodyParams.length, 0); + strictEqual(serviceOperation.bodyParam, undefined); strictEqual(serviceOperation.exceptions["*"], undefined); strictEqual(serviceOperation.parameters.length, 1); @@ -1110,7 +1110,7 @@ describe("typespec-client-generator-core: package", () => { strictEqual(queryParam.collectionFormat, undefined); strictEqual(queryParam.nullable, false); - const correspondingMethodParams = method.getParameterMapping(queryParam); + const correspondingMethodParams = queryParam.correspondingMethodParams; strictEqual(correspondingMethodParams.length, 1); //eslint-disable-next-line deprecation/deprecation strictEqual(queryParam.nameInClient, correspondingMethodParams[0].nameInClient); @@ -1187,8 +1187,8 @@ describe("typespec-client-generator-core: package", () => { strictEqual(methodContentTypeParam.optional, false); const serviceOperation = method.operation; - strictEqual(serviceOperation.bodyParams.length, 1); - const bodyParameter = serviceOperation.bodyParams[0]; + const bodyParameter = serviceOperation.bodyParam; + ok(bodyParameter); strictEqual(bodyParameter.kind, "body"); deepStrictEqual(bodyParameter.contentTypes, ["application/json"]); strictEqual(bodyParameter.defaultContentType, "application/json"); @@ -1197,7 +1197,7 @@ describe("typespec-client-generator-core: package", () => { strictEqual(bodyParameter.type, sdkPackage.models[0]); strictEqual(bodyParameter.nullable, false); - const correspondingMethodParams = method.getParameterMapping(bodyParameter); + const correspondingMethodParams = bodyParameter.correspondingMethodParams; strictEqual(correspondingMethodParams.length, 1); //eslint-disable-next-line deprecation/deprecation strictEqual(bodyParameter.nameInClient, correspondingMethodParams[0].nameInClient); @@ -1213,7 +1213,7 @@ describe("typespec-client-generator-core: package", () => { strictEqual(contentTypeParam.onClient, false); strictEqual(contentTypeParam.optional, false); - const correspondingContentTypeMethodParams = method.getParameterMapping(contentTypeParam); + const correspondingContentTypeMethodParams = contentTypeParam.correspondingMethodParams; strictEqual(correspondingContentTypeMethodParams.length, 1); strictEqual(correspondingContentTypeMethodParams[0], methodContentTypeParam); }); @@ -1236,8 +1236,8 @@ describe("typespec-client-generator-core: package", () => { strictEqual(methodBodyParam.nullable, true); const serviceOperation = method.operation; - const bodyParameter = serviceOperation.bodyParams[0]; - strictEqual(bodyParameter.nullable, true); + ok(serviceOperation.bodyParam); + strictEqual(serviceOperation.bodyParam.nullable, true); }); it("body optional", async () => { @@ -1275,8 +1275,8 @@ describe("typespec-client-generator-core: package", () => { strictEqual(methodContentTypeParam.optional, false); const serviceOperation = method.operation; - strictEqual(serviceOperation.bodyParams.length, 1); - const bodyParameter = serviceOperation.bodyParams[0]; + const bodyParameter = serviceOperation.bodyParam; + ok(bodyParameter); strictEqual(bodyParameter.kind, "body"); deepStrictEqual(bodyParameter.contentTypes, ["application/json"]); strictEqual(bodyParameter.defaultContentType, "application/json"); @@ -1284,7 +1284,7 @@ describe("typespec-client-generator-core: package", () => { strictEqual(bodyParameter.optional, true); strictEqual(bodyParameter.type, sdkPackage.models[0]); - const correspondingMethodParams = method.getParameterMapping(bodyParameter); + const correspondingMethodParams = bodyParameter.correspondingMethodParams; strictEqual(correspondingMethodParams.length, 1); //eslint-disable-next-line deprecation/deprecation strictEqual(bodyParameter.nameInClient, correspondingMethodParams[0].nameInClient); @@ -1300,7 +1300,7 @@ describe("typespec-client-generator-core: package", () => { strictEqual(contentTypeParam.onClient, false); strictEqual(contentTypeParam.optional, false); - const correspondingContentTypeMethodParams = method.getParameterMapping(contentTypeParam); + const correspondingContentTypeMethodParams = contentTypeParam.correspondingMethodParams; strictEqual(correspondingContentTypeMethodParams.length, 1); strictEqual(correspondingContentTypeMethodParams[0], methodContentTypeParam); }); @@ -1337,8 +1337,9 @@ describe("typespec-client-generator-core: package", () => { strictEqual(contentTypeParam.onClient, false); const serviceOperation = method.operation; - strictEqual(serviceOperation.bodyParams.length, 1); - const bodyParameter = serviceOperation.bodyParams[0]; + const bodyParameter = serviceOperation.bodyParam; + ok(bodyParameter); + strictEqual(bodyParameter.kind, "body"); deepStrictEqual(bodyParameter.contentTypes, ["application/json"]); strictEqual(bodyParameter.defaultContentType, "application/json"); @@ -1346,7 +1347,7 @@ describe("typespec-client-generator-core: package", () => { strictEqual(bodyParameter.optional, false); strictEqual(bodyParameter.type, sdkPackage.models[0]); - const correspondingMethodParams = method.getParameterMapping(bodyParameter); + const correspondingMethodParams = bodyParameter.correspondingMethodParams; strictEqual(correspondingMethodParams.length, 1); strictEqual( bodyParameter.type.properties[0].nameInClient, //eslint-disable-line deprecation/deprecation @@ -1387,8 +1388,8 @@ describe("typespec-client-generator-core: package", () => { strictEqual(contentTypeMethodParam.type.kind, "constant"); const serviceOperation = method.operation; - strictEqual(serviceOperation.bodyParams.length, 1); - const bodyParameter = serviceOperation.bodyParams[0]; + const bodyParameter = serviceOperation.bodyParam; + ok(bodyParameter); strictEqual(bodyParameter.kind, "body"); deepStrictEqual(bodyParameter.contentTypes, ["application/json"]); strictEqual(bodyParameter.defaultContentType, "application/json"); @@ -1400,7 +1401,7 @@ describe("typespec-client-generator-core: package", () => { strictEqual(bodyParameter.type.properties[0].nameInClient, "name"); strictEqual(bodyParameter.type.properties[0].name, "name"); - const correspondingMethodParams = method.getParameterMapping(bodyParameter); + const correspondingMethodParams = bodyParameter.correspondingMethodParams; strictEqual(correspondingMethodParams.length, 1); strictEqual( @@ -1494,8 +1495,8 @@ describe("typespec-client-generator-core: package", () => { const serviceOperation = method.operation; strictEqual(serviceOperation.parameters.length, 3); - strictEqual(serviceOperation.bodyParams.length, 1); - const correspondingBodyParams = method.getParameterMapping(serviceOperation.bodyParams[0]); + ok(serviceOperation.bodyParam); + const correspondingBodyParams = serviceOperation.bodyParam.correspondingMethodParams; strictEqual(correspondingBodyParams.length, 1); //eslint-disable-next-line deprecation/deprecation strictEqual(correspondingBodyParams[0].nameInClient, bodyParamProp.nameInClient); @@ -1506,13 +1507,13 @@ describe("typespec-client-generator-core: package", () => { const headerParams = parameters.filter((x): x is SdkHeaderParameter => x.kind === "header"); strictEqual(headerParams.length, 2); - let correspondingHeaderParams = method.getParameterMapping(headerParams[0]); + let correspondingHeaderParams = headerParams[0].correspondingMethodParams; strictEqual(correspondingHeaderParams.length, 1); //eslint-disable-next-line deprecation/deprecation strictEqual(correspondingHeaderParams[0].nameInClient, headerParamProp.nameInClient); strictEqual(correspondingHeaderParams[0].name, headerParamProp.name); - correspondingHeaderParams = method.getParameterMapping(headerParams[1]); + correspondingHeaderParams = headerParams[1].correspondingMethodParams; strictEqual(correspondingHeaderParams.length, 1); //eslint-disable-next-line deprecation/deprecation strictEqual(correspondingHeaderParams[0].nameInClient, "contentType"); @@ -1520,7 +1521,7 @@ describe("typespec-client-generator-core: package", () => { const queryParams = parameters.filter((x): x is SdkQueryParameter => x.kind === "query"); strictEqual(queryParams.length, 1); - const correspondingQueryParams = method.getParameterMapping(queryParams[0]); + const correspondingQueryParams = queryParams[0].correspondingMethodParams; strictEqual(correspondingQueryParams.length, 1); //eslint-disable-next-line deprecation/deprecation strictEqual(correspondingQueryParams[0].nameInClient, queryParamProp.nameInClient); @@ -1562,15 +1563,15 @@ describe("typespec-client-generator-core: package", () => { const serviceOperation = method.operation; strictEqual(serviceOperation.parameters.length, 1); - strictEqual(serviceOperation.bodyParams.length, 1); - const correspondingBodyParams = method.getParameterMapping(serviceOperation.bodyParams[0]); + ok(serviceOperation.bodyParam); + const correspondingBodyParams = serviceOperation.bodyParam.correspondingMethodParams; strictEqual(correspondingBodyParams.length, 1); //eslint-disable-next-line deprecation/deprecation strictEqual(correspondingBodyParams[0].nameInClient, "body"); strictEqual(correspondingBodyParams[0].name, "body"); strictEqual(serviceOperation.parameters.length, 1); - const correspondingHeaderParams = method.getParameterMapping(serviceOperation.parameters[0]); + const correspondingHeaderParams = serviceOperation.parameters[0].correspondingMethodParams; strictEqual(correspondingHeaderParams.length, 1); //eslint-disable-next-line deprecation/deprecation strictEqual(correspondingHeaderParams[0].nameInClient, "contentType"); @@ -1599,13 +1600,13 @@ describe("typespec-client-generator-core: package", () => { strictEqual(methodContentTypeParam.name, "contentType"); const serviceOperation = method.operation; - strictEqual(serviceOperation.bodyParams.length, 1); - const serviceBodyParam = serviceOperation.bodyParams[0]; + const serviceBodyParam = serviceOperation.bodyParam; + ok(serviceBodyParam); strictEqual(serviceBodyParam.kind, "body"); strictEqual(serviceBodyParam.contentTypes.length, 1); strictEqual(serviceBodyParam.defaultContentType, "application/json"); strictEqual(serviceBodyParam.contentTypes[0], "application/json"); - deepStrictEqual(method.getParameterMapping(serviceBodyParam)[0], methodBodyParam); + deepStrictEqual(serviceBodyParam.correspondingMethodParams[0], methodBodyParam); strictEqual(serviceOperation.parameters.length, 1); const serviceContentTypeParam = serviceOperation.parameters[0]; @@ -1617,10 +1618,7 @@ describe("typespec-client-generator-core: package", () => { strictEqual(serviceContentTypeParam.type.kind, "constant"); strictEqual(serviceContentTypeParam.type.value, "application/json"); strictEqual(serviceContentTypeParam.type.valueType.kind, "string"); - deepStrictEqual( - method.getParameterMapping(serviceContentTypeParam)[0], - methodContentTypeParam - ); + deepStrictEqual(serviceContentTypeParam.correspondingMethodParams[0], methodContentTypeParam); }); it("ensure accept is a constant if only one possibility (json)", async () => { @@ -1955,7 +1953,8 @@ describe("typespec-client-generator-core: package", () => { ["id", "weight", "color", "contentType", "accept"] ); - const bodyParameter = method.operation.bodyParams[0]; + const bodyParameter = method.operation.bodyParam; + ok(bodyParameter); strictEqual(bodyParameter.kind, "body"); //eslint-disable-next-line deprecation/deprecation strictEqual(bodyParameter.nameInClient, "body"); @@ -1985,7 +1984,7 @@ describe("typespec-client-generator-core: package", () => { strictEqual(contentTypeMethodParam.onClient, false); strictEqual(contentTypeMethodParam.optional, false); - strictEqual(method.getParameterMapping(contentTypeOperationParam)[0], contentTypeMethodParam); + strictEqual(contentTypeOperationParam.correspondingMethodParams[0], contentTypeMethodParam); const acceptOperationParam = headerParams.find((x) => x.serializedName === "Accept"); ok(acceptOperationParam); @@ -2000,7 +1999,7 @@ describe("typespec-client-generator-core: package", () => { strictEqual(acceptMethodParam.onClient, false); strictEqual(acceptMethodParam.optional, false); - strictEqual(method.getParameterMapping(acceptOperationParam)[0], acceptMethodParam); + strictEqual(acceptOperationParam.correspondingMethodParams[0], acceptMethodParam); }); it("vanilla widget read", async () => { await compileVanillaWidgetService(runner, "@get read(@path id: string): Widget | Error;"); @@ -2044,9 +2043,9 @@ describe("typespec-client-generator-core: package", () => { ok(operationAcceptParam); strictEqual(operationAcceptParam.clientDefaultValue, undefined); - strictEqual(method.getParameterMapping(operationAcceptParam)[0], methodAcceptParam); + strictEqual(operationAcceptParam.correspondingMethodParams[0], methodAcceptParam); - const correspondingMethodParams = method.getParameterMapping(pathParam); + const correspondingMethodParams = pathParam.correspondingMethodParams; strictEqual(correspondingMethodParams.length, 1); //eslint-disable-next-line deprecation/deprecation strictEqual(pathParam.nameInClient, correspondingMethodParams[0].nameInClient); @@ -2119,8 +2118,9 @@ describe("typespec-client-generator-core: package", () => { strictEqual(pathParam.isApiVersionParam, false); strictEqual(pathParam.type.kind, "string"); - strictEqual(serviceOperation.bodyParams.length, 1); - const bodyParameter = serviceOperation.bodyParams[0]; + const bodyParameter = serviceOperation.bodyParam; + ok(bodyParameter); + strictEqual(bodyParameter.kind, "body"); deepStrictEqual(bodyParameter.contentTypes, ["application/json"]); strictEqual(bodyParameter.defaultContentType, "application/json"); @@ -2145,17 +2145,15 @@ describe("typespec-client-generator-core: package", () => { strictEqual(operationAcceptParam.clientDefaultValue, undefined); strictEqual(operationAcceptParam.optional, false); - const correspondingMethodParams = method - .getParameterMapping(bodyParameter) - .map((x) => x.name); + const correspondingMethodParams = bodyParameter.correspondingMethodParams.map((x) => x.name); deepStrictEqual(correspondingMethodParams, ["weight", "color"]); deepStrictEqual( bodyParameter.type.properties.map((p) => p.name), ["id", "weight", "color"] ); - strictEqual(method.getParameterMapping(operationContentTypeParam)[0], methodContentTypeParam); - strictEqual(method.getParameterMapping(operationAcceptParam)[0], methodAcceptParam); + strictEqual(operationContentTypeParam.correspondingMethodParams[0], methodContentTypeParam); + strictEqual(operationAcceptParam.correspondingMethodParams[0], methodAcceptParam); }); it("vanilla widget delete", async () => { await compileVanillaWidgetService(runner, "@delete delete(@path id: string): void | Error;"); @@ -2190,7 +2188,7 @@ describe("typespec-client-generator-core: package", () => { strictEqual(pathParam.isApiVersionParam, false); strictEqual(pathParam.type.kind, "string"); - const correspondingMethodParams = method.getParameterMapping(pathParam); + const correspondingMethodParams = pathParam.correspondingMethodParams; strictEqual(correspondingMethodParams.length, 1); //eslint-disable-next-line deprecation/deprecation strictEqual(pathParam.nameInClient, correspondingMethodParams[0].nameInClient); @@ -2204,7 +2202,7 @@ describe("typespec-client-generator-core: package", () => { strictEqual(method.name, "list"); strictEqual(method.kind, "basic"); strictEqual(method.parameters.length, 1); - strictEqual(method.operation.bodyParams.length, 0); + strictEqual(method.operation.bodyParam, undefined); }); }); describe("Azure Widget Service", () => { @@ -2434,8 +2432,8 @@ describe("typespec-client-generator-core: package", () => { strictEqual(pathParam.name, "widgetName"); strictEqual(pathParam.serializedName, "widgetName"); strictEqual(pathParam.onClient, false); - strictEqual(method.getParameterMapping(pathParam).length, 1); - strictEqual(method.getParameterMapping(pathParam)[0], methodWidgetName); + strictEqual(pathParam.correspondingMethodParams.length, 1); + strictEqual(pathParam.correspondingMethodParams[0], methodWidgetName); const queryParam = method.operation.parameters.find((x) => x.kind === "query"); ok(queryParam); @@ -2445,10 +2443,10 @@ describe("typespec-client-generator-core: package", () => { strictEqual(queryParam.name, "apiVersion"); strictEqual(queryParam.serializedName, "api-version"); strictEqual(queryParam.onClient, true); - strictEqual(method.getParameterMapping(queryParam).length, 1); + strictEqual(queryParam.correspondingMethodParams.length, 1); ok(parentClient.initialization); strictEqual( - method.getParameterMapping(queryParam)[0], + queryParam.correspondingMethodParams[0], parentClient.initialization.properties.find((x) => x.isApiVersionParam) ); @@ -2468,7 +2466,7 @@ describe("typespec-client-generator-core: package", () => { strictEqual(operationAcceptParam.clientDefaultValue, undefined); strictEqual(operationAcceptParam.onClient, false); strictEqual(operationAcceptParam.optional, false); - strictEqual(method.getParameterMapping(operationAcceptParam)[0], methodAcceptParam); + strictEqual(operationAcceptParam.correspondingMethodParams[0], methodAcceptParam); strictEqual( method.parameters.some((x) => x.name === "contentType"), @@ -2543,8 +2541,8 @@ describe("typespec-client-generator-core: package", () => { strictEqual(pathParam1.name, "widgetName"); strictEqual(pathParam1.serializedName, "widgetName"); strictEqual(pathParam1.onClient, false); - strictEqual(getStatus.getParameterMapping(pathParam1).length, 1); - strictEqual(getStatus.getParameterMapping(pathParam1)[0], methodWidgetName); + strictEqual(pathParam1.correspondingMethodParams.length, 1); + strictEqual(pathParam1.correspondingMethodParams[0], methodWidgetName); const pathParam2 = pathParams[1]; strictEqual(pathParam2.kind, "path"); @@ -2553,8 +2551,8 @@ describe("typespec-client-generator-core: package", () => { strictEqual(pathParam2.name, "operationId"); strictEqual(pathParam2.serializedName, "operationId"); strictEqual(pathParam2.onClient, false); - strictEqual(getStatus.getParameterMapping(pathParam2).length, 1); - strictEqual(getStatus.getParameterMapping(pathParam2)[0], methodOperationId); + strictEqual(pathParam2.correspondingMethodParams.length, 1); + strictEqual(pathParam2.correspondingMethodParams[0], methodOperationId); const apiVersionParam = getStatus.operation.parameters.find((x) => x.kind === "query"); ok(apiVersionParam); @@ -2564,10 +2562,10 @@ describe("typespec-client-generator-core: package", () => { strictEqual(apiVersionParam.name, "apiVersion"); strictEqual(apiVersionParam.serializedName, "api-version"); strictEqual(apiVersionParam.onClient, true); - strictEqual(getStatus.getParameterMapping(apiVersionParam).length, 1); + strictEqual(apiVersionParam.correspondingMethodParams.length, 1); ok(parentClient.initialization); strictEqual( - getStatus.getParameterMapping(apiVersionParam)[0], + apiVersionParam.correspondingMethodParams[0], parentClient.initialization.properties.find((x) => x.isApiVersionParam) ); @@ -2579,7 +2577,7 @@ describe("typespec-client-generator-core: package", () => { strictEqual(operationAcceptParam.clientDefaultValue, undefined); strictEqual(operationAcceptParam.onClient, false); strictEqual(operationAcceptParam.optional, false); - strictEqual(getStatus.getParameterMapping(operationAcceptParam)[0], methodAcceptParam); + strictEqual(operationAcceptParam.correspondingMethodParams[0], methodAcceptParam); const widgetModel = sdkPackage.models.find((x) => x.name === "Widget"); @@ -2634,11 +2632,11 @@ describe("typespec-client-generator-core: package", () => { const queryParam = serviceOperation.parameters.find((x) => x.kind === "query"); ok(queryParam); strictEqual(queryParam.serializedName, "api-version"); - strictEqual(serviceOperation.bodyParams.length, 1); + ok(serviceOperation.bodyParam); //eslint-disable-next-line deprecation/deprecation - strictEqual(serviceOperation.bodyParams[0].nameInClient, "resource"); - strictEqual(serviceOperation.bodyParams[0].name, "resource"); - strictEqual(serviceOperation.bodyParams[0].type, widgetModel); + strictEqual(serviceOperation.bodyParam.nameInClient, "resource"); + strictEqual(serviceOperation.bodyParam.name, "resource"); + strictEqual(serviceOperation.bodyParam.type, widgetModel); strictEqual(Object.keys(serviceOperation.responses).length, 2); const responseHeaders = [ @@ -2732,17 +2730,14 @@ describe("typespec-client-generator-core: package", () => { ok(clientRequestId); strictEqual(clientRequestId.kind, "header"); deepStrictEqual( - listManufacturers.getParameterMapping(clientRequestId)[0], + clientRequestId.correspondingMethodParams[0], listManufacturers.parameters[0] ); const accept = operation.parameters.find((x) => x.name === "accept"); ok(accept); strictEqual(accept.kind, "header"); - deepStrictEqual( - listManufacturers.getParameterMapping(accept)[0], - listManufacturers.parameters[1] - ); + deepStrictEqual(accept.correspondingMethodParams[0], listManufacturers.parameters[1]); strictEqual(Object.keys(operation.responses).length, 1); const response200 = operation.responses[200]; From d34a2da1e673f6cae36310515e035762e0f991b1 Mon Sep 17 00:00:00 2001 From: iscai-msft Date: Tue, 26 Mar 2024 18:38:19 -0400 Subject: [PATCH 2/4] add changeset --- ...remove_parameter_response_mapping-2024-2-26-18-38-14.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .chronus/changes/remove_parameter_response_mapping-2024-2-26-18-38-14.md diff --git a/.chronus/changes/remove_parameter_response_mapping-2024-2-26-18-38-14.md b/.chronus/changes/remove_parameter_response_mapping-2024-2-26-18-38-14.md new file mode 100644 index 0000000000..42d1cecc7d --- /dev/null +++ b/.chronus/changes/remove_parameter_response_mapping-2024-2-26-18-38-14.md @@ -0,0 +1,7 @@ +--- +changeKind: breaking +packages: + - "@azure-tools/typespec-client-generator-core" +--- + +depcreate getParameterMapping and make .bodyParam on SdkHttpOperation a single optional param instead of list \ No newline at end of file From eafb41d610e5a79bc73f42c92be2193abf85973c Mon Sep 17 00:00:00 2001 From: iscai-msft Date: Tue, 26 Mar 2024 18:42:46 -0400 Subject: [PATCH 3/4] merge --- .../src/http.ts | 30 ++++----- .../src/interfaces.ts | 7 +- .../src/internal-utils.ts | 12 ++-- .../test/package.test.ts | 67 ++++++++++++------- 4 files changed, 67 insertions(+), 49 deletions(-) diff --git a/packages/typespec-client-generator-core/src/http.ts b/packages/typespec-client-generator-core/src/http.ts index 2eba12ca0b..c6bcddd712 100644 --- a/packages/typespec-client-generator-core/src/http.ts +++ b/packages/typespec-client-generator-core/src/http.ts @@ -8,6 +8,7 @@ import { } from "@typespec/compiler"; import { HttpOperation, + HttpStatusCodeRange, getHeaderFieldName, getHeaderFieldOptions, getPathParamName, @@ -54,11 +55,11 @@ export function getSdkHttpOperation( methodParameters: SdkMethodParameter[] ): [SdkHttpOperation, readonly Diagnostic[]] { const diagnostics = createDiagnosticCollector(); - const [responses, exceptions] = diagnostics.pipe( + const {responses, exceptions} = diagnostics.pipe( getSdkHttpResponseAndExceptions(context, httpOperation) ); - const responsesWithBodies = Object.values(responses) - .concat(Object.values(exceptions)) + const responsesWithBodies = [...responses.values()] + .concat([...exceptions.values()]) .filter((r) => r.type); const parameters = diagnostics.pipe( getSdkHttpParameters(context, httpOperation, methodParameters, responsesWithBodies[0]) @@ -321,12 +322,15 @@ function getSdkHttpResponseAndExceptions( context: TCGCContext, 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; @@ -370,19 +374,13 @@ function getSdkHttpResponseAndExceptions( apiVersions: getAvailableApiVersions(context, httpOperation.operation), nullable: body ? isNullable(body) : true, }; - let statusCode: number | string = ""; - if (typeof response.statusCodes === "number" || response.statusCodes === "*") { - statusCode = response.statusCodes; + if (response.statusCodes === "*" || (body && isErrorModel(context.program, body))) { + exceptions.set(response.statusCodes, sdkResponse); } else { - statusCode = `${response.statusCodes.start}-${response.statusCodes.end}`; - } - if (statusCode === "*" || (body && isErrorModel(context.program, body))) { - exceptions[statusCode] = sdkResponse; - } else { - responses[statusCode] = sdkResponse; + responses.set(response.statusCodes, sdkResponse); } } - return diagnostics.wrap([responses, exceptions]); + return diagnostics.wrap({ responses, exceptions }); } export function getCorrespondingMethodParams( diff --git a/packages/typespec-client-generator-core/src/interfaces.ts b/packages/typespec-client-generator-core/src/interfaces.ts index bb556e8d49..8a4ed77812 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"; @@ -443,9 +444,9 @@ export interface SdkHttpOperation extends SdkServiceOperationBase { path: string; verb: HttpVerb; parameters: (SdkPathParameter | SdkQueryParameter | SdkHeaderParameter)[]; - bodyParam?: 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 + bodyParam?: SdkBodyParameter; + 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/test/package.test.ts b/packages/typespec-client-generator-core/test/package.test.ts index 96af560bbc..0c3b19c8ac 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.bodyParam, undefined); - 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.bodyParam, undefined); - 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.bodyParam, undefined); - strictEqual(serviceOperation.exceptions["*"], undefined); + strictEqual(serviceOperation.exceptions.get("*"), undefined); strictEqual(serviceOperation.parameters.length, 1); const queryParam = serviceOperation.parameters[0]; @@ -1649,8 +1649,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); @@ -1692,8 +1693,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); @@ -1722,14 +1724,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"); @@ -1763,9 +1767,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, @@ -1773,7 +1778,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"); @@ -1804,11 +1810,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, @@ -1850,7 +1857,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); @@ -1871,10 +1879,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); @@ -1892,9 +1902,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); @@ -2638,28 +2649,31 @@ describe("typespec-client-generator-core: package", () => { strictEqual(serviceOperation.bodyParam.name, "resource"); strictEqual(serviceOperation.bodyParam.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"); @@ -2739,8 +2753,9 @@ describe("typespec-client-generator-core: package", () => { strictEqual(accept.kind, "header"); deepStrictEqual(accept.correspondingMethodParams[0], 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 3a324ba9973df77eef0e28e22630b2365e2a9c48 Mon Sep 17 00:00:00 2001 From: iscai-msft Date: Wed, 27 Mar 2024 14:18:09 -0400 Subject: [PATCH 4/4] fix property defs of http params --- .../src/http.ts | 22 +++++----- .../src/package.ts | 2 +- .../test/package.test.ts | 44 +++++++++++++++++++ 3 files changed, 57 insertions(+), 11 deletions(-) diff --git a/packages/typespec-client-generator-core/src/http.ts b/packages/typespec-client-generator-core/src/http.ts index d4bf40749f..07cd26f431 100644 --- a/packages/typespec-client-generator-core/src/http.ts +++ b/packages/typespec-client-generator-core/src/http.ts @@ -102,7 +102,7 @@ function getSdkHttpParameters( bodyParam: undefined, }; retval.parameters = httpOperation.parameters.parameters - .map((x) => diagnostics.pipe(getSdkHttpParameter(context, x.param))) + .map((x) => diagnostics.pipe(getSdkHttpParameter(context, x.param, x.type))) .filter( (x): x is SdkHeaderParameter | SdkQueryParameter | SdkPathParameter => x.kind === "header" || x.kind === "query" || x.kind === "path" @@ -117,7 +117,9 @@ function getSdkHttpParameters( if (tspBody) { // if there's a param on the body, we can just rely on getSdkHttpParameter if (tspBody.parameter) { - const getParamResponse = diagnostics.pipe(getSdkHttpParameter(context, tspBody.parameter)); + const getParamResponse = diagnostics.pipe( + getSdkHttpParameter(context, tspBody.parameter, "body") + ); if (getParamResponse.kind !== "body") throw new Error("blah"); retval.bodyParam = getParamResponse; } else { @@ -263,13 +265,13 @@ function addContentTypeInfoToBodyParam( export function getSdkHttpParameter( context: TCGCContext, type: ModelProperty, - isMethodParameter: boolean = false + location?: "path" | "query" | "header" | "body" ): [SdkHttpParameter, readonly Diagnostic[]] { const diagnostics = createDiagnosticCollector(); const base = diagnostics.pipe(getSdkModelPropertyTypeBase(context, type)); const program = context.program; const correspondingMethodParams: SdkParameter[] = []; // we set it later in the operation - if (isPathParam(context.program, type) || isMethodParameter) { + if (isPathParam(context.program, type) || location === "path") { // we don't url encode if the type can be assigned to url const urlEncode = !ignoreDiagnostics( program.checker.isTypeAssignableTo( @@ -282,12 +284,12 @@ export function getSdkHttpParameter( ...base, kind: "path", urlEncode, - serializedName: getPathParamName(program, type), + serializedName: getPathParamName(program, type) ?? base.name, correspondingMethodParams, optional: false, }); } - if (isBody(context.program, type)) { + if (isBody(context.program, type) || location === "body") { return diagnostics.wrap({ ...base, kind: "body", @@ -303,18 +305,18 @@ export function getSdkHttpParameter( collectionFormat: getCollectionFormat(context, type), correspondingMethodParams, }; - if (isQueryParam(context.program, type)) { + if (isQueryParam(context.program, type) || location === "query") { return diagnostics.wrap({ ...headerQueryBase, kind: "query", - serializedName: getQueryParamName(program, type), + serializedName: getQueryParamName(program, type) ?? base.name, }); } - // has to be a header + if (!(isHeader(context.program, type) || location === "header")) throw new Error(`${type.name}`); return diagnostics.wrap({ ...headerQueryBase, kind: "header", - serializedName: getHeaderFieldName(program, type), + serializedName: getHeaderFieldName(program, type) ?? base.name, }); } diff --git a/packages/typespec-client-generator-core/src/package.ts b/packages/typespec-client-generator-core/src/package.ts index 66d4d43814..bc2c6d481a 100644 --- a/packages/typespec-client-generator-core/src/package.ts +++ b/packages/typespec-client-generator-core/src/package.ts @@ -392,7 +392,7 @@ function getSdkEndpointParameter( templateArguments, }; for (const param of servers[0].parameters.values()) { - const sdkParam = diagnostics.pipe(getSdkHttpParameter(context, param, true)); + const sdkParam = diagnostics.pipe(getSdkHttpParameter(context, param, "path")); if (sdkParam.kind === "path") { templateArguments.push(sdkParam); sdkParam.description = sdkParam.description ?? servers[0].description; diff --git a/packages/typespec-client-generator-core/test/package.test.ts b/packages/typespec-client-generator-core/test/package.test.ts index 0c3b19c8ac..8445010b49 100644 --- a/packages/typespec-client-generator-core/test/package.test.ts +++ b/packages/typespec-client-generator-core/test/package.test.ts @@ -984,6 +984,50 @@ describe("typespec-client-generator-core: package", () => { strictEqual(pathParam.nullable, true); }); + it("path defined in model", async () => { + await runner.compileWithBuiltInService(` + @route("{name}") + @put + op pathInModel(...NameParameter): void; + + model NameParameter { + @doc("Name parameter") + @pattern("^[a-zA-Z0-9-]{3,24}$") + @format("UUID") + name: string; + } + `); + const sdkPackage = runner.context.experimental_sdkPackage; + const method = getServiceMethodOfClient(sdkPackage); + strictEqual(method.name, "pathInModel"); + strictEqual(method.kind, "basic"); + strictEqual(method.parameters.length, 1); + const pathMethod = method.parameters[0]; + strictEqual(pathMethod.kind, "method"); + strictEqual(pathMethod.name, "name"); + strictEqual(pathMethod.optional, false); + strictEqual(pathMethod.onClient, false); + strictEqual(pathMethod.isApiVersionParam, false); + strictEqual(pathMethod.type.kind, "string"); + strictEqual(pathMethod.nullable, false); + + const serviceOperation = method.operation; + strictEqual(serviceOperation.bodyParam, undefined); + strictEqual(serviceOperation.parameters.length, 1); + const pathParam = serviceOperation.parameters[0]; + strictEqual(pathParam.kind, "path"); + strictEqual(pathParam.serializedName, "name"); + strictEqual(pathParam.name, "name"); + strictEqual(pathParam.optional, false); + strictEqual(pathParam.onClient, false); + strictEqual(pathParam.isApiVersionParam, false); + strictEqual(pathParam.type.kind, "string"); + strictEqual(pathParam.urlEncode, true); + strictEqual(pathParam.nullable, false); + strictEqual(pathParam.correspondingMethodParams.length, 1); + deepStrictEqual(pathParam.correspondingMethodParams[0], pathMethod); + }); + it("header basic", async () => { await runner.compile(`@server("http://localhost:3000", "endpoint") @service({})