protoc-gen-grpc-gateway: Error if identical annotations are provided

[guodongq]

📚 Documentation/Question

Is there any detection mechanism to check for duplicates of the corresponding RESTful URIs when I generate the Grpc-gateway files?

[johanbrandhorst]

I think the generator should error if you have two endpoints with identical annotations, though I'm not sure if we have a test for this. Have you tried it?

[guodongq]

I tried it, and it didn't give an error

[johanbrandhorst]

Alright, we should probably make the generator error in that case. If you'd like to contribute, the first step would be to add a new test that fails under the existing behavior. It should be possible to copy and adapt one of the tests in https://github.com/grpc-ecosystem/grpc-gateway/blob/master/protoc-gen-grpc-gateway/internal/gengateway/template_test.go to reproduce this bug. Once we have a failing test, it should be easy to add some logic to check for duplicates and confirm that it works by running the test. Let me know if I can do anything more to help get you started.

[danedavid]

Hi @johanbrandhorst I'd like to tackle this issue, and I'd looked into it. I added a failing test to template_test.go by using the same Binding twice. Now, to make Generate fail, we need to check if HTTP method-URI combos are recurring across files, right? My intuition was to use a map to store HTTP method-URI as keys and set them to true once seen. But the method and URI are available only inside applyTemplate. So does that mean we need to pass the map down from generate? Or use a global map in template.go? Or is there any better, elegant way to do it?

[johanbrandhorst]

Hi Dane, thank you so much for picking up this issue! If you could create a pull request with your proposed changes it would be easier for me to review, but preliminarily your suggestion sounds OK, that we'll need some way to check duplicate annotations across all input files. Passing in using explicit parameters is almost always better than using a global.

[danedavid]

Raised a PR here for fixing this.
Have added a map and a method to the exported descriptor.Registry; not sure if that is the right place to add it. Otherwise would have had to change the API of applyTemplate.

Let me know what you think.