HybridCache: richer detection for field-only types (ref STJ)

mgravell · mgravell · commit 4f5158f46616 · 2025-03-14T21:21:04.000Z
fix dotnet/aspnetcore#60934
diff --git 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;
 
@@ -35,7 +38,7 @@ public bool TryCreateSerializer<T>([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<JsonSerializerOptions>(typeof(IHybridCacheSerializer<T>)) ?? Options;
-        if (IsValueTuple(typeof(T)) && !options.IncludeFields)
+        if (!options.IncludeFields && IsFieldOnlyType(typeof(T)))
         {
             // value-tuples expose fields, not properties; special-case this as a common scenario
             options = FieldEnabledJsonOptions;
@@ -45,8 +48,90 @@ public bool TryCreateSerializer<T>([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<Type, FieldOnlyResult>? state = null; // only needed for complex types
+        return IsFieldOnlyType(type, ref state) == FieldOnlyResult.FieldOnly;
+    }
+
+    [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<Type, FieldOnlyResult>? 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<T>
+        // (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<Type, FieldOnlyResult>? state)
+        {
+            state ??= [];
+            if (!state.ContainsKey(type))
+            {
+                state.Add(type, FieldOnlyResult.Incomplete);
+            }
+        }
+
+        static FieldOnlyResult SetState(Type type, Dictionary<Type, FieldOnlyResult>? state, bool result)
+        {
+            var value = result ? FieldOnlyResult.FieldOnly : FieldOnlyResult.NotFieldOnly;
+            if (state is not null)
+            {
+                state[type] = value;
+            }
+
+            return value;
+        }
+    }
 
     internal sealed class DefaultJsonSerializer<T> : IHybridCacheSerializer<T>
     {
@@ -76,4 +161,11 @@ void IHybridCacheSerializer<T>.Serialize(T value, IBufferWriter<byte> 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
@@ -103,6 +103,118 @@ 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<Tuple<int, string>> 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<int, string>[] 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<string> source = new NodeA<string>
+        {
+            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);
+    }
+
+    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<T>
+    {
+        public NodeB<T>? Next { get; set; }
+
+        public T? Value { get; set; }
+    }
+
+    public class NodeB<T>
+    {
+        public NodeA<T>? Next { get; set; }
+
+        public T? Value { get; set; }
+    }
+
     private static T RoundTrip<T>(T value, ReadOnlySpan<byte> expectedBytes, JsonSerializer expectedJsonOptions, JsonSerializer addSerializers = JsonSerializer.None, bool binary = false)
     {
         var services = new ServiceCollection();
