Merge each SDL type with its extensions once

AGENTS.md

@@ -5,9 +5,10 @@ resolver classes. Published to Maven Central. Kotlin, with a few Java files, tar
 
 ## Layout
 
-- `src/main/kotlin/graphql/kickstart/tools`: `SchemaParser` and its builder and options, `SchemaClassScanner` (matches schema
-  types to classes), `resolver/` (field resolvers and data fetchers), `directive/`, `relay/`, `proxy/` (finds the class
-  behind a Spring, Guice, Weld or Javassist proxied resolver), `util/`
+- `src/main/kotlin/graphql/kickstart/tools`: `SchemaParser` and its builder and options, `MergedDefinitions.kt` (merges each
+  SDL type with its extensions into a `MergedType`. Other code, apart from the Relay factory, reads SDL types only through
+  it), `SchemaClassScanner` (matches schema types to classes), `resolver/` (field resolvers and data fetchers), `directive/`,
+  `relay/`, `proxy/` (finds the class behind a Spring, Guice, Weld or Javassist proxied resolver), `util/`
 - `src/test/kotlin`: JUnit tests, one class per feature. A test for an issue goes into the class for its feature.
   `EndToEndTest` and `EndToEndSpecHelper` hold the shared schema and resolvers
 - `.claude/skills/verify-tools`: the probe that checks a change from a consuming app's side
@@ -94,7 +95,8 @@ happening. "docs only" means nothing does yet, so the rule depends on you readin
 | A resolver method's parameter layout, type variables and source are decided once, at schema build | docs only |
 | A change to how resolvers are found, bound or unwrapped keeps what a method, a property and a Map field return | `ResolverCompatibilityTest` and the bounded check in `ReturnValueUnwrappingTest`. Each lists exactly today's property and Map gaps as known findings, and fails when one is fixed until it's removed, or when a new failure would hide under one |
 | One output has one builder. Don't add a second path that builds the same thing | docs only |
-| Read an SDL type's members merged with its extensions, never from the raw definition | docs only |
+| Read an SDL type's members merged with its extensions, never from the raw definition | `MergedType` in `MergedDefinitions.kt` and `DefinitionsBoundaryTest`, which fails on a raw member, directive or child-node read, an extension class, an SDL node walk or a `TypeDefinitionRegistry` call outside it. A read by reflection gets past it |
+| Every kind of SDL type starts its graphql-java type from the same recipe | `MergedType.prepare`, `DefinitionsBoundaryTest` and `TypeKindRecipeTest` |
 | Reuse an existing structure before you add a parallel one | docs only |
 | Argument conversion goes through one entry point | docs only |
 | An input field's Java type comes from the mapping Jackson uses, not from guessed getters | docs only |

pom.xml

@@ -85,6 +85,11 @@
             </dependency>
 
             <!-- Test -->
+            <dependency>
+                <groupId>com.tngtech.archunit</groupId>
+                <artifactId>archunit</artifactId>
+                <version>1.5.1</version>
+            </dependency>
             <dependency>
                 <groupId>ch.qos.logback</groupId>
                 <artifactId>logback-classic</artifactId>
@@ -181,6 +186,11 @@
         </dependency>
 
         <!-- Test  -->
+        <dependency>
+            <groupId>com.tngtech.archunit</groupId>
+            <artifactId>archunit</artifactId>
+            <scope>test</scope>
+        </dependency>
         <dependency>
             <groupId>ch.qos.logback</groupId>
             <artifactId>logback-classic</artifactId>
@@ -261,6 +271,11 @@
                         <goals>
                             <goal>test-jar</goal>
                         </goals>
+                        <configuration>
+                            <excludes>
+                                <exclude>logback-test.xml</exclude>
+                            </excludes>
+                        </configuration>
                     </execution>
                 </executions>
             </plugin>

src/main/kotlin/graphql/kickstart/tools/DictionaryTypeResolver.kt

@@ -3,7 +3,6 @@ package graphql.kickstart.tools
 import graphql.TypeResolutionEnvironment
 import graphql.kickstart.tools.util.JavaType
 import graphql.kickstart.tools.util.unwrap
-import graphql.language.TypeDefinition
 import graphql.schema.GraphQLInterfaceType
 import graphql.schema.GraphQLObjectType
 import graphql.schema.GraphQLUnionType
@@ -14,7 +13,7 @@ import java.lang.reflect.ParameterizedType
  * @author Andrew Potter
  */
 internal abstract class DictionaryTypeResolver(
-        private val dictionary: Map<JavaType, TypeDefinition<*>>
+        private val dictionary: Map<JavaType, MergedType<*>>
 ) : TypeResolver {
     // Raw class of each parameterized entry, e.g. Box for Box<String>, unless several entries share it
     private val rawClassDictionary by lazy {
@@ -25,7 +24,7 @@ internal abstract class DictionaryTypeResolver(
             .toMap()
     }
 
-    private fun <T> getTypeDefinition(clazz: Class<T>): TypeDefinition<*>? {
+    private fun <T> getTypeDefinition(clazz: Class<T>): MergedType<*>? {
         return dictionary[clazz]
             ?: (if (clazz.superclass == null) null else getTypeDefinition(clazz.superclass))
             ?: clazz.interfaces.mapNotNull { getTypeDefinition(it) }.firstOrNull()
@@ -45,7 +44,7 @@ internal abstract class DictionaryTypeResolver(
 }
 
 internal class InterfaceTypeResolver(
-    dictionary: Map<JavaType, TypeDefinition<*>>,
+    dictionary: Map<JavaType, MergedType<*>>,
     private val thisInterface: GraphQLInterfaceType
 ) : DictionaryTypeResolver(
         dictionary
@@ -54,7 +53,7 @@ internal class InterfaceTypeResolver(
 }
 
 internal class UnionTypeResolver(
-    dictionary: Map<JavaType, TypeDefinition<*>>,
+    dictionary: Map<JavaType, MergedType<*>>,
     private val thisUnion: GraphQLUnionType
 ) : DictionaryTypeResolver(
         dictionary

src/main/kotlin/graphql/kickstart/tools/MergedDefinitions.kt

@@ -0,0 +1,229 @@
+package graphql.kickstart.tools
+
+import graphql.Directives
+import graphql.introspection.Introspection.DirectiveLocation
+import graphql.kickstart.tools.util.describe
+import graphql.kickstart.tools.util.getDocumentation
+import graphql.kickstart.tools.util.unwrap
+import graphql.language.*
+import graphql.schema.*
+import graphql.schema.idl.ScalarInfo
+
+internal class MergedDefinitions(definitions: List<Definition<*>>) {
+    val directiveDefinitions: List<DirectiveDefinition> = definitions.filterIsInstance<DirectiveDefinition>()
+    val rootInfo: RootTypeInfo = RootTypeInfo.fromSchemaDefinitions(definitions.filterIsInstance<SchemaDefinition>())
+
+    val scalarNames: Set<String> = ScalarInfo.GRAPHQL_SPECIFICATION_SCALARS_DEFINITIONS.keys + definitions.filterIsInstance<ScalarTypeDefinition>().map { it.name }
+
+    private val typeDefinitions = definitions.filterIsInstance<TypeDefinition<*>>()
+    private val baseTypeNames = typeDefinitions.filter { it !is SDLExtensionDefinition }.map { it.name }.toSet()
+
+    private val loneExtensions = typeDefinitions
+        .filter { it is SDLExtensionDefinition && it !is ObjectTypeDefinition && it !is InputObjectTypeDefinition && it.name !in baseTypeNames }
+        .distinctBy { it.name }
+        .toSet()
+
+    private val sources = typeDefinitions.filter { it !is SDLExtensionDefinition || it in loneExtensions }
+
+    private val sourcesByName = sources.associateBy { it.name }
+
+    private val extensionsByName = typeDefinitions
+        .filter { it is SDLExtensionDefinition && it !in loneExtensions && !extendsOtherKind(it) }
+        .groupBy { it.name }
+
+    private val typesBySource: List<Pair<TypeDefinition<*>, MergedType<*>>> = sources.mapNotNull { source -> merge(source)?.let { source to it } }
+
+    private val specificationScalars: List<Pair<ScalarTypeDefinition, MergedType.Scalar>> = ScalarInfo.GRAPHQL_SPECIFICATION_SCALARS_DEFINITIONS.values.map { definition ->
+        definition to MergedType.Scalar(definition, extensionsByName[definition.name].orEmpty().filterIsInstance<ScalarTypeExtensionDefinition>())
+    }
+
+    private val specificationScalarsByName: Map<String, MergedType.Scalar> = specificationScalars.associate { (_, type) -> type.name to type }
+
+    private val sourceOf: Map<MergedType<*>, TypeDefinition<*>> = (typesBySource + specificationScalars).associate { (source, type) -> type to source }
+
+    val byName: Map<String, MergedType<*>> = typesBySource.associate { (_, type) -> type.name to type }
+
+    val objectsWithRepeats: List<MergedType.Object> = typesBySource.map { it.second }.filterIsInstance<MergedType.Object>()
+
+    val ignoredExtensionWarnings: List<String> = typeDefinitions.filter(::extendsOtherKind).map { extension ->
+        "Type '${extension.name}' is defined as ${sourcesByName.getValue(extension.name).kind()}, but extended as ${extension.kind()}. The extension is ignored."
+    }
+
+    fun resolve(name: String?): MergedType<*>? = specificationScalarsByName[name] ?: byName[name]
+
+    fun definitionsOf(types: Set<MergedType<*>>): Set<TypeDefinition<*>> = types.mapNotNullTo(linkedSetOf()) { sourceOf[it] }
+
+    fun findUsage(reference: TypeName, types: Collection<MergedType<*>>): String? =
+        (types.mapNotNull { sourceOf[it] } + directiveDefinitions + extensionsByName.values.flatten()).asSequence().flatMap { definition ->
+            when (definition) {
+                is ObjectTypeDefinition -> fieldUsages(definition.name, definition.fieldDefinitions)
+                is InterfaceTypeDefinition -> fieldUsages(definition.name, definition.fieldDefinitions)
+                is InputObjectTypeDefinition -> definition.inputValueDefinitions.map { "input field '${definition.name}.${it.name}'" to it.type }
+                is DirectiveDefinition -> definition.inputValueDefinitions.map { "argument '${it.name}' of directive '@${definition.name}'" to it.type }
+                else -> emptyList()
+            }
+        }.find { (_, type) -> type.unwrap() === reference }?.first
+
+    private fun fieldUsages(typeName: String, fields: List<FieldDefinition>): List<Pair<String, Type<*>>> = fields.flatMap { field ->
+        listOf("field '$typeName.${field.name}'" to field.type) +
+            field.inputValueDefinitions.map { "argument '${it.name}' of field '$typeName.${field.name}'" to it.type }
+    }
+
+    private fun merge(source: TypeDefinition<*>): MergedType<*>? {
+        val extensions = extensionsByName[source.name].orEmpty()
+        return when (source) {
+            is ObjectTypeDefinition -> MergedType.Object(source, extensions.filterIsInstance<ObjectTypeExtensionDefinition>())
+            is InterfaceTypeDefinition -> MergedType.Interface(source, extensions.filterIsInstance<InterfaceTypeExtensionDefinition>())
+            is UnionTypeDefinition -> MergedType.Union(source, extensions.filterIsInstance<UnionTypeExtensionDefinition>())
+            is EnumTypeDefinition -> MergedType.Enum(source, extensions.filterIsInstance<EnumTypeExtensionDefinition>())
+            is InputObjectTypeDefinition -> MergedType.Input(source, extensions.filterIsInstance<InputObjectTypeExtensionDefinition>())
+            is ScalarTypeDefinition -> MergedType.Scalar(source, extensions.filterIsInstance<ScalarTypeExtensionDefinition>())
+            else -> null
+        }
+    }
+
+    private fun extendsOtherKind(definition: TypeDefinition<*>): Boolean {
+        val type = sourcesByName[definition.name]
+        return definition is SDLExtensionDefinition && type != null && type.kind() != definition.kind()
+    }
+}
+
+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
+}
+
+internal class TypeBuildContext(
+    val options: SchemaParserOptions,
+    val directives: SdlDirectives,
+    val customScalars: CustomScalarMap,
+)
+
+internal sealed class MergedType<B : GraphqlDirectivesContainerTypeBuilder<B, B>>(
+    private val definition: TypeDefinition<*>,
+    extensions: List<TypeDefinition<*>>,
+) {
+    val name: String get() = definition.name
+
+    val definitionClassName: String get() = definition.javaClass.simpleName
+
+    val kindPhrase: String get() = definition.kind()
+
+    protected val directives: List<Directive> = definition.directives + extensions.flatMap { it.directives }
+
+    protected abstract val location: DirectiveLocation
+
+    protected abstract val declarations: List<NamedNode<*>>
+
+    protected abstract fun newBuilder(context: TypeBuildContext): B
+
+    fun prepare(context: TypeBuildContext): B {
+        rejectRepeated()
+        val builder = newBuilder(context).name(name)
+        getDocumentation(definition as AbstractNode<*>, context.options)?.let { builder.description(it) }
+        return with(context.directives) { builder.withSdlDirectives(directives, location) }
+    }
+
+    private fun rejectRepeated() {
+        val noun = when (this) {
+            is Object, is Interface, is Input -> "field"
+            is Enum -> "value"
+            is Union -> "member"
+            is Scalar -> return
+        }
+        val names = mutableSetOf<String?>()
+        val repeat = declarations.firstOrNull { !names.add(it.name) } ?: return
+        throw SchemaError("Type '$name' declares $noun '${repeat.name}' more than once${repeat.sourceLocation.describe()}.")
+    }
+
+    sealed interface Implementing {
+        val fields: List<FieldDefinition>
+        val interfaces: List<Type<*>>
+    }
+
+    class Object(
+        private val definition: ObjectTypeDefinition,
+        private val extensions: List<ObjectTypeExtensionDefinition>,
+    ) : MergedType<GraphQLObjectType.Builder>(definition, extensions), Implementing {
+        override val fields: List<FieldDefinition> = definition.fieldDefinitions + extensions.flatMap { it.fieldDefinitions }
+        override val interfaces: List<Type<*>> = definition.implements + extensions.flatMap { it.implements }
+        override val location = DirectiveLocation.OBJECT
+        override val declarations get() = fields
+        override fun newBuilder(context: TypeBuildContext): GraphQLObjectType.Builder =
+            GraphQLObjectType.newObject().definition(definition).extensionDefinitions(extensions)
+    }
+
+    class Interface(
+        private val definition: InterfaceTypeDefinition,
+        private val extensions: List<InterfaceTypeExtensionDefinition>,
+    ) : MergedType<GraphQLInterfaceType.Builder>(definition, extensions), Implementing {
+        override val fields: List<FieldDefinition> = definition.fieldDefinitions + extensions.flatMap { it.fieldDefinitions }
+        override val interfaces: List<Type<*>> = definition.implements + extensions.flatMap { it.implements }
+        override val location = DirectiveLocation.INTERFACE
+        override val declarations get() = fields
+        override fun newBuilder(context: TypeBuildContext): GraphQLInterfaceType.Builder =
+            GraphQLInterfaceType.newInterface().definition(definition).extensionDefinitions(extensions)
+    }
+
+    class Union(
+        private val definition: UnionTypeDefinition,
+        private val extensions: List<UnionTypeExtensionDefinition>,
+    ) : MergedType<GraphQLUnionType.Builder>(definition, extensions) {
+        val members: List<Type<*>> = definition.memberTypes + extensions.flatMap { it.memberTypes }
+        override val location = DirectiveLocation.UNION
+        override val declarations get() = members.map { it as TypeName }
+        override fun newBuilder(context: TypeBuildContext): GraphQLUnionType.Builder =
+            GraphQLUnionType.newUnionType().definition(definition).extensionDefinitions(extensions)
+
+        fun declarationTextOf(member: Type<*>): String =
+            (listOf(definition) + extensions).first { declaration -> declaration.memberTypes.any { it === member } }.toString()
+    }
+
+    class Enum(
+        private val definition: EnumTypeDefinition,
+        private val extensions: List<EnumTypeExtensionDefinition>,
+    ) : MergedType<GraphQLEnumType.Builder>(definition, extensions) {
+        val values: List<EnumValueDefinition> = definition.enumValueDefinitions + extensions.flatMap { it.enumValueDefinitions }
+        override val location = DirectiveLocation.ENUM
+        override val declarations get() = values
+        override fun newBuilder(context: TypeBuildContext): GraphQLEnumType.Builder =
+            GraphQLEnumType.newEnum().definition(definition).extensionDefinitions(extensions)
+    }
+
+    class Input(
+        private val definition: InputObjectTypeDefinition,
+        private val extensions: List<InputObjectTypeExtensionDefinition>,
+    ) : MergedType<GraphQLInputObjectType.Builder>(definition, extensions) {
+        val fields: List<InputValueDefinition> = definition.inputValueDefinitions + extensions.flatMap { it.inputValueDefinitions }
+        override val location = DirectiveLocation.INPUT_OBJECT
+        override val declarations get() = fields
+        override fun newBuilder(context: TypeBuildContext): GraphQLInputObjectType.Builder =
+            GraphQLInputObjectType.newInputObject().definition(definition).extensionDefinitions(extensions)
+    }
+
+    class Scalar(
+        private val definition: ScalarTypeDefinition,
+        private val extensions: List<ScalarTypeExtensionDefinition>,
+    ) : MergedType<GraphQLScalarType.Builder>(definition, extensions) {
+        override val location = DirectiveLocation.SCALAR
+        override val declarations = emptyList<NamedNode<*>>()
+
+        // like graphql-java, only the schema's directives are applied
+        override fun newBuilder(context: TypeBuildContext): GraphQLScalarType.Builder {
+            val provided = context.customScalars.getValue(name)
+            return GraphQLScalarType.newScalar(provided)
+                .definition(definition)
+                .extensionDefinitions(extensions)
+                .specifiedByUrl(specifiedByUrl() ?: provided.specifiedByUrl)
+                .clearDirectives()
+        }
+
+        private fun specifiedByUrl(): String? =
+            (directives.find { it.name == Directives.SpecifiedByDirective.name }?.getArgument("url")?.value as? StringValue)?.value
+    }
+}