spring-data-commons icon indicating copy to clipboard operation
spring-data-commons copied to clipboard

Deserializing java.util.Optional throws exception [DATACMNS-1501]

Open spring-projects-issues opened this issue 6 years ago • 8 comments

Joerg opened DATACMNS-1501 and commented

After upgrading from SpringBoot 2.0.6 to SpringBoot 2.1.3 it's no longer possible deserialize data object that contain fields from type Optional.

Example:

public class TestDataWithOptional {
    private String id;
    private Optional<String> optionalString;
}

Serialization works fine (and looks same as in 2.0.6) - but when the data is read an exception is thrown:

org.springframework.web.util.NestedServletException: Request processing failed; nested exception is java.lang.UnsupportedOperationException: Cannot set immutable property java.util.Optional.value!

	at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:1013)
	at org.springframework.web.servlet.FrameworkServlet.doGet(FrameworkServlet.java:897)
	at javax.servlet.http.HttpServlet.service(HttpServlet.java:634)
	at org.springframework.web.servlet.FrameworkServlet.service(FrameworkServlet.java:882)
	at org.springframework.test.web.servlet.TestDispatcherServlet.service(TestDispatcherServlet.java:71)
	at javax.servlet.http.HttpServlet.service(HttpServlet.java:741)
	at org.springframework.mock.web.MockFilterChain$ServletFilterProxy.doFilter(MockFilterChain.java:166)
	at org.springframework.mock.web.MockFilterChain.doFilter(MockFilterChain.java:133)
	at org.springframework.test.web.servlet.MockMvc.perform(MockMvc.java:182)
	at com.example.mongorepo.optionalfails.OptionalFailsApplicationTests.testSaveAndGetWithOptional(OptionalFailsApplicationTests.java:49)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.springframework.test.context.junit4.statements.RunBeforeTestExecutionCallbacks.evaluate(RunBeforeTestExecutionCallbacks.java:74)
	at org.springframework.test.context.junit4.statements.RunAfterTestExecutionCallbacks.evaluate(RunAfterTestExecutionCallbacks.java:84)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.springframework.test.context.junit4.statements.RunBeforeTestMethodCallbacks.evaluate(RunBeforeTestMethodCallbacks.java:75)
	at org.springframework.test.context.junit4.statements.RunAfterTestMethodCallbacks.evaluate(RunAfterTestMethodCallbacks.java:86)
	at org.springframework.test.context.junit4.statements.SpringRepeat.evaluate(SpringRepeat.java:84)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
	at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.runChild(SpringJUnit4ClassRunner.java:251)
	at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.runChild(SpringJUnit4ClassRunner.java:97)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
	at org.springframework.test.context.junit4.statements.RunBeforeTestClassCallbacks.evaluate(RunBeforeTestClassCallbacks.java:61)
	at org.springframework.test.context.junit4.statements.RunAfterTestClassCallbacks.evaluate(RunAfterTestClassCallbacks.java:70)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
	at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.run(SpringJUnit4ClassRunner.java:190)
	at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
	at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:68)
	at com.intellij.rt.execution.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:47)
	at com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:242)
	at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:70)
