Skip to content

Make ExecutionStepInfo values be lazy #2749

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
andimarek merged 1 commit into from
Mar 1, 2022
Merged

Make ExecutionStepInfo values be lazy #2749

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions src/main/java/graphql/TypeResolutionEnvironment.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import graphql.schema.GraphQLType;

import java.util.Map;
import java.util.function.Supplier;

/**
* This is passed to a {@link graphql.schema.TypeResolver} to help with object type resolution.
Expand All @@ -21,7 +22,7 @@
public class TypeResolutionEnvironment {

private final Object object;
private final ImmutableMapWithNullValues<String, Object> arguments;
private final Supplier<ImmutableMapWithNullValues<String, Object>> arguments;
Copy link
Member Author

@bbakerman bbakerman Feb 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a viral change - type resolving wont materialise the ESI arguments unless it has to

private final MergedField field;
private final GraphQLType fieldType;
private final GraphQLSchema schema;
Expand All @@ -33,7 +34,7 @@ public class TypeResolutionEnvironment {
@Internal
public TypeResolutionEnvironment(TypeResolutionParameters parameters) {
this.object = parameters.getValue();
this.arguments = ImmutableMapWithNullValues.copyOf(parameters.getArgumentValues());
this.arguments = () -> ImmutableMapWithNullValues.copyOf(parameters.getArgumentValues());
this.field = parameters.getField();
this.fieldType = parameters.getFieldType();
this.schema = parameters.getSchema();
Expand All @@ -60,7 +61,7 @@ public <T> T getObject() {
* @return the runtime arguments to this the graphql field
*/
public Map<String, Object> getArguments() {
return arguments;
return arguments.get();
}

