Do not use SchemaNode.getPath() in mdsal-netconf-connector

We are using SchemaPath identification of what are strictly root
nodes. Use SchemaTreeInference instead, so that the lookup is captured
once and then reused, without multiple round-trips to acquire
EffectiveModelContext.

This also flushes out a potential inconsistency w.r.t. handling of
top-level choice containers. This is marked with a FIXME for a future
follow-up.

JIRA: NETCONF-816
Change-Id: I9061c3cf8b871d2947ecae41d6c57d51545990d5
Signed-off-by: Ivan Hrasko <ivan.hrasko@pantheon.tech>
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/netconf/mdsal-netconf-connector/src/main/java/org/opendaylight/netconf/mdsal/connector/CurrentSchemaContext.java b/netconf/mdsal-netconf-connector/src/main/java/org/opendaylight/netconf/mdsal/connector/CurrentSchemaContext.java
index 33109371344fe5ea729a2691824610bdedf29440..5409f1cda257d765c3647e4227ae17713627c34b 100644 (file)
--- a/netconf/mdsal-netconf-connector/src/main/java/org/opendaylight/netconf/mdsal/connector/CurrentSchemaContext.java
+++ b/netconf/mdsal-netconf-connector/src/main/java/org/opendaylight/netconf/mdsal/connector/CurrentSchemaContext.java
@@ -13,6 +13,7 @@ import java.util.Collections;
 import java.util.HashSet;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicReference;
+import org.eclipse.jdt.annotation.NonNull;
 import org.opendaylight.mdsal.dom.api.DOMSchemaService;
 import org.opendaylight.netconf.api.capability.Capability;
 import org.opendaylight.netconf.api.monitoring.CapabilityListener;
@@ -22,23 +23,25 @@ import org.opendaylight.yangtools.yang.model.api.EffectiveModelContextListener;
 import org.opendaylight.yangtools.yang.model.repo.api.YangTextSchemaSource;
 import org.opendaylight.yangtools.yang.model.repo.spi.SchemaSourceProvider;
 
