Skip to content
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

refactor: Added util function jsonTypeRef for simplification of TypeR… #741

Merged
merged 3 commits into from Nov 24, 2021

Conversation

npwork
Copy link
Contributor

@npwork npwork commented Nov 18, 2021

…ef creation (added to examples as well)

Pull request checklist

  • Please read our contributor guide
  • Consider creating a discussion on the discussion forum
    first
  • Make sure the PR doesn't introduce backward compatibility issues
  • Make sure to have sufficient test cases

Pull Request type

  • Bugfix
  • Feature
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Other (please describe):

Changes in this PR

Before:

dgsQueryExecutor.executeAndExtractJsonPathAsObject<List<User>>(
    graphQLQueryRequest.serialize(),
    "data.users[*]",
    object: TypeRef<List<User>>() {}
)

After

dgsQueryExecutor.executeAndExtractJsonPathAsObject<List<User>>(
    graphQLQueryRequest.serialize(),
    "data.users[*]",
    jsonTypeRef()
)

Added util function jsonTypeRef (works the same as Jackson's jacksonTypeRef)

I know it's better suited to put it in com.jayway.jsonpath directly, but unfortunately it's Java project and they won't accept kotlin functions.

Anyway I think jsonTypeRef() is better than this object: TypeRef<List<User>>() {}

Describe the new behavior from this PR, and why it's needed
Issue #

Alternatives considered

Describe alternative implementation you have considered

I can see some usages inside graphql-dgs module. Maybe it makes sense to create graphql-dgs-common to reuse it around.

@berngp
Copy link
Contributor

berngp commented Nov 23, 2021

@npwork the changes look good but the linter failed the build .
Could you please run either make format or ./gradlew formatKotlin ?

@npwork
Copy link
Contributor Author

npwork commented Nov 24, 2021

You are right, will fix.

@berngp berngp merged commit 43fe209 into Netflix:master Nov 24, 2021
@berngp
Copy link
Contributor

berngp commented Nov 24, 2021

Thanks @npwork !

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.

None yet

2 participants