From 7ef26f5f4b114f5fe29450bdc19da1e14de6cc77 Mon Sep 17 00:00:00 2001 From: Tom Gardham-Pallister Date: Tue, 29 Aug 2017 20:47:52 +0100 Subject: [PATCH] fixes issue #117 --- .../Finder/DownstreamRouteFinder.cs | 3 ++ .../DownstreamRouteFinderMiddleware.cs | 2 +- test/Ocelot.AcceptanceTests/RoutingTests.cs | 37 +++++++++++++++- .../DownstreamRouteFinderTests.cs | 43 ++++++++++++++++++- 4 files changed, 81 insertions(+), 4 deletions(-) diff --git a/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs b/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs index bf0d5a52..cd2c098f 100644 --- a/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs +++ b/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs @@ -6,6 +6,7 @@ using Ocelot.Configuration.Provider; using Ocelot.DownstreamRouteFinder.UrlMatcher; using Ocelot.Errors; using Ocelot.Responses; +using Ocelot.Utilities; namespace Ocelot.DownstreamRouteFinder.Finder { @@ -24,6 +25,8 @@ namespace Ocelot.DownstreamRouteFinder.Finder public async Task> FindDownstreamRoute(string upstreamUrlPath, string upstreamHttpMethod) { + upstreamUrlPath = upstreamUrlPath.SetLastCharacterAs('/'); + var configuration = await _configProvider.Get(); var applicableReRoutes = configuration.Data.ReRoutes.Where(r => r.UpstreamHttpMethod.Count == 0 || r.UpstreamHttpMethod.Select(x => x.Method.ToLower()).Contains(upstreamHttpMethod.ToLower())); diff --git a/src/Ocelot/DownstreamRouteFinder/Middleware/DownstreamRouteFinderMiddleware.cs b/src/Ocelot/DownstreamRouteFinder/Middleware/DownstreamRouteFinderMiddleware.cs index 51cb3a0b..f421d6d8 100644 --- a/src/Ocelot/DownstreamRouteFinder/Middleware/DownstreamRouteFinderMiddleware.cs +++ b/src/Ocelot/DownstreamRouteFinder/Middleware/DownstreamRouteFinderMiddleware.cs @@ -30,7 +30,7 @@ namespace Ocelot.DownstreamRouteFinder.Middleware public async Task Invoke(HttpContext context) { - var upstreamUrlPath = context.Request.Path.ToString().SetLastCharacterAs('/'); + var upstreamUrlPath = context.Request.Path.ToString(); _logger.LogDebug("upstream url path is {upstreamUrlPath}", upstreamUrlPath); diff --git a/test/Ocelot.AcceptanceTests/RoutingTests.cs b/test/Ocelot.AcceptanceTests/RoutingTests.cs index b32c4690..fbdf57e1 100644 --- a/test/Ocelot.AcceptanceTests/RoutingTests.cs +++ b/test/Ocelot.AcceptanceTests/RoutingTests.cs @@ -6,6 +6,7 @@ using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; using Ocelot.Configuration.File; +using Shouldly; using TestStack.BDDfy; using Xunit; @@ -15,6 +16,7 @@ namespace Ocelot.AcceptanceTests { private IWebHost _builder; private readonly Steps _steps; + private string _downstreamPath; public RoutingTests() { @@ -232,6 +234,34 @@ namespace Ocelot.AcceptanceTests .BDDfy(); } + + [Fact] + public void should_not_add_trailing_slash_to_downstream_url() + { + var configuration = new FileConfiguration + { + ReRoutes = new List + { + new FileReRoute + { + DownstreamPathTemplate = "/api/products/{productId}", + DownstreamScheme = "http", + DownstreamHost = "localhost", + DownstreamPort = 51879, + UpstreamPathTemplate = "/products/{productId}", + UpstreamHttpMethod = new List { "Get" }, + } + } + }; + + this.Given(x => GivenThereIsAServiceRunningOn("http://localhost:51879/api/products/1", 200, "Some Product")) + .And(x => _steps.GivenThereIsAConfiguration(configuration)) + .And(x => _steps.GivenOcelotIsRunning()) + .When(x => _steps.WhenIGetUrlOnTheApiGateway("/products/1")) + .Then(x => ThenTheDownstreamUrlPathShouldBe("/api/products/1")) + .BDDfy(); + } + [Fact] public void should_return_response_201_with_simple_url() { @@ -379,11 +409,11 @@ namespace Ocelot.AcceptanceTests .UseKestrel() .UseContentRoot(Directory.GetCurrentDirectory()) .UseIISIntegration() - .UseUrls(url) .Configure(app => { app.Run(async context => { + _downstreamPath = context.Request.PathBase.Value; context.Response.StatusCode = statusCode; await context.Response.WriteAsync(responseBody); }); @@ -393,6 +423,11 @@ namespace Ocelot.AcceptanceTests _builder.Start(); } + internal void ThenTheDownstreamUrlPathShouldBe(string expectedDownstreamPath) + { + _downstreamPath.ShouldBe(expectedDownstreamPath); + } + public void Dispose() { _builder?.Dispose(); diff --git a/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderTests.cs b/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderTests.cs index 8514786d..8dbe6ef4 100644 --- a/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderTests.cs +++ b/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderTests.cs @@ -36,7 +36,7 @@ namespace Ocelot.UnitTests.DownstreamRouteFinder [Fact] public void should_return_route() { - this.Given(x => x.GivenThereIsAnUpstreamUrlPath("matchInUrlMatcher")) + this.Given(x => x.GivenThereIsAnUpstreamUrlPath("matchInUrlMatcher/")) .And(x =>x.GivenTheTemplateVariableAndNameFinderReturns( new OkResponse>( new List()))) @@ -65,6 +65,39 @@ namespace Ocelot.UnitTests.DownstreamRouteFinder .BDDfy(); } + + [Fact] + public void should_append_slash_to_upstream_url_path() + { + this.Given(x => x.GivenThereIsAnUpstreamUrlPath("matchInUrlMatcher")) + .And(x =>x.GivenTheTemplateVariableAndNameFinderReturns( + new OkResponse>( + new List()))) + .And(x => x.GivenTheConfigurationIs(new List + { + new ReRouteBuilder() + .WithDownstreamPathTemplate("someDownstreamPath") + .WithUpstreamPathTemplate("someUpstreamPath") + .WithUpstreamHttpMethod(new List { "Get" }) + .WithUpstreamTemplatePattern("someUpstreamPath") + .Build() + }, string.Empty + )) + .And(x => x.GivenTheUrlMatcherReturns(new OkResponse(new UrlMatch(true)))) + .And(x => x.GivenTheUpstreamHttpMethodIs("Get")) + .When(x => x.WhenICallTheFinder()) + .Then( + x => x.ThenTheFollowingIsReturned(new DownstreamRoute( + new List(), + new ReRouteBuilder() + .WithDownstreamPathTemplate("someDownstreamPath") + .WithUpstreamHttpMethod(new List { "Get" }) + .Build() + ))) + .And(x => x.ThenTheUrlMatcherIsCalledCorrectly("matchInUrlMatcher/")) + .BDDfy(); + } + [Fact] public void should_return_route_if_upstream_path_and_upstream_template_are_the_same() { @@ -137,7 +170,7 @@ namespace Ocelot.UnitTests.DownstreamRouteFinder [Fact] public void should_not_return_route() { - this.Given(x => x.GivenThereIsAnUpstreamUrlPath("dontMatchPath")) + this.Given(x => x.GivenThereIsAnUpstreamUrlPath("dontMatchPath/")) .And(x => x.GivenTheConfigurationIs(new List { new ReRouteBuilder() @@ -269,6 +302,12 @@ namespace Ocelot.UnitTests.DownstreamRouteFinder .Verify(x => x.Match(_upstreamUrlPath, _reRoutesConfig[0].UpstreamPathTemplate.Value), Times.Once); } + private void ThenTheUrlMatcherIsCalledCorrectly(string expectedUpstreamUrlPath) + { + _mockMatcher + .Verify(x => x.Match(expectedUpstreamUrlPath, _reRoutesConfig[0].UpstreamPathTemplate.Value), Times.Once); + } + private void ThenTheUrlMatcherIsNotCalled() { _mockMatcher