Support for "UseStringProtoEnumValueNames" option

Havret · Mar 25, 2023 · 759020d · 759020d
1 parent 79ff8a5
commit 759020d
Show file tree

Hide file tree

Showing 12 changed files with 200 additions and 25 deletions.
diff --git 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
@@ -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; }
 }
diff --git 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
@@ -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<string, int> _lookup;
+    private readonly Dictionary<int, JsonEncodedText> _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);
+            }
+        }
+    }
+}
diff --git 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;
 
+/// <summary>
+/// Provides a set of options to configure the behavior of the JSON-Protobuf serialization and deserialization process.
+/// </summary>
 public class JsonProtobufSerializerOptions
 {
     /// <summary>
@@ -20,7 +23,7 @@ public class JsonProtobufSerializerOptions
     /// </summary>
     public bool TreatDurationAsTimeSpan { get; set; } = true;
 
-    
+
     /// <summary>
     /// Controls how <see cref="Google.Protobuf.WellKnownTypes.Timestamp"/> fields are handled.
     /// When set to true, <see cref="Google.Protobuf.WellKnownTypes.Timestamp"/> properties will
@@ -29,4 +32,13 @@ public class JsonProtobufSerializerOptions
     /// The default value is true.
     /// </summary>
     public bool TreatTimestampAsDateTime { get; set; } = true;
+
+    /// <summary>
+    /// 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.
+    /// </summary>
+    /// 
+    public bool UseStringProtoEnumValueNames { get; set; }
 }
diff --git 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 @@
     </PropertyGroup>
 
     <ItemGroup>
-        <PackageReference Include="Google.Protobuf" Version="3.19.0"/>
+        <PackageReference Include="Google.Protobuf" Version="3.19.0" />
     </ItemGroup>
 
     <ItemGroup Condition=" '$(TargetFramework)' == 'netstandard2.0' ">
-        <PackageReference Include="System.Text.Json" Version="5.0.0"/>
+        <PackageReference Include="System.Text.Json" Version="5.0.0" />
     </ItemGroup>
 
     <ItemGroup Condition=" '$(TargetFramework)' == 'netstandard2.1' ">
-        <PackageReference Include="System.Text.Json" Version="5.0.0"/>
+        <PackageReference Include="System.Text.Json" Version="5.0.0" />
     </ItemGroup>
 
     <ItemGroup Condition=" '$(TargetFramework)' == 'netcoreapp3.1' ">
-        <PackageReference Include="System.Text.Json" Version="5.0.0"/>
+        <PackageReference Include="System.Text.Json" Version="5.0.0" />
     </ItemGroup>
 
 </Project>
diff --git 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<FieldDescriptor, string> 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
@@ -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<MessageWithVersionMismatchV2>(payload, jsonSerializerOptions);

diff --git a/..._serialize_enum_value_as_number_when_using_proto_value_name_is_not_possible.approved.json b/..._serialize_enum_value_as_number_when_using_proto_value_name_is_not_possible.approved.json
@@ -0,0 +1,3 @@
+{
+  "enumField": 99
+}
diff --git a/...eWithEnumFieldTests.Should_serialize_enum_value_using_proto_enum_value_name.approved.json b/...eWithEnumFieldTests.Should_serialize_enum_value_using_proto_enum_value_name.approved.json
@@ -0,0 +1,3 @@
+{
+  "enumField": "FIRST_OPTION"
+}
diff --git 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<MessageWithEnum>(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<JsonException>(() => JsonSerializer.Deserialize<MessageWithEnum>(invalidPayload, jsonSerializerOptions));
+        exception.Message.ShouldContain("'INVALID_OPTION' is not a valid value for type System.Text.Json.Protobuf.Tests.TestEnum.");
+    }
 }
diff --git a/test/Protobuf.System.Text.Json.Tests/Utils/TestHelper.cs b/test/Protobuf.System.Text.Json.Tests/Utils/TestHelper.cs
@@ -1,16 +1,20 @@
+using System;
 using System.Text.Json;
 using System.Text.Json.Serialization;
 
 namespace Protobuf.System.Text.Json.Tests.Utils;
 
 public class TestHelper
 {
-    public static JsonSerializerOptions CreateJsonSerializerOptions()
+    public static JsonSerializerOptions CreateJsonSerializerOptions(Action<JsonProtobufSerializerOptions>? 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;
     }
 }