feat: add support for fields with arguments and requires directive by dkorittki · Pull Request #1452 · wundergraph/graphql-go-tools

dkorittki · 2026-03-18T15:20:33Z

Context

A change in protographic allows to treat fields with arguments to be treated as a required fields alongside needed protobuf rpc's and message to retrieve field_arguments. A necessary change to allow field arguments on @requires fields.

When giving this new proto spec and mapping to the engine such fields were silently ignoring the arguments — the gRPC call received the context (key + required fields) but no field_args, so the service always saw zero-value arguments.

Implementation

Wire field arguments through the @requires execution path (grpc datasource, federation visitor, compiler), doing it similar to how field resolvers already handle them.

Tests

Three new integration + execution-plan test scenarios covering:

Scenario	Args
Single required string arg	`prefix: String!`
Two args, one repeated (list)	`prefixes: [String!]!, maxResults: Int!`
Nullable arg (maps to `google.protobuf.StringValue`)	`prefix: String`

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.

Just to ensure a clean diff of what really changes once the grpctest SDL changes.

coderabbitai · 2026-03-18T15:20:41Z

📝 Walkthrough

Walkthrough

Propagates GraphQL field arguments from federation @requires into gRPC required-fields RPCs: the planner captures arguments, the execution plan includes an optional field_args request section when present, and the gRPC compiler conditionally builds/sets the protobuf field_args payload.

Changes

Cohort / File(s)	Summary
Planning & Visitor `v2/pkg/engine/datasource/grpc_datasource/execution_plan_visitor_federation.go`	Parse GraphQL field arguments for required fields and attach parsed `fieldArguments` to the `requiredField`; abort planning on parse errors.
Execution Plan Construction `v2/pkg/engine/datasource/grpc_datasource/execution_plan.go`	`createRequiredFieldsRPCCall` conditionally appends a `field_args` nested request message populated from `requiredField.fieldArguments` when non-empty.
gRPC Compiler `v2/pkg/engine/datasource/grpc_datasource/compiler.go`	`buildRequiredFieldsMessage` checks for a `field_args` field in the RPC message schema; if present, builds the protobuf message for args and sets it on the root request, propagating errors.
Unit Tests (planner/exec plan) `v2/pkg/engine/datasource/grpc_datasource/execution_plan_requires_test.go`	Added table-driven tests asserting execution plans include `field_args` for filtered-tag argument shapes (single, repeated, nullable, multi-arg).
Integration Tests `v2/pkg/engine/datasource/grpc_datasource/grpc_datasource_federation_test.go`	Added end-to-end tests for filtered-tag `@requires` fields driven by arguments, covering matching, no-match, truncation, and nullable-arg behaviors.
Test Mappings `v2/pkg/engine/datasource/grpc_datasource/mapping_test_helper.go`, `v2/pkg/grpctest/mapping/mapping.go`	Added Storage required-field and GraphQL field mappings for `filteredTagSummary`, `multiFilteredTagSummary`, `nullableFilteredTagSummary` with argument→protobuf field mappings.
Mock gRPC Service `v2/pkg/grpctest/mockservice_requires.go`	Added three mock RPC handlers implementing filtered-tag logic (single prefix, multiple prefixes + maxResults, nullable prefix) that read `field_args` and return nullable `StringValue` results.
Proto & Schema `v2/pkg/grpctest/product.proto`, `v2/pkg/grpctest/testdata/products.graphqls`	Added three new RequireStorage... RPCs with corresponding request/response/args messages and added the three Storage GraphQL fields annotated with `@requires(fields: "tags")`.

Sequence Diagram(s)

