Skip to content

[Backport 8.3] Fix properties serialization #6603

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
Jul 20, 2022
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
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ public DefaultRequestResponseSerializer(IElasticsearchClientSettings settings)
new SelfDeserializableConverterFactory(settings),
new SelfTwoWaySerializableConverterFactory(settings),
new IndicesJsonConverter(settings),
new PropertyNameConverter(settings),
new IdsConverter(settings),
new IsADictionaryConverter(),
new ResponseItemConverterFactory(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ public DefaultSourceSerializer(IElasticsearchClientSettings settings, JsonSerial
new IdConverter(settings),
new RelationNameConverter(settings),
new RoutingConverter(settings),
new PropertyNameConverter(settings),
new JoinFieldConverter(settings),
new LazyDocumentConverter(settings),
new IdsConverter(settings)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,17 @@
using System;
using System.Text.Json;
using System.Text.Json.Serialization;
using Elastic.Transport;

namespace Elastic.Clients.Elasticsearch
{
internal sealed class PropertyNameConverter : JsonConverter<PropertyName?>
internal sealed class PropertyNameConverter : SettingsJsonConverter<PropertyName>
{
private readonly IElasticsearchClientSettings _settings;
public override void WriteAsPropertyName(Utf8JsonWriter writer, PropertyName value, JsonSerializerOptions options) =>
writer.WritePropertyName(((IUrlParameter)value).GetString(GetSettings(options)));

public PropertyNameConverter(IElasticsearchClientSettings settings) => _settings = settings;
public override PropertyName ReadAsPropertyName(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) =>
reader.GetString();

public override PropertyName? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
{
Expand All @@ -33,7 +36,7 @@ public override void Write(Utf8JsonWriter writer, PropertyName? value, JsonSeria
return;
}

var propertyName = _settings.Inferrer.PropertyName(value);
var propertyName = GetSettings(options).Inferrer.PropertyName(value);
writer.WriteStringValue(propertyName);
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// 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.Text.Json;
using System.Text.Json.Serialization;

namespace Elastic.Clients.Elasticsearch
{
/// <summary>
/// Used for derived converters which need access to <see cref="IElasticsearchClientSettings"/> in order to serialize.
/// </summary>
/// <typeparam name="T">The type of object or value handled by the converter.</typeparam>
internal abstract class SettingsJsonConverter<T> : JsonConverter<T>
{
private IElasticsearchClientSettings _settings;

/// <summary>
/// Access the <see cref="IElasticsearchClientSettings"/> for a given <see cref="JsonSerializerOptions"/> instance.
/// </summary>
/// <param name="options">The <see cref="JsonSerializerOptions"/> for which the <see cref="IElasticsearchClientSettings"/> should be retrieved.</param>
/// <returns>An <see cref="IElasticsearchClientSettings"/> instance related to the provided <see cref="JsonSerializerOptions"/>.</returns>
/// <exception cref="JsonException">Thrown if a corresponding <see cref="IElasticsearchClientSettings"/> instance cannot be found for the <see cref="JsonSerializerOptions"/>.</exception>
protected IElasticsearchClientSettings GetSettings(JsonSerializerOptions options)
{
if (_settings is not null)
return _settings;

// We avoid locking here as the result of accessing the settings should be idemopotent and low cost.

if (options.TryGetClientSettings(out var settings))
{
_settings = settings;
return _settings;
}

throw new JsonException("Unable to retrieve ElasticsearchClientSettings settings from JsonSerializerOptions.");
}
}
}
13 changes: 12 additions & 1 deletion src/Elastic.Clients.Elasticsearch/Types/Mapping/PropertyBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,5 +40,16 @@ internal sealed partial class PropertyInterfaceConverter : JsonConverter<IProper
return DeserializeVariant(type, ref reader, options);
}

public override void Write(Utf8JsonWriter writer, IProperty value, JsonSerializerOptions options) => throw new NotImplementedException();
public override void Write(Utf8JsonWriter writer, IProperty value, JsonSerializerOptions options)
{
if (value is null)
{
writer.WriteNullValue();
return;
}

var type = value.GetType();

JsonSerializer.Serialize(writer, value, type, options);
}
}
42 changes: 42 additions & 0 deletions tests/Tests/Serialization/Mapping/PropertiesSerializationTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// 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.Threading.Tasks;
using Elastic.Clients.Elasticsearch.Mapping;
using Tests.Domain;
using VerifyXunit;

namespace Tests.Serialization.Mapping;

[UsesVerify]
public class PropertiesSerializationTests : SerializerTestBase
{
[U]
public async Task CanSerialize_Properties_WithPropertyNameExpression()
{
var result = await RoundTripAndVerifyJson(new Properties<Project>
{
{ p => p.Name, new TextProperty { Boost = 1.2 } }
});

var textProperty = result["name"].Should().BeOfType<TextProperty>().Subject;
textProperty.Boost.Should().Be(1.2);
}

[U]
public async Task CanSerialize_MultipleProperties_WithPropertyNameExpression()
{
var result = await RoundTripAndVerifyJson(new Properties<Project>
{
{ p => p.Name, new TextProperty { Boost = 1.2 } },
{ p => p.Description, new TextProperty { Boost = 1.4 } }
});

var nameTextProperty = result["name"].Should().BeOfType<TextProperty>().Subject;
nameTextProperty.Boost.Should().Be(1.2);

var descriptionTextProperty = result["description"].Should().BeOfType<TextProperty>().Subject;
descriptionTextProperty.Boost.Should().Be(1.4);
}
}
15 changes: 15 additions & 0 deletions tests/Tests/Serialization/SerializerTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// See the LICENSE file in the project root for more information.

using System.IO;
using System.Text.Json;
using System.Threading.Tasks;
using Tests.Domain.Extensions;
using VerifyXunit;
Expand Down Expand Up @@ -129,6 +130,20 @@ static SerializerTestBase()

protected static Inferrer Inferrer => _settings.Inferrer;

public static async Task<string> SerializeAndVerifyJson<T>(T data)
{
var serialisedJson = await SerializeAndGetJsonStringAsync(data);
await Verifier.VerifyJson(serialisedJson);
return serialisedJson;
}

public static async Task<T> RoundTripAndVerifyJson<T>(T data)
{
var serialisedJson = await SerializeAndGetJsonStringAsync(data);
await Verifier.VerifyJson(serialisedJson);
return JsonSerializer.Deserialize<T>(serialisedJson);
}

protected static Stream WrapInStream(string json)
{
var stream = new MemoryStream();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
description: {
boost: 1.4,
type: text
},
name: {
boost: 1.2,
type: text
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
name: {
boost: 1.2,
type: text
}
}