Merge branch 'master' into build/lock-kotlin

Author: oryan-block

.github/workflows/pull-request.yml

@@ -24,3 +24,23 @@ jobs:
           restore-keys: ${{ runner.os }}-m2
       - name: Build with Maven
         run: mvn --batch-mode --update-snapshots verify
+
+  # The same jar has to work with the oldest supported graphql-java, so build and test against it too
+  graphql-java-25:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v7
+      - name: Setup java
+        uses: actions/setup-java@v6
+        with:
+          java-version: '11'
+          distribution: 'temurin'
+      - name: Cache Maven packages
+        uses: actions/cache@v6
+        with:
+          path: ~/.m2
+          key: ${{ runner.os }}-m2-${{ hashFiles('**/pom.xml') }}
+          restore-keys: ${{ runner.os }}-m2
+      - name: Build with Maven
+        run: mvn --batch-mode --update-snapshots verify -Dgraphql-java.version=25.1

pom.xml

@@ -262,6 +262,11 @@
                 <artifactId>kotlin-maven-plugin</artifactId>
                 <groupId>org.jetbrains.kotlin</groupId>
                 <version>${kotlin.version}</version>
+                <configuration>
+                    <args>
+                        <arg>-Werror</arg>
+                    </args>
+                </configuration>
                 <executions>
                     <execution>
                         <id>compile</id>
@@ -286,8 +291,8 @@
                 <artifactId>maven-compiler-plugin</artifactId>
                 <version>3.16.0</version>
                 <configuration>
-                    <source>11</source>
-                    <target>11</target>
+                    <release>11</release>
+                    <failOnWarning>true</failOnWarning>
                 </configuration>
             </plugin>
 

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

@@ -3,6 +3,7 @@ package graphql.kickstart.tools
 import graphql.kickstart.tools.resolver.FieldResolver
 import graphql.kickstart.tools.util.BiMap
 import graphql.kickstart.tools.util.JavaType
+import graphql.language.EnumTypeDefinition
 import graphql.language.FieldDefinition
 import graphql.language.ObjectTypeDefinition
 import graphql.language.SDLNamedDefinition
@@ -18,7 +19,8 @@ internal data class ScannedSchemaObjects(
     val customScalars: CustomScalarMap,
     val rootInfo: RootTypeInfo,
     val fieldResolversByType: Map<ObjectTypeDefinition, MutableMap<FieldDefinition, FieldResolver>>,
-    val unusedDefinitions: Set<TypeDefinition<*>>
+    val unusedDefinitions: Set<TypeDefinition<*>>,
+    val directiveOnlyEnums: Set<EnumTypeDefinition>
 )
 
 internal typealias TypeClassDictionary = BiMap<TypeDefinition<*>, JavaType>

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

@@ -171,15 +171,51 @@ internal class SchemaClassScanner(
             ?: error("No ${TypeDefinition::class.java.simpleName} for type name $inputTypeName")
         when (typeDefinition) {
             is ScalarTypeDefinition -> handleFoundScalarType(typeDefinition)
-            is EnumTypeDefinition -> handleDictionaryTypes(listOf(typeDefinition)) {
-                "Enum type '${it.name}' is used in a directive, but no class could be found for that type name. Please pass a class for type '${it.name}' in the parser's dictionary."
-            }
+            is EnumTypeDefinition -> handleDirectiveEnum(typeDefinition)
             is InputObjectTypeDefinition -> handleDictionaryTypes(listOf(typeDefinition)) {
                 "Input object type '${it.name}' is used in a directive, but no class could be found for that type name. Please pass a class for type '${it.name}' in the parser's dictionary."
             }
         }
     }
 
