graphql-java-kickstart/graphql-java-tools

Old-style directives get lost, only new applied directives are present.

enriquedacostacambio opened this issue · 2 comments

Description

After the directive rework, directives are lost in favor of applied directives, this causes a problem with other tools that have not migrated to applied directives and are using the deprecated but still valid plain directives (such as graphql-java-extended-validation)

Imo both should be kept for compatibility purposes here: https://github.com/graphql-java-kickstart/graphql-java-tools/pull/663/files#diff-7817f33fe28a145f0c7b6bdcbbaea4cd6e7a9bed458b38db8abbab214ef76fddL169

Expected behavior

Both old directives and applied directives are kept in field definitions (and other objects).

Actual behavior

directives are lost, only applied ones are present.

Steps to reproduce the bug

package test;

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

import org.junit.Assert;
import org.junit.Test;

import graphql.ExecutionInput;
import graphql.GraphQL;
import graphql.kickstart.tools.GraphQLQueryResolver;
import graphql.kickstart.tools.SchemaParser;
import graphql.kickstart.tools.SchemaParserBuilder;
import graphql.schema.DataFetchingEnvironment;
import graphql.schema.GraphQLSchema;

public class IssueTest {
    static class MockQueryResolver implements GraphQLQueryResolver {
        public String mockField(DataFetchingEnvironment environment) {
            // This passes:
            Assert.assertNotNull(environment.getFieldDefinition().getAppliedDirective("mockDirective"));
            // This fails:
            Assert.assertNotNull(environment.getFieldDefinition().getDirective("mockDirective"));
            return null;
        }
    };
    
    @Test
    public void test() {
        GraphQL graphQl = graphQl(
            "directive @mockDirective on FIELD_DEFINITION",
            "type Query {",
            "    mockField: String @mockDirective",
            "}"
        );
        ExecutionInput executionInput = ExecutionInput.newExecutionInput()
            .query("{ mockField }")
            .build();
        graphQl.execute(executionInput);
    }
    
    private GraphQL graphQl(String... sdl) {
        SchemaParserBuilder builder = new SchemaParserBuilder();
        builder.schemaString(Stream.of(sdl).collect(Collectors.joining("\n")));
        SchemaParser schemaParser = builder
            .resolvers(new MockQueryResolver())
            .build();
        GraphQLSchema schema = schemaParser.makeExecutableSchema();
        return GraphQL.newGraphQL(schema).build();
    }
}

@enriquedacostacambio are you able to test #708 before I merge it to make sure it solves your problem?

Yup, #708 solves this issue. Thanks!