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

IRequestPreProcessor not called before IPipeLineBehavior #868

Closed
AleRoe opened this issue Mar 1, 2023 · 13 comments · Fixed by #922
Closed

IRequestPreProcessor not called before IPipeLineBehavior #868

AleRoe opened this issue Mar 1, 2023 · 13 comments · Fixed by #922

Comments

@AleRoe
Copy link

AleRoe commented Mar 1, 2023

Hi there,

I'm having some difficulties migrating my code to version 12.0.1.

I have a RequestPreProcessor defined as

public class RequestContextPreProcessor<TRequest> : IRequestPreProcessor<TRequest>  
where TRequest : notnull, IHomeControllerRequest

and a number of Behaviors which I register using the follwoing code:

services.AddMediatR(config => {
                        config.RegisterServicesFromAssemblies(PluginProvider.GetAssemblies());
                        config.AddOpenBehavior(typeof(TelemetryBehavior<,>));
                        config.AddOpenBehavior(typeof(LoggingBehavior<,>));
                        config.AddOpenBehavior(typeof(ErrorHandlingBehavior<,>));
                    })

I can't get the RequestPreProcessor to execute. For some reason, it's not being registered and injected as part of the pipeline. My code worked fine with version 11.
I tried explicitly registering the RequestPreProcessor, but that did not work either.

Any suggestions?

Thanks
Alexander

@AleRoe
Copy link
Author

AleRoe commented Mar 3, 2023

Update:
I did some more tests.
It turns out that the RequestPreProcesser is actually registered and being loaded (CTOR is called), but the Process() method is not being executed prior to any other behaviors (verified using breakpoints)

My workaround for now is to declare my PreProcessor as a regular IPipelineBehavior and adding it as the first behavior, but this kinda defies the purpose of the RequestPreProcessor.

Thanks,
Alexander

@AleRoe AleRoe changed the title IRequestPreProcessor not being registered IRequestPreProcessor not being executed Mar 3, 2023
@StevenTCramer
Copy link

@AleRoe do you have small reproducible repo?

@StevenTCramer
Copy link

I have an example where this is happening, and the issue is that the RequestPostProcessorBehavior isn't being registered. I am trying my hardest to debug this but right now it only happens in WASM for me and VS and Rider frankly suck at debugging in WASM.

@AleRoe
Copy link
Author

AleRoe commented Mar 10, 2023

I'll try to create a sample from my code asap.

@StevenTCramer
Copy link

@jbogard @AleRoe I found it!!! AddMediatR will NOT register the RequestPostProcessorBehavior if it thinks there are no implementations.

Previously in (version 11) if you called AddMediatR and then registered your PostProcessor it would have worked. Now you have to Register your PostProcessor before calling AddMediatR or have it in the assembly being scanned.

I couldn't get VS or Rider to debug and resulted to old school Console.Writelines.

    private static void RegisterBehaviorIfImplementationsExist(IServiceCollection services, Type behaviorType, Type subBehaviorType)
    {
        if (typeof(RequestPostProcessorBehavior<,>) == behaviorType)
        {
            Console.WriteLine("YOYO ---- This is where it is trying to register the RequestPostProcessorBehavior");
        }
        var hasAnyRegistrationsOfSubBehaviorType = services
            .Select(service => service.ImplementationType)
            .OfType<Type>()
            .SelectMany(type => type.GetInterfaces())
            .Where(type => type.IsGenericType)
            .Select(type => type.GetGenericTypeDefinition())
            .Any(type => type == subBehaviorType);
        
        if (typeof(RequestPostProcessorBehavior<,>) == behaviorType && !hasAnyRegistrationsOfSubBehaviorType)
        {
            Console.WriteLine("It doesn't think there are any subtypes therefore doesn't attempt to register the behavior");
        }
        
        if (hasAnyRegistrationsOfSubBehaviorType)
        {
            if (typeof(RequestPostProcessorBehavior<,>) == behaviorType)
            {
                Console.WriteLine("Register RequestPostProcessorBehavior if not already.");
            }
            services.TryAddEnumerable(new ServiceDescriptor(typeof(IPipelineBehavior<,>), behaviorType, ServiceLifetime.Transient));
        }
    }

@AleRoe
Copy link
Author

AleRoe commented Mar 14, 2023

Hi @StevenTCramer,
thanks for investigating. But your findings don't quite apply to my situation, as my problem relates to IRequestPreProcessor.
In my case, the IRequestPreProcessor is registered, but the Process() method is not being called prior to any other IPipelineBevahiors. Registering my IRequestPreProcessor prior to calling AddMediatR() does not help either.

After looking at the source code, I did some more testing, and as it turns out, explicitely adding the RequestPreProcessorBehavior() prior to any other custom behaviors solves my problem:

services.AddMediatR(config => {
                        config.RegisterServicesFromAssemblies(PluginProvider.GetAssemblies());
                        config.AddOpenBehavior(typeof(RequestPreProcessorBehavior<,>));
                        config.AddOpenBehavior(typeof(TelemetryBehavior<,>));
                        config.AddOpenBehavior(typeof(LoggingBehavior<,>));
                        config.AddOpenBehavior(typeof(ErrorHandlingBehavior<,>));
                    });

Now this is not quite what one would expect, as previously it was not required to register that behavior explicitely in order to process IRequestPreProcessor implementations. AddMediatR() should register the bult-in RequestPreProcessorBehavior() by default and prior to any other custom behaviors added using AddOpenBehavior(). I would assume this is a bug or should be noted as a breaking change for v12.x. @jbogard , I would appreciate if you could advise on this.

Kind regards
Alexander

@jbogard
Copy link
Owner

jbogard commented Mar 14, 2023

It's called out elsewhere, but not in the upgrade guide. I'll add a section there that the "built-in" extra behaviors are only registered if AddMediatR finds any implementations.

