From 1e6e0af0e86eb73f45b66a9a6c283533b2e848ef Mon Sep 17 00:00:00 2001 From: bbaker Date: Tue, 6 May 2025 17:11:39 +1000 Subject: [PATCH] responseMapFactory now set via unusual configuration --- src/main/java/graphql/GraphQL.java | 10 +---- .../graphql/GraphQLUnusualConfiguration.java | 41 +++++++++++++++++ .../java/graphql/execution/Execution.java | 7 +-- .../graphql/execution/ExecutionContext.java | 1 + .../execution/ExecutionContextBuilder.java | 1 + .../CustomMapImplementationTest.groovy | 2 +- .../GraphQLUnusualConfigurationTest.groovy | 44 +++++++++++++++++++ .../graphql/execution/ExecutionTest.groovy | 4 +- .../FieldValidationTest.groovy | 2 +- 9 files changed, 96 insertions(+), 16 deletions(-) diff --git a/src/main/java/graphql/GraphQL.java b/src/main/java/graphql/GraphQL.java index ce4abd32b..bfe6ab59e 100644 --- a/src/main/java/graphql/GraphQL.java +++ b/src/main/java/graphql/GraphQL.java @@ -133,7 +133,6 @@ public static GraphQLUnusualConfiguration.GraphQLContextConfiguration unusualCon private final Instrumentation instrumentation; private final PreparsedDocumentProvider preparsedDocumentProvider; private final ValueUnboxer valueUnboxer; - private final ResponseMapFactory responseMapFactory; private final boolean doNotAutomaticallyDispatchDataLoader; @@ -146,7 +145,6 @@ private GraphQL(Builder builder) { this.instrumentation = assertNotNull(builder.instrumentation, () -> "instrumentation must not be null"); this.preparsedDocumentProvider = assertNotNull(builder.preparsedDocumentProvider, () -> "preparsedDocumentProvider must be non null"); this.valueUnboxer = assertNotNull(builder.valueUnboxer, () -> "valueUnboxer must not be null"); - this.responseMapFactory = assertNotNull(builder.responseMapFactory, () -> "responseMapFactory must be not null"); this.doNotAutomaticallyDispatchDataLoader = builder.doNotAutomaticallyDispatchDataLoader; } @@ -256,7 +254,6 @@ public static class Builder { private PreparsedDocumentProvider preparsedDocumentProvider = NoOpPreparsedDocumentProvider.INSTANCE; private boolean doNotAutomaticallyDispatchDataLoader = false; private ValueUnboxer valueUnboxer = ValueUnboxer.DEFAULT; - private ResponseMapFactory responseMapFactory = ResponseMapFactory.DEFAULT; public Builder(GraphQLSchema graphQLSchema) { this.graphQLSchema = graphQLSchema; @@ -327,11 +324,6 @@ public Builder valueUnboxer(ValueUnboxer valueUnboxer) { return this; } - public Builder responseMapFactory(ResponseMapFactory responseMapFactory) { - this.responseMapFactory = responseMapFactory; - return this; - } - public GraphQL build() { // we use the data fetcher exception handler unless they set their own strategy in which case bets are off if (queryExecutionStrategy == null) { @@ -591,7 +583,7 @@ private CompletableFuture execute(ExecutionInput executionInput EngineRunningState engineRunningState ) { - Execution execution = new Execution(queryStrategy, mutationStrategy, subscriptionStrategy, instrumentation, valueUnboxer, responseMapFactory, doNotAutomaticallyDispatchDataLoader); + Execution execution = new Execution(queryStrategy, mutationStrategy, subscriptionStrategy, instrumentation, valueUnboxer, doNotAutomaticallyDispatchDataLoader); ExecutionId executionId = executionInput.getExecutionId(); return execution.execute(document, graphQLSchema, executionId, executionInput, instrumentationState, engineRunningState); diff --git a/src/main/java/graphql/GraphQLUnusualConfiguration.java b/src/main/java/graphql/GraphQLUnusualConfiguration.java index 3fbd24a39..bece0bb27 100644 --- a/src/main/java/graphql/GraphQLUnusualConfiguration.java +++ b/src/main/java/graphql/GraphQLUnusualConfiguration.java @@ -1,5 +1,6 @@ package graphql; +import graphql.execution.ResponseMapFactory; import graphql.introspection.GoodFaithIntrospection; import graphql.parser.ParserOptions; import graphql.schema.PropertyDataFetcherHelper; @@ -257,6 +258,13 @@ public IncrementalSupportConfig incrementalSupport() { return new IncrementalSupportConfig(this); } + /** + * @return an element that allows you to control the {@link ResponseMapFactory} used + */ + public ResponseMapFactoryConfig responseMapFactory() { + return new ResponseMapFactoryConfig(this); + } + private void put(String named, Object value) { if (graphQLContext != null) { graphQLContext.put(named, value); @@ -272,6 +280,15 @@ private boolean getBoolean(String named) { return assertNotNull(graphQLContextBuilder).getBoolean(named); } } + + public T get(String named) { + if (graphQLContext != null) { + return graphQLContext.get(named); + } else { + //noinspection unchecked + return (T) assertNotNull(graphQLContextBuilder).get(named); + } + } } private static class BaseContextConfig { @@ -310,4 +327,28 @@ public IncrementalSupportConfig enableIncrementalSupport(boolean enable) { return this; } } + + public static class ResponseMapFactoryConfig extends BaseContextConfig { + private ResponseMapFactoryConfig(GraphQLContextConfiguration contextConfig) { + super(contextConfig); + } + + /** + * @return the {@link ResponseMapFactory} in play - this can be null + */ + @ExperimentalApi + public ResponseMapFactory getOr(ResponseMapFactory defaultFactory) { + ResponseMapFactory responseMapFactory = contextConfig.get(ResponseMapFactory.class.getCanonicalName()); + return responseMapFactory != null ? responseMapFactory : defaultFactory; + } + + /** + * This controls the {@link ResponseMapFactory} to use for this request + */ + @ExperimentalApi + public ResponseMapFactoryConfig setFactory(ResponseMapFactory factory) { + contextConfig.put(ResponseMapFactory.class.getCanonicalName(), factory); + return this; + } + } } diff --git a/src/main/java/graphql/execution/Execution.java b/src/main/java/graphql/execution/Execution.java index 1a6504080..0da986792 100644 --- a/src/main/java/graphql/execution/Execution.java +++ b/src/main/java/graphql/execution/Execution.java @@ -7,6 +7,7 @@ import graphql.ExecutionResult; import graphql.ExecutionResultImpl; import graphql.ExperimentalApi; +import graphql.GraphQL; import graphql.GraphQLContext; import graphql.GraphQLError; import graphql.Internal; @@ -57,7 +58,6 @@ public class Execution { private final ExecutionStrategy subscriptionStrategy; private final Instrumentation instrumentation; private final ValueUnboxer valueUnboxer; - private final ResponseMapFactory responseMapFactory; private final boolean doNotAutomaticallyDispatchDataLoader; public Execution(ExecutionStrategy queryStrategy, @@ -65,14 +65,12 @@ public Execution(ExecutionStrategy queryStrategy, ExecutionStrategy subscriptionStrategy, Instrumentation instrumentation, ValueUnboxer valueUnboxer, - ResponseMapFactory responseMapFactory, boolean doNotAutomaticallyDispatchDataLoader) { this.queryStrategy = queryStrategy != null ? queryStrategy : new AsyncExecutionStrategy(); this.mutationStrategy = mutationStrategy != null ? mutationStrategy : new AsyncSerialExecutionStrategy(); this.subscriptionStrategy = subscriptionStrategy != null ? subscriptionStrategy : new AsyncExecutionStrategy(); this.instrumentation = instrumentation; this.valueUnboxer = valueUnboxer; - this.responseMapFactory = responseMapFactory; this.doNotAutomaticallyDispatchDataLoader = doNotAutomaticallyDispatchDataLoader; } @@ -93,6 +91,9 @@ public CompletableFuture execute(Document document, GraphQLSche boolean propagateErrorsOnNonNullContractFailure = propagateErrorsOnNonNullContractFailure(getOperationResult.operationDefinition.getDirectives()); + ResponseMapFactory responseMapFactory = GraphQL.unusualConfiguration(executionInput.getGraphQLContext()) + .responseMapFactory().getOr(ResponseMapFactory.DEFAULT); + ExecutionContext executionContext = newExecutionContextBuilder() .instrumentation(instrumentation) .instrumentationState(instrumentationState) diff --git a/src/main/java/graphql/execution/ExecutionContext.java b/src/main/java/graphql/execution/ExecutionContext.java index 1b1f28908..64ad76f02 100644 --- a/src/main/java/graphql/execution/ExecutionContext.java +++ b/src/main/java/graphql/execution/ExecutionContext.java @@ -297,6 +297,7 @@ public void addErrors(List errors) { }); } + @Internal public ResponseMapFactory getResponseMapFactory() { return responseMapFactory; } diff --git a/src/main/java/graphql/execution/ExecutionContextBuilder.java b/src/main/java/graphql/execution/ExecutionContextBuilder.java index 53dce7798..fc28675f0 100644 --- a/src/main/java/graphql/execution/ExecutionContextBuilder.java +++ b/src/main/java/graphql/execution/ExecutionContextBuilder.java @@ -226,6 +226,7 @@ public ExecutionContextBuilder dataLoaderDispatcherStrategy(DataLoaderDispatchSt return this; } + @Internal public ExecutionContextBuilder responseMapFactory(ResponseMapFactory responseMapFactory) { this.responseMapFactory = responseMapFactory; return this; diff --git a/src/test/groovy/graphql/CustomMapImplementationTest.groovy b/src/test/groovy/graphql/CustomMapImplementationTest.groovy index 7430c03ae..7287b8c19 100644 --- a/src/test/groovy/graphql/CustomMapImplementationTest.groovy +++ b/src/test/groovy/graphql/CustomMapImplementationTest.groovy @@ -38,7 +38,6 @@ class CustomMapImplementationTest extends Specification { it.dataFetcher("people", { List.of(new Person("Mario", 18), new Person("Luigi", 21))}) }) .build()) - .responseMapFactory(new CustomResponseMapFactory()) .build() def "customMapImplementation"() { @@ -52,6 +51,7 @@ class CustomMapImplementationTest extends Specification { } } ''') + .graphQLContext { it -> GraphQL.unusualConfiguration(it).responseMapFactory().setFactory(new CustomResponseMapFactory())} .build() def executionResult = graphql.execute(input) diff --git a/src/test/groovy/graphql/config/GraphQLUnusualConfigurationTest.groovy b/src/test/groovy/graphql/config/GraphQLUnusualConfigurationTest.groovy index 3052ee673..a039306ff 100644 --- a/src/test/groovy/graphql/config/GraphQLUnusualConfigurationTest.groovy +++ b/src/test/groovy/graphql/config/GraphQLUnusualConfigurationTest.groovy @@ -3,6 +3,7 @@ package graphql.config import graphql.ExperimentalApi import graphql.GraphQL import graphql.GraphQLContext +import graphql.execution.ResponseMapFactory import graphql.introspection.GoodFaithIntrospection import graphql.parser.ParserOptions import graphql.schema.PropertyDataFetcherHelper @@ -120,4 +121,47 @@ class GraphQLUnusualConfigurationTest extends Specification { graphqlContext.get(ExperimentalApi.ENABLE_INCREMENTAL_SUPPORT) == false !GraphQL.unusualConfiguration(graphqlContext).incrementalSupport().isIncrementalSupportEnabled() } + + def "can set response map factory"() { + def rfm1 = new ResponseMapFactory() { + @Override + Map createInsertionOrdered(List keys, List values) { + return null + } + } + + def rfm2 = new ResponseMapFactory() { + @Override + Map createInsertionOrdered(List keys, List values) { + return null + } + } + + when: + def graphqlContextBuilder = GraphQLContext.newContext() + GraphQL.unusualConfiguration(graphqlContextBuilder).responseMapFactory().setFactory(rfm1) + + then: + GraphQL.unusualConfiguration(graphqlContextBuilder).responseMapFactory().getOr(rfm2) == rfm1 + + when: + graphqlContextBuilder = GraphQLContext.newContext() + + then: "can default" + GraphQL.unusualConfiguration(graphqlContextBuilder).responseMapFactory().getOr(rfm2) == rfm2 + + when: + def graphqlContext = GraphQLContext.newContext().build() + GraphQL.unusualConfiguration(graphqlContext).responseMapFactory().setFactory(rfm1) + + then: + GraphQL.unusualConfiguration(graphqlContext).responseMapFactory().getOr(rfm2) == rfm1 + + when: + graphqlContext = GraphQLContext.newContext().build() + + then: "can default" + GraphQL.unusualConfiguration(graphqlContext).responseMapFactory().getOr(rfm2) == rfm2 + + } } diff --git a/src/test/groovy/graphql/execution/ExecutionTest.groovy b/src/test/groovy/graphql/execution/ExecutionTest.groovy index 1557d94d2..6d207ae1a 100644 --- a/src/test/groovy/graphql/execution/ExecutionTest.groovy +++ b/src/test/groovy/graphql/execution/ExecutionTest.groovy @@ -36,7 +36,7 @@ class ExecutionTest extends Specification { def subscriptionStrategy = new CountingExecutionStrategy() def mutationStrategy = new CountingExecutionStrategy() def queryStrategy = new CountingExecutionStrategy() - def execution = new Execution(queryStrategy, mutationStrategy, subscriptionStrategy, SimplePerformantInstrumentation.INSTANCE, ValueUnboxer.DEFAULT, ResponseMapFactory.DEFAULT, false) + def execution = new Execution(queryStrategy, mutationStrategy, subscriptionStrategy, SimplePerformantInstrumentation.INSTANCE, ValueUnboxer.DEFAULT, false) def emptyExecutionInput = ExecutionInput.newExecutionInput().query("query").build() def instrumentationState = new InstrumentationState() {} @@ -125,7 +125,7 @@ class ExecutionTest extends Specification { } } - def execution = new Execution(queryStrategy, mutationStrategy, subscriptionStrategy, instrumentation, ValueUnboxer.DEFAULT, ResponseMapFactory.DEFAULT, false) + def execution = new Execution(queryStrategy, mutationStrategy, subscriptionStrategy, instrumentation, ValueUnboxer.DEFAULT, false) when: diff --git a/src/test/groovy/graphql/execution/instrumentation/fieldvalidation/FieldValidationTest.groovy b/src/test/groovy/graphql/execution/instrumentation/fieldvalidation/FieldValidationTest.groovy index 67712b7fe..508fc56d1 100644 --- a/src/test/groovy/graphql/execution/instrumentation/fieldvalidation/FieldValidationTest.groovy +++ b/src/test/groovy/graphql/execution/instrumentation/fieldvalidation/FieldValidationTest.groovy @@ -307,7 +307,7 @@ class FieldValidationTest extends Specification { def document = TestUtil.parseQuery(query) def strategy = new AsyncExecutionStrategy() def instrumentation = new FieldValidationInstrumentation(validation) - def execution = new Execution(strategy, strategy, strategy, instrumentation, ValueUnboxer.DEFAULT, ResponseMapFactory.DEFAULT, false) + def execution = new Execution(strategy, strategy, strategy, instrumentation, ValueUnboxer.DEFAULT, false) def executionInput = ExecutionInput.newExecutionInput().query(query).variables(variables).build() execution.execute(document, schema, ExecutionId.generate(), executionInput, null, new EngineRunningState())