svn commit: r1798566 - in /ofbiz/ofbiz-framework/trunk: applications/party/src/main/ja
Locked
svn commit: r1798566 - in /ofbiz/ofbiz-framework/trunk: applications/party/src/main/ja
 
|
Author: jleroux
Date: Tue Jun 13 08:29:28 2017 New Revision: 1798566 URL: http://svn.apache.org/viewvc?rev=1798566&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 when possible This completes OFBIZ-9385, there are no other cases left. There are few methods like FindServices.executeFind() or performFindParty service which can't be improved because they return an EntityListIterator. So methods using them must be carefully crafted to close the EntityListIterator. Most where already documented (in the framework). I completed few in applications at r1798495 Modified: ofbiz/ofbiz-framework/trunk/applications/party/src/main/java/org/apache/ofbiz/party/party/PartyServices.java ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/feature/ParametricSearch.java ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/product/ProductSearch.java ofbiz/ofbiz-framework/trunk/framework/entityext/src/main/java/org/apache/ofbiz/entityext/synchronization/EntitySyncContext.java ofbiz/ofbiz-framework/trunk/framework/widget/src/main/java/org/apache/ofbiz/widget/model/ModelTree.java Modified: ofbiz/ofbiz-framework/trunk/applications/party/src/main/java/org/apache/ofbiz/party/party/PartyServices.java URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/party/src/main/java/org/apache/ofbiz/party/party/PartyServices.java?rev=1798566&r1=1798565&r2=1798566&view=diff ============================================================================== --- ofbiz/ofbiz-framework/trunk/applications/party/src/main/java/org/apache/ofbiz/party/party/PartyServices.java (original) +++ ofbiz/ofbiz-framework/trunk/applications/party/src/main/java/org/apache/ofbiz/party/party/PartyServices.java Tue Jun 13 08:29:28 2017 @@ -954,7 +954,7 @@ public class PartyServices { Locale locale = (Locale) context.get("locale"); try { - parties = EntityQuery.use(delegator).from("Party") + parties = EntityQuery.use(delegator).from("Party") .where(EntityCondition.makeCondition(EntityFunction.UPPER_FIELD("externalId"), EntityOperator.EQUALS, EntityFunction.UPPER(externalId))) .orderBy("externalId", "partyId") .queryList(); @@ -1502,21 +1502,19 @@ public class PartyServices { // do the lookup if (mainCond != null || "Y".equals(showAll)) { - try { - // get the indexes for the partial list - lowIndex = viewIndex * viewSize + 1; - highIndex = (viewIndex + 1) * viewSize; - - // set distinct on so we only get one row per order - // using list iterator - EntityListIterator pli = EntityQuery.use(delegator).select(UtilMisc.toSet(fieldsToSelect)) - .from(dynamicView) - .where(mainCond) - .orderBy(orderBy) - .cursorScrollInsensitive() - .fetchSize(highIndex) - .distinct() - .queryIterator(); + lowIndex = viewIndex * viewSize + 1; + highIndex = (viewIndex + 1) * viewSize; + + // set distinct on so we only get one row per order + // using list iterator + EntityQuery eq = EntityQuery.use(delegator).select(UtilMisc.toSet(fieldsToSelect)) + .from(dynamicView) + .where(mainCond) + .orderBy(orderBy) + .cursorScrollInsensitive() + .fetchSize(highIndex) + .distinct(); + try (EntityListIterator pli = eq.queryIterator()) { // get the partial list for this page partyList = pli.getPartialList(lowIndex, viewSize); @@ -1527,8 +1525,6 @@ public class PartyServices { highIndex = partyListSize; } - // close the list iterator - pli.close(); } catch (GenericEntityException e) { String errMsg = "Failure in party find operation, rolling back transaction: " + e.toString(); Debug.logError(e, errMsg, module); Modified: ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/feature/ParametricSearch.java URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/feature/ParametricSearch.java?rev=1798566&r1=1798565&r2=1798566&view=diff ============================================================================== --- ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/feature/ParametricSearch.java (original) +++ ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/feature/ParametricSearch.java Tue Jun 13 08:29:28 2017 @@ -123,9 +123,8 @@ public class ParametricSearch { } public static Map<String, List<GenericValue>> getAllFeaturesByType(Delegator delegator, int perTypeMaxSize) { Map<String, List<GenericValue>> productFeaturesByTypeMap = new HashMap<String, List<GenericValue>>(); - try { - Set<String> typesWithOverflowMessages = new HashSet<String>(); - EntityListIterator productFeatureEli = EntityQuery.use(delegator).from("ProductFeature").orderBy("description").queryIterator(); + Set<String> typesWithOverflowMessages = new HashSet<String>(); + try (EntityListIterator productFeatureEli = EntityQuery.use(delegator).from("ProductFeature").orderBy("description").queryIterator()) { GenericValue productFeature = null; while ((productFeature = productFeatureEli.next()) != null) { String productFeatureTypeId = productFeature.getString("productFeatureTypeId"); @@ -143,7 +142,6 @@ public class ParametricSearch { featuresByType.add(productFeature); } } - productFeatureEli.close(); } catch (GenericEntityException e) { Debug.logError(e, "Error getting all features", module); } Modified: ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/product/ProductSearch.java URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/product/ProductSearch.java?rev=1798566&r1=1798565&r2=1798566&view=diff ============================================================================== --- ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/product/ProductSearch.java (original) +++ ofbiz/ofbiz-framework/trunk/applications/product/src/main/java/org/apache/ofbiz/product/product/ProductSearch.java Tue Jun 13 08:29:28 2017 @@ -222,14 +222,12 @@ public class ProductSearch { long startMillis = System.currentTimeMillis(); // do the query - EntityListIterator eli = this.doQuery(delegator); - ArrayList<String> productIds = this.makeProductIdList(eli); - if (eli != null) { - try { - eli.close(); - } catch (GenericEntityException e) { - Debug.logError(e, "Error closing ProductSearch EntityListIterator"); - } + ArrayList<String> productIds = null; + try (EntityListIterator eli = this.doQuery(delegator)) { + productIds = this.makeProductIdList(eli); + } catch (GenericEntityException e) { + Debug.logError(e, module); + return null; } long endMillis = System.currentTimeMillis(); @@ -649,6 +647,12 @@ public class ProductSearch { if (Debug.infoOn()) Debug.logInfo("topCond=" + topCond.makeWhereString(null, new LinkedList<EntityConditionParam>(), EntityConfig.getDatasource(delegator.getEntityHelperName("Product"))), module); } + /** + * @param delegator the delegator + * @return EntityListIterator representing the result of the query: NOTE THAT THIS MUST BE CLOSED WHEN YOU ARE + * DONE WITH IT (preferably in a finally block), + * AND DON'T LEAVE IT OPEN TOO LONG BECAUSE IT WILL MAINTAIN A DATABASE CONNECTION. + */ public EntityListIterator doQuery(Delegator delegator) { // handle the now assembled or and and keyword fixed lists this.finishKeywordConstraints(); Modified: ofbiz/ofbiz-framework/trunk/framework/entityext/src/main/java/org/apache/ofbiz/entityext/synchronization/EntitySyncContext.java URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/entityext/src/main/java/org/apache/ofbiz/entityext/synchronization/EntitySyncContext.java?rev=1798566&r1=1798565&r2=1798566&view=diff ============================================================================== --- ofbiz/ofbiz-framework/trunk/framework/entityext/src/main/java/org/apache/ofbiz/entityext/synchronization/EntitySyncContext.java (original) +++ ofbiz/ofbiz-framework/trunk/framework/entityext/src/main/java/org/apache/ofbiz/entityext/synchronization/EntitySyncContext.java Tue Jun 13 08:29:28 2017 @@ -345,35 +345,39 @@ public class EntitySyncContext { } try { - // get the values created within the current time range EntityCondition findValCondition = EntityCondition.makeCondition( EntityCondition.makeCondition(ModelEntity.CREATE_STAMP_TX_FIELD, EntityOperator.GREATER_THAN_EQUAL_TO, currentRunStartTime), EntityCondition.makeCondition(ModelEntity.CREATE_STAMP_TX_FIELD, EntityOperator.LESS_THAN, currentRunEndTime)); - EntityListIterator eli = EntityQuery.use(delegator) - .from(modelEntity.getEntityName()) - .where(findValCondition) - .orderBy(ModelEntity.CREATE_STAMP_TX_FIELD, ModelEntity.CREATE_STAMP_FIELD) - .queryIterator(); - GenericValue nextValue = null; + EntityQuery eq = EntityQuery.use(delegator) + .from(modelEntity.getEntityName()) + .where(findValCondition) + .orderBy(ModelEntity.CREATE_STAMP_TX_FIELD, ModelEntity.CREATE_STAMP_FIELD); + long valuesPerEntity = 0; - while ((nextValue = eli.next()) != null) { - // sort by the tx stamp and then the record stamp - // find first value in valuesToCreate list, starting with the current insertBefore value, that has a CREATE_STAMP_TX_FIELD after the nextValue.CREATE_STAMP_TX_FIELD, then do the same with CREATE_STAMP_FIELD - while (insertBefore < valuesToCreate.size() && valuesToCreate.get(insertBefore).getTimestamp(ModelEntity.CREATE_STAMP_TX_FIELD).before(nextValue.getTimestamp(ModelEntity.CREATE_STAMP_TX_FIELD))) { - insertBefore++; + try (EntityListIterator eli = eq.queryIterator()) { + // get the values created within the current time range + GenericValue nextValue = null; + + while ((nextValue = eli.next()) != null) { + // sort by the tx stamp and then the record stamp + // find first value in valuesToCreate list, starting with the current insertBefore value, that has a CREATE_STAMP_TX_FIELD after the nextValue.CREATE_STAMP_TX_FIELD, then do the same with CREATE_STAMP_FIELD + while (insertBefore < valuesToCreate.size() && valuesToCreate.get(insertBefore).getTimestamp(ModelEntity.CREATE_STAMP_TX_FIELD).before(nextValue.getTimestamp(ModelEntity.CREATE_STAMP_TX_FIELD))) { + insertBefore++; + } + while (insertBefore < valuesToCreate.size() && valuesToCreate.get(insertBefore).getTimestamp(ModelEntity.CREATE_STAMP_FIELD).before(nextValue.getTimestamp(ModelEntity.CREATE_STAMP_FIELD))) { + insertBefore++; + } + valuesToCreate.add(insertBefore, nextValue); + valuesPerEntity++; } - while (insertBefore < valuesToCreate.size() && valuesToCreate.get(insertBefore).getTimestamp(ModelEntity.CREATE_STAMP_FIELD).before(nextValue.getTimestamp(ModelEntity.CREATE_STAMP_FIELD))) { - insertBefore++; + } catch (GenericEntityException e) { + try { + TransactionUtil.rollback(beganTransaction, "Entity Engine error in assembleValuesToCreate", e); + } catch (GenericTransactionException e2) { + Debug.logWarning(e2, "Unable to call rollback()", module); } - valuesToCreate.add(insertBefore, nextValue); - valuesPerEntity++; + throw new SyncDataErrorException("Error getting values to create from the datasource", e); } - eli.close(); - - // definately remove this message and related data gathering - //long preCount = delegator.findCountByCondition(modelEntity.getEntityName(), findValCondition, null); - //long entityTotalCount = delegator.findCountByCondition(modelEntity.getEntityName(), null, null); - //if (entityTotalCount > 0 || preCount > 0 || valuesPerEntity > 0) Debug.logInfo("Got " + valuesPerEntity + "/" + preCount + "/" + entityTotalCount + " values for entity " + modelEntity.getEntityName(), module); // if we didn't find anything for this entity, find the next value's Timestamp and keep track of it if (valuesPerEntity == 0) { @@ -382,14 +386,24 @@ public class EntitySyncContext { EntityCondition findNextCondition = EntityCondition.makeCondition( EntityCondition.makeCondition(ModelEntity.CREATE_STAMP_TX_FIELD, EntityOperator.NOT_EQUAL, null), EntityCondition.makeCondition(ModelEntity.CREATE_STAMP_TX_FIELD, EntityOperator.GREATER_THAN_EQUAL_TO, currentRunEndTime)); - EntityListIterator eliNext = EntityQuery.use(delegator) - .from(modelEntity.getEntityName()) - .where(findNextCondition) - .orderBy(ModelEntity.CREATE_STAMP_TX_FIELD) - .queryIterator(); - // get the first element and it's tx time value... - GenericValue firstVal = eliNext.next(); - eliNext.close(); + eq = EntityQuery.use(delegator) + .from(modelEntity.getEntityName()) + .where(findNextCondition) + .orderBy(ModelEntity.CREATE_STAMP_TX_FIELD); + + GenericValue firstVal = null; + try (EntityListIterator eliNext = eq.queryIterator()) { + // get the first element and it's tx time value... + firstVal = eliNext.next(); + } catch (GenericEntityException e) { + try { + TransactionUtil.rollback(beganTransaction, "Entity Engine error in assembleValuesToCreate", e); + } catch (GenericTransactionException e2) { + Debug.logWarning(e2, "Unable to call rollback()", module); + } + throw new SyncDataErrorException("Error getting values to create from the datasource", e); + } + Timestamp nextTxTime; if (firstVal != null) { nextTxTime = firstVal.getTimestamp(ModelEntity.CREATE_STAMP_TX_FIELD); @@ -401,20 +415,13 @@ public class EntitySyncContext { this.nextCreateTxTime = nextTxTime; Debug.logInfo("EntitySync: Set nextCreateTxTime to [" + nextTxTime + "]", module); } + Timestamp curEntityNextTxTime = this.nextEntityCreateTxTime.get(modelEntity.getEntityName()); if (curEntityNextTxTime == null || nextTxTime.before(curEntityNextTxTime)) { this.nextEntityCreateTxTime.put(modelEntity.getEntityName(), nextTxTime); Debug.logInfo("EntitySync: Set nextEntityCreateTxTime to [" + nextTxTime + "] for the entity [" + modelEntity.getEntityName() + "]", module); } } - } catch (GenericEntityException e) { - try { - TransactionUtil.rollback(beganTransaction, "Entity Engine error in assembleValuesToCreate", e); - - } catch (GenericTransactionException e2) { - Debug.logWarning(e2, "Unable to call rollback()", module); - } - throw new SyncDataErrorException("Error getting values to create from the datasource", e); } catch (Throwable t) { try { TransactionUtil.rollback(beganTransaction, "Throwable error in assembleValuesToCreate", t); @@ -423,7 +430,7 @@ public class EntitySyncContext { } throw new SyncDataErrorException("Caught runtime error while getting values to create", t); } - + try { TransactionUtil.commit(beganTransaction); } catch (GenericTransactionException e) { @@ -449,13 +456,13 @@ public class EntitySyncContext { } Debug.logInfo(toCreateInfo.toString(), module); } - + // As the this.nextCreateTxTime calculation is only based on entities without values to create, if there at least one value to create returned // this calculation is false, so it needs to be nullified if (valuesToCreate.size() > 0) { this.nextCreateTxTime = null; } - + return valuesToCreate; } Modified: ofbiz/ofbiz-framework/trunk/framework/widget/src/main/java/org/apache/ofbiz/widget/model/ModelTree.java URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/widget/src/main/java/org/apache/ofbiz/widget/model/ModelTree.java?rev=1798566&r1=1798565&r2=1798566&view=diff ============================================================================== --- ofbiz/ofbiz-framework/trunk/framework/widget/src/main/java/org/apache/ofbiz/widget/model/ModelTree.java (original) +++ ofbiz/ofbiz-framework/trunk/framework/widget/src/main/java/org/apache/ofbiz/widget/model/ModelTree.java Tue Jun 13 08:29:28 2017 @@ -396,14 +396,12 @@ public class ModelTree extends ModelWidg // List dataFound = (List)context.get("dataFound"); Iterator<? extends Map<String, ? extends Object>> dataIter = subNode.getListIterator(context); if (dataIter instanceof EntityListIterator) { - EntityListIterator eli = (EntityListIterator) dataIter; - Map<String, Object> val = null; - while ((val = eli.next()) != null) { - Object[] arr = { node, val }; - subNodeValues.add(arr); - } - try { - eli.close(); + try (EntityListIterator eli = (EntityListIterator) dataIter) { + Map<String, Object> val = null; + while ((val = eli.next()) != null) { + Object[] arr = { node, val }; + subNodeValues.add(arr); + } } catch (GenericEntityException e) { Debug.logError(e, module); throw new RuntimeException(e.getMessage()); |
«
Return to OFBiz - Commits
|
1 view|%1 views
| Free forum by Nabble | Edit this page |