Conversation
dminkovsky
force-pushed
the
async
branch
6 times, most recently
from
40a05de to
b4567bc
Compare
|
Will re-open with much simpler implementation soon. |
| private Map<String, Object> variables = new LinkedHashMap<String, Object>(); | ||
| private Object root; | ||
| private List<GraphQLError> errors = new ArrayList<GraphQLError>(); | ||
| private List<GraphQLError> errors = Collections.synchronizedList(new ArrayList<GraphQLError>()); |
There was a problem hiding this comment.
CopyOnWriteArrayList is pretty efficient list
There was a problem hiding this comment.
I'm not sure Collections.synchronizedList() is good here anyway. Thanks, I'll check out CopyOnWriteArrayList
| this(graphQLSchema, queryStrategy, null); | ||
| } | ||
|
|
||
| public GraphQL(GraphQLSchema graphQLSchema, ExecutionStrategy queryStrategy, ExecutionStrategy mutationStrategy) { |
There was a problem hiding this comment.
The GraphQL spec says that mutations WILL ALWAYS be serial. So why allow people to wire in anything BUT a serial implementation?
I guess it makes it more configureable than today but you have no way of knowing that they are following spec or not.
Maybe its a case of "dont shoot yourself" by wiring in a asynch mutation execution strategy
There was a problem hiding this comment.
My serial implementation is async. The current serial implementation is not async. Both are serial though.
|
BTW you are not far of the mark in terms of implementation. Here is an example from Shopify. Its much like yours in that a Promise (CompletableFuture) is return and understood by the executoinStrategy |
|
I am currently using monix to implement. |
| import java.util.concurrent.CompletionStage; | ||
| import java.util.function.Supplier; | ||
|
|
||
| import static com.spotify.futures.CompletableFutures.successfulAsList; |
There was a problem hiding this comment.
do you really want Spotify as a dependency. I thought graphql-java was trying to be as lightweight as possible
There was a problem hiding this comment.
That's just in there while I work this out. That artifact is tiny (50 LOC?) and can be copy/pasted into this project license-permitting.
There was a problem hiding this comment.
Thanks for taking a look!
3 participants
A Java 8
CompletableFuture-based async execution strategy.The API is:
Data fetchers can return synchronously or asynchronously (by returning a
CompletionState).This strategy requires Java 8, but uses the existing core graphql-java execution API without any changes. It will have to be broken out as a sub-module with its own artifact for Java 8 users wanting to use this strategy.
Still missing:
Still thinking about the tests.
Looking forward to your thoughts. Thank you.