BREAKING CHANGE: [spring-server] default data fetcher should use Jackson object mapper from spring context #525
Conversation
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
… from spring context Currently it is not possible to provide custom Jackson object mapper (e.g. one that includes java date time module) to be used by default function data fetcher. In order to use custom object mapper users have to create their own instance of data fetcher factory provider. This change updates the data fetcher factory provider to accept an instance of object mapper that will be used by the function data fetcher. Resolves: ExpediaGroup#524
|
LGTM |
smyrick
added
changes: minor
|
Is this ready for review? Was there some other feature you were looking to add? |
Looking into adding some unit test for this. |
Previous default implmenetation is now renamed to SimpleKotlinDataFetcherFactoryProvider.
|
@dariuszkuc This look good to me. We have the single test. Is this good to merge? |
|
@smyrick this should be good to merge - was looking into whether we need to have that jackson object mapper in the first place and unfortunately it is required in order to process any complex input objects. On the side note - currently we do an extra conversion from primitive scalar value to the same scalar type (as noted here). Unsure how big of a perf impact that is but we might want to optimize it as well. |
smyrick
approved these changes
Dec 27, 2019
smyrick
added
changes: major
|
Major version changes now that we changed from class to interface |
This was referenced Dec 29, 2019
smyrick
changed the title
dariuszkuc
added a commit
to dariuszkuc/graphql-kotlin
that referenced
this pull request
Aug 5, 2022
… from spring context (ExpediaGroup#525) * [spring-server] default data fetcher should use Jackson object mapper from spring context Currently it is not possible to provide custom Jackson object mapper (e.g. one that includes java date time module) to be used by default function data fetcher. In order to use custom object mapper users have to create their own instance of data fetcher factory provider. This change updates the data fetcher factory provider to accept an instance of object mapper that will be used by the function data fetcher. Resolves: ExpediaGroup#524 * unit test for verifying proper object mapper is used * convert KotlinDataFetcherFactoryProvider to interface Previous default implmenetation is now renamed to SimpleKotlinDataFetcherFactoryProvider. * add integration test to verify custom jackson bindings work with function data fetcher * remove unnecessary val definition
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.
📝 Description
Currently it is not possible to provide custom Jackson object mapper (e.g. one that includes java date time module) to be used by default function data fetcher. In order to use custom object mapper users have to create their own instance of data fetcher factory provider. This change updates the data fetcher factory provider to accept an instance of object mapper that will be used by the function data fetcher.
🔗 Related Issues
Resolves: #524