Skip to content

Commit

Permalink
Code Review to WebSocket
Browse files Browse the repository at this point in the history
  • Loading branch information
@geoperez
geoperez committed May 23, 2016
1 parent 49cf089 commit bc50940
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 64 deletions.
70 changes: 35 additions & 35 deletions Unosquare.Labs.EmbedIO/Extensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,8 +205,8 @@ public static bool JsonResponse(this HttpListenerContext context, string json)
context.Response.OutputStream.Write(buffer, 0, buffer.Length);

return true;
}

}

/// <summary>
/// Parses the json as a given type from the request body.
/// Please note the underlying input stream is not rewindable.
Expand All @@ -221,16 +221,16 @@ public static T ParseJson<T>(this HttpListenerContext context)
return requestBody == null ? null : JsonConvert.DeserializeObject<T>(requestBody);
}

/// <summary>
/// Parses the json as a given type from the request body string.
/// </summary>
/// <typeparam name="T"></typeparam>
/// <param name="requestBody">The request body.</param>
/// <returns></returns>
/// <summary>
/// Parses the json as a given type from the request body string.
/// </summary>
/// <typeparam name="T"></typeparam>
/// <param name="requestBody">The request body.</param>
/// <returns></returns>
public static T ParseJson<T>(this string requestBody)
where T : class
{
return requestBody == null ? null : JsonConvert.DeserializeObject<T>(requestBody);
where T : class
{
return requestBody == null ? null : JsonConvert.DeserializeObject<T>(requestBody);
}

/// <summary>
Expand Down Expand Up @@ -295,32 +295,32 @@ public static Dictionary<string, string> RequestFormData(this HttpListenerContex
return ParseFormData(stringData);
}
}
}

/// <summary>
/// Returns dictionary from Request POST data
/// </summary>
/// <param name="requestBody">The request body.</param>
/// <returns></returns>
}

/// <summary>
/// Returns dictionary from Request POST data
/// </summary>
/// <param name="requestBody">The request body.</param>
/// <returns></returns>
public static Dictionary<string, string> RequestFormData(this string requestBody)
{
return ParseFormData(requestBody);
}

/// <summary>
/// Parses the form data given the request body string.
/// </summary>
/// <param name="requestBody">The request body.</param>
/// <returns></returns>
private static Dictionary<string, string> ParseFormData(string requestBody)
{
if (string.IsNullOrWhiteSpace(requestBody)) return null;

return requestBody.Split('&')
.ToDictionary(c => c.Split('=')[0],
c => WebUtility.UrlDecode(c.Split('=')[1]));
}

return ParseFormData(requestBody);
}

/// <summary>
/// Parses the form data given the request body string.
/// </summary>
/// <param name="requestBody">The request body.</param>
/// <returns></returns>
private static Dictionary<string, string> ParseFormData(string requestBody)
{
if (string.IsNullOrWhiteSpace(requestBody)) return null;

return requestBody.Split('&')
.ToDictionary(c => c.Split('=')[0],
c => WebUtility.UrlDecode(c.Split('=')[1]));
}

/// <summary>
/// Compresses the specified buffer using the G-Zip compression algorithm.
/// </summary>
Expand Down
53 changes: 24 additions & 29 deletions Unosquare.Labs.EmbedIO/Modules/WebSocketsModule.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Net;
using System.Net.WebSockets;
using System.Threading;
using System.Threading.Tasks;

/// <summary>
/// A WebSockets module confirming to RFC 6455
Expand Down Expand Up @@ -38,15 +39,11 @@ public WebSocketsModule()
var path = context.RequestPath();
// match the request path
if (_serverMap.ContainsKey(path))
{
// Accept the WebSocket -- this is a blocking method until the WebSocketCloses
_serverMap[path].AcceptWebSocket(server, context);
return true;
}
return false;
if (!_serverMap.ContainsKey(path)) return false;
// Accept the WebSocket -- this is a blocking method until the WebSocketCloses
_serverMap[path].AcceptWebSocket(server, context);
return true;
});
}

Expand All @@ -56,10 +53,7 @@ public WebSocketsModule()
/// <value>
/// The name.
/// </value>
public override string Name
{
get { return "WebSockets Module"; }
}
public override string Name => "WebSockets Module";

