From bac99a3817688216c8c34643b42a8f248ce60a4a Mon Sep 17 00:00:00 2001 From: Tommy Parnell Date: Tue, 20 Sep 2016 09:49:05 -0400 Subject: [PATCH] handle exceptions from the application better --- src/CompressR.MVC/CompressAttribute.cs | 7 +- src/CompressR.MVC/CompressFactory.cs | 15 +++- src/CompressR.MVC/Constants.cs | 6 +- src/CompressR.MVC/DeflateAttribute.cs | 6 +- src/CompressR.MVC/Extensions.cs | 6 +- src/CompressR.MVC/GzipAttribute.cs | 10 +-- .../CompressionFactoryTests.cs | 89 ++++++++++++++++--- src/CompressR.WebApi/BaseCompressAttribute.cs | 6 +- src/CompressR.WebApi/CompressAttribute.cs | 4 + src/CompressR.WebApi/DeflateAttribute.cs | 4 + src/CompressR.WebApi/GzipAttribute.cs | 4 + 11 files changed, 116 insertions(+), 41 deletions(-) diff --git a/src/CompressR.MVC/CompressAttribute.cs b/src/CompressR.MVC/CompressAttribute.cs index edb68b6..3ae6172 100644 --- a/src/CompressR.MVC/CompressAttribute.cs +++ b/src/CompressR.MVC/CompressAttribute.cs @@ -14,12 +14,7 @@ namespace CompressR.MVC { } - /// - /// Override to compress the content that is generated by - /// an action method. - /// - /// - public override void OnActionExecuting(System.Web.Mvc.ActionExecutingContext filterContext) + public override void OnResultExecuted(ResultExecutedContext filterContext) { CompressFactory.Compress(filterContext, RequireCompression, CompressionLevel); } diff --git a/src/CompressR.MVC/CompressFactory.cs b/src/CompressR.MVC/CompressFactory.cs index d652867..61ab86b 100644 --- a/src/CompressR.MVC/CompressFactory.cs +++ b/src/CompressR.MVC/CompressFactory.cs @@ -9,7 +9,7 @@ namespace CompressR.MVC { public static class CompressFactory { - public static void Compress(System.Web.Mvc.ActionExecutingContext filterContext, bool requireCompression, string compression, CompressionLevel compressLevel = CompressionLevel.Optimal) + public static void Compress(System.Web.Mvc.ResultExecutedContext filterContext, bool requireCompression, string compression, CompressionLevel compressLevel = CompressionLevel.Optimal) { var context = filterContext.RequestContext.HttpContext; var compressionAccepted = context.Request.Headers.Get(Constants.AcceptEncoding)?.Split(',').Trim().Any(a => string.Equals(a, compression, StringComparison.OrdinalIgnoreCase)) ?? false; @@ -28,8 +28,12 @@ namespace CompressR.MVC HandleCompression(compression, filterContext, compressLevel); } - public static void Compress(System.Web.Mvc.ActionExecutingContext filterContext, bool requireCompression, CompressionLevel compressLevel = CompressionLevel.Optimal) + public static void Compress(System.Web.Mvc.ResultExecutedContext filterContext, bool requireCompression, CompressionLevel compressLevel = CompressionLevel.Optimal) { + if(filterContext.Exception != null && !filterContext.ExceptionHandled) + { + return; + } var context = filterContext.RequestContext.HttpContext; var compressionAlgorithm = context.Request.Headers.Get(Constants.AcceptEncoding)?.Split(',').Trim().Intersect(Constants.Compressors, StringComparer.OrdinalIgnoreCase)?.FirstOrDefault(); if(!string.IsNullOrWhiteSpace(compressionAlgorithm)) @@ -42,9 +46,14 @@ namespace CompressR.MVC } } - private static void HandleCompression(string compression, System.Web.Mvc.ActionExecutingContext filterContext, CompressionLevel compressLevel = CompressionLevel.Optimal) + private static void HandleCompression(string compression, System.Web.Mvc.ResultExecutedContext filterContext, CompressionLevel compressLevel = CompressionLevel.Optimal) { var context = filterContext.RequestContext.HttpContext; + HandleCompression(compression, filterContext.RequestContext.HttpContext, compressLevel); + } + + private static void HandleCompression(string compression, System.Web.HttpContextBase context, CompressionLevel compressLevel) + { switch(compression) { case Constants.Gzip: diff --git a/src/CompressR.MVC/Constants.cs b/src/CompressR.MVC/Constants.cs index d30a342..81529f7 100644 --- a/src/CompressR.MVC/Constants.cs +++ b/src/CompressR.MVC/Constants.cs @@ -1,8 +1,4 @@ -using System; -using System.Collections.Generic; -using System.Text; - -namespace CompressR.MVC +namespace CompressR.MVC { public struct Constants { diff --git a/src/CompressR.MVC/DeflateAttribute.cs b/src/CompressR.MVC/DeflateAttribute.cs index b78fce0..d2d5bf9 100644 --- a/src/CompressR.MVC/DeflateAttribute.cs +++ b/src/CompressR.MVC/DeflateAttribute.cs @@ -1,6 +1,7 @@ using System; using System.IO.Compression; using System.Linq; +using System.Web.Mvc; namespace CompressR.MVC { @@ -17,9 +18,10 @@ namespace CompressR.MVC /// an action method. /// /// - public override void OnActionExecuting(System.Web.Mvc.ActionExecutingContext filterContext) + + public override void OnResultExecuted(ResultExecutedContext filterContext) { - CompressFactory.Compress(filterContext, RequireCompression, Constants.Deflate, CompressionLevel); + CompressFactory.Compress(filterContext, RequireCompression, CompressionLevel); } } } \ No newline at end of file diff --git a/src/CompressR.MVC/Extensions.cs b/src/CompressR.MVC/Extensions.cs index 2507797..8985223 100644 --- a/src/CompressR.MVC/Extensions.cs +++ b/src/CompressR.MVC/Extensions.cs @@ -1,6 +1,4 @@ -using System; -using System.Collections.Generic; -using System.Text; +using System.Collections.Generic; namespace CompressR.MVC { @@ -8,7 +6,7 @@ namespace CompressR.MVC { public static IEnumerable Trim(this IEnumerable arr) { - foreach (var item in arr) + foreach(var item in arr) { yield return item.Trim(); } diff --git a/src/CompressR.MVC/GzipAttribute.cs b/src/CompressR.MVC/GzipAttribute.cs index a1e3ae2..229a2b4 100644 --- a/src/CompressR.MVC/GzipAttribute.cs +++ b/src/CompressR.MVC/GzipAttribute.cs @@ -1,6 +1,7 @@ using System; using System.IO.Compression; using System.Linq; +using System.Web.Mvc; namespace CompressR.MVC { @@ -11,14 +12,9 @@ namespace CompressR.MVC { } - /// - /// Override to compress the content that is generated by - /// an action method. - /// - /// - public override void OnActionExecuting(System.Web.Mvc.ActionExecutingContext filterContext) + public override void OnResultExecuted(ResultExecutedContext filterContext) { - CompressFactory.Compress(filterContext, RequireCompression, Constants.Gzip, CompressionLevel); + CompressFactory.Compress(filterContext, RequireCompression, CompressionLevel); } } } \ No newline at end of file diff --git a/src/CompressR.MVCUnitTests/CompressionFactoryTests.cs b/src/CompressR.MVCUnitTests/CompressionFactoryTests.cs index 3f800e7..db3b958 100644 --- a/src/CompressR.MVCUnitTests/CompressionFactoryTests.cs +++ b/src/CompressR.MVCUnitTests/CompressionFactoryTests.cs @@ -22,15 +22,15 @@ namespace CompressR.MVCUnitTests request.Setup(a => a.Headers).Returns(new System.Collections.Specialized.NameValueCollection() { }); var httpContext = new Moq.Mock(); httpContext.Setup(a => a.Request).Returns(request.Object); - Assert.Throws(typeof(CompressR.Exceptions.CompressRException), () => { - - CompressR.MVC.CompressFactory.Compress(new System.Web.Mvc.ActionExecutingContext() + Assert.Throws(typeof(CompressR.Exceptions.CompressRException), () => + { + CompressR.MVC.CompressFactory.Compress(new System.Web.Mvc.ResultExecutedContext() { HttpContext = httpContext.Object }, true); }); - } + [Fact] public void ShouldNotThrowButAlsoNotCompressIfFalse() { @@ -41,17 +41,16 @@ namespace CompressR.MVCUnitTests var response = new Mock(); httpContext.Setup(a => a.Response).Returns(response.Object); var contextObject = httpContext.Object; - CompressR.MVC.CompressFactory.Compress(new System.Web.Mvc.ActionExecutingContext() + CompressR.MVC.CompressFactory.Compress(new System.Web.Mvc.ResultExecutedContext() { HttpContext = contextObject }, false); Assert.Null(contextObject.Response.Filter); - } + [Fact] public void ShouldGzipWhenRequested() { - var request = new Mock(); request.Setup(a => a.Headers).Returns(new System.Collections.Specialized.NameValueCollection() { ["Accept-Encoding"] = "gzip" }); var requestContextBase = new Mock(); @@ -69,7 +68,7 @@ namespace CompressR.MVCUnitTests responseContext.Setup(a => a.Response).Returns(response.Object); requestContextBase.Setup(a => a.Response).Returns(response.Object); - var httpContext = new ActionExecutingContext() + var httpContext = new ResultExecutedContext() { RequestContext = new System.Web.Routing.RequestContext() { @@ -83,12 +82,77 @@ namespace CompressR.MVCUnitTests //var contextObject = httpContext; CompressR.MVC.CompressFactory.Compress(httpContext, false); Assert.IsType(httpContext.RequestContext.HttpContext.Response.Filter); - } + + [Fact] + public void ShouldNotGzipWhenExceptionNotHandled() + { + var request = new Mock(); + request.Setup(a => a.Headers).Returns(new System.Collections.Specialized.NameValueCollection() { ["Accept-Encoding"] = "gzip" }); + var requestContextBase = new Mock(); + requestContextBase.Setup(a => a.Request).Returns(request.Object); + var requestContext = new Mock(); + requestContext.Setup(a => a.HttpContext).Returns(requestContextBase.Object); + var cachePolicy = new Mock(); + cachePolicy.Setup(a => a.VaryByHeaders).Returns(new HttpCacheVaryByHeaders()); + //var httpContext = new Moq.Mock(); + var response = new Mock(); + response.Setup(a => a.Headers).Returns(new System.Collections.Specialized.NameValueCollection()); + response.SetupProperty(a => a.Filter, new MemoryStream(Encoding.UTF8.GetBytes("awesome")) { }); + response.Setup(a => a.Cache).Returns(cachePolicy.Object); + var responseContext = new Mock(); + responseContext.Setup(a => a.Response).Returns(response.Object); + + requestContextBase.Setup(a => a.Response).Returns(response.Object); + var httpContext = new ResultExecutedContext() + { + RequestContext = new System.Web.Routing.RequestContext() + { + HttpContext = requestContextBase.Object + }, + Exception = new Exception(), + ExceptionHandled = false + }; + CompressR.MVC.CompressFactory.Compress(httpContext, false); + Assert.IsNotType(httpContext.RequestContext.HttpContext.Response.Filter); + } + + [Fact] + public void ShouldGzipWhenExceptionHandled() + { + var request = new Mock(); + request.Setup(a => a.Headers).Returns(new System.Collections.Specialized.NameValueCollection() { ["Accept-Encoding"] = "gzip" }); + var requestContextBase = new Mock(); + requestContextBase.Setup(a => a.Request).Returns(request.Object); + var requestContext = new Mock(); + requestContext.Setup(a => a.HttpContext).Returns(requestContextBase.Object); + var cachePolicy = new Mock(); + cachePolicy.Setup(a => a.VaryByHeaders).Returns(new HttpCacheVaryByHeaders()); + //var httpContext = new Moq.Mock(); + var response = new Mock(); + response.Setup(a => a.Headers).Returns(new System.Collections.Specialized.NameValueCollection()); + response.SetupProperty(a => a.Filter, new MemoryStream(Encoding.UTF8.GetBytes("awesome")) { }); + response.Setup(a => a.Cache).Returns(cachePolicy.Object); + var responseContext = new Mock(); + responseContext.Setup(a => a.Response).Returns(response.Object); + + requestContextBase.Setup(a => a.Response).Returns(response.Object); + var httpContext = new ResultExecutedContext() + { + RequestContext = new System.Web.Routing.RequestContext() + { + HttpContext = requestContextBase.Object + }, + Exception = new Exception(), + ExceptionHandled = true + }; + CompressR.MVC.CompressFactory.Compress(httpContext, false); + Assert.IsType(httpContext.RequestContext.HttpContext.Response.Filter); + } + [Fact] public void ShouldDeflateWhenRequested() { - var request = new Mock(); request.Setup(a => a.Headers).Returns(new System.Collections.Specialized.NameValueCollection() { ["Accept-Encoding"] = "deflate" }); var requestContextBase = new Mock(); @@ -106,7 +170,7 @@ namespace CompressR.MVCUnitTests responseContext.Setup(a => a.Response).Returns(response.Object); requestContextBase.Setup(a => a.Response).Returns(response.Object); - var httpContext = new ActionExecutingContext() + var httpContext = new ResultExecutedContext() { RequestContext = new System.Web.Routing.RequestContext() { @@ -120,7 +184,6 @@ namespace CompressR.MVCUnitTests //var contextObject = httpContext; CompressR.MVC.CompressFactory.Compress(httpContext, false); Assert.IsType(httpContext.RequestContext.HttpContext.Response.Filter); - } } -} +} \ No newline at end of file diff --git a/src/CompressR.WebApi/BaseCompressAttribute.cs b/src/CompressR.WebApi/BaseCompressAttribute.cs index e0c22e1..29afd14 100644 --- a/src/CompressR.WebApi/BaseCompressAttribute.cs +++ b/src/CompressR.WebApi/BaseCompressAttribute.cs @@ -23,7 +23,11 @@ namespace CompressR.WebApi protected async Task CompressAction(HttpActionExecutedContext actionExecutedContext, params string[] compressors) { - if(actionExecutedContext.Response.Content == null) + if(actionExecutedContext?.Exception != null) + { + return; + } + if(actionExecutedContext?.Response?.Content == null) { return; } diff --git a/src/CompressR.WebApi/CompressAttribute.cs b/src/CompressR.WebApi/CompressAttribute.cs index 41f1970..cd395d4 100644 --- a/src/CompressR.WebApi/CompressAttribute.cs +++ b/src/CompressR.WebApi/CompressAttribute.cs @@ -16,6 +16,10 @@ namespace CompressR.WebApi public override Task OnActionExecutedAsync(HttpActionExecutedContext actionExecutedContext, CancellationToken cancellationToken) { + if(cancellationToken.IsCancellationRequested) + { + return Task.FromResult(0); + } return base.CompressAction(actionExecutedContext, Constants.Compressors); } } diff --git a/src/CompressR.WebApi/DeflateAttribute.cs b/src/CompressR.WebApi/DeflateAttribute.cs index 2b4949e..e8f67a5 100644 --- a/src/CompressR.WebApi/DeflateAttribute.cs +++ b/src/CompressR.WebApi/DeflateAttribute.cs @@ -24,6 +24,10 @@ namespace CompressR.WebApi public override Task OnActionExecutedAsync(HttpActionExecutedContext actionExecutedContext, CancellationToken cancellationToken) { + if(cancellationToken.IsCancellationRequested) + { + return Task.FromResult(0); + } return base.CompressAction(actionExecutedContext, Constants.Deflate); } } diff --git a/src/CompressR.WebApi/GzipAttribute.cs b/src/CompressR.WebApi/GzipAttribute.cs index b79457e..9d88899 100644 --- a/src/CompressR.WebApi/GzipAttribute.cs +++ b/src/CompressR.WebApi/GzipAttribute.cs @@ -23,6 +23,10 @@ namespace CompressR.WebApi public override Task OnActionExecutedAsync(HttpActionExecutedContext actionExecutedContext, CancellationToken cancellationToken) { + if(cancellationToken.IsCancellationRequested) + { + return Task.FromResult(0); + } return base.CompressAction(actionExecutedContext, Constants.Gzip); } }