Feature/fix admin api caching wrong re routes (#421)

* #383 added failing test for this issue

* #383 identified issue was with cached load balancer for a given upstream path template based on the key we use, have modified this to include more data, I guess this might be an issue again for other things so I will have a think about it

* #383 fixed failing tests after key change

* Seems to be an issue with coveralls new package not being on nuget...try same version as their nuget package

* bash the old manual tests json back in

build.cake

@@ -5,7 +5,7 @@
 #tool "nuget:?package=OpenCover"
 #tool "nuget:?package=ReportGenerator"
 #tool "nuget:?package=coveralls.net&version=0.7.0"
-#addin Cake.Coveralls
+#addin Cake.Coveralls&version=0.7.0
 
 // compile
 var compileConfig = Argument("configuration", "Release");

src/Ocelot/Configuration/Creator/FileInternalConfigurationCreator.cs

@@ -252,7 +252,7 @@ namespace Ocelot.Configuration.Creator
                 return $"{nameof(CookieStickySessions)}:{fileReRoute.LoadBalancerOptions.Key}";
             }
 
-            return $"{fileReRoute.UpstreamPathTemplate}|{string.Join(",", fileReRoute.UpstreamHttpMethod)}";
+            return $"{fileReRoute.UpstreamPathTemplate}|{string.Join(",", fileReRoute.UpstreamHttpMethod)}|{string.Join(",", fileReRoute.DownstreamHostAndPorts.Select(x => $"{x.Host}:{x.Port}"))}";
         }
     }
 }

test/Ocelot.AcceptanceTests/HeaderTests.cs

@@ -185,6 +185,52 @@ namespace Ocelot.AcceptanceTests
                 .BDDfy();
         }
 
