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

Opt-in to set Connection:Close on downstream requests #1441

Open
wants to merge 5 commits into
base: release/24.0
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 9 additions & 1 deletion src/Ocelot/Configuration/Builder/DownstreamRouteBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ public class DownstreamRouteBuilder
private Version _downstreamHttpVersion;
private HttpVersionPolicy _downstreamHttpVersionPolicy;
private Dictionary<string, UpstreamHeaderTemplate> _upstreamHeaders;
private bool _connectionClose;

public DownstreamRouteBuilder()
{
Expand Down Expand Up @@ -275,6 +276,12 @@ public DownstreamRouteBuilder WithDownstreamHttpVersionPolicy(HttpVersionPolicy
return this;
}

public DownstreamRouteBuilder WithConnectionClose(bool connectionClose)
{
_connectionClose = connectionClose;
return this;
}

public DownstreamRoute Build()
{
return new DownstreamRoute(
Expand Down Expand Up @@ -313,6 +320,7 @@ public DownstreamRoute Build()
_downstreamHttpMethod,
_downstreamHttpVersion,
_downstreamHttpVersionPolicy,
_upstreamHeaders);
_upstreamHeaders,
_connectionClose);
}
}
14 changes: 14 additions & 0 deletions src/Ocelot/Configuration/Creator/ConnectionCloseCreator.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
using Ocelot.Configuration.File;

namespace Ocelot.Configuration.Creator
{
public class ConnectionCloseCreator : IConnectionCloseCreator
{
public bool Create(bool fileRouteConnectionClose, FileGlobalConfiguration globalConfiguration)
{
var globalConnectionClose = globalConfiguration.ConnectionClose;

return fileRouteConnectionClose || globalConnectionClose;
}
}
}
9 changes: 9 additions & 0 deletions src/Ocelot/Configuration/Creator/IConnectionCloseCreator.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
using Ocelot.Configuration.File;

namespace Ocelot.Configuration.Creator
{
public interface IConnectionCloseCreator
{
bool Create(bool fileRouteConnectionClose, FileGlobalConfiguration globalConfiguration);
}
}
8 changes: 7 additions & 1 deletion src/Ocelot/Configuration/Creator/RoutesCreator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ public class RoutesCreator : IRoutesCreator
private readonly ISecurityOptionsCreator _securityOptionsCreator;
private readonly IVersionCreator _versionCreator;
private readonly IVersionPolicyCreator _versionPolicyCreator;
private readonly IConnectionCloseCreator _connectionCloseCreator;

public RoutesCreator(
IClaimsToThingCreator claimsToThingCreator,
Expand All @@ -41,7 +42,8 @@ public class RoutesCreator : IRoutesCreator
ISecurityOptionsCreator securityOptionsCreator,
IVersionCreator versionCreator,
IVersionPolicyCreator versionPolicyCreator,
IUpstreamHeaderTemplatePatternCreator upstreamHeaderTemplatePatternCreator)
IUpstreamHeaderTemplatePatternCreator upstreamHeaderTemplatePatternCreator,
IConnectionCloseCreator connectionCloseCreator)
{
_routeKeyCreator = routeKeyCreator;
_loadBalancerOptionsCreator = loadBalancerOptionsCreator;
Expand All @@ -61,6 +63,7 @@ public class RoutesCreator : IRoutesCreator
_versionCreator = versionCreator;
_versionPolicyCreator = versionPolicyCreator;
_upstreamHeaderTemplatePatternCreator = upstreamHeaderTemplatePatternCreator;
_connectionCloseCreator = connectionCloseCreator;
}

public List<Route> Create(FileConfiguration fileConfiguration)
Expand Down Expand Up @@ -114,6 +117,8 @@ private DownstreamRoute SetUpDownstreamRoute(FileRoute fileRoute, FileGlobalConf

var downstreamHttpVersionPolicy = _versionPolicyCreator.Create(fileRoute.DownstreamHttpVersionPolicy);

var connectionClose = _connectionCloseCreator.Create(fileRoute.ConnectionClose, globalConfiguration);

var route = new DownstreamRouteBuilder()
.WithKey(fileRoute.Key)
.WithDownstreamPathTemplate(fileRoute.DownstreamPathTemplate)
Expand Down Expand Up @@ -151,6 +156,7 @@ private DownstreamRoute SetUpDownstreamRoute(FileRoute fileRoute, FileGlobalConf
.WithDownstreamHttpVersion(downstreamHttpVersion)
.WithDownstreamHttpVersionPolicy(downstreamHttpVersionPolicy)
.WithDownStreamHttpMethod(fileRoute.DownstreamHttpMethod)
.WithConnectionClose(connectionClose)
.Build();

return route;
Expand Down
5 changes: 4 additions & 1 deletion src/Ocelot/Configuration/DownstreamRoute.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ public class DownstreamRoute
string downstreamHttpMethod,
Version downstreamHttpVersion,
HttpVersionPolicy downstreamHttpVersionPolicy,
Dictionary<string, UpstreamHeaderTemplate> upstreamHeaders)
Dictionary<string, UpstreamHeaderTemplate> upstreamHeaders,
bool connectionClose)
{
DangerousAcceptAnyServerCertificateValidator = dangerousAcceptAnyServerCertificateValidator;
AddHeadersToDownstream = addHeadersToDownstream;
Expand Down Expand Up @@ -79,6 +80,7 @@ public class DownstreamRoute
DownstreamHttpVersion = downstreamHttpVersion;
DownstreamHttpVersionPolicy = downstreamHttpVersionPolicy;
UpstreamHeaders = upstreamHeaders ?? new();
ConnectionClose = connectionClose;
}

