Use instanceof pattern in RemoteNetconfCommand
[netconf.git] / netconf / netconf-impl / src / main / java / org / opendaylight / netconf / impl / osgi / NetconfOperationRouterImpl.java
index b7db507ca577133acf9be0b6728ca0e962a9c61e..3aa1c7107243a89c3a1a92ff3e15131eb2ec3a09 100644 (file)
@@ -7,18 +7,19 @@
  */
 package org.opendaylight.netconf.impl.osgi;
 
-import com.google.common.base.Preconditions;
+import static com.google.common.base.Preconditions.checkState;
+import static java.util.Objects.requireNonNull;
+
 import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Maps;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.HashSet;
+import java.util.Map;
 import java.util.NavigableMap;
 import java.util.Set;
 import java.util.TreeMap;
-import org.opendaylight.controller.config.util.xml.DocumentedException;
-import org.opendaylight.controller.config.util.xml.XmlUtil;
+import org.opendaylight.netconf.api.DocumentedException;
 import org.opendaylight.netconf.api.monitoring.NetconfMonitoringService;
+import org.opendaylight.netconf.api.xml.XmlUtil;
 import org.opendaylight.netconf.impl.NetconfServerSession;
 import org.opendaylight.netconf.impl.mapping.operations.DefaultCloseSession;
 import org.opendaylight.netconf.impl.mapping.operations.DefaultNetconfOperation;
@@ -29,6 +30,9 @@ import org.opendaylight.netconf.mapping.api.NetconfOperation;
 import org.opendaylight.netconf.mapping.api.NetconfOperationChainedExecution;
 import org.opendaylight.netconf.mapping.api.NetconfOperationService;
 import org.opendaylight.netconf.mapping.api.SessionAwareNetconfOperation;
