Fix parameter names when using JSON names.

[brocaar]

When using json_names_for_fields=true, I noticed that this works fine for field names in the request body when generating Swagger definitions, but URL parameters remain snake_case.

I believe this should fix it. Looking forward to your feedback.

[johanbrandhorst]

Thanks for this, that looks reasonable! Could we add some tests that cover this? I think the existing framework should allow you to specify this flag.

[codecov-io]

Codecov Report

Merging #879 into master will increase coverage by 0.13%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #879      +/-   ##
==========================================
+ Coverage   52.99%   53.12%   +0.13%     
==========================================
  Files          39       39              
  Lines        3910     3891      -19     
==========================================
- Hits         2072     2067       -5     
+ Misses       1642     1630      -12     
+ Partials      196      194       -2

Impacted Files	Coverage Δ
protoc-gen-swagger/genswagger/template.go	53.68% <100%> (+0.42%)	⬆️
protoc-gen-swagger/main.go	26% <0%> (-1.36%)	⬇️
protoc-gen-grpc-gateway/gengateway/template.go	64.28% <0%> (-1.24%)	⬇️
protoc-gen-grpc-gateway/descriptor/registry.go	62.03% <0%> (+1.12%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 70fc086...a437376. Read the comment docs.

[brocaar]

@johanbrandhorst are there any tests that cover the GetJsonName functionality to which I can append this?

[johanbrandhorst]

@johanbrandhorst are there any tests that cover the GetJsonName functionality to which I can append this?

I'm not sure, but I think you should be able to copy and modify one of the tests in https://github.com/grpc-ecosystem/grpc-gateway/blob/master/protoc-gen-swagger/genswagger/template_test.go. Let me know if you need more assistance.

[brocaar]

I've pushed a test which tests a field with and without json_name. I've also added a fallback to GetName when GetJsonName returns "". Without that fallback, 413f6e2#diff-20461b822a903046234d0fa5073ba2c4R236 would be "".

I believe the same must be added to:
https://github.com/grpc-ecosystem/grpc-gateway/blob/master/protoc-gen-swagger/genswagger/template.go#L310 ?

if reg.GetUseJSONNamesForFields() {
	kv.Key = f.GetJsonName()
}

if !reg.GetUseJSONNamesForFields() || kv.Key == "" {
	kv.Key = f.GetName()
}

It was not clear to me how to add a testcase for that. Would you mind making that modification?

[johanbrandhorst]

I've pushed a test which tests a field with and without json_name. I've also added a fallback to GetName when GetJsonName returns "". Without that fallback, 413f6e2#diff-20461b822a903046234d0fa5073ba2c4R236 would be "".

I believe the same must be added to:
https://github.com/grpc-ecosystem/grpc-gateway/blob/master/protoc-gen-swagger/genswagger/template.go#L310 ?

It was not clear to me how to add a testcase for that. Would you mind making that modification?

I expect the reason that isn't required is because the JsonName is guaranteed by the protoc compiler. See https://github.com/protocolbuffers/protobuf/blob/f2ef7970fe704c4ec604d39fc1d70bec2c59e4a7/src/google/protobuf/descriptor.proto#L207. You can probably remove it from your test case and the code as well if you wish. Otherwise looks good!

[brocaar]

Ah, I didn't know that. I've removed the fallback :-)

[johanbrandhorst]

Great work, thanks for this!

[brocaar]

Thanks for reviewing and your prompt responses!