Make CharCollection return the merged result

Since this is the way it's been previously, we cannot change the behaviour of this. Unfortunately due to how the config binding works, we have to make sure it returns null the very first time it's called by the framework to ensure that overrides actually gets registered.

Author: nikolajlauridsen

src/Umbraco.Core/Configuration/Models/RequestHandlerSettings.cs

@@ -75,32 +75,53 @@ public class RequestHandlerSettings
         [DefaultValue(StaticEnableDefaultCharReplacements)]
         public bool EnableDefaultCharReplacements { get; set; } = StaticEnableDefaultCharReplacements;
 
+        private IEnumerable<CharItem> _charCollection;
+
         /// <summary>
         /// Add additional character replacements, or override defaults
         /// </summary>
-        public IEnumerable<CharItem> CharCollection { get; set; }
+        public IEnumerable<CharItem> CharCollection
+        {
+            get
+            {
+                // This is pretty ugly, but necessary.
+                // Essentially the config binding will only run properly if we return null the first time this is invoked.
+                // Otherwise whatever we return will just be used and user specific bindings won't overwrite the existing ones.
+                // However the next time this get is invoked, for instance in DefaultShortStringHelper we want to actually run the GetCharReplacements
+                // To make sure that the default bindings is used if configured to do so.
+                // Therefore we set _charCollection to be something, and still return null, to trick dotnet to actually read the config.
+                if (_charCollection is null)
+                {
+                    _charCollection = Enumerable.Empty<CharItem>();
+                    return null;
+                }
+
+                return GetCharReplacements();
+            }
+
+            set => _charCollection = value;
+        }
 
         /// <summary>
         /// Get concatenated user and default character replacements
         /// taking into account <see cref="EnableDefaultCharReplacements"/>
         /// </summary>
-        public IEnumerable<CharItem> GetCharReplacements()
+        private IEnumerable<CharItem> GetCharReplacements()
         {
             // TODO We need to special handle ":", as this character is special in keys
-
             if (!EnableDefaultCharReplacements)
             {
-                return CharCollection;
+                return _charCollection ?? Enumerable.Empty<CharItem>();
             }
 
-            if (CharCollection == null || !CharCollection.Any())
+            if (_charCollection == null || !_charCollection.Any())
             {
                 return DefaultCharCollection;
             }
 
-            foreach (var defaultReplacement in DefaultCharCollection)
+            foreach (CharItem defaultReplacement in DefaultCharCollection)
             {
-                foreach (var userReplacement in CharCollection)
+                foreach (CharItem userReplacement in _charCollection)
                 {
                     if (userReplacement.Char == defaultReplacement.Char)
                     {
@@ -109,7 +130,7 @@ public IEnumerable<CharItem> GetCharReplacements()
                 }
             }
 
-            var mergedCollections = DefaultCharCollection.Union<CharItem>(CharCollection, new CharacterReplacementEqualityComparer());
+            IEnumerable<CharItem> mergedCollections = DefaultCharCollection.Union<CharItem>(_charCollection, new CharacterReplacementEqualityComparer());
 
             return mergedCollections;
         }

src/Umbraco.Core/Strings/DefaultShortStringHelperConfig.cs

@@ -60,7 +60,7 @@ public DefaultShortStringHelperConfig WithConfig(string culture, CleanStringType
         /// <returns>The short string helper.</returns>
         public DefaultShortStringHelperConfig WithDefault(RequestHandlerSettings requestHandlerSettings)
         {
-            UrlReplaceCharacters = requestHandlerSettings.GetCharReplacements()
+            UrlReplaceCharacters = requestHandlerSettings.CharCollection
                 .Where(x => string.IsNullOrEmpty(x.Char) == false)
                 .ToDictionary(x => x.Char, x => x.Replacement);
 

tests/Umbraco.Tests.UnitTests/Umbraco.Core/Configuration/Models/RequestHandlerSettingsTests.cs

@@ -13,13 +13,13 @@ public void Given_CharCollection_With_DefaultEnabled_MergesCollection()
         {
             var userCollection = new CharItem[]
             {
-                new() { Char = "test", Replacement = "replace" },
-                new() { Char = "test2", Replacement = "replace2" }
+                new () { Char = "test", Replacement = "replace" },
+                new () { Char = "test2", Replacement = "replace2" }
             };
 
 
             var settings = new RequestHandlerSettings { CharCollection = userCollection };
-            var actual = settings.GetCharReplacements().ToList();
+            var actual = settings.CharCollection.ToList();
 
             var expectedCollection = RequestHandlerSettings.DefaultCharCollection.ToList();
             expectedCollection.AddRange(userCollection);
@@ -38,7 +38,7 @@ public void Given_CharCollection_With_DefaultDisabled_ReturnsUserCollection()
             };
 
             var settings = new RequestHandlerSettings { CharCollection = userCollection, EnableDefaultCharReplacements = false };
-            var actual = settings.GetCharReplacements().ToList();
+            var actual = settings.CharCollection.ToList();
 
             Assert.AreEqual(userCollection.Length, actual.Count);
             Assert.That(actual, Is.EquivalentTo(userCollection));
@@ -54,7 +54,7 @@ public void Given_CharCollection_That_OverridesDefaultValues_ReturnsReplacements
             };
 
             var settings = new RequestHandlerSettings { CharCollection = userCollection };
-            var actual = settings.GetCharReplacements().ToList();
+            var actual = settings.CharCollection.ToList();
 
             Assert.AreEqual(RequestHandlerSettings.DefaultCharCollection.Length, actual.Count);
 
@@ -75,7 +75,7 @@ public void Given_CharCollection_That_OverridesDefaultValues_And_ContainsNew_Ret
             };
 
             var settings = new RequestHandlerSettings { CharCollection = userCollection };
-            var actual = settings.GetCharReplacements().ToList();
+            var actual = settings.CharCollection.ToList();
 
             // Add 1 to the length, because we're expecting to only add one new one
             Assert.AreEqual(RequestHandlerSettings.DefaultCharCollection.Length + 1, actual.Count);