Implementing a Diameter Peer State Machine using Test Driven Development (Part 3).

Just a brief riepilogue of what are the missing points :

- When a peer is in "Closed" state and receives a START signal it must

1. take a I-Snd-Conn-Req action : this is currently a missing point;
2. change its state to Wait-Conn-Ack : we did that in the last episode :)

- When a peer is in "Closed" state and receives a R-Conn-CER signal it must :

1. take the following 3 actions : R-Accept, Process-CER and R-Snd-CEA.
2. change its state to R-Open.

So, concerning this episode, we are going to add the behaviour requested by the R-Conn-CER signal, leaving out for the moment all what is concerning actions. That means we will concentrate only on state transistion.

Following the same approach we used for the "Start" signal, we can write another test method :

public class PeerStateMachineTestCase
{
....@Test
....public void rConnCerSignal()
...{
......Peer peer = new Peer();
......peer.signal(PeerEventType.R_CONN_CER);

......assertTrue(peer.currentStateIs(peer.R_OPEN);
...}
}

PeerEventType.R_CONN_CER is not giving any compiler warning because we already added it in the first part.
What is really missing(from a compiler perspective) is a R_OPEN member instance on Peer class:

public class Peer
{
....final State CLOSED = new State(){};
....final State WAIT_CONN_ACK = new State(){};
....final State R_OPEN = new State(){};
...
...
}

Now our test compiles fine but it gives us a red bar: after PeerEventType.R_CONN_CER signal has been received peer is not (as expected) in R_OPEN state but in WAIT_CONN_ACK :(
This is reasonable if we remember the Peer.signal(...) implementation we did last time :

public void signal(PeerEventType type)
{
....currentState = WAIT_CONN_ACK;
}

With the new R_CONN_CER signal this implementation is not working anymore so we need to rewrite the method in order to let the peer properly react to a given signal. The most obvious solution could be :

public void signal(PeerEventType type)
{
....if (currentState == CLOSED)
....{
........if (type == START)
........{
............currentState = WAIT_CONN_ACK;
........}
........else if (type == R_CONN_CER)
........{
............currentState = R_OPEN;
........}
....}
}

Without any doubt it's working but

1. If you have a look at the state machine table defined on RFC 3588 you'll quickly realize that this approach will result in a looot of "if / else if" statements;
2. personally I believe that the transition state is something related with the state itself, while in the implementation above we made that part of the Peer logic.
   

In addition, each time a new state needs to be added you must :

1. Add a new state member instance;
2. Modify the signal method.

What I'm trying to suggest is the following : if

1. we already have something that tell us what is the current state;
2. that thing is an object (an instance of State)
   
3. obviously that object could have its own state and methods;

Why don't we put the state transition logic within the each State? With this approach we need to modify a little bit the State interface introduced in part one adding a new method and a new exception :

(Interface State)
void signal(PeerEventType eventType) throws WrongEventException;

Now each instance of State must implements this method which is supposed to

1. Change the state of the hosting Peer instance (remember that our State instances are inner classes);
2. Raise an exception if a wrong (not allowed) signal is received;

So for example, the inner CLOSED State instance will do something like that :

....final State CLOSED = new State()
....{
........public void signal(PeerEventType eventType) throws WrongEventException
........{
............switch (eventType)
............{
................case START :
................{
....................currentState = WAIT_CONN_ACK;
....................break;
................}
................case R_CONN_CER :
................{
....................currentState = R_OPEN;
....................break;
................}
................default :
................{
....................throw new WrongEventException("Some useful message");
................}
............}
........}
....};

Of course, in order to get things working as expected, we need to modify the Peer.signal method :

public void signal(PeerEventType type) throws WrongEventTypeException
{
....currentState.signal(type);
}

Now our 2 test methods should correctly work.

See you soon for the last part :)

Ciao
Andrea

Implementing a Diameter Peer State Machine using Test Driven Development (Part 2).

So, next step is to test what happens if (following the state machine picture on the previous post) a START signal is sent to our peer.
According with RFC 3588 specification, if a peer, that is in CLOSED state, successfully processes the START signal two things happen:

• the peer must take a "I-Snd-Conn-Req" action;
• a state transition occurs : the peer passes from CLOSED to WAIT-CONN_ACK state;

In this article we will take care about the second point.
Well, returning to our test case, we have to add another @Test method :

public class PeerStateMachineTestCase
{

....@Test
....public void startSignal()
....{
.......Peer peer = new Peer();
.......peer.signal(PeerEventType.START);

.......assertTrue(peer.currentStateIs(peer.WAIT_CONN_ACK);
....}
}

Obviously, this test method doesn't compile : we don't have a signal() method and a WAIT_CONN_ACK member variable on our Peer class.
So first of all we will insert the missing method :

public void signal(PeerEventType eventType)
{
}

after that, Peer class needs a new State instance member :

final State WAIT_CONN_ACK = new State(){};

This is how our Peer class behaves :

public class Peer
{
....final State CLOSED = new State(){};
....final State WAIT_CONN_ACK = new State(){};

....public void signal(PeerEventType eventType){}

....boolean currentStateIs(State state)
....{
........return true;
....}
}
Now the test method (and the enclosing testcase) compiles fine. But after running, we get a red bar. Why? There is one explicit problem : transition state doesn't really happen on peer instance so the assertion fails. I mean, the peer is not on WAIT_CONN_ACK State. Actually is not in any state because the currentStateIs() method is returning true everytime :)
In order to see that all is properly working we need to introduce another peer member instance that represents the current state of the peer.
Well, considering that we already have a type for this new member (State class), we can insert (in Peer class):

private State currentState;

With this new variable, I can rewrite the currentStateIs() method in the following way :

boolean currentStateIs(State state)
{
....return currentState == state;
}

Introducing that change causes a failure in the first test we saw in part I. Well, in order to fix that we need to initialize the "currentState" member instance :

private State currentState = CLOSED;

Now the @Test initialState() is working while the @Test startSignal() is failing :(.
Why? because nothing on the peer instance is causing the required state transition and the assertion on the currentStateIs() fails because the peer is still in CLOSED state.
Again, what is the minimum change / adjustment we would do in order to get things working? This is the failing test :

public class PeerStateMachineTestCase
{
....@Test
....public void startSignal()
...{
......Peer peer = new Peer();
......peer.signal(PeerEventType.START);

......assertTrue(peer.currentStateIs(peer.WAIT_CONN_ACK);
...}
}

Yes, we can do the state transition in the signal() method :

public void signal(PeerEventType type)
{
....currentState = WAIT_CONN_ACK;
}

In this way, the test will succeed because after calling the signal() method the peer state is WAIT_CONN_ACK as required.

See you soon for third episode :)
Bye
Andrea

Implementing a Diameter Peer State Machine using Test Driven Development (Part 1).

RFC3588 (Diameter Base Protocol), specifically par. 5.6, contains specification about a finite state machine that MUST be observed by all Diameter implementations.

In this article we will see how to implement a peer state machine using Test Driven Development. Note that this is only the first part of the whole article.

The following is an extract (just the first entry) of the RFC table (you can found that on the previously mentioned paragraph) that illustrates the peer state machine :

I choose that entry because "Closed" is the peer initial state.

As you can see when the peer is in that state it can receive only two kind of events:

1) Start : A diameter application has signaled that a connection should be initiated with the peer;

2) R-Conn-CER : An acknowledgement is received stating that the transport connection has been established, and the associated Capability Exchange Request (CER) has arrived.

The first event must be associated with the following action on the peer :

1) I-Snd-Conn-Req (Initiator Sends Connection Request) : A transport connection is initiated with the peer;

