Skip to content

Commit

Permalink
Handle bytes part correctly in multipart request (#2702)
Browse files Browse the repository at this point in the history 
fix [#2370](#2370)

---------

Co-authored-by: Mark Cowlishaw <[email protected]>
  • Loading branch information
@timotheeguerin @markcowl
timotheeguerin and markcowl authored Nov 30, 2023
1 parent 555eadd commit 9e167ad
Show file tree
Hide file tree
Showing 6 changed files with 256 additions and 81 deletions.
10 changes: 10 additions & 0 deletions common/changes/@typespec/openapi3/basic-multipart-support-openapi3_2023-11-27-23-05.json
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@typespec/openapi3",
"comment": "Handle `bytes` as a multipart part type correctly and produce `type: string, format: binary`",
"type": "none"
}
],
"packageName": "@typespec/openapi3"
}
114 changes: 44 additions & 70 deletions packages/openapi3/src/openapi.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import {
interpolatePath,
isArrayModelType,
isDeprecated,
isErrorType,
isGlobalNamespace,
isNeverType,
isNullType,
Expand Down Expand Up @@ -179,10 +178,6 @@ function createOAPIEmitter(
let metadataInfo: MetadataInfo;
let visibilityUsage: VisibilityUsageTracker;

// Keep track of inline types still in the process of having their schema computed
// This is used to detect cycles in inline types, which is an
let inProgressInlineTypes = new Set<Type>();

// Map model properties that represent shared parameters to their parameter
// definition that will go in #/components/parameters. Inlined parameters do not go in
// this map.
Expand Down Expand Up @@ -255,7 +250,6 @@ function createOAPIEmitter(
serviceNamespace = getNamespaceFullName(service.type);
currentPath = root.paths;

inProgressInlineTypes = new Set();
params = new Map();
paramModels = new Set();
tags = new Set();
Expand Down Expand Up @@ -312,9 +306,11 @@ function createOAPIEmitter(
};

if (prop.type.kind === "Enum") {
variable.enum = getSchemaValue(prop.type, Visibility.Read).enum as any;
variable.enum = getSchemaValue(prop.type, Visibility.Read, "application/json")
.enum as any;
} else if (prop.type.kind === "Union") {
variable.enum = getSchemaValue(prop.type, Visibility.Read).enum as any;
variable.enum = getSchemaValue(prop.type, Visibility.Read, "application/json")
.enum as any;
} else if (prop.type.kind === "String") {
variable.enum = [prop.type.value];
}
Expand Down Expand Up @@ -878,7 +874,7 @@ function createOAPIEmitter(
const isBinary = isBinaryPayload(data.body.type, contentType);
const schema = isBinary
? { type: "string", format: "binary" }
: getSchemaOrRef(data.body.type, Visibility.Read);
: getSchemaForBody(data.body.type, Visibility.Read, undefined);
if (schemaMap.has(contentType)) {
schemaMap.get(contentType)!.push(schema);
} else {
Expand Down Expand Up @@ -908,8 +904,12 @@ function createOAPIEmitter(
return getOpenAPIParameterBase(prop, Visibility.Read);
}

function callSchemaEmitter(type: Type, visibility: Visibility): Refable<OpenAPI3Schema> {
const result = emitTypeWithSchemaEmitter(type, visibility);
function callSchemaEmitter(
type: Type,
visibility: Visibility,
contentType?: string
): Refable<OpenAPI3Schema> {
const result = emitTypeWithSchemaEmitter(type, visibility, contentType);

switch (result.kind) {
case "code":
Expand All @@ -928,8 +928,8 @@ function createOAPIEmitter(
}
}

function getSchemaValue(type: Type, visibility: Visibility): OpenAPI3Schema {
const result = emitTypeWithSchemaEmitter(type, visibility);
function getSchemaValue(type: Type, visibility: Visibility, contentType: string): OpenAPI3Schema {
const result = emitTypeWithSchemaEmitter(type, visibility, contentType);

switch (result.kind) {
case "code":
Expand All @@ -949,63 +949,25 @@ function createOAPIEmitter(

function emitTypeWithSchemaEmitter(
type: Type,
visibility: Visibility
visibility: Visibility,
contentType?: string
): EmitEntity<OpenAPI3Schema> {
if (!metadataInfo.isTransformed(type, visibility)) {
visibility = Visibility.Read;
}
contentType = contentType === "application/json" ? undefined : contentType;
return schemaEmitter.emitType(type, {
referenceContext: { visibility, serviceNamespaceName: serviceNamespace },
referenceContext: { visibility, serviceNamespaceName: serviceNamespace, contentType },
}) as any;
}

function getSchemaOrRef(type: Type, visibility: Visibility): any {
if (
(type.kind === "Scalar" && program.checker.isStdType(type)) ||
type.kind === "String" ||
type.kind === "Number" ||
type.kind === "Boolean" ||
(type.kind === "Intrinsic" && type.name === "unknown") ||
type.kind === "EnumMember" ||
type.kind === "ModelProperty"
) {
// Those types should just be inlined.
return callSchemaEmitter(type, visibility);
}

type = metadataInfo.getEffectivePayloadType(type, visibility);

const name = getOpenAPITypeName(program, type, typeNameOptions);
if (shouldInline(program, type)) {
const schema = getSchemaForInlineType(type, visibility, name);

if (schema === undefined && isErrorType(type)) {
// Exit early so that syntax errors are exposed. This error will
// be caught and handled in emitOpenAPI.
throw new ErrorTypeFoundError();
}

return schema;
} else {
// Use shared schema when type is not transformed by visibility from the canonical read visibility.
if (!metadataInfo.isTransformed(type, visibility)) {
visibility = Visibility.Read;
}

return callSchemaEmitter(type, visibility);
}
}

function getSchemaForInlineType(type: Type, visibility: Visibility, name: string) {
if (inProgressInlineTypes.has(type)) {
reportDiagnostic(program, {
code: "inline-cycle",
format: { type: name },
target: type,
});
return {};
}
inProgressInlineTypes.add(type);
const schema = getSchemaForType(type, visibility);
inProgressInlineTypes.delete(type);
return schema;
function getSchemaForBody(
type: Type,
visibility: Visibility,
multipart: string | undefined
): any {
const effectiveType = metadataInfo.getEffectivePayloadType(type, visibility);
return callSchemaEmitter(effectiveType, visibility, multipart ?? "application/json");
}

function getParamPlaceholder(property: ModelProperty) {
Expand Down Expand Up @@ -1080,7 +1042,11 @@ function createOAPIEmitter(
const isBinary = isBinaryPayload(body.type, contentType);
const bodySchema = isBinary
? { type: "string", format: "binary" }
: getSchemaOrRef(body.type, visibility);
: getSchemaForBody(
body.type,
visibility,
contentType.startsWith("multipart/") ? contentType : undefined
);
if (schemaMap.has(contentType)) {
schemaMap.get(contentType)!.push(bodySchema);
} else {
Expand Down Expand Up @@ -1118,7 +1084,11 @@ function createOAPIEmitter(
const isBinary = isBinaryPayload(body.type, contentType);
const bodySchema = isBinary
? { type: "string", format: "binary" }
: getSchemaOrRef(body.type, visibility);
: getSchemaForBody(
body.type,
visibility,
contentType.startsWith("multipart/") ? contentType : undefined
);
const contentEntry: any = {
schema: bodySchema,
};
Expand Down Expand Up @@ -1325,7 +1295,7 @@ function createOAPIEmitter(
!paramModels.has(type) &&
!shouldInline(program, type)
) {
getSchemaOrRef(type, Visibility.Read);
callSchemaEmitter(type, Visibility.Read);
}
};
const skipSubNamespaces = isGlobalNamespace(program, serviceNamespace);
Expand Down Expand Up @@ -1476,7 +1446,7 @@ function createOAPIEmitter(
const values = getKnownValues(program, typespecType as any);
if (values) {
return {
oneOf: [newTarget, callSchemaEmitter(values, Visibility.Read)],
oneOf: [newTarget, callSchemaEmitter(values, Visibility.Read, "application/json")],
};
}

Expand All @@ -1492,7 +1462,11 @@ function createOAPIEmitter(
const encodeData = getEncode(program, typespecType);
if (encodeData) {
const newTarget = { ...target };
const newType = callSchemaEmitter(encodeData.type, Visibility.Read) as OpenAPI3Schema;
const newType = callSchemaEmitter(
encodeData.type,
Visibility.Read,
"application/json"
) as OpenAPI3Schema;
newTarget.type = newType.type;
// If the target already has a format it takes priority. (e.g. int32)
newTarget.format = mergeFormatAndEncoding(
Expand Down
42 changes: 31 additions & 11 deletions packages/openapi3/src/schema-emitter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,34 +95,38 @@ export class OpenAPI3SchemaEmitter extends TypeEmitter<
}

modelDeclarationReferenceContext(model: Model, name: string): Context {
return this.#reduceVisibilityContext(model);
return this.reduceContext(model);
}

modelLiteralReferenceContext(model: Model): Context {
return this.#reduceVisibilityContext(model);
return this.reduceContext(model);
}

scalarDeclarationReferenceContext(scalar: Scalar, name: string): Context {
return this.#reduceVisibilityContext(scalar);
return this.reduceContext(scalar);
}

enumDeclarationReferenceContext(en: Enum, name: string): Context {
return this.#reduceVisibilityContext(en);
return this.reduceContext(en);
}

unionDeclarationReferenceContext(union: Union): Context {
return this.#reduceVisibilityContext(union);
return this.reduceContext(union);
}

#reduceVisibilityContext(type: Type): Context {
reduceContext(type: Type): Context {
const visibility = this.#getVisibilityContext();
const patch: Record<string, any> = {};
if (visibility !== Visibility.Read && !this.#metadataInfo.isTransformed(type, visibility)) {
return {
visibility: Visibility.Read,
};
patch.visibility = Visibility.Read;
}
const contentType = this.#getContentType();

if (contentType === "application/json") {
delete patch.contentType;
}

return {};
return patch;
}

modelDeclaration(model: Model, _: string): EmitterOutput<object> {
Expand Down Expand Up @@ -163,7 +167,9 @@ export class OpenAPI3SchemaEmitter extends TypeEmitter<
}

const baseName = getOpenAPITypeName(program, model, this.#typeNameOptions());
return this.#createDeclaration(model, baseName, this.#applyConstraints(model, schema));
const isMultipart = this.#getContentType().startsWith("multipart/");
const name = isMultipart ? baseName + "MultiPart" : baseName;
return this.#createDeclaration(model, name, this.#applyConstraints(model, schema));
}

#applyExternalDocs(typespecType: Type, target: Record<string, unknown>) {
Expand All @@ -188,6 +194,10 @@ export class OpenAPI3SchemaEmitter extends TypeEmitter<
return this.emitter.getContext().visibility ?? Visibility.Read;
}

#getContentType(): string {
return this.emitter.getContext().contentType ?? "application/json";
}

modelLiteral(model: Model): EmitterOutput<object> {
const schema = new ObjectBuilder({
type: "object",
Expand Down Expand Up @@ -295,6 +305,16 @@ export class OpenAPI3SchemaEmitter extends TypeEmitter<

modelPropertyLiteral(prop: ModelProperty): EmitterOutput<object> {
const program = this.emitter.getProgram();
const isMultipart = this.#getContentType().startsWith("multipart/");
if (
isMultipart &&
prop.type.kind === "Scalar" &&
prop.type.name === "bytes" &&
getEncode(program, prop.type) === undefined &&
getEncode(program, prop) === undefined
) {
return { type: "string", format: "binary" };
}

const refSchema = this.emitter.emitTypeReference(prop.type);
if (refSchema.kind !== "code") {
Expand Down
Loading

0 comments on commit 9e167ad

Please sign in to comment.