From: Maros Marsalek <mmarsale@cisco.com>
Date: Wed, 8 Apr 2015 07:08:40 +0000 (+0200)
Subject: Remove isCloseMsg check for each rpc
X-Git-Tag: release/lithium~274^2
X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=9049093a8ddb4278fb98fcf0dc5d1914d1617004

Remove isCloseMsg check for each rpc

Performance improvement

The check re-read the message and checked if its close-session rpc.
DelayedClose method was added to netconf server session that replaces the
isCloseSession method with a simple bollean check.

Change-Id: Ic4f6d473b948bd4f63771748dac793ba43693828
Signed-off-by: Maros Marsalek <mmarsale@cisco.com>
---

diff --git a/opendaylight/netconf/config-netconf-connector/src/test/java/org/opendaylight/controller/netconf/confignetconfconnector/NetconfMappingTest.java b/opendaylight/netconf/config-netconf-connector/src/test/java/org/opendaylight/controller/netconf/confignetconfconnector/NetconfMappingTest.java
index e1ffcdddff..47bdcd8cc8 100644
--- a/opendaylight/netconf/config-netconf-connector/src/test/java/org/opendaylight/controller/netconf/confignetconfconnector/NetconfMappingTest.java
+++ b/opendaylight/netconf/config-netconf-connector/src/test/java/org/opendaylight/controller/netconf/confignetconfconnector/NetconfMappingTest.java
@@ -30,6 +30,7 @@ import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
+import io.netty.channel.Channel;
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.InputStream;
@@ -94,12 +95,15 @@ import org.opendaylight.controller.netconf.confignetconfconnector.operations.run
 import org.opendaylight.controller.netconf.confignetconfconnector.osgi.YangStoreContext;
 import org.opendaylight.controller.netconf.confignetconfconnector.osgi.YangStoreService;
 import org.opendaylight.controller.netconf.confignetconfconnector.transactions.TransactionProvider;
+import org.opendaylight.controller.netconf.impl.NetconfServerSession;
+import org.opendaylight.controller.netconf.impl.NetconfServerSessionListener;
 import org.opendaylight.controller.netconf.impl.mapping.operations.DefaultCloseSession;
 import org.opendaylight.controller.netconf.impl.osgi.AggregatedNetconfOperationServiceFactory;
 import org.opendaylight.controller.netconf.impl.osgi.NetconfOperationRouter;
 import org.opendaylight.controller.netconf.mapping.api.HandlingPriority;
 import org.opendaylight.controller.netconf.mapping.api.NetconfOperation;
 import org.opendaylight.controller.netconf.mapping.api.NetconfOperationChainedExecution;
+import org.opendaylight.controller.netconf.util.messages.NetconfHelloMessageAdditionalHeader;
 import org.opendaylight.controller.netconf.util.messages.NetconfMessageUtil;
 import org.opendaylight.controller.netconf.util.test.XmlFileLoader;
 import org.opendaylight.controller.netconf.util.xml.XmlUtil;
