Skip to content

Commit 8f74506

Browse files
andimarekandimarek
committed
Fix covariance checks for SDL type extensions
1 parent 4d78813 commit 8f74506

3 files changed

Lines changed: 290 additions & 42 deletions

File tree

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

Lines changed: 36 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,9 @@
3737
import java.util.LinkedHashMap;
3838
import java.util.List;
3939
import java.util.Map;
40-
import java.util.Objects;
4140
import java.util.Optional;
4241
import java.util.function.Function;
42+
import java.util.stream.Stream;
4343

4444
import static graphql.Assert.assertNotNull;
4545
import static graphql.schema.idl.SchemaExtensionsChecker.defineOperationDefs;
@@ -719,18 +719,7 @@ public <T extends TypeDefinition> Map<String, T> getTypesMap(Class<T> targetClas
719719
public List<ImplementingTypeDefinition> getAllImplementationsOf(InterfaceTypeDefinition targetInterface) {
720720
return ImmutableKit.filter(
721721
getTypes(ImplementingTypeDefinition.class),
722-
implementingTypeDefinition -> {
723-
List<Type<?>> implementsList = implementingTypeDefinition.getImplements();
724-
for (Type iFace : implementsList) {
725-
InterfaceTypeDefinition interfaceTypeDef = getTypeOrNull(iFace, InterfaceTypeDefinition.class);
726-
if (interfaceTypeDef != null) {
727-
if (interfaceTypeDef.getName().equals(targetInterface.getName())) {
728-
return true;
729-
}
730-
}
731-
}
732-
return false;
733-
});
722+
implementingTypeDefinition -> implementsInterface(implementingTypeDefinition, targetInterface));
734723
}
735724

