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 2015/11/25 09:16:56 UTC
svn commit: r1716322 - in /ofbiz/branches/release14.12: ./
framework/entityext/src/org/ofbiz/entityext/eca/EntityEcaCondition.java
framework/entityext/src/org/ofbiz/entityext/eca/EntityEcaRule.java
Author: jleroux
Date: Wed Nov 25 08:16:55 2015
New Revision: 1716322
URL: http://svn.apache.org/viewvc?rev=1716322&view=rev
Log:
"Applied fix from trunk for revision: 1716319 "
------------------------------------------------------------------------
r1716319 | jleroux | 2015-11-25 09:15:42 +0100 (mer. 25 nov. 2015) | 17 lignes
A patch from Anne Jessel for "Entity ECAs not triggered correctly when using Delegator.storeAll() method" https://issues.apache.org/jira/browse/OFBIZ-3847
As reported by Martin Kreidenweis
The conditions don't work when updating (not creating) entities using the Delegator.storeAll() method. E.g. the following condition does not work:
<eca entity="Product" operation="create-store" event="return">
<condition field-name="autoCreateKeywords" operator="not-equals" value="N"/>
<action service="indexProductKeywords" mode="sync" value-attr="productInstance"/>
</eca>
The indexProductKeywords service is called anyway when the product is updated and the autoCreateKeywords was "N" and stays "N". It works correctly for newly created products.
The problem is in the method GenericDelegator.storeAll(), where unchanged field values are not passed down to the store() method. The store method calls the ECA engine, which does not receive the unchanged values at all and thus cannot evaluate the EECA conditions correctly.
Scott's comment:We could consider having EntityEcaCondition.eval() perform a db lookup if any of the fields it needs to evaluate are missing. If they are missing it could fully populate the entity with the missing fields so that a lookup is only required at most once per delegator operation. If no ECA conditions require any missing fields then the lookup wouldn't be performed, so the performance penalty would only be incurred when necessary.
Anne: I've created a patch modeled after Scott's suggestion. I'd appreciate someone reviewing it. It does work for me, however I am concerned that it might fail when the value of a field that is part of the EECA condition is being changed from non-null to null using GenericDelegator.storeAll, because of the way storeAll works. There may be a simple solution I've missed.
jleroux: After reviewing and waiting for Anne's feedback, I commit because even if in some cases it would not work it's at least better than current situation
------------------------------------------------------------------------
Modified:
ofbiz/branches/release14.12/ (props changed)
ofbiz/branches/release14.12/framework/entityext/src/org/ofbiz/entityext/eca/EntityEcaCondition.java
ofbiz/branches/release14.12/framework/entityext/src/org/ofbiz/entityext/eca/EntityEcaRule.java
Propchange: ofbiz/branches/release14.12/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Wed Nov 25 08:16:55 2015
@@ -8,4 +8,4 @@
/ofbiz/branches/json-integration-refactoring:1634077-1635900
/ofbiz/branches/multitenant20100310:921280-927264
/ofbiz/branches/release13.07:1547657
-/ofbiz/trunk:1649072,1649083-1649084,1649086,1649090,1649096,1649230,1649238-1649239,1649248,1649272,1649275,1649280-1649281,1649283,1649285-1649286,1649291,1649329,1649331,1649384,1649393,1649666,1649742,1650240,1650348,1650357,1650583,1650642,1650678,1650821,1650882,1650887,1650938,1651593,1652312,1652361,1652638,1652641,1652672,1652688,1652706,1652725,1652731,1652739,1652852,1653248,1653296,1653456,1653597,1653614,1654175,1654273,1654509,1654670,1654672-1654673,1654683-1654684,1654824,1655046,1655668,1655979,1656014,1656185,1656198,1656445,1656983,1657323,1657506-1657507,1657514,1657714,1657790,1657848,1658364,1658662,1658882,1659224,1659965,1660031,1660053,1660389,1660444,1660579,1661303,1661328,1661760,1661778,1661853,1661862,1661873,1661940,1661951,1661977,1662119-1662120,1662361,1662500,1662812,1662919,1663202,1663912,1663979,1664602,1664604,1664696,1665154,1665162,1665535,1666404,1666511,1666633,1666836,1666939,1666949,1666958,1667055,1667253,1667483,1667492,1667774,1668207,
1668214,1668236,1668246,1668258,1668263,1668265,1668270,1668277,1668314,1668657,1669317,1669588,1672427,1672430,1672846,1672853,1672856,1672862,1672873,1673764,1674447,1674464,1674491,1674496,1674908,1676674,1677123,1677597,1677769-1677770,1678294,1678882,1678911,1679689,1679697,1679709,1679720,1679728,1679732,1679957,1680155,1680288,1680304,1680671,1680675,1680733,1680840,1680881,1682272,1682295,1682415,1682633,1683998,1684094,1686360,1686536,1686545,1686566,1686569,1686574,1686583,1686635,1686651,1686970,1687427,1688772,1690086,1690581,1692357,1692458,1692600,1692604,1693393,1693579,1695017,1696018,1696234,1697590,1697647,1697993,1698259,1698261,1698263,1701164,1701441,1701819,1701825,1701936,1702002,1702548,1702704,1703121,1703586,1703945,1703954,1703965,1703971,1703976-1703977,1703981,1704000,1704014,1704018,1704036,1704043,1704052,1704082,1704140,1704230,1705004,1705329,1705405,1705412,1705417,1705427,1705532,1706159,1706162,1706316,1706531,1706549,1706553,1706561,1706569,17065
77,1706589,1706591,1706593,1706694,1707837,1707857,1708274,1708341,1708742,1708930,1709117,1710178,1710348,1711513,1712971,1714244,1714410,1714415,1714571,1714657,1715477-1715478,1715485,1715501
+/ofbiz/trunk:1649072,1649083-1649084,1649086,1649090,1649096,1649230,1649238-1649239,1649248,1649272,1649275,1649280-1649281,1649283,1649285-1649286,1649291,1649329,1649331,1649384,1649393,1649666,1649742,1650240,1650348,1650357,1650583,1650642,1650678,1650821,1650882,1650887,1650938,1651593,1652312,1652361,1652638,1652641,1652672,1652688,1652706,1652725,1652731,1652739,1652852,1653248,1653296,1653456,1653597,1653614,1654175,1654273,1654509,1654670,1654672-1654673,1654683-1654684,1654824,1655046,1655668,1655979,1656014,1656185,1656198,1656445,1656983,1657323,1657506-1657507,1657514,1657714,1657790,1657848,1658364,1658662,1658882,1659224,1659965,1660031,1660053,1660389,1660444,1660579,1661303,1661328,1661760,1661778,1661853,1661862,1661873,1661940,1661951,1661977,1662119-1662120,1662361,1662500,1662812,1662919,1663202,1663912,1663979,1664602,1664604,1664696,1665154,1665162,1665535,1666404,1666511,1666633,1666836,1666939,1666949,1666958,1667055,1667253,1667483,1667492,1667774,1668207,
1668214,1668236,1668246,1668258,1668263,1668265,1668270,1668277,1668314,1668657,1669317,1669588,1672427,1672430,1672846,1672853,1672856,1672862,1672873,1673764,1674447,1674464,1674491,1674496,1674908,1676674,1677123,1677597,1677769-1677770,1678294,1678882,1678911,1679689,1679697,1679709,1679720,1679728,1679732,1679957,1680155,1680288,1680304,1680671,1680675,1680733,1680840,1680881,1682272,1682295,1682415,1682633,1683998,1684094,1686360,1686536,1686545,1686566,1686569,1686574,1686583,1686635,1686651,1686970,1687427,1688772,1690086,1690581,1692357,1692458,1692600,1692604,1693393,1693579,1695017,1696018,1696234,1697590,1697647,1697993,1698259,1698261,1698263,1701164,1701441,1701819,1701825,1701936,1702002,1702548,1702704,1703121,1703586,1703945,1703954,1703965,1703971,1703976-1703977,1703981,1704000,1704014,1704018,1704036,1704043,1704052,1704082,1704140,1704230,1705004,1705329,1705405,1705412,1705417,1705427,1705532,1706159,1706162,1706316,1706531,1706549,1706553,1706561,1706569,17065
77,1706589,1706591,1706593,1706694,1707837,1707857,1708274,1708341,1708742,1708930,1709117,1710178,1710348,1711513,1712971,1714244,1714410,1714415,1714571,1714657,1715477-1715478,1715485,1715501,1716319
Modified: ofbiz/branches/release14.12/framework/entityext/src/org/ofbiz/entityext/eca/EntityEcaCondition.java
URL: http://svn.apache.org/viewvc/ofbiz/branches/release14.12/framework/entityext/src/org/ofbiz/entityext/eca/EntityEcaCondition.java?rev=1716322&r1=1716321&r2=1716322&view=diff
==============================================================================
--- ofbiz/branches/release14.12/framework/entityext/src/org/ofbiz/entityext/eca/EntityEcaCondition.java (original)
+++ ofbiz/branches/release14.12/framework/entityext/src/org/ofbiz/entityext/eca/EntityEcaCondition.java Wed Nov 25 08:16:55 2015
@@ -18,11 +18,13 @@
*******************************************************************************/
package org.ofbiz.entityext.eca;
+import java.util.ArrayList;
import java.util.LinkedList;
import java.util.List;
import org.ofbiz.base.util.Debug;
import org.ofbiz.base.util.ObjectType;
+import org.ofbiz.base.util.UtilValidate;
import org.ofbiz.entity.GenericEntity;
import org.ofbiz.entity.GenericEntityException;
import org.ofbiz.service.DispatchContext;
@@ -116,4 +118,16 @@ public final class EntityEcaCondition im
buf.append("[").append(format).append("]");
return buf.toString();
}
+
+ protected List<String> getFieldNames() {
+ List<String> fieldNameList = new ArrayList<String>();
+ if( UtilValidate.isNotEmpty(lhsValueName) ) {
+ fieldNameList.add(lhsValueName);
+ }
+ if( !constant && UtilValidate.isNotEmpty(rhsValueName)) {
+ fieldNameList.add(rhsValueName);
+ }
+ return fieldNameList;
+ }
+
}
Modified: ofbiz/branches/release14.12/framework/entityext/src/org/ofbiz/entityext/eca/EntityEcaRule.java
URL: http://svn.apache.org/viewvc/ofbiz/branches/release14.12/framework/entityext/src/org/ofbiz/entityext/eca/EntityEcaRule.java?rev=1716322&r1=1716321&r2=1716322&view=diff
==============================================================================
--- ofbiz/branches/release14.12/framework/entityext/src/org/ofbiz/entityext/eca/EntityEcaRule.java (original)
+++ ofbiz/branches/release14.12/framework/entityext/src/org/ofbiz/entityext/eca/EntityEcaRule.java Wed Nov 25 08:16:55 2015
@@ -27,8 +27,10 @@ import java.util.Set;
import org.ofbiz.base.util.Debug;
import org.ofbiz.base.util.UtilXml;
+import org.ofbiz.entity.Delegator;
import org.ofbiz.entity.GenericEntity;
import org.ofbiz.entity.GenericEntityException;
+import org.ofbiz.entity.GenericValue;
import org.ofbiz.service.DispatchContext;
import org.w3c.dom.Element;
@@ -47,6 +49,7 @@ public final class EntityEcaRule impleme
private final List<EntityEcaCondition> conditions;
private final List<Object> actionsAndSets;
private boolean enabled = true;
+ private final List<String> conditionFieldNames = new ArrayList<String>();
public EntityEcaRule(Element eca) {
this.entityName = eca.getAttribute("entity");
@@ -57,9 +60,13 @@ public final class EntityEcaRule impleme
ArrayList<Object> actionsAndSets = new ArrayList<Object>();
for (Element element: UtilXml.childElementList(eca)) {
if ("condition".equals(element.getNodeName())) {
- conditions.add(new EntityEcaCondition(element, true));
+ EntityEcaCondition ecaCond = new EntityEcaCondition(element, true);
+ conditions.add(ecaCond);
+ conditionFieldNames.addAll(ecaCond.getFieldNames());
} else if ("condition-field".equals(element.getNodeName())) {
- conditions.add(new EntityEcaCondition(element, false));
+ EntityEcaCondition ecaCond = new EntityEcaCondition(element, false);
+ conditions.add(ecaCond);
+ conditionFieldNames.addAll(ecaCond.getFieldNames());
} else if ("action".equals(element.getNodeName())) {
actionsAndSets.add(new EntityEcaAction(element));
} else if ("set".equals(element.getNodeName())) {
@@ -116,6 +123,22 @@ public final class EntityEcaRule impleme
if (!"any".equals(this.operationName) && this.operationName.indexOf(currentOperation) == -1) {
return;
}
+ // Are fields tested in a condition missing? If so, we need to load them
+ List<String> fieldsToLoad = new ArrayList<String>();
+ for( String conditionFieldName : conditionFieldNames) {
+ if( value.get(conditionFieldName) == null) {
+ fieldsToLoad.add(conditionFieldName);
+ }
+ }
+
+ if( !fieldsToLoad.isEmpty()) {
+ Delegator delegator = dctx.getDelegator();
+ GenericValue oldValue = delegator.findOne(entityName, value.getPrimaryKey(), false);
+ for( String fieldName : fieldsToLoad) {
+ value.put(fieldName, oldValue.get(fieldName));
+ }
+ }
+
Map<String, Object> context = new HashMap<String, Object>();
context.putAll(value);