Improved code coverage for CpsDataServiceImpl

- Added (extended) test to cover missed scenarios
- Removed unnecesarry null check from Production code
- Improved error messages in production code
- Removed duplicate test
- Cleaned up existign test somewhat (labels and aligment)

Issue-ID: CPS-475
Signed-off-by: ToineSiebelink <toine.siebelink@est.tech>
Change-Id: I9761ad6d4777a6bcfee4cbe1b876dd39fa218fc8

3 files changed, 38 insertions, 46 deletions

diff --git a/cps-service/src/main/java/org/onap/cps/api/impl/CpsDataServiceImpl.java b/cps-service/src/main/java/org/onap/cps/api/impl/CpsDataServiceImpl.java
index 0a7afc8f6..6e7c1649d 100755
--- a/cps-service/src/main/java/org/onap/cps/api/impl/CpsDataServiceImpl.java
+++ b/cps-service/src/main/java/org/onap/cps/api/impl/CpsDataServiceImpl.java
@@ -351,7 +351,7 @@ public class CpsDataServiceImpl implements CpsDataService {
                     .withContainerNode(containerNode)
                     .buildCollection();
             if (dataNodes.isEmpty()) {
-                throw new DataValidationException("Invalid data.", "No data nodes provided");
+                throw new DataValidationException("No data nodes.", "No data nodes provided");
             }
             return dataNodes;
         }
@@ -363,7 +363,7 @@ public class CpsDataServiceImpl implements CpsDataService {
             .withContainerNode(containerNode)
             .buildCollection();
         if (dataNodes.isEmpty()) {
-            throw new DataValidationException("Invalid data.", "No data nodes provided");
+            throw new DataValidationException("No data nodes.", "No data nodes provided");
         }
         return dataNodes;
     }
@@ -381,7 +381,7 @@ public class CpsDataServiceImpl implements CpsDataService {
         try {
             notificationService.processDataUpdatedEvent(anchor, xpath, operation, observedTimestamp);
         } catch (final Exception exception) {
-            //If async message can't be queued for notification service, the initial request should not failed.
+            //If async message can't be queued for notification service, the initial request should not fail.
             log.error("Failed to send message to notification service", exception);
         }
     }
