Skip to content

Commit 209e72e

Browse files
kmoore-intuitkmoore-intuit
committed
revert changes for the variables filter
1 parent f386387 commit 209e72e

File tree

3 files changed

+19
-130
lines changed

3 files changed

+19
-130
lines changed

‎src/main/java/com/intuit/graphql/orchestrator/batch/VariableDefinitionFilter.java‎

Lines changed: 18 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -6,25 +6,20 @@
66
import graphql.analysis.QueryVisitorInlineFragmentEnvironment;
77
import graphql.analysis.QueryVisitorStub;
88
import graphql.language.Argument;
9-
import graphql.language.AstTransformer;
109
import graphql.language.Document;
1110
import graphql.language.Field;
1211
import graphql.language.FragmentDefinition;
1312
import graphql.language.FragmentSpread;
1413
import graphql.language.InlineFragment;
1514
import graphql.language.Node;
16-
import graphql.language.NodeVisitorStub;
1715
import graphql.language.OperationDefinition;
1816
import graphql.language.Value;
1917
import graphql.language.VariableReference;
2018
import graphql.schema.GraphQLObjectType;
2119
import graphql.schema.GraphQLSchema;
22-
import graphql.util.TraversalControl;
23-
import graphql.util.TraverserContext;
2420
import lombok.Getter;
2521

2622
import java.util.ArrayList;
27-
import java.util.Collection;
2823
import java.util.List;
2924
import java.util.Map;
3025
import java.util.Set;
@@ -36,8 +31,6 @@
3631
*/
3732
public class VariableDefinitionFilter {
3833

39-
private static AstTransformer astTransformer = new AstTransformer();
40-
4134
/**
4235
* Traverses a GraphQL Node and returns all VariableReference names used in all nodes in the graph.
4336
*
@@ -50,17 +43,17 @@ public class VariableDefinitionFilter {
5043
* reference indicator prefix '$' will be <b>excluded</b> in the result.
5144
*/
5245
public Set<String> getVariableReferencesFromNode(GraphQLSchema graphQLSchema, GraphQLObjectType rootType,
53-
Map<String, FragmentDefinition> fragmentsByName, Map<String, Object> variables, Node<?> rootNode) {
46+
Map<String, FragmentDefinition> fragmentsByName, Map<String, Object> variables, Node<?> rootNode) {
5447
final VariableReferenceVisitor variableReferenceVisitor = new VariableReferenceVisitor();
5548

5649
//need to utilize a better pattern for creating mockable QueryTraverser/QueryTransformer
5750
QueryTraverser queryTraverser = QueryTraverser.newQueryTraverser()
58-
.schema(graphQLSchema)
59-
.rootParentType(rootType) //need to support also for subscription
60-
.fragmentsByName(fragmentsByName)
61-
.variables(variables)
62-
.root(rootNode)
63-
.build();
51+
.schema(graphQLSchema)
52+
.rootParentType(rootType) //need to support also for subscription
53+
.fragmentsByName(fragmentsByName)
54+
.variables(variables)
55+
.root(rootNode)
56+
.build();
6457

6558
queryTraverser.visitPreOrder(variableReferenceVisitor);
6659

@@ -75,28 +68,16 @@ public Set<String> getVariableReferencesFromNode(GraphQLSchema graphQLSchema, Gr
7568

7669
Set<VariableReference> additionalReferences = operationDirectiveVariableReferences(operationDefinitions);
7770

78-
Stream<VariableReference> variableReferenceStream;
79-
if((variableReferenceVisitor.getVariableReferences().size() + additionalReferences.size()) != variables.size()) {
80-
NodeTraverser nodeTraverser = new NodeTraverser();
81-
astTransformer.transform(rootNode, nodeTraverser);
82-
83-
variableReferenceStream = Stream.of(variableReferenceVisitor.getVariableReferences(),
84-
additionalReferences,
85-
nodeTraverser.getVariableReferenceExtractor().getVariableReferences())
86-
.flatMap(Collection::stream);
87-
} else {
88-
variableReferenceStream = Stream.concat(variableReferenceVisitor.getVariableReferences().stream(), additionalReferences.stream());
89-
}
90-
return variableReferenceStream.map(VariableReference::getName).collect(Collectors.toSet());
91-
71+
return Stream.concat(variableReferenceVisitor.getVariableReferences().stream(), additionalReferences.stream())
72+
.map(VariableReference::getName).collect(Collectors.toSet());
9273
}
9374

9475
private Set<VariableReference> operationDirectiveVariableReferences(List<OperationDefinition> operationDefinitions) {
9576
final List<Value> values = operationDefinitions.stream()
96-
.flatMap(operationDefinition -> operationDefinition.getDirectives().stream())
97-
.flatMap(directive -> directive.getArguments().stream())
98-
.map(Argument::getValue)
99-
.collect(Collectors.toList());
77+
.flatMap(operationDefinition -> operationDefinition.getDirectives().stream())
78+
.flatMap(directive -> directive.getArguments().stream())
79+
.map(Argument::getValue)
80+
.collect(Collectors.toList());
10081

10182
VariableReferenceExtractor extractor = new VariableReferenceExtractor();
10283
extractor.captureVariableReferences(values);
@@ -138,7 +119,7 @@ public void visitField(final QueryVisitorFieldEnvironment env) {
138119
}
139120

140121
final Stream<Argument> directiveArgumentStream = field.getDirectives().stream()
141-
.flatMap(directive -> directive.getArguments().stream());
122+
.flatMap(directive -> directive.getArguments().stream());
142123

143124
final Stream<Argument> fieldArgumentStream = field.getArguments().stream();
144125

@@ -154,7 +135,7 @@ public void visitInlineFragment(final QueryVisitorInlineFragmentEnvironment env)
154135
}
155136

156137
Stream<Argument> arguments = env.getInlineFragment().getDirectives().stream()
157-
.flatMap(directive -> directive.getArguments().stream());
138+
.flatMap(directive -> directive.getArguments().stream());
158139

159140
captureVariableReferences(arguments);
160141
}
@@ -169,33 +150,18 @@ public void visitFragmentSpread(final QueryVisitorFragmentSpreadEnvironment env)
169150
}
170151

