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

Modify proto files to improve generated Java code #54

Open
reify-tanner-stirrat opened this issue Feb 15, 2023 · 1 comment
Open

Modify proto files to improve generated Java code #54

reify-tanner-stirrat opened this issue Feb 15, 2023 · 1 comment

Comments

@reify-tanner-stirrat
Copy link
Contributor

Background

(take this explanation with a grain of salt; I would not call myself an expert on protobuf and generated code)

You can see the problem here: https://github.com/authzed/authzed-java/blob/main/examples/v1/App.java#L10-L25

This is apparently related to an old behavior of protoc where you could only have a single top-level proto def in a file because you can only have a single top-level class def in a java file. The solution to this was to have protoc generate a wrapper class to bundle up the various definitions in a proto file, which produces the FooOuterClass definitions in the generated Java code.

This ends up being confusing to a user of the library, because whether you need to use FooService or FooServiceOuterClass is entirely down to how many things are defined in the source proto file, which isn't visible to a user of the library without diving into either the jar definition or the proto files.

Proposed fix

Using java_multiple_files in the proto definition as an option should fix this and make the produced code less janky.

Caveat

This will be a breaking change to authzed-java, because all code that referenced FooServiceOuterClass will need to now reference FooService (or something like that).

@reify-tanner-stirrat
Copy link
Contributor Author

reify-tanner-stirrat commented Feb 15, 2023

Turns out that java_multiple_files isn't related to this. That option still might be desirable, but this seems to be weird behavior of the java protoc compiler. I opened an issue on the protobuf repo, and hopefully they can shed some light: protocolbuffers/protobuf#11961

Also I realize this should probably be on the authzed/api repo if you're keen to move the issue there.

@josephschorr josephschorr transferred this issue from authzed/spicedb Feb 21, 2023
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

No branches or pull requests

1 participant