public string Key { get; }
Expand Down Expand Up @@ -128,5 +130,6 @@ public class DownstreamRoute
/// </remarks>
public HttpVersionPolicy DownstreamHttpVersionPolicy { get; }
public Dictionary<string, UpstreamHeaderTemplate> UpstreamHeaders { get; }
public bool ConnectionClose { get; }
}
}
3 changes: 3 additions & 0 deletions src/Ocelot/Configuration/File/FileGlobalConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ public FileGlobalConfiguration()
LoadBalancerOptions = new FileLoadBalancerOptions();
QoSOptions = new FileQoSOptions();
HttpHandlerOptions = new FileHttpHandlerOptions();
ConnectionClose = false;
}

public string RequestIdKey { get; set; }
Expand Down Expand Up @@ -42,5 +43,7 @@ public FileGlobalConfiguration()
/// </list>
/// </remarks>
public string DownstreamHttpVersionPolicy { get; set; }

public bool ConnectionClose { get; set; }
}
}
7 changes: 5 additions & 2 deletions src/Ocelot/Configuration/File/FileRoute.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ public FileRoute()
UpstreamHeaderTemplates = new Dictionary<string, string>();
UpstreamHeaderTransform = new Dictionary<string, string>();
UpstreamHttpMethod = new List<string>();
ConnectionClose = false;
}

