Add ability to declare bean type and explicitely mark bean factories in XML configurations [SPR-5114]

[spring-projects-issues]

Grzegorz Borkowski opened SPR-5114 and commented

This is the description of my proposition for two small improvements of Spring XML configuration files in defaults schema ("beans").

Description of the problem:

Let us first look at a fragment of typical configuration, here configuring JPA:

<bean id="emf" class="org.springframework.orm.jpa.LocalContainerEntityManagerFactoryBean">
<property name="dataSource" ref="dataSource"/>
</bean>
<bean id="dataSource" class="org.apache.commons.dbcp.BasicDataSource">
<property name="driverClassName" value="oracle.jdbc.OracleDriver"/>
...
</bean>

In Spring documentation and presentations it is often mentioned, that explicit configuration in XML has advantage of being kind of blueprint documentation of the application. So what we can find out reading this fragment?
First, we have a bean of type LocalContainerEntityManagerFactoryBean. Suppose I want to inject reference to it in other bean. What type this reference should be? If I check quickly in JavaDoc, I will find out that it extends AbstractEntityManagerFactoryBean. I suppose that it implements JPA's EntityManagerFactory. But it doesn't. So I'm confused here. After more reading, I will find finally that it implements FactoryBean and returns EntityManagarFactory bean. But is is not visible at all from the configuration. I believe it should.
Then I look at dataSource bean. It is of type org.apache.commons.dbcp.BasicDataSource. I want to inject it into my other bean. What should be type of injected field: BasicDataSource? I would say, in 90% of cases not, it should be standard jdbc DataSource. In most cases BasicDataSource configured by Spring no longer needs programatic access to its specific methods. But can I make my field being of type BasicDataSource? Yes I can. Is it good way? Definitely not. It is not a good way, because if after some time somebody decides to put our application on some JDNI-aware server and retrieve dataSource bean from JNDI, than application will suddenly crash, because one of programmers (or more) declared injected filed as BasicDataSource instead of DataSource. But how on earth could he know? He saw bean of "BasicDataSource" in spring XML.

Solution:
Solution consist of two improvements. Both of them must not be obligatory, because of backword compatibility. They introduce (almost) no behavioral changes, no logic etc. They purpose is purely to improve the XML configuration semantics, i.e. making XML more clear in the role of application blueprint.

1. If the bean implements BeanFactory interface, you can declare it as <beanfactory> instead of <bean> (you can, don't have to). This has advantage that it is immediatelly visible looking into config file, that the returned bean will be not of the type of "class" attribute and cannot be cast to any of it supertypes. You must check instead what type this factory returns.

2. Add additional optional attribute "type" (or maybe "castType" or "castTo" or ...?) is added to <bean> and <beanfactory>. This is a hint for other developers that they should in most cases cast the bean to this type. If they do, then you can safely replace implementations of this type at any time, and you will not get any nasty problems with incompatible types at runtime.

Let's rewrite our example now:

<factorybean id="emf" class="org.springframework.orm.jpa.LocalContainerEntityManagerFactoryBean" type="javax.persistence.EntityManagerFactory">
<property name="dataSource" ref="dataSource"/>
</factorybean>
<bean id="dataSource" class="org.apache.commons.dbcp.BasicDataSource" type="javax.sql.DataSource">
<property name="driverClassName" value="oracle.jdbc.OracleDriver"/>
...
</bean>

Now I don't have to searching JavaDocs to find out what is this LocalContainerEntityManagerFactoryBean. I immediately see, that is is factoryBean, which returns EntityManagerFactory.
And I am less afraid that one of programmers from my team declares injected dataSource bean somewhere in the code as BasicDataSource, becaue now everybody knows that the dataSource bean should be always referred to as javax.sql.DataSource. (Obviously the problem can still happen, sometimes you can really need some methods of BasicDataSource in your code. But now at least you were warned by the "type" attribute that you are doing it on you own risk and on your responsibility, so you must really have a good reason to do it.).
This clearly states the intent of the person who put this declarations:
"I declare the bean emf of type EntityManagerFactory, implemented by LocalContaierEntityManagerFactoryBean factory. I declare dataSource bean of DataSource type, implemented by BasicDataSource. So there are really two beans here: EntityManagerFactory and DataSource".
You cannot say it looking at original configuration fragment.

This is almost purely semantical change. Almost, because we can add small change to Spring behavior: 1. If bean is declared as <beanfactory> Spring will check if it implements BeanFactory (or we can decide that having such XML element it is no longer necessary to implement this interface); 2. If "type" attribute is declared, Spring will check if returned bean can be really casted to this type, and throw exception otherwise.

BTW: JavaConfig already implements improvement #2: Your @bean method explicitely declare the type of returned bean. I really like it, and I miss it in XML.

---

No further details from SPR-5114

[spring-projects-issues]

Grzegorz Borkowski commented

Adding "type" attribute for beans has additional advantage: if the 'type' is the interface, it can be used for jdk-proxy based aop to immediately know what interface should be proxied.

[spring-projects-issues]

Chris Beams commented

Note that FactoryBean is now generic in Spring 3.

For example, LocalEntityManagerFactoryBean now implements FactoryBean<EntityManagerFactory>

This additional generics metadata should make it possible for tooling (Spring IDE, SpringSource Tool Suite, IntelliJ) to detect that you are attempting to inject an incorrect type of bean and raise an error in the editor. This can all be accomplished without necessitating the introduction of a new type attribute as suggested above.

Please consider adding an new feature request to the Spring IDE project for this. I think it's worthwhile, but probably belongs in the domain of tooling rather than core support.

[spring-projects-issues]

Grzegorz Borkowski commented

Well, making FactoryBean generic is a step in good direction. But this solves a problem only partially.

1. It still doesn't make the XML configuration easier to read. Looking at XML, you still don't know what this factory returns.
2. Not every uses Spring-aware IDEs - at least not so much Spring-aware. For example, NetBeans is already Spring-aware, but I'm not sure if it is able to check the generic types of FactoryBeans.
3. This doesn't help a single bit with non-factory beans: the example of BasicDataSource is still valid. Declaring BasicDataSource in XML, you have absolutely no chance to tell other developers that you want to actually declare javax.sql.DataSource, and BasicDataSource is just an implementation that can be changed at any time. Personally, i treat it as serious Spring drawback from my first contact with Spring: I cannot show my intentions in XML, and I cannot say that a bean "is of type" other than actual implementation type. I would like to say: "guys, don't look at this bean as BasicDataSource; implementation can change; this bean is of type DataSource, and use type DataSource in all places where you inject this bean". I've seen many times problems arising from the fact that other developers from the team had injected some bean using its implementation type, that was supposed to be temporary, and after changing the implementation class in XML, the type needed to be changed in many places scattered around the code. That's why I really love the way it is done in JavaConfig: you declare you @Bean-marked method as

public DataSource dataSource() {
   return new BasicDataSource(...);
}

This way your intention is clear: the bean type is DataSource, not BasicDataSource. Actual type (BasicDataSource) is just an implementation details, subject to change at any time.

[spring-projects-issues]

Juergen Hoeller commented

Closing groups of outdated issues. Please reopen if still relevant.