STJ: Support serialization callbacks for collection and dictionary types #104120
Conversation
dotnet-policy-service
bot
added
the
community-contribution
| value = (TCollection)state.Current.ReturnValue!; | ||
|
|
||
| jsonTypeInfo.OnDeserialized?.Invoke(value); |
There was a problem hiding this comment.
If TCollection is a struct this would result in the value being boxed again and potentially breaking user callback that depend on the same reference being passed to the callback.
| value = (TCollection)state.Current.ReturnValue!; | |
| jsonTypeInfo.OnDeserialized?.Invoke(value); | |
| object result = state.Current.ReturnValue!; | |
| value = (TCollection)result; | |
| jsonTypeInfo.OnDeserialized?.Invoke(result); |
| state.Current.JsonPropertyInfo = state.Current.JsonTypeInfo.ElementTypeInfo!.PropertyInfoForTypeInfo; | ||
| state.Current.JsonPropertyInfo = jsonTypeInfo.ElementTypeInfo!.PropertyInfoForTypeInfo; | ||
|
|
||
| jsonTypeInfo.OnSerializing?.Invoke(value); |
There was a problem hiding this comment.
Nit: I would move the callback before the first JSON gets written to the underlying writer.
| value = (TDictionary)state.Current.ReturnValue!; | ||
|
|
||
| jsonTypeInfo.OnDeserialized?.Invoke(value); |
There was a problem hiding this comment.
Likewise
| value = (TDictionary)state.Current.ReturnValue!; | |
| jsonTypeInfo.OnDeserialized?.Invoke(value); | |
| object result = state.Current.ReturnValue!; | |
| value = (TDictionary)result; | |
| jsonTypeInfo.OnDeserialized?.Invoke(result); |
| state.Current.JsonPropertyInfo = state.Current.JsonTypeInfo.ElementTypeInfo!.PropertyInfoForTypeInfo; | ||
| state.Current.JsonPropertyInfo = jsonTypeInfo.ElementTypeInfo!.PropertyInfoForTypeInfo; | ||
|
|
||
| jsonTypeInfo.OnSerializing?.Invoke(dictionary); |
There was a problem hiding this comment.
Likewise, I would invoke the callback before metadata or the start object token get written to the wire.
| @@ -153,7 +153,7 @@ internal JsonTypeInfo(Type type, JsonConverter converter, JsonSerializerOptions | |||
| { | |||
| VerifyMutable(); | |||
|
|
|||
| if (Kind != JsonTypeInfoKind.Object) | |||
| if (Kind == JsonTypeInfoKind.None) | |||
There was a problem hiding this comment.
In the unlikely case that new kinds get added in the future, I would probably just turn this into an explicit check:
| if (Kind == JsonTypeInfoKind.None) | |
| if (Kind is not (JsonTypeInfoKind.Object or JsonTypeInfoKind.Enumerable or JsonTypeInfoKind.Dictionary)) |
| @@ -419,10 +531,7 @@ public override void Write(Utf8JsonWriter writer, MyValue value, JsonSerializerO | |||
| [Fact] | |||
| public static void NonPocosIgnored() | |||
| { | |||
| JsonSerializer.Serialize(new MyCollection()); | |||
There was a problem hiding this comment.
Could you also add a few tests in MetadataTests.cs that verify that callbacks can be specified via the JsonTypeInfo APIs?
Closes #71945