fix: fix insufficient validation of fragment condition - #649
Conversation
|
Warning Review limit reached
More reviews will be available in 1 minute and 24 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthrough
ChangesFragment Type-Condition Validation
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/structure/RequestInterpreter.kt`:
- Around line 148-152: The type resolution logic in RequestInterpreter.kt (lines
148-152) incorrectly skips type-condition validation when directives are
present. The current condition checks if directives exist and defaults to
enclosingType, bypassing the findFragmentConditionType call that validates
explicit type conditions. Remove the directive-based branching and always call
findFragmentConditionType for all inline fragments, ensuring that explicit type
conditions are validated regardless of whether directives are present.
Additionally, add a regression test case that covers inline fragments with both
an explicit typeCondition and directives to prevent this validation bypass in
the future.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6a606fa5-a977-4bb4-9468-bfeee7b36a38
📒 Files selected for processing (4)
kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/execution/ParallelRequestExecutor.ktkgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/structure/RequestInterpreter.ktkgraphql/src/test/kotlin/com/apurebase/kgraphql/integration/QueryTest.ktkgraphql/src/test/kotlin/com/apurebase/kgraphql/specification/language/FragmentsSpecificationTest.kt
💤 Files with no reviewable changes (1)
- kgraphql/src/test/kotlin/com/apurebase/kgraphql/integration/QueryTest.kt
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #649 +/- ##
==========================================
+ Coverage 84.19% 84.22% +0.02%
==========================================
Files 151 151
Lines 4992 5000 +8
Branches 857 857
==========================================
+ Hits 4203 4211 +8
- Misses 490 491 +1
+ Partials 299 298 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 4 files
Architecture diagram
sequenceDiagram
participant Client as GraphQL Client
participant Executor as ParallelRequestExecutor
participant Interpreter as RequestInterpreter
participant Schema as Schema Model
participant Type as Type System
Note over Client,Type: Fragment Condition Validation Flow
Client->>Executor: execute(query with fragments)
Executor->>Interpreter: interpret(executables)
Interpreter->>Interpreter: findFragmentConditionType(fragment)
alt Fragment definition (named fragment)
Interpreter->>Schema: allTypesByName[typeName]
Schema-->>Interpreter: Type or null
alt Type NOT found
Interpreter->>Interpreter: throw ValidationException("Unknown type 'XXX'...")
Interpreter-->>Executor: Error
Executor-->>Client: Validation error response
else Type found
Interpreter->>Type: isInputType()
alt Type is input type (ENUM, SCALAR, INPUT_OBJECT)
Interpreter->>Interpreter: throw ValidationException("Fragments can only be on object/interface/union...")
Interpreter-->>Executor: Error
Executor-->>Client: Validation error response
else Type is object/interface/union
Interpreter->>Interpreter: return Type
Interpreter-->>Executor: Valid fragment
Executor->>Executor: handleFragment(ctx, value, ...)
Executor-->>Client: Query result
end
end
else Inline fragment
Interpreter->>Interpreter: extract typeCondition.name
Interpreter->>Schema: allTypesByName[typeName]
Schema-->>Interpreter: Type or null
alt Type NOT found
Interpreter->>Interpreter: throw ValidationException("Unknown type 'XXX'...")
else Type found
Interpreter->>Type: isInputType()
alt Type is input type
Interpreter->>Interpreter: throw ValidationException("Fragments can only be on object/interface/union...")
else Type is valid
Interpreter->>Interpreter: return Type
end
end
end
Note over Interpreter,Type: CHANGED: Unified validation in findFragmentConditionType()
Note over Interpreter: Validates type existence AND type kind
Note over Interpreter: Returns proper error messages with fragment name context
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Bencher Report
b618a31 to
df7095a
Compare
|
Review limit reached for coderabbit, that seems... quick. |
df7095a to
d57c4fc
Compare
d57c4fc to
54684ec
Compare
1 participant
Resolves #648