diff --git a/spring-web/src/main/java/org/springframework/web/bind/annotation/GetMapping.java b/spring-web/src/main/java/org/springframework/web/bind/annotation/GetMapping.java index bceb66e419f..c9fc39bda34 100644 --- a/spring-web/src/main/java/org/springframework/web/bind/annotation/GetMapping.java +++ b/spring-web/src/main/java/org/springframework/web/bind/annotation/GetMapping.java @@ -31,7 +31,6 @@ import org.springframework.core.annotation.AliasFor; *

Specifically, {@code @GetMapping} is a composed annotation that * acts as a shortcut for {@code @RequestMapping(method = RequestMethod.GET)}. * - * * @author Sam Brannen * @since 4.3 * @see PostMapping diff --git a/spring-web/src/main/java/org/springframework/web/bind/annotation/RequestMapping.java b/spring-web/src/main/java/org/springframework/web/bind/annotation/RequestMapping.java index 52cf8ea526d..362373f4c5f 100644 --- a/spring-web/src/main/java/org/springframework/web/bind/annotation/RequestMapping.java +++ b/spring-web/src/main/java/org/springframework/web/bind/annotation/RequestMapping.java @@ -88,26 +88,33 @@ public @interface RequestMapping { /** * The primary mapping expressed by this annotation. - *

This is an alias for {@link #path}. For example + *

This is an alias for {@link #path}. For example, * {@code @RequestMapping("/foo")} is equivalent to * {@code @RequestMapping(path="/foo")}. *

Supported at the type level as well as at the method level! * When used at the type level, all method-level mappings inherit * this primary mapping, narrowing it for a specific handler method. + *

NOTE: Each handler method must be mapped to a + * non-empty path, either at the type level, at the method level, or a + * combination of the two. If you wish to map to all paths, please map + * explicitly to {@code "/**"} or {@code "**"}. */ @AliasFor("path") String[] value() default {}; /** - * The path mapping URIs (e.g. "/myPath.do"). - * Ant-style path patterns are also supported (e.g. "/myPath/*.do"). - * At the method level, relative paths (e.g. "edit.do") are supported + * The path mapping URIs (e.g. {@code "/myPath.do"}). + *

Ant-style path patterns are also supported (e.g. {@code "/myPath/*.do"}). + * At the method level, relative paths (e.g. {@code "edit.do"}) are supported * within the primary mapping expressed at the type level. - * Path mapping URIs may contain placeholders (e.g. "/${connect}"). + * Path mapping URIs may contain placeholders (e.g. "/${connect}"). *

Supported at the type level as well as at the method level! * When used at the type level, all method-level mappings inherit * this primary mapping, narrowing it for a specific handler method. - * @see org.springframework.web.bind.annotation.ValueConstants#DEFAULT_NONE + *

NOTE: Each handler method must be mapped to a + * non-empty path, either at the type level, at the method level, or a + * combination of the two. If you wish to map to all paths, please map + * explicitly to {@code "/**"} or {@code "**"}. * @since 4.2 */ @AliasFor("value") diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerMethodMapping.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerMethodMapping.java index f34f457c7a7..11ebf1fd894 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerMethodMapping.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerMethodMapping.java @@ -43,6 +43,7 @@ import org.springframework.util.Assert; import org.springframework.util.ClassUtils; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; +import org.springframework.util.StringUtils; import org.springframework.web.cors.CorsConfiguration; import org.springframework.web.cors.CorsUtils; import org.springframework.web.method.HandlerMethod; @@ -58,6 +59,7 @@ import org.springframework.web.servlet.HandlerMapping; * @author Arjen Poutsma * @author Rossen Stoyanchev * @author Juergen Hoeller + * @author Sam Brannen * @since 3.1 * @param the mapping for a {@link HandlerMethod} containing the conditions * needed to match the handler method to incoming request. @@ -587,6 +589,7 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap this.readWriteLock.writeLock().lock(); try { HandlerMethod handlerMethod = createHandlerMethod(handler, method); + assertMappedPathMethodMapping(handlerMethod, mapping); assertUniqueMethodMapping(handlerMethod, mapping); this.mappingLookup.put(mapping, handlerMethod); @@ -613,6 +616,21 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap } } + /** + * Assert that the supplied {@code mapping} maps the supplied {@link HandlerMethod} + * to explicit, non-empty paths. + * @since 5.2 + * @see StringUtils#hasText(String) + */ + private void assertMappedPathMethodMapping(HandlerMethod handlerMethod, T mapping) { + if (!getMappingPathPatterns(mapping).stream().allMatch(StringUtils::hasText)) { + throw new IllegalStateException(String.format("Missing path mapping. " + + "Handler method '%s' in bean '%s' must be mapped to a non-empty path. " + + "If you wish to map to all paths, please map explicitly to \"/**\" or \"**\".", + handlerMethod, handlerMethod.getBean())); + } + } + private void assertUniqueMethodMapping(HandlerMethod newHandlerMethod, T mapping) { HandlerMethod handlerMethod = this.mappingLookup.get(mapping); if (handlerMethod != null && !handlerMethod.equals(newHandlerMethod)) { diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMappingTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMappingTests.java index 1c90590e6a5..9f428c03ec2 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMappingTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMappingTests.java @@ -78,7 +78,7 @@ public class RequestMappingInfoHandlerMappingTests { private HandlerMethod barMethod; - private HandlerMethod emptyMethod; + private HandlerMethod rootMethod; @Before @@ -88,7 +88,7 @@ public class RequestMappingInfoHandlerMappingTests { this.fooMethod = new HandlerMethod(testController, "foo"); this.fooParamMethod = new HandlerMethod(testController, "fooParam"); this.barMethod = new HandlerMethod(testController, "bar"); - this.emptyMethod = new HandlerMethod(testController, "empty"); + this.rootMethod = new HandlerMethod(testController, "root"); this.handlerMapping = new TestRequestMappingInfoHandlerMapping(); this.handlerMapping.registerHandler(testController); @@ -125,12 +125,12 @@ public class RequestMappingInfoHandlerMappingTests { MockHttpServletRequest request = new MockHttpServletRequest("GET", ""); HandlerMethod handlerMethod = getHandler(request); - assertEquals(this.emptyMethod.getMethod(), handlerMethod.getMethod()); + assertEquals(this.rootMethod.getMethod(), handlerMethod.getMethod()); request = new MockHttpServletRequest("GET", "/"); handlerMethod = getHandler(request); - assertEquals(this.emptyMethod.getMethod(), handlerMethod.getMethod()); + assertEquals(this.rootMethod.getMethod(), handlerMethod.getMethod()); } @Test @@ -465,8 +465,8 @@ public class RequestMappingInfoHandlerMappingTests { public void bar() { } - @RequestMapping(value = "") - public void empty() { + @RequestMapping("/") + public void root() { } @RequestMapping(value = "/person/{id}", method = RequestMethod.PUT, consumes="application/xml") diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMappingTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMappingTests.java index 67d056d8200..e88640cfffb 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMappingTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMappingTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 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. @@ -228,7 +228,7 @@ public class RequestMappingHandlerMappingTests { @RequestMapping(consumes = MediaType.APPLICATION_JSON_VALUE) static class ComposedAnnotationController { - @RequestMapping + @RequestMapping("/**") public void handle() { } @@ -236,7 +236,7 @@ public class RequestMappingHandlerMappingTests { public void postJson() { } - @GetMapping(value = "/get", consumes = MediaType.ALL_VALUE) + @GetMapping(path = "/get", consumes = MediaType.ALL_VALUE) public void get() { } @@ -266,7 +266,7 @@ public class RequestMappingHandlerMappingTests { @Retention(RetentionPolicy.RUNTIME) @interface PostJson { - @AliasFor(annotation = RequestMapping.class, attribute = "path") + @AliasFor(annotation = RequestMapping.class) String[] value() default {}; } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodTests.java index 4094e31de73..4cc37b9d188 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 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. @@ -67,10 +67,10 @@ import org.springframework.beans.factory.BeanCreationException; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; import org.springframework.beans.factory.config.BeanDefinition; -import org.springframework.beans.factory.config.PropertyPlaceholderConfigurer; import org.springframework.beans.factory.support.RootBeanDefinition; import org.springframework.beans.propertyeditors.CustomDateEditor; import org.springframework.context.annotation.AnnotationConfigUtils; +import org.springframework.context.support.PropertySourcesPlaceholderConfigurer; import org.springframework.core.MethodParameter; import org.springframework.core.convert.converter.Converter; import org.springframework.format.annotation.DateTimeFormat; @@ -151,11 +151,13 @@ import org.springframework.web.servlet.support.RequestContextUtils; import org.springframework.web.servlet.view.AbstractView; import org.springframework.web.servlet.view.InternalResourceViewResolver; +import static org.assertj.core.api.Assertions.*; import static org.junit.Assert.*; /** * @author Rossen Stoyanchev * @author Juergen Hoeller + * @author Sam Brannen */ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServletHandlerMethodTests { @@ -251,7 +253,7 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl @Test public void defaultExpressionParameters() throws Exception { initServlet(wac -> { - RootBeanDefinition ppc = new RootBeanDefinition(PropertyPlaceholderConfigurer.class); + RootBeanDefinition ppc = new RootBeanDefinition(PropertySourcesPlaceholderConfigurer.class); ppc.getPropertyValues().add("properties", "myKey=foo"); wac.registerBeanDefinition("ppc", ppc); }, DefaultExpressionValueParamController.class); @@ -787,15 +789,19 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl } @Test - public void equivalentMappingsWithSameMethodName() throws Exception { - try { - initServletWithControllers(ChildController.class); - fail("Expected 'method already mapped' error"); - } - catch (BeanCreationException e) { - assertTrue(e.getCause() instanceof IllegalStateException); - assertTrue(e.getCause().getMessage().contains("Ambiguous mapping")); - } + public void equivalentMappingsWithSameMethodName() { + assertThatThrownBy(() -> initServletWithControllers(ChildController.class)) + .isInstanceOf(BeanCreationException.class) + .hasCauseInstanceOf(IllegalStateException.class) + .hasMessageContaining("Ambiguous mapping"); + } + + @Test + public void unmappedPathMapping() { + assertThatThrownBy(() -> initServletWithControllers(UnmappedPathController.class)) + .isInstanceOf(BeanCreationException.class) + .hasCauseInstanceOf(IllegalStateException.class) + .hasMessageContaining("Missing path mapping"); } @Test @@ -1993,7 +1999,7 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl @Controller static class ControllerWithEmptyValueMapping { - @RequestMapping("") + @RequestMapping("/**") public void myPath2(HttpServletResponse response) throws IOException { throw new IllegalStateException("test"); } @@ -2012,7 +2018,7 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl @Controller private static class ControllerWithErrorThrown { - @RequestMapping("") + @RequestMapping("/**") public void myPath2(HttpServletResponse response) throws IOException { throw new AssertionError("test"); } @@ -2726,6 +2732,15 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl } } + @Controller + @RequestMapping // path intentionally omitted + static class UnmappedPathController { + + @GetMapping // path intentionally omitted + public void get(@RequestParam(required = false) String id) { + } + } + @Target({ElementType.TYPE}) @Retention(RetentionPolicy.RUNTIME) @Controller @@ -3586,7 +3601,7 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl @Controller static class HttpHeadersResponseController { - @RequestMapping(value = "", method = RequestMethod.POST) + @RequestMapping(value = "/*", method = RequestMethod.POST) @ResponseStatus(HttpStatus.CREATED) public HttpHeaders create() throws URISyntaxException { HttpHeaders headers = new HttpHeaders();