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

Add the ability to associate a message with an interface in Java #4481

Closed
kathyvs opened this issue Apr 5, 2018 · 6 comments
Closed

Add the ability to associate a message with an interface in Java #4481

kathyvs opened this issue Apr 5, 2018 · 6 comments
Labels

Comments

@kathyvs
Copy link

kathyvs commented Apr 5, 2018

The most pain I have had using the protobuf Java classes out of the box involved:

1 - not being able to add useful methods to the class (instead using utility classes)
2 - not being able to group related classes together for the purposes of share library usage and such. This is related to there not be a subclass/superclass relationship, although there are other reasons to want to group a bunch of field and say "any with these is an X" without bundling them together.

Both these problems can be solved as of Java 8 with the ability to optionally add interfaces to the generated code. I am not certain how it would be represent in the proto code(as far as I can tell you don't have message-level options) but it would be very helpful. This solved #1 as well in Java 8 now that there are default method definitions for interfaces

Adding interfaces would also be useful in C# and possibly C++. Even in Ruby and Python, we could get #1 by allowing adding mixins to the class.

It would be the responsibility of the code writer to ensure that the generated classes actually implement the interface.

@xfxyjwf
Copy link
Contributor

xfxyjwf commented Apr 5, 2018

I think we intentionally do not want people to customize protobuf generated classes because those customization breaks encapsulation and can cause unforeseen breakages when we change generated code internals.

@chasebradford
Copy link

This is something I've wanted for a very long time, as well. I don't see how adding an interface to the generated classes would break encapsulation in anway, so it would be nice to hear more on that. I'd expect the interface to mirror accessor methods, eg. interface HasId { public int getId(); } which could be applied to any message with an int id field. That would give a clean way to pass any message with an id to some method that only needs to use the id from a message. There'd probably need to be two options, one to apply an interface with getters to MessageOrBuilder and another that could apply an interface with setters to the message's Builder classes.

@ghost
Copy link

ghost commented Jun 11, 2019

Java supports plugin insertion point: https://developers.google.com/protocol-buffers/docs/reference/java-generated#plugins. You can insert additional interface at message_implements:TYPENAME and builder_implements:TYPENAME.

@ghost ghost closed this as completed Jun 11, 2019
@abatkin
Copy link

abatkin commented Jun 12, 2019

@haon4 I don't believe that the ability to extend the code generator is sufficient here: It's an exceptionally high barrier to entry, the plugin mechanism is poorly documented (so says the page you linked to anyway), it's fragile and non-portable (except with the non-standard/custom-build binary plugin) even within Java, and even less portable to similar constructs in other languages (though I won't try to solve that here).

The most basic solution might be to have something like option java_interface 'com.example.whatever.MyInterface and the Java code generator would blindly make the generated class implement MyInterface (and leave it up to the user/Java compiler to deal with the consequences). This would be clean, only affect generated Java code, and would be useful to end-users. A common, well-known, reusable way of implementing this common need would be beneficial to users.

If a PR along these lines was created, would you/Google be willing to accept it (assuming it met general PB coding standards, etc...)? Or is this something that you/Google simply isn't interested in having as a part of protobuf?

@chasebradford
Copy link

I created a change for this and posted to the protobuf mailing list, but never heard back.
master...chasebradford:master

I have all of the same concerns with using a custom plugin (brittleness, poor documentation, etc). Unfortunately, when I actually tried with my change, I quickly realized that older compilers that didn't recognize the option would outright fail instead of ignoring it.

@shpandrak
Copy link

I created a change for this and posted to the protobuf mailing list, but never heard back.
master...chasebradford:master

I have all of the same concerns with using a custom plugin (brittleness, poor documentation, etc). Unfortunately, when I actually tried with my change, I quickly realized that older compilers that didn't recognize the option would outright fail instead of ignoring it.

This looks like exactly the addition I also need in my projects. any updates on this?

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants