Implement NetconfMessage.toString() 20/13320/5
authorRobert Varga <rovarga@cisco.com>
Tue, 2 Dec 2014 22:26:38 +0000 (23:26 +0100)
committerRobert Varga <rovarga@cisco.com>
Fri, 5 Dec 2014 09:55:02 +0000 (10:55 +0100)
Rather than explicitly converting the internal document intoa string
before logging it, teach NetconfMessage how to do toString(). This way
we do not have to check Logger configuration and pass NetconfMessages
directly. The logger will cann toString() as needed.

Change-Id: I07a18f32a1500784b81f52f0412acd73123a161e
Signed-off-by: Robert Varga <rovarga@cisco.com>
opendaylight/netconf/netconf-api/pom.xml
opendaylight/netconf/netconf-api/src/main/java/org/opendaylight/controller/netconf/api/NetconfMessage.java
opendaylight/netconf/netconf-client/src/main/java/org/opendaylight/controller/netconf/client/NetconfClientSessionNegotiator.java
opendaylight/netconf/netconf-impl/src/main/java/org/opendaylight/controller/netconf/impl/NetconfServerSessionListener.java
opendaylight/netconf/netconf-netty-util/src/main/java/org/opendaylight/controller/netconf/nettyutil/AbstractNetconfSession.java
opendaylight/netconf/netconf-netty-util/src/main/java/org/opendaylight/controller/netconf/nettyutil/AbstractNetconfSessionNegotiator.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

index c5fd8f1..11aed29 100644 (file)
         <configuration>
           <instructions>
             <Private-Package></Private-Package>
-            <Import-Package>javax.management,
-                            javax.xml.parsers,
-                            org.opendaylight.controller.config.api,
-                            org.opendaylight.controller.config.api.jmx,
-                            org.opendaylight.protocol.framework,
-                            io.netty.channel,
-                            io.netty.util.concurrent,
-                            org.w3c.dom,
-                            org.slf4j,
-                            org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev100924,
-                            org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.yang.types.rev100924,
-                            org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.netconf.monitoring.rev101004.netconf.state,
-                            org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.netconf.monitoring.rev101004.netconf.state.sessions,
-                            com.google.common.base,</Import-Package>
             <Export-Package>org.opendaylight.controller.netconf.api,
                             org.opendaylight.controller.netconf.api.jmx,
                             org.opendaylight.controller.netconf.api.xml,
index 78586a3..d8c80e5 100644 (file)
@@ -8,6 +8,15 @@
 
 package org.opendaylight.controller.netconf.api;
 
+import java.io.StringWriter;
+import javax.xml.transform.OutputKeys;
+import javax.xml.transform.Transformer;
+import javax.xml.transform.TransformerConfigurationException;
+import javax.xml.transform.TransformerException;
+import javax.xml.transform.TransformerFactory;
+import javax.xml.transform.TransformerFactoryConfigurationError;
+import javax.xml.transform.dom.DOMSource;
+import javax.xml.transform.stream.StreamResult;
 import org.w3c.dom.Document;
 
 /**
@@ -15,6 +24,21 @@ import org.w3c.dom.Document;
  * implementing ProtocolMessage interface.
  */
 public class NetconfMessage {
+    private static final Transformer TRANSFORMER;
+
+    static {
+        final Transformer t;
+        try {
+            t = TransformerFactory.newInstance().newTransformer();
+        } catch (TransformerConfigurationException | TransformerFactoryConfigurationError e) {
+            throw new ExceptionInInitializerError(e);
+        }
+        t.setOutputProperty(OutputKeys.INDENT, "yes");
+        t.setOutputProperty(OutputKeys.OMIT_XML_DECLARATION, "yes");
+
+        TRANSFORMER = t;
+    }
+
     private final Document doc;
 
     public NetconfMessage(final Document doc) {
@@ -24,4 +48,21 @@ public class NetconfMessage {
     public Document getDocument() {
         return this.doc;
     }
+
+    @Override
+    public String toString() {
+        final StreamResult result = new StreamResult(new StringWriter());
+        final DOMSource source = new DOMSource(doc.getDocumentElement());
+
+        try {
+            // Slight critical section is a tradeoff. This should be reasonably fast.
+            synchronized (TRANSFORMER) {
+                TRANSFORMER.transform(source, result);
+            }
+        } catch (TransformerException e) {
+            throw new IllegalStateException("Failed to encode document", e);
+        }
+
+        return result.getWriter().toString();
+    }
 }
index a8c9a65..06c695c 100644 (file)
@@ -169,14 +169,13 @@ public class NetconfClientSessionNegotiator extends
             } else if(NetconfMessageUtil.isErrorMessage(netconfMessage)) {
                 LOG.warn(
                         "Error response to start-exi message {}, Communication will continue without exi on session {}",
-                        XmlUtil.toString(netconfMessage.getDocument()), session);
+                        netconfMessage, session);
 
                 // Unexpected response to start-exi, throwing message away, continue without exi
             } else {
-                LOG.warn(
-                        "Unexpected response to start-exi message, should be ok, was {}, ",XmlUtil.toString(netconfMessage.getDocument()),
-                        "Communication will continue without exi and response message will be thrown away on session {}",
-                         session);
+                LOG.warn("Unexpected response to start-exi message, should be ok, was {}, " +
+                         "Communication will continue without exi and response message will be thrown away on session {}",
+                         netconfMessage, session);
             }
 
             negotiationSuccessful(session);
