From 5a2f5cb9698ec966578d2c65041b5392e3d68cd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9da=20Housni=20Alaoui?= Date: Wed, 6 Dec 2023 15:25:58 +0100 Subject: [PATCH] Add an option to JdkProxySource allowing to unwrap UndeclaredThrowableException --- .../commons/pool3/proxy/BaseProxyHandler.java | 23 +++-- .../pool3/proxy/CglibProxyHandler.java | 14 +-- .../commons/pool3/proxy/CglibProxySource.java | 17 +++- .../commons/pool3/proxy/JdkProxyHandler.java | 14 +-- .../commons/pool3/proxy/JdkProxySource.java | 18 +++- .../proxy/AbstractTestProxiedObjectPool.java | 97 ++++++++++++++++--- .../TestProxiedObjectPoolWithCglibProxy.java | 4 +- .../TestProxiedObjectPoolWithJdkProxy.java | 4 +- 8 files changed, 153 insertions(+), 38 deletions(-) diff --git a/src/main/java/org/apache/commons/pool3/proxy/BaseProxyHandler.java b/src/main/java/org/apache/commons/pool3/proxy/BaseProxyHandler.java index 8af3d809f..65e2f62a2 100644 --- a/src/main/java/org/apache/commons/pool3/proxy/BaseProxyHandler.java +++ b/src/main/java/org/apache/commons/pool3/proxy/BaseProxyHandler.java @@ -16,6 +16,7 @@ */ package org.apache.commons.pool3.proxy; +import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import org.apache.commons.pool3.UsageTracking; @@ -32,18 +33,21 @@ class BaseProxyHandler { private volatile T pooledObject; private final UsageTracking usageTracking; + private final boolean unwrapInvocationTargetException; /** * Constructs a new wrapper for the given pooled object. * - * @param pooledObject The object to wrap - * @param usageTracking The instance, if any (usually the object pool) to - * be provided with usage tracking information for this - * wrapped object + * @param pooledObject The object to wrap + * @param usageTracking The instance, if any (usually the object pool) to + * be provided with usage tracking information for this + * wrapped object + * @param unwrapInvocationTargetException True to make the proxy throw {@link InvocationTargetException#getTargetException()} instead of {@link InvocationTargetException} */ - BaseProxyHandler(final T pooledObject, final UsageTracking usageTracking) { + BaseProxyHandler(final T pooledObject, final UsageTracking usageTracking, boolean unwrapInvocationTargetException) { this.pooledObject = pooledObject; this.usageTracking = usageTracking; + this.unwrapInvocationTargetException = unwrapInvocationTargetException; } /** @@ -73,7 +77,14 @@ Object doInvoke(final Method method, final Object[] args) throws Throwable { if (usageTracking != null) { usageTracking.use(object); } - return method.invoke(object, args); + try { + return method.invoke(object, args); + } catch (InvocationTargetException e) { + if (unwrapInvocationTargetException) { + throw e.getTargetException(); + } + throw e; + } } /** diff --git a/src/main/java/org/apache/commons/pool3/proxy/CglibProxyHandler.java b/src/main/java/org/apache/commons/pool3/proxy/CglibProxyHandler.java index 635a6a7c5..f4443a3d1 100644 --- a/src/main/java/org/apache/commons/pool3/proxy/CglibProxyHandler.java +++ b/src/main/java/org/apache/commons/pool3/proxy/CglibProxyHandler.java @@ -16,6 +16,7 @@ */ package org.apache.commons.pool3.proxy; +import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import org.apache.commons.pool3.UsageTracking; @@ -36,13 +37,14 @@ final class CglibProxyHandler extends BaseProxyHandler /** * Constructs a CGLib proxy instance. * - * @param pooledObject The object to wrap - * @param usageTracking The instance, if any (usually the object pool) to - * be provided with usage tracking information for this - * wrapped object + * @param pooledObject The object to wrap + * @param usageTracking The instance, if any (usually the object pool) to + * be provided with usage tracking information for this + * wrapped object + * @param unwrapInvocationTargetException True to make the proxy throw {@link InvocationTargetException#getTargetException()} instead of {@link InvocationTargetException} */ - CglibProxyHandler(final T pooledObject, final UsageTracking usageTracking) { - super(pooledObject, usageTracking); + CglibProxyHandler(final T pooledObject, final UsageTracking usageTracking, boolean unwrapInvocationTargetException) { + super(pooledObject, usageTracking, unwrapInvocationTargetException); } @Override diff --git a/src/main/java/org/apache/commons/pool3/proxy/CglibProxySource.java b/src/main/java/org/apache/commons/pool3/proxy/CglibProxySource.java index bff89b1c1..cd17b8bb8 100644 --- a/src/main/java/org/apache/commons/pool3/proxy/CglibProxySource.java +++ b/src/main/java/org/apache/commons/pool3/proxy/CglibProxySource.java @@ -16,6 +16,7 @@ */ package org.apache.commons.pool3.proxy; +import java.lang.reflect.InvocationTargetException; import org.apache.commons.pool3.UsageTracking; import net.sf.cglib.proxy.Enhancer; @@ -31,6 +32,18 @@ public class CglibProxySource implements ProxySource { private final Class superclass; + private final boolean unwrapInvocationTargetException; + + /** + * Constructs a new proxy source for the given class. + * + * @param superclass The class to proxy + * @param unwrapInvocationTargetException True to make the proxy throw {@link InvocationTargetException#getTargetException()} instead of {@link InvocationTargetException} + */ + public CglibProxySource(final Class superclass, boolean unwrapInvocationTargetException) { + this.superclass = superclass; + this.unwrapInvocationTargetException = unwrapInvocationTargetException; + } /** * Constructs a new proxy source for the given class. @@ -38,7 +51,7 @@ public class CglibProxySource implements ProxySource { * @param superclass The class to proxy */ public CglibProxySource(final Class superclass) { - this.superclass = superclass; + this(superclass, false); } @SuppressWarnings("unchecked") // Case to T on return @@ -48,7 +61,7 @@ public T createProxy(final T pooledObject, final UsageTracking usageTracking) enhancer.setSuperclass(superclass); final CglibProxyHandler proxyInterceptor = - new CglibProxyHandler<>(pooledObject, usageTracking); + new CglibProxyHandler<>(pooledObject, usageTracking, unwrapInvocationTargetException); enhancer.setCallback(proxyInterceptor); return (T) enhancer.create(); diff --git a/src/main/java/org/apache/commons/pool3/proxy/JdkProxyHandler.java b/src/main/java/org/apache/commons/pool3/proxy/JdkProxyHandler.java index 58d2d614e..cc8031a4d 100644 --- a/src/main/java/org/apache/commons/pool3/proxy/JdkProxyHandler.java +++ b/src/main/java/org/apache/commons/pool3/proxy/JdkProxyHandler.java @@ -17,6 +17,7 @@ package org.apache.commons.pool3.proxy; import java.lang.reflect.InvocationHandler; +import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import org.apache.commons.pool3.UsageTracking; @@ -34,13 +35,14 @@ final class JdkProxyHandler extends BaseProxyHandler /** * Constructs a Java reflection proxy instance. * - * @param pooledObject The object to wrap - * @param usageTracking The instance, if any (usually the object pool) to - * be provided with usage tracking information for this - * wrapped object + * @param pooledObject The object to wrap + * @param usageTracking The instance, if any (usually the object pool) to + * be provided with usage tracking information for this + * wrapped object + * @param unwrapInvocationTargetException True to make the proxy throw {@link InvocationTargetException#getTargetException()} instead of {@link InvocationTargetException} */ - JdkProxyHandler(final T pooledObject, final UsageTracking usageTracking) { - super(pooledObject, usageTracking); + JdkProxyHandler(final T pooledObject, final UsageTracking usageTracking, boolean unwrapInvocationTargetException) { + super(pooledObject, usageTracking, unwrapInvocationTargetException); } @Override diff --git a/src/main/java/org/apache/commons/pool3/proxy/JdkProxySource.java b/src/main/java/org/apache/commons/pool3/proxy/JdkProxySource.java index 8ff793bf0..454665c60 100644 --- a/src/main/java/org/apache/commons/pool3/proxy/JdkProxySource.java +++ b/src/main/java/org/apache/commons/pool3/proxy/JdkProxySource.java @@ -16,6 +16,7 @@ */ package org.apache.commons.pool3.proxy; +import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Proxy; import java.util.Arrays; @@ -32,24 +33,37 @@ public class JdkProxySource implements ProxySource { private final ClassLoader classLoader; private final Class[] interfaces; + private final boolean unwrapInvocationTargetException; /** * Constructs a new proxy source for the given interfaces. * * @param classLoader The class loader with which to create the proxy * @param interfaces The interfaces to proxy + * @param unwrapInvocationTargetException True to make the proxy throw {@link InvocationTargetException#getTargetException()} instead of {@link InvocationTargetException} */ - public JdkProxySource(final ClassLoader classLoader, final Class[] interfaces) { + public JdkProxySource(final ClassLoader classLoader, final Class[] interfaces, boolean unwrapInvocationTargetException) { this.classLoader = classLoader; // Defensive copy this.interfaces = Arrays.copyOf(interfaces, interfaces.length); + this.unwrapInvocationTargetException = unwrapInvocationTargetException; + } + + /** + * Constructs a new proxy source for the given interfaces. + * + * @param classLoader The class loader with which to create the proxy + * @param interfaces The interfaces to proxy + */ + public JdkProxySource(final ClassLoader classLoader, final Class[] interfaces) { + this(classLoader, interfaces, false); } @SuppressWarnings("unchecked") // Cast to T on return. @Override public T createProxy(final T pooledObject, final UsageTracking usageTracking) { return (T) Proxy.newProxyInstance(classLoader, interfaces, - new JdkProxyHandler<>(pooledObject, usageTracking)); + new JdkProxyHandler<>(pooledObject, usageTracking, unwrapInvocationTargetException)); } @SuppressWarnings("unchecked") diff --git a/src/test/java/org/apache/commons/pool3/proxy/AbstractTestProxiedObjectPool.java b/src/test/java/org/apache/commons/pool3/proxy/AbstractTestProxiedObjectPool.java index c53dbcf2f..ec39b456f 100644 --- a/src/test/java/org/apache/commons/pool3/proxy/AbstractTestProxiedObjectPool.java +++ b/src/test/java/org/apache/commons/pool3/proxy/AbstractTestProxiedObjectPool.java @@ -20,11 +20,13 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; import java.io.PrintWriter; import java.io.StringWriter; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.UndeclaredThrowableException; import java.time.Duration; - import org.apache.commons.pool3.BasePooledObjectFactory; import org.apache.commons.pool3.ObjectPool; import org.apache.commons.pool3.PooledObject; @@ -45,9 +47,15 @@ protected interface TestObject { private static final class TestObjectFactory extends BasePooledObjectFactory { + private final RuntimeException exceptionToThrow; + + private TestObjectFactory(RuntimeException exceptionToThrow) { + this.exceptionToThrow = exceptionToThrow; + } + @Override public TestObject create() { - return new TestObjectImpl(); + return new TestObjectImpl(exceptionToThrow); } @Override public PooledObject wrap(final TestObject value) { @@ -57,15 +65,26 @@ public PooledObject wrap(final TestObject value) { private static final class TestObjectImpl implements TestObject { + private final RuntimeException exceptionToThrow; private String data; + private TestObjectImpl(RuntimeException exceptionToThrow) { + this.exceptionToThrow = exceptionToThrow; + } + @Override public String getData() { + if (exceptionToThrow != null) { + throw exceptionToThrow; + } return data; } @Override public void setData(final String data) { + if (exceptionToThrow != null) { + throw exceptionToThrow; + } this.data = data; } } @@ -73,16 +92,15 @@ public void setData(final String data) { private static final Duration ABANDONED_TIMEOUT_SECS = Duration.ofSeconds(3); - private ObjectPool pool; - private StringWriter log; - protected abstract ProxySource getproxySource(); + protected abstract ProxySource getproxySource(boolean unwrapInvocationTargetException); - @BeforeEach - public void setUp() { - log = new StringWriter(); + private ProxiedObjectPool createProxiedObjectPool() { + return createProxiedObjectPool(false, null); + } + private ProxiedObjectPool createProxiedObjectPool(boolean unwrapInvocationTargetException, RuntimeException exceptionToThrow) { final PrintWriter pw = new PrintWriter(log); final AbandonedConfig abandonedConfig = new AbandonedConfig(); abandonedConfig.setLogAbandoned(true); @@ -94,16 +112,23 @@ public void setUp() { final GenericObjectPoolConfig config = new GenericObjectPoolConfig<>(); config.setMaxTotal(3); - final PooledObjectFactory factory = new TestObjectFactory(); + final PooledObjectFactory factory = new TestObjectFactory(exceptionToThrow); - @SuppressWarnings("resource") - final ObjectPool innerPool = new GenericObjectPool<>(factory, config, abandonedConfig); + ObjectPool innerPool = new GenericObjectPool<>(factory, config, abandonedConfig); - pool = new ProxiedObjectPool<>(innerPool, getproxySource()); + return new ProxiedObjectPool<>(innerPool, getproxySource(unwrapInvocationTargetException)); + } + + @BeforeEach + public void setUp() { + log = new StringWriter(); } @Test public void testAccessAfterInvalidate() { + @SuppressWarnings("resource") + ObjectPool pool = createProxiedObjectPool(); + final TestObject obj = pool.borrowObject(); assertNotNull(obj); @@ -122,6 +147,9 @@ public void testAccessAfterInvalidate() { @Test public void testAccessAfterReturn() { + @SuppressWarnings("resource") + ObjectPool pool = createProxiedObjectPool(); + final TestObject obj = pool.borrowObject(); assertNotNull(obj); @@ -139,6 +167,9 @@ public void testAccessAfterReturn() { @Test public void testBorrowObject() { + @SuppressWarnings("resource") + ObjectPool pool = createProxiedObjectPool(); + final TestObject obj = pool.borrowObject(); assertNotNull(obj); @@ -151,6 +182,9 @@ public void testBorrowObject() { @Test public void testPassThroughMethods01() { + @SuppressWarnings("resource") + ObjectPool pool = createProxiedObjectPool(); + assertEquals(0, pool.getNumActive()); assertEquals(0, pool.getNumIdle()); @@ -167,6 +201,8 @@ public void testPassThroughMethods01() { @Test public void testPassThroughMethods02() { + ObjectPool pool = createProxiedObjectPool(); + pool.close(); assertThrows(IllegalStateException.class, @@ -175,6 +211,9 @@ public void testPassThroughMethods02() { @Test public void testUsageTracking() throws InterruptedException { + @SuppressWarnings("resource") + ObjectPool pool = createProxiedObjectPool(); + final TestObject obj = pool.borrowObject(); assertNotNull(obj); @@ -193,4 +232,38 @@ public void testUsageTracking() throws InterruptedException { assertTrue(logOutput.contains("The last code to use this object was")); } + @Test + public void testUnwrapInvocationTargetExceptionTrue() { + @SuppressWarnings("resource") + ObjectPool pool = createProxiedObjectPool(true, new MyException()); + + TestObject object = pool.borrowObject(); + try { + object.getData(); + fail("Expected to throw %s".formatted(MyException.class)); + } catch (MyException e) { + // As expected + } + } + + @Test + public void testUnwrapInvocationTargetExceptionFalse() { + @SuppressWarnings("resource") + ObjectPool pool = createProxiedObjectPool(false, new MyException()); + + TestObject object = pool.borrowObject(); + Exception caughtException = null; + try { + object.getData(); + } catch (Exception e) { + caughtException = e; + } + + if (caughtException instanceof UndeclaredThrowableException || caughtException instanceof InvocationTargetException) { + return; + } + fail("Expected to catch %s or %s but got %s instead".formatted(UndeclaredThrowableException.class, InvocationTargetException.class, caughtException)); + } + + private static class MyException extends RuntimeException {} } diff --git a/src/test/java/org/apache/commons/pool3/proxy/TestProxiedObjectPoolWithCglibProxy.java b/src/test/java/org/apache/commons/pool3/proxy/TestProxiedObjectPoolWithCglibProxy.java index 5eb09c12c..69610a349 100644 --- a/src/test/java/org/apache/commons/pool3/proxy/TestProxiedObjectPoolWithCglibProxy.java +++ b/src/test/java/org/apache/commons/pool3/proxy/TestProxiedObjectPoolWithCglibProxy.java @@ -20,7 +20,7 @@ public class TestProxiedObjectPoolWithCglibProxy extends AbstractTestProxiedObjectPool { @Override - protected ProxySource getproxySource() { - return new CglibProxySource<>(TestObject.class); + protected ProxySource getproxySource(boolean unwrapInvocationTargetException) { + return new CglibProxySource<>(TestObject.class, unwrapInvocationTargetException); } } diff --git a/src/test/java/org/apache/commons/pool3/proxy/TestProxiedObjectPoolWithJdkProxy.java b/src/test/java/org/apache/commons/pool3/proxy/TestProxiedObjectPoolWithJdkProxy.java index c7470bcc5..1887a3e38 100644 --- a/src/test/java/org/apache/commons/pool3/proxy/TestProxiedObjectPoolWithJdkProxy.java +++ b/src/test/java/org/apache/commons/pool3/proxy/TestProxiedObjectPoolWithJdkProxy.java @@ -20,8 +20,8 @@ public class TestProxiedObjectPoolWithJdkProxy extends AbstractTestProxiedObjectPool { @Override - protected ProxySource getproxySource() { + protected ProxySource getproxySource(boolean unwrapInvocationTargetException) { return new JdkProxySource<>(this.getClass().getClassLoader(), - new Class[] { TestObject.class }); + new Class[] { TestObject.class }, unwrapInvocationTargetException); } }