search

dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.55k stars 10.05k forks source link

SignalR Support taking over the generation of connection Id #13443

Open vicancy opened 5 years ago

vicancy commented 5 years ago

Background

For Azure SignalR, we'd like a more semantic connection id, so that some info can be carried through ConnectionId for Azure SignalR to make some decisions more efficiently.

Proposed solution

Provide some way to hook the generation of connection id which is now private for HttpConnectionManager https://github.com/aspnet/AspNetCore/blob/460f9b63e9db9f4a3a542fcdc4204e29ca90f123/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionManager.cs#L83

davidfowl commented 5 years ago

cc @bradygaster @anurse We want to look at this for 3.1

Kahbazi commented 5 years ago

@halter73 Can I do this task?

I was thinking to add this interface

public interface IConnectionIdGenerator
{
    ValueTask<string> GenerateIdAsync(HttpContext HttpContext)
}

What do you think?

davidfowl commented 5 years ago

Does it really need to be asynchronous?

Kahbazi commented 5 years ago

I cannot think of any good use case right now :confused:

Kahbazi commented 5 years ago

@davidfowl Should I provide two separate methods for ConnectionId and ConnectionToken? https://github.com/aspnet/AspNetCore/blob/master/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionManager.cs#L84-L92

analogrelay commented 5 years ago

@davidfowl I think we need a little more context here, and I think you and @vicancy may have it. I'll start a discussion offline.

vicancy commented 5 years ago

Oops, noticed that it moved out from 3.1.0 😢 Is it possible to make the progress faster?

davidfowl commented 5 years ago

I’ll discuss with @anurse tomorrow

Kahbazi commented 5 years ago

@vicancy Do you also want to create your own ConnectionToken? Do you need the methods to be async or non-async?

BrennanConroy commented 5 years ago

Sorry @Kahbazi we're still figuring out what we want here.

Once we've settled on what we want and if it's not critical to get it in ASAP we can see if you're still interested in contributing!

analogrelay commented 4 years ago

Pre-triage notes: This also feels like it falls in the bucket of things we proposed for 5.0 but don't have capacity for now.

davidfowl commented 4 years ago

@anurse Can we push it to a later preview? The cost is trivial and we can potentially get @vicancy to make the change.

davidfowl commented 3 years ago

We never did this did we? @vicancy should we still pursue this for 6.0?