Explicit reading of list keys using subtree filtering

- According to RFC-6241, section 6.2.5, NETCONF server doesn't
  have to provide values of list keys, if NETCONF client doesn't
  ask for these fields; see following statement:

  o  If any sibling nodes of the selection node are instance
     identifier components for a conceptual data structure (e.g.,
     list key leaf), then they MAY also be included
     in the filter output.

JIRA: NETCONF-735
Change-Id: I9bb048010578fd89190f616664e2175c3d3326ae
Signed-off-by: Jaroslav Tóth <jtoth@frinx.io>
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/netconf/netconf-util/src/main/java/org/opendaylight/netconf/util/StreamingContext.java b/netconf/netconf-util/src/main/java/org/opendaylight/netconf/util/StreamingContext.java
index 5cf6a7e732cc3e124a8c574822ee4ff47c11cca3..82172f2c9b7e1d11a0acb368e2aa33055738bb4d 100644 (file)
--- a/netconf/netconf-util/src/main/java/org/opendaylight/netconf/util/StreamingContext.java
+++ b/netconf/netconf-util/src/main/java/org/opendaylight/netconf/util/StreamingContext.java
@@ -20,6 +20,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Optional;
+import java.util.stream.Collectors;
 import javax.xml.transform.dom.DOMSource;
 import org.opendaylight.yangtools.concepts.Identifiable;
 import org.opendaylight.yangtools.yang.common.QName;
@@ -169,19 +170,22 @@ abstract class StreamingContext<T extends PathArgument> implements Identifiable<
         }
 
         @Override