After the peer successfully processed the action its new state MUST be "Wait-Conn-Ack".

The second event, R-Conn-CER, is firing the following actions on the peer :

1) R-Accept : The incoming connection associated with the R-Conn-CER is accepted as the responder connection;

2) Process-CER : The CER associated with the R-Conn-CER is processed.

3) R-Snd-CEA (Receiver Sends CEA)

After those three actions, the peer must be in "R-Open" state.

Well, of course if you have a look at the whole table there are a lot of states / actions / events but for simplicity we can assume that we are handling only with the "Closed" state. The whole article can be applied to other table entries.

First implicit requirement is that we should have a Peer class, it must have an internal state and the initial value of that state should be "Closed".

So, for a first implementation of our test case I'd like to write something like this :

package com.gazzax.diameter.peer;

public class PeerStateMachineTestCase

{

@Test

public void initialState()

{

Peer peer = new Peer();

assertTrue(peer.currentStateIs(peer.CLOSED));

}

}

Obviously, the test doesn't compile for several reasons; First of all, we need a Peer class :) So let's create it!

package com.gazzax.diameter.peer;

public class Peer

{

}

One little step ahead! But it is not sufficient because the TestCase still doesn't compile :( There's no a currentStateIs(...) method on the Peer class!

Note that that method is accepting a parameter that represents a peer state. Peer's states are typed instance members of peer class and they have a default visibility. Why? Because in that way we can see / use them on our test case without breaking encapsulation (PeerStateMachineTestCase and Peer are in the same package even if they usually reside in different source folders)

