Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update gRPC JSON transcoding to prioritize JSON name #47385

Merged
merged 2 commits into from
Mar 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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}";
JamesNK marked this conversation as resolved.
Show resolved Hide resolved

// 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);
Expand All @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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" />
Expand Down
Original file line number Diff line number Diff line change
@@ -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'];
}
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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()
{
Expand Down