From 074075de7ee8b67867af3494e6bc3dc1a7952522 Mon Sep 17 00:00:00 2001 From: Jon Chambers Date: Fri, 16 Dec 2016 15:08:11 -0500 Subject: [PATCH] Expose channel pool configuration to subclasses. Motivation: `SimpleChannelPool` subclasses are likely to override the `connectChannel` method, and are likely to clobber the cloned `Bootstrap` handler in the process. To allow subclasses to properly notify the pool listener of new connections, we should expose (at least) the `handler` property of the pool to subclasses. Modifications: Expose `SimpleChannelPool` properties to subclasses via `protected` getters. Result: Subclasses can now use the bootstrap, handler, health checker, and health-check-on-release preoperties from their superclass. --- .../netty/channel/pool/SimpleChannelPool.java | 41 +++++++++- .../channel/pool/SimpleChannelPoolTest.java | 74 +++++++++++++++++-- 2 files changed, 107 insertions(+), 8 deletions(-) diff --git a/transport/src/main/java/io/netty/channel/pool/SimpleChannelPool.java b/transport/src/main/java/io/netty/channel/pool/SimpleChannelPool.java index ef73b94c39..c6c2d6754f 100644 --- a/transport/src/main/java/io/netty/channel/pool/SimpleChannelPool.java +++ b/transport/src/main/java/io/netty/channel/pool/SimpleChannelPool.java @@ -82,8 +82,8 @@ public class SimpleChannelPool implements ChannelPool { * @param handler the {@link ChannelPoolHandler} that will be notified for the different pool actions * @param healthCheck the {@link ChannelHealthChecker} that will be used to check if a {@link Channel} is * still healthy when obtain from the {@link ChannelPool} - * @param releaseHealthCheck will offercheck channel health before offering back if this parameter set to - * {@code true}. + * @param releaseHealthCheck will check channel health before offering back if this parameter set to {@code true}; + * otherwise, channel health is only checked at acquisition time */ public SimpleChannelPool(Bootstrap bootstrap, final ChannelPoolHandler handler, ChannelHealthChecker healthCheck, boolean releaseHealthCheck) { @@ -101,6 +101,43 @@ public class SimpleChannelPool implements ChannelPool { }); } + /** + * Returns the {@link Bootstrap} this pool will use to open new connections. + * + * @return the {@link Bootstrap} this pool will use to open new connections + */ + protected Bootstrap bootstrap() { + return bootstrap; + } + + /** + * Returns the {@link ChannelPoolHandler} that will be notified for the different pool actions. + * + * @return the {@link ChannelPoolHandler} that will be notified for the different pool actions + */ + protected ChannelPoolHandler handler() { + return handler; + } + + /** + * Returns the {@link ChannelHealthChecker} that will be used to check if a {@link Channel} is healthy. + * + * @return the {@link ChannelHealthChecker} that will be used to check if a {@link Channel} is healthy + */ + protected ChannelHealthChecker healthChecker() { + return healthCheck; + } + + /** + * Indicates whether this pool will check the health of channels before offering them back into the pool. + * + * @return {@code true} if this pool will check the health of channels before offering them back into the pool, or + * {@code false} if channel health is only checked at acquisition time + */ + protected boolean releaseHealthCheck() { + return releaseHealthCheck; + } + @Override public final Future acquire() { return acquire(bootstrap.config().group().next().newPromise()); diff --git a/transport/src/test/java/io/netty/channel/pool/SimpleChannelPoolTest.java b/transport/src/test/java/io/netty/channel/pool/SimpleChannelPoolTest.java index 601a2df17e..efa55a2f63 100644 --- a/transport/src/test/java/io/netty/channel/pool/SimpleChannelPoolTest.java +++ b/transport/src/test/java/io/netty/channel/pool/SimpleChannelPoolTest.java @@ -34,12 +34,7 @@ import org.junit.rules.ExpectedException; import java.util.Queue; import java.util.concurrent.LinkedBlockingQueue; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotSame; -import static org.junit.Assert.assertSame; -import static org.junit.Assert.assertThat; -import static org.junit.Assert.fail; +import static org.junit.Assert.*; public class SimpleChannelPoolTest { private static final String LOCAL_ADDR_ID = "test.id"; @@ -242,4 +237,71 @@ public class SimpleChannelPoolTest { channel2.close().syncUninterruptibly(); group.shutdownGracefully(); } + + @Test + public void testBootstrap() { + final SimpleChannelPool pool = new SimpleChannelPool(new Bootstrap(), new CountingChannelPoolHandler()); + + try { + // Checking for the actual bootstrap object doesn't make sense here, since the pool uses a copy with a + // modified channel handler. + assertNotNull(pool.bootstrap()); + } finally { + pool.close(); + } + } + + @Test + public void testHandler() { + final ChannelPoolHandler handler = new CountingChannelPoolHandler(); + final SimpleChannelPool pool = new SimpleChannelPool(new Bootstrap(), handler); + + try { + assertSame(handler, pool.handler()); + } finally { + pool.close(); + } + } + + @Test + public void testHealthChecker() { + final ChannelHealthChecker healthChecker = ChannelHealthChecker.ACTIVE; + final SimpleChannelPool pool = new SimpleChannelPool( + new Bootstrap(), + new CountingChannelPoolHandler(), + healthChecker); + + try { + assertSame(healthChecker, pool.healthChecker()); + } finally { + pool.close(); + } + } + + @Test + public void testReleaseHealthCheck() { + final SimpleChannelPool healthCheckOnReleasePool = new SimpleChannelPool( + new Bootstrap(), + new CountingChannelPoolHandler(), + ChannelHealthChecker.ACTIVE, + true); + + try { + assertTrue(healthCheckOnReleasePool.releaseHealthCheck()); + } finally { + healthCheckOnReleasePool.close(); + } + + final SimpleChannelPool noHealthCheckOnReleasePool = new SimpleChannelPool( + new Bootstrap(), + new CountingChannelPoolHandler(), + ChannelHealthChecker.ACTIVE, + false); + + try { + assertFalse(noHealthCheckOnReleasePool.releaseHealthCheck()); + } finally { + noHealthCheckOnReleasePool.close(); + } + } }