diff --git a/src/test/kotlin/graphql/kickstart/tools/MissingFieldResolverTest.kt b/src/test/kotlin/graphql/kickstart/tools/MissingFieldResolverTest.kt index c3727952..a4962f67 100644 --- a/src/test/kotlin/graphql/kickstart/tools/MissingFieldResolverTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/MissingFieldResolverTest.kt @@ -29,6 +29,33 @@ class MissingFieldResolverTest { .makeExecutableSchema() } + @Test + fun `should warn about each missing field when unimplemented resolvers are allowed`() { + val (schema, warnings) = withWarnings { + SchemaParser.newParser() + .schemaString( + """ + type Query { + implementedField(input: String): String + missingField(input: Int): Int + } + """ + ) + .resolvers(object : GraphQLQueryResolver { + fun implementedField(input: Optional) = input.toString() + }) + .options(SchemaParserOptions.newOptions().allowUnimplementedResolvers(true).build()) + .build() + .makeExecutableSchema() + } + + assertEquals(warnings, listOf( + "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.", + "Missing resolver for field: ${schema.queryType.getFieldDefinition("missingField").definition}" + )) + } + @Test @Suppress("DEPRECATION") fun `should call missing resolver data fetcher if provided`() { diff --git a/src/test/kotlin/graphql/kickstart/tools/TestUtils.kt b/src/test/kotlin/graphql/kickstart/tools/TestUtils.kt index 7aa9a396..98a78d47 100644 --- a/src/test/kotlin/graphql/kickstart/tools/TestUtils.kt +++ b/src/test/kotlin/graphql/kickstart/tools/TestUtils.kt @@ -1,8 +1,13 @@ package graphql.kickstart.tools +import ch.qos.logback.classic.Level +import ch.qos.logback.classic.Logger +import ch.qos.logback.classic.spi.ILoggingEvent +import ch.qos.logback.core.read.ListAppender import com.fasterxml.jackson.databind.ObjectMapper import graphql.ExecutionInput import graphql.GraphQL +import org.slf4j.LoggerFactory private val mapper = ObjectMapper() @@ -37,3 +42,19 @@ fun assertNull(actual: T) { fun assertNotNull(actual: T) { assert(actual != null) } + +fun withWarnings(block: () -> 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 -> + ModuleLayer.boot().findModule(name).ifPresent { SchemaParser::class.java.module.addReads(it) } + } + val logger = LoggerFactory.getLogger("graphql.kickstart.tools") as Logger + val appender = ListAppender().apply { start() } + logger.addAppender(appender) + try { + val result = block() + return result to appender.list.filter { it.level.isGreaterOrEqual(Level.WARN) }.map { it.formattedMessage } + } finally { + logger.detachAppender(appender) + } +} diff --git a/src/test/kotlin/graphql/kickstart/tools/TypeExtensionTest.kt b/src/test/kotlin/graphql/kickstart/tools/TypeExtensionTest.kt index 87132bd6..d89194f6 100644 --- a/src/test/kotlin/graphql/kickstart/tools/TypeExtensionTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/TypeExtensionTest.kt @@ -210,41 +210,45 @@ class TypeExtensionTest { @Test fun `directives of extensions follow those of the type and are wired`() { val wiring = RecordingWiring() - val schema = parser( - """ - directive @tag(name: String!) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT - directive @once(name: String!) on OBJECT | INPUT_OBJECT + val (schema, warnings) = withWarnings { + parser( + """ + directive @tag(name: String!) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT + directive @once(name: String!) on OBJECT | INPUT_OBJECT - type Query @once(name: "Query") { - node: Node - fooBar: FooBar - color: Color - search(filter: Filter): String - } + type Query @once(name: "Query") { + node: Node + fooBar: FooBar + color: Color + search(filter: Filter): String + } - extend type Query @once(name: "Query extension") + extend type Query @once(name: "Query extension") - interface Node @tag(name: "Node") { id: ID! } - extend interface Node @tag(name: "Node extension") + interface Node @tag(name: "Node") { id: ID! } + extend interface Node @tag(name: "Node extension") - type Item implements Node @tag(name: "Item") { id: ID! } - extend type Item @tag(name: "Item extension") { name: String } + type Item implements Node @tag(name: "Item") { id: ID! } + extend type Item @tag(name: "Item extension") { name: String } - union FooBar @tag(name: "FooBar") = Foo - extend union FooBar @tag(name: "FooBar extension") + union FooBar @tag(name: "FooBar") = Foo + extend union FooBar @tag(name: "FooBar extension") - enum Color @tag(name: "Color") { RED GREEN } - extend enum Color @tag(name: "Color extension") + enum Color @tag(name: "Color") { RED GREEN } + extend enum Color @tag(name: "Color extension") - input Filter @tag(name: "Filter") { id: ID } - extend input Filter @tag(name: "Filter extension") @once(name: "Filter extension") + input Filter @tag(name: "Filter") { id: ID } + extend input Filter @tag(name: "Filter extension") @once(name: "Filter extension") - type Foo { foo: String } - """) { - dictionary(Item::class, Foo::class) - directive("tag", wiring) - directive("once", wiring) - }.makeExecutableSchema() + type Foo { foo: String } + """) { + dictionary(Item::class, Foo::class) + directive("tag", wiring) + directive("once", wiring) + }.makeExecutableSchema() + } + + assertEquals(warnings, listOf("Directive @once isn't repeatable but an extension of type 'Query' applies it again, only the first one is kept.")) val applied = listOf("Query", "Node", "Item", "FooBar", "Color", "Filter").associateWith { name -> (schema.getType(name) as GraphQLDirectiveContainer).appliedDirectives.map { it.describe() } @@ -304,12 +308,15 @@ class TypeExtensionTest { $fooBarBaz """ - assertEquals(parser(unusedUnion).getUnusedDefinitions().map { it.javaClass.simpleName to it.name }.toSet(), setOf( + val (unused, warnings) = withWarnings { parser(unusedUnion).getUnusedDefinitions() } + + assertEquals(unused.map { it.javaClass.simpleName to it.name }.toSet(), setOf( "UnionTypeDefinition" to "FooBar", "ObjectTypeDefinition" to "Foo", "ObjectTypeDefinition" to "Bar", "ObjectTypeDefinition" to "Baz" )) + assertEquals(warnings, listOf("FooBar", "Foo", "Bar", "Baz").map { "Schema type was defined but can never be accessed, and can be safely deleted: $it" }) val schema = parser(unusedUnion) { dictionary(Foo::class, Bar::class, Baz::class) @@ -321,35 +328,43 @@ class TypeExtensionTest { @Test fun `repeats and extensions without a type keep building`() { - val schema = parser( - """ - type Query { - color: Color - node: Node - fooBar: FooBar - } + val (schema, warnings) = withWarnings { + parser( + """ + type Query { + color: Color + node: Node + fooBar: FooBar + } - extend enum Color { - "The first red" - RED - } - extend enum Color { RED GREEN } + extend enum Color { + "The first red" + RED + } + extend enum Color { RED GREEN } - interface Node { - "The base id" - id: ID! - } - extend interface Node { id: ID! name: String } + interface Node { + "The base id" + id: ID! + } + extend interface Node { id: ID! name: String } - type Item implements Node { id: ID! name: String } + type Item implements Node { id: ID! name: String } - extend union FooBar = Foo | Bar - extend union FooBar = Bar | Baz + extend union FooBar = Foo | Bar + extend union FooBar = Bar | Baz - $fooBarBaz - """) { - dictionary(Item::class, Foo::class, Bar::class, Baz::class) - }.makeExecutableSchema() + $fooBarBaz + """) { + dictionary(Item::class, Foo::class, Bar::class, Baz::class) + }.makeExecutableSchema() + } + + assertEquals(warnings, listOf( + "Type 'Node' declares field 'id' more than once, only the first one is kept.", + "Type 'FooBar' declares member 'Bar' more than once, only the first one is kept.", + "Type 'Color' declares value 'RED' more than once, only the first one is kept." + )) val color = schema.getType("Color") as GraphQLEnumType assertEquals(color.values.map { it.name }, listOf("RED", "GREEN")) @@ -364,20 +379,28 @@ class TypeExtensionTest { @Test fun `extensions of a different kind than the type are ignored`() { - val schema = parser( - """ - type Query { node: Node } + val (schema, warnings) = withWarnings { + parser( + """ + type Query { node: Node } - interface Node { id: ID! } - extend type Node { name: String } + interface Node { id: ID! } + extend type Node { name: String } - type Item implements Node { id: ID! } - extend union Item = Foo + type Item implements Node { id: ID! } + extend union Item = Foo - type Foo { foo: String } - """) { - dictionary(Item::class) - }.makeExecutableSchema() + type Foo { foo: String } + """) { + dictionary(Item::class) + }.makeExecutableSchema() + } + + assertEquals(warnings, listOf( + "Type 'Node' is defined as an interface, but extended as an object type. The extension is ignored.", + "Type 'Item' is defined as an object type, but extended as a union. The extension is ignored.", + "Schema type was defined but can never be accessed, and can be safely deleted: Foo" + )) val node = schema.getType("Node") as GraphQLInterfaceType val item = schema.getType("Item") as GraphQLObjectType diff --git a/src/test/kotlin/graphql/kickstart/tools/UnmappedInputFieldsTest.kt b/src/test/kotlin/graphql/kickstart/tools/UnmappedInputFieldsTest.kt index 1069304a..da8f5acd 100644 --- a/src/test/kotlin/graphql/kickstart/tools/UnmappedInputFieldsTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/UnmappedInputFieldsTest.kt @@ -1,9 +1,5 @@ package graphql.kickstart.tools -import ch.qos.logback.classic.Level -import ch.qos.logback.classic.Logger -import ch.qos.logback.classic.spi.ILoggingEvent -import ch.qos.logback.core.read.ListAppender import com.fasterxml.jackson.annotation.JacksonInject import com.fasterxml.jackson.annotation.JsonCreator import com.fasterxml.jackson.annotation.JsonFormat @@ -32,7 +28,6 @@ import graphql.kickstart.tools.UnmappedInputFieldsFixtures.* import graphql.schema.DataFetchingEnvironment import org.junit.Assert.assertThrows import org.junit.Test -import org.slf4j.LoggerFactory import java.util.* class UnmappedInputFieldsTest { @@ -809,22 +804,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." - 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 -> - ModuleLayer.boot().findModule(name).ifPresent { javaClass.module.addReads(it) } - } - val logger = LoggerFactory.getLogger(org.slf4j.Logger.ROOT_LOGGER_NAME) as Logger - val appender = ListAppender().apply { start() } - logger.addAppender(appender) - try { - val result = build() - return result to appender.list.filter { it.level.isGreaterOrEqual(Level.WARN) }.map { it.formattedMessage } - } finally { - logger.detachAppender(appender) - } - } - class GreetingInput { var nmae: String? = null }