Support placeholder in service fabric services names (#722)

src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs

@@ -27,7 +27,7 @@ namespace Ocelot.DownstreamUrlCreator.Middleware
         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 @@ namespace Ocelot.DownstreamUrlCreator.Middleware
             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 @@ namespace Ocelot.DownstreamUrlCreator.Middleware
         private (string path, string query) CreateServiceFabricUri(DownstreamContext context, Response<DownstreamPath> dsPath)
         {
             var query = context.DownstreamRequest.Query;           
-            var serviceFabricPath = $"/{context.DownstreamReRoute.ServiceName + dsPath.Data.Value}";
+            var serviceName = _replacer.Replace(context.DownstreamReRoute.ServiceName, context.TemplatePlaceholderNameAndValues);
-            return (serviceFabricPath, query);
+            var pathTemplate = $"/{serviceName.Data.Value}{dsPath.Data.Value}";
+            return (pathTemplate, query);
         }
 
         private static bool ServiceFabricRequest(DownstreamContext context)

src/Ocelot/DownstreamUrlCreator/UrlTemplateReplacer/DownstreamTemplatePathPlaceholderReplacer.cs

@@ -8,11 +8,12 @@ namespace Ocelot.DownstreamUrlCreator.UrlTemplateReplacer
 {
     public class DownstreamTemplatePathPlaceholderReplacer : IDownstreamPathPlaceholderReplacer
     {
-        public Response<DownstreamPath> Replace(DownstreamPathTemplate downstreamPathTemplate, List<PlaceholderNameAndValue> urlPathPlaceholderNameAndValues)
+        public Response<DownstreamPath> Replace(string downstreamPathTemplate,
+            List<PlaceholderNameAndValue> urlPathPlaceholderNameAndValues)
         {
             var downstreamPath = new StringBuilder();
 
-            downstreamPath.Append(downstreamPathTemplate.Value);
+            downstreamPath.Append(downstreamPathTemplate);
 
             foreach (var placeholderVariableAndValue in urlPathPlaceholderNameAndValues)
             {

src/Ocelot/DownstreamUrlCreator/UrlTemplateReplacer/IDownstreamPathPlaceholderReplacer.cs

@@ -7,6 +7,6 @@ namespace Ocelot.DownstreamUrlCreator.UrlTemplateReplacer
 {
     public interface IDownstreamPathPlaceholderReplacer
     {
-        Response<DownstreamPath> Replace(DownstreamPathTemplate downstreamPathTemplate, List<PlaceholderNameAndValue> urlPathPlaceholderNameAndValues);   
+        Response<DownstreamPath> Replace(string downstreamPathTemplate, List<PlaceholderNameAndValue> urlPathPlaceholderNameAndValues);   
     }
 }

test/Ocelot.AcceptanceTests/ServiceFabricTests.cs

@@ -128,6 +128,42 @@ namespace Ocelot.AcceptanceTests
                 .BDDfy();
         }
 
+        [Fact]
+        public void should_support_placeholder_in_service_fabric_service_name()
+        {
+            var configuration = new FileConfiguration
+            {
+                ReRoutes = new List<FileReRoute>
+                    {
+                        new FileReRoute
+                        {
+                            DownstreamPathTemplate = "/values",
+                            DownstreamScheme = "http",
+                            UpstreamPathTemplate = "/api/{version}/values",
+                            UpstreamHttpMethod = new List<string> { "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 =>

test/Ocelot.UnitTests/DownstreamUrlCreator/DownstreamUrlCreatorMiddlewareTests.cs

@@ -223,7 +223,7 @@
             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 @@
             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 @@
             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<PlaceholderNameAndValue>(),
+                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 @@
                 .BDDfy();
         }
 
+
+
         private void GivenTheServiceProviderConfigIs(ServiceProviderConfiguration config)
         {
             var configuration = new InternalConfiguration(null, null, config, null, null, null, null, null);
@@ -346,11 +376,21 @@
             _downstreamContext.DownstreamRequest = new DownstreamRequest(_request);
         }
 
+        private void GivenTheUrlReplacerWillReturnSequence(params string[] paths)
+        {
+            var setup = _downstreamUrlTemplateVariableReplacer
+                .SetupSequence(x => x.Replace(It.IsAny<string>(), It.IsAny<List<PlaceholderNameAndValue>>()));
+            foreach (var path in paths)
+            {
+                var response = new OkResponse<DownstreamPath>(new DownstreamPath(path));
+                setup.Returns(response);
+            }
+        }
         private void GivenTheUrlReplacerWillReturn(string path)
         {
             _downstreamPath = new OkResponse<DownstreamPath>(new DownstreamPath(path));
             _downstreamUrlTemplateVariableReplacer
-                .Setup(x => x.Replace(It.IsAny<DownstreamPathTemplate>(), It.IsAny<List<PlaceholderNameAndValue>>()))
+                .Setup(x => x.Replace(It.IsAny<string>(), It.IsAny<List<PlaceholderNameAndValue>>()))
                 .Returns(_downstreamPath);
         }
 

test/Ocelot.UnitTests/DownstreamUrlCreator/UrlTemplateReplacer/UpstreamUrlPathTemplateVariableReplacerTests.cs

@@ -199,7 +199,7 @@ namespace Ocelot.UnitTests.DownstreamUrlCreator.UrlTemplateReplacer
 
         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)