public FileRoute(FileRoute from)
Expand All @@ -37,6 +38,7 @@ public FileRoute(FileRoute from)
public Dictionary<string, string> AddQueriesToRequest { get; set; }
public FileAuthenticationOptions AuthenticationOptions { get; set; }
public Dictionary<string, string> ChangeDownstreamPathTemplate { get; set; }
public bool ConnectionClose { get; set; }
public bool DangerousAcceptAnyServerCertificateValidator { get; set; }
public List<string> DelegatingHandlers { get; set; }
public Dictionary<string, string> DownstreamHeaderTransform { get; set; }
Expand All @@ -56,7 +58,7 @@ public FileRoute(FileRoute from)
/// </remarks>
public string DownstreamHttpVersionPolicy { get; set; }
public string DownstreamPathTemplate { get; set; }
public string DownstreamScheme { get; set; }
public string DownstreamScheme { get; set; }
public FileCacheOptions FileCacheOptions { get; set; }
public FileHttpHandlerOptions HttpHandlerOptions { get; set; }
public string Key { get; set; }
Expand Down Expand Up @@ -95,6 +97,7 @@ public static void DeepCopy(FileRoute from, FileRoute to)
to.AddQueriesToRequest = new(from.AddQueriesToRequest);
to.AuthenticationOptions = new(from.AuthenticationOptions);
to.ChangeDownstreamPathTemplate = new(from.ChangeDownstreamPathTemplate);
to.ConnectionClose = from.ConnectionClose;
to.DangerousAcceptAnyServerCertificateValidator = from.DangerousAcceptAnyServerCertificateValidator;
to.DelegatingHandlers = new(from.DelegatingHandlers);
to.DownstreamHeaderTransform = new(from.DownstreamHeaderTransform);
Expand All @@ -103,7 +106,7 @@ public static void DeepCopy(FileRoute from, FileRoute to)
to.DownstreamHttpVersion = from.DownstreamHttpVersion;
to.DownstreamHttpVersionPolicy = from.DownstreamHttpVersionPolicy;
to.DownstreamPathTemplate = from.DownstreamPathTemplate;
to.DownstreamScheme = from.DownstreamScheme;
to.DownstreamScheme = from.DownstreamScheme;
to.FileCacheOptions = new(from.FileCacheOptions);
to.HttpHandlerOptions = new(from.HttpHandlerOptions);
to.Key = from.Key;
Expand Down
1 change: 1 addition & 0 deletions src/Ocelot/DependencyInjection/OcelotBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ public OcelotBuilder(IServiceCollection services, IConfiguration configurationRo
Services.TryAddSingleton<IAuthenticationOptionsCreator, AuthenticationOptionsCreator>();
Services.TryAddSingleton<IUpstreamTemplatePatternCreator, UpstreamTemplatePatternCreator>();
Services.TryAddSingleton<IRequestIdKeyCreator, RequestIdKeyCreator>();
Services.TryAddSingleton<IConnectionCloseCreator, ConnectionCloseCreator>();
Services.TryAddSingleton<IServiceProviderConfigurationCreator, ServiceProviderConfigurationCreator>();
Services.TryAddSingleton<IQoSOptionsCreator, QoSOptionsCreator>();
Services.TryAddSingleton<IRouteOptionsCreator, RouteOptionsCreator>();
Expand Down
136 changes: 136 additions & 0 deletions src/Ocelot/Requester/HttpClientBuilder.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
using System;
using System.Linq;
using System.Net;
using System.Net.Http;

using Ocelot.Configuration;

using Ocelot.Logging;

namespace Ocelot.Requester
{
public interface IHttpClientBuilder { }
public interface IHttpClientCache
{
IHttpClient Get(DownstreamRoute cacheKey);
void Set(DownstreamRoute cacheKey, IHttpClient client, TimeSpan span);
}

public class HttpClientBuilder : IHttpClientBuilder
{
private readonly IDelegatingHandlerHandlerFactory _factory;
private readonly IHttpClientCache _cacheHandlers;
private readonly IOcelotLogger _logger;
private DownstreamRoute _cacheKey;
private HttpClient _httpClient;
private IHttpClient _client;
private readonly TimeSpan _defaultTimeout;

public HttpClientBuilder(
IDelegatingHandlerHandlerFactory factory,
IHttpClientCache cacheHandlers,
IOcelotLogger logger)
{
_factory = factory;
_cacheHandlers = cacheHandlers;
_logger = logger;

// This is hardcoded at the moment but can easily be added to configuration
// if required by a user request.
_defaultTimeout = TimeSpan.FromSeconds(90);
}

public IHttpClient Create(DownstreamRoute downstreamRoute)
{
_cacheKey = downstreamRoute;

var httpClient = _cacheHandlers.Get(_cacheKey);

if (httpClient != null)
{
_client = httpClient;
return httpClient;
}

var handler = CreateHandler(downstreamRoute);

if (downstreamRoute.DangerousAcceptAnyServerCertificateValidator)
{
handler.ServerCertificateCustomValidationCallback =
HttpClientHandler.DangerousAcceptAnyServerCertificateValidator;

_logger
.LogWarning($"You have ignored all SSL warnings by using DangerousAcceptAnyServerCertificateValidator for this DownstreamRoute, UpstreamPathTemplate: {downstreamRoute.UpstreamPathTemplate}, DownstreamPathTemplate: {downstreamRoute.DownstreamPathTemplate}");
}

var timeout = downstreamRoute.QosOptions.TimeoutValue == 0
? _defaultTimeout
: TimeSpan.FromMilliseconds(downstreamRoute.QosOptions.TimeoutValue);

_httpClient = new HttpClient(CreateHttpMessageHandler(handler, downstreamRoute))
{
Timeout = timeout,
};

_client = new HttpClientWrapper(_httpClient, downstreamRoute.ConnectionClose); // TODO

return _client;
}

private static HttpClientHandler CreateHandler(DownstreamRoute downstreamRoute)
{
// Dont' create the CookieContainer if UseCookies is not set or the HttpClient will complain
// under .Net Full Framework
var useCookies = downstreamRoute.HttpHandlerOptions.UseCookieContainer;

return useCookies ? UseCookiesHandler(downstreamRoute) : UseNonCookiesHandler(downstreamRoute);
}

private static HttpClientHandler UseNonCookiesHandler(DownstreamRoute downstreamRoute)
{
return new HttpClientHandler
{
AllowAutoRedirect = downstreamRoute.HttpHandlerOptions.AllowAutoRedirect,
UseCookies = downstreamRoute.HttpHandlerOptions.UseCookieContainer,
UseProxy = downstreamRoute.HttpHandlerOptions.UseProxy,
MaxConnectionsPerServer = downstreamRoute.HttpHandlerOptions.MaxConnectionsPerServer,

};
}

private static HttpClientHandler UseCookiesHandler(DownstreamRoute downstreamRoute)
{
return new HttpClientHandler
{
AllowAutoRedirect = downstreamRoute.HttpHandlerOptions.AllowAutoRedirect,
UseCookies = downstreamRoute.HttpHandlerOptions.UseCookieContainer,
UseProxy = downstreamRoute.HttpHandlerOptions.UseProxy,
MaxConnectionsPerServer = downstreamRoute.HttpHandlerOptions.MaxConnectionsPerServer,
CookieContainer = new CookieContainer(),
};
}

public void Save()
{
_cacheHandlers.Set(_cacheKey, _client, TimeSpan.FromHours(24));
}

private HttpMessageHandler CreateHttpMessageHandler(HttpMessageHandler httpMessageHandler, DownstreamRoute request)
{
//todo handle error
var handlers = _factory.Get(request).Data;

handlers
.Select(handler => handler)
.Reverse()
.ToList()
.ForEach(handler =>
{
var delegatingHandler = handler();
delegatingHandler.InnerHandler = httpMessageHandler;
httpMessageHandler = delegatingHandler;
});
return httpMessageHandler;
}
Comment on lines +123 to +134
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
handlers
.Select(handler => handler)
.Reverse()
.ToList()
.ForEach(handler =>
{
var delegatingHandler = handler();
delegatingHandler.InnerHandler = httpMessageHandler;
httpMessageHandler = delegatingHandler;
});
return httpMessageHandler;
}
return handlers
.Reverse() // Reverse the sequence
.Aggregate(httpMessageHandler, (currentHandler, handlerFunc) =>
{
var delegatingHandler = handlerFunc();
delegatingHandler.InnerHandler = currentHandler;
return delegatingHandler;
});
return httpMessageHandler;
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeap! ToList was a bit overhead to get access to ForEach helper.
Aggregate helper is good, for sure because it is applicable for this case.
There is another coding life hack: calling All to process all elements of collection. 😃

But!... Ray, this file is redundant one! I left this file during rebasing just to keep the author's changes: see // TODO 😉
Both files are just container of new changes

  • src/Ocelot/Requester/HttpClientBuilder.cs
  • src/Ocelot/Requester/HttpClientWrapper.cs

You may check, there are no these files in develop! 🙃
This PR is not ready for reviews: it must be developed more ❗

}
}
30 changes: 30 additions & 0 deletions src/Ocelot/Requester/HttpClientWrapper.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
using System.Net.Http;
using System.Threading;
using System.Threading.Tasks;

