fix(gengapic): ordered dynamic header values for Firestore #1685
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
bhshkh
changed the title
shollyman
approved these changes
Jan 14, 2026
| inputType := &descriptorpb.DescriptorProto{ | ||
| Name: proto.String("InputType"), | ||
| Field: []*descriptorpb.FieldDescriptorProto{ | ||
| {Name: proto.String("field1"), Type: typePtr(descriptorpb.FieldDescriptorProto_TYPE_STRING)}, |
Contributor
There was a problem hiding this comment.
I'm unsure of this typePtr usage. Can't you use https://pkg.go.dev/google.golang.org/protobuf/types/descriptorpb#FieldDescriptorProto_Type.Enum e.g. descriptorpb.FieldDescriptorProto_TYPE_STRING.Enum()?
This was referenced Jan 14, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Reintroduce #1661 only for Firestore
b/456536006
This PR re-implements the fix for ordered dynamic routing headers (originally in #1661) but scopes it strictly to Firestore clients to avoid regressions in other libraries like Storage.
Problem: The Firestore backend requires x-goog-request-params header values to be in a specific order (e.g., project_id must appear before database_id). The current generator uses a Go map iteration to build this header string, resulting in non-deterministic ordering that causes intermittent InvalidArgument errors in the Firestore Go client. A previous attempt to fix this globally (#1661) caused regressions in the Storage client and was reverted.
Solution:
Introduced an allowlist enableOrderedRoutingHeaders in internal/gengapic/generator.go, currently enabled for google.firestore.v1 and google.firestore.admin.v1.
Modified insertDynamicRequestHeaders in internal/gengapic/gengapic.go:
For allowlisted packages (Firestore): The code now iterates over the header rules in the order defined in the proto. It checks the routingHeadersMap for each key, appends the value, and deletes the key to prevent duplicates. This ensures deterministic ordering while preserving the "last value wins" semantics for duplicate keys.
For other packages (Storage, etc.): The code falls back to the existing behavior (iterating over the map), ensuring zero changes to the generated code for non-target libraries.