Request-scoped DataLoader about graphql-java-servlet HOT 11 CLOSED

Will this PR help? #83.

from graphql-java-servlet.

@vojtechhabarta 's diff could help the GraphQLServlet to have per-request instrumentation.
Besides that, we need to add the dataloader into instrumentation.
To enable dataloader per request, GraphQLContext could be the best place to instantiate the dataloaders inside.

Ideas:

1. Add DataLoaderRegistry into the GraphQLContext.

public class GraphQLContext {
....
protected Optional dataloaderRegistry;
public GraphQLContext(Optional request, Optional response, Optional dataloderRegistry) {
this.request = request;
this.response = response;
this.dataloaderRegistry = dataloaderRegistry;
}
....
}

2. Create a new class ContextA to inherit the GraphQLContext:

public class ContextA extends GraphQLContext{
public ContextA() {
...
Dataloader<Object, Object> dataloaderA = new Dataloader<>();
DataLoaderRegistry registry = new DataLoaderRegistry();
this.registry.put("A", dataloaderA)
}

public CompleteFuture<A> loadAValue(Object key) {
	return this.registry.get("A").get(Key);
}

}

3. Create a new class InstrumentationProviderA to inherit the InstrumentationProvider (based on @vojtechhabarta 's diff), then be able to connect the dataloader with the object .
   public class InstrumentationProviderA implements InstrumentationProvider {
   public Instrumentation getInstrumentation(GraphQLContext context) {
   DataLoaderDispatcherInstrumentation dispatcherInstrumentation = new DataLoaderDispatcherInstrumentation(context.dataloaderRegistry);
   return dispatcherInstrumentation;
   }
   }

from graphql-java-servlet.

Would it make sense to always add the DataLoaderDispatcherInstrumentation when the context has a DataLoaderRegistry? i.e. should it pass the context to the InstrumentationProvider or should it get the instrumentation provided by the InstrumentationProvider and then add the DataLoaderDispatcherInstrumentation to it automatically?

from graphql-java-servlet.

Always adding DataLoaderDispatcherInstrumentation when the context has a DataLoaderRegistry would probably make sense, I think. But I don't know what exactly you mean. Something similar to what I tried in mentioned commit? In 60f1093 I added GraphQLContext parameter to InstrumentationProvider.getInstrumentation() method.

Or are you thinking about supporting DataLoaderDispatcherInstrumentation automatically and adding DataLoaderRegistry field to GraphQLContext?

from graphql-java-servlet.

I would like to solve this problem somehow.

I have 2 use cases:

• Objects that can be cached regardless the user accessing the data for some amount of time. For this use case current support for DataLoaders is sufficient, moreover this can also be solved using arbitrary cache.
• Object that can be cached only per user request. Here I cannot use normal cache. Since the structure is relatively complex and one object can appear on different levels of result it would be really beneficial to have per-request cache.

Any idea, solution, hack...?

from graphql-java-servlet.

@vojtechhabarta, @freedomljc I LOVE your ideas to implement "instrumentation per request" .

Our project is using @deprecated BatchedExecutionStrategy we want to change to use DataLoader but stuck at configure DataLoader per request.

Have you guys tried to implement it?

from graphql-java-servlet.

I haven't implemented it. I thought somebody who understands better than me how GraphQL in Java works should do it 😏

from graphql-java-servlet.

@ashu-walmart, hopefully your PR is reviewed and merged soon.

from graphql-java-servlet.

I make a pull request graphql-java/graphql-java#1171 for this

with Supplier<DataLoaderRegistry>, DataLoaderDispatcherInstrumentation can get a request-scoped DataLoaderRegistry and FieldLevelTrackingApproach

from graphql-java-servlet.

@ashu-walmart et al. Anything still needed for this in this project? PR #83 has been merged. So like to know if there's still some functionality missing we need to implement. Also, if you're able to write a small readme explaining this feature for future reference would be much appreciated.

from graphql-java-servlet.

@oliemansm, This works for me now and am using the feature in my project. I will write a readme describing the feature. Thanks!

from graphql-java-servlet.