TDD Series: How to test-drive decorators - adding same logic to all wrapped methods

Hi, today we're gonna talk a little bit about test driving decorators. In particular I'm going to take on a case where we are wrapping all methods of decorated object with the same logic (a good example is synchronization or tracing).

Synchronized decorator - a simple example

The example I'm gonna give is very simplistic, however, there's no need to complicate it, since all more complex objects follow the same pattern.

Let's imagine that we're creating a stack. We support the following operations:

1. put element on top of a stack ("push element")
2. get last put element from stack without removing it
3. get last put element from stack and remove it ("pop element")
4. clear the stack
5. copy all elements of the stack on top of another stack

Now, let's imagine we have already test-driven an implementation that follows this interface:

public interface Stack<T>
{
  void Push(T element);
  T GetTopElement();
  T PopElement();
  void Clear();
  void CopyElementsOnTopOf(Stack<T> anotherStack);
}

Everything's fine until a new project is started that handles multiple simultaneous requests made to the stack object. So, we need to add synchronization to the stack.

If you've read other posts on this blog, you know that we like cohesion and Single Responsibility Principle very much and tend not to break it. That's why baking in concurrency inside the stack class (which already has a responsibility of managing storage and access to the elements) is not an option. After considering all pros and cons, we decide to make a synchronized wrapper that would implement Stack interface and route each method call to the original stack with synchronization added. Also, to make things more efficient (by the way, it doesn't always make things more efficient, but let's assume that it does in our case), we decide to use reader-writer lock (i.e. when writer lock is obtained, no one else can use the object. When reader lock is obtained, no one can obtain the writer lock, but other readers can use the object simultaneously).

How do we do it?

First, classify the operations

Looking at the method names, it's pretty evident which are "write" operations and should be guarded by writer lock and which are "read" operations and should be guarded by reader lock.

The "write" operations are:

1. put element on top of a stack ("push element")
2. get last put element from stack and remove it ("pop element")
3. clear the stack

And the "read" operations are:

1. get last put element from stack without removing it
2. copy all elements of the stack on top of another stack

Ok, done. What's the next step?

Remember - no redundancy.