NOTE : the current state of the peer IS NOT exposed (it is a private member). You can only have an indirect / read-only access through the currentStateIs(...) method.

This is done because we don't want break enapsulation and therefore peer state invariants.

Anyway, the state machine table suggests that

- a peer, regardless its state, should be able to receive a signal;

- the concrete handling of that signal (fired actions and next state) depends on the current state of the peer;

So, this suggests me that a peer state itself shuold be able to receive a signal. Here is the peer state interface:

package com.gazzax.diameter.peer;

public interface State

{

void signal(PeerEventType event) throws WrongEventException;

}

Where

- PeerEventType is an enumeration of all defined event types; for the current example it will look like this :

package com.gazzax.diameter.peer;

public enum PeerEventType

{

START,

R_CONN_CER

}

- WrongEventException : that will be thrown when the received event type is violating the rules defined on the peer state machine table. I mean, when the peer will receive a wrong event according with its current state. I mean, when the current state of the peer doesn't recognize the received event type as valid.

We left the TestCase with compilation errors. We need a currentStateIs(State peerState) method. Let's write it with a dummy implementation!

public class Peer

{

private final State CLOSED;

boolean currentStateIs(State state)

{

return false;

}

}

Now code should compile :)

Run again the test case and you will see the red bar! Bad? Absolutely not, that means "Progress"! Another little step ahead!

What is the minimal thing / adjustment that we could do in order to get a green bar?

public class Peer

{

private final State CLOSED;

boolean currentStateIs(State state)

{

return true;

}

}

Run again the test and...et voilà! We got a green bar! Another little step ahead!

Dependency Injection with EJB 2.x

Very often, I see in an EJB 2.x environment message driven or session beans with the following code :

public void onMessage(Message message)

{

Connection connection = null;

try

{

Context ctx = new Context();

DataSource datasource = (DataSource)ctx.lookup("jdbc/ds");

Connection connetion = ....

// Do somethign with connection

} catch(NamingException exception)

{

// Do something

} catch(SQLException exception)

{

// Do something

} finally

{

try { connection.close(); } catch(Exception ignore) {}

}

}

...or (better) using a service locator :

public void onMessage(Message message)

{

Connection connection = null;

try

{

ServiceLocator serviceLocator = new ServiceLocator();

DataSource datasource = serviceLocator.getDataSource("jdbc/ds");

Connection connetion = ....

// Do somethign with connection

} catch(NamingException exception)

{

// Do something

} catch(SQLException exception)

{

// Do something

} finally

{

try { connection.close(); } catch(Exception ignore) {}

}

}

What is the problem? That piece of code (doesn't matter if it belongs to a session or a message driven bean) should IMO contains only business logic;

I mean, the logic that should make sense, from a business point of view, of that EJB.

Now, again, what is the problem? JDBC Code? Why don't we use a Data Gateway (or in general a data access pattern)?

Yes, that's right, but that's not the point.

Let's make a strong precondition supposing that jdbc code is part of business logic.

What else?

Look at this line of code :

} catch(NamingException exception)

{

// Do something

}

This exception is thrown when some problem occurs with naming service. The funny thing is that we are looking up the resource (a datasource in this case) each time this method is called. What is the problem?

There are at least three issues :

