Skip to content

Commit

Permalink
Ensure RequestInvokers do not need ITransportConfiguration.
Browse files Browse the repository at this point in the history
Information is read at a later time from RequestData which is now likely a static instance.
  • Loading branch information
Mpdreamz committed Nov 7, 2024
1 parent 5cf92e1 commit c15e518
Show file tree
Hide file tree
Showing 9 changed files with 34 additions and 43 deletions.
10 changes: 10 additions & 0 deletions src/Elastic.Transport/Components/Pipeline/RequestData.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Collections.Specialized;
using System.Security.Cryptography.X509Certificates;
using Elastic.Transport.Extensions;
using Elastic.Transport.Products;

namespace Elastic.Transport;

Expand Down Expand Up @@ -40,6 +41,9 @@ public RequestData(ITransportConfiguration global, IRequestConfiguration? local
ProxyPassword = global.ProxyPassword;
DisableAutomaticProxyDetection = global.DisableAutomaticProxyDetection;
UserAgent = global.UserAgent;
ResponseBuilders = global.ResponseBuilders;
ProductResponseBuilders = global.ProductRegistration.ResponseBuilders;

KeepAliveInterval = (int)(global.KeepAliveInterval?.TotalMilliseconds ?? 2000);
KeepAliveTime = (int)(global.KeepAliveTime?.TotalMilliseconds ?? 2000);
RunAs = local?.RunAs ?? global.RunAs;
Expand Down Expand Up @@ -88,6 +92,12 @@ public RequestData(ITransportConfiguration global, IRequestConfiguration? local
}
}

/// <inheritdoc cref="ITransportConfiguration.ResponseBuilders"/>
public IReadOnlyCollection<IResponseBuilder> ProductResponseBuilders { get; }

/// <inheritdoc cref="ITransportConfiguration.ResponseBuilders"/>
public IReadOnlyCollection<IResponseBuilder> ResponseBuilders { get; }

/// <inheritdoc cref="ITransportConfiguration.MemoryStreamFactory"/>
public MemoryStreamFactory MemoryStreamFactory { get; }
/// <inheritdoc cref="ITransportConfiguration.MetaHeaderProvider"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,7 @@ public class HttpRequestInvoker : HttpWebRequestInvoker
/// <summary>
/// Create a new instance of the <see cref="HttpRequestInvoker"/>.
/// </summary>
public HttpRequestInvoker() : base() { }

/// <summary>
/// Create a new instance of the <see cref="HttpRequestInvoker"/>.
/// </summary>
/// <param name="transportConfiguration">The <see cref="ITransportConfiguration"/> from which response builders can be loaded.</param>
public HttpRequestInvoker(ITransportConfiguration transportConfiguration) : base(transportConfiguration) { }
public HttpRequestInvoker() { }

/// <summary> The default TransportClient implementation. Uses <see cref="HttpWebRequest" /> on the current .NET desktop framework.</summary>
internal HttpRequestInvoker(ResponseFactory responseFactory) : base(responseFactory) { }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,7 @@ public class HttpRequestInvoker : IRequestInvoker
/// <summary>
/// Create a new instance of the <see cref="HttpRequestInvoker"/>.
/// </summary>
public HttpRequestInvoker() : this(new TransportConfiguration()) { }

/// <summary>
/// Create a new instance of the <see cref="HttpRequestInvoker"/>.
/// </summary>
/// <param name="transportConfiguration">The <see cref="ITransportConfiguration"/> from which response builders can be loaded.</param>
public HttpRequestInvoker(ITransportConfiguration transportConfiguration) :
this(new DefaultResponseFactory(transportConfiguration)) { }
public HttpRequestInvoker() : this(new DefaultResponseFactory()) { }

