akkadotnet#4353 Config.WithFallback is acting inconsistently (akkadot…

…net#4358)

* Add bug spec

* Missing keys during merge

* Remove weird workaround that doesn't make any sense

* How root are calculated neeed to be reversed

* Original node value need to be preserved

* use Config.Root to standardize Config access

* Possible fix for "collection was modified" exception during merge

* Minor optimization

src/core/Akka.Cluster.Tests.MultiNode/Bugfix4353Specs.cs

@@ -0,0 +1,146 @@
+using System;
+using System.Linq;
+using System.Collections.Generic;
+using System.Threading;
+
+using Akka.Actor;
+using Akka.Cluster.TestKit;
+using Akka.Configuration;
+using Akka.Remote.TestKit;
+using Akka.TestKit;
+
+namespace Akka.Cluster.Tests.MultiNode
+{
+    public class Bugfix4353Spec : MultiNodeClusterSpec
+    {
+        protected RoleName First;
+        protected RoleName Second;
+        protected RoleName Third;
+
+        public Bugfix4353Spec() : this(new Bugfix4353SpecsConfig())
+        {
+        }
+
+        protected Bugfix4353Spec(Bugfix4353SpecsConfig config) : base(config, typeof(Bugfix4353Spec))
+        {
+            First = config.First;
+            Second = config.Second;
+            Third = config.Third;
+        }
+
+        [MultiNodeFact]
+        public void Bugfix4353Spec_Cluster_of_3_must_reach_cnovergence()
+        {
+            AwaitClusterUp(First, Second, Third);
+            EnterBarrier("after-1");
+        }
+    }
+
+    public class Bugfix4353SpecsConfig : MultiNodeConfig
+    {
+        private static readonly string[] Hocons =
+        {
+@"akka : {
+  actor : {
+    provider : cluster 
+  }
+}",
+
+@"akka : {
+  stdout-loglevel : INFO
+  loglevel : INFO 
+  log-config-on-start : on 
+  loggers : [""Akka.Event.DefaultLogger""],
+  actor : {
+    debug : {
+      receive : on
+      autoreceive : on
+      lifecycle : on
+      event-stream : on
+      unhandled : on 
+    } 
+  } 
+}",
+
+@"akka : {
+  remote : {
+    dot-netty : {
+      tcp : {
+        log-transport : true
+        transport-class : ""Akka.Remote.Transport.DotNetty.TcpTransport, Akka.Remote""
+        transport-protocol : tcp
+        hostname : 0.0.0.0
+        public-hostname : localhost
+      } 
+    } 
+  } 
+}",
+
+@"akka : {
+  cluster : {
+    log-info : on
+    seed-nodes : [
+      ""akka.tcp://Bugfix4353Spec@localhost:5001"",
+      ""akka.tcp://Bugfix4353Spec@localhost:5002"",
+      ""akka.tcp://Bugfix4353Spec@localhost:5003""
+    ]
+    roles : [seed]
+    role : { } 
+  } 
+}"
+        };
+
+        public static Config Config
+        {
+            get
+            {
+                var config = ConfigurationFactory.Empty;
+
+                foreach (var hocon in Hocons)
+                {
+                    config = config.WithFallback(ConfigurationFactory.ParseString(hocon));
+                }
+                return config;
+            }
+        }
+
+        public readonly RoleName First;
+        public readonly RoleName Second;
+        public readonly RoleName Third;
+
+        public Bugfix4353SpecsConfig()
+        {
+
+            First = Role("first");
+            Second = Role("second");
+            Third = Role("third");
+
+            CommonConfig = MultiNodeClusterSpec.ClusterConfig(false);
+
+            NodeConfig(
+                new[] { First },
+                new[] {
+                    ConfigurationFactory
+                    .ParseString("akka.remote.dot-netty.tcp.port : 5001")
+                    .WithFallback(Config)
+                });
+
+            NodeConfig(
+                new[] { Second },
+                new[] {
+                    ConfigurationFactory
+                    .ParseString("akka.remote.dot-netty.tcp.port : 5002")
+                    .WithFallback(Config)
+                });
+
+            NodeConfig(
+                new[] { Third },
+                new[] {
+                    ConfigurationFactory
+                    .ParseString("akka.remote.dot-netty.tcp.port : 5003")
+                    .WithFallback(Config)
+                });
+
+        }
+    }
+}

src/core/Akka/Actor/Deploy.cs

@@ -202,7 +202,7 @@ public bool Equals(Deploy other)
                    string.Equals(_path, other._path) &&
                    _routerConfig.Equals(other._routerConfig) &&
                    ((_config.IsNullOrEmpty() && other._config.IsNullOrEmpty()) ||
-                    _config.ToString().Equals(other._config.ToString())) &&
+                    _config.Root.ToString().Equals(other._config.Root.ToString())) &&
                    (_scope == null && other._scope == null || (_scope != null && _scope.Equals(other._scope)));
         }
 

src/core/Akka/Actor/Settings.cs

