Consider introducing a single "hub pipeline" abstraction for cross-cutting concerns

[analogrelay]

We had aspnet/SignalR#871 and aspnet/SignalR#924 tracking some of this, and resolved those by adding HubDispatcher as a user-overridable abstraction. However, there's still value in the simple abstraction Hub Pipeline modules provided in ASP.NET SignalR. We should consider bringing that abstraction back.

Considerations:

• We should consider supporting "require authorization on all hubs" as an global option using pipeline modules, possibly just as a sample.

[zeinali0]

Will hub pipeline be added to version 3? i think it's an essential feature for enterprise applications

[BrennanConroy]

Please let us know if this proposal will work with your scenarios or not.

HubFilter

User Scenarios

These are the core scenarios that users have asked for in GitHub issues:

• Logging before and after Hub method invocation
• Parameter validation before Hub method invocation
• Conditionally skip invoking Hub methods
• Handle exceptions from Hub method

Example of someones usage in ASP.NET SignalR 2.X:

public class IsConnectedPipeLine : HubPipelineModule
{
    protected override bool OnBeforeIncoming(IHubIncomingInvokerContext context)
    {
        if (context.MethodDescriptor.Name == "GetToken")
            return true;
        return ChatIdentity.CheckToken(context.Hub.Context.GetCurrentUserToken());
    }
}

With the below proposal:

public class IsConnectedFilter : IHubFilter
{
    public ValueTask<object> InvokeMethodAsync(HubInvocationContext invocationContext, Func<HubInvocationContext, ValueTask<object>> next)
    {
        if (invocationContext.HubMethod.Name == "GetToken" ||
            ChatIdentity.CheckToken(context.Context.GetCurrentUserToken()))
        {
            return next(invocationContext);
        }
    }
}

Proposal

We will provide a single method for incoming invocations (client to server).
And two methods to provide hooks for a connection connecting and disconnecting.

public interface IHubFilter
{
    ValueTask<object> InvokeMethodAsync(HubInvocationContext invocationContext, Func<HubInvocationContext, ValueTask<object>> next);
    Task OnConnectedAsync(HubCallerContext context, Func<HubCallerContext, Task> next) => next(context); // default interface methods so you don't need to implement these
    Task OnDisconnectedAsync(HubCallerContext context, Func<HubCallerContext, Task> next) => next(context);
}

For registration we will have two types. Global filter registration and per-hub filter registration.
Global filters will run first (and exit last since it's middleware) and per-hub filters will run after (and exit first).

For DI activated IHubFilter's they will have the same scope as the Hub; a new one per method invocation.
For perf it is recommended to either provide an instance or register the type as a singleton (services.AddSingleton()), assuming it doesn't need objects from DI.

services.AddSignalR(options =>
{
    // registration order matters
    options.AddFilter<GlobalFilter>();
    options.AddFilter<SecondFilter>();
    options.AddFilter(new CustomFilter());
}).AddHubOptions<THub>(options =>
{
    // registration order matters
    options.AddFilter<RunFirstFilter>();
    options.AddFilter<RunSecondFilter>();
});

Examples:

[AttributeUsage(AttributeTargets.Method, AllowMultiple = false, Inherited = true)]
class StreamingMethodAttribute : Attribute
{
}

class TestHub : Hub
{
    [StreamingMethod]
    IAsyncEnumerable<int> Stream()
    {
        // ...
    }
}

class CustomFilter : IHubFilter
{
    async ValueTask<object> InvokeMethodAsync(HubInvocationContext invocationContext, Func<HubInvocationContext, ValueTask<object>> next)
    {
        try
        {
            // maybe allow modification of arguments etc.
            var res = await next(invocationContext);
    
            if (Attribute.IsDefined(invocationContext.HubMethod, typeof(StreamingMethod)))
            {
                // change return value of a streaming method!
                return Add((IAsyncEnumerable<int>)res);
    
                async IAsyncEnumerable<int> Add(IAsyncEnumerable<int> enumerable)
                {
                    await foreach(var item in enumerable)
                    {
                        yield return item + 5;
                    }
                }
            }
    
            return res;
        }
        catch
        {
            throw new HubException("some error");
        }
        finally
        {
            // logging
        }
    }
}

Modifications

• Add MethodInfo and ServiceProvider from Scope to HubInvocationContext
• Change HubInvocationContext.HubType to HubInvocationContext.Hub (instance)

Provide Feedback

Please let us know if this proposal will work with your scenarios or not.

One question we have is:

• Do users want outgoing hooks (server to client)?
  • Might need a different filter registration method so you can run the filter in a different order
  • Will likely need another method or interface specific to outgoing

[reduckted]

Thanks for following up on this! 😁

Please let us know if this proposal will work with your scenarios or not.

Looks good. I think that proposal will be good for what I want to do. In MVC, I use middleware to setup services for the lifetime of the request with things like the client's address, the details of the user, the culture to use, and so on. A rough example is this:

public async Task Invoke(HttpContext httpContext) {
    var addressManager = httpContext.RequestServices.GetRequiredService<IAddressManager>();
    
    using (var token = addressManager.SetCurrentAddress(GetAddressFromRequest(httpContext.Request))) {
        await _next(httpContext);
    }
    // Note: Disposing the token will "unregister" the current address.
}

The IAddressManager can then be used by any other services during that request to get the address that the request came from. That's useful for things like auditing database changes, and so on, and means the database auditing doesn't need to reference any of the HTTP libraries.

It looks like I can do the same thing with the proposed SignalR filters. Am I correct in saying this would be the equivalent filter code?

public async ValueTask<object> InvokeMethodAsync(HubInvocationContext invocationContext, Func<HubInvocationContext, ValueTask<object>> next)
{
    var addressManager = invocationContext.ServiceProvider.GetRequiredService<IAddressManager>;
    
    using (var token = addressManager.SetCurrentAddress(GetAddressFromInvocation(invocationContext))) {
        await next(invocationContext);
    }
}

Do users want outgoing hooks (server to client)?

Personally, I haven't encountered a need for that.

[davidfowl]

@reduckted Funny, this is what we plan to do with IHttpContextAccessor to make it work properly for SignalR 😄

[JeromeKen]

When is the new version released? :D

[BrennanConroy]

It will be in 5.0.0-preview6