Skip to content

Commit fdee810

Browse files
felipe-gdrfelipe-gdr
committed
1898: Fix field redefinition check in extensions
fix: graphql-java#1898
1 parent c508ac3 commit fdee810

3 files changed

Lines changed: 83 additions & 29 deletions

File tree

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

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
import graphql.GraphQLError;
44
import graphql.Internal;
55
import graphql.language.Argument;
6-
import graphql.language.AstPrinter;
76
import graphql.language.Directive;
87
import graphql.language.EnumTypeDefinition;
98
import graphql.language.EnumValueDefinition;
@@ -14,7 +13,6 @@
1413
import graphql.language.InterfaceTypeDefinition;
1514
import graphql.language.ObjectTypeDefinition;
1615
import graphql.language.ScalarTypeDefinition;
17-
import graphql.language.Type;
1816
import graphql.language.TypeDefinition;
1917
import graphql.language.TypeName;
2018
import graphql.language.UnionTypeDefinition;
@@ -328,10 +326,7 @@ private void checkForFieldRedefinition(List<GraphQLError> errors, TypeDefinition
328326
fieldDefinitions.forEach(fld -> {
329327
FieldDefinition reference = referenceMap.get(fld.getName());
330328
if (referenceMap.containsKey(fld.getName())) {
331-
// ok they have the same field but is it the same type
332-
if (!isSameType(fld.getType(), reference.getType())) {
333-
errors.add(new TypeExtensionFieldRedefinitionError(typeDefinition, fld));
334-
}
329+
errors.add(new TypeExtensionFieldRedefinitionError(typeDefinition, fld));
335330
}
336331
});
337332
}
@@ -342,10 +337,7 @@ private void checkForInputValueRedefinition(List<GraphQLError> errors, InputObje
342337
inputValueDefinitions.forEach(fld -> {
343338
InputValueDefinition reference = referenceMap.get(fld.getName());
344339
if (referenceMap.containsKey(fld.getName())) {
345-
// ok they have the same field but is it the same type
346-
if (!isSameType(fld.getType(), reference.getType())) {
347-
errors.add(new TypeExtensionFieldRedefinitionError(typeDefinition, fld));
348-
}
340+
errors.add(new TypeExtensionFieldRedefinitionError(typeDefinition, fld));
349341
}
350342
});
351343
}
@@ -369,12 +361,4 @@ private <T> void forEachBut(T butThisOne, List<T> list, Consumer<T> consumer) {
369361
consumer.accept(t);
370362
}
371363
}
372-
373-
374-
private boolean isSameType(Type type1, Type type2) {
375-
String s1 = AstPrinter.printAst(type1);
376-
String s2 = AstPrinter.printAst(type2);
377-
return s1.equals(s2);
378-
}
379-
380364
}

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -531,11 +531,9 @@ class SchemaGeneratorTest extends Specification {
531531
extraField1 : String
532532
}
533533
extend type BaseType implements Interface2 {
534-
extraField1 : String
535534
extraField2 : Int
536535
}
537536
extend type BaseType implements Interface3 {
538-
extraField1 : String
539537
extraField3 : ID
540538
}
541539
extend type BaseType {
@@ -604,7 +602,6 @@ class SchemaGeneratorTest extends Specification {
604602
name: String!
605603
}
606604
extend type Human implements Character {
607-
name: String!
608605
friends: [Character]
609606
}
610607
extend type Human {

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

Lines changed: 81 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -400,8 +400,8 @@ class SchemaTypeCheckerTest extends Specification {
400400
result.get(0).getMessage().contains("tried to redefine field 'fieldA'")
401401
}
402402

403-
def "test ext type can redefine fields in their base type of the same type"() {
404-
403+
def "test ext type cannot redefine fields in their base type of the same type"() {
404+
given:
405405
def spec = """
406406
407407
type BaseType {
@@ -418,11 +418,12 @@ class SchemaTypeCheckerTest extends Specification {
418418
}
419419
"""
420420

421+
when:
421422
def result = check(spec)
422423

423-
expect:
424+
then:
424425

425-
result.isEmpty()
426+
errorContaining(result, "BaseType' extension type [@n:n] tried to redefine field 'fieldA' [@n:n]")
426427
}
427428

428429
def "test ext type redefines fields in their peer types"() {
@@ -454,7 +455,7 @@ class SchemaTypeCheckerTest extends Specification {
454455
}
455456

456457
def "test ext type redefines fields in their peer types of the same type is ok"() {
457-
458+
given:
458459
def spec = """
459460
460461
type BaseType {
@@ -474,11 +475,11 @@ class SchemaTypeCheckerTest extends Specification {
474475
}
475476
"""
476477

478+
when:
477479
def result = check(spec)
478480

479-
expect:
480-
481-
result.isEmpty()
481+
then:
482+
errorContaining(result, "BaseType' extension type [@n:n] tried to redefine field 'fieldB' [@n:n]")
482483
}
483484

484485
def "test ext type is missing the base type"() {
@@ -1665,4 +1666,76 @@ class SchemaTypeCheckerTest extends Specification {
16651666

16661667
}
16671668

1669+
def "field in base interface type redefined in extension type should cause an error"() {
1670+
given:
1671+
def sdl = """
1672+
type Query { hello: String }
1673+
1674+
interface Human {
1675+
id: ID!
1676+
name: String!
1677+
}
1678+
extend interface Human {
1679+
name: String!
1680+
friends: [String]
1681+
}
1682+
"""
1683+
1684+
when:
1685+
def result = check(sdl)
1686+
1687+
then:
1688+
errorContaining(result, "'Human' extension type [@n:n] tried to redefine field 'name' [@n:n]")
1689+
1690+
}
1691+
1692+
def "field in interface extension type redefined in another extension type should cause an error"() {
1693+
given:
1694+
def sdl = """
1695+
type Query { hello: String }
1696+
1697+
interface Human {
1698+
id: ID!
1699+
}
1700+
1701+
extend interface Human {
1702+
name: String!
1703+
}
1704+
1705+
extend interface Human {
1706+
name: String!
1707+
friends: [String]
1708+
}
1709+
"""
1710+
1711+
when:
1712+
def result = check(sdl)
1713+
1714+
then:
1715+
errorContaining(result, "'Human' extension type [@n:n] tried to redefine field 'name' [@n:n]")
1716+
1717+
}
1718+
1719+
def "field in base input type redefined in extension type should cause an error"() {
1720+
given:
1721+
def sdl = """
1722+
type Query { hello: String }
1723+
1724+
input Human {
1725+
id: ID!
1726+
name: String!
1727+
}
1728+
extend input Human {
1729+
name: String!
1730+
friends: [String]
1731+
}
1732+
"""
1733+
1734+
when:
1735+
def result = check(sdl)
1736+
1737+
then:
1738+
errorContaining(result, "'Human' extension type [@n:n] tried to redefine field 'name' [@n:n]")
1739+
1740+
}
16681741
}

0 commit comments

Comments
 (0)