Friday, September 03, 2010

Indigestion from Digester and BeanUtils

We had some very simple XML that needed to be parsed. Apache Commons Digester was already in the project, so I used that and quickly whipped up a solution. All the happy-path tests worked, so I started adding edge-case tests.

A test that threw total nonsense malformed XML at it passed without any problems.

The next test was to pass in XML where one the attributes should have been a number, but the test passed a String. The test expected an Exception to be thrown, but there wasn't one and it failed. Surprisingly, Digester silently ignored the fact that the attribute could not be parsed into a long and simply left the attribute on the target class as zero.

Googling revealed two issues.

The first was that the default ErrorHandler for Digester swallows Exceptions. Bad stuff, but can be remedied by creating your own ErrorHandler:

ErrorHandler errorHandler = new ErrorHandler() {
public void warning (SAXParseException e) throws SAXParseException { throw e; }
public void error (SAXParseException e) throws SAXParseException { throw e; }
public void fatalError(SAXParseException e) throws SAXParseException { throw e; }
};

Digester digester = new Digester();
digester.setErrorHandler(errorHandler);

This did not fix the test!

Further digging found that Digester depends on Apache Commons BeanUtils. In particular, it uses ConvertUtils to perform type conversions. The default converter is the culprit which silently does nothing if a conversion cannot be performed.

OK, so just right our own converter, right? Maybe.

You can right your own converter and you register it with ConvertUtils. But, the method signature for register is:

public static void register(Converter converter, Class clazz)

static!

This means that your custom converter will be used by all the Digester instances in your application. What's wrong with that? Well, in my case, this is a legacy application with test coverage around 1%. There may be code that 'depends' on the brokenness of the default converter behavior in Digester. Also, Digester is used by some other open source frameworks that may be in the project.

In the end, I decided not to use Digester and instead used dom4j which also allowed for a quick solution, but did the right thing and allowed me to control behavior on an instance by instance basis rather than using statics.

Friday, August 06, 2010

Commons-logging Headaches with Axis

I've written a web service using Axis 1.4 which depends on Apache commons-logging. Deploying on Tomcat, everything works fine, but some JUnit tests run fine in Eclipse, but break when running under CruiseControl throwing an exception:

org.apache.commons.discovery.DiscoveryException: No implementation defined for org.apache.commons.logging.LogFactory

The root cause is that commons-logging depends on Commons Discovery which plays tricks with the class-loader to 'discover' the logging implementation to use at runtime. For an exhaustive list of reasons as to why this is wrong-headed, read this article by Ceki Gülcü.

The normal way to specify which implementation to use is to have a commons-logging.properties file in the classpath with entries something like this:

org.apache.commons.logging.Log = org.apache.commons.logging.impl.Log4JLogger
org.apache.commons.logging.LogFactory = org.apache.commons.logging.impl.LogFactoryImpl

or to specify these as system properties using the -D option when running your app. In my case, I could not (easily) modify the ant targets to change the classpath because Configuration Management maintains shared build scripts that have to go through a heavy process to make any changes. To get around this, I set the system properties in code inside the test:

@BeforeClass
public static void beforeClass() {
System.setProperty("org.apache.commons.logging.Log", "org.apache.commons.logging.impl.Log4JLogger");
System.setProperty("org.apache.commons.logging.LogFactory", "org.apache.commons.logging.impl.LogFactoryImpl");
}

This fixed the issue and the tests now run under CruiseControl.