Auto ordering of variables-section when dependent variables (#737)

NLog · Apr 21, 2024 · 3f70293 · 3f70293
1 parent efcb7d8
commit 3f70293
Show file tree

Hide file tree

Showing 3 changed files with 93 additions and 10 deletions.
diff --git a/src/NLog.Extensions.Logging/Config/NLogLoggingConfiguration.cs b/src/NLog.Extensions.Logging/Config/NLogLoggingConfiguration.cs
@@ -218,7 +218,7 @@ private IEnumerable<KeyValuePair<string, string>> GetValues()
                         var configKey = GetConfigKey(child);
                         var configValue = child.Value;
                         if (_topElement && IgnoreTopElementChildNullValue(configKey, configValue))
-                            continue;   // Complex object without any properties has no children and null-value (Ex. empty targets-section)
+                            continue;   // Complex object without any properties has no children and null-value (Ex. empty targets-section / variables-section)
 
                         yield return new KeyValuePair<string, string>(configKey, configValue);
                     }
@@ -229,13 +229,24 @@ private static bool IgnoreTopElementChildNullValue(string configKey, object conf
             {
                 if (configValue is null)
                 {
-                    if (string.Equals(TargetsKey, configKey, StringComparison.OrdinalIgnoreCase)
-                        || string.Equals(VariablesKey, configKey, StringComparison.OrdinalIgnoreCase)
-                        || string.Equals(TargetDefaultParameters, configKey, StringComparison.OrdinalIgnoreCase)
-                        || string.Equals(DefaultTargetParameters, configKey, StringComparison.OrdinalIgnoreCase)
-                        || string.Equals(RulesKey, configKey, StringComparison.OrdinalIgnoreCase)
-                        || string.Equals(ExtensionsKey, configKey, StringComparison.OrdinalIgnoreCase))
-                        return true;    // Only accept known section-names as being empty (when no children and null value)
+                    // Only accept known section-names as being empty (when no children and null value)
+                    if (string.Equals(TargetDefaultParameters, configKey, StringComparison.OrdinalIgnoreCase))
+                        return true;
+
+                    if (string.Equals(DefaultTargetParameters, configKey, StringComparison.OrdinalIgnoreCase))
+                        return true;
+
+                    if (string.Equals(RulesKey, configKey, StringComparison.OrdinalIgnoreCase))
+                        return true;
+
+                    if (string.Equals(ExtensionsKey, configKey, StringComparison.OrdinalIgnoreCase))
+                        return true;
+
+                    if (string.Equals(VariablesKey, configKey, StringComparison.OrdinalIgnoreCase))
+                        return true;
+
+                    if (string.Equals(TargetsKey, configKey, StringComparison.OrdinalIgnoreCase))
+                        return true;
                 }
 
                 return false;
@@ -246,7 +257,7 @@ private IEnumerable<ILoggingConfigurationElement> GetChildren()
                 var variables = GetVariablesSection();
                 if (variables != null)
                 {
-                    foreach (var variable in variables.GetChildren())
+                    foreach (var variable in GetVariablesChildren(variables))
                     {
                         yield return new LoggingConfigurationElement(variable, VariableKey);
                     }
@@ -311,6 +322,59 @@ private IEnumerable<ILoggingConfigurationElement> GetChildren(IEnumerable<IConfi
                 }
             }
 
+            private IEnumerable<IConfigurationSection> GetVariablesChildren(IConfigurationSection variables)
+            {
+                List<KeyValuePair<string, IConfigurationSection>> sortVariables = null;
+                foreach (var variable in variables.GetChildren())
+                {
+                    var configKey = GetConfigKey(variable);
+                    var configValue = variable.Value;
+                    if (string.IsNullOrEmpty(configKey) || string.IsNullOrEmpty(configValue) || !configValue.Contains('$'))
+                        yield return variable;
+
+                    sortVariables ??= new List<KeyValuePair<string, IConfigurationSection>>();
+                    sortVariables.Insert(0, new KeyValuePair<string, IConfigurationSection>(configKey, variable));
+                }
+
+                bool foundIndependentVariable = true;
+                while (sortVariables?.Count > 0 && foundIndependentVariable)
+                {
+                    foundIndependentVariable = false;
+
+                    // Enumerate all variables that doesn't reference other variables
+                    for (int i = sortVariables.Count - 1; i >= 0; i--)
+                    {
+                        var configValue = sortVariables[i].Value.Value;
+                        var independentVariable = true;
+                        for (int j = i - 1; j >= 0; j--)
+                        {
+                            var otherConfigKey = sortVariables[j].Key;
+                            var referenceVariable = $"${{{otherConfigKey}}}";
+                            if (configValue.IndexOf(referenceVariable, StringComparison.OrdinalIgnoreCase) >= 0)
+                            {
+                                independentVariable = false;
+                                break;
+                            }
+                        }
+                        if (independentVariable)
+                        {
+                            foundIndependentVariable = true;
+                            yield return sortVariables[i].Value;
+                            sortVariables.RemoveAt(i);
+                        }
+                    }
+                }
+
+                if (sortVariables?.Count > 0)
+                {
+                    // Give up and just return the variables in their sorted order
+                    for (int i = sortVariables.Count - 1; i >= 0; i--)
+                    {
+                        yield return sortVariables[i].Value;
+                    }
+                }
+            }
+
             private static string GetConfigKey(IConfigurationSection child)
             {
                 return child.Key?.Trim() ?? string.Empty;

diff --git a/src/NLog.Extensions.Logging/README.md b/src/NLog.Extensions.Logging/README.md
@@ -19,7 +19,7 @@ If using ASP.NET Core then check [NLog.Web.AspNetCore](https://www.nuget.org/pac
 
 Supported platforms:
 
- - .NET 5, 6 and 7
+ - .NET 5, 6, 7 and 8
  - .NET Core 1, 2 and 3
  - .NET Standard 1.3+ and 2.0+
  - .NET 4.6.1 - 4.8

diff --git a/test/NLog.Extensions.Logging.Tests/NLogLoggingConfigurationTests.cs b/test/NLog.Extensions.Logging.Tests/NLogLoggingConfigurationTests.cs
@@ -187,6 +187,25 @@ public void LoadVariablesConfig()
             Assert.Equal("hello.txt", (logConfig.FindTargetByName("file") as FileTarget)?.FileName.Render(LogEventInfo.CreateNullEvent()));
         }
 
+        [Fact]
+        public void LoadVariablesSortedConfig()
+        {
+            var memoryConfig = CreateMemoryConfigConsoleTargetAndRule();
+            memoryConfig["NLog:Targets:file:type"] = "File";
+            memoryConfig["NLog:Targets:file:fileName"] = "${var_file}";
+            memoryConfig["NLog:Variables:var_folder"] = "hello";
+            memoryConfig["NLog:Variables:var_file"] = "${var_folder}/world.txt";
+
+            var logConfig = CreateNLogLoggingConfigurationWithNLogSection(memoryConfig);
+
+            Assert.Single(logConfig.LoggingRules);
+            Assert.Equal(2, logConfig.LoggingRules[0].Targets.Count);
+            Assert.Equal(2, logConfig.AllTargets.Count);
+            Assert.Single(logConfig.AllTargets.Where(t => t is FileTarget));
+            Assert.Single(logConfig.AllTargets.Where(t => t is ConsoleTarget));
+            Assert.Equal("hello/world.txt", (logConfig.FindTargetByName("file") as FileTarget)?.FileName.Render(LogEventInfo.CreateNullEvent()));
+        }
+
         [Fact]
         public void LoadVariableJsonLayoutConfig()
         {