Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skip allocating RouteData when only need to lookup single value #891

Merged
merged 1 commit into from
Nov 26, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions src/Shared/LayoutRenderers/AspNetMvcActionRenderer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,12 @@ protected override void DoAppend(StringBuilder builder, LogEventInfo logEvent)
var context = HttpContextAccessor.HttpContext;

#if !ASP_NET_CORE
var controller = RouteTable.Routes?.GetRouteData(context)?.Values[key]?.ToString();
object actionValue = null;
RouteTable.Routes?.GetRouteData(context)?.Values?.TryGetValue(key, out actionValue);
#else
var controller = context?.GetRouteData()?.Values?[key]?.ToString();
var actionValue = context?.GetRouteValue(key);
#endif
builder.Append(controller);
builder.Append(actionValue?.ToString());
}
}
}
8 changes: 5 additions & 3 deletions src/Shared/LayoutRenderers/AspNetMvcControllerRenderer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,14 @@ protected override void DoAppend(StringBuilder builder, LogEventInfo logEvent)

var context = HttpContextAccessor.HttpContext;


#if !ASP_NET_CORE
var controller = RouteTable.Routes?.GetRouteData(context)?.Values[key]?.ToString();
object controllerValue = null;
RouteTable.Routes?.GetRouteData(context)?.Values?.TryGetValue(key, out controllerValue);
#else
var controller = context?.GetRouteData()?.Values?[key]?.ToString();
var controllerValue = context?.GetRouteValue(key);
#endif
builder.Append(controller);
builder.Append(controllerValue?.ToString());
}
}
}
75 changes: 51 additions & 24 deletions src/Shared/LayoutRenderers/AspNetRequestRouteParametersRenderer.cs
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
using System.Text;
using NLog.LayoutRenderers;
using System.Collections.Generic;
using System.Linq;
#if !ASP_NET_CORE
using System.Web.Routing;
#else
using Microsoft.AspNetCore.Routing;
#endif
using NLog.LayoutRenderers;

namespace NLog.Web.LayoutRenderers
{
Expand All @@ -28,7 +27,13 @@ public class AspNetRequestRouteParametersRenderer : AspNetLayoutMultiValueRender
/// List Route Parameter' Key to be rendered from Request.
/// If empty, then render all parameters
/// </summary>
public List<string> RouteParameterKeys { get; set; }
public List<string> RouteParameterKeys { get => Items; set => Items = value; }

/// <summary>
/// List Route Parameter' Key to be rendered from Request.
/// If empty, then render all parameters
/// </summary>
public List<string> Items { get; set; }

/// <inheritdoc/>
protected override void DoAppend(StringBuilder builder, LogEventInfo logEvent)
Expand All @@ -39,41 +44,63 @@ protected override void DoAppend(StringBuilder builder, LogEventInfo logEvent)
return;
}

var routeParameterKeys = Items;

if (routeParameterKeys?.Count == 1 && !string.IsNullOrEmpty(routeParameterKeys[0]))
{
#if !ASP_NET_CORE
RouteValueDictionary routeParameters = RouteTable.Routes?.GetRouteData(context)?.Values;
object routeValue = null;
RouteTable.Routes?.GetRouteData(context)?.Values?.TryGetValue(routeParameterKeys[0], out routeValue);
#else
RouteValueDictionary routeParameters = context.GetRouteData()?.Values;
var routeValue = context?.GetRouteValue(routeParameterKeys[0]);
#endif
if (routeParameters == null || routeParameters.Count == 0)
{
return;
}
var routeStringValue = routeValue?.ToString();
if (!string.IsNullOrEmpty(routeStringValue))
{

var routeParameterKeys = RouteParameterKeys;
bool printAllRouteParameter = routeParameterKeys == null || routeParameterKeys.Count == 0;
if (printAllRouteParameter)
var pair = new[] { new KeyValuePair<string, string>(routeParameterKeys[0], routeStringValue) };
SerializePairs(pair, builder, logEvent);
}
}
else
{
routeParameterKeys = routeParameters.Keys.ToList();
#if !ASP_NET_CORE
RouteValueDictionary routeParameters = RouteTable.Routes?.GetRouteData(context)?.Values;
#else
RouteValueDictionary routeParameters = context.GetRouteData()?.Values;
#endif
if (routeParameters?.Count > 0)
{
var pairs = GetPairs(routeParameters, routeParameterKeys);
SerializePairs(pairs, builder, logEvent);
}
}

