Change scope of FunctionDataFetcher methods #588
Conversation
In ExpediaGroup#582 we opened some method of the FunctionDataFetcher so we could run some custom behaviour. Looking back at that change I realize that we only need to make the methods protected, not open, and that we can hide some others still as private. We just need to expose all the methods that we call in the basic implementation of get as protected so we can all those from custom implementations.
|
This is a major change (from 2.0.0-RC5) but there are no full releases out there that will make this backwards incompatible |
smyrick
added
type: refactor
| @@ -98,7 +98,7 @@ open class FunctionDataFetcher( | |||
| * | |||
| * This is currently achieved by using a Jackson [ObjectMapper]. | |||
| */ | |||
| protected open fun convertParameterValue(param: KParameter, environment: DataFetchingEnvironment): Any? { | |||
| private fun convertParameterValue(param: KParameter, environment: DataFetchingEnvironment): Any? { | |||
There was a problem hiding this comment.
by keeping it open we could allow others to implement some custom logic here, e.g. how to handle file uploads or skip converting the type if underlying object is already a correct type
There was a problem hiding this comment.
Did not think about that possibility. Let's discuss in more about both on the thread below.
| @@ -86,7 +86,7 @@ open class FunctionDataFetcher( | |||
| * | |||
| * - The entire environment is returned if the parameter is of type [DataFetchingEnvironment] | |||
| */ | |||
| protected open fun mapParameterToValue(param: KParameter, environment: DataFetchingEnvironment): Any? = | |||
| private fun mapParameterToValue(param: KParameter, environment: DataFetchingEnvironment): Any? = | |||
There was a problem hiding this comment.
by keeping it open one could technically implement some other autowired object besides GraphQLContext and DataFetchingEnvironment
There was a problem hiding this comment.
That is interesting. Do we need to leave this method in particular open or do we just have them re-implement get and call their own custom method?
There was a problem hiding this comment.
Since other methods are already open I'd keep those open as well. Once (or IF) we implement Kotlin native way of executing the queries (and drop the dependency on CompletableFuture) we could change the interface method signatures to something more appropriate.
|
I will close this in favor of updating docs |
📝 Description
In #582 we opened some method of the FunctionDataFetcher so we could run some custom behaviour. Looking back at that change I realize that we only need to make the methods protected, not open, and that we can hide some others still as private. We just need to expose all the methods that we call in the basic implementation of get as protected so we can all those from custom implementations.
🔗 Related Issues
#582