+        [Fact]
+        public void should_fix_issue_417()
+        {
+            var configuration = new FileConfiguration
+            {
+                ReRoutes = new List<FileReRoute>
+                    {
+                        new FileReRoute
+                        {
+                            DownstreamPathTemplate = "/",
+                            DownstreamScheme = "http",
+                            DownstreamHostAndPorts = new List<FileHostAndPort>
+                            {
+                                new FileHostAndPort
+                                {
+                                    Host = "localhost",
+                                    Port = 6773,
+                                }
+                            },
+                            UpstreamPathTemplate = "/",
+                            UpstreamHttpMethod = new List<string> { "Get" },
+                            DownstreamHeaderTransform = new Dictionary<string,string>
+                            {
+                                {"Location", "{DownstreamBaseUrl}, {BaseUrl}"}
+                            },
+                            HttpHandlerOptions = new FileHttpHandlerOptions
+                            {
+                                AllowAutoRedirect = false
+                            }
+                        }
+                    },
+                    GlobalConfiguration = new FileGlobalConfiguration
+                    {
+                        BaseUrl = "http://anotherapp.azurewebsites.net"
+                    }
+            };
+
+            this.Given(x => x.GivenThereIsAServiceRunningOn("http://localhost:6773", "/", 302, "Location", "http://localhost:6773/pay/Receive"))
+                .And(x => _steps.GivenThereIsAConfiguration(configuration))
+                .And(x => _steps.GivenOcelotIsRunning())
+                .When(x => _steps.WhenIGetUrlOnTheApiGateway("/"))
+                .Then(x => _steps.ThenTheStatusCodeShouldBe(HttpStatusCode.Redirect))
+                .And(x => _steps.ThenTheResponseHeaderIs("Location", "http://anotherapp.azurewebsites.net/pay/Receive"))
+                .BDDfy();
+        }
+
         [Fact]
         public void request_should_reuse_cookies_with_cookie_container()
         {
@@ -317,7 +363,7 @@ namespace Ocelot.AcceptanceTests
                     app.UsePathBase(basePath);
                     app.Run(async context =>
                     {
-                        if(context.Request.Headers.TryGetValue(headerKey, out var values))
+                        if (context.Request.Headers.TryGetValue(headerKey, out var values))
                         {
                             var result = values.First();
                             context.Response.StatusCode = statusCode;
@@ -330,7 +376,7 @@ namespace Ocelot.AcceptanceTests
             _builder.Start();
         }
 
-         private void GivenThereIsAServiceRunningOn(string baseUrl, string basePath, int statusCode, string headerKey, string headerValue)
+        private void GivenThereIsAServiceRunningOn(string baseUrl, string basePath, int statusCode, string headerKey, string headerValue)
         {
             _builder = new WebHostBuilder()
                 .UseUrls(baseUrl)
@@ -342,7 +388,8 @@ namespace Ocelot.AcceptanceTests
                     app.UsePathBase(basePath);
                     app.Run(context =>
                     {
-                        context.Response.OnStarting(() => {
+                        context.Response.OnStarting(() =>
+                        {
                             context.Response.Headers.Add(headerKey, headerValue);
                             context.Response.StatusCode = statusCode;
                             return Task.CompletedTask;

test/Ocelot.IntegrationTests/AdministrationTests.cs

@@ -11,6 +11,7 @@ using IdentityServer4.Models;
 using IdentityServer4.Test;
 using Microsoft.AspNetCore.Builder;
 using Microsoft.AspNetCore.Hosting;
+using Microsoft.AspNetCore.Http;
 using Microsoft.Extensions.Configuration;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
@@ -38,6 +39,8 @@ namespace Ocelot.IntegrationTests
         private IWebHostBuilder _webHostBuilderTwo;
         private IWebHost _builderTwo;
         private IWebHost _identityServerBuilder;
+        private IWebHost _fooServiceBuilder;
+        private IWebHost _barServiceBuilder;
 
         public AdministrationTests()
         {
@@ -276,6 +279,80 @@ namespace Ocelot.IntegrationTests
                 .BDDfy();
         }
 
+        [Fact]
+        public void should_get_file_configuration_edit_and_post_updated_version_redirecting_reroute()
+        {
+            var fooPort = 47689;
+            var barPort = 47690;
+
+            var initialConfiguration = new FileConfiguration
+            {
+                ReRoutes = new List<FileReRoute>()
+                {
+                    new FileReRoute()
+                    {
+                        DownstreamHostAndPorts = new List<FileHostAndPort>
+                        {
+                            new FileHostAndPort
+                            {
+                                Host = "localhost",
+                                Port = fooPort,
+                            }
+                        },
+                        DownstreamScheme = "http",
+                        DownstreamPathTemplate = "/foo",
+                        UpstreamHttpMethod = new List<string> { "get" },
+                        UpstreamPathTemplate = "/foo"
+                    }
+                }
+            };
+
+             var updatedConfiguration = new FileConfiguration
+            {
+                GlobalConfiguration = new FileGlobalConfiguration
+                {
+                },
+                ReRoutes = new List<FileReRoute>()
+                {
+                    new FileReRoute()
+                    {
+                        DownstreamHostAndPorts = new List<FileHostAndPort>
+                        {
+                            new FileHostAndPort
+                            {
+                                Host = "localhost",
+                                Port = barPort,
+                            }
+                        },
+                        DownstreamScheme = "http",
+                        DownstreamPathTemplate = "/bar",
+                        UpstreamHttpMethod = new List<string> { "get" },
+                        UpstreamPathTemplate = "/foo"
+                    }
+                }
+            };
+
+            this.Given(x => GivenThereIsAConfiguration(initialConfiguration))
+                .And(x => GivenThereIsAFooServiceRunningOn($"http://localhost:{fooPort}"))
+                .And(x => GivenThereIsABarServiceRunningOn($"http://localhost:{barPort}"))
+                .And(x => GivenOcelotIsRunning())
+                .And(x => WhenIGetUrlOnTheApiGateway("/foo"))
+                .Then(x => ThenTheResponseBodyShouldBe("foo"))
+                .And(x => GivenIHaveAnOcelotToken("/administration"))
+                .And(x => GivenIHaveAddedATokenToMyRequest())
+                .When(x => WhenIPostOnTheApiGateway("/administration/configuration", updatedConfiguration))
+                .Then(x => ThenTheStatusCodeShouldBe(HttpStatusCode.OK))
+                .And(x => ThenTheResponseShouldBe(updatedConfiguration))
+                .And(x => WhenIGetUrlOnTheApiGateway("/foo"))
+                .Then(x => ThenTheResponseBodyShouldBe("bar"))
+                .When(x => WhenIPostOnTheApiGateway("/administration/configuration", initialConfiguration))
+                .Then(x => ThenTheStatusCodeShouldBe(HttpStatusCode.OK))
+                .And(x => ThenTheResponseShouldBe(initialConfiguration))
+                .And(x => WhenIGetUrlOnTheApiGateway("/foo"))
+                .Then(x => ThenTheResponseBodyShouldBe("foo"))
+                .BDDfy();
+        }
+
         [Fact]
         public void should_clear_region()
         {
@@ -516,6 +593,12 @@ namespace Ocelot.IntegrationTests
             result.Value.ShouldBe(expected);
         }
 
+        private void ThenTheResponseBodyShouldBe(string expected)
+        {
+            var content = _response.Content.ReadAsStringAsync().Result;
+            content.ShouldBe(expected);
+        }
+
         private void ThenTheResponseShouldBe(FileConfiguration expecteds)
         {
             var response = JsonConvert.DeserializeObject<FileConfiguration>(_response.Content.ReadAsStringAsync().Result);
@@ -723,5 +806,47 @@ namespace Ocelot.IntegrationTests
             _httpClient?.Dispose();
             _identityServerBuilder?.Dispose();
         }
+
+        private void GivenThereIsAFooServiceRunningOn(string baseUrl)
+        {
+            _fooServiceBuilder = new WebHostBuilder()
+                .UseUrls(baseUrl)
+                .UseKestrel()
+                .UseContentRoot(Directory.GetCurrentDirectory())
+                .UseIISIntegration()
+                .Configure(app =>
+                {
+                    app.UsePathBase("/foo");
+                    app.Run(async context =>
+                    {   
+                        context.Response.StatusCode = 200;
+                        await context.Response.WriteAsync("foo");
+                    });
+                })
+                .Build();
+
+            _fooServiceBuilder.Start();
+        }
+
+        private void GivenThereIsABarServiceRunningOn(string baseUrl)
+        {
+            _barServiceBuilder = new WebHostBuilder()
+                .UseUrls(baseUrl)
+                .UseKestrel()
+                .UseContentRoot(Directory.GetCurrentDirectory())
+                .UseIISIntegration()
+                .Configure(app =>
+                {
+                    app.UsePathBase("/bar");
+                    app.Run(async context =>
+                    {   
+                        context.Response.StatusCode = 200;
+                        await context.Response.WriteAsync("bar");
+                    });
+                })
+                .Build();
+
+            _barServiceBuilder.Start();
+        }
     }
 }

test/Ocelot.UnitTests/Configuration/FileInternalConfigurationCreatorTests.cs

@@ -199,7 +199,7 @@
                 .WithDownstreamScheme("http")
                 .WithUpstreamHttpMethod(new List<string>() {"Get"})
                 .WithDownstreamAddresses(new List<DownstreamHostAndPort>() {new DownstreamHostAndPort("localhost", 51878)})
-                .WithLoadBalancerKey("/laura|Get")
+                .WithLoadBalancerKey("/laura|Get|localhost:51878")
                 .Build();
 
             var lauraReRoute = new ReRouteBuilder()
@@ -218,7 +218,7 @@
                 .WithDownstreamScheme("http")
                 .WithUpstreamHttpMethod(new List<string>() { "Get" })
                 .WithDownstreamAddresses(new List<DownstreamHostAndPort>() { new DownstreamHostAndPort("localhost", 51878) })
-                .WithLoadBalancerKey("/tom|Get")
+                .WithLoadBalancerKey("/tom|Get|localhost:51880")
                 .Build();
 
             var tomReRoute = new ReRouteBuilder()
@@ -409,7 +409,7 @@
                 .WithDownstreamPathTemplate("/products/{productId}")
                 .WithUpstreamPathTemplate("/api/products/{productId}")
                 .WithUpstreamHttpMethod(new List<string> {"Get"})
-                .WithLoadBalancerKey("/api/products/{productId}|Get")
+                .WithLoadBalancerKey("/api/products/{productId}|Get|127.0.0.1:0")
                 .Build();
 
             this.Given(x => x.GivenTheConfigIs(new FileConfiguration
@@ -461,7 +461,7 @@
                 .WithUpstreamPathTemplate("/api/products/{productId}")
                 .WithUpstreamHttpMethod(new List<string> {"Get"})
                 .WithDelegatingHandlers(handlers)
-                .WithLoadBalancerKey("/api/products/{productId}|Get")
+                .WithLoadBalancerKey("/api/products/{productId}|Get|")
                 .Build();
 
             this.Given(x => x.GivenTheConfigIs(new FileConfiguration
@@ -506,7 +506,7 @@
                 .WithUpstreamHttpMethod(new List<string> {"Get"})
                 .WithUseServiceDiscovery(true)
                 .WithServiceName("ProductService")
-                .WithLoadBalancerKey("/api/products/{productId}|Get")
+                .WithLoadBalancerKey("/api/products/{productId}|Get|")
                 .Build();
 
             this.Given(x => x.GivenTheConfigIs(new FileConfiguration
@@ -557,7 +557,7 @@
                 .WithUpstreamPathTemplate("/api/products/{productId}")
                 .WithUpstreamHttpMethod(new List<string> {"Get"})
                 .WithUseServiceDiscovery(false)
-                .WithLoadBalancerKey("/api/products/{productId}|Get")
+                .WithLoadBalancerKey("/api/products/{productId}|Get|")
                 .Build();
 
             this.Given(x => x.GivenTheConfigIs(new FileConfiguration
@@ -600,7 +600,7 @@
                 .WithUpstreamPathTemplate("/api/products/{productId}")
                 .WithUpstreamHttpMethod(new List<string> {"Get"})
                 .WithUpstreamTemplatePattern(new UpstreamPathTemplate("(?i)/api/products/.*/$", 1))
-                .WithLoadBalancerKey("/api/products/{productId}|Get")
+                .WithLoadBalancerKey("/api/products/{productId}|Get|")
                 .Build();
 
             this.Given(x => x.GivenTheConfigIs(new FileConfiguration
@@ -645,7 +645,7 @@
                 .WithUpstreamPathTemplate("/api/products/{productId}")
                 .WithUpstreamHttpMethod(new List<string> {"Get"})
                 .WithRequestIdKey("blahhhh")
-                .WithLoadBalancerKey("/api/products/{productId}|Get")
+                .WithLoadBalancerKey("/api/products/{productId}|Get|")
                 .Build();
 
             this.Given(x => x.GivenTheConfigIs(new FileConfiguration
@@ -740,7 +740,7 @@
                 {
                     new ClaimToThing("CustomerId", "CustomerId", "", 0),
                 })
-                .WithLoadBalancerKey("/api/products/{productId}|Get")
+                .WithLoadBalancerKey("/api/products/{productId}|Get|")
                 .Build();
 
             var expected = new List<ReRoute>
@@ -783,7 +783,7 @@
                 .WithUpstreamPathTemplate("/api/products/{productId}")
                 .WithUpstreamHttpMethod(new List<string> {"Get"})
                 .WithAuthenticationOptions(authenticationOptions)
-                .WithLoadBalancerKey("/api/products/{productId}|Get")
+                .WithLoadBalancerKey("/api/products/{productId}|Get|")
                 .Build();
 
             var expected = new List<ReRoute>