Add More Groups Functionality

[henry-chris]

My apologies for not getting back to you on my implementation for groups. I saw that you started to add support for groups and decided to add thoughts.

1. There seems to be no way to access a single group of clients. This would be helpful to manage lifetime, logging/debugging, telemetry, etc...

I'd propose just adding an accessor such as:

public IReadOnlyCollection<IServerSentEventsClient> GetClients(string groupName) { // Handle errors or something return _groups[groupName].Values.ToArray(); }

2. I'd also propose a way to allow the user to remove a client if they know the groups, too.
   internal void RemoveClient(string[] groupNames /*or list*/, ServerSentEventsClient client) { }

You could even go further and allow removal of several clients for several groups, but I haven't needed that functionality, yet.

Also, I see you used a regular Dictionary for the Group dictionary. Is there any specific reason you opted for that instead of another Concurrent Dictionary? I guess you aren't removing the groups when 0 clients exist, which could be a reason.

[henry-chris]

Adding a couple more here as I just thought of them looking over my code.

It would be good to know if a group was created or not when adding a client.

In my use case, I use Redis for Group pub/sub across nodes. Every time I add a client, I want to start a subscription for that Group if one already doesn't exist for this node. Without knowing if a group was created locally, I might need to subscribe to a Redis channel for every new client (In this case Redis subscriptions are a hash so it won't create a 2nd, but it will make the call to the Redis instance I think). If I can track new groups, then I only need to subscribe when a new group is created, and if I know when one is deleted I can unsubscribe.

So, the easiest way would be to return a boolean if a group was created on addition. On deletion, return whether the group(s) are empty or not after removal. I'm sure that could be improved upon, though.

[tpeczek]

There seems to be no way to access a single group of clients. This would be helpful to manage lifetime, logging/debugging, telemetry, etc...

Sounds like a good idea.

I'd also propose a way to allow the user to remove a client if they know the groups, too.

What do you mean by remove a client here? Remove from group or something else?

It would be good to know if a group was created or not when adding a client.

Sounds interesting. I need to give it a though (to be sure I can expose it nicely in the API) but probably this is something which can be added.

Also, I see you used a regular Dictionary for the Group dictionary. Is there any specific reason you opted for that instead of another Concurrent Dictionary? I guess you aren't removing the groups when 0 clients exist, which could be a reason.

In general, yes. The groups are only being added and in most cases it's not a very frequent thing. Synchronizing this with SemaphoreSlim is a very efficient way (and gives you async in rare cases it will take some considerable time).

[henry-chris]

What do you mean by remove a client here? Remove from group or something else?

Yes. After looking more at your code, it seems that you already do this when an 'OnClientConnected' 'OnClientDisconnected' event happens. Let me think about this more before getting back to you. Sorry that I did not make it clearer the first time.

It would be good to know if a group was created or not when adding a client.

Yes, on this one it matters to me if it's a 'new' group or not. When using Redis, I subscribe a single time for each Group on each server node. If a new group is created I need to create a new Subscription. I also need to know when a group is empty so that I can kill the subscription.

There are other ways of doing this of course. Such as through Redis Transactions since I'm using Redis. However, having access to the information locally on the server node is optimal if possible.

I implemented previously by locking group additions and group removals to the group dictionary. I did not lock on additions/removals to each group though.

In general, yes. The groups are only being added and in most cases it's not a very frequent thing. Synchronizing this with SemaphoreSlim is a very efficient way (and gives you async in rare cases it will take some considerable time).

In my case there may be 50-100 groups active at any time. That number is increasing weekly. These groups may last anywhere from 30 minutes to 6 hours (Think real-time collaboration app).

[tpeczek]

So I would scope this one to following items for now:

• Expose accessor which returns collection of clients in specified group
• Expose an information if group was created while adding a client

---

In my case there may be 50-100 groups active at any time. That number is increasing weekly. These groups may last anywhere from 30 minutes to 6 hours (Think real-time collaboration app).

In such a case I would still consider synchronizing this with SemaphoreSlim. In fact I'm going to explore replacing ConcurrentDictionary for clients with this approach as well (see #24).

[henry-chris]

This may fall under personal usage here, but here's another thing:

All of my events are sourced from a browser. For instance, 10 users are in a session with a document. One of them makes a change to the document. At that time, an HTTP call is made to the server.

That call kicks off an SSE event which propagates the change to all connected clients for that group. Howevever, by default, it also sends the change to the originating browser.

I've basically created a function which sends the event to all clients in the group EXCEPT the originating client. It just attempts to match the originating clientId with with each client on the node and skips the event for that connection.

In your library, I'm not sure how I'd add that.

Maybe with an overload which takes in a list of clientIds to exclude from the message?

[tpeczek]

I've basically created a function which sends the event to all clients in the group EXCEPT the originating client. It just attempts to match the originating clientId with with each client on the node and skips the event for that connection.

This sounds strictly like a business logic which belongs higher than this library. That said, I see a potential way of making implementation of such requirements easier by giving an option to provide predicate to send function, which would be evaluated for every client. So I'm tentatively adding one more item to the scope:

• Add SendEventAsync overload which takes a predicate against IServerSentEventsClient as a parameter

[samprakos]

This looks like what I need...is there example code for setting this up in Startup.cs (e.g. setting up code to execute on OnClientConnected event)?

[tpeczek]

@samprakos

It looks like I have failed miserably at providing any documentation for groups functionality. I will drop a simple snippet here for you later today and create a task for myself to follow with documentation.

[tpeczek]

@samprakos

The basic group management in Startup.cs can look like this:

public class Startup
{
    ...

    public void ConfigureServices(IServiceCollection services)
    {
        services.AddServerSentEvents(options =>
        {
            options.OnClientConnected = async (service, clientConnectedArgs) =>
            {
                // Logic to determine to which group client should be added.
                ...

                await service.AddToGroupAsync(groupName, clientConnectedArgs.Client);
            };
        });

        ...
    }

    ...
}

Later, when you want to send something to group it can be done like this:

await _serverSentEventsService.SendEventAsync(groupName, messageText);