From 07ebc501cd1e4753259a7d26c91b93b18f2eb74d Mon Sep 17 00:00:00 2001 From: Oliver Drotbohm Date: Wed, 6 Jul 2022 16:15:20 +0200 Subject: [PATCH] Polishing. Added test case and re-enabled the base URI to be prepended even in case a controller path prefix is configured. That functionality had been lost with the originally submitted changes. Related ticket: #2157 Original pull request: #2088. --- .../rest/webmvc/jpa/JpaRepositoryConfig.java | 6 ++- .../data/rest/webmvc/jpa/JpaWebTests.java | 6 ++- .../rest/webmvc/BasePathAwareController.java | 24 ++++++++-- .../webmvc/BasePathAwareHandlerMapping.java | 46 +++++++++++-------- .../rest/webmvc/RepositoryRestController.java | 24 ++++++++-- .../BasePathAwareHandlerMappingUnitTests.java | 25 +++++++++- 6 files changed, 101 insertions(+), 30 deletions(-) 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 2d4589796..cb7e2555c 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 @@ -58,10 +58,12 @@ public class JpaRepositoryConfig extends JpaInfrastructureConfig { void someMethod(@PathVariable String id) {} } - @RepositoryRestController(path = {"orders", "orders/v2"}) + @RepositoryRestController({ "orders", "orders/v2" }) static class OrdersJsonController { - @RequestMapping(value = {"/search/sort","/search/sorted"}, method = RequestMethod.POST, produces = "application/hal+json") + @RequestMapping(path = { "/search/sort", "/search/sorted" }, // + method = RequestMethod.POST, // + produces = "application/hal+json") void someMethodWithArgs(Sort sort, Pageable pageable, DefaultedPageable defaultedPageable) {} } } diff --git a/spring-data-rest-tests/spring-data-rest-tests-jpa/src/test/java/org/springframework/data/rest/webmvc/jpa/JpaWebTests.java b/spring-data-rest-tests/spring-data-rest-tests-jpa/src/test/java/org/springframework/data/rest/webmvc/jpa/JpaWebTests.java index eaedda21e..8edeaa9c8 100755 --- a/spring-data-rest-tests/spring-data-rest-tests-jpa/src/test/java/org/springframework/data/rest/webmvc/jpa/JpaWebTests.java +++ b/spring-data-rest-tests/spring-data-rest-tests-jpa/src/test/java/org/springframework/data/rest/webmvc/jpa/JpaWebTests.java @@ -682,9 +682,13 @@ public class JpaWebTests extends CommonWebTests { andExpect(client.hasLinkWithRel(IanaLinkRelations.SELF)); } - @Test // DATAREST-910 DATAREST-2088 + @Test // DATAREST-910, #2087 void callUnmappedCustomRepositoryController() throws Exception { + + // Invalid prefix mvc.perform(post("/orders/v3/search/sort")).andExpect(status().isNotFound()); + + // With mapped prefixes mvc.perform(post("/orders/search/sort")).andExpect(status().isOk()); mvc.perform(post("/orders/search/sorted")).andExpect(status().isOk()); mvc.perform(post("/orders/v2/search/sort")).andExpect(status().isOk()); diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/BasePathAwareController.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/BasePathAwareController.java index f1c055feb..65baeb195 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/BasePathAwareController.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/BasePathAwareController.java @@ -29,15 +29,31 @@ import org.springframework.stereotype.Component; * REST configuration. * * @author Oliver Gierke + * @author Yves Galante */ @Documented @Component @Retention(RetentionPolicy.RUNTIME) @Target({ ElementType.TYPE, ElementType.ANNOTATION_TYPE }) public @interface BasePathAwareController { - @AliasFor("path") - String[] value() default {}; - @AliasFor("value") - String[] path() default {}; + /** + * The root path to be prepended to all request mappings configured on handler methods. + * + * @return + * @since 3.7.2 + * @see #path() + */ + @AliasFor("path") + String[] value() default {}; + + /** + * The root path to be prepended to all request mappings configured on handler methods. + * + * @return + * @since 3.7.2 + * @see #value() + */ + @AliasFor("value") + String[] path() default {}; } 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 a86c0a3ce..a86a74a4f 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 @@ -15,11 +15,13 @@ */ package org.springframework.data.rest.webmvc; +import static org.springframework.core.annotation.AnnotatedElementUtils.*; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletRequestWrapper; import java.lang.reflect.Method; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.Enumeration; import java.util.List; @@ -39,10 +41,9 @@ 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; +import org.springframework.web.servlet.mvc.method.RequestMappingInfo.Builder; import org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerMapping; -import static org.springframework.core.annotation.AnnotatedElementUtils.*; - /** * A {@link RequestMappingHandlerMapping} that augments the request mappings * @@ -100,6 +101,7 @@ public class BasePathAwareHandlerMapping extends RequestMappingHandlerMapping { } @Override + @SuppressWarnings("null") protected RequestMappingInfo getMappingForMethod(Method method, Class handlerType) { RequestMappingInfo info = super.getMappingForMethod(method, handlerType); @@ -110,23 +112,17 @@ public class BasePathAwareHandlerMapping extends RequestMappingHandlerMapping { ProducesRequestCondition producesCondition = customize(info.getProducesCondition()); Set mediaTypes = producesCondition.getProducibleMediaTypes(); + String[] customPrefixes = getBasePathedPrefixes(handlerType); + Builder builder = info.mutate(); - BasePathAwareController mergedAnnotation = findMergedAnnotation(handlerType, BasePathAwareController.class); - if (mergedAnnotation != null) { - info = appendPathPrefix(info, mergedAnnotation.value()); + if ((customPrefixes.length != 0) || StringUtils.hasText(baseUri)) { + builder = builder.paths(resolveEmbeddedValuesInPatterns(customPrefixes)); } - info = appendPathPrefix(info, new String[]{this.baseUri}); - return info.mutate() - .produces(mediaTypes.stream().map(MediaType::toString).toArray(String[]::new)) - .build(); - } - private RequestMappingInfo appendPathPrefix(RequestMappingInfo info, String[] pathPrefix) { - if (pathPrefix.length > 0) { - String[] paths = this.resolveEmbeddedValuesInPatterns(pathPrefix); - return info.mutate().paths(paths).build().combine(info); - } - return info; + return builder // + .produces(mediaTypes.stream().map(MediaType::toString).toArray(String[]::new)) // + .build() // + .combine(info); } /** @@ -174,6 +170,18 @@ public class BasePathAwareHandlerMapping extends RequestMappingHandlerMapping { return type.isAnnotationPresent(BasePathAwareController.class); } + private String[] getBasePathedPrefixes(Class handlerType) { + + Assert.notNull(handlerType, "Handler type must not be null"); + + BasePathAwareController mergedAnnotation = findMergedAnnotation(handlerType, BasePathAwareController.class); + String[] customPrefixes = mergedAnnotation == null ? new String[0] : mergedAnnotation.value(); + + return customPrefixes.length == 0 // + ? new String[] { baseUri } // + : Arrays.stream(customPrefixes).map(baseUri::concat).toArray(String[]::new); + } + /** * {@link HttpServletRequest} that exposes the given media types for the {@code Accept} header. * @@ -211,7 +219,8 @@ public class BasePathAwareHandlerMapping extends RequestMappingHandlerMapping { @Override public String getHeader(String name) { - return HttpHeaders.ACCEPT.equalsIgnoreCase(name) && acceptMediaTypes != null // + return HttpHeaders.ACCEPT.equalsIgnoreCase(name) && (acceptMediaTypes != null // + ) ? StringUtils.collectionToCommaDelimitedString(acceptMediaTypes) // : super.getHeader(name); } @@ -219,7 +228,8 @@ public class BasePathAwareHandlerMapping extends RequestMappingHandlerMapping { @Override public Enumeration getHeaders(String name) { - return HttpHeaders.ACCEPT.equalsIgnoreCase(name) && acceptMediaTypes != null // + return HttpHeaders.ACCEPT.equalsIgnoreCase(name) && (acceptMediaTypes != null // + ) ? Collections.enumeration(acceptMediaTypeStrings) // : super.getHeaders(name); } diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryRestController.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryRestController.java index 2058c0e3f..e141d4928 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryRestController.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryRestController.java @@ -40,6 +40,7 @@ import org.springframework.web.servlet.HandlerMapping; * * * @author Oliver Gierke + * @author Yves Galante */ @Documented @Component @@ -47,9 +48,24 @@ import org.springframework.web.servlet.HandlerMapping; @Target({ ElementType.TYPE, ElementType.ANNOTATION_TYPE }) @BasePathAwareController public @interface RepositoryRestController { - @AliasFor("path") - String[] value() default {}; - @AliasFor("value") - String[] path() default {}; + /** + * The root path to be prepended to all request mappings configured on handler methods. + * + * @return + * @since 3.7.2 + * @see #path() + */ + @AliasFor("path") + String[] value() default {}; + + /** + * The root path to be prepended to all request mappings configured on handler methods. + * + * @return + * @since 3.7.2 + * @see #value() + */ + @AliasFor("value") + String[] path() default {}; } 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 5410cc727..ef27d70ef 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 @@ -26,6 +26,8 @@ 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.util.ReflectionUtils; +import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.RequestMapping; /** @@ -36,11 +38,11 @@ import org.springframework.web.bind.annotation.RequestMapping; class BasePathAwareHandlerMappingUnitTests { HandlerMappingStub mapping; + RepositoryRestConfiguration configuration = mock(RepositoryRestConfiguration.class); @BeforeEach void setUp() { - RepositoryRestConfiguration configuration = mock(RepositoryRestConfiguration.class); doReturn(URI.create("")).when(configuration).getBasePath(); mapping = new HandlerMappingStub(configuration); @@ -79,6 +81,20 @@ class BasePathAwareHandlerMappingUnitTests { .isThrownBy(() -> mapping.isHandler(ValidController.class)); } + @Test // #2087 + void combinesBasePathAndControllerPrefixesCorrectly() throws Exception { + + doReturn(URI.create("/base")).when(configuration).getBasePath(); + mapping = new HandlerMappingStub(configuration); + + var method = ReflectionUtils.findMethod(PrefixedController.class, "someMethod"); + var info = mapping.getMappingForMethod(method, PrefixedController.class); + + var next = info.getPatternValues().iterator().next(); + + assertThat(next).isEqualTo("/base/controllerBase/method"); + } + private static Class createProxy(Object source) { ProxyFactory factory = new ProxyFactory(source); @@ -114,4 +130,11 @@ class BasePathAwareHandlerMappingUnitTests { @Controller @RequestMapping("/sample") static class ValidController {} + + @BasePathAwareController("/controllerBase") + static class PrefixedController { + + @GetMapping("/method") + void someMethod() {} + } }