mirror of
https://github.com/VSadov/Satori.git
synced 2025-06-11 18:20:26 +09:00
Add logic to properly honor naming policy when serializing flag enums (#36726)
* Add logic to properly honor naming policy when serializing flag enums * Cache JsonEncodedText, add optimization to-do, and use .Contains * Fix CI failure * Apply feedback, use enum value as lookup key, cache result even when naming policy is not used * Use ulong as name cache key and perform caching at warm up * Address feedback - set cap for name cache, compute cache key properly, and add more tests
This commit is contained in:
parent
5bdb598ab4
commit
f511ec456f
4 changed files with 245 additions and 39 deletions
|
@ -3,8 +3,10 @@
|
|||
// See the LICENSE file in the project root for more information.
|
||||
|
||||
using System.Collections.Concurrent;
|
||||
using System.Diagnostics;
|
||||
using System.Globalization;
|
||||
using System.Runtime.CompilerServices;
|
||||
using System.Text.Encodings.Web;
|
||||
|
||||
namespace System.Text.Json.Serialization.Converters
|
||||
{
|
||||
|
@ -16,32 +18,57 @@ namespace System.Text.Json.Serialization.Converters
|
|||
// Odd type codes are conveniently signed types (for enum backing types).
|
||||
private static readonly string? s_negativeSign = ((int)s_enumTypeCode % 2) == 0 ? null : NumberFormatInfo.CurrentInfo.NegativeSign;
|
||||
|
||||
private const string ValueSeparator = ", ";
|
||||
|
||||
private readonly EnumConverterOptions _converterOptions;
|
||||
private readonly JsonNamingPolicy _namingPolicy;
|
||||
private readonly ConcurrentDictionary<string, string>? _nameCache;
|
||||
|
||||
private readonly JsonNamingPolicy? _namingPolicy;
|
||||
|
||||
private readonly ConcurrentDictionary<ulong, JsonEncodedText> _nameCache;
|
||||
|
||||
// This is used to prevent flooding the cache due to exponential bitwise combinations of flags.
|
||||
// Since multiple threads can add to the cache, a few more values might be added.
|
||||
private const int NameCacheSizeSoftLimit = 64;
|
||||
|
||||
public override bool CanConvert(Type type)
|
||||
{
|
||||
return type.IsEnum;
|
||||
}
|
||||
|
||||
public EnumConverter(EnumConverterOptions options)
|
||||
: this(options, namingPolicy: null)
|
||||
public EnumConverter(EnumConverterOptions converterOptions, JsonSerializerOptions serializerOptions)
|
||||
: this(converterOptions, namingPolicy: null, serializerOptions)
|
||||
{
|
||||
}
|
||||
|
||||
public EnumConverter(EnumConverterOptions options, JsonNamingPolicy? namingPolicy)
|
||||
public EnumConverter(EnumConverterOptions converterOptions, JsonNamingPolicy? namingPolicy, JsonSerializerOptions serializerOptions)
|
||||
{
|
||||
_converterOptions = options;
|
||||
if (namingPolicy != null)
|
||||
{
|
||||
_nameCache = new ConcurrentDictionary<string, string>();
|
||||
}
|
||||
else
|
||||
{
|
||||
namingPolicy = JsonNamingPolicy.Default;
|
||||
}
|
||||
_converterOptions = converterOptions;
|
||||
_namingPolicy = namingPolicy;
|
||||
_nameCache = new ConcurrentDictionary<ulong, JsonEncodedText>();
|
||||
|
||||
string[] names = Enum.GetNames(TypeToConvert);
|
||||
Array values = Enum.GetValues(TypeToConvert);
|
||||
Debug.Assert(names.Length == values.Length);
|
||||
|
||||
JavaScriptEncoder? encoder = serializerOptions.Encoder;
|
||||
|
||||
for (int i = 0; i < names.Length; i++)
|
||||
{
|
||||
if (_nameCache.Count >= NameCacheSizeSoftLimit)
|
||||
{
|
||||
break;
|
||||
}
|
||||
|
||||
T value = (T)values.GetValue(i)!;
|
||||
ulong key = ConvertToUInt64(value);
|
||||
string name = names[i];
|
||||
|
||||
_nameCache.TryAdd(
|
||||
key,
|
||||
namingPolicy == null
|
||||
? JsonEncodedText.Encode(name, encoder)
|
||||
: FormatEnumValue(name, encoder));
|
||||
}
|
||||
}
|
||||
|
||||
public override T Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
|
||||
|
@ -138,40 +165,46 @@ namespace System.Text.Json.Serialization.Converters
|
|||
return default;
|
||||
}
|
||||
|
||||
private static bool IsValidIdentifier(string value)
|
||||
{
|
||||
// Trying to do this check efficiently. When an enum is converted to
|
||||
// string the underlying value is given if it can't find a matching
|
||||
// identifier (or identifiers in the case of flags).
|
||||
//
|
||||
// The underlying value will be given back with a digit (e.g. 0-9) possibly
|
||||
// preceded by a negative sign. Identifiers have to start with a letter
|
||||
// so we'll just pick the first valid one and check for a negative sign
|
||||
// if needed.
|
||||
return (value[0] >= 'A' &&
|
||||
(s_negativeSign == null || !value.StartsWith(s_negativeSign)));
|
||||
}
|
||||
|
||||
public override void Write(Utf8JsonWriter writer, T value, JsonSerializerOptions options)
|
||||
{
|
||||
// If strings are allowed, attempt to write it out as a string value
|
||||
if (_converterOptions.HasFlag(EnumConverterOptions.AllowStrings))
|
||||
{
|
||||
string original = value.ToString();
|
||||
if (_nameCache != null && _nameCache.TryGetValue(original, out string? transformed))
|
||||
ulong key = ConvertToUInt64(value);
|
||||
|
||||
if (_nameCache.TryGetValue(key, out JsonEncodedText formatted))
|
||||
{
|
||||
writer.WriteStringValue(transformed);
|
||||
writer.WriteStringValue(formatted);
|
||||
return;
|
||||
}
|
||||
|
||||
string original = value.ToString();
|
||||
if (IsValidIdentifier(original))
|
||||
{
|
||||
transformed = _namingPolicy.ConvertName(original);
|
||||
writer.WriteStringValue(transformed);
|
||||
if (_nameCache != null)
|
||||
// We are dealing with a combination of flag constants since
|
||||
// all constant values were cached during warm-up.
|
||||
JavaScriptEncoder? encoder = options.Encoder;
|
||||
|
||||
if (_nameCache.Count < NameCacheSizeSoftLimit)
|
||||
{
|
||||
_nameCache.TryAdd(original, transformed);
|
||||
formatted = _namingPolicy == null
|
||||
? JsonEncodedText.Encode(original, encoder)
|
||||
: FormatEnumValue(original, encoder);
|
||||
|
||||
writer.WriteStringValue(formatted);
|
||||
|
||||
_nameCache.TryAdd(key, formatted);
|
||||
}
|
||||
else
|
||||
{
|
||||
// We also do not create a JsonEncodedText instance here because passing the string
|
||||
// directly to the writer is cheaper than creating one and not caching it for reuse.
|
||||
writer.WriteStringValue(
|
||||
_namingPolicy == null
|
||||
? original
|
||||
: FormatEnumValueToString(original, encoder));
|
||||
}
|
||||
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
@ -212,5 +245,77 @@ namespace System.Text.Json.Serialization.Converters
|
|||
break;
|
||||
}
|
||||
}
|
||||
|
||||
// This method is adapted from Enum.ToUInt64 (an internal method):
|
||||
// https://github.com/dotnet/runtime/blob/bd6cbe3642f51d70839912a6a666e5de747ad581/src/libraries/System.Private.CoreLib/src/System/Enum.cs#L240-L260
|
||||
private static ulong ConvertToUInt64(object value)
|
||||
{
|
||||
Debug.Assert(value is T);
|
||||
ulong result = s_enumTypeCode switch
|
||||
{
|
||||
TypeCode.Int32 => (ulong)(int)value,
|
||||
TypeCode.UInt32 => (uint)value,
|
||||
TypeCode.UInt64 => (ulong)value,
|
||||
TypeCode.Int64 => (ulong)(long)value,
|
||||
TypeCode.SByte => (ulong)(sbyte)value,
|
||||
TypeCode.Byte => (byte)value,
|
||||
TypeCode.Int16 => (ulong)(short)value,
|
||||
TypeCode.UInt16 => (ushort)value,
|
||||
_ => throw new InvalidOperationException(),
|
||||
};
|
||||
return result;
|
||||
}
|
||||
|
||||
private static bool IsValidIdentifier(string value)
|
||||
{
|
||||
// Trying to do this check efficiently. When an enum is converted to
|
||||
// string the underlying value is given if it can't find a matching
|
||||
// identifier (or identifiers in the case of flags).
|
||||
//
|
||||
// The underlying value will be given back with a digit (e.g. 0-9) possibly
|
||||
// preceded by a negative sign. Identifiers have to start with a letter
|
||||
// so we'll just pick the first valid one and check for a negative sign
|
||||
// if needed.
|
||||
return (value[0] >= 'A' &&
|
||||
(s_negativeSign == null || !value.StartsWith(s_negativeSign)));
|
||||
}
|
||||
|
||||
private JsonEncodedText FormatEnumValue(string value, JavaScriptEncoder? encoder)
|
||||
{
|
||||
Debug.Assert(_namingPolicy != null);
|
||||
string formatted = FormatEnumValueToString(value, encoder);
|
||||
return JsonEncodedText.Encode(formatted, encoder);
|
||||
}
|
||||
|
||||
private string FormatEnumValueToString(string value, JavaScriptEncoder? encoder)
|
||||
{
|
||||
Debug.Assert(_namingPolicy != null);
|
||||
|
||||
string converted;
|
||||
if (!value.Contains(ValueSeparator))
|
||||
{
|
||||
converted = _namingPolicy.ConvertName(value);
|
||||
}
|
||||
else
|
||||
{
|
||||
// todo: optimize implementation here by leveraging https://github.com/dotnet/runtime/issues/934.
|
||||
string[] enumValues = value.Split(
|
||||
#if BUILDING_INBOX_LIBRARY
|
||||
ValueSeparator
|
||||
#else
|
||||
new string[] { ValueSeparator }, StringSplitOptions.None
|
||||
#endif
|
||||
);
|
||||
|
||||
for (int i = 0; i < enumValues.Length; i++)
|
||||
{
|
||||
enumValues[i] = _namingPolicy.ConvertName(enumValues[i]);
|
||||
}
|
||||
|
||||
converted = string.Join(ValueSeparator, enumValues);
|
||||
}
|
||||
|
||||
return converted;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -19,7 +19,7 @@ namespace System.Text.Json.Serialization.Converters
|
|||
}
|
||||
|
||||
[PreserveDependency(
|
||||
".ctor(System.Text.Json.Serialization.Converters.EnumConverterOptions)",
|
||||
".ctor(System.Text.Json.Serialization.Converters.EnumConverterOptions, System.Text.Json.JsonSerializerOptions)",
|
||||
"System.Text.Json.Serialization.Converters.EnumConverter`1")]
|
||||
public override JsonConverter CreateConverter(Type type, JsonSerializerOptions options)
|
||||
{
|
||||
|
@ -27,7 +27,7 @@ namespace System.Text.Json.Serialization.Converters
|
|||
typeof(EnumConverter<>).MakeGenericType(type),
|
||||
BindingFlags.Instance | BindingFlags.Public,
|
||||
binder: null,
|
||||
new object[] { EnumConverterOptions.AllowNumbers },
|
||||
new object[] { EnumConverterOptions.AllowNumbers, options },
|
||||
culture: null)!;
|
||||
|
||||
return converter;
|
||||
|
|
|
@ -55,7 +55,7 @@ namespace System.Text.Json.Serialization
|
|||
|
||||
/// <inheritdoc />
|
||||
[PreserveDependency(
|
||||
".ctor(System.Text.Json.Serialization.Converters.EnumConverterOptions, System.Text.Json.JsonNamingPolicy)",
|
||||
".ctor(System.Text.Json.Serialization.Converters.EnumConverterOptions, System.Text.Json.JsonNamingPolicy, System.Text.Json.JsonSerializerOptions)",
|
||||
"System.Text.Json.Serialization.Converters.EnumConverter`1")]
|
||||
public override JsonConverter CreateConverter(Type typeToConvert, JsonSerializerOptions options)
|
||||
{
|
||||
|
@ -63,7 +63,7 @@ namespace System.Text.Json.Serialization
|
|||
typeof(EnumConverter<>).MakeGenericType(typeToConvert),
|
||||
BindingFlags.Instance | BindingFlags.Public,
|
||||
binder: null,
|
||||
new object?[] { _converterOptions, _namingPolicy },
|
||||
new object?[] { _converterOptions, _namingPolicy, options },
|
||||
culture: null)!;
|
||||
|
||||
return converter;
|
||||
|
|
|
@ -3,6 +3,7 @@
|
|||
// See the LICENSE file in the project root for more information.
|
||||
|
||||
using System.IO;
|
||||
using System.Threading.Tasks;
|
||||
using Xunit;
|
||||
|
||||
namespace System.Text.Json.Serialization.Tests
|
||||
|
@ -107,6 +108,21 @@ namespace System.Text.Json.Serialization.Tests
|
|||
options = new JsonSerializerOptions();
|
||||
options.Converters.Add(new JsonStringEnumConverter(allowIntegerValues: false));
|
||||
Assert.Throws<JsonException>(() => JsonSerializer.Serialize((FileAttributes)(-1), options));
|
||||
|
||||
// Flag values honor naming policy correctly
|
||||
options = new JsonSerializerOptions();
|
||||
options.Converters.Add(new JsonStringEnumConverter(new SimpleSnakeCasePolicy()));
|
||||
|
||||
json = JsonSerializer.Serialize(
|
||||
FileAttributes.Directory | FileAttributes.Compressed | FileAttributes.IntegrityStream,
|
||||
options);
|
||||
Assert.Equal(@"""directory, compressed, integrity_stream""", json);
|
||||
|
||||
json = JsonSerializer.Serialize(FileAttributes.Compressed & FileAttributes.Device, options);
|
||||
Assert.Equal(@"0", json);
|
||||
|
||||
json = JsonSerializer.Serialize(FileAttributes.Directory & FileAttributes.Compressed | FileAttributes.IntegrityStream, options);
|
||||
Assert.Equal(@"""integrity_stream""", json);
|
||||
}
|
||||
|
||||
public class FileState
|
||||
|
@ -185,5 +201,90 @@ namespace System.Text.Json.Serialization.Tests
|
|||
obj = JsonSerializer.Deserialize<MyCustomEnum>("2");
|
||||
Assert.Equal(MyCustomEnum.Second, obj);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public static void EnumWithNoValues()
|
||||
{
|
||||
var options = new JsonSerializerOptions
|
||||
{
|
||||
Converters = { new JsonStringEnumConverter() }
|
||||
};
|
||||
|
||||
Assert.Equal("-1", JsonSerializer.Serialize((EmptyEnum)(-1), options));
|
||||
Assert.Equal("1", JsonSerializer.Serialize((EmptyEnum)(1), options));
|
||||
}
|
||||
|
||||
public enum EmptyEnum { };
|
||||
|
||||
[Fact]
|
||||
public static void MoreThan64EnumValuesToSerialize()
|
||||
{
|
||||
var options = new JsonSerializerOptions
|
||||
{
|
||||
Converters = { new JsonStringEnumConverter() }
|
||||
};
|
||||
|
||||
for (int i = 0; i < 128; i++)
|
||||
{
|
||||
MyEnum value = (MyEnum)i;
|
||||
string asStr = value.ToString();
|
||||
string expected = char.IsLetter(asStr[0]) ? $@"""{asStr}""" : asStr;
|
||||
Assert.Equal(expected, JsonSerializer.Serialize(value, options));
|
||||
}
|
||||
}
|
||||
|
||||
[Fact, OuterLoop]
|
||||
public static void VeryLargeAmountOfEnumsToSerialize()
|
||||
{
|
||||
// Ensure we don't throw OutOfMemoryException.
|
||||
// Multiple threads are used to ensure the approximate size limit
|
||||
// for the name cache(a concurrent dictionary) is honored.
|
||||
|
||||
var options = new JsonSerializerOptions
|
||||
{
|
||||
Converters = { new JsonStringEnumConverter() }
|
||||
};
|
||||
|
||||
const int MaxValue = 33554432; // Value for MyEnum.Z
|
||||
Task[] tasks = new Task[MaxValue];
|
||||
|
||||
for (int i = 0; i < tasks.Length; i++)
|
||||
{
|
||||
tasks[i] = Task.Run(() => JsonSerializer.Serialize((MyEnum)i, options));
|
||||
}
|
||||
|
||||
Task.WaitAll(tasks);
|
||||
}
|
||||
|
||||
[Flags]
|
||||
public enum MyEnum
|
||||
{
|
||||
A = 1 << 0,
|
||||
B = 1 << 1,
|
||||
C = 1 << 2,
|
||||
D = 1 << 3,
|
||||
E = 1 << 4,
|
||||
F = 1 << 5,
|
||||
G = 1 << 6,
|
||||
H = 1 << 7,
|
||||
I = 1 << 8,
|
||||
J = 1 << 9,
|
||||
K = 1 << 10,
|
||||
L = 1 << 11,
|
||||
M = 1 << 12,
|
||||
N = 1 << 13,
|
||||
O = 1 << 14,
|
||||
P = 1 << 15,
|
||||
Q = 1 << 16,
|
||||
R = 1 << 17,
|
||||
S = 1 << 18,
|
||||
T = 1 << 19,
|
||||
U = 1 << 20,
|
||||
V = 1 << 21,
|
||||
W = 1 << 22,
|
||||
X = 1 << 23,
|
||||
Y = 1 << 24,
|
||||
Z = 1 << 25,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue