Scan directives arguments while parsing schema

[oryan-block]

Resolves #664

Checklist

• Pull requests follows the contribution guide
• New or modified functionality is covered by tests

Description

Added enum and input object type handling to directives. It should be possible to parse any combination of valid GraphQL directives and arguments now.

[LucianRadu]

hey @oryan-block , is there a timeline for when this fix will be released?

[oryan-block]

@LucianRadu soon.

I was hoping to get some feedback before releasing.

[mwt-bgoodin]

@oryan-block I checked out master and built locally and am seeing another issue.

When I build and install 13.1.1-SNAPSHOT locally I am now seeing the following error:

graphql.kickstart.tools.SchemaClassScannerError: Enum type 'Purpose' is used in a directive, but no class could be found for that type name. Please pass a class for type 'Purpose' in the parser's dictionary.

I'm assuming this is getting hung up on the following in the federation-jvm schema

directive @link(
    url: String!,
    as: String,
    import: [Import],
    for: Purpose)
repeatable on SCHEMA

scalar Import

enum Purpose {
  SECURITY
  EXECUTION
}

I'm running the following:

• Java 17
• org.springframework.boot:spring-boot-starter-web:3.1.4
• com.graphql-java-kickstart:graphql-spring-boot-starter:15.0.0
• com.apollographql.federation:federation-graphql-java-support:4.2.0
• com.graphql-java-kickstart:graphql-java-tools:13.1.1-SNAPSHOT

I have a very basic app running for testing things out.

type Query {
    version: String!
}

public class VersionQueryResolver implements GraphQLQueryResolver {

    public CompletableFuture<String> version(final DataFetchingEnvironment dataFetchingEnvironment) {
        return CompletableFuture.supplyAsync(() -> {
            final GraphQLKickstartContext context = dataFetchingEnvironment.getLocalContext();
            final var request = (HttpServletRequest) context.getMapOfContext().get(HttpServletRequest.class);
            return "1.0.0 - " + request.getRequestURI();
        });
    }

}

[oryan-block]

@mwt-bgoodin are you not able to provide this Purpose class to the dictionary?

[mwt-bgoodin]

@oryan-block I had that same thought and did the following. I figured I'd wait before I added any more noise to the thread 😄

I created the following enum and mapped it:

public enum PurposeTypeEnum {
    SECURITY,
    EXECUTION
}

I setup the SchemaParserConfiguration to add it to the SchemaParserDictionary

@Component
public class SchemaParserConfiguration {

    @Bean
    public SchemaParserDictionary schemaParserDictionary() {
        return new SchemaParserDictionary()
            .add("Purpose", PurposeTypeEnum.class);
    }

}

After doing that I find that I'm back a FieldSet error that brought me to this issue in the first place 😄

Caused by: graphql.kickstart.tools.SchemaClassScannerError: Expected a user-defined GraphQL scalar type with name 'FieldSet' but found none!

Looking in the federation-jvm project i see they have _FieldSet scalar defined, but I'm honestly not sure how that would get mapped to the FieldSet scalar that is defined. Perhaps I'm missing something obvious and I'm supposed to load these scalars somehow?

scalar FieldSet

public final class _FieldSet {
  public static final String typeName = "_FieldSet";

  public static GraphQLScalarType type =
      GraphQLScalarType.newScalar(Scalars.GraphQLString)
          .name(typeName)
          .description(null)
          .coercing(Scalars.GraphQLString.getCoercing())
          .build();

  public static final ScalarTypeDefinition definition =
      ScalarTypeDefinition.newScalarTypeDefinition().name(typeName).build();

  private _FieldSet() {
    // hidden constructor
  }

[oryan-block]

@mwt-bgoodin you need to add this scalar to the schema parser as well. Same way you'd add other custom scalars.

[mwt-bgoodin]

@oryan-block Thank you for the feedback on this. I implemented the following which has steered me around the issue, but I still find it odd. I suspect I'm missing something deeper.

@Configuration
public class FederatedScalarConfig {

    @Bean
    public GraphQLScalarType fieldSetType() {
        return GraphQLScalarType.newScalar(Scalars.GraphQLString).name("FieldSet").description(null).coercing(Scalars.GraphQLString.getCoercing()).build();
    }

    @Bean
    public ScalarTypeDefinition fieldSetTypeDefinition() {
        return ScalarTypeDefinition.newScalarTypeDefinition().name("FieldSet").build();
    }

    @Bean
    public GraphQLScalarType scopeType() {
        return GraphQLScalarType.newScalar(Scalars.GraphQLString).name("Scope").description(null).coercing(Scalars.GraphQLString.getCoercing()).build();
    }

    @Bean
    public ScalarTypeDefinition scopeTypeDefinition() {
        return ScalarTypeDefinition.newScalarTypeDefinition().name("Scope").build();
    }

    @Bean
    public GraphQLScalarType importType() {
        return GraphQLScalarType.newScalar(Scalars.GraphQLString).name("Import").description(null).coercing(Scalars.GraphQLString.getCoercing()).build();
    }

    @Bean
    public ScalarTypeDefinition importTypeDefinition() {
        return ScalarTypeDefinition.newScalarTypeDefinition().name("Import").build();
    }

}

The work I'm doing is all part of an upgrade where we depended on a very early version of federation-graphql-java (0.7.0). The 2.x versions all contain the graphqls files which get picked up during the schema scan.

Documentation is tough to piece together for gluing this all together. Thank you for engaging on this.

[oryan-block]

@mwt-bgoodin it's possible that we should be doing this automatically for people who use this federation, but I don't know much about it so it's hard to say.
Do I understand correctly that you at least managed to get it working?

[mwt-bgoodin]

@oryan-block Yes, I was able to get it working. Thank you.

[mwt-bgoodin]

@oryan-block As a point of final feedback, your change has fixed the original issue. As long as I provide the GraphQLScalarTypes and SchemaParserConfiguration, the directives load as expected. When I roll back to 13.1.0 I see the issue again. I would love to see this change in an official release, so I can begin using it.

[oryan-block]

@mwt-bgoodin that's good to hear. Thanks for testing it. I'll try to get release going soon.