In working with this legacy code, there’s been one very badly behaved class that’s all pervasive, which the whole team have tripped over time and time again.
In order to protect the guilty, I’ll call it Mr. X although its real name is SitePropertiesManager as it manages the web site’s properties. It’s badly behaved because it:
- breaks the single responsibility principal
- has uses singleton pattern summarised by a getInstance() factory method,
- has an init() method that must be called before any other method,
- loads its data using direct access to the database rather than using a DAO,
- uses an complex map of maps to store its data,
- accesses the file system to cache data returned by the database
- has a timer to decide when up update its cache.
- was written before generics and has a multitude of superfluous findXXXX() methods.
- doesn’t implement an interface,
- employs lots of copy and paste programming.
This makes it hard to create stubs when writing unit tests for new code and leaves the legacy code littered with:
SitePropertiesManager propman = SitePropertiesManager.getInstance();
This blog takes a look at ways of dealing with awkward characters and demonstrates how to create stubs for them, whilst negating the effect of the Singleton pattern. As with my previous Testing Techniques blogs, I’m basing my demo code on my Address web app sample.
In the other blogs in this series, I’ve been demonstrating how to test the AddressService and this blog is no different. In this scenario, however, the AddressService has to load a site property and decide whether or not to return an address, but before we take a look at that I first of all needed the badly written SitePropertiesManager to play with. However, I don’t own that code, so I’ve written a stunt-double version, which breaks as many rules as I can think of. I’m not going to bore you with the details here as all the source code for SitePropertiesManager is available at: git://github.com/roghughe/captaindebug.git
As I said above, in this scenario, the AddressService is using a site property to determine if it’s enabled. If it is, then it’ll send back an address object. I’m also going to pretend that the AddressService is some legacy code that uses the site properties static factory method as shown below:
public Address findAddress(int id) {
logger.info("In Address Service with id: " + id);
Address address = Address.INVALID_ADDRESS;
if (isAddressServiceEnabled()) {
address = addressDao.findAddress(id);
address = businessMethod(address);
}
logger.info("Leaving Address Service with id: " + id);
return address;
}
private boolean isAddressServiceEnabled() {
SitePropertiesManager propManager = SitePropertiesManager.getInstance();
return new Boolean(propManager.findProperty("address.enabled"));
}
In taming this type of class, he first thing to do is to stop using getInstance() to get hold and an object, removing it from the above method and to start using dependency injection. There has to be at least one call to getInstance(), but that can go somewhere in the program’s start-up code. In the Spring world, the solution is to wrap a badly behaved class in a Spring FactoryBean implementation, which becomes the sole location of getInstance() in your application – at least for new / enhancement code.
public class SitePropertiesManagerFactoryBean implements
FactoryBean {
private static SitePropertiesManager propsManager = SitePropertiesManager
.getInstance();
@Override
public SitePropertiesManager getObject() throws Exception {
return propsManager;
}
@Override
public Class getObjectType() {
return SitePropertiesManager.class;
}
@Override
public boolean isSingleton() {
return true;
}
}
This can now be autowired into the AddressService class:
@Autowired
void setPropertiesManager(SitePropertiesManager propManager) {
this.propManager = propManager;
}
These changes, however, don’t mean that we can write some proper unit tests for AddressService, they just prepare the ground. The next step is to extract an interface for SitePropertiesManager, which is easily achieved using eclipse.
public interface PropertiesManager {
public abstract String findProperty(String propertyName);
public abstract String findProperty(String propertyName, String locale);
public abstract List findListProperty(String propertyName);
public abstract List findListProperty(String propertyName, String locale);
public abstract int findIntProperty(String propertyName);
public abstract int findIntProperty(String propertyName, String locale);
}
In moving to interfaces, we also need to manually configure an instance of SitePropertiesManager in the Spring config file so that Spring knows which class to connect to what interface:
<beans:bean id="propman" class="com.captaindebug.siteproperties.SitePropertiesManager" />
We also need to update the AddressService’s @Autowired annotation with a qualifier:
@Autowired
@Qualifier("propman")
void setPropertiesManager(PropertiesManager propManager) {
this.propManager = propManager;
}
With an interface, we can now easily write a simple SitePropertiesManager stub:
public class StubPropertiesManager implements PropertiesManager {
private final Map propMap = new HashMap();
public void setProperty(String key, String value) {
propMap.put(key, value);
}
@Override
public String findProperty(String propertyName) {
return propMap.get(propertyName);
}
@Override
public String findProperty(String propertyName, String locale) {
throw new UnsupportedOperationException();
}
@Override
public List findListProperty(String propertyName) {
throw new UnsupportedOperationException();
}
@Override
public List findListProperty(String propertyName, String locale) {
throw new UnsupportedOperationException();
}
@Override
public int findIntProperty(String propertyName) {
throw new UnsupportedOperationException();
}
@Override
public int findIntProperty(String propertyName, String locale) {
throw new UnsupportedOperationException();
}
}
From having stub, it’s pretty easy to write a unit test for the AddressService that uses the stub and is isolated from the database and the file system
public class AddressServiceUnitTest {
private StubAddressDao addressDao;
private StubPropertiesManager stubProperties;
private AddressService instance;
@Before
public void setUp() {
instance = new AddressService();
stubProperties = new StubPropertiesManager();
instance.setPropertiesManager(stubProperties);
}
@Test
public void testAddressSiteProperties_AddressServiceDisabled() {
/* Set up the AddressDAO Stubb for this test */
Address address = new Address(1, "15 My Street", "My Town", "POSTCODE",
"My Country");
addressDao = new StubAddressDao(address);
instance.setAddressDao(addressDao);
stubProperties.setProperty("address.enabled", "false");
Address expected = Address.INVALID_ADDRESS;
Address result = instance.findAddress(1);
assertEquals(expected, result);
}
@Test
public void testAddressSiteProperties_AddressServiceEnabled() {
/* Set up the AddressDAO Stubb for this test */
Address address = new Address(1, "15 My Street", "My Town", "POSTCODE",
"My Country");
addressDao = new StubAddressDao(address);
instance.setAddressDao(addressDao);
stubProperties.setProperty("address.enabled", "true");
Address result = instance.findAddress(1);
assertEquals(address, result);
}
}
All of which is pretty hunky-dory, but what happens if you can’t extract an interface? I’m saving that for another day…
Reference: Creating Stubs for Legacy Code – Testing Techniques 6 from our JCG partner at the Captain Debug blog
Related Articles :
- Testing Techniques – Not Writing Tests
- The Misuse of End To End Tests – Testing Techniques 2
- What Should you Unit Test? – Testing Techniques 3
- Regular Unit Tests and Stubs – Testing Techniques 4
- Unit Testing Using Mocks – Testing Techniques 5
- More on Creating Stubs for Legacy Code – Testing Techniques 7
- Why You Should Write Unit Tests – Testing Techniques 8
- Some Definitions – Testing Techniques 9
- Using FindBugs to produce substantially less buggy code
- Developing and Testing in the Cloud
