Skip to content

Commit 097d9ea

Browse files
eiriktsarpaliseiriktsarpalis
authored
Make ObjectConverter use ConverterStrategy.Object (#65748)
* Use ConvereterStrategy.Object in ObjectConverter * address feedback
1 parent a982359 commit 097d9ea

23 files changed

+347
-378
lines changed

‎src/libraries/System.Text.Json/src/System.Text.Json.csproj‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,7 @@ System.Text.Json.Nodes.JsonValue</PackageDescription>
122122
<Compile Include="System\Text\Json\Serialization\JsonSerializer.Write.Node.cs" />
123123
<Compile Include="System\Text\Json\Serialization\JsonSerializerContext.cs" />
124124
<Compile Include="System\Text\Json\Serialization\JsonSerializerOptions.Caching.cs" />
125+
<Compile Include="System\Text\Json\Serialization\PolymorphicSerializationState.cs" />
125126
<Compile Include="System\Text\Json\Serialization\ReferenceEqualsWrapper.cs" />
126127
<Compile Include="System\Text\Json\Serialization\ConverterStrategy.cs" />
127128
<Compile Include="System\Text\Json\Serialization\ConverterList.cs" />
@@ -165,6 +166,7 @@ System.Text.Json.Nodes.JsonValue</PackageDescription>
165166
<Compile Include="System\Text\Json\Serialization\Converters\Node\JsonValueConverter.cs" />
166167
<Compile Include="System\Text\Json\Serialization\Converters\Object\JsonObjectConverter.cs" />
167168
<Compile Include="System\Text\Json\Serialization\Converters\Object\KeyValuePairConverter.cs" />
169+
<Compile Include="System\Text\Json\Serialization\Converters\Object\ObjectConverter.cs" />
168170
<Compile Include="System\Text\Json\Serialization\Converters\Object\ObjectConverterFactory.cs" />
169171
<Compile Include="System\Text\Json\Serialization\Converters\Object\ObjectDefaultConverter.cs" />
170172
<Compile Include="System\Text\Json\Serialization\Converters\Object\ObjectWithParameterizedConstructorConverter.cs" />
@@ -189,7 +191,6 @@ System.Text.Json.Nodes.JsonValue</PackageDescription>
189191
<Compile Include="System\Text\Json\Serialization\Converters\Value\JsonElementConverter.cs" />
190192
<Compile Include="System\Text\Json\Serialization\Converters\Value\NullableConverter.cs" />
191193
<Compile Include="System\Text\Json\Serialization\Converters\Value\NullableConverterFactory.cs" />
192-
<Compile Include="System\Text\Json\Serialization\Converters\Value\ObjectConverter.cs" />
193194
<Compile Include="System\Text\Json\Serialization\Converters\Value\SByteConverter.cs" />
194195
<Compile Include="System\Text\Json\Serialization\Converters\Value\SingleConverter.cs" />
195196
<Compile Include="System\Text\Json\Serialization\Converters\Value\StringConverter.cs" />

‎src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ConverterStrategy.cs‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,25 @@ namespace System.Text.Json
1212
/// </remarks>
1313
internal enum ConverterStrategy : byte
1414
{
15-
// Default - no class type.
15+
/// <summary>
16+
/// Default value; not used by any converter.
17+
/// </summary>
1618
None = 0x0,
17-
// JsonObjectConverter<> - objects with properties.
19+
/// <summary>
20+
/// Objects with properties.
21+
/// </summary>
1822
Object = 0x1,
19-
// JsonConverter<> - simple values.
23+
/// <summary>
24+
/// Simple values or user-provided custom converters.
25+
/// </summary>
2026
Value = 0x2,
21-
// JsonIEnumerableConverter<> - all enumerable collections except dictionaries.
27+
/// <summary>
28+
/// Enumerable collections except dictionaries.
29+
/// </summary>
2230
Enumerable = 0x8,
23-
// JsonDictionaryConverter<,> - dictionary types.
31+
/// <summary>
32+
/// Dictionary types.
33+
/// </summary>
2434
Dictionary = 0x10,
2535
}
2636
}

