Skip to content

Commit 53e5a25

Browse files
nikolajlauridsennikolajlauridsen
committed
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
1 parent c6d28f0 commit 53e5a25

File tree

5 files changed

+79
-72
lines changed

5 files changed

+79
-72
lines changed

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

Lines changed: 5 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
// Copyright (c) Umbraco.
22
// See LICENSE for more details.
33

4+
using System;
45
using System.Collections.Generic;
56
using System.ComponentModel;
6-
using System.Linq;
77
using Umbraco.Cms.Core.Configuration.UmbracoSettings;
88
using Umbraco.Extensions;
99

@@ -75,63 +75,15 @@ public class RequestHandlerSettings
7575
[DefaultValue(StaticEnableDefaultCharReplacements)]
7676
public bool EnableDefaultCharReplacements { get; set; } = StaticEnableDefaultCharReplacements;
7777

78-
private IEnumerable<CharItem> _charCollection;
79-
8078
/// <summary>
8179
/// Add additional character replacements, or override defaults
8280
/// </summary>
83-
public IEnumerable<CharItem> CharCollection
84-
{
85-
get
86-
{
87-
// This is pretty ugly, but necessary.
88-
// Essentially the config binding will only run properly if we return null the first time this is invoked.
89-
// Otherwise whatever we return will just be used and user specific bindings won't overwrite the existing ones.
90-
// However the next time this get is invoked, for instance in DefaultShortStringHelper we want to actually run the GetCharReplacements
91-
// To make sure that the default bindings is used if configured to do so.
92-
// Therefore we set _charCollection to be something, and still return null, to trick dotnet to actually read the config.
93-
if (_charCollection is null)
94-
{
95-
_charCollection = Enumerable.Empty<CharItem>();
96-
return null;
97-
}
98-
99-
return GetCharReplacements();
100-
}
101-
102-
set => _charCollection = value;
103-
}
81+
[Obsolete("Use the GetCharReplacements extension method in the Umbraco.Extensions namespace instead. Scheduled for removal in V11")]
82+
public IEnumerable<IChar> CharCollection { get; set; } = DefaultCharCollection;
10483

10584
/// <summary>
106-
/// Get concatenated user and default character replacements
107-
/// taking into account <see cref="EnableDefaultCharReplacements"/>
85+
/// Add additional character replacements, or override defaults
10886
/// </summary>
109-
private IEnumerable<CharItem> GetCharReplacements()
110-
{
111-
if (!EnableDefaultCharReplacements)
112-
{
113-
return _charCollection ?? Enumerable.Empty<CharItem>();
114-
}
115-
116-
if (_charCollection == null || !_charCollection.Any())
117-
{
118-
return DefaultCharCollection;
119-
}
120-
121-
foreach (CharItem defaultReplacement in DefaultCharCollection)
122-
{
123-
foreach (CharItem userReplacement in _charCollection)
124-
{
125-
if (userReplacement.Char == defaultReplacement.Char)
126-
{
127-
defaultReplacement.Replacement = userReplacement.Replacement;
128-
}
129-
}
130-
}
131-
132-
IEnumerable<CharItem> mergedCollections = DefaultCharCollection.Union<CharItem>(_charCollection, new CharacterReplacementEqualityComparer());
133-
134-
return mergedCollections;
135-
}
87+
public IEnumerable<CharItem> UserDefinedCharCollection { get; set; }
13688
}
13789
}

‎src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Configuration.cs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
using System;
2+
using System.Collections.Generic;
23
using System.Reflection;
4+
using Microsoft.Extensions.Configuration;
35
using Microsoft.Extensions.DependencyInjection;
46
using Microsoft.Extensions.Options;
57
using Umbraco.Cms.Core.Configuration.Models;
@@ -76,6 +78,24 @@ public static IUmbracoBuilder AddConfiguration(this IUmbracoBuilder builder)
7678
.AddUmbracoOptions<LegacyPasswordMigrationSettings>()
7779
.AddUmbracoOptions<PackageMigrationSettings>();
7880

81+
builder.Services.Configure<RequestHandlerSettings>(options =>
82+
{
83+
var userDefinedReplacements = new List<CharItem>();
84+
IEnumerable<IConfigurationSection> config = builder.Config
85+
.GetSection(
86+
$"{Constants.Configuration.ConfigRequestHandler}:{nameof(RequestHandlerSettings.CharCollection)}")
87+
.GetChildren();
88+
89+
foreach (IConfigurationSection section in config)
90+
{
91+
var @char = section.GetValue<string>(nameof(CharItem.Char));
92+
var replacement = section.GetValue<string>(nameof(CharItem.Replacement));
93+
userDefinedReplacements.Add(new CharItem { Char = @char, Replacement = replacement });
94+
}
95+
96+
options.UserDefinedCharCollection = userDefinedReplacements;
97+
});
98+
7999
return builder;
80100
}
81101
}

