From: Robert Varga Date: Tue, 2 Dec 2014 13:14:39 +0000 (+0100) Subject: Cleanup message logging in netconf handlers X-Git-Tag: release/lithium~796^2~1 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=0418d143fbc882b24becaa52ab115a4a42d3328c Cleanup message logging in netconf handlers 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 --- diff --git a/opendaylight/netconf/netconf-impl/src/main/java/org/opendaylight/controller/netconf/impl/mapping/operations/DefaultStartExi.java b/opendaylight/netconf/netconf-impl/src/main/java/org/opendaylight/controller/netconf/impl/mapping/operations/DefaultStartExi.java index 3f0ae27dbb..5593c22fad 100644 --- a/opendaylight/netconf/netconf-impl/src/main/java/org/opendaylight/controller/netconf/impl/mapping/operations/DefaultStartExi.java +++ b/opendaylight/netconf/netconf-impl/src/main/java/org/opendaylight/controller/netconf/impl/mapping/operations/DefaultStartExi.java @@ -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; } } diff --git a/opendaylight/netconf/netconf-impl/src/main/java/org/opendaylight/controller/netconf/impl/osgi/NetconfOperationRouterImpl.java b/opendaylight/netconf/netconf-impl/src/main/java/org/opendaylight/controller/netconf/impl/osgi/NetconfOperationRouterImpl.java index 6915ee4bfe..2aa89ba2c4 100644 --- a/opendaylight/netconf/netconf-impl/src/main/java/org/opendaylight/controller/netconf/impl/osgi/NetconfOperationRouterImpl.java +++ b/opendaylight/netconf/netconf-impl/src/main/java/org/opendaylight/controller/netconf/impl/osgi/NetconfOperationRouterImpl.java @@ -43,19 +43,19 @@ public class NetconfOperationRouterImpl implements NetconfOperationRouter { private final NetconfOperationServiceSnapshot netconfOperationServiceSnapshot; private Set allNetconfOperations; - private NetconfOperationRouterImpl(NetconfOperationServiceSnapshot netconfOperationServiceSnapshot) { + private NetconfOperationRouterImpl(final NetconfOperationServiceSnapshot netconfOperationServiceSnapshot) { this.netconfOperationServiceSnapshot = netconfOperationServiceSnapshot; } - private synchronized void initNetconfOperations(Set allOperations) { + private synchronized void initNetconfOperations(final Set 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 getAllNetconfOperations(Set defaultNetconfOperations, - NetconfOperationServiceSnapshot netconfOperationServiceSnapshot) { + private static Set getAllNetconfOperations(final Set defaultNetconfOperations, + final NetconfOperationServiceSnapshot netconfOperationServiceSnapshot) { Set 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 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 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 getSortedNetconfOperationsWithCanHandle(Document message, - NetconfServerSession session) throws NetconfDocumentedException { + private TreeMap getSortedNetconfOperationsWithCanHandle(final Document message, + final NetconfServerSession session) throws NetconfDocumentedException { TreeMap 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 sortedByPriority, HandlingPriority handlingPriority) { + final NavigableMap sortedByPriority, final HandlingPriority handlingPriority) { NetconfOperation netconfOperation = sortedByPriority.get(handlingPriority); HandlingPriority subsequentHandlingPriority = sortedByPriority.lowerKey(handlingPriority); diff --git a/opendaylight/netconf/netconf-netty-util/src/main/java/org/opendaylight/controller/netconf/nettyutil/handler/NetconfEXIToMessageDecoder.java b/opendaylight/netconf/netconf-netty-util/src/main/java/org/opendaylight/controller/netconf/nettyutil/handler/NetconfEXIToMessageDecoder.java index db3dcafbde..0d8f9eeec1 100644 --- a/opendaylight/netconf/netconf-netty-util/src/main/java/org/opendaylight/controller/netconf/nettyutil/handler/NetconfEXIToMessageDecoder.java +++ b/opendaylight/netconf/netconf-netty-util/src/main/java/org/opendaylight/controller/netconf/nettyutil/handler/NetconfEXIToMessageDecoder.java @@ -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 out) throws Exception { + protected void decode(final ChannelHandlerContext ctx, final ByteBuf in, final List 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 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(); diff --git a/opendaylight/netconf/netconf-netty-util/src/main/java/org/opendaylight/controller/netconf/nettyutil/handler/NetconfMessageToEXIEncoder.java b/opendaylight/netconf/netconf-netty-util/src/main/java/org/opendaylight/controller/netconf/nettyutil/handler/NetconfMessageToEXIEncoder.java index 0a866fffaa..6989ab2b8a 100644 --- a/opendaylight/netconf/netconf-netty-util/src/main/java/org/opendaylight/controller/netconf/nettyutil/handler/NetconfMessageToEXIEncoder.java +++ b/opendaylight/netconf/netconf-netty-util/src/main/java/org/opendaylight/controller/netconf/nettyutil/handler/NetconfMessageToEXIEncoder.java @@ -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 out) throws Exception { + public void decode(final ChannelHandlerContext ctx, final ByteBuf in, final List 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."); diff --git a/opendaylight/netconf/netconf-util/src/main/java/org/opendaylight/controller/netconf/util/messages/SendErrorExceptionUtil.java b/opendaylight/netconf/netconf-util/src/main/java/org/opendaylight/controller/netconf/util/messages/SendErrorExceptionUtil.java index fe5ed03320..490eb95b82 100644 --- a/opendaylight/netconf/netconf-util/src/main/java/org/opendaylight/controller/netconf/util/messages/SendErrorExceptionUtil.java +++ b/opendaylight/netconf/netconf-util/src/main/java/org/opendaylight/controller/netconf/util/messages/SendErrorExceptionUtil.java @@ -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));