From 6b6841365c2a346c4e25dfed2a57a153c3cfb28e Mon Sep 17 00:00:00 2001 From: Julian Verdurmen <304NotModified@users.noreply.github.com> Date: Fri, 10 Feb 2017 19:56:53 +0100 Subject: [PATCH 1/2] support for empty 'QueryStringKeys' for ${all-querystrings{aspnet-request-querystring} --- .../AspNetRequestQueryStringLayoutRenderer.cs | 67 ++++++++++++------- 1 file changed, 44 insertions(+), 23 deletions(-) diff --git a/NLog.Web.AspNetCore/LayoutRenderers/AspNetRequestQueryStringLayoutRenderer.cs b/NLog.Web.AspNetCore/LayoutRenderers/AspNetRequestQueryStringLayoutRenderer.cs index 67fe127c..36bc9eb0 100644 --- a/NLog.Web.AspNetCore/LayoutRenderers/AspNetRequestQueryStringLayoutRenderer.cs +++ b/NLog.Web.AspNetCore/LayoutRenderers/AspNetRequestQueryStringLayoutRenderer.cs @@ -10,6 +10,7 @@ using NLog.Config; using NLog.Web.Enums; using System; +using System.Linq; using NLog.Web.Internal; namespace NLog.Web.LayoutRenderers @@ -29,6 +30,7 @@ public class AspNetQueryStringLayoutRenderer : AspNetLayoutRendererBase { /// /// List Query Strings' Key to be rendered from Request. + /// If empty, then render all querystrings /// public List QueryStringKeys { get; set; } @@ -46,41 +48,60 @@ public class AspNetQueryStringLayoutRenderer : AspNetLayoutRendererBase protected override void DoAppend(StringBuilder builder, LogEventInfo logEvent) { var httpRequest = HttpContextAccessor?.HttpContext?.TryGetRequest(); + if (httpRequest == null) + return; - if (this.QueryStringKeys?.Count > 0) - { - if (httpRequest == null) - return; - - var includeArrayEndBraces = false; - var firstItem = true; + var includeArrayEndBraces = false; + var firstItem = true; + + var allQueryStrings = this.QueryStringKeys == null || this.QueryStringKeys.Count == 0; + var queryStringKeys = this.QueryStringKeys; #if !NETSTANDARD_1plus - var queryStrings = httpRequest.QueryString; + var queryStrings = httpRequest.QueryString; + if (allQueryStrings) + { + queryStringKeys = new List(queryStrings.Keys.Count); + + foreach (var key in queryStrings.Keys) + { + if (key != null) + { + queryStringKeys.Add(key.ToString()); + } + } + } #else - var queryStrings = httpRequest.Query; + var queryStrings = httpRequest.Query; + if (allQueryStrings) + { + queryStringKeys = queryStrings.Keys.ToList(); + } #endif - if (queryStrings?.Count > 0) + + + + + if (queryStrings?.Count > 0) + { + foreach (var configuredKey in queryStringKeys) { - foreach (var configuredKey in this.QueryStringKeys) - { - // This platoform specific code is to prevent an unncessary .ToString call otherwise. + // This platoform specific code is to prevent an unncessary .ToString call otherwise. #if !NETSTANDARD_1plus - var value = queryStrings[configuredKey]; + var value = queryStrings[configuredKey]; #else - var value = queryStrings[configuredKey].ToString(); + var value = queryStrings[configuredKey].ToString(); #endif - if (!String.IsNullOrEmpty(value)) - { - this.AppendKeyAndValue(builder, configuredKey, value, firstItem, ref includeArrayEndBraces); - firstItem = false; - } + if (!String.IsNullOrEmpty(value)) + { + this.AppendKeyAndValue(builder, configuredKey, value, firstItem, ref includeArrayEndBraces); + firstItem = false; } } - - if (includeArrayEndBraces) - builder.Append(GlobalConstants.jsonArrayEndBraces); } + + if (includeArrayEndBraces) + builder.Append(GlobalConstants.jsonArrayEndBraces); } /// From 0b44bfa94a249656f13a9b18c058f12dba8cbf54 Mon Sep 17 00:00:00 2001 From: Julian Verdurmen <304NotModified@users.noreply.github.com> Date: Fri, 10 Feb 2017 20:38:42 +0100 Subject: [PATCH 2/2] update tests --- .../AspNetQueryStringLayoutRendererTests.cs | 212 ++++++++++-------- .../AspNetRequestQueryStringLayoutRenderer.cs | 20 +- 2 files changed, 134 insertions(+), 98 deletions(-) diff --git a/NLog.Web.AspNetCore.Tests/LayoutRenderers/AspNetQueryStringLayoutRendererTests.cs b/NLog.Web.AspNetCore.Tests/LayoutRenderers/AspNetQueryStringLayoutRendererTests.cs index 80b3f992..7283072c 100644 --- a/NLog.Web.AspNetCore.Tests/LayoutRenderers/AspNetQueryStringLayoutRendererTests.cs +++ b/NLog.Web.AspNetCore.Tests/LayoutRenderers/AspNetQueryStringLayoutRendererTests.cs @@ -1,5 +1,4 @@ -#if !NETSTANDARD_1plus -//todo nsubstitute + using System; using System.Collections.Generic; @@ -13,70 +12,23 @@ using Microsoft.Extensions.Primitives; using HttpContextBase = Microsoft.AspNetCore.Http.HttpContext; using HttpSessionState = Microsoft.AspNetCore.Http.ISession; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.Internal; #endif using NLog.Web.LayoutRenderers; using NSubstitute; using NLog.Web.Enums; using Xunit; -using System.Collections.Specialized; -using System.Reflection; -using NLog.Targets; -using NLog.Layouts; + namespace NLog.Web.Tests.LayoutRenderers { public class AspNetQueryStringLayoutRendererTests : TestInvolvingAspNetHttpContext { - public AspNetQueryStringLayoutRendererTests() : base() - { - this.SetUp(); - } - - public void SetUp() - { - //auto load won't work yet (in DNX), so use - SetupFakeSession(); - } - - protected override void CleanUp() - { - Session.Clear(); - } - - private HttpSessionState Session - { - get - { -#if NETSTANDARD_1plus - return HttpContext.Session; -#else - return HttpContext.Current.Session; -#endif - } - } - - public void SetupFakeSession() - { - var sessionContainer = new HttpSessionStateContainer("id", new SessionStateItemCollection(), - new HttpStaticObjectsCollection(), 10, true, - HttpCookieMode.AutoDetect, - SessionStateMode.InProc, false); - - HttpContext.Items["AspSession"] = typeof(HttpSessionState).GetConstructor( - BindingFlags.NonPublic | BindingFlags.Instance, - null, CallingConventions.Standard, - new[] { typeof(HttpSessionStateContainer) }, - null) - .Invoke(new object[] { sessionContainer }); - } - [Fact] public void NullKeyRendersEmptyString() { - var httpContext = Substitute.For(); - - var renderer = new AspNetQueryStringLayoutRenderer(); - renderer.HttpContextAccessor = new FakeHttpContextAccessor(httpContext); + var renderer = CreateAndMockRenderer(); renderer.QueryStringKeys = null; string result = renderer.Render(new LogEventInfo()); @@ -87,13 +39,8 @@ public void NullKeyRendersEmptyString() [Fact] public void KeyNotFoundRendersEmptyString_Flat_Formatting() { - var httpContext = Substitute.For(); - var namedClollection = new NameValueCollection(); - namedClollection.Add("Id", "1"); - httpContext.Request.QueryString.Returns(namedClollection); + var renderer = CreateAndMockRenderer(CreateTuple("Id", "1")); - var renderer = new AspNetQueryStringLayoutRenderer(); - renderer.HttpContextAccessor = new FakeHttpContextAccessor(httpContext); renderer.QueryStringKeys = new List { "key" }; renderer.OutputFormat = AspNetRequestLayoutOutputFormat.Flat; @@ -105,13 +52,8 @@ public void KeyNotFoundRendersEmptyString_Flat_Formatting() [Fact] public void KeyNotFoundRendersEmptyString_Json_Formatting() { - var httpContext = Substitute.For(); - var namedClollection = new NameValueCollection(); - namedClollection.Add("Id", "1"); - httpContext.Request.QueryString.Returns(namedClollection); + var renderer = CreateAndMockRenderer(CreateTuple("Id", "1")); - var renderer = new AspNetQueryStringLayoutRenderer(); - renderer.HttpContextAccessor = new FakeHttpContextAccessor(httpContext); renderer.QueryStringKeys = new List { "key" }; renderer.OutputFormat = AspNetRequestLayoutOutputFormat.Json; @@ -120,17 +62,16 @@ public void KeyNotFoundRendersEmptyString_Json_Formatting() Assert.Empty(result); } + + + [Fact] public void KeyFoundRendersValue_QueryString_Single_Item_Flat_Formatting() { var expectedResult = "Id:1"; - var httpContext = Substitute.For(); - var namedClollection = new NameValueCollection(); - namedClollection.Add("Id", "1"); - httpContext.Request.QueryString.Returns(namedClollection); - var renderer = new AspNetQueryStringLayoutRenderer(); - renderer.HttpContextAccessor = new FakeHttpContextAccessor(httpContext); + var renderer = CreateAndMockRenderer(CreateTuple("Id", "1")); + renderer.QueryStringKeys = new List { "Id" }; renderer.OutputFormat = AspNetRequestLayoutOutputFormat.Flat; @@ -143,13 +84,9 @@ public void KeyFoundRendersValue_QueryString_Single_Item_Flat_Formatting() public void KeyFoundRendersValue_QueryString_Single_Item_Json_Formatting() { var expectedResult = "[{\"Id\":\"1\"}]"; - var httpContext = Substitute.For(); - var namedClollection = new NameValueCollection(); - namedClollection.Add("Id", "1"); - httpContext.Request.QueryString.Returns(namedClollection); - var renderer = new AspNetQueryStringLayoutRenderer(); - renderer.HttpContextAccessor = new FakeHttpContextAccessor(httpContext); + var renderer = CreateAndMockRenderer(CreateTuple("Id", "1")); + renderer.QueryStringKeys = new List { "Id" }; renderer.OutputFormat = AspNetRequestLayoutOutputFormat.Json; @@ -162,14 +99,9 @@ public void KeyFoundRendersValue_QueryString_Single_Item_Json_Formatting() public void KeyFoundRendersValue_QueryString_Multiple_Item_Flat_Formatting() { var expectedResult = "Id:1," + Environment.NewLine + "Id2:2"; - var httpContext = Substitute.For(); - var namedClollection = new NameValueCollection(); - namedClollection.Add("Id", "1"); - namedClollection.Add("Id2", "2"); - httpContext.Request.QueryString.Returns(namedClollection); - var renderer = new AspNetQueryStringLayoutRenderer(); - renderer.HttpContextAccessor = new FakeHttpContextAccessor(httpContext); + var renderer = CreateAndMockRenderer(CreateTuple("Id", "1"), CreateTuple("Id2", "2")); + renderer.QueryStringKeys = new List { "Id", "Id2" }; renderer.OutputFormat = AspNetRequestLayoutOutputFormat.Flat; @@ -178,25 +110,119 @@ public void KeyFoundRendersValue_QueryString_Multiple_Item_Flat_Formatting() Assert.Equal(expectedResult, result); } + [Fact] + public void EmptyProperyShouldListAll() + { + var expectedResult = "Id:1," + Environment.NewLine + "Id2:2"; + + var renderer = CreateAndMockRenderer(CreateTuple("Id", "1"), CreateTuple("Id2", "2")); + + renderer.QueryStringKeys = new List { }; + renderer.OutputFormat = AspNetRequestLayoutOutputFormat.Flat; + + string result = renderer.Render(new LogEventInfo()); + + Assert.Equal(expectedResult, result); + } + + [Fact] + public void NullProperyShouldListAll() + { + var expectedResult = "Id:1," + Environment.NewLine + "Id2:2"; + + var renderer = CreateAndMockRenderer(CreateTuple("Id", "1"), CreateTuple("Id2", "2")); + + renderer.QueryStringKeys = null; + renderer.OutputFormat = AspNetRequestLayoutOutputFormat.Flat; + + string result = renderer.Render(new LogEventInfo()); + + Assert.Equal(expectedResult, result); + } + [Fact] + public void MultipleValuesForOneKeyShouldWork() + { + var expectedResult = "Id:1,2,3"; + + var renderer = CreateAndMockRenderer(CreateTuple("Id", "1", "2", "3")); + + renderer.QueryStringKeys = null; + renderer.OutputFormat = AspNetRequestLayoutOutputFormat.Flat; + + string result = renderer.Render(new LogEventInfo()); + + Assert.Equal(expectedResult, result); + } + [Fact] public void KeyFoundRendersValue_QueryString_Multiple_Item_Json_Formatting() { var expectedResult = "[" + "{\"Id\":\"1\"}," + Environment.NewLine + "{\"Id2\":\"2\"}" + "]"; - var httpContext = Substitute.For(); - var namedClollection = new NameValueCollection(); - namedClollection.Add("Id", "1"); - namedClollection.Add("Id2", "2"); - httpContext.Request.QueryString.Returns(namedClollection); - var renderer = new AspNetQueryStringLayoutRenderer(); - renderer.HttpContextAccessor = new FakeHttpContextAccessor(httpContext); + var renderer = CreateAndMockRenderer(CreateTuple("Id", "1"), CreateTuple("Id2", "2")); + renderer.QueryStringKeys = new List { "Id", "Id2" }; renderer.OutputFormat = AspNetRequestLayoutOutputFormat.Json; string result = renderer.Render(new LogEventInfo()); Assert.Equal(expectedResult, result); - } + } + + + /// + /// Create tuple with 1 or more values (with 1 key) + /// + /// + /// + /// + private static Tuple CreateTuple(string key, params string[] values) + { + return new Tuple(key, values); + } + + + private static AspNetQueryStringLayoutRenderer CreateAndMockRenderer(params Tuple[] values) + { + var renderer = new AspNetQueryStringLayoutRenderer(); + +#if !NETSTANDARD_1plus + + var httpContext = Substitute.For(); + var namedClollection = new NameValueCollection(); + foreach (var tuple in values) + { + foreach (var value in tuple.Item2) + { + namedClollection.Add(tuple.Item1, value); + } + + } + + httpContext.Request.QueryString.Returns(namedClollection); + + renderer.HttpContextAccessor = new FakeHttpContextAccessor(httpContext); +#else + var httpContext = Substitute.For(); + var dict = new Dictionary(); + foreach (var tuple in values) + { + dict.Add(tuple.Item1, new StringValues(tuple.Item2)); + + } + IQueryCollection querystringValues = new QueryCollection(dict); + + httpContext.Request.Query.Returns(querystringValues); + + renderer.HttpContextAccessor = new FakeHttpContextAccessor(httpContext); +#endif + + return renderer; + + + + } + + } } -#endif \ No newline at end of file diff --git a/NLog.Web.AspNetCore/LayoutRenderers/AspNetRequestQueryStringLayoutRenderer.cs b/NLog.Web.AspNetCore/LayoutRenderers/AspNetRequestQueryStringLayoutRenderer.cs index 36bc9eb0..46ea41e5 100644 --- a/NLog.Web.AspNetCore/LayoutRenderers/AspNetRequestQueryStringLayoutRenderer.cs +++ b/NLog.Web.AspNetCore/LayoutRenderers/AspNetRequestQueryStringLayoutRenderer.cs @@ -51,14 +51,18 @@ protected override void DoAppend(StringBuilder builder, LogEventInfo logEvent) if (httpRequest == null) return; - var includeArrayEndBraces = false; - var firstItem = true; + var allQueryStrings = this.QueryStringKeys == null || this.QueryStringKeys.Count == 0; var queryStringKeys = this.QueryStringKeys; #if !NETSTANDARD_1plus var queryStrings = httpRequest.QueryString; + + if (queryStrings == null) + return; + + if (allQueryStrings) { queryStringKeys = new List(queryStrings.Keys.Count); @@ -73,6 +77,10 @@ protected override void DoAppend(StringBuilder builder, LogEventInfo logEvent) } #else var queryStrings = httpRequest.Query; + + if (queryStrings == null) + return; + if (allQueryStrings) { queryStringKeys = queryStrings.Keys.ToList(); @@ -80,10 +88,12 @@ protected override void DoAppend(StringBuilder builder, LogEventInfo logEvent) #endif - - - if (queryStrings?.Count > 0) + var includeArrayEndBraces = false; + + if (queryStrings.Count > 0) { + var firstItem = true; + foreach (var configuredKey in queryStringKeys) { // This platoform specific code is to prevent an unncessary .ToString call otherwise.