Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Data binding with java.util.Optional: traversal of nested paths, detection of empty holders [SPR-12241] #16855

Closed
spring-projects-issues opened this issue Sep 23, 2014 · 21 comments
Assignees
Labels
type: enhancement A general enhancement
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Sep 23, 2014

Federico Donnarumma opened SPR-12241 and commented

Hi, I have this case, when validating with JSR-303.

I Have an object which contains an Optional, when SpringValidatorAdapter cycles through properties to show something of the style

"you entered 0 but field nights should be higher than 0"

BeanWrapper can't access the object inside the Optional and I get this message:

java.lang.IllegalStateException: JSR-303 validated property 'earlyCancellation.deadlines[0].penalty.nights' does not have a corresponding accessor for Spring data binding - check your DataBinder's configuration (bean property versus direct field access)
	at org.springframework.validation.beanvalidation.SpringValidatorAdapter.processConstraintViolations(SpringValidatorAdapter.java:158) ~[spring-context-4.1.0.RELEASE.jar:4.1.0.RELEASE]
	at org.springframework.validation.beanvalidation.SpringValidatorAdapter.validate(SpringValidatorAdapter.java:107) ~[spring-context-4.1.0.RELEASE.jar:4.1.0.RELEASE]
	at org.springframework.validation.DataBinder.validate(DataBinder.java:760) ~[spring-context-4.1.0.RELEASE.jar:4.1.0.RELEASE]
	at org.springframework.web.servlet.mvc.method.annotation.RequestResponseBodyMethodProcessor.validate(RequestResponseBodyMethodProcessor.java:123) ~[spring-webmvc-4.1.0.RELEASE.jar:4.1.0.RELEASE]
	at org.springframework.web.servlet.mvc.method.annotation.RequestResponseBodyMethodProcessor.resolveArgument(RequestResponseBodyMethodProcessor.java:109) ~[spring-webmvc-4.1.0.RELEASE.jar:4.1.0.RELEASE]
	at org.springframework.web.method.support.HandlerMethodArgumentResolverComposite.resolveArgument(HandlerMethodArgumentResolverComposite.java:79) ~[spring-web-4.1.0.RELEASE.jar:4.1.0.RELEASE]
	at org.springframework.web.method.support.InvocableHandlerMethod.getMethodArgumentValues(InvocableHandlerMethod.java:157) ~[spring-web-4.1.0.RELEASE.jar:4.1.0.RELEASE]
	at org.springframework.web.method.support.InvocableHandlerMethod.invokeForRequest(InvocableHandlerMethod.java:124) ~[spring-web-4.1.0.RELEASE.jar:4.1.0.RELEASE]
	at org.springframework.web.servlet.mvc.method.annotation.ServletInvocableHandlerMethod.invokeAndHandle(ServletInvocableHandlerMethod.java:104) ~[spring-webmvc-4.1.0.RELEASE.jar:4.1.0.RELEASE]
	at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.invokeHandleMethod(RequestMappingHandlerAdapter.java:781) ~[spring-webmvc-4.1.0.RELEASE.jar:4.1.0.RELEASE]
	at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.handleInternal(RequestMappingHandlerAdapter.java:721) ~[spring-webmvc-4.1.0.RELEASE.jar:4.1.0.RELEASE]
	at org.springframework.web.servlet.mvc.method.AbstractHandlerMethodAdapter.handle(AbstractHandlerMethodAdapter.java:83) ~[spring-webmvc-4.1.0.RELEASE.jar:4.1.0.RELEASE]
	at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:943) ~[spring-webmvc-4.1.0.RELEASE.jar:4.1.0.RELEASE]
	at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:877) ~[spring-webmvc-4.1.0.RELEASE.jar:4.1.0.RELEASE]
	at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:961) [spring-webmvc-4.1.0.RELEASE.jar:4.1.0.RELEASE]
	at org.springframework.web.servlet.FrameworkServlet.doPost(FrameworkServlet.java:863) [spring-webmvc-4.1.0.RELEASE.jar:4.1.0.RELEASE]
	at javax.servlet.http.HttpServlet.service(HttpServlet.java:707) ~[javax.servlet-api-3.1.0.jar:3.1.0]
	at org.springframework.web.servlet.FrameworkServlet.service(FrameworkServlet.java:837) [spring-webmvc-4.1.0.RELEASE.jar:4.1.0.RELEASE]
	at org.springframework.test.web.servlet.TestDispatcherServlet.service(TestDispatcherServlet.java:62) [spring-test-4.1.0.RELEASE.jar:4.1.0.RELEASE]
	at javax.servlet.http.HttpServlet.service(HttpServlet.java:790) ~[javax.servlet-api-3.1.0.jar:3.1.0]
	at org.springframework.mock.web.MockFilterChain$ServletFilterProxy.doFilter(MockFilterChain.java:170) [spring-test-4.1.0.RELEASE.jar:4.1.0.RELEASE]
	at org.springframework.mock.web.MockFilterChain.doFilter(MockFilterChain.java:137) [spring-test-4.1.0.RELEASE.jar:4.1.0.RELEASE]
	at org.springframework.test.web.servlet.MockMvc.perform(MockMvc.java:145) [spring-test-4.1.0.RELEASE.jar:4.1.0.RELEASE]
	at com.despegar.bookedia.cancelpolicy.CustomCancelPolicyControllerTest.post_zeroEarlyCancellationNights_okStatus(CustomCancelPolicyControllerTest.java:357) [test-classes/:na]
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[na:1.8.0_05]
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) ~[na:1.8.0_05]
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[na:1.8.0_05]
	at java.lang.reflect.Method.invoke(Method.java:483) ~[na:1.8.0_05]
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:47) [junit-4.11.jar:na]
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12) [junit-4.11.jar:na]
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:44) [junit-4.11.jar:na]
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17) [junit-4.11.jar:na]
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26) [junit-4.11.jar:na]
	at org.springframework.test.context.junit4.statements.RunBeforeTestMethodCallbacks.evaluate(RunBeforeTestMethodCallbacks.java:72) [spring-test-4.1.0.RELEASE.jar:4.1.0.RELEASE]
	at org.springframework.test.context.junit4.statements.RunAfterTestMethodCallbacks.evaluate(RunAfterTestMethodCallbacks.java:81) [spring-test-4.1.0.RELEASE.jar:4.1.0.RELEASE]
	at org.springframework.test.context.junit4.statements.SpringRepeat.evaluate(SpringRepeat.java:72) [spring-test-4.1.0.RELEASE.jar:4.1.0.RELEASE]
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:271) [junit-4.11.jar:na]
	at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.runChild(SpringJUnit4ClassRunner.java:216) [spring-test-4.1.0.RELEASE.jar:4.1.0.RELEASE]
	at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.runChild(SpringJUnit4ClassRunner.java:82) [spring-test-4.1.0.RELEASE.jar:4.1.0.RELEASE]
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:238) [junit-4.11.jar:na]
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:63) [junit-4.11.jar:na]
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:236) [junit-4.11.jar:na]
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:53) [junit-4.11.jar:na]
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:229) [junit-4.11.jar:na]
	at org.springframework.test.context.junit4.statements.RunBeforeTestClassCallbacks.evaluate(RunBeforeTestClassCallbacks.java:60) [spring-test-4.1.0.RELEASE.jar:4.1.0.RELEASE]
	at org.springframework.test.context.junit4.statements.RunAfterTestClassCallbacks.evaluate(RunAfterTestClassCallbacks.java:67) [spring-test-4.1.0.RELEASE.jar:4.1.0.RELEASE]
	at org.junit.runners.ParentRunner.run(ParentRunner.java:309) [junit-4.11.jar:na]
	at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.run(SpringJUnit4ClassRunner.java:162) [spring-test-4.1.0.RELEASE.jar:4.1.0.RELEASE]
	at org.junit.runner.JUnitCore.run(JUnitCore.java:160) [junit-4.11.jar:na]
	at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:74) [junit-rt.jar:na]
	at com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:211) [junit-rt.jar:na]
	at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:67) [junit-rt.jar:na]
