Migrating legacy code with singletons to Dependency injection

Question

I'm working on a larger, older project. Our code is littered with classical singletons, i.e. classes like

public class ABCService {
   private static final instance = new ABCService();

   public static ABCService getInstance(){ return instance; }

   public void doTheThing(){ /* business logic */ }
}

And naturally these get called all over the place (often many dozens and sometimes hundreds of code places per singleton class) like so

public class myClass {
   public void myMethod(){
      ABCService.getInstance().doTheThing();
   }
}

Our team is aware of the drawbacks of this pattern, specifically the reduced testability due to tight coupling. We want to increase the degree of automated testing and have decided to use a CDI container to achieve looser coupling and easier mocking of dependencies in our (existing and future) unit tests. So ideally, somewhere in the future these classes will look like

@ApplicationScoped
public class ABCService(){
   public void doTheThing(){ /* business logic */ }
}

and the calls like

public class myClass {
   @Inject
   private ABCService abcService;

   public void myMethod(){
      abcService.doTheThing();
   }
}

My question is simple:

What is a good strategy to gradually change over from classical singletons to CDI managed singletons?

Changing over the whole application or large parts of the application all at once is not feasible. Therefore I cannot slap the annotation on ABCService and be done with it, because I don't want to change all (hundreds) of classes that use the getInstance()-Method to use @Inject.

My first idea was to keep the getInstance-Method but change its implementation to

@Deprecated
public static ABCService getInstance(){
   return CDI.current().select(ABCService.class).get();
}

and then step-by-step updating the other classes in the coming weeks and months. This does work in the application itself, but it breaks many of our existing tests: Because the classes using ABCService are tightly coupled to it, there is often no mocking in the "unit" tests (i.e. they aren't true unit tests - the very thing we want to change in the future) and the call to getInstance() throws an exception because the tests do not have a running CDI container. Again: Changing all (hundreds) of "unit" tests all at once to spin up and configure a CDI container for each one isn't really feasible. If it was only this one singleton, I'd do it, but there are a couple dozens of those beasts.

The other way around is similarly tricky. It strikes me as wasted time to first change all the classes using ABCService to some intermediate solution like constructur-based injection so that I can rewrite the tests with proper mocking so that I can rewrite ABCService and then have to rewrite all the classes using it again. Especially because the usage of constructor injection requires to change all the classes using those constructors as well.

Ideally, I'd have an implementation and a migration strategy that allows me to use the old classes and their old "unit" tests for now without changing their code while I update a class & its test one by one.

Answer 1

You want to improve your design, that is going to require some effort, there is no way around that. You should do this in these following steps:

1. Set up DI container in your tests.

This is a must have, as if you want to start using the application container in tests, you need to have access to it. You should focus on solving this first (without actually having to alter any existing tests, yet).

2. Transform from top to bottom.

This is the least painful approach to gradually introduce dependency injection to a project. If you started from bottom, e.g. a repository, all services injecting the repository would need to be transformed, i.e. all controllers injecting those newly transformed services would need to be transformed,... If you however start by converting a controller to a bean, it's likely no other code injects the controller, and conversion is pretty much seamless.

This means that a controller looking like this (very naive example):

class SomeController {

    public SomeDataResponse getSomeData(SomeDataRequest request) {
        return SomeService.getInstance().getSomeData(request);
    }
}

would look like this after the change:

@ApplicationScoped
class SomeController {

    public SomeDataResponse getSomeData(SomeDataRequest request) {
        return SomeService.getInstance().getSomeData(request);
    }
}

making it a bean. This is all we focus on in this step, making beans.

3. Introduce bean proxy classes for singletons

Start creating proxy classes, which wrap your existing services as beans and gradually copy the API interface of the services you want to "beanize".

E.g. for this service class:

class SomeService {

    private static SomeService instance; 

    public static SomeService getInstance() {
        if (instance == null) {
            instance = new SomeService();
        }

        return instance;
    }

    public SomeDataResponse getSomeData() {
        // call other singletons and return data
    }
}

the following proxy could be created:

@ApplicationScoped
class SomeServiceBeanProxy {

    // it is EXTREMELY important the API of this bean proxy
    // 1:1 copies the API of the singleton
    public SomeDataResponse getSomeData() {
        return SomeService.getInstance().getSomeData();
    }
}

4. Refactor top level classes to use service bean proxies

The controller in question now becomes the following:

@ApplicationScoped
class SomeController {

    @Inject
    private SomeServiceBeanProxy someServiceBeanProxy;

    public SomeDataResponse getSomeData(SomeDataRequest request) {
        someServiceBeanProxy.getSomeData(request);
    }
}

(or you can use constructor injection, that is up to you), thanks to which the call to be bean proxy can now be easily mocked.

It is extremely important to define a strong convention about naming of services for injection at this point (necessary for final step). I would recommend choosing a member variable name based on the type for implementations, such as I have done in the example choosing someServiceBeanProxy name for a variable of SomeServiceBeanProxy type, and a similar thing when mocking in tests, with the exception of using a Mock suffix, e.g. someServiceBeanProxyMock.

5. Finalizing

Once all your classes depending initially on SomeService now depend on SomeServiceBeanProxy through injection, transform your SomeService to a bean:

@ApplicationScoped
class SomeService {

    public SomeDataResponse getSomeData() {
        // call other singletons and return data
    }
}

and through simple text find and replace, replace (case sensitive) all usages of bean proxy to direct service usage (which is now a bean, too):

• replace someServiceBeanProxyMock with someServiceMock,
• replace someServiceBeanProxy with someService,
• replace SomeServiceBeanProxy with SomeService.

If you defined SomeServiceBeanProxy in the same package as SomeService, even your imports should be correctly changed and you shouldn't be required to change anything else and your code would work.

The previous steps, such as creating a strong naming convention as well as directly copying the singleton's API in the bean proxy were all preparations so that you can then sweep the boring part of going through file by file by few simple find and replace operations.

You should now be able to delete the SomeServiceBeanProxy and can repeat the same mechanism with singletons on which the SomeService (now a bean) depends.

Answer 2

As a first step in migriting I would try to use the development ide (Eclipse / IntelliJ / ....) to help you:

In every Class or BaseClass that calls "ABCService.getInstance().doTheThing();"

in the java-editor select "ABCService.getInstance().doTheThing();" and call ide function "Refactor selected statement to member method" and give this method the name "doTheThing".

This way you have only one place per class (or classhirarchy) that calls ABCService.getInstance()