Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Preserve the type of multi in the SecurityHandler #21270

Merged

Conversation

cescoffier
Copy link
Member

Avoid using transformToMulti as we want to preserve the type of multi produced by the user method. This is required for Reactive Routes to handle custom Multi serialization.

Fix #21044.

…MultiContinuation.

Do not use transformToMulti as we want to preserve the type of multi produced by the user method. This is required for Reactive Routes to handle custom Multi serialization.
@geoand
Copy link
Contributor

geoand commented Nov 8, 2021

Oh, very nice! I didn't know about this capability

.onItem().transformToMulti(new MultiContinuation(ic));
// Do not use transformToMulti as we want to preserve the type of multi produced by the user method.
// This is required for Reactive Routes to handle custom Multi serialization.
.toMulti().plug(x -> new MultiContinuation(ic).apply(x));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the actual difference?

Copy link
Contributor

@mkouba mkouba Nov 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUIC you don't wrap the Multi produced by the user method but instead call new MultiContinuation(ic).apply(x) when the Multi instance is created, i.e. when the plug() method is being executed.

Copy link
Contributor

@mkouba mkouba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uni.toMulti().plug() looks a bit like some dark magic but if it does what I think it does then +1 :D

@michalszynkiewicz
Copy link
Member

would be great to have a test that it actually fixes the linked issue :)

@quarkus-bot
Copy link

quarkus-bot bot commented Nov 8, 2021

Failing Jobs - Building 03e4b49

Status Name Step Failures Logs Raw logs
Native Tests - Windows - hibernate-validator Build Failures Logs Raw logs

Failures

⚙️ Native Tests - Windows - hibernate-validator #

- Failing: integration-tests/hibernate-validator 

📦 integration-tests/hibernate-validator

Failed to execute goal io.quarkus:quarkus-maven-plugin:999-SNAPSHOT:build (default) on project quarkus-integration-test-hibernate-validator: Failed to build quarkus application

@gsmet gsmet merged commit 76b25dd into quarkusio:main Nov 8, 2021
@quarkus-bot quarkus-bot bot added this to the 2.5 - main milestone Nov 8, 2021
@cescoffier
Copy link
Member Author

plug is not the most common API. It's used to create your own operator. Here, it's just convenient as we will return the multi we create directly.

@MM87037
Copy link

MM87037 commented Nov 10, 2021

Is it possible to backport this fix to 2.4?

@geoand
Copy link
Contributor

geoand commented Nov 10, 2021

It has already been marked for backporting (see the labels)

@MM87037
Copy link

MM87037 commented Nov 15, 2021

Verified the fix using 2.4.2 version and it resolved #21044. But it introduced a different issue where @RolesAllowed annotation is no longer enforced for resources returning a Multi. Shall I raise a new issue?

@michalszynkiewicz
Copy link
Member

@MM87037 could you create a separete issue and create a reproducer?

@MM87037
Copy link

MM87037 commented Nov 16, 2021

Sure. Thanks

@cescoffier cescoffier deleted the preserve-multi-type-in-security-handler branch March 15, 2022 07:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reactive routes incorrect json serialization
6 participants