Caused by: org.springframework.beans.NotReadablePropertyException: Invalid property 'earlyCancellation.deadlines[0]' of bean class [com.despegar.bookedia.cancelpolicy.CancelPolicyDTO]: Bean property 'earlyCancellation.deadlines[0]' is not readable or has an invalid getter method: Does the return type of the getter match the parameter type of the setter?
	at org.springframework.beans.BeanWrapperImpl.getPropertyValue(BeanWrapperImpl.java:705) ~[spring-beans-4.1.0.RELEASE.jar:4.1.0.RELEASE]
	at org.springframework.beans.BeanWrapperImpl.getNestedBeanWrapper(BeanWrapperImpl.java:551) ~[spring-beans-4.1.0.RELEASE.jar:4.1.0.RELEASE]
	at org.springframework.beans.BeanWrapperImpl.getBeanWrapperForPropertyPath(BeanWrapperImpl.java:528) ~[spring-beans-4.1.0.RELEASE.jar:4.1.0.RELEASE]
	at org.springframework.beans.BeanWrapperImpl.getBeanWrapperForPropertyPath(BeanWrapperImpl.java:529) ~[spring-beans-4.1.0.RELEASE.jar:4.1.0.RELEASE]
	at org.springframework.beans.BeanWrapperImpl.getPropertyValue(BeanWrapperImpl.java:694) ~[spring-beans-4.1.0.RELEASE.jar:4.1.0.RELEASE]
	at org.springframework.validation.AbstractPropertyBindingResult.getActualFieldValue(AbstractPropertyBindingResult.java:99) ~[spring-context-4.1.0.RELEASE.jar:4.1.0.RELEASE]
	at org.springframework.validation.AbstractBindingResult.getRawFieldValue(AbstractBindingResult.java:283) ~[spring-context-4.1.0.RELEASE.jar:4.1.0.RELEASE]
	at org.springframework.validation.beanvalidation.SpringValidatorAdapter.processConstraintViolations(SpringValidatorAdapter.java:143) ~[spring-context-4.1.0.RELEASE.jar:4.1.0.RELEASE]
	... 51 common frames omitted

