From 789782c8707cbe9bc90ffe8107eb1c6edb330953 Mon Sep 17 00:00:00 2001 From: David Lievrouw Date: Wed, 10 Jul 2019 18:10:26 +0200 Subject: [PATCH 01/13] Add 'Name' property to ILoadBalancer, for future use in LoadBalancerFactory. --- src/Ocelot/LoadBalancer/LoadBalancers/CookieStickySessions.cs | 2 ++ src/Ocelot/LoadBalancer/LoadBalancers/ILoadBalancer.cs | 2 ++ src/Ocelot/LoadBalancer/LoadBalancers/LeastConnection.cs | 2 ++ src/Ocelot/LoadBalancer/LoadBalancers/NoLoadBalancer.cs | 2 ++ src/Ocelot/LoadBalancer/LoadBalancers/RoundRobin.cs | 2 ++ test/Ocelot.UnitTests/LoadBalancer/LoadBalancerHouseTests.cs | 4 ++++ 6 files changed, 14 insertions(+) diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/CookieStickySessions.cs b/src/Ocelot/LoadBalancer/LoadBalancers/CookieStickySessions.cs index 775ed7ce..2429e158 100644 --- a/src/Ocelot/LoadBalancer/LoadBalancers/CookieStickySessions.cs +++ b/src/Ocelot/LoadBalancer/LoadBalancers/CookieStickySessions.cs @@ -84,5 +84,7 @@ namespace Ocelot.LoadBalancer.LoadBalancers public void Release(ServiceHostAndPort hostAndPort) { } + + public string Name => GetType().Name; } } diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/ILoadBalancer.cs b/src/Ocelot/LoadBalancer/LoadBalancers/ILoadBalancer.cs index 4f8ec2b7..6d74b686 100644 --- a/src/Ocelot/LoadBalancer/LoadBalancers/ILoadBalancer.cs +++ b/src/Ocelot/LoadBalancer/LoadBalancers/ILoadBalancer.cs @@ -10,5 +10,7 @@ namespace Ocelot.LoadBalancer.LoadBalancers Task> Lease(DownstreamContext context); void Release(ServiceHostAndPort hostAndPort); + + string Name { get; } } } diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/LeastConnection.cs b/src/Ocelot/LoadBalancer/LoadBalancers/LeastConnection.cs index ee7c0fdd..24d953e2 100644 --- a/src/Ocelot/LoadBalancer/LoadBalancers/LeastConnection.cs +++ b/src/Ocelot/LoadBalancer/LoadBalancers/LeastConnection.cs @@ -71,6 +71,8 @@ namespace Ocelot.LoadBalancer.LoadBalancers } } + public string Name => GetType().Name; + private Lease AddConnection(Lease lease) { return new Lease(lease.HostAndPort, lease.Connections + 1); diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/NoLoadBalancer.cs b/src/Ocelot/LoadBalancer/LoadBalancers/NoLoadBalancer.cs index 112fd5bb..becf96b6 100644 --- a/src/Ocelot/LoadBalancer/LoadBalancers/NoLoadBalancer.cs +++ b/src/Ocelot/LoadBalancer/LoadBalancers/NoLoadBalancer.cs @@ -33,5 +33,7 @@ namespace Ocelot.LoadBalancer.LoadBalancers public void Release(ServiceHostAndPort hostAndPort) { } + + public string Name => GetType().Name; } } diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/RoundRobin.cs b/src/Ocelot/LoadBalancer/LoadBalancers/RoundRobin.cs index 3500efe0..6be24f71 100644 --- a/src/Ocelot/LoadBalancer/LoadBalancers/RoundRobin.cs +++ b/src/Ocelot/LoadBalancer/LoadBalancers/RoundRobin.cs @@ -38,5 +38,7 @@ namespace Ocelot.LoadBalancer.LoadBalancers public void Release(ServiceHostAndPort hostAndPort) { } + + public string Name => GetType().Name; } } diff --git a/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerHouseTests.cs b/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerHouseTests.cs index 57252b2b..6f5a50c1 100644 --- a/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerHouseTests.cs +++ b/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerHouseTests.cs @@ -164,6 +164,8 @@ namespace Ocelot.UnitTests.LoadBalancer { throw new NotImplementedException(); } + + public string Name => GetType().Name; } private class FakeRoundRobinLoadBalancer : ILoadBalancer @@ -177,6 +179,8 @@ namespace Ocelot.UnitTests.LoadBalancer { throw new NotImplementedException(); } + + public string Name => GetType().Name; } } } From c73cf5990855a3ca4971f7ff39fddecaa9be0121 Mon Sep 17 00:00:00 2001 From: David Lievrouw Date: Wed, 10 Jul 2019 18:28:11 +0200 Subject: [PATCH 02/13] Revert "Add 'Name' property to ILoadBalancer, for future use in LoadBalancerFactory." This reverts commit 78ead1606dc6a4d6b47b5e63bbf7927ef02e9320. --- src/Ocelot/LoadBalancer/LoadBalancers/CookieStickySessions.cs | 2 -- src/Ocelot/LoadBalancer/LoadBalancers/ILoadBalancer.cs | 2 -- src/Ocelot/LoadBalancer/LoadBalancers/LeastConnection.cs | 2 -- src/Ocelot/LoadBalancer/LoadBalancers/NoLoadBalancer.cs | 2 -- src/Ocelot/LoadBalancer/LoadBalancers/RoundRobin.cs | 2 -- test/Ocelot.UnitTests/LoadBalancer/LoadBalancerHouseTests.cs | 4 ---- 6 files changed, 14 deletions(-) diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/CookieStickySessions.cs b/src/Ocelot/LoadBalancer/LoadBalancers/CookieStickySessions.cs index 2429e158..775ed7ce 100644 --- a/src/Ocelot/LoadBalancer/LoadBalancers/CookieStickySessions.cs +++ b/src/Ocelot/LoadBalancer/LoadBalancers/CookieStickySessions.cs @@ -84,7 +84,5 @@ namespace Ocelot.LoadBalancer.LoadBalancers public void Release(ServiceHostAndPort hostAndPort) { } - - public string Name => GetType().Name; } } diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/ILoadBalancer.cs b/src/Ocelot/LoadBalancer/LoadBalancers/ILoadBalancer.cs index 6d74b686..4f8ec2b7 100644 --- a/src/Ocelot/LoadBalancer/LoadBalancers/ILoadBalancer.cs +++ b/src/Ocelot/LoadBalancer/LoadBalancers/ILoadBalancer.cs @@ -10,7 +10,5 @@ namespace Ocelot.LoadBalancer.LoadBalancers Task> Lease(DownstreamContext context); void Release(ServiceHostAndPort hostAndPort); - - string Name { get; } } } diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/LeastConnection.cs b/src/Ocelot/LoadBalancer/LoadBalancers/LeastConnection.cs index 24d953e2..ee7c0fdd 100644 --- a/src/Ocelot/LoadBalancer/LoadBalancers/LeastConnection.cs +++ b/src/Ocelot/LoadBalancer/LoadBalancers/LeastConnection.cs @@ -71,8 +71,6 @@ namespace Ocelot.LoadBalancer.LoadBalancers } } - public string Name => GetType().Name; - private Lease AddConnection(Lease lease) { return new Lease(lease.HostAndPort, lease.Connections + 1); diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/NoLoadBalancer.cs b/src/Ocelot/LoadBalancer/LoadBalancers/NoLoadBalancer.cs index becf96b6..112fd5bb 100644 --- a/src/Ocelot/LoadBalancer/LoadBalancers/NoLoadBalancer.cs +++ b/src/Ocelot/LoadBalancer/LoadBalancers/NoLoadBalancer.cs @@ -33,7 +33,5 @@ namespace Ocelot.LoadBalancer.LoadBalancers public void Release(ServiceHostAndPort hostAndPort) { } - - public string Name => GetType().Name; } } diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/RoundRobin.cs b/src/Ocelot/LoadBalancer/LoadBalancers/RoundRobin.cs index 6be24f71..3500efe0 100644 --- a/src/Ocelot/LoadBalancer/LoadBalancers/RoundRobin.cs +++ b/src/Ocelot/LoadBalancer/LoadBalancers/RoundRobin.cs @@ -38,7 +38,5 @@ namespace Ocelot.LoadBalancer.LoadBalancers public void Release(ServiceHostAndPort hostAndPort) { } - - public string Name => GetType().Name; } } diff --git a/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerHouseTests.cs b/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerHouseTests.cs index 6f5a50c1..57252b2b 100644 --- a/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerHouseTests.cs +++ b/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerHouseTests.cs @@ -164,8 +164,6 @@ namespace Ocelot.UnitTests.LoadBalancer { throw new NotImplementedException(); } - - public string Name => GetType().Name; } private class FakeRoundRobinLoadBalancer : ILoadBalancer @@ -179,8 +177,6 @@ namespace Ocelot.UnitTests.LoadBalancer { throw new NotImplementedException(); } - - public string Name => GetType().Name; } } } From de012a079409fa1aad23dbeef29ee89faf871381 Mon Sep 17 00:00:00 2001 From: David Lievrouw Date: Wed, 10 Jul 2019 19:15:17 +0200 Subject: [PATCH 03/13] Rewire LoadBalancerFactory to allow injecting custom load balancers, by introducing LoadBalancerCreators --- .../DependencyInjection/OcelotBuilder.cs | 4 + .../CookieStickySessionsCreator.cs | 20 +++ .../LoadBalancers/ILoadBalancerCreator.cs | 11 ++ .../LoadBalancers/LeastConnectionCreator.cs | 15 +++ .../LoadBalancers/LoadBalancerFactory.cs | 55 ++++---- .../LoadBalancers/NoLoadBalancerCreator.cs | 15 +++ .../LoadBalancer/LoadBalancers/RoundRobin.cs | 16 +-- .../LoadBalancers/RoundRobinCreator.cs | 15 +++ .../DependencyInjection/OcelotBuilderTests.cs | 40 ++++++ .../CookieStickySessionsCreatorTests.cs | 73 +++++++++++ .../LeastConnectionCreatorTests.cs | 73 +++++++++++ .../LoadBalancer/LoadBalancerFactoryTests.cs | 117 ++++++++++++------ .../NoLoadBalancerCreatorTests.cs | 72 +++++++++++ .../LoadBalancer/RoundRobinCreatorTests.cs | 72 +++++++++++ 14 files changed, 521 insertions(+), 77 deletions(-) create mode 100644 src/Ocelot/LoadBalancer/LoadBalancers/CookieStickySessionsCreator.cs create mode 100644 src/Ocelot/LoadBalancer/LoadBalancers/ILoadBalancerCreator.cs create mode 100644 src/Ocelot/LoadBalancer/LoadBalancers/LeastConnectionCreator.cs create mode 100644 src/Ocelot/LoadBalancer/LoadBalancers/NoLoadBalancerCreator.cs create mode 100644 src/Ocelot/LoadBalancer/LoadBalancers/RoundRobinCreator.cs create mode 100644 test/Ocelot.UnitTests/LoadBalancer/CookieStickySessionsCreatorTests.cs create mode 100644 test/Ocelot.UnitTests/LoadBalancer/LeastConnectionCreatorTests.cs create mode 100644 test/Ocelot.UnitTests/LoadBalancer/NoLoadBalancerCreatorTests.cs create mode 100644 test/Ocelot.UnitTests/LoadBalancer/RoundRobinCreatorTests.cs diff --git a/src/Ocelot/DependencyInjection/OcelotBuilder.cs b/src/Ocelot/DependencyInjection/OcelotBuilder.cs index 0336f691..a9290ad7 100644 --- a/src/Ocelot/DependencyInjection/OcelotBuilder.cs +++ b/src/Ocelot/DependencyInjection/OcelotBuilder.cs @@ -87,6 +87,10 @@ namespace Ocelot.DependencyInjection Services.TryAddSingleton(); Services.TryAddSingleton(); Services.TryAddSingleton(); + Services.AddSingleton(); + Services.AddSingleton(); + Services.AddSingleton(); + Services.AddSingleton(); Services.TryAddSingleton(); Services.TryAddSingleton(); Services.TryAddSingleton(); diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/CookieStickySessionsCreator.cs b/src/Ocelot/LoadBalancer/LoadBalancers/CookieStickySessionsCreator.cs new file mode 100644 index 00000000..e78b1884 --- /dev/null +++ b/src/Ocelot/LoadBalancer/LoadBalancers/CookieStickySessionsCreator.cs @@ -0,0 +1,20 @@ +namespace Ocelot.LoadBalancer.LoadBalancers +{ + using System.Threading.Tasks; + using Ocelot.Configuration; + using Ocelot.Infrastructure; + using Ocelot.ServiceDiscovery.Providers; + + public class CookieStickySessionsCreator : ILoadBalancerCreator + { + public ILoadBalancer Create(DownstreamReRoute reRoute, IServiceDiscoveryProvider serviceProvider) + { + var loadBalancer = new RoundRobin(async () => await serviceProvider.Get()); + var bus = new InMemoryBus(); + return new CookieStickySessions(loadBalancer, reRoute.LoadBalancerOptions.Key, + reRoute.LoadBalancerOptions.ExpiryInMs, bus); + } + + public string Type => nameof(CookieStickySessions); + } +} diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/ILoadBalancerCreator.cs b/src/Ocelot/LoadBalancer/LoadBalancers/ILoadBalancerCreator.cs new file mode 100644 index 00000000..4831e62f --- /dev/null +++ b/src/Ocelot/LoadBalancer/LoadBalancers/ILoadBalancerCreator.cs @@ -0,0 +1,11 @@ +using Ocelot.Configuration; +using Ocelot.ServiceDiscovery.Providers; + +namespace Ocelot.LoadBalancer.LoadBalancers +{ + public interface ILoadBalancerCreator + { + ILoadBalancer Create(DownstreamReRoute reRoute, IServiceDiscoveryProvider serviceProvider); + string Type { get; } + } +} diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/LeastConnectionCreator.cs b/src/Ocelot/LoadBalancer/LoadBalancers/LeastConnectionCreator.cs new file mode 100644 index 00000000..e9e9c561 --- /dev/null +++ b/src/Ocelot/LoadBalancer/LoadBalancers/LeastConnectionCreator.cs @@ -0,0 +1,15 @@ +namespace Ocelot.LoadBalancer.LoadBalancers +{ + using Ocelot.Configuration; + using Ocelot.ServiceDiscovery.Providers; + + public class LeastConnectionCreator : ILoadBalancerCreator + { + public ILoadBalancer Create(DownstreamReRoute reRoute, IServiceDiscoveryProvider serviceProvider) + { + return new LeastConnection(async () => await serviceProvider.Get(), reRoute.ServiceName); + } + + public string Type => nameof(LeastConnection); + } +} diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/LoadBalancerFactory.cs b/src/Ocelot/LoadBalancer/LoadBalancers/LoadBalancerFactory.cs index 12725a57..2b7e71e2 100644 --- a/src/Ocelot/LoadBalancer/LoadBalancers/LoadBalancerFactory.cs +++ b/src/Ocelot/LoadBalancer/LoadBalancers/LoadBalancerFactory.cs @@ -1,47 +1,42 @@ -using Ocelot.Configuration; -using Ocelot.Infrastructure; -using Ocelot.Responses; -using Ocelot.ServiceDiscovery; -using System.Threading.Tasks; - -namespace Ocelot.LoadBalancer.LoadBalancers +namespace Ocelot.LoadBalancer.LoadBalancers { + using System.Collections.Generic; + using System.Linq; + using Ocelot.Configuration; + using Ocelot.Responses; + using System.Threading.Tasks; + using Ocelot.ServiceDiscovery; + public class LoadBalancerFactory : ILoadBalancerFactory { private readonly IServiceDiscoveryProviderFactory _serviceProviderFactory; + private readonly IEnumerable _loadBalancerCreators; - public LoadBalancerFactory(IServiceDiscoveryProviderFactory serviceProviderFactory) + public LoadBalancerFactory(IServiceDiscoveryProviderFactory serviceProviderFactory, IEnumerable loadBalancerCreators) { _serviceProviderFactory = serviceProviderFactory; + _loadBalancerCreators = loadBalancerCreators; } - public async Task> Get(DownstreamReRoute reRoute, ServiceProviderConfiguration config) + public Task> Get(DownstreamReRoute reRoute, ServiceProviderConfiguration config) { - var response = _serviceProviderFactory.Get(config, reRoute); + var serviceProviderFactoryResponse = _serviceProviderFactory.Get(config, reRoute); - if (response.IsError) + Response response; + if (serviceProviderFactoryResponse.IsError) { - return new ErrorResponse(response.Errors); + response = new ErrorResponse(serviceProviderFactoryResponse.Errors); + } + else + { + var serviceProvider = serviceProviderFactoryResponse.Data; + var requestedType = reRoute.LoadBalancerOptions?.Type ?? nameof(NoLoadBalancer); + var applicableCreator = _loadBalancerCreators.Single(c => c.Type == requestedType); + var createdLoadBalancer = applicableCreator.Create(reRoute, serviceProvider); + response = new OkResponse(createdLoadBalancer); } - var serviceProvider = response.Data; - - switch (reRoute.LoadBalancerOptions?.Type) - { - case nameof(RoundRobin): - return new OkResponse(new RoundRobin(async () => await serviceProvider.Get())); - - case nameof(LeastConnection): - return new OkResponse(new LeastConnection(async () => await serviceProvider.Get(), reRoute.ServiceName)); - - case nameof(CookieStickySessions): - var loadBalancer = new RoundRobin(async () => await serviceProvider.Get()); - var bus = new InMemoryBus(); - return new OkResponse(new CookieStickySessions(loadBalancer, reRoute.LoadBalancerOptions.Key, reRoute.LoadBalancerOptions.ExpiryInMs, bus)); - - default: - return new OkResponse(new NoLoadBalancer(async () => await serviceProvider.Get())); - } + return Task.FromResult(response); } } } diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/NoLoadBalancerCreator.cs b/src/Ocelot/LoadBalancer/LoadBalancers/NoLoadBalancerCreator.cs new file mode 100644 index 00000000..fc1de3fd --- /dev/null +++ b/src/Ocelot/LoadBalancer/LoadBalancers/NoLoadBalancerCreator.cs @@ -0,0 +1,15 @@ +namespace Ocelot.LoadBalancer.LoadBalancers +{ + using Ocelot.Configuration; + using Ocelot.ServiceDiscovery.Providers; + + public class NoLoadBalancerCreator : ILoadBalancerCreator + { + public ILoadBalancer Create(DownstreamReRoute reRoute, IServiceDiscoveryProvider serviceProvider) + { + return new NoLoadBalancer(async () => await serviceProvider.Get()); + } + + public string Type => nameof(NoLoadBalancer); + } +} diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/RoundRobin.cs b/src/Ocelot/LoadBalancer/LoadBalancers/RoundRobin.cs index 3500efe0..492a2c57 100644 --- a/src/Ocelot/LoadBalancer/LoadBalancers/RoundRobin.cs +++ b/src/Ocelot/LoadBalancer/LoadBalancers/RoundRobin.cs @@ -1,12 +1,12 @@ -using Ocelot.Middleware; -using Ocelot.Responses; -using Ocelot.Values; -using System; -using System.Collections.Generic; -using System.Threading.Tasks; - -namespace Ocelot.LoadBalancer.LoadBalancers +namespace Ocelot.LoadBalancer.LoadBalancers { + using Ocelot.Middleware; + using Ocelot.Responses; + using Ocelot.Values; + using System; + using System.Collections.Generic; + using System.Threading.Tasks; + public class RoundRobin : ILoadBalancer { private readonly Func>> _services; diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/RoundRobinCreator.cs b/src/Ocelot/LoadBalancer/LoadBalancers/RoundRobinCreator.cs new file mode 100644 index 00000000..8981074d --- /dev/null +++ b/src/Ocelot/LoadBalancer/LoadBalancers/RoundRobinCreator.cs @@ -0,0 +1,15 @@ +namespace Ocelot.LoadBalancer.LoadBalancers +{ + using Ocelot.Configuration; + using Ocelot.ServiceDiscovery.Providers; + + public class RoundRobinCreator : ILoadBalancerCreator + { + public ILoadBalancer Create(DownstreamReRoute reRoute, IServiceDiscoveryProvider serviceProvider) + { + return new RoundRobin(async () => await serviceProvider.Get()); + } + + public string Type => nameof(RoundRobin); + } +} diff --git a/test/Ocelot.UnitTests/DependencyInjection/OcelotBuilderTests.cs b/test/Ocelot.UnitTests/DependencyInjection/OcelotBuilderTests.cs index 0f1ddb60..499c2a75 100644 --- a/test/Ocelot.UnitTests/DependencyInjection/OcelotBuilderTests.cs +++ b/test/Ocelot.UnitTests/DependencyInjection/OcelotBuilderTests.cs @@ -1,3 +1,7 @@ +using Ocelot.Configuration; +using Ocelot.LoadBalancer.LoadBalancers; +using Ocelot.ServiceDiscovery.Providers; + namespace Ocelot.UnitTests.DependencyInjection { using Microsoft.AspNetCore.Hosting; @@ -158,11 +162,26 @@ namespace Ocelot.UnitTests.DependencyInjection .BDDfy(); } + [Fact] + public void should_add_custom_load_balancer_creators() + { + this.Given(x => WhenISetUpOcelotServices()) + .When(x => AddSingletonLoadBalancerCreator()) + .Then(x => ThenTheProviderIsRegisteredAndReturnsBothBuiltInAndCustomLoadBalancerCreators()) + .BDDfy(); + } + private void AddSingletonDefinedAggregator() where T : class, IDefinedAggregator { _ocelotBuilder.AddSingletonDefinedAggregator(); } + + private void AddSingletonLoadBalancerCreator() + where T : class, ILoadBalancerCreator + { + _ocelotBuilder.Services.AddSingleton(); + } private void AddTransientDefinedAggregator() where T : class, IDefinedAggregator @@ -238,6 +257,17 @@ namespace Ocelot.UnitTests.DependencyInjection handlers[0].ShouldBeOfType(); handlers[1].ShouldBeOfType(); } + + private void ThenTheProviderIsRegisteredAndReturnsBothBuiltInAndCustomLoadBalancerCreators() + { + _serviceProvider = _services.BuildServiceProvider(); + var creators = _serviceProvider.GetServices().ToList(); + creators.Count(c => c.GetType() == typeof(NoLoadBalancerCreator)).ShouldBe(1); + creators.Count(c => c.GetType() == typeof(RoundRobinCreator)).ShouldBe(1); + creators.Count(c => c.GetType() == typeof(CookieStickySessionsCreator)).ShouldBe(1); + creators.Count(c => c.GetType() == typeof(LeastConnectionCreator)).ShouldBe(1); + creators.Count(c => c.GetType() == typeof(FakeCustomLoadBalancerCreator)).ShouldBe(1); + } private void ThenTheAggregatorsAreTransient() { @@ -323,5 +353,15 @@ namespace Ocelot.UnitTests.DependencyInjection { _ex.ShouldBeNull(); } + + private class FakeCustomLoadBalancerCreator : ILoadBalancerCreator + { + public ILoadBalancer Create(DownstreamReRoute reRoute, IServiceDiscoveryProvider serviceProvider) + { + throw new NotImplementedException(); + } + + public string Type { get; } + } } } diff --git a/test/Ocelot.UnitTests/LoadBalancer/CookieStickySessionsCreatorTests.cs b/test/Ocelot.UnitTests/LoadBalancer/CookieStickySessionsCreatorTests.cs new file mode 100644 index 00000000..20f65da4 --- /dev/null +++ b/test/Ocelot.UnitTests/LoadBalancer/CookieStickySessionsCreatorTests.cs @@ -0,0 +1,73 @@ +using Moq; +using Ocelot.Configuration; +using Ocelot.Configuration.Builder; +using Ocelot.LoadBalancer.LoadBalancers; +using Ocelot.ServiceDiscovery.Providers; +using Shouldly; +using TestStack.BDDfy; +using Xunit; + +namespace Ocelot.UnitTests.LoadBalancer +{ + public class CookieStickySessionsCreatorTests + { + private readonly CookieStickySessionsCreator _creator; + private readonly Mock _serviceProvider; + private DownstreamReRoute _reRoute; + private ILoadBalancer _loadBalancer; + private string _typeName; + + public CookieStickySessionsCreatorTests() + { + _creator = new CookieStickySessionsCreator(); + _serviceProvider = new Mock(); + } + + [Fact] + public void should_return_expected_name() + { + var reRoute = new DownstreamReRouteBuilder() + .WithLoadBalancerOptions(new LoadBalancerOptions("myType", "myKey", 1000)) + .Build(); + + this.Given(x => x.GivenAReRoute(reRoute)) + .When(x => x.WhenIGetTheLoadBalancer()) + .Then(x => x.ThenTheLoadBalancerIsReturned()) + .BDDfy(); + } + + [Fact] + public void should_return_instance_of_expected_load_balancer_type() + { + this.When(x => x.WhenIGetTheLoadBalancerTypeName()) + .Then(x => x.ThenTheLoadBalancerTypeIs("CookieStickySessions")) + .BDDfy(); + } + + private void GivenAReRoute(DownstreamReRoute reRoute) + { + _reRoute = reRoute; + } + + private void WhenIGetTheLoadBalancer() + { + _loadBalancer = _creator.Create(_reRoute, _serviceProvider.Object); + } + + private void WhenIGetTheLoadBalancerTypeName() + { + _typeName = _creator.Type; + } + + private void ThenTheLoadBalancerIsReturned() + where T : ILoadBalancer + { + _loadBalancer.ShouldBeOfType(); + } + + private void ThenTheLoadBalancerTypeIs(string type) + { + _typeName.ShouldBe(type); + } + } +} diff --git a/test/Ocelot.UnitTests/LoadBalancer/LeastConnectionCreatorTests.cs b/test/Ocelot.UnitTests/LoadBalancer/LeastConnectionCreatorTests.cs new file mode 100644 index 00000000..507958fd --- /dev/null +++ b/test/Ocelot.UnitTests/LoadBalancer/LeastConnectionCreatorTests.cs @@ -0,0 +1,73 @@ +using Moq; +using Ocelot.Configuration; +using Ocelot.Configuration.Builder; +using Ocelot.LoadBalancer.LoadBalancers; +using Ocelot.ServiceDiscovery.Providers; +using Shouldly; +using TestStack.BDDfy; +using Xunit; + +namespace Ocelot.UnitTests.LoadBalancer +{ + public class LeastConnectionCreatorTests + { + private readonly LeastConnectionCreator _creator; + private readonly Mock _serviceProvider; + private DownstreamReRoute _reRoute; + private ILoadBalancer _loadBalancer; + private string _typeName; + + public LeastConnectionCreatorTests() + { + _creator = new LeastConnectionCreator(); + _serviceProvider = new Mock(); + } + + [Fact] + public void should_return_expected_name() + { + var reRoute = new DownstreamReRouteBuilder() + .WithServiceName("myService") + .Build(); + + this.Given(x => x.GivenAReRoute(reRoute)) + .When(x => x.WhenIGetTheLoadBalancer()) + .Then(x => x.ThenTheLoadBalancerIsReturned()) + .BDDfy(); + } + + [Fact] + public void should_return_instance_of_expected_load_balancer_type() + { + this.When(x => x.WhenIGetTheLoadBalancerTypeName()) + .Then(x => x.ThenTheLoadBalancerTypeIs("LeastConnection")) + .BDDfy(); + } + + private void GivenAReRoute(DownstreamReRoute reRoute) + { + _reRoute = reRoute; + } + + private void WhenIGetTheLoadBalancer() + { + _loadBalancer = _creator.Create(_reRoute, _serviceProvider.Object); + } + + private void WhenIGetTheLoadBalancerTypeName() + { + _typeName = _creator.Type; + } + + private void ThenTheLoadBalancerIsReturned() + where T : ILoadBalancer + { + _loadBalancer.ShouldBeOfType(); + } + + private void ThenTheLoadBalancerTypeIs(string type) + { + _typeName.ShouldBe(type); + } + } +} diff --git a/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerFactoryTests.cs b/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerFactoryTests.cs index 8df47cea..70b9a837 100644 --- a/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerFactoryTests.cs +++ b/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerFactoryTests.cs @@ -7,6 +7,9 @@ using Ocelot.ServiceDiscovery; using Ocelot.ServiceDiscovery.Providers; using Shouldly; using System.Collections.Generic; +using System.Threading.Tasks; +using Ocelot.Middleware; +using Ocelot.Values; using TestStack.BDDfy; using Xunit; @@ -18,6 +21,7 @@ namespace Ocelot.UnitTests.LoadBalancer private readonly LoadBalancerFactory _factory; private Response _result; private readonly Mock _serviceProviderFactory; + private readonly IEnumerable _loadBalancerCreators; private readonly Mock _serviceProvider; private ServiceProviderConfiguration _serviceProviderConfig; @@ -25,7 +29,13 @@ namespace Ocelot.UnitTests.LoadBalancer { _serviceProviderFactory = new Mock(); _serviceProvider = new Mock(); - _factory = new LoadBalancerFactory(_serviceProviderFactory.Object); + _loadBalancerCreators = new ILoadBalancerCreator[] + { + new FakeLoadBalancerCreator(), + new FakeLoadBalancerCreator(), + new FakeLoadBalancerCreator(nameof(NoLoadBalancer)), + }; + _factory = new LoadBalancerFactory(_serviceProviderFactory.Object, _loadBalancerCreators); } [Fact] @@ -39,15 +49,15 @@ namespace Ocelot.UnitTests.LoadBalancer .And(x => GivenAServiceProviderConfig(new ServiceProviderConfigurationBuilder().Build())) .And(x => x.GivenTheServiceProviderFactoryReturns()) .When(x => x.WhenIGetTheLoadBalancer()) - .Then(x => x.ThenTheLoadBalancerIsReturned()) + .Then(x => x.ThenTheLoadBalancerIsReturned()) .BDDfy(); } [Fact] - public void should_return_round_robin_load_balancer() + public void should_return_matching_load_balancer() { var reRoute = new DownstreamReRouteBuilder() - .WithLoadBalancerOptions(new LoadBalancerOptions("RoundRobin", "", 0)) + .WithLoadBalancerOptions(new LoadBalancerOptions("FakeLoadBalancerTwo", "", 0)) .WithUpstreamHttpMethod(new List { "Get" }) .Build(); @@ -55,31 +65,15 @@ namespace Ocelot.UnitTests.LoadBalancer .And(x => GivenAServiceProviderConfig(new ServiceProviderConfigurationBuilder().Build())) .And(x => x.GivenTheServiceProviderFactoryReturns()) .When(x => x.WhenIGetTheLoadBalancer()) - .Then(x => x.ThenTheLoadBalancerIsReturned()) + .Then(x => x.ThenTheLoadBalancerIsReturned()) .BDDfy(); } - - [Fact] - public void should_return_round_least_connection_balancer() - { - var reRoute = new DownstreamReRouteBuilder() - .WithLoadBalancerOptions(new LoadBalancerOptions("LeastConnection", "", 0)) - .WithUpstreamHttpMethod(new List { "Get" }) - .Build(); - - this.Given(x => x.GivenAReRoute(reRoute)) - .And(x => GivenAServiceProviderConfig(new ServiceProviderConfigurationBuilder().Build())) - .And(x => x.GivenTheServiceProviderFactoryReturns()) - .When(x => x.WhenIGetTheLoadBalancer()) - .Then(x => x.ThenTheLoadBalancerIsReturned()) - .BDDfy(); - } - + [Fact] public void should_call_service_provider() { var reRoute = new DownstreamReRouteBuilder() - .WithLoadBalancerOptions(new LoadBalancerOptions("RoundRobin", "", 0)) + .WithLoadBalancerOptions(new LoadBalancerOptions("FakeLoadBalancerOne", "", 0)) .WithUpstreamHttpMethod(new List { "Get" }) .Build(); @@ -91,22 +85,6 @@ namespace Ocelot.UnitTests.LoadBalancer .BDDfy(); } - [Fact] - public void should_return_sticky_session() - { - var reRoute = new DownstreamReRouteBuilder() - .WithLoadBalancerOptions(new LoadBalancerOptions("CookieStickySessions", "", 0)) - .WithUpstreamHttpMethod(new List { "Get" }) - .Build(); - - this.Given(x => x.GivenAReRoute(reRoute)) - .And(x => GivenAServiceProviderConfig(new ServiceProviderConfigurationBuilder().Build())) - .And(x => x.GivenTheServiceProviderFactoryReturns()) - .When(x => x.WhenIGetTheLoadBalancer()) - .Then(x => x.ThenTheLoadBalancerIsReturned()) - .BDDfy(); - } - private void GivenAServiceProviderConfig(ServiceProviderConfiguration serviceProviderConfig) { _serviceProviderConfig = serviceProviderConfig; @@ -139,5 +117,66 @@ namespace Ocelot.UnitTests.LoadBalancer { _result.Data.ShouldBeOfType(); } + + private class FakeLoadBalancerCreator : ILoadBalancerCreator + where T : ILoadBalancer, new() + { + + public FakeLoadBalancerCreator() + { + Type = typeof(T).Name; + } + + public FakeLoadBalancerCreator(string type) + { + Type = type; + } + + public ILoadBalancer Create(DownstreamReRoute reRoute, IServiceDiscoveryProvider serviceProvider) + { + return new T(); + } + + public string Type { get; } + } + + private class FakeLoadBalancerOne : ILoadBalancer + { + public Task> Lease(DownstreamContext context) + { + throw new System.NotImplementedException(); + } + + public void Release(ServiceHostAndPort hostAndPort) + { + throw new System.NotImplementedException(); + } + } + + private class FakeLoadBalancerTwo : ILoadBalancer + { + public Task> Lease(DownstreamContext context) + { + throw new System.NotImplementedException(); + } + + public void Release(ServiceHostAndPort hostAndPort) + { + throw new System.NotImplementedException(); + } + } + + private class FakeNoLoadBalancer : ILoadBalancer + { + public Task> Lease(DownstreamContext context) + { + throw new System.NotImplementedException(); + } + + public void Release(ServiceHostAndPort hostAndPort) + { + throw new System.NotImplementedException(); + } + } } } diff --git a/test/Ocelot.UnitTests/LoadBalancer/NoLoadBalancerCreatorTests.cs b/test/Ocelot.UnitTests/LoadBalancer/NoLoadBalancerCreatorTests.cs new file mode 100644 index 00000000..6b61c5a9 --- /dev/null +++ b/test/Ocelot.UnitTests/LoadBalancer/NoLoadBalancerCreatorTests.cs @@ -0,0 +1,72 @@ +using Moq; +using Ocelot.Configuration; +using Ocelot.Configuration.Builder; +using Ocelot.LoadBalancer.LoadBalancers; +using Ocelot.ServiceDiscovery.Providers; +using Shouldly; +using TestStack.BDDfy; +using Xunit; + +namespace Ocelot.UnitTests.LoadBalancer +{ + public class NoLoadBalancerCreatorTests + { + private readonly NoLoadBalancerCreator _creator; + private readonly Mock _serviceProvider; + private DownstreamReRoute _reRoute; + private ILoadBalancer _loadBalancer; + private string _typeName; + + public NoLoadBalancerCreatorTests() + { + _creator = new NoLoadBalancerCreator(); + _serviceProvider = new Mock(); + } + + [Fact] + public void should_return_expected_name() + { + var reRoute = new DownstreamReRouteBuilder() + .Build(); + + this.Given(x => x.GivenAReRoute(reRoute)) + .When(x => x.WhenIGetTheLoadBalancer()) + .Then(x => x.ThenTheLoadBalancerIsReturned()) + .BDDfy(); + } + + [Fact] + public void should_return_instance_of_expected_load_balancer_type() + { + this.When(x => x.WhenIGetTheLoadBalancerTypeName()) + .Then(x => x.ThenTheLoadBalancerTypeIs("NoLoadBalancer")) + .BDDfy(); + } + + private void GivenAReRoute(DownstreamReRoute reRoute) + { + _reRoute = reRoute; + } + + private void WhenIGetTheLoadBalancer() + { + _loadBalancer = _creator.Create(_reRoute, _serviceProvider.Object); + } + + private void WhenIGetTheLoadBalancerTypeName() + { + _typeName = _creator.Type; + } + + private void ThenTheLoadBalancerIsReturned() + where T : ILoadBalancer + { + _loadBalancer.ShouldBeOfType(); + } + + private void ThenTheLoadBalancerTypeIs(string type) + { + _typeName.ShouldBe(type); + } + } +} diff --git a/test/Ocelot.UnitTests/LoadBalancer/RoundRobinCreatorTests.cs b/test/Ocelot.UnitTests/LoadBalancer/RoundRobinCreatorTests.cs new file mode 100644 index 00000000..80d43568 --- /dev/null +++ b/test/Ocelot.UnitTests/LoadBalancer/RoundRobinCreatorTests.cs @@ -0,0 +1,72 @@ +using Moq; +using Ocelot.Configuration; +using Ocelot.Configuration.Builder; +using Ocelot.LoadBalancer.LoadBalancers; +using Ocelot.ServiceDiscovery.Providers; +using Shouldly; +using TestStack.BDDfy; +using Xunit; + +namespace Ocelot.UnitTests.LoadBalancer +{ + public class RoundRobinCreatorTests + { + private readonly RoundRobinCreator _creator; + private readonly Mock _serviceProvider; + private DownstreamReRoute _reRoute; + private ILoadBalancer _loadBalancer; + private string _typeName; + + public RoundRobinCreatorTests() + { + _creator = new RoundRobinCreator(); + _serviceProvider = new Mock(); + } + + [Fact] + public void should_return_expected_name() + { + var reRoute = new DownstreamReRouteBuilder() + .Build(); + + this.Given(x => x.GivenAReRoute(reRoute)) + .When(x => x.WhenIGetTheLoadBalancer()) + .Then(x => x.ThenTheLoadBalancerIsReturned()) + .BDDfy(); + } + + [Fact] + public void should_return_instance_of_expected_load_balancer_type() + { + this.When(x => x.WhenIGetTheLoadBalancerTypeName()) + .Then(x => x.ThenTheLoadBalancerTypeIs("RoundRobin")) + .BDDfy(); + } + + private void GivenAReRoute(DownstreamReRoute reRoute) + { + _reRoute = reRoute; + } + + private void WhenIGetTheLoadBalancer() + { + _loadBalancer = _creator.Create(_reRoute, _serviceProvider.Object); + } + + private void WhenIGetTheLoadBalancerTypeName() + { + _typeName = _creator.Type; + } + + private void ThenTheLoadBalancerIsReturned() + where T : ILoadBalancer + { + _loadBalancer.ShouldBeOfType(); + } + + private void ThenTheLoadBalancerTypeIs(string type) + { + _typeName.ShouldBe(type); + } + } +} From 80ab5f6a912f2225e2a3caf24d679560a52247ae Mon Sep 17 00:00:00 2001 From: David Lievrouw Date: Wed, 10 Jul 2019 19:36:01 +0200 Subject: [PATCH 04/13] Extend OcelotBuilder with overloads of the AddCustomLoadBalancer registration helper method --- .../DependencyInjection/IOcelotBuilder.cs | 20 +++++++++ .../DependencyInjection/OcelotBuilder.cs | 43 +++++++++++++++++++ .../DelegateInvokingLoadBalancerCreator.cs | 25 +++++++++++ .../DependencyInjection/OcelotBuilderTests.cs | 25 ++++++----- 4 files changed, 103 insertions(+), 10 deletions(-) create mode 100644 src/Ocelot/LoadBalancer/LoadBalancers/DelegateInvokingLoadBalancerCreator.cs diff --git a/src/Ocelot/DependencyInjection/IOcelotBuilder.cs b/src/Ocelot/DependencyInjection/IOcelotBuilder.cs index 13ec5a6a..33b3424e 100644 --- a/src/Ocelot/DependencyInjection/IOcelotBuilder.cs +++ b/src/Ocelot/DependencyInjection/IOcelotBuilder.cs @@ -3,6 +3,9 @@ using Microsoft.Extensions.DependencyInjection; using Ocelot.Middleware.Multiplexer; using System; using System.Net.Http; +using Ocelot.Configuration; +using Ocelot.LoadBalancer.LoadBalancers; +using Ocelot.ServiceDiscovery.Providers; namespace Ocelot.DependencyInjection { @@ -25,6 +28,23 @@ namespace Ocelot.DependencyInjection IOcelotBuilder AddTransientDefinedAggregator() where T : class, IDefinedAggregator; + IOcelotBuilder AddCustomLoadBalancer() + where T : ILoadBalancer, new(); + + IOcelotBuilder AddCustomLoadBalancer(Func loadBalancerFactoryFunc) + where T : ILoadBalancer; + + IOcelotBuilder AddCustomLoadBalancer(Func loadBalancerFactoryFunc) + where T : ILoadBalancer; + + IOcelotBuilder AddCustomLoadBalancer( + Func loadBalancerFactoryFunc) + where T : ILoadBalancer; + + IOcelotBuilder AddCustomLoadBalancer( + Func loadBalancerFactoryFunc) + where T : ILoadBalancer; + IOcelotBuilder AddConfigPlaceholders(); } } diff --git a/src/Ocelot/DependencyInjection/OcelotBuilder.cs b/src/Ocelot/DependencyInjection/OcelotBuilder.cs index a9290ad7..9e348183 100644 --- a/src/Ocelot/DependencyInjection/OcelotBuilder.cs +++ b/src/Ocelot/DependencyInjection/OcelotBuilder.cs @@ -1,3 +1,5 @@ +using Ocelot.ServiceDiscovery.Providers; + using Ocelot.Configuration.ChangeTracking; namespace Ocelot.DependencyInjection @@ -173,6 +175,47 @@ namespace Ocelot.DependencyInjection return this; } + public IOcelotBuilder AddCustomLoadBalancer() + where T : ILoadBalancer, new() + { + AddCustomLoadBalancer((provider, reRoute, serviceDiscoveryProvider) => new T()); + return this; + } + + public IOcelotBuilder AddCustomLoadBalancer(Func loadBalancerFactoryFunc) + where T : ILoadBalancer + { + AddCustomLoadBalancer((provider, reRoute, serviceDiscoveryProvider) => + loadBalancerFactoryFunc()); + return this; + } + + public IOcelotBuilder AddCustomLoadBalancer(Func loadBalancerFactoryFunc) + where T : ILoadBalancer + { + AddCustomLoadBalancer((provider, reRoute, serviceDiscoveryProvider) => + loadBalancerFactoryFunc(provider)); + return this; + } + + public IOcelotBuilder AddCustomLoadBalancer(Func loadBalancerFactoryFunc) + where T : ILoadBalancer + { + AddCustomLoadBalancer((provider, reRoute, serviceDiscoveryProvider) => + loadBalancerFactoryFunc(reRoute, serviceDiscoveryProvider)); + return this; + } + + public IOcelotBuilder AddCustomLoadBalancer(Func loadBalancerFactoryFunc) + where T : ILoadBalancer + { + Services.AddSingleton(provider => + new DelegateInvokingLoadBalancerCreator( + (reRoute, serviceDiscoveryProvider) => + loadBalancerFactoryFunc(provider, reRoute, serviceDiscoveryProvider))); + return this; + } + private void AddSecurity() { Services.TryAddSingleton(); diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/DelegateInvokingLoadBalancerCreator.cs b/src/Ocelot/LoadBalancer/LoadBalancers/DelegateInvokingLoadBalancerCreator.cs new file mode 100644 index 00000000..535550dc --- /dev/null +++ b/src/Ocelot/LoadBalancer/LoadBalancers/DelegateInvokingLoadBalancerCreator.cs @@ -0,0 +1,25 @@ +namespace Ocelot.LoadBalancer.LoadBalancers +{ + using System; + using Ocelot.Configuration; + using Ocelot.ServiceDiscovery.Providers; + + public class DelegateInvokingLoadBalancerCreator : ILoadBalancerCreator + where T : ILoadBalancer + { + private readonly Func _creatorFunc; + + public DelegateInvokingLoadBalancerCreator( + Func creatorFunc) + { + _creatorFunc = creatorFunc; + } + + public ILoadBalancer Create(DownstreamReRoute reRoute, IServiceDiscoveryProvider serviceProvider) + { + return _creatorFunc(reRoute, serviceProvider); + } + + public string Type => typeof(T).Name; + } +} diff --git a/test/Ocelot.UnitTests/DependencyInjection/OcelotBuilderTests.cs b/test/Ocelot.UnitTests/DependencyInjection/OcelotBuilderTests.cs index 499c2a75..f85bbb11 100644 --- a/test/Ocelot.UnitTests/DependencyInjection/OcelotBuilderTests.cs +++ b/test/Ocelot.UnitTests/DependencyInjection/OcelotBuilderTests.cs @@ -1,6 +1,8 @@ -using Ocelot.Configuration; +using System.Threading.Tasks; using Ocelot.LoadBalancer.LoadBalancers; -using Ocelot.ServiceDiscovery.Providers; +using Ocelot.Middleware; +using Ocelot.Responses; +using Ocelot.Values; namespace Ocelot.UnitTests.DependencyInjection { @@ -166,7 +168,7 @@ namespace Ocelot.UnitTests.DependencyInjection public void should_add_custom_load_balancer_creators() { this.Given(x => WhenISetUpOcelotServices()) - .When(x => AddSingletonLoadBalancerCreator()) + .When(x => AddCustomLoadBalancer()) .Then(x => ThenTheProviderIsRegisteredAndReturnsBothBuiltInAndCustomLoadBalancerCreators()) .BDDfy(); } @@ -177,10 +179,10 @@ namespace Ocelot.UnitTests.DependencyInjection _ocelotBuilder.AddSingletonDefinedAggregator(); } - private void AddSingletonLoadBalancerCreator() - where T : class, ILoadBalancerCreator + private void AddCustomLoadBalancer() + where T : ILoadBalancer, new() { - _ocelotBuilder.Services.AddSingleton(); + _ocelotBuilder.AddCustomLoadBalancer(); } private void AddTransientDefinedAggregator() @@ -266,7 +268,7 @@ namespace Ocelot.UnitTests.DependencyInjection creators.Count(c => c.GetType() == typeof(RoundRobinCreator)).ShouldBe(1); creators.Count(c => c.GetType() == typeof(CookieStickySessionsCreator)).ShouldBe(1); creators.Count(c => c.GetType() == typeof(LeastConnectionCreator)).ShouldBe(1); - creators.Count(c => c.GetType() == typeof(FakeCustomLoadBalancerCreator)).ShouldBe(1); + creators.Count(c => c.GetType() == typeof(DelegateInvokingLoadBalancerCreator)).ShouldBe(1); } private void ThenTheAggregatorsAreTransient() @@ -354,14 +356,17 @@ namespace Ocelot.UnitTests.DependencyInjection _ex.ShouldBeNull(); } - private class FakeCustomLoadBalancerCreator : ILoadBalancerCreator + private class FakeCustomLoadBalancer : ILoadBalancer { - public ILoadBalancer Create(DownstreamReRoute reRoute, IServiceDiscoveryProvider serviceProvider) + public Task> Lease(DownstreamContext context) { throw new NotImplementedException(); } - public string Type { get; } + public void Release(ServiceHostAndPort hostAndPort) + { + throw new NotImplementedException(); + } } } } From fd35ad0514bc9875cde91245066b4e2400767553 Mon Sep 17 00:00:00 2001 From: David Lievrouw Date: Wed, 10 Jul 2019 20:59:04 +0200 Subject: [PATCH 05/13] Add ability to register custom load balancers --- .../DependencyInjection/OcelotBuilderTests.cs | 44 ++++++++++++++++--- 1 file changed, 37 insertions(+), 7 deletions(-) diff --git a/test/Ocelot.UnitTests/DependencyInjection/OcelotBuilderTests.cs b/test/Ocelot.UnitTests/DependencyInjection/OcelotBuilderTests.cs index f85bbb11..577fd059 100644 --- a/test/Ocelot.UnitTests/DependencyInjection/OcelotBuilderTests.cs +++ b/test/Ocelot.UnitTests/DependencyInjection/OcelotBuilderTests.cs @@ -154,6 +154,42 @@ namespace Ocelot.UnitTests.DependencyInjection .BDDfy(); } + [Fact] + public void should_add_custom_load_balancer_creators_by_default_ctor() + { + this.Given(x => WhenISetUpOcelotServices()) + .When(x => _ocelotBuilder.AddCustomLoadBalancer()) + .Then(x => ThenTheProviderIsRegisteredAndReturnsBothBuiltInAndCustomLoadBalancerCreators()) + .BDDfy(); + } + + [Fact] + public void should_add_custom_load_balancer_creators_by_factory_method() + { + this.Given(x => WhenISetUpOcelotServices()) + .When(x => _ocelotBuilder.AddCustomLoadBalancer(() => new FakeCustomLoadBalancer())) + .Then(x => ThenTheProviderIsRegisteredAndReturnsBothBuiltInAndCustomLoadBalancerCreators()) + .BDDfy(); + } + + [Fact] + public void should_add_custom_load_balancer_creators_by_di_factory_method() + { + this.Given(x => WhenISetUpOcelotServices()) + .When(x => _ocelotBuilder.AddCustomLoadBalancer(provider => new FakeCustomLoadBalancer())) + .Then(x => ThenTheProviderIsRegisteredAndReturnsBothBuiltInAndCustomLoadBalancerCreators()) + .BDDfy(); + } + + [Fact] + public void should_add_custom_load_balancer_creators_by_factory_method_with_arguments() + { + this.Given(x => WhenISetUpOcelotServices()) + .When(x => _ocelotBuilder.AddCustomLoadBalancer((reroute, discoveryProvider) => new FakeCustomLoadBalancer())) + .Then(x => ThenTheProviderIsRegisteredAndReturnsBothBuiltInAndCustomLoadBalancerCreators()) + .BDDfy(); + } + [Fact] public void should_replace_iplaceholder() { @@ -168,7 +204,7 @@ namespace Ocelot.UnitTests.DependencyInjection public void should_add_custom_load_balancer_creators() { this.Given(x => WhenISetUpOcelotServices()) - .When(x => AddCustomLoadBalancer()) + .When(x => _ocelotBuilder.AddCustomLoadBalancer((provider, reroute, discoveryProvider) => new FakeCustomLoadBalancer())) .Then(x => ThenTheProviderIsRegisteredAndReturnsBothBuiltInAndCustomLoadBalancerCreators()) .BDDfy(); } @@ -179,12 +215,6 @@ namespace Ocelot.UnitTests.DependencyInjection _ocelotBuilder.AddSingletonDefinedAggregator(); } - private void AddCustomLoadBalancer() - where T : ILoadBalancer, new() - { - _ocelotBuilder.AddCustomLoadBalancer(); - } - private void AddTransientDefinedAggregator() where T : class, IDefinedAggregator { From 22f304cecfc25950a876afa1a96168eda7db2f42 Mon Sep 17 00:00:00 2001 From: David Lievrouw Date: Wed, 10 Jul 2019 20:59:38 +0200 Subject: [PATCH 06/13] Cover LoadBalancerFactory edge-case by a unit test. --- .../LoadBalancer/LoadBalancerFactoryTests.cs | 31 ++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerFactoryTests.cs b/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerFactoryTests.cs index 70b9a837..bd9ac6c0 100644 --- a/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerFactoryTests.cs +++ b/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerFactoryTests.cs @@ -8,6 +8,7 @@ using Ocelot.ServiceDiscovery.Providers; using Shouldly; using System.Collections.Generic; using System.Threading.Tasks; +using Ocelot.Infrastructure.RequestData; using Ocelot.Middleware; using Ocelot.Values; using TestStack.BDDfy; @@ -39,7 +40,7 @@ namespace Ocelot.UnitTests.LoadBalancer } [Fact] - public void should_return_no_load_balancer() + public void should_return_no_load_balancer_by_default() { var reRoute = new DownstreamReRouteBuilder() .WithUpstreamHttpMethod(new List { "Get" }) @@ -85,6 +86,22 @@ namespace Ocelot.UnitTests.LoadBalancer .BDDfy(); } + [Fact] + public void should_return_error_response_when_call_to_service_provider_fails() + { + var reRoute = new DownstreamReRouteBuilder() + .WithLoadBalancerOptions(new LoadBalancerOptions("FakeLoadBalancerOne", "", 0)) + .WithUpstreamHttpMethod(new List { "Get" }) + .Build(); + + this.Given(x => x.GivenAReRoute(reRoute)) + .And(x => GivenAServiceProviderConfig(new ServiceProviderConfigurationBuilder().Build())) + .And(x => x.GivenTheServiceProviderFactoryFails()) + .When(x => x.WhenIGetTheLoadBalancer()) + .Then(x => x.ThenAnErrorResponseIsReturned()) + .BDDfy(); + } + private void GivenAServiceProviderConfig(ServiceProviderConfiguration serviceProviderConfig) { _serviceProviderConfig = serviceProviderConfig; @@ -97,6 +114,13 @@ namespace Ocelot.UnitTests.LoadBalancer .Returns(new OkResponse(_serviceProvider.Object)); } + private void GivenTheServiceProviderFactoryFails() + { + _serviceProviderFactory + .Setup(x => x.Get(It.IsAny(), It.IsAny())) + .Returns(new ErrorResponse(new CannotFindDataError("For tests"))); + } + private void ThenTheServiceProviderIsCalledCorrectly() { _serviceProviderFactory @@ -118,6 +142,11 @@ namespace Ocelot.UnitTests.LoadBalancer _result.Data.ShouldBeOfType(); } + private void ThenAnErrorResponseIsReturned() + { + _result.IsError.ShouldBeTrue(); + } + private class FakeLoadBalancerCreator : ILoadBalancerCreator where T : ILoadBalancer, new() { From f9ce987cc51a4142a6a1f7065d38de66f88f07db Mon Sep 17 00:00:00 2001 From: David Lievrouw Date: Wed, 10 Jul 2019 21:11:34 +0200 Subject: [PATCH 07/13] Cover DelegateInvokingLoadBalancerCreator by unit tests. --- ...elegateInvokingLoadBalancerCreatorTests.cs | 102 ++++++++++++++++++ 1 file changed, 102 insertions(+) create mode 100644 test/Ocelot.UnitTests/LoadBalancer/DelegateInvokingLoadBalancerCreatorTests.cs diff --git a/test/Ocelot.UnitTests/LoadBalancer/DelegateInvokingLoadBalancerCreatorTests.cs b/test/Ocelot.UnitTests/LoadBalancer/DelegateInvokingLoadBalancerCreatorTests.cs new file mode 100644 index 00000000..2be4ee62 --- /dev/null +++ b/test/Ocelot.UnitTests/LoadBalancer/DelegateInvokingLoadBalancerCreatorTests.cs @@ -0,0 +1,102 @@ +using System; +using System.Threading.Tasks; +using Moq; +using Ocelot.Configuration; +using Ocelot.Configuration.Builder; +using Ocelot.LoadBalancer.LoadBalancers; +using Ocelot.Middleware; +using Ocelot.Responses; +using Ocelot.ServiceDiscovery.Providers; +using Ocelot.Values; +using Shouldly; +using TestStack.BDDfy; +using Xunit; + +namespace Ocelot.UnitTests.LoadBalancer +{ + public class DelegateInvokingLoadBalancerCreatorTests + { + private readonly DelegateInvokingLoadBalancerCreator _creator; + private readonly Func _creatorFunc; + private readonly Mock _serviceProvider; + private DownstreamReRoute _reRoute; + private ILoadBalancer _loadBalancer; + private string _typeName; + + public DelegateInvokingLoadBalancerCreatorTests() + { + _creatorFunc = (reRoute, serviceDiscoveryProvider) => + new FakeLoadBalancer(reRoute, serviceDiscoveryProvider); + _creator = new DelegateInvokingLoadBalancerCreator(_creatorFunc); + _serviceProvider = new Mock(); + } + + [Fact] + public void should_return_expected_name() + { + this.When(x => x.WhenIGetTheLoadBalancerTypeName()) + .Then(x => x.ThenTheLoadBalancerTypeIs("FakeLoadBalancer")) + .BDDfy(); + } + + [Fact] + public void should_return_result_of_specified_creator_func() + { + var reRoute = new DownstreamReRouteBuilder() + .Build(); + + this.Given(x => x.GivenAReRoute(reRoute)) + .When(x => x.WhenIGetTheLoadBalancer()) + .Then(x => x.ThenTheLoadBalancerIsReturned()) + .BDDfy(); + } + + private void GivenAReRoute(DownstreamReRoute reRoute) + { + _reRoute = reRoute; + } + + private void WhenIGetTheLoadBalancer() + { + _loadBalancer = _creator.Create(_reRoute, _serviceProvider.Object); + } + + private void WhenIGetTheLoadBalancerTypeName() + { + _typeName = _creator.Type; + } + + private void ThenTheLoadBalancerIsReturned() + where T : ILoadBalancer + { + _loadBalancer.ShouldBeOfType(); + } + + private void ThenTheLoadBalancerTypeIs(string type) + { + _typeName.ShouldBe(type); + } + + private class FakeLoadBalancer : ILoadBalancer + { + public FakeLoadBalancer(DownstreamReRoute reRoute, IServiceDiscoveryProvider serviceDiscoveryProvider) + { + ReRoute = reRoute; + ServiceDiscoveryProvider = serviceDiscoveryProvider; + } + + public DownstreamReRoute ReRoute { get; } + public IServiceDiscoveryProvider ServiceDiscoveryProvider { get; } + + public Task> Lease(DownstreamContext context) + { + throw new NotImplementedException(); + } + + public void Release(ServiceHostAndPort hostAndPort) + { + throw new NotImplementedException(); + } + } + } +} From 2606d919135ddb731fdc116c66f535a877ddfb19 Mon Sep 17 00:00:00 2001 From: David Lievrouw Date: Wed, 10 Jul 2019 21:13:21 +0200 Subject: [PATCH 08/13] Fix unit test naming issue. --- .../LoadBalancer/CookieStickySessionsCreatorTests.cs | 4 ++-- .../LoadBalancer/LeastConnectionCreatorTests.cs | 4 ++-- .../LoadBalancer/NoLoadBalancerCreatorTests.cs | 4 ++-- test/Ocelot.UnitTests/LoadBalancer/RoundRobinCreatorTests.cs | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/test/Ocelot.UnitTests/LoadBalancer/CookieStickySessionsCreatorTests.cs b/test/Ocelot.UnitTests/LoadBalancer/CookieStickySessionsCreatorTests.cs index 20f65da4..bf004010 100644 --- a/test/Ocelot.UnitTests/LoadBalancer/CookieStickySessionsCreatorTests.cs +++ b/test/Ocelot.UnitTests/LoadBalancer/CookieStickySessionsCreatorTests.cs @@ -24,7 +24,7 @@ namespace Ocelot.UnitTests.LoadBalancer } [Fact] - public void should_return_expected_name() + public void should_return_instance_of_expected_load_balancer_type() { var reRoute = new DownstreamReRouteBuilder() .WithLoadBalancerOptions(new LoadBalancerOptions("myType", "myKey", 1000)) @@ -37,7 +37,7 @@ namespace Ocelot.UnitTests.LoadBalancer } [Fact] - public void should_return_instance_of_expected_load_balancer_type() + public void should_return_expected_name() { this.When(x => x.WhenIGetTheLoadBalancerTypeName()) .Then(x => x.ThenTheLoadBalancerTypeIs("CookieStickySessions")) diff --git a/test/Ocelot.UnitTests/LoadBalancer/LeastConnectionCreatorTests.cs b/test/Ocelot.UnitTests/LoadBalancer/LeastConnectionCreatorTests.cs index 507958fd..37d678cd 100644 --- a/test/Ocelot.UnitTests/LoadBalancer/LeastConnectionCreatorTests.cs +++ b/test/Ocelot.UnitTests/LoadBalancer/LeastConnectionCreatorTests.cs @@ -24,7 +24,7 @@ namespace Ocelot.UnitTests.LoadBalancer } [Fact] - public void should_return_expected_name() + public void should_return_instance_of_expected_load_balancer_type() { var reRoute = new DownstreamReRouteBuilder() .WithServiceName("myService") @@ -37,7 +37,7 @@ namespace Ocelot.UnitTests.LoadBalancer } [Fact] - public void should_return_instance_of_expected_load_balancer_type() + public void should_return_expected_name() { this.When(x => x.WhenIGetTheLoadBalancerTypeName()) .Then(x => x.ThenTheLoadBalancerTypeIs("LeastConnection")) diff --git a/test/Ocelot.UnitTests/LoadBalancer/NoLoadBalancerCreatorTests.cs b/test/Ocelot.UnitTests/LoadBalancer/NoLoadBalancerCreatorTests.cs index 6b61c5a9..1cdcb3ab 100644 --- a/test/Ocelot.UnitTests/LoadBalancer/NoLoadBalancerCreatorTests.cs +++ b/test/Ocelot.UnitTests/LoadBalancer/NoLoadBalancerCreatorTests.cs @@ -24,7 +24,7 @@ namespace Ocelot.UnitTests.LoadBalancer } [Fact] - public void should_return_expected_name() + public void should_return_instance_of_expected_load_balancer_type() { var reRoute = new DownstreamReRouteBuilder() .Build(); @@ -36,7 +36,7 @@ namespace Ocelot.UnitTests.LoadBalancer } [Fact] - public void should_return_instance_of_expected_load_balancer_type() + public void should_return_expected_name() { this.When(x => x.WhenIGetTheLoadBalancerTypeName()) .Then(x => x.ThenTheLoadBalancerTypeIs("NoLoadBalancer")) diff --git a/test/Ocelot.UnitTests/LoadBalancer/RoundRobinCreatorTests.cs b/test/Ocelot.UnitTests/LoadBalancer/RoundRobinCreatorTests.cs index 80d43568..83f7cb6f 100644 --- a/test/Ocelot.UnitTests/LoadBalancer/RoundRobinCreatorTests.cs +++ b/test/Ocelot.UnitTests/LoadBalancer/RoundRobinCreatorTests.cs @@ -24,7 +24,7 @@ namespace Ocelot.UnitTests.LoadBalancer } [Fact] - public void should_return_expected_name() + public void should_return_instance_of_expected_load_balancer_type() { var reRoute = new DownstreamReRouteBuilder() .Build(); @@ -36,7 +36,7 @@ namespace Ocelot.UnitTests.LoadBalancer } [Fact] - public void should_return_instance_of_expected_load_balancer_type() + public void should_return_expected_name() { this.When(x => x.WhenIGetTheLoadBalancerTypeName()) .Then(x => x.ThenTheLoadBalancerTypeIs("RoundRobin")) From 1223c1985f9125dc6c38e56ea0def33d34af5896 Mon Sep 17 00:00:00 2001 From: David Lievrouw Date: Thu, 11 Jul 2019 17:10:19 +0200 Subject: [PATCH 09/13] Add comment indicating that the FakeCustomLoadBalancer implementation is not relevant for OcelotBuilder tests. --- test/Ocelot.UnitTests/DependencyInjection/OcelotBuilderTests.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/Ocelot.UnitTests/DependencyInjection/OcelotBuilderTests.cs b/test/Ocelot.UnitTests/DependencyInjection/OcelotBuilderTests.cs index 577fd059..1a3ac2cb 100644 --- a/test/Ocelot.UnitTests/DependencyInjection/OcelotBuilderTests.cs +++ b/test/Ocelot.UnitTests/DependencyInjection/OcelotBuilderTests.cs @@ -390,11 +390,13 @@ namespace Ocelot.UnitTests.DependencyInjection { public Task> Lease(DownstreamContext context) { + // Not relevant for these tests throw new NotImplementedException(); } public void Release(ServiceHostAndPort hostAndPort) { + // Not relevant for these tests throw new NotImplementedException(); } } From 2843cdbc0bc9513475b17854648df7face187b3f Mon Sep 17 00:00:00 2001 From: TomPallister Date: Mon, 13 Apr 2020 11:13:28 +0100 Subject: [PATCH 10/13] removed tasks we dont need --- .../LoadBalancer/LoadBalancers/ILoadBalancerFactory.cs | 3 +-- .../LoadBalancer/LoadBalancers/ILoadBalancerHouse.cs | 3 +-- .../LoadBalancer/LoadBalancers/LoadBalancerFactory.cs | 4 ++-- .../LoadBalancer/LoadBalancers/LoadBalancerHouse.cs | 9 ++++----- .../LoadBalancer/Middleware/LoadBalancingMiddleware.cs | 2 +- .../LoadBalancer/LoadBalancerFactoryTests.cs | 2 +- .../LoadBalancer/LoadBalancerHouseTests.cs | 10 +++++----- .../LoadBalancer/LoadBalancerMiddlewareTests.cs | 4 ++-- 8 files changed, 17 insertions(+), 20 deletions(-) diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/ILoadBalancerFactory.cs b/src/Ocelot/LoadBalancer/LoadBalancers/ILoadBalancerFactory.cs index df115c41..9053b6d4 100644 --- a/src/Ocelot/LoadBalancer/LoadBalancers/ILoadBalancerFactory.cs +++ b/src/Ocelot/LoadBalancer/LoadBalancers/ILoadBalancerFactory.cs @@ -2,10 +2,9 @@ { using Ocelot.Configuration; using Ocelot.Responses; - using System.Threading.Tasks; public interface ILoadBalancerFactory { - Task> Get(DownstreamReRoute reRoute, ServiceProviderConfiguration config); + Response Get(DownstreamReRoute reRoute, ServiceProviderConfiguration config); } } diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/ILoadBalancerHouse.cs b/src/Ocelot/LoadBalancer/LoadBalancers/ILoadBalancerHouse.cs index 92eb0e9e..b5cfcfc0 100644 --- a/src/Ocelot/LoadBalancer/LoadBalancers/ILoadBalancerHouse.cs +++ b/src/Ocelot/LoadBalancer/LoadBalancers/ILoadBalancerHouse.cs @@ -1,11 +1,10 @@ using Ocelot.Configuration; using Ocelot.Responses; -using System.Threading.Tasks; namespace Ocelot.LoadBalancer.LoadBalancers { public interface ILoadBalancerHouse { - Task> Get(DownstreamReRoute reRoute, ServiceProviderConfiguration config); + Response Get(DownstreamReRoute reRoute, ServiceProviderConfiguration config); } } diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/LoadBalancerFactory.cs b/src/Ocelot/LoadBalancer/LoadBalancers/LoadBalancerFactory.cs index 2b7e71e2..6f0fd5e9 100644 --- a/src/Ocelot/LoadBalancer/LoadBalancers/LoadBalancerFactory.cs +++ b/src/Ocelot/LoadBalancer/LoadBalancers/LoadBalancerFactory.cs @@ -18,7 +18,7 @@ _loadBalancerCreators = loadBalancerCreators; } - public Task> Get(DownstreamReRoute reRoute, ServiceProviderConfiguration config) + public Response Get(DownstreamReRoute reRoute, ServiceProviderConfiguration config) { var serviceProviderFactoryResponse = _serviceProviderFactory.Get(config, reRoute); @@ -36,7 +36,7 @@ response = new OkResponse(createdLoadBalancer); } - return Task.FromResult(response); + return response; } } } diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/LoadBalancerHouse.cs b/src/Ocelot/LoadBalancer/LoadBalancers/LoadBalancerHouse.cs index 8f7e8135..2113457a 100644 --- a/src/Ocelot/LoadBalancer/LoadBalancers/LoadBalancerHouse.cs +++ b/src/Ocelot/LoadBalancer/LoadBalancers/LoadBalancerHouse.cs @@ -3,7 +3,6 @@ using Ocelot.Responses; using System; using System.Collections.Concurrent; using System.Collections.Generic; -using System.Threading.Tasks; namespace Ocelot.LoadBalancer.LoadBalancers { @@ -18,7 +17,7 @@ namespace Ocelot.LoadBalancer.LoadBalancers _loadBalancers = new ConcurrentDictionary(); } - public async Task> Get(DownstreamReRoute reRoute, ServiceProviderConfiguration config) + public Response Get(DownstreamReRoute reRoute, ServiceProviderConfiguration config) { try { @@ -30,7 +29,7 @@ namespace Ocelot.LoadBalancer.LoadBalancers if (reRoute.LoadBalancerOptions.Type != loadBalancer.GetType().Name) { - result = await _factory.Get(reRoute, config); + result = _factory.Get(reRoute, config); if (result.IsError) { return new ErrorResponse(result.Errors); @@ -43,7 +42,7 @@ namespace Ocelot.LoadBalancer.LoadBalancers return new OkResponse(loadBalancer); } - result = await _factory.Get(reRoute, config); + result = _factory.Get(reRoute, config); if (result.IsError) { @@ -58,7 +57,7 @@ namespace Ocelot.LoadBalancer.LoadBalancers { return new ErrorResponse(new List() { - new UnableToFindLoadBalancerError($"unabe to find load balancer for {reRoute.LoadBalancerKey} exception is {ex}") + new UnableToFindLoadBalancerError($"unabe to find load balancer for {reRoute.LoadBalancerKey} exception is {ex}"), }); } } diff --git a/src/Ocelot/LoadBalancer/Middleware/LoadBalancingMiddleware.cs b/src/Ocelot/LoadBalancer/Middleware/LoadBalancingMiddleware.cs index 646af5fe..bcf9fe80 100644 --- a/src/Ocelot/LoadBalancer/Middleware/LoadBalancingMiddleware.cs +++ b/src/Ocelot/LoadBalancer/Middleware/LoadBalancingMiddleware.cs @@ -22,7 +22,7 @@ namespace Ocelot.LoadBalancer.Middleware public async Task Invoke(DownstreamContext context) { - var loadBalancer = await _loadBalancerHouse.Get(context.DownstreamReRoute, context.Configuration.ServiceProviderConfiguration); + var loadBalancer = _loadBalancerHouse.Get(context.DownstreamReRoute, context.Configuration.ServiceProviderConfiguration); if (loadBalancer.IsError) { Logger.LogDebug("there was an error retriving the loadbalancer, setting pipeline error"); diff --git a/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerFactoryTests.cs b/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerFactoryTests.cs index bd9ac6c0..a581647f 100644 --- a/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerFactoryTests.cs +++ b/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerFactoryTests.cs @@ -134,7 +134,7 @@ namespace Ocelot.UnitTests.LoadBalancer private void WhenIGetTheLoadBalancer() { - _result = _factory.Get(_reRoute, _serviceProviderConfig).Result; + _result = _factory.Get(_reRoute, _serviceProviderConfig); } private void ThenTheLoadBalancerIsReturned() diff --git a/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerHouseTests.cs b/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerHouseTests.cs index 597a79f0..be43b74e 100644 --- a/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerHouseTests.cs +++ b/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerHouseTests.cs @@ -111,8 +111,8 @@ namespace Ocelot.UnitTests.LoadBalancer private void WhenIGetTheReRouteWithTheSameKeyButDifferentLoadBalancer(DownstreamReRoute reRoute) { _reRoute = reRoute; - _factory.Setup(x => x.Get(_reRoute, _serviceProviderConfig)).ReturnsAsync(new OkResponse(new LeastConnection(null, null))); - _getResult = _loadBalancerHouse.Get(_reRoute, _serviceProviderConfig).Result; + _factory.Setup(x => x.Get(_reRoute, _serviceProviderConfig)).Returns(new OkResponse(new LeastConnection(null, null))); + _getResult = _loadBalancerHouse.Get(_reRoute, _serviceProviderConfig); } private void ThenAnErrorIsReturned() @@ -138,13 +138,13 @@ namespace Ocelot.UnitTests.LoadBalancer { _reRoute = reRoute; _loadBalancer = loadBalancer; - _factory.Setup(x => x.Get(_reRoute, _serviceProviderConfig)).ReturnsAsync(new OkResponse(loadBalancer)); - _getResult = _loadBalancerHouse.Get(reRoute, _serviceProviderConfig).Result; + _factory.Setup(x => x.Get(_reRoute, _serviceProviderConfig)).Returns(new OkResponse(loadBalancer)); + _getResult = _loadBalancerHouse.Get(reRoute, _serviceProviderConfig); } private void WhenWeGetTheLoadBalancer(DownstreamReRoute reRoute) { - _getResult = _loadBalancerHouse.Get(reRoute, _serviceProviderConfig).Result; + _getResult = _loadBalancerHouse.Get(reRoute, _serviceProviderConfig); } private void ThenItIsReturned() diff --git a/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerMiddlewareTests.cs b/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerMiddlewareTests.cs index 1a6202cf..688d43f0 100644 --- a/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerMiddlewareTests.cs +++ b/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerMiddlewareTests.cs @@ -182,7 +182,7 @@ namespace Ocelot.UnitTests.LoadBalancer { _loadBalancerHouse .Setup(x => x.Get(It.IsAny(), It.IsAny())) - .ReturnsAsync(new OkResponse(_loadBalancer.Object)); + .Returns(new OkResponse(_loadBalancer.Object)); } private void GivenTheLoadBalancerHouseReturnsAnError() @@ -194,7 +194,7 @@ namespace Ocelot.UnitTests.LoadBalancer _loadBalancerHouse .Setup(x => x.Get(It.IsAny(), It.IsAny())) - .ReturnsAsync(_getLoadBalancerHouseError); + .Returns(_getLoadBalancerHouseError); } private void ThenAnErrorStatingLoadBalancerCouldNotBeFoundIsSetOnPipeline() From b300ed9aec7b1866d14d437cd67f2923425b809e Mon Sep 17 00:00:00 2001 From: TomPallister Date: Mon, 13 Apr 2020 11:14:28 +0100 Subject: [PATCH 11/13] small refactor --- .../LoadBalancers/LoadBalancerFactory.cs | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/LoadBalancerFactory.cs b/src/Ocelot/LoadBalancer/LoadBalancers/LoadBalancerFactory.cs index 6f0fd5e9..f005bb2f 100644 --- a/src/Ocelot/LoadBalancer/LoadBalancers/LoadBalancerFactory.cs +++ b/src/Ocelot/LoadBalancer/LoadBalancers/LoadBalancerFactory.cs @@ -4,7 +4,6 @@ using System.Linq; using Ocelot.Configuration; using Ocelot.Responses; - using System.Threading.Tasks; using Ocelot.ServiceDiscovery; public class LoadBalancerFactory : ILoadBalancerFactory @@ -22,21 +21,16 @@ { var serviceProviderFactoryResponse = _serviceProviderFactory.Get(config, reRoute); - Response response; if (serviceProviderFactoryResponse.IsError) { - response = new ErrorResponse(serviceProviderFactoryResponse.Errors); - } - else - { - var serviceProvider = serviceProviderFactoryResponse.Data; - var requestedType = reRoute.LoadBalancerOptions?.Type ?? nameof(NoLoadBalancer); - var applicableCreator = _loadBalancerCreators.Single(c => c.Type == requestedType); - var createdLoadBalancer = applicableCreator.Create(reRoute, serviceProvider); - response = new OkResponse(createdLoadBalancer); + return new ErrorResponse(serviceProviderFactoryResponse.Errors); } - return response; + var serviceProvider = serviceProviderFactoryResponse.Data; + var requestedType = reRoute.LoadBalancerOptions?.Type ?? nameof(NoLoadBalancer); + var applicableCreator = _loadBalancerCreators.Single(c => c.Type == requestedType); + var createdLoadBalancer = applicableCreator.Create(reRoute, serviceProvider); + return new OkResponse(createdLoadBalancer); } } } From c9483cdad66dd37ff2440fa73d50bc4953a11d87 Mon Sep 17 00:00:00 2001 From: TomPallister Date: Mon, 13 Apr 2020 12:05:55 +0100 Subject: [PATCH 12/13] tests to handle some error cases and docs --- docs/features/loadbalancer.rst | 109 ++++++++++++++++++ src/Ocelot/Errors/OcelotErrorCode.cs | 1 + .../CouldNotFindLoadBalancerCreator.cs | 12 ++ .../LoadBalancers/LoadBalancerFactory.cs | 8 +- .../Responder/ErrorsToHttpStatusCodeMapper.cs | 3 +- .../LoadBalancerTests.cs | 88 ++++++++++++++ test/Ocelot.AcceptanceTests/Steps.cs | 36 ++++++ .../LoadBalancer/LoadBalancerFactoryTests.cs | 24 +++- .../ErrorsToHttpStatusCodeMapperTests.cs | 5 +- 9 files changed, 281 insertions(+), 5 deletions(-) create mode 100644 src/Ocelot/LoadBalancer/LoadBalancers/CouldNotFindLoadBalancerCreator.cs diff --git a/docs/features/loadbalancer.rst b/docs/features/loadbalancer.rst index 70706033..1fd54d17 100644 --- a/docs/features/loadbalancer.rst +++ b/docs/features/loadbalancer.rst @@ -108,3 +108,112 @@ subsequent requests. This means the sessions will be stuck across ReRoutes. Please note that if you give more than one DownstreamHostAndPort or you are using a Service Discovery provider such as Consul and this returns more than one service then CookieStickySessions uses round robin to select the next server. This is hard coded at the moment but could be changed. + +Custom Load Balancers +^^^^^^^^^^^^^^^^^^^^ + +`DavidLievrouw >> _services; + private readonly object _lock = new object(); + + private int _last; + + public CustomLoadBalancer(Func>> services) + { + _services = services; + } + + public async Task> Lease(DownstreamContext downstreamContext) + { + var services = await _services(); + lock (_lock) + { + if (_last >= services.Count) + { + _last = 0; + } + + var next = services[_last]; + _last++; + return new OkResponse(next.HostAndPort); + } + } + + public void Release(ServiceHostAndPort hostAndPort) + { + } + } + +Finally you need to register this class with Ocelot. I have used the most complex example below to show all of the data / types that can be passed into the factory that creates load balancers. + +.. code-block:: csharp + + Func loadBalancerFactoryFunc = (serviceProvider, reRoute, serviceDiscoveryProvider) => new CustomLoadBalancer(serviceDiscoveryProvider.Get); + + s.AddOcelot() + .AddCustomLoadBalancer(loadBalancerFactoryFunc); + +However there is a much simpler example that will work the same. + +.. code-block:: csharp + + s.AddOcelot() + .AddCustomLoadBalancer(); + +There are numerous extension methods to add a custom load balancer and the interface is as follows. + +.. code-block:: csharp + + IOcelotBuilder AddCustomLoadBalancer() + where T : ILoadBalancer, new(); + + IOcelotBuilder AddCustomLoadBalancer(Func loadBalancerFactoryFunc) + where T : ILoadBalancer; + + IOcelotBuilder AddCustomLoadBalancer(Func loadBalancerFactoryFunc) + where T : ILoadBalancer; + + IOcelotBuilder AddCustomLoadBalancer( + Func loadBalancerFactoryFunc) + where T : ILoadBalancer; + + IOcelotBuilder AddCustomLoadBalancer( + Func loadBalancerFactoryFunc) + where T : ILoadBalancer; + +When you enable custom load balancers Ocelot looks up your load balancer by its class name when it decides if it should do load balancing. If it finds a match it will load balance your request. If Ocelot cannot match the load balancer type in your configuration with the name of registered load balancer class then you will receive a HTTP 500 internal server error. + +Remember if you specify no load balancer in your config Ocelot will not try and load balance. \ No newline at end of file diff --git a/src/Ocelot/Errors/OcelotErrorCode.cs b/src/Ocelot/Errors/OcelotErrorCode.cs index 6ca67e7d..92c125a7 100644 --- a/src/Ocelot/Errors/OcelotErrorCode.cs +++ b/src/Ocelot/Errors/OcelotErrorCode.cs @@ -41,5 +41,6 @@ QuotaExceededError = 36, RequestCanceled = 37, ConnectionToDownstreamServiceError = 38, + CouldNotFindLoadBalancerCreator = 39, } } diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/CouldNotFindLoadBalancerCreator.cs b/src/Ocelot/LoadBalancer/LoadBalancers/CouldNotFindLoadBalancerCreator.cs new file mode 100644 index 00000000..9c95e239 --- /dev/null +++ b/src/Ocelot/LoadBalancer/LoadBalancers/CouldNotFindLoadBalancerCreator.cs @@ -0,0 +1,12 @@ +namespace Ocelot.LoadBalancer.LoadBalancers +{ + using Errors; + + public class CouldNotFindLoadBalancerCreator : Error + { + public CouldNotFindLoadBalancerCreator(string message) + : base(message, OcelotErrorCode.CouldNotFindLoadBalancerCreator) + { + } + } +} diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/LoadBalancerFactory.cs b/src/Ocelot/LoadBalancer/LoadBalancers/LoadBalancerFactory.cs index f005bb2f..93cd0adb 100644 --- a/src/Ocelot/LoadBalancer/LoadBalancers/LoadBalancerFactory.cs +++ b/src/Ocelot/LoadBalancer/LoadBalancers/LoadBalancerFactory.cs @@ -28,7 +28,13 @@ var serviceProvider = serviceProviderFactoryResponse.Data; var requestedType = reRoute.LoadBalancerOptions?.Type ?? nameof(NoLoadBalancer); - var applicableCreator = _loadBalancerCreators.Single(c => c.Type == requestedType); + var applicableCreator = _loadBalancerCreators.SingleOrDefault(c => c.Type == requestedType); + + if (applicableCreator == null) + { + return new ErrorResponse(new CouldNotFindLoadBalancerCreator($"Could not find load balancer creator for Type: {requestedType}, please check your config specified the correct load balancer and that you have registered a class with the same name.")); + } + var createdLoadBalancer = applicableCreator.Create(reRoute, serviceProvider); return new OkResponse(createdLoadBalancer); } diff --git a/src/Ocelot/Responder/ErrorsToHttpStatusCodeMapper.cs b/src/Ocelot/Responder/ErrorsToHttpStatusCodeMapper.cs index 6b0ee6cc..6bb85ba9 100644 --- a/src/Ocelot/Responder/ErrorsToHttpStatusCodeMapper.cs +++ b/src/Ocelot/Responder/ErrorsToHttpStatusCodeMapper.cs @@ -45,7 +45,8 @@ namespace Ocelot.Responder return 502; } - if (errors.Any(e => e.Code == OcelotErrorCode.UnableToCompleteRequestError)) + if (errors.Any(e => e.Code == OcelotErrorCode.UnableToCompleteRequestError + || e.Code == OcelotErrorCode.CouldNotFindLoadBalancerCreator)) { return 500; } diff --git a/test/Ocelot.AcceptanceTests/LoadBalancerTests.cs b/test/Ocelot.AcceptanceTests/LoadBalancerTests.cs index 3c62afea..4d0c663a 100644 --- a/test/Ocelot.AcceptanceTests/LoadBalancerTests.cs +++ b/test/Ocelot.AcceptanceTests/LoadBalancerTests.cs @@ -6,7 +6,13 @@ namespace Ocelot.AcceptanceTests using Shouldly; using System; using System.Collections.Generic; + using System.Threading.Tasks; + using Configuration; + using Middleware; + using Responses; + using ServiceDiscovery.Providers; using TestStack.BDDfy; + using Values; using Xunit; public class LoadBalancerTests : IDisposable @@ -122,6 +128,88 @@ namespace Ocelot.AcceptanceTests .BDDfy(); } + [Fact] + public void should_load_balance_request_with_custom_load_balancer() + { + var downstreamPortOne = RandomPortFinder.GetRandomPort(); + var downstreamPortTwo = RandomPortFinder.GetRandomPort(); + var downstreamServiceOneUrl = $"http://localhost:{downstreamPortOne}"; + var downstreamServiceTwoUrl = $"http://localhost:{downstreamPortTwo}"; + + var configuration = new FileConfiguration + { + ReRoutes = new List + { + new FileReRoute + { + DownstreamPathTemplate = "/", + DownstreamScheme = "http", + UpstreamPathTemplate = "/", + UpstreamHttpMethod = new List { "Get" }, + LoadBalancerOptions = new FileLoadBalancerOptions { Type = nameof(CustomLoadBalancer) }, + DownstreamHostAndPorts = new List + { + new FileHostAndPort + { + Host = "localhost", + Port = downstreamPortOne, + }, + new FileHostAndPort + { + Host = "localhost", + Port = downstreamPortTwo, + }, + }, + }, + }, + GlobalConfiguration = new FileGlobalConfiguration(), + }; + + Func loadBalancerFactoryFunc = (serviceProvider, reRoute, serviceDiscoveryProvider) => new CustomLoadBalancer(serviceDiscoveryProvider.Get); + + this.Given(x => x.GivenProductServiceOneIsRunning(downstreamServiceOneUrl, 200)) + .And(x => x.GivenProductServiceTwoIsRunning(downstreamServiceTwoUrl, 200)) + .And(x => _steps.GivenThereIsAConfiguration(configuration)) + .And(x => _steps.GivenOcelotIsRunningWithCustomLoadBalancer(loadBalancerFactoryFunc)) + .When(x => _steps.WhenIGetUrlOnTheApiGatewayMultipleTimes("/", 50)) + .Then(x => x.ThenTheTwoServicesShouldHaveBeenCalledTimes(50)) + .And(x => x.ThenBothServicesCalledRealisticAmountOfTimes(24, 26)) + .BDDfy(); + } + + private class CustomLoadBalancer : ILoadBalancer + { + private readonly Func>> _services; + private readonly object _lock = new object(); + + private int _last; + + public CustomLoadBalancer(Func>> services) + { + _services = services; + } + + public async Task> Lease(DownstreamContext downstreamContext) + { + var services = await _services(); + lock (_lock) + { + if (_last >= services.Count) + { + _last = 0; + } + + var next = services[_last]; + _last++; + return new OkResponse(next.HostAndPort); + } + } + + public void Release(ServiceHostAndPort hostAndPort) + { + } + } + private void ThenBothServicesCalledRealisticAmountOfTimes(int bottom, int top) { _counterOne.ShouldBeInRange(bottom, top); diff --git a/test/Ocelot.AcceptanceTests/Steps.cs b/test/Ocelot.AcceptanceTests/Steps.cs index 976b7422..317b2d1d 100644 --- a/test/Ocelot.AcceptanceTests/Steps.cs +++ b/test/Ocelot.AcceptanceTests/Steps.cs @@ -39,6 +39,9 @@ namespace Ocelot.AcceptanceTests using System.Text; using System.Threading; using System.Threading.Tasks; + using Configuration; + using LoadBalancer.LoadBalancers; + using ServiceDiscovery.Providers; using static Ocelot.AcceptanceTests.HttpDelegatingHandlersTests; using ConfigurationBuilder = Microsoft.Extensions.Configuration.ConfigurationBuilder; using CookieHeaderValue = Microsoft.Net.Http.Headers.CookieHeaderValue; @@ -255,6 +258,39 @@ namespace Ocelot.AcceptanceTests _ocelotClient = _ocelotServer.CreateClient(); } + /// + /// This is annoying cos it should be in the constructor but we need to set up the file before calling startup so its a step. + /// + public void GivenOcelotIsRunningWithCustomLoadBalancer(Func loadBalancerFactoryFunc) + where T : ILoadBalancer + { + _webHostBuilder = new WebHostBuilder(); + + _webHostBuilder + .ConfigureAppConfiguration((hostingContext, config) => + { + config.SetBasePath(hostingContext.HostingEnvironment.ContentRootPath); + var env = hostingContext.HostingEnvironment; + config.AddJsonFile("appsettings.json", optional: true, reloadOnChange: false) + .AddJsonFile($"appsettings.{env.EnvironmentName}.json", optional: true, reloadOnChange: false); + config.AddJsonFile("ocelot.json", false, false); + config.AddEnvironmentVariables(); + }) + .ConfigureServices(s => + { + s.AddOcelot() + .AddCustomLoadBalancer(loadBalancerFactoryFunc); + }) + .Configure(app => + { + app.UseOcelot().Wait(); + }); + + _ocelotServer = new TestServer(_webHostBuilder); + + _ocelotClient = _ocelotServer.CreateClient(); + } + public void GivenOcelotIsRunningWithConsul() { _webHostBuilder = new WebHostBuilder(); diff --git a/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerFactoryTests.cs b/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerFactoryTests.cs index a581647f..a08a808f 100644 --- a/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerFactoryTests.cs +++ b/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerFactoryTests.cs @@ -69,7 +69,24 @@ namespace Ocelot.UnitTests.LoadBalancer .Then(x => x.ThenTheLoadBalancerIsReturned()) .BDDfy(); } - + + [Fact] + public void should_return_error_response_if_cannot_find_load_balancer_creator() + { + var reRoute = new DownstreamReRouteBuilder() + .WithLoadBalancerOptions(new LoadBalancerOptions("DoesntExistLoadBalancer", "", 0)) + .WithUpstreamHttpMethod(new List { "Get" }) + .Build(); + + this.Given(x => x.GivenAReRoute(reRoute)) + .And(x => GivenAServiceProviderConfig(new ServiceProviderConfigurationBuilder().Build())) + .And(x => x.GivenTheServiceProviderFactoryReturns()) + .When(x => x.WhenIGetTheLoadBalancer()) + .Then(x => x.ThenAnErrorResponseIsReturned()) + .And(x => x.ThenTheErrorMessageIsCorrect()) + .BDDfy(); + } + [Fact] public void should_call_service_provider() { @@ -147,6 +164,11 @@ namespace Ocelot.UnitTests.LoadBalancer _result.IsError.ShouldBeTrue(); } + private void ThenTheErrorMessageIsCorrect() + { + _result.Errors[0].Message.ShouldBe("Could not find load balancer creator for Type: DoesntExistLoadBalancer, please check your config specified the correct load balancer and that you have registered a class with the same name."); + } + private class FakeLoadBalancerCreator : ILoadBalancerCreator where T : ILoadBalancer, new() { diff --git a/test/Ocelot.UnitTests/Responder/ErrorsToHttpStatusCodeMapperTests.cs b/test/Ocelot.UnitTests/Responder/ErrorsToHttpStatusCodeMapperTests.cs index d662f86f..da2898fc 100644 --- a/test/Ocelot.UnitTests/Responder/ErrorsToHttpStatusCodeMapperTests.cs +++ b/test/Ocelot.UnitTests/Responder/ErrorsToHttpStatusCodeMapperTests.cs @@ -47,6 +47,7 @@ namespace Ocelot.UnitTests.Responder [Theory] [InlineData(OcelotErrorCode.UnableToCompleteRequestError)] + [InlineData(OcelotErrorCode.CouldNotFindLoadBalancerCreator)] public void should_return_internal_server_error(OcelotErrorCode errorCode) { ShouldMapErrorToStatusCode(errorCode, HttpStatusCode.InternalServerError); @@ -120,7 +121,7 @@ namespace Ocelot.UnitTests.Responder var errors = new List { OcelotErrorCode.CannotAddDataError, - OcelotErrorCode.RequestTimedOutError + OcelotErrorCode.RequestTimedOutError, }; ShouldMapErrorsToStatusCode(errors, HttpStatusCode.ServiceUnavailable); @@ -132,7 +133,7 @@ namespace Ocelot.UnitTests.Responder // If this test fails then it's because the number of error codes has changed. // You should make the appropriate changes to the test cases here to ensure // they cover all the error codes, and then modify this assertion. - Enum.GetNames(typeof(OcelotErrorCode)).Length.ShouldBe(39, "Looks like the number of error codes has changed. Do you need to modify ErrorsToHttpStatusCodeMapper?"); + Enum.GetNames(typeof(OcelotErrorCode)).Length.ShouldBe(40, "Looks like the number of error codes has changed. Do you need to modify ErrorsToHttpStatusCodeMapper?"); } private void ShouldMapErrorToStatusCode(OcelotErrorCode errorCode, HttpStatusCode expectedHttpStatusCode) From 7408060fba69bdf15bb32dbfbff1b7641004e21c Mon Sep 17 00:00:00 2001 From: TomPallister Date: Mon, 13 Apr 2020 12:46:14 +0100 Subject: [PATCH 13/13] more error handling and docs --- docs/features/loadbalancer.rst | 2 +- src/Ocelot/Errors/OcelotErrorCode.cs | 1 + .../CookieStickySessionsCreator.cs | 7 +-- .../DelegateInvokingLoadBalancerCreator.cs | 13 ++++- .../ErrorInvokingLoadBalancerCreator.cs | 12 +++++ .../LoadBalancers/ILoadBalancerCreator.cs | 11 ++-- .../LoadBalancers/LeastConnectionCreator.cs | 5 +- .../LoadBalancers/LoadBalancerFactory.cs | 10 +++- .../LoadBalancers/NoLoadBalancerCreator.cs | 5 +- .../LoadBalancers/RoundRobinCreator.cs | 7 +-- .../Responder/ErrorsToHttpStatusCodeMapper.cs | 3 +- .../CookieStickySessionsCreatorTests.cs | 25 ++++----- ...elegateInvokingLoadBalancerCreatorTests.cs | 33 ++++++++++-- .../LeastConnectionCreatorTests.cs | 25 ++++----- .../LoadBalancer/LoadBalancerFactoryTests.cs | 53 ++++++++++++++++++- .../NoLoadBalancerCreatorTests.cs | 25 ++++----- .../LoadBalancer/RoundRobinCreatorTests.cs | 25 ++++----- .../ErrorsToHttpStatusCodeMapperTests.cs | 3 +- 18 files changed, 189 insertions(+), 76 deletions(-) create mode 100644 src/Ocelot/LoadBalancer/LoadBalancers/ErrorInvokingLoadBalancerCreator.cs diff --git a/docs/features/loadbalancer.rst b/docs/features/loadbalancer.rst index 1fd54d17..02905094 100644 --- a/docs/features/loadbalancer.rst +++ b/docs/features/loadbalancer.rst @@ -214,6 +214,6 @@ There are numerous extension methods to add a custom load balancer and the inter Func loadBalancerFactoryFunc) where T : ILoadBalancer; -When you enable custom load balancers Ocelot looks up your load balancer by its class name when it decides if it should do load balancing. If it finds a match it will load balance your request. If Ocelot cannot match the load balancer type in your configuration with the name of registered load balancer class then you will receive a HTTP 500 internal server error. +When you enable custom load balancers Ocelot looks up your load balancer by its class name when it decides if it should do load balancing. If it finds a match it will use your load balaner to load balance. If Ocelot cannot match the load balancer type in your configuration with the name of registered load balancer class then you will receive a HTTP 500 internal server error. If your load balancer factory throw an exception when Ocelot calls it you will receive a HTTP 500 internal server error. Remember if you specify no load balancer in your config Ocelot will not try and load balance. \ No newline at end of file diff --git a/src/Ocelot/Errors/OcelotErrorCode.cs b/src/Ocelot/Errors/OcelotErrorCode.cs index 92c125a7..2ca44e70 100644 --- a/src/Ocelot/Errors/OcelotErrorCode.cs +++ b/src/Ocelot/Errors/OcelotErrorCode.cs @@ -42,5 +42,6 @@ RequestCanceled = 37, ConnectionToDownstreamServiceError = 38, CouldNotFindLoadBalancerCreator = 39, + ErrorInvokingLoadBalancerCreator = 40, } } diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/CookieStickySessionsCreator.cs b/src/Ocelot/LoadBalancer/LoadBalancers/CookieStickySessionsCreator.cs index e78b1884..2f17882e 100644 --- a/src/Ocelot/LoadBalancer/LoadBalancers/CookieStickySessionsCreator.cs +++ b/src/Ocelot/LoadBalancer/LoadBalancers/CookieStickySessionsCreator.cs @@ -4,15 +4,16 @@ using Ocelot.Configuration; using Ocelot.Infrastructure; using Ocelot.ServiceDiscovery.Providers; + using Ocelot.Responses; public class CookieStickySessionsCreator : ILoadBalancerCreator { - public ILoadBalancer Create(DownstreamReRoute reRoute, IServiceDiscoveryProvider serviceProvider) + public Response Create(DownstreamReRoute reRoute, IServiceDiscoveryProvider serviceProvider) { var loadBalancer = new RoundRobin(async () => await serviceProvider.Get()); var bus = new InMemoryBus(); - return new CookieStickySessions(loadBalancer, reRoute.LoadBalancerOptions.Key, - reRoute.LoadBalancerOptions.ExpiryInMs, bus); + return new OkResponse(new CookieStickySessions(loadBalancer, reRoute.LoadBalancerOptions.Key, + reRoute.LoadBalancerOptions.ExpiryInMs, bus)); } public string Type => nameof(CookieStickySessions); diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/DelegateInvokingLoadBalancerCreator.cs b/src/Ocelot/LoadBalancer/LoadBalancers/DelegateInvokingLoadBalancerCreator.cs index 535550dc..3a055427 100644 --- a/src/Ocelot/LoadBalancer/LoadBalancers/DelegateInvokingLoadBalancerCreator.cs +++ b/src/Ocelot/LoadBalancer/LoadBalancers/DelegateInvokingLoadBalancerCreator.cs @@ -3,6 +3,7 @@ using System; using Ocelot.Configuration; using Ocelot.ServiceDiscovery.Providers; + using Ocelot.Responses; public class DelegateInvokingLoadBalancerCreator : ILoadBalancerCreator where T : ILoadBalancer @@ -15,9 +16,17 @@ _creatorFunc = creatorFunc; } - public ILoadBalancer Create(DownstreamReRoute reRoute, IServiceDiscoveryProvider serviceProvider) + public Response Create(DownstreamReRoute reRoute, IServiceDiscoveryProvider serviceProvider) { - return _creatorFunc(reRoute, serviceProvider); + try + { + return new OkResponse(_creatorFunc(reRoute, serviceProvider)); + + } + catch (Exception e) + { + return new ErrorResponse(new ErrorInvokingLoadBalancerCreator(e)); + } } public string Type => typeof(T).Name; diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/ErrorInvokingLoadBalancerCreator.cs b/src/Ocelot/LoadBalancer/LoadBalancers/ErrorInvokingLoadBalancerCreator.cs new file mode 100644 index 00000000..022814c5 --- /dev/null +++ b/src/Ocelot/LoadBalancer/LoadBalancers/ErrorInvokingLoadBalancerCreator.cs @@ -0,0 +1,12 @@ +namespace Ocelot.LoadBalancer.LoadBalancers +{ + using System; + using Errors; + + public class ErrorInvokingLoadBalancerCreator : Error + { + public ErrorInvokingLoadBalancerCreator(Exception e) : base($"Error when invoking user provided load balancer creator function, Message: {e.Message}, StackTrace: {e.StackTrace}", OcelotErrorCode.ErrorInvokingLoadBalancerCreator) + { + } + } +} diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/ILoadBalancerCreator.cs b/src/Ocelot/LoadBalancer/LoadBalancers/ILoadBalancerCreator.cs index 4831e62f..c2df1dae 100644 --- a/src/Ocelot/LoadBalancer/LoadBalancers/ILoadBalancerCreator.cs +++ b/src/Ocelot/LoadBalancer/LoadBalancers/ILoadBalancerCreator.cs @@ -1,11 +1,12 @@ -using Ocelot.Configuration; -using Ocelot.ServiceDiscovery.Providers; - -namespace Ocelot.LoadBalancer.LoadBalancers +namespace Ocelot.LoadBalancer.LoadBalancers { + using Ocelot.Responses; + using Ocelot.Configuration; + using Ocelot.ServiceDiscovery.Providers; + public interface ILoadBalancerCreator { - ILoadBalancer Create(DownstreamReRoute reRoute, IServiceDiscoveryProvider serviceProvider); + Response Create(DownstreamReRoute reRoute, IServiceDiscoveryProvider serviceProvider); string Type { get; } } } diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/LeastConnectionCreator.cs b/src/Ocelot/LoadBalancer/LoadBalancers/LeastConnectionCreator.cs index e9e9c561..509a04a1 100644 --- a/src/Ocelot/LoadBalancer/LoadBalancers/LeastConnectionCreator.cs +++ b/src/Ocelot/LoadBalancer/LoadBalancers/LeastConnectionCreator.cs @@ -2,12 +2,13 @@ { using Ocelot.Configuration; using Ocelot.ServiceDiscovery.Providers; + using Ocelot.Responses; public class LeastConnectionCreator : ILoadBalancerCreator { - public ILoadBalancer Create(DownstreamReRoute reRoute, IServiceDiscoveryProvider serviceProvider) + public Response Create(DownstreamReRoute reRoute, IServiceDiscoveryProvider serviceProvider) { - return new LeastConnection(async () => await serviceProvider.Get(), reRoute.ServiceName); + return new OkResponse(new LeastConnection(async () => await serviceProvider.Get(), reRoute.ServiceName)); } public string Type => nameof(LeastConnection); diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/LoadBalancerFactory.cs b/src/Ocelot/LoadBalancer/LoadBalancers/LoadBalancerFactory.cs index 93cd0adb..31b6f74d 100644 --- a/src/Ocelot/LoadBalancer/LoadBalancers/LoadBalancerFactory.cs +++ b/src/Ocelot/LoadBalancer/LoadBalancers/LoadBalancerFactory.cs @@ -35,8 +35,14 @@ return new ErrorResponse(new CouldNotFindLoadBalancerCreator($"Could not find load balancer creator for Type: {requestedType}, please check your config specified the correct load balancer and that you have registered a class with the same name.")); } - var createdLoadBalancer = applicableCreator.Create(reRoute, serviceProvider); - return new OkResponse(createdLoadBalancer); + var createdLoadBalancerResponse = applicableCreator.Create(reRoute, serviceProvider); + + if (createdLoadBalancerResponse.IsError) + { + return new ErrorResponse(createdLoadBalancerResponse.Errors); + } + + return new OkResponse(createdLoadBalancerResponse.Data); } } } diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/NoLoadBalancerCreator.cs b/src/Ocelot/LoadBalancer/LoadBalancers/NoLoadBalancerCreator.cs index fc1de3fd..ea2fdd7a 100644 --- a/src/Ocelot/LoadBalancer/LoadBalancers/NoLoadBalancerCreator.cs +++ b/src/Ocelot/LoadBalancer/LoadBalancers/NoLoadBalancerCreator.cs @@ -2,12 +2,13 @@ { using Ocelot.Configuration; using Ocelot.ServiceDiscovery.Providers; + using Ocelot.Responses; public class NoLoadBalancerCreator : ILoadBalancerCreator { - public ILoadBalancer Create(DownstreamReRoute reRoute, IServiceDiscoveryProvider serviceProvider) + public Response Create(DownstreamReRoute reRoute, IServiceDiscoveryProvider serviceProvider) { - return new NoLoadBalancer(async () => await serviceProvider.Get()); + return new OkResponse(new NoLoadBalancer(async () => await serviceProvider.Get())); } public string Type => nameof(NoLoadBalancer); diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/RoundRobinCreator.cs b/src/Ocelot/LoadBalancer/LoadBalancers/RoundRobinCreator.cs index 8981074d..fd698133 100644 --- a/src/Ocelot/LoadBalancer/LoadBalancers/RoundRobinCreator.cs +++ b/src/Ocelot/LoadBalancer/LoadBalancers/RoundRobinCreator.cs @@ -2,12 +2,13 @@ { using Ocelot.Configuration; using Ocelot.ServiceDiscovery.Providers; - + using Ocelot.Responses; + public class RoundRobinCreator : ILoadBalancerCreator { - public ILoadBalancer Create(DownstreamReRoute reRoute, IServiceDiscoveryProvider serviceProvider) + public Response Create(DownstreamReRoute reRoute, IServiceDiscoveryProvider serviceProvider) { - return new RoundRobin(async () => await serviceProvider.Get()); + return new OkResponse(new RoundRobin(async () => await serviceProvider.Get())); } public string Type => nameof(RoundRobin); diff --git a/src/Ocelot/Responder/ErrorsToHttpStatusCodeMapper.cs b/src/Ocelot/Responder/ErrorsToHttpStatusCodeMapper.cs index 6bb85ba9..841f788d 100644 --- a/src/Ocelot/Responder/ErrorsToHttpStatusCodeMapper.cs +++ b/src/Ocelot/Responder/ErrorsToHttpStatusCodeMapper.cs @@ -46,7 +46,8 @@ namespace Ocelot.Responder } if (errors.Any(e => e.Code == OcelotErrorCode.UnableToCompleteRequestError - || e.Code == OcelotErrorCode.CouldNotFindLoadBalancerCreator)) + || e.Code == OcelotErrorCode.CouldNotFindLoadBalancerCreator + || e.Code == OcelotErrorCode.ErrorInvokingLoadBalancerCreator)) { return 500; } diff --git a/test/Ocelot.UnitTests/LoadBalancer/CookieStickySessionsCreatorTests.cs b/test/Ocelot.UnitTests/LoadBalancer/CookieStickySessionsCreatorTests.cs index bf004010..ece2286e 100644 --- a/test/Ocelot.UnitTests/LoadBalancer/CookieStickySessionsCreatorTests.cs +++ b/test/Ocelot.UnitTests/LoadBalancer/CookieStickySessionsCreatorTests.cs @@ -1,20 +1,21 @@ -using Moq; -using Ocelot.Configuration; -using Ocelot.Configuration.Builder; -using Ocelot.LoadBalancer.LoadBalancers; -using Ocelot.ServiceDiscovery.Providers; -using Shouldly; -using TestStack.BDDfy; -using Xunit; - -namespace Ocelot.UnitTests.LoadBalancer +namespace Ocelot.UnitTests.LoadBalancer { + using Moq; + using Ocelot.Configuration; + using Ocelot.Configuration.Builder; + using Ocelot.LoadBalancer.LoadBalancers; + using Ocelot.ServiceDiscovery.Providers; + using Ocelot.Responses; + using Shouldly; + using TestStack.BDDfy; + using Xunit; + public class CookieStickySessionsCreatorTests { private readonly CookieStickySessionsCreator _creator; private readonly Mock _serviceProvider; private DownstreamReRoute _reRoute; - private ILoadBalancer _loadBalancer; + private Response _loadBalancer; private string _typeName; public CookieStickySessionsCreatorTests() @@ -62,7 +63,7 @@ namespace Ocelot.UnitTests.LoadBalancer private void ThenTheLoadBalancerIsReturned() where T : ILoadBalancer { - _loadBalancer.ShouldBeOfType(); + _loadBalancer.Data.ShouldBeOfType(); } private void ThenTheLoadBalancerTypeIs(string type) diff --git a/test/Ocelot.UnitTests/LoadBalancer/DelegateInvokingLoadBalancerCreatorTests.cs b/test/Ocelot.UnitTests/LoadBalancer/DelegateInvokingLoadBalancerCreatorTests.cs index 2be4ee62..d46be02b 100644 --- a/test/Ocelot.UnitTests/LoadBalancer/DelegateInvokingLoadBalancerCreatorTests.cs +++ b/test/Ocelot.UnitTests/LoadBalancer/DelegateInvokingLoadBalancerCreatorTests.cs @@ -16,11 +16,11 @@ namespace Ocelot.UnitTests.LoadBalancer { public class DelegateInvokingLoadBalancerCreatorTests { - private readonly DelegateInvokingLoadBalancerCreator _creator; - private readonly Func _creatorFunc; + private DelegateInvokingLoadBalancerCreator _creator; + private Func _creatorFunc; private readonly Mock _serviceProvider; private DownstreamReRoute _reRoute; - private ILoadBalancer _loadBalancer; + private Response _loadBalancer; private string _typeName; public DelegateInvokingLoadBalancerCreatorTests() @@ -51,6 +51,31 @@ namespace Ocelot.UnitTests.LoadBalancer .BDDfy(); } + [Fact] + public void should_return_error() + { + var reRoute = new DownstreamReRouteBuilder() + .Build(); + + this.Given(x => x.GivenAReRoute(reRoute)) + .And(x => x.GivenTheCreatorFuncThrows()) + .When(x => x.WhenIGetTheLoadBalancer()) + .Then(x => x.ThenAnErrorIsReturned()) + .BDDfy(); + } + + private void GivenTheCreatorFuncThrows() + { + _creatorFunc = (reRoute, serviceDiscoveryProvider) => throw new Exception(); + + _creator = new DelegateInvokingLoadBalancerCreator(_creatorFunc); + } + + private void ThenAnErrorIsReturned() + { + _loadBalancer.IsError.ShouldBeTrue(); + } + private void GivenAReRoute(DownstreamReRoute reRoute) { _reRoute = reRoute; @@ -69,7 +94,7 @@ namespace Ocelot.UnitTests.LoadBalancer private void ThenTheLoadBalancerIsReturned() where T : ILoadBalancer { - _loadBalancer.ShouldBeOfType(); + _loadBalancer.Data.ShouldBeOfType(); } private void ThenTheLoadBalancerTypeIs(string type) diff --git a/test/Ocelot.UnitTests/LoadBalancer/LeastConnectionCreatorTests.cs b/test/Ocelot.UnitTests/LoadBalancer/LeastConnectionCreatorTests.cs index 37d678cd..326ecbd7 100644 --- a/test/Ocelot.UnitTests/LoadBalancer/LeastConnectionCreatorTests.cs +++ b/test/Ocelot.UnitTests/LoadBalancer/LeastConnectionCreatorTests.cs @@ -1,20 +1,21 @@ -using Moq; -using Ocelot.Configuration; -using Ocelot.Configuration.Builder; -using Ocelot.LoadBalancer.LoadBalancers; -using Ocelot.ServiceDiscovery.Providers; -using Shouldly; -using TestStack.BDDfy; -using Xunit; - -namespace Ocelot.UnitTests.LoadBalancer +namespace Ocelot.UnitTests.LoadBalancer { + using Moq; + using Ocelot.Configuration; + using Ocelot.Configuration.Builder; + using Ocelot.LoadBalancer.LoadBalancers; + using Ocelot.Responses; + using Ocelot.ServiceDiscovery.Providers; + using Shouldly; + using TestStack.BDDfy; + using Xunit; + public class LeastConnectionCreatorTests { private readonly LeastConnectionCreator _creator; private readonly Mock _serviceProvider; private DownstreamReRoute _reRoute; - private ILoadBalancer _loadBalancer; + private Response _loadBalancer; private string _typeName; public LeastConnectionCreatorTests() @@ -62,7 +63,7 @@ namespace Ocelot.UnitTests.LoadBalancer private void ThenTheLoadBalancerIsReturned() where T : ILoadBalancer { - _loadBalancer.ShouldBeOfType(); + _loadBalancer.Data.ShouldBeOfType(); } private void ThenTheLoadBalancerTypeIs(string type) diff --git a/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerFactoryTests.cs b/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerFactoryTests.cs index a08a808f..b70f8371 100644 --- a/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerFactoryTests.cs +++ b/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerFactoryTests.cs @@ -16,6 +16,8 @@ using Xunit; namespace Ocelot.UnitTests.LoadBalancer { + using System; + public class LoadBalancerFactoryTests { private DownstreamReRoute _reRoute; @@ -35,6 +37,7 @@ namespace Ocelot.UnitTests.LoadBalancer new FakeLoadBalancerCreator(), new FakeLoadBalancerCreator(), new FakeLoadBalancerCreator(nameof(NoLoadBalancer)), + new BrokenLoadBalancerCreator(), }; _factory = new LoadBalancerFactory(_serviceProviderFactory.Object, _loadBalancerCreators); } @@ -87,6 +90,22 @@ namespace Ocelot.UnitTests.LoadBalancer .BDDfy(); } + [Fact] + public void should_return_error_response_if_creator_errors() + { + var reRoute = new DownstreamReRouteBuilder() + .WithLoadBalancerOptions(new LoadBalancerOptions("BrokenLoadBalancer", "", 0)) + .WithUpstreamHttpMethod(new List { "Get" }) + .Build(); + + this.Given(x => x.GivenAReRoute(reRoute)) + .And(x => GivenAServiceProviderConfig(new ServiceProviderConfigurationBuilder().Build())) + .And(x => x.GivenTheServiceProviderFactoryReturns()) + .When(x => x.WhenIGetTheLoadBalancer()) + .Then(x => x.ThenAnErrorResponseIsReturned()) + .BDDfy(); + } + [Fact] public void should_call_service_provider() { @@ -183,14 +202,30 @@ namespace Ocelot.UnitTests.LoadBalancer Type = type; } - public ILoadBalancer Create(DownstreamReRoute reRoute, IServiceDiscoveryProvider serviceProvider) + public Response Create(DownstreamReRoute reRoute, IServiceDiscoveryProvider serviceProvider) { - return new T(); + return new OkResponse(new T()); } public string Type { get; } } + private class BrokenLoadBalancerCreator : ILoadBalancerCreator + where T : ILoadBalancer, new() + { + public BrokenLoadBalancerCreator() + { + Type = typeof(T).Name; + } + + public Response Create(DownstreamReRoute reRoute, IServiceDiscoveryProvider serviceProvider) + { + return new ErrorResponse(new ErrorInvokingLoadBalancerCreator(new Exception())); + } + + public string Type { get; } + } + private class FakeLoadBalancerOne : ILoadBalancer { public Task> Lease(DownstreamContext context) @@ -229,5 +264,19 @@ namespace Ocelot.UnitTests.LoadBalancer throw new System.NotImplementedException(); } } + + private class BrokenLoadBalancer : ILoadBalancer + { + public Task> Lease(DownstreamContext context) + { + throw new System.NotImplementedException(); + } + + public void Release(ServiceHostAndPort hostAndPort) + { + throw new System.NotImplementedException(); + } + } + } } diff --git a/test/Ocelot.UnitTests/LoadBalancer/NoLoadBalancerCreatorTests.cs b/test/Ocelot.UnitTests/LoadBalancer/NoLoadBalancerCreatorTests.cs index 1cdcb3ab..46f4208b 100644 --- a/test/Ocelot.UnitTests/LoadBalancer/NoLoadBalancerCreatorTests.cs +++ b/test/Ocelot.UnitTests/LoadBalancer/NoLoadBalancerCreatorTests.cs @@ -1,20 +1,21 @@ -using Moq; -using Ocelot.Configuration; -using Ocelot.Configuration.Builder; -using Ocelot.LoadBalancer.LoadBalancers; -using Ocelot.ServiceDiscovery.Providers; -using Shouldly; -using TestStack.BDDfy; -using Xunit; - -namespace Ocelot.UnitTests.LoadBalancer +namespace Ocelot.UnitTests.LoadBalancer { + using Moq; + using Ocelot.Configuration; + using Ocelot.Configuration.Builder; + using Ocelot.LoadBalancer.LoadBalancers; + using Ocelot.Responses; + using Ocelot.ServiceDiscovery.Providers; + using Shouldly; + using TestStack.BDDfy; + using Xunit; + public class NoLoadBalancerCreatorTests { private readonly NoLoadBalancerCreator _creator; private readonly Mock _serviceProvider; private DownstreamReRoute _reRoute; - private ILoadBalancer _loadBalancer; + private Response _loadBalancer; private string _typeName; public NoLoadBalancerCreatorTests() @@ -61,7 +62,7 @@ namespace Ocelot.UnitTests.LoadBalancer private void ThenTheLoadBalancerIsReturned() where T : ILoadBalancer { - _loadBalancer.ShouldBeOfType(); + _loadBalancer.Data.ShouldBeOfType(); } private void ThenTheLoadBalancerTypeIs(string type) diff --git a/test/Ocelot.UnitTests/LoadBalancer/RoundRobinCreatorTests.cs b/test/Ocelot.UnitTests/LoadBalancer/RoundRobinCreatorTests.cs index 83f7cb6f..b8c436a2 100644 --- a/test/Ocelot.UnitTests/LoadBalancer/RoundRobinCreatorTests.cs +++ b/test/Ocelot.UnitTests/LoadBalancer/RoundRobinCreatorTests.cs @@ -1,20 +1,21 @@ -using Moq; -using Ocelot.Configuration; -using Ocelot.Configuration.Builder; -using Ocelot.LoadBalancer.LoadBalancers; -using Ocelot.ServiceDiscovery.Providers; -using Shouldly; -using TestStack.BDDfy; -using Xunit; - -namespace Ocelot.UnitTests.LoadBalancer +namespace Ocelot.UnitTests.LoadBalancer { + using Moq; + using Ocelot.Configuration; + using Ocelot.Configuration.Builder; + using Ocelot.LoadBalancer.LoadBalancers; + using Ocelot.Responses; + using Ocelot.ServiceDiscovery.Providers; + using Shouldly; + using TestStack.BDDfy; + using Xunit; + public class RoundRobinCreatorTests { private readonly RoundRobinCreator _creator; private readonly Mock _serviceProvider; private DownstreamReRoute _reRoute; - private ILoadBalancer _loadBalancer; + private Response _loadBalancer; private string _typeName; public RoundRobinCreatorTests() @@ -61,7 +62,7 @@ namespace Ocelot.UnitTests.LoadBalancer private void ThenTheLoadBalancerIsReturned() where T : ILoadBalancer { - _loadBalancer.ShouldBeOfType(); + _loadBalancer.Data.ShouldBeOfType(); } private void ThenTheLoadBalancerTypeIs(string type) diff --git a/test/Ocelot.UnitTests/Responder/ErrorsToHttpStatusCodeMapperTests.cs b/test/Ocelot.UnitTests/Responder/ErrorsToHttpStatusCodeMapperTests.cs index da2898fc..04a4496f 100644 --- a/test/Ocelot.UnitTests/Responder/ErrorsToHttpStatusCodeMapperTests.cs +++ b/test/Ocelot.UnitTests/Responder/ErrorsToHttpStatusCodeMapperTests.cs @@ -48,6 +48,7 @@ namespace Ocelot.UnitTests.Responder [Theory] [InlineData(OcelotErrorCode.UnableToCompleteRequestError)] [InlineData(OcelotErrorCode.CouldNotFindLoadBalancerCreator)] + [InlineData(OcelotErrorCode.ErrorInvokingLoadBalancerCreator)] public void should_return_internal_server_error(OcelotErrorCode errorCode) { ShouldMapErrorToStatusCode(errorCode, HttpStatusCode.InternalServerError); @@ -133,7 +134,7 @@ namespace Ocelot.UnitTests.Responder // If this test fails then it's because the number of error codes has changed. // You should make the appropriate changes to the test cases here to ensure // they cover all the error codes, and then modify this assertion. - Enum.GetNames(typeof(OcelotErrorCode)).Length.ShouldBe(40, "Looks like the number of error codes has changed. Do you need to modify ErrorsToHttpStatusCodeMapper?"); + Enum.GetNames(typeof(OcelotErrorCode)).Length.ShouldBe(41, "Looks like the number of error codes has changed. Do you need to modify ErrorsToHttpStatusCodeMapper?"); } private void ShouldMapErrorToStatusCode(OcelotErrorCode errorCode, HttpStatusCode expectedHttpStatusCode)