From 8d9f9e4aa9c8d3f8a39197773cf0a2cefcc689ee Mon Sep 17 00:00:00 2001 From: jlukawska Date: Wed, 1 Apr 2020 20:51:16 +0200 Subject: [PATCH 01/18] #651 merge custom properties --- .../ConfigurationBuilderExtensions.cs | 58 ++++++++++++++----- 1 file changed, 43 insertions(+), 15 deletions(-) diff --git a/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs b/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs index 9101f5b52..99b94afe2 100644 --- a/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs +++ b/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs @@ -43,7 +43,7 @@ public static IConfigurationBuilder AddOcelotBaseUrl(this IConfigurationBuilder /// Configuration builder to extend. /// Web hosting environment object. /// An object. - public static IConfigurationBuilder AddOcelot(this IConfigurationBuilder builder, IWebHostEnvironment env) + public static IConfigurationBuilder AddOcelot(this IConfigurationBuilder builder, IWebHostEnvironment env) => builder.AddOcelot(".", env); /// @@ -53,7 +53,7 @@ public static IConfigurationBuilder AddOcelot(this IConfigurationBuilder builder /// Folder to read files from. /// Web hosting environment object. /// An object. - public static IConfigurationBuilder AddOcelot(this IConfigurationBuilder builder, string folder, IWebHostEnvironment env) + public static IConfigurationBuilder AddOcelot(this IConfigurationBuilder builder, string folder, IWebHostEnvironment env) => builder.AddOcelot(folder, env, MergeOcelotJson.ToFile); /// @@ -94,7 +94,7 @@ public static IConfigurationBuilder AddOcelot(this IConfigurationBuilder builder primaryConfigFile ??= Path.Join(folder, PrimaryConfigFile); // if not specified, merge & write back to the same folder return ApplyMergeOcelotJsonOption(builder, mergeTo, json, primaryConfigFile, optional, reloadOnChange); } - + private static IConfigurationBuilder ApplyMergeOcelotJsonOption(IConfigurationBuilder builder, MergeOcelotJson mergeTo, string json, string primaryConfigFile, bool? optional, bool? reloadOnChange) { @@ -116,7 +116,12 @@ private static string GetMergedOcelotJson(string folder, IWebHostEnvironment env !fi.Name.Equals(environmentFileInfo.Name, StringComparison.OrdinalIgnoreCase) && !fi.FullName.Equals(environmentFileInfo.FullName, StringComparison.OrdinalIgnoreCase)) .ToArray(); - + + dynamic fileConfiguration = new ExpandoObject(); + fileConfiguration.GlobalConfiguration = new ExpandoObject(); + fileConfiguration.Aggregates = new List(); + fileConfiguration.ReRoutes = new List(); + fileConfiguration ??= new FileConfiguration(); primaryFile ??= Path.Join(folder, PrimaryConfigFile); globalFile ??= Path.Join(folder, GlobalConfigFile); @@ -131,28 +136,30 @@ private static string GetMergedOcelotJson(string folder, IWebHostEnvironment env continue; } + //var lines = File.ReadAllText(file.FullName); + //var config = JsonConvert.DeserializeObject(lines); 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)) + dynamic config = JsonConvert.DeserializeObject(lines); + + if (file.Name.Equals(GlobalConfigFile, StringComparison.OrdinalIgnoreCase)) { - fileConfiguration.GlobalConfiguration = config.GlobalConfiguration; + TryAddSection(fileConfiguration, config, nameof(FileConfiguration.GlobalConfiguration)); } - fileConfiguration.Aggregates.AddRange(config.Aggregates); - fileConfiguration.Routes.AddRange(config.Routes); + TryAddSection(fileConfiguration, config, nameof(FileConfiguration.Aggregates)); + TryAddSection(fileConfiguration, config, nameof(FileConfiguration.ReRoutes)); } return JsonConvert.SerializeObject(fileConfiguration, Formatting.Indented); - } - + } + /// /// Adds Ocelot configuration by ready configuration object and writes JSON to the primary configuration file.
/// Finally, adds JSON file as configuration provider. /// /// Use optional arguments for injections and overridings. /// Configuration builder to extend. - /// File configuration to add as JSON provider. + /// File configuration to add as JSON provider. /// Primary config file. /// The 2nd argument of the AddJsonFile. /// The 3rd argument of the AddJsonFile. @@ -163,7 +170,7 @@ public static IConfigurationBuilder AddOcelot(this IConfigurationBuilder builder var json = JsonConvert.SerializeObject(fileConfiguration, Formatting.Indented); return AddOcelotJsonFile(builder, json, primaryConfigFile, optional, reloadOnChange); } - + /// /// Adds Ocelot configuration by ready configuration object, environment and merge option, reading the required files from the current default folder. /// @@ -183,7 +190,7 @@ public static IConfigurationBuilder AddOcelot(this IConfigurationBuilder builder var json = GetMergedOcelotJson(".", env, fileConfiguration, primaryConfigFile, globalConfigFile, environmentConfigFile); return ApplyMergeOcelotJsonOption(builder, mergeTo, json, primaryConfigFile, optional, reloadOnChange); } - + /// /// Adds Ocelot primary configuration file (aka ocelot.json).
/// Writes JSON to the file.
@@ -203,5 +210,26 @@ private static IConfigurationBuilder AddOcelotJsonFile(IConfigurationBuilder bui File.WriteAllText(primary, json); return builder?.AddJsonFile(primary, optional ?? false, reloadOnChange ?? false); } + + private static void TryAddSection(ExpandoObject mergedConfig, ExpandoObject config, string sectionName) + { + var configAsDict = config as IDictionary; + var mergedConfigAsDict = mergedConfig as IDictionary; + if (configAsDict.ContainsKey(sectionName) && mergedConfigAsDict.ContainsKey(sectionName)) + { + var mergedSectionAsExpando = mergedConfigAsDict[sectionName] as ExpandoObject; + if (mergedSectionAsExpando != null) + { + mergedConfigAsDict[sectionName] = configAsDict[sectionName]; + } + else + { + var mergedSectionAsList = mergedConfigAsDict[sectionName] as List; + var sectionAsList = configAsDict[sectionName] as List; + + mergedSectionAsList.AddRange(sectionAsList); + } + } + } } } From d3b8d8f4fad26c8c1ef785e5fd0e8248f3bd2bd4 Mon Sep 17 00:00:00 2001 From: jlukawska Date: Sat, 4 Apr 2020 22:01:37 +0200 Subject: [PATCH 02/18] Jobject + acceptance test --- .../ConfigurationBuilderExtensions.cs | 35 ++++--- .../ConfigurationMergeTests.cs | 94 +++++++++++++++++++ .../MergeConfiguration/ocelot.global.json | 20 ++++ .../MergeConfiguration/ocelot.xservices.json | 27 ++++++ .../MergeConfiguration/ocelot.yservices.json | 17 ++++ .../Ocelot.AcceptanceTests.csproj | 25 +++++ 6 files changed, 199 insertions(+), 19 deletions(-) create mode 100644 test/Ocelot.AcceptanceTests/ConfigurationMergeTests.cs create mode 100644 test/Ocelot.AcceptanceTests/MergeConfiguration/ocelot.global.json create mode 100644 test/Ocelot.AcceptanceTests/MergeConfiguration/ocelot.xservices.json create mode 100644 test/Ocelot.AcceptanceTests/MergeConfiguration/ocelot.yservices.json diff --git a/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs b/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs index 99b94afe2..cd6722434 100644 --- a/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs +++ b/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs @@ -117,10 +117,10 @@ private static string GetMergedOcelotJson(string folder, IWebHostEnvironment env !fi.FullName.Equals(environmentFileInfo.FullName, StringComparison.OrdinalIgnoreCase)) .ToArray(); - dynamic fileConfiguration = new ExpandoObject(); - fileConfiguration.GlobalConfiguration = new ExpandoObject(); - fileConfiguration.Aggregates = new List(); - fileConfiguration.ReRoutes = new List(); + dynamic fileConfiguration = new JObject(); + fileConfiguration.GlobalConfiguration = new JObject(); + fileConfiguration.Aggregates = new JArray(); + fileConfiguration.ReRoutes = new JArray(); fileConfiguration ??= new FileConfiguration(); primaryFile ??= Path.Join(folder, PrimaryConfigFile); @@ -139,7 +139,7 @@ private static string GetMergedOcelotJson(string folder, IWebHostEnvironment env //var lines = File.ReadAllText(file.FullName); //var config = JsonConvert.DeserializeObject(lines); var lines = File.ReadAllText(file.FullName); - dynamic config = JsonConvert.DeserializeObject(lines); + dynamic config = JToken.Parse(lines); if (file.Name.Equals(GlobalConfigFile, StringComparison.OrdinalIgnoreCase)) { @@ -211,25 +211,22 @@ private static IConfigurationBuilder AddOcelotJsonFile(IConfigurationBuilder bui return builder?.AddJsonFile(primary, optional ?? false, reloadOnChange ?? false); } - private static void TryAddSection(ExpandoObject mergedConfig, ExpandoObject config, string sectionName) + private static void TryAddSection(JToken mergedConfig, JToken config, string sectionName) { - var configAsDict = config as IDictionary; - var mergedConfigAsDict = mergedConfig as IDictionary; - if (configAsDict.ContainsKey(sectionName) && mergedConfigAsDict.ContainsKey(sectionName)) + var mergedConfigSection = mergedConfig[sectionName]; + var configSection = config[sectionName]; + + if (configSection != null) { - var mergedSectionAsExpando = mergedConfigAsDict[sectionName] as ExpandoObject; - if (mergedSectionAsExpando != null) + if (configSection is JObject) { - mergedConfigAsDict[sectionName] = configAsDict[sectionName]; + mergedConfig[sectionName] = configSection; } - else + else if (configSection is JArray) { - var mergedSectionAsList = mergedConfigAsDict[sectionName] as List; - var sectionAsList = configAsDict[sectionName] as List; - - mergedSectionAsList.AddRange(sectionAsList); + (mergedConfigSection as JArray).Merge(configSection); } - } - } + } + } } } diff --git a/test/Ocelot.AcceptanceTests/ConfigurationMergeTests.cs b/test/Ocelot.AcceptanceTests/ConfigurationMergeTests.cs new file mode 100644 index 000000000..2a24cb6f7 --- /dev/null +++ b/test/Ocelot.AcceptanceTests/ConfigurationMergeTests.cs @@ -0,0 +1,94 @@ +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.TestHost; +using Microsoft.Extensions.Configuration; +using Newtonsoft.Json; +using Newtonsoft.Json.Linq; +using Ocelot.Configuration.File; +using Ocelot.DependencyInjection; +using Ocelot.Middleware; +using Shouldly; +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Net.Http; +using TestStack.BDDfy; +using Xunit; + +namespace Ocelot.AcceptanceTests +{ + public class ConfigurationMergeTests + { + private Steps _steps; + private IWebHostBuilder _webHostBuilder; + private TestServer _ocelotServer; + private HttpClient _ocelotClient; + + public ConfigurationMergeTests() + { + _steps = new Steps(); + } + + [Fact] + public void should_merge_reroutes_custom_properties() + { + this.Given(x => GivenOcelotIsRunningWithMultipleConfigs()) + .And(x => ThenConfigContentShouldBeMerged()) + .BDDfy(); + } + + private void GivenOcelotIsRunningWithMultipleConfigs() + { + _webHostBuilder = new WebHostBuilder(); + + _webHostBuilder + .UseContentRoot(Directory.GetCurrentDirectory()) + .ConfigureAppConfiguration((hostingContext, config) => + { + config.SetBasePath(hostingContext.HostingEnvironment.ContentRootPath); + var env = hostingContext.HostingEnvironment; + config.AddJsonFile("appsettings.json", optional: true, reloadOnChange: false) + .AddJsonFile($"appsettings.{env.EnvironmentName}.json", optional: true, reloadOnChange: false); + config.AddOcelot("MergeConfiguration", hostingContext.HostingEnvironment); + config.AddEnvironmentVariables(); + }) + .ConfigureServices(s => + { + s.AddOcelot(); + }) + .Configure(app => + { + app.UseOcelot().Wait(); + }); + + _ocelotServer = new TestServer(_webHostBuilder); + + _ocelotClient = _ocelotServer.CreateClient(); + } + + private void ThenConfigContentShouldBeMerged() + { + var mergedConfigFileName = "ocelot.json"; + File.Exists(mergedConfigFileName).ShouldBeTrue(); + var lines = File.ReadAllText(mergedConfigFileName); + var config = JObject.Parse(lines); + + config[nameof(FileConfiguration.ReRoutes)].ShouldNotBeNull(); + config[nameof(FileConfiguration.ReRoutes)].Children().Count().ShouldBe(3); + + var routeWithCustomProperty = config[nameof(FileConfiguration.ReRoutes)].Children().SingleOrDefault(c => c["CustomStrategyProperty"] != null); + routeWithCustomProperty.ShouldNotBeNull(); + var customProperty = routeWithCustomProperty["CustomStrategyProperty"]; + customProperty["GET"].ShouldNotBeNull(); + customProperty["GET"].Children().Count().ShouldBe(1); + customProperty["GET"].Children().FirstOrDefault().ShouldBe("SomeCustomStrategyMethodA"); + customProperty["POST"].ShouldNotBeNull(); + customProperty["POST"].Children().Count().ShouldBe(1); + customProperty["POST"].Children().FirstOrDefault().ShouldBe("SomeCustomStrategyMethodB"); + + var routeWithCustomProperty2 = config[nameof(FileConfiguration.ReRoutes)].Children().SingleOrDefault(c => c["somethingmore"] != null); + routeWithCustomProperty2.ShouldNotBeNull(); + routeWithCustomProperty2["somethingmore"].ShouldBe("something"); + } + } +} diff --git a/test/Ocelot.AcceptanceTests/MergeConfiguration/ocelot.global.json b/test/Ocelot.AcceptanceTests/MergeConfiguration/ocelot.global.json new file mode 100644 index 000000000..cf4c03f2b --- /dev/null +++ b/test/Ocelot.AcceptanceTests/MergeConfiguration/ocelot.global.json @@ -0,0 +1,20 @@ +{ + "ReRoutes": [ + { + "DownstreamPathTemplate": "/{global}", + "DownstreamScheme": "http", + "DownstreamHostAndPorts": [ + { + "Host": "localhost", + "Port": 54001 + } + ], + "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..d98453f78 --- /dev/null +++ b/test/Ocelot.AcceptanceTests/MergeConfiguration/ocelot.xservices.json @@ -0,0 +1,27 @@ +{ + "ReRoutes": [ + { + "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..86ed135af --- /dev/null +++ b/test/Ocelot.AcceptanceTests/MergeConfiguration/ocelot.yservices.json @@ -0,0 +1,17 @@ +{ + "ReRoutes": [ + { + "DownstreamPathTemplate": "/b/{action}", + "DownstreamScheme": "http", + "DownstreamHostAndPorts": [ + { + "Host": "localhost", + "Port": 54001 + } + ], + "UpstreamPathTemplate": "/b/{action}", + "UpstreamHttpMethod": [ "Get" ], + "MyCustomProperty": "bbb" + } + ] +} diff --git a/test/Ocelot.AcceptanceTests/Ocelot.AcceptanceTests.csproj b/test/Ocelot.AcceptanceTests/Ocelot.AcceptanceTests.csproj index 4a2955773..294184de8 100644 --- a/test/Ocelot.AcceptanceTests/Ocelot.AcceptanceTests.csproj +++ b/test/Ocelot.AcceptanceTests/Ocelot.AcceptanceTests.csproj @@ -19,6 +19,31 @@ $(NoWarn);CS0618;CS1591 + + + + + + + PreserveNewest + + + PreserveNewest + + + PreserveNewest + + + + + PreserveNewest + + + PreserveNewest + + + PreserveNewest + PreserveNewest From 0f355af7a6cdc729ac2a63d609a9cf7e51295bc8 Mon Sep 17 00:00:00 2001 From: jlukawska Date: Tue, 7 Apr 2020 21:19:21 +0200 Subject: [PATCH 03/18] #651 acceptance test, small changes --- .../ConfigurationBuilderExtensions.cs | 36 +++++++------ .../ConfigurationMergeTests.cs | 52 +++++++++---------- .../MergeConfiguration/ocelot.global.json | 2 +- .../MergeConfiguration/ocelot.yservices.json | 4 +- 4 files changed, 49 insertions(+), 45 deletions(-) diff --git a/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs b/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs index cd6722434..dc08b9ceb 100644 --- a/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs +++ b/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs @@ -140,14 +140,9 @@ private static string GetMergedOcelotJson(string folder, IWebHostEnvironment env //var config = JsonConvert.DeserializeObject(lines); var lines = File.ReadAllText(file.FullName); dynamic config = JToken.Parse(lines); + var isGlobal = file.Name.Equals(globalConfigFile, StringComparison.OrdinalIgnoreCase); - if (file.Name.Equals(GlobalConfigFile, StringComparison.OrdinalIgnoreCase)) - { - TryAddSection(fileConfiguration, config, nameof(FileConfiguration.GlobalConfiguration)); - } - - TryAddSection(fileConfiguration, config, nameof(FileConfiguration.Aggregates)); - TryAddSection(fileConfiguration, config, nameof(FileConfiguration.ReRoutes)); + MergeConfig(fileConfiguration, config, isGlobal); } return JsonConvert.SerializeObject(fileConfiguration, Formatting.Indented); @@ -211,20 +206,31 @@ private static IConfigurationBuilder AddOcelotJsonFile(IConfigurationBuilder bui return builder?.AddJsonFile(primary, optional ?? false, reloadOnChange ?? false); } - private static void TryAddSection(JToken mergedConfig, JToken config, string sectionName) + private static void MergeConfig(JToken destConfig, JToken srcConfig, bool isGlobal) + { + if (isGlobal) + { + MergeConfigSection(destConfig, srcConfig, nameof(FileConfiguration.GlobalConfiguration)); + } + + MergeConfigSection(destConfig, srcConfig, nameof(FileConfiguration.Aggregates)); + MergeConfigSection(destConfig, srcConfig, nameof(FileConfiguration.ReRoutes)); + } + + private static void MergeConfigSection(JToken destConfig, JToken srcConfig, string sectionName) { - var mergedConfigSection = mergedConfig[sectionName]; - var configSection = config[sectionName]; + var destConfigSection = destConfig[sectionName]; + var srcConfigSection = srcConfig[sectionName]; - if (configSection != null) + if (srcConfigSection != null) { - if (configSection is JObject) + if (srcConfigSection is JObject) { - mergedConfig[sectionName] = configSection; + destConfig[sectionName] = srcConfigSection; } - else if (configSection is JArray) + else if (srcConfigSection is JArray) { - (mergedConfigSection as JArray).Merge(configSection); + (destConfigSection as JArray).Merge(srcConfigSection); } } } diff --git a/test/Ocelot.AcceptanceTests/ConfigurationMergeTests.cs b/test/Ocelot.AcceptanceTests/ConfigurationMergeTests.cs index 2a24cb6f7..67502cedc 100644 --- a/test/Ocelot.AcceptanceTests/ConfigurationMergeTests.cs +++ b/test/Ocelot.AcceptanceTests/ConfigurationMergeTests.cs @@ -1,39 +1,28 @@ using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.TestHost; using Microsoft.Extensions.Configuration; -using Newtonsoft.Json; using Newtonsoft.Json.Linq; using Ocelot.Configuration.File; using Ocelot.DependencyInjection; using Ocelot.Middleware; using Shouldly; -using System; -using System.Collections.Generic; using System.IO; using System.Linq; -using System.Net.Http; using TestStack.BDDfy; using Xunit; namespace Ocelot.AcceptanceTests { public class ConfigurationMergeTests - { - private Steps _steps; + { private IWebHostBuilder _webHostBuilder; private TestServer _ocelotServer; - private HttpClient _ocelotClient; - - public ConfigurationMergeTests() - { - _steps = new Steps(); - } [Fact] public void should_merge_reroutes_custom_properties() { this.Given(x => GivenOcelotIsRunningWithMultipleConfigs()) - .And(x => ThenConfigContentShouldBeMerged()) + .And(x => ThenConfigContentShouldBeMergedWithReRoutesCustomProperties()) .BDDfy(); } @@ -63,10 +52,10 @@ private void GivenOcelotIsRunningWithMultipleConfigs() _ocelotServer = new TestServer(_webHostBuilder); - _ocelotClient = _ocelotServer.CreateClient(); + var ocelotClient = _ocelotServer.CreateClient(); } - private void ThenConfigContentShouldBeMerged() + private void ThenConfigContentShouldBeMergedWithReRoutesCustomProperties() { var mergedConfigFileName = "ocelot.json"; File.Exists(mergedConfigFileName).ShouldBeTrue(); @@ -76,19 +65,28 @@ private void ThenConfigContentShouldBeMerged() config[nameof(FileConfiguration.ReRoutes)].ShouldNotBeNull(); config[nameof(FileConfiguration.ReRoutes)].Children().Count().ShouldBe(3); - var routeWithCustomProperty = config[nameof(FileConfiguration.ReRoutes)].Children().SingleOrDefault(c => c["CustomStrategyProperty"] != null); - routeWithCustomProperty.ShouldNotBeNull(); - var customProperty = routeWithCustomProperty["CustomStrategyProperty"]; - customProperty["GET"].ShouldNotBeNull(); - customProperty["GET"].Children().Count().ShouldBe(1); - customProperty["GET"].Children().FirstOrDefault().ShouldBe("SomeCustomStrategyMethodA"); - customProperty["POST"].ShouldNotBeNull(); - customProperty["POST"].Children().Count().ShouldBe(1); - customProperty["POST"].Children().FirstOrDefault().ShouldBe("SomeCustomStrategyMethodB"); + var routeWithCustomPropertyX = config[nameof(FileConfiguration.ReRoutes)].Children() + .SingleOrDefault(c => c["CustomStrategyProperty"] != null); + routeWithCustomPropertyX.ShouldNotBeNull(); + var customPropertyX = routeWithCustomPropertyX["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"); + + var routeWithCustomPropertyGlobal = config[nameof(FileConfiguration.ReRoutes)].Children() + .SingleOrDefault(c => c["somethingmore"] != null); + routeWithCustomPropertyGlobal.ShouldNotBeNull(); + routeWithCustomPropertyGlobal["somethingmore"].ShouldBe("something"); - var routeWithCustomProperty2 = config[nameof(FileConfiguration.ReRoutes)].Children().SingleOrDefault(c => c["somethingmore"] != null); - routeWithCustomProperty2.ShouldNotBeNull(); - routeWithCustomProperty2["somethingmore"].ShouldBe("something"); + var routeWithCustomPropertyY = config[nameof(FileConfiguration.ReRoutes)].Children() + .SingleOrDefault(c => c["MyCustomProperty"] != null); + routeWithCustomPropertyY.ShouldNotBeNull(); + routeWithCustomPropertyY["MyCustomProperty"].ShouldBeAssignableTo(typeof(JArray)); + routeWithCustomPropertyY["MyCustomProperty"].Count().ShouldBe(1); + routeWithCustomPropertyY["MyCustomProperty"].First().ShouldBe("myValue"); } } } diff --git a/test/Ocelot.AcceptanceTests/MergeConfiguration/ocelot.global.json b/test/Ocelot.AcceptanceTests/MergeConfiguration/ocelot.global.json index cf4c03f2b..bdaeaf3c3 100644 --- a/test/Ocelot.AcceptanceTests/MergeConfiguration/ocelot.global.json +++ b/test/Ocelot.AcceptanceTests/MergeConfiguration/ocelot.global.json @@ -6,7 +6,7 @@ "DownstreamHostAndPorts": [ { "Host": "localhost", - "Port": 54001 + "Port": 24012 } ], "UpstreamPathTemplate": "/{global}", diff --git a/test/Ocelot.AcceptanceTests/MergeConfiguration/ocelot.yservices.json b/test/Ocelot.AcceptanceTests/MergeConfiguration/ocelot.yservices.json index 86ed135af..b2c122cd2 100644 --- a/test/Ocelot.AcceptanceTests/MergeConfiguration/ocelot.yservices.json +++ b/test/Ocelot.AcceptanceTests/MergeConfiguration/ocelot.yservices.json @@ -6,12 +6,12 @@ "DownstreamHostAndPorts": [ { "Host": "localhost", - "Port": 54001 + "Port": 34234 } ], "UpstreamPathTemplate": "/b/{action}", "UpstreamHttpMethod": [ "Get" ], - "MyCustomProperty": "bbb" + "MyCustomProperty": [ "myValue" ] } ] } From c72b8f42ad7ba3dac691a21749c8efe934bb710d Mon Sep 17 00:00:00 2001 From: Raman Maksimchuk Date: Wed, 26 Jul 2023 16:07:23 +0300 Subject: [PATCH 04/18] Update ConfigurationBuilderExtensions.cs potential warnings --- .../DependencyInjection/ConfigurationBuilderExtensions.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs b/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs index dc08b9ceb..f7d293132 100644 --- a/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs +++ b/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs @@ -120,7 +120,7 @@ private static string GetMergedOcelotJson(string folder, IWebHostEnvironment env dynamic fileConfiguration = new JObject(); fileConfiguration.GlobalConfiguration = new JObject(); fileConfiguration.Aggregates = new JArray(); - fileConfiguration.ReRoutes = new JArray(); + fileConfiguration.Routes = new JArray(); fileConfiguration ??= new FileConfiguration(); primaryFile ??= Path.Join(folder, PrimaryConfigFile); @@ -214,7 +214,7 @@ private static void MergeConfig(JToken destConfig, JToken srcConfig, bool isGlob } MergeConfigSection(destConfig, srcConfig, nameof(FileConfiguration.Aggregates)); - MergeConfigSection(destConfig, srcConfig, nameof(FileConfiguration.ReRoutes)); + MergeConfigSection(destConfig, srcConfig, nameof(FileConfiguration.Routes)); } private static void MergeConfigSection(JToken destConfig, JToken srcConfig, string sectionName) From 059842f0d46742a1654b64a8d5cac181f3d9b9e6 Mon Sep 17 00:00:00 2001 From: raman-m Date: Wed, 26 Jul 2023 17:35:20 +0300 Subject: [PATCH 05/18] Fix compilation errors --- .../DependencyInjection/ConfigurationBuilderExtensions.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs b/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs index f7d293132..db381bb66 100644 --- a/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs +++ b/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs @@ -1,8 +1,8 @@ -using Microsoft.AspNetCore.Hosting; -using Microsoft.Extensions.Configuration; -using Microsoft.Extensions.Configuration.Memory; +using Microsoft.AspNetCore.Hosting; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.Configuration.Memory; using Newtonsoft.Json; -using Ocelot.Configuration.File; +using Ocelot.Configuration.File; namespace Ocelot.DependencyInjection { From a43c8dea19da7b3c23272bc80888041b5f5b9d15 Mon Sep 17 00:00:00 2001 From: raman-m Date: Wed, 26 Jul 2023 17:46:06 +0300 Subject: [PATCH 06/18] Fix compilation errors --- .../ConfigurationMergeTests.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test/Ocelot.AcceptanceTests/ConfigurationMergeTests.cs b/test/Ocelot.AcceptanceTests/ConfigurationMergeTests.cs index 67502cedc..e747216d3 100644 --- a/test/Ocelot.AcceptanceTests/ConfigurationMergeTests.cs +++ b/test/Ocelot.AcceptanceTests/ConfigurationMergeTests.cs @@ -19,7 +19,7 @@ public class ConfigurationMergeTests private TestServer _ocelotServer; [Fact] - public void should_merge_reroutes_custom_properties() + public void Should_merge_reroutes_custom_properties() { this.Given(x => GivenOcelotIsRunningWithMultipleConfigs()) .And(x => ThenConfigContentShouldBeMergedWithReRoutesCustomProperties()) @@ -55,17 +55,17 @@ private void GivenOcelotIsRunningWithMultipleConfigs() var ocelotClient = _ocelotServer.CreateClient(); } - private void ThenConfigContentShouldBeMergedWithReRoutesCustomProperties() + private static void ThenConfigContentShouldBeMergedWithReRoutesCustomProperties() { var mergedConfigFileName = "ocelot.json"; File.Exists(mergedConfigFileName).ShouldBeTrue(); var lines = File.ReadAllText(mergedConfigFileName); var config = JObject.Parse(lines); - config[nameof(FileConfiguration.ReRoutes)].ShouldNotBeNull(); - config[nameof(FileConfiguration.ReRoutes)].Children().Count().ShouldBe(3); + config[nameof(FileConfiguration.Routes)].ShouldNotBeNull(); + config[nameof(FileConfiguration.Routes)].Children().Count().ShouldBe(3); - var routeWithCustomPropertyX = config[nameof(FileConfiguration.ReRoutes)].Children() + var routeWithCustomPropertyX = config[nameof(FileConfiguration.Routes)].Children() .SingleOrDefault(c => c["CustomStrategyProperty"] != null); routeWithCustomPropertyX.ShouldNotBeNull(); var customPropertyX = routeWithCustomPropertyX["CustomStrategyProperty"]; @@ -76,12 +76,12 @@ private void ThenConfigContentShouldBeMergedWithReRoutesCustomProperties() customPropertyX["POST"].Children().Count().ShouldBe(1); customPropertyX["POST"].Children().FirstOrDefault().ShouldBe("SomeCustomStrategyMethodB"); - var routeWithCustomPropertyGlobal = config[nameof(FileConfiguration.ReRoutes)].Children() + var routeWithCustomPropertyGlobal = config[nameof(FileConfiguration.Routes)].Children() .SingleOrDefault(c => c["somethingmore"] != null); routeWithCustomPropertyGlobal.ShouldNotBeNull(); routeWithCustomPropertyGlobal["somethingmore"].ShouldBe("something"); - var routeWithCustomPropertyY = config[nameof(FileConfiguration.ReRoutes)].Children() + var routeWithCustomPropertyY = config[nameof(FileConfiguration.Routes)].Children() .SingleOrDefault(c => c["MyCustomProperty"] != null); routeWithCustomPropertyY.ShouldNotBeNull(); routeWithCustomPropertyY["MyCustomProperty"].ShouldBeAssignableTo(typeof(JArray)); From 6d1ac9797bc775d91e83205b4a481220afe85513 Mon Sep 17 00:00:00 2001 From: raman-m Date: Wed, 26 Jul 2023 18:00:04 +0300 Subject: [PATCH 07/18] Fix test: Should_merge_routes_custom_properties --- test/Ocelot.AcceptanceTests/ConfigurationMergeTests.cs | 6 +++--- .../MergeConfiguration/ocelot.global.json | 2 +- .../MergeConfiguration/ocelot.xservices.json | 2 +- .../MergeConfiguration/ocelot.yservices.json | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/test/Ocelot.AcceptanceTests/ConfigurationMergeTests.cs b/test/Ocelot.AcceptanceTests/ConfigurationMergeTests.cs index e747216d3..acd694914 100644 --- a/test/Ocelot.AcceptanceTests/ConfigurationMergeTests.cs +++ b/test/Ocelot.AcceptanceTests/ConfigurationMergeTests.cs @@ -19,10 +19,10 @@ public class ConfigurationMergeTests private TestServer _ocelotServer; [Fact] - public void Should_merge_reroutes_custom_properties() + public void Should_merge_routes_custom_properties() { this.Given(x => GivenOcelotIsRunningWithMultipleConfigs()) - .And(x => ThenConfigContentShouldBeMergedWithReRoutesCustomProperties()) + .And(x => ThenConfigContentShouldBeMergedWithRoutesCustomProperties()) .BDDfy(); } @@ -55,7 +55,7 @@ private void GivenOcelotIsRunningWithMultipleConfigs() var ocelotClient = _ocelotServer.CreateClient(); } - private static void ThenConfigContentShouldBeMergedWithReRoutesCustomProperties() + private static void ThenConfigContentShouldBeMergedWithRoutesCustomProperties() { var mergedConfigFileName = "ocelot.json"; File.Exists(mergedConfigFileName).ShouldBeTrue(); diff --git a/test/Ocelot.AcceptanceTests/MergeConfiguration/ocelot.global.json b/test/Ocelot.AcceptanceTests/MergeConfiguration/ocelot.global.json index bdaeaf3c3..0e635a3be 100644 --- a/test/Ocelot.AcceptanceTests/MergeConfiguration/ocelot.global.json +++ b/test/Ocelot.AcceptanceTests/MergeConfiguration/ocelot.global.json @@ -1,5 +1,5 @@ { - "ReRoutes": [ + "Routes": [ { "DownstreamPathTemplate": "/{global}", "DownstreamScheme": "http", diff --git a/test/Ocelot.AcceptanceTests/MergeConfiguration/ocelot.xservices.json b/test/Ocelot.AcceptanceTests/MergeConfiguration/ocelot.xservices.json index d98453f78..ab44e1ad1 100644 --- a/test/Ocelot.AcceptanceTests/MergeConfiguration/ocelot.xservices.json +++ b/test/Ocelot.AcceptanceTests/MergeConfiguration/ocelot.xservices.json @@ -1,5 +1,5 @@ { - "ReRoutes": [ + "Routes": [ { "DownstreamPathTemplate": "/xservice.svc/serviceMethodA/byNumber/{number}", "UpstreamPathTemplate": "/gw/serviceMethodA/byNumber/{number}", diff --git a/test/Ocelot.AcceptanceTests/MergeConfiguration/ocelot.yservices.json b/test/Ocelot.AcceptanceTests/MergeConfiguration/ocelot.yservices.json index b2c122cd2..e47b433fe 100644 --- a/test/Ocelot.AcceptanceTests/MergeConfiguration/ocelot.yservices.json +++ b/test/Ocelot.AcceptanceTests/MergeConfiguration/ocelot.yservices.json @@ -1,5 +1,5 @@ { - "ReRoutes": [ + "Routes": [ { "DownstreamPathTemplate": "/b/{action}", "DownstreamScheme": "http", From adbfb10b851d57880f6b0c9b53adfd69ebbdd0b4 Mon Sep 17 00:00:00 2001 From: raman-m Date: Thu, 12 Oct 2023 15:15:43 +0300 Subject: [PATCH 08/18] AddOcelot overloaded version --- .../ConfigurationBuilderExtensions.cs | 19 ++++++++++++++----- .../ConfigurationMergeTests.cs | 5 ----- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs b/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs index db381bb66..235334ca9 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; namespace Ocelot.DependencyInjection @@ -108,7 +109,7 @@ private static string GetMergedOcelotJson(string folder, IWebHostEnvironment env { var envName = string.IsNullOrEmpty(env?.EnvironmentName) ? "Development" : env.EnvironmentName; environmentFile ??= Path.Join(folder, string.Format(EnvironmentConfigFile, envName)); - var reg = SubConfigRegex(); + var reg = SubConfigRegex(); var environmentFileInfo = new FileInfo(environmentFile); var files = new DirectoryInfo(folder) .EnumerateFiles() @@ -140,14 +141,18 @@ private static string GetMergedOcelotJson(string folder, IWebHostEnvironment env //var config = JsonConvert.DeserializeObject(lines); var lines = File.ReadAllText(file.FullName); dynamic config = JToken.Parse(lines); - var isGlobal = file.Name.Equals(globalConfigFile, StringComparison.OrdinalIgnoreCase); + var isGlobal = file.Name.Equals(GlobalConfigFile, StringComparison.OrdinalIgnoreCase); MergeConfig(fileConfiguration, config, isGlobal); } - return JsonConvert.SerializeObject(fileConfiguration, Formatting.Indented); + //return JsonConvert.SerializeObject(fileConfiguration, Formatting.Indented); + return AddOcelot(builder, (JObject)fileConfiguration); } + 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.
/// Finally, adds JSON file as configuration provider. @@ -159,8 +164,12 @@ private static string GetMergedOcelotJson(string folder, IWebHostEnvironment env /// The 2nd argument of the AddJsonFile. /// The 3rd argument of the AddJsonFile. /// An object. - public static IConfigurationBuilder AddOcelot(this IConfigurationBuilder builder, FileConfiguration fileConfiguration, - string primaryConfigFile = null, bool? optional = null, bool? reloadOnChange = null) // optional injections + //public static IConfigurationBuilder AddOcelot(this IConfigurationBuilder builder, FileConfiguration fileConfiguration, + // string primaryConfigFile = null, bool? optional = null, bool? reloadOnChange = null) // optional injections + public static IConfigurationBuilder AddOcelot(this IConfigurationBuilder builder, FileConfiguration fileConfiguration) + => SerializeToFile(builder, fileConfiguration); + + private static IConfigurationBuilder SerializeToFile(IConfigurationBuilder builder, object fileConfiguration) { var json = JsonConvert.SerializeObject(fileConfiguration, Formatting.Indented); return AddOcelotJsonFile(builder, json, primaryConfigFile, optional, reloadOnChange); diff --git a/test/Ocelot.AcceptanceTests/ConfigurationMergeTests.cs b/test/Ocelot.AcceptanceTests/ConfigurationMergeTests.cs index acd694914..3d59748be 100644 --- a/test/Ocelot.AcceptanceTests/ConfigurationMergeTests.cs +++ b/test/Ocelot.AcceptanceTests/ConfigurationMergeTests.cs @@ -5,11 +5,6 @@ using Ocelot.Configuration.File; using Ocelot.DependencyInjection; using Ocelot.Middleware; -using Shouldly; -using System.IO; -using System.Linq; -using TestStack.BDDfy; -using Xunit; namespace Ocelot.AcceptanceTests { From 8d84fd69cf0298f833521b2f9b55f15dd64f4401 Mon Sep 17 00:00:00 2001 From: raman-m Date: Mon, 6 Nov 2023 12:11:32 +0300 Subject: [PATCH 09/18] Rename test class --- ...tionMergeTests.cs => ConfigurationBuilderExtensionsTests.cs} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename test/Ocelot.AcceptanceTests/{ConfigurationMergeTests.cs => ConfigurationBuilderExtensionsTests.cs} (98%) diff --git a/test/Ocelot.AcceptanceTests/ConfigurationMergeTests.cs b/test/Ocelot.AcceptanceTests/ConfigurationBuilderExtensionsTests.cs similarity index 98% rename from test/Ocelot.AcceptanceTests/ConfigurationMergeTests.cs rename to test/Ocelot.AcceptanceTests/ConfigurationBuilderExtensionsTests.cs index 3d59748be..685c7138b 100644 --- a/test/Ocelot.AcceptanceTests/ConfigurationMergeTests.cs +++ b/test/Ocelot.AcceptanceTests/ConfigurationBuilderExtensionsTests.cs @@ -8,7 +8,7 @@ namespace Ocelot.AcceptanceTests { - public class ConfigurationMergeTests + public class ConfigurationBuilderExtensionsTests { private IWebHostBuilder _webHostBuilder; private TestServer _ocelotServer; From 2f33574b4defc6f55dacbf5e990df0eeafd9992b Mon Sep 17 00:00:00 2001 From: raman-m Date: Mon, 6 Nov 2023 14:24:13 +0300 Subject: [PATCH 10/18] Refactor Then methods --- .../ConfigurationBuilderExtensionsTests.cs | 60 ++++++++++++------- .../MergeConfiguration/ocelot.global.json | 2 +- 2 files changed, 40 insertions(+), 22 deletions(-) diff --git a/test/Ocelot.AcceptanceTests/ConfigurationBuilderExtensionsTests.cs b/test/Ocelot.AcceptanceTests/ConfigurationBuilderExtensionsTests.cs index 685c7138b..0fe2146aa 100644 --- a/test/Ocelot.AcceptanceTests/ConfigurationBuilderExtensionsTests.cs +++ b/test/Ocelot.AcceptanceTests/ConfigurationBuilderExtensionsTests.cs @@ -17,7 +17,11 @@ public class ConfigurationBuilderExtensionsTests public void Should_merge_routes_custom_properties() { this.Given(x => GivenOcelotIsRunningWithMultipleConfigs()) - .And(x => ThenConfigContentShouldBeMergedWithRoutesCustomProperties()) + .When(x => x.WhenICreateClient()) + .Then(x => ThenConfigContentShouldHaveThreeRoutes()) + .And(x => ShouldMergeWithCustomPropertyInXservices()) + .And(x => ShouldMergeWithCustomGlobalProperty()) + .And(x => ShouldMergeWithCustomPropertyInYservices()) .BDDfy(); } @@ -44,44 +48,58 @@ private void GivenOcelotIsRunningWithMultipleConfigs() { app.UseOcelot().Wait(); }); + } + private void WhenICreateClient() + { _ocelotServer = new TestServer(_webHostBuilder); - - var ocelotClient = _ocelotServer.CreateClient(); + _ = _ocelotServer.CreateClient(); } - private static void ThenConfigContentShouldBeMergedWithRoutesCustomProperties() + private void ThenConfigContentShouldHaveThreeRoutes() { var mergedConfigFileName = "ocelot.json"; File.Exists(mergedConfigFileName).ShouldBeTrue(); var lines = File.ReadAllText(mergedConfigFileName); - var config = JObject.Parse(lines); + _config = JObject.Parse(lines); - config[nameof(FileConfiguration.Routes)].ShouldNotBeNull(); - config[nameof(FileConfiguration.Routes)].Children().Count().ShouldBe(3); + _config[nameof(FileConfiguration.Routes)].ShouldNotBeNull() + .Children().Count().ShouldBe(3); + } - var routeWithCustomPropertyX = config[nameof(FileConfiguration.Routes)].Children() - .SingleOrDefault(c => c["CustomStrategyProperty"] != null); - routeWithCustomPropertyX.ShouldNotBeNull(); - var customPropertyX = routeWithCustomPropertyX["CustomStrategyProperty"]; + private JObject _config; + + 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"); + } - var routeWithCustomPropertyGlobal = config[nameof(FileConfiguration.Routes)].Children() - .SingleOrDefault(c => c["somethingmore"] != null); - routeWithCustomPropertyGlobal.ShouldNotBeNull(); - routeWithCustomPropertyGlobal["somethingmore"].ShouldBe("something"); + 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"); + } - var routeWithCustomPropertyY = config[nameof(FileConfiguration.Routes)].Children() - .SingleOrDefault(c => c["MyCustomProperty"] != null); - routeWithCustomPropertyY.ShouldNotBeNull(); - routeWithCustomPropertyY["MyCustomProperty"].ShouldBeAssignableTo(typeof(JArray)); - routeWithCustomPropertyY["MyCustomProperty"].Count().ShouldBe(1); - routeWithCustomPropertyY["MyCustomProperty"].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 index 0e635a3be..5ed704ced 100644 --- a/test/Ocelot.AcceptanceTests/MergeConfiguration/ocelot.global.json +++ b/test/Ocelot.AcceptanceTests/MergeConfiguration/ocelot.global.json @@ -11,7 +11,7 @@ ], "UpstreamPathTemplate": "/{global}", "UpstreamHttpMethod": [ "Get" ], - "somethingmore": "something" + "SomethingMore": "something" } ], "GlobalConfiguration": { From 8679fdae023d43bdde89afaa366c78d39b38af5e Mon Sep 17 00:00:00 2001 From: raman-m Date: Mon, 13 Nov 2023 15:52:27 +0300 Subject: [PATCH 11/18] IFileConfigurationRepository : Rework Get method --- .../ConsulFileConfigurationRepository.cs | 8 +-- .../ConsulMiddlewareConfigurationProvider.cs | 63 ++++++++++--------- .../FileConfigurationController.cs | 16 ++--- .../ConsulFileConfigurationPollerOption.cs | 36 ++++++----- .../DiskFileConfigurationRepository.cs | 5 +- .../Repository/FileConfigurationPoller.cs | 53 +++++++--------- .../IFileConfigurationRepository.cs | 19 +++--- .../Extensions/ExceptionExtensions.cs | 18 ++++++ test/Ocelot.AcceptanceTests/StartupTests.cs | 5 +- .../DiskFileConfigurationRepositoryTests.cs | 22 +++---- .../FileConfigurationPollerTests.cs | 14 ++--- .../ConsulFileConfigurationRepositoryTests.cs | 8 +-- .../FileConfigurationControllerTests.cs | 30 ++++----- 13 files changed, 161 insertions(+), 136 deletions(-) create mode 100644 src/Ocelot/Infrastructure/Extensions/ExceptionExtensions.cs diff --git a/src/Ocelot.Provider.Consul/ConsulFileConfigurationRepository.cs b/src/Ocelot.Provider.Consul/ConsulFileConfigurationRepository.cs index c95146f46..aca86fc0f 100644 --- a/src/Ocelot.Provider.Consul/ConsulFileConfigurationRepository.cs +++ b/src/Ocelot.Provider.Consul/ConsulFileConfigurationRepository.cs @@ -37,25 +37,25 @@ 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) diff --git a/src/Ocelot.Provider.Consul/ConsulMiddlewareConfigurationProvider.cs b/src/Ocelot.Provider.Consul/ConsulMiddlewareConfigurationProvider.cs index c25ee8fc8..7d79ebc62 100644 --- a/src/Ocelot.Provider.Consul/ConsulMiddlewareConfigurationProvider.cs +++ b/src/Ocelot.Provider.Consul/ConsulMiddlewareConfigurationProvider.cs @@ -4,6 +4,7 @@ using Ocelot.Configuration.Creator; using Ocelot.Configuration.File; using Ocelot.Configuration.Repository; +using Ocelot.Infrastructure.Extensions; using Ocelot.Middleware; using Ocelot.Responses; @@ -33,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.Set(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: {string.Join(',', config.Errors.Select(x => x.ToString()))}"); + => throw NewException(string.Join(',', config.Errors.Select(x => x.ToString()))); + + private static void ThrowToStopOcelotStarting(Exception ex) + => throw NewException(ex.AllMessages().ToString()); + + 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/Configuration/FileConfigurationController.cs b/src/Ocelot/Configuration/FileConfigurationController.cs index f6ead38f5..f94765742 100644 --- a/src/Ocelot/Configuration/FileConfigurationController.cs +++ b/src/Ocelot/Configuration/FileConfigurationController.cs @@ -3,6 +3,7 @@ using Ocelot.Configuration.File; using Ocelot.Configuration.Repository; using Ocelot.Configuration.Setter; +using Ocelot.Infrastructure.Extensions; namespace Ocelot.Configuration { @@ -22,16 +23,17 @@ 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.AllMessages().ToString()); } - - return new OkObjectResult(response.Data); } [HttpPost] diff --git a/src/Ocelot/Configuration/Repository/ConsulFileConfigurationPollerOption.cs b/src/Ocelot/Configuration/Repository/ConsulFileConfigurationPollerOption.cs index d9b8006dd..c232ea774 100644 --- a/src/Ocelot/Configuration/Repository/ConsulFileConfigurationPollerOption.cs +++ b/src/Ocelot/Configuration/Repository/ConsulFileConfigurationPollerOption.cs @@ -12,28 +12,32 @@ public ConsulFileConfigurationPollerOption(IInternalConfigurationRepository inte _fileConfigurationRepository = fileConfigurationRepository; } - public int Delay => GetDelay(); + public int Delay => GetDelay().GetAwaiter().GetResult(); - private int GetDelay() + private async Task 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 + try { - var internalConfig = _internalConfigRepo.Get(); - if (internalConfig?.Data?.ServiceProviderConfiguration != null && - !internalConfig.IsError && - internalConfig.Data.ServiceProviderConfiguration.PollingInterval > 0) + var fileConfig = await _fileConfigurationRepository.GetAsync(); + var provider = fileConfig?.GlobalConfiguration?.ServiceDiscoveryProvider; + if (provider != null && provider.PollingInterval > 0) { - delay = internalConfig.Data.ServiceProviderConfiguration.PollingInterval; + delay = provider.PollingInterval; } + else + { + var internalConfig = _internalConfigRepo.Get(); + var configuration = internalConfig?.Data?.ServiceProviderConfiguration; + if (configuration != null && configuration.PollingInterval > 0) + { + delay = configuration.PollingInterval; + } + } + } + catch + { + delay = 0; } return delay; diff --git a/src/Ocelot/Configuration/Repository/DiskFileConfigurationRepository.cs b/src/Ocelot/Configuration/Repository/DiskFileConfigurationRepository.cs index 023fbfafc..deeefefe8 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,8 +50,7 @@ public Task> Get() } var fileConfiguration = JsonConvert.DeserializeObject(jsonConfiguration); - - return Task.FromResult>(new OkResponse(fileConfiguration)); + return Task.FromResult(fileConfiguration); } public Task Set(FileConfiguration fileConfiguration) diff --git a/src/Ocelot/Configuration/Repository/FileConfigurationPoller.cs b/src/Ocelot/Configuration/Repository/FileConfigurationPoller.cs index 969a34161..f24a71622 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,47 +63,41 @@ public Task StopAsync(CancellationToken cancellationToken) private async Task Poll() { - _logger.LogInformation("Started polling"); - - var fileConfig = await _repo.Get(); + _logger.LogInformation($"Started {nameof(Poll)}"); + 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)}"); } - - _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 63f89791a..95e524b30 100644 --- a/src/Ocelot/Configuration/Repository/IFileConfigurationRepository.cs +++ b/src/Ocelot/Configuration/Repository/IFileConfigurationRepository.cs @@ -1,12 +1,15 @@ using Ocelot.Configuration.File; using Ocelot.Responses; -namespace Ocelot.Configuration.Repository -{ - public interface IFileConfigurationRepository - { - Task> Get(); +namespace Ocelot.Configuration.Repository; - Task Set(FileConfiguration fileConfiguration); - } -} +public interface IFileConfigurationRepository +{ + /// + /// Gets file configuration, aka ocelot.json content model. + /// + /// A model. + Task GetAsync(); + + Task Set(FileConfiguration fileConfiguration); +} diff --git a/src/Ocelot/Infrastructure/Extensions/ExceptionExtensions.cs b/src/Ocelot/Infrastructure/Extensions/ExceptionExtensions.cs new file mode 100644 index 000000000..b07198d5d --- /dev/null +++ b/src/Ocelot/Infrastructure/Extensions/ExceptionExtensions.cs @@ -0,0 +1,18 @@ +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); + } +} diff --git a/test/Ocelot.AcceptanceTests/StartupTests.cs b/test/Ocelot.AcceptanceTests/StartupTests.cs index 04672354e..b3e32b753 100644 --- a/test/Ocelot.AcceptanceTests/StartupTests.cs +++ b/test/Ocelot.AcceptanceTests/StartupTests.cs @@ -81,10 +81,7 @@ public void Dispose() private class FakeFileConfigurationRepository : IFileConfigurationRepository { - public Task> Get() - { - throw new NotImplementedException(); - } + public Task GetAsync() => throw new NotImplementedException(); public Task Set(FileConfiguration fileConfiguration) { diff --git a/test/Ocelot.UnitTests/Configuration/DiskFileConfigurationRepositoryTests.cs b/test/Ocelot.UnitTests/Configuration/DiskFileConfigurationRepositoryTests.cs index 4ca5a70b0..ecee72d91 100644 --- a/test/Ocelot.UnitTests/Configuration/DiskFileConfigurationRepositoryTests.cs +++ b/test/Ocelot.UnitTests/Configuration/DiskFileConfigurationRepositoryTests.cs @@ -20,8 +20,8 @@ public DiskFileConfigurationRepositoryTests() _hostingEnvironment = new Mock(); _changeTokenSource = new Mock(MockBehavior.Strict); _changeTokenSource.Setup(m => m.Activate()); - } - + } + private void Arrange([CallerMemberName] string testName = null) { _hostingEnvironment.Setup(he => he.EnvironmentName).Returns(testName); @@ -30,11 +30,11 @@ private void Arrange([CallerMemberName] string testName = null) [Fact] public async Task Should_return_file_configuration() - { + { Arrange(); var config = FakeFileConfigurationForGet(); GivenTheConfigurationIs(config); - + // Act await WhenIGetTheRoutes(); @@ -49,7 +49,7 @@ public async Task Should_return_file_configuration_if_environment_name_is_unavai var config = FakeFileConfigurationForGet(); GivenTheEnvironmentNameIsUnavailable(); GivenTheConfigurationIs(config); - + // Act await WhenIGetTheRoutes(); @@ -62,7 +62,7 @@ public async Task Should_set_file_configuration() { Arrange(); var config = FakeFileConfigurationForSet(); - + // Act await WhenISetTheConfiguration(config); @@ -81,7 +81,7 @@ public async Task Should_set_file_configuration_if_environment_name_is_unavailab // Act await WhenISetTheConfiguration(config); - + // Assert ThenTheConfigurationIsStoredAs(config); ThenTheConfigurationJsonIsIndented(config); @@ -97,7 +97,7 @@ public async Task Should_set_environment_file_configuration_and_ocelot_file_conf // Act await WhenISetTheConfiguration(config); - + // Assert ThenTheConfigurationIsStoredAs(config); ThenTheConfigurationJsonIsIndented(config); @@ -105,7 +105,7 @@ public async Task Should_set_environment_file_configuration_and_ocelot_file_conf } private FileInfo GivenTheUserAddedOcelotJson() - { + { var primaryFile = Path.Combine(TestID, ConfigurationBuilderExtensions.PrimaryConfigFile); var ocelotJson = new FileInfo(primaryFile); if (ocelotJson.Exists) @@ -227,8 +227,8 @@ private static FileConfiguration FakeFileConfigurationForGet() { var route = GivenRoute("localhost", "/test/test/{test}"); return GivenConfiguration(route); - } - + } + private static FileRoute GivenRoute(string host, string downstream) => new() { DownstreamHostAndPorts = new() { new(host, 80) }, diff --git a/test/Ocelot.UnitTests/Configuration/FileConfigurationPollerTests.cs b/test/Ocelot.UnitTests/Configuration/FileConfigurationPollerTests.cs index 366f40ae8..b4172a81b 100644 --- a/test/Ocelot.UnitTests/Configuration/FileConfigurationPollerTests.cs +++ b/test/Ocelot.UnitTests/Configuration/FileConfigurationPollerTests.cs @@ -28,7 +28,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(); @@ -118,7 +118,7 @@ public void should_do_nothing_if_call_to_provider_fails() }; this.Given(x => GivenPollerHasStarted()) - .Given(x => WhenProviderErrors()) + .And(x => WhenProviderErrors()) .Then(x => ThenTheSetterIsCalled(newConfig, 0)) .BDDfy(); } @@ -137,17 +137,17 @@ private void GivenPollerHasStarted() private void WhenProviderErrors() { - _repo - .Setup(x => x.Get()) - .ReturnsAsync(new ErrorResponse(new AnyError())); + _repo + .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 WhenPollerIsDisposed() diff --git a/test/Ocelot.UnitTests/Consul/ConsulFileConfigurationRepositoryTests.cs b/test/Ocelot.UnitTests/Consul/ConsulFileConfigurationRepositoryTests.cs index 0f8eaf192..d9fcfe9f5 100644 --- a/test/Ocelot.UnitTests/Consul/ConsulFileConfigurationRepositoryTests.cs +++ b/test/Ocelot.UnitTests/Consul/ConsulFileConfigurationRepositoryTests.cs @@ -22,7 +22,7 @@ public class ConsulFileConfigurationRepositoryTests : UnitTest private readonly Mock _kvEndpoint; private FileConfiguration _fileConfiguration; private Response _setResult; - private Response _getResult; + private FileConfiguration _getResult; public ConsulFileConfigurationRepositoryTests() { @@ -134,19 +134,19 @@ private void GivenTheConfigKeyComesFromFileConfig(string key) private void ThenTheConfigurationIsNull() { - _getResult.Data.ShouldBeNull(); + _getResult.ShouldBeNull(); } 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); } private async Task WhenIGetTheConfiguration() { - _getResult = await _repo.Get(); + _getResult = await _repo.GetAsync(); } private void GivenWritingToConsulSucceeds() diff --git a/test/Ocelot.UnitTests/Controllers/FileConfigurationControllerTests.cs b/test/Ocelot.UnitTests/Controllers/FileConfigurationControllerTests.cs index ebfd76490..be5ac6e57 100644 --- a/test/Ocelot.UnitTests/Controllers/FileConfigurationControllerTests.cs +++ b/test/Ocelot.UnitTests/Controllers/FileConfigurationControllerTests.cs @@ -28,7 +28,7 @@ public FileConfigurationControllerTests() [Fact] public void should_get_file_configuration() { - var expected = new OkResponse(new FileConfiguration()); + var expected = new FileConfiguration(); this.Given(x => x.GivenTheGetConfigurationReturns(expected)) .When(x => x.WhenIGetTheFileConfiguration()) @@ -39,9 +39,9 @@ public void should_get_file_configuration() [Fact] public void should_return_error_when_cannot_get_config() { - var expected = new ErrorResponse(It.IsAny()); + var exception = new FileNotFoundException(nameof(should_return_error_when_cannot_get_config)); - this.Given(x => x.GivenTheGetConfigurationReturns(expected)) + this.Given(x => x.GivenTheGetConfigurationThrowsException(exception)) .When(x => x.WhenIGetTheFileConfiguration()) .Then(x => x.TheTheGetFileConfigurationIsCalledCorrectly()) .And(x => x.ThenTheResponseIs()) @@ -75,15 +75,13 @@ public void should_return_error_when_cannot_set_config() private void GivenTheConfigSetterReturns(Response response) { - _setter - .Setup(x => x.Set(It.IsAny())) + _setter.Setup(x => x.Set(It.IsAny())) .ReturnsAsync(response); } private void ThenTheConfigrationSetterIsCalledCorrectly() { - _setter - .Verify(x => x.Set(_fileConfiguration), Times.Once); + _setter.Verify(x => x.Set(_fileConfiguration), Times.Once); } private async Task WhenIPostTheFileConfiguration() @@ -101,13 +99,18 @@ private void ThenTheResponseIs() _result.ShouldBeOfType(); } - private void GivenTheGetConfigurationReturns(Response fileConfiguration) + private void GivenTheGetConfigurationReturns(FileConfiguration fileConfiguration) { - _repo - .Setup(x => x.Get()) + _repo.Setup(x => x.GetAsync()) .ReturnsAsync(fileConfiguration); } + private void GivenTheGetConfigurationThrowsException(Exception ex) + { + _repo.Setup(x => x.GetAsync()) + .ThrowsAsync(ex); + } + private async Task WhenIGetTheFileConfiguration() { _result = await _controller.Get(); @@ -115,15 +118,12 @@ private async Task WhenIGetTheFileConfiguration() private void TheTheGetFileConfigurationIsCalledCorrectly() { - _repo - .Verify(x => x.Get(), Times.Once); + _repo.Verify(x => x.GetAsync(), Times.Once); } private class FakeError : Error { - public FakeError() : base(string.Empty, OcelotErrorCode.CannotAddDataError, 404) - { - } + public FakeError() : base(string.Empty, OcelotErrorCode.CannotAddDataError, 404) { } } } } From b1ca7f5cf71c5936fb494196e11e33afd1c652f7 Mon Sep 17 00:00:00 2001 From: raman-m Date: Mon, 13 Nov 2023 18:37:15 +0300 Subject: [PATCH 12/18] Add GetMessages helper --- .../ConsulMiddlewareConfigurationProvider.cs | 2 +- src/Ocelot/Configuration/FileConfigurationController.cs | 2 +- .../Infrastructure/Extensions/ExceptionExtensions.cs | 8 ++++++++ 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/Ocelot.Provider.Consul/ConsulMiddlewareConfigurationProvider.cs b/src/Ocelot.Provider.Consul/ConsulMiddlewareConfigurationProvider.cs index 7d79ebc62..ed5c535ca 100644 --- a/src/Ocelot.Provider.Consul/ConsulMiddlewareConfigurationProvider.cs +++ b/src/Ocelot.Provider.Consul/ConsulMiddlewareConfigurationProvider.cs @@ -77,7 +77,7 @@ private static void ThrowToStopOcelotStarting(Response config) => throw NewException(string.Join(',', config.Errors.Select(x => x.ToString()))); private static void ThrowToStopOcelotStarting(Exception ex) - => throw NewException(ex.AllMessages().ToString()); + => throw NewException(ex.GetMessages()); private static Exception NewException(string errors) => new($"Unable to start Ocelot! Errors are:{Environment.NewLine}{errors}"); diff --git a/src/Ocelot/Configuration/FileConfigurationController.cs b/src/Ocelot/Configuration/FileConfigurationController.cs index f94765742..aa2288e07 100644 --- a/src/Ocelot/Configuration/FileConfigurationController.cs +++ b/src/Ocelot/Configuration/FileConfigurationController.cs @@ -32,7 +32,7 @@ public async Task Get() } catch (Exception ex) { - return BadRequest(ex.AllMessages().ToString()); + return BadRequest(ex.GetMessages()); } } diff --git a/src/Ocelot/Infrastructure/Extensions/ExceptionExtensions.cs b/src/Ocelot/Infrastructure/Extensions/ExceptionExtensions.cs index b07198d5d..817c683ec 100644 --- a/src/Ocelot/Infrastructure/Extensions/ExceptionExtensions.cs +++ b/src/Ocelot/Infrastructure/Extensions/ExceptionExtensions.cs @@ -15,4 +15,12 @@ public static StringBuilder AllMessages(this Exception ex, StringBuilder builder ? 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(); } From 60dcf5808f19fcda956f581004261ed0fbdfd8d8 Mon Sep 17 00:00:00 2001 From: raman-m Date: Mon, 13 Nov 2023 22:38:48 +0300 Subject: [PATCH 13/18] IFileConfigurationRepository : Rework Set method --- .../ConsulFileConfigurationRepository.cs | 12 +++---- .../UnableToSetConfigInConsulError.cs | 12 ------- .../UnableToSetConfigInConsulException.cs | 7 ++++ .../DiskFileConfigurationRepository.cs | 4 +-- .../IFileConfigurationRepository.cs | 7 +++- .../FileAndInternalConfigurationSetter.cs | 24 +++++++------ src/Ocelot/Errors/OcelotError.cs | 14 ++++++++ test/Ocelot.AcceptanceTests/StartupTests.cs | 5 +-- .../FileConfigurationSetterTests.cs | 36 ++++++++++++------- .../ConsulFileConfigurationRepositoryTests.cs | 3 +- 10 files changed, 73 insertions(+), 51 deletions(-) delete mode 100644 src/Ocelot.Provider.Consul/UnableToSetConfigInConsulError.cs create mode 100644 src/Ocelot.Provider.Consul/UnableToSetConfigInConsulException.cs create mode 100644 src/Ocelot/Errors/OcelotError.cs diff --git a/src/Ocelot.Provider.Consul/ConsulFileConfigurationRepository.cs b/src/Ocelot.Provider.Consul/ConsulFileConfigurationRepository.cs index aca86fc0f..f1db94aef 100644 --- a/src/Ocelot.Provider.Consul/ConsulFileConfigurationRepository.cs +++ b/src/Ocelot.Provider.Consul/ConsulFileConfigurationRepository.cs @@ -58,7 +58,7 @@ public async Task GetAsync() return consulConfig; } - public async Task Set(FileConfiguration ocelotConfiguration) + public async Task Set(FileConfiguration ocelotConfiguration) { var json = JsonConvert.SerializeObject(ocelotConfiguration, Formatting.Indented); var bytes = Encoding.UTF8.GetBytes(json); @@ -68,14 +68,12 @@ public async Task Set(FileConfiguration ocelotConfiguration) }; var result = await _consul.KV.Put(kvPair); - if (result.Response) + if (!result.Response) { - _cache.AddAndDelete(_configurationKey, ocelotConfiguration, TimeSpan.FromSeconds(3), _configurationKey); - - 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.AddAndDelete(_configurationKey, ocelotConfiguration, TimeSpan.FromSeconds(3), _configurationKey); } } 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/Configuration/Repository/DiskFileConfigurationRepository.cs b/src/Ocelot/Configuration/Repository/DiskFileConfigurationRepository.cs index deeefefe8..c233e1407 100644 --- a/src/Ocelot/Configuration/Repository/DiskFileConfigurationRepository.cs +++ b/src/Ocelot/Configuration/Repository/DiskFileConfigurationRepository.cs @@ -53,7 +53,7 @@ public Task GetAsync() return Task.FromResult(fileConfiguration); } - public Task Set(FileConfiguration fileConfiguration) + public Task Set(FileConfiguration fileConfiguration) { var jsonConfiguration = JsonConvert.SerializeObject(fileConfiguration, Formatting.Indented); @@ -75,7 +75,7 @@ public Task Set(FileConfiguration fileConfiguration) } _changeTokenSource.Activate(); - return Task.FromResult(new OkResponse()); + return Task.CompletedTask; } } } diff --git a/src/Ocelot/Configuration/Repository/IFileConfigurationRepository.cs b/src/Ocelot/Configuration/Repository/IFileConfigurationRepository.cs index 95e524b30..e9576f1f1 100644 --- a/src/Ocelot/Configuration/Repository/IFileConfigurationRepository.cs +++ b/src/Ocelot/Configuration/Repository/IFileConfigurationRepository.cs @@ -11,5 +11,10 @@ public interface IFileConfigurationRepository /// A model. Task GetAsync(); - Task Set(FileConfiguration fileConfiguration); + /// + /// Sets file configuration rewriting ocelot.json content. + /// + /// Current model. + /// A object. + Task Set(FileConfiguration fileConfiguration); } diff --git a/src/Ocelot/Configuration/Setter/FileAndInternalConfigurationSetter.cs b/src/Ocelot/Configuration/Setter/FileAndInternalConfigurationSetter.cs index 2d92150a7..3a172a7a1 100644 --- a/src/Ocelot/Configuration/Setter/FileAndInternalConfigurationSetter.cs +++ b/src/Ocelot/Configuration/Setter/FileAndInternalConfigurationSetter.cs @@ -1,6 +1,8 @@ using Ocelot.Configuration.Creator; using Ocelot.Configuration.File; using Ocelot.Configuration.Repository; +using Ocelot.Errors; +using Ocelot.Infrastructure.Extensions; using Ocelot.Responses; namespace Ocelot.Configuration.Setter @@ -23,21 +25,23 @@ 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.Set(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/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/test/Ocelot.AcceptanceTests/StartupTests.cs b/test/Ocelot.AcceptanceTests/StartupTests.cs index b3e32b753..ba19fda70 100644 --- a/test/Ocelot.AcceptanceTests/StartupTests.cs +++ b/test/Ocelot.AcceptanceTests/StartupTests.cs @@ -83,10 +83,7 @@ private class FakeFileConfigurationRepository : IFileConfigurationRepository { public Task GetAsync() => throw new NotImplementedException(); - public Task Set(FileConfiguration fileConfiguration) - { - throw new NotImplementedException(); - } + public Task Set(FileConfiguration fileConfiguration) => throw new NotImplementedException(); } } } diff --git a/test/Ocelot.UnitTests/Configuration/FileConfigurationSetterTests.cs b/test/Ocelot.UnitTests/Configuration/FileConfigurationSetterTests.cs index 44833489d..47c1c4f36 100644 --- a/test/Ocelot.UnitTests/Configuration/FileConfigurationSetterTests.cs +++ b/test/Ocelot.UnitTests/Configuration/FileConfigurationSetterTests.cs @@ -28,7 +28,7 @@ public FileConfigurationSetterTests() } [Fact] - public void should_set_configuration() + public void Should_set_configuration() { var fileConfig = new FileConfiguration(); var serviceProviderConfig = new ServiceProviderConfigurationBuilder().Build(); @@ -45,7 +45,7 @@ public void should_set_configuration() HttpVersionPolicy.RequestVersionOrLower); this.Given(x => GivenTheFollowingConfiguration(fileConfig)) - .And(x => GivenTheRepoReturns(new OkResponse())) + .And(x => GivenTheRepoSetsSuccessfully()) .And(x => GivenTheCreatorReturns(new OkResponse(config))) .When(x => WhenISetTheConfiguration()) .Then(x => ThenTheConfigurationRepositoryIsCalledCorrectly()) @@ -53,35 +53,45 @@ public void should_set_configuration() } [Fact] - public void should_return_error_if_unable_to_set_file_configuration() + public void Should_catch_exception_if_unable_to_set_file_configuration() { var fileConfig = new FileConfiguration(); - + var exception = new InvalidOperationException("bla-bla"); this.Given(x => GivenTheFollowingConfiguration(fileConfig)) - .And(x => GivenTheRepoReturns(new ErrorResponse(It.IsAny()))) + .And(x => GivenTheRepoThrows(exception)) .When(x => WhenISetTheConfiguration()) - .And(x => ThenAnErrorResponseIsReturned()) - .BDDfy(); + .And(x => ThenAnErrorResponseIsReturned()) + .BDDfy(); + + var response = _result as ErrorResponse; + response.ShouldNotBeNull(); + var error = response.Errors.First(); + error.ShouldNotBeNull().Message.ShouldBe("bla-bla"); } [Fact] - public void should_return_error_if_unable_to_set_ocelot_configuration() + public void Should_return_error_if_unable_to_set_ocelot_configuration() { var fileConfig = new FileConfiguration(); this.Given(x => GivenTheFollowingConfiguration(fileConfig)) - .And(x => GivenTheRepoReturns(new OkResponse())) + .And(x => GivenTheRepoSetsSuccessfully()) .And(x => GivenTheCreatorReturns(new ErrorResponse(It.IsAny()))) .When(x => WhenISetTheConfiguration()) .And(x => ThenAnErrorResponseIsReturned()) .BDDfy(); } - private void GivenTheRepoReturns(Response response) + private void GivenTheRepoSetsSuccessfully() + { + _repo.Setup(x => x.Set(It.IsAny())) + .Verifiable(); + } + + private void GivenTheRepoThrows(Exception ex) { - _repo - .Setup(x => x.Set(It.IsAny())) - .ReturnsAsync(response); + _repo.Setup(x => x.Set(It.IsAny())) + .ThrowsAsync(ex); } private void ThenAnErrorResponseIsReturned() diff --git a/test/Ocelot.UnitTests/Consul/ConsulFileConfigurationRepositoryTests.cs b/test/Ocelot.UnitTests/Consul/ConsulFileConfigurationRepositoryTests.cs index d9fcfe9f5..0059122a8 100644 --- a/test/Ocelot.UnitTests/Consul/ConsulFileConfigurationRepositoryTests.cs +++ b/test/Ocelot.UnitTests/Consul/ConsulFileConfigurationRepositoryTests.cs @@ -21,7 +21,6 @@ public class ConsulFileConfigurationRepositoryTests : UnitTest private readonly Mock _client; private readonly Mock _kvEndpoint; private FileConfiguration _fileConfiguration; - private Response _setResult; private FileConfiguration _getResult; public ConsulFileConfigurationRepositoryTests() @@ -207,7 +206,7 @@ private void ThenTheConfigurationIsStoredAs(FileConfiguration config) private async Task WhenISetTheConfiguration() { - _setResult = await _repo.Set(_fileConfiguration); + await _repo.Set(_fileConfiguration); } private void GivenIHaveAConfiguration(FileConfiguration config) From 67b7e5b3e022c591ba68171084b52f4955be7af1 Mon Sep 17 00:00:00 2001 From: raman-m Date: Fri, 17 Nov 2023 18:35:17 +0300 Subject: [PATCH 14/18] IFileConfigurationRepository: Rename to SetAsync --- .../ConsulFileConfigurationRepository.cs | 2 +- .../ConsulMiddlewareConfigurationProvider.cs | 2 +- .../Repository/DiskFileConfigurationRepository.cs | 2 +- .../Repository/IFileConfigurationRepository.cs | 3 +-- .../Setter/FileAndInternalConfigurationSetter.cs | 3 +-- test/Ocelot.AcceptanceTests/StartupTests.cs | 3 +-- .../Configuration/DiskFileConfigurationRepositoryTests.cs | 6 ++++++ .../Configuration/FileConfigurationSetterTests.cs | 4 ++-- .../Consul/ConsulFileConfigurationRepositoryTests.cs | 2 +- 9 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/Ocelot.Provider.Consul/ConsulFileConfigurationRepository.cs b/src/Ocelot.Provider.Consul/ConsulFileConfigurationRepository.cs index f1db94aef..48f630dc0 100644 --- a/src/Ocelot.Provider.Consul/ConsulFileConfigurationRepository.cs +++ b/src/Ocelot.Provider.Consul/ConsulFileConfigurationRepository.cs @@ -58,7 +58,7 @@ public async Task GetAsync() 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); diff --git a/src/Ocelot.Provider.Consul/ConsulMiddlewareConfigurationProvider.cs b/src/Ocelot.Provider.Consul/ConsulMiddlewareConfigurationProvider.cs index ed5c535ca..462497e63 100644 --- a/src/Ocelot.Provider.Consul/ConsulMiddlewareConfigurationProvider.cs +++ b/src/Ocelot.Provider.Consul/ConsulMiddlewareConfigurationProvider.cs @@ -41,7 +41,7 @@ private static async Task SetFileConfigInConsul(IApplicationBuilder builder, if (fileConfigFromConsul == null) { // there was no config in Consul set the file in config in Consul - await fileConfigRepo.Set(fileConfig.CurrentValue); + await fileConfigRepo.SetAsync(fileConfig.CurrentValue); } else { diff --git a/src/Ocelot/Configuration/Repository/DiskFileConfigurationRepository.cs b/src/Ocelot/Configuration/Repository/DiskFileConfigurationRepository.cs index c233e1407..864a82731 100644 --- a/src/Ocelot/Configuration/Repository/DiskFileConfigurationRepository.cs +++ b/src/Ocelot/Configuration/Repository/DiskFileConfigurationRepository.cs @@ -53,7 +53,7 @@ public Task GetAsync() return Task.FromResult(fileConfiguration); } - public Task Set(FileConfiguration fileConfiguration) + public Task SetAsync(FileConfiguration fileConfiguration) { var jsonConfiguration = JsonConvert.SerializeObject(fileConfiguration, Formatting.Indented); diff --git a/src/Ocelot/Configuration/Repository/IFileConfigurationRepository.cs b/src/Ocelot/Configuration/Repository/IFileConfigurationRepository.cs index e9576f1f1..9ad0ff0b9 100644 --- a/src/Ocelot/Configuration/Repository/IFileConfigurationRepository.cs +++ b/src/Ocelot/Configuration/Repository/IFileConfigurationRepository.cs @@ -1,5 +1,4 @@ using Ocelot.Configuration.File; -using Ocelot.Responses; namespace Ocelot.Configuration.Repository; @@ -16,5 +15,5 @@ public interface IFileConfigurationRepository /// /// Current model. /// A object. - Task Set(FileConfiguration fileConfiguration); + Task SetAsync(FileConfiguration fileConfiguration); } diff --git a/src/Ocelot/Configuration/Setter/FileAndInternalConfigurationSetter.cs b/src/Ocelot/Configuration/Setter/FileAndInternalConfigurationSetter.cs index 3a172a7a1..0a690a6c3 100644 --- a/src/Ocelot/Configuration/Setter/FileAndInternalConfigurationSetter.cs +++ b/src/Ocelot/Configuration/Setter/FileAndInternalConfigurationSetter.cs @@ -2,7 +2,6 @@ using Ocelot.Configuration.File; using Ocelot.Configuration.Repository; using Ocelot.Errors; -using Ocelot.Infrastructure.Extensions; using Ocelot.Responses; namespace Ocelot.Configuration.Setter @@ -27,7 +26,7 @@ public async Task Set(FileConfiguration fileConfig) { try { - await _repo.Set(fileConfig); + await _repo.SetAsync(fileConfig); var config = await _configCreator.Create(fileConfig); diff --git a/test/Ocelot.AcceptanceTests/StartupTests.cs b/test/Ocelot.AcceptanceTests/StartupTests.cs index ba19fda70..7d63ac640 100644 --- a/test/Ocelot.AcceptanceTests/StartupTests.cs +++ b/test/Ocelot.AcceptanceTests/StartupTests.cs @@ -1,7 +1,6 @@ using Microsoft.AspNetCore.Http; using Ocelot.Configuration.File; using Ocelot.Configuration.Repository; -using Ocelot.Responses; namespace Ocelot.AcceptanceTests { @@ -83,7 +82,7 @@ private class FakeFileConfigurationRepository : IFileConfigurationRepository { public Task GetAsync() => throw new NotImplementedException(); - public Task Set(FileConfiguration fileConfiguration) => 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 ecee72d91..165cb8fc5 100644 --- a/test/Ocelot.UnitTests/Configuration/DiskFileConfigurationRepositoryTests.cs +++ b/test/Ocelot.UnitTests/Configuration/DiskFileConfigurationRepositoryTests.cs @@ -130,6 +130,12 @@ private async Task WhenISetTheConfiguration(FileConfiguration fileConfiguration) _result = response.Data; } + private void WhenISetTheConfiguration() + { + _repo.SetAsync(_fileConfiguration); + _result = _repo.GetAsync().Result; + } + private void ThenTheConfigurationIsStoredAs(FileConfiguration expecteds) { _result.GlobalConfiguration.RequestIdKey.ShouldBe(expecteds.GlobalConfiguration.RequestIdKey); diff --git a/test/Ocelot.UnitTests/Configuration/FileConfigurationSetterTests.cs b/test/Ocelot.UnitTests/Configuration/FileConfigurationSetterTests.cs index 47c1c4f36..baabd4e84 100644 --- a/test/Ocelot.UnitTests/Configuration/FileConfigurationSetterTests.cs +++ b/test/Ocelot.UnitTests/Configuration/FileConfigurationSetterTests.cs @@ -84,13 +84,13 @@ public void Should_return_error_if_unable_to_set_ocelot_configuration() private void GivenTheRepoSetsSuccessfully() { - _repo.Setup(x => x.Set(It.IsAny())) + _repo.Setup(x => x.SetAsync(It.IsAny())) .Verifiable(); } private void GivenTheRepoThrows(Exception ex) { - _repo.Setup(x => x.Set(It.IsAny())) + _repo.Setup(x => x.SetAsync(It.IsAny())) .ThrowsAsync(ex); } diff --git a/test/Ocelot.UnitTests/Consul/ConsulFileConfigurationRepositoryTests.cs b/test/Ocelot.UnitTests/Consul/ConsulFileConfigurationRepositoryTests.cs index 0059122a8..360e6a6ed 100644 --- a/test/Ocelot.UnitTests/Consul/ConsulFileConfigurationRepositoryTests.cs +++ b/test/Ocelot.UnitTests/Consul/ConsulFileConfigurationRepositoryTests.cs @@ -206,7 +206,7 @@ private void ThenTheConfigurationIsStoredAs(FileConfiguration config) private async Task WhenISetTheConfiguration() { - await _repo.Set(_fileConfiguration); + await _repo.SetAsync(_fileConfiguration); } private void GivenIHaveAConfiguration(FileConfiguration config) From 4eedbae47c647d32b8d162bc6a5bada4f449d1d3 Mon Sep 17 00:00:00 2001 From: jlukawska Date: Thu, 16 May 2024 14:01:24 +0200 Subject: [PATCH 15/18] merge fix --- .../ConfigurationBuilderExtensions.cs | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs b/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs index 235334ca9..901a0832d 100644 --- a/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs +++ b/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs @@ -118,10 +118,10 @@ private static string GetMergedOcelotJson(string folder, IWebHostEnvironment env !fi.FullName.Equals(environmentFileInfo.FullName, StringComparison.OrdinalIgnoreCase)) .ToArray(); - dynamic fileConfiguration = new JObject(); - fileConfiguration.GlobalConfiguration = new JObject(); - fileConfiguration.Aggregates = new JArray(); - fileConfiguration.Routes = new JArray(); + dynamic fileConfigurationMerged = fileConfiguration != null ? JObject.FromObject(fileConfiguration) : new JObject(); + fileConfigurationMerged.GlobalConfiguration ??= new JObject(); + fileConfigurationMerged.Aggregates ??= new JArray(); + fileConfigurationMerged.Routes ??= new JArray(); fileConfiguration ??= new FileConfiguration(); primaryFile ??= Path.Join(folder, PrimaryConfigFile); @@ -140,14 +140,14 @@ private static string GetMergedOcelotJson(string folder, IWebHostEnvironment env //var lines = File.ReadAllText(file.FullName); //var config = JsonConvert.DeserializeObject(lines); var lines = File.ReadAllText(file.FullName); - dynamic config = JToken.Parse(lines); - var isGlobal = file.Name.Equals(GlobalConfigFile, StringComparison.OrdinalIgnoreCase); - - MergeConfig(fileConfiguration, config, isGlobal); + dynamic config = JToken.Parse(lines); + var isGlobal = file.Name.Equals(globalFileInfo.Name, StringComparison.OrdinalIgnoreCase) && + file.FullName.Equals(globalFileInfo.FullName, StringComparison.OrdinalIgnoreCase); + + MergeConfig(fileConfigurationMerged, config, isGlobal); } - //return JsonConvert.SerializeObject(fileConfiguration, Formatting.Indented); - return AddOcelot(builder, (JObject)fileConfiguration); + return ((JObject)fileConfigurationMerged).ToString(); } public static IConfigurationBuilder AddOcelot(this IConfigurationBuilder builder, JObject fileConfiguration) @@ -169,10 +169,10 @@ public static IConfigurationBuilder AddOcelot(this IConfigurationBuilder builder public static IConfigurationBuilder AddOcelot(this IConfigurationBuilder builder, FileConfiguration fileConfiguration) => SerializeToFile(builder, fileConfiguration); - private static IConfigurationBuilder SerializeToFile(IConfigurationBuilder builder, object 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); + return AddOcelotJsonFile(builder, json, PrimaryConfigFile, optional, reloadOnChange); } /// From 1b2198c9fa39661a90034d09f55f8d584ee26add Mon Sep 17 00:00:00 2001 From: Raman Maksimchuk Date: Mon, 11 Nov 2024 18:45:32 +0300 Subject: [PATCH 16/18] Fix build after rebasing, with a refactoring --- .../ConfigurationBuilderExtensions.cs | 59 ++++++++-------- .../ConfigurationBuilderExtensionsTests.cs | 70 +++++-------------- .../DiskFileConfigurationRepositoryTests.cs | 17 +---- 3 files changed, 49 insertions(+), 97 deletions(-) diff --git a/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs b/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs index 901a0832d..beeb971bc 100644 --- a/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs +++ b/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs @@ -118,12 +118,12 @@ private static string GetMergedOcelotJson(string folder, IWebHostEnvironment env !fi.FullName.Equals(environmentFileInfo.FullName, StringComparison.OrdinalIgnoreCase)) .ToArray(); - dynamic fileConfigurationMerged = fileConfiguration != null ? JObject.FromObject(fileConfiguration) : new JObject(); - fileConfigurationMerged.GlobalConfiguration ??= new JObject(); - fileConfigurationMerged.Aggregates ??= new JArray(); - fileConfigurationMerged.Routes ??= new JArray(); + fileConfiguration ??= new FileConfiguration(); + dynamic fcMerged = JObject.FromObject(fileConfiguration); + fcMerged.GlobalConfiguration ??= new JObject(); + fcMerged.Aggregates ??= new JArray(); + fcMerged.Routes ??= new JArray(); - fileConfiguration ??= new FileConfiguration(); primaryFile ??= Path.Join(folder, PrimaryConfigFile); globalFile ??= Path.Join(folder, GlobalConfigFile); var primaryFileInfo = new FileInfo(primaryFile); @@ -137,17 +137,14 @@ private static string GetMergedOcelotJson(string folder, IWebHostEnvironment env continue; } - //var lines = File.ReadAllText(file.FullName); - //var config = JsonConvert.DeserializeObject(lines); var lines = File.ReadAllText(file.FullName); dynamic config = JToken.Parse(lines); - var isGlobal = file.Name.Equals(globalFileInfo.Name, StringComparison.OrdinalIgnoreCase) && + bool isGlobal = file.Name.Equals(globalFileInfo.Name, StringComparison.OrdinalIgnoreCase) && file.FullName.Equals(globalFileInfo.FullName, StringComparison.OrdinalIgnoreCase); - - MergeConfig(fileConfigurationMerged, config, isGlobal); + MergeConfig(fcMerged, config, isGlobal); } - return ((JObject)fileConfigurationMerged).ToString(); + return ((JObject)fcMerged).ToString(); } public static IConfigurationBuilder AddOcelot(this IConfigurationBuilder builder, JObject fileConfiguration) @@ -164,9 +161,8 @@ public static IConfigurationBuilder AddOcelot(this IConfigurationBuilder builder /// The 2nd argument of the AddJsonFile. /// The 3rd argument of the AddJsonFile. /// An object. - //public static IConfigurationBuilder AddOcelot(this IConfigurationBuilder builder, FileConfiguration fileConfiguration, - // string primaryConfigFile = null, bool? optional = null, bool? reloadOnChange = null) // optional injections - public static IConfigurationBuilder AddOcelot(this IConfigurationBuilder builder, FileConfiguration fileConfiguration) + 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) @@ -215,33 +211,34 @@ private static IConfigurationBuilder AddOcelotJsonFile(IConfigurationBuilder bui return builder?.AddJsonFile(primary, optional ?? false, reloadOnChange ?? false); } - private static void MergeConfig(JToken destConfig, JToken srcConfig, bool isGlobal) + private static void MergeConfig(JToken to, JToken from, bool isGlobal) { if (isGlobal) { - MergeConfigSection(destConfig, srcConfig, nameof(FileConfiguration.GlobalConfiguration)); + MergeConfigSection(to, from, nameof(FileConfiguration.GlobalConfiguration)); } - MergeConfigSection(destConfig, srcConfig, nameof(FileConfiguration.Aggregates)); - MergeConfigSection(destConfig, srcConfig, nameof(FileConfiguration.Routes)); + MergeConfigSection(to, from, nameof(FileConfiguration.Aggregates)); + MergeConfigSection(to, from, nameof(FileConfiguration.Routes)); } - private static void MergeConfigSection(JToken destConfig, JToken srcConfig, string sectionName) + private static void MergeConfigSection(JToken to, JToken from, string sectionName) { - var destConfigSection = destConfig[sectionName]; - var srcConfigSection = srcConfig[sectionName]; + var destination = to[sectionName]; + var source = from[sectionName]; + if (source == null || destination == null) + { + return; + } - if (srcConfigSection != null) + if (source is JObject) { - if (srcConfigSection is JObject) - { - destConfig[sectionName] = srcConfigSection; - } - else if (srcConfigSection is JArray) - { - (destConfigSection as JArray).Merge(srcConfigSection); - } - } + to[sectionName] = source; + } + else if (source is JArray) + { + (destination as JArray).Merge(source); + } } } } diff --git a/test/Ocelot.AcceptanceTests/ConfigurationBuilderExtensionsTests.cs b/test/Ocelot.AcceptanceTests/ConfigurationBuilderExtensionsTests.cs index 0fe2146aa..53be617d1 100644 --- a/test/Ocelot.AcceptanceTests/ConfigurationBuilderExtensionsTests.cs +++ b/test/Ocelot.AcceptanceTests/ConfigurationBuilderExtensionsTests.cs @@ -1,74 +1,40 @@ -using Microsoft.AspNetCore.Hosting; -using Microsoft.AspNetCore.TestHost; -using Microsoft.Extensions.Configuration; -using Newtonsoft.Json.Linq; +using Newtonsoft.Json.Linq; using Ocelot.Configuration.File; using Ocelot.DependencyInjection; -using Ocelot.Middleware; namespace Ocelot.AcceptanceTests { - public class ConfigurationBuilderExtensionsTests - { - private IWebHostBuilder _webHostBuilder; - private TestServer _ocelotServer; + public class ConfigurationBuilderExtensionsTests : Steps + { + private JObject _config; [Fact] public void Should_merge_routes_custom_properties() { - this.Given(x => GivenOcelotIsRunningWithMultipleConfigs()) - .When(x => x.WhenICreateClient()) - .Then(x => ThenConfigContentShouldHaveThreeRoutes()) + 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() - { - _webHostBuilder = new WebHostBuilder(); - - _webHostBuilder - .UseContentRoot(Directory.GetCurrentDirectory()) - .ConfigureAppConfiguration((hostingContext, config) => - { - config.SetBasePath(hostingContext.HostingEnvironment.ContentRootPath); - var env = hostingContext.HostingEnvironment; - config.AddJsonFile("appsettings.json", optional: true, reloadOnChange: false) - .AddJsonFile($"appsettings.{env.EnvironmentName}.json", optional: true, reloadOnChange: false); - config.AddOcelot("MergeConfiguration", hostingContext.HostingEnvironment); - config.AddEnvironmentVariables(); - }) - .ConfigureServices(s => - { - s.AddOcelot(); - }) - .Configure(app => - { - app.UseOcelot().Wait(); - }); - } - - private void WhenICreateClient() - { - _ocelotServer = new TestServer(_webHostBuilder); - _ = _ocelotServer.CreateClient(); - } + private void GivenOcelotIsRunningWithMultipleConfigs(string folder) => StartOcelot( + (context, config) => config.AddOcelot(folder, context.HostingEnvironment), + "Env"); - private void ThenConfigContentShouldHaveThreeRoutes() + private async Task ThenConfigContentShouldHaveThreeRoutes(string folder) { - var mergedConfigFileName = "ocelot.json"; - File.Exists(mergedConfigFileName).ShouldBeTrue(); - var lines = File.ReadAllText(mergedConfigFileName); - _config = JObject.Parse(lines); - - _config[nameof(FileConfiguration.Routes)].ShouldNotBeNull() - .Children().Count().ShouldBe(3); + 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 JObject _config; - private void ShouldMergeWithCustomPropertyInXservices() { var customPropertyX = PropertyShouldExist("CustomStrategyProperty"); diff --git a/test/Ocelot.UnitTests/Configuration/DiskFileConfigurationRepositoryTests.cs b/test/Ocelot.UnitTests/Configuration/DiskFileConfigurationRepositoryTests.cs index 165cb8fc5..63c3dfcae 100644 --- a/test/Ocelot.UnitTests/Configuration/DiskFileConfigurationRepositoryTests.cs +++ b/test/Ocelot.UnitTests/Configuration/DiskFileConfigurationRepositoryTests.cs @@ -125,15 +125,8 @@ private void GivenTheEnvironmentNameIsUnavailable() private async Task WhenISetTheConfiguration(FileConfiguration fileConfiguration) { - await _repo.Set(fileConfiguration); - var response = await _repo.Get(); - _result = response.Data; - } - - private void WhenISetTheConfiguration() - { - _repo.SetAsync(_fileConfiguration); - _result = _repo.GetAsync().Result; + await _repo.SetAsync(fileConfiguration); + _result = await _repo.GetAsync(); } private void ThenTheConfigurationIsStoredAs(FileConfiguration expecteds) @@ -189,11 +182,7 @@ private void ThenTheConfigurationJsonIsIndented(FileConfiguration expecteds, [Ca _files.Add(environmentSpecific); } - private async Task WhenIGetTheRoutes() - { - var response = await _repo.Get(); - _result = response.Data; - } + private async Task WhenIGetTheRoutes() => _result = await _repo.GetAsync(); private void ThenTheFollowingIsReturned(FileConfiguration expecteds) { From 3ccae6ee72c62354cf89d886b8672e2205ab5e49 Mon Sep 17 00:00:00 2001 From: Raman Maksimchuk Date: Mon, 12 Jan 2026 19:15:13 +0300 Subject: [PATCH 17/18] Support case-insensitive names of sections during merging --- .../ConfigurationBuilderExtensions.cs | 29 +++++++++++++++++-- .../Configuration/ConfigurationMergeTests.cs | 7 +++-- 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs b/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs index 5f0b1a281..249c1a117 100644 --- a/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs +++ b/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs @@ -239,8 +239,8 @@ private static void MergeConfig(JToken to, JToken from, bool isGlobal) private static void MergeConfigSection(JToken to, JToken from, string sectionName) { - var destination = to[sectionName]; - var source = from[sectionName]; + var destination = to.GetSection(sectionName); + var source = from.GetSection(sectionName); if (source == null || destination == null) { return; @@ -248,11 +248,34 @@ private static void MergeConfigSection(JToken to, JToken from, string sectionNam if (source is JObject) { - to[sectionName] = source; + 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/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) From 94e251d0ba4f264146bc488fff268b05249ceeda Mon Sep 17 00:00:00 2001 From: Raman Maksimchuk Date: Mon, 12 Jan 2026 19:22:42 +0300 Subject: [PATCH 18/18] Merge DynamicRoutes section --- src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs b/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs index 249c1a117..253b5c999 100644 --- a/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs +++ b/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs @@ -235,6 +235,7 @@ private static void MergeConfig(JToken to, JToken from, bool isGlobal) 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)