Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move System.Object serialization to ObjectConverter #54436

Merged
2 commits merged into from
Jun 23, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ public ObjectConverter()

public override void Write(Utf8JsonWriter writer, object? value, JsonSerializerOptions options)
{
throw new InvalidOperationException();
Debug.Assert(value?.GetType() == typeof(object));

writer.WriteStartObject();
writer.WriteEndObject();
}

internal override object ReadWithQuotes(ref Utf8JsonReader reader)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,11 @@ public abstract partial class JsonConverter<T> : JsonConverter
/// </summary>
protected internal JsonConverter()
{
// Today only typeof(object) can have polymorphic writes.
// In the future, this will be check for !IsSealed (and excluding value types).
CanBePolymorphic = TypeToConvert == JsonTypeInfo.ObjectType;
IsInternalConverter = GetType().Assembly == typeof(JsonConverter).Assembly;
// Today only the internal JsonConverter<object> can have polymorphic writes.
CanBePolymorphic = IsInternalConverter && TypeToConvert == JsonTypeInfo.ObjectType;
IsValueType = TypeToConvert.IsValueType;
CanBeNull = default(T) is null;
IsInternalConverter = GetType().Assembly == typeof(JsonConverter).Assembly;

if (HandleNull)
{
Expand Down Expand Up @@ -332,7 +331,7 @@ internal bool TryWrite(Utf8JsonWriter writer, in T value, JsonSerializerOptions
bool ignoreCyclesPopReference = false;

if (
#if NET6_0_OR_GREATER
#if NET5_0_OR_GREATER
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved
!typeof(T).IsValueType && // treated as a constant by recent versions of the JIT.
#else
!IsValueType &&
Expand Down Expand Up @@ -368,15 +367,11 @@ internal bool TryWrite(Utf8JsonWriter writer, in T value, JsonSerializerOptions

if (CanBePolymorphic)
{
Debug.Assert(IsInternalConverter);

Type type = value.GetType();
if (type == JsonTypeInfo.ObjectType)
{
writer.WriteStartObject();
writer.WriteEndObject();
return true;
}

if (type != TypeToConvert && IsInternalConverter)
if (type != TypeToConvert)
{
// For internal converter only: Handle polymorphic case and get the new converter.
// Custom converter, even though polymorphic converter, get called for reading AND writing.
Expand Down Expand Up @@ -420,6 +415,19 @@ internal bool TryWrite(Utf8JsonWriter writer, in T value, JsonSerializerOptions
}

VerifyWrite(originalPropertyDepth, writer);

if (
#if NET5_0_OR_GREATER
!typeof(T).IsValueType && // treated as a constant by recent versions of the JIT.
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved
#endif
ignoreCyclesPopReference)
{
// should only be entered if we're serializing instances
// of type object using the internal object converter.
Debug.Assert(value?.GetType() == typeof(object) && IsInternalConverter);
state.ReferenceResolver.PopReferenceForCycleDetection();
}

return true;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ internal override bool GetMemberAndWriteJson(object obj, ref WriteStack state, U
T value = Get!(obj);

if (
#if NET6_0_OR_GREATER
#if NET5_0_OR_GREATER
!typeof(T).IsValueType && // treated as a constant by recent versions of the JIT.
#else
!Converter.IsValueType &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,9 @@ public override object Read(ref Utf8JsonReader reader, Type typeToConvert, JsonS

public override void Write(Utf8JsonWriter writer, object value, JsonSerializerOptions options)
{
throw new InvalidOperationException("Should not get here.");
Assert.IsType<object>(value);
writer.WriteStartObject();
writer.WriteEndObject();
}
}

Expand Down Expand Up @@ -732,5 +734,23 @@ static void Verify(JsonSerializerOptions options)
options.Converters.Add(new SystemObjectNewtonsoftCompatibleConverter());
Verify(options);
}

[Fact]
public static void CanCustomizeSystemObjectSerialization()
{
var options = new JsonSerializerOptions { Converters = { new CustomSystemObjectConverter() } };

string expectedJson = "42";
string actualJson = JsonSerializer.Serialize(new object(), options);
Assert.Equal(expectedJson, actualJson);
}

private class CustomSystemObjectConverter : JsonConverter<object>
{
public override object? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) => throw new NotImplementedException();
public override void Write(Utf8JsonWriter writer, object value, JsonSerializerOptions options)
=> writer.WriteNumberValue(42);
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -328,12 +328,12 @@ public static void JsonEncodedTextStringsCustomAllowAll(string message, string e
[Fact]
public static void Options_GetConverterForObjectJsonElement_GivesCorrectConverter()
{
GenericObjectOrJsonElementConverterTestHelper<object>("ObjectConverter", new object(), "[3]", true);
GenericObjectOrJsonElementConverterTestHelper<object>("ObjectConverter", new object(), "{}");
JsonElement element = JsonDocument.Parse("[3]").RootElement;
GenericObjectOrJsonElementConverterTestHelper<JsonElement>("JsonElementConverter", element, "[3]", false);
GenericObjectOrJsonElementConverterTestHelper<JsonElement>("JsonElementConverter", element, "[3]");
}

private static void GenericObjectOrJsonElementConverterTestHelper<T>(string converterName, object objectValue, string stringValue, bool throws)
private static void GenericObjectOrJsonElementConverterTestHelper<T>(string converterName, object objectValue, string stringValue)
{
var options = new JsonSerializerOptions();

Expand All @@ -347,10 +347,7 @@ private static void GenericObjectOrJsonElementConverterTestHelper<T>(string conv

if (readValue is JsonElement element)
{
Assert.Equal(JsonValueKind.Array, element.ValueKind);
JsonElement.ArrayEnumerator iterator = element.EnumerateArray();
Assert.True(iterator.MoveNext());
Assert.Equal(3, iterator.Current.GetInt32());
JsonTestHelper.AssertJsonEqual(stringValue, element.ToString());
}
else
{
Expand All @@ -360,22 +357,14 @@ private static void GenericObjectOrJsonElementConverterTestHelper<T>(string conv
using (var stream = new MemoryStream())
using (var writer = new Utf8JsonWriter(stream))
{
if (throws)
{
Assert.Throws<InvalidOperationException>(() => converter.Write(writer, (T)objectValue, options));
Assert.Throws<InvalidOperationException>(() => converter.Write(writer, (T)objectValue, null));
}
else
{
converter.Write(writer, (T)objectValue, options);
writer.Flush();
Assert.Equal(stringValue, Encoding.UTF8.GetString(stream.ToArray()));

writer.Reset(stream);
converter.Write(writer, (T)objectValue, null); // Test with null option
writer.Flush();
Assert.Equal(stringValue + stringValue, Encoding.UTF8.GetString(stream.ToArray()));
}
converter.Write(writer, (T)objectValue, options);
writer.Flush();
Assert.Equal(stringValue, Encoding.UTF8.GetString(stream.ToArray()));

writer.Reset(stream);
converter.Write(writer, (T)objectValue, null); // Test with null option
writer.Flush();
Assert.Equal(stringValue + stringValue, Encoding.UTF8.GetString(stream.ToArray()));
}
}

Expand Down