Merge pull request #878 from graphql-java-kickstart/test/867-log-capture

Test the schema warnings through a shared log-capture helper

Author: oryan-block

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<String>) = 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`() {

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 <T> assertNull(actual: T) {
 fun <T> assertNotNull(actual: T) {
     assert(actual != null)
 }
+
+fun <T> withWarnings(block: () -> T): Pair<T, List<String>> {
+    // 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<ILoggingEvent>().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)
+    }
+}

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

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 <T> withWarnings(build: () -> T): Pair<T, List<String>> {
-        // 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<ILoggingEvent>().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
     }