Skip to content

fix: fix NullPointer for data properties inside union properties#681

Merged
stuebingerb merged 1 commit into
mainfrom
fix/fix-data-properties-for-union-properties
Jul 4, 2026
Merged

fix: fix NullPointer for data properties inside union properties#681
stuebingerb merged 1 commit into
mainfrom
fix/fix-data-properties-for-union-properties

Conversation

@stuebingerb

Copy link
Copy Markdown
Owner

Fixes #667

@coderabbitai

coderabbitai Bot commented Jul 3, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@stuebingerb, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 53 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 8273c184-ad8e-4a35-84f8-38b25c2a97fa

📥 Commits

Reviewing files that changed from the base of the PR and between 0f0e336 and 924a3ad.

📒 Files selected for processing (2)
  • kgraphql/src/main/kotlin/de/stuebingerb/kgraphql/schema/execution/ParallelRequestExecutor.kt
  • kgraphql/src/test/kotlin/de/stuebingerb/kgraphql/DataLoaderTest.kt
📝 Walkthrough

Walkthrough

The PR fixes an NPE in ParallelRequestExecutor where data loaders nested inside union member fields were never registered because the loader-construction traversal skipped Execution.Union member children. Loader lookup now uses checkNotNull with a descriptive error, and a test was added.

Changes

Union data loader fix

Layer / File(s) Summary
Loader traversal and lookup fix
kgraphql/.../ParallelRequestExecutor.kt
Loader construction now flattens and inspects Execution.Union member children for Field.DataLoader instances, and loader retrieval uses checkNotNull with a "Missing data loader" message instead of a non-null assertion.
Union data loader test
kgraphql/.../DataLoaderTest.kt
Adds Author, Novel, NonFiction, and BookShelf data classes and a test verifying that a data loader field nested inside a union property (BookShelf.items) resolves correctly via fragments.

Sequence Diagram(s)

sequenceDiagram
  participant Query as GraphQL Query
  participant Executor as ParallelRequestExecutor
  participant Union as Execution.Union
  participant Loader as DataLoader

  Query->>Executor: request items via union fragment
  Executor->>Union: inspect memberChildren for Field.DataLoader
  Union-->>Executor: found nested loader fields
  Executor->>Loader: construct and register loader
  Query->>Executor: resolve author field
  Executor->>Loader: checkNotNull(loaders[field])
  Loader-->>Executor: resolved Author instance
  Executor-->>Query: serialized data
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title is clearly related to the NPE fix in union-backed data properties and uses a conventional commit prefix.
Description check ✅ Passed The description references issue #667, which matches the dataloader-union fix in this changeset.
Linked Issues check ✅ Passed The code now traverses union member children and the new test verifies dataloader fields inside union branches resolve correctly.
Out of Scope Changes check ✅ Passed The PR stays focused on the union dataloader null-loader fix and its test coverage with no unrelated changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/fix-data-properties-for-union-properties

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Jul 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 50.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.20%. Comparing base (10f6e7f) to head (924a3ad).

Files with missing lines Patch % Lines
...raphql/schema/execution/ParallelRequestExecutor.kt 50.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #681      +/-   ##
==========================================
- Coverage   84.23%   84.20%   -0.04%     
==========================================
  Files         151      151              
  Lines        4991     4994       +3     
  Branches      858      860       +2     
==========================================
+ Hits         4204     4205       +1     
- Misses        489      490       +1     
- Partials      298      299       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No issues found across 2 files

Architecture diagram
sequenceDiagram
    participant Client as Test Client
    participant Schema as KGraphQL Schema
    participant Executor as ParallelRequestExecutor
    participant Union as Execution.Union
    participant Node as Execution.Node (Novel)
    participant Loader as DataLoader

    Client->>Schema: executeBlocking(query)
    Schema->>Executor: execute(rootExecution)
    Executor->>Executor: inspect() root execution tree
    alt Execution is Union
        Executor->>Union: NEW: inspect memberChildren
        Union->>Node: flatten().inspect()
        Node->>Executor: children (including 'author' field)
        alt field has DataLoader
            Executor->>Loader: prepare(authorId)
            Loader-->>Executor: preparedValue
            Note over Executor: CHANGED: checkNotNull(loader) guard
            Executor->>Loader: loadAsync(preparedValue)
            Loader-->>Executor: Author result
        end
    end
    Executor-->>Schema: resolved nodes
    Schema-->>Client: JSON result
Loading

Re-trigger cubic

@stuebingerb stuebingerb force-pushed the fix/fix-data-properties-for-union-properties branch from 0f0e336 to 924a3ad Compare July 3, 2026 17:18
@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown

🐰 Bencher Report