1) Each call to Context.lookup(...) is theoretically a remote call. Here that call is done each time a message arrives. You may think that code could be optimized, looking up the resource the first time, but you must in any case catch the Naming Exception (even if the lookup isn't really executed)

2) What you need is the resource that is bound with the naming service under the name "jdbc/ds", not the naming service itself. Once you got it, you have all what you need in order to retrieve valid database connections. So you should use that lookup code once and only once.

3) NamingException is not a business excepiton...the corresponding "throwing" code it's not part of the business logic too, but the onMessage(), where only the business logic should be, must contain the try-catch statement in order to handle a scenario where a naming service occurs...EACH TIME THE CODE IS EXECUTED!.

The usage of a service locator doesn't solve the problem completely : even if we cache the resource (on service locator), we will avoid the 1) & 2) but not the 3th issue.

In fact the signature of the getDatasource(...) method is throwing a NamingException and therefore the calling code must catch it everytime.

The ideal solution should:

1) make a lookup call only once;

2) catch the naming exception only once (when the lookup call is executed)

3) in case of naming exception (naming service failure) the call should be repeated until the resource is available.

public void GxMessageDriven implements MessageDrivenBean, MessageListener

{

private DataSource datasource;

public void onMessage()

{

// Here we can use directly the datasource instance that

// has been self-injected in ejbCreate() method at startup.

Connection connection = null;

try

{

connection = datasource.getConnection()

} catch(SQLException exception)

{

...

} finally

{

try { connection.close } catch(Exception ignore){}

}

}

public void ejbCreate()

{

try

{

// Using a service locator should be better

Context ctx = new InitialContext();

datasource = (DataSource) ctx.lookup("jdbc/ds");

} catch(NamingException exception)

{

// This ejb cannot be initialized because a needed resource cannot be retrieved

}

}

}

As you can see the lookup code (and the corresponding catch of the NamingException) is executed only once in the ejbCreate().

The first obvious consequence is that within onMessage() method the datasource member instance is used directly because it is already intialized.

There's only remaining open issue : what happens if the lookup fails in the ejbCreate? Basically that means the onMessage method will throw a NullPointerException and obviously we don't want that :)

If you are thinking about an if statement within the onMessage method :

if (datasource != null)

{

....

}

That means probably you didn't read my previous post : I hate if statements :) I think is better to equipe the ejb component with a state pattern in the following way :

1) create a member instance which will represent the current state of the component:

private MessageListener currentState;

2) create one inner class which represents the WORKING state:

private final MessageListener WORKING = new MessageListener()

{

public void onMessage()

{

// Here we can use directly the datasource instance that

// has been self-injected in ejbCreate() method at startup.

Connection connection = null;

try

{

connection = datasource.getConnection()

} catch(SQLException exception)

{

...

} finally

{

try { connection.close } catch(Exception ignore){}

}

}

}

3) Create another inner class which represents the OFF state :

private final MessageListener OFF = new MessageListener()

{

public void onMessage(Message message)

{

try

{

Context ctx = new InitialContext();

datasource = (DataSource) ctx.lookup("jdbc/ds");

// State change : OFF --> WORKING

currentState = WORKING;

// Process the message

currentState.onMessage(message);

} catch(NamingException exception)

{

// Don't change current state...resource is not yet available

}

}

}

4) Update the memeber instance variable in order to set the OFF state as default state.

private MessageListener currentState = OFF;

At this point, you should probably see that the ejbCreate is no longer useful because the check is made by the OFF state the first time the component raises up.

So, the final code should look like this :

public void GxMessageDriven implements MessageDrivenBean, MessageListener

{

private final MessageListener OFF = new MessageListener()

{

public void onMessage(Message message)

{

try

{

Context ctx = new InitialContext();

datasource = (DataSource) ctx.lookup("jdbc/ds");

// State change : OFF --> WORKING

currentState = WORKING;

// Process the message using the working state

currentState.onMessage(message);

} catch(NamingException exception)

{

// Don't change current state...resource is not yet available

}

}

}

private final MessageListener WORKING = new MessageListener()

{

public void onMessage()

{

// Here we can use directly the datasource instance that

// has been self-injected in ejbCreate() method at startup.

Connection connection = null;

try

{

connection = datasource.getConnection()

} catch(SQLException exception)

{

//...

} finally

{

try { connection.close } catch(Exception ignore){}

}

}

}

private DataSource datasource;

private MessageListener currentState= OFF;

}

