jakartaee / rest

Jakarta RESTful Web Services
Other
351 stars 114 forks source link

Rename the new @Entity annotation #1183

Closed WhiteCat22 closed 4 months ago

WhiteCat22 commented 9 months ago

Hi all,

I am a developer for Open Liberty and we are starting work on our new feature for RESTfulWS-4.0.

During our EE11 design presentation, we received strong feedback about the name of the new @Entity annotation.

The main concern is that both EJB and JPA already have an @Entity annotation and we want to avoid another annotation with the same name. Multiple annotations with the same name causes confusion and can also cause problems when IDEs auto import the incorrect class.

Would it make sense to rename @Entity to @RequestEntity? @Payload? @PayloadEntity? Something similar?

arjantijms commented 9 months ago

p.s. about naming, would it make sense to call your feature REST-4.0? We've been trying for some time to get to the name "Jakarta REST", without the perhaps those somewhat frivolous extra bits in the name.

jim-krueger commented 9 months ago

p.s. about naming, would it make sense to call your feature REST-4.0? We've been trying for some time to get to the name "Jakarta REST", without the perhaps those somewhat frivolous extra bits in the name.

What you are suggesting was one of the "candidate names" when we went from jaxrs-2.1 to restfulWS-3.0. Personally I like the simpler name and will see if it will get more traction this time around.

mkarg commented 9 months ago

I think @Entity is the perfect name, as in the HTTP standard the payload is called entity, so it is at-most intuitive in the JAX-RS environment. Prevention of name clashes is a generic topic not related to this particular name, so we should ignore this argument. +1 for @Entity, -1 for other names

spericas commented 9 months ago

@WhiteCat22 I understand the concern of reusing the name. @RequestEntity seems to be the best of the alternatives (but not as nice as @Entity). I'd like to hear more opinions, though.

jansupol commented 9 months ago

ChatGPT offers the following alternatives (I chose the best-suited):


RFC 9110 defines Content as:

HTTP messages often transfer a complete or partial representation as the message content: a stream of octets sent after the header section, as delineated by the message framing.

So while content is the binary data, the entity is more the Java entity, which fits better to our purpose.


Payload is used in RFC 7231 and RFC 9110 says about it:

The terms "payload" and "payload body" have been replaced with "content", to better align with its usage elsewhere (e.g., in field names) and to avoid confusion with frame payloads in HTTP/2 and HTTP/3.

Payload and content seem to be the same name for binary data wired over the network.


Data seems too broad and rather "logical" piece of information.


For Entity:

JAX-RS defines the Entity class and says that the Entity class encapsulates message entity.

The obsolete HTTP 1.1 RFC 2710 defines entity, but RFC 9110 uses HTTP message instead.

However, while @Entity collides with JPA, @Message collides with JMS.

EDIT: RFC 9112 talks about HTTP Message, too:

An HTTP/1.1 message consists of a start-line followed by a CRLF and a sequence of octets in a format similar to the Internet Message Format [RFC5322]: zero or more header field lines (collectively referred to as the "headers" or the "header section"), an empty line indicating the end of the header section, and an optional message body.

Message body is used very often in that RFC, and @Body is very short, fast to write for the customers, and better than @Content, @RequestEntity, @Payload, or other alternatives.

FroMage commented 9 months ago

Definitely +1 to @Body.

jamezp commented 9 months ago

@Body makes the most sense to me as well if we want to replace @Entity. I do understand the reasoning to rename the annotation as well. While you could use the FQCN, it's not great to read in code and could trip up users relying on auto-complete. The wrong annotation could easily be imported.

spericas commented 9 months ago

I don't love it, but can live with @Body.

@mkarg Can you possibly reconsider your vote?

jamezp commented 9 months ago

FWIW my thought on name clashes comes from things like @Singleton, @RequestScoped, @ApplicationScoped, etc. I realize thatjakarta.faces.bean.*` is now gone, but I feel that helps the argument to change it.

mkarg commented 9 months ago

As both EJB and JPA already do have that exact name clash even without JAX-RS, and as there are even more names clashing in Jakarta EE (like Produces) I do not see that the proposal would be a real benefit for Jakarta EE in general, if we just rename @Entity to @Body solely in JAX-RS. Hence I am still not convinced about this.

Having said that, if (and only if) there is a majority of committers for renaming the annotation, I do not stand on the way of @Body.

jim-krueger commented 9 months ago

While I see the point that other clashes currently exist and, in a vacuum, @Entity is the most descriptive, I'd prefer not to add another clash given that @Entity represents something that is persisted in JPA and EJB but not JAX-RS. I'm not overly fond of @Body but will +1 that if a better name does not appear. If we can't use @Entity or @Message, how about a bundle @MessageEntity? I haven't seen that as a suggestion yet.

mkarg commented 9 months ago

-1 for Message, is it is totally uncommon.

jim-krueger commented 9 months ago

@mkarg @Message was not the option I threw out there. Does your -1 apply to @MessageEntity as well?

mkarg commented 9 months ago

Yes, my -1 was for everything having Message in it.

jim-krueger commented 8 months ago

It seems like @Body is the most likely alternative to @Entity. While I wish there was a better alternative I'll give a +1 vote to @Body.

That makes 2 official +1 votes from committers (assuming I count @spericas comment above as a +1 vote), with @mkarg voicing a willingness to abide by the majority (with a preference for it to remain @Entity)

Can we please get some more votes (or other suggestions) from committers? If we are going to make this change it would be good to do that sooner rather than later. Thanks

jansupol commented 8 months ago

It is +1 for the @Body from me, as it was my initial proposal unless we keep going with @Entity.

spericas commented 8 months ago

I will create a PR using @Body after merging a few others that are pending

WhiteCat22 commented 8 months ago

Thanks for the discussion everyone!

WhiteCat22 commented 8 months ago

@spericas I'm looking to get more involved so I took a stab at a pull request.

jansupol commented 4 months ago

Can we close this?

jim-krueger commented 4 months ago

I believe so. Closing