Skip to content

Smarter schema visitor #3087

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
bbakerman merged 9 commits into from
May 29, 2023
Merged

Smarter schema visitor #3087

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
f9ce2b3
WIP - Smarter type visitor
bbakerman Feb 6, 2023
c92b102
Renamed a few things and added per callback envs
bbakerman Feb 10, 2023
b869dce
Now with control methods in the env
bbakerman Feb 13, 2023
07def39
More work on smarter schema visitors
bbakerman Feb 27, 2023
33cea4a
Added schema to root vars
bbakerman Feb 27, 2023
18bf953
Test for changing schema
bbakerman Feb 27, 2023
792f1cb
Added a new control type so we can know when thigns have changed
bbakerman Mar 6, 2023
9f00d25
Fixed SchemaTraverser so it can QUIT
bbakerman Mar 6, 2023
1e28acc
Made class public
bbakerman Mar 6, 2023
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
60 changes: 31 additions & 29 deletions src/main/java/graphql/schema/GraphQLTypeVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,54 +13,56 @@
*/
@PublicApi
public interface GraphQLTypeVisitor {
TraversalControl visitGraphQLArgument(GraphQLArgument node, TraverserContext<GraphQLSchemaElement> context);

TraversalControl visitGraphQLInterfaceType(GraphQLInterfaceType node, TraverserContext<GraphQLSchemaElement> context);

TraversalControl visitGraphQLEnumType(GraphQLEnumType node, TraverserContext<GraphQLSchemaElement> context);

TraversalControl visitGraphQLEnumValueDefinition(GraphQLEnumValueDefinition node, TraverserContext<GraphQLSchemaElement> context);

TraversalControl visitGraphQLFieldDefinition(GraphQLFieldDefinition node, TraverserContext<GraphQLSchemaElement> context);

/**
* This method will be called twice. Once for a directive definition in a schema and then do each time a directive is applied to a schema element
* This method will be called when a directive is applied to a schema element.
*
* When it's applied to a schema element then {@link TraverserContext#getParentNode()} will be the schema element that this is applied to.
* The {@link TraverserContext#getParentNode()} will be the schema element that this is applied to.
*
* The graphql-java code base is trying to slowly move away from using {@link GraphQLDirective}s when they really should be {@link GraphQLAppliedDirective}s
* and this is another place that has been left in. In the future this behavior will change and this will only visit directive definitions of a schema, not where
* they are applied.
*
* @param node the directive
* @param node the applied directive
* @param context the traversal context
*
* @return how to control the visitation processing
*/
TraversalControl visitGraphQLDirective(GraphQLDirective node, TraverserContext<GraphQLSchemaElement> context);
Copy link
Copy Markdown
Member Author

@bbakerman bbakerman Mar 6, 2023

Choose a reason for hiding this comment

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

I ordered all the methods here in alphabetic order so I can know what's what

default TraversalControl visitGraphQLAppliedDirective(GraphQLAppliedDirective node, TraverserContext<GraphQLSchemaElement> context) {
return TraversalControl.CONTINUE;
}

default TraversalControl visitGraphQLAppliedDirectiveArgument(GraphQLAppliedDirectiveArgument node, TraverserContext<GraphQLSchemaElement> context) {
return TraversalControl.CONTINUE;
}

TraversalControl visitGraphQLArgument(GraphQLArgument node, TraverserContext<GraphQLSchemaElement> context);

/**
* This method will be called when a directive is applied to a schema element.
* This method will be called twice. Once for a directive definition in a schema and then do each time a directive is applied to a schema element
*
* The {@link TraverserContext#getParentNode()} will be the schema element that this is applied to.
* When it's applied to a schema element then {@link TraverserContext#getParentNode()} will be the schema element that this is applied to.
*
* The graphql-java code base is trying to slowly move away from using {@link GraphQLDirective}s when they really should be {@link GraphQLAppliedDirective}s
* and this is another place that has been left in. In the future this behavior will change and this will only visit directive definitions of a schema, not where
* they are applied.
*
* @param node the applied directive
* @param node the directive
* @param context the traversal context
*
* @return how to control the visitation processing
*/
default TraversalControl visitGraphQLAppliedDirective(GraphQLAppliedDirective node, TraverserContext<GraphQLSchemaElement> context) {
return TraversalControl.CONTINUE;
}
TraversalControl visitGraphQLDirective(GraphQLDirective node, TraverserContext<GraphQLSchemaElement> context);

default TraversalControl visitGraphQLAppliedDirectiveArgument(GraphQLAppliedDirectiveArgument node, TraverserContext<GraphQLSchemaElement> context) {
return TraversalControl.CONTINUE;
}
TraversalControl visitGraphQLEnumType(GraphQLEnumType node, TraverserContext<GraphQLSchemaElement> context);

TraversalControl visitGraphQLEnumValueDefinition(GraphQLEnumValueDefinition node, TraverserContext<GraphQLSchemaElement> context);

TraversalControl visitGraphQLFieldDefinition(GraphQLFieldDefinition node, TraverserContext<GraphQLSchemaElement> context);

TraversalControl visitGraphQLInputObjectField(GraphQLInputObjectField node, TraverserContext<GraphQLSchemaElement> context);

TraversalControl visitGraphQLInputObjectType(GraphQLInputObjectType node, TraverserContext<GraphQLSchemaElement> context);

TraversalControl visitGraphQLInterfaceType(GraphQLInterfaceType node, TraverserContext<GraphQLSchemaElement> context);

TraversalControl visitGraphQLList(GraphQLList node, TraverserContext<GraphQLSchemaElement> context);

TraversalControl visitGraphQLNonNull(GraphQLNonNull node, TraverserContext<GraphQLSchemaElement> context);
Expand All @@ -86,10 +88,6 @@ default TraversalControl visitBackRef(TraverserContext<GraphQLSchemaElement> con
}

// Marker interfaces
default TraversalControl visitGraphQLModifiedType(GraphQLModifiedType node, TraverserContext<GraphQLSchemaElement> context) {
throw new UnsupportedOperationException();
}

default TraversalControl visitGraphQLCompositeType(GraphQLCompositeType node, TraverserContext<GraphQLSchemaElement> context) {
throw new UnsupportedOperationException();
}
Expand All @@ -110,6 +108,10 @@ default TraversalControl visitGraphQLInputType(GraphQLInputType node, TraverserC
throw new UnsupportedOperationException();
}

default TraversalControl visitGraphQLModifiedType(GraphQLModifiedType node, TraverserContext<GraphQLSchemaElement> context) {
throw new UnsupportedOperationException();
}

default TraversalControl visitGraphQLNullableType(GraphQLNullableType node, TraverserContext<GraphQLSchemaElement> context) {
throw new UnsupportedOperationException();
}
Expand Down
15 changes: 9 additions & 6 deletions src/main/java/graphql/schema/SchemaTransformer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,14 +144,14 @@ private Object transformImpl(final GraphQLSchema schema, GraphQLSchemaElement sc
final Map<String, GraphQLTypeReference> typeReferences = new LinkedHashMap<>();

// first pass - general transformation
boolean schemaChanged = traverseAndTransform(dummyRoot, changedTypes, typeReferences, visitor, codeRegistry);
boolean schemaChanged = traverseAndTransform(dummyRoot, changedTypes, typeReferences, visitor, codeRegistry, schema);

// if we have changed any named elements AND we have type references referring to them then
// we need to make a second pass to replace these type references to the new names
if (!changedTypes.isEmpty()) {
boolean hasTypeRefsForChangedTypes = changedTypes.keySet().stream().anyMatch(typeReferences::containsKey);
if (hasTypeRefsForChangedTypes) {
replaceTypeReferences(dummyRoot, codeRegistry, changedTypes);
replaceTypeReferences(dummyRoot, schema, codeRegistry, changedTypes);
}
}

Expand All @@ -170,7 +170,7 @@ private Object transformImpl(final GraphQLSchema schema, GraphQLSchemaElement sc
}
}

private void replaceTypeReferences(DummyRoot dummyRoot, GraphQLCodeRegistry.Builder codeRegistry, Map<String, GraphQLNamedType> changedTypes) {
private void replaceTypeReferences(DummyRoot dummyRoot, GraphQLSchema schema, GraphQLCodeRegistry.Builder codeRegistry, Map<String, GraphQLNamedType> changedTypes) {
GraphQLTypeVisitor typeRefVisitor = new GraphQLTypeVisitorStub() {
@Override
public TraversalControl visitGraphQLTypeReference(GraphQLTypeReference typeRef, TraverserContext<GraphQLSchemaElement> context) {
Expand All @@ -182,10 +182,10 @@ public TraversalControl visitGraphQLTypeReference(GraphQLTypeReference typeRef,
return CONTINUE;
}
};
traverseAndTransform(dummyRoot, new HashMap<>(), new HashMap<>(), typeRefVisitor, codeRegistry);
traverseAndTransform(dummyRoot, new HashMap<>(), new HashMap<>(), typeRefVisitor, codeRegistry, schema);
}

private boolean traverseAndTransform(DummyRoot dummyRoot, Map<String, GraphQLNamedType> changedTypes, Map<String, GraphQLTypeReference> typeReferences, GraphQLTypeVisitor visitor, GraphQLCodeRegistry.Builder codeRegistry) {
private boolean traverseAndTransform(DummyRoot dummyRoot, Map<String, GraphQLNamedType> changedTypes, Map<String, GraphQLTypeReference> typeReferences, GraphQLTypeVisitor visitor, GraphQLCodeRegistry.Builder codeRegistry, GraphQLSchema schema) {
List<NodeZipper<GraphQLSchemaElement>> zippers = new LinkedList<>();
Map<GraphQLSchemaElement, NodeZipper<GraphQLSchemaElement>> zipperByNodeAfterTraversing = new LinkedHashMap<>();
Map<GraphQLSchemaElement, NodeZipper<GraphQLSchemaElement>> zipperByOriginalNode = new LinkedHashMap<>();
Expand All @@ -195,7 +195,7 @@ private boolean traverseAndTransform(DummyRoot dummyRoot, Map<String, GraphQLNam
Map<GraphQLSchemaElement, List<GraphQLSchemaElement>> reverseDependencies = new LinkedHashMap<>();
Map<String, List<GraphQLSchemaElement>> typeRefReverseDependencies = new LinkedHashMap<>();

TraverserVisitor<GraphQLSchemaElement> nodeTraverserVisitor = new TraverserVisitor<GraphQLSchemaElement>() {
TraverserVisitor<GraphQLSchemaElement> nodeTraverserVisitor = new TraverserVisitor<>() {
@Override
public TraversalControl enter(TraverserContext<GraphQLSchemaElement> context) {
GraphQLSchemaElement currentSchemaElement = context.thisNode();
Expand Down Expand Up @@ -271,6 +271,9 @@ public TraversalControl backRef(TraverserContext<GraphQLSchemaElement> context)
if (codeRegistry != null) {
traverser.rootVar(GraphQLCodeRegistry.Builder.class, codeRegistry);
}
if (schema != null) {
traverser.rootVar(GraphQLSchema.class, schema);
}
Copy link
Copy Markdown
Member Author

@bbakerman bbakerman Mar 6, 2023

Choose a reason for hiding this comment

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

We now add the source schema as a root variable - which visitors can now access


traverser.traverse(dummyRoot, nodeTraverserVisitor);

Expand Down
9 changes: 7 additions & 2 deletions src/main/java/graphql/schema/SchemaTraverser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,11 @@ public TraverserResult depthFirstFullSchema(List<GraphQLTypeVisitor> typeVisitor
roots.addAll(schema.getAdditionalTypes());
roots.addAll(schema.getDirectives());
roots.addAll(schema.getSchemaDirectives());
roots.addAll(schema.getSchemaAppliedDirectives());
roots.add(schema.getIntrospectionSchemaType());
TraverserDelegateListVisitor traverserDelegateListVisitor = new TraverserDelegateListVisitor(typeVisitors);
return initTraverser().rootVars(rootVars).traverse(roots, traverserDelegateListVisitor);
Traverser<GraphQLSchemaElement> traverser = initTraverser().rootVars(rootVars).rootVar(GraphQLSchema.class, schema);
return traverser.traverse(roots, traverserDelegateListVisitor);
Copy link
Copy Markdown
Member Author

@bbakerman bbakerman Mar 6, 2023

Choose a reason for hiding this comment

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

Same here on SchemaTraverser - the schema being traversed is now a root variable

}

public TraverserResult depthFirst(GraphQLTypeVisitor graphQLTypeVisitor, GraphQLSchemaElement root) {
Expand Down Expand Up @@ -131,7 +133,10 @@ private static class TraverserDelegateListVisitor implements TraverserVisitor<Gr
@Override
public TraversalControl enter(TraverserContext<GraphQLSchemaElement> context) {
for (GraphQLTypeVisitor graphQLTypeVisitor : typeVisitors) {
context.thisNode().accept(context, graphQLTypeVisitor);
TraversalControl control = context.thisNode().accept(context, graphQLTypeVisitor);
if (control != CONTINUE) {
return control;
}
Copy link
Copy Markdown
Member Author

@bbakerman bbakerman Mar 6, 2023

Choose a reason for hiding this comment

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

This was broken. I fixed it and now tests broke

So I wrote a new test for this case - when NOT CONTINUE is returned

}
return CONTINUE;
}
Expand Down
81 changes: 81 additions & 0 deletions src/main/java/graphql/schema/visitor/GraphQLSchemaTraversalControl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
package graphql.schema.visitor;
Copy link
Copy Markdown
Member Author

@bbakerman bbakerman Mar 6, 2023

Choose a reason for hiding this comment

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

We have a different "control" that SchemaTraversalControl - so we can have better methods and "hide" the changeNode malarky we must do.

The only way to get a GraphQLSchemaTraversalControl (which is package level on creation) is by the helper methods and hence this simplifies the call API for consumers


import graphql.PublicApi;
import graphql.schema.GraphQLSchemaElement;
import graphql.util.TraversalControl;
import graphql.util.TraverserContext;
import graphql.util.TreeTransformerUtil;

/**
* This indicates what traversal control to apply during the visitation
* and can be created via calls to methods like {@link GraphQLSchemaVisitorEnvironment#ok()}
* or {@link GraphQLSchemaVisitorEnvironment#changeNode(GraphQLSchemaElement)} say
*/
@PublicApi
public class GraphQLSchemaTraversalControl {
private final GraphQLSchemaElement element;
private final Control control;

enum Control {
CONTINUE(TraversalControl.CONTINUE),
QUIT(TraversalControl.QUIT),
CHANGE(TraversalControl.CONTINUE),
DELETE(TraversalControl.CONTINUE),
INSERT_BEFORE(TraversalControl.CONTINUE),
INSERT_AFTER(TraversalControl.CONTINUE);

private final TraversalControl traversalControl;

Control(TraversalControl traversalControl) {
this.traversalControl = traversalControl;
}

public TraversalControl toTraversalControl() {
return traversalControl;
}
}

static final GraphQLSchemaTraversalControl CONTINUE = new GraphQLSchemaTraversalControl(Control.CONTINUE, null);
static final GraphQLSchemaTraversalControl QUIT = new GraphQLSchemaTraversalControl(Control.QUIT, null);
static final GraphQLSchemaTraversalControl DELETE = new GraphQLSchemaTraversalControl(Control.DELETE, null);

GraphQLSchemaTraversalControl(Control control, GraphQLSchemaElement element) {
this.element = element;
this.control = control;
}

GraphQLSchemaElement getElement() {
return element;
}

Control getControl() {
return control;
}

boolean isAbortive() {
return control == Control.QUIT;
}

boolean isMutative() {
return control == Control.DELETE || control == Control.CHANGE || control == Control.INSERT_AFTER || control == Control.INSERT_BEFORE;
}

TraversalControl toTraversalControl(TraverserContext<GraphQLSchemaElement> context) {
if (control == Control.CONTINUE || control == Control.QUIT) {
return control.toTraversalControl();
}
if (control == Control.DELETE) {
TreeTransformerUtil.deleteNode(context);
}
if (control == Control.CHANGE) {
TreeTransformerUtil.changeNode(context, element);
}
if (control == Control.INSERT_AFTER) {
TreeTransformerUtil.insertAfter(context, element);
}
if (control == Control.INSERT_BEFORE) {
TreeTransformerUtil.insertAfter(context, element);
}
return TraversalControl.CONTINUE;
}
}
Copy link
Copy Markdown
Member Author

@bbakerman bbakerman Mar 6, 2023

Choose a reason for hiding this comment

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

See how this hodes the TreeTransformerUtil malarky AND also we can detect intention from the GraphQLSchemaTraversalControl as a value and not a side effect

Loading