SignalR: Give IHubContext more flexibility to deal with generics

[karrocon]

Summary

Right now, it is only possible to inject generic-type IHubContext objects. Since Clients and Groups do not require the generic type we can abstract the IHubContext even further so that IHubContext<> and IHubContext<,> implement IHubContext (with no generic type).

Motivation and goals

This change gives the flexibility needed to perform generic casting from some IHubContext to IHubContext while being able to use the Clients and/or Groups properties. Can this be considered for a later major update?

In scope

You may have a unique notification service dispatching messages to some clients based on the specific hub type + connection id.

Out of scope

N/A.

Risks / unknowns

None that I can think of.

Examples

We need some codebase:

[HubName(EventStreamNames.Dashboard)]
public class MyHub : Hub { }

[AttributeUsage(AttributeTargets.Class, AllowMultiple = false)]
public class HubNameAttribute : Attribute
{
    public string HubName { get; set; }

    public HubNameAttribute(string hubName)
    {
        HubName = hubName;
    }
}

public interface IHubContextResolver
{
    IHubContext GetHubContext(string hubName);
}

internal class HubContextResolver : IHubContextResolver
{
    private readonly IServiceProvider serviceProvider;

    public HubContextResolver(IServiceProvider serviceProvider)
    {
        this.serviceProvider = serviceProvider;
    }

    public IHubContext GetHubContext(string hubName)
    {
        var hubType = AppDomain.CurrentDomain
            .GetAssemblies()
            .SelectMany(a => a.GetTypes())
            .Where(t => t.IsAssignableTo(typeof(Hub)))
            .FirstOrDefault(t => t.GetCustomAttribute<HubNameAttribute>(true)?.HubName == hubName);

        var hubContextType = typeof(IHubContext<>).MakeGenericType(hubType);

        return serviceProvider.GetService(hubContextType) as IHubContext;
    }
}

Then, it can be used by a general-purpose handler.

public class NotificationHandler : MessageHandler<SignalRNotification>
{
    private readonly IHubContextResolver _hubContextResolver;

    public NotificationHandledHandler(IHubContextResolver hubContextResolver)
    {
        _hubContextResolver = hubContextResolver;
    }

    public override async Task Handle(SignalRNotification request, CancellationToken cancellationToken)
    {
        var hub = _hubContextResolver.GetHubContext(request.Hub);
        if (hub != null)
        {
            await hub.Clients.Client(request.ConnectionId).SendCoreAsync(request.Method, new[] { request.Data });
        }
    }
}

[davidfowl]

Turning this into a more formal API proposal:

public interface IHubContext
{
   IHubClients Clients { get; }
   IGroupManager Groups { get; }
}

public interface IHubContext<THub> : IHubContext
{
}

// potentially also
public interface IHubContext<THub, T>
        where THub : Hub<T>
        where T : class
        : IHubContext
{
}

I'm not sure if this is a breaking API change.

[davidfowl]

@karrocon curious, in what scenario are you dynamically looking up the hub based on an incoming message.

[karrocon]

We have a hub per aggregate which is served by a single Notification service using Azure SignalR. Our clients can subscribe using the Notification service to a specific hub and other services can publish an event (including hubName/connectionId) using EventHub. This event will be handled by the Notification service to deliver a message back to the client.

Thanks for contacting us.
We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We will evaluate the request when we are planning the work for the next milestone. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

[HappyNomad]

@davidfowl You can also consider adding out as mentioned in #30542.

[halter73]

Since Clients and Groups do not require the generic type we can abstract the IHubContext even further so that IHubContext<> and IHubContext<,> implement IHubContext (with no generic type).

It will be interesting trying to reconcile this with IHubClients which is the opposite. IHubClients implements IHubClients<> (specifically IHubClients<IClientProxy>). Do we need another version of IHubClients (like an IHubClients<TClient> which hides IHubClients<IClientProxy> with the new operator)? That seems like it would be confusing.

[halter73]

Talked a bit @BrennanConroy and determined this shouldn't be a big issue because IHubContext<THub, T> does not implement IHubContext<THub>. So both our IHubContext<THub, T> and IHubContext<THub> can implement the IHubContext.

This would mean implemntations of IHubContext<THub, T> would need to implement both the existing IHubClients<T> Clients property and a new IHubClients<IClientProxy> Clients property. Should we

1. Make a breaking change to require IHubContext<THub, T> implementations implement the IHubClients<IClientProxy> Clients property?
2. Have a reflection-based default interface implementation for IHubClients<IClientProxy>?
3. Not have IHubContext<THub, T> implement IHubContext at all?
4. Close this issue as wontfix.

[pranavkm]

We had a discussion about this, we think going with op# 3 might be a good way forward to unblock this scenario for 6.0. We could consider doing (1) or (2) at a later point if we hear more.