From: Robert Varga Date: Sat, 31 Mar 2018 15:31:00 +0000 (+0200) Subject: Do not needlessly bridge promises X-Git-Tag: release/fluorine~104 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=81249f9abe3c66a4ae0a819ac3ac821565950265;hp=a3022c29beea055ec8e7cacaa30dd64d5a80887c;p=netconf.git 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 --- 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 79b13a233c..0b9bba0f6f 100644 --- 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 - .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() { - @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 createCapabilities(String name) throws Exception { + private static Set 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 promise = mock(Promise.class); doReturn(promise).when(promise).setSuccess(anyObject()); NetconfClientSessionNegotiator negotiator = createNetconfClientSessionNegotiator(promise, null); @@ -179,12 +173,9 @@ public class NetconfClientSessionNegotiatorTest { Set caps = Sets.newSet("exi:1.0"); NetconfHelloMessage message = NetconfHelloMessage.createServerHello(caps, 10); - doAnswer(new Answer() { - @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 1dde3d4dfa..68c1a762f0 100644 --- 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 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() { - @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 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 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 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 2566e07022..79247b45e1 100644 --- 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("")); - 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 fdcd7e6479..8b070a0b66 100644 --- 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() { - @Override - public void operationComplete(final Future 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 c372ecb34c..3626fc008b 100644 --- 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() { - @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.emptySet(), - Optional.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.emptySet(), - Optional.absent()); + final NetconfHelloMessage hello = NetconfHelloMessage.createClientHello(Collections.emptySet(), + Optional.absent()); testingNetconfSession.sendMessage(hello); - verify(channel).writeAndFlush(hello); + verify(channel).writeAndFlush(hello, writeFuture); } }