From: Peter Puškár Date: Thu, 20 Jan 2022 08:54:43 +0000 (+0100) Subject: Add missing identifier for list node in fields RESTCONF query X-Git-Tag: v4.0.3~3 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=refs%2Fchanges%2F00%2F99400%2F12;p=netconf.git Add missing identifier for list node in fields RESTCONF query Fields filtering is currently completely failing for NETCONF nodes when the fields query contains any list node, because we need both NodeIdentifier (for the entire list) and NodeIdentifierWithPredicates (for a concrete entry) to parse them correctly. WriterFieldsTranslator provides both already and this is correctly working on non-NETCONF nodes. Make this behaviour consistent for both. JIRA: NETCONF-820 Change-Id: I24d6ec1752ba1a2f551fcd4149c6efde04f1a89c Signed-off-by: Peter Puškár Signed-off-by: Robert Varga --- diff --git a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/utils/parser/NetconfFieldsTranslator.java b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/utils/parser/NetconfFieldsTranslator.java index ece8bc6365..1cbe5cd7ce 100644 --- a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/utils/parser/NetconfFieldsTranslator.java +++ b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/utils/parser/NetconfFieldsTranslator.java @@ -120,12 +120,18 @@ public final class NetconfFieldsTranslator extends AbstractFieldsTranslator collectedMixinNodes = new ArrayList<>(); DataSchemaContextNode actualContextNode = currentNode.getChild(childQName); + if (actualContextNode == null) { + actualContextNode = resolveMixinNode(currentNode, currentNode.getIdentifier().getNodeType()); + actualContextNode = actualContextNode.getChild(childQName); + } + while (actualContextNode != null && actualContextNode.isMixin()) { - if (actualContextNode.getDataSchemaNode() instanceof ListSchemaNode) { - // we need just a single node identifier from list in the path (key is not available) + final var actualDataSchemaNode = actualContextNode.getDataSchemaNode(); + if (actualDataSchemaNode instanceof ListSchemaNode listSchema && listSchema.getKeyDefinition().isEmpty()) { + // we need just a single node identifier from list in the path IFF it is an unkeyed list, otherwise + // we need both (which is the default case) actualContextNode = actualContextNode.getChild(childQName); - break; - } else if (actualContextNode.getDataSchemaNode() instanceof LeafListSchemaNode) { + } else if (actualDataSchemaNode instanceof LeafListSchemaNode) { // NodeWithValue is unusable - stop parsing break; } else { @@ -145,6 +151,15 @@ public final class NetconfFieldsTranslator extends AbstractFieldsTranslator resolveMixinNode( + final DataSchemaContextNode node, final @NonNull QName qualifiedName) { + DataSchemaContextNode currentNode = node; + while (currentNode != null && currentNode.isMixin()) { + currentNode = currentNode.getChild(qualifiedName); + } + return currentNode; + } + /** * {@link PathArgument} of data element grouped with identifiers of leading mixin nodes and previous node.
* - identifiers of mixin nodes on the path to the target node - required for construction of full valid diff --git a/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/utils/parser/AbstractFieldsTranslatorTest.java b/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/utils/parser/AbstractFieldsTranslatorTest.java index 9c40db25aa..57a287786d 100644 --- a/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/utils/parser/AbstractFieldsTranslatorTest.java +++ b/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/utils/parser/AbstractFieldsTranslatorTest.java @@ -257,6 +257,15 @@ public abstract class AbstractFieldsTranslatorTest { assertLeafList(result); } + @Test + public void testKeyedList() { + final var result = translateFields(identifierJukebox, assertFields("library/artist(name)")); + assertNotNull(result); + assertKeyedList(result); + } + + protected abstract void assertKeyedList(List result); + protected abstract void assertLeafList(@NonNull List result); /** diff --git a/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/utils/parser/NetconfFieldsTranslatorTest.java b/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/utils/parser/NetconfFieldsTranslatorTest.java index f20d07bb5c..bae3814ccd 100644 --- a/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/utils/parser/NetconfFieldsTranslatorTest.java +++ b/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/utils/parser/NetconfFieldsTranslatorTest.java @@ -42,6 +42,11 @@ public class NetconfFieldsTranslatorTest extends AbstractFieldsTranslatorTest result) { + assertEquals(1, result.size()); + } + @Override protected void assertDoublePath(final List result) { assertEquals(2, result.size()); @@ -55,23 +60,35 @@ public class NetconfFieldsTranslatorTest extends AbstractFieldsTranslatorTest result) { - // FIXME: NETCONF-820: add assertions + assertEquals(1, result.size()); + final var pathArguments = result.get(0).getPathArguments(); + assertEquals(6, pathArguments.size()); + assertEquals(LIBRARY_Q_NAME, pathArguments.get(0).getNodeType()); + assertEquals(ARTIST_Q_NAME, pathArguments.get(1).getNodeType()); + assertEquals(ARTIST_Q_NAME, pathArguments.get(2).getNodeType()); + assertEquals(ALBUM_Q_NAME, pathArguments.get(3).getNodeType()); + assertEquals(ALBUM_Q_NAME, pathArguments.get(4).getNodeType()); + assertEquals(NAME_Q_NAME, pathArguments.get(5).getNodeType()); } @Override protected void assertChildrenPath(final List result) { assertEquals(1, result.size()); final var pathArguments = result.get(0).getPathArguments(); - assertEquals(4, pathArguments.size()); + assertEquals(6, pathArguments.size()); assertEquals(LIBRARY_Q_NAME, pathArguments.get(0).getNodeType()); assertEquals(ARTIST_Q_NAME, pathArguments.get(1).getNodeType()); - assertEquals(ALBUM_Q_NAME, pathArguments.get(2).getNodeType()); - assertEquals(NAME_Q_NAME, pathArguments.get(3).getNodeType()); + assertEquals(ARTIST_Q_NAME, pathArguments.get(2).getNodeType()); + assertEquals(ALBUM_Q_NAME, pathArguments.get(3).getNodeType()); + assertEquals(ALBUM_Q_NAME, pathArguments.get(4).getNodeType()); + assertEquals(NAME_Q_NAME, pathArguments.get(5).getNodeType()); } @Override protected void assertNamespace(final List result) { - // FIXME: add assertions + assertEquals(1, result.size()); + final var augmentedLibraryPath = assertPath(result, AUGMENTED_LIBRARY_Q_NAME); + assertEquals(2, augmentedLibraryPath.getPathArguments().size()); } @Override @@ -79,23 +96,41 @@ public class NetconfFieldsTranslatorTest extends AbstractFieldsTranslatorTest result) { - // FIXME: add assertions + assertEquals(3, result.size()); + + final var tosPath = assertPath(result, TYPE_OF_SERVICE_Q_NAME); + assertEquals(3, tosPath.getPathArguments().size()); + + final var instanceNamePath = assertPath(result, INSTANCE_NAME_Q_NAME); + assertEquals(5, instanceNamePath.getPathArguments().size()); + + final var providerPath = assertPath(result, PROVIDER_Q_NAME); + assertEquals(5, providerPath.getPathArguments().size()); } @Override protected void assertMultipleChildren3(final List result) { - // FIXME: add assertions + assertEquals(3, result.size()); + + final var instanceNamePath = assertPath(result, INSTANCE_NAME_Q_NAME); + assertEquals(5, instanceNamePath.getPathArguments().size()); + + final var tosPath = assertPath(result, TYPE_OF_SERVICE_Q_NAME); + assertEquals(3, tosPath.getPathArguments().size()); + + final var nextServicePath = assertPath(result, NEXT_SERVICE_Q_NAME); + assertEquals(4, nextServicePath.getPathArguments().size()); } @Override @@ -103,16 +138,16 @@ public class NetconfFieldsTranslatorTest extends AbstractFieldsTranslatorTest result) { assertEquals(1, result.size()); assertEquals(List.of( - // FIXME: this does not look right: where are the corresponding NodeIdentifiers? + NodeIdentifier.create(SERVICES_Q_NAME), NodeIdentifierWithPredicates.of(SERVICES_Q_NAME), + NodeIdentifier.create(INSTANCE_Q_NAME), NodeIdentifierWithPredicates.of(INSTANCE_Q_NAME)), result.get(0).getPathArguments()); } diff --git a/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/utils/parser/WriterFieldsTranslatorTest.java b/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/utils/parser/WriterFieldsTranslatorTest.java index 467383ea0f..41dac3c34b 100644 --- a/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/utils/parser/WriterFieldsTranslatorTest.java +++ b/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/utils/parser/WriterFieldsTranslatorTest.java @@ -66,7 +66,7 @@ public class WriterFieldsTranslatorTest extends AbstractFieldsTranslatorTest> result) { - assertEquals(result.size(), 3); + assertEquals(3, result.size()); assertEquals(Set.of(SERVICES_Q_NAME), result.get(0)); assertEquals(Set.of(TYPE_OF_SERVICE_Q_NAME, INSTANCE_Q_NAME), result.get(1)); assertEquals(Set.of(INSTANCE_NAME_Q_NAME, PROVIDER_Q_NAME), result.get(2)); @@ -74,7 +74,7 @@ public class WriterFieldsTranslatorTest extends AbstractFieldsTranslatorTest> result) { - assertEquals(result.size(), 3); + assertEquals(3, result.size()); assertEquals(Set.of(SERVICES_Q_NAME), result.get(0)); assertEquals(Set.of(TYPE_OF_SERVICE_Q_NAME, INSTANCE_Q_NAME), result.get(1)); assertEquals(Set.of(INSTANCE_NAME_Q_NAME, PROVIDER_Q_NAME), result.get(2)); @@ -82,7 +82,7 @@ public class WriterFieldsTranslatorTest extends AbstractFieldsTranslatorTest> result) { - assertEquals(result.size(), 3); + assertEquals(3, result.size()); assertEquals(Set.of(SERVICES_Q_NAME), result.get(0)); assertEquals(Set.of(TYPE_OF_SERVICE_Q_NAME, INSTANCE_Q_NAME, NEXT_DATA_Q_NAME), result.get(1)); assertEquals(Set.of(INSTANCE_NAME_Q_NAME, NEXT_SERVICE_Q_NAME), result.get(2)); @@ -90,7 +90,7 @@ public class WriterFieldsTranslatorTest extends AbstractFieldsTranslatorTest> result) { - assertEquals(result.size(), 3); + assertEquals(3, result.size()); assertEquals(Set.of(SERVICES_Q_NAME), result.get(0)); assertEquals(Set.of(TYPE_OF_SERVICE_Q_NAME, INSTANCE_Q_NAME, NEXT_DATA_Q_NAME), result.get(1)); assertEquals(Set.of(INSTANCE_NAME_Q_NAME, PROVIDER_Q_NAME, NEXT_SERVICE_Q_NAME), result.get(2)); @@ -98,7 +98,7 @@ public class WriterFieldsTranslatorTest extends AbstractFieldsTranslatorTest> result) { - assertEquals(result.size(), 3); + assertEquals(3, result.size()); assertEquals(Set.of(SERVICES_Q_NAME), result.get(0)); assertEquals(Set.of(TYPE_OF_SERVICE_Q_NAME, INSTANCE_Q_NAME, NEXT_DATA_Q_NAME), result.get(1)); assertEquals(Set.of(INSTANCE_NAME_Q_NAME, PROVIDER_Q_NAME, NEXT_SERVICE_Q_NAME), result.get(2)); @@ -106,16 +106,29 @@ public class WriterFieldsTranslatorTest extends AbstractFieldsTranslatorTest> result) { - // FIXME: add assertions + assertEquals(2, result.size()); + assertEquals(Set.of(PLAYER_Q_NAME), result.get(0)); + assertEquals(Set.of(SPEED_Q_NAME), result.get(1)); } @Override protected void assertListFieldUnderList(final List> result) { - // FIXME: add assertions + assertEquals(2, result.size()); + assertEquals(Set.of(SERVICES_Q_NAME), result.get(0)); + assertEquals(Set.of(INSTANCE_Q_NAME), result.get(1)); + } + + @Override + protected void assertKeyedList(final List> result) { + assertEquals(3, result.size()); + assertEquals(Set.of(LIBRARY_Q_NAME), result.get(0)); + assertEquals(Set.of(ARTIST_Q_NAME), result.get(1)); + assertEquals(Set.of(NAME_Q_NAME), result.get(2)); } @Override protected void assertLeafList(final List> result) { - // FIXME: add assertions + assertEquals(1, result.size()); + assertEquals(Set.of(PROTOCOLS_Q_NAME), result.get(0)); } }