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.
This commit is contained in:
Oliver Drotbohm
2022-07-06 16:15:20 +02:00
parent 8daacadb1e
commit 07ebc501cd
6 changed files with 101 additions and 30 deletions

View File

@@ -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) {}
}
}

View File

@@ -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());

View File

@@ -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 {};
}

View File

@@ -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<MediaType> 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<String> getHeaders(String name) {
return HttpHeaders.ACCEPT.equalsIgnoreCase(name) && acceptMediaTypes != null //
return HttpHeaders.ACCEPT.equalsIgnoreCase(name) && (acceptMediaTypes != null //
)
? Collections.enumeration(acceptMediaTypeStrings) //
: super.getHeaders(name);
}

View File

@@ -40,6 +40,7 @@ import org.springframework.web.servlet.HandlerMapping;
* </ul>
*
* @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 {};
}

View File

@@ -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() {}
}
}