Fix client results with Hub<T> IHubContext

[BrennanConroy]

David found this while writing an app.

IHubContext<Hub<T>, T> didn't have Single(string connectionId) implemented and fell back to the DIM which throws NotImplementedException.

[halter73]

DIMs! It might not be worth it since we own the type, but given this has happened once, I wonder if we should have a reflection based test a little like CloneSslOptionsClonesAllProperties to make sure we've overridden all the DIMs. Nvm. I now realize it's just the one method.

[halter73]

Can you remind me why it would be a bad idea to update

aspnetcore/src/SignalR/server/Core/src/IHubClients`T.cs

Line 17 in 9a81d79

T Single(string connectionId) => throw new NotImplementedException();

to

T Single(string connectionId) => Client(connectionId);

Had we done that, we never experience this bug.

[BrennanConroy]

The problem with that is that when we try to generate the underlying code to call InvokeAsync, the proxy returned by Client(connectionId) doesn't have the method InvokeAsync.

[halter73]

The problem with that is that when we try to generate the underlying code to call InvokeAsync, the proxy returned by Client(connectionId) doesn't have the method InvokeAsync.

But it still would have worked for non-returning methods, right? I figured we were already using SingleClientProxyWithInvoke<THub> for public T Client(string connectionId), but it looks like we're using SingleClientProxy<THub> instead which is subtly different by not supporting invokes.

Is there a reason for not returning a SingleClientProxyWithInvoke<THub> from public T Client(string connectionId) too? It seems really unfriendly to throw an InvalidOperationException when a developer tries invoke a client-returning method using Client instead of Single.

aspnetcore/src/SignalR/server/Core/src/Internal/TypedClientBuilder.cs

Lines 172 to 179 in 8a4b4de

	var singleClientProxyType = typeof(ISingleClientProxy);
	/*
	if (_proxy is ISingleClientProxy singleClientProxy)
	{
	return ((ISingleClientProxy)_proxy).InvokeAsync<T>(methodName, args, cancellationToken);
	}
	throw new InvalidOperationException("InvokeAsync only works with Single clients.");
	*/

I understand the need for an additional property with a new type for the non-Hub<T> case, but it seems pointless to not support all the same things on both Client and Single in the Hub<T> case. Removing SingleClientProxy<THub> would slightly simplify things too.

[BrennanConroy]

If we did that, we would have an inconsistent experience with strongly-typed vs. non-strongly-typed. Because non-strongly-typed's version of .Client(string connectionId) returns IClientProxy which doesn't have InvokeAsync. And we can't change it to ISingleClientProxy because that caused some issue that I don't remember (I think described on the original PR). We could still return a SingleClientProxyWithInvoke but users would need to know that they can magically cast the IClientProxy to ISingleClientProxy in order to call InvokeAsync with .Client(string connectionId).

[halter73]

We should delete SingleClientProxy<THub> in favor of SingleClientProxyWithInvoke<THub> for simplicity if nothing else. We should also switch to T Single(string connectionId) => Client(connectionId); because that now works in our case. We wouldn't even have to implement it.

I see the inconsistent experience with strongly-typed vs. non-strongly-typed hubs as a complete non-issue. It's FAR better to be "inconsistent" here! We're already being inconsistent by returning the exact same type from both Single() and Client() in the strongly-typed case instead of two different types (namely ISingleClientProxy and IClientProxy) in the default case.

I understand that we can't change IHubClients : IHubClients<IClientProxy> to IHubClients : IHubClients<ISingleClientProxy> in the default case without breaking apps. It's unfortunate, but if we could do that, we wouldn't have had to introduced Single() in the first place.

However, that doesn't mean we should nerf strongly-typed hubs! So what if they are more flexible with their Client property? Single() and Client() return the exact same type for strong typed hubs. It's already different.

[davidfowl]

I don't understand what you're trying to fix @halter73.