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

ReactiveAdapterRegistry is not compatible with Mutiny 2 #31000

Closed
pbnoyz opened this issue Aug 6, 2023 · 6 comments
Closed

ReactiveAdapterRegistry is not compatible with Mutiny 2 #31000

pbnoyz opened this issue Aug 6, 2023 · 6 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Milestone

Comments

@pbnoyz
Copy link

pbnoyz commented Aug 6, 2023

Mutiny 2 is based on the java.util.concurrent.Flow API instead of the Reactive Streams API. In other words, uni.convert().toPublisher() returns a java.util.concurrent.Flow.Publisher instead of an org.reactivestreams.Publisher. However, the ReactiveAdapterRegistry class adapts to Mutiny 1, which leads to runtime errors with Mutiny 2.

See https://smallrye.io/smallrye-mutiny/2.3.1/reference/migrating-to-mutiny-2/.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Aug 6, 2023
@sbrannen sbrannen changed the title ReactiveAdapterRegistry is not compatible with Mutiny 2 ReactiveAdapterRegistry is not compatible with Mutiny 2 Aug 6, 2023
@sbrannen sbrannen added in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement labels Aug 6, 2023
@jhoeller jhoeller removed the status: waiting-for-triage An issue we've not yet triaged or decided on label Aug 6, 2023
@jhoeller jhoeller self-assigned this Aug 6, 2023
@jhoeller jhoeller added this to the 6.1.0-M4 milestone Aug 6, 2023
@jhoeller
Copy link
Contributor

jhoeller commented Aug 6, 2023

I've made our MutinyRegistrar compatible with both Mutiny 1.x and Mutiny 2.x through reflective adaptation for the latter, using a simple Flow.Publisher bridge of our own - in order to not require Reactor presence for Mutiny support. As a bonus, since we got that simple Flow.Publisher bridge now, if Reactor is not present, ReactiveAdapterRegistry registers our own bridge as an alternative to the automatically registered bridge in Reactor.

@magicprinc
Copy link

Any chance of getting a backport in 5.3 ?

@jhoeller
Copy link
Contributor

I'm afraid that would be rather unusual since there is significant impact on the default setup of ReactiveAdapterRegistry here, and since we expect 5.3.x to be used with the open source generation from its era, not with newly launched major versions. Just like there is no support for Hibernate 6.x or any of the Jakarta EE 9/10 APIs in 5.3.x either, Mutiny 2 arguably falls into that category as well. That said, you can register your own adapter for Mutiny 2, inspired by the one from our 6.1 ReactiveAdapterRegistry, using ReactiveAdapterRegistry.getSharedInstance().registerReactiveType(...). As a benefit, you don't need to do the Mutiny 1 vs 2 adaptation dance that we got there but could rather register a straight Mutiny 2 adapter instead.

What's bringing Mutiny 2 into your 5.3.x stack, actually? Any chance you could upgrade to 6.1 in the near future in general?

@magicprinc
Copy link

magicprinc commented Aug 14, 2023

@jhoeller I have handcrafted such "solution" (val comes from Lombok ≈ final var):

    try {
      val registry = ReactiveAdapterRegistry.getSharedInstance();

      val adapters = ReactiveAdapterRegistry.class.getDeclaredField("adapters");
      adapters.setAccessible(true);
      @SuppressWarnings("unchecked")
      val reactiveAdapters = (List<ReactiveAdapter>) adapters.get(registry);
      // remove Spring-own-Mutiny-v1-style adapters: they are there because Mutiny is in ClassPath: same class names for v1 and v2 AND they are before us
      reactiveAdapters.removeIf(a->Uni.class.isAssignableFrom(a.getReactiveType()) || Multi.class.isAssignableFrom(a.getReactiveType()));

      registry.registerReactiveType(
          ReactiveTypeDescriptor.singleOptionalValue(Uni.class, Uni.createFrom()::nothing),
          uni->FlowAdapters.toPublisher(( (Uni<?>) uni ).convert().toPublisher()),
          publisher->Uni.createFrom().publisher(FlowAdapters.toFlowPublisher(publisher)));

      registry.registerReactiveType(
          ReactiveTypeDescriptor.multiValue(Multi.class, Multi.createFrom()::empty),
          multi->FlowAdapters.toPublisher((Multi<?>) multi),
          publisher->Multi.createFrom().publisher(FlowAdapters.toFlowPublisher(publisher)));
    } catch (Exception e){// ClassNotFound? Access?
      LOGGER.warn("Can't fix Spring ReactiveAdapterRegistry", e);
    }

As You know, ReactiveAdapterRegistry doesn't allow removing adapters :-(

Probably you can add such method or search adapters backwards (to allow their "replacement")?

About Mutiny 2 and Spring: we work with old code base and are trying to fix everything step by step.
Migration from Spring 5.3 to Spring 6.+ requires a lot of changes (all those javax → jakarta replacements)

PS: I use org.reactivestreams.FlowAdapters from org.reactivestreams:reactive-streams:1.0.4

@jhoeller
Copy link
Contributor

Oh good point that FlowAdapters is part of the Reactive Streams API jar since 1.0.3, so there is no point in trying to avoid the extra dependency. I'll revise our 6.1 internals there to rely on FlowAdapters as well.

As for removing adapters, good point as well. I'll make sure that they are removable in some form and will backport that to 5.3.30.

@jhoeller
Copy link
Contributor

Actually, the simplest approach might indeed be to register a new adapter as an override, adding it first to the list (as an alternative to searching them backwards). So not identifying the old adapter for removal, just overriding it by a new adapter for the same reactive types which is being found first. I'm considering a new registerReactiveTypeOverride method for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

5 participants