reactor / reactor-core

Non-Blocking Reactive Foundation for the JVM
http://projectreactor.io
Apache License 2.0
4.99k stars 1.21k forks source link

Common interface for Flux and Mono to interact with context #3866

Closed mikrethor closed 3 months ago

mikrethor commented 3 months ago

In my project I had to add stuff in the context and I had to duplicate the method for Mono and Flux. It would be convenient to have an interface such as CoreContext

public interface CoreContext {

    CoreContext contextCapture();

    CoreContext contextWrite(ContextView contextToAppend);

    CoreContext contextWrite(Function<Context, Context> contextModifier);
}

And in Mono becomes :

public abstract class Mono<T> implements CorePublisher<T>, CoreContext {

//...
public final Mono<T> contextCapture() {
        if (!ContextPropagationSupport.isContextPropagationAvailable()) {
            return this;
        }
        if (ContextPropagationSupport.propagateContextToThreadLocals) {
            return onAssembly(new MonoContextWriteRestoringThreadLocals<>(
                    this, ContextPropagation.contextCapture()
            ));
        }
        return onAssembly(new MonoContextWrite<>(this, ContextPropagation.contextCapture()));
    }

public final Mono<T> contextWrite(ContextView contextToAppend) {
        return contextWrite(c -> c.putAll(contextToAppend));
    }

public final Mono<T> contextWrite(Function<Context, Context> contextModifier) {
        if (ContextPropagationSupport.shouldPropagateContextToThreadLocals()) {
            return onAssembly(new MonoContextWriteRestoringThreadLocals<>(
                    this, contextModifier
            ));
        }
        return onAssembly(new MonoContextWrite<>(this, contextModifier));
    }
//...
}

And Flux :

public abstract class Flux<T> implements CorePublisher<T>, CoreContext {

...
public final Flux<T> contextCapture() {
        if (!ContextPropagationSupport.isContextPropagationAvailable()) {
            return this;
        }
        if (ContextPropagationSupport.propagateContextToThreadLocals) {
            return onAssembly(new FluxContextWriteRestoringThreadLocals<>(
                    this, ContextPropagation.contextCapture()
            ));
        }
        return onAssembly(new FluxContextWrite<>(this, ContextPropagation.contextCapture()));
    }

public final Flux<T> contextWrite(ContextView contextToAppend) {
        return contextWrite(c -> c.putAll(contextToAppend));
    }

public final Flux<T> contextWrite(Function<Context, Context> contextModifier) {
        if (ContextPropagationSupport.shouldPropagateContextToThreadLocals()) {
            return onAssembly(new FluxContextWriteRestoringThreadLocals<>(
                    this, contextModifier
            ));
        }
        return onAssembly(new FluxContextWrite<>(this, contextModifier));
    }    

...
}

The motivation for a developer who uses reactor would be to have :

public static <T> Mono<T> addStuffInContext(Mono<T> mono){
    return mono.contextWrite(context->context.putAllMAp(Map.of("key", "value")));
}

public static <T> Flux<T> addStuffInContext(Flux<T> mono){
    return mono.contextWrite(context->context.putAllMAp(Map.of("key", "value")));
}

becoming :

public static <T> CoreContext<T> addStuffInContext(CoreContext<T> mono){
    return mono.contextWrite(context->context.putAllMAp(Map.of("key", "value")));
}

I could contribute it if we agree where in which package to put CoreContext (ex: reactor.core) and how to name it

chemicL commented 3 months ago

Hey @mikrethor,

Thanks for the suggestion. There are a number of issues with the proposed solution:

  1. Adding a common interface for both Flux and Mono is costly in terms of maintenance and limits our flexibility in the future.
  2. Returning the CoreContext type from addStuffInContext makes it impossible to use Mono or Flux-specific fluent APIs after that call and would require casting to dhere to the return type being specific for example in a controller method.
  3. There is already a common type, CorePublisher and adding to the hierarchy is not a desired outcome.
  4. Dynamic dispatch can be costly so we should avoid aiming for abstraction at all costs.

As far as I can see, the only trouble that you are facing is having to add 3 more lines of code that look duplicated. It's not a goal for us to optimize for less typing but for a comprehensive and performant codebase. With that, I'm sorry to reject the proposal at this time.