From 3417b343bd99a20f4215b7d4f6041880e8667617 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Thu, 8 Dec 2022 03:08:52 +0100 Subject: [PATCH] Turn UserPreferences into a record This is a pure DTO, turn it into a record. JIRA: NETCONF-920 Change-Id: I413d6ae35d498f98774e3570588c777a5b605519 Signed-off-by: Robert Varga --- .../listener/NetconfSessionPreferences.java | 139 +++++++++--------- .../netconf/listener/UserPreferences.java | 40 +---- .../sal/NetconfDeviceDataBrokerTest.java | 14 +- 3 files changed, 81 insertions(+), 112 deletions(-) diff --git a/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/listener/NetconfSessionPreferences.java b/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/listener/NetconfSessionPreferences.java index ed2a4ef132..1a0911287d 100644 --- a/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/listener/NetconfSessionPreferences.java +++ b/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/listener/NetconfSessionPreferences.java @@ -33,19 +33,78 @@ import org.slf4j.LoggerFactory; public record NetconfSessionPreferences( @NonNull ImmutableMap nonModuleCaps, @NonNull ImmutableMap moduleBasedCaps) { - private static final Logger LOG = LoggerFactory.getLogger(NetconfSessionPreferences.class); private static final ParameterMatcher MODULE_PARAM = new ParameterMatcher("module="); private static final ParameterMatcher REVISION_PARAM = new ParameterMatcher("revision="); private static final ParameterMatcher BROKEN_REVISON_PARAM = new ParameterMatcher("amp;revision="); private static final Splitter AMP_SPLITTER = Splitter.on('&'); - private static final Predicate CONTAINS_REVISION = input -> input.contains("revision="); public NetconfSessionPreferences { requireNonNull(nonModuleCaps); requireNonNull(moduleBasedCaps); } + public static @NonNull NetconfSessionPreferences fromNetconfSession(final NetconfClientSession session) { + return fromStrings(session.getServerCapabilities()); + } + + public static @NonNull NetconfSessionPreferences fromStrings(final Collection capabilities) { + // we do not know origin of capabilities from only Strings, so we set it to default value + return fromStrings(capabilities, CapabilityOrigin.DeviceAdvertised); + } + + public static @NonNull NetconfSessionPreferences fromStrings(final Collection capabilities, + final CapabilityOrigin capabilityOrigin) { + final var moduleBasedCaps = new HashMap(); + final var nonModuleCaps = new HashMap(); + + for (final String capability : capabilities) { + nonModuleCaps.put(capability, capabilityOrigin); + final int qmark = capability.indexOf('?'); + if (qmark == -1) { + continue; + } + + final String namespace = capability.substring(0, qmark); + final Iterable queryParams = AMP_SPLITTER.split(capability.substring(qmark + 1)); + final String moduleName = MODULE_PARAM.from(queryParams); + if (Strings.isNullOrEmpty(moduleName)) { + continue; + } + + String revision = REVISION_PARAM.from(queryParams); + if (!Strings.isNullOrEmpty(revision)) { + addModuleQName(moduleBasedCaps, nonModuleCaps, capability, cachedQName(namespace, revision, moduleName), + capabilityOrigin); + continue; + } + + /* + * We have seen devices which mis-escape revision, but the revision may not + * even be there. First check if there is a substring that matches revision. + */ + if (Iterables.any(queryParams, input -> input.contains("revision="))) { + LOG.debug("Netconf device was not reporting revision correctly, trying to get amp;revision="); + revision = BROKEN_REVISON_PARAM.from(queryParams); + if (Strings.isNullOrEmpty(revision)) { + LOG.warn("Netconf device returned revision incorrectly escaped for {}, ignoring it", capability); + addModuleQName(moduleBasedCaps, nonModuleCaps, capability, + cachedQName(namespace, moduleName), capabilityOrigin); + } else { + addModuleQName(moduleBasedCaps, nonModuleCaps, capability, + cachedQName(namespace, revision, moduleName), capabilityOrigin); + } + continue; + } + + // Fallback, no revision provided for module + addModuleQName(moduleBasedCaps, nonModuleCaps, capability, + cachedQName(namespace, moduleName), capabilityOrigin); + } + + return new NetconfSessionPreferences(ImmutableMap.copyOf(nonModuleCaps), ImmutableMap.copyOf(moduleBasedCaps)); + } + public @Nullable CapabilityOrigin capabilityOrigin(final QName capability) { return moduleBasedCaps.get(requireNonNull(capability)); } @@ -76,13 +135,13 @@ public record NetconfSessionPreferences( @Override public String toString() { return MoreObjects.toStringHelper(this) - .add("capabilities", nonModuleCaps) - .add("moduleBasedCapabilities", moduleBasedCaps) - .add("rollback", isRollbackSupported()) - .add("monitoring", isMonitoringSupported()) - .add("candidate", isCandidateSupported()) - .add("writableRunning", isRunningWritable()) - .toString(); + .add("capabilities", nonModuleCaps) + .add("moduleBasedCapabilities", moduleBasedCaps) + .add("rollback", isRollbackSupported()) + .add("monitoring", isMonitoringSupported()) + .add("candidate", isCandidateSupported()) + .add("writableRunning", isRunningWritable()) + .toString(); } public boolean isRollbackSupported() { @@ -161,10 +220,6 @@ public record NetconfSessionPreferences( return new NetconfSessionPreferences(netconfSessionPreferences.nonModuleCaps, moduleBasedCaps); } - public static NetconfSessionPreferences fromNetconfSession(final NetconfClientSession session) { - return fromStrings(session.getServerCapabilities()); - } - private static QName cachedQName(final String namespace, final String revision, final String moduleName) { return QName.create(namespace, revision, moduleName).intern(); } @@ -173,64 +228,6 @@ public record NetconfSessionPreferences( return QName.create(XMLNamespace.of(namespace), moduleName).withoutRevision().intern(); } - public static NetconfSessionPreferences fromStrings(final Collection capabilities) { - // we do not know origin of capabilities from only Strings, so we set it to default value - return fromStrings(capabilities, CapabilityOrigin.DeviceAdvertised); - } - - public static NetconfSessionPreferences fromStrings(final Collection capabilities, - final CapabilityOrigin capabilityOrigin) { - final var moduleBasedCaps = new HashMap(); - final var nonModuleCaps = new HashMap(); - - for (final String capability : capabilities) { - nonModuleCaps.put(capability, capabilityOrigin); - final int qmark = capability.indexOf('?'); - if (qmark == -1) { - continue; - } - - final String namespace = capability.substring(0, qmark); - final Iterable queryParams = AMP_SPLITTER.split(capability.substring(qmark + 1)); - final String moduleName = MODULE_PARAM.from(queryParams); - if (Strings.isNullOrEmpty(moduleName)) { - continue; - } - - String revision = REVISION_PARAM.from(queryParams); - if (!Strings.isNullOrEmpty(revision)) { - addModuleQName(moduleBasedCaps, nonModuleCaps, capability, cachedQName(namespace, revision, moduleName), - capabilityOrigin); - continue; - } - - /* - * We have seen devices which mis-escape revision, but the revision may not - * even be there. First check if there is a substring that matches revision. - */ - if (Iterables.any(queryParams, CONTAINS_REVISION)) { - - LOG.debug("Netconf device was not reporting revision correctly, trying to get amp;revision="); - revision = BROKEN_REVISON_PARAM.from(queryParams); - if (Strings.isNullOrEmpty(revision)) { - LOG.warn("Netconf device returned revision incorrectly escaped for {}, ignoring it", capability); - addModuleQName(moduleBasedCaps, nonModuleCaps, capability, - cachedQName(namespace, moduleName), capabilityOrigin); - } else { - addModuleQName(moduleBasedCaps, nonModuleCaps, capability, - cachedQName(namespace, revision, moduleName), capabilityOrigin); - } - continue; - } - - // Fallback, no revision provided for module - addModuleQName(moduleBasedCaps, nonModuleCaps, capability, - cachedQName(namespace, moduleName), capabilityOrigin); - } - - return new NetconfSessionPreferences(ImmutableMap.copyOf(nonModuleCaps), ImmutableMap.copyOf(moduleBasedCaps)); - } - private static void addModuleQName(final Map moduleBasedCaps, final Map nonModuleCaps, final String capability, final QName qualifiedName, final CapabilityOrigin capabilityOrigin) { diff --git a/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/listener/UserPreferences.java b/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/listener/UserPreferences.java index b6e3e5252b..03b7d32d6d 100644 --- a/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/listener/UserPreferences.java +++ b/netconf/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/netconf/listener/UserPreferences.java @@ -7,19 +7,20 @@ */ package org.opendaylight.netconf.sal.connect.netconf.listener; +import static java.util.Objects.requireNonNull; + import org.eclipse.jdt.annotation.NonNull; /** * DTO with user capabilities to override or merge with device specific capabilities. */ -public class UserPreferences { - - private final NetconfSessionPreferences sessionPreferences; - private final boolean overrideModuleCapabilities; - private final boolean overrideNonModuleCapabilities; +public record UserPreferences( + @NonNull NetconfSessionPreferences sessionPreferences, + boolean overrideModuleCapabilities, + boolean overrideNonModuleCapabilities) { - public UserPreferences(final @NonNull NetconfSessionPreferences sessionPreferences, - final boolean overrideModuleCapabilities, final boolean overrideNonModuleCapabilities) { + public UserPreferences { + requireNonNull(sessionPreferences); if (overrideModuleCapabilities && sessionPreferences.moduleBasedCaps().isEmpty()) { throw new IllegalStateException( @@ -29,31 +30,6 @@ public class UserPreferences { throw new IllegalStateException( "Override non-module based capabilities set true but non-module based capabilities list is empty."); } - - this.sessionPreferences = sessionPreferences; - this.overrideModuleCapabilities = overrideModuleCapabilities; - this.overrideNonModuleCapabilities = overrideNonModuleCapabilities; - } - - public NetconfSessionPreferences getSessionPreferences() { - return sessionPreferences; } - public boolean moduleBasedCapsOverrided() { - return overrideModuleCapabilities; - } - - public boolean nonModuleBasedCapsOverrided() { - return overrideNonModuleCapabilities; - } - - @Override - public String toString() { - final StringBuilder sb = new StringBuilder("UserPreferences{"); - sb.append("sessionPreferences=").append(sessionPreferences); - sb.append(", overrideModuleCapabilities=").append(overrideModuleCapabilities); - sb.append(", overrideNonModuleCapabilities=").append(overrideNonModuleCapabilities); - sb.append('}'); - return sb.toString(); - } } diff --git a/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/sal/NetconfDeviceDataBrokerTest.java b/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/sal/NetconfDeviceDataBrokerTest.java index c7ebc5f53b..a4afaca108 100644 --- a/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/sal/NetconfDeviceDataBrokerTest.java +++ b/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/sal/NetconfDeviceDataBrokerTest.java @@ -16,10 +16,8 @@ import static org.mockito.Mockito.verify; import static org.opendaylight.netconf.sal.connect.netconf.util.NetconfMessageTransformUtil.NETCONF_GET_CONFIG_QNAME; import static org.opendaylight.netconf.sal.connect.netconf.util.NetconfMessageTransformUtil.NETCONF_GET_QNAME; -import com.google.common.collect.ClassToInstanceMap; import java.net.InetSocketAddress; -import java.util.Arrays; -import java.util.Collections; +import java.util.List; import org.junit.AfterClass; import org.junit.Before; import org.junit.BeforeClass; @@ -30,7 +28,6 @@ import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; import org.opendaylight.mdsal.binding.runtime.spi.BindingRuntimeHelpers; import org.opendaylight.mdsal.common.api.LogicalDatastoreType; -import org.opendaylight.mdsal.dom.api.DOMDataBrokerExtension; import org.opendaylight.mdsal.dom.api.DOMDataTreeReadTransaction; import org.opendaylight.mdsal.dom.api.DOMDataTreeReadWriteTransaction; import org.opendaylight.mdsal.dom.api.DOMRpcService; @@ -112,21 +109,20 @@ public class NetconfDeviceDataBrokerTest { @Test public void testDOMFieldsExtensions() { - final ClassToInstanceMap extensions = dataBroker.getExtensions(); - final NetconfDOMDataBrokerFieldsExtension fieldsExtension = extensions.getInstance( + final NetconfDOMDataBrokerFieldsExtension fieldsExtension = dataBroker.getExtensions().getInstance( NetconfDOMDataBrokerFieldsExtension.class); assertNotNull(fieldsExtension); // read-only transaction final NetconfDOMFieldsReadTransaction roTx = fieldsExtension.newReadOnlyTransaction(); roTx.read(LogicalDatastoreType.CONFIGURATION, YangInstanceIdentifier.empty(), - Collections.singletonList(YangInstanceIdentifier.empty())); + List.of(YangInstanceIdentifier.empty())); verify(rpcService).invokeRpc(Mockito.eq(NETCONF_GET_CONFIG_QNAME), any(ContainerNode.class)); // read-write transaction final NetconfDOMFieldsReadWriteTransaction rwTx = fieldsExtension.newReadWriteTransaction(); rwTx.read(LogicalDatastoreType.OPERATIONAL, YangInstanceIdentifier.empty(), - Collections.singletonList(YangInstanceIdentifier.empty())); + List.of(YangInstanceIdentifier.empty())); verify(rpcService).invokeRpc(Mockito.eq(NETCONF_GET_QNAME), any(ContainerNode.class)); } @@ -137,7 +133,7 @@ public class NetconfDeviceDataBrokerTest { } private NetconfDeviceDataBroker getDataBroker(final String... caps) { - NetconfSessionPreferences prefs = NetconfSessionPreferences.fromStrings(Arrays.asList(caps)); + NetconfSessionPreferences prefs = NetconfSessionPreferences.fromStrings(List.of(caps)); final RemoteDeviceId id = new RemoteDeviceId("device-1", InetSocketAddress.createUnresolved("localhost", 17830)); return new NetconfDeviceDataBroker(id, new EmptyMountPointContext(SCHEMA_CONTEXT), rpcService, prefs); -- 2.36.6