Skip to content

[SignalR] Replace Single(connectionId) with Client(connectionId) and Caller #42236

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

Merged
merged 3 commits into from
Jun 21, 2022

Conversation

halter73
Copy link
Member

We recently discovered can add new Caller and Client members without breaking existing IHubClients and IHubCallerClients implementations unless they're being used for client results which never worked before .NET 7 making the change both non-binary and non-source breaking[1].

Before

public class MyHub : Hub
{
    public async Task BroadcastCallerResult()
    {
        var num = await Clients.Single(Context.ConnectionId).InvokeAsync<int>("GetNum");
        await Clients.Others.SendAsync("BroadcastNum", num);
    }
}

After

public class MyHub : Hub
{
    public async Task BroadcastCallerResult()
    {
        var num = await Clients.Caller.InvokeAsync<int>("GetNum");
        await Clients.Others.SendAsync("BroadcastNum", num);
    }
}

Fixes #41994

  1. TODO Still manually test with a custom IHubClients implementation built against .NET 6.

@halter73 halter73 added the area-signalr Includes: SignalR clients and servers label Jun 16, 2022
@halter73 halter73 requested a review from BrennanConroy as a code owner June 16, 2022 16:26
halter73 added 2 commits June 16, 2022 20:50
- Yes this means there are probably a fair number of other tests like it.
- We definitely will need a breaking change announcement if we merge this.
@halter73 halter73 requested a review from a team as a code owner June 17, 2022 03:52
@BrennanConroy
Copy link
Member

  1. Still manually test with a custom IHubClients implementation built against .NET 6.

Made a 6.0 app and published it, then ran the published app against a custom 7.0 runtime with these changes. Nothing broke as expected. Below is the rough code used.

var builder = WebApplication.CreateBuilder(args);
builder.Services.AddSingleton(typeof(IHubContext<>), typeof(MyHubContext<>));
builder.Services.AddSignalR();
var app = builder.Build();

app.MapGet("/custom", (IHubContext<MyHub> c, [FromQuery] string id) =>
{
    c.Clients.Client(id).SendAsync("ReceiveMessage", "test");
});

app.MapHub<MyHub>("/hub");

app.Run();

public class MyHub : Hub { }
public class MyHubContext<THub> : IHubContext<THub> where THub : Hub
{
    private readonly HubLifetimeManager<THub> _lm;
    public MyHubContext(HubLifetimeManager<THub> lm)
    {
        _lm = lm;
    }

    public IHubClients Clients => new HubClient<THub>(_lm);

    public IGroupManager Groups => throw new NotImplementedException();
}

internal class HubClient<THub> : IHubClients where THub : Hub
{
    private readonly HubLifetimeManager<THub> _lm;
    public HubClient(HubLifetimeManager<THub> lm)
    {
        _lm = lm;
    }

    public IClientProxy Client(string connectionId)
    {
        return new SingleClientProxy(_lm, connectionId);
    }

    // all the interface methods with NotImplementedException, removed for brevity

    internal sealed class SingleClientProxy : IClientProxy
    {
        private readonly string _connectionId;
        private readonly HubLifetimeManager<THub> _lifetimeManager;

        public SingleClientProxy(HubLifetimeManager<THub> lifetimeManager, string connectionId)
        {
            _lifetimeManager = lifetimeManager;
            _connectionId = connectionId;
        }

        public Task SendCoreAsync(string method, object?[] args, CancellationToken cancellationToken = default)
        {
            return _lifetimeManager.SendConnectionAsync(_connectionId, method, args, cancellationToken);
        }
    }
}

@BrennanConroy BrennanConroy merged commit d4fccce into main Jun 21, 2022
@BrennanConroy BrennanConroy deleted the halter73/less-single branch June 21, 2022 15:39
@ghost ghost added this to the 7.0-preview6 milestone Jun 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-signalr Includes: SignalR clients and servers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace Single(connectionId) with Client(connectionId) and Caller
2 participants