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 3d655ef495..64ae428313 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; @@ -31,18 +32,22 @@ 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; } /** @@ -72,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 ff3db18d1a..b269aed4b9 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; @@ -37,13 +38,15 @@ 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 ca52c23b0b..5c1d997994 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,8 @@ */ package org.apache.commons.pool3.proxy; +import java.lang.reflect.InvocationTargetException; + import org.apache.commons.pool3.UsageTracking; import net.sf.cglib.proxy.Enhancer; @@ -32,6 +34,19 @@ 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. @@ -39,7 +54,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 @@ -49,7 +64,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 662bbe9555..786d5687db 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; @@ -33,13 +34,15 @@ 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 46edc5182a..d621e52e47 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; @@ -31,24 +32,38 @@ 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 c53dbcf2f8..f584e53f51 100644 --- a/src/test/java/org/apache/commons/pool3/proxy/AbstractTestProxiedObjectPool.java +++ b/src/test/java/org/apache/commons/pool3/proxy/AbstractTestProxiedObjectPool.java @@ -45,9 +45,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 +63,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 +90,16 @@ 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 +111,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); - pool = new ProxiedObjectPool<>(innerPool, getproxySource()); + return new ProxiedObjectPool<>(innerPool, getproxySource(unwrapInvocationTargetException)); + } + + @BeforeEach + public void setUp() { + log = new StringWriter(); } @Test public void testAccessAfterInvalidate() { + @SuppressWarnings("resource") + final ObjectPool pool = createProxiedObjectPool(); + final TestObject obj = pool.borrowObject(); assertNotNull(obj); @@ -122,6 +146,9 @@ public void testAccessAfterInvalidate() { @Test public void testAccessAfterReturn() { + @SuppressWarnings("resource") + final ObjectPool pool = createProxiedObjectPool(); + final TestObject obj = pool.borrowObject(); assertNotNull(obj); @@ -139,6 +166,9 @@ public void testAccessAfterReturn() { @Test public void testBorrowObject() { + @SuppressWarnings("resource") + final ObjectPool pool = createProxiedObjectPool(); + final TestObject obj = pool.borrowObject(); assertNotNull(obj); @@ -151,6 +181,9 @@ public void testBorrowObject() { @Test public void testPassThroughMethods01() { + @SuppressWarnings("resource") + final ObjectPool pool = createProxiedObjectPool(); + assertEquals(0, pool.getNumActive()); assertEquals(0, pool.getNumIdle()); @@ -167,6 +200,8 @@ public void testPassThroughMethods01() { @Test public void testPassThroughMethods02() { + final ObjectPool pool = createProxiedObjectPool(); + pool.close(); assertThrows(IllegalStateException.class, @@ -175,6 +210,9 @@ public void testPassThroughMethods02() { @Test public void testUsageTracking() throws InterruptedException { + @SuppressWarnings("resource") + final ObjectPool pool = createProxiedObjectPool(); + final TestObject obj = pool.borrowObject(); assertNotNull(obj); @@ -193,4 +231,25 @@ public void testUsageTracking() throws InterruptedException { assertTrue(logOutput.contains("The last code to use this object was")); } + @Test + public void testUnwrapInvocationTargetExceptionTrue() { + @SuppressWarnings("resource") + final ObjectPool pool = createProxiedObjectPool(true, new MyException()); + + final TestObject object = pool.borrowObject(); + assertThrows(MyException.class, object::getData); + } + + @Test + public void testUnwrapInvocationTargetExceptionFalse() { + @SuppressWarnings("resource") + final ObjectPool pool = createProxiedObjectPool(false, new MyException()); + + final TestObject object = pool.borrowObject(); + assertThrows(getInvocationTargetExceptionType(), object::getData); + } + + protected abstract Class getInvocationTargetExceptionType(); + + 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 5eb09c12cf..289c7a2ead 100644 --- a/src/test/java/org/apache/commons/pool3/proxy/TestProxiedObjectPoolWithCglibProxy.java +++ b/src/test/java/org/apache/commons/pool3/proxy/TestProxiedObjectPoolWithCglibProxy.java @@ -16,11 +16,18 @@ */ package org.apache.commons.pool3.proxy; +import java.lang.reflect.InvocationTargetException; + 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); + } + + @Override + protected Class getInvocationTargetExceptionType() { + return InvocationTargetException.class; } } 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 c7470bcc57..73304be637 100644 --- a/src/test/java/org/apache/commons/pool3/proxy/TestProxiedObjectPoolWithJdkProxy.java +++ b/src/test/java/org/apache/commons/pool3/proxy/TestProxiedObjectPoolWithJdkProxy.java @@ -16,12 +16,19 @@ */ package org.apache.commons.pool3.proxy; +import java.lang.reflect.UndeclaredThrowableException; + 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); + } + + @Override + protected Class getInvocationTargetExceptionType() { + return UndeclaredThrowableException.class; } }