From 6d8b18c01d5e53efdd7e9e6c44ec39d01b8a4a34 Mon Sep 17 00:00:00 2001 From: Tom Pallister Date: Sun, 19 Aug 2018 12:57:43 +0100 Subject: [PATCH] Feature/inject error mapper (#562) * added delegate to select last handler * #529 implemented a way we can inject the last delegating handler * wip - moving code * #529 removed loads of qos code and moved it into Ocelot.Provider.Polly * #529 can now inject http client expcetions to ocelot errors mappers and updated docs --- docs/features/qualityofservice.rst | 17 +- src/Ocelot/Configuration/QoSOptions.cs | 2 - .../DependencyInjection/OcelotBuilder.cs | 1 + .../Requester/HttpClientHttpRequester.cs | 10 +- .../Requester/HttpExeptionToErrorMapper.cs | 29 +++ .../Requester/IExceptionToErrorMapper.cs | 10 + .../Requester/ReRouteDelegatingHandler.cs | 10 - ...bleToFindDelegatingHandlerProviderError.cs | 12 -- .../Requester/HttpClientHttpRequesterTest.cs | 8 +- .../HttpExeptionToErrorMapperTests.cs | 52 +++++ .../Requester/QoSFactoryTests.cs | 55 +++++ .../Requester/QoSProviderFactoryTests.cs | 93 --------- .../Requester/QosProviderHouseTests.cs | 195 ------------------ 13 files changed, 177 insertions(+), 317 deletions(-) create mode 100644 src/Ocelot/Requester/HttpExeptionToErrorMapper.cs create mode 100644 src/Ocelot/Requester/IExceptionToErrorMapper.cs delete mode 100644 src/Ocelot/Requester/ReRouteDelegatingHandler.cs delete mode 100644 src/Ocelot/Requester/UnableToFindDelegatingHandlerProviderError.cs create mode 100644 test/Ocelot.UnitTests/Requester/HttpExeptionToErrorMapperTests.cs create mode 100644 test/Ocelot.UnitTests/Requester/QoSFactoryTests.cs delete mode 100644 test/Ocelot.UnitTests/Requester/QoSProviderFactoryTests.cs delete mode 100644 test/Ocelot.UnitTests/Requester/QosProviderHouseTests.cs diff --git a/docs/features/qualityofservice.rst b/docs/features/qualityofservice.rst index 17bf373d9..b72f99d06 100644 --- a/docs/features/qualityofservice.rst +++ b/docs/features/qualityofservice.rst @@ -5,7 +5,22 @@ Ocelot supports one QoS capability at the current time. You can set on a per ReR want to use a circuit breaker when making requests to a downstream service. This uses the an awesome .NET library called Polly check them out `here `_. -Add the following section to a ReRoute configuration. +The first thing you need to do if you want to use the administration API is bring in the relavent NuGet package.. + +``Install-Package Ocelot.Provider.Polly`` + +Then in your ConfigureServices method + +.. code-block:: csharp + + public virtual void ConfigureServices(IServiceCollection services) + { + services + .AddOcelot() + .AddPolly(); + } + +Then add the following section to a ReRoute configuration. .. code-block:: json diff --git a/src/Ocelot/Configuration/QoSOptions.cs b/src/Ocelot/Configuration/QoSOptions.cs index b0e7df7ae..3eb907eb3 100644 --- a/src/Ocelot/Configuration/QoSOptions.cs +++ b/src/Ocelot/Configuration/QoSOptions.cs @@ -7,8 +7,6 @@ public QoSOptions( int durationofBreak, int timeoutValue, string key, - //todo - this is never set in Ocelot so always Pessimistic...I guess it doesn't - //matter to much. string timeoutStrategy = "Pessimistic") { ExceptionsAllowedBeforeBreaking = exceptionsAllowedBeforeBreaking; diff --git a/src/Ocelot/DependencyInjection/OcelotBuilder.cs b/src/Ocelot/DependencyInjection/OcelotBuilder.cs index c9b490397..62adf1fb5 100644 --- a/src/Ocelot/DependencyInjection/OcelotBuilder.cs +++ b/src/Ocelot/DependencyInjection/OcelotBuilder.cs @@ -130,6 +130,7 @@ public OcelotBuilder(IServiceCollection services, IConfiguration configurationRo Services.TryAddSingleton(); Services.TryAddSingleton(); Services.TryAddSingleton(); + Services.TryAddSingleton(); } public IOcelotBuilder AddSingletonDefinedAggregator() diff --git a/src/Ocelot/Requester/HttpClientHttpRequester.cs b/src/Ocelot/Requester/HttpClientHttpRequester.cs index ea557f66f..0705e7d28 100644 --- a/src/Ocelot/Requester/HttpClientHttpRequester.cs +++ b/src/Ocelot/Requester/HttpClientHttpRequester.cs @@ -12,14 +12,17 @@ public class HttpClientHttpRequester : IHttpRequester private readonly IHttpClientCache _cacheHandlers; private readonly IOcelotLogger _logger; private readonly IDelegatingHandlerHandlerFactory _factory; + private readonly IExceptionToErrorMapper _mapper; public HttpClientHttpRequester(IOcelotLoggerFactory loggerFactory, IHttpClientCache cacheHandlers, - IDelegatingHandlerHandlerFactory house) + IDelegatingHandlerHandlerFactory factory, + IExceptionToErrorMapper mapper) { _logger = loggerFactory.CreateLogger(); _cacheHandlers = cacheHandlers; - _factory = house; + _factory = factory; + _mapper = mapper; } public async Task> GetResponse(DownstreamContext context) @@ -35,7 +38,8 @@ public async Task> GetResponse(DownstreamContext c } catch (Exception exception) { - return new ErrorResponse(new UnableToCompleteRequestError(exception)); + var error = _mapper.Map(exception); + return new ErrorResponse(error); } finally { diff --git a/src/Ocelot/Requester/HttpExeptionToErrorMapper.cs b/src/Ocelot/Requester/HttpExeptionToErrorMapper.cs new file mode 100644 index 000000000..27e9c4bad --- /dev/null +++ b/src/Ocelot/Requester/HttpExeptionToErrorMapper.cs @@ -0,0 +1,29 @@ +namespace Ocelot.Requester +{ + using System; + using System.Collections.Generic; + using Errors; + using Microsoft.Extensions.DependencyInjection; + + public class HttpExeptionToErrorMapper : IExceptionToErrorMapper + { + private readonly Dictionary> _mappers; + + public HttpExeptionToErrorMapper(IServiceProvider serviceProvider) + { + _mappers = serviceProvider.GetService>>(); + } + + public Error Map(Exception exception) + { + var type = exception.GetType(); + + if (_mappers != null && _mappers.ContainsKey(type)) + { + return _mappers[type](exception); + } + + return new UnableToCompleteRequestError(exception); + } + } +} diff --git a/src/Ocelot/Requester/IExceptionToErrorMapper.cs b/src/Ocelot/Requester/IExceptionToErrorMapper.cs new file mode 100644 index 000000000..9d765fa7e --- /dev/null +++ b/src/Ocelot/Requester/IExceptionToErrorMapper.cs @@ -0,0 +1,10 @@ +using System; +using Ocelot.Errors; + +namespace Ocelot.Requester +{ + public interface IExceptionToErrorMapper + { + Error Map(Exception exception); + } +} diff --git a/src/Ocelot/Requester/ReRouteDelegatingHandler.cs b/src/Ocelot/Requester/ReRouteDelegatingHandler.cs deleted file mode 100644 index 0a5c5472c..000000000 --- a/src/Ocelot/Requester/ReRouteDelegatingHandler.cs +++ /dev/null @@ -1,10 +0,0 @@ -using System.Net.Http; - -namespace Ocelot.Requester -{ - public class ReRouteDelegatingHandler - where T : DelegatingHandler - { - public T DelegatingHandler { get; private set; } - } -} diff --git a/src/Ocelot/Requester/UnableToFindDelegatingHandlerProviderError.cs b/src/Ocelot/Requester/UnableToFindDelegatingHandlerProviderError.cs deleted file mode 100644 index fd8aeb9e4..000000000 --- a/src/Ocelot/Requester/UnableToFindDelegatingHandlerProviderError.cs +++ /dev/null @@ -1,12 +0,0 @@ -using Ocelot.Errors; - -namespace Ocelot.Requester -{ - public class UnableToFindDelegatingHandlerProviderError : Error - { - public UnableToFindDelegatingHandlerProviderError(string message) - : base(message, OcelotErrorCode.UnableToFindDelegatingHandlerProviderError) - { - } - } -} diff --git a/test/Ocelot.UnitTests/Requester/HttpClientHttpRequesterTest.cs b/test/Ocelot.UnitTests/Requester/HttpClientHttpRequesterTest.cs index 6b961a162..a6701035e 100644 --- a/test/Ocelot.UnitTests/Requester/HttpClientHttpRequesterTest.cs +++ b/test/Ocelot.UnitTests/Requester/HttpClientHttpRequesterTest.cs @@ -27,6 +27,7 @@ public class HttpClientHttpRequesterTest private DownstreamContext _request; private Mock _loggerFactory; private Mock _logger; + private Mock _mapper; public HttpClientHttpRequesterTest() { @@ -38,10 +39,12 @@ public HttpClientHttpRequesterTest() .Setup(x => x.CreateLogger()) .Returns(_logger.Object); _cacheHandlers = new Mock(); + _mapper = new Mock(); _httpClientRequester = new HttpClientHttpRequester( _loggerFactory.Object, _cacheHandlers.Object, - _factory.Object); + _factory.Object, + _mapper.Object); } [Fact] @@ -144,6 +147,7 @@ private void ThenTheResponseIsCalledError() private void ThenTheErrorIsTimeout() { + _mapper.Verify(x => x.Map(It.IsAny()), Times.Once); _response.Errors[0].ShouldBeOfType(); } @@ -165,6 +169,8 @@ private void GivenTheHouseReturnsTimeoutHandler() }; _factory.Setup(x => x.Get(It.IsAny())).Returns(new OkResponse>>(handlers)); + + _mapper.Setup(x => x.Map(It.IsAny())).Returns(new UnableToCompleteRequestError(new Exception())); } class OkDelegatingHandler : DelegatingHandler diff --git a/test/Ocelot.UnitTests/Requester/HttpExeptionToErrorMapperTests.cs b/test/Ocelot.UnitTests/Requester/HttpExeptionToErrorMapperTests.cs new file mode 100644 index 000000000..04fb3a5e9 --- /dev/null +++ b/test/Ocelot.UnitTests/Requester/HttpExeptionToErrorMapperTests.cs @@ -0,0 +1,52 @@ +namespace Ocelot.UnitTests.Requester +{ + using System; + using System.Collections.Generic; + using System.Threading.Tasks; + using Microsoft.Extensions.DependencyInjection; + using Ocelot.Errors; + using Ocelot.Requester; + using Responder; + using Shouldly; + using Xunit; + + public class HttpExeptionToErrorMapperTests + { + private HttpExeptionToErrorMapper _mapper; + private readonly ServiceCollection _services; + + public HttpExeptionToErrorMapperTests() + { + _services = new ServiceCollection(); + var provider = _services.BuildServiceProvider(); + _mapper = new HttpExeptionToErrorMapper(provider); + } + + [Fact] + public void should_return_default_error_because_mappers_are_null() + { + var error = _mapper.Map(new Exception()); + + error.ShouldBeOfType(); + } + + [Fact] + public void should_return_error_from_mapper() + { + var errorMapping = new Dictionary> + { + {typeof(TaskCanceledException), e => new AnyError()}, + }; + + _services.AddSingleton(errorMapping); + + var provider = _services.BuildServiceProvider(); + + _mapper = new HttpExeptionToErrorMapper(provider); + + var error = _mapper.Map(new TaskCanceledException()); + + error.ShouldBeOfType(); + } + } +} diff --git a/test/Ocelot.UnitTests/Requester/QoSFactoryTests.cs b/test/Ocelot.UnitTests/Requester/QoSFactoryTests.cs new file mode 100644 index 000000000..4caedc530 --- /dev/null +++ b/test/Ocelot.UnitTests/Requester/QoSFactoryTests.cs @@ -0,0 +1,55 @@ +namespace Ocelot.UnitTests.Requester +{ + using System.Net.Http; + using Microsoft.Extensions.DependencyInjection; + using Moq; + using Ocelot.Configuration; + using Ocelot.Configuration.Builder; + using Ocelot.Logging; + using Ocelot.Requester; + using Ocelot.Requester.QoS; + using Shouldly; + using Xunit; + + public class QoSFactoryTests + { + private QoSFactory _factory; + private ServiceCollection _services; + private readonly Mock _loggerFactory; + + public QoSFactoryTests() + { + _services = new ServiceCollection(); + _loggerFactory = new Mock(); + var provider = _services.BuildServiceProvider(); + _factory = new QoSFactory(provider, _loggerFactory.Object); + } + + [Fact] + public void should_return_error() + { + var downstreamReRoute = new DownstreamReRouteBuilder().Build(); + var handler = _factory.Get(downstreamReRoute); + handler.IsError.ShouldBeTrue(); + handler.Errors[0].ShouldBeOfType(); + } + + [Fact] + public void should_return_handler() + { + _services = new ServiceCollection(); + DelegatingHandler QosDelegatingHandlerDelegate(DownstreamReRoute a, IOcelotLoggerFactory b) => new FakeDelegatingHandler(); + _services.AddSingleton(QosDelegatingHandlerDelegate); + var provider = _services.BuildServiceProvider(); + _factory = new QoSFactory(provider, _loggerFactory.Object); + var downstreamReRoute = new DownstreamReRouteBuilder().Build(); + var handler = _factory.Get(downstreamReRoute); + handler.IsError.ShouldBeFalse(); + handler.Data.ShouldBeOfType(); + } + + class FakeDelegatingHandler : DelegatingHandler + { + } + } +} diff --git a/test/Ocelot.UnitTests/Requester/QoSProviderFactoryTests.cs b/test/Ocelot.UnitTests/Requester/QoSProviderFactoryTests.cs deleted file mode 100644 index 4310788d2..000000000 --- a/test/Ocelot.UnitTests/Requester/QoSProviderFactoryTests.cs +++ /dev/null @@ -1,93 +0,0 @@ -/* -namespace Ocelot.UnitTests.Requester -{ - using Microsoft.Extensions.DependencyInjection; - using Moq; - using Ocelot.Configuration; - using Ocelot.Configuration.Builder; - using Ocelot.Logging; - using Ocelot.Requester.QoS; - using Shouldly; - using System.Collections.Generic; - using TestStack.BDDfy; - using Xunit; - - public class QoSProviderFactoryTests - { - private readonly IQoSProviderFactory _factory; - private DownstreamReRoute _reRoute; - private IQoSProvider _result; - private Mock _loggerFactory; - private Mock _logger; - - public QoSProviderFactoryTests() - { - _logger = new Mock(); - _loggerFactory = new Mock(); - var services = new ServiceCollection(); - var provider = services.BuildServiceProvider(); - _factory = new QoSProviderFactory(_loggerFactory.Object, provider); - } - - [Fact] - public void should_return_no_qos_provider() - { - var qosOptions = new QoSOptionsBuilder() - .Build(); - - var reRoute = new DownstreamReRouteBuilder() - .WithQosOptions(qosOptions) - .WithUpstreamHttpMethod(new List { "get" }) - .Build(); - - this.Given(x => x.GivenAReRoute(reRoute)) - .When(x => x.WhenIGetTheQoSProvider()) - .Then(x => x.ThenTheQoSProviderIsReturned()) - .BDDfy(); - } - - [Fact] - public void should_return_delegate_provider() - { - var qosOptions = new QoSOptionsBuilder() - .WithTimeoutValue(100) - .WithDurationOfBreak(100) - .WithExceptionsAllowedBeforeBreaking(100) - .Build(); - - var reRoute = new DownstreamReRouteBuilder() - .WithUpstreamHttpMethod(new List { "get" }) - .WithQosOptions(qosOptions) - .Build(); - - this.Given(x => x.GivenAReRoute(reRoute)) - .When(x => x.WhenIGetTheQoSProvider()) - .Then(x => x.ThenTheQoSProviderIsReturned()) - .BDDfy(); - } - - private void GivenAReRoute(DownstreamReRoute reRoute) - { - _reRoute = reRoute; - } - - private void WhenIGetTheQoSProvider() - { - _result = _factory.Get(_reRoute); - } - - private void ThenTheQoSProviderIsReturned() - { - _result.ShouldBeOfType(); - } - } - - internal class FakeProvider : IQoSProvider - { - public T CircuitBreaker() - { - throw new System.NotImplementedException(); - } - } -} -*/ diff --git a/test/Ocelot.UnitTests/Requester/QosProviderHouseTests.cs b/test/Ocelot.UnitTests/Requester/QosProviderHouseTests.cs deleted file mode 100644 index f5c3682be..000000000 --- a/test/Ocelot.UnitTests/Requester/QosProviderHouseTests.cs +++ /dev/null @@ -1,195 +0,0 @@ -/* -using Moq; -using Ocelot.Configuration; -using Ocelot.Configuration.Builder; -using Ocelot.Requester.QoS; -using Ocelot.Responses; -using Shouldly; -using TestStack.BDDfy; -using Xunit; - -namespace Ocelot.UnitTests.Requester -{ - public class QosProviderHouseTests - { - private IQoSProvider _qoSProvider; - private readonly QosProviderHouse _qosProviderHouse; - private Response _getResult; - private DownstreamReRoute _reRoute; - private readonly Mock _factory; - - public QosProviderHouseTests() - { - _factory = new Mock(); - _qosProviderHouse = new QosProviderHouse(_factory.Object); - } - - [Fact] - public void should_store_qos_provider_on_first_request() - { - var qosOptions = new QoSOptionsBuilder() - .WithKey("test") - .Build(); - - var reRoute = new DownstreamReRouteBuilder() - .WithQosOptions(qosOptions) - .Build(); - - this.Given(x => x.GivenThereIsAQoSProvider(reRoute, new FakeQoSProvider())) - .Then(x => x.ThenItIsAdded()) - .BDDfy(); - } - - [Fact] - public void should_not_store_qos_provider_on_first_request() - { - var qosOptions = new QoSOptionsBuilder() - .WithKey("test") - .Build(); - - var reRoute = new DownstreamReRouteBuilder() - .WithQosOptions(qosOptions) - .Build(); - - this.Given(x => x.GivenThereIsAQoSProvider(reRoute, new FakeQoSProvider())) - .When(x => x.WhenWeGetTheQoSProvider(reRoute)) - .Then(x => x.ThenItIsReturned()) - .BDDfy(); - } - - [Fact] - public void should_store_qos_providers_by_key() - { - var qosOptions = new QoSOptionsBuilder() - .WithKey("test") - .Build(); - - var qosOptionsTwo = new QoSOptionsBuilder() - .WithKey("testTwo") - .Build(); - - var reRoute = new DownstreamReRouteBuilder() - .WithQosOptions(qosOptions) - .Build(); - - var reRouteTwo = new DownstreamReRouteBuilder() - .WithQosOptions(qosOptionsTwo) - .Build(); - - this.Given(x => x.GivenThereIsAQoSProvider(reRoute, new FakeQoSProvider())) - .And(x => x.GivenThereIsAQoSProvider(reRouteTwo, new FakePollyQoSProvider())) - .When(x => x.WhenWeGetTheQoSProvider(reRoute)) - .Then(x => x.ThenTheQoSProviderIs()) - .When(x => x.WhenWeGetTheQoSProvider(reRouteTwo)) - .Then(x => x.ThenTheQoSProviderIs()) - .BDDfy(); - } - - [Fact] - public void should_return_error_if_no_qos_provider_with_key() - { - var qosOptions = new QoSOptionsBuilder() - .Build(); - - var reRoute = new DownstreamReRouteBuilder() - .WithQosOptions(qosOptions) - .Build(); - - this.When(x => x.WhenWeGetTheQoSProvider(reRoute)) - .Then(x => x.ThenAnErrorIsReturned()) - .BDDfy(); - } - - [Fact] - public void should_get_new_qos_provider_if_reroute_qos_provider_has_changed() - { - var useQoSOptions = new QoSOptionsBuilder() - .WithTimeoutValue(1) - .WithKey("test") - .WithDurationOfBreak(1) - .WithExceptionsAllowedBeforeBreaking(1) - .Build(); - - var dontUseQoSOptions = new QoSOptionsBuilder() - .WithKey("test") - .Build(); - - var reRoute = new DownstreamReRouteBuilder() - .WithQosOptions(dontUseQoSOptions) - .Build(); - - var reRouteTwo = new DownstreamReRouteBuilder() - .WithQosOptions(useQoSOptions) - .Build(); - - this.Given(x => x.GivenThereIsAQoSProvider(reRoute, new FakeQoSProvider())) - .When(x => x.WhenWeGetTheQoSProvider(reRoute)) - .Then(x => x.ThenTheQoSProviderIs()) - .When(x => x.WhenIGetTheReRouteWithTheSameKeyButDifferentQosProvider(reRouteTwo)) - .Then(x => x.ThenTheQoSProviderIs()) - .BDDfy(); - } - - private void WhenIGetTheReRouteWithTheSameKeyButDifferentQosProvider(DownstreamReRoute reRoute) - { - _reRoute = reRoute; - _factory.Setup(x => x.Get(_reRoute)).Returns(new FakePollyQoSProvider()); - _getResult = _qosProviderHouse.Get(_reRoute); - } - - private void ThenAnErrorIsReturned() - { - _getResult.IsError.ShouldBeTrue(); - _getResult.Errors[0].ShouldBeOfType(); - } - - private void ThenTheQoSProviderIs() - { - _getResult.Data.ShouldBeOfType(); - } - - private void ThenItIsAdded() - { - _getResult.IsError.ShouldBe(false); - _getResult.ShouldBeOfType>(); - _factory.Verify(x => x.Get(_reRoute), Times.Once); - _getResult.Data.ShouldBe(_qoSProvider); - } - - private void GivenThereIsAQoSProvider(DownstreamReRoute reRoute, IQoSProvider qoSProvider) - { - _reRoute = reRoute; - _qoSProvider = qoSProvider; - _factory.Setup(x => x.Get(_reRoute)).Returns(_qoSProvider); - _getResult = _qosProviderHouse.Get(reRoute); - } - - private void WhenWeGetTheQoSProvider(DownstreamReRoute reRoute) - { - _getResult = _qosProviderHouse.Get(reRoute); - } - - private void ThenItIsReturned() - { - _getResult.Data.ShouldBe(_qoSProvider); - _factory.Verify(x => x.Get(_reRoute), Times.Once); - } - - class FakeQoSProvider : IQoSProvider - { - T IQoSProvider.CircuitBreaker() - { - throw new System.NotImplementedException(); - } - } - - class FakePollyQoSProvider : IQoSProvider - { - T IQoSProvider.CircuitBreaker() - { - throw new System.NotImplementedException(); - } - } - } -} -*/