sequenceDiagram
    participant Client as GraphQL Client
    participant Planner as Federation Planner
    participant Visitor as Visitor
    participant Executor as Execution Plan Executor
    participant Compiler as gRPC Compiler
    participant gRPCService as gRPC Service

    Client->>Planner: Request `@requires` field with arguments
    Planner->>Visitor: process required field
    Visitor->>Visitor: parseFieldArguments() -> attach to requiredField
    Visitor->>Executor: emit execution plan (includes fieldArguments)
    Executor->>Executor: createRequiredFieldsRPCCall() (include field_args if present)
    Executor->>Compiler: buildRequiredFieldsMessage(request with field_args)
    Compiler->>Compiler: locate `field_args` in RPC schema
    Compiler->>Compiler: buildProtoMessage() & setMessageValue()
    Executor->>gRPCService: call RPC with context + field_args
    gRPCService->>gRPCService: read field_args, compute filtered summary
    gRPCService-->>Executor: return response results
    Executor-->>Client: resolve `@requires` field

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

feat: add initial support for @requires in gRPC #1362: Adds/updates @requires gRPC flow to parse and propagate GraphQL field arguments into field_args and adjusts compiler/execution plan accordingly.
feat: allow empty arguments for field resolvers #1350: Changes planner/visitor logic to detect and attach GraphQL field arguments for gRPC datasource required-fields.
feat: support field resolvers with empty arguments #1440: Makes processing of protobuf field_args conditional in compiler/execution-plan code paths, similar conditional handling added here.

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main feature addition: support for fields with arguments in the `@requires` directive.
Description check	✅ Passed	The PR description accurately describes the changeset: adding support for field arguments in `@requires` fields, with implementation details across grpc datasource/federation and three test scenarios covering different argument patterns.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch dominik/eng-9218-implement-requires-with-field-resolvers-in-go-tools

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

…ield-resolvers-in-go-tools

claude

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review.

coderabbitai

🧹 Nitpick comments (1)

v2/pkg/grpctest/mockservice_requires.go (1)

303-321: Handle non-positive maxResults explicitly in multi-prefix filtering.

At Line 319, the cap check uses equality only. For negative values, the cap is effectively disabled, which is surprising for a “max results” contract.

