summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authordanielhanrahan <daniel.hanrahan@est.tech>2023-03-10 13:27:01 +0000
committerdanielhanrahan <daniel.hanrahan@est.tech>2023-03-13 10:30:58 +0000
commit8e08dc797c89f6e85347c10cc7a2e877c126432a (patch)
tree5bf8d3653b332e0dce846dd3ba748520029070ec
parente2e5e5337f65b49aef56d4bced4ca9e298825543 (diff)
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
-rw-r--r--cps-ri/src/main/java/org/onap/cps/spi/impl/CpsDataPersistenceServiceImpl.java102
-rwxr-xr-xcps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceServiceIntegrationSpec.groovy13
-rw-r--r--cps-ri/src/test/groovy/org/onap/cps/spi/performance/CpsDataPersistenceServiceDeletePerfTest.groovy24
3 files changed, 49 insertions, 90 deletions
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 f634008dc6..916baa8c23 100644
--- 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 28916b1c4a..71253278e9 100755
--- 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 b6763db5f9..42698a65d6 100644
--- 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])