@@ -387,7 +391,14 @@ public class NetconfMappingTest extends AbstractConfigTest {
 
     private void closeSession() throws NetconfDocumentedException, ParserConfigurationException, SAXException,
             IOException {
+        final Channel channel = mock(Channel.class);
+        doReturn("channel").when(channel).toString();
+        final NetconfServerSessionListener listener = mock(NetconfServerSessionListener.class);
+        final NetconfServerSession session =
+                new NetconfServerSession(listener, channel, 1L,
+                        NetconfHelloMessageAdditionalHeader.fromString("[netconf;10.12.0.102:48528;ssh;;;;;;]"));
         DefaultCloseSession closeOp = new DefaultCloseSession(NETCONF_SESSION_ID, sessionCloseable);
+        closeOp.setNetconfSession(session);
         executeOp(closeOp, "netconfMessages/closeSession.xml");
     }
 
diff --git a/opendaylight/netconf/netconf-impl/src/main/java/org/opendaylight/controller/netconf/impl/NetconfServerSession.java b/opendaylight/netconf/netconf-impl/src/main/java/org/opendaylight/controller/netconf/impl/NetconfServerSession.java
index 0cf2dbc281..4368f7ec79 100644
--- a/opendaylight/netconf/netconf-impl/src/main/java/org/opendaylight/controller/netconf/impl/NetconfServerSession.java
+++ b/opendaylight/netconf/netconf-impl/src/main/java/org/opendaylight/controller/netconf/impl/NetconfServerSession.java
@@ -10,6 +10,8 @@ package org.opendaylight.controller.netconf.impl;
 
 import com.google.common.base.Preconditions;
 import io.netty.channel.Channel;
+import io.netty.channel.ChannelFuture;
+import io.netty.channel.ChannelFutureListener;
 import io.netty.handler.codec.ByteToMessageDecoder;
 import io.netty.handler.codec.MessageToByteEncoder;
 import java.text.SimpleDateFormat;
@@ -45,6 +47,7 @@ public final class NetconfServerSession extends AbstractNetconfSession<NetconfSe
 
     private Date loginTime;
     private long inRpcSuccess, inRpcFail, outRpcError;
+    private volatile boolean delayedClose;
 
     public NetconfServerSession(final NetconfServerSessionListener sessionListener, final Channel channel, final long sessionId,
             final NetconfHelloMessageAdditionalHeader header) {
@@ -60,6 +63,29 @@ public final class NetconfServerSession extends AbstractNetconfSession<NetconfSe
         super.sessionUp();
     }
 
+    /**
+     * Close this session after next message is sent.
+     * Suitable for close rpc that needs to send ok response before the session is closed.
+     */
+    public void delayedClose() {
+        this.delayedClose = true;
+    }
+
+    @Override
+    public ChannelFuture sendMessage(final NetconfMessage netconfMessage) {
+        final ChannelFuture channelFuture = super.sendMessage(netconfMessage);
+        // delayed close was set, close after the message was sent
+        if(delayedClose) {
+            channelFuture.addListener(new ChannelFutureListener() {
+                @Override
+                public void operationComplete(final ChannelFuture future) throws Exception {
+                    close();
+                }
+            });
+        }
+        return channelFuture;
+    }
+
     public void onIncommingRpcSuccess() {
         inRpcSuccess++;
     }
diff --git a/opendaylight/netconf/netconf-impl/src/main/java/org/opendaylight/controller/netconf/impl/NetconfServerSessionListener.java b/opendaylight/netconf/netconf-impl/src/main/java/org/opendaylight/controller/netconf/impl/NetconfServerSessionListener.java
index a6531d3c63..e83e0b0227 100644
--- a/opendaylight/netconf/netconf-impl/src/main/java/org/opendaylight/controller/netconf/impl/NetconfServerSessionListener.java
+++ b/opendaylight/netconf/netconf-impl/src/main/java/org/opendaylight/controller/netconf/impl/NetconfServerSessionListener.java
@@ -17,10 +17,8 @@ import org.opendaylight.controller.netconf.api.NetconfSessionListener;
 import org.opendaylight.controller.netconf.api.NetconfTerminationReason;
 import org.opendaylight.controller.netconf.api.monitoring.NetconfMonitoringService;
 import org.opendaylight.controller.netconf.api.xml.XmlNetconfConstants;
-import org.opendaylight.controller.netconf.impl.mapping.operations.DefaultCloseSession;
 import org.opendaylight.controller.netconf.impl.osgi.NetconfOperationRouter;
 import org.opendaylight.controller.netconf.util.messages.SendErrorExceptionUtil;
-import org.opendaylight.controller.netconf.util.xml.XmlElement;
 import org.opendaylight.controller.netconf.util.xml.XmlUtil;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -89,11 +87,6 @@ public class NetconfServerSessionListener implements NetconfSessionListener<Netc
                     session);
             LOG.debug("Responding with message {}", message);
             session.sendMessage(message);
-
-            if (isCloseSession(netconfMessage)) {
-                closeNetconfSession(session);
-            }
-
         } catch (final RuntimeException e) {
             // TODO: should send generic error or close session?
             LOG.error("Unexpected exception", e);
@@ -107,14 +100,6 @@ public class NetconfServerSessionListener implements NetconfSessionListener<Netc
         }
     }
 
