Skip to content

Commit 49aec59

Browse files
authored
Root built-in converters in JsonSerializerOptions.GetConverter(Type) (#51897)
* Root built-in converters in JsonSerializerOptions.GetConverter(Type) * Address review feedback * Clean up changes * Mark JsonSerializerOptions.GetConverter(Type) as 'requires unreferenced code' * Address review feedback
1 parent 7f034d3 commit 49aec59

11 files changed

Lines changed: 136 additions & 17 deletions

File tree

src/libraries/System.Text.Json/ref/System.Text.Json.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,7 @@ public JsonSerializerOptions(System.Text.Json.JsonSerializerOptions options) { }
274274
public System.Text.Json.Serialization.JsonUnknownTypeHandling UnknownTypeHandling { get { throw null; } set { } }
275275
public bool WriteIndented { get { throw null; } set { } }
276276
public void AddContext<TContext>() where TContext : System.Text.Json.Serialization.JsonSerializerContext, new() { }
277+
[System.Diagnostics.CodeAnalysis.RequiresUnreferencedCode("Getting a converter for a type may require reflection which depends on unreferenced code.")]
277278
public System.Text.Json.Serialization.JsonConverter GetConverter(System.Type typeToConvert) { throw null; }
278279
}
279280
public enum JsonTokenType : byte

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/NullableConverterFactory.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ public override JsonConverter CreateConverter(Type typeToConvert, JsonSerializer
2020

2121
Type valueTypeToConvert = typeToConvert.GetGenericArguments()[0];
2222

23-
JsonConverter valueConverter = options.GetConverter(valueTypeToConvert);
23+
JsonConverter valueConverter = options.GetConverterInternal(valueTypeToConvert);
2424
Debug.Assert(valueConverter != null);
2525

2626
// If the value type has an interface or object converter, just return that converter directly.

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/ObjectConverter.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ internal override void WriteWithQuotes(Utf8JsonWriter writer, object? value, Jso
3939
Debug.Assert(value != null);
4040

4141
Type runtimeType = value.GetType();
42-
JsonConverter runtimeConverter = options.GetConverter(runtimeType);
42+
JsonConverter runtimeConverter = options.GetConverterInternal(runtimeType);
4343
if (runtimeConverter == this)
4444
{
4545
ThrowHelper.ThrowNotSupportedException_DictionaryKeyTypeNotSupported(runtimeType, this);

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Span.cs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -124,9 +124,8 @@ public static partial class JsonSerializer
124124
[RequiresUnreferencedCode(SerializationUnreferencedCodeMessage)]
125125
private static TValue? ReadUsingOptions<TValue>(ReadOnlySpan<byte> utf8Json, Type returnType, JsonSerializerOptions? options)
126126
{
127-
options ??= JsonSerializerOptions.s_defaultOptions;
128-
options.RootBuiltInConvertersAndTypeInfoCreator();
129-
return ReadUsingMetadata<TValue>(utf8Json, GetTypeInfo(returnType, options));
127+
JsonTypeInfo jsonTypeInfo = GetTypeInfo(returnType, options);
128+
return ReadUsingMetadata<TValue>(utf8Json, jsonTypeInfo);
130129
}
131130
}
132131
}

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public sealed partial class JsonSerializerOptions
2727
private readonly ConcurrentDictionary<Type, JsonConverter?> _converters = new ConcurrentDictionary<Type, JsonConverter?>();
2828

2929
[RequiresUnreferencedCode(JsonSerializer.SerializationUnreferencedCodeMessage)]
30-
internal void RootBuiltInConvertersAndTypeInfoCreator()
30+
private void RootBuiltInConverters()
3131
{
3232
s_defaultSimpleConverters ??= GetDefaultSimpleConverters();
3333
s_defaultFactoryConverters ??= new JsonConverter[]
@@ -45,11 +45,6 @@ internal void RootBuiltInConvertersAndTypeInfoCreator()
4545
// Object should always be last since it converts any type.
4646
new ObjectConverterFactory()
4747
};
48-
49-
_typeInfoCreationFunc ??= CreateJsonTypeInfo;
50-
51-
[RequiresUnreferencedCode(JsonSerializer.SerializationUnreferencedCodeMessage)]
52-
static JsonTypeInfo CreateJsonTypeInfo(Type type, JsonSerializerOptions options) => new JsonTypeInfo(type, options);
5348
}
5449

