This is an automated email from the ASF dual-hosted git repository.
henrib pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-jexl.git
The following commit(s) were added to refs/heads/master by this push:
new b10477ec JEXL-421: added methods to find 'best' common class; -
updated ArrayBuilder; - added test; - updated changes & release notes;
b10477ec is described below
commit b10477ec25c7b7ed52e58aca30e8ab3c940eeaa9
Author: Henri Biestro <hbies...@cloudera.com>
AuthorDate: Thu Feb 15 13:16:13 2024 +0100
JEXL-421: added methods to find 'best' common class;
- updated ArrayBuilder;
- added test;
- updated changes & release notes;
---
RELEASE-NOTES.txt | 1 +
src/changes/changes.xml | 3 +
.../commons/jexl3/internal/ArrayBuilder.java | 41 ++++--
.../jexl3/internal/introspection/ClassMisc.java | 128 ++++++++++++++++++
.../org/apache/commons/jexl3/ArrayTypeTest.java | 149 +++++++++++++++++++++
.../org/apache/commons/jexl3/Issues400Test.java | 4 -
6 files changed, 310 insertions(+), 16 deletions(-)
diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt
index d8bac03e..8e40650f 100644
--- a/RELEASE-NOTES.txt
+++ b/RELEASE-NOTES.txt
@@ -30,6 +30,7 @@ Version 3.3.1 is source and binary compatible with 3.3.
New Features in 3.3.1:
====================
+* JEXL-421: ArrayBuilder: array type should reflect common class of its
entries
* JEXL-419: Add permission syntax to allow class/method/field
* JEXL-408: Using JexlFeatures is tedious
* JEXL-404: Support array-access safe navigation (x?[y])
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index b5666812..0ef1e059 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -29,6 +29,9 @@
<body>
<release version="3.3.1" date="20YY-MM-DD">
<!-- ADD -->
+ <action dev="henrib" type="add" issue="JEXL-421">
+ ArrayBuilder: array type should reflect common class of its
entries
+ </action>
<action dev="henrib" type="add" issue="JEXL-419">
Add permission syntax to allow class/method/field
</action>
diff --git a/src/main/java/org/apache/commons/jexl3/internal/ArrayBuilder.java
b/src/main/java/org/apache/commons/jexl3/internal/ArrayBuilder.java
index a7825b1c..f738df30 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/ArrayBuilder.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/ArrayBuilder.java
@@ -24,6 +24,7 @@ import java.util.List;
import java.util.Map;
import org.apache.commons.jexl3.JexlArithmetic;
+import org.apache.commons.jexl3.internal.introspection.ClassMisc;
/**
* Helper class to create typed arrays.
@@ -67,6 +68,17 @@ public class ArrayBuilder implements
JexlArithmetic.ArrayBuilder {
/** Extended? */
protected final boolean extended;
+ /**
+ * Computes the best super class/super interface.
+ * <p>Used to try and maintain type safe arrays.</p>
+ * @param baseClass the baseClass
+ * @param other another class
+ * @return a common ancestor, class or interface, worst case being class
Object
+ */
+ protected Class<?> getCommonSuperClass(final Class<?> baseClass, final
Class<?> other) {
+ return ClassMisc.getCommonSuperClass(baseClass, other);
+ }
+
/**
* Creates a new builder.
* @param size the exact array size
@@ -97,19 +109,13 @@ public class ArrayBuilder implements
JexlArithmetic.ArrayBuilder {
if (commonClass == null) {
commonClass = eclass;
isNumber = isNumber &&
Number.class.isAssignableFrom(commonClass);
- } else if (!commonClass.equals(eclass)) {
+ } else if (!commonClass.isAssignableFrom(eclass)) {
// if both are numbers...
if (isNumber && Number.class.isAssignableFrom(eclass)) {
commonClass = Number.class;
} else {
- // attempt to find valid superclass
- do {
- eclass = eclass.getSuperclass();
- if (eclass == null) {
- commonClass = Object.class;
- break;
- }
- } while (!commonClass.isAssignableFrom(eclass));
+ isNumber = false;
+ commonClass = getCommonSuperClass(commonClass, eclass);
}
}
}
@@ -120,21 +126,32 @@ public class ArrayBuilder implements
JexlArithmetic.ArrayBuilder {
untyped[added++] = value;
}
+ /**
+ * Creates a new list (aka extended array)/
+ * @param clazz the class
+ * @param size the size
+ * @return the instance
+ * @param <T> the type
+ */
+ protected <T> List<T> newList(Class<? extends T> clazz, int size) {
+ return new ArrayList<>(size);
+ }
+
@Override
public Object create(final boolean e) {
if (untyped == null) {
return new Object[0];
}
+ final int size = added;
if (extended || e) {
- final List<Object> list = new ArrayList<>(added);
- list.addAll(Arrays.asList(untyped).subList(0, added));
+ final List<Object> list = newList(commonClass, size);
+ list.addAll(Arrays.asList(untyped).subList(0, size));
return list;
}
// convert untyped array to the common class if not Object.class
if (commonClass == null || Object.class.equals(commonClass)) {
return untyped.clone();
}
- final int size = added;
// if the commonClass is a number, it has an equivalent primitive
type, get it
if (unboxing) {
commonClass = unboxingClass(commonClass);
diff --git
a/src/main/java/org/apache/commons/jexl3/internal/introspection/ClassMisc.java
b/src/main/java/org/apache/commons/jexl3/internal/introspection/ClassMisc.java
new file mode 100644
index 00000000..438a13d7
--- /dev/null
+++
b/src/main/java/org/apache/commons/jexl3/internal/introspection/ClassMisc.java
@@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.jexl3.internal.introspection;
+
+import java.lang.reflect.Modifier;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.LinkedHashSet;
+import java.util.Set;
+
+/**
+ * Miscellaneous introspection methods.
+ * <p>The main algorithm is computing the actual ordered complete set of
classes and interfaces that a given class
+ * extends or implements. This set order is based on the super-class then
(recursive interface) declaration order,
+ * attempting to reflect the (hopefully intended) abstraction order (from
strong to weak).</p>
+ */
+public class ClassMisc {
+ /**
+ * Lets not instantiate it.
+ */
+ private ClassMisc() {}
+ /**
+ * Collect super classes and interfaces in super-order.
+ * <p>This orders from stronger to weaker abstraction in the sense that
+ * Integer is a stronger abstraction than Number.</p>
+ *
+ * @param superSet the set of super classes to collect into
+ * @param baseClass the root class.
+ */
+ private static void addSuperClasses(final Set<Class<?>> superSet, final
Class<?> baseClass) {
+ for (Class<?> clazz = baseClass.getSuperclass(); clazz != null &&
!Object.class.equals(clazz); clazz = clazz.getSuperclass()) {
+ if (Modifier.isPublic(clazz.getModifiers())) {
+ superSet.add(clazz);
+ }
+ }
+ // recursive visit interfaces in super order
+ for (Class<?> clazz = baseClass; clazz != null &&
!Object.class.equals(clazz); clazz = clazz.getSuperclass()) {
+ addSuperInterfaces(superSet, clazz);
+ }
+ }
+
+ /**
+ * Recursively add super-interfaces in super-order.
+ * <p>On the premise that a class also tends to enumerate interface in the
order of weaker abstraction and
+ * that interfaces follow the same convention (strong implements weak).</p>
+ *
+ * @param superSet the set of super classes to fill
+ * @param clazz the root class.
+ */
+ private static void addSuperInterfaces(final Set<Class<?>> superSet, final
Class<?> clazz) {
+ for (Class<?> inter : clazz.getInterfaces()) {
+ superSet.add(inter);
+ addSuperInterfaces(superSet, inter);
+ }
+ }
+
+ /**
+ * Gets the closest common super-class of two classes.
+ * <p>When building an array, this helps strong-typing the result.</p>
+ *
+ * @param baseClass the class to serve as base
+ * @param other the other class
+ * @return Object.class if nothing in common, the closest common class or
interface otherwise
+ */
+ public static Class<?> getCommonSuperClass(final Class<?> baseClass, final
Class<?> other) {
+ if (baseClass == null || other == null) {
+ return null;
+ }
+ if (baseClass != Object.class && other != Object.class) {
+ final Set<Class<?>> superSet = new LinkedHashSet<>();
+ addSuperClasses(superSet, baseClass);
+ for (Class<?> superClass : superSet) {
+ if (superClass.isAssignableFrom(other)) {
+ return superClass;
+ }
+ }
+ }
+ return Object.class;
+ }
+
+ /**
+ * Build the set of super classes and interfaces common to a collection of
classes.
+ * <p>The returned set is ordered and puts classes in order of super-class
appearance then
+ * interfaces of each super-class.</p>
+ *
+ * @param baseClass the class to serve as base
+ * @param otherClasses the (optional) other classes
+ * @return an empty set if nothing in common, the set of common classes and
interfaces that
+ * does not contain the baseClass nor Object class
+ */
+ public static Set<Class<?>> getSuperClasses(final Class<?> baseClass, final
Class<?>... otherClasses) {
+ if (baseClass == null) {
+ return Collections.emptySet();
+ }
+ final Set<Class<?>> superSet = new LinkedHashSet<>();
+ addSuperClasses(superSet, baseClass);
+ // intersect otherClasses
+ if (otherClasses.length > 0) {
+ for (Class<?> other : otherClasses) {
+ // remove classes from $superSet that $other is not assignable to
+ final Iterator<Class<?>> superClass = superSet.iterator();
+ while (superClass.hasNext()) {
+ if (!superClass.next().isAssignableFrom(other)) {
+ superClass.remove();
+ }
+ }
+ if (superSet.isEmpty()) {
+ return Collections.emptySet();
+ }
+ }
+ }
+ return superSet;
+ }
+}
diff --git a/src/test/java/org/apache/commons/jexl3/ArrayTypeTest.java
b/src/test/java/org/apache/commons/jexl3/ArrayTypeTest.java
new file mode 100644
index 00000000..cebafbcc
--- /dev/null
+++ b/src/test/java/org/apache/commons/jexl3/ArrayTypeTest.java
@@ -0,0 +1,149 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.jexl3;
+
+import org.apache.commons.jexl3.internal.ArrayBuilder;
+import org.apache.commons.jexl3.internal.introspection.ClassMisc;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.io.Serializable;
+import java.util.AbstractCollection;
+import java.util.AbstractList;
+import java.util.ArrayDeque;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+import java.util.Set;
+
+/**
+ * Ensure ArrayBuilder types its output by finding some common ancestor class
or interface (besides Object.class)
+ * from its entries when possible.
+ */
+public class ArrayTypeTest {
+ // A dependency tree with some complexity follows:
+ public interface Inter0 {}
+ public interface InterA {}
+ public interface InterB {}
+ public interface InterC {}
+ public interface InterX extends InterB {}
+ public abstract static class Class0 implements Inter0 {
+ private int value;
+ public Class0(int v) {
+ value = v;
+ }
+ @Override public String toString() {
+ return getClass().getSimpleName() + "{" + value + "}";
+ }
+ }
+ public static class ClassA extends Class0 implements InterA {
+ public ClassA(int v) { super(v); }
+ }
+ public static class ClassX extends Class0 implements InterX {
+ public ClassX(int v) { super(v); }
+ }
+ public static class ClassB extends ClassA implements InterB {
+ public ClassB(int v) { super(v); }
+ }
+ public static class ClassC extends ClassB implements InterC, InterX {
+ public ClassC(int v) { super(v); }
+ }
+ public static class ClassD implements InterB, Inter0 {
+ @Override public String toString() {
+ return getClass().getSimpleName() + "{" +
Integer.toHexString(hashCode()) + "}";
+ }
+ }
+
+ @Test
+ public void testArrayTypes() {
+ ArrayBuilder ab = new ArrayBuilder(1);
+ // An engine for expressions with args
+ JexlFeatures features = JexlFeatures.createScript().script(false);
+ JexlEngine jexl = new JexlBuilder()
+ .features(features)
+ .create();
+ // Super for ClassC
+ Set<Class<?>> superSet = ClassMisc.getSuperClasses(ClassC.class);
+ Assert.assertTrue(superSet.size() > 0);
+ // verify the order
+ List<Class<?>> ordered = Arrays.asList(
+ ClassB.class, ClassA.class, Class0.class,
+ InterC.class, InterX.class, InterB.class,
+ InterA.class, Inter0.class);
+ int i = 0;
+ for(Class<?> clazz : superSet) {
+ Assert.assertEquals("order " + i, clazz, ordered.get(i++));
+ }
+ // intersect ClassC, ClassX -> Class0
+ Class<?> inter = ClassMisc.getCommonSuperClass(ClassC.class, ClassX.class);
+ Assert.assertEquals(Class0.class, inter);
+
+ // intersect ClassC, ClassB -> ClassB
+ inter = ClassMisc.getCommonSuperClass(ClassC.class, ClassB.class);
+ Assert.assertEquals(ClassB.class, inter);
+
+ // intersect ArrayList, ArrayDeque -> AbstractCollection
+ Class<?> list = ClassMisc.getCommonSuperClass(ArrayList.class,
ArrayDeque.class);
+ Assert.assertEquals(list, AbstractCollection.class);
+
+ Set<Class<?>> sset = ClassMisc.getSuperClasses(ArrayList.class,
ArrayDeque.class);
+ Assert.assertFalse(sset.isEmpty());
+ List<Class<?>> expected = Arrays.asList(AbstractCollection.class,
Collection.class, Iterable.class, Cloneable.class, Serializable.class);
+ Assert.assertEquals(expected, new ArrayList(sset));
+ Class<?> collection = ClassMisc.getCommonSuperClass(ArrayList.class,
Collections.emptyList().getClass());
+ Assert.assertEquals(AbstractList.class, collection);
+ collection = ClassMisc.getSuperClasses(ArrayList.class,
Collections.emptyList().getClass())
+ .stream().findFirst().orElse(Object.class);
+
+ // apply on objects
+ Object a = new ClassA(1);
+ Object b = new ClassB(2);
+ Object c = new ClassC(3);
+ Object x = new ClassX(4);
+ JexlScript script;
+ Object result;
+
+ script = jexl.createScript("[ a, b, c, d ]", "a", "b", "c", "d");
+ // intersect a, b, c, c -> classA
+ result = script.execute(null, a, b, c, c);
+ Assert.assertTrue(result.getClass().isArray()
+ && result.getClass().getComponentType().equals(ClassA.class));
+
+ // intersect a, b, c, x -> class0
+ result = script.execute(null, a, b, c, x);
+ Assert.assertTrue(result.getClass().isArray()
+ && result.getClass().getComponentType().equals(Class0.class));
+
+ // intersect x, c, b, a -> class0
+ result = script.execute(null, x, c, b, a);
+ Assert.assertTrue(result.getClass().isArray()
+ & result.getClass().getComponentType().equals(Class0.class));
+
+ // intersect a, b, c, d -> inter0
+ Object d = new ClassD();
+ result = script.execute(null, a, b, c, d);
+ Assert.assertTrue(result.getClass().isArray()
+ && result.getClass().getComponentType().equals(Inter0.class));
+
+ script = jexl.createScript("[ a, b, c, d, ... ]", "a", "b", "c", "d");
+ // intersect a, b, c, c -> classA
+ result = script.execute(null, a, b, c, c);
+ Assert.assertTrue(result instanceof List);
+ }
+}
diff --git a/src/test/java/org/apache/commons/jexl3/Issues400Test.java
b/src/test/java/org/apache/commons/jexl3/Issues400Test.java
index 0a5695bc..d3dee093 100644
--- a/src/test/java/org/apache/commons/jexl3/Issues400Test.java
+++ b/src/test/java/org/apache/commons/jexl3/Issues400Test.java
@@ -22,19 +22,15 @@ import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.Iterator;
-import java.util.LinkedHashSet;
import java.util.Map;
import java.util.Objects;
import java.util.concurrent.atomic.AtomicLong;
-import org.apache.commons.jexl3.internal.introspection.Permissions;
import org.apache.commons.jexl3.introspection.JexlPermissions;
-import org.apache.commons.jexl3.introspection.JexlSandbox;
import org.junit.Assert;
import org.junit.Test;
import static
org.apache.commons.jexl3.introspection.JexlPermissions.RESTRICTED;
-import static org.hamcrest.MatcherAssert.assertThat;
/**
* Test cases for reported issue between JEXL-300 and JEXL-399.