internal HttpRequestInvoker(ResponseFactory responseFactory)
{
Expand All @@ -56,13 +49,13 @@ internal HttpRequestInvoker(ResponseFactory responseFactory)
/// Allows consumers to inject their own HttpMessageHandler, and optionally call our default implementation.
/// </summary>
public HttpRequestInvoker(Func<HttpMessageHandler, RequestData, HttpMessageHandler> wrappingHandler) :
this(wrappingHandler, new DefaultResponseFactory(new TransportConfiguration())) { }
this(wrappingHandler, new DefaultResponseFactory()) { }

/// <summary>
/// Allows consumers to inject their own HttpMessageHandler, and optionally call our default implementation.
/// </summary>
public HttpRequestInvoker(Func<HttpMessageHandler, RequestData, HttpMessageHandler> wrappingHandler, ITransportConfiguration transportConfiguration) :
this(wrappingHandler, new DefaultResponseFactory(transportConfiguration))
this(wrappingHandler, new DefaultResponseFactory())
{ }

internal HttpRequestInvoker(Func<HttpMessageHandler, RequestData, HttpMessageHandler> wrappingHandler, ResponseFactory responseFactory)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,7 @@ static HttpWebRequestInvoker()
/// <summary>
/// Create a new instance of the <see cref="HttpWebRequestInvoker"/>.
/// </summary>
public HttpWebRequestInvoker() : this(new TransportConfiguration()) { }

/// <summary>
/// Create a new instance of the <see cref="HttpWebRequestInvoker"/>.
/// </summary>
/// <param name="transportConfiguration">The <see cref="ITransportConfiguration"/> from which response builders can be loaded.</param>
public HttpWebRequestInvoker(ITransportConfiguration transportConfiguration) :
this(new DefaultResponseFactory(transportConfiguration))
{ }
public HttpWebRequestInvoker() : this(new DefaultResponseFactory()) { }

internal HttpWebRequestInvoker(ResponseFactory responseFactory) => ResponseFactory = responseFactory;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,11 @@ public class InMemoryRequestInvoker : IRequestInvoker
/// Every request will succeed with this overload, note that it won't actually return mocked responses
/// so using this overload might fail if you are using it to test high level bits that need to deserialize the response.
/// </summary>
public InMemoryRequestInvoker() : this(null) { }

/// <inheritdoc cref="InMemoryRequestInvoker()"/>
public InMemoryRequestInvoker(ProductRegistration? productRegistration)
public InMemoryRequestInvoker()
{
_statusCode = 200;

productRegistration ??= DefaultProductRegistration.Default;
ResponseFactory = new DefaultResponseFactory(new TransportConfiguration(null, productRegistration));
ResponseFactory = new DefaultResponseFactory();
}

/// <inheritdoc cref="InMemoryRequestInvoker"/>
Expand All @@ -49,7 +45,7 @@ public InMemoryRequestInvoker(byte[] responseBody, int statusCode = 200, Excepti
_contentType = contentType;
_headers = headers;

ResponseFactory = new DefaultResponseFactory(new TransportConfiguration(null, DefaultProductRegistration.Default));
ResponseFactory = new DefaultResponseFactory();
}

/// <inheritdoc />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ internal TransportConfiguration(
{
//non init properties
NodePool = nodePool;
RequestInvoker = requestInvoker ?? new HttpRequestInvoker(this);
RequestInvoker = requestInvoker ?? new HttpRequestInvoker();
ProductRegistration = productRegistration ?? DefaultProductRegistration.Default;
RequestInvoker = requestInvoker ?? new HttpRequestInvoker();
RequestResponseSerializer = serializer ?? new LowLevelRequestResponseSerializer();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public abstract class TransportConfigurationDescriptorBase<T> : ITransportConfig
protected TransportConfigurationDescriptorBase(NodePool nodePool, IRequestInvoker? requestInvoker, Serializer? requestResponseSerializer, ProductRegistration? productRegistration)
{
_nodePool = nodePool;
_requestInvoker = requestInvoker ?? new HttpRequestInvoker(this);
_requestInvoker = requestInvoker ?? new HttpRequestInvoker();
_productRegistration = productRegistration ?? DefaultProductRegistration.Default;
_requestInvoker = requestInvoker ?? new HttpRequestInvoker();
_requestResponseSerializer = requestResponseSerializer ?? new LowLevelRequestResponseSerializer();
Expand Down
20 changes: 13 additions & 7 deletions src/Elastic.Transport/Responses/DefaultResponseFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using System.Threading.Tasks;
using Elastic.Transport.Diagnostics;
using Elastic.Transport.Extensions;
using Elastic.Transport.Products;

namespace Elastic.Transport;

Expand All @@ -20,7 +21,7 @@ namespace Elastic.Transport;
/// <remarks>
/// Create an instance of the factory using the provided configuration.
/// </remarks>
internal sealed class DefaultResponseFactory(ITransportConfiguration transportConfiguration) : ResponseFactory
internal sealed class DefaultResponseFactory : ResponseFactory
{
private readonly ConcurrentDictionary<Type, IResponseBuilder> _resolvedBuilders = new()
{
Expand All @@ -31,7 +32,7 @@ internal sealed class DefaultResponseFactory(ITransportConfiguration transportCo
[typeof(VoidResponse)] = new VoidResponseBuilder()
};

private readonly ITransportConfiguration? _transportConfiguration = transportConfiguration;
//private readonly ITransportConfiguration? _transportConfiguration = transportConfiguration;

/// <inheritdoc/>
public override TResponse Create<TResponse>(
Expand Down Expand Up @@ -87,7 +88,8 @@ private async ValueTask<TResponse> CreateCoreAsync<TResponse>(

TResponse? response = null;

if (MayHaveBody(statusCode, endpoint.Method, contentLength) && TryResolveBuilder<TResponse>(out var builder))
if (MayHaveBody(statusCode, endpoint.Method, contentLength)
&& TryResolveBuilder<TResponse>(out var builder, requestData.ProductResponseBuilders, requestData.ResponseBuilders))
{
var ownsStream = false;

Expand Down Expand Up @@ -122,17 +124,21 @@ private async ValueTask<TResponse> CreateCoreAsync<TResponse>(
response ??= new TResponse();
response.ApiCallDetails = details;
return response;
}
}

private bool TryResolveBuilder<TResponse>(out IResponseBuilder builder) where TResponse : TransportResponse, new()
private bool TryResolveBuilder<TResponse>(
out IResponseBuilder builder,
IReadOnlyCollection<IResponseBuilder> productResponseBuilders,
IReadOnlyCollection<IResponseBuilder> responseBuilders
) where TResponse : TransportResponse, new()
{
if (_resolvedBuilders.TryGetValue(typeof(TResponse), out builder))
return true;

if (TryFindResponseBuilder(_transportConfiguration.ResponseBuilders, _resolvedBuilders, ref builder))
if (TryFindResponseBuilder(responseBuilders, _resolvedBuilders, ref builder))
return true;

return TryFindResponseBuilder(_transportConfiguration.ProductRegistration.ResponseBuilders, _resolvedBuilders, ref builder);
return TryFindResponseBuilder(productResponseBuilders, _resolvedBuilders, ref builder);

static bool TryFindResponseBuilder(IEnumerable<IResponseBuilder> responseBuilders, ConcurrentDictionary<Type, IResponseBuilder> resolvedBuilders, ref IResponseBuilder builder)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ public static class InMemoryConnectionFactory
{
public static TransportConfiguration Create(ProductRegistration productRegistration = null)
{
var invoker = new InMemoryRequestInvoker(productRegistration);
var invoker = new InMemoryRequestInvoker();
var pool = new SingleNodePool(new Uri("http://localhost:9200"));
var settings = new TransportConfiguration(pool, invoker, productRegistration: productRegistration);
return settings;
Expand Down

0 comments on commit c15e518

Please sign in to comment.