Hi all,
I'm submitting the beginnings of a patch for
https://mifos.dev.java.net/issues/show_bug.cgi?id=1533
>From the defect report, there is mention that HibernateUtil.closeSession()
isn't always being called.
So, to make this safe(r), I suggest using a transaction callback.
e.g.
before
-----------
Session session = HibernateUtil.getSessionTL();
try{
//..doStuff
}
finally{
HibernateUtil.closeSession(); //Note, sometimes this isn't called!
}
return item;
new usage
---------------
HibernateUtil.doTransaction(new SafeReturnTransaction<Item>{
public Item execute(Session session){
// ..doStuff
return item;
}
});
I tried out the usage of it in ApplicationConfiguration, and I managed to
remove a few lines of duplicated code in the process.
(I bulked up the tests to make sure I didn't break anything)
If this works for everyone, I can propagate this through the code
Feedback?
Regards
Steve Horgan
Index: /home/steve/eclipse/workspace/mifos/mifos/src/org/mifos/application/configuration/persistence/ApplicationConfigurationPersistence.java
===================================================================
--- /home/steve/eclipse/workspace/mifos/mifos/src/org/mifos/application/configuration/persistence/ApplicationConfigurationPersistence.java (revision 12273)
+++ /home/steve/eclipse/workspace/mifos/mifos/src/org/mifos/application/configuration/persistence/ApplicationConfigurationPersistence.java (working copy)
@@ -48,6 +48,7 @@
import org.mifos.application.master.business.MifosLookUpEntity;
import org.mifos.application.master.business.SupportedLocalesEntity;
import org.mifos.framework.exceptions.PersistenceException;
+import org.mifos.framework.hibernate.SafeReturnTransaction;
import org.mifos.framework.hibernate.helper.HibernateUtil;
import org.mifos.framework.persistence.Persistence;
import org.mifos.application.master.business.CustomFieldDefinitionEntity;
@@ -65,74 +66,59 @@
* an ongoing transaction
*/
public List<MifosLookUpEntity> getLookupEntities(){
-
- List<MifosLookUpEntity> entities=null;
- try
- {
- Session session = HibernateUtil.getSessionTL();
- entities = session.getNamedQuery(
- NamedQueryConstants.GET_ENTITIES).list();
-
- for (MifosLookUpEntity entity : entities) {
- Set<LookUpLabelEntity> labels = entity.getLookUpLabels();
- entity.getEntityType();
- for (LookUpLabelEntity label : labels) {
- label.getLabelText();
- label.getLocaleId();
+ return HibernateUtil.doTransaction(new SafeReturnTransaction<List<MifosLookUpEntity>>(){
+ public List<MifosLookUpEntity> execute(Session session) {
+ List<MifosLookUpEntity> entities = session.getNamedQuery(NamedQueryConstants.GET_ENTITIES).list();
+
+ for (MifosLookUpEntity entity : entities) {
+ Set<LookUpLabelEntity> labels = entity.getLookUpLabels();
+ entity.getEntityType();
+ for (LookUpLabelEntity label : labels) {
+ label.getLabelText();
+ label.getLocaleId();
+ }
}
+ return entities;
}
- } finally {
- HibernateUtil.closeSession();
- }
-
- return entities;
+ });
}
- public List<LookUpValueEntity> getLookupValues(){
- List<LookUpValueEntity> values=null;
- try{
- Session session = HibernateUtil.getSessionTL();
- values = session.getNamedQuery(
- NamedQueryConstants.GET_LOOKUPVALUES).list();
-
- for (LookUpValueEntity value : values) {
- Set<LookUpValueLocaleEntity> localeValues = value
- .getLookUpValueLocales();
- value.getLookUpName();
- for (LookUpValueLocaleEntity locale : localeValues) {
+ public List<LookUpValueEntity> getLookupValues() {
+ return HibernateUtil.doTransaction(new SafeReturnTransaction<List<LookUpValueEntity>>() {
+ public List<LookUpValueEntity> execute(Session session) {
+ List<LookUpValueEntity> values = session.getNamedQuery(
+ NamedQueryConstants.GET_LOOKUPVALUES).list();
- locale.getLookUpValue();
- locale.getLocaleId();
+ for (LookUpValueEntity value : values) {
+ Set<LookUpValueLocaleEntity> localeValues = value.getLookUpValueLocales();
+ value.getLookUpName();
+ for (LookUpValueLocaleEntity locale : localeValues) {
+ locale.getLookUpValue();
+ locale.getLocaleId();
+ }
}
-
+ return values;
}
+ });
- }
- finally{
- HibernateUtil.closeSession();
- }
- return values;
}
// this method is used by Localization class to load all the supported locales to its cache
public List<SupportedLocalesEntity> getSupportedLocale(){
- List<SupportedLocalesEntity> locales=null;
- try {
- Session session = HibernateUtil.getSessionTL();
-
- locales = session.getNamedQuery(
- NamedQueryConstants.SUPPORTED_LOCALE_LIST).list();
-
- for (SupportedLocalesEntity locale : locales) {
- locale.getLanguage().getLanguageShortName();
- locale.getCountry().getCountryShortName();
- locale.getLocaleId();
-
+ return HibernateUtil.doTransaction(new SafeReturnTransaction<List<SupportedLocalesEntity>>() {
+ public List<SupportedLocalesEntity> execute(Session session) {
+ List<SupportedLocalesEntity> locales = session.getNamedQuery(
+ NamedQueryConstants.SUPPORTED_LOCALE_LIST).list();
+
+ for (SupportedLocalesEntity locale : locales) {
+ locale.getLanguage().getLanguageShortName();
+ locale.getCountry().getCountryShortName();
+ locale.getLocaleId();
+ }
+ return locales;
}
- } finally {
- HibernateUtil.closeSession();
- }
+ });
- return locales;
+
}
public void addCustomField(CustomFieldDefinitionEntity customField) throws PersistenceException {
Index: /home/steve/eclipse/workspace/mifos/mifos/src/org/mifos/framework/hibernate/helper/HibernateUtil.java
===================================================================
--- /home/steve/eclipse/workspace/mifos/mifos/src/org/mifos/framework/hibernate/helper/HibernateUtil.java (revision 12273)
+++ /home/steve/eclipse/workspace/mifos/mifos/src/org/mifos/framework/hibernate/helper/HibernateUtil.java (working copy)
@@ -51,6 +51,8 @@
import org.mifos.framework.components.logger.MifosLogManager;
import org.mifos.framework.exceptions.ConnectionNotFoundException;
import org.mifos.framework.exceptions.HibernateProcessException;
+import org.mifos.framework.hibernate.SafeReturnTransaction;
+import org.mifos.framework.hibernate.SafeVoidTransaction;
import org.mifos.framework.hibernate.factory.HibernateSessionFactory;
import org.mifos.framework.persistence.SqlUpgrade;
@@ -239,6 +241,28 @@
getSessionHolder().setTranasction(null);
}
+
+ }
+
+ public static <T> T doTransaction(SafeReturnTransaction<T> transaction) {
+ final Session session = HibernateUtil.getSessionTL();
+ try{
+ return transaction.execute(session);
+ }
+ finally{
+ HibernateUtil.closeSession();
+ }
+ }
+
+ public static void doTransaction(SafeVoidTransaction transaction) {
+ final Session session = HibernateUtil.getSessionTL();
+ try{
+ transaction.execute(session);
+ }
+ finally{
+ HibernateUtil.closeSession();
+ }
}
+
}
Index: /home/steve/eclipse/workspace/mifos/mifos/src/org/mifos/framework/hibernate/SafeReturnTransaction.java
===================================================================
--- /home/steve/eclipse/workspace/mifos/mifos/src/org/mifos/framework/hibernate/SafeReturnTransaction.java (revision 0)
+++ /home/steve/eclipse/workspace/mifos/mifos/src/org/mifos/framework/hibernate/SafeReturnTransaction.java (revision 0)
@@ -0,0 +1,7 @@
+package org.mifos.framework.hibernate;
+
+import org.hibernate.Session;
+
+public interface SafeReturnTransaction<T> {
+ public T execute(Session session);
+}
Index: /home/steve/eclipse/workspace/mifos/mifos/src/org/mifos/framework/hibernate/SafeVoidTransaction.java
===================================================================
--- /home/steve/eclipse/workspace/mifos/mifos/src/org/mifos/framework/hibernate/SafeVoidTransaction.java (revision 0)
+++ /home/steve/eclipse/workspace/mifos/mifos/src/org/mifos/framework/hibernate/SafeVoidTransaction.java (revision 0)
@@ -0,0 +1,7 @@
+package org.mifos.framework.hibernate;
+
+import org.hibernate.Session;
+
+public interface SafeVoidTransaction {
+ public void execute(Session session);
+}
Index: /home/steve/eclipse/workspace/mifos/mifos/test/org/mifos/application/configuration/TestApplicationConfigurationPersistence.java
===================================================================
--- /home/steve/eclipse/workspace/mifos/mifos/test/org/mifos/application/configuration/TestApplicationConfigurationPersistence.java (revision 12273)
+++ /home/steve/eclipse/workspace/mifos/mifos/test/org/mifos/application/configuration/TestApplicationConfigurationPersistence.java (working copy)
@@ -3,6 +3,7 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
+import java.util.Iterator;
import java.util.List;
import java.util.Set;
@@ -17,6 +18,7 @@
import org.mifos.application.master.business.LookUpValueEntity;
import org.mifos.application.master.business.LookUpValueLocaleEntity;
import org.mifos.application.master.business.MifosLookUpEntity;
+import org.mifos.application.master.business.SupportedLocalesEntity;
import org.mifos.framework.MifosTestCase;
import org.mifos.framework.components.configuration.util.helpers.ConfigConstants;
import org.mifos.framework.hibernate.helper.HibernateUtil;
@@ -54,9 +56,35 @@
}
}
+ assertFalse(HibernateUtil.isSessionOpen());
}
+
public void testGetLookupValues(){
- assertNotNull(configurationPersistence.getLookupValues());
+ List<LookUpValueEntity> lookupValues = configurationPersistence.getLookupValues();
+ assertNotNull(lookupValues);
+ for (LookUpValueEntity lookupValueEntity : lookupValues) {
+ if (lookupValueEntity.getLookUpName().equals("SAVINGS_CLOSED")) {
+ Set<LookUpValueLocaleEntity> locales = lookupValueEntity.getLookUpValueLocales();
+ assertEquals(1, locales.size());
+ assertEquals("Closed", ((LookUpValueLocaleEntity)locales.toArray()[0]).getLookUpValue());
+ }
+ assertNotNull(lookupValueEntity.getLookUpId());
+ }
+ assertFalse(HibernateUtil.isSessionOpen());
+ }
+
+ public void testGetSupportedLocales() throws Exception {
+ List<SupportedLocalesEntity> supportedLocales = configurationPersistence.getSupportedLocale();
+ assertNotNull(supportedLocales);
+ for (SupportedLocalesEntity supportedLocale : supportedLocales) {
+ if (supportedLocale.getLocaleId() == 1) {
+ assertEquals("EN", supportedLocale.getLocaleName());
+ assertEquals(1, supportedLocale.getDefaultLocale().intValue());
+ assertEquals("English", supportedLocale.getLanguage().getLanguageName());
+ assertEquals("EN", supportedLocale.getLanguage().getLanguageShortName());
+ }
+ }
+ assertFalse(HibernateUtil.isSessionOpen());
}
/*
Index: /home/steve/eclipse/workspace/mifos/mifos/test/org/mifos/framework/hibernate/HibernateTest.java
===================================================================
--- /home/steve/eclipse/workspace/mifos/mifos/test/org/mifos/framework/hibernate/HibernateTest.java (revision 12273)
+++ /home/steve/eclipse/workspace/mifos/mifos/test/org/mifos/framework/hibernate/HibernateTest.java (working copy)
@@ -4,6 +4,7 @@
import junitx.framework.ObjectAssert;
+import org.hibernate.Session;
import org.mifos.framework.MifosTestCase;
import org.mifos.framework.exceptions.HibernateStartUpException;
import org.mifos.framework.hibernate.factory.HibernateSessionFactory;
@@ -50,4 +51,46 @@
assertFalse(HibernateUtil.isSessionOpen());
}
+ public void testSafeReturnTransactionReturnsValue() throws Exception {
+ Integer transactionResult = HibernateUtil.doTransaction(new SafeReturnTransaction<Integer>(){
+ public Integer execute(Session session){
+ return 123;
+ }
+ });
+
+ assertNotNull(transactionResult);
+ assertEquals(123, transactionResult.intValue());
+ }
+
+ public void testSafeReturnTransactionOpensAndClosesSession() throws Exception {
+ assertFalse(HibernateUtil.isSessionOpen());
+ try{
+ HibernateUtil.doTransaction(new SafeReturnTransaction<Integer>(){
+ public Integer execute(Session session){
+ assertTrue(HibernateUtil.isSessionOpen());
+ throw new RuntimeException("I am a crash test dummy");
+ }
+ });
+ fail("This should have thrown an exception");
+ }
+ catch(RuntimeException e){
+ assertFalse(HibernateUtil.isSessionOpen());
+ }
+ }
+
+ public void testSafeVoidTransactionOpensAndClosesSession() throws Exception {
+ assertFalse(HibernateUtil.isSessionOpen());
+ try{
+ HibernateUtil.doTransaction(new SafeVoidTransaction(){
+ public void execute(Session session){
+ assertTrue(HibernateUtil.isSessionOpen());
+ throw new RuntimeException("I am a crash test dummy");
+ }
+ });
+ fail("This should have thrown an exception");
+ }
+ catch(RuntimeException e){
+ assertFalse(HibernateUtil.isSessionOpen());
+ }
+ }
}
-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
