Refactor to simplify GraphQL Schema generators

[javiertoledo]

Reviewing #575 I realized that we needed to introduce a default NoOp query when no queries are defined to fix an error with GraphQL schema validations, and digging a little bit I found that it could make sense to rework the schema generation code a little bit to manage these situations better.

In the current implementation, the generateSchema function is the final step to build the GraphQLSchema object, and the fix in #575 is a workaround to avoid the query being empty and making the validation fail:

  // Defined in packages/framework-core/src/services/graphql/graphql-generator.ts

  public generateSchema(): GraphQLSchema {
    return new GraphQLSchema({
      query: this.queryGenerator.generate(), // Avoid adding this if `this.queryGenerator` is undefined
      mutation: this.mutationGenerator.generate(),
      subscription: this.subscriptionGenerator.generate(),
    })
  }

Looking at the GraphQLSchema class implementation in the graphql package, we can see that all the config parameters are actually optional, so I guess that not initializing them could be a better fix:

// Fragment of the file type/schema.d.ts in the `graphql` package
...
export class GraphQLSchema {
  ...
  constructor(config: Readonly<GraphQLSchemaConfig>);
  ...
}
...
export interface GraphQLSchemaConfig extends GraphQLSchemaValidationOptions {
  description?: Maybe<string>;
  query?: Maybe<GraphQLObjectType>;
  mutation?: Maybe<GraphQLObjectType>;
  subscription?: Maybe<GraphQLObjectType>;
  types?: Maybe<Array<GraphQLNamedType>>;
  directives?: Maybe<Array<GraphQLDirective>>;
  extensions?: Maybe<Readonly<GraphQLSchemaExtensions>>;
  astNode?: Maybe<SchemaDefinitionNode>;
  extensionASTNodes?: Maybe<ReadonlyArray<SchemaExtensionNode>>;
}

Back to our code, looking at the class GraphQLGenerator, we can see that the process involves creating and initializing many objects that are actually only depending on the config:

// Fragment of packages/framework-core/src/services/graphql/graphql-generator.ts

export class GraphQLGenerator {
  private readonly queryGenerator: GraphQLQueryGenerator
  private readonly mutationGenerator: GraphQLMutationGenerator
  private readonly subscriptionGenerator: GraphQLSubscriptionGenerator
  private readonly typeInformer: GraphQLTypeInformer

  private static singleton: GraphQLGenerator | undefined

  public static build(config: BoosterConfig, logger: Logger): GraphQLGenerator {
    this.singleton =
      this.singleton ??
      new GraphQLGenerator(
        config,
        new BoosterCommandDispatcher(config, logger),
        new BoosterReadModelDispatcher(config, logger)
      )
    return this.singleton
  }

  private constructor(
    config: BoosterConfig,
    private commandsDispatcher: BoosterCommandDispatcher,
    private readModelsDispatcher: BoosterReadModelDispatcher
  ) {
    this.typeInformer = new GraphQLTypeInformer({ ...config.readModels, ...config.commandHandlers })
    this.queryGenerator = new GraphQLQueryGenerator(
      config.readModels,
      this.typeInformer,
      this.readModelByIDResolverBuilder.bind(this),
      this.readModelResolverBuilder.bind(this)
    )
    this.mutationGenerator = new GraphQLMutationGenerator(
      config.commandHandlers,
      this.typeInformer,
      this.commandResolverBuilder.bind(this)
    )
    this.subscriptionGenerator = new GraphQLSubscriptionGenerator(
      config.readModels,
      this.typeInformer,
      this.queryGenerator,
      this.subscriptionByIDResolverBuilder.bind(this),
      this.subscriptionResolverBuilder.bind(this)
    )
  }

  public generateSchema(): GraphQLSchema {
    return new GraphQLSchema({
      query: this.queryGenerator.generate(),
      mutation: this.mutationGenerator.generate(),
      subscription: this.subscriptionGenerator.generate(),
    })
  }
  ...

I'd propose to rework this code as a sequence of static function calls that receive the config and return back the right object for each of the fields required by the GraphQLSchema constructor, avoiding all these initialization and injection of dependencies that are obfuscating this code. In this way, it will be easier to build the final GraphQLSchemaConfig only with the fields that make sense, not requiring us to generate the extra NoOp query, and likely avoiding other possible similar bugs that we haven't found yet.

[renansoares]

can I pick up this issue?

[javiertoledo]

Hi @renansoares, yeah, I think that nobody else is working on it. Feel free to assign it to you. If you have questions, you can reach out the @boostercloud/booster-core team here or in Discord (find the link in https://booster.cloud). Thanks!!

[adriangmweb]

hey @renansoares, actually I'm doing some work that affects the GraphQL query schema. My work involves the graphql-query-generator.ts and graphql-type-informer.ts, just have in mind that if you are going to work on them, there might be conflicts.