♻️ Suggested refinement

 func (s *MockService) RequireStorageMultiFilteredTagSummaryById(_ context.Context, req *productv1.RequireStorageMultiFilteredTagSummaryByIdRequest) (*productv1.RequireStorageMultiFilteredTagSummaryByIdResponse, error) {
 	prefixes := req.GetFieldArgs().GetPrefixes()
 	maxResults := int(req.GetFieldArgs().GetMaxResults())
 	results := make([]*productv1.RequireStorageMultiFilteredTagSummaryByIdResult, 0, len(req.GetContext()))

 	for _, ctx := range req.GetContext() {
+		if maxResults <= 0 {
+			results = append(results, &productv1.RequireStorageMultiFilteredTagSummaryByIdResult{})
+			continue
+		}
 		tags := ctx.GetFields().GetTags()

 		filteredTags := make([]string, 0, len(tags))
 		for _, tag := range tags {
 			for _, p := range prefixes {
 				if strings.HasPrefix(tag, p) {
 					filteredTags = append(filteredTags, tag)
 					break
 				}
 			}
-			if len(filteredTags) == maxResults {
+			if len(filteredTags) >= maxResults {
 				break
 			}
 		}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@v2/pkg/grpctest/mockservice_requires.go` around lines 303 - 321, In
RequireStorageMultiFilteredTagSummaryById, normalize the requested maxResults
and make the cap check robust: read maxResults :=
int(req.GetFieldArgs().GetMaxResults()), if maxResults <= 0 set maxResults = 0
(treat non-positive as zero) and replace the equality cap check with a >= guard
that only applies when maxResults > 0 (e.g. if maxResults > 0 &&
len(filteredTags) >= maxResults { break }), so negative values no longer disable
the cap and the loop behaves predictably; update references to prefixes,
filteredTags and maxResults in the filtering loop accordingly.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@v2/pkg/grpctest/mockservice_requires.go`:
- Around line 303-321: In RequireStorageMultiFilteredTagSummaryById, normalize
the requested maxResults and make the cap check robust: read maxResults :=
int(req.GetFieldArgs().GetMaxResults()), if maxResults <= 0 set maxResults = 0
(treat non-positive as zero) and replace the equality cap check with a >= guard
that only applies when maxResults > 0 (e.g. if maxResults > 0 &&
len(filteredTags) >= maxResults { break }), so negative values no longer disable
the cap and the loop behaves predictably; update references to prefixes,
filteredTags and maxResults in the filtering loop accordingly.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 2f71adf9-0545-407a-82bc-66cff6012a5a

📥 Commits

Reviewing files that changed from the base of the PR and between 18b292e and a7267a5.

⛔ 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 (10)

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_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_requires.go
v2/pkg/grpctest/product.proto
v2/pkg/grpctest/testdata/products.graphqls

…ield-resolvers-in-go-tools

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@v2/pkg/grpctest/mockservice_requires.go`:
- Around line 495-496: The loop that appends into filteredTags only breaks when
len(filteredTags) == maxResults, which allows results when maxResults <= 0;
update the logic in the function that builds filteredTags to first treat
non-positive maxResults as returning no matches (e.g., if maxResults <= 0 return
empty slice), and change the break condition to use >= (if len(filteredTags) >=
maxResults) to ensure we never exceed the cap; adjust the code paths that
reference filteredTags and maxResults accordingly (look for the loop appending
to filteredTags and the variables filteredTags and maxResults).

🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 7fd88360-f35e-486d-b1a7-bc1ba2812c07

📥 Commits

Reviewing files that changed from the base of the PR and between 5036bd6 and fc1c2d3.

⛔ 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 (10)

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_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_requires.go
v2/pkg/grpctest/product.proto
v2/pkg/grpctest/testdata/products.graphqls

✅ Files skipped from review due to trivial changes (2)

v2/pkg/engine/datasource/grpc_datasource/execution_plan.go
v2/pkg/engine/datasource/grpc_datasource/grpc_datasource_federation_test.go

🚧 Files skipped from review as they are similar to previous changes (6)

v2/pkg/grpctest/mapping/mapping.go
v2/pkg/engine/datasource/grpc_datasource/execution_plan_visitor_federation.go
v2/pkg/engine/datasource/grpc_datasource/mapping_test_helper.go
v2/pkg/engine/datasource/grpc_datasource/execution_plan_requires_test.go
v2/pkg/grpctest/product.proto
v2/pkg/engine/datasource/grpc_datasource/compiler.go

v2/pkg/grpctest/mockservice_requires.go

coderabbitai

♻️ Duplicate comments (1)

v2/pkg/grpctest/mockservice_requires.go (1)

484-497: ⚠️ Potential issue | 🟡 Minor

Handle non-positive maxResults before filtering.

At Line 495, the cap check runs after append, so maxResults <= 0 can still emit one match when the first tag passes the prefix filter.

Proposed fix

 for _, ctx := range req.GetContext() {
+		if maxResults <= 0 {
+			results = append(results, &productv1.RequireStorageMultiFilteredTagSummaryByIdResult{
+				MultiFilteredTagSummary: nil,
+			})
+			continue
+		}
+
 		tags := ctx.GetFields().GetTags()
 
 		filteredTags := make([]string, 0, len(tags))
 		for _, tag := range tags {
 			for _, p := range prefixes {
 				if strings.HasPrefix(tag, p) {
 					filteredTags = append(filteredTags, tag)
 					break
 				}
 			}
 			if len(filteredTags) >= maxResults {
 				break
 			}
 		}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@v2/pkg/grpctest/mockservice_requires.go` around lines 484 - 497, The loop
that builds filteredTags incorrectly allows a match when maxResults <= 0 because
the cap is checked only after appending; before iterating tags (inside the
context loop where req.GetContext() is handled), add an early guard that if
maxResults <= 0 then skip filtering (e.g., continue) or set filteredTags to
empty and skip further work for that context so no tags are appended; ensure the
check on maxResults happens before any append to filteredTags in the code that
uses tags, prefixes, and filteredTags.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@v2/pkg/grpctest/mockservice_requires.go`:
- Around line 484-497: The loop that builds filteredTags incorrectly allows a
match when maxResults <= 0 because the cap is checked only after appending;
before iterating tags (inside the context loop where req.GetContext() is
handled), add an early guard that if maxResults <= 0 then skip filtering (e.g.,
continue) or set filteredTags to empty and skip further work for that context so
no tags are appended; ensure the check on maxResults happens before any append
to filteredTags in the code that uses tags, prefixes, and filteredTags.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: cb179f90-b3e1-4879-ac5f-5d2e3722a36b

📥 Commits

Reviewing files that changed from the base of the PR and between fc1c2d3 and 7e41f24.

📒 Files selected for processing (1)

v2/pkg/grpctest/mockservice_requires.go

Noroth

LGTM

🤖 I have created a release *beep* *boop* --- ## [2.0.0-rc.268](v2.0.0-rc.267...v2.0.0-rc.268) (2026-04-07) ### Features * add support for fields with arguments and requires directive ([#1452](#1452)) ([58469c3](58469c3)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

dkorittki added 3 commits March 18, 2026 15:56

chore: regenerate proto files in grpctest

d30f8a9

Just to ensure a clean diff of what really changes once the grpctest SDL changes.

chore: extend grpctest, include field with args and @requires

6805a6b

chore: implement rpc RequireStorageFilteredTagSummaryById on grpctest

96b6b68

dkorittki added 4 commits March 20, 2026 10:24

chore: add test

23ea61e

feat: add implementation

69d01d6

chore: add tests for nullable and repeated

ba08981

Merge branch 'master' into dominik/eng-9218-implement-requires-with-f…

a7267a5

…ield-resolvers-in-go-tools

dkorittki marked this pull request as ready for review March 20, 2026 12:28

dkorittki requested review from Noroth, StarpTech, devsergiy, jensneuse and ysmolski as code owners March 20, 2026 12:28

claude bot reviewed Mar 20, 2026

View reviewed changes

coderabbitai bot reviewed Mar 20, 2026

View reviewed changes

dkorittki added 2 commits March 20, 2026 14:26

chore: invert if to early exit condition

5036bd6

Merge branch 'master' into dominik/eng-9218-implement-requires-with-f…

fc1c2d3

…ield-resolvers-in-go-tools

coderabbitai bot reviewed Apr 1, 2026

View reviewed changes

v2/pkg/grpctest/mockservice_requires.go Outdated Show resolved Hide resolved

dkorittki mentioned this pull request Apr 2, 2026

feat(router): support fields with arguments and @requires (Cosmo Connect) wundergraph/cosmo#2731

Merged

5 tasks

fix: improve loop exit condition

7e41f24

coderabbitai bot reviewed Apr 2, 2026

View reviewed changes

Noroth approved these changes Apr 2, 2026

View reviewed changes

dkorittki merged commit 58469c3 into master Apr 7, 2026
11 checks passed

dkorittki deleted the dominik/eng-9218-implement-requires-with-field-resolvers-in-go-tools branch April 7, 2026 12:48

StarpTech mentioned this pull request Apr 7, 2026

chore(master): release 2.0.0-rc.268 #1462

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add support for fields with arguments and requires directive#1452

feat: add support for fields with arguments and requires directive#1452
dkorittki merged 10 commits intomasterfrom
dominik/eng-9218-implement-requires-with-field-resolvers-in-go-tools

dkorittki commented Mar 18, 2026 •

edited

Loading

Uh oh!

coderabbitai bot commented Mar 18, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Uh oh!

claude bot left a comment

Uh oh!

coderabbitai bot left a comment

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

coderabbitai bot left a comment

Uh oh!

Noroth left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

dkorittki commented Mar 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Context

Implementation

Tests

Checklist

Uh oh!

coderabbitai bot commented Mar 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Uh oh!

claude bot left a comment

Choose a reason for hiding this comment

Claude Code Review

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Noroth left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dkorittki commented Mar 18, 2026 •

edited

Loading

coderabbitai bot commented Mar 18, 2026 •

edited

Loading