FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Fix covariance checks for SDL type extensions by andimarek · Pull Request #4420 · graphql-java/graphql-java · GitHub

/ graphql-java Public

Fix covariance checks for SDL type extensions #4420

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
andimarek merged 1 commit into from
Jul 23, 2026
Conversation 1 Commits 1 Checks 12 Files changed
Merged

Fix covariance checks for SDL type extensions #4420

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 .groovy  (2) .java  (1) All 2 file types selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
78 changes: 36 additions & 42 deletions src/main/java/graphql/schema/idl/TypeDefinitionRegistry.java
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.function.Function;
import java.util.stream.Stream;

import static graphql.Assert.assertNotNull;
import static graphql.schema.idl.SchemaExtensionsChecker.defineOperationDefs;
Expand Down Expand Up @@ -719,18 +719,7 @@ public <T extends TypeDefinition> Map<String, T> getTypesMap(Class<T> targetClas
public List<ImplementingTypeDefinition> getAllImplementationsOf(InterfaceTypeDefinition targetInterface) {
return ImmutableKit.filter(
getTypes(ImplementingTypeDefinition.class),
implementingTypeDefinition -> {
List<Type<?>> implementsList = implementingTypeDefinition.getImplements();
for (Type iFace : implementsList) {
InterfaceTypeDefinition interfaceTypeDef = getTypeOrNull(iFace, InterfaceTypeDefinition.class);
if (interfaceTypeDef != null) {
if (interfaceTypeDef.getName().equals(targetInterface.getName())) {
return true;
}
}
}
return false;
});
implementingTypeDefinition -> implementsInterface(implementingTypeDefinition, targetInterface));
}

