Fix deserializing null properties to Protobuf types (#52398)

dotnet · Nov 28, 2023 · c8ef35d · c8ef35d
1 parent 7e9b149
commit c8ef35d
Show file tree

Hide file tree

Showing 3 changed files with 91 additions and 20 deletions.
diff --git a/...coding/src/Microsoft.AspNetCore.Grpc.JsonTranscoding/Internal/Json/JsonConverterHelper.cs b/...coding/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/...ng/src/Microsoft.AspNetCore.Grpc.JsonTranscoding/Internal/Json/MessageTypeInfoResolver.cs b/...ng/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<string, FieldDescriptor> CreateJsonFieldMap(IList<FieldDescriptor> fields)

diff --git a/.../Microsoft.AspNetCore.Grpc.JsonTranscoding.Tests/ConverterTests/JsonConverterReadTests.cs b/.../Microsoft.AspNetCore.Grpc.JsonTranscoding.Tests/ConverterTests/JsonConverterReadTests.cs
@@ -69,6 +69,42 @@ public void ReadObjectProperties()
         AssertReadJson<HelloRequest>(json);
     }
 
+    [Fact]
+    public void ReadNullStringProperty()
+    {
+        var json = @"{
+  ""name"": null
+}";
+
+        AssertReadJson<HelloRequest>(json);
+    }
+
+    [Fact]
+    public void ReadNullIntProperty()
+    {
+        var json = @"{
+  ""age"": null
+}";
+
+        AssertReadJson<HelloRequest>(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<HelloRequest>(json);
+    }
+
     [Fact]
     public void RepeatedStrings()
     {
@@ -112,6 +148,34 @@ public void DataTypes_DefaultValues()
         AssertReadJson<HelloRequest.Types.DataTypes>(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<HelloRequest.Types.DataTypes>(json, descriptorRegistry: serviceDescriptorRegistry);
+    }
+
     [Theory]
     [InlineData(1)]
     [InlineData(-1)]