This is an automated email from the ASF dual-hosted git repository. henrib pushed a commit to branch JEXL-472 in repository https://gitbox.apache.org/repos/asf/commons-jexl.git
commit ded79221a3a9dfb4090c22aae3c8ea68cc621d92 Author: Henrib <[email protected]> AuthorDate: Mon Sep 14 14:47:43 2026 +0200 [JEXL-472] Consolidate record reflection into ClassTool, fix test fixture Move the Class#isRecord()/getRecordComponents() reflection out of RecordGetExecutor and into the existing ClassTool backport utility, resolved via MethodHandle to match how it already backports Java 9+ module reflection, instead of duplicating a separate Method-based lookup. Fix RecordPropertyAccessTest's on-the-fly compiled record fixture to actually compile into org.apache.commons.jexl3 (it previously compiled with no package, then looked itself up under that package, which does not resolve). Also switch the test to extend JexlTestCase and use the shared restricted-permissions engine instead of a bespoke UNRESTRICTED-permissions one, matching the rest of the suite. Builds on the record accessor support originally proposed by Aurelien Mino in #415. Co-Authored-By: Claude Code <[email protected]> --- .../jexl3/internal/introspection/ClassTool.java | 84 +++++++++++++++++-- .../internal/introspection/RecordGetExecutor.java | 97 ++-------------------- .../commons/jexl3/RecordPropertyAccessTest.java | 30 +++---- 3 files changed, 98 insertions(+), 113 deletions(-) diff --git a/src/main/java/org/apache/commons/jexl3/internal/introspection/ClassTool.java b/src/main/java/org/apache/commons/jexl3/internal/introspection/ClassTool.java index 5b1da5c6..7a4fce0a 100644 --- a/src/main/java/org/apache/commons/jexl3/internal/introspection/ClassTool.java +++ b/src/main/java/org/apache/commons/jexl3/internal/introspection/ClassTool.java @@ -21,30 +21,60 @@ import java.lang.invoke.MethodHandles; import java.lang.invoke.MethodType; /** - * Utility for Java9+ backport in Java8 of class and module related methods. + * Utility for Java9+ backport in Java8 of class and module related methods, and Java16+ backport of + * record introspection ({@code Class#isRecord()}, {@code Class#getRecordComponents()}). */ final class ClassTool { - /** The Class.getModule() method. */ + /** {@code Class#isRecord()}; null on a pre Java-16 runtime. */ + private static final MethodHandle IS_RECORD; + + /** {@code Class#getRecordComponents()}; null on a pre Java-16 runtime. */ + private static final MethodHandle GET_RECORD_COMPONENTS; + + /** {@code java.lang.reflect.RecordComponent#getName()}; null on a pre Java-16 runtime. */ + private static final MethodHandle RECORD_COMPONENT_GET_NAME; + + /** {@code Class#getModule()}; null on a pre Java-9 runtime. */ private static final MethodHandle GET_MODULE; - /** The Class.getPackageName() method. */ + /** {@code Class#getPackageName()}; null on a pre Java-9 runtime. */ private static final MethodHandle GET_PKGNAME; - /** The Module.isExported(String packageName) method. */ + /** {@code Module#isExported(String, Module)}; null on a pre Java-9 runtime. */ private static final MethodHandle IS_EXPORTED; - /** The Module of JEXL itself. */ + /** The {@code java.lang.Module} that declares this class; null on a pre Java-9 runtime. */ private static final Object JEXL_MODULE; static { final MethodHandles.Lookup LOOKUP = MethodHandles.lookup(); + final ClassLoader loader = ClassTool.class.getClassLoader(); + + // Java 16+ record introspection backport + MethodHandle isRecord = null; + MethodHandle getRecordComponents = null; + MethodHandle recordComponentGetName = null; + try { + final Class<?> componentc = loader.loadClass("java.lang.reflect.RecordComponent"); + final Class<?> componentArrayc = java.lang.reflect.Array.newInstance(componentc, 0).getClass(); + isRecord = LOOKUP.findVirtual(Class.class, "isRecord", MethodType.methodType(boolean.class)); + getRecordComponents = LOOKUP.findVirtual(Class.class, "getRecordComponents", MethodType.methodType(componentArrayc)); + recordComponentGetName = LOOKUP.findVirtual(componentc, "getName", MethodType.methodType(String.class)); + } catch (final Throwable xnotfound) { + // ignore all; records unsupported on this runtime + } + IS_RECORD = isRecord; + GET_RECORD_COMPONENTS = getRecordComponents; + RECORD_COMPONENT_GET_NAME = recordComponentGetName; + + // Java 9+ module reflection backport MethodHandle getModule = null; MethodHandle getPackageName = null; MethodHandle isExported = null; Object myModule = null; try { - final Class<?> modulec = ClassTool.class.getClassLoader().loadClass("java.lang.Module"); + final Class<?> modulec = loader.loadClass("java.lang.Module"); if (modulec != null) { getModule = LOOKUP.findVirtual(Class.class, "getModule", MethodType.methodType(modulec)); if (getModule != null) { @@ -64,6 +94,46 @@ final class ClassTool { IS_EXPORTED = isExported; } + /** + * Whether the given class is a record on this runtime. + * + * @param clazz the class to check + * @return true if clazz is a record, false if it is not or if records are unsupported here + */ + static boolean isRecord(final Class<?> clazz) { + try { + return IS_RECORD != null && (boolean) IS_RECORD.invoke(clazz); + } catch (final Throwable xfail) { + return false; + } + } + + /** + * Whether the given class declares a record component named {@code property}. + * <p>Used only to confirm {@code property} is a genuine record component before the accessor is + * resolved through the (permission-checked) {@link Introspector}; the accessor method instances + * gathered here are discarded.</p> + * + * @param clazz the record class + * @param property the property name to match against the record's components + * @return true if clazz declares a record component named property + */ + static boolean hasRecordComponent(final Class<?> clazz, final String property) { + if (GET_RECORD_COMPONENTS != null && RECORD_COMPONENT_GET_NAME != null) { + try { + final Object[] components = (Object[]) GET_RECORD_COMPONENTS.invoke(clazz); + for (final Object component : components) { + if (property.equals(RECORD_COMPONENT_GET_NAME.invoke(component))) { + return true; + } + } + } catch (final Throwable xfail) { + // ignore and fall through to return false + } + } + return false; + } + /** * Gets the package name of a class (class.getPackage() may return null). * @@ -116,7 +186,7 @@ final class ClassTool { * The code performs the following sequence through reflection (since the same jar can run * on a Java8 or Java9+ runtime and the module features does not exist on 8). * {@code - * Module jexlModule ClassTool.getClass().getModule(); + * Module jexlModule = ClassTool.class.getModule(); * Module module = declarator.getModule(); * return module.isExported(declarator.getPackageName(), jexlModule); * } diff --git a/src/main/java/org/apache/commons/jexl3/internal/introspection/RecordGetExecutor.java b/src/main/java/org/apache/commons/jexl3/internal/introspection/RecordGetExecutor.java index 8dd84d28..88a207fc 100644 --- a/src/main/java/org/apache/commons/jexl3/internal/introspection/RecordGetExecutor.java +++ b/src/main/java/org/apache/commons/jexl3/internal/introspection/RecordGetExecutor.java @@ -25,104 +25,18 @@ import org.apache.commons.jexl3.JexlException; * <p>A record (JEP 395, Java 16+) exposes one accessor per component, named exactly like the * component - {@code x()}, not {@code getX()}. {@link PropertyGetExecutor} only looks for the bean * convention, so a record component was otherwise never resolved as a property.</p> - * <p>Record detection and lookup are done entirely through reflection so this class - like the rest of - * this module - remains usable on the Java 8 baseline this project still targets; on such a runtime, - * {@code Class#isRecord()} and {@code Class#getRecordComponents()} simply do not exist and discovery - * quietly reports no match instead of failing to link.</p> + * <p>Record detection and lookup are delegated to {@link ClassTool}, which resolves the relevant + * methods through reflection so this module remains usable on the Java 8 baseline this project still + * targets; on such a runtime, {@code Class#isRecord()} and {@code Class#getRecordComponents()} simply + * do not exist and discovery quietly reports no match instead of failing to link.</p> * * @since 3.7.2 */ public final class RecordGetExecutor extends AbstractExecutor.Get { - /** {@code Class#isRecord()}, resolved once; null on a pre Java-16 runtime. */ - private static final java.lang.reflect.Method IS_RECORD = findNoArgMethod(Class.class, "isRecord"); - - /** {@code Class#getRecordComponents()}, resolved once; null on a pre Java-16 runtime. */ - private static final java.lang.reflect.Method GET_RECORD_COMPONENTS = findNoArgMethod( - Class.class, "getRecordComponents" - ); - - /** {@code java.lang.reflect.RecordComponent#getName()}, resolved once; null on a pre Java-16 runtime. */ - private static final java.lang.reflect.Method COMPONENT_GET_NAME = findComponentMethod("getName"); - /** A static signature for method(). */ private static final Object[] EMPTY_PARAMS = {}; - /** - * Looks up a public no-argument method by name, tolerating its absence. - * - * @param onClass the class to look the method up on - * @param name the method name - * @return the method, or null if it does not exist on this runtime - */ - private static java.lang.reflect.Method findNoArgMethod(final Class<?> onClass, final String name) { - try { - return onClass.getMethod(name); - } catch (final NoSuchMethodException xnotfound) { - return null; - } - } - - /** - * Looks up a public no-argument method on {@code java.lang.reflect.RecordComponent}, tolerating the - * type itself being absent on this runtime. - * - * @param name the method name - * @return the method, or null if unavailable on this runtime - */ - private static java.lang.reflect.Method findComponentMethod(final String name) { - try { - final Class<?> recordComponent = Class.forName("java.lang.reflect.RecordComponent"); - return recordComponent.getMethod(name); - } catch (final ReflectiveOperationException xnotfound) { - return null; - } - } - - /** - * Whether the given class is a record on this runtime. - * - * @param clazz the class to check - * @return true if clazz is a record, false if it is not or if records are unsupported here - */ - private static boolean isRecord(final Class<?> clazz) { - if (IS_RECORD == null) { - return false; - } - try { - return Boolean.TRUE.equals(IS_RECORD.invoke(clazz)); - } catch (final ReflectiveOperationException xfail) { - return false; - } - } - - /** - * Whether the given class declares a record component named {@code property}. - * <p>Used only to confirm {@code property} is a genuine record component before the accessor is - * resolved through the (permission-checked) {@link Introspector}; the accessor method instances - * gathered here are discarded.</p> - * - * @param clazz the record class - * @param property the property name to match against the record's components - * @return true if clazz declares a record component named property - */ - private static boolean hasComponent(final Class<?> clazz, final String property) { - if (GET_RECORD_COMPONENTS == null || COMPONENT_GET_NAME == null) { - return false; - } - try { - final Object[] components = (Object[]) GET_RECORD_COMPONENTS.invoke(clazz); - for (final Object component : components) { - if (property.equals(COMPONENT_GET_NAME.invoke(component))) { - return true; - } - } - } catch (final ReflectiveOperationException xfail) { - return false; - } - return false; - } - /** * Discovers a RecordGetExecutor. * <p>The class must be a record and declare a component named {@code property}; the accessor @@ -135,7 +49,8 @@ public final class RecordGetExecutor extends AbstractExecutor.Get { * @return the executor if found, null otherwise */ public static RecordGetExecutor discover(final Introspector is, final Class<?> clazz, final String property) { - if (property == null || property.isEmpty() || !isRecord(clazz) || !hasComponent(clazz, property)) { + if (property == null || property.isEmpty() || !ClassTool.isRecord(clazz) + || !ClassTool.hasRecordComponent(clazz, property)) { return null; } final java.lang.reflect.Method method = is.getMethod(clazz, property, EMPTY_PARAMS); diff --git a/src/test/java/org/apache/commons/jexl3/RecordPropertyAccessTest.java b/src/test/java/org/apache/commons/jexl3/RecordPropertyAccessTest.java index 334ffec3..7fb834fb 100644 --- a/src/test/java/org/apache/commons/jexl3/RecordPropertyAccessTest.java +++ b/src/test/java/org/apache/commons/jexl3/RecordPropertyAccessTest.java @@ -31,7 +31,6 @@ import java.util.Map; import javax.tools.JavaCompiler; import javax.tools.ToolProvider; -import org.apache.commons.jexl3.introspection.JexlPermissions; import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.Test; @@ -41,7 +40,11 @@ import org.junit.jupiter.api.Test; * does not exist, so the test record used here cannot be a plain source declaration in this file: it is * compiled on the fly, and the test itself is skipped on a pre Java-16 runtime.</p> */ -class RecordPropertyAccessTest { +public class RecordPropertyAccessTest extends JexlTestCase { + + public RecordPropertyAccessTest() { + super("RecordPropertyAccessTest"); + } private static int featureVersion() { final String spec = System.getProperty("java.specification.version"); @@ -55,28 +58,26 @@ class RecordPropertyAccessTest { final Path dir = Files.createTempDirectory("jexl-record-test"); final Path javaFile = dir.resolve(name + ".java"); Files.write(javaFile, source.getBytes(StandardCharsets.UTF_8)); - final int rc = compiler.run(null, null, null, javaFile.toString()); + final int rc = compiler.run(null, null, null, "-d", dir.toString(), javaFile.toString()); assertEquals(0, rc, "failed to compile test record"); try (URLClassLoader loader = new URLClassLoader(new URL[] {dir.toUri().toURL()})) { - return Class.forName(name, true, loader); + return Class.forName("org.apache.commons.jexl3." + name, true, loader); } } @Test void testRecordComponentIsReadableAsProperty() throws Exception { Assumptions.assumeTrue(featureVersion() >= 16, "records require Java 16+"); - final Class<?> pointClass = compileRecord( - "JexlRecordPoint", "public record JexlRecordPoint(int x, int y) {}" + final Class<?> pointClass = compileRecord("JexlRecordPoint", + "package org.apache.commons.jexl3; public record JexlRecordPoint(int x, int y) {}" ); final Object point = pointClass.getConstructor(int.class, int.class).newInstance(1, 2); - - final JexlEngine jexl = new JexlBuilder().permissions(JexlPermissions.UNRESTRICTED).create(); final Map<String, Object> vars = new HashMap<>(); vars.put("point", point); final JexlContext ctx = new MapContext(vars); - assertEquals(1, jexl.createExpression("point.x").evaluate(ctx)); - assertEquals(2, jexl.createExpression("point.y").evaluate(ctx)); + assertEquals(1, JEXL.createExpression("point.x").evaluate(ctx)); + assertEquals(2, JEXL.createExpression("point.y").evaluate(ctx)); } @Test @@ -86,17 +87,16 @@ class RecordPropertyAccessTest { // ordinary getFoo() convention first, RecordGetExecutor only fills the gap otherwise left open final Class<?> pointClass = compileRecord( "JexlRecordNamedPoint", - "public record JexlRecordNamedPoint(String name) { " + "package org.apache.commons.jexl3; " + + "public record JexlRecordNamedPoint(String name) { " + "public String getName() { return name() + \"!\"; } }" ); final Object point = pointClass.getConstructor(String.class).newInstance("origin"); - - final JexlEngine jexl = new JexlBuilder().permissions(JexlPermissions.UNRESTRICTED).create(); final Map<String, Object> vars = new HashMap<>(); vars.put("point", point); final JexlContext ctx = new MapContext(vars); - assertNotNull(jexl.createExpression("point.name").evaluate(ctx)); - assertEquals("origin!", jexl.createExpression("point.name").evaluate(ctx)); + assertNotNull(JEXL.createExpression("point.name").evaluate(ctx)); + assertEquals("origin!", JEXL.createExpression("point.name").evaluate(ctx)); } }