Affects: 4.1 GA

Issue Links:

Referenced from: commits cfc821d, 0934751

@spring-projects-issues
Copy link
Collaborator Author

Federico Donnarumma commented

Do you have any workaround to fix this in the meantime?

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

Hibernate Validator itself doesn't seem to support Optional yet... Could you show what your code looks like, i.e. what exactly is declared as Optional? I suppose just a getter, not the field?

Juergen

@spring-projects-issues
Copy link
Collaborator Author

Federico Donnarumma commented

Hi Jurgen, as soon as I get back to the office I'll send you te code, but. I can say that getter and field are both Optional.

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

Fixed for Spring Framework 4.1.1 now, with BeanWrapper supporting the traversal of nested paths with Java 8 Optional declarations. Note that this only works for Spring data binding, i.e. getter methods for value access. We can't do anything about Hibernate Validator's lack of support for Optional fields; this is planned for Hibernate Validator 5.2 on their side.

Juergen

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

Federico, please give the upcoming 4.1.1 snapshot a try and let me know whether the degree of Optional data binding support in there works for you. If for some reason the Optional field declaration works for your use of Hibernate Validator as well, then all the power to you!

Juergen

@spring-projects-issues
Copy link
Collaborator Author

Federico Donnarumma commented

Thanks Juergen, I'll give it a try ASAP, in the meantime I'll add a bug regarding TransactionContextHolder and TransactionalTestExecutionListener