+// Non-final for mocking
 public class CurrentSchemaContext implements EffectiveModelContextListener, AutoCloseable {
     private final AtomicReference<EffectiveModelContext> currentContext = new AtomicReference<>();
     private final ListenerRegistration<?> schemaContextListenerListenerRegistration;
     private final Set<CapabilityListener> listeners1 = Collections.synchronizedSet(new HashSet<>());
     private final SchemaSourceProvider<YangTextSchemaSource> rootSchemaSourceProvider;
 
-    public EffectiveModelContext getCurrentContext() {
-        checkState(currentContext.get() != null, "Current context not received");
-        return currentContext.get();
-    }
-
     public CurrentSchemaContext(final DOMSchemaService schemaService,
                                 final SchemaSourceProvider<YangTextSchemaSource> rootSchemaSourceProvider) {
         this.rootSchemaSourceProvider = rootSchemaSourceProvider;
         schemaContextListenerListenerRegistration = schemaService.registerSchemaContextListener(this);
     }
 
+    public @NonNull EffectiveModelContext getCurrentContext() {
+        final var ret = currentContext.get();
+        checkState(ret != null, "Current context not received");
+        return ret;
+    }
+
     @Override
     public void onModelContextUpdated(final EffectiveModelContext schemaContext) {
         currentContext.set(schemaContext);

diff --git a/netconf/mdsal-netconf-connector/src/main/java/org/opendaylight/netconf/mdsal/connector/ops/AbstractEdit.java b/netconf/mdsal-netconf-connector/src/main/java/org/opendaylight/netconf/mdsal/connector/ops/AbstractEdit.java
index 5611ef88bc8badee4730abae67ce09ac8a1597d0..fcac9217a544009d66a39083f3483ca2bb08241b 100644 (file)
--- a/netconf/mdsal-netconf-connector/src/main/java/org/opendaylight/netconf/mdsal/connector/ops/AbstractEdit.java
+++ b/netconf/mdsal-netconf-connector/src/main/java/org/opendaylight/netconf/mdsal/connector/ops/AbstractEdit.java
@@ -11,9 +11,9 @@ import com.google.common.collect.ImmutableMap;
 import java.io.IOException;
 import java.net.URISyntaxException;
 import java.util.Iterator;
-import java.util.Optional;
 import javax.xml.stream.XMLStreamException;
 import javax.xml.transform.dom.DOMSource;
+import org.eclipse.jdt.annotation.NonNull;
 import org.opendaylight.netconf.api.DocumentedException;
 import org.opendaylight.netconf.api.NetconfDocumentedException;
 import org.opendaylight.netconf.api.xml.XmlElement;
@@ -26,9 +26,10 @@ import org.opendaylight.yangtools.yang.common.XMLNamespace;
 import org.opendaylight.yangtools.yang.data.api.schema.stream.NormalizedNodeStreamWriter;
 import org.opendaylight.yangtools.yang.data.codec.xml.XmlParserStream;
 import org.opendaylight.yangtools.yang.model.api.ContainerSchemaNode;
-import org.opendaylight.yangtools.yang.model.api.DataSchemaNode;
+import org.opendaylight.yangtools.yang.model.api.EffectiveModelContext;
 import org.opendaylight.yangtools.yang.model.api.ListSchemaNode;
 import org.opendaylight.yangtools.yang.model.api.Module;
+import org.opendaylight.yangtools.yang.model.api.SchemaTreeInference;
 import org.opendaylight.yangtools.yang.model.util.SchemaInferenceStack;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -40,15 +41,17 @@ abstract class AbstractEdit extends AbstractConfigOperation {
     private static final Logger LOG = LoggerFactory.getLogger(AbstractEdit.class);
     private static final String TARGET_KEY = "target";
 
-    protected final CurrentSchemaContext schemaContext;
+    final CurrentSchemaContext schemaContext;
 
-    protected AbstractEdit(final String netconfSessionIdForReporting, final CurrentSchemaContext schemaContext) {
+    AbstractEdit(final String netconfSessionIdForReporting, final CurrentSchemaContext schemaContext) {
         super(netconfSessionIdForReporting);
         this.schemaContext = schemaContext;
     }
 
-    protected void parseIntoNormalizedNode(final DataSchemaNode schemaNode, final XmlElement element,
-                                           final NormalizedNodeStreamWriter writer) throws DocumentedException {
+    static final void parseIntoNormalizedNode(final SchemaTreeInference inference, final XmlElement element,
+                                              final NormalizedNodeStreamWriter writer) throws DocumentedException {
+        final var path = inference.statementPath();
+        final var schemaNode = path.get(path.size() - 1);
         if (!(schemaNode instanceof ContainerSchemaNode) && !(schemaNode instanceof ListSchemaNode)) {
             // This should never happen since any edit operation on any other node type
             // should not be possible nor makes sense
@@ -56,8 +59,7 @@ abstract class AbstractEdit extends AbstractConfigOperation {
             throw new UnsupportedOperationException("implement exception if parse fails");
         }
 
-        final XmlParserStream xmlParser = XmlParserStream.create(writer, SchemaInferenceStack.ofInstantiatedPath(
-            schemaContext.getCurrentContext(), schemaNode.getPath()).toInference());
+        final XmlParserStream xmlParser = XmlParserStream.create(writer, inference);
         try {
             xmlParser.traverse(new DOMSource(element.getDomElement()));
         } catch (final XMLStreamException | URISyntaxException | IOException | SAXException ex) {
@@ -66,18 +68,20 @@ abstract class AbstractEdit extends AbstractConfigOperation {
         }
     }
 
-    protected DataSchemaNode getSchemaNodeFromNamespace(final String namespace, final XmlElement element)
-        throws DocumentedException {
-        final Iterator<? extends Module> it;
+    final SchemaTreeInference getSchemaNodeFromNamespace(final String namespace, final XmlElement element)
+            throws DocumentedException {
+        final XMLNamespace ns;
         try {
-            // Returns module with newest revision since findModuleByNamespace returns a set of modules and we only
-            // need the newest one
-            it = schemaContext.getCurrentContext().findModules(XMLNamespace.of(namespace)).iterator();
+            ns = XMLNamespace.of(namespace);
         } catch (final IllegalArgumentException e) {
             throw new NetconfDocumentedException("Unable to create URI for namespace : " + namespace, e,
                 ErrorType.APPLICATION, ErrorTag.INVALID_VALUE, ErrorSeverity.ERROR);
         }
 
+        // Returns module with newest revision since findModuleByNamespace returns a set of modules and we only
+        // need the newest one
+        final EffectiveModelContext ctx = schemaContext.getCurrentContext();
+        final Iterator<? extends @NonNull Module> it = ctx.findModules(ns).iterator();
         if (!it.hasNext()) {
             // No module is present with this namespace
             throw new NetconfDocumentedException("Unable to find module by namespace: " + namespace,
@@ -85,19 +89,33 @@ abstract class AbstractEdit extends AbstractConfigOperation {
         }
 
         final Module module = it.next();
+        final SchemaInferenceStack stack = SchemaInferenceStack.of(ctx);
         final String elementName = element.getName();
-        final Optional<DataSchemaNode> schemaNode = module.findDataChildByName(QName.create(module.getQNameModule(),
-                    element.getName()));
-        if (schemaNode.isEmpty()) {
+        try {
+            // FIXME: This is a bit suspect. The element is formed using XML encoding, hence it corresponds to
+            //        enterDataTree(). But then we use the result of this method to create a NormalizedNode tree,
+            //        which contains ChoiceNode. This needs to be tested with something like to following:
+            //
+            //        module mod {
+            //          choice foo {
+            //            case bar {
+            //              leaf baz {
+            //                type string;
+            //              }
+            //            }
+            //          }
+            //        }
+            stack.enterSchemaTree(QName.create(module.getQNameModule(), elementName));
+        } catch (IllegalArgumentException e) {
             throw new DocumentedException(
-                "Unable to find node " + elementName + " with namespace: " + namespace + " in module: " + module,
+                "Unable to find node " + elementName + " with namespace: " + namespace + " in module: " + module, e,
                 ErrorType.APPLICATION, ErrorTag.UNKNOWN_NAMESPACE, ErrorSeverity.ERROR);
         }
 
-        return schemaNode.get();
+        return stack.toSchemaTreeInference();
     }
 
-    protected static XmlElement extractTargetElement(final XmlElement operationElement, final String operationName)
+    static final XmlElement extractTargetElement(final XmlElement operationElement, final String operationName)
         throws DocumentedException {
         final NodeList elementsByTagName = getElementsByTagName(operationElement, TARGET_KEY);
         // Direct lookup instead of using XmlElement class due to performance

diff --git a/netconf/mdsal-netconf-connector/src/main/java/org/opendaylight/netconf/mdsal/connector/ops/CopyConfig.java b/netconf/mdsal-netconf-connector/src/main/java/org/opendaylight/netconf/mdsal/connector/ops/CopyConfig.java
index 9ba2a1499b2a1f4f51aedb7f80cb859f30c333c4..f4b0ec29350ad6136d1416265038666a6deec3fe 100644 (file)
--- a/netconf/mdsal-netconf-connector/src/main/java/org/opendaylight/netconf/mdsal/connector/ops/CopyConfig.java
+++ b/netconf/mdsal-netconf-connector/src/main/java/org/opendaylight/netconf/mdsal/connector/ops/CopyConfig.java
@@ -44,7 +44,6 @@ import org.opendaylight.yangtools.yang.data.codec.xml.XMLStreamNormalizedNodeStr
 import org.opendaylight.yangtools.yang.data.impl.schema.Builders;
 import org.opendaylight.yangtools.yang.data.impl.schema.ImmutableNormalizedNodeStreamWriter;
 import org.opendaylight.yangtools.yang.data.impl.schema.NormalizedNodeResult;
-import org.opendaylight.yangtools.yang.model.api.DataSchemaNode;
 import org.opendaylight.yangtools.yang.model.api.SchemaContext;
 import org.opendaylight.yangtools.yang.model.api.SchemaPath;
 import org.w3c.dom.Document;
@@ -104,10 +103,9 @@ public final class CopyConfig extends AbstractEdit {
 
         // Then create nodes present in the <config> element:
         for (final XmlElement element : configElements) {
-            final String ns = element.getNamespace();
-            final DataSchemaNode schemaNode = getSchemaNodeFromNamespace(ns, element);
             final NormalizedNodeResult resultHolder = new NormalizedNodeResult();
-            parseIntoNormalizedNode(schemaNode, element, ImmutableNormalizedNodeStreamWriter.from(resultHolder));
+            parseIntoNormalizedNode(getSchemaNodeFromNamespace(element.getNamespace(), element), element,
+                ImmutableNormalizedNodeStreamWriter.from(resultHolder));
             final NormalizedNode data = resultHolder.getResult();
             final YangInstanceIdentifier path = YangInstanceIdentifier.create(data.getIdentifier());
             // Doing merge instead of put to support top-level list:

diff --git a/netconf/mdsal-netconf-connector/src/main/java/org/opendaylight/netconf/mdsal/connector/ops/EditConfig.java b/netconf/mdsal-netconf-connector/src/main/java/org/opendaylight/netconf/mdsal/connector/ops/EditConfig.java
index 66020c8dad9253a0f40f46b11c36f0197d38856d..1156c99ff223182687d28aba2c9efdc41de92bf8 100644 (file)
--- a/netconf/mdsal-netconf-connector/src/main/java/org/opendaylight/netconf/mdsal/connector/ops/EditConfig.java
+++ b/netconf/mdsal-netconf-connector/src/main/java/org/opendaylight/netconf/mdsal/connector/ops/EditConfig.java
@@ -67,12 +67,9 @@ public final class EditConfig extends AbstractEdit {
         final XmlElement configElement = getConfigElement(operationElement);
 
         for (final XmlElement element : configElement.getChildElements()) {
-            final String ns = element.getNamespace();
-            final DataSchemaNode schemaNode = getSchemaNodeFromNamespace(ns, element);
-
             final SplittingNormalizedNodeMetadataStreamWriter writer = new SplittingNormalizedNodeMetadataStreamWriter(
                 defaultAction);
-            parseIntoNormalizedNode(schemaNode, element, writer);
+            parseIntoNormalizedNode(getSchemaNodeFromNamespace(element.getNamespace(), element), element, writer);
             executeOperations(writer.getDataTreeChanges());
         }
 

diff --git a/netconf/mdsal-netconf-connector/src/main/java/org/opendaylight/netconf/mdsal/connector/ops/RuntimeRpc.java b/netconf/mdsal-netconf-connector/src/main/java/org/opendaylight/netconf/mdsal/connector/ops/RuntimeRpc.java
index ef0bcfadf414e1c845940bc068a37f0e5e9189f9..556636cd6cdaa07f5934fb99a3865753979659ae 100644 (file)
--- a/netconf/mdsal-netconf-connector/src/main/java/org/opendaylight/netconf/mdsal/connector/ops/RuntimeRpc.java
+++ b/netconf/mdsal-netconf-connector/src/main/java/org/opendaylight/netconf/mdsal/connector/ops/RuntimeRpc.java
@@ -43,7 +43,6 @@ import org.opendaylight.yangtools.yang.data.impl.schema.NormalizedNodeResult;
 import org.opendaylight.yangtools.yang.data.impl.schema.SchemaOrderedNormalizedNodeWriter;
 import org.opendaylight.yangtools.yang.model.api.Module;
 import org.opendaylight.yangtools.yang.model.api.RpcDefinition;
-import org.opendaylight.yangtools.yang.model.api.SchemaPath;
 import org.opendaylight.yangtools.yang.model.api.stmt.SchemaNodeIdentifier.Absolute;
 import org.opendaylight.yangtools.yang.model.util.SchemaInferenceStack;
 import org.slf4j.Logger;
@@ -161,7 +160,8 @@ public class RuntimeRpc extends AbstractSingletonNetconfOperation {
             return XmlUtil.createElement(document, XmlNetconfConstants.OK,
                 Optional.of(XmlNetconfConstants.URN_IETF_PARAMS_XML_NS_NETCONF_BASE_1_0));
         }
-        return transformNormalizedNode(document, result.getResult(), rpcDefinition.getOutput().getPath());
+        return transformNormalizedNode(document, result.getResult(),
+                Absolute.of(rpcDefinition.getQName(), rpcDefinition.getOutput().getQName()));
     }
 
     @Override
@@ -200,7 +200,7 @@ public class RuntimeRpc extends AbstractSingletonNetconfOperation {
     }
 
     private Element transformNormalizedNode(final Document document, final NormalizedNode data,
-                                            final SchemaPath rpcOutputPath) {
+                                            final Absolute rpcOutputPath) {
         final DOMResult result = new DOMResult(document.createElement(XmlNetconfConstants.RPC_REPLY_KEY));
 
         final XMLStreamWriter xmlWriter = getXmlStreamWriter(result);
@@ -208,8 +208,8 @@ public class RuntimeRpc extends AbstractSingletonNetconfOperation {
         final NormalizedNodeStreamWriter nnStreamWriter = XMLStreamNormalizedNodeStreamWriter.create(xmlWriter,
                 schemaContext.getCurrentContext(), rpcOutputPath);
 
-        final SchemaOrderedNormalizedNodeWriter nnWriter =
-                new SchemaOrderedNormalizedNodeWriter(nnStreamWriter, schemaContext.getCurrentContext(), rpcOutputPath);
+        final SchemaOrderedNormalizedNodeWriter nnWriter = new SchemaOrderedNormalizedNodeWriter(nnStreamWriter,
+                schemaContext.getCurrentContext(), rpcOutputPath.asSchemaPath());
 
         writeRootElement(xmlWriter, nnWriter, (ContainerNode) data);
         try {

diff --git a/netconf/mdsal-netconf-connector/src/main/java/org/opendaylight/netconf/mdsal/connector/ops/get/FilterContentValidator.java b/netconf/mdsal-netconf-connector/src/main/java/org/opendaylight/netconf/mdsal/connector/ops/get/FilterContentValidator.java
index 27c0c71064a322141ce201633f548df572c61673..03f1ace3d700f3ab30f1a31c4e3baec30c37cf0a 100644 (file)
--- a/netconf/mdsal-netconf-connector/src/main/java/org/opendaylight/netconf/mdsal/connector/ops/get/FilterContentValidator.java
+++ b/netconf/mdsal-netconf-connector/src/main/java/org/opendaylight/netconf/mdsal/connector/ops/get/FilterContentValidator.java
@@ -41,6 +41,7 @@ import org.opendaylight.yangtools.yang.model.api.LeafSchemaNode;
 import org.opendaylight.yangtools.yang.model.api.ListSchemaNode;
 import org.opendaylight.yangtools.yang.model.api.Module;
 import org.opendaylight.yangtools.yang.model.api.TypeDefinition;
+import org.opendaylight.yangtools.yang.model.api.stmt.SchemaNodeIdentifier.Absolute;
 import org.opendaylight.yangtools.yang.model.api.type.IdentityrefTypeDefinition;
 import org.opendaylight.yangtools.yang.model.api.type.LeafrefTypeDefinition;
 import org.opendaylight.yangtools.yang.model.util.SchemaInferenceStack;
@@ -167,7 +168,7 @@ public class FilterContentValidator {
             builder.node(child.getName());
             path.add(child.getName().getLocalName());
             if (child.getType() == Type.LIST) {
-                appendKeyIfPresent(child, filterContent, path, builder);
+                appendKeyIfPresent(current, child, filterContent, path, builder);
                 return builder.build();
             }
             current = child;
@@ -175,20 +176,20 @@ public class FilterContentValidator {
         return builder.build();
     }
 
-    private void appendKeyIfPresent(final FilterTree tree, final XmlElement filterContent,
-                                    final List<String> pathToList,
-                                    final InstanceIdentifierBuilder builder) {
-        Preconditions.checkArgument(tree.getSchemaNode() instanceof ListSchemaNode);
-        final ListSchemaNode listSchemaNode = (ListSchemaNode) tree.getSchemaNode();
+    private void appendKeyIfPresent(final FilterTree parent, final FilterTree list, final XmlElement filterContent,
+            final List<String> pathToList, final InstanceIdentifierBuilder builder) {
+        Preconditions.checkArgument(list.getSchemaNode() instanceof ListSchemaNode);
+        final ListSchemaNode listSchemaNode = (ListSchemaNode) list.getSchemaNode();
+        final DataSchemaNode parentSchemaNode = parent.getSchemaNode();
 
-        final Map<QName, Object> map = getKeyValues(pathToList, filterContent, listSchemaNode);
+        final Map<QName, Object> map = getKeyValues(pathToList, filterContent, parentSchemaNode, listSchemaNode);
         if (!map.isEmpty()) {
-            builder.nodeWithKey(tree.getName(), map);
+            builder.nodeWithKey(list.getName(), map);
         }
     }
 
     private Map<QName, Object> getKeyValues(final List<String> path, final XmlElement filterContent,
-                                            final ListSchemaNode listSchemaNode) {
+            final DataSchemaNode parentSchemaNode, final ListSchemaNode listSchemaNode) {
         XmlElement current = filterContent;
         //find list element
         for (final String pathElement : path) {
@@ -219,8 +220,10 @@ public class FilterContentValidator {
                         final NamespaceContext nsContext = new UniversalNamespaceContextImpl(document, false);
                         final EffectiveModelContext modelContext = schemaContext.getCurrentContext();
                         final XmlCodecFactory xmlCodecFactory = XmlCodecFactory.create(modelContext);
-                        final TypeAwareCodec<?, NamespaceContext, XMLStreamWriter> typeCodec = xmlCodecFactory.codecFor(
-                            listKey, SchemaInferenceStack.ofInstantiatedPath(modelContext, listKey.getPath()));
+                        final SchemaInferenceStack resolver = SchemaInferenceStack.of(modelContext, Absolute.of(
+                                parentSchemaNode.getQName(), listSchemaNode.getQName(), listKey.getQName()));
+                        final TypeAwareCodec<?, NamespaceContext, XMLStreamWriter> typeCodec = xmlCodecFactory
+                                .codecFor(listKey, resolver);
                         final Object deserializedKeyValue = typeCodec.parseValue(nsContext, keyValue.get());
                         keys.put(qualifiedName, deserializedKeyValue);
                     } else {