You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by gg...@apache.org on 2019/08/26 22:23:25 UTC
[commons-pool] branch master updated: [POOL-374]
org.apache.commons.pool2.impl.GenericKeyedObjectPool.returnObject(K,
T) should throw IllegalStateException instead of NullPointerException when a
key is not found in the pool map.
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 41f4e41 [POOL-374] org.apache.commons.pool2.impl.GenericKeyedObjectPool.returnObject(K, T) should throw IllegalStateException instead of NullPointerException when a key is not found in the pool map.
41f4e41 is described below
commit 41f4e410b3e7dc34b294ac9941721073bf5e5271
Author: Gary Gregory <ga...@gmail.com>
AuthorDate: Mon Aug 26 18:23:20 2019 -0400
[POOL-374]
org.apache.commons.pool2.impl.GenericKeyedObjectPool.returnObject(K, T)
should throw IllegalStateException instead of NullPointerException when
a key is not found in the pool map.
---
src/changes/changes.xml | 7 ++++++-
.../apache/commons/pool2/impl/GenericKeyedObjectPool.java | 5 +++++
.../commons/pool2/impl/TestGenericKeyedObjectPool.java | 12 ++++++++++--
3 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 929d1d9..89b89a5 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -43,7 +43,12 @@ The <action> type attribute can be add,update,fix,remove.
<title>Apache Commons Pool Release Notes</title>
</properties>
<body>
- <release version="2.7.0" date="2019-MM-DD" description="This is a feature release (Java 8).">
+ <release version="2.7.1" date="2019-MM-DD" description="This is a maintenance release (Java 8).">
+ <action dev="ggregory" issue="POOL-374" type="fix" due-to="Gary Gregory, Phil Steitz">
+ org.apache.commons.pool2.impl.GenericKeyedObjectPool.returnObject(K, T) should throw IllegalStateException instead of NullPointerException when a key is not found in the pool map.
+ </action>
+ </release>
+ <release version="2.7.0" date="2019-07-25" description="This is a feature release (Java 8).">
<action dev="ggregory" issue="POOL-364" type="update" due-to="Gary Gregory">
Update from Java 7 to Java 8.
</action>
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 b3fc96f..7a282f1 100644
--- a/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
+++ b/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
@@ -447,6 +447,11 @@ public class GenericKeyedObjectPool<K, T> extends BaseGenericObjectPool<T>
final ObjectDeque<T> objectDeque = poolMap.get(key);
+ if (objectDeque == null) {
+ throw new IllegalStateException(
+ "Returned object not currently part of this pool");
+ }
+
final PooledObject<T> p = objectDeque.getAllObjects().get(new IdentityWrapper<>(obj));
if (p == null) {
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 7b5b5ac..f148bc0 100644
--- a/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java
+++ b/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java
@@ -195,6 +195,13 @@ public class TestGenericKeyedObjectPool extends TestKeyedObjectPool {
assertEquals(2,gkoPool.getNumIdle("B"));
}
+ @Test(expected = IllegalStateException.class)
+ public void testReturnObjectThrowsIllegalStateException() {
+ try (final GenericKeyedObjectPool<String, String> pool = new GenericKeyedObjectPool<>(new SimpleFactory<String>())) {
+ pool.returnObject("Foo", "Bar");
+ }
+ }
+
@Test(timeout=60000)
public void testMaxIdle() throws Exception {
gkoPool.setMaxTotalPerKey(100);
@@ -1069,7 +1076,7 @@ public class TestGenericKeyedObjectPool extends TestKeyedObjectPool {
final int numThreads = 40;
final int maxTotal = 40;
- final GenericKeyedObjectPoolConfig config = new GenericKeyedObjectPoolConfig();
+ final GenericKeyedObjectPoolConfig<String> config = new GenericKeyedObjectPoolConfig<String>();
config.setMaxTotalPerKey(maxTotal);
config.setFairness(true);
config.setLifo(false);
@@ -1086,7 +1093,7 @@ public class TestGenericKeyedObjectPool extends TestKeyedObjectPool {
// Start and park threads waiting to borrow objects
final TestThread[] threads = new TestThread[numThreads];
for(int i=0;i<numThreads;i++) {
- threads[i] = new TestThread(gkoPool, 1, 0, 2000, false, "0" + String.valueOf(i % maxTotal), "0");
+ threads[i] = new TestThread<String>(gkoPool, 1, 0, 2000, false, "0" + String.valueOf(i % maxTotal), "0");
final Thread t = new Thread(threads[i]);
t.start();
// Short delay to ensure threads start in correct order
@@ -2371,6 +2378,7 @@ public class TestGenericKeyedObjectPool extends TestKeyedObjectPool {
return new DefaultPooledObject<>(value);
}
}
+
}
Re: [commons-pool] branch master updated: [POOL-374]
org.apache.commons.pool2.impl.GenericKeyedObjectPool.returnObject(K, T)
should throw IllegalStateException instead of NullPointerException when a key
is not found in the pool map.
Posted by Gary Gregory <ga...@gmail.com>.
On Mon, Aug 26, 2019 at 6:29 PM Phil Steitz <ph...@gmail.com> wrote:
>
>
> On 8/26/19 3:23 PM, ggregory@apache.org wrote:
> > 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 41f4e41 [POOL-374]
> org.apache.commons.pool2.impl.GenericKeyedObjectPool.returnObject(K, T)
> should throw IllegalStateException instead of NullPointerException when a
> key is not found in the pool map.
> > 41f4e41 is described below
> >
> > commit 41f4e410b3e7dc34b294ac9941721073bf5e5271
> > Author: Gary Gregory <ga...@gmail.com>
> > AuthorDate: Mon Aug 26 18:23:20 2019 -0400
> >
> > [POOL-374]
> >
> org.apache.commons.pool2.impl.GenericKeyedObjectPool.returnObject(K, T)
> > should throw IllegalStateException instead of NullPointerException
> when
> > a key is not found in the pool map.
> > ---
> > src/changes/changes.xml | 7
> ++++++-
> > .../apache/commons/pool2/impl/GenericKeyedObjectPool.java | 5 +++++
> > .../commons/pool2/impl/TestGenericKeyedObjectPool.java | 12
> ++++++++++--
> > 3 files changed, 21 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/changes/changes.xml b/src/changes/changes.xml
> > index 929d1d9..89b89a5 100644
> > --- a/src/changes/changes.xml
> > +++ b/src/changes/changes.xml
> > @@ -43,7 +43,12 @@ The <action> type attribute can be
> add,update,fix,remove.
> > <title>Apache Commons Pool Release Notes</title>
> > </properties>
> > <body>
> > - <release version="2.7.0" date="2019-MM-DD" description="This is a
> feature release (Java 8).">
> > + <release version="2.7.1" date="2019-MM-DD" description="This is a
> maintenance release (Java 8).">
> > + <action dev="ggregory" issue="POOL-374" type="fix" due-to="Gary
> Gregory, Phil Steitz">
> > +
> org.apache.commons.pool2.impl.GenericKeyedObjectPool.returnObject(K, T)
> should throw IllegalStateException instead of NullPointerException when a
> key is not found in the pool map.
> > + </action>
> > + </release>
> > + <release version="2.7.0" date="2019-07-25" description="This is a
> feature release (Java 8).">
> > <action dev="ggregory" issue="POOL-364" type="update" due-to="Gary
> Gregory">
> > Update from Java 7 to Java 8.
> > </action>
> > 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 b3fc96f..7a282f1 100644
> > ---
> a/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
> > +++
> b/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
> > @@ -447,6 +447,11 @@ public class GenericKeyedObjectPool<K, T> extends
> BaseGenericObjectPool<T>
> >
> > final ObjectDeque<T> objectDeque = poolMap.get(key);
> >
> > + if (objectDeque == null) {
> > + throw new IllegalStateException(
> > + "Returned object not currently part of this pool");
>
> Might be better to say it is the key that is not found, e.g.
>
> "Key not found" or "No keyed pool found under the given key" or
> something like that. The problem is with the key, not the returning
> object identity.
>
OK, the ISE message is now "No keyed pool found under the given key".
TY,
Gary
>
> Phil
> > + }
> > +
> > final PooledObject<T> p = objectDeque.getAllObjects().get(new
> IdentityWrapper<>(obj));
> >
> > if (p == null) {
> > 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 7b5b5ac..f148bc0 100644
> > ---
> a/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java
> > +++
> b/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java
> > @@ -195,6 +195,13 @@ public class TestGenericKeyedObjectPool extends
> TestKeyedObjectPool {
> > assertEquals(2,gkoPool.getNumIdle("B"));
> > }
> >
> > + @Test(expected = IllegalStateException.class)
> > + public void testReturnObjectThrowsIllegalStateException() {
> > + try (final GenericKeyedObjectPool<String, String> pool = new
> GenericKeyedObjectPool<>(new SimpleFactory<String>())) {
> > + pool.returnObject("Foo", "Bar");
> > + }
> > + }
> > +
> > @Test(timeout=60000)
> > public void testMaxIdle() throws Exception {
> > gkoPool.setMaxTotalPerKey(100);
> > @@ -1069,7 +1076,7 @@ public class TestGenericKeyedObjectPool extends
> TestKeyedObjectPool {
> > final int numThreads = 40;
> > final int maxTotal = 40;
> >
> > - final GenericKeyedObjectPoolConfig config = new
> GenericKeyedObjectPoolConfig();
> > + final GenericKeyedObjectPoolConfig<String> config = new
> GenericKeyedObjectPoolConfig<String>();
> > config.setMaxTotalPerKey(maxTotal);
> > config.setFairness(true);
> > config.setLifo(false);
> > @@ -1086,7 +1093,7 @@ public class TestGenericKeyedObjectPool extends
> TestKeyedObjectPool {
> > // Start and park threads waiting to borrow objects
> > final TestThread[] threads = new TestThread[numThreads];
> > for(int i=0;i<numThreads;i++) {
> > - threads[i] = new TestThread(gkoPool, 1, 0, 2000, false, "0"
> + String.valueOf(i % maxTotal), "0");
> > + threads[i] = new TestThread<String>(gkoPool, 1, 0, 2000,
> false, "0" + String.valueOf(i % maxTotal), "0");
> > final Thread t = new Thread(threads[i]);
> > t.start();
> > // Short delay to ensure threads start in correct order
> > @@ -2371,6 +2378,7 @@ public class TestGenericKeyedObjectPool extends
> TestKeyedObjectPool {
> > return new DefaultPooledObject<>(value);
> > }
> > }
> > +
> > }
> >
> >
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
Re: [commons-pool] branch master updated: [POOL-374]
org.apache.commons.pool2.impl.GenericKeyedObjectPool.returnObject(K, T)
should throw IllegalStateException instead of NullPointerException when a key
is not found in the pool map.
Posted by Phil Steitz <ph...@gmail.com>.
On 8/26/19 3:23 PM, ggregory@apache.org wrote:
> 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 41f4e41 [POOL-374] org.apache.commons.pool2.impl.GenericKeyedObjectPool.returnObject(K, T) should throw IllegalStateException instead of NullPointerException when a key is not found in the pool map.
> 41f4e41 is described below
>
> commit 41f4e410b3e7dc34b294ac9941721073bf5e5271
> Author: Gary Gregory <ga...@gmail.com>
> AuthorDate: Mon Aug 26 18:23:20 2019 -0400
>
> [POOL-374]
> org.apache.commons.pool2.impl.GenericKeyedObjectPool.returnObject(K, T)
> should throw IllegalStateException instead of NullPointerException when
> a key is not found in the pool map.
> ---
> src/changes/changes.xml | 7 ++++++-
> .../apache/commons/pool2/impl/GenericKeyedObjectPool.java | 5 +++++
> .../commons/pool2/impl/TestGenericKeyedObjectPool.java | 12 ++++++++++--
> 3 files changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/src/changes/changes.xml b/src/changes/changes.xml
> index 929d1d9..89b89a5 100644
> --- a/src/changes/changes.xml
> +++ b/src/changes/changes.xml
> @@ -43,7 +43,12 @@ The <action> type attribute can be add,update,fix,remove.
> <title>Apache Commons Pool Release Notes</title>
> </properties>
> <body>
> - <release version="2.7.0" date="2019-MM-DD" description="This is a feature release (Java 8).">
> + <release version="2.7.1" date="2019-MM-DD" description="This is a maintenance release (Java 8).">
> + <action dev="ggregory" issue="POOL-374" type="fix" due-to="Gary Gregory, Phil Steitz">
> + org.apache.commons.pool2.impl.GenericKeyedObjectPool.returnObject(K, T) should throw IllegalStateException instead of NullPointerException when a key is not found in the pool map.
> + </action>
> + </release>
> + <release version="2.7.0" date="2019-07-25" description="This is a feature release (Java 8).">
> <action dev="ggregory" issue="POOL-364" type="update" due-to="Gary Gregory">
> Update from Java 7 to Java 8.
> </action>
> 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 b3fc96f..7a282f1 100644
> --- a/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
> +++ b/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
> @@ -447,6 +447,11 @@ public class GenericKeyedObjectPool<K, T> extends BaseGenericObjectPool<T>
>
> final ObjectDeque<T> objectDeque = poolMap.get(key);
>
> + if (objectDeque == null) {
> + throw new IllegalStateException(
> + "Returned object not currently part of this pool");
Might be better to say it is the key that is not found, e.g.
"Key not found" or "No keyed pool found under the given key" or
something like that. The problem is with the key, not the returning
object identity.
Phil
> + }
> +
> final PooledObject<T> p = objectDeque.getAllObjects().get(new IdentityWrapper<>(obj));
>
> if (p == null) {
> 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 7b5b5ac..f148bc0 100644
> --- a/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java
> +++ b/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java
> @@ -195,6 +195,13 @@ public class TestGenericKeyedObjectPool extends TestKeyedObjectPool {
> assertEquals(2,gkoPool.getNumIdle("B"));
> }
>
> + @Test(expected = IllegalStateException.class)
> + public void testReturnObjectThrowsIllegalStateException() {
> + try (final GenericKeyedObjectPool<String, String> pool = new GenericKeyedObjectPool<>(new SimpleFactory<String>())) {
> + pool.returnObject("Foo", "Bar");
> + }
> + }
> +
> @Test(timeout=60000)
> public void testMaxIdle() throws Exception {
> gkoPool.setMaxTotalPerKey(100);
> @@ -1069,7 +1076,7 @@ public class TestGenericKeyedObjectPool extends TestKeyedObjectPool {
> final int numThreads = 40;
> final int maxTotal = 40;
>
> - final GenericKeyedObjectPoolConfig config = new GenericKeyedObjectPoolConfig();
> + final GenericKeyedObjectPoolConfig<String> config = new GenericKeyedObjectPoolConfig<String>();
> config.setMaxTotalPerKey(maxTotal);
> config.setFairness(true);
> config.setLifo(false);
> @@ -1086,7 +1093,7 @@ public class TestGenericKeyedObjectPool extends TestKeyedObjectPool {
> // Start and park threads waiting to borrow objects
> final TestThread[] threads = new TestThread[numThreads];
> for(int i=0;i<numThreads;i++) {
> - threads[i] = new TestThread(gkoPool, 1, 0, 2000, false, "0" + String.valueOf(i % maxTotal), "0");
> + threads[i] = new TestThread<String>(gkoPool, 1, 0, 2000, false, "0" + String.valueOf(i % maxTotal), "0");
> final Thread t = new Thread(threads[i]);
> t.start();
> // Short delay to ensure threads start in correct order
> @@ -2371,6 +2378,7 @@ public class TestGenericKeyedObjectPool extends TestKeyedObjectPool {
> return new DefaultPooledObject<>(value);
> }
> }
> +
> }
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org