@spring-projects-issues
Copy link
Collaborator Author

Federico Donnarumma commented

Thanks Juergen, now it's working as expected. You are the man :)

@spring-projects-issues
Copy link
Collaborator Author

Federico Donnarumma commented

Ok, not working for all cases, regression failed.

Seems to fail when you use Optional.emtpy

java.lang.IllegalArgumentException: Optional value must be present
	at org.springframework.util.Assert.isTrue(Assert.java:65) ~[spring-core-4.1.1.BUILD-20140924.155845-31.jar:4.1.1.BUILD-SNAPSHOT]
	at org.springframework.beans.BeanWrapperImpl$OptionalUnwrapper.unwrap(BeanWrapperImpl.java:1214) ~[spring-beans-4.1.1.BUILD-20140924.155845-31.jar:4.1.1.BUILD-SNAPSHOT]

@spring-projects-issues
Copy link
Collaborator Author

Federico Donnarumma commented

Not working for Optional.empty

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

What are the next few lines in the stacktrace there?

We can't bind against an empty Optional object, so we're treating it like a null value... rejecting it where a null value would get rejected as well. Are we getting that wrong somewhere?

Juergen

@spring-projects-issues
Copy link
Collaborator Author

Federico Donnarumma commented

The assert seems to be wrong. An empty optional is still an Optional

Optional<?> optional = (Optional<?>) optionalObject;
Assert.isTrue(optional.isPresent(), "Optional value must be present");

Full stack.

