Nancy + rest mow2012
View more presentations from Christian Horsdal.
Thursday, April 19, 2012
Slides from my MOW2012 Talk
I did talk at MOW2012 about REST and Nancy today. The code in the talk is taking from my port of RestBucks On .NET to Nancy, and here are the slide:
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):
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:
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:
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:
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:
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:
(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)
That's it. :-)
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):
11 public class DeleteDeviceWriteRequest : DataStoreWriteRequest
12 {
13 private readonly Device device;
14
15 public DeleteDeviceWriteRequest(Device device)
16 {
17 this.device = device;
18 }
19
20 protected override void 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:
229 var deleteDeviceRequest = new DeleteDeviceWriteRequest(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:
8 public interface IDataStore
9 {
10 void Execute(DataStoreWriteRequest req);
11 T Execute<T>(DataStoreReadRequest<T> req);
12 T Get<T>(long id) where T : class;
13 T Get<T>(Expression<Func<T,bool>> where) where T : class;
14 void Add<T>(T entity) where T : class;
15 void Delete<T>(T entity) where T : class;
16 int Count<T>(Expression<Func<T, bool>> where = null) where T : class;
17 }
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:
173 public void Execute(DataStoreWriteRequest req)
174 {
175 WithTryCatch(() => req.ExecuteWith(Session));
176 }
177
178 public T Execute<T>(DataStoreReadRequest<T> req)
179 {
180 return WithTryCatch(() => req.ExecuteWith(Session));
181 }
182
183 private void WithTryCatch(Action operation)
184 {
185 WithTryCatch(() => { operation(); return 0; });
186 }
187
188 private TResult WithTryCatch<TResult>(Func<TResult> operation)
189 {
190 try
191 {
192 return operation();
193 }
194 catch (Exception)
195 {
196 Dispose(); // ISession must be disposed
197 throw;
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:
5 public abstract class DataStoreWriteRequest
6 {
7 protected ISession Session { get; private set; }
8
9 internal void ExecuteWith(ISession seesion)
10 {
11 Session = seesion;
12 DoExecute();
13 }
14
15 protected abstract void 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]
40 public void ShouldDeleteDeviceWithNoRelations()
41 {
42 var device = new Device();
43 using (var arrangeUnitOfWork = CreateUnitOfWork())
44 {
45 arrangeUnitOfWork.Add(device);
46 }
47
48 using (var actUnitOfWork = CreateUnitOfWork())
49 {
50 var sut = new DeleteDeviceWriteRequest(device);
51 actUnitOfWork.Execute(sut);
52 }
53
54 using (var assertUnitOfWork = CreateUnitOfWork())
55 {
56 Assert.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]
101 public void DeleteDeviceRestCallExectuesDeleteOnDevice()
102 {
103 var dataStore = mock.StrictMock<IDataStore>();
104 var sut = new RestApi(dataStore);
105
106 var device = new Device { Id = 123 };
107
108 Expect.Call(unitOfWork.Get<Device>(device.Id)).Return(device);
109 Expect.Call(() =>
110 unitOfWork.Execute(
111 Arg<DeleteMeterWriteRequest>
112 .Is.Equal(new DeleteMeterWriteRequest(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)
That's it. :-)
Labels:
.NET,
C#,
Repository,
software design
Monday, January 30, 2012
Repositories and Single Responsibility from the Trenches
In a project I'm involved with we've done what I suspect lots and lots of projects do. We've used the repository pattern to encapsulate database adds, queries and deletes. We have one such repository per entity type. That worked well for me in earlier projects. But in this case several of the repositories have become somewhat unfocused. None of them are big hairy monsters, but the different public methods really don't have much in common except the fact that they mainly act on the same entity type. In other words cohesion is low. Let me exemplify (anonymized just as the rest of the code in this post):
This doesn't look like the interface of a class that has only one reason to change - i.e. it violates the single responsibility principle (SRP). This is in part because it in fact does not only act on one entity type but on a hierarchy of entity types; device is a base class and the are a handful of concrete types of devices. The obvious solution to that, is to move away from a repository for the base class to a number of repositories for the concrete entities - but that's where we came from. That's a route that led to duplicated code, which led to abstracting into a common device repository, so we did not want to go back down that route again.
Taking Single Responsibility Seriously
The solution we've come up with is - at it's core - simply to take SRP seriously: We want classes that do not do a bunch of database related operations around a type of enity. We want classes that does just one such operation. Take the Delete method on the interface above. That seems innocent enough, but it turns out that the implemetation is a bit complicated: There are a number of relations that need to be untied, and there is some cascading to take care of before the device can be deleted from the database. As a colleague commented: "It looks like a bad stored proc, written in C#". We decided to view that delete operation as "one thing", and decided it justified a class of its own:
The only thing public here is the constructor. The contructor takes all arguments for the delete, so once the object is constructed it completely incapsulates the delete. And - importantly - it is incapable of doing anything else than that particular delete.
How is the delete executed? -Through an execute method on the base class NHibernateWriteRequest.
This swaps the repository pattern for the command pattern plus the template method pattern. But more importantly this swaps a small collection of ever growing repositories for a large collection of small never growing query classes (which, btw, also plays better into the open/closed principle). We've only just started down this path, but I think we'll be happy about it.
There are a few twists I've left out, particularly around making these request objects easy to get at for client code, while maintaining encapsulation and testability. I might revisit those in a future post.
9 public interface IDeviceRepository
10 {
11 TDeviceType Get<TDeviceType>(long id) where TDeviceType : Device;
12 TDeviceType FindByControllerIdentifier<TDeviceType>(string controllerIdentifer)
13 where TDeviceType : Device;
14
15 IList<TDeviceType> GetAll<TDeviceType>(int offset, int max,
16 DateTimeOffset? createdAfter = null, DateTimeOffset? updatedAfter = null,
17 DateTimeOffset? disconnectedAfter = null)
18 where TDeviceType : Device;
19
20 int Count<TDeviceType>(DateTimeOffset? createdAfter = null,
21 DateTimeOffset? updatedAfter = null, DateTimeOffset? disconnectedAfter = null)
22 where TDeviceType: Device;
23
24 void Add<TDeviceType>(TDeviceType device) where TDeviceType : Device;
25 IList<TDeviceType> GetAll<TDeviceType>() where TDeviceType : Device;
26 IList<TDeviceType> GetAll<TDeviceType>(long[] deviceIds) where TDeviceType : Device;
27 IList<TDeviceType> FindByControllerIdentifiers<TDeviceType>(string[] controllerIdentifiers)
28 where TDeviceType : Device;
29 IList<NodeDevice> FindNodesDevicesForRootDevice(string controllerIdentifier,
30 bool active = false);
31 ICollection<NodeDevice> FindNodeDevicesForRootDevice(long rootDeviceId, bool active = false);
32
33 IEnumerable<DeviceLink> GetTopology<T>(long id, bool active) where T : Device;
34 DeviceLink GetDeviceLink(string controllerIdentifierA, string controllerIdentifierB);
35 DeviceLink GetDeviceLink(long id);
36
37 void AddDeviceLink(DeviceLink link);
38
39 IList<Route> FindRoutesByDeviceLink(DeviceLink deviceLink);
40 void Delete(Device device);
41 }
This doesn't look like the interface of a class that has only one reason to change - i.e. it violates the single responsibility principle (SRP). This is in part because it in fact does not only act on one entity type but on a hierarchy of entity types; device is a base class and the are a handful of concrete types of devices. The obvious solution to that, is to move away from a repository for the base class to a number of repositories for the concrete entities - but that's where we came from. That's a route that led to duplicated code, which led to abstracting into a common device repository, so we did not want to go back down that route again.
Taking Single Responsibility Seriously
The solution we've come up with is - at it's core - simply to take SRP seriously: We want classes that do not do a bunch of database related operations around a type of enity. We want classes that does just one such operation. Take the Delete method on the interface above. That seems innocent enough, but it turns out that the implemetation is a bit complicated: There are a number of relations that need to be untied, and there is some cascading to take care of before the device can be deleted from the database. As a colleague commented: "It looks like a bad stored proc, written in C#". We decided to view that delete operation as "one thing", and decided it justified a class of its own:
11 public class DeleteDeviceRequest : NHibernateWriteRequest
12 {
13 private readonly Device device;
14
15 public DeleteDeviceRequest(Device device)
16 {
17 this.device = device;
18 }
19
20 protected override void DoExecute()
21 {
22 var links =
23 Session.QueryOver<DeviceLinkReference>().Where(x => x.ControllerIdentifier == device.ControllerIdentifier).
24 List();
25
26 links.ForEach(x => x.Device = null);
27
28 DeleteStatistics();
29 new DeleteRoutesWriteRequest(device.Routes).ExecuteWith(DataStore);
30 DeleteFromGroup();
31 DeleteFromOrder();
32 DeleteProfiles();
33 DataStore.Delete(device);
34 }
35
36 // more private methods...
37
38
The only thing public here is the constructor. The contructor takes all arguments for the delete, so once the object is constructed it completely incapsulates the delete. And - importantly - it is incapable of doing anything else than that particular delete.
How is the delete executed? -Through an execute method on the base class NHibernateWriteRequest.
This swaps the repository pattern for the command pattern plus the template method pattern. But more importantly this swaps a small collection of ever growing repositories for a large collection of small never growing query classes (which, btw, also plays better into the open/closed principle). We've only just started down this path, but I think we'll be happy about it.
There are a few twists I've left out, particularly around making these request objects easy to get at for client code, while maintaining encapsulation and testability. I might revisit those in a future post.
Labels:
.NET,
C#,
Repository,
software design,
SOLID
Subscribe to:
Posts (Atom)