Should we not put this in release ?

Jacques

> 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 @@
>       [EMAIL PROTECTED] 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);
> +            }
>          }
>      }
>
>

Reply via email to