svn commit: r1797373 - in /ofbiz/ofbiz-framework/trunk/applications: content/groovyScripts/content/ content/groovyScripts/survey/ order/groovyScripts/reports/ party/groovyScripts/visit/

Author: jleroux
Date: Fri Jun  2 11:25:17 2017
New Revision: 1797373

URL: http://svn.apache.org/viewvc?rev=1797373&view=rev
Log:
Improved: EntityListIterator closed but not in case of exception
(OFBIZ-9385)

This is an improvement only because no cases were reported. But obviously in
case of unlucky exception after the EntityListIterator creation and before it's
closed the EntityListIterator remains in memory.
It should be closed in EntityListIterator.finalize() but the less happens there
the better.

I noticed Groovy has no try-with-ressources statement. I put a comment in
the description of OFBIZ-9296 "Update Gradle to 2.4.10" Maybe a solution would
be to use streams but really WTF! It seems Gradle 2.5 has autocloseable...

This contains also some refactoring and code cleaning



Modified:
    ofbiz/ofbiz-framework/trunk/applications/content/groovyScripts/content/GetContentLookupList.groovy
    ofbiz/ofbiz-framework/trunk/applications/content/groovyScripts/survey/EditSurveyQuestions.groovy
    ofbiz/ofbiz-framework/trunk/applications/order/groovyScripts/reports/OpenOrderItemsReport.groovy
    ofbiz/ofbiz-framework/trunk/applications/party/groovyScripts/visit/ShowVisits.groovy

Modified: ofbiz/ofbiz-framework/trunk/applications/content/groovyScripts/content/GetContentLookupList.groovy
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/content/groovyScripts/content/GetContentLookupList.groovy?rev=1797373&r1=1797372&r2=1797373&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/applications/content/groovyScripts/content/GetContentLookupList.groovy (original)
+++ ofbiz/ofbiz-framework/trunk/applications/content/groovyScripts/content/GetContentLookupList.groovy Fri Jun  2 11:25:17 2017
@@ -28,11 +28,13 @@
  import org.apache.ofbiz.entity.model.ModelEntity
  import org.apache.ofbiz.entity.model.ModelReader
 
-try {
-    viewIndex = Integer.valueOf((String)parameters.get("VIEW_INDEX")).intValue()
-} catch (NumberFormatException nfe) {
-        viewIndex = 0
-}
+module = "GetContentLookupList.groovy"
+
+viewIndex = parameters.VIEW_INDEX ? Integer.valueOf(parameters.VIEW_INDEX) : 0
+viewSize = parameters.VIEW_SIZE ? Integer.valueOf(parameters.VIEW_SIZE) : 20
+
+int lowIndex = viewIndex*viewSize+1
+int highIndex = (viewIndex+1)*viewSize
 
 context.viewIndexFirst = 0
 context.viewIndex = viewIndex
@@ -54,7 +56,7 @@ while (fieldIterator.hasNext()) {
             try {
                 findByEntity.setString(field.getName(), fval)
             } catch (NumberFormatException nfe) {
-                Debug.logError(nfe, "Caught an exception : " + nfe.toString(), "GetContentLookupList.groovy")
+                Debug.logError(nfe, "Caught an exception : " + nfe.toString(), module)
                 errMsgList.add("Entered value is non-numeric for numeric field: " + field.getName())
             }
         }