Branchfix/fix-data-properties-for-union-properties
Testbedubuntu-latest
Click to view all benchmark results
BenchmarkThroughputBenchmark Result
operations / second (ops/s)
(Result Δ%)
Lower Boundary
operations / second (ops/s)
(Limit %)
de.stuebingerb.kgraphql.FunctionExecutionBenchmark.benchmarkFunctionExecution📈 view plot
🚷 view threshold
5,972,436.42 ops/s
(+3.90%)Baseline: 5,748,305.81 ops/s
4,814,169.95 ops/s
(80.61%)
de.stuebingerb.kgraphql.ParallelExecutionBenchmark.queryBenchmark📈 view plot
🚷 view threshold
1.30 ops/s
(+0.06%)Baseline: 1.30 ops/s
1.30 ops/s
(99.76%)
de.stuebingerb.kgraphql.QueryBenchmark.executionError📈 view plot
🚷 view threshold
15,745.48 ops/s
(-10.02%)Baseline: 17,498.95 ops/s
10,282.32 ops/s
(65.30%)
de.stuebingerb.kgraphql.QueryBenchmark.inputFromDocument📈 view plot
🚷 view threshold
19,636.59 ops/s
(-9.10%)Baseline: 21,601.31 ops/s
12,960.48 ops/s
(66.00%)
de.stuebingerb.kgraphql.QueryBenchmark.inputFromVariable📈 view plot
🚷 view threshold
19,420.12 ops/s
(-6.85%)Baseline: 20,849.13 ops/s
12,848.51 ops/s
(66.16%)
de.stuebingerb.kgraphql.QueryBenchmark.largeList📈 view plot
🚷 view threshold
4.85 ops/s
(+2.80%)Baseline: 4.71 ops/s
4.02 ops/s
(82.87%)
de.stuebingerb.kgraphql.QueryBenchmark.largeListWithFragment📈 view plot
🚷 view threshold
5.34 ops/s
(+2.06%)Baseline: 5.23 ops/s
4.45 ops/s
(83.46%)
de.stuebingerb.kgraphql.QueryBenchmark.manyChildren📈 view plot
🚷 view threshold
192.47 ops/s
(+6.46%)Baseline: 180.80 ops/s
135.03 ops/s
(70.16%)
de.stuebingerb.kgraphql.QueryBenchmark.manyChildrenWithFragment📈 view plot
🚷 view threshold
203.66 ops/s
(+6.66%)Baseline: 190.95 ops/s
141.74 ops/s
(69.60%)
de.stuebingerb.kgraphql.QueryBenchmark.manyDataChildren📈 view plot
🚷 view threshold
8.95 ops/s
(+0.45%)Baseline: 8.91 ops/s
8.77 ops/s
(98.07%)
de.stuebingerb.kgraphql.QueryBenchmark.manyOperations📈 view plot
🚷 view threshold
314.98 ops/s
(+4.39%)Baseline: 301.73 ops/s
228.96 ops/s
(72.69%)
de.stuebingerb.kgraphql.QueryBenchmark.manyOperationsWithFragment📈 view plot
🚷 view threshold
333.83 ops/s
(+6.04%)Baseline: 314.80 ops/s
243.53 ops/s
(72.95%)
de.stuebingerb.kgraphql.QueryBenchmark.nestedObject📈 view plot
🚷 view threshold
7,808.60 ops/s
(+0.74%)Baseline: 7,751.16 ops/s
6,037.86 ops/s
(77.32%)
de.stuebingerb.kgraphql.RequestCachingBenchmark.invalidRequest📈 view plot
🚷 view threshold
139,249.79 ops/s
(-1.95%)Baseline: 142,021.37 ops/s
135,498.82 ops/s
(97.31%)
de.stuebingerb.kgraphql.RequestCachingBenchmark.largeRequest📈 view plot
🚷 view threshold
7,881.29 ops/s
(-1.29%)Baseline: 7,984.12 ops/s
5,747.18 ops/s
(72.92%)
de.stuebingerb.kgraphql.RequestCachingBenchmark.smallRequest📈 view plot
🚷 view threshold
10,885.47 ops/s
(-5.39%)Baseline: 11,505.96 ops/s
7,938.07 ops/s
(72.92%)
de.stuebingerb.kgraphql.SimpleExecutionOverheadBenchmark.benchmark📈 view plot
🚷 view threshold
455,701.37 ops/s
(-3.03%)Baseline: 469,917.54 ops/s
442,956.87 ops/s
(97.20%)
🐰 View full continuous benchmarking report in Bencher

@stuebingerb stuebingerb merged commit 06faead into main Jul 4, 2026
11 of 13 checks passed
@stuebingerb stuebingerb deleted the fix/fix-data-properties-for-union-properties branch July 4, 2026 18:35
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.

Dataloader fields nested inside a union member throw NPE (loader never registered)

2 participants