Skip to content

Fix field visibility bug with enum with enum args #2926

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bbakerman merged 1 commit into from
Aug 22, 2022
+78 −3
Merged

Fix field visibility bug with enum with enum args #2926

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions src/main/java/graphql/schema/transform/FieldVisibilitySchemaTransformation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
package graphql.schema.transform;

import com.google.common.collect.ImmutableList;
import graphql.PublicApi;
import graphql.schema.GraphQLEnumType;
import graphql.schema.GraphQLFieldDefinition;
import graphql.schema.GraphQLImplementingType;
import graphql.schema.GraphQLInputObjectField;
import graphql.schema.GraphQLInputObjectType;
import graphql.schema.GraphQLInterfaceType;
import graphql.schema.GraphQLNamedSchemaElement;
import graphql.schema.GraphQLNamedType;
Expand Down Expand Up @@ -60,7 +62,7 @@ public final GraphQLSchema apply(GraphQLSchema schema) {
Set<GraphQLType> markedForRemovalTypes = new HashSet<>();

// query, mutation, and subscription types should not be removed
final Set<String> protectedTypeNames = getRootTypes(schema).stream()
final Set<String> protectedTypeNames = getOperationTypes(schema).stream()
.map(GraphQLObjectType::getName)
.collect(Collectors.toSet());

Expand Down Expand Up @@ -216,6 +218,7 @@ public TraversalControl visitGraphQLType(GraphQLSchemaElement node,
!observedAfterTransform.contains(node) &&
(node instanceof GraphQLObjectType ||
node instanceof GraphQLEnumType ||
node instanceof GraphQLInputObjectType ||
node instanceof GraphQLInterfaceType ||
node instanceof GraphQLUnionType)) {

Expand Down Expand Up @@ -250,12 +253,21 @@ public TraversalControl visitGraphQLType(GraphQLSchemaElement node,
}
}

private List<GraphQLObjectType> getRootTypes(GraphQLSchema schema) {
private List<GraphQLSchemaElement> getRootTypes(GraphQLSchema schema) {
return ImmutableList.<GraphQLSchemaElement>builder()
.addAll(getOperationTypes(schema))
// Include directive definitions as roots, since they won't be removed in the filtering process.
// Some types (enums, input types, etc.) might be reachable only by directive definitions (and
// not by other types or fields).
.addAll(schema.getDirectives())
Copy link
Member

@bbakerman bbakerman Aug 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we have schema.getAdditionalTypes here as well

Copy link
Member

@bbakerman bbakerman Aug 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this ends up removing types that are no linked (if I recall right) so perhap no

Copy link
Member Author

@felipe-gdr felipe-gdr Aug 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the opposite would happen: types that became unreachable after fields were filtered out would still show up in the final schema, because essentially all types would be root types and thus reachable.

.build();
}

private List<GraphQLObjectType> getOperationTypes(GraphQLSchema schema) {
return Stream.of(
schema.getQueryType(),
schema.getSubscriptionType(),
schema.getMutationType()
).filter(Objects::nonNull).collect(Collectors.toList());
}

}
63 changes: 63 additions & 0 deletions src/test/groovy/graphql/schema/transform/FieldVisibilitySchemaTransformationTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1178,4 +1178,67 @@ class FieldVisibilitySchemaTransformationTest extends Specification {
(restrictedSchema.getType("Account") as GraphQLObjectType).getFieldDefinition("billingStatus") == null
restrictedSchema.getType("BillingStatus") != null
}

def "can remove a field with a directive containing enum argument"() {
given:
GraphQLSchema schema = TestUtil.schema("""

directive @private(privateType: SecretType) on FIELD_DEFINITION
enum SecretType {
SUPER_SECRET
NOT_SO_SECRET
}

type Query {
account: Account
}

type Account {
name: String
billingStatus: BillingStatus @private(privateType: NOT_SO_SECRET)
}

type BillingStatus {
accountNumber: String
}
""")

when:
GraphQLSchema restrictedSchema = visibilitySchemaTransformation.apply(schema)

then:
(restrictedSchema.getType("Account") as GraphQLObjectType).getFieldDefinition("billingStatus") == null
restrictedSchema.getType("BillingStatus") == null
}

def "can remove a field with a directive containing type argument"() {
given:
GraphQLSchema schema = TestUtil.schema("""

directive @private(privateType: SecretType) on FIELD_DEFINITION
input SecretType {
description: String
}

type Query {
account: Account
}

type Account {
name: String
billingStatus: BillingStatus @private(privateType: { description: "secret" })
}

type BillingStatus {
accountNumber: String
}
""")

when:
GraphQLSchema restrictedSchema = visibilitySchemaTransformation.apply(schema)

then:
(restrictedSchema.getType("Account") as GraphQLObjectType).getFieldDefinition("billingStatus") == null
restrictedSchema.getType("BillingStatus") == null
}
}