‎src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/JsonCollectionConverter.cs‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ internal override bool OnTryRead(
180180
{
181181
state.Current.PropertyState = StackFramePropertyState.ReadValue;
182182

183-
if (!SingleValueReadWithReadAhead(elementConverter.ConverterStrategy, ref reader, ref state))
183+
if (!SingleValueReadWithReadAhead(elementConverter.RequiresReadAhead, ref reader, ref state))
184184
{
185185
value = default;
186186
return false;
@@ -269,12 +269,8 @@ internal override bool OnTryWrite(
269269
state.Current.ProcessedStartToken = true;
270270
if (options.ReferenceHandlingStrategy == ReferenceHandlingStrategy.Preserve)
271271
{
272-
MetadataPropertyName metadata = JsonSerializer.WriteReferenceForCollection(this, value, ref state, writer);
273-
if (metadata == MetadataPropertyName.Ref)
274-
{
275-
return true;
276-
}
277-
272+
MetadataPropertyName metadata = JsonSerializer.WriteReferenceForCollection(this, ref state, writer);
273+
Debug.Assert(metadata != MetadataPropertyName.Ref);
278274
state.Current.MetadataPropertyName = metadata;
279275
}
280276
else

‎src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/JsonDictionaryConverter.cs‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,7 @@ internal sealed override bool OnTryRead(
239239
{
240240
state.Current.PropertyState = StackFramePropertyState.ReadValue;
241241

242-
if (!SingleValueReadWithReadAhead(_valueConverter.ConverterStrategy, ref reader, ref state))
242+
if (!SingleValueReadWithReadAhead(_valueConverter.RequiresReadAhead, ref reader, ref state))
243243
{
244244
state.Current.DictionaryKey = key;
245245
value = default;
@@ -311,10 +311,8 @@ internal sealed override bool OnTryWrite(
311311
writer.WriteStartObject();
312312
if (options.ReferenceHandlingStrategy == ReferenceHandlingStrategy.Preserve)
313313
{
314-
if (JsonSerializer.WriteReferenceForObject(this, dictionary, ref state, writer) == MetadataPropertyName.Ref)
315-
{
316-
return true;
317-
}
314+
MetadataPropertyName propertyName = JsonSerializer.WriteReferenceForObject(this, ref state, writer);
315+
Debug.Assert(propertyName != MetadataPropertyName.Ref);
318316
}
319317

320318
state.Current.JsonPropertyInfo = state.Current.JsonTypeInfo.ElementTypeInfo!.PropertyInfoForTypeInfo;

‎src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/FSharp/FSharpOptionConverter.cs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,11 @@ public FSharpOptionConverter(JsonConverter<TElement> elementConverter)
2929
_optionValueGetter = FSharpCoreReflectionProxy.Instance.CreateFSharpOptionValueGetter<TOption, TElement>();
3030
_optionConstructor = FSharpCoreReflectionProxy.Instance.CreateFSharpOptionSomeConstructor<TOption, TElement>();
3131

32-
// temporary workaround for JsonConverter base constructor needing to access
33-
// ConverterStrategy when calculating `CanUseDirectReadOrWrite`.
34-
// TODO move `CanUseDirectReadOrWrite` from JsonConverter to JsonTypeInfo.
35-
_converterStrategy = _elementConverter.ConverterStrategy;
36-
CanUseDirectReadOrWrite = _converterStrategy == ConverterStrategy.Value;
32+
// Workaround for the base constructor depending on the (still unset) ConverterStrategy
33+
// to derive the CanUseDirectReadOrWrite and RequiresReadAhead values.
34+
_converterStrategy = elementConverter.ConverterStrategy;
35+
CanUseDirectReadOrWrite = elementConverter.CanUseDirectReadOrWrite;
36+
RequiresReadAhead = elementConverter.RequiresReadAhead;
3737
}
3838

3939
internal override bool OnTryRead(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options, ref ReadStack state, out TOption? value)

‎src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/FSharp/FSharpValueOptionConverter.cs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,11 @@ public FSharpValueOptionConverter(JsonConverter<TElement> elementConverter)
2929
_optionValueGetter = FSharpCoreReflectionProxy.Instance.CreateFSharpValueOptionValueGetter<TValueOption, TElement>();
3030
_optionConstructor = FSharpCoreReflectionProxy.Instance.CreateFSharpValueOptionSomeConstructor<TValueOption, TElement>();
3131

32-
// temporary workaround for JsonConverter base constructor needing to access
33-
// ConverterStrategy when calculating `CanUseDirectReadOrWrite`.
34-
// TODO move `CanUseDirectReadOrWrite` from JsonConverter to JsonTypeInfo.
35-
_converterStrategy = _elementConverter.ConverterStrategy;
36-
CanUseDirectReadOrWrite = _converterStrategy == ConverterStrategy.Value;
32+
// Workaround for the base constructor depending on the (still unset) ConverterStrategy
33+
// to derive the CanUseDirectReadOrWrite and RequiresReadAhead values.
34+
_converterStrategy = elementConverter.ConverterStrategy;
35+
CanUseDirectReadOrWrite = elementConverter.CanUseDirectReadOrWrite;
36+
RequiresReadAhead = elementConverter.RequiresReadAhead;
3737
}
3838

3939
internal override bool OnTryRead(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options, ref ReadStack state, out TValueOption value)

‎src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/ObjectConverter.cs‎ renamed to ‎src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectConverter.cs‎

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,24 +7,59 @@ namespace System.Text.Json.Serialization.Converters
77
{
88
internal sealed class ObjectConverter : JsonConverter<object?>
99
{
10+
internal override ConverterStrategy ConverterStrategy => ConverterStrategy.Object;
11+
12+
public ObjectConverter()
13+
{
14+
CanBePolymorphic = true;
15+
// JsonElement/JsonNode parsing does not support async; force read ahead for now.
16+
RequiresReadAhead = true;
17+
}
18+
1019
public override object? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
1120
{
1221
if (options.UnknownTypeHandling == JsonUnknownTypeHandling.JsonElement)
1322
{
1423
return JsonElement.ParseValue(ref reader);
1524
}
1625

26+
Debug.Assert(options.UnknownTypeHandling == JsonUnknownTypeHandling.JsonNode);
1727
return JsonNodeConverter.Instance.Read(ref reader, typeToConvert, options);
1828
}
1929

2030
public override void Write(Utf8JsonWriter writer, object? value, JsonSerializerOptions options)
2131
{
2232
Debug.Assert(value?.GetType() == typeof(object));
23-
2433
writer.WriteStartObject();
2534
writer.WriteEndObject();
2635
}
2736

37+
internal override bool OnTryRead(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options, ref ReadStack state, out object? value)
38+
{
39+
if (options.UnknownTypeHandling == JsonUnknownTypeHandling.JsonElement)
40+
{
41+
JsonElement element = JsonElement.ParseValue(ref reader);
42+
43+
// Edge case where we want to lookup for a reference when parsing into typeof(object)
44+
if (options.ReferenceHandlingStrategy == ReferenceHandlingStrategy.Preserve &&
45+
JsonSerializer.TryGetReferenceFromJsonElement(ref state, element, out object? referenceValue))
46+
{
47+
value = referenceValue;
48+
}
49+
else
50+
{
51+
value = element;
52+
}
53+
54+
return true;
55+
}
56+
57+
Debug.Assert(options.UnknownTypeHandling == JsonUnknownTypeHandling.JsonNode);
58+
value = JsonNodeConverter.Instance.Read(ref reader, typeToConvert, options)!;
59+
// TODO reference lookup for JsonNode deserialization.
60+
return true;
61+
}
62+
2863
internal override object ReadAsPropertyNameCore(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
2964
{
3065
ThrowHelper.ThrowNotSupportedException_DictionaryKeyTypeNotSupported(TypeToConvert, this);

‎src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectDefaultConverter.cs‎

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -261,10 +261,8 @@ internal sealed override bool OnTryWrite(
261261
writer.WriteStartObject();
262262
if (options.ReferenceHandlingStrategy == ReferenceHandlingStrategy.Preserve)
263263
{
264-
if (JsonSerializer.WriteReferenceForObject(this, obj, ref state, writer) == MetadataPropertyName.Ref)
265-
{
266-
return true;
267-
}
264+
MetadataPropertyName propertyName = JsonSerializer.WriteReferenceForObject(this, ref state, writer);
265+
Debug.Assert(propertyName != MetadataPropertyName.Ref);
268266
}
269267

270268
if (obj is IJsonOnSerializing onSerializing)
@@ -313,10 +311,8 @@ internal sealed override bool OnTryWrite(
313311
writer.WriteStartObject();
314312
if (options.ReferenceHandlingStrategy == ReferenceHandlingStrategy.Preserve)
315313
{
316-
if (JsonSerializer.WriteReferenceForObject(this, obj, ref state, writer) == MetadataPropertyName.Ref)
317-
{
318-
return true;
319-
}
314+
MetadataPropertyName propertyName = JsonSerializer.WriteReferenceForObject(this, ref state, writer);
315+
Debug.Assert(propertyName != MetadataPropertyName.Ref);
320316
}
321317

322318
if (obj is IJsonOnSerializing onSerializing)
@@ -339,9 +335,7 @@ internal sealed override bool OnTryWrite(
339335

340336
if (!jsonPropertyInfo.GetMemberAndWriteJson(obj!, ref state, writer))
341337
{
342-
Debug.Assert(jsonPropertyInfo.ConverterBase.ConverterStrategy != ConverterStrategy.Value ||
343-
jsonPropertyInfo.ConverterBase.TypeToConvert == JsonTypeInfo.ObjectType);
344-
338+
Debug.Assert(jsonPropertyInfo.ConverterBase.ConverterStrategy != ConverterStrategy.Value);
345339
return false;
346340
}
347341

@@ -443,15 +437,15 @@ protected static bool ReadAheadPropertyValue(ref ReadStack state, ref Utf8JsonRe
443437

444438
if (!state.Current.UseExtensionProperty)
445439
{
446-
if (!SingleValueReadWithReadAhead(jsonPropertyInfo.ConverterBase.ConverterStrategy, ref reader, ref state))
440+
if (!SingleValueReadWithReadAhead(jsonPropertyInfo.ConverterBase.RequiresReadAhead, ref reader, ref state))
447441
{
448442
return false;
449443
}
450444
}
451445
else
452446
{
453447
// The actual converter is JsonElement, so force a read-ahead.
454-
if (!SingleValueReadWithReadAhead(ConverterStrategy.Value, ref reader, ref state))
448+
if (!SingleValueReadWithReadAhead(requiresReadAhead: true, ref reader, ref state))
455449
{
456450
return false;
457451
}

‎src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,7 @@ private bool HandleConstructorArgumentWithContinuation(
364364
// Returning false below will cause the read-ahead functionality to finish the read.
365365
state.Current.PropertyState = StackFramePropertyState.ReadValue;
366366

367-
if (!SingleValueReadWithReadAhead(jsonParameterInfo.ConverterBase.ConverterStrategy, ref reader, ref state))
367+
if (!SingleValueReadWithReadAhead(jsonParameterInfo.ConverterBase.RequiresReadAhead, ref reader, ref state))
368368
{
369369
return false;
370370
}

‎src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/NullableConverter.cs‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,13 @@ internal sealed class NullableConverter<T> : JsonConverter<T?> where T : struct
1616
public NullableConverter(JsonConverter<T> elementConverter)
1717
{
1818
_elementConverter = elementConverter;
19-
ConverterStrategy = elementConverter.ConverterStrategy;
2019
IsInternalConverterForNumberType = elementConverter.IsInternalConverterForNumberType;
21-
// temporary workaround for JsonConverter base constructor needing to access
22-
// ConverterStrategy when calculating `CanUseDirectReadOrWrite`.
23-
CanUseDirectReadOrWrite = elementConverter.ConverterStrategy == ConverterStrategy.Value;
20+
21+
// Workaround for the base constructor depending on the (still unset) ConverterStrategy
22+
// to derive the CanUseDirectReadOrWrite and RequiresReadAhead values.
23+
ConverterStrategy = elementConverter.ConverterStrategy;
24+
CanUseDirectReadOrWrite = elementConverter.CanUseDirectReadOrWrite;
25+
RequiresReadAhead = elementConverter.RequiresReadAhead;
2426
}
2527

2628
internal override bool OnTryRead(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options, ref ReadStack state, out T? value)

0 commit comments

Comments
 (0)