From c2ed6a629bcc46af66c5a7a578281115b27d05f9 Mon Sep 17 00:00:00 2001 From: Matthew Care Date: Fri, 15 Oct 2021 23:22:01 +0100 Subject: [PATCH 01/14] Update request handler settings `CharCollection` didn't map correctly from the config, updated to an array so that it does Add logic to concatenate user and default replacements, replacing defaults with user defined if present. Added additional option to disable the default replacements --- .../Models/RequestHandlerSettings.cs | 114 ++++++++++-------- 1 file changed, 62 insertions(+), 52 deletions(-) diff --git a/src/Umbraco.Core/Configuration/Models/RequestHandlerSettings.cs b/src/Umbraco.Core/Configuration/Models/RequestHandlerSettings.cs index ee223b36c6..79dabf9da0 100644 --- a/src/Umbraco.Core/Configuration/Models/RequestHandlerSettings.cs +++ b/src/Umbraco.Core/Configuration/Models/RequestHandlerSettings.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.ComponentModel; +using System.Linq; using Umbraco.Cms.Core.Configuration.UmbracoSettings; using Umbraco.Extensions; @@ -16,33 +17,34 @@ namespace Umbraco.Cms.Core.Configuration.Models { internal const bool StaticAddTrailingSlash = true; internal const string StaticConvertUrlsToAscii = "try"; + internal const bool StaticEnableDefaultCharReplacements = true; - internal static readonly CharItem[] DefaultCharCollection = + internal static readonly CharacterReplacement[] DefaultCharCollection = { - new CharItem { Char = " ", Replacement = "-" }, - new CharItem { Char = "\"", Replacement = string.Empty }, - new CharItem { Char = "'", Replacement = string.Empty }, - new CharItem { Char = "%", Replacement = string.Empty }, - new CharItem { Char = ".", Replacement = string.Empty }, - new CharItem { Char = ";", Replacement = string.Empty }, - new CharItem { Char = "/", Replacement = string.Empty }, - new CharItem { Char = "\\", Replacement = string.Empty }, - new CharItem { Char = ":", Replacement = string.Empty }, - new CharItem { Char = "#", Replacement = string.Empty }, - new CharItem { Char = "+", Replacement = "plus" }, - new CharItem { Char = "*", Replacement = "star" }, - new CharItem { Char = "&", Replacement = string.Empty }, - new CharItem { Char = "?", Replacement = string.Empty }, - new CharItem { Char = "æ", Replacement = "ae" }, - new CharItem { Char = "ä", Replacement = "ae" }, - new CharItem { Char = "ø", Replacement = "oe" }, - new CharItem { Char = "ö", Replacement = "oe" }, - new CharItem { Char = "å", Replacement = "aa" }, - new CharItem { Char = "ü", Replacement = "ue" }, - new CharItem { Char = "ß", Replacement = "ss" }, - new CharItem { Char = "|", Replacement = "-" }, - new CharItem { Char = "<", Replacement = string.Empty }, - new CharItem { Char = ">", Replacement = string.Empty } + new () { Char = " ", Replacement = "-" }, + new () { Char = "\"", Replacement = string.Empty }, + new () { Char = "'", Replacement = string.Empty }, + new () { Char = "%", Replacement = string.Empty }, + new () { Char = ".", Replacement = string.Empty }, + new () { Char = ";", Replacement = string.Empty }, + new () { Char = "/", Replacement = string.Empty }, + new () { Char = "\\", Replacement = string.Empty }, + new () { Char = ":", Replacement = string.Empty }, + new () { Char = "#", Replacement = string.Empty }, + new () { Char = "+", Replacement = "plus" }, + new () { Char = "*", Replacement = "star" }, + new () { Char = "&", Replacement = string.Empty }, + new () { Char = "?", Replacement = string.Empty }, + new () { Char = "æ", Replacement = "ae" }, + new () { Char = "ä", Replacement = "ae" }, + new () { Char = "ø", Replacement = "oe" }, + new () { Char = "ö", Replacement = "oe" }, + new () { Char = "å", Replacement = "aa" }, + new () { Char = "ü", Replacement = "ue" }, + new () { Char = "ß", Replacement = "ss" }, + new () { Char = "|", Replacement = "-" }, + new () { Char = "<", Replacement = string.Empty }, + new () { Char = ">", Replacement = string.Empty } }; /// @@ -67,41 +69,49 @@ namespace Umbraco.Cms.Core.Configuration.Models /// public bool ShouldTryConvertUrlsToAscii => ConvertUrlsToAscii.InvariantEquals("try"); - // We need to special handle ":", as this character is special in keys - - // TODO: implement from configuration - - //// var collection = _configuration.GetSection(Prefix + "CharCollection").GetChildren() - //// .Select(x => new CharItem() - //// { - //// Char = x.GetValue("Char"), - //// Replacement = x.GetValue("Replacement"), - //// }).ToArray(); - - //// if (collection.Any() || _configuration.GetSection("Prefix").GetChildren().Any(x => - //// x.Key.Equals("CharCollection", StringComparison.OrdinalIgnoreCase))) - //// { - //// return collection; - //// } - - //// return DefaultCharCollection; + /// + /// Disable all default character replacements + /// + [DefaultValue(StaticEnableDefaultCharReplacements)] + public bool EnableDefaultCharReplacements { get; set; } = StaticEnableDefaultCharReplacements; /// - /// Gets or sets a value for the default character collection for replacements. + /// Add additional character replacements, or override defaults /// - /// WB-TODO - public IEnumerable CharCollection { get; set; } = DefaultCharCollection; + public CharacterReplacement[] CharCollection { get; set; } /// - /// Defines a character replacement. + /// Get concatenated user and default character replacements + /// taking into account /// - public class CharItem : IChar + public IEnumerable GetCharReplacements() { - /// - public string Char { get; set; } + // TODO We need to special handle ":", as this character is special in keys - /// - public string Replacement { get; set; } + if (!EnableDefaultCharReplacements) + { + return CharCollection; + } + + if (CharCollection == null || !CharCollection.Any()) + { + return DefaultCharCollection; + } + + foreach (var defaultReplacement in DefaultCharCollection) + { + foreach (var userReplacement in CharCollection) + { + if (userReplacement.Char == defaultReplacement.Char) + { + defaultReplacement.Replacement = userReplacement.Replacement; + } + } + } + + var mergedCollections = DefaultCharCollection.Union(CharCollection, new CharacterReplacementEqualityComparer()); + + return mergedCollections; } } } From 1ee4e379e797ef95e579c0c72476e3ac98898a7f Mon Sep 17 00:00:00 2001 From: Matthew Care Date: Fri, 15 Oct 2021 23:23:07 +0100 Subject: [PATCH 02/14] Update classes Update class names, and location --- .../UmbracoSettings/CharacterReplacement.cs | 15 +++++++ .../CharacterReplacementEqualityComparer.cs | 40 +++++++++++++++++++ .../Configuration/UmbracoSettings/IChar.cs | 8 ---- 3 files changed, 55 insertions(+), 8 deletions(-) create mode 100644 src/Umbraco.Core/Configuration/UmbracoSettings/CharacterReplacement.cs create mode 100644 src/Umbraco.Core/Configuration/UmbracoSettings/CharacterReplacementEqualityComparer.cs delete mode 100644 src/Umbraco.Core/Configuration/UmbracoSettings/IChar.cs diff --git a/src/Umbraco.Core/Configuration/UmbracoSettings/CharacterReplacement.cs b/src/Umbraco.Core/Configuration/UmbracoSettings/CharacterReplacement.cs new file mode 100644 index 0000000000..ed2f97dba9 --- /dev/null +++ b/src/Umbraco.Core/Configuration/UmbracoSettings/CharacterReplacement.cs @@ -0,0 +1,15 @@ +namespace Umbraco.Cms.Core.Configuration.UmbracoSettings +{ + public class CharacterReplacement + { + /// + /// The character to replace + /// + public string Char { get; set; } + + /// + /// The replacement character + /// + public string Replacement { get; set; } + } +} diff --git a/src/Umbraco.Core/Configuration/UmbracoSettings/CharacterReplacementEqualityComparer.cs b/src/Umbraco.Core/Configuration/UmbracoSettings/CharacterReplacementEqualityComparer.cs new file mode 100644 index 0000000000..b7dbf1cd16 --- /dev/null +++ b/src/Umbraco.Core/Configuration/UmbracoSettings/CharacterReplacementEqualityComparer.cs @@ -0,0 +1,40 @@ +using System.Collections.Generic; + +namespace Umbraco.Cms.Core.Configuration.UmbracoSettings +{ + public class CharacterReplacementEqualityComparer : IEqualityComparer + { + public bool Equals(CharacterReplacement x, CharacterReplacement y) + { + if (ReferenceEquals(x, y)) + { + return true; + } + + if (x is null) + { + return false; + } + + if (y is null) + { + return false; + } + + if (x.GetType() != y.GetType()) + { + return false; + } + + return x.Char == y.Char && x.Replacement == y.Replacement; + } + + public int GetHashCode(CharacterReplacement obj) + { + unchecked + { + return ((obj.Char != null ? obj.Char.GetHashCode() : 0) * 397) ^ (obj.Replacement != null ? obj.Replacement.GetHashCode() : 0); + } + } + } +} diff --git a/src/Umbraco.Core/Configuration/UmbracoSettings/IChar.cs b/src/Umbraco.Core/Configuration/UmbracoSettings/IChar.cs deleted file mode 100644 index 4073a12149..0000000000 --- a/src/Umbraco.Core/Configuration/UmbracoSettings/IChar.cs +++ /dev/null @@ -1,8 +0,0 @@ -namespace Umbraco.Cms.Core.Configuration.UmbracoSettings -{ - public interface IChar - { - string Char { get; } - string Replacement { get; } - } -} From 12e89a01ba52ae3ea74c3b0de3dfe05d1fe13983 Mon Sep 17 00:00:00 2001 From: Matthew Care Date: Fri, 15 Oct 2021 23:23:56 +0100 Subject: [PATCH 03/14] Update usage Update string helper to use new method that uses user defined replacements --- src/Umbraco.Core/Strings/DefaultShortStringHelperConfig.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Core/Strings/DefaultShortStringHelperConfig.cs b/src/Umbraco.Core/Strings/DefaultShortStringHelperConfig.cs index cf5e71a568..287d33dd58 100644 --- a/src/Umbraco.Core/Strings/DefaultShortStringHelperConfig.cs +++ b/src/Umbraco.Core/Strings/DefaultShortStringHelperConfig.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Linq; using Umbraco.Cms.Core.Configuration.Models; @@ -60,7 +60,7 @@ namespace Umbraco.Cms.Core.Strings /// The short string helper. public DefaultShortStringHelperConfig WithDefault(RequestHandlerSettings requestHandlerSettings) { - UrlReplaceCharacters = requestHandlerSettings.CharCollection + UrlReplaceCharacters = requestHandlerSettings.GetCharReplacements() .Where(x => string.IsNullOrEmpty(x.Char) == false) .ToDictionary(x => x.Char, x => x.Replacement); From 25b1c3c0788eb1dbbd068aeef9e5a67741d50498 Mon Sep 17 00:00:00 2001 From: Matthew Care Date: Fri, 15 Oct 2021 23:24:21 +0100 Subject: [PATCH 04/14] Add tests Add new tests for the request handler settings, and fix other tests --- .../Models/RequestHandlerSettingsTests.cs | 90 +++++++++++++++++++ ...faultShortStringHelperTestsWithoutSetup.cs | 7 +- 2 files changed, 94 insertions(+), 3 deletions(-) create mode 100644 src/Umbraco.Tests.UnitTests/Umbraco.Core/Configuration/Models/RequestHandlerSettingsTests.cs diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Core/Configuration/Models/RequestHandlerSettingsTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Core/Configuration/Models/RequestHandlerSettingsTests.cs new file mode 100644 index 0000000000..c16d9b4897 --- /dev/null +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Core/Configuration/Models/RequestHandlerSettingsTests.cs @@ -0,0 +1,90 @@ +using System.Linq; +using NUnit.Framework; +using Umbraco.Cms.Core.Configuration.Models; +using Umbraco.Cms.Core.Configuration.UmbracoSettings; + +namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Configuration.Models +{ + [TestFixture] + public class RequestHandlerSettingsTests + { + [Test] + public void Given_CharCollection_With_DefaultEnabled_MergesCollection() + { + var userCollection = new CharacterReplacement[] + { + new() { Char = "test", Replacement = "replace" }, + new() { Char = "test2", Replacement = "replace2" } + }; + + + var settings = new RequestHandlerSettings { CharCollection = userCollection }; + var actual = settings.GetCharReplacements().ToList(); + + var expectedCollection = RequestHandlerSettings.DefaultCharCollection.ToList(); + expectedCollection.AddRange(userCollection); + + Assert.AreEqual(expectedCollection.Count, actual.Count); + Assert.That(actual, Is.EquivalentTo(expectedCollection)); + } + + [Test] + public void Given_CharCollection_With_DefaultDisabled_ReturnsUserCollection() + { + var userCollection = new CharacterReplacement[] + { + new() { Char = "test", Replacement = "replace" }, + new() { Char = "test2", Replacement = "replace2" } + }; + + var settings = new RequestHandlerSettings { CharCollection = userCollection, EnableDefaultCharReplacements = false }; + var actual = settings.GetCharReplacements().ToList(); + + Assert.AreEqual(userCollection.Length, actual.Count); + Assert.That(actual, Is.EquivalentTo(userCollection)); + } + + [Test] + public void Given_CharCollection_That_OverridesDefaultValues_ReturnsReplacements() + { + var userCollection = new CharacterReplacement[] + { + new() { Char = "%", Replacement = "percent" }, + new() { Char = ".", Replacement = "dot" } + }; + + var settings = new RequestHandlerSettings { CharCollection = userCollection }; + var actual = settings.GetCharReplacements().ToList(); + + Assert.AreEqual(RequestHandlerSettings.DefaultCharCollection.Length, actual.Count); + + Assert.That(actual, Has.Exactly(1).Matches(x => x.Char == "%" && x.Replacement == "percent")); + Assert.That(actual, Has.Exactly(1).Matches(x => x.Char == "." && x.Replacement == "dot")); + Assert.That(actual, Has.Exactly(0).Matches(x => x.Char == "%" && x.Replacement == string.Empty)); + Assert.That(actual, Has.Exactly(0).Matches(x => x.Char == "." && x.Replacement == string.Empty)); + } + + [Test] + public void Given_CharCollection_That_OverridesDefaultValues_And_ContainsNew_ReturnsMergedWithReplacements() + { + var userCollection = new CharacterReplacement[] + { + new() { Char = "%", Replacement = "percent" }, + new() { Char = ".", Replacement = "dot" }, + new() {Char = "new", Replacement = "new"} + }; + + var settings = new RequestHandlerSettings { CharCollection = 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); + + Assert.That(actual, Has.Exactly(1).Matches(x => x.Char == "%" && x.Replacement == "percent")); + Assert.That(actual, Has.Exactly(1).Matches(x => x.Char == "." && x.Replacement == "dot")); + Assert.That(actual, Has.Exactly(1).Matches(x => x.Char == "new" && x.Replacement == "new")); + Assert.That(actual, Has.Exactly(0).Matches(x => x.Char == "%" && x.Replacement == string.Empty)); + Assert.That(actual, Has.Exactly(0).Matches(x => x.Char == "." && x.Replacement == string.Empty)); + } + } +} diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Core/ShortStringHelper/DefaultShortStringHelperTestsWithoutSetup.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Core/ShortStringHelper/DefaultShortStringHelperTestsWithoutSetup.cs index 6f9ee481cc..5aa8198e07 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Core/ShortStringHelper/DefaultShortStringHelperTestsWithoutSetup.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Core/ShortStringHelper/DefaultShortStringHelperTestsWithoutSetup.cs @@ -1,6 +1,7 @@ // Copyright (c) Umbraco. // See LICENSE for more details. +using System; using System.Diagnostics; using System.Linq; using System.Text; @@ -19,7 +20,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.ShortStringHelper { var requestHandlerSettings = new RequestHandlerSettings() { - CharCollection = Enumerable.Empty(), + CharCollection = Array.Empty(), ConvertUrlsToAscii = "false" }; @@ -45,7 +46,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.ShortStringHelper { var requestHandlerSettings = new RequestHandlerSettings() { - CharCollection = Enumerable.Empty(), + CharCollection = Array.Empty(), ConvertUrlsToAscii = "false" }; @@ -339,7 +340,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.ShortStringHelper { var requestHandlerSettings = new RequestHandlerSettings() { - CharCollection = Enumerable.Empty(), + CharCollection = Array.Empty(), ConvertUrlsToAscii = "false" }; From 98474e111062b48f6d344b165503f98f8704438a Mon Sep 17 00:00:00 2001 From: Matthew Care Date: Fri, 15 Oct 2021 23:52:26 +0100 Subject: [PATCH 05/14] Fix broken test Update tests to disable the default replacements --- .../DefaultShortStringHelperTestsWithoutSetup.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Core/ShortStringHelper/DefaultShortStringHelperTestsWithoutSetup.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Core/ShortStringHelper/DefaultShortStringHelperTestsWithoutSetup.cs index 5aa8198e07..ee7999cac1 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Core/ShortStringHelper/DefaultShortStringHelperTestsWithoutSetup.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Core/ShortStringHelper/DefaultShortStringHelperTestsWithoutSetup.cs @@ -21,6 +21,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.ShortStringHelper var requestHandlerSettings = new RequestHandlerSettings() { CharCollection = Array.Empty(), + EnableDefaultCharReplacements = false, ConvertUrlsToAscii = "false" }; @@ -47,6 +48,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.ShortStringHelper var requestHandlerSettings = new RequestHandlerSettings() { CharCollection = Array.Empty(), + EnableDefaultCharReplacements = false, ConvertUrlsToAscii = "false" }; @@ -341,6 +343,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.ShortStringHelper var requestHandlerSettings = new RequestHandlerSettings() { CharCollection = Array.Empty(), + EnableDefaultCharReplacements = false, ConvertUrlsToAscii = "false" }; From 8a8b1e3247e6a79e8ca60e6a883305d2a2038bb5 Mon Sep 17 00:00:00 2001 From: Mole Date: Tue, 21 Dec 2021 10:30:19 +0100 Subject: [PATCH 06/14] Rename CharacterReplacement back to CharItem Since the class is public renaming this would be a breaking change, since if anyone is directly referring to the class it would break --- .../CharacterReplacement.cs => Models/CharItem.cs} | 4 ++-- .../Configuration/Models/RequestHandlerSettings.cs | 6 +++--- .../CharacterReplacementEqualityComparer.cs | 7 ++++--- .../DefaultShortStringHelperTestsWithoutSetup.cs | 6 +++--- 4 files changed, 12 insertions(+), 11 deletions(-) rename src/Umbraco.Core/Configuration/{UmbracoSettings/CharacterReplacement.cs => Models/CharItem.cs} (74%) diff --git a/src/Umbraco.Core/Configuration/UmbracoSettings/CharacterReplacement.cs b/src/Umbraco.Core/Configuration/Models/CharItem.cs similarity index 74% rename from src/Umbraco.Core/Configuration/UmbracoSettings/CharacterReplacement.cs rename to src/Umbraco.Core/Configuration/Models/CharItem.cs index ed2f97dba9..9a1178a1bf 100644 --- a/src/Umbraco.Core/Configuration/UmbracoSettings/CharacterReplacement.cs +++ b/src/Umbraco.Core/Configuration/Models/CharItem.cs @@ -1,6 +1,6 @@ -namespace Umbraco.Cms.Core.Configuration.UmbracoSettings +namespace Umbraco.Cms.Core.Configuration.Models { - public class CharacterReplacement + public class CharItem { /// /// The character to replace diff --git a/src/Umbraco.Core/Configuration/Models/RequestHandlerSettings.cs b/src/Umbraco.Core/Configuration/Models/RequestHandlerSettings.cs index 79dabf9da0..105712396d 100644 --- a/src/Umbraco.Core/Configuration/Models/RequestHandlerSettings.cs +++ b/src/Umbraco.Core/Configuration/Models/RequestHandlerSettings.cs @@ -19,7 +19,7 @@ namespace Umbraco.Cms.Core.Configuration.Models internal const string StaticConvertUrlsToAscii = "try"; internal const bool StaticEnableDefaultCharReplacements = true; - internal static readonly CharacterReplacement[] DefaultCharCollection = + internal static readonly CharItem[] DefaultCharCollection = { new () { Char = " ", Replacement = "-" }, new () { Char = "\"", Replacement = string.Empty }, @@ -78,13 +78,13 @@ namespace Umbraco.Cms.Core.Configuration.Models /// /// Add additional character replacements, or override defaults /// - public CharacterReplacement[] CharCollection { get; set; } + public CharItem[] CharCollection { get; set; } /// /// Get concatenated user and default character replacements /// taking into account /// - public IEnumerable GetCharReplacements() + public IEnumerable GetCharReplacements() { // TODO We need to special handle ":", as this character is special in keys diff --git a/src/Umbraco.Core/Configuration/UmbracoSettings/CharacterReplacementEqualityComparer.cs b/src/Umbraco.Core/Configuration/UmbracoSettings/CharacterReplacementEqualityComparer.cs index b7dbf1cd16..7a830e9655 100644 --- a/src/Umbraco.Core/Configuration/UmbracoSettings/CharacterReplacementEqualityComparer.cs +++ b/src/Umbraco.Core/Configuration/UmbracoSettings/CharacterReplacementEqualityComparer.cs @@ -1,10 +1,11 @@ using System.Collections.Generic; +using Umbraco.Cms.Core.Configuration.Models; namespace Umbraco.Cms.Core.Configuration.UmbracoSettings { - public class CharacterReplacementEqualityComparer : IEqualityComparer + public class CharacterReplacementEqualityComparer : IEqualityComparer { - public bool Equals(CharacterReplacement x, CharacterReplacement y) + public bool Equals(CharItem x, CharItem y) { if (ReferenceEquals(x, y)) { @@ -29,7 +30,7 @@ namespace Umbraco.Cms.Core.Configuration.UmbracoSettings return x.Char == y.Char && x.Replacement == y.Replacement; } - public int GetHashCode(CharacterReplacement obj) + public int GetHashCode(CharItem obj) { unchecked { diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/ShortStringHelper/DefaultShortStringHelperTestsWithoutSetup.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/ShortStringHelper/DefaultShortStringHelperTestsWithoutSetup.cs index ee7999cac1..b686aee278 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/ShortStringHelper/DefaultShortStringHelperTestsWithoutSetup.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/ShortStringHelper/DefaultShortStringHelperTestsWithoutSetup.cs @@ -20,7 +20,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.ShortStringHelper { var requestHandlerSettings = new RequestHandlerSettings() { - CharCollection = Array.Empty(), + CharCollection = Array.Empty(), EnableDefaultCharReplacements = false, ConvertUrlsToAscii = "false" }; @@ -47,7 +47,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.ShortStringHelper { var requestHandlerSettings = new RequestHandlerSettings() { - CharCollection = Array.Empty(), + CharCollection = Array.Empty(), EnableDefaultCharReplacements = false, ConvertUrlsToAscii = "false" }; @@ -342,7 +342,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.ShortStringHelper { var requestHandlerSettings = new RequestHandlerSettings() { - CharCollection = Array.Empty(), + CharCollection = Array.Empty(), EnableDefaultCharReplacements = false, ConvertUrlsToAscii = "false" }; From 338bbdd3825f89b9c5e6835313c8012be2b80b9a Mon Sep 17 00:00:00 2001 From: Mole Date: Tue, 21 Dec 2021 10:36:25 +0100 Subject: [PATCH 07/14] Reintroduce IChar interface Removing a public interface is a breaking change, in case someone is implementing the interface for some reason. --- src/Umbraco.Core/Configuration/Models/CharItem.cs | 4 +++- .../Configuration/Models/RequestHandlerSettings.cs | 6 +++--- .../CharacterReplacementEqualityComparer.cs | 6 +++--- src/Umbraco.Core/Configuration/UmbracoSettings/IChar.cs | 9 +++++++++ 4 files changed, 18 insertions(+), 7 deletions(-) create mode 100644 src/Umbraco.Core/Configuration/UmbracoSettings/IChar.cs diff --git a/src/Umbraco.Core/Configuration/Models/CharItem.cs b/src/Umbraco.Core/Configuration/Models/CharItem.cs index 9a1178a1bf..e269e0a83e 100644 --- a/src/Umbraco.Core/Configuration/Models/CharItem.cs +++ b/src/Umbraco.Core/Configuration/Models/CharItem.cs @@ -1,6 +1,8 @@ +using Umbraco.Cms.Core.Configuration.UmbracoSettings; + namespace Umbraco.Cms.Core.Configuration.Models { - public class CharItem + public class CharItem : IChar { /// /// The character to replace diff --git a/src/Umbraco.Core/Configuration/Models/RequestHandlerSettings.cs b/src/Umbraco.Core/Configuration/Models/RequestHandlerSettings.cs index 105712396d..3c76af7252 100644 --- a/src/Umbraco.Core/Configuration/Models/RequestHandlerSettings.cs +++ b/src/Umbraco.Core/Configuration/Models/RequestHandlerSettings.cs @@ -78,13 +78,13 @@ namespace Umbraco.Cms.Core.Configuration.Models /// /// Add additional character replacements, or override defaults /// - public CharItem[] CharCollection { get; set; } + public IEnumerable CharCollection { get; set; } /// /// Get concatenated user and default character replacements /// taking into account /// - public IEnumerable GetCharReplacements() + public IEnumerable GetCharReplacements() { // TODO We need to special handle ":", as this character is special in keys @@ -109,7 +109,7 @@ namespace Umbraco.Cms.Core.Configuration.Models } } - var mergedCollections = DefaultCharCollection.Union(CharCollection, new CharacterReplacementEqualityComparer()); + var mergedCollections = DefaultCharCollection.Union(CharCollection, new CharacterReplacementEqualityComparer()); return mergedCollections; } diff --git a/src/Umbraco.Core/Configuration/UmbracoSettings/CharacterReplacementEqualityComparer.cs b/src/Umbraco.Core/Configuration/UmbracoSettings/CharacterReplacementEqualityComparer.cs index 7a830e9655..a916febb93 100644 --- a/src/Umbraco.Core/Configuration/UmbracoSettings/CharacterReplacementEqualityComparer.cs +++ b/src/Umbraco.Core/Configuration/UmbracoSettings/CharacterReplacementEqualityComparer.cs @@ -3,9 +3,9 @@ using Umbraco.Cms.Core.Configuration.Models; namespace Umbraco.Cms.Core.Configuration.UmbracoSettings { - public class CharacterReplacementEqualityComparer : IEqualityComparer + public class CharacterReplacementEqualityComparer : IEqualityComparer { - public bool Equals(CharItem x, CharItem y) + public bool Equals(IChar x, IChar y) { if (ReferenceEquals(x, y)) { @@ -30,7 +30,7 @@ namespace Umbraco.Cms.Core.Configuration.UmbracoSettings return x.Char == y.Char && x.Replacement == y.Replacement; } - public int GetHashCode(CharItem obj) + public int GetHashCode(IChar obj) { unchecked { diff --git a/src/Umbraco.Core/Configuration/UmbracoSettings/IChar.cs b/src/Umbraco.Core/Configuration/UmbracoSettings/IChar.cs new file mode 100644 index 0000000000..61e840245c --- /dev/null +++ b/src/Umbraco.Core/Configuration/UmbracoSettings/IChar.cs @@ -0,0 +1,9 @@ +namespace Umbraco.Cms.Core.Configuration.UmbracoSettings +{ + public interface IChar + { + string Char { get; } + + string Replacement { get; } + } +} From d77e198d3144f21eb48034eafd63b9c7b4251ba0 Mon Sep 17 00:00:00 2001 From: Mole Date: Tue, 21 Dec 2021 10:42:54 +0100 Subject: [PATCH 08/14] Move unit test to the new location --- .../Models/RequestHandlerSettingsTests.cs | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) rename {src => tests}/Umbraco.Tests.UnitTests/Umbraco.Core/Configuration/Models/RequestHandlerSettingsTests.cs (57%) diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Core/Configuration/Models/RequestHandlerSettingsTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Configuration/Models/RequestHandlerSettingsTests.cs similarity index 57% rename from src/Umbraco.Tests.UnitTests/Umbraco.Core/Configuration/Models/RequestHandlerSettingsTests.cs rename to tests/Umbraco.Tests.UnitTests/Umbraco.Core/Configuration/Models/RequestHandlerSettingsTests.cs index c16d9b4897..9f2368d462 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Core/Configuration/Models/RequestHandlerSettingsTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Configuration/Models/RequestHandlerSettingsTests.cs @@ -11,7 +11,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Configuration.Models [Test] public void Given_CharCollection_With_DefaultEnabled_MergesCollection() { - var userCollection = new CharacterReplacement[] + var userCollection = new CharItem[] { new() { Char = "test", Replacement = "replace" }, new() { Char = "test2", Replacement = "replace2" } @@ -31,10 +31,10 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Configuration.Models [Test] public void Given_CharCollection_With_DefaultDisabled_ReturnsUserCollection() { - var userCollection = new CharacterReplacement[] + 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, EnableDefaultCharReplacements = false }; @@ -47,10 +47,10 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Configuration.Models [Test] public void Given_CharCollection_That_OverridesDefaultValues_ReturnsReplacements() { - var userCollection = new CharacterReplacement[] + var userCollection = new CharItem[] { - new() { Char = "%", Replacement = "percent" }, - new() { Char = ".", Replacement = "dot" } + new () { Char = "%", Replacement = "percent" }, + new () { Char = ".", Replacement = "dot" } }; var settings = new RequestHandlerSettings { CharCollection = userCollection }; @@ -58,20 +58,20 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Configuration.Models Assert.AreEqual(RequestHandlerSettings.DefaultCharCollection.Length, actual.Count); - Assert.That(actual, Has.Exactly(1).Matches(x => x.Char == "%" && x.Replacement == "percent")); - Assert.That(actual, Has.Exactly(1).Matches(x => x.Char == "." && x.Replacement == "dot")); - Assert.That(actual, Has.Exactly(0).Matches(x => x.Char == "%" && x.Replacement == string.Empty)); - Assert.That(actual, Has.Exactly(0).Matches(x => x.Char == "." && x.Replacement == string.Empty)); + Assert.That(actual, Has.Exactly(1).Matches(x => x.Char == "%" && x.Replacement == "percent")); + Assert.That(actual, Has.Exactly(1).Matches(x => x.Char == "." && x.Replacement == "dot")); + Assert.That(actual, Has.Exactly(0).Matches(x => x.Char == "%" && x.Replacement == string.Empty)); + Assert.That(actual, Has.Exactly(0).Matches(x => x.Char == "." && x.Replacement == string.Empty)); } [Test] public void Given_CharCollection_That_OverridesDefaultValues_And_ContainsNew_ReturnsMergedWithReplacements() { - var userCollection = new CharacterReplacement[] + var userCollection = new CharItem[] { - new() { Char = "%", Replacement = "percent" }, - new() { Char = ".", Replacement = "dot" }, - new() {Char = "new", Replacement = "new"} + new () { Char = "%", Replacement = "percent" }, + new () { Char = ".", Replacement = "dot" }, + new () { Char = "new", Replacement = "new" } }; var settings = new RequestHandlerSettings { CharCollection = userCollection }; @@ -80,11 +80,11 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Configuration.Models // Add 1 to the length, because we're expecting to only add one new one Assert.AreEqual(RequestHandlerSettings.DefaultCharCollection.Length + 1, actual.Count); - Assert.That(actual, Has.Exactly(1).Matches(x => x.Char == "%" && x.Replacement == "percent")); - Assert.That(actual, Has.Exactly(1).Matches(x => x.Char == "." && x.Replacement == "dot")); - Assert.That(actual, Has.Exactly(1).Matches(x => x.Char == "new" && x.Replacement == "new")); - Assert.That(actual, Has.Exactly(0).Matches(x => x.Char == "%" && x.Replacement == string.Empty)); - Assert.That(actual, Has.Exactly(0).Matches(x => x.Char == "." && x.Replacement == string.Empty)); + Assert.That(actual, Has.Exactly(1).Matches(x => x.Char == "%" && x.Replacement == "percent")); + Assert.That(actual, Has.Exactly(1).Matches(x => x.Char == "." && x.Replacement == "dot")); + Assert.That(actual, Has.Exactly(1).Matches(x => x.Char == "new" && x.Replacement == "new")); + Assert.That(actual, Has.Exactly(0).Matches(x => x.Char == "%" && x.Replacement == string.Empty)); + Assert.That(actual, Has.Exactly(0).Matches(x => x.Char == "." && x.Replacement == string.Empty)); } } } From 6147375a3e7d133c196c13216bc966018dfcd817 Mon Sep 17 00:00:00 2001 From: Mole Date: Tue, 21 Dec 2021 11:56:33 +0100 Subject: [PATCH 09/14] Return CharItem instead of IChar from CharCollection On second thought this is not a breaking change since CharItem implements IChar, so you could still do something lke IEnumerable chars = requestHandlerSettings.CharCollection --- .../Configuration/Models/RequestHandlerSettings.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Umbraco.Core/Configuration/Models/RequestHandlerSettings.cs b/src/Umbraco.Core/Configuration/Models/RequestHandlerSettings.cs index 3c76af7252..c27ad63092 100644 --- a/src/Umbraco.Core/Configuration/Models/RequestHandlerSettings.cs +++ b/src/Umbraco.Core/Configuration/Models/RequestHandlerSettings.cs @@ -78,13 +78,13 @@ namespace Umbraco.Cms.Core.Configuration.Models /// /// Add additional character replacements, or override defaults /// - public IEnumerable CharCollection { get; set; } + public IEnumerable CharCollection { get; set; } /// /// Get concatenated user and default character replacements /// taking into account /// - public IEnumerable GetCharReplacements() + public IEnumerable GetCharReplacements() { // TODO We need to special handle ":", as this character is special in keys @@ -109,7 +109,7 @@ namespace Umbraco.Cms.Core.Configuration.Models } } - var mergedCollections = DefaultCharCollection.Union(CharCollection, new CharacterReplacementEqualityComparer()); + var mergedCollections = DefaultCharCollection.Union(CharCollection, new CharacterReplacementEqualityComparer()); return mergedCollections; } From 80cda3364b09c3241e6284761dc416e79ef8fe71 Mon Sep 17 00:00:00 2001 From: Mole Date: Tue, 21 Dec 2021 13:35:30 +0100 Subject: [PATCH 10/14] 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); From 040116c004d5d16d553914193729b3e70f9216b7 Mon Sep 17 00:00:00 2001 From: Mole Date: Tue, 21 Dec 2021 14:48:56 +0100 Subject: [PATCH 11/14] Remove TODO With some testing this seems to be a none-issue, since we don't operate on the keys themselves --- src/Umbraco.Core/Configuration/Models/RequestHandlerSettings.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Umbraco.Core/Configuration/Models/RequestHandlerSettings.cs b/src/Umbraco.Core/Configuration/Models/RequestHandlerSettings.cs index 4894fb7125..a38f152f3b 100644 --- a/src/Umbraco.Core/Configuration/Models/RequestHandlerSettings.cs +++ b/src/Umbraco.Core/Configuration/Models/RequestHandlerSettings.cs @@ -108,7 +108,6 @@ namespace Umbraco.Cms.Core.Configuration.Models /// private IEnumerable GetCharReplacements() { - // TODO We need to special handle ":", as this character is special in keys if (!EnableDefaultCharReplacements) { return _charCollection ?? Enumerable.Empty(); From c6d28f01a8d2afba588b265529bc5549d4c5dd29 Mon Sep 17 00:00:00 2001 From: Mole Date: Tue, 21 Dec 2021 15:15:53 +0100 Subject: [PATCH 12/14] Check for null in DefaultShortStringHelperConfig.WithDefault --- .../Strings/DefaultShortStringHelperConfig.cs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/Umbraco.Core/Strings/DefaultShortStringHelperConfig.cs b/src/Umbraco.Core/Strings/DefaultShortStringHelperConfig.cs index 273d7a6562..d5bab4afb1 100644 --- a/src/Umbraco.Core/Strings/DefaultShortStringHelperConfig.cs +++ b/src/Umbraco.Core/Strings/DefaultShortStringHelperConfig.cs @@ -2,6 +2,7 @@ using System; using System.Collections.Generic; using System.Linq; using Umbraco.Cms.Core.Configuration.Models; +using Umbraco.Cms.Core.Configuration.UmbracoSettings; using Umbraco.Extensions; namespace Umbraco.Cms.Core.Strings @@ -60,7 +61,18 @@ namespace Umbraco.Cms.Core.Strings /// The short string helper. public DefaultShortStringHelperConfig WithDefault(RequestHandlerSettings requestHandlerSettings) { - UrlReplaceCharacters = requestHandlerSettings.CharCollection + // 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)); + } + } + + UrlReplaceCharacters = charCollection .Where(x => string.IsNullOrEmpty(x.Char) == false) .ToDictionary(x => x.Char, x => x.Replacement); From 53e5a25df87ef8b8bce1f5cd32d8f5d652b582c9 Mon Sep 17 00:00:00 2001 From: Mole Date: Wed, 22 Dec 2021 11:29:04 +0100 Subject: [PATCH 13/14] 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); From 04d20269ab51addc0f127750a9ddfe4109690b21 Mon Sep 17 00:00:00 2001 From: Mole Date: Wed, 22 Dec 2021 13:02:13 +0100 Subject: [PATCH 14/14] Merge CharCollection and UserDefinedCharCollection Otherwise only CharCollection would work, and that's obsolete, now you can use either, but UserDefinedCharCollection takes priority --- .../UmbracoBuilder.Configuration.cs | 19 +---- .../RequestHandlerSettingsExtension.cs | 74 ++++++++++++++++--- 2 files changed, 65 insertions(+), 28 deletions(-) diff --git a/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Configuration.cs b/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Configuration.cs index 8043698b6e..d1a8542688 100644 --- a/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Configuration.cs +++ b/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Configuration.cs @@ -6,6 +6,7 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; using Umbraco.Cms.Core.Configuration.Models; using Umbraco.Cms.Core.Configuration.Models.Validation; +using Umbraco.Extensions; namespace Umbraco.Cms.Core.DependencyInjection { @@ -78,23 +79,7 @@ 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; - }); + builder.Services.Configure(options => options.MergeReplacements(builder.Config)); return builder; } diff --git a/src/Umbraco.Core/Extensions/RequestHandlerSettingsExtension.cs b/src/Umbraco.Core/Extensions/RequestHandlerSettingsExtension.cs index d54d00188a..e9e6618f8c 100644 --- a/src/Umbraco.Core/Extensions/RequestHandlerSettingsExtension.cs +++ b/src/Umbraco.Core/Extensions/RequestHandlerSettingsExtension.cs @@ -1,5 +1,7 @@ using System.Collections.Generic; using System.Linq; +using Microsoft.Extensions.Configuration; +using Umbraco.Cms.Core; using Umbraco.Cms.Core.Configuration.Models; using Umbraco.Cms.Core.Configuration.UmbracoSettings; @@ -11,34 +13,84 @@ namespace Umbraco.Extensions /// public static class RequestHandlerSettingsExtension { + /// + /// Get concatenated user and default character replacements + /// taking into account + /// public static IEnumerable GetCharReplacements(this RequestHandlerSettings requestHandlerSettings) { - if (!requestHandlerSettings.EnableDefaultCharReplacements) + if (requestHandlerSettings.EnableDefaultCharReplacements is false) { return requestHandlerSettings.UserDefinedCharCollection ?? Enumerable.Empty(); } - if (requestHandlerSettings.UserDefinedCharCollection == null || !requestHandlerSettings.UserDefinedCharCollection.Any()) + if (requestHandlerSettings.UserDefinedCharCollection == null || requestHandlerSettings.UserDefinedCharCollection.Any() is false) { return RequestHandlerSettings.DefaultCharCollection; } - foreach (CharItem defaultReplacement in RequestHandlerSettings.DefaultCharCollection) + return MergeUnique(requestHandlerSettings.UserDefinedCharCollection, RequestHandlerSettings.DefaultCharCollection); + } + + /// + /// Merges CharCollection and UserDefinedCharCollection, prioritizing UserDefinedCharCollection + /// + internal static void MergeReplacements(this RequestHandlerSettings requestHandlerSettings, IConfiguration configuration) + { + string sectionKey = $"{Constants.Configuration.ConfigRequestHandler}:"; + + IEnumerable charCollection = GetReplacements( + configuration, + $"{sectionKey}{nameof(RequestHandlerSettings.CharCollection)}"); + + IEnumerable userDefinedCharCollection = GetReplacements( + configuration, + $"{sectionKey}{nameof(requestHandlerSettings.UserDefinedCharCollection)}"); + + IEnumerable mergedCollection = MergeUnique(userDefinedCharCollection, charCollection); + + requestHandlerSettings.UserDefinedCharCollection = mergedCollection; + } + + private static IEnumerable GetReplacements(IConfiguration configuration, string key) + { + var replacements = new List(); + IEnumerable config = configuration.GetSection(key).GetChildren(); + + foreach (IConfigurationSection section in config) { - foreach (CharItem userReplacement in requestHandlerSettings.UserDefinedCharCollection) + var @char = section.GetValue(nameof(CharItem.Char)); + var replacement = section.GetValue(nameof(CharItem.Replacement)); + replacements.Add(new CharItem { Char = @char, Replacement = replacement }); + } + + return replacements; + } + + /// + /// Merges two IEnumerable of CharItem without any duplicates, items in priorityReplacements will override those in alternativeReplacements + /// + private static IEnumerable MergeUnique( + IEnumerable priorityReplacements, + IEnumerable alternativeReplacements) + { + var priorityReplacementsList = priorityReplacements.ToList(); + var alternativeReplacementsList = alternativeReplacements.ToList(); + + foreach (CharItem alternativeReplacement in alternativeReplacementsList) + { + foreach (CharItem priorityReplacement in priorityReplacementsList) { - if (userReplacement.Char == defaultReplacement.Char) + if (priorityReplacement.Char == alternativeReplacement.Char) { - defaultReplacement.Replacement = userReplacement.Replacement; + alternativeReplacement.Replacement = priorityReplacement.Replacement; } } } - IEnumerable mergedCollections = - RequestHandlerSettings.DefaultCharCollection.Union( - requestHandlerSettings.UserDefinedCharCollection, new CharacterReplacementEqualityComparer()); - - return mergedCollections; + return priorityReplacementsList.Union( + alternativeReplacementsList, + new CharacterReplacementEqualityComparer()); } } }