+    /**
+     * Directive argument values never reach resolver methods, so an enum used in a directive doesn't need a class. Without one
+     * it goes into [unvalidatedTypes] and, unless a field uses it too, is built with its value names as values, see
+     * [validateAndCreateResult].
+     */
+    private fun handleDirectiveEnum(type: EnumTypeDefinition) {
+        if (dictionary.containsKey(type)) {
+            return
+        }
+
+        val initialEntry = initialDictionary[type.name]
+        if (initialEntry != null) {
+            handleFoundType(type, initialEntry.get(), DictionaryReference())
+        } else {
+            unvalidatedTypes.add(type)
+        }
+    }
+
+    /**
+     * The names of the types used by the fields, arguments and input fields of [definitions], including fields added by extensions.
+     */
+    private fun typeNamesUsedByFields(definitions: Set<TypeDefinition<*>>): Set<String> {
+        val names = definitions.map { it.name }.toSet()
+        return (definitions + extensionDefinitions + inputExtensionDefinitions)
+            .filter { it.name in names }
+            .flatMap { definition ->
+                when (definition) {
+                    is ImplementingTypeDefinition<*> -> definition.fieldDefinitions.flatMap { field ->
+                        field.inputValueDefinitions.map { it.type } + field.type
+                    }
+                    is InputObjectTypeDefinition -> definition.inputValueDefinitions.map { it.type }
+                    else -> emptyList()
+                }
+            }
+            .mapNotNull { (it.unwrap() as TypeName).name }
+            .toSet()
+    }
+
     private fun validateAndCreateResult(rootTypeHolder: RootTypesHolder): ScannedSchemaObjects {
         initialDictionary
             .filter { !it.value.accessed }
@@ -198,8 +234,8 @@ internal class SchemaClassScanner(
                 dictionary
                     .filter {
                         it.value.javaType != null
-                            && it.value.typeClass() != java.lang.Object::class.java
-                            && !java.util.Map::class.java.isAssignableFrom(it.value.typeClass())
+                            && it.value.typeClass() != Any::class.java
+                            && !Map::class.java.isAssignableFrom(it.value.typeClass())
                             && it.key !is InputObjectTypeDefinition
                             && it.key !is UnionTypeDefinition
                     }.mapValuesTo(it) { it.value.javaType }
@@ -246,7 +282,13 @@ internal class SchemaClassScanner(
 
         val definitions = observedDefinitions + extensionDefinitions + inputExtensionDefinitions + directiveDefinitions
 
-        return ScannedSchemaObjects(dictionary, definitions, scalars, rootInfo, fieldResolversByType.toMap(), unusedDefinitions)
+        // The enums in unvalidatedTypes come from directives without a class, so they can only be built when no field uses them.
+        // A field hands the enum's values to user code even when the scanner found no class through it, e.g. an input field bound
+        // to a Map or an unimplemented field's argument.
+        val typeNamesUsedByFields = typeNamesUsedByFields(observedDefinitions)
+        val directiveOnlyEnums = unvalidatedTypes.filterIsInstance<EnumTypeDefinition>().filter { it.name !in typeNamesUsedByFields }.toSet()
+
+        return ScannedSchemaObjects(dictionary, definitions, scalars, rootInfo, fieldResolversByType.toMap(), unusedDefinitions, directiveOnlyEnums)
     }
 
     private fun validateRootResolversWereUsed(rootType: RootType?, fieldResolvers: List<FieldResolver>) {
@@ -313,7 +355,7 @@ internal class SchemaClassScanner(
      * Scan a new object for types that haven't been mapped yet.
      */
     private fun scanQueueItemForPotentialMatches(item: QueueItem) {
-        val resolverInfo = if (item.clazz == Object::class.java) {
+        val resolverInfo = if (item.clazz == Any::class.java) {
             getResolverInfoFromTypeDictionary(item.type.name)
                 ?: throw SchemaClassScannerError("The GraphQL schema type '${item.type.name}' maps to a field of type java.lang.Object however there is no matching entry for this type in the type dictionary. You may need to add this type to the dictionary before building the schema.")
         } else {
@@ -349,7 +391,7 @@ internal class SchemaClassScanner(
     }
 
     private fun isResolverForSupertype(resolverInfo: NormalResolverInfo, dataClass: Class<*>) =
-        resolverInfo.dataClassType != Object::class.java && resolverInfo.dataClassType.isAssignableFrom(dataClass)
+        resolverInfo.dataClassType != Any::class.java && resolverInfo.dataClassType.isAssignableFrom(dataClass)
 
     private fun scanResolverInfoForPotentialMatches(type: ObjectTypeDefinition, resolverInfo: ResolverInfo) {
         type.getExtendedFieldDefinitions(extensionDefinitions).forEach { field ->
@@ -386,7 +428,7 @@ internal class SchemaClassScanner(
                 if (options.preferGraphQLResolver && realEntry.hasResolverRef()) {
                     log.warn("The real entry ${realEntry.joinReferences()} is a GraphQLResolver so ignoring this one ${javaType.unwrap()} $reference")
                 } else {
-                    if (java.util.Map::class.java.isAssignableFrom(javaType.unwrap())) {
+                    if (Map::class.java.isAssignableFrom(javaType.unwrap())) {
                         throw SchemaClassScannerError("Two different property map classes used for type ${type.name}:\n${realEntry.joinReferences()}\n\n- ${javaType}:\n|   ${reference.getDescription()}")
                     }
                     throw SchemaClassScannerError("Two different classes used for type ${type.name}:\n${realEntry.joinReferences()}\n\n- ${javaType.typeName}:\n|   ${reference.getDescription()}")

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

@@ -12,7 +12,7 @@ import graphql.language.*
 import graphql.schema.*
 import graphql.schema.idl.RuntimeWiring
 import graphql.schema.idl.ScalarInfo
-import graphql.schema.visibility.NoIntrospectionGraphqlFieldVisibility
+import graphql.schema.visibility.BlockedFields
 import org.slf4j.LoggerFactory
 import kotlin.reflect.KClass
 
@@ -39,6 +39,7 @@ class SchemaParser internal constructor(
     private val rootInfo = scanResult.rootInfo
     private val fieldResolversByType = scanResult.fieldResolversByType
     private val unusedDefinitions = scanResult.unusedDefinitions
+    private val directiveOnlyEnums = scanResult.directiveOnlyEnums
 
     private val extensionDefinitions = definitions.filterIsInstance<ObjectTypeExtensionDefinition>()
     private val inputExtensionDefinitions = definitions.filterIsInstance<InputObjectTypeExtensionDefinition>()
@@ -68,7 +69,10 @@ class SchemaParser internal constructor(
      */
     fun parseSchemaObjects(): SchemaObjects {
         if (!options.introspectionEnabled) {
-            codeRegistryBuilder.fieldVisibility(NoIntrospectionGraphqlFieldVisibility.NO_INTROSPECTION_FIELD_VISIBILITY)
+            // What the deprecated NoIntrospectionGraphqlFieldVisibility does. Its suggested replacement,
+            // Introspection.enabledJvmWide, would also turn introspection off for every other schema in the JVM.
+            // TODO deprecate introspectionEnabled and remove this in 15.0.0 (#858)
+            codeRegistryBuilder.fieldVisibility(BlockedFields.newBlock().addPattern("__.*").build())
         }
         // this overrides the above introspection enabled setting obviously... todo: add documentation
         options.fieldVisibility?.let { codeRegistryBuilder.fieldVisibility(it) }
@@ -195,9 +199,10 @@ class SchemaParser internal constructor(
 
     private fun createEnumObject(definition: EnumTypeDefinition): GraphQLEnumType {
         val name = definition.name
-        val type = dictionary[definition]
-            ?: 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!")
+        // enums only used by directives can do without a class, see SchemaClassScanner.handleDirectiveEnum
+        val enumClass = dictionary[definition]?.unwrap()
+        if (enumClass == null && definition !in directiveOnlyEnums) throw SchemaError("Expected enum with name '$name' but found none!")
+        if (enumClass != null && !enumClass.isEnum) throw SchemaError("Type '$name' is declared as an enum in the GraphQL schema but is not a Java enum!")
 
         val builder = GraphQLEnumType.newEnum()
             .name(name)
@@ -208,8 +213,12 @@ class SchemaParser internal constructor(
             .apply {
                 definition.enumValueDefinitions.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!")
+                    val enumValue = if (enumClass == null) {
+                        enumName
+                    } else {
+                        enumClass.enumConstants.find { (it as Enum<*>).name == enumName }
+                            ?: throw SchemaError("Expected value for name '$enumName' in enum '${enumClass.simpleName}' but found none!")
+                    }
 
                     value(
                         GraphQLEnumValueDefinition.newEnumValueDefinition()
@@ -427,6 +436,7 @@ class SchemaParser internal constructor(
     }
 
     // TODO remove this once directives are fully replaced with applied directives
+    @Suppress("DEPRECATION") // legacy directive argument values
     private fun buildDirectives(
         directives: List<Directive>,
         directiveLocation: Introspection.DirectiveLocation
@@ -529,7 +539,7 @@ class SchemaParser internal constructor(
             is NonNullType -> GraphQLNonNull(determineType(expectedType, typeDefinition.type, allowedTypeReferences, inputObjects))
             is InputObjectTypeDefinition -> {
                 log.info("Create input object")
-                createInputObject(typeDefinition, inputObjects, referencingInputObjects as MutableSet<String>)
+                createInputObject(typeDefinition, inputObjects, referencingInputObjects)
             }
             is TypeName -> {
                 val scalarType = customScalars[typeDefinition.name]

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

@@ -2,6 +2,8 @@ package graphql.kickstart.tools
 
 import graphql.language.Definition
 import graphql.language.Document
+import graphql.language.SourceLocation
+import graphql.parser.InvalidSyntaxException
 import graphql.parser.MultiSourceReader
 import graphql.parser.Parser
 import graphql.parser.ParserEnvironment
@@ -11,6 +13,7 @@ import graphql.schema.idl.RuntimeWiring
 import graphql.schema.idl.SchemaDirectiveWiring
 import org.antlr.v4.runtime.RecognitionException
 import org.antlr.v4.runtime.misc.ParseCancellationException
+import java.text.NumberFormat
 import kotlin.Int.Companion.MAX_VALUE
 import kotlin.reflect.KClass
 
@@ -20,7 +23,7 @@ import kotlin.reflect.KClass
 class SchemaParserBuilder {
 
     private val dictionary = SchemaParserDictionary()
-    private val schemaString = StringBuilder()
+    private val schemaStrings = mutableListOf<Source>()
     private val files = mutableListOf<String>()
     private val resolvers = mutableListOf<GraphQLResolver<*>>()
     private val scalars = mutableListOf<GraphQLScalarType>()
@@ -49,10 +52,14 @@ class SchemaParserBuilder {
      * Add a GraphQL schema string directly.
      */
     fun schemaString(string: String) = this.apply {
-        if (schemaString.isNotEmpty()) {
-            schemaString.append("\n")
-        }
-        schemaString.append(string)
+        schemaStrings.add(Source(string, null))
+    }
+
+    /**
+     * Add a GraphQL schema string directly, naming its source in parse errors and source locations.
+     */
+    fun schemaString(string: String, sourceName: String) = this.apply {
+        schemaStrings.add(Source(string, sourceName))
     }
 
     /**
@@ -172,10 +179,10 @@ class SchemaParserBuilder {
 
     private fun parseDocuments(): List<Document> {
         try {
-            val documents = files.map { parseDocument(readFile(it), it) }.toMutableList()
+            val documents = files.map { parseDocument(listOf(Source(readFile(it), it))) }.toMutableList()
 
-            if (schemaString.isNotBlank()) {
-                documents.add(parseDocument(schemaString.toString()))
+            if (schemaStrings.any { it.text.isNotBlank() }) {
+                documents.add(parseDocument(schemaStrings))
             }
 
             return documents
@@ -189,16 +196,37 @@ class SchemaParserBuilder {
         }
     }
 
-    private fun parseDocument(input: String, sourceName: String? = null): Document {
-        val sourceReader = MultiSourceReader
-            .newMultiSourceReader()
-            .string(input, sourceName)
+    private fun parseDocument(sources: List<Source>): Document {
+        // graphql-java's MultiSourceReader numbers the last line of the last source from the start of the first one.
+        // Ending every source with a line break leaves only the end of input on that line, see syntaxErrorInSource.
+        val separated = if (sources.size > 1) sources.map { it.endingWithLineBreak() } else sources
+        val sourceReader = MultiSourceReader.newMultiSourceReader()
+            .apply { separated.forEach { string(it.text, it.name) } }
             .trackData(true).build()
         val environment = ParserEnvironment
             .newParserEnvironment()
             .document(sourceReader)
             .parserOptions(parserOptions).build()
-        return parser.parseDocument(environment)
+        try {
+            return parser.parseDocument(environment)
+        } catch (e: InvalidSyntaxException) {
+            throw syntaxErrorInSource(e, separated.last())
+        }
+    }
+
+    /**
+     * Makes the line of an unexpected end of input relative to the last source and adds the source name to the message.
+     */
+    private fun syntaxErrorInSource(e: InvalidSyntaxException, lastSource: Source): InvalidSyntaxException {
+        val location = e.location ?: return e
+        val line = if (e.offendingToken == "<EOF>") lastSource.text.lines().size else location.line
+        if (line == location.line && location.sourceName == null) return e
+
+        // graphql-java formats the line for the default locale, e.g. "1,103"
+        val number = NumberFormat.getInstance()
+        val message = e.message.orEmpty().replaceFirst(" ${number.format(location.line)} ", " ${number.format(line)} ")
+        val inSource = location.sourceName?.let { " in $it" }.orEmpty()
+        return SchemaSyntaxException(message + inSource, SourceLocation(line, location.column, location.sourceName), e)
     }
 
     private fun readFile(filename: String) =
@@ -209,6 +237,10 @@ class SchemaParserBuilder {
      * Build the parser with the supplied schema and dictionary.
      */
     fun build() = SchemaParser(scan(), options, runtimeWiringBuilder.build())
+
+    private data class Source(val text: String, val name: String?) {
+        fun endingWithLineBreak() = if (text.endsWith("\n")) this else copy(text = "$text\n")
+    }
 }
 
 class InvalidSchemaError(
@@ -218,3 +250,6 @@ class InvalidSchemaError(
     override val message: String
         get() = "Invalid schema provided (${recognitionException.javaClass.name}) at: ${recognitionException.offendingToken}"
 }
+
+internal class SchemaSyntaxException(message: String, location: SourceLocation, e: InvalidSyntaxException) :
+    InvalidSyntaxException(message, location, e.offendingToken, e.sourcePreview, e)

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

@@ -10,7 +10,6 @@ import graphql.schema.DataFetcher
 import graphql.schema.DataFetchingEnvironment
 import graphql.schema.visibility.GraphqlFieldVisibility
 import kotlinx.coroutines.Dispatchers
-import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.channels.ReceiveChannel
 import kotlinx.coroutines.reactive.publish
 import org.reactivestreams.Publisher
@@ -45,7 +44,6 @@ data class SchemaParserOptions internal constructor(
         fun newOptions() = Builder()
 
         @JvmStatic
-        @ExperimentalCoroutinesApi
         fun defaultOptions() = Builder().build()
     }
 
@@ -164,7 +162,6 @@ data class SchemaParserOptions internal constructor(
             this.allowUndeclaredDirectives = allowUndeclaredDirectives
         }
 
-        @ExperimentalCoroutinesApi
         fun build(): SchemaParserOptions {
             val coroutineContextProvider = coroutineContextProvider
                 ?: DefaultCoroutineContextProvider(Dispatchers.Default)