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

Posted by Jacques Le Roux on
URL: http://ofbiz.116.s1.nabble.com/Re-svn-commit-r570706-ofbiz-trunk-framework-entity-src-org-ofbiz-entity-GenericDelegator-java-tp183378.html

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 @@
>       *@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);
> +            }
>          }
>      }
>
>