diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt index e3638fab..cbeafd38 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt @@ -172,6 +172,8 @@ class SchemaParser internal constructor( val name = objectDefinition.name val extensions = typeExtensions.extensionsOf(name) val directives = sdlDirectives.directivesOf(objectDefinition, extensions) + val fieldDefinitions = objectDefinition.withExtensions(typeExtensions).flatMap { it.fieldDefinitions } + warnRepeatedFields(name, fieldDefinitions) val builder = GraphQLObjectType.newObject() .name(name) .definition(objectDefinition) @@ -186,7 +188,7 @@ class SchemaParser internal constructor( } } .apply { - objectDefinition.withExtensions(typeExtensions).flatMap { it.fieldDefinitions }.forEach { fieldDefinition -> + fieldDefinitions.forEach { fieldDefinition -> field { field -> createField(field, fieldDefinition, inputObjects) codeRegistryBuilder.dataFetcher( @@ -210,6 +212,8 @@ class SchemaParser internal constructor( inputObjectsInProgress: MutableSet): GraphQLInputObjectType { val extensionDefinitions = typeExtensions.extensionsOf(definition.name) val directives = sdlDirectives.directivesOf(definition, extensionDefinitions) + val fieldDefinitions = (extensionDefinitions + definition).flatMap { it.inputValueDefinitions } + warnRepeatedFields(definition.name, fieldDefinitions) inputObjectsInProgress.add(definition.name) @@ -220,20 +224,18 @@ class SchemaParser internal constructor( .description(getDocumentation(definition, options)) .withSdlDirectives(directives, INPUT_OBJECT) .apply { - (extensionDefinitions + definition).forEach { typeDefinition -> - typeDefinition.inputValueDefinitions.forEach { fieldDefinition -> - field( - GraphQLInputObjectField.newInputObjectField() - .name(fieldDefinition.name) - .definition(fieldDefinition) - .description(getDocumentation(fieldDefinition, options)) - .apply { fieldDefinition.defaultValue?.let { v -> defaultValueLiteral(v) } } - .apply { sdlDirectives.getDeprecated(fieldDefinition.directives)?.let { deprecate(it) } } - .type(determineInputType(fieldDefinition.type, inputObjects, inputObjectsInProgress)) - .withSdlDirectives(fieldDefinition.directives, INPUT_FIELD_DEFINITION) - .build() - ) - } + fieldDefinitions.forEach { fieldDefinition -> + field( + GraphQLInputObjectField.newInputObjectField() + .name(fieldDefinition.name) + .definition(fieldDefinition) + .description(getDocumentation(fieldDefinition, options)) + .apply { fieldDefinition.defaultValue?.let { v -> defaultValueLiteral(v) } } + .apply { sdlDirectives.getDeprecated(fieldDefinition.directives)?.let { deprecate(it) } } + .type(determineInputType(fieldDefinition.type, inputObjects, inputObjectsInProgress)) + .withSdlDirectives(fieldDefinition.directives, INPUT_FIELD_DEFINITION) + .build() + ) } } @@ -383,6 +385,16 @@ class SchemaParser internal constructor( } } + // TODO throw a SchemaError instead in 15.0.0 (#865), like graphql-java does + private fun warnRepeatedFields(typeName: String, fieldDefinitions: List>) { + fieldDefinitions.groupBy { it.name }.values.filter { it.size > 1 }.forEach { declarations -> + val kept = declarations.last() + val location = kept.sourceLocation.describe() + val which = if (location.isEmpty()) "one of them" else "the one$location" + log.warn("Type '$typeName' declares field '${kept.name}' more than once, only $which is kept.") + } + } + private fun createField(field: GraphQLFieldDefinition.Builder, fieldDefinition: FieldDefinition, inputObjects: List): GraphQLFieldDefinition.Builder { return field .name(fieldDefinition.name) diff --git a/src/test/kotlin/graphql/kickstart/tools/TypeExtensionTest.kt b/src/test/kotlin/graphql/kickstart/tools/TypeExtensionTest.kt index 9012e67f..c6c6acb2 100644 --- a/src/test/kotlin/graphql/kickstart/tools/TypeExtensionTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/TypeExtensionTest.kt @@ -2,10 +2,13 @@ package graphql.kickstart.tools import graphql.GraphQL import graphql.Scalars +import graphql.language.Definition import graphql.language.EnumTypeDefinition +import graphql.language.FieldDefinition import graphql.language.InterfaceTypeDefinition import graphql.language.ObjectTypeDefinition import graphql.language.ScalarTypeDefinition +import graphql.language.TypeName import graphql.language.UnionTypeDefinition import graphql.schema.* import graphql.schema.idl.SchemaDirectiveWiring @@ -393,6 +396,135 @@ class TypeExtensionTest { assertEquals((schema.getType("FooBar") as GraphQLUnionType).types.map { it.name }, listOf("Foo", "Bar", "Baz")) } + @Test + fun `repeated fields of objects and inputs keep building and keep the last one`() { + assertRepeatedObjectField( + """ + type Item implements Node { + id: ID! + "From the type" + name: String + } + extend type Item { + "From the extension" + name: String + } + """, + location = "at line 9, column 5", + kept = "From the extension" + ) + assertRepeatedObjectField( + """ + type Item implements Node { id: ID! } + extend type Item { + "From the first extension" + name: String + } + extend type Item { + "From the second extension" + name: String + } + """, + location = "at line 9, column 5", + kept = "From the second extension" + ) + assertRepeatedObjectField( + """ + type Item implements Node { + id: ID! + "The first" + name: String + "The second" + name: String + } + """, + location = "at line 7, column 5", + kept = "The second" + ) + + assertRepeatedInputField( + "Filter", + """ + input Filter { + "From the type" + id: ID + } + extend input Filter { + "From the extension" + id: ID + } + """, + location = "at line 3, column 5", + kept = "From the type" + ) + assertRepeatedInputField( + "Filter", + """ + input Filter { term: String } + extend input Filter { + "From the first extension" + id: ID + } + extend input Filter { + "From the second extension" + id: ID + } + """, + location = "at line 8, column 5", + kept = "From the second extension" + ) + assertRepeatedInputField( + "Filter", + """ + input Filter { + "The first" + id: ID + "The second" + id: ID + } + """, + location = "at line 5, column 5", + kept = "The second" + ) + assertRepeatedInputField( + "Inner", + """ + input Filter { + inner: Inner + } + input Inner { + "The first" + id: ID + "The second" + id: ID + } + """, + location = "at line 8, column 5", + kept = "The second" + ) + } + + @Test + fun `a repeated field without a source location says only that one is kept`() { + fun field(name: String) = FieldDefinition.newFieldDefinition().name(name).type(TypeName("String")).build() + val item = ObjectTypeDefinition.newObjectTypeDefinition() + .name("Item") + .implementz(TypeName("Node")) + .fieldDefinitions(listOf(field("id"), field("name"), field("name"))) + .build() + + val (_, warnings) = withWarnings { + parser("type Query { node: Node }\ninterface Node { id: String }") { + dictionary(Item::class) + options(SchemaParserOptions.newOptions().typeDefinitionFactory(object : TypeDefinitionFactory { + override fun create(existing: MutableList>) = listOf(item) + }).build()) + }.makeExecutableSchema() + } + + assertEquals(warnings, listOf("Type 'Item' declares field 'name' more than once, only one of them is kept.")) + } + @Test fun `extensions of a different kind than the type are ignored`() { val (schema, warnings) = withWarnings { @@ -473,6 +605,26 @@ class TypeExtensionTest { assertEquals(schema.getSchemaAppliedDirectives("link").map { it.getArgument("for")?.getValue() }, listOf("EXECUTION")) } + private fun assertRepeatedObjectField(item: String, location: String, kept: String) { + val (schema, warnings) = withWarnings { + parser("type Query { node: Node }\ninterface Node { id: ID! }\n${item.trimIndent()}") { + dictionary(Item::class) + }.makeExecutableSchema() + } + + assertEquals(warnings, listOf("Type 'Item' declares field 'name' more than once, only the one $location is kept.")) + assertEquals((schema.getType("Item") as GraphQLObjectType).getFieldDefinition("name").description, kept) + } + + private fun assertRepeatedInputField(repeating: String, filter: String, location: String, kept: String) { + val (schema, warnings) = withWarnings { + parser("type Query { search(filter: Filter): String }\n${filter.trimIndent()}").makeExecutableSchema() + } + + assertEquals(warnings, listOf("Type '$repeating' declares field 'id' more than once, only the one $location is kept.")) + assertEquals((schema.getType(repeating) as GraphQLInputObjectType).getField("id").description, kept) + } + private val fooBarBaz = """ type Foo { foo: String } type Bar { bar: String } @@ -502,7 +654,9 @@ class TypeExtensionTest { class Item(val id: String, val name: String?, val fooBar: Foo? = null) : Node - class Filter(val id: String?) + class Filter(val id: String?, val term: String? = null, val inner: Inner? = null) + + class Inner(val id: String?) enum class Color { RED, GREEN }