Caused by: java.lang.UnsupportedOperationException: Cannot set immutable property java.util.Optional.value!
	at org.springframework.data.mapping.model.BeanWrapper.setProperty(BeanWrapper.java:86)
	at org.springframework.data.mapping.model.ConvertingPropertyAccessor.setProperty(ConvertingPropertyAccessor.java:61)
	at org.springframework.data.mongodb.core.convert.MappingMongoConverter.readProperties(MappingMongoConverter.java:378)
	at org.springframework.data.mongodb.core.convert.MappingMongoConverter.populateProperties(MappingMongoConverter.java:295)
	at org.springframework.data.mongodb.core.convert.MappingMongoConverter.read(MappingMongoConverter.java:275)
	at org.springframework.data.mongodb.core.convert.MappingMongoConverter.read(MappingMongoConverter.java:245)
	at org.springframework.data.mongodb.core.convert.MappingMongoConverter.readValue(MappingMongoConverter.java:1491)
	at org.springframework.data.mongodb.core.convert.MappingMongoConverter$MongoDbPropertyValueProvider.getPropertyValue(MappingMongoConverter.java:1389)
	at org.springframework.data.mongodb.core.convert.MappingMongoConverter$AssociationAwareMongoDbPropertyValueProvider.getPropertyValue(MappingMongoConverter.java:1438)
	at org.springframework.data.mongodb.core.convert.MappingMongoConverter$AssociationAwareMongoDbPropertyValueProvider.getPropertyValue(MappingMongoConverter.java:1401)
	at org.springframework.data.mapping.model.PersistentEntityParameterValueProvider.getParameterValue(PersistentEntityParameterValueProvider.java:71)
	at org.springframework.data.mapping.model.SpELExpressionParameterValueProvider.getParameterValue(SpELExpressionParameterValueProvider.java:49)
	at org.springframework.data.convert.ClassGeneratingEntityInstantiator$EntityInstantiatorAdapter.extractInvocationArguments(ClassGeneratingEntityInstantiator.java:250)
	at org.springframework.data.convert.ClassGeneratingEntityInstantiator$EntityInstantiatorAdapter.createInstance(ClassGeneratingEntityInstantiator.java:223)
	at org.springframework.data.convert.ClassGeneratingEntityInstantiator.createInstance(ClassGeneratingEntityInstantiator.java:84)
	at org.springframework.data.mongodb.core.convert.MappingMongoConverter.read(MappingMongoConverter.java:272)
	at org.springframework.data.mongodb.core.convert.MappingMongoConverter.read(MappingMongoConverter.java:245)
	at org.springframework.data.mongodb.core.convert.MappingMongoConverter.read(MappingMongoConverter.java:194)
	at org.springframework.data.mongodb.core.convert.MappingMongoConverter.read(MappingMongoConverter.java:190)
	at org.springframework.data.mongodb.core.convert.MappingMongoConverter.read(MappingMongoConverter.java:78)
	at org.springframework.data.mongodb.core.MongoTemplate$ReadDocumentCallback.doWith(MongoTemplate.java:3017)
	at org.springframework.data.mongodb.core.MongoTemplate.executeFindMultiInternal(MongoTemplate.java:2673)
	at org.springframework.data.mongodb.core.MongoTemplate.doFind(MongoTemplate.java:2404)
	at org.springframework.data.mongodb.core.MongoTemplate.doFind(MongoTemplate.java:2387)
	at org.springframework.data.mongodb.core.MongoTemplate.find(MongoTemplate.java:823)
	at org.springframework.data.mongodb.repository.support.SimpleMongoRepository.findAll(SimpleMongoRepository.java:360)
	at org.springframework.data.mongodb.repository.support.SimpleMongoRepository.findAll(SimpleMongoRepository.java:194)
	at org.springframework.data.mongodb.repository.support.SimpleMongoRepository.findAll(SimpleMongoRepository.java:51)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.springframework.data.repository.core.support.RepositoryComposition$RepositoryFragments.invoke(RepositoryComposition.java:359)
	at org.springframework.data.repository.core.support.RepositoryComposition.invoke(RepositoryComposition.java:200)
	at org.springframework.data.repository.core.support.RepositoryFactorySupport$ImplementationMethodExecutionInterceptor.invoke(RepositoryFactorySupport.java:644)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:186)
	at org.springframework.data.repository.core.support.RepositoryFactorySupport$QueryExecutorMethodInterceptor.doInvoke(RepositoryFactorySupport.java:608)
	at org.springframework.data.repository.core.support.RepositoryFactorySupport$QueryExecutorMethodInterceptor.lambda$invoke$3(RepositoryFactorySupport.java:595)
	at org.springframework.data.repository.core.support.RepositoryFactorySupport$QueryExecutorMethodInterceptor.invoke(RepositoryFactorySupport.java:595)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:186)
	at org.springframework.data.projection.DefaultMethodInvokingMethodInterceptor.invoke(DefaultMethodInvokingMethodInterceptor.java:59)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:186)
	at org.springframework.aop.interceptor.ExposeInvocationInterceptor.invoke(ExposeInvocationInterceptor.java:93)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:186)
	at org.springframework.data.repository.core.support.SurroundingTransactionDetectorMethodInterceptor.invoke(SurroundingTransactionDetectorMethodInterceptor.java:61)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:186)
	at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:212)
	at com.sun.proxy.$Proxy75.findAll(Unknown Source)
	at com.example.mongorepo.optionalfails.TestController.getAllWithOptional(TestController.java:30)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.springframework.web.method.support.InvocableHandlerMethod.doInvoke(InvocableHandlerMethod.java:189)
	at org.springframework.web.method.support.InvocableHandlerMethod.invokeForRequest(InvocableHandlerMethod.java:138)
	at org.springframework.web.servlet.mvc.method.annotation.ServletInvocableHandlerMethod.invokeAndHandle(ServletInvocableHandlerMethod.java:102)
	at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.invokeHandlerMethod(RequestMappingHandlerAdapter.java:895)
	at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.handleInternal(RequestMappingHandlerAdapter.java:800)
	at org.springframework.web.servlet.mvc.method.AbstractHandlerMethodAdapter.handle(AbstractHandlerMethodAdapter.java:87)
	at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:1038)
	at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:942)
	at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:1005)
	... 40 more

