Repository navigation
Run directive wirings on the assembled schema - #895
Open
oryan-block wants to merge 4 commits into
Open
oryan-block wants to merge 4 commits into
oryan-block wants to merge 4 commits into
Conversation
…cycles can be rebuilt Also skip types without a definition when wiring, and check unmapped input fields on the wired input objects.
…change self-referencing field types Also drop the input objects parameter of determineType, which can no longer match.
…n lead back to the wired type
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Fixes #875
Checklist
Description
A
SchemaDirectiveWiringon a recursive input field got aGraphQLTypeReferenceas the field's type instead of the input object:So
GraphQLTypeUtil.unwrapAll(element.type)inonInputObjectFieldthrewClassCastException: GraphQLTypeReference cannot be cast to GraphQLUnmodifiedTypeand the schema didn't build. Types are immutable, so one field of a cycle has to stay a reference while the types are built, and we ran the wirings right then.graphql-java runs them only once the schema is assembled, with
SchemaTransformer, so its wirings never see references. This does the same: thecreate*methods inSchemaParserno longer wire anything, andparseSchemaObjectsbuilds the schema, runs the wirings over it (DirectiveWiringHelper#wireSchema) and copies the wired types back intoSchemaObjects. Only when a wiring is registered, without one nothing changes.SchemaTransformercan only rebuild types whose cycles go through a type reference, so a couple of things had to follow:input Filter @described @example(value: { name: "a" })withdirective @example(value: Filter)contains itself through its own applied directive, and a wiring changingFilterfailed with "All types within a GraphQL schema must have unique names". It also means the two-stepSchemaDirectivesbuild is gone, it only existed so wirings saw real input objects in directive arguments.it.type(GraphQLNonNull.nonNull(element.type))onfriend: Personintype Personfails with "This schema is not forming an Directed Acyclic Graph". graphql-java's ownSchemaGeneratorfails on that one, but it worked here before, so I kept it working.Release notes material, these only apply when a wiring is registered unless said otherwise:
GraphQLTypeReferencetoo.InvalidSchemaExceptionfromparseSchemaObjects()and a wiring can't make an invalid schema valid anymore.SchemaDirectiveWiringso it's not affected.SchemaObjectsuntil the schema is built.toSchema()resolves them, so this only matters if you readparseSchemaObjects()directly.Tests: the #843 test in
SchemaParserTestasserted that the recursive field stays a type reference, now it's the input object. New cases inDirectiveTestfor the directive cycle, for wirings changing self-referencing field types (output, input and through a union) and building a type in code, and one inUnmappedInputFieldsTestfor a field removed by a wiring. Each of them failed on an earlier version of this branch, and the directive cycle one passes on master.mvn clean verifypasses on JDK 25 and on JDK 11 against graphql-java 25.1. Also checked through the public API with the issue's schema: master fails with theClassCastException, this branch builds andtreesreturns 4. A second schema with all the shapes above builds on master and gives an identical printed schema and results here, and it also builds and runs on graphql-java 25.1. The existingdirectivesprobe scenario is identical too.