/// <summary>
/// Registers the web sockets server given a WebSocketsServer Type.
Expand All @@ -80,14 +74,14 @@ public void RegisterWebSocketsServer<T>()
public void RegisterWebSocketsServer(Type socketType)
{
if (socketType == null)
throw new ArgumentException("Argument 'socketType' cannot be null", "socketType");
throw new ArgumentException("Argument 'socketType' cannot be null", nameof(socketType));

var attribute =
socketType.GetCustomAttributes(typeof(WebSocketHandlerAttribute), true).FirstOrDefault() as
WebSocketHandlerAttribute;

if (attribute == null)
throw new ArgumentException("Argument 'socketType' needs a WebSocketHandlerAttribute", "socketType");
throw new ArgumentException("Argument 'socketType' needs a WebSocketHandlerAttribute", nameof(socketType));

this._serverMap[attribute.Path] = (WebSocketsServer)Activator.CreateInstance(socketType);
}
Expand All @@ -102,7 +96,7 @@ public void RegisterWebSocketsServer<T>(string path)
where T : WebSocketsServer, new()
{
if (string.IsNullOrWhiteSpace(path))
throw new ArgumentException("Argument 'path' cannot be null", "path");
throw new ArgumentException("Argument 'path' cannot be null", nameof(path));

this._serverMap[path] = Activator.CreateInstance<T>();
}
Expand All @@ -118,9 +112,9 @@ public void RegisterWebSocketsServer<T>(string path, T server)
where T : WebSocketsServer
{
if (string.IsNullOrWhiteSpace(path))
throw new ArgumentException("Argument 'path' cannot be null", "path");
throw new ArgumentException("Argument 'path' cannot be null", nameof(path));
if (server == null)
throw new ArgumentException("Argument 'server' cannot be null", "server");
throw new ArgumentException("Argument 'server' cannot be null", nameof(server));

this._serverMap[path] = server;
}
Expand All @@ -140,7 +134,7 @@ public sealed class WebSocketHandlerAttribute : Attribute
/// <exception cref="System.ArgumentException">The argument 'paths' must be specified.</exception>
public WebSocketHandlerAttribute(string path)
{
if (path == null || string.IsNullOrWhiteSpace(path))
if (string.IsNullOrWhiteSpace(path))
throw new ArgumentException("The argument 'path' must be specified.");

this.Path = path;
Expand Down Expand Up @@ -218,15 +212,15 @@ protected WebSocketsServer()
/// </summary>
private void RunConnectionWatchdog()
{
var t = new Thread(() =>
var t = new Thread(async () =>
{
while (_isDisposing == false)
{
if (_isDisposing == false)
CollectDisconnected();
// TODO: make this sleep configurable.
Thread.Sleep(30 * 1000);
await Task.Delay(30*1000);
}
})
{
Expand Down Expand Up @@ -303,7 +297,7 @@ public void AcceptWebSocket(WebServer server, HttpListenerContext context)
// close the connection if message excceeds max length
webSocketContext.WebSocket.CloseAsync(
WebSocketCloseStatus.MessageTooBig,
string.Format("Message too big. Maximum is {0} bytes.", _maximumMessageSize),
$"Message too big. Maximum is {_maximumMessageSize} bytes.",
CancellationToken.None).GetAwaiter().GetResult();

// exit the loop; we're done
Expand Down Expand Up @@ -335,8 +329,7 @@ public void AcceptWebSocket(WebServer server, HttpListenerContext context)
/// <param name="webSocketContext">The web socket context.</param>
private void RemoveWebSocket(WebSocketContext webSocketContext)
{
if (webSocketContext.WebSocket != null)
webSocketContext.WebSocket.Dispose();
webSocketContext.WebSocket?.Dispose();

lock (_syncRoot)
{
Expand All @@ -350,7 +343,7 @@ private void RemoveWebSocket(WebSocketContext webSocketContext)
/// Removes and disposes all disconnected sockets
/// </summary>
/// <returns></returns>
private int CollectDisconnected()
private void CollectDisconnected()
{
var collectedCount = 0;
lock (_syncRoot)
Expand All @@ -367,11 +360,8 @@ private int CollectDisconnected()
}
}

if (this.WebServer != null)
this.WebServer.Log.DebugFormat("{0} - Collected {1} sockets. WebSocket Count: {2}", this.ServerName,
collectedCount, this.WebSockets.Count);

return collectedCount;
this.WebServer?.Log.DebugFormat("{0} - Collected {1} sockets. WebSocket Count: {2}", this.ServerName,
collectedCount, this.WebSockets.Count);
}

/// <summary>
Expand Down Expand Up @@ -405,6 +395,7 @@ protected virtual async void Send(WebSocketContext webSocket, byte[] payload)
try
{
if (payload == null) payload = new byte[0];

await
webSocket.WebSocket.SendAsync(new ArraySegment<byte>(payload), WebSocketMessageType.Binary, true,
CancellationToken.None);
Expand Down Expand Up @@ -451,6 +442,10 @@ protected virtual async void Close(WebSocketContext webSocket)
webSocket.WebSocket.CloseAsync(WebSocketCloseStatus.NormalClosure, string.Empty,
CancellationToken.None);
}
catch (Exception ex)
{
WebServer.Log.Error(ex);
}
finally
{
RemoveWebSocket(webSocket);
Expand Down

0 comments on commit bc50940

Please sign in to comment.