Do not needlessly bridge promises

Channel has an alternative writeAndFlush() method, which takes
a promise to complete. Using it saves allocation of two objects
and required listener synchronization.

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

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..0b9bba0f6f76d9abc61bdbf33220bfe4efdfb66e 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
@@ -23,12 +23,12 @@ import static org.mockito.Mockito.verify;
 import com.google.common.base.Optional;
 import com.google.common.collect.ImmutableSet;
 import io.netty.channel.Channel;
-import io.netty.channel.ChannelFuture;
 import io.netty.channel.ChannelHandler;
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.channel.ChannelInboundHandlerAdapter;
 import io.netty.channel.ChannelPipeline;
 import io.netty.channel.ChannelProgressivePromise;
+import io.netty.channel.ChannelPromise;
 import io.netty.channel.EventLoop;
 import io.netty.handler.ssl.SslHandler;
 import io.netty.util.HashedWheelTimer;
@@ -40,13 +40,11 @@ import java.util.Set;
 import org.junit.Before;
 import org.junit.Test;
 import org.mockito.internal.util.collections.Sets;
-import org.mockito.invocation.InvocationOnMock;
-import org.mockito.stubbing.Answer;
 import org.opendaylight.controller.config.util.xml.XmlUtil;
 import org.opendaylight.netconf.api.NetconfClientSessionPreferences;
+import org.opendaylight.netconf.api.NetconfDocumentedException;
 import org.opendaylight.netconf.api.NetconfMessage;
 import org.opendaylight.netconf.api.messages.NetconfHelloMessage;
-import org.opendaylight.netconf.api.messages.NetconfHelloMessageAdditionalHeader;
 import org.opendaylight.netconf.nettyutil.handler.ChunkedFramingMechanismEncoder;
 import org.opendaylight.netconf.nettyutil.handler.NetconfXMLToHelloMessageDecoder;
 import org.opendaylight.netconf.nettyutil.handler.NetconfXMLToMessageDecoder;
