diff --git a/src/Ocelot.Provider.Consul/ConsulFileConfigurationRepository.cs b/src/Ocelot.Provider.Consul/ConsulFileConfigurationRepository.cs index bfab032e8..bf2192df4 100644 --- a/src/Ocelot.Provider.Consul/ConsulFileConfigurationRepository.cs +++ b/src/Ocelot.Provider.Consul/ConsulFileConfigurationRepository.cs @@ -37,28 +37,28 @@ public ConsulFileConfigurationRepository( _consul = factory.Get(config); } - public async Task> Get() + public async Task GetAsync() { var config = _cache.Get(_configurationKey, _configurationKey); if (config != null) { - return new OkResponse(config); + return config; } var queryResult = await _consul.KV.Get(_configurationKey); if (queryResult.Response == null) { - return new OkResponse(null); + return null; } var bytes = queryResult.Response.Value; var json = Encoding.UTF8.GetString(bytes); var consulConfig = JsonConvert.DeserializeObject(json); - return new OkResponse(consulConfig); + return consulConfig; } - public async Task Set(FileConfiguration ocelotConfiguration) + public async Task SetAsync(FileConfiguration ocelotConfiguration) { var json = JsonConvert.SerializeObject(ocelotConfiguration, Formatting.Indented); var bytes = Encoding.UTF8.GetBytes(json); @@ -68,13 +68,12 @@ public async Task Set(FileConfiguration ocelotConfiguration) }; var result = await _consul.KV.Put(kvPair); - if (result.Response) + if (!result.Response) { - _cache.AddOrUpdate(_configurationKey, ocelotConfiguration, _configurationKey, TimeSpan.FromSeconds(3)); - return new OkResponse(); + throw new UnableToSetConfigInConsulException( + $"Unable to set {nameof(FileConfiguration)} in {nameof(Consul)}! Response status code from {nameof(Consul)} was {result.StatusCode} being returned in {result.RequestTime.TotalMilliseconds} ms."); } - return new ErrorResponse(new UnableToSetConfigInConsulError( - $"Unable to set {nameof(FileConfiguration)} in {nameof(Consul)}, response status code from {nameof(Consul)} was {result.StatusCode}")); + _cache.AddOrUpdate(_configurationKey, ocelotConfiguration, _configurationKey, TimeSpan.FromSeconds(3)); } } diff --git a/src/Ocelot.Provider.Consul/ConsulMiddlewareConfigurationProvider.cs b/src/Ocelot.Provider.Consul/ConsulMiddlewareConfigurationProvider.cs index 0a89a7103..462497e63 100644 --- a/src/Ocelot.Provider.Consul/ConsulMiddlewareConfigurationProvider.cs +++ b/src/Ocelot.Provider.Consul/ConsulMiddlewareConfigurationProvider.cs @@ -34,48 +34,54 @@ private static async Task SetFileConfigInConsul(IApplicationBuilder builder, IFileConfigurationRepository fileConfigRepo, IOptionsMonitor fileConfig, IInternalConfigurationCreator internalConfigCreator, IInternalConfigurationRepository internalConfigRepo) { - // Get the config from Consul - var fileConfigFromConsul = await fileConfigRepo.Get(); - if (IsError(fileConfigFromConsul)) + try { - ThrowToStopOcelotStarting(fileConfigFromConsul); - } - else if (ConfigNotStoredInConsul(fileConfigFromConsul)) - { - // 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 - var internalConfig = await internalConfigCreator.Create(fileConfigFromConsul.Data); - if (IsError(internalConfig)) + // Get the config from Consul + var fileConfigFromConsul = await fileConfigRepo.GetAsync(); + if (fileConfigFromConsul == null) { - ThrowToStopOcelotStarting(internalConfig); + // there was no config in Consul set the file in config in Consul + await fileConfigRepo.SetAsync(fileConfig.CurrentValue); } else { - // add the internal config to the internal repo - var response = internalConfigRepo.AddOrReplace(internalConfig.Data); - if (IsError(response)) + // Create the internal config from Consul data + var internalConfig = await internalConfigCreator.Create(fileConfigFromConsul); + if (IsError(internalConfig)) { - ThrowToStopOcelotStarting(response); + ThrowToStopOcelotStarting(internalConfig); + } + else + { + // add the internal config to the internal repo + var response = internalConfigRepo.AddOrReplace(internalConfig.Data); + if (IsError(response)) + { + ThrowToStopOcelotStarting(response); + } } - } - if (IsError(internalConfig)) - { - ThrowToStopOcelotStarting(internalConfig); + if (IsError(internalConfig)) + { + ThrowToStopOcelotStarting(internalConfig); + } } } + catch (Exception ex) + { + ThrowToStopOcelotStarting(ex); // performance issue? + } } private static void ThrowToStopOcelotStarting(Response config) - => throw new Exception($"Unable to start Ocelot, errors are:{config.Errors.ToErrorString(true, true)}"); + => throw NewException(string.Join(',', config.Errors.Select(x => x.ToString()))); + + private static void ThrowToStopOcelotStarting(Exception ex) + => throw NewException(ex.GetMessages()); + + private static Exception NewException(string errors) + => new($"Unable to start Ocelot! Errors are:{Environment.NewLine}{errors}"); private static bool IsError(Response response) => response == null || response.IsError; - - private static bool ConfigNotStoredInConsul(Response fileConfigFromConsul) - => fileConfigFromConsul.Data == null; } diff --git a/src/Ocelot.Provider.Consul/UnableToSetConfigInConsulError.cs b/src/Ocelot.Provider.Consul/UnableToSetConfigInConsulError.cs deleted file mode 100644 index 62b08f93e..000000000 --- a/src/Ocelot.Provider.Consul/UnableToSetConfigInConsulError.cs +++ /dev/null @@ -1,12 +0,0 @@ -using Ocelot.Errors; -using HttpStatus = System.Net.HttpStatusCode; - -namespace Ocelot.Provider.Consul; - -public class UnableToSetConfigInConsulError : Error -{ - public UnableToSetConfigInConsulError(string s) - : base(s, OcelotErrorCode.UnknownError, (int)HttpStatus.NotFound) - { - } -} diff --git a/src/Ocelot.Provider.Consul/UnableToSetConfigInConsulException.cs b/src/Ocelot.Provider.Consul/UnableToSetConfigInConsulException.cs new file mode 100644 index 000000000..ec09ce529 --- /dev/null +++ b/src/Ocelot.Provider.Consul/UnableToSetConfigInConsulException.cs @@ -0,0 +1,7 @@ +namespace Ocelot.Provider.Consul; + +public class UnableToSetConfigInConsulException : Exception +{ + public UnableToSetConfigInConsulException(string message) + : base(message) { } +} diff --git a/src/Ocelot/Administration/FileConfigurationController.cs b/src/Ocelot/Administration/FileConfigurationController.cs index 095096574..10d11c662 100644 --- a/src/Ocelot/Administration/FileConfigurationController.cs +++ b/src/Ocelot/Administration/FileConfigurationController.cs @@ -1,9 +1,11 @@ using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Http.HttpResults; using Microsoft.AspNetCore.Mvc; using Ocelot.Configuration; using Ocelot.Configuration.File; using Ocelot.Configuration.Repository; using Ocelot.Configuration.Setter; +using Ocelot.Infrastructure.Extensions; namespace Ocelot.Administration; @@ -22,35 +24,35 @@ public FileConfigurationController(IFileConfigurationRepository repo, IFileConfi } [HttpGet] - public async Task Get() + public async Task Get() { - var response = await _repo.Get(); - - if (response.IsError) + try { - return new BadRequestObjectResult(response.Errors); + var fileConfiguration = await _repo.GetAsync(); + return Ok(fileConfiguration); + } + catch (Exception ex) + { + return BadRequest(ex.GetMessages()); } - - return new OkObjectResult(response.Data); } [HttpPost] - public async Task Post([FromBody] FileConfiguration fileConfiguration) + public async Task Post([FromBody] FileConfiguration fileConfiguration) { try { var response = await _setter.Set(fileConfiguration); - if (response.IsError) { - return new BadRequestObjectResult(response.Errors); + return BadRequest(response.Errors); } - return new OkObjectResult(fileConfiguration); + return Ok(fileConfiguration); } catch (Exception e) { - return new BadRequestObjectResult($"{e.Message}:{Environment.NewLine}{e.StackTrace}"); + return BadRequest($"{e.Message}:{Environment.NewLine}{e.StackTrace}"); } } } diff --git a/src/Ocelot/Configuration/Repository/ConsulFileConfigurationPollerOption.cs b/src/Ocelot/Configuration/Repository/ConsulFileConfigurationPollerOption.cs index 68e2e10cc..9d1d5b828 100644 --- a/src/Ocelot/Configuration/Repository/ConsulFileConfigurationPollerOption.cs +++ b/src/Ocelot/Configuration/Repository/ConsulFileConfigurationPollerOption.cs @@ -12,29 +12,33 @@ public ConsulFileConfigurationPollerOption(IInternalConfigurationRepository inte _fileConfigurationRepository = fileConfigurationRepository; } - public int Delay => GetDelay(); + public int Delay => GetDelay().GetAwaiter().GetResult(); - private int GetDelay() - { - var delay = 1000; - - var fileConfig = _fileConfigurationRepository.Get().GetAwaiter().GetResult(); // sync call, so TODO extend IFileConfigurationPollerOptions interface with 2nd async method - if (fileConfig?.Data?.GlobalConfiguration?.ServiceDiscoveryProvider != null && - !fileConfig.IsError && - fileConfig.Data.GlobalConfiguration.ServiceDiscoveryProvider.PollingInterval > 0) - { - delay = fileConfig.Data.GlobalConfiguration.ServiceDiscoveryProvider.PollingInterval; - } - else + private async Task GetDelay() { - var internalConfig = _internalConfigRepo.Get(); - if (internalConfig?.Data?.ServiceProviderConfiguration != null && - !internalConfig.IsError && - internalConfig.Data.ServiceProviderConfiguration.PollingInterval > 0) + var delay = 1000; + try + { + var fileConfig = await _fileConfigurationRepository.GetAsync(); + var provider = fileConfig?.GlobalConfiguration?.ServiceDiscoveryProvider; + if (provider != null && provider.PollingInterval > 0) + { + delay = provider.PollingInterval; + } + else + { + var internalConfig = _internalConfigRepo.Get(); + var configuration = internalConfig?.Data?.ServiceProviderConfiguration; + if (configuration != null && configuration.PollingInterval > 0) + { + delay = configuration.PollingInterval; + } + } + } + catch { - delay = internalConfig.Data.ServiceProviderConfiguration.PollingInterval; + delay = 0; } - } return delay; } diff --git a/src/Ocelot/Configuration/Repository/DiskFileConfigurationRepository.cs b/src/Ocelot/Configuration/Repository/DiskFileConfigurationRepository.cs index 56933a5f2..1ba578bb4 100644 --- a/src/Ocelot/Configuration/Repository/DiskFileConfigurationRepository.cs +++ b/src/Ocelot/Configuration/Repository/DiskFileConfigurationRepository.cs @@ -40,7 +40,7 @@ private void Initialize(string folder) _environmentFile = new FileInfo(Path.Combine(folder, envFile)); } - public Task> Get() + public Task GetAsync() { string jsonConfiguration; @@ -50,11 +50,10 @@ public Task> Get() } var fileConfiguration = JsonConvert.DeserializeObject(jsonConfiguration); - - return Task.FromResult>(new OkResponse(fileConfiguration)); + return Task.FromResult(fileConfiguration); } - public Task Set(FileConfiguration fileConfiguration) + public Task SetAsync(FileConfiguration fileConfiguration) { var jsonConfiguration = JsonConvert.SerializeObject(fileConfiguration, Formatting.Indented); @@ -76,6 +75,6 @@ public Task Set(FileConfiguration fileConfiguration) } _changeTokenSource.Activate(); - return Task.FromResult(new OkResponse()); + return Task.CompletedTask; } } diff --git a/src/Ocelot/Configuration/Repository/FileConfigurationPoller.cs b/src/Ocelot/Configuration/Repository/FileConfigurationPoller.cs index b91bf6ae5..716ffa61f 100644 --- a/src/Ocelot/Configuration/Repository/FileConfigurationPoller.cs +++ b/src/Ocelot/Configuration/Repository/FileConfigurationPoller.cs @@ -3,10 +3,11 @@ using Ocelot.Configuration.Creator; using Ocelot.Configuration.File; using Ocelot.Logging; +using Ocelot.Infrastructure.Extensions; namespace Ocelot.Configuration.Repository; -public class FileConfigurationPoller : IHostedService, IDisposable +public sealed class FileConfigurationPoller : IHostedService, IDisposable { private readonly IOcelotLogger _logger; private readonly IFileConfigurationRepository _repo; @@ -62,46 +63,40 @@ public Task StopAsync(CancellationToken cancellationToken) private async Task Poll() { - _logger.LogInformation("Started polling"); - - var fileConfig = await _repo.Get(); + _logger.LogInformation($"Started {nameof(Poll)}ing"); + try + { + var fileConfig = await _repo.GetAsync(); + var asJson = ToJson(fileConfig); + if (asJson != _previousAsJson) + { + var config = await _internalConfigCreator.Create(fileConfig); + if (!config.IsError) + { + _internalConfigRepo.AddOrReplace(config.Data); + } - if (fileConfig.IsError) + _previousAsJson = asJson; + } + } + catch (Exception ex) { - _logger.LogWarning(() => $"error geting file config, errors are {string.Join(',', fileConfig.Errors.Select(x => x.Message))}"); - return; + _logger.LogWarning($"Error getting file config! Errors are:{Environment.NewLine}{ex.AllMessages}"); } - - var asJson = ToJson(fileConfig.Data); - - if (!fileConfig.IsError && asJson != _previousAsJson) + finally { - var config = await _internalConfigCreator.Create(fileConfig.Data); - - if (!config.IsError) - { - _internalConfigRepo.AddOrReplace(config.Data); - } - - _previousAsJson = asJson; + _logger.LogInformation($"Finished {nameof(Poll)}ing"); } - - _logger.LogInformation("Finished polling"); } /// - /// We could do object comparison here but performance isnt really a problem. This might be an issue one day!. + /// We could do object comparison here but performance isnt really a problem. This might be an issue one day. /// - /// hash of the config. - private static string ToJson(FileConfiguration config) - { - var currentHash = JsonConvert.SerializeObject(config); - return currentHash; - } + /// A with current hash of the config. + private static string ToJson(FileConfiguration config) => JsonConvert.SerializeObject(config); public void Dispose() { _timer?.Dispose(); - _timer = null; } } diff --git a/src/Ocelot/Configuration/Repository/IFileConfigurationRepository.cs b/src/Ocelot/Configuration/Repository/IFileConfigurationRepository.cs index 608747256..9ad0ff0b9 100644 --- a/src/Ocelot/Configuration/Repository/IFileConfigurationRepository.cs +++ b/src/Ocelot/Configuration/Repository/IFileConfigurationRepository.cs @@ -1,11 +1,19 @@ using Ocelot.Configuration.File; -using Ocelot.Responses; namespace Ocelot.Configuration.Repository; public interface IFileConfigurationRepository -{ - Task> Get(); +{ + /// + /// Gets file configuration, aka ocelot.json content model. + /// + /// A model. + Task GetAsync(); - Task Set(FileConfiguration fileConfiguration); + /// + /// Sets file configuration rewriting ocelot.json content. + /// + /// Current model. + /// A object. + Task SetAsync(FileConfiguration fileConfiguration); } diff --git a/src/Ocelot/Configuration/Setter/FileAndInternalConfigurationSetter.cs b/src/Ocelot/Configuration/Setter/FileAndInternalConfigurationSetter.cs index 834234ec9..e5319c023 100644 --- a/src/Ocelot/Configuration/Setter/FileAndInternalConfigurationSetter.cs +++ b/src/Ocelot/Configuration/Setter/FileAndInternalConfigurationSetter.cs @@ -1,6 +1,7 @@ using Ocelot.Configuration.Creator; using Ocelot.Configuration.File; using Ocelot.Configuration.Repository; +using Ocelot.Errors; using Ocelot.Responses; namespace Ocelot.Configuration.Setter; @@ -23,20 +24,22 @@ public FileAndInternalConfigurationSetter( public async Task Set(FileConfiguration fileConfig) { - var response = await _repo.Set(fileConfig); - - if (response.IsError) + try { - return new ErrorResponse(response.Errors); - } + await _repo.SetAsync(fileConfig); + + var config = await _configCreator.Create(fileConfig); - var config = await _configCreator.Create(fileConfig); + if (!config.IsError) + { + _internalConfigRepo.AddOrReplace(config.Data); + } - if (!config.IsError) + return new ErrorResponse(config.Errors); + } + catch (Exception ex) { - _internalConfigRepo.AddOrReplace(config.Data); + return new ErrorResponse(new OcelotError(ex.Message)); } - - return new ErrorResponse(config.Errors); } } diff --git a/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs b/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs index f78c4059b..253b5c999 100644 --- a/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs +++ b/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs @@ -2,6 +2,7 @@ using Microsoft.Extensions.Configuration; using Microsoft.Extensions.Configuration.Memory; using Newtonsoft.Json; +using Newtonsoft.Json.Linq; using Ocelot.Configuration.File; using Ocelot.Infrastructure; @@ -118,7 +119,12 @@ private static string GetMergedOcelotJson(string folder, IWebHostEnvironment env !fi.FullName.Equals(environmentFileInfo.FullName, StringComparison.OrdinalIgnoreCase)) .ToArray(); - fileConfiguration ??= new FileConfiguration(); + fileConfiguration ??= new FileConfiguration(); + dynamic fcMerged = JObject.FromObject(fileConfiguration); + fcMerged.GlobalConfiguration ??= new JObject(); + fcMerged.Aggregates ??= new JArray(); + fcMerged.Routes ??= new JArray(); + primaryFile ??= Path.Join(folder, PrimaryConfigFile); globalFile ??= Path.Join(folder, GlobalConfigFile); var primaryFileInfo = new FileInfo(primaryFile); @@ -133,19 +139,17 @@ private static string GetMergedOcelotJson(string folder, IWebHostEnvironment env } var lines = File.ReadAllText(file.FullName); - var config = JsonConvert.DeserializeObject(lines); - if (file.Name.Equals(globalFileInfo.Name, StringComparison.OrdinalIgnoreCase) && - file.FullName.Equals(globalFileInfo.FullName, StringComparison.OrdinalIgnoreCase)) - { - fileConfiguration.GlobalConfiguration = config.GlobalConfiguration; - } - - fileConfiguration.Aggregates.AddRange(config.Aggregates); - fileConfiguration.Routes.AddRange(config.Routes); + dynamic config = JToken.Parse(lines); + bool isGlobal = file.Name.Equals(globalFileInfo.Name, StringComparison.OrdinalIgnoreCase) && + file.FullName.Equals(globalFileInfo.FullName, StringComparison.OrdinalIgnoreCase); + MergeConfig(fcMerged, config, isGlobal); } - return JsonConvert.SerializeObject(fileConfiguration, Formatting.Indented); - } + return ((JObject)fcMerged).ToString(); + } + + public static IConfigurationBuilder AddOcelot(this IConfigurationBuilder builder, JObject fileConfiguration) + => SerializeToFile(builder, fileConfiguration); /// /// Adds Ocelot configuration by ready configuration object and writes JSON to the primary configuration file.
@@ -160,9 +164,12 @@ private static string GetMergedOcelotJson(string folder, IWebHostEnvironment env /// An object. public static IConfigurationBuilder AddOcelot(this IConfigurationBuilder builder, FileConfiguration fileConfiguration, string primaryConfigFile = null, bool? optional = null, bool? reloadOnChange = null) // optional injections + => SerializeToFile(builder, fileConfiguration); + + private static IConfigurationBuilder SerializeToFile(IConfigurationBuilder builder, object fileConfiguration, bool? optional = null, bool? reloadOnChange = null) { - var json = JsonConvert.SerializeObject(fileConfiguration, Formatting.Indented); - return AddOcelotJsonFile(builder, json, primaryConfigFile, optional, reloadOnChange); + var json = JsonConvert.SerializeObject(fileConfiguration, Formatting.Indented); + return AddOcelotJsonFile(builder, json, PrimaryConfigFile, optional, reloadOnChange); } /// @@ -218,4 +225,58 @@ private static IConfigurationBuilder AddOcelotJsonFile(IConfigurationBuilder bui public static IConfigurationBuilder AddOcelot(this IConfigurationBuilder builder, string primaryFile = null, bool? optional = null, bool? reloadOnChange = null) // optional injections => builder.AddJsonFile(primaryFile ?? PrimaryConfigFile, optional ?? false, reloadOnChange ?? false); + + private static void MergeConfig(JToken to, JToken from, bool isGlobal) + { + if (isGlobal) + { + MergeConfigSection(to, from, nameof(FileConfiguration.GlobalConfiguration)); + } + + MergeConfigSection(to, from, nameof(FileConfiguration.Aggregates)); + MergeConfigSection(to, from, nameof(FileConfiguration.Routes)); + MergeConfigSection(to, from, nameof(FileConfiguration.DynamicRoutes)); + } + + private static void MergeConfigSection(JToken to, JToken from, string sectionName) + { + var destination = to.GetSection(sectionName); + var source = from.GetSection(sectionName); + if (source == null || destination == null) + { + return; + } + + if (source is JObject) + { + to.SetSection(sectionName, source); + } + else if (source is JArray) + { + (destination as JArray).Merge(source); + } + } + + public static JToken GetSection(this JToken token, string name) + { + var obj = token as JObject; + return obj?.Properties() + .FirstOrDefault(p => p.Name.Equals(name, StringComparison.OrdinalIgnoreCase)) + ?.Value; + } + + public static void SetSection(this JToken to, string name, JToken value) + { + JObject obj = to as JObject; + var prop = obj?.Properties() + .FirstOrDefault(p => p.Name.Equals(name, StringComparison.OrdinalIgnoreCase)); + if (prop != null) + { + prop.Value = value; + } + else + { + obj.Add(name, value); + } + } } diff --git a/src/Ocelot/Errors/OcelotError.cs b/src/Ocelot/Errors/OcelotError.cs new file mode 100644 index 000000000..0d702c073 --- /dev/null +++ b/src/Ocelot/Errors/OcelotError.cs @@ -0,0 +1,14 @@ +using Microsoft.AspNetCore.Http; + +namespace Ocelot.Errors; + +public class OcelotError : Error +{ + public OcelotError() + : base(string.Empty, OcelotErrorCode.UnknownError, StatusCodes.Status500InternalServerError) + { } + + public OcelotError(string message) + : base(message, OcelotErrorCode.UnknownError, StatusCodes.Status500InternalServerError) + { } +} diff --git a/src/Ocelot/Infrastructure/Extensions/ExceptionExtensions.cs b/src/Ocelot/Infrastructure/Extensions/ExceptionExtensions.cs new file mode 100644 index 000000000..817c683ec --- /dev/null +++ b/src/Ocelot/Infrastructure/Extensions/ExceptionExtensions.cs @@ -0,0 +1,26 @@ +namespace Ocelot.Infrastructure.Extensions; + +public static class ExceptionExtensions +{ + /// + /// Gets all exception messages via traversing all inner exceptions placing each message onto new line. + /// + /// Current exception. + /// A string builder to accumulate all messages. + /// A object with all messages inside. + public static StringBuilder AllMessages(this Exception ex, StringBuilder builder = null) + { + builder ??= new StringBuilder(); + return ex.InnerException != null + ? AllMessages(ex.InnerException, builder) + : builder.AppendLine(ex.Message); + } + + /// + /// Gets all exception messages of this and inner exceptions as one string. + /// + /// Current exception. + /// A with all messages inside. + public static string GetMessages(this Exception ex) + => AllMessages(ex).ToString(); +} diff --git a/test/Ocelot.AcceptanceTests/Configuration/ConfigurationMergeTests.cs b/test/Ocelot.AcceptanceTests/Configuration/ConfigurationMergeTests.cs index 9ec81ff86..4a6a116a2 100644 --- a/test/Ocelot.AcceptanceTests/Configuration/ConfigurationMergeTests.cs +++ b/test/Ocelot.AcceptanceTests/Configuration/ConfigurationMergeTests.cs @@ -40,7 +40,8 @@ public void ShouldRunWithGlobalConfigMerged_WithExplicitGlobalConfigFileParamete } [Theory] - [Trait("Bug", "2084")] + [Trait("Bug", "2084")] // https://github.com/ThreeMammals/Ocelot/issues/2084 + [Trait("Release", "23.3.4")] // https://github.com/ThreeMammals/Ocelot/releases/tag/23.3.4 [InlineData(MergeOcelotJson.ToFile, true)] [InlineData(MergeOcelotJson.ToMemory, false)] public void ShouldRunWithGlobalConfigMerged_WithImplicitGlobalConfigFileParameter(MergeOcelotJson where, bool fileExist) @@ -84,8 +85,8 @@ public void ShouldRunWithGlobalConfigMerged_WithImplicitGlobalConfigFileParamete var internalConfig = response.Data.ShouldNotBeNull(); // Assert Arrange() setup - internalConfig.RequestId.ShouldBe(nameof(ShouldRunWithGlobalConfigMerged_WithImplicitGlobalConfigFileParameter)); - internalConfig.ServiceProviderConfiguration.ConfigurationKey.ShouldBe(nameof(ShouldRunWithGlobalConfigMerged_WithImplicitGlobalConfigFileParameter)); + internalConfig.RequestId.ShouldBe(TestName()); + internalConfig.ServiceProviderConfiguration.ConfigurationKey.ShouldBe(TestName()); } private void Arrange([CallerMemberName] string testName = null) diff --git a/test/Ocelot.AcceptanceTests/ConfigurationBuilderExtensionsTests.cs b/test/Ocelot.AcceptanceTests/ConfigurationBuilderExtensionsTests.cs new file mode 100644 index 000000000..02895b335 --- /dev/null +++ b/test/Ocelot.AcceptanceTests/ConfigurationBuilderExtensionsTests.cs @@ -0,0 +1,71 @@ +using Newtonsoft.Json.Linq; +using Ocelot.Configuration.File; +using Ocelot.DependencyInjection; + +namespace Ocelot.AcceptanceTests; + +public class ConfigurationBuilderExtensionsTests : Steps +{ + private JObject _config; + + [Fact] + public void Should_merge_routes_custom_properties() + { + var folder = "MergeConfiguration"; // TODO Convert to dynamic temp test folder instead of static one + this.Given(x => GivenOcelotIsRunningWithMultipleConfigs(folder)) + .Then(x => ThenConfigContentShouldHaveThreeRoutes(folder)) + .And(x => ShouldMergeWithCustomPropertyInXservices()) + .And(x => ShouldMergeWithCustomGlobalProperty()) + .And(x => ShouldMergeWithCustomPropertyInYservices()) + .BDDfy(); + } + + private void GivenOcelotIsRunningWithMultipleConfigs(string folder) + + // => StartOcelot((context, config) => config.AddOcelot(folder, context.HostingEnvironment), "Env"); + => GivenOcelotIsRunning((context, config) => config.AddOcelot(folder, context.HostingEnvironment)); + + private async Task ThenConfigContentShouldHaveThreeRoutes(string folder) + { + const int three = 3; + var mergedConfigFile = Path.Combine(folder, ConfigurationBuilderExtensions.PrimaryConfigFile); + File.Exists(mergedConfigFile).ShouldBeTrue(); + var lines = await File.ReadAllTextAsync(mergedConfigFile); + _config = JObject.Parse(lines).ShouldNotBeNull(); + var routes = _config[nameof(FileConfiguration.Routes)].ShouldNotBeNull(); + routes.Children().Count().ShouldBe(three); + } + + private void ShouldMergeWithCustomPropertyInXservices() + { + var customPropertyX = PropertyShouldExist("CustomStrategyProperty"); + customPropertyX["GET"].ShouldNotBeNull(); + customPropertyX["GET"].Children().Count().ShouldBe(1); + customPropertyX["GET"].Children().FirstOrDefault().ShouldBe("SomeCustomStrategyMethodA"); + customPropertyX["POST"].ShouldNotBeNull(); + customPropertyX["POST"].Children().Count().ShouldBe(1); + customPropertyX["POST"].Children().FirstOrDefault().ShouldBe("SomeCustomStrategyMethodB"); + } + + private void ShouldMergeWithCustomGlobalProperty() + { + var customPropertyGlobal = PropertyShouldExist("SomethingMore"); + customPropertyGlobal.ShouldBe("something"); + } + + private void ShouldMergeWithCustomPropertyInYservices() + { + var customPropertyY = PropertyShouldExist("MyCustomProperty"); + customPropertyY.ShouldBeAssignableTo(typeof(JArray)); + customPropertyY.Count().ShouldBe(1); + customPropertyY.First().ShouldBe("myValue"); + } + + private JToken PropertyShouldExist(string propertyName) + { + var routeWithProperty = _config[nameof(FileConfiguration.Routes)].Children() + .SingleOrDefault(route => route[propertyName] != null) + .ShouldNotBeNull(); + return routeWithProperty[propertyName].ShouldNotBeNull(); + } +} diff --git a/test/Ocelot.AcceptanceTests/MergeConfiguration/ocelot.global.json b/test/Ocelot.AcceptanceTests/MergeConfiguration/ocelot.global.json new file mode 100644 index 000000000..5ed704ced --- /dev/null +++ b/test/Ocelot.AcceptanceTests/MergeConfiguration/ocelot.global.json @@ -0,0 +1,20 @@ +{ + "Routes": [ + { + "DownstreamPathTemplate": "/{global}", + "DownstreamScheme": "http", + "DownstreamHostAndPorts": [ + { + "Host": "localhost", + "Port": 24012 + } + ], + "UpstreamPathTemplate": "/{global}", + "UpstreamHttpMethod": [ "Get" ], + "SomethingMore": "something" + } + ], + "GlobalConfiguration": { + "BaseUrl": "http://api.test.com" + } +} diff --git a/test/Ocelot.AcceptanceTests/MergeConfiguration/ocelot.xservices.json b/test/Ocelot.AcceptanceTests/MergeConfiguration/ocelot.xservices.json new file mode 100644 index 000000000..ab44e1ad1 --- /dev/null +++ b/test/Ocelot.AcceptanceTests/MergeConfiguration/ocelot.xservices.json @@ -0,0 +1,27 @@ +{ + "Routes": [ + { + "DownstreamPathTemplate": "/xservice.svc/serviceMethodA/byNumber/{number}", + "UpstreamPathTemplate": "/gw/serviceMethodA/byNumber/{number}", + "UpstreamHttpMethod": [ + "Get", + "Post" + ], + "DownstreamScheme": "http", + "DownstreamHostAndPorts": [ + { + "Host": "host", + "Port": 80 + } + ], + "CustomStrategyProperty": { + "GET": [ + "SomeCustomStrategyMethodA" + ], + "POST": [ + "SomeCustomStrategyMethodB" + ] + } + } + ] +} diff --git a/test/Ocelot.AcceptanceTests/MergeConfiguration/ocelot.yservices.json b/test/Ocelot.AcceptanceTests/MergeConfiguration/ocelot.yservices.json new file mode 100644 index 000000000..e47b433fe --- /dev/null +++ b/test/Ocelot.AcceptanceTests/MergeConfiguration/ocelot.yservices.json @@ -0,0 +1,17 @@ +{ + "Routes": [ + { + "DownstreamPathTemplate": "/b/{action}", + "DownstreamScheme": "http", + "DownstreamHostAndPorts": [ + { + "Host": "localhost", + "Port": 34234 + } + ], + "UpstreamPathTemplate": "/b/{action}", + "UpstreamHttpMethod": [ "Get" ], + "MyCustomProperty": [ "myValue" ] + } + ] +} diff --git a/test/Ocelot.AcceptanceTests/Ocelot.AcceptanceTests.csproj b/test/Ocelot.AcceptanceTests/Ocelot.AcceptanceTests.csproj index 767528da3..519743244 100644 --- a/test/Ocelot.AcceptanceTests/Ocelot.AcceptanceTests.csproj +++ b/test/Ocelot.AcceptanceTests/Ocelot.AcceptanceTests.csproj @@ -18,6 +18,31 @@ $(NoWarn);CS0618;CS1591 + + + + + + + PreserveNewest + + + PreserveNewest + + + PreserveNewest + + + + + PreserveNewest + + + PreserveNewest + + + PreserveNewest + PreserveNewest diff --git a/test/Ocelot.AcceptanceTests/StartupTests.cs b/test/Ocelot.AcceptanceTests/StartupTests.cs index 3863c1b8a..2ed9ad03a 100644 --- a/test/Ocelot.AcceptanceTests/StartupTests.cs +++ b/test/Ocelot.AcceptanceTests/StartupTests.cs @@ -47,7 +47,7 @@ private void GivenThereIsAServiceRunningOn(int port, string basePath, HttpStatus private class FakeFileConfigurationRepository : IFileConfigurationRepository { - public Task> Get() => throw new NotImplementedException(); - public Task Set(FileConfiguration fileConfiguration) => throw new NotImplementedException(); + public Task GetAsync() => throw new NotImplementedException(); + public Task SetAsync(FileConfiguration fileConfiguration) => throw new NotImplementedException(); } } diff --git a/test/Ocelot.UnitTests/Configuration/DiskFileConfigurationRepositoryTests.cs b/test/Ocelot.UnitTests/Configuration/DiskFileConfigurationRepositoryTests.cs index 8323b9747..3456b346c 100644 --- a/test/Ocelot.UnitTests/Configuration/DiskFileConfigurationRepositoryTests.cs +++ b/test/Ocelot.UnitTests/Configuration/DiskFileConfigurationRepositoryTests.cs @@ -36,7 +36,7 @@ public async Task Should_return_file_configuration() GivenTheConfigurationIs(config); // Act - _result = (await _repo.Get()).Data; + _result = await _repo.GetAsync(); // Assert ThenTheFollowingIsReturned(config); @@ -51,7 +51,7 @@ public async Task Should_return_file_configuration_if_environment_name_is_unavai GivenTheConfigurationIs(config); // Act - _result = (await _repo.Get()).Data; + _result = await _repo.GetAsync(); // Assert ThenTheFollowingIsReturned(config); @@ -125,9 +125,8 @@ private void GivenTheEnvironmentNameIsUnavailable() private async Task WhenISetTheConfiguration(FileConfiguration fileConfiguration) { - await _repo.Set(fileConfiguration); - var response = await _repo.Get(); - _result = response.Data; + await _repo.SetAsync(fileConfiguration); + _result = await _repo.GetAsync(); } private void ThenTheConfigurationIsStoredAs(FileConfiguration expecteds) diff --git a/test/Ocelot.UnitTests/Configuration/FileConfigurationPollerTests.cs b/test/Ocelot.UnitTests/Configuration/FileConfigurationPollerTests.cs index b96a1188c..66e635580 100644 --- a/test/Ocelot.UnitTests/Configuration/FileConfigurationPollerTests.cs +++ b/test/Ocelot.UnitTests/Configuration/FileConfigurationPollerTests.cs @@ -4,7 +4,7 @@ using Ocelot.Configuration.Repository; using Ocelot.Logging; using Ocelot.Responses; -using Ocelot.UnitTests.Responder; +using Ocelot.UnitTests.Responder; namespace Ocelot.UnitTests.Configuration; @@ -27,7 +27,7 @@ public FileConfigurationPollerTests() _repo = new Mock(); _fileConfig = new FileConfiguration(); _config = new Mock(); - _repo.Setup(x => x.Get()).ReturnsAsync(new OkResponse(_fileConfig)); + _repo.Setup(x => x.GetAsync()).ReturnsAsync(_fileConfig); _config.Setup(x => x.Delay).Returns(100); _internalConfig = new Mock(); _internalConfigRepo = new Mock(); @@ -134,16 +134,16 @@ public void Should_dispose_cleanly_without_starting() private void WhenProviderErrors() { _repo - .Setup(x => x.Get()) - .ReturnsAsync(new ErrorResponse(new AnyError())); + .Setup(x => x.GetAsync()) + .ThrowsAsync(new FileNotFoundException(new AnyError().Message)); } private void WhenTheConfigIsChanged(FileConfiguration newConfig, int delay) { _repo - .Setup(x => x.Get()) + .Setup(x => x.GetAsync()) .Callback(() => Thread.Sleep(delay)) - .ReturnsAsync(new OkResponse(newConfig)); + .ReturnsAsync(newConfig); } private void ThenTheSetterIsCalled(FileConfiguration fileConfig, int times) diff --git a/test/Ocelot.UnitTests/Configuration/FileConfigurationSetterTests.cs b/test/Ocelot.UnitTests/Configuration/FileConfigurationSetterTests.cs index 17e3c1b21..cd389e883 100644 --- a/test/Ocelot.UnitTests/Configuration/FileConfigurationSetterTests.cs +++ b/test/Ocelot.UnitTests/Configuration/FileConfigurationSetterTests.cs @@ -48,7 +48,7 @@ public async Task Should_set_configuration() RateLimitOptions = new(), Timeout = 111, }; - GivenTheRepoReturns(new OkResponse()); + GivenTheRepoSetsSuccessfully(); GivenTheCreatorReturns(new OkResponse(config)); // Act @@ -59,17 +59,22 @@ public async Task Should_set_configuration() } [Fact] - public async Task Should_return_error_if_unable_to_set_file_configuration() + public async Task Should_catch_exception_if_unable_to_set_file_configuration() { // Arrange _fileConfiguration = new FileConfiguration(); - GivenTheRepoReturns(new ErrorResponse(It.IsAny())); + var exception = new InvalidOperationException("bla-bla"); + GivenTheRepoThrows(exception); // Act _result = await _configSetter.Set(_fileConfiguration); // Assert _result.ShouldBeOfType(); + var response = _result as ErrorResponse; + response.ShouldNotBeNull(); + var error = response.Errors.First(); + error.ShouldNotBeNull().Message.ShouldBe("bla-bla"); } [Fact] @@ -77,7 +82,7 @@ public async Task Should_return_error_if_unable_to_set_ocelot_configuration() { // Arrange _fileConfiguration = new FileConfiguration(); - GivenTheRepoReturns(new OkResponse()); + GivenTheRepoSetsSuccessfully(); GivenTheCreatorReturns(new ErrorResponse(It.IsAny())); // Act @@ -87,11 +92,16 @@ public async Task Should_return_error_if_unable_to_set_ocelot_configuration() _result.ShouldBeOfType(); } - private void GivenTheRepoReturns(Response response) + private void GivenTheRepoSetsSuccessfully() { - _repo - .Setup(x => x.Set(It.IsAny())) - .ReturnsAsync(response); + _repo.Setup(x => x.SetAsync(It.IsAny())) + .Verifiable(); + } + + private void GivenTheRepoThrows(Exception ex) + { + _repo.Setup(x => x.SetAsync(It.IsAny())) + .ThrowsAsync(ex); } private void GivenTheCreatorReturns(Response configuration) diff --git a/test/Ocelot.UnitTests/Consul/ConsulFileConfigurationRepositoryTests.cs b/test/Ocelot.UnitTests/Consul/ConsulFileConfigurationRepositoryTests.cs index 474c5c1ac..6413a4985 100644 --- a/test/Ocelot.UnitTests/Consul/ConsulFileConfigurationRepositoryTests.cs +++ b/test/Ocelot.UnitTests/Consul/ConsulFileConfigurationRepositoryTests.cs @@ -6,7 +6,6 @@ using Ocelot.Logging; using Ocelot.Provider.Consul; using Ocelot.Provider.Consul.Interfaces; -using Ocelot.Responses; using System.Text; namespace Ocelot.UnitTests.Consul; @@ -21,7 +20,7 @@ public class ConsulFileConfigurationRepositoryTests : UnitTest private readonly Mock _client; private readonly Mock _kvEndpoint; private FileConfiguration _fileConfiguration; - private Response _getResult; + private FileConfiguration _getResult; public ConsulFileConfigurationRepositoryTests() { @@ -32,12 +31,15 @@ public ConsulFileConfigurationRepositoryTests() _factory = new Mock(); _client = new Mock(); _kvEndpoint = new Mock(); + _client .Setup(x => x.KV) .Returns(_kvEndpoint.Object); + _factory .Setup(x => x.Get(It.IsAny())) .Returns(_client.Object); + _options .SetupGet(x => x.Value) .Returns(() => _fileConfiguration); @@ -51,7 +53,7 @@ public async Task Should_set_config() GivenWritingToConsulSucceeds(); // Act - _ = await _repo.Set(config); + await _repo.SetAsync(config); // Assert ThenTheConfigurationIsStoredAs(config); @@ -65,7 +67,7 @@ public async Task Should_get_config() GivenFetchFromConsulSucceeds(); // Act - _getResult = await _repo.Get(); + _getResult = await _repo.GetAsync(); // Assert ThenTheConfigurationIs(config); @@ -79,10 +81,10 @@ public async Task Should_get_null_config() GivenFetchFromConsulReturnsNull(); // Act - _getResult = await _repo.Get(); + _getResult = await _repo.GetAsync(); // Assert - _getResult.Data.ShouldBeNull(); + _getResult.ShouldBeNull(); } [Fact] @@ -93,7 +95,7 @@ public async Task Should_get_config_from_cache() GivenFetchFromCacheSucceeds(); // Act - _getResult = await _repo.Get(); + _getResult = await _repo.GetAsync(); // Assert ThenTheConfigurationIs(config); @@ -108,7 +110,7 @@ public async Task Should_set_config_key() GivenFetchFromConsulSucceeds(); // Act - _getResult = await _repo.Get(); + _getResult = await _repo.GetAsync(); // Assert ThenTheConfigKeyIs("Tom"); @@ -122,7 +124,7 @@ public async Task Should_set_default_config_key() GivenFetchFromConsulSucceeds(); // Act - _getResult = await _repo.Get(); + _getResult = await _repo.GetAsync(); // Assert ThenTheConfigKeyIs("InternalConfiguration"); @@ -142,7 +144,7 @@ private void GivenTheConfigKeyComesFromFileConfig(string key) private void ThenTheConfigurationIs(FileConfiguration config) { var expected = JsonConvert.SerializeObject(config, Formatting.Indented); - var result = JsonConvert.SerializeObject(_getResult.Data, Formatting.Indented); + var result = JsonConvert.SerializeObject(_getResult, Formatting.Indented); result.ShouldBe(expected); } diff --git a/test/Ocelot.UnitTests/Controllers/FileConfigurationControllerTests.cs b/test/Ocelot.UnitTests/Controllers/FileConfigurationControllerTests.cs index fdbbe7aa4..0a79ccbbd 100644 --- a/test/Ocelot.UnitTests/Controllers/FileConfigurationControllerTests.cs +++ b/test/Ocelot.UnitTests/Controllers/FileConfigurationControllerTests.cs @@ -25,28 +25,28 @@ public FileConfigurationControllerTests() public async Task Should_get_file_configuration() { // Arrange - var expected = new OkResponse(new FileConfiguration()); - _repo.Setup(x => x.Get()).ReturnsAsync(expected); + var expected = new FileConfiguration(); + _repo.Setup(x => x.GetAsync()).ReturnsAsync(expected); // Act var result = await _controller.Get(); // Assert - _repo.Verify(x => x.Get(), Times.Once); + _repo.Verify(x => x.GetAsync(), Times.Once); } [Fact] public async Task Should_return_error_when_cannot_get_config() { // Arrange - var expected = new ErrorResponse(It.IsAny()); - _repo.Setup(x => x.Get()).ReturnsAsync(expected); + var exception = new FileNotFoundException(nameof(Should_return_error_when_cannot_get_config)); + _repo.Setup(x => x.GetAsync()).ThrowsAsync(exception); // Act var result = await _controller.Get(); // Assert - _repo.Verify(x => x.Get(), Times.Once); + _repo.Verify(x => x.GetAsync(), Times.Once); result.ShouldBeOfType(); }