171152
final Stream<Argument> allArguments = Stream.concat(
172-
fragmentDefinition.getDirectives().stream(),
173-
fragmentSpread.getDirectives().stream()
153+
fragmentDefinition.getDirectives().stream(),
154+
fragmentSpread.getDirectives().stream()
174155
).flatMap(directive -> directive.getArguments().stream());
175156

176157
captureVariableReferences(allArguments);
177158
}
178159

179160
private void captureVariableReferences(Stream<Argument> arguments) {
180161
final List<Value> values = arguments.map(Argument::getValue)
181-
.collect(Collectors.toList());
162+
.collect(Collectors.toList());
182163

183164
variableReferenceExtractor.captureVariableReferences(values);
184165
}
185166
}
186-
187-
static class NodeTraverser extends NodeVisitorStub {
188-
189-
@Getter
190-
private final VariableReferenceExtractor variableReferenceExtractor = new VariableReferenceExtractor();
191-
192-
public TraversalControl visitArgument(Argument node, TraverserContext<Node> context) {
193-
return this.visitNode(node, context);
194-
}
195-
196-
public TraversalControl visitVariableReference(VariableReference node, TraverserContext<Node> context) {
197-
variableReferenceExtractor.captureVariableReference(node);
198-
return this.visitValue(node, context);
199-
}
200-
}
201167
}

