Skip to content

Commit

Permalink
- Added ReflectionUtils method to get all constructors, reusing exist…
Browse files Browse the repository at this point in the history
…ing construtor cache

- ClassUtilities does not attempt to instantiate abstract classes
- ClassUtilities added MethodHandle (JDK 8) to the no-no list.  VarHandle is Java 9+
  • Loading branch information
jdereg committed Jan 13, 2025
1 parent 292a15a commit d7f1ace
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 20 deletions.
17 changes: 7 additions & 10 deletions src/main/java/com/cedarsoftware/util/ClassUtilities.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import java.io.IOException;
import java.io.InputStream;
import java.io.UncheckedIOException;
import java.lang.invoke.MethodHandle;
import java.lang.reflect.AccessibleObject;
import java.lang.reflect.Array;
import java.lang.reflect.Constructor;
Expand Down Expand Up @@ -1265,14 +1266,14 @@ private static class CachedConstructor {
* }</pre>
*/
public static Object newInstance(Converter converter, Class<?> c, Collection<?> argumentValues) {
if (c == null) {
throw new IllegalArgumentException("Class cannot be null");
}

if (c == null) { throw new IllegalArgumentException("Class cannot be null"); }
if (c.isInterface()) { throw new IllegalArgumentException("Cannot instantiate interface: " + c.getName()); }
if (Modifier.isAbstract(c.getModifiers())) { throw new IllegalArgumentException("Cannot instantiate abstract class: " + c.getName()); }
// Security checks
Set<Class<?>> securityChecks = CollectionUtilities.setOf(
ProcessBuilder.class, Process.class, ClassLoader.class,
Constructor.class, Method.class, Field.class);
Constructor.class, Method.class, Field.class, MethodHandle.class);

