Skip to content

Commit 2944646

Browse files
andimarekandimarek
committed
use two callbacks instead boolean proOrder flag, fix skipping logic
and added tests for that
1 parent aa63eea commit 2944646

File tree

2 files changed

+134
-13
lines changed

2 files changed

+134
-13
lines changed

‎src/main/java/graphql/analysis/QueryTraversal.java‎

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -106,18 +106,23 @@ private void visitImpl(FieldVisitor visitFieldCallback, SelectionSet selectionSe
106106
Map<Class<?>, Object> rootVars = new LinkedHashMap<>();
107107
rootVars.put(QueryTraversalContext.class, new QueryTraversalContext(type, null));
108108

109+
FieldVisitor noOp = notUsed -> {
110+
};
111+
FieldVisitor preOrderCallback = preOrder ? visitFieldCallback : noOp;
112+
FieldVisitor postOrderCallback = !preOrder ? visitFieldCallback : noOp;
113+
109114
NodeTraverser nodeTraverser = new NodeTraverser(rootVars, this::childrenOf);
110-
nodeTraverser.depthFirst(new NodeVisitorImpl(visitFieldCallback, preOrder), selectionSet.getSelections());
115+
nodeTraverser.depthFirst(new NodeVisitorImpl(preOrderCallback, postOrderCallback), selectionSet.getSelections());
111116
}
112117

113118
private class NodeVisitorImpl extends NodeVisitorStub {
114119

115-
private FieldVisitor visitFieldCallback;
116-
boolean preOrder;
120+
final FieldVisitor preOrderCallback;
121+
final FieldVisitor postOrderCallback;
117122

118-
NodeVisitorImpl(FieldVisitor visitFieldCallback, boolean preOrder) {
119-
this.visitFieldCallback = visitFieldCallback;
120-
this.preOrder = preOrder;
123+
NodeVisitorImpl(FieldVisitor preOrderCallback, FieldVisitor postOrderCallback) {
124+
this.preOrderCallback = preOrderCallback;
125+
this.postOrderCallback = postOrderCallback;
121126
}
122127

123128
@Override
@@ -180,17 +185,14 @@ public TraversalControl visitField(Field field, TraverserContext<Node> context)
180185

181186
LeaveOrEnter leaveOrEnter = context.getVar(LeaveOrEnter.class);
182187
if (leaveOrEnter == LEAVE) {
183-
if (!preOrder) {
184-
visitFieldCallback.visitField(environment);
185-
}
188+
postOrderCallback.visitField(environment);
186189
return TraversalControl.CONTINUE;
187190
}
188-
if (preOrder) {
189-
visitFieldCallback.visitField(environment);
190-
}
191191

192192
if (!conditionalNodes.shouldInclude(variables, field.getDirectives()))
193-
return TraversalControl.ABORT; // stop recursion
193+
return TraversalControl.ABORT;
194+
195+
preOrderCallback.visitField(environment);
194196

195197
GraphQLUnmodifiedType unmodifiedType = schemaUtil.getUnmodifiedType(fieldDefinition.getType());
196198
QueryTraversalContext fieldEnv = (unmodifiedType instanceof GraphQLCompositeType)

‎src/test/groovy/graphql/analysis/QueryTraversalTest.groovy‎

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -539,6 +539,125 @@ class QueryTraversalTest extends Specification {
539539
540540
}
541541
542+
@Unroll
543+
def "skipped Fragment (#order)"() {
544+
given:
545+
def schema = TestUtil.schema("""
546+
type Query{
547+
foo: Foo1
548+
bar: String
549+
}
550+
type Foo1 {
551+
string: String
552+
subFoo: Foo2
553+
}
554+
type Foo2 {
555+
otherString: String
556+
}
557+
""")
558+
def visitor = Mock(FieldVisitor)
559+
def query = createQuery("""
560+
query MyQuery(\$variableFoo: Boolean) {
561+
bar
562+
...Test @include(if: \$variableFoo)
563+
}
564+
fragment Test on Query {
565+
bar
566+
}
567+
""")
568+
QueryTraversal queryTraversal = createQueryTraversal(query, schema, [variableFoo: false])
569+
when:
570+
queryTraversal."$visitFn"(visitor)
571+
572+
then:
573+
1 * visitor.visitField({ QueryVisitorEnvironment it -> it.field.name == "bar" && it.fieldDefinition.type.name == "String" && it.parentType.name == "Query" })
574+
0 * visitor.visitField(_)
575+
576+
where:
577+
order | visitFn
578+
'postOrder' | 'visitPostOrder'
579+
'preOrder' | 'visitPreOrder'
580+
581+
}
582+
583+
@Unroll
584+
def "skipped inline Fragment (#order)"() {
585+
given:
586+
def schema = TestUtil.schema("""
587+
type Query{
588+
foo: Foo1
589+
bar: String
590+
}
591+
type Foo1 {
592+
string: String
593+
subFoo: Foo2
594+
}
595+
type Foo2 {
596+
otherString: String
597+
}
598+
""")
599+
def visitor = Mock(FieldVisitor)
600+
def query = createQuery("""
601+
query MyQuery(\$variableFoo: Boolean) {
602+
bar
603+
...@include(if: \$variableFoo) {
604+
foo
605+
}
606+
}
607+
""")
608+
QueryTraversal queryTraversal = createQueryTraversal(query, schema, [variableFoo: false])
609+
when:
610+
queryTraversal."$visitFn"(visitor)
611+
612+
then:
613+
1 * visitor.visitField({ QueryVisitorEnvironment it -> it.field.name == "bar" && it.fieldDefinition.type.name == "String" && it.parentType.name == "Query" })
614+
0 * visitor.visitField(_)
615+
616+
where:
617+
order | visitFn
618+
'postOrder' | 'visitPostOrder'
619+
'preOrder' | 'visitPreOrder'
620+
621+
}
622+
623+
@Unroll
624+
def "skipped Field (#order)"() {
625+
given:
626+
def schema = TestUtil.schema("""
627+
type Query{
628+
foo: Foo1
629+
bar: String
630+
}
631+
type Foo1 {
632+
string: String
633+
subFoo: Foo2
634+
}
635+
type Foo2 {
636+
otherString: String
637+
}
638+
""")
639+
def visitor = Mock(FieldVisitor)
640+
def query = createQuery("""
641+
query MyQuery(\$variableFoo: Boolean) {
642+
bar
643+
foo @include(if: \$variableFoo)
644+
}
645+
""")
646+
QueryTraversal queryTraversal = createQueryTraversal(query, schema, [variableFoo: false])
647+
when:
648+
queryTraversal."$visitFn"(visitor)
649+
650+
then:
651+
1 * visitor.visitField({ QueryVisitorEnvironment it -> it.field.name == "bar" && it.fieldDefinition.type.name == "String" && it.parentType.name == "Query" })
652+
0 * visitor.visitField(_)
653+
654+
where:
655+
order | visitFn
656+
'postOrder' | 'visitPostOrder'
657+
'preOrder' | 'visitPreOrder'
658+
659+
}
660+
542661
543662
def "reduce preOrder"() {
544663
given:

0 commit comments

Comments
 (0)