jakartaee / rest

Jakarta RESTful Web Services
Other
367 stars 121 forks source link

Consider having @BeanParam support Record Classes #913

Open fwgreen opened 4 years ago

fwgreen commented 4 years ago

Records look like a good fit for @BeanParam support:

record PersonForm(@FormParam("name") String name, @FormParam("age") int age) {}
//...

@POST
@Produces(MediaType.TEXT_HTML)
public Response post(@BeanParam PersonForm form) {
    //...
}
mkarg commented 4 years ago

Unfortunately records are just in preview even in JDK 15.

fwgreen commented 4 years ago

Because of the faster release cadence, I'm hoping for a conversation on what's required to start now, as the plan (as of July) is to finalize Records in JDK 16 (due in March 2021).

arjantijms commented 4 years ago

Although we're set to solving the issues, and taking advantage of Java SE is very important, the Jakarta Rest spec is bound by the version of Java that the TCK describes (both standalone as platform).

That should therefor allow certification on JDK 16. I'm greatly in favour to make this possible/allow this, but as of yet the TCKs are very, very tightly tied to JDK 8.

jansupol commented 3 years ago

I do not think that TCK's flexibility should decide what the Spec shall support. It is up to the Spec to define what JDK version it supports. There is a bunch of requirements in many specs that are untestable by the TCKs at the moment.

pearceeverydaylabs commented 3 years ago

any possibility of reconsidering this now that java 16 records have arrived (and given that there seems to be differing opinions about whether current TCK should block this)?

andymc12 commented 3 years ago

IIUC, a record is really just an immutable class. If we supported immutable classes (which would work in all Java versions), we could implicitly support records. So we could modify the spec to indicate that implementations must support constructor injection of *Params for BeanParam classes. Then we would be able to support something like this (for all Java versions):

@POST
@Produces(MediaType.TEXT_HTML)
public Response post(@BeanParam PersonForm form) {
    //...
}

//...

class PersonForm {
    private final String name;
    private final int age;
    PersonForm(@FormParam("name") String name, @FormParam("age") int age) {
        this.name = name;
        this.age = age;
    }
    //....
}

In Java 16, a record like the following would effectively create that class anyway:

record PersonForm(@FormParam("name") String name, @FormParam("age") int age) {}

so we would have support for Java 16.

I haven't tested it yet, but I wouldn't be surprised if some JAX-RS implementations already support constructor injection of *Params...

mkarg commented 3 years ago

Andy's approach sounds feasable to me.

zanmagerl commented 1 year ago

Is there any new progress on that? Records are now available in Java 17 (which is LTS) and it makes sense to use records for @BeanParam. For example, given query parameters in REST call are most commonly immutable and records are intended for such use cases.

spericas commented 1 year ago

Jakarta REST 4.0 will likely be supporting JDK 17 or later (in alignment with Jakarta 11). We can add a note about records in the spec.

jansupol commented 1 year ago

I would be concerned about this. Jakarta REST 4.0 should use CDI. CDI would instantiate the records, but the injection into records does not work with CDI, since the records are immutable.

ksdev-pl commented 3 months ago

Hello. Are there any plans related to it, as it looks like Jakarta REST 4.0 doesn't support records?

mkarg commented 3 months ago

@ksdev-pl Thank you for reactivating this discussion!

@jansupol IMHO CDI supports constructor injection, so what actual issue do you see?