From 88e51971c5fda0afebe7bc5d5e13de047b5174cf Mon Sep 17 00:00:00 2001 From: Tom Gardham-Pallister Date: Fri, 10 Nov 2017 18:07:08 +0000 Subject: [PATCH] only get config once in a request...could make this its own middleware one day? --- .../Finder/DownstreamRouteFinder.cs | 11 ++++----- .../Finder/IDownstreamRouteFinder.cs | 3 ++- .../DownstreamRouteFinderMiddleware.cs | 19 +++++++++++++-- .../Middleware/LoadBalancingMiddleware.cs | 9 ++----- src/Ocelot/Middleware/OcelotMiddleware.cs | 8 +++++++ .../DownstreamRouteFinderMiddlewareTests.cs | 24 +++++++++++++++++-- .../DownstreamRouteFinderTests.cs | 11 ++++----- .../LoadBalancerMiddlewareTests.cs | 7 ++---- 8 files changed, 61 insertions(+), 31 deletions(-) diff --git a/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs b/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs index d6509b23..e39fffc6 100644 --- a/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs +++ b/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; +using Ocelot.Configuration; using Ocelot.Configuration.Provider; using Ocelot.DownstreamRouteFinder.UrlMatcher; using Ocelot.Errors; @@ -12,24 +13,20 @@ namespace Ocelot.DownstreamRouteFinder.Finder { public class DownstreamRouteFinder : IDownstreamRouteFinder { - private readonly IOcelotConfigurationProvider _configProvider; private readonly IUrlPathToUrlTemplateMatcher _urlMatcher; private readonly IUrlPathPlaceholderNameAndValueFinder _urlPathPlaceholderNameAndValueFinder; - public DownstreamRouteFinder(IOcelotConfigurationProvider configProvider, IUrlPathToUrlTemplateMatcher urlMatcher, IUrlPathPlaceholderNameAndValueFinder urlPathPlaceholderNameAndValueFinder) + public DownstreamRouteFinder(IUrlPathToUrlTemplateMatcher urlMatcher, IUrlPathPlaceholderNameAndValueFinder urlPathPlaceholderNameAndValueFinder) { - _configProvider = configProvider; _urlMatcher = urlMatcher; _urlPathPlaceholderNameAndValueFinder = urlPathPlaceholderNameAndValueFinder; } - public async Task> FindDownstreamRoute(string upstreamUrlPath, string upstreamHttpMethod) + public Response FindDownstreamRoute(string upstreamUrlPath, string upstreamHttpMethod, IOcelotConfiguration configuration) { upstreamUrlPath = upstreamUrlPath.SetLastCharacterAs('/'); - var configuration = await _configProvider.Get(); - - var applicableReRoutes = configuration.Data.ReRoutes.Where(r => r.UpstreamHttpMethod.Count == 0 || r.UpstreamHttpMethod.Select(x => x.Method.ToLower()).Contains(upstreamHttpMethod.ToLower())); + var applicableReRoutes = configuration.ReRoutes.Where(r => r.UpstreamHttpMethod.Count == 0 || r.UpstreamHttpMethod.Select(x => x.Method.ToLower()).Contains(upstreamHttpMethod.ToLower())); foreach (var reRoute in applicableReRoutes) { diff --git a/src/Ocelot/DownstreamRouteFinder/Finder/IDownstreamRouteFinder.cs b/src/Ocelot/DownstreamRouteFinder/Finder/IDownstreamRouteFinder.cs index 7ae3ff79..eda3acac 100644 --- a/src/Ocelot/DownstreamRouteFinder/Finder/IDownstreamRouteFinder.cs +++ b/src/Ocelot/DownstreamRouteFinder/Finder/IDownstreamRouteFinder.cs @@ -1,10 +1,11 @@ using System.Threading.Tasks; +using Ocelot.Configuration; using Ocelot.Responses; namespace Ocelot.DownstreamRouteFinder.Finder { public interface IDownstreamRouteFinder { - Task> FindDownstreamRoute(string upstreamUrlPath, string upstreamHttpMethod); + Response FindDownstreamRoute(string upstreamUrlPath, string upstreamHttpMethod, IOcelotConfiguration configuration); } } diff --git a/src/Ocelot/DownstreamRouteFinder/Middleware/DownstreamRouteFinderMiddleware.cs b/src/Ocelot/DownstreamRouteFinder/Middleware/DownstreamRouteFinderMiddleware.cs index f421d6d8..b5351e2a 100644 --- a/src/Ocelot/DownstreamRouteFinder/Middleware/DownstreamRouteFinderMiddleware.cs +++ b/src/Ocelot/DownstreamRouteFinder/Middleware/DownstreamRouteFinderMiddleware.cs @@ -2,6 +2,7 @@ using System.Linq; using System.Security.Cryptography.X509Certificates; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; +using Ocelot.Configuration.Provider; using Ocelot.DownstreamRouteFinder.Finder; using Ocelot.Infrastructure.Extensions; using Ocelot.Infrastructure.RequestData; @@ -16,13 +17,17 @@ namespace Ocelot.DownstreamRouteFinder.Middleware private readonly RequestDelegate _next; private readonly IDownstreamRouteFinder _downstreamRouteFinder; private readonly IOcelotLogger _logger; + private readonly IOcelotConfigurationProvider _configProvider; + public DownstreamRouteFinderMiddleware(RequestDelegate next, IOcelotLoggerFactory loggerFactory, IDownstreamRouteFinder downstreamRouteFinder, - IRequestScopedDataRepository requestScopedDataRepository) + IRequestScopedDataRepository requestScopedDataRepository, + IOcelotConfigurationProvider configProvider) :base(requestScopedDataRepository) { + _configProvider = configProvider; _next = next; _downstreamRouteFinder = downstreamRouteFinder; _logger = loggerFactory.CreateLogger(); @@ -32,9 +37,19 @@ namespace Ocelot.DownstreamRouteFinder.Middleware { var upstreamUrlPath = context.Request.Path.ToString(); + //todo make this getting config its own middleware one day? + var configuration = await _configProvider.Get(); + if(configuration.IsError) + { + _logger.LogError($"{MiddlewareName} setting pipeline errors. IOcelotConfigurationProvider returned {configuration.Errors.ToErrorString()}"); + SetPipelineError(configuration.Errors); + } + + SetServiceProviderConfigurationForThisRequest(configuration.Data.ServiceProviderConfiguration); + _logger.LogDebug("upstream url path is {upstreamUrlPath}", upstreamUrlPath); - var downstreamRoute = await _downstreamRouteFinder.FindDownstreamRoute(upstreamUrlPath, context.Request.Method); + var downstreamRoute = _downstreamRouteFinder.FindDownstreamRoute(upstreamUrlPath, context.Request.Method, configuration.Data); if (downstreamRoute.IsError) { diff --git a/src/Ocelot/LoadBalancer/Middleware/LoadBalancingMiddleware.cs b/src/Ocelot/LoadBalancer/Middleware/LoadBalancingMiddleware.cs index 079f3584..dc3b1a8f 100644 --- a/src/Ocelot/LoadBalancer/Middleware/LoadBalancingMiddleware.cs +++ b/src/Ocelot/LoadBalancer/Middleware/LoadBalancingMiddleware.cs @@ -12,7 +12,6 @@ namespace Ocelot.LoadBalancer.Middleware { public class LoadBalancingMiddleware : OcelotMiddleware { - private readonly IOcelotConfigurationProvider _configProvider; private readonly RequestDelegate _next; private readonly IOcelotLogger _logger; private readonly ILoadBalancerHouse _loadBalancerHouse; @@ -20,11 +19,9 @@ namespace Ocelot.LoadBalancer.Middleware public LoadBalancingMiddleware(RequestDelegate next, IOcelotLoggerFactory loggerFactory, IRequestScopedDataRepository requestScopedDataRepository, - ILoadBalancerHouse loadBalancerHouse, - IOcelotConfigurationProvider configProvider) + ILoadBalancerHouse loadBalancerHouse) : base(requestScopedDataRepository) { - _configProvider = configProvider; _next = next; _logger = loggerFactory.CreateLogger(); _loadBalancerHouse = loadBalancerHouse; @@ -32,9 +29,7 @@ namespace Ocelot.LoadBalancer.Middleware public async Task Invoke(HttpContext context) { - var configuration = await _configProvider.Get(); - - var loadBalancer = await _loadBalancerHouse.Get(DownstreamRoute.ReRoute, configuration.Data.ServiceProviderConfiguration); + var loadBalancer = await _loadBalancerHouse.Get(DownstreamRoute.ReRoute, ServiceProviderConfiguration); if(loadBalancer.IsError) { _logger.LogDebug("there was an error retriving the loadbalancer, setting pipeline error"); diff --git a/src/Ocelot/Middleware/OcelotMiddleware.cs b/src/Ocelot/Middleware/OcelotMiddleware.cs index 0060cbb3..50d5baa5 100644 --- a/src/Ocelot/Middleware/OcelotMiddleware.cs +++ b/src/Ocelot/Middleware/OcelotMiddleware.cs @@ -1,5 +1,6 @@ using System.Collections.Generic; using System.Net.Http; +using Ocelot.Configuration; using Ocelot.DownstreamRouteFinder; using Ocelot.Errors; using Ocelot.Infrastructure.RequestData; @@ -30,11 +31,18 @@ namespace Ocelot.Middleware public HttpResponseMessage HttpResponseMessage => _requestScopedDataRepository.Get("HttpResponseMessage").Data; + public ServiceProviderConfiguration ServiceProviderConfiguration => _requestScopedDataRepository.Get("ServiceProviderConfiguration").Data; + public void SetDownstreamRouteForThisRequest(DownstreamRoute downstreamRoute) { _requestScopedDataRepository.Add("DownstreamRoute", downstreamRoute); } + public void SetServiceProviderConfigurationForThisRequest(ServiceProviderConfiguration serviceProviderConfiguration) + { + _requestScopedDataRepository.Add("ServiceProviderConfiguration", serviceProviderConfiguration); + } + public void SetUpstreamRequestForThisRequest(Request.Request request) { _requestScopedDataRepository.Add("Request", request); diff --git a/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderMiddlewareTests.cs b/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderMiddlewareTests.cs index fb312e21..9b241fbe 100644 --- a/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderMiddlewareTests.cs +++ b/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderMiddlewareTests.cs @@ -4,7 +4,9 @@ using Microsoft.AspNetCore.Builder; using Microsoft.Extensions.DependencyInjection; using Moq; + using Ocelot.Configuration; using Ocelot.Configuration.Builder; + using Ocelot.Configuration.Provider; using Ocelot.DownstreamRouteFinder; using Ocelot.DownstreamRouteFinder.Finder; using Ocelot.DownstreamRouteFinder.Middleware; @@ -17,10 +19,13 @@ public class DownstreamRouteFinderMiddlewareTests : ServerHostedMiddlewareTest { private readonly Mock _downstreamRouteFinder; + private readonly Mock _provider; private Response _downstreamRoute; + private IOcelotConfiguration _config; public DownstreamRouteFinderMiddlewareTests() { + _provider = new Mock(); _downstreamRouteFinder = new Mock(); GivenTheTestServerIsConfigured(); @@ -29,6 +34,8 @@ [Fact] public void should_call_scoped_data_repository_correctly() { + var config = new OcelotConfiguration(null, null, new ServiceProviderConfigurationBuilder().Build()); + this.Given(x => x.GivenTheDownStreamRouteFinderReturns( new DownstreamRoute( new List(), @@ -36,16 +43,26 @@ .WithDownstreamPathTemplate("any old string") .WithUpstreamHttpMethod(new List { "Get" }) .Build()))) + .And(x => GivenTheFollowingConfig(config)) .When(x => x.WhenICallTheMiddleware()) .Then(x => x.ThenTheScopedDataRepositoryIsCalledCorrectly()) .BDDfy(); } + private void GivenTheFollowingConfig(IOcelotConfiguration config) + { + _config = config; + _provider + .Setup(x => x.Get()) + .ReturnsAsync(new OkResponse(_config)); + } + protected override void GivenTheTestServerServicesAreConfigured(IServiceCollection services) { services.AddSingleton(); services.AddLogging(); services.AddSingleton(_downstreamRouteFinder.Object); + services.AddSingleton(_provider.Object); services.AddSingleton(ScopedRepository.Object); } @@ -58,14 +75,17 @@ { _downstreamRoute = new OkResponse(downstreamRoute); _downstreamRouteFinder - .Setup(x => x.FindDownstreamRoute(It.IsAny(), It.IsAny())) - .ReturnsAsync(_downstreamRoute); + .Setup(x => x.FindDownstreamRoute(It.IsAny(), It.IsAny(), It.IsAny())) + .Returns(_downstreamRoute); } private void ThenTheScopedDataRepositoryIsCalledCorrectly() { ScopedRepository .Verify(x => x.Add("DownstreamRoute", _downstreamRoute.Data), Times.Once()); + + ScopedRepository + .Verify(x => x.Add("ServiceProviderConfiguration", _config.ServiceProviderConfiguration), Times.Once()); } } } diff --git a/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderTests.cs b/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderTests.cs index 2ae88eb3..2e586c23 100644 --- a/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderTests.cs +++ b/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderTests.cs @@ -16,21 +16,20 @@ namespace Ocelot.UnitTests.DownstreamRouteFinder public class DownstreamRouteFinderTests { private readonly IDownstreamRouteFinder _downstreamRouteFinder; - private readonly Mock _mockConfig; private readonly Mock _mockMatcher; private readonly Mock _finder; private string _upstreamUrlPath; private Response _result; private List _reRoutesConfig; + private OcelotConfiguration _config; private Response _match; private string _upstreamHttpMethod; public DownstreamRouteFinderTests() { - _mockConfig = new Mock(); _mockMatcher = new Mock(); _finder = new Mock(); - _downstreamRouteFinder = new Ocelot.DownstreamRouteFinder.Finder.DownstreamRouteFinder(_mockConfig.Object, _mockMatcher.Object, _finder.Object); + _downstreamRouteFinder = new Ocelot.DownstreamRouteFinder.Finder.DownstreamRouteFinder(_mockMatcher.Object, _finder.Object); } [Fact] @@ -341,9 +340,7 @@ namespace Ocelot.UnitTests.DownstreamRouteFinder private void GivenTheConfigurationIs(List reRoutesConfig, string adminPath, ServiceProviderConfiguration serviceProviderConfig) { _reRoutesConfig = reRoutesConfig; - _mockConfig - .Setup(x => x.Get()) - .ReturnsAsync(new OkResponse(new OcelotConfiguration(_reRoutesConfig, adminPath, serviceProviderConfig))); + _config = new OcelotConfiguration(_reRoutesConfig, adminPath, serviceProviderConfig); } private void GivenThereIsAnUpstreamUrlPath(string upstreamUrlPath) @@ -353,7 +350,7 @@ namespace Ocelot.UnitTests.DownstreamRouteFinder private void WhenICallTheFinder() { - _result = _downstreamRouteFinder.FindDownstreamRoute(_upstreamUrlPath, _upstreamHttpMethod).Result; + _result = _downstreamRouteFinder.FindDownstreamRoute(_upstreamUrlPath, _upstreamHttpMethod, _config); } private void ThenTheFollowingIsReturned(DownstreamRoute expected) diff --git a/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerMiddlewareTests.cs b/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerMiddlewareTests.cs index 987b75ea..002e64a0 100644 --- a/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerMiddlewareTests.cs +++ b/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerMiddlewareTests.cs @@ -23,7 +23,6 @@ namespace Ocelot.UnitTests.LoadBalancer { private readonly Mock _loadBalancerHouse; private readonly Mock _loadBalancer; - private readonly Mock _configProvider; private HostAndPort _hostAndPort; private OkResponse _downstreamRoute; private ErrorResponse _getLoadBalancerHouseError; @@ -33,7 +32,6 @@ namespace Ocelot.UnitTests.LoadBalancer public LoadBalancerMiddlewareTests() { - _configProvider = new Mock(); _loadBalancerHouse = new Mock(); _loadBalancer = new Mock(); _loadBalancerHouse = new Mock(); @@ -111,8 +109,8 @@ namespace Ocelot.UnitTests.LoadBalancer private void GivenTheConfigurationIs(ServiceProviderConfiguration config) { _config = config; - _configProvider - .Setup(x => x.Get()).ReturnsAsync(new OkResponse(new OcelotConfiguration(null, null, _config))); + ScopedRepository + .Setup(x => x.Get("ServiceProviderConfiguration")).Returns(new OkResponse(config)); } protected override void GivenTheTestServerServicesAreConfigured(IServiceCollection services) @@ -120,7 +118,6 @@ namespace Ocelot.UnitTests.LoadBalancer services.AddSingleton(); services.AddLogging(); services.AddSingleton(_loadBalancerHouse.Object); - services.AddSingleton(_configProvider.Object); services.AddSingleton(ScopedRepository.Object); }