IEnumerable<KeyValuePair<string, string>> pairs = GetPairs(routeParameters, routeParameterKeys);
SerializePairs(pairs, builder, logEvent);
}

private static IEnumerable<KeyValuePair<string, string>> GetPairs(RouteValueDictionary routeParameters, List<string> routeParameterKeys)
{
foreach (string key in routeParameterKeys)
if (routeParameterKeys?.Count > 0)
{
// This platform specific code is to prevent an unncessary .ToString call otherwise.
if (!routeParameters.TryGetValue(key, out object objValue))
foreach (var routeKey in routeParameterKeys)
{
continue;
if (routeParameters.TryGetValue(routeKey, out var routeValue))
{
string value = routeValue?.ToString();
if (!string.IsNullOrEmpty(value))
yield return new KeyValuePair<string, string>(routeKey, value);
}
}

string value = objValue?.ToString();
if (!string.IsNullOrEmpty(value))
}
else
{
// Output all route-values
foreach (var routeItem in routeParameters)
{
yield return new KeyValuePair<string, string>(key, value);
string routeValue = routeItem.Value?.ToString();
if (!string.IsNullOrEmpty(routeValue))
{
yield return new KeyValuePair<string, string>(routeItem.Key, routeValue);
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,19 +125,6 @@ public void ProtocolTest()
// Assert
Assert.Equal(SslProtocols.Tls13.ToString(), result);
}


protected override void NullRendersEmptyString()
{
// Arrange
var (renderer, _) = CreateWithHttpContext();

// Act
string result = renderer.Render(LogEventInfo.CreateNullEvent());

// Assert
Assert.Equal("None", result);
}
}
}
#endif
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ protected override void NullRendersEmptyString()
string result = renderer.Render(LogEventInfo.CreateNullEvent());

// Assert
Assert.Equal("0", result);
Assert.Equal("1", result);
}
}
}
43 changes: 38 additions & 5 deletions tests/Shared/LayoutRenderers/AspNetMvcActionRendererTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,32 +6,65 @@
using System.Collections.Specialized;
using System.Web.SessionState;
#else
using Microsoft.Extensions.Primitives;
using HttpContextBase = Microsoft.AspNetCore.Http.HttpContext;
using Microsoft.AspNetCore.Routing;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Http.Features;
using Microsoft.AspNetCore.Routing;
#endif
using NLog.Web.LayoutRenderers;
using NSubstitute;
using Xunit;


namespace NLog.Web.Tests.LayoutRenderers
{
public class AspNetMvcActionRendererTests : LayoutRenderersTestBase<AspNetMvcActionRenderer>
{
[Fact]
public void NullRoutesRenderersEmptyString()
{
// Arrange
var (renderer, _) = CreateWithHttpContext();

// Act
string result = renderer.Render(LogEventInfo.CreateNullEvent());

// Assert
Assert.Empty(result);
}

#if ASP_NET_CORE
[Fact]
public void ActionKeyRendersRouteParameter()
{
// Arrange
var (renderer, httpContext) = CreateWithHttpContext();

AddRoutingFeature(httpContext);
SetupRouteParameters(httpContext);

// Act
string result = renderer.Render(LogEventInfo.CreateNullEvent());

// Assert
Assert.Empty(result);
Assert.Equal("actionName", result);
}

private void SetupRouteParameters(HttpContext httpContext)
{
var collection = new FeatureCollection();
var routeData = new RouteData();
var routingFeature = Substitute.For<IRoutingFeature>();
collection.Set(routingFeature);
#if ASP_NET_CORE3
var routingValuesFeature = Substitute.For<IRouteValuesFeature>();
routingValuesFeature.RouteValues.Returns(routeData.Values);
collection.Set(routingValuesFeature);
#endif
httpContext.Features.Returns(collection);

routeData.Values.Add("action", "actionName");
routeData.Values.Add("controller", "controllerName");
routingFeature.RouteData.Returns(routeData);
}
#endif
}
}
42 changes: 37 additions & 5 deletions tests/Shared/LayoutRenderers/AspNetMvcControllerRendererTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@
using System.Collections.Specialized;
using System.Web.SessionState;
#else
using Microsoft.Extensions.Primitives;
using HttpContextBase = Microsoft.AspNetCore.Http.HttpContext;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Routing;
using Microsoft.AspNetCore.Http.Features;
#endif
Expand All @@ -25,15 +24,48 @@ public class AspNetMvcControllerRendererTests : LayoutRenderersTestBase<AspNetMv
public void NullRoutesRenderersEmptyString()
{
// Arrange
var (renderer, httpContext) = CreateWithHttpContext();
var (renderer, _) = CreateWithHttpContext();

AddRoutingFeature(httpContext);

// Act
string result = renderer.Render(LogEventInfo.CreateNullEvent());

// Assert
Assert.Empty(result);
}