736725
/**
@@ -765,37 +754,42 @@ public boolean isPossibleType(Type abstractType, Type possibleType) {
765754
if (!isObjectTypeOrInterface(possibleType)) {
766755
return false;
767756
}
768-
TypeDefinition targetObjectTypeDef = Objects.requireNonNull(getTypeOrNull(possibleType));
769-
TypeDefinition abstractTypeDef = Objects.requireNonNull(getTypeOrNull(abstractType));
757+
TypeDefinition possibleTypeDef = assertNotNull(getTypeOrNull(possibleType));
758+
TypeDefinition abstractTypeDef = assertNotNull(getTypeOrNull(abstractType));
770759
if (abstractTypeDef instanceof UnionTypeDefinition) {
771-
List<Type> memberTypes = ((UnionTypeDefinition) abstractTypeDef).getMemberTypes();
772-
for (Type memberType : memberTypes) {
773-
ObjectTypeDefinition checkType = getTypeOrNull(memberType, ObjectTypeDefinition.class);
774-
if (checkType != null) {
775-
if (checkType.getName().equals(targetObjectTypeDef.getName())) {
776-
return true;
777-
}
778-
}
779-
}
780-
return false;
781-
} else {
782-
InterfaceTypeDefinition iFace = (InterfaceTypeDefinition) abstractTypeDef;
783-
for (TypeDefinition<?> t : types.values()) {
784-
if (t instanceof ImplementingTypeDefinition) {
785-
if (t.getName().equals(targetObjectTypeDef.getName())) {
786-
ImplementingTypeDefinition<?> itd = (ImplementingTypeDefinition<?>) t;
787-
788-
for (Type implementsType : itd.getImplements()) {
789-
TypeDefinition<?> matchingInterface = types.get(typeName(implementsType));
790-
if (matchingInterface != null && matchingInterface.getName().equals(iFace.getName())) {
791-
return true;
792-
}
793-
}
794-
}
795-
}
796-
}
797-
return false;
760+
return isUnionMember((UnionTypeDefinition) abstractTypeDef, possibleTypeDef);
761+
}
762+
return implementsInterface(
763+
(ImplementingTypeDefinition<?>) possibleTypeDef,
764+
(InterfaceTypeDefinition) abstractTypeDef);
765+
}
766+
767+
private boolean implementsInterface(
768+
ImplementingTypeDefinition<?> implementingType,
769+
InterfaceTypeDefinition targetInterface) {
770+
return Stream.concat(
771+
Stream.of(implementingType),
772+
getImplementingTypeExtensions(implementingType).stream())
773+
.flatMap(type -> type.getImplements().stream())
774+
.map(TypeInfo::typeName)
775+
.anyMatch(targetInterface.getName()::equals);
776+
}
777+
778+
private List<? extends ImplementingTypeDefinition<?>> getImplementingTypeExtensions(
779+
ImplementingTypeDefinition<?> implementingType) {
780+
if (implementingType instanceof InterfaceTypeDefinition) {
781+
return interfaceTypeExtensions.getOrDefault(implementingType.getName(), List.of());
798782
}
783+
return objectTypeExtensions.getOrDefault(implementingType.getName(), List.of());
784+
}
785+
786+
private boolean isUnionMember(UnionTypeDefinition unionType, TypeDefinition<?> possibleType) {
787+
return Stream.concat(
788+
unionType.getMemberTypes().stream(),
789+
unionTypeExtensions.getOrDefault(unionType.getName(), List.of()).stream()
790+
.flatMap(extension -> extension.getMemberTypes().stream()))
791+
.map(TypeInfo::typeName)
792+
.anyMatch(possibleType.getName()::equals);
799793
}
800794

801795
/**

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

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1337,6 +1337,171 @@ class SchemaTypeCheckerTest extends Specification {
13371337

13381338
}
13391339

1340+
def "covariant object type implemented through an extension is supported"() {
1341+
def spec = '''
1342+
type Query {
1343+
base: Base
1344+
}
1345+
1346+
interface Pet {
1347+
id: ID
1348+
}
1349+
1350+
type Dog {
1351+
id: ID
1352+
}
1353+
1354+
extend type Dog implements Pet
1355+
1356+
type Base {
1357+
foo: String
1358+
}
1359+
1360+
interface PetContainer {
1361+
pet: Pet
1362+
}
1363+
1364+
extend type Base implements PetContainer {
1365+
pet: Dog
1366+
}
1367+
'''
1368+
1369+
def result = check(spec, ["Pet", "PetContainer"])
1370+
1371+
expect:
1372+
result.isEmpty()
1373+
}
1374+
1375+
def "covariant interface type implemented through an extension is supported"() {
1376+
def spec = '''
1377+
type Query {
1378+
base: Base
1379+
}
1380+
1381+
interface Pet {
1382+
id: ID
1383+
}
1384+
1385+
interface WorkingPet {
1386+
id: ID
1387+
}
1388+
1389+
extend interface WorkingPet implements Pet
1390+
1391+
interface PetContainer {
1392+
pet: Pet
1393+
}
1394+
1395+
type Base implements PetContainer {
1396+
pet: WorkingPet
1397+
}
1398+
'''
1399+
1400+
def result = check(spec, ["Pet", "WorkingPet", "PetContainer"])
1401+
1402+
expect:
1403+
result.isEmpty()
1404+
}
1405+
1406+
def "covariant union member added through an extension is supported"() {
1407+
def spec = '''
1408+
type Query {
1409+
base: Base
1410+
}
1411+
1412+
type Cat {
1413+
id: ID
1414+
}
1415+
1416+
type Dog {
1417+
id: ID
1418+
}
1419+
1420+
union Pets = Cat
1421+
1422+
extend union Pets = Dog
1423+
1424+
interface PetContainer {
1425+
pet: Pets
1426+
}
1427+
1428+
type Base implements PetContainer {
1429+
pet: Dog
1430+
}
1431+
'''
1432+
1433+
def result = check(spec, ["Pets", "PetContainer"])
1434+
1435+
expect:
1436+
result.isEmpty()
1437+
}
1438+
1439+
def "wrapped covariant object type implemented through an extension is supported"() {
1440+
def spec = '''
1441+
type Query {
1442+
base: Base
1443+
}
1444+
1445+
interface Pet {
1446+
id: ID
1447+
}
1448+
1449+
type Dog {
1450+
id: ID
1451+
}
1452+
1453+
extend type Dog implements Pet
1454+
1455+
interface PetContainer {
1456+
pets: [Pet]!
1457+
}
1458+
1459+
type Base implements PetContainer {
1460+
pets: [Dog!]!
1461+
}
1462+
'''
1463+
1464+
def result = check(spec, ["Pet", "PetContainer"])
1465+
1466+
expect:
1467+
result.isEmpty()
1468+
}
1469+
1470+
def "unrelated type remains invalid when other interface relationships use extensions"() {
1471+
def spec = '''
1472+
type Query {
1473+
base: Base
1474+
}
1475+
1476+
interface Pet {
1477+
id: ID
1478+
}
1479+
1480+
interface Vehicle {
1481+
id: ID
1482+
}
1483+
1484+
type Car {
1485+
id: ID
1486+
}
1487+
1488+
extend type Car implements Vehicle
1489+
1490+
interface PetContainer {
1491+
pet: Pet
1492+
}
1493+
1494+
type Base implements PetContainer {
1495+
pet: Car
1496+
}
1497+
'''
1498+
1499+
def result = check(spec, ["Pet", "Vehicle", "PetContainer"])
1500+
1501+
expect:
1502+
errorContaining(result, "has tried to redefine field 'pet' defined via interface 'PetContainer'")
1503+
}
1504+
13401505
def "deviant covariant object types are detected"() {
13411506

13421507
def spec = '''

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

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -469,6 +469,87 @@ class TypeDefinitionRegistryTest extends Specification {
469469
470470
'''
471471

472+
def extensionRelationships = '''
473+
interface Pet {
474+
id: ID
475+
}
476+
477+
interface WorkingPet {
478+
id: ID
479+
}
480+
481+
extend interface WorkingPet implements Pet
482+
483+
type Dog {
484+
id: ID
485+
}
486+
487+
extend type Dog implements Pet
488+
489+
type Cat {
490+
id: ID
491+
}
492+
493+
union Pets = Cat
494+
495+
extend union Pets = Dog
496+
'''
497+
498+
def "possible type detection includes interface implementations from extensions for #typeOfReg registry"() {
499+
when:
500+
def registry = registry(extensionRelationships, typeOfReg)
501+
502+
then:
503+
registry.isPossibleType(type("Pet"), type("Dog"))
504+
registry.isPossibleType(type("Pet"), type("WorkingPet"))
505+
!registry.isPossibleType(type("Pet"), type("Cat"))
506+
507+
where:
508+
typeOfReg << ["mutable", "immutable"]
509+
}
510+
511+
def "possible type detection includes union members from extensions for #typeOfReg registry"() {
512+
when:
513+
def registry = registry(extensionRelationships, typeOfReg)
514+
515+
then:
516+
registry.isPossibleType(type("Pets"), type("Cat"))
517+
registry.isPossibleType(type("Pets"), type("Dog"))
518+
!registry.isPossibleType(type("Pets"), type("WorkingPet"))
519+
520+
where:
521+
typeOfReg << ["mutable", "immutable"]
522+
}
523+
524+
def "subtype detection unwraps types implemented through extensions for #typeOfReg registry"() {
525+
when:
526+
def registry = registry(extensionRelationships, typeOfReg)
527+
528+
then:
529+
registry.isSubTypeOf(nonNullType("Dog"), type("Pet"))
530+
registry.isSubTypeOf(listType(type("Dog")), listType(type("Pet")))
531+
registry.isSubTypeOf(
532+
listType(nonNullType(listType(type("Dog")))),
533+
listType(nonNullType(listType(type("Pet")))))
534+
!registry.isSubTypeOf(type("Cat"), type("Pet"))
535+
536+
where:
537+
typeOfReg << ["mutable", "immutable"]
538+
}
539+
540+
def "implementation lookup includes relationships from extensions for #typeOfReg registry"() {
541+
when:
542+
def registry = registry(extensionRelationships, typeOfReg)
543+
def pet = registry.getTypeOrNull("Pet", InterfaceTypeDefinition.class)
544+
545+
then:
546+
registry.getAllImplementationsOf(pet)*.name == ["WorkingPet", "Dog"]
547+
registry.getImplementationsOf(pet)*.name == ["Dog"]
548+
549+
where:
550+
typeOfReg << ["mutable", "immutable"]
551+
}
552+
472553
def "test possible type detection #typeOfReg"() {
473554
given:
474555
TypeDefinitionRegistry mutableReg = parse(animalia)
@@ -506,6 +587,14 @@ class TypeDefinitionRegistryTest extends Specification {
506587
"immutable" | _
507588
}
508589

590+
private static TypeDefinitionRegistry registry(String spec, String typeOfRegistry) {
591+
def registry = parse(spec)
592+
if (typeOfRegistry == "immutable") {
593+
return registry.readOnly()
594+
}
595+
return registry
596+
}
597+
509598

510599
def "isSubTypeOf detection #typeOfReg"() {
511600
when:

0 commit comments

Comments
 (0)