Skip to content

Commit

Permalink
Fix #72614. (#72630)
Browse files Browse the repository at this point in the history
  • Loading branch information
eiriktsarpalis authored Jul 22, 2022
1 parent ea2c907 commit cbe29e8
Show file tree
Hide file tree
Showing 8 changed files with 77 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public JsonMetadataServicesConverter(JsonConverter<T> converter)
}

internal override bool OnTryRead(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options, ref ReadStack state, out T? value)
=> Converter.OnTryRead(ref reader, typeToConvert, options, ref state, out value);
=> Converter.OnTryRead(ref reader, typeToConvert, options, ref state, out value);

internal override bool OnTryWrite(Utf8JsonWriter writer, T value, JsonSerializerOptions options, ref WriteStack state)
{
Expand All @@ -70,15 +70,17 @@ internal override bool OnTryWrite(Utf8JsonWriter writer, T value, JsonSerializer
Debug.Assert(options == jsonTypeInfo.Options);

if (!state.SupportContinuation &&
jsonTypeInfo.HasSerializeHandler &&
jsonTypeInfo is JsonTypeInfo<T> info &&
info.SerializeHandler != null &&
!state.CurrentContainsMetadata && // Do not use the fast path if state needs to write metadata.
info.Options.SerializerContext?.CanUseSerializationLogic == true)
{
Debug.Assert(info.SerializeHandler != null);
info.SerializeHandler(writer, value);
return true;
}

jsonTypeInfo.ValidateCanBeUsedForMetadataSerialization();
return Converter.OnTryWrite(writer, value, options, ref state);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,7 @@ private void BeginRead(ref ReadStack state, ref Utf8JsonReader reader, JsonSeria
{
JsonTypeInfo jsonTypeInfo = state.Current.JsonTypeInfo;

jsonTypeInfo.ValidateCanBeUsedForDeserialization();
jsonTypeInfo.ValidateCanBeUsedForMetadataSerialization();

if (jsonTypeInfo.ParameterCount != jsonTypeInfo.ParameterCache!.Count)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ private static void WriteUsingGeneratedSerializer<TValue>(Utf8JsonWriter writer,
{
Debug.Assert(writer != null);

if (jsonTypeInfo.HasSerialize &&
if (jsonTypeInfo.HasSerializeHandler &&
jsonTypeInfo is JsonTypeInfo<TValue> typedInfo &&
typedInfo.Options.SerializerContext?.CanUseSerializationLogic == true)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ internal JsonPropertyInfo GetProperty(
{
PropertyRef propertyRef;

ValidateCanBeUsedForDeserialization();
ValidateCanBeUsedForMetadataSerialization();
ulong key = GetKey(propertyName);

// Keep a local copy of the cache in case it changes by another thread.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -267,16 +267,16 @@ public JsonPolymorphismOptions? PolymorphismOptions
private JsonTypeInfo? _elementTypeInfo;

// Avoids having to perform an expensive cast to JsonTypeInfo<T> to check if there is a Serialize method.
internal bool HasSerialize { get; private protected set; }
internal bool HasSerializeHandler { get; private protected set; }

// Configure would normally have thrown why initializing properties for source gen but type had SerializeHandler
// so it is allowed to be used for serialization but it will throw if used for deserialization
internal bool ThrowOnDeserialize { get; private protected set; }
// so it is allowed to be used for fast-path serialization but it will throw if used for metadata-based serialization
internal bool MetadataSerializationNotSupported { get; private protected set; }

[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal void ValidateCanBeUsedForDeserialization()
internal void ValidateCanBeUsedForMetadataSerialization()
{
if (ThrowOnDeserialize)
if (MetadataSerializationNotSupported)
{
ThrowHelper.ThrowInvalidOperationException_NoMetadataForTypeProperties(Options.TypeInfoResolver, Type);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ private protected set
{
Debug.Assert(!IsConfigured, "We should not mutate configured JsonTypeInfo");
_serialize = value;
HasSerialize = value != null;
HasSerializeHandler = value != null;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,13 @@ internal override JsonParameterInfoValues[] GetParameterInfoValues()
JsonParameterInfoValues[] array;
if (CtorParamInitFunc == null || (array = CtorParamInitFunc()) == null)
{
ThrowHelper.ThrowInvalidOperationException_NoMetadataForTypeCtorParams(Options.TypeInfoResolver, Type);
return null!;
if (SerializeHandler == null)
{
ThrowHelper.ThrowInvalidOperationException_NoMetadataForTypeCtorParams(Options.TypeInfoResolver, Type);
}

array = Array.Empty<JsonParameterInfoValues>();
MetadataSerializationNotSupported = true;
}

return array;
Expand Down Expand Up @@ -139,13 +144,12 @@ internal override void LateAddProperties()
return;
}

if (SerializeHandler != null && context?.CanUseSerializationLogic == true)
if (SerializeHandler == null)
{
ThrowOnDeserialize = true;
return;
ThrowHelper.ThrowInvalidOperationException_NoMetadataForTypeProperties(Options.TypeInfoResolver, Type);
}

ThrowHelper.ThrowInvalidOperationException_NoMetadataForTypeProperties(Options.TypeInfoResolver, Type);
MetadataSerializationNotSupported = true;
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,60 @@ public static void CombiningContexts_ResolveJsonTypeInfo_DifferentCasing()
Assert.Equal("lastName", personInfo.Properties[1].Name);
}

[Fact]
public static void FastPathSerialization_ResolvingJsonTypeInfo()
{
JsonSerializerOptions options = FastPathSerializationContext.Default.Options;

JsonTypeInfo<JsonMessage> jsonMessageInfo = (JsonTypeInfo<JsonMessage>)options.GetTypeInfo(typeof(JsonMessage));
Assert.NotNull(jsonMessageInfo.SerializeHandler);

var value = new JsonMessage { Message = "Hi" };
string expectedJson = """{"Message":"Hi","Length":2}""";

Assert.Equal(expectedJson, JsonSerializer.Serialize(value, jsonMessageInfo));
Assert.Equal(expectedJson, JsonSerializer.Serialize(value, options));

// Throws since deserialization without metadata is not supported
Assert.Throws<InvalidOperationException>(() => JsonSerializer.Deserialize<JsonMessage>(expectedJson, jsonMessageInfo));
Assert.Throws<InvalidOperationException>(() => JsonSerializer.Deserialize<JsonMessage>(expectedJson, options));
}

[Fact]
public static void FastPathSerialization_CombinedContext_ThrowsInvalidOperationException()
{
// TODO change exception assertions once https://github.com/dotnet/runtime/issues/71933 is fixed.

var options = new JsonSerializerOptions
{
TypeInfoResolver = JsonTypeInfoResolver.Combine(FastPathSerializationContext.Default, new DefaultJsonTypeInfoResolver())
};

JsonTypeInfo<JsonMessage> jsonMessageInfo = (JsonTypeInfo<JsonMessage>)options.GetTypeInfo(typeof(JsonMessage));
Assert.NotNull(jsonMessageInfo.SerializeHandler);

var value = new JsonMessage { Message = "Hi" };
Assert.Throws<InvalidOperationException>(() => JsonSerializer.Serialize(value, jsonMessageInfo));
Assert.Throws<InvalidOperationException>(() => JsonSerializer.Serialize(value, options));

JsonTypeInfo<ClassWithJsonMessage> classInfo = (JsonTypeInfo<ClassWithJsonMessage>)options.GetTypeInfo(typeof(ClassWithJsonMessage));
Assert.Null(classInfo.SerializeHandler);

var largerValue = new ClassWithJsonMessage { Message = value };
Assert.Throws<InvalidOperationException>(() => JsonSerializer.Serialize(largerValue, classInfo));
Assert.Throws<InvalidOperationException>(() => JsonSerializer.Serialize(largerValue, options));
}

[JsonSourceGenerationOptions(GenerationMode = JsonSourceGenerationMode.Serialization)]
[JsonSerializable(typeof(JsonMessage))]
public partial class FastPathSerializationContext : JsonSerializerContext
{ }

public class ClassWithJsonMessage
{
public JsonMessage Message { get; set; }
}

[Theory]
[MemberData(nameof(GetCombiningContextsData))]
public static void CombiningContexts_Serialization<T>(T value, string expectedJson)
Expand Down

0 comments on commit cbe29e8

Please sign in to comment.