Skip to content

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

Closed
Noroth wants to merge 3 commits into
masterfrom
ludwig/router-606-engine-add-test-cases-for-list-wrappers-for-required-fields
Closed

fix: improve handling of nullable lists for required fields#1630
Noroth wants to merge 3 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 test cases for handling nullable lists as arguments for @requires fields. It also improves the behavior for passing nullable lists as arguments to distinguish between empty and null lists.

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

  • New Features

    • Added support for additional storage-related operations, including recommended items, operation history, metadata history, latest operations, and tag filtering by lengths.
    • Expanded federation support for fields returning interfaces, unions, nested abstract values, and nullable lists.
    • Added handling for nullable list arguments, distinguishing absent, null, empty, and populated values.
  • Bug Fixes

    • Corrected optional list processing so empty lists are preserved instead of being treated as omitted.

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

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: a8f6d043-01e1-4751-a23b-5e34415106cf

📥 Commits

Reviewing files that changed from the base of the PR and between b90db54 and 2518c8b.

⛔ Files ignored due to path filters (2)
  • v2/pkg/grpctest/productv1/product.pb.go is excluded by !**/*.pb.go
  • v2/pkg/grpctest/productv1/product_grpc.pb.go is excluded by !**/*.pb.go
📒 Files selected for processing (14)
  • v2/pkg/engine/datasource/grpc_datasource/compiler.go
  • v2/pkg/engine/datasource/grpc_datasource/execution_plan.go
  • v2/pkg/engine/datasource/grpc_datasource/execution_plan_requires_test.go
  • v2/pkg/engine/datasource/grpc_datasource/execution_plan_visitor.go
  • v2/pkg/engine/datasource/grpc_datasource/execution_plan_visitor_federation.go
  • v2/pkg/engine/datasource/grpc_datasource/grpc_datasource_federation_test.go
  • v2/pkg/engine/datasource/grpc_datasource/mapping_test_helper.go
  • v2/pkg/grpctest/mapping/mapping.go
  • v2/pkg/grpctest/mockservice_connect.go
  • v2/pkg/grpctest/mockservice_requires.go
  • v2/pkg/grpctest/product.proto
  • v2/pkg/grpctest/productv1/productv1connect/product.connect.go
  • v2/pkg/grpctest/schema.go
  • v2/pkg/grpctest/testdata/products.graphqls

📝 Walkthrough

Walkthrough

The PR adds gRPC federation @requires support for abstract return types and nullable lists. It refactors composite selection planning, adds Storage RPC contracts and mock handlers, preserves null versus empty list behavior, and expands execution-plan and datasource tests.

Changes

Federation @requires support

Layer / File(s) Summary
Schema and RPC contracts
v2/pkg/grpctest/product.proto, v2/pkg/grpctest/schema.go, v2/pkg/grpctest/testdata/products.graphqls, v2/pkg/grpctest/productv1/productv1connect/product.connect.go
Adds Storage fields, list wrappers, required-field RPC messages, Connect clients, handlers, routes, and defaults.
Composite execution planning
v2/pkg/engine/datasource/grpc_datasource/compiler.go, v2/pkg/engine/datasource/grpc_datasource/execution_plan.go, v2/pkg/engine/datasource/grpc_datasource/execution_plan_visitor*.go, v2/pkg/engine/datasource/grpc_datasource/execution_plan_requires_test.go
Builds reusable composite type configuration and preserves abstract member, fragment, and nullable-list metadata in required-field plans.
Required-field RPC execution
v2/pkg/grpctest/mapping/mapping.go, v2/pkg/engine/datasource/grpc_datasource/mapping_test_helper.go, v2/pkg/grpctest/mockservice_*.go, v2/pkg/grpctest/productv1/productv1connect/product.connect.go
Adds Storage mappings, Connect passthroughs, and mock implementations for recommended items, operations, metadata history, and tag-length filtering.
Federation datasource validation
v2/pkg/engine/datasource/grpc_datasource/grpc_datasource_federation_test.go
Tests interface and union results, nested abstract values, fragments, concrete __typename fields, nullable lists, and nullable list arguments through the gRPC datasource pipeline.

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

Possibly related PRs

✨ 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.

@Noroth Noroth closed this Aug 11, 2026
@Noroth
Noroth deleted the ludwig/router-606-engine-add-test-cases-for-list-wrappers-for-required-fields branch August 11, 2026 11:22
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