diff --git a/src/main/java/graphql/execution/ExecutionStrategy.java b/src/main/java/graphql/execution/ExecutionStrategy.java index feb831b6d6..255dff4ceb 100644 --- a/src/main/java/graphql/execution/ExecutionStrategy.java +++ b/src/main/java/graphql/execution/ExecutionStrategy.java @@ -160,7 +160,7 @@ public static String mkNameForPath(Field currentField) { @Internal public static String mkNameForPath(MergedField mergedField) { - return mkNameForPath(mergedField.getFields()); + return mergedField.getResultKey(); } @Internal diff --git a/src/main/java/graphql/execution/FieldCollector.java b/src/main/java/graphql/execution/FieldCollector.java index 93d58f97ab..9a63e009b3 100644 --- a/src/main/java/graphql/execution/FieldCollector.java +++ b/src/main/java/graphql/execution/FieldCollector.java @@ -41,12 +41,11 @@ public MergedSelectionSet collectFields(FieldCollectorParameters parameters, Mer public MergedSelectionSet collectFields(FieldCollectorParameters parameters, MergedField mergedField, boolean incrementalSupport) { Map subFields = new LinkedHashMap<>(); Set visitedFragments = new LinkedHashSet<>(); - for (Field field : mergedField.getFields()) { - if (field.getSelectionSet() == null) { - continue; + mergedField.forEach(field -> { + if (field.getSelectionSet() != null) { + this.collectFields(parameters, field.getSelectionSet(), visitedFragments, subFields, null, incrementalSupport); } - this.collectFields(parameters, field.getSelectionSet(), visitedFragments, subFields, null, incrementalSupport); - } + }); return newMergedSelectionSet().subFields(subFields).build(); } @@ -142,11 +141,8 @@ private void collectField(FieldCollectorParameters parameters, Map builder - .addField(field) - .addDeferredExecution(deferredExecution)) - ); + MergedField currentMergedField = fields.get(name); + fields.put(name, currentMergedField.newMergedFieldWith(field,deferredExecution)); } else { fields.put(name, MergedField.newSingletonMergedField(field, deferredExecution)); } diff --git a/src/main/java/graphql/execution/MergedField.java b/src/main/java/graphql/execution/MergedField.java index 64eb43361c..4dce66aa4f 100644 --- a/src/main/java/graphql/execution/MergedField.java +++ b/src/main/java/graphql/execution/MergedField.java @@ -3,9 +3,11 @@ import com.google.common.collect.ImmutableList; import graphql.ExperimentalApi; import graphql.PublicApi; +import graphql.collect.ImmutableKit; import graphql.execution.incremental.DeferredExecution; import graphql.language.Argument; import graphql.language.Field; +import org.jspecify.annotations.NullMarked; import org.jspecify.annotations.Nullable; import java.util.List; @@ -13,11 +15,12 @@ import java.util.function.Consumer; import static graphql.Assert.assertNotEmpty; +import static graphql.Assert.assertNotNull; /** * This represents all Fields in a query which overlap and are merged into one. * This means they all represent the same field actually when the query is executed. - * + *

* Example query with more than one Field merged together: * * 

@@ -45,7 +48,7 @@
  *
* * Here the field is merged together including the sub selections. - * + *

* A third example with different directives: * 

  * {@code
@@ -56,30 +59,24 @@
  * }
  *
* These examples make clear that you need to consider all merged fields together to have the full picture. - * + *

* The actual logic when fields can be successfully merged together is implemented in {#graphql.validation.rules.OverlappingFieldsCanBeMerged} */ @PublicApi +@NullMarked public class MergedField { - private final ImmutableList fields; private final Field singleField; private final ImmutableList deferredExecutions; - private MergedField(ImmutableList fields, ImmutableList deferredExecutions) { - assertNotEmpty(fields); - this.fields = fields; - this.singleField = fields.get(0); + private MergedField(Field field, ImmutableList deferredExecutions) { + this.singleField = field; this.deferredExecutions = deferredExecutions; } - private MergedField(Field field, DeferredExecution deferredExecution) { - this(ImmutableList.of(field), deferredExecution == null ? ImmutableList.of() : ImmutableList.of(deferredExecution)); - } - /** * All merged fields have the same name. - * + *

* WARNING: This is not always the key in the execution result, because of possible aliases. See {@link #getResultKey()} * * @return the name of the merged fields. @@ -100,7 +97,7 @@ public String getResultKey() { /** * The first of the merged fields. - * + *

* Because all fields are almost identically * often only one of the merged fields are used. * @@ -126,7 +123,28 @@ public List getArguments() { * @return all merged fields */ public List getFields() { - return fields; + return ImmutableList.of(singleField); + } + + /** + * @return how many fields are in this merged field + */ + public int getFieldsCount() { + return 1; + } + + /** + * @return true if the field has a sub selection + */ + public boolean hasSubSelection() { + return singleField.getSelectionSet() != null; + } + + /** + * @return true if this {@link MergedField} represents a single {@link Field} in the operation + */ + public boolean isSingleField() { + return true; } /** @@ -149,6 +167,129 @@ public boolean isDeferred() { return !deferredExecutions.isEmpty(); } + @Override + public boolean equals(@Nullable Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + MergedField that = (MergedField) o; + return this.singleField.equals(that.singleField); + } + + @Override + public int hashCode() { + return Objects.hashCode(singleField); + } + + @Override + public String toString() { + return "MergedField{" + + "field(s)=" + singleField + + '}'; + } + + /** + * This is an important method because it creates a new MergedField from the existing one without using a builder + * to save memory. + * + * @param field the new field to add to the current merged field + * @param deferredExecution the deferred execution + * + * @return a new {@link MergedField} instance + */ + MergedField newMergedFieldWith(Field field, @Nullable DeferredExecution deferredExecution) { + ImmutableList deferredExecutions = mkDeferredExecutions(deferredExecution); + ImmutableList fields = ImmutableList.of(singleField, field); + return new MultiMergedField(fields, deferredExecutions); + } + + ImmutableList mkDeferredExecutions(@Nullable DeferredExecution deferredExecution) { + ImmutableList deferredExecutions = this.deferredExecutions; + if (deferredExecution != null) { + deferredExecutions = ImmutableKit.addToList(deferredExecutions, deferredExecution); + } + return deferredExecutions; + } + + /** + * Most of the time we have a single field inside a MergedField but when we need more than one field + * represented then this {@link MultiMergedField} is used + */ + static final class MultiMergedField extends MergedField { + private final ImmutableList fields; + + MultiMergedField(ImmutableList fields, ImmutableList deferredExecutions) { + super(fields.get(0), deferredExecutions); + this.fields = fields; + } + + @Override + public List getFields() { + return fields; + } + + @Override + public boolean hasSubSelection() { + for (Field field : this.fields) { + if (field.getSelectionSet() != null) { + return true; + } + } + return false; + } + + @Override + public int getFieldsCount() { + return fields.size(); + } + + @Override + public boolean isSingleField() { + return fields.size() == 1; + } + + @Override + public boolean equals(@Nullable Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + MultiMergedField that = (MultiMergedField) o; + return fields.equals(that.fields); + } + + @Override + public int hashCode() { + return Objects.hashCode(fields); + } + + @Override + public String toString() { + return "MultiMergedField{" + + "field(s)=" + fields + + '}'; + } + + + @Override + public void forEach(Consumer fieldConsumer) { + fields.forEach(fieldConsumer); + } + + @Override + MergedField newMergedFieldWith(Field field, @Nullable DeferredExecution deferredExecution) { + ImmutableList deferredExecutions = mkDeferredExecutions(deferredExecution); + ImmutableList fields = ImmutableKit.addToList(this.fields, field); + return new MultiMergedField(fields, deferredExecutions); + } + } + + public static Builder newMergedField() { return new Builder(); } @@ -161,8 +302,18 @@ public static Builder newMergedField(List fields) { return new Builder().fields(fields); } - static MergedField newSingletonMergedField(Field field, DeferredExecution deferredExecution) { - return new MergedField(field, deferredExecution); + + /** + * This is an important method in that it creates a MergedField direct without the list and without a builder and hence + * saves some micro memory in not allocating a list of 1 + * + * @param field the field to wrap + * @param deferredExecution the deferred execution + * + * @return a new {@link MergedField} + */ + static MergedField newSingletonMergedField(Field field, @Nullable DeferredExecution deferredExecution) { + return new MergedField(field, deferredExecution == null ? ImmutableList.of() : ImmutableList.of(deferredExecution)); } public MergedField transform(Consumer builderConsumer) { @@ -171,68 +322,114 @@ public MergedField transform(Consumer builderConsumer) { return builder.build(); } + /** + * Runs a consumer for each field + * + * @param fieldConsumer the consumer to run + */ + public void forEach(Consumer fieldConsumer) { + fieldConsumer.accept(singleField); + } + public static class Builder { - private final ImmutableList.Builder fields = new ImmutableList.Builder<>(); - private final ImmutableList.Builder deferredExecutions = new ImmutableList.Builder<>(); + /* + The builder logic is complicated by these dual singleton / list duality code, + but it prevents memory allocation and every bit counts + when the CPU is running hot and an operation has lots of fields! + */ + private ImmutableList.@Nullable Builder fields; + private @Nullable Field singleField; + private ImmutableList.@Nullable Builder deferredExecutions; private Builder() { } private Builder(MergedField existing) { - fields.addAll(existing.getFields()); - deferredExecutions.addAll(existing.deferredExecutions); + if (existing instanceof MultiMergedField) { + this.singleField = null; + this.fields = new ImmutableList.Builder<>(); + this.fields.addAll(existing.getFields()); + } else { + this.singleField = existing.singleField; + } + if (!existing.deferredExecutions.isEmpty()) { + this.deferredExecutions = ensureDeferredExecutionsListBuilder(); + this.deferredExecutions.addAll(existing.deferredExecutions); + } + } + + private ImmutableList.Builder ensureDeferredExecutionsListBuilder() { + if (this.deferredExecutions == null) { + this.deferredExecutions = new ImmutableList.Builder<>(); + } + return this.deferredExecutions; + } + + private ImmutableList.Builder ensureFieldsListBuilder() { + if (this.fields == null) { + this.fields = new ImmutableList.Builder<>(); + if (this.singleField != null) { + this.fields.add(this.singleField); + this.singleField = null; + } + } + return this.fields; } public Builder fields(List fields) { - this.fields.addAll(fields); + if (singleField == null && this.fields == null && fields.size() == 1) { + // even if you present a list - if its a list of one, we dont allocate a list + singleField = fields.get(0); + return this; + } else { + this.fields = ensureFieldsListBuilder(); + this.fields.addAll(fields); + } return this; } public Builder addField(Field field) { + if (singleField == null && this.fields == null) { + singleField = field; + return this; + } else { + this.fields = ensureFieldsListBuilder(); + } this.fields.add(field); return this; } public Builder addDeferredExecutions(List deferredExecutions) { - this.deferredExecutions.addAll(deferredExecutions); + if (!deferredExecutions.isEmpty()) { + this.deferredExecutions = ensureDeferredExecutionsListBuilder(); + this.deferredExecutions.addAll(deferredExecutions); + } return this; } + @SuppressWarnings("UnusedReturnValue") public Builder addDeferredExecution(@Nullable DeferredExecution deferredExecution) { - if(deferredExecution != null) { + if (deferredExecution != null) { + this.deferredExecutions = ensureDeferredExecutionsListBuilder(); this.deferredExecutions.add(deferredExecution); } return this; } public MergedField build() { - return new MergedField(fields.build(), deferredExecutions.build()); - } - - } - - @Override - public boolean equals(Object o) { - if (this == o) { - return true; - } - if (o == null || getClass() != o.getClass()) { - return false; + ImmutableList deferredExecutions; + if (this.deferredExecutions == null) { + deferredExecutions = ImmutableList.of(); + } else { + deferredExecutions = this.deferredExecutions.build(); + } + if (this.singleField != null && this.fields == null) { + return new MergedField(singleField, deferredExecutions); + } + ImmutableList fields = assertNotNull(this.fields, () -> "You MUST add at least one field via the builder").build(); + assertNotEmpty(fields); + return new MultiMergedField(fields, deferredExecutions); } - MergedField that = (MergedField) o; - return fields.equals(that.fields); - } - - @Override - public int hashCode() { - return Objects.hashCode(fields); - } - - @Override - public String toString() { - return "MergedField{" + - "fields=" + fields + - '}'; } } diff --git a/src/main/java/graphql/execution/directives/QueryDirectivesImpl.java b/src/main/java/graphql/execution/directives/QueryDirectivesImpl.java index 4b75df378e..87f00d6f97 100644 --- a/src/main/java/graphql/execution/directives/QueryDirectivesImpl.java +++ b/src/main/java/graphql/execution/directives/QueryDirectivesImpl.java @@ -69,7 +69,7 @@ private void computeValuesLazily() { BiMap directiveCounterParts = HashBiMap.create(); BiMap gqlDirectiveCounterParts = HashBiMap.create(); BiMap gqlDirectiveCounterPartsInverse = gqlDirectiveCounterParts.inverse(); - mergedField.getFields().forEach(field -> { + mergedField.forEach(field -> { List directives = field.getDirectives(); BiMap directivesMap = directivesResolver .resolveDirectives(directives, schema, coercedVariables, graphQLContext, locale); diff --git a/src/main/java/graphql/execution/incremental/DeferredExecution.java b/src/main/java/graphql/execution/incremental/DeferredExecution.java index 321ecc1603..ae63808989 100644 --- a/src/main/java/graphql/execution/incremental/DeferredExecution.java +++ b/src/main/java/graphql/execution/incremental/DeferredExecution.java @@ -22,4 +22,18 @@ public DeferredExecution(String label) { public String getLabel() { return label; } + + // this class uses object identity - do not put .equals() / .hashCode() implementations on it + // otherwise it will break defer handling. I have put the code just to be explicit that object identity + // is needed + + @Override + public int hashCode() { + return super.hashCode(); + } + + @Override + public boolean equals(Object obj) { + return super.equals(obj); + } } diff --git a/src/main/java/graphql/execution/incremental/DeferredExecutionSupport.java b/src/main/java/graphql/execution/incremental/DeferredExecutionSupport.java index 3055e14c48..9a46020d83 100644 --- a/src/main/java/graphql/execution/incremental/DeferredExecutionSupport.java +++ b/src/main/java/graphql/execution/incremental/DeferredExecutionSupport.java @@ -74,7 +74,7 @@ public DeferredExecutionSupportImpl( ImmutableList.Builder nonDeferredFieldNamesBuilder = ImmutableList.builder(); mergedSelectionSet.getSubFields().values().forEach(mergedField -> { - if (mergedField.getFields().size() > mergedField.getDeferredExecutions().size()) { + if (mergedField.getFieldsCount() > mergedField.getDeferredExecutions().size()) { nonDeferredFieldNamesBuilder.add(mergedField.getSingleField().getResultKey()); return; } diff --git a/src/test/groovy/graphql/execution/MergedFieldTest.groovy b/src/test/groovy/graphql/execution/MergedFieldTest.groovy new file mode 100644 index 0000000000..f327339891 --- /dev/null +++ b/src/test/groovy/graphql/execution/MergedFieldTest.groovy @@ -0,0 +1,179 @@ +package graphql.execution + +import graphql.execution.incremental.DeferredExecution +import graphql.language.Field +import graphql.language.SelectionSet +import spock.lang.Specification + +class MergedFieldTest extends Specification { + + def fa_1 = Field.newField("fa").build() + def fa_2 = Field.newField("fa").build() + def fa_3 = Field.newField("fa").build() + def alias1 = Field.newField("a1").alias("alias1").build() + def ss = SelectionSet.newSelectionSet([fa_1, fa_2]).build() + def sub1 = Field.newField("s1").selectionSet(ss).build() + def deferred1 = new DeferredExecution("defer1") + def deferred2 = new DeferredExecution("defer2") + + def "can construct from a single field"() { + when: + def mergedField = MergedField.newSingletonMergedField(fa_1, null) + then: + mergedField.getName() == "fa" + mergedField.getResultKey() == "fa" + mergedField.getSingleField() == fa_1 + mergedField.isSingleField() + !mergedField.hasSubSelection() + // lazily make the list + mergedField.getFields() == [fa_1] + + // these should still work + mergedField.getName() == "fa" + mergedField.getResultKey() == "fa" + mergedField.getSingleField() == fa_1 + mergedField.isSingleField() + !mergedField.hasSubSelection() + } + + def "can construct from multiple fields"() { + when: + def mergedField = MergedField.newMergedField().addField(fa_1).addField(fa_2).build() + + then: + mergedField.getName() == "fa" + mergedField.getResultKey() == "fa" + mergedField.getSingleField() == fa_1 + !mergedField.isSingleField() + !mergedField.hasSubSelection() + mergedField.getFields() == [fa_1, fa_2] + } + + def "can have aliases"() { + when: + def mergedField = MergedField.newSingletonMergedField(alias1, null) + then: + mergedField.getName() == "a1" + mergedField.getResultKey() == "alias1" + mergedField.getSingleField() == alias1 + mergedField.isSingleField() + !mergedField.hasSubSelection() + } + + def "can have selection set on single field"() { + when: + def mergedField = MergedField.newSingletonMergedField(sub1, null) + then: + mergedField.getName() == "s1" + mergedField.getResultKey() == "s1" + mergedField.getSingleField() == sub1 + mergedField.isSingleField() + mergedField.hasSubSelection() + } + + def "builder can build a singleton via addField()"() { + when: + def mergedField = MergedField.newMergedField().addField(sub1).build() + then: + mergedField.getName() == "s1" + mergedField.getResultKey() == "s1" + mergedField.getSingleField() == sub1 + mergedField.isSingleField() + mergedField.hasSubSelection() + } + + def "builder can build a multi via addField() / addField() combo"() { + when: + def mergedField = MergedField.newMergedField().addField(fa_1).addField(fa_2).build() + then: + mergedField.getName() == "fa" + mergedField.getResultKey() == "fa" + mergedField.getSingleField() == fa_1 + !mergedField.isSingleField() + mergedField.getFields() == [fa_1, fa_2] + } + + def "builder can build a multi via addField() / fields() combo"() { + when: + def mergedField = MergedField.newMergedField().addField(fa_1).fields([fa_2, fa_3]).build() + then: + mergedField.getName() == "fa" + mergedField.getResultKey() == "fa" + mergedField.getSingleField() == fa_1 + !mergedField.isSingleField() + mergedField.getFields() == [fa_1, fa_2, fa_3] + } + + def "builder can build a multi via fields()"() { + when: + def mergedField = MergedField.newMergedField().fields([fa_1, fa_2, fa_3]).build() + then: + mergedField.getName() == "fa" + mergedField.getResultKey() == "fa" + mergedField.getSingleField() == fa_1 + !mergedField.isSingleField() + mergedField.getFields() == [fa_1, fa_2, fa_3] + } + + def "can add to an existing field"() { + when: + def mergedField = MergedField.newSingletonMergedField(fa_1, null) + then: + mergedField.getName() == "fa" + mergedField.getDeferredExecutions().isEmpty() + mergedField.getSingleField() == fa_1 + mergedField.getFields() == [fa_1] + + when: + def mergedField2 = mergedField.newMergedFieldWith(fa_2, null) + then: + mergedField2.getName() == "fa" + mergedField2.getDeferredExecutions().isEmpty() + mergedField2.getSingleField() == fa_1 + mergedField2.getFields() == [fa_1, fa_2] + + // its a new instance + !(mergedField2 === mergedField) + } + + def "builder can handle no deferred executions"() { + when: + def mergedField = MergedField.newMergedField().addField(fa_1) + .addDeferredExecutions([]).build() + then: + mergedField.getName() == "fa" + mergedField.getSingleField() == fa_1 + mergedField.getDeferredExecutions().isEmpty() + } + + def "builder can handle list of deferred executions"() { + when: + def mergedField = MergedField.newMergedField().addField(fa_1) + .addDeferredExecutions([deferred1, deferred2]).build() + then: + mergedField.getName() == "fa" + mergedField.getSingleField() == fa_1 + mergedField.getDeferredExecutions() == [deferred1, deferred2] + + } + + def "builder can handle a single deferred executions"() { + when: + def mergedField = MergedField.newMergedField().addField(fa_1) + .addDeferredExecution(deferred1).build() + then: + mergedField.getName() == "fa" + mergedField.getSingleField() == fa_1 + mergedField.getDeferredExecutions() == [deferred1] + } + + def "builder can handle a single deferred execution at a time"() { + when: + def mergedField = MergedField.newMergedField().addField(fa_1) + .addDeferredExecution(deferred1).addDeferredExecution(deferred2).build() + then: + mergedField.getName() == "fa" + mergedField.getSingleField() == fa_1 + mergedField.getDeferredExecutions() == [deferred1,deferred2] + } +}