Skip to content

Commit a7c2d20

Browse files
andimarekandimarek
authored
Merge pull request #2147 from dugenkui03/patch-34
Ensure repeatable directive on extend type run correctly
2 parents 70c1c92 + d909b46 commit a7c2d20

2 files changed

Lines changed: 57 additions & 17 deletions

File tree

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

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -46,12 +46,12 @@ class SchemaTypeExtensionsChecker {
4646

4747
void checkTypeExtensions(List<GraphQLError> errors, TypeDefinitionRegistry typeRegistry) {
4848
Map<String, DirectiveDefinition> directiveDefinitionMap = typeRegistry.getDirectiveDefinitions();
49-
checkObjectTypeExtensions(errors, typeRegistry,directiveDefinitionMap);
50-
checkInterfaceTypeExtensions(errors, typeRegistry,directiveDefinitionMap);
51-
checkUnionTypeExtensions(errors, typeRegistry);
52-
checkEnumTypeExtensions(errors, typeRegistry);
53-
checkScalarTypeExtensions(errors, typeRegistry);
54-
checkInputObjectTypeExtensions(errors, typeRegistry,directiveDefinitionMap);
49+
checkObjectTypeExtensions(errors, typeRegistry, directiveDefinitionMap);
50+
checkInterfaceTypeExtensions(errors, typeRegistry, directiveDefinitionMap);
51+
checkUnionTypeExtensions(errors, typeRegistry, directiveDefinitionMap);
52+
checkEnumTypeExtensions(errors, typeRegistry, directiveDefinitionMap);
53+
checkScalarTypeExtensions(errors, typeRegistry, directiveDefinitionMap);
54+
checkInputObjectTypeExtensions(errors, typeRegistry, directiveDefinitionMap);
5555
}
5656

5757

@@ -69,7 +69,7 @@ private void checkObjectTypeExtensions(List<GraphQLError> errors, TypeDefinition
6969
typeRegistry.objectTypeExtensions()
7070
.forEach((name, extensions) -> {
7171
checkTypeExtensionHasCorrespondingType(errors, typeRegistry, name, extensions, ObjectTypeDefinition.class);
72-
checkTypeExtensionDirectiveRedefinition(errors, typeRegistry, name, extensions, ObjectTypeDefinition.class);
72+
checkTypeExtensionDirectiveRedefinition(errors, typeRegistry, name, extensions, ObjectTypeDefinition.class, directiveDefinitionMap);
7373

7474
extensions.forEach(extension -> {
7575
List<FieldDefinition> fieldDefinitions = extension.getFieldDefinitions();
@@ -117,7 +117,7 @@ private void checkInterfaceTypeExtensions(List<GraphQLError> errors, TypeDefinit
117117
typeRegistry.interfaceTypeExtensions()
118118
.forEach((name, extensions) -> {
119119
checkTypeExtensionHasCorrespondingType(errors, typeRegistry, name, extensions, InterfaceTypeDefinition.class);
120-
checkTypeExtensionDirectiveRedefinition(errors, typeRegistry, name, extensions, InterfaceTypeDefinition.class);
120+
checkTypeExtensionDirectiveRedefinition(errors, typeRegistry, name, extensions, InterfaceTypeDefinition.class, directiveDefinitionMap);
121121

122122
extensions.forEach(extension -> {
123123
List<FieldDefinition> fieldDefinitions = extension.getFieldDefinitions();
@@ -159,11 +159,11 @@ private void checkInterfaceTypeExtensions(List<GraphQLError> errors, TypeDefinit
159159
* All member types of a Union type extension must not already be a member of the original Union type.
160160
* Any directives provided must not already apply to the original Union type.
161161
*/
162-
private void checkUnionTypeExtensions(List<GraphQLError> errors, TypeDefinitionRegistry typeRegistry) {
162+
private void checkUnionTypeExtensions(List<GraphQLError> errors, TypeDefinitionRegistry typeRegistry, Map<String, DirectiveDefinition> directiveDefinitionMap) {
163163
typeRegistry.unionTypeExtensions()
164164
.forEach((name, extensions) -> {
165165
checkTypeExtensionHasCorrespondingType(errors, typeRegistry, name, extensions, UnionTypeDefinition.class);
166-
checkTypeExtensionDirectiveRedefinition(errors, typeRegistry, name, extensions, UnionTypeDefinition.class);
166+
checkTypeExtensionDirectiveRedefinition(errors, typeRegistry, name, extensions, UnionTypeDefinition.class, directiveDefinitionMap);
167167

168168
extensions.forEach(extension -> {
169169
List<TypeName> memberTypes = extension.getMemberTypes().stream()
@@ -192,11 +192,11 @@ private void checkUnionTypeExtensions(List<GraphQLError> errors, TypeDefinitionR
192192
* All values of an Enum type extension must not already be a value of the original Enum.
193193
* Any directives provided must not already apply to the original Enum type.
194194
*/
195-
private void checkEnumTypeExtensions(List<GraphQLError> errors, TypeDefinitionRegistry typeRegistry) {
195+
private void checkEnumTypeExtensions(List<GraphQLError> errors, TypeDefinitionRegistry typeRegistry, Map<String, DirectiveDefinition> directiveDefinitionMap) {
196196
typeRegistry.enumTypeExtensions()
197197
.forEach((name, extensions) -> {
198198
checkTypeExtensionHasCorrespondingType(errors, typeRegistry, name, extensions, EnumTypeDefinition.class);
199-
checkTypeExtensionDirectiveRedefinition(errors, typeRegistry, name, extensions, EnumTypeDefinition.class);
199+
checkTypeExtensionDirectiveRedefinition(errors, typeRegistry, name, extensions, EnumTypeDefinition.class, directiveDefinitionMap);
200200

201201
extensions.forEach(extension -> {
202202
// field unique ness
@@ -227,11 +227,11 @@ private void checkEnumTypeExtensions(List<GraphQLError> errors, TypeDefinitionRe
227227
* Any directives provided must not already apply to the original Scalar type.
228228
*/
229229

230-
private void checkScalarTypeExtensions(List<GraphQLError> errors, TypeDefinitionRegistry typeRegistry) {
230+
private void checkScalarTypeExtensions(List<GraphQLError> errors, TypeDefinitionRegistry typeRegistry, Map<String, DirectiveDefinition> directiveDefinitionMap) {
231231
typeRegistry.scalarTypeExtensions()
232232
.forEach((name, extensions) -> {
233233
checkTypeExtensionHasCorrespondingType(errors, typeRegistry, name, extensions, ScalarTypeDefinition.class);
234-
checkTypeExtensionDirectiveRedefinition(errors, typeRegistry, name, extensions, ScalarTypeDefinition.class);
234+
checkTypeExtensionDirectiveRedefinition(errors, typeRegistry, name, extensions, ScalarTypeDefinition.class, directiveDefinitionMap);
235235
});
236236

237237
}
@@ -248,7 +248,7 @@ private void checkInputObjectTypeExtensions(List<GraphQLError> errors, TypeDefin
248248
typeRegistry.inputObjectTypeExtensions()
249249
.forEach((name, extensions) -> {
250250
checkTypeExtensionHasCorrespondingType(errors, typeRegistry, name, extensions, InputObjectTypeDefinition.class);
251-
checkTypeExtensionDirectiveRedefinition(errors, typeRegistry, name, extensions, InputObjectTypeDefinition.class);
251+
checkTypeExtensionDirectiveRedefinition(errors, typeRegistry, name, extensions, InputObjectTypeDefinition.class, directiveDefinitionMap);
252252
// field redefinitions
253253
extensions.forEach(extension -> {
254254
List<InputValueDefinition> inputValueDefinitions = extension.getInputValueDefinitions();
@@ -287,13 +287,13 @@ private void checkTypeExtensionHasCorrespondingType(List<GraphQLError> errors, T
287287
}
288288

289289
@SuppressWarnings("unchecked")
290-
private void checkTypeExtensionDirectiveRedefinition(List<GraphQLError> errors, TypeDefinitionRegistry typeRegistry, String name, List<? extends TypeDefinition> extensions, Class<? extends TypeDefinition> targetClass) {
290+
private void checkTypeExtensionDirectiveRedefinition(List<GraphQLError> errors, TypeDefinitionRegistry typeRegistry, String name, List<? extends TypeDefinition> extensions, Class<? extends TypeDefinition> targetClass, Map<String, DirectiveDefinition> directiveDefinitionMap) {
291291
Optional<? extends TypeDefinition> typeDefinition = typeRegistry.getType(TypeName.newTypeName().name(name).build(), targetClass);
292292
if (typeDefinition.isPresent() && typeDefinition.get().getClass().equals(targetClass)) {
293293
List<Directive> directives = typeDefinition.get().getDirectives();
294294
Map<String, Directive> directiveMap = FpKit.getByName(directives, Directive::getName, mergeFirst());
295295
extensions.forEach(typeExt -> {
296-
List<Directive> extDirectives = typeExt.getDirectives();
296+
List<Directive> extDirectives = nonRepeatableDirectivesOnly(directiveDefinitionMap, typeExt.getDirectives());
297297
extDirectives.forEach(directive -> {
298298
if (directiveMap.containsKey(directive.getName())) {
299299
errors.add(new TypeExtensionDirectiveRedefinitionError(typeDefinition.get(), directive));

‎src/test/groovy/graphql/execution/directives/RepeatableDirectivesTest.groovy‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@ import graphql.language.Directive
55
import graphql.language.Field
66
import graphql.language.OperationDefinition
77
import graphql.language.StringValue
8+
import graphql.schema.idl.MockedWiringFactory
9+
import graphql.schema.idl.RuntimeWiring
10+
import graphql.schema.idl.SchemaGenerator
11+
import graphql.schema.idl.SchemaParser
812
import graphql.validation.Validator
913
import spock.lang.Specification
1014

@@ -82,5 +86,41 @@ class RepeatableDirectivesTest extends Specification {
8286
((StringValue) directives[1].getArgument("arg").getValue()).getValue() == "value2"
8387
}
8488

89+
def " ensure repeatable directive on extend type run correctly "() {
90+
given:
91+
def spec = '''
92+
directive @key(dirArg:String!) repeatable on OBJECT
93+
type Query{
94+
field: String!
95+
pTypedField(fieldArg: String!): PType
96+
}
97+
98+
type PType @key(dirArg:"a") @key(dirArg:"b") {
99+
name: String
100+
}
101+
102+
extend type PType @key(dirArg:"c") {
103+
extendField: String
104+
}
105+
'''
106+
107+
when:
108+
SchemaParser parser = new SchemaParser();
109+
def typeDefinitionRegistry = parser.parse(spec);
110+
def runtimeWiring = RuntimeWiring.newRuntimeWiring()
111+
.wiringFactory(new MockedWiringFactory())
112+
.build();
113+
def schemaGenerator = new SchemaGenerator();
114+
def schema = schemaGenerator.makeExecutableSchema(typeDefinitionRegistry, runtimeWiring);
115+
def pType = schema.getObjectType("PType")
116+
117+
then:
118+
pType.getExtensionDefinitions().size() == 1
119+
def extensionType = pType.getExtensionDefinitions().get(0)
120+
extensionType.getDirectives().size() == 1
121+
extensionType.getDirectives("key") != null
122+
extensionType.getFieldDefinitions().size() == 1
123+
extensionType.getFieldDefinitions().get(0).getName() == "extendField"
124+
}
85125

86126
}

0 commit comments

Comments
 (0)