From 6dc18aeafc773abcc7217c462e3f0920a88cd4f6 Mon Sep 17 00:00:00 2001 From: zhannur Date: Sat, 25 May 2024 20:16:26 +0500 Subject: [PATCH 01/50] RequestTimeoutSeconds FileGlobalConfiguration --- .../File/FileGlobalConfiguration.cs | 2 ++ src/Ocelot/Requester/MessageInvokerPool.cs | 23 +++++++++++-------- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/src/Ocelot/Configuration/File/FileGlobalConfiguration.cs b/src/Ocelot/Configuration/File/FileGlobalConfiguration.cs index 7ce35f99e..ffa175193 100644 --- a/src/Ocelot/Configuration/File/FileGlobalConfiguration.cs +++ b/src/Ocelot/Configuration/File/FileGlobalConfiguration.cs @@ -17,6 +17,8 @@ public FileGlobalConfiguration() public string RequestIdKey { get; set; } + public int RequestTimeoutSeconds { get; set; } = 90; + public FileServiceDiscoveryProvider ServiceDiscoveryProvider { get; set; } public FileRateLimitOptions RateLimitOptions { get; set; } diff --git a/src/Ocelot/Requester/MessageInvokerPool.cs b/src/Ocelot/Requester/MessageInvokerPool.cs index 130b4bccb..1f237b54e 100644 --- a/src/Ocelot/Requester/MessageInvokerPool.cs +++ b/src/Ocelot/Requester/MessageInvokerPool.cs @@ -1,4 +1,6 @@ -using Ocelot.Configuration; +using Microsoft.Extensions.Options; +using Ocelot.Configuration; +using Ocelot.Configuration.File; using Ocelot.Logging; using System.Net.Security; @@ -10,13 +12,18 @@ public class MessageInvokerPool : IMessageInvokerPool private readonly IDelegatingHandlerHandlerFactory _handlerFactory; private readonly IOcelotLogger _logger; - public MessageInvokerPool(IDelegatingHandlerHandlerFactory handlerFactory, IOcelotLoggerFactory loggerFactory) + public MessageInvokerPool( + IDelegatingHandlerHandlerFactory handlerFactory, + IOcelotLoggerFactory loggerFactory, + IOptions options) { _handlerFactory = handlerFactory ?? throw new ArgumentNullException(nameof(handlerFactory)); _handlersPool = new ConcurrentDictionary>(); ArgumentNullException.ThrowIfNull(loggerFactory); _logger = loggerFactory.CreateLogger(); + + _defaultRequestTimeoutSeconds = options.Value.GlobalConfiguration.RequestTimeoutSeconds; } public HttpMessageInvoker Get(DownstreamRoute downstreamRoute) @@ -31,17 +38,15 @@ public HttpMessageInvoker Get(DownstreamRoute downstreamRoute) } public void Clear() => _handlersPool.Clear(); - - /// - /// TODO This should be configurable and available as global config parameter in ocelot.json. - /// - public const int DefaultRequestTimeoutSeconds = 90; + + private readonly int _defaultRequestTimeoutSeconds; + private int _requestTimeoutSeconds; public int RequestTimeoutSeconds { - get => _requestTimeoutSeconds > 0 ? _requestTimeoutSeconds : DefaultRequestTimeoutSeconds; - set => _requestTimeoutSeconds = value > 0 ? value : DefaultRequestTimeoutSeconds; + get => _requestTimeoutSeconds > 0 ? _requestTimeoutSeconds : _defaultRequestTimeoutSeconds; + set => _requestTimeoutSeconds = value > 0 ? value : _defaultRequestTimeoutSeconds; } private HttpMessageInvoker CreateMessageInvoker(DownstreamRoute downstreamRoute) From e4769bed89bb50e8f47fcdb1bf3420cff717258a Mon Sep 17 00:00:00 2001 From: zhannur Date: Sat, 25 May 2024 21:37:29 +0500 Subject: [PATCH 02/50] Fix Test --- test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs b/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs index 81d7005c4..b07138ea3 100644 --- a/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs +++ b/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs @@ -1,6 +1,7 @@ using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.Options; using Ocelot.Configuration; using Ocelot.Configuration.Builder; using Ocelot.Configuration.File; @@ -27,12 +28,15 @@ public class MessageInvokerPoolTests : UnitTest private HttpContext _context; private HttpResponseMessage _response; private IWebHost _host; + private IOptions _fileConfiguration; public MessageInvokerPoolTests() { _ocelotLoggerFactory = new Mock(); _ocelotLogger = new Mock(); _ocelotLoggerFactory.Setup(x => x.CreateLogger()).Returns(_ocelotLogger.Object); + _fileConfiguration = new OptionsWrapper( + new FileConfiguration { GlobalConfiguration = new FileGlobalConfiguration() }); } [Fact] @@ -231,7 +235,7 @@ private void GivenTwoDifferentDownstreamRoutes(string path1, string path2) private void AndAHandlerFactory() => _handlerFactory = GetHandlerFactory(); private void GivenAMessageInvokerPool() => - _pool = new MessageInvokerPool(_handlerFactory.Object, _ocelotLoggerFactory.Object); + _pool = new MessageInvokerPool(_handlerFactory.Object, _ocelotLoggerFactory.Object, _fileConfiguration); private void WhenGettingMessageInvokerTwice() { From 972d88c78982a6d3a864e8fa73ea4cd1c219d1ce Mon Sep 17 00:00:00 2001 From: zhannur Date: Sun, 26 May 2024 20:18:24 +0500 Subject: [PATCH 03/50] Fix Remarks --- .../Configuration/File/FileGlobalConfiguration.cs | 2 +- src/Ocelot/Requester/MessageInvokerPool.cs | 10 +++++----- .../Requester/MessageInvokerPoolTests.cs | 8 +++----- 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/src/Ocelot/Configuration/File/FileGlobalConfiguration.cs b/src/Ocelot/Configuration/File/FileGlobalConfiguration.cs index ffa175193..18bc89a84 100644 --- a/src/Ocelot/Configuration/File/FileGlobalConfiguration.cs +++ b/src/Ocelot/Configuration/File/FileGlobalConfiguration.cs @@ -17,7 +17,7 @@ public FileGlobalConfiguration() public string RequestIdKey { get; set; } - public int RequestTimeoutSeconds { get; set; } = 90; + public int? RequestTimeoutSeconds { get; set; } public FileServiceDiscoveryProvider ServiceDiscoveryProvider { get; set; } diff --git a/src/Ocelot/Requester/MessageInvokerPool.cs b/src/Ocelot/Requester/MessageInvokerPool.cs index 1f237b54e..a97084b7d 100644 --- a/src/Ocelot/Requester/MessageInvokerPool.cs +++ b/src/Ocelot/Requester/MessageInvokerPool.cs @@ -15,7 +15,7 @@ public class MessageInvokerPool : IMessageInvokerPool public MessageInvokerPool( IDelegatingHandlerHandlerFactory handlerFactory, IOcelotLoggerFactory loggerFactory, - IOptions options) + FileGlobalConfiguration fileGlobalConfigure) { _handlerFactory = handlerFactory ?? throw new ArgumentNullException(nameof(handlerFactory)); _handlersPool = new ConcurrentDictionary>(); @@ -23,7 +23,7 @@ public MessageInvokerPool( ArgumentNullException.ThrowIfNull(loggerFactory); _logger = loggerFactory.CreateLogger(); - _defaultRequestTimeoutSeconds = options.Value.GlobalConfiguration.RequestTimeoutSeconds; + RequestTimeoutSeconds = fileGlobalConfigure.RequestTimeoutSeconds ?? 0; } public HttpMessageInvoker Get(DownstreamRoute downstreamRoute) @@ -39,14 +39,14 @@ public HttpMessageInvoker Get(DownstreamRoute downstreamRoute) public void Clear() => _handlersPool.Clear(); - private readonly int _defaultRequestTimeoutSeconds; + public const int DefaultRequestTimeoutSeconds = 90; private int _requestTimeoutSeconds; public int RequestTimeoutSeconds { - get => _requestTimeoutSeconds > 0 ? _requestTimeoutSeconds : _defaultRequestTimeoutSeconds; - set => _requestTimeoutSeconds = value > 0 ? value : _defaultRequestTimeoutSeconds; + get => _requestTimeoutSeconds > 0 ? _requestTimeoutSeconds : DefaultRequestTimeoutSeconds; + set => _requestTimeoutSeconds = value > 0 ? value : DefaultRequestTimeoutSeconds; } private HttpMessageInvoker CreateMessageInvoker(DownstreamRoute downstreamRoute) diff --git a/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs b/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs index b07138ea3..185daf8d3 100644 --- a/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs +++ b/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs @@ -1,7 +1,6 @@ using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; -using Microsoft.Extensions.Options; using Ocelot.Configuration; using Ocelot.Configuration.Builder; using Ocelot.Configuration.File; @@ -28,15 +27,14 @@ public class MessageInvokerPoolTests : UnitTest private HttpContext _context; private HttpResponseMessage _response; private IWebHost _host; - private IOptions _fileConfiguration; + private FileGlobalConfiguration _fileGlobalConfiguration; public MessageInvokerPoolTests() { _ocelotLoggerFactory = new Mock(); _ocelotLogger = new Mock(); _ocelotLoggerFactory.Setup(x => x.CreateLogger()).Returns(_ocelotLogger.Object); - _fileConfiguration = new OptionsWrapper( - new FileConfiguration { GlobalConfiguration = new FileGlobalConfiguration() }); + _fileGlobalConfiguration = new FileGlobalConfiguration(); } [Fact] @@ -235,7 +233,7 @@ private void GivenTwoDifferentDownstreamRoutes(string path1, string path2) private void AndAHandlerFactory() => _handlerFactory = GetHandlerFactory(); private void GivenAMessageInvokerPool() => - _pool = new MessageInvokerPool(_handlerFactory.Object, _ocelotLoggerFactory.Object, _fileConfiguration); + _pool = new MessageInvokerPool(_handlerFactory.Object, _ocelotLoggerFactory.Object, _fileGlobalConfiguration); private void WhenGettingMessageInvokerTwice() { From 03edc49fece62114fb1ff0eb852ae94a97011f5e Mon Sep 17 00:00:00 2001 From: zhannur Date: Mon, 27 May 2024 16:36:25 +0500 Subject: [PATCH 04/50] Add DI FileGlobalConfiguration. PollyQoSProvider, PollyQoSResiliencePipelineProvider add DefaultRequestTimeoutSeconds --- .../PollyQoSResiliencePipelineProvider.cs | 1 + src/Ocelot/DependencyInjection/OcelotBuilder.cs | 6 +++++- src/Ocelot/Requester/MessageInvokerPool.cs | 3 +-- .../Polly/PollyQoSResiliencePipelineProviderTests.cs | 1 + 4 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/Ocelot.Provider.Polly/PollyQoSResiliencePipelineProvider.cs b/src/Ocelot.Provider.Polly/PollyQoSResiliencePipelineProvider.cs index 6ec9b5d53..fedb32516 100644 --- a/src/Ocelot.Provider.Polly/PollyQoSResiliencePipelineProvider.cs +++ b/src/Ocelot.Provider.Polly/PollyQoSResiliencePipelineProvider.cs @@ -1,4 +1,5 @@ using Ocelot.Configuration; +using Ocelot.Configuration.File; using Ocelot.Logging; using Ocelot.Provider.Polly.Interfaces; using Polly.CircuitBreaker; diff --git a/src/Ocelot/DependencyInjection/OcelotBuilder.cs b/src/Ocelot/DependencyInjection/OcelotBuilder.cs index 642697744..e1a1eed4f 100644 --- a/src/Ocelot/DependencyInjection/OcelotBuilder.cs +++ b/src/Ocelot/DependencyInjection/OcelotBuilder.cs @@ -50,7 +50,11 @@ public OcelotBuilder(IServiceCollection services, IConfiguration configurationRo Configuration = configurationRoot; Services = services; Services.Configure(configurationRoot); - + Services.TryAddSingleton( + sp => sp + .GetRequiredService>() + .Value.GlobalConfiguration); + Services.TryAddSingleton(); Services.TryAddSingleton(); Services.TryAddSingleton(); diff --git a/src/Ocelot/Requester/MessageInvokerPool.cs b/src/Ocelot/Requester/MessageInvokerPool.cs index a97084b7d..2dc152bd6 100644 --- a/src/Ocelot/Requester/MessageInvokerPool.cs +++ b/src/Ocelot/Requester/MessageInvokerPool.cs @@ -1,5 +1,4 @@ -using Microsoft.Extensions.Options; -using Ocelot.Configuration; +using Ocelot.Configuration; using Ocelot.Configuration.File; using Ocelot.Logging; using System.Net.Security; diff --git a/test/Ocelot.UnitTests/Polly/PollyQoSResiliencePipelineProviderTests.cs b/test/Ocelot.UnitTests/Polly/PollyQoSResiliencePipelineProviderTests.cs index 6a80d2bea..6c7b56b7c 100644 --- a/test/Ocelot.UnitTests/Polly/PollyQoSResiliencePipelineProviderTests.cs +++ b/test/Ocelot.UnitTests/Polly/PollyQoSResiliencePipelineProviderTests.cs @@ -1,5 +1,6 @@ using Ocelot.Configuration; using Ocelot.Configuration.Builder; +using Ocelot.Configuration.File; using Ocelot.Logging; using Ocelot.Provider.Polly; using Polly; From c85d6a272fdd34e4279075175fcc782643c2e815 Mon Sep 17 00:00:00 2001 From: zhannur Date: Tue, 28 May 2024 21:33:08 +0500 Subject: [PATCH 05/50] Fix Remarks --- src/Ocelot/DependencyInjection/OcelotBuilder.cs | 5 +---- src/Ocelot/Requester/MessageInvokerPool.cs | 13 ++++++++----- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/Ocelot/DependencyInjection/OcelotBuilder.cs b/src/Ocelot/DependencyInjection/OcelotBuilder.cs index e1a1eed4f..9150269b0 100644 --- a/src/Ocelot/DependencyInjection/OcelotBuilder.cs +++ b/src/Ocelot/DependencyInjection/OcelotBuilder.cs @@ -50,10 +50,7 @@ public OcelotBuilder(IServiceCollection services, IConfiguration configurationRo Configuration = configurationRoot; Services = services; Services.Configure(configurationRoot); - Services.TryAddSingleton( - sp => sp - .GetRequiredService>() - .Value.GlobalConfiguration); + Services.Configure(configurationRoot.GetSection("GlobalConfiguration")); Services.TryAddSingleton(); Services.TryAddSingleton(); diff --git a/src/Ocelot/Requester/MessageInvokerPool.cs b/src/Ocelot/Requester/MessageInvokerPool.cs index 2dc152bd6..c02839ed3 100644 --- a/src/Ocelot/Requester/MessageInvokerPool.cs +++ b/src/Ocelot/Requester/MessageInvokerPool.cs @@ -1,4 +1,5 @@ -using Ocelot.Configuration; +using Microsoft.Extensions.Options; +using Ocelot.Configuration; using Ocelot.Configuration.File; using Ocelot.Logging; using System.Net.Security; @@ -10,11 +11,12 @@ public class MessageInvokerPool : IMessageInvokerPool private readonly ConcurrentDictionary> _handlersPool; private readonly IDelegatingHandlerHandlerFactory _handlerFactory; private readonly IOcelotLogger _logger; + private readonly FileGlobalConfiguration _global; public MessageInvokerPool( IDelegatingHandlerHandlerFactory handlerFactory, IOcelotLoggerFactory loggerFactory, - FileGlobalConfiguration fileGlobalConfigure) + IOptions global) { _handlerFactory = handlerFactory ?? throw new ArgumentNullException(nameof(handlerFactory)); _handlersPool = new ConcurrentDictionary>(); @@ -22,7 +24,8 @@ public MessageInvokerPool( ArgumentNullException.ThrowIfNull(loggerFactory); _logger = loggerFactory.CreateLogger(); - RequestTimeoutSeconds = fileGlobalConfigure.RequestTimeoutSeconds ?? 0; + _global = global.Value; + _requestTimeoutSeconds = _global.RequestTimeoutSeconds; } public HttpMessageInvoker Get(DownstreamRoute downstreamRoute) @@ -40,11 +43,11 @@ public HttpMessageInvoker Get(DownstreamRoute downstreamRoute) public const int DefaultRequestTimeoutSeconds = 90; - private int _requestTimeoutSeconds; + private int? _requestTimeoutSeconds; public int RequestTimeoutSeconds { - get => _requestTimeoutSeconds > 0 ? _requestTimeoutSeconds : DefaultRequestTimeoutSeconds; + get => _requestTimeoutSeconds ?? _global?.RequestTimeoutSeconds ?? DefaultRequestTimeoutSeconds; set => _requestTimeoutSeconds = value > 0 ? value : DefaultRequestTimeoutSeconds; } From 697f8b982e31d97810f3872b033c6df5b9cbe8bf Mon Sep 17 00:00:00 2001 From: zhannur Date: Tue, 28 May 2024 21:44:00 +0500 Subject: [PATCH 06/50] Fix remarks --- .../PollyQoSResiliencePipelineProvider.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Ocelot.Provider.Polly/PollyQoSResiliencePipelineProvider.cs b/src/Ocelot.Provider.Polly/PollyQoSResiliencePipelineProvider.cs index fedb32516..62edbb511 100644 --- a/src/Ocelot.Provider.Polly/PollyQoSResiliencePipelineProvider.cs +++ b/src/Ocelot.Provider.Polly/PollyQoSResiliencePipelineProvider.cs @@ -1,3 +1,4 @@ +using Microsoft.Extensions.Options; using Ocelot.Configuration; using Ocelot.Configuration.File; using Ocelot.Logging; @@ -19,7 +20,8 @@ public class PollyQoSResiliencePipelineProvider : IPollyQoSResiliencePipelinePro public PollyQoSResiliencePipelineProvider( IOcelotLoggerFactory loggerFactory, - ResiliencePipelineRegistry registry) + ResiliencePipelineRegistry registry, + IOptions global)) { _logger = loggerFactory.CreateLogger(); _registry = registry; From 295b4f27c1d7efbb254fa202f30401c87f902a53 Mon Sep 17 00:00:00 2001 From: zhannur Date: Wed, 29 May 2024 10:05:08 +0500 Subject: [PATCH 07/50] Add DownStream Timeout --- .../Builder/DownstreamRouteBuilder.cs | 10 ++++++- .../Configuration/Creator/ITimeoutCreator.cs | 9 ++++++ .../Configuration/Creator/RoutesCreator.cs | 28 +++++++++++-------- .../Configuration/Creator/TimeoutCreator.cs | 14 ++++++++++ src/Ocelot/Configuration/DownstreamRoute.cs | 7 +++-- .../DependencyInjection/OcelotBuilder.cs | 1 + 6 files changed, 55 insertions(+), 14 deletions(-) create mode 100644 src/Ocelot/Configuration/Creator/ITimeoutCreator.cs create mode 100644 src/Ocelot/Configuration/Creator/TimeoutCreator.cs diff --git a/src/Ocelot/Configuration/Builder/DownstreamRouteBuilder.cs b/src/Ocelot/Configuration/Builder/DownstreamRouteBuilder.cs index 2f01cd429..06c1260cf 100644 --- a/src/Ocelot/Configuration/Builder/DownstreamRouteBuilder.cs +++ b/src/Ocelot/Configuration/Builder/DownstreamRouteBuilder.cs @@ -43,6 +43,7 @@ public class DownstreamRouteBuilder private HttpVersionPolicy _downstreamHttpVersionPolicy; private Dictionary _upstreamHeaders; private MetadataOptions _metadataOptions; + private int _timeout; public DownstreamRouteBuilder() { @@ -282,6 +283,12 @@ public DownstreamRouteBuilder WithMetadata(MetadataOptions metadataOptions) return this; } + public DownstreamRouteBuilder WithTimeout(int timeout) + { + _timeout = timeout; + return this; + } + public DownstreamRoute Build() { return new DownstreamRoute( @@ -321,6 +328,7 @@ public DownstreamRoute Build() _downstreamHttpVersion, _downstreamHttpVersionPolicy, _upstreamHeaders, - _metadataOptions); + _metadataOptions, + _timeout); } } diff --git a/src/Ocelot/Configuration/Creator/ITimeoutCreator.cs b/src/Ocelot/Configuration/Creator/ITimeoutCreator.cs new file mode 100644 index 000000000..d7bbc24f2 --- /dev/null +++ b/src/Ocelot/Configuration/Creator/ITimeoutCreator.cs @@ -0,0 +1,9 @@ +using Ocelot.Configuration.File; + +namespace Ocelot.Configuration.Creator +{ + public interface ITimeoutCreator + { + int Create(FileRoute fileRoute, FileGlobalConfiguration globalConfiguration); + } +} diff --git a/src/Ocelot/Configuration/Creator/RoutesCreator.cs b/src/Ocelot/Configuration/Creator/RoutesCreator.cs index 015944014..673c1fe27 100644 --- a/src/Ocelot/Configuration/Creator/RoutesCreator.cs +++ b/src/Ocelot/Configuration/Creator/RoutesCreator.cs @@ -1,4 +1,4 @@ -using Ocelot.Configuration.Builder; +using Ocelot.Configuration.Builder; using Ocelot.Configuration.File; namespace Ocelot.Configuration.Creator @@ -21,8 +21,9 @@ public class RoutesCreator : IRoutesCreator private readonly IRouteKeyCreator _routeKeyCreator; private readonly ISecurityOptionsCreator _securityOptionsCreator; private readonly IVersionCreator _versionCreator; - private readonly IVersionPolicyCreator _versionPolicyCreator; - private readonly IMetadataCreator _metadataCreator; + private readonly IVersionPolicyCreator _versionPolicyCreator; + private readonly IMetadataCreator _metadataCreator; + private readonly ITimeoutCreator _timeoutCreator; public RoutesCreator( IClaimsToThingCreator claimsToThingCreator, @@ -40,9 +41,10 @@ public RoutesCreator( IRouteKeyCreator routeKeyCreator, ISecurityOptionsCreator securityOptionsCreator, IVersionCreator versionCreator, - IVersionPolicyCreator versionPolicyCreator, + IVersionPolicyCreator versionPolicyCreator, IUpstreamHeaderTemplatePatternCreator upstreamHeaderTemplatePatternCreator, - IMetadataCreator metadataCreator) + IMetadataCreator metadataCreator, + ITimeoutCreator timeoutCreator) { _routeKeyCreator = routeKeyCreator; _loadBalancerOptionsCreator = loadBalancerOptionsCreator; @@ -60,9 +62,10 @@ public RoutesCreator( _loadBalancerOptionsCreator = loadBalancerOptionsCreator; _securityOptionsCreator = securityOptionsCreator; _versionCreator = versionCreator; - _versionPolicyCreator = versionPolicyCreator; + _versionPolicyCreator = versionPolicyCreator; _upstreamHeaderTemplatePatternCreator = upstreamHeaderTemplatePatternCreator; - _metadataCreator = metadataCreator; + _metadataCreator = metadataCreator; + _timeoutCreator = timeoutCreator; } public List Create(FileConfiguration fileConfiguration) @@ -112,11 +115,13 @@ private DownstreamRoute SetUpDownstreamRoute(FileRoute fileRoute, FileGlobalConf var downstreamHttpVersion = _versionCreator.Create(fileRoute.DownstreamHttpVersion); - var downstreamHttpVersionPolicy = _versionPolicyCreator.Create(fileRoute.DownstreamHttpVersionPolicy); + var downstreamHttpVersionPolicy = _versionPolicyCreator.Create(fileRoute.DownstreamHttpVersionPolicy); var cacheOptions = _cacheOptionsCreator.Create(fileRoute.FileCacheOptions, globalConfiguration, fileRoute.UpstreamPathTemplate, fileRoute.UpstreamHttpMethod); - var metadata = _metadataCreator.Create(fileRoute.Metadata, globalConfiguration); + var metadata = _metadataCreator.Create(fileRoute.Metadata, globalConfiguration); + + var timeout = _timeoutCreator.Create(fileRoute, globalConfiguration); var route = new DownstreamRouteBuilder() .WithKey(fileRoute.Key) @@ -153,9 +158,10 @@ private DownstreamRoute SetUpDownstreamRoute(FileRoute fileRoute, FileGlobalConf .WithDangerousAcceptAnyServerCertificateValidator(fileRoute.DangerousAcceptAnyServerCertificateValidator) .WithSecurityOptions(securityOptions) .WithDownstreamHttpVersion(downstreamHttpVersion) - .WithDownstreamHttpVersionPolicy(downstreamHttpVersionPolicy) + .WithDownstreamHttpVersionPolicy(downstreamHttpVersionPolicy) .WithDownStreamHttpMethod(fileRoute.DownstreamHttpMethod) - .WithMetadata(metadata) + .WithMetadata(metadata) + .WithTimeout(timeout) .Build(); return route; diff --git a/src/Ocelot/Configuration/Creator/TimeoutCreator.cs b/src/Ocelot/Configuration/Creator/TimeoutCreator.cs new file mode 100644 index 000000000..24a33a027 --- /dev/null +++ b/src/Ocelot/Configuration/Creator/TimeoutCreator.cs @@ -0,0 +1,14 @@ +using Ocelot.Configuration.File; + +namespace Ocelot.Configuration.Creator +{ + public class TimeoutCreator : ITimeoutCreator + { + public int Create(FileRoute fileRoute, FileGlobalConfiguration globalConfiguration) + { + return fileRoute.Timeout > 0 + ? fileRoute.Timeout + : globalConfiguration.RequestTimeoutSeconds ?? 0; + } + } +} diff --git a/src/Ocelot/Configuration/DownstreamRoute.cs b/src/Ocelot/Configuration/DownstreamRoute.cs index 818390a02..45d59fe9f 100644 --- a/src/Ocelot/Configuration/DownstreamRoute.cs +++ b/src/Ocelot/Configuration/DownstreamRoute.cs @@ -42,7 +42,8 @@ public DownstreamRoute( Version downstreamHttpVersion, HttpVersionPolicy downstreamHttpVersionPolicy, Dictionary upstreamHeaders, - MetadataOptions metadataOptions) + MetadataOptions metadataOptions, + int timeout) { DangerousAcceptAnyServerCertificateValidator = dangerousAcceptAnyServerCertificateValidator; AddHeadersToDownstream = addHeadersToDownstream; @@ -81,6 +82,7 @@ public DownstreamRoute( DownstreamHttpVersionPolicy = downstreamHttpVersionPolicy; UpstreamHeaders = upstreamHeaders ?? new(); MetadataOptions = metadataOptions; + Timeout = timeout; } public string Key { get; } @@ -131,11 +133,12 @@ public DownstreamRoute( public Dictionary UpstreamHeaders { get; } public bool UseServiceDiscovery { get; } public MetadataOptions MetadataOptions { get; } + public int Timeout { get; } /// Gets the route name depending on whether the service discovery mode is enabled or disabled. /// A object with the name. public string Name() => string.IsNullOrEmpty(ServiceName) && !UseServiceDiscovery ? UpstreamPathTemplate?.Template ?? DownstreamPathTemplate?.Value ?? "?" - : string.Join(':', ServiceNamespace, ServiceName, UpstreamPathTemplate?.Template); + : string.Join(':', ServiceNamespace, ServiceName, UpstreamPathTemplate?.Template); } } diff --git a/src/Ocelot/DependencyInjection/OcelotBuilder.cs b/src/Ocelot/DependencyInjection/OcelotBuilder.cs index 9150269b0..4ae416cf9 100644 --- a/src/Ocelot/DependencyInjection/OcelotBuilder.cs +++ b/src/Ocelot/DependencyInjection/OcelotBuilder.cs @@ -72,6 +72,7 @@ public OcelotBuilder(IServiceCollection services, IConfiguration configurationRo Services.TryAddSingleton(); Services.TryAddSingleton(); Services.TryAddSingleton(); + Services.TryAddSingleton(); Services.TryAddSingleton(); Services.TryAddSingleton(); Services.TryAddSingleton(); From 4d28961654222dc8b230b18a97551e413923d49c Mon Sep 17 00:00:00 2001 From: zhannur Date: Wed, 29 May 2024 10:48:27 +0500 Subject: [PATCH 08/50] Use One Timeout --- .../PollyQoSResiliencePipelineProvider.cs | 2 - .../Configuration/Creator/TimeoutCreator.cs | 2 +- .../DependencyInjection/OcelotBuilder.cs | 1 - src/Ocelot/Requester/MessageInvokerPool.cs | 23 ++++---- .../DownstreamRouteExtensionsTests.cs | 3 +- .../Configuration/RoutesCreatorTests.cs | 53 ++++++++++--------- .../Requester/MessageInvokerPoolTests.cs | 4 +- 7 files changed, 41 insertions(+), 47 deletions(-) diff --git a/src/Ocelot.Provider.Polly/PollyQoSResiliencePipelineProvider.cs b/src/Ocelot.Provider.Polly/PollyQoSResiliencePipelineProvider.cs index 62edbb511..93c153a0a 100644 --- a/src/Ocelot.Provider.Polly/PollyQoSResiliencePipelineProvider.cs +++ b/src/Ocelot.Provider.Polly/PollyQoSResiliencePipelineProvider.cs @@ -1,6 +1,4 @@ -using Microsoft.Extensions.Options; using Ocelot.Configuration; -using Ocelot.Configuration.File; using Ocelot.Logging; using Ocelot.Provider.Polly.Interfaces; using Polly.CircuitBreaker; diff --git a/src/Ocelot/Configuration/Creator/TimeoutCreator.cs b/src/Ocelot/Configuration/Creator/TimeoutCreator.cs index 24a33a027..f3e5e6d77 100644 --- a/src/Ocelot/Configuration/Creator/TimeoutCreator.cs +++ b/src/Ocelot/Configuration/Creator/TimeoutCreator.cs @@ -8,7 +8,7 @@ public int Create(FileRoute fileRoute, FileGlobalConfiguration globalConfigurati { return fileRoute.Timeout > 0 ? fileRoute.Timeout - : globalConfiguration.RequestTimeoutSeconds ?? 0; + : (globalConfiguration.RequestTimeoutSeconds ?? 0) * 1000; } } } diff --git a/src/Ocelot/DependencyInjection/OcelotBuilder.cs b/src/Ocelot/DependencyInjection/OcelotBuilder.cs index 4ae416cf9..dccd7a9c3 100644 --- a/src/Ocelot/DependencyInjection/OcelotBuilder.cs +++ b/src/Ocelot/DependencyInjection/OcelotBuilder.cs @@ -50,7 +50,6 @@ public OcelotBuilder(IServiceCollection services, IConfiguration configurationRo Configuration = configurationRoot; Services = services; Services.Configure(configurationRoot); - Services.Configure(configurationRoot.GetSection("GlobalConfiguration")); Services.TryAddSingleton(); Services.TryAddSingleton(); diff --git a/src/Ocelot/Requester/MessageInvokerPool.cs b/src/Ocelot/Requester/MessageInvokerPool.cs index c02839ed3..beb6162a7 100644 --- a/src/Ocelot/Requester/MessageInvokerPool.cs +++ b/src/Ocelot/Requester/MessageInvokerPool.cs @@ -1,6 +1,4 @@ -using Microsoft.Extensions.Options; -using Ocelot.Configuration; -using Ocelot.Configuration.File; +using Ocelot.Configuration; using Ocelot.Logging; using System.Net.Security; @@ -11,21 +9,16 @@ public class MessageInvokerPool : IMessageInvokerPool private readonly ConcurrentDictionary> _handlersPool; private readonly IDelegatingHandlerHandlerFactory _handlerFactory; private readonly IOcelotLogger _logger; - private readonly FileGlobalConfiguration _global; public MessageInvokerPool( IDelegatingHandlerHandlerFactory handlerFactory, - IOcelotLoggerFactory loggerFactory, - IOptions global) + IOcelotLoggerFactory loggerFactory) { _handlerFactory = handlerFactory ?? throw new ArgumentNullException(nameof(handlerFactory)); _handlersPool = new ConcurrentDictionary>(); ArgumentNullException.ThrowIfNull(loggerFactory); _logger = loggerFactory.CreateLogger(); - - _global = global.Value; - _requestTimeoutSeconds = _global.RequestTimeoutSeconds; } public HttpMessageInvoker Get(DownstreamRoute downstreamRoute) @@ -43,11 +36,11 @@ public HttpMessageInvoker Get(DownstreamRoute downstreamRoute) public const int DefaultRequestTimeoutSeconds = 90; - private int? _requestTimeoutSeconds; + private int _requestTimeoutSeconds; public int RequestTimeoutSeconds { - get => _requestTimeoutSeconds ?? _global?.RequestTimeoutSeconds ?? DefaultRequestTimeoutSeconds; + get => _requestTimeoutSeconds > 0 ? _requestTimeoutSeconds : DefaultRequestTimeoutSeconds; set => _requestTimeoutSeconds = value > 0 ? value : DefaultRequestTimeoutSeconds; } @@ -65,9 +58,11 @@ private HttpMessageInvoker CreateMessageInvoker(DownstreamRoute downstreamRoute) // Adding timeout handler to the top of the chain. // It's standard behavior to throw TimeoutException after the defined timeout (90 seconds by default) - var timeoutHandler = new TimeoutDelegatingHandler(downstreamRoute.QosOptions.TimeoutValue == 0 - ? TimeSpan.FromSeconds(RequestTimeoutSeconds) - : TimeSpan.FromMilliseconds(downstreamRoute.QosOptions.TimeoutValue)) + var timeoutHandler = new TimeoutDelegatingHandler(downstreamRoute.QosOptions.TimeoutValue > 0 + ? TimeSpan.FromMilliseconds(downstreamRoute.QosOptions.TimeoutValue) + : downstreamRoute.Timeout > 0 + ? TimeSpan.FromMilliseconds(downstreamRoute.Timeout) + : TimeSpan.FromSeconds(RequestTimeoutSeconds)) { InnerHandler = baseHandler, }; diff --git a/test/Ocelot.UnitTests/Configuration/DownstreamRouteExtensionsTests.cs b/test/Ocelot.UnitTests/Configuration/DownstreamRouteExtensionsTests.cs index 0c3b3bcc3..1163bcfb6 100644 --- a/test/Ocelot.UnitTests/Configuration/DownstreamRouteExtensionsTests.cs +++ b/test/Ocelot.UnitTests/Configuration/DownstreamRouteExtensionsTests.cs @@ -51,7 +51,8 @@ public DownstreamRouteExtensionsTests() new Version(), HttpVersionPolicy.RequestVersionExact, new(), - new MetadataOptions(new FileMetadataOptions())); + new MetadataOptions(new FileMetadataOptions()), + 0); } [Theory] diff --git a/test/Ocelot.UnitTests/Configuration/RoutesCreatorTests.cs b/test/Ocelot.UnitTests/Configuration/RoutesCreatorTests.cs index dbb9ef25a..75a870c0d 100644 --- a/test/Ocelot.UnitTests/Configuration/RoutesCreatorTests.cs +++ b/test/Ocelot.UnitTests/Configuration/RoutesCreatorTests.cs @@ -1,8 +1,8 @@ -using Ocelot.Configuration; -using Ocelot.Configuration.Builder; -using Ocelot.Configuration.Creator; -using Ocelot.Configuration.File; -using Ocelot.Values; +using Ocelot.Configuration; +using Ocelot.Configuration.Builder; +using Ocelot.Configuration.Creator; +using Ocelot.Configuration.File; +using Ocelot.Values; namespace Ocelot.UnitTests.Configuration { @@ -25,8 +25,9 @@ public class RoutesCreatorTests : UnitTest private readonly Mock _rrkCreator; private readonly Mock _soCreator; private readonly Mock _versionCreator; - private readonly Mock _versionPolicyCreator; - private readonly Mock _metadataCreator; + private readonly Mock _versionPolicyCreator; + private readonly Mock _metadataCreator; + private readonly Mock _timeoutCreator; private FileConfiguration _fileConfig; private RouteOptions _rro; private string _requestId; @@ -42,8 +43,8 @@ public class RoutesCreatorTests : UnitTest private List _dhp; private LoadBalancerOptions _lbo; private List _result; - private Version _expectedVersion; - private HttpVersionPolicy _expectedVersionPolicy; + private Version _expectedVersion; + private HttpVersionPolicy _expectedVersionPolicy; private Dictionary _uht; private Dictionary _expectedMetadata; @@ -64,9 +65,10 @@ public RoutesCreatorTests() _rrkCreator = new Mock(); _soCreator = new Mock(); _versionCreator = new Mock(); - _versionPolicyCreator = new Mock(); + _versionPolicyCreator = new Mock(); _uhtpCreator = new Mock(); - _metadataCreator = new Mock(); + _metadataCreator = new Mock(); + _timeoutCreator = new Mock(); _creator = new RoutesCreator( _cthCreator.Object, @@ -83,10 +85,11 @@ public RoutesCreatorTests() _lboCreator.Object, _rrkCreator.Object, _soCreator.Object, - _versionCreator.Object, + _versionCreator.Object, _versionPolicyCreator.Object, _uhtpCreator.Object, - _metadataCreator.Object); + _metadataCreator.Object, + _timeoutCreator.Object); } [Fact] @@ -123,7 +126,7 @@ public void Should_return_routes() { { "e","f" }, }, - UpstreamHttpMethod = new List { "GET", "POST" }, + UpstreamHttpMethod = new List { "GET", "POST" }, Metadata = new Dictionary { ["foo"] = "bar", @@ -149,7 +152,7 @@ public void Should_return_routes() Metadata = new Dictionary { ["foo"] = "baz", - }, + }, }, }, }; @@ -171,7 +174,7 @@ private void ThenTheDependenciesAreCalledCorrectly() private void GivenTheDependenciesAreSetUpCorrectly() { _expectedVersion = new Version("1.1"); - _expectedVersionPolicy = HttpVersionPolicy.RequestVersionOrLower; + _expectedVersionPolicy = HttpVersionPolicy.RequestVersionOrLower; _rro = new RouteOptions(false, false, false, false, false); _requestId = "testy"; _rrk = "besty"; @@ -187,10 +190,10 @@ private void GivenTheDependenciesAreSetUpCorrectly() _dhp = new List(); _lbo = new LoadBalancerOptionsBuilder().Build(); _uht = new Dictionary(); - _expectedMetadata = new Dictionary() - { - ["foo"] = "bar", - }; + _expectedMetadata = new Dictionary() + { + ["foo"] = "bar", + }; _rroCreator.Setup(x => x.Create(It.IsAny())).Returns(_rro); _ridkCreator.Setup(x => x.Create(It.IsAny(), It.IsAny())).Returns(_requestId); @@ -206,12 +209,12 @@ private void GivenTheDependenciesAreSetUpCorrectly() _daCreator.Setup(x => x.Create(It.IsAny())).Returns(_dhp); _lboCreator.Setup(x => x.Create(It.IsAny())).Returns(_lbo); _versionCreator.Setup(x => x.Create(It.IsAny())).Returns(_expectedVersion); - _versionPolicyCreator.Setup(x => x.Create(It.IsAny())).Returns(_expectedVersionPolicy); + _versionPolicyCreator.Setup(x => x.Create(It.IsAny())).Returns(_expectedVersionPolicy); _uhtpCreator.Setup(x => x.Create(It.IsAny())).Returns(_uht); _metadataCreator.Setup(x => x.Create(It.IsAny>(), It.IsAny())).Returns(new MetadataOptions(new FileMetadataOptions { Metadata = _expectedMetadata, - })); + })); } private void ThenTheRoutesAreCreated() @@ -240,7 +243,7 @@ private void WhenICreate() private void ThenTheRouteIsSet(FileRoute expected, int routeIndex) { _result[routeIndex].DownstreamRoute[0].DownstreamHttpVersion.ShouldBe(_expectedVersion); - _result[routeIndex].DownstreamRoute[0].DownstreamHttpVersionPolicy.ShouldBe(_expectedVersionPolicy); + _result[routeIndex].DownstreamRoute[0].DownstreamHttpVersionPolicy.ShouldBe(_expectedVersionPolicy); _result[routeIndex].DownstreamRoute[0].IsAuthenticated.ShouldBe(_rro.IsAuthenticated); _result[routeIndex].DownstreamRoute[0].IsAuthorized.ShouldBe(_rro.IsAuthorized); _result[routeIndex].DownstreamRoute[0].IsCached.ShouldBe(_rro.IsCached); @@ -271,7 +274,7 @@ private void ThenTheRouteIsSet(FileRoute expected, int routeIndex) _result[routeIndex].DownstreamRoute[0].RouteClaimsRequirement.ShouldBe(expected.RouteClaimsRequirement); _result[routeIndex].DownstreamRoute[0].DownstreamPathTemplate.Value.ShouldBe(expected.DownstreamPathTemplate); _result[routeIndex].DownstreamRoute[0].Key.ShouldBe(expected.Key); - _result[routeIndex].DownstreamRoute[0].MetadataOptions.Metadata.ShouldBe(_expectedMetadata); + _result[routeIndex].DownstreamRoute[0].MetadataOptions.Metadata.ShouldBe(_expectedMetadata); _result[routeIndex].UpstreamHttpMethod .Select(x => x.Method) .ToList() @@ -303,7 +306,7 @@ private void ThenTheDepsAreCalledFor(FileRoute fileRoute, FileGlobalConfiguratio _hfarCreator.Verify(x => x.Create(fileRoute), Times.Once); _daCreator.Verify(x => x.Create(fileRoute), Times.Once); _lboCreator.Verify(x => x.Create(fileRoute.LoadBalancerOptions), Times.Once); - _soCreator.Verify(x => x.Create(fileRoute.SecurityOptions), Times.Once); + _soCreator.Verify(x => x.Create(fileRoute.SecurityOptions), Times.Once); _metadataCreator.Verify(x => x.Create(fileRoute.Metadata, globalConfig), Times.Once); } } diff --git a/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs b/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs index 185daf8d3..81d7005c4 100644 --- a/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs +++ b/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs @@ -27,14 +27,12 @@ public class MessageInvokerPoolTests : UnitTest private HttpContext _context; private HttpResponseMessage _response; private IWebHost _host; - private FileGlobalConfiguration _fileGlobalConfiguration; public MessageInvokerPoolTests() { _ocelotLoggerFactory = new Mock(); _ocelotLogger = new Mock(); _ocelotLoggerFactory.Setup(x => x.CreateLogger()).Returns(_ocelotLogger.Object); - _fileGlobalConfiguration = new FileGlobalConfiguration(); } [Fact] @@ -233,7 +231,7 @@ private void GivenTwoDifferentDownstreamRoutes(string path1, string path2) private void AndAHandlerFactory() => _handlerFactory = GetHandlerFactory(); private void GivenAMessageInvokerPool() => - _pool = new MessageInvokerPool(_handlerFactory.Object, _ocelotLoggerFactory.Object, _fileGlobalConfiguration); + _pool = new MessageInvokerPool(_handlerFactory.Object, _ocelotLoggerFactory.Object); private void WhenGettingMessageInvokerTwice() { From b164cc84a92c315979010d08492fd846a15432bc Mon Sep 17 00:00:00 2001 From: zhannur Date: Wed, 29 May 2024 10:51:02 +0500 Subject: [PATCH 09/50] Fix --- .../Polly/PollyQoSResiliencePipelineProviderTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/test/Ocelot.UnitTests/Polly/PollyQoSResiliencePipelineProviderTests.cs b/test/Ocelot.UnitTests/Polly/PollyQoSResiliencePipelineProviderTests.cs index 6c7b56b7c..6a80d2bea 100644 --- a/test/Ocelot.UnitTests/Polly/PollyQoSResiliencePipelineProviderTests.cs +++ b/test/Ocelot.UnitTests/Polly/PollyQoSResiliencePipelineProviderTests.cs @@ -1,6 +1,5 @@ using Ocelot.Configuration; using Ocelot.Configuration.Builder; -using Ocelot.Configuration.File; using Ocelot.Logging; using Ocelot.Provider.Polly; using Polly; From 2f15ba8cfcf78e7a2a155164579892147f9f6fbc Mon Sep 17 00:00:00 2001 From: zhannur Date: Wed, 29 May 2024 14:20:31 +0500 Subject: [PATCH 10/50] Fix Remarks --- src/Ocelot/Configuration/Creator/TimeoutCreator.cs | 2 +- .../Configuration/File/FileGlobalConfiguration.cs | 3 ++- src/Ocelot/Requester/MessageInvokerPool.cs | 14 +------------- 3 files changed, 4 insertions(+), 15 deletions(-) diff --git a/src/Ocelot/Configuration/Creator/TimeoutCreator.cs b/src/Ocelot/Configuration/Creator/TimeoutCreator.cs index f3e5e6d77..e0a6ddff2 100644 --- a/src/Ocelot/Configuration/Creator/TimeoutCreator.cs +++ b/src/Ocelot/Configuration/Creator/TimeoutCreator.cs @@ -8,7 +8,7 @@ public int Create(FileRoute fileRoute, FileGlobalConfiguration globalConfigurati { return fileRoute.Timeout > 0 ? fileRoute.Timeout - : (globalConfiguration.RequestTimeoutSeconds ?? 0) * 1000; + : globalConfiguration.RequestTimeoutSeconds * 1000; } } } diff --git a/src/Ocelot/Configuration/File/FileGlobalConfiguration.cs b/src/Ocelot/Configuration/File/FileGlobalConfiguration.cs index 18bc89a84..11f703a0e 100644 --- a/src/Ocelot/Configuration/File/FileGlobalConfiguration.cs +++ b/src/Ocelot/Configuration/File/FileGlobalConfiguration.cs @@ -13,11 +13,12 @@ public FileGlobalConfiguration() HttpHandlerOptions = new FileHttpHandlerOptions(); CacheOptions = new FileCacheOptions(); MetadataOptions = new FileMetadataOptions(); + RequestTimeoutSeconds = 90; } public string RequestIdKey { get; set; } - public int? RequestTimeoutSeconds { get; set; } + public int RequestTimeoutSeconds { get; set; } public FileServiceDiscoveryProvider ServiceDiscoveryProvider { get; set; } diff --git a/src/Ocelot/Requester/MessageInvokerPool.cs b/src/Ocelot/Requester/MessageInvokerPool.cs index beb6162a7..83870fc29 100644 --- a/src/Ocelot/Requester/MessageInvokerPool.cs +++ b/src/Ocelot/Requester/MessageInvokerPool.cs @@ -33,16 +33,6 @@ public HttpMessageInvoker Get(DownstreamRoute downstreamRoute) } public void Clear() => _handlersPool.Clear(); - - public const int DefaultRequestTimeoutSeconds = 90; - - private int _requestTimeoutSeconds; - - public int RequestTimeoutSeconds - { - get => _requestTimeoutSeconds > 0 ? _requestTimeoutSeconds : DefaultRequestTimeoutSeconds; - set => _requestTimeoutSeconds = value > 0 ? value : DefaultRequestTimeoutSeconds; - } private HttpMessageInvoker CreateMessageInvoker(DownstreamRoute downstreamRoute) { @@ -60,9 +50,7 @@ private HttpMessageInvoker CreateMessageInvoker(DownstreamRoute downstreamRoute) // It's standard behavior to throw TimeoutException after the defined timeout (90 seconds by default) var timeoutHandler = new TimeoutDelegatingHandler(downstreamRoute.QosOptions.TimeoutValue > 0 ? TimeSpan.FromMilliseconds(downstreamRoute.QosOptions.TimeoutValue) - : downstreamRoute.Timeout > 0 - ? TimeSpan.FromMilliseconds(downstreamRoute.Timeout) - : TimeSpan.FromSeconds(RequestTimeoutSeconds)) + : TimeSpan.FromMilliseconds(downstreamRoute.Timeout)) { InnerHandler = baseHandler, }; From 27f5f5ca93707bba0503a9f406201a6db40b0a63 Mon Sep 17 00:00:00 2001 From: zhannur Date: Thu, 30 May 2024 09:10:10 +0500 Subject: [PATCH 11/50] Nullable timeouts --- .../File/FileGlobalConfiguration.cs | 3 +- .../Configuration/File/FileQoSOptions.cs | 2 +- src/Ocelot/Configuration/File/FileRoute.cs | 174 +++++++++--------- 3 files changed, 89 insertions(+), 90 deletions(-) diff --git a/src/Ocelot/Configuration/File/FileGlobalConfiguration.cs b/src/Ocelot/Configuration/File/FileGlobalConfiguration.cs index 11f703a0e..2e0c4bcd2 100644 --- a/src/Ocelot/Configuration/File/FileGlobalConfiguration.cs +++ b/src/Ocelot/Configuration/File/FileGlobalConfiguration.cs @@ -13,12 +13,11 @@ public FileGlobalConfiguration() HttpHandlerOptions = new FileHttpHandlerOptions(); CacheOptions = new FileCacheOptions(); MetadataOptions = new FileMetadataOptions(); - RequestTimeoutSeconds = 90; } public string RequestIdKey { get; set; } - public int RequestTimeoutSeconds { get; set; } + public int? TimeoutSeconds { get; set; } public FileServiceDiscoveryProvider ServiceDiscoveryProvider { get; set; } diff --git a/src/Ocelot/Configuration/File/FileQoSOptions.cs b/src/Ocelot/Configuration/File/FileQoSOptions.cs index 85282707b..cee99efd7 100644 --- a/src/Ocelot/Configuration/File/FileQoSOptions.cs +++ b/src/Ocelot/Configuration/File/FileQoSOptions.cs @@ -32,6 +32,6 @@ public FileQoSOptions(QoSOptions from) public int DurationOfBreak { get; set; } public int ExceptionsAllowedBeforeBreaking { get; set; } - public int TimeoutValue { get; set; } + public int? TimeoutValue { get; set; } } } diff --git a/src/Ocelot/Configuration/File/FileRoute.cs b/src/Ocelot/Configuration/File/FileRoute.cs index b9a3c4eda..7b8a1783c 100644 --- a/src/Ocelot/Configuration/File/FileRoute.cs +++ b/src/Ocelot/Configuration/File/FileRoute.cs @@ -1,7 +1,7 @@ using Ocelot.Configuration.Creator; namespace Ocelot.Configuration.File -{ +{ public class FileRoute : IRoute, ICloneable { public FileRoute() @@ -23,27 +23,27 @@ public FileRoute() RateLimitOptions = new FileRateLimitRule(); RouteClaimsRequirement = new Dictionary(); SecurityOptions = new FileSecurityOptions(); - UpstreamHeaderTemplates = new Dictionary(); + UpstreamHeaderTemplates = new Dictionary(); UpstreamHeaderTransform = new Dictionary(); UpstreamHttpMethod = new List(); - } - - public FileRoute(FileRoute from) - { - DeepCopy(from, this); - } - - public Dictionary AddClaimsToRequest { get; set; } - public Dictionary AddHeadersToRequest { get; set; } - public Dictionary AddQueriesToRequest { get; set; } - public FileAuthenticationOptions AuthenticationOptions { get; set; } - public Dictionary ChangeDownstreamPathTemplate { get; set; } - public bool DangerousAcceptAnyServerCertificateValidator { get; set; } - public List DelegatingHandlers { get; set; } - public Dictionary DownstreamHeaderTransform { get; set; } - public List DownstreamHostAndPorts { get; set; } - public string DownstreamHttpMethod { get; set; } - public string DownstreamHttpVersion { get; set; } + } + + public FileRoute(FileRoute from) + { + DeepCopy(from, this); + } + + public Dictionary AddClaimsToRequest { get; set; } + public Dictionary AddHeadersToRequest { get; set; } + public Dictionary AddQueriesToRequest { get; set; } + public FileAuthenticationOptions AuthenticationOptions { get; set; } + public Dictionary ChangeDownstreamPathTemplate { get; set; } + public bool DangerousAcceptAnyServerCertificateValidator { get; set; } + public List DelegatingHandlers { get; set; } + public Dictionary DownstreamHeaderTransform { get; set; } + public List DownstreamHostAndPorts { get; set; } + public string DownstreamHttpMethod { get; set; } + public string DownstreamHttpVersion { get; set; } /// The enum specifies behaviors for selecting and negotiating the HTTP version for a request. /// A value of defined constants. @@ -55,77 +55,77 @@ public FileRoute(FileRoute from) /// HttpRequestMessage.VersionPolicy Property /// /// - public string DownstreamHttpVersionPolicy { get; set; } - public string DownstreamPathTemplate { get; set; } + public string DownstreamHttpVersionPolicy { get; set; } + public string DownstreamPathTemplate { get; set; } public string DownstreamScheme { get; set; } - public FileCacheOptions FileCacheOptions { get; set; } - public FileHttpHandlerOptions HttpHandlerOptions { get; set; } - public string Key { get; set; } - public FileLoadBalancerOptions LoadBalancerOptions { get; set; } + public FileCacheOptions FileCacheOptions { get; set; } + public FileHttpHandlerOptions HttpHandlerOptions { get; set; } + public string Key { get; set; } + public FileLoadBalancerOptions LoadBalancerOptions { get; set; } public IDictionary Metadata { get; set; } - public int Priority { get; set; } - public FileQoSOptions QoSOptions { get; set; } - public FileRateLimitRule RateLimitOptions { get; set; } - public string RequestIdKey { get; set; } - public Dictionary RouteClaimsRequirement { get; set; } - public bool RouteIsCaseSensitive { get; set; } - public FileSecurityOptions SecurityOptions { get; set; } - public string ServiceName { get; set; } - public string ServiceNamespace { get; set; } - public int Timeout { get; set; } - public Dictionary UpstreamHeaderTransform { get; set; } - public string UpstreamHost { get; set; } - public List UpstreamHttpMethod { get; set; } - public string UpstreamPathTemplate { get; set; } - public IDictionary UpstreamHeaderTemplates { get; set; } - - /// - /// Clones this object by making a deep copy. - /// - /// A deeply copied object. - public object Clone() - { - var other = (FileRoute)MemberwiseClone(); - DeepCopy(this, other); - return other; - } - - public static void DeepCopy(FileRoute from, FileRoute to) - { - to.AddClaimsToRequest = new(from.AddClaimsToRequest); - to.AddHeadersToRequest = new(from.AddHeadersToRequest); - to.AddQueriesToRequest = new(from.AddQueriesToRequest); - to.AuthenticationOptions = new(from.AuthenticationOptions); - to.ChangeDownstreamPathTemplate = new(from.ChangeDownstreamPathTemplate); - to.DangerousAcceptAnyServerCertificateValidator = from.DangerousAcceptAnyServerCertificateValidator; - to.DelegatingHandlers = new(from.DelegatingHandlers); - to.DownstreamHeaderTransform = new(from.DownstreamHeaderTransform); - to.DownstreamHostAndPorts = from.DownstreamHostAndPorts.Select(x => new FileHostAndPort(x)).ToList(); - to.DownstreamHttpMethod = from.DownstreamHttpMethod; - to.DownstreamHttpVersion = from.DownstreamHttpVersion; + public int Priority { get; set; } + public FileQoSOptions QoSOptions { get; set; } + public FileRateLimitRule RateLimitOptions { get; set; } + public string RequestIdKey { get; set; } + public Dictionary RouteClaimsRequirement { get; set; } + public bool RouteIsCaseSensitive { get; set; } + public FileSecurityOptions SecurityOptions { get; set; } + public string ServiceName { get; set; } + public string ServiceNamespace { get; set; } + public int? Timeout { get; set; } + public Dictionary UpstreamHeaderTransform { get; set; } + public string UpstreamHost { get; set; } + public List UpstreamHttpMethod { get; set; } + public string UpstreamPathTemplate { get; set; } + public IDictionary UpstreamHeaderTemplates { get; set; } + + /// + /// Clones this object by making a deep copy. + /// + /// A deeply copied object. + public object Clone() + { + var other = (FileRoute)MemberwiseClone(); + DeepCopy(this, other); + return other; + } + + public static void DeepCopy(FileRoute from, FileRoute to) + { + to.AddClaimsToRequest = new(from.AddClaimsToRequest); + to.AddHeadersToRequest = new(from.AddHeadersToRequest); + to.AddQueriesToRequest = new(from.AddQueriesToRequest); + to.AuthenticationOptions = new(from.AuthenticationOptions); + to.ChangeDownstreamPathTemplate = new(from.ChangeDownstreamPathTemplate); + to.DangerousAcceptAnyServerCertificateValidator = from.DangerousAcceptAnyServerCertificateValidator; + to.DelegatingHandlers = new(from.DelegatingHandlers); + to.DownstreamHeaderTransform = new(from.DownstreamHeaderTransform); + to.DownstreamHostAndPorts = from.DownstreamHostAndPorts.Select(x => new FileHostAndPort(x)).ToList(); + to.DownstreamHttpMethod = from.DownstreamHttpMethod; + to.DownstreamHttpVersion = from.DownstreamHttpVersion; to.DownstreamHttpVersionPolicy = from.DownstreamHttpVersionPolicy; - to.DownstreamPathTemplate = from.DownstreamPathTemplate; + to.DownstreamPathTemplate = from.DownstreamPathTemplate; to.DownstreamScheme = from.DownstreamScheme; - to.FileCacheOptions = new(from.FileCacheOptions); - to.HttpHandlerOptions = new(from.HttpHandlerOptions); - to.Key = from.Key; - to.LoadBalancerOptions = new(from.LoadBalancerOptions); + to.FileCacheOptions = new(from.FileCacheOptions); + to.HttpHandlerOptions = new(from.HttpHandlerOptions); + to.Key = from.Key; + to.LoadBalancerOptions = new(from.LoadBalancerOptions); to.Metadata = new Dictionary(from.Metadata); - to.Priority = from.Priority; - to.QoSOptions = new(from.QoSOptions); - to.RateLimitOptions = new(from.RateLimitOptions); - to.RequestIdKey = from.RequestIdKey; - to.RouteClaimsRequirement = new(from.RouteClaimsRequirement); - to.RouteIsCaseSensitive = from.RouteIsCaseSensitive; - to.SecurityOptions = new(from.SecurityOptions); - to.ServiceName = from.ServiceName; - to.ServiceNamespace = from.ServiceNamespace; - to.Timeout = from.Timeout; - to.UpstreamHeaderTemplates = new Dictionary(from.UpstreamHeaderTemplates); - to.UpstreamHeaderTransform = new(from.UpstreamHeaderTransform); - to.UpstreamHost = from.UpstreamHost; - to.UpstreamHttpMethod = new(from.UpstreamHttpMethod); - to.UpstreamPathTemplate = from.UpstreamPathTemplate; - } + to.Priority = from.Priority; + to.QoSOptions = new(from.QoSOptions); + to.RateLimitOptions = new(from.RateLimitOptions); + to.RequestIdKey = from.RequestIdKey; + to.RouteClaimsRequirement = new(from.RouteClaimsRequirement); + to.RouteIsCaseSensitive = from.RouteIsCaseSensitive; + to.SecurityOptions = new(from.SecurityOptions); + to.ServiceName = from.ServiceName; + to.ServiceNamespace = from.ServiceNamespace; + to.Timeout = from.Timeout; + to.UpstreamHeaderTemplates = new Dictionary(from.UpstreamHeaderTemplates); + to.UpstreamHeaderTransform = new(from.UpstreamHeaderTransform); + to.UpstreamHost = from.UpstreamHost; + to.UpstreamHttpMethod = new(from.UpstreamHttpMethod); + to.UpstreamPathTemplate = from.UpstreamPathTemplate; + } } } From 054ea32498247603c94d6b2f86ac5845ada66c45 Mon Sep 17 00:00:00 2001 From: zhannur Date: Thu, 30 May 2024 09:16:32 +0500 Subject: [PATCH 12/50] Remove TimeoutCreator --- .../Configuration/Creator/ITimeoutCreator.cs | 9 --------- src/Ocelot/Configuration/Creator/TimeoutCreator.cs | 14 -------------- src/Ocelot/DependencyInjection/OcelotBuilder.cs | 1 - 3 files changed, 24 deletions(-) delete mode 100644 src/Ocelot/Configuration/Creator/ITimeoutCreator.cs delete mode 100644 src/Ocelot/Configuration/Creator/TimeoutCreator.cs diff --git a/src/Ocelot/Configuration/Creator/ITimeoutCreator.cs b/src/Ocelot/Configuration/Creator/ITimeoutCreator.cs deleted file mode 100644 index d7bbc24f2..000000000 --- a/src/Ocelot/Configuration/Creator/ITimeoutCreator.cs +++ /dev/null @@ -1,9 +0,0 @@ -using Ocelot.Configuration.File; - -namespace Ocelot.Configuration.Creator -{ - public interface ITimeoutCreator - { - int Create(FileRoute fileRoute, FileGlobalConfiguration globalConfiguration); - } -} diff --git a/src/Ocelot/Configuration/Creator/TimeoutCreator.cs b/src/Ocelot/Configuration/Creator/TimeoutCreator.cs deleted file mode 100644 index e0a6ddff2..000000000 --- a/src/Ocelot/Configuration/Creator/TimeoutCreator.cs +++ /dev/null @@ -1,14 +0,0 @@ -using Ocelot.Configuration.File; - -namespace Ocelot.Configuration.Creator -{ - public class TimeoutCreator : ITimeoutCreator - { - public int Create(FileRoute fileRoute, FileGlobalConfiguration globalConfiguration) - { - return fileRoute.Timeout > 0 - ? fileRoute.Timeout - : globalConfiguration.RequestTimeoutSeconds * 1000; - } - } -} diff --git a/src/Ocelot/DependencyInjection/OcelotBuilder.cs b/src/Ocelot/DependencyInjection/OcelotBuilder.cs index dccd7a9c3..33c861232 100644 --- a/src/Ocelot/DependencyInjection/OcelotBuilder.cs +++ b/src/Ocelot/DependencyInjection/OcelotBuilder.cs @@ -71,7 +71,6 @@ public OcelotBuilder(IServiceCollection services, IConfiguration configurationRo Services.TryAddSingleton(); Services.TryAddSingleton(); Services.TryAddSingleton(); - Services.TryAddSingleton(); Services.TryAddSingleton(); Services.TryAddSingleton(); Services.TryAddSingleton(); From 7680e77d72b99ab71cfb03b69533ba0526437e08 Mon Sep 17 00:00:00 2001 From: zhannur Date: Thu, 30 May 2024 09:27:09 +0500 Subject: [PATCH 13/50] RoutesCreator CreateTimeout --- .../Configuration/Creator/IRoutesCreator.cs | 2 ++ .../Configuration/Creator/RoutesCreator.cs | 18 +++++++++++------- .../File/FileGlobalConfiguration.cs | 6 ++++++ 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/Ocelot/Configuration/Creator/IRoutesCreator.cs b/src/Ocelot/Configuration/Creator/IRoutesCreator.cs index e148b84ef..40a29a8cb 100644 --- a/src/Ocelot/Configuration/Creator/IRoutesCreator.cs +++ b/src/Ocelot/Configuration/Creator/IRoutesCreator.cs @@ -5,4 +5,6 @@ namespace Ocelot.Configuration.Creator; public interface IRoutesCreator { List Create(FileConfiguration fileConfiguration); + + int CreateTimeout(FileRoute fileRoute, FileGlobalConfiguration globalConfiguration); } diff --git a/src/Ocelot/Configuration/Creator/RoutesCreator.cs b/src/Ocelot/Configuration/Creator/RoutesCreator.cs index 673c1fe27..b91dc2967 100644 --- a/src/Ocelot/Configuration/Creator/RoutesCreator.cs +++ b/src/Ocelot/Configuration/Creator/RoutesCreator.cs @@ -23,7 +23,8 @@ public class RoutesCreator : IRoutesCreator private readonly IVersionCreator _versionCreator; private readonly IVersionPolicyCreator _versionPolicyCreator; private readonly IMetadataCreator _metadataCreator; - private readonly ITimeoutCreator _timeoutCreator; + + public const int DefaultRequestTimeoutSeconds = 90; public RoutesCreator( IClaimsToThingCreator claimsToThingCreator, @@ -43,8 +44,7 @@ public RoutesCreator( IVersionCreator versionCreator, IVersionPolicyCreator versionPolicyCreator, IUpstreamHeaderTemplatePatternCreator upstreamHeaderTemplatePatternCreator, - IMetadataCreator metadataCreator, - ITimeoutCreator timeoutCreator) + IMetadataCreator metadataCreator) { _routeKeyCreator = routeKeyCreator; _loadBalancerOptionsCreator = loadBalancerOptionsCreator; @@ -65,7 +65,6 @@ public RoutesCreator( _versionPolicyCreator = versionPolicyCreator; _upstreamHeaderTemplatePatternCreator = upstreamHeaderTemplatePatternCreator; _metadataCreator = metadataCreator; - _timeoutCreator = timeoutCreator; } public List Create(FileConfiguration fileConfiguration) @@ -79,6 +78,13 @@ public List Create(FileConfiguration fileConfiguration) .ToList(); } + public int CreateTimeout(FileRoute fileRoute, FileGlobalConfiguration globalConfiguration) + { + return fileRoute.Timeout > 0 + ? fileRoute.Timeout.Value + : (globalConfiguration.TimeoutSeconds ?? DefaultRequestTimeoutSeconds) * 1000; + } + private DownstreamRoute SetUpDownstreamRoute(FileRoute fileRoute, FileGlobalConfiguration globalConfiguration) { var fileRouteOptions = _fileRouteOptionsCreator.Create(fileRoute); @@ -121,8 +127,6 @@ private DownstreamRoute SetUpDownstreamRoute(FileRoute fileRoute, FileGlobalConf var metadata = _metadataCreator.Create(fileRoute.Metadata, globalConfiguration); - var timeout = _timeoutCreator.Create(fileRoute, globalConfiguration); - var route = new DownstreamRouteBuilder() .WithKey(fileRoute.Key) .WithDownstreamPathTemplate(fileRoute.DownstreamPathTemplate) @@ -161,7 +165,7 @@ private DownstreamRoute SetUpDownstreamRoute(FileRoute fileRoute, FileGlobalConf .WithDownstreamHttpVersionPolicy(downstreamHttpVersionPolicy) .WithDownStreamHttpMethod(fileRoute.DownstreamHttpMethod) .WithMetadata(metadata) - .WithTimeout(timeout) + .WithTimeout(CreateTimeout(fileRoute, globalConfiguration)) .Build(); return route; diff --git a/src/Ocelot/Configuration/File/FileGlobalConfiguration.cs b/src/Ocelot/Configuration/File/FileGlobalConfiguration.cs index 2e0c4bcd2..294a5ee65 100644 --- a/src/Ocelot/Configuration/File/FileGlobalConfiguration.cs +++ b/src/Ocelot/Configuration/File/FileGlobalConfiguration.cs @@ -17,6 +17,12 @@ public FileGlobalConfiguration() public string RequestIdKey { get; set; } + /// + /// The timeout in seconds for requests. + /// + /// + /// The timeout value in seconds. + /// public int? TimeoutSeconds { get; set; } public FileServiceDiscoveryProvider ServiceDiscoveryProvider { get; set; } From 7e574f7ba89c71857c5d0d5fbbd4f39b19c725a1 Mon Sep 17 00:00:00 2001 From: zhannur Date: Thu, 30 May 2024 09:39:36 +0500 Subject: [PATCH 14/50] PollyQoSResiliencePipelineProvider Return Old Logic And if-condition --- .../PollyQoSResiliencePipelineProvider.cs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/Ocelot.Provider.Polly/PollyQoSResiliencePipelineProvider.cs b/src/Ocelot.Provider.Polly/PollyQoSResiliencePipelineProvider.cs index 93c153a0a..73582ef37 100644 --- a/src/Ocelot.Provider.Polly/PollyQoSResiliencePipelineProvider.cs +++ b/src/Ocelot.Provider.Polly/PollyQoSResiliencePipelineProvider.cs @@ -4,6 +4,7 @@ using Polly.CircuitBreaker; using Polly.Registry; using Polly.Timeout; +using System; using System.Net; namespace Ocelot.Provider.Polly; @@ -118,6 +119,15 @@ protected virtual ResiliencePipelineBuilder ConfigureTimeou return builder; } + if (options.TimeoutValue != int.MaxValue && options.TimeoutValue > 0) + { + builder.AddTimeout(TimeSpan.FromMilliseconds(options.TimeoutValue)); + } + else + { + builder.AddTimeout(TimeSpan.FromMilliseconds(route.Timeout)); + } + var strategyOptions = new TimeoutStrategyOptions { Timeout = TimeSpan.FromMilliseconds(options.TimeoutValue), From b270e6a63f85fdaaf07830cbc35ce6841b02626d Mon Sep 17 00:00:00 2001 From: zhannur Date: Thu, 30 May 2024 09:47:09 +0500 Subject: [PATCH 15/50] Fix --- src/Ocelot/Configuration/Creator/QoSOptionsCreator.cs | 4 ++-- src/Ocelot/Configuration/QoSOptions.cs | 2 +- src/Ocelot/Requester/MessageInvokerPool.cs | 7 ++++--- test/Ocelot.UnitTests/Configuration/RoutesCreatorTests.cs | 5 +---- 4 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/Ocelot/Configuration/Creator/QoSOptionsCreator.cs b/src/Ocelot/Configuration/Creator/QoSOptionsCreator.cs index c4009f146..471ff149a 100644 --- a/src/Ocelot/Configuration/Creator/QoSOptionsCreator.cs +++ b/src/Ocelot/Configuration/Creator/QoSOptionsCreator.cs @@ -10,7 +10,7 @@ public QoSOptions Create(FileQoSOptions options) return new QoSOptionsBuilder() .WithExceptionsAllowedBeforeBreaking(options.ExceptionsAllowedBeforeBreaking) .WithDurationOfBreak(options.DurationOfBreak) - .WithTimeoutValue(options.TimeoutValue) + .WithTimeoutValue(options.TimeoutValue ?? 0) .Build(); } @@ -18,7 +18,7 @@ public QoSOptions Create(FileQoSOptions options, string pathTemplate, List httpMethods) diff --git a/src/Ocelot/Configuration/QoSOptions.cs b/src/Ocelot/Configuration/QoSOptions.cs index 071bfbf95..5ebba4347 100644 --- a/src/Ocelot/Configuration/QoSOptions.cs +++ b/src/Ocelot/Configuration/QoSOptions.cs @@ -17,7 +17,7 @@ public QoSOptions(FileQoSOptions from) DurationOfBreak = from.DurationOfBreak; ExceptionsAllowedBeforeBreaking = from.ExceptionsAllowedBeforeBreaking; Key = string.Empty; - TimeoutValue = from.TimeoutValue; + TimeoutValue = from.TimeoutValue ?? 0; } public QoSOptions( diff --git a/src/Ocelot/Requester/MessageInvokerPool.cs b/src/Ocelot/Requester/MessageInvokerPool.cs index 83870fc29..464de748a 100644 --- a/src/Ocelot/Requester/MessageInvokerPool.cs +++ b/src/Ocelot/Requester/MessageInvokerPool.cs @@ -48,9 +48,10 @@ private HttpMessageInvoker CreateMessageInvoker(DownstreamRoute downstreamRoute) // Adding timeout handler to the top of the chain. // It's standard behavior to throw TimeoutException after the defined timeout (90 seconds by default) - var timeoutHandler = new TimeoutDelegatingHandler(downstreamRoute.QosOptions.TimeoutValue > 0 - ? TimeSpan.FromMilliseconds(downstreamRoute.QosOptions.TimeoutValue) - : TimeSpan.FromMilliseconds(downstreamRoute.Timeout)) + int milliseconds = downstreamRoute.QosOptions.TimeoutValue > 0 + ? downstreamRoute.QosOptions.TimeoutValue + : downstreamRoute.Timeout; + var timeoutHandler = new TimeoutDelegatingHandler(TimeSpan.FromMilliseconds(milliseconds)) { InnerHandler = baseHandler, }; diff --git a/test/Ocelot.UnitTests/Configuration/RoutesCreatorTests.cs b/test/Ocelot.UnitTests/Configuration/RoutesCreatorTests.cs index 75a870c0d..8bb5f2347 100644 --- a/test/Ocelot.UnitTests/Configuration/RoutesCreatorTests.cs +++ b/test/Ocelot.UnitTests/Configuration/RoutesCreatorTests.cs @@ -27,7 +27,6 @@ public class RoutesCreatorTests : UnitTest private readonly Mock _versionCreator; private readonly Mock _versionPolicyCreator; private readonly Mock _metadataCreator; - private readonly Mock _timeoutCreator; private FileConfiguration _fileConfig; private RouteOptions _rro; private string _requestId; @@ -68,7 +67,6 @@ public RoutesCreatorTests() _versionPolicyCreator = new Mock(); _uhtpCreator = new Mock(); _metadataCreator = new Mock(); - _timeoutCreator = new Mock(); _creator = new RoutesCreator( _cthCreator.Object, @@ -88,8 +86,7 @@ public RoutesCreatorTests() _versionCreator.Object, _versionPolicyCreator.Object, _uhtpCreator.Object, - _metadataCreator.Object, - _timeoutCreator.Object); + _metadataCreator.Object); } [Fact] From 1c37135f5918f63147427ee378d59a3f29506969 Mon Sep 17 00:00:00 2001 From: zhannur Date: Sat, 1 Jun 2024 20:21:37 +0500 Subject: [PATCH 16/50] Fix Remarks --- .../PollyQoSResiliencePipelineProvider.cs | 10 ++++++++-- src/Ocelot/Configuration/Creator/RoutesCreator.cs | 2 +- .../Configuration/File/FileGlobalConfiguration.cs | 2 +- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/Ocelot.Provider.Polly/PollyQoSResiliencePipelineProvider.cs b/src/Ocelot.Provider.Polly/PollyQoSResiliencePipelineProvider.cs index 73582ef37..78f58bde3 100644 --- a/src/Ocelot.Provider.Polly/PollyQoSResiliencePipelineProvider.cs +++ b/src/Ocelot.Provider.Polly/PollyQoSResiliencePipelineProvider.cs @@ -1,4 +1,6 @@ +using Microsoft.Extensions.Options; using Ocelot.Configuration; +using Ocelot.Configuration.File; using Ocelot.Logging; using Ocelot.Provider.Polly.Interfaces; using Polly.CircuitBreaker; @@ -16,14 +18,18 @@ public class PollyQoSResiliencePipelineProvider : IPollyQoSResiliencePipelinePro { private readonly ResiliencePipelineRegistry _registry; private readonly IOcelotLogger _logger; + private readonly FileGlobalConfiguration _global; + + public const int DefaultRequestTimeoutSeconds = 40; public PollyQoSResiliencePipelineProvider( IOcelotLoggerFactory loggerFactory, ResiliencePipelineRegistry registry, - IOptions global)) + IOptions global) { _logger = loggerFactory.CreateLogger(); _registry = registry; + _global = global.Value; } protected static readonly HashSet DefaultServerErrorCodes = new() @@ -125,7 +131,7 @@ protected virtual ResiliencePipelineBuilder ConfigureTimeou } else { - builder.AddTimeout(TimeSpan.FromMilliseconds(route.Timeout)); + builder.AddTimeout(TimeSpan.FromMilliseconds(_global.QoSOptions.TimeoutValue ?? DefaultRequestTimeoutSeconds)); } var strategyOptions = new TimeoutStrategyOptions diff --git a/src/Ocelot/Configuration/Creator/RoutesCreator.cs b/src/Ocelot/Configuration/Creator/RoutesCreator.cs index b91dc2967..653d6c440 100644 --- a/src/Ocelot/Configuration/Creator/RoutesCreator.cs +++ b/src/Ocelot/Configuration/Creator/RoutesCreator.cs @@ -82,7 +82,7 @@ public int CreateTimeout(FileRoute fileRoute, FileGlobalConfiguration globalConf { return fileRoute.Timeout > 0 ? fileRoute.Timeout.Value - : (globalConfiguration.TimeoutSeconds ?? DefaultRequestTimeoutSeconds) * 1000; + : (globalConfiguration.Timeout ?? DefaultRequestTimeoutSeconds) * 1000; } private DownstreamRoute SetUpDownstreamRoute(FileRoute fileRoute, FileGlobalConfiguration globalConfiguration) diff --git a/src/Ocelot/Configuration/File/FileGlobalConfiguration.cs b/src/Ocelot/Configuration/File/FileGlobalConfiguration.cs index 294a5ee65..0ca5555da 100644 --- a/src/Ocelot/Configuration/File/FileGlobalConfiguration.cs +++ b/src/Ocelot/Configuration/File/FileGlobalConfiguration.cs @@ -23,7 +23,7 @@ public FileGlobalConfiguration() /// /// The timeout value in seconds. /// - public int? TimeoutSeconds { get; set; } + public int? Timeout { get; set; } public FileServiceDiscoveryProvider ServiceDiscoveryProvider { get; set; } From ad69a80fc698984621f18f26ac88b8f38574d216 Mon Sep 17 00:00:00 2001 From: zhannur Date: Sun, 2 Jun 2024 19:51:13 +0500 Subject: [PATCH 17/50] Fix remarks --- .../PollyQoSResiliencePipelineProvider.cs | 2 +- .../Configuration/Creator/IRoutesCreator.cs | 6 +++++ .../Configuration/Creator/RoutesCreator.cs | 2 +- src/Ocelot/Configuration/DownstreamRoute.cs | 23 ++++++++++++------- src/Ocelot/Configuration/File/FileRoute.cs | 7 ++++++ .../DependencyInjection/OcelotBuilder.cs | 1 + 6 files changed, 31 insertions(+), 10 deletions(-) diff --git a/src/Ocelot.Provider.Polly/PollyQoSResiliencePipelineProvider.cs b/src/Ocelot.Provider.Polly/PollyQoSResiliencePipelineProvider.cs index 78f58bde3..d3358335e 100644 --- a/src/Ocelot.Provider.Polly/PollyQoSResiliencePipelineProvider.cs +++ b/src/Ocelot.Provider.Polly/PollyQoSResiliencePipelineProvider.cs @@ -20,7 +20,7 @@ public class PollyQoSResiliencePipelineProvider : IPollyQoSResiliencePipelinePro private readonly IOcelotLogger _logger; private readonly FileGlobalConfiguration _global; - public const int DefaultRequestTimeoutSeconds = 40; + public const int DefaultQoSTimeoutMilliseconds = 40_000; public PollyQoSResiliencePipelineProvider( IOcelotLoggerFactory loggerFactory, diff --git a/src/Ocelot/Configuration/Creator/IRoutesCreator.cs b/src/Ocelot/Configuration/Creator/IRoutesCreator.cs index 40a29a8cb..3aeba60c6 100644 --- a/src/Ocelot/Configuration/Creator/IRoutesCreator.cs +++ b/src/Ocelot/Configuration/Creator/IRoutesCreator.cs @@ -6,5 +6,11 @@ public interface IRoutesCreator { List Create(FileConfiguration fileConfiguration); + /// + /// Creates a timeout value for a given file route based on the global configuration. + /// + /// The file route for which to create the timeout. + /// The global configuration to use for creating the timeout. + /// The timeout value in seconds. int CreateTimeout(FileRoute fileRoute, FileGlobalConfiguration globalConfiguration); } diff --git a/src/Ocelot/Configuration/Creator/RoutesCreator.cs b/src/Ocelot/Configuration/Creator/RoutesCreator.cs index 653d6c440..f7eb98b11 100644 --- a/src/Ocelot/Configuration/Creator/RoutesCreator.cs +++ b/src/Ocelot/Configuration/Creator/RoutesCreator.cs @@ -82,7 +82,7 @@ public int CreateTimeout(FileRoute fileRoute, FileGlobalConfiguration globalConf { return fileRoute.Timeout > 0 ? fileRoute.Timeout.Value - : (globalConfiguration.Timeout ?? DefaultRequestTimeoutSeconds) * 1000; + : globalConfiguration.Timeout ?? DefaultRequestTimeoutSeconds; } private DownstreamRoute SetUpDownstreamRoute(FileRoute fileRoute, FileGlobalConfiguration globalConfiguration) diff --git a/src/Ocelot/Configuration/DownstreamRoute.cs b/src/Ocelot/Configuration/DownstreamRoute.cs index 45d59fe9f..3aed91915 100644 --- a/src/Ocelot/Configuration/DownstreamRoute.cs +++ b/src/Ocelot/Configuration/DownstreamRoute.cs @@ -1,4 +1,4 @@ -using Ocelot.Configuration.Creator; +using Ocelot.Configuration.Creator; using Ocelot.Values; namespace Ocelot.Configuration @@ -39,7 +39,7 @@ public DownstreamRoute( bool dangerousAcceptAnyServerCertificateValidator, SecurityOptions securityOptions, string downstreamHttpMethod, - Version downstreamHttpVersion, + Version downstreamHttpVersion, HttpVersionPolicy downstreamHttpVersionPolicy, Dictionary upstreamHeaders, MetadataOptions metadataOptions, @@ -78,10 +78,10 @@ public DownstreamRoute( AddHeadersToUpstream = addHeadersToUpstream; SecurityOptions = securityOptions; DownstreamHttpMethod = downstreamHttpMethod; - DownstreamHttpVersion = downstreamHttpVersion; - DownstreamHttpVersionPolicy = downstreamHttpVersionPolicy; + DownstreamHttpVersion = downstreamHttpVersion; + DownstreamHttpVersionPolicy = downstreamHttpVersionPolicy; UpstreamHeaders = upstreamHeaders ?? new(); - MetadataOptions = metadataOptions; + MetadataOptions = metadataOptions; Timeout = timeout; } @@ -117,7 +117,7 @@ public DownstreamRoute( public bool DangerousAcceptAnyServerCertificateValidator { get; } public SecurityOptions SecurityOptions { get; } public string DownstreamHttpMethod { get; } - public Version DownstreamHttpVersion { get; } + public Version DownstreamHttpVersion { get; } /// The enum specifies behaviors for selecting and negotiating the HTTP version for a request. /// An enum value being mapped from a constant. @@ -129,16 +129,23 @@ public DownstreamRoute( /// HttpRequestMessage.VersionPolicy Property /// /// - public HttpVersionPolicy DownstreamHttpVersionPolicy { get; } + public HttpVersionPolicy DownstreamHttpVersionPolicy { get; } public Dictionary UpstreamHeaders { get; } public bool UseServiceDiscovery { get; } public MetadataOptions MetadataOptions { get; } - public int Timeout { get; } /// Gets the route name depending on whether the service discovery mode is enabled or disabled. /// A object with the name. public string Name() => string.IsNullOrEmpty(ServiceName) && !UseServiceDiscovery ? UpstreamPathTemplate?.Template ?? DownstreamPathTemplate?.Value ?? "?" : string.Join(':', ServiceNamespace, ServiceName, UpstreamPathTemplate?.Template); + + /// + /// The timeout duration for the downstream request in seconds. + /// + /// + /// The timeout value in seconds. + /// + public int Timeout { get; } } } diff --git a/src/Ocelot/Configuration/File/FileRoute.cs b/src/Ocelot/Configuration/File/FileRoute.cs index 7b8a1783c..47b34409d 100644 --- a/src/Ocelot/Configuration/File/FileRoute.cs +++ b/src/Ocelot/Configuration/File/FileRoute.cs @@ -72,6 +72,13 @@ public FileRoute(FileRoute from) public FileSecurityOptions SecurityOptions { get; set; } public string ServiceName { get; set; } public string ServiceNamespace { get; set; } + + /// + /// The timeout in seconds for requests. + /// + /// + /// The timeout value in seconds. + /// public int? Timeout { get; set; } public Dictionary UpstreamHeaderTransform { get; set; } public string UpstreamHost { get; set; } diff --git a/src/Ocelot/DependencyInjection/OcelotBuilder.cs b/src/Ocelot/DependencyInjection/OcelotBuilder.cs index 33c861232..9150269b0 100644 --- a/src/Ocelot/DependencyInjection/OcelotBuilder.cs +++ b/src/Ocelot/DependencyInjection/OcelotBuilder.cs @@ -50,6 +50,7 @@ public OcelotBuilder(IServiceCollection services, IConfiguration configurationRo Configuration = configurationRoot; Services = services; Services.Configure(configurationRoot); + Services.Configure(configurationRoot.GetSection("GlobalConfiguration")); Services.TryAddSingleton(); Services.TryAddSingleton(); From f4dc07c709aae94afe9c796343e573c44ac93430 Mon Sep 17 00:00:00 2001 From: zhannur Date: Sun, 2 Jun 2024 20:04:11 +0500 Subject: [PATCH 18/50] Fix --- src/Ocelot/Requester/MessageInvokerPool.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Ocelot/Requester/MessageInvokerPool.cs b/src/Ocelot/Requester/MessageInvokerPool.cs index 464de748a..208fed035 100644 --- a/src/Ocelot/Requester/MessageInvokerPool.cs +++ b/src/Ocelot/Requester/MessageInvokerPool.cs @@ -48,10 +48,10 @@ private HttpMessageInvoker CreateMessageInvoker(DownstreamRoute downstreamRoute) // Adding timeout handler to the top of the chain. // It's standard behavior to throw TimeoutException after the defined timeout (90 seconds by default) - int milliseconds = downstreamRoute.QosOptions.TimeoutValue > 0 - ? downstreamRoute.QosOptions.TimeoutValue - : downstreamRoute.Timeout; - var timeoutHandler = new TimeoutDelegatingHandler(TimeSpan.FromMilliseconds(milliseconds)) + var timeout = downstreamRoute.QosOptions.TimeoutValue > 0 + ? TimeSpan.FromMilliseconds(downstreamRoute.QosOptions.TimeoutValue) + : TimeSpan.FromSeconds(downstreamRoute.Timeout); + var timeoutHandler = new TimeoutDelegatingHandler(timeout) { InnerHandler = baseHandler, }; From c21b1ca02e2d886a72bbac3ec4148ffbb8c0f29a Mon Sep 17 00:00:00 2001 From: zhannur Date: Sun, 2 Jun 2024 20:20:00 +0500 Subject: [PATCH 19/50] Fix Tests --- .../Polly/PollyQoSResiliencePipelineProviderTests.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/Ocelot.UnitTests/Polly/PollyQoSResiliencePipelineProviderTests.cs b/test/Ocelot.UnitTests/Polly/PollyQoSResiliencePipelineProviderTests.cs index 6a80d2bea..bdbe0ed40 100644 --- a/test/Ocelot.UnitTests/Polly/PollyQoSResiliencePipelineProviderTests.cs +++ b/test/Ocelot.UnitTests/Polly/PollyQoSResiliencePipelineProviderTests.cs @@ -1,5 +1,7 @@ -using Ocelot.Configuration; +using Microsoft.Extensions.Options; +using Ocelot.Configuration; using Ocelot.Configuration.Builder; +using Ocelot.Configuration.File; using Ocelot.Logging; using Ocelot.Provider.Polly; using Polly; @@ -365,8 +367,10 @@ private static PollyQoSResiliencePipelineProvider GivenProvider() loggerFactoryMock .Setup(x => x.CreateLogger()) .Returns(new Mock().Object); + var globalConfigurationOption = new OptionsWrapper( + new FileGlobalConfiguration()); var registry = new ResiliencePipelineRegistry(); - return new PollyQoSResiliencePipelineProvider(loggerFactoryMock.Object, registry); + return new PollyQoSResiliencePipelineProvider(loggerFactoryMock.Object, registry, globalConfigurationOption); } private static DownstreamRoute GivenDownstreamRoute(string routeTemplate, bool inactiveExceptionsAllowedBeforeBreaking = false, int timeOut = 10000) From 2b5e94327a8153e09824b28361a2981ea4db9481 Mon Sep 17 00:00:00 2001 From: Zhannur Akhmetkhanov <63536315+hogwartsdeveloper@users.noreply.github.com> Date: Sat, 1 Jun 2024 20:22:14 +0500 Subject: [PATCH 20/50] Update src/Ocelot/Configuration/File/FileRoute.cs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Raynald Messié --- src/Ocelot/Configuration/File/FileRoute.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Ocelot/Configuration/File/FileRoute.cs b/src/Ocelot/Configuration/File/FileRoute.cs index 47b34409d..89d5cca0a 100644 --- a/src/Ocelot/Configuration/File/FileRoute.cs +++ b/src/Ocelot/Configuration/File/FileRoute.cs @@ -80,7 +80,8 @@ public FileRoute(FileRoute from) /// The timeout value in seconds. /// public int? Timeout { get; set; } - public Dictionary UpstreamHeaderTransform { get; set; } + public IDictionary UpstreamHeaderTransform { get; set; } + public string UpstreamHost { get; set; } public List UpstreamHttpMethod { get; set; } public string UpstreamPathTemplate { get; set; } From 0a8c77c7ada471b71e3b6a2dc4c6e885cfe7846f Mon Sep 17 00:00:00 2001 From: zhannur Date: Sun, 2 Jun 2024 20:25:51 +0500 Subject: [PATCH 21/50] Fix build error --- src/Ocelot/Configuration/File/FileRoute.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Ocelot/Configuration/File/FileRoute.cs b/src/Ocelot/Configuration/File/FileRoute.cs index 89d5cca0a..a4034c7f6 100644 --- a/src/Ocelot/Configuration/File/FileRoute.cs +++ b/src/Ocelot/Configuration/File/FileRoute.cs @@ -130,7 +130,7 @@ public static void DeepCopy(FileRoute from, FileRoute to) to.ServiceNamespace = from.ServiceNamespace; to.Timeout = from.Timeout; to.UpstreamHeaderTemplates = new Dictionary(from.UpstreamHeaderTemplates); - to.UpstreamHeaderTransform = new(from.UpstreamHeaderTransform); + to.UpstreamHeaderTransform = new Dictionary(from.UpstreamHeaderTransform); to.UpstreamHost = from.UpstreamHost; to.UpstreamHttpMethod = new(from.UpstreamHttpMethod); to.UpstreamPathTemplate = from.UpstreamPathTemplate; From 80123c8d331bd65e4553996a8408ba206ec6c0fd Mon Sep 17 00:00:00 2001 From: Raman Maksimchuk Date: Mon, 3 Jun 2024 22:06:28 +0300 Subject: [PATCH 22/50] Try to fix... failed --- test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs b/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs index 81d7005c4..c918f011b 100644 --- a/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs +++ b/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs @@ -87,7 +87,7 @@ public void Should_log_if_ignoring_ssl_errors() var route = new DownstreamRouteBuilder() .WithQosOptions(qosOptions) - .WithHttpHandlerOptions(new HttpHandlerOptions(false, false, false, true, int.MaxValue, TimeSpan.FromSeconds(90))) + .WithHttpHandlerOptions(new HttpHandlerOptions(false, false, false, true, int.MaxValue, TimeSpan.FromSeconds(120))) .WithLoadBalancerKey(string.Empty) .WithUpstreamPathTemplate(new UpstreamPathTemplateBuilder().WithOriginalValue(string.Empty).Build()) .WithQosOptions(new QoSOptionsBuilder().Build()) @@ -110,7 +110,7 @@ public void Should_re_use_cookies_from_container() var route = new DownstreamRouteBuilder() .WithQosOptions(qosOptions) - .WithHttpHandlerOptions(new HttpHandlerOptions(false, true, false, true, int.MaxValue, TimeSpan.FromSeconds(90))) + .WithHttpHandlerOptions(new HttpHandlerOptions(false, true, false, true, int.MaxValue, TimeSpan.FromSeconds(30))) .WithLoadBalancerKey(string.Empty) .WithUpstreamPathTemplate(new UpstreamPathTemplateBuilder().WithOriginalValue(string.Empty).Build()) .WithQosOptions(new QoSOptionsBuilder().Build()) From fe6c3c44a7b13d873f93901625a4934caef58a6e Mon Sep 17 00:00:00 2001 From: zhannur Date: Tue, 4 Jun 2024 13:55:41 +0500 Subject: [PATCH 23/50] Fix Tests --- test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs b/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs index c918f011b..8b9691ab1 100644 --- a/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs +++ b/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs @@ -92,6 +92,7 @@ public void Should_log_if_ignoring_ssl_errors() .WithUpstreamPathTemplate(new UpstreamPathTemplateBuilder().WithOriginalValue(string.Empty).Build()) .WithQosOptions(new QoSOptionsBuilder().Build()) .WithDangerousAcceptAnyServerCertificateValidator(true) + .WithTimeout(90) .Build(); this.Given(x => GivenTheFactoryReturns(new List>())) @@ -114,6 +115,7 @@ public void Should_re_use_cookies_from_container() .WithLoadBalancerKey(string.Empty) .WithUpstreamPathTemplate(new UpstreamPathTemplateBuilder().WithOriginalValue(string.Empty).Build()) .WithQosOptions(new QoSOptionsBuilder().Build()) + .WithTimeout(90) .Build(); this.Given(_ => GivenADownstreamService()) From 76471c2df92bbbabc41824d39802a0f41bd68191 Mon Sep 17 00:00:00 2001 From: zhannur Date: Tue, 4 Jun 2024 14:36:47 +0500 Subject: [PATCH 24/50] MessageInvokerPoolTests --- .../Requester/MessageInvokerPoolTests.cs | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs b/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs index 8b9691ab1..72c621b12 100644 --- a/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs +++ b/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs @@ -154,6 +154,61 @@ public void Create_TimeoutValueInQosOptions_MessageInvokerTimeout(int qosTimeout .Then(x => WhenICallTheClientWillThrowAfterTimeout(TimeSpan.FromSeconds(expectedSeconds))) .BDDfy(); } + + [Theory] + [Trait("Issue", "1869")] + [InlineData(5)] + [InlineData(10)] + public void Create_TimeoutValueInRoute_MessageInvokerTimeout(int timeoutSeconds) + { + // Arrange + var qosOptions = new QoSOptionsBuilder() + .Build(); + var handlerOptions = new HttpHandlerOptionsBuilder() + .WithUseMaxConnectionPerServer(int.MaxValue) + .Build(); + var route = new DownstreamRouteBuilder() + .WithQosOptions(qosOptions) + .WithHttpHandlerOptions(handlerOptions) + .WithTimeout(timeoutSeconds) + .Build(); + GivenTheFactoryReturnsNothing(); + + this.Given(x => GivenTheFactoryReturns(new List>())) + .And(x => GivenAMessageInvokerPool()) + .And(x => GivenARequest(route)) + .Then(x => WhenICallTheClientWillThrowAfterTimeout(TimeSpan.FromSeconds(timeoutSeconds))) + .BDDfy(); + } + + [Theory] + [Trait("Issue", "1869")] + [InlineData(5, 6)] + [InlineData(10, 12)] + public void Create_TimeoutValueInQosOptions_And_RouteTimeout_MessageInvokerTimeout(int qosTimeout, int routeTimeout) + { + // Arrange + var qosOptions = new QoSOptionsBuilder() + .WithTimeoutValue(qosTimeout * 1000) + .Build(); + + var handlerOptions = new HttpHandlerOptionsBuilder() + .WithUseMaxConnectionPerServer(int.MaxValue) + .Build(); + + var route = new DownstreamRouteBuilder() + .WithQosOptions(qosOptions) + .WithHttpHandlerOptions(handlerOptions) + .WithTimeout(routeTimeout) + .Build(); + + // Assert + this.Given(x => GivenTheFactoryReturns(new List>())) + .And(x => GivenAMessageInvokerPool()) + .And(x => GivenARequest(route)) + .Then(x => WhenICallTheClientWillThrowAfterTimeout(TimeSpan.FromSeconds(qosTimeout))) + .BDDfy(); + } private void ThenTheDangerousAcceptAnyServerCertificateValidatorWarningIsLogged() { From 12d8659e3cd1f1274abe78bc247e5ea887120367 Mon Sep 17 00:00:00 2001 From: zhannur Date: Tue, 4 Jun 2024 15:58:04 +0500 Subject: [PATCH 25/50] RoutesCreatorTests --- .../Configuration/RoutesCreatorTests.cs | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/test/Ocelot.UnitTests/Configuration/RoutesCreatorTests.cs b/test/Ocelot.UnitTests/Configuration/RoutesCreatorTests.cs index 8bb5f2347..9a3e9f1f7 100644 --- a/test/Ocelot.UnitTests/Configuration/RoutesCreatorTests.cs +++ b/test/Ocelot.UnitTests/Configuration/RoutesCreatorTests.cs @@ -162,6 +162,34 @@ public void Should_return_routes() .BDDfy(); } + [Fact] + public void Should_route_timeout() + { + // Arrange + var fileRoute = new FileRoute { Timeout = 10 }; + var globalConfiguration = new FileGlobalConfiguration { Timeout = 20 }; + + // Act + var timeout = _creator.CreateTimeout(fileRoute, globalConfiguration); + + // Assert + Assert.Equal(fileRoute.Timeout, timeout); + } + + [Fact] + public void Should_global_timeout_when_not_route_timeout() + { + // Arrange + var fileRoute = new FileRoute(); + var globalConfiguration = new FileGlobalConfiguration { Timeout = 20 }; + + // Act + var timeout = _creator.CreateTimeout(fileRoute, globalConfiguration); + + // Assert + Assert.Equal(globalConfiguration.Timeout, timeout); + } + private void ThenTheDependenciesAreCalledCorrectly() { ThenTheDepsAreCalledFor(_fileConfig.Routes[0], _fileConfig.GlobalConfiguration); From 30e425619c2e23513823275dea97c0827ccd4eff Mon Sep 17 00:00:00 2001 From: Raman Maksimchuk Date: Tue, 4 Jun 2024 13:38:58 +0300 Subject: [PATCH 26/50] Review fixed/new unit tests --- .../Requester/MessageInvokerPoolTests.cs | 34 ++++++++++--------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs b/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs index 72c621b12..8d72c3846 100644 --- a/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs +++ b/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs @@ -3,6 +3,7 @@ using Microsoft.AspNetCore.Http; using Ocelot.Configuration; using Ocelot.Configuration.Builder; +using Ocelot.Configuration.Creator; using Ocelot.Configuration.File; using Ocelot.Logging; using Ocelot.Middleware; @@ -82,17 +83,17 @@ public void If_two_delegating_handlers_are_defined_then_these_should_be_call_in_ [Fact] public void Should_log_if_ignoring_ssl_errors() { + const int DefaultTimeout = RoutesCreator.DefaultRequestTimeoutSeconds; var qosOptions = new QoSOptionsBuilder() .Build(); - var route = new DownstreamRouteBuilder() .WithQosOptions(qosOptions) - .WithHttpHandlerOptions(new HttpHandlerOptions(false, false, false, true, int.MaxValue, TimeSpan.FromSeconds(120))) + .WithHttpHandlerOptions(new HttpHandlerOptions(false, false, false, true, int.MaxValue, TimeSpan.FromSeconds(DefaultTimeout))) .WithLoadBalancerKey(string.Empty) .WithUpstreamPathTemplate(new UpstreamPathTemplateBuilder().WithOriginalValue(string.Empty).Build()) .WithQosOptions(new QoSOptionsBuilder().Build()) .WithDangerousAcceptAnyServerCertificateValidator(true) - .WithTimeout(90) + .WithTimeout(DefaultTimeout) .Build(); this.Given(x => GivenTheFactoryReturns(new List>())) @@ -106,16 +107,16 @@ public void Should_log_if_ignoring_ssl_errors() [Fact] public void Should_re_use_cookies_from_container() { + const int DefaultTimeout = RoutesCreator.DefaultRequestTimeoutSeconds; var qosOptions = new QoSOptionsBuilder() .Build(); - var route = new DownstreamRouteBuilder() .WithQosOptions(qosOptions) - .WithHttpHandlerOptions(new HttpHandlerOptions(false, true, false, true, int.MaxValue, TimeSpan.FromSeconds(30))) + .WithHttpHandlerOptions(new HttpHandlerOptions(false, true, false, true, int.MaxValue, TimeSpan.FromSeconds(DefaultTimeout))) .WithLoadBalancerKey(string.Empty) .WithUpstreamPathTemplate(new UpstreamPathTemplateBuilder().WithOriginalValue(string.Empty).Build()) .WithQosOptions(new QoSOptionsBuilder().Build()) - .WithTimeout(90) + .WithTimeout(DefaultTimeout) .Build(); this.Given(_ => GivenADownstreamService()) @@ -130,10 +131,10 @@ public void Should_re_use_cookies_from_container() } [Theory] - [Trait("Issue", "1833")] + [Trait("Bug", "1833")] [InlineData(5, 5)] [InlineData(10, 10)] - public void Create_TimeoutValueInQosOptions_MessageInvokerTimeout(int qosTimeout, int expectedSeconds) + public void SendAsync_TimeoutValueInQosOptions_ThrowTimeoutException(int qosTimeout, int expectedSeconds) { // Arrange var qosOptions = new QoSOptionsBuilder() @@ -156,10 +157,11 @@ public void Create_TimeoutValueInQosOptions_MessageInvokerTimeout(int qosTimeout } [Theory] - [Trait("Issue", "1869")] + [Trait("PR", "2073")] + [Trait("Feat", "1314 1869")] [InlineData(5)] [InlineData(10)] - public void Create_TimeoutValueInRoute_MessageInvokerTimeout(int timeoutSeconds) + public void SendAsync_TimeoutValueInRoute_ThrowTimeoutExceptionAfterRouteTimeout(int timeoutSeconds) { // Arrange var qosOptions = new QoSOptionsBuilder() @@ -170,7 +172,7 @@ public void Create_TimeoutValueInRoute_MessageInvokerTimeout(int timeoutSeconds) var route = new DownstreamRouteBuilder() .WithQosOptions(qosOptions) .WithHttpHandlerOptions(handlerOptions) - .WithTimeout(timeoutSeconds) + .WithTimeout(timeoutSeconds) // !!! TimeoutValueInRoute .Build(); GivenTheFactoryReturnsNothing(); @@ -182,16 +184,16 @@ public void Create_TimeoutValueInRoute_MessageInvokerTimeout(int timeoutSeconds) } [Theory] - [Trait("Issue", "1869")] + [Trait("PR", "2073")] + [Trait("Feat", "1314 1869")] [InlineData(5, 6)] [InlineData(10, 12)] - public void Create_TimeoutValueInQosOptions_And_RouteTimeout_MessageInvokerTimeout(int qosTimeout, int routeTimeout) + public void SendAsync_TimeoutValueInQosOptionsIsLessThanRouteTimeout_ThrowTimeoutExceptionAfterQoSTimeout(int qosTimeout, int routeTimeout) { // Arrange var qosOptions = new QoSOptionsBuilder() - .WithTimeoutValue(qosTimeout * 1000) + .WithTimeoutValue(qosTimeout * 1000) // !!! TimeoutValueInQosOptionsIsLessThanRouteTimeout .Build(); - var handlerOptions = new HttpHandlerOptionsBuilder() .WithUseMaxConnectionPerServer(int.MaxValue) .Build(); @@ -199,7 +201,7 @@ public void Create_TimeoutValueInQosOptions_And_RouteTimeout_MessageInvokerTimeo var route = new DownstreamRouteBuilder() .WithQosOptions(qosOptions) .WithHttpHandlerOptions(handlerOptions) - .WithTimeout(routeTimeout) + .WithTimeout(routeTimeout) // this value is greater than QoS one .Build(); // Assert From 830f029c75ce0a66fa6edb30474d74c6f345efd4 Mon Sep 17 00:00:00 2001 From: Raman Maksimchuk Date: Tue, 4 Jun 2024 13:56:18 +0300 Subject: [PATCH 27/50] Remove BDDfy from unit tests --- .../Requester/MessageInvokerPoolTests.cs | 96 +++++++++---------- 1 file changed, 44 insertions(+), 52 deletions(-) diff --git a/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs b/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs index 8d72c3846..9a717f389 100644 --- a/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs +++ b/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs @@ -39,23 +39,21 @@ public MessageInvokerPoolTests() [Fact] public void If_calling_the_same_downstream_route_twice_should_return_the_same_message_invoker() { - this.Given(x => x.GivenADownstreamRoute("/super-test")) - .And(x => x.AndAHandlerFactory()) - .And(x => x.GivenAMessageInvokerPool()) - .When(x => x.WhenGettingMessageInvokerTwice()) - .Then(x => x.ThenTheInvokersShouldBeTheSame()) - .BDDfy(); + GivenADownstreamRoute("/super-test"); + AndAHandlerFactory(); + GivenAMessageInvokerPool(); + WhenGettingMessageInvokerTwice(); + ThenTheInvokersShouldBeTheSame(); } [Fact] public void If_calling_two_different_downstream_routes_should_return_different_message_invokers() { - this.Given(x => x.GivenTwoDifferentDownstreamRoutes("/super-test", "/super-test")) - .And(x => x.AndAHandlerFactory()) - .And(x => x.GivenAMessageInvokerPool()) - .When(x => x.WhenGettingMessageInvokerForBothRoutes()) - .Then(x => x.ThenTheInvokersShouldNotBeTheSame()) - .BDDfy(); + GivenTwoDifferentDownstreamRoutes("/super-test", "/super-test"); + AndAHandlerFactory(); + GivenAMessageInvokerPool(); + WhenGettingMessageInvokerForBothRoutes(); + ThenTheInvokersShouldNotBeTheSame(); } [Fact] @@ -70,14 +68,13 @@ public void If_two_delegating_handlers_are_defined_then_these_should_be_call_in_ () => fakeTwo, }; - this.Given(x => GivenTheFactoryReturns(handlers)) - .And(x => GivenADownstreamRoute("/super-test")) - .And(x => GivenAMessageInvokerPool()) - .And(x => GivenARequest()) - .When(x => WhenICallTheClient("http://www.bbc.co.uk")) - .Then(x => ThenTheFakeAreHandledInOrder(fakeOne, fakeTwo)) - .And(x => ThenSomethingIsReturned()) - .BDDfy(); + GivenTheFactoryReturns(handlers); + GivenADownstreamRoute("/super-test"); + GivenAMessageInvokerPool(); + GivenARequest(); + WhenICallTheClient("http://www.bbc.co.uk"); + ThenTheFakeAreHandledInOrder(fakeOne, fakeTwo); + ThenSomethingIsReturned(); } [Fact] @@ -96,12 +93,11 @@ public void Should_log_if_ignoring_ssl_errors() .WithTimeout(DefaultTimeout) .Build(); - this.Given(x => GivenTheFactoryReturns(new List>())) - .And(x => GivenAMessageInvokerPool()) - .And(x => GivenARequest(route)) - .When(x => WhenICallTheClient("http://www.google.com/")) - .Then(x => ThenTheDangerousAcceptAnyServerCertificateValidatorWarningIsLogged()) - .BDDfy(); + GivenTheFactoryReturns(new List>()); + GivenAMessageInvokerPool(); + GivenARequest(route); + WhenICallTheClient("http://www.google.com/"); + ThenTheDangerousAcceptAnyServerCertificateValidatorWarningIsLogged(); } [Fact] @@ -119,15 +115,14 @@ public void Should_re_use_cookies_from_container() .WithTimeout(DefaultTimeout) .Build(); - this.Given(_ => GivenADownstreamService()) - .And(x => GivenTheFactoryReturns(new List>())) - .And(x => GivenAMessageInvokerPool()) - .And(x => GivenARequest(route)) - .And(_ => WhenICallTheClient("http://localhost:5003")) - .And(_ => ThenTheCookieIsSet()) - .When(_ => WhenICallTheClient("http://localhost:5003")) - .Then(_ => ThenTheResponseIsOk()) - .BDDfy(); + GivenADownstreamService(); + GivenTheFactoryReturns(new List>()); + GivenAMessageInvokerPool(); + GivenARequest(route); + WhenICallTheClient("http://localhost:5003"); + ThenTheCookieIsSet(); + WhenICallTheClient("http://localhost:5003"); + ThenTheResponseIsOk(); } [Theory] @@ -149,11 +144,10 @@ public void SendAsync_TimeoutValueInQosOptions_ThrowTimeoutException(int qosTime .Build(); GivenTheFactoryReturnsNothing(); - this.Given(x => GivenTheFactoryReturns(new List>())) - .And(x => GivenAMessageInvokerPool()) - .And(x => GivenARequest(route)) - .Then(x => WhenICallTheClientWillThrowAfterTimeout(TimeSpan.FromSeconds(expectedSeconds))) - .BDDfy(); + GivenTheFactoryReturns(new List>()); + GivenAMessageInvokerPool(); + GivenARequest(route); + WhenICallTheClientWillThrowAfterTimeout(TimeSpan.FromSeconds(expectedSeconds)); } [Theory] @@ -176,11 +170,10 @@ public void SendAsync_TimeoutValueInRoute_ThrowTimeoutExceptionAfterRouteTimeout .Build(); GivenTheFactoryReturnsNothing(); - this.Given(x => GivenTheFactoryReturns(new List>())) - .And(x => GivenAMessageInvokerPool()) - .And(x => GivenARequest(route)) - .Then(x => WhenICallTheClientWillThrowAfterTimeout(TimeSpan.FromSeconds(timeoutSeconds))) - .BDDfy(); + GivenTheFactoryReturns(new List>()); + GivenAMessageInvokerPool(); + GivenARequest(route); + WhenICallTheClientWillThrowAfterTimeout(TimeSpan.FromSeconds(timeoutSeconds)); } [Theory] @@ -197,19 +190,18 @@ public void SendAsync_TimeoutValueInQosOptionsIsLessThanRouteTimeout_ThrowTimeou var handlerOptions = new HttpHandlerOptionsBuilder() .WithUseMaxConnectionPerServer(int.MaxValue) .Build(); - + var route = new DownstreamRouteBuilder() .WithQosOptions(qosOptions) .WithHttpHandlerOptions(handlerOptions) .WithTimeout(routeTimeout) // this value is greater than QoS one .Build(); - + // Assert - this.Given(x => GivenTheFactoryReturns(new List>())) - .And(x => GivenAMessageInvokerPool()) - .And(x => GivenARequest(route)) - .Then(x => WhenICallTheClientWillThrowAfterTimeout(TimeSpan.FromSeconds(qosTimeout))) - .BDDfy(); + GivenTheFactoryReturns(new List>()); + GivenAMessageInvokerPool(); + GivenARequest(route); + WhenICallTheClientWillThrowAfterTimeout(TimeSpan.FromSeconds(qosTimeout)); } private void ThenTheDangerousAcceptAnyServerCertificateValidatorWarningIsLogged() From 567ae5ce01b0980342e509987265beca422e06d2 Mon Sep 17 00:00:00 2001 From: Raman Maksimchuk Date: Tue, 4 Jun 2024 14:43:47 +0300 Subject: [PATCH 28/50] Review `MessageInvokerPoolTests` and AAA pattern --- .../Requester/MessageInvokerPoolTests.cs | 61 +++++++++++++------ 1 file changed, 44 insertions(+), 17 deletions(-) diff --git a/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs b/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs index 9a717f389..7298f5e43 100644 --- a/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs +++ b/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs @@ -14,7 +14,6 @@ namespace Ocelot.UnitTests.Requester; -[Trait("PR", "1824")] public class MessageInvokerPoolTests : UnitTest { private DownstreamRoute _downstreamRoute1; @@ -37,90 +36,117 @@ public MessageInvokerPoolTests() } [Fact] + [Trait("PR", "1824")] public void If_calling_the_same_downstream_route_twice_should_return_the_same_message_invoker() { + // Arrange GivenADownstreamRoute("/super-test"); AndAHandlerFactory(); GivenAMessageInvokerPool(); + + // Act WhenGettingMessageInvokerTwice(); + + // Assert ThenTheInvokersShouldBeTheSame(); } [Fact] + [Trait("PR", "1824")] public void If_calling_two_different_downstream_routes_should_return_different_message_invokers() { + // Arrange GivenTwoDifferentDownstreamRoutes("/super-test", "/super-test"); AndAHandlerFactory(); GivenAMessageInvokerPool(); + + // Act WhenGettingMessageInvokerForBothRoutes(); + + // Assert ThenTheInvokersShouldNotBeTheSame(); } [Fact] + [Trait("PR", "1824")] public void If_two_delegating_handlers_are_defined_then_these_should_be_call_in_order() { + // Arrange var fakeOne = new FakeDelegatingHandler(); var fakeTwo = new FakeDelegatingHandler(); - var handlers = new List> { () => fakeOne, () => fakeTwo, }; - GivenTheFactoryReturns(handlers); GivenADownstreamRoute("/super-test"); GivenAMessageInvokerPool(); GivenARequest(); + + // Act WhenICallTheClient("http://www.bbc.co.uk"); + + // Assert ThenTheFakeAreHandledInOrder(fakeOne, fakeTwo); ThenSomethingIsReturned(); } + /// 120 seconds. + private static TimeSpan DefaultPooledConnectionLifeTime => TimeSpan.FromSeconds(HttpHandlerOptionsCreator.DefaultPooledConnectionLifetimeSeconds); + [Fact] + [Trait("PR", "1824")] public void Should_log_if_ignoring_ssl_errors() { - const int DefaultTimeout = RoutesCreator.DefaultRequestTimeoutSeconds; + // Arrange var qosOptions = new QoSOptionsBuilder() .Build(); var route = new DownstreamRouteBuilder() .WithQosOptions(qosOptions) - .WithHttpHandlerOptions(new HttpHandlerOptions(false, false, false, true, int.MaxValue, TimeSpan.FromSeconds(DefaultTimeout))) + .WithHttpHandlerOptions(new HttpHandlerOptions(false, false, false, true, int.MaxValue, DefaultPooledConnectionLifeTime)) .WithLoadBalancerKey(string.Empty) .WithUpstreamPathTemplate(new UpstreamPathTemplateBuilder().WithOriginalValue(string.Empty).Build()) .WithQosOptions(new QoSOptionsBuilder().Build()) .WithDangerousAcceptAnyServerCertificateValidator(true) - .WithTimeout(DefaultTimeout) + .WithTimeout(RoutesCreator.DefaultRequestTimeoutSeconds) .Build(); - GivenTheFactoryReturns(new List>()); GivenAMessageInvokerPool(); GivenARequest(route); - WhenICallTheClient("http://www.google.com/"); + + // Act + WhenICallTheClient("http://www.bbc.co.uk"); + + // Assert ThenTheDangerousAcceptAnyServerCertificateValidatorWarningIsLogged(); } [Fact] + [Trait("PR", "1824")] public void Should_re_use_cookies_from_container() { - const int DefaultTimeout = RoutesCreator.DefaultRequestTimeoutSeconds; + // Arrange var qosOptions = new QoSOptionsBuilder() .Build(); var route = new DownstreamRouteBuilder() .WithQosOptions(qosOptions) - .WithHttpHandlerOptions(new HttpHandlerOptions(false, true, false, true, int.MaxValue, TimeSpan.FromSeconds(DefaultTimeout))) + .WithHttpHandlerOptions(new HttpHandlerOptions(false, true, false, true, int.MaxValue, DefaultPooledConnectionLifeTime)) .WithLoadBalancerKey(string.Empty) .WithUpstreamPathTemplate(new UpstreamPathTemplateBuilder().WithOriginalValue(string.Empty).Build()) .WithQosOptions(new QoSOptionsBuilder().Build()) - .WithTimeout(DefaultTimeout) + .WithTimeout(RoutesCreator.DefaultRequestTimeoutSeconds) .Build(); - GivenADownstreamService(); GivenTheFactoryReturns(new List>()); GivenAMessageInvokerPool(); GivenARequest(route); + + // Act, Assert 1 WhenICallTheClient("http://localhost:5003"); ThenTheCookieIsSet(); + + // Act, Assert 2 WhenICallTheClient("http://localhost:5003"); ThenTheResponseIsOk(); } @@ -143,10 +169,11 @@ public void SendAsync_TimeoutValueInQosOptions_ThrowTimeoutException(int qosTime .WithHttpHandlerOptions(handlerOptions) .Build(); GivenTheFactoryReturnsNothing(); - GivenTheFactoryReturns(new List>()); GivenAMessageInvokerPool(); GivenARequest(route); + + // Act, Assert WhenICallTheClientWillThrowAfterTimeout(TimeSpan.FromSeconds(expectedSeconds)); } @@ -169,10 +196,11 @@ public void SendAsync_TimeoutValueInRoute_ThrowTimeoutExceptionAfterRouteTimeout .WithTimeout(timeoutSeconds) // !!! TimeoutValueInRoute .Build(); GivenTheFactoryReturnsNothing(); - GivenTheFactoryReturns(new List>()); GivenAMessageInvokerPool(); GivenARequest(route); + + // Act, Assert WhenICallTheClientWillThrowAfterTimeout(TimeSpan.FromSeconds(timeoutSeconds)); } @@ -190,17 +218,16 @@ public void SendAsync_TimeoutValueInQosOptionsIsLessThanRouteTimeout_ThrowTimeou var handlerOptions = new HttpHandlerOptionsBuilder() .WithUseMaxConnectionPerServer(int.MaxValue) .Build(); - var route = new DownstreamRouteBuilder() .WithQosOptions(qosOptions) .WithHttpHandlerOptions(handlerOptions) .WithTimeout(routeTimeout) // this value is greater than QoS one .Build(); - - // Assert GivenTheFactoryReturns(new List>()); GivenAMessageInvokerPool(); GivenARequest(route); + + // Act, Assert WhenICallTheClientWillThrowAfterTimeout(TimeSpan.FromSeconds(qosTimeout)); } From ae638b1ace9496590e0c8f879aca10807c468615 Mon Sep 17 00:00:00 2001 From: Raman Maksimchuk Date: Tue, 4 Jun 2024 18:11:09 +0300 Subject: [PATCH 29/50] Fix test : `Should_timeout_per_default_after_90_seconds` --- src/Ocelot/Requester/MessageInvokerPool.cs | 13 ++++++++--- test/Ocelot.AcceptanceTests/PollyQoSTests.cs | 23 +++++++++++--------- 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/src/Ocelot/Requester/MessageInvokerPool.cs b/src/Ocelot/Requester/MessageInvokerPool.cs index 208fed035..5f858facf 100644 --- a/src/Ocelot/Requester/MessageInvokerPool.cs +++ b/src/Ocelot/Requester/MessageInvokerPool.cs @@ -34,6 +34,8 @@ public HttpMessageInvoker Get(DownstreamRoute downstreamRoute) public void Clear() => _handlersPool.Clear(); + private int? _timeoutMilliseconds = null; + private HttpMessageInvoker CreateMessageInvoker(DownstreamRoute downstreamRoute) { var baseHandler = CreateHandler(downstreamRoute); @@ -46,11 +48,16 @@ private HttpMessageInvoker CreateMessageInvoker(DownstreamRoute downstreamRoute) baseHandler = delegatingHandler; } + if (!_timeoutMilliseconds.HasValue) + { + _timeoutMilliseconds = downstreamRoute.QosOptions.TimeoutValue > 0 + ? downstreamRoute.QosOptions.TimeoutValue + : downstreamRoute.Timeout * 1000; + } + // Adding timeout handler to the top of the chain. // It's standard behavior to throw TimeoutException after the defined timeout (90 seconds by default) - var timeout = downstreamRoute.QosOptions.TimeoutValue > 0 - ? TimeSpan.FromMilliseconds(downstreamRoute.QosOptions.TimeoutValue) - : TimeSpan.FromSeconds(downstreamRoute.Timeout); + var timeout = TimeSpan.FromMilliseconds(_timeoutMilliseconds.Value); var timeoutHandler = new TimeoutDelegatingHandler(timeout) { InnerHandler = baseHandler, diff --git a/test/Ocelot.AcceptanceTests/PollyQoSTests.cs b/test/Ocelot.AcceptanceTests/PollyQoSTests.cs index 5a086019a..f78805041 100644 --- a/test/Ocelot.AcceptanceTests/PollyQoSTests.cs +++ b/test/Ocelot.AcceptanceTests/PollyQoSTests.cs @@ -1,5 +1,6 @@ using Microsoft.AspNetCore.Http; using Ocelot.Configuration; +using Ocelot.Configuration.Creator; using Ocelot.Configuration.File; using Ocelot.Requester; using System.Reflection; @@ -182,26 +183,28 @@ public void Open_circuit_should_not_effect_different_route() [Fact] [Trait("Bug", "1833")] public void Should_timeout_per_default_after_90_seconds() - { + { + var defTimeoutMs = 1_000 * RoutesCreator.DefaultRequestTimeoutSeconds; // original value is 90 seconds + defTimeoutMs = 1_000 * 3; // override value var port = PortFinder.GetRandomPort(); var route = GivenRoute(port, new QoSOptions(new FileQoSOptions()), HttpMethods.Get); var configuration = GivenConfiguration(route); - - this.Given(x => x.GivenThereIsAServiceRunningOn(port, HttpStatusCode.Created, string.Empty, 3500)) // 3.5s > 3s -> ServiceUnavailable + + this.Given(x => x.GivenThereIsAServiceRunningOn(port, HttpStatusCode.Created, string.Empty, defTimeoutMs + 500)) // 3.5s > 3s -> ServiceUnavailable .And(x => GivenThereIsAConfiguration(configuration)) .And(x => GivenOcelotIsRunningWithPolly()) - .And(x => GivenIHackDefaultTimeoutValue(3)) // after 3 secs -> Timeout exception aka request cancellation + .And(x => GivenIHackDefaultTimeoutValue(defTimeoutMs)) // after 3 secs -> Timeout exception aka request cancellation .When(x => WhenIGetUrlOnTheApiGateway("/")) .Then(x => ThenTheStatusCodeShouldBe(HttpStatusCode.ServiceUnavailable)) .BDDfy(); } - private void GivenIHackDefaultTimeoutValue(int defaultTimeoutSeconds) - { - var field = typeof(MessageInvokerPool).GetField("_requestTimeoutSeconds", BindingFlags.NonPublic | BindingFlags.Instance); - var service = _ocelotServer.Services.GetService(typeof(IMessageInvokerPool)); - field.SetValue(service, defaultTimeoutSeconds); // hack the value of default 90 seconds - } + private void GivenIHackDefaultTimeoutValue(int defaultTimeoutMs) + { + var field = typeof(MessageInvokerPool).GetField("_timeoutMilliseconds", BindingFlags.NonPublic | BindingFlags.Instance); + var service = _ocelotServer.Services.GetService(typeof(IMessageInvokerPool)); + field.SetValue(service, defaultTimeoutMs); // hack the value of default 90 seconds + } private static void GivenIWaitMilliseconds(int ms) => Thread.Sleep(ms); From 62336fad2c040845c53869115ed53a446504ab62 Mon Sep 17 00:00:00 2001 From: Raman Maksimchuk Date: Tue, 4 Jun 2024 20:54:47 +0300 Subject: [PATCH 30/50] More refactoring: QoS vs DynRoute timeouts --- .../PollyQoSResiliencePipelineProvider.cs | 8 +- .../Builder/QoSOptionsBuilder.cs | 4 +- .../Configuration/Creator/DynamicsCreator.cs | 25 ++++-- .../Configuration/Creator/IDynamicsCreator.cs | 8 ++ .../Configuration/Creator/IRoutesCreator.cs | 6 +- .../Creator/QoSOptionsCreator.cs | 6 +- .../Configuration/Creator/RoutesCreator.cs | 13 +-- .../Configuration/File/FileDynamicRoute.cs | 7 ++ src/Ocelot/Configuration/File/FileRoute.cs | 10 +-- src/Ocelot/Configuration/QoSOptions.cs | 87 ++++++++----------- src/Ocelot/Requester/MessageInvokerPool.cs | 5 +- 11 files changed, 97 insertions(+), 82 deletions(-) diff --git a/src/Ocelot.Provider.Polly/PollyQoSResiliencePipelineProvider.cs b/src/Ocelot.Provider.Polly/PollyQoSResiliencePipelineProvider.cs index d3358335e..675661765 100644 --- a/src/Ocelot.Provider.Polly/PollyQoSResiliencePipelineProvider.cs +++ b/src/Ocelot.Provider.Polly/PollyQoSResiliencePipelineProvider.cs @@ -125,13 +125,15 @@ protected virtual ResiliencePipelineBuilder ConfigureTimeou return builder; } - if (options.TimeoutValue != int.MaxValue && options.TimeoutValue > 0) + var options = route.QosOptions; + var timeout = options.TimeoutValue; + if (timeout.HasValue && timeout.Value > 0) { - builder.AddTimeout(TimeSpan.FromMilliseconds(options.TimeoutValue)); + builder.AddTimeout(TimeSpan.FromMilliseconds(timeout.Value)); } else { - builder.AddTimeout(TimeSpan.FromMilliseconds(_global.QoSOptions.TimeoutValue ?? DefaultRequestTimeoutSeconds)); + builder.AddTimeout(TimeSpan.FromMilliseconds(_global.QoSOptions.TimeoutValue ?? DefaultQoSTimeoutMilliseconds)); } var strategyOptions = new TimeoutStrategyOptions diff --git a/src/Ocelot/Configuration/Builder/QoSOptionsBuilder.cs b/src/Ocelot/Configuration/Builder/QoSOptionsBuilder.cs index 717918485..0849eff61 100644 --- a/src/Ocelot/Configuration/Builder/QoSOptionsBuilder.cs +++ b/src/Ocelot/Configuration/Builder/QoSOptionsBuilder.cs @@ -6,7 +6,7 @@ public class QoSOptionsBuilder private int _durationOfBreak; - private int _timeoutValue; + private int? _timeoutValue; private string _key; @@ -22,7 +22,7 @@ public QoSOptionsBuilder WithDurationOfBreak(int durationOfBreak) return this; } - public QoSOptionsBuilder WithTimeoutValue(int timeoutValue) + public QoSOptionsBuilder WithTimeoutValue(int? timeoutValue) { _timeoutValue = timeoutValue; return this; diff --git a/src/Ocelot/Configuration/Creator/DynamicsCreator.cs b/src/Ocelot/Configuration/Creator/DynamicsCreator.cs index cb5cdf419..6693d1f01 100644 --- a/src/Ocelot/Configuration/Creator/DynamicsCreator.cs +++ b/src/Ocelot/Configuration/Creator/DynamicsCreator.cs @@ -10,6 +10,11 @@ public class DynamicsCreator : IDynamicsCreator private readonly IVersionPolicyCreator _versionPolicyCreator; private readonly IMetadataCreator _metadataCreator; + /// + /// Defines the default timeout in seconds for all routes, applicable at both the Route-level and globally. + /// + public const int DefaultRequestTimeoutSeconds = 90; + public DynamicsCreator( IRateLimitOptionsCreator rateLimitOptionsCreator, IVersionCreator versionCreator, @@ -29,22 +34,28 @@ public List Create(FileConfiguration fileConfiguration) .ToList(); } - private Route SetUpDynamicRoute(FileDynamicRoute fileDynamicRoute, FileGlobalConfiguration globalConfiguration) + public int CreateTimeout(FileDynamicRoute route, FileGlobalConfiguration global) + => route.Timeout > 0 + ? route.Timeout.Value + : global.Timeout ?? DefaultRequestTimeoutSeconds; + + private Route SetUpDynamicRoute(FileDynamicRoute dynamicRoute, FileGlobalConfiguration globalConfiguration) { var rateLimitOption = _rateLimitOptionsCreator - .Create(fileDynamicRoute.RateLimitRule, globalConfiguration); + .Create(dynamicRoute.RateLimitRule, globalConfiguration); - var version = _versionCreator.Create(fileDynamicRoute.DownstreamHttpVersion); - var versionPolicy = _versionPolicyCreator.Create(fileDynamicRoute.DownstreamHttpVersionPolicy); - var metadata = _metadataCreator.Create(fileDynamicRoute.Metadata, globalConfiguration); + var version = _versionCreator.Create(dynamicRoute.DownstreamHttpVersion); + var versionPolicy = _versionPolicyCreator.Create(dynamicRoute.DownstreamHttpVersionPolicy); + var metadata = _metadataCreator.Create(dynamicRoute.Metadata, globalConfiguration); var downstreamRoute = new DownstreamRouteBuilder() .WithEnableRateLimiting(rateLimitOption.EnableRateLimiting) .WithRateLimitOptions(rateLimitOption) - .WithServiceName(fileDynamicRoute.ServiceName) + .WithServiceName(dynamicRoute.ServiceName) .WithDownstreamHttpVersion(version) .WithDownstreamHttpVersionPolicy(versionPolicy) - .WithMetadata(metadata) + .WithMetadata(metadata) + .WithTimeout(CreateTimeout(dynamicRoute, globalConfiguration)) .Build(); var route = new RouteBuilder() diff --git a/src/Ocelot/Configuration/Creator/IDynamicsCreator.cs b/src/Ocelot/Configuration/Creator/IDynamicsCreator.cs index b030af9f8..6fcde0679 100644 --- a/src/Ocelot/Configuration/Creator/IDynamicsCreator.cs +++ b/src/Ocelot/Configuration/Creator/IDynamicsCreator.cs @@ -5,5 +5,13 @@ namespace Ocelot.Configuration.Creator public interface IDynamicsCreator { List Create(FileConfiguration fileConfiguration); + + /// + /// Creates a timeout value for a given file route based on the global configuration. + /// + /// The file route for which to create the timeout. + /// The global configuration to use for creating the timeout. + /// The timeout value in seconds. + int CreateTimeout(FileDynamicRoute route, FileGlobalConfiguration global); } } diff --git a/src/Ocelot/Configuration/Creator/IRoutesCreator.cs b/src/Ocelot/Configuration/Creator/IRoutesCreator.cs index 3aeba60c6..e54f7c2c0 100644 --- a/src/Ocelot/Configuration/Creator/IRoutesCreator.cs +++ b/src/Ocelot/Configuration/Creator/IRoutesCreator.cs @@ -9,8 +9,8 @@ public interface IRoutesCreator /// /// Creates a timeout value for a given file route based on the global configuration. /// - /// The file route for which to create the timeout. - /// The global configuration to use for creating the timeout. + /// The file route for which to create the timeout. + /// The global configuration to use for creating the timeout. /// The timeout value in seconds. - int CreateTimeout(FileRoute fileRoute, FileGlobalConfiguration globalConfiguration); + int CreateTimeout(FileRoute route, FileGlobalConfiguration global); } diff --git a/src/Ocelot/Configuration/Creator/QoSOptionsCreator.cs b/src/Ocelot/Configuration/Creator/QoSOptionsCreator.cs index 471ff149a..f6fce957a 100644 --- a/src/Ocelot/Configuration/Creator/QoSOptionsCreator.cs +++ b/src/Ocelot/Configuration/Creator/QoSOptionsCreator.cs @@ -10,7 +10,7 @@ public QoSOptions Create(FileQoSOptions options) return new QoSOptionsBuilder() .WithExceptionsAllowedBeforeBreaking(options.ExceptionsAllowedBeforeBreaking) .WithDurationOfBreak(options.DurationOfBreak) - .WithTimeoutValue(options.TimeoutValue ?? 0) + .WithTimeoutValue(options.TimeoutValue) .Build(); } @@ -18,7 +18,7 @@ public QoSOptions Create(FileQoSOptions options, string pathTemplate, List httpMethods) @@ -28,7 +28,7 @@ public QoSOptions Create(QoSOptions options, string pathTemplate, List h return Map(key, options.TimeoutValue, options.DurationOfBreak, options.ExceptionsAllowedBeforeBreaking); } - private static QoSOptions Map(string key, int timeoutValue, int durationOfBreak, int exceptionsAllowedBeforeBreaking) + private static QoSOptions Map(string key, int? timeoutValue, int durationOfBreak, int exceptionsAllowedBeforeBreaking) { return new QoSOptionsBuilder() .WithExceptionsAllowedBeforeBreaking(exceptionsAllowedBeforeBreaking) diff --git a/src/Ocelot/Configuration/Creator/RoutesCreator.cs b/src/Ocelot/Configuration/Creator/RoutesCreator.cs index f7eb98b11..b64da68da 100644 --- a/src/Ocelot/Configuration/Creator/RoutesCreator.cs +++ b/src/Ocelot/Configuration/Creator/RoutesCreator.cs @@ -23,7 +23,10 @@ public class RoutesCreator : IRoutesCreator private readonly IVersionCreator _versionCreator; private readonly IVersionPolicyCreator _versionPolicyCreator; private readonly IMetadataCreator _metadataCreator; - + + /// + /// Defines the default timeout in seconds for all routes, applicable at both the Route-level and globally. + /// public const int DefaultRequestTimeoutSeconds = 90; public RoutesCreator( @@ -78,11 +81,11 @@ public List Create(FileConfiguration fileConfiguration) .ToList(); } - public int CreateTimeout(FileRoute fileRoute, FileGlobalConfiguration globalConfiguration) + public int CreateTimeout(FileRoute route, FileGlobalConfiguration global) { - return fileRoute.Timeout > 0 - ? fileRoute.Timeout.Value - : globalConfiguration.Timeout ?? DefaultRequestTimeoutSeconds; + return route.Timeout > 0 + ? route.Timeout.Value + : global.Timeout ?? DefaultRequestTimeoutSeconds; } private DownstreamRoute SetUpDownstreamRoute(FileRoute fileRoute, FileGlobalConfiguration globalConfiguration) diff --git a/src/Ocelot/Configuration/File/FileDynamicRoute.cs b/src/Ocelot/Configuration/File/FileDynamicRoute.cs index a42581531..0d8bbd834 100644 --- a/src/Ocelot/Configuration/File/FileDynamicRoute.cs +++ b/src/Ocelot/Configuration/File/FileDynamicRoute.cs @@ -2,6 +2,9 @@ namespace Ocelot.Configuration.File { + /// + /// TODO: Make it as a base Route File-model. + /// public class FileDynamicRoute { public string ServiceName { get; set; } @@ -20,5 +23,9 @@ public class FileDynamicRoute /// public string DownstreamHttpVersionPolicy { get; set; } public IDictionary Metadata { get; set; } + + /// The timeout in seconds for requests. + /// A where T is value in seconds. + public int? Timeout { get; set; } } } diff --git a/src/Ocelot/Configuration/File/FileRoute.cs b/src/Ocelot/Configuration/File/FileRoute.cs index a4034c7f6..694ccfa30 100644 --- a/src/Ocelot/Configuration/File/FileRoute.cs +++ b/src/Ocelot/Configuration/File/FileRoute.cs @@ -2,7 +2,7 @@ namespace Ocelot.Configuration.File { - public class FileRoute : IRoute, ICloneable + public class FileRoute : IRoute, ICloneable // TODO: Inherit from FileDynamicRoute (FileRouteBase) or an interface with FileDynamicRoute props { public FileRoute() { @@ -73,12 +73,8 @@ public FileRoute(FileRoute from) public string ServiceName { get; set; } public string ServiceNamespace { get; set; } - /// - /// The timeout in seconds for requests. - /// - /// - /// The timeout value in seconds. - /// + /// The timeout in seconds for requests. + /// A where T is value in seconds. public int? Timeout { get; set; } public IDictionary UpstreamHeaderTransform { get; set; } diff --git a/src/Ocelot/Configuration/QoSOptions.cs b/src/Ocelot/Configuration/QoSOptions.cs index 5ebba4347..1a4f6255f 100644 --- a/src/Ocelot/Configuration/QoSOptions.cs +++ b/src/Ocelot/Configuration/QoSOptions.cs @@ -1,36 +1,36 @@ using Ocelot.Configuration.File; -namespace Ocelot.Configuration +namespace Ocelot.Configuration; + +public class QoSOptions { - public class QoSOptions + public QoSOptions(QoSOptions from) { - public QoSOptions(QoSOptions from) - { - DurationOfBreak = from.DurationOfBreak; - ExceptionsAllowedBeforeBreaking = from.ExceptionsAllowedBeforeBreaking; - Key = from.Key; - TimeoutValue = from.TimeoutValue; - } + DurationOfBreak = from.DurationOfBreak; + ExceptionsAllowedBeforeBreaking = from.ExceptionsAllowedBeforeBreaking; + Key = from.Key; + TimeoutValue = from.TimeoutValue; + } - public QoSOptions(FileQoSOptions from) - { - DurationOfBreak = from.DurationOfBreak; - ExceptionsAllowedBeforeBreaking = from.ExceptionsAllowedBeforeBreaking; - Key = string.Empty; - TimeoutValue = from.TimeoutValue ?? 0; - } + public QoSOptions(FileQoSOptions from) + { + DurationOfBreak = from.DurationOfBreak; + ExceptionsAllowedBeforeBreaking = from.ExceptionsAllowedBeforeBreaking; + Key = string.Empty; + TimeoutValue = from.TimeoutValue; + } - public QoSOptions( - int exceptionsAllowedBeforeBreaking, - int durationOfBreak, - int timeoutValue, - string key) - { - DurationOfBreak = durationOfBreak; - ExceptionsAllowedBeforeBreaking = exceptionsAllowedBeforeBreaking; - Key = key; - TimeoutValue = timeoutValue; - } + public QoSOptions( + int exceptionsAllowedBeforeBreaking, + int durationOfBreak, + int? timeoutValue, + string key) + { + DurationOfBreak = durationOfBreak; + ExceptionsAllowedBeforeBreaking = exceptionsAllowedBeforeBreaking; + Key = key; + TimeoutValue = timeoutValue; + } /// How long the circuit should stay open before resetting in milliseconds. /// If using Polly version 8 or above, this value must be 500 (0.5 sec) or greater. @@ -39,30 +39,17 @@ public QoSOptions( public const int LowBreakDuration = 500; // 0.5 seconds public const int DefaultBreakDuration = 5_000; // 5 seconds - /// - /// How many times a circuit can fail before being set to open. - /// - /// - /// If using Polly version 8 or above, this value must be 2 or greater. - /// - /// - /// An value (no of exceptions). - /// - public int ExceptionsAllowedBeforeBreaking { get; } + /// How many times a circuit can fail before being set to open. + /// If using Polly version 8 or above, this value must be 2 or greater. + /// An value (no of exceptions). + public int ExceptionsAllowedBeforeBreaking { get; } - public string Key { get; } + public string Key { get; } - /// - /// Value for TimeoutStrategy in milliseconds. - /// - /// - /// If using Polly version 8 or above, this value must be 1000 (1 sec) or greater. - /// - /// - /// An value (milliseconds). - /// - public int TimeoutValue { get; } + /// Value for TimeoutStrategy in milliseconds. + /// If using Polly version 8 or above, this value must be 1000 (1 sec) or greater. + /// A (T is ) value (milliseconds). + public int? TimeoutValue { get; } - public bool UseQos => ExceptionsAllowedBeforeBreaking > 0 || TimeoutValue > 0; - } + public bool UseQos => ExceptionsAllowedBeforeBreaking > 0 || TimeoutValue > 0; } diff --git a/src/Ocelot/Requester/MessageInvokerPool.cs b/src/Ocelot/Requester/MessageInvokerPool.cs index 5f858facf..323db9761 100644 --- a/src/Ocelot/Requester/MessageInvokerPool.cs +++ b/src/Ocelot/Requester/MessageInvokerPool.cs @@ -50,8 +50,9 @@ private HttpMessageInvoker CreateMessageInvoker(DownstreamRoute downstreamRoute) if (!_timeoutMilliseconds.HasValue) { - _timeoutMilliseconds = downstreamRoute.QosOptions.TimeoutValue > 0 - ? downstreamRoute.QosOptions.TimeoutValue + var qosTimeout = downstreamRoute.QosOptions.TimeoutValue; + _timeoutMilliseconds = qosTimeout.HasValue && qosTimeout.Value > 0 + ? qosTimeout.Value : downstreamRoute.Timeout * 1000; } From 6ab5355dac02cbe151750627b26cd2bf0484d2d2 Mon Sep 17 00:00:00 2001 From: Raman Maksimchuk Date: Thu, 6 Jun 2024 14:07:57 +0300 Subject: [PATCH 31/50] Polly constraints --- .../PollyQoSResiliencePipelineProvider.cs | 39 ++++++++++--------- src/Ocelot/Configuration/QoSOptions.cs | 17 +++++--- ...PollyQoSResiliencePipelineProviderTests.cs | 22 ++++------- 3 files changed, 38 insertions(+), 40 deletions(-) diff --git a/src/Ocelot.Provider.Polly/PollyQoSResiliencePipelineProvider.cs b/src/Ocelot.Provider.Polly/PollyQoSResiliencePipelineProvider.cs index 675661765..bb53d70b7 100644 --- a/src/Ocelot.Provider.Polly/PollyQoSResiliencePipelineProvider.cs +++ b/src/Ocelot.Provider.Polly/PollyQoSResiliencePipelineProvider.cs @@ -82,14 +82,21 @@ protected virtual ResiliencePipelineBuilder ConfigureCircui var options = route.QosOptions; var info = $"Circuit Breaker for the route: {GetRouteName(route)}: "; + + // Polly constraints + int minimumThroughput = options.ExceptionsAllowedBeforeBreaking >= QoSOptions.LowMinimumThroughput + ? options.ExceptionsAllowedBeforeBreaking + : QoSOptions.DefaultMinimumThroughput; + int breakDurationMs = options.DurationOfBreak > QoSOptions.LowBreakDuration + ? options.DurationOfBreak + : QoSOptions.DefaultBreakDuration; + var strategyOptions = new CircuitBreakerStrategyOptions { FailureRatio = 0.8, SamplingDuration = TimeSpan.FromSeconds(10), - MinimumThroughput = options.ExceptionsAllowedBeforeBreaking, - BreakDuration = options.DurationOfBreak > QoSOptions.LowBreakDuration - ? TimeSpan.FromMilliseconds(options.DurationOfBreak) - : TimeSpan.FromMilliseconds(QoSOptions.DefaultBreakDuration), + MinimumThroughput = minimumThroughput, + BreakDuration = TimeSpan.FromMilliseconds(breakDurationMs), ShouldHandle = new PredicateBuilder() .HandleResult(message => ServerErrorCodes.Contains(message.StatusCode)) .Handle() @@ -116,29 +123,23 @@ protected virtual ResiliencePipelineBuilder ConfigureCircui protected virtual ResiliencePipelineBuilder ConfigureTimeout(ResiliencePipelineBuilder builder, DownstreamRoute route) { - var options = route.QosOptions; + int? timeoutMs = route?.QosOptions?.TimeoutValue + ?? _global?.QoSOptions?.TimeoutValue + ?? DefaultQoSTimeoutMilliseconds; // TODO Need team's consensus!!! Prefer QoSOptions.DefaultTimeout ? - // Add Timeout strategy if TimeoutValue is not int.MaxValue and greater than 0 - // TimeoutValue must be defined in QosOptions! - if (options.TimeoutValue == int.MaxValue || options.TimeoutValue <= 0) + if (!timeoutMs.HasValue || timeoutMs.Value <= 0) // 0 means No option! { return builder; } - var options = route.QosOptions; - var timeout = options.TimeoutValue; - if (timeout.HasValue && timeout.Value > 0) - { - builder.AddTimeout(TimeSpan.FromMilliseconds(timeout.Value)); - } - else - { - builder.AddTimeout(TimeSpan.FromMilliseconds(_global.QoSOptions.TimeoutValue ?? DefaultQoSTimeoutMilliseconds)); - } + // Polly constraints + timeoutMs = timeoutMs.Value > QoSOptions.LowTimeout && timeoutMs.Value < QoSOptions.HighTimeout + ? timeoutMs.Value + : QoSOptions.DefaultTimeout; var strategyOptions = new TimeoutStrategyOptions { - Timeout = TimeSpan.FromMilliseconds(options.TimeoutValue), + Timeout = TimeSpan.FromMilliseconds(timeoutMs.Value), OnTimeout = _ => { _logger.LogInformation(() => $"Timeout for the route: {GetRouteName(route)}"); diff --git a/src/Ocelot/Configuration/QoSOptions.cs b/src/Ocelot/Configuration/QoSOptions.cs index 1a4f6255f..77df6436b 100644 --- a/src/Ocelot/Configuration/QoSOptions.cs +++ b/src/Ocelot/Configuration/QoSOptions.cs @@ -32,17 +32,19 @@ public QoSOptions( TimeoutValue = timeoutValue; } - /// How long the circuit should stay open before resetting in milliseconds. - /// If using Polly version 8 or above, this value must be 500 (0.5 sec) or greater. - /// An value (milliseconds). - public int DurationOfBreak { get; } = DefaultBreakDuration; - public const int LowBreakDuration = 500; // 0.5 seconds - public const int DefaultBreakDuration = 5_000; // 5 seconds + /// How long the circuit should stay open before resetting in milliseconds. + /// If using Polly version 8 or above, this value must be 500 (0.5 sec) or greater. + /// An value (milliseconds). + public int DurationOfBreak { get; } = DefaultBreakDuration; + public const int LowBreakDuration = 500; // 0.5 seconds + public const int DefaultBreakDuration = 5_000; // 5 seconds /// How many times a circuit can fail before being set to open. /// If using Polly version 8 or above, this value must be 2 or greater. /// An value (no of exceptions). public int ExceptionsAllowedBeforeBreaking { get; } + public const int LowMinimumThroughput = 2; + public const int DefaultMinimumThroughput = 100; public string Key { get; } @@ -50,6 +52,9 @@ public QoSOptions( /// If using Polly version 8 or above, this value must be 1000 (1 sec) or greater. /// A (T is ) value (milliseconds). public int? TimeoutValue { get; } + public const int LowTimeout = 10; // 10 ms + public const int HighTimeout = 86_400_000; // 24 hours in milliseconds + public const int DefaultTimeout = 30_000; // 30 seconds public bool UseQos => ExceptionsAllowedBeforeBreaking > 0 || TimeoutValue > 0; } diff --git a/test/Ocelot.UnitTests/Polly/PollyQoSResiliencePipelineProviderTests.cs b/test/Ocelot.UnitTests/Polly/PollyQoSResiliencePipelineProviderTests.cs index bdbe0ed40..7f0fea216 100644 --- a/test/Ocelot.UnitTests/Polly/PollyQoSResiliencePipelineProviderTests.cs +++ b/test/Ocelot.UnitTests/Polly/PollyQoSResiliencePipelineProviderTests.cs @@ -26,9 +26,7 @@ public void ShouldBuild() var route = new DownstreamRouteBuilder() .WithQosOptions(options) .Build(); - var loggerFactoryMock = new Mock(); - var registry = new ResiliencePipelineRegistry(); - var provider = new PollyQoSResiliencePipelineProvider(loggerFactoryMock.Object, registry); + var provider = GivenProvider(); // Act var resiliencePipeline = provider.GetResiliencePipeline(route); @@ -48,9 +46,7 @@ public void ShouldNotBuild_ReturnedEmpty() var route = new DownstreamRouteBuilder() .WithQosOptions(options) .Build(); - var loggerFactoryMock = new Mock(); - var registry = new ResiliencePipelineRegistry(); - var provider = new PollyQoSResiliencePipelineProvider(loggerFactoryMock.Object, registry); + var provider = GivenProvider(); // Act var resiliencePipeline = provider.GetResiliencePipeline(route); @@ -78,9 +74,7 @@ public void ShouldBuild_WithDefaultBreakDuration(int durationOfBreak, int expect var route = new DownstreamRouteBuilder() .WithQosOptions(options) .Build(); - var loggerFactoryMock = new Mock(); - var registry = new ResiliencePipelineRegistry(); - var provider = new PollyQoSResiliencePipelineProvider(loggerFactoryMock.Object, registry); + var provider = GivenProvider(); // Act var resiliencePipeline = provider.GetResiliencePipeline(route); @@ -363,14 +357,12 @@ await Assert.ThrowsAsync(async () => private static PollyQoSResiliencePipelineProvider GivenProvider() { - var loggerFactoryMock = new Mock(); - loggerFactoryMock - .Setup(x => x.CreateLogger()) + var loggerFactory = new Mock(); + loggerFactory.Setup(x => x.CreateLogger()) .Returns(new Mock().Object); - var globalConfigurationOption = new OptionsWrapper( - new FileGlobalConfiguration()); + var globalConfiguration = new OptionsWrapper(new()); var registry = new ResiliencePipelineRegistry(); - return new PollyQoSResiliencePipelineProvider(loggerFactoryMock.Object, registry, globalConfigurationOption); + return new PollyQoSResiliencePipelineProvider(loggerFactory.Object, registry, globalConfiguration); } private static DownstreamRoute GivenDownstreamRoute(string routeTemplate, bool inactiveExceptionsAllowedBeforeBreaking = false, int timeOut = 10000) From 046bd51a6f794c19de9c6887be099957e75568e7 Mon Sep 17 00:00:00 2001 From: Raman Maksimchuk Date: Mon, 14 Oct 2024 12:14:39 +0300 Subject: [PATCH 32/50] Fix SA warnings --- .../PollyQoSResiliencePipelineProvider.cs | 3 ++- test/Ocelot.AcceptanceTests/PollyQoSTests.cs | 14 +++++++------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/Ocelot.Provider.Polly/PollyQoSResiliencePipelineProvider.cs b/src/Ocelot.Provider.Polly/PollyQoSResiliencePipelineProvider.cs index bb53d70b7..28f742241 100644 --- a/src/Ocelot.Provider.Polly/PollyQoSResiliencePipelineProvider.cs +++ b/src/Ocelot.Provider.Polly/PollyQoSResiliencePipelineProvider.cs @@ -127,7 +127,8 @@ protected virtual ResiliencePipelineBuilder ConfigureTimeou ?? _global?.QoSOptions?.TimeoutValue ?? DefaultQoSTimeoutMilliseconds; // TODO Need team's consensus!!! Prefer QoSOptions.DefaultTimeout ? - if (!timeoutMs.HasValue || timeoutMs.Value <= 0) // 0 means No option! + // 0 means No option! + if (!timeoutMs.HasValue || timeoutMs.Value <= 0) { return builder; } diff --git a/test/Ocelot.AcceptanceTests/PollyQoSTests.cs b/test/Ocelot.AcceptanceTests/PollyQoSTests.cs index f78805041..573f12f0e 100644 --- a/test/Ocelot.AcceptanceTests/PollyQoSTests.cs +++ b/test/Ocelot.AcceptanceTests/PollyQoSTests.cs @@ -197,15 +197,15 @@ public void Should_timeout_per_default_after_90_seconds() .When(x => WhenIGetUrlOnTheApiGateway("/")) .Then(x => ThenTheStatusCodeShouldBe(HttpStatusCode.ServiceUnavailable)) .BDDfy(); + } + + private void GivenIHackDefaultTimeoutValue(int defaultTimeoutMs) + { + var field = typeof(MessageInvokerPool).GetField("_timeoutMilliseconds", BindingFlags.NonPublic | BindingFlags.Instance); + var service = _ocelotServer.Services.GetService(typeof(IMessageInvokerPool)); + field.SetValue(service, defaultTimeoutMs); // hack the value of default 90 seconds } - private void GivenIHackDefaultTimeoutValue(int defaultTimeoutMs) - { - var field = typeof(MessageInvokerPool).GetField("_timeoutMilliseconds", BindingFlags.NonPublic | BindingFlags.Instance); - var service = _ocelotServer.Services.GetService(typeof(IMessageInvokerPool)); - field.SetValue(service, defaultTimeoutMs); // hack the value of default 90 seconds - } - private static void GivenIWaitMilliseconds(int ms) => Thread.Sleep(ms); private HttpStatusCode _brokenServiceStatusCode; From 6f763ee985cd93d27d4e65b82f04500436779af1 Mon Sep 17 00:00:00 2001 From: Raman Maksimchuk Date: Mon, 14 Oct 2024 13:27:00 +0300 Subject: [PATCH 33/50] Inherit from Steps --- .../ServiceDiscovery/ConsulConfigurationInConsulTests.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/Ocelot.AcceptanceTests/ServiceDiscovery/ConsulConfigurationInConsulTests.cs b/test/Ocelot.AcceptanceTests/ServiceDiscovery/ConsulConfigurationInConsulTests.cs index 60ccb0559..cfabc197a 100644 --- a/test/Ocelot.AcceptanceTests/ServiceDiscovery/ConsulConfigurationInConsulTests.cs +++ b/test/Ocelot.AcceptanceTests/ServiceDiscovery/ConsulConfigurationInConsulTests.cs @@ -26,6 +26,13 @@ public ConsulConfigurationInConsulTests() _consulServices = new List(); } + public override void Dispose() + { + _builder?.Dispose(); + _fakeConsulBuilder?.Dispose(); + base.Dispose(); + } + [Fact] public void Should_return_response_200_with_simple_url() { From 7cb4c6ec3de601dffe8255492591bc8630310c84 Mon Sep 17 00:00:00 2001 From: Raman Maksimchuk Date: Mon, 14 Oct 2024 18:45:09 +0300 Subject: [PATCH 34/50] The `HttpMessageInvoker` retrieves the timeout value from `DownstreamRoute`, which determines this value based on the QoS mode. The `DefaultTimeoutSeconds` should be defined in a single reusable location, currently within `DownstreamRoute`. --- .../Builder/DownstreamRouteBuilder.cs | 4 +- .../Configuration/Creator/DynamicsCreator.cs | 9 +-- .../Configuration/Creator/RoutesCreator.cs | 11 +-- src/Ocelot/Configuration/DownstreamRoute.cs | 21 ++++-- .../Configuration/File/FileQoSOptions.cs | 13 +++- src/Ocelot/Configuration/File/FileRoute.cs | 13 +++- src/Ocelot/Configuration/QoSOptions.cs | 4 +- .../Repository/FileConfigurationPoller.cs | 2 +- src/Ocelot/Requester/MessageInvokerPool.cs | 15 ++--- test/Ocelot.AcceptanceTests/PollyQoSTests.cs | 2 +- .../ConsulConfigurationInConsulTests.cs | 67 ++++++++++--------- .../FileModels/FileQoSOptionsTests.cs | 8 ++- .../Requester/MessageInvokerPoolTests.cs | 10 ++- 13 files changed, 95 insertions(+), 84 deletions(-) diff --git a/src/Ocelot/Configuration/Builder/DownstreamRouteBuilder.cs b/src/Ocelot/Configuration/Builder/DownstreamRouteBuilder.cs index 06c1260cf..f231b1405 100644 --- a/src/Ocelot/Configuration/Builder/DownstreamRouteBuilder.cs +++ b/src/Ocelot/Configuration/Builder/DownstreamRouteBuilder.cs @@ -43,7 +43,7 @@ public class DownstreamRouteBuilder private HttpVersionPolicy _downstreamHttpVersionPolicy; private Dictionary _upstreamHeaders; private MetadataOptions _metadataOptions; - private int _timeout; + private int? _timeout; public DownstreamRouteBuilder() { @@ -283,7 +283,7 @@ public DownstreamRouteBuilder WithMetadata(MetadataOptions metadataOptions) return this; } - public DownstreamRouteBuilder WithTimeout(int timeout) + public DownstreamRouteBuilder WithTimeout(int? timeout) { _timeout = timeout; return this; diff --git a/src/Ocelot/Configuration/Creator/DynamicsCreator.cs b/src/Ocelot/Configuration/Creator/DynamicsCreator.cs index 6693d1f01..89286e761 100644 --- a/src/Ocelot/Configuration/Creator/DynamicsCreator.cs +++ b/src/Ocelot/Configuration/Creator/DynamicsCreator.cs @@ -10,11 +10,6 @@ public class DynamicsCreator : IDynamicsCreator private readonly IVersionPolicyCreator _versionPolicyCreator; private readonly IMetadataCreator _metadataCreator; - /// - /// Defines the default timeout in seconds for all routes, applicable at both the Route-level and globally. - /// - public const int DefaultRequestTimeoutSeconds = 90; - public DynamicsCreator( IRateLimitOptionsCreator rateLimitOptionsCreator, IVersionCreator versionCreator, @@ -35,9 +30,7 @@ public List Create(FileConfiguration fileConfiguration) } public int CreateTimeout(FileDynamicRoute route, FileGlobalConfiguration global) - => route.Timeout > 0 - ? route.Timeout.Value - : global.Timeout ?? DefaultRequestTimeoutSeconds; + => route.Timeout ?? global.Timeout ?? DownstreamRoute.DefaultTimeoutSeconds; private Route SetUpDynamicRoute(FileDynamicRoute dynamicRoute, FileGlobalConfiguration globalConfiguration) { diff --git a/src/Ocelot/Configuration/Creator/RoutesCreator.cs b/src/Ocelot/Configuration/Creator/RoutesCreator.cs index b64da68da..5039126a0 100644 --- a/src/Ocelot/Configuration/Creator/RoutesCreator.cs +++ b/src/Ocelot/Configuration/Creator/RoutesCreator.cs @@ -24,11 +24,6 @@ public class RoutesCreator : IRoutesCreator private readonly IVersionPolicyCreator _versionPolicyCreator; private readonly IMetadataCreator _metadataCreator; - /// - /// Defines the default timeout in seconds for all routes, applicable at both the Route-level and globally. - /// - public const int DefaultRequestTimeoutSeconds = 90; - public RoutesCreator( IClaimsToThingCreator claimsToThingCreator, IAuthenticationOptionsCreator authOptionsCreator, @@ -82,11 +77,7 @@ public List Create(FileConfiguration fileConfiguration) } public int CreateTimeout(FileRoute route, FileGlobalConfiguration global) - { - return route.Timeout > 0 - ? route.Timeout.Value - : global.Timeout ?? DefaultRequestTimeoutSeconds; - } + => route.Timeout ?? global.Timeout ?? DownstreamRoute.DefaultTimeoutSeconds; private DownstreamRoute SetUpDownstreamRoute(FileRoute fileRoute, FileGlobalConfiguration globalConfiguration) { diff --git a/src/Ocelot/Configuration/DownstreamRoute.cs b/src/Ocelot/Configuration/DownstreamRoute.cs index 3aed91915..70a44dd37 100644 --- a/src/Ocelot/Configuration/DownstreamRoute.cs +++ b/src/Ocelot/Configuration/DownstreamRoute.cs @@ -43,7 +43,7 @@ public DownstreamRoute( HttpVersionPolicy downstreamHttpVersionPolicy, Dictionary upstreamHeaders, MetadataOptions metadataOptions, - int timeout) + int? timeout) { DangerousAcceptAnyServerCertificateValidator = dangerousAcceptAnyServerCertificateValidator; AddHeadersToDownstream = addHeadersToDownstream; @@ -138,14 +138,21 @@ public DownstreamRoute( /// A object with the name. public string Name() => string.IsNullOrEmpty(ServiceName) && !UseServiceDiscovery ? UpstreamPathTemplate?.Template ?? DownstreamPathTemplate?.Value ?? "?" - : string.Join(':', ServiceNamespace, ServiceName, UpstreamPathTemplate?.Template); + : string.Join(':', ServiceNamespace, ServiceName, UpstreamPathTemplate?.Template); + + /// The timeout duration for the downstream request in seconds. + /// A (T is ) value in seconds. + public int? Timeout { get; } + + /// Defines the default timeout in seconds for all routes, applicable at both the Route-level and globally. + public const int DefaultTimeoutSeconds = 90; /// - /// The timeout duration for the downstream request in seconds. + /// Calculates timeout in milliseconds based on QoS options with applying default timeout values. /// - /// - /// The timeout value in seconds. - /// - public int Timeout { get; } + /// An value in milliseconds. + public int TimeoutMilliseconds() => QosOptions.UseQos + ? QosOptions.TimeoutValue ?? QoSOptions.DefaultTimeout + : 1000 * (Timeout ?? DefaultTimeoutSeconds); } } diff --git a/src/Ocelot/Configuration/File/FileQoSOptions.cs b/src/Ocelot/Configuration/File/FileQoSOptions.cs index cee99efd7..6e7ea69ab 100644 --- a/src/Ocelot/Configuration/File/FileQoSOptions.cs +++ b/src/Ocelot/Configuration/File/FileQoSOptions.cs @@ -13,7 +13,7 @@ public FileQoSOptions() { DurationOfBreak = 1; ExceptionsAllowedBeforeBreaking = 0; - TimeoutValue = 0; + TimeoutValue = null; // default value will be assigned in consumer services: see DownstreamRoute. } public FileQoSOptions(FileQoSOptions from) @@ -32,6 +32,17 @@ public FileQoSOptions(QoSOptions from) public int DurationOfBreak { get; set; } public int ExceptionsAllowedBeforeBreaking { get; set; } + + /// Explicit timeout value which overrides default one. + /// Reused in, or ignored in favor of implicit default value: + /// + /// + /// + /// + /// + /// + /// + /// A (T is ) value in milliseconds. public int? TimeoutValue { get; set; } } } diff --git a/src/Ocelot/Configuration/File/FileRoute.cs b/src/Ocelot/Configuration/File/FileRoute.cs index 694ccfa30..2e939eef5 100644 --- a/src/Ocelot/Configuration/File/FileRoute.cs +++ b/src/Ocelot/Configuration/File/FileRoute.cs @@ -73,11 +73,18 @@ public FileRoute(FileRoute from) public string ServiceName { get; set; } public string ServiceNamespace { get; set; } - /// The timeout in seconds for requests. - /// A where T is value in seconds. + /// Explicit timeout value which overrides default one. + /// Reused in, or ignored in favor of implicit default value: + /// + /// + /// + /// + /// + /// + /// A (T is ) value in seconds. public int? Timeout { get; set; } - public IDictionary UpstreamHeaderTransform { get; set; } + public IDictionary UpstreamHeaderTransform { get; set; } public string UpstreamHost { get; set; } public List UpstreamHttpMethod { get; set; } public string UpstreamPathTemplate { get; set; } diff --git a/src/Ocelot/Configuration/QoSOptions.cs b/src/Ocelot/Configuration/QoSOptions.cs index 77df6436b..41797e7ca 100644 --- a/src/Ocelot/Configuration/QoSOptions.cs +++ b/src/Ocelot/Configuration/QoSOptions.cs @@ -52,9 +52,9 @@ public QoSOptions( /// If using Polly version 8 or above, this value must be 1000 (1 sec) or greater. /// A (T is ) value (milliseconds). public int? TimeoutValue { get; } - public const int LowTimeout = 10; // 10 ms + public const int LowTimeout = 10; // 10 ms // TODO Double check the Polly docs public const int HighTimeout = 86_400_000; // 24 hours in milliseconds public const int DefaultTimeout = 30_000; // 30 seconds - public bool UseQos => ExceptionsAllowedBeforeBreaking > 0 || TimeoutValue > 0; + public bool UseQos => ExceptionsAllowedBeforeBreaking > 0 || (TimeoutValue.HasValue && TimeoutValue > 0); } diff --git a/src/Ocelot/Configuration/Repository/FileConfigurationPoller.cs b/src/Ocelot/Configuration/Repository/FileConfigurationPoller.cs index 969a34161..cc792e143 100644 --- a/src/Ocelot/Configuration/Repository/FileConfigurationPoller.cs +++ b/src/Ocelot/Configuration/Repository/FileConfigurationPoller.cs @@ -68,7 +68,7 @@ private async Task Poll() if (fileConfig.IsError) { - _logger.LogWarning(() =>$"error geting file config, errors are {string.Join(',', fileConfig.Errors.Select(x => x.Message))}"); + _logger.LogWarning(() => $"error geting file config, errors are {string.Join(',', fileConfig.Errors.Select(x => x.Message))}"); return; } diff --git a/src/Ocelot/Requester/MessageInvokerPool.cs b/src/Ocelot/Requester/MessageInvokerPool.cs index 323db9761..2bde575e9 100644 --- a/src/Ocelot/Requester/MessageInvokerPool.cs +++ b/src/Ocelot/Requester/MessageInvokerPool.cs @@ -10,9 +10,7 @@ public class MessageInvokerPool : IMessageInvokerPool private readonly IDelegatingHandlerHandlerFactory _handlerFactory; private readonly IOcelotLogger _logger; - public MessageInvokerPool( - IDelegatingHandlerHandlerFactory handlerFactory, - IOcelotLoggerFactory loggerFactory) + public MessageInvokerPool(IDelegatingHandlerHandlerFactory handlerFactory, IOcelotLoggerFactory loggerFactory) { _handlerFactory = handlerFactory ?? throw new ArgumentNullException(nameof(handlerFactory)); _handlersPool = new ConcurrentDictionary>(); @@ -48,17 +46,12 @@ private HttpMessageInvoker CreateMessageInvoker(DownstreamRoute downstreamRoute) baseHandler = delegatingHandler; } - if (!_timeoutMilliseconds.HasValue) - { - var qosTimeout = downstreamRoute.QosOptions.TimeoutValue; - _timeoutMilliseconds = qosTimeout.HasValue && qosTimeout.Value > 0 - ? qosTimeout.Value - : downstreamRoute.Timeout * 1000; - } + _timeoutMilliseconds ??= downstreamRoute.TimeoutMilliseconds(); + var timeout = TimeSpan.FromMilliseconds( + _timeoutMilliseconds < QoSOptions.LowTimeout ? QoSOptions.DefaultTimeout : _timeoutMilliseconds.Value); // Adding timeout handler to the top of the chain. // It's standard behavior to throw TimeoutException after the defined timeout (90 seconds by default) - var timeout = TimeSpan.FromMilliseconds(_timeoutMilliseconds.Value); var timeoutHandler = new TimeoutDelegatingHandler(timeout) { InnerHandler = baseHandler, diff --git a/test/Ocelot.AcceptanceTests/PollyQoSTests.cs b/test/Ocelot.AcceptanceTests/PollyQoSTests.cs index 573f12f0e..80ad0924c 100644 --- a/test/Ocelot.AcceptanceTests/PollyQoSTests.cs +++ b/test/Ocelot.AcceptanceTests/PollyQoSTests.cs @@ -184,7 +184,7 @@ public void Open_circuit_should_not_effect_different_route() [Trait("Bug", "1833")] public void Should_timeout_per_default_after_90_seconds() { - var defTimeoutMs = 1_000 * RoutesCreator.DefaultRequestTimeoutSeconds; // original value is 90 seconds + var defTimeoutMs = 1_000 * DownstreamRoute.DefaultTimeoutSeconds; // original value is 90 seconds defTimeoutMs = 1_000 * 3; // override value var port = PortFinder.GetRandomPort(); var route = GivenRoute(port, new QoSOptions(new FileQoSOptions()), HttpMethods.Get); diff --git a/test/Ocelot.AcceptanceTests/ServiceDiscovery/ConsulConfigurationInConsulTests.cs b/test/Ocelot.AcceptanceTests/ServiceDiscovery/ConsulConfigurationInConsulTests.cs index cfabc197a..9687e994a 100644 --- a/test/Ocelot.AcceptanceTests/ServiceDiscovery/ConsulConfigurationInConsulTests.cs +++ b/test/Ocelot.AcceptanceTests/ServiceDiscovery/ConsulConfigurationInConsulTests.cs @@ -10,6 +10,7 @@ using Ocelot.DependencyInjection; using Ocelot.Middleware; using Ocelot.Provider.Consul; +using System.Net; using System.Text; namespace Ocelot.AcceptanceTests.ServiceDiscovery @@ -70,10 +71,10 @@ public void Should_return_response_200_with_simple_url() }, }; - var fakeConsulServiceDiscoveryUrl = $"http://localhost:{consulPort}"; + var fakeConsulServiceDiscoveryUrl = DownstreamUrl(consulPort); this.Given(x => GivenThereIsAFakeConsulServiceDiscoveryProvider(fakeConsulServiceDiscoveryUrl, string.Empty)) - .And(x => x.GivenThereIsAServiceRunningOn($"http://localhost:{servicePort}", string.Empty, 200, "Hello from Laura")) + .And(x => x.GivenThereIsAServiceRunningOn(DownstreamUrl(servicePort), string.Empty, 200, "Hello from Laura")) .And(x => GivenThereIsAConfiguration(configuration)) .And(x => x.GivenOcelotIsRunningUsingConsulToStoreConfig()) .When(x => WhenIGetUrlOnTheApiGateway("/")) @@ -101,14 +102,14 @@ public void Should_load_configuration_out_of_consul() }, }; - var fakeConsulServiceDiscoveryUrl = $"http://localhost:{consulPort}"; - - var consulConfig = new FileConfiguration + var fakeConsulServiceDiscoveryUrl = DownstreamUrl(consulPort); + var consulConfig = new FileConfiguration + { + Routes = new List { - Routes = new List + new() { - new() - { + DownstreamPathTemplate = "/status", DownstreamPathTemplate = "/status", DownstreamScheme = "http", DownstreamHostAndPorts = new List @@ -141,7 +142,7 @@ public void Should_load_configuration_out_of_consul() .And(x => x.GivenOcelotIsRunningUsingConsulToStoreConfig()) .When(x => WhenIGetUrlOnTheApiGateway("/cs/status")) .Then(x => ThenTheStatusCodeShouldBe(HttpStatusCode.OK)) - .And(x => ThenTheResponseBodyShouldBe("Hello from Laura")) + .And(x => ThenTheResponseBodyShouldBe("Hello from Laura")) .BDDfy(); } @@ -164,7 +165,7 @@ public void Should_load_configuration_out_of_consul_if_it_is_changed() }, }; - var fakeConsulServiceDiscoveryUrl = $"http://localhost:{consulPort}"; + var fakeConsulServiceDiscoveryUrl = $"http://localhost:{consulPort}"; var consulConfig = new FileConfiguration { @@ -228,14 +229,14 @@ public void Should_load_configuration_out_of_consul_if_it_is_changed() }, }; - this.Given(x => GivenTheConsulConfigurationIs(consulConfig)) - .And(x => GivenThereIsAFakeConsulServiceDiscoveryProvider(fakeConsulServiceDiscoveryUrl, string.Empty)) - .And(x => x.GivenThereIsAServiceRunningOn($"http://localhost:{servicePort}", "/status", 200, "Hello from Laura")) - .And(x => GivenThereIsAConfiguration(configuration)) - .And(x => x.GivenOcelotIsRunningUsingConsulToStoreConfig()) - .And(x => WhenIGetUrlOnTheApiGateway("/cs/status")) - .And(x => ThenTheStatusCodeShouldBe(HttpStatusCode.OK)) - .And(x => ThenTheResponseBodyShouldBe("Hello from Laura")) + this.Given(x => GivenTheConsulConfigurationIs(consulConfig)) + .And(x => GivenThereIsAFakeConsulServiceDiscoveryProvider(fakeConsulServiceDiscoveryUrl, string.Empty)) + .And(x => x.GivenThereIsAServiceRunningOn(DownstreamUrl(servicePort), "/status", 200, "Hello from Laura")) + .And(x => GivenThereIsAConfiguration(configuration)) + .And(x => GivenOcelotIsRunningUsingConsulToStoreConfig()) + .And(x => WhenIGetUrlOnTheApiGateway("/cs/status")) + .And(x => ThenTheStatusCodeShouldBe(HttpStatusCode.OK)) + .And(x => ThenTheResponseBodyShouldBe("Hello from Laura")) .When(x => GivenTheConsulConfigurationIs(secondConsulConfig)) .Then(x => ThenTheConfigIsUpdatedInOcelot()) .BDDfy(); @@ -247,8 +248,8 @@ public void Should_handle_request_to_consul_for_downstream_service_and_make_requ var consulPort = PortFinder.GetRandomPort(); const string serviceName = "web"; var downstreamServicePort = PortFinder.GetRandomPort(); - var downstreamServiceOneUrl = $"http://localhost:{downstreamServicePort}"; - var fakeConsulServiceDiscoveryUrl = $"http://localhost:{consulPort}"; + var downstreamServiceOneUrl = DownstreamUrl(downstreamServicePort); + var fakeConsulServiceDiscoveryUrl = DownstreamUrl(consulPort); var serviceEntryOne = new ServiceEntry { Service = new AgentService @@ -268,7 +269,7 @@ public void Should_handle_request_to_consul_for_downstream_service_and_make_requ new() { ServiceName = serviceName, - RateLimitRule = new FileRateLimitRule + RateLimitRule = new FileRateLimitRule { EnableRateLimiting = true, ClientWhitelist = new List(), @@ -292,7 +293,7 @@ public void Should_handle_request_to_consul_for_downstream_service_and_make_requ DisableRateLimitHeaders = false, QuotaExceededMessage = string.Empty, RateLimitCounterPrefix = string.Empty, - HttpStatusCode = 428, + HttpStatusCode = (int)HttpStatusCode.PreconditionRequired, }, DownstreamScheme = "http", }, @@ -318,11 +319,11 @@ public void Should_handle_request_to_consul_for_downstream_service_and_make_requ .And(x => GivenThereIsAConfiguration(configuration)) .And(x => x.GivenOcelotIsRunningUsingConsulToStoreConfig()) .When(x => WhenIGetUrlOnTheApiGatewayMultipleTimesForRateLimit("/web/something", 1)) - .Then(x => ThenTheStatusCodeShouldBe(200)) + .Then(x => _steps.ThenTheStatusCodeShouldBe(HttpStatusCode.OK)) .When(x => WhenIGetUrlOnTheApiGatewayMultipleTimesForRateLimit("/web/something", 2)) - .Then(x => ThenTheStatusCodeShouldBe(200)) + .Then(x => ThenTheStatusCodeShouldBe(HttpStatusCode.OK)) .When(x => WhenIGetUrlOnTheApiGatewayMultipleTimesForRateLimit("/web/something", 1)) - .Then(x => ThenTheStatusCodeShouldBe(428)) + .Then(x => ThenTheStatusCodeShouldBe(HttpStatusCode.PreconditionRequired)) .BDDfy(); } @@ -378,14 +379,14 @@ private void GivenOcelotIsRunningUsingConsulToStoreConfig() Thread.Sleep(1000); } - private Task GivenThereIsAFakeConsulServiceDiscoveryProvider(string url, string serviceName) - { - _fakeConsulBuilder = TestHostBuilder.Create() - .UseUrls(url) - .UseKestrel() - .UseContentRoot(Directory.GetCurrentDirectory()) - .UseIISIntegration() - .UseUrls(url) + private Task GivenThereIsAFakeConsulServiceDiscoveryProvider(string url, string serviceName) + { + _fakeConsulBuilder = new WebHostBuilder() + .UseUrls(url) + .UseKestrel() + .UseContentRoot(Directory.GetCurrentDirectory()) + .UseIISIntegration() + .UseUrls(url) .Configure(app => { app.Run(async context => diff --git a/test/Ocelot.UnitTests/Configuration/FileModels/FileQoSOptionsTests.cs b/test/Ocelot.UnitTests/Configuration/FileModels/FileQoSOptionsTests.cs index 003e1123f..cadb4c5a4 100644 --- a/test/Ocelot.UnitTests/Configuration/FileModels/FileQoSOptionsTests.cs +++ b/test/Ocelot.UnitTests/Configuration/FileModels/FileQoSOptionsTests.cs @@ -4,14 +4,16 @@ namespace Ocelot.UnitTests.Configuration.FileModels; public class FileQoSOptionsTests { - [Fact(DisplayName = "1833: Default constructor must assign zero to the TimeoutValue property")] - public void Cstor_Default_AssignedZeroToTimeoutValue() + [Fact] + [Trait("Bug", "1833")] + [Trait("Feat", "2073")] + public void Cstor_Default_NoTimeoutValue() { // Arrange, Act var actual = new FileQoSOptions(); // Assert - Assert.Equal(0, actual.TimeoutValue); + Assert.Null(actual.TimeoutValue); } [Fact] diff --git a/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs b/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs index 7298f5e43..a84a106a6 100644 --- a/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs +++ b/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs @@ -109,8 +109,11 @@ public void Should_log_if_ignoring_ssl_errors() .WithUpstreamPathTemplate(new UpstreamPathTemplateBuilder().WithOriginalValue(string.Empty).Build()) .WithQosOptions(new QoSOptionsBuilder().Build()) .WithDangerousAcceptAnyServerCertificateValidator(true) - .WithTimeout(RoutesCreator.DefaultRequestTimeoutSeconds) + + // The test should pass without timeout definition -> implicit default timeout + //.WithTimeout(DownstreamRoute.DefaultTimeoutSeconds) .Build(); + GivenTheFactoryReturns(new List>()); GivenAMessageInvokerPool(); GivenARequest(route); @@ -135,8 +138,11 @@ public void Should_re_use_cookies_from_container() .WithLoadBalancerKey(string.Empty) .WithUpstreamPathTemplate(new UpstreamPathTemplateBuilder().WithOriginalValue(string.Empty).Build()) .WithQosOptions(new QoSOptionsBuilder().Build()) - .WithTimeout(RoutesCreator.DefaultRequestTimeoutSeconds) + + // The test should pass without timeout definition -> implicit default timeout + //.WithTimeout(DownstreamRoute.DefaultTimeoutSeconds) .Build(); + GivenADownstreamService(); GivenTheFactoryReturns(new List>()); GivenAMessageInvokerPool(); From 62f8dcd9a225ee03fc4175f4fe990522fc0b0b5a Mon Sep 17 00:00:00 2001 From: Raman Maksimchuk Date: Mon, 14 Oct 2024 19:58:09 +0300 Subject: [PATCH 35/50] Reduce timeouts in unit tests to improve execution speed --- .../Requester/MessageInvokerPoolTests.cs | 29 ++++++++++++------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs b/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs index a84a106a6..fa91d925d 100644 --- a/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs +++ b/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs @@ -11,6 +11,7 @@ using Ocelot.Requester; using Ocelot.Responses; using System.Diagnostics; +using System.Threading.Tasks; namespace Ocelot.UnitTests.Requester; @@ -159,8 +160,8 @@ public void Should_re_use_cookies_from_container() [Theory] [Trait("Bug", "1833")] - [InlineData(5, 5)] - [InlineData(10, 10)] + [InlineData(1, 1)] + [InlineData(3, 3)] public void SendAsync_TimeoutValueInQosOptions_ThrowTimeoutException(int qosTimeout, int expectedSeconds) { // Arrange @@ -180,14 +181,15 @@ public void SendAsync_TimeoutValueInQosOptions_ThrowTimeoutException(int qosTime GivenARequest(route); // Act, Assert - WhenICallTheClientWillThrowAfterTimeout(TimeSpan.FromSeconds(expectedSeconds)); + var watcher = WhenICallTheClientWillThrowAfterTimeout(TimeSpan.FromSeconds(expectedSeconds)); + Assert.True(watcher.Elapsed > TimeSpan.FromSeconds(expectedSeconds)); } - + [Theory] [Trait("PR", "2073")] [Trait("Feat", "1314 1869")] - [InlineData(5)] - [InlineData(10)] + [InlineData(1)] + [InlineData(3)] public void SendAsync_TimeoutValueInRoute_ThrowTimeoutExceptionAfterRouteTimeout(int timeoutSeconds) { // Arrange @@ -207,14 +209,15 @@ public void SendAsync_TimeoutValueInRoute_ThrowTimeoutExceptionAfterRouteTimeout GivenARequest(route); // Act, Assert - WhenICallTheClientWillThrowAfterTimeout(TimeSpan.FromSeconds(timeoutSeconds)); + var watcher = WhenICallTheClientWillThrowAfterTimeout(TimeSpan.FromSeconds(timeoutSeconds)); + Assert.True(watcher.Elapsed > TimeSpan.FromSeconds(timeoutSeconds)); } [Theory] [Trait("PR", "2073")] [Trait("Feat", "1314 1869")] - [InlineData(5, 6)] - [InlineData(10, 12)] + [InlineData(1, 2)] + [InlineData(3, 4)] public void SendAsync_TimeoutValueInQosOptionsIsLessThanRouteTimeout_ThrowTimeoutExceptionAfterQoSTimeout(int qosTimeout, int routeTimeout) { // Arrange @@ -234,7 +237,9 @@ public void SendAsync_TimeoutValueInQosOptionsIsLessThanRouteTimeout_ThrowTimeou GivenARequest(route); // Act, Assert - WhenICallTheClientWillThrowAfterTimeout(TimeSpan.FromSeconds(qosTimeout)); + var watcher = WhenICallTheClientWillThrowAfterTimeout(TimeSpan.FromSeconds(qosTimeout)); + watcher.Elapsed.ShouldBeGreaterThan(TimeSpan.FromSeconds(qosTimeout)); + watcher.Elapsed.ShouldBeLessThan(TimeSpan.FromSeconds(routeTimeout)); } private void ThenTheDangerousAcceptAnyServerCertificateValidatorWarningIsLogged() @@ -355,7 +360,7 @@ private async Task WhenICallTheClient(string url) .SendAsync(new HttpRequestMessage(HttpMethod.Get, url), CancellationToken.None); } - private async Task WhenICallTheClientWillThrowAfterTimeout(TimeSpan timeout) + private async Task WhenICallTheClientWillThrowAfterTimeout(TimeSpan timeout) { var messageInvoker = _pool.Get(_context.Items.DownstreamRoute()); var stopwatch = new Stopwatch(); @@ -376,6 +381,8 @@ private async Task WhenICallTheClientWillThrowAfterTimeout(TimeSpan timeout) Assert.True(elapsed >= timeout.Subtract(TimeSpan.FromMilliseconds(500)), $"Elapsed time {elapsed} is smaller than expected timeout {timeout} - 500 ms"); Assert.True(elapsed < timeout.Add(TimeSpan.FromMilliseconds(500)), $"Elapsed time {elapsed} is bigger than expected timeout {timeout} + 500 ms"); } + + return stopwatch; } private static void ThenTheFakeAreHandledInOrder(FakeDelegatingHandler fakeOne, FakeDelegatingHandler fakeTwo) => From 22adbae234516958e59b61a9b66b502e77b03cb9 Mon Sep 17 00:00:00 2001 From: Raman Maksimchuk Date: Mon, 14 Oct 2024 20:24:52 +0300 Subject: [PATCH 36/50] add user message --- .../Requester/MessageInvokerPoolTests.cs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs b/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs index fa91d925d..fa20ec212 100644 --- a/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs +++ b/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs @@ -181,8 +181,9 @@ public void SendAsync_TimeoutValueInQosOptions_ThrowTimeoutException(int qosTime GivenARequest(route); // Act, Assert - var watcher = WhenICallTheClientWillThrowAfterTimeout(TimeSpan.FromSeconds(expectedSeconds)); - Assert.True(watcher.Elapsed > TimeSpan.FromSeconds(expectedSeconds)); + var expected = TimeSpan.FromSeconds(expectedSeconds); + var watcher = WhenICallTheClientWillThrowAfterTimeout(expected); + Assert.True(watcher.Elapsed > expected, $"Elapsed time {watcher.Elapsed} is less than expected timeout {expected} with margin {watcher.Elapsed - expected}."); } [Theory] @@ -209,8 +210,9 @@ public void SendAsync_TimeoutValueInRoute_ThrowTimeoutExceptionAfterRouteTimeout GivenARequest(route); // Act, Assert - var watcher = WhenICallTheClientWillThrowAfterTimeout(TimeSpan.FromSeconds(timeoutSeconds)); - Assert.True(watcher.Elapsed > TimeSpan.FromSeconds(timeoutSeconds)); + var expected = TimeSpan.FromSeconds(timeoutSeconds); + var watcher = WhenICallTheClientWillThrowAfterTimeout(expected); + Assert.True(watcher.Elapsed > expected, $"Elapsed time {watcher.Elapsed} is less than expected timeout {expected} with margin {watcher.Elapsed - expected}."); } [Theory] From 87723ddd72679a4a3bf67db5538f24e7897ad682 Mon Sep 17 00:00:00 2001 From: Raman Maksimchuk Date: Mon, 14 Oct 2024 20:43:08 +0300 Subject: [PATCH 37/50] let's be more precise --- .../Requester/MessageInvokerPoolTests.cs | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs b/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs index fa20ec212..52ea07bec 100644 --- a/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs +++ b/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs @@ -364,26 +364,29 @@ private async Task WhenICallTheClient(string url) private async Task WhenICallTheClientWillThrowAfterTimeout(TimeSpan timeout) { + Exception ex = null; var messageInvoker = _pool.Get(_context.Items.DownstreamRoute()); var stopwatch = new Stopwatch(); + stopwatch.Start(); try { - stopwatch.Start(); _response = await messageInvoker .SendAsync(new HttpRequestMessage(HttpMethod.Get, "http://test.com"), CancellationToken.None); } catch (Exception e) { - stopwatch.Stop(); - var elapsed = stopwatch.Elapsed; - - // Compare the elapsed time with the given timeout - // You can use elapsed.CompareTo(timeout) or simply check if elapsed > timeout, based on your requirement - Assert.IsType(e); - Assert.True(elapsed >= timeout.Subtract(TimeSpan.FromMilliseconds(500)), $"Elapsed time {elapsed} is smaller than expected timeout {timeout} - 500 ms"); - Assert.True(elapsed < timeout.Add(TimeSpan.FromMilliseconds(500)), $"Elapsed time {elapsed} is bigger than expected timeout {timeout} + 500 ms"); + ex = e; } + Assert.NotNull(ex); + Assert.IsType(ex); + + // Compare the elapsed time with the given timeout + // You can use elapsed.CompareTo(timeout) or simply check if elapsed > timeout, based on your requirement + stopwatch.Stop(); + var elapsed = stopwatch.Elapsed; + Assert.True(elapsed >= timeout.Subtract(TimeSpan.FromMilliseconds(500)), $"Elapsed time {elapsed} is smaller than expected timeout {timeout} - 500 ms"); + Assert.True(elapsed < timeout.Add(TimeSpan.FromMilliseconds(500)), $"Elapsed time {elapsed} is bigger than expected timeout {timeout} + 500 ms"); return stopwatch; } From c4ea6fb6f09f1d35e807a5943073136df117c04f Mon Sep 17 00:00:00 2001 From: Raman Maksimchuk Date: Mon, 14 Oct 2024 21:09:34 +0300 Subject: [PATCH 38/50] 100ms margin should be fine --- .../Requester/MessageInvokerPoolTests.cs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs b/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs index 52ea07bec..b2520694b 100644 --- a/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs +++ b/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs @@ -182,7 +182,7 @@ public void SendAsync_TimeoutValueInQosOptions_ThrowTimeoutException(int qosTime // Act, Assert var expected = TimeSpan.FromSeconds(expectedSeconds); - var watcher = WhenICallTheClientWillThrowAfterTimeout(expected); + var watcher = WhenICallTheClientWillThrowAfterTimeout(expected, 100); Assert.True(watcher.Elapsed > expected, $"Elapsed time {watcher.Elapsed} is less than expected timeout {expected} with margin {watcher.Elapsed - expected}."); } @@ -211,7 +211,7 @@ public void SendAsync_TimeoutValueInRoute_ThrowTimeoutExceptionAfterRouteTimeout // Act, Assert var expected = TimeSpan.FromSeconds(timeoutSeconds); - var watcher = WhenICallTheClientWillThrowAfterTimeout(expected); + var watcher = WhenICallTheClientWillThrowAfterTimeout(expected, 100); Assert.True(watcher.Elapsed > expected, $"Elapsed time {watcher.Elapsed} is less than expected timeout {expected} with margin {watcher.Elapsed - expected}."); } @@ -239,7 +239,7 @@ public void SendAsync_TimeoutValueInQosOptionsIsLessThanRouteTimeout_ThrowTimeou GivenARequest(route); // Act, Assert - var watcher = WhenICallTheClientWillThrowAfterTimeout(TimeSpan.FromSeconds(qosTimeout)); + var watcher = WhenICallTheClientWillThrowAfterTimeout(TimeSpan.FromSeconds(qosTimeout), 100); watcher.Elapsed.ShouldBeGreaterThan(TimeSpan.FromSeconds(qosTimeout)); watcher.Elapsed.ShouldBeLessThan(TimeSpan.FromSeconds(routeTimeout)); } @@ -362,7 +362,7 @@ private async Task WhenICallTheClient(string url) .SendAsync(new HttpRequestMessage(HttpMethod.Get, url), CancellationToken.None); } - private async Task WhenICallTheClientWillThrowAfterTimeout(TimeSpan timeout) + private async Task WhenICallTheClientWillThrowAfterTimeout(TimeSpan timeout, int marginMilliseconds) { Exception ex = null; var messageInvoker = _pool.Get(_context.Items.DownstreamRoute()); @@ -385,8 +385,9 @@ private async Task WhenICallTheClientWillThrowAfterTimeout(TimeSpan t // You can use elapsed.CompareTo(timeout) or simply check if elapsed > timeout, based on your requirement stopwatch.Stop(); var elapsed = stopwatch.Elapsed; - Assert.True(elapsed >= timeout.Subtract(TimeSpan.FromMilliseconds(500)), $"Elapsed time {elapsed} is smaller than expected timeout {timeout} - 500 ms"); - Assert.True(elapsed < timeout.Add(TimeSpan.FromMilliseconds(500)), $"Elapsed time {elapsed} is bigger than expected timeout {timeout} + 500 ms"); + var margin = TimeSpan.FromMilliseconds(marginMilliseconds); + Assert.True(elapsed >= timeout.Subtract(margin), $"Elapsed time {elapsed} is smaller than expected timeout {timeout} - {marginMilliseconds}ms"); + Assert.True(elapsed < timeout.Add(margin), $"Elapsed time {elapsed} is bigger than expected timeout {timeout} + {marginMilliseconds}ms"); return stopwatch; } From 52d84e36c29297ef875f12212e7c6ff9094d9cbc Mon Sep 17 00:00:00 2001 From: Raman Maksimchuk Date: Tue, 15 Oct 2024 11:54:16 +0300 Subject: [PATCH 39/50] Assert timeout precisely --- .../Requester/MessageInvokerPoolTests.cs | 32 +++++++++++++------ 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs b/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs index b2520694b..f71ed55b3 100644 --- a/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs +++ b/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs @@ -12,6 +12,7 @@ using Ocelot.Responses; using System.Diagnostics; using System.Threading.Tasks; +using Xunit.Sdk; namespace Ocelot.UnitTests.Requester; @@ -158,6 +159,22 @@ public void Should_re_use_cookies_from_container() ThenTheResponseIsOk(); } + private static TimeSpan C10ms => TimeSpan.FromMilliseconds(10); + private static void AssertTimeoutPrecisely(Stopwatch watcher, TimeSpan expected, TimeSpan? precision = null) + { + precision ??= C10ms; + TimeSpan elapsed = watcher.Elapsed, margin = elapsed - expected; + try + { + Assert.True(elapsed >= expected, $"Elapsed time {elapsed} is less than expected timeout {expected} with margin {margin}."); + } + catch (TrueException) + { + // The elapsed time is approximately 0.998xxx or 2.99xxx, with a 10ms margin of precision accepted. + Assert.True(elapsed.Add(precision.Value) >= expected, $"Elapsed time {elapsed} is less than expected timeout {expected} with margin {margin} which module is >= {precision.Value.Milliseconds}ms."); + } + } + [Theory] [Trait("Bug", "1833")] [InlineData(1, 1)] @@ -183,7 +200,7 @@ public void SendAsync_TimeoutValueInQosOptions_ThrowTimeoutException(int qosTime // Act, Assert var expected = TimeSpan.FromSeconds(expectedSeconds); var watcher = WhenICallTheClientWillThrowAfterTimeout(expected, 100); - Assert.True(watcher.Elapsed > expected, $"Elapsed time {watcher.Elapsed} is less than expected timeout {expected} with margin {watcher.Elapsed - expected}."); + AssertTimeoutPrecisely(watcher, expected); } [Theory] @@ -212,7 +229,7 @@ public void SendAsync_TimeoutValueInRoute_ThrowTimeoutExceptionAfterRouteTimeout // Act, Assert var expected = TimeSpan.FromSeconds(timeoutSeconds); var watcher = WhenICallTheClientWillThrowAfterTimeout(expected, 100); - Assert.True(watcher.Elapsed > expected, $"Elapsed time {watcher.Elapsed} is less than expected timeout {expected} with margin {watcher.Elapsed - expected}."); + AssertTimeoutPrecisely(watcher, expected); } [Theory] @@ -239,9 +256,10 @@ public void SendAsync_TimeoutValueInQosOptionsIsLessThanRouteTimeout_ThrowTimeou GivenARequest(route); // Act, Assert - var watcher = WhenICallTheClientWillThrowAfterTimeout(TimeSpan.FromSeconds(qosTimeout), 100); - watcher.Elapsed.ShouldBeGreaterThan(TimeSpan.FromSeconds(qosTimeout)); + var expected = TimeSpan.FromSeconds(qosTimeout); + var watcher = WhenICallTheClientWillThrowAfterTimeout(expected, 100); watcher.Elapsed.ShouldBeLessThan(TimeSpan.FromSeconds(routeTimeout)); + AssertTimeoutPrecisely(watcher, expected); } private void ThenTheDangerousAcceptAnyServerCertificateValidatorWarningIsLogged() @@ -364,7 +382,6 @@ private async Task WhenICallTheClient(string url) private async Task WhenICallTheClientWillThrowAfterTimeout(TimeSpan timeout, int marginMilliseconds) { - Exception ex = null; var messageInvoker = _pool.Get(_context.Items.DownstreamRoute()); var stopwatch = new Stopwatch(); stopwatch.Start(); @@ -375,12 +392,9 @@ private async Task WhenICallTheClientWillThrowAfterTimeout(TimeSpan t } catch (Exception e) { - ex = e; + Assert.IsType(e); } - Assert.NotNull(ex); - Assert.IsType(ex); - // Compare the elapsed time with the given timeout // You can use elapsed.CompareTo(timeout) or simply check if elapsed > timeout, based on your requirement stopwatch.Stop(); From b6741bc9cd6b2db7159906d38f8106e25df88fdd Mon Sep 17 00:00:00 2001 From: Raman Maksimchuk Date: Tue, 15 Oct 2024 14:25:36 +0300 Subject: [PATCH 40/50] Test each failed attempt with an incremented margin. Add the NoWait test helper, which is based on the Retry pattern. --- .../Infrastructure/DesignPatterns/Retry.cs | 9 ++++++-- test/Ocelot.Testing/Ocelot.Testing.csproj | 2 +- test/Ocelot.Testing/TestRetry.cs | 21 +++++++++++++++++++ .../Requester/MessageInvokerPoolTests.cs | 13 +++++++++--- 4 files changed, 39 insertions(+), 6 deletions(-) create mode 100644 test/Ocelot.Testing/TestRetry.cs diff --git a/src/Ocelot/Infrastructure/DesignPatterns/Retry.cs b/src/Ocelot/Infrastructure/DesignPatterns/Retry.cs index be402e114..6967bd864 100644 --- a/src/Ocelot/Infrastructure/DesignPatterns/Retry.cs +++ b/src/Ocelot/Infrastructure/DesignPatterns/Retry.cs @@ -35,6 +35,11 @@ public static TResult Operation( int retryTimes = DefaultRetryTimes, int waitTime = DefaultWaitTimeMilliseconds, IOcelotLogger logger = null) { + if (waitTime < 0) + { + waitTime = 0; // 0 means no thread sleeping + } + for (int n = 1; n < retryTimes; n++) { TResult result; @@ -92,7 +97,7 @@ public static async Task OperationAsync( catch (Exception e) { logger?.LogError(() => GetMessage(operation, n, $"Caught exception of the {e.GetType()} type -> Message: {e.Message}."), e); - await Task.Delay(waitTime); + await (waitTime > 0 ? Task.Delay(waitTime) : Task.CompletedTask); continue; // the result is unknown, so continue to retry } @@ -100,7 +105,7 @@ public static async Task OperationAsync( if (predicate?.Invoke(result) == true) { logger?.LogWarning(() => GetMessage(operation, n, $"The predicate has identified erroneous state in the returned result. For further details, implement logging of the result's value or properties within the predicate method.")); - await Task.Delay(waitTime); + await (waitTime > 0 ? Task.Delay(waitTime) : Task.CompletedTask); continue; // on erroneous state } diff --git a/test/Ocelot.Testing/Ocelot.Testing.csproj b/test/Ocelot.Testing/Ocelot.Testing.csproj index b7a4f4253..1f3bdc25a 100644 --- a/test/Ocelot.Testing/Ocelot.Testing.csproj +++ b/test/Ocelot.Testing/Ocelot.Testing.csproj @@ -4,7 +4,7 @@ 0.0.0-dev net6.0;net7.0;net8.0 enable - enable + disable diff --git a/test/Ocelot.Testing/TestRetry.cs b/test/Ocelot.Testing/TestRetry.cs new file mode 100644 index 000000000..34c796f9e --- /dev/null +++ b/test/Ocelot.Testing/TestRetry.cs @@ -0,0 +1,21 @@ +using Ocelot.Infrastructure.DesignPatterns; +using Ocelot.Logging; + +namespace Ocelot.Testing; + +public static class TestRetry +{ + public static TResult NoWait( + Func operation, + Predicate predicate = null, + int retryTimes = Retry.DefaultRetryTimes, + IOcelotLogger logger = null) + => Retry.Operation(operation, predicate, retryTimes, 0, logger); + + public static async Task NoWaitAsync( + Func> operation, + Predicate predicate = null, + int retryTimes = Retry.DefaultRetryTimes, + IOcelotLogger logger = null) + => await Retry.OperationAsync(operation, predicate, retryTimes, 0, logger); +} diff --git a/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs b/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs index f71ed55b3..09e8eb7c6 100644 --- a/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs +++ b/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs @@ -10,6 +10,7 @@ using Ocelot.Request.Middleware; using Ocelot.Requester; using Ocelot.Responses; +using Ocelot.Testing; using System.Diagnostics; using System.Threading.Tasks; using Xunit.Sdk; @@ -198,8 +199,10 @@ public void SendAsync_TimeoutValueInQosOptions_ThrowTimeoutException(int qosTime GivenARequest(route); // Act, Assert + int marginMs = 50; var expected = TimeSpan.FromSeconds(expectedSeconds); - var watcher = WhenICallTheClientWillThrowAfterTimeout(expected, 100); + var watcher = TestRetry.NoWait( + () => WhenICallTheClientWillThrowAfterTimeout(expected, marginMs *= 2)); // call up to 3 times with margins 100, 200, 400 AssertTimeoutPrecisely(watcher, expected); } @@ -227,8 +230,10 @@ public void SendAsync_TimeoutValueInRoute_ThrowTimeoutExceptionAfterRouteTimeout GivenARequest(route); // Act, Assert + int marginMs = 50; var expected = TimeSpan.FromSeconds(timeoutSeconds); - var watcher = WhenICallTheClientWillThrowAfterTimeout(expected, 100); + var watcher = TestRetry.NoWait( + () => WhenICallTheClientWillThrowAfterTimeout(expected, marginMs *= 2)); // call up to 3 times with margins 100, 200, 400 AssertTimeoutPrecisely(watcher, expected); } @@ -256,8 +261,10 @@ public void SendAsync_TimeoutValueInQosOptionsIsLessThanRouteTimeout_ThrowTimeou GivenARequest(route); // Act, Assert + int marginMs = 50; var expected = TimeSpan.FromSeconds(qosTimeout); - var watcher = WhenICallTheClientWillThrowAfterTimeout(expected, 100); + var watcher = TestRetry.NoWait( + () => WhenICallTheClientWillThrowAfterTimeout(expected, marginMs *= 2)); // call up to 3 times with margins 100, 200, 400 watcher.Elapsed.ShouldBeLessThan(TimeSpan.FromSeconds(routeTimeout)); AssertTimeoutPrecisely(watcher, expected); } From a0a7c49e51a5cb2c1c7df7d760bc740e7add705f Mon Sep 17 00:00:00 2001 From: Raman Maksimchuk Date: Wed, 16 Oct 2024 23:15:43 +0300 Subject: [PATCH 41/50] Fix build after rebasing --- test/Ocelot.Testing/TestRetry.cs | 4 +-- .../Requester/MessageInvokerPoolTests.cs | 30 +++++++++---------- 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/test/Ocelot.Testing/TestRetry.cs b/test/Ocelot.Testing/TestRetry.cs index 34c796f9e..29904ddad 100644 --- a/test/Ocelot.Testing/TestRetry.cs +++ b/test/Ocelot.Testing/TestRetry.cs @@ -12,10 +12,10 @@ public static TResult NoWait( IOcelotLogger logger = null) => Retry.Operation(operation, predicate, retryTimes, 0, logger); - public static async Task NoWaitAsync( + public static Task NoWaitAsync( Func> operation, Predicate predicate = null, int retryTimes = Retry.DefaultRetryTimes, IOcelotLogger logger = null) - => await Retry.OperationAsync(operation, predicate, retryTimes, 0, logger); + => Retry.OperationAsync(operation, predicate, retryTimes, 0, logger); } diff --git a/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs b/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs index 09e8eb7c6..b4cccf89e 100644 --- a/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs +++ b/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs @@ -12,7 +12,6 @@ using Ocelot.Responses; using Ocelot.Testing; using System.Diagnostics; -using System.Threading.Tasks; using Xunit.Sdk; namespace Ocelot.UnitTests.Requester; @@ -72,7 +71,7 @@ public void If_calling_two_different_downstream_routes_should_return_different_m [Fact] [Trait("PR", "1824")] - public void If_two_delegating_handlers_are_defined_then_these_should_be_call_in_order() + public async Task If_two_delegating_handlers_are_defined_then_these_should_be_call_in_order() { // Arrange var fakeOne = new FakeDelegatingHandler(); @@ -88,7 +87,7 @@ public void If_two_delegating_handlers_are_defined_then_these_should_be_call_in_ GivenARequest(); // Act - WhenICallTheClient("http://www.bbc.co.uk"); + await WhenICallTheClient("http://www.bbc.co.uk"); // Assert ThenTheFakeAreHandledInOrder(fakeOne, fakeTwo); @@ -100,7 +99,7 @@ public void If_two_delegating_handlers_are_defined_then_these_should_be_call_in_ [Fact] [Trait("PR", "1824")] - public void Should_log_if_ignoring_ssl_errors() + public async Task Should_log_if_ignoring_ssl_errors() { // Arrange var qosOptions = new QoSOptionsBuilder() @@ -122,7 +121,7 @@ public void Should_log_if_ignoring_ssl_errors() GivenARequest(route); // Act - WhenICallTheClient("http://www.bbc.co.uk"); + await WhenICallTheClient("http://www.bbc.co.uk"); // Assert ThenTheDangerousAcceptAnyServerCertificateValidatorWarningIsLogged(); @@ -130,7 +129,7 @@ public void Should_log_if_ignoring_ssl_errors() [Fact] [Trait("PR", "1824")] - public void Should_re_use_cookies_from_container() + public async Task Should_re_use_cookies_from_container() { // Arrange var qosOptions = new QoSOptionsBuilder() @@ -152,11 +151,11 @@ public void Should_re_use_cookies_from_container() GivenARequest(route); // Act, Assert 1 - WhenICallTheClient("http://localhost:5003"); + await WhenICallTheClient("http://localhost:5003"); ThenTheCookieIsSet(); // Act, Assert 2 - WhenICallTheClient("http://localhost:5003"); + await WhenICallTheClient("http://localhost:5003"); ThenTheResponseIsOk(); } @@ -180,7 +179,7 @@ private static void AssertTimeoutPrecisely(Stopwatch watcher, TimeSpan expected, [Trait("Bug", "1833")] [InlineData(1, 1)] [InlineData(3, 3)] - public void SendAsync_TimeoutValueInQosOptions_ThrowTimeoutException(int qosTimeout, int expectedSeconds) + public async Task SendAsync_TimeoutValueInQosOptions_ThrowTimeoutException(int qosTimeout, int expectedSeconds) { // Arrange var qosOptions = new QoSOptionsBuilder() @@ -201,7 +200,7 @@ public void SendAsync_TimeoutValueInQosOptions_ThrowTimeoutException(int qosTime // Act, Assert int marginMs = 50; var expected = TimeSpan.FromSeconds(expectedSeconds); - var watcher = TestRetry.NoWait( + var watcher = await TestRetry.NoWaitAsync( () => WhenICallTheClientWillThrowAfterTimeout(expected, marginMs *= 2)); // call up to 3 times with margins 100, 200, 400 AssertTimeoutPrecisely(watcher, expected); } @@ -211,7 +210,7 @@ public void SendAsync_TimeoutValueInQosOptions_ThrowTimeoutException(int qosTime [Trait("Feat", "1314 1869")] [InlineData(1)] [InlineData(3)] - public void SendAsync_TimeoutValueInRoute_ThrowTimeoutExceptionAfterRouteTimeout(int timeoutSeconds) + public async Task SendAsync_TimeoutValueInRoute_ThrowTimeoutExceptionAfterRouteTimeout(int timeoutSeconds) { // Arrange var qosOptions = new QoSOptionsBuilder() @@ -232,7 +231,7 @@ public void SendAsync_TimeoutValueInRoute_ThrowTimeoutExceptionAfterRouteTimeout // Act, Assert int marginMs = 50; var expected = TimeSpan.FromSeconds(timeoutSeconds); - var watcher = TestRetry.NoWait( + var watcher = await TestRetry.NoWaitAsync( () => WhenICallTheClientWillThrowAfterTimeout(expected, marginMs *= 2)); // call up to 3 times with margins 100, 200, 400 AssertTimeoutPrecisely(watcher, expected); } @@ -242,7 +241,7 @@ public void SendAsync_TimeoutValueInRoute_ThrowTimeoutExceptionAfterRouteTimeout [Trait("Feat", "1314 1869")] [InlineData(1, 2)] [InlineData(3, 4)] - public void SendAsync_TimeoutValueInQosOptionsIsLessThanRouteTimeout_ThrowTimeoutExceptionAfterQoSTimeout(int qosTimeout, int routeTimeout) + public async Task SendAsync_TimeoutValueInQosOptionsIsLessThanRouteTimeout_ThrowTimeoutExceptionAfterQoSTimeout(int qosTimeout, int routeTimeout) { // Arrange var qosOptions = new QoSOptionsBuilder() @@ -263,7 +262,7 @@ public void SendAsync_TimeoutValueInQosOptionsIsLessThanRouteTimeout_ThrowTimeou // Act, Assert int marginMs = 50; var expected = TimeSpan.FromSeconds(qosTimeout); - var watcher = TestRetry.NoWait( + var watcher = await TestRetry.NoWaitAsync( () => WhenICallTheClientWillThrowAfterTimeout(expected, marginMs *= 2)); // call up to 3 times with margins 100, 200, 400 watcher.Elapsed.ShouldBeLessThan(TimeSpan.FromSeconds(routeTimeout)); AssertTimeoutPrecisely(watcher, expected); @@ -367,8 +366,7 @@ private void WhenGettingMessageInvokerForBothRoutes() private void GivenARequest(string url) => GivenARequestWithAUrlAndMethod(_downstreamRoute1, url, HttpMethod.Get); - private void GivenARequest() => - GivenARequestWithAUrlAndMethod(_downstreamRoute1, "http://localhost:5003", HttpMethod.Get); + private void GivenARequest() => GivenARequestWithAUrlAndMethod(_downstreamRoute1, "http://localhost:5003", HttpMethod.Get); private void GivenARequestWithAUrlAndMethod(DownstreamRoute downstream, string url, HttpMethod method) { From a51bb31664756179107b2f717e2b8c76b997927e Mon Sep 17 00:00:00 2001 From: Raman Maksimchuk Date: Thu, 17 Oct 2024 19:51:36 +0300 Subject: [PATCH 42/50] `MessageInvokerPool` shouldn't have internal state for timeouts because it is singleton service! Make default timeout properties static. --- .../PollyQoSResiliencePipelineProvider.cs | 5 +---- src/Ocelot/Configuration/DownstreamRoute.cs | 4 +++- src/Ocelot/Configuration/QoSOptions.cs | 5 ++++- src/Ocelot/Requester/MessageInvokerPool.cs | 7 ++----- 4 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/Ocelot.Provider.Polly/PollyQoSResiliencePipelineProvider.cs b/src/Ocelot.Provider.Polly/PollyQoSResiliencePipelineProvider.cs index 28f742241..ebb959719 100644 --- a/src/Ocelot.Provider.Polly/PollyQoSResiliencePipelineProvider.cs +++ b/src/Ocelot.Provider.Polly/PollyQoSResiliencePipelineProvider.cs @@ -6,7 +6,6 @@ using Polly.CircuitBreaker; using Polly.Registry; using Polly.Timeout; -using System; using System.Net; namespace Ocelot.Provider.Polly; @@ -20,8 +19,6 @@ public class PollyQoSResiliencePipelineProvider : IPollyQoSResiliencePipelinePro private readonly IOcelotLogger _logger; private readonly FileGlobalConfiguration _global; - public const int DefaultQoSTimeoutMilliseconds = 40_000; - public PollyQoSResiliencePipelineProvider( IOcelotLoggerFactory loggerFactory, ResiliencePipelineRegistry registry, @@ -125,7 +122,7 @@ protected virtual ResiliencePipelineBuilder ConfigureTimeou { int? timeoutMs = route?.QosOptions?.TimeoutValue ?? _global?.QoSOptions?.TimeoutValue - ?? DefaultQoSTimeoutMilliseconds; // TODO Need team's consensus!!! Prefer QoSOptions.DefaultTimeout ? + ?? QoSOptions.DefaultTimeout; // 0 means No option! if (!timeoutMs.HasValue || timeoutMs.Value <= 0) diff --git a/src/Ocelot/Configuration/DownstreamRoute.cs b/src/Ocelot/Configuration/DownstreamRoute.cs index 70a44dd37..e9be2261b 100644 --- a/src/Ocelot/Configuration/DownstreamRoute.cs +++ b/src/Ocelot/Configuration/DownstreamRoute.cs @@ -145,7 +145,9 @@ public string Name() => string.IsNullOrEmpty(ServiceName) && !UseServiceDiscover public int? Timeout { get; } /// Defines the default timeout in seconds for all routes, applicable at both the Route-level and globally. - public const int DefaultTimeoutSeconds = 90; + /// By default, initialized to 90 seconds. + /// An value, default. + public static int DefaultTimeoutSeconds { get; set; } = 90; /// /// Calculates timeout in milliseconds based on QoS options with applying default timeout values. diff --git a/src/Ocelot/Configuration/QoSOptions.cs b/src/Ocelot/Configuration/QoSOptions.cs index 41797e7ca..06ad03d3b 100644 --- a/src/Ocelot/Configuration/QoSOptions.cs +++ b/src/Ocelot/Configuration/QoSOptions.cs @@ -54,7 +54,10 @@ public QoSOptions( public int? TimeoutValue { get; } public const int LowTimeout = 10; // 10 ms // TODO Double check the Polly docs public const int HighTimeout = 86_400_000; // 24 hours in milliseconds - public const int DefaultTimeout = 30_000; // 30 seconds + + /// The default is set to 30 seconds, but it can be globally assigned any value. + /// An value. + public static int DefaultTimeout { get; set; } = 30_000; public bool UseQos => ExceptionsAllowedBeforeBreaking > 0 || (TimeoutValue.HasValue && TimeoutValue > 0); } diff --git a/src/Ocelot/Requester/MessageInvokerPool.cs b/src/Ocelot/Requester/MessageInvokerPool.cs index 2bde575e9..f2baf88d4 100644 --- a/src/Ocelot/Requester/MessageInvokerPool.cs +++ b/src/Ocelot/Requester/MessageInvokerPool.cs @@ -32,8 +32,6 @@ public HttpMessageInvoker Get(DownstreamRoute downstreamRoute) public void Clear() => _handlersPool.Clear(); - private int? _timeoutMilliseconds = null; - private HttpMessageInvoker CreateMessageInvoker(DownstreamRoute downstreamRoute) { var baseHandler = CreateHandler(downstreamRoute); @@ -46,9 +44,8 @@ private HttpMessageInvoker CreateMessageInvoker(DownstreamRoute downstreamRoute) baseHandler = delegatingHandler; } - _timeoutMilliseconds ??= downstreamRoute.TimeoutMilliseconds(); - var timeout = TimeSpan.FromMilliseconds( - _timeoutMilliseconds < QoSOptions.LowTimeout ? QoSOptions.DefaultTimeout : _timeoutMilliseconds.Value); + int milliseconds = downstreamRoute.TimeoutMilliseconds(); + var timeout = TimeSpan.FromMilliseconds(milliseconds); // Adding timeout handler to the top of the chain. // It's standard behavior to throw TimeoutException after the defined timeout (90 seconds by default) From 6950f0e2f56a167c4b3ea225657d30df41a4539b Mon Sep 17 00:00:00 2001 From: Raman Maksimchuk Date: Thu, 17 Oct 2024 19:54:00 +0300 Subject: [PATCH 43/50] Review logic for disabled test parallelization --- .../ClaimsToHeadersForwardingTests.cs | 2 - test/Ocelot.AcceptanceTests/PollyQoSTests.cs | 47 +++++++++---------- .../Ocelot.AcceptanceTests/SequentialTests.cs | 6 +-- 3 files changed, 25 insertions(+), 30 deletions(-) diff --git a/test/Ocelot.AcceptanceTests/ClaimsToHeadersForwardingTests.cs b/test/Ocelot.AcceptanceTests/ClaimsToHeadersForwardingTests.cs index 1ca691cf2..2c11c8f46 100644 --- a/test/Ocelot.AcceptanceTests/ClaimsToHeadersForwardingTests.cs +++ b/test/Ocelot.AcceptanceTests/ClaimsToHeadersForwardingTests.cs @@ -8,8 +8,6 @@ using Ocelot.Configuration.File; using System.Security.Claims; -[assembly: CollectionBehavior(DisableTestParallelization = true)] - namespace Ocelot.AcceptanceTests { public class ClaimsToHeadersForwardingTests : IDisposable diff --git a/test/Ocelot.AcceptanceTests/PollyQoSTests.cs b/test/Ocelot.AcceptanceTests/PollyQoSTests.cs index 80ad0924c..9d526c41c 100644 --- a/test/Ocelot.AcceptanceTests/PollyQoSTests.cs +++ b/test/Ocelot.AcceptanceTests/PollyQoSTests.cs @@ -1,9 +1,6 @@ using Microsoft.AspNetCore.Http; using Ocelot.Configuration; -using Ocelot.Configuration.Creator; using Ocelot.Configuration.File; -using Ocelot.Requester; -using System.Reflection; namespace Ocelot.AcceptanceTests; @@ -180,31 +177,33 @@ public void Open_circuit_should_not_effect_different_route() .BDDfy(); } - [Fact] + // TODO: If failed in parallel execution mode, switch to SequentialTests + // This issue may arise when transitioning all tests to parallel execution + // This test must be sequential because of usage of the static DownstreamRoute.DefaultTimeoutSeconds + [Fact] [Trait("Bug", "1833")] public void Should_timeout_per_default_after_90_seconds() { - var defTimeoutMs = 1_000 * DownstreamRoute.DefaultTimeoutSeconds; // original value is 90 seconds - defTimeoutMs = 1_000 * 3; // override value - var port = PortFinder.GetRandomPort(); - var route = GivenRoute(port, new QoSOptions(new FileQoSOptions()), HttpMethods.Get); - var configuration = GivenConfiguration(route); - - this.Given(x => x.GivenThereIsAServiceRunningOn(port, HttpStatusCode.Created, string.Empty, defTimeoutMs + 500)) // 3.5s > 3s -> ServiceUnavailable - .And(x => GivenThereIsAConfiguration(configuration)) - .And(x => GivenOcelotIsRunningWithPolly()) - .And(x => GivenIHackDefaultTimeoutValue(defTimeoutMs)) // after 3 secs -> Timeout exception aka request cancellation - .When(x => WhenIGetUrlOnTheApiGateway("/")) - .Then(x => ThenTheStatusCodeShouldBe(HttpStatusCode.ServiceUnavailable)) - .BDDfy(); + int originalValue = DownstreamRoute.DefaultTimeoutSeconds; // original value is 90 seconds + try + { + DownstreamRoute.DefaultTimeoutSeconds = 3; // override original value + var defTimeoutMs = 1_000 * DownstreamRoute.DefaultTimeoutSeconds; + var port = PortFinder.GetRandomPort(); + var route = GivenRoute(port, new QoSOptions(new FileQoSOptions()), HttpMethods.Get); + var configuration = GivenConfiguration(route); + this.Given(x => x.GivenThereIsAServiceRunningOn(port, HttpStatusCode.Created, string.Empty, defTimeoutMs + 500)) // 3.5s > 3s -> ServiceUnavailable + .And(x => GivenThereIsAConfiguration(configuration)) + .And(x => GivenOcelotIsRunningWithPolly()) + .When(x => WhenIGetUrlOnTheApiGateway("/")) + .Then(x => ThenTheStatusCodeShouldBe(HttpStatusCode.ServiceUnavailable)) // after 3 secs -> Timeout exception aka request cancellation + .BDDfy(); + } + finally + { + DownstreamRoute.DefaultTimeoutSeconds = originalValue; + } } - - private void GivenIHackDefaultTimeoutValue(int defaultTimeoutMs) - { - var field = typeof(MessageInvokerPool).GetField("_timeoutMilliseconds", BindingFlags.NonPublic | BindingFlags.Instance); - var service = _ocelotServer.Services.GetService(typeof(IMessageInvokerPool)); - field.SetValue(service, defaultTimeoutMs); // hack the value of default 90 seconds - } private static void GivenIWaitMilliseconds(int ms) => Thread.Sleep(ms); diff --git a/test/Ocelot.AcceptanceTests/SequentialTests.cs b/test/Ocelot.AcceptanceTests/SequentialTests.cs index 19c2494a2..169833f28 100644 --- a/test/Ocelot.AcceptanceTests/SequentialTests.cs +++ b/test/Ocelot.AcceptanceTests/SequentialTests.cs @@ -1,4 +1,4 @@ -using Xunit; +[assembly: CollectionBehavior(DisableTestParallelization = true)] namespace Ocelot.AcceptanceTests; @@ -6,6 +6,4 @@ namespace Ocelot.AcceptanceTests; /// Apply to classes to disable parallelization. /// [CollectionDefinition(nameof(SequentialTests), DisableParallelization = true)] -public class SequentialTests -{ -} +public class SequentialTests { } From 47793c15535a7c1f3f7d0db44274c64ff50d1c54 Mon Sep 17 00:00:00 2001 From: Raman Maksimchuk Date: Thu, 17 Oct 2024 20:25:59 +0300 Subject: [PATCH 44/50] EOL: src/Ocelot/Configuration/Creator/RoutesCreator.cs --- .../Configuration/Creator/RoutesCreator.cs | 30 ++++++++----------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/src/Ocelot/Configuration/Creator/RoutesCreator.cs b/src/Ocelot/Configuration/Creator/RoutesCreator.cs index 5039126a0..1107f9ad6 100644 --- a/src/Ocelot/Configuration/Creator/RoutesCreator.cs +++ b/src/Ocelot/Configuration/Creator/RoutesCreator.cs @@ -1,4 +1,4 @@ -using Ocelot.Configuration.Builder; +using Ocelot.Configuration.Builder; using Ocelot.Configuration.File; namespace Ocelot.Configuration.Creator @@ -21,8 +21,8 @@ public class RoutesCreator : IRoutesCreator private readonly IRouteKeyCreator _routeKeyCreator; private readonly ISecurityOptionsCreator _securityOptionsCreator; private readonly IVersionCreator _versionCreator; - private readonly IVersionPolicyCreator _versionPolicyCreator; - private readonly IMetadataCreator _metadataCreator; + private readonly IVersionPolicyCreator _versionPolicyCreator; + private readonly IMetadataCreator _metadataCreator; public RoutesCreator( IClaimsToThingCreator claimsToThingCreator, @@ -40,7 +40,7 @@ public RoutesCreator( IRouteKeyCreator routeKeyCreator, ISecurityOptionsCreator securityOptionsCreator, IVersionCreator versionCreator, - IVersionPolicyCreator versionPolicyCreator, + IVersionPolicyCreator versionPolicyCreator, IUpstreamHeaderTemplatePatternCreator upstreamHeaderTemplatePatternCreator, IMetadataCreator metadataCreator) { @@ -60,19 +60,17 @@ public RoutesCreator( _loadBalancerOptionsCreator = loadBalancerOptionsCreator; _securityOptionsCreator = securityOptionsCreator; _versionCreator = versionCreator; - _versionPolicyCreator = versionPolicyCreator; + _versionPolicyCreator = versionPolicyCreator; _upstreamHeaderTemplatePatternCreator = upstreamHeaderTemplatePatternCreator; - _metadataCreator = metadataCreator; + _metadataCreator = metadataCreator; } public List Create(FileConfiguration fileConfiguration) { + Route CreateRoute(FileRoute route) + => SetUpRoute(route, SetUpDownstreamRoute(route, fileConfiguration.GlobalConfiguration)); return fileConfiguration.Routes - .Select(route => - { - var downstreamRoute = SetUpDownstreamRoute(route, fileConfiguration.GlobalConfiguration); - return SetUpRoute(route, downstreamRoute); - }) + .Select(CreateRoute) .ToList(); } @@ -115,11 +113,11 @@ private DownstreamRoute SetUpDownstreamRoute(FileRoute fileRoute, FileGlobalConf var downstreamHttpVersion = _versionCreator.Create(fileRoute.DownstreamHttpVersion); - var downstreamHttpVersionPolicy = _versionPolicyCreator.Create(fileRoute.DownstreamHttpVersionPolicy); + var downstreamHttpVersionPolicy = _versionPolicyCreator.Create(fileRoute.DownstreamHttpVersionPolicy); var cacheOptions = _cacheOptionsCreator.Create(fileRoute.FileCacheOptions, globalConfiguration, fileRoute.UpstreamPathTemplate, fileRoute.UpstreamHttpMethod); - var metadata = _metadataCreator.Create(fileRoute.Metadata, globalConfiguration); + var metadata = _metadataCreator.Create(fileRoute.Metadata, globalConfiguration); var route = new DownstreamRouteBuilder() .WithKey(fileRoute.Key) @@ -156,12 +154,11 @@ private DownstreamRoute SetUpDownstreamRoute(FileRoute fileRoute, FileGlobalConf .WithDangerousAcceptAnyServerCertificateValidator(fileRoute.DangerousAcceptAnyServerCertificateValidator) .WithSecurityOptions(securityOptions) .WithDownstreamHttpVersion(downstreamHttpVersion) - .WithDownstreamHttpVersionPolicy(downstreamHttpVersionPolicy) + .WithDownstreamHttpVersionPolicy(downstreamHttpVersionPolicy) .WithDownStreamHttpMethod(fileRoute.DownstreamHttpMethod) - .WithMetadata(metadata) + .WithMetadata(metadata) .WithTimeout(CreateTimeout(fileRoute, globalConfiguration)) .Build(); - return route; } @@ -177,7 +174,6 @@ private Route SetUpRoute(FileRoute fileRoute, DownstreamRoute downstreamRoutes) .WithUpstreamHost(fileRoute.UpstreamHost) .WithUpstreamHeaders(upstreamHeaderTemplates) .Build(); - return route; } } From 14de21bc32ebaef71749ef69eda9efeb915bc131 Mon Sep 17 00:00:00 2001 From: Raman Maksimchuk Date: Thu, 17 Oct 2024 20:35:15 +0300 Subject: [PATCH 45/50] EOL: src/Ocelot/Configuration/DownstreamRoute.cs --- src/Ocelot/Configuration/DownstreamRoute.cs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/Ocelot/Configuration/DownstreamRoute.cs b/src/Ocelot/Configuration/DownstreamRoute.cs index e9be2261b..25095b29e 100644 --- a/src/Ocelot/Configuration/DownstreamRoute.cs +++ b/src/Ocelot/Configuration/DownstreamRoute.cs @@ -1,4 +1,4 @@ -using Ocelot.Configuration.Creator; +using Ocelot.Configuration.Creator; using Ocelot.Values; namespace Ocelot.Configuration @@ -39,7 +39,7 @@ public DownstreamRoute( bool dangerousAcceptAnyServerCertificateValidator, SecurityOptions securityOptions, string downstreamHttpMethod, - Version downstreamHttpVersion, + Version downstreamHttpVersion, HttpVersionPolicy downstreamHttpVersionPolicy, Dictionary upstreamHeaders, MetadataOptions metadataOptions, @@ -78,10 +78,10 @@ public DownstreamRoute( AddHeadersToUpstream = addHeadersToUpstream; SecurityOptions = securityOptions; DownstreamHttpMethod = downstreamHttpMethod; - DownstreamHttpVersion = downstreamHttpVersion; - DownstreamHttpVersionPolicy = downstreamHttpVersionPolicy; + DownstreamHttpVersion = downstreamHttpVersion; + DownstreamHttpVersionPolicy = downstreamHttpVersionPolicy; UpstreamHeaders = upstreamHeaders ?? new(); - MetadataOptions = metadataOptions; + MetadataOptions = metadataOptions; Timeout = timeout; } @@ -117,7 +117,7 @@ public DownstreamRoute( public bool DangerousAcceptAnyServerCertificateValidator { get; } public SecurityOptions SecurityOptions { get; } public string DownstreamHttpMethod { get; } - public Version DownstreamHttpVersion { get; } + public Version DownstreamHttpVersion { get; } /// The enum specifies behaviors for selecting and negotiating the HTTP version for a request. /// An enum value being mapped from a constant. @@ -129,7 +129,7 @@ public DownstreamRoute( /// HttpRequestMessage.VersionPolicy Property /// /// - public HttpVersionPolicy DownstreamHttpVersionPolicy { get; } + public HttpVersionPolicy DownstreamHttpVersionPolicy { get; } public Dictionary UpstreamHeaders { get; } public bool UseServiceDiscovery { get; } public MetadataOptions MetadataOptions { get; } @@ -141,18 +141,18 @@ public string Name() => string.IsNullOrEmpty(ServiceName) && !UseServiceDiscover : string.Join(':', ServiceNamespace, ServiceName, UpstreamPathTemplate?.Template); /// The timeout duration for the downstream request in seconds. - /// A (T is ) value in seconds. + /// A (T is ) value, in seconds. public int? Timeout { get; } /// Defines the default timeout in seconds for all routes, applicable at both the Route-level and globally. /// By default, initialized to 90 seconds. - /// An value, default. + /// An value. public static int DefaultTimeoutSeconds { get; set; } = 90; /// /// Calculates timeout in milliseconds based on QoS options with applying default timeout values. /// - /// An value in milliseconds. + /// An value, in milliseconds. public int TimeoutMilliseconds() => QosOptions.UseQos ? QosOptions.TimeoutValue ?? QoSOptions.DefaultTimeout : 1000 * (Timeout ?? DefaultTimeoutSeconds); From 0af9423bdb6e0069bcaddf49bf99c0a5321f0d2a Mon Sep 17 00:00:00 2001 From: Raman Maksimchuk Date: Thu, 17 Oct 2024 20:53:15 +0300 Subject: [PATCH 46/50] EOL: src/Ocelot/Configuration/File/FileRoute.cs --- src/Ocelot/Configuration/File/FileRoute.cs | 174 ++++++++++----------- 1 file changed, 87 insertions(+), 87 deletions(-) diff --git a/src/Ocelot/Configuration/File/FileRoute.cs b/src/Ocelot/Configuration/File/FileRoute.cs index 2e939eef5..6f80f3840 100644 --- a/src/Ocelot/Configuration/File/FileRoute.cs +++ b/src/Ocelot/Configuration/File/FileRoute.cs @@ -1,7 +1,7 @@ using Ocelot.Configuration.Creator; namespace Ocelot.Configuration.File -{ +{ public class FileRoute : IRoute, ICloneable // TODO: Inherit from FileDynamicRoute (FileRouteBase) or an interface with FileDynamicRoute props { public FileRoute() @@ -23,27 +23,27 @@ public FileRoute() RateLimitOptions = new FileRateLimitRule(); RouteClaimsRequirement = new Dictionary(); SecurityOptions = new FileSecurityOptions(); - UpstreamHeaderTemplates = new Dictionary(); + UpstreamHeaderTemplates = new Dictionary(); UpstreamHeaderTransform = new Dictionary(); UpstreamHttpMethod = new List(); - } - - public FileRoute(FileRoute from) - { - DeepCopy(from, this); - } - - public Dictionary AddClaimsToRequest { get; set; } - public Dictionary AddHeadersToRequest { get; set; } - public Dictionary AddQueriesToRequest { get; set; } - public FileAuthenticationOptions AuthenticationOptions { get; set; } - public Dictionary ChangeDownstreamPathTemplate { get; set; } - public bool DangerousAcceptAnyServerCertificateValidator { get; set; } - public List DelegatingHandlers { get; set; } - public Dictionary DownstreamHeaderTransform { get; set; } - public List DownstreamHostAndPorts { get; set; } - public string DownstreamHttpMethod { get; set; } - public string DownstreamHttpVersion { get; set; } + } + + public FileRoute(FileRoute from) + { + DeepCopy(from, this); + } + + public Dictionary AddClaimsToRequest { get; set; } + public Dictionary AddHeadersToRequest { get; set; } + public Dictionary AddQueriesToRequest { get; set; } + public FileAuthenticationOptions AuthenticationOptions { get; set; } + public Dictionary ChangeDownstreamPathTemplate { get; set; } + public bool DangerousAcceptAnyServerCertificateValidator { get; set; } + public List DelegatingHandlers { get; set; } + public Dictionary DownstreamHeaderTransform { get; set; } + public List DownstreamHostAndPorts { get; set; } + public string DownstreamHttpMethod { get; set; } + public string DownstreamHttpVersion { get; set; } /// The enum specifies behaviors for selecting and negotiating the HTTP version for a request. /// A value of defined constants. @@ -55,23 +55,23 @@ public FileRoute(FileRoute from) /// HttpRequestMessage.VersionPolicy Property /// /// - public string DownstreamHttpVersionPolicy { get; set; } - public string DownstreamPathTemplate { get; set; } + public string DownstreamHttpVersionPolicy { get; set; } + public string DownstreamPathTemplate { get; set; } public string DownstreamScheme { get; set; } - public FileCacheOptions FileCacheOptions { get; set; } - public FileHttpHandlerOptions HttpHandlerOptions { get; set; } - public string Key { get; set; } - public FileLoadBalancerOptions LoadBalancerOptions { get; set; } + public FileCacheOptions FileCacheOptions { get; set; } + public FileHttpHandlerOptions HttpHandlerOptions { get; set; } + public string Key { get; set; } + public FileLoadBalancerOptions LoadBalancerOptions { get; set; } public IDictionary Metadata { get; set; } - public int Priority { get; set; } - public FileQoSOptions QoSOptions { get; set; } - public FileRateLimitRule RateLimitOptions { get; set; } - public string RequestIdKey { get; set; } - public Dictionary RouteClaimsRequirement { get; set; } - public bool RouteIsCaseSensitive { get; set; } - public FileSecurityOptions SecurityOptions { get; set; } - public string ServiceName { get; set; } - public string ServiceNamespace { get; set; } + public int Priority { get; set; } + public FileQoSOptions QoSOptions { get; set; } + public FileRateLimitRule RateLimitOptions { get; set; } + public string RequestIdKey { get; set; } + public Dictionary RouteClaimsRequirement { get; set; } + public bool RouteIsCaseSensitive { get; set; } + public FileSecurityOptions SecurityOptions { get; set; } + public string ServiceName { get; set; } + public string ServiceNamespace { get; set; } /// Explicit timeout value which overrides default one. /// Reused in, or ignored in favor of implicit default value: @@ -81,62 +81,62 @@ public FileRoute(FileRoute from) /// /// /// - /// A (T is ) value in seconds. + /// A (T is ) value, in seconds. public int? Timeout { get; set; } - public IDictionary UpstreamHeaderTransform { get; set; } - public string UpstreamHost { get; set; } - public List UpstreamHttpMethod { get; set; } - public string UpstreamPathTemplate { get; set; } - public IDictionary UpstreamHeaderTemplates { get; set; } - - /// - /// Clones this object by making a deep copy. - /// - /// A deeply copied object. - public object Clone() - { - var other = (FileRoute)MemberwiseClone(); - DeepCopy(this, other); - return other; - } - - public static void DeepCopy(FileRoute from, FileRoute to) - { - to.AddClaimsToRequest = new(from.AddClaimsToRequest); - to.AddHeadersToRequest = new(from.AddHeadersToRequest); - to.AddQueriesToRequest = new(from.AddQueriesToRequest); - to.AuthenticationOptions = new(from.AuthenticationOptions); - to.ChangeDownstreamPathTemplate = new(from.ChangeDownstreamPathTemplate); - to.DangerousAcceptAnyServerCertificateValidator = from.DangerousAcceptAnyServerCertificateValidator; - to.DelegatingHandlers = new(from.DelegatingHandlers); - to.DownstreamHeaderTransform = new(from.DownstreamHeaderTransform); - to.DownstreamHostAndPorts = from.DownstreamHostAndPorts.Select(x => new FileHostAndPort(x)).ToList(); - to.DownstreamHttpMethod = from.DownstreamHttpMethod; - to.DownstreamHttpVersion = from.DownstreamHttpVersion; + public IDictionary UpstreamHeaderTransform { get; set; } + public string UpstreamHost { get; set; } + public List UpstreamHttpMethod { get; set; } + public string UpstreamPathTemplate { get; set; } + public IDictionary UpstreamHeaderTemplates { get; set; } + + /// + /// Clones this object by making a deep copy. + /// + /// A deeply copied object. + public object Clone() + { + var other = (FileRoute)MemberwiseClone(); + DeepCopy(this, other); + return other; + } + + public static void DeepCopy(FileRoute from, FileRoute to) + { + to.AddClaimsToRequest = new(from.AddClaimsToRequest); + to.AddHeadersToRequest = new(from.AddHeadersToRequest); + to.AddQueriesToRequest = new(from.AddQueriesToRequest); + to.AuthenticationOptions = new(from.AuthenticationOptions); + to.ChangeDownstreamPathTemplate = new(from.ChangeDownstreamPathTemplate); + to.DangerousAcceptAnyServerCertificateValidator = from.DangerousAcceptAnyServerCertificateValidator; + to.DelegatingHandlers = new(from.DelegatingHandlers); + to.DownstreamHeaderTransform = new(from.DownstreamHeaderTransform); + to.DownstreamHostAndPorts = from.DownstreamHostAndPorts.Select(x => new FileHostAndPort(x)).ToList(); + to.DownstreamHttpMethod = from.DownstreamHttpMethod; + to.DownstreamHttpVersion = from.DownstreamHttpVersion; to.DownstreamHttpVersionPolicy = from.DownstreamHttpVersionPolicy; - to.DownstreamPathTemplate = from.DownstreamPathTemplate; + to.DownstreamPathTemplate = from.DownstreamPathTemplate; to.DownstreamScheme = from.DownstreamScheme; - to.FileCacheOptions = new(from.FileCacheOptions); - to.HttpHandlerOptions = new(from.HttpHandlerOptions); - to.Key = from.Key; - to.LoadBalancerOptions = new(from.LoadBalancerOptions); + to.FileCacheOptions = new(from.FileCacheOptions); + to.HttpHandlerOptions = new(from.HttpHandlerOptions); + to.Key = from.Key; + to.LoadBalancerOptions = new(from.LoadBalancerOptions); to.Metadata = new Dictionary(from.Metadata); - to.Priority = from.Priority; - to.QoSOptions = new(from.QoSOptions); - to.RateLimitOptions = new(from.RateLimitOptions); - to.RequestIdKey = from.RequestIdKey; - to.RouteClaimsRequirement = new(from.RouteClaimsRequirement); - to.RouteIsCaseSensitive = from.RouteIsCaseSensitive; - to.SecurityOptions = new(from.SecurityOptions); - to.ServiceName = from.ServiceName; - to.ServiceNamespace = from.ServiceNamespace; - to.Timeout = from.Timeout; - to.UpstreamHeaderTemplates = new Dictionary(from.UpstreamHeaderTemplates); - to.UpstreamHeaderTransform = new Dictionary(from.UpstreamHeaderTransform); - to.UpstreamHost = from.UpstreamHost; - to.UpstreamHttpMethod = new(from.UpstreamHttpMethod); - to.UpstreamPathTemplate = from.UpstreamPathTemplate; - } + to.Priority = from.Priority; + to.QoSOptions = new(from.QoSOptions); + to.RateLimitOptions = new(from.RateLimitOptions); + to.RequestIdKey = from.RequestIdKey; + to.RouteClaimsRequirement = new(from.RouteClaimsRequirement); + to.RouteIsCaseSensitive = from.RouteIsCaseSensitive; + to.SecurityOptions = new(from.SecurityOptions); + to.ServiceName = from.ServiceName; + to.ServiceNamespace = from.ServiceNamespace; + to.Timeout = from.Timeout; + to.UpstreamHeaderTemplates = new Dictionary(from.UpstreamHeaderTemplates); + to.UpstreamHeaderTransform = new Dictionary(from.UpstreamHeaderTransform); + to.UpstreamHost = from.UpstreamHost; + to.UpstreamHttpMethod = new(from.UpstreamHttpMethod); + to.UpstreamPathTemplate = from.UpstreamPathTemplate; + } } } From 04d2b5bb59be87fa68c77bb547ef58111d248362 Mon Sep 17 00:00:00 2001 From: Raman Maksimchuk Date: Thu, 17 Oct 2024 21:10:23 +0300 Subject: [PATCH 47/50] EOL: test/Ocelot.UnitTests/Configuration/RoutesCreatorTests.cs --- .../Configuration/RoutesCreatorTests.cs | 60 +++++++++---------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/test/Ocelot.UnitTests/Configuration/RoutesCreatorTests.cs b/test/Ocelot.UnitTests/Configuration/RoutesCreatorTests.cs index 9a3e9f1f7..4fdd9cd95 100644 --- a/test/Ocelot.UnitTests/Configuration/RoutesCreatorTests.cs +++ b/test/Ocelot.UnitTests/Configuration/RoutesCreatorTests.cs @@ -1,8 +1,8 @@ -using Ocelot.Configuration; -using Ocelot.Configuration.Builder; -using Ocelot.Configuration.Creator; -using Ocelot.Configuration.File; -using Ocelot.Values; +using Ocelot.Configuration; +using Ocelot.Configuration.Builder; +using Ocelot.Configuration.Creator; +using Ocelot.Configuration.File; +using Ocelot.Values; namespace Ocelot.UnitTests.Configuration { @@ -25,8 +25,8 @@ public class RoutesCreatorTests : UnitTest private readonly Mock _rrkCreator; private readonly Mock _soCreator; private readonly Mock _versionCreator; - private readonly Mock _versionPolicyCreator; - private readonly Mock _metadataCreator; + private readonly Mock _versionPolicyCreator; + private readonly Mock _metadataCreator; private FileConfiguration _fileConfig; private RouteOptions _rro; private string _requestId; @@ -42,8 +42,8 @@ public class RoutesCreatorTests : UnitTest private List _dhp; private LoadBalancerOptions _lbo; private List _result; - private Version _expectedVersion; - private HttpVersionPolicy _expectedVersionPolicy; + private Version _expectedVersion; + private HttpVersionPolicy _expectedVersionPolicy; private Dictionary _uht; private Dictionary _expectedMetadata; @@ -64,9 +64,9 @@ public RoutesCreatorTests() _rrkCreator = new Mock(); _soCreator = new Mock(); _versionCreator = new Mock(); - _versionPolicyCreator = new Mock(); + _versionPolicyCreator = new Mock(); _uhtpCreator = new Mock(); - _metadataCreator = new Mock(); + _metadataCreator = new Mock(); _creator = new RoutesCreator( _cthCreator.Object, @@ -83,7 +83,7 @@ public RoutesCreatorTests() _lboCreator.Object, _rrkCreator.Object, _soCreator.Object, - _versionCreator.Object, + _versionCreator.Object, _versionPolicyCreator.Object, _uhtpCreator.Object, _metadataCreator.Object); @@ -123,7 +123,7 @@ public void Should_return_routes() { { "e","f" }, }, - UpstreamHttpMethod = new List { "GET", "POST" }, + UpstreamHttpMethod = new List { "GET", "POST" }, Metadata = new Dictionary { ["foo"] = "bar", @@ -149,7 +149,7 @@ public void Should_return_routes() Metadata = new Dictionary { ["foo"] = "baz", - }, + }, }, }, }; @@ -163,29 +163,29 @@ public void Should_return_routes() } [Fact] - public void Should_route_timeout() + public void CreateTimeout_RouteTimeoutIsLessThanGlobalOne_CreatedFromRouteValue() { // Arrange var fileRoute = new FileRoute { Timeout = 10 }; var globalConfiguration = new FileGlobalConfiguration { Timeout = 20 }; - + // Act var timeout = _creator.CreateTimeout(fileRoute, globalConfiguration); - + // Assert Assert.Equal(fileRoute.Timeout, timeout); } [Fact] - public void Should_global_timeout_when_not_route_timeout() + public void CreateTimeout_NoRouteTimeoutAndHasGlobalTimeout_CreatedFromGlobalValue() { // Arrange var fileRoute = new FileRoute(); var globalConfiguration = new FileGlobalConfiguration { Timeout = 20 }; - + // Act var timeout = _creator.CreateTimeout(fileRoute, globalConfiguration); - + // Assert Assert.Equal(globalConfiguration.Timeout, timeout); } @@ -199,7 +199,7 @@ private void ThenTheDependenciesAreCalledCorrectly() private void GivenTheDependenciesAreSetUpCorrectly() { _expectedVersion = new Version("1.1"); - _expectedVersionPolicy = HttpVersionPolicy.RequestVersionOrLower; + _expectedVersionPolicy = HttpVersionPolicy.RequestVersionOrLower; _rro = new RouteOptions(false, false, false, false, false); _requestId = "testy"; _rrk = "besty"; @@ -215,10 +215,10 @@ private void GivenTheDependenciesAreSetUpCorrectly() _dhp = new List(); _lbo = new LoadBalancerOptionsBuilder().Build(); _uht = new Dictionary(); - _expectedMetadata = new Dictionary() - { - ["foo"] = "bar", - }; + _expectedMetadata = new Dictionary() + { + ["foo"] = "bar", + }; _rroCreator.Setup(x => x.Create(It.IsAny())).Returns(_rro); _ridkCreator.Setup(x => x.Create(It.IsAny(), It.IsAny())).Returns(_requestId); @@ -234,12 +234,12 @@ private void GivenTheDependenciesAreSetUpCorrectly() _daCreator.Setup(x => x.Create(It.IsAny())).Returns(_dhp); _lboCreator.Setup(x => x.Create(It.IsAny())).Returns(_lbo); _versionCreator.Setup(x => x.Create(It.IsAny())).Returns(_expectedVersion); - _versionPolicyCreator.Setup(x => x.Create(It.IsAny())).Returns(_expectedVersionPolicy); + _versionPolicyCreator.Setup(x => x.Create(It.IsAny())).Returns(_expectedVersionPolicy); _uhtpCreator.Setup(x => x.Create(It.IsAny())).Returns(_uht); _metadataCreator.Setup(x => x.Create(It.IsAny>(), It.IsAny())).Returns(new MetadataOptions(new FileMetadataOptions { Metadata = _expectedMetadata, - })); + })); } private void ThenTheRoutesAreCreated() @@ -268,7 +268,7 @@ private void WhenICreate() private void ThenTheRouteIsSet(FileRoute expected, int routeIndex) { _result[routeIndex].DownstreamRoute[0].DownstreamHttpVersion.ShouldBe(_expectedVersion); - _result[routeIndex].DownstreamRoute[0].DownstreamHttpVersionPolicy.ShouldBe(_expectedVersionPolicy); + _result[routeIndex].DownstreamRoute[0].DownstreamHttpVersionPolicy.ShouldBe(_expectedVersionPolicy); _result[routeIndex].DownstreamRoute[0].IsAuthenticated.ShouldBe(_rro.IsAuthenticated); _result[routeIndex].DownstreamRoute[0].IsAuthorized.ShouldBe(_rro.IsAuthorized); _result[routeIndex].DownstreamRoute[0].IsCached.ShouldBe(_rro.IsCached); @@ -299,7 +299,7 @@ private void ThenTheRouteIsSet(FileRoute expected, int routeIndex) _result[routeIndex].DownstreamRoute[0].RouteClaimsRequirement.ShouldBe(expected.RouteClaimsRequirement); _result[routeIndex].DownstreamRoute[0].DownstreamPathTemplate.Value.ShouldBe(expected.DownstreamPathTemplate); _result[routeIndex].DownstreamRoute[0].Key.ShouldBe(expected.Key); - _result[routeIndex].DownstreamRoute[0].MetadataOptions.Metadata.ShouldBe(_expectedMetadata); + _result[routeIndex].DownstreamRoute[0].MetadataOptions.Metadata.ShouldBe(_expectedMetadata); _result[routeIndex].UpstreamHttpMethod .Select(x => x.Method) .ToList() @@ -331,7 +331,7 @@ private void ThenTheDepsAreCalledFor(FileRoute fileRoute, FileGlobalConfiguratio _hfarCreator.Verify(x => x.Create(fileRoute), Times.Once); _daCreator.Verify(x => x.Create(fileRoute), Times.Once); _lboCreator.Verify(x => x.Create(fileRoute.LoadBalancerOptions), Times.Once); - _soCreator.Verify(x => x.Create(fileRoute.SecurityOptions), Times.Once); + _soCreator.Verify(x => x.Create(fileRoute.SecurityOptions), Times.Once); _metadataCreator.Verify(x => x.Create(fileRoute.Metadata, globalConfig), Times.Once); } } From cfd0759d9f89a437aea53758f89a02fc81359755 Mon Sep 17 00:00:00 2001 From: Raman Maksimchuk Date: Thu, 17 Oct 2024 21:15:57 +0300 Subject: [PATCH 48/50] Review `FileGlobalConfiguration` --- src/Ocelot/Configuration/File/FileGlobalConfiguration.cs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/Ocelot/Configuration/File/FileGlobalConfiguration.cs b/src/Ocelot/Configuration/File/FileGlobalConfiguration.cs index 0ca5555da..a03135786 100644 --- a/src/Ocelot/Configuration/File/FileGlobalConfiguration.cs +++ b/src/Ocelot/Configuration/File/FileGlobalConfiguration.cs @@ -17,12 +17,8 @@ public FileGlobalConfiguration() public string RequestIdKey { get; set; } - /// - /// The timeout in seconds for requests. - /// - /// - /// The timeout value in seconds. - /// + /// The timeout in seconds for requests. + /// A (T is ) value in seconds. public int? Timeout { get; set; } public FileServiceDiscoveryProvider ServiceDiscoveryProvider { get; set; } From db1fe0ac69d60fa1828a76ceff96fba46584e658 Mon Sep 17 00:00:00 2001 From: Raman Maksimchuk Date: Fri, 1 Nov 2024 18:46:21 +0300 Subject: [PATCH 49/50] Fix build after rebasing --- .../ConsulConfigurationInConsulTests.cs | 134 ++++++++---------- 1 file changed, 62 insertions(+), 72 deletions(-) diff --git a/test/Ocelot.AcceptanceTests/ServiceDiscovery/ConsulConfigurationInConsulTests.cs b/test/Ocelot.AcceptanceTests/ServiceDiscovery/ConsulConfigurationInConsulTests.cs index 9687e994a..fd5b98c60 100644 --- a/test/Ocelot.AcceptanceTests/ServiceDiscovery/ConsulConfigurationInConsulTests.cs +++ b/test/Ocelot.AcceptanceTests/ServiceDiscovery/ConsulConfigurationInConsulTests.cs @@ -10,7 +10,6 @@ using Ocelot.DependencyInjection; using Ocelot.Middleware; using Ocelot.Provider.Consul; -using System.Net; using System.Text; namespace Ocelot.AcceptanceTests.ServiceDiscovery @@ -102,14 +101,13 @@ public void Should_load_configuration_out_of_consul() }, }; - var fakeConsulServiceDiscoveryUrl = DownstreamUrl(consulPort); - var consulConfig = new FileConfiguration - { - Routes = new List + var fakeConsulServiceDiscoveryUrl = DownstreamUrl(consulPort); + var consulConfig = new FileConfiguration { - new() + Routes = new List { - DownstreamPathTemplate = "/status", + new() + { DownstreamPathTemplate = "/status", DownstreamScheme = "http", DownstreamHostAndPorts = new List @@ -165,7 +163,7 @@ public void Should_load_configuration_out_of_consul_if_it_is_changed() }, }; - var fakeConsulServiceDiscoveryUrl = $"http://localhost:{consulPort}"; + var fakeConsulServiceDiscoveryUrl = $"http://localhost:{consulPort}"; var consulConfig = new FileConfiguration { @@ -229,17 +227,17 @@ public void Should_load_configuration_out_of_consul_if_it_is_changed() }, }; - this.Given(x => GivenTheConsulConfigurationIs(consulConfig)) - .And(x => GivenThereIsAFakeConsulServiceDiscoveryProvider(fakeConsulServiceDiscoveryUrl, string.Empty)) - .And(x => x.GivenThereIsAServiceRunningOn(DownstreamUrl(servicePort), "/status", 200, "Hello from Laura")) - .And(x => GivenThereIsAConfiguration(configuration)) - .And(x => GivenOcelotIsRunningUsingConsulToStoreConfig()) - .And(x => WhenIGetUrlOnTheApiGateway("/cs/status")) - .And(x => ThenTheStatusCodeShouldBe(HttpStatusCode.OK)) - .And(x => ThenTheResponseBodyShouldBe("Hello from Laura")) - .When(x => GivenTheConsulConfigurationIs(secondConsulConfig)) - .Then(x => ThenTheConfigIsUpdatedInOcelot()) - .BDDfy(); + this.Given(x => GivenTheConsulConfigurationIs(consulConfig)) + .And(x => GivenThereIsAFakeConsulServiceDiscoveryProvider(fakeConsulServiceDiscoveryUrl, string.Empty)) + .And(x => x.GivenThereIsAServiceRunningOn(DownstreamUrl(servicePort), "/status", 200, "Hello from Laura")) + .And(x => GivenThereIsAConfiguration(configuration)) + .And(x => GivenOcelotIsRunningUsingConsulToStoreConfig()) + .And(x => WhenIGetUrlOnTheApiGateway("/cs/status")) + .And(x => ThenTheStatusCodeShouldBe(HttpStatusCode.OK)) + .And(x => ThenTheResponseBodyShouldBe("Hello from Laura")) + .When(x => GivenTheConsulConfigurationIs(secondConsulConfig)) + .Then(x => ThenTheConfigIsUpdatedInOcelot()) + .BDDfy(); } [Fact] @@ -269,7 +267,7 @@ public void Should_handle_request_to_consul_for_downstream_service_and_make_requ new() { ServiceName = serviceName, - RateLimitRule = new FileRateLimitRule + RateLimitRule = new FileRateLimitRule { EnableRateLimiting = true, ClientWhitelist = new List(), @@ -319,7 +317,7 @@ public void Should_handle_request_to_consul_for_downstream_service_and_make_requ .And(x => GivenThereIsAConfiguration(configuration)) .And(x => x.GivenOcelotIsRunningUsingConsulToStoreConfig()) .When(x => WhenIGetUrlOnTheApiGatewayMultipleTimesForRateLimit("/web/something", 1)) - .Then(x => _steps.ThenTheStatusCodeShouldBe(HttpStatusCode.OK)) + .Then(x => ThenTheStatusCodeShouldBe(HttpStatusCode.OK)) .When(x => WhenIGetUrlOnTheApiGatewayMultipleTimesForRateLimit("/web/something", 2)) .Then(x => ThenTheStatusCodeShouldBe(HttpStatusCode.OK)) .When(x => WhenIGetUrlOnTheApiGatewayMultipleTimesForRateLimit("/web/something", 1)) @@ -379,62 +377,62 @@ private void GivenOcelotIsRunningUsingConsulToStoreConfig() Thread.Sleep(1000); } - private Task GivenThereIsAFakeConsulServiceDiscoveryProvider(string url, string serviceName) - { - _fakeConsulBuilder = new WebHostBuilder() - .UseUrls(url) - .UseKestrel() - .UseContentRoot(Directory.GetCurrentDirectory()) - .UseIISIntegration() - .UseUrls(url) - .Configure(app => - { - app.Run(async context => + private Task GivenThereIsAFakeConsulServiceDiscoveryProvider(string url, string serviceName) + { + _fakeConsulBuilder = new WebHostBuilder() + .UseUrls(url) + .UseKestrel() + .UseContentRoot(Directory.GetCurrentDirectory()) + .UseIISIntegration() + .UseUrls(url) + .Configure(app => { - if (context.Request.Method.ToLower() == "get" && context.Request.Path.Value == "/v1/kv/InternalConfiguration") + app.Run(async context => { - var json = JsonConvert.SerializeObject(_config); + if (context.Request.Method.Equals(HttpMethods.Get, StringComparison.OrdinalIgnoreCase) && context.Request.Path.Value == " /v1/kv/InternalConfiguration") + { + var json = JsonConvert.SerializeObject(_config); - var bytes = Encoding.UTF8.GetBytes(json); + var bytes = Encoding.UTF8.GetBytes(json); - var base64 = Convert.ToBase64String(bytes); + var base64 = Convert.ToBase64String(bytes); - var kvp = new FakeConsulGetResponse(base64); - json = JsonConvert.SerializeObject(new[] { kvp }); - context.Response.Headers.Append("Content-Type", "application/json"); - await context.Response.WriteAsync(json); - } - else if (context.Request.Method.ToLower() == "put" && context.Request.Path.Value == "/v1/kv/InternalConfiguration") - { - try + var kvp = new FakeConsulGetResponse(base64); + json = JsonConvert.SerializeObject(new[] { kvp }); + context.Response.Headers.Append("Content-Type", "application/json"); + await context.Response.WriteAsync(json); + } + else if (context.Request.Method.Equals(HttpMethods.Put, StringComparison.OrdinalIgnoreCase) && context.Request.Path.Value == "/v1/kv/InternalConfiguration") { - var reader = new StreamReader(context.Request.Body); + try + { + var reader = new StreamReader(context.Request.Body); - // Synchronous operations are disallowed. Call ReadAsync or set AllowSynchronousIO to true instead. - // var json = reader.ReadToEnd(); - var json = await reader.ReadToEndAsync(); + // Synchronous operations are disallowed. Call ReadAsync or set AllowSynchronousIO to true instead. + // var json = reader.ReadToEnd(); + var json = await reader.ReadToEndAsync(); - _config = JsonConvert.DeserializeObject(json); + _config = JsonConvert.DeserializeObject(json); - var response = JsonConvert.SerializeObject(true); + var response = JsonConvert.SerializeObject(true); - await context.Response.WriteAsync(response); + await context.Response.WriteAsync(response); + } + catch (Exception e) + { + Console.WriteLine(e); + throw; + } } - catch (Exception e) + else if (context.Request.Path.Value == $"/v1/health/service/{serviceName}") { - Console.WriteLine(e); - throw; + var json = JsonConvert.SerializeObject(_consulServices); + context.Response.Headers.Append("Content-Type", "application/json"); + await context.Response.WriteAsync(json); } - } - else if (context.Request.Path.Value == $"/v1/health/service/{serviceName}") - { - var json = JsonConvert.SerializeObject(_consulServices); - context.Response.Headers.Append("Content-Type", "application/json"); - await context.Response.WriteAsync(json); - } - }); - }) - .Build(); + }); + }) + .Build(); return _fakeConsulBuilder.StartAsync(); } @@ -465,7 +463,6 @@ private Task GivenThereIsAServiceRunningOn(string url, string basePath, int stat .Configure(app => { app.UsePathBase(basePath); - app.Run(async context => { context.Response.StatusCode = statusCode; @@ -476,13 +473,6 @@ private Task GivenThereIsAServiceRunningOn(string url, string basePath, int stat return _builder.StartAsync(); } - public override void Dispose() - { - _builder?.Dispose(); - _fakeConsulBuilder?.Dispose(); - base.Dispose(); - } - private class FakeCache : IOcelotCache { public void Add(string key, FileConfiguration value, TimeSpan ttl, string region) => throw new NotImplementedException(); From 2810c0359a130e2205db92f8293327f26f7e294b Mon Sep 17 00:00:00 2001 From: Raman Maksimchuk Date: Fri, 1 Nov 2024 19:52:33 +0300 Subject: [PATCH 50/50] Fix failed tests --- .../ConsulConfigurationInConsulTests.cs | 107 ++++++++---------- 1 file changed, 45 insertions(+), 62 deletions(-) diff --git a/test/Ocelot.AcceptanceTests/ServiceDiscovery/ConsulConfigurationInConsulTests.cs b/test/Ocelot.AcceptanceTests/ServiceDiscovery/ConsulConfigurationInConsulTests.cs index fd5b98c60..3c16b926c 100644 --- a/test/Ocelot.AcceptanceTests/ServiceDiscovery/ConsulConfigurationInConsulTests.cs +++ b/test/Ocelot.AcceptanceTests/ServiceDiscovery/ConsulConfigurationInConsulTests.cs @@ -135,7 +135,7 @@ public void Should_load_configuration_out_of_consul() this.Given(x => GivenTheConsulConfigurationIs(consulConfig)) .And(x => GivenThereIsAFakeConsulServiceDiscoveryProvider(fakeConsulServiceDiscoveryUrl, string.Empty)) - .And(x => x.GivenThereIsAServiceRunningOn($"http://localhost:{servicePort}", "/status", 200, "Hello from Laura")) + .And(x => x.GivenThereIsAServiceRunningOn(DownstreamUrl(servicePort), "/status", 200, "Hello from Laura")) .And(x => GivenThereIsAConfiguration(configuration)) .And(x => x.GivenOcelotIsRunningUsingConsulToStoreConfig()) .When(x => WhenIGetUrlOnTheApiGateway("/cs/status")) @@ -163,8 +163,6 @@ public void Should_load_configuration_out_of_consul_if_it_is_changed() }, }; - var fakeConsulServiceDiscoveryUrl = $"http://localhost:{consulPort}"; - var consulConfig = new FileConfiguration { Routes = new List @@ -228,7 +226,7 @@ public void Should_load_configuration_out_of_consul_if_it_is_changed() }; this.Given(x => GivenTheConsulConfigurationIs(consulConfig)) - .And(x => GivenThereIsAFakeConsulServiceDiscoveryProvider(fakeConsulServiceDiscoveryUrl, string.Empty)) + .And(x => GivenThereIsAFakeConsulServiceDiscoveryProvider(DownstreamUrl(consulPort), string.Empty)) .And(x => x.GivenThereIsAServiceRunningOn(DownstreamUrl(servicePort), "/status", 200, "Hello from Laura")) .And(x => GivenThereIsAConfiguration(configuration)) .And(x => GivenOcelotIsRunningUsingConsulToStoreConfig()) @@ -379,69 +377,54 @@ private void GivenOcelotIsRunningUsingConsulToStoreConfig() private Task GivenThereIsAFakeConsulServiceDiscoveryProvider(string url, string serviceName) { - _fakeConsulBuilder = new WebHostBuilder() - .UseUrls(url) - .UseKestrel() - .UseContentRoot(Directory.GetCurrentDirectory()) - .UseIISIntegration() - .UseUrls(url) - .Configure(app => - { - app.Run(async context => - { - if (context.Request.Method.Equals(HttpMethods.Get, StringComparison.OrdinalIgnoreCase) && context.Request.Path.Value == " /v1/kv/InternalConfiguration") - { - var json = JsonConvert.SerializeObject(_config); - - var bytes = Encoding.UTF8.GetBytes(json); - - var base64 = Convert.ToBase64String(bytes); - - var kvp = new FakeConsulGetResponse(base64); - json = JsonConvert.SerializeObject(new[] { kvp }); - context.Response.Headers.Append("Content-Type", "application/json"); - await context.Response.WriteAsync(json); - } - else if (context.Request.Method.Equals(HttpMethods.Put, StringComparison.OrdinalIgnoreCase) && context.Request.Path.Value == "/v1/kv/InternalConfiguration") - { - try - { - var reader = new StreamReader(context.Request.Body); - - // Synchronous operations are disallowed. Call ReadAsync or set AllowSynchronousIO to true instead. - // var json = reader.ReadToEnd(); - var json = await reader.ReadToEndAsync(); - - _config = JsonConvert.DeserializeObject(json); - - var response = JsonConvert.SerializeObject(true); - - await context.Response.WriteAsync(response); - } - catch (Exception e) - { - Console.WriteLine(e); - throw; - } - } - else if (context.Request.Path.Value == $"/v1/health/service/{serviceName}") - { - var json = JsonConvert.SerializeObject(_consulServices); - context.Response.Headers.Append("Content-Type", "application/json"); - await context.Response.WriteAsync(json); - } - }); - }) - .Build(); + _fakeConsulBuilder = TestHostBuilder.Create() + .UseUrls(url) + .UseKestrel() + .UseContentRoot(Directory.GetCurrentDirectory()) + .UseIISIntegration() + .UseUrls(url) + .Configure(app => app.Run(async context => + { + if (context.Request.Method.Equals(HttpMethods.Get, StringComparison.OrdinalIgnoreCase) && context.Request.Path.Value == "/v1/kv/InternalConfiguration") + { + var json = JsonConvert.SerializeObject(_config); + var bytes = Encoding.UTF8.GetBytes(json); + var base64 = Convert.ToBase64String(bytes); + var kvp = new FakeConsulGetResponse(base64); + json = JsonConvert.SerializeObject(new[] { kvp }); + context.Response.Headers.Append("Content-Type", "application/json"); + await context.Response.WriteAsync(json); + } + else if (context.Request.Method.Equals(HttpMethods.Put, StringComparison.OrdinalIgnoreCase) && context.Request.Path.Value == "/v1/kv/InternalConfiguration") + { + try + { + using var reader = new StreamReader(context.Request.Body); + var json = await reader.ReadToEndAsync(); + _config = JsonConvert.DeserializeObject(json); + var response = JsonConvert.SerializeObject(true); + await context.Response.WriteAsync(response); + } + catch (Exception e) + { + Console.WriteLine(e); + throw; + } + } + else if (context.Request.Path.Value == $"/v1/health/service/{serviceName}") + { + var json = JsonConvert.SerializeObject(_consulServices); + context.Response.Headers.Append("Content-Type", "application/json"); + await context.Response.WriteAsync(json); + } + })) + .Build(); return _fakeConsulBuilder.StartAsync(); } public class FakeConsulGetResponse { - public FakeConsulGetResponse(string value) - { - Value = value; - } + public FakeConsulGetResponse(string value) => Value = value; public int CreateIndex => 100; public int ModifyIndex => 200;