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;
}
