From 9a60de53a11ec07b8d57455f5cde301a9d80f087 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristoffer=20Lerb=C3=A6k=20Pedersen?= Date: Thu, 24 Aug 2023 14:53:23 +0200 Subject: [PATCH 1/6] Pass correct type to error message for unsupported field types. --- src/Protobuf.System.Text.Json/FieldTypeResolver.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Protobuf.System.Text.Json/FieldTypeResolver.cs b/src/Protobuf.System.Text.Json/FieldTypeResolver.cs index c0f70fc..189f6e7 100644 --- a/src/Protobuf.System.Text.Json/FieldTypeResolver.cs +++ b/src/Protobuf.System.Text.Json/FieldTypeResolver.cs @@ -58,7 +58,7 @@ public static Type ResolverFieldType(FieldDescriptor fieldDescriptor, Dictionary return propertyTypeLookup[fieldDescriptor.PropertyName]; default: throw new ArgumentOutOfRangeException(nameof(fieldDescriptor), - $"FieldType: '{fieldDescriptor}' is not supported."); + $"FieldType: '{fieldDescriptor.FieldType}' is not supported."); } } } \ No newline at end of file From afb15316b73b7937a9d23a8522a77f055af401d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristoffer=20Lerb=C3=A6k=20Pedersen?= Date: Thu, 24 Aug 2023 14:53:37 +0200 Subject: [PATCH 2/6] Add ByteString support. --- src/Protobuf.System.Text.Json/FieldTypeResolver.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Protobuf.System.Text.Json/FieldTypeResolver.cs b/src/Protobuf.System.Text.Json/FieldTypeResolver.cs index 189f6e7..5600087 100644 --- a/src/Protobuf.System.Text.Json/FieldTypeResolver.cs +++ b/src/Protobuf.System.Text.Json/FieldTypeResolver.cs @@ -32,6 +32,8 @@ public static Type ResolverFieldType(FieldDescriptor fieldDescriptor, Dictionary return typeof(bool); case FieldType.String: return typeof(string); + case FieldType.Bytes: + return typeof(Google.Protobuf.ByteString); case FieldType.Message when fieldDescriptor.MessageType.ClrType is { } clrType: if (clrType == typeof(DoubleValue)) return typeof(double?); From 8de75abd2a730dadc5612a95ff5b22127fef823d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristoffer=20Lerb=C3=A6k=20Pedersen?= Date: Thu, 24 Aug 2023 14:58:17 +0200 Subject: [PATCH 3/6] Use using directive for ByteString's namespace. --- src/Protobuf.System.Text.Json/FieldTypeResolver.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Protobuf.System.Text.Json/FieldTypeResolver.cs b/src/Protobuf.System.Text.Json/FieldTypeResolver.cs index 5600087..d55b13e 100644 --- a/src/Protobuf.System.Text.Json/FieldTypeResolver.cs +++ b/src/Protobuf.System.Text.Json/FieldTypeResolver.cs @@ -1,3 +1,4 @@ +using Google.Protobuf; using Google.Protobuf.Reflection; using Google.Protobuf.WellKnownTypes; using Type = System.Type; @@ -33,7 +34,7 @@ public static Type ResolverFieldType(FieldDescriptor fieldDescriptor, Dictionary case FieldType.String: return typeof(string); case FieldType.Bytes: - return typeof(Google.Protobuf.ByteString); + return typeof(ByteString); case FieldType.Message when fieldDescriptor.MessageType.ClrType is { } clrType: if (clrType == typeof(DoubleValue)) return typeof(double?); From c8f058b9f21fc12c41b1c009588ae06b29cebafd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristoffer=20Lerb=C3=A6k=20Pedersen?= Date: Fri, 25 Aug 2023 12:02:54 +0200 Subject: [PATCH 4/6] Add ByteString converter --- .../InternalConverters/ByteStringConverter.cs | 25 +++++++++++++++++++ .../InternalConverterFactory.cs | 6 +++++ 2 files changed, 31 insertions(+) create mode 100644 src/Protobuf.System.Text.Json/InternalConverters/ByteStringConverter.cs diff --git a/src/Protobuf.System.Text.Json/InternalConverters/ByteStringConverter.cs b/src/Protobuf.System.Text.Json/InternalConverters/ByteStringConverter.cs new file mode 100644 index 0000000..7afeab3 --- /dev/null +++ b/src/Protobuf.System.Text.Json/InternalConverters/ByteStringConverter.cs @@ -0,0 +1,25 @@ +using System.Text.Json; +using Google.Protobuf; +using Google.Protobuf.Reflection; +using Protobuf.System.Text.Json.InternalConverters; + +internal class ByteStringConverter : InternalConverter +{ + public override void Write(Utf8JsonWriter writer, object value, JsonSerializerOptions options) + { + var base64String = ((ByteString)value).ToBase64(); + writer.WriteStringValue(base64String); + } + + public override void Read(ref Utf8JsonReader reader, IMessage obj, Type typeToConvert, JsonSerializerOptions options, + IFieldAccessor fieldAccessor) + { + var base64String = reader.GetString(); + if (base64String is null) + { + return; + } + var value = ByteString.FromBase64(base64String); + fieldAccessor.SetValue(obj, value); + } +} \ 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 ce9241d..ffb49a7 100644 --- a/src/Protobuf.System.Text.Json/InternalConverters/InternalConverterFactory.cs +++ b/src/Protobuf.System.Text.Json/InternalConverters/InternalConverterFactory.cs @@ -1,4 +1,5 @@ using System.Text.Json; +using Google.Protobuf; namespace Protobuf.System.Text.Json.InternalConverters; @@ -23,6 +24,11 @@ public static InternalConverter Create(FieldInfo fieldInfo, JsonSerializerOption var internalConverter = (InternalConverter) Activator.CreateInstance(typeof(ProtoEnumConverter), args: new object[] { fieldInfo.EnumType, jsonSerializerOptions.Encoder! })!; return internalConverter; } + else if (fieldInfo.FieldType == typeof(ByteString)) + { + var internalConverter = new ByteStringConverter(); + return internalConverter; + } else { var converterType = typeof(FieldConverter<>).MakeGenericType(fieldInfo.FieldType); From 9fa3ad6b018013da211f80ceee05d58c46cebc53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristoffer=20Lerb=C3=A6k=20Pedersen?= Date: Fri, 25 Aug 2023 12:03:23 +0200 Subject: [PATCH 5/6] Uncomment tests and update approved results --- ...noreCondition_set_to_WhenWritingNull.approved.json | 3 ++- ..._DefaultIgnoreCondition_set_to_Never.approved.json | 3 ++- ...hen_UseProtobufJsonNames_set_to_true.approved.json | 3 ++- ...rialize_message_with_primitive_types.approved.json | 3 ++- .../Protos/simple_message.proto | 3 +-- ...rialize_message_with_primitive_types.approved.json | 3 ++- .../SimpleMessageTests.cs | 11 +++++------ 7 files changed, 16 insertions(+), 13 deletions(-) diff --git a/test/Protobuf.System.Text.Json.Tests/DefaultIgnoreConditionTests.Should_not_ignore_default_non_nullable_properties_when_DefaultIgnoreCondition_set_to_WhenWritingNull.approved.json b/test/Protobuf.System.Text.Json.Tests/DefaultIgnoreConditionTests.Should_not_ignore_default_non_nullable_properties_when_DefaultIgnoreCondition_set_to_WhenWritingNull.approved.json index c263be5..87c6505 100644 --- a/test/Protobuf.System.Text.Json.Tests/DefaultIgnoreConditionTests.Should_not_ignore_default_non_nullable_properties_when_DefaultIgnoreCondition_set_to_WhenWritingNull.approved.json +++ b/test/Protobuf.System.Text.Json.Tests/DefaultIgnoreConditionTests.Should_not_ignore_default_non_nullable_properties_when_DefaultIgnoreCondition_set_to_WhenWritingNull.approved.json @@ -12,5 +12,6 @@ "sfixed32Property": 0, "sfixed64Property": 0, "boolProperty": false, - "stringProperty": "" + "stringProperty": "", + "bytesProperty": "" } \ No newline at end of file diff --git a/test/Protobuf.System.Text.Json.Tests/DefaultIgnoreConditionTests.Should_not_ignore_default_properties_when_DefaultIgnoreCondition_set_to_Never.approved.json b/test/Protobuf.System.Text.Json.Tests/DefaultIgnoreConditionTests.Should_not_ignore_default_properties_when_DefaultIgnoreCondition_set_to_Never.approved.json index c263be5..87c6505 100644 --- a/test/Protobuf.System.Text.Json.Tests/DefaultIgnoreConditionTests.Should_not_ignore_default_properties_when_DefaultIgnoreCondition_set_to_Never.approved.json +++ b/test/Protobuf.System.Text.Json.Tests/DefaultIgnoreConditionTests.Should_not_ignore_default_properties_when_DefaultIgnoreCondition_set_to_Never.approved.json @@ -12,5 +12,6 @@ "sfixed32Property": 0, "sfixed64Property": 0, "boolProperty": false, - "stringProperty": "" + "stringProperty": "", + "bytesProperty": "" } \ No newline at end of file diff --git a/test/Protobuf.System.Text.Json.Tests/JsonNamingPolicyTests.Should_ignore_PropertyNamingPolicy_when_UseProtobufJsonNames_set_to_true.approved.json b/test/Protobuf.System.Text.Json.Tests/JsonNamingPolicyTests.Should_ignore_PropertyNamingPolicy_when_UseProtobufJsonNames_set_to_true.approved.json index 9a88710..1283b7d 100644 --- a/test/Protobuf.System.Text.Json.Tests/JsonNamingPolicyTests.Should_ignore_PropertyNamingPolicy_when_UseProtobufJsonNames_set_to_true.approved.json +++ b/test/Protobuf.System.Text.Json.Tests/JsonNamingPolicyTests.Should_ignore_PropertyNamingPolicy_when_UseProtobufJsonNames_set_to_true.approved.json @@ -12,5 +12,6 @@ "sfixed32Property": 0, "sfixed64Property": 0, "boolProperty": false, - "stringProperty": "" + "stringProperty": "", + "bytesProperty": "" } \ No newline at end of file diff --git a/test/Protobuf.System.Text.Json.Tests/JsonNamingPolicyTests.Should_serialize_message_with_primitive_types.approved.json b/test/Protobuf.System.Text.Json.Tests/JsonNamingPolicyTests.Should_serialize_message_with_primitive_types.approved.json index 488c716..30351d3 100644 --- a/test/Protobuf.System.Text.Json.Tests/JsonNamingPolicyTests.Should_serialize_message_with_primitive_types.approved.json +++ b/test/Protobuf.System.Text.Json.Tests/JsonNamingPolicyTests.Should_serialize_message_with_primitive_types.approved.json @@ -12,5 +12,6 @@ "sfixed32property": 0, "sfixed64property": 0, "boolproperty": false, - "stringproperty": "" + "stringproperty": "", + "bytesproperty": "" } \ No newline at end of file diff --git a/test/Protobuf.System.Text.Json.Tests/Protos/simple_message.proto b/test/Protobuf.System.Text.Json.Tests/Protos/simple_message.proto index 1719c6c..2805a7e 100644 --- a/test/Protobuf.System.Text.Json.Tests/Protos/simple_message.proto +++ b/test/Protobuf.System.Text.Json.Tests/Protos/simple_message.proto @@ -31,6 +31,5 @@ message SimpleMessage { string string_property = 14; - // TODO: Support bytes property - // bytes bytes_property = 15; + bytes bytes_property = 15; } \ No newline at end of file diff --git a/test/Protobuf.System.Text.Json.Tests/SimpleMessageTests.Should_serialize_message_with_primitive_types.approved.json b/test/Protobuf.System.Text.Json.Tests/SimpleMessageTests.Should_serialize_message_with_primitive_types.approved.json index b0c1fb0..bab7abd 100644 --- a/test/Protobuf.System.Text.Json.Tests/SimpleMessageTests.Should_serialize_message_with_primitive_types.approved.json +++ b/test/Protobuf.System.Text.Json.Tests/SimpleMessageTests.Should_serialize_message_with_primitive_types.approved.json @@ -12,5 +12,6 @@ "sfixed32Property": 9, "sfixed64Property": 10, "boolProperty": true, - "stringProperty": "hello" + "stringProperty": "hello", + "bytesProperty": "YWJj" } \ No newline at end of file diff --git a/test/Protobuf.System.Text.Json.Tests/SimpleMessageTests.cs b/test/Protobuf.System.Text.Json.Tests/SimpleMessageTests.cs index 2f69e24..6899cf9 100644 --- a/test/Protobuf.System.Text.Json.Tests/SimpleMessageTests.cs +++ b/test/Protobuf.System.Text.Json.Tests/SimpleMessageTests.cs @@ -1,5 +1,6 @@ using System.Text.Json; using System.Text.Json.Protobuf.Tests; +using Google.Protobuf; using Protobuf.System.Text.Json.Tests.Utils; using Shouldly; using SmartAnalyzers.ApprovalTestsExtensions; @@ -28,9 +29,8 @@ public void Should_serialize_message_with_primitive_types() Sfixed32Property = 9, Sfixed64Property = 10, BoolProperty = true, - StringProperty = "hello" - // TODO: Support bytes property - // BytesProperty = ByteString.CopyFromUtf8("abc") + StringProperty = "hello", + BytesProperty = ByteString.CopyFromUtf8("abc") }; var jsonSerializerOptions = TestHelper.CreateJsonSerializerOptions(); @@ -61,9 +61,8 @@ public void Should_deserialize_message_with_primitive_types() Sfixed32Property = 9, Sfixed64Property = 10, BoolProperty = true, - StringProperty = "hello" - // TODO: Support bytes property - // BytesProperty = ByteString.CopyFromUtf8("abc") + StringProperty = "hello", + BytesProperty = ByteString.CopyFromUtf8("abc") }; var jsonSerializerOptions = TestHelper.CreateJsonSerializerOptions(); From 50d8c07ae260c822c25c55b8dc7c1497bded73b1 Mon Sep 17 00:00:00 2001 From: Havret Date: Fri, 25 Aug 2023 22:09:18 +0200 Subject: [PATCH 6/6] Generalize ByteString conversion --- .../InternalConverters/ByteStringConverter.cs | 25 ------------------ .../InternalConverterFactory.cs | 6 ----- .../JsonSerializerOptionsExtensions.cs | 1 + .../ByteStringConverter.cs | 26 +++++++++++++++++++ ...alize_message_with_map_field.approved.json | 3 +++ .../MessageWithMapsTests.cs | 7 +++-- .../Protos/message_with_maps.proto | 1 + 7 files changed, 36 insertions(+), 33 deletions(-) delete mode 100644 src/Protobuf.System.Text.Json/InternalConverters/ByteStringConverter.cs create mode 100644 src/Protobuf.System.Text.Json/WellKnownTypesConverters/ByteStringConverter.cs diff --git a/src/Protobuf.System.Text.Json/InternalConverters/ByteStringConverter.cs b/src/Protobuf.System.Text.Json/InternalConverters/ByteStringConverter.cs deleted file mode 100644 index 7afeab3..0000000 --- a/src/Protobuf.System.Text.Json/InternalConverters/ByteStringConverter.cs +++ /dev/null @@ -1,25 +0,0 @@ -using System.Text.Json; -using Google.Protobuf; -using Google.Protobuf.Reflection; -using Protobuf.System.Text.Json.InternalConverters; - -internal class ByteStringConverter : InternalConverter -{ - public override void Write(Utf8JsonWriter writer, object value, JsonSerializerOptions options) - { - var base64String = ((ByteString)value).ToBase64(); - writer.WriteStringValue(base64String); - } - - public override void Read(ref Utf8JsonReader reader, IMessage obj, Type typeToConvert, JsonSerializerOptions options, - IFieldAccessor fieldAccessor) - { - var base64String = reader.GetString(); - if (base64String is null) - { - return; - } - var value = ByteString.FromBase64(base64String); - fieldAccessor.SetValue(obj, value); - } -} \ 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 ffb49a7..ce9241d 100644 --- a/src/Protobuf.System.Text.Json/InternalConverters/InternalConverterFactory.cs +++ b/src/Protobuf.System.Text.Json/InternalConverters/InternalConverterFactory.cs @@ -1,5 +1,4 @@ using System.Text.Json; -using Google.Protobuf; namespace Protobuf.System.Text.Json.InternalConverters; @@ -24,11 +23,6 @@ public static InternalConverter Create(FieldInfo fieldInfo, JsonSerializerOption var internalConverter = (InternalConverter) Activator.CreateInstance(typeof(ProtoEnumConverter), args: new object[] { fieldInfo.EnumType, jsonSerializerOptions.Encoder! })!; return internalConverter; } - else if (fieldInfo.FieldType == typeof(ByteString)) - { - var internalConverter = new ByteStringConverter(); - return internalConverter; - } else { var converterType = typeof(FieldConverter<>).MakeGenericType(fieldInfo.FieldType); diff --git a/src/Protobuf.System.Text.Json/JsonSerializerOptionsExtensions.cs b/src/Protobuf.System.Text.Json/JsonSerializerOptionsExtensions.cs index c36b0c4..fec419c 100644 --- a/src/Protobuf.System.Text.Json/JsonSerializerOptionsExtensions.cs +++ b/src/Protobuf.System.Text.Json/JsonSerializerOptionsExtensions.cs @@ -23,6 +23,7 @@ public static void AddProtobufSupport(this JsonSerializerOptions options, Action { options.Converters.Add(new TimestampConverter()); } + options.Converters.Add(new ByteStringConverter()); options.Converters.Add(new ProtobufJsonConverterFactory(jsonProtobufSerializerOptions)); } diff --git a/src/Protobuf.System.Text.Json/WellKnownTypesConverters/ByteStringConverter.cs b/src/Protobuf.System.Text.Json/WellKnownTypesConverters/ByteStringConverter.cs new file mode 100644 index 0000000..c3479a9 --- /dev/null +++ b/src/Protobuf.System.Text.Json/WellKnownTypesConverters/ByteStringConverter.cs @@ -0,0 +1,26 @@ +using System.Text.Json; +using System.Text.Json.Serialization; +using Google.Protobuf; + +namespace Protobuf.System.Text.Json.WellKnownTypesConverters; + +internal class ByteStringConverter : JsonConverter +{ + public override ByteString? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + if (reader.GetString() is { } base64String) + { + return ByteString.FromBase64(base64String); + } + + return null; + } + + public override void Write(Utf8JsonWriter writer, ByteString? value, JsonSerializerOptions options) + { + if (value != null) + { + writer.WriteStringValue(value.ToBase64()); + } + } +} \ No newline at end of file diff --git a/test/Protobuf.System.Text.Json.Tests/MessageWithMapsTests.Should_serialize_message_with_map_field.approved.json b/test/Protobuf.System.Text.Json.Tests/MessageWithMapsTests.Should_serialize_message_with_map_field.approved.json index 34e6706..449aff5 100644 --- a/test/Protobuf.System.Text.Json.Tests/MessageWithMapsTests.Should_serialize_message_with_map_field.approved.json +++ b/test/Protobuf.System.Text.Json.Tests/MessageWithMapsTests.Should_serialize_message_with_map_field.approved.json @@ -8,5 +8,8 @@ "a": 1 } } + }, + "mapStringToBytesType": { + "string_key": "YWJj" } } \ No newline at end of file diff --git a/test/Protobuf.System.Text.Json.Tests/MessageWithMapsTests.cs b/test/Protobuf.System.Text.Json.Tests/MessageWithMapsTests.cs index 04d6ab5..3fbb86b 100644 --- a/test/Protobuf.System.Text.Json.Tests/MessageWithMapsTests.cs +++ b/test/Protobuf.System.Text.Json.Tests/MessageWithMapsTests.cs @@ -1,5 +1,6 @@ using System.Text.Json; using System.Text.Json.Protobuf.Tests; +using Google.Protobuf; using Protobuf.System.Text.Json.Tests.Utils; using Shouldly; using SmartAnalyzers.ApprovalTestsExtensions; @@ -22,7 +23,8 @@ public void Should_serialize_message_with_map_field() { MapStringToInt = {["a"] = 1} } - } + }, + MapStringToBytesType = { ["string_key"] = ByteString.CopyFromUtf8("abc") } }; var jsonSerializerOptions = TestHelper.CreateJsonSerializerOptions(); @@ -47,7 +49,8 @@ public void Should_deserialize_message_with_map_field() { MapStringToInt = {["a"] = 1} } - } + }, + MapStringToBytesType = { ["string_key"] = ByteString.CopyFromUtf8("abc") } }; var jsonSerializerOptions = TestHelper.CreateJsonSerializerOptions(); diff --git a/test/Protobuf.System.Text.Json.Tests/Protos/message_with_maps.proto b/test/Protobuf.System.Text.Json.Tests/Protos/message_with_maps.proto index 1d42802..a71527d 100644 --- a/test/Protobuf.System.Text.Json.Tests/Protos/message_with_maps.proto +++ b/test/Protobuf.System.Text.Json.Tests/Protos/message_with_maps.proto @@ -5,6 +5,7 @@ option csharp_namespace = "System.Text.Json.Protobuf.Tests"; message MessageWithMaps { map map_int_to_string = 1; map map_string_to_complex_type = 2; + map map_string_to_bytes_type = 3; } message NestedMessageAsKey {