/**
Expand Down Expand Up @@ -765,37 +754,42 @@ public boolean isPossibleType(Type abstractType, Type possibleType) {
if (!isObjectTypeOrInterface(possibleType)) {
return false;
}
TypeDefinition targetObjectTypeDef = Objects.requireNonNull(getTypeOrNull(possibleType));
TypeDefinition abstractTypeDef = Objects.requireNonNull(getTypeOrNull(abstractType));
TypeDefinition possibleTypeDef = assertNotNull(getTypeOrNull(possibleType));
TypeDefinition abstractTypeDef = assertNotNull(getTypeOrNull(abstractType));
if (abstractTypeDef instanceof UnionTypeDefinition) {
List<Type> memberTypes = ((UnionTypeDefinition) abstractTypeDef).getMemberTypes();
for (Type memberType : memberTypes) {
ObjectTypeDefinition checkType = getTypeOrNull(memberType, ObjectTypeDefinition.class);
if (checkType != null) {
if (checkType.getName().equals(targetObjectTypeDef.getName())) {
return true;
}
}
}
return false;
} else {
InterfaceTypeDefinition iFace = (InterfaceTypeDefinition) abstractTypeDef;
for (TypeDefinition<?> t : types.values()) {
if (t instanceof ImplementingTypeDefinition) {
if (t.getName().equals(targetObjectTypeDef.getName())) {
ImplementingTypeDefinition<?> itd = (ImplementingTypeDefinition<?>) t;

for (Type implementsType : itd.getImplements()) {
TypeDefinition<?> matchingInterface = types.get(typeName(implementsType));
if (matchingInterface != null && matchingInterface.getName().equals(iFace.getName())) {
return true;
}
}
}
}
}
return false;
return isUnionMember((UnionTypeDefinition) abstractTypeDef, possibleTypeDef);
}
return implementsInterface(
(ImplementingTypeDefinition<?>) possibleTypeDef,
(InterfaceTypeDefinition) abstractTypeDef);
}

private boolean implementsInterface(
ImplementingTypeDefinition<?> implementingType,
InterfaceTypeDefinition targetInterface) {
return Stream.concat(
Stream.of(implementingType),
getImplementingTypeExtensions(implementingType).stream())
.flatMap(type -> type.getImplements().stream())
.map(TypeInfo::typeName)
.anyMatch(targetInterface.getName()::equals);
}

private List<? extends ImplementingTypeDefinition<?>> getImplementingTypeExtensions(
ImplementingTypeDefinition<?> implementingType) {
if (implementingType instanceof InterfaceTypeDefinition) {
return interfaceTypeExtensions.getOrDefault(implementingType.getName(), List.of());
}
return objectTypeExtensions.getOrDefault(implementingType.getName(), List.of());
}

private boolean isUnionMember(UnionTypeDefinition unionType, TypeDefinition<?> possibleType) {
return Stream.concat(
unionType.getMemberTypes().stream(),
unionTypeExtensions.getOrDefault(unionType.getName(), List.of()).stream()
.flatMap(extension -> extension.getMemberTypes().stream()))
.map(TypeInfo::typeName)
.anyMatch(possibleType.getName()::equals);
}

/**
Expand Down
165 changes: 165 additions & 0 deletions src/test/groovy/graphql/schema/idl/SchemaTypeCheckerTest.groovy
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -1337,6 +1337,171 @@ class SchemaTypeCheckerTest extends Specification {

}

def "covariant object type implemented through an extension is supported"() {
def spec = '''
type Query {
base: Base
}

interface Pet {
id: ID
}

type Dog {
id: ID
}

extend type Dog implements Pet

type Base {
foo: String
}

interface PetContainer {
pet: Pet
}

extend type Base implements PetContainer {
pet: Dog
}
'''

def result = check(spec, ["Pet", "PetContainer"])

expect:
result.isEmpty()
}

def "covariant interface type implemented through an extension is supported"() {
def spec = '''
type Query {
base: Base
}

interface Pet {
id: ID
}

interface WorkingPet {
id: ID
}

extend interface WorkingPet implements Pet

interface PetContainer {
pet: Pet
}

type Base implements PetContainer {
pet: WorkingPet
}
'''

def result = check(spec, ["Pet", "WorkingPet", "PetContainer"])

expect:
result.isEmpty()
}

def "covariant union member added through an extension is supported"() {
def spec = '''
type Query {
base: Base
}

type Cat {
id: ID
}

type Dog {
id: ID
}

union Pets = Cat

extend union Pets = Dog

interface PetContainer {
pet: Pets
}

type Base implements PetContainer {
pet: Dog
}
'''

def result = check(spec, ["Pets", "PetContainer"])

expect:
result.isEmpty()
}

def "wrapped covariant object type implemented through an extension is supported"() {
def spec = '''
type Query {
base: Base
}

interface Pet {
id: ID
}

type Dog {
id: ID
}

extend type Dog implements Pet

interface PetContainer {
pets: [Pet]!
}

type Base implements PetContainer {
pets: [Dog!]!
}
'''

def result = check(spec, ["Pet", "PetContainer"])

expect:
result.isEmpty()
}

def "unrelated type remains invalid when other interface relationships use extensions"() {
def spec = '''
type Query {
base: Base
}

interface Pet {
id: ID
}

interface Vehicle {
id: ID
}

type Car {
id: ID
}

extend type Car implements Vehicle

interface PetContainer {
pet: Pet
}

type Base implements PetContainer {
pet: Car
}
'''

def result = check(spec, ["Pet", "Vehicle", "PetContainer"])

expect:
errorContaining(result, "has tried to redefine field 'pet' defined via interface 'PetContainer'")
}

def "deviant covariant object types are detected"() {

def spec = '''
Expand Down
89 changes: 89 additions & 0 deletions src/test/groovy/graphql/schema/idl/TypeDefinitionRegistryTest.groovy
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,87 @@ class TypeDefinitionRegistryTest extends Specification {

'''

def extensionRelationships = '''
interface Pet {
id: ID
}

interface WorkingPet {
id: ID
}

extend interface WorkingPet implements Pet

type Dog {
id: ID
}

extend type Dog implements Pet

type Cat {
id: ID
}

union Pets = Cat

extend union Pets = Dog
'''

def "possible type detection includes interface implementations from extensions for #typeOfReg registry"() {
when:
def registry = registry(extensionRelationships, typeOfReg)

then:
registry.isPossibleType(type("Pet"), type("Dog"))
registry.isPossibleType(type("Pet"), type("WorkingPet"))
!registry.isPossibleType(type("Pet"), type("Cat"))

where:
typeOfReg << ["mutable", "immutable"]
}

def "possible type detection includes union members from extensions for #typeOfReg registry"() {
when:
def registry = registry(extensionRelationships, typeOfReg)

then:
registry.isPossibleType(type("Pets"), type("Cat"))
registry.isPossibleType(type("Pets"), type("Dog"))
!registry.isPossibleType(type("Pets"), type("WorkingPet"))

where:
typeOfReg << ["mutable", "immutable"]
}

def "subtype detection unwraps types implemented through extensions for #typeOfReg registry"() {
when:
def registry = registry(extensionRelationships, typeOfReg)

then:
registry.isSubTypeOf(nonNullType("Dog"), type("Pet"))
registry.isSubTypeOf(listType(type("Dog")), listType(type("Pet")))
registry.isSubTypeOf(
listType(nonNullType(listType(type("Dog")))),
listType(nonNullType(listType(type("Pet")))))
!registry.isSubTypeOf(type("Cat"), type("Pet"))

where:
typeOfReg << ["mutable", "immutable"]
}

def "implementation lookup includes relationships from extensions for #typeOfReg registry"() {
when:
def registry = registry(extensionRelationships, typeOfReg)
def pet = registry.getTypeOrNull("Pet", InterfaceTypeDefinition.class)

then:
registry.getAllImplementationsOf(pet)*.name == ["WorkingPet", "Dog"]
registry.getImplementationsOf(pet)*.name == ["Dog"]

where:
typeOfReg << ["mutable", "immutable"]
}

def "test possible type detection #typeOfReg"() {
given:
TypeDefinitionRegistry mutableReg = parse(animalia)
Expand Down Expand Up @@ -506,6 +587,14 @@ class TypeDefinitionRegistryTest extends Specification {
"immutable" | _
}

private static TypeDefinitionRegistry registry(String spec, String typeOfRegistry) {
def registry = parse(spec)
if (typeOfRegistry == "immutable") {
return registry.readOnly()
}
return registry
}


def "isSubTypeOf detection #typeOfReg"() {
when:
Expand Down
Loading
Back | FazBrowse Home | New Git URL