Skip to content

Commit

Permalink
Code Review
Browse files Browse the repository at this point in the history
  • Loading branch information
@geoperez
geoperez committed Sep 9, 2016
1 parent c245db4 commit 2b9d073
Show file tree
Hide file tree
Showing 7 changed files with 54 additions and 62 deletions.
2 changes: 1 addition & 1 deletion Unosquare.Labs.EmbedIO/Log/SimpleConsoleLog.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ private static void WriteLine(ConsoleColor color, string format, params object[]

format = dateTimeString + "\t" + format;

ThreadPool.QueueUserWorkItem((context) =>
ThreadPool.QueueUserWorkItem(context =>
{
var current = Console.ForegroundColor;
Console.ForegroundColor = color;
Expand Down
3 changes: 1 addition & 2 deletions Unosquare.Labs.EmbedIO/Modules/CorsModule.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ public CorsModule(string origins = Constants.CorsWildcard, string headers = Cons
if (methods == null) throw new ArgumentException(Constants.ArgumentNullExceptionMessage, nameof(methods));

var validOrigins = origins.ToLower().Split(Constants.CommaSplitChar, StringSplitOptions.RemoveEmptyEntries).Select(x => x.Trim());
var validHeaders = headers.ToLower().Split(Constants.CommaSplitChar, StringSplitOptions.RemoveEmptyEntries).Select(x => x.Trim());
var validMethods = methods.ToLower().Split(Constants.CommaSplitChar, StringSplitOptions.RemoveEmptyEntries).Select(x => x.Trim());

AddHandler(ModuleMap.AnyPath, HttpVerbs.Any, (server, context) =>
Expand All @@ -47,7 +46,7 @@ public CorsModule(string origins = Constants.CorsWildcard, string headers = Cons
var currentHeader = context.RequestHeader(Constants.HeaderAccessControlRequestHeaders);
var currentMethod = context.RequestHeader(Constants.HeaderAccessControlRequestMethod);
if (String.IsNullOrWhiteSpace(currentOrigin) && context.Request.IsLocal) return false;
if (string.IsNullOrWhiteSpace(currentOrigin) && context.Request.IsLocal) return false;
if (origins != Constants.CorsWildcard)
{
Expand Down
17 changes: 6 additions & 11 deletions Unosquare.Labs.EmbedIO/Modules/LocalSessionModule.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ private void FixupSessionCookie(HttpListenerContext context)
if (nameValue.Length == 2 && nameValue[0].Equals(SessionCookieName))
{
var sessionIdValue = nameValue[1].Trim();

if (this.Sessions.ContainsKey(sessionIdValue))
{
context.Request.Cookies[SessionCookieName].Value = sessionIdValue;
Expand Down Expand Up @@ -94,7 +95,7 @@ public LocalSessionModule()
if (requestCookie == null)
{
// create the session if sesison not available on the request
// create the session if session not available on the request
var sessionCookie = CreateSession();
context.Response.SetCookie(sessionCookie);
context.Request.Cookies.Add(sessionCookie);
Expand All @@ -108,15 +109,15 @@ public LocalSessionModule()
context.Request.Cookies[SessionCookieName].Value = sessionCookie.Value;
server.Log.DebugFormat("Updated session identifier to '{0}'", sessionCookie.Value);
}
else if (this.Sessions.ContainsKey(context.Request.Cookies[SessionCookieName].Value) == true)
else if (this.Sessions.ContainsKey(context.Request.Cookies[SessionCookieName].Value))
{
// If it does exist in the request, check if we're tracking it
var requestSessionId = context.Request.Cookies[SessionCookieName].Value;
this.Sessions[requestSessionId].LastActivity = DateTime.Now;
server.Log.DebugFormat("Session Identified '{0}'", requestSessionId);
}
// Always returns false because we need it to handle the rest fo the modules
// Always returns false because we need it to handle the rest for the modules
return false;
});

Expand All @@ -128,10 +129,7 @@ public LocalSessionModule()
/// <value>
/// The sessions.
/// </value>
public ConcurrentDictionary<string, SessionInfo> Sessions
{
get { return this.m_Sessions; }
}
public ConcurrentDictionary<string, SessionInfo> Sessions => this.m_Sessions;

/// <summary>
/// Gets a session object for the given server context.
Expand Down Expand Up @@ -177,9 +175,6 @@ public SessionInfo GetSession(WebSocketContext context)
/// <value>
/// The name.
/// </value>
public override string Name
{
get { return "Local Session Module"; }
}
public override string Name => "Local Session Module";
}
}
25 changes: 11 additions & 14 deletions Unosquare.Labs.EmbedIO/Modules/StaticFilesModule.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@
/// </summary>
public class StaticFilesModule : WebModuleBase
{
private const int chuckSize = 256*1024;
/// <summary>
/// The chuck size for sending files
/// </summary>
private const int ChuckSize = 256*1024;

private readonly Dictionary<string, string> m_VirtualPaths =
new Dictionary<string, string>(StringComparer.InvariantCultureIgnoreCase);
Expand Down Expand Up @@ -91,8 +94,7 @@ public class StaticFilesModule : WebModuleBase
/// <value>
/// The virtual paths.
/// </value>
public ReadOnlyDictionary<string, string> VirtualPaths => new ReadOnlyDictionary<string, string>(m_VirtualPaths)
;
public ReadOnlyDictionary<string, string> VirtualPaths => new ReadOnlyDictionary<string, string>(m_VirtualPaths);

/// <summary>
/// Gets the name of this module.
Expand All @@ -116,7 +118,7 @@ public void ClearRamCache()
/// <value>
/// The ram cache.
/// </value>
private ConcurrentDictionary<string, RamCacheEntry> RamCache { get; set; }
private ConcurrentDictionary<string, RamCacheEntry> RamCache { get; }

/// <summary>
/// Represents a RAM Cache dictionary entry
Expand Down Expand Up @@ -259,7 +261,7 @@ private bool HandleGet(HttpListenerContext context, WebServer server, bool sendB

var lowerByteIndex = 0;
var upperByteIndex = 0;
var byteLength = (long) 0;
long byteLength;
var isPartial = usingPartial &&
CalculateRange(partialHeader, fileSize, out lowerByteIndex, out upperByteIndex);

Expand Down Expand Up @@ -320,13 +322,13 @@ private bool HandleGet(HttpListenerContext context, WebServer server, bool sendB
private static void WriteToOutputStream(HttpListenerContext context, long byteLength, Stream buffer,
int lowerByteIndex)
{
var streamBuffer = new byte[chuckSize];
var streamBuffer = new byte[ChuckSize];
var sendData = 0;
var readBufferSize = chuckSize;
var readBufferSize = ChuckSize;

while (true)
{
if (sendData + chuckSize > byteLength) readBufferSize = (int) (byteLength - sendData);
if (sendData + ChuckSize > byteLength) readBufferSize = (int) (byteLength - sendData);

buffer.Seek(lowerByteIndex + sendData, SeekOrigin.Begin);
var read = buffer.Read(streamBuffer, 0, readBufferSize);
Expand Down Expand Up @@ -432,12 +434,7 @@ private bool ExistsLocalPath(string urlPath, ref string localPath)
if (string.IsNullOrWhiteSpace(DefaultExtension) == false && DefaultExtension.StartsWith(".") &&
File.Exists(localPath) == false)
{
var newPath = localPath + DefaultExtension;

if (File.Exists(newPath))
{
localPath = newPath;
}
localPath += DefaultExtension;
}

if (File.Exists(localPath)) return true;
Expand Down
31 changes: 15 additions & 16 deletions Unosquare.Labs.EmbedIO/Modules/WebApiModule.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,17 +231,17 @@ private string NormalizeWildcardPath(HttpVerbs verb, HttpListenerContext context
if (_delegateMap.ContainsKey(path) == false)
return null;

if (_delegateMap[path].ContainsKey(verb) == false) // TODO: Fix Any Verb
if (_delegateMap[path].ContainsKey(verb)) return path;

var originalPath = context.RequestPath();

if (_delegateMap.ContainsKey(originalPath) &&
_delegateMap[originalPath].ContainsKey(verb))
{
var originalPath = context.RequestPath();
if (_delegateMap.ContainsKey(originalPath) &&
_delegateMap[originalPath].ContainsKey(verb))
{
path = originalPath;
}
else
return null;
path = originalPath;
}
else
return null;

return path;
}
Expand Down Expand Up @@ -328,14 +328,13 @@ public void RegisterController(Type controllerType, Func<object> controllerFacto

foreach (var path in attribute.Paths)
{
var delegatePath = new Dictionary<HttpVerbs, Tuple<Func<object>, MethodInfo>>();

if (_delegateMap.ContainsKey(path))
delegatePath = _delegateMap[path]; // update
else
_delegateMap.Add(path, delegatePath); // add
if (_delegateMap.ContainsKey(path) == false)
{
_delegateMap.Add(path, new Dictionary<HttpVerbs, Tuple<Func<object>, MethodInfo>>()); // add
}

var delegatePair = new Tuple<Func<object>, MethodInfo>(controllerFactory, method);

if (_delegateMap[path].ContainsKey(attribute.Verb))
_delegateMap[path][attribute.Verb] = delegatePair; // update
else
Expand Down Expand Up @@ -410,7 +409,7 @@ public abstract class WebApiController
/// <summary>
/// Initializes a new instance of the <see cref="WebApiController"/> class.
/// </summary>
public WebApiController()
protected WebApiController()
{
// placeholder
}
Expand Down
19 changes: 9 additions & 10 deletions Unosquare.Labs.EmbedIO/Modules/WebSocketsModule.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,9 @@ public class WebSocketsModule : WebModuleBase
/// Initialize WebSocket module
/// </summary>
public WebSocketsModule()
: base()
{
this.AddHandler(ModuleMap.AnyPath, HttpVerbs.Any, (server, context) =>
{
// check if it is a WebSocket request (this only works with Win8 and Windows 2012)
if (context.Request.IsWebSocketRequest == false)
return false;
Expand Down Expand Up @@ -63,7 +61,7 @@ public WebSocketsModule()
public void RegisterWebSocketsServer<T>()
where T : WebSocketsServer, new()
{
RegisterWebSocketsServer(typeof(T));
RegisterWebSocketsServer(typeof (T));
}

/// <summary>
Expand All @@ -77,13 +75,14 @@ public void RegisterWebSocketsServer(Type socketType)
throw new ArgumentException("Argument 'socketType' cannot be null", nameof(socketType));

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

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

this._serverMap[attribute.Path] = (WebSocketsServer)Activator.CreateInstance(socketType);
this._serverMap[attribute.Path] = (WebSocketsServer) Activator.CreateInstance(socketType);
}

/// <summary>
Expand Down Expand Up @@ -124,7 +123,7 @@ public void RegisterWebSocketsServer<T>(string path, T server)
/// Decorate methods within controllers with this attribute in order to make them callable from the Web API Module
/// Method Must match the WebServerModule.
/// </summary>
[AttributeUsage(AttributeTargets.Class, AllowMultiple = false)]
[AttributeUsage(AttributeTargets.Class)]
public sealed class WebSocketHandlerAttribute : Attribute
{
/// <summary>
Expand All @@ -146,7 +145,7 @@ public WebSocketHandlerAttribute(string path)
/// <value>
/// The paths.
/// </value>
public string Path { get; private set; }
public string Path { get; }
}

/// <summary>
Expand Down Expand Up @@ -193,7 +192,7 @@ protected WebSocketsServer(bool enableConnectionWatchdog, int maxMessageSize)
{
this._enableDisconnectedSocketColletion = enableConnectionWatchdog;
this._maximumMessageSize = maxMessageSize;

RunConnectionWatchdog();
}

Expand Down Expand Up @@ -268,7 +267,7 @@ public void AcceptWebSocket(WebServer server, HttpListenerContext context)
// define a receive buffer
var receiveBuffer = new byte[receiveBufferSize];
// define a dynamic buffer that holds multi-part receptions
var receivedMessage = new List<byte>(receiveBuffer.Length * 2);
var receivedMessage = new List<byte>(receiveBuffer.Length*2);

// poll the WebSockets connections for reception
while (webSocketContext.WebSocket.State == WebSocketState.Open)
Expand Down
19 changes: 11 additions & 8 deletions Unosquare.Labs.EmbedIO/WebServer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,8 @@ public WebServer(string[] urlPrefixes, ILog log, RoutingStrategy routingStrategy
foreach (var prefix in urlPrefixes)
{
var urlPrefix = prefix.Clone() as string;
if (urlPrefix == null) continue;

if (urlPrefix.EndsWith("/") == false) urlPrefix = urlPrefix + "/";
urlPrefix = urlPrefix.ToLowerInvariant();

Expand All @@ -190,7 +192,7 @@ public WebServer(string[] urlPrefixes, ILog log, RoutingStrategy routingStrategy

this.Log.Info("Finished Loading Web Server.");
}

/// <summary>
/// Gets the module registered for the given type.
/// Returns null if no module matches the given type.
Expand Down Expand Up @@ -228,8 +230,10 @@ public void RegisterModule(IWebModule module)
module.Server = this;
this._modules.Add(module);

if (module is ISessionWebModule)
this.SessionModule = module as ISessionWebModule;
var webModule = module as ISessionWebModule;

if (webModule != null)
this.SessionModule = webModule;
}
else
{
Expand Down Expand Up @@ -412,9 +416,8 @@ public bool ProcessRequest(HttpListenerContext context)
clientSocketTask.Wait(ct);
var clientSocket = clientSocketTask.Result;
var clientTask =
Task.Factory.StartNew((context) => HandleClientRequest(context as HttpListenerContext, app),
clientSocket, ct);
Task.Factory.StartNew(context => HandleClientRequest(context as HttpListenerContext, app),
clientSocket, ct);
}
catch (OperationCanceledException)
{
Expand All @@ -441,14 +444,14 @@ public void Run()

this.Log.Info("Started HTTP Listener");

ThreadPool.QueueUserWorkItem((o) =>
ThreadPool.QueueUserWorkItem(o =>
{
while (this.Listener != null && this.Listener.IsListening)
{
try
{
// Asynchronously queue a response by using a thread from the thread pool
ThreadPool.QueueUserWorkItem((contextState) =>
ThreadPool.QueueUserWorkItem(contextState =>
{
// get a reference to the HTTP Listener Context
var context = contextState as HttpListenerContext;
Expand Down

0 comments on commit 2b9d073

Please sign in to comment.