diff --git a/gradle.properties b/gradle.properties index 636bbca9..d306cb83 100644 --- a/gradle.properties +++ b/gradle.properties @@ -7,7 +7,7 @@ PROJECT_LICENSE=MIT PROJECT_LICENSE_URL=https://github.com/graphql-java-kickstart/spring-java-servlet/blob/master/LICENSE.md PROJECT_DEV_ID=oliemansm PROJECT_DEV_NAME=Michiel Oliemans -LIB_GRAPHQL_JAVA_VER=21.3 +LIB_GRAPHQL_JAVA_VER=22.1 LIB_JACKSON_VER=2.16.1 LIB_SLF4J_VER=2.0.12 LIB_LOMBOK_VER=1.18.30 diff --git a/graphql-java-kickstart/src/main/java/graphql/kickstart/execution/BatchedDataLoaderGraphQLBuilder.java b/graphql-java-kickstart/src/main/java/graphql/kickstart/execution/BatchedDataLoaderGraphQLBuilder.java index c1d843f7..b79c21cc 100644 --- a/graphql-java-kickstart/src/main/java/graphql/kickstart/execution/BatchedDataLoaderGraphQLBuilder.java +++ b/graphql-java-kickstart/src/main/java/graphql/kickstart/execution/BatchedDataLoaderGraphQLBuilder.java @@ -1,48 +1,20 @@ package graphql.kickstart.execution; -import graphql.ExecutionInput; import graphql.GraphQL; -import graphql.execution.instrumentation.Instrumentation; -import graphql.execution.instrumentation.dataloader.DataLoaderDispatcherInstrumentationOptions; import graphql.kickstart.execution.config.GraphQLBuilder; import graphql.kickstart.execution.input.GraphQLBatchedInvocationInput; import graphql.kickstart.execution.input.GraphQLSingleInvocationInput; -import java.util.List; -import java.util.function.Supplier; public class BatchedDataLoaderGraphQLBuilder { - private final Supplier optionsSupplier; - - public BatchedDataLoaderGraphQLBuilder( - Supplier optionsSupplier) { - if (optionsSupplier != null) { - this.optionsSupplier = optionsSupplier; - } else { - this.optionsSupplier = DataLoaderDispatcherInstrumentationOptions::newOptions; - } - } - GraphQL newGraphQL(GraphQLBatchedInvocationInput invocationInput, GraphQLBuilder graphQLBuilder) { - Supplier supplier = - augment(invocationInput, graphQLBuilder.getInstrumentationSupplier()); return invocationInput.getInvocationInputs().stream() .findFirst() .map(GraphQLSingleInvocationInput::getSchema) - .map(schema -> graphQLBuilder.build(schema, supplier)) + .map(schema -> graphQLBuilder.build(schema, graphQLBuilder.getInstrumentationSupplier())) .orElseThrow( () -> new IllegalArgumentException( "Batched invocation input must contain at least one query")); } - - private Supplier augment( - GraphQLBatchedInvocationInput batchedInvocationInput, - Supplier instrumentationSupplier) { - List executionInputs = batchedInvocationInput.getExecutionInputs(); - return batchedInvocationInput - .getContextSetting() - .configureInstrumentationForContext( - instrumentationSupplier, executionInputs, optionsSupplier.get()); - } } diff --git a/graphql-java-kickstart/src/main/java/graphql/kickstart/execution/GraphQLQueryInvoker.java b/graphql-java-kickstart/src/main/java/graphql/kickstart/execution/GraphQLQueryInvoker.java index b03f2ec8..601d6505 100644 --- a/graphql-java-kickstart/src/main/java/graphql/kickstart/execution/GraphQLQueryInvoker.java +++ b/graphql-java-kickstart/src/main/java/graphql/kickstart/execution/GraphQLQueryInvoker.java @@ -3,7 +3,6 @@ import graphql.execution.instrumentation.ChainedInstrumentation; import graphql.execution.instrumentation.Instrumentation; import graphql.execution.instrumentation.SimplePerformantInstrumentation; -import graphql.execution.instrumentation.dataloader.DataLoaderDispatcherInstrumentationOptions; import graphql.execution.preparsed.NoOpPreparsedDocumentProvider; import graphql.execution.preparsed.PreparsedDocumentProvider; import graphql.kickstart.execution.config.DefaultExecutionStrategyProvider; @@ -20,17 +19,14 @@ public class GraphQLQueryInvoker { private final Supplier getExecutionStrategyProvider; private final Supplier getInstrumentation; private final Supplier getPreparsedDocumentProvider; - private final Supplier optionsSupplier; protected GraphQLQueryInvoker( Supplier getExecutionStrategyProvider, Supplier getInstrumentation, - Supplier getPreparsedDocumentProvider, - Supplier optionsSupplier) { + Supplier getPreparsedDocumentProvider) { this.getExecutionStrategyProvider = getExecutionStrategyProvider; this.getInstrumentation = getInstrumentation; this.getPreparsedDocumentProvider = getPreparsedDocumentProvider; - this.optionsSupplier = optionsSupplier; } public static Builder newBuilder() { @@ -43,7 +39,7 @@ public GraphQLInvoker toGraphQLInvoker() { .executionStrategyProvider(getExecutionStrategyProvider) .instrumentation(getInstrumentation) .preparsedDocumentProvider(getPreparsedDocumentProvider); - return new GraphQLInvoker(graphQLBuilder, new BatchedDataLoaderGraphQLBuilder(optionsSupplier)); + return new GraphQLInvoker(graphQLBuilder, new BatchedDataLoaderGraphQLBuilder()); } public static class Builder { @@ -54,9 +50,6 @@ public static class Builder { () -> SimplePerformantInstrumentation.INSTANCE; private Supplier getPreparsedDocumentProvider = () -> NoOpPreparsedDocumentProvider.INSTANCE; - private Supplier - dataLoaderDispatcherInstrumentationOptionsSupplier = - DataLoaderDispatcherInstrumentationOptions::newOptions; public Builder withExecutionStrategyProvider(ExecutionStrategyProvider provider) { return withExecutionStrategyProvider(() -> provider); @@ -97,23 +90,11 @@ public Builder withPreparsedDocumentProvider(Supplier return this; } - public Builder withDataLoaderDispatcherInstrumentationOptions( - DataLoaderDispatcherInstrumentationOptions options) { - return withDataLoaderDispatcherInstrumentationOptions(() -> options); - } - - public Builder withDataLoaderDispatcherInstrumentationOptions( - Supplier supplier) { - this.dataLoaderDispatcherInstrumentationOptionsSupplier = supplier; - return this; - } - public GraphQLQueryInvoker build() { return new GraphQLQueryInvoker( getExecutionStrategyProvider, getInstrumentation, - getPreparsedDocumentProvider, - dataLoaderDispatcherInstrumentationOptionsSupplier); + getPreparsedDocumentProvider); } } } diff --git a/graphql-java-kickstart/src/main/java/graphql/kickstart/execution/config/GraphQLBuilder.java b/graphql-java-kickstart/src/main/java/graphql/kickstart/execution/config/GraphQLBuilder.java index aa43deaa..3f898f43 100644 --- a/graphql-java-kickstart/src/main/java/graphql/kickstart/execution/config/GraphQLBuilder.java +++ b/graphql-java-kickstart/src/main/java/graphql/kickstart/execution/config/GraphQLBuilder.java @@ -2,10 +2,8 @@ import graphql.GraphQL; import graphql.execution.ExecutionStrategy; -import graphql.execution.instrumentation.ChainedInstrumentation; import graphql.execution.instrumentation.Instrumentation; import graphql.execution.instrumentation.SimplePerformantInstrumentation; -import graphql.execution.instrumentation.dataloader.DataLoaderDispatcherInstrumentation; import graphql.execution.preparsed.NoOpPreparsedDocumentProvider; import graphql.execution.preparsed.PreparsedDocumentProvider; import graphql.schema.GraphQLSchema; @@ -89,18 +87,7 @@ public GraphQL build( Instrumentation instrumentation = configuredInstrumentationSupplier.get(); builder.instrumentation(instrumentation); - if (containsDispatchInstrumentation(instrumentation)) { - builder.doNotAddDefaultInstrumentations(); - } graphQLBuilderConfigurerSupplier.get().configure(builder); return builder.build(); } - - private boolean containsDispatchInstrumentation(Instrumentation instrumentation) { - if (instrumentation instanceof ChainedInstrumentation) { - return ((ChainedInstrumentation) instrumentation) - .getInstrumentations().stream().anyMatch(this::containsDispatchInstrumentation); - } - return instrumentation instanceof DataLoaderDispatcherInstrumentation; - } } diff --git a/graphql-java-kickstart/src/main/java/graphql/kickstart/execution/context/ContextSetting.java b/graphql-java-kickstart/src/main/java/graphql/kickstart/execution/context/ContextSetting.java index c9d1060f..2a18dd09 100644 --- a/graphql-java-kickstart/src/main/java/graphql/kickstart/execution/context/ContextSetting.java +++ b/graphql-java-kickstart/src/main/java/graphql/kickstart/execution/context/ContextSetting.java @@ -1,23 +1,12 @@ package graphql.kickstart.execution.context; -import graphql.ExecutionInput; -import graphql.execution.ExecutionId; -import graphql.execution.instrumentation.ChainedInstrumentation; -import graphql.execution.instrumentation.Instrumentation; -import graphql.execution.instrumentation.dataloader.DataLoaderDispatcherInstrumentationOptions; import graphql.kickstart.execution.GraphQLRequest; import graphql.kickstart.execution.input.GraphQLBatchedInvocationInput; import graphql.kickstart.execution.input.PerQueryBatchedInvocationInput; import graphql.kickstart.execution.input.PerRequestBatchedInvocationInput; -import graphql.kickstart.execution.instrumentation.ConfigurableDispatchInstrumentation; -import graphql.kickstart.execution.instrumentation.FieldLevelTrackingApproach; -import graphql.kickstart.execution.instrumentation.RequestLevelTrackingApproach; import graphql.schema.GraphQLSchema; -import java.util.Arrays; import java.util.List; import java.util.function.Supplier; -import java.util.stream.Collectors; -import org.dataloader.DataLoaderRegistry; /** * An enum representing possible context settings. These are modeled after Apollo's link settings. @@ -28,11 +17,9 @@ public enum ContextSetting { * A context object, and therefor dataloader registry and subject, should be shared between all * GraphQL executions in a http request. */ - PER_REQUEST_WITH_INSTRUMENTATION, - PER_REQUEST_WITHOUT_INSTRUMENTATION, + PER_REQUEST, /** Each GraphQL execution should always have its own context. */ - PER_QUERY_WITH_INSTRUMENTATION, - PER_QUERY_WITHOUT_INSTRUMENTATION; + PER_QUERY; /** * Creates a set of inputs with the correct context based on the setting. @@ -50,62 +37,12 @@ public GraphQLBatchedInvocationInput getBatch( Supplier contextSupplier, Object root) { switch (this) { - case PER_QUERY_WITH_INSTRUMENTATION: - // Intentional fallthrough - case PER_QUERY_WITHOUT_INSTRUMENTATION: + case PER_QUERY: return new PerQueryBatchedInvocationInput(requests, schema, contextSupplier, root, this); - case PER_REQUEST_WITHOUT_INSTRUMENTATION: - // Intentional fallthrough - case PER_REQUEST_WITH_INSTRUMENTATION: + case PER_REQUEST: return new PerRequestBatchedInvocationInput(requests, schema, contextSupplier, root, this); default: throw new ContextSettingNotConfiguredException(); } } - - /** - * Augments the provided instrumentation supplier to also supply the correct dispatching - * instrumentation. - * - * @param instrumentation the instrumentation supplier to augment - * @param executionInputs the inputs that will be dispatched by the instrumentation - * @param options the DataLoader dispatching instrumentation options that will be used. - * @return augmented instrumentation supplier. - */ - public Supplier configureInstrumentationForContext( - Supplier instrumentation, - List executionInputs, - DataLoaderDispatcherInstrumentationOptions options) { - ConfigurableDispatchInstrumentation dispatchInstrumentation; - switch (this) { - case PER_REQUEST_WITH_INSTRUMENTATION: - DataLoaderRegistry registry = - executionInputs.stream() - .findFirst() - .map(ExecutionInput::getDataLoaderRegistry) - .orElseThrow(IllegalArgumentException::new); - List executionIds = - executionInputs.stream() - .map(ExecutionInput::getExecutionId) - .collect(Collectors.toList()); - RequestLevelTrackingApproach requestTrackingApproach = - new RequestLevelTrackingApproach(executionIds, registry); - dispatchInstrumentation = - new ConfigurableDispatchInstrumentation( - options, (dataLoaderRegistry -> requestTrackingApproach)); - break; - case PER_QUERY_WITH_INSTRUMENTATION: - dispatchInstrumentation = - new ConfigurableDispatchInstrumentation(options, FieldLevelTrackingApproach::new); - break; - case PER_REQUEST_WITHOUT_INSTRUMENTATION: - // Intentional fallthrough - case PER_QUERY_WITHOUT_INSTRUMENTATION: - return instrumentation; - default: - throw new ContextSettingNotConfiguredException(); - } - return () -> - new ChainedInstrumentation(Arrays.asList(dispatchInstrumentation, instrumentation.get())); - } } diff --git a/graphql-java-kickstart/src/main/java/graphql/kickstart/execution/instrumentation/AbstractTrackingApproach.java b/graphql-java-kickstart/src/main/java/graphql/kickstart/execution/instrumentation/AbstractTrackingApproach.java index 0a4a3e74..20e821ba 100644 --- a/graphql-java-kickstart/src/main/java/graphql/kickstart/execution/instrumentation/AbstractTrackingApproach.java +++ b/graphql-java-kickstart/src/main/java/graphql/kickstart/execution/instrumentation/AbstractTrackingApproach.java @@ -45,7 +45,7 @@ public ExecutionStrategyInstrumentationContext beginExecutionStrategy( return new ExecutionStrategyInstrumentationContext() { @Override - public void onDispatched(CompletableFuture result) { + public void onDispatched() { // default empty implementation } @@ -110,7 +110,7 @@ public InstrumentationContext beginFieldFetch( return new InstrumentationContext() { @Override - public void onDispatched(CompletableFuture result) { + public void onDispatched() { synchronized (stack) { stack.increaseFetchCount(executionId, level); stack.setStatus(executionId, dispatchIfNeeded(stack, executionId, level)); diff --git a/graphql-java-kickstart/src/main/java/graphql/kickstart/execution/instrumentation/ConfigurableDispatchInstrumentation.java b/graphql-java-kickstart/src/main/java/graphql/kickstart/execution/instrumentation/ConfigurableDispatchInstrumentation.java deleted file mode 100644 index 477071be..00000000 --- a/graphql-java-kickstart/src/main/java/graphql/kickstart/execution/instrumentation/ConfigurableDispatchInstrumentation.java +++ /dev/null @@ -1,199 +0,0 @@ -package graphql.kickstart.execution.instrumentation; - -import graphql.ExecutionResult; -import graphql.ExecutionResultImpl; -import graphql.execution.AsyncExecutionStrategy; -import graphql.execution.ExecutionContext; -import graphql.execution.ExecutionStrategy; -import graphql.execution.instrumentation.ExecutionStrategyInstrumentationContext; -import graphql.execution.instrumentation.InstrumentationContext; -import graphql.execution.instrumentation.InstrumentationState; -import graphql.execution.instrumentation.SimpleInstrumentationContext; -import graphql.execution.instrumentation.dataloader.DataLoaderDispatcherInstrumentation; -import graphql.execution.instrumentation.dataloader.DataLoaderDispatcherInstrumentationOptions; -import graphql.execution.instrumentation.parameters.InstrumentationCreateStateParameters; -import graphql.execution.instrumentation.parameters.InstrumentationExecuteOperationParameters; -import graphql.execution.instrumentation.parameters.InstrumentationExecutionParameters; -import graphql.execution.instrumentation.parameters.InstrumentationExecutionStrategyParameters; -import graphql.execution.instrumentation.parameters.InstrumentationFieldFetchParameters; -import graphql.language.OperationDefinition; -import graphql.schema.DataFetcher; -import java.util.Collections; -import java.util.LinkedHashMap; -import java.util.Map; -import java.util.concurrent.CompletableFuture; -import java.util.function.Function; -import lombok.extern.slf4j.Slf4j; -import org.dataloader.DataLoader; -import org.dataloader.DataLoaderRegistry; -import org.dataloader.stats.Statistics; - -@Slf4j -public class ConfigurableDispatchInstrumentation extends DataLoaderDispatcherInstrumentation { - - private final DataLoaderDispatcherInstrumentationOptions options; - - private final Function approachFunction; - - /** Creates a DataLoaderDispatcherInstrumentation with the default options */ - public ConfigurableDispatchInstrumentation( - Function approachFunction) { - this(DataLoaderDispatcherInstrumentationOptions.newOptions(), approachFunction); - } - - /** - * Creates a DataLoaderDispatcherInstrumentation with the specified options - * - * @param options the options to control the behaviour - */ - public ConfigurableDispatchInstrumentation( - DataLoaderDispatcherInstrumentationOptions options, - Function approachFunction) { - this.options = options; - this.approachFunction = approachFunction; - } - - @Override - public InstrumentationState createState(InstrumentationCreateStateParameters parameters) { - DataLoaderRegistry registry = parameters.getExecutionInput().getDataLoaderRegistry(); - return new DataLoaderDispatcherInstrumentationState( - registry, - approachFunction.apply(registry), - parameters.getExecutionInput().getExecutionId()); - } - - @Override - public DataFetcher instrumentDataFetcher( - DataFetcher dataFetcher, - InstrumentationFieldFetchParameters parameters, - InstrumentationState instrumentationState) { - DataLoaderDispatcherInstrumentationState state = - InstrumentationState.ofState(instrumentationState); - if (state.isAggressivelyBatching()) { - return dataFetcher; - } - // - // currently only AsyncExecutionStrategy with DataLoader and hence this allows us to "dispatch" - // on every object if it's not using aggressive batching for other execution strategies - // which allows them to work if used. - return (DataFetcher) - environment -> { - Object obj = dataFetcher.get(environment); - doImmediatelyDispatch(state); - return obj; - }; - } - - private void doImmediatelyDispatch(DataLoaderDispatcherInstrumentationState state) { - state.getApproach().dispatch(); - } - - @Override - public InstrumentationContext beginExecuteOperation( - InstrumentationExecuteOperationParameters parameters, - InstrumentationState instrumentationState) { - if (!isDataLoaderCompatible(parameters.getExecutionContext())) { - DataLoaderDispatcherInstrumentationState state = - InstrumentationState.ofState(instrumentationState); - state.setAggressivelyBatching(false); - } - return SimpleInstrumentationContext.noOp(); - } - - private boolean isDataLoaderCompatible(ExecutionContext executionContext) { - // - // currently we only support Query operations and ONLY with AsyncExecutionStrategy as the query - // ES - // This may change in the future but this is the fix for now - // - if (executionContext.getOperationDefinition().getOperation() - == OperationDefinition.Operation.QUERY) { - ExecutionStrategy queryStrategy = executionContext.getQueryStrategy(); - return (queryStrategy instanceof AsyncExecutionStrategy); - } - return false; - } - - @Override - public ExecutionStrategyInstrumentationContext beginExecutionStrategy( - InstrumentationExecutionStrategyParameters parameters, - InstrumentationState instrumentationState) { - DataLoaderDispatcherInstrumentationState state = - InstrumentationState.ofState(instrumentationState); - // - // if there are no data loaders, there is nothing to do - // - if (state.hasNoDataLoaders()) { - return new ExecutionStrategyInstrumentationContext() { - @Override - public void onDispatched(CompletableFuture result) { - // default empty implementation - } - - @Override - public void onCompleted(ExecutionResult result, Throwable t) { - // default empty implementation - } - }; - } - return state.getApproach().beginExecutionStrategy(parameters); - } - - @Override - public InstrumentationContext beginFieldFetch( - InstrumentationFieldFetchParameters parameters, InstrumentationState instrumentationState) { - DataLoaderDispatcherInstrumentationState state = - InstrumentationState.ofState(instrumentationState); - // - // if there are no data loaders, there is nothing to do - // - if (state.hasNoDataLoaders()) { - return SimpleInstrumentationContext.noOp(); - } - return state.getApproach().beginFieldFetch(parameters); - } - - @Override - public CompletableFuture instrumentExecutionResult( - ExecutionResult executionResult, - InstrumentationExecutionParameters parameters, - InstrumentationState instrumentationState) { - DataLoaderDispatcherInstrumentationState state = - InstrumentationState.ofState(instrumentationState); - state.getApproach().removeTracking(parameters.getExecutionInput().getExecutionId()); - if (!options.isIncludeStatistics()) { - return CompletableFuture.completedFuture(executionResult); - } else { - Map currentExt = executionResult.getExtensions(); - Map statsMap = - new LinkedHashMap<>(currentExt == null ? Collections.emptyMap() : currentExt); - Map dataLoaderStats = buildStatisticsMap(state); - statsMap.put("dataloader", dataLoaderStats); - - log.debug("Data loader stats : {}", dataLoaderStats); - - return CompletableFuture.completedFuture( - new ExecutionResultImpl( - executionResult.getData(), executionResult.getErrors(), statsMap)); - } - } - - private Map buildStatisticsMap(DataLoaderDispatcherInstrumentationState state) { - DataLoaderRegistry dataLoaderRegistry = state.getDataLoaderRegistry(); - Statistics allStats = dataLoaderRegistry.getStatistics(); - Map statsMap = new LinkedHashMap<>(); - statsMap.put("overall-statistics", allStats.toMap()); - - Map individualStatsMap = new LinkedHashMap<>(); - - for (String dlKey : dataLoaderRegistry.getKeys()) { - DataLoader dl = dataLoaderRegistry.getDataLoader(dlKey); - Statistics statistics = dl.getStatistics(); - individualStatsMap.put(dlKey, statistics.toMap()); - } - - statsMap.put("individual-statistics", individualStatsMap); - - return statsMap; - } -} diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/GraphQLConfiguration.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/GraphQLConfiguration.java index f009889e..46dd9042 100644 --- a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/GraphQLConfiguration.java +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/GraphQLConfiguration.java @@ -140,7 +140,7 @@ public static class Builder { private List listeners = new ArrayList<>(); private long subscriptionTimeout = 0; private long asyncTimeout = 30000; - private ContextSetting contextSetting = ContextSetting.PER_QUERY_WITH_INSTRUMENTATION; + private ContextSetting contextSetting = ContextSetting.PER_QUERY; private Supplier batchInputPreProcessorSupplier = NoOpBatchInputPreProcessor::new; private GraphQLResponseCacheManager responseCacheManager; diff --git a/graphql-java-servlet/src/test/groovy/graphql/kickstart/servlet/DataLoaderDispatchingSpec.groovy b/graphql-java-servlet/src/test/groovy/graphql/kickstart/servlet/DataLoaderDispatchingSpec.groovy index 89f1abe1..b8c7791b 100644 --- a/graphql-java-servlet/src/test/groovy/graphql/kickstart/servlet/DataLoaderDispatchingSpec.groovy +++ b/graphql-java-servlet/src/test/groovy/graphql/kickstart/servlet/DataLoaderDispatchingSpec.groovy @@ -1,15 +1,12 @@ package graphql.kickstart.servlet import com.fasterxml.jackson.databind.ObjectMapper -import graphql.ExecutionInput import graphql.execution.instrumentation.ChainedInstrumentation import graphql.execution.instrumentation.Instrumentation import graphql.execution.instrumentation.SimplePerformantInstrumentation -import graphql.execution.instrumentation.dataloader.DataLoaderDispatcherInstrumentationOptions import graphql.kickstart.execution.context.ContextSetting import graphql.kickstart.execution.context.DefaultGraphQLContext import graphql.kickstart.execution.context.GraphQLKickstartContext -import graphql.kickstart.execution.instrumentation.ConfigurableDispatchInstrumentation import graphql.kickstart.servlet.context.GraphQLServletContextBuilder import graphql.schema.DataFetcher import graphql.schema.DataFetchingEnvironment @@ -120,14 +117,9 @@ class DataLoaderDispatchingSpec extends Specification { mapper.readValue(response.getContentAsByteArray(), List) } - Instrumentation simpleInstrumentation = new SimplePerformantInstrumentation() - ChainedInstrumentation chainedInstrumentation = new ChainedInstrumentation(Collections.singletonList(simpleInstrumentation)) - def simpleSupplier = { simpleInstrumentation } - def chainedSupplier = { chainedInstrumentation } - def "batched query with per query context does not batch loads together"() { setup: - configureServlet(ContextSetting.PER_QUERY_WITH_INSTRUMENTATION) + configureServlet(ContextSetting.PER_QUERY) request.addParameter('query', '[{ "query": "query { query(arg:\\"test\\") { echo(arg:\\"test\\") { echo(arg:\\"test\\") } }}" }, { "query": "query{query(arg:\\"test\\") { echo (arg:\\"test\\") { echo(arg:\\"test\\")} }}" },' + ' { "query": "query{queryTwo(arg:\\"test\\") { echo (arg:\\"test\\")}}" }, { "query": "query{queryTwo(arg:\\"test\\") { echo (arg:\\"test\\")}}" }]') resetCounters() @@ -153,7 +145,7 @@ class DataLoaderDispatchingSpec extends Specification { def "batched query with per request context batches all queries within the request"() { setup: - servlet = configureServlet(ContextSetting.PER_REQUEST_WITH_INSTRUMENTATION) + servlet = configureServlet(ContextSetting.PER_REQUEST) request.addParameter('query', '[{ "query": "query { query(arg:\\"test\\") { echo(arg:\\"test\\") { echo(arg:\\"test\\") } }}" }, { "query": "query{query(arg:\\"test\\") { echo (arg:\\"test\\") { echo(arg:\\"test\\")} }}" },' + ' { "query": "query{queryTwo(arg:\\"test\\") { echo (arg:\\"test\\")}}" }, { "query": "query{queryTwo(arg:\\"test\\") { echo (arg:\\"test\\")}}" }]') resetCounters() @@ -176,78 +168,4 @@ class DataLoaderDispatchingSpec extends Specification { fetchCounterC.get() == 1 loadCounterC.get() == 2 } - - def unwrapChainedInstrumentations(Instrumentation instrumentation) { - if (!instrumentation instanceof ChainedInstrumentation) { - return Collections.singletonList(instrumentation) - } else { - List instrumentations = new ArrayList<>() - for (Instrumentation current : ((ChainedInstrumentation) instrumentation).getInstrumentations()) { - if (current instanceof ChainedInstrumentation) { - instrumentations.addAll(unwrapChainedInstrumentations(current)) - } else { - instrumentations.add(current) - } - } - return instrumentations - } - } - - def "PER_QUERY_WITHOUT_INSTRUMENTATION does not add instrumentation"() { - when: - def chainedFromContext = ContextSetting.PER_QUERY_WITHOUT_INSTRUMENTATION - .configureInstrumentationForContext(chainedSupplier, Collections.emptyList(), DataLoaderDispatcherInstrumentationOptions.newOptions()) - def simpleFromContext = ContextSetting.PER_QUERY_WITHOUT_INSTRUMENTATION - .configureInstrumentationForContext(simpleSupplier, Collections.emptyList(), DataLoaderDispatcherInstrumentationOptions.newOptions()) - then: - simpleInstrumentation == simpleFromContext.get() - chainedInstrumentation == chainedFromContext.get() - } - - def "PER_REQUEST_WITHOUT_INSTRUMENTATION does not add instrumentation"() { - when: - def chainedFromContext = ContextSetting.PER_REQUEST_WITHOUT_INSTRUMENTATION - .configureInstrumentationForContext(chainedSupplier, Collections.emptyList(), DataLoaderDispatcherInstrumentationOptions.newOptions()) - def simpleFromContext = ContextSetting.PER_REQUEST_WITHOUT_INSTRUMENTATION - .configureInstrumentationForContext(simpleSupplier, Collections.emptyList(), DataLoaderDispatcherInstrumentationOptions.newOptions()) - then: - simpleInstrumentation == simpleFromContext.get() - chainedInstrumentation == chainedFromContext.get() - } - - def "PER_QUERY_WITH_INSTRUMENTATION adds instrumentation"() { - when: - def chainedFromContext = ContextSetting.PER_QUERY_WITH_INSTRUMENTATION - .configureInstrumentationForContext(chainedSupplier, Collections.emptyList(), DataLoaderDispatcherInstrumentationOptions.newOptions()) - def simpleFromContext = ContextSetting.PER_QUERY_WITH_INSTRUMENTATION - .configureInstrumentationForContext(simpleSupplier, Collections.emptyList(), DataLoaderDispatcherInstrumentationOptions.newOptions()) - def fromSimple = unwrapChainedInstrumentations(simpleFromContext.get()) - def fromChained = unwrapChainedInstrumentations(chainedFromContext.get()) - then: - fromSimple.size() == 2 - fromSimple.contains(simpleInstrumentation) - fromSimple.stream().anyMatch({ inst -> inst instanceof ConfigurableDispatchInstrumentation }) - fromChained.size() == 2 - fromChained.contains(simpleInstrumentation) - fromChained.stream().anyMatch({ inst -> inst instanceof ConfigurableDispatchInstrumentation }) - } - - def "PER_REQUEST_WITH_INSTRUMENTATION adds instrumentation"() { - setup: - ExecutionInput mockInput = ExecutionInput.newExecutionInput().query("query { query(arg:\"test\")").dataLoaderRegistry(new DataLoaderRegistry()).build() - when: - def chainedFromContext = ContextSetting.PER_REQUEST_WITH_INSTRUMENTATION - .configureInstrumentationForContext(chainedSupplier, Collections.singletonList(mockInput), DataLoaderDispatcherInstrumentationOptions.newOptions()) - def simpleFromContext = ContextSetting.PER_REQUEST_WITH_INSTRUMENTATION - .configureInstrumentationForContext(simpleSupplier, Collections.singletonList(mockInput), DataLoaderDispatcherInstrumentationOptions.newOptions()) - def fromSimple = unwrapChainedInstrumentations(simpleFromContext.get()) - def fromChained = unwrapChainedInstrumentations(chainedFromContext.get()) - then: - fromSimple.size() == 2 - fromSimple.contains(simpleInstrumentation) - fromSimple.stream().anyMatch({ inst -> inst instanceof ConfigurableDispatchInstrumentation }) - fromChained.size() == 2 - fromChained.contains(simpleInstrumentation) - fromChained.stream().anyMatch({ inst -> inst instanceof ConfigurableDispatchInstrumentation }) - } } diff --git a/graphql-java-servlet/src/test/groovy/graphql/kickstart/servlet/OsgiGraphQLHttpServletSpec.groovy b/graphql-java-servlet/src/test/groovy/graphql/kickstart/servlet/OsgiGraphQLHttpServletSpec.groovy index 0cab0577..8b37c227 100644 --- a/graphql-java-servlet/src/test/groovy/graphql/kickstart/servlet/OsgiGraphQLHttpServletSpec.groovy +++ b/graphql-java-servlet/src/test/groovy/graphql/kickstart/servlet/OsgiGraphQLHttpServletSpec.groovy @@ -7,6 +7,7 @@ import graphql.annotations.processor.GraphQLAnnotations import graphql.execution.instrumentation.InstrumentationState import graphql.execution.instrumentation.SimplePerformantInstrumentation import graphql.execution.instrumentation.parameters.InstrumentationCreateStateParameters +import graphql.introspection.Introspection import graphql.kickstart.execution.GraphQLRequest import graphql.kickstart.execution.config.ExecutionStrategyProvider import graphql.kickstart.execution.config.InstrumentationProvider @@ -268,7 +269,7 @@ class OsgiGraphQLHttpServletSpec extends Specification { static class TestDirectiveProvider implements GraphQLDirectiveProvider { @Override Set getDirectives() { - return new HashSet<>(Arrays.asList(GraphQLDirective.newDirective().name("myDirective").build())); + return new HashSet<>(Arrays.asList(GraphQLDirective.newDirective().name("myDirective").validLocation(Introspection.DirectiveLocation.FIELD).build())); } }