From c3fb9cffb8e32ccd335ece5a479a5052f8718a9c Mon Sep 17 00:00:00 2001 From: raman-m Date: Fri, 29 Sep 2023 18:45:41 +0300 Subject: [PATCH 1/9] Code refactoring and improvements --- .../MyServiceDiscoveryProvider.cs | 2 +- src/Ocelot.Provider.Consul/Consul.cs | 38 ++++++++++--------- .../ConsulClientFactory.cs | 13 ++++--- .../ConsulFileConfigurationRepository.cs | 27 +++++-------- .../ConsulMiddlewareConfigurationProvider.cs | 32 ++++++---------- .../ConsulProviderFactory.cs | 9 ++--- .../ConsulRegistryConfiguration.cs | 4 +- .../OcelotBuilderExtensions.cs | 16 ++++---- src/Ocelot.Provider.Consul/PollConsul.cs | 24 +++++++----- .../UnableToSetConfigInConsulError.cs | 3 +- src/Ocelot.Provider.Consul/Usings.cs | 1 - src/Ocelot.Provider.Eureka/Eureka.cs | 3 +- .../EurekaProviderFactory.cs | 12 ++---- .../OcelotBuilderExtensions.cs | 7 ++-- .../EndPointClientV1.cs | 23 +++++------ .../KubeRegistryConfiguration.cs | 1 - .../KubernetesProviderFactory.cs | 17 +++------ .../KubernetesServiceDiscoveryProvider.cs | 7 +--- .../OcelotBuilderExtensions.cs | 8 ++-- src/Ocelot.Provider.Kubernetes/PollKube.cs | 38 ++++++++++--------- .../CookieStickySessionsCreator.cs | 2 +- .../LoadBalancers/LeastConnectionCreator.cs | 2 +- .../LoadBalancers/NoLoadBalancerCreator.cs | 2 +- .../LoadBalancers/RoundRobinCreator.cs | 2 +- .../Providers/ConfigurationServiceProvider.cs | 2 +- .../Providers/IServiceDiscoveryProvider.cs | 2 +- .../ServiceFabricServiceDiscoveryProvider.cs | 2 +- .../ServiceDiscoveryProviderFactory.cs | 20 +++++----- .../LoadBalancerTests.cs | 2 +- .../SequentialTestsCollection.cs | 8 ---- .../FileConfigurationFluentValidatorTests.cs | 2 +- .../ConsulServiceDiscoveryProviderTests.cs | 2 +- ...lingConsulServiceDiscoveryProviderTests.cs | 6 +-- .../Consul/ProviderFactoryTests.cs | 30 +++++++-------- .../EurekaServiceDiscoveryProviderTests.cs | 2 +- .../KubeServiceDiscoveryProviderTests.cs | 4 +- ...ollingKubeServiceDiscoveryProviderTests.cs | 4 +- .../ConfigurationServiceProviderTests.cs | 2 +- .../ServiceDiscoveryProviderFactoryTests.cs | 4 +- ...viceFabricServiceDiscoveryProviderTests.cs | 2 +- 40 files changed, 175 insertions(+), 212 deletions(-) rename src/Ocelot.Provider.Kubernetes/{KubeApiClientExtensions => }/EndPointClientV1.cs (55%) delete mode 100644 test/Ocelot.AcceptanceTests/SequentialTestsCollection.cs diff --git a/samples/OcelotServiceDiscovery/ApiGateway/ServiceDiscovery/MyServiceDiscoveryProvider.cs b/samples/OcelotServiceDiscovery/ApiGateway/ServiceDiscovery/MyServiceDiscoveryProvider.cs index 76e655bd1..b874ffedb 100644 --- a/samples/OcelotServiceDiscovery/ApiGateway/ServiceDiscovery/MyServiceDiscoveryProvider.cs +++ b/samples/OcelotServiceDiscovery/ApiGateway/ServiceDiscovery/MyServiceDiscoveryProvider.cs @@ -20,7 +20,7 @@ public MyServiceDiscoveryProvider(IServiceProvider serviceProvider, ServiceProvi _downstreamRoute = downstreamRoute; } - public Task> Get() + public Task> GetAsync() { // Returns a list of service(s) that match the downstream route passed to the provider diff --git a/src/Ocelot.Provider.Consul/Consul.cs b/src/Ocelot.Provider.Consul/Consul.cs index ecb61b2ab..8c7caf0a4 100644 --- a/src/Ocelot.Provider.Consul/Consul.cs +++ b/src/Ocelot.Provider.Consul/Consul.cs @@ -14,12 +14,12 @@ public class Consul : IServiceDiscoveryProvider public Consul(ConsulRegistryConfiguration config, IOcelotLoggerFactory factory, IConsulClientFactory clientFactory) { - _logger = factory.CreateLogger(); _config = config; _consul = clientFactory.Get(_config); + _logger = factory.CreateLogger(); } - public async Task> Get() + public async Task> GetAsync() { var queryResult = await _consul.Health.Service(_config.KeyOfServiceInConsul, string.Empty, true); @@ -27,6 +27,8 @@ public async Task> Get() foreach (var serviceEntry in queryResult.Response) { + var address = serviceEntry.Service.Address; + var port = serviceEntry.Service.Port; if (IsValid(serviceEntry)) { var nodes = await _consul.Catalog.Nodes(); @@ -36,14 +38,14 @@ public async Task> Get() } else { - var serviceNode = nodes.Response.FirstOrDefault(n => n.Address == serviceEntry.Service.Address); + var serviceNode = nodes.Response.FirstOrDefault(n => n.Address == address); services.Add(BuildService(serviceEntry, serviceNode)); } } else { _logger.LogWarning( - $"Unable to use service Address: {serviceEntry.Service.Address} and Port: {serviceEntry.Service.Port} as it is invalid. Address must contain host only e.g. localhost and port must be greater than 0"); + $"Unable to use service address: {address} and Port: {port} as it is invalid. Address must contain host only e.g. 'localhost', and port must be greater than 0."); } } @@ -52,27 +54,27 @@ public async Task> Get() private static Service BuildService(ServiceEntry serviceEntry, Node serviceNode) { + var service = serviceEntry.Service; return new Service( - serviceEntry.Service.Service, - new ServiceHostAndPort(serviceNode == null ? serviceEntry.Service.Address : serviceNode.Name, - serviceEntry.Service.Port), - serviceEntry.Service.ID, - GetVersionFromStrings(serviceEntry.Service.Tags), - serviceEntry.Service.Tags ?? Enumerable.Empty()); + service.Service, + new ServiceHostAndPort( + serviceNode == null ? service.Address : serviceNode.Name, + service.Port), + service.ID, + GetVersionFromStrings(service.Tags), + service.Tags ?? Enumerable.Empty()); } private static bool IsValid(ServiceEntry serviceEntry) { - return !string.IsNullOrEmpty(serviceEntry.Service.Address) - && !serviceEntry.Service.Address.Contains("http://") - && !serviceEntry.Service.Address.Contains("https://") - && serviceEntry.Service.Port > 0; + var service = serviceEntry.Service; + return !string.IsNullOrEmpty(service.Address) + && !service.Address.Contains($"{Uri.UriSchemeHttp}://") + && !service.Address.Contains($"{Uri.UriSchemeHttps}://") + && service.Port > 0; } private static string GetVersionFromStrings(IEnumerable strings) - { - return strings - ?.FirstOrDefault(x => x.StartsWith(VersionPrefix, StringComparison.Ordinal)) + => strings?.FirstOrDefault(x => x.StartsWith(VersionPrefix, StringComparison.Ordinal)) .TrimStart(VersionPrefix); - } } diff --git a/src/Ocelot.Provider.Consul/ConsulClientFactory.cs b/src/Ocelot.Provider.Consul/ConsulClientFactory.cs index 5f5009d9b..4a7478c59 100644 --- a/src/Ocelot.Provider.Consul/ConsulClientFactory.cs +++ b/src/Ocelot.Provider.Consul/ConsulClientFactory.cs @@ -3,12 +3,15 @@ public class ConsulClientFactory : IConsulClientFactory { public IConsulClient Get(ConsulRegistryConfiguration config) + => new ConsulClient(c => OverrideConfig(c, config)); + + private static void OverrideConfig(ConsulClientConfiguration to, ConsulRegistryConfiguration from) { - return new ConsulClient(c => - { - c.Address = new Uri($"{config.Scheme}://{config.Host}:{config.Port}"); + to.Address = new Uri($"{from.Scheme}://{from.Host}:{from.Port}"); - if (!string.IsNullOrEmpty(config?.Token)) c.Token = config.Token; - }); + if (!string.IsNullOrEmpty(from?.Token)) + { + to.Token = from.Token; + } } } diff --git a/src/Ocelot.Provider.Consul/ConsulFileConfigurationRepository.cs b/src/Ocelot.Provider.Consul/ConsulFileConfigurationRepository.cs index 7ac7612aa..2f9569362 100644 --- a/src/Ocelot.Provider.Consul/ConsulFileConfigurationRepository.cs +++ b/src/Ocelot.Provider.Consul/ConsulFileConfigurationRepository.cs @@ -1,11 +1,12 @@ -using System.Text; -using Microsoft.Extensions.Options; +using Microsoft.Extensions.Options; using Newtonsoft.Json; using Ocelot.Cache; +using Ocelot.Configuration; using Ocelot.Configuration.File; using Ocelot.Configuration.Repository; using Ocelot.Logging; using Ocelot.Responses; +using System.Text; namespace Ocelot.Provider.Consul; @@ -25,37 +26,32 @@ public ConsulFileConfigurationRepository( _logger = loggerFactory.CreateLogger(); _cache = cache; - var serviceDiscoveryProvider = fileConfiguration.Value.GlobalConfiguration.ServiceDiscoveryProvider; - _configurationKey = string.IsNullOrWhiteSpace(serviceDiscoveryProvider.ConfigurationKey) - ? "InternalConfiguration" - : serviceDiscoveryProvider.ConfigurationKey; - - var config = new ConsulRegistryConfiguration(serviceDiscoveryProvider.Scheme, serviceDiscoveryProvider.Host, - serviceDiscoveryProvider.Port, _configurationKey, serviceDiscoveryProvider.Token); + var provider = fileConfiguration.Value.GlobalConfiguration.ServiceDiscoveryProvider; + _configurationKey = string.IsNullOrWhiteSpace(provider.ConfigurationKey) + ? nameof(InternalConfiguration) + : provider.ConfigurationKey; + var config = new ConsulRegistryConfiguration(provider.Scheme, provider.Host, + provider.Port, _configurationKey, provider.Token); _consul = factory.Get(config); } public async Task> Get() { var config = _cache.Get(_configurationKey, _configurationKey); - if (config != null) { return new OkResponse(config); } var queryResult = await _consul.KV.Get(_configurationKey); - if (queryResult.Response == null) { return new OkResponse(null); } var bytes = queryResult.Response.Value; - var json = Encoding.UTF8.GetString(bytes); - var consulConfig = JsonConvert.DeserializeObject(json); return new OkResponse(consulConfig); @@ -64,16 +60,13 @@ public async Task> Get() public async Task Set(FileConfiguration ocelotConfiguration) { var json = JsonConvert.SerializeObject(ocelotConfiguration, Formatting.Indented); - var bytes = Encoding.UTF8.GetBytes(json); - var kvPair = new KVPair(_configurationKey) { Value = bytes, }; var result = await _consul.KV.Put(kvPair); - if (result.Response) { _cache.AddAndDelete(_configurationKey, ocelotConfiguration, TimeSpan.FromSeconds(3), _configurationKey); @@ -82,6 +75,6 @@ public async Task Set(FileConfiguration ocelotConfiguration) } return new ErrorResponse(new UnableToSetConfigInConsulError( - $"Unable to set FileConfiguration in consul, response status code from consul was {result.StatusCode}")); + $"Unable to set {nameof(FileConfiguration)} in {nameof(Consul)}, response status code from {nameof(Consul)} was {result.StatusCode}")); } } diff --git a/src/Ocelot.Provider.Consul/ConsulMiddlewareConfigurationProvider.cs b/src/Ocelot.Provider.Consul/ConsulMiddlewareConfigurationProvider.cs index f6b644199..c25ee8fc8 100644 --- a/src/Ocelot.Provider.Consul/ConsulMiddlewareConfigurationProvider.cs +++ b/src/Ocelot.Provider.Consul/ConsulMiddlewareConfigurationProvider.cs @@ -11,7 +11,9 @@ namespace Ocelot.Provider.Consul; public static class ConsulMiddlewareConfigurationProvider { - public static OcelotMiddlewareConfigurationDelegate Get = async builder => + public static OcelotMiddlewareConfigurationDelegate Get { get; } = GetAsync; + + private static async Task GetAsync(IApplicationBuilder builder) { var fileConfigRepo = builder.ApplicationServices.GetService(); var fileConfig = builder.ApplicationServices.GetService>(); @@ -22,34 +24,30 @@ public static class ConsulMiddlewareConfigurationProvider { await SetFileConfigInConsul(builder, fileConfigRepo, fileConfig, internalConfigCreator, internalConfigRepo); } - }; + } private static bool UsingConsul(IFileConfigurationRepository fileConfigRepo) - { - return fileConfigRepo.GetType() == typeof(ConsulFileConfigurationRepository); - } + => fileConfigRepo.GetType() == typeof(ConsulFileConfigurationRepository); private static async Task SetFileConfigInConsul(IApplicationBuilder builder, IFileConfigurationRepository fileConfigRepo, IOptionsMonitor fileConfig, IInternalConfigurationCreator internalConfigCreator, IInternalConfigurationRepository internalConfigRepo) { - // get the config from consul. + // Get the config from Consul var fileConfigFromConsul = await fileConfigRepo.Get(); - if (IsError(fileConfigFromConsul)) { ThrowToStopOcelotStarting(fileConfigFromConsul); } else if (ConfigNotStoredInConsul(fileConfigFromConsul)) { - //there was no config in consul set the file in config in consul + // there was no config in Consul set the file in config in Consul await fileConfigRepo.Set(fileConfig.CurrentValue); } else { - // create the internal config from consul data + // Create the internal config from Consul data var internalConfig = await internalConfigCreator.Create(fileConfigFromConsul.Data); - if (IsError(internalConfig)) { ThrowToStopOcelotStarting(internalConfig); @@ -58,7 +56,6 @@ private static async Task SetFileConfigInConsul(IApplicationBuilder builder, { // add the internal config to the internal repo var response = internalConfigRepo.AddOrReplace(internalConfig.Data); - if (IsError(response)) { ThrowToStopOcelotStarting(response); @@ -73,18 +70,11 @@ private static async Task SetFileConfigInConsul(IApplicationBuilder builder, } private static void ThrowToStopOcelotStarting(Response config) - { - throw new Exception( - $"Unable to start Ocelot, errors are: {string.Join(',', config.Errors.Select(x => x.ToString()))}"); - } + => throw new Exception($"Unable to start Ocelot, errors are: {string.Join(',', config.Errors.Select(x => x.ToString()))}"); private static bool IsError(Response response) - { - return response == null || response.IsError; - } + => response == null || response.IsError; private static bool ConfigNotStoredInConsul(Response fileConfigFromConsul) - { - return fileConfigFromConsul.Data == null; - } + => fileConfigFromConsul.Data == null; } diff --git a/src/Ocelot.Provider.Consul/ConsulProviderFactory.cs b/src/Ocelot.Provider.Consul/ConsulProviderFactory.cs index 724c6fb9d..c769f49c0 100644 --- a/src/Ocelot.Provider.Consul/ConsulProviderFactory.cs +++ b/src/Ocelot.Provider.Consul/ConsulProviderFactory.cs @@ -8,7 +8,7 @@ namespace Ocelot.Provider.Consul; public static class ConsulProviderFactory { /// - /// String constant used for provider type definition. + /// String constant used for provider type definition. /// public const string PollConsul = nameof(Provider.Consul.PollConsul); @@ -32,17 +32,14 @@ private static IServiceDiscoveryProvider CreateProvider(IServiceProvider provide { lock (LockObject) { - var discoveryProvider = - ServiceDiscoveryProviders.FirstOrDefault(x => x.ServiceName == route.ServiceName); + var discoveryProvider = ServiceDiscoveryProviders.FirstOrDefault(x => x.ServiceName == route.ServiceName); if (discoveryProvider != null) { return discoveryProvider; } - discoveryProvider = new PollConsul( - config.PollingInterval, route.ServiceName, factory, consulProvider); + discoveryProvider = new PollConsul(config.PollingInterval, route.ServiceName, factory, consulProvider); ServiceDiscoveryProviders.Add(discoveryProvider); - return discoveryProvider; } } diff --git a/src/Ocelot.Provider.Consul/ConsulRegistryConfiguration.cs b/src/Ocelot.Provider.Consul/ConsulRegistryConfiguration.cs index 52fba7893..7147f4e34 100644 --- a/src/Ocelot.Provider.Consul/ConsulRegistryConfiguration.cs +++ b/src/Ocelot.Provider.Consul/ConsulRegistryConfiguration.cs @@ -5,8 +5,8 @@ public class ConsulRegistryConfiguration public ConsulRegistryConfiguration(string scheme, string host, int port, string keyOfServiceInConsul, string token) { Host = string.IsNullOrEmpty(host) ? "localhost" : host; - Port = port > 0 ? port : 8500; - Scheme = string.IsNullOrEmpty(scheme) ? "http" : scheme; + Port = port > 0 ? port : 8500; // Is 8500 default port of Consul? + Scheme = string.IsNullOrEmpty(scheme) ? Uri.UriSchemeHttp : scheme; KeyOfServiceInConsul = keyOfServiceInConsul; Token = token; } diff --git a/src/Ocelot.Provider.Consul/OcelotBuilderExtensions.cs b/src/Ocelot.Provider.Consul/OcelotBuilderExtensions.cs index 48bce6f6e..dac7aecff 100644 --- a/src/Ocelot.Provider.Consul/OcelotBuilderExtensions.cs +++ b/src/Ocelot.Provider.Consul/OcelotBuilderExtensions.cs @@ -9,18 +9,20 @@ public static class OcelotBuilderExtensions { public static IOcelotBuilder AddConsul(this IOcelotBuilder builder) { - builder.Services.AddSingleton(ConsulProviderFactory.Get); - builder.Services.AddSingleton(); - builder.Services.RemoveAll(typeof(IFileConfigurationPollerOptions)); - builder.Services.AddSingleton(); + builder.Services + .AddSingleton(ConsulProviderFactory.Get) + .AddSingleton() + .RemoveAll(typeof(IFileConfigurationPollerOptions)) + .AddSingleton(); return builder; } public static IOcelotBuilder AddConfigStoredInConsul(this IOcelotBuilder builder) { - builder.Services.AddSingleton(ConsulMiddlewareConfigurationProvider.Get); - builder.Services.AddHostedService(); - builder.Services.AddSingleton(); + builder.Services + .AddSingleton(ConsulMiddlewareConfigurationProvider.Get) + .AddHostedService() + .AddSingleton(); return builder; } } diff --git a/src/Ocelot.Provider.Consul/PollConsul.cs b/src/Ocelot.Provider.Consul/PollConsul.cs index 295a17e15..5806e60f1 100644 --- a/src/Ocelot.Provider.Consul/PollConsul.cs +++ b/src/Ocelot.Provider.Consul/PollConsul.cs @@ -6,10 +6,9 @@ namespace Ocelot.Provider.Consul; public sealed class PollConsul : IServiceDiscoveryProvider { - private readonly IServiceDiscoveryProvider _consulServiceDiscoveryProvider; private readonly object _lockObject = new(); private readonly IOcelotLogger _logger; - + private readonly IServiceDiscoveryProvider _consulServiceDiscoveryProvider; private readonly int _pollingInterval; private DateTime _lastUpdateTime; @@ -33,11 +32,11 @@ public PollConsul(int pollingInterval, string serviceName, IOcelotLoggerFactory public string ServiceName { get; } /// - /// Get the services. - /// If the first call, retrieve the services and then start the timer. + /// Gets the services. + /// If the first call, retrieves the services and then starts the timer. /// /// A with a result of . - public Task> Get() + public Task> GetAsync() { lock (_lockObject) { @@ -49,11 +48,16 @@ public Task> Get() return Task.FromResult(_services); } - _logger.LogInformation($"Retrieving new client information for service: {ServiceName}."); - _services = _consulServiceDiscoveryProvider.Get().Result; - _lastUpdateTime = DateTime.UtcNow; - - return Task.FromResult(_services); + try + { + _logger.LogInformation($"Retrieving new client information for service: {ServiceName}..."); + _services = _consulServiceDiscoveryProvider.GetAsync().Result; + return Task.FromResult(_services); + } + finally + { + _lastUpdateTime = DateTime.UtcNow; + } } } } diff --git a/src/Ocelot.Provider.Consul/UnableToSetConfigInConsulError.cs b/src/Ocelot.Provider.Consul/UnableToSetConfigInConsulError.cs index 0e6481aad..62b08f93e 100644 --- a/src/Ocelot.Provider.Consul/UnableToSetConfigInConsulError.cs +++ b/src/Ocelot.Provider.Consul/UnableToSetConfigInConsulError.cs @@ -1,11 +1,12 @@ using Ocelot.Errors; +using HttpStatus = System.Net.HttpStatusCode; namespace Ocelot.Provider.Consul; public class UnableToSetConfigInConsulError : Error { public UnableToSetConfigInConsulError(string s) - : base(s, OcelotErrorCode.UnknownError, 404) + : base(s, OcelotErrorCode.UnknownError, (int)HttpStatus.NotFound) { } } diff --git a/src/Ocelot.Provider.Consul/Usings.cs b/src/Ocelot.Provider.Consul/Usings.cs index 777ae303e..30819a42e 100644 --- a/src/Ocelot.Provider.Consul/Usings.cs +++ b/src/Ocelot.Provider.Consul/Usings.cs @@ -1,5 +1,4 @@ // Default Microsoft.NET.Sdk namespaces - global using System; global using System.Collections.Generic; global using System.Linq; diff --git a/src/Ocelot.Provider.Eureka/Eureka.cs b/src/Ocelot.Provider.Eureka/Eureka.cs index cbb73785f..5fd6b4e73 100644 --- a/src/Ocelot.Provider.Eureka/Eureka.cs +++ b/src/Ocelot.Provider.Eureka/Eureka.cs @@ -14,12 +14,11 @@ public Eureka(string serviceName, IDiscoveryClient client) _serviceName = serviceName; } - public Task> Get() + public Task> GetAsync() { var services = new List(); var instances = _client.GetInstances(_serviceName); - if (instances != null && instances.Any()) { services.AddRange(instances.Select(i => new Service(i.ServiceId, new ServiceHostAndPort(i.Host, i.Port, i.Uri.Scheme), string.Empty, string.Empty, new List()))); diff --git a/src/Ocelot.Provider.Eureka/EurekaProviderFactory.cs b/src/Ocelot.Provider.Eureka/EurekaProviderFactory.cs index c45d0b540..da3c602ee 100644 --- a/src/Ocelot.Provider.Eureka/EurekaProviderFactory.cs +++ b/src/Ocelot.Provider.Eureka/EurekaProviderFactory.cs @@ -1,9 +1,6 @@ using Microsoft.Extensions.DependencyInjection; using Ocelot.Configuration; -using Ocelot.ServiceDiscovery; using Ocelot.ServiceDiscovery.Providers; -using Steeltoe.Discovery; -using System; namespace Ocelot.Provider.Eureka; @@ -20,11 +17,8 @@ private static IServiceDiscoveryProvider CreateProvider(IServiceProvider provide { var client = provider.GetService(); - if (Eureka.Equals(config.Type, StringComparison.OrdinalIgnoreCase) && client != null) - { - return new Eureka(route.ServiceName, client); - } - - return null; + return Eureka.Equals(config.Type, StringComparison.OrdinalIgnoreCase) && client != null + ? new Eureka(route.ServiceName, client) + : null; } } diff --git a/src/Ocelot.Provider.Eureka/OcelotBuilderExtensions.cs b/src/Ocelot.Provider.Eureka/OcelotBuilderExtensions.cs index 14e4685fb..ae4348b1f 100644 --- a/src/Ocelot.Provider.Eureka/OcelotBuilderExtensions.cs +++ b/src/Ocelot.Provider.Eureka/OcelotBuilderExtensions.cs @@ -8,9 +8,10 @@ public static class OcelotBuilderExtensions { public static IOcelotBuilder AddEureka(this IOcelotBuilder builder) { - builder.Services.AddDiscoveryClient(builder.Configuration); - builder.Services.AddSingleton(EurekaProviderFactory.Get); - builder.Services.AddSingleton(EurekaMiddlewareConfigurationProvider.Get); + builder.Services + .AddDiscoveryClient(builder.Configuration) + .AddSingleton(EurekaProviderFactory.Get) + .AddSingleton(EurekaMiddlewareConfigurationProvider.Get); return builder; } } diff --git a/src/Ocelot.Provider.Kubernetes/KubeApiClientExtensions/EndPointClientV1.cs b/src/Ocelot.Provider.Kubernetes/EndPointClientV1.cs similarity index 55% rename from src/Ocelot.Provider.Kubernetes/KubeApiClientExtensions/EndPointClientV1.cs rename to src/Ocelot.Provider.Kubernetes/EndPointClientV1.cs index 4246a03ec..bf1fc815c 100644 --- a/src/Ocelot.Provider.Kubernetes/KubeApiClientExtensions/EndPointClientV1.cs +++ b/src/Ocelot.Provider.Kubernetes/EndPointClientV1.cs @@ -1,16 +1,16 @@ using HTTPlease; -using KubeClient; using KubeClient.Models; using KubeClient.ResourceClients; -namespace Ocelot.Provider.Kubernetes.KubeApiClientExtensions +namespace Ocelot.Provider.Kubernetes { public class EndPointClientV1 : KubeResourceClient { - private readonly HttpRequest _collection = KubeRequest.Create("api/v1/namespaces/{Namespace}/endpoints/{ServiceName}"); + private readonly HttpRequest _collection; public EndPointClientV1(IKubeApiClient client) : base(client) { + _collection = KubeRequest.Create("api/v1/namespaces/{Namespace}/endpoints/{ServiceName}"); } public async Task Get(string serviceName, string kubeNamespace = null, CancellationToken cancellationToken = default) @@ -20,21 +20,18 @@ public async Task Get(string serviceName, string kubeNamespace = nu throw new ArgumentNullException(nameof(serviceName)); } - var response = await Http.GetAsync( - _collection.WithTemplateParameters(new + var request = _collection + .WithTemplateParameters(new { Namespace = kubeNamespace ?? KubeClient.DefaultNamespace, ServiceName = serviceName, - }), - cancellationToken - ); + }); - if (response.IsSuccessStatusCode) - { - return await response.ReadContentAsAsync(); - } + var response = await Http.GetAsync(request, cancellationToken); - return null; + return response.IsSuccessStatusCode + ? await response.ReadContentAsAsync() + : null; } } } diff --git a/src/Ocelot.Provider.Kubernetes/KubeRegistryConfiguration.cs b/src/Ocelot.Provider.Kubernetes/KubeRegistryConfiguration.cs index 3aea206e1..2a3d7e815 100644 --- a/src/Ocelot.Provider.Kubernetes/KubeRegistryConfiguration.cs +++ b/src/Ocelot.Provider.Kubernetes/KubeRegistryConfiguration.cs @@ -3,7 +3,6 @@ public class KubeRegistryConfiguration { public string KubeNamespace { get; set; } - public string KeyOfServiceInK8s { get; set; } } } diff --git a/src/Ocelot.Provider.Kubernetes/KubernetesProviderFactory.cs b/src/Ocelot.Provider.Kubernetes/KubernetesProviderFactory.cs index 5d9fae016..97d58f5eb 100644 --- a/src/Ocelot.Provider.Kubernetes/KubernetesProviderFactory.cs +++ b/src/Ocelot.Provider.Kubernetes/KubernetesProviderFactory.cs @@ -1,10 +1,6 @@ -using KubeClient; -using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection; using Ocelot.Configuration; using Ocelot.Logging; -using Ocelot.ServiceDiscovery; -using Ocelot.ServiceDiscovery.Providers; -using System; namespace Ocelot.Provider.Kubernetes { @@ -29,13 +25,10 @@ private static IServiceDiscoveryProvider CreateProvider(IServiceProvider provide }; var k8SServiceDiscoveryProvider = new KubernetesServiceDiscoveryProvider(k8SRegistryConfiguration, factory, kubeClient); - - if (PollKube.Equals(config.Type, StringComparison.OrdinalIgnoreCase)) - { - return new PollKube(config.PollingInterval, factory, k8SServiceDiscoveryProvider); - } - - return k8SServiceDiscoveryProvider; + + return PollKube.Equals(config.Type, StringComparison.OrdinalIgnoreCase) + ? new PollKube(config.PollingInterval, factory, k8SServiceDiscoveryProvider) + : k8SServiceDiscoveryProvider; } } } diff --git a/src/Ocelot.Provider.Kubernetes/KubernetesServiceDiscoveryProvider.cs b/src/Ocelot.Provider.Kubernetes/KubernetesServiceDiscoveryProvider.cs index 42dc09aea..ce3ca316f 100644 --- a/src/Ocelot.Provider.Kubernetes/KubernetesServiceDiscoveryProvider.cs +++ b/src/Ocelot.Provider.Kubernetes/KubernetesServiceDiscoveryProvider.cs @@ -1,8 +1,5 @@ -using KubeClient; -using KubeClient.Models; +using KubeClient.Models; using Ocelot.Logging; -using Ocelot.Provider.Kubernetes.KubeApiClientExtensions; -using Ocelot.ServiceDiscovery.Providers; using Ocelot.Values; namespace Ocelot.Provider.Kubernetes; @@ -20,7 +17,7 @@ public KubernetesServiceDiscoveryProvider(KubeRegistryConfiguration kubeRegistry _kubeApi = kubeApi; } - public async Task> Get() + public async Task> GetAsync() { var endpoint = await _kubeApi .ResourceClient(client => new EndPointClientV1(client)) diff --git a/src/Ocelot.Provider.Kubernetes/OcelotBuilderExtensions.cs b/src/Ocelot.Provider.Kubernetes/OcelotBuilderExtensions.cs index f979586a5..0110bddbe 100644 --- a/src/Ocelot.Provider.Kubernetes/OcelotBuilderExtensions.cs +++ b/src/Ocelot.Provider.Kubernetes/OcelotBuilderExtensions.cs @@ -1,5 +1,4 @@ -using KubeClient; -using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection; using Ocelot.DependencyInjection; namespace Ocelot.Provider.Kubernetes @@ -8,8 +7,9 @@ public static class OcelotBuilderExtensions { public static IOcelotBuilder AddKubernetes(this IOcelotBuilder builder, bool usePodServiceAccount = true) { - builder.Services.AddSingleton(KubernetesProviderFactory.Get); - builder.Services.AddKubeClient(usePodServiceAccount); + builder.Services + .AddSingleton(KubernetesProviderFactory.Get) + .AddKubeClient(usePodServiceAccount); return builder; } } diff --git a/src/Ocelot.Provider.Kubernetes/PollKube.cs b/src/Ocelot.Provider.Kubernetes/PollKube.cs index 31d42a56b..f2d70c3b9 100644 --- a/src/Ocelot.Provider.Kubernetes/PollKube.cs +++ b/src/Ocelot.Provider.Kubernetes/PollKube.cs @@ -1,17 +1,14 @@ using Ocelot.Logging; -using Ocelot.ServiceDiscovery.Providers; using Ocelot.Values; -using System.Collections.Generic; -using System.Threading; -using System.Threading.Tasks; namespace Ocelot.Provider.Kubernetes { - public class PollKube : IServiceDiscoveryProvider + public class PollKube : IServiceDiscoveryProvider, IDisposable { private readonly IOcelotLogger _logger; private readonly IServiceDiscoveryProvider _kubeServiceDiscoveryProvider; private readonly Timer _timer; + private bool _polling; private List _services; @@ -21,27 +18,34 @@ public PollKube(int pollingInterval, IOcelotLoggerFactory factory, IServiceDisco _kubeServiceDiscoveryProvider = kubeServiceDiscoveryProvider; _services = new List(); - _timer = new Timer(async x => + _timer = new Timer(OnTimerCallbackAsync, null, pollingInterval, pollingInterval); + } + + private async void OnTimerCallbackAsync(object state) + { + if (_polling) { - if (_polling) - { - return; - } - - _polling = true; - await Poll(); - _polling = false; - }, null, pollingInterval, pollingInterval); + return; + } + + _polling = true; + await Poll(); + _polling = false; } - public Task> Get() + public Task> GetAsync() { return Task.FromResult(_services); } private async Task Poll() { - _services = await _kubeServiceDiscoveryProvider.Get(); + _services = await _kubeServiceDiscoveryProvider.GetAsync(); + } + + public void Dispose() + { + _timer.Dispose(); } } } diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/CookieStickySessionsCreator.cs b/src/Ocelot/LoadBalancer/LoadBalancers/CookieStickySessionsCreator.cs index 160ba17ef..2994bdbdc 100644 --- a/src/Ocelot/LoadBalancer/LoadBalancers/CookieStickySessionsCreator.cs +++ b/src/Ocelot/LoadBalancer/LoadBalancers/CookieStickySessionsCreator.cs @@ -9,7 +9,7 @@ public class CookieStickySessionsCreator : ILoadBalancerCreator { public Response Create(DownstreamRoute route, IServiceDiscoveryProvider serviceProvider) { - var loadBalancer = new RoundRobin(async () => await serviceProvider.Get()); + var loadBalancer = new RoundRobin(async () => await serviceProvider.GetAsync()); var bus = new InMemoryBus(); return new OkResponse(new CookieStickySessions(loadBalancer, route.LoadBalancerOptions.Key, route.LoadBalancerOptions.ExpiryInMs, bus)); diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/LeastConnectionCreator.cs b/src/Ocelot/LoadBalancer/LoadBalancers/LeastConnectionCreator.cs index a4880f75e..e5d15fa2d 100644 --- a/src/Ocelot/LoadBalancer/LoadBalancers/LeastConnectionCreator.cs +++ b/src/Ocelot/LoadBalancer/LoadBalancers/LeastConnectionCreator.cs @@ -8,7 +8,7 @@ public class LeastConnectionCreator : ILoadBalancerCreator { public Response Create(DownstreamRoute route, IServiceDiscoveryProvider serviceProvider) { - return new OkResponse(new LeastConnection(async () => await serviceProvider.Get(), route.ServiceName)); + return new OkResponse(new LeastConnection(async () => await serviceProvider.GetAsync(), route.ServiceName)); } public string Type => nameof(LeastConnection); diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/NoLoadBalancerCreator.cs b/src/Ocelot/LoadBalancer/LoadBalancers/NoLoadBalancerCreator.cs index de4c9c801..a7a931617 100644 --- a/src/Ocelot/LoadBalancer/LoadBalancers/NoLoadBalancerCreator.cs +++ b/src/Ocelot/LoadBalancer/LoadBalancers/NoLoadBalancerCreator.cs @@ -8,7 +8,7 @@ public class NoLoadBalancerCreator : ILoadBalancerCreator { public Response Create(DownstreamRoute route, IServiceDiscoveryProvider serviceProvider) { - return new OkResponse(new NoLoadBalancer(async () => await serviceProvider.Get())); + return new OkResponse(new NoLoadBalancer(async () => await serviceProvider.GetAsync())); } public string Type => nameof(NoLoadBalancer); diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/RoundRobinCreator.cs b/src/Ocelot/LoadBalancer/LoadBalancers/RoundRobinCreator.cs index 0929a5e10..e0c0e1a81 100644 --- a/src/Ocelot/LoadBalancer/LoadBalancers/RoundRobinCreator.cs +++ b/src/Ocelot/LoadBalancer/LoadBalancers/RoundRobinCreator.cs @@ -8,7 +8,7 @@ public class RoundRobinCreator : ILoadBalancerCreator { public Response Create(DownstreamRoute route, IServiceDiscoveryProvider serviceProvider) { - return new OkResponse(new RoundRobin(async () => await serviceProvider.Get())); + return new OkResponse(new RoundRobin(async () => await serviceProvider.GetAsync())); } public string Type => nameof(RoundRobin); diff --git a/src/Ocelot/ServiceDiscovery/Providers/ConfigurationServiceProvider.cs b/src/Ocelot/ServiceDiscovery/Providers/ConfigurationServiceProvider.cs index 6f9488098..b417c4c7c 100644 --- a/src/Ocelot/ServiceDiscovery/Providers/ConfigurationServiceProvider.cs +++ b/src/Ocelot/ServiceDiscovery/Providers/ConfigurationServiceProvider.cs @@ -11,7 +11,7 @@ public ConfigurationServiceProvider(List services) _services = services; } - public async Task> Get() + public async Task> GetAsync() { return await Task.FromResult(_services); } diff --git a/src/Ocelot/ServiceDiscovery/Providers/IServiceDiscoveryProvider.cs b/src/Ocelot/ServiceDiscovery/Providers/IServiceDiscoveryProvider.cs index 7d0d3684b..7127cb489 100644 --- a/src/Ocelot/ServiceDiscovery/Providers/IServiceDiscoveryProvider.cs +++ b/src/Ocelot/ServiceDiscovery/Providers/IServiceDiscoveryProvider.cs @@ -4,6 +4,6 @@ namespace Ocelot.ServiceDiscovery.Providers { public interface IServiceDiscoveryProvider { - Task> Get(); + Task> GetAsync(); } } diff --git a/src/Ocelot/ServiceDiscovery/Providers/ServiceFabricServiceDiscoveryProvider.cs b/src/Ocelot/ServiceDiscovery/Providers/ServiceFabricServiceDiscoveryProvider.cs index a48d7b7d6..dcf2bf389 100644 --- a/src/Ocelot/ServiceDiscovery/Providers/ServiceFabricServiceDiscoveryProvider.cs +++ b/src/Ocelot/ServiceDiscovery/Providers/ServiceFabricServiceDiscoveryProvider.cs @@ -12,7 +12,7 @@ public ServiceFabricServiceDiscoveryProvider(ServiceFabricConfiguration configur _configuration = configuration; } - public Task> Get() + public Task> GetAsync() { return Task.FromResult(new List { diff --git a/src/Ocelot/ServiceDiscovery/ServiceDiscoveryProviderFactory.cs b/src/Ocelot/ServiceDiscovery/ServiceDiscoveryProviderFactory.cs index c58bd1b9f..174c973ab 100644 --- a/src/Ocelot/ServiceDiscovery/ServiceDiscoveryProviderFactory.cs +++ b/src/Ocelot/ServiceDiscovery/ServiceDiscoveryProviderFactory.cs @@ -5,15 +5,13 @@ using Ocelot.ServiceDiscovery.Configuration; using Ocelot.ServiceDiscovery.Providers; using Ocelot.Values; -using System; -using System.Collections.Generic; namespace Ocelot.ServiceDiscovery { public class ServiceDiscoveryProviderFactory : IServiceDiscoveryProviderFactory { - private readonly ServiceDiscoveryFinderDelegate _delegates; private readonly IServiceProvider _provider; + private readonly ServiceDiscoveryFinderDelegate _delegates; private readonly IOcelotLogger _logger; public ServiceDiscoveryProviderFactory(IOcelotLoggerFactory factory, IServiceProvider provider) @@ -32,14 +30,14 @@ public Response Get(ServiceProviderConfiguration serv return GetServiceDiscoveryProvider(serviceConfig, route); } - var services = new List(); - - foreach (var downstreamAddress in route.DownstreamAddresses) - { - var service = new Service(route.ServiceName, new ServiceHostAndPort(downstreamAddress.Host, downstreamAddress.Port, route.DownstreamScheme), string.Empty, string.Empty, Array.Empty()); - - services.Add(service); - } + var services = route.DownstreamAddresses + .Select(address => new Service( + route.ServiceName, + new ServiceHostAndPort(address.Host, address.Port, route.DownstreamScheme), + string.Empty, + string.Empty, + Enumerable.Empty())) + .ToList(); return new OkResponse(new ConfigurationServiceProvider(services)); } diff --git a/test/Ocelot.AcceptanceTests/LoadBalancerTests.cs b/test/Ocelot.AcceptanceTests/LoadBalancerTests.cs index 91ec136e1..dfc4c3486 100644 --- a/test/Ocelot.AcceptanceTests/LoadBalancerTests.cs +++ b/test/Ocelot.AcceptanceTests/LoadBalancerTests.cs @@ -154,7 +154,7 @@ public void should_load_balance_request_with_custom_load_balancer() GlobalConfiguration = new FileGlobalConfiguration(), }; - Func loadBalancerFactoryFunc = (serviceProvider, route, serviceDiscoveryProvider) => new CustomLoadBalancer(serviceDiscoveryProvider.Get); + Func loadBalancerFactoryFunc = (serviceProvider, route, serviceDiscoveryProvider) => new CustomLoadBalancer(serviceDiscoveryProvider.GetAsync); this.Given(x => x.GivenProductServiceOneIsRunning(downstreamServiceOneUrl, 200)) .And(x => x.GivenProductServiceTwoIsRunning(downstreamServiceTwoUrl, 200)) diff --git a/test/Ocelot.AcceptanceTests/SequentialTestsCollection.cs b/test/Ocelot.AcceptanceTests/SequentialTestsCollection.cs deleted file mode 100644 index 2b9c8d474..000000000 --- a/test/Ocelot.AcceptanceTests/SequentialTestsCollection.cs +++ /dev/null @@ -1,8 +0,0 @@ -using Xunit; - -namespace Ocelot.AcceptanceTests; - -[CollectionDefinition("Sequential", DisableParallelization = true)] -public class SequentialTestsCollection -{ -} diff --git a/test/Ocelot.UnitTests/Configuration/Validation/FileConfigurationFluentValidatorTests.cs b/test/Ocelot.UnitTests/Configuration/Validation/FileConfigurationFluentValidatorTests.cs index 849f65159..98bfd1ea0 100644 --- a/test/Ocelot.UnitTests/Configuration/Validation/FileConfigurationFluentValidatorTests.cs +++ b/test/Ocelot.UnitTests/Configuration/Validation/FileConfigurationFluentValidatorTests.cs @@ -1553,7 +1553,7 @@ private void GivenAServiceDiscoveryHandler() private class FakeServiceDiscoveryProvider : IServiceDiscoveryProvider { - public Task> Get() + public Task> GetAsync() { throw new System.NotImplementedException(); } diff --git a/test/Ocelot.UnitTests/Consul/ConsulServiceDiscoveryProviderTests.cs b/test/Ocelot.UnitTests/Consul/ConsulServiceDiscoveryProviderTests.cs index 9b26c1beb..4a9daa41b 100644 --- a/test/Ocelot.UnitTests/Consul/ConsulServiceDiscoveryProviderTests.cs +++ b/test/Ocelot.UnitTests/Consul/ConsulServiceDiscoveryProviderTests.cs @@ -243,7 +243,7 @@ private void ThenTheCountIs(int count) private void WhenIGetTheServices() { - _services = _provider.Get().GetAwaiter().GetResult(); + _services = _provider.GetAsync().GetAwaiter().GetResult(); } private void ThenTheTokenIs(string token) diff --git a/test/Ocelot.UnitTests/Consul/PollingConsulServiceDiscoveryProviderTests.cs b/test/Ocelot.UnitTests/Consul/PollingConsulServiceDiscoveryProviderTests.cs index 1313923c0..a5dc99585 100644 --- a/test/Ocelot.UnitTests/Consul/PollingConsulServiceDiscoveryProviderTests.cs +++ b/test/Ocelot.UnitTests/Consul/PollingConsulServiceDiscoveryProviderTests.cs @@ -50,7 +50,7 @@ public void should_return_service_from_consul_without_delay() private void GivenConsulReturns(Service service) { _services.Add(service); - _consulServiceDiscoveryProvider.Setup(x => x.Get()).ReturnsAsync(_services); + _consulServiceDiscoveryProvider.Setup(x => x.GetAsync()).ReturnsAsync(_services); } private void ThenTheCountIs(int count) @@ -65,7 +65,7 @@ private void WhenIGetTheServices(int expected) { try { - _result = provider.Get().GetAwaiter().GetResult(); + _result = provider.GetAsync().GetAwaiter().GetResult(); return _result.Count == expected; } catch (Exception) @@ -83,7 +83,7 @@ private void WhenIGetTheServicesWithoutDelay(int expected) bool result; try { - _result = provider.Get().GetAwaiter().GetResult(); + _result = provider.GetAsync().GetAwaiter().GetResult(); result = _result.Count == expected; } catch (Exception) diff --git a/test/Ocelot.UnitTests/Consul/ProviderFactoryTests.cs b/test/Ocelot.UnitTests/Consul/ProviderFactoryTests.cs index f4265b5b7..623d31b3d 100644 --- a/test/Ocelot.UnitTests/Consul/ProviderFactoryTests.cs +++ b/test/Ocelot.UnitTests/Consul/ProviderFactoryTests.cs @@ -1,14 +1,9 @@ -using System; -using System.Linq; -using Microsoft.Extensions.DependencyInjection; -using Moq; +using Microsoft.Extensions.DependencyInjection; using Ocelot.Configuration; using Ocelot.Configuration.Builder; using Ocelot.Logging; using Ocelot.Provider.Consul; using Ocelot.ServiceDiscovery.Providers; -using Shouldly; -using Xunit; namespace Ocelot.UnitTests.Consul; @@ -30,7 +25,7 @@ public ProviderFactoryTests() } [Fact] - public void should_return_ConsulServiceDiscoveryProvider() + public void ShouldReturnConsulServiceDiscoveryProvider() { var route = new DownstreamRouteBuilder() .WithServiceName(string.Empty) @@ -43,7 +38,7 @@ public void should_return_ConsulServiceDiscoveryProvider() } [Fact] - public void should_return_PollingConsulServiceDiscoveryProvider() + public void ShouldReturnPollingConsulServiceDiscoveryProvider() { var provider = DummyPollingConsulServiceFactory(string.Empty); var pollProvider = provider as PollConsul; @@ -51,7 +46,7 @@ public void should_return_PollingConsulServiceDiscoveryProvider() } [Fact] - public void should_return_SameProviderForGivenServiceName() + public void ShouldReturnSameProviderForGivenServiceName() { var provider = DummyPollingConsulServiceFactory("test"); var provider2 = DummyPollingConsulServiceFactory("test"); @@ -69,7 +64,7 @@ public void should_return_SameProviderForGivenServiceName() [Theory] [InlineData(new object[] { new[] { "service1", "service2", "service3", "service4" } })] - public void should_return_ProviderAccordingToServiceName(string[] serviceNames) + public void ShouldReturnProviderAccordingToServiceName(string[] serviceNames) { var providersList = serviceNames.Select(DummyPollingConsulServiceFactory).ToList(); @@ -93,12 +88,14 @@ public void should_return_ProviderAccordingToServiceName(string[] serviceNames) convertedCProvider.ShouldNotBeNull(); - var matchingProviders = convertedProvidersList.Where(x => x.ServiceName == convertedCProvider.ServiceName) + var matchingProviders = convertedProvidersList + .Where(x => x.ServiceName == convertedCProvider.ServiceName) .ToList(); matchingProviders.ShouldHaveSingleItem(); - matchingProviders.First().ShouldNotBeNull(); - matchingProviders.First().ServiceName.ShouldBeEquivalentTo(convertedCProvider.ServiceName); + matchingProviders.First() + .ShouldNotBeNull() + .ServiceName.ShouldBeEquivalentTo(convertedCProvider.ServiceName); } } @@ -110,8 +107,9 @@ private IServiceDiscoveryProvider DummyPollingConsulServiceFactory(string servic .WithServiceName(serviceName) .Build(); - return ConsulProviderFactory.Get(_provider, - new ServiceProviderConfiguration("pollconsul", "http", string.Empty, 1, string.Empty, string.Empty, - stopsFromPolling), route); + return ConsulProviderFactory.Get?.Invoke( + _provider, + new ServiceProviderConfiguration(ConsulProviderFactory.PollConsul, Uri.UriSchemeHttp, string.Empty, 1, string.Empty, string.Empty, stopsFromPolling), + route); } } diff --git a/test/Ocelot.UnitTests/Eureka/EurekaServiceDiscoveryProviderTests.cs b/test/Ocelot.UnitTests/Eureka/EurekaServiceDiscoveryProviderTests.cs index 1aa775140..b14534afc 100644 --- a/test/Ocelot.UnitTests/Eureka/EurekaServiceDiscoveryProviderTests.cs +++ b/test/Ocelot.UnitTests/Eureka/EurekaServiceDiscoveryProviderTests.cs @@ -79,7 +79,7 @@ private void ThenTheClientIsCalledCorrectly() private async Task WhenIGet() { - _result = await _provider.Get(); + _result = await _provider.GetAsync(); } private void GivenThe(List instances) diff --git a/test/Ocelot.UnitTests/Kubernetes/KubeServiceDiscoveryProviderTests.cs b/test/Ocelot.UnitTests/Kubernetes/KubeServiceDiscoveryProviderTests.cs index 49174ec6f..2e9fe7ab5 100644 --- a/test/Ocelot.UnitTests/Kubernetes/KubeServiceDiscoveryProviderTests.cs +++ b/test/Ocelot.UnitTests/Kubernetes/KubeServiceDiscoveryProviderTests.cs @@ -32,7 +32,7 @@ public KubeServiceDiscoveryProviderTests() _namespaces = "dev"; _port = 5567; _kubeHost = "localhost"; - _fakekubeServiceDiscoveryUrl = $"http://{_kubeHost}:{_port}"; + _fakekubeServiceDiscoveryUrl = $"{Uri.UriSchemeHttp}://{_kubeHost}:{_port}"; _endpointEntries = new EndpointsV1(); _factory = new Mock(); @@ -100,7 +100,7 @@ private void ThenTheCountIs(int count) private void WhenIGetTheServices() { - _services = _provider.Get().GetAwaiter().GetResult(); + _services = _provider.GetAsync().GetAwaiter().GetResult(); } private void GivenTheServicesAreRegisteredWithKube(EndpointsV1 endpointEntries) diff --git a/test/Ocelot.UnitTests/Kubernetes/PollingKubeServiceDiscoveryProviderTests.cs b/test/Ocelot.UnitTests/Kubernetes/PollingKubeServiceDiscoveryProviderTests.cs index 3d21728df..94ea8884d 100644 --- a/test/Ocelot.UnitTests/Kubernetes/PollingKubeServiceDiscoveryProviderTests.cs +++ b/test/Ocelot.UnitTests/Kubernetes/PollingKubeServiceDiscoveryProviderTests.cs @@ -40,7 +40,7 @@ public void should_return_service_from_kube() private void GivenKubeReturns(Service service) { _services.Add(service); - _kubeServiceDiscoveryProvider.Setup(x => x.Get()).ReturnsAsync(_services); + _kubeServiceDiscoveryProvider.Setup(x => x.GetAsync()).ReturnsAsync(_services); } private void ThenTheCountIs(int count) @@ -56,7 +56,7 @@ private void WhenIGetTheServices(int expected) { try { - _result = _provider.Get().GetAwaiter().GetResult(); + _result = _provider.GetAsync().GetAwaiter().GetResult(); if (_result.Count == expected) { return true; diff --git a/test/Ocelot.UnitTests/ServiceDiscovery/ConfigurationServiceProviderTests.cs b/test/Ocelot.UnitTests/ServiceDiscovery/ConfigurationServiceProviderTests.cs index 200b87ebf..1de3f5cea 100644 --- a/test/Ocelot.UnitTests/ServiceDiscovery/ConfigurationServiceProviderTests.cs +++ b/test/Ocelot.UnitTests/ServiceDiscovery/ConfigurationServiceProviderTests.cs @@ -33,7 +33,7 @@ private void GivenServices(List services) private void WhenIGetTheService() { _serviceProvider = new ConfigurationServiceProvider(_expected); - _result = _serviceProvider.Get().Result; + _result = _serviceProvider.GetAsync().Result; } private void ThenTheFollowingIsReturned(List services) diff --git a/test/Ocelot.UnitTests/ServiceDiscovery/ServiceDiscoveryProviderFactoryTests.cs b/test/Ocelot.UnitTests/ServiceDiscovery/ServiceDiscoveryProviderFactoryTests.cs index d83f146d1..fd4fb72e5 100644 --- a/test/Ocelot.UnitTests/ServiceDiscovery/ServiceDiscoveryProviderFactoryTests.cs +++ b/test/Ocelot.UnitTests/ServiceDiscovery/ServiceDiscoveryProviderFactoryTests.cs @@ -134,7 +134,7 @@ private void GivenAFakeDelegate() private class Fake : IServiceDiscoveryProvider { - public Task> Get() + public Task> GetAsync() { return null; } @@ -164,7 +164,7 @@ private void ThenTheResultIsError() private void ThenTheFollowingServicesAreReturned(List downstreamAddresses) { var result = (ConfigurationServiceProvider)_result.Data; - var services = result.Get().Result; + var services = result.GetAsync().Result; for (var i = 0; i < services.Count; i++) { diff --git a/test/Ocelot.UnitTests/ServiceDiscovery/ServiceFabricServiceDiscoveryProviderTests.cs b/test/Ocelot.UnitTests/ServiceDiscovery/ServiceFabricServiceDiscoveryProviderTests.cs index 603555396..0836afffc 100644 --- a/test/Ocelot.UnitTests/ServiceDiscovery/ServiceFabricServiceDiscoveryProviderTests.cs +++ b/test/Ocelot.UnitTests/ServiceDiscovery/ServiceFabricServiceDiscoveryProviderTests.cs @@ -33,7 +33,7 @@ private void WhenIGet() { _config = new ServiceFabricConfiguration(_host, _port, _serviceName); _provider = new ServiceFabricServiceDiscoveryProvider(_config); - _services = _provider.Get().GetAwaiter().GetResult(); + _services = _provider.GetAsync().GetAwaiter().GetResult(); } private void ThenTheServiceFabricNamingServiceIsRetured() From b905fde4d3d2f20d62fac35b1a3561941d6c6c90 Mon Sep 17 00:00:00 2001 From: raman-m Date: Sat, 30 Sep 2023 15:14:25 +0300 Subject: [PATCH 2/9] Fix unit tests for ConsulServiceDiscoveryProvider --- src/Ocelot.Provider.Consul/Consul.cs | 10 ++-- .../ConsulServiceDiscoveryProviderTests.cs | 49 ++++--------------- 2 files changed, 15 insertions(+), 44 deletions(-) diff --git a/src/Ocelot.Provider.Consul/Consul.cs b/src/Ocelot.Provider.Consul/Consul.cs index 8c7caf0a4..d6281c514 100644 --- a/src/Ocelot.Provider.Consul/Consul.cs +++ b/src/Ocelot.Provider.Consul/Consul.cs @@ -19,6 +19,8 @@ public Consul(ConsulRegistryConfiguration config, IOcelotLoggerFactory factory, _logger = factory.CreateLogger(); } + public const string ServiceValidationWarningFormat = "Unable to use service address: '{0}' and port: {1} as it is invalid for the service: '{2}'. Address must contain host only e.g. 'localhost', and port must be greater than 0."; + public async Task> GetAsync() { var queryResult = await _consul.Health.Service(_config.KeyOfServiceInConsul, string.Empty, true); @@ -27,8 +29,7 @@ public async Task> GetAsync() foreach (var serviceEntry in queryResult.Response) { - var address = serviceEntry.Service.Address; - var port = serviceEntry.Service.Port; + var service = serviceEntry.Service; if (IsValid(serviceEntry)) { var nodes = await _consul.Catalog.Nodes(); @@ -38,14 +39,13 @@ public async Task> GetAsync() } else { - var serviceNode = nodes.Response.FirstOrDefault(n => n.Address == address); + var serviceNode = nodes.Response.FirstOrDefault(n => n.Address == service.Address); services.Add(BuildService(serviceEntry, serviceNode)); } } else { - _logger.LogWarning( - $"Unable to use service address: {address} and Port: {port} as it is invalid. Address must contain host only e.g. 'localhost', and port must be greater than 0."); + _logger.LogWarning(string.Format(ServiceValidationWarningFormat, service.Address, service.Port, service.Service)); } } diff --git a/test/Ocelot.UnitTests/Consul/ConsulServiceDiscoveryProviderTests.cs b/test/Ocelot.UnitTests/Consul/ConsulServiceDiscoveryProviderTests.cs index 4a9daa41b..23761a212 100644 --- a/test/Ocelot.UnitTests/Consul/ConsulServiceDiscoveryProviderTests.cs +++ b/test/Ocelot.UnitTests/Consul/ConsulServiceDiscoveryProviderTests.cs @@ -123,7 +123,7 @@ public void should_not_return_services_with_invalid_address() .And(x => GivenTheServicesAreRegisteredWithConsul(serviceEntryOne, serviceEntryTwo)) .When(x => WhenIGetTheServices()) .Then(x => ThenTheCountIs(0)) - .And(x => ThenTheLoggerHasBeenCalledCorrectlyForInvalidAddress()) + .And(x => ThenTheLoggerHasBeenCalledCorrectlyWithValidationWarning(serviceEntryOne, serviceEntryTwo)) .BDDfy(); } @@ -158,7 +158,7 @@ public void should_not_return_services_with_empty_address() .And(x => GivenTheServicesAreRegisteredWithConsul(serviceEntryOne, serviceEntryTwo)) .When(x => WhenIGetTheServices()) .Then(x => ThenTheCountIs(0)) - .And(x => ThenTheLoggerHasBeenCalledCorrectlyForEmptyAddress()) + .And(x => ThenTheLoggerHasBeenCalledCorrectlyWithValidationWarning(serviceEntryOne, serviceEntryTwo)) .BDDfy(); } @@ -193,47 +193,18 @@ public void should_not_return_services_with_invalid_port() .And(x => GivenTheServicesAreRegisteredWithConsul(serviceEntryOne, serviceEntryTwo)) .When(x => WhenIGetTheServices()) .Then(x => ThenTheCountIs(0)) - .And(x => ThenTheLoggerHasBeenCalledCorrectlyForInvalidPorts()) + .And(x => ThenTheLoggerHasBeenCalledCorrectlyWithValidationWarning(serviceEntryOne, serviceEntryTwo)) .BDDfy(); } - private void ThenTheLoggerHasBeenCalledCorrectlyForInvalidAddress() + private void ThenTheLoggerHasBeenCalledCorrectlyWithValidationWarning(params ServiceEntry[] serviceEntries) { - _logger.Verify( - x => x.LogWarning( - "Unable to use service Address: http://localhost and Port: 50881 as it is invalid. Address must contain host only e.g. localhost and port must be greater than 0"), - Times.Once); - - _logger.Verify( - x => x.LogWarning( - "Unable to use service Address: http://localhost and Port: 50888 as it is invalid. Address must contain host only e.g. localhost and port must be greater than 0"), - Times.Once); - } - - private void ThenTheLoggerHasBeenCalledCorrectlyForEmptyAddress() - { - _logger.Verify( - x => x.LogWarning( - "Unable to use service Address: and Port: 50881 as it is invalid. Address must contain host only e.g. localhost and port must be greater than 0"), - Times.Once); - - _logger.Verify( - x => x.LogWarning( - "Unable to use service Address: and Port: 50888 as it is invalid. Address must contain host only e.g. localhost and port must be greater than 0"), - Times.Once); - } - - private void ThenTheLoggerHasBeenCalledCorrectlyForInvalidPorts() - { - _logger.Verify( - x => x.LogWarning( - "Unable to use service Address: localhost and Port: -1 as it is invalid. Address must contain host only e.g. localhost and port must be greater than 0"), - Times.Once); - - _logger.Verify( - x => x.LogWarning( - "Unable to use service Address: localhost and Port: 0 as it is invalid. Address must contain host only e.g. localhost and port must be greater than 0"), - Times.Once); + foreach (var entry in serviceEntries) + { + var service = entry.Service; + var expected = string.Format(ConsulProvider.ServiceValidationWarningFormat, service.Address, service.Port, service.Service); + _logger.Verify(x => x.LogWarning(expected), Times.Once); + } } private void ThenTheCountIs(int count) From 679c4fff46d90139b97422f8c1db03df5c71d40a Mon Sep 17 00:00:00 2001 From: raman-m Date: Mon, 2 Oct 2023 10:51:04 +0300 Subject: [PATCH 3/9] Add default port --- .../ConsulRegistryConfiguration.cs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/Ocelot.Provider.Consul/ConsulRegistryConfiguration.cs b/src/Ocelot.Provider.Consul/ConsulRegistryConfiguration.cs index 7147f4e34..255c7b686 100644 --- a/src/Ocelot.Provider.Consul/ConsulRegistryConfiguration.cs +++ b/src/Ocelot.Provider.Consul/ConsulRegistryConfiguration.cs @@ -2,10 +2,18 @@ public class ConsulRegistryConfiguration { + /// + /// Consul HTTP client default port. + /// + /// HashiCorp Developer docs: Consul | Required Ports. + /// + /// + public const int DefaultHttpPort = 8500; + public ConsulRegistryConfiguration(string scheme, string host, int port, string keyOfServiceInConsul, string token) { Host = string.IsNullOrEmpty(host) ? "localhost" : host; - Port = port > 0 ? port : 8500; // Is 8500 default port of Consul? + Port = port > 0 ? port : DefaultHttpPort; Scheme = string.IsNullOrEmpty(scheme) ? Uri.UriSchemeHttp : scheme; KeyOfServiceInConsul = keyOfServiceInConsul; Token = token; From ef754e101d3334b448a1f257305c7fc45171aa08 Mon Sep 17 00:00:00 2001 From: raman-m Date: Mon, 2 Oct 2023 11:08:44 +0300 Subject: [PATCH 4/9] Throw NullRef exception if no client --- src/Ocelot.Provider.Eureka/Eureka.cs | 6 +++--- src/Ocelot.Provider.Eureka/EurekaProviderFactory.cs | 6 +++++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/Ocelot.Provider.Eureka/Eureka.cs b/src/Ocelot.Provider.Eureka/Eureka.cs index 5fd6b4e73..9b1372ddc 100644 --- a/src/Ocelot.Provider.Eureka/Eureka.cs +++ b/src/Ocelot.Provider.Eureka/Eureka.cs @@ -5,13 +5,13 @@ namespace Ocelot.Provider.Eureka { public class Eureka : IServiceDiscoveryProvider { - private readonly IDiscoveryClient _client; private readonly string _serviceName; + private readonly IDiscoveryClient _client; public Eureka(string serviceName, IDiscoveryClient client) { - _client = client; - _serviceName = serviceName; + _serviceName = serviceName ?? throw new ArgumentNullException(nameof(serviceName)); + _client = client ?? throw new ArgumentNullException(nameof(client)); } public Task> GetAsync() diff --git a/src/Ocelot.Provider.Eureka/EurekaProviderFactory.cs b/src/Ocelot.Provider.Eureka/EurekaProviderFactory.cs index da3c602ee..c1e545a28 100644 --- a/src/Ocelot.Provider.Eureka/EurekaProviderFactory.cs +++ b/src/Ocelot.Provider.Eureka/EurekaProviderFactory.cs @@ -16,8 +16,12 @@ public static class EurekaProviderFactory private static IServiceDiscoveryProvider CreateProvider(IServiceProvider provider, ServiceProviderConfiguration config, DownstreamRoute route) { var client = provider.GetService(); + if (client == null) + { + throw new NullReferenceException($"Cannot get an {nameof(IDiscoveryClient)} service during {nameof(CreateProvider)} operation to instanciate the {nameof(Eureka)} provider!"); + } - return Eureka.Equals(config.Type, StringComparison.OrdinalIgnoreCase) && client != null + return Eureka.Equals(config.Type, StringComparison.OrdinalIgnoreCase) ? new Eureka(route.ServiceName, client) : null; } From 39fd5382d28fed08c30194c8aa81298b39d23125 Mon Sep 17 00:00:00 2001 From: raman-m Date: Mon, 2 Oct 2023 11:18:32 +0300 Subject: [PATCH 5/9] Fix 'should_not_get' unit test --- test/Ocelot.UnitTests/Eureka/EurekaProviderFactoryTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/Ocelot.UnitTests/Eureka/EurekaProviderFactoryTests.cs b/test/Ocelot.UnitTests/Eureka/EurekaProviderFactoryTests.cs index 2b38d760b..c41f6bc62 100644 --- a/test/Ocelot.UnitTests/Eureka/EurekaProviderFactoryTests.cs +++ b/test/Ocelot.UnitTests/Eureka/EurekaProviderFactoryTests.cs @@ -12,8 +12,8 @@ public void should_not_get() { var config = new ServiceProviderConfigurationBuilder().Build(); var sp = new ServiceCollection().BuildServiceProvider(); - var provider = EurekaProviderFactory.Get(sp, config, null); - provider.ShouldBeNull(); + Should.Throw(() => + EurekaProviderFactory.Get(sp, config, null)); } [Fact] From 5851300815762cfe23304fc23defef2fbbabd119 Mon Sep 17 00:00:00 2001 From: raman-m Date: Mon, 2 Oct 2023 15:06:30 +0300 Subject: [PATCH 6/9] Add AgentService extensions --- .../Consul/AgentServiceExtensions.cs | 29 ++++++ .../ConsulServiceDiscoveryProviderTests.cs | 93 +++++-------------- 2 files changed, 53 insertions(+), 69 deletions(-) create mode 100644 test/Ocelot.UnitTests/Consul/AgentServiceExtensions.cs diff --git a/test/Ocelot.UnitTests/Consul/AgentServiceExtensions.cs b/test/Ocelot.UnitTests/Consul/AgentServiceExtensions.cs new file mode 100644 index 000000000..da60ff529 --- /dev/null +++ b/test/Ocelot.UnitTests/Consul/AgentServiceExtensions.cs @@ -0,0 +1,29 @@ +using Consul; + +namespace Ocelot.UnitTests.Consul; + +internal static class AgentServiceExtensions +{ + public static AgentService WithServiceName(this AgentService agent, string serviceName) + { + agent.Service = serviceName; + return agent; + } + + public static AgentService WithPort(this AgentService agent, int port) + { + agent.Port = port; + return agent; + } + + public static AgentService WithAddress(this AgentService agent, string address) + { + agent.Address = address; + return agent; + } + + public static ServiceEntry ToServiceEntry(this AgentService agent) => new() + { + Service = agent, + }; +} diff --git a/test/Ocelot.UnitTests/Consul/ConsulServiceDiscoveryProviderTests.cs b/test/Ocelot.UnitTests/Consul/ConsulServiceDiscoveryProviderTests.cs index 23761a212..cab644f2e 100644 --- a/test/Ocelot.UnitTests/Consul/ConsulServiceDiscoveryProviderTests.cs +++ b/test/Ocelot.UnitTests/Consul/ConsulServiceDiscoveryProviderTests.cs @@ -95,29 +95,10 @@ public void should_use_token() [Fact] public void should_not_return_services_with_invalid_address() { - var serviceEntryOne = new ServiceEntry - { - Service = new AgentService - { - Service = _serviceName, - Address = "http://localhost", - Port = 50881, - ID = Guid.NewGuid().ToString(), - Tags = Array.Empty(), - }, - }; - - var serviceEntryTwo = new ServiceEntry - { - Service = new AgentService - { - Service = _serviceName, - Address = "http://localhost", - Port = 50888, - ID = Guid.NewGuid().ToString(), - Tags = Array.Empty(), - }, - }; + var serviceEntryOne = GivenService(address: "http://localhost", port: 50881) + .ToServiceEntry(); + var serviceEntryTwo = GivenService(address: "http://localhost", port: 50888) + .ToServiceEntry(); this.Given(x => GivenThereIsAFakeConsulServiceDiscoveryProvider(_fakeConsulServiceDiscoveryUrl, _serviceName)) .And(x => GivenTheServicesAreRegisteredWithConsul(serviceEntryOne, serviceEntryTwo)) @@ -130,29 +111,12 @@ public void should_not_return_services_with_invalid_address() [Fact] public void should_not_return_services_with_empty_address() { - var serviceEntryOne = new ServiceEntry - { - Service = new AgentService - { - Service = _serviceName, - Address = string.Empty, - Port = 50881, - ID = Guid.NewGuid().ToString(), - Tags = Array.Empty(), - }, - }; - - var serviceEntryTwo = new ServiceEntry - { - Service = new AgentService - { - Service = _serviceName, - Address = null, - Port = 50888, - ID = Guid.NewGuid().ToString(), - Tags = Array.Empty(), - }, - }; + var serviceEntryOne = GivenService(port: 50881) + .WithAddress(string.Empty) + .ToServiceEntry(); + var serviceEntryTwo = GivenService(port: 50888) + .WithAddress(null) + .ToServiceEntry(); this.Given(x => GivenThereIsAFakeConsulServiceDiscoveryProvider(_fakeConsulServiceDiscoveryUrl, _serviceName)) .And(x => GivenTheServicesAreRegisteredWithConsul(serviceEntryOne, serviceEntryTwo)) @@ -165,29 +129,10 @@ public void should_not_return_services_with_empty_address() [Fact] public void should_not_return_services_with_invalid_port() { - var serviceEntryOne = new ServiceEntry - { - Service = new AgentService - { - Service = _serviceName, - Address = "localhost", - Port = -1, - ID = Guid.NewGuid().ToString(), - Tags = Array.Empty(), - }, - }; - - var serviceEntryTwo = new ServiceEntry - { - Service = new AgentService - { - Service = _serviceName, - Address = "localhost", - Port = 0, - ID = Guid.NewGuid().ToString(), - Tags = Array.Empty(), - }, - }; + var serviceEntryOne = GivenService(port: -1) + .ToServiceEntry(); + var serviceEntryTwo = GivenService(port: 0) + .ToServiceEntry(); this.Given(x => GivenThereIsAFakeConsulServiceDiscoveryProvider(_fakeConsulServiceDiscoveryUrl, _serviceName)) .And(x => GivenTheServicesAreRegisteredWithConsul(serviceEntryOne, serviceEntryTwo)) @@ -197,6 +142,16 @@ public void should_not_return_services_with_invalid_port() .BDDfy(); } + private AgentService GivenService(string address = null, int? port = null, string id = null, string[] tags = null) + => new() + { + Service = _serviceName, + Address = address ?? "localhost", + Port = port ?? 123, + ID = id ?? Guid.NewGuid().ToString(), + Tags = tags ?? Array.Empty(), + }; + private void ThenTheLoggerHasBeenCalledCorrectlyWithValidationWarning(params ServiceEntry[] serviceEntries) { foreach (var entry in serviceEntries) From 842a94135973c71c7770070936b2930f9ab16bc5 Mon Sep 17 00:00:00 2001 From: raman-m Date: Mon, 2 Oct 2023 16:05:49 +0300 Subject: [PATCH 7/9] Use current naming conventions for unit tests --- test/Ocelot.UnitTests/Consul/ProviderFactoryTests.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/Ocelot.UnitTests/Consul/ProviderFactoryTests.cs b/test/Ocelot.UnitTests/Consul/ProviderFactoryTests.cs index 623d31b3d..f71b4ffe5 100644 --- a/test/Ocelot.UnitTests/Consul/ProviderFactoryTests.cs +++ b/test/Ocelot.UnitTests/Consul/ProviderFactoryTests.cs @@ -25,7 +25,7 @@ public ProviderFactoryTests() } [Fact] - public void ShouldReturnConsulServiceDiscoveryProvider() + public void should_return_consul_service_discovery_provider() { var route = new DownstreamRouteBuilder() .WithServiceName(string.Empty) @@ -38,7 +38,7 @@ public void ShouldReturnConsulServiceDiscoveryProvider() } [Fact] - public void ShouldReturnPollingConsulServiceDiscoveryProvider() + public void should_return_polling_consul_service_discovery_provider() { var provider = DummyPollingConsulServiceFactory(string.Empty); var pollProvider = provider as PollConsul; @@ -46,7 +46,7 @@ public void ShouldReturnPollingConsulServiceDiscoveryProvider() } [Fact] - public void ShouldReturnSameProviderForGivenServiceName() + public void should_return_same_provider_for_given_service_name() { var provider = DummyPollingConsulServiceFactory("test"); var provider2 = DummyPollingConsulServiceFactory("test"); @@ -64,7 +64,7 @@ public void ShouldReturnSameProviderForGivenServiceName() [Theory] [InlineData(new object[] { new[] { "service1", "service2", "service3", "service4" } })] - public void ShouldReturnProviderAccordingToServiceName(string[] serviceNames) + public void should_return_provider_according_to_service_name(string[] serviceNames) { var providersList = serviceNames.Select(DummyPollingConsulServiceFactory).ToList(); From ec84a0c0d124db7ca372858550abf36427b6ef09 Mon Sep 17 00:00:00 2001 From: raman-m Date: Mon, 2 Oct 2023 16:38:32 +0300 Subject: [PATCH 8/9] Fix code smell with methos signature --- src/Ocelot.Provider.Consul/Consul.cs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/Ocelot.Provider.Consul/Consul.cs b/src/Ocelot.Provider.Consul/Consul.cs index d6281c514..a7beffdd5 100644 --- a/src/Ocelot.Provider.Consul/Consul.cs +++ b/src/Ocelot.Provider.Consul/Consul.cs @@ -30,7 +30,7 @@ public async Task> GetAsync() foreach (var serviceEntry in queryResult.Response) { var service = serviceEntry.Service; - if (IsValid(serviceEntry)) + if (IsValid(service)) { var nodes = await _consul.Catalog.Nodes(); if (nodes.Response == null) @@ -65,14 +65,11 @@ private static Service BuildService(ServiceEntry serviceEntry, Node serviceNode) service.Tags ?? Enumerable.Empty()); } - private static bool IsValid(ServiceEntry serviceEntry) - { - var service = serviceEntry.Service; - return !string.IsNullOrEmpty(service.Address) - && !service.Address.Contains($"{Uri.UriSchemeHttp}://") - && !service.Address.Contains($"{Uri.UriSchemeHttps}://") - && service.Port > 0; - } + private static bool IsValid(AgentService service) + => !string.IsNullOrEmpty(service.Address) + && !service.Address.Contains($"{Uri.UriSchemeHttp}://") + && !service.Address.Contains($"{Uri.UriSchemeHttps}://") + && service.Port > 0; private static string GetVersionFromStrings(IEnumerable strings) => strings?.FirstOrDefault(x => x.StartsWith(VersionPrefix, StringComparison.Ordinal)) From eefa04272e6b8f435408f8eef23132194b7706d7 Mon Sep 17 00:00:00 2001 From: raman-m Date: Mon, 2 Oct 2023 16:55:13 +0300 Subject: [PATCH 9/9] Encapsulate warning string creation. Create expected value from a hardcoded string in unit tests --- src/Ocelot.Provider.Consul/Consul.cs | 5 ++--- .../Consul/ConsulServiceDiscoveryProviderTests.cs | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/Ocelot.Provider.Consul/Consul.cs b/src/Ocelot.Provider.Consul/Consul.cs index a7beffdd5..84bc7ceee 100644 --- a/src/Ocelot.Provider.Consul/Consul.cs +++ b/src/Ocelot.Provider.Consul/Consul.cs @@ -19,8 +19,6 @@ public Consul(ConsulRegistryConfiguration config, IOcelotLoggerFactory factory, _logger = factory.CreateLogger(); } - public const string ServiceValidationWarningFormat = "Unable to use service address: '{0}' and port: {1} as it is invalid for the service: '{2}'. Address must contain host only e.g. 'localhost', and port must be greater than 0."; - public async Task> GetAsync() { var queryResult = await _consul.Health.Service(_config.KeyOfServiceInConsul, string.Empty, true); @@ -45,7 +43,8 @@ public async Task> GetAsync() } else { - _logger.LogWarning(string.Format(ServiceValidationWarningFormat, service.Address, service.Port, service.Service)); + _logger.LogWarning( + $"Unable to use service address: '{service.Address}' and port: {service.Port} as it is invalid for the service: '{service.Service}'. Address must contain host only e.g. 'localhost', and port must be greater than 0."); } } diff --git a/test/Ocelot.UnitTests/Consul/ConsulServiceDiscoveryProviderTests.cs b/test/Ocelot.UnitTests/Consul/ConsulServiceDiscoveryProviderTests.cs index cab644f2e..115727676 100644 --- a/test/Ocelot.UnitTests/Consul/ConsulServiceDiscoveryProviderTests.cs +++ b/test/Ocelot.UnitTests/Consul/ConsulServiceDiscoveryProviderTests.cs @@ -157,7 +157,7 @@ private void ThenTheLoggerHasBeenCalledCorrectlyWithValidationWarning(params Ser foreach (var entry in serviceEntries) { var service = entry.Service; - var expected = string.Format(ConsulProvider.ServiceValidationWarningFormat, service.Address, service.Port, service.Service); + var expected = $"Unable to use service address: '{service.Address}' and port: {service.Port} as it is invalid for the service: '{service.Service}'. Address must contain host only e.g. 'localhost', and port must be greater than 0."; _logger.Verify(x => x.LogWarning(expected), Times.Once); } }