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 <ppuskar@frinx.io>
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 552fc3462d3c9ca8b924b29e21bbd762871d8fa6..fa79bdd94530af2617ba8e9ed092425b0d343440 100644 (file)
--- 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<T> {
 
         final List<Set<T>> 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<T> {
             @NonNull QName childQName, @NonNull Set<T> level);
 
     private void processSelectors(final List<Set<T>> parsed, final EffectiveModelContext context,
-            final QNameModule startNamespace, final DataSchemaContextNode<?> startNode,
-            final List<NodeSelector> selectors) {
-        final Set<T> startLevel = new HashSet<>();
-        parsed.add(startLevel);
-
+                                  final QNameModule startNamespace, final DataSchemaContextNode<?> startNode,
+                                  final List<NodeSelector> selectors, final int index) {
+        final Set<T> 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<T> {
 
                 // 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 33fc50f7aec24d1e471cba14c5c22fb4ea83bec6..9c40db25aa18a73bfa706d578e25fa47ffa9d523 100644 (file)
--- 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<T> {
 
     protected abstract void assertMultipleChildren3(@NonNull List<T> 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<T> 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<T> 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 137ae44c8ea78136353e7dbac5fc1d75fb0e9a66..f20d07bb5caa516c8636e1271e11fedbf221e33f 100644 (file)
--- 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<Ya
         // FIXME: add assertions
     }
 
+    @Override
+    protected void assertMultipleChildren4(final List<YangInstanceIdentifier> 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<YangInstanceIdentifier> 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<YangInstanceIdentifier> 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 d1f5a7ef2e155c426a1e8d4812e02efde24b0453..467383ea0fb9967a0406b6c5b6c7cb06a768a6da 100644 (file)
--- 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<Set
         assertEquals(Set.of(INSTANCE_NAME_Q_NAME, NEXT_SERVICE_Q_NAME), result.get(2));
     }
 
+    @Override
+    protected void assertMultipleChildren4(final List<Set<QName>> 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<Set<QName>> 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<Set<QName>> result) {
         // FIXME: add assertions