diff --git a/.changeset/fix-openapi-response-representations.md b/.changeset/fix-openapi-response-representations.md new file mode 100644 index 00000000000..8e28046347b --- /dev/null +++ b/.changeset/fix-openapi-response-representations.md @@ -0,0 +1,5 @@ +--- +"@effect/openapi-generator": patch +--- + +Preserve JSON-compatible response representations per status and decode successful binary responses in generated HTTP clients. diff --git a/packages/tools/openapi-generator/src/OpenApiGenerator.ts b/packages/tools/openapi-generator/src/OpenApiGenerator.ts index d447a0452d3..32e104eeec7 100644 --- a/packages/tools/openapi-generator/src/OpenApiGenerator.ts +++ b/packages/tools/openapi-generator/src/OpenApiGenerator.ts @@ -493,22 +493,31 @@ const parseOpenApi = ( } const representable: Array = [] + const jsonResponseEntries = findContentEntriesByMediaType(content, isJsonMediaType) let jsonSchemaName: string | undefined - const jsonResponseSchema = content?.["application/json"]?.schema - if (Predicate.isNotUndefined(jsonResponseSchema)) { + if (jsonResponseEntries.length > 0) { + const jsonResponseSchema = combineJsonResponseSchemas(jsonResponseEntries) jsonSchemaName = addSchema(`${schemaId}${status}`, jsonResponseSchema, op) + if (isHttpApi) { - representable.push({ - contentType: "application/json", - encoding: "json", - schema: jsonSchemaName - }) + const multipleJsonRepresentations = jsonResponseEntries.length > 1 + for (const [contentType, mediaType] of jsonResponseEntries) { + const schema = mediaType.schema as JsonSchema.JsonSchema + const schemaName = multipleJsonRepresentations + ? addSchema(`${schemaId}${status}${mediaTypeToSuffix(contentType)}`, schema, op) + : jsonSchemaName + representable.push({ + contentType, + encoding: "json", + schema: schemaName + }) + } } } if (isHttpApi) { for (const [contentType, mediaType] of Object.entries(content ?? {})) { - if (contentType === "application/json") { + if (isJsonMediaType(contentType.toLowerCase())) { continue } if (!Predicate.isObject(mediaType)) { @@ -612,10 +621,20 @@ const parseOpenApi = ( } } - if (Predicate.isNotUndefined(content?.["application/octet-stream"])) { - const statusMajorNumber = Number(parsedStatus[0]) - if (!Number.isNaN(statusMajorNumber) && statusMajorNumber < 4) { - op.binaryResponse = true + if (Predicate.isNotUndefined(content)) { + for (const [contentType, mediaType] of Object.entries(content)) { + const schema = Predicate.isObject(mediaType) ? mediaType.schema : undefined + const isBinary = isBinaryMediaType(contentType.toLowerCase()) || + (Predicate.isObject(schema) && schema.type === "string" && schema.format === "binary") + if (!isBinary) { + continue + } + const statusMajorNumber = Number(parsedStatus[0]) + if (!Number.isNaN(statusMajorNumber) && statusMajorNumber < 4) { + op.binaryResponse = true + op.binarySuccessStatuses.add(parsedStatus.toLowerCase()) + } + break } } @@ -912,6 +931,38 @@ const isMultipartBinaryFiles = (value: Record, singleFileRef: s return isMultipartBinaryFile(items) || (Predicate.isObject(items) && items.$ref === singleFileRef) } +const combineJsonResponseSchemas = ( + entries: ReadonlyArray +): JsonSchema.JsonSchema => { + const schemas = entries.map(([, mediaType]) => mediaType.schema as JsonSchema.JsonSchema) + return schemas.length === 1 + ? schemas[0] + : { + anyOf: schemas + } +} + +const findContentEntriesByMediaType = ( + content: Record | undefined, + predicate: (contentType: string) => boolean +): Array => { + if (Predicate.isUndefined(content)) { + return [] + } + + const matches: Array = [] + for (const [contentType, mediaType] of Object.entries(content)) { + if ( + predicate(contentType.toLowerCase()) && + Predicate.isObject(mediaType) && + Predicate.isNotUndefined(mediaType.schema) + ) { + matches.push([contentType, mediaType as { readonly schema: unknown }]) + } + } + return matches +} + const isJsonMediaType = (contentType: string): boolean => contentType === "application/json" || (contentType.startsWith("application/") && contentType.endsWith("+json")) @@ -920,6 +971,12 @@ const isTextMediaType = (contentType: string): boolean => contentType.startsWith const isBinaryMediaType = (contentType: string): boolean => contentType === "application/octet-stream" || + contentType === "application/zip" || + contentType === "application/gzip" || + contentType === "application/pdf" || + contentType.startsWith("image/") || + contentType.startsWith("audio/") || + contentType.startsWith("video/") || (contentType.startsWith("application/") && (contentType.includes("binary") || contentType.endsWith("+octet-stream"))) const getRequestMediaTypeEncoding = ( diff --git a/packages/tools/openapi-generator/src/OpenApiTransformer.ts b/packages/tools/openapi-generator/src/OpenApiTransformer.ts index e511a64072a..c5e701c0d8c 100644 --- a/packages/tools/openapi-generator/src/OpenApiTransformer.ts +++ b/packages/tools/openapi-generator/src/OpenApiTransformer.ts @@ -41,11 +41,13 @@ export class OpenApiTransformer extends Context.Service< interface ImportRequirements { readonly eventStream: boolean readonly octetStream: boolean + readonly binarySuccess: boolean } const computeImportRequirements = (operations: ReadonlyArray): ImportRequirements => { let eventStream = false let octetStream = false + let binarySuccess = false for (const op of operations) { if (op.sseSchema) { eventStream = true @@ -53,8 +55,11 @@ const computeImportRequirements = (operations: ReadonlyArray): if (op.binaryResponse) { octetStream = true } + if (op.binarySuccessStatuses.size > 0) { + binarySuccess = true + } } - return { eventStream, octetStream } + return { eventStream, octetStream, binarySuccess } } const requiresStreaming = (requirements: ImportRequirements): boolean => @@ -124,10 +129,13 @@ ${clientErrorSource(name)}` } let success = "void" - if (operation.successSchemas.size > 0) { - success = Array.from(operation.successSchemas.values()) - .map((schema) => `typeof ${schema}.Type`) - .join(" | ") + const successTypes = Array.from(operation.successSchemas.values()) + .map((schema) => `typeof ${schema}.Type`) + if (operation.binarySuccessStatuses.size > 0) { + successTypes.push("Uint8Array") + } + if (successTypes.length > 0) { + success = successTypes.join(" | ") } const errors = ["HttpClientError.HttpClientError", "SchemaError"] if (operation.errorSchemas.size > 0) { @@ -232,6 +240,9 @@ ${clientErrorSource(name)}` if (requirements.octetStream) { helpers.push(binaryRequestSource) } + if (requirements.binarySuccess) { + helpers.push(decodeBinarySource) + } return `export interface OperationConfig { /** @@ -318,6 +329,11 @@ export const make = ( const statusCode = singleSuccessCode && status.startsWith("2") ? "2xx" : status decodes.push(`"${statusCode}": decodeSuccess(${schema})`) }) + const singleBinaryCode = operation.binarySuccessStatuses.size === 1 && operation.successSchemas.size === 0 + operation.binarySuccessStatuses.forEach((status) => { + const statusCode = singleBinaryCode && status.startsWith("2") ? "2xx" : status + decodes.push(`"${statusCode}": decodeBinary`) + }) operation.errorSchemas.forEach((schema, status) => { decodes.push(`"${status}": decodeError("${schema}", ${schema})`) }) @@ -533,8 +549,12 @@ ${clientErrorSource(name)}` } let success = "void" - if (operation.successSchemas.size > 0) { - success = Array.from(operation.successSchemas.values()).join(" | ") + const successTypes = Array.from(operation.successSchemas.values()) + if (operation.binarySuccessStatuses.size > 0) { + successTypes.push("Uint8Array") + } + if (successTypes.length > 0) { + success = successTypes.join(" | ") } const errors = ["HttpClientError.HttpClientError"] @@ -620,7 +640,7 @@ ${clientErrorSource(name)}` const requirements = computeImportRequirements(operations) const implMethods: Array = [] for (const op of operations) { - implMethods.push(operationToImpl(op)) + implMethods.push(operationToImpl(op, requirements.binarySuccess)) if (op.sseSchema) { implMethods.push(operationToSseImpl(op)) } @@ -636,6 +656,9 @@ ${clientErrorSource(name)}` if (requirements.octetStream) { helpers.push(binaryRequestSourceTs) } + if (requirements.binarySuccess) { + helpers.push(decodeBinarySourceTs) + } return `export interface OperationConfig { /** @@ -683,19 +706,33 @@ export const make = ( (cause) => Effect.fail(${name}Error(tag, cause, response)), ) const onRequest = (config: Config | undefined) => ( - successCodes: ReadonlyArray, + successCodes: ReadonlyArray,${ + requirements.binarySuccess + ? ` + binarySuccessCodes?: ReadonlyArray,` + : "" + } errorCodes?: Record, ) => { const cases: any = { orElse: unexpectedStatus } for (const code of successCodes) { cases[code] = decodeSuccess + }${ + requirements.binarySuccess + ? ` + if (binarySuccessCodes) { + for (const code of binarySuccessCodes) { + cases[code] = decodeBinary + } + }` + : "" } if (errorCodes) { for (const [code, tag] of Object.entries(errorCodes)) { cases[code] = decodeError(tag) } } - if (successCodes.length === 0) { + if (successCodes.length === 0${requirements.binarySuccess ? " && (binarySuccessCodes?.length ?? 0) === 0" : ""}) { cases["2xx"] = decodeVoid } return withResponse(config)(HttpClientResponse.matchStatus(cases) as any) @@ -707,7 +744,7 @@ export const make = ( }` } - const operationToImpl = (operation: ParsedOperation) => { + const operationToImpl = (operation: ParsedOperation, includeBinaryArgument: boolean) => { const args: Array = [...operation.pathIds, "options"] const params = `${args.join(", ")}` @@ -738,18 +775,27 @@ export const make = ( } const successCodesRaw = Array.from(operation.successSchemas.keys()) + const binarySuccessCodesRaw = Array.from(operation.binarySuccessStatuses) const successCodes = successCodesRaw .map((_) => JSON.stringify(_)) .join(", ") - const singleSuccessCode = successCodesRaw.length === 1 && successCodesRaw[0].startsWith("2") + const binarySuccessCodes = binarySuccessCodesRaw + .map((_) => JSON.stringify(_)) + .join(", ") + const singleSuccessCode = successCodesRaw.length === 1 && + binarySuccessCodesRaw.length === 0 && + successCodesRaw[0].startsWith("2") const errorCodes = operation.errorSchemas.size > 0 && Object.fromEntries(operation.errorSchemas.entries()) const configAccessor = resolveConfigAccessor(operation, "options", "config") - pipeline.push( - `onRequest(${configAccessor})([${singleSuccessCode ? `"2xx"` : successCodes}]${ - errorCodes ? `, ${JSON.stringify(errorCodes)}` : "" - })` - ) + const onRequestArgs = [`[${singleSuccessCode ? `"2xx"` : successCodes}]`] + if (includeBinaryArgument) { + onRequestArgs.push(binarySuccessCodes.length > 0 ? `[${binarySuccessCodes}]` : "undefined") + } + if (errorCodes) { + onRequestArgs.push(JSON.stringify(errorCodes)) + } + pipeline.push(`onRequest(${configAccessor})(${onRequestArgs.join(", ")})`) return ( `"${operation.id}": (${params}) => ` + @@ -940,6 +986,11 @@ const binaryRequestSource = Stream.unwrap )` +const decodeBinarySource = `const decodeBinary = (response: HttpClientResponse.HttpClientResponse) => + response.arrayBuffer.pipe( + Effect.map((buffer) => new Uint8Array(buffer)) + )` + // Type-only mode helpers (no schema decoding) const sseRequestSourceTs = `const sseRequest = (request: HttpClientRequest.HttpClientRequest): Stream.Stream => @@ -959,6 +1010,12 @@ const binaryRequestSourceTs = Stream.unwrap )` +const decodeBinarySourceTs = + `const decodeBinary = (response: HttpClientResponse.HttpClientResponse): Effect.Effect => + response.arrayBuffer.pipe( + Effect.map((buffer) => new Uint8Array(buffer)) + )` + const clientErrorSource = ( name: string ) => diff --git a/packages/tools/openapi-generator/src/ParsedOperation.ts b/packages/tools/openapi-generator/src/ParsedOperation.ts index 725133d369a..69d7f66439c 100644 --- a/packages/tools/openapi-generator/src/ParsedOperation.ts +++ b/packages/tools/openapi-generator/src/ParsedOperation.ts @@ -221,8 +221,10 @@ export interface ParsedOperation { readonly voidSchemas: ReadonlySet // SSE streaming response schema (text/event-stream) readonly sseSchema?: string - // Binary stream response (application/octet-stream) + // Binary stream response for any successful binary media type. readonly binaryResponse: boolean + // Successful statuses that should decode the response body as binary. + readonly binarySuccessStatuses: ReadonlySet } /** @@ -273,5 +275,6 @@ export const makeDeepMutable = (options: { errorSchemas: new Map(), voidSchemas: new Set(), paramsOptional: true, - binaryResponse: false + binaryResponse: false, + binarySuccessStatuses: new Set() }) diff --git a/packages/tools/openapi-generator/test/OpenApiGenerator.test.ts b/packages/tools/openapi-generator/test/OpenApiGenerator.test.ts index bf93c46b416..4942fe9176d 100644 --- a/packages/tools/openapi-generator/test/OpenApiGenerator.test.ts +++ b/packages/tools/openapi-generator/test/OpenApiGenerator.test.ts @@ -52,6 +52,23 @@ function assertRuntimeIncludes(spec: OpenAPISpec, includes: ReadonlyArray) { + return Effect.gen(function*() { + const generator = yield* OpenApiGenerator.OpenApiGenerator + + const result = yield* generator.generate(spec, { + name: "TestClient", + format: "httpclient-type-only" + }) + + for (const expected of includes) { + assert.include(result, expected) + } + }).pipe( + Effect.provide(OpenApiGenerator.layerTransformerTs) + ) +} + function assertHttpApiIncludes( spec: OpenAPISpec, includes: ReadonlyArray, @@ -588,9 +605,235 @@ export const TestClientError = ( `readonly payload: typeof IssueTokenRequestFormUrlEncoded.Encoded` ] )) + + it.effect("unions JSON-compatible response media types on the same status in httpclient format", () => + assertRuntimeIncludes( + { + openapi: "3.1.0", + info: { + title: "Test API", + version: "1.0.0" + }, + paths: { + "/auth/device/token": { + post: { + operationId: "exchangeDeviceCode", + parameters: [], + responses: { + 400: { + description: "Problem or OAuth error", + content: { + "application/problem+json": { + schema: { + $ref: "#/components/schemas/ProblemError" + } + }, + "application/json": { + schema: { + $ref: "#/components/schemas/DeviceTokenOAuthError" + } + } + } + } + }, + tags: ["Auth"], + security: [] + } + } + }, + components: { + schemas: { + ProblemError: { + type: "object", + properties: { + kind: { const: "ProblemError" }, + title: { type: "string" } + }, + required: ["kind", "title"], + additionalProperties: false + }, + DeviceTokenOAuthError: { + type: "object", + properties: { + error: { type: "string" }, + error_description: { type: "string" } + }, + required: ["error"], + additionalProperties: false + } + }, + securitySchemes: {} + }, + security: [], + tags: [] + }, + [ + `export type ExchangeDeviceCode400 = ProblemError | DeviceTokenOAuthError`, + `export const ExchangeDeviceCode400 = Schema.Union([ProblemError, DeviceTokenOAuthError])`, + `"400": decodeError("ExchangeDeviceCode400", ExchangeDeviceCode400)` + ] + )) + + it.effect("supports binary success and problem+json error responses in httpclient format", () => + assertRuntimeIncludes( + { + openapi: "3.1.0", + info: { + title: "Test API", + version: "1.0.0" + }, + paths: { + "/archive": { + get: { + operationId: "downloadArchive", + parameters: [], + responses: { + 200: { + description: "Archive bytes", + content: { + "application/zip": { + schema: { + type: "string", + format: "binary" + } + } + } + }, + 404: { + description: "Missing archive", + content: { + "application/problem+json": { + schema: { + $ref: "#/components/schemas/ProblemError" + } + } + } + } + }, + tags: ["Archive"], + security: [] + } + }, + "/metadata": { + get: { + operationId: "getMetadata", + parameters: [], + responses: { + 200: { + description: "Archive metadata", + content: { + "application/json": { + schema: { + type: "object", + properties: { + name: { type: "string" } + }, + required: ["name"] + } + } + } + }, + 404: { + description: "Missing metadata", + content: { + "application/problem+json": { + schema: { + $ref: "#/components/schemas/ProblemError" + } + } + } + } + }, + tags: ["Archive"], + security: [] + } + } + }, + components: { + schemas: { + ProblemError: { + type: "object", + properties: { + kind: { const: "ProblemError" }, + title: { type: "string" } + }, + required: ["kind", "title"], + additionalProperties: false + } + }, + securitySchemes: {} + }, + security: [], + tags: [] + }, + [ + `const decodeBinary = (response: HttpClientResponse.HttpClientResponse) =>`, + `"2xx": decodeBinary`, + `"404": decodeError("DownloadArchive404", DownloadArchive404)`, + `readonly "downloadArchiveStream": () => Stream.Stream` + ] + )) }) describe("type-only", () => { + it.effect("keeps error decoder arguments aligned when another operation has a binary success", () => + assertTypeOnlyIncludes( + { + openapi: "3.1.0", + info: { title: "Test API", version: "1.0.0" }, + paths: { + "/archive": { + get: { + operationId: "downloadArchive", + parameters: [], + responses: { + 200: { + description: "Archive bytes", + content: { + "application/zip": { + schema: { type: "string", format: "binary" } + } + } + } + }, + tags: ["Archive"], + security: [] + } + }, + "/metadata": { + get: { + operationId: "getMetadata", + parameters: [], + responses: { + 200: { + description: "Archive metadata", + content: { + "application/json": { + schema: { type: "string" } + } + } + }, + 404: { + description: "Missing metadata", + content: { + "application/problem+json": { + schema: { type: "string" } + } + } + } + }, + tags: ["Archive"], + security: [] + } + } + }, + components: { schemas: {}, securitySchemes: {} }, + security: [], + tags: [] + }, + [`onRequest(options?.config)(["2xx"], undefined, {"404":"GetMetadata404"})`] + )) + it.effect("get operation", () => assertTypeOnly( {