Replacing an application service class with multiple command handlers

Question

I have a Controller which uses an application service to accomplish its tasks. The service class is starting to grow big and developing multiple dependencies. So I am thinking of replacing the single big service class with multiple command handlers, each handling a single responsibility. This is how the old and the new code will look like (simplified):

Old Code

@Component
@Path("/app/jobs")
public class JobController {

    @GET
    @Produces(MediaType.APPLICATION_JSON)
    public List<JobDto> getJobs() throws JobException {
         return jobService.getJobs();
    }

    @PUT
    @Path("{job_id}/assigned_to/{user_id}")
    public Response assignTo(
            @PathParam("job_id") @NotNull String jobId,
            @PathParam("user_id") @NotNull String userId
            ) throws JobException {

        jobService.assignJobTo(jobId, userId);
        return Response.status(Response.Status.CREATED).build();
    }
    ...
}

New Code

@Path("/app/jobs")
public class JobController {

    @GET
    @Produces(MediaType.APPLICATION_JSON)
    public List<JobDto> getJobs() throws JobException {
         return new GetJobListCommandHandler(
                new GetJobListCommand()
        ).handle();
    }

    @PUT
    @Path("{job_id}/assigned_to/{user_id}")
    public Response assignTo(
            @PathParam("job_id") @NotNull String jobId,
            @PathParam("user_id") @NotNull String userId
            ) throws JobException {

        new AssignJobCommandHandler(
                new AssignJobCommand(jobId, userId)
        ).handle();
        return Response.status(Response.Status.CREATED).build();
    }
    ...
}

I am basically following this tutorial in using command handlers. This patterns isolates each responsibility into one class which is great. But now, since my controller methods create the command handler objects, the handlers have become difficult to mock in unit tests. How do I write unit tests for the controller? Earlier the controller class had just a mocked version of the service object. One option could be to break the controller to service one uri (GetJobsController, AssignJobController, etc). But I don't want to do that. I like seeing the hierarchy of resources together and the path annotations work well like this. Is this the right way of going about this? What are the best practices or this approach and how do I write my unit test?

My application is a Spring+Jersey application.

Answer 1

What's wrong

First of all read the tutorial again, you missed his whole point.

You cannot use decorators with your current design. Why?

new AssignJobCommandHandler(
        new AssignJobCommand(jobId, userId)
).handle();

• You use new therefore you cannot use a decorated version. You have a AssignJobCommandHandler and it is not a TransactionCommandHandlerDecorator.

• You pass the command to the constructor. Constructors are not polymorphic. You cannot customize how a command is handle if you pass it to a constructor, (or equivalently a static method)

• You don't pass the dependencies in the CommandHandlers, so you would have to use the Service Locator, Evil Singleton anti-patterns. I'm guessing it doesn't currently work at all.

Getters are not command handlers. A request is either a query (GET, etc) or a command (POST, etc), never both. Make sure your service methods follow CQS. Don't do this:

return new GetJobListCommandHandler(
                new GetJobListCommand()
        ).handle();

What to do

Depend on abstractions.

Create said the interface:

public interface CommandHandler<TCommand> {
    void handle(TCommand command);
}

and use it like this.

@Path("/app/jobs")
public class AssignJobController {

    @Autowired
    CommandHandler<AssignJobCommand> handler;

    @PUT
    @Path("{job_id}/assigned_to/{user_id}")
    public Response assignTo(
            @PathParam("job_id") @NotNull String jobId,
            @PathParam("user_id") @NotNull String userId
            ) throws JobException {

        handler.handle(new AssignJobCommand(jobId, userId));

        return Response.status(Response.Status.CREATED).build();
    }
}

You can use decorators like this, for example:

@Configuration
public class Config {
    @Autowired
    JobDAO jobDAO;

    @Bean
    public CommandHandler<AssignJobCommand> assignJobCommandHandler() {
        // pass in the dependencies as shown in the tutorial
        // I use a DAO instead of a UnitOfWork, as they are more common with Spring applications.
        CommandHandler<AssignJobCommand> handler = new AssignJobCommandHandler(jobDAO);


        // you can use multiple decorators
        // this kind of decorators do not need to use reflection, proxies, bytecode manipulation etc magic. They are plain-old java objects.
        handler = new TransactionCommandHandlerDecorator(handler);
        handler = new DeadlockRetryCommandHandlerDecorator(handler);
        ...
        ...
        return handler;
    }
}

Testing

Now that we pass the dependencies in the handlers, we can test them.

One way to do it is using a mocking framework, there are others.

Test may look like this, for example:

@RunWith(MockitoJUnitRunner.class)
public class AssignJobCommandHandlerTest {

    @Mock
    JobDAO jobDAO;

    AssignJobCommandHandler handler;

    @Before
    public void setUp() {
        handler = AssignJobCommandHandler(jobDAO);
    }

    @Test
    public void assignsAJobToAUser() {
        long jobId = 123L;
        long userId = 456L;

        Job job = new Job();

        when(jobDAO.getJob(jobId)).thenReturn(job);

        handler.handle(new AssignJobCommand(jobId, userId));

        assertThat(job.getAssignedUserId(), is(userId));
    }
}

Answer 2

Following the TDD guidelines you should be able to do the refactor without having to modify the tests, or at least not so much... so I personally wouldn't go for this solution.

My point of view

• First I would try to split the controller in several ones. Trying to allocate together the same functionality. Ex. JobController and JobAssignmentController
• In the same way I would be splitting the service and take advantage of DI of Spring.

I think with this you are not going to have the problem of a "God Object" or the problem or testing.