RESTEasy Reactive Links #14249

gytis · 2021-01-12T12:28:17Z

@FroMage @geoand this is a a draft PR for RESTEasy Reactive Links extension to discuss the API and see if I'm going in a right direction here.

There are three main user facing pieces:

@RestLink annotation which marks which methods should be considered for link injection (similar to the RESTEasy @LinkResource)
@InjectRestLinks annotation which indicates a method which response should get link headers injected (it uses RestLinksProvider under the covers). Similar to the RESTEasy RESTServiceDiscovery but instead of injecting the links into an object, it injects them as headers.
RestLinksProvider interface that has two methods for getting relevant links based on type or object instance. We could use it to generate HAL objects in REST Data Panache or if we'd decide to implement RESTServiceDiscovery from RESTEasy Links.

All JAX-RS methods are scanned in build time when LinksContext is created. In runtime we don't need to do any scans or reflection operations like in RESTEasy Links. It's enough to perform a map lookup to get the link templates and build the actual links.

See TestResource for an example.

I still need to improve this with more testing, EJB security check and a few other bits. But since I have a roughly functioning prototype I'd like to check in now and see what should be adjusted.

geoand · 2021-01-12T12:30:10Z

Great, thanks a lot @gytis!

I'll have a look tomorrow

.../io/quarkus/resteasy/reactive/server/deployment/ResteasyReactiveDeploymentInfoBuildItem.java

...n/java/io/quarkus/resteasy/reactive/links/deployment/PathParameterValueGettersGenerator.java

...s/deployment/src/main/java/io/quarkus/resteasy/reactive/links/deployment/LinksProcessor.java

...-links/runtime/src/main/java/io/quarkus/resteasy/reactive/links/RestLinksResponseFilter.java

geoand · 2021-01-13T07:55:19Z

Thanks a lot for this work @gytis!

I can't really comment on the API as I don't know much about the topic in hand (@FroMage is much better suited to comment on that), but I have added a few comments about the code itself

FroMage

I added some comments and questions. This looks like limited to Link headers ATM, which is fine.
I'm not too sure about the API though, I think it would help if you added examples which we can use as use-cases to verify what it does.
It looks like you're resolving path parameters directly as fields from the resources, which is probably fine, but for resteasy classic (https://docs.jboss.org/resteasy/docs/2.0.0.GA/userguide/html/LinkHeader.html) I only handled IDs. I guess this is more powerful, but requires that the path parameter name matches the entity property.

FroMage · 2021-01-19T16:46:37Z

...n/java/io/quarkus/resteasy/reactive/links/deployment/PathParameterValueGettersGenerator.java

