diff --git a/spring-data-rest-tests/spring-data-rest-tests-jpa/src/test/java/org/springframework/data/rest/webmvc/jpa/JpaRepositoryConfig.java b/spring-data-rest-tests/spring-data-rest-tests-jpa/src/test/java/org/springframework/data/rest/webmvc/jpa/JpaRepositoryConfig.java index 7f40041de..35fa4663e 100644 --- a/spring-data-rest-tests/spring-data-rest-tests-jpa/src/test/java/org/springframework/data/rest/webmvc/jpa/JpaRepositoryConfig.java +++ b/spring-data-rest-tests/spring-data-rest-tests-jpa/src/test/java/org/springframework/data/rest/webmvc/jpa/JpaRepositoryConfig.java @@ -59,10 +59,9 @@ public class JpaRepositoryConfig extends JpaInfrastructureConfig { } @RepositoryRestController - @RequestMapping("orders") static class OrdersJsonController { - @RequestMapping(value = "/search/sort", method = RequestMethod.POST, produces = "application/hal+json") + @RequestMapping(value = "/orders/search/sort", method = RequestMethod.POST, produces = "application/hal+json") void someMethodWithArgs(Sort sort, Pageable pageable, DefaultedPageable defaultedPageable) {} } } diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/BasePathAwareHandlerMapping.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/BasePathAwareHandlerMapping.java index 2a8046578..0a5005791 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/BasePathAwareHandlerMapping.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/BasePathAwareHandlerMapping.java @@ -28,12 +28,14 @@ import java.util.function.Predicate; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletRequestWrapper; +import org.springframework.core.annotation.AnnotatedElementUtils; import org.springframework.data.rest.core.config.RepositoryRestConfiguration; import org.springframework.data.util.ProxyUtils; import org.springframework.http.HttpHeaders; import org.springframework.http.MediaType; import org.springframework.util.Assert; import org.springframework.util.StringUtils; +import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.method.HandlerMethod; import org.springframework.web.servlet.mvc.condition.ProducesRequestCondition; import org.springframework.web.servlet.mvc.method.RequestMappingInfo; @@ -46,6 +48,7 @@ import org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandl */ public class BasePathAwareHandlerMapping extends RequestMappingHandlerMapping { + private static final String AT_REQUEST_MAPPING_ON_TYPE = "Spring Data REST controller %s must not use @RequestMapping on class level as this would cause double registration with Spring MVC!"; private final RepositoryRestConfiguration configuration; /** @@ -140,15 +143,38 @@ public class BasePathAwareHandlerMapping extends RequestMappingHandlerMapping { return condition; } - /* - * (non-Javadoc) + /** + * Returns whether the given type is considered a handler. Performs additional configuration checks. If you only want + * to customize the handler selection criteria, override {@link #isHandlerInternal(Class)}. Will be made final in 4.0. + * * @see org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerMapping#isHandler(java.lang.Class) + * @deprecated for overriding in 3.6. Will be made final in 4.0. */ @Override + @Deprecated protected boolean isHandler(Class beanType) { Class type = ProxyUtils.getUserClass(beanType); + boolean isSpringDataRestHandler = isHandlerInternal(type); + if (!isSpringDataRestHandler) { + return false; + } + + if (AnnotatedElementUtils.hasAnnotation(type, RequestMapping.class)) { + throw new IllegalStateException(String.format(AT_REQUEST_MAPPING_ON_TYPE, beanType.getName())); + } + + return isSpringDataRestHandler; + } + + /** + * Returns whether the given controller type is considered a handler. + * + * @param type will never be {@literal null}. + * @return + */ + protected boolean isHandlerInternal(Class type) { return type.isAnnotationPresent(BasePathAwareController.class); } diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryRestHandlerMapping.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryRestHandlerMapping.java index c899bb5ce..83d989f93 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryRestHandlerMapping.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryRestHandlerMapping.java @@ -25,14 +25,12 @@ import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import org.springframework.core.annotation.AnnotatedElementUtils; -import org.springframework.core.annotation.AnnotationUtils; import org.springframework.data.repository.support.Repositories; import org.springframework.data.rest.core.config.RepositoryRestConfiguration; import org.springframework.data.rest.core.mapping.HttpMethods; import org.springframework.data.rest.core.mapping.ResourceMappings; import org.springframework.data.rest.core.mapping.ResourceMetadata; import org.springframework.data.rest.webmvc.support.JpaHelper; -import org.springframework.data.util.ProxyUtils; import org.springframework.data.util.Streamable; import org.springframework.hateoas.MediaTypes; import org.springframework.http.HttpMethod; @@ -182,14 +180,11 @@ public class RepositoryRestHandlerMapping extends BasePathAwareHandlerMapping { /* * (non-Javadoc) - * @see org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerMapping#isHandler(java.lang.Class) + * @see org.springframework.data.rest.webmvc.BasePathAwareHandlerMapping#isHandlerInternal(java.lang.Class) */ @Override - protected boolean isHandler(Class beanType) { - - Class type = ProxyUtils.getUserClass(beanType); - - return AnnotationUtils.findAnnotation(type, RepositoryRestController.class) != null; + protected boolean isHandlerInternal(Class type) { + return AnnotatedElementUtils.hasAnnotation(type, RepositoryRestController.class); } /* diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/BasePathAwareHandlerMappingUnitTests.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/BasePathAwareHandlerMappingUnitTests.java index c392668ef..6853c4c4a 100644 --- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/BasePathAwareHandlerMappingUnitTests.java +++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/BasePathAwareHandlerMappingUnitTests.java @@ -25,6 +25,8 @@ import org.junit.Test; import org.springframework.aop.framework.ProxyFactory; import org.springframework.aop.support.AopUtils; import org.springframework.data.rest.core.config.RepositoryRestConfiguration; +import org.springframework.stereotype.Controller; +import org.springframework.web.bind.annotation.RequestMapping; /** * Unit tests for {@link BasePathAwareHandlerMapping}. @@ -60,6 +62,23 @@ public class BasePathAwareHandlerMappingUnitTests { assertThat(mapping.isHandler(type)).isFalse(); } + @Test // #1342, #1628, #1686, #1946 + public void rejectsAtRequestMappingOnCustomController() { + + assertThatIllegalStateException() + .isThrownBy(() -> { + mapping.isHandler(InvalidController.class); + }) + .withMessageContaining(InvalidController.class.getName()); + } + + @Test // #1342, #1628, #1686, #1946 + public void doesNotRejectAtRequestMappingOnStandardMvcController() { + + assertThatNoException() + .isThrownBy(() -> mapping.isHandler(ValidController.class)); + } + private static Class createProxy(Object source) { ProxyFactory factory = new ProxyFactory(source); @@ -87,4 +106,12 @@ public class BasePathAwareHandlerMappingUnitTests { return super.isHandler(beanType); } } + + @BasePathAwareController + @RequestMapping("/sample") + static class InvalidController {} + + @Controller + @RequestMapping("/sample") + static class ValidController {} }