Skip to content

Fix opaque missing imports casing#6304

Merged
johanbrandhorst merged 19 commits intogrpc-ecosystem:mainfrom
kellen-miller:fix-opaque-missing-imports-casing
Feb 2, 2026
Merged

Fix opaque missing imports casing#6304
johanbrandhorst merged 19 commits intogrpc-ecosystem:mainfrom
kellen-miller:fix-opaque-missing-imports-casing

Conversation

@kellen-miller
Copy link
Copy Markdown
Contributor

@kellen-miller kellen-miller commented Jan 30, 2026

References to other Issues or PRs

Post Revert changes

After the revert #6299 due to #6294, I added a gate to make sure that the new addBodyFieldImports function only executes if the opaque api is being used.

// addBodyFieldImports ensures nested body message types pull in their Go packages.
func (g *generator) addBodyFieldImports(
	file *descriptor.File,
	m *descriptor.Method,
	pkgSeen map[string]bool,
) []descriptor.GoPackage {
	if g.reg == nil || !g.useOpaqueAPI {
		return nil
	}

My camel_case_service should have caught that except it imported an enum from the sub message, which caused the sub message import to be used in the gateway file due to runtime.Enum using the type from the sub message. Sorry for the miss on that @ryanriccio1 @johanbrandhorst.

Have you read the Contributing Guidelines?

Yes

Brief description of what is fixed or changed

  • Register the Go packages that define nested body messages so opaque handlers (--use_opaque_api) import the modules
    they instantiate.
  • Camel-case every identifier produced by descriptor.Message.GoType / Enum.GoType, ensuring proto names such as
    Create_book resolve to valid Go identifiers.
  • Add exercised examples under examples/internal/proto/examplepb (opaque import with snake_case service/RPC names) and
    examples/internal/proto/sub (snake-case messages) so go test ./... guards both regressions without generator-only
    unit tests.

Other comments

  • go build ./...
  • go test ./...

[WHY] Opaque-enabled protos expose nested fields only through getter/setter chains, so generated Setkey.* calls failed to compile yet path params remained unset.
[WHAT] Added FieldPath helpers to build setter chains, swapped template setter sites to the helper, and codified the behavior with unit tests.
[HOW] Build the accessor expression once from the descriptor so every opaque path-param assignment reuses it.
[TEST] go test ./internal/descriptor; go test ./protoc-gen-grpc-gateway/internal/gengateway
[RISK] low; changes affect only opaque path-param generation paths.
[RAG] finxact_engineering_domain_customerv1poc_proto::module::buf|1.30,finxact_engineering_domain_customerv1poc_proto::module::finxact_type|1.59,finxact_engineering_domain_customerv1poc_gen_go::repo_overview|1.70
[WHY] Prevent regressions for the nested enum path-param case that triggered the bug.
[WHAT] Add a generator test that synthesizes a proto3 file with an opaque nested enum binding and asserts the emitted setter chain.
[HOW] Load the fake file into the descriptor registry so LookupEnum succeeds and search for the expected SetKind expression.
[TEST] go test ./protoc-gen-grpc-gateway/internal/gengateway
[RISK] none; test-only change
[RAG] finxact_engineering_domain_customerv1poc_proto::module::buf|1.30,finxact_engineering_domain_customerv1poc_proto::module::finxact_type|1.59,finxact_engineering_domain_customerv1poc_gen_go::repo_overview|1.70
[WHY] The opaque setter bug isn’t limited to enums or PATCH bindings, so tests should cover both scalar and enum parameters across HTTP methods.
[WHAT] Simplified the generator test to synthesize a proto with top-level path params and assert the opaque setter is emitted for GET (string) and PATCH (enum).
[HOW] Build descriptors directly, cross-link them once, and verify the generated code contains the expected setter expression for each case.
[TEST] go test ./protoc-gen-grpc-gateway/internal/gengateway
[RISK] none; test-only change
[RAG] finxact_engineering_domain_customerv1poc_proto::module::buf|1.30,finxact_engineering_domain_customerv1poc_proto::module::finxact_type|1.59,finxact_engineering_domain_customerv1poc_gen_go::repo_overview|1.70
[WHY] Opaque handlers were missing imports for nested body types and
snake_case messages/enums produced invalid Go identifiers, causing
broken builds when `--use_opaque_api` was enabled.

[WHAT] Camel-case message/enum GoType helpers, register body-field
packages during generation, and add example protos under
`examples/internal/proto/examplepb` (opaque import + snake-case
service/RPC names) and `examples/internal/proto/sub` (snake-case type) so
`go test ./...` guards the regressions without extra generator-only
tests.

[HOW] Relocated the opaque example, regenerated it against the opaque
API, extended the gateway generator to track external body packages, and
leaned on the compiled examples instead of bespoke generator tests.

[TEST] go test ./...
[RISK] Low; scoped to generators/examples.

Fixes grpc-ecosystem#6276, grpc-ecosystem#6277
…update message references

- Introduced `OpaquePatchProductNameRequest` and `OpaquePatchProductNameResponse` types to the example proto.
- Registered new request/response types in the services and updated references to message indices accordingly.
This commit deletes the outdated `examplepb/BUILD.bazel` file and eliminates unused proto and Bazel rule definitions. It also updates dependencies in `examplepb_opaque_proto` and `examplepb_opaque_go_proto` to include a missing reference to `sub_proto`, improving build correctness and simplifying the overall setup.
…ts-casing

# Conflicts:
#	examples/internal/proto/examplepb/opaque.pb.go
#	examples/internal/proto/examplepb/opaque.pb.gw.go
#	examples/internal/proto/examplepb/opaque.proto
#	examples/internal/proto/examplepb/opaque.swagger.json
#	examples/internal/proto/examplepb/opaque_grpc.pb.go
  [WHY] Non-opaque builds started failing with unused imports after the opaque body fix because we always pulled nested body packages,
  even when the generated code never referenced them.
  [WHAT] Only add body-field imports when --use_opaque_api is enabled, add an OpaqueEchoNote example under examplepb/opaque.proto that
  exercises cross-package opaque bodies, and regenerate the example artifacts.
  [HOW] Gated addBodyFieldImports, created a registry-backed unit test to cover both modes, and regenerated the opaque example outputs
  with the latest plugin.
  [TEST] go test ./...
  [RISK] Low; generator change scoped to opaque mode plus example updates.
@johanbrandhorst
Copy link
Copy Markdown
Collaborator

Thanks a lot for the quick fix. @ryanriccio1 could you please try out this PR in your environment and confirm if it fixes the issue you were having? You should be able to install it by cloning the branch off @kellen-miller's fork and running go install ./protoc-gen-grpc-gateway. If that doesn't work for you I can merge this and you'll be able to get it from our main branch.

@ryanriccio1
Copy link
Copy Markdown

@kellen-miller @johanbrandhorst Just pulled it and it looks good on my end!

@johanbrandhorst johanbrandhorst merged commit b7a1deb into grpc-ecosystem:main Feb 2, 2026
14 checks passed
@johanbrandhorst
Copy link
Copy Markdown
Collaborator

This is why I love open source, thanks all :)

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.

Opaque API handlers miss imports for nested request bodies Snake_case proto names not changed into generated Go identifiers

3 participants