svn commit: r1798300 - /ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/product/ProductSearchEvents.java

Author: jleroux
Date: Sat Jun 10 09:12:05 2017
New Revision: 1798300

URL: http://svn.apache.org/viewvc?rev=1798300&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.

The solution is to use try-with-ressources

The getProductSearchResults() method was public but only used in the class.
I made it private

Modified:
    ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/product/ProductSearchEvents.java

Modified: ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/product/ProductSearchEvents.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/product/ProductSearchEvents.java?rev=1798300&r1=1798299&r2=1798300&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/product/ProductSearchEvents.java (original)
+++ ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/product/ProductSearchEvents.java Sat Jun 10 09:12:05 2017
@@ -35,7 +35,6 @@ import org.apache.ofbiz.base.util.UtilHt
 import org.apache.ofbiz.base.util.UtilMisc;
 import org.apache.ofbiz.base.util.UtilProperties;
 import org.apache.ofbiz.base.util.UtilValidate;
-import org.apache.ofbiz.webapp.stats.VisitHandler;
 import org.apache.ofbiz.entity.Delegator;
 import org.apache.ofbiz.entity.GenericEntityException;
 import org.apache.ofbiz.entity.GenericValue;
@@ -45,6 +44,7 @@ import org.apache.ofbiz.entity.util.Enti
 import org.apache.ofbiz.entity.util.EntityQuery;
 import org.apache.ofbiz.product.product.ProductSearch.ProductSearchContext;
 import org.apache.ofbiz.product.product.ProductSearch.ResultSortOrder;
