Skip to content

[Backport 8.1] Source serialization always sends fractional format for double and floats. #7468

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

Merged
merged 1 commit into from
Mar 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
3 changes: 2 additions & 1 deletion exclusion.dic
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@ inferrer
elasticsearch
asciidocs
yyyy
enum
enum
trippable
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ public class DefaultSourceSerializer : SystemTextJsonSerializer
/// </summary>
public static JsonConverter[] DefaultBuiltInConverters => new JsonConverter[]
{
new JsonStringEnumConverter()
new JsonStringEnumConverter(),
new DoubleWithFractionalPortionConverter(),
new FloatWithFractionalPortionConverter()
};

private readonly JsonSerializerOptions _jsonSerializerOptions;
Expand Down Expand Up @@ -63,7 +65,7 @@ public static JsonSerializerOptions CreateDefaultJsonSerializerOptions(bool incl
{
DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull,
PropertyNamingPolicy = JsonNamingPolicy.CamelCase,
NumberHandling = JsonNumberHandling.AllowReadingFromString | JsonNumberHandling.AllowNamedFloatingPointLiterals
NumberHandling = JsonNumberHandling.AllowReadingFromString // For numerically mapped fields, it is possible for values in the source to be returned as strings, if they were indexed as such.
};

if (includeDefaultConverters)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
// 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 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))
{
var value = reader.GetString();

if (!double.TryParse(value, out var parsedValue))
ThrowHelper.ThrowJsonException($"Unable to parse '{value}' as a double.");

return parsedValue;
}

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

// 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("G17", CultureInfo.InvariantCulture);

if (utf16Text.Length < utf8bytes.Length)
{
try
{
var bytes = Encoding.UTF8.GetBytes(utf16Text);

if (bytes.Length < utf8bytes.Length)
{
bytes.CopyTo(utf8bytes);
}
}
catch
{
// Swallow this and fall through to our general exception.
}
}
#endif

ThrowHelper.ThrowJsonException($"Unable to serialize double value.");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
// 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<float>
{
// 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, 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<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

// 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);
}
}
catch
{
// Swallow this and fall through to our general exception.
}
}
#endif

ThrowHelper.ThrowJsonException($"Unable to serialize float value.");
}
}
14 changes: 14 additions & 0 deletions src/Elastic.Clients.Elasticsearch/Serialization/JsonConstants.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// 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;

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
}
17 changes: 17 additions & 0 deletions tests/Tests/Serialization/SerializerTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ protected string SerializeAndGetJsonString<T>(T data, ElasticsearchClientSetting
public abstract class SerializerTestBase
{
protected static readonly Serializer _requestResponseSerializer;
protected static readonly Serializer _sourceSerializer;
protected static readonly IElasticsearchClientSettings _settings;

static SerializerTestBase()
Expand All @@ -125,6 +126,7 @@ static SerializerTestBase()
var client = new ElasticsearchClient(settings);

_requestResponseSerializer = client.RequestResponseSerializer;
_sourceSerializer = client.SourceSerializer;
_settings = client.ElasticsearchClientSettings;
}

Expand Down Expand Up @@ -163,12 +165,27 @@ protected static string SerializeAndGetJsonString<T>(T data)
return reader.ReadToEnd();
}

protected static string SourceSerializeAndGetJsonString<T>(T data)
{
var stream = new MemoryStream();
_sourceSerializer.Serialize(data, stream);
stream.Position = 0;
var reader = new StreamReader(stream);
return reader.ReadToEnd();
}

protected static T DeserializeJsonString<T>(string json)
{
var stream = WrapInStream(json);
return _requestResponseSerializer.Deserialize<T>(stream);
}

protected static T SourceDeserializeJsonString<T>(string json)
{
var stream = WrapInStream(json);
return _sourceSerializer.Deserialize<T>(stream);
}

/// <summary>
/// Serialises the <paramref name="data"/> using the sync and async request/response serializer methods, comparing the results.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
// 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.

namespace Tests.Serialization;

public class SourceSerializationForNumericPropertiesTests : SerializerTestBase
{
[U]
public void FloatValuesIncludeDecimal_AndAreNotRounded()
{
var numericTests = new NumericTests { Float = 1.0f };

var json = SourceSerializeAndGetJsonString(numericTests);

json.Should().Be("{\"float\":1.0}");

var deserialized = SourceDeserializeJsonString<NumericTests>(json);

deserialized.Float.Should().Be(1.0f);
}

[U]
public void FloatMinValue_SerializesCorrectly()
{
var numericTests = new NumericTests { Float = float.MinValue };

var json = SourceSerializeAndGetJsonString(numericTests);

json.Should().Be("{\"float\":-3.4028235E+38}");

var deserialized = SourceDeserializeJsonString<NumericTests>(json);

deserialized.Float.Should().Be(float.MinValue);
}

[U]
public void FloatMaxValue_SerializesCorrectly()
{
var numericTests = new NumericTests { Float = float.MaxValue };

var json = SourceSerializeAndGetJsonString(numericTests);

json.Should().Be("{\"float\":3.4028235E+38}");

var deserialized = SourceDeserializeJsonString<NumericTests>(json);

deserialized.Float.Should().Be(float.MaxValue);
}

[U]
public void DoubleValuesIncludeFractionalPart_AndAreNotRounded()
{
var numericTests = new NumericTests { Double = 1.0 };

var json = SourceSerializeAndGetJsonString(numericTests);

json.Should().Be("{\"double\":1.0}");

var deserialized = SourceDeserializeJsonString<NumericTests>(json);

deserialized.Double.Should().Be(1.0);
}

[U]
public void DoubleMinValue_SerializesCorrectly()
{
var numericTests = new NumericTests { Double = double.MinValue };

var json = SourceSerializeAndGetJsonString(numericTests);

json.Should().Be("{\"double\":-1.7976931348623157E+308}");

var deserialized = SourceDeserializeJsonString<NumericTests>(json);

deserialized.Double.Should().Be(double.MinValue);
}

[U]
public void DoubleMaxValue_SerializesCorrectly()
{
var numericTests = new NumericTests { Double = double.MaxValue };

var json = SourceSerializeAndGetJsonString(numericTests);

json.Should().Be("{\"double\":1.7976931348623157E+308}");

var deserialized = SourceDeserializeJsonString<NumericTests>(json);

deserialized.Double.Should().Be(double.MaxValue);
}

[U]
public void DoubleAsString_DeserializesCorrectly()
{
var json = "{\"double\":\"1.0\"}";

var deserialized = SourceDeserializeJsonString<NumericTests>(json);

deserialized.Double.Should().Be(1.0);
}

[U]
public void DecimalValuesIncludeDecimal_AndAreNotRounded()
{
var numericTests = new NumericTests { Decimal = 1.0m };

var json = SourceSerializeAndGetJsonString(numericTests);

json.Should().Be("{\"decimal\":1.0}");
}

private class NumericTests
{
public float? Float { get; set; }
public double? Double { get; set; }
public decimal? Decimal { get; set; }
}
}