Skip to content

Conversation

@bbakerman
Copy link
Member

Adds a level of indirection into the data fetchers which are specified at
schema def time but needed "wired" at query execution time

see #734

Adds a level of indirection into the data fetchers which are specified at
schema def time but needed "wired" at query execution time
@bbakerman bbakerman added this to the 5.0 milestone Sep 25, 2017
*
* @return a data fetcher factory that always returns the provided data fetcher
*/
static <DF> DataFetcherFactory<DF> useDataFetcher(DataFetcher<DF> dataFetcher) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not really sure about that one: Can we the static factory method to somewhere else?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

public GraphQLFieldDefinition(String name, String description, GraphQLOutputType type, DataFetcherFactory dataFetcherFactory, List<GraphQLArgument> arguments, String deprecationReason, FieldDefinition definition) {
assertValidName(name);
assertNotNull(dataFetcher, "dataFetcher can't be null");
assertNotNull(dataFetcherFactory, "dataFetcherFactory can't be null");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is confusing I think: until know we only have DataFetcher ... no we say DataFetcherFactory can't be null: maybe something like 'you have to provide a DataFetcher (or DataFetcherFactory)' ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

*
* @return a data fetcher
*/
DataFetcher<T> get(GraphQLFieldDefinition fieldDefinition);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is safer for the future provide an ....Environment Object containing the fieldDefitinion, but also the parent "container".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great point - on it

…factory-support

# Conflicts:
#	src/main/java/graphql/schema/GraphQLFieldDefinition.java
@bbakerman bbakerman merged commit bb2df57 into graphql-java:master Oct 2, 2017
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