Skip to content

Commit 61cdc84

Browse files
jhoellerjhoeller
committed
BindingResult support for constructor argument mismatch on immutable data object
Issue: SPR-15542
1 parent b8f5e97 commit 61cdc84

File tree

4 files changed

+415
-345
lines changed

4 files changed

+415
-345
lines changed

‎spring-web/src/main/java/org/springframework/web/method/annotation/ModelAttributeMethodProcessor.java

Lines changed: 112 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -20,19 +20,23 @@
2020
import java.lang.annotation.Annotation;
2121
import java.lang.reflect.Constructor;
2222
import java.util.Map;
23+
import java.util.Optional;
2324

2425
import org.apache.commons.logging.Log;
2526
import org.apache.commons.logging.LogFactory;
2627

2728
import org.springframework.beans.BeanUtils;
29+
import org.springframework.beans.TypeMismatchException;
2830
import org.springframework.core.DefaultParameterNameDiscoverer;
2931
import org.springframework.core.MethodParameter;
3032
import org.springframework.core.ParameterNameDiscoverer;
3133
import org.springframework.core.annotation.AnnotationUtils;
3234
import org.springframework.lang.Nullable;
3335
import org.springframework.util.Assert;
3436
import org.springframework.validation.BindException;
37+
import org.springframework.validation.BindingResult;
3538
import org.springframework.validation.Errors;
39+
import org.springframework.validation.FieldError;
3640
import org.springframework.validation.annotation.Validated;
3741
import org.springframework.web.bind.WebDataBinder;
3842
import org.springframework.web.bind.annotation.ModelAttribute;
@@ -110,61 +114,120 @@ public final Object resolveArgument(MethodParameter parameter, @Nullable ModelAn
110114
Assert.state(binderFactory != null, "ModelAttributeMethodProcessor requires WebDataBinderFactory");
111115

112116
String name = ModelFactory.getNameForParameter(parameter);
113-
Object attribute = (mavContainer.containsAttribute(name) ? mavContainer.getModel().get(name) :
114-
createAttribute(name, parameter, binderFactory, webRequest));
115-
116117
if (!mavContainer.isBindingDisabled(name)) {
117118
ModelAttribute ann = parameter.getParameterAnnotation(ModelAttribute.class);
118119
if (ann != null && !ann.binding()) {
119120
mavContainer.setBindingDisabled(name);
120121
}
121122
}
122123

123-
WebDataBinder binder = binderFactory.createBinder(webRequest, attribute, name);
124-
if (binder.getTarget() != null) {
125-
if (!mavContainer.isBindingDisabled(name)) {
126-
bindRequestParameters(binder, webRequest);
124+
Object attribute = null;
125+
BindingResult bindingResult = null;
126+
127+
if (mavContainer.containsAttribute(name)) {
128+
attribute = mavContainer.getModel().get(name);
129+
}
130+
else {
131+
// Create attribute instance
132+
try {
133+
attribute = createAttribute(name, parameter, binderFactory, webRequest);
134+
}
135+
catch (BindException ex) {
136+
if (isBindExceptionRequired(parameter)) {
137+
// No BindingResult parameter -> fail with BindException
138+
throw ex;
139+
}
140+
// Otherwise, expose null/empty value and associated BindingResult
141+
if (parameter.getParameterType() == Optional.class) {
142+
attribute = Optional.empty();
143+
}
144+
bindingResult = ex.getBindingResult();
145+
}
146+
}
147+
148+
if (bindingResult == null) {
149+
// Bean property binding and validation;
150+
// skipped in case of binding failure on construction.
151+
WebDataBinder binder = binderFactory.createBinder(webRequest, attribute, name);
152+
if (binder.getTarget() != null) {
153+
if (!mavContainer.isBindingDisabled(name)) {
154+
bindRequestParameters(binder, webRequest);
155+
}
156+
validateIfApplicable(binder, parameter);
157+
if (binder.getBindingResult().hasErrors() && isBindExceptionRequired(binder, parameter)) {
158+
throw new BindException(binder.getBindingResult());
159+
}
127160
}
128-
validateIfApplicable(binder, parameter);
129-
if (binder.getBindingResult().hasErrors() && isBindExceptionRequired(binder, parameter)) {
130-
throw new BindException(binder.getBindingResult());
161+
// Value type adaptation, also covering java.util.Optional
162+
if (!parameter.getParameterType().isInstance(attribute)) {
163+
attribute = binder.convertIfNecessary(binder.getTarget(), parameter.getParameterType(), parameter);
131164
}
165+
bindingResult = binder.getBindingResult();
132166
}
133167

134168
// Add resolved attribute and BindingResult at the end of the model
135-
Map<String, Object> bindingResultModel = binder.getBindingResult().getModel();
169+
Map<String, Object> bindingResultModel = bindingResult.getModel();
136170
mavContainer.removeAttributes(bindingResultModel);
137171
mavContainer.addAllAttributes(bindingResultModel);
138172

139-
return (parameter.getParameterType().isInstance(attribute) ? attribute :
140-
binder.convertIfNecessary(binder.getTarget(), parameter.getParameterType(), parameter));
173+
return attribute;
141174
}
142175

143176
/**
144177
* Extension point to create the model attribute if not found in the model,
145178
* with subsequent parameter binding through bean properties (unless suppressed).
146-
* <p>The default implementation uses the unique public no-arg constructor, if any,
147-
* which may have arguments: It understands the JavaBeans {@link ConstructorProperties}
148-
* annotation as well as runtime-retained parameter names in the bytecode,
149-
* associating request parameters with constructor arguments by name. If no such
150-
* constructor is found, the default constructor will be used (even if not public),
151-
* assuming subsequent bean property bindings through setter methods.
179+
* <p>The default implementation typically uses the unique public no-arg constructor
180+
* if available but also handles a "primary constructor" approach for data classes:
181+
* It understands the JavaBeans {@link ConstructorProperties} annotation as well as
182+
* runtime-retained parameter names in the bytecode, associating request parameters
183+
* with constructor arguments by name. If no such constructor is found, the default
184+
* constructor will be used (even if not public), assuming subsequent bean property
185+
* bindings through setter methods.
152186
* @param attributeName the name of the attribute (never {@code null})
153187
* @param parameter the method parameter declaration
154188
* @param binderFactory for creating WebDataBinder instance
155189
* @param webRequest the current request
156190
* @return the created model attribute (never {@code null})
191+
* @throws BindException in case of constructor argument binding failure
192+
* @throws Exception in case of constructor invocation failure
193+
* @see #constructAttribute(Constructor, String, WebDataBinderFactory, NativeWebRequest)
194+
* @see BeanUtils#findPrimaryConstructor(Class)
157195
*/
158196
protected Object createAttribute(String attributeName, MethodParameter parameter,
159197
WebDataBinderFactory binderFactory, NativeWebRequest webRequest) throws Exception {
160198

161-
Class<?> type = parameter.getParameterType();
162-
199+
MethodParameter nestedParameter = parameter.nestedIfOptional();
200+
Class<?> type = nestedParameter.getNestedParameterType();
201+
163202
Constructor<?> ctor = BeanUtils.findPrimaryConstructor(type);
164203
if (ctor == null) {
165204
throw new IllegalStateException("No primary constructor found for " + type.getName());
166205
}
167206

207+
Object attribute = constructAttribute(ctor, attributeName, binderFactory, webRequest);
208+
if (parameter != nestedParameter) {
209+
attribute = Optional.of(attribute);
210+
}
211+
return attribute;
212+
}
213+
214+
/**
215+
* Construct a new attribute instance with the given constructor.
216+
* <p>Called from
217+
* {@link #createAttribute(String, MethodParameter, WebDataBinderFactory, NativeWebRequest)}
218+
* after constructor resolution.
219+
* @param ctor the constructor to use
220+
* @param attributeName the name of the attribute (never {@code null})
221+
* @param binderFactory for creating WebDataBinder instance
222+
* @param webRequest the current request
223+
* @return the created model attribute (never {@code null})
224+
* @throws BindException in case of constructor argument binding failure
225+
* @throws Exception in case of constructor invocation failure
226+
* @since 5.0
227+
*/
228+
protected Object constructAttribute(Constructor<?> ctor, String attributeName,
229+
WebDataBinderFactory binderFactory, NativeWebRequest webRequest) throws Exception {
230+
168231
if (ctor.getParameterCount() == 0) {
169232
// A single default constructor -> clearly a standard JavaBeans arrangement.
170233
return BeanUtils.instantiateClass(ctor);
@@ -179,10 +242,22 @@ protected Object createAttribute(String attributeName, MethodParameter parameter
179242
() -> "Invalid number of parameter names: " + paramNames.length + " for constructor " + ctor);
180243
Object[] args = new Object[paramTypes.length];
181244
WebDataBinder binder = binderFactory.createBinder(webRequest, null, attributeName);
245+
boolean bindingFailure = false;
182246
for (int i = 0; i < paramNames.length; i++) {
183-
String[] parameterValues = webRequest.getParameterValues(paramNames[i]);
184-
args[i] = (parameterValues != null ? binder.convertIfNecessary(parameterValues, paramTypes[i],
185-
new MethodParameter(ctor, i)) : null);
247+
String[] paramValues = webRequest.getParameterValues(paramNames[i]);
248+
try {
249+
args[i] = (paramValues != null ?
250+
binder.convertIfNecessary(paramValues, paramTypes[i], new MethodParameter(ctor, i)) : null);
251+
}
252+
catch (TypeMismatchException ex) {
253+
bindingFailure = true;
254+
binder.getBindingResult().addError(new FieldError(
255+
binder.getObjectName(), paramNames[i], ex.getValue(), true,
256+
new String[] {ex.getErrorCode()}, null, ex.getLocalizedMessage()));
257+
}
258+
}
259+
if (bindingFailure) {
260+
throw new BindException(binder.getBindingResult());
186261
}
187262
return BeanUtils.instantiateClass(ctor, args);
188263
}
@@ -219,11 +294,23 @@ protected void validateIfApplicable(WebDataBinder binder, MethodParameter parame
219294

220295
/**
221296
* Whether to raise a fatal bind exception on validation errors.
297+
* <p>The default implementation delegates to {@link #isBindExceptionRequired(MethodParameter)}.
222298
* @param binder the data binder used to perform data binding
223299
* @param parameter the method parameter declaration
224-
* @return {@code true} if the next method argument is not of type {@link Errors}
300+
* @return {@code true} if the next method parameter is not of type {@link Errors}
301+
* @see #isBindExceptionRequired(MethodParameter)
225302
*/
226303
protected boolean isBindExceptionRequired(WebDataBinder binder, MethodParameter parameter) {
304+
return isBindExceptionRequired(parameter);
305+
}
306+
307+
/**
308+
* Whether to raise a fatal bind exception on validation errors.
309+
* @param parameter the method parameter declaration
310+
* @return {@code true} if the next method parameter is not of type {@link Errors}
311+
* @since 5.0
312+
*/
313+
protected boolean isBindExceptionRequired(MethodParameter parameter) {
227314
int i = parameter.getParameterIndex();
228315
Class<?>[] paramTypes = parameter.getExecutable().getParameterTypes();
229316
boolean hasBindingResult = (paramTypes.length > (i + 1) && Errors.class.isAssignableFrom(paramTypes[i + 1]));

‎spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ServletModelAttributeMethodProcessor.java

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
* model attribute name and there is an appropriate type conversion strategy.
4646
*
4747
* @author Rossen Stoyanchev
48+
* @author Juergen Hoeller
4849
* @since 3.1
4950
*/
5051
public class ServletModelAttributeMethodProcessor extends ModelAttributeMethodProcessor {
@@ -68,19 +69,19 @@ public ServletModelAttributeMethodProcessor(boolean annotationNotRequired) {
6869
* @see #createAttributeFromRequestValue
6970
*/
7071
@Override
71-
protected final Object createAttribute(String attributeName, MethodParameter methodParam,
72+
protected final Object createAttribute(String attributeName, MethodParameter parameter,
7273
WebDataBinderFactory binderFactory, NativeWebRequest request) throws Exception {
7374

7475
String value = getRequestValueForAttribute(attributeName, request);
7576
if (value != null) {
7677
Object attribute = createAttributeFromRequestValue(
77-
value, attributeName, methodParam, binderFactory, request);
78+
value, attributeName, parameter, binderFactory, request);
7879
if (attribute != null) {
7980
return attribute;
8081
}
8182
}
8283

83-
return super.createAttribute(attributeName, methodParam, binderFactory, request);
84+
return super.createAttribute(attributeName, parameter, binderFactory, request);
8485
}
8586

8687
/**
@@ -120,7 +121,7 @@ protected final Map<String, String> getUriTemplateVariables(NativeWebRequest req
120121
* {@link Converter} that can perform the conversion.
121122
* @param sourceValue the source value to create the model attribute from
122123
* @param attributeName the name of the attribute (never {@code null})
123-
* @param methodParam the method parameter
124+
* @param parameter the method parameter
124125
* @param binderFactory for creating WebDataBinder instance
125126
* @param request the current request
126127
* @return the created model attribute, or {@code null} if no suitable
@@ -129,16 +130,16 @@ protected final Map<String, String> getUriTemplateVariables(NativeWebRequest req
129130
*/
130131
@Nullable
131132
protected Object createAttributeFromRequestValue(String sourceValue, String attributeName,
132-
MethodParameter methodParam, WebDataBinderFactory binderFactory, NativeWebRequest request)
133+
MethodParameter parameter, WebDataBinderFactory binderFactory, NativeWebRequest request)
133134
throws Exception {
134135

135136
DataBinder binder = binderFactory.createBinder(request, null, attributeName);
136137
ConversionService conversionService = binder.getConversionService();
137138
if (conversionService != null) {
138139
TypeDescriptor source = TypeDescriptor.valueOf(String.class);
139-
TypeDescriptor target = new TypeDescriptor(methodParam);
140+
TypeDescriptor target = new TypeDescriptor(parameter);
140141
if (conversionService.canConvert(source, target)) {
141-
return binder.convertIfNecessary(sourceValue, methodParam.getParameterType(), methodParam);
142+
return binder.convertIfNecessary(sourceValue, parameter.getParameterType(), parameter);
142143
}
143144
}
144145
return null;

‎spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/AbstractServletHandlerMethodTests.java

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2012 the original author or authors.
2+
* Copyright 2002-2017 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -35,9 +35,9 @@
3535
/**
3636
* Base class for tests using on the DispatcherServlet and HandlerMethod infrastructure classes:
3737
* <ul>
38-
* <li>RequestMappingHandlerMapping
39-
* <li>RequestMappingHandlerAdapter
40-
* <li>ExceptionHandlerExceptionResolver
38+
* <li>RequestMappingHandlerMapping
39+
* <li>RequestMappingHandlerAdapter
40+
* <li>ExceptionHandlerExceptionResolver
4141
* </ul>
4242
*
4343
* @author Rossen Stoyanchev
@@ -46,21 +46,23 @@ public abstract class AbstractServletHandlerMethodTests {
4646

4747
private DispatcherServlet servlet;
4848

49-
@After
50-
public void tearDown() {
51-
this.servlet = null;
52-
}
5349

5450
protected DispatcherServlet getServlet() {
5551
assertNotNull("DispatcherServlet not initialized", servlet);
5652
return servlet;
5753
}
5854

55+
@After
56+
public void tearDown() {
57+
this.servlet = null;
58+
}
59+
5960
/**
6061
* Initialize a DispatcherServlet instance registering zero or more controller classes.
6162
*/
6263
protected WebApplicationContext initServletWithControllers(final Class<?>... controllerClasses)
6364
throws ServletException {
65+
6466
return initServlet(null, controllerClasses);
6567
}
6668

@@ -82,22 +84,17 @@ protected WebApplicationContext createWebApplicationContext(@Nullable WebApplica
8284
wac.registerBeanDefinition(clazz.getSimpleName(), new RootBeanDefinition(clazz));
8385
}
8486

85-
Class<?> mappingType = RequestMappingHandlerMapping.class;
86-
RootBeanDefinition beanDef = new RootBeanDefinition(mappingType);
87-
beanDef.getPropertyValues().add("removeSemicolonContent", "false");
88-
wac.registerBeanDefinition("handlerMapping", beanDef);
87+
RootBeanDefinition mappingDef = new RootBeanDefinition(RequestMappingHandlerMapping.class);
88+
mappingDef.getPropertyValues().add("removeSemicolonContent", "false");
89+
wac.registerBeanDefinition("handlerMapping", mappingDef);
8990

90-
Class<?> adapterType = RequestMappingHandlerAdapter.class;
91-
wac.registerBeanDefinition("handlerAdapter", new RootBeanDefinition(adapterType));
91+
wac.registerBeanDefinition("handlerAdapter", new RootBeanDefinition(RequestMappingHandlerAdapter.class));
9292

93-
Class<?> resolverType = ExceptionHandlerExceptionResolver.class;
94-
wac.registerBeanDefinition("requestMappingResolver", new RootBeanDefinition(resolverType));
93+
wac.registerBeanDefinition("requestMappingResolver", new RootBeanDefinition(ExceptionHandlerExceptionResolver.class));
9594

96-
resolverType = ResponseStatusExceptionResolver.class;
97-
wac.registerBeanDefinition("responseStatusResolver", new RootBeanDefinition(resolverType));
95+
wac.registerBeanDefinition("responseStatusResolver", new RootBeanDefinition(ResponseStatusExceptionResolver.class));
9896

99-
resolverType = DefaultHandlerExceptionResolver.class;
100-
wac.registerBeanDefinition("defaultResolver", new RootBeanDefinition(resolverType));
97+
wac.registerBeanDefinition("defaultResolver", new RootBeanDefinition(DefaultHandlerExceptionResolver.class));
10198

10299
if (initializer != null) {
103100
initializer.initialize(wac);

0 commit comments

Comments
 (0)