When Spring DI Becomes Evil

I recently started to work on an Open Source SIP project called OpenMRS, and realised that most (if not all) of the unit tests are actually integration tests. The way the OpenMRS code (and test) base uses the Spring IoC is I believe a large part of the problem. Although I am not a big fan of DI, I think there is value in using the Spring IoC in some cases. But there are some pitfalls to avoid, and good practices to follow.

The OBFA Factor

Nowadays, spring contexts seem to be so embedded in Java applications that developers forget that they are meant to be treated parsely, and with caution. One good example of this symptom is the one-big-fat-a*** (OBFA) applicationContext file for the entire stack. Having an OBFA spring context file makes the customisation of parts of it more difficult and dangerous.

Instead, it is preferable to split the contexts into smaller files, so that a layer, or a part of the stack, can be independently reimplemented or reconfigured. As an example, a sample Java app (cashman) I keep for demo purpose has:

  • spring-appcontext.xml Root Spring context file e.g.
<?xml version="1.0" encoding="UTF-8"?>
<beans xmlns="http://www.springframework.org/schema/beans"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://www.springframework.org/schema/beans
http://www.springframework.org/schema/beans/spring-beans-2.5.xsd http://www.springframework.org/schema/context http://www.springframework.org/schema/context/spring-context-3.1.xsd">

<import resource="classpath:spring-datasource.xml"/>
<import resource="classpath:spring-txmanager.xml"/>
<import resource="classpath:spring-hibernate.xml"/>
<import resource="classpath:spring-dao.xml"/>
<import resource="classpath:spring-cashmachine.xml"/>

</beans>
  • spring-cashmachine.xml Domain Service config e.g.
<?xml version="1.0" encoding="UTF-8"?>
<beans xmlns="http://www.springframework.org/schema/beans"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans.xsd http://www.springframework.org/schema/context http://www.springframework.org/schema/context/spring-context-3.1.xsd">

<bean name="cashMachine" factory-method="getInstance">
    <property name="stockDAO" ref="stockDAO" />
</bean>

</beans>
  • spring-dao.xml DAO config e.g.
<?xml version="1.0" encoding="UTF-8"?>
<beans xmlns="http://www.springframework.org/schema/beans"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns:context="http://www.springframework.org/schema/context"
xsi:schemaLocation="http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans.xsd http://www.springframework.org/schema/context http://www.springframework.org/schema/context/spring-context-3.1.xsd">

<bean id="stockDAO">
    <property name="sessionFactory" ref="sessionFactory"/>
</bean>

</beans>
  • spring-datasource.xml Database connection config e.g.
<?xml version="1.0" encoding="UTF-8"?>
<beans xmlns="http://www.springframework.org/schema/beans"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans.xsd">

<bean id="dataSource" destroy-method="close">
    <property name="driverClassName" value="org.hsqldb.jdbcDriver"/>
    <property name="url" value="jdbc:hsqldb:mem:spring-playground"/>
    <property name="username" value="sa"/>
    <property name="password" value=""/>
</bean>

</beans>
  • spring-hibernate.xml Hibernate config e.g.
<?xml version="1.0" encoding="UTF-8"?>
<beans xmlns="http://www.springframework.org/schema/beans"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans.xsd">

<bean id="sessionFactory">
    <property name="dataSource" ref="dataSource" />
    <property name="annotatedClasses">
        <list>
            <value>com.suncorp.cashman.persistence.StockItem</value>
        </list>
    </property>
    <property name="hibernateProperties">
        <props>
            <prop key="hibernate.dialect"> org.hibernate.dialect.HSQLDialect</prop>
            <prop key="hibernate.show_sql">true</prop>
            <prop key="hibernate.hbm2ddl.auto">create</prop>
        </props>
    </property>
</bean>

</beans>
  • spring-txmanager.xml Transaction Manager config e.g.
<?xml version="1.0" encoding="UTF-8"?>
<beans xmlns="http://www.springframework.org/schema/beans"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans.xsd">

<bean id="txManager" name="txManager">
    <property name="dataSource" ref="dataSource"/>
</bean>

</beans>

