Thoughts around WrapperImpls and providing out own Impls

[nzcoward]

Intro

Currently you are using a fixed WrapperImpl in Mediator.cs, i.e. one of Notification, Request, StreamRequest.

For example, for IRequest sending:

public class Mediator : IMediator
{
    //....
    public Task<TResponse> Send<TResponse>(IRequest<TResponse> request, CancellationToken cancellationToken = default)
    {
        if (request == null)
        {
            throw new ArgumentNullException(nameof(request));
        }

        var requestType = request.GetType();

        var handler = (RequestHandlerWrapper<TResponse>)_requestHandlers.GetOrAdd(requestType,
            static t => (RequestHandlerBase)(Activator.CreateInstance(typeof(RequestHandlerWrapperImpl<,>).MakeGenericType(t, typeof(TResponse)))
                                             ?? throw new InvalidOperationException($"Could not create wrapper type for {t}")));

        return handler.Handle(request, cancellationToken, _serviceFactory);
    }
    // ....
}

I thought I would open an issue to get thoughts around a possible implementation, or even the idea in general - does it even warrant considering when there are other options.

Motivation

I would like to provide my own WrapperImpl in order to e.g. have a pipeline that uses a single DB transaction throughout.

Implementation

I am not sure of the best way to do this, but was thinking of using a type 'provider', e.g.

public interface IRequestHandlerWrapperTypeProvider
{
    Type ImplementationType { get; }
}

public class DefaultRequestHandlerWrapperTypeProvider : IRequestHandlerWrapperTypeProvider
{
    public Type ImplementationType => typeof(RequestHandlerWrapperImpl<,>);
}

public class TransactionAwareRequestHandlerWrapperTypeProvider : IRequestHandlerWrapperTypeProvider
{
    public Type ImplementationType => typeof(TransactionAwareRequestHandlerWrapperImpl<,>);
}

The issue with this is that there is no guarantee that the implementation type is of type RequestHandlerBase, which is not ideal. But it does mean that we can change up the Mediator to look a little like the below, and then provide our own implementation type. (It does remove your static functions used in GetOrAdd, though, as _serviceFactory is needed, and I assume that's not static in order to handle threading).

public class Mediator : IMediator
{
    //....
    public Task<object?> Send(object request, CancellationToken cancellationToken = default)
    {
        // ....
                var responseType = requestInterfaceType.GetGenericArguments()[0];

                // Here we create out own implementation type by pulling the type provider from the service factory..                 
                var wrapperType = _serviceFactory.GetInstance<IRequestHandlerWrapperTypeProvider>().ImplementationType;

                return (RequestHandlerBase)(Activator.CreateInstance(wrapperType.MakeGenericType(requestTypeKey, responseType)) 
                                            ?? throw new InvalidOperationException($"Could not create wrapper for type {wrapperType}"));
            });

        // call via dynamic dispatch to avoid calling through reflection for performance reasons
        return handler.Handle(request, cancellationToken, _serviceFactory);
    }
    // ....
}

The WrapperImpl would look something like this:

namespace MyNameSpace;

public class TransactionAwareRequestHandlerWrapperImpl<TRequest, TResponse> : RequestHandlerWrapper<TResponse>
    where TRequest : IRequest<TResponse>
{
    public override async Task<object?> Handle(object request, CancellationToken cancellationToken,
        ServiceFactory serviceFactory) =>
        await Handle((IRequest<TResponse>) request, cancellationToken, serviceFactory).ConfigureAwait(false);

    public async override Task<TResponse> Handle(IRequest<TResponse> request, CancellationToken cancellationToken,
        ServiceFactory serviceFactory)
    {
        using var context = serviceFactory.GetInstance<IDbContextFactory<Context>>().CreateDbContext();
        using var transaction = await context.Database.BeginTransactionAsync();

        Task<TResponse> Handler() => GetHandler<ITransactionAwareRequestHandler<TRequest, TResponse>>(serviceFactory).Handle((TRequest) request, transaction, cancellationToken);

        var response = await serviceFactory
            .GetInstances<ITransactionAwarePipelineBehavior<TRequest, TResponse>>()
            .Reverse()
            .Aggregate((RequestHandlerDelegate<TResponse>) Handler, (next, pipeline) => () => pipeline.Handle((TRequest) request, transaction, cancellationToken, next))();

        transaction.Commit();

        return response;
    }
}

With my own defined interfaces, ITransactionAwareRequestHandler and ITransactionAwarePipelineBehavior.

There are other options that do not require code changes to Mediatr. For example using some sort of Transaction Provider that could be injected into my own PipelineBehaviours which can provide a specific transaction based on e.g. the current Thread.

Thoughts and discussions welcome!

[jbogard]

Why not use a pipeline behavior? You don't need to necessarily have a transaction based on a thread. ASP.NET Core handles this by registering the DbContext as Scoped, which for a given request, will share the same instance and therefore same transaction.

I don't know what hosting context you're using but having resources shared for a given scope/context is one of the biggest reasons to use Scoped services.

[nzcoward]

If I was using asp.net then a scoped context would be ok. I was thinking about ways to manage the scope per pipeline and came across this in the source.

I'm building an offline-first MAUI/Xamarin app, so I have to manage the scope myself.

And in all honesty it's not really a big issue duplicating transaction management code in each handler, or wrapping handlers to do what I need - I just like the pipeline model (and agreed with one of your older posts that it was generally better for DI than composing handlers).

[jbogard]

I would still go the scope route and create it yourself. That's what I do in background services.

[nzcoward]

Yeah I'll have a think about the best way to do that. The limitation is discoverability deeper into the pipeline - I'm not a fan of pulling magic contexts out of the air - we did that in an older asp.net project years ago with owin middleware, and coming back to it later took me a little time to realise what in earth we had done!