Skip to content

Accept valid empty collections after gRPC transport - #3988

Merged
raphael merged 1 commit into
fix/goa-generation-planfrom
fix/grpc-required-collections
Sep 10, 2026
Merged

Accept valid empty collections after gRPC transport#3988
raphael merged 1 commit into
fix/goa-generation-planfrom
fix/grpc-required-collections

Conversation

@raphael

@raphael raphael commented Sep 10, 2026

Copy link
Copy Markdown
Member

Empty collections must survive gRPC

A required array can validly contain no items. In a downstream audit result, changes: [] means the comparison found no changes; a second required list can likewise be empty. Both lists are correctly present in JSON, but protobuf omits empty repeated fields and decodes them as nil Go slices. The generated gRPC response validator then incorrectly returns a missing-field error. Maps have the same protobuf presence limitation.

This change accepts those valid empty collections after a protobuf round trip. It does not remove the authored Required declarations or change JSON: omitted and null required JSON collections still fail validation, while explicit empty collections pass unless a minimum length forbids them.

Validation stays with the transport that can enforce it

The protobuf attribute context enables a collection-presence policy used only while generating validation. It suppresses missing-field checks for actual array and map fields. Length limits, array-item checks, required messages, explicit scalar presence, and oneof selection checks remain unchanged. No protobuf schema, service conversion, defaults, HTTP validation, or runtime mode changes are introduced.

Both validation implementations now use the same required-field selector, replacing duplicated selection logic. Two expected-output fixtures lose functions whose only purpose was the impossible collection-presence check. The repository instruction about empty required collections is corrected to distinguish JSON property presence from protobuf collection values.

Review protoBufTypeContext, generatedRequiredValidationNames, and the generated dual-transport test first. That test generates real client/server and protobuf packages from one design, serializes and decodes empty/populated collections, and exercises the generated gRPC and HTTP validators. Existing required-oneof and scalar-presence tests retain their expectations.

Verification and adoption

Passed locally on macOS:

  • Full make test with coverage.
  • make integration-test, using a temporary binary built from this checkout; JSON-RPC integration tests passed.
  • Full pinned golangci-lint: zero issues. The initial make lint invocation encountered another linter's lock; the equivalent full command passed with serial-runner waiting enabled.
  • Focused race tests for the new generated round-trip/JSON proof and existing collection-wrapper, required-oneof, and scalar-presence tests.
  • Independent reasoning review of the complete final diff.

Regenerate affected gRPC packages to adopt the corrected validators. The wire contract and ordinary service/client interfaces do not change; no data migration or ordered deployment is required. Rolling back generation restores the erroneous rejection of empty collections. Presence-only generated validation helpers may disappear when no enforceable checks remain.

This PR is deliberately based on fix/goa-generation-plan, the current preview branch used by the affected application, rather than adding that branch's unrelated changes to a v3 PR. It is a small follow-up to #3971, not authorization to merge that parent. The existing GitHub workflow runs only for PRs targeting v3, so this stacked PR will not automatically receive that workflow; the local passes above are not claims of Linux/Windows CI results. No version publication, dependency update, or merge is included.

@raphael
raphael merged commit 3a1f59e into fix/goa-generation-plan Sep 10, 2026
1 check passed
@raphael
raphael deleted the fix/grpc-required-collections branch September 10, 2026 19:31
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