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);

Reply via email to