René's URL Explorer Experiment

Title: Fix @skip definition is added twice to the GraphQLSchema when defined in sdl by tinnou · Pull Request #2940 · graphql-java/graphql-java · GitHub

Open Graph Title: Fix @skip definition is added twice to the GraphQLSchema when defined in sdl by tinnou · Pull Request #2940 · graphql-java/graphql-java

X Title: Fix @skip definition is added twice to the GraphQLSchema when defined in sdl by tinnou · Pull Request #2940 · graphql-java/graphql-java

Description: Description When @skip/@include is defined in a SDL, schema generation will create a GraphQLSchema with multiple skip/include definitions. This can cause problems when a schema is serialized and parsed multiple times. A little far fetched example: @Test fun parse() { val schema: GraphQLSchema = """ type Query { hello: String } """.trimIndent().mockSchema() val sdl = schema.printSdl() val schema2 = sdl.mockSchema() val sdl2 = schema2.printSdl() sdl2.mockSchema() // throws } fun GraphQLSchema.printSdl(): String { return SchemaPrinter(SchemaPrinter.Options.defaultOptions() .includeDirectiveDefinitions(true) ).print(this) } will throw: Caused by: SchemaProblem{errors=['include' type [@13:1] tried to redefine existing directive 'include' type [@7:1], 'skip' type [@25:1] tried to redefine existing directive 'skip' type [@19:1]]} at graphql.schema.idl.SchemaParser.buildRegistry(SchemaParser.java:156) at graphql.schema.idl.SchemaParser.parseImpl(SchemaParser.java:124) at graphql.schema.idl.SchemaParser.parse(SchemaParser.java:90) at graphql.schema.idl.SchemaParser.parse(SchemaParser.java:75) We could add a predicate to avoid printing directives skip and include but it would sit better if we could ensure printing the sdl generates a valid SDL (that can be parsed back into a valid GraphQLSchema). The actual issue comes from the fact @skip and @include are added by default on the GraphQLSchema builder. Expected Behavior To preserve the behavior of being able to redefine the skip and include directives without raising an error, we should probably just ignore any directive definition named skip or include and rely on the added by default definitions provided by the GraphQL library implementation. I'm on the fence as to add validation to check if a provided skip/include definition is semantically equivalent to the ones defined by the server. Since these are guaranteed to be provided by the GraphQL server implementation, I don't feel this is really needed and it will likely be an artifact of a tool having printed them in the SDL. Changes The fix consists of skipping any directive defined in the sdl with the name skip/include with the expectation graphql-java will use its own. Testing The following test fails and show that redefining the skip/include directive leads to multiple definitions added to the GraphQLSchema instance. def "skip and include should be added to the schema only if not already defined"() { def sdl = ''' "Directs the executor to skip this field or fragment when the `if`'argument is true." directive @skip( "Skipped when true." if: Boolean! ) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT "Directs the executor to include this field or fragment only when the `if` argument is true" directive @include( "Included when true." if: Boolean! ) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT type Query { hello: String } ''' when: def schema = TestUtil.schema(sdl) then: schema.getDirectives().findAll { it.name == "skip" }.size() == 1 schema.getDirectives().findAll { it.name == "include" }.size() == 1 and: def newSchema = GraphQLSchema.newSchema(schema).build() then: newSchema.getDirectives().findAll { it.name == "skip" }.size() == 1 newSchema.getDirectives().findAll { it.name == "include" }.size() == 1 }

Open Graph Description: Description When @skip/@include is defined in a SDL, schema generation will create a GraphQLSchema with multiple skip/include definitions. This can cause problems when a schema is serialized and pa...

X Description: Description When @skip/@include is defined in a SDL, schema generation will create a GraphQLSchema with multiple skip/include definitions. This can cause problems when a schema is serialized and pa...

Opengraph URL: https://github.com/graphql-java/graphql-java/pull/2940

X: @github

direct link

Domain: github.com

Links:

Viewport: width=device-width