Apache Qpid Committer!

Hi all, it happened about two months ago...I became an Apache Committer for Qpid project  (http://qpid.apache.org)!

"Just a brief note, we have voted Andrea on as a committer, all the accounts and details have
been completed and he now has Karma! A warm welcome to the team Andrea."

YAHOOOO!!!

Basically I developed (I'm still developing) a JMX / WS-DM adapter for Qpid broker in order to enable remote management using one of the mentioned technologies...I already knew JMX but WS-DM was (is still so :) ) completely unknown so at the end is a nice challenge :)

If you are interested on that here you are some useful links :

- Apache Qpid home page : http://qpid.apache.org 

- This is what I've done (really what I'm still doing...) :   http://qpid.apache.org/qman-qpid-management-bridge.html

...and last but not least...

http://people.apache.org/list_G.html#agazzarini

Best regards,

Andrea

IF-LESS code : State / Strategy Pattern

We have a simple class that is looping on a given List. Requirement are :

1) Service must preserve the first value of the list;

2) Each other element must be printed in uppercase

3) At the end of the loop the "Exit" string must be printed out;

The first version is the following :

public class Service {

String theFirst;

public void loop(List strings){

int index = 0;

for (String string : strings) {

if (index == 0) {

theFirst = string;

} else {

System.out.println(string.toUpperCase());

}

index++;

}

System.out.println("Exit");

}

}

Ok, brief and (moreless) clear; Let's try to modify a little bit that class; First, I'd introduce a compose method in order isolate what needs to be done 

for middle iterations (uppercase printing) so I'll introduce :

private void uppercase(String value) {

System.out.println(value.toUppercase());

}

Next, I don't like the explicit loop & condition logic. The tasks that need to be executed in the current example are very simple but let's try to imagine a more complicated task 

(for initial and middle iteration) : the code should result complicated to understand...

 I'd prefer to isolate those tasks in a separate method / class.

So, in order to do that, let's introduce an Iteration interface that represents the task that needs to be executed on a given iteration.

// Determines what needs to be done on a given iteration

interface Iteration {

void execute(String value);

}

we have two concrete implementors; something like that:

Iteration firstIteration = new Iteration() {

public void execute(String value) {

// Store the value of the first element

}

}

Iteration middleIteration = new Iteration() {

public void execute(String value) {

uppercase(value);

}

}

Now, each task should be responsible to execute its logic and in addition should provide some mechanism to compute & execute the next iteration. 

In order to do that we change a little the Iteration interface and the hosting class introducing a new Iterator member that is initialized on the loop() method.

interface Iteration {

void execute();

}

Iteration firstIteration = new Iteration() {

public void execute() {

// Store the value of the iterator.next() that is the first element of the list;

}

}

Iteration middleIteration = new Iteration() {

public void execute() {

uppercase(iterator.next());

}

}

private Iterator iterator;

...

public void loop(List strings) {

iterator = strings.iterator();

}

At this point we need to maintain a reference to the current iteration. So let's introduce another instance member :

private Iteration currentIteration = firstIteration; 

The initial value of this currentIteration is obviously the previously declared inner class "firstIteration". After that the loop method should be changed in order to start the loop : 

public void loop(List strings) {

iterator = strings.iterator();

currentIteration.execute();

}

Next, we need a way to continue the execution of the loop. We can do that introducing another method :

public void nextIteration() {

currentIteration = (iterator.hasNext()) ? middleIteration: noIteration;

currentIteration.execute();

}

 

ok, ok, I know...this is still a conditional logic but is not exaclty the same as the previous one...I'm not deal with indexes or temporary variables...just with the given collection...

As you can saw, we need another instance of Iteration inner interface for determine what needs to be done when the end of the loop is reached.

private final Iteration noIteration = new Iteration() {

public void execute() {

System.out.println("Exit");

}

}

That's all! Do you think I'm paranoic? :)....mmmmmm...yes! Note that the code above is just an exercise and the benefits are not evident because the starting class is very simple (I mean, the logic of the initial version of that class) but I can ensure you that they are more evident when those things are applied to a more complicated scenario...

This is the final version of the IlLessService :

