Fix the datanode build logic (incorrect parsing of containers and mapped lists)

Issue-ID: CPS-198
Change-Id: Ideb89f777a1bc155603152991174680fad8bb513
Signed-off-by: Ruslan Kashapov <ruslan.kashapov@pantheon.tech>

diff --git a/cps-service/src/main/java/org/onap/cps/spi/model/DataNode.java b/cps-service/src/main/java/org/onap/cps/spi/model/DataNode.java
index 721a7c0..5613894 100644 (file)
--- a/cps-service/src/main/java/org/onap/cps/spi/model/DataNode.java
+++ b/cps-service/src/main/java/org/onap/cps/spi/model/DataNode.java
@@ -24,8 +24,6 @@ package org.onap.cps.spi.model;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Map;
-import java.util.Optional;
-import java.util.Set;
 import lombok.AccessLevel;
 import lombok.Getter;
 import lombok.Setter;
@@ -44,5 +42,4 @@ public class DataNode {
     private Map<String, Object> leaves = Collections.emptyMap();
     private Collection<String> xpathsChildren;
     private Collection<DataNode> childDataNodes = Collections.emptySet();
-    private Optional<Set<String>> optionalLeafListNames = Optional.empty();
 }

diff --git a/cps-service/src/main/java/org/onap/cps/spi/model/DataNodeBuilder.java b/cps-service/src/main/java/org/onap/cps/spi/model/DataNodeBuilder.java
index cd6a3a2..d187f62 100644 (file)
--- a/cps-service/src/main/java/org/onap/cps/spi/model/DataNodeBuilder.java
+++ b/cps-service/src/main/java/org/onap/cps/spi/model/DataNodeBuilder.java
@@ -23,13 +23,13 @@ import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
-import java.util.Optional;
 import java.util.Set;
+import java.util.stream.Collectors;
 import lombok.extern.slf4j.Slf4j;
 import org.onap.cps.utils.YangUtils;
+import org.opendaylight.yangtools.yang.data.api.schema.DataContainerChild;
 import org.opendaylight.yangtools.yang.data.api.schema.DataContainerNode;
 import org.opendaylight.yangtools.yang.data.api.schema.LeafSetNode;
 import org.opendaylight.yangtools.yang.data.api.schema.MapEntryNode;
