From 6495891a07254ed8be316f5c464f2c93831d8846 Mon Sep 17 00:00:00 2001 From: Felix Boers Date: Fri, 11 Jan 2019 14:54:54 +0100 Subject: [PATCH] Support placeholder in service fabric services names (#722) --- .../DownstreamUrlCreatorMiddleware.cs | 9 ++-- ...wnstreamTemplatePathPlaceholderReplacer.cs | 5 +- .../IDownstreamPathPlaceholderReplacer.cs | 2 +- .../ServiceFabricTests.cs | 36 ++++++++++++++ .../DownstreamUrlCreatorMiddlewareTests.cs | 48 +++++++++++++++++-- ...eamUrlPathTemplateVariableReplacerTests.cs | 2 +- 6 files changed, 90 insertions(+), 12 deletions(-) diff --git a/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs b/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs index 02b40c071..1e545d28f 100644 --- a/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs +++ b/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs @@ -27,7 +27,7 @@ public DownstreamUrlCreatorMiddleware(OcelotRequestDelegate next, public async Task Invoke(DownstreamContext context) { var response = _replacer - .Replace(context.DownstreamReRoute.DownstreamPathTemplate, context.TemplatePlaceholderNameAndValues); + .Replace(context.DownstreamReRoute.DownstreamPathTemplate.Value, context.TemplatePlaceholderNameAndValues); if (response.IsError) { @@ -103,7 +103,7 @@ private string GetPath(DownstreamPath dsPath) return dsPath.Value.Substring(0, dsPath.Value.IndexOf("?", StringComparison.Ordinal)); } - private string GetQueryString(DownstreamPath dsPath) + private string GetQueryString(DownstreamPath dsPath) { return dsPath.Value.Substring(dsPath.Value.IndexOf("?", StringComparison.Ordinal)); } @@ -116,8 +116,9 @@ private bool ContainsQueryString(DownstreamPath dsPath) private (string path, string query) CreateServiceFabricUri(DownstreamContext context, Response dsPath) { var query = context.DownstreamRequest.Query; - var serviceFabricPath = $"/{context.DownstreamReRoute.ServiceName + dsPath.Data.Value}"; - return (serviceFabricPath, query); + var serviceName = _replacer.Replace(context.DownstreamReRoute.ServiceName, context.TemplatePlaceholderNameAndValues); + var pathTemplate = $"/{serviceName.Data.Value}{dsPath.Data.Value}"; + return (pathTemplate, query); } private static bool ServiceFabricRequest(DownstreamContext context) diff --git a/src/Ocelot/DownstreamUrlCreator/UrlTemplateReplacer/DownstreamTemplatePathPlaceholderReplacer.cs b/src/Ocelot/DownstreamUrlCreator/UrlTemplateReplacer/DownstreamTemplatePathPlaceholderReplacer.cs index 76588596e..ac91ef2e4 100644 --- a/src/Ocelot/DownstreamUrlCreator/UrlTemplateReplacer/DownstreamTemplatePathPlaceholderReplacer.cs +++ b/src/Ocelot/DownstreamUrlCreator/UrlTemplateReplacer/DownstreamTemplatePathPlaceholderReplacer.cs @@ -8,11 +8,12 @@ namespace Ocelot.DownstreamUrlCreator.UrlTemplateReplacer { public class DownstreamTemplatePathPlaceholderReplacer : IDownstreamPathPlaceholderReplacer { - public Response Replace(DownstreamPathTemplate downstreamPathTemplate, List urlPathPlaceholderNameAndValues) + public Response Replace(string downstreamPathTemplate, + List urlPathPlaceholderNameAndValues) { var downstreamPath = new StringBuilder(); - downstreamPath.Append(downstreamPathTemplate.Value); + downstreamPath.Append(downstreamPathTemplate); foreach (var placeholderVariableAndValue in urlPathPlaceholderNameAndValues) { diff --git a/src/Ocelot/DownstreamUrlCreator/UrlTemplateReplacer/IDownstreamPathPlaceholderReplacer.cs b/src/Ocelot/DownstreamUrlCreator/UrlTemplateReplacer/IDownstreamPathPlaceholderReplacer.cs index dbfeaa895..237f20fc1 100644 --- a/src/Ocelot/DownstreamUrlCreator/UrlTemplateReplacer/IDownstreamPathPlaceholderReplacer.cs +++ b/src/Ocelot/DownstreamUrlCreator/UrlTemplateReplacer/IDownstreamPathPlaceholderReplacer.cs @@ -7,6 +7,6 @@ namespace Ocelot.DownstreamUrlCreator.UrlTemplateReplacer { public interface IDownstreamPathPlaceholderReplacer { - Response Replace(DownstreamPathTemplate downstreamPathTemplate, List urlPathPlaceholderNameAndValues); + Response Replace(string downstreamPathTemplate, List urlPathPlaceholderNameAndValues); } } diff --git a/test/Ocelot.AcceptanceTests/ServiceFabricTests.cs b/test/Ocelot.AcceptanceTests/ServiceFabricTests.cs index 4e92b54d9..606bd6917 100644 --- a/test/Ocelot.AcceptanceTests/ServiceFabricTests.cs +++ b/test/Ocelot.AcceptanceTests/ServiceFabricTests.cs @@ -128,6 +128,42 @@ public void should_support_service_fabric_naming_and_dns_service_statefull_and_a .BDDfy(); } + [Fact] + public void should_support_placeholder_in_service_fabric_service_name() + { + var configuration = new FileConfiguration + { + ReRoutes = new List + { + new FileReRoute + { + DownstreamPathTemplate = "/values", + DownstreamScheme = "http", + UpstreamPathTemplate = "/api/{version}/values", + UpstreamHttpMethod = new List { "Get" }, + ServiceName = "Service_{version}/Api" + } + }, + GlobalConfiguration = new FileGlobalConfiguration + { + ServiceDiscoveryProvider = new FileServiceDiscoveryProvider() + { + Host = "localhost", + Port = 19081, + Type = "ServiceFabric" + } + } + }; + + this.Given(x => x.GivenThereIsAServiceRunningOn("http://localhost:19081", "/Service_1.0/Api/values", 200, "Hello from Laura", "test=best")) + .And(x => _steps.GivenThereIsAConfiguration(configuration)) + .And(x => _steps.GivenOcelotIsRunning()) + .When(x => _steps.WhenIGetUrlOnTheApiGateway("/api/1.0/values?test=best")) + .Then(x => _steps.ThenTheStatusCodeShouldBe(HttpStatusCode.OK)) + .And(x => _steps.ThenTheResponseBodyShouldBe("Hello from Laura")) + .BDDfy(); + } + private void GivenThereIsAServiceRunningOn(string baseUrl, string basePath, int statusCode, string responseBody, string expectedQueryString) { _serviceHandler.GivenThereIsAServiceRunningOn(baseUrl, basePath, async context => diff --git a/test/Ocelot.UnitTests/DownstreamUrlCreator/DownstreamUrlCreatorMiddlewareTests.cs b/test/Ocelot.UnitTests/DownstreamUrlCreator/DownstreamUrlCreatorMiddlewareTests.cs index 014326f4b..612ee5347 100644 --- a/test/Ocelot.UnitTests/DownstreamUrlCreator/DownstreamUrlCreatorMiddlewareTests.cs +++ b/test/Ocelot.UnitTests/DownstreamUrlCreator/DownstreamUrlCreatorMiddlewareTests.cs @@ -223,7 +223,7 @@ public void should_create_service_fabric_url() this.Given(x => x.GivenTheDownStreamRouteIs(downstreamRoute)) .And(x => GivenTheServiceProviderConfigIs(config)) .And(x => x.GivenTheDownstreamRequestUriIs("http://localhost:19081")) - .And(x => x.GivenTheUrlReplacerWillReturn("/api/products/1")) + .And(x => x.GivenTheUrlReplacerWillReturnSequence("/api/products/1", "Ocelot/OcelotApp")) .When(x => x.WhenICallTheMiddleware()) .Then(x => x.ThenTheDownstreamRequestUriIs("http://localhost:19081/Ocelot/OcelotApp/api/products/1")) .BDDfy(); @@ -253,7 +253,7 @@ public void should_create_service_fabric_url_with_query_string_for_stateless_ser this.Given(x => x.GivenTheDownStreamRouteIs(downstreamRoute)) .And(x => GivenTheServiceProviderConfigIs(config)) .And(x => x.GivenTheDownstreamRequestUriIs("http://localhost:19081?Tom=test&laura=1")) - .And(x => x.GivenTheUrlReplacerWillReturn("/api/products/1")) + .And(x => x.GivenTheUrlReplacerWillReturnSequence("/api/products/1", "Ocelot/OcelotApp")) .When(x => x.WhenICallTheMiddleware()) .Then(x => x.ThenTheDownstreamRequestUriIs("http://localhost:19081/Ocelot/OcelotApp/api/products/1?Tom=test&laura=1")) .BDDfy(); @@ -283,12 +283,40 @@ public void should_create_service_fabric_url_with_query_string_for_stateful_serv this.Given(x => x.GivenTheDownStreamRouteIs(downstreamRoute)) .And(x => GivenTheServiceProviderConfigIs(config)) .And(x => x.GivenTheDownstreamRequestUriIs("http://localhost:19081?PartitionKind=test&PartitionKey=1")) - .And(x => x.GivenTheUrlReplacerWillReturn("/api/products/1")) + .And(x => x.GivenTheUrlReplacerWillReturnSequence("/api/products/1", "Ocelot/OcelotApp")) .When(x => x.WhenICallTheMiddleware()) .Then(x => x.ThenTheDownstreamRequestUriIs("http://localhost:19081/Ocelot/OcelotApp/api/products/1?PartitionKind=test&PartitionKey=1")) .BDDfy(); } + [Fact] + public void should_create_service_fabric_url_with_version_from_upstream_path_template() + { + var downstreamRoute = new DownstreamRoute( + new List(), + new ReRouteBuilder().WithDownstreamReRoute( + new DownstreamReRouteBuilder() + .WithDownstreamScheme("http") + .WithUpstreamPathTemplate(new UpstreamPathTemplateBuilder().WithOriginalValue("/products").Build()) + .WithUseServiceDiscovery(true) + .Build() + ).Build()); + + var config = new ServiceProviderConfigurationBuilder() + .WithType("ServiceFabric") + .WithHost("localhost") + .WithPort(19081) + .Build(); + + this.Given(x => x.GivenTheDownStreamRouteIs(downstreamRoute)) + .And(x => GivenTheServiceProviderConfigIs(config)) + .And(x => x.GivenTheDownstreamRequestUriIs("http://localhost:19081?PartitionKind=test&PartitionKey=1")) + .And(x => x.GivenTheUrlReplacerWillReturnSequence("/products", "Service_1.0/Api")) + .When(x => x.WhenICallTheMiddleware()) + .Then(x => x.ThenTheDownstreamRequestUriIs("http://localhost:19081/Service_1.0/Api/products?PartitionKind=test&PartitionKey=1")) + .BDDfy(); + } + [Fact] public void issue_473_should_not_remove_additional_query_string() { @@ -322,6 +350,8 @@ public void issue_473_should_not_remove_additional_query_string() .BDDfy(); } + + private void GivenTheServiceProviderConfigIs(ServiceProviderConfiguration config) { var configuration = new InternalConfiguration(null, null, config, null, null, null, null, null); @@ -346,11 +376,21 @@ private void GivenTheDownstreamRequestUriIs(string uri) _downstreamContext.DownstreamRequest = new DownstreamRequest(_request); } + private void GivenTheUrlReplacerWillReturnSequence(params string[] paths) + { + var setup = _downstreamUrlTemplateVariableReplacer + .SetupSequence(x => x.Replace(It.IsAny(), It.IsAny>())); + foreach (var path in paths) + { + var response = new OkResponse(new DownstreamPath(path)); + setup.Returns(response); + } + } private void GivenTheUrlReplacerWillReturn(string path) { _downstreamPath = new OkResponse(new DownstreamPath(path)); _downstreamUrlTemplateVariableReplacer - .Setup(x => x.Replace(It.IsAny(), It.IsAny>())) + .Setup(x => x.Replace(It.IsAny(), It.IsAny>())) .Returns(_downstreamPath); } diff --git a/test/Ocelot.UnitTests/DownstreamUrlCreator/UrlTemplateReplacer/UpstreamUrlPathTemplateVariableReplacerTests.cs b/test/Ocelot.UnitTests/DownstreamUrlCreator/UrlTemplateReplacer/UpstreamUrlPathTemplateVariableReplacerTests.cs index cd8cf3c81..a82f2c534 100644 --- a/test/Ocelot.UnitTests/DownstreamUrlCreator/UrlTemplateReplacer/UpstreamUrlPathTemplateVariableReplacerTests.cs +++ b/test/Ocelot.UnitTests/DownstreamUrlCreator/UrlTemplateReplacer/UpstreamUrlPathTemplateVariableReplacerTests.cs @@ -199,7 +199,7 @@ private void GivenThereIsAUrlMatch(DownstreamRoute downstreamRoute) private void WhenIReplaceTheTemplateVariables() { - _result = _downstreamPathReplacer.Replace(_downstreamRoute.ReRoute.DownstreamReRoute[0].DownstreamPathTemplate, _downstreamRoute.TemplatePlaceholderNameAndValues); + _result = _downstreamPathReplacer.Replace(_downstreamRoute.ReRoute.DownstreamReRoute[0].DownstreamPathTemplate.Value, _downstreamRoute.TemplatePlaceholderNameAndValues); } private void ThenTheDownstreamUrlPathIsReturned(string expected)