Cleanup message logging in netconf handlers 05/13305/6
authorRobert Varga <rovarga@cisco.com>
Tue, 2 Dec 2014 13:14:39 +0000 (14:14 +0100)
committerRobert Varga <rovarga@cisco.com>
Fri, 5 Dec 2014 09:40:56 +0000 (10:40 +0100)
This wraps LOG.trace() invocations in LOG.isTraceEnabled(), as they
construct potentially big objects which may end up being unused.

Also cleans up method declarations to not use Exception, as Sonar warns
about these.

Change-Id: I8a1a01291b04eea0823b72e254f9ca4cc11557ea
Signed-off-by: Robert Varga <rovarga@cisco.com>
opendaylight/netconf/netconf-impl/src/main/java/org/opendaylight/controller/netconf/impl/mapping/operations/DefaultStartExi.java
opendaylight/netconf/netconf-impl/src/main/java/org/opendaylight/controller/netconf/impl/osgi/NetconfOperationRouterImpl.java
opendaylight/netconf/netconf-netty-util/src/main/java/org/opendaylight/controller/netconf/nettyutil/handler/NetconfEXIToMessageDecoder.java
opendaylight/netconf/netconf-netty-util/src/main/java/org/opendaylight/controller/netconf/nettyutil/handler/NetconfMessageToEXIEncoder.java
opendaylight/netconf/netconf-netty-util/src/main/java/org/opendaylight/controller/netconf/nettyutil/handler/NetconfMessageToXMLEncoder.java
opendaylight/netconf/netconf-netty-util/src/main/java/org/opendaylight/controller/netconf/nettyutil/handler/NetconfXMLToHelloMessageDecoder.java
opendaylight/netconf/netconf-netty-util/src/main/java/org/opendaylight/controller/netconf/nettyutil/handler/NetconfXMLToMessageDecoder.java
opendaylight/netconf/netconf-util/src/main/java/org/opendaylight/controller/netconf/util/messages/SendErrorExceptionUtil.java

index 3f0ae27..5593c22 100644 (file)
@@ -28,14 +28,16 @@ public class DefaultStartExi extends AbstractSingletonNetconfOperation implement
     private static final Logger LOG = LoggerFactory.getLogger(DefaultStartExi.class);
     private NetconfServerSession netconfSession;
 
