Issue Details (XML | Word | Printable)

Key: SPR-1076
Type: Improvement Improvement
Status: Resolved Resolved
Resolution: Fixed
Priority: Major Major
Assignee: Juergen Hoeller
Reporter: Keith Garry Boyce
Votes: 1
Watchers: 3
Operations

If you were logged in you would be able to see more operations.
Spring Framework

AbstractApplicationContext shouldn't eagerly instantiate all BeanFactoryPostProcessors

Created: 24/Jun/05 09:15 PM   Updated: 20/Nov/07 10:13 PM
Component/s: SpringCORE
Affects Version/s: 1.2.1
Fix Version/s: 2.1 M2

Time Tracking:
Not Specified

File Attachments: 1. Java Source File AbstractApplicationContext.java (30 kB)
2. Java Source File EagerPropertyPlaceholderConfigurer.java (4 kB)
3. Zip Archive PlaceHolderOrderTest.zip (2 kB)

Issue Links:
Duplicate
 


 Description  « Hide
The call to getBean(factoryProcessorNames[i]) is flawed because it then must go out to get dependencies just to do ordering.. Below is an example where I had a problem. And below is a work around.. I think rather than implement order perhaps you might want to add an attribute to bean definition i.e: order= and use that instead.

What was happening is that to find the order of ConfigurationPlaceholderProcessor it had to satisfy dependency jconfigBean. The placeholder in that bean was supposed to be replaced first by propertyPlaceholderConfigurer so even though it gets the order for ConfigurationPlaceholderProcessor it's then to late to instantiate jconfigBean with appropriate placeholder from the propertyPlaceholderConfigurer since it had already been instantiated.

--------- flawed code -------

for (int i = 0; i < factoryProcessorNames.length; i++) {
if (Ordered.class.isAssignableFrom(getType(factoryProcessorNames[i]))) {
orderedFactoryProcessors.add(getBean(factoryProcessorNames[i]));
}
else {
nonOrderedFactoryProcessorNames.add(factoryProcessorNames[i]);
}
}

------------- Here is where I had problem ------------

    <bean id="jmxExporter"
        class="org.springframework.jmx.export.MBeanExporter">
        <property name="beans">
            <map>

                <entry key="Configuration:name=jmxConfig">
                    <ref local="jconfigBean" />
                </entry>
            </map>
        </property>
    </bean>
    
    <bean id="jconfigBean" class="org.jconfig.jmx.ConfigHandler">
        <property name="resourceName">
            <value>${config.filename}</value>
        </property>
        <property name="configurationName">
            <value>jmxConfig</value>
        </property>
    </bean>


<bean id="configurationFactory"
class="org.reactive.configuration.JConfigConfigurationFactory"
singleton="true">

<property name="fileName"><value>jmxConfig</value></property>
</bean>


<bean id="configuration"
            factory-bean="configurationFactory"
            factory-method="getConfiguration"
            depends-on="jmxExporter"
            />
  
<bean id="configurationPlaceholderProcessor"
class="org.reactive.beans.factory.config.ConfigurationPlaceholderProcessor">
<property name="placeholderEvaluator">
<bean class="org.reactive.beans.factory.config.jconfig.JConfigPlaceholderEvaluator">
<property name="configuration">
<ref bean="configuration"/>
</property>
</bean>
</property>
<property name="order">
<value>2</value>
</property>
</bean>

<bean id="propertyPlaceholderConfigurer" class="org.springframework.beans.factory.config.PropertyPlaceholderConfigurer">
<property name="properties">
<props>
<prop key="config.filename">dev_config.xml</prop>
</props>
</property>
<property name="systemPropertiesModeName">
<value>SYSTEM_PROPERTIES_MODE_OVERRIDE</value>
</property>
<property name="order">
<value>1</value>
</property>
</bean>

------------- Here is possible (not ultimate) fix -----------
for (int i = 0; i < factoryProcessorNames.length; i++) {
if (Ordered.class.isAssignableFrom(getType(factoryProcessorNames[i]))) {
MutablePropertyValues values = getBeanFactory().getBeanDefinition(factoryProcessorNames[i]).getPropertyValues();
int order = Integer.MAX_VALUE;
if (values != null) {
PropertyValue orderValue = values.getPropertyValue("order");
if (orderValue != null) {
String orderInt = (String) orderValue.getValue();
if (orderInt != null) {
order = new Integer(orderInt).intValue();
}
}
}
OrderedFactoryHolder factoryProcessorHolder = new OrderedFactoryHolder(factoryProcessorNames[i],order);
orderedFactoryProcessorHolders.add(factoryProcessorHolder);
}
else {
nonOrderedFactoryProcessorNames.add(factoryProcessorNames[i]);
}
}



