From 759020df78e95ace824a4263f6529d62f6fedbbf Mon Sep 17 00:00:00 2001 From: Havret Date: Tue, 21 Mar 2023 21:08:33 +0100 Subject: [PATCH] Support for "UseStringProtoEnumValueNames" option --- .../NewtonsoftProtobufJsonConverter.cs | 4 +- src/Protobuf.System.Text.Json/FieldInfo.cs | 3 +- .../InternalConverterFactory.cs | 9 +- .../InternalConverters/ProtoEnumConverter.cs | 57 +++++++++++++ .../JsonProtobufSerializerOptions.cs | 14 +++- .../Protobuf.System.Text.Json.csproj | 8 +- .../ProtobufConverter.cs | 24 +++--- .../ContractEvolutionTests.cs | 2 +- ...o_value_name_is_not_possible.approved.json | 3 + ..._using_proto_enum_value_name.approved.json | 3 + .../MessageWithEnumFieldTests.cs | 84 +++++++++++++++++++ .../Utils/TestHelper.cs | 14 ++-- 12 files changed, 200 insertions(+), 25 deletions(-) create mode 100644 src/Protobuf.System.Text.Json/InternalConverters/ProtoEnumConverter.cs create mode 100644 test/Protobuf.System.Text.Json.Tests/MessageWithEnumFieldTests.Should_serialize_enum_value_as_number_when_using_proto_value_name_is_not_possible.approved.json create mode 100644 test/Protobuf.System.Text.Json.Tests/MessageWithEnumFieldTests.Should_serialize_enum_value_using_proto_enum_value_name.approved.json diff --git a/benchmark/Protobuf.System.Text.Json.Benchmark.Shared/NewtonsoftProtobufJsonConverter.cs b/benchmark/Protobuf.System.Text.Json.Benchmark.Shared/NewtonsoftProtobufJsonConverter.cs index b026063..7b58606 100644 --- a/benchmark/Protobuf.System.Text.Json.Benchmark.Shared/NewtonsoftProtobufJsonConverter.cs +++ b/benchmark/Protobuf.System.Text.Json.Benchmark.Shared/NewtonsoftProtobufJsonConverter.cs @@ -25,12 +25,12 @@ public NewtonsoftProtobufJsonConverter(int messageRecursionLimit = 3, bool forma _jsonFormatter = new JsonFormatter(formatterSettings); } - public override void WriteJson(JsonWriter writer, object? value, Newtonsoft.Json.JsonSerializer serializer) + public override void WriteJson(JsonWriter writer, object? value, JsonSerializer serializer) { writer.WriteRawValue(_jsonFormatter.Format((IMessage?) value)); } - public override object? ReadJson(JsonReader reader, Type objectType, object? existingValue, Newtonsoft.Json.JsonSerializer serializer) + public override object? ReadJson(JsonReader reader, Type objectType, object? existingValue, JsonSerializer serializer) { return _jsonParser.Parse(JObject.Load(reader).ToString(), ExtractMessageDescriptor(objectType)); } diff --git a/src/Protobuf.System.Text.Json/FieldInfo.cs b/src/Protobuf.System.Text.Json/FieldInfo.cs index 8ab45d8..5fd3f0f 100644 --- a/src/Protobuf.System.Text.Json/FieldInfo.cs +++ b/src/Protobuf.System.Text.Json/FieldInfo.cs @@ -6,10 +6,11 @@ namespace Protobuf.System.Text.Json; internal class FieldInfo { public IFieldAccessor Accessor { get; set; } = null!; - public InternalConverter? Converter { get; set; } + public InternalConverter Converter { get; set; } = null!; public bool IsRepeated { get; set; } public Type FieldType { get; set; } = null!; public string JsonName { get; set; } = null!; public bool IsOneOf { get; set; } public bool IsMap { get; set; } + public EnumDescriptor? EnumType { get; set; } } \ No newline at end of file diff --git a/src/Protobuf.System.Text.Json/InternalConverters/InternalConverterFactory.cs b/src/Protobuf.System.Text.Json/InternalConverters/InternalConverterFactory.cs index 8bc0fcf..ce9241d 100644 --- a/src/Protobuf.System.Text.Json/InternalConverters/InternalConverterFactory.cs +++ b/src/Protobuf.System.Text.Json/InternalConverters/InternalConverterFactory.cs @@ -1,8 +1,10 @@ +using System.Text.Json; + namespace Protobuf.System.Text.Json.InternalConverters; internal class InternalConverterFactory { - public static InternalConverter Create(FieldInfo fieldInfo) + public static InternalConverter Create(FieldInfo fieldInfo, JsonSerializerOptions jsonSerializerOptions) { if (fieldInfo.IsMap) { @@ -16,6 +18,11 @@ public static InternalConverter Create(FieldInfo fieldInfo) var internalConverter = (InternalConverter) Activator.CreateInstance(converterType)!; return internalConverter; } + else if (fieldInfo.EnumType != null) + { + var internalConverter = (InternalConverter) Activator.CreateInstance(typeof(ProtoEnumConverter), args: new object[] { fieldInfo.EnumType, jsonSerializerOptions.Encoder! })!; + return internalConverter; + } else { var converterType = typeof(FieldConverter<>).MakeGenericType(fieldInfo.FieldType); diff --git a/src/Protobuf.System.Text.Json/InternalConverters/ProtoEnumConverter.cs b/src/Protobuf.System.Text.Json/InternalConverters/ProtoEnumConverter.cs new file mode 100644 index 0000000..608fee3 --- /dev/null +++ b/src/Protobuf.System.Text.Json/InternalConverters/ProtoEnumConverter.cs @@ -0,0 +1,57 @@ +using System.Text.Encodings.Web; +using System.Text.Json; +using Google.Protobuf; +using Google.Protobuf.Reflection; + +namespace Protobuf.System.Text.Json.InternalConverters; + +internal class ProtoEnumConverter : InternalConverter +{ + private readonly Dictionary _lookup; + private readonly Dictionary _reversedLookup; + private readonly Type _clrType; + + public ProtoEnumConverter(EnumDescriptor fieldInfoEnumType, JavaScriptEncoder? encoder) + { + _clrType = fieldInfoEnumType.ClrType; + _lookup = fieldInfoEnumType.Values.ToDictionary(x => x.Name, x => x.Number); + _reversedLookup = fieldInfoEnumType.Values.ToDictionary(x => x.Number, x => JsonEncodedText.Encode(x.Name, encoder)); + } + + public override void Write(Utf8JsonWriter writer, object value, JsonSerializerOptions options) + { + var intValue = (int) value; + if (_reversedLookup.TryGetValue(intValue, out var stringValue)) + { + writer.WriteStringValue(stringValue); + } + else + { + writer.WriteNumberValue(intValue); + } + } + + public override void Read(ref Utf8JsonReader reader, IMessage obj, Type typeToConvert, JsonSerializerOptions options, IFieldAccessor fieldAccessor) + { + if (reader.TokenType == JsonTokenType.String) + { + if (reader.GetString() is { } stringValue) + { + if (_lookup.TryGetValue(stringValue, out var value)) + { + fieldAccessor.SetValue(obj, value); + return; + } + + throw new JsonException($"'{stringValue}' is not a valid value for type {_clrType.FullName}."); + } + } + else if (reader.TokenType == JsonTokenType.Number) + { + if (reader.TryGetInt32(out var value)) + { + fieldAccessor.SetValue(obj, value); + } + } + } +} \ No newline at end of file diff --git a/src/Protobuf.System.Text.Json/JsonProtobufSerializerOptions.cs b/src/Protobuf.System.Text.Json/JsonProtobufSerializerOptions.cs index 6fe6797..46095e7 100644 --- a/src/Protobuf.System.Text.Json/JsonProtobufSerializerOptions.cs +++ b/src/Protobuf.System.Text.Json/JsonProtobufSerializerOptions.cs @@ -1,5 +1,8 @@ namespace Protobuf.System.Text.Json; +/// +/// Provides a set of options to configure the behavior of the JSON-Protobuf serialization and deserialization process. +/// public class JsonProtobufSerializerOptions { /// @@ -20,7 +23,7 @@ public class JsonProtobufSerializerOptions /// public bool TreatDurationAsTimeSpan { get; set; } = true; - + /// /// Controls how fields are handled. /// When set to true, properties will @@ -29,4 +32,13 @@ public class JsonProtobufSerializerOptions /// The default value is true. /// public bool TreatTimestampAsDateTime { get; set; } = true; + + /// + /// Controls how enums defined as part of the protobuf contract are handled. + /// When set to true enum values will be serialized as strings based on the naming + /// specified in the .proto file. The same format will be expected during deserialization. + /// The default value is false. + /// + /// + public bool UseStringProtoEnumValueNames { get; set; } } \ No newline at end of file diff --git a/src/Protobuf.System.Text.Json/Protobuf.System.Text.Json.csproj b/src/Protobuf.System.Text.Json/Protobuf.System.Text.Json.csproj index 3e67e97..bcab01d 100644 --- a/src/Protobuf.System.Text.Json/Protobuf.System.Text.Json.csproj +++ b/src/Protobuf.System.Text.Json/Protobuf.System.Text.Json.csproj @@ -22,19 +22,19 @@ - + - + - + - + diff --git a/src/Protobuf.System.Text.Json/ProtobufConverter.cs b/src/Protobuf.System.Text.Json/ProtobufConverter.cs index d53f39a..cb6ce2f 100644 --- a/src/Protobuf.System.Text.Json/ProtobufConverter.cs +++ b/src/Protobuf.System.Text.Json/ProtobufConverter.cs @@ -17,7 +17,7 @@ namespace Protobuf.System.Text.Json; public ProtobufConverter(JsonSerializerOptions jsonSerializerOptions, JsonProtobufSerializerOptions jsonProtobufSerializerOptions) { _defaultIgnoreCondition = jsonSerializerOptions.DefaultIgnoreCondition; - + var type = typeof(T); var propertyTypeLookup = type.GetProperties().ToDictionary(x => x.Name, x => x.PropertyType); @@ -27,14 +27,20 @@ public ProtobufConverter(JsonSerializerOptions jsonSerializerOptions, JsonProtob var convertNameFunc = GetConvertNameFunc(jsonSerializerOptions.PropertyNamingPolicy, jsonProtobufSerializerOptions.UseProtobufJsonNames); - _fields = messageDescriptor.Fields.InDeclarationOrder().Select(fieldDescriptor => new FieldInfo + _fields = messageDescriptor.Fields.InDeclarationOrder().Select(fieldDescriptor => { - Accessor = fieldDescriptor.Accessor, - IsRepeated = fieldDescriptor.IsRepeated, - IsMap = fieldDescriptor.IsMap, - FieldType = FieldTypeResolver.ResolverFieldType(fieldDescriptor, propertyTypeLookup), - JsonName = convertNameFunc(fieldDescriptor), - IsOneOf = fieldDescriptor.ContainingOneof != null + var fieldInfo = new FieldInfo + { + Accessor = fieldDescriptor.Accessor, + IsRepeated = fieldDescriptor.IsRepeated, + EnumType = jsonProtobufSerializerOptions.UseStringProtoEnumValueNames ? fieldDescriptor.EnumType : null, + IsMap = fieldDescriptor.IsMap, + FieldType = FieldTypeResolver.ResolverFieldType(fieldDescriptor, propertyTypeLookup), + JsonName = convertNameFunc(fieldDescriptor), + IsOneOf = fieldDescriptor.ContainingOneof != null, + }; + fieldInfo.Converter = InternalConverterFactory.Create(fieldInfo, jsonSerializerOptions); + return fieldInfo; }).ToArray(); var stringComparer = jsonSerializerOptions.PropertyNameCaseInsensitive ? StringComparer.OrdinalIgnoreCase : StringComparer.Ordinal; @@ -98,7 +104,6 @@ private static Func GetConvertNameFunc(JsonNamingPolicy } reader.Read(); - fieldInfo.Converter ??= InternalConverterFactory.Create(fieldInfo); fieldInfo.Converter.Read(ref reader, obj, fieldInfo.FieldType, options, fieldInfo.Accessor); } @@ -128,7 +133,6 @@ public override void Write(Utf8JsonWriter writer, T? value, JsonSerializerOption if (_defaultIgnoreCondition is JsonIgnoreCondition.Never or not JsonIgnoreCondition.WhenWritingDefault) { writer.WritePropertyName(fieldInfo.JsonName); - fieldInfo.Converter ??= InternalConverterFactory.Create(fieldInfo); fieldInfo.Converter.Write(writer, propertyValue, options); } } diff --git a/test/Protobuf.System.Text.Json.Tests/ContractEvolutionTests.cs b/test/Protobuf.System.Text.Json.Tests/ContractEvolutionTests.cs index bdbb656..ef47041 100644 --- a/test/Protobuf.System.Text.Json.Tests/ContractEvolutionTests.cs +++ b/test/Protobuf.System.Text.Json.Tests/ContractEvolutionTests.cs @@ -38,7 +38,7 @@ public void Should_deserialize_the_old_version_of_a_message_using_the_new_versio DoubleProperty = 1d, }; var jsonSerializerOptions = TestHelper.CreateJsonSerializerOptions(); - + // Act var payload = JsonSerializer.Serialize(msg, jsonSerializerOptions); var deserialized = JsonSerializer.Deserialize(payload, jsonSerializerOptions); diff --git a/test/Protobuf.System.Text.Json.Tests/MessageWithEnumFieldTests.Should_serialize_enum_value_as_number_when_using_proto_value_name_is_not_possible.approved.json b/test/Protobuf.System.Text.Json.Tests/MessageWithEnumFieldTests.Should_serialize_enum_value_as_number_when_using_proto_value_name_is_not_possible.approved.json new file mode 100644 index 0000000..7277345 --- /dev/null +++ b/test/Protobuf.System.Text.Json.Tests/MessageWithEnumFieldTests.Should_serialize_enum_value_as_number_when_using_proto_value_name_is_not_possible.approved.json @@ -0,0 +1,3 @@ +{ + "enumField": 99 +} \ No newline at end of file diff --git a/test/Protobuf.System.Text.Json.Tests/MessageWithEnumFieldTests.Should_serialize_enum_value_using_proto_enum_value_name.approved.json b/test/Protobuf.System.Text.Json.Tests/MessageWithEnumFieldTests.Should_serialize_enum_value_using_proto_enum_value_name.approved.json new file mode 100644 index 0000000..e55ecdf --- /dev/null +++ b/test/Protobuf.System.Text.Json.Tests/MessageWithEnumFieldTests.Should_serialize_enum_value_using_proto_enum_value_name.approved.json @@ -0,0 +1,3 @@ +{ + "enumField": "FIRST_OPTION" +} \ No newline at end of file diff --git a/test/Protobuf.System.Text.Json.Tests/MessageWithEnumFieldTests.cs b/test/Protobuf.System.Text.Json.Tests/MessageWithEnumFieldTests.cs index 800a4b9..5ce93d6 100644 --- a/test/Protobuf.System.Text.Json.Tests/MessageWithEnumFieldTests.cs +++ b/test/Protobuf.System.Text.Json.Tests/MessageWithEnumFieldTests.cs @@ -45,4 +45,88 @@ public void Should_deserialize_message_with_with_enum_field() deserialized.ShouldNotBeNull(); deserialized.ShouldBeEquivalentTo(msg); } + + [Fact] + public void Should_serialize_enum_value_using_proto_enum_value_name() + { + // Arrange + var msg = new MessageWithEnum + { + EnumField = TestEnum.FirstOption + }; + var jsonSerializerOptions = TestHelper.CreateJsonSerializerOptions(options => + { + options.UseStringProtoEnumValueNames = true; + }); + + // Act + var serialized = JsonSerializer.Serialize(msg, jsonSerializerOptions); + + // Assert + var approver = new ExplicitApprover(); + approver.VerifyJson(serialized); + } + + [Fact] + public void Should_serialize_enum_value_as_number_when_using_proto_value_name_is_not_possible() + { + // Arrange + var msg = new MessageWithEnum + { + EnumField = (TestEnum) 99 + }; + var jsonSerializerOptions = TestHelper.CreateJsonSerializerOptions(options => + { + options.UseStringProtoEnumValueNames = true; + }); + + // Act + var serialized = JsonSerializer.Serialize(msg, jsonSerializerOptions); + + // Assert + var approver = new ExplicitApprover(); + approver.VerifyJson(serialized); + } + + [Fact] + public void Should_deserialize_message_with_enum_field_when_value_serialized_using_proto_value_name() + { + // Arrange + var msg = new MessageWithEnum + { + EnumField = TestEnum.FirstOption + }; + var jsonSerializerOptions = TestHelper.CreateJsonSerializerOptions(options => + { + options.UseStringProtoEnumValueNames = true; + }); + + // Act + var serialized = JsonSerializer.Serialize(msg, jsonSerializerOptions); + var deserialized = JsonSerializer.Deserialize(serialized, jsonSerializerOptions); + + // Assert + deserialized.ShouldNotBeNull(); + deserialized.ShouldBeEquivalentTo(msg); + } + + [Fact] + public void Should_throw_exception_when_string_enum_value_cannot_be_deserialized() + { + // Arrange + var msg = new MessageWithEnum + { + EnumField = TestEnum.FirstOption + }; + var jsonSerializerOptions = TestHelper.CreateJsonSerializerOptions(options => + { + options.UseStringProtoEnumValueNames = true; + }); + var serialized = JsonSerializer.Serialize(msg, jsonSerializerOptions); + var invalidPayload = serialized.Replace("FIRST_OPTION", "INVALID_OPTION"); + + // Act & Assert + var exception = Assert.Throws(() => JsonSerializer.Deserialize(invalidPayload, jsonSerializerOptions)); + exception.Message.ShouldContain("'INVALID_OPTION' is not a valid value for type System.Text.Json.Protobuf.Tests.TestEnum."); + } } \ No newline at end of file diff --git a/test/Protobuf.System.Text.Json.Tests/Utils/TestHelper.cs b/test/Protobuf.System.Text.Json.Tests/Utils/TestHelper.cs index d7d39cb..c139f92 100644 --- a/test/Protobuf.System.Text.Json.Tests/Utils/TestHelper.cs +++ b/test/Protobuf.System.Text.Json.Tests/Utils/TestHelper.cs @@ -1,3 +1,4 @@ +using System; using System.Text.Json; using System.Text.Json.Serialization; @@ -5,12 +6,15 @@ namespace Protobuf.System.Text.Json.Tests.Utils; public class TestHelper { - public static JsonSerializerOptions CreateJsonSerializerOptions() + public static JsonSerializerOptions CreateJsonSerializerOptions(Action? configure = null) { - var jsonSerializerOptions = new JsonSerializerOptions(); - jsonSerializerOptions.PropertyNamingPolicy = JsonNamingPolicy.CamelCase; - jsonSerializerOptions.DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull; - jsonSerializerOptions.AddProtobufSupport(); + var jsonSerializerOptions = new JsonSerializerOptions + { + PropertyNamingPolicy = JsonNamingPolicy.CamelCase, + DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull + + }; + jsonSerializerOptions.AddProtobufSupport(configure ?? (_ => { })); return jsonSerializerOptions; } } \ No newline at end of file