Fix double @RequestMapping with empty value (#245)

* Fix double @RequestMapping with empty value

* Fix code style

* Fix for review

* add author to SpringMvcContract

* split SpringMvcContractTests#testDoubleRequestMapping() test
This commit is contained in:
nekkiy
2020-03-27 13:58:26 +03:00
committed by GitHub
parent a77bd2bdf1
commit 0afb60a72a
3 changed files with 100 additions and 10 deletions

View File

@@ -177,6 +177,12 @@
<artifactId>spring-cloud-loadbalancer</artifactId>
<optional>true</optional>
</dependency>
<dependency>
<groupId>pl.pragmatists</groupId>
<artifactId>JUnitParams</artifactId>
<version>1.1.1</version>
<scope>test</scope>
</dependency>
</dependencies>
<profiles>
<profile>

View File

@@ -71,6 +71,7 @@ import static org.springframework.core.annotation.AnnotatedElementUtils.findMerg
* @author Aram Peres
* @author Olga Maciaszek-Sharma
* @author Aaron Whiteside
* @author Artyom Romanenko
*/
public class SpringMvcContract extends Contract.BaseContract
implements ResourceLoaderAware {
@@ -167,15 +168,14 @@ public class SpringMvcContract extends Contract.BaseContract
if (clz.getInterfaces().length == 0) {
RequestMapping classAnnotation = findMergedAnnotation(clz,
RequestMapping.class);
if (classAnnotation != null) {
if (classAnnotation != null && classAnnotation.value().length > 0) {
// Prepend path from class annotation if specified
if (classAnnotation.value().length > 0) {
String pathValue = emptyToNull(classAnnotation.value()[0]);
String pathValue = emptyToNull(classAnnotation.value()[0]);
if (pathValue != null) {
pathValue = resolve(pathValue);
if (!pathValue.startsWith("/")) {
pathValue = "/" + pathValue;
if (!pathValue.equals("/")) {
data.template().uri(pathValue);
}
data.template().uri(pathValue);
}
}
}
@@ -229,11 +229,9 @@ public class SpringMvcContract extends Contract.BaseContract
String pathValue = emptyToNull(methodMapping.value()[0]);
if (pathValue != null) {
pathValue = resolve(pathValue);
// Append path from @RequestMapping if value is present on method
if (!pathValue.startsWith("/") && !data.template().path().endsWith("/")) {
pathValue = "/" + pathValue;
if (!pathValue.equals("/")) {
data.template().uri(pathValue, true);
}
data.template().uri(pathValue, true);
}
}

View File

@@ -31,8 +31,11 @@ import java.util.Map;
import com.fasterxml.jackson.annotation.JsonAutoDetect;
import feign.MethodMetadata;
import feign.Param;
import junitparams.JUnitParamsRunner;
import junitparams.Parameters;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.springframework.cloud.openfeign.SpringQueryMap;
import org.springframework.core.convert.ConversionService;
@@ -67,7 +70,9 @@ import static org.junit.Assume.assumeTrue;
* @author Halvdan Hoem Grelland
* @author Aram Peres
* @author Aaron Whiteside
* @author Artyom Romanenko
*/
@RunWith(JUnitParamsRunner.class)
public class SpringMvcContractTests {
private static final Class<?> EXECUTABLE_TYPE;
@@ -582,6 +587,53 @@ public class SpringMvcContractTests {
"{Accept}");
}
private Class[] doubleMappingClassesProvider() {
return new Class[] { TestTemplate_RequestMapping_Empty_Class.class,
TestTemplate_RequestMapping_Empty_Method.class };
}
@Test
@Parameters(method = "doubleMappingClassesProvider")
public void testDoubleRequestMapping_root(Class clazz) throws NoSuchMethodException {
Method method = clazz.getDeclaredMethod("root");
MethodMetadata data = contract
.parseAndValidateMetadata(method.getDeclaringClass(), method);
assertThat(data.template().url()).isEqualTo("/");
}
@Test
@Parameters(method = "doubleMappingClassesProvider")
public void testDoubleRequestMapping_rootReverse(Class clazz)
throws NoSuchMethodException {
Method method = clazz.getDeclaredMethod("rootReverse");
MethodMetadata data = contract
.parseAndValidateMetadata(method.getDeclaringClass(), method);
assertThat(data.template().url()).isEqualTo("/");
}
@Test
@Parameters(method = "doubleMappingClassesProvider")
public void testDoubleRequestMapping_sub(Class clazz) throws NoSuchMethodException {
Method method = clazz.getDeclaredMethod("sub");
MethodMetadata data = contract
.parseAndValidateMetadata(method.getDeclaringClass(), method);
assertThat(data.template().url()).isEqualTo("/sub");
}
@Test
@Parameters(method = "doubleMappingClassesProvider")
public void testDoubleRequestMapping_subEmpty(Class clazz)
throws NoSuchMethodException {
Method method = clazz.getDeclaredMethod("subEmpty");
MethodMetadata data = contract
.parseAndValidateMetadata(method.getDeclaringClass(), method);
assertThat(data.template().url()).isEqualTo("/subEmpty");
}
@Test
public void testMultipleRequestPartAnnotations() throws NoSuchMethodException {
Method method = TestTemplate_RequestPart.class.getDeclaredMethod(
@@ -770,6 +822,40 @@ public class SpringMvcContractTests {
}
@RequestMapping("")
public interface TestTemplate_RequestMapping_Empty_Class {
@RequestMapping("/")
String root();
@RequestMapping("")
String rootReverse();
@RequestMapping("/sub")
String sub();
@RequestMapping("subEmpty")
String subEmpty();
}
@RequestMapping("/")
public interface TestTemplate_RequestMapping_Empty_Method {
@RequestMapping("")
String root();
@RequestMapping("/")
String rootReverse();
@RequestMapping("/sub")
String sub();
@RequestMapping("subEmpty")
String subEmpty();
}
@JsonAutoDetect(fieldVisibility = ANY, getterVisibility = NONE,
setterVisibility = NONE)
public class TestObject {