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

Created on 15 Oct 2018  路  6Comments  路  Source: dotnet/aspnetcore

We had https://github.com/aspnet/SignalR/issues/871 and https://github.com/aspnet/SignalR/issues/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.
area-signalr enhancement
Source
picture anurse
👍11 😕1

Most helpful comment

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:

```c#
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:

```c#
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.

```c#
public interface IHubFilter
{
ValueTask InvokeMethodAsync(HubInvocationContext invocationContext, Func> next);
Task OnConnectedAsync(HubCallerContext context, Func next) => next(context); // default interface methods so you don't need to implement these
Task OnDisconnectedAsync(HubCallerContext context, Func 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<GlobalFilter>()), assuming it doesn't need objects from DI.

```c#
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:
```c#
[AttributeUsage(AttributeTargets.Method, AllowMultiple = false, Inherited = true)]
class StreamingMethodAttribute : Attribute
{
}

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

class CustomFilter : IHubFilter
{
async ValueTask InvokeMethodAsync(HubInvocationContext invocationContext, Func> 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

picture BrennanConroy on 9 Apr 2020
👍3

All 6 comments

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

zeinali0 picture zeinali0 on 19 Mar 2019
😕1

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:

```c#
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:

```c#
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.

```c#
public interface IHubFilter
{
ValueTask InvokeMethodAsync(HubInvocationContext invocationContext, Func> next);
Task OnConnectedAsync(HubCallerContext context, Func next) => next(context); // default interface methods so you don't need to implement these
Task OnDisconnectedAsync(HubCallerContext context, Func 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<GlobalFilter>()), assuming it doesn't need objects from DI.

```c#
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:
```c#
[AttributeUsage(AttributeTargets.Method, AllowMultiple = false, Inherited = true)]
class StreamingMethodAttribute : Attribute
{
}

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

class CustomFilter : IHubFilter
{
async ValueTask InvokeMethodAsync(HubInvocationContext invocationContext, Func> 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

BrennanConroy picture BrennanConroy on 9 Apr 2020
👍3

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:

```c#
public async Task Invoke(HttpContext httpContext) {
var addressManager = httpContext.RequestServices.GetRequiredService();

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?

```c#
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.

reduckted picture reduckted on 10 Apr 2020

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

davidfowl picture davidfowl on 10 Apr 2020

When is the new version released? :D

JeromeKen picture JeromeKen on 10 Jun 2020

It will be in 5.0.0-preview6

BrennanConroy picture BrennanConroy on 10 Jun 2020
Was this page helpful?
0 / 5 - 0 ratings

Related issues

UweKeim picture UweKeim  路  3Comments
Kevenvz picture Kevenvz  路  3Comments
mj1856 picture mj1856  路  3Comments
farhadibehnam picture farhadibehnam  路  3Comments
githubgitgit picture githubgitgit  路  3Comments