Update gRPC JSON transcoding to prioritize JSON name (dotnet#47385)

src/Grpc/JsonTranscoding/src/Microsoft.AspNetCore.Grpc.JsonTranscoding/Internal/Json/MessageTypeInfoResolver.cs

@@ -167,9 +167,15 @@ private JsonPropertyInfo CreatePropertyInfo(JsonTypeInfo typeInfo, string name,
     private static Dictionary<string, FieldDescriptor> CreateJsonFieldMap(IList<FieldDescriptor> fields)
     {
         var map = new Dictionary<string, FieldDescriptor>();
+        // The ordering is important here: JsonName takes priority over Name,
+        // which means we need to put JsonName values in the map after *all*
+        // Name keys have been added. See https://github.com/protocolbuffers/protobuf/issues/11987
         foreach (var field in fields)
         {
             map[field.Name] = field;
+        }
+        foreach (var field in fields)
+        {
             map[field.JsonName] = field;
         }
         return map;

src/Grpc/JsonTranscoding/src/Shared/ServiceDescriptorHelpers.cs

@@ -103,29 +103,30 @@ public static bool TryResolveDescriptors(MessageDescriptor messageDescriptor, IL
     private static FieldDescriptor? GetFieldByName(MessageDescriptor messageDescriptor, string fieldName)
     {
         // Search fields by field name and JSON name. Both names can be referenced.
-        // If there are conflicts, then the last field with a name wins.
+        // JSON name takes precendence. If there are conflicts, then the last field with a name wins.
         // This logic matches how properties are used in JSON serialization's MessageTypeInfoResolver.
         var fields = messageDescriptor.Fields.InFieldNumberOrder();
 
-        FieldDescriptor? fieldDescriptor = null;
+        FieldDescriptor? fieldNameDescriptorMatch = null;
         for (var i = fields.Count - 1; i >= 0; i--)
         {
             // We're checking JSON name first, in reverse order through fields.
             // That means the method can exit early on match because the match has the highest precedence.
             var field = fields[i];
             if (field.JsonName == fieldName)
             {
-                fieldDescriptor = field;
-                break;
+                return field;
             }
-            else if (field.Name == fieldName)
+
+            // If there is a match on field name then store the first match.
+            if (fieldNameDescriptorMatch is null && field.Name == fieldName)
             {
-                fieldDescriptor = field;
-                break;
+                fieldNameDescriptorMatch = field;
             }
         }
 
-        return fieldDescriptor;
+        // No match with JSON name. If there is a field name match then return it.
+        return fieldNameDescriptorMatch;
     }
 
     private static object? ConvertValue(object? value, FieldDescriptor descriptor)

src/Grpc/JsonTranscoding/test/Microsoft.AspNetCore.Grpc.JsonTranscoding.Tests/ConverterTests/JsonConverterReadTests.cs

@@ -1,6 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Diagnostics;
 using System.Text.Json;
 using Example.Hello;
 using Google.Protobuf;
@@ -472,17 +473,49 @@ public void Enum_Imported()
         AssertReadJson<SayRequest>(json);
     }
 
-    private TValue AssertReadJson<TValue>(string value, GrpcJsonSettings? settings = null, DescriptorRegistry? descriptorRegistry = null) where TValue : IMessage, new()
+    // See See https://github.com/protocolbuffers/protobuf/issues/11987
+    [Fact]
+    public void JsonNamePriority_JsonName()
+    {
+        var json = @"{""b"":10,""a"":20,""d"":30}";
+
+        // TODO: Current Google.Protobuf version doesn't have fix. Update when available. 3.23.0 or later?
+        var m = AssertReadJson<Issue047349Message>(json, serializeOld: false);
+
+        Assert.Equal(10, m.A);
+        Assert.Equal(20, m.B);
+        Assert.Equal(30, m.C);
+    }
+
+    [Fact]
+    public void JsonNamePriority_FieldNameFallback()
+    {
+        var json = @"{""b"":10,""a"":20,""c"":30}";
+
+        // TODO: Current Google.Protobuf version doesn't have fix. Update when available. 3.23.0 or later?
+        var m = AssertReadJson<Issue047349Message>(json, serializeOld: false);
+
+        Assert.Equal(10, m.A);
+        Assert.Equal(20, m.B);
+        Assert.Equal(30, m.C);
+    }
+
+    private TValue AssertReadJson<TValue>(string value, GrpcJsonSettings? settings = null, DescriptorRegistry? descriptorRegistry = null, bool serializeOld = true) where TValue : IMessage, new()
     {
         var typeRegistery = TypeRegistry.FromFiles(
             HelloRequest.Descriptor.File,
             Timestamp.Descriptor.File);
 
-        var formatter = new JsonParser(new JsonParser.Settings(
-            recursionLimit: int.MaxValue,
-            typeRegistery));
+        TValue? objectOld = default;
 
-        var objectOld = formatter.Parse<TValue>(value);
+        if (serializeOld)
+        {
+            var formatter = new JsonParser(new JsonParser.Settings(
+                recursionLimit: int.MaxValue,
+                typeRegistery));
+
+            objectOld = formatter.Parse<TValue>(value);
+        }
 
         descriptorRegistry ??= new DescriptorRegistry();
         descriptorRegistry.RegisterFileDescriptor(TestHelpers.GetMessageDescriptor(typeof(TValue)).File);
@@ -493,10 +526,15 @@ public void Enum_Imported()
         _output.WriteLine("New:");
         _output.WriteLine(objectNew.ToString());
 
-        _output.WriteLine("Old:");
-        _output.WriteLine(objectOld.ToString());
+        if (serializeOld)
+        {
+            Debug.Assert(objectOld != null);
+
+            _output.WriteLine("Old:");
+            _output.WriteLine(objectOld.ToString());
 
-        Assert.True(objectNew.Equals(objectOld));
+            Assert.True(objectNew.Equals(objectOld));
+        }
 
         return objectNew;
     }

src/Grpc/JsonTranscoding/test/Microsoft.AspNetCore.Grpc.JsonTranscoding.Tests/ConverterTests/JsonConverterWriteTests.cs

@@ -481,7 +481,17 @@ public void Enum_Imported()
         AssertWrittenJson(m);
     }
 
-    private void AssertWrittenJson<TValue>(TValue value, GrpcJsonSettings? settings = null, bool? compareRawStrings = null) where TValue : IMessage
+    // See See https://github.com/protocolbuffers/protobuf/issues/11987
+    [Fact]
+    public void JsonNamePriority()
+    {
+        var m = new Issue047349Message { A = 10, B = 20, C = 30 };
+        var json = AssertWrittenJson(m);
+
+        Assert.Equal(@"{""b"":10,""a"":20,""d"":30}", json);
+    }
+
+    private string AssertWrittenJson<TValue>(TValue value, GrpcJsonSettings? settings = null, bool? compareRawStrings = null) where TValue : IMessage
     {
         var typeRegistery = TypeRegistry.FromFiles(
             HelloRequest.Descriptor.File,
@@ -512,6 +522,8 @@ private void AssertWrittenJson<TValue>(TValue value, GrpcJsonSettings? settings
 
         var comparer = new JsonElementComparer(maxHashDepth: -1, compareRawStrings: compareRawStrings ?? false);
         Assert.True(comparer.Equals(doc1.RootElement, doc2.RootElement));
+
+        return jsonNew;
     }
 
     private static DescriptorRegistry CreateDescriptorRegistry(Type type)

src/Grpc/JsonTranscoding/test/Microsoft.AspNetCore.Grpc.JsonTranscoding.Tests/Microsoft.AspNetCore.Grpc.JsonTranscoding.Tests.csproj

@@ -6,6 +6,7 @@
 
   <ItemGroup>
     <Protobuf Include="Proto\httpbody.proto" GrpcServices="Both" />
+    <Protobuf Include="Proto\Issue047349\message.proto" GrpcServices="Both" />
     <Protobuf Include="Proto\transcoding.proto" GrpcServices="Both" />
     <Protobuf Include="Proto\Issue045270\hello.proto" GrpcServices="Both" />
     <Protobuf Include="Proto\Issue045270\country.proto" GrpcServices="Both" />

src/Grpc/JsonTranscoding/test/Microsoft.AspNetCore.Grpc.JsonTranscoding.Tests/Proto/Issue047349/message.proto

@@ -0,0 +1,7 @@
+syntax = "proto3";
+
+message Issue047349Message {
+  int32 a = 1 [json_name = 'b'];
+  int32 b = 2 [json_name = 'a'];
+  int32 c = 3 [json_name = 'd'];
+}

src/Grpc/JsonTranscoding/test/Microsoft.AspNetCore.Grpc.JsonTranscoding.Tests/UnaryServerCallHandlerTests.cs

@@ -1,6 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Diagnostics;
 using System.Text;
 using System.Text.Json;
 using System.Xml.Linq;
@@ -363,6 +364,78 @@ public async Task HandleCallAsync_MatchingQueryStringValues_JsonName_SetOnReques
         Assert.Equal("TestName!", request!.FieldName);
     }
 
+    [Fact]
+    public async Task HandleCallAsync_MatchingQueryStringValues_JsonNamePriority_JsonName_SetOnRequestMessage()
+    {
+        // Arrange
+        Issue047349Message? requestMessage = null;
+        UnaryServerMethod<JsonTranscodingGreeterService, Issue047349Message, HelloReply> invoker = (s, r, c) =>
+        {
+            requestMessage = r;
+
+            return Task.FromResult(new HelloReply());
+        };
+
+        var unaryServerCallHandler = CreateCallHandler(
+            invoker,
+            CreateServiceMethod("JsonNamePriority", Issue047349Message.Parser, HelloReply.Parser),
+            descriptorInfo: TestHelpers.CreateDescriptorInfo());
+
+        var httpContext = TestHelpers.CreateHttpContext();
+        httpContext.Request.Query = new QueryCollection(new Dictionary<string, StringValues>
+        {
+            ["b"] = "10",
+            ["a"] = "20",
+            ["d"] = "30"
+        });
+
+        // Act
+        await unaryServerCallHandler.HandleCallAsync(httpContext);
+
+        // Assert
+        Debug.Assert(requestMessage != null);
+
+        Assert.Equal(10, requestMessage.A);
+        Assert.Equal(20, requestMessage.B);
+        Assert.Equal(30, requestMessage.C);
+    }
+
+    [Fact]
+    public async Task HandleCallAsync_MatchingQueryStringValues_JsonNamePriority_FieldNameFallback_SetOnRequestMessage()
+    {
+        // Arrange
+        Issue047349Message? requestMessage = null;
+        UnaryServerMethod<JsonTranscodingGreeterService, Issue047349Message, HelloReply> invoker = (s, r, c) =>
+        {
+            requestMessage = r;
+
+            return Task.FromResult(new HelloReply());
+        };
+
+        var unaryServerCallHandler = CreateCallHandler(
+            invoker,
+            CreateServiceMethod("JsonNamePriority", Issue047349Message.Parser, HelloReply.Parser),
+            descriptorInfo: TestHelpers.CreateDescriptorInfo());
+
+        var httpContext = TestHelpers.CreateHttpContext();
+        httpContext.Request.Query = new QueryCollection(new Dictionary<string, StringValues>
+        {
+            ["b"] = "10",
+            ["a"] = "20",
+            ["c"] = "30"
+        });
+
+        // Act
+        await unaryServerCallHandler.HandleCallAsync(httpContext);
+
+        // Assert
+        Debug.Assert(requestMessage != null);
+
+        Assert.Equal(10, requestMessage.A);
+        Assert.Equal(20, requestMessage.B);
+        Assert.Equal(30, requestMessage.C);
+    }
+
     [Fact]
     public async Task HandleCallAsync_MatchingQueryStringValues_JsonNameAndValueObject_SetOnRequestMessage()
     {