You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by jl...@apache.org on 2017/06/13 08:29:28 UTC
svn commit: r1798566 - in /ofbiz/ofbiz-framework/trunk:
applications/party/src/main/java/org/apache/ofbiz/party/party/
applications/product/src/main/java/org/apache/ofbiz/product/feature/
applications/product/src/main/java/org/apache/ofbiz/product/prod...
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());