How to implement Chain of Responsibility in a testable fashion?

Question

I'd like to implement a Chain of Responsibility pattern, taking care of the 'broken link' problem as follows:

 public abstract class Handler{

   private Handler m_successor;

   public void setSuccessor(Handler successor)
   {
     m_successor = successor;
   }

   protected abstract boolean handleRequestImpl(Request request);

   public final void handleRequest(Request request)
   {
     boolean handledByThisNode = this.handleRequestImpl(request);
     if (m_successor != null && !handledByThisNode)
     {
       m_successor.handleRequest(request);
     }
   }
 }

Seems like a common enough approach. But how is this testable with a protected abstract method? Ways to handle this seem to be:

1. Implement a test-only subclass of Handler that implements the abstract method. This seems bad for test maintenance.
2. Change the visibility of the abstract method to public, but I shouldn't need to change the SUT to accomodate the test.
3. Regard the abstract class as sufficiently simple to not require unit tests. Mmmm.
4. Implement unit tests for the handleRequest method on one or more concrete subclasses. But this doesn't seem like a sensible way to organize tests.
5. Is there someway to use a mock object? I've tried Mockito, but can't seem to get around the protected visibility.

I've read[1] that this sort of testing problem implies that the design is wrong, and suggest using composition rather than inheritence. I'm trying this now, but it seems strange that the recommended implementation of this pattern has this problem, yet I can't find any advice about unit testing it.

UPDATED: I've replaced the abstract class with dependency inversion as shown, and this is now easily easily testable using Mockito. It still looks like Chain of Responsibility... am I missing something?

// Implement a concrete class instead
public class ChainLink {

  // Successor as before, but with new class type
  private ChainLink m_successor;

  // New type, RequestHandler
  private RequestHandler m_handler;

  // Constructor, with RequestHandler injected
  public ChainLink(RequestHandler m_handler) {
    this.m_handler = m_handler;
  }

  // Setter as before, but with new class type
  public void setSuccessor(ChainLink successor) {
    m_successor = successor;
  }

  public final void handleRequest(Request request) {
    boolean handledByThisNode = m_handler.handleRequest(request);
    if (m_successor != null && !handledByThisNode) {
      m_successor.handleRequest(request);
    }
  }
}

Answer 1

If you use PowerMock + Mockito, you can write a test similar to this:

@RunWith(PowerMockRunner.class)
@PrepareForTest(Tests.class)
public class Tests {
    @Test
    public void testHandledByFirst() throws Exception {
        Request req = ...;
        Handler h1 = mock(Handler.class);
        Handler h2 = mock(Handler.class);

        when(h1, "setSuccessor", h2).thenCallRealMethod();
        when(h1, "handleRequestImpl", req).thenReturn(true);

        h1.setSuccessor(h2);
        h1.handleRequest(req);
        verify(h2, times(0)).handleRequest(req);
    }

    @Test
    public void testHandledBySecond() throws Exception {
        Request req = ...;
        Handler h1 = mock(Handler.class);
        Handler h2 = mock(Handler.class);

        when(h1, "setSuccessor", h2).thenCallRealMethod();
        when(h1, "handleRequestImpl", req).thenReturn(false);
        h1.setSuccessor(h2);

        h1.handleRequest(req);
        verify(h2, times(1)).handleRequest(req);
    }
}

Which will verify that your second handler's method is called when the first one returns false and that it is not called when it returns true.

Another option is to follow the well-known rule of "favor composition over inheritance" and change your class to something like this:

public interface Callable {
    public boolean call(Request request);
}

public class Handler {
    private Callable thisCallable;
    private Callable nextCallable;

    public Handler(Callable thisCallable, Callable nextCallable) {
        this.thisCallable = thisCallable;
        this.nextCallable = nextCallable;
    }

    public boolean handle(Request request) {
        return thisCallable.call(request) 
            || (nextCallable != null && nextCallable.call(request));
    }
}

Then you can mock it in this way (or use pretty much any mock framework since you don't have protected methods):

@RunWith(PowerMockRunner.class)
@PrepareForTest(Tests.class)
public class Tests {
    @Test
    public void testHandledByFirst() throws Exception {
        Request req = ...;
        Callable c1 = mock(Callable.class);
        Callable c2 = mock(Callable.class);
        Handler handler = new Handler(c1, c2);

        when(c1.call(req)).thenReturn(true);

        handler.handle(req);

        verify(c1, times(1)).call(req);
        verify(c2, times(0)).call(req);
    }

    @Test
    public void testHandledBySecond() throws Exception {
        Request req = ...;
        Callable c1 = mock(Callable.class);
        Callable c2 = mock(Callable.class);
        Handler handler = new Handler(c1, c2);

        when(c1.call(req)).thenReturn(false);

        handler.handle(req);

        verify(c1, times(1)).call(req);
        verify(c2, times(1)).call(req);
    }
}

In this solution, you can also make Handler inherit after Callable and then you are able to wrap it over any other callable which may have a successor and use the handler instead of the original callable; it is much more flexible.

Answer 2

I would go for option 1. That fake sub class is simple enough and you can place it in your tets class. There is nothing wrong with test-only sub classes.