Separate REST Data Panache controller from resource

[gytis]

This change separates a generated REST Data with Panache JAX-RS controller class from a generated RestDataResource class. Here are the main benefits of this separation.

• RestDataResource interface that is used by a user is more user-friendly. JAX-RS Response result type is replaced with type-safe interfaces such as List<T> or 'T'. list method has Page and Sort as parameters instead of integers and strings.
• JAX-RS controller is generated separately and does not implement RestDataResource. This allows methods to have different definitions depending on the configuration. E.g. if pagination is disabled, page index and size parameters are not defined and as a result not shown in an OpenAPI spec.
• JAX-RS controller can specify all the query and path parameters with annotations and does not need to use UriInfo to access them.
• @ResourceProperties and @MethodProperties are now checked when generating methods. Generated code is now smaller because it can avoid unnecessary runtime checks.
• As a bonus, controller and resource separation should make it easier to implement new features, such as custom endpoints, in the future.

This PR also fixes #11391.

[geoand]

I assume you want a review from @FroMage ?

[gytis]

That would be great. I think many of the files could be simply skimmed in a review. Especially the tests because I didn't add any new ones but modified the old ones to comply with the API changes.

[gytis]

Is there something I could do to help out with this review? Maybe provide more information about the change or try to split the PR up into a few smaller ones?

[geoand]

@FroMage could you take a look at this one when you get a chance?

Thanks!

[FroMage]

Sorry for the late review.

The code is fine. I've some questions about the docs.
But ultimately, the idea seems to be to treat the resource interface as a service/DAO layer that the generated endpoint calls. That's what the implementation looks like.
But then, how am I supposed to add extra endpoints to the resource?

What if I write:

@ResourceProperties(hal = true, path = "my-people")
public interface PeopleResource extends PanacheEntityResource<Person, Long> {
  @GET
  @Path("extra")
  public String extraEndpoint() {
    return "Hello";
  }
}

Will you copy the extra method into the generated resource class?

[FroMage]

So why not return a List<Person>?

[FroMage]

Isn't it List<Person>?

Also it feels weird that these are Page and Sort when the generated code you showed up there uses int and String, is it normal?

[gytis]

Yes, it's List<Person> my bad. I think I'll rewrite the doc to clarify the differences better.

[FroMage]

We end up returning a boolean response?

[gytis]

Yes. Since I separated the RestDataResource implementation from a JAX-RS resource I decided to add a return value to the delete method so that a JAX-RS resource would know whether an entity was deleted or not and return either 204 or 404 response.

[gytis]

The code is fine. I've some questions about the docs.
But ultimately, the idea seems to be to treat the resource interface as a service/DAO layer that the generated endpoint calls. That's what the implementation looks like.
But then, how am I supposed to add extra endpoints to the resource?

What if I write:

@ResourceProperties(hal = true, path = "my-people")
public interface PeopleResource extends PanacheEntityResource<Person, Long> {
  @GET
  @Path("extra")
  public String extraEndpoint() {
    return "Hello";
  }
}

Will you copy the extra method into the generated resource class?

You understood the main idea correctly. This separates data access from JAX-RS. User would define an interface that specifies the data access which we could generate based on the actual interface used and customization annotations. The JAX-RS part is hidden from the user and is the same for all data access implementations. It defines JAX-RS annotations and then calls generated data access classes (the ones that implement PanacheEntityResource or PanacheRepositoryResource).

The custom method logic is up for discussion. But I was imagining extra methods that would follow a particular naming pattern. For example, findBy..., findOneBy..., deleteBy... or something like that. Those methods wouldn't have JAX-RS annotations they could only have @MethodProperties annotation. JAX-RS annotations would be added based on the name (e.g. @GET and return List for findBy..., @GET and return an entity for findOneBy... etc.)

[gytis]

Since this haven't been merged yet, I'll add some changes over the next few days. I ran into an issue working on Spring Data REST extension that could benefit from this PR if only it would be slightly changed.

[FroMage]

I'm not sure about this way to add endpoints. I feel like we're adding a new way to declare endpoints that is not necessarily going to be the same new way we end up adding to regular REST endpoints. We are already talking of making @GET optional and based on the name, so what you're suggesting makes sense, but I want to make sure this ends up being the exact same syntax, since we don't want our users to learn multiple ways to write endpoints.

I think it'd be better if I could add normal endpoint methods, either pure JAX-RS (at the moment) or whatever nice way we come up with in the future.

OTOH ATM we don't allow extra endpoints, so that discussion can wait, but I'd like to make sure that we have a plan for when we allow that. IF we allow it. Perhaps it makes sense to just not allow adding any extra endpoint, and the user will have to place them somewhere else.

Do we have use-cases for adding extra endpoints? Or replacing one?

[FroMage]

Or did I get this wrong and your idea is to allow (in the future) the user to override what happens inside the existing pre-defined endpoints? As opposed to modifying the endpoint "REST signature" or adding new endpoints?

[gytis]

Well, we don't have to introduce custom endpoints now and this change doesn't do it. The main improvement, in my mind, is that the data access implementation is separated from JAX-RS implementation (before data access was done in JAX-RS class implementation). This gives more freedom when defining custom things like adding query without having to change user facing interface, having custom data access implementation (e.g. Spring Data REST extension would implement it differently than Mongo and Hibernate does).
Also, as you said, in the future we could allow user to modify how does data access operation behave (e.g. add custom filter or sorting) but still generate JAX-RS class in the same way.

[FroMage]

So, to recap, the new design will take this user code:

public interface/class MyStuff implements RestDataResource<Stuff> {}

And turn it into:

@Path("mystuff")
public class MyStuffEndpoint {

 @Inject
 MyStuffDAO myStuffDAO;

 @GET
 public List<Stuff> list(@QueryParam String sort, @QueryParam int page, @QueryParam int pageSize) {
   return myStuffDAO.list(parseSort(sort), makePage(page, pageSize));
 }
 …
}

@Singleton
public class MyStuffDAO extends/implements MyStuff {
  public List<Stuff> list(Sort sort, Page page) {
    return Stuff.find(sort).page(page).list();
  }
  …
}

So we get two beans: endpoint and DAO.

The question was how do we allow users to add or modify methods, given that they would end up in the DAO since the endpoint one doesn't implement anything and is only a JAXRS shell to the DAO.

Thinking back on the reasons why people would want to add methods:

• To group them, but if that need rarely happens, then can put them somewhere else with the same path, that'll work.
• To compose them, but the body of the generated DAO methods are so thin/stupid that there's no point in composing them.

As for modifying methods:

• To change their JAX-RS signature, but we can't think of any valid reason to do that, since we already allow changing the path, and more would just be a completely different endpoint so what's the value?
• To change the JAX-RS behaviour, like, add a header? Not very compelling, again, for the same reason.
• To change the DAO behaviour, in which case it's fine we can support it in the future by just copying the body of the overridden method to the DAO we generate, and that seems like a more compelling use-case than the previous ones, because we standardise on the JAX-RS API, and allow customisation to the data layer.

As a result, I think the current proposal is fine. It has consequences, but they all seem acceptable.

[FroMage]

You do have a conflict, though.

[FroMage]

Sorry I closed it by accident.

[gytis]

I'll fix the conflict in a bit. We merged a couple of related commits a day or two ago.

[gytis]

@FroMage conflicts resolved.