Skip to content

Commit 1a2f095

Browse files
eiriktsarpaliseiriktsarpalis
authored
[JsonSerializer] Add support for out-of-order metadata reads. (#97474)
* JsonSerializer: Add support for out-of-order metadata reads. * Address feedback * Address feedback & add more tests. * Improve exception message for polymorphic abstract types. * Improve error message.
1 parent 2f59305 commit 1a2f095

25 files changed

+523
-122
lines changed

‎src/libraries/System.Text.Json/Common/JsonSourceGenerationOptionsAttribute.cs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,11 @@ public JsonSourceGenerationOptionsAttribute(JsonSerializerDefaults defaults)
4040
}
4141
}
4242

43+
/// <summary>
44+
/// Specifies the default value of <see cref="JsonSerializerOptions.AllowOutOfOrderMetadataProperties"/> when set.
45+
/// </summary>
46+
public bool AllowOutOfOrderMetadataProperties { get; set; }
47+
4348
/// <summary>
4449
/// Specifies the default value of <see cref="JsonSerializerOptions.AllowTrailingCommas"/> when set.
4550
/// </summary>

‎src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1105,6 +1105,9 @@ private static void GetLogicForDefaultSerializerOptionsInit(SourceGenerationOpti
11051105
writer.WriteLine('{');
11061106
writer.Indentation++;
11071107

1108+
if (optionsSpec.AllowOutOfOrderMetadataProperties is bool allowOutOfOrderMetadataProperties)
1109+
writer.WriteLine($"AllowOutOfOrderMetadataProperties = {FormatBool(allowOutOfOrderMetadataProperties)},");
1110+
11081111
if (optionsSpec.AllowTrailingCommas is bool allowTrailingCommas)
11091112
writer.WriteLine($"AllowTrailingCommas = {FormatBool(allowTrailingCommas)},");
11101113

‎src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,7 @@ private SourceGenerationOptionsSpec ParseJsonSourceGenerationOptionsAttribute(IN
263263
JsonSourceGenerationMode? generationMode = null;
264264
List<TypeRef>? converters = null;
265265
JsonSerializerDefaults? defaults = null;
266+
bool? allowOutOfOrderMetadataProperties = null;
266267
bool? allowTrailingCommas = null;
267268
int? defaultBufferSize = null;
268269
JsonIgnoreCondition? defaultIgnoreCondition = null;
@@ -293,6 +294,10 @@ private SourceGenerationOptionsSpec ParseJsonSourceGenerationOptionsAttribute(IN
293294
{
294295
switch (namedArg.Key)
295296
{
297+
case nameof(JsonSourceGenerationOptionsAttribute.AllowOutOfOrderMetadataProperties):
298+
allowOutOfOrderMetadataProperties = (bool)namedArg.Value.Value!;
299+
break;
300+
296301
case nameof(JsonSourceGenerationOptionsAttribute.AllowTrailingCommas):
297302
allowTrailingCommas = (bool)namedArg.Value.Value!;
298303
break;
@@ -396,6 +401,7 @@ private SourceGenerationOptionsSpec ParseJsonSourceGenerationOptionsAttribute(IN
396401
{
397402
GenerationMode = generationMode,
398403
Defaults = defaults,
404+
AllowOutOfOrderMetadataProperties = allowOutOfOrderMetadataProperties,
399405
AllowTrailingCommas = allowTrailingCommas,
400406
DefaultBufferSize = defaultBufferSize,
401407
Converters = converters?.ToImmutableEquatableArray(),

‎src/libraries/System.Text.Json/gen/Model/SourceGenerationOptionsSpec.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ public sealed record SourceGenerationOptionsSpec
1616

1717
public required JsonSerializerDefaults? Defaults { get; init; }
1818

19+
public required bool? AllowOutOfOrderMetadataProperties { get; init; }
20+
1921
public required bool? AllowTrailingCommas { get; init; }
2022

2123
public required ImmutableEquatableArray<TypeRef>? Converters { get; init; }

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,7 @@ public sealed partial class JsonSerializerOptions
368368
public JsonSerializerOptions() { }
369369
public JsonSerializerOptions(System.Text.Json.JsonSerializerDefaults defaults) { }
370370
public JsonSerializerOptions(System.Text.Json.JsonSerializerOptions options) { }
371+
public bool AllowOutOfOrderMetadataProperties { get { throw null; } set { } }
371372
public bool AllowTrailingCommas { get { throw null; } set { } }
372373
public System.Collections.Generic.IList<System.Text.Json.Serialization.JsonConverter> Converters { get { throw null; } }
373374
public static System.Text.Json.JsonSerializerOptions Default { [System.Diagnostics.CodeAnalysis.RequiresDynamicCodeAttribute("JSON serialization and deserialization might require types that cannot be statically analyzed and might need runtime code generation. Use System.Text.Json source generation for native AOT applications."), System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute("JSON serialization and deserialization might require types that cannot be statically analyzed. Use the overload that takes a JsonTypeInfo or JsonSerializerContext, or make sure all of the required types are preserved.")] get { throw null; } }
@@ -1060,6 +1061,7 @@ public sealed partial class JsonSourceGenerationOptionsAttribute : System.Text.J
10601061
{
10611062
public JsonSourceGenerationOptionsAttribute() { }
10621063
public JsonSourceGenerationOptionsAttribute(System.Text.Json.JsonSerializerDefaults defaults) { }
1064+
public bool AllowOutOfOrderMetadataProperties { get { throw null; } set { } }
10631065
public bool AllowTrailingCommas { get { throw null; } set { } }
10641066
public System.Type[]? Converters { get { throw null; } set { } }
10651067
public int DefaultBufferSize { get { throw null; } set { } }

‎src/libraries/System.Text.Json/src/Resources/Strings.resx‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -382,8 +382,11 @@
382382
<data name="DeserializeNoConstructor" xml:space="preserve">
383383
<value>Deserialization of types without a parameterless constructor, a singular parameterized constructor, or a parameterized constructor annotated with '{0}' is not supported. Type '{1}'.</value>
384384
</data>
385-
<data name="DeserializePolymorphicInterface" xml:space="preserve">
386-
<value>Deserialization of interface types is not supported. Type '{0}'.</value>
385+
<data name="DeserializeInterfaceOrAbstractType" xml:space="preserve">
386+
<value>Deserialization of interface or abstract types is not supported. Type '{0}'.</value>
387+
</data>
388+
<data name="DeserializationMustSpecifyTypeDiscriminator" xml:space="preserve">
389+
<value>The JSON payload for polymorphic interface or abstract type '{0}' must specify a type discriminator.</value>
387390
</data>
388391
<data name="SerializationConverterOnAttributeNotCompatible" xml:space="preserve">
389392
<value>The converter specified on '{0}' is not compatible with the type '{1}'.</value>
@@ -439,8 +442,8 @@
439442
<data name="MetadataDuplicateIdFound" xml:space="preserve">
440443
<value>The value of the '$id' metadata property '{0}' conflicts with an existing identifier.</value>
441444
</data>
442-
<data name="MetadataIdIsNotFirstProperty" xml:space="preserve">
443-
<value>The metadata property '$id' must be the first reference preservation property in the JSON object.</value>
445+
<data name="MetadataIdCannotBeCombinedWithRef" xml:space="preserve">
446+
<value>The metadata property '$id' cannot be used together with '$ref' metadata properties.</value>
444447
</data>
445448
<data name="MetadataInvalidReferenceToValueType" xml:space="preserve">
446449
<value>Invalid reference to value type '{0}'.</value>
@@ -477,8 +480,8 @@
477480
<data name="UnmappedJsonProperty" xml:space="preserve">
478481
<value>The JSON property '{0}' could not be mapped to any .NET member contained in type '{1}'.</value>
479482
</data>
480-
<data name="MetadataDuplicateTypeProperty" xml:space="preserve">
481-
<value>Deserialized object contains a duplicate type discriminator metadata property.</value>
483+
<data name="DuplicateMetadataProperty" xml:space="preserve">
484+
<value>Deserialized object contains a duplicate '{0}' metadata property.</value>
482485
</data>
483486
<data name="MultipleMembersBindWithConstructorParameter" xml:space="preserve">
484487
<value>Members '{0}' and '{1}' on type '{2}' cannot both bind with parameter '{3}' in the deserialization constructor.</value>

‎src/libraries/System.Text.Json/src/System/Text/Json/JsonHelpers.cs‎

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,14 @@ namespace System.Text.Json
1313
internal static partial class JsonHelpers
1414
{
1515
/// <summary>
16-
/// Returns the span for the given reader.
16+
/// Returns the unescaped span for the given reader.
1717
/// </summary>
1818
[MethodImpl(MethodImplOptions.AggressiveInlining)]
19-
public static ReadOnlySpan<byte> GetSpan(this scoped ref Utf8JsonReader reader)
19+
public static ReadOnlySpan<byte> GetUnescapedSpan(this scoped ref Utf8JsonReader reader)
2020
{
21-
return reader.HasValueSequence ? reader.ValueSequence.ToArray() : reader.ValueSpan;
21+
Debug.Assert(reader.TokenType is JsonTokenType.String or JsonTokenType.PropertyName);
22+
ReadOnlySpan<byte> span = reader.HasValueSequence ? reader.ValueSequence.ToArray() : reader.ValueSpan;
23+
return reader.ValueIsEscaped ? JsonReaderHelper.GetUnescapedSpan(span) : span;
2224
}
2325

2426
/// <summary>
@@ -53,7 +55,7 @@ static bool TryAdvanceWithReadAhead(scoped ref Utf8JsonReader reader)
5355
if (tokenType is JsonTokenType.StartObject or JsonTokenType.StartArray)
5456
{
5557
// Attempt to skip to make sure we have all the data we need.
56-
bool complete = reader.TrySkipPartial(targetDepth: reader.CurrentDepth);
58+
bool complete = reader.TrySkipPartial();
5759

5860
// We need to restore the state in all cases as we need to be positioned back before
5961
// the current token to either attempt to skip again or to actually read the value.
@@ -140,6 +142,22 @@ public static void ReadWithVerify(this ref Utf8JsonReader reader)
140142
Debug.Assert(result);
141143
}
142144

145+
/// <summary>
146+
/// Performs a TrySkip() with a Debug.Assert verifying the reader did not return false.
147+
/// </summary>
148+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
149+
public static void SkipWithVerify(this ref Utf8JsonReader reader)
150+
{
151+
bool success = reader.TrySkipPartial(reader.CurrentDepth);
152+
Debug.Assert(success, "The skipped value should have already been buffered.");
153+
}
154+
155+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
156+
public static bool TrySkipPartial(this ref Utf8JsonReader reader)
157+
{
158+
return reader.TrySkipPartial(reader.CurrentDepth);
159+
}
160+
143161
/// <summary>
144162
/// Calls Encoding.UTF8.GetString that supports netstandard.
145163
/// </summary>

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

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -34,17 +34,7 @@ protected virtual void CreateCollection(ref Utf8JsonReader reader, scoped ref Re
3434

3535
if (typeInfo.CreateObject is null)
3636
{
37-
// The contract model was not able to produce a default constructor for two possible reasons:
38-
// 1. Either the declared collection type is abstract and cannot be instantiated.
39-
// 2. The collection type does not specify a default constructor.
40-
if (Type.IsAbstract || Type.IsInterface)
41-
{
42-
ThrowHelper.ThrowNotSupportedException_CannotPopulateCollection(Type, ref reader, ref state);
43-
}
44-
else
45-
{
46-
ThrowHelper.ThrowNotSupportedException_DeserializeNoConstructor(Type, ref reader, ref state);
47-
}
37+
ThrowHelper.ThrowNotSupportedException_DeserializeNoConstructor(typeInfo, ref reader, ref state);
4838
}
4939

5040
state.Current.ReturnValue = typeInfo.CreateObject();
@@ -267,7 +257,17 @@ internal override bool OnTryRead(
267257
if (reader.TokenType != JsonTokenType.EndObject)
268258
{
269259
Debug.Assert(reader.TokenType == JsonTokenType.PropertyName);
270-
ThrowHelper.ThrowJsonException_MetadataInvalidPropertyInArrayMetadata(ref state, typeToConvert, reader);
260+
if (options.AllowOutOfOrderMetadataProperties)
261+
{
262+
Debug.Assert(JsonSerializer.IsMetadataPropertyName(reader.GetUnescapedSpan(), (state.Current.BaseJsonTypeInfo ?? jsonTypeInfo).PolymorphicTypeResolver), "should only be hit if metadata property.");
263+
bool result = reader.TrySkipPartial(reader.CurrentDepth - 1); // skip to the end of the object
264+
Debug.Assert(result, "Metadata reader must have buffered all contents.");
265+
Debug.Assert(reader.TokenType is JsonTokenType.EndObject);
266+
}
267+
else
268+
{
269+
ThrowHelper.ThrowJsonException_MetadataInvalidPropertyInArrayMetadata(ref state, typeToConvert, reader);
270+
}
271271
}
272272
}
273273
}

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

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -49,20 +49,10 @@ protected virtual void CreateCollection(ref Utf8JsonReader reader, scoped ref Re
4949

5050
if (typeInfo.CreateObject is null)
5151
{
52-
// The contract model was not able to produce a default constructor for two possible reasons:
53-
// 1. Either the declared collection type is abstract and cannot be instantiated.
54-
// 2. The collection type does not specify a default constructor.
55-
if (Type.IsAbstract || Type.IsInterface)
56-
{
57-
ThrowHelper.ThrowNotSupportedException_CannotPopulateCollection(Type, ref reader, ref state);
58-
}
59-
else
60-
{
61-
ThrowHelper.ThrowNotSupportedException_DeserializeNoConstructor(Type, ref reader, ref state);
62-
}
52+
ThrowHelper.ThrowNotSupportedException_DeserializeNoConstructor(typeInfo, ref reader, ref state);
6353
}
6454

65-
state.Current.ReturnValue = typeInfo.CreateObject()!;
55+
state.Current.ReturnValue = typeInfo.CreateObject();
6656
Debug.Assert(state.Current.ReturnValue is TDictionary);
6757
}
6858

@@ -256,10 +246,19 @@ internal sealed override bool OnTryRead(
256246

257247
if (state.Current.CanContainMetadata)
258248
{
259-
ReadOnlySpan<byte> propertyName = reader.GetSpan();
249+
ReadOnlySpan<byte> propertyName = reader.GetUnescapedSpan();
260250
if (JsonSerializer.IsMetadataPropertyName(propertyName, state.Current.BaseJsonTypeInfo.PolymorphicTypeResolver))
261251
{
262-
ThrowHelper.ThrowUnexpectedMetadataException(propertyName, ref reader, ref state);
252+
if (options.AllowOutOfOrderMetadataProperties)
253+
{
254+
reader.SkipWithVerify();
255+
state.Current.EndElement();
256+
continue;
257+
}
258+
else
259+
{
260+
ThrowHelper.ThrowUnexpectedMetadataException(propertyName, ref reader, ref state);
261+
}
263262
}
264263
}
265264

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

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,10 @@ internal override bool OnTryRead(ref Utf8JsonReader reader, Type typeToConvert,
4040
{
4141
if (jsonTypeInfo.CreateObject == null)
4242
{
43-
ThrowHelper.ThrowNotSupportedException_DeserializeNoConstructor(jsonTypeInfo.Type, ref reader, ref state);
43+
ThrowHelper.ThrowNotSupportedException_DeserializeNoConstructor(jsonTypeInfo, ref reader, ref state);
4444
}
4545

46-
obj = jsonTypeInfo.CreateObject()!;
46+
obj = jsonTypeInfo.CreateObject();
4747
}
4848

4949
PopulatePropertiesFastPath(obj, jsonTypeInfo, options, ref reader, ref state);
@@ -116,10 +116,10 @@ internal override bool OnTryRead(ref Utf8JsonReader reader, Type typeToConvert,
116116
{
117117
if (jsonTypeInfo.CreateObject == null)
118118
{
119-
ThrowHelper.ThrowNotSupportedException_DeserializeNoConstructor(jsonTypeInfo.Type, ref reader, ref state);
119+
ThrowHelper.ThrowNotSupportedException_DeserializeNoConstructor(jsonTypeInfo, ref reader, ref state);
120120
}
121121

122-
obj = jsonTypeInfo.CreateObject()!;
122+
obj = jsonTypeInfo.CreateObject();
123123
}
124124

125125
if ((state.Current.MetadataPropertyNames & MetadataPropertyName.Id) != 0)
@@ -171,7 +171,15 @@ internal override bool OnTryRead(ref Utf8JsonReader reader, Type typeToConvert,
171171
// Read method would have thrown if otherwise.
172172
Debug.Assert(tokenType == JsonTokenType.PropertyName);
173173

174-
ReadOnlySpan<byte> unescapedPropertyName = JsonSerializer.GetPropertyName(ref state, ref reader);
174+
ReadOnlySpan<byte> unescapedPropertyName = JsonSerializer.GetPropertyName(ref state, ref reader, options, out bool isAlreadyReadMetadataProperty);
175+
if (isAlreadyReadMetadataProperty)
176+
{
177+
Debug.Assert(options.AllowOutOfOrderMetadataProperties);
178+
reader.SkipWithVerify();
179+
state.Current.EndProperty();
180+
continue;
181+
}
182+
175183
jsonPropertyInfo = JsonSerializer.LookupProperty(
176184
obj,
177185
unescapedPropertyName,
@@ -258,9 +266,7 @@ internal override bool OnTryRead(ref Utf8JsonReader reader, Type typeToConvert,
258266
}
259267

260268
// This method is using aggressive inlining to avoid extra stack frame for deep object graphs.
261-
#if !DEBUG
262269
[MethodImpl(MethodImplOptions.AggressiveInlining)]
263-
#endif
264270
internal static void PopulatePropertiesFastPath(object obj, JsonTypeInfo jsonTypeInfo, JsonSerializerOptions options, ref Utf8JsonReader reader, scoped ref ReadStack state)
265271
{
266272
jsonTypeInfo.OnDeserializing?.Invoke(obj);
@@ -282,7 +288,9 @@ internal static void PopulatePropertiesFastPath(object obj, JsonTypeInfo jsonTyp
282288
// Read method would have thrown if otherwise.
283289
Debug.Assert(tokenType == JsonTokenType.PropertyName);
284290

285-
ReadOnlySpan<byte> unescapedPropertyName = JsonSerializer.GetPropertyName(ref state, ref reader);
291+
ReadOnlySpan<byte> unescapedPropertyName = JsonSerializer.GetPropertyName(ref state, ref reader, options, out bool isAlreadyReadMetadataProperty);
292+
Debug.Assert(!isAlreadyReadMetadataProperty, "Only possible for types that can read metadata, which do not call into the fast-path method.");
293+
286294
JsonPropertyInfo jsonPropertyInfo = JsonSerializer.LookupProperty(
287295
obj,
288296
unescapedPropertyName,

0 commit comments

Comments
 (0)