FunctionBuilder ignores parameters annotated with GraphQLIgnore

[sgeb]

Currently FunctionBuilder includes all parameters except those annotated with GraphQLContext and those of type DataFetchingEnvironment: https://github.com/ExpediaDotCom/graphql-kotlin/blob/9915070a96b68d36528ee7cf28ba825e1e53dd85/src/main/kotlin/com/expedia/graphql/generator/types/FunctionBuilder.kt#L38-L41

With this present change, FunctionBuilder would also ignore parameters annotated with @GraphQLIgnore.

What follows is my concrete use case in the context of a Spring app, but the change would be convenient in other situations as well.

===

The example in the repo resolves nested queries by virtue of a lateinit var field (see https://github.com/ExpediaDotCom/graphql-kotlin/blob/3132d8851d67f2147f08590207279841a74338ad/example/src/main/kotlin/com/expedia/graphql/sample/query/NestedQueries.kt), with the associated data fetcher component getting resolved from the BeanFactory via a custom SpringDataFetcherFactory. This is quite cumbersome when nested queries require parameters, as seen here: https://github.com/ExpediaDotCom/graphql-kotlin/blob/3132d8851d67f2147f08590207279841a74338ad/example/src/main/kotlin/com/expedia/graphql/sample/query/NestedQueries.kt#L44-L49

A more convenient way would be to provide the nested query as a function in the data class, with bean component parameters annotated with Spring's @Autowired. A custom AutowiringFunctionDataFetcher resolves these annotated parameters to appropriate beans retrieved from the BeanFactory. The resulting data class looks as follows (contrived to match the repo's example):

data class NestedAnimal(
        val id: Int,
        val type: String
) {
    fun details(filter: String, @Autowired @GraphQLIgnore detailsRepository: DetailsRepository): Details {
        // retrieve details via detailsRepository
    }
}

Consequently the logic of the nested query lives in the data class itself instead of a separate DataFetcher class. Note that in my project I don't actually call data repositories directly in the data classes. Instead, the business logic is encapsulated in UseCase classes and the query is merely a call to a specific UseCase or Service (usually a one-liner). In this case having the logic in the data class isn't an issue.

Unfortunately the above approach doesn't currently work due to FunctionBuilder including all parameters, including the @Autowired @GraphQLIgnore parameters. Hence my proposed change.

[codecov]

Codecov Report

Merging #207 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@             Coverage Diff              @@
##             master     #207      +/-   ##
============================================
+ Coverage     98.35%   98.36%   +<.01%     
- Complexity      170      171       +1     
============================================
  Files            55       55              
  Lines           548      549       +1     
  Branches         94       95       +1     
============================================
+ Hits            539      540       +1     
  Misses            4        4              
  Partials          5        5

Impacted Files	Coverage Δ	Complexity Δ
...expedia/graphql/generator/types/FunctionBuilder.kt	100% <100%> (ø)	12 <0> (+1)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1a5e745...38dd4dd. Read the comment docs.

[smyrick]

Nice change! I agree that it makes sense to add even without the context of needing it in Spring

[smyrick]

@sgeb I am releasing 0.2.12 with the changes. https://github.com/ExpediaDotCom/graphql-kotlin/releases/tag/0.2.12

[sgeb]

Great, thanks for the super quick reaction.

BTW, I have a couple of more changes which could make sense to merge upstream. For example, I'm currently working around the fact that com.expedia.graphql.execution.FunctionDataFetcher#mapParameterToValue is private by copying the whole class into my project. The function could instead be protected to allow for easy specialization via inheritance.

Not sure when I'll get to providing a PR though.

[sgeb]

Also, the Spring DI approach for nested queries described above has proven very convenient in my project. Happy to contribute it into this repo's example project. Is that something you'd be interested in? (again, not sure when I'll get to providing a PR though).

[smyrick]

More examples are always helpful and we would welcome any PRs made.

With the changes to FunctionDataFetcher , we intended consumers to use the DataFetcherExecutionPredicate to modify the behaviour after we have mapped the values already.

I.e. You can modify the return value used here: https://github.com/ExpediaDotCom/graphql-kotlin/blob/af9667df8fa8901e64dc11cf56147b01b0ab5dc5/src/main/kotlin/com/expedia/graphql/execution/FunctionDataFetcher.kt#L65

Is there some data that is not available to you that you need?