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
This commit is contained in:
Mole
2021-12-22 11:29:04 +01:00
parent c6d28f01a8
commit 53e5a25df8
5 changed files with 81 additions and 74 deletions

View File

@@ -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<CharItem> _charCollection;
/// <summary>
/// Add additional character replacements, or override defaults
/// </summary>
[Obsolete("Use the GetCharReplacements extension method in the Umbraco.Extensions namespace instead. Scheduled for removal in V11")]
public IEnumerable<IChar> CharCollection { get; set; } = DefaultCharCollection;
/// <summary>
/// Add additional character replacements, or override defaults
/// </summary>
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>
private IEnumerable<CharItem> GetCharReplacements()
{
if (!EnableDefaultCharReplacements)
{
return _charCollection ?? Enumerable.Empty<CharItem>();
}
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<CharItem> mergedCollections = DefaultCharCollection.Union<CharItem>(_charCollection, new CharacterReplacementEqualityComparer());
return mergedCollections;
}
public IEnumerable<CharItem> UserDefinedCharCollection { get; set; }
}
}

View File

@@ -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<LegacyPasswordMigrationSettings>()
.AddUmbracoOptions<PackageMigrationSettings>();
builder.Services.Configure<RequestHandlerSettings>(options =>
{
var userDefinedReplacements = new List<CharItem>();
IEnumerable<IConfigurationSection> config = builder.Config
.GetSection(
$"{Constants.Configuration.ConfigRequestHandler}:{nameof(RequestHandlerSettings.CharCollection)}")
.GetChildren();
foreach (IConfigurationSection section in config)
{
var @char = section.GetValue<string>(nameof(CharItem.Char));
var replacement = section.GetValue<string>(nameof(CharItem.Replacement));
userDefinedReplacements.Add(new CharItem { Char = @char, Replacement = replacement });
}
options.UserDefinedCharCollection = userDefinedReplacements;
});
return builder;
}
}

View File

@@ -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
{
/// <summary>
/// Get concatenated user and default character replacements
/// taking into account <see cref="RequestHandlerSettings.EnableDefaultCharReplacements"/>
/// </summary>
public static class RequestHandlerSettingsExtension
{
public static IEnumerable<CharItem> GetCharReplacements(this RequestHandlerSettings requestHandlerSettings)
{
if (!requestHandlerSettings.EnableDefaultCharReplacements)
{
return requestHandlerSettings.UserDefinedCharCollection ?? Enumerable.Empty<CharItem>();
}
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<CharItem> mergedCollections =
RequestHandlerSettings.DefaultCharCollection.Union<CharItem>(
requestHandlerSettings.UserDefinedCharCollection, new CharacterReplacementEqualityComparer());
return mergedCollections;
}
}
}

View File

@@ -61,16 +61,7 @@ namespace Umbraco.Cms.Core.Strings
/// <returns>The short string helper.</returns>
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<IChar> charCollection = requestHandlerSettings.CharCollection;
if (charCollection is null)
{
charCollection = requestHandlerSettings.CharCollection;
if (charCollection is null)
{
throw new ArgumentNullException(nameof(requestHandlerSettings.CharCollection));
}
}
IEnumerable<IChar> charCollection = requestHandlerSettings.GetCharReplacements();
UrlReplaceCharacters = charCollection
.Where(x => string.IsNullOrEmpty(x.Char) == false)

View File

@@ -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);