NLogLoggingConfiguration Reload now fully support KeepVariablesOnReload (#522)

Author: snakefoot

src/NLog.Extensions.Logging/Config/NLogLoggingConfiguration.cs

@@ -36,7 +36,16 @@ public NLogLoggingConfiguration(IConfigurationSection nlogConfig, LogFactory log
             : base(logFactory)
         {
             _originalConfigSection = nlogConfig;
-            _autoReload = LoadConfigurationSection(nlogConfig);
+            LoadConfigurationSection(nlogConfig);
+        }
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="NLogLoggingConfiguration" /> class.
+        /// </summary>
+        private NLogLoggingConfiguration(LogFactory logFactory, IConfigurationSection nlogConfig)
+            : base(logFactory)
+        {
+            _originalConfigSection = nlogConfig;
         }
 
         /// <summary>
@@ -60,14 +69,22 @@ public override IEnumerable<string> FileNamesToWatch
         /// <inheritdoc />
         public override LoggingConfiguration Reload()
         {
-            return new NLogLoggingConfiguration(_originalConfigSection, LogFactory);
+            return ReloadLoggingConfiguration(_originalConfigSection);
+        }
+
+        private LoggingConfiguration ReloadLoggingConfiguration(IConfigurationSection nlogConfig)
+        {
+            var newConfig = new NLogLoggingConfiguration(LogFactory, nlogConfig);
+            newConfig.PrepareForReload(this);   // Ensure KeepVariablesOnReload works as intended
+            newConfig.LoadConfigurationSection(nlogConfig);
+            return newConfig;
         }
 
-        private bool LoadConfigurationSection(IConfigurationSection nlogConfig)
+        private void LoadConfigurationSection(IConfigurationSection nlogConfig)
         {
             var configElement = new LoggingConfigurationElement(nlogConfig, RootSectionKey);
             LoadConfig(configElement, null);
-            return configElement.AutoReload;
+            _autoReload = configElement.AutoReload;
         }
 
         private void LogFactory_ConfigurationChanged(object sender, LoggingConfigurationChangedEventArgs e)
@@ -100,15 +117,10 @@ private void ReloadConfigurationSection(IConfigurationSection nlogConfig)
                 }
 
                 InternalLogger.Info("Reloading NLogLoggingConfiguration...");
-                var newConfig = new NLogLoggingConfiguration(nlogConfig, LogFactory);
+                var newConfig = ReloadLoggingConfiguration(nlogConfig);
                 var oldConfig = LogFactory.Configuration;
                 if (oldConfig != null)
                 {
-                    if (LogFactory.KeepVariablesOnReload)
-                    {
-                        foreach (var variable in oldConfig.Variables)
-                            newConfig.Variables[variable.Key] = variable.Value;
-                    }
                     LogFactory.Configuration = newConfig;
                 }
             }

test/NLog.Extensions.Logging.Tests/NLogLoggingConfigurationTests.cs

@@ -255,17 +255,15 @@ public void ReloadLogFactoryConfigurationKeepVariables()
         {
             var memoryConfig = CreateMemoryConfigConsoleTargetAndRule();
             memoryConfig["NLog:Targets:file:type"] = "File";
-            memoryConfig["NLog:Targets:file:fileName"] = "${var:var_filename}";
+            memoryConfig["NLog:Targets:file:fileName"] = "${var_filename}";
             memoryConfig["NLog:autoreload"] = "true";
-            memoryConfig["NLog:KeepVariablesOnReload"] = "true";
             memoryConfig["NLog:variables:var_filename"] = "hello.txt";
             var configuration = new ConfigurationBuilder().AddInMemoryCollection(memoryConfig).Build();
             var logFactory = new LogFactory();
             var logConfig = new NLogLoggingConfiguration(configuration.GetSection("NLog"), logFactory);
             logFactory.Configuration = logConfig;
             Assert.Equal("hello.txt", (logFactory.Configuration.FindTargetByName("file") as FileTarget)?.FileName.Render(LogEventInfo.CreateNullEvent()));
             logFactory.Configuration.Variables["var_filename"] = "updated.txt";
-            Assert.Equal("updated.txt", (logFactory.Configuration.FindTargetByName("file") as FileTarget)?.FileName.Render(LogEventInfo.CreateNullEvent()));
             configuration.Reload(); // Automatic Reload
             Assert.Equal("updated.txt", (logFactory.Configuration.FindTargetByName("file") as FileTarget)?.FileName.Render(LogEventInfo.CreateNullEvent()));
         }