Skip to content

Commit 914acff

Browse files
JamesNKJamesNK
authored
Update gRPC JSON transcoding to prioritize JSON name (#47385)
1 parent 9c38b37 commit 914acff

File tree

7 files changed

+155
-17
lines changed

7 files changed

+155
-17
lines changed

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,9 +167,15 @@ private JsonPropertyInfo CreatePropertyInfo(JsonTypeInfo typeInfo, string name,
167167
private static Dictionary<string, FieldDescriptor> CreateJsonFieldMap(IList<FieldDescriptor> fields)
168168
{
169169
var map = new Dictionary<string, FieldDescriptor>();
170+
// The ordering is important here: JsonName takes priority over Name,
171+
// which means we need to put JsonName values in the map after *all*
172+
// Name keys have been added. See https://github.com/protocolbuffers/protobuf/issues/11987
170173
foreach (var field in fields)
171174
{
172175
map[field.Name] = field;
176+
}
177+
foreach (var field in fields)
178+
{
173179
map[field.JsonName] = field;
174180
}
175181
return map;

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

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -103,29 +103,30 @@ public static bool TryResolveDescriptors(MessageDescriptor messageDescriptor, IL
103103
private static FieldDescriptor? GetFieldByName(MessageDescriptor messageDescriptor, string fieldName)
104104
{
105105
// Search fields by field name and JSON name. Both names can be referenced.
106-
// If there are conflicts, then the last field with a name wins.
106+
// JSON name takes precendence. If there are conflicts, then the last field with a name wins.
107107
// This logic matches how properties are used in JSON serialization's MessageTypeInfoResolver.
108108
var fields = messageDescriptor.Fields.InFieldNumberOrder();
109109

110-
FieldDescriptor? fieldDescriptor = null;
110+
FieldDescriptor? fieldNameDescriptorMatch = null;
111111
for (var i = fields.Count - 1; i >= 0; i--)
112112
{
113113
// We're checking JSON name first, in reverse order through fields.
114114
// That means the method can exit early on match because the match has the highest precedence.
115115
var field = fields[i];
116116
if (field.JsonName == fieldName)
117117
{
118-
fieldDescriptor = field;
119-
break;
118+
return field;
120119
}
121-
else if (field.Name == fieldName)
120+
121+
// If there is a match on field name then store the first match.
122+
if (fieldNameDescriptorMatch is null && field.Name == fieldName)
122123
{
123-
fieldDescriptor = field;
124-
break;
124+
fieldNameDescriptorMatch = field;
125125
}
126126
}
127127

128-
return fieldDescriptor;
128+
// No match with JSON name. If there is a field name match then return it.
129+
return fieldNameDescriptorMatch;
129130
}
130131

131132
private static object? ConvertValue(object? value, FieldDescriptor descriptor)

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

Lines changed: 46 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4+
using System.Diagnostics;
45
using System.Text.Json;
56
using Example.Hello;
67
using Google.Protobuf;
@@ -472,17 +473,49 @@ public void Enum_Imported()
472473
AssertReadJson<SayRequest>(json);
473474
}
474475

475-
private TValue AssertReadJson<TValue>(string value, GrpcJsonSettings? settings = null, DescriptorRegistry? descriptorRegistry = null) where TValue : IMessage, new()
476+
// See See https://github.com/protocolbuffers/protobuf/issues/11987
477+
[Fact]
478+
public void JsonNamePriority_JsonName()
479+
{
480+
var json = @"{""b"":10,""a"":20,""d"":30}";
481+
482+
// TODO: Current Google.Protobuf version doesn't have fix. Update when available. 3.23.0 or later?
483+
var m = AssertReadJson<Issue047349Message>(json, serializeOld: false);
484+
485+
Assert.Equal(10, m.A);
486+
Assert.Equal(20, m.B);
487+
Assert.Equal(30, m.C);
488+
}
489+
490+
[Fact]
491+
public void JsonNamePriority_FieldNameFallback()
492+
{
493+
var json = @"{""b"":10,""a"":20,""c"":30}";
494+
495+
// TODO: Current Google.Protobuf version doesn't have fix. Update when available. 3.23.0 or later?
496+
var m = AssertReadJson<Issue047349Message>(json, serializeOld: false);
497+
498+
Assert.Equal(10, m.A);
499+
Assert.Equal(20, m.B);
500+
Assert.Equal(30, m.C);
501+
}
502+
503+
private TValue AssertReadJson<TValue>(string value, GrpcJsonSettings? settings = null, DescriptorRegistry? descriptorRegistry = null, bool serializeOld = true) where TValue : IMessage, new()
476504
{
477505
var typeRegistery = TypeRegistry.FromFiles(
478506
HelloRequest.Descriptor.File,
479507
Timestamp.Descriptor.File);
480508

481-
var formatter = new JsonParser(new JsonParser.Settings(
482-
recursionLimit: int.MaxValue,
483-
typeRegistery));
509+
TValue? objectOld = default;
484510

485-
var objectOld = formatter.Parse<TValue>(value);
511+
if (serializeOld)
512+
{
513+
var formatter = new JsonParser(new JsonParser.Settings(
514+
recursionLimit: int.MaxValue,
515+
typeRegistery));
516+
517+
objectOld = formatter.Parse<TValue>(value);
518+
}
486519

487520
descriptorRegistry ??= new DescriptorRegistry();
488521
descriptorRegistry.RegisterFileDescriptor(TestHelpers.GetMessageDescriptor(typeof(TValue)).File);
@@ -493,10 +526,15 @@ public void Enum_Imported()
493526
_output.WriteLine("New:");
494527
_output.WriteLine(objectNew.ToString());
495528

496-
_output.WriteLine("Old:");
497-
_output.WriteLine(objectOld.ToString());
529+
if (serializeOld)
530+
{
531+
Debug.Assert(objectOld != null);
532+
533+
_output.WriteLine("Old:");
534+
_output.WriteLine(objectOld.ToString());
498535

499-
Assert.True(objectNew.Equals(objectOld));
536+
Assert.True(objectNew.Equals(objectOld));
537+
}
500538

501539
return objectNew;
502540
}

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -481,7 +481,17 @@ public void Enum_Imported()
481481
AssertWrittenJson(m);
482482
}
483483

484-
private void AssertWrittenJson<TValue>(TValue value, GrpcJsonSettings? settings = null, bool? compareRawStrings = null) where TValue : IMessage
484+
// See See https://github.com/protocolbuffers/protobuf/issues/11987
485+
[Fact]
486+
public void JsonNamePriority()
487+
{
488+
var m = new Issue047349Message { A = 10, B = 20, C = 30 };
489+
var json = AssertWrittenJson(m);
490+
491+
Assert.Equal(@"{""b"":10,""a"":20,""d"":30}", json);
492+
}
493+
494+
private string AssertWrittenJson<TValue>(TValue value, GrpcJsonSettings? settings = null, bool? compareRawStrings = null) where TValue : IMessage
485495
{
486496
var typeRegistery = TypeRegistry.FromFiles(
487497
HelloRequest.Descriptor.File,
@@ -512,6 +522,8 @@ private void AssertWrittenJson<TValue>(TValue value, GrpcJsonSettings? settings
512522

513523
var comparer = new JsonElementComparer(maxHashDepth: -1, compareRawStrings: compareRawStrings ?? false);
514524
Assert.True(comparer.Equals(doc1.RootElement, doc2.RootElement));
525+
526+
return jsonNew;
515527
}
516528

517529
private static DescriptorRegistry CreateDescriptorRegistry(Type type)

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
<ItemGroup>
88
<Protobuf Include="Proto\httpbody.proto" GrpcServices="Both" />
9+
<Protobuf Include="Proto\Issue047349\message.proto" GrpcServices="Both" />
910
<Protobuf Include="Proto\transcoding.proto" GrpcServices="Both" />
1011
<Protobuf Include="Proto\Issue045270\hello.proto" GrpcServices="Both" />
1112
<Protobuf Include="Proto\Issue045270\country.proto" GrpcServices="Both" />

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
syntax = "proto3";
2+
3+
message Issue047349Message {
4+
int32 a = 1 [json_name = 'b'];
5+
int32 b = 2 [json_name = 'a'];
6+
int32 c = 3 [json_name = 'd'];
7+
}

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

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4+
using System.Diagnostics;
45
using System.Text;
56
using System.Text.Json;
67
using System.Xml.Linq;
@@ -363,6 +364,78 @@ public async Task HandleCallAsync_MatchingQueryStringValues_JsonName_SetOnReques
363364
Assert.Equal("TestName!", request!.FieldName);
364365
}
365366

367+
[Fact]
368+
public async Task HandleCallAsync_MatchingQueryStringValues_JsonNamePriority_JsonName_SetOnRequestMessage()
369+
{
370+
// Arrange
371+
Issue047349Message? requestMessage = null;
372+
UnaryServerMethod<JsonTranscodingGreeterService, Issue047349Message, HelloReply> invoker = (s, r, c) =>
373+
{
374+
requestMessage = r;
375+
376+
return Task.FromResult(new HelloReply());
377+
};
378+
379+
var unaryServerCallHandler = CreateCallHandler(
380+
invoker,
381+
CreateServiceMethod("JsonNamePriority", Issue047349Message.Parser, HelloReply.Parser),
382+
descriptorInfo: TestHelpers.CreateDescriptorInfo());
383+
384+
var httpContext = TestHelpers.CreateHttpContext();
385+
httpContext.Request.Query = new QueryCollection(new Dictionary<string, StringValues>
386+
{
387+
["b"] = "10",
388+
["a"] = "20",
389+
["d"] = "30"
390+
});
391+
392+
// Act
393+
await unaryServerCallHandler.HandleCallAsync(httpContext);
394+
395+
// Assert
396+
Debug.Assert(requestMessage != null);
397+
398+
Assert.Equal(10, requestMessage.A);
399+
Assert.Equal(20, requestMessage.B);
400+
Assert.Equal(30, requestMessage.C);
401+
}
402+
403+
[Fact]
404+
public async Task HandleCallAsync_MatchingQueryStringValues_JsonNamePriority_FieldNameFallback_SetOnRequestMessage()
405+
{
406+
// Arrange
407+
Issue047349Message? requestMessage = null;
408+
UnaryServerMethod<JsonTranscodingGreeterService, Issue047349Message, HelloReply> invoker = (s, r, c) =>
409+
{
410+
requestMessage = r;
411+
412+
return Task.FromResult(new HelloReply());
413+
};
414+
415+
var unaryServerCallHandler = CreateCallHandler(
416+
invoker,
417+
CreateServiceMethod("JsonNamePriority", Issue047349Message.Parser, HelloReply.Parser),
418+
descriptorInfo: TestHelpers.CreateDescriptorInfo());
419+
420+
var httpContext = TestHelpers.CreateHttpContext();
421+
httpContext.Request.Query = new QueryCollection(new Dictionary<string, StringValues>
422+
{
423+
["b"] = "10",
424+
["a"] = "20",
425+
["c"] = "30"
426+
});
427+
428+
// Act
429+
await unaryServerCallHandler.HandleCallAsync(httpContext);
430+
431+
// Assert
432+
Debug.Assert(requestMessage != null);
433+
434+
Assert.Equal(10, requestMessage.A);
435+
Assert.Equal(20, requestMessage.B);
436+
Assert.Equal(30, requestMessage.C);
437+
}
438+
366439
[Fact]
367440
public async Task HandleCallAsync_MatchingQueryStringValues_JsonNameAndValueObject_SetOnRequestMessage()
368441
{

0 commit comments

Comments
 (0)