‎src/main/java/com/intuit/graphql/orchestrator/batch/VariableReferenceExtractor.java‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,10 @@ public Set<VariableReference> getVariableReferences() {
1919

2020
public void captureVariableReferences(List<Value> values) {
2121
for (final Value value : values) {
22-
captureVariableReference(value);
22+
doSwitch(value);
2323
}
2424
}
2525

26-
public void captureVariableReference(Value value) {
27-
doSwitch(value);
28-
}
29-
3026
private void doSwitch(Value value) {
3127
if (value instanceof ArrayValue) {
3228
handleArrayValue((ArrayValue) value);

‎src/test/groovy/com/intuit/graphql/orchestrator/batch/VariableDefinitionFilterSpec.groovy‎

Lines changed: 0 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -47,17 +47,6 @@ class VariableDefinitionFilterSpec extends Specification {
4747
directive @field_directive_argument(arg: InputObject) on FIELD_DEFINITION
4848
'''
4949

50-
private String schema2 = '''
51-
type Query { person: Person }
52-
53-
type Person {
54-
address : Address
55-
id: String
56-
}
57-
58-
type Address { city: String state: String zip: String }
59-
'''
60-
6150
private GraphQLSchema graphQLSchema
6251

6352
private VariableDefinitionFilter variableDefinitionFilter
@@ -74,12 +63,6 @@ class VariableDefinitionFilterSpec extends Specification {
7463
RuntimeWiring.newRuntimeWiring().build())
7564
}
7665

77-
private GraphQLSchema getSchema2() {
78-
return new SchemaGenerator()
79-
.makeExecutableSchema(new SchemaParser().parse(schema2),
80-
RuntimeWiring.newRuntimeWiring().build())
81-
}
82-
8366
private Map<String, FragmentDefinition> getFragmentsByName(Document document) {
8467
return document.getDefinitionsOfType(FragmentDefinition.class).stream()
8568
.inject([:]) {map, it -> map << [(it.getName()): it]}
@@ -196,62 +179,6 @@ class VariableDefinitionFilterSpec extends Specification {
196179
results.containsAll("int_arg", "string_arg")
197180
}
198181

199-
def "variable References In Built in Query Directive includes"() {
200-
given:
201-
String query = '''
202-
query($includeContext: Boolean!) {
203-
consumer {
204-
liabilities(arg: 1) @include(if: $includeContext) {
205-
totalDebt(arg: 1)
206-
}
207-
income
208-
}
209-
}
210-
'''
211-
212-
Document document = parser.parseDocument(query)
213-
HashMap<String, Object> variables = new HashMap<>()
214-
variables.put("includeContext", false)
215-
216-
when:
217-
final Set<String> results = variableDefinitionFilter
218-
.getVariableReferencesFromNode(graphQLSchema, graphQLSchema.getQueryType(), Collections.emptyMap(),
219-
variables, document)
220-
221-
then:
222-
results.size() == 1
223-
224-
results.containsAll("includeContext")
225-
}
226-
227-
def "variable References In Built in Query Directive skip"() {
228-
given:
229-
String query = '''
230-
query($includeContext: Boolean!) {
231-
consumer {
232-
liabilities(arg: 1) @skip(if: $includeContext) {
233-
totalDebt(arg: 1)
234-
}
235-
income
236-
}
237-
}
238-
'''
239-
240-
Document document = parser.parseDocument(query)
241-
HashMap<String, Object> variables = new HashMap<>()
242-
variables.put("includeContext", true)
243-
244-
when:
245-
final Set<String> results = variableDefinitionFilter
246-
.getVariableReferencesFromNode(graphQLSchema, graphQLSchema.getQueryType(), Collections.emptyMap(),
247-
variables, document)
248-
249-
then:
250-
results.size() == 1
251-
252-
results.containsAll("includeContext")
253-
}
254-
255182
def "test Negative Cases"() {
256183
given:
257184
final String negativeTestCaseQuery = "query { consumer { liabilities { totalDebt(arg: 1234) } } }"

0 commit comments

Comments
 (0)