A mock to save the day

The other day I had to complete the code of a colleague. To deliver. I went optimistic and lazy and as the code presented a problematic dependency, I didn't push the testing as far as I should have.

Here is the summary of the problem as C# code (disclaimer: none of the code in this article is real code):

public ReturnedObject NotifyOnlineSalesMonitoring(Notication notification)
{
    var contract = this.contractRepository.GetByCode(notification.ContractCode);
    NotificationReturn serviceReturn;

    // bad dependancy bellow
    var salesMonitoringService = Container.Resolve<OnlineSalesMonitoringService>();
    try
    {
        serviceReturn = salesMonitoringService.Notify(notification);
        loggingService.log(serviceReturn.ReturnCode, contract.ContractCode, contract.ProductCode.ProductId);
    }
    catch (SalesMonitorException)
    {
        loggingService.log("KO", contract.ContractCode, contract.ProductCode);
        return ReturnedObject(contract, NotificationReturn { ReturnCode = "KO" });
    }

    contract.logNotification(serviceReturn);
    modifiedContract = this.contractRepository.ApplyModification(contract);

    // Why don't you keep initial contract object???
    loggingService.log(serviceReturn.ReturnCode, modifiedContract.ContractCode, modifiedContract.ProductCode.ProductId);

    return new ReturnedObject(contract, serviceReturn);
}

We build a orchestration service application. It exposes an unified SOAP API and call and call different services and databases to deliver its functionalities.

The use case is to notify OnlineSalesMonitoringService that we created a contract, then log this call, add notified service returned information if there's been no error on the contract and finally log the contract modification.

The nasty dependency lies in the call of Container.Resolve<OnlineSalesMonitoringService>. Container object represents our dependency injection framework and provides the resolve method to get managed references. It's a mandatory here since the object is decorated during this call by contextual data (basically user information). The dependency cannot be simply injected as a constructor argument.

For this reason also, calling the service from VS2008 test runner always get an exception, because the entire context is built by the framework when processing incoming SOAP call.

I left the situation like this: in all my tests, I get systematically into the catch block, and the rest of the method was not run, as I thought that contract modification would not be problematic. By the way we do modification from lots of place in the code!

The problem is that the loggingService calls were new… And that ProductCode property in modifiedContract object is never set.

So we shipped and QA faced an almost systematic null reference exception!

The error here is to have been too lazy to wrap the Container call behind an interface to test with a mock and check every possible path.

Regarding this constraint, how do I do that since the OnlineSalesMonitoringService cannot be properly injected?

Actually, the important idea is not to hide every dependency behind the DI container, but to push then at the boundaries of our system so that they won't mess into our business logic.

Then the solution is always the same: adding a level of indirection:

public class OnlineSalesMonitoringServiceAdapter : IOnlineSalesMonitoringServiceAdapter
{
    public NotificationReturn Notify(Notification notification)
    {
        var salesMonitoringService = Container.Resolve<OnlineSalesMonitoringService>();
        return salesMonitoringService.Notify(notification);
    }
}

public interface IOnlineSalesMonitoringServiceAdapter
{
    NotificationReturn Notify(Notification notification);
}

Then I can reference IOnlineSalesMonitoringServiceAdapter in NotifyOnlineSalesMonitoring method, mock it in the test, check and fix the error. Phew.

Lessons of the day:

• Laziness will always be paid back.
• Adding a level of indirection is what you need most of the time.
• Dependencies with infra and tier party services should be pushed to the boundaries to make your testing easier.
• I have to review the object model to avoid NullReferenceException. But I feel tired, I'll do that later…