@@ -309,7 +309,7 @@ private static string GetProviderClass(string provider)
         /// <inheritdoc/>
         public override string ToString()
         {
-            return Config.ToString();
+            return Config.Root.ToString();
         }
     }
 }

src/core/Akka/Configuration/Config.cs

@@ -37,6 +37,7 @@ public Config(HoconRoot root)
             if (root.Value == null)
                 throw new ArgumentNullException(nameof(root), "The root value cannot be null.");
 
+            Value = root.Value;
             Root = root.Value;
             Substitutions = root.Substitutions;
         }
@@ -52,6 +53,7 @@ public Config(Config source, Config fallback)
             if (source == null)
                 throw new ArgumentNullException(nameof(source), "The source configuration cannot be null.");
 
+            Value = source.Value;
             Root = source.Root;
             Fallback = fallback;
         }
@@ -69,6 +71,8 @@ public virtual bool IsEmpty
             get { return Root == null || Root.IsEmpty; }
         }
 
+        private HoconValue Value { get; set; }
+
         /// <summary>
         /// The root node of this configuration section
         /// </summary>
@@ -90,6 +94,7 @@ protected Config Copy(Config fallback = null)
             {
                 Fallback = Fallback != null ? Fallback.Copy(fallback) : fallback,
                 Root = Root,
+                Value = Value,
                 Substitutions = Substitutions
             };
         }
@@ -423,10 +428,10 @@ public virtual TimeSpan GetTimeSpan(string path, TimeSpan? @default = null, bool
         /// <returns>A string containing the current configuration.</returns>
         public override string ToString()
         {
-            if (Root == null)
+            if (Value == null)
                 return "";
 
-            return Root.ToString();
+            return Value.ToString();
         }
 
         /// <summary>
@@ -438,21 +443,7 @@ public string ToString(bool includeFallback)
         {
             if (includeFallback == false)
                 return ToString();
-
-            Config current = this;
-
-            if (current.Fallback == null)
-                return current.ToString();
-
-            Config clone = Copy();
-
-            while (current.Fallback != null)
-            {
-                clone.Root.GetObject().Merge(current.Fallback.Root.GetObject());
-                current = current.Fallback;
-            }
-
-            return clone.ToString();
+            return Root.ToString();
         }
 
         /// <summary>
@@ -467,7 +458,10 @@ public virtual Config WithFallback(Config fallback)
                 throw new ArgumentException("Config can not have itself as fallback", nameof(fallback));
             if (fallback == null)
                 return this;
-            var mergedRoot = Root.GetObject().MergeImmutable(fallback.Root.GetObject());
+            if (IsEmpty)
+                return fallback;
+
+            var mergedRoot = fallback.Root.GetObject().MergeImmutable(Root.GetObject());
             var newRoot = new HoconValue();
             newRoot.AppendValue(mergedRoot);
             var mergedConfig = Copy(fallback);

src/core/Akka/Configuration/Hocon/HoconObject.cs

@@ -159,7 +159,6 @@ public string ToString(int indent)
             var sb = new StringBuilder();
             foreach (var kvp in Items)
             {
-                if (kvp.Value.AdoptedFromFallback) continue;
                 string key = QuoteIfNeeded(kvp.Key);
                 sb.AppendFormat("{0}{1} : {2}\r\n", i, key, kvp.Value.ToString(indent));
             }
@@ -186,6 +185,7 @@ public void Merge(HoconObject other)
         {
             var thisItems = Items;
             var otherItems = other.Items;
+            var modified = new List<KeyValuePair<string, HoconValue>>();
 
             foreach (var otherItem in otherItems)
             {
@@ -195,14 +195,27 @@ public void Merge(HoconObject other)
                 {
                     //if both values are objects, merge them
                     if (thisItem.IsObject() && otherItem.Value.IsObject())
-                        thisItem.GetObject().Merge(otherItem.Value.GetObject());
+                    {
+                        var newObject = thisItem.GetObject().MergeImmutable(otherItem.Value.GetObject());
+                        var value = new HoconValue();
+                        value.Values.Add(newObject);
+                        modified.Add(new KeyValuePair<string, HoconValue>(otherItem.Key, value));
+                    }
+                    else
+                        modified.Add(new KeyValuePair<string, HoconValue>(otherItem.Key, otherItem.Value));
                 }
                 else
                 {
                     //other key was not present in this object, just copy it over
-                    Items.Add(otherItem.Key, otherItem.Value);
+                    modified.Add(new KeyValuePair<string, HoconValue>(otherItem.Key, otherItem.Value));
                 }
             }
+
+            if (modified.Count == 0)
+                return;
+
+            foreach(var kvp in modified)
+                Items[kvp.Key] = kvp.Value;
         }
 
         /// <summary>
@@ -228,6 +241,8 @@ internal HoconObject MergeImmutable(HoconObject other)
                         mergedValue.AppendValue(mergedObject);
                         thisItems[otherItem.Key] = mergedValue;
                     }
+                    else
+                        thisItems[otherItem.Key] = otherItem.Value;
                 }
                 else
                 {