Make single deleteDataNode use plural deleteDataNodes

- Make deleteDataNode and deleteListDataNode call deleteDataNodes
- Add onlySupportListDeletion option to deleteDataNodes to support
  original deleteListDataNode behaviour
- Allow delete root xpath in deleteDataNodes
- Fix incorrect use of PathParsingException in deleteDataNode
- Update performance tests timings

Issue-ID: CPS-1523
Signed-off-by: danielhanrahan <daniel.hanrahan@est.tech>
Change-Id: I92c3c3ce606a5ab2cb8e6779d1ee0f9853529982

diff --git a/cps-ri/src/main/java/org/onap/cps/spi/impl/CpsDataPersistenceServiceImpl.java b/cps-ri/src/main/java/org/onap/cps/spi/impl/CpsDataPersistenceServiceImpl.java
index f634008..916baa8 100644 (file)
--- a/cps-ri/src/main/java/org/onap/cps/spi/impl/CpsDataPersistenceServiceImpl.java
+++ b/cps-ri/src/main/java/org/onap/cps/spi/impl/CpsDataPersistenceServiceImpl.java
@@ -253,7 +253,7 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService
     public Collection<DataNode> getDataNodes(final String dataspaceName, final String anchorName,
                                              final String xpath,
                                              final FetchDescendantsOption fetchDescendantsOption) {
-        final String targetXpath = isRootXpath(xpath) ? xpath : CpsPathUtil.getNormalizedXpath(xpath);
+        final String targetXpath = getNormalizedXpath(xpath);
         final Collection<DataNode> dataNodes = getDataNodesForMultipleXpaths(dataspaceName, anchorName,
                 Collections.singletonList(targetXpath), fetchDescendantsOption);
         if (dataNodes.isEmpty()) {
@@ -411,13 +411,14 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService
     }
 
     private static String getNormalizedXpath(final String xpathSource) {
-        final String normalizedXpath;
+        if (isRootXpath(xpathSource)) {
+            return xpathSource;
+        }
         try {
-            normalizedXpath = CpsPathUtil.getNormalizedXpath(xpathSource);
+            return CpsPathUtil.getNormalizedXpath(xpathSource);
         } catch (final PathParsingException e) {
             throw new CpsPathException(e.getMessage());
         }
-        return normalizedXpath;
     }
 
     @Override
@@ -583,7 +584,7 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService
         final String listElementXpathPrefix = getListElementXpathPrefix(newListElements);
         final Map<String, FragmentEntity> existingListElementFragmentEntitiesByXPath =
                 extractListElementFragmentEntitiesByXPath(parentEntity.getChildFragments(), listElementXpathPrefix);
-        deleteListElements(parentEntity.getChildFragments(), existingListElementFragmentEntitiesByXPath);
+        parentEntity.getChildFragments().removeAll(existingListElementFragmentEntitiesByXPath.values());
         final Set<FragmentEntity> updatedChildFragmentEntities = new HashSet<>();
         for (final DataNode newListElement : newListElements) {
             final FragmentEntity existingListElementEntity =
@@ -602,8 +603,7 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService
     public void deleteDataNodes(final String dataspaceName, final String anchorName) {
         final DataspaceEntity dataspaceEntity = dataspaceRepository.getByName(dataspaceName);
         anchorRepository.findByDataspaceAndName(dataspaceEntity, anchorName)
-                .ifPresent(
-                        anchorEntity -> fragmentRepository.deleteByAnchorIn(Set.of(anchorEntity)));
+            .ifPresent(anchorEntity -> fragmentRepository.deleteByAnchorIn(Collections.singletonList(anchorEntity)));
     }
 
     @Override
@@ -619,6 +619,17 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService
     @Transactional
     public void deleteDataNodes(final String dataspaceName, final String anchorName,
                                 final Collection<String> xpathsToDelete) {
+        deleteDataNodes(dataspaceName, anchorName, xpathsToDelete, false);
+    }
+
+    private void deleteDataNodes(final String dataspaceName, final String anchorName,
+                                 final Collection<String> xpathsToDelete, final boolean onlySupportListDeletion) {
+        final boolean haveRootXpath = xpathsToDelete.stream().anyMatch(CpsDataPersistenceServiceImpl::isRootXpath);
+        if (haveRootXpath) {
+            deleteDataNodes(dataspaceName, anchorName);
+            return;
+        }
+
         final DataspaceEntity dataspaceEntity = dataspaceRepository.getByName(dataspaceName);
         final AnchorEntity anchorEntity = anchorRepository.getByDataspaceAndName(dataspaceEntity, anchorName);
 
@@ -633,7 +644,13 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService
 
         final Collection<String> xpathsToExistingContainers =
             fragmentRepository.findAllXpathByAnchorAndXpathIn(anchorEntity, deleteChecklist);
-        deleteChecklist.removeAll(xpathsToExistingContainers);
+        if (onlySupportListDeletion) {
+            final Collection<String> xpathsToExistingListElements = xpathsToExistingContainers.stream()
+                .filter(CpsPathUtil::isPathToListElement).collect(Collectors.toList());
+            deleteChecklist.removeAll(xpathsToExistingListElements);
+        } else {
+            deleteChecklist.removeAll(xpathsToExistingContainers);
+        }
 
         final Collection<String> xpathsToExistingLists = deleteChecklist.stream()
             .filter(xpath -> fragmentRepository.existsByAnchorAndXpathStartsWith(anchorEntity, xpath + "["))
@@ -663,66 +680,13 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService
 
     private void deleteDataNode(final String dataspaceName, final String anchorName, final String targetXpath,
                                 final boolean onlySupportListNodeDeletion) {
-        final String parentNodeXpath;
-        FragmentEntity parentFragmentEntity = null;
-        boolean targetDeleted;
-        if (isRootXpath(targetXpath)) {
-            deleteDataNodes(dataspaceName, anchorName);
-            targetDeleted = true;
-        } else {
-            if (isRootContainerNodeXpath(targetXpath)) {
-                parentNodeXpath = targetXpath;
-            } else {
-                parentNodeXpath = CpsPathUtil.getNormalizedParentXpath(targetXpath);
-            }
-            parentFragmentEntity = getFragmentEntity(dataspaceName, anchorName, parentNodeXpath);
-            if (CpsPathUtil.isPathToListElement(targetXpath)) {
-                targetDeleted = deleteDataNode(parentFragmentEntity, targetXpath);
-            } else {
-                targetDeleted = deleteAllListElements(parentFragmentEntity, targetXpath);
-                final boolean tryToDeleteDataNode = !targetDeleted && !onlySupportListNodeDeletion;
-                if (tryToDeleteDataNode) {
-                    targetDeleted = deleteDataNode(parentFragmentEntity, targetXpath);
-                }
-            }
-        }
-        if (!targetDeleted) {
-            final String additionalInformation = onlySupportListNodeDeletion
-                    ? "The target is probably not a List." : "";
-            throw new DataNodeNotFoundException(parentFragmentEntity.getDataspace().getName(),
-                    parentFragmentEntity.getAnchor().getName(), targetXpath, additionalInformation);
-        }
-    }
-
-    private boolean deleteDataNode(final FragmentEntity parentFragmentEntity, final String targetXpath) {
-        final String normalizedTargetXpath = CpsPathUtil.getNormalizedXpath(targetXpath);
-        if (parentFragmentEntity.getXpath().equals(normalizedTargetXpath)) {
-            fragmentRepository.deleteFragmentEntity(parentFragmentEntity.getId());
-            return true;
-        }
-        if (parentFragmentEntity.getChildFragments()
-                .removeIf(fragment -> fragment.getXpath().equals(normalizedTargetXpath))) {
-            fragmentRepository.save(parentFragmentEntity);
-            return true;
-        }
-        return false;
-    }
-
-    private boolean deleteAllListElements(final FragmentEntity parentFragmentEntity, final String listXpath) {
-        final String normalizedListXpath = CpsPathUtil.getNormalizedXpath(listXpath);
-        final String deleteTargetXpathPrefix = normalizedListXpath + "[";
-        if (parentFragmentEntity.getChildFragments()
-                .removeIf(fragment -> fragment.getXpath().startsWith(deleteTargetXpathPrefix))) {
-            fragmentRepository.save(parentFragmentEntity);
-            return true;
+        final String normalizedXpath = getNormalizedXpath(targetXpath);
+        try {
+            deleteDataNodes(dataspaceName, anchorName, Collections.singletonList(normalizedXpath),
+                onlySupportListNodeDeletion);
+        } catch (final DataNodeNotFoundExceptionBatch dataNodeNotFoundExceptionBatch) {
+            throw new DataNodeNotFoundException(dataspaceName, anchorName, targetXpath);
         }
-        return false;
-    }
-
-    private static void deleteListElements(
-            final Collection<FragmentEntity> fragmentEntities,
-            final Map<String, FragmentEntity> existingListElementFragmentEntitiesByXPath) {
-        fragmentEntities.removeAll(existingListElementFragmentEntitiesByXPath.values());
     }
 
     private static String getListElementXpathPrefix(final Collection<DataNode> newListElements) {
@@ -755,10 +719,6 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService
         return !existingListElementsByXpath.containsKey(replacementDataNode.getXpath());
     }
 
-    private static boolean isRootContainerNodeXpath(final String xpath) {
-        return 0 == xpath.lastIndexOf('/');
-    }
-
     private void copyAttributesFromNewListElement(final FragmentEntity existingListElementEntity,
                                                   final DataNode newListElement) {
         final FragmentEntity replacementFragmentEntity =

diff --git a/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceServiceIntegrationSpec.groovy b/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceServiceIntegrationSpec.groovy
index 28916b1..7125327 100755 (executable)
--- a/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceServiceIntegrationSpec.groovy
+++ b/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceServiceIntegrationSpec.groovy
@@ -25,7 +25,6 @@ package org.onap.cps.spi.impl
 
 import com.fasterxml.jackson.databind.ObjectMapper
 import com.google.common.collect.ImmutableSet
-import org.onap.cps.cpspath.parser.PathParsingException
 import org.onap.cps.spi.CpsDataPersistenceService
 import org.onap.cps.spi.entities.FragmentEntity
 import org.onap.cps.spi.exceptions.AlreadyDefinedExceptionBatch
@@ -253,8 +252,8 @@ class CpsDataPersistenceServiceIntegrationSpec extends CpsPersistenceSpecBase {
         when: 'trying to execute a query with a syntax (parsing) error'
             objectUnderTest.getDataNodes(DATASPACE_NAME, ANCHOR_FOR_DATA_NODES_WITH_LEAVES, 'invalid-cps-path/child' , OMIT_DESCENDANTS)
         then: 'exception is thrown'
-            def exceptionThrown = thrown(PathParsingException)
-            assert exceptionThrown.getMessage().contains('failed to parse at line 1 due to extraneous input \'invalid-cps-path\' expecting \'/\'')
+            def exceptionThrown = thrown(CpsPathException)
+            assert exceptionThrown.getDetails() == "failed to parse at line 1 due to extraneous input 'invalid-cps-path' expecting '/'"
     }
 
     @Sql([CLEAR_DATA, SET_DATA])
@@ -288,7 +287,7 @@ class CpsDataPersistenceServiceIntegrationSpec extends CpsPersistenceSpecBase {
         where: 'the following data is used'
             scenario             | dataspaceName  | anchorName                        | xpath           || expectedException
             'non existing xpath' | DATASPACE_NAME | ANCHOR_FOR_DATA_NODES_WITH_LEAVES | '/NO-XPATH'     || DataNodeNotFoundException
-            'invalid Xpath'      | DATASPACE_NAME | ANCHOR_FOR_DATA_NODES_WITH_LEAVES | 'INVALID XPATH' || PathParsingException
+            'invalid Xpath'      | DATASPACE_NAME | ANCHOR_FOR_DATA_NODES_WITH_LEAVES | 'INVALID XPATH' || CpsPathException
     }
 
     @Sql([CLEAR_DATA, SET_DATA])
@@ -667,9 +666,9 @@ class CpsDataPersistenceServiceIntegrationSpec extends CpsPersistenceSpecBase {
         then: 'a #expectedException is thrown'
             thrown(expectedException)
         where: 'the following parameters were used'
-            scenario                                        | datanodeXpath                                    | expectedException
-            'valid data node, non existent child node'      | '/parent-203/child-non-existent'                 | DataNodeNotFoundException
-            'invalid list element'                          | '/parent-206/child-206/grand-child-206@key="A"]' | PathParsingException
+            scenario                                   | datanodeXpath                                    | expectedException
+            'valid data node, non existent child node' | '/parent-203/child-non-existent'                 | DataNodeNotFoundException
+            'invalid list element'                     | '/parent-206/child-206/grand-child-206@key="A"]' | CpsPathException
     }
 
     @Sql([CLEAR_DATA, SET_DATA])

diff --git a/cps-ri/src/test/groovy/org/onap/cps/spi/performance/CpsDataPersistenceServiceDeletePerfTest.groovy b/cps-ri/src/test/groovy/org/onap/cps/spi/performance/CpsDataPersistenceServiceDeletePerfTest.groovy
index b6763db..42698a6 100644 (file)
--- a/cps-ri/src/test/groovy/org/onap/cps/spi/performance/CpsDataPersistenceServiceDeletePerfTest.groovy
+++ b/cps-ri/src/test/groovy/org/onap/cps/spi/performance/CpsDataPersistenceServiceDeletePerfTest.groovy
@@ -50,8 +50,8 @@ class CpsDataPersistenceServiceDeletePerfTest extends CpsPersistencePerfSpecBase
             }
             stopWatch.stop()
             def deleteDurationInMillis = stopWatch.getTotalTimeMillis()
-        then: 'delete duration is under 400 milliseconds'
-            recordAndAssertPerformance('Delete 5 children', 400, deleteDurationInMillis)
+        then: 'delete duration is under 150 milliseconds'
+            recordAndAssertPerformance('Delete 5 children', 150, deleteDurationInMillis)
     }
 
     def 'Batch delete 100 children with grandchildren'() {
@@ -77,8 +77,8 @@ class CpsDataPersistenceServiceDeletePerfTest extends CpsPersistencePerfSpecBase
             }
             stopWatch.stop()
             def deleteDurationInMillis = stopWatch.getTotalTimeMillis()
-        then: 'delete duration is under 400 milliseconds'
-            recordAndAssertPerformance('Delete 50 grandchildren', 400, deleteDurationInMillis)
+        then: 'delete duration is under 600 milliseconds'
+            recordAndAssertPerformance('Delete 50 grandchildren', 600, deleteDurationInMillis)
     }
 
     def 'Batch delete 500 grandchildren (that have no descendants)'() {
@@ -118,8 +118,8 @@ class CpsDataPersistenceServiceDeletePerfTest extends CpsPersistencePerfSpecBase
             }
             stopWatch.stop()
             def deleteDurationInMillis = stopWatch.getTotalTimeMillis()
-        then: 'delete duration is under 2 seconds'
-            recordAndAssertPerformance('Delete 5 whole lists', 2_000, deleteDurationInMillis)
+        then: 'delete duration is under 130 milliseconds'
+            recordAndAssertPerformance('Delete 5 whole lists', 130, deleteDurationInMillis)
     }
 
     def 'Batch delete 100 whole lists'() {
@@ -145,8 +145,8 @@ class CpsDataPersistenceServiceDeletePerfTest extends CpsPersistencePerfSpecBase
             }
             stopWatch.stop()
             def deleteDurationInMillis = stopWatch.getTotalTimeMillis()
-        then: 'delete duration is under 700 milliseconds'
-            recordAndAssertPerformance('Delete 10 lists elements', 700, deleteDurationInMillis)
+        then: 'delete duration is under 180 milliseconds'
+            recordAndAssertPerformance('Delete 10 lists elements', 180, deleteDurationInMillis)
     }
 
     def 'Batch delete 500 list elements'() {
@@ -176,8 +176,8 @@ class CpsDataPersistenceServiceDeletePerfTest extends CpsPersistencePerfSpecBase
             objectUnderTest.deleteDataNode(PERF_DATASPACE, PERF_ANCHOR, PERF_TEST_PARENT)
             stopWatch.stop()
             def deleteDurationInMillis = stopWatch.getTotalTimeMillis()
-        then: 'delete duration is under 400 milliseconds'
-            recordAndAssertPerformance('Delete one large node', 400, deleteDurationInMillis)
+        then: 'delete duration is under 220 milliseconds'
+            recordAndAssertPerformance('Delete one large node', 220, deleteDurationInMillis)
     }
 
     @Sql([CLEAR_DATA, PERF_TEST_DATA])
@@ -190,8 +190,8 @@ class CpsDataPersistenceServiceDeletePerfTest extends CpsPersistencePerfSpecBase
             objectUnderTest.deleteDataNodes(PERF_DATASPACE, PERF_ANCHOR, [PERF_TEST_PARENT])
             stopWatch.stop()
             def deleteDurationInMillis = stopWatch.getTotalTimeMillis()
-        then: 'delete duration is under 300 milliseconds'
-            recordAndAssertPerformance('Batch delete one large node', 300, deleteDurationInMillis)
+        then: 'delete duration is under 220 milliseconds'
+            recordAndAssertPerformance('Batch delete one large node', 220, deleteDurationInMillis)
     }
 
     @Sql([CLEAR_DATA, PERF_TEST_DATA])