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

Improve conversion of Mutiny handlers #1037

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

tsegismont
Copy link
Contributor

This is port of vert-x3/vertx-rx#324

It fixes Vert.x Web router (and others) functionnality that is broken with Mutiny API. See vert-x3/vertx-rx#314

The purpose of this change is to avoid decorating handlers if they are instances of a generated Mutiny type (e.g. io.vertx.mutiny.ext.web.handler.StaticHandler).

@tsegismont
Copy link
Contributor Author

@jponge @cescoffier please take a look

I will add a test asap for the fixed functionnality, but at least we should see the current test suite still passes.

@jponge
Copy link
Member

jponge commented Jan 9, 2025

@cescoffier I had a meeting with @tsegismont who explained me the case, it does avoid some extra boxing and fixes dispatch bugs due to type comparisons not expecting shims but the wrapped types.

Let's do a final review once tests have been added.

@tsegismont tsegismont marked this pull request as ready for review January 9, 2025 14:14
@tsegismont tsegismont force-pushed the mutiny-handlers-conversion branch 2 times, most recently from 1a25ecd to d74bbe6 Compare January 9, 2025 14:17
@tsegismont
Copy link
Contributor Author

@jponge @cescoffier I've added the test

@tsegismont tsegismont force-pushed the mutiny-handlers-conversion branch from d74bbe6 to f7b96d4 Compare January 9, 2025 14:26
This is port of vert-x3/vertx-rx#324

It fixes Vert.x Web router (and others) that is broken with Mutiny API.
See vert-x3/vertx-rx#314

The purpose of this change is to avoid decorating handlers if they are instances of a generated Mutiny type (e.g. io.vertx.mutiny.ext.web.handler.StaticHandler).

Signed-off-by: Thomas Segismont <[email protected]>
This test verifies that, when a generated Mutiny handler is set, the bare Vert.x handler is provided to Vert.x, instead of a delegating handler applying redundant conversions (bare->Mutiny->bare).

Signed-off-by: Thomas Segismont <[email protected]>
@tsegismont tsegismont force-pushed the mutiny-handlers-conversion branch from f7b96d4 to 9b011d3 Compare January 9, 2025 15:17
@jponge jponge requested a review from cescoffier January 9, 2025 18:27
@tsegismont
Copy link
Contributor Author

Hold off merging the PR, I'm trying to determine if we can improve it a little further

@tsegismont
Copy link
Contributor Author

Hold off merging the PR, I'm trying to determine if we can improve it a little further

Seems not. Sorry for the noise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants