Quarkus: Improve the usability of RESTEasy

[paulrobinson]

Description

There have been various comments about RESTEasy's small usability quirkiness (E.g named parameters being optional). These issues might have gone with some of the recent work, but maybe not.

This Epic is about identifying the complaints and fixing the important issues.

Analysis

_Links to analysis docs (E.g Design, Requirements, etc)_

Tasks

• [ ] _Checklist of sub tasks goes here_

[emmanuelbernard]

@geoand @asoldano @FroMage what would be your wish list for the next 6 months?

[emmanuelbernard]

This one needs a list and likely split it into phases

[FroMage]

From my email:

• Add a Controller class that REST endpoints extend to:
  -- opt-in to using their class name as the default @Path() annotation if not present
  -- opt-in to treating public methods as @GET unless another method annotation is present
  -- add utility methods such as notFoundIfNull(Object), ok(Object), validate(Object), etc…
• Add a JsonController subclass that adds @Consumes/@Produces for JSON and a JsonErrorMapper that turns all exceptions into JSON error objects
• Add a JsonEntity which is a transfer object that can be either a Json error, or a Json result (pretty standard usage for js-frontend stuff)
• Add a @WithExceptionMapper annotation that makes JAX-RS ExceptionMapper only be registered on certain endpoints (JsonController for example), because otherwise they're global and most apps have both json and non-json endpoints, so we want different exception mappers on different endpoints, even if they map to all exception types. This belongs in RESTEasy
• Add a @Required annotation to validate that endpoint parameters have been set. I assumed BV had something like that but I could not find it and found only @Valid which can't apply to String-sort of parameters. I must have missed something obvious.
• Add a Validatable interface which entities can implement to add custom multi-field validation, which is much simpler than the current BV ConstraintValidator contract which requires two entire classes for what should be one method. Perhaps BV has a simpler way, so feel free to correct me, I'm a BV noob and stackoverflow told me I needed this otherwise (https://stackoverflow.com/questions/2781771/how-can-i-validate-two-or-more-fields-in-combination)

Note that those are things I had already either observed/wanted or implemented in Redpipe so those needs are things that I expect to be common.

With this, I believe the code to be almost boilerplate-free, with the exception of Stuart's suggestion, which I'll address in another reply.

Check it out and comment please: https://github.com/FroMage/hotel-quarkus-demo/tree/refactor/src/main/java/org/lunatech
All the stuff in the quarkus package would be provided by our quarkus extensions, obviously, so would move out.

[FroMage]

From @stuartwdouglas 's suggestion:

• Default to application/json for JAX-RS, with the option to turn this off

I think that 99% of apps will want this as the default, it would get rid of all the @Produces and @Consume annotations in this example.

• Make @Path optional on a class level, and do a sensible default based on class name

Something like:

HotelResource would be /hotel
BookingService would be /booking-service

• Ability to directly inject JPA entities based on id?

Not so sure about this one, but basically this:

public Hotel getDetails(@PathParam String hotelId) {
    Hotel entity = Hotel.find("hotelId",hotelId).firstResult();
    if (entity == null) {
        throw new WebApplicationException("Hotel with hotelId of " + hotelId + " does not exist.", 404);
    }

Would become something like this (note that this will be more interesting with Hibernate RX, as you can do this via async injection):

public Hotel getDetails(@EntityPathParam(missingCode=404, field="hotelId") Hotel hotelId) {
}

Actually you could expand on this a lot if you allow Panache query syntax:

@EntityById("hotelId") Hotel hotel; -> Injects the hotel based on the hotelId path param, equivalent to Panache findById
@EntityQuery("where name=:hotelName and user.id=:userId") -> runs this query, and injects the result, where hotelName and userId are both path params. If this is a single param it is equivalent to Panache find(), otherwise list() or stream() based on param type.

This saves a little bit of boilerplate for blocking ops, but for reactive methods with multiple parameters it could save a lot of boilerplate. e.g. If you need the results of three different queries for your method, and they all can be expressed in terms of the parameters then instead of dealing with combining the results of 3 different reactive operations we handle this for you, and your reactive method looks almost exactly the same as the blocking version.

[FroMage]

And of course we still have the albatrosses:

• Get rid of second vert.x instance (make resteasy support async)
• Make RESTEasy able to run on IO thread
• Move RESTEasy config/setup to build-time, get rid of run-time reflection

[FroMage]

Wait… you said 6 years, right?

[geoand]

I :100: agree with the suggestion to default to JSON.

And I have nothing more to add since @FroMage 's list very very comprehensive

[geoand]

I was about to write that I would like the ability to have an exception mapper specific for a controller, but re-reading through the list that @FroMage added, I see the proposed @WithExceptionMapper.

[geoand]

I also want to add OOTB support for JsonView.
I should add that Spring already has this

[murphye]

Per this epic, please see my feature request documented here: https://github.com/quarkusio/quarkus/issues/6234

[FroMage]

More examples from @kenfinnigan:

= Setting HTTP Response Codes

Currently to set success response codes you need to return a Response object to be able to use something like Response.created(). Maybe it could be done with annotations instead?

@Path()
@POST
@ResponseCode(201)
public void create(User user) {
  // Save the user
}

= Handling Exceptions

Currently you can create providers to handle exceptions, but this separates the resource code from handling exceptions for it. Maybe something like:

@POST
@Path("/category")
@Consumes(MediaType.APPLICATION_JSON)
@Produces(MediaType.APPLICATION_JSON)
@Transactional
@ExceptionResponse(EntityExistsException.class, 409, “Entity already exists”)
@ExceptionResponse(Exception.class, 500, “Server Error”)
public Response create(Category category) throws Exception {
  em.persist(category);
  return Response
         .created(new URI(category.getId().toString()))
         .build();
}

[FroMage]

So after discussion, we'll start with these features:

• Add a Controller class (or interface?) that REST endpoints extend to:
  
  
  
  • opt-in to using their class name as the default @Path() annotation if not present (with special mapping for removing trailing Resource and turning camel-case into brochette-case).
  
  
  • opt-in to treating public methods as @GET unless another method annotation is present
  
  
  • add utility methods such as notFoundIfNull(Object), ok(Object), etc…
  
  
• Add a JsonController subclass that adds @Consumes/@Produces for JSON and a JsonErrorMapper that turns all exceptions into JSON error objects
• Add a JsonEntity which is a transfer object that can be either a Json error, or a Json result (pretty standard usage for js-frontend stuff)
• Add a @WithExceptionMapper annotation in RESTEasy that makes JAX-RS ExceptionMapper only be registered on certain endpoints (JsonController for example), because otherwise they're global and most apps have both json and non-json endpoints, so we want different exception mappers on different endpoints, even if they map to all exception types. This belongs in RESTEasy
• Support injection of entities loaded by ID: @EntityPathParam(missingCode=404, param="hotelId") Hotel hotel
  
  
  
  • This would load the hotelId path param (defaults to the first declared path param?) and call EntityManager.findById(Hotel.class, hotelId) and generate an HTTP missingCode (defaults to 404) if not found.
  
  
  • I guess this can be implemented with RESTEasy pluggable async injection: https://docs.jboss.org/resteasy/docs/4.4.2.Final/userguide/html/Async_Injection.html unless it's simpler to do it with ArC
  
  
  • Once we know how to make this work, we can look into extending for queries and streams/lists if required.
  
  
• Only do validation after

[FroMage]

Note that this requires being able to tell openapi and resteasy about the magic.

[geoand]

cc @gytis

[kenfinnigan]

FYI, https://github.com/eclipse/microprofile-sandbox/pull/78 was posted today which might be interesting to look at for HTTP status code mapping for exceptions

[loicrouchon]

* Add a @required annotation to validate that endpoint parameters have been set. I assumed BV had something like that but I could not find it and found only @Valid which can't apply to String-sort of parameters. I must have missed something obvious.

@FroMage Isn't the @required you're mentioning the @NotNull from bean validation spec?
https://jakarta.ee/specifications/bean-validation/2.0/bean-validation_2.0.html#builtinconstraints-notnull
Of course, it just performs the null check, so you need other constraints then to ensure that it's not empty for a string, contains has fields set if it's an object, ...

[nimo23]

I am not against annotations in general if the intentions are clearly separated. But having someting like this:

// INSANE:(
@POST
@Path("/category")
@Consumes(MediaType.APPLICATION_JSON)
@Produces(MediaType.APPLICATION_JSON)
@Transactional
@ExceptionResponse(EntityExistsException.class, 409, “Entity already exists”)
@ExceptionResponse(Exception.class, 500, “Server Error”)
public Response create(Category category) throws Exception {
  em.persist(category);
  return Response
         .created(new URI(category.getId().toString()))
         .build();
}

how should one know which annotations can or should be (not) mixed up without searching for that stuff which is very time consuming? Why not using lambda style method chaining instead? It is by far easier to construct such expressions as above, because with method-chaining the user can see which method is available or composable at which point just in time with code completion. For example, something like this:

// SANE:)
// all possible compositions are available at your fingertips..
 return Endpoint
      .withPath("/hello")
      .method(GET)
      .produces(MediaType.TEXT_PLAIN)
      .invoke(hello::helloWorld);

is by far better then such an annotation hell - annotations which are disjoint and mostly complementary could be indeed annotations (e.g. @ApplicationScoped, @SessionScoped and other little things, BUT not such things as above.). I also think that lambda style patterns would be a far better fit for quarkus in general (e.g. no need to add much logic for annotation processors, type safety, easier error handlings, faster compiling,..). Look at the JAVA Streams API..it is lambda style and we know all: it has a great usability. So why not adapting such a pattern in quarkus world instead of inventing dozens of annotations living isolated anywhere and waiting for the user (and the compiler) to find out how they can be mixed up or not.

An interesting article about what I mean: in https://blog.softwaremill.com/the-case-against-annotations-4b2fb170ed67.

[emmanuelbernard]

Declarative vs imperative is important for Quarkus because it allows us to detect and do a lot at build time. From Stef example, I guess the Exception response could be programmatic instead of being annotation driven.

BTW @nimo23 you are unfair in your comparison, your programmatic code does not do the same as the annotation drive code, that's why it feels simpler.

[geoand]

To get back to the last point, I can't agree more with @emmanuelbernard. The declarative style gives Quarkus a lot more information for doing build time processing - thus optimizing runtime behavior.
In a DSL like the one mentioned above, it would be very much harder and slower (and in some cases not possible) to do those optimizations.