‎src/Umbraco.Core/Extensions/RequestHandlerSettingsExtension.cs

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
using System.Collections.Generic;
2+
using System.Linq;
3+
using Umbraco.Cms.Core.Configuration.Models;
4+
using Umbraco.Cms.Core.Configuration.UmbracoSettings;
5+
6+
namespace Umbraco.Extensions
7+
{
8+
/// <summary>
9+
/// Get concatenated user and default character replacements
10+
/// taking into account <see cref="RequestHandlerSettings.EnableDefaultCharReplacements"/>
11+
/// </summary>
12+
public static class RequestHandlerSettingsExtension
13+
{
14+
public static IEnumerable<CharItem> GetCharReplacements(this RequestHandlerSettings requestHandlerSettings)
15+
{
16+
if (!requestHandlerSettings.EnableDefaultCharReplacements)
17+
{
18+
return requestHandlerSettings.UserDefinedCharCollection ?? Enumerable.Empty<CharItem>();
19+
}
20+
21+
if (requestHandlerSettings.UserDefinedCharCollection == null || !requestHandlerSettings.UserDefinedCharCollection.Any())
22+
{
23+
return RequestHandlerSettings.DefaultCharCollection;
24+
}
25+
26+
foreach (CharItem defaultReplacement in RequestHandlerSettings.DefaultCharCollection)
27+
{
28+
foreach (CharItem userReplacement in requestHandlerSettings.UserDefinedCharCollection)
29+
{
30+
if (userReplacement.Char == defaultReplacement.Char)
31+
{
32+
defaultReplacement.Replacement = userReplacement.Replacement;
33+
}
34+
}
35+
}
36+
37+
IEnumerable<CharItem> mergedCollections =
38+
RequestHandlerSettings.DefaultCharCollection.Union<CharItem>(
39+
requestHandlerSettings.UserDefinedCharCollection, new CharacterReplacementEqualityComparer());
40+
41+
return mergedCollections;
42+
}
43+
}
44+
}

‎src/Umbraco.Core/Strings/DefaultShortStringHelperConfig.cs

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -61,16 +61,7 @@ public DefaultShortStringHelperConfig WithConfig(string culture, CleanStringType
6161
/// <returns>The short string helper.</returns>
6262
public DefaultShortStringHelperConfig WithDefault(RequestHandlerSettings requestHandlerSettings)
6363
{
64-
// CharCollection could potentially be null if not invoked first by the framework, for instance in tests, so ensure that it's initialized.
65-
IEnumerable<IChar> charCollection = requestHandlerSettings.CharCollection;
66-
if (charCollection is null)
67-
{
68-
charCollection = requestHandlerSettings.CharCollection;
69-
if (charCollection is null)
70-
{
71-
throw new ArgumentNullException(nameof(requestHandlerSettings.CharCollection));
72-
}
73-
}
64+
IEnumerable<IChar> charCollection = requestHandlerSettings.GetCharReplacements();
7465

7566
UrlReplaceCharacters = charCollection
7667
.Where(x => string.IsNullOrEmpty(x.Char) == false)

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

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
using System.Linq;
22
using NUnit.Framework;
33
using Umbraco.Cms.Core.Configuration.Models;
4-
using Umbraco.Cms.Core.Configuration.UmbracoSettings;
4+
using Umbraco.Extensions;
55

66
namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Configuration.Models
77
{
@@ -18,8 +18,8 @@ public void Given_CharCollection_With_DefaultEnabled_MergesCollection()
1818
};
1919

2020

21-
var settings = new RequestHandlerSettings { CharCollection = userCollection };
22-
var actual = settings.CharCollection.ToList();
21+
var settings = new RequestHandlerSettings { UserDefinedCharCollection = userCollection };
22+
var actual = settings.GetCharReplacements().ToList();
2323

2424
var expectedCollection = RequestHandlerSettings.DefaultCharCollection.ToList();
2525
expectedCollection.AddRange(userCollection);
@@ -37,8 +37,8 @@ public void Given_CharCollection_With_DefaultDisabled_ReturnsUserCollection()
3737
new () { Char = "test2", Replacement = "replace2" }
3838
};
3939

40-
var settings = new RequestHandlerSettings { CharCollection = userCollection, EnableDefaultCharReplacements = false };
41-
var actual = settings.CharCollection.ToList();
40+
var settings = new RequestHandlerSettings { UserDefinedCharCollection = userCollection, EnableDefaultCharReplacements = false };
41+
var actual = settings.GetCharReplacements().ToList();
4242

4343
Assert.AreEqual(userCollection.Length, actual.Count);
4444
Assert.That(actual, Is.EquivalentTo(userCollection));
@@ -53,8 +53,8 @@ public void Given_CharCollection_That_OverridesDefaultValues_ReturnsReplacements
5353
new () { Char = ".", Replacement = "dot" }
5454
};
5555

56-
var settings = new RequestHandlerSettings { CharCollection = userCollection };
57-
var actual = settings.CharCollection.ToList();
56+
var settings = new RequestHandlerSettings { UserDefinedCharCollection = userCollection };
57+
var actual = settings.GetCharReplacements().ToList();
5858

5959
Assert.AreEqual(RequestHandlerSettings.DefaultCharCollection.Length, actual.Count);
6060

@@ -74,8 +74,8 @@ public void Given_CharCollection_That_OverridesDefaultValues_And_ContainsNew_Ret
7474
new () { Char = "new", Replacement = "new" }
7575
};
7676

77-
var settings = new RequestHandlerSettings { CharCollection = userCollection };
78-
var actual = settings.CharCollection.ToList();
77+
var settings = new RequestHandlerSettings { UserDefinedCharCollection = userCollection };
78+
var actual = settings.GetCharReplacements().ToList();
7979

8080
// Add 1 to the length, because we're expecting to only add one new one
8181
Assert.AreEqual(RequestHandlerSettings.DefaultCharCollection.Length + 1, actual.Count);

0 commit comments

Comments
 (0)