Those extra behaviors were removed from the default pipeline because there's a performance hit to having them, so I only add them if I can determine they're needed. This behavior is now by design. If there's a missing scenario in your case, we can add it to the scanning or add other similar methods to register them.

@AleRoe
Copy link
Author

AleRoe commented Mar 18, 2023

Hi @jbogard!
Thanks for getting back and the explanations.
Funny thing though is that I do have an IRequestPreProcessor implementation in the main assembly, but its Process() method does not get called (the class is being loaded though, because the constructor is called).
The only way to get it to work is by manually adding the RequestPreProcessorBehavior<,> prior to any other custom behaviors.

So this works:

services.AddMediatR(config => {
                        config.RegisterServicesFromAssemblies(PluginProvider.GetAssemblies());
                        config.AddOpenBehavior(typeof(RequestPreProcessorBehavior<,>));
                        config.AddOpenBehavior(typeof(TelemetryBehavior<,>));
                        config.AddOpenBehavior(typeof(LoggingBehavior<,>));
                        config.AddOpenBehavior(typeof(ErrorHandlingBehavior<,>));
                    });

Also, my code worked fine with v11.

I can live with manually adding the RequestPreProcessorBehavior, but it does seem strange and not how it's supposed to work.
So I did some more testing:

And as it turns out, my IRequestPreProcessor is being called, but in the wrong order.
It gets called after my custom IPipeLineBehaviors, instead of before.
I verified this by first removing all my custom behaviors and then by adding only a single custom beviour that does not depend on the IRequestPreProcessor, and then checking the call-order using breakpoints.

So it seems that the built in pipeline discovery is adding the behaviors in the wrong order, at least when configuring AddMediatR() the way I do it (see my first post please).

Hope this helps.
Kind regards
Alexander

@AleRoe AleRoe changed the title IRequestPreProcessor not being executed IRequestPreProcessor not called before IPipeLineBehavior Mar 26, 2023
@kipkoei
Copy link

kipkoei commented Apr 6, 2023

I'm having the exact same issue. Since upgrading to v12, My IRequestPreProcessors are being called after my IPipelineBehaviors. This breaks a lot in my application since I rely on PreProcessors to fetch some information in the database which is required for both validation and regular handling of the request. The validator is called before my preprocessor now, which means the data required to validate is not yet available.

See the following example:

Let's say we have a CreateTransactionCommand which depends on an Order. The Order is needed in the validation fase to check whether creating a transaction is even allowed and it is needed when handling the request to get the appropriate metadata. Since we don't want to do the same database call twice, we fetch the Order in a IRequestPreProcessor (which is generic so we can reuse it for similar requests):

abstract record WithOrder(int OrderId)
{
    public Order? Order { get; set; }
}

internal class WithOrderPreProcessor<T> : IRequestPreProcessor<T> where T : WithOrder
{
    private readonly IOrderRepository _orderRepository;

    public WithOrderPreProcessor(IOrderRepository orderRepository)
    {
        _orderRepository = orderRepository;
    }

    public async Task Process(T request, CancellationToken cancellationToken)
    {
        request.Order = await _orderRepository.Get(request.OrderId);
    }
}

Furthermore, we have a ValidationBehavior which looks as such:

public class ValidationBehaviour<TRequest, TResponse> : IPipelineBehavior<TRequest, TResponse> where TRequest : notnull
{
    private readonly IEnumerable<IValidator<TRequest>> _validators;

    public ValidationBehaviour(IEnumerable<IValidator<TRequest>> validators)
    {
        _validators = validators;
    }

    // Is somehow called before WithOrderPreProcessor.Process
    public async Task<TResponse> Handle(TRequest request, RequestHandlerDelegate<TResponse> next, CancellationToken cancellationToken)
    {
       // Handle the registered Validators for this request...

        return await next();
    }
}

The request is then handled like this:

public record CreateTransactionCommand(int OrderId) : WithOrder(OrderId), IRequest<Transaction>;

internal class CreateTransactionCommandHandler : IRequestHandler<CreateTransactionCommand, Transaction>
{
    public async Task<Transaction> Handle(CreateTransactionCommand request, CancellationToken cancellationToken)
    {
        // This is filled by the preprocessor
        var order = request.Order;

        // Do stuff...
    }
}

public class CreateTransactionCommandValidator : AbstractValidator<CreateTransactionCommand>
{
    public CreateTransactionCommandValidator()
    {
        // This fails since v12 because the ValidationBehavior is called before the WithOrderPreProcessor
        RuleFor(c => c.Order)
            .Cascade(CascadeMode.Stop)
            .NotNull();

        RuleFor(c => c.Order!.OrderStatus)
            .Equal(OrderStatus.Approved);
    }
}

The DI registration looks like this:

        services.AddValidatorsFromAssemblies(assemblies.Where(p => !p.IsDynamic));
        services.AddMediatR(o =>
        {
            o.RegisterServicesFromAssembly(applicationAssembly)
                .AddOpenBehavior(typeof(ValidationBehaviour<,>));
        });

Hope this helps to pinpoint the origin of the bug.

Kind regards,
Lou

@github-actions
Copy link

github-actions bot commented Jun 6, 2023

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@github-actions github-actions bot added the Stale label Jun 6, 2023
@github-actions
Copy link

This issue was closed because it has been stalled for 14 days with no activity.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jun 20, 2023
@jbogard jbogard reopened this Jul 6, 2023
@jbogard
Copy link
Owner

jbogard commented Jul 6, 2023

I'm going to address this in the next release, to put request pre/post processors before behaviors.

@AleRoe
Copy link
Author

AleRoe commented Jul 7, 2023

Thanks for revisiting this topic!

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 a pull request may close this issue.

4 participants