From ceda6a0b4aa498ae236092cf36d396c004c61cd7 Mon Sep 17 00:00:00 2001 From: danielhanrahan Date: Tue, 28 Feb 2023 16:25:08 +0000 Subject: Add DataNodeNotFoundException to deleteDataNodes Current implementation of NCMP handle de-registration relies on DataNodeNotFoundException being thrown to report errors. - Make deleteDataNodes throw DataNodeFoundExceptionBatch - Update performance test timings Issue-ID: CPS-1481 Signed-off-by: danielhanrahan Change-Id: Ib833bdb4a23d24e1784bdaf4e5e5e8a9acb41c54 --- .../spi/impl/CpsDataPersistenceServiceImpl.java | 28 ++++++++++++++-------- .../cps/spi/repository/FragmentRepository.java | 6 +++++ ...CpsDataPersistenceServiceIntegrationSpec.groovy | 17 ++++++++++--- .../CpsDataPersistenceServiceDeletePerfTest.groovy | 24 +++++++++---------- 4 files changed, 50 insertions(+), 25 deletions(-) (limited to 'cps-ri/src') 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 2159ceae8a..f634008dc6 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 @@ -57,6 +57,7 @@ import org.onap.cps.spi.exceptions.ConcurrencyException; import org.onap.cps.spi.exceptions.CpsAdminException; import org.onap.cps.spi.exceptions.CpsPathException; import org.onap.cps.spi.exceptions.DataNodeNotFoundException; +import org.onap.cps.spi.exceptions.DataNodeNotFoundExceptionBatch; import org.onap.cps.spi.model.DataNode; import org.onap.cps.spi.model.DataNodeBuilder; import org.onap.cps.spi.repository.AnchorRepository; @@ -621,23 +622,30 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService final DataspaceEntity dataspaceEntity = dataspaceRepository.getByName(dataspaceName); final AnchorEntity anchorEntity = anchorRepository.getByDataspaceAndName(dataspaceEntity, anchorName); - final Collection normalizedXPathsToDelete = new ArrayList<>(xpathsToDelete.size()); - final Collection normalizedXpathsToPotentialLists = new ArrayList<>(); + final Collection deleteChecklist = new HashSet<>(xpathsToDelete.size()); for (final String xpath : xpathsToDelete) { try { - final CpsPathQuery cpsPathQuery = CpsPathUtil.getCpsPathQuery(xpath); - final String normalizedXpath = cpsPathQuery.getNormalizedXpath(); - normalizedXPathsToDelete.add(normalizedXpath); - if (!cpsPathQuery.isPathToListElement()) { - normalizedXpathsToPotentialLists.add(normalizedXpath); - } + deleteChecklist.add(CpsPathUtil.getNormalizedXpath(xpath)); } catch (final PathParsingException e) { log.debug("Error parsing xpath \"{}\": {}", xpath, e.getMessage()); } } - fragmentRepository.deleteByAnchorIdAndXpaths(anchorEntity.getId(), normalizedXPathsToDelete); - fragmentRepository.deleteListsByAnchorIdAndXpaths(anchorEntity.getId(), normalizedXpathsToPotentialLists); + final Collection xpathsToExistingContainers = + fragmentRepository.findAllXpathByAnchorAndXpathIn(anchorEntity, deleteChecklist); + deleteChecklist.removeAll(xpathsToExistingContainers); + + final Collection xpathsToExistingLists = deleteChecklist.stream() + .filter(xpath -> fragmentRepository.existsByAnchorAndXpathStartsWith(anchorEntity, xpath + "[")) + .collect(Collectors.toList()); + deleteChecklist.removeAll(xpathsToExistingLists); + + if (!deleteChecklist.isEmpty()) { + throw new DataNodeNotFoundExceptionBatch(dataspaceName, anchorName, deleteChecklist); + } + + fragmentRepository.deleteByAnchorIdAndXpaths(anchorEntity.getId(), xpathsToExistingContainers); + fragmentRepository.deleteListsByAnchorIdAndXpaths(anchorEntity.getId(), xpathsToExistingLists); } @Override diff --git a/cps-ri/src/main/java/org/onap/cps/spi/repository/FragmentRepository.java b/cps-ri/src/main/java/org/onap/cps/spi/repository/FragmentRepository.java index 8bdb7d985b..51ebcb4127 100755 --- a/cps-ri/src/main/java/org/onap/cps/spi/repository/FragmentRepository.java +++ b/cps-ri/src/main/java/org/onap/cps/spi/repository/FragmentRepository.java @@ -100,4 +100,10 @@ public interface FragmentRepository extends JpaRepository, nativeQuery = true) List quickFindWithDescendants(@Param("anchorId") int anchorId, @Param("xpathRegex") String xpathRegex); + + @Query("SELECT f.xpath FROM FragmentEntity f WHERE f.anchor = :anchor AND f.xpath IN :xpaths") + List findAllXpathByAnchorAndXpathIn(@Param("anchor") AnchorEntity anchorEntity, + @Param("xpaths") Collection xpaths); + + boolean existsByAnchorAndXpathStartsWith(AnchorEntity anchorEntity, String xpath); } 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 bb80a13206..28916b1c4a 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 @@ -41,7 +41,6 @@ import org.springframework.beans.factory.annotation.Autowired import org.springframework.test.context.jdbc.Sql import javax.validation.ConstraintViolationException -import java.nio.file.Path import static org.onap.cps.spi.FetchDescendantsOption.INCLUDE_ALL_DESCENDANTS import static org.onap.cps.spi.FetchDescendantsOption.OMIT_DESCENDANTS @@ -57,7 +56,6 @@ class CpsDataPersistenceServiceIntegrationSpec extends CpsPersistenceSpecBase { static final String SET_DATA = '/data/fragment.sql' static long ID_DATA_NODE_WITH_DESCENDANTS = 4001 static String XPATH_DATA_NODE_WITH_DESCENDANTS = '/parent-1' - static String XPATH_DATA_NODE_WITH_LEAVES = '/parent-207' static long DATA_NODE_202_FRAGMENT_ID = 4202L static long CHILD_OF_DATA_NODE_202_FRAGMENT_ID = 4203L static long LIST_DATA_NODE_PARENT201_FRAGMENT_ID = 4206L @@ -309,6 +307,7 @@ class CpsDataPersistenceServiceIntegrationSpec extends CpsPersistenceSpecBase { '2 unique nodes with duplicate xpath' | ["/parent-200", "/parent-202", "/parent-200"] || 2 'list element with key (single quote)' | ["/parent-201/child-204[@key='A']"] || 1 'list element with key (double quote)' | ['/parent-201/child-204[@key="A"]'] || 1 + 'whole list (not implemented)' | ["/parent-201/child-204"] || 0 'non-existing xpath' | ["/NO-XPATH"] || 0 'existing and non-existing xpaths' | ["/parent-200", "/NO-XPATH", "/parent-201"] || 2 'invalid xpath' | ["INVALID XPATH"] || 0 @@ -585,10 +584,22 @@ class CpsDataPersistenceServiceIntegrationSpec extends CpsPersistenceSpecBase { 'whole list' | ['/parent-203/child-204'] || ['/parent-203/child-203'] 'list and element in same list' | ['/parent-203/child-204', '/parent-203/child-204[@key="A"]'] || ['/parent-203/child-203'] 'list element under list element' | ['/parent-203/child-204[@key="B"]/grand-child-204[@key2="Y"]'] || ["/parent-203/child-203", "/parent-203/child-204[@key='A']", "/parent-203/child-204[@key='B']"] - 'valid but non-existing xpath' | ['/non-existing', '/parent-203/child-204'] || ['/parent-203/child-203'] 'invalid xpath' | ['INVALID XPATH', '/parent-203/child-204'] || ['/parent-203/child-203'] } + @Sql([CLEAR_DATA, SET_DATA]) + def 'Delete multiple data nodes error scenario: #scenario.'() { + when: 'deleting nodes is executed for: #scenario.' + objectUnderTest.deleteDataNodes(dataspaceName, anchorName, targetXpaths) + then: 'a #expectedException is thrown' + thrown(expectedException) + where: 'the following data is used' + scenario | dataspaceName | anchorName | targetXpaths || expectedException + 'non-existing dataspace' | 'NO DATASPACE' | 'not relevant' | ['/not relevant'] || DataspaceNotFoundException + 'non-existing anchor' | DATASPACE_NAME | 'NO ANCHOR' | ['/not relevant'] || AnchorNotFoundException + 'non-existing datanode' | DATASPACE_NAME | ANCHOR_NAME3 | ['/NON-EXISTING-XPATH'] || DataNodeNotFoundException + } + @Sql([CLEAR_DATA, SET_DATA]) def 'Delete data nodes with "/"-token in list key value: #scenario. (CPS-1409)'() { given: 'a data nodes with list-element child with "/" in index value (and grandchild)' 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 a08d8c66d2..eb138b98be 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 @@ -94,8 +94,8 @@ class CpsDataPersistenceServiceDeletePerfTest extends CpsPersistencePerfSpecBase objectUnderTest.deleteDataNodes(PERF_DATASPACE, PERF_ANCHOR, xpathsToDelete) stopWatch.stop() def deleteDurationInMillis = stopWatch.getTotalTimeMillis() - then: 'delete duration is under 125 milliseconds' - recordAndAssertPerformance('Batch delete 500 grandchildren', 125, deleteDurationInMillis) + then: 'delete duration is under 75 milliseconds' + recordAndAssertPerformance('Batch delete 500 grandchildren', 75, deleteDurationInMillis) } @Sql([CLEAR_DATA, PERF_TEST_DATA]) @@ -105,8 +105,8 @@ class CpsDataPersistenceServiceDeletePerfTest extends CpsPersistencePerfSpecBase createLineage(objectUnderTest, 150, 50, true) stopWatch.stop() def setupDurationInMillis = stopWatch.getTotalTimeMillis() - then: 'setup duration is under 10 seconds' - recordAndAssertPerformance('Setup lists', 10_000, setupDurationInMillis) + then: 'setup duration is under 5 seconds' + recordAndAssertPerformance('Setup lists', 5_000, setupDurationInMillis) } def 'Delete 5 whole lists'() { @@ -132,8 +132,8 @@ class CpsDataPersistenceServiceDeletePerfTest extends CpsPersistencePerfSpecBase objectUnderTest.deleteDataNodes(PERF_DATASPACE, PERF_ANCHOR, xpathsToDelete) stopWatch.stop() def deleteDurationInMillis = stopWatch.getTotalTimeMillis() - then: 'delete duration is under 250 milliseconds' - recordAndAssertPerformance('Batch delete 100 whole lists', 250, deleteDurationInMillis) + then: 'delete duration is under 500 milliseconds' + recordAndAssertPerformance('Batch delete 100 whole lists', 500, deleteDurationInMillis) } def 'Delete 10 list elements'() { @@ -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 250 milliseconds' - recordAndAssertPerformance('Batch delete one large node', 250, deleteDurationInMillis) + then: 'delete duration is under 200 milliseconds' + recordAndAssertPerformance('Batch delete one large node', 200, deleteDurationInMillis) } @Sql([CLEAR_DATA, PERF_TEST_DATA]) @@ -204,8 +204,8 @@ class CpsDataPersistenceServiceDeletePerfTest extends CpsPersistencePerfSpecBase objectUnderTest.deleteDataNode(PERF_DATASPACE, PERF_ANCHOR, '/') stopWatch.stop() def deleteDurationInMillis = stopWatch.getTotalTimeMillis() - then: 'delete duration is under 300 milliseconds' - recordAndAssertPerformance('Delete root node', 300, deleteDurationInMillis) + then: 'delete duration is under 250 milliseconds' + recordAndAssertPerformance('Delete root node', 250, deleteDurationInMillis) } @Sql([CLEAR_DATA, PERF_TEST_DATA]) @@ -218,8 +218,8 @@ class CpsDataPersistenceServiceDeletePerfTest extends CpsPersistencePerfSpecBase objectUnderTest.deleteDataNodes(PERF_DATASPACE, PERF_ANCHOR) stopWatch.stop() def deleteDurationInMillis = stopWatch.getTotalTimeMillis() - then: 'delete duration is under 300 milliseconds' - recordAndAssertPerformance('Delete data nodes for anchor', 300, deleteDurationInMillis) + then: 'delete duration is under 250 milliseconds' + recordAndAssertPerformance('Delete data nodes for anchor', 250, deleteDurationInMillis) } @Sql([CLEAR_DATA, PERF_TEST_DATA]) -- cgit 1.2.3-korg