diff --git a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultJsonSerializerFactory.cs b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultJsonSerializerFactory.cs index 43d36c868c7..f499ba485b3 100644 --- a/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultJsonSerializerFactory.cs +++ b/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultJsonSerializerFactory.cs @@ -3,7 +3,10 @@ using System; using System.Buffers; +using System.Collections; +using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; +using System.Reflection; using System.Text.Json; using Microsoft.Extensions.DependencyInjection; @@ -22,11 +25,7 @@ public DefaultJsonSerializerFactory(IServiceProvider serviceProvider) // store the service provider and obtain the default JSON options, keyed by the **open** generic interface type _serviceProvider = serviceProvider; -#pragma warning disable IDE0079 // unnecessary suppression: TFM-dependent -#pragma warning disable IL2026, IL3050 // AOT bits - Options = serviceProvider.GetKeyedService(typeof(IHybridCacheSerializer<>)) ?? JsonSerializerOptions.Default; -#pragma warning restore IL2026, IL3050 -#pragma warning restore IDE0079 + Options = serviceProvider.GetKeyedService(typeof(IHybridCacheSerializer<>)) ?? SystemDefaultJsonOptions; } public bool TryCreateSerializer([NotNullWhen(true)] out IHybridCacheSerializer? serializer) @@ -35,7 +34,7 @@ public bool TryCreateSerializer([NotNullWhen(true)] out IHybridCacheSerialize // see if there is a per-type options registered (keyed by the **closed** generic type), otherwise use the default JsonSerializerOptions options = _serviceProvider.GetKeyedService(typeof(IHybridCacheSerializer)) ?? Options; - if (IsValueTuple(typeof(T)) && !options.IncludeFields) + if (!options.IncludeFields && ReferenceEquals(options, SystemDefaultJsonOptions) && IsFieldOnlyType(typeof(T))) { // value-tuples expose fields, not properties; special-case this as a common scenario options = FieldEnabledJsonOptions; @@ -45,8 +44,96 @@ public bool TryCreateSerializer([NotNullWhen(true)] out IHybridCacheSerialize return true; } - private static bool IsValueTuple(Type type) - => type.IsValueType && (type.FullName ?? string.Empty).StartsWith("System.ValueTuple`", StringComparison.Ordinal); + internal static bool IsFieldOnlyType(Type type) + { + Dictionary? state = null; // only needed for complex types + return IsFieldOnlyType(type, ref state) == FieldOnlyResult.FieldOnly; + } + +#pragma warning disable IDE0079 // unnecessary suppression: TFM-dependent +#pragma warning disable IL2026, IL3050 // AOT bits + private static JsonSerializerOptions SystemDefaultJsonOptions => JsonSerializerOptions.Default; +#pragma warning restore IL2026, IL3050 +#pragma warning restore IDE0079 + + [SuppressMessage("Trimming", "IL2070:'this' argument does not satisfy 'DynamicallyAccessedMembersAttribute' in call to target method. The parameter of method does not have matching annotations.", + Justification = "Custom serializers may be needed for AOT with STJ")] + [SuppressMessage("Performance", "CA1864:Prefer the 'IDictionary.TryAdd(TKey, TValue)' method", Justification = "Not available in all platforms")] + private static FieldOnlyResult IsFieldOnlyType( + Type type, ref Dictionary? state) + { + if (type is null || type.IsPrimitive || type == typeof(string)) + { + return FieldOnlyResult.NotFieldOnly; + } + + // re-use existing results, and more importantly: prevent infinite recursion + if (state is not null && state.TryGetValue(type, out var existingResult)) + { + return existingResult; + } + + // check for collection types; start at IEnumerable and then look for IEnumerable + // (this is broadly comparable to STJ) + if (typeof(IEnumerable).IsAssignableFrom(type)) + { + PrepareStateForDepth(type, ref state); + foreach (var iType in type.GetInterfaces()) + { + if (iType.IsGenericType && iType.GetGenericTypeDefinition() == typeof(IEnumerable<>)) + { + if (IsFieldOnlyType(iType.GetGenericArguments()[0], ref state) == FieldOnlyResult.FieldOnly) + { + return SetState(type, state, true); + } + } + } + + // no problems detected + return SetState(type, state, false); + } + + // not a collection; check for field-only scenario - look for properties first + var props = type.GetProperties(BindingFlags.Public | BindingFlags.Instance); + if (props.Length != 0) + { + PrepareStateForDepth(type, ref state); + foreach (var prop in props) + { + if (IsFieldOnlyType(prop.PropertyType, ref state) == FieldOnlyResult.FieldOnly) + { + return SetState(type, state, true); + } + } + + // then we *do* have public instance properties, that aren't themselves problems; we're good + return SetState(type, state, false); + } + + // no properties; if there are fields, this is the problem scenario we're trying to detect + var haveFields = type.GetFields(BindingFlags.Public | BindingFlags.Instance).Length != 0; + return SetState(type, state, haveFields); + + static void PrepareStateForDepth(Type type, ref Dictionary? state) + { + state ??= []; + if (!state.ContainsKey(type)) + { + state.Add(type, FieldOnlyResult.Incomplete); + } + } + + static FieldOnlyResult SetState(Type type, Dictionary? state, bool result) + { + var value = result ? FieldOnlyResult.FieldOnly : FieldOnlyResult.NotFieldOnly; + if (state is not null) + { + state[type] = value; + } + + return value; + } + } internal sealed class DefaultJsonSerializer : IHybridCacheSerializer { @@ -76,4 +163,11 @@ void IHybridCacheSerializer.Serialize(T value, IBufferWriter target) #pragma warning restore IDE0079 } + // used to store intermediate state when calculating IsFieldOnlyType + private enum FieldOnlyResult + { + Incomplete = 0, + FieldOnly = 1, + NotFieldOnly = 2, + } } diff --git a/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/SerializerTests.cs b/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/SerializerTests.cs index 6a6dae76342..5153fc643a7 100644 --- a/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/SerializerTests.cs +++ b/test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/SerializerTests.cs @@ -81,9 +81,9 @@ public void RoundTripTuple(JsonSerializer addSerializers, JsonSerializer expecte [Theory] [InlineData(JsonSerializer.None, JsonSerializer.FieldEnabled)] - [InlineData(JsonSerializer.CustomGlobal, JsonSerializer.FieldEnabled)] - [InlineData(JsonSerializer.CustomPerType, JsonSerializer.FieldEnabled)] - [InlineData(JsonSerializer.CustomPerType | JsonSerializer.CustomGlobal, JsonSerializer.FieldEnabled)] + [InlineData(JsonSerializer.CustomGlobal, JsonSerializer.CustomGlobal)] + [InlineData(JsonSerializer.CustomPerType, JsonSerializer.CustomPerType)] + [InlineData(JsonSerializer.CustomPerType | JsonSerializer.CustomGlobal, JsonSerializer.CustomPerType)] public void RoundTripValueTuple(JsonSerializer addSerializers, JsonSerializer expectedSerializer) { var obj = RoundTrip((42, "abc"), """{"Item1":42,"Item2":"abc"}"""u8, expectedSerializer, addSerializers); @@ -93,9 +93,9 @@ public void RoundTripValueTuple(JsonSerializer addSerializers, JsonSerializer ex [Theory] [InlineData(JsonSerializer.None, JsonSerializer.FieldEnabled)] - [InlineData(JsonSerializer.CustomGlobal, JsonSerializer.FieldEnabled)] - [InlineData(JsonSerializer.CustomPerType, JsonSerializer.FieldEnabled)] - [InlineData(JsonSerializer.CustomPerType | JsonSerializer.CustomGlobal, JsonSerializer.FieldEnabled)] + [InlineData(JsonSerializer.CustomGlobal, JsonSerializer.CustomGlobal)] + [InlineData(JsonSerializer.CustomPerType, JsonSerializer.CustomPerType)] + [InlineData(JsonSerializer.CustomPerType | JsonSerializer.CustomGlobal, JsonSerializer.CustomPerType)] public void RoundTripNamedValueTuple(JsonSerializer addSerializers, JsonSerializer expectedSerializer) { var obj = RoundTrip((X: 42, Y: "abc"), """{"Item1":42,"Item2":"abc"}"""u8, expectedSerializer, addSerializers); @@ -103,6 +103,136 @@ public void RoundTripNamedValueTuple(JsonSerializer addSerializers, JsonSerializ Assert.Equal("abc", obj.Y); } + [Fact] + public void RoundTripValueTupleList() + { + List<(int, string)> source = [(1, "a"), (2, "b")]; + var clone = RoundTrip(source, """[{"Item1":1,"Item2":"a"},{"Item1":2,"Item2":"b"}]"""u8, JsonSerializer.FieldEnabled); + Assert.Equal(source, clone); + } + + [Fact] + public void RoundTripValueTupleArray() + { + (int, string)[] source = [(1, "a"), (2, "b")]; + var clone = RoundTrip(source, """[{"Item1":1,"Item2":"a"},{"Item1":2,"Item2":"b"}]"""u8, JsonSerializer.FieldEnabled); + Assert.Equal(source, clone); + } + + [Fact] + public void RoundTripTupleList() + { + List> source = [Tuple.Create(1, "a"), Tuple.Create(2, "b")]; + var clone = RoundTrip(source, """[{"Item1":1,"Item2":"a"},{"Item1":2,"Item2":"b"}]"""u8, JsonSerializer.Default); + Assert.Equal(source, clone); + } + + [Fact] + public void RoundTripTupleArray() + { + Tuple[] source = [Tuple.Create(1, "a"), Tuple.Create(2, "b")]; + var clone = RoundTrip(source, """[{"Item1":1,"Item2":"a"},{"Item1":2,"Item2":"b"}]"""u8, JsonSerializer.Default); + Assert.Equal(source, clone); + } + + [Fact] + public void RoundTripFieldOnlyPoco() + { + var source = new FieldOnlyPoco { X = 1, Y = "a" }; + var clone = RoundTrip(source, """{"X":1,"Y":"a"}"""u8, JsonSerializer.FieldEnabled); + Assert.Equal(1, clone.X); + Assert.Equal("a", clone.Y); + } + + [Fact] + public void RoundTripPropertyOnlyPoco() + { + var source = new PropertyOnlyPoco { X = 1, Y = "a" }; + var clone = RoundTrip(source, """{"X":1,"Y":"a"}"""u8, JsonSerializer.Default); + Assert.Equal(1, clone.X); + Assert.Equal("a", clone.Y); + } + + [Fact] + public void RoundTripMixedPoco() + { + // this is the self-inflicted scenario; intent isn't obvious, so: we defer to STJ conventions, + // which means we lose the field + var source = new MixedFieldPropertyPoco { X = 1, Y = "a" }; + var clone = RoundTrip(source, """{"Y":"a"}"""u8, JsonSerializer.Default); + Assert.Equal(0, clone.X); // <== drop + Assert.Equal("a", clone.Y); + } + + [Fact] + public void RoundTripTree() + { + NodeA source = new NodeA + { + Value = "abc", + Next = new() + { + Value = "def" + } + }; + + var clone = RoundTrip(source, """{"Next":{"Next":null,"Value":"def"},"Value":"abc"}"""u8, JsonSerializer.Default); + Assert.Equal("abc", clone.Value); + Assert.NotNull(clone.Next); + Assert.Equal("def", clone.Next.Value); + Assert.Null(clone.Next.Next); + } + + [Fact] + public void RoundTripDictionary() + { + Dictionary source = new() + { + ["x"] = (42, "Fred", new(2025, 03, 18)), + ["y"] = (43, "Barney", new(2025, 03, 22)), + }; + var clone = RoundTrip(source, + """{"x":{"Item1":42,"Item2":"Fred","Item3":"2025-03-18T00:00:00"},"y":{"Item1":43,"Item2":"Barney","Item3":"2025-03-22T00:00:00"}}"""u8, + JsonSerializer.FieldEnabled); + Assert.Equal(2, clone.Count); + Assert.True(clone.TryGetValue("x", out var val)); + Assert.Equal(source["x"], val); + Assert.True(clone.TryGetValue("y", out val)); + Assert.Equal(source["y"], val); + } + + public class FieldOnlyPoco + { + public int X; + public string? Y; + } + + public class PropertyOnlyPoco + { + public int X { get; set; } + public string? Y { get; set; } + } + + public class MixedFieldPropertyPoco + { + public int X; // field + public string? Y { get; set; } // property + } + + public class NodeA + { + public NodeB? Next { get; set; } + + public T? Value { get; set; } + } + + public class NodeB + { + public NodeA? Next { get; set; } + + public T? Value { get; set; } + } + private static T RoundTrip(T value, ReadOnlySpan expectedBytes, JsonSerializer expectedJsonOptions, JsonSerializer addSerializers = JsonSerializer.None, bool binary = false) { var services = new ServiceCollection(); @@ -112,13 +242,13 @@ private static T RoundTrip(T value, ReadOnlySpan expectedBytes, JsonSer if ((addSerializers & JsonSerializer.CustomGlobal) != JsonSerializer.None) { - globalOptions = new(); + globalOptions = new() { IncludeFields = true }; // assume any custom options will serialize the whole type services.AddKeyedSingleton(typeof(IHybridCacheSerializer<>), globalOptions); } if ((addSerializers & JsonSerializer.CustomPerType) != JsonSerializer.None) { - perTypeOptions = new(); + perTypeOptions = new() { IncludeFields = true }; // assume any custom options will serialize the whole type services.AddKeyedSingleton(typeof(IHybridCacheSerializer), perTypeOptions); }