From 554b1ee746ca74dfa3afce3f39761a43c6dc248c Mon Sep 17 00:00:00 2001 From: Florian Bernd Date: Mon, 24 Jul 2023 12:40:08 +0200 Subject: [PATCH] Fix custom floating-point JSON converters (#7854) --- Directory.Build.props | 2 +- .../Serialization/DefaultSourceSerializer.cs | 2 +- .../DoubleWithFractionalPortionConverter.cs | 114 ++++++++++------ .../FloatWithFractionalPortionConverter.cs | 90 ------------- .../Serialization/JsonConstants.cs | 33 ++++- .../SingleWithFractionalPortionConverter.cs | 122 ++++++++++++++++++ ...eSerializationForNumericPropertiesTests.cs | 4 +- ...tlySerializes_TypesUsingQuery.verified.txt | 2 +- 8 files changed, 230 insertions(+), 139 deletions(-) delete mode 100644 src/Elastic.Clients.Elasticsearch/Serialization/FloatWithFractionalPortionConverter.cs create mode 100644 src/Elastic.Clients.Elasticsearch/Serialization/SingleWithFractionalPortionConverter.cs diff --git a/Directory.Build.props b/Directory.Build.props index e674fd499aa..2eb5c4c005d 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -28,7 +28,7 @@ - latest + preview true false diff --git a/src/Elastic.Clients.Elasticsearch/Serialization/DefaultSourceSerializer.cs b/src/Elastic.Clients.Elasticsearch/Serialization/DefaultSourceSerializer.cs index ceb91c8d9be..9243018cb84 100644 --- a/src/Elastic.Clients.Elasticsearch/Serialization/DefaultSourceSerializer.cs +++ b/src/Elastic.Clients.Elasticsearch/Serialization/DefaultSourceSerializer.cs @@ -22,7 +22,7 @@ public class DefaultSourceSerializer : SystemTextJsonSerializer { new JsonStringEnumConverter(), new DoubleWithFractionalPortionConverter(), - new FloatWithFractionalPortionConverter() + new SingleWithFractionalPortionConverter() }; private readonly JsonSerializerOptions _jsonSerializerOptions; diff --git a/src/Elastic.Clients.Elasticsearch/Serialization/DoubleWithFractionalPortionConverter.cs b/src/Elastic.Clients.Elasticsearch/Serialization/DoubleWithFractionalPortionConverter.cs index e5e48375c6d..911f69f399e 100644 --- a/src/Elastic.Clients.Elasticsearch/Serialization/DoubleWithFractionalPortionConverter.cs +++ b/src/Elastic.Clients.Elasticsearch/Serialization/DoubleWithFractionalPortionConverter.cs @@ -2,33 +2,53 @@ // Elasticsearch B.V licenses this file to you under the Apache 2.0 License. // See the LICENSE file in the project root for more information. -#pragma warning disable IDE0005 using System; + +#if NETCOREAPP + using System.Buffers.Text; + +#endif + using System.Globalization; + +#if !NETCOREAPP + using System.Text; + +#endif + using System.Text.Json; using System.Text.Json.Serialization; -using static Elastic.Clients.Elasticsearch.Serialization.JsonConstants; -#pragma warning restore IDE0005 namespace Elastic.Clients.Elasticsearch.Serialization; internal sealed class DoubleWithFractionalPortionConverter : JsonConverter { - // We don't handle floating point literals (NaN, etc.) because for source serialization because Elasticsearch only support finite values for numeric fields. - // We must handle the possibility of numbers as strings in the source however. - public override double Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) { - if (reader.TokenType == JsonTokenType.String && options.NumberHandling.HasFlag(JsonNumberHandling.AllowReadingFromString)) + if (reader.TokenType != JsonTokenType.String) + return reader.GetDouble(); + + if (options.NumberHandling.HasFlag(JsonNumberHandling.AllowNamedFloatingPointLiterals)) + { + // TODO: Handle 'reader.HasValueSequence' + if (reader.ValueSpan.SequenceEqual(JsonConstants.LiteralNaN)) + return float.NaN; + if (reader.ValueSpan.SequenceEqual(JsonConstants.LiteralPositiveInfinity)) + return float.PositiveInfinity; + if (reader.ValueSpan.SequenceEqual(JsonConstants.LiteralNegativeInfinity)) + return float.NegativeInfinity; + } + + if (options.NumberHandling.HasFlag(JsonNumberHandling.AllowReadingFromString)) { var value = reader.GetString(); - if (!double.TryParse(value, NumberStyles.Any, CultureInfo.InvariantCulture, out var parsedValue)) + if (!double.TryParse(value, NumberStyles.Any, CultureInfo.InvariantCulture, out var result)) ThrowHelper.ThrowJsonException($"Unable to parse '{value}' as a double."); - return parsedValue; + return result; } return reader.GetDouble(); @@ -36,55 +56,67 @@ public override double Read(ref Utf8JsonReader reader, Type typeToConvert, JsonS public override void Write(Utf8JsonWriter writer, double value, JsonSerializerOptions options) { - Span utf8bytes = stackalloc byte[128]; // This is the size used in STJ for future proofing. https://github.com/dotnet/runtime/blob/dae6c2472b699b7cff2efeb5ce06b75c9551bc40/src/libraries/System.Text.Json/src/System/Text/Json/JsonConstants.cs#L79 + if (options.NumberHandling.HasFlag(JsonNumberHandling.AllowNamedFloatingPointLiterals)) + { + switch (value) + { + case double.NaN: + writer.WriteStringValue(JsonConstants.EncodedNaN); + return; - // NOTE: This code is based on https://github.com/dotnet/runtime/blob/dae6c2472b699b7cff2efeb5ce06b75c9551bc40/src/libraries/System.Text.Json/src/System/Text/Json/JsonConstants.cs#L79 + case double.PositiveInfinity: + writer.WriteStringValue(JsonConstants.EncodedPositiveInfinity); + return; + + case double.NegativeInfinity: + writer.WriteStringValue(JsonConstants.EncodedNegativeInfinity); + return; + } + } - // Frameworks that are not .NET Core 3.0 or higher do not produce round-trippable strings by - // default. Further, the Utf8Formatter on older frameworks does not support taking a precision - // specifier for 'G' nor does it represent other formats such as 'R'. As such, we duplicate - // the .NET Core 3.0 logic of forwarding to the UTF16 formatter and transcoding it back to UTF8, - // with some additional changes to remove dependencies on Span APIs which don't exist downlevel. + if (options.NumberHandling.HasFlag(JsonNumberHandling.WriteAsString)) + { + // TODO: Implement as needed + throw new NotImplementedException("The 'JsonNumberHandling.WriteAsString' is currently not supported."); + } - // PERFORMANCE: This code could be benchmarked and tweaked to make it faster. + // This code is based on: + // https://github.com/dotnet/runtime/blob/main/src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteValues.Double.cs#L101 #if NETCOREAPP - if (Utf8Formatter.TryFormat(value, utf8bytes, out var bytesWritten)) + Span utf8Text = stackalloc byte[JsonConstants.MaximumFormatDoubleLength]; + + if (Utf8Formatter.TryFormat(value, utf8Text, out var bytesWritten, JsonConstants.DoubleStandardFormat)) { - if (utf8bytes.IndexOfAny(NonIntegerBytes) == -1) + if (utf8Text.IndexOfAny(JsonConstants.NonIntegerChars) == -1) { - utf8bytes[bytesWritten++] = (byte)'.'; - utf8bytes[bytesWritten++] = (byte)'0'; + utf8Text[bytesWritten++] = (byte)'.'; + utf8Text[bytesWritten++] = (byte)'0'; } -#pragma warning disable IDE0057 // Use range operator - writer.WriteRawValue(utf8bytes.Slice(0, bytesWritten), skipInputValidation: true); -#pragma warning restore IDE0057 // Use range operator - + writer.WriteRawValue(utf8Text[..bytesWritten], true); return; } #else - var utf16Text = value.ToString("G17", CultureInfo.InvariantCulture); + var utf16Text = value.ToString(JsonConstants.DoubleFormatString, CultureInfo.InvariantCulture); + if (utf16Text.IndexOfAny(JsonConstants.NonIntegerChars) == -1) + { + utf16Text += ".0"; + } - if (utf16Text.Length < utf8bytes.Length) + try { - try - { - var bytes = Encoding.UTF8.GetBytes(utf16Text); + var utf8Text = Encoding.UTF8.GetBytes(utf16Text); - if (bytes.Length < utf8bytes.Length) - { - bytes.CopyTo(utf8bytes); - return; - } - } - catch - { - // Swallow this and fall through to our general exception. - } + writer.WriteRawValue(utf8Text, true); + return; + } + catch + { + // Swallow this and fall through to our general exception. } #endif - ThrowHelper.ThrowJsonException($"Unable to serialize double value."); + ThrowHelper.ThrowJsonException("Unable to serialize double value."); } } diff --git a/src/Elastic.Clients.Elasticsearch/Serialization/FloatWithFractionalPortionConverter.cs b/src/Elastic.Clients.Elasticsearch/Serialization/FloatWithFractionalPortionConverter.cs deleted file mode 100644 index 23ba2b5eaa4..00000000000 --- a/src/Elastic.Clients.Elasticsearch/Serialization/FloatWithFractionalPortionConverter.cs +++ /dev/null @@ -1,90 +0,0 @@ -// Licensed to Elasticsearch B.V under one or more agreements. -// Elasticsearch B.V licenses this file to you under the Apache 2.0 License. -// See the LICENSE file in the project root for more information. - -#pragma warning disable IDE0005 -using System; -using System.Buffers.Text; -using System.Globalization; -using System.Text; -using System.Text.Json; -using System.Text.Json.Serialization; -using static Elastic.Clients.Elasticsearch.Serialization.JsonConstants; -#pragma warning restore IDE0005 - -namespace Elastic.Clients.Elasticsearch.Serialization; - -internal sealed class FloatWithFractionalPortionConverter : JsonConverter -{ - // We don't handle floating point literals (NaN, etc.) because for source serialization because Elasticsearch only supports finite values for numeric fields. - // We must handle the possibility of numbers as strings in the source however. - - public override float Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) - { - if (reader.TokenType == JsonTokenType.String && options.NumberHandling.HasFlag(JsonNumberHandling.AllowReadingFromString)) - { - var value = reader.GetString(); - - if (!float.TryParse(value, NumberStyles.Any, CultureInfo.InvariantCulture, out var parsedValue)) - ThrowHelper.ThrowJsonException($"Unable to parse '{value}' as a float."); - - return parsedValue; - } - - return reader.GetSingle(); - } - - public override void Write(Utf8JsonWriter writer, float value, JsonSerializerOptions options) - { - Span utf8bytes = stackalloc byte[128]; // This is the size used in STJ for future proofing. https://github.com/dotnet/runtime/blob/dae6c2472b699b7cff2efeb5ce06b75c9551bc40/src/libraries/System.Text.Json/src/System/Text/Json/JsonConstants.cs#L79 - - // NOTE: This code is based on https://github.com/dotnet/runtime/blob/dae6c2472b699b7cff2efeb5ce06b75c9551bc40/src/libraries/System.Text.Json/src/System/Text/Json/JsonConstants.cs#L79 - - // Frameworks that are not .NET Core 3.0 or higher do not produce round-trippable strings by - // default. Further, the Utf8Formatter on older frameworks does not support taking a precision - // specifier for 'G' nor does it represent other formats such as 'R'. As such, we duplicate - // the .NET Core 3.0 logic of forwarding to the UTF16 formatter and transcoding it back to UTF8, - // with some additional changes to remove dependencies on Span APIs which don't exist downlevel. - - // PERFORMANCE: This code could be benchmarked and tweaked to make it faster. - -#if NETCOREAPP - if (Utf8Formatter.TryFormat(value, utf8bytes, out var bytesWritten)) - { - if (utf8bytes.IndexOfAny(NonIntegerBytes) == -1) - { - utf8bytes[bytesWritten++] = (byte)'.'; - utf8bytes[bytesWritten++] = (byte)'0'; - } - -#pragma warning disable IDE0057 // Use range operator - writer.WriteRawValue(utf8bytes.Slice(0, bytesWritten), skipInputValidation: true); -#pragma warning restore IDE0057 // Use range operator - - return; - } -#else - var utf16Text = value.ToString("G9", CultureInfo.InvariantCulture); - - if (utf16Text.Length < utf8bytes.Length) - { - try - { - var bytes = Encoding.UTF8.GetBytes(utf16Text); - - if (bytes.Length < utf8bytes.Length) - { - bytes.CopyTo(utf8bytes); - return; - } - } - catch - { - // Swallow this and fall through to our general exception. - } - } -#endif - - ThrowHelper.ThrowJsonException($"Unable to serialize float value."); - } -} diff --git a/src/Elastic.Clients.Elasticsearch/Serialization/JsonConstants.cs b/src/Elastic.Clients.Elasticsearch/Serialization/JsonConstants.cs index 064de1ac507..3c28f552a46 100644 --- a/src/Elastic.Clients.Elasticsearch/Serialization/JsonConstants.cs +++ b/src/Elastic.Clients.Elasticsearch/Serialization/JsonConstants.cs @@ -3,12 +3,39 @@ // See the LICENSE file in the project root for more information. using System; +using System.Text.Json; + +#if NETCOREAPP + +using System.Buffers; + +#endif namespace Elastic.Clients.Elasticsearch.Serialization; internal static class JsonConstants { -#pragma warning disable IDE0230 // Use UTF-8 string literal - public static ReadOnlySpan NonIntegerBytes => new[] { (byte)'E', (byte)'.' }; // In the future, when we move to the .NET 7 SDK, it would be nice to use u8 literals e.g. "E."u8 -#pragma warning restore IDE0230 // Use UTF-8 string literal + public static ReadOnlySpan LiteralNaN => "NaN"u8; + public static ReadOnlySpan LiteralPositiveInfinity => "Infinity"u8; + public static ReadOnlySpan LiteralNegativeInfinity => "-Infinity"u8; + public static JsonEncodedText EncodedNaN => JsonEncodedText.Encode(LiteralNaN); + public static JsonEncodedText EncodedPositiveInfinity => JsonEncodedText.Encode(LiteralPositiveInfinity); + public static JsonEncodedText EncodedNegativeInfinity => JsonEncodedText.Encode(LiteralNegativeInfinity); + +#if NETCOREAPP + public static ReadOnlySpan NonIntegerChars => "E."u8; +#else + public static char[] NonIntegerChars => new[] { 'E', '.' }; +#endif + + public const string DoubleFormatString = "G17"; // 'R' does not roundtrip correctly in some cases prior to .NET Core 3 + public const string SingleFormatString = "G9"; // https://learn.microsoft.com/en-us/dotnet/standard/base-types/standard-numeric-format-strings#round-trip-format-specifier-r + +#if NETCOREAPP + public static readonly StandardFormat DoubleStandardFormat = StandardFormat.Parse(DoubleFormatString); + public static readonly StandardFormat SingleStandardFormat = StandardFormat.Parse(SingleFormatString); + + public const int MaximumFormatDoubleLength = 128; // https://github.com/dotnet/runtime/blob/main/src/libraries/System.Text.Json/src/System/Text/Json/JsonConstants.cs#L78 + public const int MaximumFormatSingleLength = 128; // https://github.com/dotnet/runtime/blob/main/src/libraries/System.Text.Json/src/System/Text/Json/JsonConstants.cs#L79 +#endif } diff --git a/src/Elastic.Clients.Elasticsearch/Serialization/SingleWithFractionalPortionConverter.cs b/src/Elastic.Clients.Elasticsearch/Serialization/SingleWithFractionalPortionConverter.cs new file mode 100644 index 00000000000..279516fcfbf --- /dev/null +++ b/src/Elastic.Clients.Elasticsearch/Serialization/SingleWithFractionalPortionConverter.cs @@ -0,0 +1,122 @@ +// Licensed to Elasticsearch B.V under one or more agreements. +// Elasticsearch B.V licenses this file to you under the Apache 2.0 License. +// See the LICENSE file in the project root for more information. + +using System; + +#if NETCOREAPP + +using System.Buffers.Text; + +#endif + +using System.Globalization; + +#if !NETCOREAPP + +using System.Text; + +#endif + +using System.Text.Json; +using System.Text.Json.Serialization; + +namespace Elastic.Clients.Elasticsearch.Serialization; + +internal sealed class SingleWithFractionalPortionConverter : JsonConverter +{ + public override float Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + if (reader.TokenType != JsonTokenType.String) + return reader.GetSingle(); + + if (options.NumberHandling.HasFlag(JsonNumberHandling.AllowNamedFloatingPointLiterals)) + { + // TODO: Handle 'reader.HasValueSequence' + if (reader.ValueSpan.SequenceEqual(JsonConstants.LiteralNaN)) + return float.NaN; + if (reader.ValueSpan.SequenceEqual(JsonConstants.LiteralPositiveInfinity)) + return float.PositiveInfinity; + if (reader.ValueSpan.SequenceEqual(JsonConstants.LiteralNegativeInfinity)) + return float.NegativeInfinity; + } + + if (options.NumberHandling.HasFlag(JsonNumberHandling.AllowReadingFromString)) + { + var value = reader.GetString(); + + if (!float.TryParse(value, NumberStyles.Any, CultureInfo.InvariantCulture, out var result)) + ThrowHelper.ThrowJsonException($"Unable to parse '{value}' as a single."); + + return result; + } + + return reader.GetSingle(); + } + + public override void Write(Utf8JsonWriter writer, float value, JsonSerializerOptions options) + { + if (options.NumberHandling.HasFlag(JsonNumberHandling.AllowNamedFloatingPointLiterals)) + { + switch (value) + { + case float.NaN: + writer.WriteStringValue(JsonConstants.EncodedNaN); + return; + + case float.PositiveInfinity: + writer.WriteStringValue(JsonConstants.EncodedPositiveInfinity); + return; + + case float.NegativeInfinity: + writer.WriteStringValue(JsonConstants.EncodedNegativeInfinity); + return; + } + } + + if (options.NumberHandling.HasFlag(JsonNumberHandling.WriteAsString)) + { + // TODO: Implement as needed + throw new NotImplementedException("The 'JsonNumberHandling.WriteAsString' is currently not supported."); + } + + // This code is based on: + // https://github.com/dotnet/runtime/blob/main/src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteValues.Double.cs#L101 + +#if NETCOREAPP + Span utf8Text = stackalloc byte[JsonConstants.MaximumFormatSingleLength]; + + if (Utf8Formatter.TryFormat(value, utf8Text, out var bytesWritten, JsonConstants.SingleStandardFormat)) + { + if (utf8Text.IndexOfAny(JsonConstants.NonIntegerChars) == -1) + { + utf8Text[bytesWritten++] = (byte)'.'; + utf8Text[bytesWritten++] = (byte)'0'; + } + + writer.WriteRawValue(utf8Text[..bytesWritten], true); + return; + } +#else + var utf16Text = value.ToString(JsonConstants.SingleFormatString, CultureInfo.InvariantCulture); + if (utf16Text.IndexOfAny(JsonConstants.NonIntegerChars) == -1) + { + utf16Text += ".0"; + } + + try + { + var utf8Text = Encoding.UTF8.GetBytes(utf16Text); + + writer.WriteRawValue(utf8Text, true); + return; + } + catch + { + // Swallow this and fall through to our general exception. + } +#endif + + ThrowHelper.ThrowJsonException("Unable to serialize single value."); + } +} diff --git a/tests/Tests/Serialization/SourceSerializationForNumericPropertiesTests.cs b/tests/Tests/Serialization/SourceSerializationForNumericPropertiesTests.cs index 95a36c08e21..378b4fbb9ae 100644 --- a/tests/Tests/Serialization/SourceSerializationForNumericPropertiesTests.cs +++ b/tests/Tests/Serialization/SourceSerializationForNumericPropertiesTests.cs @@ -27,7 +27,7 @@ public void FloatMinValue_SerializesCorrectly() var json = SourceSerializeAndGetJsonString(numericTests); - json.Should().Be("{\"float\":-3.4028235E+38}"); + json.Should().Be("{\"float\":-3.40282347E+38}"); var deserialized = SourceDeserializeJsonString(json); @@ -41,7 +41,7 @@ public void FloatMaxValue_SerializesCorrectly() var json = SourceSerializeAndGetJsonString(numericTests); - json.Should().Be("{\"float\":3.4028235E+38}"); + json.Should().Be("{\"float\":3.40282347E+38}"); var deserialized = SourceDeserializeJsonString(json); diff --git a/tests/Tests/_VerifySnapshots/DefaultSourceSerializerTests.SourceSerialization_WithBuiltInDefaultSourceSerializer_CorrectlySerializes_TypesUsingQuery.verified.txt b/tests/Tests/_VerifySnapshots/DefaultSourceSerializerTests.SourceSerialization_WithBuiltInDefaultSourceSerializer_CorrectlySerializes_TypesUsingQuery.verified.txt index 35f2180ee22..4c4f012dde6 100644 --- a/tests/Tests/_VerifySnapshots/DefaultSourceSerializerTests.SourceSerialization_WithBuiltInDefaultSourceSerializer_CorrectlySerializes_TypesUsingQuery.verified.txt +++ b/tests/Tests/_VerifySnapshots/DefaultSourceSerializerTests.SourceSerialization_WithBuiltInDefaultSourceSerializer_CorrectlySerializes_TypesUsingQuery.verified.txt @@ -1 +1 @@ -{"query":{"match_all":{"boost":2.1}}} \ No newline at end of file +{"query":{"match_all":{"boost":2.0999999}}} \ No newline at end of file