-        void streamToWriter(final NormalizedNodeStreamWriter writer, final PathArgument first,
-                            final PathNode subtree) throws IOException {
+        final void streamToWriter(final NormalizedNodeStreamWriter writer, final PathArgument first,
+                                  final PathNode subtree) throws IOException {
             verifyActualPathArgument(first);
 
             emitElementStart(writer, first);
             for (final PathNode node : subtree.children()) {
-                final PathArgument childPath = node.element();
-                final StreamingContext<?> childOp = getChildOperation(childPath);
-                childOp.streamToWriter(writer, childPath, node);
+                emitChildTreeNode(writer, node);
             }
             writer.endNode();
         }
 
+        void emitChildTreeNode(final NormalizedNodeStreamWriter writer, final PathNode node) throws IOException {
+            final PathArgument childPath = node.element();
+            getChildOperation(childPath).streamToWriter(writer, childPath, node);
+        }
+
         private void verifyActualPathArgument(final PathArgument first) {
             if (!isMixin()) {
                 final QName type = getIdentifier().getNodeType();
@@ -193,7 +197,7 @@ abstract class StreamingContext<T extends PathArgument> implements Identifiable<
         abstract void emitElementStart(NormalizedNodeStreamWriter writer, PathArgument arg) throws IOException;
 
         @SuppressWarnings("checkstyle:illegalCatch")
-        private StreamingContext<?> getChildOperation(final PathArgument childPath) {
+        StreamingContext<?> getChildOperation(final PathArgument childPath) {
             final StreamingContext<?> childOp;
             try {
                 childOp = getChild(childPath);
@@ -238,10 +242,12 @@ abstract class StreamingContext<T extends PathArgument> implements Identifiable<
 
     private abstract static class AbstractMapMixin extends AbstractComposite<NodeIdentifier> {
         private final ListEntry innerNode;
+        private final List<QName> keyLeaves;
 
         AbstractMapMixin(final ListSchemaNode list) {
             super(NodeIdentifier.create(list.getQName()));
             this.innerNode = new ListEntry(NodeIdentifierWithPredicates.of(list.getQName()), list);
+            this.keyLeaves = list.getKeyDefinition();
         }
 
         @Override
@@ -253,6 +259,24 @@ abstract class StreamingContext<T extends PathArgument> implements Identifiable<
         final boolean isMixin() {
             return true;
         }
+
+        @Override
+        final void emitChildTreeNode(final NormalizedNodeStreamWriter writer, final PathNode node) throws IOException {
+            final NodeIdentifierWithPredicates childPath = (NodeIdentifierWithPredicates) node.element();
+            final StreamingContext<?> childOp = getChildOperation(childPath);
+            if (childPath.size() == 0 && node.isEmpty() || childPath.keySet().containsAll(keyLeaves)) {
+                // This is a query for the entire list, or the query specifies everything we need
+                childOp.streamToWriter(writer, childPath, node);
+                return;
+            }
+
+            // Inexact query, we need to also request the leaf nodes we need to for reconstructing a valid instance
+            // NodeIdentifierWithPredicates.
+            childOp.streamToWriter(writer, childPath, node.copyWith(keyLeaves.stream()
+                .filter(qname -> !childPath.containsKey(qname))
+                .map(NodeIdentifier::new)
+                .collect(Collectors.toUnmodifiableList())));
+        }
     }
 
     private abstract static class AbstractSimple<T extends PathArgument> extends StreamingContext<T> {
@@ -271,7 +295,7 @@ abstract class StreamingContext<T extends PathArgument> implements Identifiable<
         }
 
         @Override
-        void streamToWriter(final NormalizedNodeStreamWriter writer, final PathArgument first,
+        final void streamToWriter(final NormalizedNodeStreamWriter writer, final PathArgument first,
                             final PathNode tree) throws IOException {
             streamToWriter(writer, first, Collections.emptyIterator());
         }

diff --git a/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/schema/mapping/NetconfMessageTransformerTest.java b/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/schema/mapping/NetconfMessageTransformerTest.java
index 2bfea943962aea83323355bc975b310509769439..e2f3878b467f036c8ae56ba0471ce3a3b46db769 100644 (file)
--- a/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/schema/mapping/NetconfMessageTransformerTest.java
+++ b/netconf/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/schema/mapping/NetconfMessageTransformerTest.java
@@ -69,6 +69,7 @@ import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.netconf.mon
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.netconf.monitoring.rev101004.netconf.state.Statistics;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.netconf.monitoring.rev101004.netconf.state.datastores.Datastore;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.netconf.monitoring.rev101004.netconf.state.datastores.datastore.Locks;
+import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.netconf.monitoring.rev101004.netconf.state.datastores.datastore.locks.lock.type.partial.lock.PartialLock;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.netconf.monitoring.rev101004.netconf.state.schemas.Schema;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.netconf.monitoring.rev101004.netconf.state.sessions.Session;
 import org.opendaylight.yangtools.rcf8528.data.util.EmptyMountPointContext;
@@ -908,7 +909,7 @@ public class NetconfMessageTransformerTest extends AbstractBaseSchemasTest {
         final YangInstanceIdentifier versionField = YangInstanceIdentifier.create(
                 toId(QName.create(Schema.QNAME, "version").intern()));
         final YangInstanceIdentifier identifierField = YangInstanceIdentifier.create(
-                toId(QName.create(Schema.QNAME, "identifier").intern()));
+                toId(QName.create(Schema.QNAME, "namespace").intern()));
 
         // building filter structure and NETCONF message
         final DataContainerChild<?, ?> filterStructure = toFilterStructure(Collections.singletonList(FieldsFilter.of(
@@ -924,7 +925,10 @@ public class NetconfMessageTransformerTest extends AbstractBaseSchemasTest {
                 + "<schemas>\n"
                 + "<schema>\n"
                 + "<version/>\n"
+                + "<namespace/>\n"
+                // explicitly fetched list keys - identifier and format
                 + "<identifier/>\n"
+                + "<format/>\n"
                 + "</schema>\n"
                 + "</schemas>\n"
                 + "</netconf-state>\n"
@@ -933,6 +937,49 @@ public class NetconfMessageTransformerTest extends AbstractBaseSchemasTest {
                 + "</rpc>");
     }
 
+    @Test
+    public void getSpecificFieldsUnderMultipleLists() throws IOException, SAXException {
+        // preparation of the fields
+        final YangInstanceIdentifier parentYiid = YangInstanceIdentifier.create(
+                toId(NetconfState.QNAME), toId(Datastores.QNAME));
+        final YangInstanceIdentifier partialLockYiid = YangInstanceIdentifier.create(toId(Datastore.QNAME),
+                NodeIdentifierWithPredicates.of(Datastore.QNAME), toId(Locks.QNAME),
+                toId(QName.create(Locks.QNAME, "lock-type").intern()), toId(PartialLock.QNAME),
+                NodeIdentifierWithPredicates.of(PartialLock.QNAME));
+        final YangInstanceIdentifier lockedTimeField = partialLockYiid.node(
+                QName.create(Locks.QNAME, "locked-time").intern());
+        final YangInstanceIdentifier lockedBySessionField = partialLockYiid.node(
+                QName.create(Locks.QNAME, "locked-by-session").intern());
+
+        // building filter structure and NETCONF message
+        final DataContainerChild<?, ?> filterStructure = toFilterStructure(Collections.singletonList(FieldsFilter.of(
+                parentYiid, List.of(lockedTimeField, lockedBySessionField))), SCHEMA);
+        final NetconfMessage netconfMessage = netconfMessageTransformer.toRpcRequest(NETCONF_GET_QNAME,
+                NetconfMessageTransformUtil.wrap(toId(NETCONF_GET_QNAME), filterStructure));
+
+        // testing
+        assertSimilarXml(netconfMessage, "<rpc message-id=\"m-0\" xmlns=\"urn:ietf:params:xml:ns:netconf:base:1.0\">\n"
+                + "<get>\n"
+                + "<filter xmlns:ns0=\"urn:ietf:params:xml:ns:netconf:base:1.0\" ns0:type=\"subtree\">\n"
+                + "<netconf-state xmlns=\"urn:ietf:params:xml:ns:yang:ietf-netconf-monitoring\">\n"
+                + "<datastores>\n"
+                + "<datastore>\n"
+                + "<locks>\n"
+                + "<partial-lock>\n"
+                + "<locked-time/>\n"
+                + "<locked-by-session/>\n"
+                + "<lock-id/>\n"
+                + "</partial-lock>\n"
+                + "</locks>\n"
+                + "<name/>\n"
+                + "</datastore>\n"
+                + "</datastores>\n"
+                + "</netconf-state>\n"
+                + "</filter>\n"
+                + "</get>\n"
+                + "</rpc>");
+    }
+
     @Test
     public void getWholeListsUsingFieldsTest() throws IOException, SAXException {
         // preparation of the fields