diff --git a/spring-web-reactive/src/main/java/org/springframework/web/reactive/result/method/annotation/PathVariableMethodArgumentResolver.java b/spring-web-reactive/src/main/java/org/springframework/web/reactive/result/method/annotation/PathVariableMethodArgumentResolver.java index 817f069a1d..d625096726 100644 --- a/spring-web-reactive/src/main/java/org/springframework/web/reactive/result/method/annotation/PathVariableMethodArgumentResolver.java +++ b/spring-web-reactive/src/main/java/org/springframework/web/reactive/result/method/annotation/PathVariableMethodArgumentResolver.java @@ -48,12 +48,12 @@ import org.springframework.web.server.ServerWebExchange; * {@link Converter}. * * @author Rossen Stoyanchev + * @author Juergen Hoeller * @since 5.0 * @see PathVariableMapMethodArgumentResolver */ public class PathVariableMethodArgumentResolver extends AbstractNamedValueMethodArgumentResolver { - public PathVariableMethodArgumentResolver(ConversionService conversionService, ConfigurableBeanFactory beanFactory) { @@ -108,7 +108,7 @@ public class PathVariableMethodArgumentResolver extends AbstractNamedValueMethod private static class PathVariableNamedValueInfo extends NamedValueInfo { public PathVariableNamedValueInfo(PathVariable annotation) { - super(annotation.value(), true, ValueConstants.DEFAULT_NONE); + super(annotation.name(), annotation.required(), ValueConstants.DEFAULT_NONE); } } diff --git a/spring-web-reactive/src/test/java/org/springframework/web/reactive/result/method/annotation/PathVariableMethodArgumentResolverTests.java b/spring-web-reactive/src/test/java/org/springframework/web/reactive/result/method/annotation/PathVariableMethodArgumentResolverTests.java index afb9290a7e..b7bc0702a5 100644 --- a/spring-web-reactive/src/test/java/org/springframework/web/reactive/result/method/annotation/PathVariableMethodArgumentResolverTests.java +++ b/spring-web-reactive/src/test/java/org/springframework/web/reactive/result/method/annotation/PathVariableMethodArgumentResolverTests.java @@ -20,12 +20,14 @@ import java.lang.reflect.Method; import java.net.URI; import java.util.HashMap; import java.util.Map; +import java.util.Optional; import org.junit.Before; import org.junit.Test; import reactor.core.publisher.Mono; import org.springframework.core.MethodParameter; +import org.springframework.core.annotation.SynthesizingMethodParameter; import org.springframework.core.convert.ConversionService; import org.springframework.core.convert.support.DefaultConversionService; import org.springframework.http.HttpMethod; @@ -34,6 +36,7 @@ import org.springframework.http.server.reactive.MockServerHttpResponse; import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.tests.TestSubscriber; import org.springframework.ui.ModelMap; +import org.springframework.util.ReflectionUtils; import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.reactive.HandlerMapping; import org.springframework.web.server.ServerErrorException; @@ -42,14 +45,13 @@ import org.springframework.web.server.adapter.DefaultServerWebExchange; import org.springframework.web.server.session.MockWebSessionManager; import org.springframework.web.server.session.WebSessionManager; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.*; /** * Unit tests for {@link PathVariableMethodArgumentResolver}. * * @author Rossen Stoyanchev + * @author Juergen Hoeller */ public class PathVariableMethodArgumentResolverTests { @@ -58,8 +60,13 @@ public class PathVariableMethodArgumentResolverTests { private ServerWebExchange exchange; private MethodParameter paramNamedString; + private MethodParameter paramString; + private MethodParameter paramNotRequired; + + private MethodParameter paramOptional; + @Before public void setUp() throws Exception { @@ -70,9 +77,11 @@ public class PathVariableMethodArgumentResolverTests { WebSessionManager sessionManager = new MockWebSessionManager(); this.exchange = new DefaultServerWebExchange(request, new MockServerHttpResponse(), sessionManager); - Method method = getClass().getMethod("handle", String.class, String.class); - this.paramNamedString = new MethodParameter(method, 0); - this.paramString = new MethodParameter(method, 1); + Method method = ReflectionUtils.findMethod(getClass(), "handle", (Class[]) null); + paramNamedString = new SynthesizingMethodParameter(method, 0); + paramString = new SynthesizingMethodParameter(method, 1); + paramNotRequired = new SynthesizingMethodParameter(method, 2); + paramOptional = new SynthesizingMethodParameter(method, 3); } @@ -90,11 +99,31 @@ public class PathVariableMethodArgumentResolverTests { Mono mono = this.resolver.resolveArgument(this.paramNamedString, new ModelMap(), this.exchange); Object result = mono.block(); - - assertTrue(result instanceof String); assertEquals("value", result); } + @Test + public void resolveArgumentNotRequired() throws Exception { + Map uriTemplateVars = new HashMap<>(); + uriTemplateVars.put("name", "value"); + this.exchange.getAttributes().put(HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE, uriTemplateVars); + + Mono mono = this.resolver.resolveArgument(this.paramNotRequired, new ModelMap(), this.exchange); + Object result = mono.block(); + assertEquals("value", result); + } + + @Test + public void resolveArgumentWrappedAsOptional() throws Exception { + Map uriTemplateVars = new HashMap<>(); + uriTemplateVars.put("name", "value"); + this.exchange.getAttributes().put(HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE, uriTemplateVars); + + Mono mono = this.resolver.resolveArgument(this.paramOptional, new ModelMap(), this.exchange); + Object result = mono.block(); + assertEquals(Optional.of("value"), result); + } + @Test public void handleMissingValue() throws Exception { Mono mono = this.resolver.resolveArgument(this.paramNamedString, new ModelMap(), this.exchange); @@ -103,8 +132,29 @@ public class PathVariableMethodArgumentResolverTests { .assertError(ServerErrorException.class); } - @SuppressWarnings("unused") - public void handle(@PathVariable(value = "name") String param1, String param2) { + @Test + public void nullIfNotRequired() throws Exception { + Mono mono = this.resolver.resolveArgument(this.paramNotRequired, new ModelMap(), this.exchange); + TestSubscriber + .subscribe(mono) + .assertComplete() + .assertNoValues(); } -} \ No newline at end of file + @Test + public void wrapEmptyWithOptional() throws Exception { + Mono mono = this.resolver.resolveArgument(this.paramOptional, new ModelMap(), this.exchange); + Object result = mono.block(); + TestSubscriber + .subscribe(mono) + .assertValues(Optional.empty()); + } + + + @SuppressWarnings("unused") + public void handle(@PathVariable(value = "name") String param1, String param2, + @PathVariable(name="name", required = false) String param3, + @PathVariable("name") Optional param4) { + } + +} diff --git a/spring-web/src/main/java/org/springframework/web/bind/annotation/PathVariable.java b/spring-web/src/main/java/org/springframework/web/bind/annotation/PathVariable.java index 6a4ba43967..d07619f76b 100644 --- a/spring-web/src/main/java/org/springframework/web/bind/annotation/PathVariable.java +++ b/spring-web/src/main/java/org/springframework/web/bind/annotation/PathVariable.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2016 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. @@ -22,6 +22,8 @@ import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; +import org.springframework.core.annotation.AliasFor; + /** * Annotation which indicates that a method parameter should be bound to a URI template * variable. Supported for {@link RequestMapping} annotated handler methods in Servlet @@ -32,6 +34,7 @@ import java.lang.annotation.Target; * then the map is populated with all path variable names and values. * * @author Arjen Poutsma + * @author Juergen Hoeller * @since 3.0 * @see RequestMapping * @see org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter @@ -42,8 +45,26 @@ import java.lang.annotation.Target; public @interface PathVariable { /** - * The URI template variable to bind to. + * Alias for {@link #name}. */ + @AliasFor("name") String value() default ""; + /** + * The name of the path variable to bind to. + * @since 4.3.3 + */ + @AliasFor("value") + String name() default ""; + + /** + * Whether the path variable is required. + *

Defaults to {@code true}, leading to an exception being thrown if the path + * variable is missing in the incoming request. Switch this to {@code false} if + * you prefer a {@code null} or Java 8 {@code java.util.Optional} in this case. + * e.g. on a {@code ModelAttribute} method which serves for different requests. + * @since 4.3.3 + */ + boolean required() default true; + } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/PathVariableMethodArgumentResolver.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/PathVariableMethodArgumentResolver.java index 12aa853c93..bd88bc9eaa 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/PathVariableMethodArgumentResolver.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/PathVariableMethodArgumentResolver.java @@ -57,6 +57,7 @@ import org.springframework.web.util.UriComponentsBuilder; * * @author Rossen Stoyanchev * @author Arjen Poutsma + * @author Juergen Hoeller * @since 3.1 */ public class PathVariableMethodArgumentResolver extends AbstractNamedValueMethodArgumentResolver @@ -65,10 +66,6 @@ public class PathVariableMethodArgumentResolver extends AbstractNamedValueMethod private static final TypeDescriptor STRING_TYPE_DESCRIPTOR = TypeDescriptor.valueOf(String.class); - public PathVariableMethodArgumentResolver() { - } - - @Override public boolean supportsParameter(MethodParameter parameter) { if (!parameter.hasParameterAnnotation(PathVariable.class)) { @@ -96,9 +93,7 @@ public class PathVariableMethodArgumentResolver extends AbstractNamedValueMethod } @Override - protected void handleMissingValue(String name, MethodParameter parameter) - throws ServletRequestBindingException { - + protected void handleMissingValue(String name, MethodParameter parameter) throws ServletRequestBindingException { throw new MissingPathVariableException(name, parameter); } @@ -121,13 +116,13 @@ public class PathVariableMethodArgumentResolver extends AbstractNamedValueMethod public void contributeMethodArgument(MethodParameter parameter, Object value, UriComponentsBuilder builder, Map uriVariables, ConversionService conversionService) { - if (Map.class.isAssignableFrom(parameter.getNestedParameterType())) { + if (Map.class.isAssignableFrom(parameter.nestedIfOptional().getNestedParameterType())) { return; } PathVariable ann = parameter.getParameterAnnotation(PathVariable.class); - String name = (ann == null || StringUtils.isEmpty(ann.value()) ? parameter.getParameterName() : ann.value()); - value = formatUriValue(conversionService, new TypeDescriptor(parameter), value); + String name = (ann != null && !StringUtils.isEmpty(ann.value()) ? ann.value() : parameter.getParameterName()); + value = formatUriValue(conversionService, new TypeDescriptor(parameter.nestedIfOptional()), value); uriVariables.put(name, value); } @@ -150,7 +145,7 @@ public class PathVariableMethodArgumentResolver extends AbstractNamedValueMethod private static class PathVariableNamedValueInfo extends NamedValueInfo { public PathVariableNamedValueInfo(PathVariable annotation) { - super(annotation.value(), true, ValueConstants.DEFAULT_NONE); + super(annotation.name(), annotation.required(), ValueConstants.DEFAULT_NONE); } } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/PathVariableMethodArgumentResolverTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/PathVariableMethodArgumentResolverTests.java index 2e291ad379..4cc08a760f 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/PathVariableMethodArgumentResolverTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/PathVariableMethodArgumentResolverTests.java @@ -16,29 +16,37 @@ package org.springframework.web.servlet.mvc.method.annotation; -import static org.junit.Assert.*; - import java.lang.reflect.Method; import java.util.HashMap; import java.util.Map; +import java.util.Optional; import org.junit.Before; import org.junit.Test; import org.springframework.core.MethodParameter; +import org.springframework.core.annotation.SynthesizingMethodParameter; +import org.springframework.core.convert.support.DefaultConversionService; import org.springframework.mock.web.test.MockHttpServletRequest; import org.springframework.mock.web.test.MockHttpServletResponse; +import org.springframework.util.ReflectionUtils; import org.springframework.web.bind.MissingPathVariableException; import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.support.ConfigurableWebBindingInitializer; +import org.springframework.web.bind.support.DefaultDataBinderFactory; +import org.springframework.web.bind.support.WebDataBinderFactory; import org.springframework.web.context.request.ServletWebRequest; import org.springframework.web.method.support.ModelAndViewContainer; import org.springframework.web.servlet.HandlerMapping; import org.springframework.web.servlet.View; +import static org.junit.Assert.*; + /** * Test fixture with {@link PathVariableMethodArgumentResolver}. * * @author Rossen Stoyanchev + * @author Juergen Hoeller */ public class PathVariableMethodArgumentResolverTests { @@ -48,25 +56,33 @@ public class PathVariableMethodArgumentResolverTests { private MethodParameter paramString; + private MethodParameter paramNotRequired; + + private MethodParameter paramOptional; + private ModelAndViewContainer mavContainer; private ServletWebRequest webRequest; private MockHttpServletRequest request; + @Before public void setUp() throws Exception { resolver = new PathVariableMethodArgumentResolver(); - Method method = getClass().getMethod("handle", String.class, String.class); - paramNamedString = new MethodParameter(method, 0); - paramString = new MethodParameter(method, 1); + Method method = ReflectionUtils.findMethod(getClass(), "handle", (Class[]) null); + paramNamedString = new SynthesizingMethodParameter(method, 0); + paramString = new SynthesizingMethodParameter(method, 1); + paramNotRequired = new SynthesizingMethodParameter(method, 2); + paramOptional = new SynthesizingMethodParameter(method, 3); mavContainer = new ModelAndViewContainer(); request = new MockHttpServletRequest(); webRequest = new ServletWebRequest(request, new MockHttpServletResponse()); } + @Test public void supportsParameter() { assertTrue("Parameter with @PathVariable annotation", resolver.supportsParameter(paramNamedString)); @@ -89,21 +105,58 @@ public class PathVariableMethodArgumentResolverTests { assertEquals("value", pathVars.get("name")); } - @SuppressWarnings("unchecked") + @Test + public void resolveArgumentNotRequired() throws Exception { + Map uriTemplateVars = new HashMap<>(); + uriTemplateVars.put("name", "value"); + request.setAttribute(HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE, uriTemplateVars); + + String result = (String) resolver.resolveArgument(paramNotRequired, mavContainer, webRequest, null); + assertEquals("PathVariable not resolved correctly", "value", result); + + @SuppressWarnings("unchecked") + Map pathVars = (Map) request.getAttribute(View.PATH_VARIABLES); + assertNotNull(pathVars); + assertEquals(1, pathVars.size()); + assertEquals("value", pathVars.get("name")); + } + + @Test + public void resolveArgumentWrappedAsOptional() throws Exception { + Map uriTemplateVars = new HashMap<>(); + uriTemplateVars.put("name", "value"); + request.setAttribute(HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE, uriTemplateVars); + + ConfigurableWebBindingInitializer initializer = new ConfigurableWebBindingInitializer(); + initializer.setConversionService(new DefaultConversionService()); + WebDataBinderFactory binderFactory = new DefaultDataBinderFactory(initializer); + + @SuppressWarnings("unchecked") + Optional result = (Optional) + resolver.resolveArgument(paramOptional, mavContainer, webRequest, binderFactory); + assertEquals("PathVariable not resolved correctly", "value", result.get()); + + @SuppressWarnings("unchecked") + Map pathVars = (Map) request.getAttribute(View.PATH_VARIABLES); + assertNotNull(pathVars); + assertEquals(1, pathVars.size()); + assertEquals(Optional.of("value"), pathVars.get("name")); + } + @Test public void resolveArgumentWithExistingPathVars() throws Exception { Map uriTemplateVars = new HashMap<>(); uriTemplateVars.put("name", "value"); request.setAttribute(HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE, uriTemplateVars); - Map pathVars; uriTemplateVars.put("oldName", "oldValue"); request.setAttribute(View.PATH_VARIABLES, uriTemplateVars); String result = (String) resolver.resolveArgument(paramNamedString, mavContainer, webRequest, null); assertEquals("PathVariable not resolved correctly", "value", result); - pathVars = (Map) request.getAttribute(View.PATH_VARIABLES); + @SuppressWarnings("unchecked") + Map pathVars = (Map) request.getAttribute(View.PATH_VARIABLES); assertNotNull(pathVars); assertEquals(2, pathVars.size()); assertEquals("value", pathVars.get("name")); @@ -113,11 +166,28 @@ public class PathVariableMethodArgumentResolverTests { @Test(expected = MissingPathVariableException.class) public void handleMissingValue() throws Exception { resolver.resolveArgument(paramNamedString, mavContainer, webRequest, null); - fail("Unresolved path variable should lead to exception."); + fail("Unresolved path variable should lead to exception"); } + @Test + public void nullIfNotRequired() throws Exception { + assertNull(resolver.resolveArgument(paramNotRequired, mavContainer, webRequest, null)); + } + + @Test + public void wrapEmptyWithOptional() throws Exception { + ConfigurableWebBindingInitializer initializer = new ConfigurableWebBindingInitializer(); + initializer.setConversionService(new DefaultConversionService()); + WebDataBinderFactory binderFactory = new DefaultDataBinderFactory(initializer); + + assertEquals(Optional.empty(), resolver.resolveArgument(paramOptional, mavContainer, webRequest, binderFactory)); + } + + @SuppressWarnings("unused") - public void handle(@PathVariable(value = "name") String param1, String param2) { + public void handle(@PathVariable("name") String param1, String param2, + @PathVariable(name="name", required = false) String param3, + @PathVariable("name") Optional param4) { } -} \ No newline at end of file +}