Skip to content

Commit bdd8aa2

Browse files
jozkeejozkee
committed
Address feedback
1 parent a2fbfd0 commit bdd8aa2

13 files changed

Lines changed: 114 additions & 60 deletions

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

Lines changed: 29 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ public JsonException() { }
143143
protected JsonException(System.Runtime.Serialization.SerializationInfo info, System.Runtime.Serialization.StreamingContext context) { }
144144
public JsonException(string? message) { }
145145
public JsonException(string? message, System.Exception? innerException) { }
146-
public JsonException(string? message, string? path, long? lineNumber, long? bytePositionInLine) { }
146+
public JsonException(string? message, string path, long? lineNumber, long? bytePositionInLine) { }
147147
public JsonException(string? message, string path, long? lineNumber, long? bytePositionInLine, System.Exception? innerException) { }
148148
public long? BytePositionInLine { get { throw null; } }
149149
public long? LineNumber { get { throw null; } }
@@ -190,11 +190,11 @@ public static partial class JsonSerializer
190190
public static object? Deserialize(ref System.Text.Json.Utf8JsonReader reader, System.Type returnType, System.Text.Json.JsonSerializerOptions? options = null) { throw null; }
191191
public static System.Threading.Tasks.ValueTask<object?> DeserializeAsync(System.IO.Stream utf8Json, System.Type returnType, System.Text.Json.JsonSerializerOptions? options = null, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
192192
public static System.Threading.Tasks.ValueTask<TValue> DeserializeAsync<TValue>(System.IO.Stream utf8Json, System.Text.Json.JsonSerializerOptions? options = null, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
193-
[return: System.Diagnostics.CodeAnalysis.MaybeNullAttribute]
193+
[return: System.Diagnostics.CodeAnalysis.MaybeNull]
194194
public static TValue Deserialize<TValue>(System.ReadOnlySpan<byte> utf8Json, System.Text.Json.JsonSerializerOptions? options = null) { throw null; }
195-
[return: System.Diagnostics.CodeAnalysis.MaybeNullAttribute]
195+
[return: System.Diagnostics.CodeAnalysis.MaybeNull]
196196
public static TValue Deserialize<TValue>(string json, System.Text.Json.JsonSerializerOptions? options = null) { throw null; }
197-
[return: System.Diagnostics.CodeAnalysis.MaybeNullAttribute]
197+
[return: System.Diagnostics.CodeAnalysis.MaybeNull]
198198
public static TValue Deserialize<TValue>(ref System.Text.Json.Utf8JsonReader reader, System.Text.Json.JsonSerializerOptions? options = null) { throw null; }
199199
public static string Serialize(object? value, System.Type inputType, System.Text.Json.JsonSerializerOptions? options = null) { throw null; }
200200
public static void Serialize(System.Text.Json.Utf8JsonWriter writer, object? value, System.Type inputType, System.Text.Json.JsonSerializerOptions? options = null) { }
@@ -205,16 +205,11 @@ public static void Serialize(System.Text.Json.Utf8JsonWriter writer, object? val
205205
public static void Serialize<TValue>(System.Text.Json.Utf8JsonWriter writer, TValue value, System.Text.Json.JsonSerializerOptions? options = null) { }
206206
public static string Serialize<TValue>(TValue value, System.Text.Json.JsonSerializerOptions? options = null) { throw null; }
207207
}
208-
public enum JsonSerializerDefaults
209-
{
210-
General = 0,
211-
Web = 1,
212-
}
213208
public sealed partial class JsonSerializerOptions
214209
{
215210
public JsonSerializerOptions() { }
216-
public JsonSerializerOptions(System.Text.Json.JsonSerializerDefaults defaults) { }
217211
public JsonSerializerOptions(System.Text.Json.JsonSerializerOptions options) { }
212+
public JsonSerializerOptions(System.Text.Json.JsonSerializerDefaults defaults) { }
218213
public bool AllowTrailingCommas { get { throw null; } set { } }
219214
public System.Collections.Generic.IList<System.Text.Json.Serialization.JsonConverter> Converters { get { throw null; } }
220215
public int DefaultBufferSize { get { throw null; } set { } }
@@ -231,6 +226,11 @@ public JsonSerializerOptions(System.Text.Json.JsonSerializerOptions options) { }
231226
public bool WriteIndented { get { throw null; } set { } }
232227
public System.Text.Json.Serialization.JsonConverter GetConverter(System.Type typeToConvert) { throw null; }
233228
}
229+
public enum JsonSerializerDefaults
230+
{
231+
General = 0,
232+
Web = 1,
233+
}
234234
public enum JsonTokenType : byte
235235
{
236236
None = (byte)0,
@@ -464,21 +464,22 @@ public void WriteStringValue(System.Text.Json.JsonEncodedText value) { }
464464
}
465465
namespace System.Text.Json.Serialization
466466
{
467-
public abstract partial class JsonAttribute : System.Attribute
467+
public enum JsonIgnoreCondition
468468
{
469-
protected JsonAttribute() { }
469+
Never = 0,
470+
Always = 1,
471+
WhenWritingDefault = 2,
470472
}
471-
[System.AttributeUsageAttribute(System.AttributeTargets.Constructor, AllowMultiple=false)]
472-
public sealed partial class JsonConstructorAttribute : System.Text.Json.Serialization.JsonAttribute
473+
public abstract partial class JsonAttribute : System.Attribute
473474
{
474-
public JsonConstructorAttribute() { }
475+
protected JsonAttribute() { }
475476
}
476477
public abstract partial class JsonConverter
477478
{
478479
internal JsonConverter() { }
479480
public abstract bool CanConvert(System.Type typeToConvert);
480481
}
481-
[System.AttributeUsageAttribute(System.AttributeTargets.Class | System.AttributeTargets.Enum | System.AttributeTargets.Property | System.AttributeTargets.Struct, AllowMultiple=false)]
482+
[System.AttributeUsageAttribute(System.AttributeTargets.Class | System.AttributeTargets.Enum | System.AttributeTargets.Property | System.AttributeTargets.Struct, AllowMultiple = false)]
482483
public partial class JsonConverterAttribute : System.Text.Json.Serialization.JsonAttribute
483484
{
484485
protected JsonConverterAttribute() { }
@@ -489,40 +490,39 @@ public JsonConverterAttribute(System.Type converterType) { }
489490
public abstract partial class JsonConverterFactory : System.Text.Json.Serialization.JsonConverter
490491
{
491492
protected JsonConverterFactory() { }
492-
public abstract System.Text.Json.Serialization.JsonConverter? CreateConverter(System.Type typeToConvert, System.Text.Json.JsonSerializerOptions options);
493+
public abstract System.Text.Json.Serialization.JsonConverter CreateConverter(System.Type typeToConvert, System.Text.Json.JsonSerializerOptions options);
493494
}
494495
public abstract partial class JsonConverter<T> : System.Text.Json.Serialization.JsonConverter
495496
{
496497
protected internal JsonConverter() { }
497-
public virtual bool HandleNull { get { throw null; } }
498498
public override bool CanConvert(System.Type typeToConvert) { throw null; }
499-
[return: System.Diagnostics.CodeAnalysis.MaybeNullAttribute]
499+
public virtual bool HandleNull { get { throw null; } }
500+
[return: System.Diagnostics.CodeAnalysis.MaybeNull]
500501
public abstract T Read(ref System.Text.Json.Utf8JsonReader reader, System.Type typeToConvert, System.Text.Json.JsonSerializerOptions options);
501502
public abstract void Write(System.Text.Json.Utf8JsonWriter writer, T value, System.Text.Json.JsonSerializerOptions options);
502503
}
503-
[System.AttributeUsageAttribute(System.AttributeTargets.Property, AllowMultiple=false)]
504+
[System.AttributeUsageAttribute(System.AttributeTargets.Constructor, AllowMultiple = false)]
505+
public sealed partial class JsonConstructorAttribute : System.Text.Json.Serialization.JsonAttribute
506+
{
507+
public JsonConstructorAttribute() { }
508+
}
509+
[System.AttributeUsageAttribute(System.AttributeTargets.Property, AllowMultiple = false)]
504510
public sealed partial class JsonExtensionDataAttribute : System.Text.Json.Serialization.JsonAttribute
505511
{
506512
public JsonExtensionDataAttribute() { }
507513
}
508-
[System.AttributeUsageAttribute(System.AttributeTargets.Property, AllowMultiple=false)]
514+
[System.AttributeUsageAttribute(System.AttributeTargets.Property, AllowMultiple = false)]
509515
public sealed partial class JsonIgnoreAttribute : System.Text.Json.Serialization.JsonAttribute
510516
{
511517
public JsonIgnoreAttribute() { }
512518
public System.Text.Json.Serialization.JsonIgnoreCondition Condition { get { throw null; } set { } }
513519
}
514-
public enum JsonIgnoreCondition
515-
{
516-
Never = 0,
517-
Always = 1,
518-
WhenWritingDefault = 2,
519-
}
520-
[System.AttributeUsageAttribute(System.AttributeTargets.Property, AllowMultiple=false)]
520+
[System.AttributeUsageAttribute(System.AttributeTargets.Property, AllowMultiple = false)]
521521
public sealed partial class JsonIncludeAttribute : System.Text.Json.Serialization.JsonAttribute
522522
{
523523
public JsonIncludeAttribute() { }
524524
}
525-
[System.AttributeUsageAttribute(System.AttributeTargets.Property, AllowMultiple=false)]
525+
[System.AttributeUsageAttribute(System.AttributeTargets.Property, AllowMultiple = false)]
526526
public sealed partial class JsonPropertyNameAttribute : System.Text.Json.Serialization.JsonAttribute
527527
{
528528
public JsonPropertyNameAttribute(string name) { }
@@ -538,7 +538,6 @@ public JsonStringEnumConverter(System.Text.Json.JsonNamingPolicy? namingPolicy =
538538
public abstract partial class ReferenceHandler
539539
{
540540
protected ReferenceHandler() { }
541-
public static System.Text.Json.Serialization.ReferenceHandler? Default { get { throw null; } }
542541
public static System.Text.Json.Serialization.ReferenceHandler Preserve { get { throw null; } }
543542
public abstract System.Text.Json.Serialization.ReferenceResolver CreateResolver();
544543
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,6 @@
123123
<Compile Include="System\Text\Json\Serialization\Converters\Value\UInt32Converter.cs" />
124124
<Compile Include="System\Text\Json\Serialization\Converters\Value\UInt64Converter.cs" />
125125
<Compile Include="System\Text\Json\Serialization\Converters\Value\UriConverter.cs" />
126-
<Compile Include="System\Text\Json\Serialization\DefaultReferenceResolver.cs" />
127126
<Compile Include="System\Text\Json\Serialization\JsonCamelCaseNamingPolicy.cs" />
128127
<Compile Include="System\Text\Json\Serialization\JsonClassInfo.cs" />
129128
<Compile Include="System\Text\Json\Serialization\JsonClassInfo.Cache.cs" />
@@ -163,6 +162,8 @@
163162
<Compile Include="System\Text\Json\Serialization\MetadataPropertyName.cs" />
164163
<Compile Include="System\Text\Json\Serialization\ParameterRef.cs" />
165164
<Compile Include="System\Text\Json\Serialization\PooledByteBufferWriter.cs" />
165+
<Compile Include="System\Text\Json\Serialization\PreserveReferenceHandler.cs" />
166+
<Compile Include="System\Text\Json\Serialization\PreserveReferenceResolver.cs" />
166167
<Compile Include="System\Text\Json\Serialization\PropertyRef.cs" />
167168
<Compile Include="System\Text\Json\Serialization\ReadStack.cs" />
168169
<Compile Include="System\Text\Json\Serialization\ReadStackFrame.cs" />

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,9 @@ internal sealed override bool OnTryRead(
174174

175175
value = (TCollection)state.Current.ReturnValue!;
176176
state.ReferenceResolver.AddReference(state.Current.MetadataId, value);
177+
// Clear metadata name, if the next read fails
178+
// we want to point the JSON path to the property's object.
179+
state.Current.JsonPropertyName = null;
177180
}
178181

179182
state.Current.ObjectState = StackFrameObjectState.CreatedObject;

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,14 @@ internal override bool OnTryRead(
135135
if (state.Current.MetadataId != null)
136136
{
137137
value = (TCollection)state.Current.ReturnValue!;
138+
139+
// Remember the prior metadata and temporarily use '$id' to write it in the path in case AddReference throws
140+
// in this case, the last property seen will be '$values' when we reach this point.
141+
byte[]? lastMetadataProperty = state.Current.JsonPropertyName;
142+
state.Current.JsonPropertyName = JsonSerializer.s_idPropertyName;
143+
138144
state.ReferenceResolver.AddReference(state.Current.MetadataId, value);
145+
state.Current.JsonPropertyName = lastMetadataProperty;
139146
}
140147

141148
state.Current.JsonPropertyInfo = state.Current.JsonClassInfo.ElementClassInfo!.PropertyInfoForClassInfo;

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,9 @@ internal override bool OnTryRead(ref Utf8JsonReader reader, Type typeToConvert,
106106
if (state.Current.MetadataId != null)
107107
{
108108
state.ReferenceResolver.AddReference(state.Current.MetadataId, obj);
109+
// Clear metadata name, if the next read fails
110+
// we want to point the JSON path to the property's object.
111+
state.Current.JsonPropertyName = null;
109112
}
110113

111114
state.Current.ReturnValue = obj;

‎src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandleMetadata.cs‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@ namespace System.Text.Json
1010
{
1111
public static partial class JsonSerializer
1212
{
13+
internal static byte[] s_idPropertyName
14+
= new byte[] { (byte)'$', (byte)'i', (byte)'d' };
15+
1316
/// <summary>
1417
/// Returns true if successful, false is the reader ran out of buffer.
1518
/// Sets state.Current.ReturnValue to the $ref target for MetadataRefProperty cases.
@@ -135,9 +138,6 @@ internal static bool ResolveMetadata(
135138

136139
state.Current.MetadataId = reader.GetString();
137140

138-
// Clear the MetadataPropertyName since we are done processing Id.
139-
state.Current.JsonPropertyName = default;
140-
141141
if (converter.ClassType == ClassType.Enumerable)
142142
{
143143
// Need to Read $values property name.
@@ -184,6 +184,8 @@ internal static bool ResolveMetadata(
184184
{
185185
if (reader.TokenType != JsonTokenType.PropertyName)
186186
{
187+
// Missing $values, JSON path should point to the property's object.
188+
state.Current.JsonPropertyName = null;
187189
ThrowHelper.ThrowJsonException_MetadataPreservedArrayValuesNotFound(converter.TypeToConvert);
188190
}
189191

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
namespace System.Text.Json.Serialization
6+
{
7+
internal sealed class PreserveReferenceHandler : ReferenceHandler
8+
{
9+
public override ReferenceResolver CreateResolver() => throw new InvalidOperationException();
10+
11+
internal override ReferenceResolver CreateResolver(bool writing) => new PreserveReferenceResolver(writing);
12+
}
13+
}

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/DefaultReferenceResolver.cs renamed to src/libraries/System.Text.Json/src/System/Text/Json/Serialization/PreserveReferenceResolver.cs

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,17 +10,23 @@ namespace System.Text.Json.Serialization
1010
/// <summary>
1111
/// The default ReferenceResolver implementation to handle duplicate object references.
1212
/// </summary>
13-
internal sealed class DefaultReferenceResolver : ReferenceResolver
13+
internal sealed class PreserveReferenceResolver : ReferenceResolver
1414
{
1515
private uint _referenceCount;
1616
private readonly Dictionary<string, object>? _referenceIdToObjectMap;
1717
private readonly Dictionary<object, string>? _objectToReferenceIdMap;
1818

19-
public DefaultReferenceResolver()
19+
public PreserveReferenceResolver(bool writing)
2020
{
21-
// Comparer used here does a reference equality comparison on serialization, which is where we use the objects as the dictionary keys.
22-
_objectToReferenceIdMap = new Dictionary<object, string>(ReferenceEqualityComparer.Instance);
23-
_referenceIdToObjectMap = new Dictionary<string, object>();
21+
if (writing)
22+
{
23+
// Comparer used here does a reference equality comparison on serialization, which is where we use the objects as the dictionary keys.
24+
_objectToReferenceIdMap = new Dictionary<object, string>(ReferenceEqualityComparer.Instance);
25+
}
26+
else
27+
{
28+
_referenceIdToObjectMap = new Dictionary<string, object>();
29+
}
2430
}
2531

2632
public override void AddReference(string referenceId, object value)

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ public void Initialize(Type type, JsonSerializerOptions options, bool supportCon
9191

9292
if (options.ReferenceHandler != null)
9393
{
94-
ReferenceResolver = options.ReferenceHandler.CreateResolver();
94+
ReferenceResolver = options.ReferenceHandler.CreateResolver(writing: false);
9595
ShouldReadPreservedReferences = true;
9696
}
9797

@@ -250,8 +250,10 @@ static void AppendStackFrame(StringBuilder sb, in ReadStackFrame frame)
250250
return;
251251
}
252252

253-
// Once all elements are read, the exception is not within the array.
254-
if (frame.ObjectState < StackFrameObjectState.ReadElements)
253+
// For continuation scenarios only, before or after all elements are read, the exception is not within the array.
254+
if (frame.ObjectState == StackFrameObjectState.None ||
255+
frame.ObjectState == StackFrameObjectState.CreatedObject ||
256+
frame.ObjectState == StackFrameObjectState.ReadElements)
255257
{
256258
sb.Append('[');
257259
sb.Append(GetCount(enumerable));

0 commit comments

Comments
 (0)