Eliminate HandlingPriority.CANNOT_HANDLE

This is an ugly special case. It really is used only as a return from
canHandle(), which in turn can use a @Nullable return to represent this.

Change-Id: I87ce5f1a4391e04de78377c1d4d27b55e2fe80e2
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/plugins/netconf-server-mdsal/src/main/java/org/opendaylight/netconf/server/mdsal/operations/RuntimeRpc.java b/plugins/netconf-server-mdsal/src/main/java/org/opendaylight/netconf/server/mdsal/operations/RuntimeRpc.java
index db2b4fe2d19c1cb1216cef5e12452419be0bdc4c..2c5b8d6e0ff7f40698f3bcf950f19cd8423e2f66 100644 (file)
--- a/plugins/netconf-server-mdsal/src/main/java/org/opendaylight/netconf/server/mdsal/operations/RuntimeRpc.java
+++ b/plugins/netconf-server-mdsal/src/main/java/org/opendaylight/netconf/server/mdsal/operations/RuntimeRpc.java
@@ -79,7 +79,7 @@ public final class RuntimeRpc extends AbstractSingletonNetconfOperation {
             .flatMap(module -> getRpcDefinitionFromModule(module, xmlNamespace, netconfOperationName));
         if (rpcDef.isEmpty()) {
             LOG.debug("Cannot handle rpc: {}, {}", netconfOperationName, namespace);
-            return HandlingPriority.CANNOT_HANDLE;
+            return null;
         }
         return HandlingPriority.HANDLE_WITH_DEFAULT_PRIORITY;
     }

diff --git a/plugins/netconf-server-mdsal/src/test/java/org/opendaylight/netconf/server/mdsal/operations/RuntimeRpcTest.java b/plugins/netconf-server-mdsal/src/test/java/org/opendaylight/netconf/server/mdsal/operations/RuntimeRpcTest.java
index 905c26650f2ac9f9e6d182333b24765ec1f9682a..501b0051385795f2d69d78265d3c3090a66ebf12 100644 (file)
--- a/plugins/netconf-server-mdsal/src/test/java/org/opendaylight/netconf/server/mdsal/operations/RuntimeRpcTest.java
+++ b/plugins/netconf-server-mdsal/src/test/java/org/opendaylight/netconf/server/mdsal/operations/RuntimeRpcTest.java
@@ -8,13 +8,13 @@
 package org.opendaylight.netconf.server.mdsal.operations;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.doNothing;
 
-import com.google.common.base.Preconditions;
 import com.google.common.io.CharSource;
 import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
