Skip to content

fix: prevent duplicate field names through configure #787

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
stuebingerb merged 1 commit into from
Nov 13, 2026
Merged

fix: prevent duplicate field names through configure #787

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
151 changes: 74 additions & 77 deletions kgraphql/src/main/kotlin/de/stuebingerb/kgraphql/schema/structure/SchemaCompilation.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import de.stuebingerb.kgraphql.schema.SchemaException
import de.stuebingerb.kgraphql.schema.builtin.BuiltInScalars
import de.stuebingerb.kgraphql.schema.directive.Directive
import de.stuebingerb.kgraphql.schema.execution.Execution
import de.stuebingerb.kgraphql.schema.introspection.NotIntrospected
import de.stuebingerb.kgraphql.schema.introspection.SchemaProxy
import de.stuebingerb.kgraphql.schema.introspection.TypeKind
import de.stuebingerb.kgraphql.schema.introspection.__Schema
Expand All @@ -34,7 +33,6 @@ import kotlin.reflect.KProperty1
import kotlin.reflect.KType
import kotlin.reflect.KVisibility
import kotlin.reflect.full.createType
import kotlin.reflect.full.findAnnotation
import kotlin.reflect.full.isSubclassOf
import kotlin.reflect.full.isSuperclassOf
import kotlin.reflect.full.memberProperties
Expand Down Expand Up @@ -356,7 +354,8 @@ open class SchemaCompilation(
acc + def.transformations.mapKeys { (property) -> property.name }
}

val kotlinFields = kClass.memberProperties
val declaredFields = mutableListOf<Field>()
kClass.memberProperties
.filter { field -> field.visibility == KVisibility.PUBLIC }
.filterNot { field -> objectDefs.any { it.isIgnored(field.name) } }
.map { property ->
Expand All @@ -365,6 +364,8 @@ open class SchemaCompilation(
kqlProperty = allKotlinProperties[property.name],
transformation = allTransformations[property.name]
)
}.forEach {
declaredFields.addField(it)
}

