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

fix: proto message unable to bind path param #3303

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

chenliang
Copy link
Contributor

Description (what this PR does / why we need it):

proto-gen-go-http 在调用时使用binding.EncodeURL 进行参数绑定,path中使用snakeCase命名方法会导致参数绑定失败,使用camelCase则正常。
proto 的命名规范为snakeCase。
例如

pathTemplate := "/v1/enterprise/{enterprise_id}"
in := &pcv1.GetEnterpriseInfoReq{
EnterpriseId: "123",
}
path := binding.EncodeURL(pathTemplate, in, true)
fmt.Println(path) // 输出: /v1/enterprise/?enterpriseId=123
path = binding.EncodeURL("/v1/enterprise/{enterpriseId}", in, true)
fmt.Println(path) // 输出: /v1/enterprise/123

Which issue(s) this PR fixes (resolves / be part of):

原因:
form.EncodeValues读取字段的值如果是proto类型的使用的是protobuf中的json字段命名camelCase。
后续匹配path参数(命名规则为snakeCase)则无法匹配到。
解决方法:
绑定参数时候判断是否为proto 消息,如果是proto消息并且未匹配到参数直接修改key为camelCase 进行匹配

@dosubot dosubot bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Apr 25, 2024
@demoManito
Copy link
Member

I think enterprise_id and enterpriseId should be two different params in the URL? If your param is enterprise_id should add [json_name = "enterprise_id"] in the field.

@chenliang
Copy link
Contributor Author

I think enterprise_id and enterpriseId should be two different params in the URL? If your param is enterprise_id should add [json_name = "enterprise_id"] in the field.

可能是我描述的不太清楚,我的proto定义

service EnterpriseService {
  //获取企业信息
  rpc GetEnterpriseInfo(GetEnterpriseInfoReq) returns (GetEnterpriseInfoResp) {
    option (google.api.http) = {get: "/v1/enterprise/{enterprise_id}"};
  }
}

message GetEnterpriseInfoReq {
  string enterprise_id = 1 [(validate.rules).string.min_len = 1];
}
message GetEnterpriseInfoResp {}

使用proto-gen-go-http生成的httpClient调用中无法正确的填充path参数的值,导致调用失败
原因在于 proto-gen-go-http生成的代码中,直接使用 /v1/enterprise/{enterprise_id} 作为路由
binding时直接取enterprise_id这个作为key,但是解析请求参数的时候form.EncodeValues如果是proto.Message,取的key为enterpriseId ,并不是 enterprise_id,导致无法正确的填充path参数的值

添加json_name确实能解决path填充失败问题,但是也产生了新的问题,导致json输出的时候 字段命名也变为了json_name的值,即enterprise_id。
这并不是我想要的,我希望在proto定义中使用下划线,但是在json输出的时候使用驼峰命名.这也是kratos默认的行为。
proto在不通语言生成的代码命名规则是不一样的,python使用under_score,java使用驼峰命名,js也是驼峰命名,我希望符合每个语言的命名规范。
解决方案也有好几种,可以在proto-gen-go-http中使用驼峰命名,也能解决问题,就看如何解决这个问题。
另外kratos作为http服务,不同的命名规则参数也是兼容的,即 enterprise_id或者enterpriseId都能正确的填充到 proto.Message中.
我修改binding.EncodeURL方法也是为了这种兼容性处理。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:S This PR changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants