svn commit: r1797158 - in /ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product: product/ProductUtilServices.java promo/PromoServices.java

Author: jleroux
Date: Thu Jun  1 08:10:51 2017
New Revision: 1797158

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

Modified:
    ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/product/ProductUtilServices.java
    ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/promo/PromoServices.java

Modified: ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/product/ProductUtilServices.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/product/ProductUtilServices.java?rev=1797158&r1=1797157&r2=1797158&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/product/ProductUtilServices.java (original)
+++ ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/product/ProductUtilServices.java Thu Jun  1 08:10:51 2017
@@ -22,9 +22,9 @@ import java.sql.Timestamp;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 import java.util.Set;
-import java.util.Locale;
 
 import org.apache.ofbiz.base.util.Debug;
 import org.apache.ofbiz.base.util.StringUtil;
@@ -68,14 +68,13 @@ public final class ProductUtilServices {
         Timestamp nowTimestamp = UtilDateTime.nowTimestamp();
         Locale locale = (Locale) context.get("locale");
         String errMsg = null;
+        EntityCondition conditionOne = EntityCondition.makeCondition(UtilMisc.toList(
+                EntityCondition.makeCondition("isVariant", EntityOperator.EQUALS, "Y"),
+                EntityCondition.makeCondition("salesDiscontinuationDate", EntityOperator.NOT_EQUAL, null),
+                EntityCondition.makeCondition("salesDiscontinuationDate", EntityOperator.LESS_THAN_EQUAL_TO, nowTimestamp)
+               ), EntityOperator.AND);
 
-        try {
-            EntityCondition conditionOne = EntityCondition.makeCondition(UtilMisc.toList(
-                    EntityCondition.makeCondition("isVariant", EntityOperator.EQUALS, "Y"),
-                    EntityCondition.makeCondition("salesDiscontinuationDate", EntityOperator.NOT_EQUAL, null),
-                    EntityCondition.makeCondition("salesDiscontinuationDate", EntityOperator.LESS_THAN_EQUAL_TO, nowTimestamp)
-                   ), EntityOperator.AND);
-            EntityListIterator eliOne = EntityQuery.use(delegator).from("Product").where(conditionOne).queryIterator();
+        try (EntityListIterator eliOne = EntityQuery.use(delegator).from("Product").where(conditionOne).queryIterator()) {
             GenericValue productOne = null;
             int numSoFarOne = 0;
             while ((productOne = eliOne.next()) != null) {
@@ -100,29 +99,32 @@ public final class ProductUtilServices {
                     }
                 }
             }
-            eliOne.close();
-
             // get all non-discontinued virtuals, see if all variant ProductAssocs are expired, if discontinue
             EntityCondition condition = EntityCondition.makeCondition(UtilMisc.toList(
                     EntityCondition.makeCondition("isVirtual", EntityOperator.EQUALS, "Y"),
                     EntityCondition.makeCondition(EntityCondition.makeCondition("salesDiscontinuationDate", EntityOperator.EQUALS, null), EntityOperator.OR, EntityCondition.makeCondition("salesDiscontinuationDate", EntityOperator.GREATER_THAN_EQUAL_TO, nowTimestamp))
                    ), EntityOperator.AND);
-            EntityListIterator eli = EntityQuery.use(delegator).from("Product").where(condition).queryIterator();
-            GenericValue product = null;
-            int numSoFar = 0;
-            while ((product = eli.next()) != null) {
-                List<GenericValue> passocList = EntityQuery.use(delegator).from("ProductAssoc").where("productId", product.get("productId"), "productAssocTypeId", "PRODUCT_VARIANT").filterByDate().queryList();
-                if (passocList.size() == 0) {
-                    product.set("salesDiscontinuationDate", nowTimestamp);
-                    delegator.store(product);
-
-                    numSoFar++;
-                    if (numSoFar % 500 == 0) {
-                        Debug.logInfo("Sales discontinued " + numSoFar + " virtual products that have no valid variants.", module);
-                    }
-                }
+            try (EntityListIterator eli = EntityQuery.use(delegator).from("Product").where(condition).queryIterator()) {
+                GenericValue product = null;
+                int numSoFar = 0;
+                while ((product = eli.next()) != null) {
+                    List<GenericValue> passocList = EntityQuery.use(delegator).from("ProductAssoc").where("productId", product.get("productId"), "productAssocTypeId", "PRODUCT_VARIANT").filterByDate().queryList();
+                    if (passocList.size() == 0) {
+                        product.set("salesDiscontinuationDate", nowTimestamp);
+                        delegator.store(product);
+
+                        numSoFar++;
+                        if (numSoFar % 500 == 0) {
+                            Debug.logInfo("Sales discontinued " + numSoFar + " virtual products that have no valid variants.", module);
+                        }
+                    }
+                }
+            } catch (GenericEntityException e) {
+                Map<String, String> messageMap = UtilMisc.toMap("errMessage", e.toString());
+                errMsg = UtilProperties.getMessage(resourceError,"productutilservices.entity_error_running_discVirtualsWithDiscVariants", messageMap, locale);
+                Debug.logError(e, errMsg, module);
+                return ServiceUtil.returnError(errMsg);
             }
-            eli.close();
         } catch (GenericEntityException e) {
             Map<String, String> messageMap = UtilMisc.toMap("errMessage", e.toString());
             errMsg = UtilProperties.getMessage(resourceError,"productutilservices.entity_error_running_discVirtualsWithDiscVariants", messageMap, locale);
@@ -139,13 +141,12 @@ public final class ProductUtilServices {
         Timestamp nowTimestamp = UtilDateTime.nowTimestamp();
         Locale locale = (Locale) context.get("locale");
         String errMsg = null;
+        EntityCondition condition = EntityCondition.makeCondition(UtilMisc.toList(
+                EntityCondition.makeCondition("salesDiscontinuationDate", EntityOperator.NOT_EQUAL, null),
+                EntityCondition.makeCondition("salesDiscontinuationDate", EntityOperator.LESS_THAN_EQUAL_TO, nowTimestamp)
+               ), EntityOperator.AND);
 
-        try {
-            EntityCondition condition = EntityCondition.makeCondition(UtilMisc.toList(
-                    EntityCondition.makeCondition("salesDiscontinuationDate", EntityOperator.NOT_EQUAL, null),
-                    EntityCondition.makeCondition("salesDiscontinuationDate", EntityOperator.LESS_THAN_EQUAL_TO, nowTimestamp)
-                   ), EntityOperator.AND);
-            EntityListIterator eli = EntityQuery.use(delegator).from("Product").where(condition).queryIterator();
+        try (EntityListIterator eli = EntityQuery.use(delegator).from("Product").where(condition).queryIterator()) {
             GenericValue product = null;
             int numSoFar = 0;
             while ((product = eli.next()) != null) {
@@ -162,7 +163,6 @@ public final class ProductUtilServices {
                     }
                 }
             }
-            eli.close();
             Debug.logInfo("Completed - Removed category members for " + numSoFar + " sales discontinued products.", module);
         } catch (GenericEntityException e) {
             Map<String, String> messageMap = UtilMisc.toMap("errMessage", e.toString());
@@ -179,22 +179,21 @@ public final class ProductUtilServices {
         Timestamp nowTimestamp = UtilDateTime.nowTimestamp();
         Locale locale = (Locale) context.get("locale");
         String errMsg = null;
+        DynamicViewEntity dve = new DynamicViewEntity();
+        dve.addMemberEntity("PCM", "ProductCategoryMember");
+        dve.addAlias("PCM", "productId", null, null, null, Boolean.TRUE, null);
+        dve.addAlias("PCM", "productCategoryId", null, null, null, Boolean.TRUE, null);
+        dve.addAlias("PCM", "fromDate", null, null, null, null, null);
+        dve.addAlias("PCM", "thruDate", null, null, null, null, null);
+        dve.addAlias("PCM", "productIdCount", "productId", null, null, null, "count");
+
+        EntityCondition condition = EntityCondition.makeCondition(UtilMisc.toList(
+                EntityCondition.makeCondition("fromDate", EntityOperator.LESS_THAN, nowTimestamp),
+                EntityCondition.makeCondition("thruDate", EntityOperator.EQUALS, null)
+               ), EntityOperator.AND);
+        EntityCondition havingCond = EntityCondition.makeCondition("productIdCount", EntityOperator.GREATER_THAN, Long.valueOf(1));
 
-        try {
-            DynamicViewEntity dve = new DynamicViewEntity();
-            dve.addMemberEntity("PCM", "ProductCategoryMember");
-            dve.addAlias("PCM", "productId", null, null, null, Boolean.TRUE, null);
-            dve.addAlias("PCM", "productCategoryId", null, null, null, Boolean.TRUE, null);
-            dve.addAlias("PCM", "fromDate", null, null, null, null, null);
-            dve.addAlias("PCM", "thruDate", null, null, null, null, null);
-            dve.addAlias("PCM", "productIdCount", "productId", null, null, null, "count");
-
-            EntityCondition condition = EntityCondition.makeCondition(UtilMisc.toList(
-                    EntityCondition.makeCondition("fromDate", EntityOperator.LESS_THAN, nowTimestamp),
-                    EntityCondition.makeCondition("thruDate", EntityOperator.EQUALS, null)
-                   ), EntityOperator.AND);
-            EntityCondition havingCond = EntityCondition.makeCondition("productIdCount", EntityOperator.GREATER_THAN, Long.valueOf(1));
-            EntityListIterator eli = EntityQuery.use(delegator).select("productId", "productCategoryId", "productIdCount").from(dve).where(condition).having(havingCond).queryIterator();
+        try (EntityListIterator eli = EntityQuery.use(delegator).select("productId", "productCategoryId", "productIdCount").from(dve).where(condition).having(havingCond).queryIterator()) {
             GenericValue pcm = null;
             int numSoFar = 0;
             while ((pcm = eli.next()) != null) {
@@ -211,7 +210,6 @@ public final class ProductUtilServices {
                     }
                 }
             }
-            eli.close();
             Debug.logInfo("Completed - Removed category members for " + numSoFar + " products with duplicate category members.", module);
         } catch (GenericEntityException e) {
             Map<String, String> messageMap = UtilMisc.toMap("errMessage", e.toString());
@@ -243,19 +241,21 @@ public final class ProductUtilServices {
         dve.addAlias("PVA", "fromDate", null, null, null, null, null);
         dve.addAlias("PVA", "thruDate", null, null, null, null, null);
         dve.addAlias("PVA", "productIdToCount", "productIdTo", null, null, null, "count-distinct");
-
-        try {
-            EntityCondition condition = EntityCondition.makeCondition(UtilMisc.toList(
-                    EntityCondition.makeCondition("productAssocTypeId", EntityOperator.EQUALS, "PRODUCT_VARIANT"),
-                    EntityCondition.makeCondition(EntityCondition.makeCondition("salesDiscontinuationDate", EntityOperator.EQUALS, null), EntityOperator.OR, EntityCondition.makeCondition("salesDiscontinuationDate", EntityOperator.GREATER_THAN, nowTimestamp))
-                   ), EntityOperator.AND);
-            EntityCondition havingCond = EntityCondition.makeCondition("productIdToCount", EntityOperator.EQUALS, Long.valueOf(1));
-            EntityListIterator eliOne = EntityQuery.use(delegator).select("productId", "productIdToCount").from(dve)
-                                            .where(condition)
-                                            .having(havingCond)
-                                            .queryIterator();
+        EntityCondition condition = EntityCondition.makeCondition(UtilMisc.toList(
+                EntityCondition.makeCondition("productAssocTypeId", EntityOperator.EQUALS, "PRODUCT_VARIANT"),
+                EntityCondition.makeCondition(EntityCondition.makeCondition("salesDiscontinuationDate", EntityOperator.EQUALS, null),
+                        EntityOperator.OR,
+                        EntityCondition.makeCondition("salesDiscontinuationDate", EntityOperator.GREATER_THAN, nowTimestamp))
+               ), EntityOperator.AND);
+        EntityCondition havingCond = EntityCondition.makeCondition("productIdToCount", EntityOperator.EQUALS, Long.valueOf(1));
+        EntityQuery eq = EntityQuery.use(delegator)
+                .select("productId", "productIdToCount")
+                .from(dve)
+                .where(condition)
+                .having(havingCond);
+        
+        try (EntityListIterator eliOne = eq.queryIterator()) {
             List<GenericValue> valueList = eliOne.getCompleteList();
-            eliOne.close();
 
             Debug.logInfo("Found " + valueList.size() + " virtual products with one variant to turn into a stand alone product.", module);
 
@@ -270,7 +270,6 @@ public final class ProductUtilServices {
                 } else {
                     // for all virtuals with one variant move all info from virtual to variant and remove virtual, make variant as not a variant
                     dispatcher.runSync("mergeVirtualWithSingleVariant", UtilMisc.<String, Object>toMap("productId", productId, "removeOld", Boolean.TRUE, "userLogin", userLogin));
-
                     numWithOneOnly++;
                     if (numWithOneOnly % 100 == 0) {
                         Debug.logInfo("Made " + numWithOneOnly + " virtual products with only one valid variant stand-alone products.", module);
@@ -284,37 +283,42 @@ public final class ProductUtilServices {
                     EntityCondition.makeCondition("fromDate", EntityOperator.LESS_THAN_EQUAL_TO, nowTimestamp),
                     EntityCondition.makeCondition(EntityCondition.makeCondition("thruDate", EntityOperator.EQUALS, null), EntityOperator.OR, EntityCondition.makeCondition("thruDate", EntityOperator.GREATER_THAN_EQUAL_TO, nowTimestamp))
                    ), EntityOperator.AND);
-            EntityListIterator eliMulti = EntityQuery.use(delegator).select("productId", "productIdToCount").from(dve)
-                                              .where(conditionWithDates)
-                                              .having(havingCond)
-                                              .queryIterator();
-            List<GenericValue> valueMultiList = eliMulti.getCompleteList();
-            eliMulti.close();
-
-            Debug.logInfo("Found " + valueMultiList.size() + " virtual products with one VALID variant to pull the variant from to make a stand alone product.", module);
-
-            int numWithOneValid = 0;
-            for (GenericValue value: valueMultiList) {
-                // has only one valid variant
-                String productId = value.getString("productId");
-
-                List<GenericValue> paList = EntityQuery.use(delegator).from("ProductAssoc").where("productId", productId, "productAssocTypeId", "PRODUCT_VARIANT").filterByDate().queryList();
-
-                // verify the query; tested on a bunch, looks good
-                if (paList.size() != 1) {
-                    Debug.logInfo("Virtual product with ID " + productId + " should have 1 assoc, has " + paList.size(), module);
-                } else {
-                    // for all virtuals with one valid variant move info from virtual to variant, put variant in categories from virtual, remove virtual from all categories but leave "family" otherwise intact, mark variant as not a variant
-                    dispatcher.runSync("mergeVirtualWithSingleVariant", UtilMisc.<String, Object>toMap("productId", productId, "removeOld", Boolean.FALSE, "userLogin", userLogin));
+            eq = EntityQuery.use(delegator).
+                    select("productId", "productIdToCount").
+                    from(dve)
+                    .where(conditionWithDates)
+                    .having(havingCond);
+            try (EntityListIterator eliMulti = eq.queryIterator()) {
+                List<GenericValue> valueMultiList = eliMulti.getCompleteList();
+                Debug.logInfo("Found " + valueMultiList.size() + " virtual products with one VALID variant to pull the variant from to make a stand alone product.", module);
+
+                int numWithOneValid = 0;
+                for (GenericValue value: valueMultiList) {
+                    // has only one valid variant
+                    String productId = value.getString("productId");
+
+                    List<GenericValue> paList = EntityQuery.use(delegator).from("ProductAssoc").where("productId", productId, "productAssocTypeId", "PRODUCT_VARIANT").filterByDate().queryList();
+
+                    // verify the query; tested on a bunch, looks good
+                    if (paList.size() != 1) {
+                        Debug.logInfo("Virtual product with ID " + productId + " should have 1 assoc, has " + paList.size(), module);
+                    } else {
+                        // for all virtuals with one valid variant move info from virtual to variant, put variant in categories from virtual, remove virtual from all categories but leave "family" otherwise intact, mark variant as not a variant
+                        dispatcher.runSync("mergeVirtualWithSingleVariant", UtilMisc.<String, Object>toMap("productId", productId, "removeOld", Boolean.FALSE, "userLogin", userLogin));
 
-                    numWithOneValid++;
-                    if (numWithOneValid % 100 == 0) {
-                        Debug.logInfo("Made " + numWithOneValid + " virtual products with one valid variant stand-alone products.", module);
+                        numWithOneValid++;
+                        if (numWithOneValid % 100 == 0) {
+                            Debug.logInfo("Made " + numWithOneValid + " virtual products with one valid variant stand-alone products.", module);
+                        }
                     }
                 }
-            }
-
             Debug.logInfo("Found virtual products with one valid variant: " + numWithOneValid + ", with one variant only: " + numWithOneOnly, module);
+            } catch (GenericEntityException e) {
+                Map<String, String> messageMap = UtilMisc.toMap("errMessage", e.toString());
+                errMsg = UtilProperties.getMessage(resourceError,"productutilservices.entity_error_running_makeStandAloneFromSingleVariantVirtuals", messageMap, locale);
+                Debug.logError(e, errMsg, module);
+                return ServiceUtil.returnError(errMsg);
+            }
         } catch (GenericEntityException e) {
             Map<String, String> messageMap = UtilMisc.toMap("errMessage", e.toString());
             errMsg = UtilProperties.getMessage(resourceError,"productutilservices.entity_error_running_makeStandAloneFromSingleVariantVirtuals", messageMap, locale);
@@ -504,8 +508,7 @@ public final class ProductUtilServices {
             pattern = imageUrlPrefix + "/" + imageFilenameFormat;
         }
 
-        try {
-            EntityListIterator eli = EntityQuery.use(delegator).from("Product").queryIterator();
+        try (EntityListIterator eli = EntityQuery.use(delegator).from("Product").queryIterator()) {
             GenericValue product = null;
             int numSoFar = 0;
             while ((product = eli.next()) != null) {
@@ -543,7 +546,6 @@ public final class ProductUtilServices {
                     Debug.logInfo("Image URLs set for " + numSoFar + " products.", module);
                 }
             }
-            eli.close();
             Debug.logInfo("Completed - Image URLs set for " + numSoFar + " products.", module);
         } catch (GenericEntityException e) {
             Map<String, String> messageMap = UtilMisc.toMap("errMessage", e.toString());
@@ -560,8 +562,7 @@ public final class ProductUtilServices {
         Locale locale = (Locale) context.get("locale");
         String errMsg = null;
 
-        try {
-            EntityListIterator eli = EntityQuery.use(delegator).from("Product").where("isVirtual", "Y").queryIterator();
+        try (EntityListIterator eli = EntityQuery.use(delegator).from("Product").where("isVirtual", "Y").queryIterator()) {
             GenericValue product = null;
             int numSoFar = 0;
             while ((product = eli.next()) != null) {
@@ -575,7 +576,6 @@ public final class ProductUtilServices {
                     Debug.logInfo("Image URLs cleared for " + numSoFar + " products.", module);
                 }
             }
-            eli.close();
             Debug.logInfo("Completed - Image URLs set for " + numSoFar + " products.", module);
         } catch (GenericEntityException e) {
             Map<String, String> messageMap = UtilMisc.toMap("errMessage", e.toString());
@@ -630,7 +630,8 @@ public final class ProductUtilServices {
     /** Get all features associated with products and associate them with a feature group attached to the category for each feature type;
      * includes products associated with this category only, but will also associate all feature groups of sub-categories with this category, optionally calls this method for all sub-categories too
      */
-    public static void attachProductFeaturesToCategory(String productCategoryId, Set<String> productFeatureTypeIdsToInclude, Set<String> productFeatureTypeIdsToExclude, Delegator delegator, boolean doSubCategories, Timestamp nowTimestamp) throws GenericEntityException {
+    public static void attachProductFeaturesToCategory(String productCategoryId, Set<String> productFeatureTypeIdsToInclude, Set<String> productFeatureTypeIdsToExclude,
+            Delegator delegator, boolean doSubCategories, Timestamp nowTimestamp) throws GenericEntityException {
         if (nowTimestamp == null) {
             nowTimestamp = UtilDateTime.nowTimestamp();
         }
@@ -639,7 +640,8 @@ public final class ProductUtilServices {
         List<GenericValue> subCategoryList = EntityQuery.use(delegator).from("ProductCategoryRollup").where("parentProductCategoryId", productCategoryId).queryList();
         if (doSubCategories) {
             for (GenericValue productCategoryRollup: subCategoryList) {
-                attachProductFeaturesToCategory(productCategoryRollup.getString("productCategoryId"), productFeatureTypeIdsToInclude, productFeatureTypeIdsToExclude, delegator, true, nowTimestamp);
+                attachProductFeaturesToCategory(productCategoryRollup.getString("productCategoryId"), productFeatureTypeIdsToInclude, productFeatureTypeIdsToExclude,
+                        delegator, true, nowTimestamp);
             }
         }
 
@@ -651,91 +653,93 @@ public final class ProductUtilServices {
             EntityCondition condition = EntityCondition.makeCondition(UtilMisc.toList(
                     EntityCondition.makeCondition("productId", EntityOperator.EQUALS, productId),
                     EntityCondition.makeCondition("fromDate", EntityOperator.LESS_THAN_EQUAL_TO, nowTimestamp),
-                    EntityCondition.makeCondition(EntityCondition.makeCondition("thruDate", EntityOperator.EQUALS, null), EntityOperator.OR, EntityCondition.makeCondition("thruDate", EntityOperator.GREATER_THAN_EQUAL_TO, nowTimestamp))
-           ), EntityOperator.AND);
-            EntityListIterator productFeatureAndApplEli = EntityQuery.use(delegator).from("ProductFeatureAndAppl").where(condition).queryIterator();
-            GenericValue productFeatureAndAppl = null;
-            while ((productFeatureAndAppl = productFeatureAndApplEli.next()) != null) {
-                String productFeatureId = productFeatureAndAppl.getString("productFeatureId");
-                String productFeatureTypeId = productFeatureAndAppl.getString("productFeatureTypeId");
-                if (UtilValidate.isNotEmpty(productFeatureTypeIdsToInclude) && !productFeatureTypeIdsToInclude.contains(productFeatureTypeId)) {
-                    continue;
-                }
-                if (productFeatureTypeIdsToExclude != null && productFeatureTypeIdsToExclude.contains(productFeatureTypeId)) {
-                    continue;
-                }
-                Set<String> productFeatureIdSet = productFeatureIdByTypeIdSetMap.get(productFeatureTypeId);
-                if (productFeatureIdSet == null) {
-                    productFeatureIdSet = new HashSet<String>();
-                    productFeatureIdByTypeIdSetMap.put(productFeatureTypeId, productFeatureIdSet);
-                }
-                productFeatureIdSet.add(productFeatureId);
-            }
-            productFeatureAndApplEli.close();
-        }
-
-        for (Map.Entry<String, Set<String>> entry: productFeatureIdByTypeIdSetMap.entrySet()) {
-            String productFeatureTypeId = entry.getKey();
-            Set<String> productFeatureIdSet = entry.getValue();
-
-            String productFeatureGroupId = productCategoryId + "_" + productFeatureTypeId;
-            if (productFeatureGroupId.length() > 20) {
-                Debug.logWarning("Manufactured productFeatureGroupId was greater than 20 characters, means that we had some long productCategoryId and/or productFeatureTypeId values, at the category part should be unique since it is first, so if the feature type isn't unique it just means more than one type of feature will go into the category...", module);
-                productFeatureGroupId = productFeatureGroupId.substring(0, 20);
-            }
-
-            GenericValue productFeatureGroup = EntityQuery.use(delegator).from("ProductFeatureGroup").where("productFeatureGroupId", productFeatureGroupId).queryOne();
-            if (productFeatureGroup == null) {
-                // auto-create the group
-                String description = "Feature Group for type [" + productFeatureTypeId + "] features in category [" + productCategoryId + "]";
-                productFeatureGroup = delegator.makeValue("ProductFeatureGroup", UtilMisc.toMap("productFeatureGroupId", productFeatureGroupId, "description", description));
-                productFeatureGroup.create();
-
-                GenericValue productFeatureCatGrpAppl = delegator.makeValue("ProductFeatureCatGrpAppl", UtilMisc.toMap("productFeatureGroupId", productFeatureGroupId, "productCategoryId", productCategoryId, "fromDate", nowTimestamp));
-                productFeatureCatGrpAppl.create();
-            }
-
-            // now put all of the features in the group, if there is not already a valid feature placement there...
-            for (String productFeatureId: productFeatureIdSet) {
-                EntityCondition condition = EntityCondition.makeCondition(UtilMisc.toList(
-                        EntityCondition.makeCondition("productFeatureId", EntityOperator.EQUALS, productFeatureId),
-                        EntityCondition.makeCondition("productFeatureGroupId", EntityOperator.EQUALS, productFeatureGroupId),
-                        EntityCondition.makeCondition("fromDate", EntityOperator.LESS_THAN_EQUAL_TO, nowTimestamp),
-                        EntityCondition.makeCondition(EntityCondition.makeCondition("thruDate", EntityOperator.EQUALS, null), EntityOperator.OR, EntityCondition.makeCondition("thruDate", EntityOperator.GREATER_THAN_EQUAL_TO, nowTimestamp))
-               ), EntityOperator.AND);
-                if (EntityQuery.use(delegator).from("ProductFeatureGroupAppl").where(condition).queryCount() == 0) {
-                    // if no valid ones, create one
-                    GenericValue productFeatureGroupAppl = delegator.makeValue("ProductFeatureGroupAppl", UtilMisc.toMap("productFeatureGroupId", productFeatureGroupId, "productFeatureId", productFeatureId, "fromDate", nowTimestamp));
-                    productFeatureGroupAppl.create();
-                }
-            }
-        }
-
-        // now get all feature groups associated with sub-categories and associate them with this category
-        for (GenericValue productCategoryRollup: subCategoryList) {
-            String subProductCategoryId = productCategoryRollup.getString("productCategoryId");
-            EntityCondition condition = EntityCondition.makeCondition(UtilMisc.toList(
-                    EntityCondition.makeCondition("productCategoryId", EntityOperator.EQUALS, subProductCategoryId),
-                    EntityCondition.makeCondition("fromDate", EntityOperator.LESS_THAN_EQUAL_TO, nowTimestamp),
-                    EntityCondition.makeCondition(EntityCondition.makeCondition("thruDate", EntityOperator.EQUALS, null), EntityOperator.OR, EntityCondition.makeCondition("thruDate", EntityOperator.GREATER_THAN_EQUAL_TO, nowTimestamp))
-           ), EntityOperator.AND);
-            EntityListIterator productFeatureCatGrpApplEli = EntityQuery.use(delegator).from("ProductFeatureCatGrpAppl").where(condition).queryIterator();
-            GenericValue productFeatureCatGrpAppl = null;
-            while ((productFeatureCatGrpAppl = productFeatureCatGrpApplEli.next()) != null) {
-                String productFeatureGroupId = productFeatureCatGrpAppl.getString("productFeatureGroupId");
-                EntityCondition checkCondition = EntityCondition.makeCondition(UtilMisc.toList(
-                        EntityCondition.makeCondition("productCategoryId", EntityOperator.EQUALS, productCategoryId),
-                        EntityCondition.makeCondition("productFeatureGroupId", EntityOperator.EQUALS, productFeatureGroupId),
-                        EntityCondition.makeCondition("fromDate", EntityOperator.LESS_THAN_EQUAL_TO, nowTimestamp),
-                        EntityCondition.makeCondition(EntityCondition.makeCondition("thruDate", EntityOperator.EQUALS, null), EntityOperator.OR, EntityCondition.makeCondition("thruDate", EntityOperator.GREATER_THAN_EQUAL_TO, nowTimestamp))
-               ), EntityOperator.AND);
-                if (EntityQuery.use(delegator).from("ProductFeatureCatGrpAppl").where(checkCondition).queryCount() == 0) {
-                    // if no valid ones, create one
-                    GenericValue productFeatureGroupAppl = delegator.makeValue("ProductFeatureCatGrpAppl", UtilMisc.toMap("productFeatureGroupId", productFeatureGroupId, "productCategoryId", productCategoryId, "fromDate", nowTimestamp));
-                    productFeatureGroupAppl.create();
+                    EntityCondition.makeCondition(EntityCondition.makeCondition("thruDate", EntityOperator.EQUALS, null),
+                            EntityOperator.OR, EntityCondition.makeCondition("thruDate", EntityOperator.GREATER_THAN_EQUAL_TO, nowTimestamp))
+                    ), EntityOperator.AND);
+
+            try (EntityListIterator productFeatureAndApplEli = EntityQuery.use(delegator).from("ProductFeatureAndAppl").where(condition).queryIterator()) {
+                GenericValue productFeatureAndAppl = null;
+                while ((productFeatureAndAppl = productFeatureAndApplEli.next()) != null) {
+                    String productFeatureId = productFeatureAndAppl.getString("productFeatureId");
+                    String productFeatureTypeId = productFeatureAndAppl.getString("productFeatureTypeId");
+                    if (UtilValidate.isNotEmpty(productFeatureTypeIdsToInclude) && !productFeatureTypeIdsToInclude.contains(productFeatureTypeId)) {
+                        continue;
+                    }
+                    if (productFeatureTypeIdsToExclude != null && productFeatureTypeIdsToExclude.contains(productFeatureTypeId)) {
+                        continue;
+                    }
+                    Set<String> productFeatureIdSet = productFeatureIdByTypeIdSetMap.get(productFeatureTypeId);
+                    if (productFeatureIdSet == null) {
+                        productFeatureIdSet = new HashSet<String>();
+                        productFeatureIdByTypeIdSetMap.put(productFeatureTypeId, productFeatureIdSet);
+                    }
+                    productFeatureIdSet.add(productFeatureId);
+                }
+
+                for (Map.Entry<String, Set<String>> entry: productFeatureIdByTypeIdSetMap.entrySet()) {
+                    String productFeatureTypeId = entry.getKey();
+                    Set<String> productFeatureIdSet = entry.getValue();
+
+                    String productFeatureGroupId = productCategoryId + "_" + productFeatureTypeId;
+                    if (productFeatureGroupId.length() > 20) {
+                        Debug.logWarning("Manufactured productFeatureGroupId was greater than 20 characters, means that we had some long productCategoryId and/or productFeatureTypeId values, at the category part should be unique since it is first, so if the feature type isn't unique it just means more than one type of feature will go into the category...", module);
+                        productFeatureGroupId = productFeatureGroupId.substring(0, 20);
+                    }
+
+                    GenericValue productFeatureGroup = EntityQuery.use(delegator).from("ProductFeatureGroup").where("productFeatureGroupId", productFeatureGroupId).queryOne();
+                    if (productFeatureGroup == null) {
+                        // auto-create the group
+                        String description = "Feature Group for type [" + productFeatureTypeId + "] features in category [" + productCategoryId + "]";
+                        productFeatureGroup = delegator.makeValue("ProductFeatureGroup", UtilMisc.toMap("productFeatureGroupId", productFeatureGroupId, "description", description));
+                        productFeatureGroup.create();
+
+                        GenericValue productFeatureCatGrpAppl = delegator.makeValue("ProductFeatureCatGrpAppl", UtilMisc.toMap("productFeatureGroupId", productFeatureGroupId, "productCategoryId", productCategoryId, "fromDate", nowTimestamp));
+                        productFeatureCatGrpAppl.create();
+                    }
+
+                    // now put all of the features in the group, if there is not already a valid feature placement there...
+                    for (String productFeatureId: productFeatureIdSet) {
+                        condition = EntityCondition.makeCondition(UtilMisc.toList(
+                                EntityCondition.makeCondition("productFeatureId", EntityOperator.EQUALS, productFeatureId),
+                                EntityCondition.makeCondition("productFeatureGroupId", EntityOperator.EQUALS, productFeatureGroupId),
+                                EntityCondition.makeCondition("fromDate", EntityOperator.LESS_THAN_EQUAL_TO, nowTimestamp),
+                                EntityCondition.makeCondition(EntityCondition.makeCondition("thruDate", EntityOperator.EQUALS, null), EntityOperator.OR, EntityCondition.makeCondition("thruDate", EntityOperator.GREATER_THAN_EQUAL_TO, nowTimestamp))
+                                ), EntityOperator.AND);
+                        if (EntityQuery.use(delegator).from("ProductFeatureGroupAppl").where(condition).queryCount() == 0) {
+                            // if no valid ones, create one
+                            GenericValue productFeatureGroupAppl = delegator.makeValue("ProductFeatureGroupAppl", UtilMisc.toMap("productFeatureGroupId", productFeatureGroupId, "productFeatureId", productFeatureId, "fromDate", nowTimestamp));
+                            productFeatureGroupAppl.create();
+                        }
+                    }
+                }
+
+                // now get all feature groups associated with sub-categories and associate them with this category
+                for (GenericValue productCategoryRollup: subCategoryList) {
+                    String subProductCategoryId = productCategoryRollup.getString("productCategoryId");
+                    condition = EntityCondition.makeCondition(UtilMisc.toList(
+                            EntityCondition.makeCondition("productCategoryId", EntityOperator.EQUALS, subProductCategoryId),
+                            EntityCondition.makeCondition("fromDate", EntityOperator.LESS_THAN_EQUAL_TO, nowTimestamp),
+                            EntityCondition.makeCondition(EntityCondition.makeCondition("thruDate", EntityOperator.EQUALS, null), EntityOperator.OR, EntityCondition.makeCondition("thruDate", EntityOperator.GREATER_THAN_EQUAL_TO, nowTimestamp))
+                            ), EntityOperator.AND);
+                    try (EntityListIterator productFeatureCatGrpApplEli = EntityQuery.use(delegator).from("ProductFeatureCatGrpAppl").where(condition).queryIterator()) {
+                        GenericValue productFeatureCatGrpAppl = null;
+                        while ((productFeatureCatGrpAppl = productFeatureCatGrpApplEli.next()) != null) {
+                            String productFeatureGroupId = productFeatureCatGrpAppl.getString("productFeatureGroupId");
+                            EntityCondition checkCondition = EntityCondition.makeCondition(UtilMisc.toList(
+                                    EntityCondition.makeCondition("productCategoryId", EntityOperator.EQUALS, productCategoryId),
+                                    EntityCondition.makeCondition("productFeatureGroupId", EntityOperator.EQUALS, productFeatureGroupId),
+                                    EntityCondition.makeCondition("fromDate", EntityOperator.LESS_THAN_EQUAL_TO, nowTimestamp),
+                                    EntityCondition.makeCondition(EntityCondition.makeCondition("thruDate", EntityOperator.EQUALS, null), EntityOperator.OR, EntityCondition.makeCondition("thruDate", EntityOperator.GREATER_THAN_EQUAL_TO, nowTimestamp))
+                                    ), EntityOperator.AND);
+                            if (EntityQuery.use(delegator).from("ProductFeatureCatGrpAppl").where(checkCondition).queryCount() == 0) {
+                                // if no valid ones, create one
+                                GenericValue productFeatureGroupAppl = delegator.makeValue("ProductFeatureCatGrpAppl", UtilMisc.toMap("productFeatureGroupId", productFeatureGroupId, "productCategoryId", productCategoryId, "fromDate", nowTimestamp));
+                                productFeatureGroupAppl.create();
+                            }
+                        }
+                    }
                 }
             }
-            productFeatureCatGrpApplEli.close();
         }
     }
 

Modified: ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/promo/PromoServices.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/promo/PromoServices.java?rev=1797158&r1=1797157&r2=1797158&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/promo/PromoServices.java (original)
+++ ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/promo/PromoServices.java Thu Jun  1 08:10:51 2017
@@ -138,15 +138,13 @@ public class PromoServices {
         condList.add(EntityCondition.makeCondition("thruDate", EntityOperator.NOT_EQUAL, null));
         condList.add(EntityCondition.makeCondition("thruDate", EntityOperator.LESS_THAN, nowTimestamp));
 
-        try {
-            EntityListIterator eli = EntityQuery.use(delegator).from("ProductStorePromoAndAppl").where(condList).queryIterator();
+        try (EntityListIterator eli = EntityQuery.use(delegator).from("ProductStorePromoAndAppl").where(condList).queryIterator()) {
             GenericValue productStorePromoAndAppl = null;
             while ((productStorePromoAndAppl = eli.next()) != null) {
                 GenericValue productStorePromo = delegator.makeValue("ProductStorePromoAppl");
                 productStorePromo.setAllFields(productStorePromoAndAppl, true, null, null);
                 productStorePromo.remove();
             }
-            eli.close();
         } catch (GenericEntityException e) {
             Debug.logError(e, "Error removing expired ProductStorePromo records: " + e.toString(), module);
             return ServiceUtil.returnError(UtilProperties.getMessage(resource,