You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by ad...@apache.org on 2012/07/04 15:13:55 UTC
svn commit: r1357241 - in /ofbiz/branches/release12.04/framework/entity: ./
src/org/ofbiz/entity/finder/EntityFinderUtil.java
Author: adrianc
Date: Wed Jul 4 13:13:55 2012
New Revision: 1357241
URL: http://svn.apache.org/viewvc?rev=1357241&view=rev
Log:
Merging revs 1356024, 1356168, 1356201 from trunk. Those revisions were mostly bug fixes, but they also included some optimizations.
1. Made the Condition implementations immutable and thread-safe.
2. Moved operator lookup from run-time to parse-time.
3. Removed unnecessary calls to UtilValidate.
4. Replaced LinkedList with ArrayList.
5. Optimized code in the ConditionExpr.createCondition method.
6. LimitRange and LimitView classes treated ELI and List indexes the same - causing inconsistent results because ELI indexes are one-based and List indexes are zero-based.
7. LimitRange and LimitView classes returned different results at the end of the list depending on whether an ELI or List was being used.
8. LimitView class view index calculation was wrong.
Modified:
ofbiz/branches/release12.04/framework/entity/ (props changed)
ofbiz/branches/release12.04/framework/entity/src/org/ofbiz/entity/finder/EntityFinderUtil.java
Propchange: ofbiz/branches/release12.04/framework/entity/
------------------------------------------------------------------------------
--- svn:mergeinfo (added)
+++ svn:mergeinfo Wed Jul 4 13:13:55 2012
@@ -0,0 +1,6 @@
+/ofbiz/branches/addbirt/framework/entity:831210-885099,885686-886087
+/ofbiz/branches/dojo1.4/framework/entity:951708-952957
+/ofbiz/branches/jackrabbit20100709/framework/entity:962442-1231517
+/ofbiz/branches/jquery/framework/entity:952958-1044489
+/ofbiz/branches/multitenant20100310/framework/entity:921280-927264
+/ofbiz/trunk/framework/entity:1332097,1333885,1334201,1334336,1334483,1335047,1335343,1335347,1335351,1335946,1336921,1337046,1337057-1337059,1337202,1337502,1337524,1337644,1337789,1337800,1338065,1338101,1338224,1338570,1338591,1338700,1338831,1338845,1338974,1339081,1339122,1340273,1340352,1340357,1340400,1340405,1340415,1340657,1340661,1340774,1340821,1340826,1340943,1341314,1341399,1342875,1342893,1342980,1343088,1345473,1345484,1345532,1345547,1345553,1347762,1351778,1351999,1355660,1355801,1355859,1355975,1356024,1356168,1356201
Modified: ofbiz/branches/release12.04/framework/entity/src/org/ofbiz/entity/finder/EntityFinderUtil.java
URL: http://svn.apache.org/viewvc/ofbiz/branches/release12.04/framework/entity/src/org/ofbiz/entity/finder/EntityFinderUtil.java?rev=1357241&r1=1357240&r2=1357241&view=diff
==============================================================================
--- ofbiz/branches/release12.04/framework/entity/src/org/ofbiz/entity/finder/EntityFinderUtil.java (original)
+++ ofbiz/branches/release12.04/framework/entity/src/org/ofbiz/entity/finder/EntityFinderUtil.java Wed Jul 4 13:13:55 2012
@@ -21,19 +21,20 @@ package org.ofbiz.entity.finder;
import static org.ofbiz.base.util.UtilGenerics.cast;
import java.io.Serializable;
+import java.util.ArrayList;
import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
import java.util.HashSet;
-import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Set;
-import javolution.util.FastMap;
+import javolution.util.FastList;
import org.ofbiz.base.util.Debug;
import org.ofbiz.base.util.ObjectType;
import org.ofbiz.base.util.StringUtil;
-import org.ofbiz.base.util.UtilFormatOut;
import org.ofbiz.base.util.UtilGenerics;
import org.ofbiz.base.util.UtilValidate;
import org.ofbiz.base.util.UtilXml;
@@ -63,24 +64,24 @@ public class EntityFinderUtil {
Map<FlexibleMapAccessor<Object>, Object> fieldMap = null;
List<? extends Element> fieldMapElementList = UtilXml.childElementList(element, "field-map");
if (fieldMapElementList.size() > 0) {
- fieldMap = FastMap.newInstance();
+ fieldMap = new HashMap<FlexibleMapAccessor<Object>, Object>(fieldMapElementList.size());
for (Element fieldMapElement: fieldMapElementList) {
// set the env-name for each field-name, noting that if no field-name is specified it defaults to the env-name
String fieldName = fieldMapElement.getAttribute("field-name");
String envName = fieldMapElement.getAttribute("from-field");
- if (UtilValidate.isEmpty(envName)) {
+ if (envName.isEmpty()) {
envName = fieldMapElement.getAttribute("env-name");
}
String value = fieldMapElement.getAttribute("value");
- if (UtilValidate.isEmpty(fieldName)) {
+ if (fieldName.isEmpty()) {
// no fieldName, use envName for both
fieldMap.put(FlexibleMapAccessor.getInstance(envName), FlexibleMapAccessor.getInstance(envName));
} else {
- if (UtilValidate.isNotEmpty(value)) {
+ if (!value.isEmpty()) {
fieldMap.put(FlexibleMapAccessor.getInstance(fieldName), FlexibleStringExpander.getInstance(value));
} else {
// at this point we have a fieldName and no value, do we have a envName?
- if (UtilValidate.isNotEmpty(envName)) {
+ if (!envName.isEmpty()) {
fieldMap.put(FlexibleMapAccessor.getInstance(fieldName), FlexibleMapAccessor.getInstance(envName));
} else {
// no envName, use fieldName for both
@@ -116,7 +117,7 @@ public class EntityFinderUtil {
List<FlexibleStringExpander> selectFieldExpanderList = null;
List<? extends Element> selectFieldElementList = UtilXml.childElementList(element, "select-field");
if (selectFieldElementList.size() > 0) {
- selectFieldExpanderList = new LinkedList<FlexibleStringExpander>();
+ selectFieldExpanderList = new ArrayList<FlexibleStringExpander>(selectFieldElementList.size());
for (Element selectFieldElement: selectFieldElementList) {
selectFieldExpanderList.add(FlexibleStringExpander.getInstance(selectFieldElement.getAttribute("field-name")));
}
@@ -138,7 +139,7 @@ public class EntityFinderUtil {
public static List<String> makeOrderByFieldList(List<FlexibleStringExpander> orderByExpanderList, Map<String, Object> context) {
List<String> orderByFields = null;
if (UtilValidate.isNotEmpty(orderByExpanderList)) {
- orderByFields = new LinkedList<String>();
+ orderByFields = new ArrayList<String>(orderByExpanderList.size());
for (FlexibleStringExpander orderByExpander: orderByExpanderList) {
orderByFields.add(orderByExpander.expandString(context));
}
@@ -149,30 +150,37 @@ public class EntityFinderUtil {
public static interface Condition extends Serializable {
public EntityCondition createCondition(Map<String, ? extends Object> context, ModelEntity modelEntity, ModelFieldTypeReader modelFieldTypeReader);
}
+
@SuppressWarnings("serial")
- public static class ConditionExpr implements Condition {
- protected FlexibleStringExpander fieldNameExdr;
- protected FlexibleStringExpander operatorExdr;
- protected FlexibleMapAccessor<Object> envNameAcsr;
- protected FlexibleStringExpander valueExdr;
- protected FlexibleStringExpander ignoreExdr;
- protected boolean ignoreIfNull;
- protected boolean ignoreIfEmpty;
- protected boolean ignoreCase;
+ public static final class ConditionExpr implements Condition {
+ private final String fieldName;
+ private final EntityOperator<?,?,?> operator;
+ private final FlexibleMapAccessor<Object> envNameAcsr;
+ private final FlexibleStringExpander valueExdr;
+ private final FlexibleStringExpander ignoreExdr;
+ private final boolean ignoreIfNull;
+ private final boolean ignoreIfEmpty;
+ private final boolean ignoreCase;
public ConditionExpr(Element conditionExprElement) {
- this.fieldNameExdr = FlexibleStringExpander.getInstance(conditionExprElement.getAttribute("field-name"));
- if (this.fieldNameExdr.isEmpty()) {
- // no "field-name"? try "name"
- this.fieldNameExdr = FlexibleStringExpander.getInstance(conditionExprElement.getAttribute("name"));
- }
-
- this.operatorExdr = FlexibleStringExpander.getInstance(UtilFormatOut.checkEmpty(conditionExprElement.getAttribute("operator"), "equals"));
- if (UtilValidate.isNotEmpty(conditionExprElement.getAttribute("from-field"))) {
- this.envNameAcsr = FlexibleMapAccessor.getInstance(conditionExprElement.getAttribute("from-field"));
- } else {
- this.envNameAcsr = FlexibleMapAccessor.getInstance(conditionExprElement.getAttribute("env-name"));
+ String fieldNameAttribute = conditionExprElement.getAttribute("field-name");
+ if (fieldNameAttribute.isEmpty()) {
+ fieldNameAttribute = conditionExprElement.getAttribute("name");
+ }
+ this.fieldName = fieldNameAttribute;
+ String operatorAttribute = conditionExprElement.getAttribute("operator");
+ if (operatorAttribute.isEmpty()) {
+ operatorAttribute = "equals";
+ }
+ this.operator = EntityOperator.lookup(operatorAttribute);
+ if (this.operator == null) {
+ throw new IllegalArgumentException("Could not find an entity operator for the name: " + operatorAttribute);
+ }
+ String fromFieldAttribute = conditionExprElement.getAttribute("from-field");
+ if (fromFieldAttribute.isEmpty()) {
+ fromFieldAttribute = conditionExprElement.getAttribute("env-name");
}
+ this.envNameAcsr = FlexibleMapAccessor.getInstance(fromFieldAttribute);
this.valueExdr = FlexibleStringExpander.getInstance(conditionExprElement.getAttribute("value"));
this.ignoreIfNull = "true".equals(conditionExprElement.getAttribute("ignore-if-null"));
this.ignoreIfEmpty = "true".equals(conditionExprElement.getAttribute("ignore-if-empty"));
@@ -181,22 +189,19 @@ public class EntityFinderUtil {
}
public EntityCondition createCondition(Map<String, ? extends Object> context, ModelEntity modelEntity, ModelFieldTypeReader modelFieldTypeReader) {
- String fieldName = fieldNameExdr.expandString(context);
-
- Object value = null;
- // start with the environment variable, will override if exists and a value is specified
- if (envNameAcsr != null) {
- value = envNameAcsr.get(context);
+ if ("true".equals(this.ignoreExdr.expandString(context))) {
+ return null;
}
- // no value so far, and a string value is specified, use that
- if (value == null && valueExdr != null) {
- value = valueExdr.expandString(context);
+ if (modelEntity.getField(fieldName) == null) {
+ throw new IllegalArgumentException("Error in Entity Find: could not find field [" + fieldName + "] in entity with name [" + modelEntity.getEntityName() + "]");
}
- String operatorName = operatorExdr.expandString(context);
- EntityOperator<?,?,?> operator = EntityOperator.lookup(operatorName);
- if (operator == null) {
- throw new IllegalArgumentException("Could not find an entity operator for the name: " + operatorName);
+ Object value = envNameAcsr.get(context);
+ if (value == null && !valueExdr.isEmpty()) {
+ value = valueExdr.expandString(context);
+ }
+ if (this.ignoreIfNull && value == null) {
+ return null;
}
// If IN or BETWEEN operator, see if value is a literal list and split it
@@ -208,15 +213,11 @@ public class EntityFinderUtil {
} else if (((String)value).indexOf(",") >= 0) {
delim = ",";
}
- if (UtilValidate.isNotEmpty(delim)) {
+ if (delim != null) {
value = StringUtil.split((String)value, delim);
}
}
- if (modelEntity.getField(fieldName) == null) {
- throw new IllegalArgumentException("Error in Entity Find: could not find field [" + fieldName + "] in entity with name [" + modelEntity.getEntityName() + "]");
- }
-
// don't convert the field to the desired type if this is an IN or BETWEEN operator and we have a Collection
if (!((operator.equals(EntityOperator.IN) || operator.equals(EntityOperator.BETWEEN) || operator.equals(EntityOperator.NOT_IN))
&& value instanceof Collection<?>)) {
@@ -226,17 +227,10 @@ public class EntityFinderUtil {
if (Debug.verboseOn()) Debug.logVerbose("Got value for fieldName [" + fieldName + "]: " + value, module);
- if (this.ignoreIfNull && value == null) {
- return null;
- }
if (this.ignoreIfEmpty && ObjectType.isEmpty(value)) {
return null;
}
- if ("true".equals(this.ignoreExdr.expandString(context))) {
- return null;
- }
-
if (operator == EntityOperator.NOT_EQUAL && value != null) {
// since some databases don't consider nulls in != comparisons, explicitly include them
// this makes more sense logically, but if anyone ever needs it to not behave this way we should add an "or-null" attribute that is true by default
@@ -263,68 +257,72 @@ public class EntityFinderUtil {
}
@SuppressWarnings("serial")
- public static class ConditionList implements Condition {
- List<Condition> conditionList = new LinkedList<Condition>();
- FlexibleStringExpander combineExdr;
+ public static final class ConditionList implements Condition {
+ private final List<Condition> conditionList;
+ private final EntityOperator<?,?,?> operator;
public ConditionList(Element conditionListElement) {
- this.combineExdr = FlexibleStringExpander.getInstance(conditionListElement.getAttribute("combine"));
-
+ String operatorAttribute = conditionListElement.getAttribute("combine");
+ if (operatorAttribute.isEmpty()) {
+ operatorAttribute = "and";
+ }
+ this.operator = EntityOperator.lookup(operatorAttribute);
+ if (this.operator == null) {
+ throw new IllegalArgumentException("Could not find an entity operator for the name: " + operatorAttribute);
+ }
List<? extends Element> subElements = UtilXml.childElementList(conditionListElement);
- for (Element subElement: subElements) {
- if ("condition-expr".equals(subElement.getNodeName())) {
- conditionList.add(new ConditionExpr(subElement));
- } else if ("condition-list".equals(subElement.getNodeName())) {
- conditionList.add(new ConditionList(subElement));
- } else if ("condition-object".equals(subElement.getNodeName())) {
- conditionList.add(new ConditionObject(subElement));
- } else {
- throw new IllegalArgumentException("Invalid element with name [" + subElement.getNodeName() + "] found under a condition-list element.");
+ if (subElements.isEmpty()) {
+ this.conditionList = null;
+ } else {
+ List<Condition> conditionList = new ArrayList<Condition>(subElements.size());
+ for (Element subElement : subElements) {
+ if ("condition-expr".equals(subElement.getNodeName())) {
+ conditionList.add(new ConditionExpr(subElement));
+ } else if ("condition-list".equals(subElement.getNodeName())) {
+ conditionList.add(new ConditionList(subElement));
+ } else if ("condition-object".equals(subElement.getNodeName())) {
+ conditionList.add(new ConditionObject(subElement));
+ } else {
+ throw new IllegalArgumentException("Invalid element with name [" + subElement.getNodeName() + "] found under a condition-list element.");
+ }
}
+ this.conditionList = Collections.unmodifiableList(conditionList);
}
}
public EntityCondition createCondition(Map<String, ? extends Object> context, ModelEntity modelEntity, ModelFieldTypeReader modelFieldTypeReader) {
- if (this.conditionList.size() == 0) {
+ if (this.conditionList == null) {
return null;
}
if (this.conditionList.size() == 1) {
Condition condition = this.conditionList.get(0);
return condition.createCondition(context, modelEntity, modelFieldTypeReader);
}
-
- List<EntityCondition> entityConditionList = new LinkedList<EntityCondition>();
- for (Condition curCondition: conditionList) {
+ List<EntityCondition> entityConditionList = new ArrayList<EntityCondition>(this.conditionList.size());
+ for (Condition curCondition: this.conditionList) {
EntityCondition econd = curCondition.createCondition(context, modelEntity, modelFieldTypeReader);
if (econd != null) {
entityConditionList.add(econd);
}
}
-
- String operatorName = combineExdr.expandString(context);
- EntityOperator<?,?,?> operator = EntityOperator.lookup(operatorName);
- if (operator == null) {
- throw new IllegalArgumentException("Could not find an entity operator for the name: " + operatorName);
- }
-
return EntityCondition.makeCondition(entityConditionList, UtilGenerics.<EntityJoinOperator>cast(operator));
}
}
+
@SuppressWarnings("serial")
- public static class ConditionObject implements Condition {
- protected FlexibleMapAccessor<Object> fieldNameAcsr;
+ public static final class ConditionObject implements Condition {
+ private final FlexibleMapAccessor<Object> fieldNameAcsr;
public ConditionObject(Element conditionExprElement) {
- this.fieldNameAcsr = FlexibleMapAccessor.getInstance(conditionExprElement.getAttribute("field"));
- if (this.fieldNameAcsr.isEmpty()) {
- // no "field"? try "field-name"
- this.fieldNameAcsr = FlexibleMapAccessor.getInstance(conditionExprElement.getAttribute("field-name"));
+ String fieldNameAttribute = conditionExprElement.getAttribute("field");
+ if (fieldNameAttribute.isEmpty()) {
+ fieldNameAttribute = conditionExprElement.getAttribute("field-name");
}
+ this.fieldNameAcsr = FlexibleMapAccessor.getInstance(fieldNameAttribute);
}
public EntityCondition createCondition(Map<String, ? extends Object> context, ModelEntity modelEntity, ModelFieldTypeReader modelFieldTypeReader) {
- EntityCondition condition = (EntityCondition) fieldNameAcsr.get(context);
- return condition;
+ return (EntityCondition) fieldNameAcsr.get(context);
}
}
@@ -332,6 +330,7 @@ public class EntityFinderUtil {
public void handleOutput(EntityListIterator eli, Map<String, Object> context, FlexibleMapAccessor<Object> listAcsr);
public void handleOutput(List<GenericValue> results, Map<String, Object> context, FlexibleMapAccessor<Object> listAcsr);
}
+
@SuppressWarnings("serial")
public static class LimitRange implements OutputHandler {
FlexibleStringExpander startExdr;
@@ -365,7 +364,7 @@ public class EntityFinderUtil {
}
public void handleOutput(EntityListIterator eli, Map<String, Object> context, FlexibleMapAccessor<Object> listAcsr) {
- int start = getStart(context);
+ int start = getStart(context) + 1; // ELI index is one-based.
int size = getSize(context);
try {
listAcsr.put(context, eli.getPartialList(start, size));
@@ -378,15 +377,22 @@ public class EntityFinderUtil {
}
public void handleOutput(List<GenericValue> results, Map<String, Object> context, FlexibleMapAccessor<Object> listAcsr) {
+ List<GenericValue> result = null;
int start = getStart(context);
- int size = getSize(context);
-
- int end = start + size;
- if (end > results.size()) end = results.size();
-
- listAcsr.put(context, results.subList(start, end));
+ if (start < results.size()) {
+ int size = getSize(context);
+ int end = start + size;
+ if (end > results.size()) {
+ end = results.size();
+ }
+ result = results.subList(start, end);
+ } else {
+ result = FastList.newInstance();
+ }
+ listAcsr.put(context, result);
}
}
+
@SuppressWarnings("serial")
public static class LimitView implements OutputHandler {
FlexibleStringExpander viewIndexExdr;
@@ -422,9 +428,8 @@ public class EntityFinderUtil {
public void handleOutput(EntityListIterator eli, Map<String, Object> context, FlexibleMapAccessor<Object> listAcsr) {
int index = this.getIndex(context);
int size = this.getSize(context);
-
try {
- listAcsr.put(context, eli.getPartialList(((index - 1) * size) + 1, size));
+ listAcsr.put(context, eli.getPartialList(((index - 1) * size) + 1, size)); // ELI index is one-based.
eli.close();
} catch (GenericEntityException e) {
String errMsg = "Error getting partial list in limit-view with index=" + index + " and size=" + size + ": " + e.toString();
@@ -434,16 +439,23 @@ public class EntityFinderUtil {
}
public void handleOutput(List<GenericValue> results, Map<String, Object> context, FlexibleMapAccessor<Object> listAcsr) {
+ List<GenericValue> result = null;
int index = this.getIndex(context);
int size = this.getSize(context);
-
- int begin = index * size;
- int end = index * size + size;
- if (end > results.size()) end = results.size();
-
- listAcsr.put(context, results.subList(begin, end));
+ int begin = (index - 1) * size;
+ if (begin < results.size()) {
+ int end = begin + size;
+ if (end > results.size()) {
+ end = results.size();
+ }
+ result = results.subList(begin, end);
+ } else {
+ result = FastList.newInstance();
+ }
+ listAcsr.put(context, result);
}
}
+
@SuppressWarnings("serial")
public static class UseIterator implements OutputHandler {
public UseIterator(Element useIteratorElement) {