Skip to content

Fix broken request wrappers in generated gRPC code (#75) - #82

Open
thzgajendra wants to merge 1 commit into
gofr-dev:mainfrom
thzgajendra:fix/grpc-request-wrapper
Open

Fix broken request wrappers in generated gRPC code (#75)#82
thzgajendra wants to merge 1 commit into
gofr-dev:mainfrom
thzgajendra:fix/grpc-request-wrapper

Conversation

@thzgajendra

Copy link
Copy Markdown
Contributor

Objective

Fixes #75.

gofr wrap grpc server generates broken Go: every request-wrapper type/method renders as a struct literal (e.g. {GetThingRequest GetThingRequest}Wrapper) instead of GetThingRequestWrapper, so request_gofr.go and its use sites don't compile (syntax error: unexpected {, expected name).

Root cause

The request-wrapper block ranges over .Requests ([]ServiceRequest) but referenced the loop variable directly:

{{- range $request := .Requests }}
type {{ $request }}Wrapper struct {   // $request is a struct, not a string

text/template falls back to fmt's default struct format {field1 field2}, so {{ $request }} renders as {GetThingRequest GetThingRequest}.

Fix

Use {{ $request.Request }} at every site in the block — the type, the embedded field, all five receiver methods, and the reflect.ValueOf(h.…) call in Bind. One-line-per-site template change; no behavior change beyond emitting the correct identifier.

Tests

Adds wrap/template_test.go (first test coverage for the wrap package): renders generateGoFrRequestWrapper for multiple request types and asserts each becomes <Type>Wrapper (type, Context, Bind, embedded field, reflect call) and that no struct literal leaks into the output.

Test plan

  • go test ./... passes (incl. the new wrap test).
  • End-to-end, no manual edits: generated protoc stubs + the GoFr wrapper via this CLI for a proto with two request types, wired main.go, and go build ./... succeeds — request_gofr.go now contains type GetThingRequestWrapper struct / *GetThingRequest / reflect.ValueOf(h.GetThingRequest).
  • gofmt / go vet ./... clean.

Docs

No documentation change needed — the README only lists wrap grpc as a feature and doesn't reference the generated wrapper internals; this is a code-generation correctness fix.

Note

The same one-line fix is also present, bundled with unrelated naming changes, in the still-open #81. This PR isolates it as the focused fix for #75 so it can merge on its own.

The request-wrapper template ranges over .Requests ([]ServiceRequest) but
referenced the loop variable directly as `{{ $request }}`. text/template
falls back to fmt's struct format, so it rendered `{GetThingRequest
GetThingRequest}` — producing `type {GetThingRequest GetThingRequest}Wrapper`
and `*{GetThingRequest GetThingRequest}`, which do not compile
("syntax error: unexpected {, expected name/type").

Use `{{ $request.Request }}` at every site in the request-wrapper block
(type, embedded field, all receiver methods, and the Bind reflect call).

Adds the first test for the wrap package, asserting the request wrappers
render as `<Type>Wrapper` for multiple request types and that no struct
literal leaks into the output.

Fixes gofr-dev#75

@Umang01-hash Umang01-hash left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified locally — nice fix. Confirmed the bug: on main the generated request wrapper doesn't even parse as Go ({GetThingRequest GetThingRequest}Wrapperexpected 'IDENT', found '{'), and with this change it parses cleanly. Every {{ $request }} site is updated to {{ $request.Request }}, and the regression test asserts no struct-literal leaks. Build + go test ./wrap green.

Since this fixes non-compiling output (any proto with >1 request type), I'd merge this first.

Heads up: #81 also adds wrap/template_test.go with the same createTestContext helper, so whichever of the two lands second will need a quick rebase to combine the test file (one createTestContext). LGTM.

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.

wrap grpc generates broken request wrappers — {X X}Wrapper template literal leaks into output (v0.8.1)

2 participants