Added UseProtoNames functionality
…Allows the preservation of the proto snake_case names rather than the lowerCamelCase that is used by default when marshaling to json. I wasn't exactly sure where you wanted me to set the flag for this to get passed on to the marshaler so I added the nrpc.UseProtoNames bool. Let me know if you would like it somewhere else and I will make changes accordingly.
I also added the optional use_proto_names option that can get passed through via protoc args. (like prometheus)
A global MarshalOptions, named JsonMarshalOptions, would be better so we can change any option, not only UseProtoNames.
About the use_proto_names option it should not be a protoc flag because it changes the protocol itself, unlike prometheus. I suggest you remove the protoc flag, and if really needed you can add if as a nrpc option in the proto files.
That's what I figured you might say. I will get on it :)
Thinking things through I figured it was best to make the option be part of the service since that is really what is important. Now here is the problem I need your thoughts on. If I use a global MarshalOptions in nrpc.go the option would be set globally not per service. This would mean that if someone technically wanted to run two services in the same app, they would both be using the same MarshalOptions which would not honor the "per service" idea, or even per proto file for that matter. If I am reading things right, I should be able to add the MarshalOptions to nrpc.Request and handler for the server and I will have to add it to the generated client and pass it around to nrpc.Call for the client. Is this right?
I "think" I got everything. It's setup to be tied to the service, not global. Its also setup to allow for more MarshalOptions in the future.
Better indeed thanks.
Passing the options to the base functions is not very elegant but avoiding this without having global variables requires a little more refactoring so we will keep things as you did.
However I think having this as a service option is misleading. If two services use the same message type but not the same json settings, it will result in the same message encoded in two different ways depending on the service. Plus having different encoding options in the same API is confusing. So I would prefer the option to be global to the API.
Last thing, you also need to update the examples by regenerating their files (go generate ./examples/... should do it). Ideally use the same protoc and protoc-go-gen versions as I did previously.
I agree refactoring could make things a great deal more elegant with what I added. I didn't want to try and change too much in my first go around with the project :)
I may have misunderstood your first bit of feedback. If you don't really think it needs to be an nrpc proto option it would be a great deal simpler to just add the global JsonMarshalOptions into the nrpc package and just have a setter func for cleanliness. Then the API could just call something like nrpc.SetJsonUseProtoNames(true). Then I can move the Marshaler back to just using the nrpc.JsonMarshalOptions variable.
If that's not the case and you would like it to be part of the protobuf package, would you like it to be a file level option?
I downloaded the latest versions of protoc & protoc-go-gen. Is it listen somewhere what versions you used?
Thanks for your patients working with me on this :)
- Setting 'UseProtoNames' changes the protocol, so it should be in the .proto files
- We want a same message to always be encoded the same way, so it should be a file-wise option, not service-wise.
- If 2 different APIs with different settings are used in the same program we want each to respect their own setting. So we cannot use a global nrpc setting.
-> I suggest we generate a JsonMarshalOptions in the .nrpc.go file and pass it to the various nrpc functions.
With all this we will still have a corner case when the services of a .proto file use types defined in another .proto files. The ideal solution would require always knowing in which file a given message was defined, but it feels too complicated to start with, and easy to workaround (ie, have all the files define the same option).
The versions of the generators are written in the generated .pb.go files.
If we stick to protobuf good practices, this feature is better not implemented.
We should respect the protobuf json mapping (https://protobuf.dev/programming-guides/proto3/#json), and not temper with generated code (https://protobuf.dev/reference/go/faq/#custom-code).