5550
private static Dictionary<Type, JsonConverter> GetDefaultSimpleConverters()
@@ -119,7 +114,7 @@ internal JsonConverter DetermineConverter(Type? parentClassType, Type runtimePro
119114

120115
if (converter == null)
121116
{
122-
converter = GetConverter(runtimePropertyType);
117+
converter = GetConverterInternal(runtimePropertyType);
123118
Debug.Assert(converter != null);
124119
}
125120

@@ -160,8 +155,22 @@ internal JsonConverter DetermineConverter(Type? parentClassType, Type runtimePro
160155
/// There is no compatible <see cref="System.Text.Json.Serialization.JsonConverter"/>
161156
/// for <paramref name="typeToConvert"/> or its serializable members.
162157
/// </exception>
158+
[RequiresUnreferencedCode("Getting a converter for a type may require reflection which depends on unreferenced code.")]
163159
public JsonConverter GetConverter(Type typeToConvert)
164160
{
161+
if (typeToConvert == null)
162+
{
163+
throw new ArgumentNullException(nameof(typeToConvert));
164+
}
165+
166+
RootBuiltInConverters();
167+
return GetConverterInternal(typeToConvert);
168+
}
169+
170+
internal JsonConverter GetConverterInternal(Type typeToConvert)
171+
{
172+
Debug.Assert(typeToConvert != null);
173+
165174
if (_converters.TryGetValue(typeToConvert, out JsonConverter? converter))
166175
{
167176
Debug.Assert(converter != null);

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using System.Collections.Concurrent;
55
using System.ComponentModel;
66
using System.Diagnostics;
7+
using System.Diagnostics.CodeAnalysis;
78
using System.Runtime.CompilerServices;
89
using System.Text.Encodings.Web;
910
using System.Text.Json.Node;
@@ -569,6 +570,16 @@ internal MemberAccessor MemberAccessorStrategy
569570
}
570571
}
571572

573+
[RequiresUnreferencedCode(JsonSerializer.SerializationUnreferencedCodeMessage)]
574+
internal void RootBuiltInConvertersAndTypeInfoCreator()
575+
{
576+
RootBuiltInConverters();
577+
_typeInfoCreationFunc ??= CreateJsonTypeInfo;
578+
579+
[RequiresUnreferencedCode(JsonSerializer.SerializationUnreferencedCodeMessage)]
580+
static JsonTypeInfo CreateJsonTypeInfo(Type type, JsonSerializerOptions options) => new JsonTypeInfo(type, options);
581+
}
582+
572583
internal JsonTypeInfo GetOrAddClass(Type type)
573584
{
574585
_haveTypesBeenCreated = true;

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -435,7 +435,7 @@ internal bool ReadJsonExtensionDataValue(ref ReadStack state, ref Utf8JsonReader
435435
return true;
436436
}
437437

438-
JsonConverter<JsonElement> converter = (JsonConverter<JsonElement>)Options.GetConverter(typeof(JsonElement));
438+
JsonConverter<JsonElement> converter = (JsonConverter<JsonElement>)Options.GetConverterInternal(typeof(JsonElement));
439439
if (!converter.TryRead(ref reader, typeof(JsonElement), Options, ref state, out JsonElement jsonElement))
440440
{
441441
// JsonElement is a struct that must be read in full.

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -517,7 +517,7 @@ private bool DetermineExtensionDataProperty(Dictionary<string, JsonPropertyInfo>
517517
// Avoid a reference to typeof(JsonNode) to support trimming.
518518
(declaredPropertyType.FullName == JsonObjectTypeName && ReferenceEquals(declaredPropertyType.Assembly, GetType().Assembly)))
519519
{
520-
converter = Options.GetConverter(declaredPropertyType);
520+
converter = Options.GetConverterInternal(declaredPropertyType);
521521
Debug.Assert(converter != null);
522522
}
523523

src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/JsonSourceGeneratorTests.cs

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33

44
using System.Collections.Generic;
55
using System.Linq;
6+
using System.Reflection;
67
using System.Text.Json.Serialization;
8+
using System.Text.Json.Serialization.Metadata;
79
using System.Text.Json.SourceGeneration.Tests;
810
using System.Text.Json.SourceGeneration.Tests.JsonSourceGeneration;
911
using Xunit;
@@ -466,5 +468,84 @@ public class ClassWithEnumAndNullable
466468
public DayOfWeek Day { get; set; }
467469
public DayOfWeek? NullableDay { get; set; }
468470
}
471+
472+
[Fact]
473+
public static void Converters_AndTypeInfoCreator_NotRooted_WhenMetadataNotPresent()
474+
{
475+
object[] objArr = new object[] { new MyStruct() };
476+
477+
// Metadata not generated for MyStruct without JsonSerializableAttribute.
478+
NotSupportedException ex = Assert.Throws<NotSupportedException>(
479+
() => JsonSerializer.Serialize(objArr, JsonContext.Default.ObjectArray));
480+
string exAsStr = ex.ToString();
481+
Assert.Contains(typeof(MyStruct).ToString(), exAsStr);
482+
Assert.Contains("JsonSerializerOptions", exAsStr);
483+
484+
// This test uses reflection to:
485+
// - Access JsonSerializerOptions.s_defaultSimpleConverters
486+
// - Access JsonSerializerOptions.s_defaultFactoryConverters
487+
// - Access JsonSerializerOptions._typeInfoCreationFunc
488+
//
489+
// If any of them changes, this test will need to be kept in sync.
490+
491+
// Confirm built-in converters not set.
492+
AssertFieldNull("s_defaultSimpleConverters", optionsInstance: null);
493+
AssertFieldNull("s_defaultFactoryConverters", optionsInstance: null);
494+
495+
// Confirm type info dynamic creator not set.
496+
AssertFieldNull("_typeInfoCreationFunc", JsonContext.Default.Options);
497+
498+
static void AssertFieldNull(string fieldName, JsonSerializerOptions? optionsInstance)
499+
{
500+
BindingFlags bindingFlags = BindingFlags.NonPublic | (optionsInstance == null ? BindingFlags.Static : BindingFlags.Instance);
501+
FieldInfo fieldInfo = typeof(JsonSerializerOptions).GetField(fieldName, bindingFlags);
502+
Assert.NotNull(fieldInfo);
503+
Assert.Null(fieldInfo.GetValue(optionsInstance));
504+
}
505+
}
506+
507+
private const string ExceptionMessageFromCustomContext = "Exception thrown from custom context.";
508+
509+
[Fact]
510+
public static void GetTypeInfoCalledDuringPolymorphicSerialization()
511+
{
512+
CustomContext context = new(new JsonSerializerOptions());
513+
514+
// Empty array is fine since we don't need metadata for children.
515+
Assert.Equal("[]", JsonSerializer.Serialize(Array.Empty<object>(), context.ObjectArray));
516+
Assert.Equal("[]", JsonSerializer.Serialize(Array.Empty<object>(), typeof(object[]), context));
517+
518+
// GetTypeInfo method called to get metadata for element run-time type.
519+
object[] objArr = new object[] { new MyStruct() };
520+
521+
InvalidOperationException ex = Assert.Throws<InvalidOperationException>(() => JsonSerializer.Serialize(objArr, context.ObjectArray));
522+
Assert.Contains(ExceptionMessageFromCustomContext, ex.ToString());
523+
524+
ex = Assert.Throws<InvalidOperationException>(() => JsonSerializer.Serialize(objArr, typeof(object[]), context));
525+
Assert.Contains(ExceptionMessageFromCustomContext, ex.ToString());
526+
}
527+
528+
internal struct MyStruct { }
529+
530+
internal class CustomContext : JsonSerializerContext
531+
{
532+
public CustomContext(JsonSerializerOptions options) : base(options) { }
533+
534+
private JsonTypeInfo<object> _object;
535+
public JsonTypeInfo<object> Object => _object ??= JsonMetadataServices.CreateValueInfo<object>(Options, JsonMetadataServices.ObjectConverter);
536+
537+
private JsonTypeInfo<object[]> _objectArray;
538+
public JsonTypeInfo<object[]> ObjectArray => _objectArray ??= JsonMetadataServices.CreateArrayInfo<object>(Options, Object, default);
539+
540+
public override JsonTypeInfo GetTypeInfo(Type type)
541+
{
542+
if (type == typeof(object[]))
543+
{
544+
return ObjectArray;
545+
}
546+
547+
throw new InvalidOperationException(ExceptionMessageFromCustomContext);
548+
}
549+
}
469550
}
470551
}

src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/CustomConverterTests/CustomConverterTests.cs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,5 +160,26 @@ public static void VerifyObjectConverterWithPreservedReferences()
160160
Assert.IsType<bool>(obj);
161161
Assert.Equal(true, obj);
162162
}
163+
164+
[Fact]
165+
public static void GetConverterRootsBuiltInConverters()
166+
{
167+
JsonSerializerOptions options = new();
168+
RunTest<DateTime>();
169+
RunTest<Point_2D>();
170+
171+
void RunTest<TConverterReturn>()
172+
{
173+
JsonConverter converter = options.GetConverter(typeof(TConverterReturn));
174+
Assert.NotNull(converter);
175+
Assert.True(converter is JsonConverter<TConverterReturn>);
176+
}
177+
}
178+
179+
[Fact]
180+
public static void GetConverterTypeToConvertNull()
181+
{
182+
Assert.Throws<ArgumentNullException>(() => (new JsonSerializerOptions()).GetConverter(typeToConvert: null!));
183+
}
163184
}
164185
}

0 commit comments

Comments
 (0)