java.lang.IllegalArgumentException: Optional value must be present
	at org.springframework.util.Assert.isTrue(Assert.java:65) ~[spring-core-4.1.1.BUILD-20140924.155845-31.jar:4.1.1.BUILD-SNAPSHOT]
	at org.springframework.beans.BeanWrapperImpl$OptionalUnwrapper.unwrap(BeanWrapperImpl.java:1214) ~[spring-beans-4.1.1.BUILD-20140924.155845-31.jar:4.1.1.BUILD-SNAPSHOT]
	at org.springframework.beans.BeanWrapperImpl.setWrappedInstance(BeanWrapperImpl.java:228) ~[spring-beans-4.1.1.BUILD-20140924.155845-31.jar:4.1.1.BUILD-SNAPSHOT]
	at org.springframework.beans.BeanWrapperImpl.setWrappedInstance(BeanWrapperImpl.java:215) ~[spring-beans-4.1.1.BUILD-20140924.155845-31.jar:4.1.1.BUILD-SNAPSHOT]
	at org.springframework.beans.BeanWrapperImpl.<init>(BeanWrapperImpl.java:164) ~[spring-beans-4.1.1.BUILD-20140924.155845-31.jar:4.1.1.BUILD-SNAPSHOT]
	at org.springframework.beans.PropertyAccessorFactory.forBeanPropertyAccess(PropertyAccessorFactory.java:37) ~[spring-beans-4.1.1.BUILD-20140924.155845-31.jar:4.1.1.BUILD-SNAPSHOT]
	at org.springframework.validation.BeanPropertyBindingResult.createBeanWrapper(BeanPropertyBindingResult.java:106) ~[spring-context-4.1.1.BUILD-20140924.155845-31.jar:4.1.1.BUILD-SNAPSHOT]
	at org.springframework.validation.BeanPropertyBindingResult.getPropertyAccessor(BeanPropertyBindingResult.java:92) ~[spring-context-4.1.1.BUILD-20140924.155845-31.jar:4.1.1.BUILD-SNAPSHOT]
	at org.springframework.validation.AbstractPropertyBindingResult.initConversion(AbstractPropertyBindingResult.java:62) ~[spring-context-4.1.1.BUILD-20140924.155845-31.jar:4.1.1.BUILD-SNAPSHOT]
	at org.springframework.validation.DataBinder.initBeanPropertyAccess(DataBinder.java:237) ~[spring-context-4.1.1.BUILD-20140924.155845-31.jar:4.1.1.BUILD-SNAPSHOT]
	at org.springframework.validation.DataBinder.getInternalBindingResult(DataBinder.java:261) ~[spring-context-4.1.1.BUILD-20140924.155845-31.jar:4.1.1.BUILD-SNAPSHOT]
	at org.springframework.validation.DataBinder.getPropertyAccessor(DataBinder.java:270) ~[spring-context-4.1.1.BUILD-20140924.155845-31.jar:4.1.1.BUILD-SNAPSHOT]
	at org.springframework.validation.DataBinder.applyPropertyValues(DataBinder.java:728) ~[spring-context-4.1.1.BUILD-20140924.155845-31.jar:4.1.1.BUILD-SNAPSHOT]
	at org.springframework.validation.DataBinder.doBind(DataBinder.java:624) ~[spring-context-4.1.1.BUILD-20140924.155845-31.jar:4.1.1.BUILD-SNAPSHOT]
	at org.springframework.web.bind.WebDataBinder.doBind(WebDataBinder.java:189) ~[spring-web-4.1.1.BUILD-20140924.155845-31.jar:4.1.1.BUILD-SNAPSHOT]
	at org.springframework.web.bind.ServletRequestDataBinder.bind(ServletRequestDataBinder.java:106) ~[spring-web-4.1.1.BUILD-20140924.155845-31.jar:4.1.1.BUILD-SNAPSHOT]
	at org.springframework.web.servlet.mvc.method.annotation.ServletModelAttributeMethodProcessor.bindRequestParameters(ServletModelAttributeMethodProcessor.java:153) ~[spring-webmvc-4.1.1.BUILD-20140924.155845-31.jar:4.1.1.BUILD-SNAPSHOT]
	at org.springframework.web.method.annotation.ModelAttributeMethodProcessor.resolveArgument(ModelAttributeMethodProcessor.java:108) ~[spring-web-4.1.1.BUILD-20140924.155845-31.jar:4.1.1.BUILD-SNAPSHOT]
	at org.springframework.web.method.support.HandlerMethodArgumentResolverComposite.resolveArgument(HandlerMethodArgumentResolverComposite.java:79) ~[spring-web-4.1.1.BUILD-20140924.155845-31.jar:4.1.1.BUILD-SNAPSHOT]
	at org.springframework.web.method.support.InvocableHandlerMethod.getMethodArgumentValues(InvocableHandlerMethod.java:157) ~[spring-web-4.1.1.BUILD-20140924.155845-31.jar:4.1.1.BUILD-SNAPSHOT]
	at org.springframework.web.method.support.InvocableHandlerMethod.invokeForRequest(InvocableHandlerMethod.java:124) ~[spring-web-4.1.1.BUILD-20140924.155845-31.jar:4.1.1.BUILD-SNAPSHOT]
	at org.springframework.web.servlet.mvc.method.annotation.ServletInvocableHandlerMethod.invokeAndHandle(ServletInvocableHandlerMethod.java:104) ~[spring-webmvc-4.1.1.BUILD-20140924.155845-31.jar:4.1.1.BUILD-SNAPSHOT]
	at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.invokeHandleMethod(RequestMappingHandlerAdapter.java:781) ~[spring-webmvc-4.1.1.BUILD-20140924.155845-31.jar:4.1.1.BUILD-SNAPSHOT]
	at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.handleInternal(RequestMappingHandlerAdapter.java:721) ~[spring-webmvc-4.1.1.BUILD-20140924.155845-31.jar:4.1.1.BUILD-SNAPSHOT]
	at org.springframework.web.servlet.mvc.method.AbstractHandlerMethodAdapter.handle(AbstractHandlerMethodAdapter.java:83) ~[spring-webmvc-4.1.1.BUILD-20140924.155845-31.jar:4.1.1.BUILD-SNAPSHOT]
	at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:943) ~[spring-webmvc-4.1.1.BUILD-20140924.155845-31.jar:4.1.1.BUILD-SNAPSHOT]
	at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:877) ~[spring-webmvc-4.1.1.BUILD-20140924.155845-31.jar:4.1.1.BUILD-SNAPSHOT]
	at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:966) [spring-webmvc-4.1.1.BUILD-20140924.155845-31.jar:4.1.1.BUILD-SNAPSHOT]
	at org.springframework.web.servlet.FrameworkServlet.doGet(FrameworkServlet.java:857) [spring-webmvc-4.1.1.BUILD-20140924.155845-31.jar:4.1.1.BUILD-SNAPSHOT]
	at javax.servlet.http.HttpServlet.service(HttpServlet.java:687) ~[javax.servlet-api-3.1.0.jar:3.1.0]
	at org.springframework.web.servlet.FrameworkServlet.service(FrameworkServlet.java:842) [spring-webmvc-4.1.1.BUILD-20140924.155845-31.jar:4.1.1.BUILD-SNAPSHOT]
	at org.springframework.test.web.servlet.TestDispatcherServlet.service(TestDispatcherServlet.java:62) [spring-test-4.1.1.BUILD-20140924.155845-31.jar:4.1.1.BUILD-SNAPSHOT]
	at javax.servlet.http.HttpServlet.service(HttpServlet.java:790) ~[javax.servlet-api-3.1.0.jar:3.1.0]
	at org.springframework.mock.web.MockFilterChain$ServletFilterProxy.doFilter(MockFilterChain.java:170) [spring-test-4.1.1.BUILD-20140924.155845-31.jar:4.1.1.BUILD-SNAPSHOT]
	at org.springframework.mock.web.MockFilterChain.doFilter(MockFilterChain.java:137) [spring-test-4.1.1.BUILD-20140924.155845-31.jar:4.1.1.BUILD-SNAPSHOT]
	at org.springframework.test.web.servlet.MockMvc.perform(MockMvc.java:145) [spring-test-4.1.1.BUILD-20140924.155845-31.jar:4.1.1.BUILD-SNAPSHOT]
	at com.despegar.bookedia.cancelpolicy.CustomCancelPolicyControllerTest.get_sortedListByName_okStatus(CustomCancelPolicyControllerTest.java:150) [test-classes/:na]
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[na:1.8.0_05]
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) ~[na:1.8.0_05]
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[na:1.8.0_05]
	at java.lang.reflect.Method.invoke(Method.java:483) ~[na:1.8.0_05]
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:47) [junit-4.11.jar:na]
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12) [junit-4.11.jar:na]
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:44) [junit-4.11.jar:na]
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17) [junit-4.11.jar:na]
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26) [junit-4.11.jar:na]
	at org.springframework.test.context.junit4.statements.RunBeforeTestMethodCallbacks.evaluate(RunBeforeTestMethodCallbacks.java:72) [spring-test-4.1.1.BUILD-20140924.155845-31.jar:4.1.1.BUILD-SNAPSHOT]
	at org.springframework.test.context.junit4.statements.RunAfterTestMethodCallbacks.evaluate(RunAfterTestMethodCallbacks.java:81) [spring-test-4.1.1.BUILD-20140924.155845-31.jar:4.1.1.BUILD-SNAPSHOT]
	at org.springframework.test.context.junit4.statements.SpringRepeat.evaluate(SpringRepeat.java:72) [spring-test-4.1.1.BUILD-20140924.155845-31.jar:4.1.1.BUILD-SNAPSHOT]
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:271) [junit-4.11.jar:na]
	at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.runChild(SpringJUnit4ClassRunner.java:216) [spring-test-4.1.1.BUILD-20140924.155845-31.jar:4.1.1.BUILD-SNAPSHOT]
	at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.runChild(SpringJUnit4ClassRunner.java:82) [spring-test-4.1.1.BUILD-20140924.155845-31.jar:4.1.1.BUILD-SNAPSHOT]
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:238) [junit-4.11.jar:na]
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:63) [junit-4.11.jar:na]
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:236) [junit-4.11.jar:na]
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:53) [junit-4.11.jar:na]
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:229) [junit-4.11.jar:na]
	at org.springframework.test.context.junit4.statements.RunBeforeTestClassCallbacks.evaluate(RunBeforeTestClassCallbacks.java:60) [spring-test-4.1.1.BUILD-20140924.155845-31.jar:4.1.1.BUILD-SNAPSHOT]
	at org.springframework.test.context.junit4.statements.RunAfterTestClassCallbacks.evaluate(RunAfterTestClassCallbacks.java:67) [spring-test-4.1.1.BUILD-20140924.155845-31.jar:4.1.1.BUILD-SNAPSHOT]
	at org.junit.runners.ParentRunner.run(ParentRunner.java:309) [junit-4.11.jar:na]
	at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.run(SpringJUnit4ClassRunner.java:162) [spring-test-4.1.1.BUILD-20140924.155845-31.jar:4.1.1.BUILD-SNAPSHOT]
	at org.junit.runner.JUnitCore.run(JUnitCore.java:160) [junit-4.11.jar:na]
	at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:74) [junit-rt.jar:na]
	at com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:211) [junit-rt.jar:na]
	at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:67) [junit-rt.jar:na]

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

