Skip to content

Commit e860aff

Browse files
andimarekandimarek
authored
Merge pull request #4212 from graphql-java/field-visibility-fix
fixes and improves field visibility
2 parents 1a80205 + bfc2a7c commit e860aff

2 files changed

Lines changed: 690 additions & 118 deletions

File tree

‎src/main/java/graphql/schema/transform/FieldVisibilitySchemaTransformation.java‎

Lines changed: 87 additions & 109 deletions
Original file line numberDiff line numberDiff line change
@@ -2,38 +2,34 @@
22

33
import com.google.common.collect.ImmutableList;
44
import graphql.PublicApi;
5+
import graphql.introspection.Introspection;
56
import graphql.schema.GraphQLEnumType;
67
import graphql.schema.GraphQLFieldDefinition;
78
import graphql.schema.GraphQLFieldsContainer;
8-
import graphql.schema.GraphQLImplementingType;
99
import graphql.schema.GraphQLInputObjectField;
1010
import graphql.schema.GraphQLInputObjectType;
1111
import graphql.schema.GraphQLInterfaceType;
1212
import graphql.schema.GraphQLNamedType;
1313
import graphql.schema.GraphQLObjectType;
14+
import graphql.schema.GraphQLScalarType;
1415
import graphql.schema.GraphQLSchema;
1516
import graphql.schema.GraphQLSchemaElement;
16-
import graphql.schema.GraphQLType;
1717
import graphql.schema.GraphQLTypeVisitorStub;
1818
import graphql.schema.GraphQLUnionType;
1919
import graphql.schema.SchemaTraverser;
20-
import graphql.schema.impl.SchemaUtil;
2120
import graphql.schema.transform.VisibleFieldPredicateEnvironment.VisibleFieldPredicateEnvironmentImpl;
2221
import graphql.util.TraversalControl;
2322
import graphql.util.TraverserContext;
2423

2524
import java.util.ArrayList;
26-
import java.util.HashSet;
2725
import java.util.LinkedHashSet;
2826
import java.util.List;
29-
import java.util.Map;
3027
import java.util.Objects;
3128
import java.util.Set;
3229
import java.util.function.Function;
3330
import java.util.stream.Collectors;
3431
import java.util.stream.Stream;
3532

36-
import static graphql.schema.SchemaTransformer.transformSchema;
3733
import static graphql.schema.SchemaTransformer.transformSchemaWithDeletes;
3834

