A design that is both too simple and too complex at the same time

| 0 Comments
Except of course, the refactored filters can't actually be layered that well using the design that I outlined in the previous blog posting.

The main problem, the "too simple" part of the design is that, well, it just doesn't work. The filter chain needs to be walked in one direction for request filtering and the opposite direction for completion processing and the previous design was only a singly linked list (and so was always walked in the same direction). The tests didn't check for the order that the filters were accessed in and so passed although they failed (if you see what I mean). The reason that the ordering is important is that if you have layered filters on top of each other then you need to allow them access to the byte stream in one order to process bytes on the way out of the program and the opposite order to process the bytes on the way in; imagine a compression filter layered on an SSL filter, when writing data out you compress then encrypt and when reading data in you decrypt then decompress...

The "too complex" part is that we're relying on the authors of the filters to 'do the right thing' with regards to managing the filter chain traversal. The fact that we let them know they're in a chain and allow them access to the next filter in the chain so that they can explicitly call it seems a little dangerous, also it means that for most filters there's an extra call in every method body that's just 'boiler plate' and if it's forgotten then the filter will fail to work...

The first thing to do is change the tests so that they show the problem, this is relatively easy, at present the tests look something like this:

   CTestSocketServer server;
  
   const CAddressIPv4 address(_T("localhost"), server.GetPort());
  
   {
      CSmartStreamSocket socket = manager.ConnectNoThrow(address);
  
      THROW_ON_FAILURE(functionName, true == server.WaitForAccept(1000));
  
      ioPool.CheckResult(_T("|AssociateDevice|"));
  
      manager.CheckResult(_T("|OnPreOutgoingConnect|OnConnectionEstablished|"));
  
      filter1.CheckResult(_T("|FilterConnect: SyncNoThrow|FilterConnectionEstablished|"));
      filter2.CheckResult(_T("|FilterConnect: SyncNoThrow|FilterConnectionEstablished|"));

and they need to look a bit more like this:

   CTestSocketServer server;
  
   const CAddressIPv4 address(_T("localhost"), server.GetPort());
  
   {
      CSmartStreamSocket socket = manager.ConnectNoThrow(address);
  
      THROW_ON_FAILURE(functionName, true == server.WaitForAccept(1000));
  
      ioPool.CheckResult(_T("|AssociateDevice|"));
  
      manager.CheckResult(_T("|OnPreOutgoingConnect|OnConnectionEstablished|"));
  
      filters.CheckResult(_T("|2:FilterConnect: SyncNoThrow|1:FilterConnect: SyncNoThrow|1:FilterConnectionEstablished|2:FilterConnectionEstablished|"));
   }

By sharing a log between the filters and numbering the filter log messages we can ensure that the ordering of the calls to them is how we want it...

Now the tests fail...

To deal with the fact that we need to process the filter chain in both directions, and to remove the complexity of having the filter itself pass the call to the next filter in line we can have the connection manager maintain a std::list of filters. This allows it to traverse the list in either direction so that layering actually works! Since the connection manager now manages the list traversal we need a way for each filter to 'swallow' calls; before they would simply fail to pass the call along the chain, now they need to tell the connection manager to stop processing the chain. If we do this by passing in a reference to an enum which has two values, continue filtering and stop filtering, and where the value passed in is initially to continue filtering then the filter chain can be processed until one of the filters changes the enum to indicate that filtering should end for this operation. Now most filters can leave this enum alone and everything will 'just work'.

This all changes the filtering interface a bit but the result is simpler to implement, simpler to document and what's more it actually does the intended job...

Leave a comment