@Sgitario this is a draft PR. Which I would like you to have a look at. There is still one issue to be solved. The Jsonb serializers don't seem to be triggered any more since the HalWrapper types got added a type argument. The Jackson serializer works correctly. I think the typed HalWrappers no longer are matched against the serializer, but why this happens I do not know. Looking at eclipse yasson it should work. Do you have any idea?

Note: Builds of this PR will fail until the issue is resolved

🙈 The PR is closed and the preview is expired.

We need the parameterized type to be able to determine the wrapped type which needs to be scanned for id field, @RestLinkId annotation or @Id annotation or the build will fail with an exception stating it can't find any of those. Further more we need to scan the class to be able to implement getters for all path variables used. The id field for instance is used for the 'self' link. If we don't do that those never get rendered.

Concerning the change in behaviour creating the wrappers, the documentation is already wrong. If you use the example it will give a compile error. The constructor used in the documented example does not exist. HalCollectionWrapper requires a Collection<HalEntityWrapper> and not a pure Collection as used in the example.

If you use the correct constructor it also works. In my opinion using the HalService is a more robust solution if any runtime magic needs to be added while constructing the wrapper objects.

We need the parameterized type to be able to determine the wrapped type which needs to be scanned for id field, @RestLinkId annotation or @Id annotation or the build will fail with an exception stating it can't find any of those. Further more we need to scan the class to be able to implement getters for all path variables used. The id field for instance is used for the 'self' link. If we don't do that those never get rendered.

I see, thanks.

Concerning the change in behaviour creating the wrappers, the documentation is already wrong. If you use the example it will give a compile error. The constructor used in the documented example does not exist. HalCollectionWrapper requires a Collection<HalEntityWrapper> and not a pure Collection as used in the example.

Still, this needs to be noted in the migration guide.

If you use the correct constructor it also works. In my opinion using the HalService is a more robust solution if any runtime magic needs to be added while constructing the wrapper objects.

+1

@geoand any hints about the jsonb failures?

@geoand any hints about the jsonb failures?

I can't think of anything off hand. I would start by looking backwards to where the working Jackson ones are being called and comparing with that with the Jsonb ones to see what is different

@geoand @Sgitario I did a backtrace and the normaly I would expect io.quarkus.jsonb.JsonbProducer#jsonbConfig() to be called which adds the serializer. But it seems this does not happen. Could this be due to the fact I added a test dependency on quarkus-hal-deployment to be able to use the wrapper classes in a testcase?

Could this be due to the fact I added a test dependency on quarkus-hal-deployment to be able to use the wrapper classes in a testcase?

Have you tried without it?

I can check if the producer is called without the dependency but I will have to remove the extra testcases as they use the wrapper classes. Will post the results of my findings here.

@geoand @Sgitario I did some testing and have the idea the problem is a bug in eclipse yasson. I tried the following definitions of the serializer:

1 HalCollectionWrapperJsonbSerializer implements JsonbSerializer<HalCollectionWrapper<?>> which fails.
2 HalCollectionWrapperJsonbSerializer implements JsonbSerializer<HalCollectionWrapper> which works.

It looks eclipse yasson has an issue with serializers and probably deserializers when the root type is generic. Maybe it is related or the same as eclipse-ee4j/yasson#243 which is fixed, but that seemed to target annotated serializers / deserializers. These are added though configuration.

I've updated the PR to use the raw typed serializers as that works. I'll report an issue with eclipse yasson and update the comment in the serializers to reference that issue.

What do you think?

Sounds good to me!

Thanks for looking into it.

@geoand I updated the PR to point to eclipse-ee4j/yasson#639 for as why the use of raw types in the Jsonb serializers.

@Sgitario How can I add something to the migration guide?

@Sgitario How can I add something to the migration guide?

Here: https://github.com/quarkusio/quarkus/wiki/Migration-Guide-3.9
If you don't have permission, drop the content here and I can add it for you.

Many thanks for your changes here @bpasson !

@Sgitario How can I add something to the migration guide?

Here: https://github.com/quarkusio/quarkus/wiki/Migration-Guide-3.9 If you don't have permission, drop the content here and I can add it for you.

@Sgitario I don't have permissions to edit the wiki. The following should be added to the migration documentation for 3.9. Feel free to update where needed.

RESTEasy Reactive HAL wrapper changes

Wrapper Usage

The HAL wrappers class signatures have been updated to include the type of elements in the collection as listed below.

• HalCollectionWrapper<T>
• HalEntityWrapper<T>

You should add the type argument everywhere you use the HAL wrapper classes.

Wrapper Creation

The preferred way of creating the wrapper classes changed from constructor to helper methods exposed on the io.quarkus.hal.HalService bean as shown below:

@Path("/records")
public class RecordsResource {

    @Inject
    HalService halService;

    @GET
    @Produces({ MediaType.APPLICATION_JSON, RestMediaType.APPLICATION_HAL_JSON })
    @RestLink(rel = "list")
    public HalCollectionWrapper<Record> getAll() {
        List<Record> list = // ...
        HalCollectionWrapper<Record> halCollection = halService.toHalCollectionWrapper( list, "collectionName", Record.class);
        // ...
        return halCollection;
    }

    @GET
    @Produces({ MediaType.APPLICATION_JSON, RestMediaType.APPLICATION_HAL_JSON })
    @Path("/{id}")
    @RestLink(rel = "self")
    @InjectRestLinks(RestLinkType.INSTANCE)
    public HalEntityWrapper<Record> get(@PathParam("id") int id) {
        Record entity = // ...
        HalEntityWrapper<Record> halEntity = halService.toHalWrapper(entity);
        // ...
        return halEntity;
    }
}

Creating the wrappers using the constructors will still work for now, but this might change in the future.

@Sgitario this probably needs to be backported to 3.7 as it is broken there atm.

@Sgitario a side note here. The type validation expects to find the id, @RestlInkId or @Id on the first level. You could theoretically do HalCollectionWrapper<Collection<Record>> but that will fail, as was already failing previously.

Can you also remove the label area/infinispan? This got added when I let GitHub update the branch. It defaults to merge commit in stead of rebase.

@Sgitario a side note here. The type validation expects to find the id, @RestlInkId or @Id on the first level. You could theoretically do HalCollectionWrapper<Collection<Record>> but that will fail, as was already failing previously.

Can you also remove the label area/infinispan? This got added when I let GitHub update the branch. It defaults to merge commit in stead of rebase.

Item added into the migration guide. I've renamed a bit the title to match the section in the guide: https://github.com/quarkusio/quarkus/wiki/Migration-Guide-3.9#resteasy-reactive-adding-web-links-programmatically

I've updated the labels as well.

cc @geoand

🙏🏼

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit 9855040.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

⚠️ There are other check runs running, make sure you don't need to wait for their status before merging.

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 9855040.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

@geoand @Sgitario many thanks for the fast support on my questions getting this fixed.

Thank you for the contribution!