Doing so enables creating and swapping environment-specific spring configs. The idea is to make environment-specific the configuration elements that will change often. In my case spring-datasource.xml and spring-hibernate.xml are good candidates. So the config contextes be structured as follows:

With this structure, if the app is deployed locally, then the build script will take the spring files in the dev directory. Similarly, if unit tests are run (e.g. in your build pipeline), the files in test would be put on the classpath.

Hence, if developers want to use a local mysql instance, they can configure the dev/spring-datasource.xml file with the following line:

<property name="url" value="jdbc:hsqldb:hsql://localhost:9001"/>

For speed’s sake, unit tests would preferably run on an in-memory database, hence test/spring-datasource.xml would have:

<property name="url" value="jdbc:hsqldb:mem:spring-playground"/>

Note: A similar approach would be to override the default spring config using the @ContextConfiguration Spring 3 annotation. For instance, one could create a supertype class for all the integration tests to load the test-specific context as follows:

@ContextConfiguration(locations={"classpath:spring-test-*.xml"})
public class MyUnitTestBaseClass { }

In that case, each test-specific spring config file’s name starts with spring-test. So the files can stay on the classpath (no need to swap through build script), as they will not clash with the default config, and will only be used by unit tests.

When Spring Hides The Mocking Game

Most OpenMRS JUnit tests extend BaseContextSensitiveTest, which loads the Spring context applicationContext-service.xml for the Application Services, hence automatically wiring the DAOs.

@ContextConfiguration(locations = { "classpath:applicationContext-service.xml", "classpath*:openmrs-servlet.xml",
"classpath*:moduleApplicationContext.xml" })
@TestExecutionListeners( { TransactionalTestExecutionListener.class, SkipBaseSetupAnnotationExecutionListener.class,
StartModuleExecutionListener.class })
@Transactional
public abstract class BaseContextSensitiveTest extends AbstractJUnit4SpringContextTests { [...] }

Because of that, the service unit tests do not mock nor stub the DAOs. The first issue here is that they are not unit tests but integration tests. The second issue is that it becomes very difficult to write proper unit tests (with mocked dependencies) using the same context without breaking other tests.

Indeed, I was trying to add the following test method to ProgramWorkflowServiceTest:

@Test
@Verifies(value="should call the DAO method getProgramsByName", method = "getProgramByName")
public void getProgramByName_shouldCallDaoGetProgramsByName() {     
    ProgramWorkflowDAO mockDao = Mockito.mock(ProgramWorkflowDAO.class);
    pws.setProgramWorkflowDAO(mockDao);
    pws.getProgramByName("A name");
    Mockito.verify(mockDao).getProgramsByName("A name");
}

pws is an instance of ProgramWorkflowService obtained from the Spring context. By doing so, I actually ended up screwing-up all other subsequent unit tests whose class under tests depends on ProgramWorkflowService (since dependencies are obtained from the same context).

As an example, all unit tests for the following method in ProgramEditor will fail because Context.getProgramWorkflowService() at line 7 will return an object that has a mocked DAO:

1. public void setAsText(String text) throws IllegalArgumentException {
2.    if (StringUtils.hasText(text)) {
3.        try {
4.            if (text.startsWith("concept.")) {
5.                Integer conceptId = Integer.valueOf(text.substring(text.indexOf('.') + 1));
6.                Concept c = Context.getConceptService().getConcept(conceptId);
7.                setValue(Context.getProgramWorkflowService().getProgramByName(c.getName().getName()));
8.            } 
9.            else {
10.               Integer programId = Integer.valueOf(text);
11.               setValue(Context.getProgramWorkflowService().getProgram(programId));
12.           }
13.        }
[...]
}

So, by mocking a dependency of my object under test, I unexpectedly modify the behaviour of the code in another part of the application!

Make Spring DI Less Evil

To conclude, my recommendations for a more pragmatic and flexible Spring IoC config:

1. Split config files into small files, and make environment specific those that need to be.
2. Use the IoC only for integration tests, in which case use the @ContextConfiguration annotation on a layer supertype of all your integration tests.
3. Do not use the IoC for unit tests: instantiate, mock, and inject through setters in the @Before method manually.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s