From 9fcd85710e3cc6cb279d419782598803afe84c15 Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Wed, 16 Oct 2024 10:48:01 +0200 Subject: [PATCH 1/2] Introduce providing json options as its own construct --- .../IJsonSerializerOptionsProvider.cs | 47 ++++++++++ .../LowLevelRequestResponseSerializer.cs | 47 ++-------- .../Serialization/SystemTextJsonSerializer.cs | 93 ++++--------------- 3 files changed, 74 insertions(+), 113 deletions(-) create mode 100644 src/Elastic.Transport/Components/Serialization/IJsonSerializerOptionsProvider.cs diff --git a/src/Elastic.Transport/Components/Serialization/IJsonSerializerOptionsProvider.cs b/src/Elastic.Transport/Components/Serialization/IJsonSerializerOptionsProvider.cs new file mode 100644 index 0000000..eb040a7 --- /dev/null +++ b/src/Elastic.Transport/Components/Serialization/IJsonSerializerOptionsProvider.cs @@ -0,0 +1,47 @@ +// 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; +using System.Collections.Generic; +using System.Text.Json; +using System.Text.Json.Serialization; + +namespace Elastic.Transport; + +/// +/// Provides an instance of to +/// +public interface IJsonSerializerOptionsProvider +{ + /// + JsonSerializerOptions CreateJsonSerializerOptions(); +} +/// +/// Default implementation of specialized in providing more converters and +/// altering the shared used by and its derrived classes +/// +public class TransportSerializerOptionsProvider : IJsonSerializerOptionsProvider +{ + private readonly JsonSerializerOptions _options = new(); + + /// + public JsonSerializerOptions? CreateJsonSerializerOptions() => _options; + + /// + public TransportSerializerOptionsProvider() { } + + /// + public TransportSerializerOptionsProvider(IReadOnlyCollection bakedIn, IReadOnlyCollection? userProvided, Action? optionsAction = null) + { + foreach (var converter in bakedIn) + _options.Converters.Add(converter); + + foreach (var converter in userProvided ?? []) + _options.Converters.Add(converter); + + optionsAction?.Invoke(_options); + + } +} + diff --git a/src/Elastic.Transport/Components/Serialization/LowLevelRequestResponseSerializer.cs b/src/Elastic.Transport/Components/Serialization/LowLevelRequestResponseSerializer.cs index 5533549..183d4cf 100644 --- a/src/Elastic.Transport/Components/Serialization/LowLevelRequestResponseSerializer.cs +++ b/src/Elastic.Transport/Components/Serialization/LowLevelRequestResponseSerializer.cs @@ -3,37 +3,22 @@ // See the LICENSE file in the project root for more information using System.Collections.Generic; -using System.Collections.ObjectModel; -using System.Linq; using System.Text.Json; using System.Text.Json.Serialization; -using Elastic.Transport.Extensions; - namespace Elastic.Transport; /// /// Default low level request/response-serializer implementation for which serializes using /// the Microsoft System.Text.Json library /// -internal sealed class LowLevelRequestResponseSerializer : - SystemTextJsonSerializer +internal sealed class LowLevelRequestResponseSerializer : SystemTextJsonSerializer { /// /// Provides a static reusable reference to an instance of to promote reuse. /// internal static readonly LowLevelRequestResponseSerializer Instance = new(); - private IReadOnlyCollection AdditionalConverters { get; } - - private IList BakedInConverters { get; } = new List - { - new ExceptionConverter(), - new ErrorCauseConverter(), - new ErrorConverter(), - new DynamicDictionaryConverter() - }; - /// > public LowLevelRequestResponseSerializer() : this(null) { } @@ -41,28 +26,12 @@ public LowLevelRequestResponseSerializer() : this(null) { } /// > /// /// Add more default converters onto being used - public LowLevelRequestResponseSerializer(IEnumerable? converters) => - AdditionalConverters = converters != null - ? new ReadOnlyCollection(converters.ToList()) - : EmptyReadOnly.Collection; - - /// - /// Creates used for serialization. - /// Override on a derived serializer to change serialization. - /// - protected override JsonSerializerOptions? CreateJsonSerializerOptions() - { - var options = new JsonSerializerOptions - { - DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull - }; - - foreach (var converter in BakedInConverters) - options.Converters.Add(converter); - - foreach (var converter in AdditionalConverters) - options.Converters.Add(converter); + public LowLevelRequestResponseSerializer(IReadOnlyCollection? converters) + : base(new TransportSerializerOptionsProvider([ + new ExceptionConverter(), + new ErrorCauseConverter(), + new ErrorConverter(), + new DynamicDictionaryConverter() + ], converters, options => { options.DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull; })) { } - return options; - } } diff --git a/src/Elastic.Transport/Components/Serialization/SystemTextJsonSerializer.cs b/src/Elastic.Transport/Components/Serialization/SystemTextJsonSerializer.cs index 6d8fe3b..6e9a5c0 100644 --- a/src/Elastic.Transport/Components/Serialization/SystemTextJsonSerializer.cs +++ b/src/Elastic.Transport/Components/Serialization/SystemTextJsonSerializer.cs @@ -3,8 +3,10 @@ // See the LICENSE file in the project root for more information using System; +using System.Collections.Generic; using System.IO; using System.Text.Json; +using System.Text.Json.Serialization; using System.Threading; using System.Threading.Tasks; @@ -14,14 +16,23 @@ namespace Elastic.Transport; /// An abstract implementation of a transport which serializes using the Microsoft /// System.Text.Json library. /// -public abstract class SystemTextJsonSerializer : - Serializer +public abstract class SystemTextJsonSerializer : Serializer { - private readonly SemaphoreSlim _semaphore = new(1); + private readonly JsonSerializerOptions? _options; + private readonly JsonSerializerOptions? _indentedOptions; - private bool _initialized; - private JsonSerializerOptions? _options; - private JsonSerializerOptions? _indentedOptions; + /// + /// An abstract implementation of a transport which serializes using the Microsoft + /// System.Text.Json library. + /// + protected SystemTextJsonSerializer(IJsonSerializerOptionsProvider? provider = null) + { + + provider ??= new TransportSerializerOptionsProvider(); + _options = provider.CreateJsonSerializerOptions(); + _indentedOptions = provider.CreateJsonSerializerOptions(); + _indentedOptions.WriteIndented = true; + } #region Serializer @@ -74,80 +85,14 @@ public override Task SerializeAsync(T data, Stream stream, #endregion Serializer - /// - /// A factory method that can create an instance of that will - /// be used when serializing. - /// - /// - protected abstract JsonSerializerOptions? CreateJsonSerializerOptions(); - - /// - /// A callback function that is invoked after the have been created and the - /// serializer got fully initialized. - /// - protected virtual void Initialized() - { - } - /// /// Returns the for this serializer, based on the given . /// /// The serialization formatting. /// The requested or null, if the serializer is not initialized yet. - protected internal JsonSerializerOptions? GetJsonSerializerOptions(SerializationFormatting formatting = SerializationFormatting.None) - { - Initialize(); - - return (formatting is SerializationFormatting.None) - ? _options - : _indentedOptions; - } + protected internal JsonSerializerOptions? GetJsonSerializerOptions(SerializationFormatting formatting = SerializationFormatting.None) => + formatting is SerializationFormatting.None ? _options : _indentedOptions; - /// - /// Initializes a serializer instance such that its are populated. - /// - private void Initialize() - { - // Exit early, if already initialized - if (_initialized) - return; - - _semaphore.Wait(); - - try - { - // Exit early, if the current thread lost the race - if (_initialized) - return; - - var options = CreateJsonSerializerOptions(); - - if (options is null) - { - _options = new JsonSerializerOptions(); - _indentedOptions = new JsonSerializerOptions - { - WriteIndented = true - }; - } - else - { - _options = options; - _indentedOptions = new JsonSerializerOptions(options) - { - WriteIndented = true - }; - } - - _initialized = true; - - Initialized(); - } - finally - { - _semaphore.Release(); - } - } private static bool TryReturnDefault(Stream? stream, out T deserialize) { From e53e509a3ed9396475d9c87c7966fb0bc9278449 Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Wed, 16 Oct 2024 10:53:56 +0200 Subject: [PATCH 2/2] CreateJsonSerializerOptions needs to be pure because its called multiple times --- .../IJsonSerializerOptionsProvider.cs | 35 +++++++++++++------ 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/src/Elastic.Transport/Components/Serialization/IJsonSerializerOptionsProvider.cs b/src/Elastic.Transport/Components/Serialization/IJsonSerializerOptionsProvider.cs index eb040a7..1660828 100644 --- a/src/Elastic.Transport/Components/Serialization/IJsonSerializerOptionsProvider.cs +++ b/src/Elastic.Transport/Components/Serialization/IJsonSerializerOptionsProvider.cs @@ -23,25 +23,38 @@ public interface IJsonSerializerOptionsProvider /// public class TransportSerializerOptionsProvider : IJsonSerializerOptionsProvider { - private readonly JsonSerializerOptions _options = new(); + private readonly IReadOnlyCollection? _bakedInConverters; + private readonly IReadOnlyCollection? _userProvidedConverters; + private readonly Action? _mutateOptions; /// - public JsonSerializerOptions? CreateJsonSerializerOptions() => _options; + public JsonSerializerOptions? CreateJsonSerializerOptions() + { + var options = new JsonSerializerOptions(); + foreach (var converter in _bakedInConverters ?? []) + options.Converters.Add(converter); + + foreach (var converter in _userProvidedConverters ?? []) + options.Converters.Add(converter); + + _mutateOptions?.Invoke(options); + + return options; + } /// public TransportSerializerOptionsProvider() { } /// - public TransportSerializerOptionsProvider(IReadOnlyCollection bakedIn, IReadOnlyCollection? userProvided, Action? optionsAction = null) + public TransportSerializerOptionsProvider( + IReadOnlyCollection bakedInConverters, + IReadOnlyCollection? userProvidedConverters, + Action? mutateOptions = null + ) { - foreach (var converter in bakedIn) - _options.Converters.Add(converter); - - foreach (var converter in userProvided ?? []) - _options.Converters.Add(converter); - - optionsAction?.Invoke(_options); - + _bakedInConverters = bakedInConverters; + _userProvidedConverters = userProvidedConverters; + _mutateOptions = mutateOptions; } }