@@ -392,9 +392,6 @@ public class CpsDataServiceImpl implements CpsDataService {
     }
 
     private void processDataNodeUpdate(final Anchor anchor, final DataNode dataNodeUpdate) {
-        if (dataNodeUpdate == null) {
-            return;
-        }
         cpsDataPersistenceService.batchUpdateDataLeaves(anchor.getDataspaceName(), anchor.getName(),
                 Collections.singletonMap(dataNodeUpdate.getXpath(), dataNodeUpdate.getLeaves()));
         final Collection<DataNode> childDataNodeUpdates = dataNodeUpdate.getChildDataNodes();
diff --git a/cps-service/src/test/groovy/org/onap/cps/api/impl/CpsDataServiceImplSpec.groovy b/cps-service/src/test/groovy/org/onap/cps/api/impl/CpsDataServiceImplSpec.groovy
index e357d2462..db8664042 100644
--- a/cps-service/src/test/groovy/org/onap/cps/api/impl/CpsDataServiceImplSpec.groovy
+++ b/cps-service/src/test/groovy/org/onap/cps/api/impl/CpsDataServiceImplSpec.groovy
@@ -64,26 +64,6 @@ class CpsDataServiceImplSpec extends Specification {
     def anchor = Anchor.builder().name(anchorName).dataspaceName(dataspaceName).schemaSetName(schemaSetName).build()
     def observedTimestamp = OffsetDateTime.now()
 
-    def 'Saving multicontainer json data.'() {
-        given: 'schema set for given anchor and dataspace references test-tree model'
-            setupSchemaSetMocks('multipleDataTree.yang')
-        when: 'save data method is invoked with test-tree json data'
-            def jsonData = TestUtils.getResourceFileContent('multiple-object-data.json')
-            objectUnderTest.saveData(dataspaceName, anchorName, jsonData, observedTimestamp)
-        then: 'the persistence service method is invoked with correct parameters'
-            1 * mockCpsDataPersistenceService.storeDataNodes(dataspaceName, anchorName,
-                { dataNode -> dataNode.xpath[index] == xpath })
-        and: 'the CpsValidator is called on the dataspaceName and AnchorName'
-            1 * mockCpsValidator.validateNameCharacters(dataspaceName, anchorName)
-        and: 'data updated event is sent to notification service'
-            1 * mockNotificationService.processDataUpdatedEvent(anchor, '/', Operation.CREATE, observedTimestamp)
-        where:
-            index   |   xpath
-                0   | '/first-container'
-                1   | '/last-container'
-
-    }
-
     def 'Saving #scenario data.'() {
         given: 'schema set for given anchor and dataspace references test-tree model'
             setupSchemaSetMocks('test-tree.yang')
@@ -103,19 +83,32 @@ class CpsDataServiceImplSpec extends Specification {
             'xml'    | 'test-tree.xml'  | ContentType.XML
     }
 
-    def 'Saving #scenarioDesired data with invalid data.'() {
+    def 'Saving data with error: #scenario.'() {
         given: 'schema set for given anchor and dataspace references test-tree model'
-        setupSchemaSetMocks('test-tree.yang')
+            setupSchemaSetMocks('test-tree.yang')
         when: 'save data method is invoked with test-tree json data'
             objectUnderTest.saveData(dataspaceName, anchorName, invalidData, observedTimestamp, contentType)
-        then: 'a data validation exception is thrown'
-            thrown(DataValidationException)
+        then: 'a data validation exception is thrown with the correct message'
+            def exceptionThrown  = thrown(DataValidationException)
+            assert exceptionThrown.message.startsWith(expectedMessage)
         where: 'given parameters'
-            scenarioDesired | invalidData             | contentType
-            'json'          | '{invalid  json'        | ContentType.XML
-            'xml'           | '<invalid xml'          | ContentType.JSON
+            scenario        | invalidData     | contentType      || expectedMessage
+            'no data nodes' | '{}'            | ContentType.JSON || 'No data nodes'
+            'invalid json'  | '{invalid json' | ContentType.JSON || 'Failed to parse json data'
+            'invalid xml'   | '<invalid xml'  | ContentType.XML  || 'Failed to parse xml data'
     }
 
+    def 'Saving #scenarioDesired data exception during notification.'() {
+        given: 'schema set for given anchor and dataspace references test-tree model'
+            setupSchemaSetMocks('test-tree.yang')
+        and: 'the notification service throws an exception'
+            mockNotificationService.processDataUpdatedEvent(*_) >> { throw new RuntimeException('to be ignored')}
+        when: 'save data method is invoked with test-tree json data'
+            def data = TestUtils.getResourceFileContent('test-tree.json')
+            objectUnderTest.saveData(dataspaceName, anchorName, data, observedTimestamp)
+        then: 'the exception is ignored'
+            noExceptionThrown()
+    }
 
     def 'Saving child data fragment under existing node.'() {
         given: 'schema set for given anchor and dataspace references test-tree model'
@@ -235,12 +228,12 @@ class CpsDataServiceImplSpec extends Specification {
         then: 'the persistence service method is invoked with correct parameters'
             thrown(DataValidationException)
         where: 'following parameters were used'
-            scenario          | jsonData
+            scenario                  | jsonData
             'multiple expectedLeaves' | '{"code": "01","name": "some-name"}'
-            'one leaf'        | '{"name": "some-name"}'
+            'one leaf'                | '{"name": "some-name"}'
     }
 
-    def 'Update multiple data nodes' () {
+    def 'Update data nodes in different containers.' () {
         given: 'schema set for given dataspace and anchor refers multipleDataTree model'
             setupSchemaSetMocks('multipleDataTree.yang')
         and: 'json string with multiple data trees'
@@ -258,21 +251,23 @@ class CpsDataServiceImplSpec extends Specification {
             index | expectedNodeXpath
             0     | '/first-container'
             1     | '/last-container'
-
     }
 
-    def 'Update Bookstore node leaves' () {
+    def 'Update Bookstore node leaves and child.' () {
         given: 'a DMI registry model'
             setupSchemaSetMocks('bookstore.yang')
-        and: 'the expected json string'
-            def jsonData = '{"categories":[{"code":01,"name":"Romance"}]}'
+        and: 'json update for a category (parent) and new book (child)'
+            def jsonData = '{"categories":[{"code":01,"name":"Romance","books": [{"title": "new"}]}]}'
         when: 'update data method is invoked with json data and parent node xpath'
-            objectUnderTest.updateNodeLeavesAndExistingDescendantLeaves(dataspaceName, anchorName,
-                '/bookstore', jsonData, observedTimestamp)
-        then: 'the persistence service method is invoked with correct parameters'
+            objectUnderTest.updateNodeLeavesAndExistingDescendantLeaves(dataspaceName, anchorName, '/bookstore', jsonData, observedTimestamp)
+        then: 'the persistence service method is invoked for the category (parent)'
             1 * mockCpsDataPersistenceService.batchUpdateDataLeaves(dataspaceName, anchorName,
                     {updatedDataNodesPerXPath -> updatedDataNodesPerXPath.keySet()
                                                 .iterator().next() == "/bookstore/categories[@code='01']"})
+        and: 'the persistence service method is invoked for the new book (child)'
+            1 * mockCpsDataPersistenceService.batchUpdateDataLeaves(dataspaceName, anchorName,
+                {updatedDataNodesPerXPath -> updatedDataNodesPerXPath.keySet()
+                    .iterator().next() == "/bookstore/categories[@code='01']/books[@title='new']"})
         and: 'the CpsValidator is called on the dataspaceName and AnchorName'
             1 * mockCpsValidator.validateNameCharacters(dataspaceName, anchorName)
         and: 'the data updated event is sent to the notification service'
diff --git a/cps-service/src/test/groovy/org/onap/cps/utils/YangUtilsSpec.groovy b/cps-service/src/test/groovy/org/onap/cps/utils/YangUtilsSpec.groovy
index bf6e134a6..50b630643 100644
--- a/cps-service/src/test/groovy/org/onap/cps/utils/YangUtilsSpec.groovy
+++ b/cps-service/src/test/groovy/org/onap/cps/utils/YangUtilsSpec.groovy
@@ -44,9 +44,9 @@ class YangUtilsSpec extends Specification {
         then: 'qualified name of children created is as expected'
             result.body().getAt(index).getIdentifier().nodeType == QName.create('org:onap:ccsdk:multiDataTree', '2020-09-15', nodeName)
         where:
-            index   | nodeName
-            0       | 'first-container'
-            1       | 'last-container'
+            index | nodeName
+            0     | 'first-container'
+            1     | 'last-container'
     }
 
     def 'Parsing a valid #scenario String.'() {