diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt index 8844ba6b..5b081e2d 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt @@ -1,5 +1,6 @@ package graphql.kickstart.tools +import com.fasterxml.jackson.databind.JsonNode import graphql.kickstart.tools.resolver.FieldResolver import graphql.kickstart.tools.resolver.FieldResolverScanner import graphql.kickstart.tools.util.* @@ -8,13 +9,14 @@ import graphql.schema.GraphQLScalarType import graphql.schema.idl.ScalarInfo import org.apache.commons.lang3.ClassUtils import org.slf4j.LoggerFactory +import java.lang.reflect.Member import java.lang.reflect.Method /** * @author Andrew Potter */ internal class SchemaClassScanner( - initialDictionary: BiMap>, + initialDictionary: Map>, allDefinitions: List>, resolvers: List>, private val scalars: CustomScalarMap, @@ -45,6 +47,7 @@ internal class SchemaClassScanner( private val typeClassMatcher = TypeClassMatcher(definitionsByName) private val dictionary = mutableMapOf, DictionaryEntry>() private val unvalidatedTypes = mutableSetOf>() + private val enumsWithoutClass = mutableMapOf() private val queue = linkedSetOf() private val fieldResolversByType = mutableMapOf>() @@ -56,6 +59,18 @@ internal class SchemaClassScanner( } } + // Input types never go into the type => class bimap, so several of them can share a class, e.g. ObjectNode. Every + // other kind still gets one class per type, as before. Most of them end up in the bimap, which interface and union + // type resolution use to find the type of an object. + initialDictionary + .filterKeys { definitionsByName[it] !is InputObjectTypeDefinition } + .entries.groupBy({ it.value }, { it.key }) + .filterValues { it.size > 1 } + .entries.firstOrNull() + ?.let { (clazz, names) -> + throw SchemaClassScannerError("Class in supplied dictionary '${clazz.name}' specified type names ${names.joinToString { "'$it'" }}, but only input object types can share a class!") + } + 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.") } @@ -181,6 +196,11 @@ internal class SchemaClassScanner( } private fun validateAndCreateResult(rootTypeHolder: RootTypesHolder): ScannedSchemaObjects { + enumsWithoutClass + .filter { dictionary[it.key]?.javaType == null } + .values.firstOrNull() + ?.let { throw SchemaClassScannerError(it) } + initialDictionary .filter { !it.value.accessed } .forEach { @@ -412,31 +432,83 @@ internal class SchemaClassScanner( scanInterfacesOfType(graphQLType) } - is InputObjectTypeDefinition -> { - val inputObjectTypes = listOf(graphQLType) + inputExtensionDefinitions.filter { it.name == graphQLType.name } - inputObjectTypes - .flatMap { it.inputValueDefinitions } - .forEach { inputValueDefinition -> - val inputGraphQLType = inputValueDefinition.type.unwrap() - if (inputGraphQLType is TypeName && !ScalarInfo.GRAPHQL_SPECIFICATION_SCALARS_DEFINITIONS.containsKey(inputGraphQLType.name)) { - val inputValueJavaType = findInputValueType(inputValueDefinition.name, inputGraphQLType, javaType.unwrap()) - if (inputValueJavaType != null) { - handleFoundType(typeClassMatcher.match(TypeClassMatcher.PotentialMatch.parameterType( - inputValueDefinition.type, - inputValueJavaType, - GenericTypeResolver(javaType, options), - InputObjectReference(inputValueDefinition) - ))) - } else { - var mappingAdvice = "Try adding it manually to the dictionary" - if (javaType.unwrap().name.contains("Map")) { - mappingAdvice = " or add a class to represent your input type instead of a Map." - } - log.warn("Cannot find definition for field '${inputValueDefinition.name}: ${inputGraphQLType.name}' on input type '${graphQLType.name}' -> ${javaType.unwrap().name}. $mappingAdvice") - } - } + is InputObjectTypeDefinition -> handleInputObjectFields(graphQLType, javaType) + } + } + + /** + * Finds the types of the input object's fields through the properties of its class. A null [javaType] means the input object + * has no known class, e.g. because it's only reachable through a Map. + */ + private fun handleInputObjectFields(graphQLType: InputObjectTypeDefinition, javaType: JavaType?) { + val inputObjectTypes = listOf(graphQLType) + inputExtensionDefinitions.filter { it.name == graphQLType.name } + inputObjectTypes + .flatMap { it.inputValueDefinitions } + .forEach { inputValueDefinition -> + val inputGraphQLType = inputValueDefinition.type.unwrap() + if (inputGraphQLType is TypeName && !ScalarInfo.GRAPHQL_SPECIFICATION_SCALARS_DEFINITIONS.containsKey(inputGraphQLType.name)) { + val propertyType = javaType?.unwrap()?.let { findInputValueTypeInType(inputValueDefinition.name, it) } + val inputValueJavaType = propertyType ?: initialDictionary[inputGraphQLType.name]?.get() + if (inputValueJavaType != null) { + val genericContext = javaType ?: inputValueJavaType + handleFoundType(typeClassMatcher.match(TypeClassMatcher.PotentialMatch.parameterType( + // a class from the dictionary is for the unwrapped type, not for its list or non-null wrapper + if (propertyType != null) inputValueDefinition.type else inputGraphQLType, + inputValueJavaType, + GenericTypeResolver(genericContext, options), + InputObjectReference(inputValueDefinition) + ))) + } else { + handleInputValueWithoutJavaType(graphQLType, inputValueDefinition, inputGraphQLType, javaType?.unwrap()) } + } + } + } + + /** + * Without a Java property for an input field its type can still be built, as long as it doesn't need a class of its own. + * At runtime the value is whatever the input object's class (or the Map) deserializes it into. + */ + private fun handleInputValueWithoutJavaType(graphQLType: InputObjectTypeDefinition, inputValueDefinition: InputValueDefinition, inputGraphQLType: TypeName, clazz: Class<*>?) { + if (clazz != null && !isPropertyMap(clazz)) { + log.warn("Cannot find a public getter or field for field '${inputValueDefinition.name}: ${inputGraphQLType.name}' of input type '${graphQLType.name}' on ${clazz.name}. " + + "Unless Jackson maps it some other way, e.g. through @JsonProperty, check that the property name matches the schema, " + + "otherwise the field's value is rejected or dropped when the input is deserialized.") + } + + when (val definition = definitionsByName[inputGraphQLType.name]) { + is ScalarTypeDefinition -> handleFoundScalarType(definition) + is InputObjectTypeDefinition -> { + val isNewType = !dictionary.containsKey(definition) + // registered without a class, so a class found for it later still gets set and scanned + handleFoundType(definition, null, InputObjectReference(inputValueDefinition)) + if (isNewType) { + handleInputObjectFields(definition, null) + } } + // the enum class may still be found through another field, so this is only reported once scanning is done + is EnumTypeDefinition -> enumsWithoutClass.putIfAbsent(definition, enumWithoutClassMessage(definition, graphQLType, inputValueDefinition, clazz)) + // undefined types and output types are reported by the SchemaParser + else -> {} + } + } + + private fun isPropertyMap(clazz: Class<*>) = + java.util.Map::class.java.isAssignableFrom(clazz) || JsonNode::class.java.isAssignableFrom(clazz) || clazz == Object::class.java + + private fun isLibraryClass(clazz: Class<*>) = + listOf("java.", "javax.", "kotlin.", "com.fasterxml.jackson.").any { clazz.name.startsWith(it) } + + private fun enumWithoutClassMessage(enum: EnumTypeDefinition, graphQLType: InputObjectTypeDefinition, inputValueDefinition: InputValueDefinition, clazz: Class<*>?): String { + val field = inputValueDefinition.name + val problem = "Enum type '${enum.name}' is used by field '$field' of input type '${graphQLType.name}', but its Java enum can't be determined" + return when { + clazz == null -> "$problem because no class was found for input type '${graphQLType.name}'. " + + "Pass the enum class for type '${enum.name}' in the parser's dictionary, or a class for '${graphQLType.name}' with a public getter or field '$field'." + isPropertyMap(clazz) && isLibraryClass(clazz) -> "$problem because '${graphQLType.name}' is bound to ${clazz.name}, which has no type information for its fields. " + + "Pass the enum class for type '${enum.name}' in the parser's dictionary, or use a class for '${graphQLType.name}' with a public getter or field '$field'." + else -> "$problem because ${clazz.name} has no public getter or field named '$field'. " + + "Add a public getter or field '$field' to the class used for '${graphQLType.name}', or pass the enum class for type '${enum.name}' in the parser's dictionary." } } @@ -453,17 +525,11 @@ internal class SchemaClassScanner( queue.add(QueueItem(graphQLType, javaType)) } - private fun findInputValueType(name: String, inputGraphQLType: TypeName, clazz: Class): JavaType? { - val inputValueType = findInputValueTypeInType(name, clazz) - if (inputValueType != null) { - return inputValueType - } - - return initialDictionary[inputGraphQLType.name]?.get() - } - private fun findInputValueTypeInType(name: String, clazz: Class<*>): JavaType? { - val methods = clazz.methods + // a Map or JsonNode has no properties for the input's fields, only methods that may share their names, e.g. ObjectNode.fields(), + // but a subclass of one may still declare typed getters or fields of its own + val isProperty = { member: Member -> !isPropertyMap(clazz) || !isLibraryClass(member.declaringClass) } + val methods = clazz.methods.filter(isProperty) val (getters, methodsWithParameters) = methods.filter { it.name == name || it.name == "get${name.replaceFirstChar(Char::titlecase)}" @@ -471,7 +537,7 @@ internal class SchemaClassScanner( return getters.find { !it.isSynthetic }?.genericReturnType ?: getters.firstOrNull()?.genericReturnType - ?: clazz.fields.find { it.name == name }?.genericType + ?: clazz.fields.find { it.name == name && isProperty(it) }?.genericType ?: methodsWithParameters.find { !it.isSynthetic }?.genericReturnType ?: methodsWithParameters.firstOrNull()?.genericReturnType } diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt index 7417e54c..050ff2a0 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt @@ -506,8 +506,7 @@ class SchemaParser internal constructor( scalarType } else { if (!allowedTypeReferences.contains(typeDefinition.name)) { - throw SchemaError("Expected type '${typeDefinition.name}' to be a ${expectedType.simpleName}, but it wasn't! " + - "Was a type only permitted for object types incorrectly used as an input type, or vice-versa?") + throw unexpectedTypeError(typeDefinition, expectedType) } inputObjects.find { it.name == typeDefinition.name } ?: GraphQLTypeReference(typeDefinition.name) } @@ -538,8 +537,7 @@ class SchemaParser internal constructor( scalarType } else { if (!allowedTypeReferences.contains(typeDefinition.name)) { - throw SchemaError("Expected type '${typeDefinition.name}' to be a ${expectedType.simpleName}, but it wasn't! " + - "Was a type only permitted for object types incorrectly used as an input type, or vice-versa?") + throw unexpectedTypeError(typeDefinition, expectedType) } val found = inputObjects.filter { it.name == typeDefinition.name } if (found.size == 1) { @@ -565,6 +563,65 @@ class SchemaParser internal constructor( else -> throw SchemaError("Unknown type: $typeDefinition") } + private fun unexpectedTypeError(typeName: TypeName, expectedType: KClass<*>): SchemaError { + val name = typeName.name + val subject = "Type '$name'" + + (findUsage(typeName)?.let { " used by $it" } ?: "") + + (typeName.sourceLocation?.let { " at line ${it.line}, column ${it.column}" + (it.sourceName?.let { source -> " of $source" } ?: "") } ?: "") + // unused ones first, since the extensions of an unused type are still among the definitions + val definition = unusedDefinitions.find { it.name == name } + ?: definitions.filterIsInstance>().find { it.name == name } + val isInput = expectedType == GraphQLInputType::class + val kind = when (definition) { + is InputObjectTypeDefinition -> "an input object type" + is InterfaceTypeDefinition -> "an interface" + is UnionTypeDefinition -> "a union" + else -> "an object type" + } + + return SchemaError(when { + definition == null -> "$subject is not defined in the schema." + isInput && definition !is InputObjectTypeDefinition && definition !is EnumTypeDefinition && definition !is ScalarTypeDefinition -> + "$subject is $kind, which can't be used as an input type. Input types are input objects, enums and scalars." + !isInput && definition is InputObjectTypeDefinition -> + "$subject is $kind, which can't be used as an output type. Output types are object types, interfaces, unions, enums and scalars." + definition in unusedDefinitions -> + "$subject is defined in the schema, but was not reached while scanning the resolvers and the classes they use, so it wasn't built. " + + unreachedTypeHint(definition) + else -> "$subject can't be used as ${if (isInput) "an input" else "an output"} type." + }) + } + + // what makes the scanner reach the type depends on its kind, see SchemaClassScanner.canIncludeUnusedType + private fun unreachedTypeHint(definition: TypeDefinition<*>): String = when (definition) { + is InterfaceTypeDefinition, is UnionTypeDefinition -> + "Return it from a resolver method or one of those classes, or set SchemaParserOptions.includeUnusedTypes." + is EnumTypeDefinition -> + "Use it in a resolver method or one of those classes, or set SchemaParserOptions.includeUnusedTypes and pass its enum class in the parser's dictionary." + is InputObjectTypeDefinition -> + "Use it in an argument of a resolver method, directly or through another input type. SchemaParserOptions.includeUnusedTypes doesn't include input types." + is ScalarTypeDefinition -> + "Use it in a resolver method or one of those classes. SchemaParserOptions.includeUnusedTypes doesn't include scalars." + else -> + "Return it from a resolver method or one of those classes, or set SchemaParserOptions.includeUnusedTypes and pass a class for it in the parser's dictionary." + } + + // determineType only gets the type reference, so the definition using it is looked up for the error message + private fun findUsage(typeName: TypeName): String? = definitions.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() === typeName }?.first + + private fun fieldUsages(typeName: String, fields: List): List>> = 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 } + } + /** * Returns an optional [String] describing a deprecated field/enum. * If a deprecation directive was defined using the @deprecated directive, diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaParserDictionary.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaParserDictionary.kt index 116fa7ca..1fc10eeb 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaParserDictionary.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaParserDictionary.kt @@ -5,9 +5,14 @@ import kotlin.reflect.KClass class SchemaParserDictionary { - private val dictionary: BiMap> = BiMap.create() + // Not a BiMap, since several input types can share a class, e.g. ObjectNode. The scanner checks that other types don't. + private val dictionary = linkedMapOf>() - fun getDictionary(): BiMap> = BiMap.unmodifiableBiMap(dictionary) + /** + * 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. + */ + fun getDictionary(): BiMap> = BiMap.unmodifiableCopyOf(dictionary) /** * Add arbitrary classes to the parser's dictionary, overriding the generated type name. diff --git a/src/main/kotlin/graphql/kickstart/tools/util/BiMap.java b/src/main/kotlin/graphql/kickstart/tools/util/BiMap.java index 230f995c..e1ec9603 100644 --- a/src/main/kotlin/graphql/kickstart/tools/util/BiMap.java +++ b/src/main/kotlin/graphql/kickstart/tools/util/BiMap.java @@ -1,6 +1,7 @@ package graphql.kickstart.tools.util; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.Map; import java.util.Objects; import java.util.Set; @@ -21,6 +22,17 @@ public static BiMap unmodifiableBiMap(BiMap biMap) { return new BiMap<>(unmodifiableMap(biMap.delegate), unmodifiableMap(biMap.inverse)); } + /** + * Returns an unmodifiable bimap with the entries of the given map. Unlike {@link #put(Object, Object)}, several keys may + * have the same value. The inverse only maps such a value back to the first of its keys, so the inverse and + * {@link #values()} can have fewer entries than the bimap. + */ + public static BiMap unmodifiableCopyOf(Map map) { + Map inverse = new HashMap<>(); + map.forEach((key, value) -> inverse.putIfAbsent(value, key)); + return new BiMap<>(unmodifiableMap(new LinkedHashMap<>(map)), unmodifiableMap(inverse)); + } + private BiMap(Map delegate, Map inverse) { this.delegate = delegate; this.inverse = inverse; diff --git a/src/test/kotlin/graphql/kickstart/tools/NestedInputTypesTest.kt b/src/test/kotlin/graphql/kickstart/tools/NestedInputTypesTest.kt index 2079cb91..f2b093c4 100644 --- a/src/test/kotlin/graphql/kickstart/tools/NestedInputTypesTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/NestedInputTypesTest.kt @@ -1,5 +1,7 @@ package graphql.kickstart.tools +import com.fasterxml.jackson.annotation.JsonProperty +import com.fasterxml.jackson.databind.node.ObjectNode import graphql.GraphQL import graphql.execution.AsyncExecutionStrategy import org.junit.Test @@ -103,6 +105,369 @@ class NestedInputTypesTest { assertEquals((data["materials"]), emptyList()) } + @Test + fun `nested input types are found through a Map parameter`() { + val schema = SchemaParser.newParser() + .schemaString( + """ + type Query { + discountName(filter: MaterialFilter!): String + } + + input MaterialFilter { + title: String + requestFilter: RequestFilter + } + + input RequestFilter { + and: [RequestFilter!] + or: [RequestFilter!] + discountTypeFilter: DiscountTypeFilter + } + + input DiscountTypeFilter { + name: String + } + """) + .resolvers(object : GraphQLQueryResolver { + fun discountName(filter: Map): String? { + val requestFilter = filter["requestFilter"] as Map<*, *> + val discountTypeFilter = (requestFilter["and"] as List<*>).single() as Map<*, *> + return (discountTypeFilter["discountTypeFilter"] as Map<*, *>)["name"] as String? + } + }) + .build() + .makeExecutableSchema() + val gql = GraphQL.newGraphQL(schema).build() + + val data = assertNoGraphQlErrors(gql) { + """ + query { + discountName(filter: { requestFilter: { and: [{ discountTypeFilter: { name: "discount" } }] } }) + } + """ + } + + assertEquals(data["discountName"], "discount") + } + + @Test + fun `nested input types are found through ObjectNode parameters`() { + val schema = SchemaParser.newParser() + .schemaString( + """ + type Query { + test: String + } + + type Mutation { + create(input: EntityCreateInput!): String + update(input: EntityUpdateInput!): String + } + + input EntityCreateInput { + name: String + nested: Nested + } + + input EntityUpdateInput { + id: ID! + nested: Nested + } + + input Nested { + x: Int + } + """) + .resolvers(object : GraphQLQueryResolver { + fun test(): String? = null + }, object : GraphQLMutationResolver { + fun create(input: ObjectNode): String = input.toString() + fun update(input: ObjectNode): String = input.toString() + }) + .build() + .makeExecutableSchema() + val gql = GraphQL.newGraphQL(schema).build() + + val data = assertNoGraphQlErrors(gql) { + """ + mutation { + create(input: { name: "n", nested: { x: 1 } }) + update(input: { id: "2", nested: { x: 3 } }) + } + """ + } + + assertEquals(data["create"], """{"name":"n","nested":{"x":1}}""") + assertEquals(data["update"], """{"id":"2","nested":{"x":3}}""") + } + + @Test + fun `nested input types are found when the input class has no matching property`() { + val schema = SchemaParser.newParser() + .schemaString( + """ + type Query { + test: String + } + + type Mutation { + createAgreement(data: AgreementCreateInput!): String + } + + input AgreementCreateInput { + name: String + IncludedAgreement_back: IncludedAgreementCreateManyInput + } + + input IncludedAgreementCreateManyInput { + create: [IncludedAgreementCreateInput!] + } + + input IncludedAgreementCreateInput { + name: String + } + """) + .resolvers(object : GraphQLQueryResolver { + fun test(): String? = null + }, object : GraphQLMutationResolver { + fun createAgreement(data: AgreementCreateInput): String? = + data.getIncludedAgreement_Back()?.create?.joinToString { it.name.orEmpty() } + }) + .build() + .makeExecutableSchema() + val gql = GraphQL.newGraphQL(schema).build() + + val data = assertNoGraphQlErrors(gql) { + """ + mutation { + createAgreement(data: { name: "a", IncludedAgreement_back: { create: [{ name: "b" }, { name: "c" }] } }) + } + """ + } + + assertEquals(data["createAgreement"], "b, c") + } + + @Test + fun `nested input type reached without a class is scanned again once its class is found`() { + val schema = SchemaParser.newParser() + .schemaString( + """ + type Query { + byMap(filter: ColorFilter): String + byClass(color: ColorInput): String + } + + input ColorFilter { + color: ColorInput + } + + input ColorInput { + value: Color + } + + enum Color { + RED + GREEN + } + """) + .resolvers(object : GraphQLQueryResolver { + fun byMap(filter: Map): String = (filter["color"] as Map<*, *>)["value"].toString() + fun byClass(color: ColorInput): String = color.value.toString() + }) + .build() + .makeExecutableSchema() + val gql = GraphQL.newGraphQL(schema).build() + + val data = assertNoGraphQlErrors(gql) { + """ + query { + byMap(filter: { color: { value: RED } }) + byClass(color: { value: GREEN }) + } + """ + } + + assertEquals(data["byMap"], "RED") + assertEquals(data["byClass"], "GREEN") + } + + @Test + fun `list field of a nested input type reached without a class uses the dictionary class of its element`() { + val schema = SchemaParser.newParser() + .schemaString( + """ + type Query { + byMap(filter: ShipmentFilter): String + byClass(shipment: ShipmentInput): String + } + + input ShipmentFilter { + shipment: ShipmentInput + } + + input ShipmentInput { + lines: [ShipmentLineInput!] + } + + input ShipmentLineInput { + sku: String + } + """) + .resolvers(object : GraphQLQueryResolver { + fun byMap(filter: Map): String = filter["shipment"].toString() + fun byClass(shipment: ShipmentInput): String = shipment.lines.orEmpty().joinToString { it.sku.orEmpty() } + }) + .dictionary("ShipmentLineInput", ShipmentLineInput::class.java) + .build() + .makeExecutableSchema() + val gql = GraphQL.newGraphQL(schema).build() + + val data = assertNoGraphQlErrors(gql) { + """ + query { + byMap(filter: { shipment: { lines: [{ sku: "a" }] } }) + byClass(shipment: { lines: [{ sku: "b" }, { sku: "c" }] }) + } + """ + } + + assertEquals(data["byMap"], "{lines=[{sku=a}]}") + assertEquals(data["byClass"], "b, c") + } + + @Test + fun `input fields bound to ObjectNode are not matched against its own methods`() { + val schema = SchemaParser.newParser() + .schemaString( + """ + type Query { + create(input: EntityInput!): String + } + + input EntityInput { + values: [Nested] + elements: [Nested] + fields: [Nested] + } + + input Nested { + x: Int + } + """) + .resolvers(object : GraphQLQueryResolver { + fun create(input: ObjectNode): String = input.toString() + }) + .build() + .makeExecutableSchema() + val gql = GraphQL.newGraphQL(schema).build() + + val data = assertNoGraphQlErrors(gql) { + """ + query { + create(input: { values: [{ x: 1 }], elements: [{ x: 2 }], fields: [{ x: 3 }] }) + } + """ + } + + assertEquals(data["create"], """{"values":[{"x":1}],"elements":[{"x":2}],"fields":[{"x":3}]}""") + } + + @Test + fun `input field types are found through the getters of a Map subclass`() { + val schema = SchemaParser.newParser() + .schemaString( + """ + type Query { + kind(filter: KindFilter!): String + nested(filter: NestedKindFilter!): String + } + + input KindFilter { + kind: Kind + } + + input NestedKindFilter { + inner: InnerKindFilter + } + + input InnerKindFilter { + kind: Kind + } + + enum Kind { + A + B + } + """) + .resolvers(object : GraphQLQueryResolver { + fun kind(filter: KindFilterMap): String = filter.kind.toString() + fun nested(filter: NestedKindFilterMap): String = filter["inner"].toString() + }) + .build() + .makeExecutableSchema() + val gql = GraphQL.newGraphQL(schema).build() + + val data = assertNoGraphQlErrors(gql) { + """ + query { + kind(filter: { kind: B }) + nested(filter: { inner: { kind: A } }) + } + """ + } + + assertEquals(data["kind"], "B") + assertEquals(data["nested"], "{kind=A}") + } + + class KindFilterMap : HashMap() { + val kind: Kind? get() = this["kind"]?.let { Kind.valueOf(it.toString()) } + } + + class NestedKindFilterMap : HashMap() { + val inner: InnerKindFilter? get() = null + } + + class InnerKindFilter { + var kind: Kind? = null + } + + enum class Kind { A, B } + + class AgreementCreateInput { + var name: String? = null + + // the getter doesn't follow the bean naming of the schema field, so only Jackson knows about the property + @field:JsonProperty("IncludedAgreement_back") + private var includedAgreementBack: IncludedAgreementCreateManyInput? = null + + fun getIncludedAgreement_Back(): IncludedAgreementCreateManyInput? = includedAgreementBack + } + + class IncludedAgreementCreateManyInput { + var create: List? = null + } + + class IncludedAgreementCreateInput { + var name: String? = null + } + + class ColorInput { + var value: Color? = null + } + + enum class Color { RED, GREEN } + + class ShipmentInput { + var lines: List? = null + } + + class ShipmentLineInput { + var sku: String? = null + } + class QueryResolver : GraphQLQueryResolver { fun materials(filter: MaterialFilter): List = listOf() } diff --git a/src/test/kotlin/graphql/kickstart/tools/SchemaClassScannerTest.kt b/src/test/kotlin/graphql/kickstart/tools/SchemaClassScannerTest.kt index 24f0e662..2199fd94 100644 --- a/src/test/kotlin/graphql/kickstart/tools/SchemaClassScannerTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/SchemaClassScannerTest.kt @@ -1,6 +1,8 @@ package graphql.kickstart.tools +import graphql.GraphQL import graphql.GraphQLContext +import graphql.Scalars import graphql.execution.CoercedVariables import graphql.language.Value import graphql.schema.* @@ -462,6 +464,53 @@ class SchemaClassScannerTest { var map: Map? = null } + @Test + fun `scanner handles custom scalars only used in input types bound to a Map`() { + val foo = GraphQLScalarType.newScalar(Scalars.GraphQLString) + .name("Foo") + .description("foo") + .build() + + val schema = SchemaParser.newParser() + .resolvers(object : GraphQLQueryResolver { + fun test(): String? = null + }, object : GraphQLMutationResolver { + fun createHuman(input: Map): String = "${input["name"]} ${input["foo"]}" + }) + .scalars(foo) + .schemaString( + """ + scalar Foo + + type Query { + test: String + } + + type Mutation { + createHuman(input: CreateHumanInput!): String + } + + input CreateHumanInput { + name: String + homePlanet: String + foo: Foo + } + """) + .build() + .makeExecutableSchema() + val gql = GraphQL.newGraphQL(schema).build() + + val data = assertNoGraphQlErrors(gql) { + """ + mutation { + createHuman(input: { name: "Luke", foo: "bar" }) + } + """ + } + + assertEquals(data["createHuman"], "Luke bar") + } + @Test fun `scanner allows class to be used for object type and input object type`() { val schema = SchemaParser.newParser() diff --git a/src/test/kotlin/graphql/kickstart/tools/SchemaParserDictionaryTest.kt b/src/test/kotlin/graphql/kickstart/tools/SchemaParserDictionaryTest.kt new file mode 100644 index 00000000..625decea --- /dev/null +++ b/src/test/kotlin/graphql/kickstart/tools/SchemaParserDictionaryTest.kt @@ -0,0 +1,273 @@ +package graphql.kickstart.tools + +import com.fasterxml.jackson.databind.node.ObjectNode +import graphql.GraphQL +import org.junit.Assert.assertThrows +import org.junit.Test + +class SchemaParserDictionaryTest { + + private val entitySchema = """ + type Query { + test: String + } + + type Mutation { + create(input: EntityCreateInput!): String + update(input: EntityUpdateInput!): String + } + + input EntityCreateInput { + name: String + nested: Nested + } + + input EntityUpdateInput { + id: ID! + nested: Nested + } + + input Nested { + x: Int + } + """ + + private val directiveSchema = """ + directive @a(arg: AInput) on FIELD_DEFINITION + directive @b(arg: BInput) on FIELD_DEFINITION + + type Query { + test: String @a(arg: { x: 1 }) @b(arg: { y: "b" }) + } + + input AInput { + x: Int + } + + input BInput { + y: String + } + """ + + @Test + fun `input types can share a dictionary class`() { + val schema = SchemaParser.newParser() + .schemaString(entitySchema) + .resolvers(EntityQuery(), EntityMutation()) + .dictionary("EntityCreateInput", ObjectNode::class.java) + .dictionary("EntityUpdateInput", ObjectNode::class.java) + .build() + .makeExecutableSchema() + val gql = GraphQL.newGraphQL(schema).build() + + val data = assertNoGraphQlErrors(gql) { + """ + mutation { + create(input: { name: "n", nested: { x: 1 } }) + update(input: { id: "2", nested: { x: 3 } }) + } + """ + } + + assertEquals(data["create"], """{"name":"n","nested":{"x":1}}""") + assertEquals(data["update"], """{"id":"2","nested":{"x":3}}""") + } + + @Test + fun `nested input types can share a dictionary class`() { + val schema = SchemaParser.newParser() + .schemaString( + """ + type Query { + create(input: EntityInput!): String + } + + input EntityInput { + name: String + a: NestedA + b: [NestedB!] + } + + input NestedA { + x: Int + } + + input NestedB { + y: Int + } + """) + .resolvers(object : GraphQLQueryResolver { + fun create(input: ObjectNode): String = input.toString() + }) + .dictionary("NestedA", ObjectNode::class.java) + .dictionary("NestedB", ObjectNode::class.java) + .build() + .makeExecutableSchema() + val gql = GraphQL.newGraphQL(schema).build() + + val data = assertNoGraphQlErrors(gql) { + """ + query { + create(input: { name: "n", a: { x: 1 }, b: [{ y: 2 }] }) + } + """ + } + + assertEquals(data["create"], """{"name":"n","a":{"x":1},"b":[{"y":2}]}""") + } + + @Test + fun `directive input types can share a dictionary class`() { + val schema = SchemaParser.newParser() + .schemaString(directiveSchema) + .resolvers(DirectiveQuery()) + .dictionary("AInput", Map::class.java) + .dictionary("BInput", Map::class.java) + .build() + .makeExecutableSchema() + + val field = schema.queryType.getFieldDefinition("test") + assertEquals(field.getAppliedDirective("a").getArgument("arg")!!.getValue>(), mapOf("x" to 1)) + assertEquals(field.getAppliedDirective("b").getArgument("arg")!!.getValue>(), mapOf("y" to "b")) + + val gql = GraphQL.newGraphQL(schema).build() + val data = assertNoGraphQlErrors(gql) { + """ + query { + test + } + """ + } + + assertEquals(data["test"], "test") + } + + @Test + fun `input types sharing a class can be passed to the builder from a SchemaParserDictionary`() { + val dictionary = SchemaParserDictionary() + .add("AInput", Map::class) + .add("BInput", Map::class) + + assertEquals(dictionary.getDictionary().toMap(), mapOf("AInput" to Map::class.java, "BInput" to Map::class.java)) + + // graphql-spring-boot reads a SchemaParserDictionary bean this way + val schema = SchemaParser.newParser() + .schemaString(directiveSchema) + .resolvers(DirectiveQuery()) + .dictionary(dictionary.getDictionary()) + .build() + .makeExecutableSchema() + + val field = schema.queryType.getFieldDefinition("test") + assertEquals(field.getAppliedDirective("b").getArgument("arg")!!.getValue>(), mapOf("y" to "b")) + } + + @Test + fun `input type and object type can share a dictionary class`() { + // ItemInput is only used by a directive, so its class can only come from the dictionary + val schema = SchemaParser.newParser() + .schemaString( + """ + directive @d(arg: ItemInput) on FIELD_DEFINITION + + type Query { + node: Node @d(arg: { name: "z" }) + } + + interface Node { + name: String + } + + type Item implements Node { + name: String + } + + input ItemInput { + name: String + } + """) + .resolvers(object : GraphQLQueryResolver { + fun node(): Node = Item().apply { name = "a" } + }) + .dictionary("Item", Item::class.java) + .dictionary("ItemInput", Item::class.java) + .build() + .makeExecutableSchema() + + val field = schema.queryType.getFieldDefinition("node") + assertEquals(field.getAppliedDirective("d").getArgument("arg")!!.getValue>(), mapOf("name" to "z")) + + val gql = GraphQL.newGraphQL(schema).build() + val data = assertNoGraphQlErrors(gql) { + """ + query { + node { + __typename + name + } + } + """ + } + + assertEquals(data["node"], mapOf("__typename" to "Item", "name" to "a")) + } + + @Test + fun `object types can't share a dictionary class`() { + val error = assertThrows(SchemaClassScannerError::class.java) { + SchemaParser.newParser() + .schemaString( + """ + type Query { + a: A + b: B + } + + type A { + name: String + } + + type B { + name: String + } + + input AInput { + name: String + } + """) + .resolvers(object : GraphQLQueryResolver { + fun a(): Item? = null + fun b(): Item? = null + }) + .dictionary("A", Item::class.java) + .dictionary("AInput", Item::class.java) + .dictionary("B", Item::class.java) + .build() + } + + // the input type sharing the class isn't part of the conflict + assertEquals(error.message, "Class in supplied dictionary '${Item::class.java.name}' specified type names 'A', 'B', " + + "but only input object types can share a class!") + } + + class EntityQuery : GraphQLQueryResolver { + fun test(): String? = null + } + + class EntityMutation : GraphQLMutationResolver { + fun create(input: ObjectNode): String = input.toString() + fun update(input: ObjectNode): String = input.toString() + } + + class DirectiveQuery : GraphQLQueryResolver { + fun test(): String = "test" + } + + interface Node { + val name: String? + } + + class Item : Node { + override var name: String? = null + } +} diff --git a/src/test/kotlin/graphql/kickstart/tools/SchemaParserTest.kt b/src/test/kotlin/graphql/kickstart/tools/SchemaParserTest.kt index e5e2ba30..4353764e 100644 --- a/src/test/kotlin/graphql/kickstart/tools/SchemaParserTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/SchemaParserTest.kt @@ -200,24 +200,146 @@ class SchemaParserTest { @Test fun `parser should throw descriptive exception when object is used as input type incorrectly`() { - assertThrows("Was a type only permitted for object types incorrectly used as an input type, or vice-versa", SchemaError::class.java) { + val error = assertThrows(SchemaError::class.java) { SchemaParser.newParser() .schemaString( """ - type Query { - name(filter: Filter): [String] - } - - type Filter { - filter: String - } - """) + |type Query { + | name(filter: Filter): [String] + |} + | + |type Filter { + | filter: String + |} + """.trimMargin()) .resolvers(object : GraphQLQueryResolver { fun name(filter: Filter): List? = null }) .build() .makeExecutableSchema() } + + assertEquals(error.message, "Type 'Filter' used by argument 'filter' of field 'Query.name' at line 2, column 18 is an object type, " + + "which can't be used as an input type. Input types are input objects, enums and scalars.") + } + + @Test + fun `parser should throw descriptive exception when input object is used as output type incorrectly`() { + val error = assertThrows(SchemaError::class.java) { + SchemaParser.newParser() + .schemaString( + """ + |type Query { + | filters: [Filter!] + |} + | + |input Filter { + | filter: String + |} + """.trimMargin()) + .resolvers(object : GraphQLQueryResolver { + fun filters(): List? = null + }) + .build() + .makeExecutableSchema() + } + + assertEquals(error.message, "Type 'Filter' used by field 'Query.filters' at line 2, column 15 is an input object type, " + + "which can't be used as an output type. Output types are object types, interfaces, unions, enums and scalars.") + } + + @Test + fun `parser should throw descriptive exception when input type is not defined`() { + val error = assertThrows(SchemaError::class.java) { + SchemaParser.newParser() + .schemaString( + """ + |type Query { + | test(input: TestInput): String + |} + | + |input TestInput { + | nested: [Nested!] + |} + """.trimMargin()) + .resolvers(object : GraphQLQueryResolver { + fun test(input: Map): String? = null + }) + .build() + .makeExecutableSchema() + } + + assertEquals(error.message, "Type 'Nested' used by input field 'TestInput.nested' at line 6, column 14 is not defined in the schema.") + } + + @Test + fun `parser should throw descriptive exception when type was never reached by the scanner`() { + val error = assertThrows(SchemaError::class.java) { + SchemaParser.newParser() + .file("UnreachedType.graphqls") + .resolvers(object : GraphQLQueryResolver { + fun pet(): Pet? = null + }) + .build() + .makeExecutableSchema() + } + + assertEquals(error.message, "Type 'Cat' used by field 'Pet.friend' at line 6, column 11 of UnreachedType.graphqls is defined in the schema, " + + "but was not reached while scanning the resolvers and the classes they use, so it wasn't built. " + + "Return it from a resolver method or one of those classes, or set SchemaParserOptions.includeUnusedTypes and pass a class for it in the parser's dictionary.") + } + + @Test + fun `parser should build type that was never reached by the scanner when including unused types`() { + SchemaParser.newParser() + .file("UnreachedType.graphqls") + .resolvers(object : GraphQLQueryResolver { + fun pet(): Pet? = null + }) + .dictionary(Cat::class) + .options(SchemaParserOptions.newOptions().includeUnusedTypes(true).build()) + .build() + .makeExecutableSchema() + } + + @Test + fun `parser should throw descriptive exception when input type is only used by an unused interface`() { + val error = assertThrows(SchemaError::class.java) { + SchemaParser.newParser() + .schemaString( + """ + |type Query { + | name: String + |} + | + |interface Searchable { + | search(criteria: Criteria): String + |} + | + |input Criteria { + | text: String + |} + """.trimMargin()) + .resolvers(object : GraphQLQueryResolver { + fun name(): String? = null + }) + .options(SchemaParserOptions.newOptions().includeUnusedTypes(true).build()) + .build() + .makeExecutableSchema() + } + + assertEquals(error.message, "Type 'Criteria' used by argument 'criteria' of field 'Searchable.search' at line 6, column 22 is defined in the schema, " + + "but was not reached while scanning the resolvers and the classes they use, so it wasn't built. " + + "Use it in an argument of a resolver method, directly or through another input type. " + + "SchemaParserOptions.includeUnusedTypes doesn't include input types.") + } + + interface Pet { + val friend: Cat? + } + + class Cat { + var name: String? = null } @Test @@ -356,6 +478,123 @@ class SchemaParserTest { .makeExecutableSchema() } + @Test + fun `support enum types used in input Map if their class is found elsewhere`() { + SchemaParser.newParser() + .schemaString( + """ + type Query { + save(input: SaveInput!): Boolean + type: EnumType + } + + input SaveInput { + type: EnumType! + } + + enum EnumType { + TEST + } + """) + .resolvers(object : GraphQLQueryResolver { + fun save(input: Map<*, *>): Boolean = false + fun type(): EnumType? = null + }) + .build() + .makeExecutableSchema() + } + + @Test + fun `parser should throw descriptive exception when enum is only used in input Map without a class`() { + val error = assertThrows(SchemaClassScannerError::class.java) { + SchemaParser.newParser() + .schemaString( + """ + type Query { + save(input: SaveInput!): Boolean + } + + input SaveInput { + type: EnumType! + } + + enum EnumType { + TEST + } + """) + .resolvers(object : GraphQLQueryResolver { + fun save(input: Map<*, *>): Boolean = false + }) + .build() + } + + assertEquals(error.message, "Enum type 'EnumType' is used by field 'type' of input type 'SaveInput', but its Java enum can't be determined " + + "because 'SaveInput' is bound to java.util.Map, which has no type information for its fields. Pass the enum class for type " + + "'EnumType' in the parser's dictionary, or use a class for 'SaveInput' with a public getter or field 'type'.") + } + + @Test + fun `parser should throw descriptive exception when enum is only used in input Map subclass without a getter for it`() { + val error = assertThrows(SchemaClassScannerError::class.java) { + SchemaParser.newParser() + .schemaString( + """ + type Query { + save(input: SaveInput!): Boolean + } + + input SaveInput { + type: EnumType! + } + + enum EnumType { + TEST + } + """) + .resolvers(object : GraphQLQueryResolver { + fun save(input: SaveInputMap): Boolean = false + }) + .build() + } + + assertEquals(error.message, "Enum type 'EnumType' is used by field 'type' of input type 'SaveInput', but its Java enum can't be determined " + + "because ${SaveInputMap::class.java.name} has no public getter or field named 'type'. Add a public getter or field 'type' to the class " + + "used for 'SaveInput', or pass the enum class for type 'EnumType' in the parser's dictionary.") + } + + @Test + fun `parser should throw descriptive exception when enum is only used in a nested input without a class`() { + val error = assertThrows(SchemaClassScannerError::class.java) { + SchemaParser.newParser() + .schemaString( + """ + type Query { + save(input: SaveInput!): Boolean + } + + input SaveInput { + nested: NestedInput + } + + input NestedInput { + type: EnumType! + } + + enum EnumType { + TEST + } + """) + .resolvers(object : GraphQLQueryResolver { + fun save(input: Map<*, *>): Boolean = false + }) + .build() + } + + assertEquals(error.message, "Enum type 'EnumType' is used by field 'type' of input type 'NestedInput', but its Java enum can't be determined " + + "because no class was found for input type 'NestedInput'. Pass the enum class for type 'EnumType' in the parser's dictionary, " + + "or a class for 'NestedInput' with a public getter or field 'type'.") + } + @Test fun `allow circular relations in input objects`() { SchemaParser.newParser() @@ -682,6 +921,8 @@ class SchemaParserTest { TEST } + class SaveInputMap : HashMap() + class QueryWithIdResolver : GraphQLQueryResolver { fun getId(): String? = null } diff --git a/src/test/kotlin/graphql/kickstart/tools/util/BiMapTest.kt b/src/test/kotlin/graphql/kickstart/tools/util/BiMapTest.kt index fc753990..fdc56f30 100644 --- a/src/test/kotlin/graphql/kickstart/tools/util/BiMapTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/util/BiMapTest.kt @@ -67,4 +67,19 @@ class BiMapTest { val expected = setOf(1, 2, 3) assertEquals(expected, values) } + + @Test + fun `unmodifiable copy should keep keys with the same value`() { + val copy = BiMap.unmodifiableCopyOf(linkedMapOf("foo" to 1, "bar" to 2, "baz" to 1)) + + assertEquals(mapOf("foo" to 1, "bar" to 2, "baz" to 1), copy.toMap()) + assertEquals(listOf("foo", "bar", "baz"), copy.keys.toList()) + assertEquals(setOf(1, 2), copy.values) + assertEquals(mapOf(1 to "foo", 2 to "bar"), copy.inverse().toMap()) + } + + @Test(expected = UnsupportedOperationException::class) + fun `unmodifiable copy should not permit changes`() { + BiMap.unmodifiableCopyOf(mapOf("foo" to 1))["bar"] = 2 + } } diff --git a/src/test/resources/UnreachedType.graphqls b/src/test/resources/UnreachedType.graphqls new file mode 100644 index 00000000..d6fe977f --- /dev/null +++ b/src/test/resources/UnreachedType.graphqls @@ -0,0 +1,11 @@ +type Query { + pet: Pet +} + +interface Pet { + friend: Cat +} + +type Cat { + name: String +}