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

Suggestion/Question: Making automatic wireup of ASP.NET Core or other CancellationToken easier #496

Closed
Schmaga opened this issue Feb 19, 2020 · 12 comments

Comments

@Schmaga
Copy link

Schmaga commented Feb 19, 2020

Hi everyone,

I am using MediatR in an ASP.NET Core 3.1 environment and I was looking for an easy way to automatically pass the HttpContext.RequestAborted CancellationToken to the RequestHandlers.

Trying different approaches and failing, I ended up with a solution like this:

public class AmbientHttpCancellationTokenMediatorDecorator : IMediator
{
    private readonly IHttpContextAccessor _httpContextAccessor;
    private readonly Mediator _mediator;

    public AmbientHttpCancellationTokenMediatorDecorator(
        ServiceFactory serviceFactory,
        IHttpContextAccessor httpContextAccessor)
    {
        _httpContextAccessor = httpContextAccessor;
        _mediator = new Mediator(serviceFactory);
    }

    public async Task<TResponse> Send<TResponse>(IRequest<TResponse> request, CancellationToken cancellationToken = new CancellationToken())
    {
        var cancellationTokenToUse = GetRequestAbortedCancellationTokenOrUseTokenFromMethodArguments(cancellationToken);
        return await _mediator.Send(request, cancellationTokenToUse);
    }

    public async Task Publish(object notification, CancellationToken cancellationToken = new CancellationToken())
    {
        var cancellationTokenToUse = GetRequestAbortedCancellationTokenOrUseTokenFromMethodArguments(cancellationToken);
        await _mediator.Publish(notification, cancellationTokenToUse);
    }

    public async Task Publish<TNotification>(TNotification notification, CancellationToken cancellationToken = new CancellationToken()) where TNotification : INotification
    {
        var cancellationTokenToUse = GetRequestAbortedCancellationTokenOrUseTokenFromMethodArguments(cancellationToken);
        await _mediator.Publish(notification, cancellationTokenToUse);
    }

    private CancellationToken GetRequestAbortedCancellationTokenOrUseTokenFromMethodArguments(CancellationToken cancellationToken)
    {
        return _httpContextAccessor.HttpContext?.RequestAborted ?? cancellationToken;
    }
}

And then binding the above implementation as an IMediatR replacement:

services.AddMediatR(typeof(Startup).Assembly);
services.AddScoped<IMediator, AmbientCancellationTokenMediatorDecorator>();

This works fine, but seems a bit clunky to me (reflected in the name as well :)), so I was wondering if there were any easier ways to this that I did not find. I had hoped to simply use a PipelineBehavior that has the IHttpContextAccessor injected and overrides the cancellation Token from the method call. But that is not possible because the RequestHandlerDelegate does not expose any method parameters for the request and cancellationToken, so there is no way to override them this way. I also would prefer to not put them in the requests themselves.

Any ideas or did I miss an easy way?

@lilasquared
Copy link
Contributor

This looks like a pretty elegant solution to me. The only thing I would change with this would be to inject the original mediator from CI and not build up your own internally.

@Schmaga
Copy link
Author

Schmaga commented Feb 19, 2020

That is actually a good idea, but injecting the original mediator is not that easy without some extra work I think.

This will result in a stackoverflow:

services.AddScoped<IMediator, AmbientHttpCancellationTokenMediatorDecorator>(
    provider =>new AmbientHttpCancellationTokenMediatorDecorator(provider.GetService<IMediator>(), 
    provider.GetService<IHttpContextAccessor>()));

This will not work because the Mediator is only bound via the IMediator interface, not its implementation:

services.AddScoped<IMediator, AmbientHttpCancellationTokenMediatorDecorator>(
    provider => new AmbientHttpCancellationTokenMediatorDecorator(provider.GetService<Mediator>(),
    provider.GetService<IHttpContextAccessor>()));

Any suggestions?

@lilasquared
Copy link
Contributor

lilasquared commented Feb 19, 2020

Can you bind it via implementation just before?

services.AddMediatR(typeof(Startup).Assembly);
services.AddScoped<Mediator>()
services.AddScoped<IMediator, AmbientHttpCancellationTokenMediatorDecorator>(
    provider => new AmbientHttpCancellationTokenMediatorDecorator(
        provider.GetService<Mediator>(),
        provider.GetService<IHttpContextAccessor>()));

not ideal but might get you there. Other DI like structuremap have methods for decorating but looks like aspnetcore doesn't have any natively other than the above

@Schmaga
Copy link
Author

Schmaga commented Feb 19, 2020

That seems to work, yea.

Is this solution something that would make sense in an extension library you provide? Outfitted with an extension method on the MediatR setup of ASP.NET Core named UseHttpRequestCancellationToken() (or similar) this could probably make a lot of people's lives easier. I could refine it a bit and create a PR if you want. Otherwise I will just close this issue and just leave it for documentation purposes in case anyone else wants to solve the same problem.

@lilasquared
Copy link
Contributor

I could see it being part of https://github.com/jbogard/MediatR.Extensions.Microsoft.DependencyInjection as a build in custom mediator decorator, then the method can chain off the .AddMediator(). Even then this is specific to an http context implementation, where as both MediatR and the DI extension project are agnostic of currently. A third option would be your own extension project entirely target to web projects.