@@ -188,7 +188,7 @@ public class RuntimeRpcTest {
 
         final Document rpcDocument = XmlFileLoader.xmlFileToDocument("messages/mapping/rpcs/rpc-void-output.xml");
         final HandlingPriority priority = rpc.canHandle(rpcDocument);
-        Preconditions.checkState(priority != HandlingPriority.CANNOT_HANDLE);
+        assertNotNull(priority);
 
         final Document response = rpc.handle(rpcDocument, NetconfOperationChainedExecution.EXECUTION_TERMINATION_POINT);
 
@@ -202,7 +202,7 @@ public class RuntimeRpcTest {
 
         final Document rpcDocument = XmlFileLoader.xmlFileToDocument("messages/mapping/rpcs/rpc-nonvoid.xml");
         final HandlingPriority priority = rpc.canHandle(rpcDocument);
-        Preconditions.checkState(priority != HandlingPriority.CANNOT_HANDLE);
+        assertNotNull(priority);
 
         final Document response = rpc.handle(rpcDocument, NetconfOperationChainedExecution.EXECUTION_TERMINATION_POINT);
         verifyResponse(response, XmlFileLoader.xmlFileToDocument("messages/mapping/rpcs/rpc-nonvoid-control.xml"));
@@ -215,7 +215,7 @@ public class RuntimeRpcTest {
 
         final Document rpcDocument = XmlFileLoader.xmlFileToDocument("messages/mapping/rpcs/rpc-container.xml");
         final HandlingPriority priority = rpc.canHandle(rpcDocument);
-        Preconditions.checkState(priority != HandlingPriority.CANNOT_HANDLE);
+        assertNotNull(priority);
 
         final Document response = rpc.handle(rpcDocument, NetconfOperationChainedExecution.EXECUTION_TERMINATION_POINT);
         verifyResponse(response, XmlFileLoader.xmlFileToDocument("messages/mapping/rpcs/rpc-container-control.xml"));
@@ -228,7 +228,7 @@ public class RuntimeRpcTest {
 
         final Document rpcDocument = XmlFileLoader.xmlFileToDocument("messages/mapping/rpcs/rpc-nonvoid.xml");
         final HandlingPriority priority = rpc.canHandle(rpcDocument);
-        Preconditions.checkState(priority != HandlingPriority.CANNOT_HANDLE);
+        assertNotNull(priority);
 
         final DocumentedException e = assertThrows(DocumentedException.class,
                 () -> rpc.handle(rpcDocument, NetconfOperationChainedExecution.EXECUTION_TERMINATION_POINT));
@@ -244,7 +244,7 @@ public class RuntimeRpcTest {
 
         final Document rpcDocument = XmlFileLoader.xmlFileToDocument("messages/mapping/rpcs/rpc-void-input-output.xml");
         final HandlingPriority priority = rpc.canHandle(rpcDocument);
-        Preconditions.checkState(priority != HandlingPriority.CANNOT_HANDLE);
+        assertNotNull(priority);
 
         final Document response = rpc.handle(rpcDocument, NetconfOperationChainedExecution.EXECUTION_TERMINATION_POINT);
 

diff --git a/protocol/netconf-server/src/main/java/org/opendaylight/netconf/server/api/operations/AbstractNetconfOperation.java b/protocol/netconf-server/src/main/java/org/opendaylight/netconf/server/api/operations/AbstractNetconfOperation.java
index 2d3c8b53fe098325151d37a3b4e7eaa74a7a0281..99f83111959231ffda29867e1c7665c33469530a 100644 (file)
--- a/protocol/netconf-server/src/main/java/org/opendaylight/netconf/server/api/operations/AbstractNetconfOperation.java
+++ b/protocol/netconf-server/src/main/java/org/opendaylight/netconf/server/api/operations/AbstractNetconfOperation.java
@@ -40,8 +40,7 @@ public abstract class AbstractNetconfOperation implements NetconfOperation {
 
     protected HandlingPriority canHandle(final String operationName, final String operationNamespace) {
         return operationName.equals(getOperationName()) && operationNamespace.equals(getOperationNamespace())
-                ? getHandlingPriority()
-                : HandlingPriority.CANNOT_HANDLE;
+            ? getHandlingPriority() : null;
     }
 
     public static final class OperationNameAndNamespace {
@@ -74,7 +73,7 @@ public abstract class AbstractNetconfOperation implements NetconfOperation {
             NamespaceURN.BASE);
     }
 
-    protected HandlingPriority getHandlingPriority() {
+    protected @NonNull HandlingPriority getHandlingPriority() {
         return HandlingPriority.HANDLE_WITH_DEFAULT_PRIORITY;
     }
 

diff --git a/protocol/netconf-server/src/main/java/org/opendaylight/netconf/server/api/operations/HandlingPriority.java b/protocol/netconf-server/src/main/java/org/opendaylight/netconf/server/api/operations/HandlingPriority.java
index aa076eb4d672113e376e1bb42ef1f6e08d451918..5fe4adbce7cce39dce46a7d31f716529e4afe620 100644 (file)
--- a/protocol/netconf-server/src/main/java/org/opendaylight/netconf/server/api/operations/HandlingPriority.java
+++ b/protocol/netconf-server/src/main/java/org/opendaylight/netconf/server/api/operations/HandlingPriority.java
@@ -8,67 +8,24 @@
 package org.opendaylight.netconf.server.api.operations;
 
 import static com.google.common.base.Preconditions.checkArgument;
-import static com.google.common.base.Preconditions.checkState;
 
-import com.google.common.base.MoreObjects;
-import java.util.Objects;
-import java.util.Optional;
 import org.eclipse.jdt.annotation.NonNull;
 
-public final class HandlingPriority implements Comparable<HandlingPriority> {
-    // FIXME: remote this constant
-    public static final HandlingPriority CANNOT_HANDLE = new HandlingPriority(null);
-    public static final HandlingPriority HANDLE_WITH_DEFAULT_PRIORITY = new HandlingPriority(Integer.MIN_VALUE);
-    public static final HandlingPriority HANDLE_WITH_MAX_PRIORITY = new HandlingPriority(Integer.MAX_VALUE);
-
-    private final Integer priority;
-
-    private HandlingPriority(final Integer priority) {
-        this.priority = priority;
-    }
-
-    public static @NonNull HandlingPriority of(final int priority) {
-        return new HandlingPriority(priority);
-    }
-
-    /**
-     * Get priority number.
-     *
-     * @return priority number or Optional.absent otherwise
-     */
-    public Optional<Integer> getPriority() {
-        return Optional.ofNullable(priority);
-    }
+public record HandlingPriority(int priority) implements Comparable<HandlingPriority> {
+    public static final @NonNull HandlingPriority HANDLE_WITH_DEFAULT_PRIORITY =
+        new HandlingPriority(Integer.MIN_VALUE);
+    public static final @NonNull HandlingPriority HANDLE_WITH_MAX_PRIORITY = new HandlingPriority(Integer.MAX_VALUE);
 
     public HandlingPriority increasePriority(final int priorityIncrease) {
-        checkState(priority != null, "Unable to increase priority for %s", this);
         checkArgument(priorityIncrease > 0, "Negative increase");
         checkArgument(Long.valueOf(priority) + priorityIncrease < Integer.MAX_VALUE,
                 "Resulting priority cannot be higher than %s", Integer.MAX_VALUE);
-        return of(priority + priorityIncrease);
+        return new HandlingPriority(priority + priorityIncrease);
     }
 
     @Override
     @SuppressWarnings("checkstyle:parameterName")
     public int compareTo(final HandlingPriority o) {
-        if (priority == null) {
-            return o.priority == null ? 0 : -1;
-        }
-        return o.priority == null ? 1 : Integer.compare(priority, o.priority);
-    }
-
-    @Override
-    public boolean equals(final Object obj) {
-        return this == obj || obj instanceof HandlingPriority other && Objects.equals(priority, other.priority);
-    }
-
-    @Override
-    public int hashCode() {
-        return Objects.hashCode(priority) ;
-    }
-
-    @Override
-    public String toString() {
-        return MoreObjects.toStringHelper(this).add("priority", priority).toString();
+        return Integer.compare(priority, o.priority);
     }
 }

diff --git a/protocol/netconf-server/src/main/java/org/opendaylight/netconf/server/api/operations/NetconfOperation.java b/protocol/netconf-server/src/main/java/org/opendaylight/netconf/server/api/operations/NetconfOperation.java
index 395a0c0524738887e68268dc8ea62eca52ba01b3..c6f1af826b0ba99707d547d8756893bec85e3054 100644 (file)
--- a/protocol/netconf-server/src/main/java/org/opendaylight/netconf/server/api/operations/NetconfOperation.java
+++ b/protocol/netconf-server/src/main/java/org/opendaylight/netconf/server/api/operations/NetconfOperation.java
@@ -8,6 +8,7 @@
 
 package org.opendaylight.netconf.server.api.operations;
 
+import org.eclipse.jdt.annotation.Nullable;
 import org.opendaylight.netconf.api.DocumentedException;
 import org.w3c.dom.Document;
 
@@ -24,16 +25,15 @@ import org.w3c.dom.Document;
  * the results.
  */
 public interface NetconfOperation {
-
     /**
-     * Singleton operations should return
-     * HandlingPriority.HANDLE_WITH_MAX_PRIORITY, last operations
-     * HandlingPriority.HANDLE_WITH_DEFAULT_PRIORITY.
+     * Singleton operations should return {@link HandlingPriority#HANDLE_WITH_MAX_PRIORITY}, last operations
+     * {@link HandlingPriority#HANDLE_WITH_DEFAULT_PRIORITY}.
      *
      * @param message request message
-     * @return {@code handling priority}
+     * @return A {@link HandlingPriority}, or {@code null} if the message is not handled
+     * @throws DocumentedException if the message is found to have structural errors
      */
-    HandlingPriority canHandle(Document message) throws DocumentedException;
+    @Nullable HandlingPriority canHandle(Document message) throws DocumentedException;
 
     /**
      * Execute current netconf operation and trigger execution of subsequent

diff --git a/protocol/netconf-server/src/main/java/org/opendaylight/netconf/server/osgi/NetconfOperationRouterImpl.java b/protocol/netconf-server/src/main/java/org/opendaylight/netconf/server/osgi/NetconfOperationRouterImpl.java
index fd65b91396d11f00b4ed470a984d8ca5196a1568..6b644c838599c8d22515a0f6c1909d22d161c660 100644 (file)
--- a/protocol/netconf-server/src/main/java/org/opendaylight/netconf/server/osgi/NetconfOperationRouterImpl.java
+++ b/protocol/netconf-server/src/main/java/org/opendaylight/netconf/server/osgi/NetconfOperationRouterImpl.java
@@ -7,7 +7,6 @@
  */
 package org.opendaylight.netconf.server.osgi;
 
-import static com.google.common.base.Preconditions.checkState;
 import static java.util.Objects.requireNonNull;
 
 import com.google.common.collect.ImmutableSet;
@@ -148,23 +147,23 @@ public class NetconfOperationRouterImpl implements NetconfOperationRouter, AutoC
 
     private TreeMap<HandlingPriority, NetconfOperation> getSortedNetconfOperationsWithCanHandle(
             final Document message, final NetconfServerSession session) throws DocumentedException {
-        final TreeMap<HandlingPriority, NetconfOperation> sortedPriority = new TreeMap<>();
-
-        for (final NetconfOperation netconfOperation : allNetconfOperations) {
-            final HandlingPriority handlingPriority = netconfOperation.canHandle(message);
-            if (netconfOperation instanceof DefaultNetconfOperation defaultOperation) {
-                defaultOperation.setNetconfSession(session);
-            }
-            if (netconfOperation instanceof SessionAwareNetconfOperation sessionAwareOperation) {
-                sessionAwareOperation.setSession(session);
-            }
-            if (!handlingPriority.equals(HandlingPriority.CANNOT_HANDLE)) {
-
-                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));
-                sortedPriority.put(handlingPriority, netconfOperation);
+        final var sortedPriority = new TreeMap<HandlingPriority, NetconfOperation>();
+        for (var netconfOperation : allNetconfOperations) {
+            final var handlingPriority = netconfOperation.canHandle(message);
+            if (handlingPriority != null) {
+                final var existing = sortedPriority.putIfAbsent(handlingPriority, netconfOperation);
+                if (existing != null) {
+                    throw new IllegalStateException(
+                        "Multiple %s available to handle message %s with priority %s, %s and %s".formatted(
+                            NetconfOperation.class.getName(), message, handlingPriority, netconfOperation, existing));
+                }
+
+                if (netconfOperation instanceof DefaultNetconfOperation defaultOperation) {
+                    defaultOperation.setNetconfSession(session);
+                }
+                if (netconfOperation instanceof SessionAwareNetconfOperation sessionAwareOperation) {
+                    sessionAwareOperation.setSession(session);
+                }
             }
         }
         return sortedPriority;

diff --git a/protocol/netconf-server/src/test/java/org/opendaylight/netconf/server/ConcurrentClientsTest.java b/protocol/netconf-server/src/test/java/org/opendaylight/netconf/server/ConcurrentClientsTest.java
index 701bd82436c41810b035367acdd134e53098a565..e978fe8d6d4315a307946780de6b869838480338 100644 (file)
--- a/protocol/netconf-server/src/test/java/org/opendaylight/netconf/server/ConcurrentClientsTest.java
+++ b/protocol/netconf-server/src/test/java/org/opendaylight/netconf/server/ConcurrentClientsTest.java
@@ -212,7 +212,7 @@ public class ConcurrentClientsTest {
             }
         }
 
-        assertEquals(CONCURRENCY, testingNetconfOperation.getMessageCount());
+        assertEquals(CONCURRENCY, testingNetconfOperation.counter.get());
     }
 
     public static Set<String> getOnlyExiServerCaps() {
@@ -236,26 +236,21 @@ public class ConcurrentClientsTest {
         @Override
         public HandlingPriority canHandle(final Document message) {
             return XmlUtil.toString(message).contains(NetconfStartExiMessageProvider.START_EXI)
-                    ? HandlingPriority.CANNOT_HANDLE :
-                    HandlingPriority.HANDLE_WITH_MAX_PRIORITY;
+                ? null : HandlingPriority.HANDLE_WITH_MAX_PRIORITY;
         }
 
         @SuppressWarnings("checkstyle:IllegalCatch")
         @Override
         public Document handle(final Document requestMessage,
                 final NetconfOperationChainedExecution subsequentOperation) throws DocumentedException {
+            LOG.info("Handling netconf message from test {}", XmlUtil.toString(requestMessage));
+            counter.getAndIncrement();
             try {
-                LOG.info("Handling netconf message from test {}", XmlUtil.toString(requestMessage));
-                counter.getAndIncrement();
                 return XmlUtil.readXmlToDocument("<test/>");
             } catch (Exception e) {
                 throw new RuntimeException(e);
             }
         }
-
-        public long getMessageCount() {
-            return counter.get();
-        }
     }
 
     /**

diff --git a/protocol/netconf-server/src/test/java/org/opendaylight/netconf/server/api/operations/AbstractNetconfOperationTest.java b/protocol/netconf-server/src/test/java/org/opendaylight/netconf/server/api/operations/AbstractNetconfOperationTest.java
index 2933ad4774e055ad0907ef7550c51a4cd205b4f8..490f93e9c4bfd3e6c078a4c56fd1974e43dfa5db 100644 (file)
--- a/protocol/netconf-server/src/test/java/org/opendaylight/netconf/server/api/operations/AbstractNetconfOperationTest.java
+++ b/protocol/netconf-server/src/test/java/org/opendaylight/netconf/server/api/operations/AbstractNetconfOperationTest.java
@@ -8,7 +8,6 @@
 package org.opendaylight.netconf.server.api.operations;
 
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.Mockito.mock;
 
@@ -35,7 +34,7 @@ public class AbstractNetconfOperationTest {
 
         @Override
         protected String getOperationName() {
-            return null;
+            return "edit-config";
         }
 
         @Override
@@ -62,8 +61,7 @@ public class AbstractNetconfOperationTest {
     public void testAbstractNetconfOperation() throws Exception {
         Document helloMessage = XmlFileLoader.xmlFileToDocument("netconfMessages/edit_config.xml");
         assertEquals(new SessionIdType(Uint32.ONE), netconfOperation.sessionId());
-        assertNotNull(netconfOperation.canHandle(helloMessage));
-        assertEquals(HandlingPriority.HANDLE_WITH_DEFAULT_PRIORITY, netconfOperation.getHandlingPriority());
+        assertEquals(HandlingPriority.HANDLE_WITH_DEFAULT_PRIORITY, netconfOperation.canHandle(helloMessage));
 
         netconfOperation.handle(helloMessage, operation);
         assertTrue(netconfOperation.handleRun);

diff --git a/protocol/netconf-server/src/test/java/org/opendaylight/netconf/server/api/operations/HandlingPriorityTest.java b/protocol/netconf-server/src/test/java/org/opendaylight/netconf/server/api/operations/HandlingPriorityTest.java
index 15fe60ac62b58ecfd2e6577302c15293c1bc760f..ac75de18c53414ba5564e01790db540329480500 100644 (file)
--- a/protocol/netconf-server/src/test/java/org/opendaylight/netconf/server/api/operations/HandlingPriorityTest.java
+++ b/protocol/netconf-server/src/test/java/org/opendaylight/netconf/server/api/operations/HandlingPriorityTest.java
@@ -18,23 +18,20 @@ public class HandlingPriorityTest {
     public void testHandlingPriority() {
         assertEquals(0,
             HandlingPriority.HANDLE_WITH_DEFAULT_PRIORITY.compareTo(HandlingPriority.HANDLE_WITH_DEFAULT_PRIORITY));
-        assertEquals(-1, HandlingPriority.CANNOT_HANDLE.compareTo(HandlingPriority.HANDLE_WITH_DEFAULT_PRIORITY));
-        assertEquals(1, HandlingPriority.HANDLE_WITH_DEFAULT_PRIORITY.compareTo(HandlingPriority.CANNOT_HANDLE));
 
         assertEquals(-1,
             HandlingPriority.HANDLE_WITH_DEFAULT_PRIORITY.compareTo(HandlingPriority.HANDLE_WITH_MAX_PRIORITY));
         assertEquals(1,
             HandlingPriority.HANDLE_WITH_MAX_PRIORITY.compareTo(HandlingPriority.HANDLE_WITH_DEFAULT_PRIORITY));
-        assertEquals(0, HandlingPriority.of(Integer.MIN_VALUE)
+        assertEquals(0, new HandlingPriority(Integer.MIN_VALUE)
             .compareTo(HandlingPriority.HANDLE_WITH_DEFAULT_PRIORITY));
 
-        HandlingPriority prio = HandlingPriority.of(10);
-        assertTrue(prio.increasePriority(1).compareTo(HandlingPriority.of(11)) == 0);
+        HandlingPriority prio = new HandlingPriority(10);
+        assertTrue(prio.increasePriority(1).compareTo(new HandlingPriority(11)) == 0);
 
-        assertFalse(HandlingPriority.CANNOT_HANDLE.getPriority().isPresent());
         assertFalse(HandlingPriority.HANDLE_WITH_MAX_PRIORITY.equals(new Object()));
-        assertEquals(HandlingPriority.HANDLE_WITH_MAX_PRIORITY, HandlingPriority.of(Integer.MAX_VALUE));
+        assertEquals(HandlingPriority.HANDLE_WITH_MAX_PRIORITY, new HandlingPriority(Integer.MAX_VALUE));
         assertEquals(HandlingPriority.HANDLE_WITH_MAX_PRIORITY.hashCode(),
-            HandlingPriority.of(Integer.MAX_VALUE).hashCode());
+            new HandlingPriority(Integer.MAX_VALUE).hashCode());
     }
 }