From 50814224e7e902f80f53cedfc0fb2ae512debabf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Lytek?= Date: Fri, 7 Jun 2024 13:02:40 +0200 Subject: [PATCH] fix(schema): prevent issues with subsequent schema builds (#1698) * Add test to reproduce the issue * Prevent subsequent build of metadata storage * Add test case for generic resolver and field resolvers * Make a local copy of metadata storage to build it * Revert "Prevent subsequent build of metadata storage" This reverts commit cc94f9798fc9e3e14053d04765eff9d0735100cb. * Add changelog entry --- CHANGELOG.md | 4 + src/schema/schema-generator.ts | 46 +++++----- tests/functional/generic-types.ts | 2 +- tests/functional/resolvers.ts | 145 ++++++++++++++++++++++++++++++ 4 files changed, 176 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bc414b4eb..1a97b06bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,10 @@ - make possible creating custom decorators on resolver class level - `createResolverClassMiddlewareDecorator` - support registering custom arg decorator via `createParameterDecorator` and its second argument `CustomParameterOptions` - `arg` (#1325) +## Fixes + +- properly build multiple schemas with generic resolvers, args and field resolvers (#1321) + ### Others - **Breaking Change**: update `graphql-scalars` peer dependency to `^1.23.0` diff --git a/src/schema/schema-generator.ts b/src/schema/schema-generator.ts index 56ed7a9c9..ffa11adec 100644 --- a/src/schema/schema-generator.ts +++ b/src/schema/schema-generator.ts @@ -40,6 +40,7 @@ import { import { type InterfaceClassMetadata } from "@/metadata/definitions/interface-class-metadata"; import { type ObjectClassMetadata } from "@/metadata/definitions/object-class-metadata"; import { getMetadataStorage } from "@/metadata/getMetadataStorage"; +import { MetadataStorage } from "@/metadata/metadata-storage"; import { createAdvancedFieldResolver, createBasicFieldResolver, @@ -118,10 +119,15 @@ export abstract class SchemaGenerator { private static usedInterfaceTypes = new Set(); + private static metadataStorage: MetadataStorage; + static generateFromMetadata(options: SchemaGeneratorOptions): GraphQLSchema { + this.metadataStorage = Object.assign(new MetadataStorage(), getMetadataStorage()); + this.metadataStorage.build(options); + this.checkForErrors(options); BuildContext.create(options); - getMetadataStorage().build(options); + this.buildTypesInfo(options.resolvers); const orphanedTypes = options.orphanedTypes ?? []; @@ -152,7 +158,7 @@ export abstract class SchemaGenerator { private static checkForErrors(options: SchemaGeneratorOptions) { ensureInstalledCorrectGraphQLPackage(); - if (getMetadataStorage().authorizedFields.length !== 0 && options.authChecker === undefined) { + if (this.metadataStorage.authorizedFields.length !== 0 && options.authChecker === undefined) { throw new Error( "You need to provide `authChecker` function for `@Authorized` decorator usage!", ); @@ -189,7 +195,7 @@ export abstract class SchemaGenerator { } private static buildTypesInfo(resolvers: Function[]) { - this.unionTypesInfo = getMetadataStorage().unions.map(unionMetadata => { + this.unionTypesInfo = this.metadataStorage.unions.map(unionMetadata => { // use closure to capture values from this selected schema build const unionObjectTypesInfo: ObjectTypeInfo[] = []; // called once after building all `objectTypesInfo` @@ -232,7 +238,7 @@ export abstract class SchemaGenerator { }; }); - this.enumTypesInfo = getMetadataStorage().enums.map(enumMetadata => { + this.enumTypesInfo = this.metadataStorage.enums.map(enumMetadata => { const enumMap = getEnumValuesMap(enumMetadata.enumObj); return { enumObj: enumMetadata.enumObj, @@ -253,7 +259,7 @@ export abstract class SchemaGenerator { }; }); - this.objectTypesInfo = getMetadataStorage().objectTypes.map(objectType => { + this.objectTypesInfo = this.metadataStorage.objectTypes.map(objectType => { const objectSuperClass = Object.getPrototypeOf(objectType.target); const hasExtended = objectSuperClass.prototype !== undefined; const getSuperClassType = () => { @@ -300,7 +306,7 @@ export abstract class SchemaGenerator { // support for implicitly implementing interfaces // get fields from interfaces definitions if (objectType.interfaceClasses) { - const implementedInterfaces = getMetadataStorage().interfaceTypes.filter(it => + const implementedInterfaces = this.metadataStorage.interfaceTypes.filter(it => objectType.interfaceClasses!.includes(it.target), ); implementedInterfaces.forEach(it => { @@ -312,7 +318,7 @@ export abstract class SchemaGenerator { let fields = fieldsMetadata.reduce>( (fieldsMap, field) => { - const { fieldResolvers } = getMetadataStorage(); + const { fieldResolvers } = this.metadataStorage; const filteredFieldResolversMetadata = fieldResolvers.filter( it => it.kind === "internal" || resolvers.includes(it.target), ); @@ -369,7 +375,7 @@ export abstract class SchemaGenerator { }; }); - this.interfaceTypesInfo = getMetadataStorage().interfaceTypes.map( + this.interfaceTypesInfo = this.metadataStorage.interfaceTypes.map( interfaceType => { const interfaceSuperClass = Object.getPrototypeOf(interfaceType.target); const hasExtended = interfaceSuperClass.prototype !== undefined; @@ -381,8 +387,8 @@ export abstract class SchemaGenerator { }; // fetch ahead the subset of object types that implements this interface - const implementingObjectTypesTargets = getMetadataStorage() - .objectTypes.filter( + const implementingObjectTypesTargets = this.metadataStorage.objectTypes + .filter( objectType => objectType.interfaceClasses && objectType.interfaceClasses.includes(interfaceType.target), @@ -419,7 +425,7 @@ export abstract class SchemaGenerator { // support for implicitly implementing interfaces // get fields from interfaces definitions if (interfaceType.interfaceClasses) { - const implementedInterfacesMetadata = getMetadataStorage().interfaceTypes.filter( + const implementedInterfacesMetadata = this.metadataStorage.interfaceTypes.filter( it => interfaceType.interfaceClasses!.includes(it.target), ); implementedInterfacesMetadata.forEach(it => { @@ -431,7 +437,7 @@ export abstract class SchemaGenerator { let fields = fieldsMetadata!.reduce>( (fieldsMap, field) => { - const fieldResolverMetadata = getMetadataStorage().fieldResolvers.find( + const fieldResolverMetadata = this.metadataStorage.fieldResolvers.find( resolver => resolver.getObjectType!() === field.target && resolver.methodName === field.name, @@ -490,7 +496,7 @@ export abstract class SchemaGenerator { }, ); - this.inputTypesInfo = getMetadataStorage().inputTypes.map(inputType => { + this.inputTypesInfo = this.metadataStorage.inputTypes.map(inputType => { const objectSuperClass = Object.getPrototypeOf(inputType.target); const getSuperClassType = () => { const superClassTypeInfo = this.inputTypesInfo.find( @@ -549,7 +555,7 @@ export abstract class SchemaGenerator { } private static buildRootQueryType(resolvers: Function[]): GraphQLObjectType { - const queriesHandlers = this.filterHandlersByResolvers(getMetadataStorage().queries, resolvers); + const queriesHandlers = this.filterHandlersByResolvers(this.metadataStorage.queries, resolvers); return new GraphQLObjectType({ name: "Query", @@ -559,7 +565,7 @@ export abstract class SchemaGenerator { private static buildRootMutationType(resolvers: Function[]): GraphQLObjectType | undefined { const mutationsHandlers = this.filterHandlersByResolvers( - getMetadataStorage().mutations, + this.metadataStorage.mutations, resolvers, ); if (mutationsHandlers.length === 0) { @@ -574,7 +580,7 @@ export abstract class SchemaGenerator { private static buildRootSubscriptionType(resolvers: Function[]): GraphQLObjectType | undefined { const subscriptionsHandlers = this.filterHandlersByResolvers( - getMetadataStorage().subscriptions, + this.metadataStorage.subscriptions, resolvers, ); if (subscriptionsHandlers.length === 0) { @@ -735,8 +741,8 @@ export abstract class SchemaGenerator { input.index, input.name, ); - const argDirectives = getMetadataStorage() - .argumentDirectives.filter( + const argDirectives = this.metadataStorage.argumentDirectives + .filter( it => it.target === target && it.fieldName === propertyName && @@ -752,7 +758,7 @@ export abstract class SchemaGenerator { astNode: getInputValueDefinitionNode(input.name, type, argDirectives), }; } else if (param.kind === "args") { - const argumentType = getMetadataStorage().argumentTypes.find( + const argumentType = this.metadataStorage.argumentTypes.find( it => it.target === param.getType(), ); if (!argumentType) { @@ -771,7 +777,7 @@ export abstract class SchemaGenerator { inheritanceChainClasses.push(superClass); } for (const argsTypeClass of inheritanceChainClasses.reverse()) { - const inheritedArgumentType = getMetadataStorage().argumentTypes.find( + const inheritedArgumentType = this.metadataStorage.argumentTypes.find( it => it.target === argsTypeClass, ); if (inheritedArgumentType) { diff --git a/tests/functional/generic-types.ts b/tests/functional/generic-types.ts index aeae47b87..2831bf078 100644 --- a/tests/functional/generic-types.ts +++ b/tests/functional/generic-types.ts @@ -408,7 +408,7 @@ describe("Generic types", () => { let schema: GraphQLSchema; let schemaIntrospection: IntrospectionSchema; - beforeAll(async () => { + beforeEach(async () => { function Base(TTypeClass: ClassType) { @ObjectType() class BaseClass { diff --git a/tests/functional/resolvers.ts b/tests/functional/resolvers.ts index c4ac43d5f..35f90ec1d 100644 --- a/tests/functional/resolvers.ts +++ b/tests/functional/resolvers.ts @@ -2582,4 +2582,149 @@ describe("Resolvers", () => { expect(dynamicField2).toBeDefined(); }); }); + + describe("Shared generic resolver", () => { + beforeEach(async () => { + getMetadataStorage().clear(); + }); + + it("should handle arguments correctly on multiple buildSchema runs", async () => { + @ObjectType() + class TestResponse { + @Field() + data!: string; + } + + @ArgsType() + class TestArgs { + @Field(() => Int, { defaultValue: 0 }) + testField!: number; + } + + function makeResolverClass() { + @Resolver(() => TestResponse) + abstract class TestResolver { + @Query(() => TestResponse) + async exampleQuery(@Args() args: TestArgs): Promise { + return { + data: `resolver ${args.testField}`, + }; + } + } + + return TestResolver; + } + + @Resolver() + class TestResolver extends makeResolverClass() {} + + const fistSchemaInfo = await getSchemaInfo({ + resolvers: [TestResolver], + }); + + expect(fistSchemaInfo.queryType.fields).toHaveLength(1); + expect(fistSchemaInfo.queryType.fields[0].args).toHaveLength(1); + + const secondSchemaInfo = await getSchemaInfo({ + resolvers: [TestResolver], + }); + + expect(secondSchemaInfo.queryType.fields).toHaveLength(1); + expect(secondSchemaInfo.queryType.fields[0].args).toHaveLength(1); + }); + + it("should handle field resolvers correctly on multiple buildSchema runs", async () => { + @ObjectType() + class TestResponse { + @Field() + data!: string; + } + + @ArgsType() + class TestArgs { + @Field(() => Int, { defaultValue: 0 }) + testField!: number; + } + + function makeResolverClass() { + @Resolver(() => TestResponse) + abstract class TestResolver { + @Query(() => TestResponse) + async exampleQuery(@Args() args: TestArgs): Promise { + return { + data: `resolver ${args.testField}`, + }; + } + } + + return TestResolver; + } + + @Resolver(() => TestResponse) + class TestResolver extends makeResolverClass() { + @FieldResolver(() => Boolean, { nullable: false }) + public async exampleFieldResolver(): Promise { + return true; + } + } + + @ObjectType() + class OtherTestResponse { + @Field() + data!: string; + } + + @ArgsType() + class OtherTestArgs { + @Field(() => Int, { defaultValue: 0 }) + testField!: number; + } + + function makeOtherResolverClass() { + @Resolver(() => OtherTestResponse) + abstract class OtherTestResolver { + @Query(() => OtherTestResponse) + async exampleQuery(@Args() args: OtherTestArgs): Promise { + return { + data: `resolver ${args.testField}`, + }; + } + } + + return OtherTestResolver; + } + + @Resolver(() => OtherTestResponse) + class OtherTestResolver extends makeOtherResolverClass() { + @FieldResolver(() => Boolean, { nullable: false }) + public async exampleFieldResolver(): Promise { + return true; + } + } + + const fistSchemaInfo = await getSchemaInfo({ + resolvers: [TestResolver], + }); + + const hasFoundFieldResolverInSchema = fistSchemaInfo.schemaIntrospection.types.some( + type => + type.kind === "OBJECT" && + type.name === "TestResponse" && + type.fields?.some(field => field.name === "exampleFieldResolver"), + ); + expect(hasFoundFieldResolverInSchema).toBeTruthy(); + + const secondSchemaInfo = await getSchemaInfo({ + resolvers: [OtherTestResolver], + }); + + const hasFoundFieldResolverInOtherSchema = secondSchemaInfo.schemaIntrospection.types.some( + type => + type.kind === "OBJECT" && + type.name === "OtherTestResponse" && + type.fields?.some(field => field.name === "exampleFieldResolver"), + ); + expect(hasFoundFieldResolverInOtherSchema).toBeTruthy(); + }); + }); });