Fix naming conventions for generated gRPC templates - #81
Open
thzgajendra wants to merge 4 commits into
Open
Conversation
The generated GoFr gRPC wrapper used a "Server" suffix for the CLI's own
service types, e.g. RegisterHelloServerWithGofr / NewHelloGoFrServer. These
represent a service, not a server, and clashed conceptually with the
protoc-generated *Server symbols. Rename the CLI-owned identifiers to use
"Service":
- {{.Service}}GoFrServer -> {{.Service}}GoFrService
- {{.Service}}ServerWithGofr -> {{.Service}}ServiceWithGofr
- {{.Service}}ServerWrapper -> {{.Service}}ServiceWrapper
- Register{{.Service}}ServerWithGofr -> Register{{.Service}}ServiceWithGofr
protoc-gen-go-grpc contract symbols are intentionally left as-is
(embedded {{.Service}}Server, mustEmbedUnimplemented{{.Service}}Server,
Register{{.Service}}Server, {{.Service}}_{{.Name}}Server streams,
{{.Service}}Client) since the generated code must reference them.
Resolves gofr-dev#40
Cover the generated GoFr gRPC wrapper/server/client naming so gofr-dev#40 cannot regress: assert the CLI-owned types are service-named (RegisterHelloServiceWithGofr, HelloGoFrService, HelloServiceWithGofr, HelloServiceWrapper) while the protoc-gen-go-grpc contract symbols the code references are left unchanged (mustEmbedUnimplementedHelloServer, RegisterHelloServer, Hello_<Method>Server streams). This is the first test coverage for the wrap package.
…y imports)
Found while running the generated gRPC server end-to-end. Two pre-existing
bugs made the generated server package fail to compile:
1. The request-wrapper template printed the whole ServiceRequest struct via
{{ $request }}, emitting `type {HelloRequest HelloRequest}Wrapper struct`
instead of `type HelloRequestWrapper struct`. Use {{ $request.Request }}.
2. The server wrapper imported "time" and the gofr gRPC logger
unconditionally, but both are only used by the streaming instrumentation.
A unary-only service therefore failed with "imported and not used". Gate
those two imports on the existing hasStream flag (hoisted above the import
block).
Verified end-to-end: generated + built + ran a GoFr gRPC server for both a
unary-only and a streaming service (no manual edits) and called it with a
real client. Added tests covering both fixes.
…ream-only imports)" This reverts commit 26522fc.
Umang01-hash
approved these changes
Jul 30, 2026
Umang01-hash
left a comment
Umang01-hash
left a comment
Member
There was a problem hiding this comment.
Verified locally — this matches what #40 asked for. Renames the CLI-owned generated types to Service (NewXGoFrService, XServiceWithGofr, XServiceWrapper, RegisterXServiceWithGofr) while correctly leaving the protoc-contract symbols as Server (mustEmbedUnimplementedXServer, RegisterXServer, the generic registerServerWithGofr helper). Grep confirms no stray old names left; the test pins both halves. Build + go test ./wrap green.
Two notes:
- This changes generated output, so existing users'
main.gocalls (RegisterXServerWithGofr) will need updating on regenerate — worth a line in the release notes. Also the gofr.dev gRPC docs still show the old name; worth updating there too. - #82 also adds
wrap/template_test.gowith the samecreateTestContext, so this and #82 conflict on that file — whichever merges second needs a quick rebase to combine them. LGTM.
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
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.
Objective
Resolves #40 — the generated GoFr gRPC wrapper names the CLI's own service types with a
Serversuffix; rename them toService.What changed
Renamed the CLI-owned identifiers in
wrap/template.gofromServer→Service:{{.Service}}GoFrServer{{.Service}}GoFrService{{.Service}}ServerWithGofr{{.Service}}ServiceWithGofr{{.Service}}ServerWrapper{{.Service}}ServiceWrapperRegister{{.Service}}ServerWithGofrRegister{{.Service}}ServiceWithGofrLeft as
Server(protoc-gen-go-grpc contract symbols the generated code references): embedded{{.Service}}Server,mustEmbedUnimplemented{{.Service}}Server(),Register{{.Service}}Server(...),{{.Service}}_{{.Name}}Serverstreams,{{.Service}}Client/New{{.Service}}Client(...), the generic helperregisterServerWithGofr, andgrpc.ServerStream. The client template needed no change.Tests
Adds
wrap/template_test.go(first coverage for the package) asserting the CLI-owned types are service-named and the old names are gone, while every protoc contract symbol is preserved, and the client surface is unaffected.Test plan
go test ./...— all pass, incl. the newwrapnaming tests.gofmt/go vet ./...clean.Scope note
This PR is now naming-only. Two unrelated code-generation bugs I'd originally bundled here were split out so each fix is independently mergeable:
{X X}Wrappertemplate literal leaks into output (v0.8.1) #75).time/gofrgRPC) bug → still unfiled; happy to open a separate issue/PR.