Pages

Friday, February 24, 2012

Repositories and Single Responsibility from the Trenches - Part II

In my last post I wrote about how we swapped sa few ever-growing repository classes for a lot of small focused requests classes, thar each take core of one request to our database. That post showed the gist of how to implement these request classes. This post will focus on how to use them, test them and mock them.

First I'll re-iterate how one of these request classes look (NB. compared to the code from the last post the base class has been renamed):

11publicclassDeleteDeviceWriteRequest : DataStoreWriteRequest

12 {

13privatereadonlyDevice device;

14

15public DeleteDeviceWriteRequest(Device device)

16 {

17this.device = device;

18 }

19

20protectedoverridevoid DoExecute()

21 {

22// NHibernate trickery cut out for brewity

23 Session.Delete(device);

24 }

25 }

And below I show how to use, test and mock this class.

Usage
Usage of these requests is really simple: You just new one up, and ask your local data store object to execute it:

229var deleteDeviceRequest = newDeleteDeviceWriteRequest(meter);

230 dataStore.Execute(deleteDeviceRequest);

Say, what? I'm newing up an object that uses NHibernate directly. Gasp. That's a hard coupling to the ORM and to the database, isn't it? Well, kind of. But that is where the data store object comes into play: The request can only be exectuted through that object, because the request's only public method is its contructor and because it's base class 'DataStoreWriteRequest' has no public methods. The interface for that data store is:

That could be implemented towards any database/ORM. I our case it's implemented against NHibernate, and is pretty standard except maybe for the two execute methods - but then again they turn out to be really straightforward as well:

173publicvoid Execute(DataStoreWriteRequest req)

174 {

175 WithTryCatch(() => req.ExecuteWith(Session));

176 }

177

178public T Execute<T>(DataStoreReadRequest<T> req)

179 {

180return WithTryCatch(() => req.ExecuteWith(Session));

181 }

182

183privatevoid WithTryCatch(Action operation)

184 {

185 WithTryCatch(() => { operation(); return 0; });

186 }

187

188private TResult WithTryCatch<TResult>(Func<TResult> operation)

189 {

190try

191 {

192return operation();

193 }

194catch (Exception)

195 {

196 Dispose(); // ISession must be disposed

197throw;

198 }

199 }

Notice the calls to ExecuteWith? Those are calls to internal methods on the abstract DataStoreReadRequest and DataStoreWriteRequest classes. In fact those internal methods are the reason that DataStoreReadRequest and DataStoreWriteRequest exists. Using a template method declared internal they provide inheritors - the concrete data base requests - a way to get executed, while hiding everything but the contrustors from client code. Only our NHibernate implementation of IDataStore ever calls the ExecuteWith methods. All the code outside the our data access assembly can not even see those methods. As it turns out this is really simple code as well:

5publicabstractclassDataStoreWriteRequest

6 {

7protectedISession Session { get; privateset; }

8

9internalvoid ExecuteWith(ISession seesion)

10 {

11 Session = seesion;

12 DoExecute();

13 }

14

15protectedabstractvoid DoExecute();

16 }

To sum up; the client code just news the requests it needs, and then hands them off to the data store object. Simple. Requests only expose constructors to the client code, nothing else. Simple.

Testing the Requests

Testing the requests individually is as simple as using them. This is no surprise since tests - as we know - are just the first clients. The tests do whatever setup of the database they need, then new up the request and the data store, asks the data store to execute the request, and then asserts. Simple. Just like the client production code.

In fact one of the big wins with this design over our old repositories is that the tests become a whole lot simpler: Although you can split up tests classes in many ways, the reality for us (as I suspect it is for many others too) is that we tend to have one test class per production class. Sometimes two, but almost never three or more. Since the repository classes grew and grew so did the corresponding test classes resulting in some quite hairy setup code. With the new design each test class tests just one very specific request leading to much, much more cohesive test code.

To illustrate here is the first simple of test for the above DeleteDeviceRequest - note that the UnitOfWork objects in this test implement IDataStore:

39 [Test]

40publicvoid ShouldDeleteDeviceWithNoRelations()

41 {

42var device = newDevice();

43using (var arrangeUnitOfWork = CreateUnitOfWork())

44 {

45 arrangeUnitOfWork.Add(device);

46 }

47

48using (var actUnitOfWork = CreateUnitOfWork())

49 {

50var sut = newDeleteDeviceWriteRequest(device);

51 actUnitOfWork.Execute(sut);

52 }

53

54using (var assertUnitOfWork = CreateUnitOfWork())

55 {

56Assert.That(assertUnitOfWork.Get<Device>(device.Id), Is.Null);

57 }

58 }

Mocking the Request

The other part of testing is testing the code that uses these requests; testing the client code. For those tests we don't want the request to be executed, since we don't want to get slowed down by those tests hitting the database. No, we want to mock the requests out completely. But there is a catch: The code under test like the code in the first snippet in the Usage section above new's up the request. That's a hard compile time coupling to the concrete request class. There is no seam allowing us to swap the implementation. What we're doing about this is that we're sidestepping the problem, by mocking the data store object instead. That allows us redifne what executing the reqeust means: Our mock data store never executes any of the requests it's asked to execute, it just records that it was asked to execute a certain request, and in the case of read requests returns whatever object we set it up to return. So the data store is our seam. The data store is never newed up directly in production code, it's always injected through constructors. Either by the IoC/DI container or by tests as here:

100 [Test]

101publicvoid DeleteDeviceRestCallExectuesDeleteOnDevice()

102 {

103var dataStore = mock.StrictMock<IDataStore>();

104var sut = newRestApi(dataStore);

105

106var device = newDevice { Id = 123 };

107

108Expect.Call(unitOfWork.Get<Device>(device.Id)).Return(device);

109Expect.Call(() =>

110 unitOfWork.Execute(

111Arg<DeleteMeterWriteRequest>

112 .Is.Equal(newDeleteMeterWriteRequest(device))));

113

114 mock.ReplayAll();

115

116 sut.DeleteMeter(device.Id.ToString());

117

118 mock.VerifyAll();

119 }

120

(the above code uses Rhino.Mocks to mock out the data store, but that's could have been done quite simply by hand as well or by any other mocking library)

2 comments:

Interesting post. I looked at this approach recently and I liked. I used a variation of it though. Passing the request in the constructor wouldn't allow me to inject it into a controller (as you pointed out), so did something similar to what Service Stack does(#2 in https://github.com/ServiceStack/ServiceStack/wiki/Create-your-first-webservice) and pass in the request into the method call. This exposes the execute call, but you can inject it(and mock for testing from the caller if needed). How are you handling queries that return results?

We handle read queries with a type-parameterized request base class: "DataStoreReadRequest" which has an "ExecuteWith" method that returns a "T" and an abstract "DoExecute" that also returns a "T". The "ExecuteWith" is called from the implementation of "T Execute(DataStoreReadRequest req)" method on "IDataStore".