From a08584ead75045e4ddb6e3db175ce4d6c1e6b43c Mon Sep 17 00:00:00 2001 From: Martin PAUCOT Date: Wed, 13 Nov 2024 03:56:59 +0100 Subject: [PATCH 1/2] feat(openapi-metadata): handle array types and improve api property types --- packages/openapi-metadata/src/loaders/type.ts | 73 ++++++++-- .../openapi-metadata/src/metadata/property.ts | 9 +- packages/openapi-metadata/src/types.ts | 2 +- .../openapi-metadata/test/decorators.test.ts | 131 ++++++++++++++++-- .../test/loaders/array.test.ts | 38 +++++ .../test/loaders/class.test.ts | 25 ++++ .../test/loaders/primitives.test.ts | 36 +++++ 7 files changed, 285 insertions(+), 29 deletions(-) create mode 100644 packages/openapi-metadata/test/loaders/array.test.ts create mode 100644 packages/openapi-metadata/test/loaders/class.test.ts create mode 100644 packages/openapi-metadata/test/loaders/primitives.test.ts diff --git a/packages/openapi-metadata/src/loaders/type.ts b/packages/openapi-metadata/src/loaders/type.ts index db751d8d3..6c9b10ea6 100644 --- a/packages/openapi-metadata/src/loaders/type.ts +++ b/packages/openapi-metadata/src/loaders/type.ts @@ -7,7 +7,7 @@ import { PropertyMetadataStorage } from "../metadata/property.js"; import { schemaPath } from "../utils/schema.js"; import { isThunk } from "../utils/metadata.js"; -const PrimitiveTypeLoader: TypeLoaderFn = async (_context, value) => { +export const PrimitiveTypeLoader: TypeLoaderFn = async (_context, value) => { if (typeof value === "string") { return { type: value }; } @@ -28,7 +28,40 @@ const PrimitiveTypeLoader: TypeLoaderFn = async (_context, value) => { } }; -const ClassTypeLoader: TypeLoaderFn = async (context, value) => { +export const ArrayTypeLoader: TypeLoaderFn = async (context, value) => { + if (!Array.isArray(value)) { + return; + } + + if (value.length <= 0) { + context.logger.warn("You tried to specify an array type without any item"); + return; + } + + if (value.length > 1) { + context.logger.warn( + "You tried to specify an array type with multiple items. Please use the 'enum' option if you want to specify an enum.", + ); + return; + } + + const itemsSchema = await loadType(context, { type: value[0] }); + + // TODO: Better warn stack trace + if (!itemsSchema) { + context.logger.warn( + "You tried to specify an array type with an item that resolves to undefined.", + ); + return; + } + + return { + type: "array", + items: itemsSchema, + }; +}; + +export const ClassTypeLoader: TypeLoaderFn = async (context, value) => { if (typeof value !== "function" || !value.prototype) { return; } @@ -39,24 +72,32 @@ const ClassTypeLoader: TypeLoaderFn = async (context, value) => { return { $ref: schemaPath(model) }; } - const schema: SetRequired = { - type: "object", - properties: {}, - required: [], - }; + const schema: SetRequired = + { + type: "object", + properties: {}, + required: [], + }; const properties = PropertyMetadataStorage.getMetadata(value.prototype); if (!properties) { - context.logger.warn(`You tried to use '${model}' as a type but it does not contain any ApiProperty.`); - - return; + context.logger.warn( + `You tried to use '${model}' as a type but it does not contain any ApiProperty.`, + ); } context.schemas[model] = schema; for (const [key, property] of Object.entries(properties)) { - const { required, type, name, enum: e, schema: s, ...metadata } = property as any; + const { + required, + type, + name, + enum: e, + schema: s, + ...metadata + } = property as any; schema.properties[key] = { ...(await loadType(context, property)), ...metadata, @@ -96,12 +137,18 @@ export async function loadType( const thunk = isThunk(options.type); const value = thunk ? (options.type as Function)(context) : options.type; - for (const loader of [PrimitiveTypeLoader, ...context.typeLoaders, ClassTypeLoader]) { + for (const loader of [ + PrimitiveTypeLoader, + ...context.typeLoaders, + ClassTypeLoader, + ]) { const result = await loader(context, value, options.type); if (result) { return result; } } - context.logger.warn(`You tried to use '${options.type.toString()}' as a type but no loader supports it ${thunk}`); + context.logger.warn( + `You tried to use '${options.type.toString()}' as a type but no loader supports it ${thunk}`, + ); } diff --git a/packages/openapi-metadata/src/metadata/property.ts b/packages/openapi-metadata/src/metadata/property.ts index 83d6d7721..ead75659c 100644 --- a/packages/openapi-metadata/src/metadata/property.ts +++ b/packages/openapi-metadata/src/metadata/property.ts @@ -1,11 +1,16 @@ +import type { OpenAPIV3 } from "openapi-types"; import type { TypeOptions } from "../types.js"; import { createMetadataStorage } from "./factory.js"; -export type PropertyMetadata = { +export type PropertyMetadata = Omit< + OpenAPIV3.NonArraySchemaObject, + "type" | "enum" | "properties" | "required" +> & { name: string; required: boolean; } & TypeOptions; export const PropertyMetadataKey = Symbol("Property"); -export const PropertyMetadataStorage = createMetadataStorage>(PropertyMetadataKey); +export const PropertyMetadataStorage = + createMetadataStorage>(PropertyMetadataKey); diff --git a/packages/openapi-metadata/src/types.ts b/packages/openapi-metadata/src/types.ts index d5ddd0c02..c18975019 100644 --- a/packages/openapi-metadata/src/types.ts +++ b/packages/openapi-metadata/src/types.ts @@ -5,7 +5,7 @@ export type HttpMethods = `${OpenAPIV3.HttpMethods}`; export type PrimitiveType = OpenAPIV3.NonArraySchemaObjectType; -export type TypeValue = Function | PrimitiveType; +export type TypeValue = Function | PrimitiveType | [PrimitiveType | Function]; export type Thunk = (context: Context) => T; export type EnumTypeValue = string[] | number[] | Record; diff --git a/packages/openapi-metadata/test/decorators.test.ts b/packages/openapi-metadata/test/decorators.test.ts index a04772f7d..88d834267 100644 --- a/packages/openapi-metadata/test/decorators.test.ts +++ b/packages/openapi-metadata/test/decorators.test.ts @@ -8,11 +8,12 @@ import { ApiHeader, ApiOperation, ApiParam, + ApiProperty, ApiQuery, ApiResponse, ApiSecurity, ApiTags, -} from "../src/decorators"; +} from "../src/decorators/index.js"; import { ExcludeMetadataStorage, ExtraModelsMetadataStorage, @@ -21,8 +22,14 @@ import { OperationParameterMetadataStorage, OperationResponseMetadataStorage, OperationSecurityMetadataStorage, -} from "../src/metadata"; -import { ApiBasicAuth, ApiBearerAuth, ApiCookieAuth, ApiOauth2 } from "../src/decorators/api-security"; + PropertyMetadataStorage, +} from "../src/metadata/index.js"; +import { + ApiBasicAuth, + ApiBearerAuth, + ApiCookieAuth, + ApiOauth2, +} from "../src/decorators/api-security.js"; test("@ApiOperation", () => { class MyController { @@ -30,7 +37,10 @@ test("@ApiOperation", () => { operation() {} } - const metadata = OperationMetadataStorage.getMetadata(MyController.prototype, "operation"); + const metadata = OperationMetadataStorage.getMetadata( + MyController.prototype, + "operation", + ); expect(metadata).toEqual({ summary: "Hello", @@ -45,7 +55,10 @@ test("@ApiBody", () => { operation() {} } - const metadata = OperationBodyMetadataStorage.getMetadata(MyController.prototype, "operation"); + const metadata = OperationBodyMetadataStorage.getMetadata( + MyController.prototype, + "operation", + ); expect(metadata).toEqual({ type: "string", @@ -60,7 +73,11 @@ test("@ApiParam", () => { operation() {} } - const metadata = OperationParameterMetadataStorage.getMetadata(MyController.prototype, "operation", true); + const metadata = OperationParameterMetadataStorage.getMetadata( + MyController.prototype, + "operation", + true, + ); expect(metadata).toEqual([ { in: "path", name: "test" }, @@ -75,7 +92,11 @@ test("@ApiHeader", () => { operation() {} } - const metadata = OperationParameterMetadataStorage.getMetadata(MyController.prototype, "operation", true); + const metadata = OperationParameterMetadataStorage.getMetadata( + MyController.prototype, + "operation", + true, + ); expect(metadata).toEqual([ { in: "header", name: "test" }, @@ -90,7 +111,11 @@ test("@ApiCookie", () => { operation() {} } - const metadata = OperationParameterMetadataStorage.getMetadata(MyController.prototype, "operation", true); + const metadata = OperationParameterMetadataStorage.getMetadata( + MyController.prototype, + "operation", + true, + ); expect(metadata).toEqual([ { in: "cookie", name: "test" }, @@ -105,7 +130,11 @@ test("@ApiQuery", () => { operation() {} } - const metadata = OperationParameterMetadataStorage.getMetadata(MyController.prototype, "operation", true); + const metadata = OperationParameterMetadataStorage.getMetadata( + MyController.prototype, + "operation", + true, + ); expect(metadata).toEqual([ { in: "query", name: "test" }, @@ -120,7 +149,11 @@ test("@ApiResponse", () => { operation() {} } - const metadata = OperationResponseMetadataStorage.getMetadata(MyController.prototype, "operation", true); + const metadata = OperationResponseMetadataStorage.getMetadata( + MyController.prototype, + "operation", + true, + ); expect(metadata).toEqual({ default: { status: "default", mediaType: "text/html", type: "string" }, @@ -135,7 +168,11 @@ test("@ApiTags", () => { operation() {} } - const metadata = OperationMetadataStorage.getMetadata(MyController.prototype, "operation", true); + const metadata = OperationMetadataStorage.getMetadata( + MyController.prototype, + "operation", + true, + ); expect(metadata.tags).toEqual(["Root", "Hello", "World"]); }); @@ -150,7 +187,11 @@ test("@ApiSecurity", () => { operation() {} } - const metadata = OperationSecurityMetadataStorage.getMetadata(MyController.prototype, "operation", true); + const metadata = OperationSecurityMetadataStorage.getMetadata( + MyController.prototype, + "operation", + true, + ); expect(metadata).toEqual({ custom: [], @@ -175,7 +216,10 @@ test("@ApiExcludeOperation", () => { operation() {} } - const metadata = ExcludeMetadataStorage.getMetadata(MyController.prototype, "operation"); + const metadata = ExcludeMetadataStorage.getMetadata( + MyController.prototype, + "operation", + ); expect(metadata).toBe(true); }); @@ -189,3 +233,64 @@ test("@ApiExtraModels", () => { expect(metadata).toEqual(["string"]); }); + +test("@ApiProperty", () => { + class User { + @ApiProperty() + declare declared: string; + + @ApiProperty() + // biome-ignore lint/style/noInferrableTypes: required for metadata + defined: number = 4; + + @ApiProperty({ type: "string" }) + explicitType = "test"; + + @ApiProperty({ example: "hey" }) + get getter(): string { + return "hello"; + } + + @ApiProperty() + func(): boolean { + return false; + } + } + + const metadata = PropertyMetadataStorage.getMetadata(User.prototype); + + expect(metadata.declared).toMatchObject({ + name: "declared", + required: true, + }); + // @ts-expect-error + expect(metadata.declared?.type()).toEqual(String); + + expect(metadata.defined).toMatchObject({ + name: "defined", + required: true, + }); + // @ts-expect-error + expect(metadata.defined?.type()).toEqual(Number); + + expect(metadata.explicitType).toMatchObject({ + name: "explicitType", + required: true, + type: "string", + }); + + expect(metadata.getter).toMatchObject({ + name: "getter", + required: true, + example: "hey", + }); + // @ts-expect-error + expect(metadata.getter?.type()).toEqual(String); + + expect(metadata.func).toMatchObject({ + name: "func", + required: true, + }); + // @ts-expect-error + expect(metadata.func?.type()).toEqual(Boolean); +}); diff --git a/packages/openapi-metadata/test/loaders/array.test.ts b/packages/openapi-metadata/test/loaders/array.test.ts new file mode 100644 index 000000000..37371858b --- /dev/null +++ b/packages/openapi-metadata/test/loaders/array.test.ts @@ -0,0 +1,38 @@ +import type { Context } from "../../src/context.js"; +import { ArrayTypeLoader } from "../../src/loaders/type.js"; + +let error: string | undefined = undefined; +const context: Context = { + schemas: {}, + typeLoaders: [], + logger: { + warn: (message) => { + error = message; + }, + }, +}; + +test("simple array", async () => { + expect(await ArrayTypeLoader(context, [String])).toEqual({ + type: "array", + items: { + type: "string", + }, + }); +}); + +test("empty array should warn", async () => { + // @ts-expect-error + expect(await ArrayTypeLoader(context, [])).toEqual(undefined); + expect(error).toContain( + "You tried to specify an array type without any item", + ); +}); + +test("array with multiple items should warn", async () => { + // @ts-expect-error + expect(await ArrayTypeLoader(context, [String, Number])).toEqual(undefined); + expect(error).toContain( + "You tried to specify an array type with multiple items.", + ); +}); diff --git a/packages/openapi-metadata/test/loaders/class.test.ts b/packages/openapi-metadata/test/loaders/class.test.ts new file mode 100644 index 000000000..68fd80e0a --- /dev/null +++ b/packages/openapi-metadata/test/loaders/class.test.ts @@ -0,0 +1,25 @@ +import "reflect-metadata"; +import { ApiProperty } from "../../src/decorators/api-property.js"; +import { ClassTypeLoader } from "../../src/loaders/type.js"; +import type { Context } from "../../src/context.js"; + +test("simple class", async () => { + const context: Context = { schemas: {}, typeLoaders: [], logger: console }; + class Post { + @ApiProperty() + declare id: string; + } + + const result = await ClassTypeLoader(context, Post); + + expect(result).toEqual({ $ref: "#/components/schemas/Post" }); + expect(context.schemas.Post).toEqual({ + type: "object", + properties: { + id: { + type: "string", + }, + }, + required: ["id"], + }); +}); diff --git a/packages/openapi-metadata/test/loaders/primitives.test.ts b/packages/openapi-metadata/test/loaders/primitives.test.ts new file mode 100644 index 000000000..ee2745320 --- /dev/null +++ b/packages/openapi-metadata/test/loaders/primitives.test.ts @@ -0,0 +1,36 @@ +import type { Context } from "../../src/context.js"; +import { PrimitiveTypeLoader } from "../../src/loaders/type.js"; + +const context: Context = { schemas: {}, typeLoaders: [], logger: console }; + +test("string", async () => { + expect(await PrimitiveTypeLoader(context, "string")).toEqual({ + type: "string", + }); + + expect(await PrimitiveTypeLoader(context, "number")).toEqual({ + type: "number", + }); + + expect(await PrimitiveTypeLoader(context, "boolean")).toEqual({ + type: "boolean", + }); + + expect(await PrimitiveTypeLoader(context, "integer")).toEqual({ + type: "integer", + }); +}); + +test("constructor", async () => { + expect(await PrimitiveTypeLoader(context, String)).toEqual({ + type: "string", + }); + + expect(await PrimitiveTypeLoader(context, Number)).toEqual({ + type: "number", + }); + + expect(await PrimitiveTypeLoader(context, Boolean)).toEqual({ + type: "boolean", + }); +}); From 7e4124083567d444d7f136612b56fbc251cd6ddf Mon Sep 17 00:00:00 2001 From: Martin PAUCOT Date: Wed, 13 Nov 2024 04:00:11 +0100 Subject: [PATCH 2/2] lint and changeset --- .changeset/selfish-items-jump.md | 5 ++ packages/openapi-metadata/src/loaders/type.ts | 38 +++-------- .../openapi-metadata/src/metadata/property.ts | 8 +-- .../openapi-metadata/test/decorators.test.ts | 64 ++++--------------- .../test/loaders/array.test.ts | 8 +-- 5 files changed, 30 insertions(+), 93 deletions(-) create mode 100644 .changeset/selfish-items-jump.md diff --git a/.changeset/selfish-items-jump.md b/.changeset/selfish-items-jump.md new file mode 100644 index 000000000..5a2d88f68 --- /dev/null +++ b/.changeset/selfish-items-jump.md @@ -0,0 +1,5 @@ +--- +"openapi-metadata": minor +--- + +Handle array types and fix ApiProperty decorator type diff --git a/packages/openapi-metadata/src/loaders/type.ts b/packages/openapi-metadata/src/loaders/type.ts index 6c9b10ea6..9277a9285 100644 --- a/packages/openapi-metadata/src/loaders/type.ts +++ b/packages/openapi-metadata/src/loaders/type.ts @@ -49,9 +49,7 @@ export const ArrayTypeLoader: TypeLoaderFn = async (context, value) => { // TODO: Better warn stack trace if (!itemsSchema) { - context.logger.warn( - "You tried to specify an array type with an item that resolves to undefined.", - ); + context.logger.warn("You tried to specify an array type with an item that resolves to undefined."); return; } @@ -72,32 +70,22 @@ export const ClassTypeLoader: TypeLoaderFn = async (context, value) => { return { $ref: schemaPath(model) }; } - const schema: SetRequired = - { - type: "object", - properties: {}, - required: [], - }; + const schema: SetRequired = { + type: "object", + properties: {}, + required: [], + }; const properties = PropertyMetadataStorage.getMetadata(value.prototype); if (!properties) { - context.logger.warn( - `You tried to use '${model}' as a type but it does not contain any ApiProperty.`, - ); + context.logger.warn(`You tried to use '${model}' as a type but it does not contain any ApiProperty.`); } context.schemas[model] = schema; for (const [key, property] of Object.entries(properties)) { - const { - required, - type, - name, - enum: e, - schema: s, - ...metadata - } = property as any; + const { required, type, name, enum: e, schema: s, ...metadata } = property as any; schema.properties[key] = { ...(await loadType(context, property)), ...metadata, @@ -137,18 +125,12 @@ export async function loadType( const thunk = isThunk(options.type); const value = thunk ? (options.type as Function)(context) : options.type; - for (const loader of [ - PrimitiveTypeLoader, - ...context.typeLoaders, - ClassTypeLoader, - ]) { + for (const loader of [PrimitiveTypeLoader, ...context.typeLoaders, ClassTypeLoader]) { const result = await loader(context, value, options.type); if (result) { return result; } } - context.logger.warn( - `You tried to use '${options.type.toString()}' as a type but no loader supports it ${thunk}`, - ); + context.logger.warn(`You tried to use '${options.type.toString()}' as a type but no loader supports it ${thunk}`); } diff --git a/packages/openapi-metadata/src/metadata/property.ts b/packages/openapi-metadata/src/metadata/property.ts index ead75659c..23825ec6f 100644 --- a/packages/openapi-metadata/src/metadata/property.ts +++ b/packages/openapi-metadata/src/metadata/property.ts @@ -2,15 +2,11 @@ import type { OpenAPIV3 } from "openapi-types"; import type { TypeOptions } from "../types.js"; import { createMetadataStorage } from "./factory.js"; -export type PropertyMetadata = Omit< - OpenAPIV3.NonArraySchemaObject, - "type" | "enum" | "properties" | "required" -> & { +export type PropertyMetadata = Omit & { name: string; required: boolean; } & TypeOptions; export const PropertyMetadataKey = Symbol("Property"); -export const PropertyMetadataStorage = - createMetadataStorage>(PropertyMetadataKey); +export const PropertyMetadataStorage = createMetadataStorage>(PropertyMetadataKey); diff --git a/packages/openapi-metadata/test/decorators.test.ts b/packages/openapi-metadata/test/decorators.test.ts index 88d834267..989d9cee4 100644 --- a/packages/openapi-metadata/test/decorators.test.ts +++ b/packages/openapi-metadata/test/decorators.test.ts @@ -24,12 +24,7 @@ import { OperationSecurityMetadataStorage, PropertyMetadataStorage, } from "../src/metadata/index.js"; -import { - ApiBasicAuth, - ApiBearerAuth, - ApiCookieAuth, - ApiOauth2, -} from "../src/decorators/api-security.js"; +import { ApiBasicAuth, ApiBearerAuth, ApiCookieAuth, ApiOauth2 } from "../src/decorators/api-security.js"; test("@ApiOperation", () => { class MyController { @@ -37,10 +32,7 @@ test("@ApiOperation", () => { operation() {} } - const metadata = OperationMetadataStorage.getMetadata( - MyController.prototype, - "operation", - ); + const metadata = OperationMetadataStorage.getMetadata(MyController.prototype, "operation"); expect(metadata).toEqual({ summary: "Hello", @@ -55,10 +47,7 @@ test("@ApiBody", () => { operation() {} } - const metadata = OperationBodyMetadataStorage.getMetadata( - MyController.prototype, - "operation", - ); + const metadata = OperationBodyMetadataStorage.getMetadata(MyController.prototype, "operation"); expect(metadata).toEqual({ type: "string", @@ -73,11 +62,7 @@ test("@ApiParam", () => { operation() {} } - const metadata = OperationParameterMetadataStorage.getMetadata( - MyController.prototype, - "operation", - true, - ); + const metadata = OperationParameterMetadataStorage.getMetadata(MyController.prototype, "operation", true); expect(metadata).toEqual([ { in: "path", name: "test" }, @@ -92,11 +77,7 @@ test("@ApiHeader", () => { operation() {} } - const metadata = OperationParameterMetadataStorage.getMetadata( - MyController.prototype, - "operation", - true, - ); + const metadata = OperationParameterMetadataStorage.getMetadata(MyController.prototype, "operation", true); expect(metadata).toEqual([ { in: "header", name: "test" }, @@ -111,11 +92,7 @@ test("@ApiCookie", () => { operation() {} } - const metadata = OperationParameterMetadataStorage.getMetadata( - MyController.prototype, - "operation", - true, - ); + const metadata = OperationParameterMetadataStorage.getMetadata(MyController.prototype, "operation", true); expect(metadata).toEqual([ { in: "cookie", name: "test" }, @@ -130,11 +107,7 @@ test("@ApiQuery", () => { operation() {} } - const metadata = OperationParameterMetadataStorage.getMetadata( - MyController.prototype, - "operation", - true, - ); + const metadata = OperationParameterMetadataStorage.getMetadata(MyController.prototype, "operation", true); expect(metadata).toEqual([ { in: "query", name: "test" }, @@ -149,11 +122,7 @@ test("@ApiResponse", () => { operation() {} } - const metadata = OperationResponseMetadataStorage.getMetadata( - MyController.prototype, - "operation", - true, - ); + const metadata = OperationResponseMetadataStorage.getMetadata(MyController.prototype, "operation", true); expect(metadata).toEqual({ default: { status: "default", mediaType: "text/html", type: "string" }, @@ -168,11 +137,7 @@ test("@ApiTags", () => { operation() {} } - const metadata = OperationMetadataStorage.getMetadata( - MyController.prototype, - "operation", - true, - ); + const metadata = OperationMetadataStorage.getMetadata(MyController.prototype, "operation", true); expect(metadata.tags).toEqual(["Root", "Hello", "World"]); }); @@ -187,11 +152,7 @@ test("@ApiSecurity", () => { operation() {} } - const metadata = OperationSecurityMetadataStorage.getMetadata( - MyController.prototype, - "operation", - true, - ); + const metadata = OperationSecurityMetadataStorage.getMetadata(MyController.prototype, "operation", true); expect(metadata).toEqual({ custom: [], @@ -216,10 +177,7 @@ test("@ApiExcludeOperation", () => { operation() {} } - const metadata = ExcludeMetadataStorage.getMetadata( - MyController.prototype, - "operation", - ); + const metadata = ExcludeMetadataStorage.getMetadata(MyController.prototype, "operation"); expect(metadata).toBe(true); }); diff --git a/packages/openapi-metadata/test/loaders/array.test.ts b/packages/openapi-metadata/test/loaders/array.test.ts index 37371858b..c78a6b085 100644 --- a/packages/openapi-metadata/test/loaders/array.test.ts +++ b/packages/openapi-metadata/test/loaders/array.test.ts @@ -24,15 +24,11 @@ test("simple array", async () => { test("empty array should warn", async () => { // @ts-expect-error expect(await ArrayTypeLoader(context, [])).toEqual(undefined); - expect(error).toContain( - "You tried to specify an array type without any item", - ); + expect(error).toContain("You tried to specify an array type without any item"); }); test("array with multiple items should warn", async () => { // @ts-expect-error expect(await ArrayTypeLoader(context, [String, Number])).toEqual(undefined); - expect(error).toContain( - "You tried to specify an array type with multiple items.", - ); + expect(error).toContain("You tried to specify an array type with multiple items."); });