Skip to content

Commit fe82d4f

Browse files
andimarekandimarek
committed
simplify
1 parent 44aeed6 commit fe82d4f

File tree

1 file changed

+49
-44
lines changed

1 file changed

+49
-44
lines changed

‎src/main/java/graphql/schema/impl/GraphQLTypeCollectingVisitor.java‎

Lines changed: 49 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,23 @@
3030
import static graphql.util.TraversalControl.CONTINUE;
3131
import static java.lang.String.format;
3232

33+
/**
34+
* A visitor that collects all {@link GraphQLNamedType}s during schema traversal.
35+
* <p>
36+
* This visitor must be used with a traverser that calls {@code getChildrenWithTypeReferences()}
37+
* to get children (see {@link SchemaUtil#visitPartiallySchema}). This means that when a field,
38+
* argument, or input field references a type via {@link GraphQLTypeReference}, the traverser
39+
* will see the type reference as a child, not the actual type it points to. Type references
40+
* themselves are not collected - only concrete type instances are stored in the result map.
41+
* <p>
42+
* Because type references are not followed, this visitor also tracks "indirect strong references"
43+
* - types that are directly referenced (not via type reference) by fields, arguments, and input
44+
* fields. This handles edge cases where schema transformations replace type references with
45+
* actual types, which would otherwise be missed during traversal.
46+
*
47+
* @see SchemaUtil#visitPartiallySchema
48+
* @see #fixDanglingReplacedTypes
49+
*/
3350
@Internal
3451
public class GraphQLTypeCollectingVisitor extends GraphQLTypeVisitorStub {
3552

@@ -55,31 +72,22 @@ public TraversalControl visitGraphQLScalarType(GraphQLScalarType node, Traverser
5572

5673
@Override
5774
public TraversalControl visitGraphQLObjectType(GraphQLObjectType node, TraverserContext<GraphQLSchemaElement> context) {
58-
if (isNotTypeReference(node.getName())) {
59-
assertTypeUniqueness(node, result);
60-
} else {
61-
save(node.getName(), node);
62-
}
75+
assertTypeUniqueness(node, result);
76+
save(node.getName(), node);
6377
return CONTINUE;
6478
}
6579

6680
@Override
6781
public TraversalControl visitGraphQLInputObjectType(GraphQLInputObjectType node, TraverserContext<GraphQLSchemaElement> context) {
68-
if (isNotTypeReference(node.getName())) {
69-
assertTypeUniqueness(node, result);
70-
} else {
71-
save(node.getName(), node);
72-
}
82+
assertTypeUniqueness(node, result);
83+
save(node.getName(), node);
7384
return CONTINUE;
7485
}
7586

7687
@Override
7788
public TraversalControl visitGraphQLInterfaceType(GraphQLInterfaceType node, TraverserContext<GraphQLSchemaElement> context) {
78-
if (isNotTypeReference(node.getName())) {
79-
assertTypeUniqueness(node, result);
80-
} else {
81-
save(node.getName(), node);
82-
}
89+
assertTypeUniqueness(node, result);
90+
save(node.getName(), node);
8391
return CONTINUE;
8492
}
8593

@@ -114,43 +122,25 @@ public TraversalControl visitGraphQLAppliedDirectiveArgument(GraphQLAppliedDirec
114122
return CONTINUE;
115123
}
116124

117-
private <T> void saveIndirectStrongReference(Supplier<GraphQLType> typeSupplier) {
125+
private void saveIndirectStrongReference(Supplier<GraphQLType> typeSupplier) {
118126
GraphQLNamedType type = unwrapAllAs(typeSupplier.get());
119127
if (!(type instanceof GraphQLTypeReference)) {
120128
indirectStrongReferences.put(type.getName(), type);
121129
}
122130
}
123131

124-
125-
private boolean isNotTypeReference(String name) {
126-
return result.containsKey(name) && !(result.get(name) instanceof GraphQLTypeReference);
127-
}
128-
129132
private void save(String name, GraphQLNamedType type) {
130133
result.put(name, type);
131134
}
132135

133-
134-
/*
135-
From https://spec.graphql.org/October2021/#sec-Type-System
136-
137-
All types within a GraphQL schema must have unique names. No two provided types may have the same name.
138-
No provided type may have a name which conflicts with any built in types (including Scalar and Introspection types).
139-
140-
Enforcing this helps avoid problems later down the track fo example https://github.com/graphql-java/graphql-java/issues/373
141-
*/
142136
private void assertTypeUniqueness(GraphQLNamedType type, Map<String, GraphQLNamedType> result) {
143-
GraphQLType existingType = result.get(type.getName());
144-
// do we have an existing definition
137+
GraphQLNamedType existingType = result.get(type.getName());
145138
if (existingType != null) {
146-
// type references are ok
147-
if (!(existingType instanceof GraphQLTypeReference || type instanceof GraphQLTypeReference)) {
148-
assertUniqueTypeObjects(type, existingType);
149-
}
139+
assertUniqueTypeObjects(type, existingType);
150140
}
151141
}
152142

153-
private void assertUniqueTypeObjects(GraphQLNamedType type, GraphQLType existingType) {
143+
private void assertUniqueTypeObjects(GraphQLNamedType type, GraphQLNamedType existingType) {
154144
// object comparison here is deliberate
155145
if (existingType != type) {
156146
throw new AssertException(format("All types within a GraphQL schema must have unique names. No two provided types may have the same name.\n" +
@@ -166,15 +156,30 @@ public ImmutableMap<String, GraphQLNamedType> getResult() {
166156
}
167157

168158
/**
169-
* It's possible for certain schema edits to create a situation where a field / arg / input field had a type reference, then
170-
* it got replaced with an actual strong reference and then the schema gets edited such that the only reference
171-
* to that type is the replaced strong reference. This edge case means that the replaced reference can be
172-
* missed if it's the only way to get to that type because this visitor asks for the children as original type,
173-
* e.g. the type reference and not the replaced reference.
159+
* Fixes an edge case where types might be missed during traversal due to replaced type references.
160+
* <p>
161+
* The problem: During schema construction or transformation, a field's type might initially
162+
* be a {@link GraphQLTypeReference} (a placeholder). Later, this reference gets replaced with the
163+
* actual type instance. However, the schema traverser uses {@code getChildrenWithTypeReferences()}
164+
* to discover children, which returns the original type references, not the replaced strong references.
165+
* <p>
166+
* The scenario:
167+
* <ol>
168+
* <li>Field {@code foo} has type reference to {@code Bar}</li>
169+
* <li>During schema editing, the reference is replaced with the actual {@code Bar} type</li>
170+
* <li>Further edits remove all other paths to {@code Bar}</li>
171+
* <li>Now the only way to reach {@code Bar} is via the replaced reference in {@code foo}</li>
172+
* <li>But the traverser still sees the original type reference as the child, so {@code Bar}
173+
* is never visited as a child node</li>
174+
* </ol>
175+
* <p>
176+
* The fix: During traversal, we also capture types directly from fields/arguments/inputs
177+
* (in {@link #indirectStrongReferences}). After traversal, we merge any types that were captured
178+
* this way but weren't found through normal traversal.
174179
*
175-
* @param visitedTypes the types collected by this visitor
180+
* @param visitedTypes the types collected through normal traversal
176181
*
177-
* @return a fixed up map where the only
182+
* @return the fixed map including any dangling replaced types
178183
*/
179184
private Map<String, GraphQLNamedType> fixDanglingReplacedTypes(Map<String, GraphQLNamedType> visitedTypes) {
180185
for (GraphQLNamedType indirectStrongReference : indirectStrongReferences.values()) {

0 commit comments

Comments
 (0)