fix failure during connecting device when channelActive happens later than handleMessage

fix bug NETCONF-533
--https://jira.opendaylight.org/browse/NETCONF-533

Change-Id: I46e4a85fc7be1bc221cfbb43386eae95768ac5ac
Signed-off-by: jiang.wei <351183359@qq.com>

diff --git a/netconf/netconf-client/src/main/java/org/opendaylight/netconf/client/NetconfClientSessionNegotiator.java b/netconf/netconf-client/src/main/java/org/opendaylight/netconf/client/NetconfClientSessionNegotiator.java
index 608450225f52d84d71cff3d1443b739905edaa5d..0b456f447f8ce33be11239ec8901dcffe0a041aa 100644 (file)
--- a/netconf/netconf-client/src/main/java/org/opendaylight/netconf/client/NetconfClientSessionNegotiator.java
+++ b/netconf/netconf-client/src/main/java/org/opendaylight/netconf/client/NetconfClientSessionNegotiator.java
@@ -63,8 +63,19 @@ public class NetconfClientSessionNegotiator extends
         super(sessionPreferences, promise, channel, timer, sessionListener, connectionTimeoutMillis);
     }
 
+    @SuppressWarnings("checkstyle:IllegalCatch")
     @Override
     protected void handleMessage(final NetconfHelloMessage netconfMessage) throws NetconfDocumentedException {
+        if (!ifNegotiatedAlready()) {
+            LOG.debug("Server hello message received, starting negotiation on channel {}", channel);
+            try {
+                startNegotiation();
+            } catch (final Exception e) {
+                LOG.warn("Unexpected negotiation failure", e);
+                negotiationFailed(e);
+                return;
+            }
+        }
         final NetconfClientSession session = getSessionForHelloMessage(netconfMessage);
         replaceHelloMessageInboundHandler(session);
 

diff --git a/netconf/netconf-client/src/test/java/org/opendaylight/netconf/client/NetconfClientSessionNegotiatorTest.java b/netconf/netconf-client/src/test/java/org/opendaylight/netconf/client/NetconfClientSessionNegotiatorTest.java
index 79b13a233c1aac2ea3fc229e2efeca786a4cae6f..12d4322ea081ba6625bb6fb3fde55a57f5495856 100644 (file)
--- a/netconf/netconf-client/src/test/java/org/opendaylight/netconf/client/NetconfClientSessionNegotiatorTest.java
+++ b/netconf/netconf-client/src/test/java/org/opendaylight/netconf/client/NetconfClientSessionNegotiatorTest.java
@@ -168,6 +168,22 @@ public class NetconfClientSessionNegotiatorTest {
         verify(promise).setSuccess(anyObject());
     }
 
+    @Test
+    public void testNegotiatorWhenChannelActiveHappenAfterHandleMessage() throws Exception {
+        Promise promise = mock(Promise.class);
+        doReturn(false).when(promise).isDone();
+        doReturn(promise).when(promise).setSuccess(anyObject());
+        NetconfClientSessionNegotiator negotiator = createNetconfClientSessionNegotiator(promise, null);
+        Set<String> caps = Sets.newSet("a", "b");
+        NetconfHelloMessage helloServerMessage = NetconfHelloMessage.createServerHello(caps, 10);
+
+        negotiator.handleMessage(helloServerMessage);
+        negotiator.channelActive(null);
+
+        verify(promise).setSuccess(anyObject());
+    }
+
+
     @Test
     public void testNetconfClientSessionNegotiatorWithEXI() throws Exception {
         Promise<NetconfClientSession> promise = mock(Promise.class);

diff --git a/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/AbstractNetconfSessionNegotiator.java b/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/AbstractNetconfSessionNegotiator.java
index a9adf2a668177af9dc0e5c165e028a96cea5def1..f8f486b2a0f63770b984e66a532ec332c7c9e7d0 100644 (file)
--- a/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/AbstractNetconfSessionNegotiator.java
+++ b/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/AbstractNetconfSessionNegotiator.java
@@ -79,22 +79,31 @@ public abstract class AbstractNetconfSessionNegotiator<P extends NetconfSessionP
 
     @Override
     protected final void startNegotiation() {
-        final Optional<SslHandler> sslHandler = getSslHandler(channel);
-        if (sslHandler.isPresent()) {
-            Future<Channel> future = sslHandler.get().handshakeFuture();
-            future.addListener(new GenericFutureListener<Future<? super Channel>>() {
-                @Override
-                public void operationComplete(final Future<? super Channel> future) {
-                    Preconditions.checkState(future.isSuccess(), "Ssl handshake was not successful");
-                    LOG.debug("Ssl handshake complete");
-                    start();
-                }
-            });
+        if (ifNegotiatedAlready()) {
+            LOG.debug("Negotiation on channel {} already started", channel);
         } else {
-            start();
+            final Optional<SslHandler> sslHandler = getSslHandler(channel);
+            if (sslHandler.isPresent()) {
+                Future<Channel> future = sslHandler.get().handshakeFuture();
+                future.addListener(new GenericFutureListener<Future<? super Channel>>() {
+                    @Override
+                    public void operationComplete(final Future<? super Channel> future) {
+                        Preconditions.checkState(future.isSuccess(), "Ssl handshake was not successful");
+                        LOG.debug("Ssl handshake complete");
+                        start();
+                    }
+                });
+            } else {
+                start();
+            }
         }
     }
 
+    protected final boolean ifNegotiatedAlready() {
+        // Indicates whether negotiation already started
+        return this.state != State.IDLE;
+    }
+
     private static Optional<SslHandler> getSslHandler(final Channel channel) {
         final SslHandler sslHandler = channel.pipeline().get(SslHandler.class);
         return sslHandler == null ? Optional.<SslHandler>absent() : Optional.of(sslHandler);