index b2b8c50..951d0dd 100644 (file)
@@ -35,25 +35,25 @@ public class NetconfServerSessionListener implements NetconfSessionListener<Netc
     private final NetconfOperationRouter operationRouter;
     private final AutoCloseable onSessionDownCloseable;
 
-    public NetconfServerSessionListener(NetconfOperationRouter operationRouter, SessionMonitoringService monitoringService,
-                                        AutoCloseable onSessionDownCloseable) {
+    public NetconfServerSessionListener(final NetconfOperationRouter operationRouter, final SessionMonitoringService monitoringService,
+                                        final AutoCloseable onSessionDownCloseable) {
         this.operationRouter = operationRouter;
         this.monitoringService = monitoringService;
         this.onSessionDownCloseable = onSessionDownCloseable;
     }
 
     @Override
-    public void onSessionUp(NetconfServerSession netconfNetconfServerSession) {
+    public void onSessionUp(final NetconfServerSession netconfNetconfServerSession) {
         monitoringService.onSessionUp(netconfNetconfServerSession);
     }
 
     @Override
-    public void onSessionDown(NetconfServerSession netconfNetconfServerSession, Exception cause) {
+    public void onSessionDown(final NetconfServerSession netconfNetconfServerSession, final Exception cause) {
         LOG.debug("Session {} down, reason: {}", netconfNetconfServerSession, cause.getMessage());
         onDown(netconfNetconfServerSession);
     }
 
-    public void onDown(NetconfServerSession netconfNetconfServerSession) {
+    public void onDown(final NetconfServerSession netconfNetconfServerSession) {
         monitoringService.onSessionDown(netconfNetconfServerSession);
 
         try {
@@ -69,15 +69,15 @@ public class NetconfServerSessionListener implements NetconfSessionListener<Netc
     }
 
     @Override
-    public void onSessionTerminated(NetconfServerSession netconfNetconfServerSession,
-            NetconfTerminationReason netconfTerminationReason) {
+    public void onSessionTerminated(final NetconfServerSession netconfNetconfServerSession,
+            final NetconfTerminationReason netconfTerminationReason) {
         LOG.debug("Session {} terminated, reason: {}", netconfNetconfServerSession,
                 netconfTerminationReason.getErrorMessage());
         onDown(netconfNetconfServerSession);
     }
 
     @Override
-    public void onMessage(NetconfServerSession session, NetconfMessage netconfMessage) {
+    public void onMessage(final NetconfServerSession session, final NetconfMessage netconfMessage) {
         try {
 
             Preconditions.checkState(operationRouter != null, "Cannot handle message, session up was not yet received");
@@ -85,7 +85,7 @@ public class NetconfServerSessionListener implements NetconfSessionListener<Netc
             // schemas
             final NetconfMessage message = processDocument(netconfMessage,
                     session);
-            LOG.debug("Responding with message {}", XmlUtil.toString(message.getDocument()));
+            LOG.debug("Responding with message {}", message);
             session.sendMessage(message);
 
             if (isCloseSession(netconfMessage)) {
@@ -105,7 +105,7 @@ public class NetconfServerSessionListener implements NetconfSessionListener<Netc
         }
     }
 
-    private void closeNetconfSession(NetconfServerSession session) {
+    private void closeNetconfSession(final NetconfServerSession session) {
         // destroy NetconfOperationService
         session.close();
         LOG.info("Session {} closed successfully", session.getSessionId());
@@ -113,7 +113,7 @@ public class NetconfServerSessionListener implements NetconfSessionListener<Netc
 
 
 
-    private NetconfMessage processDocument(final NetconfMessage netconfMessage, NetconfServerSession session)
+    private NetconfMessage processDocument(final NetconfMessage netconfMessage, final NetconfServerSession session)
             throws NetconfDocumentedException {
 
         final Document incomingDocument = netconfMessage.getDocument();
@@ -146,7 +146,7 @@ public class NetconfServerSessionListener implements NetconfSessionListener<Netc
         }
     }
 
-    private void checkMessageId(Node rootNode) throws NetconfDocumentedException {
+    private void checkMessageId(final Node rootNode) throws NetconfDocumentedException {
 
         NamedNodeMap attributes = rootNode.getAttributes();
 
index 4eaeee9..efa1c73 100644 (file)
@@ -19,7 +19,6 @@ import org.opendaylight.controller.netconf.api.NetconfTerminationReason;
 import org.opendaylight.controller.netconf.nettyutil.handler.NetconfEXICodec;
 import org.opendaylight.controller.netconf.nettyutil.handler.exi.EXIParameters;
 import org.opendaylight.controller.netconf.util.xml.XmlElement;
-import org.opendaylight.controller.netconf.util.xml.XmlUtil;
 import org.opendaylight.protocol.framework.AbstractProtocolSession;
 import org.openexi.proc.common.EXIOptionsException;
 import org.slf4j.Logger;
@@ -114,8 +113,8 @@ public abstract class AbstractNetconfSession<S extends NetconfSession, L extends
         try {
             exiParams = EXIParameters.fromXmlElement(XmlElement.fromDomDocument(startExiMessage.getDocument()));
         } catch (final EXIOptionsException e) {
-            LOG.warn("Unable to parse EXI parameters from {} om session {}", XmlUtil.toString(startExiMessage.getDocument()), this, e);
-            throw new IllegalArgumentException(e);
+            LOG.warn("Unable to parse EXI parameters from {} on session {}", startExiMessage, this, e);
+            throw new IllegalArgumentException("Cannot parse options", e);
         }
         final NetconfEXICodec exiCodec = new NetconfEXICodec(exiParams.getOptions());
         addExiHandlers(exiCodec);
index e5c3c12..6d4c106 100644 (file)
@@ -34,7 +34,6 @@ import org.opendaylight.controller.netconf.nettyutil.handler.NetconfXMLToHelloMe
 import org.opendaylight.controller.netconf.nettyutil.handler.NetconfXMLToMessageDecoder;
 import org.opendaylight.controller.netconf.util.messages.FramingMechanism;
 import org.opendaylight.controller.netconf.util.messages.NetconfHelloMessage;
-import org.opendaylight.controller.netconf.util.xml.XmlUtil;
 import org.opendaylight.protocol.framework.AbstractSessionNegotiator;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -66,8 +65,8 @@ public abstract class AbstractNetconfSessionNegotiator<P extends NetconfSessionP
     private final long connectionTimeoutMillis;
 
     // TODO shrink constructor
-    protected AbstractNetconfSessionNegotiator(P sessionPreferences, Promise<S> promise, Channel channel, Timer timer,
-            L sessionListener, long connectionTimeoutMillis) {
+    protected AbstractNetconfSessionNegotiator(final P sessionPreferences, final Promise<S> promise, final Channel channel, final Timer timer,
+            final L sessionListener, final long connectionTimeoutMillis) {
         super(promise, channel);
         this.sessionPreferences = sessionPreferences;
         this.promise = promise;
@@ -83,7 +82,7 @@ public abstract class AbstractNetconfSessionNegotiator<P extends NetconfSessionP
             Future<Channel> future = sslHandler.get().handshakeFuture();
             future.addListener(new GenericFutureListener<Future<? super Channel>>() {
                 @Override
-                public void operationComplete(Future<? super Channel> future) {
+                public void operationComplete(final Future<? super Channel> future) {
                     Preconditions.checkState(future.isSuccess(), "Ssl handshake was not successful");
                     LOG.debug("Ssl handshake complete");
                     start();
@@ -94,7 +93,7 @@ public abstract class AbstractNetconfSessionNegotiator<P extends NetconfSessionP
         }
     }
 
-    private static Optional<SslHandler> getSslHandler(Channel channel) {
+    private static Optional<SslHandler> getSslHandler(final Channel channel) {
         final SslHandler sslHandler = channel.pipeline().get(SslHandler.class);
         return sslHandler == null ? Optional.<SslHandler> absent() : Optional.of(sslHandler);
     }
@@ -105,7 +104,7 @@ public abstract class AbstractNetconfSessionNegotiator<P extends NetconfSessionP
 
     private void start() {
         final NetconfMessage helloMessage = this.sessionPreferences.getHelloMessage();
-        LOG.debug("Session negotiation started with hello message {} on channel {}", XmlUtil.toString(helloMessage.getDocument()), channel);
+        LOG.debug("Session negotiation started with hello message {} on channel {}", helloMessage, channel);
 
         channel.pipeline().addLast(NAME_OF_EXCEPTION_HANDLER, new ExceptionHandlingInboundChannelHandler());
 
@@ -131,7 +130,7 @@ public abstract class AbstractNetconfSessionNegotiator<P extends NetconfSessionP
 
                             channel.closeFuture().addListener(new GenericFutureListener<ChannelFuture>() {
                                 @Override
-                                public void operationComplete(ChannelFuture future) throws Exception {
+                                public void operationComplete(final ChannelFuture future) throws Exception {
                                     if(future.isSuccess()) {
                                         LOG.debug("Channel {} closed: success", future.channel());
                                     } else {
@@ -159,7 +158,7 @@ public abstract class AbstractNetconfSessionNegotiator<P extends NetconfSessionP
         }
     }
 
-    protected final S getSessionForHelloMessage(NetconfHelloMessage netconfMessage) throws NetconfDocumentedException {
+    protected final S getSessionForHelloMessage(final NetconfHelloMessage netconfMessage) throws NetconfDocumentedException {
         Preconditions.checkNotNull(netconfMessage, "netconfMessage");
 
         final Document doc = netconfMessage.getDocument();
@@ -182,7 +181,7 @@ public abstract class AbstractNetconfSessionNegotiator<P extends NetconfSessionP
                 new NetconfChunkAggregator());
     }
 
-    private boolean shouldUseChunkFraming(Document doc) {
+    private boolean shouldUseChunkFraming(final Document doc) {
         return containsBase11Capability(doc)
                 && containsBase11Capability(sessionPreferences.getHelloMessage().getDocument());
     }
@@ -216,7 +215,7 @@ public abstract class AbstractNetconfSessionNegotiator<P extends NetconfSessionP
         replaceChannelHandler(channel, AbstractChannelInitializer.NETCONF_MESSAGE_ENCODER, new NetconfMessageToXMLEncoder());
     }
 
-    private static ChannelHandler replaceChannelHandler(Channel channel, String handlerKey, ChannelHandler decoder) {
+    private static ChannelHandler replaceChannelHandler(final Channel channel, final String handlerKey, final ChannelHandler decoder) {
         return channel.pipeline().replace(handlerKey, handlerKey, decoder);
     }
 
@@ -239,7 +238,7 @@ public abstract class AbstractNetconfSessionNegotiator<P extends NetconfSessionP
         return false;
     }
 
-    private static boolean isStateChangePermitted(State state, State newState) {
+    private static boolean isStateChangePermitted(final State state, final State newState) {
         if (state == State.IDLE && newState == State.OPEN_WAIT) {
             return true;
         }
@@ -258,7 +257,7 @@ public abstract class AbstractNetconfSessionNegotiator<P extends NetconfSessionP
      */
     private final class ExceptionHandlingInboundChannelHandler extends ChannelInboundHandlerAdapter {
         @Override
-        public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) {
+        public void exceptionCaught(final ChannelHandlerContext ctx, final Throwable cause) {
             LOG.warn("An exception occurred during negotiation with {}", channel.remoteAddress(), cause);
             cancelTimeout();
             negotiationFailed(cause);
index 6989ab2..55dcd9d 100644 (file)
@@ -20,7 +20,6 @@ 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;
@@ -39,9 +38,7 @@ public final class NetconfMessageToEXIEncoder extends MessageToByteEncoder<Netco
 
     @Override
     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()));
-        }
+        LOG.trace("Sent to encode : {}", msg);
 
         try (final OutputStream os = new ByteBufOutputStream(out)) {
             final Transmogrifier transmogrifier = codec.getTransmogrifier();
index 302e24e..d109e2d 100644 (file)
@@ -24,7 +24,6 @@ import javax.xml.transform.TransformerFactory;
 import javax.xml.transform.dom.DOMSource;
 import javax.xml.transform.stream.StreamResult;
 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.w3c.dom.Comment;
@@ -46,9 +45,7 @@ public class NetconfMessageToXMLEncoder extends MessageToByteEncoder<NetconfMess
     @Override
     @VisibleForTesting
     public void encode(ChannelHandlerContext ctx, NetconfMessage msg, ByteBuf out) throws IOException, TransformerException {
-        if (LOG.isTraceEnabled()) {
-            LOG.trace("Sent to encode : {}", XmlUtil.toString(msg.getDocument()));
-        }
+        LOG.trace("Sent to encode : {}", msg);
 
         if (clientId.isPresent()) {
             Comment comment = msg.getDocument().createComment("clientId:" + clientId.get());
index 1fbac0b..3ba49fc 100644 (file)
@@ -56,12 +56,12 @@ public final class NetconfXMLToHelloMessageDecoder extends ByteToMessageDecoder
     // State variables do not have to by synchronized
     // Netty uses always the same (1) thread per pipeline
     // We use instance of this per pipeline
-    private List<NetconfMessage> nonHelloMessages = Lists.newArrayList();
+    private final List<NetconfMessage> nonHelloMessages = Lists.newArrayList();
     private boolean helloReceived = false;
 
     @Override
     @VisibleForTesting
-    public void decode(ChannelHandlerContext ctx, ByteBuf in, List<Object> out) throws IOException, SAXException, NetconfDocumentedException {
+    public void decode(final ChannelHandlerContext ctx, final ByteBuf in, final List<Object> out) throws IOException, SAXException, NetconfDocumentedException {
         if (in.readableBytes() == 0) {
             LOG.debug("No more content in incoming buffer.");
             return;
@@ -95,16 +95,13 @@ public final class NetconfXMLToHelloMessageDecoder extends ByteToMessageDecoder
             final NetconfMessage message = getNetconfMessage(additionalHeader, doc);
             if (message instanceof NetconfHelloMessage) {
                 Preconditions.checkState(helloReceived == false,
-                        "Multiple hello messages received, unexpected hello: %s",
-                        XmlUtil.toString(message.getDocument()));
+                        "Multiple hello messages received, unexpected hello: %s", message);
                 out.add(message);
                 helloReceived = true;
             // Non hello message, suspend the message and insert into cache
             } else {
-                Preconditions.checkState(helloReceived, "Hello message not received, instead received: %s",
-                        XmlUtil.toString(message.getDocument()));
-                LOG.debug("Netconf message received during negotiation, caching {}",
-                        XmlUtil.toString(message.getDocument()));
+                Preconditions.checkState(helloReceived, "Hello message not received, instead received: %s", message);
+                LOG.debug("Netconf message received during negotiation, caching {}", message);
                 nonHelloMessages.add(message);
             }
         } finally {
@@ -125,7 +122,7 @@ public final class NetconfXMLToHelloMessageDecoder extends ByteToMessageDecoder
         return msg;
     }
 
-    private int getAdditionalHeaderEndIndex(byte[] bytes) {
+    private int getAdditionalHeaderEndIndex(final byte[] bytes) {
         for (byte[] possibleEnd : POSSIBLE_ENDS) {
             int idx = findByteSequence(bytes, possibleEnd);
 
@@ -163,12 +160,12 @@ public final class NetconfXMLToHelloMessageDecoder extends ByteToMessageDecoder
     }
 
 
-    private void logMessage(byte[] bytes) {
+    private void logMessage(final byte[] bytes) {
         String s = Charsets.UTF_8.decode(ByteBuffer.wrap(bytes)).toString();
         LOG.debug("Parsing message \n{}", s);
     }
 
-    private boolean startsWithAdditionalHeader(byte[] bytes) {
+    private boolean startsWithAdditionalHeader(final byte[] bytes) {
         for (byte[] possibleStart : POSSIBLE_STARTS) {
             int i = 0;
             for (byte b : possibleStart) {
@@ -185,7 +182,7 @@ public final class NetconfXMLToHelloMessageDecoder extends ByteToMessageDecoder
         return false;
     }
 
-    private String additionalHeaderToString(byte[] bytes) {
+    private String additionalHeaderToString(final byte[] bytes) {
         return Charsets.UTF_8.decode(ByteBuffer.wrap(bytes)).toString();
     }
 

©2013 OpenDaylight, A Linux Foundation Collaborative Project. All Rights Reserved.
OpenDaylight is a registered trademark of The OpenDaylight Project, Inc.
Linux Foundation and OpenDaylight are registered trademarks of the Linux Foundation.
Linux is a registered trademark of Linus Torvalds.