From 53e5a25df87ef8b8bce1f5cd32d8f5d652b582c9 Mon Sep 17 00:00:00 2001 From: Mole Date: Wed, 22 Dec 2021 11:29:04 +0100 Subject: [PATCH] Remove ugly CharCollection hack By adding a new configuration and mapping the old CharCollection to that, we can get around having to return null the first time, by obsoleting the old one and redirecting to the new GetCharReplacements method --- .../Models/RequestHandlerSettings.cs | 62 +++---------------- .../UmbracoBuilder.Configuration.cs | 20 ++++++ .../RequestHandlerSettingsExtension.cs | 44 +++++++++++++ .../Strings/DefaultShortStringHelperConfig.cs | 11 +--- .../Models/RequestHandlerSettingsTests.cs | 18 +++--- 5 files changed, 81 insertions(+), 74 deletions(-) create mode 100644 src/Umbraco.Core/Extensions/RequestHandlerSettingsExtension.cs diff --git a/src/Umbraco.Core/Configuration/Models/RequestHandlerSettings.cs b/src/Umbraco.Core/Configuration/Models/RequestHandlerSettings.cs index a38f152f3b..051c31dc26 100644 --- a/src/Umbraco.Core/Configuration/Models/RequestHandlerSettings.cs +++ b/src/Umbraco.Core/Configuration/Models/RequestHandlerSettings.cs @@ -1,9 +1,9 @@ // Copyright (c) Umbraco. // See LICENSE for more details. +using System; using System.Collections.Generic; using System.ComponentModel; -using System.Linq; using Umbraco.Cms.Core.Configuration.UmbracoSettings; using Umbraco.Extensions; @@ -75,63 +75,15 @@ namespace Umbraco.Cms.Core.Configuration.Models [DefaultValue(StaticEnableDefaultCharReplacements)] public bool EnableDefaultCharReplacements { get; set; } = StaticEnableDefaultCharReplacements; - private IEnumerable _charCollection; + /// + /// Add additional character replacements, or override defaults + /// + [Obsolete("Use the GetCharReplacements extension method in the Umbraco.Extensions namespace instead. Scheduled for removal in V11")] + public IEnumerable CharCollection { get; set; } = DefaultCharCollection; /// /// Add additional character replacements, or override defaults /// - 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 - /// - private IEnumerable GetCharReplacements() - { - if (!EnableDefaultCharReplacements) - { - return _charCollection ?? Enumerable.Empty(); - } - - if (_charCollection == null || !_charCollection.Any()) - { - return DefaultCharCollection; - } - - foreach (CharItem defaultReplacement in DefaultCharCollection) - { - foreach (CharItem userReplacement in _charCollection) - { - if (userReplacement.Char == defaultReplacement.Char) - { - defaultReplacement.Replacement = userReplacement.Replacement; - } - } - } - - IEnumerable mergedCollections = DefaultCharCollection.Union(_charCollection, new CharacterReplacementEqualityComparer()); - - return mergedCollections; - } + public IEnumerable UserDefinedCharCollection { get; set; } } } diff --git a/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Configuration.cs b/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Configuration.cs index 6ef87464e8..8043698b6e 100644 --- a/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Configuration.cs +++ b/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Configuration.cs @@ -1,5 +1,7 @@ using System; +using System.Collections.Generic; using System.Reflection; +using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; using Umbraco.Cms.Core.Configuration.Models; @@ -76,6 +78,24 @@ namespace Umbraco.Cms.Core.DependencyInjection .AddUmbracoOptions() .AddUmbracoOptions(); + builder.Services.Configure(options => + { + var userDefinedReplacements = new List(); + IEnumerable config = builder.Config + .GetSection( + $"{Constants.Configuration.ConfigRequestHandler}:{nameof(RequestHandlerSettings.CharCollection)}") + .GetChildren(); + + foreach (IConfigurationSection section in config) + { + var @char = section.GetValue(nameof(CharItem.Char)); + var replacement = section.GetValue(nameof(CharItem.Replacement)); + userDefinedReplacements.Add(new CharItem { Char = @char, Replacement = replacement }); + } + + options.UserDefinedCharCollection = userDefinedReplacements; + }); + return builder; } } diff --git a/src/Umbraco.Core/Extensions/RequestHandlerSettingsExtension.cs b/src/Umbraco.Core/Extensions/RequestHandlerSettingsExtension.cs new file mode 100644 index 0000000000..d54d00188a --- /dev/null +++ b/src/Umbraco.Core/Extensions/RequestHandlerSettingsExtension.cs @@ -0,0 +1,44 @@ +using System.Collections.Generic; +using System.Linq; +using Umbraco.Cms.Core.Configuration.Models; +using Umbraco.Cms.Core.Configuration.UmbracoSettings; + +namespace Umbraco.Extensions +{ + /// + /// Get concatenated user and default character replacements + /// taking into account + /// + public static class RequestHandlerSettingsExtension + { + public static IEnumerable GetCharReplacements(this RequestHandlerSettings requestHandlerSettings) + { + if (!requestHandlerSettings.EnableDefaultCharReplacements) + { + return requestHandlerSettings.UserDefinedCharCollection ?? Enumerable.Empty(); + } + + if (requestHandlerSettings.UserDefinedCharCollection == null || !requestHandlerSettings.UserDefinedCharCollection.Any()) + { + return RequestHandlerSettings.DefaultCharCollection; + } + + foreach (CharItem defaultReplacement in RequestHandlerSettings.DefaultCharCollection) + { + foreach (CharItem userReplacement in requestHandlerSettings.UserDefinedCharCollection) + { + if (userReplacement.Char == defaultReplacement.Char) + { + defaultReplacement.Replacement = userReplacement.Replacement; + } + } + } + + IEnumerable mergedCollections = + RequestHandlerSettings.DefaultCharCollection.Union( + requestHandlerSettings.UserDefinedCharCollection, new CharacterReplacementEqualityComparer()); + + return mergedCollections; + } + } +} diff --git a/src/Umbraco.Core/Strings/DefaultShortStringHelperConfig.cs b/src/Umbraco.Core/Strings/DefaultShortStringHelperConfig.cs index d5bab4afb1..b0f0a9b003 100644 --- a/src/Umbraco.Core/Strings/DefaultShortStringHelperConfig.cs +++ b/src/Umbraco.Core/Strings/DefaultShortStringHelperConfig.cs @@ -61,16 +61,7 @@ namespace Umbraco.Cms.Core.Strings /// The short string helper. public DefaultShortStringHelperConfig WithDefault(RequestHandlerSettings requestHandlerSettings) { - // CharCollection could potentially be null if not invoked first by the framework, for instance in tests, so ensure that it's initialized. - IEnumerable charCollection = requestHandlerSettings.CharCollection; - if (charCollection is null) - { - charCollection = requestHandlerSettings.CharCollection; - if (charCollection is null) - { - throw new ArgumentNullException(nameof(requestHandlerSettings.CharCollection)); - } - } + IEnumerable charCollection = requestHandlerSettings.GetCharReplacements(); UrlReplaceCharacters = charCollection .Where(x => string.IsNullOrEmpty(x.Char) == false) 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 d681d4a70e..f159ecbc85 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Configuration/Models/RequestHandlerSettingsTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Configuration/Models/RequestHandlerSettingsTests.cs @@ -1,7 +1,7 @@ using System.Linq; using NUnit.Framework; using Umbraco.Cms.Core.Configuration.Models; -using Umbraco.Cms.Core.Configuration.UmbracoSettings; +using Umbraco.Extensions; namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Configuration.Models { @@ -18,8 +18,8 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Configuration.Models }; - var settings = new RequestHandlerSettings { CharCollection = userCollection }; - var actual = settings.CharCollection.ToList(); + var settings = new RequestHandlerSettings { UserDefinedCharCollection = userCollection }; + var actual = settings.GetCharReplacements().ToList(); var expectedCollection = RequestHandlerSettings.DefaultCharCollection.ToList(); expectedCollection.AddRange(userCollection); @@ -37,8 +37,8 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Configuration.Models new () { Char = "test2", Replacement = "replace2" } }; - var settings = new RequestHandlerSettings { CharCollection = userCollection, EnableDefaultCharReplacements = false }; - var actual = settings.CharCollection.ToList(); + var settings = new RequestHandlerSettings { UserDefinedCharCollection = userCollection, EnableDefaultCharReplacements = false }; + var actual = settings.GetCharReplacements().ToList(); Assert.AreEqual(userCollection.Length, actual.Count); Assert.That(actual, Is.EquivalentTo(userCollection)); @@ -53,8 +53,8 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Configuration.Models new () { Char = ".", Replacement = "dot" } }; - var settings = new RequestHandlerSettings { CharCollection = userCollection }; - var actual = settings.CharCollection.ToList(); + var settings = new RequestHandlerSettings { UserDefinedCharCollection = userCollection }; + var actual = settings.GetCharReplacements().ToList(); Assert.AreEqual(RequestHandlerSettings.DefaultCharCollection.Length, actual.Count); @@ -74,8 +74,8 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Configuration.Models new () { Char = "new", Replacement = "new" } }; - var settings = new RequestHandlerSettings { CharCollection = userCollection }; - var actual = settings.CharCollection.ToList(); + var settings = new RequestHandlerSettings { UserDefinedCharCollection = userCollection }; + var actual = settings.GetCharReplacements().ToList(); // Add 1 to the length, because we're expecting to only add one new one Assert.AreEqual(RequestHandlerSettings.DefaultCharCollection.Length + 1, actual.Count);