Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix custom floating-point JSON converters #7854

Merged
merged 4 commits into from
Jul 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
</PropertyGroup>

<PropertyGroup>
<LangVersion>latest</LangVersion>
<LangVersion>preview</LangVersion>
<!-- Default Version numbers -->
<TreatWarningsAsErrors>true</TreatWarningsAsErrors>
<IsPackable>false</IsPackable>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public class DefaultSourceSerializer : SystemTextJsonSerializer
{
new JsonStringEnumConverter(),
new DoubleWithFractionalPortionConverter(),
new FloatWithFractionalPortionConverter()
new SingleWithFractionalPortionConverter()
};

private readonly JsonSerializerOptions _jsonSerializerOptions;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,89 +2,121 @@
// 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<double>
{
// 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();
}

public override void Write(Utf8JsonWriter writer, double value, JsonSerializerOptions options)
{
Span<byte> 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<byte> 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.");
}
}

This file was deleted.

33 changes: 30 additions & 3 deletions src/Elastic.Clients.Elasticsearch/Serialization/JsonConstants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<byte> 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<byte> LiteralNaN => "NaN"u8;
public static ReadOnlySpan<byte> LiteralPositiveInfinity => "Infinity"u8;
public static ReadOnlySpan<byte> 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<byte> 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
}
Loading