@@ -40,24 +40,25 @@ import org.opendaylight.yangtools.yang.data.api.schema.ValueNode;
 @Slf4j
 public class DataNodeBuilder {
 
-    private NormalizedNode normalizedNodeTree;
+    private NormalizedNode<?, ?> normalizedNodeTree;
     private String xpath;
     private Collection<DataNode> childDataNodes = Collections.emptySet();
 
 
-    /** To use {@link NormalizedNode} for creating {@link DataNode}.
+    /**
+     * To use {@link NormalizedNode} for creating {@link DataNode}.
      *
      * @param normalizedNodeTree used for creating the Data Node
-     *
      * @return this {@link DataNodeBuilder} object
      */
-    public DataNodeBuilder withNormalizedNodeTree(final NormalizedNode normalizedNodeTree) {
+    public DataNodeBuilder withNormalizedNodeTree(final NormalizedNode<?, ?> normalizedNodeTree) {
         this.normalizedNodeTree = normalizedNodeTree;
         return this;
     }
 
     /**
      * To use xpath for creating {@link DataNode}.
+     *
      * @param xpath for the data node
      * @return DataNodeBuilder
      */
@@ -68,6 +69,7 @@ public class DataNodeBuilder {
 
     /**
      * To specify child nodes needs to be used while creating {@link DataNode}.
+     *
      * @param childDataNodes to be added to the dataNode
      * @return DataNodeBuilder
      */
@@ -99,71 +101,65 @@ public class DataNodeBuilder {
     }
 
     private DataNode buildFromNormalizedNodeTree() {
-        xpath = YangUtils.buildXpath(normalizedNodeTree.getIdentifier());
-        final DataNode dataNode = new DataNodeBuilder().withXpath(xpath).build();
-        addDataNodeFromNormalizedNode(dataNode, normalizedNodeTree);
-        return dataNode;
+        final DataNode formalRootDataNode = new DataNodeBuilder().withXpath("").build();
+        addDataNodeFromNormalizedNode(formalRootDataNode, normalizedNodeTree);
+        return formalRootDataNode.getChildDataNodes().iterator().next();
     }
 
-    private void addDataNodeFromNormalizedNode(final DataNode currentDataNode,
-        final NormalizedNode normalizedNode) {
+    private static void addDataNodeFromNormalizedNode(final DataNode currentDataNode,
+        final NormalizedNode<?, ?> normalizedNode) {
+
         if (normalizedNode instanceof DataContainerNode) {
-            addYangContainer(currentDataNode, (DataContainerNode) normalizedNode);
+            addYangContainer(currentDataNode, (DataContainerNode<?>) normalizedNode);
         } else if (normalizedNode instanceof MapNode) {
             addDataNodeForEachListElement(currentDataNode, (MapNode) normalizedNode);
         } else if (normalizedNode instanceof ValueNode) {
-            final ValueNode valuesNode = (ValueNode) normalizedNode;
+            final ValueNode<?, ?> valuesNode = (ValueNode<?, ?>) normalizedNode;
             addYangLeaf(currentDataNode, valuesNode.getNodeType().getLocalName(), valuesNode.getValue());
         } else if (normalizedNode instanceof LeafSetNode) {
-            addYangLeafList(currentDataNode, (LeafSetNode) normalizedNode);
+            addYangLeafList(currentDataNode, (LeafSetNode<?>) normalizedNode);
         } else {
-            log.warn("Cannot normalize {}", normalizedNode.getClass());
+            log.warn("Unsupported NormalizedNode type detected: {}", normalizedNode.getClass());
         }
     }
 
-    private void addYangContainer(final DataNode currentDataNode, final DataContainerNode dataContainerNode) {
-        final Collection<NormalizedNode> normalizedChildNodes = dataContainerNode.getValue();
-        for (final NormalizedNode normalizedNode : normalizedChildNodes) {
-            addDataNodeFromNormalizedNode(currentDataNode, normalizedNode);
+    private static void addYangContainer(final DataNode currentDataNode, final DataContainerNode<?> dataContainerNode) {
+        final DataNode dataContainerDataNode = createAndAddChildDataNode(currentDataNode,
+            YangUtils.buildXpath(dataContainerNode.getIdentifier()));
+        final Collection<DataContainerChild<?, ?>> normalizedChildNodes = dataContainerNode.getValue();
+        for (final NormalizedNode<?, ?> normalizedNode : normalizedChildNodes) {
+            addDataNodeFromNormalizedNode(dataContainerDataNode, normalizedNode);
         }
     }
 
-    private void addYangLeaf(final DataNode currentDataNode, final String leafName, final Object leafValue) {
-        final Map leaves = new ImmutableMap.Builder<String, Object>()
+    private static void addYangLeaf(final DataNode currentDataNode, final String leafName, final Object leafValue) {
+        final Map<String, Object> leaves = new ImmutableMap.Builder<String, Object>()
             .putAll(currentDataNode.getLeaves())
             .put(leafName, leafValue)
             .build();
         currentDataNode.setLeaves(leaves);
     }
 
-    private void addYangLeafList(final DataNode currentDataNode, final LeafSetNode leafSetNode) {
-        final ImmutableSet.Builder builder = new ImmutableSet.Builder<String>();
+    private static void addYangLeafList(final DataNode currentDataNode, final LeafSetNode<?> leafSetNode) {
         final String leafListName = leafSetNode.getNodeType().getLocalName();
-        final Optional<Set<String>> optionalLeafListNames = currentDataNode.getOptionalLeafListNames();
-        if (optionalLeafListNames.isPresent()) {
-            builder.addAll(optionalLeafListNames.get());
-        }
-        builder.add(leafListName);
-        final ImmutableSet leafListNames = builder.build();
-        currentDataNode.setOptionalLeafListNames(Optional.of(leafListNames));
-        final List leafListValues = new LinkedList();
-        for (final NormalizedNode normalizedNode : (Collection<NormalizedNode>) leafSetNode.getValue()) {
-            leafListValues.add(((ValueNode) normalizedNode).getValue());
-        }
+        final List<?> leafListValues = ((Collection<? extends NormalizedNode<?, ?>>) leafSetNode.getValue())
+            .stream()
+            .map(normalizedNode -> ((ValueNode<?, ?>) normalizedNode).getValue())
+            .collect(Collectors.toUnmodifiableList());
         addYangLeaf(currentDataNode, leafListName, leafListValues);
     }
 
-    private void addDataNodeForEachListElement(final DataNode currentDataNode, final MapNode mapNode) {
+    private static void addDataNodeForEachListElement(final DataNode currentDataNode, final MapNode mapNode) {
         final Collection<MapEntryNode> mapEntryNodes = mapNode.getValue();
         for (final MapEntryNode mapEntryNode : mapEntryNodes) {
-            final String xpathChild = YangUtils.buildXpath(mapEntryNode.getIdentifier());
-            final DataNode childDataNode = createAndAddChildDataNode(currentDataNode, xpathChild);
-            addDataNodeFromNormalizedNode(childDataNode, mapEntryNode);
+            addDataNodeFromNormalizedNode(currentDataNode, mapEntryNode);
         }
     }
 
-    private DataNode createAndAddChildDataNode(final DataNode parentDataNode, final String childXpath) {
-        final DataNode newChildDataNode = new DataNodeBuilder().withXpath(xpath + childXpath)
+    private static DataNode createAndAddChildDataNode(final DataNode parentDataNode, final String childXpath) {
+
+        final DataNode newChildDataNode = new DataNodeBuilder()
+            .withXpath(parentDataNode.getXpath() + childXpath)
             .build();
         final Set<DataNode> allChildDataNodes = new ImmutableSet.Builder<DataNode>()
             .addAll(parentDataNode.getChildDataNodes())

diff --git a/cps-service/src/test/groovy/org/onap/cps/model/DataNodeBuilderSpec.groovy b/cps-service/src/test/groovy/org/onap/cps/model/DataNodeBuilderSpec.groovy
index 0dbde88..d881e77 100644 (file)
--- a/cps-service/src/test/groovy/org/onap/cps/model/DataNodeBuilderSpec.groovy
+++ b/cps-service/src/test/groovy/org/onap/cps/model/DataNodeBuilderSpec.groovy
@@ -1,6 +1,7 @@
 package org.onap.cps.model
 
 import org.onap.cps.TestUtils
+import org.onap.cps.spi.model.DataNode
 import org.onap.cps.spi.model.DataNodeBuilder
 import org.onap.cps.utils.YangUtils
 import org.onap.cps.yang.YangTextSchemaSourceSetBuilder
@@ -8,22 +9,52 @@ import spock.lang.Specification
 
 class DataNodeBuilderSpec extends Specification {
 
+    Map<String, Map<String, Object>> expectedLeavesByXpathMap = [
+            '/test-tree'                             : [],
+            '/test-tree/branch[@name=\'Left\']'      : [name: 'Left'],
+            '/test-tree/branch[@name=\'Left\']/nest' : [name: 'Small', birds: ['Sparrow', 'Robin', 'Finch']],
+            '/test-tree/branch[@name=\'Right\']'     : [name: 'Right'],
+            '/test-tree/branch[@name=\'Right\']/nest': [name: 'Big', birds: ['Owl', 'Raven', 'Crow']]
+    ]
+
     def 'Converting Normalized Node (tree) to a DataNode (tree).'() {
         given: 'a Yang module'
-            def yangResourceNameToContent = TestUtils.getYangResourcesAsMap('bookstore.yang')
-            def schemaContext = YangTextSchemaSourceSetBuilder.of(yangResourceNameToContent)getSchemaContext()
+            def yangResourceNameToContent = TestUtils.getYangResourcesAsMap('test-tree.yang')
+            def schemaContext = YangTextSchemaSourceSetBuilder.of(yangResourceNameToContent) getSchemaContext()
         and: 'a normalized node for that model'
-            def jsonData = TestUtils.getResourceFileContent('bookstore.json')
+            def jsonData = TestUtils.getResourceFileContent('test-tree.json')
             def normalizedNode = YangUtils.parseJsonData(jsonData, schemaContext)
         when: 'the normalized node is converted to a DataNode (tree)'
             def result = new DataNodeBuilder().withNormalizedNodeTree(normalizedNode).build()
-        then: 'the system creates a (root) fragment without a parent and 2 children (categories)'
-            result.childDataNodes.size() == 2
-        and: 'each child (category) has the root fragment (result) as parent and in turn as 1 child (a list of books)'
-            result.childDataNodes.each { it.childDataNodes.size() == 1 }
-        and: 'the fragments have the correct xpaths'
-            assert result.xpath == '/bookstore'
-            assert result.childDataNodes.collect { it.xpath }
-                    .containsAll(["/bookstore/categories[@code='01']", "/bookstore/categories[@code='02']"])
+            def mappedResult = treeToFlatMapByXpath(new HashMap<>(), result)
+        then: '5 DataNode objects with unique xpath were created in total'
+            mappedResult.size() == 5
+        and: 'all expected xpaths were built'
+            mappedResult.keySet().containsAll(expectedLeavesByXpathMap.keySet())
+        and: 'each data node contains the expected attributes'
+            mappedResult.each {
+                xpath, dataNode -> assertLeavesMaps(dataNode.getLeaves(), expectedLeavesByXpathMap[xpath])
+            }
+    }
+
+    def static assertLeavesMaps(actualLeavesMap, expectedLeavesMap) {
+        expectedLeavesMap.each { key, value ->
+            {
+                def actualValue = actualLeavesMap[key]
+                if (value instanceof Collection<?> && actualValue instanceof Collection<?>) {
+                    assert value.size() == actualValue.size()
+                    assert value.containsAll(actualValue)
+                } else {
+                    assert value == actualValue
+                }
+            }
+        }
+    }
+
+    def treeToFlatMapByXpath(Map<String, DataNode> flatMap, DataNode dataNodeTree) {
+        flatMap.put(dataNodeTree.getXpath(), dataNodeTree)
+        dataNodeTree.getChildDataNodes()
+                .forEach(childDataNode -> treeToFlatMapByXpath(flatMap, childDataNode))
+        return flatMap
     }
 }

diff --git a/cps-service/src/test/resources/test-tree.json b/cps-service/src/test/resources/test-tree.json
new file mode 100644 (file)
index 0000000..bc9cbd7
--- /dev/null
+++ b/cps-service/src/test/resources/test-tree.json
@@ -0,0 +1,28 @@
+{
+  "test-tree": {
+    "branch": [
+      {
+        "name": "Left",
+        "nest": {
+          "name": "Small",
+          "birds": [
+            "Sparrow",
+            "Robin",
+            "Finch"
+          ]
+        }
+      },
+      {
+        "name": "Right",
+        "nest": {
+          "name": "Big",
+          "birds": [
+            "Owl",
+            "Raven",
+            "Crow"
+          ]
+        }
+      }
+    ]
+  }
+}
\ No newline at end of file

diff --git a/cps-service/src/test/resources/test-tree.yang b/cps-service/src/test/resources/test-tree.yang
new file mode 100644 (file)
index 0000000..faba8a1
--- /dev/null
+++ b/cps-service/src/test/resources/test-tree.yang
@@ -0,0 +1,24 @@
+module test-tree {
+    yang-version 1.1;
+
+    namespace "org:onap:cps:test:test-tree";
+    prefix tree;
+    revision "2020-02-02";
+
+    container test-tree {
+        list branch {
+            key "name";
+            leaf name {
+                type string;
+            }
+            container nest {
+                leaf name {
+                    type string;
+                }
+                leaf-list birds {
+                    type string;
+                }
+            }
+        }
+    }
+}