private class OrderedFactoryHolder implements Ordered {


        /**
         * @param factoryName
         * @param order
         */
        public OrderedFactoryHolder(String factoryProcessorName, int order) {
            this.factoryProcessorName = factoryProcessorName;
            this.order = order;
        }
private String factoryProcessorName;
private int order;



        /**
         * @return Returns the factoryProcessorName.
         */
        public String getFactoryProcessorName() {
            return factoryProcessorName;
        }
        /* (non-Javadoc)
         * @see org.springframework.core.Ordered#getOrder()
         */
        public int getOrder() {
            return order;
        }

}

 All   Comments   Work Log   Change History   FishEye   Builds      Sort Order: Ascending order - Click to sort in descending order
Keith Garry Boyce added a comment - 25/Jun/05 11:19 AM
BTW:

If you use suggestion order=
then

if (Ordered.class.isAssignableFrom(getType(factoryProcessorNames[i]))) {

should be changed to look for that instead...

Keith Garry Boyce added a comment - 25/Jun/05 11:39 AM
BTW: There is one other way to implement this. There could be a getBean method that does not resolve dependencies and does not add bean to app context...

i.e: getTemporaryBean(String beanName, boolean resolveDependencies)

Then they object could be instantiated temporarily to retreive the order and everything would also work...

In this case I would pass false for resolveDependencies since order is the only thing important in the bean.. Of course the problem would be if the value of order setter was itself a dependency or it was a value that required post-processing first.

For this reason order= might be cleaner and more failsafe..

Keith Garry Boyce added a comment - 25/Jun/05 12:10 PM
NB:

If you decide to implement temporary bean method and choose to pass true to resolveDependencies then it is important that the beans are only instantiated temporarily and not added to app context. This is because any of the attributes assigned in ant if the dependencies including the actual bean being retreived may require post-processing first dependent on ordering.

Keith Garry Boyce added a comment - 25/Jun/05 12:12 PM
Here is the attached code for one proposed work around...

Keith Garry Boyce added a comment - 27/Jun/05 10:41 AM
Any particular reason why not fix version 1.2.2

Juergen Hoeller added a comment - 21/Jul/05 02:38 PM
A proper solution for this constitutes a significant change. Checking the "order" value from the PropertyValues object is a workaround only... A bean could also implement Ordered directly, through a hard-coded "getOrder" method - not having a "setOrder" method at all.

Juergen

Alex Wei added a comment - 31/Jul/06 07:28 PM
I have create EagerPropertyPlaceholderConfigurer, which resolves property placeholders in bean definitions eagerly (immediately after initialisation by implementing InitializingBean).

For it to resolve property placeholders in bean definitions of other BeanFactoryPostProcessors, it must be defined before those that depend on it.

This should cover most of issues reported by Spring users for BeanFactoryPostProcessor property placeholders.

Alex Wei added a comment - 31/Jul/06 07:37 PM
EagerPropertyPlaceholderConfigurer can be downloaded from the file attachments (# 2).

Dave Syer added a comment - 12/Aug/06 02:31 AM
SPR-1310 also had a suggestion for a workaround: use parent context to set the properties that need to be loaded first. I have attached a test case that show how to do this.

Juergen Hoeller added a comment - 24/May/07 03:03 AM
As of Spring 2.1 M2, we support a PriorityOrdered concept, where we have three phases of initialization: PriorityOrdered beans, then plain Ordered beans, then unordered beans. PropertyResourceConfigurer (and hence PropertyPlaceholderConfigurer) implement PriorityOrdered now, which allows to to apply to other BeanFactoryPostProcessors as well - before those get instantiated.

One effect from this is that "order" property values expressed on the bean will now be relative to PriorityOrdered and Ordered: a PriorityOrdered bean with order value 5 will be initialized *before* an Ordered bean with value 2. However, since people will always have defined PropertyPlaceholderConfigurers as the first in the chain anyway, this shouldn't pose a problem, and PriorityOrdered beans are generally supposed to have lower order values than Ordered beans anyway.

It's more important that things work appropriately by default now, with PropertyPlaceholderConfigurers automatically initializing first - no need to specify custom order values for that. So in most cases, the ordering concept disappears from the user view now, at least when dealing with BeanFactoryPostProcessors.

Juergen

Antony Stubbs added a comment - 20/Nov/07 09:49 PM
The problem still exists that the PropertyPlaceholderConfigurer are not loaded before <import> statements. This means that you can't conditionally import bean definitions.

I want to be able to do this so that I don't have to have a jndi environment setup in development, and can instead import a different xml file that defines my datasource bean directly.

Antony Stubbs added a comment - 20/Nov/07 10:13 PM