3935
/**
@@ -61,84 +57,103 @@ public FieldVisibilitySchemaTransformation(VisibleFieldPredicate visibleFieldPre
6157
}
6258

6359
public final GraphQLSchema apply(GraphQLSchema schema) {
64-
Set<String> observedBeforeTransform = new LinkedHashSet<>();
65-
Set<String> observedAfterTransform = new LinkedHashSet<>();
66-
Set<GraphQLType> markedForRemovalTypes = new HashSet<>();
67-
68-
// query, mutation, and subscription types should not be removed
69-
final Set<String> protectedTypeNames = new HashSet<>();
70-
for (GraphQLObjectType graphQLObjectType : getOperationTypes(schema)) {
71-
protectedTypeNames.add(graphQLObjectType.getName());
72-
}
7360

7461
beforeTransformationHook.run();
7562

76-
new SchemaTraverser(getChildrenFn(schema)).depthFirst(new TypeObservingVisitor(observedBeforeTransform), getRootTypes(schema));
63+
// Find root unused types BEFORE transformation
64+
// These are types that exist in the schema but are NOT reachable from operation types + directives
65+
Set<String> rootUnusedTypes = findRootUnusedTypes(schema);
7766

78-
// remove fields
67+
// we delete all fields that should be deleted
68+
// this assumes the field remove itself is semantically valid
7969
GraphQLSchema interimSchema = transformSchemaWithDeletes(schema,
80-
new FieldRemovalVisitor(visibleFieldPredicate, markedForRemovalTypes));
70+
new FieldRemovalVisitor(visibleFieldPredicate));
71+
72+
73+
// cleanup schema
74+
// now we want to remove all types which are not reachable via root types, directives and the interface implements relationship
75+
SchemaTraverser schemaTraverser = new SchemaTraverser(childrenWithInterfaceImplementations(interimSchema));
76+
77+
// first we observe all types we don't want to delete
78+
Set<String> observedTypes = new LinkedHashSet<>();
79+
TypeObservingVisitor typeObservingVisitor = new TypeObservingVisitor(observedTypes);
80+
schemaTraverser.depthFirst(typeObservingVisitor, getRootTypes(interimSchema));
81+
82+
// Traverse from root unused types that still exist after transformation
83+
// This preserves originally unused types and their dependencies
84+
List<GraphQLSchemaElement> existingRootUnusedTypes = rootUnusedTypes.stream()
85+
.map(interimSchema::getType)
86+
.filter(Objects::nonNull)
87+
.map(type -> (GraphQLSchemaElement) type)
88+
.collect(Collectors.toList());
8189

82-
new SchemaTraverser(getChildrenFn(interimSchema)).depthFirst(new TypeObservingVisitor(observedAfterTransform), getRootTypes(interimSchema));
90+
if (!existingRootUnusedTypes.isEmpty()) {
91+
schemaTraverser.depthFirst(typeObservingVisitor, existingRootUnusedTypes);
92+
}
8393

84-
// remove types that are not used after removing fields - (connected schema only)
85-
GraphQLSchema connectedSchema = transformSchema(interimSchema,
86-
new TypeVisibilityVisitor(protectedTypeNames, observedBeforeTransform, observedAfterTransform));
94+
// then we delete all the types which are not used anymore
95+
GraphQLSchema finalSchema = transformSchemaWithDeletes(interimSchema,
96+
new TypeRemovalVisitor(observedTypes));
8797

88-
// ensure markedForRemovalTypes are not referenced by other schema elements, and delete from the schema
89-
// the ones that aren't.
90-
GraphQLSchema finalSchema = removeUnreferencedTypes(markedForRemovalTypes, connectedSchema);
9198

9299
afterTransformationHook.run();
93100

94101
return finalSchema;
95102
}
96103

97-
// Creates a getChildrenFn that includes interface
98-
private Function<GraphQLSchemaElement, List<GraphQLSchemaElement>> getChildrenFn(GraphQLSchema schema) {
99-
Map<String, List<GraphQLImplementingType>> interfaceImplementations = new SchemaUtil().groupImplementationsForInterfacesAndObjects(schema);
100-
101-
return graphQLSchemaElement -> {
102-
if (!(graphQLSchemaElement instanceof GraphQLInterfaceType)) {
103-
return graphQLSchemaElement.getChildren();
104-
}
105-
ArrayList<GraphQLSchemaElement> children = new ArrayList<>(graphQLSchemaElement.getChildren());
106-
List<GraphQLImplementingType> implementations = interfaceImplementations.get(((GraphQLInterfaceType) graphQLSchemaElement).getName());
107-
if (implementations != null) {
108-
children.addAll(implementations);
104+
/**
105+
* Finds root unused types - types that exist in additional types but are NOT reachable
106+
* from operation types (Query, Mutation, Subscription) and directives.
107+
*/
108+
private Set<String> findRootUnusedTypes(GraphQLSchema schema) {
109+
// Collect all types reachable from operation roots + directives
110+
// Use a traverser that includes interface implementations
111+
Set<String> typesReachableFromRoots = new LinkedHashSet<>();
112+
SchemaTraverser traverser = new SchemaTraverser(childrenWithInterfaceImplementations(schema));
113+
TypeObservingVisitor visitor = new TypeObservingVisitor(typesReachableFromRoots);
114+
traverser.depthFirst(visitor, getRootTypes(schema));
115+
116+
// Root unused types are additional types that are NOT reachable from roots
117+
Set<String> rootUnusedTypes = new LinkedHashSet<>();
118+
for (GraphQLNamedType type : schema.getAdditionalTypes()) {
119+
String typeName = type.getName();
120+
if (!typesReachableFromRoots.contains(typeName) && !isIntrospectionType(typeName)) {
121+
rootUnusedTypes.add(typeName);
109122
}
110-
return children;
111-
};
123+
}
124+
return rootUnusedTypes;
112125
}
113126

