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

mgravell · web-flow · commit 354bf8da3d48 · 2025-03-18T15:46:27.000Z
* HybridCache: richer detection for field-only types (ref STJ) fix dotnet/aspnetcore#60934 * as per PR notes: only apply field-only test versus the *default* options instance * add dictionary test
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;
 
@@ -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<JsonSerializerOptions>(typeof(IHybridCacheSerializer<>)) ?? JsonSerializerOptions.Default;
-#pragma warning restore IL2026, IL3050
-#pragma warning restore IDE0079
+        Options = serviceProvider.GetKeyedService<JsonSerializerOptions>(typeof(IHybridCacheSerializer<>)) ?? SystemDefaultJsonOptions;
     }
 
     public bool TryCreateSerializer<T>([NotNullWhen(true)] out IHybridCacheSerializer<T>? serializer)
@@ -35,7 +34,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 && 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<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;
+    }
+
+#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<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 +163,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
@@ -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,16 +93,146 @@ 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);
         Assert.Equal(42, obj.X);
         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);
+    }
+
+    [Fact]
+    public void RoundTripDictionary()
+    {
+        Dictionary<string, (int id, string who, DateTime when)> 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<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();
@@ -112,13 +242,13 @@ private static T RoundTrip<T>(T value, ReadOnlySpan<byte> 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<JsonSerializerOptions>(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<JsonSerializerOptions>(typeof(IHybridCacheSerializer<T>), perTypeOptions);
         }
 
