Please critique this reusable PersistenceManager Session EJB

single · **Joined:** Thu Feb 05, 2004 10:39 pm **Posts:** 44

Hi all,

I am trying to create a reusable persistence manager Session bean that I can use across my application. Naturally, I am happy to share the code here if it is of use to anyone.

However, I would like to ask everyone her for a bit of code review in terms of the reusability of this Class, robustness of Exception handling, potential problems etc.

1. Are there any direct problems with implementing a persistence manager as a Session bean? (i.e. storing session and factory references as class memebers etc)

2. Is exception handling sufficient or can improvements be done to avoid problems?

3. Performance? What can be done to better optimize the session bean's performance?

4. Opening and closing sessions...as it is set up right now, would there be significant problems for a high volume of transactions or concurrency?

ok, here is the Bean code:

Code:

/*
 * Created on Feb 6, 2004
 *
 */
package com.forisent.framework.productManager.store;

import java.io.File;
import java.net.URL;
import java.rmi.RemoteException;

import javax.ejb.EJBException;
import javax.ejb.SessionBean;
import javax.ejb.SessionContext;

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;

import net.sf.hibernate.HibernateException;
import net.sf.hibernate.Session;
import net.sf.hibernate.SessionFactory;

import com.forisent.framework.common.exception.PersistenceException;
import com.forisent.framework.productManager.store.factory.PersistenceFactory;

/**
 * @author 
 *
 */
public class PersistenceManagerBean implements SessionBean{

   private PersistenceManagerBean _PersistanceManager = null;
      
   private SessionFactory _SessionFactory = null;
   private Session session = null;
   
   private Log log = null;
   
   private void initializeLog(){
      if(log == null){
         log = LogFactory.getLog(PersistenceManagerBean.class);      
      }
   }

   public Object persist(Object obj) throws PersistenceException{
         
      Session session = null;
      log.info("Persisting object: " + obj.getClass().toString() + "...");
      try{
         session = getSession();
         session.saveOrUpdate(obj);   
         session.flush();
         
      }catch(HibernateException e){
         log.info("Rolling Back Persist Operation");
         try{
            session.evict(obj);
         }catch(Exception ex){
            throw new PersistenceException(ex.getMessage(), ex);
         }
         throw new PersistenceException(e.getMessage(), e);
      }finally{
         try{
            closeSession();
         }catch(HibernateException e){
            throw new PersistenceException(e.getMessage(), e);
         }
      }
      return obj;   
   }
   
   public boolean delete(Object obj) throws PersistenceException {
      
      Session session = null;
      log.info("Deleting object: " + obj.getClass().toString() + "...");
      try{
         session = getSession();
         session.delete(obj);   
         session.flush();
               
      }catch(HibernateException e){
         log.info("Rolling Back Delete Operation");
         try{
            session.evict(obj);
         }catch(Exception ex){
            throw new PersistenceException(ex.getMessage(), ex);
         }
         throw new PersistenceException(e.getMessage(), e);
      }finally{
         try{
            closeSession();
         }catch(HibernateException e){
            throw new PersistenceException(e.getMessage(), e);
         }
      }
      return true;
   }

   private Session getSession() throws HibernateException{
      if(session == null){
         session = _SessionFactory.openSession();
      }else if(!session.isConnected()){
         session.reconnect();
      }
      return session;
   }
   
   private void closeSession() throws HibernateException{
      if(session != null){
         session.disconnect();
      }
   }

//Other EJB lifecycle methods left out...
      
   public void ejbCreate() throws EJBException{
      initializeLog();
      
      try{
         _SessionFactory = PersistenceFactory.getFactory("META-INF/hibernate.cfg.xml");
      }catch(Exception e){
         e.printStackTrace();
      }
      
      log.info("Connection established...");
      
   }

   
   
}

Would be very glad if I could get some critique. Thanx!

emmanuel · **Posted:** Mon Mar 08, 2004 6:38 pm

Session is intended to be a user unit-of-work. You planned to use it as a application unit-of-work.
You'll face dirty data issues, session size increase and stuffs like that.

I think thread local pattern is better suited of SLSB.

single · **Joined:** Thu Feb 05, 2004 10:39 pm **Posts:** 44