+import org.opendaylight.yangtools.yang.common.ErrorSeverity;
+import org.opendaylight.yangtools.yang.common.ErrorTag;
+import org.opendaylight.yangtools.yang.common.ErrorType;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.w3c.dom.Document;
@@ -41,7 +45,7 @@ public class NetconfOperationRouterImpl implements NetconfOperationRouter {
 
     public NetconfOperationRouterImpl(final NetconfOperationService netconfOperationServiceSnapshot,
                                       final NetconfMonitoringService netconfMonitoringService, final String sessionId) {
-        this.netconfOperationServiceSnapshot = Preconditions.checkNotNull(netconfOperationServiceSnapshot);
+        this.netconfOperationServiceSnapshot = requireNonNull(netconfOperationServiceSnapshot);
 
         final Set<NetconfOperation> ops = new HashSet<>();
         ops.add(new DefaultCloseSession(sessionId, this));
@@ -57,7 +61,7 @@ public class NetconfOperationRouterImpl implements NetconfOperationRouter {
     @Override
     public Document onNetconfMessage(final Document message, final NetconfServerSession session) throws
             DocumentedException {
-        Preconditions.checkNotNull(allNetconfOperations, "Operation router was not initialized properly");
+        requireNonNull(allNetconfOperations, "Operation router was not initialized properly");
 
         final NetconfOperationExecution netconfOperationExecution;
         try {
@@ -66,47 +70,57 @@ public class NetconfOperationRouterImpl implements NetconfOperationRouter {
             final String messageAsString = XmlUtil.toString(message);
             LOG.warn("Unable to handle rpc {} on session {}", messageAsString, session, e);
 
-            final DocumentedException.ErrorTag tag;
-            if (e instanceof IllegalArgumentException) {
-                tag = DocumentedException.ErrorTag.OPERATION_NOT_SUPPORTED;
-            } else {
-                tag = DocumentedException.ErrorTag.OPERATION_FAILED;
-            }
+            final ErrorTag tag = e instanceof IllegalArgumentException ? ErrorTag.OPERATION_NOT_SUPPORTED
+                : ErrorTag.OPERATION_FAILED;
 
             throw new DocumentedException(
-                    String.format("Unable to handle rpc %s on session %s", messageAsString, session),
-                    e, DocumentedException.ErrorType.APPLICATION,
-                    tag, DocumentedException.ErrorSeverity.ERROR,
-                    Collections.singletonMap(tag.toString(), e.getMessage()));
+                    String.format("Unable to handle rpc %s on session %s", messageAsString, session), e,
+                    ErrorType.APPLICATION, tag, ErrorSeverity.ERROR,
+                    // FIXME: i.e. in what namespace are we providing these tags? why is this not just:
+                    //
+                    // <java-throwable xmlns="org.opendaylight.something">
+                    //   <message>e.getMessage()</message>
+                    // </java-throwable>
+                    //
+                    // for each place where we are mapping Exception.getMessage() ? We probably do not want to propagate
+                    // stack traces out, but suppressed exceptions and causal list might be interesting:
+                    //
+                    // <java-throwable xmlns="org.opendaylight.something">
+                    //   <message>reported exception</message>
+                    // </java-throwable>
+                    // <java-throwable xmlns="org.opendaylight.something">
+                    //   <message>cause of reported exception</message>
+                    // </java-throwable>
+                    // <java-throwable xmlns="org.opendaylight.something">
+                    //   <message>cause of cause of reported exception</message>
+                    // </java-throwable>
+                    Map.of(tag.elementBody(), e.getMessage()));
         } catch (final RuntimeException e) {
-            throw handleUnexpectedEx("Unexpected exception during netconf operation sort", e);
+            throw handleUnexpectedEx("sort", e);
         }
 
         try {
             return executeOperationWithHighestPriority(message, netconfOperationExecution);
         } catch (final RuntimeException e) {
-            throw handleUnexpectedEx("Unexpected exception during netconf operation execution", e);
+            throw handleUnexpectedEx("execution", e);
         }
     }
 
     @Override
-    public void close() throws Exception {
+    public void close() {
         netconfOperationServiceSnapshot.close();
     }
 
-    private static DocumentedException handleUnexpectedEx(final String message, final Exception exception) throws
-            DocumentedException {
-        LOG.error("{}", message, exception);
+    private static DocumentedException handleUnexpectedEx(final String op, final Exception exception) {
+        LOG.error("Unexpected exception during netconf operation {}", op, exception);
         return new DocumentedException("Unexpected error",
-                DocumentedException.ErrorType.APPLICATION,
-                DocumentedException.ErrorTag.OPERATION_FAILED,
-                DocumentedException.ErrorSeverity.ERROR,
-                Collections.singletonMap(DocumentedException.ErrorSeverity.ERROR.toString(), exception.toString()));
+                ErrorType.APPLICATION, ErrorTag.OPERATION_FAILED, ErrorSeverity.ERROR,
+                // FIXME: i.e. <error>exception.toString()</error>? That looks wrong on a few levels.
+                Map.of(ErrorSeverity.ERROR.elementBody(), exception.toString()));
     }
 
-    private Document executeOperationWithHighestPriority(final Document message,
-                                                         final NetconfOperationExecution netconfOperationExecution)
-            throws DocumentedException {
+    private static Document executeOperationWithHighestPriority(final Document message,
+            final NetconfOperationExecution netconfOperationExecution) throws DocumentedException {
         if (LOG.isDebugEnabled()) {
             LOG.debug("Forwarding netconf message {} to {}", XmlUtil.toString(message), netconfOperationExecution
                     .netconfOperation);
@@ -132,7 +146,7 @@ public class NetconfOperationRouterImpl implements NetconfOperationRouter {
 
     private TreeMap<HandlingPriority, NetconfOperation> getSortedNetconfOperationsWithCanHandle(
             final Document message, final NetconfServerSession session) throws DocumentedException {
-        final TreeMap<HandlingPriority, NetconfOperation> sortedPriority = Maps.newTreeMap();
+        final TreeMap<HandlingPriority, NetconfOperation> sortedPriority = new TreeMap<>();
 
         for (final NetconfOperation netconfOperation : allNetconfOperations) {
             final HandlingPriority handlingPriority = netconfOperation.canHandle(message);
@@ -144,7 +158,7 @@ public class NetconfOperationRouterImpl implements NetconfOperationRouter {
             }
             if (!handlingPriority.equals(HandlingPriority.CANNOT_HANDLE)) {
 
-                Preconditions.checkState(!sortedPriority.containsKey(handlingPriority),
+                checkState(!sortedPriority.containsKey(handlingPriority),
                         "Multiple %s available to handle message %s with priority %s, %s and %s",
                         NetconfOperation.class.getName(), message, handlingPriority, netconfOperation, sortedPriority
                                 .get(handlingPriority));
@@ -154,7 +168,7 @@ public class NetconfOperationRouterImpl implements NetconfOperationRouter {
         return sortedPriority;
     }
 
-    private static class NetconfOperationExecution implements NetconfOperationChainedExecution {
+    private static final class NetconfOperationExecution implements NetconfOperationChainedExecution {
         private final NetconfOperation netconfOperation;
         private final NetconfOperationChainedExecution subsequentExecution;