/**
Expand Down
55 changes: 28 additions & 27 deletions src/main/java/graphql/execution/ExecutionStepInfo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

import java.util.Map;
import java.util.function.Consumer;
import java.util.function.Supplier;

import static graphql.Assert.assertNotNull;
import static graphql.Assert.assertTrue;
Expand Down Expand Up @@ -64,28 +65,23 @@ public class ExecutionStepInfo {
private final MergedField field;
private final GraphQLFieldDefinition fieldDefinition;
private final GraphQLObjectType fieldContainer;
private final ImmutableMapWithNullValues<String, Object> arguments;

private ExecutionStepInfo(GraphQLOutputType type,
GraphQLFieldDefinition fieldDefinition,
MergedField field,
ResultPath path,
ExecutionStepInfo parent,
ImmutableMapWithNullValues<String, Object> arguments,
GraphQLObjectType fieldsContainer) {
this.fieldDefinition = fieldDefinition;
this.field = field;
this.path = path;
this.parent = parent;
this.type = assertNotNull(type, () -> "you must provide a graphql type");
this.arguments = arguments;
this.fieldContainer = fieldsContainer;
private final Supplier<ImmutableMapWithNullValues<String, Object>> arguments;

private ExecutionStepInfo(Builder builder) {
this.fieldDefinition = builder.fieldDefinition;
this.field = builder.field;
this.path = builder.path;
this.parent = builder.parentInfo;
this.type = assertNotNull(builder.type, () -> "you must provide a graphql type");
this.arguments = builder.arguments;
this.fieldContainer = builder.fieldContainer;
Copy link
Member Author

@bbakerman bbakerman Feb 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move to builder as arg - modernisation

}

/**
* @return the GraphQLObjectType defining the {@link #getFieldDefinition()}
* @deprecated use {@link #getObjectType()} instead as it is named better
*
* @see ExecutionStepInfo#getObjectType()
* @deprecated use {@link #getObjectType()} instead as it is named better
*/
@Deprecated
public GraphQLObjectType getFieldContainer() {
Expand Down Expand Up @@ -165,19 +161,20 @@ public boolean isListType() {
* @return the resolved arguments that have been passed to this field
*/
public Map<String, Object> getArguments() {
return arguments;
return arguments.get();
}

/**
* Returns the named argument
*
* @param name the name of the argument
* @param <T> you decide what type it is
*
* @return the named argument or null if its not present
*/
@SuppressWarnings("unchecked")
public <T> T getArgument(String name) {
return (T) arguments.get(name);
return (T) getArguments().get(name);
}

/**
Expand All @@ -201,14 +198,15 @@ public boolean hasParent() {
* after type resolution has occurred
*
* @param newType the new type to be
*
* @return a new type info with the same
*/
public ExecutionStepInfo changeTypeWithPreservedNonNull(GraphQLOutputType newType) {
assertTrue(!GraphQLTypeUtil.isNonNull(newType), () -> "newType can't be non null");
if (isNonNullType()) {
return new ExecutionStepInfo(GraphQLNonNull.nonNull(newType), fieldDefinition, field, path, this.parent, arguments, this.fieldContainer);
return newExecutionStepInfo(this).type(GraphQLNonNull.nonNull(newType)).build();
} else {
return new ExecutionStepInfo(newType, fieldDefinition, field, path, this.parent, arguments, this.fieldContainer);
return newExecutionStepInfo(this).type(newType).build();
}
}

Expand Down Expand Up @@ -257,13 +255,13 @@ public static class Builder {
GraphQLObjectType fieldContainer;
MergedField field;
ResultPath path;
ImmutableMapWithNullValues<String, Object> arguments;
Supplier<ImmutableMapWithNullValues<String, Object>> arguments;

/**
* @see ExecutionStepInfo#newExecutionStepInfo()
*/
private Builder() {
arguments = ImmutableMapWithNullValues.emptyMap();
arguments = ImmutableMapWithNullValues::emptyMap;
}

private Builder(ExecutionStepInfo existing) {
Expand All @@ -273,7 +271,7 @@ private Builder(ExecutionStepInfo existing) {
this.fieldContainer = existing.fieldContainer;
this.field = existing.field;
this.path = existing.path;
this.arguments = ImmutableMapWithNullValues.copyOf(existing.getArguments());
this.arguments = existing.arguments;
}

public Builder type(GraphQLOutputType type) {
Expand Down Expand Up @@ -301,8 +299,11 @@ public Builder path(ResultPath resultPath) {
return this;
}

public Builder arguments(Map<String, Object> arguments) {
this.arguments = arguments == null ? ImmutableMapWithNullValues.emptyMap() : ImmutableMapWithNullValues.copyOf(arguments);
public Builder arguments(Supplier<Map<String, Object>> arguments) {
this.arguments = () -> {
Map<String, Object> map = arguments.get();
return map == null ? ImmutableMapWithNullValues.emptyMap() : ImmutableMapWithNullValues.copyOf(map);
};
return this;
}

Expand All @@ -312,7 +313,7 @@ public Builder fieldContainer(GraphQLObjectType fieldContainer) {
}

public ExecutionStepInfo build() {
return new ExecutionStepInfo(type, fieldDefinition, field, path, parentInfo, arguments, fieldContainer);
return new ExecutionStepInfo(this);
}
}
}
4 changes: 3 additions & 1 deletion src/main/java/graphql/execution/ExecutionStepInfoFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@
import graphql.schema.GraphQLList;
import graphql.schema.GraphQLObjectType;
import graphql.schema.GraphQLOutputType;
import graphql.util.FpKit;

import java.util.List;
import java.util.Map;
import java.util.function.Supplier;

@Internal
public class ExecutionStepInfoFactory {
Expand All @@ -25,7 +27,7 @@ public ExecutionStepInfo newExecutionStepInfoForSubField(ExecutionContext execut
GraphQLOutputType fieldType = fieldDefinition.getType();
List<Argument> fieldArgs = mergedField.getArguments();
GraphQLCodeRegistry codeRegistry = executionContext.getGraphQLSchema().getCodeRegistry();
Map<String, Object> argumentValues = valuesResolver.getArgumentValues(codeRegistry, fieldDefinition.getArguments(), fieldArgs, executionContext.getVariables());
Supplier<Map<String, Object>> argumentValues = FpKit.intraThreadMemoize(() -> valuesResolver.getArgumentValues(codeRegistry, fieldDefinition.getArguments(), fieldArgs, executionContext.getVariables()));

ResultPath newPath = parentInfo.getPath().segment(mergedField.getResultKey());

Expand Down
4 changes: 2 additions & 2 deletions src/main/java/graphql/execution/ExecutionStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -810,14 +810,14 @@ protected ExecutionStepInfo createExecutionStepInfo(ExecutionContext executionCo
ExecutionStepInfo parentStepInfo = parameters.getExecutionStepInfo();
GraphQLOutputType fieldType = fieldDefinition.getType();
List<GraphQLArgument> fieldArgDefs = fieldDefinition.getArguments();
Map<String, Object> argumentValues = Collections.emptyMap();
Supplier<Map<String, Object>> argumentValues = Collections::emptyMap;
//
// no need to create args at all if there are none on the field def
//
if (!fieldArgDefs.isEmpty()) {
List<Argument> fieldArgs = field.getArguments();
GraphQLCodeRegistry codeRegistry = executionContext.getGraphQLSchema().getCodeRegistry();
argumentValues = valuesResolver.getArgumentValues(codeRegistry, fieldArgDefs, fieldArgs, executionContext.getVariables());
argumentValues = FpKit.intraThreadMemoize(() -> valuesResolver.getArgumentValues(codeRegistry, fieldArgDefs, fieldArgs, executionContext.getVariables()));
}


Expand Down
2 changes: 1 addition & 1 deletion src/main/java/graphql/execution/ResolveType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public GraphQLObjectType resolveType(ExecutionContext executionContext, MergedFi
.field(field)
.fieldType(fieldType)
.value(source)
.argumentValues(executionStepInfo.getArguments())
.argumentValues(executionStepInfo::getArguments)
.selectionSet(fieldSelectionSet)
.context(executionContext.getContext())
.graphQLContext(executionContext.getGraphQLContext())
Expand Down
11 changes: 6 additions & 5 deletions src/main/java/graphql/execution/TypeResolutionParameters.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import graphql.schema.GraphQLType;

import java.util.Map;
import java.util.function.Supplier;

/**
* This class is a classic builder style one that SHOULD have been on have been on {@link TypeResolutionEnvironment}
Expand All @@ -20,7 +21,7 @@ public class TypeResolutionParameters {
private final MergedField field;
private final GraphQLType fieldType;
private final Object value;
private final ImmutableMapWithNullValues<String, Object> argumentValues;
private final Supplier<ImmutableMapWithNullValues<String, Object>> argumentValues;
private final GraphQLSchema schema;
private final Object context;
private final Object localContext;
Expand Down Expand Up @@ -52,7 +53,7 @@ public Object getValue() {
}

public Map<String, Object> getArgumentValues() {
return argumentValues;
return argumentValues.get();
}

public GraphQLSchema getSchema() {
Expand Down Expand Up @@ -90,7 +91,7 @@ public static class Builder {
private MergedField field;
private GraphQLType fieldType;
private Object value;
private ImmutableMapWithNullValues<String, Object> argumentValues;
private Supplier<ImmutableMapWithNullValues<String, Object>> argumentValues;
private GraphQLSchema schema;
private Object context;
private GraphQLContext graphQLContext;
Expand All @@ -112,8 +113,8 @@ public Builder value(Object value) {
return this;
}

public Builder argumentValues(Map<String, Object> argumentValues) {
this.argumentValues = ImmutableMapWithNullValues.copyOf(argumentValues);
public Builder argumentValues(Supplier<Map<String, Object>> argumentValues) {
this.argumentValues = () -> ImmutableMapWithNullValues.copyOf(argumentValues.get());
return this;
}

Expand Down
6 changes: 3 additions & 3 deletions src/test/groovy/graphql/TypeResolutionEnvironmentTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class TypeResolutionEnvironmentTest extends Specification {

def environment = TypeResolutionParameters.newParameters()
.value("source")
.argumentValues([a: "b"])
.argumentValues(() -> [a: "b"])
.field(mergedField(new Field("field")))
.fieldType(interfaceType)
.schema(schema)
Expand Down Expand Up @@ -117,7 +117,7 @@ class TypeResolutionEnvironmentTest extends Specification {
when:
def environmentFooBar = TypeResolutionParameters.newParameters()
.value("source")
.argumentValues([:])
.argumentValues(() -> [:])
.field(mergedField(new Field("field")))
.fieldType(interfaceType)
.schema(schema)
Expand All @@ -133,7 +133,7 @@ class TypeResolutionEnvironmentTest extends Specification {
when:
def environmentFooImpl = TypeResolutionParameters.newParameters()
.value("source")
.argumentValues([:])
.argumentValues(() -> [:])
.field(mergedField(new Field("field")))
.fieldType(interfaceType)
.schema(schema)
Expand Down