Skip to content

Coerce field arguments before invoking data fetchers - #4522

Open
andimarek wants to merge 2 commits into
masterfrom
codex/fix-lazy-argument-coercion
Open

andimarek wants to merge 2 commits into
masterfrom
codex/fix-lazy-argument-coercion

Conversation

@andimarek

@andimarek andimarek commented Nov 1, 2026 •

Copy link
Copy Markdown
Member

Argument coercion currently runs only when something reads a field's arguments. A data fetcher that ignores them can therefore return a result for invalid inputs; one that reads them can perform side effects before coercion fails.

For example, query($v: String = "default") { f(arg: [$v]) } with {"v": null} passes document and variable validation against f(arg: [String!]!): String, but argument coercion must fail before calling f. This complements #4507, which added the non-null checks themselves.

Force the existing memoized argument supplier before invoking either a regular or lightweight data fetcher, inside the existing exception-handling boundary. This preserves field-error handling, null propagation, instrumentation, and DataLoader dispatch. Fields without argument definitions keep lazy environment construction.

Adds 45 Spock cases covering queries, mutation side effects and ordering, subscriptions, nested inputs, property/lightweight fetchers, aliases and list paths, skipped/unreached fields, deferred results, scalar coercion/memoization, exception handlers, and all three DataLoader dispatch modes. On master, 36 of these cases fail; with this change, all 45 pass.

Validation: ./gradlew test testng jacocoTestReport on Java 25 passed (6,223 tests, zero failures, 57 existing skips). The local per-class coverage comparison against the committed baseline found no regressions.

Exploratory local JMH comparison with master (Java 25, 2 forks, 3 × 1s warmup, 5 × 1s measurement): a 100-item query took 15.1 → 15.7 µs without arguments, 57.9 → 57.8 µs when fetchers read literal arguments, and 15.8 → 53.8 µs when property fetchers ignored literal arguments. The latter now performs the required coercion. The existing ExecutionBenchmark measured 269 → 262 µs, within measurement uncertainty. The no-argument path retains its allocation savings but does add the argument-definition check.

@github-actions

github-actions Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Test Report

Test Results

Java Version Total Passed Failed Errors Skipped
Java 17 6222 (+45 🟢) 6166 (+45 🟢) 0 (±0) 0 (±0) 56 (±0)
Java 21 6222 (+45 🟢) 6166 (+45 🟢) 0 (±0) 0 (±0) 56 (±0)
Java 25 6222 (+45 🟢) 6166 (+45 🟢) 0 (±0) 0 (±0) 56 (±0)
jcstress 32 (±0) 32 (±0) 0 (±0) 0 (±0) 0 (±0)
Total 18698 (+135 🟢) 18530 (+135 🟢) 0 (±0) 0 (±0) 168 (±0)

Code Coverage (Java 25)

Metric Covered Missed Coverage vs Master
Lines 30305 3081 90.8% ±0.0%
Branches 8932 1506 85.6% ±0.0%
Methods 8093 1188 87.2% ±0.0%

Changed Class Coverage (1 class)

Class Line Branch Method
g.e.ExecutionStrategy ±0.0% +0.1% 🟢 ±0.0%

Full HTML report: build artifact jacoco-html-report

Updated: 2026-11-01 06:30:02 UTC

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant