Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert the first letter of method name to upper #300

Merged
merged 1 commit into from
Jan 27, 2017

Conversation

lipixun
Copy link
Contributor

@lipixun lipixun commented Jan 15, 2017

It'll generate wrong method name if the method name starts with a lower-case letter in the proto file of gRPC.

The code generation rule of protoc-go will convert the first letter to upper-case but grpc-gateway do not.

So I just slightly changed the method name before applying the template.

@tmc
Copy link
Collaborator

tmc commented Jan 15, 2017

Thanks for your contribution. Can you sign the CLA and demonstrate this behavior in an example or a test?

@lipixun
Copy link
Contributor Author

lipixun commented Jan 27, 2017

All right. I have signed the CLA.

It seems a little hard to write test code into current test cases, the tests are ran on a mock output of protoc but this pr focus on fix the issue of integration with protoc-go.

Here's the test case:

Write a protobuf file:

syntax = "proto3";

package test;

import "google/api/annotations.proto";

option go_package = "testpkg";

message SomeRequest {
    int32 start = 1;
    int32 size = 2;
}

message SomeResponse {
    repeated string items = 1;
}

service SomeService {
    rpc list(SomeRequest) returns (SomeResponse) {
        option (google.api.http) = {
            post: "/resource/list"
            body: "*"
        };
    }
}

Pay attention to the method list not List, and run protoc protoc -I . -I $GOPATH/src/github.com/grpc-ecosystem/grpc-gateway/third_party/googleapis --go_out=plugins=grpc:. test.proto

This will generate the go source code file with the following codes:

func (c *someServiceClient) List(ctx context.Context, in *SomeRequest, opts ...grpc.CallOption) (*SomeResponse, error) {
	out := new(SomeResponse)
	err := grpc.Invoke(ctx, "/test.SomeService/list", in, out, c.cc, opts...)
	if err != nil {
		return nil, err
	}
	return out, nil
}

The method name is List not list.

Run the grpc gateway compile command: protoc -I=. -I $GOPATH/src/github.com/grpc-ecosystem/grpc-gateway/third_party/googleapis --grpc-gateway_out=logtostderr=true:. test.proto

The generated grpc gateway code:

func request_SomeService_list_0(ctx context.Context, marshaler runtime.Marshaler, client SomeServiceClient, req *http.Request, pathParams map[string]string) (proto.Message, runtime.ServerMetadata, error) {
	var protoReq SomeRequest
	var metadata runtime.ServerMetadata

	if err := marshaler.NewDecoder(req.Body).Decode(&protoReq); err != nil {
		return nil, metadata, grpc.Errorf(codes.InvalidArgument, "%v", err)
	}

	msg, err := client.list(ctx, &protoReq, grpc.Header(&metadata.HeaderMD), grpc.Trailer(&metadata.TrailerMD))
	return msg, metadata, err

}

Take a look at client.list, it should be client.List. And what I've committed is to convert the first letter from lower case to upper case.

The protobuf official document says:

The first letter is capitalized for export. If the first character is an underscore, it is removed and a capital X is prepended.

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@lipixun
Copy link
Contributor Author

lipixun commented Jan 27, 2017

I signed it!

1 similar comment
@lipixun
Copy link
Contributor Author

lipixun commented Jan 27, 2017

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

@tmc tmc merged commit 6863684 into grpc-ecosystem:master Jan 27, 2017
@tamalsaha tamalsaha mentioned this pull request Mar 30, 2017
1 task
adasari pushed a commit to adasari/grpc-gateway that referenced this pull request Apr 9, 2020
Convert the first letter of method name to upper
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.

None yet

3 participants