+    }
+
+    @Override
+    public void visitEnd() {


This looks like you're generating getters even if one already exists.

It did initially.
I wasn't sure how to check if a method exists from a visitor and I couldn't use an index, because it doesn't get updated. So I've added an extra mapping here https://github.com/quarkusio/quarkus/pull/14249/files/a6de4a1aa44bc8a1102de72835ba98a1355d5c8d#diff-938ce5408bf74bba0f73f4922b625306f3a9deced50ca5afcde36c03d431f3abR49 to make sure that only one getter is generated.

I usually collect signatures in visitMethod

FroMage · 2021-01-19T16:57:46Z

...ment/src/test/java/io/quarkus/resteasy/reactive/links/deployment/RestLinksInjectionTest.java

+                .getHeaders()
+                .getValues("Link");
+        assertThat(links).containsOnly(
+                Link.fromUriBuilder(UriBuilder.fromUri(testWithLinksUrl).path("/1")).rel("getById").build().toString(),


IMO according to https://www.iana.org/assignments/link-relations/link-relations.xhtml this should be self for the rel.

I thought that because we cannot infer all the possible rel values I would simply default it to the method name and allow it to be overwritten with an annotation (@RestLink(rel="self")).

FroMage · 2021-01-19T16:58:05Z

...ment/src/test/java/io/quarkus/resteasy/reactive/links/deployment/RestLinksInjectionTest.java

+                .getValues("Link");
+        assertThat(links).containsOnly(
+                Link.fromUriBuilder(UriBuilder.fromUri(testWithLinksUrl).path("/1")).rel("getById").build().toString(),
+                Link.fromUri(testWithLinksUrl).rel("list").build().toString(),


Again according to IANA this should probably be collection.

FroMage · 2021-01-19T16:58:42Z

...ment/src/test/java/io/quarkus/resteasy/reactive/links/deployment/RestLinksInjectionTest.java

+        assertThat(links).containsOnly(
+                Link.fromUriBuilder(UriBuilder.fromUri(testWithLinksUrl).path("/1")).rel("getById").build().toString(),
+                Link.fromUri(testWithLinksUrl).rel("list").build().toString(),
+                Link.fromUri(testWithoutLinksUrl).rel("listWithoutLinks").build().toString());


But then, not sure what to do about this. Need to check if we can have multiple similar relations.

FroMage · 2021-01-19T16:59:34Z

...nks/deployment/src/test/java/io/quarkus/resteasy/reactive/links/deployment/TestResource.java

+    @Path("/with-links/{id}")
+    @Produces(MediaType.APPLICATION_JSON)
+    @RestLink
+    @InjectRestLinks(RestLinkType.INSTANCE)


I'm not sure what instance refers to here: we do have an instance returned, so…

At the moment by default TYPE links are injected i.e. get all and create.

FroMage · 2021-01-19T17:00:20Z

...nks/deployment/src/test/java/io/quarkus/resteasy/reactive/links/deployment/TestResource.java

+    @GET
+    @Path("/with-links")
+    @Produces(MediaType.APPLICATION_JSON)
+    @RestLink(entityType = TestRecord.class, rel = "list")


This should be optional, in which case we can infer that the entity type is TestRecord (from the return type List<TestRecord> and same for the rel which should be collection.

I don't have this implemented yet, but inferring an entity type from a generic is in my todo. So for now this is a workaround.

As for the rel, this annotation allows to define any rel you want.

FroMage · 2021-01-19T17:01:19Z

...reactive-links/runtime/src/main/java/io/quarkus/resteasy/reactive/links/InjectRestLinks.java

+@Target({ ElementType.TYPE, ElementType.METHOD })
+public @interface InjectRestLinks {
+
+    RestLinkType value() default RestLinkType.TYPE;


I wonder what purpose this serves. Can you show me use-cases? The example test could deduce that from the method return type.

I thought that this would allow user to specify which links does he want to be injected.
E.g. if a DELETE operation returns a deleted entity, you don't necessarily want to return the links specific for that entity (self, update, delete) because it doesn't exist any more. But you might still want to return the links for getting all and creating new entities.

OK I see, but that feels special-casy.

FroMage · 2021-01-19T17:11:54Z

...ime/src/main/java/io/quarkus/resteasy/reactive/links/runtime/PathParameterValueResolver.java

+        for (Method method : instance.getClass().getMethods()) {
+            if (method.getName().equals(pathParameterInfo.getValueGetter())) {
+                try {
+                    return method.invoke(instance);


We can get rid of this reflection by generating bytecode that invokes it for a given instance like this:

public class $$Order$Id implements Getter<Order,Long> { public static Long get(Order order){ return order.$$generated_getter_id(); } }

And you instantiate those getters at static init and can just invoke them without reflection when you need to.

Nice, I'll add that.

gytis · 2021-01-20T11:18:33Z

Thanks for the comments @FroMage.

I added some comments and questions. This looks like limited to Link headers ATM, which is fine.

For now I've added two things. A RestLinksProvider bean, which returns a collection of links based on type or instance. And a Link headers injection, which simply takes the links from the provider and adds link headers. We could inject other types by reusing the same provider. I just didn't want to add too many features without discussing the API.

I'm not too sure about the API though, I think it would help if you added examples which we can use as use-cases to verify what it does.

This is just a draft to discuss the API. I don't expect it to be final and I'm happy to change it. That's the whole point.

It looks like you're resolving path parameters directly as fields from the resources, which is probably fine, but for resteasy classic (https://docs.jboss.org/resteasy/docs/2.0.0.GA/userguide/html/LinkHeader.html) I only handled IDs. I guess this is more powerful, but requires that the path parameter name matches the entity property.

Yes I've added a possibility to resolve any entity field. I haven't yet added an override option to the @RestLink annotation. With that the path parameter won't need to match the field name.
Do you think this is too much and I should stick to the ID only?

FroMage · 2021-01-20T13:57:24Z

Yes I've added a possibility to resolve any entity field. I haven't yet added an override option to the @RestLink annotation. With that the path parameter won't need to match the field name.
Do you think this is too much and I should stick to the ID only?

No, I think it's an interesting approach, and more powerful. Let's try it.

gytis · 2021-02-11T10:50:23Z

@FroMage @geoand, sorry for a slight delay in the update but I think I've covered all the comments. However, I still have a couple of points that need to be cleared up.

For now I left the RestLinkType enum to specify whether type or instance links should be injected. @FroMage had some reservations regarding this point, so I'm open for a debate about that.
How sophisticated do we want the Entity type resolution from a generic type to be? For now I only check one level of a single argument parameterised type (e.g. Link, Collection, Set). Does any of you have experience with this? Should we make this deduction more sophisticated? Or maybe we should step back an only check a predefined set of interfaces like RESTEasy Links does?

geoand · 2021-02-12T11:32:30Z

My comments have been addressed, the rest I can really comment on since I don't have any experience with links

FroMage

LGTM

FroMage · 2021-02-12T16:37:52Z

How sophisticated do we want the Entity type resolution from a generic type to be? For now I only check one level of a single argument parameterised type (e.g. Link, Collection, Set). Does any of you have experience with this? Should we make this deduction more sophisticated? Or maybe we should step back an only check a predefined set of interfaces like RESTEasy Links does?

Just handle collections and special types on a single level, yeah.

gytis · 2021-02-16T12:21:23Z

I've just squashed the commits and added the extension file.

geoand · 2021-03-26T09:10:28Z

Ah damn... We forgot to merge and now there are conflicts :(

Can you rebase Gytis so we can get this in?

geoand · 2022-04-21T11:06:36Z

@Sgitario would you be interested in writing a guide for RESTEasy Reactive links?
The idea being that this module although it's currently only a building block for REST Data Panache, it could however provide a general solution for JAX-RS Paging.

maxandersen · 2022-04-21T11:10:45Z

related to #9052 ?

geoand · 2022-04-21T11:11:51Z

Related, but we might want something ever higher level

Sgitario · 2022-04-22T04:40:01Z

@Sgitario would you be interested in writing a guide for RESTEasy Reactive links? The idea being that this module although it's currently only a building block for REST Data Panache, it could however provide a general solution for JAX-RS Paging.

I really like the idea. Count on me to do this.

geoand · 2022-04-22T04:52:11Z

Thanks a lot!

ghost added area/dependencies Pull requests that update a dependency file area/rest labels Jan 12, 2021

geoand reviewed Jan 13, 2021

View reviewed changes

.../io/quarkus/resteasy/reactive/server/deployment/ResteasyReactiveDeploymentInfoBuildItem.java Show resolved Hide resolved

geoand reviewed Jan 13, 2021

View reviewed changes

...n/java/io/quarkus/resteasy/reactive/links/deployment/PathParameterValueGettersGenerator.java Outdated Show resolved Hide resolved

geoand reviewed Jan 13, 2021

View reviewed changes

...s/deployment/src/main/java/io/quarkus/resteasy/reactive/links/deployment/LinksProcessor.java Outdated Show resolved Hide resolved

geoand reviewed Jan 13, 2021

View reviewed changes

...-links/runtime/src/main/java/io/quarkus/resteasy/reactive/links/RestLinksResponseFilter.java Outdated Show resolved Hide resolved

FroMage requested changes Jan 19, 2021

View reviewed changes

FroMage reviewed Jan 19, 2021

View reviewed changes

gytis force-pushed the resteasy-reactive-links branch 2 times, most recently from 4f58ec1 to 00d7bfb Compare February 10, 2021 17:34

ghost added the area/core label Feb 10, 2021

FroMage approved these changes Feb 12, 2021

View reviewed changes

gytis force-pushed the resteasy-reactive-links branch from 00d7bfb to 0ce15a3 Compare February 16, 2021 12:18

gytis marked this pull request as ready for review February 16, 2021 12:20

gytis force-pushed the resteasy-reactive-links branch 2 times, most recently from 5af2ef5 to bcc5339 Compare February 16, 2021 17:35

quarkus-bot bot added area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/documentation labels Feb 16, 2021

Base automatically changed from master to main March 12, 2021 15:55

gytis force-pushed the resteasy-reactive-links branch 2 times, most recently from 665dd43 to e212cbc Compare March 15, 2021 12:05

geoand added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Mar 26, 2021

RESTEasy Reactive Links

ff688c4

gytis force-pushed the resteasy-reactive-links branch from e212cbc to ff688c4 Compare March 26, 2021 11:26

quarkus-bot bot added the area/panache label Mar 26, 2021

geoand merged commit 7f41b56 into quarkusio:main Mar 30, 2021

quarkus-bot bot added this to the 1.14 - main milestone Mar 30, 2021

geoand removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Apr 21, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RESTEasy Reactive Links #14249

RESTEasy Reactive Links #14249

gytis commented Jan 12, 2021 •

edited

Loading

geoand commented Jan 12, 2021

geoand commented Jan 13, 2021 •

edited

Loading

FroMage left a comment

FroMage Jan 19, 2021

gytis Jan 20, 2021

FroMage Jan 20, 2021

FroMage Jan 19, 2021

gytis Jan 20, 2021

FroMage Jan 19, 2021

FroMage Jan 19, 2021

FroMage Jan 19, 2021

gytis Jan 20, 2021

FroMage Jan 19, 2021

gytis Jan 20, 2021

gytis Jan 20, 2021

FroMage Jan 20, 2021

FroMage Jan 19, 2021

gytis Jan 20, 2021

FroMage Jan 20, 2021

FroMage Jan 19, 2021

gytis Jan 20, 2021

gytis commented Jan 20, 2021 •

edited

Loading

FroMage commented Jan 20, 2021

gytis commented Feb 11, 2021

geoand commented Feb 12, 2021

FroMage left a comment

FroMage commented Feb 12, 2021

gytis commented Feb 16, 2021

geoand commented Mar 26, 2021

geoand commented Apr 21, 2022

maxandersen commented Apr 21, 2022

geoand commented Apr 21, 2022

Sgitario commented Apr 22, 2022

geoand commented Apr 22, 2022

RESTEasy Reactive Links #14249

RESTEasy Reactive Links #14249

Conversation

gytis commented Jan 12, 2021 • edited Loading

geoand commented Jan 12, 2021

geoand commented Jan 13, 2021 • edited Loading

FroMage left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gytis commented Jan 20, 2021 • edited Loading

FroMage commented Jan 20, 2021

gytis commented Feb 11, 2021

geoand commented Feb 12, 2021

FroMage left a comment

Choose a reason for hiding this comment

FroMage commented Feb 12, 2021

gytis commented Feb 16, 2021

geoand commented Mar 26, 2021

geoand commented Apr 21, 2022

maxandersen commented Apr 21, 2022

geoand commented Apr 21, 2022

Sgitario commented Apr 22, 2022

geoand commented Apr 22, 2022

gytis commented Jan 12, 2021 •

edited

Loading

geoand commented Jan 13, 2021 •

edited

Loading

gytis commented Jan 20, 2021 •

edited

Loading