From 0f49919d334b490849be71cf74e8f3b552b24ee9 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 14 Apr 2010 12:11:56 +0000 Subject: [PATCH] fixed constructor argument caching for prototypes with multiple constructor matches (SPR-7084) git-svn-id: https://src.springframework.org/svn/spring-framework/trunk@3244 50f2f4bb-b051-0410-bef5-90022cba6387 --- .../factory/support/ConstructorResolver.java | 58 +++++++++++-------- .../DefaultListableBeanFactoryTests.java | 17 +++++- 2 files changed, 49 insertions(+), 26 deletions(-) diff --git a/org.springframework.beans/src/main/java/org/springframework/beans/factory/support/ConstructorResolver.java b/org.springframework.beans/src/main/java/org/springframework/beans/factory/support/ConstructorResolver.java index 67f716604fe..be5ab01d5d7 100644 --- a/org.springframework.beans/src/main/java/org/springframework/beans/factory/support/ConstructorResolver.java +++ b/org.springframework.beans/src/main/java/org/springframework/beans/factory/support/ConstructorResolver.java @@ -16,8 +16,6 @@ package org.springframework.beans.factory.support; -import static org.springframework.util.StringUtils.collectionToCommaDelimitedString; - import java.beans.ConstructorProperties; import java.lang.reflect.Constructor; import java.lang.reflect.Member; @@ -44,9 +42,9 @@ import org.springframework.beans.factory.BeanCreationException; import org.springframework.beans.factory.BeanDefinitionStoreException; import org.springframework.beans.factory.UnsatisfiedDependencyException; import org.springframework.beans.factory.config.ConstructorArgumentValues; +import org.springframework.beans.factory.config.ConstructorArgumentValues.ValueHolder; import org.springframework.beans.factory.config.DependencyDescriptor; import org.springframework.beans.factory.config.TypedStringValue; -import org.springframework.beans.factory.config.ConstructorArgumentValues.ValueHolder; import org.springframework.core.GenericTypeResolver; import org.springframework.core.MethodParameter; import org.springframework.core.ParameterNameDiscoverer; @@ -112,6 +110,7 @@ class ConstructorResolver { this.beanFactory.initBeanWrapper(bw); Constructor constructorToUse = null; + ArgumentsHolder argsHolderToUse = null; Object[] argsToUse = null; if (explicitArgs != null) { @@ -179,7 +178,7 @@ class ConstructorResolver { "(hint: specify index/type/name arguments for simple parameters to avoid type ambiguities)"); } - ArgumentsHolder args; + ArgumentsHolder argsHolder; if (resolvedValues != null) { try { String[] paramNames = null; @@ -192,7 +191,7 @@ class ConstructorResolver { paramNames = pnd.getParameterNames(candidate); } } - args = createArgumentArray( + argsHolder = createArgumentArray( beanName, mbd, resolvedValues, bw, paramTypes, paramNames, candidate, autowiring); } catch (UnsatisfiedDependencyException ex) { @@ -223,15 +222,16 @@ class ConstructorResolver { if (paramTypes.length != explicitArgs.length) { continue; } - args = new ArgumentsHolder(explicitArgs); + argsHolder = new ArgumentsHolder(explicitArgs); } int typeDiffWeight = (mbd.isLenientConstructorResolution() ? - args.getTypeDifferenceWeight(paramTypes) : args.getAssignabilityWeight(paramTypes)); + argsHolder.getTypeDifferenceWeight(paramTypes) : argsHolder.getAssignabilityWeight(paramTypes)); // Choose this constructor if it represents the closest match. if (typeDiffWeight < minTypeDiffWeight) { constructorToUse = candidate; - argsToUse = args.arguments; + argsHolderToUse = argsHolder; + argsToUse = argsHolder.arguments; minTypeDiffWeight = typeDiffWeight; ambiguousConstructors = null; } @@ -257,6 +257,7 @@ class ConstructorResolver { if (explicitArgs == null) { mbd.resolvedConstructorOrFactoryMethod = constructorToUse; + argsHolderToUse.storeCache(mbd); } } @@ -274,8 +275,8 @@ class ConstructorResolver { }, beanFactory.getAccessControlContext()); } else { - beanInstance = beanFactory.getInstantiationStrategy().instantiate( - mbd, beanName, beanFactory, constructorToUse, argsToUse); + beanInstance = this.beanFactory.getInstantiationStrategy().instantiate( + mbd, beanName, this.beanFactory, constructorToUse, argsToUse); } bw.setWrappedInstance(beanInstance); @@ -365,6 +366,7 @@ class ConstructorResolver { } Method factoryMethodToUse = null; + ArgumentsHolder argsHolderToUse = null; Object[] argsToUse = null; if (explicitArgs != null) { @@ -436,7 +438,7 @@ class ConstructorResolver { Class[] paramTypes = candidate.getParameterTypes(); if (paramTypes.length >= minNrOfArgs) { - ArgumentsHolder args; + ArgumentsHolder argsHolder; if (resolvedValues != null) { // Resolved constructor arguments: type conversion and/or autowiring necessary. @@ -446,7 +448,7 @@ class ConstructorResolver { if (pnd != null) { paramNames = pnd.getParameterNames(candidate); } - args = createArgumentArray( + argsHolder = createArgumentArray( beanName, mbd, resolvedValues, bw, paramTypes, paramNames, candidate, autowiring); } catch (UnsatisfiedDependencyException ex) { @@ -478,15 +480,16 @@ class ConstructorResolver { if (paramTypes.length != explicitArgs.length) { continue; } - args = new ArgumentsHolder(explicitArgs); + argsHolder = new ArgumentsHolder(explicitArgs); } int typeDiffWeight = (mbd.isLenientConstructorResolution() ? - args.getTypeDifferenceWeight(paramTypes) : args.getAssignabilityWeight(paramTypes)); + argsHolder.getTypeDifferenceWeight(paramTypes) : argsHolder.getAssignabilityWeight(paramTypes)); // Choose this factory method if it represents the closest match. if (typeDiffWeight < minTypeDiffWeight) { factoryMethodToUse = candidate; - argsToUse = args.arguments; + argsHolderToUse = argsHolder; + argsToUse = argsHolder.arguments; minTypeDiffWeight = typeDiffWeight; ambiguousFactoryMethods = null; } @@ -536,6 +539,7 @@ class ConstructorResolver { if (explicitArgs == null) { mbd.resolvedConstructorOrFactoryMethod = factoryMethodToUse; + argsHolderToUse.storeCache(mbd); } } @@ -642,7 +646,6 @@ class ConstructorResolver { Set usedValueHolders = new HashSet(paramTypes.length); Set autowiredBeanNames = new LinkedHashSet(4); - boolean resolveNecessary = false; for (int paramIndex = 0; paramIndex < paramTypes.length; paramIndex++) { Class paramType = paramTypes[paramIndex]; @@ -679,7 +682,7 @@ class ConstructorResolver { args.preparedArguments[paramIndex] = convertedValue; } else { - resolveNecessary = true; + args.resolveNecessary = true; args.preparedArguments[paramIndex] = sourceValue; } } @@ -709,7 +712,7 @@ class ConstructorResolver { args.rawArguments[paramIndex] = autowiredArgument; args.arguments[paramIndex] = autowiredArgument; args.preparedArguments[paramIndex] = new AutowiredArgumentMarker(); - resolveNecessary = true; + args.resolveNecessary = true; } catch (BeansException ex) { throw new UnsatisfiedDependencyException( @@ -726,13 +729,6 @@ class ConstructorResolver { } } - if (resolveNecessary) { - mbd.preparedConstructorArguments = args.preparedArguments; - } - else { - mbd.resolvedConstructorArguments = args.arguments; - } - mbd.constructorArgumentsResolved = true; return args; } @@ -801,6 +797,8 @@ class ConstructorResolver { public Object preparedArguments[]; + public boolean resolveNecessary = false; + public ArgumentsHolder(int size) { this.rawArguments = new Object[size]; this.arguments = new Object[size]; @@ -836,6 +834,16 @@ class ConstructorResolver { } return Integer.MAX_VALUE - 1024; } + + public void storeCache(RootBeanDefinition mbd) { + if (this.resolveNecessary) { + mbd.preparedConstructorArguments = this.preparedArguments; + } + else { + mbd.resolvedConstructorArguments = this.arguments; + } + mbd.constructorArgumentsResolved = true; + } } diff --git a/org.springframework.beans/src/test/java/org/springframework/beans/factory/DefaultListableBeanFactoryTests.java b/org.springframework.beans/src/test/java/org/springframework/beans/factory/DefaultListableBeanFactoryTests.java index 6a1a4abf427..ab1b3647404 100644 --- a/org.springframework.beans/src/test/java/org/springframework/beans/factory/DefaultListableBeanFactoryTests.java +++ b/org.springframework.beans/src/test/java/org/springframework/beans/factory/DefaultListableBeanFactoryTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2008 the original author or authors. + * Copyright 2002-2010 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. @@ -59,6 +59,7 @@ import org.springframework.beans.factory.config.ConfigurableBeanFactory; import org.springframework.beans.factory.config.ConstructorArgumentValues; import org.springframework.beans.factory.config.InstantiationAwareBeanPostProcessorAdapter; import org.springframework.beans.factory.config.RuntimeBeanReference; +import org.springframework.beans.factory.config.TypedStringValue; import org.springframework.beans.factory.support.AbstractBeanDefinition; import org.springframework.beans.factory.support.AbstractBeanFactory; import org.springframework.beans.factory.support.BeanDefinitionBuilder; @@ -1577,6 +1578,20 @@ public final class DefaultListableBeanFactoryTests { lbf.preInstantiateSingletons(); } + @Test + public void testPrototypeStringCreatedRepeatedly() { + DefaultListableBeanFactory lbf = new DefaultListableBeanFactory(); + RootBeanDefinition stringDef = new RootBeanDefinition(String.class); + stringDef.setScope(RootBeanDefinition.SCOPE_PROTOTYPE); + stringDef.getConstructorArgumentValues().addGenericArgumentValue(new TypedStringValue("value")); + lbf.registerBeanDefinition("string", stringDef); + String val1 = lbf.getBean("string", String.class); + String val2 = lbf.getBean("string", String.class); + assertEquals("value", val1); + assertEquals("value", val2); + assertNotSame(val1, val2); + } + @Test public void testPrototypeWithArrayConversionForConstructor() { DefaultListableBeanFactory lbf = new DefaultListableBeanFactory();