graphql-java-kickstart/graphql-java-tools

SchemaClassScanner doesn't scan directive arguments

oryan-block opened this issue · 8 comments

Description

Custom scalars and input objects used in directives are not scanned, and subsequently their type cannot be determined when building directives in SchemaParser

Expected behavior

scalar CustomScalar
directive @something(arg: CustomScalar) on FIELD_DEFINITION

type Query {
    allProducts: [Product]
}

type Product {
    name: String @something(arg: "1")
}

Should parse successfully

Actual behavior

graphql.kickstart.tools.SchemaError: Expected type 'CustomScalar' to be a GraphQLInputType, but it wasn't! Was a type only permitted for object types incorrectly used as an input type, or vice-versa?

This causes apollo-federated schemas to fail because _FieldSet is only used in directives.

Caused by: graphql.kickstart.tools.SchemaError: Expected type '_FieldSet' to be a GraphQLInputType, but it wasn't!  Was a type only permitted for object types incorrectly used as an input type, or vice-versa?
	at graphql.kickstart.tools.SchemaParser.determineType(SchemaParser.kt:355) ~[graphql-java-tools-13.0.0.jar:na]
	at graphql.kickstart.tools.SchemaParser.determineInputType(SchemaParser.kt:373) ~[graphql-java-tools-13.0.0.jar:na]
	at graphql.kickstart.tools.SchemaParser.determineInputType(SchemaParser.kt:365) ~[graphql-java-tools-13.0.0.jar:na]
	at graphql.kickstart.tools.SchemaParser.createDirective(SchemaParser.kt:307) ~[graphql-java-tools-13.0.0.jar:na]
	at graphql.kickstart.tools.SchemaParser.parseSchemaObjects(SchemaParser.kt:83) ~[graphql-java-tools-13.0.0.jar:na]
	at graphql.kickstart.tools.SchemaParser.makeExecutableSchema(SchemaParser.kt:110) ~[graphql-java-tools-13.0.0.jar:na]

this can be reproduced with:

package test;

import java.util.stream.Collectors;
import java.util.stream.Stream;

import org.junit.Test;

import graphql.GraphQL;
import graphql.kickstart.tools.GraphQLQueryResolver;
import graphql.kickstart.tools.SchemaParser;
import graphql.kickstart.tools.SchemaParserBuilder;
import graphql.scalar.GraphqlStringCoercing;
import graphql.schema.DataFetchingEnvironment;
import graphql.schema.GraphQLScalarType;
import graphql.schema.GraphQLSchema;

public class IssueTest {
    static class MockQueryResolver implements GraphQLQueryResolver {
        public String mockField(String input, DataFetchingEnvironment environment) {
            return null;
        }
    };
    
    @Test
    public void testUsedInField() {
        // this passes
        graphQl(
            "scalar MockScalar",
            "directive @mockDirective(arg: MockScalar) on FIELD_DEFINITION",
            "type Query {",
            "    mockField(input: MockScalar): String @mockDirective(arg: \"mock-value\")",
            "}"
        );
    }
    
    @Test
    public void testUsedOnlyInDirectives() {
        // this fails with:
        // graphql.kickstart.tools.SchemaError: Expected type 'MockScalar' to be
        // a GraphQLInputType, but it wasn't!  Was a type only permitted for
        // object types incorrectly used as an input type, or vice-versa?
        graphQl(
            "scalar MockScalar",
            "directive @mockDirective(arg: MockScalar) on FIELD_DEFINITION",
            "type Query {",
            "    mockField(input: String) : String @mockDirective(arg: \"mock-value\")",
            "}"
        );
    }
    
    private GraphQL graphQl(String... sdl) {
        SchemaParserBuilder builder = new SchemaParserBuilder();
        builder.schemaString(Stream.of(sdl).collect(Collectors.joining("\n")));
        SchemaParser schemaParser = builder
            .resolvers(new MockQueryResolver())
            .scalars(GraphQLScalarType.newScalar()
                .name("MockScalar")
                .coercing(new GraphqlStringCoercing())
                .build()
            )
            .build();
        GraphQLSchema schema = schemaParser.makeExecutableSchema();
        return GraphQL.newGraphQL(schema).build();
    }
}

@oryan-block is this easy to fix? i never coded in kotlin but if you give me a few pointers I can give it a shot.

Unfortunately because of the way the scanner works and how different directives are from everything else, this is not a trivial fix.

This should be partially resolved by #763 for scalars.
However, directives with input object arguments/parameters are still not supported (this was the case even before the applied directives change)

@oryan-block Any updates on this or work arounds?

This should be partially resolved by #763 for scalars. However, directives with input object arguments/parameters are still not supported (this was the case even before the applied directives change)

@airvine-r7 that fix is merged. You can test it now.
#764 is a more complete solution I think should make directives fully supported.