svn commit: r570706 - /ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java

Posted by jonesde on
URL: http://ofbiz.116.s1.nabble.com/svn-commit-r570706-ofbiz-trunk-framework-entity-src-org-ofbiz-entity-GenericDelegator-java-tp212958.html

Author: jonesde
Date: Wed Aug 29 01:58:10 2007
New Revision: 570706

URL: http://svn.apache.org/viewvc?rev=570706&view=rev
Log:
Cleaned up exception and error handling in getNextSeqId code, was redundant and the tx code in getNextSeqIdLong that getNextSeqId always calls wasn't handling commits right

Modified:
    ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java

Modified: ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java?rev=570706&r1=570705&r2=570706&view=diff
==============================================================================
--- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java (original)
+++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java Wed Aug 29 01:58:10 2007
@@ -37,6 +37,7 @@
 import org.xml.sax.SAXException;
 
 import org.ofbiz.base.util.Debug;
+import org.ofbiz.base.util.GeneralRuntimeException;
 import org.ofbiz.base.util.UtilFormatOut;
 import org.ofbiz.base.util.UtilMisc;
 import org.ofbiz.base.util.UtilValidate;
@@ -2434,41 +2435,17 @@
      *@return Long with the next seq id for the given sequence name
      */
     public String getNextSeqId(String seqName, long staggerMax) {
-        boolean beganTransaction = false;
-        try {
-            if (alwaysUseTransaction) {
-                beganTransaction = TransactionUtil.begin();
-            }
+        Long nextSeqLong = this.getNextSeqIdLong(seqName, staggerMax);
 
-            Long nextSeqLong = this.getNextSeqIdLong(seqName, staggerMax);
-
-            if (nextSeqLong == null) {
-                throw new IllegalArgumentException("Could not get next sequenced ID for sequence name: " + seqName);
-            }
+        if (nextSeqLong == null) {
+            // NOTE: the getNextSeqIdLong method SHOULD throw a runtime exception when no sequence value is found, which means we should never see it get here
+            throw new IllegalArgumentException("Could not get next sequenced ID for sequence name: " + seqName);
+        }
 
-            if (UtilValidate.isNotEmpty(this.getDelegatorInfo().sequencedIdPrefix)) {
-                return this.getDelegatorInfo().sequencedIdPrefix + nextSeqLong.toString();
-            } else {
-                return nextSeqLong.toString();
-            }
-        } catch (Exception e) {
-            String errMsg = "Failure in getNextSeqId operation for seqName [" + seqName + "]: " + e.toString() + ". Rolling back transaction";
-            Debug.logError(e, errMsg, module);
-            try {
-                // only rollback the transaction if we started one...
-                TransactionUtil.rollback(beganTransaction, errMsg, e);
-            } catch (GenericEntityException e2) {
-                Debug.logError(e2, "[GenericDelegator] Could not rollback transaction: " + e2.toString(), module);
-            }
-            Debug.logError(e, "[GenericDelegator] Error getting next sequence ID: " + e.toString(), module);
-            return null;
-        } finally {
-            try {
-                // only commit the transaction if we started one...
-                TransactionUtil.commit(beganTransaction);
-            } catch (GenericTransactionException e1) {
-                Debug.logError(e1, "[GenericDelegator] Could not commit transaction: " + e1.toString(), module);
-            }
+        if (UtilValidate.isNotEmpty(this.getDelegatorInfo().sequencedIdPrefix)) {
+            return this.getDelegatorInfo().sequencedIdPrefix + nextSeqLong.toString();
+        } else {
+            return nextSeqLong.toString();
         }
     }
 
@@ -2506,21 +2483,24 @@
 
             Long newSeqId = sequencer == null ? null : sequencer.getNextSeqId(seqName, staggerMax);
 
-            // only commit the transaction if we started one...
-            TransactionUtil.commit(beganTransaction);
-
             return newSeqId;
         } catch (GenericEntityException e) {
             String errMsg = "Failure in getNextSeqIdLong operation for seqName [" + seqName + "]: " + e.toString() + ". Rolling back transaction.";
-            Debug.logError(e, errMsg, module);
             try {
                 // only rollback the transaction if we started one...
                 TransactionUtil.rollback(beganTransaction, errMsg, e);
             } catch (GenericEntityException e2) {
                 Debug.logError(e2, "[GenericDelegator] Could not rollback transaction: " + e2.toString(), module);
             }
-            Debug.logError(e, "[GenericDelegator] Error getting next sequence ID: " + e.toString(), module);
-            return null;
+            // rather than logging the problem and returning null, thus hiding the problem, throw an exception
+            throw new GeneralRuntimeException(errMsg, e);
+        } finally {
+            try {
+                // only commit the transaction if we started one...
+                TransactionUtil.commit(beganTransaction);
+            } catch (GenericTransactionException e1) {
+                Debug.logError(e1, "[GenericDelegator] Could not commit transaction: " + e1.toString(), module);
+            }
         }
     }