Repository navigation
Build unions that are only members of other unions - #887
Open
oryan-block wants to merge 1 commit into
Open
oryan-block wants to merge 1 commit into
oryan-block wants to merge 1 commit into
Conversation
The scanner only expanded a nested union that some field returned, so a union listed only as a member of another union failed with "No object type found". It now discovers those unions too, so they build the same way as when a field returns them.
|
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 #866
Checklist
Description
A union that's only listed as a member of another union failed to build:
It built once some field returned
Media.SchemaClassScanner#getAllObjectTypeMembersOfDiscoveredUnionsonly skips member unions that are already in the dictionary, andSchemaParser#getLeafUnionObjectsonly flattens unions the scanner found. So just skippingMediain the scanner would have moved the error to the parser.So the scanner loop has a new step,
handleNestedUnions, that adds the unions listed by discovered unions to the dictionary without a class, likehandleInterfaceFieldTypesdoes for unions only used by interface fields. The existing union step then requires their members. The schema comes out the same as when a field returnsMedia:ResultisBook | MovieandMediais its own union. The step only finds something when a member union isn't in the dictionary yet, which is exactly when master throws, so schemas that build today don't change.Nested unions aren't in the spec, but they've been supported since #83 and
EndToEndTestcovers them, so I fixed them instead of rejecting them.The nested union test in
TypeExtensionTesthad abarBaz: BarBazfield only to get around this. It's gone now, and the test has a second level of nesting, since with one level it passes even if the loop doesn't restart after the new step. Checked that it fails both without the step (the error above) and without the restart.mvn clean verifypasses on JDK 25, on JDK 25 against graphql-java 25.1, and on JDK 11. Also built the issue's schema through the public API: it fails on master with the error above, and with this change it builds andresultsreturns both aBookand aMovieon graphql-java 26.1 and 25.1, with no warnings.Unions that list each other still end in a
StackOverflowErroringetLeafUnionObjects. That's #886. Before this change, the scanner failed first when no field returned the inner union, so that case now overflows too.