This is an automated email from the ASF dual-hosted git repository.
mridulpathak pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git
The following commit(s) were added to refs/heads/trunk by this push:
new 68a5b47c7b Make entity condition lists and field maps immutable
68a5b47c7b is described below
commit 68a5b47c7b4901fc32584c7ddc1c3f047e39cb4f
Author: Mridul Pathak <[email protected]>
AuthorDate: Mon Oct 26 23:17:24 2026 +0530
Make entity condition lists and field maps immutable
EntityConditionListBase and EntityFieldMap now keep an unmodifiable copy of
the list or map they are built from, instead of holding on to the caller's
collection, so later changes to the source no longer alter the condition and
getConditions() can't be used to modify it. The varargs makeCondition factories
copy their elements into a new list rather than wrapping the array with
Arrays.asList, which lets the build drop the -Xlint:-varargs exclusion. The
EntityFieldMap varargs constructo [...]
---
build.gradle | 2 --
.../ofbiz/base/util/collections/MapContextTest.java | 2 --
.../ofbiz/entity/condition/EntityCondition.java | 20 ++++++++++++++------
.../entity/condition/EntityConditionListBase.java | 7 +++++--
.../ofbiz/entity/condition/EntityFieldMap.java | 21 ++++-----------------
.../org/apache/ofbiz/entity/util/EntityUtil.java | 9 ++++-----
6 files changed, 27 insertions(+), 34 deletions(-)
diff --git a/build.gradle b/build.gradle
index 3875426e1f..254b3b130e 100644
--- a/build.gradle
+++ b/build.gradle
@@ -212,8 +212,6 @@ tasks.withType(JavaCompile) {
options.encoding = 'UTF-8'
if (!project.hasProperty('Xlint:none')) {
options.compilerArgs << '-Xlint:all'
- // Exclude varargs warnings which are not silenced by @SafeVarargs.
- options.compilerArgs << '-Xlint:-varargs'
// this-escape lint was introduced in JDK 21's javac and is an invalid
flag on JDK 17,
// so only pass it when compiling with a JDK that recognizes it.
if (JavaVersion.current().isCompatibleWith(JavaVersion.VERSION_21)) {
diff --git
a/framework/base/src/test/java/org/apache/ofbiz/base/util/collections/MapContextTest.java
b/framework/base/src/test/java/org/apache/ofbiz/base/util/collections/MapContextTest.java
index 723da34b77..c9acf602a4 100644
---
a/framework/base/src/test/java/org/apache/ofbiz/base/util/collections/MapContextTest.java
+++
b/framework/base/src/test/java/org/apache/ofbiz/base/util/collections/MapContextTest.java
@@ -48,7 +48,6 @@ public class MapContextTest {
* Constructs a node without properties.
* @param includes the included nodes
*/
- @SafeVarargs
PNode(PNode... includes) {
this(Collections.emptyMap(), includes);
}
@@ -58,7 +57,6 @@ public class MapContextTest {
* @param props the properties of the node
* @param includes the included nodes
*/
- @SafeVarargs
PNode(Map<String, String> props, PNode... includes) {
this.props = props;
this.includes = Arrays.asList(includes);
diff --git
a/framework/entity/src/main/java/org/apache/ofbiz/entity/condition/EntityCondition.java
b/framework/entity/src/main/java/org/apache/ofbiz/entity/condition/EntityCondition.java
index a994adc351..538800c1af 100644
---
a/framework/entity/src/main/java/org/apache/ofbiz/entity/condition/EntityCondition.java
+++
b/framework/entity/src/main/java/org/apache/ofbiz/entity/condition/EntityCondition.java
@@ -20,7 +20,6 @@ package org.apache.ofbiz.entity.condition;
import java.io.Serializable;
import java.util.ArrayList;
-import java.util.Arrays;
import java.util.List;
import java.util.Map;
@@ -30,6 +29,7 @@ import org.apache.ofbiz.entity.GenericEntity;
import org.apache.ofbiz.entity.GenericModelException;
import org.apache.ofbiz.entity.config.model.Datasource;
import org.apache.ofbiz.entity.model.ModelEntity;
+import org.apache.ofbiz.entity.util.EntityUtil;
/**
* Represents a condition expression that can be used in a SQL 'WHERE' clause
@@ -88,7 +88,11 @@ public interface EntityCondition extends IsEmpty,
Serializable {
@SafeVarargs
static <R extends EntityCondition, T extends R>
EntityConditionList<R> makeCondition(EntityJoinOperator operator,
T... conditionList) {
- return new EntityConditionList<>(Arrays.asList(conditionList),
operator);
+ List<R> conditions = new ArrayList<>(conditionList.length);
+ for (T condition : conditionList) {
+ conditions.add(condition);
+ }
+ return new EntityConditionList<>(conditions, operator);
}
/**
@@ -99,7 +103,11 @@ public interface EntityCondition extends IsEmpty,
Serializable {
@SafeVarargs
static <R extends EntityCondition, T extends R>
EntityConditionList<R> makeCondition(T... conditionList) {
- return new EntityConditionList<>(Arrays.asList(conditionList),
EntityOperator.AND);
+ List<R> conditions = new ArrayList<>(conditionList.length);
+ for (T condition : conditionList) {
+ conditions.add(condition);
+ }
+ return new EntityConditionList<>(conditions, EntityOperator.AND);
}
/**
@@ -162,7 +170,7 @@ public interface EntityCondition extends IsEmpty,
Serializable {
*/
static <L, R> EntityFieldMap makeCondition(EntityComparisonOperator<L, R>
compOp, EntityJoinOperator joinOp,
Object... keysValues) {
- return new EntityFieldMap(compOp, joinOp, keysValues);
+ return new EntityFieldMap(EntityUtil.makeFields(keysValues), compOp,
joinOp);
}
/**
@@ -172,7 +180,7 @@ public interface EntityCondition extends IsEmpty,
Serializable {
* @return a condition expression representing a combination of
field/value equality comparisons
*/
static EntityFieldMap makeCondition(EntityJoinOperator joinOp, Object...
keysValues) {
- return new EntityFieldMap(EntityOperator.EQUALS, joinOp, keysValues);
+ return new EntityFieldMap(EntityUtil.makeFields(keysValues),
EntityOperator.EQUALS, joinOp);
}
/**
@@ -181,7 +189,7 @@ public interface EntityCondition extends IsEmpty,
Serializable {
* @return a condition expression representing a conjunction of
field/value equality comparisons
*/
static EntityFieldMap makeConditionMap(Object... keysValues) {
- return new EntityFieldMap(EntityOperator.EQUALS, EntityOperator.AND,
keysValues);
+ return new EntityFieldMap(EntityUtil.makeFields(keysValues),
EntityOperator.EQUALS, EntityOperator.AND);
}
/**
diff --git
a/framework/entity/src/main/java/org/apache/ofbiz/entity/condition/EntityConditionListBase.java
b/framework/entity/src/main/java/org/apache/ofbiz/entity/condition/EntityConditionListBase.java
index 90042f3e82..0e5faf9bac 100644
---
a/framework/entity/src/main/java/org/apache/ofbiz/entity/condition/EntityConditionListBase.java
+++
b/framework/entity/src/main/java/org/apache/ofbiz/entity/condition/EntityConditionListBase.java
@@ -18,6 +18,8 @@
*******************************************************************************/
package org.apache.ofbiz.entity.condition;
+import java.util.ArrayList;
+import java.util.Collections;
import java.util.List;
import java.util.Map;
@@ -37,18 +39,19 @@ abstract class EntityConditionListBase<T extends
EntityCondition> implements Ent
return conditions;
}
- /** The list of condition expressions to combine. */
+ /** The unmodifiable list of condition expressions to combine. */
private final List<? extends T> conditions;
/** The infix operator used to combine every elements in the list of
conditions. */
private final EntityJoinOperator operator;
/**
* Constructs a combination of multiple condition expressions.
+ * The list is copied, so later changes to {@code conditions} do not
affect this condition.
* @param conditions the list of condition expressions to combine
* @param operator the infix operator used to combine every elements in
the list of conditions
*/
protected EntityConditionListBase(List<? extends T> conditions,
EntityJoinOperator operator) {
- this.conditions = conditions;
+ this.conditions = Collections.unmodifiableList(new
ArrayList<>(conditions));
this.operator = operator;
}
diff --git
a/framework/entity/src/main/java/org/apache/ofbiz/entity/condition/EntityFieldMap.java
b/framework/entity/src/main/java/org/apache/ofbiz/entity/condition/EntityFieldMap.java
index 2cff636708..67af58856b 100644
---
a/framework/entity/src/main/java/org/apache/ofbiz/entity/condition/EntityFieldMap.java
+++
b/framework/entity/src/main/java/org/apache/ofbiz/entity/condition/EntityFieldMap.java
@@ -20,12 +20,12 @@ package org.apache.ofbiz.entity.condition;
import java.util.Collections;
import java.util.Iterator;
+import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
import org.apache.ofbiz.base.util.UtilGenerics;
-import org.apache.ofbiz.entity.util.EntityUtil;
/**
* A condition expression corresponding to an unordered collection of
@@ -40,7 +40,7 @@ import org.apache.ofbiz.entity.util.EntityUtil;
@SuppressWarnings("serial")
public final class EntityFieldMap extends EntityConditionListBase<EntityExpr> {
- /** The map whose entries correspond to the set of equality checks
conditions. */
+ /** The unmodifiable map whose entries correspond to the set of equality
checks conditions. */
private final Map<String, ?> fieldMap;
/**
@@ -59,20 +59,7 @@ public final class EntityFieldMap extends
EntityConditionListBase<EntityExpr> {
/**
* Constructs a map of fields.
- * @param compOp the operator used to compare fields
- * @param joinOp the operator used to join field comparisons
- * @param keysValues a list of values that the field map will contain.
- * This list must be of even length and each successive pair will
- * be associated in the field map.
- * @param <V> The type of values that are compared.
- */
- @SafeVarargs
- public <V> EntityFieldMap(EntityComparisonOperator<?, ?> compOp,
EntityJoinOperator joinOp, V... keysValues) {
- this(EntityUtil.makeFields(keysValues), UtilGenerics.cast(compOp),
joinOp);
- }
-
- /**
- * Constructs a map of fields.
+ * The map is copied in its iteration order, so later changes to {@code
fieldMap} do not affect this condition.
* @param fieldMap the map containing the fields to compare
* @param compOp the operator to compare fields
* @param joinOp the operator to join entries in the field map
@@ -81,7 +68,7 @@ public final class EntityFieldMap extends
EntityConditionListBase<EntityExpr> {
public <V> EntityFieldMap(Map<String, V> fieldMap,
EntityComparisonOperator<?, ?> compOp,
EntityJoinOperator joinOp) {
super(makeConditionList(fieldMap, UtilGenerics.cast(compOp)), joinOp);
- this.fieldMap = (fieldMap == null) ? Collections.emptyMap() : fieldMap;
+ this.fieldMap = (fieldMap == null) ? Collections.emptyMap() :
Collections.unmodifiableMap(new LinkedHashMap<>(fieldMap));
}
/**
diff --git
a/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityUtil.java
b/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityUtil.java
index f9a1749cc9..7a6d4b5a96 100644
---
a/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityUtil.java
+++
b/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityUtil.java
@@ -65,16 +65,15 @@ public final class EntityUtil {
private EntityUtil() { }
- @SafeVarargs
- public static <V> Map<String, V> makeFields(V... args) {
- Map<String, V> fields = new HashMap<>();
+ public static Map<String, Object> makeFields(Object... args) {
+ Map<String, Object> fields = new HashMap<>();
if (args != null) {
for (int i = 0; i < args.length;) {
- V keyValue = args[i];
+ Object keyValue = args[i];
if (!(keyValue instanceof String)) throw new
IllegalArgumentException("Key(" + i + "), with value(" + args[i] + ") is not a
String.");
String key = (String) keyValue;
i++;
- V value = args[i];
+ Object value = args[i];
if (value != null) {
if (!(value instanceof Comparable<?>)) {
throw new IllegalArgumentException("Value(" + i + "),
with value(" + args[i] + ") does not implement Comparable.");
