Skip to content

Merge type extensions into the types they extend #855

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

Open
oryan-block wants to merge 1 commit into
base: master
Choose a base branch
from
+1,052 −26
Open

Merge type extensions into the types they extend #855

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
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/main/kotlin/graphql/kickstart/tools/ScannedSchemaObjects.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import graphql.schema.GraphQLScalarType
internal data class ScannedSchemaObjects(
val dictionary: TypeClassDictionary,
val definitions: Set<SDLNamedDefinition<*>>,
val typeExtensions: TypeExtensions,
val customScalars: CustomScalarMap,
val rootInfo: RootTypeInfo,
val fieldResolversByType: Map<ObjectTypeDefinition, MutableMap<FieldDefinition, FieldResolver>>,
Expand All @@ -23,3 +24,5 @@ internal data class ScannedSchemaObjects(

internal typealias TypeClassDictionary = BiMap<TypeDefinition<*>, JavaType>
internal typealias CustomScalarMap = Map<String, GraphQLScalarType>
// the extensions of each type by type name, in the order they're declared
internal typealias TypeExtensions = Map<String, List<TypeDefinition<*>>>
53 changes: 44 additions & 9 deletions src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,21 @@ internal class SchemaClassScanner(
private val directiveDefinitions = allDefinitions.filterIsInstance<DirectiveDefinition>()
private val scalarDefinitions = allDefinitions.filterIsInstance<ScalarTypeDefinition>()

private val definitionsByName = (allDefinitions.filterIsInstance<TypeDefinition<*>>() - extensionDefinitions - inputExtensionDefinitions).associateBy { it.name }
private val typeDefinitions = allDefinitions.filterIsInstance<TypeDefinition<*>>()
private val baseTypeNames = typeDefinitions.filter { it !is SDLExtensionDefinition }.map { it.name }.toSet()

// The spec doesn't allow extending a type that isn't defined. Union, enum, interface and scalar extensions are subclasses of
// their definitions though, so one without a definition used to be built as the type itself. To keep those schemas working the
// first one still is, and the others extend it. Object and input extensions without a definition are still ignored.
private val loneExtensions = typeDefinitions
.filter { it is SDLExtensionDefinition && it !is ObjectTypeDefinition && it !is InputObjectTypeDefinition && it.name !in baseTypeNames }
.distinctBy { it.name }

private val definitionsByName = typeDefinitions.filter { it !is SDLExtensionDefinition || it in loneExtensions }.associateBy { it.name }
private val typeExtensions: TypeExtensions = typeDefinitions.filter { it is SDLExtensionDefinition && it !in loneExtensions }.groupBy { it.name }
private val objectDefinitions = (allDefinitions.filterIsInstance<ObjectTypeDefinition>() - extensionDefinitions)
private val objectDefinitionsByName = objectDefinitions.associateBy { it.name }
private val interfaceDefinitionsByName = allDefinitions.filterIsInstance<InterfaceTypeDefinition>().associateBy { it.name }
private val interfaceDefinitionsByName = definitionsByName.values.filterIsInstance<InterfaceTypeDefinition>().associateBy { it.name }

private val fieldResolverScanner = FieldResolverScanner(options)
private val typeClassMatcher = TypeClassMatcher(definitionsByName)
Expand All @@ -56,6 +67,13 @@ internal class SchemaClassScanner(
}
}

typeExtensions.values.flatten().forEach { extension ->
val definition = definitionsByName[extension.name]
if (definition != null && definition.kind() != extension.kind()) {
throw SchemaClassScannerError("Type '${extension.name}' is defined as ${definition.kind()}, but extended as ${extension.kind()}.")
}
}

if (options.allowUnimplementedResolvers) {
log.warn("Option 'allowUnimplementedResolvers' should only be set to true during development, as it can cause schema errors to be moved to query time instead of schema creation time. Make sure this is turned off in production.")
}
Expand Down Expand Up @@ -222,6 +240,7 @@ internal class SchemaClassScanner(
.description(getDocumentation(definition, options) ?: provided.description)
.coercing(provided.coercing)
.definition(definition)
.extensionDefinitions(typeExtensions.extensionsOf<ScalarTypeExtensionDefinition>(definition.name))
.build()
}.associateBy { it.name }

Expand All @@ -246,7 +265,7 @@ internal class SchemaClassScanner(

val definitions = observedDefinitions + extensionDefinitions + inputExtensionDefinitions + directiveDefinitions

return ScannedSchemaObjects(dictionary, definitions, scalars, rootInfo, fieldResolversByType.toMap(), unusedDefinitions)
return ScannedSchemaObjects(dictionary, definitions, typeExtensions, scalars, rootInfo, fieldResolversByType.toMap(), unusedDefinitions)
}

private fun validateRootResolversWereUsed(rootType: RootType?, fieldResolvers: List<FieldResolver>) {
Expand All @@ -264,16 +283,18 @@ internal class SchemaClassScanner(

private fun getAllObjectTypesImplementingDiscoveredInterfaces(): List<ObjectTypeDefinition> {
return dictionary.keys.filterIsInstance<InterfaceTypeDefinition>().map { iface ->
objectDefinitions.filter { obj -> obj.implements.filterIsInstance<TypeName>().any { it.name == iface.name } }
objectDefinitions.filter { obj -> obj.getExtendedImplements().filterIsInstance<TypeName>().any { it.name == iface.name } }
}.flatten().distinctBy { it.name }
}

private fun getAllObjectTypeMembersOfDiscoveredUnions(): List<ObjectTypeDefinition> {
val unionTypeNames = dictionary.keys.filterIsInstance<UnionTypeDefinition>().map { union -> union.name }.toSet()
return dictionary.keys.filterIsInstance<UnionTypeDefinition>().map { union ->
union.memberTypes.filterIsInstance<TypeName>().filter { !unionTypeNames.contains(it.name) }.map {
objectDefinitionsByName[it.name]
?: throw SchemaClassScannerError("No object type found with name '${it.name}' for union: $union")
(listOf(union) + typeExtensions.extensionsOf<UnionTypeExtensionDefinition>(union.name)).flatMap { definition ->
definition.memberTypes.filterIsInstance<TypeName>().filter { !unionTypeNames.contains(it.name) }.map {
objectDefinitionsByName[it.name]
?: throw SchemaClassScannerError("No object type found with name '${it.name}' for union: $definition")
}
}
}.flatten().distinct()
}
Expand All @@ -284,7 +305,8 @@ internal class SchemaClassScanner(
*/
private fun handleInterfaceFieldTypes(): Boolean {
val newTypes = dictionary.keys.filterIsInstance<InterfaceTypeDefinition>().flatMap { iface ->
iface.fieldDefinitions.mapNotNull { field ->
val fieldDefinitions = iface.fieldDefinitions + typeExtensions.extensionsOf<InterfaceTypeExtensionDefinition>(iface.name).flatMap { it.fieldDefinitions }
fieldDefinitions.mapNotNull { field ->
val type = definitionsByName[(field.type.unwrap() as TypeName).name]
if ((type is UnionTypeDefinition || type is InterfaceTypeDefinition) && !dictionary.containsKey(type)) {
type to FieldTypeReference("${iface.name}.${field.name}")
Expand Down Expand Up @@ -441,14 +463,27 @@ internal class SchemaClassScanner(
}

private fun scanInterfacesOfType(graphQLType: ObjectTypeDefinition) {
graphQLType.implements.forEach {
graphQLType.getExtendedImplements().forEach {
if (it is TypeName) {
handleFoundType(interfaceDefinitionsByName[it.name]
?: throw SchemaClassScannerError("Object type ${graphQLType.name} declared interface ${it.name}, but no interface with that name was found in the schema!"), null, InterfaceReference(graphQLType))
}
}
}

private fun ObjectTypeDefinition.getExtendedImplements(): List<Type<*>> =
implements + typeExtensions.extensionsOf<ObjectTypeExtensionDefinition>(name).flatMap { it.implements }

private fun TypeDefinition<*>.kind(): String = when (this) {
is ObjectTypeDefinition -> "an object type"
is InterfaceTypeDefinition -> "an interface"
is UnionTypeDefinition -> "a union"
is EnumTypeDefinition -> "an enum"
is InputObjectTypeDefinition -> "an input object type"
is ScalarTypeDefinition -> "a scalar"
else -> javaClass.simpleName
}

private fun enqueue(graphQLType: ObjectTypeDefinition, javaType: JavaType) {
queue.add(QueueItem(graphQLType, javaType))
}
Expand Down
71 changes: 54 additions & 17 deletions src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import graphql.Scalars
import graphql.introspection.Introspection
import graphql.introspection.Introspection.DirectiveLocation.INPUT_FIELD_DEFINITION
import graphql.kickstart.tools.directive.DirectiveWiringHelper
import graphql.kickstart.tools.util.extensionsOf
import graphql.kickstart.tools.util.getDocumentation
import graphql.kickstart.tools.util.getExtendedFieldDefinitions
import graphql.kickstart.tools.util.unwrap
Expand Down Expand Up @@ -35,6 +36,7 @@ class SchemaParser internal constructor(

private val dictionary = scanResult.dictionary
private val definitions = scanResult.definitions
private val typeExtensions = scanResult.typeExtensions
private val customScalars = scanResult.customScalars
private val rootInfo = scanResult.rootInfo
private val fieldResolversByType = scanResult.fieldResolversByType
Expand Down Expand Up @@ -124,21 +126,24 @@ class SchemaParser internal constructor(

private fun createObject(objectDefinition: ObjectTypeDefinition, interfaces: List<GraphQLInterfaceType>, inputObjects: List<GraphQLInputObjectType>): GraphQLObjectType {
val name = objectDefinition.name
val extensions = typeExtensions.extensionsOf<ObjectTypeExtensionDefinition>(name)
val directives = directivesOf(objectDefinition, extensions)
val builder = GraphQLObjectType.newObject()
.name(name)
.definition(objectDefinition)
.extensionDefinitions(extensions)
.description(getDocumentation(objectDefinition, options))
.withAppliedDirectives(*buildAppliedDirectives(objectDefinition.directives))
.withDirectives(*buildDirectives(objectDefinition.directives, Introspection.DirectiveLocation.OBJECT))
.withAppliedDirectives(*buildAppliedDirectives(directives))
.withDirectives(*buildDirectives(directives, Introspection.DirectiveLocation.OBJECT))
.apply {
objectDefinition.implements.forEach { implementsDefinition ->
(objectDefinition.implements + extensions.flatMap { it.implements }).forEach { implementsDefinition ->
val interfaceName = (implementsDefinition as TypeName).name
withInterface(interfaces.find { it.name == interfaceName }
?: throw SchemaError("Expected interface type with name '$interfaceName' but found none!"))
}
}
.apply {
objectDefinition.getExtendedFieldDefinitions(extensionDefinitions).forEach { fieldDefinition ->
objectDefinition.getExtendedFieldDefinitions(extensions).forEach { fieldDefinition ->
field { field ->
createField(field, fieldDefinition, inputObjects)
codeRegistryBuilder.dataFetcher(
Expand All @@ -160,7 +165,8 @@ class SchemaParser internal constructor(

private fun createInputObject(definition: InputObjectTypeDefinition, inputObjects: List<GraphQLInputObjectType>,
referencingInputObjects: MutableSet<String>): GraphQLInputObjectType {
val extensionDefinitions = inputExtensionDefinitions.filter { it.name == definition.name }
val extensionDefinitions = typeExtensions.extensionsOf<InputObjectTypeExtensionDefinition>(definition.name)
val directives = directivesOf(definition, extensionDefinitions)

referencingInputObjects.add(definition.name)

Expand All @@ -169,9 +175,10 @@ class SchemaParser internal constructor(
.definition(definition)
.extensionDefinitions(extensionDefinitions)
.description(getDocumentation(definition, options))
.withAppliedDirectives(*buildAppliedDirectives(definition.directives))
.withDirectives(*buildDirectives(definition.directives, Introspection.DirectiveLocation.INPUT_OBJECT))
.withAppliedDirectives(*buildAppliedDirectives(directives))
.withDirectives(*buildDirectives(directives, Introspection.DirectiveLocation.INPUT_OBJECT))
.apply {
// unlike graphql-java the fields of the extensions come first, which is kept so the order of the fields doesn't change
(extensionDefinitions + definition).forEach { typeDefinition ->
typeDefinition.inputValueDefinitions.forEach { fieldDefinition ->
field(
Expand Down Expand Up @@ -199,14 +206,19 @@ class SchemaParser internal constructor(
?: throw SchemaError("Expected enum with name '$name' but found none!")
if (!type.unwrap().isEnum) throw SchemaError("Type '$name' is declared as an enum in the GraphQL schema but is not a Java enum!")

val extensions = typeExtensions.extensionsOf<EnumTypeExtensionDefinition>(name)
val directives = directivesOf(definition, extensions)
val builder = GraphQLEnumType.newEnum()
.name(name)
.definition(definition)
.extensionDefinitions(extensions)
.description(getDocumentation(definition, options))
.withAppliedDirectives(*buildAppliedDirectives(definition.directives))
.withDirectives(*buildDirectives(definition.directives, Introspection.DirectiveLocation.ENUM))
.withAppliedDirectives(*buildAppliedDirectives(directives))
.withDirectives(*buildDirectives(directives, Introspection.DirectiveLocation.ENUM))
.apply {
definition.enumValueDefinitions.forEach { valueDefinition ->
// graphql-java rejects a value that an extension repeats. Here it's kept once, with its first definition, so schemas
// that repeated the values of the enum in an extension to work around #552 keep building.
(definition.enumValueDefinitions + extensions.flatMap { it.enumValueDefinitions }).distinctBy { it.name }.forEach { valueDefinition ->
val enumName = valueDefinition.name
val enumValue = type.unwrap().enumConstants.find { (it as Enum<*>).name == enumName }
?: throw SchemaError("Expected value for name '$enumName' in enum '${type.unwrap().simpleName}' but found none!")
Expand All @@ -229,19 +241,24 @@ class SchemaParser internal constructor(
}

private fun createInterfaceObject(interfaceDefinition: InterfaceTypeDefinition, inputObjects: List<GraphQLInputObjectType>): GraphQLInterfaceType {
val extensions = typeExtensions.extensionsOf<InterfaceTypeExtensionDefinition>(interfaceDefinition.name)
val directives = directivesOf(interfaceDefinition, extensions)
val builder = GraphQLInterfaceType.newInterface()
.name(interfaceDefinition.name)
.definition(interfaceDefinition)
.extensionDefinitions(extensions)
.description(getDocumentation(interfaceDefinition, options))
.withAppliedDirectives(*buildAppliedDirectives(interfaceDefinition.directives))
.withDirectives(*buildDirectives(interfaceDefinition.directives, Introspection.DirectiveLocation.INTERFACE))
.withAppliedDirectives(*buildAppliedDirectives(directives))
.withDirectives(*buildDirectives(directives, Introspection.DirectiveLocation.INTERFACE))
.apply {
interfaceDefinition.fieldDefinitions.forEach { fieldDefinition ->
// graphql-java rejects a field that an extension repeats. Here it's kept once, with its first definition, so schemas
// that repeated the fields of the interface in an extension to work around #552 keep building.
(interfaceDefinition.fieldDefinitions + extensions.flatMap { it.fieldDefinitions }).distinctBy { it.name }.forEach { fieldDefinition ->
field { field -> createField(field, fieldDefinition, inputObjects) }
}
}
.apply {
interfaceDefinition.implements.forEach { implementsDefinition ->
(interfaceDefinition.implements + extensions.flatMap { it.implements }).forEach { implementsDefinition ->
val interfaceName = (implementsDefinition as TypeName).name
withInterface(GraphQLTypeReference(interfaceName))
}
Expand All @@ -251,12 +268,15 @@ class SchemaParser internal constructor(
}

private fun createUnionObject(definition: UnionTypeDefinition, types: List<GraphQLObjectType>): GraphQLUnionType {
val extensions = typeExtensions.extensionsOf<UnionTypeExtensionDefinition>(definition.name)
val directives = directivesOf(definition, extensions)
val builder = GraphQLUnionType.newUnionType()
.name(definition.name)
.definition(definition)
.extensionDefinitions(extensions)
.description(getDocumentation(definition, options))
.withAppliedDirectives(*buildAppliedDirectives(definition.directives))
.withDirectives(*buildDirectives(definition.directives, Introspection.DirectiveLocation.UNION))
.withAppliedDirectives(*buildAppliedDirectives(directives))
.withDirectives(*buildDirectives(directives, Introspection.DirectiveLocation.UNION))
.apply {
getLeafUnionObjects(definition, types).forEach { possibleType(it) }
}
Expand All @@ -268,7 +288,7 @@ class SchemaParser internal constructor(
val name = definition.name
val leafObjects = mutableListOf<GraphQLObjectType>()

definition.memberTypes.forEach {
(definition.memberTypes + typeExtensions.extensionsOf<UnionTypeExtensionDefinition>(name).flatMap { it.memberTypes }).forEach {
val typeName = (it as TypeName).name

// Is this a nested union? If so, expand
Expand All @@ -284,6 +304,23 @@ class SchemaParser internal constructor(
return leafObjects
}

// Like graphql-java, the directives of the extensions follow those of the type. graphql-java rejects a non-repeatable directive
// that an extension applies again. Here the repeat is dropped and the first one kept, so those schemas still build, as they did
// when extension directives were ignored, and the applied directives agree with the legacy ones (see buildDirectives).
// Undeclared directives may be repeatable, so they're all kept.
private fun directivesOf(definition: TypeDefinition<*>, extensions: List<TypeDefinition<*>>): List<Directive> {
val applied = definition.directives.mapTo(mutableSetOf()) { it.name }
return definition.directives + extensions.flatMap { extension ->
val (repeated, kept) = extension.directives.partition { it.name in applied && isNonRepeatable(it.name) }
repeated.forEach { log.warn("Directive @${it.name} isn't repeatable but an extension of type '${definition.name}' applies it again, only the first one is kept.") }
kept.mapTo(applied) { it.name }
kept
}
}

private fun isNonRepeatable(directiveName: String): Boolean =
directiveDefinitions.find { it.name == directiveName }?.let { !it.isRepeatable } ?: BUILT_IN_DIRECTIVES.containsKey(directiveName)

private fun createField(field: GraphQLFieldDefinition.Builder, fieldDefinition: FieldDefinition, inputObjects: List<GraphQLInputObjectType>): GraphQLFieldDefinition.Builder {
return field
.name(fieldDefinition.name)
Expand Down
Loading
Loading