Fixes rewrite path example and discovery (#2144)

Given that we would like to make the following rewriting (in both discovery and declarative context)

- '/foo'  to '/'
- '/foo/' to '/'
- '/foo/anything/else' to '/anything/else'

We need to

- force the presence of a leading '/' after rewriting as we may result in an empty uri path otherwise
- exclude it from 'remaining' group

See #1282
See #1359
See #1360

docs/src/main/asciidoc/spring-cloud-gateway.adoc

@@ -1132,7 +1132,7 @@ spring:
         predicates:
         - Path=/red/**
         filters:
-        - RewritePath=/red(?<segment>/?.*), $\{segment}
+        - RewritePath=/red/?(?<segment>.*), /$\{segment}
 ----
 ====
 
@@ -2226,7 +2226,7 @@ By default, the gateway defines a single predicate and filter for routes created
 The default predicate is a path predicate defined with the pattern `/serviceId/**`, where `serviceId` is
 the ID of the service from the `DiscoveryClient`.
 
-The default filter is a rewrite path filter with the regex `/serviceId/(?<remaining>.*)` and the replacement `/${remaining}`.
+The default filter is a rewrite path filter with the regex `/serviceId/?(?<remaining>.*)` and the replacement `/${remaining}`.
 This strips the service ID from the path before the request is sent downstream.
 
 If you want to customize the predicates or filters used by the `DiscoveryClient` routes, set `spring.cloud.gateway.discovery.locator.predicates[x]` and `spring.cloud.gateway.discovery.locator.filters[y]`.
@@ -2244,7 +2244,7 @@ spring.cloud.gateway.discovery.locator.predicates[1].args[pattern]: "'**.foo.com
 spring.cloud.gateway.discovery.locator.filters[0].name: CircuitBreaker
 spring.cloud.gateway.discovery.locator.filters[0].args[name]: serviceId
 spring.cloud.gateway.discovery.locator.filters[1].name: RewritePath
-spring.cloud.gateway.discovery.locator.filters[1].args[regexp]: "'/' + serviceId + '/(?<remaining>.*)'"
+spring.cloud.gateway.discovery.locator.filters[1].args[regexp]: "'/' + serviceId + '/?(?<remaining>.*)'"
 spring.cloud.gateway.discovery.locator.filters[1].args[replacement]: "'/${remaining}'"
 ----
 ====

spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/discovery/GatewayDiscoveryClientAutoConfiguration.java

@@ -70,7 +70,7 @@ public class GatewayDiscoveryClientAutoConfiguration {
 		// add a filter that removes /serviceId by default
 		FilterDefinition filter = new FilterDefinition();
 		filter.setName(normalizeFilterFactoryName(RewritePathGatewayFilterFactory.class));
-		String regex = "'/' + serviceId + '/(?<remaining>.*)'";
+		String regex = "'/' + serviceId + '/?(?<remaining>.*)'";
 		String replacement = "'/${remaining}'";
 		filter.addArg(REGEXP_KEY, regex);
 		filter.addArg(REPLACEMENT_KEY, replacement);

spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/discovery/DiscoveryClientRouteDefinitionLocatorTests.java

@@ -83,7 +83,7 @@ public class DiscoveryClientRouteDefinitionLocatorTests {
 		assertThat(definition.getFilters()).hasSize(1);
 		FilterDefinition filter = definition.getFilters().get(0);
 		assertThat(filter.getName()).isEqualTo("RewritePath");
-		assertThat(filter.getArgs()).hasSize(2).containsEntry(REGEXP_KEY, "/service1/(?<remaining>.*)")
+		assertThat(filter.getArgs()).hasSize(2).containsEntry(REGEXP_KEY, "/service1/?(?<remaining>.*)")
 				.containsEntry(REPLACEMENT_KEY, "/${remaining}");
 	}
 

spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/discovery/GatewayDiscoveryClientAutoConfigurationTest.java

@@ -0,0 +1,93 @@
+/*
+ * Copyright 2013-2020 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.cloud.gateway.discovery;
+
+import java.util.Objects;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import org.springframework.cloud.client.ServiceInstance;
+import org.springframework.cloud.gateway.filter.FilterDefinition;
+import org.springframework.expression.spel.standard.SpelExpressionParser;
+import org.springframework.expression.spel.support.SimpleEvaluationContext;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+import static org.springframework.cloud.gateway.filter.factory.RewritePathGatewayFilterFactory.REGEXP_KEY;
+import static org.springframework.cloud.gateway.filter.factory.RewritePathGatewayFilterFactory.REPLACEMENT_KEY;
+
+public class GatewayDiscoveryClientAutoConfigurationTest {
+
+	private static final String SERVICE_ID = "foo";
+
+	private static final String BASE_URI = "/" + SERVICE_ID;
+
+	private static final SpelExpressionParser PARSER = new SpelExpressionParser();
+
+	private static final SimpleEvaluationContext CONTEXT = SimpleEvaluationContext.forReadOnlyDataBinding().withInstanceMethods().build();
+
+	private static final FilterDefinition DEFAULT_FILTER = GatewayDiscoveryClientAutoConfiguration.initFilters().get(0);
+
+	private ServiceInstance serviceInstance;
+	
+	@BeforeEach
+	public void buildServiceInstance(){
+		this.serviceInstance = mock(ServiceInstance.class);
+		when(this.serviceInstance.getServiceId()).thenReturn(SERVICE_ID);
+	}
+
+	@Test
+	public void defaultRewritePathShouldHandleEmptyRemainingWithoutSlash(){
+		String expectedRemotePath = "/";
+
+		final String result = replace(BASE_URI);
+
+		assertThat(result).isEqualTo(expectedRemotePath);
+	}
+
+	@Test
+	public void defaultRewritePathShouldHandleEmptyRemainingWithSlash(){
+		String extraUri = "/";
+
+		final String result = replace(BASE_URI + extraUri);
+
+		assertThat(result).isEqualTo(extraUri);
+	}
+
+
+	@Test
+	public void defaultRewritePathShouldHandleNonEmptyRemainingPath(){
+		String extraUri = "/some/additional/uri";
+
+		final String result = replace(BASE_URI + extraUri);
+
+		assertThat(result).isEqualTo(extraUri);
+	}
+
+	private String replace(String enteringPath) {
+		return enteringPath.replaceAll(evaluateExpression(DEFAULT_FILTER.getArgs().get(REGEXP_KEY)),
+				evaluateExpression(DEFAULT_FILTER.getArgs().get(REPLACEMENT_KEY)));
+	}
+
+	private String evaluateExpression(String expression) {
+		return Objects.requireNonNull(PARSER.parseExpression(expression)
+				.getValue(CONTEXT, serviceInstance, String.class));
+	}
+
+}