Skip to content

Commit a1c123d

Browse files
bbakermanbbakerman
authored
Fix a bug where enum value extensions were not picked up by schema validation correctly (#1942)
1 parent fccbe35 commit a1c123d

2 files changed

Lines changed: 101 additions & 37 deletions

File tree

‎src/main/java/graphql/schema/idl/ArgValueOfAllowedTypeChecker.java‎

Lines changed: 50 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,11 @@
77
import graphql.language.ArrayValue;
88
import graphql.language.Directive;
99
import graphql.language.EnumTypeDefinition;
10+
import graphql.language.EnumTypeExtensionDefinition;
1011
import graphql.language.EnumValue;
1112
import graphql.language.EnumValueDefinition;
1213
import graphql.language.InputObjectTypeDefinition;
14+
import graphql.language.InputObjectTypeExtensionDefinition;
1315
import graphql.language.InputValueDefinition;
1416
import graphql.language.ListType;
1517
import graphql.language.Node;
@@ -31,7 +33,7 @@
3133
import java.util.List;
3234
import java.util.Map;
3335
import java.util.function.Function;
34-
import java.util.stream.Collectors;
36+
import java.util.stream.Stream;
3537

3638
import static graphql.Assert.assertShouldNeverHappen;
3739
import static graphql.schema.idl.errors.DirectiveIllegalArgumentTypeError.DUPLICATED_KEYS_MESSAGE;
@@ -44,26 +46,31 @@
4446
import static graphql.schema.idl.errors.DirectiveIllegalArgumentTypeError.MUST_BE_VALID_ENUM_VALUE_MESSAGE;
4547
import static graphql.schema.idl.errors.DirectiveIllegalArgumentTypeError.NOT_A_VALID_SCALAR_LITERAL_MESSAGE;
4648
import static graphql.schema.idl.errors.DirectiveIllegalArgumentTypeError.UNKNOWN_FIELDS_MESSAGE;
49+
import static java.util.Collections.emptyList;
50+
import static java.util.stream.Collectors.counting;
51+
import static java.util.stream.Collectors.groupingBy;
52+
import static java.util.stream.Collectors.joining;
53+
import static java.util.stream.Collectors.toList;
54+
import static java.util.stream.Collectors.toMap;
4755

4856
/**
4957
* Class to check whether a given directive argument value
5058
* matches a given directive definition.
51-
*
5259
*/
5360
@Internal
5461
class ArgValueOfAllowedTypeChecker {
5562

5663
private static final Logger logNotSafe = LogKit.getNotPrivacySafeLogger(ArgValueOfAllowedTypeChecker.class);
5764

5865
private final Directive directive;
59-
private final Node element;
66+
private final Node<?> element;
6067
private final String elementName;
6168
private final Argument argument;
6269
private final TypeDefinitionRegistry typeRegistry;
6370
private final RuntimeWiring runtimeWiring;
6471

6572
ArgValueOfAllowedTypeChecker(final Directive directive,
66-
final Node element,
73+
final Node<?> element,
6774
final String elementName,
6875
final Argument argument,
6976
final TypeDefinitionRegistry typeRegistry,
@@ -79,20 +86,21 @@ class ArgValueOfAllowedTypeChecker {
7986
/**
8087
* Recursively inspects an argument value given an allowed type.
8188
* Given the (invalid) SDL below:
82-
*
83-
* directive @myDirective(arg: [[String]] ) on FIELD_DEFINITION
84-
*
85-
* query {
86-
* f: String @myDirective(arg: ["A String"])
87-
* }
88-
*
89+
* <p>
90+
* directive @myDirective(arg: [[String]] ) on FIELD_DEFINITION
91+
* <p>
92+
* query {
93+
* f: String @myDirective(arg: ["A String"])
94+
* }
95+
* <p>
8996
* it will first check that the `myDirective.arg` type is an array
9097
* and fail when finding "A String" as it expected a nested array ([[String]]).
91-
* @param errors validation error collector
92-
* @param instanceValue directive argument value
98+
*
99+
* @param errors validation error collector
100+
* @param instanceValue directive argument value
93101
* @param allowedArgType directive definition argument allowed type
94102
*/
95-
void checkArgValueMatchesAllowedType(List<GraphQLError> errors, Value instanceValue, Type allowedArgType) {
103+
void checkArgValueMatchesAllowedType(List<GraphQLError> errors, Value<?> instanceValue, Type<?> allowedArgType) {
96104
if (allowedArgType instanceof TypeName) {
97105
checkArgValueMatchesAllowedTypeName(errors, instanceValue, allowedArgType);
98106
} else if (allowedArgType instanceof ListType) {
@@ -108,13 +116,13 @@ private void addValidationError(List<GraphQLError> errors, String message, Objec
108116
errors.add(new DirectiveIllegalArgumentTypeError(element, elementName, directive.getName(), argument.getName(), String.format(message, args)));
109117
}
110118

111-
private void checkArgValueMatchesAllowedTypeName(List<GraphQLError> errors, Value instanceValue, Type allowedArgType) {
119+
private void checkArgValueMatchesAllowedTypeName(List<GraphQLError> errors, Value<?> instanceValue, Type<?> allowedArgType) {
112120
if (instanceValue instanceof NullValue) {
113121
return;
114122
}
115123

116124
String allowedTypeName = ((TypeName) allowedArgType).getName();
117-
TypeDefinition allowedTypeDefinition = typeRegistry.getType(allowedTypeName)
125+
TypeDefinition<?> allowedTypeDefinition = typeRegistry.getType(allowedTypeName)
118126
.orElseThrow(() -> new AssertException("Directive unknown argument type '%s'. This should have been validated before."));
119127

120128
if (allowedTypeDefinition instanceof ScalarTypeDefinition) {
@@ -128,7 +136,7 @@ private void checkArgValueMatchesAllowedTypeName(List<GraphQLError> errors, Valu
128136
}
129137
}
130138

131-
private void checkArgValueMatchesAllowedInputType(List<GraphQLError> errors, Value instanceValue, InputObjectTypeDefinition allowedTypeDefinition) {
139+
private void checkArgValueMatchesAllowedInputType(List<GraphQLError> errors, Value<?> instanceValue, InputObjectTypeDefinition allowedTypeDefinition) {
132140
if (!(instanceValue instanceof ObjectValue)) {
133141
addValidationError(errors, EXPECTED_OBJECT_MESSAGE, instanceValue.getClass().getSimpleName());
134142
return;
@@ -139,66 +147,72 @@ private void checkArgValueMatchesAllowedInputType(List<GraphQLError> errors, Val
139147
// then it must be the same type as the definition
140148

141149
List<ObjectField> fields = objectValue.getObjectFields();
142-
List<InputValueDefinition> inputValueDefinitions = allowedTypeDefinition.getInputValueDefinitions();
150+
List<InputObjectTypeExtensionDefinition> inputObjExt = typeRegistry.inputObjectTypeExtensions().getOrDefault(allowedTypeDefinition.getName(), emptyList());
151+
Stream<InputValueDefinition> inputObjExtValues = inputObjExt.stream().flatMap(inputObj -> inputObj.getInputValueDefinitions().stream());
152+
List<InputValueDefinition> inputValueDefinitions = Stream.concat(allowedTypeDefinition.getInputValueDefinitions().stream(), inputObjExtValues).collect(toList());
143153

144154
// check for duplicated fields
145155
Map<String, Long> fieldsToOccurrenceMap = fields.stream().map(ObjectField::getName)
146-
.collect(Collectors.groupingBy(Function.identity(), Collectors.counting()));
156+
.collect(groupingBy(Function.identity(), counting()));
147157

148158
if (fieldsToOccurrenceMap.values().stream().anyMatch(count -> count > 1)) {
149159
addValidationError(errors, DUPLICATED_KEYS_MESSAGE, fieldsToOccurrenceMap.entrySet().stream()
150160
.filter(entry -> entry.getValue() > 1)
151161
.map(Map.Entry::getKey)
152-
.collect(Collectors.joining(",")));
162+
.collect(joining(",")));
153163
return;
154164
}
155165

156166
// check for unknown fields
157167
Map<String, InputValueDefinition> nameToInputValueDefMap = inputValueDefinitions.stream()
158-
.collect(Collectors.toMap(InputValueDefinition::getName, inputValueDef -> inputValueDef));
168+
.collect(toMap(InputValueDefinition::getName, inputValueDef -> inputValueDef));
159169

160170
List<ObjectField> unknownFields = fields.stream()
161171
.filter(field -> !nameToInputValueDefMap.containsKey(field.getName()))
162-
.collect(Collectors.toList());
172+
.collect(toList());
163173

164174
if (!unknownFields.isEmpty()) {
165175
addValidationError(errors, UNKNOWN_FIELDS_MESSAGE,
166176
unknownFields.stream()
167177
.map(ObjectField::getName)
168-
.collect(Collectors.joining(",")),
178+
.collect(joining(",")),
169179
allowedTypeDefinition.getName());
170180
return;
171181
}
172182

173183
// fields to map for easy access
174184
Map<String, ObjectField> nameToFieldsMap = fields.stream()
175-
.collect(Collectors.toMap(ObjectField::getName, objectField -> objectField));
185+
.collect(toMap(ObjectField::getName, objectField -> objectField));
176186
// check each single field with its definition
177187
inputValueDefinitions.forEach(allowedValueDef -> {
178188
ObjectField objectField = nameToFieldsMap.get(allowedValueDef.getName());
179189
checkArgInputObjectValueFieldMatchesAllowedDefinition(errors, objectField, allowedValueDef);
180190
});
181191
}
182192

183-
private void checkArgValueMatchesAllowedEnum(List<GraphQLError> errors, Value instanceValue, EnumTypeDefinition allowedTypeDefinition) {
193+
private void checkArgValueMatchesAllowedEnum(List<GraphQLError> errors, Value<?> instanceValue, EnumTypeDefinition allowedTypeDefinition) {
184194
if (!(instanceValue instanceof EnumValue)) {
185195
addValidationError(errors, EXPECTED_ENUM_MESSAGE, instanceValue.getClass().getSimpleName());
186196
return;
187197
}
188198

189199
EnumValue enumValue = ((EnumValue) instanceValue);
190200

191-
boolean noneMatchAllowedEnumValue = allowedTypeDefinition.getEnumValueDefinitions().stream()
201+
List<EnumTypeExtensionDefinition> enumExtensions = typeRegistry.enumTypeExtensions().getOrDefault(allowedTypeDefinition.getName(), emptyList());
202+
Stream<EnumValueDefinition> enumExtStream = enumExtensions.stream().flatMap(enumExt -> enumExt.getEnumValueDefinitions().stream());
203+
List<EnumValueDefinition> enumValueDefinitions = Stream.concat(allowedTypeDefinition.getEnumValueDefinitions().stream(), enumExtStream).collect(toList());
204+
205+
boolean noneMatchAllowedEnumValue = enumValueDefinitions.stream()
192206
.noneMatch(enumAllowedValue -> enumAllowedValue.getName().equals(enumValue.getName()));
193207

194208
if (noneMatchAllowedEnumValue) {
195-
addValidationError(errors, MUST_BE_VALID_ENUM_VALUE_MESSAGE, enumValue.getName(), allowedTypeDefinition.getEnumValueDefinitions().stream()
196-
.map(EnumValueDefinition::getName)
197-
.collect(Collectors.joining(",")));
209+
addValidationError(errors, MUST_BE_VALID_ENUM_VALUE_MESSAGE, enumValue.getName(), enumValueDefinitions.stream()
210+
.map(EnumValueDefinition::getName)
211+
.collect(joining(",")));
198212
}
199213
}
200214

201-
private void checkArgValueMatchesAllowedScalar(List<GraphQLError> errors, Value instanceValue, String allowedTypeName) {
215+
private void checkArgValueMatchesAllowedScalar(List<GraphQLError> errors, Value<?> instanceValue, String allowedTypeName) {
202216
if (instanceValue instanceof ArrayValue
203217
|| instanceValue instanceof EnumValue
204218
|| instanceValue instanceof ObjectValue) {
@@ -231,23 +245,22 @@ private void checkArgInputObjectValueFieldMatchesAllowedDefinition(List<GraphQLE
231245
// - field definition is nullable hence null can be used
232246
}
233247

234-
private void checkArgValueMatchesAllowedNonNullType(List<GraphQLError> errors, Value instanceValue, NonNullType allowedArgType) {
248+
private void checkArgValueMatchesAllowedNonNullType(List<GraphQLError> errors, Value<?> instanceValue, NonNullType allowedArgType) {
235249
if (instanceValue instanceof NullValue) {
236250
addValidationError(errors, EXPECTED_NON_NULL_MESSAGE);
237251
return;
238252
}
239253

240-
Type unwrappedAllowedType = allowedArgType.getType();
254+
Type<?> unwrappedAllowedType = allowedArgType.getType();
241255
checkArgValueMatchesAllowedType(errors, instanceValue, unwrappedAllowedType);
242256
}
243257

244-
private void checkArgValueMatchesAllowedListType(List<GraphQLError> errors, Value instanceValue, ListType allowedArgType) {
258+
private void checkArgValueMatchesAllowedListType(List<GraphQLError> errors, Value<?> instanceValue, ListType allowedArgType) {
245259
if (instanceValue instanceof NullValue) {
246260
return;
247261
}
248262

249-
250-
Type unwrappedAllowedType = allowedArgType.getType();
263+
Type<?> unwrappedAllowedType = allowedArgType.getType();
251264
if (!(instanceValue instanceof ArrayValue)) {
252265
checkArgValueMatchesAllowedType(errors, instanceValue, unwrappedAllowedType);
253266
return;
@@ -259,14 +272,14 @@ private void checkArgValueMatchesAllowedListType(List<GraphQLError> errors, Valu
259272
// validate each instance value in the list, all instances must match for the list to match
260273
arrayValue.getValues().forEach(value -> {
261274
// restrictive check for sub-arrays
262-
if (isUnwrappedList && ! (value instanceof ArrayValue)) {
275+
if (isUnwrappedList && !(value instanceof ArrayValue)) {
263276
addValidationError(errors, EXPECTED_LIST_MESSAGE, value.getClass().getSimpleName());
264277
}
265278
checkArgValueMatchesAllowedType(errors, value, unwrappedAllowedType);
266279
});
267280
}
268281

269-
private boolean isArgumentValueScalarLiteral(GraphQLScalarType scalarType, Value instanceValue) {
282+
private boolean isArgumentValueScalarLiteral(GraphQLScalarType scalarType, Value<?> instanceValue) {
270283
try {
271284
scalarType.getCoercing().parseLiteral(instanceValue);
272285
return true;

‎src/test/groovy/graphql/schema/idl/SchemaGeneratorTest.groovy‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2156,4 +2156,55 @@ class SchemaGeneratorTest extends Specification {
21562156
21572157
}
21582158
2159+
def "extended enums work as expected for arg values"() {
2160+
2161+
given:
2162+
def spec = """
2163+
enum AuthRoles {
2164+
USER
2165+
}
2166+
2167+
extend enum AuthRoles {
2168+
AUTHENTICATED
2169+
}
2170+
2171+
directive @auth(if: AuthRoles) on FIELD_DEFINITION
2172+
2173+
type Query {
2174+
danger: String @auth(if: AUTHENTICATED)
2175+
}
2176+
"""
2177+
when:
2178+
def schema = schema(spec)
2179+
then:
2180+
def enumType = schema.getType("AuthRoles") as GraphQLEnumType
2181+
def listOfEnumValues = enumType.getValues().collect({ it.getValue() })
2182+
listOfEnumValues.sort() == ["AUTHENTICATED", "USER"]
2183+
}
2184+
2185+
def "extended input objects work as expected for arg values"() {
2186+
2187+
given:
2188+
def spec = """
2189+
input ArgInput {
2190+
fieldA : String
2191+
}
2192+
2193+
extend input ArgInput {
2194+
fieldB : String
2195+
}
2196+
2197+
directive @auth(if: ArgInput) on FIELD_DEFINITION
2198+
2199+
type Query {
2200+
danger: String @auth(if: { fieldB : "B"} )
2201+
}
2202+
"""
2203+
when:
2204+
def schema = schema(spec)
2205+
then:
2206+
def inputType = schema.getType("ArgInput") as GraphQLInputObjectType
2207+
def listOfEnumValues = inputType.getFieldDefinitions().collect({ it.getName() })
2208+
listOfEnumValues.sort() == ["fieldA", "fieldB"]
2209+
}
21592210
}

0 commit comments

Comments
 (0)