Merge branch 'master' into feature/893-reject-scalar-directives

Author: oryan-block

README.md

@@ -26,7 +26,7 @@ kotlin.version=2.3.10
 
 Add the dependency:
 ```groovy
-compile 'io.github.graphql-java-kickstart:graphql-java-tools:14.4.0'
+compile 'io.github.graphql-java-kickstart:graphql-java-tools:14.5.0'
 ```
 
 ### Using Maven
@@ -43,7 +43,7 @@ Add the dependency:
 <dependency>
     <groupId>io.github.graphql-java-kickstart</groupId>
     <artifactId>graphql-java-tools</artifactId>
-    <version>14.4.0</version>
+    <version>14.5.0</version>
 </dependency>
 ```
 

pom.xml

@@ -4,7 +4,7 @@
 
     <groupId>io.github.graphql-java-kickstart</groupId>
     <artifactId>graphql-java-tools</artifactId>
-    <version>14.4.1-SNAPSHOT</version>
+    <version>14.5.1-SNAPSHOT</version>
     <packaging>jar</packaging>
 
     <name>GraphQL Java Tools</name>

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

@@ -29,7 +29,7 @@ internal class GenericTypeResolver(private val containingType: JavaType, private
      * - wildcards are replaced by their upper bound, e.g. `? extends Foo` becomes `Foo`
      * - primitives are replaced by their boxed class, e.g. `int` becomes `Integer`
      */
-    fun resolve(javaType: JavaType, onUnwrap: ((wrapped: ParameterizedType) -> Unit)? = null): JavaType {
+    fun resolve(javaType: JavaType, onUnwrap: ((SchemaParserOptions.GenericWrapper) -> Unit)? = null): JavaType {
         return when (val type = resolveTypeVariables(javaType)) {
             is ParameterizedType -> {
                 val rawType = type.rawType
@@ -41,7 +41,7 @@ internal class GenericTypeResolver(private val containingType: JavaType, private
                     throw IndexOutOfBoundsException("Generic type '${TypeUtils.toString(type)}' does not have a type argument at index ${wrapper.index}!")
                 }
 
-                onUnwrap?.invoke(type)
+                onUnwrap?.invoke(wrapper)
                 val unwrapsTo = wrapper.schemaWrapper.invoke(typeArguments[wrapper.index])
                 resolve(unwrapsTo, onUnwrap)
             }

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

@@ -79,10 +79,11 @@ class SchemaParser internal constructor(
      * Parses the given schema with respect to the given dictionary and returns GraphQL objects.
      */
     fun parseSchemaObjects(): SchemaObjects {
+        @Suppress("DEPRECATION")
         if (!options.introspectionEnabled) {
             // 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)
+            // TODO remove this in 15.0.0 (#858)
             codeRegistryBuilder.fieldVisibility(BlockedFields.newBlock().addPattern("__.*").build())
         }
         // this overrides the above introspection enabled setting obviously... todo: add documentation
@@ -171,6 +172,8 @@ class SchemaParser internal constructor(
         val name = objectDefinition.name
         val extensions = typeExtensions.extensionsOf<ObjectTypeExtensionDefinition>(name)
         val directives = sdlDirectives.directivesOf(objectDefinition, extensions)
+        val fieldDefinitions = objectDefinition.withExtensions(typeExtensions).flatMap { it.fieldDefinitions }
+        warnRepeatedFields(name, fieldDefinitions)
         val builder = GraphQLObjectType.newObject()
             .name(name)
             .definition(objectDefinition)
@@ -185,7 +188,7 @@ class SchemaParser internal constructor(
                 }
             }
             .apply {
-                objectDefinition.withExtensions(typeExtensions).flatMap { it.fieldDefinitions }.forEach { fieldDefinition ->
+                fieldDefinitions.forEach { fieldDefinition ->
                     field { field ->
                         createField(field, fieldDefinition, inputObjects)
                         codeRegistryBuilder.dataFetcher(
@@ -209,6 +212,8 @@ class SchemaParser internal constructor(
                                   inputObjectsInProgress: MutableSet<String>): GraphQLInputObjectType {
         val extensionDefinitions = typeExtensions.extensionsOf<InputObjectTypeExtensionDefinition>(definition.name)
         val directives = sdlDirectives.directivesOf(definition, extensionDefinitions)
+        val fieldDefinitions = (extensionDefinitions + definition).flatMap { it.inputValueDefinitions }
+        warnRepeatedFields(definition.name, fieldDefinitions)
 
         inputObjectsInProgress.add(definition.name)
 
@@ -219,20 +224,18 @@ class SchemaParser internal constructor(
             .description(getDocumentation(definition, options))
             .withSdlDirectives(directives, INPUT_OBJECT)
             .apply {
-                (extensionDefinitions + definition).forEach { typeDefinition ->
-                    typeDefinition.inputValueDefinitions.forEach { fieldDefinition ->
-                        field(
-                            GraphQLInputObjectField.newInputObjectField()
-                                .name(fieldDefinition.name)
-                                .definition(fieldDefinition)
-                                .description(getDocumentation(fieldDefinition, options))
-                                .apply { fieldDefinition.defaultValue?.let { v -> defaultValueLiteral(v) } }
-                                .apply { sdlDirectives.getDeprecated(fieldDefinition.directives)?.let { deprecate(it) } }
-                                .type(determineInputType(fieldDefinition.type, inputObjects, inputObjectsInProgress))
-                                .withSdlDirectives(fieldDefinition.directives, INPUT_FIELD_DEFINITION)
-                                .build()
-                        )
-                    }
+                fieldDefinitions.forEach { fieldDefinition ->
+                    field(
+                        GraphQLInputObjectField.newInputObjectField()
+                            .name(fieldDefinition.name)
+                            .definition(fieldDefinition)
+                            .description(getDocumentation(fieldDefinition, options))
+                            .apply { fieldDefinition.defaultValue?.let { v -> defaultValueLiteral(v) } }
+                            .apply { sdlDirectives.getDeprecated(fieldDefinition.directives)?.let { deprecate(it) } }
+                            .type(determineInputType(fieldDefinition.type, inputObjects, inputObjectsInProgress))
+                            .withSdlDirectives(fieldDefinition.directives, INPUT_FIELD_DEFINITION)
+                            .build()
+                    )
                 }
             }
 
@@ -377,6 +380,16 @@ class SchemaParser internal constructor(
         }
     }
 
+    // TODO throw a SchemaError instead in 15.0.0 (#865), like graphql-java does
+    private fun warnRepeatedFields(typeName: String, fieldDefinitions: List<NamedNode<*>>) {
+        fieldDefinitions.groupBy { it.name }.values.filter { it.size > 1 }.forEach { declarations ->
+            val kept = declarations.last()
+            val location = kept.sourceLocation.describe()
+            val which = if (location.isEmpty()) "one of them" else "the one$location"
+            log.warn("Type '$typeName' declares field '${kept.name}' more than once, only $which is kept.")
+        }
+    }
+
     private fun createField(field: GraphQLFieldDefinition.Builder, fieldDefinition: FieldDefinition, inputObjects: List<GraphQLInputObjectType>): GraphQLFieldDefinition.Builder {
         return field
             .name(fieldDefinition.name)

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

@@ -11,16 +11,6 @@ class SchemaParserDictionary {
      */
     fun toMap(): Map<String, Class<*>> = dictionary.toMap()
 
-    // TODO remove in 15.0.0 (#900)
-    /**
-     * Returns a copy of the entries added so far. If several types share a class, the inverse of the copy only maps that class
-     * back to the first of them.
-     */
-    @Deprecated("Use toMap() instead", ReplaceWith("toMap()"))
-    @Suppress("DEPRECATION")
-    fun getDictionary(): graphql.kickstart.tools.util.BiMap<String, Class<*>> =
-        graphql.kickstart.tools.util.BiMap.unmodifiableCopyOf(dictionary)
-
     /**
      * Add arbitrary classes to the parser's dictionary, overriding the generated type name.
      */

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

@@ -31,6 +31,7 @@ data class SchemaParserOptions internal constructor(
     val proxyHandlers: List<ProxyHandler>,
     val inputArgumentOptionalDetectOmission: Boolean,
     val preferGraphQLResolver: Boolean,
+    @Deprecated("Will be removed in 15.0.0. Use Introspection.enabledJvmWide(false) for the whole JVM, or put Introspection.INTROSPECTION_DISABLED in the GraphQLContext of the request.")
     val introspectionEnabled: Boolean,
     val coroutineContextProvider: CoroutineContextProvider,
     val typeDefinitionFactories: List<TypeDefinitionFactory>,
@@ -128,6 +129,7 @@ data class SchemaParserOptions internal constructor(
             this.proxyHandlers.add(proxyHandler)
         }
 
+        @Deprecated("Will be removed in 15.0.0. Use Introspection.enabledJvmWide(false) for the whole JVM, or put Introspection.INTROSPECTION_DISABLED in the GraphQLContext of the request.")
         fun introspectionEnabled(introspectionEnabled: Boolean) = this.apply {
             this.introspectionEnabled = introspectionEnabled
         }

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

@@ -1,6 +1,7 @@
 package graphql.kickstart.tools
 
 import graphql.execution.DataFetcherResult
+import graphql.kickstart.tools.SchemaParserOptions.GenericWrapper
 import graphql.kickstart.tools.util.GraphQLLangType
 import graphql.kickstart.tools.util.JavaType
 import graphql.kickstart.tools.util.isSubtypeOf
@@ -15,79 +16,22 @@ import java.util.*
  */
 internal class TypeClassMatcher(private val definitionsByName: Map<String, TypeDefinition<*>>) {
 
-    private fun error(potentialMatch: PotentialMatch, msg: String) = SchemaClassScannerError("Unable to match type definition (${TypeUtil.simplePrint(potentialMatch.graphQLType)}) for reference ${potentialMatch.reference} with java type (${potentialMatch.javaType}): $msg")
-
     fun match(potentialMatch: PotentialMatch): Match {
-        return match(potentialMatch, potentialMatch.graphQLType, potentialMatch.javaType, true)
-    }
-
-    private fun match(potentialMatch: PotentialMatch, graphQLType: GraphQLLangType, javaType: JavaType, root: Boolean = false): Match {
-
-        var realType = potentialMatch.typeResolver.resolve(javaType)
-
-        if (realType is ParameterizedType && realType.isSubtypeOf(DataFetcherResult::class.java)) {
-            if (potentialMatch.location != Location.RETURN_TYPE) {
-                throw error(potentialMatch, "${DataFetcherResult::class.java.name} can only be used as a return type")
-            }
-
-            realType = potentialMatch.typeResolver.resolve(realType.actualTypeArguments.first())
-
-            if (realType is ParameterizedType && realType.isSubtypeOf(DataFetcherResult::class.java)) {
-                throw error(potentialMatch, "${DataFetcherResult::class.java.name} cannot be nested within itself")
-            }
-        }
-
-        var optional = false
-
-        // Handle jdk8 Optionals
-        if (realType is ParameterizedType && realType.isSubtypeOf(Optional::class.java)) {
-            optional = true
-
-            if (potentialMatch.location == Location.RETURN_TYPE && !root) {
-                throw error(potentialMatch, "${Optional::class.java.name} can only be used at the top level of a return type")
-            }
-
-            realType = potentialMatch.typeResolver.resolve(realType.actualTypeArguments.first())
-
-            if (realType is ParameterizedType && realType.isSubtypeOf(Optional::class.java)) {
-                throw error(potentialMatch, "${Optional::class.java.name} cannot be nested within itself")
-            }
-        }
-
-        // Match graphql type to java type.
-        return when (graphQLType) {
-            is NonNullType -> {
-                if (optional) {
-                    throw error(potentialMatch, "graphql type is marked as nonnull but ${Optional::class.java.name} was used")
-                }
-                match(potentialMatch, graphQLType.type, realType)
-            }
-
-            is ListType -> when {
-                realType is ParameterizedType && realType.isSubtypeOf(Iterable::class.java) ->
-                    match(potentialMatch, graphQLType.type, realType.actualTypeArguments.first())
-                realType is Class<*> && realType.isArray ->
-                    match(potentialMatch, graphQLType.type, realType.componentType)
-                // e.g. a property map value: the element type is unknown, so match it as Object too
-                realType == Any::class.java ->
-                    match(potentialMatch, graphQLType.type, realType)
-                else ->
-                    throw error(potentialMatch, "Java class is not a List or generic type information was lost: $realType")
-            }
-
+        val unwrapping = potentialMatch.unwrapping
+        return when (val graphQLType = unwrapping.graphQLType) {
             is TypeName -> {
                 val typeDefinition = ScalarInfo.GRAPHQL_SPECIFICATION_SCALARS_DEFINITIONS[graphQLType.name]
                     ?: definitionsByName[graphQLType.name]
-                    ?: throw error(potentialMatch, "No ${TypeDefinition::class.java.simpleName} for type name ${graphQLType.name}")
+                    ?: throw potentialMatch.error("No ${TypeDefinition::class.java.simpleName} for type name ${graphQLType.name}")
                 if (typeDefinition is ScalarTypeDefinition) {
                     ScalarMatch(typeDefinition)
                 } else {
-                    ValidMatch(typeDefinition, realType, potentialMatch.reference)
+                    ValidMatch(typeDefinition, unwrapping.javaType, potentialMatch.reference)
                 }
             }
 
-            is TypeDefinition<*> -> ValidMatch(graphQLType, realType, potentialMatch.reference)
-            else -> throw error(potentialMatch, "Unknown type: ${realType.javaClass.name}")
+            is TypeDefinition<*> -> ValidMatch(graphQLType, unwrapping.javaType, potentialMatch.reference)
+            else -> throw potentialMatch.error("Unknown type: ${unwrapping.javaType.javaClass.name}")
         }
     }
 
@@ -109,6 +53,72 @@ internal class TypeClassMatcher(private val definitionsByName: Map<String, TypeD
         val reference: ScanDictionary.Reference,
         val location: Location
     ) {
+        val unwrapping: Unwrapping by lazy { unwrap(graphQLType, javaType, root = true, mutableListOf()) }
+
+        fun error(msg: String) = SchemaClassScannerError("Unable to match type definition (${TypeUtil.simplePrint(graphQLType)}) for reference $reference with java type ($javaType): $msg")
+
+        private fun unwrap(graphQLType: GraphQLLangType, javaType: JavaType, root: Boolean, unwrapped: MutableList<Unwrapped>): Unwrapping {
+            val onUnwrap = { wrapper: GenericWrapper -> unwrapped += Unwrapped.Wrapper(wrapper) }
+            var realType = typeResolver.resolve(javaType, onUnwrap)
+
+            if (realType is ParameterizedType && realType.isSubtypeOf(DataFetcherResult::class.java)) {
+                if (location != Location.RETURN_TYPE) {
+                    throw error("${DataFetcherResult::class.java.name} can only be used as a return type")
+                }
+
+                unwrapped += Unwrapped.Data
+                realType = typeResolver.resolve(realType.actualTypeArguments.first(), onUnwrap)
+
+                if (realType is ParameterizedType && realType.isSubtypeOf(DataFetcherResult::class.java)) {
+                    throw error("${DataFetcherResult::class.java.name} cannot be nested within itself")
+                }
+            }
+
+            var optional = false
+
+            // Handle jdk8 Optionals
+            if (realType is ParameterizedType && realType.isSubtypeOf(Optional::class.java)) {
+                optional = true
+
+                if (location == Location.RETURN_TYPE && !root) {
+                    throw error("${Optional::class.java.name} can only be used at the top level of a return type")
+                }
+
+                unwrapped += Unwrapped.Present
+                realType = typeResolver.resolve(realType.actualTypeArguments.first(), onUnwrap)
+
+                if (realType is ParameterizedType && realType.isSubtypeOf(Optional::class.java)) {
+                    throw error("${Optional::class.java.name} cannot be nested within itself")
+                }
+            }
+
+            // Match graphql type to java type.
+            return when (graphQLType) {
+                is NonNullType -> {
+                    if (optional) {
+                        throw error("graphql type is marked as nonnull but ${Optional::class.java.name} was used")
+                    }
+                    unwrap(graphQLType.type, realType, root = false, unwrapped)
+                }
+
+                is ListType -> when {
+                    realType is ParameterizedType && realType.isSubtypeOf(Iterable::class.java) -> {
+                        unwrapped += Unwrapped.Elements
+                        unwrap(graphQLType.type, realType.actualTypeArguments.first(), root = false, unwrapped)
+                    }
+                    realType is Class<*> && realType.isArray ->
+                        unwrap(graphQLType.type, realType.componentType, root = false, unwrapped)
+                    // e.g. a property map value: the element type is unknown, so match it as Object too
+                    realType == Any::class.java ->
+                        unwrap(graphQLType.type, realType, root = false, unwrapped)
+                    else ->
+                        throw error("Java class is not a List or generic type information was lost: $realType")
+                }
+
+                else -> Unwrapping(unwrapped, graphQLType, realType)
+            }
+        }
+
         companion object {
             fun returnValue(graphQLType: GraphQLLangType, javaType: JavaType, typeResolver: GenericTypeResolver, reference: ScanDictionary.Reference) =
                 PotentialMatch(graphQLType, javaType, typeResolver, reference, Location.RETURN_TYPE)
@@ -117,4 +127,13 @@ internal class TypeClassMatcher(private val definitionsByName: Map<String, TypeD
                 PotentialMatch(graphQLType, javaType, typeResolver, reference, Location.PARAMETER_TYPE)
         }
     }
+
+    internal class Unwrapping(val unwrapped: List<Unwrapped>, val graphQLType: GraphQLLangType, val javaType: JavaType)
+
+    internal sealed interface Unwrapped {
+        class Wrapper(val wrapper: GenericWrapper) : Unwrapped
+        object Data : Unwrapped
+        object Present : Unwrapped
+        object Elements : Unwrapped
+    }
 }