Skip to content

Conversation

@rrva
Copy link
Contributor

@rrva rrva commented Dec 4, 2019

When using a non-nullable scalar as input type
a regression was introduced by the upgrade of jackson.

A jackson exception occurs internally in the MethodFieldResolver
because the correct deserialiser/serialiser cannot be found.

Somehow this was hidden when using earlier versions of jackson
where this just worked anyhow, but due to changes in jackson this
bug is uncovered. The correct way is of course to fix this in
isScalarType to cover non-nullable scalars, and not rely on
functionality in jackson which no longer seems to be supported.

When using a non-nullable scalar as input type
a regression was introduced by the upgrade of jackson.

A jackson exception occurs internally in the MethodFieldResolver
because the correct deserialiser/serialiser cannot be found.

Somehow this was hidden when using earlier versions of jackson
where this just worked anyhow, but due to changes in jackson this
bug is uncovered. The correct way is of course to fix this in
isScalarType to cover non-nullable scalars, and not rely on
functionality in jackson which no longer seems to be supported.
@rrva
Copy link
Contributor Author

rrva commented Jan 15, 2020

Anything left to do before this could be merged?

@oliemansm
Copy link
Member

Sorry, didn't have time to look at it earlier. Thanks for the contribution!

@oliemansm oliemansm merged commit a5db848 into graphql-java-kickstart:master Jan 15, 2020
adisesha added a commit to adisesha/graphql-java-tools that referenced this pull request Jan 22, 2020
This is fix for graphql-java-kickstart#240, and test for same issue.

When array of non-nullable scalars is input, an error is thrown from Jackson ObjectMapper. The problem then was, NonNullType was ignored by MethodFieldResolver.isScalarType. Changes in graphql-java-kickstart#342 handles NonNullType but it doesn't handle NonNullType.type is ListType.
@oliemansm oliemansm added this to the 6.0.0 milestone Feb 20, 2020
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.

2 participants