From c8ef35df22225230b5524ba824b97c5099c87475 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Tue, 28 Nov 2023 09:31:10 +0800 Subject: [PATCH] Fix deserializing null properties to Protobuf types (#52398) --- .../Internal/Json/JsonConverterHelper.cs | 31 ++++----- .../Internal/Json/MessageTypeInfoResolver.cs | 16 ++++- .../ConverterTests/JsonConverterReadTests.cs | 64 +++++++++++++++++++ 3 files changed, 91 insertions(+), 20 deletions(-) diff --git a/src/Grpc/JsonTranscoding/src/Microsoft.AspNetCore.Grpc.JsonTranscoding/Internal/Json/JsonConverterHelper.cs b/src/Grpc/JsonTranscoding/src/Microsoft.AspNetCore.Grpc.JsonTranscoding/Internal/Json/JsonConverterHelper.cs index b86535fca29b..0f5ebefdce18 100644 --- a/src/Grpc/JsonTranscoding/src/Microsoft.AspNetCore.Grpc.JsonTranscoding/Internal/Json/JsonConverterHelper.cs +++ b/src/Grpc/JsonTranscoding/src/Microsoft.AspNetCore.Grpc.JsonTranscoding/Internal/Json/JsonConverterHelper.cs @@ -78,51 +78,46 @@ internal static Type GetFieldType(FieldDescriptor descriptor) } else { - return GetFieldTypeCore(descriptor); - } + // Return nullable field types so the serializer can successfully deserialize null value. + return GetFieldTypeCore(descriptor, nullableType: true); + } } - private static Type GetFieldTypeCore(FieldDescriptor descriptor) + private static Type GetFieldTypeCore(FieldDescriptor descriptor, bool nullableType = false) { switch (descriptor.FieldType) { case FieldType.Bool: - return typeof(bool); + return nullableType ? typeof(bool?) : typeof(bool); case FieldType.Bytes: return typeof(ByteString); case FieldType.String: return typeof(string); case FieldType.Double: - return typeof(double); + return nullableType ? typeof(double?) : typeof(double); case FieldType.SInt32: case FieldType.Int32: case FieldType.SFixed32: - return typeof(int); + return nullableType ? typeof(int?) : typeof(int); case FieldType.Enum: - return descriptor.EnumType.ClrType; + return nullableType ? typeof(Nullable<>).MakeGenericType(descriptor.EnumType.ClrType) : descriptor.EnumType.ClrType; case FieldType.Fixed32: case FieldType.UInt32: - return typeof(uint); + return nullableType ? typeof(uint?) : typeof(uint); case FieldType.Fixed64: case FieldType.UInt64: - return typeof(ulong); + return nullableType ? typeof(ulong?) : typeof(ulong); case FieldType.SFixed64: case FieldType.Int64: case FieldType.SInt64: - return typeof(long); + return nullableType ? typeof(long?) : typeof(long); case FieldType.Float: - return typeof(float); + return nullableType ? typeof(float?) : typeof(float); case FieldType.Message: case FieldType.Group: // Never expect to get this, but... if (ServiceDescriptorHelpers.IsWrapperType(descriptor.MessageType)) { - var t = GetFieldType(descriptor.MessageType.Fields[WrapperValueFieldNumber]); - if (t.IsValueType) - { - return typeof(Nullable<>).MakeGenericType(t); - } - - return t; + return GetFieldType(descriptor.MessageType.Fields[WrapperValueFieldNumber]); } return descriptor.MessageType.ClrType; diff --git a/src/Grpc/JsonTranscoding/src/Microsoft.AspNetCore.Grpc.JsonTranscoding/Internal/Json/MessageTypeInfoResolver.cs b/src/Grpc/JsonTranscoding/src/Microsoft.AspNetCore.Grpc.JsonTranscoding/Internal/Json/MessageTypeInfoResolver.cs index b7c680d93d71..8835897b001f 100644 --- a/src/Grpc/JsonTranscoding/src/Microsoft.AspNetCore.Grpc.JsonTranscoding/Internal/Json/MessageTypeInfoResolver.cs +++ b/src/Grpc/JsonTranscoding/src/Microsoft.AspNetCore.Grpc.JsonTranscoding/Internal/Json/MessageTypeInfoResolver.cs @@ -135,14 +135,26 @@ private JsonPropertyInfo CreatePropertyInfo(JsonTypeInfo typeInfo, string name, throw new InvalidOperationException($"Multiple values specified for oneof {field.RealContainingOneof.Name}."); } - field.Accessor.SetValue((IMessage)o, v); + SetFieldValue(field, (IMessage)o, v); }; } return (o, v) => { - field.Accessor.SetValue((IMessage)o, v); + SetFieldValue(field, (IMessage)o, v); }; + + static void SetFieldValue(FieldDescriptor field, IMessage m, object? v) + { + if (v != null) + { + field.Accessor.SetValue(m, v); + } + else + { + field.Accessor.Clear(m); + } + } } private static Dictionary CreateJsonFieldMap(IList fields) diff --git a/src/Grpc/JsonTranscoding/test/Microsoft.AspNetCore.Grpc.JsonTranscoding.Tests/ConverterTests/JsonConverterReadTests.cs b/src/Grpc/JsonTranscoding/test/Microsoft.AspNetCore.Grpc.JsonTranscoding.Tests/ConverterTests/JsonConverterReadTests.cs index 0215b7536189..15726ca269c3 100644 --- a/src/Grpc/JsonTranscoding/test/Microsoft.AspNetCore.Grpc.JsonTranscoding.Tests/ConverterTests/JsonConverterReadTests.cs +++ b/src/Grpc/JsonTranscoding/test/Microsoft.AspNetCore.Grpc.JsonTranscoding.Tests/ConverterTests/JsonConverterReadTests.cs @@ -69,6 +69,42 @@ public void ReadObjectProperties() AssertReadJson(json); } + [Fact] + public void ReadNullStringProperty() + { + var json = @"{ + ""name"": null +}"; + + AssertReadJson(json); + } + + [Fact] + public void ReadNullIntProperty() + { + var json = @"{ + ""age"": null +}"; + + AssertReadJson(json); + } + + [Fact] + public void ReadNullProperties() + { + var json = @"{ + ""age"": null, + ""nullValue"": null, + ""json_customized_name"": null, + ""field_name"": null, + ""oneof_name1"": null, + ""sub"": null, + ""timestamp_value"": null +}"; + + AssertReadJson(json); + } + [Fact] public void RepeatedStrings() { @@ -112,6 +148,34 @@ public void DataTypes_DefaultValues() AssertReadJson(json, descriptorRegistry: serviceDescriptorRegistry); } + [Fact] + public void DataTypes_NullValues() + { + var json = @"{ + ""singleInt32"": null, + ""singleInt64"": null, + ""singleUint32"": null, + ""singleUint64"": null, + ""singleSint32"": null, + ""singleSint64"": null, + ""singleFixed32"": null, + ""singleFixed64"": null, + ""singleSfixed32"": null, + ""singleSfixed64"": null, + ""singleFloat"": null, + ""singleDouble"": null, + ""singleBool"": null, + ""singleString"": null, + ""singleBytes"": null, + ""singleEnum"": null +}"; + + var serviceDescriptorRegistry = new DescriptorRegistry(); + serviceDescriptorRegistry.RegisterFileDescriptor(JsonTranscodingGreeter.Descriptor.File); + + AssertReadJson(json, descriptorRegistry: serviceDescriptorRegistry); + } + [Theory] [InlineData(1)] [InlineData(-1)]