Skip to content

Commit

Permalink
fix(schema): prevent issues with subsequent schema builds (#1698)
Browse files Browse the repository at this point in the history
* 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 cc94f97.

* Add changelog entry
  • Loading branch information
MichalLytek authored Jun 7, 2024
1 parent 1336d84 commit 5081422
Show file tree
Hide file tree
Showing 4 changed files with 176 additions and 21 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
46 changes: 26 additions & 20 deletions src/schema/schema-generator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -118,10 +119,15 @@ export abstract class SchemaGenerator {

private static usedInterfaceTypes = new Set<Function>();

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 ?? [];
Expand Down Expand Up @@ -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!",
);
Expand Down Expand Up @@ -189,7 +195,7 @@ export abstract class SchemaGenerator {
}

private static buildTypesInfo(resolvers: Function[]) {
this.unionTypesInfo = getMetadataStorage().unions.map<UnionTypeInfo>(unionMetadata => {
this.unionTypesInfo = this.metadataStorage.unions.map<UnionTypeInfo>(unionMetadata => {
// use closure to capture values from this selected schema build
const unionObjectTypesInfo: ObjectTypeInfo[] = [];
// called once after building all `objectTypesInfo`
Expand Down Expand Up @@ -232,7 +238,7 @@ export abstract class SchemaGenerator {
};
});

this.enumTypesInfo = getMetadataStorage().enums.map<EnumTypeInfo>(enumMetadata => {
this.enumTypesInfo = this.metadataStorage.enums.map<EnumTypeInfo>(enumMetadata => {
const enumMap = getEnumValuesMap(enumMetadata.enumObj);
return {
enumObj: enumMetadata.enumObj,
Expand All @@ -253,7 +259,7 @@ export abstract class SchemaGenerator {
};
});

this.objectTypesInfo = getMetadataStorage().objectTypes.map<ObjectTypeInfo>(objectType => {
this.objectTypesInfo = this.metadataStorage.objectTypes.map<ObjectTypeInfo>(objectType => {
const objectSuperClass = Object.getPrototypeOf(objectType.target);
const hasExtended = objectSuperClass.prototype !== undefined;
const getSuperClassType = () => {
Expand Down Expand Up @@ -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 => {
Expand All @@ -312,7 +318,7 @@ export abstract class SchemaGenerator {

let fields = fieldsMetadata.reduce<GraphQLFieldConfigMap<any, any>>(
(fieldsMap, field) => {
const { fieldResolvers } = getMetadataStorage();
const { fieldResolvers } = this.metadataStorage;
const filteredFieldResolversMetadata = fieldResolvers.filter(
it => it.kind === "internal" || resolvers.includes(it.target),
);
Expand Down Expand Up @@ -369,7 +375,7 @@ export abstract class SchemaGenerator {
};
});

this.interfaceTypesInfo = getMetadataStorage().interfaceTypes.map<InterfaceTypeInfo>(
this.interfaceTypesInfo = this.metadataStorage.interfaceTypes.map<InterfaceTypeInfo>(
interfaceType => {
const interfaceSuperClass = Object.getPrototypeOf(interfaceType.target);
const hasExtended = interfaceSuperClass.prototype !== undefined;
Expand All @@ -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),
Expand Down Expand Up @@ -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 => {
Expand All @@ -431,7 +437,7 @@ export abstract class SchemaGenerator {

let fields = fieldsMetadata!.reduce<GraphQLFieldConfigMap<any, any>>(
(fieldsMap, field) => {
const fieldResolverMetadata = getMetadataStorage().fieldResolvers.find(
const fieldResolverMetadata = this.metadataStorage.fieldResolvers.find(
resolver =>
resolver.getObjectType!() === field.target &&
resolver.methodName === field.name,
Expand Down Expand Up @@ -490,7 +496,7 @@ export abstract class SchemaGenerator {
},
);

this.inputTypesInfo = getMetadataStorage().inputTypes.map<InputObjectTypeInfo>(inputType => {
this.inputTypesInfo = this.metadataStorage.inputTypes.map<InputObjectTypeInfo>(inputType => {
const objectSuperClass = Object.getPrototypeOf(inputType.target);
const getSuperClassType = () => {
const superClassTypeInfo = this.inputTypesInfo.find(
Expand Down Expand Up @@ -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",
Expand All @@ -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) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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 &&
Expand All @@ -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) {
Expand All @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion tests/functional/generic-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,7 @@ describe("Generic types", () => {
let schema: GraphQLSchema;
let schemaIntrospection: IntrospectionSchema;

beforeAll(async () => {
beforeEach(async () => {
function Base<TType extends object>(TTypeClass: ClassType<TType>) {
@ObjectType()
class BaseClass {
Expand Down
145 changes: 145 additions & 0 deletions tests/functional/resolvers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<TestResponse> {
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<TestResponse> {
return {
data: `resolver ${args.testField}`,
};
}
}

return TestResolver;
}

@Resolver(() => TestResponse)
class TestResolver extends makeResolverClass() {
@FieldResolver(() => Boolean, { nullable: false })
public async exampleFieldResolver(): Promise<boolean> {
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<OtherTestResponse> {
return {
data: `resolver ${args.testField}`,
};
}
}

return OtherTestResolver;
}

@Resolver(() => OtherTestResponse)
class OtherTestResolver extends makeOtherResolverClass() {
@FieldResolver(() => Boolean, { nullable: false })
public async exampleFieldResolver(): Promise<boolean> {
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();
});
});
});

0 comments on commit 5081422

Please sign in to comment.