I figured it'd be a top-level binding target like in that stacktrace... Note that we are rejecting a null value in such a scenario since we can't bind any properties against a null target; with the same reasoning, we're rejecting an empty Optional object as well.

So I'm wondering: What would you expect to happen in such a case? Any incoming property values could not get bound since there's no target object... Would you expect that to fail later - i.e. on actual binding of a property value, with possibly none specified anyway, therefore effectively skipping the binding step?

Juergen

@spring-projects-issues
Copy link
Collaborator Author

Federico Donnarumma commented

I don't get why it's failing. We are using a custom validator that validates the object inside the Optional so if the controller is mapping that to an empty optional that seems fine, can you clarify a little more?

Thanks in advance.

@spring-projects-issues
Copy link
Collaborator Author

Federico Donnarumma commented

what do you think about this?

public static Object unwrap(Object optionalObject) {
    Optional<?> optional = (Optional<?>) optionalObject;
        if(optional.isPresent()) {
            return optional.get();
        } else {
            return Optional.empty();
        }
}

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

Ah, that problem seems to be in ModelAttributeMethodProcessor which does a plain not-null check and ends up triggering the bind step that way... I guess we have to check for not-empty Optional objects there as well, i.e. at the DataBinder level.

In any case, an empty Optional object ending up as top-level object in BeanWrapperImpl is pointless, so the assertion there is correct from my perspective. It's the code that triggers the bind step which needs to defend itself against empty Optional objects; I'll do a revision of all such places in the codebase.

Juergen

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

Seems we posted at the same time!

Refining the assertion that way would allow for entering code paths in BeanWrapperImpl which are not meant to deal with that state. I'd rather keep the original assertion and make DataBinder more defensive, skipping the bind step altogether like we do for plain null references.

Juergen

@spring-projects-issues
Copy link
Collaborator Author

Federico Donnarumma commented

Great, thanks for your help. This Java 8 Optional thing is giving us a couple of headaches.

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

DataBinder unwraps Optional objects and allows for proper handling of Optional.empty() now.

Juergen

@spring-projects-issues
Copy link
Collaborator Author

Federico Donnarumma commented

Great, I'll give it a try with SNAPSHOT release.

@spring-projects-issues
Copy link
Collaborator Author

Federico Donnarumma commented

Works great, thanks Juergen. Do you have an estimated release date for Spring 4.1.1?

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

Cool, thanks for the immediate feedback! 4.1.1 is scheduled for next week (Sep 30). You can always check the JIRA roadmap - we usually keep the release targets up to date there.

Juergen

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants