diff --git a/README.md b/README.md index e19aca81beb5..229f5bb0b118 100755 --- a/README.md +++ b/README.md @@ -28,7 +28,7 @@ More information about these can be found [here](https://github.com/Azure/autore ```yaml version: 3.0.6246 use-extension: - "@autorest/modelerfour": "4.10.268" + "@autorest/modelerfour": "4.12.277" modelerfour: # this runs a pre-namer step to clean up names diff --git a/src/generators/operationGenerator.ts b/src/generators/operationGenerator.ts index 6da74c4d9092..fb9f45ce1c79 100644 --- a/src/generators/operationGenerator.ts +++ b/src/generators/operationGenerator.ts @@ -498,46 +498,27 @@ function trackParameterGroups( function getBaseMethodParameterDeclarations( overloadParameterDeclarations: ParameterWithDescription[][] ): ParameterWithDescription[] { - const baseMethodParameters: ParameterWithDescription[] = []; - // Need to know which overload has the most parameters to set our upper bound. - const maxOverloadSize = Math.max( - ...overloadParameterDeclarations.map(params => params.length) - ); - for (let i = 0; i < maxOverloadSize; i++) { - // attempt to combine types - const declarations: ParameterWithDescription[] = []; - for (const overloadParameterDeclaration of overloadParameterDeclarations) { - if (overloadParameterDeclaration[i]) { - declarations.push(overloadParameterDeclaration[i]); - } - } - - const declaration = declarations.reduce( - (prevDeclaration, curDeclaration) => { - prevDeclaration = { ...prevDeclaration }; - if (curDeclaration.name !== prevDeclaration.name) { - // Currently we only generate overloads if an operation supports multiple media types. - // Since contentType is always required and the 1st parameter, parameter names/ordering - // shouldn't change. - // In order to support more variance in parameter naming/ordering, we'll need to be able - // to construct the OperationArguments separately for each overload. - throw new Error( - `Operation overloads with different parameter names/ordering not supported.` - ); - } - if (curDeclaration.type !== prevDeclaration.type) { - prevDeclaration.type += ` | ${curDeclaration.type}`; - } - // parameters should be optional if any declaration is optional - if (!curDeclaration.hasQuestionToken) { - prevDeclaration.hasQuestionToken = curDeclaration.hasQuestionToken; - } - return prevDeclaration; - } - ); - baseMethodParameters.push(declaration); + if (!overloadParameterDeclarations.length) { + return []; } - return baseMethodParameters; + if (overloadParameterDeclarations.length === 1) { + return [...overloadParameterDeclarations[0]]; + } + + const baseMethodArg: ParameterWithDescription = { + name: "args", + isRestParameter: true, + description: "Includes all the parameters for this operation.", + type: overloadParameterDeclarations + .map(overloadParams => { + return `[ ${overloadParams + .map(p => (p.hasQuestionToken ? `${p.type}?` : p.type)) + .join(", ")} ]`; + }) + .join(" | ") + }; + + return [baseMethodArg]; } /** @@ -571,8 +552,10 @@ export function writeOperations( ] }); - const sendParams = baseMethodParameters.map(p => p.name).join(","); if (overloadParameterDeclarations.length === 1) { + const sendParams = overloadParameterDeclarations[0] + .map(p => p.name) + .join(","); operationMethod.addStatements( `return this${ isInline ? "" : ".client" @@ -596,7 +579,7 @@ export function writeOperations( writeMultiMediaTypeOperationBody( operationMethod, operation, - sendParams, + overloadParameterDeclarations, responseName, isInline ); @@ -611,54 +594,79 @@ export function writeOperations( function writeMultiMediaTypeOperationBody( operationMethod: MethodDeclaration, operation: OperationDetails, - sendParams: string, + overloadParameterDeclarations: ParameterWithDescription[][], responseName: string, isInline = false ): void { - let statements = `let operationSpec: coreHttp.OperationSpec;`; + let statements = ` + let operationSpec: coreHttp.OperationSpec; + let operationArguments: coreHttp.OperationArguments; + `; // We need to use the contentType parameter to determine which spec to use. const conditionals: string[] = []; - for (const request of operation.requests) { + const requests = operation.requests; + if (overloadParameterDeclarations.length !== requests.length) { + throw new Error( + `Expected ${requests.length} overloads, but found generated ${overloadParameterDeclarations.length} for ${operation.name}` + ); + } + + // Since contentType is always added as a synthetic parameter by modelerfour, it should always + // be in the same position for all overloads. + let contentTypePosition: number = 0; + for (let i = 0; i < requests.length; i++) { + const request = requests[i]; + const overloadParameters = overloadParameterDeclarations[i]; const mediaType = request.mediaType!; - const validContentTypes = getContentTypeValues(request); + const contentTypeInfo = getContentTypeInfo(request); // Ensure that a contentType exists, otherwise we won't be able to determine which operation spec to use. - if (!validContentTypes) { + if (!contentTypeInfo) { throw new Error( `Encountered an operation media type that has unspecified values for the contentType for operation "${operation.fullName}".` ); } + contentTypePosition = contentTypeInfo.location; + + const assignments = ` + operationSpec = ${operation.name}$${mediaType}OperationSpec + operationArguments = { + ${overloadParameters + .map((param, index) => `${param.name}: args[${index}]`) + .join(",")} + }; + `; + conditionals.push( `if ( - [${validContentTypes - .map(type => `"${type}"`) - .join(", ")}].indexOf(contentType) > -1 + ${contentTypeInfo.values + .map(type => `args[${contentTypeInfo.location}] === "${type}"`) + .join(" || ")} ) { - operationSpec = ${operation.name}$${mediaType}OperationSpec; + ${assignments} }` ); } // Add an else clause that throws an error. This should never happen as long as a contentType was provided by the user. conditionals.push(`{ - throw new TypeError(\`"contentType" must be a valid value but instead was "\${contentType}".\`); + throw new TypeError(\`"contentType" must be a valid value but instead was "\${args[0]}".\`); }`); statements += conditionals.join(" else "); statements += `return this${ isInline ? "" : ".client" - }.sendOperationRequest({${sendParams}${ - !!sendParams ? "," : "" - }}, operationSpec) as Promise<${responseName}>`; + }.sendOperationRequest(operationArguments, operationSpec) as Promise<${responseName}>`; operationMethod.addStatements(statements); } -function getContentTypeValues( +function getContentTypeInfo( request: OperationRequestDetails -): string[] | undefined { +): { location: number; values: string[] } | undefined { const parameters = request.parameters ?? []; - for (const parameter of parameters) { + for (let i = 0; i < parameters.length; i++) { + const parameter = parameters[i]; const parameterMetadata = getLanguageMetadata(parameter.language); const paramLowerSerializedName = parameterMetadata.serializedName?.toLowerCase(); const parameterInHeader = @@ -670,15 +678,21 @@ function getContentTypeValues( paramLowerSerializedName === "content-type" && parameterInHeader ) { - return (schema as ChoiceSchema | SealedChoiceSchema).choices.map( - c => c.value as string - ); + return { + location: i, + values: (schema as ChoiceSchema | SealedChoiceSchema).choices.map( + c => c.value as string + ) + }; } else if ( schema.type === SchemaType.Constant && paramLowerSerializedName === "content-type" && parameterInHeader ) { - return [(schema as ConstantSchema).value.value]; + return { + location: i, + values: [(schema as ConstantSchema).value.value] + }; } } return; diff --git a/src/transforms/extensions.ts b/src/transforms/extensions.ts index b457f6fd35ae..0bc224271df7 100644 --- a/src/transforms/extensions.ts +++ b/src/transforms/extensions.ts @@ -36,7 +36,7 @@ function normalizeMultipleContentTypes(codeModel: CodeModel) { for (const operation of operations) { const requests = operation.requests; - if (!requests || requests.length <= 1) { + if (!requests || (requests && requests.length > 1)) { continue; } @@ -49,7 +49,7 @@ function normalizeMultipleContentTypes(codeModel: CodeModel) { for (const parameter of parameters) { const parameterMetadata = getLanguageMetadata(parameter.language); if (parameterMetadata.name.toLowerCase() === "contenttype") { - parameter.required = true; + parameter.required = false; } } } diff --git a/test/integration/generated/mediaTypes/src/mediaTypesClient.ts b/test/integration/generated/mediaTypes/src/mediaTypesClient.ts index 04fc99b78222..dc28a06f72bc 100644 --- a/test/integration/generated/mediaTypes/src/mediaTypesClient.ts +++ b/test/integration/generated/mediaTypes/src/mediaTypesClient.ts @@ -30,10 +30,12 @@ class MediaTypesClient extends MediaTypesClientContext { /** * Analyze body, that could be different media types. * @param contentType Upload file type + * @param input Input parameter. * @param options The options parameters. */ analyzeBody( contentType: ContentType, + input: coreHttp.HttpRequestBody, options?: MediaTypesClientAnalyzeBody$binaryOptionalParams ): Promise; /** @@ -47,31 +49,44 @@ class MediaTypesClient extends MediaTypesClientContext { ): Promise; /** * Analyze body, that could be different media types. - * @param contentType Upload file type - * @param options The options parameters. + * @param args Includes all the parameters for this operation. */ analyzeBody( - contentType: ContentType | "application/json", - options?: - | MediaTypesClientAnalyzeBody$binaryOptionalParams - | MediaTypesClientAnalyzeBody$jsonOptionalParams + ...args: + | [ + ContentType, + coreHttp.HttpRequestBody, + MediaTypesClientAnalyzeBody$binaryOptionalParams? + ] + | ["application/json", MediaTypesClientAnalyzeBody$jsonOptionalParams?] ): Promise { let operationSpec: coreHttp.OperationSpec; + let operationArguments: coreHttp.OperationArguments; if ( - ["application/pdf", "image/jpeg", "image/png", "image/tiff"].indexOf( - contentType - ) > -1 + args[0] === "application/pdf" || + args[0] === "image/jpeg" || + args[0] === "image/png" || + args[0] === "image/tiff" ) { operationSpec = analyzeBody$binaryOperationSpec; - } else if (["application/json"].indexOf(contentType) > -1) { + operationArguments = { + contentType: args[0], + input: args[1], + options: args[2] + }; + } else if (args[0] === "application/json") { operationSpec = analyzeBody$jsonOperationSpec; + operationArguments = { + contentType: args[0], + options: args[1] + }; } else { throw new TypeError( - `"contentType" must be a valid value but instead was "${contentType}".` + `"contentType" must be a valid value but instead was "${args[0]}".` ); } return this.sendOperationRequest( - { contentType, options }, + operationArguments, operationSpec ) as Promise; } diff --git a/test/integration/generated/mediaTypes/src/models/index.ts b/test/integration/generated/mediaTypes/src/models/index.ts index 85a82b90cf08..755b562c26fc 100644 --- a/test/integration/generated/mediaTypes/src/models/index.ts +++ b/test/integration/generated/mediaTypes/src/models/index.ts @@ -31,12 +31,7 @@ export type ContentType = * Optional parameters. */ export interface MediaTypesClientAnalyzeBody$binaryOptionalParams - extends coreHttp.OperationOptions { - /** - * Input parameter. - */ - input?: coreHttp.HttpRequestBody; -} + extends coreHttp.OperationOptions {} /** * Optional parameters. diff --git a/test/integration/generated/mediaTypes/src/models/parameters.ts b/test/integration/generated/mediaTypes/src/models/parameters.ts index e21b790e2e0f..628b2a408b77 100644 --- a/test/integration/generated/mediaTypes/src/models/parameters.ts +++ b/test/integration/generated/mediaTypes/src/models/parameters.ts @@ -27,9 +27,10 @@ export const contentType: coreHttp.OperationParameter = { }; export const input: coreHttp.OperationParameter = { - parameterPath: ["options", "input"], + parameterPath: "input", mapper: { serializedName: "input", + required: true, type: { name: "Stream" } diff --git a/test/integration/mediaType.spec.ts b/test/integration/mediaType.spec.ts index d2b6c1f74398..e9edb38daeca 100644 --- a/test/integration/mediaType.spec.ts +++ b/test/integration/mediaType.spec.ts @@ -31,9 +31,7 @@ describe("Integration tests for MediaTypes", () => { describe("#analyzeBody", () => { it("works with binary content type", async () => { - const response = await client.analyzeBody("application/pdf", { - input: "PDF" - }); + const response = await client.analyzeBody("application/pdf", "PDF"); expect(response.body).to.equal( "Nice job with PDF",