AspNetLayoutRendererBase.Register should cache IHttpContextAccessor

examples/ASP.NET Core 2/Visual Studio 2017/ASP.NET Core 2 - VS2017/Program.cs

@@ -3,6 +3,7 @@
 using Microsoft.AspNetCore.Hosting;
 using Microsoft.Extensions.Configuration;
 using Microsoft.Extensions.Logging;
+using NLog;
 using NLog.Extensions.Logging;
 using NLog.Web;
 

src/Shared/LayoutRenderers/AspNetLayoutRendererBase.cs

@@ -44,7 +44,7 @@ public IHttpContextAccessor HttpContextAccessor
         private static IHttpContextAccessor RetrieveHttpContextAccessor(Type _) => DefaultHttpContextAccessor;
 #else
 
-        private static IHttpContextAccessor RetrieveHttpContextAccessor(Type classType)
+        internal static IHttpContextAccessor RetrieveHttpContextAccessor(Type classType)
         {
             var serviceProvider = ServiceLocator.ServiceProvider;
             if (serviceProvider == null)
@@ -113,16 +113,14 @@ protected override void CloseLayoutRenderer()
 #endif
 
         /// <summary>
-        /// Register a custom layout renderer with a callback function <paramref name="func" />. The callback recieves the logEvent and the current configuration.
+        /// Register a custom layout renderer with a callback function <paramref name="func" />. The callback receives the logEvent and the current configuration.
         /// </summary>
         /// <param name="name">Name of the layout renderer - without ${}.</param>
         /// <param name="func">Callback that returns the value for the layout renderer.</param>
         public static void Register(string name, Func<LogEventInfo, HttpContextBase, LoggingConfiguration, object> func)
         {
-            // TODO Missing caching (and cache-reset) of HttpContextAccessor - Constant lookup in ServiceProvider can lead to deadlock situation
-            object NewFunc(LogEventInfo logEventInfo, LoggingConfiguration configuration) => func(logEventInfo, RetrieveHttpContextAccessor(null)?.HttpContext, configuration);
-
-            Register(name, NewFunc);
+            var renderer = new NLogWebLayoutRenderer(name, func);
+            Register(renderer);
         }
     }
 }

src/Shared/LayoutRenderers/NLogWebLayoutRenderer.cs

@@ -0,0 +1,45 @@
+using System;
+#if ASP_NET_CORE
+using Microsoft.AspNetCore.Http;
+using HttpContextBase = Microsoft.AspNetCore.Http.HttpContext;
+#else
+using System.Web;
+#endif
+using NLog.Config;
+using NLog.LayoutRenderers;
+
+namespace NLog.Web.LayoutRenderers
+{
+    /// <summary>
+    /// Specialized layout render which has a cached <see cref="IHttpContextAccessor"/>
+    /// </summary>
+    internal class NLogWebLayoutRenderer : FuncLayoutRenderer
+    {
+        readonly Func<LogEventInfo, HttpContextBase, LoggingConfiguration, object> _func;
+
+#if !ASP_NET_CORE
+
+        private static IHttpContextAccessor HttpContextAccessor { get; } = AspNetLayoutRendererBase.DefaultHttpContextAccessor;
+#else
+
+        private IHttpContextAccessor _httpContextAccessor;
+        public IHttpContextAccessor HttpContextAccessor
+        {
+            get => _httpContextAccessor ?? (_httpContextAccessor = AspNetLayoutRendererBase.RetrieveHttpContextAccessor(GetType()));
+            set => _httpContextAccessor = value;
+        }
+#endif
+
+        public NLogWebLayoutRenderer(string name, Func<LogEventInfo, HttpContextBase, LoggingConfiguration, object> func) : base(name)
+        {
+            _func = func;
+        }
+
+        /// <inheritdoc />
+        protected override object RenderValue(LogEventInfo logEvent)
+        {
+            var httpContext = HttpContextAccessor?.HttpContext;
+            return _func(logEvent, httpContext, LoggingConfiguration);
+        }
+    }
+}

tests/Shared/RegisterCustomLayoutRenderer.cs

@@ -1,10 +1,5 @@
 using System;
-using System.Collections.Generic;
 using System.Linq;
-using System.Text;
-using System.Threading.Tasks;
-
-
 using NLog.Layouts;
 #if ASP_NET_CORE
 using Microsoft.AspNetCore.Http;
@@ -30,11 +25,11 @@ public class RegisterCustomLayoutRenderer : TestBase
         [Fact]
         public void RegisterLayoutRendererTest()
         {
-            var httpcontextMock = SetupHttpAccessorWithHttpContext();
+            var httpContextMock = SetupHttpAccessorWithHttpContext();
 #if ASP_NET_CORE
-            httpcontextMock.Connection.LocalPort.Returns(123);
+            httpContextMock.Connection.LocalPort.Returns(123);
 #else
-            httpcontextMock.Request.RawUrl.Returns("123");
+            httpContextMock.Request.RawUrl.Returns("123");
 
 #endif
 
@@ -47,10 +42,10 @@ public void RegisterLayoutRendererTest()
                     httpContext.Request.RawUrl);
 #endif
             Layout l = "${test-web}";
-            var restult = l.Render(LogEventInfo.CreateNullEvent());
+            var result = l.Render(LogEventInfo.CreateNullEvent());
 
             // Assert
-            Assert.Equal("123", restult);
+            Assert.Equal("123", result);
         }
 
         private static
@@ -68,17 +63,17 @@ private static
 #if ASP_NET_CORE
             var serviceProviderMock = Substitute.For<IServiceProvider>();
             serviceProviderMock.GetService(typeof(IHttpContextAccessor)).Returns(httpContextAccessorMock);
-            var httpcontext = Substitute.For<HttpContext>();
+            var httpContext = Substitute.For<HttpContext>();
             ServiceLocator.ServiceProvider = serviceProviderMock;
 #else
-            var httpcontext = Substitute.For<HttpContextBase>();
-            httpContextAccessorMock.HttpContext.Returns(httpcontext);
+            var httpContext = Substitute.For<HttpContextBase>();
+            httpContextAccessorMock.HttpContext.Returns(httpContext);
             AspNetLayoutRendererBase.DefaultHttpContextAccessor = httpContextAccessorMock;
 #endif
 
 
-            httpContextAccessorMock.HttpContext.Returns(httpcontext);
-            return httpcontext;
+            httpContextAccessorMock.HttpContext.Returns(httpContext);
+            return httpContext;
         }
     }
 }