diff --git a/pom.xml b/pom.xml index f9e50d44..dac83477 100644 --- a/pom.xml +++ b/pom.xml @@ -373,7 +373,6 @@ - java17-tests [17,) diff --git a/src/main/kotlin/graphql/kickstart/tools/ScannedSchemaObjects.kt b/src/main/kotlin/graphql/kickstart/tools/ScannedSchemaObjects.kt index ddb5df3e..5009c117 100644 --- a/src/main/kotlin/graphql/kickstart/tools/ScannedSchemaObjects.kt +++ b/src/main/kotlin/graphql/kickstart/tools/ScannedSchemaObjects.kt @@ -26,5 +26,4 @@ internal data class ScannedSchemaObjects( internal typealias TypeClassDictionary = BiMap, JavaType> internal typealias CustomScalarMap = Map -// the extensions of each type by type name, in the order they're declared internal typealias TypeExtensions = Map>> diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt index fe903b99..b6daded3 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt @@ -45,13 +45,11 @@ internal class SchemaClassScanner( private val typeDefinitions = allDefinitions.filterIsInstance>() private val baseTypeNames = typeDefinitions.filter { it !is SDLExtensionDefinition }.map { it.name }.toSet() - // the first extension of an undefined union, enum, interface or scalar stands in for its definition private val loneExtensions = typeDefinitions .filter { it is SDLExtensionDefinition && it !is ObjectTypeDefinition && it !is InputObjectTypeDefinition && it.name !in baseTypeNames } .distinctBy { it.name } private val definitionsByName = typeDefinitions.filter { it !is SDLExtensionDefinition || it in loneExtensions }.associateBy { it.name } - // without extensions of a different kind than their type, which are ignored with a warning, see init private val typeExtensions: TypeExtensions = typeDefinitions .filter { it is SDLExtensionDefinition && it !in loneExtensions && !extendsOtherKind(it) } .groupBy { it.name } @@ -80,8 +78,6 @@ internal class SchemaClassScanner( log.warn("Type '${extension.name}' is defined as ${definition.kind()}, but extended as ${extension.kind()}. The extension is ignored.") } - // Input object types never go into the type => class bimap that interface and union type resolution use, so several of - // them can share a class, e.g. ObjectNode initialDictionary .filterKeys { definitionsByName[it] !is InputObjectTypeDefinition } .entries.groupBy({ it.value }, { it.key }) @@ -397,8 +393,6 @@ internal class SchemaClassScanner( } } - // Scanning a sealed subclass can find the classes of the other types, so only fail when no type has exactly one sealed - // subclass named after it. Types still without a class fail on the next pass. val sealedMatches = typesWithoutClass.filterIsInstance().mapNotNull { sealedSubclassMatches(it).singleOrNull() } if (sealedMatches.isEmpty()) { typesWithoutClass.firstOrNull()?.let { throw SchemaClassScannerError(failureMessage(it) + sealedSubclassHint(it)) } @@ -407,9 +401,6 @@ internal class SchemaClassScanner( sealedMatches.forEach { handleFoundType(it) } } - /** - * Explains why no sealed subclass was used for the type, if any of its unions or interfaces has a sealed class. - */ private fun sealedSubclassHint(type: TypeDefinition<*>): String { if (type !is ObjectTypeDefinition) { return "" @@ -429,25 +420,15 @@ internal class SchemaClassScanner( } } - /** - * A member of a union or an implementor of an interface may never be returned directly, e.g. `NormalPet` when only - * `pets(): List` returns `union Pet`. If the class found for the union or interface is sealed, its subclass named - * after the object type can be used instead. - */ private fun sealedSubclassMatches(type: ObjectTypeDefinition): List = sealedTypesOf(type) .flatMap { sealedType -> sealedSubclasses(sealedType.unwrap()) .filter { it.simpleName == type.name } - .map { TypeClassMatcher.ValidMatch(type, parameterize(it, sealedType), SealedSubclassReference(sealedType)) } + .map { TypeClassMatcher.ValidMatch(type, withSealedTypeArguments(it, sealedType), SealedSubclassReference(sealedType)) } } - // a subclass is found twice when it's under two of these sealed types, or extends both a sealed type and one of its - // sealed subclasses .distinctBy { it.javaType.unwrap() } - /** - * The sealed classes found for the unions the type is a member of and the interfaces it implements. - */ private fun sealedTypesOf(type: ObjectTypeDefinition): List = dictionary .filterKeys { definition -> @@ -461,18 +442,11 @@ internal class SchemaClassScanner( .distinct() .filter { it.unwrap().kotlin.isSealed } - /** - * All subclasses of a sealed class, including those of its sealed subclasses. kotlin-reflect also returns the permitted - * subclasses of Java sealed classes. - */ + // kotlin-reflect also returns the permitted subclasses of Java sealed classes private fun sealedSubclasses(clazz: Class<*>): List> = clazz.kotlin.sealedSubclasses.flatMap { listOf(it.java) + sealedSubclasses(it.java) } - /** - * Passes the type arguments of a parameterized sealed class on to its generic subclass, e.g. `Success` for - * `Result`, so the types of the subclass's fields can be resolved. - */ - private fun parameterize(subclass: Class<*>, sealedType: JavaType): JavaType { + private fun withSealedTypeArguments(subclass: Class<*>, sealedType: JavaType): JavaType { if (sealedType !is ParameterizedType || subclass.typeParameters.isEmpty()) { return subclass } @@ -538,7 +512,6 @@ internal class SchemaClassScanner( } } - // a JDK dynamic proxy only implements its target's interfaces, so the target's own methods and fields can't be called on it // TODO throw a FieldResolverError instead in 15.0.0 (#868) private fun warnIfNotCallableOnResolver(type: ObjectTypeDefinition, fieldResolver: FieldResolver) { val source = fieldResolver.search.source ?: return @@ -693,7 +666,6 @@ internal class SchemaClassScanner( } } - // e.g. `extend union Foo` of `type Foo` private fun extendsOtherKind(definition: TypeDefinition<*>): Boolean { val type = definitionsByName[definition.name] return definition is SDLExtensionDefinition && type != null && type.kind() != definition.kind() diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt index 9253cc9f..f99a1413 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt @@ -187,7 +187,6 @@ class SchemaParser internal constructor( .withAppliedDirectives(*buildAppliedDirectives(directives)) .withDirectives(*buildDirectives(directives, Introspection.DirectiveLocation.INPUT_OBJECT)) .apply { - // unlike graphql-java the fields of the extensions come first, which is kept so the order of the fields doesn't change (extensionDefinitions + definition).forEach { typeDefinition -> typeDefinition.inputValueDefinitions.forEach { fieldDefinition -> field( @@ -326,9 +325,6 @@ class SchemaParser internal constructor( } } - // The directives of the extensions follow those of the type, like in graphql-java. A non-repeatable directive that an extension - // applies again is dropped, where graphql-java rejects the schema, so those schemas keep building and the applied directives - // match the legacy ones (see buildDirectives). Undeclared directives may be repeatable, so they're all kept. private fun directivesOf(definition: TypeDefinition<*>, extensions: List>): List { val applied = definition.directives.mapTo(mutableSetOf()) { it.name } return definition.directives + extensions.flatMap { extension -> diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaParserDictionary.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaParserDictionary.kt index 1fc10eeb..56224fa1 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaParserDictionary.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaParserDictionary.kt @@ -5,7 +5,6 @@ import kotlin.reflect.KClass class SchemaParserDictionary { - // 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>() /** diff --git a/src/main/kotlin/graphql/kickstart/tools/UnmappedInputFieldChecker.kt b/src/main/kotlin/graphql/kickstart/tools/UnmappedInputFieldChecker.kt index 1980bdac..c10b823b 100644 --- a/src/main/kotlin/graphql/kickstart/tools/UnmappedInputFieldChecker.kt +++ b/src/main/kotlin/graphql/kickstart/tools/UnmappedInputFieldChecker.kt @@ -21,23 +21,14 @@ import graphql.schema.GraphQLType import graphql.schema.GraphQLTypeUtil import org.slf4j.LoggerFactory -/** - * Checks that the classes of resolver arguments have a property for each field of their input objects, see [UnmappedInputFields]. - * - * The fields are looked up in the deserializer that the resolver's ObjectMapper converts the argument with, not in the class - * itself, so that a field counts as mapped whenever Jackson maps it. An input type can be used by fields whose mappers are - * configured differently, so it's checked once for each mapper. - */ internal class UnmappedInputFieldChecker( private val inputObjects: Map, private val unmappedInputFields: UnmappedInputFields ) { private val log = LoggerFactory.getLogger(javaClass) - // a deserializer is cached by its mapper, so this is per mapper, class and input type, and ends recursive input types private val checked = mutableSetOf, String>>() - // reported once for each consequence, with the first argument found using it, since mappers differ in ignoring unknown properties private val messages = linkedMapOf() fun check(objectType: String, resolver: MethodFieldResolver) { @@ -50,14 +41,11 @@ internal class UnmappedInputFieldChecker( val mapper = resolver.objectMapper val context = (mapper.deserializationContext as? DefaultDeserializationContext) ?.createDummyInstance(mapper.deserializationConfig) ?: return@forEachIndexed - // the same lookup as convertValue's when it converts the argument val deserializer = context.findRootValueDeserializer(mapper.typeFactory.constructType(parameterType)) checkFields(mapper, context, deserializer, null, inputObject, usage) } catch (e: VirtualMachineError) { throw e } catch (e: Throwable) { - // e.g. a class Jackson can't deserialize, or one that refers to a missing class, which only fails once the argument is - // converted, as it did before this check. Errors are caught too, since the lookup used to run on the first query log.debug("Can't check the input fields of $usage with Jackson", e) } } @@ -101,12 +89,9 @@ internal class UnmappedInputFieldChecker( } } - // the deserializer of the class an input object is converted to, past the lists, arrays and Optionals around it private tailrec fun beanDeserializer(context: DeserializationContext, deserializer: JsonDeserializer<*>?, property: BeanProperty?): BeanDeserializerBase? = when (deserializer) { - // a delegating creator gets the whole input object, e.g. as a Map is BeanDeserializerBase -> deserializer.takeUnless { it.valueInstantiator.canCreateUsingDelegate() } - // a Map gets the fields as its keys is ContainerDeserializerBase<*> -> if (deserializer.valueType.isMapLikeType || isPolymorphic(context, deserializer.contentType)) null else beanDeserializer(context, deserializer.contentDeserializer, property) @@ -120,20 +105,17 @@ internal class UnmappedInputFieldChecker( } /** - * Whether the elements of a list, an array or an Optional are read with type info, so that their class is only known once their - * type id is read. Their deserializer is the base class's, which doesn't know the subclasses' properties. The same lookup as - * Jackson's when it creates the deserializer: type info from a property's annotation, or else from the class or default typing. + * The deserializer of elements read with type info is the base class's, which doesn't know the subclasses' properties. The + * same lookup as Jackson's when it creates the deserializer: type info from a property's annotation, or else from the class + * or default typing. */ private fun isPolymorphic(context: DeserializationContext, contentType: JavaType): Boolean = contentType.getTypeHandler() != null || context.factory.findTypeDeserializer(context.config, contentType) != null /** - * Returns the properties Jackson knows if it fails on the field, or null if it maps or ignores it. - * * `{"": null}` is read into a placeholder object rather than an instance of the class, so neither its constructors nor * its setters are called, but the field still goes through Jackson's handling of a property the class doesn't have: ignored - * names, any setters, ignoreUnknown and problem handlers. Unknown properties are made to fail even if the mapper ignores them. - * Any other exception, e.g. from a deserializer that can't read into an existing object, means the field can't be checked. + * names, any setters, ignoreUnknown and problem handlers. */ private fun knownPropertiesIfUnmapped( mapper: ObjectMapper, diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/ArgumentConverter.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/ArgumentConverter.kt index 6c55b85f..c3708cf1 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/ArgumentConverter.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/ArgumentConverter.kt @@ -18,15 +18,10 @@ import java.io.IOException import java.lang.reflect.Type /** - * Converts an argument value that graphql-java already coerced to the type of the resolver method's parameter. - * * ObjectMapper.convertValue serializes a value and reads it back, which the coerced values of many custom scalars don't * survive, e.g. an upload, or a LocalDate without the jsr310 module. Those values are written as embedded objects * instead, which Jackson's deserializers return as they are if they have the type being read, and convert with * toString() if a String is read. - * - * An untyped parameter, e.g. a Map, gets every coerced value as it is, apart from strings, numbers, - * booleans and enums, which are converted as before. */ internal class ArgumentConverter(private val mapper: ObjectMapper, type: Type) { @@ -39,16 +34,12 @@ internal class ArgumentConverter(private val mapper: ObjectMapper, type: Type) { } if (isUntyped) { - // nothing reads an untyped value through its serialized form, so every coerced value is kept return read(embed(value) { true } ?: value) } - // a deserializer may read the serialized form, so values are only embedded if converting fails return try { mapper.convertValue(value, javaType) } catch (e: IllegalArgumentException) { - // a class whose value failed to convert isn't tried again in this value, since that can be expensive, e.g. for - // a list of uploads. It isn't remembered across calls, so the same input always gives the same result val nonConvertingClasses = HashSet>() val embedded = embed(value) { scalar -> when { @@ -63,16 +54,11 @@ internal class ArgumentConverter(private val mapper: ObjectMapper, type: Type) { try { read(embedded) } catch (retryError: IllegalArgumentException) { - // the values that failed to convert likely hid the actual problem throw retryError.apply { addSuppressed(e) } } } } - /** - * Returns a copy of the value in which the scalar values that [shouldEmbed] accepts are wrapped in [EmbeddedValue], - * or null if it accepts none. - */ private fun embed(value: Any, shouldEmbed: (Any) -> Boolean): Any? { var embedsAny = false fun copy(value: Any?): Any? = when (value) { @@ -131,7 +117,6 @@ internal class ArgumentConverter(private val mapper: ObjectMapper, type: Type) { .withHandler(embeddedValueHandler) } - // written to the buffer as an embedded object, which it keeps as it is private class EmbeddedValue(private val value: Any) : JsonSerializable { override fun serialize(gen: JsonGenerator, serializers: SerializerProvider) = gen.writeEmbeddedObject(value) diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt index 50d296d9..4f954e1e 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt @@ -49,7 +49,6 @@ internal class MethodFieldResolver( private val numberOfParameters = method.parameterCountWithoutContinuation() private val hasAdditionalParameter = numberOfParameters == (field.inputValueDefinitions.size + getIndexOffset() + 1) - // the arguments are converted with the same ObjectMapper that the UnmappedInputFieldChecker checks their classes with internal val objectMapper: ObjectMapper by lazy { options.objectMapperProvider.provide(field) } override fun createDataFetcher(): DataFetcher<*> { @@ -202,7 +201,6 @@ internal class MethodFieldResolver( private fun getJavaMethodParameterIndex(index: Int) = index + getIndexOffset() - // the type that the value of the argument at the index is converted to internal fun getMethodParameterType(index: Int): JavaType? { val methodIndex = getJavaMethodParameterIndex(index) val parameters = method.parameterTypes diff --git a/src/main/kotlin/graphql/kickstart/tools/util/Utils.kt b/src/main/kotlin/graphql/kickstart/tools/util/Utils.kt index f12608eb..913d0b5b 100644 --- a/src/main/kotlin/graphql/kickstart/tools/util/Utils.kt +++ b/src/main/kotlin/graphql/kickstart/tools/util/Utils.kt @@ -38,8 +38,7 @@ internal fun Type<*>.unwrap(): Type<*> = when (this) { internal inline fun > TypeExtensions.extensionsOf(name: String): List = this[name].orEmpty().filterIsInstance() -// The type followed by its extensions, e.g. `union.withExtensions(typeExtensions).flatMap { it.memberTypes }`. Every extension class -// is a subclass of its type's class, e.g. UnionTypeExtensionDefinition of UnionTypeDefinition. +// Every extension class is a subclass of its type's class, e.g. UnionTypeExtensionDefinition of UnionTypeDefinition. internal inline fun > T.withExtensions(typeExtensions: TypeExtensions): List = listOf(this) + typeExtensions.extensionsOf(name) diff --git a/src/test/java17/graphql/kickstart/tools/SchemaClassScannerSealedJavaTypesTest.java b/src/test/java17/graphql/kickstart/tools/SchemaClassScannerSealedJavaTypesTest.java index 18803f14..8880db03 100644 --- a/src/test/java17/graphql/kickstart/tools/SchemaClassScannerSealedJavaTypesTest.java +++ b/src/test/java17/graphql/kickstart/tools/SchemaClassScannerSealedJavaTypesTest.java @@ -12,7 +12,6 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; -// Sealed classes need Java 17, so the java17-tests profile only compiles and runs this on Java 17 and later. public class SchemaClassScannerSealedJavaTypesTest { @Test diff --git a/src/test/kotlin/graphql/kickstart/tools/CustomScalarInputTest.kt b/src/test/kotlin/graphql/kickstart/tools/CustomScalarInputTest.kt index ffb8f9cc..04b20e0e 100644 --- a/src/test/kotlin/graphql/kickstart/tools/CustomScalarInputTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/CustomScalarInputTest.kt @@ -67,7 +67,6 @@ class CustomScalarInputTest { "dateNode" to "LocalDate 2020-01-02", "dateLabel" to "2020-01-02 null" )) - // a String gets toString(), which is meaningless for a class that doesn't override it assert((data["opaqueLabel"] as String).startsWith("null ${Opaque::class.java.name}@")) { data["opaqueLabel"].toString() } } @@ -113,7 +112,6 @@ class CustomScalarInputTest { """ } - // including those the ObjectMapper has a serializer for, e.g. UUID assertEquals(resolver.received, mapOf( "map" to mapOf( "name" to null, @@ -135,7 +133,6 @@ class CustomScalarInputTest { """ } - // the configured (de)serializers aren't used, but the inclusion setting is assertEquals(resolver.received["map"], mapOf("date" to LocalDate.of(2020, 1, 2))) } @@ -161,8 +158,6 @@ class CustomScalarInputTest { """ } - // a box of a date can't be converted, so the deserializer gets it embedded and has no serialized form to read. - // That doesn't change how a later box of text is read assertEquals(data, mapOf( "payment" to "MoneyDto(amount=5, currency=CAD) OpaqueDto(value=ref) text", "withDate" to "MoneyDto(amount=5, currency=CAD) OpaqueDto(value=ref) 2020-01-02 text", @@ -279,7 +274,6 @@ class CustomScalarInputTest { fun mapList(inputs: List>) = receive("mapList", inputs) fun optionalMap(input: Optional>) = receive("optionalMap", input) - // the signature from https://github.com/graphql-java-kickstart/graphql-java-tools/issues/424 fun rawMap(input: LinkedHashMap<*, *>) = receive("rawMap", input) private fun receive(field: String, input: Any): Boolean { @@ -309,7 +303,6 @@ class CustomScalarInputTest { var box: String? = null } - // serialized as {"value": ...}, which Jackson can't create it from, like the coerced values of many custom scalars class Opaque(text: String) { val value = text } @@ -320,10 +313,8 @@ class CustomScalarInputTest { data class MoneyDto(val amount: Int, val currency: String) - // only converts if its content does data class Box(val content: Any) - // reads the serialized form, and an embedded Box wouldn't fail it class BoxContentDeserializer : StdDeserializer(String::class.java) { override fun deserialize(p: JsonParser, context: DeserializationContext): String = p.codec.readTree(p).path("content").textValue() ?: "" @@ -333,7 +324,6 @@ class CustomScalarInputTest { override fun deserialize(p: JsonParser, context: DeserializationContext): LocalDate = LocalDate.parse(p.text).plusDays(1) } - // the workaround from https://github.com/graphql-java-kickstart/graphql-java-tools/issues/466 class PartSerializer : StdSerializer(Part::class.java) { override fun serialize(value: Part, gen: JsonGenerator, provider: SerializerProvider) { gen.writeStartObject() diff --git a/src/test/kotlin/graphql/kickstart/tools/SchemaClassScannerSealedTypesTest.kt b/src/test/kotlin/graphql/kickstart/tools/SchemaClassScannerSealedTypesTest.kt index 6efac0f5..9fa50c27 100644 --- a/src/test/kotlin/graphql/kickstart/tools/SchemaClassScannerSealedTypesTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/SchemaClassScannerSealedTypesTest.kt @@ -122,7 +122,6 @@ class SchemaClassScannerSealedTypesTest { sealed class Mammal : Animal - // a subclass of both Animal and Mammal, so it's found twice among the subclasses of Animal class Dog(override val name: String?, val barks: Boolean?) : Mammal(), Animal class Cat(override val name: String?, val lives: Int?) : Animal @@ -225,7 +224,6 @@ class SchemaClassScannerSealedTypesTest { assert(message.contains("- ${ConflictingSealedSubclass.AuthorDto::class.java.name}:")) { message } } - // Author isn't a subclass of SearchResult, so it's only found by scanning Book class MemberReachableThroughSealedSubclass : GraphQLQueryResolver { fun results(): List = listOf() @@ -236,7 +234,6 @@ class SchemaClassScannerSealedTypesTest { class Author(val name: String?) } - // scanning Book finds another class for Author than its sealed subclass class ConflictingSealedSubclass : GraphQLQueryResolver { fun results(): List = listOf() @@ -269,7 +266,6 @@ class SchemaClassScannerSealedTypesTest { sealed class Pet { class NormalPet(val name: String?) : Pet() - // no classification, so the build fails if this is scanned instead of the class in the dictionary open class WeirdPet : Pet() } diff --git a/src/test/kotlin/graphql/kickstart/tools/SchemaParserDictionaryTest.kt b/src/test/kotlin/graphql/kickstart/tools/SchemaParserDictionaryTest.kt index fffc1fbd..b5938d96 100644 --- a/src/test/kotlin/graphql/kickstart/tools/SchemaParserDictionaryTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/SchemaParserDictionaryTest.kt @@ -41,7 +41,6 @@ class SchemaParserDictionaryTest { }) .dictionary("EntityCreateInput", ObjectNode::class.java) .dictionary("EntityUpdateInput", ObjectNode::class.java) - // ObjectNode has no properties, so the classes of the nested inputs come from the dictionary .dictionary("NestedA", ObjectNode::class.java) .dictionary("NestedB", ObjectNode::class.java) .build() @@ -69,7 +68,6 @@ class SchemaParserDictionaryTest { 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( """ @@ -102,7 +100,6 @@ class SchemaParserDictionaryTest { @Test fun `input type and object type can share a dictionary class`() { - // DogInput is only used by a directive, so its class can only come from the dictionary val schema = SchemaParser.newParser() .schemaString( """ @@ -183,7 +180,6 @@ class SchemaParserDictionaryTest { .build() } - // the input type sharing the class isn't part of the conflict assertEquals(error.message, "Class in supplied dictionary '${ThirdItem::class.java.name}' specified type names 'A', 'B', " + "but only input object types can share a class!") } diff --git a/src/test/kotlin/graphql/kickstart/tools/TypeExtensionTest.kt b/src/test/kotlin/graphql/kickstart/tools/TypeExtensionTest.kt index bcd0bcb4..87132bd6 100644 --- a/src/test/kotlin/graphql/kickstart/tools/TypeExtensionTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/TypeExtensionTest.kt @@ -33,7 +33,6 @@ class TypeExtensionTest { union FooBar = Foo | Bar extend union FooBar = Baz - # the order doesn't matter extend enum Color { GREEN } enum Color { RED } @@ -93,7 +92,6 @@ class TypeExtensionTest { @Test fun `union extensions can add nested unions`() { - // BarBaz has a field of its own, since the scanner only expands nested unions it finds some other way val schema = parser( """ type Query { @@ -224,7 +222,6 @@ class TypeExtensionTest { search(filter: Filter): String } - # graphql-java rejects the schema for this, here the extension's @once is dropped extend type Query @once(name: "Query extension") interface Node @tag(name: "Node") { id: ID! } @@ -260,7 +257,6 @@ class TypeExtensionTest { "Color" to listOf("@tag(Color)", "@tag(Color extension)"), "Filter" to listOf("@tag(Filter)", "@tag(Filter extension)", "@once(Filter extension)") )) - // each applied directive is wired once assertEquals(wiring.wired.sorted(), applied.values.flatten().sorted()) val item = schema.getType("Item") as GraphQLObjectType @@ -288,7 +284,6 @@ class TypeExtensionTest { assertEquals(error.message, "Found applied directive key without corresponding directive definition.") - // they may be repeatable, like federation's @key val schema = parser( """ type Query @key(fields: "upc") { today: String } @@ -326,7 +321,6 @@ class TypeExtensionTest { @Test fun `repeats and extensions without a type keep building`() { - // these built before extensions were merged, so the repeats only log a warning val schema = parser( """ type Query { diff --git a/src/test/kotlin/graphql/kickstart/tools/UnmappedInputFieldsFixtures.java b/src/test/kotlin/graphql/kickstart/tools/UnmappedInputFieldsFixtures.java index 3ba4faf8..8cf3683e 100644 --- a/src/test/kotlin/graphql/kickstart/tools/UnmappedInputFieldsFixtures.java +++ b/src/test/kotlin/graphql/kickstart/tools/UnmappedInputFieldsFixtures.java @@ -16,7 +16,6 @@ // Input classes as they're written in Java, by hand or with Lombok, so their properties aren't named by the Kotlin module. public class UnmappedInputFieldsFixtures { - // the class from https://github.com/graphql-java-kickstart/graphql-java-tools/issues/77#issuecomment-662855626 public static class ProjectInput { private Integer id; private String name; diff --git a/src/test/kotlin/graphql/kickstart/tools/UnmappedInputFieldsTest.kt b/src/test/kotlin/graphql/kickstart/tools/UnmappedInputFieldsTest.kt index 7bb65261..1069304a 100644 --- a/src/test/kotlin/graphql/kickstart/tools/UnmappedInputFieldsTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/UnmappedInputFieldsTest.kt @@ -43,7 +43,6 @@ class UnmappedInputFieldsTest { assertEquals(warnings, listOf(TECHNOLOGIES_MESSAGE)) - // the warning predicts what happens when a client sends the field val result = GraphQL.newGraphQL(schema).build().execute( """ mutation { @@ -466,7 +465,6 @@ class UnmappedInputFieldsTest { .makeExecutableSchema() } - // the scanner can't find a getter for SetterOnlyInput.nested either, but that's no reason to warn assertEquals(warnings, emptyList()) val data = assertNoGraphQlErrors(GraphQL.newGraphQL(schema).build()) { @@ -552,7 +550,6 @@ class UnmappedInputFieldsTest { fun caseInsensitive(input: CaseInsensitiveInput): String? = input.name fun propertyIgnorals(input: PropertyIgnoralsInput): String = "${input.inner?.name}, ${input.optionalInner?.get()?.name}" fun filter(input: RequestFilter): String? = input.and?.single()?.or?.single()?.name - // the fields are the map's keys, not properties of its values fun beanMap(input: Map): String = input.mapValues { it.value.name }.toString() }) .options(failOptions().build()) @@ -640,7 +637,6 @@ class UnmappedInputFieldsTest { is Circle -> "circle ${input.radius}" is Square -> "square ${input.side}" } - // the base classes are concrete, so their properties are found, but not their subclasses' fun animals(input: List): String = input.joinToString { it.describe() } fun animal(input: Optional): String = input.map { it.describe() }.orElse("none") fun zoo(input: Zoo): String = @@ -720,7 +716,6 @@ class UnmappedInputFieldsTest { .objectMapperConfigurer { mapper, _ -> mapper.setInjectableValues(InjectableValues.Std().addValue("tenant", "acme")) } .build()) .build() - // the parameter of 'generic' is a type variable, which isn't resolved to its class here, nor when the argument is converted .makeExecutableSchema() val result = GraphQL.newGraphQL(schema).build().execute( @@ -732,7 +727,6 @@ class UnmappedInputFieldsTest { } """) - // neither injecting values nor a builder works with the placeholder object, so the fields are never read assertEquals(result.errors.map { it.path }, listOf(listOf("injected"), listOf("builder"))) result.errors.forEach { assertContains(it.message, "Unrecognized field \"name\"") } // Jackson drops what none of the unwrapped properties know @@ -771,7 +765,6 @@ class UnmappedInputFieldsTest { .build() .makeExecutableSchema() - // the lookups only fail once the arguments are converted, as they did before the check val result = GraphQL.newGraphQL(schema).build().execute("query { broken(input: { name: \"a\" }) }") assertContains(result.errors.single().message, "Conflicting property-based creators") } @@ -816,7 +809,6 @@ class UnmappedInputFieldsTest { "Input field '$field' has no matching property in ${clazz.name}. Converting $usage $consequence. Known properties: $knownProperties. " + "Rename the property or map it with @JsonProperty(\"${field.substringAfter('.')}\"). If it's left unmapped on purpose, list it in @JsonIgnoreProperties on the class." - // the warnings logged by any logger while building private fun withWarnings(build: () -> T): Pair> { // the tests run in the library's module, which doesn't read logback's listOf("ch.qos.logback.classic", "ch.qos.logback.core").forEach { name -> @@ -959,7 +951,6 @@ class UnmappedInputFieldsTest { override fun describe() = "dog $name barks=$barks" } - // only polymorphic where a property says so open class Pet { var name: String? = null @@ -981,7 +972,6 @@ class UnmappedInputFieldsTest { var pets: List? = null } - // the property doesn't match the schema, only the deserializer does class Money(val amount: Int) class MoneyDeserializer : StdDeserializer(Money::class.java) { @@ -1017,7 +1007,6 @@ class UnmappedInputFieldsTest { var stret: String? = null } - // like a class that refers to one missing from the classpath class MissingClassDeserializers : SimpleDeserializers() { override fun findBeanDeserializer(type: JavaType, config: DeserializationConfig, beanDesc: BeanDescription): JsonDeserializer<*> = throw NoClassDefFoundError("com/example/Missing")