Spring 1.1.2 abstract bean and backward compatibility

[sjivan]

I had a txProxyTemplate bean defined in my applicationContext configuration without the abstract="true" attribute and things ran fine with Spring 1.1.1. I upgraded to Spring 1.1.2 and get an IllegalArgumentException stating that the template bean *must* have the abstract="true" attribute if it does not declare a 'target' property.

I went ahead and added it and it was not a big deal. I'm not saying that the IllegalArgumentException thrown under this circumstance is bad, but given the prior behavior, it would have been nicer if it simply logged a warning since the new behavior does break backward compatibility.


Sanjiv

--------------------------------------------


org.springframework.beans.factory.BeanCreationExce ption: Error creating bean with name 'txProxyTemplate' defined in ServletContext resource [/WEB-INF/applicationContext.xml]: Initialization of bean failed; nested exception is java.lang.IllegalArgumentException: 'target' is required
java.lang.IllegalArgumentException: 'target' is required
at org.springframework.transaction.interceptor.Transa ctionProxyFactoryBean.afterPropertiesSet(Transacti onProxyFactoryBean.java:194)
at org.springframework.beans.factory.support.Abstract AutowireCapableBeanFactory.invokeInitMethods(Abstr actAutowireCapableBeanFactory.java:990)
at org.springframework.beans.factory.support.Abstract AutowireCapableBeanFactory.createBean(AbstractAuto wireCapableBeanFactory.java:275)
at org.springframework.beans.factory.support.Abstract AutowireCapableBeanFactory.createBean(AbstractAuto wireCapableBeanFactory.java:193)
at org.springframework.beans.factory.support.Abstract BeanFactory.getBean(AbstractBeanFactory.java:240)
at org.springframework.beans.factory.support.Abstract BeanFactory.getBean(AbstractBeanFactory.java:163)
at org.springframework.beans.factory.support.Abstract BeanFactory.getType(AbstractBeanFactory.java:345)
at org.springframework.beans.factory.support.DefaultL istableBeanFactory.isBeanTypeMatch(DefaultListable BeanFactory.java:193)
at org.springframework.beans.factory.support.DefaultL istableBeanFactory.getBeansOfType(DefaultListableB eanFactory.java:157)
at org.springframework.context.support.AbstractApplic ationContext.getBeansOfType(AbstractApplicationCon text.java:526)
at net.sf.acegisecurity.util.FilterToBeanProxy.doInit (FilterToBeanProxy.java:170)
at net.sf.acegisecurity.util.FilterToBeanProxy.doFilt er(FilterToBeanProxy.java:102)

[Alef Arendsen]

You're absolutely right, this must have slipped through. We're always on the lookout for backward compatibility problems (which we try to prevent) but sometimes things might go unnoticed.

I've posted this to the developers list as well, we might include a notice somewhere...

Sorry if it caused you problems!

regards,
Alef

[Colin Sampaleanu]

Unless I'm missing something, this doesn't have anything to do with 'abstract' per se, but rather the fact that the transaction proxy factory bean didn't use to check that it was given a target, and now it verifies this upfront.

So without the abstract, the template itself was being pre-instantiated as a singleton before, and now that's no longer happening since the check for target is catching the missing target.

So I would simply call this a bad configuration. However, I don't understand why this didn't happen in 1.1.1, it should have behaved the same...

[sjivan]

Colin,
Thanks for the explanation about the previous and current behavior. As I mentioned in my post, I'm not arguing that throwing an IllegalAgrumentException is bad under this circumstance however given that an older version of Spring had a certain default behavior when the 'abstract="true" was not provided, upgrading did result in backward incompatibility. And I think there's a subtle difference between this being a backward incompatible change versus a bad user configuration. (Correct me if I'm wrong, but if memory serves me right, I think the original example of txProxyTemplate on your blog did not have the abstract attribute.)

I'm still in development phase so it did not matter much to me but consider an application that was live with a txProxyTemplate defn. without abstract="true". Upgrading would break the app.

For reference, my original bean definition used to look like the following

<bean id="txProxyTemplate" lazy-init="true" class="org.springframework.transaction.interceptor.TransactionProxyFactoryBean">
  <property name="transactionManager">
    <ref bean="transactionManager"/>
  </property>
  <property name="transactionAttributes">
   <props>
    <prop key="save*">PROPAGATION_REQUIRED</prop>
    <prop key="delete*">PROPAGATION_REQUIRED</prop>
    <prop key="*">PROPAGATION_REQUIRED,readOnly</prop>
   </props>
  </property>
</bean>

Regards,
Sanjiv

[sjivan]

