bryceg / Owin.WebSocket

A library for handling OWIN WebSocket connections
MIT License
47 stars 28 forks source link

Created a Fleck implementation of Owin.WebSocket #20

Open tdupont750 opened 8 years ago

NVentimiglia commented 8 years ago

First I want to complement you on integrating Fleck, really smooth move.

Second, I feel this implementation would be better if it exposed some of the functionality that Fleck provides. For instance the _connectionInfo and _connection are private and not exposed to the implementing class. This makes caching connections and sending to everyone / groups difficult. I would strongly suggest exposing these. Maybe there is a reason you did not do this ?

Third, I downloaded the sample and exposed _connection myself. This resulted in some slight code smell in my OnMessage method.

 [WebSocketRoute("/ws")]
    public class FleckWebSocket : FleckWebSocketConnection
    {
        public static List<IWebSocketConnection> connections = new List<IWebSocketConnection>(); 

        public override Task OnMessageReceived(ArraySegment<byte> message, WebSocketMessageType type)
        {
            //Handle the message from the client
            //Example of JSON serialization with the client
            var json = Encoding.UTF8.GetString(message.Array, message.Offset, message.Count);

            var toSend = Encoding.UTF8.GetBytes("ECHO " + json+" From "+this._connectionInfo.Id+" total "+connections.Count);

            foreach (var connection in connections)
            {
                //Send to All
                connection.Send("ECHO " + json + " From " + this._connectionInfo.Id + " total " + connections.Count);
            }

            //Echo Send
            return SendText(toSend, true);
        }

        public override void OnOpen()
        {
            connections.Add(_connection);
            base.OnOpen();
        }

        public override void OnClose(WebSocketCloseStatus? closeStatus, string closeStatusDescription)
        {
            connections.Remove(_connection);
            base.OnClose(closeStatus, closeStatusDescription);
        }
    }
}

Notice how I am sending the sending client two messages. Once via the connection list and again in the return echo. I guess the obvious answer would be to use one or the other. That said, is there any reason I should still be using the inherited Send method over the send method in Fleck's IWebsocketConnection ? If not, should we even include the inherited Send Method as it only adds confusion ?

tdupont750 commented 8 years ago

Hey Nicholas,

Sorry for the delay in responding. The whole thing is setup to be a plug and play replacement for a Fleck server.

All of the connection data is actually exposed explicitly via interface: https://github.com/tdupont750/Owin.WebSocket/blob/master/src/Owin.WebSocket.Fleck/FleckWebSocketConnection.cs#L129

Here is a demo of how it could be used to keep a list of connections without using a static list: http://www.tomdupont.net/2015/11/net-websocket-libraries.html

Did that answer your question?

Thanks, Tom

bryceg commented 8 years ago

Hey guys, I'll try and get to this sometime this week. Sorry for the delay.

matneyx commented 8 years ago

:+1: I'm interested in how this turns out... My work uses Owin a lot, and recently has been pushing to use websockets with Fleck, though we haven't really implemented them.

tdupont750 commented 8 years ago

@bryceg If this project is not a good fit the the repository, then I can create a separate one. Please let me know!