Seems like it first creates the object using new Optional() and tries to set the value afterwards (which is impossible for an immutable object).

I uploaded an example project to https://github.com/joemat/mongorepo-optional-fails

There are two branches:

  • master (uses SpringBoot 2.1.3) test fail
  • springboot-2.0.6 - test pass

Affects: 2.1.5 (Lovelace SR5)

Reference URL: https://github.com/joemat/mongorepo-optional-fails

spring-projects-issues avatar Mar 21 '19 14:03 spring-projects-issues

Joerg commented

See also this discussion on stackoverflow https://stackoverflow.com/questions/53395198/spring-common-data-2-1-2-mongodb-cant-deserialise-optional-value

spring-projects-issues avatar Mar 21 '19 14:03 spring-projects-issues

Mark Paluch commented

That's expected behavior in the first place. Optional has no public constructor that takes an argument and we do not write to immutable properties unless classes provide with…(…) methods or are Kotlin data classes

spring-projects-issues avatar Mar 21 '19 20:03 spring-projects-issues

Joerg commented

From a technical point of view I agree, the interface of Optional does not provide such a constructor and makes it therefore difficult for frameworks like spriong-data. On the other hand it's a class from the java.util package and widely used. One may argue that you shouldn't use Optional in data classes you want to persist, but if spring-data is able to persist it I expect that it is also able to read it

This issue currently blocks us from updating to spring-boot 2.1 - is there any workaround you suggest to make our code work again?

spring-projects-issues avatar Mar 22 '19 07:03 spring-projects-issues

Mark Paluch commented

There are a couple of aspect here:

  • We can serialize Optional but not deserialize it.
  • We can serialize a lot more types but not deserialize them because of mapping rules.
  • Using Optional (or other generic wrappers like Stream) isn't the best thing to do for properties. In fact, using Optional fields is discouraged as Optional should be a method return value.
  • You still might want to use Optional if e.g. your getters are generated which again raises the question whether that's the right thing to do.

We aren't convinced that Optional is an appropriate property type.

However, one might want to use a domain-specific value type in the entity model. E.g. you might want to represent an EmailAddress using a proper value type in your domain but persisting it as plain string value in your database without additional document nesting.

Properly modeling a value type with factory methods is a common pattern. We might be able to store a value type but not deserialize it. In the scope of discussing value types, we might want to consider an approach how to interact with value types and solving Optional deserialization is likely to be a by-product

spring-projects-issues avatar Mar 22 '19 08:03 spring-projects-issues

Joerg commented

Thanks for the explanation, I understand that there probably won't be a short term fix for that.

Is there any workaround for that? Is it possible to register an own mapper for Optionals?

spring-projects-issues avatar Mar 25 '19 08:03 spring-projects-issues

Mark Paluch commented

You can provide a Converter that converts e.g. Document to Optional<String>:

@ReadingConverter
enum MyConverter implements Converter<org.bson.Document, Optional<String>>{
	INSTANCE;
	@Override
	public Optional<String> convert(org.bson.Document document) {
		return Optional.ofNullable((String) document.get("value"));
	}
}

Please note that this converter applies on a property/value type-basis (database value type to property type) and will affect all entities

spring-projects-issues avatar Mar 25 '19 09:03 spring-projects-issues

Dennis Doubleday commented

Adding to this, I see the same error because my class is implementing org.springframework.data.domain.Auditable, so now I have to have Optional as part of the signature for 

Optional<T> getCreatedDate(); Optional<U> getLastModifiedBy(); Optional<T> getLastModifiedDate();

Shouldn't this work out of the box without a custom converter?

spring-projects-issues avatar Feb 25 '20 21:02 spring-projects-issues

I am also stuck with this issues, getting below exception when I try to read Optional value from document.

java.lang.UnsupportedOperationException: Cannot set immutable property java.util.Optional.value! at org.springframework.data.mapping.model.BeanWrapper.setProperty(BeanWrapper.java:87) at org.springframework.data.mapping.model.ConvertingPropertyAccessor.setProperty(ConvertingPropertyAccessor.java:63)

Is it possible to write converter for all Optional fields?

sundar-roc avatar Oct 08 '21 15:10 sundar-roc

We decided not to change the current behaviour regarding Optional. The type is intended to be used as a method return type and we do consider its usage for fields an anti pattern. There will also be no support for @ReadingConverter / @WritingConverter targeting Optional as those would violate the Converter API contract.

christophstrobl avatar Mar 07 '23 06:03 christophstrobl