You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Jacques Le Roux <ja...@les7arts.com> on 2007/08/30 09:09:23 UTC
Re: svn commit: r570706 -
/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java
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);
> + }
> }
> }
>
>
Re: svn commit: r570706 -
/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java
Posted by Jacques Le Roux <ja...@les7arts.com>.
In 571088, I used my new interactive merging batch :o)
Jacques
De : "David E Jones" <jo...@hotwaxmedia.com>
>
> This could go into the release branch. It is the sort of bug that probably wouldn't come up much because of the way this code is
generally used... but it is pretty much a bug fix.
>
> -David
>
>
> Jacques Le Roux wrote:
> > 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);
> >> + }
> >> }
> >> }
> >>
> >>
> >
Re: svn commit: r570706 - /ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java
Posted by David E Jones <jo...@hotwaxmedia.com>.
This could go into the release branch. It is the sort of bug that probably wouldn't come up much because of the way this code is generally used... but it is pretty much a bug fix.
-David
Jacques Le Roux wrote:
> 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);
>> + }
>> }
>> }
>>
>>
>