Cleanup ReconnectPromise a bit

ClosedChannelHandler is really a stateless object used by
ReconnectPromise wiring. Acknowledge that fact by removing the class in
favor of a field initialized by an anonoymous inner class.

While we are here, also document locking rules and generally tighten
things up, eliminating a SpotBugs suppression in the process.

Change-Id: I6abc0ddf296dd9e2f5ea6de5a709003cf14edc7a
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
(cherry picked from commit 82f3e83d6e777a5e76b8b6bca5a8c2646a4b82be)

diff --git a/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/ReconnectPromise.java b/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/ReconnectPromise.java
index 60f5d205604d16b70b0fa7bd240d7a4b5a7b56fe..b4799811a9c03e519c7e68270a4e3934f281d26f 100644 (file)
--- a/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/ReconnectPromise.java
+++ b/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/ReconnectPromise.java
@@ -9,7 +9,6 @@ package org.opendaylight.netconf.nettyutil;
 
 import static java.util.Objects.requireNonNull;
 
-import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import io.netty.bootstrap.Bootstrap;
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.channel.ChannelInboundHandlerAdapter;
@@ -68,7 +67,7 @@ final class ReconnectPromise<S extends NetconfSession, L extends NetconfSessionL
 
     ReconnectPromise(final EventExecutor executor, final AbstractNetconfDispatcher<S, L> dispatcher,
             final InetSocketAddress address, final ReconnectStrategyFactory connectStrategyFactory,
-            final Bootstrap bootstrap, final AbstractNetconfDispatcher.PipelineInitializer<S> initializer) {
+            final Bootstrap bootstrap, final PipelineInitializer<S> initializer) {
         super(executor);
         this.firstSessionFuture = new DefaultPromise<>(executor);
         this.bootstrap = requireNonNull(bootstrap);
@@ -99,11 +98,11 @@ final class ReconnectPromise<S extends NetconfSession, L extends NetconfSessionL
 
     @Holding("this")
     private void lockedConnect() {
-        final ReconnectStrategy cs = this.strategyFactory.createReconnectStrategy();
+        final ReconnectStrategy cs = strategyFactory.createReconnectStrategy();
 
         // Set up a client with pre-configured bootstrap, but add a closed channel handler into the pipeline to support
         // reconnect attempts
-        pending = this.dispatcher.createClient(this.address, cs, bootstrap, (channel, promise) -> {
+        pending = dispatcher.createClient(address, cs, bootstrap, (channel, promise) -> {
             initializer.initializeChannel(channel, promise);
             // add closed channel handler
             // This handler has to be added as last channel handler and the channel inactive event has to be caught by
@@ -112,7 +111,7 @@ final class ReconnectPromise<S extends NetconfSession, L extends NetconfSessionL
             // reconnect will not work
             // This handler is last so all handlers in front of it can handle channel inactive (to e.g. resource
             // cleanup) before a new connection is started
-            channel.pipeline().addLast(new ClosedChannelHandler(ReconnectPromise.this));
+            channel.pipeline().addLast(inboundHandler);
         });
 
         if (!firstSessionFuture.isDone()) {
@@ -123,44 +122,4 @@ final class ReconnectPromise<S extends NetconfSession, L extends NetconfSessionL
             });
         }
     }
-
-    /**
-     * Indicate if the initial connection succeeded.
-     *
-     * @return true if initial connection was established successfully, false if initial connection failed due to e.g.
-     *         Connection refused, Negotiation failed
-     */
-    @SuppressFBWarnings(value = "UPM_UNCALLED_PRIVATE_METHOD",
-            justification = "https://github.com/spotbugs/spotbugs/issues/811")
-    private synchronized boolean isInitialConnectFinished() {
-        requireNonNull(pending);
-        return pending.isDone() && pending.isSuccess();
-    }
-
-    /**
-     * Channel handler that responds to channelInactive event and reconnects the session.
-     * Only if the promise was not canceled.
-     */
-    private static final class ClosedChannelHandler extends ChannelInboundHandlerAdapter {
-        private final ReconnectPromise<?, ?> promise;
-
-        ClosedChannelHandler(final ReconnectPromise<?, ?> promise) {
-            this.promise = promise;
-        }
-
-        @Override
-        public void channelInactive(final ChannelHandlerContext ctx) {
-            // This is the ultimate channel inactive handler, not forwarding
-            if (promise.isCancelled()) {
-                return;
-            }
-
-            if (promise.isInitialConnectFinished() == false) {
-                LOG.debug("Connection to {} was dropped during negotiation, reattempting", promise.address);
-            }
-
-            LOG.debug("Reconnecting after connection to {} was dropped", promise.address);
-            promise.connect();
-        }
-    }
 }