From e74bdae6243d39996fb2c456303e594d8920354c Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Fri, 10 May 2024 09:10:36 -0700 Subject: [PATCH 01/39] Add new decorators --- .vscode/settings.json | 1 + .../generated-defs/TypeSpec.Http.Private.ts | 2 + packages/http/generated-defs/TypeSpec.Http.ts | 17 +++++++ .../generated-defs/TypeSpec.Http.ts-test.ts | 4 ++ .../{http-decorators.tsp => decorators.tsp} | 22 +++++++--- packages/http/lib/http.tsp | 10 ++++- packages/http/lib/private.decorators.tsp | 9 ++++ packages/http/src/decorators.ts | 44 +++++-------------- packages/http/src/lib.ts | 4 ++ packages/http/src/private.decorators.ts | 37 ++++++++++++++++ 10 files changed, 110 insertions(+), 40 deletions(-) rename packages/http/lib/{http-decorators.tsp => decorators.tsp} (96%) create mode 100644 packages/http/lib/private.decorators.tsp create mode 100644 packages/http/src/private.decorators.ts diff --git a/.vscode/settings.json b/.vscode/settings.json index f8e1479646..8e894d52f4 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -10,6 +10,7 @@ "**/node_modules/**": true, "packages/compiler/templates/__snapshots__/**": true, "packages/website/versioned_docs/**": true, + "packages/http-client-csharp/**/Generated/**": true, "packages/samples/scratch/**": false // Those files are in gitignore but we still want to search for them }, "files.exclude": { diff --git a/packages/http/generated-defs/TypeSpec.Http.Private.ts b/packages/http/generated-defs/TypeSpec.Http.Private.ts index c1bbdcf252..3ccbb2698b 100644 --- a/packages/http/generated-defs/TypeSpec.Http.Private.ts +++ b/packages/http/generated-defs/TypeSpec.Http.Private.ts @@ -1,3 +1,5 @@ import type { DecoratorContext, Model } from "@typespec/compiler"; export type PlainDataDecorator = (context: DecoratorContext, target: Model) => void; + +export type HttpFileDecorator = (context: DecoratorContext, target: Model) => void; diff --git a/packages/http/generated-defs/TypeSpec.Http.ts b/packages/http/generated-defs/TypeSpec.Http.ts index 449ecf2904..4fe55b3b75 100644 --- a/packages/http/generated-defs/TypeSpec.Http.ts +++ b/packages/http/generated-defs/TypeSpec.Http.ts @@ -111,6 +111,23 @@ export type BodyRootDecorator = (context: DecoratorContext, target: ModelPropert */ export type BodyIgnoreDecorator = (context: DecoratorContext, target: ModelProperty) => void; +/** + * + * + * + * @example + * ```tsp + * op upload( + * @header `content-type`: "multipart/form-data", + * @multipartBody body: { + * fullName: HttpPart, + * headShots: HttpPart[] + * } + * ): void; + * ``` + */ +export type MultipartBodyDecorator = (context: DecoratorContext, target: ModelProperty) => void; + /** * Specify the HTTP verb for the target operation to be `GET`. * diff --git a/packages/http/generated-defs/TypeSpec.Http.ts-test.ts b/packages/http/generated-defs/TypeSpec.Http.ts-test.ts index efd098a96a..fcdfdf9078 100644 --- a/packages/http/generated-defs/TypeSpec.Http.ts-test.ts +++ b/packages/http/generated-defs/TypeSpec.Http.ts-test.ts @@ -8,6 +8,7 @@ import { $head, $header, $includeInapplicableMetadataInPayload, + $multipartBody, $patch, $path, $post, @@ -28,6 +29,7 @@ import type { HeadDecorator, HeaderDecorator, IncludeInapplicableMetadataInPayloadDecorator, + MultipartBodyDecorator, PatchDecorator, PathDecorator, PostDecorator, @@ -48,6 +50,7 @@ type Decorators = { $path: PathDecorator; $bodyRoot: BodyRootDecorator; $bodyIgnore: BodyIgnoreDecorator; + $multipartBody: MultipartBodyDecorator; $get: GetDecorator; $put: PutDecorator; $post: PostDecorator; @@ -70,6 +73,7 @@ const _: Decorators = { $path, $bodyRoot, $bodyIgnore, + $multipartBody, $get, $put, $post, diff --git a/packages/http/lib/http-decorators.tsp b/packages/http/lib/decorators.tsp similarity index 96% rename from packages/http/lib/http-decorators.tsp rename to packages/http/lib/decorators.tsp index f8631f23cb..231da35d39 100644 --- a/packages/http/lib/http-decorators.tsp +++ b/packages/http/lib/decorators.tsp @@ -122,6 +122,21 @@ extern dec bodyRoot(target: ModelProperty); */ extern dec bodyIgnore(target: ModelProperty); +/** + * @example + * + * ```tsp + * op upload( + * @header `content-type`: "multipart/form-data", + * @multipartBody body: { + * fullName: HttpPart, + * headShots: HttpPart[] + * } + * ): void; + * ``` + */ +extern dec multipartBody(target: ModelProperty); + /** * Specify the status code for this response. Property type must be a status code integer or a union of status code integer. * @@ -291,10 +306,3 @@ extern dec route( * ``` */ extern dec sharedRoute(target: Operation); - -/** - * Private decorators. Those are meant for internal use inside Http types only. - */ -namespace Private { - extern dec plainData(target: TypeSpec.Reflection.Model); -} diff --git a/packages/http/lib/http.tsp b/packages/http/lib/http.tsp index 1b1d8d7557..d812ca5cce 100644 --- a/packages/http/lib/http.tsp +++ b/packages/http/lib/http.tsp @@ -1,5 +1,6 @@ import "../dist/src/index.js"; -import "./http-decorators.tsp"; +import "./decorators.tsp"; +import "./private.decorators.tsp"; import "./auth.tsp"; namespace TypeSpec.Http; @@ -104,3 +105,10 @@ model ConflictResponse is Response<409>; model PlainData { ...Data; } + +@Private.httpFile +model File { + contentType?: string; + filename?: string; + contents: bytes; +} diff --git a/packages/http/lib/private.decorators.tsp b/packages/http/lib/private.decorators.tsp new file mode 100644 index 0000000000..35052bc888 --- /dev/null +++ b/packages/http/lib/private.decorators.tsp @@ -0,0 +1,9 @@ +import "../dist/src/private.decorators.js"; + +/** + * Private decorators. Those are meant for internal use inside Http types only. + */ +namespace TypeSpec.Http.Private; + +extern dec plainData(target: TypeSpec.Reflection.Model); +extern dec httpFile(target: TypeSpec.Reflection.Model); diff --git a/packages/http/src/decorators.ts b/packages/http/src/decorators.ts index 7325cd5531..3d300e4909 100644 --- a/packages/http/src/decorators.ts +++ b/packages/http/src/decorators.ts @@ -17,12 +17,10 @@ import { ignoreDiagnostics, isArrayModelType, reportDeprecated, - setTypeSpecNamespace, typespecTypeToJson, validateDecoratorTarget, validateDecoratorUniqueOnNode, } from "@typespec/compiler"; -import { PlainDataDecorator } from "../generated-defs/TypeSpec.Http.Private.js"; import { BodyDecorator, BodyIgnoreDecorator, @@ -31,6 +29,7 @@ import { GetDecorator, HeadDecorator, HeaderDecorator, + MultipartBodyDecorator, PatchDecorator, PathDecorator, PostDecorator, @@ -221,6 +220,17 @@ export function isBodyIgnore(program: Program, entity: ModelProperty): boolean { return program.stateSet(HttpStateKeys.bodyIgnore).has(entity); } +export const $multipartBody: MultipartBodyDecorator = ( + context: DecoratorContext, + entity: ModelProperty +) => { + context.program.stateSet(HttpStateKeys.multipartBody).add(entity); +}; + +export function isMultipartBody(program: Program, entity: Type): boolean { + return program.stateSet(HttpStateKeys.multipartBody).has(entity); +} + export const $statusCode: StatusCodeDecorator = ( context: DecoratorContext, entity: ModelProperty @@ -450,36 +460,6 @@ export function getServers(program: Program, type: Namespace): HttpServer[] | un return program.stateMap(HttpStateKeys.servers).get(type); } -export const $plainData: PlainDataDecorator = (context: DecoratorContext, entity: Model) => { - const { program } = context; - - const decoratorsToRemove = ["$header", "$body", "$query", "$path", "$statusCode"]; - const [headers, bodies, queries, paths, statusCodes] = [ - program.stateMap(HttpStateKeys.header), - program.stateSet(HttpStateKeys.body), - program.stateMap(HttpStateKeys.query), - program.stateMap(HttpStateKeys.path), - program.stateMap(HttpStateKeys.statusCode), - ]; - - for (const property of entity.properties.values()) { - // Remove the decorators so that they do not run in the future, for example, - // if this model is later spread into another. - property.decorators = property.decorators.filter( - (d) => !decoratorsToRemove.includes(d.decorator.name) - ); - - // Remove the impact the decorators already had on this model. - headers.delete(property); - bodies.delete(property); - queries.delete(property); - paths.delete(property); - statusCodes.delete(property); - } -}; - -setTypeSpecNamespace("Private", $plainData); - export function $useAuth( context: DecoratorContext, entity: Namespace | Interface | Operation, diff --git a/packages/http/src/lib.ts b/packages/http/src/lib.ts index 8d8ed02b7a..5cf0f27102 100644 --- a/packages/http/src/lib.ts +++ b/packages/http/src/lib.ts @@ -144,6 +144,7 @@ export const $lib = createTypeSpecLibrary({ body: { description: "State for the @body decorator" }, bodyRoot: { description: "State for the @bodyRoot decorator" }, bodyIgnore: { description: "State for the @bodyIgnore decorator" }, + multipartBody: { description: "State for the @bodyIgnore decorator" }, statusCode: { description: "State for the @statusCode decorator" }, verbs: { description: "State for the verb decorators (@get, @post, @put, etc.)" }, servers: { description: "State for the @server decorator" }, @@ -157,6 +158,9 @@ export const $lib = createTypeSpecLibrary({ routes: {}, sharedRoutes: { description: "State for the @sharedRoute decorator" }, routeOptions: {}, + + // private + file: { description: "State for the @Private.file decorator" }, }, } as const); diff --git a/packages/http/src/private.decorators.ts b/packages/http/src/private.decorators.ts new file mode 100644 index 0000000000..cb63b44fdf --- /dev/null +++ b/packages/http/src/private.decorators.ts @@ -0,0 +1,37 @@ +import { DecoratorContext, Model } from "@typespec/compiler"; +import { HttpFileDecorator, PlainDataDecorator } from "../generated-defs/TypeSpec.Http.Private.js"; +import { HttpStateKeys } from "./lib.js"; + +export const namespace = "TypeSpec.Http.Private"; + +export const $plainData: PlainDataDecorator = (context: DecoratorContext, entity: Model) => { + const { program } = context; + + const decoratorsToRemove = ["$header", "$body", "$query", "$path", "$statusCode"]; + const [headers, bodies, queries, paths, statusCodes] = [ + program.stateMap(HttpStateKeys.header), + program.stateSet(HttpStateKeys.body), + program.stateMap(HttpStateKeys.query), + program.stateMap(HttpStateKeys.path), + program.stateMap(HttpStateKeys.statusCode), + ]; + + for (const property of entity.properties.values()) { + // Remove the decorators so that they do not run in the future, for example, + // if this model is later spread into another. + property.decorators = property.decorators.filter( + (d) => !decoratorsToRemove.includes(d.decorator.name) + ); + + // Remove the impact the decorators already had on this model. + headers.delete(property); + bodies.delete(property); + queries.delete(property); + paths.delete(property); + statusCodes.delete(property); + } +}; + +export const $httpFile: HttpFileDecorator = (context: DecoratorContext, entity: Model) => { + context.program.stateSet(HttpStateKeys.file).add(entity); +}; From c1aa4c3bbad10a3f4dca0703ed0000b2bb42cd48 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Mon, 13 May 2024 08:45:14 -0700 Subject: [PATCH 02/39] Add other models and decorators --- .../generated-defs/TypeSpec.Http.Private.ts | 9 +++- packages/http/lib/http.tsp | 8 ++++ packages/http/lib/private.decorators.tsp | 5 +++ packages/http/src/index.ts | 1 + packages/http/src/lib.ts | 1 + packages/http/src/private.decorators.ts | 42 +++++++++++++++++-- 6 files changed, 61 insertions(+), 5 deletions(-) diff --git a/packages/http/generated-defs/TypeSpec.Http.Private.ts b/packages/http/generated-defs/TypeSpec.Http.Private.ts index 3ccbb2698b..c78126bd7a 100644 --- a/packages/http/generated-defs/TypeSpec.Http.Private.ts +++ b/packages/http/generated-defs/TypeSpec.Http.Private.ts @@ -1,5 +1,12 @@ -import type { DecoratorContext, Model } from "@typespec/compiler"; +import type { DecoratorContext, Model, Type } from "@typespec/compiler"; export type PlainDataDecorator = (context: DecoratorContext, target: Model) => void; export type HttpFileDecorator = (context: DecoratorContext, target: Model) => void; + +export type HttpPartDecorator = ( + context: DecoratorContext, + target: Model, + type: Type, + options: unknown +) => void; diff --git a/packages/http/lib/http.tsp b/packages/http/lib/http.tsp index d812ca5cce..411a7c1f3d 100644 --- a/packages/http/lib/http.tsp +++ b/packages/http/lib/http.tsp @@ -112,3 +112,11 @@ model File { filename?: string; contents: bytes; } + +model HttpPartOptions { + /** Name of the part when using the array form. */ + name?: string; +} + +@Private.httpPart(Type, Options) +model HttpPart {} diff --git a/packages/http/lib/private.decorators.tsp b/packages/http/lib/private.decorators.tsp index 35052bc888..87b4649870 100644 --- a/packages/http/lib/private.decorators.tsp +++ b/packages/http/lib/private.decorators.tsp @@ -7,3 +7,8 @@ namespace TypeSpec.Http.Private; extern dec plainData(target: TypeSpec.Reflection.Model); extern dec httpFile(target: TypeSpec.Reflection.Model); +extern dec httpPart( + target: TypeSpec.Reflection.Model, + type: unknown, + options: valueof HttpPartOptions +); diff --git a/packages/http/src/index.ts b/packages/http/src/index.ts index c3d8bcd881..1ae4ce2da0 100644 --- a/packages/http/src/index.ts +++ b/packages/http/src/index.ts @@ -7,6 +7,7 @@ export * from "./decorators.js"; export * from "./metadata.js"; export * from "./operations.js"; export * from "./parameters.js"; +export { isHttpFile } from "./private.decorators.js"; export * from "./responses.js"; export * from "./route.js"; export * from "./types.js"; diff --git a/packages/http/src/lib.ts b/packages/http/src/lib.ts index 5cf0f27102..cb2fa43a8e 100644 --- a/packages/http/src/lib.ts +++ b/packages/http/src/lib.ts @@ -161,6 +161,7 @@ export const $lib = createTypeSpecLibrary({ // private file: { description: "State for the @Private.file decorator" }, + httpPart: { description: "State for the @Private.httpPart decorator" }, }, } as const); diff --git a/packages/http/src/private.decorators.ts b/packages/http/src/private.decorators.ts index cb63b44fdf..0b28089bc3 100644 --- a/packages/http/src/private.decorators.ts +++ b/packages/http/src/private.decorators.ts @@ -1,5 +1,9 @@ -import { DecoratorContext, Model } from "@typespec/compiler"; -import { HttpFileDecorator, PlainDataDecorator } from "../generated-defs/TypeSpec.Http.Private.js"; +import { DecoratorContext, Model, Program, Type } from "@typespec/compiler"; +import { + HttpFileDecorator, + HttpPartDecorator, + PlainDataDecorator, +} from "../generated-defs/TypeSpec.Http.Private.js"; import { HttpStateKeys } from "./lib.js"; export const namespace = "TypeSpec.Http.Private"; @@ -32,6 +36,36 @@ export const $plainData: PlainDataDecorator = (context: DecoratorContext, entity } }; -export const $httpFile: HttpFileDecorator = (context: DecoratorContext, entity: Model) => { - context.program.stateSet(HttpStateKeys.file).add(entity); +export const $httpFile: HttpFileDecorator = (context: DecoratorContext, target: Model) => { + context.program.stateSet(HttpStateKeys.file).add(target); }; + +/** + * Check if the given type is an `HttpFile` + */ +export function isHttpFile(program: Program, target: Type) { + return program.stateSet(HttpStateKeys.file).has(target); +} + +export interface HttpPartOptions { + readonly name?: string; +} + +export const $httpPart: HttpPartDecorator = ( + context: DecoratorContext, + target: Model, + type, + options +) => { + context.program.stateMap(HttpStateKeys.file).set(target, { type, options }); +}; + +export interface HttpPart { + readonly type: Type; + readonly options: HttpPartOptions; +} + +/** Return the http part information on a model that is an `HttpPart` */ +export function getHttpPart(program: Program, target: Type): HttpPart | undefined { + return program.stateMap(HttpStateKeys.file).get(target); +} From 8c5f8b1531fbbc294c514087e71ae5f29dfb322a Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Mon, 13 May 2024 10:02:00 -0700 Subject: [PATCH 03/39] Add new types --- packages/http/src/body.ts | 18 ++------ packages/http/src/parameters.ts | 19 ++++---- packages/http/src/responses.ts | 9 ++-- packages/http/src/types.ts | 78 +++++++++++++++++++++----------- packages/http/test/test-host.ts | 2 +- packages/openapi3/src/openapi.ts | 65 ++++++++++++++++++-------- 6 files changed, 117 insertions(+), 74 deletions(-) diff --git a/packages/http/src/body.ts b/packages/http/src/body.ts index 462396bc4e..6900359081 100644 --- a/packages/http/src/body.ts +++ b/packages/http/src/body.ts @@ -1,6 +1,5 @@ import { Diagnostic, - DuplicateTracker, ModelProperty, Program, Type, @@ -10,6 +9,7 @@ import { isArrayModelType, navigateType, } from "@typespec/compiler"; +import { DuplicateTracker } from "@typespec/compiler/utils"; import { isBody, isBodyRoot, @@ -20,16 +20,7 @@ import { } from "./decorators.js"; import { createDiagnostic } from "./lib.js"; import { Visibility, isVisible } from "./metadata.js"; - -export interface ResolvedBody { - readonly type: Type; - /** `true` if the body was specified with `@body` */ - readonly isExplicit: boolean; - /** If the body original model contained property annotated with metadata properties. */ - readonly containsMetadataAnnotations: boolean; - /** If body is defined with `@body` or `@bodyRoot` this is the property */ - readonly property?: ModelProperty; -} +import { HttpBody } from "./types.js"; export function resolveBody( program: Program, @@ -38,11 +29,12 @@ export function resolveBody( rootPropertyMap: Map, visibility: Visibility, usedIn: "request" | "response" -): [ResolvedBody | undefined, readonly Diagnostic[]] { +): [HttpBody | undefined, readonly Diagnostic[]] { const diagnostics = createDiagnosticCollector(); // non-model or intrinsic/array model -> response body is response type if (requestOrResponseType.kind !== "Model" || isArrayModelType(program, requestOrResponseType)) { return diagnostics.wrap({ + bodyKind: "single", type: requestOrResponseType, isExplicit: false, containsMetadataAnnotations: false, @@ -52,7 +44,7 @@ export function resolveBody( const duplicateTracker = new DuplicateTracker(); // look for explicit body - let resolvedBody: ResolvedBody | undefined; + let resolvedBody: HttpBody | undefined; for (const property of metadata) { const isBodyVal = isBody(program, property); const isBodyRootVal = isBodyRoot(program, property); diff --git a/packages/http/src/parameters.ts b/packages/http/src/parameters.ts index 1b26aa9ace..4e9fc88019 100644 --- a/packages/http/src/parameters.ts +++ b/packages/http/src/parameters.ts @@ -5,7 +5,7 @@ import { Operation, Program, } from "@typespec/compiler"; -import { resolveBody, ResolvedBody } from "./body.js"; +import { resolveBody } from "./body.js"; import { getContentTypes, isContentTypeHeader } from "./content-types.js"; import { getHeaderFieldOptions, @@ -18,10 +18,11 @@ import { import { createDiagnostic } from "./lib.js"; import { gatherMetadata, isMetadata, resolveRequestVisibility } from "./metadata.js"; import { + HttpBody, HttpOperation, + HttpOperationBody, HttpOperationParameter, HttpOperationParameters, - HttpOperationRequestBody, HttpVerb, OperationParameterOptions, } from "./types.js"; @@ -145,16 +146,16 @@ function getOperationParametersForVerb( return body?.type; }, get bodyParameter() { - return body?.parameter; + return body?.property; }, }); } function computeHttpOperationBody( operation: Operation, - resolvedBody: ResolvedBody | undefined, + resolvedBody: HttpBody | undefined, contentTypes: string[] | undefined -): [HttpOperationRequestBody | undefined, readonly Diagnostic[]] { +): [HttpOperationBody | undefined, readonly Diagnostic[]] { contentTypes ??= []; const diagnostics: Diagnostic[] = []; if (resolvedBody === undefined) { @@ -179,14 +180,14 @@ function computeHttpOperationBody( return [undefined, diagnostics]; } - const body: HttpOperationRequestBody = { + const body: HttpOperationBody = { + bodyKind: "single", type: resolvedBody.type, isExplicit: resolvedBody.isExplicit, containsMetadataAnnotations: resolvedBody.containsMetadataAnnotations, contentTypes, + property: resolvedBody.property, + parameter: resolvedBody.property, }; - if (resolvedBody.property) { - body.parameter = resolvedBody.property; - } return [body, diagnostics]; } diff --git a/packages/http/src/responses.ts b/packages/http/src/responses.ts index 37d16402c8..522f27db70 100644 --- a/packages/http/src/responses.ts +++ b/packages/http/src/responses.ts @@ -14,7 +14,7 @@ import { Program, Type, } from "@typespec/compiler"; -import { resolveBody, ResolvedBody } from "./body.js"; +import { resolveBody } from "./body.js"; import { getContentTypes, isContentTypeHeader } from "./content-types.js"; import { getHeaderFieldName, @@ -25,7 +25,7 @@ import { } from "./decorators.js"; import { createDiagnostic, HttpStateKeys, reportDiagnostic } from "./lib.js"; import { gatherMetadata, Visibility } from "./metadata.js"; -import { HttpOperationResponse, HttpStatusCodes, HttpStatusCodesEntry } from "./types.js"; +import { HttpBody, HttpOperationResponse, HttpStatusCodes, HttpStatusCodesEntry } from "./types.js"; /** * Get the responses for a given operation. @@ -153,7 +153,8 @@ function processResponseType( if (resolvedBody !== undefined) { response.responses.push({ body: { - contentTypes: contentTypes, + bodyKind: "single", + contentTypes, ...resolvedBody, }, headers, @@ -255,7 +256,7 @@ function getResponseDescription( operation: Operation, responseType: Type, statusCode: HttpStatusCodes[number], - body: ResolvedBody | undefined + body: HttpBody | undefined ): string | undefined { // NOTE: If the response type is an envelope and not the same as the body // type, then use its @doc as the response description. However, if the diff --git a/packages/http/src/types.ts b/packages/http/src/types.ts index e7ab7deb5d..78606cd698 100644 --- a/packages/http/src/types.ts +++ b/packages/http/src/types.ts @@ -2,10 +2,12 @@ import { DiagnosticResult, Interface, ListOperationOptions, + Model, ModelProperty, Namespace, Operation, Program, + Tuple, Type, } from "@typespec/compiler"; @@ -317,28 +319,18 @@ export type HttpOperationParameter = ( }; /** - * Represent the body information for an http request. - * - * @note the `type` must be a `Model` if the content type is multipart. + * @deprecated use {@link HttpOperationBody} */ -export interface HttpOperationRequestBody extends HttpOperationBody { - /** - * If the body was explicitly set as a property. Correspond to the property with `@body` or `@bodyRoot` - */ - parameter?: ModelProperty; -} - -export interface HttpOperationResponseBody extends HttpOperationBody { - /** - * If the body was explicitly set as a property. Correspond to the property with `@body` or `@bodyRoot` - */ - readonly property?: ModelProperty; -} +export type HttpOperationRequestBody = HttpOperationBody; +/** + * @deprecated use {@link HttpOperationBody} + */ +export type HttpOperationResponseBody = HttpOperationBody; export interface HttpOperationParameters { parameters: HttpOperationParameter[]; - body?: HttpOperationRequestBody; + body?: HttpOperationBody | HttpOperationMultipartBody; /** @deprecated use {@link body.type} */ bodyType?: Type; @@ -446,25 +438,57 @@ export interface HttpOperationResponse { export interface HttpOperationResponseContent { headers?: Record; - body?: HttpOperationResponseBody; + body?: HttpOperationBody | HttpOperationMultipartBody; } -export interface HttpOperationBody { - /** - * Content types. - */ - contentTypes: string[]; +export interface HttpOperationBodyBase { + /** Content types. */ + readonly contentTypes: string[]; +} - /** - * Type of the operation body. - */ - type: Type; +export interface HttpBody { + readonly type: Type; /** If the body was explicitly set with `@body`. */ readonly isExplicit: boolean; /** If the body contains metadata annotations to ignore. For example `@header`. */ readonly containsMetadataAnnotations: boolean; + + /** + * @deprecated use {@link parameter} + */ + parameter?: ModelProperty; + + /** + * If the body was explicitly set as a property. Correspond to the property with `@body` or `@bodyRoot` + */ + readonly property?: ModelProperty; +} + +export interface HttpOperationBody extends HttpOperationBodyBase, HttpBody { + readonly bodyKind: "single"; +} + +/** Body marked with `@multipartBody` */ +export interface HttpOperationMultipartBody extends HttpOperationBodyBase { + readonly bodyKind: "multipart"; + readonly type: Model | Tuple; + /** Property annotated with `@multipartBody` */ + readonly property: ModelProperty; + readonly parts: HttpOperationPart[]; +} + +/** Represent an part in a multipart body. */ +export interface HttpOperationPart { + /** Part name */ + readonly name?: string; + /** Part body */ + readonly body: HttpOperationBody; + /** Part headers */ + readonly headers?: Record; + /** If there can be multiple of that part */ + readonly multi: boolean; } export interface HttpStatusCodeRange { diff --git a/packages/http/test/test-host.ts b/packages/http/test/test-host.ts index 0280b7588a..96268bcffc 100644 --- a/packages/http/test/test-host.ts +++ b/packages/http/test/test-host.ts @@ -70,7 +70,7 @@ export async function compileOperations( 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/openapi3/src/openapi.ts b/packages/openapi3/src/openapi.ts index 4cf5c04e4f..c288529127 100644 --- a/packages/openapi3/src/openapi.ts +++ b/packages/openapi3/src/openapi.ts @@ -60,6 +60,8 @@ import { HeaderFieldOptions, HttpAuth, HttpOperation, + HttpOperationBody, + HttpOperationMultipartBody, HttpOperationParameter, HttpOperationParameters, HttpOperationRequestBody, @@ -692,7 +694,7 @@ function createOAPIEmitter( operations: operations.map((op) => op.operation), parameters: { parameters: [], - }, + } as any, bodies: undefined, authentication: operations[0].authentication, responses: new Map(), @@ -1019,12 +1021,7 @@ function createOAPIEmitter( const isBinary = isBinaryPayload(data.body.type, contentType); const schema = isBinary ? { type: "string", format: "binary" } - : getSchemaForBody( - data.body.type, - Visibility.Read, - data.body.isExplicit && data.body.containsMetadataAnnotations, - undefined - ); + : getSchemaForBody(data.body, Visibility.Read, contentType); if (schemaMap.has(contentType)) { schemaMap.get(contentType)!.push(schema); } else { @@ -1128,6 +1125,28 @@ function createOAPIEmitter( } function getSchemaForBody( + body: HttpOperationBody | HttpOperationMultipartBody, + visibility: Visibility, + contentType: string + ): any { + const isBinary = isBinaryPayload(body.type, contentType); + if (isBinary) { + return { type: "string", format: "binary" }; + } + switch (body.bodyKind) { + case "single": + return getSchemaForSingleBody( + body.type, + visibility, + body.isExplicit && body.containsMetadataAnnotations, + contentType + ); + case "multipart": + // return getSchemaForMultipartBody(body, visibility); + } + } + + function getSchemaForSingleBody( type: Type, visibility: Visibility, ignoreMetadataAnnotations: boolean, @@ -1211,15 +1230,7 @@ function createOAPIEmitter( } const contentTypes = body.contentTypes.length > 0 ? body.contentTypes : ["application/json"]; for (const contentType of contentTypes) { - const isBinary = isBinaryPayload(body.type, contentType); - const bodySchema = isBinary - ? { type: "string", format: "binary" } - : getSchemaForBody( - body.type, - visibility, - body.isExplicit, - contentType.startsWith("multipart/") ? contentType : undefined - ); + const bodySchema = getSchemaForBody(body, visibility, contentType); if (schemaMap.has(contentType)) { schemaMap.get(contentType)!.push(bodySchema); } else { @@ -1241,14 +1252,28 @@ function createOAPIEmitter( currentEndpoint.requestBody = requestBody; } - function emitRequestBody(body: HttpOperationRequestBody | undefined, visibility: Visibility) { + function emitRequestBody( + body: HttpOperationBody | HttpOperationMultipartBody | undefined, + visibility: Visibility + ) { if (body === undefined || isVoidType(body.type)) { return; } + switch (body.bodyKind) { + case "single": + emitSingleRequestBody(body, visibility); + break; + case "multipart": + // emitMultipartRequestBody(body, visibility); + break; + } + } + + function emitSingleRequestBody(body: HttpOperationBody, visibility: Visibility) { const requestBody: any = { - description: body.parameter ? getDoc(program, body.parameter) : undefined, - required: body.parameter ? !body.parameter.optional : true, + description: body.property ? getDoc(program, body.property) : undefined, + required: body.property ? !body.property.optional : true, content: {}, }; @@ -1257,7 +1282,7 @@ function createOAPIEmitter( const isBinary = isBinaryPayload(body.type, contentType); const bodySchema = isBinary ? { type: "string", format: "binary" } - : getSchemaForBody( + : getSchemaForSingleBody( body.type, visibility, body.isExplicit && body.containsMetadataAnnotations, From c45a66023d2e401d58f65500fe810d27fa3cc752 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Mon, 13 May 2024 12:10:05 -0700 Subject: [PATCH 04/39] Progress --- packages/http/src/body.ts | 217 ++++++++++++++++++++++++++------ packages/http/src/decorators.ts | 2 +- packages/http/src/lib.ts | 6 + packages/http/src/metadata.ts | 3 +- packages/http/src/parameters.ts | 18 +-- packages/http/src/responses.ts | 21 +--- 6 files changed, 194 insertions(+), 73 deletions(-) diff --git a/packages/http/src/body.ts b/packages/http/src/body.ts index 6900359081..b57d599a23 100644 --- a/packages/http/src/body.ts +++ b/packages/http/src/body.ts @@ -1,7 +1,9 @@ import { Diagnostic, + Model, ModelProperty, Program, + Tuple, Type, createDiagnosticCollector, filterModelProperties, @@ -14,21 +16,52 @@ import { isBody, isBodyRoot, isHeader, + isMultipartBodyProperty, isPathParam, isQueryParam, isStatusCode, } from "./decorators.js"; import { createDiagnostic } from "./lib.js"; -import { Visibility, isVisible } from "./metadata.js"; -import { HttpBody } from "./types.js"; +import { Visibility, gatherMetadata, isVisible } from "./metadata.js"; +import { getHttpPart } from "./private.decorators.js"; +import { HttpBody, HttpOperationMultipartBody, HttpOperationPart } from "./types.js"; -export function resolveBody( +interface BodyAndMetadata { + body?: HttpBody; + metadata: Set; +} +export function extractBodyAndMetadata( + program: Program, + responseType: Type, + visibility: Visibility, + usedIn: "request" | "response" | "multipart" +): [BodyAndMetadata, readonly Diagnostic[]] { + const diagnostics = createDiagnosticCollector(); + + const rootPropertyMap = new Map(); + const metadata = gatherMetadata( + program, + diagnostics, + responseType, + visibility, + undefined, + rootPropertyMap + ); + + const body = diagnostics.pipe( + resolveBody(program, responseType, metadata, rootPropertyMap, visibility, usedIn) + ); + + return diagnostics.wrap({ body, metadata }); +} + +function resolveBody( program: Program, requestOrResponseType: Type, metadata: Set, rootPropertyMap: Map, visibility: Visibility, - usedIn: "request" | "response" + usedIn: "request" | "response" | "multipart" ): [HttpBody | undefined, readonly Diagnostic[]] { const diagnostics = createDiagnosticCollector(); // non-model or intrinsic/array model -> response body is response type @@ -41,40 +74,11 @@ export function resolveBody( }); } - const duplicateTracker = new DuplicateTracker(); - // look for explicit body - let resolvedBody: HttpBody | undefined; - for (const property of metadata) { - const isBodyVal = isBody(program, property); - const isBodyRootVal = isBodyRoot(program, property); - if (isBodyVal || isBodyRootVal) { - duplicateTracker.track("body", property); - let containsMetadataAnnotations = false; - if (isBodyVal) { - const valid = diagnostics.pipe(validateBodyProperty(program, property, usedIn)); - containsMetadataAnnotations = !valid; - } - if (resolvedBody === undefined) { - resolvedBody = { - type: property.type, - isExplicit: isBodyVal, - containsMetadataAnnotations, - property, - }; - } - } - } - for (const [_, items] of duplicateTracker.entries()) { - for (const prop of items) { - diagnostics.add( - createDiagnostic({ - code: "duplicate-body", - target: prop, - }) - ); - } - } + const resolvedBody: HttpBody | undefined = diagnostics.pipe( + resolveExplicitBodyProperty(program, metadata, usedIn) + ); + if (resolvedBody === undefined) { // Special case if the model as a parent model then we'll return an empty object as this is assumed to be a nominal type. // Special Case if the model has an indexer then it means it can return props so cannot be void. @@ -128,11 +132,58 @@ export function resolveBody( return diagnostics.wrap(resolvedBody); } +function resolveExplicitBodyProperty( + program: Program, + metadata: Set, + usedIn: "request" | "response" | "multipart" +): [HttpBody | undefined, readonly Diagnostic[]] { + const diagnostics = createDiagnosticCollector(); + let resolvedBody: HttpBody | undefined; + const duplicateTracker = new DuplicateTracker(); + for (const property of metadata) { + const isBodyVal = isBody(program, property); + const isBodyRootVal = isBodyRoot(program, property); + const isMultiPartBody = isMultipartBodyProperty(program, property); + + if (isBodyVal || isBodyRootVal || isMultiPartBody) { + duplicateTracker.track("body", property); + } + if (isBodyVal || isBodyRootVal) { + let containsMetadataAnnotations = false; + if (isBodyVal) { + const valid = diagnostics.pipe(validateBodyProperty(program, property, usedIn)); + containsMetadataAnnotations = !valid; + } + if (resolvedBody === undefined) { + resolvedBody = { + type: property.type, + isExplicit: isBodyVal, + containsMetadataAnnotations, + property, + }; + } + } else if (isMultiPartBody) { + } + } + for (const [_, items] of duplicateTracker.entries()) { + for (const prop of items) { + diagnostics.add( + createDiagnostic({ + code: "duplicate-body", + target: prop, + }) + ); + } + } + + return diagnostics.wrap(resolvedBody); +} + /** Validate a property marked with `@body` */ -export function validateBodyProperty( +function validateBodyProperty( program: Program, property: ModelProperty, - usedIn: "request" | "response" + usedIn: "request" | "response" | "multipart" ): [boolean, readonly Diagnostic[]] { const diagnostics = createDiagnosticCollector(); navigateType( @@ -141,7 +192,7 @@ export function validateBodyProperty( modelProperty: (prop) => { const kind = isHeader(program, prop) ? "header" - : usedIn === "request" && isQueryParam(program, prop) + : (usedIn === "request" || usedIn === "multipart") && isQueryParam(program, prop) ? "query" : usedIn === "request" && isPathParam(program, prop) ? "path" @@ -164,3 +215,89 @@ export function validateBodyProperty( ); return diagnostics.wrap(diagnostics.diagnostics.length === 0); } + +function resolveMultiPartBody( + program: Program, + property: ModelProperty, + visibility: Visibility +): [HttpOperationMultipartBody | undefined, readonly Diagnostic[]] { + const type = property.type; + if (type.kind === "Model") { + return resolveMultiPartBodyFromModel(program, property, type, visibility); + } else if (type.kind === "Tuple") { + return resolveMultiPartBodyFromTuple(program, property, type, visibility); + } else { + return [undefined, [createDiagnostic({ code: "multipart-model", target: property })]]; + } +} + +function resolveMultiPartBodyFromModel( + program: Program, + property: ModelProperty, + type: Model, + visibility: Visibility +): [HttpOperationMultipartBody | undefined, readonly Diagnostic[]] { + const diagnostics = createDiagnosticCollector(); + const parts: HttpOperationPart[] = []; + for (const item of type.properties.values()) { + const part = diagnostics.pipe(resolvePartOrParts(program, item.type, visibility)); + if (part) { + parts.push(part); + } + } + + return diagnostics.wrap({ bodyKind: "multipart", contentTypes: [], parts, property, type }); +} + +function resolveMultiPartBodyFromTuple( + program: Program, + property: ModelProperty, + type: Tuple, + visibility: Visibility +): [HttpOperationMultipartBody | undefined, readonly Diagnostic[]] { + const diagnostics = createDiagnosticCollector(); + const parts: HttpOperationPart[] = []; + for (const item of type.values) { + const part = diagnostics.pipe(resolvePartOrParts(program, item, visibility)); + if (part) { + parts.push(part); + } + } + + return diagnostics.wrap({ bodyKind: "multipart", contentTypes: [], parts, property, type }); +} + +function resolvePartOrParts( + program: Program, + type: Type, + visibility: Visibility +): [HttpOperationPart | undefined, readonly Diagnostic[]] { + if (type.kind === "Model" && isArrayModelType(program, type)) { + const [part, diagnostics] = resolvePart(program, type.indexer.value, visibility); + if (part) { + return [{ ...part, multi: true }, diagnostics]; + } + return [part, diagnostics]; + } else { + return resolvePart(program, type, visibility); + } +} + +function resolvePart( + program: Program, + type: Type, + visibility: Visibility +): [HttpOperationPart | undefined, readonly Diagnostic[]] { + const part = getHttpPart(program, type); + if (part) { + const [{ body, metadata }, diagnostics] = extractBodyAndMetadata( + program, + type, + visibility, + "multipart" + ); + + return [{ multi: false, name: part.options.name, body: null!, headers: {} }, diagnostics]; + } + return [undefined, [createDiagnostic({ code: "multipart-part", target: type })]]; +} diff --git a/packages/http/src/decorators.ts b/packages/http/src/decorators.ts index 3d300e4909..a8473ed52b 100644 --- a/packages/http/src/decorators.ts +++ b/packages/http/src/decorators.ts @@ -227,7 +227,7 @@ export const $multipartBody: MultipartBodyDecorator = ( context.program.stateSet(HttpStateKeys.multipartBody).add(entity); }; -export function isMultipartBody(program: Program, entity: Type): boolean { +export function isMultipartBodyProperty(program: Program, entity: Type): boolean { return program.stateSet(HttpStateKeys.multipartBody).has(entity); } diff --git a/packages/http/src/lib.ts b/packages/http/src/lib.ts index cb2fa43a8e..bb9c6f764a 100644 --- a/packages/http/src/lib.ts +++ b/packages/http/src/lib.ts @@ -123,6 +123,12 @@ export const $lib = createTypeSpecLibrary({ default: "Multipart request body must be a model.", }, }, + "multipart-part": { + severity: "error", + messages: { + default: "Expect item to be an HttpPart model.", + }, + }, "header-format-required": { severity: "error", messages: { diff --git a/packages/http/src/metadata.ts b/packages/http/src/metadata.ts index 5e2c0ad4c0..602e02f86c 100644 --- a/packages/http/src/metadata.ts +++ b/packages/http/src/metadata.ts @@ -8,12 +8,11 @@ import { ModelProperty, Operation, Program, - Queue, - TwoLevelMap, Type, Union, walkPropertiesInherited, } from "@typespec/compiler"; +import { Queue, TwoLevelMap } from "@typespec/compiler/utils"; import { includeInapplicableMetadataInPayload, isBody, diff --git a/packages/http/src/parameters.ts b/packages/http/src/parameters.ts index 4e9fc88019..9dbcf6644a 100644 --- a/packages/http/src/parameters.ts +++ b/packages/http/src/parameters.ts @@ -5,7 +5,7 @@ import { Operation, Program, } from "@typespec/compiler"; -import { resolveBody } from "./body.js"; +import { extractBodyAndMetadata } from "./body.js"; import { getContentTypes, isContentTypeHeader } from "./content-types.js"; import { getHeaderFieldOptions, @@ -16,7 +16,7 @@ import { isBodyRoot, } from "./decorators.js"; import { createDiagnostic } from "./lib.js"; -import { gatherMetadata, isMetadata, resolveRequestVisibility } from "./metadata.js"; +import { resolveRequestVisibility } from "./metadata.js"; import { HttpBody, HttpOperation, @@ -61,24 +61,14 @@ function getOperationParametersForVerb( ): [HttpOperationParameters, readonly Diagnostic[]] { const diagnostics = createDiagnosticCollector(); const visibility = resolveRequestVisibility(program, operation, verb); - const rootPropertyMap = new Map(); - const metadata = gatherMetadata( - program, - diagnostics, - operation.parameters, - visibility, - (_, param) => isMetadata(program, param) || isImplicitPathParam(param), - rootPropertyMap - ); - function isImplicitPathParam(param: ModelProperty) { const isTopLevel = param.model === operation.parameters; return isTopLevel && knownPathParamNames.includes(param.name); } const parameters: HttpOperationParameter[] = []; - const resolvedBody = diagnostics.pipe( - resolveBody(program, operation.parameters, metadata, rootPropertyMap, visibility, "request") + const { body: resolvedBody, metadata } = diagnostics.pipe( + extractBodyAndMetadata(program, operation.parameters, visibility, "request") ); let contentTypes: string[] | undefined; diff --git a/packages/http/src/responses.ts b/packages/http/src/responses.ts index 522f27db70..d3e2ce6028 100644 --- a/packages/http/src/responses.ts +++ b/packages/http/src/responses.ts @@ -14,7 +14,7 @@ import { Program, Type, } from "@typespec/compiler"; -import { resolveBody } from "./body.js"; +import { extractBodyAndMetadata } from "./body.js"; import { getContentTypes, isContentTypeHeader } from "./content-types.js"; import { getHeaderFieldName, @@ -24,7 +24,7 @@ import { isStatusCode, } from "./decorators.js"; import { createDiagnostic, HttpStateKeys, reportDiagnostic } from "./lib.js"; -import { gatherMetadata, Visibility } from "./metadata.js"; +import { Visibility } from "./metadata.js"; import { HttpBody, HttpOperationResponse, HttpStatusCodes, HttpStatusCodesEntry } from "./types.js"; /** @@ -86,16 +86,10 @@ function processResponseType( responses: ResponseIndex, responseType: Type ) { - const rootPropertyMap = new Map(); - const metadata = gatherMetadata( - program, - diagnostics, - responseType, - Visibility.Read, - undefined, - rootPropertyMap + // Get body + let { body: resolvedBody, metadata } = diagnostics.pipe( + extractBodyAndMetadata(program, responseType, Visibility.Read, "response") ); - // Get explicity defined status codes const statusCodes: HttpStatusCodes = diagnostics.pipe( getResponseStatusCodes(program, responseType, metadata) @@ -107,11 +101,6 @@ function processResponseType( // Get response headers const headers = getResponseHeaders(program, metadata); - // Get body - let resolvedBody = diagnostics.pipe( - resolveBody(program, responseType, metadata, rootPropertyMap, Visibility.Read, "response") - ); - // If there is no explicit status code, check if it should be 204 if (statusCodes.length === 0) { if (isErrorModel(program, responseType)) { From 510f97ed468a03f1f26ebbc51f8006fa80a8d573 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Mon, 13 May 2024 13:22:37 -0700 Subject: [PATCH 05/39] progress --- packages/http/src/body.ts | 94 +++++++++++++++++++++++----- packages/http/src/lib.ts | 6 ++ packages/http/src/parameters.ts | 50 +-------------- packages/http/src/responses.ts | 53 +++------------- packages/http/test/responses.test.ts | 3 - 5 files changed, 95 insertions(+), 111 deletions(-) diff --git a/packages/http/src/body.ts b/packages/http/src/body.ts index b57d599a23..1f7af8f8ad 100644 --- a/packages/http/src/body.ts +++ b/packages/http/src/body.ts @@ -12,6 +12,7 @@ import { navigateType, } from "@typespec/compiler"; import { DuplicateTracker } from "@typespec/compiler/utils"; +import { getContentTypes, isContentTypeHeader } from "./content-types.js"; import { isBody, isBodyRoot, @@ -24,15 +25,15 @@ import { import { createDiagnostic } from "./lib.js"; import { Visibility, gatherMetadata, isVisible } from "./metadata.js"; import { getHttpPart } from "./private.decorators.js"; -import { HttpBody, HttpOperationMultipartBody, HttpOperationPart } from "./types.js"; +import { HttpOperationBody, HttpOperationMultipartBody, HttpOperationPart } from "./types.js"; interface BodyAndMetadata { - body?: HttpBody; + body?: HttpOperationBody | HttpOperationMultipartBody; metadata: Set; } export function extractBodyAndMetadata( program: Program, - responseType: Type, + type: Type, visibility: Visibility, usedIn: "request" | "response" | "multipart" ): [BodyAndMetadata, readonly Diagnostic[]] { @@ -42,16 +43,32 @@ export function extractBodyAndMetadata( const metadata = gatherMetadata( program, diagnostics, - responseType, + type, visibility, undefined, rootPropertyMap ); const body = diagnostics.pipe( - resolveBody(program, responseType, metadata, rootPropertyMap, visibility, usedIn) + resolveBody(program, type, metadata, rootPropertyMap, visibility, usedIn) ); + if (body) { + if ( + body.contentTypes.includes("multipart/form-data") && + body.bodyKind === "single" && + body.type.kind !== "Model" + ) { + diagnostics.add( + createDiagnostic({ + code: "multipart-model", + target: body.property ?? type, + }) + ); + return diagnostics.wrap({ body: undefined, metadata }); + } + } + return diagnostics.wrap({ body, metadata }); } @@ -62,12 +79,16 @@ function resolveBody( rootPropertyMap: Map, visibility: Visibility, usedIn: "request" | "response" | "multipart" -): [HttpBody | undefined, readonly Diagnostic[]] { +): [HttpOperationBody | HttpOperationMultipartBody | undefined, readonly Diagnostic[]] { const diagnostics = createDiagnosticCollector(); + const { contentTypes, contentTypeProperty } = diagnostics.pipe( + resolveContentTypes(program, metadata) + ); // non-model or intrinsic/array model -> response body is response type if (requestOrResponseType.kind !== "Model" || isArrayModelType(program, requestOrResponseType)) { return diagnostics.wrap({ bodyKind: "single", + contentTypes, type: requestOrResponseType, isExplicit: false, containsMetadataAnnotations: false, @@ -75,8 +96,8 @@ function resolveBody( } // look for explicit body - const resolvedBody: HttpBody | undefined = diagnostics.pipe( - resolveExplicitBodyProperty(program, metadata, usedIn) + const resolvedBody: HttpOperationBody | HttpOperationMultipartBody | undefined = diagnostics.pipe( + resolveExplicitBodyProperty(program, metadata, contentTypes, visibility, usedIn) ); if (resolvedBody === undefined) { @@ -84,6 +105,8 @@ function resolveBody( // Special Case if the model has an indexer then it means it can return props so cannot be void. if (requestOrResponseType.baseModel || requestOrResponseType.indexer) { return diagnostics.wrap({ + bodyKind: "single", + contentTypes, type: requestOrResponseType, isExplicit: false, containsMetadataAnnotations: false, @@ -96,6 +119,8 @@ function resolveBody( getDiscriminator(program, requestOrResponseType) ) { return diagnostics.wrap({ + bodyKind: "single", + contentTypes, type: requestOrResponseType, isExplicit: false, containsMetadataAnnotations: false, @@ -114,6 +139,8 @@ function resolveBody( if (unannotatedProperties.properties.size > 0) { if (resolvedBody === undefined) { return diagnostics.wrap({ + bodyKind: "single", + contentTypes, type: unannotatedProperties, isExplicit: false, containsMetadataAnnotations: false, @@ -128,18 +155,40 @@ function resolveBody( ); } } - + if (resolvedBody === undefined && contentTypeProperty) { + diagnostics.add( + createDiagnostic({ + code: "content-type-ignored", + target: contentTypeProperty, + }) + ); + } return diagnostics.wrap(resolvedBody); } +function resolveContentTypes( + program: Program, + metadata: Set +): [{ contentTypes: string[]; contentTypeProperty?: ModelProperty }, readonly Diagnostic[]] { + for (const prop of metadata) { + if (isContentTypeHeader(program, prop)) { + const [contentTypes, diagnostics] = getContentTypes(prop); + return [{ contentTypes, contentTypeProperty: prop }, diagnostics]; + } + } + return [{ contentTypes: ["application/json"] }, []]; +} function resolveExplicitBodyProperty( program: Program, metadata: Set, + contentTypes: string[], + visibility: Visibility, usedIn: "request" | "response" | "multipart" -): [HttpBody | undefined, readonly Diagnostic[]] { +): [HttpOperationBody | HttpOperationMultipartBody | undefined, readonly Diagnostic[]] { const diagnostics = createDiagnosticCollector(); - let resolvedBody: HttpBody | undefined; + let resolvedBody: HttpOperationBody | HttpOperationMultipartBody | undefined; const duplicateTracker = new DuplicateTracker(); + for (const property of metadata) { const isBodyVal = isBody(program, property); const isBodyRootVal = isBodyRoot(program, property); @@ -156,6 +205,8 @@ function resolveExplicitBodyProperty( } if (resolvedBody === undefined) { resolvedBody = { + bodyKind: "single", + contentTypes, type: property.type, isExplicit: isBodyVal, containsMetadataAnnotations, @@ -163,6 +214,9 @@ function resolveExplicitBodyProperty( }; } } else if (isMultiPartBody) { + resolvedBody = diagnostics.pipe( + resolveMultiPartBody(program, property, contentTypes, visibility) + ); } } for (const [_, items] of duplicateTracker.entries()) { @@ -219,13 +273,14 @@ function validateBodyProperty( function resolveMultiPartBody( program: Program, property: ModelProperty, + contentTypes: string[], visibility: Visibility ): [HttpOperationMultipartBody | undefined, readonly Diagnostic[]] { const type = property.type; if (type.kind === "Model") { - return resolveMultiPartBodyFromModel(program, property, type, visibility); + return resolveMultiPartBodyFromModel(program, property, type, contentTypes, visibility); } else if (type.kind === "Tuple") { - return resolveMultiPartBodyFromTuple(program, property, type, visibility); + return resolveMultiPartBodyFromTuple(program, property, type, contentTypes, visibility); } else { return [undefined, [createDiagnostic({ code: "multipart-model", target: property })]]; } @@ -235,6 +290,7 @@ function resolveMultiPartBodyFromModel( program: Program, property: ModelProperty, type: Model, + contentTypes: string[], visibility: Visibility ): [HttpOperationMultipartBody | undefined, readonly Diagnostic[]] { const diagnostics = createDiagnosticCollector(); @@ -246,13 +302,14 @@ function resolveMultiPartBodyFromModel( } } - return diagnostics.wrap({ bodyKind: "multipart", contentTypes: [], parts, property, type }); + return diagnostics.wrap({ bodyKind: "multipart", contentTypes, parts, property, type }); } function resolveMultiPartBodyFromTuple( program: Program, property: ModelProperty, type: Tuple, + contentTypes: string[], visibility: Visibility ): [HttpOperationMultipartBody | undefined, readonly Diagnostic[]] { const diagnostics = createDiagnosticCollector(); @@ -264,7 +321,7 @@ function resolveMultiPartBodyFromTuple( } } - return diagnostics.wrap({ bodyKind: "multipart", contentTypes: [], parts, property, type }); + return diagnostics.wrap({ bodyKind: "multipart", contentTypes, parts, property, type }); } function resolvePartOrParts( @@ -296,8 +353,13 @@ function resolvePart( visibility, "multipart" ); + if (body === undefined) { + return [undefined, diagnostics]; + } else if (body.bodyKind === "multipart") { + return [undefined, [createDiagnostic({ code: "multipart-nested", target: type })]]; + } - return [{ multi: false, name: part.options.name, body: null!, headers: {} }, diagnostics]; + return [{ multi: false, name: part.options.name, body, headers: {} }, diagnostics]; } return [undefined, [createDiagnostic({ code: "multipart-part", target: type })]]; } diff --git a/packages/http/src/lib.ts b/packages/http/src/lib.ts index bb9c6f764a..678fa0ff08 100644 --- a/packages/http/src/lib.ts +++ b/packages/http/src/lib.ts @@ -129,6 +129,12 @@ export const $lib = createTypeSpecLibrary({ default: "Expect item to be an HttpPart model.", }, }, + "multipart-nested": { + severity: "error", + messages: { + default: "Cannot use @multipartBody inside of an HttpPart", + }, + }, "header-format-required": { severity: "error", messages: { diff --git a/packages/http/src/parameters.ts b/packages/http/src/parameters.ts index 9dbcf6644a..a51fefdf59 100644 --- a/packages/http/src/parameters.ts +++ b/packages/http/src/parameters.ts @@ -6,7 +6,6 @@ import { Program, } from "@typespec/compiler"; import { extractBodyAndMetadata } from "./body.js"; -import { getContentTypes, isContentTypeHeader } from "./content-types.js"; import { getHeaderFieldOptions, getOperationVerb, @@ -18,9 +17,7 @@ import { import { createDiagnostic } from "./lib.js"; import { resolveRequestVisibility } from "./metadata.js"; import { - HttpBody, HttpOperation, - HttpOperationBody, HttpOperationParameter, HttpOperationParameters, HttpVerb, @@ -70,7 +67,6 @@ function getOperationParametersForVerb( const { body: resolvedBody, metadata } = diagnostics.pipe( extractBodyAndMetadata(program, operation.parameters, visibility, "request") ); - let contentTypes: string[] | undefined; for (const param of metadata) { const queryOptions = getQueryParamOptions(program, param); @@ -116,9 +112,6 @@ function getOperationParametersForVerb( param, }); } else if (headerOptions) { - if (isContentTypeHeader(program, param)) { - contentTypes = diagnostics.pipe(getContentTypes(param)); - } parameters.push({ ...headerOptions, param, @@ -126,7 +119,7 @@ function getOperationParametersForVerb( } } - const body = diagnostics.pipe(computeHttpOperationBody(operation, resolvedBody, contentTypes)); + const body = resolvedBody; return diagnostics.wrap({ parameters, @@ -140,44 +133,3 @@ function getOperationParametersForVerb( }, }); } - -function computeHttpOperationBody( - operation: Operation, - resolvedBody: HttpBody | undefined, - contentTypes: string[] | undefined -): [HttpOperationBody | undefined, readonly Diagnostic[]] { - contentTypes ??= []; - const diagnostics: Diagnostic[] = []; - if (resolvedBody === undefined) { - if (contentTypes.length > 0) { - diagnostics.push( - createDiagnostic({ - code: "content-type-ignored", - target: operation.parameters, - }) - ); - } - return [undefined, diagnostics]; - } - - if (contentTypes.includes("multipart/form-data") && resolvedBody.type.kind !== "Model") { - diagnostics.push( - createDiagnostic({ - code: "multipart-model", - target: resolvedBody.property ?? operation.parameters, - }) - ); - return [undefined, diagnostics]; - } - - const body: HttpOperationBody = { - bodyKind: "single", - type: resolvedBody.type, - isExplicit: resolvedBody.isExplicit, - containsMetadataAnnotations: resolvedBody.containsMetadataAnnotations, - contentTypes, - property: resolvedBody.property, - parameter: resolvedBody.property, - }; - return [body, diagnostics]; -} diff --git a/packages/http/src/responses.ts b/packages/http/src/responses.ts index d3e2ce6028..09f51481eb 100644 --- a/packages/http/src/responses.ts +++ b/packages/http/src/responses.ts @@ -15,7 +15,6 @@ import { Type, } from "@typespec/compiler"; import { extractBodyAndMetadata } from "./body.js"; -import { getContentTypes, isContentTypeHeader } from "./content-types.js"; import { getHeaderFieldName, getStatusCodeDescription, @@ -23,9 +22,15 @@ import { isHeader, isStatusCode, } from "./decorators.js"; -import { createDiagnostic, HttpStateKeys, reportDiagnostic } from "./lib.js"; +import { HttpStateKeys, reportDiagnostic } from "./lib.js"; import { Visibility } from "./metadata.js"; -import { HttpBody, HttpOperationResponse, HttpStatusCodes, HttpStatusCodesEntry } from "./types.js"; +import { + HttpOperationBody, + HttpOperationMultipartBody, + HttpOperationResponse, + HttpStatusCodes, + HttpStatusCodesEntry, +} from "./types.js"; /** * Get the responses for a given operation. @@ -95,9 +100,6 @@ function processResponseType( getResponseStatusCodes(program, responseType, metadata) ); - // Get explicitly defined content types - const contentTypes = getResponseContentTypes(program, diagnostics, metadata); - // Get response headers const headers = getResponseHeaders(program, metadata); @@ -116,11 +118,6 @@ function processResponseType( } } - // If there is a body but no explicit content types, use application/json - if (resolvedBody && contentTypes.length === 0) { - contentTypes.push("application/json"); - } - // Put them into currentEndpoint.responses for (const statusCode of statusCodes) { // the first model for this statusCode/content type pair carries the @@ -141,20 +138,9 @@ function processResponseType( if (resolvedBody !== undefined) { response.responses.push({ - body: { - bodyKind: "single", - contentTypes, - ...resolvedBody, - }, + body: resolvedBody, headers, }); - } else if (contentTypes.length > 0) { - diagnostics.add( - createDiagnostic({ - code: "content-type-ignored", - target: responseType, - }) - ); } else { response.responses.push({ headers }); } @@ -204,25 +190,6 @@ function getExplicitSetStatusCode(program: Program, entity: Model | ModelPropert return program.stateMap(HttpStateKeys.statusCode).get(entity) ?? []; } -/** - * Get explicity defined content-types from response metadata - * Return is an array of strings, possibly empty, which indicates no explicitly defined content-type. - * We do not check for duplicates here -- that will be done by the caller. - */ -function getResponseContentTypes( - program: Program, - diagnostics: DiagnosticCollector, - metadata: Set -): string[] { - const contentTypes: string[] = []; - for (const prop of metadata) { - if (isHeader(program, prop) && isContentTypeHeader(program, prop)) { - contentTypes.push(...diagnostics.pipe(getContentTypes(prop))); - } - } - return contentTypes; -} - /** * Get response headers from response metadata */ @@ -245,7 +212,7 @@ function getResponseDescription( operation: Operation, responseType: Type, statusCode: HttpStatusCodes[number], - body: HttpBody | undefined + body: HttpOperationBody | HttpOperationMultipartBody | undefined ): string | undefined { // NOTE: If the response type is an envelope and not the same as the body // type, then use its @doc as the response description. However, if the diff --git a/packages/http/test/responses.test.ts b/packages/http/test/responses.test.ts index bd830ae776..82bc9bab35 100644 --- a/packages/http/test/responses.test.ts +++ b/packages/http/test/responses.test.ts @@ -106,15 +106,12 @@ it("supports any casing for string literal 'Content-Type' header properties.", a model Foo {} @route("/test1") - @get op test1(): { @header "content-Type": "text/html", @body body: Foo }; @route("/test2") - @get op test2(): { @header "CONTENT-type": "text/plain", @body body: Foo }; @route("/test3") - @get op test3(): { @header "content-type": "application/json", @body body: Foo }; ` ); From 6645dd2c06e1e62e31e48a63f4ddfa99eee19365 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Mon, 13 May 2024 13:45:40 -0700 Subject: [PATCH 06/39] Fix --- packages/http/src/body.ts | 14 ++++++++++---- packages/http/src/parameters.ts | 4 +++- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/packages/http/src/body.ts b/packages/http/src/body.ts index 1f7af8f8ad..57f8ed298f 100644 --- a/packages/http/src/body.ts +++ b/packages/http/src/body.ts @@ -23,19 +23,23 @@ import { isStatusCode, } from "./decorators.js"; import { createDiagnostic } from "./lib.js"; -import { Visibility, gatherMetadata, isVisible } from "./metadata.js"; +import { Visibility, gatherMetadata, isMetadata, isVisible } from "./metadata.js"; import { getHttpPart } from "./private.decorators.js"; import { HttpOperationBody, HttpOperationMultipartBody, HttpOperationPart } from "./types.js"; -interface BodyAndMetadata { +export interface BodyAndMetadata { body?: HttpOperationBody | HttpOperationMultipartBody; metadata: Set; } +export interface ExtractBodyAndMetadataOptions { + isImplicitPathParam?: (param: ModelProperty) => boolean; +} export function extractBodyAndMetadata( program: Program, type: Type, visibility: Visibility, - usedIn: "request" | "response" | "multipart" + usedIn: "request" | "response" | "multipart", + options: ExtractBodyAndMetadataOptions = {} ): [BodyAndMetadata, readonly Diagnostic[]] { const diagnostics = createDiagnosticCollector(); @@ -45,7 +49,9 @@ export function extractBodyAndMetadata( diagnostics, type, visibility, - undefined, + (_, param) => + isMetadata(program, param) || + Boolean(options.isImplicitPathParam && options.isImplicitPathParam(param)), rootPropertyMap ); diff --git a/packages/http/src/parameters.ts b/packages/http/src/parameters.ts index a51fefdf59..389ec194c9 100644 --- a/packages/http/src/parameters.ts +++ b/packages/http/src/parameters.ts @@ -65,7 +65,9 @@ function getOperationParametersForVerb( const parameters: HttpOperationParameter[] = []; const { body: resolvedBody, metadata } = diagnostics.pipe( - extractBodyAndMetadata(program, operation.parameters, visibility, "request") + extractBodyAndMetadata(program, operation.parameters, visibility, "request", { + isImplicitPathParam, + }) ); for (const param of metadata) { From 340c328a9bd9dfb8dc770fafbc4d53fda2b50023 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Mon, 13 May 2024 14:29:18 -0700 Subject: [PATCH 07/39] basic support in openapi3 --- packages/http/src/body.ts | 3 ++- packages/http/src/metadata.ts | 8 ++++++- packages/openapi3/src/openapi.ts | 36 ++++++++++++++++---------------- 3 files changed, 27 insertions(+), 20 deletions(-) diff --git a/packages/http/src/body.ts b/packages/http/src/body.ts index 57f8ed298f..09b68943ac 100644 --- a/packages/http/src/body.ts +++ b/packages/http/src/body.ts @@ -184,6 +184,7 @@ function resolveContentTypes( } return [{ contentTypes: ["application/json"] }, []]; } + function resolveExplicitBodyProperty( program: Program, metadata: Set, @@ -355,7 +356,7 @@ function resolvePart( if (part) { const [{ body, metadata }, diagnostics] = extractBodyAndMetadata( program, - type, + part.type, visibility, "multipart" ); diff --git a/packages/http/src/metadata.ts b/packages/http/src/metadata.ts index 602e02f86c..5c3dc98c2a 100644 --- a/packages/http/src/metadata.ts +++ b/packages/http/src/metadata.ts @@ -19,6 +19,7 @@ import { isBodyIgnore, isBodyRoot, isHeader, + isMultipartBodyProperty, isPathParam, isQueryParam, isStatusCode, @@ -347,7 +348,12 @@ function isApplicableMetadataCore( return false; // no metadata is applicable to collection items } - if (treatBodyAsMetadata && (isBody(program, property) || isBodyRoot(program, property))) { + if ( + treatBodyAsMetadata && + (isBody(program, property) || + isBodyRoot(program, property) || + isMultipartBodyProperty(program, property)) + ) { return true; } diff --git a/packages/openapi3/src/openapi.ts b/packages/openapi3/src/openapi.ts index c288529127..c6457eff6e 100644 --- a/packages/openapi3/src/openapi.ts +++ b/packages/openapi3/src/openapi.ts @@ -1133,6 +1133,7 @@ function createOAPIEmitter( if (isBinary) { return { type: "string", format: "binary" }; } + switch (body.bodyKind) { case "single": return getSchemaForSingleBody( @@ -1142,7 +1143,7 @@ function createOAPIEmitter( contentType ); case "multipart": - // return getSchemaForMultipartBody(body, visibility); + return getSchemaForMultipartBody(body, visibility); } } @@ -1161,6 +1162,21 @@ function createOAPIEmitter( ); } + function getSchemaForMultipartBody( + body: HttpOperationMultipartBody, + visibility: Visibility + ): OpenAPI3Schema { + const properties: Record = {}; + for (const [partIndex, part] of body.parts.entries()) { + const schema = getSchemaForBody(part.body, visibility, "application/json"); + properties[part.name ?? partIndex.toString()] = schema; + } + return { + type: "object", + properties, + }; + } + function getParamPlaceholder(property: ModelProperty) { let spreadParam = false; @@ -1260,17 +1276,6 @@ function createOAPIEmitter( return; } - switch (body.bodyKind) { - case "single": - emitSingleRequestBody(body, visibility); - break; - case "multipart": - // emitMultipartRequestBody(body, visibility); - break; - } - } - - function emitSingleRequestBody(body: HttpOperationBody, visibility: Visibility) { const requestBody: any = { description: body.property ? getDoc(program, body.property) : undefined, required: body.property ? !body.property.optional : true, @@ -1282,12 +1287,7 @@ function createOAPIEmitter( const isBinary = isBinaryPayload(body.type, contentType); const bodySchema = isBinary ? { type: "string", format: "binary" } - : getSchemaForSingleBody( - body.type, - visibility, - body.isExplicit && body.containsMetadataAnnotations, - contentType.startsWith("multipart/") ? contentType : undefined - ); + : getSchemaForBody(body, visibility, contentType); const contentEntry: any = { schema: bodySchema, }; From 20d963c5318a28b29761fb5f397ad261dd938bd6 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Mon, 13 May 2024 15:17:37 -0700 Subject: [PATCH 08/39] Boilerplate for encoding --- packages/openapi3/src/openapi.ts | 87 +++++++++++++++++++------------- 1 file changed, 53 insertions(+), 34 deletions(-) diff --git a/packages/openapi3/src/openapi.ts b/packages/openapi3/src/openapi.ts index c6457eff6e..3e5e51b576 100644 --- a/packages/openapi3/src/openapi.ts +++ b/packages/openapi3/src/openapi.ts @@ -64,6 +64,7 @@ import { HttpOperationMultipartBody, HttpOperationParameter, HttpOperationParameters, + HttpOperationPart, HttpOperationRequestBody, HttpOperationResponse, HttpOperationResponseContent, @@ -98,7 +99,9 @@ import { createDiagnostic, FileType, OpenAPI3EmitterOptions } from "./lib.js"; import { getDefaultValue, OpenAPI3SchemaEmitter } from "./schema-emitter.js"; import { OpenAPI3Document, + OpenAPI3Encoding, OpenAPI3Header, + OpenAPI3MediaType, OpenAPI3OAuthFlows, OpenAPI3Operation, OpenAPI3Parameter, @@ -1018,10 +1021,7 @@ function createOAPIEmitter( } obj.content ??= {}; for (const contentType of data.body.contentTypes) { - const isBinary = isBinaryPayload(data.body.type, contentType); - const schema = isBinary - ? { type: "string", format: "binary" } - : getSchemaForBody(data.body, Visibility.Read, contentType); + const { schema } = getBodyContentEntry(data.body, Visibility.Read, contentType); if (schemaMap.has(contentType)) { schemaMap.get(contentType)!.push(schema); } else { @@ -1124,26 +1124,28 @@ function createOAPIEmitter( }) as any; } - function getSchemaForBody( + function getBodyContentEntry( body: HttpOperationBody | HttpOperationMultipartBody, visibility: Visibility, contentType: string - ): any { + ): OpenAPI3MediaType { const isBinary = isBinaryPayload(body.type, contentType); if (isBinary) { - return { type: "string", format: "binary" }; + return { schema: { type: "string", format: "binary" } }; } switch (body.bodyKind) { case "single": - return getSchemaForSingleBody( - body.type, - visibility, - body.isExplicit && body.containsMetadataAnnotations, - contentType - ); + return { + schema: getSchemaForSingleBody( + body.type, + visibility, + body.isExplicit && body.containsMetadataAnnotations, + contentType + ), + }; case "multipart": - return getSchemaForMultipartBody(body, visibility); + return getBodyContentForMultipartBody(body, visibility); } } @@ -1162,19 +1164,46 @@ function createOAPIEmitter( ); } - function getSchemaForMultipartBody( + function getBodyContentForMultipartBody( body: HttpOperationMultipartBody, visibility: Visibility - ): OpenAPI3Schema { + ): OpenAPI3MediaType { const properties: Record = {}; + const encodings: Record = {}; for (const [partIndex, part] of body.parts.entries()) { - const schema = getSchemaForBody(part.body, visibility, "application/json"); + const partName = part.name ?? `part${partIndex}`; + const schema = getSchemaForSingleBody( + part.body.type, + visibility, + part.body.isExplicit && part.body.containsMetadataAnnotations, + "application/json" + ); properties[part.name ?? partIndex.toString()] = schema; + + const encoding = resolveEncodingForMultipartPart(part); + if (encoding) { + encodings[partName] = encoding; + } } - return { - type: "object", - properties, + const result: OpenAPI3MediaType = { + schema: { + type: "object", + properties, + }, }; + + if (Object.keys(encodings).length === 0) { + result.encoding = encodings; + } + return result; + } + + function resolveEncodingForMultipartPart(part: HttpOperationPart): OpenAPI3Encoding { + const encoding: OpenAPI3Encoding = {}; + if (part.body.contentTypes) { + encoding.contentType = part.body.contentTypes.join(", "); + } + return encoding; } function getParamPlaceholder(property: ModelProperty) { @@ -1225,10 +1254,7 @@ function createOAPIEmitter( } } - function emitMergedRequestBody( - bodies: HttpOperationRequestBody[] | undefined, - visibility: Visibility - ) { + function emitMergedRequestBody(bodies: HttpOperationBody[] | undefined, visibility: Visibility) { if (bodies === undefined) { return; } @@ -1238,7 +1264,7 @@ function createOAPIEmitter( }; const schemaMap = new Map(); for (const body of bodies) { - const desc = body.parameter ? getDoc(program, body.parameter) : undefined; + const desc = body.property ? getDoc(program, body.property) : undefined; if (desc) { requestBody.description = requestBody.description ? `${requestBody.description} ${desc}` @@ -1246,7 +1272,7 @@ function createOAPIEmitter( } const contentTypes = body.contentTypes.length > 0 ? body.contentTypes : ["application/json"]; for (const contentType of contentTypes) { - const bodySchema = getSchemaForBody(body, visibility, contentType); + const { schema: bodySchema } = getBodyContentEntry(body, visibility, contentType); if (schemaMap.has(contentType)) { schemaMap.get(contentType)!.push(bodySchema); } else { @@ -1284,14 +1310,7 @@ function createOAPIEmitter( const contentTypes = body.contentTypes.length > 0 ? body.contentTypes : ["application/json"]; for (const contentType of contentTypes) { - const isBinary = isBinaryPayload(body.type, contentType); - const bodySchema = isBinary - ? { type: "string", format: "binary" } - : getSchemaForBody(body, visibility, contentType); - const contentEntry: any = { - schema: bodySchema, - }; - requestBody.content[contentType] = contentEntry; + requestBody.content[contentType] = getBodyContentEntry(body, visibility, contentType); } currentEndpoint.requestBody = requestBody; From fc1f1cb5f7e23092c8ff45944e6e35aaa81d1b9d Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 14 May 2024 08:41:51 -0700 Subject: [PATCH 09/39] Fix --- packages/openapi3/src/openapi.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/openapi3/src/openapi.ts b/packages/openapi3/src/openapi.ts index 3e5e51b576..93803bc0e9 100644 --- a/packages/openapi3/src/openapi.ts +++ b/packages/openapi3/src/openapi.ts @@ -1192,7 +1192,7 @@ function createOAPIEmitter( }, }; - if (Object.keys(encodings).length === 0) { + if (Object.keys(encodings).length > 0) { result.encoding = encodings; } return result; From bef66844f6d1d0ad582e2aa14b05918fbdab858f Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 14 May 2024 09:39:21 -0700 Subject: [PATCH 10/39] Refactor better resolution of property --- packages/http/src/body.ts | 109 +++++++++++------------ packages/http/src/http-property.ts | 135 +++++++++++++++++++++++++++++ packages/http/src/parameters.ts | 68 +++------------ packages/http/src/responses.ts | 23 ++--- 4 files changed, 207 insertions(+), 128 deletions(-) create mode 100644 packages/http/src/http-property.ts diff --git a/packages/http/src/body.ts b/packages/http/src/body.ts index 09b68943ac..e8c41e1637 100644 --- a/packages/http/src/body.ts +++ b/packages/http/src/body.ts @@ -13,15 +13,8 @@ import { } from "@typespec/compiler"; import { DuplicateTracker } from "@typespec/compiler/utils"; import { getContentTypes, isContentTypeHeader } from "./content-types.js"; -import { - isBody, - isBodyRoot, - isHeader, - isMultipartBodyProperty, - isPathParam, - isQueryParam, - isStatusCode, -} from "./decorators.js"; +import { isHeader, isPathParam, isQueryParam, isStatusCode } from "./decorators.js"; +import { GetHttpPropertyOptions, HttpProperty, getHttpProperty } from "./http-property.js"; import { createDiagnostic } from "./lib.js"; import { Visibility, gatherMetadata, isMetadata, isVisible } from "./metadata.js"; import { getHttpPart } from "./private.decorators.js"; @@ -29,11 +22,9 @@ import { HttpOperationBody, HttpOperationMultipartBody, HttpOperationPart } from export interface BodyAndMetadata { body?: HttpOperationBody | HttpOperationMultipartBody; - metadata: Set; -} -export interface ExtractBodyAndMetadataOptions { - isImplicitPathParam?: (param: ModelProperty) => boolean; + metadata: HttpProperty[]; } +export interface ExtractBodyAndMetadataOptions extends GetHttpPropertyOptions {} export function extractBodyAndMetadata( program: Program, type: Type, @@ -44,16 +35,18 @@ export function extractBodyAndMetadata( const diagnostics = createDiagnosticCollector(); const rootPropertyMap = new Map(); - const metadata = gatherMetadata( - program, - diagnostics, - type, - visibility, - (_, param) => - isMetadata(program, param) || - Boolean(options.isImplicitPathParam && options.isImplicitPathParam(param)), - rootPropertyMap - ); + const metadata = [ + ...gatherMetadata( + program, + diagnostics, + type, + visibility, + (_, param) => + isMetadata(program, param) || + Boolean(options.isImplicitPathParam && options.isImplicitPathParam(param)), + rootPropertyMap + ), + ].map((x) => diagnostics.pipe(getHttpProperty(program, x, options))); const body = diagnostics.pipe( resolveBody(program, type, metadata, rootPropertyMap, visibility, usedIn) @@ -81,7 +74,7 @@ export function extractBodyAndMetadata( function resolveBody( program: Program, requestOrResponseType: Type, - metadata: Set, + metadata: HttpProperty[], rootPropertyMap: Map, visibility: Visibility, usedIn: "request" | "response" | "multipart" @@ -139,7 +132,8 @@ function resolveBody( const unannotatedProperties = filterModelProperties( program, requestOrResponseType, - (p) => !metadata.has(p) && p !== bodyRoot && isVisible(program, p, visibility) + (p) => + !metadata.some((x) => x.property === p) && p !== bodyRoot && isVisible(program, p, visibility) ); if (unannotatedProperties.properties.size > 0) { @@ -174,12 +168,12 @@ function resolveBody( function resolveContentTypes( program: Program, - metadata: Set + metadata: HttpProperty[] ): [{ contentTypes: string[]; contentTypeProperty?: ModelProperty }, readonly Diagnostic[]] { for (const prop of metadata) { - if (isContentTypeHeader(program, prop)) { - const [contentTypes, diagnostics] = getContentTypes(prop); - return [{ contentTypes, contentTypeProperty: prop }, diagnostics]; + if (prop.kind === "header" && isContentTypeHeader(program, prop.property)) { + const [contentTypes, diagnostics] = getContentTypes(prop.property); + return [{ contentTypes, contentTypeProperty: prop.property }, diagnostics]; } } return [{ contentTypes: ["application/json"] }, []]; @@ -187,7 +181,7 @@ function resolveContentTypes( function resolveExplicitBodyProperty( program: Program, - metadata: Set, + metadata: HttpProperty[], contentTypes: string[], visibility: Visibility, usedIn: "request" | "response" | "multipart" @@ -196,34 +190,35 @@ function resolveExplicitBodyProperty( let resolvedBody: HttpOperationBody | HttpOperationMultipartBody | undefined; const duplicateTracker = new DuplicateTracker(); - for (const property of metadata) { - const isBodyVal = isBody(program, property); - const isBodyRootVal = isBodyRoot(program, property); - const isMultiPartBody = isMultipartBodyProperty(program, property); - - if (isBodyVal || isBodyRootVal || isMultiPartBody) { - duplicateTracker.track("body", property); + for (const item of metadata) { + if (item.kind === "body" || item.kind === "bodyRoot" || item.kind === "multipartBody") { + duplicateTracker.track("body", item.property); } - if (isBodyVal || isBodyRootVal) { - let containsMetadataAnnotations = false; - if (isBodyVal) { - const valid = diagnostics.pipe(validateBodyProperty(program, property, usedIn)); - containsMetadataAnnotations = !valid; - } - if (resolvedBody === undefined) { - resolvedBody = { - bodyKind: "single", - contentTypes, - type: property.type, - isExplicit: isBodyVal, - containsMetadataAnnotations, - property, - }; - } - } else if (isMultiPartBody) { - resolvedBody = diagnostics.pipe( - resolveMultiPartBody(program, property, contentTypes, visibility) - ); + + switch (item.kind) { + case "body": + case "bodyRoot": + let containsMetadataAnnotations = false; + if (item.kind === "body") { + const valid = diagnostics.pipe(validateBodyProperty(program, item.property, usedIn)); + containsMetadataAnnotations = !valid; + } + if (resolvedBody === undefined) { + resolvedBody = { + bodyKind: "single", + contentTypes, + type: item.property.type, + isExplicit: item.kind === "body", + containsMetadataAnnotations, + property: item.property, + }; + } + break; + case "multipartBody": + resolvedBody = diagnostics.pipe( + resolveMultiPartBody(program, item.property, contentTypes, visibility) + ); + break; } } for (const [_, items] of duplicateTracker.entries()) { diff --git a/packages/http/src/http-property.ts b/packages/http/src/http-property.ts new file mode 100644 index 0000000000..2e09de56f9 --- /dev/null +++ b/packages/http/src/http-property.ts @@ -0,0 +1,135 @@ +import { + compilerAssert, + type Diagnostic, + type ModelProperty, + type Program, +} from "@typespec/compiler"; +import { + getHeaderFieldOptions, + getPathParamOptions, + getQueryParamOptions, + isBody, + isBodyRoot, + isMultipartBodyProperty, + isStatusCode, +} from "./decorators.js"; +import { createDiagnostic } from "./lib.js"; +import { HeaderFieldOptions, PathParameterOptions, QueryParameterOptions } from "./types.js"; + +export type HttpProperty = + | HeaderProperty + | QueryProperty + | PathProperty + | StatusCodeProperty + | BodyProperty + | BodyRootProperty + | MultipartBodyProperty + | BodyPropertyProperty; + +export interface HttpPropertyBase { + readonly property: ModelProperty; +} + +export interface HeaderProperty extends HttpPropertyBase { + readonly kind: "header"; + readonly options: HeaderFieldOptions; +} +export interface QueryProperty extends HttpPropertyBase { + readonly kind: "query"; + readonly options: QueryParameterOptions; +} +export interface PathProperty extends HttpPropertyBase { + readonly kind: "path"; + readonly options: PathParameterOptions; +} +export interface StatusCodeProperty extends HttpPropertyBase { + readonly kind: "statusCode"; +} +export interface BodyProperty extends HttpPropertyBase { + readonly kind: "body"; +} +export interface BodyRootProperty extends HttpPropertyBase { + readonly kind: "bodyRoot"; +} +export interface MultipartBodyProperty extends HttpPropertyBase { + readonly kind: "multipartBody"; +} +/** Property to include inside the body */ +export interface BodyPropertyProperty extends HttpPropertyBase { + readonly kind: "bodyProperty"; +} + +export interface GetHttpPropertyOptions { + isImplicitPathParam?: (param: ModelProperty) => boolean; +} +/** + * Find the type of a property in a model + */ +export function getHttpProperty( + program: Program, + property: ModelProperty, + options: GetHttpPropertyOptions = {} +): [HttpProperty, readonly Diagnostic[]] { + const diagnostics: Diagnostic[] = []; + function createResult(opts: T): [T, readonly Diagnostic[]] { + return [{ ...opts, property } as any, diagnostics]; + } + + const annotations = { + header: getHeaderFieldOptions(program, property), + query: getQueryParamOptions(program, property), + path: getPathParamOptions(program, property), + body: isBody(program, property), + bodyRoot: isBodyRoot(program, property), + multipartBody: isMultipartBodyProperty(program, property), + statusCode: isStatusCode(program, property), + }; + const defined = Object.entries(annotations).filter((x) => !!x[1]); + if (defined.length === 0) { + if (options.isImplicitPathParam && options.isImplicitPathParam(property)) { + return createResult({ + kind: "path", + options: { + name: property.name, + type: "path", + }, + property, + }); + } + return [{ kind: "bodyProperty", property }, []]; + } else if (defined.length > 1) { + diagnostics.push( + createDiagnostic({ + code: "operation-param-duplicate-type", + format: { paramName: property.name, types: defined.map((x) => x[0]).join(", ") }, + target: property, + }) + ); + } + + if (annotations.header) { + return createResult({ kind: "header", options: annotations.header, property }); + } else if (annotations.query) { + return createResult({ kind: "query", options: annotations.query, property }); + } else if (annotations.path) { + if (property.optional) { + diagnostics.push( + createDiagnostic({ + code: "optional-path-param", + format: { paramName: property.name }, + target: property, + }) + ); + } + return createResult({ kind: "path", options: annotations.path, property }); + } else if (annotations.statusCode) { + return createResult({ kind: "statusCode", property }); + } else if (annotations.body) { + return createResult({ kind: "body", property }); + } else if (annotations.bodyRoot) { + return createResult({ kind: "bodyRoot", property }); + } else if (annotations.multipartBody) { + return createResult({ kind: "multipartBody", property }); + } + compilerAssert(false, `Unexpected http property type`, property); +} diff --git a/packages/http/src/parameters.ts b/packages/http/src/parameters.ts index 389ec194c9..e67df09763 100644 --- a/packages/http/src/parameters.ts +++ b/packages/http/src/parameters.ts @@ -6,15 +6,7 @@ import { Program, } from "@typespec/compiler"; import { extractBodyAndMetadata } from "./body.js"; -import { - getHeaderFieldOptions, - getOperationVerb, - getPathParamOptions, - getQueryParamOptions, - isBody, - isBodyRoot, -} from "./decorators.js"; -import { createDiagnostic } from "./lib.js"; +import { getOperationVerb } from "./decorators.js"; import { resolveRequestVisibility } from "./metadata.js"; import { HttpOperation, @@ -70,54 +62,16 @@ function getOperationParametersForVerb( }) ); - for (const param of metadata) { - const queryOptions = getQueryParamOptions(program, param); - const pathOptions = - getPathParamOptions(program, param) ?? - (isImplicitPathParam(param) && { type: "path", name: param.name }); - const headerOptions = getHeaderFieldOptions(program, param); - const isBodyVal = isBody(program, param); - const isBodyRootVal = isBodyRoot(program, param); - const defined = [ - ["query", queryOptions], - ["path", pathOptions], - ["header", headerOptions], - ["body", isBodyVal || isBodyRootVal], - ].filter((x) => !!x[1]); - if (defined.length >= 2) { - diagnostics.add( - createDiagnostic({ - code: "operation-param-duplicate-type", - format: { paramName: param.name, types: defined.map((x) => x[0]).join(", ") }, - target: param, - }) - ); - } - - if (queryOptions) { - parameters.push({ - ...queryOptions, - param, - }); - } else if (pathOptions) { - if (param.optional) { - diagnostics.add( - createDiagnostic({ - code: "optional-path-param", - format: { paramName: param.name }, - target: operation, - }) - ); - } - parameters.push({ - ...pathOptions, - param, - }); - } else if (headerOptions) { - parameters.push({ - ...headerOptions, - param, - }); + for (const item of metadata) { + switch (item.kind) { + case "path": + case "query": + case "header": + parameters.push({ + ...item.options, + param: item.property, + }); + break; } } diff --git a/packages/http/src/responses.ts b/packages/http/src/responses.ts index 09f51481eb..4a38581d8c 100644 --- a/packages/http/src/responses.ts +++ b/packages/http/src/responses.ts @@ -15,13 +15,9 @@ import { Type, } from "@typespec/compiler"; import { extractBodyAndMetadata } from "./body.js"; -import { - getHeaderFieldName, - getStatusCodeDescription, - getStatusCodesWithDiagnostics, - isHeader, - isStatusCode, -} from "./decorators.js"; +import { isContentTypeHeader } from "./content-types.js"; +import { getStatusCodeDescription, getStatusCodesWithDiagnostics } from "./decorators.js"; +import { HttpProperty } from "./http-property.js"; import { HttpStateKeys, reportDiagnostic } from "./lib.js"; import { Visibility } from "./metadata.js"; import { @@ -156,14 +152,14 @@ function processResponseType( function getResponseStatusCodes( program: Program, responseType: Type, - metadata: Set + metadata: HttpProperty[] ): [HttpStatusCodes, readonly Diagnostic[]] { const codes: HttpStatusCodes = []; const diagnostics = createDiagnosticCollector(); let statusFound = false; for (const prop of metadata) { - if (isStatusCode(program, prop)) { + if (prop.kind === "statusCode") { if (statusFound) { reportDiagnostic(program, { code: "multiple-status-codes", @@ -171,7 +167,7 @@ function getResponseStatusCodes( }); } statusFound = true; - codes.push(...diagnostics.pipe(getStatusCodesWithDiagnostics(program, prop))); + codes.push(...diagnostics.pipe(getStatusCodesWithDiagnostics(program, prop.property))); } } @@ -195,13 +191,12 @@ function getExplicitSetStatusCode(program: Program, entity: Model | ModelPropert */ function getResponseHeaders( program: Program, - metadata: Set + metadata: HttpProperty[] ): Record { const responseHeaders: Record = {}; for (const prop of metadata) { - const headerName = getHeaderFieldName(program, prop); - if (isHeader(program, prop) && headerName !== "content-type") { - responseHeaders[headerName] = prop; + if (prop.kind === "header" && !isContentTypeHeader(program, prop.property)) { + responseHeaders[prop.options.name] = prop.property; } } return responseHeaders; From 922d86b3135a40dedd625524248a6e0b66b66002 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 14 May 2024 11:08:02 -0700 Subject: [PATCH 11/39] Standarized handling of http properties --- packages/http/src/body.ts | 47 +++++++-------- packages/http/src/http-property.ts | 93 +++++++++++++++++++++++++++--- packages/http/src/metadata.ts | 72 +---------------------- packages/http/src/parameters.ts | 11 ++++ packages/http/src/types.ts | 3 +- 5 files changed, 119 insertions(+), 107 deletions(-) diff --git a/packages/http/src/body.ts b/packages/http/src/body.ts index e8c41e1637..24c764bd91 100644 --- a/packages/http/src/body.ts +++ b/packages/http/src/body.ts @@ -14,9 +14,14 @@ import { import { DuplicateTracker } from "@typespec/compiler/utils"; import { getContentTypes, isContentTypeHeader } from "./content-types.js"; import { isHeader, isPathParam, isQueryParam, isStatusCode } from "./decorators.js"; -import { GetHttpPropertyOptions, HttpProperty, getHttpProperty } from "./http-property.js"; +import { + GetHttpPropertyOptions, + HeaderProperty, + HttpProperty, + resolvePayloadProperties, +} from "./http-property.js"; import { createDiagnostic } from "./lib.js"; -import { Visibility, gatherMetadata, isMetadata, isVisible } from "./metadata.js"; +import { Visibility } from "./metadata.js"; import { getHttpPart } from "./private.decorators.js"; import { HttpOperationBody, HttpOperationMultipartBody, HttpOperationPart } from "./types.js"; @@ -34,23 +39,9 @@ export function extractBodyAndMetadata( ): [BodyAndMetadata, readonly Diagnostic[]] { const diagnostics = createDiagnosticCollector(); - const rootPropertyMap = new Map(); - const metadata = [ - ...gatherMetadata( - program, - diagnostics, - type, - visibility, - (_, param) => - isMetadata(program, param) || - Boolean(options.isImplicitPathParam && options.isImplicitPathParam(param)), - rootPropertyMap - ), - ].map((x) => diagnostics.pipe(getHttpProperty(program, x, options))); + const metadata = diagnostics.pipe(resolvePayloadProperties(program, type, visibility, options)); - const body = diagnostics.pipe( - resolveBody(program, type, metadata, rootPropertyMap, visibility, usedIn) - ); + const body = diagnostics.pipe(resolveBody(program, type, metadata, visibility, usedIn)); if (body) { if ( @@ -75,7 +66,6 @@ function resolveBody( program: Program, requestOrResponseType: Type, metadata: HttpProperty[], - rootPropertyMap: Map, visibility: Visibility, usedIn: "request" | "response" | "multipart" ): [HttpOperationBody | HttpOperationMultipartBody | undefined, readonly Diagnostic[]] { @@ -127,13 +117,8 @@ function resolveBody( } } - const bodyRoot = resolvedBody?.property ? rootPropertyMap.get(resolvedBody.property) : undefined; - - const unannotatedProperties = filterModelProperties( - program, - requestOrResponseType, - (p) => - !metadata.some((x) => x.property === p) && p !== bodyRoot && isVisible(program, p, visibility) + const unannotatedProperties = filterModelProperties(program, requestOrResponseType, (p) => + metadata.some((x) => x.property === p && x.kind === "bodyProperty") ); if (unannotatedProperties.properties.size > 0) { @@ -361,7 +346,15 @@ function resolvePart( return [undefined, [createDiagnostic({ code: "multipart-nested", target: type })]]; } - return [{ multi: false, name: part.options.name, body, headers: {} }, diagnostics]; + return [ + { + multi: false, + name: part.options.name, + body, + headers: metadata.filter((x): x is HeaderProperty => x.kind === "header"), + }, + diagnostics, + ]; } return [undefined, [createDiagnostic({ code: "multipart-part", target: type })]]; } diff --git a/packages/http/src/http-property.ts b/packages/http/src/http-property.ts index 2e09de56f9..c8fa2b203f 100644 --- a/packages/http/src/http-property.ts +++ b/packages/http/src/http-property.ts @@ -1,9 +1,15 @@ import { + DiagnosticResult, + Model, + Type, compilerAssert, + createDiagnosticCollector, + walkPropertiesInherited, type Diagnostic, type ModelProperty, type Program, } from "@typespec/compiler"; +import { Queue } from "@typespec/compiler/utils"; import { getHeaderFieldOptions, getPathParamOptions, @@ -14,6 +20,7 @@ import { isStatusCode, } from "./decorators.js"; import { createDiagnostic } from "./lib.js"; +import { Visibility, isVisible } from "./metadata.js"; import { HeaderFieldOptions, PathParameterOptions, QueryParameterOptions } from "./types.js"; export type HttpProperty = @@ -112,15 +119,6 @@ export function getHttpProperty( } else if (annotations.query) { return createResult({ kind: "query", options: annotations.query, property }); } else if (annotations.path) { - if (property.optional) { - diagnostics.push( - createDiagnostic({ - code: "optional-path-param", - format: { paramName: property.name }, - target: property, - }) - ); - } return createResult({ kind: "path", options: annotations.path, property }); } else if (annotations.statusCode) { return createResult({ kind: "statusCode", property }); @@ -133,3 +131,80 @@ export function getHttpProperty( } compilerAssert(false, `Unexpected http property type`, property); } + +/** + * Walks the given input(request parameters or response) and return all the properties and where they should be included(header, query, path, body, as a body property, etc.) + * + * @param rootMapOut If provided, the map will be populated to link nested metadata properties to their root properties. + */ +export function resolvePayloadProperties( + program: Program, + type: Type, + visibility: Visibility, + options: GetHttpPropertyOptions = {} +): DiagnosticResult { + const diagnostics = createDiagnosticCollector(); + const httpProperties = new Map(); + + if (type.kind !== "Model" || type.properties.size === 0) { + return diagnostics.wrap([]); + } + + const visited = new Set(); + const queue = new Queue<[Model, ModelProperty | undefined]>([[type, undefined]]); + + while (!queue.isEmpty()) { + const [model, rootOpt] = queue.dequeue(); + visited.add(model); + + for (const property of walkPropertiesInherited(model)) { + const root = rootOpt ?? property; + + if (!isVisible(program, property, visibility)) { + continue; + } + + let httpProperty = diagnostics.pipe(getHttpProperty(program, property, options)); + if (shouldTreatAsBodyProperty(httpProperty, visibility)) { + httpProperty = { kind: "bodyProperty", property }; + } + httpProperties.set(property, httpProperty); + if ( + property !== root && + (httpProperty.kind === "body" || + httpProperty.kind === "bodyRoot" || + httpProperty.kind === "multipartBody") + ) { + const parent = httpProperties.get(root); + if (parent?.kind === "bodyProperty") { + httpProperties.delete(root); + } + } + if (httpProperty.kind === "body" || httpProperty.kind === "multipartBody") { + continue; // We ignore any properties under `@body` or `@multipartBody` + } + + if ( + property.type.kind === "Model" && + !type.indexer && + type.properties.size > 0 && + !visited.has(property.type) + ) { + queue.enqueue([property.type, root]); + } + } + } + + return diagnostics.wrap([...httpProperties.values()]); +} + +function shouldTreatAsBodyProperty(property: HttpProperty, visibility: Visibility): boolean { + if (visibility & Visibility.Read) { + return property.kind === "query" || property.kind === "path"; + } + + if (!(visibility & Visibility.Read)) { + return property.kind === "statusCode"; + } + return false; +} diff --git a/packages/http/src/metadata.ts b/packages/http/src/metadata.ts index 5c3dc98c2a..8bf409c3a6 100644 --- a/packages/http/src/metadata.ts +++ b/packages/http/src/metadata.ts @@ -1,6 +1,5 @@ import { compilerAssert, - DiagnosticCollector, getEffectiveModelType, getParameterVisibility, isVisible as isVisibleCore, @@ -10,9 +9,8 @@ import { Program, Type, Union, - walkPropertiesInherited, } from "@typespec/compiler"; -import { Queue, TwoLevelMap } from "@typespec/compiler/utils"; +import { TwoLevelMap } from "@typespec/compiler/utils"; import { includeInapplicableMetadataInPayload, isBody, @@ -219,72 +217,6 @@ export function resolveRequestVisibility( return visibility; } -/** - * Walks the given type and collects all applicable metadata and `@body` - * properties recursively. - * - * @param rootMapOut If provided, the map will be populated to link - * nested metadata properties to their root properties. - */ -export function gatherMetadata( - program: Program, - diagnostics: DiagnosticCollector, // currently unused, but reserved for future diagnostics - type: Type, - visibility: Visibility, - isMetadataCallback = isMetadata, - rootMapOut?: Map -): Set { - const metadata = new Map(); - if (type.kind !== "Model" || type.properties.size === 0) { - return new Set(); - } - - const visited = new Set(); - const queue = new Queue<[Model, ModelProperty | undefined]>([[type, undefined]]); - - while (!queue.isEmpty()) { - const [model, rootOpt] = queue.dequeue(); - visited.add(model); - - for (const property of walkPropertiesInherited(model)) { - const root = rootOpt ?? property; - - if (!isVisible(program, property, visibility)) { - continue; - } - - // ISSUE: This should probably be an error, but that's a breaking - // change that currently breaks some samples and tests. - // - // The traversal here is level-order so that the preferred metadata in - // the case of duplicates, which is the most compatible with prior - // behavior where nested metadata was always dropped. - if (metadata.has(property.name)) { - continue; - } - - if (isApplicableMetadataOrBody(program, property, visibility, isMetadataCallback)) { - metadata.set(property.name, property); - rootMapOut?.set(property, root); - if (isBody(program, property)) { - continue; // We ignore any properties under `@body` - } - } - - if ( - property.type.kind === "Model" && - !type.indexer && - type.properties.size > 0 && - !visited.has(property.type) - ) { - queue.enqueue([property.type, root]); - } - } - } - - return new Set(metadata.values()); -} - /** * Determines if a property is metadata. A property is defined to be * metadata if it is marked `@header`, `@query`, `@path`, or `@statusCode`. @@ -607,7 +539,7 @@ export function createMetadataInfo(program: Program, options?: MetadataInfoOptio /** * If the type is an anonymous model, tries to find a named model that has the same - * set of properties when non-payload properties are excluded. + * set of properties when non-payload properties are excluded.we */ function getEffectivePayloadType(type: Type, visibility: Visibility): Type { if (type.kind === "Model" && !type.name) { diff --git a/packages/http/src/parameters.ts b/packages/http/src/parameters.ts index e67df09763..f5ce002873 100644 --- a/packages/http/src/parameters.ts +++ b/packages/http/src/parameters.ts @@ -7,6 +7,7 @@ import { } from "@typespec/compiler"; import { extractBodyAndMetadata } from "./body.js"; import { getOperationVerb } from "./decorators.js"; +import { createDiagnostic } from "./lib.js"; import { resolveRequestVisibility } from "./metadata.js"; import { HttpOperation, @@ -65,6 +66,16 @@ function getOperationParametersForVerb( for (const item of metadata) { switch (item.kind) { case "path": + if (item.property.optional) { + diagnostics.add( + createDiagnostic({ + code: "optional-path-param", + format: { paramName: item.property.name }, + target: item.property, + }) + ); + } + // eslint-disable-next-line no-fallthrough case "query": case "header": parameters.push({ diff --git a/packages/http/src/types.ts b/packages/http/src/types.ts index 78606cd698..231f8a75d8 100644 --- a/packages/http/src/types.ts +++ b/packages/http/src/types.ts @@ -10,6 +10,7 @@ import { Tuple, Type, } from "@typespec/compiler"; +import { HeaderProperty } from "./http-property.js"; /** * @deprecated use `HttpOperation`. To remove in November 2022 release. @@ -486,7 +487,7 @@ export interface HttpOperationPart { /** Part body */ readonly body: HttpOperationBody; /** Part headers */ - readonly headers?: Record; + readonly headers?: HeaderProperty[]; /** If there can be multiple of that part */ readonly multi: boolean; } From 4299e330e2eaa54a33cc72b33ec5241aa443828a Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 14 May 2024 11:44:05 -0700 Subject: [PATCH 12/39] fix --- packages/openapi3/src/openapi.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/openapi3/src/openapi.ts b/packages/openapi3/src/openapi.ts index 93803bc0e9..17f9c8d443 100644 --- a/packages/openapi3/src/openapi.ts +++ b/packages/openapi3/src/openapi.ts @@ -1141,7 +1141,7 @@ function createOAPIEmitter( body.type, visibility, body.isExplicit && body.containsMetadataAnnotations, - contentType + contentType.startsWith("multipart/") ? contentType : undefined ), }; case "multipart": From 15fb6a78be353f969080ffe35e7879b508109322 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 14 May 2024 11:57:46 -0700 Subject: [PATCH 13/39] Include headers --- packages/http/src/types.ts | 2 +- packages/openapi3/src/openapi.ts | 18 +++++++++++++++--- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/packages/http/src/types.ts b/packages/http/src/types.ts index 231f8a75d8..c23c6ee53a 100644 --- a/packages/http/src/types.ts +++ b/packages/http/src/types.ts @@ -487,7 +487,7 @@ export interface HttpOperationPart { /** Part body */ readonly body: HttpOperationBody; /** Part headers */ - readonly headers?: HeaderProperty[]; + readonly headers: HeaderProperty[]; /** If there can be multiple of that part */ readonly multi: boolean; } diff --git a/packages/openapi3/src/openapi.ts b/packages/openapi3/src/openapi.ts index 17f9c8d443..e29457ac71 100644 --- a/packages/openapi3/src/openapi.ts +++ b/packages/openapi3/src/openapi.ts @@ -1178,9 +1178,9 @@ function createOAPIEmitter( part.body.isExplicit && part.body.containsMetadataAnnotations, "application/json" ); - properties[part.name ?? partIndex.toString()] = schema; + properties[partName] = schema; - const encoding = resolveEncodingForMultipartPart(part); + const encoding = resolveEncodingForMultipartPart(part, visibility); if (encoding) { encodings[partName] = encoding; } @@ -1198,11 +1198,23 @@ function createOAPIEmitter( return result; } - function resolveEncodingForMultipartPart(part: HttpOperationPart): OpenAPI3Encoding { + function resolveEncodingForMultipartPart( + part: HttpOperationPart, + visibility: Visibility + ): OpenAPI3Encoding { const encoding: OpenAPI3Encoding = {}; if (part.body.contentTypes) { encoding.contentType = part.body.contentTypes.join(", "); } + if (part.headers.length > 0) { + encoding.headers = {}; + for (const header of part.headers) { + const schema = getOpenAPIParameterBase(header.property, visibility); + if (schema !== undefined) { + encoding.headers[header.options.name] = schema; + } + } + } return encoding; } From 4eceb538c327909730ea1fef552ff608c59fb8c0 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 14 May 2024 12:10:40 -0700 Subject: [PATCH 14/39] rename --- packages/http/src/parameters.ts | 2 +- packages/http/src/{body.ts => payload.ts} | 4 ++-- packages/http/src/responses.ts | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) rename packages/http/src/{body.ts => payload.ts} (99%) diff --git a/packages/http/src/parameters.ts b/packages/http/src/parameters.ts index f5ce002873..cc26dec868 100644 --- a/packages/http/src/parameters.ts +++ b/packages/http/src/parameters.ts @@ -5,10 +5,10 @@ import { Operation, Program, } from "@typespec/compiler"; -import { extractBodyAndMetadata } from "./body.js"; import { getOperationVerb } from "./decorators.js"; import { createDiagnostic } from "./lib.js"; import { resolveRequestVisibility } from "./metadata.js"; +import { extractBodyAndMetadata } from "./payload.js"; import { HttpOperation, HttpOperationParameter, diff --git a/packages/http/src/body.ts b/packages/http/src/payload.ts similarity index 99% rename from packages/http/src/body.ts rename to packages/http/src/payload.ts index 24c764bd91..d9ee3406ea 100644 --- a/packages/http/src/body.ts +++ b/packages/http/src/payload.ts @@ -25,7 +25,7 @@ import { Visibility } from "./metadata.js"; import { getHttpPart } from "./private.decorators.js"; import { HttpOperationBody, HttpOperationMultipartBody, HttpOperationPart } from "./types.js"; -export interface BodyAndMetadata { +export interface HttpPayload { body?: HttpOperationBody | HttpOperationMultipartBody; metadata: HttpProperty[]; } @@ -36,7 +36,7 @@ export function extractBodyAndMetadata( visibility: Visibility, usedIn: "request" | "response" | "multipart", options: ExtractBodyAndMetadataOptions = {} -): [BodyAndMetadata, readonly Diagnostic[]] { +): [HttpPayload, readonly Diagnostic[]] { const diagnostics = createDiagnosticCollector(); const metadata = diagnostics.pipe(resolvePayloadProperties(program, type, visibility, options)); diff --git a/packages/http/src/responses.ts b/packages/http/src/responses.ts index 4a38581d8c..04a72456e7 100644 --- a/packages/http/src/responses.ts +++ b/packages/http/src/responses.ts @@ -14,12 +14,12 @@ import { Program, Type, } from "@typespec/compiler"; -import { extractBodyAndMetadata } from "./body.js"; import { isContentTypeHeader } from "./content-types.js"; import { getStatusCodeDescription, getStatusCodesWithDiagnostics } from "./decorators.js"; import { HttpProperty } from "./http-property.js"; import { HttpStateKeys, reportDiagnostic } from "./lib.js"; import { Visibility } from "./metadata.js"; +import { extractBodyAndMetadata } from "./payload.js"; import { HttpOperationBody, HttpOperationMultipartBody, From 78a74b40077e09176d2e03308820433b455fe602 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 14 May 2024 12:11:25 -0700 Subject: [PATCH 15/39] . --- packages/http/src/payload.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/http/src/payload.ts b/packages/http/src/payload.ts index d9ee3406ea..abc6b00364 100644 --- a/packages/http/src/payload.ts +++ b/packages/http/src/payload.ts @@ -26,8 +26,8 @@ import { getHttpPart } from "./private.decorators.js"; import { HttpOperationBody, HttpOperationMultipartBody, HttpOperationPart } from "./types.js"; export interface HttpPayload { - body?: HttpOperationBody | HttpOperationMultipartBody; - metadata: HttpProperty[]; + readonly body?: HttpOperationBody | HttpOperationMultipartBody; + readonly metadata: HttpProperty[]; } export interface ExtractBodyAndMetadataOptions extends GetHttpPropertyOptions {} export function extractBodyAndMetadata( From 1ea94bd789e458a2e2ca3a2e44072fb07a92111e Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 14 May 2024 12:13:37 -0700 Subject: [PATCH 16/39] Renmae --- packages/http/src/parameters.ts | 4 ++-- packages/http/src/payload.ts | 4 ++-- packages/http/src/responses.ts | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/http/src/parameters.ts b/packages/http/src/parameters.ts index cc26dec868..ccaec0d2e4 100644 --- a/packages/http/src/parameters.ts +++ b/packages/http/src/parameters.ts @@ -8,7 +8,7 @@ import { import { getOperationVerb } from "./decorators.js"; import { createDiagnostic } from "./lib.js"; import { resolveRequestVisibility } from "./metadata.js"; -import { extractBodyAndMetadata } from "./payload.js"; +import { resolveHttpPayload } from "./payload.js"; import { HttpOperation, HttpOperationParameter, @@ -58,7 +58,7 @@ function getOperationParametersForVerb( const parameters: HttpOperationParameter[] = []; const { body: resolvedBody, metadata } = diagnostics.pipe( - extractBodyAndMetadata(program, operation.parameters, visibility, "request", { + resolveHttpPayload(program, operation.parameters, visibility, "request", { isImplicitPathParam, }) ); diff --git a/packages/http/src/payload.ts b/packages/http/src/payload.ts index abc6b00364..8fae2f6cf1 100644 --- a/packages/http/src/payload.ts +++ b/packages/http/src/payload.ts @@ -30,7 +30,7 @@ export interface HttpPayload { readonly metadata: HttpProperty[]; } export interface ExtractBodyAndMetadataOptions extends GetHttpPropertyOptions {} -export function extractBodyAndMetadata( +export function resolveHttpPayload( program: Program, type: Type, visibility: Visibility, @@ -334,7 +334,7 @@ function resolvePart( ): [HttpOperationPart | undefined, readonly Diagnostic[]] { const part = getHttpPart(program, type); if (part) { - const [{ body, metadata }, diagnostics] = extractBodyAndMetadata( + const [{ body, metadata }, diagnostics] = resolveHttpPayload( program, part.type, visibility, diff --git a/packages/http/src/responses.ts b/packages/http/src/responses.ts index 04a72456e7..7510ad701d 100644 --- a/packages/http/src/responses.ts +++ b/packages/http/src/responses.ts @@ -19,7 +19,7 @@ import { getStatusCodeDescription, getStatusCodesWithDiagnostics } from "./decor import { HttpProperty } from "./http-property.js"; import { HttpStateKeys, reportDiagnostic } from "./lib.js"; import { Visibility } from "./metadata.js"; -import { extractBodyAndMetadata } from "./payload.js"; +import { resolveHttpPayload } from "./payload.js"; import { HttpOperationBody, HttpOperationMultipartBody, @@ -89,7 +89,7 @@ function processResponseType( ) { // Get body let { body: resolvedBody, metadata } = diagnostics.pipe( - extractBodyAndMetadata(program, responseType, Visibility.Read, "response") + resolveHttpPayload(program, responseType, Visibility.Read, "response") ); // Get explicity defined status codes const statusCodes: HttpStatusCodes = diagnostics.pipe( From 118e4e08ec70c863a06613722320b8c5f369fafc Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 14 May 2024 12:18:05 -0700 Subject: [PATCH 17/39] Add nameless part validation for form-data --- packages/http/src/lib.ts | 6 ++++++ packages/http/src/payload.ts | 15 ++++++++++++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/packages/http/src/lib.ts b/packages/http/src/lib.ts index 678fa0ff08..22f5bbac2d 100644 --- a/packages/http/src/lib.ts +++ b/packages/http/src/lib.ts @@ -135,6 +135,12 @@ export const $lib = createTypeSpecLibrary({ default: "Cannot use @multipartBody inside of an HttpPart", }, }, + "formdata-no-part-name": { + severity: "error", + messages: { + default: "Part used in application/form-data must have a name.", + }, + }, "header-format-required": { severity: "error", messages: { diff --git a/packages/http/src/payload.ts b/packages/http/src/payload.ts index 8fae2f6cf1..21d8658331 100644 --- a/packages/http/src/payload.ts +++ b/packages/http/src/payload.ts @@ -292,6 +292,11 @@ function resolveMultiPartBodyFromModel( return diagnostics.wrap({ bodyKind: "multipart", contentTypes, parts, property, type }); } +const multipartContentTypes = { + formData: "multipart/form-data", + mixed: "multipart/mixed", +} as const; + function resolveMultiPartBodyFromTuple( program: Program, property: ModelProperty, @@ -301,8 +306,16 @@ function resolveMultiPartBodyFromTuple( ): [HttpOperationMultipartBody | undefined, readonly Diagnostic[]] { const diagnostics = createDiagnosticCollector(); const parts: HttpOperationPart[] = []; - for (const item of type.values) { + for (const [index, item] of type.values.entries()) { const part = diagnostics.pipe(resolvePartOrParts(program, item, visibility)); + if (part?.name === undefined && contentTypes.includes(multipartContentTypes.formData)) { + diagnostics.add( + createDiagnostic({ + code: "multipart-part", + target: type.node.values[index], + }) + ); + } if (part) { parts.push(part); } From 27903911450f4471baa4080308e1bd058a088127 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 14 May 2024 12:35:24 -0700 Subject: [PATCH 18/39] Resolve correct --- packages/http/src/payload.ts | 40 ++++++++++++++++++++++++++++++++---- 1 file changed, 36 insertions(+), 4 deletions(-) diff --git a/packages/http/src/payload.ts b/packages/http/src/payload.ts index 21d8658331..94aa1572da 100644 --- a/packages/http/src/payload.ts +++ b/packages/http/src/payload.ts @@ -8,6 +8,8 @@ import { createDiagnosticCollector, filterModelProperties, getDiscriminator, + getEncode, + ignoreDiagnostics, isArrayModelType, navigateType, } from "@typespec/compiler"; @@ -71,7 +73,7 @@ function resolveBody( ): [HttpOperationBody | HttpOperationMultipartBody | undefined, readonly Diagnostic[]] { const diagnostics = createDiagnosticCollector(); const { contentTypes, contentTypeProperty } = diagnostics.pipe( - resolveContentTypes(program, metadata) + resolveContentTypes(program, metadata, usedIn) ); // non-model or intrinsic/array model -> response body is response type if (requestOrResponseType.kind !== "Model" || isArrayModelType(program, requestOrResponseType)) { @@ -153,7 +155,8 @@ function resolveBody( function resolveContentTypes( program: Program, - metadata: HttpProperty[] + metadata: HttpProperty[], + usedIn: "request" | "response" | "multipart" ): [{ contentTypes: string[]; contentTypeProperty?: ModelProperty }, readonly Diagnostic[]] { for (const prop of metadata) { if (prop.kind === "header" && isContentTypeHeader(program, prop.property)) { @@ -161,7 +164,13 @@ function resolveContentTypes( return [{ contentTypes, contentTypeProperty: prop.property }, diagnostics]; } } - return [{ contentTypes: ["application/json"] }, []]; + switch (usedIn) { + case "multipart": + // Figure this out later + return [{ contentTypes: [] }, []]; + default: + return [{ contentTypes: ["application/json"] }, []]; + } } function resolveExplicitBodyProperty( @@ -347,7 +356,7 @@ function resolvePart( ): [HttpOperationPart | undefined, readonly Diagnostic[]] { const part = getHttpPart(program, type); if (part) { - const [{ body, metadata }, diagnostics] = resolveHttpPayload( + let [{ body, metadata }, diagnostics] = resolveHttpPayload( program, part.type, visibility, @@ -359,6 +368,9 @@ function resolvePart( return [undefined, [createDiagnostic({ code: "multipart-nested", target: type })]]; } + if (body.contentTypes.length === 0) { + body = { ...body, contentTypes: [resolveDefaultContentTypeForPart(program, body.type)] }; + } return [ { multi: false, @@ -371,3 +383,23 @@ function resolvePart( } return [undefined, [createDiagnostic({ code: "multipart-part", target: type })]]; } + +function resolveDefaultContentTypeForPart(program: Program, type: Type): string { + if (type.kind === "Scalar") { + const encodedAs = getEncode(program, type); + if (encodedAs) { + type = encodedAs.type; + } + if ( + ignoreDiagnostics( + program.checker.isTypeAssignableTo(type, program.checker.getStdType("bytes"), type) + ) + ) { + return "application/octet-stream"; + } else { + return "text/plain"; + } + } else { + return "application/json"; + } +} From c486f637fd100a6eab838f489553f9edd1368bf9 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 14 May 2024 12:43:12 -0700 Subject: [PATCH 19/39] omit uncessary contnet types --- packages/openapi3/src/openapi.ts | 31 ++++++++++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/packages/openapi3/src/openapi.ts b/packages/openapi3/src/openapi.ts index e29457ac71..5a445b6bf6 100644 --- a/packages/openapi3/src/openapi.ts +++ b/packages/openapi3/src/openapi.ts @@ -1180,7 +1180,7 @@ function createOAPIEmitter( ); properties[partName] = schema; - const encoding = resolveEncodingForMultipartPart(part, visibility); + const encoding = resolveEncodingForMultipartPart(part, visibility, schema); if (encoding) { encodings[partName] = encoding; } @@ -1200,10 +1200,11 @@ function createOAPIEmitter( function resolveEncodingForMultipartPart( part: HttpOperationPart, - visibility: Visibility + visibility: Visibility, + schema: OpenAPI3Schema ): OpenAPI3Encoding { const encoding: OpenAPI3Encoding = {}; - if (part.body.contentTypes) { + if (!isDefaultContentTypeForOpenAPI3(part.body.contentTypes, schema)) { encoding.contentType = part.body.contentTypes.join(", "); } if (part.headers.length > 0) { @@ -1218,6 +1219,30 @@ function createOAPIEmitter( return encoding; } + function isDefaultContentTypeForOpenAPI3( + contentTypes: string[], + schema: OpenAPI3Schema + ): boolean { + if (contentTypes.length === 0) { + return false; + } + if (contentTypes.length > 1) { + return false; + } + const contentType = contentTypes[0]; + + switch (contentType) { + case "text/plain": + return schema.type === "string" || schema.type === "number"; + case "application/octet-stream": + return schema.type === "string" && schema.format === "binary"; + case "application/json": + return schema.type === "object"; + } + + return false; + } + function getParamPlaceholder(property: ModelProperty) { let spreadParam = false; From 47fe13bfedda899e6753fa0bfd2887791e4e8607 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 14 May 2024 13:38:23 -0700 Subject: [PATCH 20/39] Get content type from file --- packages/http/src/content-types.ts | 2 + packages/http/src/payload.ts | 15 +++++++- packages/http/src/private.decorators.ts | 50 +++++++++++++++++++++++-- 3 files changed, 63 insertions(+), 4 deletions(-) diff --git a/packages/http/src/content-types.ts b/packages/http/src/content-types.ts index eaf9dc99c6..2f43478de3 100644 --- a/packages/http/src/content-types.ts +++ b/packages/http/src/content-types.ts @@ -39,6 +39,8 @@ export function getContentTypes(property: ModelProperty): [string[], readonly Di } return diagnostics.wrap(contentTypes); + } else if (property.type.kind === "Scalar" && property.type.name === "string") { + return [["*/*"], []]; } return [[], [createDiagnostic({ code: "content-type-string", target: property })]]; diff --git a/packages/http/src/payload.ts b/packages/http/src/payload.ts index 94aa1572da..68d549fe8d 100644 --- a/packages/http/src/payload.ts +++ b/packages/http/src/payload.ts @@ -24,7 +24,7 @@ import { } from "./http-property.js"; import { createDiagnostic } from "./lib.js"; import { Visibility } from "./metadata.js"; -import { getHttpPart } from "./private.decorators.js"; +import { getHttpFileModel, getHttpPart } from "./private.decorators.js"; import { HttpOperationBody, HttpOperationMultipartBody, HttpOperationPart } from "./types.js"; export interface HttpPayload { @@ -75,6 +75,19 @@ function resolveBody( const { contentTypes, contentTypeProperty } = diagnostics.pipe( resolveContentTypes(program, metadata, usedIn) ); + + const file = getHttpFileModel(program, requestOrResponseType); + if (file !== undefined) { + const file = getHttpFileModel(program, requestOrResponseType)!; + return diagnostics.wrap({ + bodyKind: "single", + contentTypes: diagnostics.pipe(getContentTypes(file.contentType)), + type: file.contents.type, + isExplicit: false, + containsMetadataAnnotations: false, + }); + } + // non-model or intrinsic/array model -> response body is response type if (requestOrResponseType.kind !== "Model" || isArrayModelType(program, requestOrResponseType)) { return diagnostics.wrap({ diff --git a/packages/http/src/private.decorators.ts b/packages/http/src/private.decorators.ts index 0b28089bc3..d6a22c63e8 100644 --- a/packages/http/src/private.decorators.ts +++ b/packages/http/src/private.decorators.ts @@ -1,4 +1,11 @@ -import { DecoratorContext, Model, Program, Type } from "@typespec/compiler"; +import { + DecoratorContext, + Model, + ModelProperty, + Program, + Type, + getProperty, +} from "@typespec/compiler"; import { HttpFileDecorator, HttpPartDecorator, @@ -43,8 +50,45 @@ export const $httpFile: HttpFileDecorator = (context: DecoratorContext, target: /** * Check if the given type is an `HttpFile` */ -export function isHttpFile(program: Program, target: Type) { - return program.stateSet(HttpStateKeys.file).has(target); +export function isHttpFile(program: Program, type: Type) { + return program.stateSet(HttpStateKeys.file).has(type); +} + +export function isOrExtendsHttpFile(program: Program, type: Type) { + if (type.kind !== "Model") { + return false; + } + + let current: Model | undefined = type; + + while (current) { + if (isHttpFile(program, current)) { + return true; + } + + current = current.baseModel; + } + + return false; +} + +export interface HttpFileModel { + readonly type: Type; + readonly contentType: ModelProperty; + readonly filename: ModelProperty; + readonly contents: ModelProperty; +} + +export function getHttpFileModel(program: Program, type: Type): HttpFileModel | undefined { + if (type.kind !== "Model" || !isOrExtendsHttpFile(program, type)) { + return undefined; + } + + const contentType = getProperty(type, "contentType")!; + const filename = getProperty(type, "filename")!; + const contents = getProperty(type, "contents")!; + + return { contents, contentType, filename, type }; } export interface HttpPartOptions { From 98f1a787c04ef2ea19b24f1d48998f8bea3f6011 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 14 May 2024 14:30:18 -0700 Subject: [PATCH 21/39] Multipart http tests --- packages/http/src/lib.ts | 8 +- packages/http/src/payload.ts | 17 ++- packages/http/test/multipart.test.ts | 196 +++++++++++++++++++++++++++ vitest.workspace.ts | 3 - 4 files changed, 218 insertions(+), 6 deletions(-) create mode 100644 packages/http/test/multipart.test.ts diff --git a/packages/http/src/lib.ts b/packages/http/src/lib.ts index 22f5bbac2d..2d367444b1 100644 --- a/packages/http/src/lib.ts +++ b/packages/http/src/lib.ts @@ -117,6 +117,12 @@ export const $lib = createTypeSpecLibrary({ default: `@visibility("write") is not supported. Use @visibility("update"), @visibility("create") or @visibility("create", "update") as appropriate.`, }, }, + "multipart-invalid-content-type": { + severity: "error", + messages: { + default: paramMessage`Content type '${"contentType"}' is not a multipart content type. Supported content types are: ${"supportedContentTypes"}.`, + }, + }, "multipart-model": { severity: "error", messages: { @@ -138,7 +144,7 @@ export const $lib = createTypeSpecLibrary({ "formdata-no-part-name": { severity: "error", messages: { - default: "Part used in application/form-data must have a name.", + default: "Part used in multipart/form-data must have a name.", }, }, "header-format-required": { diff --git a/packages/http/src/payload.ts b/packages/http/src/payload.ts index 68d549fe8d..2b58a3593e 100644 --- a/packages/http/src/payload.ts +++ b/packages/http/src/payload.ts @@ -307,7 +307,7 @@ function resolveMultiPartBodyFromModel( for (const item of type.properties.values()) { const part = diagnostics.pipe(resolvePartOrParts(program, item.type, visibility)); if (part) { - parts.push(part); + parts.push({ ...part, name: item.name }); } } @@ -318,6 +318,7 @@ const multipartContentTypes = { formData: "multipart/form-data", mixed: "multipart/mixed", } as const; +const multipartContentTypesValues = Object.values(multipartContentTypes); function resolveMultiPartBodyFromTuple( program: Program, @@ -328,12 +329,24 @@ function resolveMultiPartBodyFromTuple( ): [HttpOperationMultipartBody | undefined, readonly Diagnostic[]] { const diagnostics = createDiagnosticCollector(); const parts: HttpOperationPart[] = []; + + for (const contentType of contentTypes) { + if (!multipartContentTypesValues.includes(contentType as any)) { + diagnostics.add( + createDiagnostic({ + code: "multipart-invalid-content-type", + format: { contentType, valid: multipartContentTypesValues.join(", ") }, + target: type, + }) + ); + } + } for (const [index, item] of type.values.entries()) { const part = diagnostics.pipe(resolvePartOrParts(program, item, visibility)); if (part?.name === undefined && contentTypes.includes(multipartContentTypes.formData)) { diagnostics.add( createDiagnostic({ - code: "multipart-part", + code: "formdata-no-part-name", target: type.node.values[index], }) ); diff --git a/packages/http/test/multipart.test.ts b/packages/http/test/multipart.test.ts new file mode 100644 index 0000000000..7aed57c270 --- /dev/null +++ b/packages/http/test/multipart.test.ts @@ -0,0 +1,196 @@ +import { expectDiagnosticEmpty, expectDiagnostics } from "@typespec/compiler/testing"; +import { deepStrictEqual, strictEqual } from "assert"; +import { describe, it } from "vitest"; +import { HttpOperationMultipartBody } from "../src/types.js"; +import { getOperationsWithServiceNamespace } from "./test-host.js"; + +it("emit diagnostic when using invalid content type for multipart ", async () => { + const diagnostics = await diagnoseHttpOp(` + op read( + @header contentType: "application/json", + @multipartBody body: [HttpPart]): void; + `); + expectDiagnostics(diagnostics, { + code: "@typespec/http/multipart-invalid-content-type", + message: + "Content type 'application/json' is not a multipart content type. Supported content types are: .", + }); +}); + +describe("define with the tuple form", () => { + describe("part without name", () => { + it("emit diagnostic when using multipart/form-data", async () => { + const diagnostics = await diagnoseHttpOp(` + op read( + @header contentType: "multipart/form-data", + @multipartBody body: [HttpPart]): void; + `); + expectDiagnostics(diagnostics, { + code: "@typespec/http/formdata-no-part-name", + message: "Part used in multipart/form-data must have a name.", + }); + }); + + it("include anonymous part when multipart/form-data", async () => { + const body = await getMultipartBody(` + op read( + @header contentType: "multipart/mixed", + @multipartBody body: [HttpPart]): void; + `); + strictEqual(body.parts.length, 1); + strictEqual(body.parts[0].name, undefined); + }); + }); + + it("resolve name from HttpPart options", async () => { + const body = await getMultipartBody(` + op read( + @header contentType: "multipart/mixed", + @multipartBody body: [HttpPart]): void; + `); + strictEqual(body.parts.length, 1); + strictEqual(body.parts[0].name, "myPart"); + }); + + it("using an array of parts marks the part as multi: true", async () => { + const body = await getMultipartBody(` + op read( + @header contentType: "multipart/mixed", + @multipartBody body: [ + HttpPart[] + ]): void; + `); + + strictEqual(body.parts.length, 1); + strictEqual(body.parts[0].multi, true); + }); + + it("emit diagnostic if using non HttpPart in tuple", async () => { + const diagnostics = await diagnoseHttpOp(` + op read( + @header contentType: "multipart/mixed", + @multipartBody body: [string]): void; + `); + expectDiagnostics(diagnostics, { + code: "@typespec/http/multipart-part", + message: "Expect item to be an HttpPart model.", + }); + }); +}); + +describe("define with the object form", () => { + it("use name from property name", async () => { + const body = await getMultipartBody(` + op read( + @header contentType: "multipart/mixed", + @multipartBody body: { + myPropertyPart: HttpPart + }): void; + `); + + strictEqual(body.parts.length, 1); + strictEqual(body.parts[0].name, "myPropertyPart"); + }); + + it("using an array of parts marks the part as multi: true", async () => { + const body = await getMultipartBody(` + op read( + @header contentType: "multipart/mixed", + @multipartBody body: { + part: HttpPart[] + }): void; + `); + + strictEqual(body.parts.length, 1); + strictEqual(body.parts[0].multi, true); + }); + + it("emit diagnostic if using non HttpPart in tuple", async () => { + const diagnostics = await diagnoseHttpOp(` + op read( + @header contentType: "multipart/mixed", + @multipartBody body: { part: string }): void; + `); + expectDiagnostics(diagnostics, { + code: "@typespec/http/multipart-part", + message: "Expect item to be an HttpPart model.", + }); + }); +}); + +describe("resolving part payload", () => { + it("emit diagnostic if trying to use @multipartBody inside an HttpPart", async () => { + const diagnostics = await diagnoseHttpOp(` + op read( + @header contentType: "multipart/mixed", + @multipartBody body: [HttpPart<{@multipartBody nested: []}>]): void; + `); + expectDiagnostics(diagnostics, { + code: "@typespec/http/multipart-nested", + message: "Cannot use @multipartBody inside of an HttpPart", + }); + }); + it("extract headers for the part", async () => { + const op = await getHttpOp(` + op read( + @header contentType: "multipart/mixed", + @header operationHeader: string; + @multipartBody body: [ + HttpPart<{ + @body nested: string, + @header partHeader: string; + }>]): void; + `); + + strictEqual(op.parameters.parameters.length, 2); + strictEqual(op.parameters.parameters[0].name, "content-type"); + strictEqual(op.parameters.parameters[1].name, "operation-header"); + + const body = op.parameters.body; + strictEqual(body?.bodyKind, "multipart"); + strictEqual(body.parts.length, 1); + strictEqual(body.parts[0].headers.length, 1); + strictEqual(body.parts[0].headers[0].options.name, "part-header"); + }); + + describe("part default content type", () => { + it.each([ + ["bytes", "application/octet-stream"], + ["File", "*/*"], + ["string", "text/plain"], + ["int32", "text/plain"], + ["Foo", "application/json", `model Foo { value: string }`], + ])("%s body", async (type, expectedContentType, extra?: string) => { + const body = await getMultipartBody(` + op read( + @header contentType: "multipart/mixed", + @multipartBody body: [ + HttpPart<${type}> + ]): void; + ${extra ?? ""} + `); + + strictEqual(body.parts.length, 1); + deepStrictEqual(body.parts[0].body.contentTypes, [expectedContentType]); + }); + }); +}); + +async function getHttpOp(code: string) { + const [ops, diagnostics] = await getOperationsWithServiceNamespace(code); + expectDiagnosticEmpty(diagnostics); + strictEqual(ops.length, 1); + return ops[0]; +} + +async function getMultipartBody(code: string): Promise { + const op = await getHttpOp(code); + const body = op.parameters.body; + strictEqual(body?.bodyKind, "multipart"); + return body; +} + +async function diagnoseHttpOp(code: string) { + const [_, diagnostics] = await getOperationsWithServiceNamespace(code); + return diagnostics; +} diff --git a/vitest.workspace.ts b/vitest.workspace.ts index 959f05ce29..28272b3823 100644 --- a/vitest.workspace.ts +++ b/vitest.workspace.ts @@ -15,7 +15,4 @@ export const defaultTypeSpecVitestConfig = { }, watchExclude: [], }, - build: { - outDir: "dummy", // Workaround for bug https://github.com/vitest-dev/vitest/issues/5429 - }, }; From 574a98ee55dd20a0a8b68fc3d010ad9f59277e0d Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 14 May 2024 15:56:47 -0700 Subject: [PATCH 22/39] Initial openapi3 tests --- packages/http/src/payload.ts | 46 ++++--- packages/http/src/types.ts | 2 + packages/http/test/multipart.test.ts | 9 +- packages/http/test/responses.test.ts | 2 +- packages/openapi3/src/openapi.ts | 63 ++++++--- packages/openapi3/src/schema-emitter.ts | 19 ++- packages/openapi3/test/multipart.test.ts | 161 ++++++++++++++++++++++- vitest.workspace.ts | 7 +- 8 files changed, 259 insertions(+), 50 deletions(-) diff --git a/packages/http/src/payload.ts b/packages/http/src/payload.ts index 2b58a3593e..7ac2dab1de 100644 --- a/packages/http/src/payload.ts +++ b/packages/http/src/payload.ts @@ -307,7 +307,7 @@ function resolveMultiPartBodyFromModel( for (const item of type.properties.values()) { const part = diagnostics.pipe(resolvePartOrParts(program, item.type, visibility)); if (part) { - parts.push({ ...part, name: item.name }); + parts.push({ ...part, name: item.name, optional: item.optional }); } } @@ -395,13 +395,14 @@ function resolvePart( } if (body.contentTypes.length === 0) { - body = { ...body, contentTypes: [resolveDefaultContentTypeForPart(program, body.type)] }; + body = { ...body, contentTypes: resolveDefaultContentTypeForPart(program, body.type) }; } return [ { multi: false, name: part.options.name, body, + optional: false, headers: metadata.filter((x): x is HeaderProperty => x.kind === "header"), }, diagnostics, @@ -410,22 +411,33 @@ function resolvePart( return [undefined, [createDiagnostic({ code: "multipart-part", target: type })]]; } -function resolveDefaultContentTypeForPart(program: Program, type: Type): string { - if (type.kind === "Scalar") { - const encodedAs = getEncode(program, type); - if (encodedAs) { - type = encodedAs.type; - } - if ( - ignoreDiagnostics( - program.checker.isTypeAssignableTo(type, program.checker.getStdType("bytes"), type) - ) - ) { - return "application/octet-stream"; +function resolveDefaultContentTypeForPart(program: Program, type: Type): string[] { + function resolve(type: Type): string[] { + if (type.kind === "Scalar") { + const encodedAs = getEncode(program, type); + if (encodedAs) { + type = encodedAs.type; + } + + if ( + ignoreDiagnostics( + program.checker.isTypeAssignableTo( + type.projectionBase ?? type, + program.checker.getStdType("bytes"), + type + ) + ) + ) { + return ["application/octet-stream"]; + } else { + return ["text/plain"]; + } + } else if (type.kind === "Union") { + return [...type.variants.values()].flatMap((x) => resolve(x.type)); } else { - return "text/plain"; + return ["application/json"]; } - } else { - return "application/json"; } + + return [...new Set(resolve(type))]; } diff --git a/packages/http/src/types.ts b/packages/http/src/types.ts index c23c6ee53a..eceda526b1 100644 --- a/packages/http/src/types.ts +++ b/packages/http/src/types.ts @@ -484,6 +484,8 @@ export interface HttpOperationMultipartBody extends HttpOperationBodyBase { export interface HttpOperationPart { /** Part name */ readonly name?: string; + /** If the part is optional */ + readonly optional: boolean; /** Part body */ readonly body: HttpOperationBody; /** Part headers */ diff --git a/packages/http/test/multipart.test.ts b/packages/http/test/multipart.test.ts index 7aed57c270..b3fd83b4dc 100644 --- a/packages/http/test/multipart.test.ts +++ b/packages/http/test/multipart.test.ts @@ -159,19 +159,22 @@ describe("resolving part payload", () => { ["File", "*/*"], ["string", "text/plain"], ["int32", "text/plain"], + ["string[]", "application/json"], ["Foo", "application/json", `model Foo { value: string }`], ])("%s body", async (type, expectedContentType, extra?: string) => { const body = await getMultipartBody(` - op read( + op upload( @header contentType: "multipart/mixed", @multipartBody body: [ - HttpPart<${type}> + HttpPart<${type}>, + HttpPart<${type}>[] ]): void; ${extra ?? ""} `); - strictEqual(body.parts.length, 1); + strictEqual(body.parts.length, 2); deepStrictEqual(body.parts[0].body.contentTypes, [expectedContentType]); + deepStrictEqual(body.parts[1].body.contentTypes, [expectedContentType]); }); }); }); diff --git a/packages/http/test/responses.test.ts b/packages/http/test/responses.test.ts index 82bc9bab35..b8dc963da8 100644 --- a/packages/http/test/responses.test.ts +++ b/packages/http/test/responses.test.ts @@ -84,7 +84,7 @@ it("issues diagnostics for invalid content types", async () => { @route("/test1") @get - op test1(): { @header contentType: string, @body body: Foo }; + op test1(): { @header contentType: int32, @body body: Foo }; @route("/test2") @get op test2(): { @header contentType: 42, @body body: Foo }; diff --git a/packages/openapi3/src/openapi.ts b/packages/openapi3/src/openapi.ts index 5a445b6bf6..638160b125 100644 --- a/packages/openapi3/src/openapi.ts +++ b/packages/openapi3/src/openapi.ts @@ -96,7 +96,7 @@ import { buildVersionProjections, VersionProjections } from "@typespec/versionin import { stringify } from "yaml"; import { getRef } from "./decorators.js"; import { createDiagnostic, FileType, OpenAPI3EmitterOptions } from "./lib.js"; -import { getDefaultValue, OpenAPI3SchemaEmitter } from "./schema-emitter.js"; +import { getDefaultValue, isBytesKeptRaw, OpenAPI3SchemaEmitter } from "./schema-emitter.js"; import { OpenAPI3Document, OpenAPI3Encoding, @@ -1145,7 +1145,7 @@ function createOAPIEmitter( ), }; case "multipart": - return getBodyContentForMultipartBody(body, visibility); + return getBodyContentForMultipartBody(body, visibility, contentType); } } @@ -1166,30 +1166,55 @@ function createOAPIEmitter( function getBodyContentForMultipartBody( body: HttpOperationMultipartBody, - visibility: Visibility + visibility: Visibility, + contentType: string ): OpenAPI3MediaType { const properties: Record = {}; + const requiredProperties: string[] = []; const encodings: Record = {}; for (const [partIndex, part] of body.parts.entries()) { const partName = part.name ?? `part${partIndex}`; - const schema = getSchemaForSingleBody( - part.body.type, - visibility, - part.body.isExplicit && part.body.containsMetadataAnnotations, - "application/json" - ); + let schema = isBytesKeptRaw(program, part.body.type) + ? { type: "string", format: "binary" } + : getSchemaForSingleBody( + part.body.type, + visibility, + part.body.isExplicit && part.body.containsMetadataAnnotations, + part.body.type.kind === "Union" ? contentType : undefined + ); + + if (part.multi) { + schema = { + type: "array", + items: schema, + }; + } properties[partName] = schema; const encoding = resolveEncodingForMultipartPart(part, visibility, schema); if (encoding) { encodings[partName] = encoding; } + if (!part.optional) { + requiredProperties.push(partName); + } + } + + const schema: OpenAPI3Schema = { + type: "object", + properties, + required: requiredProperties, + }; + + const name = + "name" in body.type && body.type.name !== "" + ? getOpenAPITypeName(program, body.type, typeNameOptions) + : undefined; + if (name) { + root.components!.schemas![name] = schema; } const result: OpenAPI3MediaType = { - schema: { - type: "object", - properties, - }, + schema: name ? { $ref: "#/components/schemas/" + name } : schema, }; if (Object.keys(encodings).length > 0) { @@ -1202,7 +1227,7 @@ function createOAPIEmitter( part: HttpOperationPart, visibility: Visibility, schema: OpenAPI3Schema - ): OpenAPI3Encoding { + ): OpenAPI3Encoding | undefined { const encoding: OpenAPI3Encoding = {}; if (!isDefaultContentTypeForOpenAPI3(part.body.contentTypes, schema)) { encoding.contentType = part.body.contentTypes.join(", "); @@ -1216,6 +1241,9 @@ function createOAPIEmitter( } } } + if (Object.keys(encoding).length === 0) { + return undefined; + } return encoding; } @@ -1235,7 +1263,12 @@ function createOAPIEmitter( case "text/plain": return schema.type === "string" || schema.type === "number"; case "application/octet-stream": - return schema.type === "string" && schema.format === "binary"; + return ( + (schema.type === "string" && schema.format === "binary") || + (schema.type === "array" && + (schema.items as any)?.type === "string" && + (schema.items as any)?.format === "binary") + ); case "application/json": return schema.type === "object"; } diff --git a/packages/openapi3/src/schema-emitter.ts b/packages/openapi3/src/schema-emitter.ts index 3413b284c1..44f10ae4a5 100644 --- a/packages/openapi3/src/schema-emitter.ts +++ b/packages/openapi3/src/schema-emitter.ts @@ -325,24 +325,17 @@ export class OpenAPI3SchemaEmitter extends TypeEmitter< return props; } - #isBytesKeptRaw(type: Type) { - const program = this.emitter.getProgram(); - return ( - type.kind === "Scalar" && type.name === "bytes" && getEncode(program, type) === undefined - ); - } - modelPropertyLiteral(prop: ModelProperty): EmitterOutput { const program = this.emitter.getProgram(); const isMultipart = this.#getContentType().startsWith("multipart/"); if (isMultipart) { - if (this.#isBytesKeptRaw(prop.type) && getEncode(program, prop) === undefined) { + if (isBytesKeptRaw(program, prop.type) && getEncode(program, prop) === undefined) { return { type: "string", format: "binary" }; } if ( prop.type.kind === "Model" && isArrayModelType(program, prop.type) && - this.#isBytesKeptRaw(prop.type.indexer.value) + isBytesKeptRaw(program, prop.type.indexer.value) ) { return { type: "array", items: { type: "string", format: "binary" } }; } @@ -352,7 +345,7 @@ export class OpenAPI3SchemaEmitter extends TypeEmitter< referenceContext: isMultipart && (prop.type.kind !== "Union" || - ![...prop.type.variants.values()].some((x) => this.#isBytesKeptRaw(x.type))) + ![...prop.type.variants.values()].some((x) => isBytesKeptRaw(program, x.type))) ? { contentType: "application/json" } : {}, }); @@ -494,7 +487,7 @@ export class OpenAPI3SchemaEmitter extends TypeEmitter< continue; } - if (isMultipart && this.#isBytesKeptRaw(variant.type)) { + if (isMultipart && isBytesKeptRaw(program, variant.type)) { schemaMembers.push({ schema: { type: "string", format: "binary" }, type: variant.type }); continue; } @@ -1012,3 +1005,7 @@ export function getDefaultValue(program: Program, defaultType: Value): any { }); } } + +export function isBytesKeptRaw(program: Program, type: Type) { + return type.kind === "Scalar" && type.name === "bytes" && getEncode(program, type) === undefined; +} diff --git a/packages/openapi3/test/multipart.test.ts b/packages/openapi3/test/multipart.test.ts index 9301cb71d1..06394953e7 100644 --- a/packages/openapi3/test/multipart.test.ts +++ b/packages/openapi3/test/multipart.test.ts @@ -2,7 +2,166 @@ import { deepStrictEqual } from "assert"; import { describe, it } from "vitest"; import { openApiFor } from "./test-host.js"; -describe("typespec-autorest: multipart", () => { +it("create dedicated model for multipart", 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.requestBody.content["multipart/form-data"], { + schema: { + $ref: "#/components/schemas/Form", + }, + }); +}); + +it("part of type `bytes` produce `type: string, format: binary`", async () => { + const res = await openApiFor( + ` + op upload(@header contentType: "multipart/form-data", @multipartBody body: { profileImage: HttpPart }): void; + ` + ); + const op = res.paths["/"].post; + deepStrictEqual(op.requestBody.content["multipart/form-data"], { + schema: { + type: "object", + properties: { + profileImage: { + format: "binary", + type: "string", + }, + }, + required: ["profileImage"], + }, + }); +}); + +it("part of type union `HttpPart` produce `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.requestBody.content["multipart/form-data"], { + schema: { + type: "object", + properties: { + profileImage: { + anyOf: [ + { + type: "string", + format: "binary", + }, + { + type: "object", + properties: { + content: { type: "string", format: "byte" }, + }, + required: ["content"], + }, + ], + }, + }, + required: ["profileImage"], + }, + encoding: { + profileImage: { + contentType: "application/octet-stream, application/json", + }, + }, + }); +}); + +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.requestBody.content["multipart/form-data"], { + schema: { + type: "object", + properties: { + profileImage: { + type: "array", + items: { type: "string", format: "binary" }, + }, + }, + required: ["profileImage"], + }, + }); +}); + +it("part of type `string` produce `type: string`", async () => { + const res = await openApiFor( + ` + op upload(@header contentType: "multipart/form-data", @multipartBody _: { name: HttpPart }): void; + ` + ); + const op = res.paths["/"].post; + deepStrictEqual(op.requestBody.content["multipart/form-data"], { + schema: { + type: "object", + properties: { + name: { + type: "string", + }, + }, + required: ["name"], + }, + }); +}); + +it("part of type `object` produce an object", async () => { + const res = await openApiFor( + ` + op upload(@header contentType: "multipart/form-data", @multipartBody _: { address: HttpPart<{city: string, street: string}>}): void; + ` + ); + const op = res.paths["/"].post; + deepStrictEqual(op.requestBody.content["multipart/form-data"], { + schema: { + type: "object", + properties: { + address: { + type: "object", + properties: { + city: { + type: "string", + }, + street: { + type: "string", + }, + }, + required: ["city", "street"], + }, + }, + required: ["address"], + }, + }); +}); + +it("bytes inside a json part will be treated as base64 encoded by default(same as for a json body)", async () => { + const res = await openApiFor( + ` + op upload(@header contentType: "multipart/form-data", @multipartBody _: { address: HttpPart<{city: string, icon: bytes}> }): void; + ` + ); + const op = res.paths["/"].post; + deepStrictEqual( + op.requestBody.content["multipart/form-data"].schema.properties.address.properties.icon, + { + type: "string", + format: "byte", + } + ); +}); + +describe("legacy implicit form", () => { it("add MultiPart suffix to model name", async () => { const res = await openApiFor( ` diff --git a/vitest.workspace.ts b/vitest.workspace.ts index 28272b3823..86af1eea61 100644 --- a/vitest.workspace.ts +++ b/vitest.workspace.ts @@ -1,9 +1,11 @@ +import { defineConfig } from "vitest/config"; + export default ["packages/*/vitest.config.ts", "packages/*/vitest.config.mts"]; /** * Default Config For all TypeSpec projects using vitest. */ -export const defaultTypeSpecVitestConfig = { +export const defaultTypeSpecVitestConfig = defineConfig({ test: { environment: "node", isolate: false, @@ -14,5 +16,6 @@ export const defaultTypeSpecVitestConfig = { junit: "./test-results.xml", }, watchExclude: [], + exclude: ["node_modules", "dist/test"], }, -}; +}); From 506fa04a2c28716d76cf479bec75157d1ff119ed Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 14 May 2024 16:03:10 -0700 Subject: [PATCH 23/39] Create feature-multipart-v2-2024-4-14-22-58-52.md --- .../feature-multipart-v2-2024-4-14-22-58-52.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 .chronus/changes/feature-multipart-v2-2024-4-14-22-58-52.md diff --git a/.chronus/changes/feature-multipart-v2-2024-4-14-22-58-52.md b/.chronus/changes/feature-multipart-v2-2024-4-14-22-58-52.md new file mode 100644 index 0000000000..a082e13d3e --- /dev/null +++ b/.chronus/changes/feature-multipart-v2-2024-4-14-22-58-52.md @@ -0,0 +1,15 @@ +--- +# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking +changeKind: feature +packages: + - "@typespec/http" +--- + +Add new multipart handling. Using `@multipartBody` with `HttpPart`. See [multipart docs] for more information https://typespec.io/docs/next/libraries/http/multipart + + ```tsp + op upload(@header contentType: "multipart/mixed", @multipartBody body: { + name: HttpPart; + avatar: HttpPart[]; + }): void; + ``` From 62704d16ea0f8abc3348f3ec1cac0dfd34f74bd0 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 14 May 2024 16:17:07 -0700 Subject: [PATCH 24/39] Create feature-multipart-v2-2024-4-14-23-5-59.md --- .../changes/feature-multipart-v2-2024-4-14-23-5-59.md | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 .chronus/changes/feature-multipart-v2-2024-4-14-23-5-59.md diff --git a/.chronus/changes/feature-multipart-v2-2024-4-14-23-5-59.md b/.chronus/changes/feature-multipart-v2-2024-4-14-23-5-59.md new file mode 100644 index 0000000000..e84b41362f --- /dev/null +++ b/.chronus/changes/feature-multipart-v2-2024-4-14-23-5-59.md @@ -0,0 +1,8 @@ +--- +# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking +changeKind: fix +packages: + - "@typespec/openapi3" +--- + +Add support for new multipart constructs in http library From fee0142a898ed6820db2d130a9fde68fce7886bb Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 14 May 2024 16:20:21 -0700 Subject: [PATCH 25/39] ADd more tests --- packages/openapi3/src/openapi.ts | 5 ++- packages/openapi3/test/multipart.test.ts | 51 +++++++++++++++++++++++- 2 files changed, 53 insertions(+), 3 deletions(-) diff --git a/packages/openapi3/src/openapi.ts b/packages/openapi3/src/openapi.ts index 638160b125..d10be47bb3 100644 --- a/packages/openapi3/src/openapi.ts +++ b/packages/openapi3/src/openapi.ts @@ -1232,9 +1232,10 @@ function createOAPIEmitter( if (!isDefaultContentTypeForOpenAPI3(part.body.contentTypes, schema)) { encoding.contentType = part.body.contentTypes.join(", "); } - if (part.headers.length > 0) { + const headers = part.headers.filter((x) => !isContentTypeHeader(program, x.property)); + if (headers.length > 0) { encoding.headers = {}; - for (const header of part.headers) { + for (const header of headers) { const schema = getOpenAPIParameterBase(header.property, visibility); if (schema !== undefined) { encoding.headers[header.options.name] = schema; diff --git a/packages/openapi3/test/multipart.test.ts b/packages/openapi3/test/multipart.test.ts index 06394953e7..c7f8728b57 100644 --- a/packages/openapi3/test/multipart.test.ts +++ b/packages/openapi3/test/multipart.test.ts @@ -1,5 +1,6 @@ import { deepStrictEqual } from "assert"; -import { describe, it } from "vitest"; +import { describe, expect, it } from "vitest"; +import { OpenAPI3Encoding, OpenAPI3Schema } from "../src/types.js"; import { openApiFor } from "./test-host.js"; it("create dedicated model for multipart", async () => { @@ -161,6 +162,54 @@ it("bytes inside a json part will be treated as base64 encoded by default(same a ); }); +describe("part mapping", () => { + it.each([ + [`string`, { type: "string" }], + [`bytes`, { type: "string", format: "binary" }], + [`string[]`, { type: "array", items: { type: "string" } }, { contentType: "application/json" }], + [ + `bytes[]`, + { type: "array", items: { type: "string", format: "byte" } }, + { contentType: "application/json" }, + ], + [ + `{@header contentType: "image/png", @body image: bytes}`, + { type: "string", format: "binary" }, + { contentType: "image/png" }, + ], + [`File`, { type: "string", format: "binary" }, { contentType: "*/*" }], + [ + `{@header extra: string, @body body: string}`, + { type: "string" }, + { + headers: { + extra: { + required: true, + schema: { + type: "string", + }, + }, + }, + }, + ], + ] satisfies [string, OpenAPI3Schema, OpenAPI3Encoding?][])( + `HttpPart<%s>`, + async (type: string, expectedSchema: OpenAPI3Schema, expectedEncoding?: OpenAPI3Encoding) => { + const res = await openApiFor( + ` + op upload(@header contentType: "multipart/form-data", @multipartBody _: { part: HttpPart<${type}> }): void; + ` + ); + const content = res.paths["/"].post.requestBody.content["multipart/form-data"]; + expect(content.schema.properties.part).toEqual(expectedSchema); + + if (expectedEncoding || content.encoding?.part) { + expect(content.encoding?.part).toEqual(expectedEncoding); + } + } + ); +}); + describe("legacy implicit form", () => { it("add MultiPart suffix to model name", async () => { const res = await openApiFor( From aa429adb38ff5ff9e1f87aecf75ef3931c22e4de Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 14 May 2024 16:27:33 -0700 Subject: [PATCH 26/39] tweaks --- packages/rest/test/test-host.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/rest/test/test-host.ts b/packages/rest/test/test-host.ts index 23fca47899..64947cf3a5 100644 --- a/packages/rest/test/test-host.ts +++ b/packages/rest/test/test-host.ts @@ -71,7 +71,7 @@ export async function compileOperations( 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), From 8bccf0a4c1358d9ad12ffbdc854dd6fed78b090e Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 14 May 2024 16:27:54 -0700 Subject: [PATCH 27/39] Missing --- .chronus/changes/feature-multipart-v2-2024-4-14-16-27-48.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .chronus/changes/feature-multipart-v2-2024-4-14-16-27-48.md diff --git a/.chronus/changes/feature-multipart-v2-2024-4-14-16-27-48.md b/.chronus/changes/feature-multipart-v2-2024-4-14-16-27-48.md new file mode 100644 index 0000000000..cfd31b34e2 --- /dev/null +++ b/.chronus/changes/feature-multipart-v2-2024-4-14-16-27-48.md @@ -0,0 +1,6 @@ +--- +changeKind: internal +packages: + - "@typespec/rest" +--- + From f83b5c11a461b99bcd0a18ffa0d2adaddf6fe4bb Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 14 May 2024 16:30:19 -0700 Subject: [PATCH 28/39] Sample --- packages/samples/specs/multipart/main.tsp | 9 +++++---- .../multipart/@typespec/openapi3/openapi.yaml | 15 +++++++++------ 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/packages/samples/specs/multipart/main.tsp b/packages/samples/specs/multipart/main.tsp index c16fb063d5..5c8e1c3e50 100644 --- a/packages/samples/specs/multipart/main.tsp +++ b/packages/samples/specs/multipart/main.tsp @@ -3,9 +3,10 @@ import "@typespec/rest"; using TypeSpec.Http; model Data { - id: string; - profileImage: bytes; - address: Address; + id: HttpPart; + profileImage: HttpPart; + photos: HttpPart[]; + address: HttpPart
; } model Address { @@ -13,4 +14,4 @@ model Address { street: string; } -op basic(@header contentType: "multipart/form-data", @body body: Data): string; +op basic(@header contentType: "multipart/form-data", @multipartBody body: Data): string; diff --git a/packages/samples/test/output/multipart/@typespec/openapi3/openapi.yaml b/packages/samples/test/output/multipart/@typespec/openapi3/openapi.yaml index 08b373c5f8..ae6d151475 100644 --- a/packages/samples/test/output/multipart/@typespec/openapi3/openapi.yaml +++ b/packages/samples/test/output/multipart/@typespec/openapi3/openapi.yaml @@ -20,7 +20,10 @@ paths: content: multipart/form-data: schema: - $ref: '#/components/schemas/DataMultiPart' + $ref: '#/components/schemas/Data' + encoding: + address: + contentType: application/json components: schemas: Address: @@ -33,12 +36,8 @@ components: type: string street: type: string - DataMultiPart: + Data: type: object - required: - - id - - profileImage - - address properties: id: type: string @@ -47,3 +46,7 @@ components: format: binary address: $ref: '#/components/schemas/Address' + required: + - id + - profileImage + - address From 644edff6a971c79f337644268e284dd728e0e2ae Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 14 May 2024 16:33:02 -0700 Subject: [PATCH 29/39] Fix --- packages/http/src/index.ts | 2 +- packages/openapi3/src/openapi.ts | 4 ++++ packages/samples/specs/multipart/main.tsp | 8 ++++++-- .../output/multipart/@typespec/openapi3/openapi.yaml | 10 ++++++++++ 4 files changed, 21 insertions(+), 3 deletions(-) diff --git a/packages/http/src/index.ts b/packages/http/src/index.ts index 1ae4ce2da0..ee792cda39 100644 --- a/packages/http/src/index.ts +++ b/packages/http/src/index.ts @@ -7,7 +7,7 @@ export * from "./decorators.js"; export * from "./metadata.js"; export * from "./operations.js"; export * from "./parameters.js"; -export { isHttpFile } from "./private.decorators.js"; +export { getHttpFileModel, isHttpFile, isOrExtendsHttpFile } from "./private.decorators.js"; export * from "./responses.js"; export * from "./route.js"; export * from "./types.js"; diff --git a/packages/openapi3/src/openapi.ts b/packages/openapi3/src/openapi.ts index d10be47bb3..014d1c2734 100644 --- a/packages/openapi3/src/openapi.ts +++ b/packages/openapi3/src/openapi.ts @@ -73,6 +73,7 @@ import { HttpStatusCodesEntry, HttpVerb, isContentTypeHeader, + isOrExtendsHttpFile, isOverloadSameEndpoint, MetadataInfo, QueryParameterOptions, @@ -1583,6 +1584,9 @@ function createOAPIEmitter( function processUnreferencedSchemas() { const addSchema = (type: Type) => { + if (isOrExtendsHttpFile(program, type)) { + return; + } if ( visibilityUsage.isUnreachable(type) && !paramModels.has(type) && diff --git a/packages/samples/specs/multipart/main.tsp b/packages/samples/specs/multipart/main.tsp index 5c8e1c3e50..b6e3460837 100644 --- a/packages/samples/specs/multipart/main.tsp +++ b/packages/samples/specs/multipart/main.tsp @@ -2,10 +2,14 @@ import "@typespec/rest"; using TypeSpec.Http; +model Jpeg extends File { + contentType: "image/jpeg"; +} + model Data { id: HttpPart; - profileImage: HttpPart; - photos: HttpPart[]; + profileImage: HttpPart; + photos: HttpPart[]; address: HttpPart
; } diff --git a/packages/samples/test/output/multipart/@typespec/openapi3/openapi.yaml b/packages/samples/test/output/multipart/@typespec/openapi3/openapi.yaml index ae6d151475..786dee2741 100644 --- a/packages/samples/test/output/multipart/@typespec/openapi3/openapi.yaml +++ b/packages/samples/test/output/multipart/@typespec/openapi3/openapi.yaml @@ -22,6 +22,10 @@ paths: schema: $ref: '#/components/schemas/Data' encoding: + profileImage: + contentType: '*/*' + photos: + contentType: image/jpeg address: contentType: application/json components: @@ -44,9 +48,15 @@ components: profileImage: type: string format: binary + photos: + type: array + items: + type: string + format: binary address: $ref: '#/components/schemas/Address' required: - id - profileImage + - photos - address From d9a9ee89fc6909d658db4fe2085439826c53373a Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 14 May 2024 16:33:54 -0700 Subject: [PATCH 30/39] Fix deprecation --- packages/openapi3/src/openapi.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/openapi3/src/openapi.ts b/packages/openapi3/src/openapi.ts index 014d1c2734..d44a6c1c9f 100644 --- a/packages/openapi3/src/openapi.ts +++ b/packages/openapi3/src/openapi.ts @@ -65,7 +65,6 @@ import { HttpOperationParameter, HttpOperationParameters, HttpOperationPart, - HttpOperationRequestBody, HttpOperationResponse, HttpOperationResponseContent, HttpServer, @@ -533,8 +532,8 @@ function createOAPIEmitter( /** * Validates that common bodies are consistent and returns the minimal set that describes the differences. */ - function validateCommonBodies(ops: HttpOperation[]): HttpOperationRequestBody[] | undefined { - const allBodies = ops.map((op) => op.parameters.body) as HttpOperationRequestBody[]; + function validateCommonBodies(ops: HttpOperation[]): HttpOperationBody[] | undefined { + const allBodies = ops.map((op) => op.parameters.body) as HttpOperationBody[]; return [...new Set(allBodies)]; } @@ -594,7 +593,7 @@ function createOAPIEmitter( summary: string | undefined; verb: HttpVerb; parameters: HttpOperationParameters; - bodies: HttpOperationRequestBody[] | undefined; + bodies: HttpOperationBody[] | undefined; authentication?: Authentication; responses: Map; operations: Operation[]; From 2ef886c6d7bcb614b3438cd01be693ad402cd8ea Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 14 May 2024 17:39:21 -0700 Subject: [PATCH 31/39] fix types --- .../rest-metadata-emitter/rest-metadata-emitter-sample.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/samples/specs/rest-metadata-emitter/rest-metadata-emitter-sample.ts b/packages/samples/specs/rest-metadata-emitter/rest-metadata-emitter-sample.ts index 78a4d404a0..703986e01f 100644 --- a/packages/samples/specs/rest-metadata-emitter/rest-metadata-emitter-sample.ts +++ b/packages/samples/specs/rest-metadata-emitter/rest-metadata-emitter-sample.ts @@ -18,6 +18,7 @@ import { getVisibilitySuffix, HttpOperation, HttpOperationBody, + HttpOperationMultipartBody, HttpOperationResponse, resolveRequestVisibility, Visibility, @@ -247,7 +248,9 @@ export async function $onEmit(context: EmitContext): Promise { return remarks.length === 0 ? "" : ` (${remarks.join(", ")})`; } - function getContentTypeRemark(body: HttpOperationBody | undefined) { + function getContentTypeRemark( + body: HttpOperationBody | HttpOperationMultipartBody | undefined + ) { const ct = body?.contentTypes; if (!ct || ct.length === 0 || (ct.length === 1 && ct[0] === "application/json")) { return ""; From b69d897ede4b54f917526cfdd2d80d629833cacb Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 14 May 2024 19:09:41 -0700 Subject: [PATCH 32/39] Regen docs --- docs/libraries/http/reference/data-types.md | 43 +++++++++++++++++++++ docs/libraries/http/reference/decorators.md | 26 +++++++++++++ docs/libraries/http/reference/index.mdx | 4 ++ packages/http/README.md | 27 +++++++++++++ 4 files changed, 100 insertions(+) diff --git a/docs/libraries/http/reference/data-types.md b/docs/libraries/http/reference/data-types.md index 109a8f8373..dc8106701e 100644 --- a/docs/libraries/http/reference/data-types.md +++ b/docs/libraries/http/reference/data-types.md @@ -205,6 +205,20 @@ model TypeSpec.Http.CreatedResponse | ---------- | ----- | ---------------- | | statusCode | `201` | The status code. | +### `File` {#TypeSpec.Http.File} + +```typespec +model TypeSpec.Http.File +``` + +#### Properties + +| Name | Type | Description | +| ------------ | -------- | ----------- | +| contentType? | `string` | | +| filename? | `string` | | +| contents | `bytes` | | + ### `ForbiddenResponse` {#TypeSpec.Http.ForbiddenResponse} Access is forbidden. @@ -234,6 +248,35 @@ model TypeSpec.Http.HeaderOptions | name? | `string` | Name of the header when sent over HTTP. | | format? | `"csv" \| "multi" \| "tsv" \| "ssv" \| "pipes" \| "simple" \| "form"` | Determines the format of the array if type array is used. | +### `HttpPart` {#TypeSpec.Http.HttpPart} + +```typespec +model TypeSpec.Http.HttpPart +``` + +#### Template Parameters + +| Name | Description | +| ------- | ----------- | +| Type | | +| Options | | + +#### Properties + +None + +### `HttpPartOptions` {#TypeSpec.Http.HttpPartOptions} + +```typespec +model TypeSpec.Http.HttpPartOptions +``` + +#### Properties + +| Name | Type | Description | +| ----- | -------- | ------------------------------------------- | +| name? | `string` | Name of the part when using the array form. | + ### `ImplicitFlow` {#TypeSpec.Http.ImplicitFlow} Implicit flow diff --git a/docs/libraries/http/reference/decorators.md b/docs/libraries/http/reference/decorators.md index 24c42c8d45..3989e9f8e1 100644 --- a/docs/libraries/http/reference/decorators.md +++ b/docs/libraries/http/reference/decorators.md @@ -225,6 +225,32 @@ Specify if inapplicable metadata should be included in the payload for the given | ----- | ----------------- | --------------------------------------------------------------- | | value | `valueof boolean` | If true, inapplicable metadata will be included in the payload. | +### `@multipartBody` {#@TypeSpec.Http.multipartBody} + +```typespec +@TypeSpec.Http.multipartBody +``` + +#### Target + +`ModelProperty` + +#### Parameters + +None + +#### Examples + +```tsp +op upload( + @header `content-type`: "multipart/form-data", + @multipartBody body: { + fullName: HttpPart; + headShots: HttpPart[]; + }, +): void; +``` + ### `@patch` {#@TypeSpec.Http.patch} Specify the HTTP verb for the target operation to be `PATCH`. diff --git a/docs/libraries/http/reference/index.mdx b/docs/libraries/http/reference/index.mdx index ec9a88f52e..d85da81df6 100644 --- a/docs/libraries/http/reference/index.mdx +++ b/docs/libraries/http/reference/index.mdx @@ -43,6 +43,7 @@ npm install --save-peer @typespec/http - [`@head`](./decorators.md#@TypeSpec.Http.head) - [`@header`](./decorators.md#@TypeSpec.Http.header) - [`@includeInapplicableMetadataInPayload`](./decorators.md#@TypeSpec.Http.includeInapplicableMetadataInPayload) +- [`@multipartBody`](./decorators.md#@TypeSpec.Http.multipartBody) - [`@patch`](./decorators.md#@TypeSpec.Http.patch) - [`@path`](./decorators.md#@TypeSpec.Http.path) - [`@post`](./decorators.md#@TypeSpec.Http.post) @@ -66,8 +67,11 @@ npm install --save-peer @typespec/http - [`ClientCredentialsFlow`](./data-types.md#TypeSpec.Http.ClientCredentialsFlow) - [`ConflictResponse`](./data-types.md#TypeSpec.Http.ConflictResponse) - [`CreatedResponse`](./data-types.md#TypeSpec.Http.CreatedResponse) +- [`File`](./data-types.md#TypeSpec.Http.File) - [`ForbiddenResponse`](./data-types.md#TypeSpec.Http.ForbiddenResponse) - [`HeaderOptions`](./data-types.md#TypeSpec.Http.HeaderOptions) +- [`HttpPart`](./data-types.md#TypeSpec.Http.HttpPart) +- [`HttpPartOptions`](./data-types.md#TypeSpec.Http.HttpPartOptions) - [`ImplicitFlow`](./data-types.md#TypeSpec.Http.ImplicitFlow) - [`LocationHeader`](./data-types.md#TypeSpec.Http.LocationHeader) - [`MovedResponse`](./data-types.md#TypeSpec.Http.MovedResponse) diff --git a/packages/http/README.md b/packages/http/README.md index 95f35ff1d6..406a0cc762 100644 --- a/packages/http/README.md +++ b/packages/http/README.md @@ -44,6 +44,7 @@ Available ruleSets: - [`@head`](#@head) - [`@header`](#@header) - [`@includeInapplicableMetadataInPayload`](#@includeinapplicablemetadatainpayload) +- [`@multipartBody`](#@multipartbody) - [`@patch`](#@patch) - [`@path`](#@path) - [`@post`](#@post) @@ -272,6 +273,32 @@ Specify if inapplicable metadata should be included in the payload for the given | ----- | ----------------- | --------------------------------------------------------------- | | value | `valueof boolean` | If true, inapplicable metadata will be included in the payload. | +#### `@multipartBody` + +```typespec +@TypeSpec.Http.multipartBody +``` + +##### Target + +`ModelProperty` + +##### Parameters + +None + +##### Examples + +```tsp +op upload( + @header `content-type`: "multipart/form-data", + @multipartBody body: { + fullName: HttpPart; + headShots: HttpPart[]; + }, +): void; +``` + #### `@patch` Specify the HTTP verb for the target operation to be `PATCH`. From a63a6f616c3a90e28f6ccb8ffddcb6642962b000 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Wed, 22 May 2024 07:33:36 -0700 Subject: [PATCH 33/39] regen docs --- packages/http/generated-defs/TypeSpec.Http.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/http/generated-defs/TypeSpec.Http.ts b/packages/http/generated-defs/TypeSpec.Http.ts index 23d43caa85..35e2319831 100644 --- a/packages/http/generated-defs/TypeSpec.Http.ts +++ b/packages/http/generated-defs/TypeSpec.Http.ts @@ -118,11 +118,11 @@ export type BodyIgnoreDecorator = (context: DecoratorContext, target: ModelPrope * @example * ```tsp * op upload( - * @header `content-type`: "multipart/form-data", - * @multipartBody body: { - * fullName: HttpPart, - * headShots: HttpPart[] - * } + * @header `content-type`: "multipart/form-data", + * @multipartBody body: { + * fullName: HttpPart, + * headShots: HttpPart[] + * } * ): void; * ``` */ From 54b2abee8abb1740b74a86278372fc6012baaf73 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Wed, 22 May 2024 08:59:47 -0700 Subject: [PATCH 34/39] Fix --- packages/http/src/types.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/http/src/types.ts b/packages/http/src/types.ts index 11a55616c0..3c9e2bb7b3 100644 --- a/packages/http/src/types.ts +++ b/packages/http/src/types.ts @@ -457,7 +457,7 @@ export interface HttpBody { readonly containsMetadataAnnotations: boolean; /** - * @deprecated use {@link parameter} + * @deprecated use {@link property} */ parameter?: ModelProperty; From f999aaf622a84ae00db80b6262a266c139cc0aee Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Wed, 22 May 2024 09:08:56 -0700 Subject: [PATCH 35/39] Split contenttype into its own property --- packages/http/src/content-types.ts | 1 + packages/http/src/http-property.ts | 12 +++++++++++- packages/http/src/payload.ts | 4 ++-- packages/http/src/responses.ts | 3 +-- packages/openapi3/src/openapi.ts | 2 +- 5 files changed, 16 insertions(+), 6 deletions(-) diff --git a/packages/http/src/content-types.ts b/packages/http/src/content-types.ts index 2f43478de3..f7a8751a7a 100644 --- a/packages/http/src/content-types.ts +++ b/packages/http/src/content-types.ts @@ -3,6 +3,7 @@ import { getHeaderFieldName } from "./decorators.js"; import { createDiagnostic } from "./lib.js"; /** + * @deprecated Use `OperationProperty.kind === 'contentType'` instead. * Check if the given model property is the content type header. * @param program Program * @param property Model property. diff --git a/packages/http/src/http-property.ts b/packages/http/src/http-property.ts index c8fa2b203f..fffa6b4047 100644 --- a/packages/http/src/http-property.ts +++ b/packages/http/src/http-property.ts @@ -25,6 +25,7 @@ import { HeaderFieldOptions, PathParameterOptions, QueryParameterOptions } from export type HttpProperty = | HeaderProperty + | ContentTypeProperty | QueryProperty | PathProperty | StatusCodeProperty @@ -41,6 +42,11 @@ export interface HeaderProperty extends HttpPropertyBase { readonly kind: "header"; readonly options: HeaderFieldOptions; } + +export interface ContentTypeProperty extends HttpPropertyBase { + readonly kind: "contentType"; +} + export interface QueryProperty extends HttpPropertyBase { readonly kind: "query"; readonly options: QueryParameterOptions; @@ -115,7 +121,11 @@ export function getHttpProperty( } if (annotations.header) { - return createResult({ kind: "header", options: annotations.header, property }); + if (annotations.header.name.toLowerCase() === "content-type") { + return createResult({ kind: "contentType", property }); + } else { + return createResult({ kind: "header", options: annotations.header, property }); + } } else if (annotations.query) { return createResult({ kind: "query", options: annotations.query, property }); } else if (annotations.path) { diff --git a/packages/http/src/payload.ts b/packages/http/src/payload.ts index 7ac2dab1de..07f2bf7b02 100644 --- a/packages/http/src/payload.ts +++ b/packages/http/src/payload.ts @@ -14,7 +14,7 @@ import { navigateType, } from "@typespec/compiler"; import { DuplicateTracker } from "@typespec/compiler/utils"; -import { getContentTypes, isContentTypeHeader } from "./content-types.js"; +import { getContentTypes } from "./content-types.js"; import { isHeader, isPathParam, isQueryParam, isStatusCode } from "./decorators.js"; import { GetHttpPropertyOptions, @@ -172,7 +172,7 @@ function resolveContentTypes( usedIn: "request" | "response" | "multipart" ): [{ contentTypes: string[]; contentTypeProperty?: ModelProperty }, readonly Diagnostic[]] { for (const prop of metadata) { - if (prop.kind === "header" && isContentTypeHeader(program, prop.property)) { + if (prop.kind === "contentType") { const [contentTypes, diagnostics] = getContentTypes(prop.property); return [{ contentTypes, contentTypeProperty: prop.property }, diagnostics]; } diff --git a/packages/http/src/responses.ts b/packages/http/src/responses.ts index 7510ad701d..6e2d952d55 100644 --- a/packages/http/src/responses.ts +++ b/packages/http/src/responses.ts @@ -14,7 +14,6 @@ import { Program, Type, } from "@typespec/compiler"; -import { isContentTypeHeader } from "./content-types.js"; import { getStatusCodeDescription, getStatusCodesWithDiagnostics } from "./decorators.js"; import { HttpProperty } from "./http-property.js"; import { HttpStateKeys, reportDiagnostic } from "./lib.js"; @@ -195,7 +194,7 @@ function getResponseHeaders( ): Record { const responseHeaders: Record = {}; for (const prop of metadata) { - if (prop.kind === "header" && !isContentTypeHeader(program, prop.property)) { + if (prop.kind === "header") { responseHeaders[prop.options.name] = prop.property; } } diff --git a/packages/openapi3/src/openapi.ts b/packages/openapi3/src/openapi.ts index d44a6c1c9f..6922ab3aef 100644 --- a/packages/openapi3/src/openapi.ts +++ b/packages/openapi3/src/openapi.ts @@ -1232,7 +1232,7 @@ function createOAPIEmitter( if (!isDefaultContentTypeForOpenAPI3(part.body.contentTypes, schema)) { encoding.contentType = part.body.contentTypes.join(", "); } - const headers = part.headers.filter((x) => !isContentTypeHeader(program, x.property)); + const headers = part.headers; if (headers.length > 0) { encoding.headers = {}; for (const header of headers) { From 3f6193bb46d6a5ac998f8f24610d9858a5ea88d0 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Thu, 23 May 2024 14:27:32 -0700 Subject: [PATCH 36/39] fix --- packages/http/src/parameters.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/http/src/parameters.ts b/packages/http/src/parameters.ts index ccaec0d2e4..d3aa0d9430 100644 --- a/packages/http/src/parameters.ts +++ b/packages/http/src/parameters.ts @@ -65,6 +65,13 @@ function getOperationParametersForVerb( for (const item of metadata) { switch (item.kind) { + case "contentType": + parameters.push({ + name: "content-type", + type: "header", + param: item.property, + }); + break; case "path": if (item.property.optional) { diagnostics.add( From e6aa61c310c5ccfc2ded8870d98dc9714a309d1e Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Fri, 24 May 2024 11:33:14 -0700 Subject: [PATCH 37/39] fix --- packages/http/src/parameters.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/http/src/parameters.ts b/packages/http/src/parameters.ts index d3aa0d9430..9486c10193 100644 --- a/packages/http/src/parameters.ts +++ b/packages/http/src/parameters.ts @@ -67,7 +67,7 @@ function getOperationParametersForVerb( switch (item.kind) { case "contentType": parameters.push({ - name: "content-type", + name: "Content-Type", type: "header", param: item.property, }); From 23a6709abc940cb281013cfcb48949e1480ca771 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Mon, 3 Jun 2024 08:59:17 -0700 Subject: [PATCH 38/39] Fix deprecatged parmaeter not set --- packages/http/src/payload.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/http/src/payload.ts b/packages/http/src/payload.ts index 07f2bf7b02..ffa19ae35d 100644 --- a/packages/http/src/payload.ts +++ b/packages/http/src/payload.ts @@ -218,6 +218,7 @@ function resolveExplicitBodyProperty( isExplicit: item.kind === "body", containsMetadataAnnotations, property: item.property, + parameter: item.property, }; } break; From 0a08dbcc5de2f7db8f4d9eb9bc2816f77e3e70d1 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Mon, 3 Jun 2024 12:52:39 -0700 Subject: [PATCH 39/39] fix --- packages/http/test/multipart.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/http/test/multipart.test.ts b/packages/http/test/multipart.test.ts index b3fd83b4dc..1653a90355 100644 --- a/packages/http/test/multipart.test.ts +++ b/packages/http/test/multipart.test.ts @@ -143,7 +143,7 @@ describe("resolving part payload", () => { `); strictEqual(op.parameters.parameters.length, 2); - strictEqual(op.parameters.parameters[0].name, "content-type"); + strictEqual(op.parameters.parameters[0].name, "Content-Type"); strictEqual(op.parameters.parameters[1].name, "operation-header"); const body = op.parameters.body;