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 1/4] 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); } } From f9a0082d83095b77d97dd0c159203b1e33495e7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9da=20Housni=20Alaoui?= Date: Wed, 6 Dec 2023 15:39:52 +0100 Subject: [PATCH 2/4] Fix code format --- .../java/org/apache/commons/pool3/proxy/BaseProxyHandler.java | 3 ++- .../org/apache/commons/pool3/proxy/CglibProxyHandler.java | 3 ++- .../java/org/apache/commons/pool3/proxy/CglibProxySource.java | 4 +++- .../java/org/apache/commons/pool3/proxy/JdkProxyHandler.java | 3 ++- .../java/org/apache/commons/pool3/proxy/JdkProxySource.java | 3 ++- 5 files changed, 11 insertions(+), 5 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 65e2f62a2..7bd1e7aa0 100644 --- a/src/main/java/org/apache/commons/pool3/proxy/BaseProxyHandler.java +++ b/src/main/java/org/apache/commons/pool3/proxy/BaseProxyHandler.java @@ -42,7 +42,8 @@ class BaseProxyHandler { * @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} + * @param unwrapInvocationTargetException True to make the proxy throw {@link InvocationTargetException#getTargetException()} + * instead of {@link InvocationTargetException} */ BaseProxyHandler(final T pooledObject, final UsageTracking usageTracking, boolean unwrapInvocationTargetException) { this.pooledObject = pooledObject; 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 f4443a3d1..90a056393 100644 --- a/src/main/java/org/apache/commons/pool3/proxy/CglibProxyHandler.java +++ b/src/main/java/org/apache/commons/pool3/proxy/CglibProxyHandler.java @@ -41,7 +41,8 @@ final class CglibProxyHandler extends BaseProxyHandler * @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} + * @param unwrapInvocationTargetException True to make the proxy throw {@link InvocationTargetException#getTargetException()} + * instead of {@link InvocationTargetException} */ CglibProxyHandler(final T pooledObject, final UsageTracking usageTracking, boolean unwrapInvocationTargetException) { super(pooledObject, usageTracking, unwrapInvocationTargetException); 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 cd17b8bb8..8b97113d6 100644 --- a/src/main/java/org/apache/commons/pool3/proxy/CglibProxySource.java +++ b/src/main/java/org/apache/commons/pool3/proxy/CglibProxySource.java @@ -17,6 +17,7 @@ package org.apache.commons.pool3.proxy; import java.lang.reflect.InvocationTargetException; + import org.apache.commons.pool3.UsageTracking; import net.sf.cglib.proxy.Enhancer; @@ -38,7 +39,8 @@ public class CglibProxySource implements ProxySource { * 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} + * @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; 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 cc8031a4d..82982b079 100644 --- a/src/main/java/org/apache/commons/pool3/proxy/JdkProxyHandler.java +++ b/src/main/java/org/apache/commons/pool3/proxy/JdkProxyHandler.java @@ -39,7 +39,8 @@ final class JdkProxyHandler extends BaseProxyHandler * @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} + * @param unwrapInvocationTargetException True to make the proxy throw {@link InvocationTargetException#getTargetException()} + * instead of {@link InvocationTargetException} */ JdkProxyHandler(final T pooledObject, final UsageTracking usageTracking, boolean unwrapInvocationTargetException) { super(pooledObject, usageTracking, unwrapInvocationTargetException); 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 454665c60..b3e166be9 100644 --- a/src/main/java/org/apache/commons/pool3/proxy/JdkProxySource.java +++ b/src/main/java/org/apache/commons/pool3/proxy/JdkProxySource.java @@ -40,7 +40,8 @@ public class JdkProxySource implements ProxySource { * * @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} + * @param unwrapInvocationTargetException True to make the proxy throw {@link InvocationTargetException#getTargetException()} + * instead of {@link InvocationTargetException} */ public JdkProxySource(final ClassLoader classLoader, final Class[] interfaces, boolean unwrapInvocationTargetException) { this.classLoader = classLoader; From 79739d6605c6ced6c178988138e7cdc2e221af51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9da=20Housni=20Alaoui?= Date: Fri, 18 Apr 2025 16:20:26 +0200 Subject: [PATCH 3/4] Cleanup tests --- .../proxy/AbstractTestProxiedObjectPool.java | 24 ++++--------------- .../TestProxiedObjectPoolWithCglibProxy.java | 7 ++++++ .../TestProxiedObjectPoolWithJdkProxy.java | 7 ++++++ 3 files changed, 18 insertions(+), 20 deletions(-) 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 ec39b456f..b01b58789 100644 --- a/src/test/java/org/apache/commons/pool3/proxy/AbstractTestProxiedObjectPool.java +++ b/src/test/java/org/apache/commons/pool3/proxy/AbstractTestProxiedObjectPool.java @@ -20,12 +20,9 @@ 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; @@ -238,12 +235,7 @@ public void testUnwrapInvocationTargetExceptionTrue() { 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 - } + assertThrows(MyException.class, object::getData); } @Test @@ -252,18 +244,10 @@ public void testUnwrapInvocationTargetExceptionFalse() { 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)); + 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 69610a349..289c7a2ea 100644 --- a/src/test/java/org/apache/commons/pool3/proxy/TestProxiedObjectPoolWithCglibProxy.java +++ b/src/test/java/org/apache/commons/pool3/proxy/TestProxiedObjectPoolWithCglibProxy.java @@ -16,6 +16,8 @@ */ package org.apache.commons.pool3.proxy; +import java.lang.reflect.InvocationTargetException; + public class TestProxiedObjectPoolWithCglibProxy extends AbstractTestProxiedObjectPool { @@ -23,4 +25,9 @@ public class TestProxiedObjectPoolWithCglibProxy extends 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 1887a3e38..73304be63 100644 --- a/src/test/java/org/apache/commons/pool3/proxy/TestProxiedObjectPoolWithJdkProxy.java +++ b/src/test/java/org/apache/commons/pool3/proxy/TestProxiedObjectPoolWithJdkProxy.java @@ -16,6 +16,8 @@ */ package org.apache.commons.pool3.proxy; +import java.lang.reflect.UndeclaredThrowableException; + public class TestProxiedObjectPoolWithJdkProxy extends AbstractTestProxiedObjectPool { @@ -24,4 +26,9 @@ protected ProxySource getproxySource(boolean unwrapInvocationTargetE return new JdkProxySource<>(this.getClass().getClassLoader(), new Class[] { TestObject.class }, unwrapInvocationTargetException); } + + @Override + protected Class getInvocationTargetExceptionType() { + return UndeclaredThrowableException.class; + } } From 3f1f63320ec11904b03456151ff9384e3a8253b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9da=20Housni=20Alaoui?= Date: Fri, 18 Apr 2025 16:38:50 +0200 Subject: [PATCH 4/4] Fix checkstyle issues --- .../proxy/AbstractTestProxiedObjectPool.java | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) 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 b01b58789..f584e53f5 100644 --- a/src/test/java/org/apache/commons/pool3/proxy/AbstractTestProxiedObjectPool.java +++ b/src/test/java/org/apache/commons/pool3/proxy/AbstractTestProxiedObjectPool.java @@ -24,6 +24,7 @@ import java.io.PrintWriter; import java.io.StringWriter; import java.time.Duration; + import org.apache.commons.pool3.BasePooledObjectFactory; import org.apache.commons.pool3.ObjectPool; import org.apache.commons.pool3.PooledObject; @@ -97,7 +98,8 @@ private ProxiedObjectPool createProxiedObjectPool( return createProxiedObjectPool(false, null); } - private ProxiedObjectPool createProxiedObjectPool(boolean unwrapInvocationTargetException, RuntimeException exceptionToThrow) { + private ProxiedObjectPool createProxiedObjectPool( + boolean unwrapInvocationTargetException, RuntimeException exceptionToThrow) { final PrintWriter pw = new PrintWriter(log); final AbandonedConfig abandonedConfig = new AbandonedConfig(); abandonedConfig.setLogAbandoned(true); @@ -111,7 +113,7 @@ private ProxiedObjectPool createProxiedObjectPool( final PooledObjectFactory factory = new TestObjectFactory(exceptionToThrow); - ObjectPool innerPool = new GenericObjectPool<>(factory, config, abandonedConfig); + final ObjectPool innerPool = new GenericObjectPool<>(factory, config, abandonedConfig); return new ProxiedObjectPool<>(innerPool, getproxySource(unwrapInvocationTargetException)); } @@ -124,7 +126,7 @@ public void setUp() { @Test public void testAccessAfterInvalidate() { @SuppressWarnings("resource") - ObjectPool pool = createProxiedObjectPool(); + final ObjectPool pool = createProxiedObjectPool(); final TestObject obj = pool.borrowObject(); assertNotNull(obj); @@ -145,7 +147,7 @@ public void testAccessAfterInvalidate() { @Test public void testAccessAfterReturn() { @SuppressWarnings("resource") - ObjectPool pool = createProxiedObjectPool(); + final ObjectPool pool = createProxiedObjectPool(); final TestObject obj = pool.borrowObject(); assertNotNull(obj); @@ -165,7 +167,7 @@ public void testAccessAfterReturn() { @Test public void testBorrowObject() { @SuppressWarnings("resource") - ObjectPool pool = createProxiedObjectPool(); + final ObjectPool pool = createProxiedObjectPool(); final TestObject obj = pool.borrowObject(); assertNotNull(obj); @@ -180,7 +182,7 @@ public void testBorrowObject() { @Test public void testPassThroughMethods01() { @SuppressWarnings("resource") - ObjectPool pool = createProxiedObjectPool(); + final ObjectPool pool = createProxiedObjectPool(); assertEquals(0, pool.getNumActive()); assertEquals(0, pool.getNumIdle()); @@ -198,7 +200,7 @@ public void testPassThroughMethods01() { @Test public void testPassThroughMethods02() { - ObjectPool pool = createProxiedObjectPool(); + final ObjectPool pool = createProxiedObjectPool(); pool.close(); @@ -209,7 +211,7 @@ public void testPassThroughMethods02() { @Test public void testUsageTracking() throws InterruptedException { @SuppressWarnings("resource") - ObjectPool pool = createProxiedObjectPool(); + final ObjectPool pool = createProxiedObjectPool(); final TestObject obj = pool.borrowObject(); assertNotNull(obj); @@ -232,22 +234,22 @@ public void testUsageTracking() throws InterruptedException { @Test public void testUnwrapInvocationTargetExceptionTrue() { @SuppressWarnings("resource") - ObjectPool pool = createProxiedObjectPool(true, new MyException()); + final ObjectPool pool = createProxiedObjectPool(true, new MyException()); - TestObject object = pool.borrowObject(); + final TestObject object = pool.borrowObject(); assertThrows(MyException.class, object::getData); } @Test public void testUnwrapInvocationTargetExceptionFalse() { @SuppressWarnings("resource") - ObjectPool pool = createProxiedObjectPool(false, new MyException()); + final ObjectPool pool = createProxiedObjectPool(false, new MyException()); - TestObject object = pool.borrowObject(); + final TestObject object = pool.borrowObject(); assertThrows(getInvocationTargetExceptionType(), object::getData); } protected abstract Class getInvocationTargetExceptionType(); - private static class MyException extends RuntimeException {} + private static class MyException extends RuntimeException { } }