From 94faf9d73658e6c2422a3f6f5170963e03f76399 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Sat, 19 Jul 2014 16:53:14 +0200 Subject: [PATCH] Optimize netconf capability handling Inefficient splitting, forward scans and similar. This patch optimizes it as much as possible. Change-Id: Ibf52b3b8f512c6b4936dca3089e73bbb1671eaa7 Signed-off-by: Robert Varga --- .../listener/NetconfSessionCapabilities.java | 120 +++++++++++------- 1 file changed, 76 insertions(+), 44 deletions(-) diff --git a/opendaylight/md-sal/sal-netconf-connector/src/main/java/org/opendaylight/controller/sal/connect/netconf/listener/NetconfSessionCapabilities.java b/opendaylight/md-sal/sal-netconf-connector/src/main/java/org/opendaylight/controller/sal/connect/netconf/listener/NetconfSessionCapabilities.java index 82903ea4ec..8964a80228 100644 --- a/opendaylight/md-sal/sal-netconf-connector/src/main/java/org/opendaylight/controller/sal/connect/netconf/listener/NetconfSessionCapabilities.java +++ b/opendaylight/md-sal/sal-netconf-connector/src/main/java/org/opendaylight/controller/sal/connect/netconf/listener/NetconfSessionCapabilities.java @@ -1,7 +1,15 @@ package org.opendaylight.controller.sal.connect.netconf.listener; -import java.util.Arrays; +import com.google.common.base.Objects; +import com.google.common.base.Optional; +import com.google.common.base.Preconditions; +import com.google.common.base.Predicate; +import com.google.common.base.Splitter; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; + import java.util.Collection; +import java.util.HashSet; import java.util.Set; import org.opendaylight.controller.netconf.client.NetconfClientSession; @@ -10,24 +18,50 @@ import org.opendaylight.yangtools.yang.common.QName; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import com.google.common.base.Objects; -import com.google.common.base.Optional; -import com.google.common.base.Predicate; -import com.google.common.collect.FluentIterable; -import com.google.common.collect.Iterables; -import com.google.common.collect.Sets; - public final class NetconfSessionCapabilities { + private static final class ParameterMatcher { + private final Predicate predicate; + private final int skipLength; + + ParameterMatcher(final String name) { + predicate = new Predicate() { + @Override + public boolean apply(final String input) { + return input.startsWith(name); + } + }; - private static final Logger logger = LoggerFactory.getLogger(NetconfSessionCapabilities.class); + this.skipLength = name.length(); + } - private final Set capabilities; + private String from(final Iterable params) { + final Optional o = Iterables.tryFind(params, predicate); + if (!o.isPresent()) { + return null; + } + + return o.get().substring(skipLength); + } + } + + private static final Logger LOG = LoggerFactory.getLogger(NetconfSessionCapabilities.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 = new Predicate() { + @Override + public boolean apply(final String input) { + return input.contains("revision="); + } + }; private final Set moduleBasedCaps; + private final Set capabilities; private NetconfSessionCapabilities(final Set capabilities, final Set moduleBasedCaps) { - this.capabilities = capabilities; - this.moduleBasedCaps = moduleBasedCaps; + this.capabilities = Preconditions.checkNotNull(capabilities); + this.moduleBasedCaps = Preconditions.checkNotNull(moduleBasedCaps); } public Set getModuleBasedCaps() { @@ -65,47 +99,45 @@ public final class NetconfSessionCapabilities { } public static NetconfSessionCapabilities fromStrings(final Collection capabilities) { - final Set moduleBasedCaps = Sets.newHashSet(); + final Set moduleBasedCaps = new HashSet<>(); for (final String capability : capabilities) { - if(isModuleBasedCapability(capability)) { - final String[] parts = capability.split("\\?"); - final String namespace = parts[0]; - final FluentIterable queryParams = FluentIterable.from(Arrays.asList(parts[1].split("&"))); - - String revision = getStringAndTransform(queryParams, "revision=", "revision="); - - final String moduleName = getStringAndTransform(queryParams, "module=", "module="); + final int qmark = capability.indexOf('?'); + if (qmark == -1) { + continue; + } - if (revision == null) { - logger.debug("Netconf device was not reporting revision correctly, trying to get amp;revision="); - revision = getStringAndTransform(queryParams, "amp;revision=", "amp;revision="); + 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 (moduleName == null) { + continue; + } - if (revision == null) { - logger.warn("Netconf device returned revision incorrectly escaped for {}", capability); - } - } + String revision = REVISION_PARAM.from(queryParams); + if (revision != null) { moduleBasedCaps.add(QName.create(namespace, revision, moduleName)); + continue; } - } - - return new NetconfSessionCapabilities(Sets.newHashSet(capabilities), moduleBasedCaps); - } - private static boolean isModuleBasedCapability(final String capability) { - return capability.contains("?") && capability.contains("module=") && capability.contains("revision="); - } + /* + * 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)) { + continue; + } - private static String getStringAndTransform(final Iterable queryParams, final String match, - final String substringToRemove) { - final Optional found = Iterables.tryFind(queryParams, new Predicate() { - @Override - public boolean apply(final String input) { - return input.startsWith(match); + LOG.debug("Netconf device was not reporting revision correctly, trying to get amp;revision="); + revision = BROKEN_REVISON_PARAM.from(queryParams); + if (revision == null) { + LOG.warn("Netconf device returned revision incorrectly escaped for {}, ignoring it", capability); } - }); - return found.isPresent() ? found.get().replaceAll(substringToRemove, "") : null; - } + // FIXME: do we really want to continue here? + moduleBasedCaps.add(QName.create(namespace, revision, moduleName)); + } + return new NetconfSessionCapabilities(ImmutableSet.copyOf(capabilities), ImmutableSet.copyOf(moduleBasedCaps)); + } } -- 2.36.6