public class IfLessService {

String theFirst;

private ListIterator iterator;

private interface Iteration {

void execute();

}

// What needs to be done in the first iteration?

private final Iteration firstIteration= new Iteration() {

public void execute() {

theFirst = iterator.next();

executeNextIteration();

}

};

// And in the middle iteration(s)?

private final Iteration middleIteration = new Iteration() {

public void execute() {

uppercase(iterator.next());

executeNextIteration();

}

};

// And when the iteration will terminate?

private final Iteration noIteration = new Iteration() {

public void execute() {

System.out.println("Exit");

}

};

private Iteration currentIteration = firstIteration;

public void loop(List strings){

iterator = strings.listIterator();

currentIteration.execute();

}

private void uppercase(String aString) {

System.out.println(aString.toUpperCase());

}

private void executeNextIteration(){

currentIteration = (iterator.hasNext()) ? middleIteration: noIteration;

currentIteration.execute();

}

}

Regards,

Andrea

IF-LESS code : State Pattern

Hi all, let's start with a post on this interesting topic (at least for me) with a simple example.

We have an interface named IService that represents a generic service. The interface offers three methods : start(), stop() and isRunning(). 

public interface IService {

void start(); // Starts the service.

void stop(); // Stops the service. 

boolean isRunning(); // Returns true if the service is running.

}

While looking at the following implementation, I wasn't satisfied about the conditional statements on start() and stop() method :

public class ServiceImpl implements IService {

   private volatile boolean started;

   private Thread thread;

   public synchronized void start() { 

      if (started) {

         return;

      }

      

      started = true;

      

      thread = ... // Create your daemon implementation.

      thread.setDaemon(true);

      thread.start();

   }

   

   public synchronized void stop()

   {      

      if (!started) {

         return;

      }

      started = false;

      thread.interrupt();

      

      try {

         thread.join();

      }

      catch (InterruptedException ignore) {         

      }

   }

   

   public synchronized boolean isRunning() {

      return started;

   }

}

To be honest, what I don't like is :

- Maintaining the state of the service in a boolean member and therefore relying on that in order to know if the service is alive or not (the isRunning method)

- Conditional logic used for checking everytime the (start & stop) methods are called if the service is alive or not. I think the service should know that without using a boolean member to remember! An example : if you are driving a car, how do you know that it's running? You know that because it's running and you're driving! ;-) and not because after turned it on you put a post-it on the dashboard with "RUNNING" written over! 

I mean : it's the status itself that suggests you what's going on!

As result of that, I prefer this version of the service implementation:

public class IfLessService implements IService {

   private Thread _watcher;

   

   /**

    * RUNNING STATE.

    */

   private final IService running = new IService() {

      public boolean isRunning()

      {

         return true; // We are inside the running state and so...

      }

      public void start() {

         // Nothing to do here...it is already started.

      }

      public void stop()  {

         _watcher.interrupt();      

         try  {

            _watcher.join();

         }

         catch (InterruptedException ignore)

         {         

         }

        state = notRunning;

      }

   };

   

   /**

    * NOT RUNNING STATE.

    */

   private final IService notRunning = new IService()  {      

      public boolean isRunning()  {

         return false; // We are inside the not running state and so...

      }

      public synchronized void start()  {

      _watcher = ...// Create your daemon implementation...

         _watcher.setDaemon(true);

         _watcher.start();

         // ...and make a state change too...from NOT-RUNNING to RUNNING.

         state = running;

      }

      public void stop() {

         // Nothing to do here...it is already stopped.         

      }      

   };   

   

   // Default initial state is stopped (not running).

   private IService state = notRunning;

   public synchronized void start() {      

      state.start();  // Current state delegation

   }

   public synchronized void stop() {      

      state.stop();    // Current state delegation   

   }

   public boolean isRunning() {

      return state.isRunning();  // Current state delegation

   }

}

Now the service implementation is delegating the execution of the IService methods to the current internal state. Those states are themselves implementors of IService interface. We have two states : running and notRunning. 

Note that the responsibility of  each state is not only to manage the "state" of the object at a specific moment but also to provide an eventual state transition: for example when the service is not running (state = notRunning), if you call the start() method there will be a state transition (state = running).

Any comment would be very very appreciated...