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

[halter73]

Background and Motivation

When approving API for #5280 we considered replacing Single with new Caller and new Client members but we were worried it would be source breaking because:

• The suggestion of using new Caller and Client methods ISingleClientProxy would be source breaking with existing IHubClients and IHubCallerClients implementations, decorators etc... if someone replace the IHubContext<> in DI, so we'll stick with Single

However further investigation shows that we 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].

Proposed API

namespace Microsoft.AspNetCore.SignalR;

public interface IHubClients<T>
{
-   T Single(string connectionId) => throw new NotImplementedException();
}


public interface IHubClients : IHubClients<IClientProxy>
{
-   new ISingleClientProxy Single(string connectionId) => throw new NotImplementedException();
+   new ISingleClientProxy Client(string connectionId) => new NonInvokingSingleClientProxy(((IHubClients<IClientProxy>)this).//...
}

public interface IHubCallerClients : IHubCallerClients<IClientProxy>
{
-   new ISingleClientProxy Single(string connectionId) => throw new NotImplementedException();
+   new ISingleClientProxy Client(string connectionId) => new NonInvokingSingleClientProxy(((IHubCallerClients<IClientProxy>)this).//...
+   new ISingleClientProxy Caller => new NonInvokingSingleClientProxy(((IHubCallerClients<IClientProxy>)this).//...
}

Usage Examples

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

Alternative Designs

1. Keep the existing Single(connectionId) method and add new Caller and new Client.
2. Leave it as is with just Single(connectionId) being the only way to get client return results.

Risks

1. This is within reason. Even before, if someone had a custom interface that implemented IHubClients and added a void Single(string whatever), that would be source breaking. I don't think anyone has done this though. I also don't think anyone is doing anything like the following which would still technically be source breaking:

public class MyHub : Hub
{
    public void SourceBreakingMethod()
    {
        var selectedProxy = Clients.Caller;
        selectedProxy = Clients.All; // <--- Cannot implicitly convert type 'Microsoft.AspNetCore.SignalR.IClientProxy' to 'Microsoft.AspNetCore.SignalR.ISingleClientProxy'.
    }
}

But that involves using the returned type of Caller or Client(connectionId) to infer a non-covariant type. The above example is the easiest way I can think of doing this, but I don't think anyone does this in practice.

[ghost]

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

• The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
• The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
• Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

[halter73]

API Review Notes:

• Update docs!
  • Breaking change notes
  • Existing docs
• Verify that this is not binary breaking by first building a dll with a custom IHubCallerClients implementation against .NET 6 and then using the dlls with .NET 7 previews.

Approved assuming no binary break!

namespace Microsoft.AspNetCore.SignalR;

public interface IHubClients<T>
{
-   T Single(string connectionId) => throw new NotImplementedException();
}


public interface IHubClients : IHubClients<IClientProxy>
{
-   new ISingleClientProxy Single(string connectionId) => throw new NotImplementedException();
+   new ISingleClientProxy Client(string connectionId) => new NonInvokingSingleClientProxy(((IHubClients<IClientProxy>)this).//...
}

public interface IHubCallerClients : IHubCallerClients<IClientProxy>
{
-   new ISingleClientProxy Single(string connectionId) => throw new NotImplementedException();
+   new ISingleClientProxy Client(string connectionId) => new NonInvokingSingleClientProxy(((IHubCallerClients<IClientProxy>)this).//...
+   new ISingleClientProxy Caller => new NonInvokingSingleClientProxy(((IHubCallerClients<IClientProxy>)this).//...
}

[davidfowl]

This should be labeled as breaking change and there should be a write up for preview6

[BrennanConroy]

Done: #42326