@@ -66,50 +68,38 @@ if (errMsgList) {
 
 curFindString = UtilFormatOut.encodeQuery(curFindString)
 context.curFindString = curFindString
-try {
-    viewSize = Integer.valueOf((String)parameters.get("VIEW_SIZE")).intValue()
-} catch (NumberFormatException nfe) {
-    Debug.logError(nfe, "Caught an exception : " + nfe.toString(), "GetContentLookupList.groovy")
-}
-
 context.viewSize = viewSize
-
-int lowIndex = viewIndex*viewSize+1
-int highIndex = (viewIndex+1)*viewSize
-
 context.lowIndex = lowIndex
 int arraySize = 0
-List resultPartialList = null
+List resultPartialList
 
 if ((highIndex - lowIndex + 1) > 0) {
     // get the results as an entity list iterator
     boolean beganTransaction = false
-    if(resultPartialList==null){
     try {
         beganTransaction = TransactionUtil.begin()
-        EntityListIterator listIt = from("ContentAssocViewTo").where("contentIdStart", (String)parameters.get("contentId")).orderBy("contentId ASC").cursorScrollInsensitive().cache(true).queryIterator()
+        listIt = from("ContentAssocViewTo").where("contentIdStart", (String)parameters.get("contentId")).orderBy("contentId ASC").cursorScrollInsensitive().cache(true).queryIterator()
         resultPartialList = listIt.getPartialList(lowIndex, highIndex - lowIndex + 1)
         
         arraySize = listIt.getResultsSizeAfterPartialList()
         if (arraySize < highIndex) {
             highIndex = arraySize
         }
-        listIt.close()
     } catch (GenericEntityException e) {
-        Debug.logError(e, "Failure in operation, rolling back transaction", "GetContentLookupList.groovy")
+        Debug.logError(e, "Failure in operation, rolling back transaction", module)
         try {
             // only rollback the transaction if we started one...
             TransactionUtil.rollback(beganTransaction, "Error looking up entity values in WebTools Entity Data Maintenance", e)
         } catch (GenericEntityException e2) {
-            Debug.logError(e2, "Could not rollback transaction: " + e2.toString(), "GetContentLookupList.groovy")
+            Debug.logError(e2, "Could not rollback transaction: " + e2.toString(), module)
         }
         // after rolling back, rethrow the exception
         throw e
     } finally {
+        listIt.close()
         // only commit the transaction if we started one... this will throw an exception if it fails
         TransactionUtil.commit(beganTransaction)
     }
-    }
 }
 context.highIndex = highIndex
 context.arraySize = arraySize

Modified: ofbiz/ofbiz-framework/trunk/applications/content/groovyScripts/survey/EditSurveyQuestions.groovy
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/content/groovyScripts/survey/EditSurveyQuestions.groovy?rev=1797373&r1=1797372&r2=1797373&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/applications/content/groovyScripts/survey/EditSurveyQuestions.groovy (original)
+++ ofbiz/ofbiz-framework/trunk/applications/content/groovyScripts/survey/EditSurveyQuestions.groovy Fri Jun  2 11:25:17 2017
@@ -21,6 +21,7 @@ import org.apache.ofbiz.entity.*
 import org.apache.ofbiz.entity.condition.*
 import org.apache.ofbiz.base.util.*
 
+module = "EditSurveyQuestions.groovy"
 surveyQuestionId = parameters.surveyQuestionId
 context.surveyQuestionId = surveyQuestionId
 
@@ -40,19 +41,29 @@ context.viewSize = viewSize
 context.lowIndex = lowIndex
 int listSize = 0
 
-listIt = from("SurveyQuestionAndAppl").where("surveyId", surveyId).orderBy("sequenceNum").cursorScrollInsensitive().cache(true).queryIterator()
-surveyQuestionAndApplList = listIt.getPartialList(lowIndex, highIndex - lowIndex + 1)
-
-listSize = listIt.getResultsSizeAfterPartialList()
-if (listSize < highIndex) {
-    highIndex = listSize
+try {
+    listIt = from("SurveyQuestionAndAppl")
+                .where("surveyId", surveyId)
+                .orderBy("sequenceNum")
+                .cursorScrollInsensitive()
+                .cache(true)
+                .queryIterator()
+    surveyQuestionAndApplList = listIt.getPartialList(lowIndex, highIndex - lowIndex + 1)
+    
+    listSize = listIt.getResultsSizeAfterPartialList()
+    if (listSize < highIndex) {
+        highIndex = listSize
+    }
+    
+    context.viewIndexLast = (int) (listSize / viewSize)
+    context.highIndex = highIndex
+    context.listSize = listSize
+} catch (GenericEntityException e) {
+    Debug.logError(e, "Failure in " + module)
+} finally {
+    listIt.close()
 }
 
-context.viewIndexLast = (int) (listSize / viewSize)
-context.highIndex = highIndex
-context.listSize = listSize
-listIt.close()
-
 surveyPageList = from("SurveyPage").where("surveyId", surveyId).orderBy("sequenceNum").queryList()
 surveyMultiRespList = from("SurveyMultiResp").where("surveyId", surveyId).orderBy("multiRespTitle").queryList()
 

Modified: ofbiz/ofbiz-framework/trunk/applications/order/groovyScripts/reports/OpenOrderItemsReport.groovy
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/order/groovyScripts/reports/OpenOrderItemsReport.groovy?rev=1797373&r1=1797372&r2=1797373&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/applications/order/groovyScripts/reports/OpenOrderItemsReport.groovy (original)
+++ ofbiz/ofbiz-framework/trunk/applications/order/groovyScripts/reports/OpenOrderItemsReport.groovy Fri Jun  2 11:25:17 2017
@@ -29,6 +29,7 @@ import org.apache.ofbiz.entity.util.*
 import org.apache.ofbiz.entity.*
 import org.apache.ofbiz.base.util.*
 
+module = "OpenOrderItemsReport.groovy"
 productStoreId = ObjectType.simpleTypeConvert(parameters.productStoreId, "List", null, null)
 orderTypeId = ObjectType.simpleTypeConvert(parameters.orderTypeId, "List", null, null)
 orderStatusId = ObjectType.simpleTypeConvert(parameters.orderStatusId, "List", null, null)
@@ -71,78 +72,83 @@ conditions.add(EntityCondition.makeCondi
 conditions.add(EntityCondition.makeCondition("orderItemStatusId", EntityOperator.NOT_EQUAL, "ITEM_REJECTED"))
 
 // get the results as an entity list iterator
-listIt = select("orderId", "orderDate", "productId", "quantityOrdered", "quantityIssued", "quantityOpen", "shipBeforeDate", "shipAfterDate", "itemDescription")
-            .from("OrderItemQuantityReportGroupByItem")
-            .where(conditions)
-            .orderBy("orderDate DESC")
-            .cursorScrollInsensitive()
-            .distinct()
-            .queryIterator()
-orderItemList = []
-totalCostPrice = 0.0
-totalListPrice = 0.0
-totalMarkup = 0.0
-totalDiscount = 0.0
-totalRetailPrice = 0.0
-totalquantityOrdered = 0.0
-totalquantityOpen = 0.0
-
-listIt.each { listValue ->
-    orderId = listValue.orderId
-    productId = listValue.productId
-    orderDate = listValue.orderDate
-    quantityOrdered = listValue.quantityOrdered
-    quantityOpen = listValue.quantityOpen
-    quantityIssued = listValue.quantityIssued
-    itemDescription = listValue.itemDescription
-    shipAfterDate = listValue.shipAfterDate
-    shipBeforeDate = listValue.shipBeforeDate
-    productIdCondExpr =  [EntityCondition.makeCondition("productId", EntityOperator.EQUALS, productId)]
-    productPrices = select("price","productPriceTypeId").from("ProductPrice").where(productIdCondExpr).queryList()
-    costPrice = 0.0
-    retailPrice = 0.0
-    listPrice = 0.0
-
-    productPrices.each { productPriceMap ->
-        if ("AVERAGE_COST".equals(productPriceMap.productPriceTypeId)) {
-            costPrice = productPriceMap.price
-        } else if ("DEFAULT_PRICE".equals(productPriceMap.productPriceTypeId)) {
-            retailPrice = productPriceMap.price
-        } else if ("LIST_PRICE".equals(productPriceMap.productPriceTypeId)) {
-            listPrice = productPriceMap.price
+try {
+    listIt = select("orderId", "orderDate", "productId", "quantityOrdered", "quantityIssued", "quantityOpen", "shipBeforeDate", "shipAfterDate", "itemDescription")
+                .from("OrderItemQuantityReportGroupByItem")
+                .where(conditions)
+                .orderBy("orderDate DESC")
+                .cursorScrollInsensitive()
+                .distinct()
+                .queryIterator()
+    orderItemList = []
+    totalCostPrice = 0.0
+    totalListPrice = 0.0
+    totalMarkup = 0.0
+    totalDiscount = 0.0
+    totalRetailPrice = 0.0
+    totalquantityOrdered = 0.0
+    totalquantityOpen = 0.0
+
+    listIt.each { listValue ->
+        orderId = listValue.orderId
+        productId = listValue.productId
+        orderDate = listValue.orderDate
+        quantityOrdered = listValue.quantityOrdered
+        quantityOpen = listValue.quantityOpen
+        quantityIssued = listValue.quantityIssued
+        itemDescription = listValue.itemDescription
+        shipAfterDate = listValue.shipAfterDate
+        shipBeforeDate = listValue.shipBeforeDate
+        productIdCondExpr =  [EntityCondition.makeCondition("productId", EntityOperator.EQUALS, productId)]
+        productPrices = select("price","productPriceTypeId").from("ProductPrice").where(productIdCondExpr).queryList()
+        costPrice = 0.0
+        retailPrice = 0.0
+        listPrice = 0.0
+    
+        productPrices.each { productPriceMap ->
+            if ("AVERAGE_COST".equals(productPriceMap.productPriceTypeId)) {
+                costPrice = productPriceMap.price
+            } else if ("DEFAULT_PRICE".equals(productPriceMap.productPriceTypeId)) {
+                retailPrice = productPriceMap.price
+            } else if ("LIST_PRICE".equals(productPriceMap.productPriceTypeId)) {
+                listPrice = productPriceMap.price
+            }
         }
+    
+        totalListPrice += listPrice
+        totalRetailPrice += retailPrice
+        totalCostPrice += costPrice
+        totalquantityOrdered += quantityOrdered
+        totalquantityOpen += quantityOpen
+        costPriceDividendValue = costPrice
+        if (costPriceDividendValue) {
+            percentMarkup = ((retailPrice - costPrice)/costPrice)*100
+        } else{
+            percentMarkup = ""
+        }
+        orderItemMap = [orderDate : orderDate,
+                        orderId : orderId,
+                        productId : productId,
+                        itemDescription : itemDescription,
+                        quantityOrdered : quantityOrdered,
+                        quantityIssued : quantityIssued,
+                        quantityOpen : quantityOpen,
+                        shipAfterDate : shipAfterDate,
+                        shipBeforeDate : shipBeforeDate,
+                        costPrice : costPrice,
+                        retailPrice : retailPrice,
+                        listPrice : listPrice,
+                        discount : listPrice - retailPrice,
+                        calculatedMarkup : retailPrice - costPrice,
+                        percentMarkup : percentMarkup]
+        orderItemList.add(orderItemMap)
     }
-
-    totalListPrice += listPrice
-    totalRetailPrice += retailPrice
-    totalCostPrice += costPrice
-    totalquantityOrdered += quantityOrdered
-    totalquantityOpen += quantityOpen
-    costPriceDividendValue = costPrice
-    if (costPriceDividendValue) {
-        percentMarkup = ((retailPrice - costPrice)/costPrice)*100
-    } else{
-        percentMarkup = ""
-    }
-    orderItemMap = [orderDate : orderDate,
-                    orderId : orderId,
-                    productId : productId,
-                    itemDescription : itemDescription,
-                    quantityOrdered : quantityOrdered,
-                    quantityIssued : quantityIssued,
-                    quantityOpen : quantityOpen,
-                    shipAfterDate : shipAfterDate,
-                    shipBeforeDate : shipBeforeDate,
-                    costPrice : costPrice,
-                    retailPrice : retailPrice,
-                    listPrice : listPrice,
-                    discount : listPrice - retailPrice,
-                    calculatedMarkup : retailPrice - costPrice,
-                    percentMarkup : percentMarkup]
-    orderItemList.add(orderItemMap)
+} catch (GenericEntityException e) {
+    Debug.logError(e, "Failure in " + module)
+} finally {
+    listIt.close()
 }
 
-listIt.close()
 totalAmountList = []
 if (orderItemList) {
     totalCostPriceDividendValue = totalCostPrice

Modified: ofbiz/ofbiz-framework/trunk/applications/party/groovyScripts/visit/ShowVisits.groovy
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/party/groovyScripts/visit/ShowVisits.groovy?rev=1797373&r1=1797372&r2=1797373&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/applications/party/groovyScripts/visit/ShowVisits.groovy (original)
+++ ofbiz/ofbiz-framework/trunk/applications/party/groovyScripts/visit/ShowVisits.groovy Fri Jun  2 11:25:17 2017
@@ -70,7 +70,6 @@ try {
     }
     context.visitSize = visitListSize
 
-    visitListIt.close()
 } catch (Exception e) {
     String errMsg = "Failure in operation, rolling back transaction"
     Debug.logError(e, errMsg, module)
@@ -84,7 +83,9 @@ try {
     throw e
 } finally {
     // only commit the transaction if we started one... this will throw an exception if it fails
+    visitListIt.close()
     TransactionUtil.commit(beganTransaction)
+    }
 }
 
 context.visitList = visitList