Skip to content

Commit 4468757

Browse files
exyiexyi
committed
Fix null handling when calling custom JsonConverters
System.Text.Json converters have HandleNull property, which we should respect: * when false (default), both serialization and deserialization should handle null values before the converter is called. Conterintuitively this is the case even if converter is explicitely defined for Nullable<...> type * when true: we don't check for null and let converter handle it We always allow null in DotvvmConverters, because they all already handle it, and it keeps the generated code smaller.
1 parent 2427f9b commit 4468757

5 files changed

Lines changed: 113 additions & 12 deletions

File tree

‎src/Framework/Framework/ViewModel/Serialization/CustomPrimitiveTypeJsonConverter.cs‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ public override bool CanConvert(Type typeToConvert) =>
2121
class InnerConverter<T>: JsonConverter<T> where T: IDotvvmPrimitiveType
2222
{
2323
private CustomPrimitiveTypeRegistration registration = ReflectionUtils.TryGetCustomPrimitiveTypeRegistration(typeof(T)) ?? throw new InvalidOperationException($"The type {typeof(T)} is not a custom primitive type!");
24+
public override bool HandleNull => true;
2425
public override T? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
2526
{
2627
if (reader.TokenType is JsonTokenType.String
@@ -51,7 +52,10 @@ or JsonTokenType.False
5152

5253
public override void Write(Utf8JsonWriter writer, T value, JsonSerializerOptions options)
5354
{
54-
writer.WriteStringValue(registration.ToStringMethod(value));
55+
if (value is null)
56+
writer.WriteNullValue();
57+
else
58+
writer.WriteStringValue(registration.ToStringMethod(value));
5559
}
5660
}
5761
}

‎src/Framework/Framework/ViewModel/Serialization/DotvvmByteArrayConverter.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ namespace DotVVM.Framework.ViewModel.Serialization
1010
{
1111
public class DotvvmByteArrayConverter : JsonConverter<byte[]>
1212
{
13+
public override bool HandleNull => true;
1314
public override byte[]? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
1415
{
1516
if (reader.TokenType == JsonTokenType.Null)

‎src/Framework/Framework/ViewModel/Serialization/DotvvmObjectConverter.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ namespace DotVVM.Framework.ViewModel.Serialization
99
/// <summary> Mimicks Newtonsoft.Json behavior for System.Object - number -> double, string -> string, true/false -> bool, otherwise JsonElement </summary>
1010
public class DotvvmObjectConverter : JsonConverter<object?>
1111
{
12+
public override bool HandleNull => true;
1213
public override void Write(Utf8JsonWriter writer, object? value, JsonSerializerOptions options)
1314
{
1415
if (value is null)

‎src/Framework/Framework/ViewModel/Serialization/ViewModelSerializationMap.cs‎

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -543,6 +543,13 @@ private bool CanContainEncryptedValues(Type type)
543543
return property.JsonConverter;
544544
}
545545

546+
private static bool ConverterHandlesNull(JsonConverter converter) =>
547+
// converter.HandleNull is defined only on JsonConverter<T> :/
548+
(bool)typeof(JsonConverter<>)
549+
.MakeGenericType(converter.Type.NotNull())
550+
.GetProperty(nameof(JsonConverter<object>.HandleNull))!
551+
.GetValue(converter)!;
552+
546553
private Expression CallPropertyConverterRead(JsonConverter converter, Type type, Expression reader, Expression jsonOptions, Expression dotvvmState, Expression? existingValue)
547554
{
548555
Debug.Assert(reader.Type == typeof(Utf8JsonReader).MakeByRefType() || reader.Type == typeof(Utf8JsonReader), $"{reader.Type} != {typeof(Utf8JsonReader).MakeByRefType()}");
@@ -562,7 +569,7 @@ private Expression CallPropertyConverterRead(JsonConverter converter, Type type,
562569
else
563570
{
564571
var read = Call(Constant(converter), "Read", Type.EmptyTypes, reader, Constant(type), jsonOptions);
565-
if (read.Type.IsValueType)
572+
if (!type.IsAssignableFromNull() || ConverterHandlesNull(converter))
566573
return read;
567574
else
568575
return Condition(
@@ -586,7 +593,16 @@ private Expression CallPropertyConverterWrite(JsonConverter converter, Expressio
586593
}
587594
else
588595
{
589-
return Call(Constant(converter), nameof(IDotvvmJsonConverter<object>.Write), Type.EmptyTypes, writer, value, jsonOptions);
596+
var write = Call(Constant(converter), nameof(IDotvvmJsonConverter<object>.Write), Type.EmptyTypes, writer, value, jsonOptions);
597+
598+
if (!value.Type.IsAssignableFromNull() || ConverterHandlesNull(converter))
599+
return write;
600+
else
601+
return IfThenElse(
602+
Equal(value, Default(value.Type)),
603+
Call(writer, nameof(Utf8JsonWriter.WriteNullValue), Type.EmptyTypes),
604+
write
605+
);
590606
}
591607
}
592608

@@ -674,6 +690,10 @@ private Expression DeserializePropertyValue(ViewModelPropertyMap property, Expre
674690
var type = existingValue.Type;
675691
Debug.Assert(type.UnwrapNullableType() == property.Type.UnwrapNullableType(), $"{type} != {property.Type}, property: {property.PropertyInfo.DeclaringType}.{property.Name}");
676692

693+
if (GetPropertyConverter(property, type) is {} customConverter)
694+
{
695+
return CallPropertyConverterRead(customConverter, type, reader, jsonOptions, dotvvmState, property.Populate ? existingValue : null);
696+
}
677697
if (ReflectionUtils.IsNullable(existingValue.Type))
678698
{
679699
return Condition(
@@ -682,10 +702,6 @@ private Expression DeserializePropertyValue(ViewModelPropertyMap property, Expre
682702
ifFalse: Convert(DeserializePropertyValue(property, reader, existingValue.UnwrapNullable(throwOnNull: false), jsonOptions, dotvvmState), type)
683703
);
684704
}
685-
if (GetPropertyConverter(property, type) is {} customConverter)
686-
{
687-
return CallPropertyConverterRead(customConverter, type, reader, jsonOptions, dotvvmState, property.Populate ? existingValue : null);
688-
}
689705

690706
if (TryDeserializePrimitive(reader, type) is {} primitive)
691707
{
@@ -730,6 +746,10 @@ private Expression GetSerializeExpression(ViewModelPropertyMap property, Express
730746
Debug.Assert(dotvvmState.Type == typeof(DotvvmSerializationState));
731747
Debug.Assert(value.Type.UnwrapNullableType() == property.Type.UnwrapNullableType(), $"{value.Type} != {property.Type}");
732748

749+
if (GetPropertyConverter(property, value.Type) is {} converter)
750+
{
751+
return CallPropertyConverterWrite(converter, writer, value, jsonOptions, dotvvmState);
752+
}
733753
if (ReflectionUtils.IsNullableType(value.Type))
734754
{
735755
return IfThenElse(
@@ -738,11 +758,6 @@ private Expression GetSerializeExpression(ViewModelPropertyMap property, Express
738758
Call(writer, "WriteNullValue", Type.EmptyTypes)
739759
);
740760
}
741-
742-
if (GetPropertyConverter(property, value.Type) is {} converter)
743-
{
744-
return CallPropertyConverterWrite(converter, writer, value, jsonOptions, dotvvmState);
745-
}
746761
if (TrySerializePrimitive(writer, value) is {} primitive)
747762
{
748763
return primitive;

‎src/Tests/ViewModel/SerializerTests.cs‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1546,6 +1546,38 @@ public void SupportCustomConverters()
15461546
Assert.AreEqual("D", obj3.Property2);
15471547
}
15481548

1549+
[TestMethod]
1550+
public void PropertyJsonConverter_DoesNotReceiveNull()
1551+
{
1552+
var (viewModel, json) = SerializeAndDeserialize(new TestViewModelWithNullPropertyJsonConverter());
1553+
1554+
Assert.IsNull(viewModel.Value);
1555+
Assert.IsNull(json["Value"]);
1556+
}
1557+
1558+
[TestMethod]
1559+
public void PropertyJsonConverter_ReceivesNullWhenSupported()
1560+
{
1561+
var json = Serialize(new TestViewModelWithNullHandlingPropertyJsonConverter(), out _);
1562+
Assert.AreEqual("written null", JsonNode.Parse(json)["Value"].GetValue<string>());
1563+
1564+
var viewModel = Deserialize<TestViewModelWithNullHandlingPropertyJsonConverter>("""{"Value":null}""");
1565+
Assert.AreEqual("read null", viewModel.Value);
1566+
}
1567+
1568+
[TestMethod]
1569+
public void PropertyJsonConverter_SupportsNullableValueType()
1570+
{
1571+
var (viewModel, json) = SerializeAndDeserialize(new TestViewModelWithNullablePropertyJsonConverter { Value = 42 });
1572+
1573+
Assert.AreEqual(42, viewModel.Value);
1574+
Assert.AreEqual("42", json["Value"].GetValue<string>());
1575+
1576+
(viewModel, json) = SerializeAndDeserialize(new TestViewModelWithNullablePropertyJsonConverter());
1577+
Assert.IsNull(viewModel.Value);
1578+
Assert.IsNull(json["Value"]);
1579+
}
1580+
15491581
[TestMethod]
15501582
public void SupportCustomConverters_DynamicDispatch()
15511583
{
@@ -2034,6 +2066,54 @@ public override void Write(Utf8JsonWriter writer, TestViewModelWithCustomConvert
20342066
}
20352067
}
20362068

2069+
public class TestViewModelWithNullPropertyJsonConverter
2070+
{
2071+
[JsonConverter(typeof(NullRejectingStringJsonConverter))]
2072+
public string Value { get; set; }
2073+
}
2074+
2075+
public class NullRejectingStringJsonConverter : JsonConverter<string>
2076+
{
2077+
public override string Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) =>
2078+
reader.TokenType == JsonTokenType.Null ? throw new InvalidOperationException("Converter received null.") : reader.GetString();
2079+
2080+
public override void Write(Utf8JsonWriter writer, string value, JsonSerializerOptions options) =>
2081+
writer.WriteStringValue(value ?? throw new InvalidOperationException("Converter received null."));
2082+
}
2083+
2084+
public class TestViewModelWithNullHandlingPropertyJsonConverter
2085+
{
2086+
[JsonConverter(typeof(NullHandlingStringJsonConverter))]
2087+
public string Value { get; set; }
2088+
}
2089+
2090+
public class NullHandlingStringJsonConverter : JsonConverter<string>
2091+
{
2092+
public override bool HandleNull => true;
2093+
2094+
public override string Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) =>
2095+
reader.TokenType == JsonTokenType.Null ? "read null" : reader.GetString();
2096+
2097+
public override void Write(Utf8JsonWriter writer, string value, JsonSerializerOptions options) =>
2098+
writer.WriteStringValue(value ?? "written null");
2099+
}
2100+
2101+
public class TestViewModelWithNullablePropertyJsonConverter
2102+
{
2103+
[JsonConverter(typeof(NullableIntJsonConverter))]
2104+
public int? Value { get; set; }
2105+
}
2106+
2107+
public class NullableIntJsonConverter : JsonConverter<int?>
2108+
{
2109+
// HandleNull defaults to false, so we shoudln't get null value in either direction
2110+
public override int? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) =>
2111+
int.Parse(reader.GetString());
2112+
2113+
public override void Write(Utf8JsonWriter writer, int? value, JsonSerializerOptions options) =>
2114+
writer.WriteStringValue(value.Value.ToString());
2115+
}
2116+
20372117
[JsonConverter(typeof(TestEnumCustomConverter))]
20382118
public enum TestEnumWithCustomConverter { Case1, Case2, Case3, Case4, Case5, Case6, Case7 }
20392119

0 commit comments

Comments
 (0)