Skip to content

Commit

Permalink
Make json_name take priority over name (fully) in C# parsing (#12262)
Browse files Browse the repository at this point in the history
Fixes #11987

(testprotos.pb and UnittestIssues.pb.cs are both generated; no need to review.)

Closes #12262

COPYBARA_INTEGRATE_REVIEW=#12262 from jskeet:json-mapping 9578524
PiperOrigin-RevId: 518186721
  • Loading branch information
jskeet authored and copybara-github committed Mar 21, 2023
1 parent 38ab857 commit 4326e0f
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 29 deletions.
64 changes: 35 additions & 29 deletions csharp/protos/unittest_issues.proto
Original file line number Diff line number Diff line change
Expand Up @@ -33,24 +33,25 @@ syntax = "proto3";
// These proto descriptors have at one time been reported as an issue or defect.
// They are kept here to replicate the issue, and continue to verify the fix.

// Issue: Non-"Google.Protobuffers" namespace will ensure that protobuffer library types are qualified
option csharp_namespace = "UnitTest.Issues.TestProtos";

// Issue: Non-"Google.Protobuffers" namespace will ensure that protobuffer
// library types are qualified
package unittest_issues;

import "google/protobuf/struct.proto";

option csharp_namespace = "UnitTest.Issues.TestProtos";

// Issue 307: when generating doubly-nested types, any references
// should be of the form A.Types.B.Types.C.
message Issue307 {
message NestedOnce {
message NestedTwice {
}
message NestedTwice {}
}
}

// Old issue 13: http://code.google.com/p/protobuf-csharp-port/issues/detail?id=13
// New issue 309: https://github.com/protocolbuffers/protobuf/issues/309
// Old issue 13:
// http://code.google.com/p/protobuf-csharp-port/issues/detail?id=13 New issue
// 309: https://github.com/protocolbuffers/protobuf/issues/309

// message A {
// optional int32 _A = 1;
Expand All @@ -60,9 +61,9 @@ message Issue307 {
// optional int32 B_ = 1;
// }

//message AB {
// optional int32 a_b = 1;
//}
// message AB {
// optional int32 a_b = 1;
// }

// Similar issue with numeric names
// Java code failed too, so probably best for this to be a restriction.
Expand All @@ -74,39 +75,40 @@ message Issue307 {
// issue 19 - negative enum values

enum NegativeEnum {
NEGATIVE_ENUM_ZERO = 0;
FiveBelow = -5;
MinusOne = -1;
NEGATIVE_ENUM_ZERO = 0;
FiveBelow = -5;
MinusOne = -1;
}

message NegativeEnumMessage {
NegativeEnum value = 1;
repeated NegativeEnum values = 2 [packed = false];
repeated NegativeEnum packed_values = 3 [packed=true];
NegativeEnum value = 1;
repeated NegativeEnum values = 2 [packed = false];
repeated NegativeEnum packed_values = 3 [packed = true];
}

// Issue 21: http://code.google.com/p/protobuf-csharp-port/issues/detail?id=21
// Decorate fields with [deprecated=true] as [System.Obsolete]

message DeprecatedChild {
option deprecated = true;
option deprecated = true;
}

enum DeprecatedEnum {
option deprecated = true;
DEPRECATED_ZERO = 0 [deprecated = true];
one = 1;
option deprecated = true;

DEPRECATED_ZERO = 0 [deprecated = true];
one = 1;
}

message DeprecatedFieldsMessage {
int32 PrimitiveValue = 1 [deprecated = true];
repeated int32 PrimitiveArray = 2 [deprecated = true];
int32 PrimitiveValue = 1 [deprecated = true];
repeated int32 PrimitiveArray = 2 [deprecated = true];

DeprecatedChild MessageValue = 3 [deprecated = true];
repeated DeprecatedChild MessageArray = 4 [deprecated = true];
DeprecatedChild MessageValue = 3 [deprecated = true];
repeated DeprecatedChild MessageArray = 4 [deprecated = true];

DeprecatedEnum EnumValue = 5 [deprecated = true];
repeated DeprecatedEnum EnumArray = 6 [deprecated = true];
DeprecatedEnum EnumValue = 5 [deprecated = true];
repeated DeprecatedEnum EnumArray = 6 [deprecated = true];
}

// Issue 45: http://code.google.com/p/protobuf-csharp-port/issues/detail?id=45
Expand All @@ -116,8 +118,7 @@ message ItemField {

message ReservedNames {
// Force a nested type called Types
message SomeNestedType {
}
message SomeNestedType {}

int32 types = 1;
int32 descriptor = 2;
Expand Down Expand Up @@ -148,7 +149,6 @@ message TestJsonFieldOrdering {
int32 o2_int32 = 6;
string o2_string = 3;
}

}

message TestJsonName {
Expand Down Expand Up @@ -218,3 +218,9 @@ message DisambiguateCommonMembers {
int32 on_construction = 11;
int32 parser = 12;
}

message Issue11987Message {
int32 a = 1 [json_name = 'b'];
int32 b = 2 [json_name = 'a'];
int32 c = 3 [json_name = 'd'];
}
12 changes: 12 additions & 0 deletions csharp/src/Google.Protobuf.Test/JsonFormatterTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -630,6 +630,18 @@ public void Wrappers_Standalone(System.Type wrapperType, object value, string ex
Assert.AreEqual(expectedJson, JsonFormatter.Default.Format(populated));
}

// See See https://github.com/protocolbuffers/protobuf/issues/11987
[Test]
public void JsonNamePriority()
{
// This tests both the formatter and the parser, but the issue was when parsing.
var original = new Issue11987Message { A = 10, B = 20, C = 30 };
var json = JsonFormatter.Default.Format(original);
AssertJson("{ 'b': 10, 'a': 20, 'd': 30 }", json);
var parsed = Issue11987Message.Parser.ParseJson(json);
Assert.AreEqual(original, parsed);
}

// Sanity tests for WriteValue. Not particularly comprehensive, as it's all covered above already,
// as FormatMessage uses WriteValue.

Expand Down
6 changes: 6 additions & 0 deletions csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,15 @@ internal MessageDescriptor(DescriptorProto proto, FileDescriptor file, MessageDe
private static ReadOnlyDictionary<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 new ReadOnlyDictionary<string, FieldDescriptor>(map);
Expand Down

0 comments on commit 4326e0f

Please sign in to comment.