jbogard / MediatR.Extensions.Microsoft.DependencyInjection

MediatR extensions for Microsoft.Extensions.DependencyInjection
MIT License
327 stars 89 forks source link

Multiple pre/post request processors for same request #58

Closed henkmollema closed 5 years ago

henkmollema commented 5 years ago

Is it by design that only a single request pre/post processor is registered per request?

https://github.com/jbogard/MediatR.Extensions.Microsoft.DependencyInjection/blob/ee630a5d5d4a235255aa02938af4b9d034adf57c/src/MediatR.Extensions.Microsoft.DependencyInjection/ServiceCollectionExtensions.cs#L119-L120

I'd like to decorate my requests or responses with behavior by using multiple pre/post processors, however only one instance of the processor is invoked. In my opinion it would make sense to pass addIfAlreadyExists: true to ConnectImplementationsToTypesClosing for IRequestPreProcessor<> and IRequestPostProcessor<,>.

jbogard commented 5 years ago

Nope, that's a bug.

jbogard commented 5 years ago

It should be checking the implementation type, so that you don't create more than one concrete registration.

henkmollema commented 5 years ago

Alright. I'll see if I can create a fix for it.

henkmollema commented 5 years ago

Odd, there seem to be tests in place which tests two pre- and post processors. However, this repro seems similar but is not working as expected:

class Program
{
    public static async Task Main(string[] args)
    {
        var services = new ServiceCollection();
        services.AddMediatR(typeof(Program).Assembly);
        services.AddTransient(typeof(IPipelineBehavior<,>), typeof(GenericProcessor<,>));
        var sp = services.BuildServiceProvider();
        var scoped = sp.CreateScope().ServiceProvider;
        var mediator = scoped.GetRequiredService<IMediator>();

        var request = new Command();
        var response = await mediator.Send(request);

        Console.ReadKey();
    }
}

public class Command : IRequest<int>
{
    public bool Generic { get; set; }
    public bool Pre { get; set; }
    public bool Pre2 { get; set; }
    public bool Handler { get; set; }
    public bool Post { get; set; }
    public bool Post2 { get; set; }
}

public class PreProcessor : IRequestPreProcessor<Command>
{
    public Task Process(Command request, CancellationToken cancellationToken)
    {
        request.Pre = true;
        return Task.CompletedTask;
    }
}

public class PreProcessor2 : IRequestPreProcessor<Command>
{
    public Task Process(Command request, CancellationToken cancellationToken)
    {
        request.Pre2 = true;
        return Task.CompletedTask;
    }
}

public class CommandHandler : IRequestHandler<Command, int>
{
    public Task<int> Handle(Command request, CancellationToken cancellationToken)
    {
        request.Handler = true;
        return Task.FromResult(42);
    }
}

public class PostProcessor : IRequestPostProcessor<Command, int>
{
    public Task Process(Command request, int response)
    {
        request.Post = true;
        return Task.CompletedTask;
    }
}

public class PostProcessor2 : IRequestPostProcessor<Command, int>
{
    public Task Process(Command request, int response)
    {
        request.Post2 = true;
        return Task.CompletedTask;
    }
}

public class GenericProcessor<TRequest, TResponse> : IPipelineBehavior<TRequest, TResponse>
{
    public async Task<TResponse> Handle(TRequest request, CancellationToken cancellationToken, RequestHandlerDelegate<TResponse> next)
    {
        if (request is Command command)
        {
            command.Generic = true;
        }

        return await next();
    }
}

The Pre2 and Post2 booleans both stay false.

henkmollema commented 5 years ago

It works for open generic pre/post processors like:

public class PreProcessor<TRequest> : IRequestPreProcessor<TRequest>

But not for closed ones like:

public class PreProcessor : IRequestPreProcessor<Command>
henkmollema commented 5 years ago

The problem lies here:

https://github.com/jbogard/MediatR.Extensions.Microsoft.DependencyInjection/blob/ee630a5d5d4a235255aa02938af4b9d034adf57c/src/MediatR.Extensions.Microsoft.DependencyInjection/ServiceCollectionExtensions.cs#L193-L196

Only the first concrete type will be added as they share the same service type. The open generic pre processor gets add as part of the multi open generic interfaces registration where AddTransient is used:

https://github.com/jbogard/MediatR.Extensions.Microsoft.DependencyInjection/blob/ee630a5d5d4a235255aa02938af4b9d034adf57c/src/MediatR.Extensions.Microsoft.DependencyInjection/ServiceCollectionExtensions.cs#L129-L142

I'm not sure if it's safe to use AddTransient for both cases. The tests seem to get upset if I do that.

jbogard commented 5 years ago

Perhaps both paths shouldn't use the same registration method then?

henkmollema commented 5 years ago

Changing the Boolean which allows multiple registrations seems to do the trick. Could you take a look at #59?

rus-art commented 5 years ago

When will it be released?

robjackstewart commented 5 years ago

Any news on when this will be released as running into this problem?