Peter Luttrell opened SPR-15371 and commented
If I create a RestController that uses BeanValidations to validate the input object such as the following:
@RestController
public class CarController {
@PostMapping("/cars")
public Car createCar(@RequestBody @Valid Car car) {
return car;
}
}
And the entity class contains a Java 8 Optional such as the following:
public class Car {
@Valid
private List<Driver> drivers;
public Car() {
}
public Optional<List<Driver>> getDrivers() {
return Optional.ofNullable(drivers);
}
public void setDrivers(List<Driver> drivers) {
this.drivers = drivers;
}
}
public class Driver {
@NotEmpty()
private String name;
@Min(16)
private int age;
public Driver() {
}
public String getName() { return name; }
public void setName(String name) { this.name = name; }
public int getAge() { return age; }
public void setAge(int age) { this.age = age; }
}
And you Post the following JSON to it:
{
"drivers": [
{
"name": "driver1",
"age": 14
}
]
}
You'll get the following error:
org.springframework.beans.InvalidPropertyException: Invalid property 'drivers[0]' of bean class [example.Car]: Property referenced in indexed property path 'drivers[0]' is neither an array nor a List nor a Set nor a Map; returned value was [Optional[[Driver{name='driver1', age=14}]]]
at org.springframework.beans.AbstractNestablePropertyAccessor.getPropertyValue(AbstractNestablePropertyAccessor.java:696) ~[spring-beans-4.3.7.RELEASE.jar:4.3.7.RELEASE]
at org.springframework.beans.AbstractNestablePropertyAccessor.getNestedPropertyAccessor(AbstractNestablePropertyAccessor.java:850) ~[spring-beans-4.3.7.RELEASE.jar:4.3.7.RELEASE]
at org.springframework.beans.AbstractNestablePropertyAccessor.getPropertyAccessorForPropertyPath(AbstractNestablePropertyAccessor.java:827) ~[spring-beans-4.3.7.RELEASE.jar:4.3.7.RELEASE]
at org.springframework.beans.AbstractNestablePropertyAccessor.getPropertyValue(AbstractNestablePropertyAccessor.java:622) ~[spring-beans-4.3.7.RELEASE.jar:4.3.7.RELEASE]
at org.springframework.validation.AbstractPropertyBindingResult.getActualFieldValue(AbstractPropertyBindingResult.java:99) ~[spring-context-4.3.7.RELEASE.jar:4.3.7.RELEASE]
at org.springframework.validation.AbstractBindingResult.getRawFieldValue(AbstractBindingResult.java:283) ~[spring-context-4.3.7.RELEASE.jar:4.3.7.RELEASE]
at org.springframework.validation.beanvalidation.SpringValidatorAdapter.getRejectedValue(SpringValidatorAdapter.java:260) ~[spring-context-4.3.7.RELEASE.jar:4.3.7.RELEASE]
at org.springframework.validation.beanvalidation.SpringValidatorAdapter.processConstraintViolations(SpringValidatorAdapter.java:140) ~[spring-context-4.3.7.RELEASE.jar:4.3.7.RELEASE]
at org.springframework.validation.beanvalidation.SpringValidatorAdapter.validate(SpringValidatorAdapter.java:109) ~[spring-context-4.3.7.RELEASE.jar:4.3.7.RELEASE]
at org.springframework.boot.autoconfigure.web.WebMvcValidator.validate(WebMvcValidator.java:69) ~[spring-boot-autoconfigure-1.5.2.RELEASE.jar:1.5.2.RELEASE]
at org.springframework.validation.DataBinder.validate(DataBinder.java:891) ~[spring-context-4.3.7.RELEASE.jar:4.3.7.RELEASE]
at org.springframework.web.servlet.mvc.method.annotation.AbstractMessageConverterMethodArgumentResolver.validateIfApplicable(AbstractMessageConverterMethodArgumentResolver.java:270) ~[spring-webmvc-4.3.7.RELEASE.jar:4.3.7.RELEASE]
at org.springframework.web.servlet.mvc.method.annotation.RequestResponseBodyMethodProcessor.resolveArgument(RequestResponseBodyMethodProcessor.java:133) ~[spring-webmvc-4.3.7.RELEASE.jar:4.3.7.RELEASE]
at org.springframework.web.method.support.HandlerMethodArgumentResolverComposite.resolveArgument(HandlerMethodArgumentResolverComposite.java:121) ~[spring-web-4.3.7.RELEASE.jar:4.3.7.RELEASE]
at org.springframework.web.method.support.InvocableHandlerMethod.getMethodArgumentValues(InvocableHandlerMethod.java:158) ~[spring-web-4.3.7.RELEASE.jar:4.3.7.RELEASE]
at org.springframework.web.method.support.InvocableHandlerMethod.invokeForRequest(InvocableHandlerMethod.java:128) ~[spring-web-4.3.7.RELEASE.jar:4.3.7.RELEASE]
at org.springframework.web.servlet.mvc.method.annotation.ServletInvocableHandlerMethod.invokeAndHandle(ServletInvocableHandlerMethod.java:116) ~[spring-webmvc-4.3.7.RELEASE.jar:4.3.7.RELEASE]
at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.invokeHandlerMethod(RequestMappingHandlerAdapter.java:827) ~[spring-webmvc-4.3.7.RELEASE.jar:4.3.7.RELEASE]
at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.handleInternal(RequestMappingHandlerAdapter.java:738) ~[spring-webmvc-4.3.7.RELEASE.jar:4.3.7.RELEASE]
at org.springframework.web.servlet.mvc.method.AbstractHandlerMethodAdapter.handle(AbstractHandlerMethodAdapter.java:85) ~[spring-webmvc-4.3.7.RELEASE.jar:4.3.7.RELEASE]
at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:963) ~[spring-webmvc-4.3.7.RELEASE.jar:4.3.7.RELEASE]
at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:897) ~[spring-webmvc-4.3.7.RELEASE.jar:4.3.7.RELEASE]
at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:970) ~[spring-webmvc-4.3.7.RELEASE.jar:4.3.7.RELEASE]
at org.springframework.web.servlet.FrameworkServlet.doPost(FrameworkServlet.java:872) ~[spring-webmvc-4.3.7.RELEASE.jar:4.3.7.RELEASE]
at javax.servlet.http.HttpServlet.service(HttpServlet.java:648) ~[tomcat-embed-core-8.5.11.jar:8.5.11]
at org.springframework.web.servlet.FrameworkServlet.service(FrameworkServlet.java:846) ~[spring-webmvc-4.3.7.RELEASE.jar:4.3.7.RELEASE]
at javax.servlet.http.HttpServlet.service(HttpServlet.java:729) ~[tomcat-embed-core-8.5.11.jar:8.5.11]
This appears to be caused by the fact that org.springframework.beans.AbstractNestablePropertyAccessor doesn't unwrap Optional insances.
The request with this ticket is that it supports Optional return types for collection entities if running with Java 8.
I know that Spring wants to remain backward compatible to pre-Java 8, so a pluggable solution is likely going to be necessary to resolve this.
Affects: 4.3.7
Comment From: spring-projects-issues
Juergen Hoeller commented
Generally speaking, we have plenty of java.util.Optional support across the framework at this point, e.g. for injection points and handler method arguments. We introduced this back in the 4.x line, conditionally kicking in when running on Java 8.
As for your scenario, there is a mismatch with two design conventions: A JavaBeans property needs to have the same type declared on the setter and the getter (otherwise java.beans.Introspector is not going to recognize is as a valid property), and Collection types are generally considered "monadic enough", i.e. not combined with java.util.Optional since they arguably imply optionality through the notion of an empty collection already.
Comment From: spring-projects-issues
Peter Luttrell commented
I understand the points about the JavaBeans convention, though in looking at the code I don't see where it would fail as a result of this difference. I also understand that Collections provide an additional way given that they could (and arguably) should be implemented never as null, but instead initialized as an empty collection.
I took a look at the code in org.springframework.beans.AbstractNestablePropertyAccessor and wasn't sure what harm there would be to simply unwrap an Optional in the protected Object getPropertyValue(PropertyTokenHolder tokens) method right after the null check and right before else if (value.getClass().isArray()) {. If this would work, it might add a bit of flexibility to the API.
Comment From: Bas83
I would really love if something could be done about this too. I realize that using Optional as a field here isn't a convention. But it is an extremely simple way to implement PATCH requests while keeping the type-safety of using objects.
For example with this DTO here:
@Data
@NoArgsConstructor
public class UISettingsPatchDTO {
private Optional<@NotNull ProjectTheme> theme;
private Optional<@Valid LogoDTO> logo;
private Optional<List<@Valid @NotNull SubjectReportColumnDTO>> subjectReportColumns;
}
it is fairly trivial to implement a PATCH request. Because when the optional field is null, it was never passed from the client. When the optional is not present, that means the client has set the value to null. It is the difference between
{
"theme" : "DARK",
"subjectReportColumns" : null
}
and
{
"theme" : "DARK",
}
which is a big difference in meaning.
If you want to implement PATCH in a different way, you'll quickly end up with having to use untyped maps of values.
Unfortunately right now validation will throw the error mentioned in this ticket.
Comment From: lpeter91
I'm almost in the same boot as @Bas83 except that I use a custom wrapper type instead of Optional.
So in more general terms it should be possible to somehow integrate Spring's Property Accessors with Bean Validation Value Extractors (both built-in and custom ones). I assume Property Accessors are completely oblivious to Bean Validation, so this might not be a trivial task.
Comment From: tukez
We have a similar problem with Vavr collections. Right now we have a very ugly workaround, but I wonder if there is currently an easier way to solve this. Definitely needs an improvement.
Workaround:
public class BaseEnableWebMvcConfig extends DelegatingWebMvcConfiguration {
@Override
protected RequestMappingHandlerAdapter createRequestMappingHandlerAdapter() {
return new RequestMappingHandlerAdapter() {
@Override
protected InitBinderDataBinderFactory createDataBinderFactory(List<InvocableHandlerMethod> binderMethods) throws Exception {
return new ServletRequestDataBinderFactory(binderMethods, getWebBindingInitializer()) {
@Override
protected ServletRequestDataBinder createBinderInstance(@Nullable Object target,
String objectName,
NativeWebRequest request) throws Exception {
return new ExtendedServletRequestDataBinder(target, objectName) {
@Override
protected AbstractPropertyBindingResult createDirectFieldBindingResult() {
var result = new VavrSupportingDirectFieldBindingResult(getTarget(),
getObjectName(),
isAutoGrowNestedPaths());
if (getConversionService() != null) {
result.initConversion(getConversionService());
}
if (getMessageCodesResolver() != null) {
result.setMessageCodesResolver(getMessageCodesResolver());
}
return result;
}
};
}
};
}
};
}
public static class VavrSupportingDirectFieldBindingResult extends DirectFieldBindingResult {
private static final long serialVersionUID = 1L;
public VavrSupportingDirectFieldBindingResult(Object target, String objectName, boolean autoGrowNestedPaths) {
super(target, objectName, autoGrowNestedPaths);
}
public VavrSupportingDirectFieldBindingResult(Object target, String objectName) {
super(target, objectName);
}
@Override
protected ConfigurablePropertyAccessor createDirectFieldAccessor() {
if (getTarget() == null) {
throw new IllegalStateException("Cannot access fields on null target instance '" + getObjectName() +
"'");
}
return new VavrSupportingDirectFieldAccessor(getTarget());
}
}
/**
* Adapted from {@link DirectFieldAccessor} to support convert Vavr collections to JDK collections because only they
* are supported in the base class ({@link AbstractNestablePropertyAccessor}) and the behaviour is hard to
* override/extend.
*/
private static class VavrSupportingDirectFieldAccessor extends AbstractNestablePropertyAccessor {
private final Map<String, FieldPropertyHandler> fieldMap = new HashMap<>();
public VavrSupportingDirectFieldAccessor(Object object) {
super(object);
}
protected VavrSupportingDirectFieldAccessor(Object object,
String nestedPath,
VavrSupportingDirectFieldAccessor parent) {
super(object, nestedPath, parent);
}
@Override
@Nullable
protected FieldPropertyHandler getLocalPropertyHandler(String propertyName) {
FieldPropertyHandler propertyHandler = this.fieldMap.get(propertyName);
if (propertyHandler == null) {
Field field = ReflectionUtils.findField(getWrappedClass(), propertyName);
if (field != null) {
propertyHandler = new FieldPropertyHandler(field);
this.fieldMap.put(propertyName, propertyHandler);
}
}
return propertyHandler;
}
@Override
protected VavrSupportingDirectFieldAccessor newNestedPropertyAccessor(Object object, String nestedPath) {
return new VavrSupportingDirectFieldAccessor(object, nestedPath, this);
}
@Override
protected NotWritablePropertyException createNotWritablePropertyException(String propertyName) {
PropertyMatches matches = PropertyMatches.forField(propertyName, getRootClass());
throw new NotWritablePropertyException(getRootClass(),
getNestedPath() + propertyName,
matches.buildErrorMessage(),
matches.getPossibleMatches());
}
private class FieldPropertyHandler extends PropertyHandler {
private final Field field;
public FieldPropertyHandler(Field field) {
super(field.getType(), true, true);
this.field = field;
}
@Override
public TypeDescriptor toTypeDescriptor() {
return new TypeDescriptor(this.field);
}
@Override
public ResolvableType getResolvableType() {
return ResolvableType.forField(this.field);
}
@Override
@Nullable
public TypeDescriptor nested(int level) {
return TypeDescriptor.nested(this.field, level);
}
@SuppressWarnings("rawtypes")
@Override
@Nullable
public Object getValue() throws Exception {
try {
ReflectionUtils.makeAccessible(this.field);
Object value = this.field.get(getWrappedInstance());
if (value instanceof io.vavr.collection.Seq) {
return ((io.vavr.collection.Seq) value).toJavaList();
}
if (value instanceof io.vavr.collection.Set) {
return ((io.vavr.collection.Set) value).toJavaSet();
}
if (value instanceof io.vavr.collection.Map) {
return ((io.vavr.collection.Map) value).toJavaMap();
}
return value;
} catch (IllegalAccessException ex) {
throw new InvalidPropertyException(getWrappedClass(),
this.field.getName(),
"Field is not accessible",
ex);
}
}
@Override
public void setValue(@Nullable Object value) throws Exception {
try {
ReflectionUtils.makeAccessible(this.field);
this.field.set(getWrappedInstance(), value);
} catch (IllegalAccessException ex) {
throw new InvalidPropertyException(getWrappedClass(),
this.field.getName(),
"Field is not accessible",
ex);
}
}
}
}
}