114-
private GraphQLSchema removeUnreferencedTypes(Set<GraphQLType> markedForRemovalTypes, GraphQLSchema connectedSchema) {
115-
GraphQLSchema withoutAdditionalTypes = connectedSchema.transform(builder -> {
116-
Set<GraphQLNamedType> additionalTypes = new HashSet<>(connectedSchema.getAdditionalTypes());
117-
additionalTypes.removeAll(markedForRemovalTypes);
118-
builder.clearAdditionalTypes();
119-
builder.additionalTypes(additionalTypes);
120-
});
127+
/**
128+
* Checks if a type is an introspection type that should be protected from removal.
129+
* This includes standard introspection types (starting with "__") and special types
130+
* like _AppliedDirective (starting with "_") added by IntrospectionWithDirectivesSupport.
131+
*/
132+
private static boolean isIntrospectionType(String typeName) {
133+
return Introspection.isIntrospectionTypes(typeName) || typeName.startsWith("_");
134+
}
121135

122-
// remove from markedForRemovalTypes any type that might still be referenced by other schema elements
123-
transformSchema(withoutAdditionalTypes, new AdditionalTypeVisibilityVisitor(markedForRemovalTypes));
136+
/**
137+
* Creates a function that returns children of a schema element, including interface implementations.
138+
* This ensures that when traversing from an interface, we also visit all types that implement it.
139+
*/
140+
private Function<GraphQLSchemaElement, List<GraphQLSchemaElement>> childrenWithInterfaceImplementations(GraphQLSchema schema) {
124141

125-
// finally remove the types on the schema we are certain aren't referenced by any other node.
126-
return transformSchema(connectedSchema, new GraphQLTypeVisitorStub() {
127-
@Override
128-
protected TraversalControl visitGraphQLType(GraphQLSchemaElement node, TraverserContext<GraphQLSchemaElement> context) {
129-
if (node instanceof GraphQLType && markedForRemovalTypes.contains(node)) {
130-
return deleteNode(context);
131-
}
132-
return super.visitGraphQLType(node, context);
142+
return schemaElement -> {
143+
if (!(schemaElement instanceof GraphQLInterfaceType)) {
144+
return schemaElement.getChildren();
133145
}
134-
});
146+
ArrayList<GraphQLSchemaElement> children = new ArrayList<>(schemaElement.getChildren());
147+
List<GraphQLObjectType> implementations = schema.getImplementations((GraphQLInterfaceType) schemaElement);
148+
children.addAll(implementations);
149+
return children;
150+
};
135151
}
136152

137153
private static class TypeObservingVisitor extends GraphQLTypeVisitorStub {
138154

139155
private final Set<String> observedTypes;
140156

141-
142157
private TypeObservingVisitor(Set<String> observedTypes) {
143158
this.observedTypes = observedTypes;
144159
}
@@ -150,7 +165,8 @@ protected TraversalControl visitGraphQLType(GraphQLSchemaElement node,
150165
node instanceof GraphQLEnumType ||
151166
node instanceof GraphQLInputObjectType ||
152167
node instanceof GraphQLInterfaceType ||
153-
node instanceof GraphQLUnionType) {
168+
node instanceof GraphQLUnionType ||
169+
node instanceof GraphQLScalarType) {
154170
observedTypes.add(((GraphQLNamedType) node).getName());
155171
}
156172

@@ -161,15 +177,12 @@ protected TraversalControl visitGraphQLType(GraphQLSchemaElement node,
161177
private static class FieldRemovalVisitor extends GraphQLTypeVisitorStub {
162178

163179
private final VisibleFieldPredicate visibilityPredicate;
164-
private final Set<GraphQLType> removedTypes;
165180

166181
private final Set<GraphQLFieldDefinition> fieldDefinitionsToActuallyRemove = new LinkedHashSet<>();
167182
private final Set<GraphQLInputObjectField> inputObjectFieldsToDelete = new LinkedHashSet<>();
168183

169-
private FieldRemovalVisitor(VisibleFieldPredicate visibilityPredicate,
170-
Set<GraphQLType> removedTypes) {
184+
private FieldRemovalVisitor(VisibleFieldPredicate visibilityPredicate) {
171185
this.visibilityPredicate = visibilityPredicate;
172-
this.removedTypes = removedTypes;
173186
}
174187

175188
@Override
@@ -189,7 +202,6 @@ private TraversalControl visitFieldsContainer(GraphQLFieldsContainer fieldsConta
189202
fieldDefinition, fieldsContainer);
190203
if (!visibilityPredicate.isVisible(environment)) {
191204
fieldDefinitionsToActuallyRemove.add(fieldDefinition);
192-
removedTypes.add(fieldDefinition.getType());
193205
} else {
194206
allFieldsDeleted = false;
195207
}
@@ -210,7 +222,6 @@ public TraversalControl visitGraphQLInputObjectType(GraphQLInputObjectType input
210222
inputField, inputObjectType);
211223
if (!visibilityPredicate.isVisible(environment)) {
212224
inputObjectFieldsToDelete.add(inputField);
213-
removedTypes.add(inputField.getType());
214225
} else {
215226
allFieldsDeleted = false;
216227
}
@@ -245,76 +256,43 @@ public TraversalControl visitGraphQLInputObjectField(GraphQLInputObjectField def
245256
}
246257
}
247258

248-
private static class TypeVisibilityVisitor extends GraphQLTypeVisitorStub {
259+
private static class TypeRemovalVisitor extends GraphQLTypeVisitorStub {
249260

250261
private final Set<String> protectedTypeNames;
251-
private final Set<String> observedBeforeTransform;
252-
private final Set<String> observedAfterTransform;
253262

254-
private TypeVisibilityVisitor(Set<String> protectedTypeNames,
255-
Set<String> observedTypes,
256-
Set<String> observedAfterTransform) {
263+
private TypeRemovalVisitor(Set<String> protectedTypeNames) {
257264
this.protectedTypeNames = protectedTypeNames;
258-
this.observedBeforeTransform = observedTypes;
259-
this.observedAfterTransform = observedAfterTransform;
260265
}
261266

262-
@Override
263-
public TraversalControl visitGraphQLInterfaceType(GraphQLInterfaceType node,
264-
TraverserContext<GraphQLSchemaElement> context) {
265-
return super.visitGraphQLInterfaceType(node, context);
266-
}
267267

268268
@Override
269269
public TraversalControl visitGraphQLType(GraphQLSchemaElement node,
270270
TraverserContext<GraphQLSchemaElement> context) {
271+
if (node instanceof GraphQLNamedType) {
272+
String name = ((GraphQLNamedType) node).getName();
273+
if (isIntrospectionType(name)) {
274+
return TraversalControl.CONTINUE;
275+
}
276+
}
271277
if (node instanceof GraphQLObjectType ||
272278
node instanceof GraphQLEnumType ||
273279
node instanceof GraphQLInputObjectType ||
274280
node instanceof GraphQLInterfaceType ||
275-
node instanceof GraphQLUnionType) {
281+
node instanceof GraphQLUnionType ||
282+
node instanceof GraphQLScalarType) {
276283
String name = ((GraphQLNamedType) node).getName();
277-
if (observedBeforeTransform.contains(name) &&
278-
!observedAfterTransform.contains(name)
279-
&& !protectedTypeNames.contains(name)
280-
) {
284+
if (!protectedTypeNames.contains(name)) {
281285
return deleteNode(context);
282286
}
283287
}
284288
return TraversalControl.CONTINUE;
285289
}
286290
}
287291

288-
private static class AdditionalTypeVisibilityVisitor extends GraphQLTypeVisitorStub {
289-
290-
private final Set<GraphQLType> markedForRemovalTypes;
291-
292-
private AdditionalTypeVisibilityVisitor(Set<GraphQLType> markedForRemovalTypes) {
293-
this.markedForRemovalTypes = markedForRemovalTypes;
294-
}
295-
296-
@Override
297-
public TraversalControl visitGraphQLType(GraphQLSchemaElement node,
298-
TraverserContext<GraphQLSchemaElement> context) {
299-
300-
if (node instanceof GraphQLNamedType) {
301-
GraphQLNamedType namedType = (GraphQLNamedType) node;
302-
// we encountered a node referencing one of the marked types, so it should not be removed.
303-
if (markedForRemovalTypes.contains(node)) {
304-
markedForRemovalTypes.remove(namedType);
305-
}
306-
}
307-
308-
return TraversalControl.CONTINUE;
309-
}
310-
}
311292

312293
private List<GraphQLSchemaElement> getRootTypes(GraphQLSchema schema) {
313294
return ImmutableList.<GraphQLSchemaElement>builder()
314295
.addAll(getOperationTypes(schema))
315-
// Include directive definitions as roots, since they won't be removed in the filtering process.
316-
// Some types (enums, input types, etc.) might be reachable only by directive definitions (and
317-
// not by other types or fields).
318296
.addAll(schema.getDirectives())
319297
.build();
320298
}

0 commit comments

Comments
 (0)