ofbiz-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jacques Le Roux <jacques.le.r...@les7arts.com>
Subject Re: svn commit: r1798353 - in /ofbiz/ofbiz-framework/trunk/applications: content/src/main/java/org/apache/ofbiz/content/survey/SurveyWrapper.java product/src/main/java/org/apache/ofbiz/product/product/ProductEvents.java
Date Sun, 11 Jun 2017 12:23:40 GMT
Hi Taher, Michael

Sorry guys, missed this ugly one due to a previouysly abandoned refactoring and stupidly trusting
an IDE suggestion. I did not see your messages 
because I was "in the flow" and did not retrieves emails. I then saw Buildbot and your messages,
a bit late I must say...

Fixed with r1798358

Jacques


Le 11/06/2017 à 13:22, Michael Brohl a écrit :
> I agree that we should not use buildbot as the first instance to test changes for a commit.
This has to be done by the committer in his own sandbox, 
> with no other present changes. I know it's more complicated using svn instead of git,
where you can more easily work on different feature branches, 
> but we need a reliable code base.
>
> Jacques, please change your approach, it's confusing and might prevent others from doing
their work. Thank you.
>
> I really appreciate your efforts to improve the codebase  but I'm also not sure if it
is good invested time to fix something which is not broken (or 
> not reported to be broken) in this case of EntityListIterator. I cannot dig deeper there
but I have the impression that this is a mechanical 
> approach and it does not seem to fit in all cases.
>
> Best regards,
>
> Michael Brohl
> ecomify GmbH
> www.ecomify.de
>
>
> Am 11.06.17 um 13:03 schrieb Taher Alkhateeb:
>> Hi Jacques, the build is still crashing, and I suspect this is from your
>> commit r1798335. Again, this means you're committing without testing and
>> doing "gradlew cleanAll loadDefault testIntegration".
>>
>> I also think this mass searching for try blocks in the code base and
>> changing them to try-with-resources is not being properly studied (the
>> proof of which is your reverts). I recommend not taking that approach but
>> instead refactoring slowly each code segment in isolation with proper
>> review and study.
>>
>> I'm still waiting for you to revert the code which is crashing the system
>> as I'm trying to test some code. Would appreciate fixing it.
>>
>> On Sun, Jun 11, 2017 at 1:12 PM, <jleroux@apache.org> wrote:
>>
>>> Author: jleroux
>>> Date: Sun Jun 11 10:12:32 2017
>>> New Revision: 1798353
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1798353&view=rev
>>> Log:
>>> Fixed: ERROR: Cannot do a find that returns an EntityListIterator with no
>>> transaction in place
>>> (OFBIZ-9286)
>>>
>>> While "Use try-with-resources statement wherever it's possible" I missed
>>> that
>>> you must begin the transaction before the try-with-resources, this fixes
>>> last
>>> cases, lesson learned!
>>>
>>> Modified:
>>> ofbiz/ofbiz-framework/trunk/applications/content/src/main/
>>> java/org/apache/ofbiz/content/survey/SurveyWrapper.java
>>> ofbiz/ofbiz-framework/trunk/applications/product/src/main/
>>> java/org/apache/ofbiz/product/product/ProductEvents.java
>>>
>>> Modified: ofbiz/ofbiz-framework/trunk/applications/content/src/main/
>>> java/org/apache/ofbiz/content/survey/SurveyWrapper.java
>>> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/
>>> applications/content/src/main/java/org/apache/ofbiz/content/
>>> survey/SurveyWrapper.java?rev=1798353&r1=1798352&r2=1798353&view=diff
>>> ============================================================
>>> ==================
>>> --- ofbiz/ofbiz-framework/trunk/applications/content/src/main/
>>> java/org/apache/ofbiz/content/survey/SurveyWrapper.java (original)
>>> +++ ofbiz/ofbiz-framework/trunk/applications/content/src/main/
>>> java/org/apache/ofbiz/content/survey/SurveyWrapper.java Sun Jun 11
>>> 10:12:32 2017
>>> @@ -43,6 +43,7 @@ import org.apache.ofbiz.entity.GenericEn
>>>   import org.apache.ofbiz.entity.GenericValue;
>>>   import org.apache.ofbiz.entity.condition.EntityCondition;
>>>   import org.apache.ofbiz.entity.condition.EntityOperator;
>>> +import org.apache.ofbiz.entity.transaction.GenericTransactionException;
>>>   import org.apache.ofbiz.entity.transaction.TransactionUtil;
>>>   import org.apache.ofbiz.entity.util.EntityListIterator;
>>>   import org.apache.ofbiz.entity.util.EntityQuery;
>>> @@ -398,8 +399,12 @@ public class SurveyWrapper {
>>>           List<GenericValue> resp = null;
>>>           boolean beganTransaction = false;
>>>           int maxRows = startIndex + number;
>>> -        try (EntityListIterator eli = this.getEli(question, maxRows)) {
>>> +        try {
>>>               beganTransaction = TransactionUtil.begin();
>>> +        } catch (GenericTransactionException gte) {
>>> +            Debug.logError(gte, "Unable to begin transaction", module);
>>> +        }
>>> +        try (EntityListIterator eli = this.getEli(question, maxRows)) {
>>>               if (startIndex > 0 && number > 0) {
>>>                   resp = eli.getPartialList(startIndex, number);
>>>               } else {
>>> @@ -574,9 +579,13 @@ public class SurveyWrapper {
>>>           // index 2 = average
>>>
>>>           boolean beganTransaction = false;
>>> -        try (EntityListIterator eli = this.getEli(question, -1)) {
>>> +        try {
>>>               beganTransaction = TransactionUtil.begin();
>>> -
>>> +        } catch (GenericTransactionException gte) {
>>> +            Debug.logError(gte, "Unable to begin transaction", module);
>>> +        }
>>> +
>>> +        try (EntityListIterator eli = this.getEli(question, -1)) {
>>>               if (eli != null) {
>>>                   GenericValue value;
>>>                   while (((value = eli.next()) != null)) {
>>> @@ -660,8 +669,13 @@ public class SurveyWrapper {
>>>           long total = 0;
>>>
>>>           boolean beganTransaction = false;
>>> -        try (EntityListIterator eli = this.getEli(question, -1)) {
>>> +        try {
>>>               beganTransaction = TransactionUtil.begin();
>>> +        } catch (GenericTransactionException gte) {
>>> +            Debug.logError(gte, "Unable to begin transaction", module);
>>> +        }
>>> +
>>> +        try (EntityListIterator eli = this.getEli(question, -1)) {
>>>               if (eli != null) {
>>>                   GenericValue value;
>>>                   while (((value = eli.next()) != null)) {
>>>
>>> Modified: ofbiz/ofbiz-framework/trunk/applications/product/src/main/
>>> java/org/apache/ofbiz/product/product/ProductEvents.java
>>> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/
>>> applications/product/src/main/java/org/apache/ofbiz/product/
>>> product/ProductEvents.java?rev=1798353&r1=1798352&r2=1798353&view=diff
>>> ============================================================
>>> ==================
>>> --- ofbiz/ofbiz-framework/trunk/applications/product/src/main/
>>> java/org/apache/ofbiz/product/product/ProductEvents.java (original)
>>> +++ ofbiz/ofbiz-framework/trunk/applications/product/src/main/
>>> java/org/apache/ofbiz/product/product/ProductEvents.java Sun Jun 11
>>> 10:12:32 2017
>>> @@ -113,9 +113,13 @@ public class ProductEvents {
>>>           int errProds = 0;
>>>
>>>           boolean beganTx = false;
>>> -        try (EntityListIterator  entityListIterator =
>>> EntityQuery.use(delegator).from("Product").where(condition).queryIterator())
>>> {
>>> +        try {
>>>               // begin the transaction
>>>               beganTx = TransactionUtil.begin(7200);
>>> +        } catch (GenericTransactionException gte) {
>>> +            Debug.logError(gte, "Unable to begin transaction", module);
>>> +        }
>>> +        try (EntityListIterator  entityListIterator =
>>> EntityQuery.use(delegator).from("Product").where(condition).queryIterator())
>>> {
>>>               try {
>>>                   if (Debug.infoOn()) {
>>>                       long count = EntityQuery.use(delegator).
>>> from("Product").where(condition).queryCount();
>>> @@ -126,7 +130,7 @@ public class ProductEvents {
>>>                   Map<String, String> messageMap = UtilMisc.toMap("gee",
>>> gee.toString());
>>>                   errMsg = UtilProperties.getMessage(
>>> resource,"productevents.error_getting_product_list", messageMap,
>>> UtilHttp.getLocale(request));
>>>                   request.setAttribute("_ERROR_MESSAGE_", errMsg);
>>> -                throw gee;
>>> +                return "error";
>>>               }
>>>
>>>               GenericValue product;
>>>
>>>
>>>
>
>


Mime
View raw message