From 72cec38c0e05cfd723960161ec1f36704d3c13f7 Mon Sep 17 00:00:00 2001 From: TomPallister Date: Sun, 11 Sep 2016 21:32:56 +0100 Subject: [PATCH] some tidying up and a bit of refactoring...not too happy about how the proxy is working at the moment! May need a rethink! --- .../ConfigurationValidationResult.cs | 21 ++++++++--- .../Configuration/ConfigurationValidator.cs | 8 ++--- .../DownstreamRouteFinder.cs | 1 - .../Requester/HttpClientHttpRequester.cs | 21 +++++++++++ .../Requester/IHttpRequester.cs | 11 ++++++ .../Responder/HttpContextResponder.cs | 27 ++++++++++++++ .../Responder/IHttpResponder.cs | 13 +++++++ .../DownstreamUrlTemplateVariableReplacer.cs | 2 +- ...wnstreamUrlPathTemplateVariableReplacer.cs | 2 +- .../Middleware/ProxyMiddleware.cs | 32 +++++++---------- src/Ocelot.Library/project.json | 35 ++++++++++--------- src/Ocelot/Startup.cs | 4 +++ test/Ocelot.AcceptanceTests/OcelotTests.cs | 11 +++--- .../ConfigurationValidationTests.cs | 10 ++++-- .../DownstreamRouteFinderTests.cs | 11 +++--- ...eamUrlPathTemplateVariableReplacerTests.cs | 2 +- 16 files changed, 151 insertions(+), 60 deletions(-) create mode 100644 src/Ocelot.Library/Infrastructure/Requester/HttpClientHttpRequester.cs create mode 100644 src/Ocelot.Library/Infrastructure/Requester/IHttpRequester.cs create mode 100644 src/Ocelot.Library/Infrastructure/Responder/HttpContextResponder.cs create mode 100644 src/Ocelot.Library/Infrastructure/Responder/IHttpResponder.cs diff --git a/src/Ocelot.Library/Infrastructure/Configuration/ConfigurationValidationResult.cs b/src/Ocelot.Library/Infrastructure/Configuration/ConfigurationValidationResult.cs index 976279ca..abbbcfae 100644 --- a/src/Ocelot.Library/Infrastructure/Configuration/ConfigurationValidationResult.cs +++ b/src/Ocelot.Library/Infrastructure/Configuration/ConfigurationValidationResult.cs @@ -1,11 +1,24 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using System.Threading.Tasks; +using System.Collections.Generic; +using Ocelot.Library.Infrastructure.Responses; namespace Ocelot.Library.Infrastructure.Configuration { public class ConfigurationValidationResult { + public ConfigurationValidationResult(bool isError) + { + IsError = isError; + Errors = new List(); + } + + public ConfigurationValidationResult(bool isError, List errors) + { + IsError = isError; + Errors = errors; + } + + public bool IsError { get; private set; } + + public List Errors { get; private set; } } } diff --git a/src/Ocelot.Library/Infrastructure/Configuration/ConfigurationValidator.cs b/src/Ocelot.Library/Infrastructure/Configuration/ConfigurationValidator.cs index bcb6b0bb..5038baf3 100644 --- a/src/Ocelot.Library/Infrastructure/Configuration/ConfigurationValidator.cs +++ b/src/Ocelot.Library/Infrastructure/Configuration/ConfigurationValidator.cs @@ -1,7 +1,5 @@ -using System; -using System.Collections.Generic; +using System.Collections.Generic; using System.Linq; -using System.Text; using Ocelot.Library.Infrastructure.Responses; namespace Ocelot.Library.Infrastructure.Configuration @@ -19,7 +17,7 @@ namespace Ocelot.Library.Infrastructure.Configuration if (duplicateUpstreamTemplates.Count <= 0) { - return new OkResponse(new ConfigurationValidationResult()); + return new OkResponse(new ConfigurationValidationResult(false)); } var errors = new List(); @@ -31,7 +29,7 @@ namespace Ocelot.Library.Infrastructure.Configuration errors.Add(error); } - return new ErrorResponse(errors); + return new OkResponse(new ConfigurationValidationResult(true, errors)); } } } diff --git a/src/Ocelot.Library/Infrastructure/DownstreamRouteFinder/DownstreamRouteFinder.cs b/src/Ocelot.Library/Infrastructure/DownstreamRouteFinder/DownstreamRouteFinder.cs index 63ddfacd..0cc2f18e 100644 --- a/src/Ocelot.Library/Infrastructure/DownstreamRouteFinder/DownstreamRouteFinder.cs +++ b/src/Ocelot.Library/Infrastructure/DownstreamRouteFinder/DownstreamRouteFinder.cs @@ -18,7 +18,6 @@ namespace Ocelot.Library.Infrastructure.DownstreamRouteFinder public Response FindDownstreamRoute(string upstreamUrlPath) { - foreach (var template in _configuration.Value.ReRoutes) { var urlMatch = _urlMatcher.Match(upstreamUrlPath, template.UpstreamTemplate); diff --git a/src/Ocelot.Library/Infrastructure/Requester/HttpClientHttpRequester.cs b/src/Ocelot.Library/Infrastructure/Requester/HttpClientHttpRequester.cs new file mode 100644 index 00000000..1b76ba8c --- /dev/null +++ b/src/Ocelot.Library/Infrastructure/Requester/HttpClientHttpRequester.cs @@ -0,0 +1,21 @@ +using System.Net.Http; +using System.Threading.Tasks; + +namespace Ocelot.Library.Infrastructure.Requester +{ + public class HttpClientHttpRequester : IHttpRequester + { + public async Task GetResponse(string httpMethod, string downstreamUrl) + { + var method = new HttpMethod(httpMethod); + + var httpRequestMessage = new HttpRequestMessage(method, downstreamUrl); + + using (var httpClient = new HttpClient()) + { + var response = await httpClient.SendAsync(httpRequestMessage); + return response; + } + } + } +} \ No newline at end of file diff --git a/src/Ocelot.Library/Infrastructure/Requester/IHttpRequester.cs b/src/Ocelot.Library/Infrastructure/Requester/IHttpRequester.cs new file mode 100644 index 00000000..310f25b3 --- /dev/null +++ b/src/Ocelot.Library/Infrastructure/Requester/IHttpRequester.cs @@ -0,0 +1,11 @@ +using System.Net.Http; +using System.Threading.Tasks; +using Flurl.Http; + +namespace Ocelot.Library.Infrastructure.Requester +{ + public interface IHttpRequester + { + Task GetResponse(string httpMethod, string downstreamUrl); + } +} diff --git a/src/Ocelot.Library/Infrastructure/Responder/HttpContextResponder.cs b/src/Ocelot.Library/Infrastructure/Responder/HttpContextResponder.cs new file mode 100644 index 00000000..a90543fc --- /dev/null +++ b/src/Ocelot.Library/Infrastructure/Responder/HttpContextResponder.cs @@ -0,0 +1,27 @@ +using System.Net; +using System.Net.Http; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Http; + +namespace Ocelot.Library.Infrastructure.Responder +{ + public class HttpContextResponder : IHttpResponder + { + public async Task CreateSuccessResponse(HttpContext context, HttpResponseMessage response) + { + if (!response.IsSuccessStatusCode) + { + context.Response.StatusCode = (int)response.StatusCode; + return context; + } + await context.Response.WriteAsync(await response.Content.ReadAsStringAsync()); + return context; + } + + public async Task CreateNotFoundResponse(HttpContext context) + { + context.Response.StatusCode = (int)HttpStatusCode.NotFound; + return context; + } + } +} \ No newline at end of file diff --git a/src/Ocelot.Library/Infrastructure/Responder/IHttpResponder.cs b/src/Ocelot.Library/Infrastructure/Responder/IHttpResponder.cs new file mode 100644 index 00000000..3e2dc91b --- /dev/null +++ b/src/Ocelot.Library/Infrastructure/Responder/IHttpResponder.cs @@ -0,0 +1,13 @@ +using System.Net.Http; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Http; + +namespace Ocelot.Library.Infrastructure.Responder +{ + public interface IHttpResponder + { + Task CreateSuccessResponse(HttpContext context, HttpResponseMessage response); + Task CreateNotFoundResponse(HttpContext context); + + } +} diff --git a/src/Ocelot.Library/Infrastructure/UrlTemplateReplacer/DownstreamUrlTemplateVariableReplacer.cs b/src/Ocelot.Library/Infrastructure/UrlTemplateReplacer/DownstreamUrlTemplateVariableReplacer.cs index 204eb212..34987a8b 100644 --- a/src/Ocelot.Library/Infrastructure/UrlTemplateReplacer/DownstreamUrlTemplateVariableReplacer.cs +++ b/src/Ocelot.Library/Infrastructure/UrlTemplateReplacer/DownstreamUrlTemplateVariableReplacer.cs @@ -5,7 +5,7 @@ namespace Ocelot.Library.Infrastructure.UrlTemplateReplacer { public class DownstreamUrlTemplateVariableReplacer : IDownstreamUrlTemplateVariableReplacer { - public string ReplaceTemplateVariable(DownstreamRoute downstreamRoute) + public string ReplaceTemplateVariables(DownstreamRoute downstreamRoute) { var upstreamUrl = new StringBuilder(); diff --git a/src/Ocelot.Library/Infrastructure/UrlTemplateReplacer/IDownstreamUrlPathTemplateVariableReplacer.cs b/src/Ocelot.Library/Infrastructure/UrlTemplateReplacer/IDownstreamUrlPathTemplateVariableReplacer.cs index bc50a8fa..9f27db84 100644 --- a/src/Ocelot.Library/Infrastructure/UrlTemplateReplacer/IDownstreamUrlPathTemplateVariableReplacer.cs +++ b/src/Ocelot.Library/Infrastructure/UrlTemplateReplacer/IDownstreamUrlPathTemplateVariableReplacer.cs @@ -5,6 +5,6 @@ namespace Ocelot.Library.Infrastructure.UrlTemplateReplacer { public interface IDownstreamUrlTemplateVariableReplacer { - string ReplaceTemplateVariable(DownstreamRoute downstreamRoute); + string ReplaceTemplateVariables(DownstreamRoute downstreamRoute); } } \ No newline at end of file diff --git a/src/Ocelot.Library/Middleware/ProxyMiddleware.cs b/src/Ocelot.Library/Middleware/ProxyMiddleware.cs index b155dc15..31b7c487 100644 --- a/src/Ocelot.Library/Middleware/ProxyMiddleware.cs +++ b/src/Ocelot.Library/Middleware/ProxyMiddleware.cs @@ -1,10 +1,10 @@ -using System.Net; -using System.Net.Http; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.Options; using Ocelot.Library.Infrastructure.Configuration; using Ocelot.Library.Infrastructure.DownstreamRouteFinder; +using Ocelot.Library.Infrastructure.Requester; +using Ocelot.Library.Infrastructure.Responder; using Ocelot.Library.Infrastructure.UrlTemplateReplacer; namespace Ocelot.Library.Middleware @@ -15,16 +15,22 @@ namespace Ocelot.Library.Middleware private readonly IDownstreamUrlTemplateVariableReplacer _urlReplacer; private readonly IOptions _configuration; private readonly IDownstreamRouteFinder _downstreamRouteFinder; + private readonly IHttpRequester _requester; + private readonly IHttpResponder _responder; public ProxyMiddleware(RequestDelegate next, IDownstreamUrlTemplateVariableReplacer urlReplacer, IOptions configuration, - IDownstreamRouteFinder downstreamRouteFinder) + IDownstreamRouteFinder downstreamRouteFinder, + IHttpRequester requester, + IHttpResponder responder) { _next = next; _urlReplacer = urlReplacer; _configuration = configuration; _downstreamRouteFinder = downstreamRouteFinder; + _requester = requester; + _responder = responder; } public async Task Invoke(HttpContext context) @@ -35,27 +41,15 @@ namespace Ocelot.Library.Middleware if (downstreamRoute.IsError) { - context.Response.StatusCode = (int)HttpStatusCode.NotFound; + await _responder.CreateNotFoundResponse(context); return; } - var downstreamUrl = _urlReplacer.ReplaceTemplateVariable(downstreamRoute.Data); + var downstreamUrl = _urlReplacer.ReplaceTemplateVariables(downstreamRoute.Data); - using (var httpClient = new HttpClient()) - { - var httpMethod = new HttpMethod(context.Request.Method); + var response = await _requester.GetResponse(context.Request.Method, downstreamUrl); - var httpRequestMessage = new HttpRequestMessage(httpMethod, downstreamUrl); - - var response = await httpClient.SendAsync(httpRequestMessage); - - if (!response.IsSuccessStatusCode) - { - context.Response.StatusCode = (int)response.StatusCode; - return; - } - await context.Response.WriteAsync(await response.Content.ReadAsStringAsync()); - } + context = await _responder.CreateSuccessResponse(context, response); await _next.Invoke(context); } diff --git a/src/Ocelot.Library/project.json b/src/Ocelot.Library/project.json index 7037f0a7..07d34181 100644 --- a/src/Ocelot.Library/project.json +++ b/src/Ocelot.Library/project.json @@ -1,24 +1,25 @@ { "version": "1.0.0-*", - "dependencies": { - "Microsoft.NETCore.App": { - "version": "1.0.0", - "type": "platform" + "dependencies": { + "Microsoft.NETCore.App": { + "version": "1.0.0", + "type": "platform" + }, + "Microsoft.AspNetCore.Mvc": "1.0.0", + "Microsoft.AspNetCore.Server.IISIntegration": "1.0.0", + "Microsoft.AspNetCore.Server.Kestrel": "1.0.0", + "Microsoft.Extensions.Configuration.EnvironmentVariables": "1.0.0", + "Microsoft.Extensions.Configuration.FileExtensions": "1.0.0", + "Microsoft.Extensions.Configuration.Json": "1.0.0", + "Microsoft.Extensions.Logging": "1.0.0", + "Microsoft.Extensions.Logging.Console": "1.0.0", + "Microsoft.Extensions.Logging.Debug": "1.0.0", + "Microsoft.Extensions.Options.ConfigurationExtensions": "1.0.0", + "Microsoft.AspNetCore.Http": "1.0.0", + "YamlDotNet": "3.9.0", + "Flurl.Http": "1.0.1" }, - "Microsoft.AspNetCore.Mvc": "1.0.0", - "Microsoft.AspNetCore.Server.IISIntegration": "1.0.0", - "Microsoft.AspNetCore.Server.Kestrel": "1.0.0", - "Microsoft.Extensions.Configuration.EnvironmentVariables": "1.0.0", - "Microsoft.Extensions.Configuration.FileExtensions": "1.0.0", - "Microsoft.Extensions.Configuration.Json": "1.0.0", - "Microsoft.Extensions.Logging": "1.0.0", - "Microsoft.Extensions.Logging.Console": "1.0.0", - "Microsoft.Extensions.Logging.Debug": "1.0.0", - "Microsoft.Extensions.Options.ConfigurationExtensions": "1.0.0", - "Microsoft.AspNetCore.Http": "1.0.0", - "YamlDotNet": "3.9.0" - }, "frameworks": { "netcoreapp1.0": { diff --git a/src/Ocelot/Startup.cs b/src/Ocelot/Startup.cs index 18d5ba6b..5b1fff8e 100644 --- a/src/Ocelot/Startup.cs +++ b/src/Ocelot/Startup.cs @@ -4,6 +4,8 @@ using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Ocelot.Library.Infrastructure.DownstreamRouteFinder; +using Ocelot.Library.Infrastructure.Requester; +using Ocelot.Library.Infrastructure.Responder; using Ocelot.Library.Middleware; namespace Ocelot @@ -38,6 +40,8 @@ namespace Ocelot services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); + services.AddSingleton(); + services.AddSingleton(); } // This method gets called by the runtime. Use this method to configure the HTTP request pipeline. diff --git a/test/Ocelot.AcceptanceTests/OcelotTests.cs b/test/Ocelot.AcceptanceTests/OcelotTests.cs index 6eea27ab..20164ea8 100644 --- a/test/Ocelot.AcceptanceTests/OcelotTests.cs +++ b/test/Ocelot.AcceptanceTests/OcelotTests.cs @@ -20,16 +20,19 @@ namespace Ocelot.AcceptanceTests private TestServer _server; private HttpClient _client; private HttpResponseMessage _response; + private readonly string _configurationPath; public OcelotTests() { + _configurationPath = "./bin/Debug/netcoreapp1.0/configuration.yaml"; _fakeService = new FakeService(); } [Fact] public void should_return_response_404() { - this.Given(x => x.GivenTheApiGatewayIsRunning()) + this.Given(x => x.GivenThereIsAConfiguration(new Configuration())) + .And(x => x.GivenTheApiGatewayIsRunning()) .When(x => x.WhenIRequestTheUrlOnTheApiGateway("/")) .Then(x => x.ThenTheStatusCodeShouldBe(HttpStatusCode.NotFound)) .BDDfy(); @@ -72,12 +75,12 @@ namespace Ocelot.AcceptanceTests { var serializer = new Serializer(); - if (File.Exists("./configuration.yaml")) + if (File.Exists(_configurationPath)) { - File.Delete("./configuration.yaml"); + File.Delete(_configurationPath); } - using (TextWriter writer = File.CreateText("./configuration.yaml")) + using (TextWriter writer = File.CreateText(_configurationPath)) { serializer.Serialize(writer, configuration); } diff --git a/test/Ocelot.UnitTests/ConfigurationValidationTests.cs b/test/Ocelot.UnitTests/ConfigurationValidationTests.cs index 1a65e3db..615e6e90 100644 --- a/test/Ocelot.UnitTests/ConfigurationValidationTests.cs +++ b/test/Ocelot.UnitTests/ConfigurationValidationTests.cs @@ -58,6 +58,7 @@ namespace Ocelot.UnitTests })) .When(x => x.WhenIValidateTheConfiguration()) .Then(x => x.ThenTheResultIsNotValid()) + .And(x => x.ThenTheErrorIs()) .BDDfy(); } @@ -73,12 +74,17 @@ namespace Ocelot.UnitTests private void ThenTheResultIsValid() { - _result.IsError.ShouldBeFalse(); + _result.Data.IsError.ShouldBeFalse(); } private void ThenTheResultIsNotValid() { - _result.IsError.ShouldBeTrue(); + _result.Data.IsError.ShouldBeTrue(); + } + + private void ThenTheErrorIs() + { + _result.Data.Errors[0].ShouldBeOfType(); } } } diff --git a/test/Ocelot.UnitTests/DownstreamRouteFinderTests.cs b/test/Ocelot.UnitTests/DownstreamRouteFinderTests.cs index 212e2029..1f796adb 100644 --- a/test/Ocelot.UnitTests/DownstreamRouteFinderTests.cs +++ b/test/Ocelot.UnitTests/DownstreamRouteFinderTests.cs @@ -35,21 +35,21 @@ namespace Ocelot.UnitTests [Fact] public void should_return_route() { - this.Given(x => x.GivenThereIsAnUpstreamUrlPath("somePath")) + this.Given(x => x.GivenThereIsAnUpstreamUrlPath("someUpstreamPath")) .And(x => x.GivenTheConfigurationIs(new Configuration { ReRoutes = new List { new ReRoute() { - UpstreamTemplate = "somePath", - DownstreamTemplate = "somPath" + UpstreamTemplate = "someUpstreamPath", + DownstreamTemplate = "someDownstreamPath" } } })) - .And(x => x.GivenTheUrlMatcherReturns(new UrlMatch(true, new List(), "somePath"))) + .And(x => x.GivenTheUrlMatcherReturns(new UrlMatch(true, new List(), "someDownstreamPath"))) .When(x => x.WhenICallTheFinder()) .Then( - x => x.ThenTheFollowingIsReturned(new DownstreamRoute(new List(), "somePath"))) + x => x.ThenTheFollowingIsReturned(new DownstreamRoute(new List(), "someDownstreamPath"))) .And(x => x.ThenTheUrlMatcherIsCalledCorrectly()) .BDDfy(); } @@ -117,6 +117,7 @@ namespace Ocelot.UnitTests private void ThenTheFollowingIsReturned(DownstreamRoute expected) { _result.Data.DownstreamUrlTemplate.ShouldBe(expected.DownstreamUrlTemplate); + for (int i = 0; i < _result.Data.TemplateVariableNameAndValues.Count; i++) { _result.Data.TemplateVariableNameAndValues[i].TemplateVariableName.ShouldBe( diff --git a/test/Ocelot.UnitTests/UpstreamUrlPathTemplateVariableReplacerTests.cs b/test/Ocelot.UnitTests/UpstreamUrlPathTemplateVariableReplacerTests.cs index b280e04f..64bd0a1f 100644 --- a/test/Ocelot.UnitTests/UpstreamUrlPathTemplateVariableReplacerTests.cs +++ b/test/Ocelot.UnitTests/UpstreamUrlPathTemplateVariableReplacerTests.cs @@ -131,7 +131,7 @@ namespace Ocelot.UnitTests private void WhenIReplaceTheTemplateVariables() { - _result = _downstreamUrlPathReplacer.ReplaceTemplateVariable(_downstreamRoute); + _result = _downstreamUrlPathReplacer.ReplaceTemplateVariables(_downstreamRoute); } private void ThenTheDownstreamUrlPathIsReturned(string expected)