From dcf84ad73f0301ef41049e692b9963f6dcac3661 Mon Sep 17 00:00:00 2001 From: ToineSiebelink Date: Mon, 17 Jul 2023 09:22:05 +0100 Subject: 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 Change-Id: I9761ad6d4777a6bcfee4cbe1b876dd39fa218fc8 --- .../org/onap/cps/api/impl/CpsDataServiceImpl.java | 9 +-- .../cps/api/impl/CpsDataServiceImplSpec.groovy | 69 ++++++++++------------ .../groovy/org/onap/cps/utils/YangUtilsSpec.groovy | 6 +- 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 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' | '> { 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.'() { -- cgit 1.2.3-korg