-    public DefaultStartExi(String netconfSessionIdForReporting) {
+    public DefaultStartExi(final String netconfSessionIdForReporting) {
         super(netconfSessionIdForReporting);
     }
 
     @Override
-    public Document handle(Document message,
-                           NetconfOperationChainedExecution subsequentOperation) throws NetconfDocumentedException {
-        LOG.debug("Received start-exi message {} ", XmlUtil.toString(message));
+    public Document handle(final Document message,
+                           final NetconfOperationChainedExecution subsequentOperation) throws NetconfDocumentedException {
+        if (LOG.isDebugEnabled()) {
+            LOG.debug("Received start-exi message {} ", XmlUtil.toString(message));
+        }
 
         try {
             netconfSession.startExiCommunication(new NetconfMessage(message));
@@ -48,7 +50,7 @@ public class DefaultStartExi extends AbstractSingletonNetconfOperation implement
     }
 
     @Override
-    protected Element handleWithNoSubsequentOperations(Document document, XmlElement operationElement) throws NetconfDocumentedException {
+    protected Element handleWithNoSubsequentOperations(final Document document, final XmlElement operationElement) throws NetconfDocumentedException {
         Element getSchemaResult = document.createElementNS( XmlNetconfConstants.URN_IETF_PARAMS_XML_NS_NETCONF_BASE_1_0, XmlNetconfConstants.OK);
         LOG.trace("{} operation successful", START_EXI);
         return getSchemaResult;
@@ -65,7 +67,7 @@ public class DefaultStartExi extends AbstractSingletonNetconfOperation implement
     }
 
     @Override
-    public void setNetconfSession(NetconfServerSession s) {
+    public void setNetconfSession(final NetconfServerSession s) {
         netconfSession = s;
     }
 }
index 6915ee4..2aa89ba 100644 (file)
@@ -43,19 +43,19 @@ public class NetconfOperationRouterImpl implements NetconfOperationRouter {
     private final NetconfOperationServiceSnapshot netconfOperationServiceSnapshot;
     private Set<NetconfOperation> allNetconfOperations;
 
-    private NetconfOperationRouterImpl(NetconfOperationServiceSnapshot netconfOperationServiceSnapshot) {
+    private NetconfOperationRouterImpl(final NetconfOperationServiceSnapshot netconfOperationServiceSnapshot) {
         this.netconfOperationServiceSnapshot = netconfOperationServiceSnapshot;
     }
 
-    private synchronized void initNetconfOperations(Set<NetconfOperation> allOperations) {
+    private synchronized void initNetconfOperations(final Set<NetconfOperation> allOperations) {
         allNetconfOperations = allOperations;
     }
 
     /**
      * Factory method to produce instance of NetconfOperationRouter
      */
-    public static NetconfOperationRouter createOperationRouter(NetconfOperationServiceSnapshot netconfOperationServiceSnapshot,
-                                                               CapabilityProvider capabilityProvider, DefaultCommitNotificationProducer commitNotifier) {
+    public static NetconfOperationRouter createOperationRouter(final NetconfOperationServiceSnapshot netconfOperationServiceSnapshot,
+                                                               final CapabilityProvider capabilityProvider, final DefaultCommitNotificationProducer commitNotifier) {
         NetconfOperationRouterImpl router = new NetconfOperationRouterImpl(netconfOperationServiceSnapshot);
 
         Preconditions.checkNotNull(netconfOperationServiceSnapshot);
@@ -75,8 +75,8 @@ public class NetconfOperationRouterImpl implements NetconfOperationRouter {
         return router;
     }
 
-    private static Set<NetconfOperation> getAllNetconfOperations(Set<NetconfOperation> defaultNetconfOperations,
-            NetconfOperationServiceSnapshot netconfOperationServiceSnapshot) {
+    private static Set<NetconfOperation> getAllNetconfOperations(final Set<NetconfOperation> defaultNetconfOperations,
+            final NetconfOperationServiceSnapshot netconfOperationServiceSnapshot) {
         Set<NetconfOperation> result = new HashSet<>();
         result.addAll(defaultNetconfOperations);
 
@@ -92,8 +92,8 @@ public class NetconfOperationRouterImpl implements NetconfOperationRouter {
     }
 
     @Override
-    public synchronized Document onNetconfMessage(Document message,
-            NetconfServerSession session) throws NetconfDocumentedException {
+    public synchronized Document onNetconfMessage(final Document message,
+            final NetconfServerSession session) throws NetconfDocumentedException {
         Preconditions.checkNotNull(allNetconfOperations, "Operation router was not initialized properly");
 
         NetconfOperationExecution netconfOperationExecution;
@@ -135,7 +135,7 @@ public class NetconfOperationRouterImpl implements NetconfOperationRouter {
         netconfOperationServiceSnapshot.close();
     }
 
-    private NetconfDocumentedException handleUnexpectedEx(String s, Exception e) throws NetconfDocumentedException {
+    private NetconfDocumentedException handleUnexpectedEx(final String s, final Exception e) throws NetconfDocumentedException {
         LOG.error(s, e);
 
         Map<String, String> info = Maps.newHashMap();
@@ -146,28 +146,29 @@ public class NetconfOperationRouterImpl implements NetconfOperationRouter {
                 NetconfDocumentedException.ErrorSeverity.error, info);
     }
 
-    private Document executeOperationWithHighestPriority(Document message,
-            NetconfOperationExecution netconfOperationExecution, String messageAsString)
+    private Document executeOperationWithHighestPriority(final Document message,
+            final NetconfOperationExecution netconfOperationExecution, final String messageAsString)
             throws NetconfDocumentedException {
         LOG.debug("Forwarding netconf message {} to {}", messageAsString, netconfOperationExecution.netconfOperation);
         return netconfOperationExecution.execute(message);
     }
 
     private NetconfOperationExecution getNetconfOperationWithHighestPriority(
-            Document message, NetconfServerSession session) throws NetconfDocumentedException {
+            final Document message, final NetconfServerSession session) throws NetconfDocumentedException {
 
         NavigableMap<HandlingPriority, NetconfOperation> sortedByPriority = getSortedNetconfOperationsWithCanHandle(
                 message, session);
 
-        Preconditions.checkArgument(sortedByPriority.isEmpty() == false,
-                "No %s available to handle message %s", NetconfOperation.class.getName(),
-                XmlUtil.toString(message));
+        if (sortedByPriority.isEmpty()) {
+            throw new IllegalArgumentException(String.format("No %s available to handle message %s",
+                NetconfOperation.class.getName(), XmlUtil.toString(message)));
+        }
 
         return NetconfOperationExecution.createExecutionChain(sortedByPriority, sortedByPriority.lastKey());
     }
 
-    private TreeMap<HandlingPriority, NetconfOperation> getSortedNetconfOperationsWithCanHandle(Document message,
-            NetconfServerSession session) throws NetconfDocumentedException {
+    private TreeMap<HandlingPriority, NetconfOperation> getSortedNetconfOperationsWithCanHandle(final Document message,
+            final NetconfServerSession session) throws NetconfDocumentedException {
         TreeMap<HandlingPriority, NetconfOperation> sortedPriority = Maps.newTreeMap();
 
         for (NetconfOperation netconfOperation : allNetconfOperations) {
@@ -193,7 +194,7 @@ public class NetconfOperationRouterImpl implements NetconfOperationRouter {
         }
 
         @Override
-        public Document execute(Document requestMessage) throws NetconfDocumentedException {
+        public Document execute(final Document requestMessage) throws NetconfDocumentedException {
             throw new NetconfDocumentedException("This execution represents the termination point in operation execution and cannot be executed itself",
                     NetconfDocumentedException.ErrorType.application,
                     NetconfDocumentedException.ErrorTag.operation_failed,
@@ -203,9 +204,9 @@ public class NetconfOperationRouterImpl implements NetconfOperationRouter {
 
     private static class NetconfOperationExecution implements NetconfOperationChainedExecution {
         private final NetconfOperation netconfOperation;
-        private NetconfOperationChainedExecution subsequentExecution;
+        private final NetconfOperationChainedExecution subsequentExecution;
 
-        private NetconfOperationExecution(NetconfOperation netconfOperation, NetconfOperationChainedExecution subsequentExecution) {
+        private NetconfOperationExecution(final NetconfOperation netconfOperation, final NetconfOperationChainedExecution subsequentExecution) {
             this.netconfOperation = netconfOperation;
             this.subsequentExecution = subsequentExecution;
         }
@@ -216,12 +217,12 @@ public class NetconfOperationRouterImpl implements NetconfOperationRouter {
         }
 
         @Override
-        public Document execute(Document message) throws NetconfDocumentedException {
+        public Document execute(final Document message) throws NetconfDocumentedException {
             return netconfOperation.handle(message, subsequentExecution);
         }
 
         public static NetconfOperationExecution createExecutionChain(
-                NavigableMap<HandlingPriority, NetconfOperation> sortedByPriority, HandlingPriority handlingPriority) {
+                final NavigableMap<HandlingPriority, NetconfOperation> sortedByPriority, final HandlingPriority handlingPriority) {
             NetconfOperation netconfOperation = sortedByPriority.get(handlingPriority);
             HandlingPriority subsequentHandlingPriority = sortedByPriority.lowerKey(handlingPriority);
 
index db3dcaf..0d8f9ee 100644 (file)
@@ -13,18 +13,22 @@ import io.netty.buffer.ByteBufInputStream;
 import io.netty.buffer.ByteBufUtil;
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.handler.codec.ByteToMessageDecoder;
+import java.io.IOException;
 import java.io.InputStream;
 import java.util.List;
+import javax.xml.transform.TransformerConfigurationException;
 import javax.xml.transform.TransformerFactory;
 import javax.xml.transform.dom.DOMResult;
 import javax.xml.transform.sax.SAXTransformerFactory;
 import javax.xml.transform.sax.TransformerHandler;
 import org.opendaylight.controller.netconf.api.NetconfMessage;
+import org.openexi.proc.common.EXIOptionsException;
 import org.openexi.sax.EXIReader;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.w3c.dom.Document;
 import org.xml.sax.InputSource;
+import org.xml.sax.SAXException;
 
 public final class NetconfEXIToMessageDecoder extends ByteToMessageDecoder {
 
@@ -37,7 +41,7 @@ public final class NetconfEXIToMessageDecoder extends ByteToMessageDecoder {
     }
 
     @Override
-    protected void decode(final ChannelHandlerContext ctx, final ByteBuf in, final List<Object> out) throws Exception {
+    protected void decode(final ChannelHandlerContext ctx, final ByteBuf in, final List<Object> out) throws EXIOptionsException, IOException, SAXException, TransformerConfigurationException  {
         /*
          * Note that we could loop here and process all the messages, but we can't do that.
          * The reason is <stop-exi> operation, which has the contract of immediately stopping
@@ -46,12 +50,14 @@ public final class NetconfEXIToMessageDecoder extends ByteToMessageDecoder {
          */
 
         // If empty Byte buffer is passed to r.parse, EOFException is thrown
-        if (in.isReadable() == false) {
+        if (!in.isReadable()) {
             LOG.debug("No more content in incoming buffer.");
             return;
         }
 
-        LOG.trace("Received to decode: {}", ByteBufUtil.hexDump(in));
+        if (LOG.isTraceEnabled()) {
+            LOG.trace("Received to decode: {}", ByteBufUtil.hexDump(in));
+        }
 
         final EXIReader r = codec.getReader();
 
index 0a866ff..6989ab2 100644 (file)
@@ -12,13 +12,16 @@ import io.netty.buffer.ByteBuf;
 import io.netty.buffer.ByteBufOutputStream;
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.handler.codec.MessageToByteEncoder;
+import java.io.IOException;
 import java.io.OutputStream;
 import javax.xml.transform.Transformer;
+import javax.xml.transform.TransformerException;
 import javax.xml.transform.dom.DOMSource;
 import javax.xml.transform.sax.SAXResult;
 import javax.xml.transform.sax.SAXTransformerFactory;
 import org.opendaylight.controller.netconf.api.NetconfMessage;
 import org.opendaylight.controller.netconf.util.xml.XmlUtil;
+import org.openexi.proc.common.EXIOptionsException;
 import org.openexi.sax.Transmogrifier;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -35,8 +38,10 @@ public final class NetconfMessageToEXIEncoder extends MessageToByteEncoder<Netco
     }
 
     @Override
-    protected void encode(final ChannelHandlerContext ctx, final NetconfMessage msg, final ByteBuf out) throws Exception {
-        LOG.trace("Sent to encode : {}", XmlUtil.toString(msg.getDocument()));
+    protected void encode(final ChannelHandlerContext ctx, final NetconfMessage msg, final ByteBuf out) throws EXIOptionsException, IOException, TransformerException {
+        if (LOG.isTraceEnabled()) {
+            LOG.trace("Sent to encode : {}", XmlUtil.toString(msg.getDocument()));
+        }
 
         try (final OutputStream os = new ByteBufOutputStream(out)) {
             final Transmogrifier transmogrifier = codec.getTransmogrifier();
index 8ce9411..302e24e 100644 (file)
@@ -46,7 +46,9 @@ public class NetconfMessageToXMLEncoder extends MessageToByteEncoder<NetconfMess
     @Override
     @VisibleForTesting
     public void encode(ChannelHandlerContext ctx, NetconfMessage msg, ByteBuf out) throws IOException, TransformerException {
-        LOG.trace("Sent to encode : {}", XmlUtil.toString(msg.getDocument()));
+        if (LOG.isTraceEnabled()) {
+            LOG.trace("Sent to encode : {}", XmlUtil.toString(msg.getDocument()));
+        }
 
         if (clientId.isPresent()) {
             Comment comment = msg.getDocument().createComment("clientId:" + clientId.get());
index 197ae5f..1fbac0b 100644 (file)
@@ -69,7 +69,10 @@ public final class NetconfXMLToHelloMessageDecoder extends ByteToMessageDecoder
 
         in.markReaderIndex();
         try {
-            LOG.trace("Received to decode: {}", ByteBufUtil.hexDump(in));
+            if (LOG.isTraceEnabled()) {
+                LOG.trace("Received to decode: {}", ByteBufUtil.hexDump(in));
+            }
+
             byte[] bytes = new byte[in.readableBytes()];
             in.readBytes(bytes);
 
index bfc8d77..1dc8e02 100644 (file)
@@ -12,20 +12,24 @@ import io.netty.buffer.ByteBufInputStream;
 import io.netty.buffer.ByteBufUtil;
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.handler.codec.ByteToMessageDecoder;
+import java.io.IOException;
 import java.util.List;
 import org.opendaylight.controller.netconf.api.NetconfMessage;
 import org.opendaylight.controller.netconf.util.xml.XmlUtil;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.xml.sax.SAXException;
 
 public final class NetconfXMLToMessageDecoder extends ByteToMessageDecoder {
     private static final Logger LOG = LoggerFactory.getLogger(NetconfXMLToMessageDecoder.class);
 
     @Override
-    public void decode(ChannelHandlerContext ctx, ByteBuf in, List<Object> out) throws Exception {
+    public void decode(final ChannelHandlerContext ctx, final ByteBuf in, final List<Object> out) throws IOException, SAXException {
+        if (in.isReadable()) {
+            if (LOG.isTraceEnabled()) {
+                LOG.trace("Received to decode: {}", ByteBufUtil.hexDump(in));
+            }
 
-        if (in.readableBytes() != 0) {
-            LOG.trace("Received to decode: {}", ByteBufUtil.hexDump(in));
             out.add(new NetconfMessage(XmlUtil.readXmlToDocument(new ByteBufInputStream(in))));
         } else {
             LOG.debug("No more content in incoming buffer.");
index fe5ed03..490eb95 100644 (file)
@@ -37,17 +37,20 @@ public final class SendErrorExceptionUtil {
         f.addListener(new SendErrorVerifyingListener(sendErrorException));
     }
 
-    public static void sendErrorMessage(Channel channel, NetconfDocumentedException sendErrorException) {
+    public static void sendErrorMessage(final Channel channel, final NetconfDocumentedException sendErrorException) {
         LOG.trace("Sending error {}", sendErrorException.getMessage(), sendErrorException);
         final Document errorDocument = createDocument(sendErrorException);
         ChannelFuture f = channel.writeAndFlush(new NetconfMessage(errorDocument));
         f.addListener(new SendErrorVerifyingListener(sendErrorException));
     }
 
-    public static void sendErrorMessage(NetconfSession session, NetconfDocumentedException sendErrorException,
-            NetconfMessage incommingMessage) {
+    public static void sendErrorMessage(final NetconfSession session, final NetconfDocumentedException sendErrorException,
+            final NetconfMessage incommingMessage) {
         final Document errorDocument = createDocument(sendErrorException);
-        LOG.trace("Sending error {}", XmlUtil.toString(errorDocument));
+        if (LOG.isTraceEnabled()) {
+            LOG.trace("Sending error {}", XmlUtil.toString(errorDocument));
+        }
+
         tryToCopyAttributes(incommingMessage.getDocument(), errorDocument, sendErrorException);
         ChannelFuture f = session.sendMessage(new NetconfMessage(errorDocument));
         f.addListener(new SendErrorVerifyingListener(sendErrorException));