@Schmaga
Copy link
Author

Schmaga commented Feb 19, 2020

You're right, the DI extension library is independent of specific environments, where HttpContext might not even be available.

I will think on this a bit more and maybe create a small MediatR.Extensions.Microsoft.AspNetCore library (name not final, but could work) that uses the DI library. I'm wondering why no one else ever thought about building this :) In the meantime, if you want to share any suggestions, I would be happy to hear them.

@Schmaga
Copy link
Author

Schmaga commented Feb 22, 2020

Ok, I created a repository for this: https://github.com/Schmaga/MediatR.Extensions.Microsoft.AspNetCore

It is actually my first open source project, so there might be some issues. If you like and have the time, have a look at it. Would love to hear what you think. I will close this issue here.

@georgy93
Copy link

georgy93 commented Apr 30, 2021

Hello!
@Schmaga 's library looks pretty good and I almost used it in a project but then I remembered something important which stopped me.
MediatR is a CQRS approached library and we know that Commands modify state and Queries just return data without side-effects.
However, MediatR doesn't know if the message we are sending in our code is Command or Query. For us there is a conceptual difference but for MediatR - it's just a message.

The conceputal difference is very important, though.
A rule I believe in and follow is that state-changing operations should not be aborted because they can complete halfway and leave the system in inconsistent state.
For example, a user sends a request to create/update a resource in the system which takes some time, but their connection drops for some reason (they lost internet connection or didn't want to wait and closed the browser, or by accident).
However, internally, the system is writing to a persistence storage, maybe publishing integration event to a message broker and also changing some other objects states, leading to more integration events to be published.

  • What happens here if the request was aborted in the middle?
  • How do we properly stop the operation and keep the system's data consistency?
    Of course, such issues can be handled locally in the transactional-script method (the Command handler), but it is up to the developers on the team to be aware of it.
    However, there is another more important question that raises:
  • WHY do we have to stop the operation, when the user intentionally started it (probably even going through a confirm dialog), and we have no idea what action caused the request to be aborted?

On the other hand, if this user aborted their request when they were just querying data - who cares?
The intention of the user was just to read some data, but they will no longer be able to see it, so we can safely propagate the RequestAborted cancellation.

Having all this in mind, I can simply do this:

    using MediatR;
    using Microsoft.AspNetCore.Mvc;
    using System.Threading;
    using System.Threading.Tasks;

    [ApiController]
    public abstract class BaseController : ControllerBase
    {
        private IMediator GetMediator() => HttpContext.Features.Get<IMediator>();

        protected Task<TResponse> QueryAsync<TResponse>(IRequest<TResponse> request) => GetMediator().Send(request, HttpContext.RequestAborted);

        protected Task<TResponse> CommandAsync<TResponse>(IRequest<TResponse> request) => GetMediator().Send(request, CancellationToken.None);
// or
// protected Task<TResponse> CommandAsync<TResponse>(IRequest<TResponse> request, CancellationToken cancellationToken = default) => GetMediator().Send(request, cancellationToken);
    }

Please correct me if my thoughts are wrong.

@jbogard
Copy link
Owner

jbogard commented Apr 30, 2021

A rule I believe in and follow is that state-changing operations should not be aborted because they can complete halfway and leave the system in inconsistent state.

What? Don't you use transactions? Or if you're coordinating multiple non-coordinating resources, you can use a saga. Or if you're calling an API, then yes, you don't know if the operation succeeded or not so you wrap the call in something Polly.

These things have to be explicitly designed though, and depend highly on the operation you're performing, and are difficult to generalize. You can use behaviors though to get some commonality, if you like.

@georgy93
Copy link

@jbogard I agree that such things are difficult to generalize and should be explicitly designed.

@Schmaga May I suggest an improvement for your library?
I saw that you are always taking Request Aborted cancellationtoken with preference over the one supplied in the method

Wouldn't it be better if you combine both?

private CancellationTokenSource MergeRequestAbortedCancellationTokenWithDispatchedCancellationToken(CancellationToken cancellationToken) =>
   _httpContextAccessor?.HttpContext is null
      ? CancellationTokenSource.CreateLinkedTokenSource(cancellationToken)
      : CancellationTokenSource.CreateLinkedTokenSource(_httpContextAccessor.HttpContext.RequestAborted, cancellationToken);

And then use this as follows:

  public async Task<TResponse> Send<TResponse>(IRequest<TResponse> request, CancellationToken cancellationToken = new CancellationToken())
  {
       using var cts = MergeRequestAbortedCancellationTokenWithDispatchedCancellationToken(cancellationToken);

       return await _mediator.Send(request, cts.Token);
  }

@Schmaga
Copy link
Author

Schmaga commented May 11, 2021

Suggestions and PRs are always welcome. How about you open an issue in my Repo and we discuss a solution there?

@georgy93
Copy link

Hi!
Sure, here is the issue I opened Schmaga/MediatR.Extensions.Microsoft.AspNetCore#1
I post a link here for a follow up.

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

No branches or pull requests

4 participants