Skip to content

fix: improve handling of nullable lists for required fields - #1631

Merged
Noroth merged 2 commits into
masterfrom
ludwig/router-606-engine-add-test-cases-for-list-wrappers-for-required-fields
Aug 13, 2026
Merged

fix: improve handling of nullable lists for required fields#1631
Noroth merged 2 commits into
masterfrom
ludwig/router-606-engine-add-test-cases-for-list-wrappers-for-required-fields

Conversation

@Noroth

@Noroth Noroth commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

This PR adds tests for nullable lists as arguments for @requires fields. It also improves the behavior when nullable vs empty lists are provided as arguments

Checklist

  • I have discussed my proposed changes in an issue and have received approval to proceed.
  • I have followed the coding standards of the project.
  • Tests or benchmarks have been added or updated.

Open Source AI Manifesto

This project follows the principles of the Open Source AI Manifesto. Please ensure your contribution aligns with its principles.

Summary by CodeRabbit

  • Bug Fixes

    • Corrected handling of nullable lists so empty lists remain available while explicitly null lists remain unset.
    • Improved federation field resolution for nullable list values, including object, interface, and union results.
  • New Features

    • Added support for optional storage histories, recommended items, and tag filtering by list lengths.
    • Added GraphQL and service support for nullable-list arguments and responses.
  • Tests

    • Added comprehensive coverage for populated, empty, and null list scenarios.

@Noroth
Noroth requested a review from a team as a code owner August 11, 2026 11:29
@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 320acb05-acb4-43ae-813e-d66d5a5077e3

📥 Commits

Reviewing files that changed from the base of the PR and between 634e76b and d63d2bd.

📒 Files selected for processing (1)
  • v2/pkg/engine/datasource/grpc_datasource/compiler.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • v2/pkg/engine/datasource/grpc_datasource/compiler.go

📝 Walkthrough

Walkthrough

The change adds nullable-list @requires fields and arguments across GraphQL schemas, protobuf contracts, Connect handlers, mock services, mappings, compiler traversal, execution-plan tests, and federation integration tests.

Changes

Nullable @requires lists

Layer / File(s) Summary
Nullable-list contracts and Connect surface
v2/pkg/grpctest/product.proto, v2/pkg/grpctest/productv1/productv1connect/product.connect.go, v2/pkg/grpctest/schema.go, v2/pkg/grpctest/testdata/products.graphqls
Added list wrapper messages, four storage requirement RPCs, Connect client and handler support, GraphQL field definitions, and federation requirement metadata.
Mock RPC implementations and mappings
v2/pkg/grpctest/mockservice_requires.go, v2/pkg/grpctest/mockservice_connect.go, v2/pkg/grpctest/mapping/mapping.go, v2/pkg/engine/datasource/grpc_datasource/mapping_test_helper.go
Added nullable wrapped-list responses, nullable list-length filtering, Connect passthroughs, and storage field mappings.
List traversal and execution planning
v2/pkg/engine/datasource/grpc_datasource/compiler.go, v2/pkg/engine/datasource/grpc_datasource/execution_plan_requires_test.go
Preserved non-null empty list wrappers and added plan coverage for object, interface, union, and nullable argument cases.
Federation execution validation
v2/pkg/engine/datasource/grpc_datasource/grpc_datasource_federation_test.go
Added execution coverage for populated, empty, and null lists, abstract types, and nullable list arguments.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant GraphQLClient
  participant GRPCDatasource
  participant MockServiceConnect
  participant MockService
  GraphQLClient->>GRPCDatasource: Execute nullable-list `@requires` field
  GRPCDatasource->>MockServiceConnect: Call storage requirement RPC
  MockServiceConnect->>MockService: Forward request and metadata
  MockService-->>MockServiceConnect: Return populated, empty, or null wrapper
  MockServiceConnect-->>GRPCDatasource: Return Connect response
  GRPCDatasource-->>GraphQLClient: Resolve nullable-list result
Loading

Possibly related PRs

<f­­ixed_issue_severity>Low</fixed_issue_severity>

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: improved handling of nullable lists for required fields.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ludwig/router-606-engine-add-test-cases-for-list-wrappers-for-required-fields

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

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Caution

CodeRabbit couldn't update its existing comment. The review summary may be out of date.

Error details
putComment timed out

@fiam fiam left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@Noroth
Noroth merged commit 0af4dd3 into master Aug 13, 2026
11 checks passed
@Noroth
Noroth deleted the ludwig/router-606-engine-add-test-cases-for-list-wrappers-for-required-fields branch August 13, 2026 08:18
ysmolski pushed a commit that referenced this pull request Aug 13, 2026
🤖 I have created a release *beep* *boop*
---


##
[2.16.0](v2.15.1...v2.16.0)
(2026-08-13)


### Features

* implement multi fetch to the same subgraph
([#1594](#1594))
([22584e2](22584e2))
* schedule fetch trees optimally
([#1612](#1612))
([bd03deb](bd03deb))


### Bug Fixes

* check interface implementation in `potentiallySameObject` for
nullability relaxation
([#1454](#1454))
([5bacb9e](5bacb9e))
* improve handling of nullable lists for required fields
([#1631](#1631))
([0af4dd3](0af4dd3))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: wundergraph-bot[bot] <285992168+wundergraph-bot[bot]@users.noreply.github.com>
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.

3 participants