So, I should be using one session per transaction really? I was kind of suspicious that my session management was no good. I have not had problems with it but I have only run a limited amount of test cases.

If I create and close one session per persistence call that comes in, this would be better? Or, should I maybe have the client call start transaction, perform all the data manipulation and then have the client again close the session? This would allow the client to perform multiple persist and/or delete operations per used up session.

What do you think of that?

mohitgupta_india · **Posted:** Tue Mar 09, 2004 2:59 am

Hello Single,

It is very true that session should be used for unit of work, otherwise you will face dirty data problem or memory+performance related issues due to some of the implemenation related issues also. This is my personal experience.

Better you go for other ideas like either start session for every call or give access to client to start or close session. This just depends on you requirements ie if your client layer having requirement to perform more than one operations at time, you may design accordingly.

This would certainly be better.

single · **Joined:** Thu Feb 05, 2004 10:39 pm **Posts:** 44

ok, so my current getSession and closeSession methods now look like this:

Code:

   private Session getSession() throws HibernateException{
      return _SessionFactory.openSession();
   }
   
   private void closeSession(Session session) throws HibernateException{
      session.close();
      
   }

This will start a new session per call.

Now, how about the rollback code...

Code:

}catch(HibernateException e){
         log.info("Rolling Back Persist Operation");
         try{
            session.evict(obj);
         }catch(Exception ex){
            throw new PersistenceException(ex.getMessage(), ex);
         }
         throw new PersistenceException(e.getMessage(), e);

...is session.evict sufficient rollback handling or should something more be done?

emmanuel · **Posted:** Tue Mar 09, 2004 2:41 pm

Well no, if you must not use the Hibernate session if it raised an Exception. It can be in a inconsistent state.
Note that your EJB definition is very fine grained. Usually it contains several calls to Hibernate and is more business centric.

single · **Joined:** Thu Feb 05, 2004 10:39 pm **Posts:** 44

thanks emmanuel,

So, if session.evict is not sufficient, should I call session.clear then? I did not want to put that in since I would loose my cache....although, if you say that my session would be left in an inconsistent state, then clearing it out completely is what I want?

single · **Joined:** Thu Feb 05, 2004 10:39 pm **Posts:** 44

or....are you saying that I should close the session right away if there is an exception and not even clear it out?

Do you see a significant problem with the fact that I have created a fine grained session bean instead of duplicating persistence logic across my app and performing it more context specific and business oriented?

emmanuel · **Posted:** Tue Mar 09, 2004 2:49 pm

If it raised an exception, then close it a build another one

single · **Joined:** Thu Feb 05, 2004 10:39 pm **Posts:** 44

emmanuel, excellent. Thanks for the input. This is great code review.

Now, I have taken this very generic and reusable approach to encapsulating persistence in my app. Would there maybe be an overall different architectural approach to all of this that someone would suggest over what I have done?

I would be very interested to hear suggestions.

isomorph · **Posted:** Thu Mar 11, 2004 11:07 pm

If you are concerned about unnecessary session opening/closing, why not consider using the spring-framework's HibernateTemplate, or SessionFactoryUtils.getSession(), which

"...Is aware of a respective Session bound to the current thread..."

Hanson

single · **Joined:** Thu Feb 05, 2004 10:39 pm **Posts:** 44

iomorph, I have actually lately been considering the spring framework for my app. What holds me back though is the lack of thorough and extensive documentation. But, since you have now mentioned this, I will look into it futher.

single · **Joined:** Thu Feb 05, 2004 10:39 pm **Posts:** 44

Sorry, correct that statement. The documentation is there, I just need some good tutorials that will quickly get my up to speed.

baliukas · **Posted:** Fri Mar 12, 2004 5:04 am

I think it is good idea, it must be better to use Statefull SessionBean, you can store hibernate session as field. Implement SessionSynchronization interface for cleanup and demarcate transactions on client (UserTransaction, session/transaction per client thread) and you will have remote hibernate session.

isomorph · **Posted:** Fri Mar 12, 2004 5:21 am

Since the spring-framework's HibernateTemplate, or SessionFactoryUtils.getSession()

"...Is aware of a respective Session bound to the current thread..."

why is it necessary to use a StateFul Session Bean ? Wouldn't invoking a method of a Stateless Session Bean be a separate thread and therefore bound to a separate Hibernate session ?