Improve NetconfDeviceCommunicator error checking

Use Future.cause() to discern failures -- meaning we have a non-null
cause in the error path.

Change-Id: Ifb88cfcf666d1aea99c09189992d80224c0862e1
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/plugins/netconf-client-mdsal/src/main/java/org/opendaylight/netconf/client/mdsal/NetconfDeviceCommunicator.java b/plugins/netconf-client-mdsal/src/main/java/org/opendaylight/netconf/client/mdsal/NetconfDeviceCommunicator.java
index a356f0f7a63d9a7374858a774627d9b7153bceff..4e9d7008789e0983c82313367545b3fe9d369fb4 100644 (file)
--- a/plugins/netconf-client-mdsal/src/main/java/org/opendaylight/netconf/client/mdsal/NetconfDeviceCommunicator.java
+++ b/plugins/netconf-client-mdsal/src/main/java/org/opendaylight/netconf/client/mdsal/NetconfDeviceCommunicator.java
@@ -384,21 +384,14 @@ public class NetconfDeviceCommunicator implements NetconfClientSessionListener,
         requests.add(req);
 
         currentSession.sendMessage(req.request).addListener(future -> {
-            if (!future.isSuccess()) {
+            final var cause = future.cause();
+            if (cause != null) {
                 // We expect that a session down will occur at this point
-                final var cause = future.cause();
                 if (LOG.isDebugEnabled()) {
                     LOG.debug("{}: Failed to send request {}", id, XmlUtil.toString(req.request.getDocument()), cause);
                 }
 
-                final RpcResult<NetconfMessage> result;
-                if (cause == null) {
-                    // assume session is down
-                    result = createSessionDownRpcResult();
-                } else {
-                    result = createErrorRpcResult(ErrorType.TRANSPORT, cause.getLocalizedMessage());
-                }
-                req.future.set(result);
+                req.future.set(createErrorRpcResult(ErrorType.TRANSPORT, cause.getLocalizedMessage()));
             } else {
                 LOG.trace("Finished sending request {}", req.request);
             }

diff --git a/plugins/netconf-client-mdsal/src/test/java/org/opendaylight/netconf/client/mdsal/NetconfDeviceCommunicatorTest.java b/plugins/netconf-client-mdsal/src/test/java/org/opendaylight/netconf/client/mdsal/NetconfDeviceCommunicatorTest.java
index 256b70cab23f4f61ac56a0540aeecd3dead92d76..769a26deb45400e8ab281980dd5272281d619134 100644 (file)
--- a/plugins/netconf-client-mdsal/src/test/java/org/opendaylight/netconf/client/mdsal/NetconfDeviceCommunicatorTest.java
+++ b/plugins/netconf-client-mdsal/src/test/java/org/opendaylight/netconf/client/mdsal/NetconfDeviceCommunicatorTest.java
@@ -24,6 +24,7 @@ import static org.mockito.Mockito.verify;
 
 import com.google.common.base.CharMatcher;
 import com.google.common.base.Strings;
+import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
 import io.netty.channel.Channel;
 import io.netty.channel.ChannelFuture;
@@ -34,8 +35,6 @@ import java.net.InetSocketAddress;
 import java.util.ArrayList;
 import java.util.Set;
 import java.util.UUID;
-import java.util.concurrent.TimeUnit;
-import java.util.concurrent.TimeoutException;
 import javax.xml.parsers.ParserConfigurationException;
 import org.junit.Before;
 import org.junit.Test;
@@ -63,14 +62,11 @@ import org.opendaylight.yangtools.yang.common.QName;
 import org.opendaylight.yangtools.yang.common.RpcError;
 import org.opendaylight.yangtools.yang.common.RpcResult;
 import org.opendaylight.yangtools.yang.common.Uint32;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 import org.w3c.dom.Document;
 import org.w3c.dom.Element;
 
 @RunWith(MockitoJUnitRunner.StrictStubs.class)
 public class NetconfDeviceCommunicatorTest {
-    private static final Logger LOG = LoggerFactory.getLogger(NetconfDeviceCommunicatorTest.class);
     private static final SessionIdType SESSION_ID = new SessionIdType(Uint32.ONE);
 
     @Mock
@@ -195,7 +191,7 @@ public class NetconfDeviceCommunicatorTest {
         verify(mockDevice, never()).onRemoteSessionDown();
     }
 
-    @SuppressWarnings({"rawtypes", "unchecked"})
+    @SuppressWarnings("unchecked")
     @Test
     public void testSendRequest() throws Exception {
         setupSession();
@@ -203,8 +199,7 @@ public class NetconfDeviceCommunicatorTest {
         NetconfMessage message = new NetconfMessage(UntrustedXML.newDocumentBuilder().newDocument());
         QName rpc = QName.create("", "mockRpc");
 
-        ArgumentCaptor<GenericFutureListener> futureListener =
-                ArgumentCaptor.forClass(GenericFutureListener.class);
+        final var futureListener = ArgumentCaptor.forClass(GenericFutureListener.class);
 
         ChannelFuture mockChannelFuture = mock(ChannelFuture.class);
         doReturn(mockChannelFuture).when(mockChannelFuture).addListener(futureListener.capture());
@@ -218,14 +213,11 @@ public class NetconfDeviceCommunicatorTest {
 
         verify(mockChannelFuture).addListener(futureListener.capture());
         Future<Void> operationFuture = mock(Future.class);
-        doReturn(true).when(operationFuture).isSuccess();
+        doReturn(null).when(operationFuture).cause();
         futureListener.getValue().operationComplete(operationFuture);
 
-        try {
-            resultFuture.get(1, TimeUnit.MILLISECONDS); // verify it's not cancelled or has an error set
-        } catch (TimeoutException e) {
-            LOG.info("Operation failed due timeout.");
-        } // expected
+        // verify it is not cancelled nor has an error set
+        assertFalse(resultFuture.isDone());
     }
 
     @Test
@@ -238,7 +230,7 @@ public class NetconfDeviceCommunicatorTest {
         assertNotNull("ListenableFuture is null", resultFuture);
 
         // Should have an immediate result
-        RpcResult<NetconfMessage> rpcResult = resultFuture.get(3, TimeUnit.MILLISECONDS);
+        RpcResult<NetconfMessage> rpcResult = Futures.getDone(resultFuture);
 
         verifyErrorRpcResult(rpcResult, ErrorType.TRANSPORT, ErrorTag.OPERATION_FAILED);
     }
@@ -256,7 +248,7 @@ public class NetconfDeviceCommunicatorTest {
         return new NetconfMessage(doc);
     }
 
-    @SuppressWarnings({ "rawtypes", "unchecked" })
+    @SuppressWarnings("unchecked")
     @Test
     public void testSendRequestWithWithSendFailure() throws Exception {
         setupSession();
@@ -264,8 +256,7 @@ public class NetconfDeviceCommunicatorTest {
         NetconfMessage message = new NetconfMessage(UntrustedXML.newDocumentBuilder().newDocument());
         QName rpc = QName.create("", "mockRpc");
 
-        ArgumentCaptor<GenericFutureListener> futureListener =
-                ArgumentCaptor.forClass(GenericFutureListener.class);
+        final var futureListener = ArgumentCaptor.forClass(GenericFutureListener.class);
 
         ChannelFuture mockChannelFuture = mock(ChannelFuture.class);
         doReturn(mockChannelFuture).when(mockChannelFuture).addListener(futureListener.capture());
@@ -278,12 +269,11 @@ public class NetconfDeviceCommunicatorTest {
         verify(mockChannelFuture).addListener(futureListener.capture());
 
         Future<Void> operationFuture = mock(Future.class);
-        doReturn(false).when(operationFuture).isSuccess();
         doReturn(new Exception("mock error")).when(operationFuture).cause();
         futureListener.getValue().operationComplete(operationFuture);
 
         // Should have an immediate result
-        RpcResult<NetconfMessage> rpcResult = resultFuture.get(3, TimeUnit.MILLISECONDS);
+        RpcResult<NetconfMessage> rpcResult = Futures.getDone(resultFuture);
 
         RpcError rpcError = verifyErrorRpcResult(rpcResult, ErrorType.TRANSPORT, ErrorTag.OPERATION_FAILED);
         assertEquals("RpcError message contains \"mock error\"", true,