Skip to content

#434 Add GraphQlInputObjectType reference for NonNull INput types at … #435

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

elielsonms
Copy link
Contributor

Fixes #434 Add GraphQlInputObjectType reference for NonNull INput types at …Runtime Wiring

Checklist

  • Pull requests follows the contribution guide
  • New or modified functionality is covered by tests

Description

  • Added to the determineType the same approach as determineInputType, but without the creation of nonexisting INput types (this could lead to infinity recursion)
  • Added Groovy Unit Test to ensure the problem is solved

…onNull INput types at Runtime Wiring

- Added to the determineType the same approeach as  determineInputType, but without the creation of non existing INput types (this could lead to infinity recursion)
- Added Groovy Unit Test to ensure the problem is solved
Copy link
Member

@vojtapol vojtapol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please check how does this relate to this: #414 ?

@elielsonms
Copy link
Contributor Author

elielsonms commented Nov 18, 2020

Could you please check how does this relate to this: #414 ?

They seems to be similar, but I saw that issue before and looking by its code I wondered it would not solve "my problem". But now I download that branch and ran my Unit test and it failed, so sadly they don't relate. As far as I rechecked in #414 code he's tackling an issue about cyclic reference, which can lead to stackoverflow. And my PR try to solve the problem of at runtime the library graphql-java-extended-validation (but any other "code" could fall into this issue too) is expecting an GraphQlInputObjectType but for Non Null types we just have the GraphQLTypeRefernce for the type that should not be null. This GraphQLTypeRefernce will only be replaced by "its imeplementation" of GraphQlInputObjectType at the end of the Schema parsing, so any DirectiveWiring, that executes in the middle of the Schema parsing, would not have access to the already created GraphQlInputObjectType

@vojtapol vojtapol requested a review from oryan-block December 14, 2020 21:43
Copy link
Collaborator

@oryan-block oryan-block left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. Just a couple minor suggestions.

oryan-block
oryan-block previously approved these changes Dec 16, 2020
@oryan-block oryan-block requested a review from vojtapol December 16, 2020 14:08
oryan-block and others added 3 commits December 17, 2020 17:01
…ing_on_nonnull_input

# Conflicts:
#	src/test/groovy/graphql/kickstart/tools/SchemaParserSpec.groovy
oryan-block
oryan-block previously approved these changes Mar 5, 2021
…ing_on_nonnull_input

# Conflicts:
#	src/test/kotlin/graphql/kickstart/tools/SchemaParserTest.kt
@vojtapol vojtapol merged commit 580475b into graphql-java-kickstart:master Mar 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GraphQLTypeReference instead of proper GraphQlInputObjectType during runtime (Directive wiring) on NonNull Input types
3 participants