From 0c5e09d18f55d89aa2c279d2bd6c4ac04c480db9 Mon Sep 17 00:00:00 2001 From: TomPallister Date: Sun, 9 Feb 2020 15:51:22 +0000 Subject: [PATCH] moved logic to request mapper and public setter gone --- src/Ocelot/Request/Mapper/IRequestMapper.cs | 3 +- src/Ocelot/Request/Mapper/RequestMapper.cs | 12 +++-- .../Request/Middleware/DownstreamRequest.cs | 2 +- .../DownstreamRequestInitialiserMiddleware.cs | 7 +-- ...streamRequestInitialiserMiddlewareTests.cs | 22 +++------ .../Request/Mapper/RequestMapperTests.cs | 47 ++++++++++++++----- 6 files changed, 56 insertions(+), 37 deletions(-) diff --git a/src/Ocelot/Request/Mapper/IRequestMapper.cs b/src/Ocelot/Request/Mapper/IRequestMapper.cs index d16a2d06..cd075c5d 100644 --- a/src/Ocelot/Request/Mapper/IRequestMapper.cs +++ b/src/Ocelot/Request/Mapper/IRequestMapper.cs @@ -1,12 +1,13 @@ namespace Ocelot.Request.Mapper { using Microsoft.AspNetCore.Http; + using Ocelot.Configuration; using Ocelot.Responses; using System.Net.Http; using System.Threading.Tasks; public interface IRequestMapper { - Task> Map(HttpRequest request); + Task> Map(HttpRequest request, DownstreamReRoute downstreamReRoute); } } diff --git a/src/Ocelot/Request/Mapper/RequestMapper.cs b/src/Ocelot/Request/Mapper/RequestMapper.cs index f669c286..e050f1a6 100644 --- a/src/Ocelot/Request/Mapper/RequestMapper.cs +++ b/src/Ocelot/Request/Mapper/RequestMapper.cs @@ -3,6 +3,7 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Extensions; using Microsoft.Extensions.Primitives; + using Ocelot.Configuration; using Ocelot.Responses; using System; using System.Collections.Generic; @@ -15,14 +16,14 @@ { private readonly string[] _unsupportedHeaders = { "host" }; - public async Task> Map(HttpRequest request) + public async Task> Map(HttpRequest request, DownstreamReRoute downstreamReRoute) { try { var requestMessage = new HttpRequestMessage() { Content = await MapContent(request), - Method = MapMethod(request), + Method = MapMethod(request, downstreamReRoute), RequestUri = MapUri(request) }; @@ -71,8 +72,13 @@ } } - private HttpMethod MapMethod(HttpRequest request) + private HttpMethod MapMethod(HttpRequest request, DownstreamReRoute downstreamReRoute) { + if (!string.IsNullOrEmpty(downstreamReRoute?.DownstreamHttpMethod)) + { + return new HttpMethod(downstreamReRoute.DownstreamHttpMethod); + } + return new HttpMethod(request.Method); } diff --git a/src/Ocelot/Request/Middleware/DownstreamRequest.cs b/src/Ocelot/Request/Middleware/DownstreamRequest.cs index 24d96cfe..cf973d94 100644 --- a/src/Ocelot/Request/Middleware/DownstreamRequest.cs +++ b/src/Ocelot/Request/Middleware/DownstreamRequest.cs @@ -24,7 +24,7 @@ namespace Ocelot.Request.Middleware public HttpRequestHeaders Headers { get; } - public string Method { get; set; } + public string Method { get; } public string OriginalString { get; } diff --git a/src/Ocelot/Request/Middleware/DownstreamRequestInitialiserMiddleware.cs b/src/Ocelot/Request/Middleware/DownstreamRequestInitialiserMiddleware.cs index c9f6f859..83a2ecb9 100644 --- a/src/Ocelot/Request/Middleware/DownstreamRequestInitialiserMiddleware.cs +++ b/src/Ocelot/Request/Middleware/DownstreamRequestInitialiserMiddleware.cs @@ -24,7 +24,7 @@ namespace Ocelot.Request.Middleware public async Task Invoke(DownstreamContext context) { - var downstreamRequest = await _requestMapper.Map(context.HttpContext.Request); + var downstreamRequest = await _requestMapper.Map(context.HttpContext.Request, context.DownstreamReRoute); if (downstreamRequest.IsError) { @@ -34,11 +34,6 @@ namespace Ocelot.Request.Middleware context.DownstreamRequest = _creator.Create(downstreamRequest.Data); - if (!string.IsNullOrEmpty(context.DownstreamReRoute?.DownstreamHttpMethod)) - { - context.DownstreamRequest.Method = context.DownstreamReRoute.DownstreamHttpMethod; - } - await _next.Invoke(context); } } diff --git a/test/Ocelot.UnitTests/Request/DownstreamRequestInitialiserMiddlewareTests.cs b/test/Ocelot.UnitTests/Request/DownstreamRequestInitialiserMiddlewareTests.cs index aadc551c..13bf024b 100644 --- a/test/Ocelot.UnitTests/Request/DownstreamRequestInitialiserMiddlewareTests.cs +++ b/test/Ocelot.UnitTests/Request/DownstreamRequestInitialiserMiddlewareTests.cs @@ -12,6 +12,7 @@ using Ocelot.Responses; using Shouldly; using System.Net.Http; + using Ocelot.Configuration; using TestStack.BDDfy; using Xunit; @@ -69,20 +70,16 @@ .BDDfy(); } - [Theory] - [InlineData("POST", "POST")] - [InlineData(null, "GET")] - [InlineData("", "GET")] - public void Should_map_downstream_reroute_method_to_downstream_request(string input, string expected) + [Fact] + public void Should_map_downstream_reroute_method_to_downstream_request() { this.Given(_ => GivenTheHttpContextContainsARequest()) - .And(_ => GivenTheDownstreamReRouteMethodIs(input)) .And(_ => GivenTheMapperWillReturnAMappedRequest()) .When(_ => WhenTheMiddlewareIsInvoked()) .Then(_ => ThenTheContexRequestIsMappedToADownstreamRequest()) .And(_ => ThenTheDownstreamRequestIsStored()) .And(_ => ThenTheNextMiddlewareIsInvoked()) - .And(_ => ThenTheDownstreamRequestMethodIs(expected)) + .And(_ => ThenTheDownstreamRequestMethodIs("GET")) .BDDfy(); } @@ -98,11 +95,6 @@ .BDDfy(); } - private void GivenTheDownstreamReRouteMethodIs(string input) - { - _downstreamContext.DownstreamReRoute = new DownstreamReRouteBuilder().WithDownStreamHttpMethod(input).Build(); - } - private void ThenTheDownstreamRequestMethodIs(string expected) { _downstreamContext.DownstreamRequest.Method.ShouldBe(expected); @@ -120,7 +112,7 @@ _mappedRequest = new OkResponse(new HttpRequestMessage(HttpMethod.Get, "http://www.bbc.co.uk")); _requestMapper - .Setup(rm => rm.Map(It.IsAny())) + .Setup(rm => rm.Map(It.IsAny(), It.IsAny())) .ReturnsAsync(_mappedRequest); } @@ -129,7 +121,7 @@ _mappedRequest = new ErrorResponse(new UnmappableRequestError(new System.Exception("boooom!"))); _requestMapper - .Setup(rm => rm.Map(It.IsAny())) + .Setup(rm => rm.Map(It.IsAny(), It.IsAny())) .ReturnsAsync(_mappedRequest); } @@ -140,7 +132,7 @@ private void ThenTheContexRequestIsMappedToADownstreamRequest() { - _requestMapper.Verify(rm => rm.Map(_httpRequest.Object), Times.Once); + _requestMapper.Verify(rm => rm.Map(_httpRequest.Object, _downstreamContext.DownstreamReRoute), Times.Once); } private void ThenTheDownstreamRequestIsStored() diff --git a/test/Ocelot.UnitTests/Request/Mapper/RequestMapperTests.cs b/test/Ocelot.UnitTests/Request/Mapper/RequestMapperTests.cs index ac81a4d4..77091fc6 100644 --- a/test/Ocelot.UnitTests/Request/Mapper/RequestMapperTests.cs +++ b/test/Ocelot.UnitTests/Request/Mapper/RequestMapperTests.cs @@ -13,6 +13,8 @@ using System.Security.Cryptography; using System.Text; using System.Threading.Tasks; + using Ocelot.Configuration; + using Ocelot.Configuration.Builder; using TestStack.BDDfy; using Xunit; @@ -27,6 +29,8 @@ private List> _inputHeaders = null; + private DownstreamReRoute _downstreamReRoute; + public RequestMapperTests() { _httpContext = new DefaultHttpContext(); @@ -82,6 +86,21 @@ .BDDfy(); } + [Theory] + [InlineData("", "GET")] + [InlineData(null, "GET")] + [InlineData("POST", "POST")] + public void Should_use_downstream_reroute_method_if_set(string input, string expected) + { + this.Given(_ => GivenTheInputRequestHasMethod("GET")) + .And(_ => GivenTheDownstreamReRouteMethodIs(input)) + .And(_ => GivenTheInputRequestHasAValidUri()) + .When(_ => WhenMapped()) + .Then(_ => ThenNoErrorIsReturned()) + .And(_ => ThenTheMappedRequestHasMethod(expected)) + .BDDfy(); + } + [Fact] public void Should_map_all_headers() { @@ -154,16 +173,6 @@ .BDDfy(); } - private void GivenTheInputRequestHasNoContentLength() - { - _inputRequest.ContentLength = null; - } - - private void GivenTheInputRequestHasNoContentType() - { - _inputRequest.ContentType = null; - } - [Fact] public void Should_map_content_headers() { @@ -212,6 +221,22 @@ .BDDfy(); } + private void GivenTheDownstreamReRouteMethodIs(string input) + { + _downstreamReRoute = new DownstreamReRouteBuilder().WithDownStreamHttpMethod(input).Build(); + } + + private void GivenTheInputRequestHasNoContentLength() + { + _inputRequest.ContentLength = null; + } + + private void GivenTheInputRequestHasNoContentType() + { + _inputRequest.ContentType = null; + } + + private void ThenTheContentHeadersAreNotAddedToNonContentHeaders() { _mappedRequest.Data.Headers.ShouldNotContain(x => x.Key == "Content-Disposition"); @@ -380,7 +405,7 @@ private async Task WhenMapped() { - _mappedRequest = await _requestMapper.Map(_inputRequest); + _mappedRequest = await _requestMapper.Map(_inputRequest, _downstreamReRoute); } private void ThenNoErrorIsReturned()