You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix: prevent duplicate field names through configure - #787
No actionable comments were generated in the recent review.
Summary
Summary by CodeRabbit
Bug Fixes
Schema generation now consistently validates field names and rejects duplicate names across object and input fields, including configured names.
Introspected input types now include their fields in the schema.
Reserved-name errors for input types now identify the correct type.
Schema compilation now validates field names and rejects duplicates as fields are added to object and input types. Input fields are no longer omitted based on the NotIntrospected annotation. Tests cover duplicate and swapped configured property names.
Changes: Schema field validation
Layer / File(s)
Summary
Shared validation and object fields kgraphql/src/main/kotlin/de/stuebingerb/kgraphql/schema/structure/SchemaCompilation.kt, kgraphql/src/test/kotlin/de/stuebingerb/kgraphql/integration/ObjectTest.kt
Object Kotlin, extension, union, and dataloaded fields are added through shared name validation. Tests check that duplicate configured names raise a SchemaException and swapped names are accepted.
Input field validation kgraphql/src/main/kotlin/de/stuebingerb/kgraphql/schema/structure/SchemaCompilation.kt, kgraphql/src/test/kotlin/de/stuebingerb/kgraphql/integration/InputObjectTest.kt, kgraphql/src/test/kotlin/de/stuebingerb/kgraphql/specification/typesystem/InputObjectsSpecificationTest.kt
Sealed one-of alternatives and constructor parameters use shared name validation. Tests cover duplicate configured names, swapped names, and updated error expectations.
Change: Bug fix
Merge Risk: Minimal · up to 172b2
The change rejects duplicate field names that configure can produce, and swapping two names is still allowed. No actionable merge-blocking risk was identified in the supplied changes.
Pre-merge checks: 4
Check name
Status
Explanation
Title check
The title names the change, which is preventing duplicate field names when configure renames a property. The changeset adds field-name validation through shared addField and addInputValue helper…
Description check
The description only references issue #781 with 'Resolves #781'. It is brief, but it relates to the changeset because the linked issue concerns duplicate input-field names created by configure, whic…
Linked Issues check
Issue #781 requires that PropertyDSL must not create duplicate input-field names when configure renames a property (example: InputObject::bar renamed to foo while InputObject::foo exists). The PR summ…
Out of Scope Changes check
The changes center on duplicate-name validation in SchemaCompilation.kt (addField for object fields, addInputValue for input fields) and on tests for the duplicate-name scenario in InputObjectTest.kt,…
Full details: Title check
Explanation
The title names the change, which is preventing duplicate field names when configure renames a property. The changeset adds field-name validation through shared addField and addInputValue helpers that reject duplicates, and adds tests for the duplicate-name case. The title follows the conventional commits prefix fix:. It is 54 characters, slightly over the 50-character guideline.
Full details: Description check
Explanation
The description only references issue #781 with 'Resolves #781'. It is brief, but it relates to the changeset because the linked issue concerns duplicate input-field names created by configure, which the changes address.
Full details: Linked Issues check
Explanation
Issue #781 requires that PropertyDSL must not create duplicate input-field names when configure renames a property (example: InputObject::bar renamed to foo while InputObject::foo exists). The PR summary for SchemaCompilation.kt states that input fields are accumulated through addInputValue, which validates names and rejects duplicates across sealed one-of fields and constructor parameters. The old per-type duplicate tracking was removed in favor of this shared check. InputObjectTest.kt adds a test where two input properties configured to the same name raise SchemaException. The same file adds a test showing that swapping two names is accepted and yields both fields. The visible portion of SchemaCompilation.kt confirms that object fields use the shared addField validation. The addInputValue body was not readable because the file read was truncated at 16000 bytes. The PASS relies on the PR summary and the described tests for the input path. The repository diff could not be read because changed blobs are missing from the checkout.
Full details: Out of Scope Changes check
Explanation
The changes center on duplicate-name validation in SchemaCompilation.kt (addField for object fields, addInputValue for input fields) and on tests for the duplicate-name scenario in InputObjectTest.kt, ObjectTest.kt, and InputObjectsSpecificationTest.kt. The removal of the NotIntrospected check lets introspected input types use the same validation path. This is a broader behavior change, but it follows from unifying validation and is not shown to be unrelated. The changed expected error text (PersonInput name, 'Cannot add field with duplicated name') follows from the shared validation path. No unrelated feature change appears in the summary. The full diff was not readable, so the NotIntrospected removal was assessed from the summary only.
Finishing Touches
Generate unit tests (beta)
Commit to this branch
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
❌ Patch coverage is 96.07843% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.49%. Comparing base (f11820e) to head (172b272).
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. 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. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="kgraphql/src/main/kotlin/de/stuebingerb/kgraphql/schema/structure/SchemaCompilation.kt">
<violation number="1" location="kgraphql/src/main/kotlin/de/stuebingerb/kgraphql/schema/structure/SchemaCompilation.kt:454">
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.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Config as Schema Configuration DSL
participant Compiler as SchemaCompilation
participant DuplicateGuard as Duplicate Field Guard
participant Validator as Name Validator
Note over Config,Validator: Schema Validation Flow
Config->>Compiler: configure(inputType/type definitions)
alt Object type compilation
Compiler->>Compiler: Collect memberProperties
Compiler->>DuplicateGuard: addField(field)
DuplicateGuard->>Validator: validateName(field.name)
Validator-->>DuplicateGuard: Name valid
alt Field name already exists
DuplicateGuard->>Compiler: Throw SchemaException (duplicate)
else Unique field name
DuplicateGuard->>Compiler: Return field list
end
else Input type compilation
Compiler->>Compiler: Collect constructor parameters
Compiler->>DuplicateGuard: addInputValue(value)
DuplicateGuard->>Validator: validateName(value.name)
Validator-->>DuplicateGuard: Name valid
alt Field name already exists
DuplicateGuard->>Compiler: Throw SchemaException (duplicate)
else Unique field name
DuplicateGuard->>Compiler: Return input field list
end
else OneOf sealed input compilation
Compiler->>Compiler: Collect sealed subclasses
Compiler->>DuplicateGuard: addInputValue(value)
DuplicateGuard->>Validator: validateName(value.name)
Validator-->>DuplicateGuard: Name valid
alt Field name already exists
DuplicateGuard->>Compiler: Throw SchemaException (duplicate)
else Unique field name
DuplicateGuard->>Compiler: Return input field list
end
end
Compiler->>Compiler: Build Type.Object/Type.InputObject
Compiler->>Config: Return compiled schema
Loading
Reply with feedback, questions, or to request a fix.
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>
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Bencher Report
Resolves #781