From bff9b62864eb6a15301677cb38d992d085c2ff64 Mon Sep 17 00:00:00 2001 From: Davide Angelocola Date: Sat, 5 Apr 2025 15:57:25 +0200 Subject: [PATCH] Introduce ResponseMapFactory This will allows to customize which java.util.Map concrete class to use. For example, eclipse-collections has Map implementations with better memory footprint than JDK maps. --- src/main/java/graphql/GraphQL.java | 14 +++- .../AbstractAsyncExecutionStrategy.java | 8 +-- .../execution/DefaultResponseMapFactory.java | 26 +++++++ .../java/graphql/execution/Execution.java | 4 ++ .../graphql/execution/ExecutionContext.java | 8 ++- .../execution/ExecutionContextBuilder.java | 7 ++ .../graphql/execution/ExecutionStrategy.java | 16 +---- .../graphql/execution/ResponseMapFactory.java | 32 +++++++++ .../CustomMapImplementationTest.groovy | 69 +++++++++++++++++++ .../DefaultResponseMapFactoryTest.groovy | 42 +++++++++++ .../graphql/execution/ExecutionTest.groovy | 4 +- .../FieldValidationTest.groovy | 3 +- 12 files changed, 205 insertions(+), 28 deletions(-) create mode 100644 src/main/java/graphql/execution/DefaultResponseMapFactory.java create mode 100644 src/main/java/graphql/execution/ResponseMapFactory.java create mode 100644 src/test/groovy/graphql/CustomMapImplementationTest.groovy create mode 100644 src/test/groovy/graphql/execution/DefaultResponseMapFactoryTest.groovy diff --git a/src/main/java/graphql/GraphQL.java b/src/main/java/graphql/GraphQL.java index 5d8dfc87d5..a279dab2fd 100644 --- a/src/main/java/graphql/GraphQL.java +++ b/src/main/java/graphql/GraphQL.java @@ -9,6 +9,7 @@ import graphql.execution.ExecutionId; import graphql.execution.ExecutionIdProvider; import graphql.execution.ExecutionStrategy; +import graphql.execution.ResponseMapFactory; import graphql.execution.SimpleDataFetcherExceptionHandler; import graphql.execution.SubscriptionExecutionStrategy; import graphql.execution.ValueUnboxer; @@ -64,7 +65,7 @@ * * *
  • {@link graphql.execution.UnresolvedTypeException} - is thrown if a {@link graphql.schema.TypeResolver} fails to provide a concrete - * object type given a interface or union type. + * object type given an interface or union type. *
    • * *
  • {@link graphql.schema.validation.InvalidSchemaException} - is thrown if the schema is not valid when built via @@ -92,6 +93,7 @@ public class GraphQL { private final Instrumentation instrumentation; private final PreparsedDocumentProvider preparsedDocumentProvider; private final ValueUnboxer valueUnboxer; + private final ResponseMapFactory responseMapFactory; private final boolean doNotAutomaticallyDispatchDataLoader; @@ -104,6 +106,7 @@ 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; } @@ -213,7 +216,7 @@ 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; @@ -284,6 +287,11 @@ 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) { @@ -543,7 +551,7 @@ private CompletableFuture execute(ExecutionInput executionInput EngineRunningState engineRunningState ) { - Execution execution = new Execution(queryStrategy, mutationStrategy, subscriptionStrategy, instrumentation, valueUnboxer, doNotAutomaticallyDispatchDataLoader); + Execution execution = new Execution(queryStrategy, mutationStrategy, subscriptionStrategy, instrumentation, valueUnboxer, responseMapFactory, doNotAutomaticallyDispatchDataLoader); ExecutionId executionId = executionInput.getExecutionId(); return execution.execute(document, graphQLSchema, executionId, executionInput, instrumentationState, engineRunningState); diff --git a/src/main/java/graphql/execution/AbstractAsyncExecutionStrategy.java b/src/main/java/graphql/execution/AbstractAsyncExecutionStrategy.java index 6ae7f851ea..d04ba001c3 100644 --- a/src/main/java/graphql/execution/AbstractAsyncExecutionStrategy.java +++ b/src/main/java/graphql/execution/AbstractAsyncExecutionStrategy.java @@ -1,6 +1,5 @@ package graphql.execution; -import com.google.common.collect.Maps; import graphql.ExecutionResult; import graphql.ExecutionResultImpl; import graphql.PublicSpi; @@ -28,12 +27,7 @@ protected BiConsumer, Throwable> handleResults(ExecutionContext exe return; } - Map resolvedValuesByField = Maps.newLinkedHashMapWithExpectedSize(fieldNames.size()); - int ix = 0; - for (Object result : results) { - String fieldName = fieldNames.get(ix++); - resolvedValuesByField.put(fieldName, result); - } + Map resolvedValuesByField = executionContext.getResponseMapFactory().createInsertionOrdered(fieldNames, results); overallResult.complete(new ExecutionResultImpl(resolvedValuesByField, executionContext.getErrors())); }; } diff --git a/src/main/java/graphql/execution/DefaultResponseMapFactory.java b/src/main/java/graphql/execution/DefaultResponseMapFactory.java new file mode 100644 index 0000000000..3f3392c903 --- /dev/null +++ b/src/main/java/graphql/execution/DefaultResponseMapFactory.java @@ -0,0 +1,26 @@ +package graphql.execution; + +import com.google.common.collect.Maps; +import graphql.Internal; + +import java.util.List; +import java.util.Map; + +/** + * Implements the contract of {@link ResponseMapFactory} with {@link java.util.LinkedHashMap}. + * This is the default of graphql-java since a long time and changing it could cause breaking changes. + */ +@Internal +public class DefaultResponseMapFactory implements ResponseMapFactory { + + @Override + public Map createInsertionOrdered(List keys, List values) { + Map result = Maps.newLinkedHashMapWithExpectedSize(keys.size()); + int ix = 0; + for (Object fieldValue : values) { + String fieldName = keys.get(ix++); + result.put(fieldName, fieldValue); + } + return result; + } +} diff --git a/src/main/java/graphql/execution/Execution.java b/src/main/java/graphql/execution/Execution.java index a784325f3d..1a65040809 100644 --- a/src/main/java/graphql/execution/Execution.java +++ b/src/main/java/graphql/execution/Execution.java @@ -57,6 +57,7 @@ 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, @@ -64,12 +65,14 @@ 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; } @@ -110,6 +113,7 @@ public CompletableFuture execute(Document document, GraphQLSche .dataLoaderRegistry(executionInput.getDataLoaderRegistry()) .locale(executionInput.getLocale()) .valueUnboxer(valueUnboxer) + .responseMapFactory(responseMapFactory) .executionInput(executionInput) .propagapropagateErrorsOnNonNullContractFailureeErrors(propagateErrorsOnNonNullContractFailure) .engineRunningState(engineRunningState) diff --git a/src/main/java/graphql/execution/ExecutionContext.java b/src/main/java/graphql/execution/ExecutionContext.java index a53ae621e1..1b1f289083 100644 --- a/src/main/java/graphql/execution/ExecutionContext.java +++ b/src/main/java/graphql/execution/ExecutionContext.java @@ -61,6 +61,8 @@ public class ExecutionContext { private final Locale locale; private final IncrementalCallState incrementalCallState = new IncrementalCallState(); private final ValueUnboxer valueUnboxer; + private final ResponseMapFactory responseMapFactory; + private final ExecutionInput executionInput; private final Supplier queryTree; private final boolean propagateErrorsOnNonNullContractFailure; @@ -92,6 +94,7 @@ public class ExecutionContext { this.dataLoaderRegistry = builder.dataLoaderRegistry; this.locale = builder.locale; this.valueUnboxer = builder.valueUnboxer; + this.responseMapFactory = builder.responseMapFactory; this.errors.set(builder.errors); this.localContext = builder.localContext; this.executionInput = builder.executionInput; @@ -101,7 +104,6 @@ public class ExecutionContext { this.engineRunningState = builder.engineRunningState; } - public ExecutionId getExecutionId() { return executionId; } @@ -295,6 +297,10 @@ public void addErrors(List errors) { }); } + public ResponseMapFactory getResponseMapFactory() { + return responseMapFactory; + } + /** * @return the total list of errors for this execution context */ diff --git a/src/main/java/graphql/execution/ExecutionContextBuilder.java b/src/main/java/graphql/execution/ExecutionContextBuilder.java index 014bab516a..53dce77981 100644 --- a/src/main/java/graphql/execution/ExecutionContextBuilder.java +++ b/src/main/java/graphql/execution/ExecutionContextBuilder.java @@ -52,6 +52,7 @@ public class ExecutionContextBuilder { DataLoaderDispatchStrategy dataLoaderDispatcherStrategy = DataLoaderDispatchStrategy.NO_OP; boolean propagateErrorsOnNonNullContractFailure = true; EngineRunningState engineRunningState; + ResponseMapFactory responseMapFactory = ResponseMapFactory.DEFAULT; /** * @return a new builder of {@link graphql.execution.ExecutionContext}s @@ -100,6 +101,7 @@ public ExecutionContextBuilder() { dataLoaderDispatcherStrategy = other.getDataLoaderDispatcherStrategy(); propagateErrorsOnNonNullContractFailure = other.propagateErrorsOnNonNullContractFailure(); engineRunningState = other.getEngineRunningState(); + responseMapFactory = other.getResponseMapFactory(); } public ExecutionContextBuilder instrumentation(Instrumentation instrumentation) { @@ -224,6 +226,11 @@ public ExecutionContextBuilder dataLoaderDispatcherStrategy(DataLoaderDispatchSt return this; } + public ExecutionContextBuilder responseMapFactory(ResponseMapFactory responseMapFactory) { + this.responseMapFactory = responseMapFactory; + return this; + } + public ExecutionContextBuilder resetErrors() { this.errors = emptyList(); return this; diff --git a/src/main/java/graphql/execution/ExecutionStrategy.java b/src/main/java/graphql/execution/ExecutionStrategy.java index f9370d1e91..2db611a60b 100644 --- a/src/main/java/graphql/execution/ExecutionStrategy.java +++ b/src/main/java/graphql/execution/ExecutionStrategy.java @@ -1,7 +1,6 @@ package graphql.execution; import com.google.common.collect.ImmutableList; -import com.google.common.collect.Maps; import graphql.DuckTyped; import graphql.EngineRunningState; import graphql.ExecutionResult; @@ -260,7 +259,7 @@ protected Object executeObject(ExecutionContext executionContext, ExecutionStrat overallResult.whenComplete(resolveObjectCtx::onCompleted); return overallResult; } else { - Map fieldValueMap = buildFieldValueMap(fieldsExecutedOnInitialResult, (List) completedValuesObject); + Map fieldValueMap = executionContext.getResponseMapFactory().createInsertionOrdered(fieldsExecutedOnInitialResult, (List) completedValuesObject); resolveObjectCtx.onCompleted(fieldValueMap, null); return fieldValueMap; } @@ -281,22 +280,11 @@ private BiConsumer, Throwable> buildFieldValueMap(List fiel handleValueException(overallResult, exception, executionContext); return; } - Map resolvedValuesByField = buildFieldValueMap(fieldNames, results); + Map resolvedValuesByField = executionContext.getResponseMapFactory().createInsertionOrdered(fieldNames, results); overallResult.complete(resolvedValuesByField); }; } - @NonNull - private static Map buildFieldValueMap(List fieldNames, List results) { - Map resolvedValuesByField = Maps.newLinkedHashMapWithExpectedSize(fieldNames.size()); - int ix = 0; - for (Object fieldValue : results) { - String fieldName = fieldNames.get(ix++); - resolvedValuesByField.put(fieldName, fieldValue); - } - return resolvedValuesByField; - } - DeferredExecutionSupport createDeferredExecutionSupport(ExecutionContext executionContext, ExecutionStrategyParameters parameters) { MergedSelectionSet fields = parameters.getFields(); diff --git a/src/main/java/graphql/execution/ResponseMapFactory.java b/src/main/java/graphql/execution/ResponseMapFactory.java new file mode 100644 index 0000000000..01a37d8491 --- /dev/null +++ b/src/main/java/graphql/execution/ResponseMapFactory.java @@ -0,0 +1,32 @@ +package graphql.execution; + +import graphql.ExperimentalApi; +import graphql.PublicSpi; + +import java.util.List; +import java.util.Map; + +/** + * Allows to customize the concrete class {@link Map} implementation. For example, it could be possible to use + * memory-efficient implementations, like eclipse-collections. + */ +@ExperimentalApi +@PublicSpi +public interface ResponseMapFactory { + + /** + * The default implementation uses JDK's {@link java.util.LinkedHashMap}. + */ + ResponseMapFactory DEFAULT = new DefaultResponseMapFactory(); + + /** + * The general contract is that the resulting map keeps the insertion orders of keys. Values are nullable but keys are not. + * Implementations are free to create or to reuse map instances. + * + * @param keys the keys like k1, k2, ..., kn + * @param values the values like v1, v2, ..., vn + * @return a new or reused map instance with (k1,v1), (k2, v2), ... (kn, vn) + */ + Map createInsertionOrdered(List keys, List values); + +} diff --git a/src/test/groovy/graphql/CustomMapImplementationTest.groovy b/src/test/groovy/graphql/CustomMapImplementationTest.groovy new file mode 100644 index 0000000000..7430c03ae8 --- /dev/null +++ b/src/test/groovy/graphql/CustomMapImplementationTest.groovy @@ -0,0 +1,69 @@ +package graphql + +import graphql.execution.ResponseMapFactory +import graphql.schema.idl.RuntimeWiring +import groovy.transform.Immutable +import spock.lang.Specification + +class CustomMapImplementationTest extends Specification { + + @Immutable + static class Person { + String name + @SuppressWarnings('unused') // used by graphql-java + int age + } + + class CustomResponseMapFactory implements ResponseMapFactory { + + @Override + Map createInsertionOrdered(List keys, List values) { + return Collections.unmodifiableMap(DEFAULT.createInsertionOrdered(keys, values)) + } + } + + def graphql = TestUtil.graphQL(""" + type Query { + people: [Person!]! + } + + type Person { + name: String! + age: Int! + } + + """, + RuntimeWiring.newRuntimeWiring() + .type("Query", { + it.dataFetcher("people", { List.of(new Person("Mario", 18), new Person("Luigi", 21))}) + }) + .build()) + .responseMapFactory(new CustomResponseMapFactory()) + .build() + + def "customMapImplementation"() { + when: + def input = ExecutionInput.newExecutionInput() + .query(''' + query { + people { + name + age + } + } + ''') + .build() + + def executionResult = graphql.execute(input) + + then: + executionResult.errors.isEmpty() + executionResult.data == [ people: [ + [name: "Mario", age: 18], + [name: "Luigi", age: 21], + ]] + executionResult.data.getClass().getSimpleName() == 'UnmodifiableMap' + executionResult.data['people'].each { it -> it.getClass().getSimpleName() == 'UnmodifiableMap' } + } + +} diff --git a/src/test/groovy/graphql/execution/DefaultResponseMapFactoryTest.groovy b/src/test/groovy/graphql/execution/DefaultResponseMapFactoryTest.groovy new file mode 100644 index 0000000000..f8ea921bad --- /dev/null +++ b/src/test/groovy/graphql/execution/DefaultResponseMapFactoryTest.groovy @@ -0,0 +1,42 @@ +package graphql.execution + +import spock.lang.Specification + +class DefaultResponseMapFactoryTest extends Specification { + + def "no keys"() { + given: + var sut = new DefaultResponseMapFactory() + + when: + var result = sut.createInsertionOrdered(List.of(), List.of()) + + then: + result.isEmpty() + result instanceof LinkedHashMap + } + + def "1 key"() { + given: + var sut = new DefaultResponseMapFactory() + + when: + var result = sut.createInsertionOrdered(List.of("name"), List.of("Mario")) + + then: + result == ["name": "Mario"] + result instanceof LinkedHashMap + } + + def "2 keys"() { + given: + var sut = new DefaultResponseMapFactory() + + when: + var result = sut.createInsertionOrdered(List.of("name", "age"), List.of("Mario", 18)) + + then: + result == ["name": "Mario", "age": 18] + result instanceof LinkedHashMap + } +} diff --git a/src/test/groovy/graphql/execution/ExecutionTest.groovy b/src/test/groovy/graphql/execution/ExecutionTest.groovy index 6d207ae1a1..1557d94d29 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, false) + def execution = new Execution(queryStrategy, mutationStrategy, subscriptionStrategy, SimplePerformantInstrumentation.INSTANCE, ValueUnboxer.DEFAULT, ResponseMapFactory.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, false) + def execution = new Execution(queryStrategy, mutationStrategy, subscriptionStrategy, instrumentation, ValueUnboxer.DEFAULT, ResponseMapFactory.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 c3b3f40994..67712b7fe9 100644 --- a/src/test/groovy/graphql/execution/instrumentation/fieldvalidation/FieldValidationTest.groovy +++ b/src/test/groovy/graphql/execution/instrumentation/fieldvalidation/FieldValidationTest.groovy @@ -10,6 +10,7 @@ import graphql.execution.AbortExecutionException import graphql.execution.AsyncExecutionStrategy import graphql.execution.Execution import graphql.execution.ExecutionId +import graphql.execution.ResponseMapFactory import graphql.execution.ResultPath import graphql.execution.ValueUnboxer import graphql.execution.instrumentation.ChainedInstrumentation @@ -306,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, false) + def execution = new Execution(strategy, strategy, strategy, instrumentation, ValueUnboxer.DEFAULT, ResponseMapFactory.DEFAULT, false) def executionInput = ExecutionInput.newExecutionInput().query(query).variables(variables).build() execution.execute(document, schema, ExecutionId.generate(), executionInput, null, new EngineRunningState())