Tuesday, 29 November 2011

Creating Stubs for Legacy Code - Testing Techniques 6

Any one who reads this blog will probably have realised that at present I’m working on a project that contains a whole bunch of legacy code that’s large, expansive, and written without any tests what so ever. 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 sample1.


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<SitePropertiesManager> {

 
private static SitePropertiesManager propsManager = SitePropertiesManager
      .getInstance
();

 
@Override
 
public SitePropertiesManager getObject() throws Exception {

   
return propsManager;
 
}

 
@Override
 
public Class<SitePropertiesManager> 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<String> findListProperty(String propertyName);

 
public abstract List<String> 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<String, String> propMap = new HashMap<String, String>();

 
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<String> findListProperty(String propertyName) {
   
throw new UnsupportedOperationException();
 
}

 
@Override
 
public List<String> 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...



A list of Blogs on Testing Techniques


1 The source code is available from GitHub at:

git://github.com/roghughe/captaindebug.git

1 comment:

Tomasz Nurkiewicz said...

You don't need a separate SitePropertiesManagerFactoryBean, just use: <bean class="SitePropertiesManager" factory-method="getInstance" />. Besides, great series of posts.