Refactor Groovy EntityListIterator usage with auto-closeable handling… (#1167)

Improved : Updated Groovy files to use safer resource management
patterns for EntityListIterator (OFBIZ-13399)

- Replaced manual iterator close handling with try-with-resources style
implementations where applicable
- Used Groovy withCloseable for automatic resource cleanup
- Replaced EntityListIterator usage with EntityQuery.queryPagedList()
where only partial/paged results were required
- Removed unnecessary manual close() calls
- Reduced risk of JDBC cursor and ResultSet resource leaks
- Improved readability and aligned code with modern OFBiz best practices

Changes were applied to Groovy files; full regression testing has not
yet been completed for all updated flows.

Author: dixitdeepak

applications/accounting/src/main/groovy/org/apache/ofbiz/accounting/reports/SalesInvoiceByProductCategorySummary.groovy

@@ -117,16 +117,17 @@ for (int currentDay = 0; currentDay <= daysInMonth; currentDay++) {
     productAndExprs.add(EntityCondition.makeCondition('invoiceDate', EntityOperator.GREATER_THAN_EQUAL_TO, currentDayBegin))
     productAndExprs.add(EntityCondition.makeCondition('invoiceDate', EntityOperator.LESS_THAN, nextDayBegin))
 
-    productResultListIterator = select('productId', 'quantityTotal', 'amountTotal')
-            .from('InvoiceItemProductSummary').where(productAndExprs).cursorScrollInsensitive().cache(true).queryIterator()
+    productResultListQuery = select('productId', 'quantityTotal', 'amountTotal')
+            .from('InvoiceItemProductSummary').where(productAndExprs).cursorScrollInsensitive().cache(true)
     productResultMap = [:]
-    while ((productResult = productResultListIterator.next()) != null) {
-        productResultMap[productResult.productId] = productResult
-        monthProductResult = UtilMisc.getMapFromMap(monthProductResultMap, productResult.productId)
-        UtilMisc.addToBigDecimalInMap(monthProductResult, 'quantityTotal', productResult.getBigDecimal('quantityTotal'))
-        UtilMisc.addToBigDecimalInMap(monthProductResult, 'amountTotal', productResult.getBigDecimal('amountTotal'))
+    productResultListQuery.queryIterator().withCloseable { productResultListIterator ->
+        while ((productResult = productResultListIterator.next()) != null) {
+            productResultMap[productResult.productId] = productResult
+            monthProductResult = UtilMisc.getMapFromMap(monthProductResultMap, productResult.productId)
+            UtilMisc.addToBigDecimalInMap(monthProductResult, 'quantityTotal', productResult.getBigDecimal('quantityTotal'))
+            UtilMisc.addToBigDecimalInMap(monthProductResult, 'amountTotal', productResult.getBigDecimal('amountTotal'))
+        }
     }
-    productResultListIterator.close()
     productResultMapByDayList.add(productResultMap)
 
     // do the category find
@@ -135,16 +136,17 @@ for (int currentDay = 0; currentDay <= daysInMonth; currentDay++) {
     categoryAndExprs.add(EntityCondition.makeCondition('invoiceDate', EntityOperator.GREATER_THAN_EQUAL_TO, currentDayBegin))
     categoryAndExprs.add(EntityCondition.makeCondition('invoiceDate', EntityOperator.LESS_THAN, nextDayBegin))
 
-    categoryResultListIterator = select('productCategoryId', 'quantityTotal', 'amountTotal')
-            .from('InvoiceItemCategorySummary').where(categoryAndExprs).cursorScrollInsensitive().cache(true).queryIterator()
+    categoryResultListQuery = select('productCategoryId', 'quantityTotal', 'amountTotal')
+            .from('InvoiceItemCategorySummary').where(categoryAndExprs).cursorScrollInsensitive().cache(true)
     categoryResultMap = [:]
-    while ((categoryResult = categoryResultListIterator.next()) != null) {
-        categoryResultMap[categoryResult.productCategoryId] = categoryResult
-        monthCategoryResult = UtilMisc.getMapFromMap(monthCategoryResultMap, categoryResult.productCategoryId)
-        UtilMisc.addToBigDecimalInMap(monthCategoryResult, 'quantityTotal', categoryResult.getBigDecimal('quantityTotal'))
-        UtilMisc.addToBigDecimalInMap(monthCategoryResult, 'amountTotal', categoryResult.getBigDecimal('amountTotal'))
+    categoryResultListQuery.queryIterator().with { categoryResultListIterator ->
+        while ((categoryResult = categoryResultListIterator.next()) != null) {
+            categoryResultMap[categoryResult.productCategoryId] = categoryResult
+            monthCategoryResult = UtilMisc.getMapFromMap(monthCategoryResultMap, categoryResult.productCategoryId)
+            UtilMisc.addToBigDecimalInMap(monthCategoryResult, 'quantityTotal', categoryResult.getBigDecimal('quantityTotal'))
+            UtilMisc.addToBigDecimalInMap(monthCategoryResult, 'amountTotal', categoryResult.getBigDecimal('amountTotal'))
+        }
     }
-    categoryResultListIterator.close()
     categoryResultMapByDayList.add(categoryResultMap)
 
     // do a find for InvoiceItem with a null productId
@@ -153,11 +155,8 @@ for (int currentDay = 0; currentDay <= daysInMonth; currentDay++) {
     productNullAndExprs.add(EntityCondition.makeCondition('productId', EntityOperator.EQUALS, null))
     productNullAndExprs.add(EntityCondition.makeCondition('invoiceDate', EntityOperator.GREATER_THAN_EQUAL_TO, currentDayBegin))
     productNullAndExprs.add(EntityCondition.makeCondition('invoiceDate', EntityOperator.LESS_THAN, nextDayBegin))
-    productNullResultListIterator = select('productId', 'quantityTotal', 'amountTotal')
-            .from('InvoiceItemProductSummary').where(productNullAndExprs).cursorScrollInsensitive().cache(true).queryIterator()
-    // should just be 1 result
-    productNullResult = productNullResultListIterator.next()
-    productNullResultListIterator.close()
+    productNullResult = select('productId', 'quantityTotal', 'amountTotal')
+            .from('InvoiceItemProductSummary').where(productNullAndExprs).cursorScrollInsensitive().cache(true).queryFirst()
     if (productNullResult) {
         productNullResultByDayList.add(productNullResult)
         UtilMisc.addToBigDecimalInMap(monthProductNullResult, 'quantityTotal', productNullResult.getBigDecimal('quantityTotal'))

applications/content/src/main/groovy/org/apache/ofbiz/content/content/GetContentLookupList.groovy

@@ -26,6 +26,7 @@ import org.apache.ofbiz.entity.GenericEntity
 import org.apache.ofbiz.entity.model.ModelField
 import org.apache.ofbiz.entity.model.ModelEntity
 import org.apache.ofbiz.entity.model.ModelReader
+import org.apache.ofbiz.entity.util.EntityListIterator
 
 viewIndex = parameters.VIEW_INDEX ? Integer.valueOf(parameters.VIEW_INDEX) : 0
 viewSize = parameters.VIEW_SIZE ? Integer.valueOf(parameters.VIEW_SIZE) : 20
@@ -68,17 +69,17 @@ context.curFindString = curFindString
 context.viewSize = viewSize
 context.lowIndex = lowIndex
 int arraySize = 0
-List resultPartialList
+List resultPartialList = []
 
 if ((highIndex - lowIndex + 1) > 0) {
     // get the results as an entity list iterator
     boolean beganTransaction = false
-    try {
+    entityQuery = from('ContentAssocViewTo')
+            .where('contentIdStart', (String) parameters.get('contentId'))
+            .orderBy('contentId ASC')
+            .cursorScrollInsensitive().cache(true)
+    try (EntityListIterator listIt = entityQuery.queryIterator()) {
         beganTransaction = TransactionUtil.begin()
-        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()
@@ -96,7 +97,6 @@ if ((highIndex - lowIndex + 1) > 0) {
         // 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)
     }

applications/content/src/main/groovy/org/apache/ofbiz/content/survey/EditSurveyQuestions.groovy

@@ -40,15 +40,14 @@ context.lowIndex = lowIndex
 int listSize = 0
 
 try {
-    listIt = from('SurveyQuestionAndAppl')
+    pagedList = from('SurveyQuestionAndAppl')
                 .where('surveyId', surveyId)
                 .orderBy('sequenceNum')
                 .cursorScrollInsensitive()
                 .cache(true)
-                .queryIterator()
-    surveyQuestionAndApplList = listIt.getPartialList(lowIndex, highIndex - lowIndex + 1)
-
-    listSize = listIt.getResultsSizeAfterPartialList()
+                .queryPagedList(lowIndex, highIndex - lowIndex + 1)
+    surveyQuestionAndApplList = pagedList.getData()
+    listSize = pagedList.getSize()
     if (listSize < highIndex) {
         highIndex = listSize
     }
@@ -58,8 +57,6 @@ try {
     context.listSize = listSize
 } catch (GenericEntityException e) {
     logError(e,  'Failure in ' + module)
-} finally {
-    listIt.close()
 }
 surveyPageList = from('SurveyPage').where('surveyId', surveyId).orderBy('sequenceNum').queryList()
 surveyMultiRespList = from('SurveyMultiResp').where('surveyId', surveyId).orderBy('multiRespTitle').queryList()

applications/order/src/main/groovy/org/apache/ofbiz/order/reports/OpenOrderItemsReport.groovy

@@ -70,14 +70,13 @@ conditions.add(EntityCondition.makeCondition('orderItemStatusId', EntityOperator
 
 // get the results as an entity list iterator
 try {
-    listIt = select('orderId', 'orderDate', 'productId', 'quantityOrdered',
+    listQuery = 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
@@ -87,68 +86,68 @@ try {
     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
+    listQuery.queryIterator().withCloseable { listIt ->
+        while ((listValue = listIt.next()) != null ) {
+            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 ->
-            switch (productPriceMap.productPriceTypeId) {
-                case 'AVERAGE_COST':
-                    costPrice = productPriceMap.price
-                    break
-                case 'DEFAULT_PRICE':
-                    retailPrice = productPriceMap.price
-                    break
-                case 'LIST_PRICE':
-                    listPrice = productPriceMap.price
-                    break
+            productPrices.each { productPriceMap ->
+                switch (productPriceMap.productPriceTypeId) {
+                    case 'AVERAGE_COST':
+                        costPrice = productPriceMap.price
+                        break
+                    case 'DEFAULT_PRICE':
+                        retailPrice = productPriceMap.price
+                        break
+                    case 'LIST_PRICE':
+                        listPrice = productPriceMap.price
+                        break
+                }
             }
-        }
 
-        totalListPrice += listPrice
-        totalRetailPrice += retailPrice
-        totalCostPrice += costPrice
-        totalquantityOrdered += quantityOrdered
-        totalquantityOpen += quantityOpen
-        costPriceDividendValue = costPrice
-        if (costPriceDividendValue) {
-            percentMarkup = ((retailPrice - costPrice) / costPrice) * 100
-        } else {
-            percentMarkup = ''
+            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)
         }
-        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) {
     logError(e, 'Failure in ' + module)
-} finally {
-    listIt.close()
 }
 
 totalAmountList = []

applications/party/src/main/groovy/org/apache/ofbiz/party/party/PartyFinancialHistory.groovy

@@ -19,7 +19,6 @@
 package org.apache.ofbiz.party.party
 
 import java.math.RoundingMode
-
 import org.apache.ofbiz.accounting.invoice.InvoiceWorker
 import org.apache.ofbiz.accounting.payment.PaymentWorker
 import org.apache.ofbiz.entity.condition.EntityCondition
@@ -54,30 +53,28 @@ invExprs =
         ], EntityOperator.OR)
     ], EntityOperator.AND)
 
-invIterator = from('InvoiceAndType').where(invExprs).cursorScrollInsensitive().distinct().queryIterator()
-
-/* codenarc-disable */
-while (invoice = invIterator.next()) {
-/* codenarc-enable */
-    Boolean isPurchaseInvoice = EntityTypeUtil.hasParentType(delegator, 'InvoiceType', 'invoiceTypeId',
-            invoice.getString('invoiceTypeId'), 'parentTypeId', 'PURCHASE_INVOICE')
-    Boolean isSalesInvoice = EntityTypeUtil.hasParentType(delegator, 'InvoiceType', 'invoiceTypeId', (String) invoice.getString('invoiceTypeId'),
-            'parentTypeId', 'SALES_INVOICE')
-    if (isPurchaseInvoice) {
-        totalInvPuApplied += InvoiceWorker.getInvoiceApplied(invoice, actualCurrency).setScale(2, RoundingMode.HALF_UP)
-        totalInvPuNotApplied += InvoiceWorker.getInvoiceNotApplied(invoice, actualCurrency).setScale(2, RoundingMode.HALF_UP)
-    }
-    else if (isSalesInvoice) {
-        totalInvSaApplied += InvoiceWorker.getInvoiceApplied(invoice, actualCurrency).setScale(2, RoundingMode.HALF_UP)
-        totalInvSaNotApplied += InvoiceWorker.getInvoiceNotApplied(invoice, actualCurrency).setScale(2, RoundingMode.HALF_UP)
-    }
-    else {
-        logError('InvoiceType: ' + invoice.invoiceTypeId + ' without a valid parentTypeId: ' + invoice.parentTypeId
-                + ' !!!! Should be either PURCHASE_INVOICE or SALES_INVOICE')
+invQuery = from('InvoiceAndType').where(invExprs).cursorScrollInsensitive().distinct()
+invQuery.queryIterator().withCloseable { invIterator ->
+    /* codenarc-disable */
+        while (invoice = invIterator.next()) {
+    /* codenarc-enable */
+            Boolean isPurchaseInvoice = EntityTypeUtil.hasParentType(delegator, 'InvoiceType', 'invoiceTypeId',
+                    invoice.getString('invoiceTypeId'), 'parentTypeId', 'PURCHASE_INVOICE')
+            Boolean isSalesInvoice = EntityTypeUtil.hasParentType(delegator, 'InvoiceType',
+                        'invoiceTypeId', (String) invoice.getString('invoiceTypeId'),
+                    'parentTypeId', 'SALES_INVOICE')
+            if (isPurchaseInvoice) {
+                totalInvPuApplied += InvoiceWorker.getInvoiceApplied(invoice, actualCurrency).setScale(2, RoundingMode.HALF_UP)
+                totalInvPuNotApplied += InvoiceWorker.getInvoiceNotApplied(invoice, actualCurrency).setScale(2, RoundingMode.HALF_UP)
+            } else if (isSalesInvoice) {
+                totalInvSaApplied += InvoiceWorker.getInvoiceApplied(invoice, actualCurrency).setScale(2, RoundingMode.HALF_UP)
+                totalInvSaNotApplied += InvoiceWorker.getInvoiceNotApplied(invoice, actualCurrency).setScale(2, RoundingMode.HALF_UP)
+            } else {
+                logError('InvoiceType: ' + invoice.invoiceTypeId + ' without a valid parentTypeId: ' + invoice.parentTypeId
+                        + ' !!!! Should be either PURCHASE_INVOICE or SALES_INVOICE')
+            }
+        }
     }
-}
-
-invIterator.close()
 
 //get total/unapplied/applied payment in/out total amount:
 totalPayInApplied = BigDecimal.ZERO
@@ -101,25 +98,23 @@ payExprs =
             ], EntityOperator.OR)
         ], EntityOperator.AND)
 
-payIterator = from('PaymentAndType').where(payExprs).cursorScrollInsensitive().distinct().queryIterator()
-
+payQuery = from('PaymentAndType').where(payExprs).cursorScrollInsensitive().distinct()
+payQuery.queryIterator().withCloseable { payIterator ->
 /* codenarc-disable */
-while (payment = payIterator.next()) {
+    while (payment = payIterator.next()) {
 /* codenarc-enable */
-    if (payment.parentTypeId == 'DISBURSEMENT' || payment.parentTypeId == 'TAX_PAYMENT') {
-        totalPayOutApplied += PaymentWorker.getPaymentApplied(payment, actualCurrency).setScale(2, RoundingMode.HALF_UP)
-        totalPayOutNotApplied += PaymentWorker.getPaymentNotApplied(payment, actualCurrency).setScale(2, RoundingMode.HALF_UP)
-    }
-    else if (payment.parentTypeId == 'RECEIPT') {
-        totalPayInApplied += PaymentWorker.getPaymentApplied(payment, actualCurrency).setScale(2, RoundingMode.HALF_UP)
-        totalPayInNotApplied += PaymentWorker.getPaymentNotApplied(payment, actualCurrency).setScale(2, RoundingMode.HALF_UP)
-    }
-    else {
-        logError('PaymentTypeId: ' + payment.paymentTypeId + ' without a valid parentTypeId: ' + payment.parentTypeId
-                + ' !!!! Should be either DISBURSEMENT, TAX_PAYMENT or RECEIPT')
+        if (payment.parentTypeId == 'DISBURSEMENT' || payment.parentTypeId == 'TAX_PAYMENT') {
+            totalPayOutApplied += PaymentWorker.getPaymentApplied(payment, actualCurrency).setScale(2, RoundingMode.HALF_UP)
+            totalPayOutNotApplied += PaymentWorker.getPaymentNotApplied(payment, actualCurrency).setScale(2, RoundingMode.HALF_UP)
+        } else if (payment.parentTypeId == 'RECEIPT') {
+            totalPayInApplied += PaymentWorker.getPaymentApplied(payment, actualCurrency).setScale(2, RoundingMode.HALF_UP)
+            totalPayInNotApplied += PaymentWorker.getPaymentNotApplied(payment, actualCurrency).setScale(2, RoundingMode.HALF_UP)
+        } else {
+            logError('PaymentTypeId: ' + payment.paymentTypeId + ' without a valid parentTypeId: ' + payment.parentTypeId
+                    + ' !!!! Should be either DISBURSEMENT, TAX_PAYMENT or RECEIPT')
+        }
     }
 }
-payIterator.close()
 
 context.finanSummary = [:]
 context.finanSummary.totalSalesInvoice = totalSalesInvoice = totalInvSaApplied.add(totalInvSaNotApplied)