From: Peter Puškár Date: Mon, 24 Jan 2022 11:30:39 +0000 (+0100) Subject: Fix incorrect level assignment during fields parsing X-Git-Tag: v4.0.1~3 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=netconf.git;a=commitdiff_plain;h=b09e4a54fe5f4d5d409996e00eee21818225ef4d Fix incorrect level assignment during fields parsing Everytime a sub-selector is encountered, new level is created. This behaviour is not in accordance to the one described in Javadoc. Side effects of this issue are missing data in RESTCONF, for NETCONF mount-points it will fail with NullPointerException because LinkedPathElement is expected on previous level but it cannot be found. Add proper level tracking and test-cases. Example: fields=security:objects(zones/zone);application-identification(user-defined-applications/user-defined-application/app-name) incorrect structure before: 0 = security:objects, application-identification 1 = zones 2 = zone 3 = user-defined-applications 4 = user-defined-application 5 = app-name correct structure after: 0 = security:objects, application-identification 1 = zones, user-defined-applications 2 = zone, user-defined-application 3 = app-name JIRA: NETCONF-660 Signed-off-by: Peter Puškár Change-Id: Icaf7c181b23ce9f7110cd6f63f17b55b4c1bc322 --- diff --git a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/utils/parser/AbstractFieldsTranslator.java b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/utils/parser/AbstractFieldsTranslator.java index 552fc3462d..fa79bdd945 100644 --- a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/utils/parser/AbstractFieldsTranslator.java +++ b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/utils/parser/AbstractFieldsTranslator.java @@ -54,7 +54,7 @@ public abstract class AbstractFieldsTranslator { final List> parsed = new ArrayList<>(); processSelectors(parsed, identifier.getSchemaContext(), identifier.getSchemaNode().getQName().getModule(), - startNode, input.nodeSelectors()); + startNode, input.nodeSelectors(), 0); return parsed; } @@ -70,16 +70,20 @@ public abstract class AbstractFieldsTranslator { @NonNull QName childQName, @NonNull Set level); private void processSelectors(final List> parsed, final EffectiveModelContext context, - final QNameModule startNamespace, final DataSchemaContextNode startNode, - final List selectors) { - final Set startLevel = new HashSet<>(); - parsed.add(startLevel); - + final QNameModule startNamespace, final DataSchemaContextNode startNode, + final List selectors, final int index) { + final Set startLevel; + if (parsed.size() <= index) { + startLevel = new HashSet<>(); + parsed.add(startLevel); + } else { + startLevel = parsed.get(index); + } for (var selector : selectors) { var node = startNode; var namespace = startNamespace; var level = startLevel; - + var levelIndex = index; // Note: path is guaranteed to have at least one step final var it = selector.path().iterator(); @@ -105,11 +109,12 @@ public abstract class AbstractFieldsTranslator { // go one level down level = prepareQNameLevel(parsed, level); + levelIndex++; } final var subs = selector.subSelectors(); if (!subs.isEmpty()) { - processSelectors(parsed, context, namespace, node, subs); + processSelectors(parsed, context, namespace, node, subs, levelIndex + 1); } } } 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 33fc50f7ae..9c40db25aa 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 @@ -212,6 +212,26 @@ public abstract class AbstractFieldsTranslatorTest { protected abstract void assertMultipleChildren3(@NonNull List result); + @Test + public void testMultipleChildren4() { + final var result = translateFields(identifierTestServices, + assertFields("services(type-of-service;instance(instance-name;provider);next-data(next-service))")); + assertNotNull(result); + assertMultipleChildren4(result); + } + + protected abstract void assertMultipleChildren4(@NonNull List result); + + @Test + public void testMultipleChildren5() { + final var result = translateFields(identifierTestServices, + assertFields("services(type-of-service;instance(instance-name;provider);next-data/next-service)")); + assertNotNull(result); + assertMultipleChildren5(result); + } + + protected abstract void assertMultipleChildren5(@NonNull List result); + @Test public void testAugmentedChild() { final var result = translateFields(identifierJukebox, assertFields("player/augmented-jukebox:speed")); 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 137ae44c8e..f20d07bb5c 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 @@ -98,6 +98,40 @@ public class NetconfFieldsTranslatorTest extends AbstractFieldsTranslatorTest result) { + assertEquals(4, result.size()); + + final var instanceNamePath = assertPath(result, INSTANCE_NAME_Q_NAME); + assertEquals(3, instanceNamePath.getPathArguments().size()); + + final var tosPath = assertPath(result, TYPE_OF_SERVICE_Q_NAME); + assertEquals(2, tosPath.getPathArguments().size()); + + final var nextServicePath = assertPath(result, NEXT_SERVICE_Q_NAME); + assertEquals(3, nextServicePath.getPathArguments().size()); + + final var providerPath = assertPath(result, PROVIDER_Q_NAME); + assertEquals(3, providerPath.getPathArguments().size()); + } + + @Override + protected void assertMultipleChildren5(final List result) { + assertEquals(4, result.size()); + + final var instanceNamePath = assertPath(result, INSTANCE_NAME_Q_NAME); + assertEquals(3, instanceNamePath.getPathArguments().size()); + + final var tosPath = assertPath(result, TYPE_OF_SERVICE_Q_NAME); + assertEquals(2, tosPath.getPathArguments().size()); + + final var nextServicePath = assertPath(result, NEXT_SERVICE_Q_NAME); + assertEquals(3, nextServicePath.getPathArguments().size()); + + final var providerPath = assertPath(result, PROVIDER_Q_NAME); + assertEquals(3, providerPath.getPathArguments().size()); + } + @Override protected void assertAugmentedChild(final List result) { assertEquals(1, result.size()); 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 d1f5a7ef2e..467383ea0f 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 @@ -88,6 +88,22 @@ public class WriterFieldsTranslatorTest extends AbstractFieldsTranslatorTest> result) { + assertEquals(result.size(), 3); + 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)); + } + + @Override + protected void assertMultipleChildren5(final List> result) { + assertEquals(result.size(), 3); + 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)); + } + @Override protected void assertAugmentedChild(final List> result) { // FIXME: add assertions