Skip to content

graphql.execute can accept a builder#551

Merged
bbakerman merged 5 commits into
graphql-java:masterfrom
bbakerman:graphql-accepts-builder
Jul 5, 2017
Merged

graphql.execute can accept a builder#551
bbakerman merged 5 commits into
graphql-java:masterfrom
bbakerman:graphql-accepts-builder

Conversation

@bbakerman

Copy link
Copy Markdown
Member

Like the other parts of the code that use a builder, it now takes one as input plus used the builder function pattern

@bbakerman bbakerman added this to the 4.0.0 milestone Jul 5, 2017
@bbakerman bbakerman self-assigned this Jul 5, 2017
@bbakerman bbakerman requested a review from andimarek July 5, 2017 03:21
bbakerman added 2 commits July 5, 2017 13:26
…uilder

# Conflicts:
#	src/main/java/graphql/GraphQL.java
#	src/test/groovy/graphql/GraphQLTest.groovy
* @deprecated Use {@link #execute(ExecutionInput)}
*/
@Deprecated
public ExecutionResult execute(String query) {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is one contentious aspect of this PR.

The most basic way to execute a query (just the string) was deprecated.

50 of our tests are now invalid. It tells me that we need a simple bit of sugar to allow an easy way to get going and then have the ExecutionInput as the way to do it in a more complex way.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

yes, I totally agree: execute(String) should not be deprecated

def builderFunction = { it.query('{hello}')} as UnaryOperator<ExecutionInput.Builder>
def result = GraphQL.newGraphQL(schema).build().execute(builderFunction).data

then:

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

groovy and java closures are not so sweet :(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

yes :(

@andimarek andimarek left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In general of course totally ok, but we do now have in addition executeAsync, and we should support everything for that method too.

def builderFunction = { it.query('{hello}')} as UnaryOperator<ExecutionInput.Builder>
def result = GraphQL.newGraphQL(schema).build().execute(builderFunction).data

then:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

yes :(

bbakerman added 2 commits July 5, 2017 17:16
…uilder

# Conflicts:
#	src/test/groovy/graphql/GraphQLTest.groovy
@bbakerman bbakerman merged commit 5d7d265 into graphql-java:master Jul 5, 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