From 605c77bef80cc4436f4000992e4237a7f6cdf23a Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Thu, 8 Dec 2022 19:32:19 +0100 Subject: [PATCH] Do not store Optional in NetconfDeviceCommunicator The instantiation pattern can be made significantly better without an Optional, which leads to quite a bit of simplification. Change-Id: I9e67ff28faa1d4ac4fc99c3632409ed77cdf381e Signed-off-by: Robert Varga --- .../impl/RemoteDeviceConnectorImpl.java | 37 +++++++------- .../topology/spi/AbstractNetconfTopology.java | 50 ++++++++++--------- .../listener/NetconfDeviceCommunicator.java | 26 +++++----- 3 files changed, 60 insertions(+), 53 deletions(-) diff --git a/netconf/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/RemoteDeviceConnectorImpl.java b/netconf/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/RemoteDeviceConnectorImpl.java index 1fcba210f0..3ce67e804e 100644 --- a/netconf/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/RemoteDeviceConnectorImpl.java +++ b/netconf/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/RemoteDeviceConnectorImpl.java @@ -21,7 +21,7 @@ import java.net.URL; import java.util.ArrayList; import java.util.List; import java.util.Map; -import java.util.Optional; +import org.eclipse.jdt.annotation.Nullable; import org.opendaylight.aaa.encrypt.AAAEncryptionService; import org.opendaylight.netconf.client.NetconfClientSessionListener; import org.opendaylight.netconf.client.conf.NetconfClientConfiguration; @@ -209,17 +209,16 @@ public class RemoteDeviceConnectorImpl implements RemoteDeviceConnector { .build(); } - final Optional userCapabilities = getUserCapabilities(node); - final int rpcMessageLimit = - node.getConcurrentRpcLimit() == null - ? NetconfTopologyUtils.DEFAULT_CONCURRENT_RPC_LIMIT : node.getConcurrentRpcLimit().toJava(); + final int rpcMessageLimit = node.getConcurrentRpcLimit() == null + ? NetconfTopologyUtils.DEFAULT_CONCURRENT_RPC_LIMIT : node.getConcurrentRpcLimit().toJava(); if (rpcMessageLimit < 1) { LOG.info("{}: Concurrent rpc limit is smaller than 1, no limit will be enforced.", remoteDeviceId); } - NetconfDeviceCommunicator netconfDeviceCommunicator = userCapabilities.isPresent() - ? new NetconfDeviceCommunicator(remoteDeviceId, device, new UserPreferences(userCapabilities.get(), + final var userCapabilities = extractUserCapabilities(node); + final var netconfDeviceCommunicator = userCapabilities != null + ? new NetconfDeviceCommunicator(remoteDeviceId, device, new UserPreferences(userCapabilities, node.getYangModuleCapabilities() == null ? false : node.getYangModuleCapabilities().getOverride(), node.getNonModuleCapabilities() == null ? false : node.getNonModuleCapabilities().getOverride()), rpcMessageLimit) @@ -231,27 +230,31 @@ public class RemoteDeviceConnectorImpl implements RemoteDeviceConnector { return new NetconfConnectorDTO(netconfDeviceCommunicator, salFacade, registeredYangLibSources); } - private static Optional getUserCapabilities(final NetconfNode node) { - if (node.getYangModuleCapabilities() == null && node.getNonModuleCapabilities() == null) { - return Optional.empty(); + // FIXME: reconcile with AbstractNetconfTopology + private static @Nullable NetconfSessionPreferences extractUserCapabilities(final NetconfNode node) { + final var moduleCaps = node.getYangModuleCapabilities(); + final var nonModuleCaps = node.getNonModuleCapabilities(); + + if (moduleCaps == null && node.getNonModuleCapabilities() == null) { + return null; } - final List capabilities = new ArrayList<>(); - if (node.getYangModuleCapabilities() != null) { - capabilities.addAll(node.getYangModuleCapabilities().getCapability()); + final var capabilities = new ArrayList(); + if (moduleCaps != null) { + capabilities.addAll(moduleCaps.getCapability()); } //non-module capabilities should not exist in yang module capabilities - final NetconfSessionPreferences netconfSessionPreferences = NetconfSessionPreferences.fromStrings(capabilities); + final var netconfSessionPreferences = NetconfSessionPreferences.fromStrings(capabilities); checkState(netconfSessionPreferences.getNonModuleCaps().isEmpty(), "List yang-module-capabilities/capability should contain only module based capabilities. " + "Non-module capabilities used: " + netconfSessionPreferences.getNonModuleCaps()); - if (node.getNonModuleCapabilities() != null) { - capabilities.addAll(node.getNonModuleCapabilities().getCapability()); + if (nonModuleCaps != null) { + capabilities.addAll(nonModuleCaps.getCapability()); } - return Optional.of(NetconfSessionPreferences.fromStrings(capabilities, CapabilityOrigin.UserDefined)); + return NetconfSessionPreferences.fromStrings(capabilities, CapabilityOrigin.UserDefined); } @VisibleForTesting diff --git a/netconf/netconf-topology/src/main/java/org/opendaylight/netconf/topology/spi/AbstractNetconfTopology.java b/netconf/netconf-topology/src/main/java/org/opendaylight/netconf/topology/spi/AbstractNetconfTopology.java index ccd258ccf0..d7bf00e8f6 100644 --- a/netconf/netconf-topology/src/main/java/org/opendaylight/netconf/topology/spi/AbstractNetconfTopology.java +++ b/netconf/netconf-topology/src/main/java/org/opendaylight/netconf/topology/spi/AbstractNetconfTopology.java @@ -23,8 +23,8 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.concurrent.ExecutionException; +import org.eclipse.jdt.annotation.Nullable; import org.opendaylight.aaa.encrypt.AAAEncryptionService; import org.opendaylight.controller.config.threadpool.ScheduledThreadPool; import org.opendaylight.controller.config.threadpool.ThreadPool; @@ -263,16 +263,13 @@ public abstract class AbstractNetconfTopology implements NetconfTopology { yanglibRegistrations = registerDeviceSchemaSources(deviceId, node, resources); } - final Optional userCapabilities = getUserCapabilities(node); final int rpcMessageLimit = node.requireConcurrentRpcLimit().toJava(); if (rpcMessageLimit < 1) { LOG.info("Concurrent rpc limit is smaller than 1, no limit will be enforced for device {}", deviceId); } - final NetconfDeviceCommunicator netconfDeviceCommunicator = - userCapabilities.isPresent() ? new NetconfDeviceCommunicator(deviceId, device, - userCapabilities.get(), rpcMessageLimit) - : new NetconfDeviceCommunicator(deviceId, device, rpcMessageLimit); + final var netconfDeviceCommunicator = new NetconfDeviceCommunicator(deviceId, device, rpcMessageLimit, + extractUserCapabilities(node)); if (keepAliveFacade != null) { keepAliveFacade.setListener(netconfDeviceCommunicator); @@ -390,34 +387,39 @@ public abstract class AbstractNetconfTopology implements NetconfTopology { throw new IllegalStateException("Unsupported credential type: " + credentials.getClass()); } - private static Optional getUserCapabilities(final NetconfNode node) { - // if none of yang-module-capabilities or non-module-capabilities is specified - // just return absent - if (node.getYangModuleCapabilities() == null && node.getNonModuleCapabilities() == null) { - return Optional.empty(); - } + private static @Nullable UserPreferences extractUserCapabilities(final NetconfNode node) { + final var moduleCaps = node.getYangModuleCapabilities(); + final var nonModuleCaps = node.getNonModuleCapabilities(); - final List capabilities = new ArrayList<>(); + if (moduleCaps == null && nonModuleCaps == null) { + // if none of yang-module-capabilities or non-module-capabilities is specified + return null; + } - boolean overrideYangModuleCaps = false; - if (node.getYangModuleCapabilities() != null) { - capabilities.addAll(node.getYangModuleCapabilities().getCapability()); - overrideYangModuleCaps = node.getYangModuleCapabilities().getOverride(); + final var capabilities = new ArrayList(); + final boolean overrideYangModuleCaps; + if (moduleCaps != null) { + capabilities.addAll(moduleCaps.getCapability()); + overrideYangModuleCaps = moduleCaps.getOverride(); + } else { + overrideYangModuleCaps = false; } //non-module capabilities should not exist in yang module capabilities - final NetconfSessionPreferences netconfSessionPreferences = NetconfSessionPreferences.fromStrings(capabilities); + final var netconfSessionPreferences = NetconfSessionPreferences.fromStrings(capabilities); Preconditions.checkState(netconfSessionPreferences.getNonModuleCaps().isEmpty(), "List yang-module-capabilities/capability should contain only module based capabilities. " + "Non-module capabilities used: " + netconfSessionPreferences.getNonModuleCaps()); - boolean overrideNonModuleCaps = false; - if (node.getNonModuleCapabilities() != null) { - capabilities.addAll(node.getNonModuleCapabilities().getCapability()); - overrideNonModuleCaps = node.getNonModuleCapabilities().getOverride(); + final boolean overrideNonModuleCaps; + if (nonModuleCaps != null) { + capabilities.addAll(nonModuleCaps.getCapability()); + overrideNonModuleCaps = nonModuleCaps.getOverride(); + } else { + overrideNonModuleCaps = false; } - return Optional.of(new UserPreferences(NetconfSessionPreferences - .fromStrings(capabilities, CapabilityOrigin.UserDefined), overrideYangModuleCaps, overrideNonModuleCaps)); + return new UserPreferences(NetconfSessionPreferences.fromStrings(capabilities, CapabilityOrigin.UserDefined), + overrideYangModuleCaps, overrideNonModuleCaps); } } diff --git a/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/listener/NetconfDeviceCommunicator.java b/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/listener/NetconfDeviceCommunicator.java index 510de67785..25518c54e9 100644 --- a/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/listener/NetconfDeviceCommunicator.java +++ b/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/listener/NetconfDeviceCommunicator.java @@ -7,6 +7,8 @@ */ package org.opendaylight.netconf.sal.connect.netconf.listener; +import static java.util.Objects.requireNonNull; + import com.google.common.base.Strings; import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.SettableFuture; @@ -16,12 +18,12 @@ import java.util.ArrayDeque; import java.util.ArrayList; import java.util.Iterator; import java.util.List; -import java.util.Optional; import java.util.Queue; import java.util.concurrent.Semaphore; import java.util.concurrent.atomic.AtomicIntegerFieldUpdater; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantLock; +import org.eclipse.jdt.annotation.Nullable; import org.opendaylight.netconf.api.FailedNetconfMessage; import org.opendaylight.netconf.api.NetconfDocumentedException; import org.opendaylight.netconf.api.NetconfMessage; @@ -52,7 +54,7 @@ public class NetconfDeviceCommunicator implements NetconfClientSessionListener, private static final Logger LOG = LoggerFactory.getLogger(NetconfDeviceCommunicator.class); protected final RemoteDevice remoteDevice; - private final Optional overrideNetconfCapabilities; + private final @Nullable UserPreferences overrideNetconfCapabilities; protected final RemoteDeviceId id; private final Lock sessionLock = new ReentrantLock(); @@ -81,17 +83,17 @@ public class NetconfDeviceCommunicator implements NetconfClientSessionListener, public NetconfDeviceCommunicator(final RemoteDeviceId id, final RemoteDevice remoteDevice, final UserPreferences netconfSessionPreferences, final int rpcMessageLimit) { - this(id, remoteDevice, Optional.of(netconfSessionPreferences), rpcMessageLimit); + this(id, remoteDevice, rpcMessageLimit, requireNonNull(netconfSessionPreferences)); } public NetconfDeviceCommunicator(final RemoteDeviceId id, final RemoteDevice remoteDevice, final int rpcMessageLimit) { - this(id, remoteDevice, Optional.empty(), rpcMessageLimit); + this(id, remoteDevice, rpcMessageLimit, null); } - private NetconfDeviceCommunicator(final RemoteDeviceId id, - final RemoteDevice remoteDevice, - final Optional overrideNetconfCapabilities, final int rpcMessageLimit) { + public NetconfDeviceCommunicator(final RemoteDeviceId id, + final RemoteDevice remoteDevice, final int rpcMessageLimit, + final @Nullable UserPreferences overrideNetconfCapabilities) { concurentRpcMsgs = rpcMessageLimit; this.id = id; this.remoteDevice = remoteDevice; @@ -111,14 +113,14 @@ public class NetconfDeviceCommunicator implements NetconfClientSessionListener, LOG.trace("{}: Session advertised capabilities: {}", id, netconfSessionPreferences); - if (overrideNetconfCapabilities.isPresent()) { - final NetconfSessionPreferences sessionPreferences = overrideNetconfCapabilities - .get().getSessionPreferences(); - netconfSessionPreferences = overrideNetconfCapabilities.get().moduleBasedCapsOverrided() + final var localOverride = overrideNetconfCapabilities; + if (localOverride != null) { + final var sessionPreferences = localOverride.getSessionPreferences(); + netconfSessionPreferences = localOverride.moduleBasedCapsOverrided() ? netconfSessionPreferences.replaceModuleCaps(sessionPreferences) : netconfSessionPreferences.addModuleCaps(sessionPreferences); - netconfSessionPreferences = overrideNetconfCapabilities.get().nonModuleBasedCapsOverrided() + netconfSessionPreferences = localOverride.nonModuleBasedCapsOverrided() ? netconfSessionPreferences.replaceNonModuleCaps(sessionPreferences) : netconfSessionPreferences.addNonModuleCaps(sessionPreferences); LOG.debug("{}: Session capabilities overridden, capabilities that will be used: {}", id, -- 2.36.6