namespace Ocelot.Requester
{
public interface IHttpClient { }

/// <summary>
/// This class was made to make unit testing easier when HttpClient is used.
/// </summary>
public class HttpClientWrapper : IHttpClient
{
public HttpClient Client { get; }

public bool ConnectionClose { get; } // TODO

public HttpClientWrapper(HttpClient client, bool connectionClose = false)
{
Client = client;
ConnectionClose = connectionClose; // TODO
}

public Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken = default)
{
request.Headers.ConnectionClose = ConnectionClose; // TODO
return Client.SendAsync(request, cancellationToken);
}
}
}
9 changes: 6 additions & 3 deletions test/Ocelot.UnitTests/Configuration/RoutesCreatorTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ public class RoutesCreatorTests : UnitTest
private readonly Mock<ISecurityOptionsCreator> _soCreator;
private readonly Mock<IVersionCreator> _versionCreator;
private readonly Mock<IVersionPolicyCreator> _versionPolicyCreator;
private readonly Mock<IConnectionCloseCreator> _connectionCloseCreator;
private FileConfiguration _fileConfig;
private RouteOptions _rro;
private string _requestId;
Expand Down Expand Up @@ -65,6 +66,7 @@ public RoutesCreatorTests()
_versionCreator = new Mock<IVersionCreator>();
_versionPolicyCreator = new Mock<IVersionPolicyCreator>();
_uhtpCreator = new Mock<IUpstreamHeaderTemplatePatternCreator>();
_connectionCloseCreator = new Mock<IConnectionCloseCreator>();

_creator = new RoutesCreator(
_cthCreator.Object,
Expand All @@ -83,11 +85,12 @@ public RoutesCreatorTests()
_soCreator.Object,
_versionCreator.Object,
_versionPolicyCreator.Object,
_uhtpCreator.Object);
_uhtpCreator.Object,
_connectionCloseCreator.Object);
}

[Fact]
public void should_return_nothing()
public void Should_return_nothing()
{
var fileConfig = new FileConfiguration();

Expand All @@ -98,7 +101,7 @@ public void should_return_nothing()
}

[Fact]
public void should_return_re_routes()
public void Should_return_re_routes()
{
var fileConfig = new FileConfiguration
{
Expand Down