diff --git a/.chronus/changes/uptake-multipartv2-2024-4-22-16-3-12.2.md b/.chronus/changes/uptake-multipartv2-2024-4-22-16-3-12.2.md new file mode 100644 index 0000000000..84b508ab18 --- /dev/null +++ b/.chronus/changes/uptake-multipartv2-2024-4-22-16-3-12.2.md @@ -0,0 +1,8 @@ +--- +# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking +changeKind: internal +packages: + - "@azure-tools/typespec-azure-resource-manager" + - "@azure-tools/typespec-client-generator-core" + - "@azure-tools/typespec-azure-core" +--- diff --git a/.chronus/changes/uptake-multipartv2-2024-4-22-16-3-12.md b/.chronus/changes/uptake-multipartv2-2024-4-22-16-3-12.md new file mode 100644 index 0000000000..74d948dfd6 --- /dev/null +++ b/.chronus/changes/uptake-multipartv2-2024-4-22-16-3-12.md @@ -0,0 +1,8 @@ +--- +# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking +changeKind: fix +packages: + - "@azure-tools/typespec-autorest" +--- + +Add support for new multipart constructs in http library diff --git a/core b/core index 405addf0f4..40df1ec9a3 160000 --- a/core +++ b/core @@ -1 +1 @@ -Subproject commit 405addf0f44332b6d8436776c12ad805ffa735f8 +Subproject commit 40df1ec9a307a6bde664b2ad08c4f823d6956cc0 diff --git a/eng/pipelines/templates/install.yml b/eng/pipelines/templates/install.yml index 0717d63a82..da7647b03f 100644 --- a/eng/pipelines/templates/install.yml +++ b/eng/pipelines/templates/install.yml @@ -9,7 +9,7 @@ parameters: steps: - task: UseDotNet@2 inputs: - version: 6.0.x + version: 8.0.x - task: NodeTool@0 inputs: diff --git a/packages/typespec-autorest/src/openapi.ts b/packages/typespec-autorest/src/openapi.ts index 191693c9d0..11b7de2a2f 100644 --- a/packages/typespec-autorest/src/openapi.ts +++ b/packages/typespec-autorest/src/openapi.ts @@ -90,9 +90,10 @@ import { Authentication, HttpAuth, HttpOperation, + HttpOperationBody, + HttpOperationMultipartBody, HttpOperationParameters, HttpOperationResponse, - HttpOperationResponseBody, HttpStatusCodeRange, HttpStatusCodesEntry, MetadataInfo, @@ -128,6 +129,7 @@ import { sortWithJsonSchema } from "./json-schema-sorter/sorter.js"; import { createDiagnostic, reportDiagnostic } from "./lib.js"; import { OpenAPI2Document, + OpenAPI2FileSchema, OpenAPI2FormDataParameter, OpenAPI2HeaderDefinition, OpenAPI2OAuth2FlowType, @@ -717,7 +719,7 @@ export async function getOpenAPIForService( openapiResponse["x-ms-error-response"] = true; } const contentTypes: string[] = []; - let body: HttpOperationResponseBody | undefined; + let body: HttpOperationBody | HttpOperationMultipartBody | undefined; for (const data of response.responses) { if (data.headers && Object.keys(data.headers).length > 0) { openapiResponse.headers ??= {}; @@ -739,13 +741,7 @@ export async function getOpenAPIForService( } if (body) { - const isBinary = contentTypes.every((t) => isBinaryPayload(body!.type, t)); - openapiResponse.schema = isBinary - ? { type: "file" } - : getSchemaOrRef(body.type, { - visibility: Visibility.Read, - ignoreMetadataAnnotations: body.isExplicit && body.containsMetadataAnnotations, - }); + openapiResponse.schema = getSchemaForResponseBody(body, contentTypes); } for (const contentType of contentTypes) { @@ -755,6 +751,24 @@ export async function getOpenAPIForService( currentEndpoint.responses![statusCode] = openapiResponse; } + function getSchemaForResponseBody( + body: HttpOperationBody | HttpOperationMultipartBody, + contentTypes: string[] + ): OpenAPI2Schema | OpenAPI2FileSchema { + const isBinary = contentTypes.every((t) => isBinaryPayload(body!.type, t)); + if (isBinary) { + return { type: "file" }; + } + if (body.bodyKind === "multipart") { + // OpenAPI2 doesn't support multipart responses, so we just return a string schema + return { type: "string" }; + } + return getSchemaOrRef(body.type, { + visibility: Visibility.Read, + ignoreMetadataAnnotations: body.isExplicit && body.containsMetadataAnnotations, + }); + } + function getResponseHeader(prop: ModelProperty): OpenAPI2HeaderDefinition { const header: any = {}; populateParameter(header, prop, "header", { @@ -954,39 +968,81 @@ export async function getOpenAPIForService( } if (methodParams.body && !isVoidType(methodParams.body.type)) { - const isBinary = isBinaryPayload(methodParams.body.type, consumes); - const schemaContext = { - visibility, - ignoreMetadataAnnotations: - methodParams.body.isExplicit && methodParams.body.containsMetadataAnnotations, - }; - const schema = isBinary - ? { type: "string", format: "binary" } - : getSchemaOrRef(methodParams.body.type, schemaContext); - - if (currentConsumes.has("multipart/form-data")) { - const bodyModelType = methodParams.body.type; - // Assert, this should never happen. Rest library guard against that. - compilerAssert(bodyModelType.kind === "Model", "Body should always be a Model."); - if (bodyModelType) { - for (const param of bodyModelType.properties.values()) { - emitParameter(param, "formData", schemaContext, getJsonName(param)); - } + emitBodyParameters(methodParams.body, visibility); + } + } + + function emitBodyParameters( + body: HttpOperationBody | HttpOperationMultipartBody, + visibility: Visibility + ) { + switch (body.bodyKind) { + case "single": + emitSingleBodyParameters(body, visibility); + break; + case "multipart": + emitMultipartBodyParameters(body, visibility); + break; + } + } + + function emitSingleBodyParameters(body: HttpOperationBody, visibility: Visibility) { + const isBinary = isBinaryPayload(body.type, body.contentTypes); + const schemaContext = { + visibility, + ignoreMetadataAnnotations: body.isExplicit && body.containsMetadataAnnotations, + }; + const schema = isBinary + ? { type: "string", format: "binary" } + : getSchemaOrRef(body.type, schemaContext); + + if (currentConsumes.has("multipart/form-data")) { + const bodyModelType = body.type; + // Assert, this should never happen. Rest library guard against that. + compilerAssert(bodyModelType.kind === "Model", "Body should always be a Model."); + if (bodyModelType) { + for (const param of bodyModelType.properties.values()) { + emitParameter(param, "formData", schemaContext, getJsonName(param)); + } + } + } else if (body.property) { + emitParameter( + body.property, + "body", + { visibility, ignoreMetadataAnnotations: false }, + getJsonName(body.property), + schema + ); + } else { + currentEndpoint.parameters.push({ + name: "body", + in: "body", + schema, + required: true, + }); + } + } + + function emitMultipartBodyParameters(body: HttpOperationMultipartBody, visibility: Visibility) { + for (const [index, part] of body.parts.entries()) { + const partName = part.name ?? `part${index}`; + let schema = getFormDataSchema( + part.body.type, + { visibility, ignoreMetadataAnnotations: false }, + partName + ); + if (schema) { + if (part.multi) { + schema = { + type: "array", + items: schema.type === "file" ? { type: "string", format: "binary" } : schema, + }; } - } else if (methodParams.body.parameter) { - emitParameter( - methodParams.body.parameter, - "body", - { visibility, ignoreMetadataAnnotations: false }, - getJsonName(methodParams.body.parameter), - schema - ); - } else { currentEndpoint.parameters.push({ - name: "body", - in: "body", - schema, - required: true, + name: partName, + in: "formData", + required: !part.optional, + ...schema, }); } } diff --git a/packages/typespec-autorest/src/openapi2-document.ts b/packages/typespec-autorest/src/openapi2-document.ts index fba900eabd..b6d041394e 100644 --- a/packages/typespec-autorest/src/openapi2-document.ts +++ b/packages/typespec-autorest/src/openapi2-document.ts @@ -285,6 +285,18 @@ export type OpenAPI2Schema = Extensions & { "x-ms-mutability"?: string[]; }; +export type OpenAPI2FileSchema = { + type: "file"; + format?: string; + title?: string; + description?: string; + default?: unknown; + required?: string[]; + readonly?: boolean; + externalDocs?: OpenAPI2ExternalDocs; + example?: unknown; +}; + export type OpenAPI2ParameterType = OpenAPI2Parameter["in"]; export interface OpenAPI2HeaderDefinition { @@ -465,7 +477,7 @@ export interface OpenAPI2Response { /** A short description of the response. Commonmark syntax can be used for rich text representation */ description: string; /** A definition of the response structure. It can be a primitive, an array or an object. If this field does not exist, it means no content is returned as part of the response. As an extension to the Schema Object, its root type value may also be "file". This SHOULD be accompanied by a relevant produces mime-type. */ - schema?: OpenAPI2Schema; + schema?: OpenAPI2Schema | OpenAPI2FileSchema; /** A list of headers that are sent with the response. */ headers?: Record; /** An example of the response message. */ diff --git a/packages/typespec-autorest/test/multipart.test.ts b/packages/typespec-autorest/test/multipart.test.ts index 5310a6f263..f4ba1520c2 100644 --- a/packages/typespec-autorest/test/multipart.test.ts +++ b/packages/typespec-autorest/test/multipart.test.ts @@ -2,7 +2,105 @@ import { deepStrictEqual } from "assert"; import { describe, it } from "vitest"; import { openApiFor } from "./test-host.js"; -describe("typespec-autorest: multipart", () => { +it("model properties are spread into individual parameters", async () => { + const res = await openApiFor( + ` + model Form { name: HttpPart, profileImage: HttpPart } + op upload(@header contentType: "multipart/form-data", @multipartBody body: Form): void; + ` + ); + const op = res.paths["/"].post; + deepStrictEqual(op.parameters, [ + { + in: "formData", + name: "name", + required: true, + type: "string", + }, + { + in: "formData", + name: "profileImage", + required: true, + type: "file", + }, + ]); +}); + +it("part of type `bytes` produce `type: file`", async () => { + const res = await openApiFor( + ` + op upload(@header contentType: "multipart/form-data", @multipartBody body: { profileImage: HttpPart }): void; + ` + ); + const op = res.paths["/"].post; + deepStrictEqual(op.parameters, [ + { + in: "formData", + name: "profileImage", + required: true, + type: "file", + }, + ]); +}); + +it("part of type `bytes[]` produce `type: array, items: { type: string, format: binary }`", async () => { + const res = await openApiFor( + ` + op upload(@header contentType: "multipart/form-data", @multipartBody _: { profileImage: HttpPart[]}): void; + ` + ); + const op = res.paths["/"].post; + deepStrictEqual(op.parameters, [ + { + in: "formData", + name: "profileImage", + required: true, + type: "array", + items: { + type: "string", + format: "binary", + }, + }, + ]); +}); + +it("part of type `string` produce `type: string`", async () => { + const res = await openApiFor( + ` + op upload(@header contentType: "multipart/form-data", @multipartBody body: { name: HttpPart }): void; + ` + ); + const op = res.paths["/"].post; + deepStrictEqual(op.parameters, [ + { + in: "formData", + name: "name", + required: true, + type: "string", + }, + ]); +}); + +// https://github.com/Azure/typespec-azure/issues/3860 +it("part of type `object` produce `type: string`", async () => { + const res = await openApiFor( + ` + #suppress "@azure-tools/typespec-autorest/unsupported-multipart-type" "For test" + op upload(@header contentType: "multipart/form-data", @multipartBody _: { address: HttpPart<{city: string, street: string}>}): void; + ` + ); + const op = res.paths["/"].post; + deepStrictEqual(op.parameters, [ + { + in: "formData", + name: "address", + required: true, + type: "string", + }, + ]); +}); + +describe("legacy implicit form", () => { it("part of type `bytes` produce `type: file`", async () => { const res = await openApiFor( ` diff --git a/packages/typespec-azure-core/src/lro-info.ts b/packages/typespec-azure-core/src/lro-info.ts index 7cbc9ef951..1997d23486 100644 --- a/packages/typespec-azure-core/src/lro-info.ts +++ b/packages/typespec-azure-core/src/lro-info.ts @@ -137,8 +137,8 @@ export function getLroOperationInfo( } if (targetParameters.body) { const body = targetParameters.body; - if (body.parameter) { - targetProperties.set(body.parameter.name, body.parameter); + if (body.bodyKind === "single" && body.property) { + targetProperties.set(body.property.name, body.property); } else if (body.type.kind === "Model") { for (const [name, param] of getAllProperties(body.type)) { targetProperties.set(name, param); diff --git a/packages/typespec-azure-core/test/test-host.ts b/packages/typespec-azure-core/test/test-host.ts index 7a4fef050e..c2cfc60f02 100644 --- a/packages/typespec-azure-core/test/test-host.ts +++ b/packages/typespec-azure-core/test/test-host.ts @@ -102,7 +102,7 @@ export async function getSimplifiedOperations( params: { params: r.parameters.parameters.map(({ type, name }) => ({ type, name })), body: - r.parameters.body?.parameter?.name ?? + r.parameters.body?.property?.name ?? (r.parameters.body?.type?.kind === "Model" ? Array.from(r.parameters.body.type.properties.keys()) : undefined), diff --git a/packages/typespec-azure-playground-website/samples/azure-core.tsp b/packages/typespec-azure-playground-website/samples/azure-core.tsp index 9bae9beb73..ed597517a2 100644 --- a/packages/typespec-azure-playground-website/samples/azure-core.tsp +++ b/packages/typespec-azure-playground-website/samples/azure-core.tsp @@ -256,7 +256,7 @@ interface WidgetParts { reorderParts is Operations.LongRunningResourceCollectionAction< WidgetPart, WidgetPartReorderRequest, - TypeSpec.Http.AcceptedResponse + never >; } diff --git a/packages/typespec-azure-resource-manager/src/rules/arm-post-response-codes.ts b/packages/typespec-azure-resource-manager/src/rules/arm-post-response-codes.ts index 369b216eeb..2e1063b49c 100644 --- a/packages/typespec-azure-resource-manager/src/rules/arm-post-response-codes.ts +++ b/packages/typespec-azure-resource-manager/src/rules/arm-post-response-codes.ts @@ -1,7 +1,11 @@ import { Program, createRule } from "@typespec/compiler"; import { getLroMetadata } from "@azure-tools/typespec-azure-core"; -import { HttpOperationBody, HttpOperationResponse } from "@typespec/http"; +import { + HttpOperationBody, + HttpOperationMultipartBody, + HttpOperationResponse, +} from "@typespec/http"; import { ArmResourceOperation } from "../operations.js"; import { getArmResources } from "../resource.js"; @@ -20,7 +24,7 @@ export const armPostResponseCodesRule = createRule({ create(context) { function getResponseBody( response: HttpOperationResponse | undefined - ): HttpOperationBody | undefined { + ): HttpOperationBody | HttpOperationMultipartBody | undefined { if (response === undefined) return undefined; if (response.responses.length > 1) { throw new Error("Multiple responses are not supported."); diff --git a/packages/typespec-azure-resource-manager/test/rules/arm-post-response-codes.test.ts b/packages/typespec-azure-resource-manager/test/rules/arm-post-response-codes.test.ts index 1651b020eb..2569c581d1 100644 --- a/packages/typespec-azure-resource-manager/test/rules/arm-post-response-codes.test.ts +++ b/packages/typespec-azure-resource-manager/test/rules/arm-post-response-codes.test.ts @@ -53,6 +53,41 @@ it("Emits a warning for a synchronous post operation that does not contain the a }); }); +it("Emits a warning for a synchronous post operation that does not contain the appropriate response codes and use multipart/mixed", async () => { + await tester + .expect( + ` + @armProviderNamespace + @useDependency(Azure.ResourceManager.Versions.v1_0_Preview_1) + namespace Microsoft.Contoso; + + model Employee is ProxyResource<{}> { + @pattern("^[a-zA-Z0-9-]{3,24}$") + @key("employeeName") + @path + @segment("employees") + name: string; + } + + @armResourceOperations + interface Employees { + @post + @armResourceAction(Employee) + hire(...ApiVersionParameter): { + @statusCode _: 203; + @header contentType: "multipart/mixed"; + @multipartBody result: [HttpPart] + } | ErrorResponse; + } + ` + ) + .toEmitDiagnostics({ + code: "@azure-tools/typespec-azure-resource-manager/arm-post-operation-response-codes", + message: + "Synchronous post operations must have a 200 or 204 response and a default response. They must not have any other responses.", + }); +}); + it("Does not emit a warning for a synchronous post operation that contains the 200 and default response codes", async () => { await tester .expect( diff --git a/packages/typespec-client-generator-core/src/http.ts b/packages/typespec-client-generator-core/src/http.ts index 4f80cbbbee..21e68c8dd9 100644 --- a/packages/typespec-client-generator-core/src/http.ts +++ b/packages/typespec-client-generator-core/src/http.ts @@ -124,7 +124,7 @@ function getSdkHttpParameters( 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 (tspBody && tspBody?.bodyKind !== "multipart") { // if there's a param on the body, we can just rely on getSdkHttpParameter if (tspBody.parameter && !isNeverOrVoidType(tspBody.parameter.type)) { const getParamResponse = diagnostics.pipe( diff --git a/packages/typespec-client-generator-core/src/package.ts b/packages/typespec-client-generator-core/src/package.ts index c2cbcb24eb..899a968724 100644 --- a/packages/typespec-client-generator-core/src/package.ts +++ b/packages/typespec-client-generator-core/src/package.ts @@ -254,9 +254,13 @@ function getSdkBasicServiceMethod< methodParameters.push(diagnostics.pipe(getSdkMethodParameter(context, param.param, operation))); } // body parameters - if (parameters.body?.parameter && !isNeverOrVoidType(parameters.body.parameter.type)) { + if ( + parameters.body?.bodyKind !== "multipart" && + parameters.body?.property && + !isNeverOrVoidType(parameters.body.property.type) + ) { methodParameters.push( - diagnostics.pipe(getSdkMethodParameter(context, parameters.body?.parameter, operation)) + diagnostics.pipe(getSdkMethodParameter(context, parameters.body?.property, operation)) ); } else if (parameters.body && !isNeverOrVoidType(parameters.body.type)) { if (parameters.body.type.kind === "Model") { diff --git a/packages/typespec-client-generator-core/tsconfig.json b/packages/typespec-client-generator-core/tsconfig.json index 1aef16e920..b58c2a0526 100644 --- a/packages/typespec-client-generator-core/tsconfig.json +++ b/packages/typespec-client-generator-core/tsconfig.json @@ -3,7 +3,9 @@ "composite": true, "references": [ { "path": "../../core/packages/compiler/tsconfig.json" }, - { "path": "../../core/packages/rest/tsconfig.json" } + { "path": "../../core/packages/http/tsconfig.json" }, + { "path": "../typespec-azure-core/tsconfig.json" }, + { "path": "../typespec-azure-resource-manager/tsconfig.json" } ], "compilerOptions": { "outDir": "dist", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 89a9ba2673..5db8b370b4 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -114,6 +114,9 @@ importers: '@typescript-eslint/utils': specifier: ^7.9.0 version: 7.9.0(eslint@8.57.0)(typescript@5.4.5) + '@vitest/coverage-v8': + specifier: ^1.6.0 + version: 1.6.0(vitest@1.6.0) c8: specifier: ^9.1.0 version: 9.1.0