Permalink
Browse files

Added unit test for QueuedKeyedResourcePool. Minor clean up of

QueuedKeyedResourcePool and of KeyedResourcePoolTest. Still one
outstanding TODO in QueuedKeyedResourcePool wrt semantics of
close(K key) method.
  • Loading branch information...
1 parent ca6dcd7 commit cd507929a86f26d270f26b14e7b7d025565b801f @jayjwylie jayjwylie committed Aug 30, 2012
@@ -14,8 +14,21 @@
* Extends simple implementation of a per-key resource pool with a non-blocking
* interface to enqueue requests for a resource when one becomes available. <br>
* <ul>
- * <li>allocates resources in FIFO order
- * <li>Pools are per key and there is no global maximum pool limit.
+ * <li>Allocates resources in FIFOish order: blocking requests via checkout are
+ * FIFO and non-blocking enqueued requests are FIFO, however, there is no
+ * ordering between blocking (checkout) and non-blocking (requestResource).
+ * <li>Pools and Queues are per key and there is no global maximum pool or queue
+ * limit.
+ * </ul>
+ *
+ * Beyond the expectations documented in KeyedResourcePool, the following is
+ * expected of the user of this class:
+ * <ul>
+ * <li>A resource acquired via {@link #checkout(K)) checkout} or via {@link
+ * #requestResource(K , ResourceRequest<V>) requestResource} is checked in
+ * exactly once.
+ * <li>A resource that is checked in was previously checked out or requested.
+ * <li>Also, reqeustResource is never called after close.
* </ul>
*/
public class QueuedKeyedResourcePool<K, V> extends KeyedResourcePool<K, V> {
@@ -112,9 +125,6 @@ public void requestResource(K key, ResourceRequest<V> resourceRequest) {
resourceRequest.handleException(e);
}
if(resource != null) {
- // TODO: Is another try/catch block needed anywhere to ensure
- // resource is destroyed if/when anything bad happens in
- // useResource method?
resourceRequest.useResource(resource);
return;
}
@@ -191,11 +201,9 @@ private boolean processQueue(K key) throws Exception {
*/
@Override
public void checkin(K key, V resource) throws Exception {
- // TODO: Unclear if invoking checkin and then invoking processQueue is
- // "fair" or is "FIFO". In particular, is super.checkout invoked
- // directly? If so, how should such a blocking checkout interact with
- // non-blocking checkouts?
super.checkin(key, resource);
+ // NB: Blocking checkout calls may get checked in resource before
+ // processQueue.
while(processQueue(key)) {}
}
@@ -21,12 +21,12 @@
public class KeyedResourcePoolTest {
- private static int POOL_SIZE = 5;
- private static long TIMEOUT_MS = 100;
+ protected static int POOL_SIZE = 5;
+ protected static long TIMEOUT_MS = 100;
- private TestResourceFactory factory;
- private KeyedResourcePool<String, TestResource> pool;
- private ResourcePoolConfig config;
+ protected TestResourceFactory factory;
+ protected KeyedResourcePool<String, TestResource> pool;
+ protected ResourcePoolConfig config;
@Before
public void setUp() {
@@ -140,6 +140,7 @@ public void testExceptionOnCreate() throws Exception {
assertEquals(0, this.pool.getCheckedInResourceCount());
}
+ // NEGATIVE Test. See comment in line.
@Test
public void repeatedCheckins() throws Exception {
assertEquals(0, this.pool.getTotalResourceCount());
@@ -164,6 +165,7 @@ public void repeatedCheckins() throws Exception {
// assertEquals(1, this.pool.getCheckedInResourceCount());
}
+ // NEGATIVE Test. See comment in line.
@Test
public void testExceptionOnFullCheckin() throws Exception {
assertEquals(0, this.pool.getTotalResourceCount());
@@ -196,6 +198,7 @@ public void testExceptionOnFullCheckin() throws Exception {
// assertEquals(POOL_SIZE, this.pool.getTotalResourceCount());
}
+ // NEGATIVE Test. See comment in line.
@Test
public void testCheckinExtraneousResource() throws Exception {
assertEquals(0, this.pool.getTotalResourceCount());
@@ -212,6 +215,7 @@ public void testCheckinExtraneousResource() throws Exception {
assertEquals(2, this.pool.getCheckedInResourceCount());
}
+ // NEGATIVE Test. See comment in line.
@Test
public void testNeverCheckin() throws Exception {
assertEquals(0, this.pool.getTotalResourceCount());
@@ -344,7 +348,7 @@ public void run() {
}
}
- private static class TestResource {
+ protected static class TestResource {
private String value;
private AtomicBoolean isValid;
@@ -379,7 +383,7 @@ public String toString() {
}
- private static class TestResourceFactory implements ResourceFactory<String, TestResource> {
+ protected static class TestResourceFactory implements ResourceFactory<String, TestResource> {
private final AtomicInteger created = new AtomicInteger(0);
private final AtomicInteger destroyed = new AtomicInteger(0);
Oops, something went wrong.

0 comments on commit cd50792

Please sign in to comment.