From 52db5139c4f122bbd34ef34b4103d0b650e7c218 Mon Sep 17 00:00:00 2001 From: Jon Skeet Date: Fri, 15 Jan 2016 13:45:53 +0000 Subject: Change handling of unknown enums: we now write out the value as a number. --- .../src/Google.Protobuf.Test/JsonFormatterTest.cs | 19 ++++++++----------- csharp/src/Google.Protobuf.Test/JsonParserTest.cs | 10 +++++----- csharp/src/Google.Protobuf/JsonFormatter.cs | 22 ++++++++-------------- csharp/src/Google.Protobuf/JsonParser.cs | 8 ++------ 4 files changed, 23 insertions(+), 36 deletions(-) diff --git a/csharp/src/Google.Protobuf.Test/JsonFormatterTest.cs b/csharp/src/Google.Protobuf.Test/JsonFormatterTest.cs index 98b00f46..b0f58744 100644 --- a/csharp/src/Google.Protobuf.Test/JsonFormatterTest.cs +++ b/csharp/src/Google.Protobuf.Test/JsonFormatterTest.cs @@ -165,34 +165,31 @@ namespace Google.Protobuf } [Test] - public void UnknownEnumValueOmitted_SingleField() + public void UnknownEnumValueNumeric_SingleField() { var message = new TestAllTypes { SingleForeignEnum = (ForeignEnum) 100 }; - AssertJson("{ }", JsonFormatter.Default.Format(message)); + AssertJson("{ 'singleForeignEnum': 100 }", JsonFormatter.Default.Format(message)); } [Test] - public void UnknownEnumValueOmitted_RepeatedField() + public void UnknownEnumValueNumeric_RepeatedField() { var message = new TestAllTypes { RepeatedForeignEnum = { ForeignEnum.FOREIGN_BAZ, (ForeignEnum) 100, ForeignEnum.FOREIGN_FOO } }; - AssertJson("{ 'repeatedForeignEnum': [ 'FOREIGN_BAZ', 'FOREIGN_FOO' ] }", JsonFormatter.Default.Format(message)); + AssertJson("{ 'repeatedForeignEnum': [ 'FOREIGN_BAZ', 100, 'FOREIGN_FOO' ] }", JsonFormatter.Default.Format(message)); } [Test] - public void UnknownEnumValueOmitted_MapField() + public void UnknownEnumValueNumeric_MapField() { - // This matches the C++ behaviour. var message = new TestMap { MapInt32Enum = { { 1, MapEnum.MAP_ENUM_FOO }, { 2, (MapEnum) 100 }, { 3, MapEnum.MAP_ENUM_BAR } } }; - AssertJson("{ 'mapInt32Enum': { '1': 'MAP_ENUM_FOO', '3': 'MAP_ENUM_BAR' } }", JsonFormatter.Default.Format(message)); + AssertJson("{ 'mapInt32Enum': { '1': 'MAP_ENUM_FOO', '2': 100, '3': 'MAP_ENUM_BAR' } }", JsonFormatter.Default.Format(message)); } [Test] - public void UnknownEnumValueOmitted_RepeatedField_AllEntriesUnknown() + public void UnknownEnumValue_RepeatedField_AllEntriesUnknown() { - // *Maybe* we should hold off on writing the "[" until we find that we've got at least one value to write... - // but this is what happens at the moment, and it doesn't seem too awful. var message = new TestAllTypes { RepeatedForeignEnum = { (ForeignEnum) 200, (ForeignEnum) 100 } }; - AssertJson("{ 'repeatedForeignEnum': [ ] }", JsonFormatter.Default.Format(message)); + AssertJson("{ 'repeatedForeignEnum': [ 200, 100 ] }", JsonFormatter.Default.Format(message)); } [Test] diff --git a/csharp/src/Google.Protobuf.Test/JsonParserTest.cs b/csharp/src/Google.Protobuf.Test/JsonParserTest.cs index e8666b36..fb5e083e 100644 --- a/csharp/src/Google.Protobuf.Test/JsonParserTest.cs +++ b/csharp/src/Google.Protobuf.Test/JsonParserTest.cs @@ -876,18 +876,18 @@ namespace Google.Protobuf } [Test] - [TestCase("\"FOREIGN_BAR\"")] - [TestCase("5")] - public void EnumValid(string value) + [TestCase("\"FOREIGN_BAR\"", ForeignEnum.FOREIGN_BAR)] + [TestCase("5", ForeignEnum.FOREIGN_BAR)] + [TestCase("100", (ForeignEnum) 100)] + public void EnumValid(string value, ForeignEnum expectedValue) { string json = "{ \"singleForeignEnum\": " + value + " }"; var parsed = TestAllTypes.Parser.ParseJson(json); - Assert.AreEqual(new TestAllTypes { SingleForeignEnum = ForeignEnum.FOREIGN_BAR }, parsed); + Assert.AreEqual(new TestAllTypes { SingleForeignEnum = expectedValue }, parsed); } [Test] [TestCase("\"NOT_A_VALID_VALUE\"")] - [TestCase("100")] [TestCase("5.5")] public void Enum_Invalid(string value) { diff --git a/csharp/src/Google.Protobuf/JsonFormatter.cs b/csharp/src/Google.Protobuf/JsonFormatter.cs index 573ca766..61961c4c 100644 --- a/csharp/src/Google.Protobuf/JsonFormatter.cs +++ b/csharp/src/Google.Protobuf/JsonFormatter.cs @@ -205,11 +205,6 @@ namespace Google.Protobuf { continue; } - // Omit awkward (single) values such as unknown enum values - if (!field.IsRepeated && !field.IsMap && !CanWriteSingleValue(value)) - { - continue; - } // Okay, all tests complete: let's write the field value... if (!first) @@ -397,7 +392,14 @@ namespace Google.Protobuf } else if (value is System.Enum) { - WriteString(builder, value.ToString()); + if (System.Enum.IsDefined(value.GetType(), value)) + { + WriteString(builder, value.ToString()); + } + else + { + WriteValue(builder, (int) value); + } } else if (value is float || value is double) { @@ -704,10 +706,6 @@ namespace Google.Protobuf bool first = true; foreach (var value in list) { - if (!CanWriteSingleValue(value)) - { - continue; - } if (!first) { builder.Append(PropertySeparator); @@ -725,10 +723,6 @@ namespace Google.Protobuf // This will box each pair. Could use IDictionaryEnumerator, but that's ugly in terms of disposal. foreach (DictionaryEntry pair in dictionary) { - if (!CanWriteSingleValue(pair.Value)) - { - continue; - } if (!first) { builder.Append(PropertySeparator); diff --git a/csharp/src/Google.Protobuf/JsonParser.cs b/csharp/src/Google.Protobuf/JsonParser.cs index 9e5d8711..92029e06 100644 --- a/csharp/src/Google.Protobuf/JsonParser.cs +++ b/csharp/src/Google.Protobuf/JsonParser.cs @@ -617,13 +617,9 @@ namespace Google.Protobuf return (float) value; case FieldType.Enum: CheckInteger(value); - var enumValue = field.EnumType.FindValueByNumber((int) value); - if (enumValue == null) - { - throw new InvalidProtocolBufferException($"Invalid enum value: {value} for enum type: {field.EnumType.FullName}"); - } // Just return it as an int, and let the CLR convert it. - return enumValue.Number; + // Note that we deliberately don't check that it's a known value. + return (int) value; default: throw new InvalidProtocolBufferException($"Unsupported conversion from JSON number for field type {field.FieldType}"); } -- cgit v1.2.3