val extensionFields = objectDefs
Expand All @@ -386,32 +387,20 @@ open class SchemaCompilation(
val __typenameField = typenameField(FunctionWrapper.on(typenameResolver, true))

// https://spec.graphql.org/October2021/#sec-Objects.Type-Validation
val declaredFields = kotlinFields.toMutableList<Field>()
extensionFields.forEach {
if (declaredFields.any { field -> field.name == it.name }) {
throw SchemaException("Cannot add extension field with duplicated name '${it.name}'")
}
declaredFields.add(it)
declaredFields.addField(it, "extension")
}
unionFields.forEach {
if (declaredFields.any { field -> field.name == it.name }) {
throw SchemaException("Cannot add union field with duplicated name '${it.name}'")
}
declaredFields.add(it)
declaredFields.addField(it, "union")
}
dataloadExtensionFields.forEach {
if (declaredFields.any { field -> field.name == it.name }) {
throw SchemaException("Cannot add dataloaded field with duplicated name '${it.name}'")
}
declaredFields.add(it)
declaredFields.addField(it, "dataloaded")
}

if (declaredFields.isEmpty()) {
throw SchemaException("An object type must define one or more fields. Found none on type '${objectDef.name}'")
}

declaredFields.forEach { validateName(it.name) }

val allFields = declaredFields + __typenameField
typeProxy.proxied = if (kind == TypeKind.OBJECT) {
Type.Object(objectDef, allFields)
Expand All @@ -421,6 +410,23 @@ open class SchemaCompilation(
return typeProxy
}

private fun MutableList<Field>.addField(field: Field, fieldType: String? = null) {
validateName(field.name)
if (any { it.name == field.name }) {
val fieldTypeString = fieldType?.let { "$it " } ?: ""
throw SchemaException("Cannot add ${fieldTypeString}field with duplicated name '${field.name}'")
}
add(field)
}

private fun MutableList<InputValue<*>>.addInputValue(value: InputValue<*>) {
validateName(value.name)
if (any { it.name == value.name }) {
throw SchemaException("Cannot add field with duplicated name '${value.name}'")
}
add(value)
}

private suspend fun handleInputType(kClass: KClass<*>): Type {
assertValidInputObjectType(kClass)

Expand All @@ -445,66 +451,60 @@ open class SchemaCompilation(
val typeProxy = TypeProxy(objectType)
inputTypeProxies[kClass] = typeProxy

val fieldNames = mutableSetOf<String>()
val fields = if (kClass.findAnnotation<NotIntrospected>() == null) {
if (kClass.isSealed) {
// OneOf input types have one field per allowed sub type
val sealedSubclasses = kClass.sealedSubclasses.toSet()
if (sealedSubclasses.isEmpty()) {
val type = if (kClass.java.isInterface) {
"interface"
} else {
"class"
}
throw SchemaException("The sealed $type '${kClass.simpleName}' does not have known subclasses")
}
sealedSubclasses.sortedBy { it.simpleName }.mapNotNull { clazz ->
val kProperty = inputObjectDef.oneOfProperties[clazz]

if (kProperty?.isIgnored == true) {
return@mapNotNull null
}

// All properties must be nullable
val type = handlePossiblyWrappedType(clazz.createType(nullable = true), TypeCategory.INPUT)
val parameterName = kProperty?.name ?: clazz.defaultOneOfPropertyName()
if (parameterName in fieldNames) {
throw SchemaException("Cannot add oneOf field with duplicated name '${parameterName}'")
}
fieldNames.add(parameterName)
InputValue(
InputValueDef(
kClass = clazz,
name = parameterName,
description = kProperty?.description,
isDeprecated = kProperty?.isDeprecated ?: false,
deprecationReason = kProperty?.deprecationReason,
parameterName = parameterName
),
type
)
val fields = mutableListOf<InputValue<*>>()

@cubic-dev-ai cubic-dev-ai Bot Nov 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The @NotIntrospected check in handleInputType was removed without explanation. Input classes with that annotation used to fail with "must define one or more fields", and now they compile into normal input types. This change is unrelated to duplicate-name detection, so either restore the check or document the intended behavior in the PR.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At kgraphql/src/main/kotlin/de/stuebingerb/kgraphql/schema/structure/SchemaCompilation.kt, line 454:

<comment>The `@NotIntrospected` check in `handleInputType` was removed without explanation. Input classes with that annotation used to fail with "must define one or more fields", and now they compile into normal input types. This change is unrelated to duplicate-name detection, so either restore the check or document the intended behavior in the PR.</comment>

<file context>
@@ -445,66 +451,60 @@ open class SchemaCompilation(
-                        ),
-                        type
-                    )
+        val fields = mutableListOf<InputValue<*>>()
+        if (kClass.isSealed) {
+            // OneOf input types have one field per allowed sub type
</file context>

@stuebingerb stuebingerb Nov 13, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous failure was also wrong/unhelpful, and classes annotated with @NotIntrospected were not handled anywhere else during schema compilation. This is a topic for another day.

if (kClass.isSealed) {
// OneOf input types have one field per allowed sub type
val sealedSubclasses = kClass.sealedSubclasses.toSet()
if (sealedSubclasses.isEmpty()) {
val type = if (kClass.java.isInterface) {
"interface"
} else {
"class"
}
} else {
val primaryConstructor = kClass.primaryConstructor
?: throw SchemaException("Java class '${kClass.simpleName}' as input type is not supported")
val memberPropertiesByName = kClass.memberProperties.associateBy { it.name }
// Input types are created using their primary constructor. Therefore, it makes sense to (only) use the
// parameters of this constructor for the fields (cf. https://github.com/stuebingerb/KGraphQL/issues/235).
// Member properties are sorted by name (https://youtrack.jetbrains.com/issue/KT-41042), and SDL is
// sorting by name, so we'll also sort constructor parameters to be consistent.
primaryConstructor.parameters.sortedBy { it.name }.map { parameter ->
// kProperty is used to configure deprecation and description in the DSL, so we use it
// if available
val kProperty = memberPropertiesByName[parameter.name]
handleKotlinInputProperty(
parameter = parameter,
kProperty = kProperty,
kqlProperty = kProperty?.let { inputObjectDef.kotlinProperties[it] }
)
throw SchemaException("The sealed $type '${kClass.simpleName}' does not have known subclasses")
}
sealedSubclasses.sortedBy { it.simpleName }.mapNotNull { clazz ->
val kProperty = inputObjectDef.oneOfProperties[clazz]

if (kProperty?.isIgnored == true) {
return@mapNotNull null
}

// All properties must be nullable
val type = handlePossiblyWrappedType(clazz.createType(nullable = true), TypeCategory.INPUT)
val parameterName = kProperty?.name ?: clazz.defaultOneOfPropertyName()
val inputValue = InputValue(
InputValueDef(
kClass = clazz,
name = parameterName,
description = kProperty?.description,
isDeprecated = kProperty?.isDeprecated ?: false,
deprecationReason = kProperty?.deprecationReason,
parameterName = parameterName
),
type
)
fields.addInputValue(inputValue)
}
} else {
listOf()
val primaryConstructor = kClass.primaryConstructor
?: throw SchemaException("Java class '${kClass.simpleName}' as input type is not supported")
val memberPropertiesByName = kClass.memberProperties.associateBy { it.name }
// Input types are created using their primary constructor. Therefore, it makes sense to (only) use the
// parameters of this constructor for the fields (cf. https://github.com/stuebingerb/KGraphQL/issues/235).
// Member properties are sorted by name (https://youtrack.jetbrains.com/issue/KT-41042), and SDL is
// sorting by name, so we'll also sort constructor parameters to be consistent.
primaryConstructor.parameters.sortedBy { it.name }.map { parameter ->
// kProperty is used to configure deprecation and description in the DSL, so we use it
// if available
val kProperty = memberPropertiesByName[parameter.name]
val inputValue = handleKotlinInputProperty(
parameter = parameter,
kProperty = kProperty,
kqlProperty = kProperty?.let { inputObjectDef.kotlinProperties[it] }
)
fields.addInputValue(inputValue)
}
}

if (fields.isEmpty()) {
Expand All @@ -515,9 +515,6 @@ open class SchemaCompilation(
// OneOf input types require that exactly one field is set, which means that all others must be nullable
throw SchemaException("OneOf input types must only have nullable fields. Fields ${notNullableFields.map { it.name }} are non-nullable on type '${inputObjectDef.name}'")
}

fields.forEach { validateName(it.name) }

typeProxy.proxied = Type.Input(inputObjectDef, fields)
return typeProxy
}
Expand Down
51 changes: 50 additions & 1 deletion kgraphql/src/test/kotlin/de/stuebingerb/kgraphql/integration/InputObjectTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,9 +165,11 @@ class InputObjectTest {

@Test
fun `property name must not start with __ when configured`() {
expect<SchemaException>("Unable to handle input type 'Person': Illegal name '__name'. Names starting with '__' are reserved for introspection system") {
expect<SchemaException>("Unable to handle input type 'PersonInput': Illegal name '__name'. Names starting with '__' are reserved for introspection system") {
KGraphQL.schema {
inputType<Person> {
name = "PersonInput"

property(Person::name) {
name = "__name"
}
Expand Down Expand Up @@ -1200,4 +1202,51 @@ class InputObjectTest {
}
""".trimIndent()
}

@Test
fun `property names must be unique when configured`() {
data class MyInputObject(val foo: String?, val bar: String?)
expect<SchemaException>("Unable to handle input type 'MyInputObject': Cannot add field with duplicated name 'foo'") {
KGraphQL.schema {
query("test") {
resolver { input: MyInputObject -> input.foo }
}
inputType<MyInputObject> {
MyInputObject::foo.configure {
deprecate("Deprecated old input value")
}
MyInputObject::bar.configure {
name = "foo"
}
}
}
}

val schema = KGraphQL.schema {
query("test") {
resolver { input: MyInputObject -> input.foo }
}
inputType<MyInputObject> {
// Although highly confusing, this should work
MyInputObject::foo.configure {
name = "bar"
}
MyInputObject::bar.configure {
name = "foo"
}
}
}

SchemaPrinter().print(schema) shouldBe """
type Query {
test(input: MyInputObject!): String
}

input MyInputObject {
bar: String
foo: String
}

""".trimIndent()
}
}
48 changes: 48 additions & 0 deletions kgraphql/src/test/kotlin/de/stuebingerb/kgraphql/integration/ObjectTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package de.stuebingerb.kgraphql.integration
import de.stuebingerb.kgraphql.KGraphQL
import de.stuebingerb.kgraphql.expect
import de.stuebingerb.kgraphql.schema.SchemaException
import de.stuebingerb.kgraphql.schema.SchemaPrinter
import io.kotest.matchers.shouldBe
import org.junit.jupiter.api.Test

Expand Down Expand Up @@ -98,4 +99,51 @@ class ObjectTest {
}
}
}

@Test
fun `property names must be unique when configured`() {
data class MyObject(val foo: String?, val bar: String?)
expect<SchemaException>("Unable to handle object type 'MyObject': Cannot add field with duplicated name 'foo'") {
KGraphQL.schema {
query("test") {
resolver { foo: String, bar: String -> MyObject(foo, bar) }
}
type<MyObject> {
MyObject::foo.configure {
deprecate("Deprecated old input value")
}
MyObject::bar.configure {
name = "foo"
}
}
}
}

val schema = KGraphQL.schema {
query("test") {
resolver { foo: String, bar: String -> MyObject(foo, bar) }
}
type<MyObject> {
// Although highly confusing, this should work
MyObject::foo.configure {
name = "bar"
}
MyObject::bar.configure {
name = "foo"
}
}
}

SchemaPrinter().print(schema) shouldBe """
type MyObject {
bar: String
foo: String
}

type Query {
test(bar: String!, foo: String!): MyObject!
}

""".trimIndent()
}
}
2 changes: 1 addition & 1 deletion .../kotlin/de/stuebingerb/kgraphql/specification/typesystem/InputObjectsSpecificationTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -804,7 +804,7 @@ class InputObjectsSpecificationTest {

@Test
fun `oneOf input objects out of sealed classes must have unique field names`() {
expect<SchemaException>("Unable to handle input type 'InvalidInputSealed': Cannot add oneOf field with duplicated name 'inputSealedSub1'") {
expect<SchemaException>("Unable to handle input type 'InvalidInputSealed': Cannot add field with duplicated name 'inputSealedSub1'") {
KGraphQL.schema {
query("test") {
resolver { input: InputSealed? -> input }
Expand Down
Loading