From 5c1d3fca15d9c020a07a0130a3992926d7087f1a Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Thu, 7 Apr 2016 14:18:30 +0200 Subject: [PATCH] BeanFactory does not unwrap java.util.Optional for top-level bean Issue: SPR-14121 --- .../AbstractNestablePropertyAccessor.java | 41 ++++++------ .../beans/BeanWrapperImpl.java | 63 +++++++++++-------- .../factory/support/ConstructorResolver.java | 4 +- .../DefaultListableBeanFactoryTests.java | 22 ++++++- 4 files changed, 80 insertions(+), 50 deletions(-) diff --git a/spring-beans/src/main/java/org/springframework/beans/AbstractNestablePropertyAccessor.java b/spring-beans/src/main/java/org/springframework/beans/AbstractNestablePropertyAccessor.java index 12b304666c2..2f12def1e2c 100644 --- a/spring-beans/src/main/java/org/springframework/beans/AbstractNestablePropertyAccessor.java +++ b/spring-beans/src/main/java/org/springframework/beans/AbstractNestablePropertyAccessor.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 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. @@ -87,10 +87,10 @@ public abstract class AbstractNestablePropertyAccessor extends AbstractPropertyA } } + private int autoGrowCollectionLimit = Integer.MAX_VALUE; - /** The wrapped object */ - private Object object; + Object wrappedObject; private String nestedPath = ""; @@ -204,23 +204,23 @@ public abstract class AbstractNestablePropertyAccessor extends AbstractPropertyA public void setWrappedInstance(Object object, String nestedPath, Object rootObject) { Assert.notNull(object, "Target object must not be null"); if (object.getClass() == javaUtilOptionalClass) { - this.object = OptionalUnwrapper.unwrap(object); + this.wrappedObject = OptionalUnwrapper.unwrap(object); } else { - this.object = object; + this.wrappedObject = object; } this.nestedPath = (nestedPath != null ? nestedPath : ""); - this.rootObject = (!"".equals(this.nestedPath) ? rootObject : this.object); + this.rootObject = (!"".equals(this.nestedPath) ? rootObject : this.wrappedObject); this.nestedPropertyAccessors = null; - this.typeConverterDelegate = new TypeConverterDelegate(this, this.object); + this.typeConverterDelegate = new TypeConverterDelegate(this, this.wrappedObject); } public final Object getWrappedInstance() { - return this.object; + return this.wrappedObject; } public final Class getWrappedClass() { - return (this.object != null ? this.object.getClass() : null); + return (this.wrappedObject != null ? this.wrappedObject.getClass() : null); } /** @@ -303,7 +303,7 @@ public abstract class AbstractNestablePropertyAccessor extends AbstractPropertyA catch (NotReadablePropertyException ex) { throw new NotWritablePropertyException(getRootClass(), this.nestedPath + propertyName, "Cannot access indexed value in property referenced " + - "in indexed property path '" + propertyName + "'", ex); + "in indexed property path '" + propertyName + "'", ex); } // Set value for last key. String key = tokens.keys[tokens.keys.length - 1]; @@ -318,7 +318,7 @@ public abstract class AbstractNestablePropertyAccessor extends AbstractPropertyA else { throw new NullValueInNestedPathException(getRootClass(), this.nestedPath + propertyName, "Cannot access indexed value in property referenced " + - "in indexed property path '" + propertyName + "': returned null"); + "in indexed property path '" + propertyName + "': returned null"); } } if (propValue.getClass().isArray()) { @@ -367,8 +367,8 @@ public abstract class AbstractNestablePropertyAccessor extends AbstractPropertyA catch (NullPointerException ex) { throw new InvalidPropertyException(getRootClass(), this.nestedPath + propertyName, "Cannot set element with index " + index + " in List of size " + - size + ", accessed using property path '" + propertyName + - "': List does not support filling up gaps with null elements"); + size + ", accessed using property path '" + propertyName + + "': List does not support filling up gaps with null elements"); } } list.add(convertedValue); @@ -405,7 +405,7 @@ public abstract class AbstractNestablePropertyAccessor extends AbstractPropertyA else { throw new InvalidPropertyException(getRootClass(), this.nestedPath + propertyName, "Property referenced in indexed property path '" + propertyName + - "' is neither an array nor a List nor a Map; returned value was [" + propValue + "]"); + "' is neither an array nor a List nor a Map; returned value was [" + propValue + "]"); } } @@ -451,7 +451,7 @@ public abstract class AbstractNestablePropertyAccessor extends AbstractPropertyA } pv.getOriginalPropertyValue().conversionNecessary = (valueToApply != originalValue); } - ph.setValue(object, valueToApply); + ph.setValue(this.wrappedObject, valueToApply); } catch (TypeMismatchException ex) { throw ex; @@ -953,10 +953,9 @@ public abstract class AbstractNestablePropertyAccessor extends AbstractPropertyA tokens.actualName = (actualName != null ? actualName : propertyName); tokens.canonicalName = tokens.actualName; if (!keys.isEmpty()) { - tokens.canonicalName += - PROPERTY_KEY_PREFIX + - StringUtils.collectionToDelimitedString(keys, PROPERTY_KEY_SUFFIX + PROPERTY_KEY_PREFIX) + - PROPERTY_KEY_SUFFIX; + tokens.canonicalName += PROPERTY_KEY_PREFIX + + StringUtils.collectionToDelimitedString(keys, PROPERTY_KEY_SUFFIX + PROPERTY_KEY_PREFIX) + + PROPERTY_KEY_SUFFIX; tokens.keys = StringUtils.toStringArray(keys); } return tokens; @@ -965,8 +964,8 @@ public abstract class AbstractNestablePropertyAccessor extends AbstractPropertyA @Override public String toString() { StringBuilder sb = new StringBuilder(getClass().getName()); - if (this.object != null) { - sb.append(": wrapping object [").append(ObjectUtils.identityToString(this.object)).append("]"); + if (this.wrappedObject != null) { + sb.append(": wrapping object [").append(ObjectUtils.identityToString(this.wrappedObject)).append("]"); } else { sb.append(": no wrapped object set"); diff --git a/spring-beans/src/main/java/org/springframework/beans/BeanWrapperImpl.java b/spring-beans/src/main/java/org/springframework/beans/BeanWrapperImpl.java index 6625cc1aa46..e297d85fb8e 100644 --- a/spring-beans/src/main/java/org/springframework/beans/BeanWrapperImpl.java +++ b/spring-beans/src/main/java/org/springframework/beans/BeanWrapperImpl.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 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. @@ -133,10 +133,45 @@ public class BeanWrapperImpl extends AbstractNestablePropertyAccessor implements } + /** + * Set a bean instance to hold, without any unwrapping of {@link java.util.Optional}. + * @param object the actual target object + * @since 4.3 + * @see #setWrappedInstance(Object) + */ + public void setBeanInstance(Object object) { + this.wrappedObject = object; + this.typeConverterDelegate = new TypeConverterDelegate(this, this.wrappedObject); + setIntrospectionClass(object.getClass()); + } + @Override public void setWrappedInstance(Object object, String nestedPath, Object rootObject) { super.setWrappedInstance(object, nestedPath, rootObject); - setIntrospectionClass(getWrappedInstance().getClass()); + setIntrospectionClass(getWrappedClass()); + } + + /** + * Set the class to introspect. + * Needs to be called when the target object changes. + * @param clazz the class to introspect + */ + protected void setIntrospectionClass(Class clazz) { + if (this.cachedIntrospectionResults != null && this.cachedIntrospectionResults.getBeanClass() != clazz) { + this.cachedIntrospectionResults = null; + } + } + + /** + * Obtain a lazily initializted CachedIntrospectionResults instance + * for the wrapped object. + */ + private CachedIntrospectionResults getCachedIntrospectionResults() { + Assert.state(getWrappedInstance() != null, "BeanWrapper does not hold a bean instance"); + if (this.cachedIntrospectionResults == null) { + this.cachedIntrospectionResults = CachedIntrospectionResults.forClass(getWrappedClass()); + } + return this.cachedIntrospectionResults; } /** @@ -155,30 +190,6 @@ public class BeanWrapperImpl extends AbstractNestablePropertyAccessor implements return this.acc; } - /** - * Set the class to introspect. - * Needs to be called when the target object changes. - * @param clazz the class to introspect - */ - protected void setIntrospectionClass(Class clazz) { - if (this.cachedIntrospectionResults != null && - !clazz.equals(this.cachedIntrospectionResults.getBeanClass())) { - this.cachedIntrospectionResults = null; - } - } - - /** - * Obtain a lazily initializted CachedIntrospectionResults instance - * for the wrapped object. - */ - private CachedIntrospectionResults getCachedIntrospectionResults() { - Assert.state(getWrappedInstance() != null, "BeanWrapper does not hold a bean instance"); - if (this.cachedIntrospectionResults == null) { - this.cachedIntrospectionResults = CachedIntrospectionResults.forClass(getWrappedClass()); - } - return this.cachedIntrospectionResults; - } - /** * Convert the given value for the specified property to the latter's type. diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/ConstructorResolver.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/ConstructorResolver.java index 00437ae86b9..157bb1cd9e8 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/ConstructorResolver.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/ConstructorResolver.java @@ -272,7 +272,7 @@ class ConstructorResolver { mbd, beanName, this.beanFactory, constructorToUse, argsToUse); } - bw.setWrappedInstance(beanInstance); + bw.setBeanInstance(beanInstance); return bw; } catch (Throwable ex) { @@ -592,7 +592,7 @@ class ConstructorResolver { if (beanInstance == null) { return null; } - bw.setWrappedInstance(beanInstance); + bw.setBeanInstance(beanInstance); return bw; } catch (Throwable ex) { diff --git a/spring-beans/src/test/java/org/springframework/beans/factory/DefaultListableBeanFactoryTests.java b/spring-beans/src/test/java/org/springframework/beans/factory/DefaultListableBeanFactoryTests.java index cbf2f37b929..00e41497aae 100644 --- a/spring-beans/src/test/java/org/springframework/beans/factory/DefaultListableBeanFactoryTests.java +++ b/spring-beans/src/test/java/org/springframework/beans/factory/DefaultListableBeanFactoryTests.java @@ -31,6 +31,7 @@ import java.util.Iterator; import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Optional; import java.util.Properties; import java.util.Set; import java.util.concurrent.Callable; @@ -2711,7 +2712,7 @@ public class DefaultListableBeanFactoryTests { } @Test - public void resolveEmbeddedValue() throws Exception { + public void resolveEmbeddedValue() { DefaultListableBeanFactory bf = new DefaultListableBeanFactory(); StringValueResolver r1 = mock(StringValueResolver.class); StringValueResolver r2 = mock(StringValueResolver.class); @@ -2730,6 +2731,25 @@ public class DefaultListableBeanFactoryTests { verify(r3, never()).resolveStringValue(isNull(String.class)); } + @Test + public void populatedJavaUtilOptionalBean() { + DefaultListableBeanFactory bf = new DefaultListableBeanFactory(); + RootBeanDefinition bd = new RootBeanDefinition(Optional.class); + bd.setFactoryMethodName("of"); + bd.getConstructorArgumentValues().addGenericArgumentValue("CONTENT"); + bf.registerBeanDefinition("optionalBean", bd); + assertEquals(Optional.of("CONTENT"), bf.getBean(Optional.class)); + } + + @Test + public void emptyJavaUtilOptionalBean() { + DefaultListableBeanFactory bf = new DefaultListableBeanFactory(); + RootBeanDefinition bd = new RootBeanDefinition(Optional.class); + bd.setFactoryMethodName("empty"); + bf.registerBeanDefinition("optionalBean", bd); + assertSame(Optional.empty(), bf.getBean(Optional.class)); + } + /** * Test that by-type bean lookup caching is working effectively by searching for a * bean of type B 10K times within a container having 1K additional beans of type A.