-    private void closeNetconfSession(final NetconfServerSession session) {
-        // destroy NetconfOperationService
-        session.close();
-        LOG.info("Session {} closed successfully", session.getSessionId());
-    }
-
-
-
     private NetconfMessage processDocument(final NetconfMessage netconfMessage, final NetconfServerSession session)
             throws NetconfDocumentedException {
 
@@ -166,14 +151,4 @@ public class NetconfServerSessionListener implements NetconfSessionListener<Netc
                 ImmutableMap.of(NetconfDocumentedException.ErrorTag.missing_attribute.toString(),
                         XmlNetconfConstants.MESSAGE_ID));
     }
-
-    private static boolean isCloseSession(final NetconfMessage incomingDocument) {
-        final Document document = incomingDocument.getDocument();
-        XmlElement rpcElement = XmlElement.fromDomDocument(document);
-        if (rpcElement.getOnlyChildElementOptionally(DefaultCloseSession.CLOSE_SESSION).isPresent()) {
-            return true;
-        }
-
-        return false;
-    }
 }
diff --git a/opendaylight/netconf/netconf-impl/src/main/java/org/opendaylight/controller/netconf/impl/mapping/operations/DefaultCloseSession.java b/opendaylight/netconf/netconf-impl/src/main/java/org/opendaylight/controller/netconf/impl/mapping/operations/DefaultCloseSession.java
index 8c7465b029..303352c9c5 100644
--- a/opendaylight/netconf/netconf-impl/src/main/java/org/opendaylight/controller/netconf/impl/mapping/operations/DefaultCloseSession.java
+++ b/opendaylight/netconf/netconf-impl/src/main/java/org/opendaylight/controller/netconf/impl/mapping/operations/DefaultCloseSession.java
@@ -9,18 +9,26 @@
 package org.opendaylight.controller.netconf.impl.mapping.operations;
 
 import com.google.common.base.Optional;
+import com.google.common.base.Preconditions;
 import java.util.Collections;
 import org.opendaylight.controller.netconf.api.NetconfDocumentedException;
 import org.opendaylight.controller.netconf.api.xml.XmlNetconfConstants;
+import org.opendaylight.controller.netconf.impl.NetconfServerSession;
 import org.opendaylight.controller.netconf.util.mapping.AbstractSingletonNetconfOperation;
 import org.opendaylight.controller.netconf.util.xml.XmlElement;
 import org.opendaylight.controller.netconf.util.xml.XmlUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.w3c.dom.Document;
 import org.w3c.dom.Element;
 