And regarding a difference between backward incompatibilty verus bad configuration, I got another exception when I upgraded to Spring 1.1.2. Spring now raises a BeanDefinitionStoreException exception if a bean is declared with multiple definitions of the same property. I view this as being more of a bad configuration issue versus backward incompatible change.

<bean id="fooBean" class="com.Foo">
  <property name="bar">
    <ref bean="barBean"/>
  </property>
  <property name="bar">
    <ref bean="barBean"/>
  </property>
 </bean>


org.springframework.beans.factory.BeanDefinitionStoreException&#58; Error registering bean with name 'fooBean' defined in ServletContext resource &#91;/WEB-INF/applicationContext.xml&#93;&#58; Multiple 'property' definitions for property 'bar'
	at org.springframework.beans.factory.xml.DefaultXmlBeanDefinitionParser.parsePropertyElement&#40;DefaultXmlBeanDefinitionParser.java&#58;475&#41;
	at org.springframework.beans.factory.xml.DefaultXmlBeanDefinitionParser.getPropertyValueSubElements&#40;DefaultXmlBeanDefinitionParser.java&#58;386&#41;
	at org.springframework.beans.factory.xml.DefaultXmlBeanDefinitionParser.parseBeanDefinition&#40;DefaultXmlBeanDefinitionParser.java&#58;291&#41;
	at org.springframework.beans.factory.xml.DefaultXmlBeanDefinitionParser.parseBeanDefinition&#40;DefaultXmlBeanDefinitionParser.java&#58;261&#41;
	at org.springframework.beans.factory.xml.DefaultXmlBeanDefinitionParser.registerBeanDefinitions&#40;DefaultXmlBeanDefinitionParser.java&#58;184&#41;
	at org.springframework.beans.factory.xml.XmlBeanDefinitionReader.registerBeanDefinitions&#40;XmlBeanDefinitionReader.java&#58;170&#41;
	at org.springframework.beans.factory.xml.XmlBeanDefinitionReader.loadBeanDefinitions&#40;XmlBeanDefinitionReader.java&#58;129&#41;
	at org.springframework.context.support.AbstractXmlApplicationContext.loadBeanDefinitions&#40;AbstractXmlApplicationContext.java&#58;144&#41;
	at org.springframework.context.support.AbstractXmlApplicationContext.refreshBeanFactory&#40;AbstractXmlApplicationContext.java&#58;79&#41;
	at org.springframework.context.support.AbstractApplicationContext.refresh&#40;AbstractApplicationContext.java&#58;249&#41;
	at org.springframework.web.context.support.XmlWebApplicationContext.refresh&#40;XmlWebApplicationContext.java&#58;131&#41;

[Colin Sampaleanu]

Colin,
Thanks for the explanation about the previous and current behavior. As I mentioned in my post, I'm not arguing that throwing an IllegalAgrumentException is bad under this circumstance however given that an older version of Spring had a certain default behavior when the 'abstract="true" was not provided, upgrading did result in backward incompatibility. And I think there's a subtle difference between this being a backward incompatible change versus a bad user configuration. (Correct me if I'm wrong, but if memory serves me right, I think the original example of txProxyTemplate on your blog did not have the abstract attribute.)

I'm still in development phase so it did not matter much to me but consider an application that was live with a txProxyTemplate defn. without abstract="true". Upgrading would break the app.

For reference, my original bean definition used to look like the following

<bean id="txProxyTemplate" lazy-init="true" class="org.springframework.transaction.interceptor.TransactionProxyFactoryBean">
  <property name="transactionManager">
    <ref bean="transactionManager"/>
  </property>
  <property name="transactionAttributes">
   <props>
    <prop key="save*">PROPAGATION_REQUIRED</prop>
    <prop key="delete*">PROPAGATION_REQUIRED</prop>
    <prop key="*">PROPAGATION_REQUIRED,readOnly</prop>
   </props>
  </property>
</bean>

Regards,
Sanjiv

Well, your original definition had lazy-init=true, to try to stop it from being accidentally pre-instantiated. But in your case, what happened with 1.1.2 is that _it was_ being explicitly instantiated. Now looking at your stack trace, this is happening as a result of a call from the Acegi Security filter, calling getBeansOfType. If it was picking up a factory bean like this, then the filter was specifying true on the "include factory beans" flag. What I think happened here though is the old getBeansOfType function had a bug which Juergen fixed for 1.1.2, where it didn't actually return everything it was supposed to. So when 1.1.2 started providing this instance too (which was not marked abstract) from getBeansOfType(), it all broke for you.

This whole need for a real distinction between abstract beans and none-abstract beans for cased like getBeansOfType is why the abstract flag was created in the first place, as opposed to relying just on lazy-init=true.

Hopefully that clears up when happened...

[sjivan]

Cool. Thanks for the explanation.

Sanjiv