Merge pull request #916 from graphql-java-kickstart/feature/863-reject-repeated-extensions

Reject fields, values and members that a type and its extensions declare twice

Author: oryan-block

src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt

@@ -173,7 +173,7 @@ class SchemaParser internal constructor(
         val extensions = typeExtensions.extensionsOf<ObjectTypeExtensionDefinition>(name)
         val directives = sdlDirectives.directivesOf(objectDefinition, extensions)
         val fieldDefinitions = objectDefinition.withExtensions(typeExtensions).flatMap { it.fieldDefinitions }
-        warnRepeatedFields(name, fieldDefinitions)
+        rejectRepeated(name, "field", fieldDefinitions)
         val builder = GraphQLObjectType.newObject()
             .name(name)
             .definition(objectDefinition)
@@ -212,8 +212,8 @@ class SchemaParser internal constructor(
                                   inputObjectsInProgress: MutableSet<String>): GraphQLInputObjectType {
         val extensionDefinitions = typeExtensions.extensionsOf<InputObjectTypeExtensionDefinition>(definition.name)
         val directives = sdlDirectives.directivesOf(definition, extensionDefinitions)
-        val fieldDefinitions = (extensionDefinitions + definition).flatMap { it.inputValueDefinitions }
-        warnRepeatedFields(definition.name, fieldDefinitions)
+        val inputValueDefinitions = definition.withExtensions(typeExtensions).flatMap { it.inputValueDefinitions }
+        rejectRepeated(definition.name, "field", inputValueDefinitions)
 
         inputObjectsInProgress.add(definition.name)
 
@@ -224,7 +224,7 @@ class SchemaParser internal constructor(
             .description(getDocumentation(definition, options))
             .withSdlDirectives(directives, INPUT_OBJECT)
             .apply {
-                fieldDefinitions.forEach { fieldDefinition ->
+                inputValueDefinitions.forEach { fieldDefinition ->
                     field(
                         GraphQLInputObjectField.newInputObjectField()
                             .name(fieldDefinition.name)
@@ -252,15 +252,15 @@ class SchemaParser internal constructor(
         val extensions = typeExtensions.extensionsOf<EnumTypeExtensionDefinition>(name)
         val directives = sdlDirectives.directivesOf(definition, extensions)
         val values = definition.withExtensions(typeExtensions).flatMap { it.enumValueDefinitions }
-        warnRepeated(name, "value", values.map { it.name })
+        rejectRepeated(name, "value", values)
         val builder = GraphQLEnumType.newEnum()
             .name(name)
             .definition(definition)
             .extensionDefinitions(extensions)
             .description(getDocumentation(definition, options))
             .withSdlDirectives(directives, ENUM)
             .apply {
-                values.distinctBy { it.name }.forEach { valueDefinition ->
+                values.forEach { valueDefinition ->
                     val enumName = valueDefinition.name
                     val enumValue = if (enumClass == null) {
                         enumName
@@ -306,15 +306,15 @@ class SchemaParser internal constructor(
         val extensions = typeExtensions.extensionsOf<InterfaceTypeExtensionDefinition>(interfaceDefinition.name)
         val directives = sdlDirectives.directivesOf(interfaceDefinition, extensions)
         val fieldDefinitions = interfaceDefinition.withExtensions(typeExtensions).flatMap { it.fieldDefinitions }
-        warnRepeated(interfaceDefinition.name, "field", fieldDefinitions.map { it.name })
+        rejectRepeated(interfaceDefinition.name, "field", fieldDefinitions)
         val builder = GraphQLInterfaceType.newInterface()
             .name(interfaceDefinition.name)
             .definition(interfaceDefinition)
             .extensionDefinitions(extensions)
             .description(getDocumentation(interfaceDefinition, options))
             .withSdlDirectives(directives, INTERFACE)
             .apply {
-                fieldDefinitions.distinctBy { it.name }.forEach { fieldDefinition ->
+                fieldDefinitions.forEach { fieldDefinition ->
                     field { field -> createField(field, fieldDefinition, inputObjects) }
                 }
             }
@@ -331,7 +331,7 @@ class SchemaParser internal constructor(
     private fun createUnionObject(definition: UnionTypeDefinition, types: List<GraphQLObjectType>): GraphQLUnionType {
         val extensions = typeExtensions.extensionsOf<UnionTypeExtensionDefinition>(definition.name)
         val directives = sdlDirectives.directivesOf(definition, extensions)
-        warnRepeated(definition.name, "member", definition.withExtensions(typeExtensions).flatMap { it.memberTypes }.map { (it as TypeName).name })
+        rejectRepeated(definition.name, "member", definition.withExtensions(typeExtensions).flatMap { it.memberTypes }.map { it as TypeName })
         val builder = GraphQLUnionType.newUnionType()
             .name(definition.name)
             .definition(definition)
@@ -373,21 +373,10 @@ class SchemaParser internal constructor(
         return leafObjects
     }
 
-    // TODO throw a SchemaError instead in 15.0.0 (#863), like graphql-java does
-    private fun warnRepeated(typeName: String, kind: String, names: List<String?>) {
-        names.groupingBy { it }.eachCount().filterValues { it > 1 }.keys.forEach {
-            log.warn("Type '$typeName' declares $kind '$it' more than once, only the first one is kept.")
-        }
-    }
-
-    // TODO throw a SchemaError instead in 15.0.0 (#865), like graphql-java does
-    private fun warnRepeatedFields(typeName: String, fieldDefinitions: List<NamedNode<*>>) {
-        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 rejectRepeated(typeName: String, kind: String, declarations: List<NamedNode<*>>) {
+        val names = mutableSetOf<String?>()
+        val repeat = declarations.firstOrNull { !names.add(it.name) } ?: return
+        throw SchemaError("Type '$typeName' declares $kind '${repeat.name}' more than once${repeat.sourceLocation.describe()}.")
     }
 
     private fun createField(field: GraphQLFieldDefinition.Builder, fieldDefinition: FieldDefinition, inputObjects: List<GraphQLInputObjectType>): GraphQLFieldDefinition.Builder {