-public class DefaultCloseSession extends AbstractSingletonNetconfOperation {
+public class DefaultCloseSession extends AbstractSingletonNetconfOperation implements DefaultNetconfOperation {
+    private static final Logger LOG = LoggerFactory.getLogger(DefaultCloseSession.class);
+
     public static final String CLOSE_SESSION = "close-session";
+
     private final AutoCloseable sessionResources;
+    private NetconfServerSession session;
 
     public DefaultCloseSession(String netconfSessionIdForReporting, AutoCloseable sessionResources) {
         super(netconfSessionIdForReporting);
@@ -42,6 +50,8 @@ public class DefaultCloseSession extends AbstractSingletonNetconfOperation {
             throws NetconfDocumentedException {
         try {
             sessionResources.close();
+            Preconditions.checkNotNull(session, "Session was not set").delayedClose();
+            LOG.info("Session {} closing", session.getSessionId());
         } catch (Exception e) {
             throw new NetconfDocumentedException("Unable to properly close session "
                     + getNetconfSessionIdForReporting(), NetconfDocumentedException.ErrorType.application,
@@ -51,4 +61,9 @@ public class DefaultCloseSession extends AbstractSingletonNetconfOperation {
         }
         return XmlUtil.createElement(document, XmlNetconfConstants.OK, Optional.<String>absent());
     }
+
+    @Override
+    public void setNetconfSession(final NetconfServerSession s) {
+        this.session = s;
+    }
 }
diff --git a/opendaylight/netconf/netconf-impl/src/test/java/org/opendaylight/controller/netconf/impl/mapping/operations/DefaultCloseSessionTest.java b/opendaylight/netconf/netconf-impl/src/test/java/org/opendaylight/controller/netconf/impl/mapping/operations/DefaultCloseSessionTest.java
index d82c1484e4..b45b116b12 100644
--- a/opendaylight/netconf/netconf-impl/src/test/java/org/opendaylight/controller/netconf/impl/mapping/operations/DefaultCloseSessionTest.java
+++ b/opendaylight/netconf/netconf-impl/src/test/java/org/opendaylight/controller/netconf/impl/mapping/operations/DefaultCloseSessionTest.java
@@ -8,25 +8,67 @@
 
 package org.opendaylight.controller.netconf.impl.mapping.operations;
 
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.anyObject;
+import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
 
+import io.netty.channel.Channel;
+import io.netty.channel.ChannelFuture;
+import io.netty.util.concurrent.GenericFutureListener;
 import org.junit.Test;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;
 import org.opendaylight.controller.netconf.api.NetconfDocumentedException;
+import org.opendaylight.controller.netconf.api.NetconfMessage;
+import org.opendaylight.controller.netconf.api.NetconfTerminationReason;
+import org.opendaylight.controller.netconf.impl.NetconfServerSession;
+import org.opendaylight.controller.netconf.impl.NetconfServerSessionListener;
+import org.opendaylight.controller.netconf.util.messages.NetconfHelloMessageAdditionalHeader;
 import org.opendaylight.controller.netconf.util.xml.XmlElement;
 import org.opendaylight.controller.netconf.util.xml.XmlUtil;
 import org.w3c.dom.Document;
 
 public class DefaultCloseSessionTest {
+
     @Test
     public void testDefaultCloseSession() throws Exception {
         AutoCloseable res = mock(AutoCloseable.class);
         doNothing().when(res).close();
-        DefaultCloseSession session = new DefaultCloseSession("", res);
+        DefaultCloseSession close = new DefaultCloseSession("", res);
         Document doc = XmlUtil.newDocument();
         XmlElement elem = XmlElement.fromDomElement(XmlUtil.readXmlToElement("<elem/>"));
-        session.handleWithNoSubsequentOperations(doc, elem);
+        final Channel channel = mock(Channel.class);
+        doReturn("channel").when(channel).toString();
+        doReturn(mock(ChannelFuture.class)).when(channel).close();
+
+        final ChannelFuture sendFuture = mock(ChannelFuture.class);
+        doAnswer(new Answer() {
+            @Override
+            public Object answer(final InvocationOnMock invocation) throws Throwable {
+                ((GenericFutureListener) invocation.getArguments()[0]).operationComplete(sendFuture);
+                return null;
+            }
+        }).when(sendFuture).addListener(any(GenericFutureListener.class));
+        doReturn(sendFuture).when(channel).writeAndFlush(anyObject());
+        final NetconfServerSessionListener listener = mock(NetconfServerSessionListener.class);
+        doNothing().when(listener).onSessionTerminated(any(NetconfServerSession.class), any(NetconfTerminationReason.class));
+        final NetconfServerSession session =
+                new NetconfServerSession(listener, channel, 1L,
+                        NetconfHelloMessageAdditionalHeader.fromString("[netconf;10.12.0.102:48528;ssh;;;;;;]"));
+        close.setNetconfSession(session);
+        close.handleWithNoSubsequentOperations(doc, elem);
+        // Fake close response to trigger delayed close
+        session.sendMessage(new NetconfMessage(XmlUtil.readXmlToDocument("<rpc-reply message-id=\"101\"\n" +
+                "xmlns=\"urn:ietf:params:xml:ns:netconf:base:1.0\">\n" +
+                "<ok/>\n" +
+                "</rpc-reply>")));
+        verify(channel).close();
+        verify(listener).onSessionTerminated(any(NetconfServerSession.class), any(NetconfTerminationReason.class));
     }
 
     @Test(expected = NetconfDocumentedException.class)