+import org.apache.ofbiz.webapp.stats.VisitHandler;
 
 /**
  * Product Search Related Events
@@ -67,8 +67,7 @@ public class ProductSearchEvents {
 
         try {
             boolean beganTransaction = TransactionUtil.begin(DEFAULT_TX_TIMEOUT);
-            try {
-                EntityListIterator eli = getProductSearchResults(request);
+            try (EntityListIterator eli = getProductSearchResults(request)) {
                 if (eli == null) {
                     errMsg = UtilProperties.getMessage(resource,"productsearchevents.no_results_found_probably_error_constraints", UtilHttp.getLocale(request));
                     request.setAttribute("_ERROR_MESSAGE_", errMsg);
@@ -81,7 +80,6 @@ public class ProductSearchEvents {
                     String productId = searchResultView.getString("mainProductId");
                     numRemoved += delegator.removeByAnd("ProductCategoryMember", UtilMisc.toMap("productCategoryId", productCategoryId, "productId", productId)) ;
                 }
-                eli.close();
                 Map<String, String> messageMap = UtilMisc.toMap("numRemoved", Integer.toString(numRemoved));
                 errMsg = UtilProperties.getMessage(resource,"productsearchevents.removed_x_items", messageMap, UtilHttp.getLocale(request));
                 request.setAttribute("_EVENT_MESSAGE_", errMsg);
@@ -129,8 +127,7 @@ public class ProductSearchEvents {
 
        try {
            boolean beganTransaction = TransactionUtil.begin(DEFAULT_TX_TIMEOUT);
-           try {
-               EntityListIterator eli = getProductSearchResults(request);
+           try (EntityListIterator eli = getProductSearchResults(request)) {
                if (eli == null) {
                    errMsg = UtilProperties.getMessage(resource,"productsearchevents.no_results_found_probably_error_constraints", UtilHttp.getLocale(request));
                    request.setAttribute("_ERROR_MESSAGE_", errMsg);
@@ -156,7 +153,6 @@ public class ProductSearchEvents {
                Map<String, String> messageMap = UtilMisc.toMap("numExpired", Integer.toString(numExpired));
                errMsg = UtilProperties.getMessage(resource,"productsearchevents.expired_x_items", messageMap, UtilHttp.getLocale(request));
                request.setAttribute("_EVENT_MESSAGE_", errMsg);
-               eli.close();
            } catch (GenericEntityException e) {
                Map<String, String> messageMap = UtilMisc.toMap("errSearchResult", e.toString());
                errMsg = UtilProperties.getMessage(resource,"productsearchevents.error_getting_search_results", messageMap, UtilHttp.getLocale(request));
@@ -201,8 +197,7 @@ public class ProductSearchEvents {
 
        try {
            boolean beganTransaction = TransactionUtil.begin(DEFAULT_TX_TIMEOUT);
-           try {
-               EntityListIterator eli = getProductSearchResults(request);
+           try (EntityListIterator eli = getProductSearchResults(request)) {
                if (eli == null) {
                    errMsg = UtilProperties.getMessage(resource,"productsearchevents.no_results_found_probably_error_constraints", UtilHttp.getLocale(request));
                    request.setAttribute("_ERROR_MESSAGE_", errMsg);
@@ -225,7 +220,6 @@ public class ProductSearchEvents {
                Map<String, String> messageMap = UtilMisc.toMap("numAdded", Integer.toString(numAdded));
                errMsg = UtilProperties.getMessage(resource,"productsearchevents.added_x_product_category_members", messageMap, UtilHttp.getLocale(request));
                request.setAttribute("_EVENT_MESSAGE_", errMsg);
-               eli.close();
            } catch (GenericEntityException e) {
                Map<String, String> messageMap = UtilMisc.toMap("errSearchResult", e.toString());
                errMsg = UtilProperties.getMessage(resource,"productsearchevents.error_getting_search_results", messageMap, UtilHttp.getLocale(request));
@@ -290,8 +284,7 @@ public class ProductSearchEvents {
 
         try {
             boolean beganTransaction = TransactionUtil.begin(DEFAULT_TX_TIMEOUT);
-            try {
-                EntityListIterator eli = getProductSearchResults(request);
+            try (EntityListIterator eli = getProductSearchResults(request)) {
                 if (eli == null) {
                     String errMsg = UtilProperties.getMessage(resource,"productsearchevents.no_results_found_probably_error_constraints", UtilHttp.getLocale(request));
                     request.setAttribute("_ERROR_MESSAGE_", errMsg);
@@ -316,7 +309,6 @@ public class ProductSearchEvents {
                 Map<String, Object> messageMap = UtilMisc.toMap("numAdded", Integer.valueOf(numAdded), "productFeatureId", productFeatureId);
                 String eventMsg = UtilProperties.getMessage(resource, "productSearchEvents.added_param_features", messageMap, locale) + ".";
                 request.setAttribute("_EVENT_MESSAGE_", eventMsg);
-                eli.close();
             } catch (GenericEntityException e) {
                 String errorMsg = UtilProperties.getMessage(resource, "productSearchEvents.error_getting_results", locale) + " : " + e.toString();
                 request.setAttribute("_ERROR_MESSAGE_", errorMsg);
@@ -349,8 +341,7 @@ public class ProductSearchEvents {
 
         try {
             boolean beganTransaction = TransactionUtil.begin(DEFAULT_TX_TIMEOUT);
-            try {
-                EntityListIterator eli = getProductSearchResults(request);
+            try (EntityListIterator eli = getProductSearchResults(request)) {
                 if (eli == null) {
                     String errMsg = UtilProperties.getMessage(resource,"productsearchevents.no_results_found_probably_error_constraints", UtilHttp.getLocale(request));
                     request.setAttribute("_ERROR_MESSAGE_", errMsg);
@@ -366,7 +357,6 @@ public class ProductSearchEvents {
                 Map<String, Object> messageMap = UtilMisc.toMap("numRemoved", Integer.valueOf(numRemoved), "productFeatureId", productFeatureId);
                 String eventMsg = UtilProperties.getMessage(resource, "productSearchEvents.removed_param_features", messageMap, locale) + ".";
                 request.setAttribute("_EVENT_MESSAGE_", eventMsg);
-                eli.close();
             } catch (GenericEntityException e) {
                 String errorMsg = UtilProperties.getMessage(resource, "productSearchEvents.error_getting_results", locale) + " : " + e.toString();
                 request.setAttribute("_ERROR_MESSAGE_", errorMsg);
@@ -398,8 +388,7 @@ public class ProductSearchEvents {
 
         try {
             boolean beganTransaction = TransactionUtil.begin(DEFAULT_TX_TIMEOUT);
-            try {
-                EntityListIterator eli = getProductSearchResults(request);
+            try (EntityListIterator eli = getProductSearchResults(request)) {
                 if (eli == null) {
                     errMsg = UtilProperties.getMessage(resource,"productsearchevents.no_results_found_probably_error_constraints", UtilHttp.getLocale(request));
                     request.setAttribute("_ERROR_MESSAGE_", errMsg);
@@ -420,7 +409,6 @@ public class ProductSearchEvents {
                     productMap.put("productFeatures", productFeatures);
                     productExportList.add(productMap);
                 }
-                eli.close();
             } catch (GenericEntityException e) {
                 Map<String, String> messageMap = UtilMisc.toMap("errSearchResult", e.toString());
                 errMsg = UtilProperties.getMessage(resource,"productsearchevents.error_getting_search_results", messageMap, UtilHttp.getLocale(request));
@@ -443,7 +431,7 @@ public class ProductSearchEvents {
         return "success";
     }
 
-    public static EntityListIterator getProductSearchResults(HttpServletRequest request) {
+    private static EntityListIterator getProductSearchResults(HttpServletRequest request) {
         HttpSession session = request.getSession();
         Delegator delegator = (Delegator) request.getAttribute("delegator");
         String visitId = VisitHandler.getVisitId(session);