for (Class<?> check : securityChecks) {
if (check.isAssignableFrom(c)) {
Expand All @@ -1285,10 +1286,6 @@ public static Object newInstance(Converter converter, Class<?> c, Collection<?>
throw new IllegalArgumentException("For security reasons, json-io does not allow instantiation of: java.lang.ProcessImpl");
}

if (c.isInterface()) {
throw new IllegalArgumentException("Cannot instantiate interface: " + c.getName());
}

// Normalize arguments
List<Object> normalizedArgs = argumentValues == null ? new ArrayList<>() : new ArrayList<>(argumentValues);

Expand Down Expand Up @@ -1319,7 +1316,7 @@ private static Object tryConstructorInstantiation(CachedConstructor cached, List
}

private static Object tryNewConstructors(Class<?> c, List<Object> args, Converter converter, String cacheKey) {
Constructor<?>[] declaredConstructors = c.getDeclaredConstructors();
Constructor<?>[] declaredConstructors = ReflectionUtils.getAllConstructors(c);
Set<ConstructorWithValues> constructorOrder = new TreeSet<>();

// Prepare all constructors with their argument matches
Expand Down
49 changes: 46 additions & 3 deletions src/main/java/com/cedarsoftware/util/ReflectionUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
* limitations under the License.
*/
public final class ReflectionUtils {
private static final int CACHE_SIZE = 1000;
private static final int CACHE_SIZE = 1500;

private static volatile Map<ConstructorCacheKey, Constructor<?>> CONSTRUCTOR_CACHE = new LRUCache<>(CACHE_SIZE);
private static volatile Map<MethodCacheKey, Method> METHOD_CACHE = new LRUCache<>(CACHE_SIZE);
Expand Down Expand Up @@ -1197,7 +1197,7 @@ public static Method getMethod(Object instance, String methodName, int argCount)
if (!selected.isAccessible()) {
try {
selected.setAccessible(true);
} catch (SecurityException ignored) {
} catch (Exception ignored) {
// Return the method even if we can't make it accessible
}
}
Expand Down Expand Up @@ -1290,7 +1290,7 @@ public static Constructor<?> getConstructor(Class<?> clazz, Class<?>... paramete
if (!Modifier.isPublic(found.getModifiers())) {
try {
found.setAccessible(true);
} catch (SecurityException ignored) {
} catch (Exception ignored) {
// Return the constructor even if we can't make it accessible
}
}
Expand All @@ -1302,6 +1302,49 @@ public static Constructor<?> getConstructor(Class<?> clazz, Class<?>... paramete
CONSTRUCTOR_CACHE.put(key, found);
return found;
}

/**
* Returns all declared constructors for the given class, storing each one in
* the existing CONSTRUCTOR_CACHE (keyed by (classLoader + className + paramTypes)).
* <p>
* If the constructor is not yet in the cache, we setAccessible(true) when possible
* and store it. Subsequent calls will retrieve the same Constructor from the cache.
*
* @param clazz The class whose constructors we want.
* @return An array of all declared constructors for that class.
*/
public static Constructor<?>[] getAllConstructors(Class<?> clazz) {
if (clazz == null) {
return new Constructor<?>[0];
}
// Reflectively find them all
Constructor<?>[] declared = clazz.getDeclaredConstructors();
if (declared.length == 0) {
return declared; // no constructors
}

// For each constructor, see if it’s in CONSTRUCTOR_CACHE.
// If not, cache it (and setAccessible if possible).
for (Constructor<?> ctor : declared) {
Class<?>[] paramTypes = ctor.getParameterTypes();
ConstructorCacheKey key = new ConstructorCacheKey(clazz, paramTypes);
Constructor<?> cached = CONSTRUCTOR_CACHE.get(key);

if (cached == null && !CONSTRUCTOR_CACHE.containsKey(key)) {
// Not yet cached, set it accessible and cache it
try {
ctor.setAccessible(true);
} catch (Exception ignored) {
// Even if we cannot set it accessible, we still cache it
}
CONSTRUCTOR_CACHE.put(key, ctor);
}
}

// Now, we either found them or just cached them.
// But we still return a fresh array so the caller sees *all* of them.
return declared;
}

private static String makeParamKey(Class<?>... parameterTypes) {
if (parameterTypes == null || parameterTypes.length == 0) {
Expand Down
16 changes: 9 additions & 7 deletions src/test/java/com/cedarsoftware/util/ClassUtilitiesTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
Expand Down Expand Up @@ -114,7 +115,7 @@ void setUp() {
void shouldCreateInstanceWithNoArgConstructor() {
Object instance = ClassUtilities.newInstance(converter, NoArgConstructor.class, null);
assertNotNull(instance);
assertTrue(instance instanceof NoArgConstructor);
assertInstanceOf(NoArgConstructor.class, instance);
}

@Test
Expand All @@ -124,7 +125,7 @@ void shouldCreateInstanceWithSingleArgument() {
Object instance = ClassUtilities.newInstance(converter, SingleArgConstructor.class, args);

assertNotNull(instance);
assertTrue(instance instanceof SingleArgConstructor);
assertInstanceOf(SingleArgConstructor.class, instance);
assertEquals("test", ((SingleArgConstructor) instance).getValue());
}

Expand All @@ -135,7 +136,7 @@ void shouldCreateInstanceWithMultipleArguments() {
Object instance = ClassUtilities.newInstance(converter, MultiArgConstructor.class, args);

assertNotNull(instance);
assertTrue(instance instanceof MultiArgConstructor);
assertInstanceOf(MultiArgConstructor.class, instance);
MultiArgConstructor mac = (MultiArgConstructor) instance;
assertEquals("test", mac.getStr());
assertEquals(42, mac.getNum());
Expand All @@ -148,7 +149,7 @@ void shouldHandlePrivateConstructors() {
Object instance = ClassUtilities.newInstance(converter, PrivateConstructor.class, args);

assertNotNull(instance);
assertTrue(instance instanceof PrivateConstructor);
assertInstanceOf(PrivateConstructor.class, instance);
assertEquals("private", ((PrivateConstructor) instance).getValue());
}

Expand All @@ -158,7 +159,7 @@ void shouldHandlePrimitiveParametersWithNullArguments() {
Object instance = ClassUtilities.newInstance(converter, PrimitiveConstructor.class, null);

assertNotNull(instance);
assertTrue(instance instanceof PrimitiveConstructor);
assertInstanceOf(PrimitiveConstructor.class, instance);
PrimitiveConstructor pc = (PrimitiveConstructor) instance;
assertEquals(0, pc.getIntValue()); // default int value
assertFalse(pc.getBoolValue()); // default boolean value
Expand All @@ -171,7 +172,7 @@ void shouldChooseBestMatchingConstructor() {
Object instance = ClassUtilities.newInstance(converter, OverloadedConstructors.class, args);

assertNotNull(instance);
assertTrue(instance instanceof OverloadedConstructors);
assertInstanceOf(OverloadedConstructors.class, instance);
OverloadedConstructors oc = (OverloadedConstructors) instance;
assertEquals("custom", oc.getValue());
assertEquals(42, oc.getNumber());
Expand All @@ -194,7 +195,8 @@ void shouldThrowExceptionForSecuritySensitiveClasses() {
IllegalArgumentException.class,
() -> ClassUtilities.newInstance(converter, sensitiveClass, null)
);
assertTrue(exception.getMessage().contains("security reasons"));
assertTrue(exception.getMessage().contains("not"));
assertInstanceOf(IllegalArgumentException.class, exception);
}
}

Expand Down

0 comments on commit d7f1ace

Please sign in to comment.