Skip to content

POOL-415 Add an option to JdkProxySource allowing to unwrap UndeclaredThrowableException #261

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 18 additions & 6 deletions src/main/java/org/apache/commons/pool3/proxy/BaseProxyHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -31,18 +32,22 @@ class BaseProxyHandler<T> {

private volatile T pooledObject;
private final UsageTracking<T> 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<T> usageTracking) {
BaseProxyHandler(final T pooledObject, final UsageTracking<T> usageTracking, boolean unwrapInvocationTargetException) {
this.pooledObject = pooledObject;
this.usageTracking = usageTracking;
this.unwrapInvocationTargetException = unwrapInvocationTargetException;
}

/**
Expand Down Expand Up @@ -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;
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -37,13 +38,15 @@ final class CglibProxyHandler<T> extends BaseProxyHandler<T>
/**
* 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<T> usageTracking) {
super(pooledObject, usageTracking);
CglibProxyHandler(final T pooledObject, final UsageTracking<T> usageTracking, boolean unwrapInvocationTargetException) {
super(pooledObject, usageTracking, unwrapInvocationTargetException);
}

@Override
Expand Down
19 changes: 17 additions & 2 deletions src/main/java/org/apache/commons/pool3/proxy/CglibProxySource.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -32,14 +34,27 @@
public class CglibProxySource<T> implements ProxySource<T> {

private final Class<? extends T> 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<? extends T> superclass, boolean unwrapInvocationTargetException) {
this.superclass = superclass;
this.unwrapInvocationTargetException = unwrapInvocationTargetException;
}

/**
* Constructs a new proxy source for the given class.
*
* @param superclass The class to proxy
*/
public CglibProxySource(final Class<? extends T> superclass) {
this.superclass = superclass;
this(superclass, false);
}

@SuppressWarnings("unchecked") // Case to T on return
Expand All @@ -49,7 +64,7 @@ public T createProxy(final T pooledObject, final UsageTracking<T> usageTracking)
enhancer.setSuperclass(superclass);

final CglibProxyHandler<T> proxyInterceptor =
new CglibProxyHandler<>(pooledObject, usageTracking);
new CglibProxyHandler<>(pooledObject, usageTracking, unwrapInvocationTargetException);
enhancer.setCallback(proxyInterceptor);

return (T) enhancer.create();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -33,13 +34,15 @@ final class JdkProxyHandler<T> extends BaseProxyHandler<T>
/**
* 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<T> usageTracking) {
super(pooledObject, usageTracking);
JdkProxyHandler(final T pooledObject, final UsageTracking<T> usageTracking, boolean unwrapInvocationTargetException) {
super(pooledObject, usageTracking, unwrapInvocationTargetException);
}

@Override
Expand Down
19 changes: 17 additions & 2 deletions src/main/java/org/apache/commons/pool3/proxy/JdkProxySource.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/
package org.apache.commons.pool3.proxy;

import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Proxy;
import java.util.Arrays;

Expand All @@ -31,24 +32,38 @@ public class JdkProxySource<T> implements ProxySource<T> {

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<T> usageTracking) {
return (T) Proxy.newProxyInstance(classLoader, interfaces,
new JdkProxyHandler<>(pooledObject, usageTracking));
new JdkProxyHandler<>(pooledObject, usageTracking, unwrapInvocationTargetException));
}

@SuppressWarnings("unchecked")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,15 @@ protected interface TestObject {
private static final class TestObjectFactory extends
BasePooledObjectFactory<TestObject, RuntimeException> {

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<TestObject> wrap(final TestObject value) {
Expand All @@ -57,32 +63,43 @@ public PooledObject<TestObject> 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;
}
}
private static final String DATA1 = "data1";

private static final Duration ABANDONED_TIMEOUT_SECS = Duration.ofSeconds(3);

private ObjectPool<TestObject, RuntimeException> pool;

private StringWriter log;

protected abstract ProxySource<TestObject> getproxySource();
protected abstract ProxySource<TestObject> getproxySource(boolean unwrapInvocationTargetException);

@BeforeEach
public void setUp() {
log = new StringWriter();
private ProxiedObjectPool<TestObject, RuntimeException> createProxiedObjectPool() {
return createProxiedObjectPool(false, null);
}

private ProxiedObjectPool<TestObject, RuntimeException> createProxiedObjectPool(
boolean unwrapInvocationTargetException, RuntimeException exceptionToThrow) {
final PrintWriter pw = new PrintWriter(log);
final AbandonedConfig abandonedConfig = new AbandonedConfig();
abandonedConfig.setLogAbandoned(true);
Expand All @@ -94,16 +111,23 @@ public void setUp() {
final GenericObjectPoolConfig<TestObject> config = new GenericObjectPoolConfig<>();
config.setMaxTotal(3);

final PooledObjectFactory<TestObject, RuntimeException> factory = new TestObjectFactory();
final PooledObjectFactory<TestObject, RuntimeException> factory = new TestObjectFactory(exceptionToThrow);

@SuppressWarnings("resource")
final ObjectPool<TestObject, RuntimeException> 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<TestObject, RuntimeException> pool = createProxiedObjectPool();

final TestObject obj = pool.borrowObject();
assertNotNull(obj);

Expand All @@ -122,6 +146,9 @@ public void testAccessAfterInvalidate() {

@Test
public void testAccessAfterReturn() {
@SuppressWarnings("resource")
final ObjectPool<TestObject, RuntimeException> pool = createProxiedObjectPool();

final TestObject obj = pool.borrowObject();
assertNotNull(obj);

Expand All @@ -139,6 +166,9 @@ public void testAccessAfterReturn() {

@Test
public void testBorrowObject() {
@SuppressWarnings("resource")
final ObjectPool<TestObject, RuntimeException> pool = createProxiedObjectPool();

final TestObject obj = pool.borrowObject();
assertNotNull(obj);

Expand All @@ -151,6 +181,9 @@ public void testBorrowObject() {

@Test
public void testPassThroughMethods01() {
@SuppressWarnings("resource")
final ObjectPool<TestObject, RuntimeException> pool = createProxiedObjectPool();

assertEquals(0, pool.getNumActive());
assertEquals(0, pool.getNumIdle());

Expand All @@ -167,6 +200,8 @@ public void testPassThroughMethods01() {

@Test
public void testPassThroughMethods02() {
final ObjectPool<TestObject, RuntimeException> pool = createProxiedObjectPool();

pool.close();

assertThrows(IllegalStateException.class,
Expand All @@ -175,6 +210,9 @@ public void testPassThroughMethods02() {

@Test
public void testUsageTracking() throws InterruptedException {
@SuppressWarnings("resource")
final ObjectPool<TestObject, RuntimeException> pool = createProxiedObjectPool();

final TestObject obj = pool.borrowObject();
assertNotNull(obj);

Expand All @@ -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<TestObject, RuntimeException> pool = createProxiedObjectPool(true, new MyException());

final TestObject object = pool.borrowObject();
assertThrows(MyException.class, object::getData);
}

@Test
public void testUnwrapInvocationTargetExceptionFalse() {
@SuppressWarnings("resource")
final ObjectPool<TestObject, RuntimeException> pool = createProxiedObjectPool(false, new MyException());

final TestObject object = pool.borrowObject();
assertThrows(getInvocationTargetExceptionType(), object::getData);
}

protected abstract Class<? extends Throwable> getInvocationTargetExceptionType();

private static class MyException extends RuntimeException { }
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,18 @@
*/
package org.apache.commons.pool3.proxy;

import java.lang.reflect.InvocationTargetException;

public class TestProxiedObjectPoolWithCglibProxy extends
AbstractTestProxiedObjectPool {

@Override
protected ProxySource<TestObject> getproxySource() {
return new CglibProxySource<>(TestObject.class);
protected ProxySource<TestObject> getproxySource(boolean unwrapInvocationTargetException) {
return new CglibProxySource<>(TestObject.class, unwrapInvocationTargetException);
}

@Override
protected Class<? extends Throwable> getInvocationTargetExceptionType() {
return InvocationTargetException.class;
}
}
Loading
Loading