From 80cda3364b09c3241e6284761dc416e79ef8fe71 Mon Sep 17 00:00:00 2001 From: Mole Date: Tue, 21 Dec 2021 13:35:30 +0100 Subject: [PATCH] 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. --- .../Models/RequestHandlerSettings.cs | 37 +++++++++++++++---- .../Strings/DefaultShortStringHelperConfig.cs | 2 +- .../Models/RequestHandlerSettingsTests.cs | 12 +++--- 3 files changed, 36 insertions(+), 15 deletions(-) diff --git a/src/Umbraco.Core/Configuration/Models/RequestHandlerSettings.cs b/src/Umbraco.Core/Configuration/Models/RequestHandlerSettings.cs index c27ad63092..4894fb7125 100644 --- a/src/Umbraco.Core/Configuration/Models/RequestHandlerSettings.cs +++ b/src/Umbraco.Core/Configuration/Models/RequestHandlerSettings.cs @@ -75,32 +75,53 @@ namespace Umbraco.Cms.Core.Configuration.Models [DefaultValue(StaticEnableDefaultCharReplacements)] public bool EnableDefaultCharReplacements { get; set; } = StaticEnableDefaultCharReplacements; + private IEnumerable _charCollection; + /// /// Add additional character replacements, or override defaults /// - public IEnumerable CharCollection { get; set; } + public IEnumerable 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(); + return null; + } + + return GetCharReplacements(); + } + + set => _charCollection = value; + } /// /// Get concatenated user and default character replacements /// taking into account /// - public IEnumerable GetCharReplacements() + private IEnumerable GetCharReplacements() { // TODO We need to special handle ":", as this character is special in keys - if (!EnableDefaultCharReplacements) { - return CharCollection; + return _charCollection ?? Enumerable.Empty(); } - 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 @@ namespace Umbraco.Cms.Core.Configuration.Models } } - var mergedCollections = DefaultCharCollection.Union(CharCollection, new CharacterReplacementEqualityComparer()); + IEnumerable mergedCollections = DefaultCharCollection.Union(_charCollection, new CharacterReplacementEqualityComparer()); return mergedCollections; } diff --git a/src/Umbraco.Core/Strings/DefaultShortStringHelperConfig.cs b/src/Umbraco.Core/Strings/DefaultShortStringHelperConfig.cs index 287d33dd58..273d7a6562 100644 --- a/src/Umbraco.Core/Strings/DefaultShortStringHelperConfig.cs +++ b/src/Umbraco.Core/Strings/DefaultShortStringHelperConfig.cs @@ -60,7 +60,7 @@ namespace Umbraco.Cms.Core.Strings /// The short string helper. 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); diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Configuration/Models/RequestHandlerSettingsTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Configuration/Models/RequestHandlerSettingsTests.cs index 9f2368d462..d681d4a70e 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Configuration/Models/RequestHandlerSettingsTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Configuration/Models/RequestHandlerSettingsTests.cs @@ -13,13 +13,13 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Configuration.Models { 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 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Configuration.Models }; 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 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Configuration.Models }; 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 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Configuration.Models }; 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);