Skip to content

Commit b2eb162

Browse files
andimarekandimarek
committed
fixes complex case for Field visibility transformer. See test case
1 parent 21b9743 commit b2eb162

File tree

2 files changed

+66
-24
lines changed

2 files changed

+66
-24
lines changed

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

Lines changed: 26 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,8 @@ public FieldVisibilitySchemaTransformation(VisibleFieldPredicate visibleFieldPre
6060
}
6161

6262
public final GraphQLSchema apply(GraphQLSchema schema) {
63-
Set<GraphQLType> observedBeforeTransform = new HashSet<>();
64-
Set<GraphQLType> observedAfterTransform = new HashSet<>();
63+
Set<String> observedBeforeTransform = new LinkedHashSet<>();
64+
Set<String> observedAfterTransform = new LinkedHashSet<>();
6565
Set<GraphQLType> markedForRemovalTypes = new HashSet<>();
6666

6767
// query, mutation, and subscription types should not be removed
@@ -135,18 +135,22 @@ protected TraversalControl visitGraphQLType(GraphQLSchemaElement node, Traverser
135135

136136
private static class TypeObservingVisitor extends GraphQLTypeVisitorStub {
137137

138-
private final Set<GraphQLType> observedTypes;
138+
private final Set<String> observedTypes;
139139

140140

141-
private TypeObservingVisitor(Set<GraphQLType> observedTypes) {
141+
private TypeObservingVisitor(Set<String> observedTypes) {
142142
this.observedTypes = observedTypes;
143143
}
144144

145145
@Override
146146
protected TraversalControl visitGraphQLType(GraphQLSchemaElement node,
147147
TraverserContext<GraphQLSchemaElement> context) {
148-
if (node instanceof GraphQLType) {
149-
observedTypes.add((GraphQLType) node);
148+
if (node instanceof GraphQLObjectType ||
149+
node instanceof GraphQLEnumType ||
150+
node instanceof GraphQLInputObjectType ||
151+
node instanceof GraphQLInterfaceType ||
152+
node instanceof GraphQLUnionType) {
153+
observedTypes.add(((GraphQLNamedType) node).getName());
150154
}
151155

152156
return TraversalControl.CONTINUE;
@@ -243,12 +247,12 @@ public TraversalControl visitGraphQLInputObjectField(GraphQLInputObjectField def
243247
private static class TypeVisibilityVisitor extends GraphQLTypeVisitorStub {
244248

245249
private final Set<String> protectedTypeNames;
246-
private final Set<GraphQLType> observedBeforeTransform;
247-
private final Set<GraphQLType> observedAfterTransform;
250+
private final Set<String> observedBeforeTransform;
251+
private final Set<String> observedAfterTransform;
248252

249253
private TypeVisibilityVisitor(Set<String> protectedTypeNames,
250-
Set<GraphQLType> observedTypes,
251-
Set<GraphQLType> observedAfterTransform) {
254+
Set<String> observedTypes,
255+
Set<String> observedAfterTransform) {
252256
this.protectedTypeNames = protectedTypeNames;
253257
this.observedBeforeTransform = observedTypes;
254258
this.observedAfterTransform = observedAfterTransform;
@@ -263,17 +267,19 @@ public TraversalControl visitGraphQLInterfaceType(GraphQLInterfaceType node,
263267
@Override
264268
public TraversalControl visitGraphQLType(GraphQLSchemaElement node,
265269
TraverserContext<GraphQLSchemaElement> context) {
266-
if (observedBeforeTransform.contains(node) &&
267-
!observedAfterTransform.contains(node) &&
268-
(node instanceof GraphQLObjectType ||
269-
node instanceof GraphQLEnumType ||
270-
node instanceof GraphQLInputObjectType ||
271-
node instanceof GraphQLInterfaceType ||
272-
node instanceof GraphQLUnionType)) {
273-
274-
return deleteNode(context);
270+
if (node instanceof GraphQLObjectType ||
271+
node instanceof GraphQLEnumType ||
272+
node instanceof GraphQLInputObjectType ||
273+
node instanceof GraphQLInterfaceType ||
274+
node instanceof GraphQLUnionType) {
275+
String name = ((GraphQLNamedType) node).getName();
276+
if (observedBeforeTransform.contains(name) &&
277+
!observedAfterTransform.contains(name)
278+
&& !protectedTypeNames.contains(name)
279+
) {
280+
return deleteNode(context);
281+
}
275282
}
276-
277283
return TraversalControl.CONTINUE;
278284
}
279285
}

‎src/test/groovy/graphql/schema/transform/FieldVisibilitySchemaTransformationTest.groovy‎

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import graphql.schema.GraphQLInputObjectType
99
import graphql.schema.GraphQLObjectType
1010
import graphql.schema.GraphQLSchema
1111
import graphql.schema.TypeResolver
12-
import graphql.schema.idl.SchemaPrinter
1312
import spock.lang.Specification
1413

1514
import static graphql.schema.GraphQLFieldDefinition.newFieldDefinition
@@ -973,7 +972,6 @@ class FieldVisibilitySchemaTransformationTest extends Specification {
973972
.build()
974973
when:
975974

976-
System.out.println((new SchemaPrinter()).print(schema))
977975
GraphQLSchema restrictedSchema = visibilitySchemaTransformation.apply(schema)
978976

979977
then:
@@ -1022,7 +1020,6 @@ class FieldVisibilitySchemaTransformationTest extends Specification {
10221020
.build()
10231021
when:
10241022

1025-
System.out.println((new SchemaPrinter()).print(schema))
10261023
GraphQLSchema restrictedSchema = visibilitySchemaTransformation.apply(schema)
10271024

10281025
then:
@@ -1057,7 +1054,6 @@ class FieldVisibilitySchemaTransformationTest extends Specification {
10571054
.build()
10581055
when:
10591056

1060-
System.out.println((new SchemaPrinter()).print(schema))
10611057
GraphQLSchema restrictedSchema = visibilitySchemaTransformation.apply(schema)
10621058

10631059
then:
@@ -1279,6 +1275,46 @@ class FieldVisibilitySchemaTransformationTest extends Specification {
12791275
(restrictedSchema.getType("Foo") as GraphQLObjectType).getFieldDefinition("toDelete") == null
12801276
}
12811277

1278+
def "remove field from a type which is referenced via additional types and an additional not reachable child is deleted"() {
1279+
given:
1280+
/*
1281+
the test case here is that ToDelete is changed, because ToDelete.toDelete is deleted
1282+
and additionally Indirect needs to be deleted because it is not reachable via the
1283+
Query type anymore.
1284+
We had a bug where ToDeleted was not deleted correctly, but because Indirect was, it resulted
1285+
in an invalid schema and exception.
1286+
*/
1287+
GraphQLSchema schema = TestUtil.schema("""
1288+
directive @private on FIELD_DEFINITION
1289+
type Query {
1290+
foo: String
1291+
toDelete: ToDelete @private
1292+
}
1293+
type ToDelete {
1294+
bare: String @private
1295+
toDelete:[Indirect!]
1296+
}
1297+
type Indirect {
1298+
foo: String
1299+
}
1300+
""")
1301+
1302+
when:
1303+
schema.typeMap
1304+
def patchedSchema = schema.transform { builder ->
1305+
schema.typeMap.each { entry ->
1306+
def type = entry.value
1307+
if (type != schema.queryType && type != schema.mutationType && type != schema.subscriptionType) {
1308+
builder.additionalType(type)
1309+
}
1310+
}
1311+
}
1312+
GraphQLSchema restrictedSchema = visibilitySchemaTransformation.apply(patchedSchema)
1313+
then:
1314+
(restrictedSchema.getType("Query") as GraphQLObjectType).getFieldDefinition("toDelete") == null
1315+
}
1316+
1317+
12821318
def "remove all fields from an input type which is referenced via additional types"() {
12831319
given:
12841320
GraphQLSchema schema = TestUtil.schema("""

0 commit comments

Comments
 (0)