This is an automated email from the ASF dual-hosted git repository. ggregory pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/commons-pool.git
The following commit(s) were added to refs/heads/master by this push: new d9cf897 Move validation for newly created objects into create(). Fixes POOL-361. (#23) d9cf897 is described below commit d9cf8976af4e3a246f197dad71c5eb7caeca66b5 Author: Phil Steitz <pste...@apache.org> AuthorDate: Mon Jul 22 14:27:25 2019 -0700 Move validation for newly created objects into create(). Fixes POOL-361. (#23) --- .../commons/pool2/impl/GenericKeyedObjectPool.java | 7 ++++- .../commons/pool2/impl/GenericObjectPool.java | 6 ++++- .../pool2/impl/TestGenericKeyedObjectPool.java | 30 ++++++++++++++++++++++ .../commons/pool2/impl/TestGenericObjectPool.java | 28 ++++++++++++++++++++ 4 files changed, 69 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java b/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java index 0622bfb..562e1ac 100644 --- a/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java +++ b/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java @@ -383,7 +383,7 @@ public class GenericKeyedObjectPool<K, T> extends BaseGenericObjectPool<T> throw nsee; } } - if (p != null && (getTestOnBorrow() || create && getTestOnCreate())) { + if (p != null && getTestOnBorrow()) { boolean validate = false; Throwable validationThrowable = null; try { @@ -1039,6 +1039,11 @@ public class GenericKeyedObjectPool<K, T> extends BaseGenericObjectPool<T> PooledObject<T> p = null; try { p = factory.makeObject(key); + if (getTestOnCreate() && !factory.validateObject(key, p)) { + numTotal.decrementAndGet(); + objectDeque.getCreateCount().decrementAndGet(); + return null; + } } catch (final Exception e) { numTotal.decrementAndGet(); objectDeque.getCreateCount().decrementAndGet(); diff --git a/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java b/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java index 695f611..ffe22b1 100644 --- a/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java +++ b/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java @@ -465,7 +465,7 @@ public class GenericObjectPool<T> extends BaseGenericObjectPool<T> throw nsee; } } - if (p != null && (getTestOnBorrow() || create && getTestOnCreate())) { + if (p != null && getTestOnBorrow()) { boolean validate = false; Throwable validationThrowable = null; try { @@ -887,6 +887,10 @@ public class GenericObjectPool<T> extends BaseGenericObjectPool<T> final PooledObject<T> p; try { p = factory.makeObject(); + if (getTestOnCreate() && !factory.validateObject(p)) { + createCount.decrementAndGet(); + return null; + } } catch (final Throwable e) { createCount.decrementAndGet(); throw e; diff --git a/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java b/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java index ae18c90..7b5b5ac 100644 --- a/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java +++ b/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java @@ -2275,6 +2275,36 @@ public class TestGenericKeyedObjectPool extends TestKeyedObjectPool { Assert.assertFalse(testB._failed); } + // Pool-361 + @Test + public void testValidateOnCreate() throws Exception { + gkoPool.setTestOnCreate(true); + simpleFactory.setValidationEnabled(true); + gkoPool.addObject("one"); + Assert.assertEquals(1, simpleFactory.validateCounter); + } + + @Test + public void testValidateOnCreateFailure() throws Exception { + gkoPool.setTestOnCreate(true); + gkoPool.setTestOnBorrow(false); + gkoPool.setMaxTotal(2); + simpleFactory.setValidationEnabled(true); + simpleFactory.setValid(false); + // Make sure failed validations do not leak capacity + gkoPool.addObject("one"); + gkoPool.addObject("one"); + assertEquals(0, gkoPool.getNumIdle()); + assertEquals(0, gkoPool.getNumActive()); + simpleFactory.setValid(true); + final String obj = gkoPool.borrowObject("one"); + assertNotNull(obj); + gkoPool.addObject("one"); + // Should have one idle, one out now + assertEquals(1, gkoPool.getNumIdle()); + assertEquals(1, gkoPool.getNumActive()); + } + private static class DummyFactory extends BaseKeyedPooledObjectFactory<Object,Object> { diff --git a/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java b/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java index d9be87f..27c81b2 100644 --- a/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java +++ b/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java @@ -2839,6 +2839,34 @@ public class TestGenericObjectPool extends TestBaseObjectPool { Assert.assertEquals(1, simpleFactory.validateCounter); } + // Pool-361 + @Test + public void testValidateOnCreate() throws Exception { + genericObjectPool.setTestOnCreate(true); + genericObjectPool.addObject(); + Assert.assertEquals(1, simpleFactory.validateCounter); + } + + @Test + public void testValidateOnCreateFailure() throws Exception { + genericObjectPool.setTestOnCreate(true); + genericObjectPool.setTestOnBorrow(false); + genericObjectPool.setMaxTotal(2); + simpleFactory.setValid(false); + // Make sure failed validations do not leak capacity + genericObjectPool.addObject(); + genericObjectPool.addObject(); + assertEquals(0, genericObjectPool.getNumIdle()); + assertEquals(0, genericObjectPool.getNumActive()); + simpleFactory.setValid(true); + final String obj = genericObjectPool.borrowObject(); + assertNotNull(obj); + genericObjectPool.addObject(); + // Should have one idle, one out now + assertEquals(1, genericObjectPool.getNumIdle()); + assertEquals(1, genericObjectPool.getNumActive()); + } + @Test(timeout = 60000) public void testWhenExhaustedBlock() throws Exception { genericObjectPool.setMaxTotal(1);