@@ -60,14 +58,13 @@ public class NetconfClientSessionNegotiatorTest {
 
     private NetconfHelloMessage helloMessage;
     private ChannelPipeline pipeline;
-    private ChannelFuture future;
+    private ChannelPromise future;
     private Channel channel;
     private ChannelInboundHandlerAdapter channelInboundHandlerAdapter;
 
     @Before
     public void setUp() throws Exception {
-        helloMessage = NetconfHelloMessage.createClientHello(Sets.newSet("exi:1.0"), Optional
-                .<NetconfHelloMessageAdditionalHeader>absent());
+        helloMessage = NetconfHelloMessage.createClientHello(Sets.newSet("exi:1.0"), Optional.absent());
         pipeline = mockChannelPipeline();
         future = mockChannelFuture();
         channel = mockChannel();
@@ -83,8 +80,10 @@ public class NetconfClientSessionNegotiatorTest {
         Channel ret = mock(Channel.class);
         ChannelHandler channelHandler = mockChannelHandler();
         doReturn("").when(ret).toString();
+        doReturn(future).when(ret).newPromise();
         doReturn(future).when(ret).close();
         doReturn(future).when(ret).writeAndFlush(anyObject());
+        doReturn(future).when(ret).writeAndFlush(anyObject(), anyObject());
         doReturn(true).when(ret).isOpen();
         doReturn(pipeline).when(ret).pipeline();
         doReturn("").when(pipeline).toString();
@@ -93,8 +92,8 @@ public class NetconfClientSessionNegotiatorTest {
         return ret;
     }
 
-    private static ChannelFuture mockChannelFuture() {
-        ChannelFuture future = mock(ChannelFuture.class);
+    private static ChannelPromise mockChannelFuture() {
+        ChannelPromise future = mock(ChannelPromise.class);
         doReturn(future).when(future).addListener(any(GenericFutureListener.class));
         return future;
     }
@@ -118,14 +117,9 @@ public class NetconfClientSessionNegotiatorTest {
     private void mockEventLoop() {
         final EventLoop eventLoop = mock(EventLoop.class);
         doReturn(eventLoop).when(channel).eventLoop();
-        doAnswer(new Answer<Void>() {
-            @Override
-            public Void answer(InvocationOnMock invocation) throws Throwable {
-                final Object[] args = invocation.getArguments();
-                final Runnable runnable = (Runnable) args[0];
-                runnable.run();
-                return null;
-            }
+        doAnswer(invocation -> {
+            invocation.getArgumentAt(0, Runnable.class).run();
+            return null;
         }).when(eventLoop).execute(any(Runnable.class));
     }
 
@@ -142,21 +136,21 @@ public class NetconfClientSessionNegotiatorTest {
         return new NetconfClientSessionNegotiator(preferences, promise, channel, timer, sessionListener, timeout);
     }
 
-    private NetconfHelloMessage createHelloMsg(final String name) throws Exception {
+    private static NetconfHelloMessage createHelloMsg(final String name) throws Exception {
         final InputStream stream = NetconfClientSessionNegotiatorTest.class.getResourceAsStream(name);
         final Document doc = XmlUtil.readXmlToDocument(stream);
 
         return new NetconfHelloMessage(doc);
     }
 
-    private Set<String> createCapabilities(String name) throws Exception {
+    private static Set<String> createCapabilities(final String name) throws Exception {
         NetconfHelloMessage hello = createHelloMsg(name);
 
         return ImmutableSet.copyOf(NetconfMessageUtil.extractCapabilitiesFromHello(hello.getDocument()));
     }
 
     @Test
-    public void testNetconfClientSessionNegotiator() throws Exception {
+    public void testNetconfClientSessionNegotiator() throws NetconfDocumentedException {
         Promise<NetconfClientSession> promise = mock(Promise.class);
         doReturn(promise).when(promise).setSuccess(anyObject());
         NetconfClientSessionNegotiator negotiator = createNetconfClientSessionNegotiator(promise, null);
@@ -179,12 +173,9 @@ public class NetconfClientSessionNegotiatorTest {
         Set<String> caps = Sets.newSet("exi:1.0");
         NetconfHelloMessage message = NetconfHelloMessage.createServerHello(caps, 10);
 
-        doAnswer(new Answer<Object>() {
-            @Override
-            public Object answer(final InvocationOnMock invocationOnMock) throws Throwable {
-                channelInboundHandlerAdapter = ((ChannelInboundHandlerAdapter) invocationOnMock.getArguments()[2]);
-                return null;
-            }
+        doAnswer(invocationOnMock -> {
+            channelInboundHandlerAdapter = (ChannelInboundHandlerAdapter) invocationOnMock.getArguments()[2];
+            return null;
         }).when(pipeline).addAfter(anyString(), anyString(), any(ChannelHandler.class));
 
         ChannelHandlerContext handlerContext = mock(ChannelHandlerContext.class);

diff --git a/netconf/netconf-client/src/test/java/org/opendaylight/netconf/client/SimpleNetconfClientSessionListenerTest.java b/netconf/netconf-client/src/test/java/org/opendaylight/netconf/client/SimpleNetconfClientSessionListenerTest.java
index 1dde3d4dfa64f6f7b4151d807ec722557dd52c28..68c1a762f079d47bb062f64cfe4e58f877200f28 100644 (file)
--- a/netconf/netconf-client/src/test/java/org/opendaylight/netconf/client/SimpleNetconfClientSessionListenerTest.java
+++ b/netconf/netconf-client/src/test/java/org/opendaylight/netconf/client/SimpleNetconfClientSessionListenerTest.java
@@ -19,7 +19,7 @@ import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 
 import io.netty.channel.Channel;
-import io.netty.channel.ChannelFuture;
+import io.netty.channel.ChannelPromise;
 import io.netty.channel.EventLoop;
 import io.netty.util.concurrent.Future;
 import io.netty.util.concurrent.GenericFutureListener;
@@ -27,15 +27,14 @@ import java.util.Set;
 import org.junit.Before;
 import org.junit.Test;
 import org.mockito.internal.util.collections.Sets;
-import org.mockito.invocation.InvocationOnMock;
-import org.mockito.stubbing.Answer;
+import org.opendaylight.netconf.api.NetconfDocumentedException;
 import org.opendaylight.netconf.api.NetconfMessage;
 import org.opendaylight.netconf.api.messages.NetconfHelloMessage;
 
 public class SimpleNetconfClientSessionListenerTest {
 
     private Channel channel;
-    private ChannelFuture channelFuture;
+    private ChannelPromise channelFuture;
     Set<String> caps;
     private NetconfHelloMessage helloMessage;
     private NetconfMessage message;
@@ -43,11 +42,13 @@ public class SimpleNetconfClientSessionListenerTest {
     private NetconfClientSession clientSession;
 
     @Before
-    public void setUp() throws Exception {
+    public void setUp() throws NetconfDocumentedException {
         channel = mock(Channel.class);
-        channelFuture = mock(ChannelFuture.class);
+        channelFuture = mock(ChannelPromise.class);
         mockEventLoop();
+        doReturn(channelFuture).when(channel).newPromise();
         doReturn(channelFuture).when(channel).writeAndFlush(anyObject());
+        doReturn(channelFuture).when(channel).writeAndFlush(anyObject(), any(ChannelPromise.class));
         doReturn(channelFuture).when(channelFuture).addListener(any(GenericFutureListener.class));
         caps = Sets.newSet("a", "b");
         helloMessage = NetconfHelloMessage.createServerHello(caps, 10);
@@ -59,14 +60,9 @@ public class SimpleNetconfClientSessionListenerTest {
     private void mockEventLoop() {
         final EventLoop eventLoop = mock(EventLoop.class);
         doReturn(eventLoop).when(channel).eventLoop();
-        doAnswer(new Answer<Void>() {
-            @Override
-            public Void answer(InvocationOnMock invocation) throws Throwable {
-                final Object[] args = invocation.getArguments();
-                final Runnable runnable = (Runnable) args[0];
-                runnable.run();
-                return null;
-            }
+        doAnswer(invocation -> {
+            invocation.getArgumentAt(0, Runnable.class).run();
+            return null;
         }).when(eventLoop).execute(any(Runnable.class));
     }
 
@@ -75,29 +71,29 @@ public class SimpleNetconfClientSessionListenerTest {
         SimpleNetconfClientSessionListener simpleListener = new SimpleNetconfClientSessionListener();
         final Future<NetconfMessage> promise = simpleListener.sendRequest(message);
         simpleListener.onSessionUp(clientSession);
-        verify(channel, times(1)).writeAndFlush(anyObject());
+        verify(channel, times(1)).writeAndFlush(anyObject(), anyObject());
 
         simpleListener.onSessionDown(clientSession, new Exception());
         assertFalse(promise.isSuccess());
     }
 
     @Test
-    public void testSendRequest() throws Exception {
+    public void testSendRequest() {
         SimpleNetconfClientSessionListener simpleListener = new SimpleNetconfClientSessionListener();
         final Future<NetconfMessage> promise = simpleListener.sendRequest(message);
         simpleListener.onSessionUp(clientSession);
-        verify(channel, times(1)).writeAndFlush(anyObject());
+        verify(channel, times(1)).writeAndFlush(anyObject(), anyObject());
 
         simpleListener.sendRequest(message);
         assertFalse(promise.isSuccess());
     }
 
     @Test
-    public void testOnMessage() throws Exception {
+    public void testOnMessage() {
         SimpleNetconfClientSessionListener simpleListener = new SimpleNetconfClientSessionListener();
         final Future<NetconfMessage> promise = simpleListener.sendRequest(message);
         simpleListener.onSessionUp(clientSession);
-        verify(channel, times(1)).writeAndFlush(anyObject());
+        verify(channel, times(1)).writeAndFlush(anyObject(), anyObject());
 
         simpleListener.onMessage(clientSession, message);
         assertTrue(promise.isSuccess());

diff --git a/netconf/netconf-impl/src/test/java/org/opendaylight/netconf/impl/mapping/operations/DefaultCloseSessionTest.java b/netconf/netconf-impl/src/test/java/org/opendaylight/netconf/impl/mapping/operations/DefaultCloseSessionTest.java
index 2566e070224e0fdffe987e1462cee71412e02227..79247b45e1cb58c1d41b6ee6697eb06b6f26a0d2 100644 (file)
--- a/netconf/netconf-impl/src/test/java/org/opendaylight/netconf/impl/mapping/operations/DefaultCloseSessionTest.java
+++ b/netconf/netconf-impl/src/test/java/org/opendaylight/netconf/impl/mapping/operations/DefaultCloseSessionTest.java
@@ -19,6 +19,7 @@ import static org.mockito.Mockito.verify;
 
 import io.netty.channel.Channel;
 import io.netty.channel.ChannelFuture;
+import io.netty.channel.ChannelPromise;
 import io.netty.channel.EventLoop;
 import io.netty.util.concurrent.GenericFutureListener;
 import org.junit.Test;
@@ -39,9 +40,7 @@ public class DefaultCloseSessionTest {
         final EventLoop eventLoop = mock(EventLoop.class);
         doReturn(eventLoop).when(channel).eventLoop();
         doAnswer(invocation -> {
-            final Object[] args = invocation.getArguments();
-            final Runnable runnable = (Runnable) args[0];
-            runnable.run();
+            invocation.getArgumentAt(0, Runnable.class).run();
             return null;
         }).when(eventLoop).execute(any(Runnable.class));
         doReturn(true).when(eventLoop).inEventLoop();
@@ -61,12 +60,13 @@ public class DefaultCloseSessionTest {
         doReturn(channelFuture).when(channel).close();
         doReturn(channelFuture).when(channelFuture).addListener(any(GenericFutureListener.class));
 
-        final ChannelFuture sendFuture = mock(ChannelFuture.class);
+        final ChannelPromise sendFuture = mock(ChannelPromise.class);
         doAnswer(invocation -> {
-            ((GenericFutureListener) invocation.getArguments()[0]).operationComplete(sendFuture);
+            invocation.getArgumentAt(0, GenericFutureListener.class).operationComplete(sendFuture);
             return null;
         }).when(sendFuture).addListener(any(GenericFutureListener.class));
-        doReturn(sendFuture).when(channel).writeAndFlush(anyObject());
+        doReturn(sendFuture).when(channel).newPromise();
+        doReturn(sendFuture).when(channel).writeAndFlush(anyObject(), anyObject());
         doReturn(true).when(sendFuture).isSuccess();
         final NetconfServerSessionListener listener = mock(NetconfServerSessionListener.class);
         doNothing().when(listener).onSessionTerminated(any(NetconfServerSession.class),
@@ -90,8 +90,7 @@ public class DefaultCloseSessionTest {
         AutoCloseable res = mock(AutoCloseable.class);
         doThrow(NetconfDocumentedException.class).when(res).close();
         DefaultCloseSession session = new DefaultCloseSession("", res);
-        Document doc = XmlUtil.newDocument();
         XmlElement elem = XmlElement.fromDomElement(XmlUtil.readXmlToElement("<elem/>"));
-        session.handleWithNoSubsequentOperations(doc, elem);
+        session.handleWithNoSubsequentOperations(XmlUtil.newDocument(), elem);
     }
 }

diff --git a/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/AbstractNetconfSession.java b/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/AbstractNetconfSession.java
index fdcd7e6479a91f8ef2b69b7ec3ec771730f4e701..8b070a0b6653df3a107863b9211b0c8f349f3d13 100644 (file)
--- a/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/AbstractNetconfSession.java
+++ b/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/AbstractNetconfSession.java
@@ -12,11 +12,9 @@ import com.siemens.ct.exi.exceptions.UnsupportedOption;
 import io.netty.channel.Channel;
 import io.netty.channel.ChannelFuture;
 import io.netty.channel.ChannelHandler;
-import io.netty.channel.DefaultChannelPromise;
+import io.netty.channel.ChannelPromise;
 import io.netty.handler.codec.ByteToMessageDecoder;
 import io.netty.handler.codec.MessageToByteEncoder;
-import io.netty.util.concurrent.Future;
-import io.netty.util.concurrent.FutureListener;
 import java.io.IOException;
 import org.opendaylight.controller.config.util.xml.XmlElement;
 import org.opendaylight.netconf.api.NetconfExiSession;
@@ -75,29 +73,17 @@ public abstract class AbstractNetconfSession<S extends NetconfSession,L extends
         // Restconf writes to a netconf mountpoint execute multiple messages
         // and one of these was executed from a restconf thread thus breaking ordering so
         // we need to execute all messages from an EventLoop thread.
-        final DefaultChannelPromise proxyFuture = new DefaultChannelPromise(channel);
-        channel.eventLoop().execute(new Runnable() {
-            @Override
-            public void run() {
-                final ChannelFuture future = channel.writeAndFlush(netconfMessage);
-                future.addListener(new FutureListener<Void>() {
-                    @Override
-                    public void operationComplete(final Future<Void> future) throws Exception {
-                        if (future.isSuccess()) {
-                            proxyFuture.setSuccess();
-                        } else {
-                            proxyFuture.setFailure(future.cause());
-                        }
-                    }
-                });
-                if (delayedEncoder != null) {
-                    replaceMessageEncoder(delayedEncoder);
-                    delayedEncoder = null;
-                }
+
+        final ChannelPromise promise = channel.newPromise();
+        channel.eventLoop().execute(() -> {
+            channel.writeAndFlush(netconfMessage, promise);
+            if (delayedEncoder != null) {
+                replaceMessageEncoder(delayedEncoder);
+                delayedEncoder = null;
             }
         });
 
-        return proxyFuture;
+        return promise;
     }
 
     @Override

diff --git a/netconf/netconf-netty-util/src/test/java/org/opendaylight/netconf/nettyutil/AbstractNetconfSessionTest.java b/netconf/netconf-netty-util/src/test/java/org/opendaylight/netconf/nettyutil/AbstractNetconfSessionTest.java
index c372ecb34c7b17e0b9d74752c60d99407b4c75e5..3626fc008bd453a2fc80f83e81dd25aefe559c37 100644 (file)
--- a/netconf/netconf-netty-util/src/test/java/org/opendaylight/netconf/nettyutil/AbstractNetconfSessionTest.java
+++ b/netconf/netconf-netty-util/src/test/java/org/opendaylight/netconf/nettyutil/AbstractNetconfSessionTest.java
@@ -26,6 +26,7 @@ import io.netty.channel.Channel;
 import io.netty.channel.ChannelFuture;
 import io.netty.channel.ChannelHandler;
 import io.netty.channel.ChannelPipeline;
+import io.netty.channel.ChannelPromise;
 import io.netty.channel.EventLoop;
 import io.netty.handler.codec.ByteToMessageDecoder;
 import io.netty.handler.codec.MessageToByteEncoder;
@@ -35,13 +36,10 @@ import org.junit.Before;
 import org.junit.Test;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
-import org.mockito.invocation.InvocationOnMock;
-import org.mockito.stubbing.Answer;
 import org.opendaylight.netconf.api.NetconfMessage;
 import org.opendaylight.netconf.api.NetconfSessionListener;
 import org.opendaylight.netconf.api.NetconfTerminationReason;
 import org.opendaylight.netconf.api.messages.NetconfHelloMessage;
-import org.opendaylight.netconf.api.messages.NetconfHelloMessageAdditionalHeader;
 import org.opendaylight.netconf.nettyutil.handler.exi.EXIParameters;
 import org.opendaylight.netconf.nettyutil.handler.exi.NetconfStartExiMessage;
 
@@ -56,7 +54,7 @@ public class AbstractNetconfSessionTest {
     @Mock
     private EventLoop eventLoop;
     @Mock
-    private ChannelFuture writeFuture;
+    private ChannelPromise writeFuture;
 
     private NetconfHelloMessage clientHello;
 
@@ -71,7 +69,9 @@ public class AbstractNetconfSessionTest {
 
         doReturn(writeFuture).when(writeFuture).addListener(any(GenericFutureListener.class));
 
+        doReturn(writeFuture).when(channel).newPromise();
         doReturn(writeFuture).when(channel).writeAndFlush(any(NetconfMessage.class));
+        doReturn(writeFuture).when(channel).writeAndFlush(any(NetconfMessage.class), any(ChannelPromise.class));
         doReturn(pipeline).when(channel).pipeline();
         doReturn("mockChannel").when(channel).toString();
         doReturn(mock(ChannelFuture.class)).when(channel).close();
@@ -79,18 +79,12 @@ public class AbstractNetconfSessionTest {
         doReturn(null).when(pipeline).replace(anyString(), anyString(), any(ChannelHandler.class));
 
         doReturn(eventLoop).when(channel).eventLoop();
-        doAnswer(new Answer<Void>() {
-            @Override
-            public Void answer(final InvocationOnMock invocation) throws Throwable {
-                final Object[] args = invocation.getArguments();
-                final Runnable runnable = (Runnable) args[0];
-                runnable.run();
-                return null;
-            }
+        doAnswer(invocation -> {
+            invocation.getArgumentAt(0, Runnable.class).run();
+            return null;
         }).when(eventLoop).execute(any(Runnable.class));
 
-        clientHello = NetconfHelloMessage.createClientHello(Collections.<String>emptySet(),
-                Optional.<NetconfHelloMessageAdditionalHeader>absent());
+        clientHello = NetconfHelloMessage.createClientHello(Collections.emptySet(), Optional.absent());
     }
 
     @Test
@@ -159,10 +153,10 @@ public class AbstractNetconfSessionTest {
     @Test
     public void testSendMessage() throws Exception {
         final TestingNetconfSession testingNetconfSession = new TestingNetconfSession(listener, channel, 1L);
-        final NetconfHelloMessage hello = NetconfHelloMessage.createClientHello(Collections.<String>emptySet(),
-                Optional.<NetconfHelloMessageAdditionalHeader>absent());
+        final NetconfHelloMessage hello = NetconfHelloMessage.createClientHello(Collections.emptySet(),
+            Optional.absent());
         testingNetconfSession.sendMessage(hello);
-        verify(channel).writeAndFlush(hello);
+        verify(channel).writeAndFlush(hello, writeFuture);
     }
 
 }