#if ASP_NET_CORE
[Fact]
public void ControllerKeyRendersRouteParameter()
{
// Arrange
var (renderer, httpContext) = CreateWithHttpContext();

SetupRouteParameters(httpContext);

// Act
string result = renderer.Render(LogEventInfo.CreateNullEvent());

// Assert
Assert.Equal("controllerName", result);
}

private void SetupRouteParameters(HttpContext httpContext)
{
var collection = new FeatureCollection();
var routeData = new RouteData();
var routingFeature = Substitute.For<IRoutingFeature>();
collection.Set(routingFeature);
#if ASP_NET_CORE3
var routingValuesFeature = Substitute.For<IRouteValuesFeature>();
routingValuesFeature.RouteValues.Returns(routeData.Values);
collection.Set(routingValuesFeature);
#endif
httpContext.Features.Returns(collection);

routeData.Values.Add("action", "actionName");
routeData.Values.Add("controller", "controllerName");
routingFeature.RouteData.Returns(routeData);
}
#endif
}
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
using System;
using System.Collections.Generic;
using NLog.Web.LayoutRenderers;
using NLog.Web.Enums;
using Xunit;
using System.Collections.Specialized;
using NSubstitute;
#if ASP_NET_CORE
using Microsoft.AspNetCore.Http;
Expand All @@ -15,21 +13,6 @@ namespace NLog.Web.Tests.LayoutRenderers
{
public class AspNetRequestRouteParametersRendererTests : LayoutRenderersTestBase<AspNetRequestRouteParametersRenderer>
{
[Fact]
public void NullRouteParametersRenderersEmptyString()
{
// Arrange
var (renderer, httpContext) = CreateWithHttpContext();

AddRoutingFeature(httpContext);

// Act
string result = renderer.Render(LogEventInfo.CreateNullEvent());

// Assert
Assert.Empty(result);
}

#if ASP_NET_CORE
[Fact]
public void NullKeyRendersAllRouteParameters()
Expand Down Expand Up @@ -69,6 +52,11 @@ private void SetupRouteParameters(HttpContext httpContext)
var routingFeature = Substitute.For<IRoutingFeature>();
var collection = new FeatureCollection();
collection.Set(routingFeature);
#if ASP_NET_CORE3
var routingValuesFeature = Substitute.For<IRouteValuesFeature>();
routingValuesFeature.RouteValues.Returns(routeData.Values);
collection.Set(routingValuesFeature);
#endif
httpContext.Features.Returns(collection);

routeData.Values.Add("key1", "value1");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
using System.Web.Routing;
using System.Collections.Specialized;
using System.Web.SessionState;
#else
using HttpContextBase = Microsoft.AspNetCore.Http.HttpContext;
#endif
using NLog.Web.LayoutRenderers;
using NSubstitute;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,7 @@ private static (AspNetSessionValueLayoutRenderer, HttpContext) CreateRenderer(bo
mockSession.SetInt32("b", 123);
httpContext.Session = mockSession;
httpContext.Items = new Dictionary<object, object>();
var sessionFeature = new SessionFeatureMock(mockSession);
httpContext.Features.Get<ISessionFeature>().Returns(sessionFeature);

return (renderer, httpContext);
}

Expand Down
Loading