Accept valid empty collections after gRPC transport - #3988
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
Requireddeclarations 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:
make testwith coverage.make integration-test, using a temporary binary built from this checkout; JSON-RPC integration tests passed.make lintinvocation encountered another linter's lock; the equivalent full command passed with serial-runner waiting enabled.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 av3PR. It is a small follow-up to #3971, not authorization to merge that parent. The existing GitHub workflow runs only for PRs targetingv3, 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.