Skip to content

20.x Backport PR 3539 #3542

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
dondonz merged 5 commits into from
Mar 26, 2024
Merged

20.x Backport PR 3539 #3542

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
469caf6
Cherry pick PR 3539 and update ConditionalNodes and related files to …
dondonz Mar 24, 2024
1f5e70b
Add explainer note to v20 introspection node count being smaller by 1
dondonz Mar 25, 2024
965ae5c
remove conditional node decision feature and cleanup
andimarek Mar 25, 2024
c3d5bb4
Merge remote-tracking branch 'origin/20.x-backport-enf-introspection'…
andimarek Mar 25, 2024
851bcd6
Fix typo (backport PR #3544)
dondonz Mar 26, 2024
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
17 changes: 17 additions & 0 deletions src/main/java/graphql/Assert.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,20 @@ public static void assertTrue(boolean condition) {
throw new AssertException("condition expected to be true");
}

public static void assertTrue(boolean condition, String constantMsg) {
if (condition) {
return;
}
throwAssert(constantMsg);
}

public static void assertTrue(boolean condition, String msgFmt, Object arg1) {
if (condition) {
return;
}
throwAssert(msgFmt, arg1);
}

Copy link
Member Author

@dondonz dondonz Mar 24, 2024

Choose a reason for hiding this comment

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

Backport utils

public static void assertFalse(boolean condition, Supplier<String> msg) {
if (!condition) {
return;
Expand Down Expand Up @@ -117,4 +131,7 @@ public static String assertValidName(String name) {
throw new AssertException(String.format(invalidNameErrorMessage, name));
}

private static <T> T throwAssert(String format, Object... args) {
throw new AssertException(format(format, args));
}
}
40 changes: 22 additions & 18 deletions src/main/java/graphql/analysis/NodeVisitorWithTypeTracking.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
import graphql.GraphQLContext;
Copy link
Member Author

@dondonz dondonz Mar 24, 2024

Choose a reason for hiding this comment

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

Update file to bring in line with v21 - it's because ConditionalNodes are referenced in the ExecutableNormalizedOperationFactory, and those changes are viral in a few other files

import graphql.Internal;
import graphql.execution.CoercedVariables;
import graphql.execution.ConditionalNodes;
import graphql.execution.ValuesResolver;
import graphql.execution.conditional.ConditionalNodes;
import graphql.introspection.Introspection;
import graphql.language.Argument;
import graphql.language.Directive;
Expand Down Expand Up @@ -68,7 +68,9 @@ public TraversalControl visitDirective(Directive node, TraverserContext<Node> co

@Override
public TraversalControl visitInlineFragment(InlineFragment inlineFragment, TraverserContext<Node> context) {
if (!conditionalNodes.shouldInclude(variables, inlineFragment.getDirectives())) {
QueryTraversalContext parentEnv = context.getVarFromParents(QueryTraversalContext.class);
GraphQLContext graphQLContext = parentEnv.getGraphQLContext();
if (!conditionalNodes.shouldInclude(inlineFragment, variables, null, graphQLContext)) {
return TraversalControl.ABORT;
}

Expand All @@ -82,7 +84,6 @@ public TraversalControl visitInlineFragment(InlineFragment inlineFragment, Trave
preOrderCallback.visitInlineFragment(inlineFragmentEnvironment);

// inline fragments are allowed not have type conditions, if so the parent type counts
QueryTraversalContext parentEnv = context.getVarFromParents(QueryTraversalContext.class);

GraphQLCompositeType fragmentCondition;
if (inlineFragment.getTypeCondition() != null) {
Expand All @@ -92,38 +93,41 @@ public TraversalControl visitInlineFragment(InlineFragment inlineFragment, Trave
fragmentCondition = parentEnv.getUnwrappedOutputType();
}
// for unions we only have other fragments inside
context.setVar(QueryTraversalContext.class, new QueryTraversalContext(fragmentCondition, parentEnv.getEnvironment(), inlineFragment));
context.setVar(QueryTraversalContext.class, new QueryTraversalContext(fragmentCondition, parentEnv.getEnvironment(), inlineFragment, graphQLContext));
return TraversalControl.CONTINUE;
}

@Override
public TraversalControl visitFragmentDefinition(FragmentDefinition node, TraverserContext<Node> context) {
if (!conditionalNodes.shouldInclude(variables, node.getDirectives())) {
public TraversalControl visitFragmentDefinition(FragmentDefinition fragmentDefinition, TraverserContext<Node> context) {
QueryTraversalContext parentEnv = context.getVarFromParents(QueryTraversalContext.class);
GraphQLContext graphQLContext = parentEnv.getGraphQLContext();
if (!conditionalNodes.shouldInclude(fragmentDefinition, variables, null, graphQLContext)) {
return TraversalControl.ABORT;
}

QueryVisitorFragmentDefinitionEnvironment fragmentEnvironment = new QueryVisitorFragmentDefinitionEnvironmentImpl(node, context, schema);
QueryVisitorFragmentDefinitionEnvironment fragmentEnvironment = new QueryVisitorFragmentDefinitionEnvironmentImpl(fragmentDefinition, context, schema);

if (context.getPhase() == LEAVE) {
postOrderCallback.visitFragmentDefinition(fragmentEnvironment);
return TraversalControl.CONTINUE;
}
preOrderCallback.visitFragmentDefinition(fragmentEnvironment);

QueryTraversalContext parentEnv = context.getVarFromParents(QueryTraversalContext.class);
GraphQLCompositeType typeCondition = (GraphQLCompositeType) schema.getType(node.getTypeCondition().getName());
context.setVar(QueryTraversalContext.class, new QueryTraversalContext(typeCondition, parentEnv.getEnvironment(), node));
GraphQLCompositeType typeCondition = (GraphQLCompositeType) schema.getType(fragmentDefinition.getTypeCondition().getName());
context.setVar(QueryTraversalContext.class, new QueryTraversalContext(typeCondition, parentEnv.getEnvironment(), fragmentDefinition, graphQLContext));
return TraversalControl.CONTINUE;
}

@Override
public TraversalControl visitFragmentSpread(FragmentSpread fragmentSpread, TraverserContext<Node> context) {
if (!conditionalNodes.shouldInclude(variables, fragmentSpread.getDirectives())) {
QueryTraversalContext parentEnv = context.getVarFromParents(QueryTraversalContext.class);
GraphQLContext graphQLContext = parentEnv.getGraphQLContext();
if (!conditionalNodes.shouldInclude(fragmentSpread, variables, null, graphQLContext)) {
return TraversalControl.ABORT;
}

FragmentDefinition fragmentDefinition = fragmentsByName.get(fragmentSpread.getName());
if (!conditionalNodes.shouldInclude(variables, fragmentDefinition.getDirectives())) {
if (!conditionalNodes.shouldInclude(fragmentDefinition, variables, null, graphQLContext)) {
return TraversalControl.ABORT;
}

Expand All @@ -135,19 +139,19 @@ public TraversalControl visitFragmentSpread(FragmentSpread fragmentSpread, Trave

preOrderCallback.visitFragmentSpread(fragmentSpreadEnvironment);

QueryTraversalContext parentEnv = context.getVarFromParents(QueryTraversalContext.class);

GraphQLCompositeType typeCondition = (GraphQLCompositeType) schema.getType(fragmentDefinition.getTypeCondition().getName());
assertNotNull(typeCondition,
() -> format("Invalid type condition '%s' in fragment '%s'", fragmentDefinition.getTypeCondition().getName(),
fragmentDefinition.getName()));
context.setVar(QueryTraversalContext.class, new QueryTraversalContext(typeCondition, parentEnv.getEnvironment(), fragmentDefinition));
context.setVar(QueryTraversalContext.class, new QueryTraversalContext(typeCondition, parentEnv.getEnvironment(), fragmentDefinition, graphQLContext));
return TraversalControl.CONTINUE;
}

@Override
public TraversalControl visitField(Field field, TraverserContext<Node> context) {
QueryTraversalContext parentEnv = context.getVarFromParents(QueryTraversalContext.class);
GraphQLContext graphQLContext = parentEnv.getGraphQLContext();

GraphQLFieldDefinition fieldDefinition = Introspection.getFieldDef(schema, (GraphQLCompositeType) unwrapAll(parentEnv.getOutputType()), field.getName());
boolean isTypeNameIntrospectionField = fieldDefinition == schema.getIntrospectionTypenameFieldDefinition();
Expand All @@ -174,16 +178,16 @@ public TraversalControl visitField(Field field, TraverserContext<Node> context)
return TraversalControl.CONTINUE;
}

if (!conditionalNodes.shouldInclude(variables, field.getDirectives())) {
if (!conditionalNodes.shouldInclude(field, variables, null, graphQLContext)) {
return TraversalControl.ABORT;
}

TraversalControl traversalControl = preOrderCallback.visitFieldWithControl(environment);

GraphQLUnmodifiedType unmodifiedType = unwrapAll(fieldDefinition.getType());
QueryTraversalContext fieldEnv = (unmodifiedType instanceof GraphQLCompositeType)
? new QueryTraversalContext(fieldDefinition.getType(), environment, field)
: new QueryTraversalContext(null, environment, field);// Terminal (scalar) node, EMPTY FRAME
? new QueryTraversalContext(fieldDefinition.getType(), environment, field, graphQLContext)
: new QueryTraversalContext(null, environment, field, graphQLContext);// Terminal (scalar) node, EMPTY FRAME


context.setVar(QueryTraversalContext.class, fieldEnv);
Expand Down Expand Up @@ -259,4 +263,4 @@ protected TraversalControl visitValue(Value<?> value, TraverserContext<Node> con
}
return preOrderCallback.visitArgumentValue(environment);
}
}
}
5 changes: 3 additions & 2 deletions src/main/java/graphql/analysis/QueryTransformer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package graphql.analysis;
Copy link
Member Author

@dondonz dondonz Mar 24, 2024

Choose a reason for hiding this comment

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

Update file to bring in line with v21 - it's because ConditionalNodes are referenced in the ExecutableNormalizedOperationFactory, and those changes are viral in a few other files


import graphql.GraphQLContext;
import graphql.PublicApi;
import graphql.language.FragmentDefinition;
import graphql.language.Node;
Expand Down Expand Up @@ -67,7 +68,7 @@ public Node transform(QueryVisitor queryVisitor) {
NodeVisitorWithTypeTracking nodeVisitor = new NodeVisitorWithTypeTracking(queryVisitor, noOp, variables, schema, fragmentsByName);

Map<Class<?>, Object> rootVars = new LinkedHashMap<>();
rootVars.put(QueryTraversalContext.class, new QueryTraversalContext(rootParentType, null, null));
rootVars.put(QueryTraversalContext.class, new QueryTraversalContext(rootParentType, null, null, GraphQLContext.getDefault()));

TraverserVisitor<Node> nodeTraverserVisitor = new TraverserVisitor<Node>() {

Expand Down Expand Up @@ -163,4 +164,4 @@ public QueryTransformer build() {
return new QueryTransformer(schema, root, rootParentType, fragmentsByName, variables);
}
}
}
}
18 changes: 12 additions & 6 deletions src/main/java/graphql/analysis/QueryTraversalContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package graphql.analysis;
Copy link
Member Author

@dondonz dondonz Mar 24, 2024

Choose a reason for hiding this comment

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

Update file to bring in line with v21 - it's because ConditionalNodes are referenced in the ExecutableNormalizedOperationFactory, and those changes are viral in a few other files


import graphql.GraphQLContext;
import graphql.Internal;
import graphql.language.SelectionSetContainer;
import graphql.schema.GraphQLCompositeType;
Expand All @@ -16,14 +17,17 @@ class QueryTraversalContext {
// never used for scalars/enums, always a possibly wrapped composite type
private final GraphQLOutputType outputType;
private final QueryVisitorFieldEnvironment environment;
private final SelectionSetContainer selectionSetContainer;
private final SelectionSetContainer<?> selectionSetContainer;
private final GraphQLContext graphQLContext;

QueryTraversalContext(GraphQLOutputType outputType,
QueryVisitorFieldEnvironment environment,
SelectionSetContainer selectionSetContainer) {
SelectionSetContainer<?> selectionSetContainer,
GraphQLContext graphQLContext) {
this.outputType = outputType;
this.environment = environment;
this.selectionSetContainer = selectionSetContainer;
this.graphQLContext = graphQLContext;
}

public GraphQLOutputType getOutputType() {
Expand All @@ -34,13 +38,15 @@ public GraphQLCompositeType getUnwrappedOutputType() {
return (GraphQLCompositeType) GraphQLTypeUtil.unwrapAll(outputType);
}


public QueryVisitorFieldEnvironment getEnvironment() {
return environment;
}

public SelectionSetContainer getSelectionSetContainer() {

public SelectionSetContainer<?> getSelectionSetContainer() {
return selectionSetContainer;
}
}

public GraphQLContext getGraphQLContext() {
return graphQLContext;
}
}
4 changes: 2 additions & 2 deletions src/main/java/graphql/analysis/QueryTraverser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ private List<Node> childrenOf(Node<?> node) {

Copy link
Member Author

@dondonz dondonz Mar 24, 2024

Choose a reason for hiding this comment

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

Update file to bring in line with v21 - it's because ConditionalNodes are referenced in the ExecutableNormalizedOperationFactory, and those changes are viral in a few other files

private Object visitImpl(QueryVisitor visitFieldCallback, Boolean preOrder) {
Map<Class<?>, Object> rootVars = new LinkedHashMap<>();
rootVars.put(QueryTraversalContext.class, new QueryTraversalContext(rootParentType, null, null));
rootVars.put(QueryTraversalContext.class, new QueryTraversalContext(rootParentType, null, null, GraphQLContext.getDefault()));

QueryVisitor preOrderCallback;
QueryVisitor postOrderCallback;
Expand Down Expand Up @@ -343,4 +343,4 @@ private void checkState() {
}

}
}
}
2 changes: 1 addition & 1 deletion src/main/java/graphql/execution/ExecutionContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public class ExecutionContext {
this.errors.set(builder.errors);
this.localContext = builder.localContext;
this.executionInput = builder.executionInput;
queryTree = FpKit.interThreadMemoize(() -> ExecutableNormalizedOperationFactory.createExecutableNormalizedOperation(graphQLSchema, operationDefinition, fragmentsByName, coercedVariables));
this.queryTree = FpKit.interThreadMemoize(() -> ExecutableNormalizedOperationFactory.createExecutableNormalizedOperation(graphQLSchema, operationDefinition, fragmentsByName, coercedVariables));
}


Expand Down
23 changes: 18 additions & 5 deletions src/main/java/graphql/execution/FieldCollector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

Copy link
Member Author

@dondonz dondonz Mar 24, 2024

Choose a reason for hiding this comment

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

Update file to bring in line with v21 - it's because ConditionalNodes are referenced in the ExecutableNormalizedOperationFactory, and those changes are viral in a few other files


import graphql.Internal;
import graphql.execution.conditional.ConditionalNodes;
import graphql.language.Field;
import graphql.language.FragmentDefinition;
import graphql.language.FragmentSpread;
Expand Down Expand Up @@ -76,13 +77,19 @@ private void collectFragmentSpread(FieldCollectorParameters parameters, Set<Stri
if (visitedFragments.contains(fragmentSpread.getName())) {
return;
}
if (!conditionalNodes.shouldInclude(parameters.getVariables(), fragmentSpread.getDirectives())) {
if (!conditionalNodes.shouldInclude(fragmentSpread,
parameters.getVariables(),
parameters.getGraphQLSchema(),
parameters.getGraphQLContext())) {
return;
}
visitedFragments.add(fragmentSpread.getName());
FragmentDefinition fragmentDefinition = parameters.getFragmentsByName().get(fragmentSpread.getName());

if (!conditionalNodes.shouldInclude(parameters.getVariables(), fragmentDefinition.getDirectives())) {
if (!conditionalNodes.shouldInclude(fragmentDefinition,
parameters.getVariables(),
parameters.getGraphQLSchema(),
parameters.getGraphQLContext())) {
return;
}
if (!doesFragmentConditionMatch(parameters, fragmentDefinition)) {
Expand All @@ -92,15 +99,21 @@ private void collectFragmentSpread(FieldCollectorParameters parameters, Set<Stri
}

private void collectInlineFragment(FieldCollectorParameters parameters, Set<String> visitedFragments, Map<String, MergedField> fields, InlineFragment inlineFragment) {
if (!conditionalNodes.shouldInclude(parameters.getVariables(), inlineFragment.getDirectives()) ||
if (!conditionalNodes.shouldInclude(inlineFragment,
parameters.getVariables(),
parameters.getGraphQLSchema(),
parameters.getGraphQLContext()) ||
!doesFragmentConditionMatch(parameters, inlineFragment)) {
return;
}
collectFields(parameters, inlineFragment.getSelectionSet(), visitedFragments, fields);
}

private void collectField(FieldCollectorParameters parameters, Map<String, MergedField> fields, Field field) {
if (!conditionalNodes.shouldInclude(parameters.getVariables(), field.getDirectives())) {
if (!conditionalNodes.shouldInclude(field,
parameters.getVariables(),
parameters.getGraphQLSchema(),
parameters.getGraphQLContext())) {
return;
}
String name = field.getResultKey();
Expand Down Expand Up @@ -143,4 +156,4 @@ private boolean checkTypeCondition(FieldCollectorParameters parameters, GraphQLT
}


}
}
27 changes: 20 additions & 7 deletions src/main/java/graphql/execution/FieldCollectorParameters.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package graphql.execution;
Copy link
Member Author

@dondonz dondonz Mar 24, 2024

Choose a reason for hiding this comment

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

Update file to bring in line with v21 - it's because ConditionalNodes are referenced in the ExecutableNormalizedOperationFactory, and those changes are viral in a few other files


import graphql.Assert;
import graphql.GraphQLContext;
import graphql.Internal;
import graphql.language.FragmentDefinition;
import graphql.schema.GraphQLObjectType;
Expand All @@ -17,6 +18,7 @@ public class FieldCollectorParameters {
private final Map<String, FragmentDefinition> fragmentsByName;
private final Map<String, Object> variables;
private final GraphQLObjectType objectType;
private final GraphQLContext graphQLContext;

public GraphQLSchema getGraphQLSchema() {
return graphQLSchema;
Expand All @@ -34,11 +36,16 @@ public GraphQLObjectType getObjectType() {
return objectType;
}

private FieldCollectorParameters(GraphQLSchema graphQLSchema, Map<String, Object> variables, Map<String, FragmentDefinition> fragmentsByName, GraphQLObjectType objectType) {
this.fragmentsByName = fragmentsByName;
this.graphQLSchema = graphQLSchema;
this.variables = variables;
this.objectType = objectType;
public GraphQLContext getGraphQLContext() {
return graphQLContext;
}

private FieldCollectorParameters(Builder builder) {
this.fragmentsByName = builder.fragmentsByName;
this.graphQLSchema = builder.graphQLSchema;
this.variables = builder.variables;
this.objectType = builder.objectType;
this.graphQLContext = builder.graphQLContext;
}

public static Builder newParameters() {
Expand All @@ -50,6 +57,7 @@ public static class Builder {
private Map<String, FragmentDefinition> fragmentsByName;
private Map<String, Object> variables;
private GraphQLObjectType objectType;
private GraphQLContext graphQLContext = GraphQLContext.getDefault();

/**
* @see FieldCollectorParameters#newParameters()
Expand All @@ -68,6 +76,11 @@ public Builder objectType(GraphQLObjectType objectType) {
return this;
}

public Builder graphQLContext(GraphQLContext graphQLContext) {
this.graphQLContext = graphQLContext;
return this;
}

public Builder fragments(Map<String, FragmentDefinition> fragmentsByName) {
this.fragmentsByName = fragmentsByName;
return this;
Expand All @@ -80,8 +93,8 @@ public Builder variables(Map<String, Object> variables) {

public FieldCollectorParameters build() {
Assert.assertNotNull(graphQLSchema, () -> "You must provide a schema");
return new FieldCollectorParameters(graphQLSchema, variables, fragmentsByName, objectType);
return new FieldCollectorParameters(this);
}

}
}
}
Loading