What we'd like to be able to do is to write the result of this analysis in the form of executable specifications AKA unit tests. So, for example, such specification would say: "putting an element on top of a stack should perform the same operation of wrapped object, with write access acquired". The same would go for all other "write" operations. So, not to be redundant, we'd like to encapsulate the "write access acquired" into its own method (and maybe in its own object, but let's leave that alone for now).

Also, from what we know about synchronization, the workflow for performing operations in context of a lock is like this (example for writing):

1. acquire write lock
2. perform operation
3. release write lock

What we'd like to do is to leave the middle step varying while encapsulating the first and last step (i.e. a context of the operation) into one method. There are at least three ways to do it in C# (pass the middle step as parameter to lambda expression or implement IDisposable and use using construct or use some funny hacking that will be a topic of a separate post ;-)), of which we'll choose the lambda solution (since it's applicable in other languages as well, so I expect to draw the most value for you - my readers - from it).

Applying the same reasoning to the "read" methods, we can come up with the following interface of the synchronization decorator:

public class SynchronizedStack<T> : Stack<T>
{
  public void Push(T element);
  public T GetTopElement();
  public T PopElement();
  public void Clear();
  public void CopyElementsOnTopOf(Stack<T> anotherStack);
  
  //New operations not in Stack<T> interface:
  public SynchronizedStack(Stack<T> stack);
  public virtual void InWriteContext(Action action);
  public virtual void InReadContext(Action action);

  //This is the read-write lock we were talking about.
  public readonly ReaderWriterLockSlim SyncObject 
    = new ReaderWriterLockSlim(LockRecursionPolicy.SupportsRecursion);

}

Now, for each method, we should write two specs. In the following example, please note how we're going to deal with verifying that a method was called in a lambda expression that's passed to another method (this is a situation where matchers built inside mock object frameworks won't suffice, at least not in C#). The example is for PopElement():

[Test]
public void ShouldPopElementInWriterContextIfWriterContextImplemented()
{
  // GIVEN
  var anyPoppedElement = Any.Integer();
  var wrappedStack = new Mock<Stack<int>>();
  var synchronizedStack 
    = new Mock<SynchronizedStack<int>>(wrappedStack.Object);
  wrappedStack.Setup(m => m.PopElement()).Returns(anyPoppedElement);

  //Any action passed to write context will just be executed
  synchronizedStack
    .Setup(m => m.InWriteContext(It.IsAny<Action>()))
    .Callback<Action>(a => a.Invoke());

  // WHEN
  var element = synchronizedStack.Object.PopElement();

  // THEN
  Assert.AreEqual(anyPoppedElement, element);
}

[Test]
public void 
ShouldNotPopElementInWriterContextIfWriterContextNotImplemented()
{
  // GIVEN
  var wrappedStack = new Mock<Stack<int>>();
  var synchronizedStack 
    = new Mock<SynchronizedStack<int>>(wrappedStack.Object);

  // Any action passed to write context will 
  // not be executed - empty implementation:
  synchronizedStack
    .Setup(m => m.InWriteContext(It.IsAny<Action>()))
    .Callback(() => {});

  // WHEN
  synchronizedStack.Object.PopElement();

  // THEN
  wrappedStack.Verify(m => m.PopElement(), Times.Never());
}

The whole trick is about providing a different implementation for InWriteContext() each time: in the first spec, the implementation is just a pass-through and invokes everything it gets. In the second spec, the implementation ignores anything it gets. So, if in first spec the PopElement() of wrapped object gets called and in the second one it doesn't, it must go through the InWriteContext(). Also note that in the first spec I don't use Verify() - that's ok since I've got the return value PopElement()'s result to assert against and check the call this way.

Ok, what more do we need? We have certainly omitted a spec for the SyncObject field (I decided to use a field instead of property, but you may choose otherwise), where it should be specified that it should be a non-null object and that it should support recursion (that's my design choice, yours may be different). Also, we need to somehow specify the InWriteContext() and InReadContext(). Each of them needs two specs:

1. The proper lock should be acquired before the call and release it after the call finishes successfully
2. If any exception is thrown by the action executed in context, the lock should be released anyway

Let's go with an example for InWriteContext()

[Test]
public void 
ShouldObtainWriterLockDuringTheCallAndReleaseItAfterTheCall()
{
  // GIVEN
  var wrappedStack = new Mock<Stack<int>>();
  var synchronizedStack 
    = new SynchronizedStack<int>(wrappedStack.Object);
  var gotCalled = false;

  // WHEN
  synchronizedStack.InWriteContext(() => 
  {
    gotCalled = true;
    Assert.IsTrue(synchronizedStack.SyncObject.IsWriteLockHeld);
    Assert.IsFalse(synchronizedStack.SyncObject.IsReadLockHeld);
  });

  // THEN
  Assert.IsTrue(gotCalled);
  Assert.IsFalse(synchronizedStack.SyncObject.IsWriteLockHeld);
  Assert.IsFalse(synchronizedStack.SyncObject.IsReadLockHeld);
}

[Test]
public void 
ShouldReleaseWriteLockWhenAWrappedCallExecutionEndsWithException()
{
  // GIVEN
  var wrappedStack = new Mock<Stack<int>>();
  var synchronizedStack 
    = new SynchronizedStack<int>(wrappedStack.Object);

  // WHEN
  Assert.Throws<Exception>( () =>
    synchronizedStack.InWriteContext(() => {throw new Exception();})
  );

  // THEN  
  Assert.IsFalse(synchronizedStack.SyncObject.IsWriteLockHeld);
  Assert.IsFalse(synchronizedStack.SyncObject.IsReadLockHeld);
}

Is it worth the hassle?

"Well", you might say, "it's two specs for each InXContext() plus two per each wrapped method. Isn't it too much for a wrapper that's implemented almost mechanically?". And you might be right. This is a case where I'd weigh pros against cons - you might decide not to test-drive this code, since the only value added by such tests is of the "living documentation" sort. I usually do it, but if you decide it's stupid, I won't blame you (much :-)). Another reason why I decided it's a good blog post is that it provides one way of specifying that something goes through lambda, which are a real horror for mock frameworks to match (you can't just say: match every method with argument being a lambda like THIS, at least not always). You could decide to go another way: take the specs we've written for InWriterContext() as a template, and use it for every method, just ignoring the existence of lambda and InXContext() methods and focus on the synchronization in each spec, but that would be potentially harder to maintain.

In one of my future posts, I'll show you how to achieve one spec per wrapped method instead of two by using some C# magic, but for now, that's it.

Good night, everyone!