From c85ea41951409a4fc1a0a38077847edb38e9492b Mon Sep 17 00:00:00 2001 From: Tom Gardham-Pallister Date: Wed, 1 Mar 2017 07:54:07 +0000 Subject: [PATCH 1/9] refactoring ocelot config creation process --- .../Creator/AuthenticationOptionsCreator.cs | 20 ++ .../Creator/ClaimsToThingCreator.cs | 41 +++ .../Creator/FileOcelotConfigurationCreator.cs | 98 ++----- .../Creator/IAuthenticationOptionsCreator.cs | 9 + .../Creator/IClaimsToThingCreator.cs | 9 + .../IUpstreamTemplatePatternCreator.cs | 9 + .../Creator/UpstreamTemplatePatternCreator.cs | 56 ++++ .../ServiceCollectionExtensions.cs | 3 + .../AuthenticationOptionsCreatorTests.cs | 74 ++++++ .../ClaimsToThingCreatorTests.cs | 110 ++++++++ .../FileConfigurationCreatorTests.cs | 246 ++++-------------- .../UpstreamTemplatePatternCreatorTests.cs | 122 +++++++++ 12 files changed, 517 insertions(+), 280 deletions(-) create mode 100644 src/Ocelot/Configuration/Creator/AuthenticationOptionsCreator.cs create mode 100644 src/Ocelot/Configuration/Creator/ClaimsToThingCreator.cs create mode 100644 src/Ocelot/Configuration/Creator/IAuthenticationOptionsCreator.cs create mode 100644 src/Ocelot/Configuration/Creator/IClaimsToThingCreator.cs create mode 100644 src/Ocelot/Configuration/Creator/IUpstreamTemplatePatternCreator.cs create mode 100644 src/Ocelot/Configuration/Creator/UpstreamTemplatePatternCreator.cs create mode 100644 test/Ocelot.UnitTests/Configuration/AuthenticationOptionsCreatorTests.cs create mode 100644 test/Ocelot.UnitTests/Configuration/ClaimsToThingCreatorTests.cs create mode 100644 test/Ocelot.UnitTests/Configuration/UpstreamTemplatePatternCreatorTests.cs diff --git a/src/Ocelot/Configuration/Creator/AuthenticationOptionsCreator.cs b/src/Ocelot/Configuration/Creator/AuthenticationOptionsCreator.cs new file mode 100644 index 00000000..85219296 --- /dev/null +++ b/src/Ocelot/Configuration/Creator/AuthenticationOptionsCreator.cs @@ -0,0 +1,20 @@ +using Ocelot.Configuration.Builder; +using Ocelot.Configuration.File; + +namespace Ocelot.Configuration.Creator +{ + public class AuthenticationOptionsCreator : IAuthenticationOptionsCreator + { + public AuthenticationOptions Create(FileReRoute fileReRoute) + { + return new AuthenticationOptionsBuilder() + .WithProvider(fileReRoute.AuthenticationOptions?.Provider) + .WithProviderRootUrl(fileReRoute.AuthenticationOptions?.ProviderRootUrl) + .WithScopeName(fileReRoute.AuthenticationOptions?.ScopeName) + .WithRequireHttps(fileReRoute.AuthenticationOptions.RequireHttps) + .WithAdditionalScopes(fileReRoute.AuthenticationOptions?.AdditionalScopes) + .WithScopeSecret(fileReRoute.AuthenticationOptions?.ScopeSecret) + .Build(); + } + } +} \ No newline at end of file diff --git a/src/Ocelot/Configuration/Creator/ClaimsToThingCreator.cs b/src/Ocelot/Configuration/Creator/ClaimsToThingCreator.cs new file mode 100644 index 00000000..27f52a5f --- /dev/null +++ b/src/Ocelot/Configuration/Creator/ClaimsToThingCreator.cs @@ -0,0 +1,41 @@ +using System.Collections.Generic; +using Ocelot.Configuration.Parser; +using Ocelot.Logging; + +namespace Ocelot.Configuration.Creator +{ + public class ClaimsToThingCreator : IClaimsToThingCreator + { + private readonly IClaimToThingConfigurationParser _claimToThingConfigParser; + private readonly IOcelotLogger _logger; + + public ClaimsToThingCreator(IClaimToThingConfigurationParser claimToThingConfigurationParser, + IOcelotLoggerFactory loggerFactory) + { + _logger = loggerFactory.CreateLogger(); + _claimToThingConfigParser = claimToThingConfigurationParser; + } + + public List Create(Dictionary inputToBeParsed) + { + var claimsToThings = new List(); + + foreach (var input in inputToBeParsed) + { + var claimToThing = _claimToThingConfigParser.Extract(input.Key, input.Value); + + if (claimToThing.IsError) + { + _logger.LogDebug("ClaimsToThingCreator.BuildAddThingsToRequest", + $"Unable to extract configuration for key: {input.Key} and value: {input.Value} your configuration file is incorrect"); + } + else + { + claimsToThings.Add(claimToThing.Data); + } + } + + return claimsToThings; + } + } +} \ No newline at end of file diff --git a/src/Ocelot/Configuration/Creator/FileOcelotConfigurationCreator.cs b/src/Ocelot/Configuration/Creator/FileOcelotConfigurationCreator.cs index b4abdec8..0d853f94 100644 --- a/src/Ocelot/Configuration/Creator/FileOcelotConfigurationCreator.cs +++ b/src/Ocelot/Configuration/Creator/FileOcelotConfigurationCreator.cs @@ -22,36 +22,38 @@ namespace Ocelot.Configuration.Creator { private readonly IOptions _options; private readonly IConfigurationValidator _configurationValidator; - private const string RegExMatchEverything = ".*"; - private const string RegExMatchEndString = "$"; - private const string RegExIgnoreCase = "(?i)"; - private const string RegExForwardSlashOnly = "^/$"; - private readonly IClaimToThingConfigurationParser _claimToThingConfigurationParser; private readonly ILogger _logger; private readonly ILoadBalancerFactory _loadBalanceFactory; private readonly ILoadBalancerHouse _loadBalancerHouse; private readonly IQoSProviderFactory _qoSProviderFactory; private readonly IQosProviderHouse _qosProviderHouse; + private readonly IClaimsToThingCreator _claimsToThingCreator; + private readonly IAuthenticationOptionsCreator _authOptionsCreator; + private IUpstreamTemplatePatternCreator _upstreamTemplatePatternCreator; public FileOcelotConfigurationCreator( IOptions options, IConfigurationValidator configurationValidator, - IClaimToThingConfigurationParser claimToThingConfigurationParser, ILogger logger, ILoadBalancerFactory loadBalancerFactory, ILoadBalancerHouse loadBalancerHouse, IQoSProviderFactory qoSProviderFactory, - IQosProviderHouse qosProviderHouse) + IQosProviderHouse qosProviderHouse, + IClaimsToThingCreator claimsToThingCreator, + IAuthenticationOptionsCreator authOptionsCreator, + IUpstreamTemplatePatternCreator upstreamTemplatePatternCreator) { + _upstreamTemplatePatternCreator = upstreamTemplatePatternCreator; + _authOptionsCreator = authOptionsCreator; _loadBalanceFactory = loadBalancerFactory; _loadBalancerHouse = loadBalancerHouse; _qoSProviderFactory = qoSProviderFactory; _qosProviderHouse = qosProviderHouse; _options = options; _configurationValidator = configurationValidator; - _claimToThingConfigurationParser = claimToThingConfigurationParser; _logger = logger; + _claimsToThingCreator = claimsToThingCreator; } public async Task> Create() @@ -107,19 +109,19 @@ namespace Ocelot.Configuration.Creator var reRouteKey = BuildReRouteKey(fileReRoute); - var upstreamTemplatePattern = BuildUpstreamTemplatePattern(fileReRoute); + var upstreamTemplatePattern = _upstreamTemplatePatternCreator.Create(fileReRoute); var isQos = IsQoS(fileReRoute); var serviceProviderConfiguration = BuildServiceProviderConfiguration(fileReRoute, globalConfiguration); - var authOptionsForRoute = BuildAuthenticationOptions(fileReRoute); + var authOptionsForRoute = _authOptionsCreator.Create(fileReRoute); - var claimsToHeaders = BuildAddThingsToRequest(fileReRoute.AddHeadersToRequest); + var claimsToHeaders = _claimsToThingCreator.Create(fileReRoute.AddHeadersToRequest); - var claimsToClaims = BuildAddThingsToRequest(fileReRoute.AddClaimsToRequest); + var claimsToClaims = _claimsToThingCreator.Create(fileReRoute.AddClaimsToRequest); - var claimsToQueries = BuildAddThingsToRequest(fileReRoute.AddQueriesToRequest); + var claimsToQueries = _claimsToThingCreator.Create(fileReRoute.AddQueriesToRequest); var qosOptions = BuildQoSOptions(fileReRoute); @@ -153,6 +155,7 @@ namespace Ocelot.Configuration.Creator .WithEnableRateLimiting(enableRateLimiting) .WithRateLimitOptions(rateLimitOption) .Build(); + await SetupLoadBalancer(reRoute); SetupQosProvider(reRoute); return reRoute; @@ -225,18 +228,6 @@ namespace Ocelot.Configuration.Creator return loadBalancerKey; } - private AuthenticationOptions BuildAuthenticationOptions(FileReRoute fileReRoute) - { - return new AuthenticationOptionsBuilder() - .WithProvider(fileReRoute.AuthenticationOptions?.Provider) - .WithProviderRootUrl(fileReRoute.AuthenticationOptions?.ProviderRootUrl) - .WithScopeName(fileReRoute.AuthenticationOptions?.ScopeName) - .WithRequireHttps(fileReRoute.AuthenticationOptions.RequireHttps) - .WithAdditionalScopes(fileReRoute.AuthenticationOptions?.AdditionalScopes) - .WithScopeSecret(fileReRoute.AuthenticationOptions?.ScopeSecret) - .Build(); - } - private async Task SetupLoadBalancer(ReRoute reRoute) { var loadBalancer = await _loadBalanceFactory.Get(reRoute); @@ -267,63 +258,6 @@ namespace Ocelot.Configuration.Creator .Build(); } - private string BuildUpstreamTemplatePattern(FileReRoute reRoute) - { - var upstreamTemplate = reRoute.UpstreamPathTemplate; - - upstreamTemplate = upstreamTemplate.SetLastCharacterAs('/'); - - var placeholders = new List(); - - for (var i = 0; i < upstreamTemplate.Length; i++) - { - if (IsPlaceHolder(upstreamTemplate, i)) - { - var postitionOfPlaceHolderClosingBracket = upstreamTemplate.IndexOf('}', i); - var difference = postitionOfPlaceHolderClosingBracket - i + 1; - var variableName = upstreamTemplate.Substring(i, difference); - placeholders.Add(variableName); - } - } - - foreach (var placeholder in placeholders) - { - upstreamTemplate = upstreamTemplate.Replace(placeholder, RegExMatchEverything); - } - - if (upstreamTemplate == "/") - { - return RegExForwardSlashOnly; - } - - var route = reRoute.ReRouteIsCaseSensitive - ? $"{upstreamTemplate}{RegExMatchEndString}" - : $"{RegExIgnoreCase}{upstreamTemplate}{RegExMatchEndString}"; - - return route; - } - - private List BuildAddThingsToRequest(Dictionary thingBeingAdded) - { - var claimsToTHings = new List(); - - foreach (var add in thingBeingAdded) - { - var claimToHeader = _claimToThingConfigurationParser.Extract(add.Key, add.Value); - - if (claimToHeader.IsError) - { - _logger.LogCritical(new EventId(1, "Application Failed to start"), - $"Unable to extract configuration for key: {add.Key} and value: {add.Value} your configuration file is incorrect"); - - throw new Exception(claimToHeader.Errors[0].Message); - } - claimsToTHings.Add(claimToHeader.Data); - } - - return claimsToTHings; - } - private bool IsPlaceHolder(string upstreamTemplate, int i) { return upstreamTemplate[i] == '{'; diff --git a/src/Ocelot/Configuration/Creator/IAuthenticationOptionsCreator.cs b/src/Ocelot/Configuration/Creator/IAuthenticationOptionsCreator.cs new file mode 100644 index 00000000..e5e82ca8 --- /dev/null +++ b/src/Ocelot/Configuration/Creator/IAuthenticationOptionsCreator.cs @@ -0,0 +1,9 @@ +using Ocelot.Configuration.File; + +namespace Ocelot.Configuration.Creator +{ + public interface IAuthenticationOptionsCreator + { + AuthenticationOptions Create(FileReRoute fileReRoute); + } +} \ No newline at end of file diff --git a/src/Ocelot/Configuration/Creator/IClaimsToThingCreator.cs b/src/Ocelot/Configuration/Creator/IClaimsToThingCreator.cs new file mode 100644 index 00000000..54ff8ddc --- /dev/null +++ b/src/Ocelot/Configuration/Creator/IClaimsToThingCreator.cs @@ -0,0 +1,9 @@ +using System.Collections.Generic; + +namespace Ocelot.Configuration.Creator +{ + public interface IClaimsToThingCreator + { + List Create(Dictionary thingsBeingAdded); + } +} \ No newline at end of file diff --git a/src/Ocelot/Configuration/Creator/IUpstreamTemplatePatternCreator.cs b/src/Ocelot/Configuration/Creator/IUpstreamTemplatePatternCreator.cs new file mode 100644 index 00000000..ae62c47a --- /dev/null +++ b/src/Ocelot/Configuration/Creator/IUpstreamTemplatePatternCreator.cs @@ -0,0 +1,9 @@ +using Ocelot.Configuration.File; + +namespace Ocelot.Configuration.Creator +{ + public interface IUpstreamTemplatePatternCreator + { + string Create(FileReRoute reRoute); + } +} \ No newline at end of file diff --git a/src/Ocelot/Configuration/Creator/UpstreamTemplatePatternCreator.cs b/src/Ocelot/Configuration/Creator/UpstreamTemplatePatternCreator.cs new file mode 100644 index 00000000..95b339a9 --- /dev/null +++ b/src/Ocelot/Configuration/Creator/UpstreamTemplatePatternCreator.cs @@ -0,0 +1,56 @@ +using System.Collections.Generic; +using Ocelot.Configuration.File; +using Ocelot.Utilities; + +namespace Ocelot.Configuration.Creator +{ + public class UpstreamTemplatePatternCreator : IUpstreamTemplatePatternCreator + { + private const string RegExMatchEverything = ".*"; + private const string RegExMatchEndString = "$"; + private const string RegExIgnoreCase = "(?i)"; + private const string RegExForwardSlashOnly = "^/$"; + + public string Create(FileReRoute reRoute) + { + var upstreamTemplate = reRoute.UpstreamPathTemplate; + + upstreamTemplate = upstreamTemplate.SetLastCharacterAs('/'); + + var placeholders = new List(); + + for (var i = 0; i < upstreamTemplate.Length; i++) + { + if (IsPlaceHolder(upstreamTemplate, i)) + { + var postitionOfPlaceHolderClosingBracket = upstreamTemplate.IndexOf('}', i); + var difference = postitionOfPlaceHolderClosingBracket - i + 1; + var variableName = upstreamTemplate.Substring(i, difference); + placeholders.Add(variableName); + } + } + + foreach (var placeholder in placeholders) + { + upstreamTemplate = upstreamTemplate.Replace(placeholder, RegExMatchEverything); + } + + if (upstreamTemplate == "/") + { + return RegExForwardSlashOnly; + } + + var route = reRoute.ReRouteIsCaseSensitive + ? $"{upstreamTemplate}{RegExMatchEndString}" + : $"{RegExIgnoreCase}{upstreamTemplate}{RegExMatchEndString}"; + + return route; + } + + + private bool IsPlaceHolder(string upstreamTemplate, int i) + { + return upstreamTemplate[i] == '{'; + } + } +} \ No newline at end of file diff --git a/src/Ocelot/DependencyInjection/ServiceCollectionExtensions.cs b/src/Ocelot/DependencyInjection/ServiceCollectionExtensions.cs index c076f367..54fef846 100644 --- a/src/Ocelot/DependencyInjection/ServiceCollectionExtensions.cs +++ b/src/Ocelot/DependencyInjection/ServiceCollectionExtensions.cs @@ -60,6 +60,9 @@ namespace Ocelot.DependencyInjection services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); + services.AddSingleton(); + services.AddSingleton(); + services.AddSingleton(); var identityServerConfiguration = IdentityServerConfigurationCreator.GetIdentityServerConfiguration(); diff --git a/test/Ocelot.UnitTests/Configuration/AuthenticationOptionsCreatorTests.cs b/test/Ocelot.UnitTests/Configuration/AuthenticationOptionsCreatorTests.cs new file mode 100644 index 00000000..9273caf7 --- /dev/null +++ b/test/Ocelot.UnitTests/Configuration/AuthenticationOptionsCreatorTests.cs @@ -0,0 +1,74 @@ +using System.Collections.Generic; +using Ocelot.Configuration; +using Ocelot.Configuration.Builder; +using Ocelot.Configuration.Creator; +using Ocelot.Configuration.File; +using Shouldly; +using TestStack.BDDfy; +using Xunit; + +namespace Ocelot.UnitTests.Configuration +{ + public class AuthenticationOptionsCreatorTests + { + private AuthenticationOptionsCreator _authOptionsCreator; + private FileReRoute _fileReRoute; + private AuthenticationOptions _result; + + public AuthenticationOptionsCreatorTests() + { + _authOptionsCreator = new AuthenticationOptionsCreator(); + } + + [Fact] + public void should_return_auth_options() + { + var fileReRoute = new FileReRoute() + { + AuthenticationOptions = new FileAuthenticationOptions + { + Provider = "Geoff", + ProviderRootUrl = "http://www.bbc.co.uk/", + ScopeName = "Laura", + RequireHttps = true, + AdditionalScopes = new List {"cheese"}, + ScopeSecret = "secret" + } + }; + + var expected = new AuthenticationOptionsBuilder() + .WithProvider(fileReRoute.AuthenticationOptions?.Provider) + .WithProviderRootUrl(fileReRoute.AuthenticationOptions?.ProviderRootUrl) + .WithScopeName(fileReRoute.AuthenticationOptions?.ScopeName) + .WithRequireHttps(fileReRoute.AuthenticationOptions.RequireHttps) + .WithAdditionalScopes(fileReRoute.AuthenticationOptions?.AdditionalScopes) + .WithScopeSecret(fileReRoute.AuthenticationOptions?.ScopeSecret) + .Build(); + + this.Given(x => x.GivenTheFollowing(fileReRoute)) + .When(x => x.WhenICreateTheAuthenticationOptions()) + .Then(x => x.ThenTheFollowingIsReturned(expected)) + .BDDfy(); + } + + private void GivenTheFollowing(FileReRoute fileReRoute) + { + _fileReRoute = fileReRoute; + } + + private void WhenICreateTheAuthenticationOptions() + { + _result = _authOptionsCreator.Create(_fileReRoute); + } + + private void ThenTheFollowingIsReturned(AuthenticationOptions expected) + { + _result.AdditionalScopes.ShouldBe(expected.AdditionalScopes); + _result.Provider.ShouldBe(expected.Provider); + _result.ProviderRootUrl.ShouldBe(expected.ProviderRootUrl); + _result.RequireHttps.ShouldBe(expected.RequireHttps); + _result.ScopeName.ShouldBe(expected.ScopeName); + _result.ScopeSecret.ShouldBe(expected.ScopeSecret); + } + } +} \ No newline at end of file diff --git a/test/Ocelot.UnitTests/Configuration/ClaimsToThingCreatorTests.cs b/test/Ocelot.UnitTests/Configuration/ClaimsToThingCreatorTests.cs new file mode 100644 index 00000000..467835d0 --- /dev/null +++ b/test/Ocelot.UnitTests/Configuration/ClaimsToThingCreatorTests.cs @@ -0,0 +1,110 @@ +using System.Collections.Generic; +using System.Linq; +using Moq; +using Ocelot.Configuration; +using Ocelot.Configuration.Creator; +using Ocelot.Configuration.Parser; +using Ocelot.Errors; +using Ocelot.Logging; +using Ocelot.Responses; +using Shouldly; +using TestStack.BDDfy; +using Xunit; + +namespace Ocelot.UnitTests.Configuration +{ + public class ClaimsToThingCreatorTests + { + private readonly Mock _configParser; + private Dictionary _claimsToThings; + private ClaimsToThingCreator _claimsToThingsCreator; + private Mock _loggerFactory; + private List _result; + private Mock _logger; + + public ClaimsToThingCreatorTests() + { + _loggerFactory = new Mock(); + _logger = new Mock(); + _loggerFactory + .Setup(x => x.CreateLogger()) + .Returns(_logger.Object); + _configParser = new Mock(); + _claimsToThingsCreator = new ClaimsToThingCreator(_configParser.Object, _loggerFactory.Object); + } + + [Fact] + public void should_return_claims_to_things() + { + var userInput = new Dictionary() + { + {"CustomerId", "Claims[CustomerId] > value"} + }; + + var claimsToThing = new OkResponse(new ClaimToThing("CustomerId", "CustomerId", "", 0)); + + this.Given(x => x.GivenTheFollowingDictionary(userInput)) + .And(x => x.GivenTheConfigHeaderExtractorReturns(claimsToThing)) + .When(x => x.WhenIGetTheThings()) + .Then(x => x.ThenTheConfigParserIsCalledCorrectly()) + .And(x => x.ThenClaimsToThingsAreReturned()) + .BDDfy(); + } + + [Fact] + public void should_log_error_if_cannot_parse_claim_to_thing() + { + var userInput = new Dictionary() + { + {"CustomerId", "Claims[CustomerId] > value"} + }; + + var claimsToThing = new ErrorResponse(It.IsAny()); + + this.Given(x => x.GivenTheFollowingDictionary(userInput)) + .And(x => x.GivenTheConfigHeaderExtractorReturns(claimsToThing)) + .When(x => x.WhenIGetTheThings()) + .Then(x => x.ThenTheConfigParserIsCalledCorrectly()) + .And(x => x.ThenNoClaimsToThingsAreReturned()) + .BDDfy(); + } + + private void ThenTheLoggerIsCalledCorrectly() + { + _logger + .Verify(x => x.LogDebug(It.IsAny(), It.IsAny()), Times.Once); + } + + private void ThenClaimsToThingsAreReturned() + { + _result.Count.ShouldBeGreaterThan(0); + } + private void GivenTheFollowingDictionary(Dictionary claimsToThings) + { + _claimsToThings = claimsToThings; + } + + private void GivenTheConfigHeaderExtractorReturns(Response expected) + { + _configParser + .Setup(x => x.Extract(It.IsAny(), It.IsAny())) + .Returns(expected); + } + + private void ThenNoClaimsToThingsAreReturned() + { + _result.Count.ShouldBe(0); + } + + private void WhenIGetTheThings() + { + _result = _claimsToThingsCreator.Create(_claimsToThings); + } + + private void ThenTheConfigParserIsCalledCorrectly() + { + _configParser + .Verify(x => x.Extract(_claimsToThings.First().Key, _claimsToThings.First().Value), Times.Once); + } + } +} \ No newline at end of file diff --git a/test/Ocelot.UnitTests/Configuration/FileConfigurationCreatorTests.cs b/test/Ocelot.UnitTests/Configuration/FileConfigurationCreatorTests.cs index fa1bbda3..bafea3cd 100644 --- a/test/Ocelot.UnitTests/Configuration/FileConfigurationCreatorTests.cs +++ b/test/Ocelot.UnitTests/Configuration/FileConfigurationCreatorTests.cs @@ -23,7 +23,6 @@ namespace Ocelot.UnitTests.Configuration private readonly Mock _validator; private Response _config; private FileConfiguration _fileConfiguration; - private readonly Mock _configParser; private readonly Mock> _logger; private readonly FileOcelotConfigurationCreator _ocelotConfigurationCreator; private readonly Mock _loadBalancerFactory; @@ -32,6 +31,9 @@ namespace Ocelot.UnitTests.Configuration private readonly Mock _qosProviderFactory; private readonly Mock _qosProviderHouse; private readonly Mock _qosProvider; + private Mock _claimsToThingCreator; + private Mock _authOptionsCreator; + private Mock _upstreamTemplatePatternCreator; public FileConfigurationCreatorTests() { @@ -39,16 +41,20 @@ namespace Ocelot.UnitTests.Configuration _qosProviderHouse = new Mock(); _qosProvider = new Mock(); _logger = new Mock>(); - _configParser = new Mock(); _validator = new Mock(); _fileConfig = new Mock>(); _loadBalancerFactory = new Mock(); _loadBalancerHouse = new Mock(); _loadBalancer = new Mock(); + _claimsToThingCreator = new Mock(); + _authOptionsCreator = new Mock(); + _upstreamTemplatePatternCreator = new Mock(); + _ocelotConfigurationCreator = new FileOcelotConfigurationCreator( - _fileConfig.Object, _validator.Object, _configParser.Object, _logger.Object, + _fileConfig.Object, _validator.Object, _logger.Object, _loadBalancerFactory.Object, _loadBalancerHouse.Object, - _qosProviderFactory.Object, _qosProviderHouse.Object); + _qosProviderFactory.Object, _qosProviderHouse.Object, _claimsToThingCreator.Object, + _authOptionsCreator.Object, _upstreamTemplatePatternCreator.Object); } [Fact] @@ -130,7 +136,6 @@ namespace Ocelot.UnitTests.Configuration .WithDownstreamPathTemplate("/products/{productId}") .WithUpstreamPathTemplate("/api/products/{productId}") .WithUpstreamHttpMethod("Get") - .WithUpstreamTemplatePattern("(?i)/api/products/.*/$") .Build() })) .BDDfy(); @@ -161,7 +166,6 @@ namespace Ocelot.UnitTests.Configuration .WithDownstreamPathTemplate("/products/{productId}") .WithUpstreamPathTemplate("/api/products/{productId}") .WithUpstreamHttpMethod("Get") - .WithUpstreamTemplatePattern("(?i)/api/products/.*/$") .Build() })) .BDDfy(); @@ -200,7 +204,6 @@ namespace Ocelot.UnitTests.Configuration .WithDownstreamPathTemplate("/products/{productId}") .WithUpstreamPathTemplate("/api/products/{productId}") .WithUpstreamHttpMethod("Get") - .WithUpstreamTemplatePattern("(?i)/api/products/.*/$") .WithServiceProviderConfiguraion(new ServiceProviderConfiguraionBuilder() .WithUseServiceDiscovery(true) .WithServiceDiscoveryProvider("consul") @@ -236,7 +239,6 @@ namespace Ocelot.UnitTests.Configuration .WithDownstreamPathTemplate("/products/{productId}") .WithUpstreamPathTemplate("/api/products/{productId}") .WithUpstreamHttpMethod("Get") - .WithUpstreamTemplatePattern("(?i)/api/products/.*/$") .WithServiceProviderConfiguraion(new ServiceProviderConfiguraionBuilder() .WithUseServiceDiscovery(false) .Build()) @@ -246,7 +248,7 @@ namespace Ocelot.UnitTests.Configuration } [Fact] - public void should_use_reroute_case_sensitivity_value() + public void should_call_template_pattern_creator_correctly() { this.Given(x => x.GivenTheConfigIs(new FileConfiguration { @@ -262,6 +264,7 @@ namespace Ocelot.UnitTests.Configuration } })) .And(x => x.GivenTheConfigIsValid()) + .And(x => x.GivenTheUpstreamTemplatePatternCreatorReturns("(?i)/api/products/.*/$")) .When(x => x.WhenICreateTheConfig()) .Then(x => x.ThenTheReRoutesAre(new List { @@ -275,65 +278,6 @@ namespace Ocelot.UnitTests.Configuration .BDDfy(); } - [Fact] - public void should_set_upstream_template_pattern_to_ignore_case_sensitivity() - { - this.Given(x => x.GivenTheConfigIs(new FileConfiguration - { - ReRoutes = new List - { - new FileReRoute - { - UpstreamPathTemplate = "/api/products/{productId}", - DownstreamPathTemplate = "/products/{productId}", - UpstreamHttpMethod = "Get" - } - } - })) - .And(x => x.GivenTheConfigIsValid()) - .When(x => x.WhenICreateTheConfig()) - .Then(x => x.ThenTheReRoutesAre(new List - { - new ReRouteBuilder() - .WithDownstreamPathTemplate("/products/{productId}") - .WithUpstreamPathTemplate("/api/products/{productId}") - .WithUpstreamHttpMethod("Get") - .WithUpstreamTemplatePattern("(?i)/api/products/.*/$") - .Build() - })) - .BDDfy(); - } - - [Fact] - public void should_set_upstream_template_pattern_to_respect_case_sensitivity() - { - this.Given(x => x.GivenTheConfigIs(new FileConfiguration - { - ReRoutes = new List - { - new FileReRoute - { - UpstreamPathTemplate = "/api/products/{productId}", - DownstreamPathTemplate = "/products/{productId}", - UpstreamHttpMethod = "Get", - ReRouteIsCaseSensitive = true - } - } - })) - .And(x => x.GivenTheConfigIsValid()) - .When(x => x.WhenICreateTheConfig()) - .Then(x => x.ThenTheReRoutesAre(new List - { - new ReRouteBuilder() - .WithDownstreamPathTemplate("/products/{productId}") - .WithUpstreamPathTemplate("/api/products/{productId}") - .WithUpstreamHttpMethod("Get") - .WithUpstreamTemplatePattern("/api/products/.*/$") - .Build() - })) - .BDDfy(); - } - [Fact] public void should_set_global_request_id_key() { @@ -362,43 +306,12 @@ namespace Ocelot.UnitTests.Configuration .WithDownstreamPathTemplate("/products/{productId}") .WithUpstreamPathTemplate("/api/products/{productId}") .WithUpstreamHttpMethod("Get") - .WithUpstreamTemplatePattern("/api/products/.*/$") .WithRequestIdKey("blahhhh") .Build() })) .BDDfy(); } - [Fact] - public void should_create_template_pattern_that_matches_anything_to_end_of_string() - { - this.Given(x => x.GivenTheConfigIs(new FileConfiguration - { - ReRoutes = new List - { - new FileReRoute - { - UpstreamPathTemplate = "/api/products/{productId}", - DownstreamPathTemplate = "/products/{productId}", - UpstreamHttpMethod = "Get", - ReRouteIsCaseSensitive = true - } - } - })) - .And(x => x.GivenTheConfigIsValid()) - .When(x => x.WhenICreateTheConfig()) - .Then(x => x.ThenTheReRoutesAre(new List - { - new ReRouteBuilder() - .WithDownstreamPathTemplate("/products/{productId}") - .WithUpstreamPathTemplate("/api/products/{productId}") - .WithUpstreamHttpMethod("Get") - .WithUpstreamTemplatePattern("/api/products/.*/$") - .Build() - })) - .BDDfy(); - } - [Fact] public void should_create_with_headers_to_extract() { @@ -417,7 +330,6 @@ namespace Ocelot.UnitTests.Configuration .WithDownstreamPathTemplate("/products/{productId}") .WithUpstreamPathTemplate("/api/products/{productId}") .WithUpstreamHttpMethod("Get") - .WithUpstreamTemplatePattern("/api/products/.*/$") .WithAuthenticationOptions(authenticationOptions) .WithClaimsToHeaders(new List { @@ -453,20 +365,17 @@ namespace Ocelot.UnitTests.Configuration } })) .And(x => x.GivenTheConfigIsValid()) - .And(x => x.GivenTheConfigHeaderExtractorReturns(new ClaimToThing("CustomerId", "CustomerId", "", 0))) + .And(x => x.GivenTheAuthOptionsCreatorReturns(authenticationOptions)) + .And(x => x.GivenTheClaimsToThingCreatorReturns(new List{new ClaimToThing("CustomerId", "CustomerId", "", 0)})) .And(x => x.GivenTheLoadBalancerFactoryReturns()) .When(x => x.WhenICreateTheConfig()) .Then(x => x.ThenTheReRoutesAre(expected)) .And(x => x.ThenTheAuthenticationOptionsAre(expected)) + .And(x => x.ThenTheAuthOptionsCreatorIsCalledCorrectly()) .BDDfy(); } - private void GivenTheConfigHeaderExtractorReturns(ClaimToThing expected) - { - _configParser - .Setup(x => x.Extract(It.IsAny(), It.IsAny())) - .Returns(new OkResponse(expected)); - } + [Fact] public void should_create_with_authentication_properties() @@ -486,7 +395,6 @@ namespace Ocelot.UnitTests.Configuration .WithDownstreamPathTemplate("/products/{productId}") .WithUpstreamPathTemplate("/api/products/{productId}") .WithUpstreamHttpMethod("Get") - .WithUpstreamTemplatePattern("/api/products/.*/$") .WithAuthenticationOptions(authenticationOptions) .Build() }; @@ -514,100 +422,12 @@ namespace Ocelot.UnitTests.Configuration } })) .And(x => x.GivenTheConfigIsValid()) + .And(x => x.GivenTheAuthOptionsCreatorReturns(authenticationOptions)) .And(x => x.GivenTheLoadBalancerFactoryReturns()) .When(x => x.WhenICreateTheConfig()) .Then(x => x.ThenTheReRoutesAre(expected)) .And(x => x.ThenTheAuthenticationOptionsAre(expected)) - .BDDfy(); - } - - [Fact] - public void should_create_template_pattern_that_matches_more_than_one_placeholder() - { - this.Given(x => x.GivenTheConfigIs(new FileConfiguration - { - ReRoutes = new List - { - new FileReRoute - { - UpstreamPathTemplate = "/api/products/{productId}/variants/{variantId}", - DownstreamPathTemplate = "/products/{productId}", - UpstreamHttpMethod = "Get", - ReRouteIsCaseSensitive = true - } - } - })) - .And(x => x.GivenTheConfigIsValid()) - .When(x => x.WhenICreateTheConfig()) - .Then(x => x.ThenTheReRoutesAre(new List - { - new ReRouteBuilder() - .WithDownstreamPathTemplate("/products/{productId}") - .WithUpstreamPathTemplate("/api/products/{productId}/variants/{variantId}") - .WithUpstreamHttpMethod("Get") - .WithUpstreamTemplatePattern("/api/products/.*/variants/.*/$") - .Build() - })) - .BDDfy(); - } - - [Fact] - public void should_create_template_pattern_that_matches_more_than_one_placeholder_with_trailing_slash() - { - this.Given(x => x.GivenTheConfigIs(new FileConfiguration - { - ReRoutes = new List - { - new FileReRoute - { - UpstreamPathTemplate = "/api/products/{productId}/variants/{variantId}/", - DownstreamPathTemplate = "/products/{productId}", - UpstreamHttpMethod = "Get", - ReRouteIsCaseSensitive = true - } - } - })) - .And(x => x.GivenTheConfigIsValid()) - .When(x => x.WhenICreateTheConfig()) - .Then(x => x.ThenTheReRoutesAre(new List - { - new ReRouteBuilder() - .WithDownstreamPathTemplate("/products/{productId}") - .WithUpstreamPathTemplate("/api/products/{productId}/variants/{variantId}/") - .WithUpstreamHttpMethod("Get") - .WithUpstreamTemplatePattern("/api/products/.*/variants/.*/$") - .Build() - })) - .BDDfy(); - } - - [Fact] - public void should_create_template_pattern_that_matches_to_end_of_string() - { - this.Given(x => x.GivenTheConfigIs(new FileConfiguration - { - ReRoutes = new List - { - new FileReRoute - { - UpstreamPathTemplate = "/", - DownstreamPathTemplate = "/api/products/", - UpstreamHttpMethod = "Get", - ReRouteIsCaseSensitive = true - } - } - })) - .And(x => x.GivenTheConfigIsValid()) - .When(x => x.WhenICreateTheConfig()) - .Then(x => x.ThenTheReRoutesAre(new List - { - new ReRouteBuilder() - .WithDownstreamPathTemplate("/api/products/") - .WithUpstreamPathTemplate("/") - .WithUpstreamHttpMethod("Get") - .WithUpstreamTemplatePattern("^/$") - .Build() - })) + .And(x => x.ThenTheAuthOptionsCreatorIsCalledCorrectly()) .BDDfy(); } @@ -642,6 +462,9 @@ namespace Ocelot.UnitTests.Configuration result.UpstreamHttpMethod.ShouldBe(expected.UpstreamHttpMethod); result.UpstreamPathTemplate.Value.ShouldBe(expected.UpstreamPathTemplate.Value); result.UpstreamTemplatePattern.ShouldBe(expected.UpstreamTemplatePattern); + result.ClaimsToClaims.Count.ShouldBe(expected.ClaimsToClaims.Count); + result.ClaimsToHeaders.Count.ShouldBe(expected.ClaimsToHeaders.Count); + result.ClaimsToQueries.Count.ShouldBe(expected.ClaimsToQueries.Count); } } @@ -699,5 +522,32 @@ namespace Ocelot.UnitTests.Configuration _qosProviderHouse .Verify(x => x.Add(It.IsAny(), _qosProvider.Object), Times.Once); } + + private void GivenTheClaimsToThingCreatorReturns(List claimsToThing) + { + _claimsToThingCreator + .Setup(x => x.Create(_fileConfiguration.ReRoutes[0].AddHeadersToRequest)) + .Returns(claimsToThing); + } + + private void GivenTheAuthOptionsCreatorReturns(AuthenticationOptions authOptions) + { + _authOptionsCreator + .Setup(x => x.Create(It.IsAny())) + .Returns(authOptions); + } + + private void ThenTheAuthOptionsCreatorIsCalledCorrectly() + { + _authOptionsCreator + .Verify(x => x.Create(_fileConfiguration.ReRoutes[0]), Times.Once); + } + + private void GivenTheUpstreamTemplatePatternCreatorReturns(string pattern) + { + _upstreamTemplatePatternCreator + .Setup(x => x.Create(It.IsAny())) + .Returns(pattern); + } } } diff --git a/test/Ocelot.UnitTests/Configuration/UpstreamTemplatePatternCreatorTests.cs b/test/Ocelot.UnitTests/Configuration/UpstreamTemplatePatternCreatorTests.cs new file mode 100644 index 00000000..34db1232 --- /dev/null +++ b/test/Ocelot.UnitTests/Configuration/UpstreamTemplatePatternCreatorTests.cs @@ -0,0 +1,122 @@ +using Ocelot.Configuration.Creator; +using Ocelot.Configuration.File; +using Shouldly; +using TestStack.BDDfy; +using Xunit; + +namespace Ocelot.UnitTests.Configuration +{ + public class UpstreamTemplatePatternCreatorTests + { + private FileReRoute _fileReRoute; + private UpstreamTemplatePatternCreator _creator; + private string _result; + + public UpstreamTemplatePatternCreatorTests() + { + _creator = new UpstreamTemplatePatternCreator(); + } + + [Fact] + public void should_set_upstream_template_pattern_to_ignore_case_sensitivity() + { + var fileReRoute = new FileReRoute + { + UpstreamPathTemplate = "/PRODUCTS/{productId}", + ReRouteIsCaseSensitive = false + }; + + this.Given(x => x.GivenTheFollowingFileReRoute(fileReRoute)) + .When(x => x.WhenICreateTheTemplatePattern()) + .Then(x => x.ThenTheFollowingIsReturned("(?i)/PRODUCTS/.*/$")) + .BDDfy(); + } + + [Fact] + public void should_set_upstream_template_pattern_to_respect_case_sensitivity() + { + var fileReRoute = new FileReRoute + { + UpstreamPathTemplate = "/PRODUCTS/{productId}", + ReRouteIsCaseSensitive = true + }; + this.Given(x => x.GivenTheFollowingFileReRoute(fileReRoute)) + .When(x => x.WhenICreateTheTemplatePattern()) + .Then(x => x.ThenTheFollowingIsReturned("/PRODUCTS/.*/$")) + .BDDfy(); + } + + [Fact] + public void should_create_template_pattern_that_matches_anything_to_end_of_string() + { + var fileReRoute = new FileReRoute + { + UpstreamPathTemplate = "/api/products/{productId}", + ReRouteIsCaseSensitive = true + }; + + this.Given(x => x.GivenTheFollowingFileReRoute(fileReRoute)) + .When(x => x.WhenICreateTheTemplatePattern()) + .Then(x => x.ThenTheFollowingIsReturned("/api/products/.*/$")) + .BDDfy(); + } + + [Fact] + public void should_create_template_pattern_that_matches_more_than_one_placeholder() + { + var fileReRoute = new FileReRoute + { + UpstreamPathTemplate = "/api/products/{productId}/variants/{variantId}", + ReRouteIsCaseSensitive = true + }; + + this.Given(x => x.GivenTheFollowingFileReRoute(fileReRoute)) + .When(x => x.WhenICreateTheTemplatePattern()) + .Then(x => x.ThenTheFollowingIsReturned("/api/products/.*/variants/.*/$")) + .BDDfy(); + } + [Fact] + public void should_create_template_pattern_that_matches_more_than_one_placeholder_with_trailing_slash() + { + var fileReRoute = new FileReRoute + { + UpstreamPathTemplate = "/api/products/{productId}/variants/{variantId}/", + ReRouteIsCaseSensitive = true + }; + + this.Given(x => x.GivenTheFollowingFileReRoute(fileReRoute)) + .When(x => x.WhenICreateTheTemplatePattern()) + .Then(x => x.ThenTheFollowingIsReturned("/api/products/.*/variants/.*/$")) + .BDDfy(); + } + + [Fact] + public void should_create_template_pattern_that_matches_to_end_of_string() + { + var fileReRoute = new FileReRoute + { + UpstreamPathTemplate = "/" + }; + + this.Given(x => x.GivenTheFollowingFileReRoute(fileReRoute)) + .When(x => x.WhenICreateTheTemplatePattern()) + .Then(x => x.ThenTheFollowingIsReturned("^/$")) + .BDDfy(); + } + + private void GivenTheFollowingFileReRoute(FileReRoute fileReRoute) + { + _fileReRoute = fileReRoute; + } + + private void WhenICreateTheTemplatePattern() + { + _result = _creator.Create(_fileReRoute); + } + + private void ThenTheFollowingIsReturned(string expected) + { + _result.ShouldBe(expected); + } + } +} \ No newline at end of file From d4119ab33dfeb1bcd4268fa9e1dca6eb921329f6 Mon Sep 17 00:00:00 2001 From: Tom Gardham-Pallister Date: Wed, 1 Mar 2017 08:11:39 +0000 Subject: [PATCH 2/9] extracted thing that creates request id key --- .../Creator/FileOcelotConfigurationCreator.cs | 18 +--- .../Creator/IRequestIdKeyCreator.cs | 9 ++ .../Creator/RequestIdKeyCreator.cs | 18 ++++ .../ServiceCollectionExtensions.cs | 1 + .../FileConfigurationCreatorTests.cs | 25 ++++- .../Configuration/RequestIdKeyCreatorTests.cs | 91 +++++++++++++++++++ 6 files changed, 146 insertions(+), 16 deletions(-) create mode 100644 src/Ocelot/Configuration/Creator/IRequestIdKeyCreator.cs create mode 100644 src/Ocelot/Configuration/Creator/RequestIdKeyCreator.cs create mode 100644 test/Ocelot.UnitTests/Configuration/RequestIdKeyCreatorTests.cs diff --git a/src/Ocelot/Configuration/Creator/FileOcelotConfigurationCreator.cs b/src/Ocelot/Configuration/Creator/FileOcelotConfigurationCreator.cs index 0d853f94..c9161dee 100644 --- a/src/Ocelot/Configuration/Creator/FileOcelotConfigurationCreator.cs +++ b/src/Ocelot/Configuration/Creator/FileOcelotConfigurationCreator.cs @@ -31,6 +31,7 @@ namespace Ocelot.Configuration.Creator private readonly IClaimsToThingCreator _claimsToThingCreator; private readonly IAuthenticationOptionsCreator _authOptionsCreator; private IUpstreamTemplatePatternCreator _upstreamTemplatePatternCreator; + private IRequestIdKeyCreator _requestIdKeyCreator; public FileOcelotConfigurationCreator( IOptions options, @@ -42,8 +43,10 @@ namespace Ocelot.Configuration.Creator IQosProviderHouse qosProviderHouse, IClaimsToThingCreator claimsToThingCreator, IAuthenticationOptionsCreator authOptionsCreator, - IUpstreamTemplatePatternCreator upstreamTemplatePatternCreator) + IUpstreamTemplatePatternCreator upstreamTemplatePatternCreator, + IRequestIdKeyCreator requestIdKeyCreator) { + _requestIdKeyCreator = requestIdKeyCreator; _upstreamTemplatePatternCreator = upstreamTemplatePatternCreator; _authOptionsCreator = authOptionsCreator; _loadBalanceFactory = loadBalancerFactory; @@ -105,7 +108,7 @@ namespace Ocelot.Configuration.Creator var isCached = IsCached(fileReRoute); - var requestIdKey = BuildRequestId(fileReRoute, globalConfiguration); + var requestIdKey = _requestIdKeyCreator.Create(fileReRoute, globalConfiguration); var reRouteKey = BuildReRouteKey(fileReRoute); @@ -210,17 +213,6 @@ namespace Ocelot.Configuration.Creator return fileReRoute.FileCacheOptions.TtlSeconds > 0; } - private string BuildRequestId(FileReRoute fileReRoute, FileGlobalConfiguration globalConfiguration) - { - var globalRequestIdConfiguration = !string.IsNullOrEmpty(globalConfiguration?.RequestIdKey); - - var requestIdKey = globalRequestIdConfiguration - ? globalConfiguration.RequestIdKey - : fileReRoute.RequestIdKey; - - return requestIdKey; - } - private string BuildReRouteKey(FileReRoute fileReRoute) { //note - not sure if this is the correct key, but this is probably the only unique key i can think of given my poor brain diff --git a/src/Ocelot/Configuration/Creator/IRequestIdKeyCreator.cs b/src/Ocelot/Configuration/Creator/IRequestIdKeyCreator.cs new file mode 100644 index 00000000..2800f8a5 --- /dev/null +++ b/src/Ocelot/Configuration/Creator/IRequestIdKeyCreator.cs @@ -0,0 +1,9 @@ +using Ocelot.Configuration.File; + +namespace Ocelot.Configuration.Creator +{ + public interface IRequestIdKeyCreator + { + string Create(FileReRoute fileReRoute, FileGlobalConfiguration globalConfiguration); + } +} \ No newline at end of file diff --git a/src/Ocelot/Configuration/Creator/RequestIdKeyCreator.cs b/src/Ocelot/Configuration/Creator/RequestIdKeyCreator.cs new file mode 100644 index 00000000..caf00402 --- /dev/null +++ b/src/Ocelot/Configuration/Creator/RequestIdKeyCreator.cs @@ -0,0 +1,18 @@ +using Ocelot.Configuration.File; + +namespace Ocelot.Configuration.Creator +{ + public class RequestIdKeyCreator : IRequestIdKeyCreator + { + public string Create(FileReRoute fileReRoute, FileGlobalConfiguration globalConfiguration) + { + var globalRequestIdConfiguration = !string.IsNullOrEmpty(globalConfiguration?.RequestIdKey); + + var requestIdKey = globalRequestIdConfiguration + ? globalConfiguration.RequestIdKey + : fileReRoute.RequestIdKey; + + return requestIdKey; + } + } +} \ No newline at end of file diff --git a/src/Ocelot/DependencyInjection/ServiceCollectionExtensions.cs b/src/Ocelot/DependencyInjection/ServiceCollectionExtensions.cs index 54fef846..9b777248 100644 --- a/src/Ocelot/DependencyInjection/ServiceCollectionExtensions.cs +++ b/src/Ocelot/DependencyInjection/ServiceCollectionExtensions.cs @@ -63,6 +63,7 @@ namespace Ocelot.DependencyInjection services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); + services.AddSingleton(); var identityServerConfiguration = IdentityServerConfigurationCreator.GetIdentityServerConfiguration(); diff --git a/test/Ocelot.UnitTests/Configuration/FileConfigurationCreatorTests.cs b/test/Ocelot.UnitTests/Configuration/FileConfigurationCreatorTests.cs index bafea3cd..b0a4fa06 100644 --- a/test/Ocelot.UnitTests/Configuration/FileConfigurationCreatorTests.cs +++ b/test/Ocelot.UnitTests/Configuration/FileConfigurationCreatorTests.cs @@ -34,6 +34,7 @@ namespace Ocelot.UnitTests.Configuration private Mock _claimsToThingCreator; private Mock _authOptionsCreator; private Mock _upstreamTemplatePatternCreator; + private Mock _requestIdKeyCreator; public FileConfigurationCreatorTests() { @@ -49,12 +50,13 @@ namespace Ocelot.UnitTests.Configuration _claimsToThingCreator = new Mock(); _authOptionsCreator = new Mock(); _upstreamTemplatePatternCreator = new Mock(); + _requestIdKeyCreator = new Mock(); _ocelotConfigurationCreator = new FileOcelotConfigurationCreator( _fileConfig.Object, _validator.Object, _logger.Object, _loadBalancerFactory.Object, _loadBalancerHouse.Object, _qosProviderFactory.Object, _qosProviderHouse.Object, _claimsToThingCreator.Object, - _authOptionsCreator.Object, _upstreamTemplatePatternCreator.Object); + _authOptionsCreator.Object, _upstreamTemplatePatternCreator.Object, _requestIdKeyCreator.Object); } [Fact] @@ -279,7 +281,7 @@ namespace Ocelot.UnitTests.Configuration } [Fact] - public void should_set_global_request_id_key() + public void should_call_request_id_creator() { this.Given(x => x.GivenTheConfigIs(new FileConfiguration { @@ -298,7 +300,8 @@ namespace Ocelot.UnitTests.Configuration RequestIdKey = "blahhhh" } })) - .And(x => x.GivenTheConfigIsValid()) + .And(x => x.GivenTheConfigIsValid()) + .And(x => x.GivenTheRequestIdCreatorReturns("blahhhh")) .When(x => x.WhenICreateTheConfig()) .Then(x => x.ThenTheReRoutesAre(new List { @@ -309,6 +312,7 @@ namespace Ocelot.UnitTests.Configuration .WithRequestIdKey("blahhhh") .Build() })) + .And(x => x.ThenTheRequestIdKeyCreatorIsCalledCorrectly()) .BDDfy(); } @@ -465,6 +469,7 @@ namespace Ocelot.UnitTests.Configuration result.ClaimsToClaims.Count.ShouldBe(expected.ClaimsToClaims.Count); result.ClaimsToHeaders.Count.ShouldBe(expected.ClaimsToHeaders.Count); result.ClaimsToQueries.Count.ShouldBe(expected.ClaimsToQueries.Count); + result.RequestIdKey.ShouldBe(expected.RequestIdKey); } } @@ -549,5 +554,19 @@ namespace Ocelot.UnitTests.Configuration .Setup(x => x.Create(It.IsAny())) .Returns(pattern); } + + private void ThenTheRequestIdKeyCreatorIsCalledCorrectly() + { + _requestIdKeyCreator + .Verify(x => x.Create(_fileConfiguration.ReRoutes[0], _fileConfiguration.GlobalConfiguration), Times.Once); + } + + private void GivenTheRequestIdCreatorReturns(string requestId) + { + _requestIdKeyCreator + .Setup(x => x.Create(It.IsAny(), It.IsAny())) + .Returns(requestId); + } + } } diff --git a/test/Ocelot.UnitTests/Configuration/RequestIdKeyCreatorTests.cs b/test/Ocelot.UnitTests/Configuration/RequestIdKeyCreatorTests.cs new file mode 100644 index 00000000..f52cb808 --- /dev/null +++ b/test/Ocelot.UnitTests/Configuration/RequestIdKeyCreatorTests.cs @@ -0,0 +1,91 @@ +using Ocelot.Configuration.Creator; +using Ocelot.Configuration.File; +using Shouldly; +using TestStack.BDDfy; +using Xunit; + +namespace Ocelot.UnitTests.Configuration +{ + public class RequestIdKeyCreatorTests + { + private FileReRoute _fileReRoute; + private FileGlobalConfiguration _fileGlobalConfig; + private string _result; + private RequestIdKeyCreator _creator; + + public RequestIdKeyCreatorTests() + { + _creator = new RequestIdKeyCreator(); + } + + [Fact] + public void should_use_global_configuration() + { + var reRoute = new FileReRoute(); + var globalConfig = new FileGlobalConfiguration + { + RequestIdKey = "cheese" + }; + + this.Given(x => x.GivenTheFollowingReRoute(reRoute)) + .And(x => x.GivenTheFollowingGlobalConfig(globalConfig)) + .When(x => x.WhenICreate()) + .Then(x => x.ThenTheFollowingIsReturned("cheese")) + .BDDfy(); + } + + [Fact] + public void should_use_re_route_specific() + { + var reRoute = new FileReRoute + { + RequestIdKey = "cheese" + }; + var globalConfig = new FileGlobalConfiguration(); + + this.Given(x => x.GivenTheFollowingReRoute(reRoute)) + .And(x => x.GivenTheFollowingGlobalConfig(globalConfig)) + .When(x => x.WhenICreate()) + .Then(x => x.ThenTheFollowingIsReturned("cheese")) + .BDDfy(); + } + + [Fact] + public void should_use_global_cofiguration_over_re_route_specific() + { + var reRoute = new FileReRoute + { + RequestIdKey = "cheese" + }; var globalConfig = new FileGlobalConfiguration + { + RequestIdKey = "cheese" + }; + + this.Given(x => x.GivenTheFollowingReRoute(reRoute)) + .And(x => x.GivenTheFollowingGlobalConfig(globalConfig)) + .When(x => x.WhenICreate()) + .Then(x => x.ThenTheFollowingIsReturned("cheese")) + .BDDfy(); + } + + private void GivenTheFollowingReRoute(FileReRoute fileReRoute) + { + _fileReRoute = fileReRoute; + } + + private void GivenTheFollowingGlobalConfig(FileGlobalConfiguration globalConfig) + { + _fileGlobalConfig = globalConfig; + } + + private void WhenICreate() + { + _result = _creator.Create(_fileReRoute, _fileGlobalConfig); + } + + private void ThenTheFollowingIsReturned(string expected) + { + _result.ShouldBe(expected); + } + } +} \ No newline at end of file From fff743ccf8102f528900aa7ef407866f92bcf394 Mon Sep 17 00:00:00 2001 From: Tom Gardham-Pallister Date: Wed, 1 Mar 2017 12:42:37 +0000 Subject: [PATCH 3/9] pulling out service config cretor --- .../Configuration/Builder/ReRouteBuilder.cs | 4 +-- .../ServiceProviderConfiguraionBuilder.cs | 4 +-- .../Creator/FileOcelotConfigurationCreator.cs | 29 +++++-------------- .../IServiceProviderConfigurationCreator.cs | 9 ++++++ .../ServiceProviderConfigurationCreator.cs | 26 +++++++++++++++++ src/Ocelot/Configuration/ReRoute.cs | 4 +-- .../ServiceProviderConfiguraion.cs | 4 +-- .../ServiceCollectionExtensions.cs | 1 + .../IServiceDiscoveryProviderFactory.cs | 2 +- .../ServiceDiscoveryProviderFactory.cs | 2 +- .../FileConfigurationCreatorTests.cs | 21 +++++++++++++- .../LoadBalancer/LoadBalancerFactoryTests.cs | 4 +-- .../ServiceProviderFactoryTests.cs | 4 +-- 13 files changed, 77 insertions(+), 37 deletions(-) create mode 100644 src/Ocelot/Configuration/Creator/IServiceProviderConfigurationCreator.cs create mode 100644 src/Ocelot/Configuration/Creator/ServiceProviderConfigurationCreator.cs diff --git a/src/Ocelot/Configuration/Builder/ReRouteBuilder.cs b/src/Ocelot/Configuration/Builder/ReRouteBuilder.cs index 1fc31f03..30d9d87a 100644 --- a/src/Ocelot/Configuration/Builder/ReRouteBuilder.cs +++ b/src/Ocelot/Configuration/Builder/ReRouteBuilder.cs @@ -25,7 +25,7 @@ namespace Ocelot.Configuration.Builder private string _downstreamHost; private int _downstreamPort; private string _loadBalancer; - private ServiceProviderConfiguraion _serviceProviderConfiguraion; + private ServiceProviderConfiguration _serviceProviderConfiguraion; private bool _useQos; private QoSOptions _qosOptions; public bool _enableRateLimiting; @@ -150,7 +150,7 @@ namespace Ocelot.Configuration.Builder return this; } - public ReRouteBuilder WithServiceProviderConfiguraion(ServiceProviderConfiguraion serviceProviderConfiguraion) + public ReRouteBuilder WithServiceProviderConfiguraion(ServiceProviderConfiguration serviceProviderConfiguraion) { _serviceProviderConfiguraion = serviceProviderConfiguraion; return this; diff --git a/src/Ocelot/Configuration/Builder/ServiceProviderConfiguraionBuilder.cs b/src/Ocelot/Configuration/Builder/ServiceProviderConfiguraionBuilder.cs index 129acae8..7c3e598a 100644 --- a/src/Ocelot/Configuration/Builder/ServiceProviderConfiguraionBuilder.cs +++ b/src/Ocelot/Configuration/Builder/ServiceProviderConfiguraionBuilder.cs @@ -53,9 +53,9 @@ namespace Ocelot.Configuration.Builder } - public ServiceProviderConfiguraion Build() + public ServiceProviderConfiguration Build() { - return new ServiceProviderConfiguraion(_serviceName, _downstreamHost, _downstreamPort, _userServiceDiscovery, + return new ServiceProviderConfiguration(_serviceName, _downstreamHost, _downstreamPort, _userServiceDiscovery, _serviceDiscoveryProvider, _serviceDiscoveryProviderHost,_serviceDiscoveryProviderPort); } } diff --git a/src/Ocelot/Configuration/Creator/FileOcelotConfigurationCreator.cs b/src/Ocelot/Configuration/Creator/FileOcelotConfigurationCreator.cs index c9161dee..9432aeee 100644 --- a/src/Ocelot/Configuration/Creator/FileOcelotConfigurationCreator.cs +++ b/src/Ocelot/Configuration/Creator/FileOcelotConfigurationCreator.cs @@ -32,6 +32,7 @@ namespace Ocelot.Configuration.Creator private readonly IAuthenticationOptionsCreator _authOptionsCreator; private IUpstreamTemplatePatternCreator _upstreamTemplatePatternCreator; private IRequestIdKeyCreator _requestIdKeyCreator; + private IServiceProviderConfigurationCreator _serviceProviderConfigCreator; public FileOcelotConfigurationCreator( IOptions options, @@ -44,7 +45,8 @@ namespace Ocelot.Configuration.Creator IClaimsToThingCreator claimsToThingCreator, IAuthenticationOptionsCreator authOptionsCreator, IUpstreamTemplatePatternCreator upstreamTemplatePatternCreator, - IRequestIdKeyCreator requestIdKeyCreator) + IRequestIdKeyCreator requestIdKeyCreator, + IServiceProviderConfigurationCreator serviceProviderConfigCreator) { _requestIdKeyCreator = requestIdKeyCreator; _upstreamTemplatePatternCreator = upstreamTemplatePatternCreator; @@ -57,6 +59,7 @@ namespace Ocelot.Configuration.Creator _configurationValidator = configurationValidator; _logger = logger; _claimsToThingCreator = claimsToThingCreator; + _serviceProviderConfigCreator = serviceProviderConfigCreator; } public async Task> Create() @@ -110,13 +113,13 @@ namespace Ocelot.Configuration.Creator var requestIdKey = _requestIdKeyCreator.Create(fileReRoute, globalConfiguration); - var reRouteKey = BuildReRouteKey(fileReRoute); + var reRouteKey = CreateReRouteKey(fileReRoute); var upstreamTemplatePattern = _upstreamTemplatePatternCreator.Create(fileReRoute); var isQos = IsQoS(fileReRoute); - var serviceProviderConfiguration = BuildServiceProviderConfiguration(fileReRoute, globalConfiguration); + var serviceProviderConfiguration = _serviceProviderConfigCreator.Create(fileReRoute, globalConfiguration); var authOptionsForRoute = _authOptionsCreator.Create(fileReRoute); @@ -213,7 +216,7 @@ namespace Ocelot.Configuration.Creator return fileReRoute.FileCacheOptions.TtlSeconds > 0; } - private string BuildReRouteKey(FileReRoute fileReRoute) + private string CreateReRouteKey(FileReRoute fileReRoute) { //note - not sure if this is the correct key, but this is probably the only unique key i can think of given my poor brain var loadBalancerKey = $"{fileReRoute.UpstreamPathTemplate}{fileReRoute.UpstreamHttpMethod}"; @@ -232,24 +235,6 @@ namespace Ocelot.Configuration.Creator _qosProviderHouse.Add(reRoute.ReRouteKey, loadBalancer); } - private ServiceProviderConfiguraion BuildServiceProviderConfiguration(FileReRoute fileReRoute, FileGlobalConfiguration globalConfiguration) - { - var useServiceDiscovery = !string.IsNullOrEmpty(fileReRoute.ServiceName) - && !string.IsNullOrEmpty(globalConfiguration?.ServiceDiscoveryProvider?.Provider); - - var serviceProviderPort = globalConfiguration?.ServiceDiscoveryProvider?.Port ?? 0; - - return new ServiceProviderConfiguraionBuilder() - .WithServiceName(fileReRoute.ServiceName) - .WithDownstreamHost(fileReRoute.DownstreamHost) - .WithDownstreamPort(fileReRoute.DownstreamPort) - .WithUseServiceDiscovery(useServiceDiscovery) - .WithServiceDiscoveryProvider(globalConfiguration?.ServiceDiscoveryProvider?.Provider) - .WithServiceDiscoveryProviderHost(globalConfiguration?.ServiceDiscoveryProvider?.Host) - .WithServiceDiscoveryProviderPort(serviceProviderPort) - .Build(); - } - private bool IsPlaceHolder(string upstreamTemplate, int i) { return upstreamTemplate[i] == '{'; diff --git a/src/Ocelot/Configuration/Creator/IServiceProviderConfigurationCreator.cs b/src/Ocelot/Configuration/Creator/IServiceProviderConfigurationCreator.cs new file mode 100644 index 00000000..1c03d893 --- /dev/null +++ b/src/Ocelot/Configuration/Creator/IServiceProviderConfigurationCreator.cs @@ -0,0 +1,9 @@ +using Ocelot.Configuration.File; + +namespace Ocelot.Configuration.Creator +{ + public interface IServiceProviderConfigurationCreator + { + ServiceProviderConfiguration Create(FileReRoute fileReRoute, FileGlobalConfiguration globalConfiguration); + } +} \ No newline at end of file diff --git a/src/Ocelot/Configuration/Creator/ServiceProviderConfigurationCreator.cs b/src/Ocelot/Configuration/Creator/ServiceProviderConfigurationCreator.cs new file mode 100644 index 00000000..074d1661 --- /dev/null +++ b/src/Ocelot/Configuration/Creator/ServiceProviderConfigurationCreator.cs @@ -0,0 +1,26 @@ +using Ocelot.Configuration.Builder; +using Ocelot.Configuration.File; + +namespace Ocelot.Configuration.Creator +{ + public class ServiceProviderConfigurationCreator : IServiceProviderConfigurationCreator + { + public ServiceProviderConfiguration Create(FileReRoute fileReRoute, FileGlobalConfiguration globalConfiguration) + { + var useServiceDiscovery = !string.IsNullOrEmpty(fileReRoute.ServiceName) + && !string.IsNullOrEmpty(globalConfiguration?.ServiceDiscoveryProvider?.Provider); + + var serviceProviderPort = globalConfiguration?.ServiceDiscoveryProvider?.Port ?? 0; + + return new ServiceProviderConfiguraionBuilder() + .WithServiceName(fileReRoute.ServiceName) + .WithDownstreamHost(fileReRoute.DownstreamHost) + .WithDownstreamPort(fileReRoute.DownstreamPort) + .WithUseServiceDiscovery(useServiceDiscovery) + .WithServiceDiscoveryProvider(globalConfiguration?.ServiceDiscoveryProvider?.Provider) + .WithServiceDiscoveryProviderHost(globalConfiguration?.ServiceDiscoveryProvider?.Host) + .WithServiceDiscoveryProviderPort(serviceProviderPort) + .Build(); + } + } +} \ No newline at end of file diff --git a/src/Ocelot/Configuration/ReRoute.cs b/src/Ocelot/Configuration/ReRoute.cs index f629867a..97bf8e39 100644 --- a/src/Ocelot/Configuration/ReRoute.cs +++ b/src/Ocelot/Configuration/ReRoute.cs @@ -25,7 +25,7 @@ namespace Ocelot.Configuration string downstreamHost, int downstreamPort, string reRouteKey, - ServiceProviderConfiguraion serviceProviderConfiguraion, + ServiceProviderConfiguration serviceProviderConfiguraion, bool isQos, QoSOptions qos, bool enableRateLimit, @@ -81,7 +81,7 @@ namespace Ocelot.Configuration public string LoadBalancer {get;private set;} public string DownstreamHost { get; private set; } public int DownstreamPort { get; private set; } - public ServiceProviderConfiguraion ServiceProviderConfiguraion { get; private set; } + public ServiceProviderConfiguration ServiceProviderConfiguraion { get; private set; } public bool EnableEndpointRateLimiting { get; private set; } public RateLimitOptions RateLimitOptions { get; private set; } } diff --git a/src/Ocelot/Configuration/ServiceProviderConfiguraion.cs b/src/Ocelot/Configuration/ServiceProviderConfiguraion.cs index d471a9e5..664fa4c3 100644 --- a/src/Ocelot/Configuration/ServiceProviderConfiguraion.cs +++ b/src/Ocelot/Configuration/ServiceProviderConfiguraion.cs @@ -1,8 +1,8 @@ namespace Ocelot.Configuration { - public class ServiceProviderConfiguraion + public class ServiceProviderConfiguration { - public ServiceProviderConfiguraion(string serviceName, string downstreamHost, + public ServiceProviderConfiguration(string serviceName, string downstreamHost, int downstreamPort, bool useServiceDiscovery, string serviceDiscoveryProvider, string serviceProviderHost, int serviceProviderPort) { ServiceName = serviceName; diff --git a/src/Ocelot/DependencyInjection/ServiceCollectionExtensions.cs b/src/Ocelot/DependencyInjection/ServiceCollectionExtensions.cs index 9b777248..7d950e84 100644 --- a/src/Ocelot/DependencyInjection/ServiceCollectionExtensions.cs +++ b/src/Ocelot/DependencyInjection/ServiceCollectionExtensions.cs @@ -64,6 +64,7 @@ namespace Ocelot.DependencyInjection services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); + services.AddSingleton(); var identityServerConfiguration = IdentityServerConfigurationCreator.GetIdentityServerConfiguration(); diff --git a/src/Ocelot/ServiceDiscovery/IServiceDiscoveryProviderFactory.cs b/src/Ocelot/ServiceDiscovery/IServiceDiscoveryProviderFactory.cs index 6c6c3d4c..bece9fc6 100644 --- a/src/Ocelot/ServiceDiscovery/IServiceDiscoveryProviderFactory.cs +++ b/src/Ocelot/ServiceDiscovery/IServiceDiscoveryProviderFactory.cs @@ -5,6 +5,6 @@ namespace Ocelot.ServiceDiscovery { public interface IServiceDiscoveryProviderFactory { - IServiceDiscoveryProvider Get(ServiceProviderConfiguraion serviceConfig); + IServiceDiscoveryProvider Get(ServiceProviderConfiguration serviceConfig); } } \ No newline at end of file diff --git a/src/Ocelot/ServiceDiscovery/ServiceDiscoveryProviderFactory.cs b/src/Ocelot/ServiceDiscovery/ServiceDiscoveryProviderFactory.cs index 00622190..49151c01 100644 --- a/src/Ocelot/ServiceDiscovery/ServiceDiscoveryProviderFactory.cs +++ b/src/Ocelot/ServiceDiscovery/ServiceDiscoveryProviderFactory.cs @@ -6,7 +6,7 @@ namespace Ocelot.ServiceDiscovery { public class ServiceDiscoveryProviderFactory : IServiceDiscoveryProviderFactory { - public IServiceDiscoveryProvider Get(ServiceProviderConfiguraion serviceConfig) + public IServiceDiscoveryProvider Get(ServiceProviderConfiguration serviceConfig) { if (serviceConfig.UseServiceDiscovery) { diff --git a/test/Ocelot.UnitTests/Configuration/FileConfigurationCreatorTests.cs b/test/Ocelot.UnitTests/Configuration/FileConfigurationCreatorTests.cs index b0a4fa06..987816fd 100644 --- a/test/Ocelot.UnitTests/Configuration/FileConfigurationCreatorTests.cs +++ b/test/Ocelot.UnitTests/Configuration/FileConfigurationCreatorTests.cs @@ -35,6 +35,7 @@ namespace Ocelot.UnitTests.Configuration private Mock _authOptionsCreator; private Mock _upstreamTemplatePatternCreator; private Mock _requestIdKeyCreator; + private Mock _serviceProviderConfigCreator; public FileConfigurationCreatorTests() { @@ -51,12 +52,14 @@ namespace Ocelot.UnitTests.Configuration _authOptionsCreator = new Mock(); _upstreamTemplatePatternCreator = new Mock(); _requestIdKeyCreator = new Mock(); + _serviceProviderConfigCreator = new Mock(); _ocelotConfigurationCreator = new FileOcelotConfigurationCreator( _fileConfig.Object, _validator.Object, _logger.Object, _loadBalancerFactory.Object, _loadBalancerHouse.Object, _qosProviderFactory.Object, _qosProviderHouse.Object, _claimsToThingCreator.Object, - _authOptionsCreator.Object, _upstreamTemplatePatternCreator.Object, _requestIdKeyCreator.Object); + _authOptionsCreator.Object, _upstreamTemplatePatternCreator.Object, _requestIdKeyCreator.Object, + _serviceProviderConfigCreator.Object); } [Fact] @@ -470,9 +473,25 @@ namespace Ocelot.UnitTests.Configuration result.ClaimsToHeaders.Count.ShouldBe(expected.ClaimsToHeaders.Count); result.ClaimsToQueries.Count.ShouldBe(expected.ClaimsToQueries.Count); result.RequestIdKey.ShouldBe(expected.RequestIdKey); + } } + private void ThenTheServiceConfigurationIs(ServiceProviderConfiguration expected) + { + for (int i = 0; i < _config.Data.ReRoutes.Count; i++) + { + var result = _config.Data.ReRoutes[i]; + result.ServiceProviderConfiguraion.DownstreamHost.ShouldBe(expected.DownstreamHost); + result.ServiceProviderConfiguraion.DownstreamPort.ShouldBe(expected.DownstreamPort); + result.ServiceProviderConfiguraion.ServiceDiscoveryProvider.ShouldBe(expected.ServiceDiscoveryProvider); + result.ServiceProviderConfiguraion.ServiceName.ShouldBe(expected.ServiceName); + result.ServiceProviderConfiguraion.ServiceProviderHost.ShouldBe(expected.ServiceProviderHost); + result.ServiceProviderConfiguraion.ServiceProviderPort.ShouldBe(expected.ServiceProviderPort); + } + } + + private void ThenTheAuthenticationOptionsAre(List expectedReRoutes) { for (int i = 0; i < _config.Data.ReRoutes.Count; i++) diff --git a/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerFactoryTests.cs b/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerFactoryTests.cs index 767b4272..9b2fc727 100644 --- a/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerFactoryTests.cs +++ b/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerFactoryTests.cs @@ -90,14 +90,14 @@ namespace Ocelot.UnitTests.LoadBalancer private void GivenTheServiceProviderFactoryReturns() { _serviceProviderFactory - .Setup(x => x.Get(It.IsAny())) + .Setup(x => x.Get(It.IsAny())) .Returns(_serviceProvider.Object); } private void ThenTheServiceProviderIsCalledCorrectly() { _serviceProviderFactory - .Verify(x => x.Get(It.IsAny()), Times.Once); + .Verify(x => x.Get(It.IsAny()), Times.Once); } private void GivenAReRoute(ReRoute reRoute) diff --git a/test/Ocelot.UnitTests/ServiceDiscovery/ServiceProviderFactoryTests.cs b/test/Ocelot.UnitTests/ServiceDiscovery/ServiceProviderFactoryTests.cs index b3053afa..9f88b95e 100644 --- a/test/Ocelot.UnitTests/ServiceDiscovery/ServiceProviderFactoryTests.cs +++ b/test/Ocelot.UnitTests/ServiceDiscovery/ServiceProviderFactoryTests.cs @@ -9,7 +9,7 @@ namespace Ocelot.UnitTests.ServiceDiscovery { public class ServiceProviderFactoryTests { - private ServiceProviderConfiguraion _serviceConfig; + private ServiceProviderConfiguration _serviceConfig; private IServiceDiscoveryProvider _result; private readonly ServiceDiscoveryProviderFactory _factory; @@ -48,7 +48,7 @@ namespace Ocelot.UnitTests.ServiceDiscovery .BDDfy(); } - private void GivenTheReRoute(ServiceProviderConfiguraion serviceConfig) + private void GivenTheReRoute(ServiceProviderConfiguration serviceConfig) { _serviceConfig = serviceConfig; } From 034732ce909ae2898553ab8e073720ca5d5da259 Mon Sep 17 00:00:00 2001 From: Tom Gardham-Pallister Date: Wed, 1 Mar 2017 23:12:00 +0000 Subject: [PATCH 4/9] added lame test for service config creator --- ...=> ServiceProviderConfigurationBuilder.cs} | 16 ++-- .../ServiceProviderConfigurationCreator.cs | 2 +- .../FileConfigurationCreatorTests.cs | 4 +- .../ServiceProviderCreatorTests.cs | 76 +++++++++++++++++++ .../LoadBalancer/LoadBalancerFactoryTests.cs | 8 +- .../ServiceProviderFactoryTests.cs | 4 +- 6 files changed, 93 insertions(+), 17 deletions(-) rename src/Ocelot/Configuration/Builder/{ServiceProviderConfiguraionBuilder.cs => ServiceProviderConfigurationBuilder.cs} (64%) create mode 100644 test/Ocelot.UnitTests/Configuration/ServiceProviderCreatorTests.cs diff --git a/src/Ocelot/Configuration/Builder/ServiceProviderConfiguraionBuilder.cs b/src/Ocelot/Configuration/Builder/ServiceProviderConfigurationBuilder.cs similarity index 64% rename from src/Ocelot/Configuration/Builder/ServiceProviderConfiguraionBuilder.cs rename to src/Ocelot/Configuration/Builder/ServiceProviderConfigurationBuilder.cs index 7c3e598a..cb3c521c 100644 --- a/src/Ocelot/Configuration/Builder/ServiceProviderConfiguraionBuilder.cs +++ b/src/Ocelot/Configuration/Builder/ServiceProviderConfigurationBuilder.cs @@ -1,6 +1,6 @@ namespace Ocelot.Configuration.Builder { - public class ServiceProviderConfiguraionBuilder + public class ServiceProviderConfigurationBuilder { private string _serviceName; private string _downstreamHost; @@ -10,43 +10,43 @@ namespace Ocelot.Configuration.Builder private string _serviceDiscoveryProviderHost; private int _serviceDiscoveryProviderPort; - public ServiceProviderConfiguraionBuilder WithServiceName(string serviceName) + public ServiceProviderConfigurationBuilder WithServiceName(string serviceName) { _serviceName = serviceName; return this; } - public ServiceProviderConfiguraionBuilder WithDownstreamHost(string downstreamHost) + public ServiceProviderConfigurationBuilder WithDownstreamHost(string downstreamHost) { _downstreamHost = downstreamHost; return this; } - public ServiceProviderConfiguraionBuilder WithDownstreamPort(int downstreamPort) + public ServiceProviderConfigurationBuilder WithDownstreamPort(int downstreamPort) { _downstreamPort = downstreamPort; return this; } - public ServiceProviderConfiguraionBuilder WithUseServiceDiscovery(bool userServiceDiscovery) + public ServiceProviderConfigurationBuilder WithUseServiceDiscovery(bool userServiceDiscovery) { _userServiceDiscovery = userServiceDiscovery; return this; } - public ServiceProviderConfiguraionBuilder WithServiceDiscoveryProvider(string serviceDiscoveryProvider) + public ServiceProviderConfigurationBuilder WithServiceDiscoveryProvider(string serviceDiscoveryProvider) { _serviceDiscoveryProvider = serviceDiscoveryProvider; return this; } - public ServiceProviderConfiguraionBuilder WithServiceDiscoveryProviderHost(string serviceDiscoveryProviderHost) + public ServiceProviderConfigurationBuilder WithServiceDiscoveryProviderHost(string serviceDiscoveryProviderHost) { _serviceDiscoveryProviderHost = serviceDiscoveryProviderHost; return this; } - public ServiceProviderConfiguraionBuilder WithServiceDiscoveryProviderPort(int serviceDiscoveryProviderPort) + public ServiceProviderConfigurationBuilder WithServiceDiscoveryProviderPort(int serviceDiscoveryProviderPort) { _serviceDiscoveryProviderPort = serviceDiscoveryProviderPort; return this; diff --git a/src/Ocelot/Configuration/Creator/ServiceProviderConfigurationCreator.cs b/src/Ocelot/Configuration/Creator/ServiceProviderConfigurationCreator.cs index 074d1661..8132d021 100644 --- a/src/Ocelot/Configuration/Creator/ServiceProviderConfigurationCreator.cs +++ b/src/Ocelot/Configuration/Creator/ServiceProviderConfigurationCreator.cs @@ -12,7 +12,7 @@ namespace Ocelot.Configuration.Creator var serviceProviderPort = globalConfiguration?.ServiceDiscoveryProvider?.Port ?? 0; - return new ServiceProviderConfiguraionBuilder() + return new ServiceProviderConfigurationBuilder() .WithServiceName(fileReRoute.ServiceName) .WithDownstreamHost(fileReRoute.DownstreamHost) .WithDownstreamPort(fileReRoute.DownstreamPort) diff --git a/test/Ocelot.UnitTests/Configuration/FileConfigurationCreatorTests.cs b/test/Ocelot.UnitTests/Configuration/FileConfigurationCreatorTests.cs index 987816fd..3ea7af3b 100644 --- a/test/Ocelot.UnitTests/Configuration/FileConfigurationCreatorTests.cs +++ b/test/Ocelot.UnitTests/Configuration/FileConfigurationCreatorTests.cs @@ -209,7 +209,7 @@ namespace Ocelot.UnitTests.Configuration .WithDownstreamPathTemplate("/products/{productId}") .WithUpstreamPathTemplate("/api/products/{productId}") .WithUpstreamHttpMethod("Get") - .WithServiceProviderConfiguraion(new ServiceProviderConfiguraionBuilder() + .WithServiceProviderConfiguraion(new ServiceProviderConfigurationBuilder() .WithUseServiceDiscovery(true) .WithServiceDiscoveryProvider("consul") .WithServiceDiscoveryProviderHost("127.0.0.1") @@ -244,7 +244,7 @@ namespace Ocelot.UnitTests.Configuration .WithDownstreamPathTemplate("/products/{productId}") .WithUpstreamPathTemplate("/api/products/{productId}") .WithUpstreamHttpMethod("Get") - .WithServiceProviderConfiguraion(new ServiceProviderConfiguraionBuilder() + .WithServiceProviderConfiguraion(new ServiceProviderConfigurationBuilder() .WithUseServiceDiscovery(false) .Build()) .Build() diff --git a/test/Ocelot.UnitTests/Configuration/ServiceProviderCreatorTests.cs b/test/Ocelot.UnitTests/Configuration/ServiceProviderCreatorTests.cs new file mode 100644 index 00000000..8d76e864 --- /dev/null +++ b/test/Ocelot.UnitTests/Configuration/ServiceProviderCreatorTests.cs @@ -0,0 +1,76 @@ +using Ocelot.Configuration; +using Ocelot.Configuration.Builder; +using Ocelot.Configuration.Creator; +using Ocelot.Configuration.File; +using Shouldly; +using TestStack.BDDfy; +using Xunit; + +namespace Ocelot.UnitTests.Configuration +{ + public class ServiceProviderCreatorTests + { + private ServiceProviderConfigurationCreator _creator; + private FileReRoute _reRoute; + private FileGlobalConfiguration _globalConfig; + private ServiceProviderConfiguration _result; + + public ServiceProviderCreatorTests() + { + _creator = new ServiceProviderConfigurationCreator(); + } + + [Fact] + public void should_create_service_provider_config() + { + var reRoute = new FileReRoute(); + + var globalConfig = new FileGlobalConfiguration + { + ServiceDiscoveryProvider = new FileServiceDiscoveryProvider + { + Provider = "consul", + Host = "127.0.0.1", + Port = 1234 + } + }; + + var expected = new ServiceProviderConfigurationBuilder() + .WithServiceDiscoveryProvider("consul") + .WithServiceDiscoveryProviderHost("127.0.0.1") + .WithServiceDiscoveryProviderPort(1234) + .Build(); + + this.Given(x => x.GivenTheFollowingReRoute(reRoute)) + .And(x => x.GivenTheFollowingGlobalConfig(globalConfig)) + .When(x => x.WhenICreate()) + .Then(x => x.ThenTheConfigIs(expected)) + .BDDfy(); + } + + private void GivenTheFollowingReRoute(FileReRoute fileReRoute) + { + _reRoute = fileReRoute; + } + + private void GivenTheFollowingGlobalConfig(FileGlobalConfiguration fileGlobalConfig) + { + _globalConfig = fileGlobalConfig; + } + + private void WhenICreate() + { + _result = _creator.Create(_reRoute, _globalConfig); + } + + private void ThenTheConfigIs(ServiceProviderConfiguration expected) + { + _result.DownstreamHost.ShouldBe(expected.DownstreamHost); + _result.DownstreamPort.ShouldBe(expected.DownstreamPort); + _result.ServiceDiscoveryProvider.ShouldBe(expected.ServiceDiscoveryProvider); + _result.ServiceName.ShouldBe(expected.ServiceName); + _result.ServiceProviderHost.ShouldBe(expected.ServiceProviderHost); + _result.ServiceProviderPort.ShouldBe(expected.ServiceProviderPort); + } + } +} \ No newline at end of file diff --git a/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerFactoryTests.cs b/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerFactoryTests.cs index 9b2fc727..f8bf8f6c 100644 --- a/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerFactoryTests.cs +++ b/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerFactoryTests.cs @@ -28,7 +28,7 @@ namespace Ocelot.UnitTests.LoadBalancer public void should_return_no_load_balancer() { var reRoute = new ReRouteBuilder() - .WithServiceProviderConfiguraion(new ServiceProviderConfiguraionBuilder().Build()) + .WithServiceProviderConfiguraion(new ServiceProviderConfigurationBuilder().Build()) .WithUpstreamHttpMethod("Get") .Build(); @@ -45,7 +45,7 @@ namespace Ocelot.UnitTests.LoadBalancer var reRoute = new ReRouteBuilder() .WithLoadBalancer("RoundRobin") .WithUpstreamHttpMethod("Get") - .WithServiceProviderConfiguraion(new ServiceProviderConfiguraionBuilder().Build()) + .WithServiceProviderConfiguraion(new ServiceProviderConfigurationBuilder().Build()) .Build(); this.Given(x => x.GivenAReRoute(reRoute)) @@ -61,7 +61,7 @@ namespace Ocelot.UnitTests.LoadBalancer var reRoute = new ReRouteBuilder() .WithLoadBalancer("LeastConnection") .WithUpstreamHttpMethod("Get") - .WithServiceProviderConfiguraion(new ServiceProviderConfiguraionBuilder().Build()) + .WithServiceProviderConfiguraion(new ServiceProviderConfigurationBuilder().Build()) .Build(); this.Given(x => x.GivenAReRoute(reRoute)) @@ -77,7 +77,7 @@ namespace Ocelot.UnitTests.LoadBalancer var reRoute = new ReRouteBuilder() .WithLoadBalancer("RoundRobin") .WithUpstreamHttpMethod("Get") - .WithServiceProviderConfiguraion(new ServiceProviderConfiguraionBuilder().Build()) + .WithServiceProviderConfiguraion(new ServiceProviderConfigurationBuilder().Build()) .Build(); this.Given(x => x.GivenAReRoute(reRoute)) diff --git a/test/Ocelot.UnitTests/ServiceDiscovery/ServiceProviderFactoryTests.cs b/test/Ocelot.UnitTests/ServiceDiscovery/ServiceProviderFactoryTests.cs index 9f88b95e..afa86c1b 100644 --- a/test/Ocelot.UnitTests/ServiceDiscovery/ServiceProviderFactoryTests.cs +++ b/test/Ocelot.UnitTests/ServiceDiscovery/ServiceProviderFactoryTests.cs @@ -21,7 +21,7 @@ namespace Ocelot.UnitTests.ServiceDiscovery [Fact] public void should_return_no_service_provider() { - var serviceConfig = new ServiceProviderConfiguraionBuilder() + var serviceConfig = new ServiceProviderConfigurationBuilder() .WithDownstreamHost("127.0.0.1") .WithDownstreamPort(80) .WithUseServiceDiscovery(false) @@ -36,7 +36,7 @@ namespace Ocelot.UnitTests.ServiceDiscovery [Fact] public void should_return_consul_service_provider() { - var serviceConfig = new ServiceProviderConfiguraionBuilder() + var serviceConfig = new ServiceProviderConfigurationBuilder() .WithServiceName("product") .WithUseServiceDiscovery(true) .WithServiceDiscoveryProvider("Consul") From 6661cb5f32f9d5562123ab7aeed6188bc58f1126 Mon Sep 17 00:00:00 2001 From: Tom Gardham-Pallister Date: Wed, 1 Mar 2017 23:15:30 +0000 Subject: [PATCH 5/9] use config tests --- .../Configuration/Creator/FileOcelotConfigurationCreator.cs | 1 - src/Ocelot/ServiceDiscovery/IServiceDiscoveryProviderFactory.cs | 1 - .../Configuration/FileConfigurationCreatorTests.cs | 2 -- 3 files changed, 4 deletions(-) diff --git a/src/Ocelot/Configuration/Creator/FileOcelotConfigurationCreator.cs b/src/Ocelot/Configuration/Creator/FileOcelotConfigurationCreator.cs index 9432aeee..65962070 100644 --- a/src/Ocelot/Configuration/Creator/FileOcelotConfigurationCreator.cs +++ b/src/Ocelot/Configuration/Creator/FileOcelotConfigurationCreator.cs @@ -22,7 +22,6 @@ namespace Ocelot.Configuration.Creator { private readonly IOptions _options; private readonly IConfigurationValidator _configurationValidator; - private readonly ILogger _logger; private readonly ILoadBalancerFactory _loadBalanceFactory; private readonly ILoadBalancerHouse _loadBalancerHouse; diff --git a/src/Ocelot/ServiceDiscovery/IServiceDiscoveryProviderFactory.cs b/src/Ocelot/ServiceDiscovery/IServiceDiscoveryProviderFactory.cs index bece9fc6..18c88c76 100644 --- a/src/Ocelot/ServiceDiscovery/IServiceDiscoveryProviderFactory.cs +++ b/src/Ocelot/ServiceDiscovery/IServiceDiscoveryProviderFactory.cs @@ -1,4 +1,3 @@ -using System; using Ocelot.Configuration; namespace Ocelot.ServiceDiscovery diff --git a/test/Ocelot.UnitTests/Configuration/FileConfigurationCreatorTests.cs b/test/Ocelot.UnitTests/Configuration/FileConfigurationCreatorTests.cs index 3ea7af3b..f8b9425f 100644 --- a/test/Ocelot.UnitTests/Configuration/FileConfigurationCreatorTests.cs +++ b/test/Ocelot.UnitTests/Configuration/FileConfigurationCreatorTests.cs @@ -6,7 +6,6 @@ using Ocelot.Configuration; using Ocelot.Configuration.Builder; using Ocelot.Configuration.Creator; using Ocelot.Configuration.File; -using Ocelot.Configuration.Parser; using Ocelot.Configuration.Validator; using Ocelot.LoadBalancer.LoadBalancers; using Ocelot.Requester.QoS; @@ -491,7 +490,6 @@ namespace Ocelot.UnitTests.Configuration } } - private void ThenTheAuthenticationOptionsAre(List expectedReRoutes) { for (int i = 0; i < _config.Data.ReRoutes.Count; i++) From 0a2d7a6922e0508a67db3fd50489b241e7e05035 Mon Sep 17 00:00:00 2001 From: Tom Gardham-Pallister Date: Wed, 1 Mar 2017 23:28:32 +0000 Subject: [PATCH 6/9] qos options creator in own class --- .../Creator/FileOcelotConfigurationCreator.cs | 17 +++--- .../Creator/IQoSOptionsCreator.cs | 9 ++++ .../Creator/QoSOptionsCreator.cs | 17 ++++++ .../ServiceCollectionExtensions.cs | 1 + .../FileConfigurationCreatorTests.cs | 54 ++++++++++++++++++- .../FileConfigurationControllerTests.cs | 2 - 6 files changed, 86 insertions(+), 14 deletions(-) create mode 100644 src/Ocelot/Configuration/Creator/IQoSOptionsCreator.cs create mode 100644 src/Ocelot/Configuration/Creator/QoSOptionsCreator.cs diff --git a/src/Ocelot/Configuration/Creator/FileOcelotConfigurationCreator.cs b/src/Ocelot/Configuration/Creator/FileOcelotConfigurationCreator.cs index 65962070..4dd873cd 100644 --- a/src/Ocelot/Configuration/Creator/FileOcelotConfigurationCreator.cs +++ b/src/Ocelot/Configuration/Creator/FileOcelotConfigurationCreator.cs @@ -32,6 +32,7 @@ namespace Ocelot.Configuration.Creator private IUpstreamTemplatePatternCreator _upstreamTemplatePatternCreator; private IRequestIdKeyCreator _requestIdKeyCreator; private IServiceProviderConfigurationCreator _serviceProviderConfigCreator; + private IQoSOptionsCreator _qosOptionsCreator; public FileOcelotConfigurationCreator( IOptions options, @@ -45,7 +46,9 @@ namespace Ocelot.Configuration.Creator IAuthenticationOptionsCreator authOptionsCreator, IUpstreamTemplatePatternCreator upstreamTemplatePatternCreator, IRequestIdKeyCreator requestIdKeyCreator, - IServiceProviderConfigurationCreator serviceProviderConfigCreator) + IServiceProviderConfigurationCreator serviceProviderConfigCreator, + IQoSOptionsCreator qosOptionsCreator + ) { _requestIdKeyCreator = requestIdKeyCreator; _upstreamTemplatePatternCreator = upstreamTemplatePatternCreator; @@ -59,6 +62,7 @@ namespace Ocelot.Configuration.Creator _logger = logger; _claimsToThingCreator = claimsToThingCreator; _serviceProviderConfigCreator = serviceProviderConfigCreator; + _qosOptionsCreator = qosOptionsCreator; } public async Task> Create() @@ -128,7 +132,7 @@ namespace Ocelot.Configuration.Creator var claimsToQueries = _claimsToThingCreator.Create(fileReRoute.AddQueriesToRequest); - var qosOptions = BuildQoSOptions(fileReRoute); + var qosOptions = _qosOptionsCreator.Create(fileReRoute); var enableRateLimiting = IsEnableRateLimiting(fileReRoute); @@ -186,15 +190,6 @@ namespace Ocelot.Configuration.Creator return (fileReRoute.RateLimitOptions != null && fileReRoute.RateLimitOptions.EnableRateLimiting) ? true : false; } - private QoSOptions BuildQoSOptions(FileReRoute fileReRoute) - { - return new QoSOptionsBuilder() - .WithExceptionsAllowedBeforeBreaking(fileReRoute.QoSOptions.ExceptionsAllowedBeforeBreaking) - .WithDurationOfBreak(fileReRoute.QoSOptions.DurationOfBreak) - .WithTimeoutValue(fileReRoute.QoSOptions.TimeoutValue) - .Build(); - } - private bool IsQoS(FileReRoute fileReRoute) { return fileReRoute.QoSOptions?.ExceptionsAllowedBeforeBreaking > 0 && fileReRoute.QoSOptions?.TimeoutValue > 0; diff --git a/src/Ocelot/Configuration/Creator/IQoSOptionsCreator.cs b/src/Ocelot/Configuration/Creator/IQoSOptionsCreator.cs new file mode 100644 index 00000000..a0686ae5 --- /dev/null +++ b/src/Ocelot/Configuration/Creator/IQoSOptionsCreator.cs @@ -0,0 +1,9 @@ +using Ocelot.Configuration.File; + +namespace Ocelot.Configuration.Creator +{ + public interface IQoSOptionsCreator + { + QoSOptions Create(FileReRoute fileReRoute); + } +} \ No newline at end of file diff --git a/src/Ocelot/Configuration/Creator/QoSOptionsCreator.cs b/src/Ocelot/Configuration/Creator/QoSOptionsCreator.cs new file mode 100644 index 00000000..ddeef910 --- /dev/null +++ b/src/Ocelot/Configuration/Creator/QoSOptionsCreator.cs @@ -0,0 +1,17 @@ +using Ocelot.Configuration.Builder; +using Ocelot.Configuration.File; + +namespace Ocelot.Configuration.Creator +{ + public class QoSOptionsCreator : IQoSOptionsCreator + { + public QoSOptions Create(FileReRoute fileReRoute) + { + return new QoSOptionsBuilder() + .WithExceptionsAllowedBeforeBreaking(fileReRoute.QoSOptions.ExceptionsAllowedBeforeBreaking) + .WithDurationOfBreak(fileReRoute.QoSOptions.DurationOfBreak) + .WithTimeoutValue(fileReRoute.QoSOptions.TimeoutValue) + .Build(); + } + } +} \ No newline at end of file diff --git a/src/Ocelot/DependencyInjection/ServiceCollectionExtensions.cs b/src/Ocelot/DependencyInjection/ServiceCollectionExtensions.cs index 7d950e84..1ebb6d42 100644 --- a/src/Ocelot/DependencyInjection/ServiceCollectionExtensions.cs +++ b/src/Ocelot/DependencyInjection/ServiceCollectionExtensions.cs @@ -65,6 +65,7 @@ namespace Ocelot.DependencyInjection services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); + services.AddSingleton(); var identityServerConfiguration = IdentityServerConfigurationCreator.GetIdentityServerConfiguration(); diff --git a/test/Ocelot.UnitTests/Configuration/FileConfigurationCreatorTests.cs b/test/Ocelot.UnitTests/Configuration/FileConfigurationCreatorTests.cs index f8b9425f..bb5866f9 100644 --- a/test/Ocelot.UnitTests/Configuration/FileConfigurationCreatorTests.cs +++ b/test/Ocelot.UnitTests/Configuration/FileConfigurationCreatorTests.cs @@ -35,6 +35,7 @@ namespace Ocelot.UnitTests.Configuration private Mock _upstreamTemplatePatternCreator; private Mock _requestIdKeyCreator; private Mock _serviceProviderConfigCreator; + private Mock _qosOptionsCreator; public FileConfigurationCreatorTests() { @@ -52,13 +53,50 @@ namespace Ocelot.UnitTests.Configuration _upstreamTemplatePatternCreator = new Mock(); _requestIdKeyCreator = new Mock(); _serviceProviderConfigCreator = new Mock(); + _qosOptionsCreator = new Mock(); _ocelotConfigurationCreator = new FileOcelotConfigurationCreator( _fileConfig.Object, _validator.Object, _logger.Object, _loadBalancerFactory.Object, _loadBalancerHouse.Object, _qosProviderFactory.Object, _qosProviderHouse.Object, _claimsToThingCreator.Object, _authOptionsCreator.Object, _upstreamTemplatePatternCreator.Object, _requestIdKeyCreator.Object, - _serviceProviderConfigCreator.Object); + _serviceProviderConfigCreator.Object, _qosOptionsCreator.Object); + } + + [Fact] + public void should_call_qos_options_creator() + { + var expected = new QoSOptionsBuilder() + .WithDurationOfBreak(1) + .WithExceptionsAllowedBeforeBreaking(1) + .WithTimeoutValue(1) + .Build(); + + this.Given(x => x.GivenTheConfigIs(new FileConfiguration + { + ReRoutes = new List + { + new FileReRoute + { + DownstreamHost = "127.0.0.1", + UpstreamPathTemplate = "/api/products/{productId}", + DownstreamPathTemplate = "/products/{productId}", + UpstreamHttpMethod = "Get", + QoSOptions = new FileQoSOptions + { + TimeoutValue = 1, + DurationOfBreak = 1, + ExceptionsAllowedBeforeBreaking = 1 + } + } + }, + })) + .And(x => x.GivenTheConfigIsValid()) + .And(x => x.GivenTheQosProviderFactoryReturns()) + .And(x => x.GivenTheQosOptionsCreatorReturns(expected)) + .When(x => x.WhenICreateTheConfig()) + .Then(x => x.ThenTheQosOptionsAre(expected)) + .BDDfy(); } [Fact] @@ -585,5 +623,19 @@ namespace Ocelot.UnitTests.Configuration .Returns(requestId); } + private void GivenTheQosOptionsCreatorReturns(QoSOptions qosOptions) + { + _qosOptionsCreator + .Setup(x => x.Create(_fileConfiguration.ReRoutes[0])) + .Returns(qosOptions); + } + + private void ThenTheQosOptionsAre(QoSOptions qosOptions) + { + _config.Data.ReRoutes[0].QosOptions.DurationOfBreak.ShouldBe(qosOptions.DurationOfBreak); + + _config.Data.ReRoutes[0].QosOptions.ExceptionsAllowedBeforeBreaking.ShouldBe(qosOptions.ExceptionsAllowedBeforeBreaking); + _config.Data.ReRoutes[0].QosOptions.TimeoutValue.ShouldBe(qosOptions.TimeoutValue); + } } } diff --git a/test/Ocelot.UnitTests/Controllers/FileConfigurationControllerTests.cs b/test/Ocelot.UnitTests/Controllers/FileConfigurationControllerTests.cs index 7f4d7b87..5e041e7e 100644 --- a/test/Ocelot.UnitTests/Controllers/FileConfigurationControllerTests.cs +++ b/test/Ocelot.UnitTests/Controllers/FileConfigurationControllerTests.cs @@ -1,5 +1,3 @@ -using System; -using System.Net; using Microsoft.AspNetCore.Mvc; using Moq; using Ocelot.Configuration.File; From b44c02510af9904a5253ab0a3e6f1f6be9cd8aeb Mon Sep 17 00:00:00 2001 From: Tom Gardham-Pallister Date: Wed, 1 Mar 2017 23:34:56 +0000 Subject: [PATCH 7/9] unit test for qos --- .../Configuration/QoSOptionsCreatorTests.cs | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 test/Ocelot.UnitTests/Configuration/QoSOptionsCreatorTests.cs diff --git a/test/Ocelot.UnitTests/Configuration/QoSOptionsCreatorTests.cs b/test/Ocelot.UnitTests/Configuration/QoSOptionsCreatorTests.cs new file mode 100644 index 00000000..be3ac27a --- /dev/null +++ b/test/Ocelot.UnitTests/Configuration/QoSOptionsCreatorTests.cs @@ -0,0 +1,63 @@ +using Ocelot.Configuration; +using Ocelot.Configuration.Builder; +using Ocelot.Configuration.Creator; +using Ocelot.Configuration.File; +using Shouldly; +using TestStack.BDDfy; +using Xunit; + +namespace Ocelot.UnitTests.Configuration +{ + public class QoSOptionsCreatorTests + { + private QoSOptionsCreator _creator; + private FileReRoute _fileReRoute; + private QoSOptions _result; + + public QoSOptionsCreatorTests() + { + _creator = new QoSOptionsCreator(); + } + + [Fact] + public void should_create_qos_options() + { + var reRoute = new FileReRoute + { + QoSOptions = new FileQoSOptions + { + ExceptionsAllowedBeforeBreaking = 1, + DurationOfBreak = 1, + TimeoutValue = 1 + } + }; + var expected = new QoSOptionsBuilder() + .WithDurationOfBreak(1) + .WithExceptionsAllowedBeforeBreaking(1) + .WithTimeoutValue(1) + .Build(); + + this.Given(x => x.GivenTheFollowingReRoute(reRoute)) + .When(x => x.WhenICreate()) + .Then(x => x.ThenTheFollowingIsReturned(expected)) + .BDDfy(); + } + + private void GivenTheFollowingReRoute(FileReRoute fileReRoute) + { + _fileReRoute = fileReRoute; + } + + private void WhenICreate() + { + _result = _creator.Create(_fileReRoute); + } + + private void ThenTheFollowingIsReturned(QoSOptions expected) + { + _result.DurationOfBreak.ShouldBe(expected.DurationOfBreak); + _result.ExceptionsAllowedBeforeBreaking.ShouldBe(expected.ExceptionsAllowedBeforeBreaking); + _result.TimeoutValue.ShouldBe(expected.TimeoutValue); + } + } +} \ No newline at end of file From 8bbd781820a5429ef36705e1c7742123ff46f552 Mon Sep 17 00:00:00 2001 From: Tom Gardham-Pallister Date: Thu, 2 Mar 2017 09:18:53 +0000 Subject: [PATCH 8/9] updated file options --- .../Builder/ReRouteOptionsBuilder.cs | 46 ++++++ .../Creator/FileOcelotConfigurationCreator.cs | 57 ++------ .../Creator/IReRouteOptionsCreator.cs | 9 ++ .../Creator/ReRouteOptionsCreator.cs | 52 +++++++ src/Ocelot/Configuration/ReRouteOptions.cs | 20 +++ .../ServiceCollectionExtensions.cs | 1 + .../FileConfigurationCreatorTests.cs | 131 +++++++++++------- .../ReRouteOptionsCreatorTests.cs | 84 +++++++++++ 8 files changed, 301 insertions(+), 99 deletions(-) create mode 100644 src/Ocelot/Configuration/Builder/ReRouteOptionsBuilder.cs create mode 100644 src/Ocelot/Configuration/Creator/IReRouteOptionsCreator.cs create mode 100644 src/Ocelot/Configuration/Creator/ReRouteOptionsCreator.cs create mode 100644 src/Ocelot/Configuration/ReRouteOptions.cs create mode 100644 test/Ocelot.UnitTests/Configuration/ReRouteOptionsCreatorTests.cs diff --git a/src/Ocelot/Configuration/Builder/ReRouteOptionsBuilder.cs b/src/Ocelot/Configuration/Builder/ReRouteOptionsBuilder.cs new file mode 100644 index 00000000..b8520a26 --- /dev/null +++ b/src/Ocelot/Configuration/Builder/ReRouteOptionsBuilder.cs @@ -0,0 +1,46 @@ +namespace Ocelot.Configuration.Builder +{ + public class ReRouteOptionsBuilder + { + private bool _isAuthenticated; + private bool _isAuthorised; + private bool _isCached; + private bool _isQoS; + private bool _enableRateLimiting; + + public ReRouteOptionsBuilder WithIsCached(bool isCached) + { + _isCached = isCached; + return this; + } + + public ReRouteOptionsBuilder WithIsAuthenticated(bool isAuthenticated) + { + _isAuthenticated = isAuthenticated; + return this; + } + + public ReRouteOptionsBuilder WithIsAuthorised(bool isAuthorised) + { + _isAuthorised = isAuthorised; + return this; + } + + public ReRouteOptionsBuilder WithIsQos(bool isQoS) + { + _isQoS = isQoS; + return this; + } + + public ReRouteOptionsBuilder WithRateLimiting(bool enableRateLimiting) + { + _enableRateLimiting = enableRateLimiting; + return this; + } + + public ReRouteOptions Build() + { + return new ReRouteOptions(_isAuthenticated, _isAuthorised, _isCached, _isQoS, _enableRateLimiting); + } + } +} \ No newline at end of file diff --git a/src/Ocelot/Configuration/Creator/FileOcelotConfigurationCreator.cs b/src/Ocelot/Configuration/Creator/FileOcelotConfigurationCreator.cs index 4dd873cd..d6caf6a0 100644 --- a/src/Ocelot/Configuration/Creator/FileOcelotConfigurationCreator.cs +++ b/src/Ocelot/Configuration/Creator/FileOcelotConfigurationCreator.cs @@ -33,6 +33,7 @@ namespace Ocelot.Configuration.Creator private IRequestIdKeyCreator _requestIdKeyCreator; private IServiceProviderConfigurationCreator _serviceProviderConfigCreator; private IQoSOptionsCreator _qosOptionsCreator; + private IReRouteOptionsCreator _fileReRouteOptionsCreator; public FileOcelotConfigurationCreator( IOptions options, @@ -47,7 +48,8 @@ namespace Ocelot.Configuration.Creator IUpstreamTemplatePatternCreator upstreamTemplatePatternCreator, IRequestIdKeyCreator requestIdKeyCreator, IServiceProviderConfigurationCreator serviceProviderConfigCreator, - IQoSOptionsCreator qosOptionsCreator + IQoSOptionsCreator qosOptionsCreator, + IReRouteOptionsCreator fileReRouteOptionsCreator ) { _requestIdKeyCreator = requestIdKeyCreator; @@ -63,6 +65,7 @@ namespace Ocelot.Configuration.Creator _claimsToThingCreator = claimsToThingCreator; _serviceProviderConfigCreator = serviceProviderConfigCreator; _qosOptionsCreator = qosOptionsCreator; + _fileReRouteOptionsCreator = fileReRouteOptionsCreator; } public async Task> Create() @@ -108,11 +111,7 @@ namespace Ocelot.Configuration.Creator private async Task SetUpReRoute(FileReRoute fileReRoute, FileGlobalConfiguration globalConfiguration) { - var isAuthenticated = IsAuthenticated(fileReRoute); - - var isAuthorised = IsAuthorised(fileReRoute); - - var isCached = IsCached(fileReRoute); + var fileReRouteOptions = _fileReRouteOptionsCreator.Create(fileReRoute); var requestIdKey = _requestIdKeyCreator.Create(fileReRoute, globalConfiguration); @@ -120,8 +119,6 @@ namespace Ocelot.Configuration.Creator var upstreamTemplatePattern = _upstreamTemplatePatternCreator.Create(fileReRoute); - var isQos = IsQoS(fileReRoute); - var serviceProviderConfiguration = _serviceProviderConfigCreator.Create(fileReRoute, globalConfiguration); var authOptionsForRoute = _authOptionsCreator.Create(fileReRoute); @@ -134,24 +131,22 @@ namespace Ocelot.Configuration.Creator var qosOptions = _qosOptionsCreator.Create(fileReRoute); - var enableRateLimiting = IsEnableRateLimiting(fileReRoute); - - var rateLimitOption = BuildRateLimitOptions(fileReRoute, globalConfiguration, enableRateLimiting); + var rateLimitOption = BuildRateLimitOptions(fileReRoute, globalConfiguration, fileReRouteOptions.EnableRateLimiting); var reRoute = new ReRouteBuilder() .WithDownstreamPathTemplate(fileReRoute.DownstreamPathTemplate) .WithUpstreamPathTemplate(fileReRoute.UpstreamPathTemplate) .WithUpstreamHttpMethod(fileReRoute.UpstreamHttpMethod) .WithUpstreamTemplatePattern(upstreamTemplatePattern) - .WithIsAuthenticated(isAuthenticated) + .WithIsAuthenticated(fileReRouteOptions.IsAuthenticated) .WithAuthenticationOptions(authOptionsForRoute) .WithClaimsToHeaders(claimsToHeaders) .WithClaimsToClaims(claimsToClaims) .WithRouteClaimsRequirement(fileReRoute.RouteClaimsRequirement) - .WithIsAuthorised(isAuthorised) + .WithIsAuthorised(fileReRouteOptions.IsAuthorised) .WithClaimsToQueries(claimsToQueries) .WithRequestIdKey(requestIdKey) - .WithIsCached(isCached) + .WithIsCached(fileReRouteOptions.IsCached) .WithCacheOptions(new CacheOptions(fileReRoute.FileCacheOptions.TtlSeconds)) .WithDownstreamScheme(fileReRoute.DownstreamScheme) .WithLoadBalancer(fileReRoute.LoadBalancer) @@ -159,9 +154,9 @@ namespace Ocelot.Configuration.Creator .WithDownstreamPort(fileReRoute.DownstreamPort) .WithLoadBalancerKey(reRouteKey) .WithServiceProviderConfiguraion(serviceProviderConfiguration) - .WithIsQos(isQos) + .WithIsQos(fileReRouteOptions.IsQos) .WithQosOptions(qosOptions) - .WithEnableRateLimiting(enableRateLimiting) + .WithEnableRateLimiting(fileReRouteOptions.EnableRateLimiting) .WithRateLimitOptions(rateLimitOption) .Build(); @@ -185,31 +180,6 @@ namespace Ocelot.Configuration.Creator return rateLimitOption; } - private static bool IsEnableRateLimiting(FileReRoute fileReRoute) - { - return (fileReRoute.RateLimitOptions != null && fileReRoute.RateLimitOptions.EnableRateLimiting) ? true : false; - } - - private bool IsQoS(FileReRoute fileReRoute) - { - return fileReRoute.QoSOptions?.ExceptionsAllowedBeforeBreaking > 0 && fileReRoute.QoSOptions?.TimeoutValue > 0; - } - - private bool IsAuthenticated(FileReRoute fileReRoute) - { - return !string.IsNullOrEmpty(fileReRoute.AuthenticationOptions?.Provider); - } - - private bool IsAuthorised(FileReRoute fileReRoute) - { - return fileReRoute.RouteClaimsRequirement?.Count > 0; - } - - private bool IsCached(FileReRoute fileReRoute) - { - return fileReRoute.FileCacheOptions.TtlSeconds > 0; - } - private string CreateReRouteKey(FileReRoute fileReRoute) { //note - not sure if this is the correct key, but this is probably the only unique key i can think of given my poor brain @@ -228,10 +198,5 @@ namespace Ocelot.Configuration.Creator var loadBalancer = _qoSProviderFactory.Get(reRoute); _qosProviderHouse.Add(reRoute.ReRouteKey, loadBalancer); } - - private bool IsPlaceHolder(string upstreamTemplate, int i) - { - return upstreamTemplate[i] == '{'; - } } } \ No newline at end of file diff --git a/src/Ocelot/Configuration/Creator/IReRouteOptionsCreator.cs b/src/Ocelot/Configuration/Creator/IReRouteOptionsCreator.cs new file mode 100644 index 00000000..9bc4a07a --- /dev/null +++ b/src/Ocelot/Configuration/Creator/IReRouteOptionsCreator.cs @@ -0,0 +1,9 @@ +using Ocelot.Configuration.File; + +namespace Ocelot.Configuration.Creator +{ + public interface IReRouteOptionsCreator + { + ReRouteOptions Create(FileReRoute fileReRoute); + } +} \ No newline at end of file diff --git a/src/Ocelot/Configuration/Creator/ReRouteOptionsCreator.cs b/src/Ocelot/Configuration/Creator/ReRouteOptionsCreator.cs new file mode 100644 index 00000000..21e46932 --- /dev/null +++ b/src/Ocelot/Configuration/Creator/ReRouteOptionsCreator.cs @@ -0,0 +1,52 @@ +using Ocelot.Configuration.Builder; +using Ocelot.Configuration.File; + +namespace Ocelot.Configuration.Creator +{ + public class ReRouteOptionsCreator : IReRouteOptionsCreator + { + public ReRouteOptions Create(FileReRoute fileReRoute) + { + var isAuthenticated = IsAuthenticated(fileReRoute); + var isAuthorised = IsAuthorised(fileReRoute); + var isCached = IsCached(fileReRoute); + var isQos = IsQoS(fileReRoute); + var enableRateLimiting = IsEnableRateLimiting(fileReRoute); + + var options = new ReRouteOptionsBuilder() + .WithIsAuthenticated(isAuthenticated) + .WithIsAuthorised(isAuthorised) + .WithIsCached(isCached) + .WithIsQos(isQos) + .WithRateLimiting(enableRateLimiting) + .Build(); + + return options; + } + + private static bool IsEnableRateLimiting(FileReRoute fileReRoute) + { + return (fileReRoute.RateLimitOptions != null && fileReRoute.RateLimitOptions.EnableRateLimiting) ? true : false; + } + + private bool IsQoS(FileReRoute fileReRoute) + { + return fileReRoute.QoSOptions?.ExceptionsAllowedBeforeBreaking > 0 && fileReRoute.QoSOptions?.TimeoutValue > 0; + } + + private bool IsAuthenticated(FileReRoute fileReRoute) + { + return !string.IsNullOrEmpty(fileReRoute.AuthenticationOptions?.Provider); + } + + private bool IsAuthorised(FileReRoute fileReRoute) + { + return fileReRoute.RouteClaimsRequirement?.Count > 0; + } + + private bool IsCached(FileReRoute fileReRoute) + { + return fileReRoute.FileCacheOptions.TtlSeconds > 0; + } + } +} \ No newline at end of file diff --git a/src/Ocelot/Configuration/ReRouteOptions.cs b/src/Ocelot/Configuration/ReRouteOptions.cs new file mode 100644 index 00000000..2e42f161 --- /dev/null +++ b/src/Ocelot/Configuration/ReRouteOptions.cs @@ -0,0 +1,20 @@ +namespace Ocelot.Configuration +{ + public class ReRouteOptions + { + public ReRouteOptions(bool isAuthenticated, bool isAuthorised, bool isCached, bool isQos, bool isEnableRateLimiting) + { + IsAuthenticated = isAuthenticated; + IsAuthorised = isAuthorised; + IsCached = isCached; + IsQos = isQos; + EnableRateLimiting = isEnableRateLimiting; + + } + public bool IsAuthenticated { get; private set; } + public bool IsAuthorised { get; private set; } + public bool IsCached { get; private set; } + public bool IsQos { get; private set; } + public bool EnableRateLimiting { get; private set; } + } +} \ No newline at end of file diff --git a/src/Ocelot/DependencyInjection/ServiceCollectionExtensions.cs b/src/Ocelot/DependencyInjection/ServiceCollectionExtensions.cs index 1ebb6d42..d69e7741 100644 --- a/src/Ocelot/DependencyInjection/ServiceCollectionExtensions.cs +++ b/src/Ocelot/DependencyInjection/ServiceCollectionExtensions.cs @@ -66,6 +66,7 @@ namespace Ocelot.DependencyInjection services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); + services.AddSingleton(); var identityServerConfiguration = IdentityServerConfigurationCreator.GetIdentityServerConfiguration(); diff --git a/test/Ocelot.UnitTests/Configuration/FileConfigurationCreatorTests.cs b/test/Ocelot.UnitTests/Configuration/FileConfigurationCreatorTests.cs index bb5866f9..8a64b654 100644 --- a/test/Ocelot.UnitTests/Configuration/FileConfigurationCreatorTests.cs +++ b/test/Ocelot.UnitTests/Configuration/FileConfigurationCreatorTests.cs @@ -36,6 +36,7 @@ namespace Ocelot.UnitTests.Configuration private Mock _requestIdKeyCreator; private Mock _serviceProviderConfigCreator; private Mock _qosOptionsCreator; + private Mock _fileReRouteOptionsCreator; public FileConfigurationCreatorTests() { @@ -54,13 +55,14 @@ namespace Ocelot.UnitTests.Configuration _requestIdKeyCreator = new Mock(); _serviceProviderConfigCreator = new Mock(); _qosOptionsCreator = new Mock(); + _fileReRouteOptionsCreator = new Mock(); _ocelotConfigurationCreator = new FileOcelotConfigurationCreator( _fileConfig.Object, _validator.Object, _logger.Object, _loadBalancerFactory.Object, _loadBalancerHouse.Object, _qosProviderFactory.Object, _qosProviderHouse.Object, _claimsToThingCreator.Object, _authOptionsCreator.Object, _upstreamTemplatePatternCreator.Object, _requestIdKeyCreator.Object, - _serviceProviderConfigCreator.Object, _qosOptionsCreator.Object); + _serviceProviderConfigCreator.Object, _qosOptionsCreator.Object, _fileReRouteOptionsCreator.Object); } [Fact] @@ -72,6 +74,10 @@ namespace Ocelot.UnitTests.Configuration .WithTimeoutValue(1) .Build(); + var serviceOptions = new ReRouteOptionsBuilder() + .WithIsQos(true) + .Build(); + this.Given(x => x.GivenTheConfigIs(new FileConfiguration { ReRoutes = new List @@ -92,16 +98,22 @@ namespace Ocelot.UnitTests.Configuration }, })) .And(x => x.GivenTheConfigIsValid()) + .And(x => x.GivenTheFollowingOptionsAreReturned(serviceOptions)) .And(x => x.GivenTheQosProviderFactoryReturns()) .And(x => x.GivenTheQosOptionsCreatorReturns(expected)) .When(x => x.WhenICreateTheConfig()) .Then(x => x.ThenTheQosOptionsAre(expected)) + .And(x => x.TheQosProviderFactoryIsCalledCorrectly()) + .And(x => x.ThenTheQosProviderHouseIsCalledCorrectly()) .BDDfy(); } [Fact] public void should_create_load_balancer() { + var reRouteOptions = new ReRouteOptionsBuilder() + .Build(); + this.Given(x => x.GivenTheConfigIs(new FileConfiguration { ReRoutes = new List @@ -116,6 +128,7 @@ namespace Ocelot.UnitTests.Configuration }, })) .And(x => x.GivenTheConfigIsValid()) + .And(x => x.GivenTheFollowingOptionsAreReturned(reRouteOptions)) .And(x => x.GivenTheLoadBalancerFactoryReturns()) .When(x => x.WhenICreateTheConfig()) .Then(x => x.TheLoadBalancerFactoryIsCalledCorrectly()) @@ -123,69 +136,46 @@ namespace Ocelot.UnitTests.Configuration .BDDfy(); } - [Fact] - public void should_create_qos_provider() - { - this.Given(x => x.GivenTheConfigIs(new FileConfiguration - { - ReRoutes = new List - { - new FileReRoute - { - DownstreamHost = "127.0.0.1", - UpstreamPathTemplate = "/api/products/{productId}", - DownstreamPathTemplate = "/products/{productId}", - UpstreamHttpMethod = "Get", - QoSOptions = new FileQoSOptions - { - TimeoutValue = 1, - DurationOfBreak = 1, - ExceptionsAllowedBeforeBreaking = 1 - } - } - }, - })) - .And(x => x.GivenTheConfigIsValid()) - .And(x => x.GivenTheQosProviderFactoryReturns()) - .When(x => x.WhenICreateTheConfig()) - .Then(x => x.TheQosProviderFactoryIsCalledCorrectly()) - .And(x => x.ThenTheQosProviderHouseIsCalledCorrectly()) - .BDDfy(); - } - [Fact] public void should_use_downstream_host() { - this.Given(x => x.GivenTheConfigIs(new FileConfiguration + var reRouteOptions = new ReRouteOptionsBuilder() + .Build(); + + this.Given(x => x.GivenTheConfigIs(new FileConfiguration + { + ReRoutes = new List { - ReRoutes = new List + new FileReRoute { - new FileReRoute - { - DownstreamHost = "127.0.0.1", - UpstreamPathTemplate = "/api/products/{productId}", - DownstreamPathTemplate = "/products/{productId}", - UpstreamHttpMethod = "Get", - } - }, + DownstreamHost = "127.0.0.1", + UpstreamPathTemplate = "/api/products/{productId}", + DownstreamPathTemplate = "/products/{productId}", + UpstreamHttpMethod = "Get", + } + }, + })) + .And(x => x.GivenTheConfigIsValid()) + .And(x => x.GivenTheFollowingOptionsAreReturned(reRouteOptions)) + .When(x => x.WhenICreateTheConfig()) + .Then(x => x.ThenTheReRoutesAre(new List + { + new ReRouteBuilder() + .WithDownstreamHost("127.0.0.1") + .WithDownstreamPathTemplate("/products/{productId}") + .WithUpstreamPathTemplate("/api/products/{productId}") + .WithUpstreamHttpMethod("Get") + .Build() })) - .And(x => x.GivenTheConfigIsValid()) - .When(x => x.WhenICreateTheConfig()) - .Then(x => x.ThenTheReRoutesAre(new List - { - new ReRouteBuilder() - .WithDownstreamHost("127.0.0.1") - .WithDownstreamPathTemplate("/products/{productId}") - .WithUpstreamPathTemplate("/api/products/{productId}") - .WithUpstreamHttpMethod("Get") - .Build() - })) - .BDDfy(); + .BDDfy(); } [Fact] public void should_use_downstream_scheme() { + var reRouteOptions = new ReRouteOptionsBuilder() + .Build(); + this.Given(x => x.GivenTheConfigIs(new FileConfiguration { ReRoutes = new List @@ -200,6 +190,7 @@ namespace Ocelot.UnitTests.Configuration }, })) .And(x => x.GivenTheConfigIsValid()) + .And(x => x.GivenTheFollowingOptionsAreReturned(reRouteOptions)) .When(x => x.WhenICreateTheConfig()) .Then(x => x.ThenTheReRoutesAre(new List { @@ -216,6 +207,9 @@ namespace Ocelot.UnitTests.Configuration [Fact] public void should_use_service_discovery_for_downstream_service_host() { + var reRouteOptions = new ReRouteOptionsBuilder() + .Build(); + this.Given(x => x.GivenTheConfigIs(new FileConfiguration { ReRoutes = new List @@ -239,6 +233,7 @@ namespace Ocelot.UnitTests.Configuration } })) .And(x => x.GivenTheConfigIsValid()) + .And(x => x.GivenTheFollowingOptionsAreReturned(reRouteOptions)) .When(x => x.WhenICreateTheConfig()) .Then(x => x.ThenTheReRoutesAre(new List { @@ -260,6 +255,9 @@ namespace Ocelot.UnitTests.Configuration [Fact] public void should_not_use_service_discovery_for_downstream_host_url_when_no_service_name() { + var reRouteOptions = new ReRouteOptionsBuilder() + .Build(); + this.Given(x => x.GivenTheConfigIs(new FileConfiguration { ReRoutes = new List @@ -274,6 +272,7 @@ namespace Ocelot.UnitTests.Configuration } })) .And(x => x.GivenTheConfigIsValid()) + .And(x => x.GivenTheFollowingOptionsAreReturned(reRouteOptions)) .When(x => x.WhenICreateTheConfig()) .Then(x => x.ThenTheReRoutesAre(new List { @@ -292,6 +291,9 @@ namespace Ocelot.UnitTests.Configuration [Fact] public void should_call_template_pattern_creator_correctly() { + var reRouteOptions = new ReRouteOptionsBuilder() + .Build(); + this.Given(x => x.GivenTheConfigIs(new FileConfiguration { ReRoutes = new List @@ -306,6 +308,7 @@ namespace Ocelot.UnitTests.Configuration } })) .And(x => x.GivenTheConfigIsValid()) + .And(x => x.GivenTheFollowingOptionsAreReturned(reRouteOptions)) .And(x => x.GivenTheUpstreamTemplatePatternCreatorReturns("(?i)/api/products/.*/$")) .When(x => x.WhenICreateTheConfig()) .Then(x => x.ThenTheReRoutesAre(new List @@ -323,6 +326,9 @@ namespace Ocelot.UnitTests.Configuration [Fact] public void should_call_request_id_creator() { + var reRouteOptions = new ReRouteOptionsBuilder() + .Build(); + this.Given(x => x.GivenTheConfigIs(new FileConfiguration { ReRoutes = new List @@ -341,6 +347,7 @@ namespace Ocelot.UnitTests.Configuration } })) .And(x => x.GivenTheConfigIsValid()) + .And(x => x.GivenTheFollowingOptionsAreReturned(reRouteOptions)) .And(x => x.GivenTheRequestIdCreatorReturns("blahhhh")) .When(x => x.WhenICreateTheConfig()) .Then(x => x.ThenTheReRoutesAre(new List @@ -359,6 +366,10 @@ namespace Ocelot.UnitTests.Configuration [Fact] public void should_create_with_headers_to_extract() { + var reRouteOptions = new ReRouteOptionsBuilder() + .WithIsAuthenticated(true) + .Build(); + var authenticationOptions = new AuthenticationOptionsBuilder() .WithProvider("IdentityServer") .WithProviderRootUrl("http://localhost:51888") @@ -410,6 +421,7 @@ namespace Ocelot.UnitTests.Configuration })) .And(x => x.GivenTheConfigIsValid()) .And(x => x.GivenTheAuthOptionsCreatorReturns(authenticationOptions)) + .And(x => x.GivenTheFollowingOptionsAreReturned(reRouteOptions)) .And(x => x.GivenTheClaimsToThingCreatorReturns(new List{new ClaimToThing("CustomerId", "CustomerId", "", 0)})) .And(x => x.GivenTheLoadBalancerFactoryReturns()) .When(x => x.WhenICreateTheConfig()) @@ -424,6 +436,10 @@ namespace Ocelot.UnitTests.Configuration [Fact] public void should_create_with_authentication_properties() { + var reRouteOptions = new ReRouteOptionsBuilder() + .WithIsAuthenticated(true) + .Build(); + var authenticationOptions = new AuthenticationOptionsBuilder() .WithProvider("IdentityServer") .WithProviderRootUrl("http://localhost:51888") @@ -466,6 +482,7 @@ namespace Ocelot.UnitTests.Configuration } })) .And(x => x.GivenTheConfigIsValid()) + .And(x => x.GivenTheFollowingOptionsAreReturned(reRouteOptions)) .And(x => x.GivenTheAuthOptionsCreatorReturns(authenticationOptions)) .And(x => x.GivenTheLoadBalancerFactoryReturns()) .When(x => x.WhenICreateTheConfig()) @@ -475,6 +492,14 @@ namespace Ocelot.UnitTests.Configuration .BDDfy(); } + private void GivenTheFollowingOptionsAreReturned(ReRouteOptions fileReRouteOptions) + { + _fileReRouteOptionsCreator + .Setup(x => x.Create(It.IsAny())) + .Returns(fileReRouteOptions); + } + + private void GivenTheConfigIsValid() { _validator diff --git a/test/Ocelot.UnitTests/Configuration/ReRouteOptionsCreatorTests.cs b/test/Ocelot.UnitTests/Configuration/ReRouteOptionsCreatorTests.cs new file mode 100644 index 00000000..79872ea1 --- /dev/null +++ b/test/Ocelot.UnitTests/Configuration/ReRouteOptionsCreatorTests.cs @@ -0,0 +1,84 @@ +using System.Collections.Generic; +using Ocelot.Configuration; +using Ocelot.Configuration.Builder; +using Ocelot.Configuration.Creator; +using Ocelot.Configuration.File; +using Shouldly; +using TestStack.BDDfy; +using Xunit; + +namespace Ocelot.UnitTests.Configuration +{ + public class ReRouteOptionsCreatorTests + { + private ReRouteOptionsCreator _creator; + private FileReRoute _reRoute; + private ReRouteOptions _result; + + public ReRouteOptionsCreatorTests() + { + _creator = new ReRouteOptionsCreator(); + } + + [Fact] + public void should_create_re_route_options() + { + var reRoute = new FileReRoute + { + RateLimitOptions = new FileRateLimitRule + { + EnableRateLimiting = true + }, + QoSOptions = new FileQoSOptions + { + ExceptionsAllowedBeforeBreaking = 1, + TimeoutValue = 1 + }, + AuthenticationOptions = new FileAuthenticationOptions + { + Provider = "IdentityServer" + }, + RouteClaimsRequirement = new Dictionary() + { + {"",""} + }, + FileCacheOptions = new FileCacheOptions + { + TtlSeconds = 1 + } + }; + + var expected = new ReRouteOptionsBuilder() + .WithIsAuthenticated(true) + .WithIsAuthorised(true) + .WithIsCached(true) + .WithIsQos(true) + .WithRateLimiting(true) + .Build(); + + this.Given(x => x.GivenTheFollowing(reRoute)) + .When(x => x.WhenICreate()) + .Then(x => x.ThenTheFollowingIsReturned(expected)) + .BDDfy(); + } + + private void GivenTheFollowing(FileReRoute reRoute) + { + _reRoute = reRoute; + } + + private void WhenICreate() + { + _result = _creator.Create(_reRoute); + } + + private void ThenTheFollowingIsReturned(ReRouteOptions expected) + { + _result.IsAuthenticated.ShouldBe(expected.IsAuthenticated); + _result.IsAuthorised.ShouldBe(expected.IsAuthorised); + _result.IsQos.ShouldBe(expected.IsQos); + _result.IsCached.ShouldBe(expected.IsCached); + _result.EnableRateLimiting.ShouldBe(expected.EnableRateLimiting); + } + } +} \ No newline at end of file From 558a0dfdab1027989889a5b36afcde1efafac7ca Mon Sep 17 00:00:00 2001 From: TomPallister Date: Sun, 5 Mar 2017 16:56:41 +0000 Subject: [PATCH 9/9] finished refactoring config cretor --- .../Builder/RateLimitOptionsBuilder.cs | 71 ++++++++++++ .../Creator/FileOcelotConfigurationCreator.cs | 22 +--- .../Creator/IRateLimitOptionsCreator.cs | 9 ++ .../Creator/RateLimitOptionsCreator.cs | 32 ++++++ .../ServiceCollectionExtensions.cs | 1 + .../FileConfigurationCreatorTests.cs | 38 +++++- .../RateLimitOptionsCreatorTests.cs | 108 ++++++++++++++++++ 7 files changed, 261 insertions(+), 20 deletions(-) create mode 100644 src/Ocelot/Configuration/Builder/RateLimitOptionsBuilder.cs create mode 100644 src/Ocelot/Configuration/Creator/IRateLimitOptionsCreator.cs create mode 100644 src/Ocelot/Configuration/Creator/RateLimitOptionsCreator.cs create mode 100644 test/Ocelot.UnitTests/Configuration/RateLimitOptionsCreatorTests.cs diff --git a/src/Ocelot/Configuration/Builder/RateLimitOptionsBuilder.cs b/src/Ocelot/Configuration/Builder/RateLimitOptionsBuilder.cs new file mode 100644 index 00000000..532d990b --- /dev/null +++ b/src/Ocelot/Configuration/Builder/RateLimitOptionsBuilder.cs @@ -0,0 +1,71 @@ +using System.Collections.Generic; + +namespace Ocelot.Configuration.Builder +{ + public class RateLimitOptionsBuilder + { + private bool _enableRateLimiting; + private string _clientIdHeader; + private List _clientWhitelist; + private bool _disableRateLimitHeaders; + private string _quotaExceededMessage; + private string _rateLimitCounterPrefix; + private RateLimitRule _rateLimitRule; + private int _httpStatusCode; + + public RateLimitOptionsBuilder WithEnableRateLimiting(bool enableRateLimiting) + { + _enableRateLimiting = enableRateLimiting; + return this; + } + + public RateLimitOptionsBuilder WithClientIdHeader(string clientIdheader) + { + _clientIdHeader = clientIdheader; + return this; + } + + public RateLimitOptionsBuilder WithClientWhiteList(List clientWhitelist) + { + _clientWhitelist = clientWhitelist; + return this; + } + + public RateLimitOptionsBuilder WithDisableRateLimitHeaders(bool disableRateLimitHeaders) + { + _disableRateLimitHeaders = disableRateLimitHeaders; + return this; + } + + public RateLimitOptionsBuilder WithQuotaExceededMessage(string quotaExceededMessage) + { + _quotaExceededMessage = quotaExceededMessage; + return this; + } + + public RateLimitOptionsBuilder WithRateLimitCounterPrefix(string rateLimitCounterPrefix) + { + _rateLimitCounterPrefix = rateLimitCounterPrefix; + return this; + } + + public RateLimitOptionsBuilder WithRateLimitRule(RateLimitRule rateLimitRule) + { + _rateLimitRule = rateLimitRule; + return this; + } + + public RateLimitOptionsBuilder WithHttpStatusCode(int httpStatusCode) + { + _httpStatusCode = httpStatusCode; + return this; + } + + public RateLimitOptions Build() + { + return new RateLimitOptions(_enableRateLimiting, _clientIdHeader, _clientWhitelist, + _disableRateLimitHeaders, _quotaExceededMessage, _rateLimitCounterPrefix, + _rateLimitRule, _httpStatusCode); + } + } +} diff --git a/src/Ocelot/Configuration/Creator/FileOcelotConfigurationCreator.cs b/src/Ocelot/Configuration/Creator/FileOcelotConfigurationCreator.cs index d6caf6a0..7147b40a 100644 --- a/src/Ocelot/Configuration/Creator/FileOcelotConfigurationCreator.cs +++ b/src/Ocelot/Configuration/Creator/FileOcelotConfigurationCreator.cs @@ -34,6 +34,7 @@ namespace Ocelot.Configuration.Creator private IServiceProviderConfigurationCreator _serviceProviderConfigCreator; private IQoSOptionsCreator _qosOptionsCreator; private IReRouteOptionsCreator _fileReRouteOptionsCreator; + private IRateLimitOptionsCreator _rateLimitOptionsCreator; public FileOcelotConfigurationCreator( IOptions options, @@ -49,9 +50,11 @@ namespace Ocelot.Configuration.Creator IRequestIdKeyCreator requestIdKeyCreator, IServiceProviderConfigurationCreator serviceProviderConfigCreator, IQoSOptionsCreator qosOptionsCreator, - IReRouteOptionsCreator fileReRouteOptionsCreator + IReRouteOptionsCreator fileReRouteOptionsCreator, + IRateLimitOptionsCreator rateLimitOptionsCreator ) { + _rateLimitOptionsCreator = rateLimitOptionsCreator; _requestIdKeyCreator = requestIdKeyCreator; _upstreamTemplatePatternCreator = upstreamTemplatePatternCreator; _authOptionsCreator = authOptionsCreator; @@ -131,7 +134,7 @@ namespace Ocelot.Configuration.Creator var qosOptions = _qosOptionsCreator.Create(fileReRoute); - var rateLimitOption = BuildRateLimitOptions(fileReRoute, globalConfiguration, fileReRouteOptions.EnableRateLimiting); + var rateLimitOption = _rateLimitOptionsCreator.Create(fileReRoute, globalConfiguration, fileReRouteOptions.EnableRateLimiting); var reRoute = new ReRouteBuilder() .WithDownstreamPathTemplate(fileReRoute.DownstreamPathTemplate) @@ -165,21 +168,6 @@ namespace Ocelot.Configuration.Creator return reRoute; } - private static RateLimitOptions BuildRateLimitOptions(FileReRoute fileReRoute, FileGlobalConfiguration globalConfiguration, bool enableRateLimiting) - { - RateLimitOptions rateLimitOption = null; - if (enableRateLimiting) - { - rateLimitOption = new RateLimitOptions(enableRateLimiting, globalConfiguration.RateLimitOptions.ClientIdHeader, - fileReRoute.RateLimitOptions.ClientWhitelist, globalConfiguration.RateLimitOptions.DisableRateLimitHeaders, - globalConfiguration.RateLimitOptions.QuotaExceededMessage, globalConfiguration.RateLimitOptions.RateLimitCounterPrefix, - new RateLimitRule(fileReRoute.RateLimitOptions.Period, TimeSpan.FromSeconds(fileReRoute.RateLimitOptions.PeriodTimespan), fileReRoute.RateLimitOptions.Limit) - , globalConfiguration.RateLimitOptions.HttpStatusCode); - } - - return rateLimitOption; - } - private string CreateReRouteKey(FileReRoute fileReRoute) { //note - not sure if this is the correct key, but this is probably the only unique key i can think of given my poor brain diff --git a/src/Ocelot/Configuration/Creator/IRateLimitOptionsCreator.cs b/src/Ocelot/Configuration/Creator/IRateLimitOptionsCreator.cs new file mode 100644 index 00000000..42f03a96 --- /dev/null +++ b/src/Ocelot/Configuration/Creator/IRateLimitOptionsCreator.cs @@ -0,0 +1,9 @@ +using Ocelot.Configuration.File; + +namespace Ocelot.Configuration.Creator +{ + public interface IRateLimitOptionsCreator + { + RateLimitOptions Create(FileReRoute fileReRoute, FileGlobalConfiguration globalConfiguration, bool enableRateLimiting); + } +} \ No newline at end of file diff --git a/src/Ocelot/Configuration/Creator/RateLimitOptionsCreator.cs b/src/Ocelot/Configuration/Creator/RateLimitOptionsCreator.cs new file mode 100644 index 00000000..68bf2a33 --- /dev/null +++ b/src/Ocelot/Configuration/Creator/RateLimitOptionsCreator.cs @@ -0,0 +1,32 @@ +using System; +using Ocelot.Configuration.Builder; +using Ocelot.Configuration.File; + +namespace Ocelot.Configuration.Creator +{ + public class RateLimitOptionsCreator : IRateLimitOptionsCreator + { + public RateLimitOptions Create(FileReRoute fileReRoute, FileGlobalConfiguration globalConfiguration, bool enableRateLimiting) + { + RateLimitOptions rateLimitOption = null; + + if (enableRateLimiting) + { + rateLimitOption = new RateLimitOptionsBuilder() + .WithClientIdHeader(globalConfiguration.RateLimitOptions.ClientIdHeader) + .WithClientWhiteList(fileReRoute.RateLimitOptions.ClientWhitelist) + .WithDisableRateLimitHeaders(globalConfiguration.RateLimitOptions.DisableRateLimitHeaders) + .WithEnableRateLimiting(fileReRoute.RateLimitOptions.EnableRateLimiting) + .WithHttpStatusCode(globalConfiguration.RateLimitOptions.HttpStatusCode) + .WithQuotaExceededMessage(globalConfiguration.RateLimitOptions.QuotaExceededMessage) + .WithRateLimitCounterPrefix(globalConfiguration.RateLimitOptions.RateLimitCounterPrefix) + .WithRateLimitRule(new RateLimitRule(fileReRoute.RateLimitOptions.Period, + TimeSpan.FromSeconds(fileReRoute.RateLimitOptions.PeriodTimespan), + fileReRoute.RateLimitOptions.Limit)) + .Build(); + } + + return rateLimitOption; + } + } +} diff --git a/src/Ocelot/DependencyInjection/ServiceCollectionExtensions.cs b/src/Ocelot/DependencyInjection/ServiceCollectionExtensions.cs index d69e7741..e99b2490 100644 --- a/src/Ocelot/DependencyInjection/ServiceCollectionExtensions.cs +++ b/src/Ocelot/DependencyInjection/ServiceCollectionExtensions.cs @@ -67,6 +67,7 @@ namespace Ocelot.DependencyInjection services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); + services.AddSingleton(); var identityServerConfiguration = IdentityServerConfigurationCreator.GetIdentityServerConfiguration(); diff --git a/test/Ocelot.UnitTests/Configuration/FileConfigurationCreatorTests.cs b/test/Ocelot.UnitTests/Configuration/FileConfigurationCreatorTests.cs index 8a64b654..e3293a90 100644 --- a/test/Ocelot.UnitTests/Configuration/FileConfigurationCreatorTests.cs +++ b/test/Ocelot.UnitTests/Configuration/FileConfigurationCreatorTests.cs @@ -37,6 +37,7 @@ namespace Ocelot.UnitTests.Configuration private Mock _serviceProviderConfigCreator; private Mock _qosOptionsCreator; private Mock _fileReRouteOptionsCreator; + private Mock _rateLimitOptions; public FileConfigurationCreatorTests() { @@ -56,13 +57,41 @@ namespace Ocelot.UnitTests.Configuration _serviceProviderConfigCreator = new Mock(); _qosOptionsCreator = new Mock(); _fileReRouteOptionsCreator = new Mock(); + _rateLimitOptions = new Mock(); _ocelotConfigurationCreator = new FileOcelotConfigurationCreator( _fileConfig.Object, _validator.Object, _logger.Object, _loadBalancerFactory.Object, _loadBalancerHouse.Object, _qosProviderFactory.Object, _qosProviderHouse.Object, _claimsToThingCreator.Object, _authOptionsCreator.Object, _upstreamTemplatePatternCreator.Object, _requestIdKeyCreator.Object, - _serviceProviderConfigCreator.Object, _qosOptionsCreator.Object, _fileReRouteOptionsCreator.Object); + _serviceProviderConfigCreator.Object, _qosOptionsCreator.Object, _fileReRouteOptionsCreator.Object, + _rateLimitOptions.Object); + } + + [Fact] + public void should_call_rate_limit_options_creator() + { + var reRouteOptions = new ReRouteOptionsBuilder() + .Build(); + + this.Given(x => x.GivenTheConfigIs(new FileConfiguration + { + ReRoutes = new List + { + new FileReRoute + { + DownstreamHost = "127.0.0.1", + UpstreamPathTemplate = "/api/products/{productId}", + DownstreamPathTemplate = "/products/{productId}", + UpstreamHttpMethod = "Get", + } + }, + })) + .And(x => x.GivenTheConfigIsValid()) + .And(x => x.GivenTheFollowingOptionsAreReturned(reRouteOptions)) + .When(x => x.WhenICreateTheConfig()) + .Then(x => x.ThenTheRateLimitOptionsCreatorIsCalledCorrectly()) + .BDDfy(); } [Fact] @@ -431,8 +460,6 @@ namespace Ocelot.UnitTests.Configuration .BDDfy(); } - - [Fact] public void should_create_with_authentication_properties() { @@ -499,6 +526,11 @@ namespace Ocelot.UnitTests.Configuration .Returns(fileReRouteOptions); } + private void ThenTheRateLimitOptionsCreatorIsCalledCorrectly() + { + _rateLimitOptions + .Verify(x => x.Create(It.IsAny(), It.IsAny(), It.IsAny()), Times.Once); + } private void GivenTheConfigIsValid() { diff --git a/test/Ocelot.UnitTests/Configuration/RateLimitOptionsCreatorTests.cs b/test/Ocelot.UnitTests/Configuration/RateLimitOptionsCreatorTests.cs new file mode 100644 index 00000000..027de7d4 --- /dev/null +++ b/test/Ocelot.UnitTests/Configuration/RateLimitOptionsCreatorTests.cs @@ -0,0 +1,108 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; +using Ocelot.Configuration; +using Ocelot.Configuration.Builder; +using Ocelot.Configuration.Creator; +using Ocelot.Configuration.File; +using Shouldly; +using TestStack.BDDfy; +using Xunit; + +namespace Ocelot.UnitTests.Configuration +{ + public class RateLimitOptionsCreatorTests + { + private FileReRoute _fileReRoute; + private FileGlobalConfiguration _fileGlobalConfig; + private bool _enabled; + private RateLimitOptionsCreator _creator; + private RateLimitOptions _result; + + public RateLimitOptionsCreatorTests() + { + _creator = new RateLimitOptionsCreator(); + } + + [Fact] + public void should_create_rate_limit_options() + { + var fileReRoute = new FileReRoute + { + RateLimitOptions = new FileRateLimitRule + { + ClientWhitelist = new List(), + Period = "Period", + Limit = 1, + PeriodTimespan = 1, + EnableRateLimiting = true + } + }; + var fileGlobalConfig = new FileGlobalConfiguration + { + RateLimitOptions = new FileRateLimitOptions + { + ClientIdHeader = "ClientIdHeader", + DisableRateLimitHeaders = true, + QuotaExceededMessage = "QuotaExceededMessage", + RateLimitCounterPrefix = "RateLimitCounterPrefix", + HttpStatusCode = 200 + } + }; + var expected = new RateLimitOptionsBuilder() + .WithClientIdHeader("ClientIdHeader") + .WithClientWhiteList(fileReRoute.RateLimitOptions.ClientWhitelist) + .WithDisableRateLimitHeaders(true) + .WithEnableRateLimiting(true) + .WithHttpStatusCode(200) + .WithQuotaExceededMessage("QuotaExceededMessage") + .WithRateLimitCounterPrefix("RateLimitCounterPrefix") + .WithRateLimitRule(new RateLimitRule(fileReRoute.RateLimitOptions.Period, + TimeSpan.FromSeconds(fileReRoute.RateLimitOptions.PeriodTimespan), + fileReRoute.RateLimitOptions.Limit)) + .Build(); + + this.Given(x => x.GivenTheFollowingFileReRoute(fileReRoute)) + .And(x => x.GivenTheFollowingFileGlobalConfig(fileGlobalConfig)) + .And(x => x.GivenRateLimitingIsEnabled()) + .When(x => x.WhenICreate()) + .Then(x => x.ThenTheFollowingIsReturned(expected)) + .BDDfy(); + } + + private void GivenTheFollowingFileReRoute(FileReRoute fileReRoute) + { + _fileReRoute = fileReRoute; + } + + private void GivenTheFollowingFileGlobalConfig(FileGlobalConfiguration fileGlobalConfig) + { + _fileGlobalConfig = fileGlobalConfig; + } + + private void GivenRateLimitingIsEnabled() + { + _enabled = true; + } + + private void WhenICreate() + { + _result = _creator.Create(_fileReRoute, _fileGlobalConfig, _enabled); + } + + private void ThenTheFollowingIsReturned(RateLimitOptions expected) + { + _result.ClientIdHeader.ShouldBe(expected.ClientIdHeader); + _result.ClientWhitelist.ShouldBe(expected.ClientWhitelist); + _result.DisableRateLimitHeaders.ShouldBe(expected.DisableRateLimitHeaders); + _result.EnableRateLimiting.ShouldBe(expected.EnableRateLimiting); + _result.HttpStatusCode.ShouldBe(expected.HttpStatusCode); + _result.QuotaExceededMessage.ShouldBe(expected.QuotaExceededMessage); + _result.RateLimitCounterPrefix.ShouldBe(expected.RateLimitCounterPrefix); + _result.RateLimitRule.Limit.ShouldBe(expected.RateLimitRule.Limit); + _result.RateLimitRule.Period.ShouldBe(expected.RateLimitRule.Period); + _result.RateLimitRule.PeriodTimespan.Ticks.ShouldBe(expected.RateLimitRule.PeriodTimespan.Ticks); + } + } +}